From 9d04ddb79bff59f42d49e73f8878a57f3b232b02 Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Wed, 25 May 2016 14:35:56 -0500 Subject: [PATCH 1/7] Resolve #210: [HF] Check authorities on custom_operation The required_auths field on custom_operation was being ignored during authority checking. This commit causes it to be checked correctly, and adds a unit test verifying as much. --- .../include/graphene/protocol/custom.hpp | 3 +++ tests/tests/authority_tests.cpp | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/libraries/protocol/include/graphene/protocol/custom.hpp b/libraries/protocol/include/graphene/protocol/custom.hpp index 8be5ab1e60..ff9912e159 100644 --- a/libraries/protocol/include/graphene/protocol/custom.hpp +++ b/libraries/protocol/include/graphene/protocol/custom.hpp @@ -51,6 +51,9 @@ namespace graphene { namespace protocol { account_id_type fee_payer()const { return payer; } void validate()const; share_type calculate_fee(const fee_parameters_type& k)const; + void get_required_active_authorities( flat_set& auths )const { + auths.insert(required_auths.begin(), required_auths.end()); + } }; } } // namespace graphene::protocol diff --git a/tests/tests/authority_tests.cpp b/tests/tests/authority_tests.cpp index 4ffa93460e..c7efd8f303 100644 --- a/tests/tests/authority_tests.cpp +++ b/tests/tests/authority_tests.cpp @@ -1858,6 +1858,29 @@ BOOST_FIXTURE_TEST_CASE( parent_owner_test, database_fixture ) } } +BOOST_AUTO_TEST_CASE( custom_operation_required_auths ) { + try { + ACTORS((alice)(bob)); + fund(alice); + enable_fees(); + + signed_transaction trx; + custom_operation op; + op.payer = alice_id; + op.required_auths.insert(bob_id); + op.fee = op.calculate_fee(db.current_fee_schedule().get()); + trx.operations.emplace_back(op); + trx.set_expiration(db.head_block_time() + 30); + sign(trx, alice_private_key); + GRAPHENE_REQUIRE_THROW(db.push_transaction(trx), tx_missing_active_auth); + sign(trx, bob_private_key); + PUSH_TX(db, trx); + } catch (fc::exception& e) { + edump((e.to_detail_string())); + throw; + } +} + BOOST_FIXTURE_TEST_CASE( owner_delegation_test, database_fixture ) { try { ACTORS( (alice)(bob) ); From 771e03806d2f83c6ee7ba4a72ccd599decd4e31b Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Fri, 1 Mar 2019 10:44:37 -0600 Subject: [PATCH 2/7] Ref #210: Add hardfork guards Hardfork guards are complex for this issue, because we can't access chain time in the operation's get_required_active_authorities() method. There was no apparent 'good' way to solve this, so I settled for a fairly nasty hack which seemed the least bad of bad options. Add a boolean parameter to the verify_authority() functions determining whether the custom_operation::required_auths field should be ignored or not, and call these functions setting the boolean appropriately for whether we have passed the hardfork or not. This works because chain time is available at the verify_authority() call sites. --- libraries/chain/db_block.cpp | 18 +++-- libraries/chain/hardfork.d/CORE_210.hf | 4 ++ libraries/chain/proposal_object.cpp | 11 +++ .../include/graphene/protocol/transaction.hpp | 33 +++++---- libraries/protocol/transaction.cpp | 39 +++++----- tests/tests/authority_tests.cpp | 71 +++++++++---------- 6 files changed, 102 insertions(+), 74 deletions(-) create mode 100644 libraries/chain/hardfork.d/CORE_210.hf diff --git a/libraries/chain/db_block.cpp b/libraries/chain/db_block.cpp index ace071ec0d..a20acc780e 100644 --- a/libraries/chain/db_block.cpp +++ b/libraries/chain/db_block.cpp @@ -646,11 +646,19 @@ processed_transaction database::_apply_transaction(const signed_transaction& trx bool allow_non_immediate_owner = ( head_block_time() >= HARDFORK_CORE_584_TIME ); auto get_active = [&]( account_id_type id ) { return &id(*this).active; }; auto get_owner = [&]( account_id_type id ) { return &id(*this).owner; }; - trx.verify_authority( chain_id, - get_active, - get_owner, - allow_non_immediate_owner, - get_global_properties().parameters.max_authority_depth ); + + // See bitshares-core issue #210 for discussion + // The custom_operation::get_required_active_authorities() method initially failed to report the authorities + // from the custom_operaton::required_auths field. This was a bug. It's a simple fix in that method, but the + // fix is a hardfork, and thus we need a hardfork guard. Since that method cannot access chain time, we must + // implement the guard here, and skip the call to get_required_active_authorities() prior to the hardfork. + // Therefore, if the head_block_time() is prior to the 210 hardfork, we ignore the required auths specified + // by the custom_operation. + bool ignore_custom_operation_required_auths = (head_block_time() <= HARDFORK_CORE_210_TIME); + + trx.verify_authority(chain_id, get_active, get_owner, allow_non_immediate_owner, + ignore_custom_operation_required_auths, + get_global_properties().parameters.max_authority_depth); } //Skip all manner of expiration and TaPoS checking if we're on block 1; It's impossible that the transaction is diff --git a/libraries/chain/hardfork.d/CORE_210.hf b/libraries/chain/hardfork.d/CORE_210.hf new file mode 100644 index 0000000000..deea4f0e43 --- /dev/null +++ b/libraries/chain/hardfork.d/CORE_210.hf @@ -0,0 +1,4 @@ +// #210 Check authorities on custom_operation +#ifndef HARDFORK_CORE_210_TIME +#define HARDFORK_CORE_210_TIME (fc::time_point_sec(1893456000)) // Jan 1 00:00:00 2030 (Not yet scheduled) +#endif diff --git a/libraries/chain/proposal_object.cpp b/libraries/chain/proposal_object.cpp index 62502e2736..eebd18483b 100644 --- a/libraries/chain/proposal_object.cpp +++ b/libraries/chain/proposal_object.cpp @@ -25,6 +25,7 @@ #include #include #include +#include namespace graphene { namespace chain { @@ -33,12 +34,22 @@ bool proposal_object::is_authorized_to_execute(database& db) const transaction_evaluation_state dry_run_eval(&db); try { + // See bitshares-core issue #210 for discussion + // The custom_operation::get_required_active_authorities() method initially failed to report the authorities + // from the custom_operaton::required_auths field. This was a bug. It's a simple fix in that method, but the + // fix is a hardfork, and thus we need a hardfork guard. Since that method cannot access chain time, we must + // implement the guard here, and skip the call to get_required_active_authorities() prior to the hardfork. + // Therefore, if the head_block_time() is prior to the 210 hardfork, we ignore the required auths specified + // by the custom_operation. + bool ignore_custom_operation_required_auths = (db.head_block_time() <= HARDFORK_CORE_210_TIME); + bool allow_non_immediate_owner = ( db.head_block_time() >= HARDFORK_CORE_584_TIME ); verify_authority( proposed_transaction.operations, available_key_approvals, [&]( account_id_type id ){ return &id(db).active; }, [&]( account_id_type id ){ return &id(db).owner; }, allow_non_immediate_owner, + ignore_custom_operation_required_auths, db.get_global_properties().parameters.max_authority_depth, true, /* allow committee */ available_active_approvals, diff --git a/libraries/protocol/include/graphene/protocol/transaction.hpp b/libraries/protocol/include/graphene/protocol/transaction.hpp index 17b7874d41..5448bbea43 100644 --- a/libraries/protocol/include/graphene/protocol/transaction.hpp +++ b/libraries/protocol/include/graphene/protocol/transaction.hpp @@ -162,15 +162,17 @@ namespace graphene { namespace protocol { * @param get_owner callback function to retrieve owner authorities of a given account * @param allow_non_immediate_owner whether to allow owner authority of non-immediately * required accounts to authorize operations in the transaction + * @param ignore_custom_operation_required_auths See issue #210; whether to ignore the + * required_auths field of custom_operation or not * @param max_recursion maximum level of recursion when verifying, since an account * can have another account in active authorities and/or owner authorities */ - void verify_authority( - const chain_id_type& chain_id, - const std::function& get_active, - const std::function& get_owner, - bool allow_non_immediate_owner, - uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH )const; + void verify_authority(const chain_id_type& chain_id, + const std::function& get_active, + const std::function& get_owner, + bool allow_non_immediate_owner, + bool ignore_custom_operation_required_auths = false, + uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH)const; /** * This is a slower replacement for get_required_signatures() @@ -243,20 +245,23 @@ namespace graphene { namespace protocol { * @param get_owner callback function to retrieve owner authorities of a given account * @param allow_non_immediate_owner whether to allow owner authority of non-immediately * required accounts to authorize operations + * @param ignore_custom_operation_required_auths See issue #210; whether to ignore the + * required_auths field of custom_operation or not * @param max_recursion maximum level of recursion when verifying, since an account * can have another account in active authorities and/or owner authorities * @param allow_committee whether to allow the special "committee account" to authorize the operations * @param active_approvals accounts that approved the operations with their active authories * @param owner_approvals accounts that approved the operations with their owner authories */ - void verify_authority( const vector& ops, const flat_set& sigs, - const std::function& get_active, - const std::function& get_owner, - bool allow_non_immediate_owner, - uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH, - bool allow_committe = false, - const flat_set& active_aprovals = flat_set(), - const flat_set& owner_approvals = flat_set()); + void verify_authority(const vector& ops, const flat_set& sigs, + const std::function& get_active, + const std::function& get_owner, + bool allow_non_immediate_owner, + bool ignore_custom_operation_required_auths = false, + uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH, + bool allow_committe = false, + const flat_set& active_aprovals = flat_set(), + const flat_set& owner_approvals = flat_set()); /** * @brief captures the result of evaluating the operations contained in the transaction diff --git a/libraries/protocol/transaction.cpp b/libraries/protocol/transaction.cpp index 42b1d8ee0f..9b50c6b692 100644 --- a/libraries/protocol/transaction.cpp +++ b/libraries/protocol/transaction.cpp @@ -264,21 +264,26 @@ struct sign_state }; -void verify_authority( const vector& ops, const flat_set& sigs, - const std::function& get_active, - const std::function& get_owner, - bool allow_non_immediate_owner, - uint32_t max_recursion_depth, - bool allow_committe, - const flat_set& active_aprovals, - const flat_set& owner_approvals ) +void verify_authority(const vector& ops, const flat_set& sigs, + const std::function& get_active, + const std::function& get_owner, + bool allow_non_immediate_owner, + bool ignore_custom_operation_required_auths, + uint32_t max_recursion_depth, + bool allow_committe, + const flat_set& active_aprovals, + const flat_set& owner_approvals) { try { flat_set required_active; flat_set required_owner; vector other; - for( const auto& op : ops ) - operation_get_required_authorities( op, required_active, required_owner, other ); + for (const auto& op : ops) { + if (op.which() == operation::tag::value && ignore_custom_operation_required_auths) + required_active.insert(op.get().fee_payer()); + else + operation_get_required_authorities( op, required_active, required_owner, other ); + } if( !allow_committe ) GRAPHENE_ASSERT( required_active.find(GRAPHENE_COMMITTEE_ACCOUNT) == required_active.end(), @@ -429,15 +434,15 @@ const flat_set& precomputable_transaction::get_signature_keys( return _signees; } -void signed_transaction::verify_authority( - const chain_id_type& chain_id, - const std::function& get_active, - const std::function& get_owner, - bool allow_non_immediate_owner, - uint32_t max_recursion )const +void signed_transaction::verify_authority(const chain_id_type& chain_id, + const std::function& get_active, + const std::function& get_owner, + bool allow_non_immediate_owner, + bool ignore_custom_operation_required_auths, + uint32_t max_recursion)const { try { graphene::protocol::verify_authority( operations, get_signature_keys( chain_id ), get_active, get_owner, - allow_non_immediate_owner, max_recursion ); + allow_non_immediate_owner, ignore_custom_operation_required_auths, max_recursion ); } FC_CAPTURE_AND_RETHROW( (*this) ) } } } // graphene::protocol diff --git a/tests/tests/authority_tests.cpp b/tests/tests/authority_tests.cpp index c7efd8f303..15dade43f8 100644 --- a/tests/tests/authority_tests.cpp +++ b/tests/tests/authority_tests.cpp @@ -1858,12 +1858,17 @@ BOOST_FIXTURE_TEST_CASE( parent_owner_test, database_fixture ) } } -BOOST_AUTO_TEST_CASE( custom_operation_required_auths ) { +BOOST_AUTO_TEST_CASE( custom_operation_required_auths_before_fork ) { try { ACTORS((alice)(bob)); fund(alice); enable_fees(); + if (db.head_block_time() >= HARDFORK_CORE_210_TIME) { + wlog("Unable to test custom_operation required auths before fork: hardfork already passed"); + return; + } + signed_transaction trx; custom_operation op; op.payer = alice_id; @@ -1872,52 +1877,42 @@ BOOST_AUTO_TEST_CASE( custom_operation_required_auths ) { trx.operations.emplace_back(op); trx.set_expiration(db.head_block_time() + 30); sign(trx, alice_private_key); - GRAPHENE_REQUIRE_THROW(db.push_transaction(trx), tx_missing_active_auth); - sign(trx, bob_private_key); - PUSH_TX(db, trx); + // Op requires bob's authorization, but only alice signed. We're before the fork, so this should work anyways. + db.push_transaction(trx); } catch (fc::exception& e) { edump((e.to_detail_string())); throw; } } -BOOST_FIXTURE_TEST_CASE( owner_delegation_test, database_fixture ) -{ try { - ACTORS( (alice)(bob) ); - - fc::ecc::private_key bob_active_key = fc::ecc::private_key::regenerate(fc::digest("bob_active")); - fc::ecc::private_key bob_owner_key = fc::ecc::private_key::regenerate(fc::digest("bob_owner")); - - trx.clear(); +BOOST_AUTO_TEST_CASE( custom_operation_required_auths_after_fork ) { + try { + ACTORS((alice)(bob)); + fund(alice); - // Make sure Bob has different keys - account_update_operation auo; - auo.account = bob_id; - auo.active = authority( 1, public_key_type(bob_active_key.get_public_key()), 1 ); - auo.owner = authority( 1, public_key_type(bob_owner_key.get_public_key()), 1 ); - trx.operations.push_back( auo ); - sign( trx, bob_private_key ); - PUSH_TX( db, trx ); - trx.clear(); + if (db.head_block_time() < HARDFORK_CORE_210_TIME) + generate_blocks(HARDFORK_CORE_210_TIME + 10); - // Delegate Alice's owner auth to herself and active auth to Bob - auo.account = alice_id; - auo.active = authority( 1, bob_id, 1 ); - auo.owner = authority( 1, alice_id, 1 ); - trx.operations.push_back( auo ); - sign( trx, alice_private_key ); - PUSH_TX( db, trx ); - trx.clear(); + enable_fees(); - // Now Bob has full control over Alice's account - auo.account = alice_id; - auo.active.reset(); - auo.owner = authority( 1, bob_id, 1 ); - trx.operations.push_back( auo ); - sign( trx, bob_active_key ); - PUSH_TX( db, trx ); - trx.clear(); -} FC_LOG_AND_RETHROW() } + signed_transaction trx; + custom_operation op; + op.payer = alice_id; + op.required_auths.insert(bob_id); + op.fee = op.calculate_fee(db.current_fee_schedule().get()); + trx.operations.emplace_back(op); + trx.set_expiration(db.head_block_time() + 30); + sign(trx, alice_private_key); + // Op require's bob's authorization, but only alice signed. This should throw. + GRAPHENE_REQUIRE_THROW(db.push_transaction(trx), tx_missing_active_auth); + sign(trx, bob_private_key); + // Now that bob has signed, it should work. + PUSH_TX(db, trx); + } catch (fc::exception& e) { + edump((e.to_detail_string())); + throw; + } +} /// This test case reproduces https://github.com/bitshares/bitshares-core/issues/944 /// and https://github.com/bitshares/bitshares-core/issues/580 From 974c8b677a6144cb1e338f6bc6de6b50c97ecb98 Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Sun, 3 Mar 2019 18:17:08 -0600 Subject: [PATCH 3/7] Progress #210: Apply requested changes --- libraries/app/database_api.cpp | 100 +++++---- libraries/chain/db_block.cpp | 11 +- libraries/chain/db_notify.cpp | 61 +++--- libraries/chain/hardfork.d/CORE_210.hf | 2 + .../chain/include/graphene/chain/impacted.hpp | 15 +- .../graphene/chain/proposal_evaluator.hpp | 2 + libraries/chain/proposal_evaluator.cpp | 111 +++++----- libraries/chain/proposal_object.cpp | 11 +- .../account_history_plugin.cpp | 7 +- .../elasticsearch/elasticsearch_plugin.cpp | 7 +- .../include/graphene/protocol/operations.hpp | 9 +- .../include/graphene/protocol/transaction.hpp | 43 ++-- libraries/protocol/operations.cpp | 44 ++-- libraries/protocol/transaction.cpp | 90 ++++---- tests/tests/authority_tests.cpp | 202 ++++++++++++------ 15 files changed, 397 insertions(+), 318 deletions(-) diff --git a/libraries/app/database_api.cpp b/libraries/app/database_api.cpp index 6c12eeecf5..218479410f 100644 --- a/libraries/app/database_api.cpp +++ b/libraries/app/database_api.cpp @@ -2268,65 +2268,71 @@ set
database_api::get_potential_address_signatures( const signed_transa return my->get_potential_address_signatures( trx ); } -set database_api_impl::get_potential_signatures( const signed_transaction& trx )const +set database_api_impl::get_potential_signatures(const signed_transaction& trx) const { - bool allow_non_immediate_owner = ( _db.head_block_time() >= HARDFORK_CORE_584_TIME ); + auto chain_time = _db.head_block_time(); + bool allow_non_immediate_owner = (chain_time >= HARDFORK_CORE_584_TIME); + bool ignore_custom_op_reqd_auths = MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(chain_time); + set result; - trx.get_required_signatures( - _db.get_chain_id(), - flat_set(), - [&]( account_id_type id ) - { - const auto& auth = id(_db).active; - for( const auto& k : auth.get_keys() ) - result.insert(k); - return &auth; - }, - [&]( account_id_type id ) - { - const auto& auth = id(_db).owner; - for( const auto& k : auth.get_keys() ) - result.insert(k); - return &auth; - }, - allow_non_immediate_owner, - _db.get_global_properties().parameters.max_authority_depth - ); + auto get_active = [&](account_id_type id) { + const auto& auth = id(_db).active; + for (const auto& k : auth.get_keys()) + result.insert(k); + return &auth; + }; + auto get_owner = [&](account_id_type id) { + const auto& auth = id(_db).owner; + for (const auto& k : auth.get_keys()) + result.insert(k); + return &auth; + }; + + trx.get_required_signatures(_db.get_chain_id(), + flat_set(), + get_active, get_owner, + allow_non_immediate_owner, + ignore_custom_op_reqd_auths, + _db.get_global_properties().parameters.max_authority_depth); // Insert keys in required "other" authories flat_set required_active; flat_set required_owner; vector other; - trx.get_required_authorities( required_active, required_owner, other ); - for( const auto& auth : other ) - for( const auto& key : auth.get_keys() ) - result.insert( key ); + trx.get_required_authorities(required_active, required_owner, other, ignore_custom_op_reqd_auths); + for (const auto& auth : other) + for (const auto& key : auth.get_keys()) + result.insert(key); return result; } -set
database_api_impl::get_potential_address_signatures( const signed_transaction& trx )const +set
database_api_impl::get_potential_address_signatures(const signed_transaction& trx)const { + auto chain_time = _db.head_block_time(); + bool allow_non_immediate_owner = (chain_time >= HARDFORK_CORE_584_TIME); + bool ignore_custom_op_reqd_auths = MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(chain_time); + set
result; - trx.get_required_signatures( - _db.get_chain_id(), - flat_set(), - [&]( account_id_type id ) - { - const auto& auth = id(_db).active; - for( const auto& k : auth.get_addresses() ) - result.insert(k); - return &auth; - }, - [&]( account_id_type id ) - { - const auto& auth = id(_db).owner; - for( const auto& k : auth.get_addresses() ) - result.insert(k); - return &auth; - }, - _db.get_global_properties().parameters.max_authority_depth - ); + auto get_active = [&](account_id_type id) { + const auto& auth = id(_db).active; + for (const auto& k : auth.get_addresses()) + result.insert(k); + return &auth; + }; + auto get_owner = [&](account_id_type id) { + const auto& auth = id(_db).owner; + for (const auto& k : auth.get_addresses()) + result.insert(k); + return &auth; + }; + + trx.get_required_signatures(_db.get_chain_id(), + flat_set(), + get_active, get_owner, + allow_non_immediate_owner, + ignore_custom_op_reqd_auths, + _db.get_global_properties().parameters.max_authority_depth); return result; } @@ -2365,7 +2371,7 @@ bool database_api_impl::verify_account_authority( const string& account_name_or_ graphene::chain::verify_authority(ops, keys, [this]( account_id_type id ){ return &id(_db).active; }, [this]( account_id_type id ){ return &id(_db).owner; }, - true ); + true, MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(_db.head_block_time()) ); } catch (fc::exception& ex) { diff --git a/libraries/chain/db_block.cpp b/libraries/chain/db_block.cpp index a20acc780e..fe7b86c83a 100644 --- a/libraries/chain/db_block.cpp +++ b/libraries/chain/db_block.cpp @@ -647,17 +647,8 @@ processed_transaction database::_apply_transaction(const signed_transaction& trx auto get_active = [&]( account_id_type id ) { return &id(*this).active; }; auto get_owner = [&]( account_id_type id ) { return &id(*this).owner; }; - // See bitshares-core issue #210 for discussion - // The custom_operation::get_required_active_authorities() method initially failed to report the authorities - // from the custom_operaton::required_auths field. This was a bug. It's a simple fix in that method, but the - // fix is a hardfork, and thus we need a hardfork guard. Since that method cannot access chain time, we must - // implement the guard here, and skip the call to get_required_active_authorities() prior to the hardfork. - // Therefore, if the head_block_time() is prior to the 210 hardfork, we ignore the required auths specified - // by the custom_operation. - bool ignore_custom_operation_required_auths = (head_block_time() <= HARDFORK_CORE_210_TIME); - trx.verify_authority(chain_id, get_active, get_owner, allow_non_immediate_owner, - ignore_custom_operation_required_auths, + MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(head_block_time()), get_global_properties().parameters.max_authority_depth); } diff --git a/libraries/chain/db_notify.cpp b/libraries/chain/db_notify.cpp index d17bec6821..298461c05b 100644 --- a/libraries/chain/db_notify.cpp +++ b/libraries/chain/db_notify.cpp @@ -25,8 +25,13 @@ using namespace graphene::chain; struct get_impacted_account_visitor { flat_set& _impacted; - get_impacted_account_visitor( flat_set& impact ):_impacted(impact) {} - typedef void result_type; + bool _ignore_custom_op_reqd_auths; + + get_impacted_account_visitor(flat_set& impact, bool ignore_custom_operation_required_auths) + : _impacted(impact), _ignore_custom_op_reqd_auths(ignore_custom_operation_required_auths) + {} + + using result_type = void; void operator()( const transfer_operation& op ) { @@ -153,10 +158,10 @@ struct get_impacted_account_visitor { _impacted.insert( op.fee_payer() ); // fee_paying_account vector other; - for( const auto& proposed_op : op.proposed_ops ) - operation_get_required_authorities( proposed_op.op, _impacted, _impacted, other ); - for( auto& o : other ) - add_authority_accounts( _impacted, o ); + for (const auto& proposed_op : op.proposed_ops) + operation_get_required_authorities(proposed_op.op, _impacted, _impacted, other, _ignore_custom_op_reqd_auths); + for (auto& o : other) + add_authority_accounts(_impacted, o); } void operator()( const proposal_update_operation& op ) { @@ -213,7 +218,9 @@ struct get_impacted_account_visitor } void operator()( const custom_operation& op ) { - _impacted.insert( op.fee_payer() ); // payer + _impacted.insert(op.fee_payer()); // payer + if (!_ignore_custom_op_reqd_auths) + _impacted.insert(op.required_auths.begin(), op.required_auths.end()); } void operator()( const assert_operation& op ) { @@ -283,20 +290,17 @@ struct get_impacted_account_visitor } }; -void graphene::chain::operation_get_impacted_accounts( const operation& op, flat_set& result ) -{ - get_impacted_account_visitor vtor = get_impacted_account_visitor( result ); - op.visit( vtor ); +void graphene::chain::operation_get_impacted_accounts(const operation& op, flat_set& result, bool ignore_custom_operation_required_auths) { + get_impacted_account_visitor vtor = get_impacted_account_visitor(result, ignore_custom_operation_required_auths); + op.visit(vtor); } -void graphene::chain::transaction_get_impacted_accounts( const transaction& tx, flat_set& result ) -{ - for( const auto& op : tx.operations ) - operation_get_impacted_accounts( op, result ); +void graphene::chain::transaction_get_impacted_accounts(const transaction& tx, flat_set& result, bool ignore_custom_operation_required_auths) { + for (const auto& op : tx.operations) + operation_get_impacted_accounts(op, result, ignore_custom_operation_required_auths); } -void get_relevant_accounts( const object* obj, flat_set& accounts ) -{ +void get_relevant_accounts(const object* obj, flat_set& accounts, bool ignore_custom_operation_required_auths) { if( obj->id.space() == protocol_ids ) { switch( (object_type)obj->id.type() ) @@ -342,12 +346,14 @@ void get_relevant_accounts( const object* obj, flat_set& accoun } case proposal_object_type:{ const auto& aobj = dynamic_cast(obj); FC_ASSERT( aobj != nullptr ); - transaction_get_impacted_accounts( aobj->proposed_transaction, accounts ); + transaction_get_impacted_accounts(aobj->proposed_transaction, accounts, + ignore_custom_operation_required_auths); break; } case operation_history_object_type:{ const auto& aobj = dynamic_cast(obj); FC_ASSERT( aobj != nullptr ); - operation_get_impacted_accounts( aobj->op, accounts ); + operation_get_impacted_accounts(aobj->op, accounts, + ignore_custom_operation_required_auths); break; } case withdraw_permission_object_type:{ const auto& aobj = dynamic_cast(obj); @@ -404,7 +410,8 @@ void get_relevant_accounts( const object* obj, flat_set& accoun } case impl_transaction_history_object_type:{ const auto& aobj = dynamic_cast(obj); FC_ASSERT( aobj != nullptr ); - transaction_get_impacted_accounts( aobj->trx, accounts ); + transaction_get_impacted_accounts(aobj->trx, accounts, + ignore_custom_operation_required_auths); break; } case impl_blinded_balance_object_type:{ const auto& aobj = dynamic_cast(obj); @@ -458,6 +465,7 @@ void database::notify_changed_objects() if( _undo_db.enabled() ) { const auto& head_undo = _undo_db.head(); + auto chain_time = head_block_time(); // New if( !new_objects.empty() ) @@ -469,7 +477,8 @@ void database::notify_changed_objects() new_ids.push_back(item); auto obj = find_object(item); if(obj != nullptr) - get_relevant_accounts(obj, new_accounts_impacted); + get_relevant_accounts(obj, new_accounts_impacted, + MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(chain_time)); } if( new_ids.size() ) @@ -484,7 +493,8 @@ void database::notify_changed_objects() for( const auto& item : head_undo.old_values ) { changed_ids.push_back(item.first); - get_relevant_accounts(item.second.get(), changed_accounts_impacted); + get_relevant_accounts(item.second.get(), changed_accounts_impacted, + MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(chain_time)); } if( changed_ids.size() ) @@ -502,13 +512,14 @@ void database::notify_changed_objects() removed_ids.emplace_back( item.first ); auto obj = item.second.get(); removed.emplace_back( obj ); - get_relevant_accounts(obj, removed_accounts_impacted); + get_relevant_accounts(obj, removed_accounts_impacted, + MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(chain_time)); } if( removed_ids.size() ) - GRAPHENE_TRY_NOTIFY( removed_objects, removed_ids, removed, removed_accounts_impacted) + GRAPHENE_TRY_NOTIFY(removed_objects, removed_ids, removed, removed_accounts_impacted) } } -} FC_CAPTURE_AND_LOG( (0) ) } +} FC_LOG_AND_RETHROW() } } } diff --git a/libraries/chain/hardfork.d/CORE_210.hf b/libraries/chain/hardfork.d/CORE_210.hf index deea4f0e43..0f28527475 100644 --- a/libraries/chain/hardfork.d/CORE_210.hf +++ b/libraries/chain/hardfork.d/CORE_210.hf @@ -1,4 +1,6 @@ // #210 Check authorities on custom_operation #ifndef HARDFORK_CORE_210_TIME #define HARDFORK_CORE_210_TIME (fc::time_point_sec(1893456000)) // Jan 1 00:00:00 2030 (Not yet scheduled) +// Bugfix: pre-HF 210, custom_operation's required_auths field was ignored. +#define MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(chain_time) (chain_time <= HARDFORK_CORE_210_TIME) #endif diff --git a/libraries/chain/include/graphene/chain/impacted.hpp b/libraries/chain/include/graphene/chain/impacted.hpp index 9d986cb8ab..e5b0ea35b7 100644 --- a/libraries/chain/include/graphene/chain/impacted.hpp +++ b/libraries/chain/include/graphene/chain/impacted.hpp @@ -30,13 +30,12 @@ namespace graphene { namespace chain { -void operation_get_impacted_accounts( - const graphene::chain::operation& op, - fc::flat_set& result ); +void operation_get_impacted_accounts(const graphene::chain::operation& op, + fc::flat_set& result, + bool ignore_custom_operation_required_auths); -void transaction_get_impacted_accounts( - const graphene::chain::transaction& tx, - fc::flat_set& result - ); +void transaction_get_impacted_accounts(const graphene::chain::transaction& tx, + fc::flat_set& result, + bool ignore_custom_operation_required_auths); -} } // graphene::app \ No newline at end of file +} } // graphene::app diff --git a/libraries/chain/include/graphene/chain/proposal_evaluator.hpp b/libraries/chain/include/graphene/chain/proposal_evaluator.hpp index e18ddd4226..454fb7ff48 100644 --- a/libraries/chain/include/graphene/chain/proposal_evaluator.hpp +++ b/libraries/chain/include/graphene/chain/proposal_evaluator.hpp @@ -56,6 +56,8 @@ namespace graphene { namespace chain { object_id_type do_apply( const proposal_create_operation& o ); transaction _proposed_trx; + flat_set _required_active_auths; + flat_set _required_owner_auths; hardfork_visitor_1479 vtor_1479; }; diff --git a/libraries/chain/proposal_evaluator.cpp b/libraries/chain/proposal_evaluator.cpp index 4e9a3ea656..a2033030e2 100644 --- a/libraries/chain/proposal_evaluator.cpp +++ b/libraries/chain/proposal_evaluator.cpp @@ -166,54 +166,54 @@ void_result proposal_create_evaluator::do_evaluate(const proposal_create_operati // Calling the proposal hardfork visitor const fc::time_point_sec block_time = d.head_block_time(); - proposal_operation_hardfork_visitor vtor( d, block_time ); - vtor( o ); - if( block_time < HARDFORK_CORE_214_TIME ) - { // cannot be removed after hf, unfortunately + proposal_operation_hardfork_visitor vtor(d, block_time); + vtor(o); + if (block_time < HARDFORK_CORE_214_TIME) { + // cannot be removed after hf, unfortunately hardfork_visitor_214 hf214; for (const op_wrapper &op : o.proposed_ops) - op.op.visit( hf214 ); + op.op.visit(hf214); } - vtor_1479( o ); + vtor_1479(o); const auto& global_parameters = d.get_global_properties().parameters; - FC_ASSERT( o.expiration_time > block_time, "Proposal has already expired on creation." ); - FC_ASSERT( o.expiration_time <= block_time + global_parameters.maximum_proposal_lifetime, + FC_ASSERT (o.expiration_time > block_time, "Proposal has already expired on creation."); + FC_ASSERT (o.expiration_time <= block_time + global_parameters.maximum_proposal_lifetime, "Proposal expiration time is too far in the future."); - FC_ASSERT( !o.review_period_seconds || fc::seconds(*o.review_period_seconds) < (o.expiration_time - block_time), - "Proposal review period must be less than its overall lifetime." ); - - { - // If we're dealing with the committee authority, make sure this transaction has a sufficient review period. - flat_set auths; - vector other; - for( auto& op : o.proposed_ops ) - { - operation_get_required_authorities(op.op, auths, auths, other); - } - - FC_ASSERT( other.size() == 0 ); // TODO: what about other??? - - if( auths.find(GRAPHENE_COMMITTEE_ACCOUNT) != auths.end() ) - { - GRAPHENE_ASSERT( - o.review_period_seconds.valid(), - proposal_create_review_period_required, - "Review period not given, but at least ${min} required", - ("min", global_parameters.committee_proposal_review_period) - ); - GRAPHENE_ASSERT( - *o.review_period_seconds >= global_parameters.committee_proposal_review_period, - proposal_create_review_period_insufficient, - "Review period of ${t} specified, but at least ${min} required", - ("t", *o.review_period_seconds) - ("min", global_parameters.committee_proposal_review_period) - ); - } + FC_ASSERT (!o.review_period_seconds || fc::seconds(*o.review_period_seconds) < (o.expiration_time - block_time), + "Proposal review period must be less than its overall lifetime."); + + // Find all authorities required by the proposed operations + flat_set tmp_required_active_auths; + vector other; + for (auto& op : o.proposed_ops) { + operation_get_required_authorities(op.op, tmp_required_active_auths, _required_owner_auths, other, + MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(block_time)); + } + // All accounts which must provide both owner and active authority should be omitted from the active authority set; + // owner authority approval implies active authority approval. + std::set_difference(tmp_required_active_auths.begin(), tmp_required_active_auths.end(), + _required_owner_auths.begin(), _required_owner_auths.end(), + std::inserter(_required_active_auths, _required_active_auths.begin())); + + // TODO: what about other??? + FC_ASSERT (other.empty(), "Proposals containing operations requiring non-account authorities are not yet implemented."); + + // If we're dealing with the committee authority, make sure this transaction has a sufficient review period. + if (_required_active_auths.count(GRAPHENE_COMMITTEE_ACCOUNT) || _required_owner_auths.count(GRAPHENE_COMMITTEE_ACCOUNT)) { + GRAPHENE_ASSERT(o.review_period_seconds.valid(), + proposal_create_review_period_required, + "Review period not given, but at least ${min} required", + ("min", global_parameters.committee_proposal_review_period)); + GRAPHENE_ASSERT(*o.review_period_seconds >= global_parameters.committee_proposal_review_period, + proposal_create_review_period_insufficient, + "Review period of ${t} specified, but at least ${min} required", + ("t", *o.review_period_seconds) + ("min", global_parameters.committee_proposal_review_period)); } - for( const op_wrapper& op : o.proposed_ops ) + for (const op_wrapper& op : o.proposed_ops) _proposed_trx.operations.push_back(op.op); _proposed_trx.validate(); @@ -230,35 +230,24 @@ object_id_type proposal_create_evaluator::do_apply(const proposal_create_operati proposal.proposed_transaction = _proposed_trx; proposal.expiration_time = o.expiration_time; proposal.proposer = o.fee_paying_account; - if( o.review_period_seconds ) + if (o.review_period_seconds) proposal.review_period_time = o.expiration_time - *o.review_period_seconds; //Populate the required approval sets - flat_set required_active; - vector other; - - // TODO: consider caching values from evaluate? - for( auto& op : _proposed_trx.operations ) - operation_get_required_authorities(op, required_active, proposal.required_owner_approvals, other); - - //All accounts which must provide both owner and active authority should be omitted from the active authority set; - //owner authority approval implies active authority approval. - std::set_difference(required_active.begin(), required_active.end(), - proposal.required_owner_approvals.begin(), proposal.required_owner_approvals.end(), - std::inserter(proposal.required_active_approvals, proposal.required_active_approvals.begin())); - - if( d.head_block_time() > HARDFORK_CORE_1479_TIME ) - FC_ASSERT( vtor_1479.nested_update_count == 0 || proposal.id.instance() > vtor_1479.max_update_instance, - "Cannot update/delete a proposal with a future id!" ); - else if( vtor_1479.nested_update_count > 0 && proposal.id.instance() <= vtor_1479.max_update_instance ) - { + proposal.required_owner_approvals.insert(_required_owner_auths.begin(), _required_owner_auths.end()); + proposal.required_active_approvals.insert(_required_active_auths.begin(), _required_active_auths.end()); + + if (d.head_block_time() > HARDFORK_CORE_1479_TIME) + FC_ASSERT (vtor_1479.nested_update_count == 0 || proposal.id.instance() > vtor_1479.max_update_instance, + "Cannot update/delete a proposal with a future id!"); + else if (vtor_1479.nested_update_count > 0 && proposal.id.instance() <= vtor_1479.max_update_instance) { // prevent approval transfer_operation top; top.from = GRAPHENE_NULL_ACCOUNT; top.to = GRAPHENE_RELAXED_COMMITTEE_ACCOUNT; - top.amount = asset( GRAPHENE_MAX_SHARE_SUPPLY ); - proposal.proposed_transaction.operations.emplace_back( top ); - wlog( "Issue 1479: ${p}", ("p",proposal) ); + top.amount = asset(GRAPHENE_MAX_SHARE_SUPPLY); + proposal.proposed_transaction.operations.emplace_back(top); + wlog("Issue 1479: ${p}", ("p",proposal)); } }); diff --git a/libraries/chain/proposal_object.cpp b/libraries/chain/proposal_object.cpp index eebd18483b..d87e331d9e 100644 --- a/libraries/chain/proposal_object.cpp +++ b/libraries/chain/proposal_object.cpp @@ -34,22 +34,13 @@ bool proposal_object::is_authorized_to_execute(database& db) const transaction_evaluation_state dry_run_eval(&db); try { - // See bitshares-core issue #210 for discussion - // The custom_operation::get_required_active_authorities() method initially failed to report the authorities - // from the custom_operaton::required_auths field. This was a bug. It's a simple fix in that method, but the - // fix is a hardfork, and thus we need a hardfork guard. Since that method cannot access chain time, we must - // implement the guard here, and skip the call to get_required_active_authorities() prior to the hardfork. - // Therefore, if the head_block_time() is prior to the 210 hardfork, we ignore the required auths specified - // by the custom_operation. - bool ignore_custom_operation_required_auths = (db.head_block_time() <= HARDFORK_CORE_210_TIME); - bool allow_non_immediate_owner = ( db.head_block_time() >= HARDFORK_CORE_584_TIME ); verify_authority( proposed_transaction.operations, available_key_approvals, [&]( account_id_type id ){ return &id(db).active; }, [&]( account_id_type id ){ return &id(db).owner; }, allow_non_immediate_owner, - ignore_custom_operation_required_auths, + MUST_IGNORE_CUSTOM_OP_REQD_AUTHS( db.head_block_time() ), db.get_global_properties().parameters.max_authority_depth, true, /* allow committee */ available_active_approvals, diff --git a/libraries/plugins/account_history/account_history_plugin.cpp b/libraries/plugins/account_history/account_history_plugin.cpp index c3d0826077..95c36a1fe7 100644 --- a/libraries/plugins/account_history/account_history_plugin.cpp +++ b/libraries/plugins/account_history/account_history_plugin.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include @@ -128,12 +129,14 @@ void account_history_plugin_impl::update_account_histories( const signed_block& // get the set of accounts this operation applies to flat_set impacted; vector other; - operation_get_required_authorities( op.op, impacted, impacted, other ); // fee_payer is added here + operation_get_required_authorities(op.op, impacted, impacted, other, + MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(db.head_block_time())); // fee_payer is added here if( op.op.is_type< account_create_operation >() ) impacted.insert( op.result.get() ); else - graphene::chain::operation_get_impacted_accounts( op.op, impacted ); + operation_get_impacted_accounts(op.op, impacted, + MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(db.head_block_time())); for( auto& a : other ) for( auto& item : a.account_auths ) diff --git a/libraries/plugins/elasticsearch/elasticsearch_plugin.cpp b/libraries/plugins/elasticsearch/elasticsearch_plugin.cpp index d6ee6d719a..cd19f0ca5f 100644 --- a/libraries/plugins/elasticsearch/elasticsearch_plugin.cpp +++ b/libraries/plugins/elasticsearch/elasticsearch_plugin.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -154,12 +155,14 @@ bool elasticsearch_plugin_impl::update_account_histories( const signed_block& b // get the set of accounts this operation applies to flat_set impacted; vector other; - operation_get_required_authorities( op.op, impacted, impacted, other ); // fee_payer is added here + operation_get_required_authorities(op.op, impacted, impacted, other, + MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(db.head_block_time())); // fee_payer is added here if( op.op.is_type< account_create_operation >() ) impacted.insert( op.result.get() ); else - graphene::chain::operation_get_impacted_accounts( op.op, impacted ); + operation_get_impacted_accounts(op.op, impacted, + MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(db.head_block_time())); for( auto& a : other ) for( auto& item : a.account_auths ) diff --git a/libraries/protocol/include/graphene/protocol/operations.hpp b/libraries/protocol/include/graphene/protocol/operations.hpp index 3a603ebf64..7d9b45d2c7 100644 --- a/libraries/protocol/include/graphene/protocol/operations.hpp +++ b/libraries/protocol/include/graphene/protocol/operations.hpp @@ -112,10 +112,11 @@ namespace graphene { namespace protocol { * * @return a set of required authorities for @ref op */ - void operation_get_required_authorities( const operation& op, - flat_set& active, - flat_set& owner, - vector& other ); + void operation_get_required_authorities(const operation& op, + flat_set& active, + flat_set& owner, + vector& other, + bool ignore_custom_operation_required_auths); void operation_validate( const operation& op ); diff --git a/libraries/protocol/include/graphene/protocol/transaction.hpp b/libraries/protocol/include/graphene/protocol/transaction.hpp index 5448bbea43..fd63171cc1 100644 --- a/libraries/protocol/include/graphene/protocol/transaction.hpp +++ b/libraries/protocol/include/graphene/protocol/transaction.hpp @@ -109,9 +109,10 @@ namespace graphene { namespace protocol { return results; } - void get_required_authorities( flat_set& active, - flat_set& owner, - vector& other )const; + void get_required_authorities(flat_set& active, + flat_set& owner, + vector& other, + bool ignore_custom_operation_required_auths)const; virtual uint64_t get_packed_size()const; @@ -144,14 +145,13 @@ namespace graphene { namespace protocol { * signatures, but any non-minimal result will still pass * validation. */ - set get_required_signatures( - const chain_id_type& chain_id, - const flat_set& available_keys, - const std::function& get_active, - const std::function& get_owner, - bool allow_non_immediate_owner, - uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH - )const; + set get_required_signatures(const chain_id_type& chain_id, + const flat_set& available_keys, + const std::function& get_active, + const std::function& get_owner, + bool allow_non_immediate_owner, + bool ignore_custom_operation_required_authorities, + uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH)const; /** * Checks whether signatures in this signed transaction are sufficient to authorize the transaction. @@ -171,7 +171,7 @@ namespace graphene { namespace protocol { const std::function& get_active, const std::function& get_owner, bool allow_non_immediate_owner, - bool ignore_custom_operation_required_auths = false, + bool ignore_custom_operation_required_auths, uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH)const; /** @@ -180,14 +180,13 @@ namespace graphene { namespace protocol { * some cases where get_required_signatures() returns a * non-minimal set. */ - set minimize_required_signatures( - const chain_id_type& chain_id, - const flat_set& available_keys, - const std::function& get_active, - const std::function& get_owner, - bool allow_non_immediate_owner, - uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH - ) const; + set minimize_required_signatures(const chain_id_type& chain_id, + const flat_set& available_keys, + const std::function& get_active, + const std::function& get_owner, + bool allow_non_immediate_owner, + bool ignore_custom_operation_required_auths, + uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH) const; /** * @brief Extract public keys from signatures with given chain ID. @@ -257,9 +256,9 @@ namespace graphene { namespace protocol { const std::function& get_active, const std::function& get_owner, bool allow_non_immediate_owner, - bool ignore_custom_operation_required_auths = false, + bool ignore_custom_operation_required_auths, uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH, - bool allow_committe = false, + bool allow_committee = false, const flat_set& active_aprovals = flat_set(), const flat_set& owner_approvals = flat_set()); diff --git a/libraries/protocol/operations.cpp b/libraries/protocol/operations.cpp index 8045ed3261..1c9c08f37d 100644 --- a/libraries/protocol/operations.cpp +++ b/libraries/protocol/operations.cpp @@ -59,24 +59,37 @@ struct operation_validator struct operation_get_required_auth { - typedef void result_type; + using result_type = void; flat_set& active; flat_set& owner; vector& other; + bool ignore_custom_op_reqd_auths; - operation_get_required_auth( flat_set& a, - flat_set& own, - vector& oth ):active(a),owner(own),other(oth){} + operation_get_required_auth(flat_set& a, + flat_set& own, + vector& oth, + bool ignore_custom_operation_required_auths) + : active(a), owner(own), other(oth), + ignore_custom_op_reqd_auths(ignore_custom_operation_required_auths) + {} template - void operator()( const T& v )const - { - active.insert( v.fee_payer() ); - v.get_required_active_authorities( active ); - v.get_required_owner_authorities( owner ); - v.get_required_authorities( other ); + void operator()(const T& v) const { + active.insert(v.fee_payer()); + v.get_required_active_authorities(active); + v.get_required_owner_authorities(owner); + v.get_required_authorities(other); + } + + void operator()(const custom_operation& op) const { + active.insert(op.fee_payer()); + if (!ignore_custom_op_reqd_auths) { + op.get_required_active_authorities(active); + op.get_required_owner_authorities(owner); + op.get_required_authorities(other); + } } }; @@ -85,12 +98,13 @@ void operation_validate( const operation& op ) op.visit( operation_validator() ); } -void operation_get_required_authorities( const operation& op, - flat_set& active, - flat_set& owner, - vector& other ) +void operation_get_required_authorities(const operation& op, + flat_set& active, + flat_set& owner, + vector& other, + bool ignore_custom_operation_required_auths) { - op.visit( operation_get_required_auth( active, owner, other ) ); + op.visit(operation_get_required_auth(active, owner, other, ignore_custom_operation_required_auths)); } } } // namespace graphene::protocol diff --git a/libraries/protocol/transaction.cpp b/libraries/protocol/transaction.cpp index 9b50c6b692..2644e568fa 100644 --- a/libraries/protocol/transaction.cpp +++ b/libraries/protocol/transaction.cpp @@ -99,14 +99,15 @@ void transaction::set_reference_block( const block_id_type& reference_block ) ref_block_prefix = reference_block._hash[1].value(); } -void transaction::get_required_authorities( flat_set& active, - flat_set& owner, - vector& other )const +void transaction::get_required_authorities(flat_set& active, + flat_set& owner, + vector& other, + bool ignore_custom_operation_required_auths)const { - for( const auto& op : operations ) - operation_get_required_authorities( op, active, owner, other ); - for( const auto& account : owner ) - active.erase( account ); + for (const auto& op : operations) + operation_get_required_authorities(op, active, owner, other, ignore_custom_operation_required_auths); + for (const auto& account : owner) + active.erase(account); } @@ -270,7 +271,7 @@ void verify_authority(const vector& ops, const flat_set& active_aprovals, const flat_set& owner_approvals) { try { @@ -279,13 +280,10 @@ void verify_authority(const vector& ops, const flat_set other; for (const auto& op : ops) { - if (op.which() == operation::tag::value && ignore_custom_operation_required_auths) - required_active.insert(op.get().fee_payer()); - else - operation_get_required_authorities( op, required_active, required_owner, other ); + operation_get_required_authorities(op, required_active, required_owner, other, ignore_custom_operation_required_auths); } - if( !allow_committe ) + if( !allow_committee ) GRAPHENE_ASSERT( required_active.find(GRAPHENE_COMMITTEE_ACCOUNT) == required_active.end(), invalid_committee_approval, "Committee account may only propose transactions" ); @@ -340,51 +338,52 @@ const flat_set& signed_transaction::get_signature_keys( const c } FC_CAPTURE_AND_RETHROW() } -set signed_transaction::get_required_signatures( - const chain_id_type& chain_id, - const flat_set& available_keys, - const std::function& get_active, - const std::function& get_owner, - bool allow_non_immediate_owner, - uint32_t max_recursion_depth )const +set signed_transaction::get_required_signatures(const chain_id_type& chain_id, + const flat_set& available_keys, + const std::function& get_active, + const std::function& get_owner, + bool allow_non_immediate_owner, + bool ignore_custom_operation_required_authorities, + uint32_t max_recursion_depth)const { flat_set required_active; flat_set required_owner; vector other; - get_required_authorities( required_active, required_owner, other ); + get_required_authorities(required_active, required_owner, other, ignore_custom_operation_required_authorities); - const flat_set& signature_keys = get_signature_keys( chain_id ); - sign_state s( signature_keys, get_active, get_owner, allow_non_immediate_owner, max_recursion_depth, available_keys ); + const flat_set& signature_keys = get_signature_keys(chain_id); + sign_state s(signature_keys, get_active, get_owner, allow_non_immediate_owner, max_recursion_depth, available_keys); - for( const auto& auth : other ) + for (const auto& auth : other) s.check_authority(&auth); - for( auto& owner : required_owner ) - s.check_authority( get_owner( owner ) ); - for( auto& active : required_active ) - s.check_authority( active ) || s.check_authority( get_owner( active ) ); + for (auto& owner : required_owner) + s.check_authority(get_owner(owner)); + for (auto& active : required_active) + s.check_authority(active) || s.check_authority(get_owner(active)); s.remove_unused_signatures(); set result; - for( auto& provided_sig : s.provided_signatures ) - if( available_keys.find( provided_sig.first ) != available_keys.end() - && signature_keys.find( provided_sig.first ) == signature_keys.end() ) - result.insert( provided_sig.first ); + for (auto& provided_sig : s.provided_signatures) + if (available_keys.find(provided_sig.first) != available_keys.end() + && signature_keys.find(provided_sig.first) == signature_keys.end()) + result.insert(provided_sig.first); return result; } set signed_transaction::minimize_required_signatures( - const chain_id_type& chain_id, - const flat_set& available_keys, - const std::function& get_active, - const std::function& get_owner, - bool allow_non_immediate_owner, - uint32_t max_recursion - ) const + const chain_id_type& chain_id, + const flat_set& available_keys, + const std::function& get_active, + const std::function& get_owner, + bool allow_non_immediate_owner, + bool ignore_custom_operation_required_auths, + uint32_t max_recursion )const { - set< public_key_type > s = get_required_signatures( chain_id, available_keys, get_active, get_owner, max_recursion ); + set< public_key_type > s = get_required_signatures( chain_id, available_keys, get_active, get_owner, + ignore_custom_operation_required_auths, max_recursion ); flat_set< public_key_type > result( s.begin(), s.end() ); for( const public_key_type& k : s ) @@ -393,15 +392,16 @@ set signed_transaction::minimize_required_signatures( try { graphene::protocol::verify_authority( operations, result, get_active, get_owner, + ignore_custom_operation_required_auths, allow_non_immediate_owner, max_recursion ); continue; // element stays erased if verify_authority is ok } - catch( const tx_missing_owner_auth& e ) {} - catch( const tx_missing_active_auth& e ) {} - catch( const tx_missing_other_auth& e ) {} - result.insert( k ); + catch(const tx_missing_owner_auth& e) {} + catch(const tx_missing_active_auth& e) {} + catch(const tx_missing_other_auth& e) {} + result.insert(k); } - return set( result.begin(), result.end() ); + return set(result.begin(), result.end()); } const transaction_id_type& precomputable_transaction::id()const diff --git a/tests/tests/authority_tests.cpp b/tests/tests/authority_tests.cpp index 15dade43f8..5ded8e6e36 100644 --- a/tests/tests/authority_tests.cpp +++ b/tests/tests/authority_tests.cpp @@ -320,7 +320,7 @@ BOOST_AUTO_TEST_CASE( proposed_single_account ) { vector other; flat_set active_set, owner_set; - operation_get_required_authorities(op,active_set,owner_set,other); + operation_get_required_authorities(op, active_set, owner_set, other, false); BOOST_CHECK_EQUAL(active_set.size(), 1lu); BOOST_CHECK_EQUAL(owner_set.size(), 0lu); BOOST_CHECK_EQUAL(other.size(), 0lu); @@ -328,7 +328,7 @@ BOOST_AUTO_TEST_CASE( proposed_single_account ) active_set.clear(); other.clear(); - operation_get_required_authorities(op.proposed_ops.front().op,active_set,owner_set,other); + operation_get_required_authorities(op.proposed_ops.front().op, active_set, owner_set, other, false); BOOST_CHECK_EQUAL(active_set.size(), 1lu); BOOST_CHECK_EQUAL(owner_set.size(), 0lu); BOOST_CHECK_EQUAL(other.size(), 0lu); @@ -1052,7 +1052,7 @@ BOOST_FIXTURE_TEST_CASE( bogus_signature, database_fixture ) flat_set active_set, owner_set; vector others; - trx.get_required_authorities( active_set, owner_set, others ); + trx.get_required_authorities(active_set, owner_set, others, false); PUSH_TX( db, trx, skip ); @@ -1192,10 +1192,10 @@ BOOST_FIXTURE_TEST_CASE( get_required_signatures_test, database_fixture ) ) -> bool { //wdump( (tx)(available_keys) ); - set result_set = tx.get_required_signatures( db.get_chain_id(), available_keys, - get_active, get_owner, false ); - set result_set2 = tx.get_required_signatures( db.get_chain_id(), available_keys, - get_active, get_owner, true ); + set result_set = tx.get_required_signatures(db.get_chain_id(), available_keys, + get_active, get_owner, false, false); + set result_set2 = tx.get_required_signatures(db.get_chain_id(), available_keys, + get_active, get_owner, true, false); //wdump( (result_set)(result_set2)(ref_set) ); return result_set == ref_set && result_set2 == ref_set; } ; @@ -1309,10 +1309,10 @@ BOOST_FIXTURE_TEST_CASE( nonminimal_sig_test, database_fixture ) ) -> bool { //wdump( (tx)(available_keys) ); - set result_set = tx.get_required_signatures( db.get_chain_id(), available_keys, - get_active, get_owner, false ); - set result_set2 = tx.get_required_signatures( db.get_chain_id(), available_keys, - get_active, get_owner, true ); + set result_set = tx.get_required_signatures(db.get_chain_id(), available_keys, + get_active, get_owner, false, false); + set result_set2 = tx.get_required_signatures(db.get_chain_id(), available_keys, + get_active, get_owner, true, false); //wdump( (result_set)(result_set2)(ref_set) ); return result_set == ref_set && result_set2 == ref_set; } ; @@ -1324,10 +1324,10 @@ BOOST_FIXTURE_TEST_CASE( nonminimal_sig_test, database_fixture ) ) -> bool { //wdump( (tx)(available_keys) ); - set result_set = tx.minimize_required_signatures( db.get_chain_id(), available_keys, - get_active, get_owner, false ); - set result_set2 = tx.minimize_required_signatures( db.get_chain_id(), available_keys, - get_active, get_owner, true ); + set result_set = tx.minimize_required_signatures(db.get_chain_id(), available_keys, + get_active, get_owner, false, false); + set result_set2 = tx.minimize_required_signatures(db.get_chain_id(), available_keys, + get_active, get_owner, true, false); //wdump( (result_set)(result_set2)(ref_set) ); return result_set == ref_set && result_set2 == ref_set; } ; @@ -1346,11 +1346,11 @@ BOOST_FIXTURE_TEST_CASE( nonminimal_sig_test, database_fixture ) BOOST_CHECK( chk( tx, { alice_public_key, bob_public_key }, { alice_public_key, bob_public_key } ) ); BOOST_CHECK( chk_min( tx, { alice_public_key, bob_public_key }, { alice_public_key } ) ); - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ), fc::exception ); - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ), fc::exception ); + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ), fc::exception ); + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ), fc::exception ); sign( tx, alice_private_key ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); } catch(fc::exception& e) { @@ -1432,8 +1432,8 @@ BOOST_FIXTURE_TEST_CASE( parent_owner_test, database_fixture ) ) -> bool { //wdump( (tx)(available_keys) ); - set result_set = tx.get_required_signatures( db.get_chain_id(), available_keys, - get_active, get_owner, after_hf_584 ); + set result_set = tx.get_required_signatures(db.get_chain_id(), available_keys, + get_active, get_owner, after_hf_584, false); //wdump( (result_set)(ref_set) ); return result_set == ref_set; } ; @@ -1549,87 +1549,87 @@ BOOST_FIXTURE_TEST_CASE( parent_owner_test, database_fixture ) BOOST_CHECK( chk( tx, true, { gavin_active_pub, gavin_owner_pub }, { gavin_active_pub } ) ); sign( tx, alice_owner_key ); - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ), fc::exception ); + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ), fc::exception ); GRAPHENE_REQUIRE_THROW( PUSH_TX( db, tx, database::skip_transaction_dupe_check ), fc::exception ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); tx.clear_signatures(); sign( tx, alice_active_key ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ); PUSH_TX( db, tx, database::skip_transaction_dupe_check ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); tx.clear_signatures(); sign( tx, bob_owner_key ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ); PUSH_TX( db, tx, database::skip_transaction_dupe_check ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); tx.clear_signatures(); sign( tx, bob_active_key ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ); PUSH_TX( db, tx, database::skip_transaction_dupe_check ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); tx.clear_signatures(); sign( tx, cindy_owner_key ); - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ), fc::exception ); + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ), fc::exception ); GRAPHENE_REQUIRE_THROW( PUSH_TX( db, tx, database::skip_transaction_dupe_check ), fc::exception ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); tx.clear_signatures(); sign( tx, cindy_active_key ); - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ), fc::exception ); + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ), fc::exception ); GRAPHENE_REQUIRE_THROW( PUSH_TX( db, tx, database::skip_transaction_dupe_check ), fc::exception ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); tx.clear_signatures(); sign( tx, daisy_owner_key ); - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ), fc::exception ); + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ), fc::exception ); GRAPHENE_REQUIRE_THROW( PUSH_TX( db, tx, database::skip_transaction_dupe_check ), fc::exception ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); tx.clear_signatures(); sign( tx, daisy_active_key ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ); PUSH_TX( db, tx, database::skip_transaction_dupe_check ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); tx.clear_signatures(); sign( tx, edwin_owner_key ); - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ), fc::exception ); + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ), fc::exception ); GRAPHENE_REQUIRE_THROW( PUSH_TX( db, tx, database::skip_transaction_dupe_check ), fc::exception ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); tx.clear_signatures(); sign( tx, edwin_active_key ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ); PUSH_TX( db, tx, database::skip_transaction_dupe_check ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); tx.clear_signatures(); sign( tx, frank_owner_key ); - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ), fc::exception ); + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ), fc::exception ); GRAPHENE_REQUIRE_THROW( PUSH_TX( db, tx, database::skip_transaction_dupe_check ), fc::exception ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); tx.clear_signatures(); sign( tx, frank_active_key ); - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ), fc::exception ); + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ), fc::exception ); GRAPHENE_REQUIRE_THROW( PUSH_TX( db, tx, database::skip_transaction_dupe_check ), fc::exception ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); tx.clear_signatures(); sign( tx, gavin_owner_key ); - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ), fc::exception ); + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ), fc::exception ); GRAPHENE_REQUIRE_THROW( PUSH_TX( db, tx, database::skip_transaction_dupe_check ), fc::exception ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); tx.clear_signatures(); sign( tx, gavin_active_key ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ); PUSH_TX( db, tx, database::skip_transaction_dupe_check ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); tx.clear_signatures(); // proposal tests @@ -1861,7 +1861,7 @@ BOOST_FIXTURE_TEST_CASE( parent_owner_test, database_fixture ) BOOST_AUTO_TEST_CASE( custom_operation_required_auths_before_fork ) { try { ACTORS((alice)(bob)); - fund(alice); + fund(alice, asset(10000000)); enable_fees(); if (db.head_block_time() >= HARDFORK_CORE_210_TIME) { @@ -1879,6 +1879,34 @@ BOOST_AUTO_TEST_CASE( custom_operation_required_auths_before_fork ) { sign(trx, alice_private_key); // Op requires bob's authorization, but only alice signed. We're before the fork, so this should work anyways. db.push_transaction(trx); + + // Now try the same thing, but with a proposal + proposal_create_operation pcop; + pcop.fee_paying_account = alice_id; + pcop.proposed_ops = {op_wrapper(op)}; + pcop.expiration_time = db.head_block_time() + 10; + pcop.fee = pcop.calculate_fee(db.current_fee_schedule().get()); + trx.operations = {pcop}; + trx.signatures.clear(); + sign(trx, alice_private_key); + proposal_id_type pid = db.push_transaction(trx).operation_results[0].get(); + + // Check bob is not listed as a required approver + BOOST_REQUIRE_EQUAL(pid(db).required_active_approvals.count(bob_id), 0); + + // Add alice's approval + proposal_update_operation puop; + puop.fee_paying_account = alice_id; + puop.proposal = pid; + puop.active_approvals_to_add = {alice_id}; + puop.fee = puop.calculate_fee(db.current_fee_schedule().get()); + trx.operations = {puop}; + trx.signatures.clear(); + sign(trx, alice_private_key); + db.push_transaction(trx); + + // The proposal should have processed. Check it's not still in the database + BOOST_REQUIRE_EQUAL(db.find(pid), nullptr); } catch (fc::exception& e) { edump((e.to_detail_string())); throw; @@ -1888,7 +1916,7 @@ BOOST_AUTO_TEST_CASE( custom_operation_required_auths_before_fork ) { BOOST_AUTO_TEST_CASE( custom_operation_required_auths_after_fork ) { try { ACTORS((alice)(bob)); - fund(alice); + fund(alice, asset(10000000)); if (db.head_block_time() < HARDFORK_CORE_210_TIME) generate_blocks(HARDFORK_CORE_210_TIME + 10); @@ -1908,6 +1936,46 @@ BOOST_AUTO_TEST_CASE( custom_operation_required_auths_after_fork ) { sign(trx, bob_private_key); // Now that bob has signed, it should work. PUSH_TX(db, trx); + + // Now try the same thing, but with a proposal + proposal_create_operation pcop; + pcop.fee_paying_account = alice_id; + pcop.proposed_ops = {op_wrapper(op)}; + pcop.expiration_time = db.head_block_time() + 10; + pcop.fee = pcop.calculate_fee(db.current_fee_schedule().get()); + trx.operations = {pcop}; + trx.signatures.clear(); + sign(trx, alice_private_key); + proposal_id_type pid = db.push_transaction(trx).operation_results[0].get(); + + // Check bob is listed as a required approver + BOOST_REQUIRE_EQUAL(pid(db).required_active_approvals.count(bob_id), 1); + + // Add alice's approval + proposal_update_operation puop; + puop.fee_paying_account = alice_id; + puop.proposal = pid; + puop.active_approvals_to_add = {alice_id}; + puop.fee = puop.calculate_fee(db.current_fee_schedule().get()); + trx.operations = {puop}; + trx.signatures.clear(); + sign(trx, alice_private_key); + db.push_transaction(trx); + + // The proposal should not have processed without bob's approval. + // Check it's still in the database + BOOST_REQUIRE_EQUAL(pid(db).required_active_approvals.count(bob_id), 1); + + // Now add bob's approval + puop.active_approvals_to_add = {bob_id}; + trx.operations = {puop}; + trx.signatures.clear(); + sign(trx, alice_private_key); // Alice still pays fee + sign(trx, bob_private_key); + db.push_transaction(trx); + + // Now the proposal should have processed and been removed from the database + BOOST_REQUIRE_EQUAL(db.find(pid), nullptr); } catch (fc::exception& e) { edump((e.to_detail_string())); throw; @@ -1968,29 +2036,29 @@ BOOST_FIXTURE_TEST_CASE( missing_owner_auth_test, database_fixture ) tx.operations.push_back( op ); // not signed, should throw tx_missing_owner_auth - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ), + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ), graphene::chain::tx_missing_owner_auth ); - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ), + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ), graphene::chain::tx_missing_owner_auth ); // signed with alice's active key, should throw tx_missing_owner_auth sign( tx, alice_active_key ); - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ), + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ), graphene::chain::tx_missing_owner_auth ); - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ), + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ), graphene::chain::tx_missing_owner_auth ); // signed with alice's owner key, should not throw tx.clear_signatures(); sign( tx, alice_owner_key ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); // signed with both alice's owner key and active key, // it does not throw due to https://github.com/bitshares/bitshares-core/issues/580 sign( tx, alice_active_key ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); // creating a transaction that needs active permission tx.clear(); @@ -1999,27 +2067,27 @@ BOOST_FIXTURE_TEST_CASE( missing_owner_auth_test, database_fixture ) tx.operations.push_back( op ); // not signed, should throw tx_missing_active_auth - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ), + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ), graphene::chain::tx_missing_active_auth ); - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ), + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ), graphene::chain::tx_missing_active_auth ); // signed with alice's active key, should not throw sign( tx, alice_active_key ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); // signed with alice's owner key, should not throw tx.clear_signatures(); sign( tx, alice_owner_key ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ); - tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ); + tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ); // signed with both alice's owner key and active key, should throw tx_irrelevant_sig sign( tx, alice_active_key ); - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false ), + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, false, false ), graphene::chain::tx_irrelevant_sig ); - GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, true ), + GRAPHENE_REQUIRE_THROW( tx.verify_authority( db.get_chain_id(), get_active, get_owner, true, false ), graphene::chain::tx_irrelevant_sig ); } From d5503e28f50f619b0f44a2b8d0e7fe4dc00ad509 Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Wed, 6 Mar 2019 11:25:12 -0600 Subject: [PATCH 4/7] Progress #210: Requested changes Mostly polish, but one mistake fix as well. --- libraries/app/database_api.cpp | 8 ++++---- libraries/chain/db_notify.cpp | 2 +- libraries/chain/proposal_evaluator.cpp | 6 ++++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/libraries/app/database_api.cpp b/libraries/app/database_api.cpp index 218479410f..664e629155 100644 --- a/libraries/app/database_api.cpp +++ b/libraries/app/database_api.cpp @@ -2275,13 +2275,13 @@ set database_api_impl::get_potential_signatures(const signed_tr bool ignore_custom_op_reqd_auths = MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(chain_time); set result; - auto get_active = [&](account_id_type id) { + auto get_active = [this, &result](account_id_type id) { const auto& auth = id(_db).active; for (const auto& k : auth.get_keys()) result.insert(k); return &auth; }; - auto get_owner = [&](account_id_type id) { + auto get_owner = [this, &result](account_id_type id) { const auto& auth = id(_db).owner; for (const auto& k : auth.get_keys()) result.insert(k); @@ -2314,13 +2314,13 @@ set
database_api_impl::get_potential_address_signatures(const signed_tr bool ignore_custom_op_reqd_auths = MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(chain_time); set
result; - auto get_active = [&](account_id_type id) { + auto get_active = [this, &result](account_id_type id) { const auto& auth = id(_db).active; for (const auto& k : auth.get_addresses()) result.insert(k); return &auth; }; - auto get_owner = [&](account_id_type id) { + auto get_owner = [this, &result](account_id_type id) { const auto& auth = id(_db).owner; for (const auto& k : auth.get_addresses()) result.insert(k); diff --git a/libraries/chain/db_notify.cpp b/libraries/chain/db_notify.cpp index 298461c05b..e0b46730f6 100644 --- a/libraries/chain/db_notify.cpp +++ b/libraries/chain/db_notify.cpp @@ -520,6 +520,6 @@ void database::notify_changed_objects() GRAPHENE_TRY_NOTIFY(removed_objects, removed_ids, removed, removed_accounts_impacted) } } -} FC_LOG_AND_RETHROW() } +} FC_CAPTURE_AND_LOG( (0) ) } } } diff --git a/libraries/chain/proposal_evaluator.cpp b/libraries/chain/proposal_evaluator.cpp index a2033030e2..4f51e9091d 100644 --- a/libraries/chain/proposal_evaluator.cpp +++ b/libraries/chain/proposal_evaluator.cpp @@ -198,10 +198,12 @@ void_result proposal_create_evaluator::do_evaluate(const proposal_create_operati std::inserter(_required_active_auths, _required_active_auths.begin())); // TODO: what about other??? - FC_ASSERT (other.empty(), "Proposals containing operations requiring non-account authorities are not yet implemented."); + FC_ASSERT (other.empty(), + "Proposals containing operations requiring non-account authorities are not yet implemented."); // If we're dealing with the committee authority, make sure this transaction has a sufficient review period. - if (_required_active_auths.count(GRAPHENE_COMMITTEE_ACCOUNT) || _required_owner_auths.count(GRAPHENE_COMMITTEE_ACCOUNT)) { + if (_required_active_auths.count(GRAPHENE_COMMITTEE_ACCOUNT) || + _required_owner_auths.count(GRAPHENE_COMMITTEE_ACCOUNT)) { GRAPHENE_ASSERT(o.review_period_seconds.valid(), proposal_create_review_period_required, "Review period not given, but at least ${min} required", From a081fff7aa3b49ee39100f621741e357c62cdbdc Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Wed, 13 Mar 2019 10:08:26 -0600 Subject: [PATCH 5/7] Ref #210: More explicit lambda captures --- libraries/chain/db_block.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/chain/db_block.cpp b/libraries/chain/db_block.cpp index fe7b86c83a..2abbca1b2d 100644 --- a/libraries/chain/db_block.cpp +++ b/libraries/chain/db_block.cpp @@ -644,8 +644,8 @@ processed_transaction database::_apply_transaction(const signed_transaction& trx if( !(skip & skip_transaction_signatures) ) { bool allow_non_immediate_owner = ( head_block_time() >= HARDFORK_CORE_584_TIME ); - auto get_active = [&]( account_id_type id ) { return &id(*this).active; }; - auto get_owner = [&]( account_id_type id ) { return &id(*this).owner; }; + auto get_active = [this]( account_id_type id ) { return &id(*this).active; }; + auto get_owner = [this]( account_id_type id ) { return &id(*this).owner; }; trx.verify_authority(chain_id, get_active, get_owner, allow_non_immediate_owner, MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(head_block_time()), From 48a54573458411128cc8871668268b622b74fbf4 Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Wed, 13 Mar 2019 11:03:01 -0600 Subject: [PATCH 6/7] Issue #210: Formatting and build fixes --- libraries/app/database_api.cpp | 74 ++++++------ libraries/chain/db_notify.cpp | 46 ++++---- .../chain/include/graphene/chain/impacted.hpp | 12 +- libraries/chain/proposal_evaluator.cpp | 97 ++++++++-------- libraries/chain/proposal_object.cpp | 10 +- .../account_history_plugin.cpp | 9 +- .../elasticsearch/elasticsearch_plugin.cpp | 9 +- .../include/graphene/protocol/custom.hpp | 2 +- .../include/graphene/protocol/operations.hpp | 10 +- .../include/graphene/protocol/transaction.hpp | 69 ++++++------ libraries/protocol/operations.cpp | 46 ++++---- libraries/protocol/transaction.cpp | 106 +++++++++--------- tests/tests/authority_tests.cpp | 4 +- 13 files changed, 253 insertions(+), 241 deletions(-) diff --git a/libraries/app/database_api.cpp b/libraries/app/database_api.cpp index 664e629155..2e6739814a 100644 --- a/libraries/app/database_api.cpp +++ b/libraries/app/database_api.cpp @@ -2268,71 +2268,71 @@ set
database_api::get_potential_address_signatures( const signed_transa return my->get_potential_address_signatures( trx ); } -set database_api_impl::get_potential_signatures(const signed_transaction& trx) const +set database_api_impl::get_potential_signatures( const signed_transaction& trx )const { auto chain_time = _db.head_block_time(); - bool allow_non_immediate_owner = (chain_time >= HARDFORK_CORE_584_TIME); - bool ignore_custom_op_reqd_auths = MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(chain_time); + bool allow_non_immediate_owner = ( chain_time >= HARDFORK_CORE_584_TIME ); + bool ignore_custom_op_reqd_auths = MUST_IGNORE_CUSTOM_OP_REQD_AUTHS( chain_time ); set result; - auto get_active = [this, &result](account_id_type id) { - const auto& auth = id(_db).active; - for (const auto& k : auth.get_keys()) - result.insert(k); + auto get_active = [this, &result]( account_id_type id ){ + const auto& auth = id( _db ).active; + for( const auto& k : auth.get_keys() ) + result.insert( k ); return &auth; }; - auto get_owner = [this, &result](account_id_type id) { - const auto& auth = id(_db).owner; - for (const auto& k : auth.get_keys()) - result.insert(k); + auto get_owner = [this, &result]( account_id_type id ){ + const auto& auth = id( _db ).owner; + for( const auto& k : auth.get_keys() ) + result.insert( k ); return &auth; }; - trx.get_required_signatures(_db.get_chain_id(), - flat_set(), - get_active, get_owner, - allow_non_immediate_owner, - ignore_custom_op_reqd_auths, - _db.get_global_properties().parameters.max_authority_depth); + trx.get_required_signatures( _db.get_chain_id(), + flat_set(), + get_active, get_owner, + allow_non_immediate_owner, + ignore_custom_op_reqd_auths, + _db.get_global_properties().parameters.max_authority_depth ); // Insert keys in required "other" authories flat_set required_active; flat_set required_owner; vector other; - trx.get_required_authorities(required_active, required_owner, other, ignore_custom_op_reqd_auths); - for (const auto& auth : other) - for (const auto& key : auth.get_keys()) - result.insert(key); + trx.get_required_authorities( required_active, required_owner, other, ignore_custom_op_reqd_auths ); + for( const auto& auth : other ) + for( const auto& key : auth.get_keys() ) + result.insert( key ); return result; } -set
database_api_impl::get_potential_address_signatures(const signed_transaction& trx)const +set
database_api_impl::get_potential_address_signatures( const signed_transaction& trx )const { auto chain_time = _db.head_block_time(); - bool allow_non_immediate_owner = (chain_time >= HARDFORK_CORE_584_TIME); - bool ignore_custom_op_reqd_auths = MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(chain_time); + bool allow_non_immediate_owner = ( chain_time >= HARDFORK_CORE_584_TIME ); + bool ignore_custom_op_reqd_auths = MUST_IGNORE_CUSTOM_OP_REQD_AUTHS( chain_time ); set
result; - auto get_active = [this, &result](account_id_type id) { - const auto& auth = id(_db).active; - for (const auto& k : auth.get_addresses()) - result.insert(k); + auto get_active = [this, &result]( account_id_type id ){ + const auto& auth = id( _db ).active; + for( const auto& k : auth.get_addresses() ) + result.insert( k ); return &auth; }; - auto get_owner = [this, &result](account_id_type id) { - const auto& auth = id(_db).owner; + auto get_owner = [this, &result]( account_id_type id ) { + const auto& auth = id( _db ).owner; for (const auto& k : auth.get_addresses()) - result.insert(k); + result.insert( k ); return &auth; }; - trx.get_required_signatures(_db.get_chain_id(), - flat_set(), - get_active, get_owner, - allow_non_immediate_owner, - ignore_custom_op_reqd_auths, - _db.get_global_properties().parameters.max_authority_depth); + trx.get_required_signatures( _db.get_chain_id(), + flat_set(), + get_active, get_owner, + allow_non_immediate_owner, + ignore_custom_op_reqd_auths, + _db.get_global_properties().parameters.max_authority_depth ); return result; } diff --git a/libraries/chain/db_notify.cpp b/libraries/chain/db_notify.cpp index e0b46730f6..791367267d 100644 --- a/libraries/chain/db_notify.cpp +++ b/libraries/chain/db_notify.cpp @@ -27,8 +27,8 @@ struct get_impacted_account_visitor flat_set& _impacted; bool _ignore_custom_op_reqd_auths; - get_impacted_account_visitor(flat_set& impact, bool ignore_custom_operation_required_auths) - : _impacted(impact), _ignore_custom_op_reqd_auths(ignore_custom_operation_required_auths) + get_impacted_account_visitor( flat_set& impact, bool ignore_custom_operation_required_auths ) + : _impacted( impact ), _ignore_custom_op_reqd_auths( ignore_custom_operation_required_auths ) {} using result_type = void; @@ -158,10 +158,10 @@ struct get_impacted_account_visitor { _impacted.insert( op.fee_payer() ); // fee_paying_account vector other; - for (const auto& proposed_op : op.proposed_ops) - operation_get_required_authorities(proposed_op.op, _impacted, _impacted, other, _ignore_custom_op_reqd_auths); - for (auto& o : other) - add_authority_accounts(_impacted, o); + for( const auto& proposed_op : op.proposed_ops ) + operation_get_required_authorities( proposed_op.op, _impacted, _impacted, other, _ignore_custom_op_reqd_auths ); + for( auto& o : other ) + add_authority_accounts( _impacted, o ); } void operator()( const proposal_update_operation& op ) { @@ -218,9 +218,9 @@ struct get_impacted_account_visitor } void operator()( const custom_operation& op ) { - _impacted.insert(op.fee_payer()); // payer - if (!_ignore_custom_op_reqd_auths) - _impacted.insert(op.required_auths.begin(), op.required_auths.end()); + _impacted.insert( op.fee_payer() ); // payer + if( !_ignore_custom_op_reqd_auths ) + _impacted.insert( op.required_auths.begin(), op.required_auths.end() ); } void operator()( const assert_operation& op ) { @@ -290,17 +290,17 @@ struct get_impacted_account_visitor } }; -void graphene::chain::operation_get_impacted_accounts(const operation& op, flat_set& result, bool ignore_custom_operation_required_auths) { - get_impacted_account_visitor vtor = get_impacted_account_visitor(result, ignore_custom_operation_required_auths); - op.visit(vtor); +void graphene::chain::operation_get_impacted_accounts( const operation& op, flat_set& result, bool ignore_custom_operation_required_auths ) { + get_impacted_account_visitor vtor = get_impacted_account_visitor( result, ignore_custom_operation_required_auths ); + op.visit( vtor ); } -void graphene::chain::transaction_get_impacted_accounts(const transaction& tx, flat_set& result, bool ignore_custom_operation_required_auths) { - for (const auto& op : tx.operations) - operation_get_impacted_accounts(op, result, ignore_custom_operation_required_auths); +void graphene::chain::transaction_get_impacted_accounts( const transaction& tx, flat_set& result, bool ignore_custom_operation_required_auths ) { + for( const auto& op : tx.operations ) + operation_get_impacted_accounts( op, result, ignore_custom_operation_required_auths ); } -void get_relevant_accounts(const object* obj, flat_set& accounts, bool ignore_custom_operation_required_auths) { +void get_relevant_accounts( const object* obj, flat_set& accounts, bool ignore_custom_operation_required_auths ) { if( obj->id.space() == protocol_ids ) { switch( (object_type)obj->id.type() ) @@ -346,14 +346,14 @@ void get_relevant_accounts(const object* obj, flat_set& account } case proposal_object_type:{ const auto& aobj = dynamic_cast(obj); FC_ASSERT( aobj != nullptr ); - transaction_get_impacted_accounts(aobj->proposed_transaction, accounts, - ignore_custom_operation_required_auths); + transaction_get_impacted_accounts( aobj->proposed_transaction, accounts, + ignore_custom_operation_required_auths ); break; } case operation_history_object_type:{ const auto& aobj = dynamic_cast(obj); FC_ASSERT( aobj != nullptr ); - operation_get_impacted_accounts(aobj->op, accounts, - ignore_custom_operation_required_auths); + operation_get_impacted_accounts( aobj->op, accounts, + ignore_custom_operation_required_auths ); break; } case withdraw_permission_object_type:{ const auto& aobj = dynamic_cast(obj); @@ -410,8 +410,8 @@ void get_relevant_accounts(const object* obj, flat_set& account } case impl_transaction_history_object_type:{ const auto& aobj = dynamic_cast(obj); FC_ASSERT( aobj != nullptr ); - transaction_get_impacted_accounts(aobj->trx, accounts, - ignore_custom_operation_required_auths); + transaction_get_impacted_accounts( aobj->trx, accounts, + ignore_custom_operation_required_auths ); break; } case impl_blinded_balance_object_type:{ const auto& aobj = dynamic_cast(obj); @@ -517,7 +517,7 @@ void database::notify_changed_objects() } if( removed_ids.size() ) - GRAPHENE_TRY_NOTIFY(removed_objects, removed_ids, removed, removed_accounts_impacted) + GRAPHENE_TRY_NOTIFY( removed_objects, removed_ids, removed, removed_accounts_impacted ) } } } FC_CAPTURE_AND_LOG( (0) ) } diff --git a/libraries/chain/include/graphene/chain/impacted.hpp b/libraries/chain/include/graphene/chain/impacted.hpp index e5b0ea35b7..6bb43048ac 100644 --- a/libraries/chain/include/graphene/chain/impacted.hpp +++ b/libraries/chain/include/graphene/chain/impacted.hpp @@ -30,12 +30,12 @@ namespace graphene { namespace chain { -void operation_get_impacted_accounts(const graphene::chain::operation& op, - fc::flat_set& result, - bool ignore_custom_operation_required_auths); +void operation_get_impacted_accounts( const graphene::chain::operation& op, + fc::flat_set& result, + bool ignore_custom_operation_required_auths ); -void transaction_get_impacted_accounts(const graphene::chain::transaction& tx, - fc::flat_set& result, - bool ignore_custom_operation_required_auths); +void transaction_get_impacted_accounts( const graphene::chain::transaction& tx, + fc::flat_set& result, + bool ignore_custom_operation_required_auths ); } } // graphene::app diff --git a/libraries/chain/proposal_evaluator.cpp b/libraries/chain/proposal_evaluator.cpp index 4f51e9091d..39f838183a 100644 --- a/libraries/chain/proposal_evaluator.cpp +++ b/libraries/chain/proposal_evaluator.cpp @@ -160,103 +160,108 @@ void hardfork_visitor_1479::operator()(const graphene::chain::proposal_create_op op.op.visit(*this); } -void_result proposal_create_evaluator::do_evaluate(const proposal_create_operation& o) +void_result proposal_create_evaluator::do_evaluate( const proposal_create_operation& o ) { try { const database& d = db(); // Calling the proposal hardfork visitor const fc::time_point_sec block_time = d.head_block_time(); - proposal_operation_hardfork_visitor vtor(d, block_time); - vtor(o); - if (block_time < HARDFORK_CORE_214_TIME) { + proposal_operation_hardfork_visitor vtor( d, block_time ); + vtor( o ); + if( block_time < HARDFORK_CORE_214_TIME ) + { // cannot be removed after hf, unfortunately hardfork_visitor_214 hf214; - for (const op_wrapper &op : o.proposed_ops) - op.op.visit(hf214); + for( const op_wrapper &op : o.proposed_ops ) + op.op.visit( hf214 ); } - vtor_1479(o); + vtor_1479( o ); const auto& global_parameters = d.get_global_properties().parameters; - FC_ASSERT (o.expiration_time > block_time, "Proposal has already expired on creation."); - FC_ASSERT (o.expiration_time <= block_time + global_parameters.maximum_proposal_lifetime, - "Proposal expiration time is too far in the future."); - FC_ASSERT (!o.review_period_seconds || fc::seconds(*o.review_period_seconds) < (o.expiration_time - block_time), - "Proposal review period must be less than its overall lifetime."); + FC_ASSERT( o.expiration_time > block_time, "Proposal has already expired on creation." ); + FC_ASSERT( o.expiration_time <= block_time + global_parameters.maximum_proposal_lifetime, + "Proposal expiration time is too far in the future." ); + FC_ASSERT( !o.review_period_seconds || fc::seconds( *o.review_period_seconds ) < ( o.expiration_time - block_time ), + "Proposal review period must be less than its overall lifetime." ); // Find all authorities required by the proposed operations flat_set tmp_required_active_auths; vector other; - for (auto& op : o.proposed_ops) { - operation_get_required_authorities(op.op, tmp_required_active_auths, _required_owner_auths, other, - MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(block_time)); + for( auto& op : o.proposed_ops ) + { + operation_get_required_authorities( op.op, tmp_required_active_auths, _required_owner_auths, other, + MUST_IGNORE_CUSTOM_OP_REQD_AUTHS( block_time ) ); } // All accounts which must provide both owner and active authority should be omitted from the active authority set; // owner authority approval implies active authority approval. - std::set_difference(tmp_required_active_auths.begin(), tmp_required_active_auths.end(), - _required_owner_auths.begin(), _required_owner_auths.end(), - std::inserter(_required_active_auths, _required_active_auths.begin())); + std::set_difference( tmp_required_active_auths.begin(), tmp_required_active_auths.end(), + _required_owner_auths.begin(), _required_owner_auths.end(), + std::inserter( _required_active_auths, _required_active_auths.begin() ) ); // TODO: what about other??? - FC_ASSERT (other.empty(), - "Proposals containing operations requiring non-account authorities are not yet implemented."); + FC_ASSERT ( other.empty(), + "Proposals containing operations requiring non-account authorities are not yet implemented." ); // If we're dealing with the committee authority, make sure this transaction has a sufficient review period. - if (_required_active_auths.count(GRAPHENE_COMMITTEE_ACCOUNT) || - _required_owner_auths.count(GRAPHENE_COMMITTEE_ACCOUNT)) { - GRAPHENE_ASSERT(o.review_period_seconds.valid(), - proposal_create_review_period_required, - "Review period not given, but at least ${min} required", - ("min", global_parameters.committee_proposal_review_period)); - GRAPHENE_ASSERT(*o.review_period_seconds >= global_parameters.committee_proposal_review_period, - proposal_create_review_period_insufficient, - "Review period of ${t} specified, but at least ${min} required", - ("t", *o.review_period_seconds) - ("min", global_parameters.committee_proposal_review_period)); + if( _required_active_auths.count( GRAPHENE_COMMITTEE_ACCOUNT ) || + _required_owner_auths.count( GRAPHENE_COMMITTEE_ACCOUNT ) ) + { + GRAPHENE_ASSERT( o.review_period_seconds.valid(), + proposal_create_review_period_required, + "Review period not given, but at least ${min} required", + ("min", global_parameters.committee_proposal_review_period) ); + GRAPHENE_ASSERT( *o.review_period_seconds >= global_parameters.committee_proposal_review_period, + proposal_create_review_period_insufficient, + "Review period of ${t} specified, but at least ${min} required", + ("t", *o.review_period_seconds) + ("min", global_parameters.committee_proposal_review_period) ); } - for (const op_wrapper& op : o.proposed_ops) - _proposed_trx.operations.push_back(op.op); + for( const op_wrapper& op : o.proposed_ops ) + _proposed_trx.operations.push_back( op.op ); _proposed_trx.validate(); return void_result(); } FC_CAPTURE_AND_RETHROW( (o) ) } -object_id_type proposal_create_evaluator::do_apply(const proposal_create_operation& o) +object_id_type proposal_create_evaluator::do_apply( const proposal_create_operation& o ) { try { database& d = db(); + auto chain_time = d.head_block_time(); - const proposal_object& proposal = d.create([&](proposal_object& proposal) { + const proposal_object& proposal = d.create( [&o, this, chain_time](proposal_object& proposal) { _proposed_trx.expiration = o.expiration_time; proposal.proposed_transaction = _proposed_trx; proposal.expiration_time = o.expiration_time; proposal.proposer = o.fee_paying_account; - if (o.review_period_seconds) + if( o.review_period_seconds ) proposal.review_period_time = o.expiration_time - *o.review_period_seconds; //Populate the required approval sets - proposal.required_owner_approvals.insert(_required_owner_auths.begin(), _required_owner_auths.end()); - proposal.required_active_approvals.insert(_required_active_auths.begin(), _required_active_auths.end()); + proposal.required_owner_approvals.insert( _required_owner_auths.begin(), _required_owner_auths.end() ); + proposal.required_active_approvals.insert( _required_active_auths.begin(), _required_active_auths.end() ); - if (d.head_block_time() > HARDFORK_CORE_1479_TIME) - FC_ASSERT (vtor_1479.nested_update_count == 0 || proposal.id.instance() > vtor_1479.max_update_instance, - "Cannot update/delete a proposal with a future id!"); - else if (vtor_1479.nested_update_count > 0 && proposal.id.instance() <= vtor_1479.max_update_instance) { + if( chain_time > HARDFORK_CORE_1479_TIME ) + FC_ASSERT( vtor_1479.nested_update_count == 0 || proposal.id.instance() > vtor_1479.max_update_instance, + "Cannot update/delete a proposal with a future id!" ); + else if( vtor_1479.nested_update_count > 0 && proposal.id.instance() <= vtor_1479.max_update_instance ) + { // prevent approval transfer_operation top; top.from = GRAPHENE_NULL_ACCOUNT; top.to = GRAPHENE_RELAXED_COMMITTEE_ACCOUNT; - top.amount = asset(GRAPHENE_MAX_SHARE_SUPPLY); - proposal.proposed_transaction.operations.emplace_back(top); - wlog("Issue 1479: ${p}", ("p",proposal)); + top.amount = asset( GRAPHENE_MAX_SHARE_SUPPLY ); + proposal.proposed_transaction.operations.emplace_back( top ); + wlog( "Issue 1479: ${p}", ("p",proposal) ); } }); return proposal.id; } FC_CAPTURE_AND_RETHROW( (o) ) } -void_result proposal_update_evaluator::do_evaluate(const proposal_update_operation& o) +void_result proposal_update_evaluator::do_evaluate( const proposal_update_operation& o ) { try { database& d = db(); diff --git a/libraries/chain/proposal_object.cpp b/libraries/chain/proposal_object.cpp index d87e331d9e..1b192d8475 100644 --- a/libraries/chain/proposal_object.cpp +++ b/libraries/chain/proposal_object.cpp @@ -29,16 +29,16 @@ namespace graphene { namespace chain { -bool proposal_object::is_authorized_to_execute(database& db) const +bool proposal_object::is_authorized_to_execute( database& db ) const { - transaction_evaluation_state dry_run_eval(&db); + transaction_evaluation_state dry_run_eval( &db ); try { bool allow_non_immediate_owner = ( db.head_block_time() >= HARDFORK_CORE_584_TIME ); - verify_authority( proposed_transaction.operations, + verify_authority( proposed_transaction.operations, available_key_approvals, - [&]( account_id_type id ){ return &id(db).active; }, - [&]( account_id_type id ){ return &id(db).owner; }, + [&db]( account_id_type id ){ return &id( db ).active; }, + [&db]( account_id_type id ){ return &id( db ).owner; }, allow_non_immediate_owner, MUST_IGNORE_CUSTOM_OP_REQD_AUTHS( db.head_block_time() ), db.get_global_properties().parameters.max_authority_depth, diff --git a/libraries/plugins/account_history/account_history_plugin.cpp b/libraries/plugins/account_history/account_history_plugin.cpp index 95c36a1fe7..2a6e62114e 100644 --- a/libraries/plugins/account_history/account_history_plugin.cpp +++ b/libraries/plugins/account_history/account_history_plugin.cpp @@ -129,14 +129,15 @@ void account_history_plugin_impl::update_account_histories( const signed_block& // get the set of accounts this operation applies to flat_set impacted; vector other; - operation_get_required_authorities(op.op, impacted, impacted, other, - MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(db.head_block_time())); // fee_payer is added here + // fee payer is added here + operation_get_required_authorities( op.op, impacted, impacted, other, + MUST_IGNORE_CUSTOM_OP_REQD_AUTHS( db.head_block_time() ) ); if( op.op.is_type< account_create_operation >() ) impacted.insert( op.result.get() ); else - operation_get_impacted_accounts(op.op, impacted, - MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(db.head_block_time())); + operation_get_impacted_accounts( op.op, impacted, + MUST_IGNORE_CUSTOM_OP_REQD_AUTHS( db.head_block_time() ) ); for( auto& a : other ) for( auto& item : a.account_auths ) diff --git a/libraries/plugins/elasticsearch/elasticsearch_plugin.cpp b/libraries/plugins/elasticsearch/elasticsearch_plugin.cpp index cd19f0ca5f..7987192754 100644 --- a/libraries/plugins/elasticsearch/elasticsearch_plugin.cpp +++ b/libraries/plugins/elasticsearch/elasticsearch_plugin.cpp @@ -155,14 +155,15 @@ bool elasticsearch_plugin_impl::update_account_histories( const signed_block& b // get the set of accounts this operation applies to flat_set impacted; vector other; - operation_get_required_authorities(op.op, impacted, impacted, other, - MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(db.head_block_time())); // fee_payer is added here + // fee_payer is added here + operation_get_required_authorities( op.op, impacted, impacted, other, + MUST_IGNORE_CUSTOM_OP_REQD_AUTHS( db.head_block_time() ) ); if( op.op.is_type< account_create_operation >() ) impacted.insert( op.result.get() ); else - operation_get_impacted_accounts(op.op, impacted, - MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(db.head_block_time())); + operation_get_impacted_accounts( op.op, impacted, + MUST_IGNORE_CUSTOM_OP_REQD_AUTHS( db.head_block_time() ) ); for( auto& a : other ) for( auto& item : a.account_auths ) diff --git a/libraries/protocol/include/graphene/protocol/custom.hpp b/libraries/protocol/include/graphene/protocol/custom.hpp index ff9912e159..59ef3757db 100644 --- a/libraries/protocol/include/graphene/protocol/custom.hpp +++ b/libraries/protocol/include/graphene/protocol/custom.hpp @@ -52,7 +52,7 @@ namespace graphene { namespace protocol { void validate()const; share_type calculate_fee(const fee_parameters_type& k)const; void get_required_active_authorities( flat_set& auths )const { - auths.insert(required_auths.begin(), required_auths.end()); + auths.insert( required_auths.begin(), required_auths.end() ); } }; diff --git a/libraries/protocol/include/graphene/protocol/operations.hpp b/libraries/protocol/include/graphene/protocol/operations.hpp index 7d9b45d2c7..f7ddc01a02 100644 --- a/libraries/protocol/include/graphene/protocol/operations.hpp +++ b/libraries/protocol/include/graphene/protocol/operations.hpp @@ -112,11 +112,11 @@ namespace graphene { namespace protocol { * * @return a set of required authorities for @ref op */ - void operation_get_required_authorities(const operation& op, - flat_set& active, - flat_set& owner, - vector& other, - bool ignore_custom_operation_required_auths); + void operation_get_required_authorities( const operation& op, + flat_set& active, + flat_set& owner, + vector& other, + bool ignore_custom_operation_required_auths ); void operation_validate( const operation& op ); diff --git a/libraries/protocol/include/graphene/protocol/transaction.hpp b/libraries/protocol/include/graphene/protocol/transaction.hpp index fd63171cc1..505c3bebef 100644 --- a/libraries/protocol/include/graphene/protocol/transaction.hpp +++ b/libraries/protocol/include/graphene/protocol/transaction.hpp @@ -109,10 +109,10 @@ namespace graphene { namespace protocol { return results; } - void get_required_authorities(flat_set& active, - flat_set& owner, - vector& other, - bool ignore_custom_operation_required_auths)const; + void get_required_authorities( flat_set& active, + flat_set& owner, + vector& other, + bool ignore_custom_operation_required_auths )const; virtual uint64_t get_packed_size()const; @@ -145,13 +145,14 @@ namespace graphene { namespace protocol { * signatures, but any non-minimal result will still pass * validation. */ - set get_required_signatures(const chain_id_type& chain_id, - const flat_set& available_keys, - const std::function& get_active, - const std::function& get_owner, - bool allow_non_immediate_owner, - bool ignore_custom_operation_required_authorities, - uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH)const; + set get_required_signatures( + const chain_id_type& chain_id, + const flat_set& available_keys, + const std::function& get_active, + const std::function& get_owner, + bool allow_non_immediate_owner, + bool ignore_custom_operation_required_authorities, + uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH )const; /** * Checks whether signatures in this signed transaction are sufficient to authorize the transaction. @@ -167,12 +168,13 @@ namespace graphene { namespace protocol { * @param max_recursion maximum level of recursion when verifying, since an account * can have another account in active authorities and/or owner authorities */ - void verify_authority(const chain_id_type& chain_id, - const std::function& get_active, - const std::function& get_owner, - bool allow_non_immediate_owner, - bool ignore_custom_operation_required_auths, - uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH)const; + void verify_authority( + const chain_id_type& chain_id, + const std::function& get_active, + const std::function& get_owner, + bool allow_non_immediate_owner, + bool ignore_custom_operation_required_auths, + uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH )const; /** * This is a slower replacement for get_required_signatures() @@ -180,13 +182,14 @@ namespace graphene { namespace protocol { * some cases where get_required_signatures() returns a * non-minimal set. */ - set minimize_required_signatures(const chain_id_type& chain_id, - const flat_set& available_keys, - const std::function& get_active, - const std::function& get_owner, - bool allow_non_immediate_owner, - bool ignore_custom_operation_required_auths, - uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH) const; + set minimize_required_signatures( + const chain_id_type& chain_id, + const flat_set& available_keys, + const std::function& get_active, + const std::function& get_owner, + bool allow_non_immediate_owner, + bool ignore_custom_operation_required_auths, + uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH) const; /** * @brief Extract public keys from signatures with given chain ID. @@ -252,15 +255,15 @@ namespace graphene { namespace protocol { * @param active_approvals accounts that approved the operations with their active authories * @param owner_approvals accounts that approved the operations with their owner authories */ - void verify_authority(const vector& ops, const flat_set& sigs, - const std::function& get_active, - const std::function& get_owner, - bool allow_non_immediate_owner, - bool ignore_custom_operation_required_auths, - uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH, - bool allow_committee = false, - const flat_set& active_aprovals = flat_set(), - const flat_set& owner_approvals = flat_set()); + void verify_authority( const vector& ops, const flat_set& sigs, + const std::function& get_active, + const std::function& get_owner, + bool allow_non_immediate_owner, + bool ignore_custom_operation_required_auths, + uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH, + bool allow_committee = false, + const flat_set& active_aprovals = flat_set(), + const flat_set& owner_approvals = flat_set() ); /** * @brief captures the result of evaluating the operations contained in the transaction diff --git a/libraries/protocol/operations.cpp b/libraries/protocol/operations.cpp index 1c9c08f37d..dd3ea4961f 100644 --- a/libraries/protocol/operations.cpp +++ b/libraries/protocol/operations.cpp @@ -67,28 +67,28 @@ struct operation_get_required_auth bool ignore_custom_op_reqd_auths; - operation_get_required_auth(flat_set& a, - flat_set& own, - vector& oth, - bool ignore_custom_operation_required_auths) - : active(a), owner(own), other(oth), - ignore_custom_op_reqd_auths(ignore_custom_operation_required_auths) + operation_get_required_auth( flat_set& a, + flat_set& own, + vector& oth, + bool ignore_custom_operation_required_auths ) + : active( a ), owner( own ), other( oth ), + ignore_custom_op_reqd_auths( ignore_custom_operation_required_auths ) {} template - void operator()(const T& v) const { - active.insert(v.fee_payer()); - v.get_required_active_authorities(active); - v.get_required_owner_authorities(owner); - v.get_required_authorities(other); + void operator()( const T& v ) const { + active.insert( v.fee_payer() ); + v.get_required_active_authorities( active ); + v.get_required_owner_authorities( owner ); + v.get_required_authorities( other ); } - void operator()(const custom_operation& op) const { - active.insert(op.fee_payer()); - if (!ignore_custom_op_reqd_auths) { - op.get_required_active_authorities(active); - op.get_required_owner_authorities(owner); - op.get_required_authorities(other); + void operator()( const custom_operation& op ) const { + active.insert( op.fee_payer() ); + if( !ignore_custom_op_reqd_auths ) { + op.get_required_active_authorities( active ); + op.get_required_owner_authorities( owner ); + op.get_required_authorities( other ); } } }; @@ -98,13 +98,13 @@ void operation_validate( const operation& op ) op.visit( operation_validator() ); } -void operation_get_required_authorities(const operation& op, - flat_set& active, - flat_set& owner, - vector& other, - bool ignore_custom_operation_required_auths) +void operation_get_required_authorities( const operation& op, + flat_set& active, + flat_set& owner, + vector& other, + bool ignore_custom_operation_required_auths ) { - op.visit(operation_get_required_auth(active, owner, other, ignore_custom_operation_required_auths)); + op.visit( operation_get_required_auth( active, owner, other, ignore_custom_operation_required_auths ) ); } } } // namespace graphene::protocol diff --git a/libraries/protocol/transaction.cpp b/libraries/protocol/transaction.cpp index 2644e568fa..153bed7ea5 100644 --- a/libraries/protocol/transaction.cpp +++ b/libraries/protocol/transaction.cpp @@ -99,15 +99,15 @@ void transaction::set_reference_block( const block_id_type& reference_block ) ref_block_prefix = reference_block._hash[1].value(); } -void transaction::get_required_authorities(flat_set& active, - flat_set& owner, - vector& other, - bool ignore_custom_operation_required_auths)const +void transaction::get_required_authorities( flat_set& active, + flat_set& owner, + vector& other, + bool ignore_custom_operation_required_auths )const { - for (const auto& op : operations) - operation_get_required_authorities(op, active, owner, other, ignore_custom_operation_required_auths); - for (const auto& account : owner) - active.erase(account); + for( const auto& op : operations ) + operation_get_required_authorities( op, active, owner, other, ignore_custom_operation_required_auths ); + for( const auto& account : owner ) + active.erase( account ); } @@ -265,22 +265,23 @@ struct sign_state }; -void verify_authority(const vector& ops, const flat_set& sigs, - const std::function& get_active, - const std::function& get_owner, - bool allow_non_immediate_owner, - bool ignore_custom_operation_required_auths, - uint32_t max_recursion_depth, - bool allow_committee, - const flat_set& active_aprovals, - const flat_set& owner_approvals) +void verify_authority( const vector& ops, const flat_set& sigs, + const std::function& get_active, + const std::function& get_owner, + bool allow_non_immediate_owner, + bool ignore_custom_operation_required_auths, + uint32_t max_recursion_depth, + bool allow_committee, + const flat_set& active_aprovals, + const flat_set& owner_approvals ) { try { flat_set required_active; flat_set required_owner; vector other; - for (const auto& op : ops) { - operation_get_required_authorities(op, required_active, required_owner, other, ignore_custom_operation_required_auths); + for( const auto& op : ops ) { + operation_get_required_authorities( op, required_active, required_owner, other, + ignore_custom_operation_required_auths ); } if( !allow_committee ) @@ -338,37 +339,37 @@ const flat_set& signed_transaction::get_signature_keys( const c } FC_CAPTURE_AND_RETHROW() } -set signed_transaction::get_required_signatures(const chain_id_type& chain_id, - const flat_set& available_keys, - const std::function& get_active, - const std::function& get_owner, - bool allow_non_immediate_owner, - bool ignore_custom_operation_required_authorities, - uint32_t max_recursion_depth)const +set signed_transaction::get_required_signatures( const chain_id_type& chain_id, + const flat_set& available_keys, + const std::function& get_active, + const std::function& get_owner, + bool allow_non_immediate_owner, + bool ignore_custom_operation_required_authorities, + uint32_t max_recursion_depth )const { flat_set required_active; flat_set required_owner; vector other; - get_required_authorities(required_active, required_owner, other, ignore_custom_operation_required_authorities); + get_required_authorities( required_active, required_owner, other, ignore_custom_operation_required_authorities ); const flat_set& signature_keys = get_signature_keys(chain_id); - sign_state s(signature_keys, get_active, get_owner, allow_non_immediate_owner, max_recursion_depth, available_keys); + sign_state s( signature_keys, get_active, get_owner, allow_non_immediate_owner, max_recursion_depth, available_keys ); - for (const auto& auth : other) - s.check_authority(&auth); - for (auto& owner : required_owner) - s.check_authority(get_owner(owner)); - for (auto& active : required_active) - s.check_authority(active) || s.check_authority(get_owner(active)); + for( const auto& auth : other ) + s.check_authority( &auth ); + for( auto& owner : required_owner ) + s.check_authority( get_owner( owner ) ); + for( auto& active : required_active ) + s.check_authority( active ) || s.check_authority( get_owner( active ) ); s.remove_unused_signatures(); set result; - for (auto& provided_sig : s.provided_signatures) - if (available_keys.find(provided_sig.first) != available_keys.end() - && signature_keys.find(provided_sig.first) == signature_keys.end()) - result.insert(provided_sig.first); + for( auto& provided_sig : s.provided_signatures ) + if( available_keys.find( provided_sig.first ) != available_keys.end() + && signature_keys.find( provided_sig.first ) == signature_keys.end() ) + result.insert( provided_sig.first ); return result; } @@ -392,16 +393,16 @@ set signed_transaction::minimize_required_signatures( try { graphene::protocol::verify_authority( operations, result, get_active, get_owner, - ignore_custom_operation_required_auths, - allow_non_immediate_owner, max_recursion ); + allow_non_immediate_owner,ignore_custom_operation_required_auths, + max_recursion ); continue; // element stays erased if verify_authority is ok } - catch(const tx_missing_owner_auth& e) {} - catch(const tx_missing_active_auth& e) {} - catch(const tx_missing_other_auth& e) {} - result.insert(k); + catch( const tx_missing_owner_auth& e ) {} + catch( const tx_missing_active_auth& e ) {} + catch( const tx_missing_other_auth& e ) {} + result.insert( k ); } - return set(result.begin(), result.end()); + return set( result.begin(), result.end() ); } const transaction_id_type& precomputable_transaction::id()const @@ -434,15 +435,16 @@ const flat_set& precomputable_transaction::get_signature_keys( return _signees; } -void signed_transaction::verify_authority(const chain_id_type& chain_id, - const std::function& get_active, - const std::function& get_owner, - bool allow_non_immediate_owner, - bool ignore_custom_operation_required_auths, - uint32_t max_recursion)const +void signed_transaction::verify_authority( const chain_id_type& chain_id, + const std::function& get_active, + const std::function& get_owner, + bool allow_non_immediate_owner, + bool ignore_custom_operation_required_auths, + uint32_t max_recursion )const { try { graphene::protocol::verify_authority( operations, get_signature_keys( chain_id ), get_active, get_owner, - allow_non_immediate_owner, ignore_custom_operation_required_auths, max_recursion ); + allow_non_immediate_owner, ignore_custom_operation_required_auths, + max_recursion ); } FC_CAPTURE_AND_RETHROW( (*this) ) } } } // graphene::protocol diff --git a/tests/tests/authority_tests.cpp b/tests/tests/authority_tests.cpp index 5ded8e6e36..c5e4031fe3 100644 --- a/tests/tests/authority_tests.cpp +++ b/tests/tests/authority_tests.cpp @@ -1906,7 +1906,7 @@ BOOST_AUTO_TEST_CASE( custom_operation_required_auths_before_fork ) { db.push_transaction(trx); // The proposal should have processed. Check it's not still in the database - BOOST_REQUIRE_EQUAL(db.find(pid), nullptr); + BOOST_REQUIRE(db.find(pid) == nullptr); } catch (fc::exception& e) { edump((e.to_detail_string())); throw; @@ -1975,7 +1975,7 @@ BOOST_AUTO_TEST_CASE( custom_operation_required_auths_after_fork ) { db.push_transaction(trx); // Now the proposal should have processed and been removed from the database - BOOST_REQUIRE_EQUAL(db.find(pid), nullptr); + BOOST_REQUIRE(db.find(pid) == nullptr); } catch (fc::exception& e) { edump((e.to_detail_string())); throw; From e4a10184fb815f20ac176fa2b15d7a28e490bf0c Mon Sep 17 00:00:00 2001 From: Nathan Hourt Date: Sun, 4 Aug 2019 23:07:44 -0500 Subject: [PATCH 7/7] Include hardfork.hpp in db_notify.cpp --- libraries/chain/db_notify.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/chain/db_notify.cpp b/libraries/chain/db_notify.cpp index 791367267d..b3e9933c6d 100644 --- a/libraries/chain/db_notify.cpp +++ b/libraries/chain/db_notify.cpp @@ -17,6 +17,7 @@ #include #include #include +#include using namespace fc; using namespace graphene::chain;