From b08b6cb55340776d922a9cdd94445acab408cb8d Mon Sep 17 00:00:00 2001 From: Daniel Larimer Date: Thu, 16 Jul 2015 18:13:11 -0400 Subject: [PATCH 1/2] Partial work toward auth refactor --- libraries/chain/db_block.cpp | 14 +- libraries/chain/evaluator.cpp | 4 +- .../include/graphene/chain/evaluator.hpp | 4 +- .../graphene/chain/protocol/transaction.hpp | 30 +-- .../chain/transaction_evaluation_state.hpp | 8 +- libraries/chain/proposal_evaluator.cpp | 2 +- libraries/chain/proposal_object.cpp | 18 +- libraries/chain/protocol/transaction.cpp | 180 +++++++++++------- .../chain/transaction_evaluation_state.cpp | 2 + 9 files changed, 164 insertions(+), 98 deletions(-) diff --git a/libraries/chain/db_block.cpp b/libraries/chain/db_block.cpp index b08b0959..68c0fa03 100644 --- a/libraries/chain/db_block.cpp +++ b/libraries/chain/db_block.cpp @@ -216,6 +216,7 @@ processed_transaction database::push_proposal(const proposal_object& proposal) eval_state._is_proposed_trx = true; //Inject the approving authorities into the transaction eval state + /* std::transform(proposal.required_active_approvals.begin(), proposal.required_active_approvals.end(), std::inserter(eval_state.approved_by, eval_state.approved_by.begin()), @@ -228,6 +229,7 @@ processed_transaction database::push_proposal(const proposal_object& proposal) []( account_id_type id ) { return std::make_pair(id, authority::owner); }); + */ eval_state.operation_results.reserve(proposal.proposed_transaction.operations.size()); processed_transaction ptrx(proposal.proposed_transaction); @@ -482,8 +484,13 @@ processed_transaction database::_apply_transaction(const signed_transaction& trx const chain_parameters& chain_parameters = get_global_properties().parameters; eval_state._trx = &trx; - if( !(skip & skip_transaction_signatures) ) + if( !(skip & (skip_transaction_signatures | skip_authority_check) ) ) { + 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( get_active, get_owner ); + + /* eval_state._sigs.reserve(trx.signatures.size()); for( const auto& sig : trx.signatures ) @@ -492,6 +499,7 @@ processed_transaction database::_apply_transaction(const signed_transaction& trx false)).second, "Multiple signatures by same key detected" ); } + */ } //Skip all manner of expiration and TaPoS checking if we're on block 1; It's impossible that the transaction is @@ -538,6 +546,7 @@ processed_transaction database::_apply_transaction(const signed_transaction& trx std::for_each(range.first, range.second, [](const account_balance_object& b) { FC_ASSERT(b.balance == 0); }); //Make sure all signatures were needed to validate the transaction + /* if( !(skip & (skip_transaction_signatures|skip_authority_check)) ) { for( const auto& item : eval_state._sigs ) @@ -545,6 +554,7 @@ processed_transaction database::_apply_transaction(const signed_transaction& trx FC_ASSERT( item.second, "All signatures must be used", ("item",item) ); } } + */ return ptrx; } FC_CAPTURE_AND_RETHROW( (trx) ) } @@ -564,7 +574,7 @@ operation_result database::apply_operation(transaction_evaluation_state& eval_st auto result = eval->evaluate( eval_state, op, true ); set_applied_operation_result( op_id, result ); return result; -} FC_CAPTURE_AND_RETHROW( (eval_state._sigs) ) } +} FC_CAPTURE_AND_RETHROW( ) } const witness_object& database::validate_block_header( uint32_t skip, const signed_block& next_block )const { diff --git a/libraries/chain/evaluator.cpp b/libraries/chain/evaluator.cpp index 13d78646..b396611e 100644 --- a/libraries/chain/evaluator.cpp +++ b/libraries/chain/evaluator.cpp @@ -34,7 +34,7 @@ database& generic_evaluator::db()const { return trx_state->db(); } operation_result generic_evaluator::start_evaluate( transaction_evaluation_state& eval_state, const operation& op, bool apply ) { try { trx_state = &eval_state; - check_required_authorities(op); + //check_required_authorities(op); auto result = evaluate( op ); if( apply ) result = this->apply( op ); @@ -77,6 +77,7 @@ database& generic_evaluator::db()const { return trx_state->db(); } }); } FC_CAPTURE_AND_RETHROW() } + /* bool generic_evaluator::verify_authority( const account_object& a, authority::classification c ) { try { return trx_state->check_authority( a, c ); @@ -117,6 +118,7 @@ database& generic_evaluator::db()const { return trx_state->db(); } } } FC_CAPTURE_AND_RETHROW( (op) ) } + */ void generic_evaluator::verify_authority_accounts( const authority& a )const { diff --git a/libraries/chain/include/graphene/chain/evaluator.hpp b/libraries/chain/include/graphene/chain/evaluator.hpp index 74d31378..218d2224 100644 --- a/libraries/chain/include/graphene/chain/evaluator.hpp +++ b/libraries/chain/include/graphene/chain/evaluator.hpp @@ -84,7 +84,7 @@ namespace graphene { namespace chain { database& db()const; - void check_required_authorities(const operation& op); + //void check_required_authorities(const operation& op); protected: /** * @brief Fetch objects relevant to fee payer and set pointer members @@ -98,7 +98,7 @@ namespace graphene { namespace chain { /// Pays the fee and returns the number of CORE asset that were paid. void pay_fee(); - bool verify_authority(const account_object&, authority::classification); + //bool verify_authority(const account_object&, authority::classification); object_id_type get_relative_id( object_id_type rel_id )const; void verify_authority_accounts( const authority& a )const; diff --git a/libraries/chain/include/graphene/chain/protocol/transaction.hpp b/libraries/chain/include/graphene/chain/protocol/transaction.hpp index 4ee3ca88..30e373d1 100644 --- a/libraries/chain/include/graphene/chain/protocol/transaction.hpp +++ b/libraries/chain/include/graphene/chain/protocol/transaction.hpp @@ -118,23 +118,17 @@ namespace graphene { namespace chain { /** * The purpose of this method is to identify the minimal subset of @ref available_keys that are - * required to sign + * required to sign given the signatures that are already provided. */ - set get_required_signatures( const set& available_keys, - const map& active_authorities, - const map& owner_authorities )const; + set get_required_signatures( const flat_set& available_keys, + const std::function& get_active, + const std::function& get_owner + )const; - /** - * Given a set of private keys sign this transaction with a minimial subset of required keys. - * - * @pram get_auth - used to fetch the active authority required for an account referenced by another authority - */ - void sign( const vector& keys, - const std::function& get_active, - const std::function& get_owner ); - - bool verify( const std::function& get_active, - const std::function& get_owner )const; + void verify_authority( const std::function& get_active, + const std::function& get_owner )const; + + flat_set get_signature_keys()const; vector signatures; @@ -142,6 +136,12 @@ namespace graphene { namespace chain { void clear() { operations.clear(); signatures.clear(); } }; + void verify_authority( const vector& ops, const flat_set& sigs, + const std::function& get_active, + const std::function& get_owner, + 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/chain/include/graphene/chain/transaction_evaluation_state.hpp b/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp index 6054763c..027beb9a 100644 --- a/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp +++ b/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp @@ -32,6 +32,7 @@ namespace graphene { namespace chain { transaction_evaluation_state( database* db = nullptr ) :_db(db){} + /* bool check_authority(const account_object&, authority::classification auth_class = authority::active, int depth = 0); @@ -39,14 +40,17 @@ namespace graphene { namespace chain { bool check_authority(const authority&, authority::classification auth_class = authority::active, int depth = 0); + */ - database& db()const { FC_ASSERT( _db ); return *_db; } + database& db()const { assert( _db ); return *_db; } + /* bool signed_by(const public_key_type& k); bool signed_by(const address& k); /// cached approval (accounts and keys) flat_set> approved_by; + */ /// Used to look up new objects using transaction relative IDs vector operation_results; @@ -55,7 +59,7 @@ namespace graphene { namespace chain { * When an address is referenced via check authority it is flagged as being used, all addresses must be * flagged as being used or the transaction will fail. */ - flat_map _sigs; + // flat_map _sigs; const signed_transaction* _trx = nullptr; database* _db = nullptr; bool _is_proposed_trx = false; diff --git a/libraries/chain/proposal_evaluator.cpp b/libraries/chain/proposal_evaluator.cpp index 613b7f59..4e9341f2 100644 --- a/libraries/chain/proposal_evaluator.cpp +++ b/libraries/chain/proposal_evaluator.cpp @@ -121,7 +121,6 @@ void_result proposal_update_evaluator::do_evaluate(const proposal_update_operati } /* All authority checks happen outside of evaluators - */ if( (d.get_node_properties().skip_flags & database::skip_authority_check) == 0 ) { for( const auto& id : o.key_approvals_to_add ) @@ -133,6 +132,7 @@ void_result proposal_update_evaluator::do_evaluate(const proposal_update_operati FC_ASSERT( trx_state->signed_by(id) ); } } + */ return void_result(); } FC_CAPTURE_AND_RETHROW( (o) ) } diff --git a/libraries/chain/proposal_object.cpp b/libraries/chain/proposal_object.cpp index ae7aa559..d67b4f8a 100644 --- a/libraries/chain/proposal_object.cpp +++ b/libraries/chain/proposal_object.cpp @@ -24,6 +24,22 @@ namespace graphene { namespace chain { bool proposal_object::is_authorized_to_execute(database& db) const { transaction_evaluation_state dry_run_eval(&db); + + try { + verify_authority( proposed_transaction.operations, + available_key_approvals, + [&]( account_id_type id ){ return &id(db).active; }, + [&]( account_id_type id ){ return &id(db).owner; }, + available_active_approvals, + available_owner_approvals ); + } catch ( const fc::exception& e ) + { + return false; + } + return true; + + + /* dry_run_eval._is_proposed_trx = true; std::transform(available_active_approvals.begin(), available_active_approvals.end(), std::inserter(dry_run_eval.approved_by, dry_run_eval.approved_by.end()), [](object_id_type id) { @@ -50,8 +66,8 @@ bool proposal_object::is_authorized_to_execute(database& db) const for( const auto& id : required_owner_approvals ) if( !dry_run_eval.check_authority(id(db), authority::owner) ) return false; + */ - return true; } diff --git a/libraries/chain/protocol/transaction.cpp b/libraries/chain/protocol/transaction.cpp index e7ff5ec1..253cbee6 100644 --- a/libraries/chain/protocol/transaction.cpp +++ b/libraries/chain/protocol/transaction.cpp @@ -15,7 +15,8 @@ * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#include +#include +#include #include #include #include @@ -79,6 +80,9 @@ void transaction::get_required_authorities( flat_set& active, f operation_get_required_authorities( op, active, owner, other ); } + + + struct sign_state { /** returns true if we have a signature for this key or can @@ -86,20 +90,21 @@ struct sign_state */ bool signed_by( const public_key_type& k ) { - auto itr = signatures.find(k); - if( itr == signatures.end() ) + auto itr = provided_signatures.find(k); + if( itr == provided_signatures.end() ) { - auto pk = keys.find(k); - if( pk != keys.end() ) - { - signatures[k] = trx.sign(pk->second); - checked_sigs.insert(k); - return true; - } + auto pk = available_keys.find(k); + if( pk != available_keys.end() ) + return provided_signatures[k] = true; return false; } - checked_sigs.insert(k); - return true; + return itr->second = true; + } + + bool check_authority( account_id_type id ) + { + if( id == GRAPHENE_TEMP_ACCOUNT ) return true; + return check_authority( get_active(id) ); } /** @@ -147,89 +152,116 @@ struct sign_state bool remove_unused_signatures() { vector remove_sigs; - for( const auto& sig : signatures ) - if( checked_sigs.find(sig.first) == checked_sigs.end() ) - remove_sigs.push_back( sig.first ); + for( const auto& sig : provided_signatures ) + if( !sig.second ) remove_sigs.push_back( sig.first ); + for( auto& sig : remove_sigs ) - signatures.erase(sig); + provided_signatures.erase(sig); + return remove_sigs.size() != 0; } - sign_state( const signed_transaction& t, const std::function& a, - const vector& kys = vector() ) - :trx(t),get_active(a) + sign_state( const flat_set& sigs, + const std::function& a, + const flat_set& keys = flat_set() ) + :get_active(a),available_keys(keys) { - auto d = trx.digest(); - for( const auto& sig : trx.signatures ) - signatures[ fc::ecc::public_key( sig, d ) ] = sig; - - for( const auto& key : kys ) - keys[key.get_public_key()] = key; + for( const auto& key : sigs ) + provided_signatures[ key ] = false; + approved_by.insert( GRAPHENE_TEMP_ACCOUNT ); } - const signed_transaction& trx; const std::function& get_active; + const flat_set& available_keys; - flat_map keys; - flat_map signatures; - - set checked_sigs; - flat_set approved_by; - + flat_map provided_signatures; + flat_set approved_by; }; -/** - * Given a set of private keys sign this transaction with a minimial subset of required keys. - */ -void signed_transaction::sign( const vector& keys, - const std::function& get_active, - const std::function& get_owner ) -{ + +void verify_authority( const vector& ops, const flat_set& sigs, + const std::function& get_active, + const std::function& get_owner, + const flat_set& active_aprovals, + const flat_set& owner_approvals ) +{ try { flat_set required_active; flat_set required_owner; vector other; - get_required_authorities( required_active, required_owner, other ); - sign_state s(*this,get_active,keys); + for( const auto& op : ops ) + operation_get_required_authorities( op, required_active, required_owner, other ); + + sign_state s(sigs,get_active); + for( auto& id : active_aprovals ) + s.approved_by.insert( id ); + for( auto& id : owner_approvals ) + s.approved_by.insert( id ); for( const auto& auth : other ) - s.check_authority(&auth); - for( auto id : required_active ) - s.check_authority(get_active(id)); - for( auto id : required_owner ) - s.check_authority(get_owner(id)); - - s.remove_unused_signatures(); - - signatures.clear(); - for( const auto& sig : s.signatures ) - signatures.push_back(sig.second); -} - -bool signed_transaction::verify( const std::function& get_active, - const std::function& get_owner )const -{ - flat_set required_active; - flat_set required_owner; - vector other; - get_required_authorities( required_active, required_owner, other ); - - sign_state s(*this,get_active); - - for( const auto& auth : other ) - if( !s.check_authority(&auth) ) - return false; + GRAPHENE_ASSERT( s.check_authority(&auth), tx_missing_other_auth, "Missing Authority", ("auth",auth)("sigs",sigs) ); // fetch all of the top level authorities for( auto id : required_active ) - if( !s.check_authority(get_active(id)) ) - return false; + GRAPHENE_ASSERT( s.check_authority(id) || + s.check_authority(get_owner(id)), + tx_missing_active_auth, "Missing Active Authority ${id}", ("id",id)("auth",*get_active(id))("owner",*get_owner(id)) ); + for( auto id : required_owner ) - if( !s.check_authority(get_owner(id)) ) - return false; - if( s.remove_unused_signatures() ) - return false; - return true; + GRAPHENE_ASSERT( owner_approvals.find(id) != owner_approvals.end() || + s.check_authority(get_owner(id)), + tx_missing_other_auth, "Missing Owner Authority ${id}", ("id",id)("auth",*get_owner(id)) ); + + FC_ASSERT( !s.remove_unused_signatures(), "Unnecessary signatures detected" ); +} FC_CAPTURE_AND_RETHROW( (ops)(sigs) ) } + + +flat_set signed_transaction::get_signature_keys()const +{ try { + auto d = digest(); + flat_set result; + for( const auto& sig : signatures ) + FC_ASSERT( result.insert( fc::ecc::public_key(sig,d) ).second, "Duplicate Signature detected" ); + return result; +} FC_CAPTURE_AND_RETHROW() } + + + +set signed_transaction::get_required_signatures( const flat_set& available_keys, + const std::function& get_active, + const std::function& get_owner )const +{ + flat_set required_active; + flat_set required_owner; + vector other; + get_required_authorities( required_active, required_owner, other ); + + + sign_state s(get_signature_keys(),get_active,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.remove_unused_signatures(); + + set result; + + for( auto& provided_sig : s.provided_signatures ) + if( available_keys.find( provided_sig.first ) != available_keys.end() ) + result.insert( provided_sig.first ); + + return result; } + +void signed_transaction::verify_authority( const std::function& get_active, + const std::function& get_owner )const +{ try { + graphene::chain::verify_authority( operations, get_signature_keys(), get_active, get_owner ); +} FC_CAPTURE_AND_RETHROW( (*this) ) } + } } // graphene::chain diff --git a/libraries/chain/transaction_evaluation_state.cpp b/libraries/chain/transaction_evaluation_state.cpp index 4e6dae18..f6b4fc70 100644 --- a/libraries/chain/transaction_evaluation_state.cpp +++ b/libraries/chain/transaction_evaluation_state.cpp @@ -23,6 +23,7 @@ #include namespace graphene { namespace chain { + /* bool transaction_evaluation_state::check_authority( const account_object& account, authority::classification auth_class, int depth ) { if( (!_is_proposed_trx) && (_db->get_node_properties().skip_flags & database::skip_authority_check) ) @@ -121,5 +122,6 @@ namespace graphene { namespace chain { if( itr->first == k ) return itr->second = true; return false; } + */ } } // namespace graphene::chain From 8c6e0b9e55cd59e500f58ba7289cebb0d70fc6a8 Mon Sep 17 00:00:00 2001 From: Daniel Larimer Date: Fri, 17 Jul 2015 00:41:43 -0400 Subject: [PATCH 2/2] Refactor Authority Checking transaction_evaluation_state no longer tracks authority validation. Authority validation is now compeltely independent of the database. --- libraries/chain/CMakeLists.txt | 1 - libraries/chain/db_block.cpp | 38 ------------------- .../include/graphene/chain/exceptions.hpp | 4 +- .../graphene/chain/protocol/transaction.hpp | 8 +++- .../chain/transaction_evaluation_state.hpp | 24 ------------ libraries/chain/proposal_object.cpp | 7 +++- libraries/chain/protocol/transaction.cpp | 21 +++++++--- tests/tests/authority_tests.cpp | 6 +-- 8 files changed, 34 insertions(+), 75 deletions(-) diff --git a/libraries/chain/CMakeLists.txt b/libraries/chain/CMakeLists.txt index 698b5089..3ad4183a 100644 --- a/libraries/chain/CMakeLists.txt +++ b/libraries/chain/CMakeLists.txt @@ -43,7 +43,6 @@ add_library( graphene_chain proposal_object.cpp vesting_balance_object.cpp - transaction_evaluation_state.cpp fork_database.cpp block_database.cpp diff --git a/libraries/chain/db_block.cpp b/libraries/chain/db_block.cpp index 68c0fa03..489897d3 100644 --- a/libraries/chain/db_block.cpp +++ b/libraries/chain/db_block.cpp @@ -215,22 +215,6 @@ processed_transaction database::push_proposal(const proposal_object& proposal) transaction_evaluation_state eval_state(this); eval_state._is_proposed_trx = true; - //Inject the approving authorities into the transaction eval state - /* - std::transform(proposal.required_active_approvals.begin(), - proposal.required_active_approvals.end(), - std::inserter(eval_state.approved_by, eval_state.approved_by.begin()), - []( account_id_type id ) { - return std::make_pair(id, authority::active); - }); - std::transform(proposal.required_owner_approvals.begin(), - proposal.required_owner_approvals.end(), - std::inserter(eval_state.approved_by, eval_state.approved_by.begin()), - []( account_id_type id ) { - return std::make_pair(id, authority::owner); - }); - */ - eval_state.operation_results.reserve(proposal.proposed_transaction.operations.size()); processed_transaction ptrx(proposal.proposed_transaction); eval_state._trx = &ptrx; @@ -489,17 +473,6 @@ 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; }; trx.verify_authority( get_active, get_owner ); - - /* - eval_state._sigs.reserve(trx.signatures.size()); - - for( const auto& sig : trx.signatures ) - { - FC_ASSERT( eval_state._sigs.insert(std::make_pair(public_key_type(fc::ecc::public_key(sig, trx.digest())), - false)).second, - "Multiple signatures by same key detected" ); - } - */ } //Skip all manner of expiration and TaPoS checking if we're on block 1; It's impossible that the transaction is @@ -545,17 +518,6 @@ processed_transaction database::_apply_transaction(const signed_transaction& trx auto range = index.equal_range(GRAPHENE_TEMP_ACCOUNT); std::for_each(range.first, range.second, [](const account_balance_object& b) { FC_ASSERT(b.balance == 0); }); - //Make sure all signatures were needed to validate the transaction - /* - if( !(skip & (skip_transaction_signatures|skip_authority_check)) ) - { - for( const auto& item : eval_state._sigs ) - { - FC_ASSERT( item.second, "All signatures must be used", ("item",item) ); - } - } - */ - return ptrx; } FC_CAPTURE_AND_RETHROW( (trx) ) } diff --git a/libraries/chain/include/graphene/chain/exceptions.hpp b/libraries/chain/include/graphene/chain/exceptions.hpp index a75e3bba..05bde928 100644 --- a/libraries/chain/include/graphene/chain/exceptions.hpp +++ b/libraries/chain/include/graphene/chain/exceptions.hpp @@ -73,7 +73,9 @@ namespace graphene { namespace chain { FC_DECLARE_DERIVED_EXCEPTION( tx_missing_active_auth, graphene::chain::transaction_exception, 3030001, "missing required active authority" ) FC_DECLARE_DERIVED_EXCEPTION( tx_missing_owner_auth, graphene::chain::transaction_exception, 3030002, "missing required owner authority" ) FC_DECLARE_DERIVED_EXCEPTION( tx_missing_other_auth, graphene::chain::transaction_exception, 3030003, "missing required other authority" ) - //FC_DECLARE_DERIVED_EXCEPTION( tx_irrelevant_authority, graphene::chain::transaction_exception, 3030004, "irrelevant authority" ) + FC_DECLARE_DERIVED_EXCEPTION( tx_irrelevant_authority, graphene::chain::transaction_exception, 3030004, "irrelevant authority" ) + FC_DECLARE_DERIVED_EXCEPTION( invalid_committee_approval, graphene::chain::transaction_exception, 3030005, + "committee account cannot directly approve transaction" ) FC_DECLARE_DERIVED_EXCEPTION( invalid_pts_address, graphene::chain::utility_exception, 3060001, "invalid pts address" ) FC_DECLARE_DERIVED_EXCEPTION( insufficient_feeds, graphene::chain::chain_exception, 37006, "insufficient feeds" ) diff --git a/libraries/chain/include/graphene/chain/protocol/transaction.hpp b/libraries/chain/include/graphene/chain/protocol/transaction.hpp index 30e373d1..4ada2b34 100644 --- a/libraries/chain/include/graphene/chain/protocol/transaction.hpp +++ b/libraries/chain/include/graphene/chain/protocol/transaction.hpp @@ -122,11 +122,13 @@ namespace graphene { namespace chain { */ set get_required_signatures( const flat_set& available_keys, const std::function& get_active, - const std::function& get_owner + const std::function& get_owner, + uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH )const; void verify_authority( const std::function& get_active, - const std::function& get_owner )const; + const std::function& get_owner, + uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH )const; flat_set get_signature_keys()const; @@ -139,6 +141,8 @@ namespace graphene { namespace chain { void verify_authority( const vector& ops, const flat_set& sigs, const std::function& get_active, const std::function& get_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()); diff --git a/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp b/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp index 027beb9a..49a69a98 100644 --- a/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp +++ b/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp @@ -32,34 +32,10 @@ namespace graphene { namespace chain { transaction_evaluation_state( database* db = nullptr ) :_db(db){} - /* - bool check_authority(const account_object&, - authority::classification auth_class = authority::active, - int depth = 0); - - bool check_authority(const authority&, - authority::classification auth_class = authority::active, - int depth = 0); - */ database& db()const { assert( _db ); return *_db; } - - /* - bool signed_by(const public_key_type& k); - bool signed_by(const address& k); - - /// cached approval (accounts and keys) - flat_set> approved_by; - */ - - /// Used to look up new objects using transaction relative IDs vector operation_results; - /** - * When an address is referenced via check authority it is flagged as being used, all addresses must be - * flagged as being used or the transaction will fail. - */ - // flat_map _sigs; const signed_transaction* _trx = nullptr; database* _db = nullptr; bool _is_proposed_trx = false; diff --git a/libraries/chain/proposal_object.cpp b/libraries/chain/proposal_object.cpp index d67b4f8a..f40d22e6 100644 --- a/libraries/chain/proposal_object.cpp +++ b/libraries/chain/proposal_object.cpp @@ -30,10 +30,15 @@ bool proposal_object::is_authorized_to_execute(database& db) const available_key_approvals, [&]( account_id_type id ){ return &id(db).active; }, [&]( account_id_type id ){ return &id(db).owner; }, + GRAPHENE_MAX_SIG_CHECK_DEPTH, // TODO make chain param + true, /* allow committeee */ available_active_approvals, available_owner_approvals ); - } catch ( const fc::exception& e ) + } + catch ( const fc::exception& e ) { + //idump((available_active_approvals)); + //wlog((e.to_detail_string())); return false; } return true; diff --git a/libraries/chain/protocol/transaction.cpp b/libraries/chain/protocol/transaction.cpp index 253cbee6..d3e662db 100644 --- a/libraries/chain/protocol/transaction.cpp +++ b/libraries/chain/protocol/transaction.cpp @@ -103,7 +103,7 @@ struct sign_state bool check_authority( account_id_type id ) { - if( id == GRAPHENE_TEMP_ACCOUNT ) return true; + if( approved_by.find(id) != approved_by.end() ) return true; return check_authority( get_active(id) ); } @@ -129,7 +129,7 @@ struct sign_state { if( approved_by.find(a.first) == approved_by.end() ) { - if( depth == GRAPHENE_MAX_SIG_CHECK_DEPTH ) + if( depth == max_recursion ) return false; if( check_authority( get_active( a.first ), depth+1 ) ) { @@ -176,12 +176,15 @@ struct sign_state flat_map provided_signatures; flat_set approved_by; + uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH; }; void verify_authority( const vector& ops, const flat_set& sigs, const std::function& get_active, const std::function& get_owner, + uint32_t max_recursion_depth, + bool allow_committe, const flat_set& active_aprovals, const flat_set& owner_approvals ) { try { @@ -192,7 +195,12 @@ void verify_authority( const vector& ops, const flat_set signed_transaction::get_signature_keys()const set signed_transaction::get_required_signatures( const flat_set& available_keys, const std::function& get_active, - const std::function& get_owner )const + const std::function& get_owner, + uint32_t max_recursion_depth )const { flat_set required_active; flat_set required_owner; @@ -238,6 +247,7 @@ set signed_transaction::get_required_signatures( const flat_set sign_state s(get_signature_keys(),get_active,available_keys); + s.max_recursion = max_recursion_depth; for( const auto& auth : other ) s.check_authority(&auth); @@ -259,9 +269,10 @@ set signed_transaction::get_required_signatures( const flat_set void signed_transaction::verify_authority( const std::function& get_active, - const std::function& get_owner )const + const std::function& get_owner, + uint32_t max_recursion )const { try { - graphene::chain::verify_authority( operations, get_signature_keys(), get_active, get_owner ); + graphene::chain::verify_authority( operations, get_signature_keys(), get_active, get_owner, max_recursion ); } FC_CAPTURE_AND_RETHROW( (*this) ) } } } // graphene::chain diff --git a/tests/tests/authority_tests.cpp b/tests/tests/authority_tests.cpp index 8b0c6cbf..dfabf40a 100644 --- a/tests/tests/authority_tests.cpp +++ b/tests/tests/authority_tests.cpp @@ -340,7 +340,6 @@ BOOST_AUTO_TEST_CASE( proposed_single_account ) trx.set_expiration( db.head_block_time() + fc::seconds( 3 * db.get_global_properties().parameters.block_interval )); trx.set_reference_block( db.head_block_id() ); - //idump((moneyman)); trx.sign( init_account_priv_key ); const proposal_object& proposal = db.get(PUSH_TX( db, trx ).operation_results.front().get()); @@ -353,6 +352,7 @@ BOOST_AUTO_TEST_CASE( proposed_single_account ) proposal_update_operation pup; pup.proposal = proposal.id; pup.fee_paying_account = nathan.id; + BOOST_TEST_MESSAGE( "Updating the proposal to have nathan's authority" ); pup.active_approvals_to_add.insert(nathan.id); trx.operations = {pup}; @@ -399,13 +399,13 @@ BOOST_AUTO_TEST_CASE( committee_authority ) p.parameters.committee_proposal_review_period = fc::days(1).to_seconds(); }); - BOOST_TEST_MESSAGE( "transfering 100000 CORE to nathan, signing with committee key" ); + BOOST_TEST_MESSAGE( "transfering 100000 CORE to nathan, signing with committee key should fail because this requires it to be part of a proposal" ); transfer_operation top; top.to = nathan.id; top.amount = asset(100000); trx.operations.push_back(top); sign(trx, committee_key); - GRAPHENE_CHECK_THROW(PUSH_TX( db, trx ), fc::exception); + GRAPHENE_CHECK_THROW(PUSH_TX( db, trx ), graphene::chain::invalid_committee_approval ); auto sign = [&] { trx.signatures.clear(); trx.sign(nathan_key); };