From 56a5e676f0069e55cee6c16b8681fbe8faed1aa1 Mon Sep 17 00:00:00 2001 From: sierra19XX <15652887+sierra19XX@users.noreply.github.com> Date: Fri, 4 Sep 2020 23:56:05 +1000 Subject: [PATCH] Account Roles Permission 4 - Add chain params and limits --- libraries/chain/account_role_evaluator.cpp | 17 +++++++++++++- libraries/chain/db_getter.cpp | 7 +++--- libraries/chain/db_maint.cpp | 22 ++++++++++++++++++- .../graphene/chain/account_role_object.hpp | 9 +++++++- .../chain/include/graphene/chain/config.hpp | 5 ++++- .../graphene/chain/protocol/account_role.hpp | 6 +++-- .../chain/protocol/chain_parameters.hpp | 11 ++++++++++ .../wallet/include/graphene/wallet/wallet.hpp | 2 ++ libraries/wallet/wallet.cpp | 4 ++++ tests/tests/account_role_tests.cpp | 4 ++++ 10 files changed, 78 insertions(+), 9 deletions(-) diff --git a/libraries/chain/account_role_evaluator.cpp b/libraries/chain/account_role_evaluator.cpp index bdb82a22..9c0274e4 100644 --- a/libraries/chain/account_role_evaluator.cpp +++ b/libraries/chain/account_role_evaluator.cpp @@ -29,6 +29,13 @@ namespace graphene { acc(d); } + + FC_ASSERT(op.valid_to > now, "valid_to expiry should be in future"); + FC_ASSERT((op.valid_to - now) <= fc::seconds(d.get_global_properties().parameters.account_roles_max_lifetime()), "Validity of the account role beyond max expiry"); + + const auto &ar_idx = d.get_index_type().indices().get(); + auto aro_range = ar_idx.equal_range(op.owner); + FC_ASSERT(std::distance(aro_range.first, aro_range.second) < d.get_global_properties().parameters.account_roles_max_per_account(), "Max account roles that can be created by one owner is reached"); return void_result(); } FC_CAPTURE_AND_RETHROW((op)) @@ -45,6 +52,7 @@ namespace graphene obj.metadata = op.metadata; obj.allowed_operations = op.allowed_operations; obj.whitelisted_accounts = op.whitelisted_accounts; + obj.valid_to = op.valid_to; }) .id; } @@ -87,6 +95,12 @@ namespace graphene } FC_ASSERT((aobj.whitelisted_accounts.size() + op.accounts_to_add.size() - op.accounts_to_remove.size()) > 0, "Accounts should be positive"); + if (op.valid_to) + { + FC_ASSERT(*op.valid_to > now, "valid_to expiry should be in future"); + FC_ASSERT((*op.valid_to - now) <= fc::seconds(d.get_global_properties().parameters.account_roles_max_lifetime()), "Validity of the account role beyond max expiry"); + } + return void_result(); } FC_CAPTURE_AND_RETHROW((op)) @@ -109,6 +123,8 @@ namespace graphene obj.allowed_operations.erase(op_type); for (const auto &acc : op.accounts_to_remove) obj.whitelisted_accounts.erase(acc); + if (op.valid_to) + obj.valid_to = *op.valid_to; }); return void_result(); } @@ -136,7 +152,6 @@ namespace graphene { database &d = db(); const account_role_object &aobj = op.account_role_id(d); - // TODO: Remove from Resource Objects d.remove(aobj); return void_result(); } diff --git a/libraries/chain/db_getter.cpp b/libraries/chain/db_getter.cpp index 71710695..ccdfa7ad 100644 --- a/libraries/chain/db_getter.cpp +++ b/libraries/chain/db_getter.cpp @@ -194,9 +194,10 @@ bool database::item_locked(const nft_id_type &item) const return (items_itr != market_items._locked_items.end()); } -bool database::account_role_valid(const account_role_object& aro, account_id_type account, optional op_type) const +bool database::account_role_valid(const account_role_object &aro, account_id_type account, optional op_type) const { - return (aro.whitelisted_accounts.find(account) != aro.whitelisted_accounts.end()) && - (!op_type || (aro.allowed_operations.find(*op_type) != aro.allowed_operations.end())); + return (aro.valid_to > head_block_time()) && + (aro.whitelisted_accounts.find(account) != aro.whitelisted_accounts.end()) && + (!op_type || (aro.allowed_operations.find(*op_type) != aro.allowed_operations.end())); } } } diff --git a/libraries/chain/db_maint.cpp b/libraries/chain/db_maint.cpp index 03d2a274..2a1a30ae 100644 --- a/libraries/chain/db_maint.cpp +++ b/libraries/chain/db_maint.cpp @@ -941,6 +941,15 @@ void clear_expired_custom_account_authorities(database& db) } } +void clear_expired_account_roles(database& db) +{ + const auto& arindex = db.get_index_type().indices().get(); + while(!arindex.empty() && arindex.begin()->valid_to < db.head_block_time()) + { + db.remove(*arindex.begin()); + } +} + // Schedules payouts from a dividend distribution account to the current holders of the // dividend-paying asset. This takes any deposits made to the dividend distribution account // since the last time it was called, and distributes them to the current owners of the @@ -1668,6 +1677,16 @@ void database::perform_chain_maintenance(const signed_block& next_block, const g p.pending_parameters->extensions.value.gpos_subperiod = p.parameters.extensions.value.gpos_subperiod; if( !p.pending_parameters->extensions.value.gpos_vesting_lockin_period.valid() ) p.pending_parameters->extensions.value.gpos_vesting_lockin_period = p.parameters.extensions.value.gpos_vesting_lockin_period; + if( !p.pending_parameters->extensions.value.rbac_max_permissions_per_account.valid() ) + p.pending_parameters->extensions.value.rbac_max_permissions_per_account = p.parameters.extensions.value.rbac_max_permissions_per_account; + if( !p.pending_parameters->extensions.value.rbac_max_account_authority_lifetime.valid() ) + p.pending_parameters->extensions.value.rbac_max_account_authority_lifetime = p.parameters.extensions.value.rbac_max_account_authority_lifetime; + if( !p.pending_parameters->extensions.value.rbac_max_authorities_per_permission.valid() ) + p.pending_parameters->extensions.value.rbac_max_authorities_per_permission = p.parameters.extensions.value.rbac_max_authorities_per_permission; + if( !p.pending_parameters->extensions.value.account_roles_max_per_account.valid() ) + p.pending_parameters->extensions.value.account_roles_max_per_account = p.parameters.extensions.value.account_roles_max_per_account; + if( !p.pending_parameters->extensions.value.account_roles_max_lifetime.valid() ) + p.pending_parameters->extensions.value.account_roles_max_lifetime = p.parameters.extensions.value.account_roles_max_lifetime; p.parameters = std::move(*p.pending_parameters); p.pending_parameters.reset(); } @@ -1717,8 +1736,9 @@ void database::perform_chain_maintenance(const signed_block& next_block, const g modify( d, [](asset_bitasset_data_object& o) { o.force_settled_volume = 0; }); // Ideally we have to do this after every block but that leads to longer block applicaiton/replay times. // So keep it here as it is not critical. valid_to check ensures - // these custom account auths are not usable. + // these custom account auths and account roles are not usable. clear_expired_custom_account_authorities(*this); + clear_expired_account_roles(*this); // process_budget needs to run at the bottom because // it needs to know the next_maintenance_time process_budget(); diff --git a/libraries/chain/include/graphene/chain/account_role_object.hpp b/libraries/chain/include/graphene/chain/account_role_object.hpp index 7ba14301..9455f475 100644 --- a/libraries/chain/include/graphene/chain/account_role_object.hpp +++ b/libraries/chain/include/graphene/chain/account_role_object.hpp @@ -20,9 +20,11 @@ namespace graphene std::string metadata; flat_set allowed_operations; flat_set whitelisted_accounts; + time_point_sec valid_to; }; struct by_owner; + struct by_expiration; using account_role_multi_index_type = multi_index_container< account_role_object, indexed_by< @@ -31,6 +33,11 @@ namespace graphene >, ordered_non_unique< tag, member + >, + ordered_unique< tag, + composite_key, + member> > > >; @@ -39,4 +46,4 @@ namespace graphene } // namespace graphene FC_REFLECT_DERIVED(graphene::chain::account_role_object, (graphene::db::object), - (owner)(name)(metadata)(allowed_operations)(whitelisted_accounts)) \ No newline at end of file + (owner)(name)(metadata)(allowed_operations)(whitelisted_accounts)(valid_to)) \ No newline at end of file diff --git a/libraries/chain/include/graphene/chain/config.hpp b/libraries/chain/include/graphene/chain/config.hpp index 42c2fd13..a166aad1 100644 --- a/libraries/chain/include/graphene/chain/config.hpp +++ b/libraries/chain/include/graphene/chain/config.hpp @@ -243,4 +243,7 @@ #define NFT_TOKEN_MIN_LENGTH 3 #define NFT_TOKEN_MAX_LENGTH 15 -#define NFT_URI_MAX_LENGTH GRAPHENE_MAX_URL_LENGTH \ No newline at end of file +#define NFT_URI_MAX_LENGTH GRAPHENE_MAX_URL_LENGTH + +#define ACCOUNT_ROLES_MAX_PER_ACCOUNT 20 // Max 20 roles can be created by a resource owner +#define ACCOUNT_ROLES_MAX_LIFETIME 365*24*60*60 // 1 Year \ No newline at end of file diff --git a/libraries/chain/include/graphene/chain/protocol/account_role.hpp b/libraries/chain/include/graphene/chain/protocol/account_role.hpp index 719a16f1..6a532cb7 100644 --- a/libraries/chain/include/graphene/chain/protocol/account_role.hpp +++ b/libraries/chain/include/graphene/chain/protocol/account_role.hpp @@ -21,6 +21,7 @@ namespace graphene std::string metadata; flat_set allowed_operations; flat_set whitelisted_accounts; + time_point_sec valid_to; extensions_type extensions; account_id_type fee_payer() const { return owner; } @@ -45,6 +46,7 @@ namespace graphene flat_set allowed_operations_to_remove; flat_set accounts_to_add; flat_set accounts_to_remove; + optional valid_to; extensions_type extensions; account_id_type fee_payer() const { return owner; } @@ -75,6 +77,6 @@ FC_REFLECT(graphene::chain::account_role_create_operation::fee_parameters_type, FC_REFLECT(graphene::chain::account_role_update_operation::fee_parameters_type, (fee)(price_per_kbyte)) FC_REFLECT(graphene::chain::account_role_delete_operation::fee_parameters_type, (fee)) -FC_REFLECT(graphene::chain::account_role_create_operation, (fee)(owner)(name)(metadata)(allowed_operations)(whitelisted_accounts)(extensions)) -FC_REFLECT(graphene::chain::account_role_update_operation, (fee)(owner)(account_role_id)(name)(metadata)(allowed_operations_to_add)(allowed_operations_to_remove)(accounts_to_add)(accounts_to_remove)(extensions)) +FC_REFLECT(graphene::chain::account_role_create_operation, (fee)(owner)(name)(metadata)(allowed_operations)(whitelisted_accounts)(valid_to)(extensions)) +FC_REFLECT(graphene::chain::account_role_update_operation, (fee)(owner)(account_role_id)(name)(metadata)(allowed_operations_to_add)(allowed_operations_to_remove)(accounts_to_add)(accounts_to_remove)(valid_to)(extensions)) FC_REFLECT(graphene::chain::account_role_delete_operation, (fee)(owner)(account_role_id)(owner)(extensions)) diff --git a/libraries/chain/include/graphene/chain/protocol/chain_parameters.hpp b/libraries/chain/include/graphene/chain/protocol/chain_parameters.hpp index ddac3115..dbd8ec4e 100644 --- a/libraries/chain/include/graphene/chain/protocol/chain_parameters.hpp +++ b/libraries/chain/include/graphene/chain/protocol/chain_parameters.hpp @@ -52,6 +52,9 @@ namespace graphene { namespace chain { optional < uint16_t > rbac_max_permissions_per_account = RBAC_MAX_PERMISSIONS_PER_ACCOUNT; optional < uint32_t > rbac_max_account_authority_lifetime = RBAC_MAX_ACCOUNT_AUTHORITY_LIFETIME; optional < uint16_t > rbac_max_authorities_per_permission = RBAC_MAX_AUTHS_PER_PERMISSION; + /* Account Roles - Permissions Parameters */ + optional < uint16_t > account_roles_max_per_account = ACCOUNT_ROLES_MAX_PER_ACCOUNT; + optional < uint32_t > account_roles_max_lifetime = ACCOUNT_ROLES_MAX_LIFETIME; }; struct chain_parameters @@ -152,6 +155,12 @@ namespace graphene { namespace chain { inline uint16_t rbac_max_authorities_per_permission()const { return extensions.value.rbac_max_authorities_per_permission.valid() ? *extensions.value.rbac_max_authorities_per_permission : RBAC_MAX_AUTHS_PER_PERMISSION; } + inline uint16_t account_roles_max_per_account()const { + return extensions.value.account_roles_max_per_account.valid() ? *extensions.value.account_roles_max_per_account : ACCOUNT_ROLES_MAX_PER_ACCOUNT; + } + inline uint32_t account_roles_max_lifetime()const { + return extensions.value.account_roles_max_lifetime.valid() ? *extensions.value.account_roles_max_lifetime : ACCOUNT_ROLES_MAX_LIFETIME; + } }; } } // graphene::chain @@ -172,6 +181,8 @@ FC_REFLECT( graphene::chain::parameter_extension, (rbac_max_permissions_per_account) (rbac_max_account_authority_lifetime) (rbac_max_authorities_per_permission) + (account_roles_max_per_account) + (account_roles_max_lifetime) ) FC_REFLECT( graphene::chain::chain_parameters, diff --git a/libraries/wallet/include/graphene/wallet/wallet.hpp b/libraries/wallet/include/graphene/wallet/wallet.hpp index 40dd12cd..d37394b6 100644 --- a/libraries/wallet/include/graphene/wallet/wallet.hpp +++ b/libraries/wallet/include/graphene/wallet/wallet.hpp @@ -2120,6 +2120,7 @@ class wallet_api string metadata, flat_set allowed_operations, flat_set whitelisted_accounts, + time_point_sec valid_to, bool broadcast); signed_transaction update_account_role(string owner_account_id_or_name, account_role_id_type role_id, @@ -2129,6 +2130,7 @@ class wallet_api flat_set operations_to_remove, flat_set accounts_to_add, flat_set accounts_to_remove, + optional valid_to, bool broadcast); signed_transaction delete_account_role(string owner_account_id_or_name, account_role_id_type role_id, diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index bbaa41ac..b253f191 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -6729,6 +6729,7 @@ signed_transaction wallet_api::create_account_role(string owner_account_id_or_na string metadata, flat_set allowed_operations, flat_set whitelisted_accounts, + time_point_sec valid_to, bool broadcast) { account_object owner_account = my->get_account(owner_account_id_or_name); @@ -6739,6 +6740,7 @@ signed_transaction wallet_api::create_account_role(string owner_account_id_or_na op.metadata = metadata; op.allowed_operations = allowed_operations; op.whitelisted_accounts = whitelisted_accounts; + op.valid_to = valid_to; signed_transaction trx; trx.operations.push_back(op); @@ -6756,6 +6758,7 @@ signed_transaction wallet_api::update_account_role(string owner_account_id_or_na flat_set operations_to_remove, flat_set accounts_to_add, flat_set accounts_to_remove, + optional valid_to, bool broadcast) { account_object owner_account = my->get_account(owner_account_id_or_name); @@ -6769,6 +6772,7 @@ signed_transaction wallet_api::update_account_role(string owner_account_id_or_na op.allowed_operations_to_remove = operations_to_remove; op.accounts_to_add = accounts_to_add; op.accounts_to_remove = accounts_to_remove; + op.valid_to = valid_to; signed_transaction trx; trx.operations.push_back(op); diff --git a/tests/tests/account_role_tests.cpp b/tests/tests/account_role_tests.cpp index f6e6a46b..73824678 100644 --- a/tests/tests/account_role_tests.cpp +++ b/tests/tests/account_role_tests.cpp @@ -61,6 +61,7 @@ BOOST_AUTO_TEST_CASE(account_role_create_test) op.allowed_operations.insert(ops, ops + 6); op.whitelisted_accounts.emplace(alice_id); op.whitelisted_accounts.emplace(bob_id); + op.valid_to = db.head_block_time() + 1000; trx.operations.push_back(op); sign(trx, resourceowner_private_key); @@ -83,6 +84,7 @@ BOOST_AUTO_TEST_CASE(account_role_create_test) BOOST_CHECK(obj->allowed_operations == expected_allowed_operations); flat_set expected_whitelisted_accounts = {alice_id, bob_id}; BOOST_CHECK(obj->whitelisted_accounts == expected_whitelisted_accounts); + BOOST_CHECK(obj->valid_to == db.head_block_time() + 1000); } FC_LOG_AND_RETHROW() } @@ -111,6 +113,7 @@ BOOST_AUTO_TEST_CASE(account_role_update_test) int ops_delete[] = {operation::tag::value, operation::tag::value}; op.allowed_operations_to_add.insert(ops_add, ops_add + 1); op.allowed_operations_to_remove.insert(ops_delete, ops_delete + 2); + op.valid_to = db.head_block_time() + 10000; trx.operations.push_back(op); sign(trx, resourceowner_private_key); @@ -132,6 +135,7 @@ BOOST_AUTO_TEST_CASE(account_role_update_test) BOOST_CHECK(obj->allowed_operations == expected_allowed_operations); flat_set expected_whitelisted_accounts = {alice_id, bob_id}; BOOST_CHECK(obj->whitelisted_accounts == expected_whitelisted_accounts); + BOOST_CHECK(obj->valid_to == db.head_block_time() + 10000); } FC_LOG_AND_RETHROW() }