From b1dd41ff3ab1ddc329bd9d06299ec633b1549b8d Mon Sep 17 00:00:00 2001 From: Daniel Larimer Date: Tue, 7 Jul 2015 10:57:01 -0400 Subject: [PATCH] Refactor get_required_auths on operations --- libraries/chain/evaluator.cpp | 16 +- .../include/graphene/chain/operations.hpp | 96 +----- .../include/graphene/chain/transaction.hpp | 2 + .../chain/transaction_evaluation_state.hpp | 4 + libraries/chain/operations.cpp | 294 ++++++++---------- libraries/chain/proposal_evaluator.cpp | 13 +- libraries/chain/transaction.cpp | 11 +- .../chain/transaction_evaluation_state.cpp | 32 +- .../account_history_plugin.cpp | 8 +- libraries/wallet/wallet.cpp | 22 +- tests/tests/authority_tests.cpp | 11 +- tests/tests/operation_tests.cpp | 2 - 12 files changed, 223 insertions(+), 288 deletions(-) diff --git a/libraries/chain/evaluator.cpp b/libraries/chain/evaluator.cpp index 3a7f5f12..66d81d5c 100644 --- a/libraries/chain/evaluator.cpp +++ b/libraries/chain/evaluator.cpp @@ -83,18 +83,22 @@ database& generic_evaluator::db()const { return trx_state->db(); } { try { flat_set active_auths; flat_set owner_auths; - op.visit(operation_get_required_auths(active_auths, owner_auths)); - // idump((active_auths)(owner_auths)(op)); + vector other_auths; + + operation_get_required_active_authorities( op, active_auths ); + operation_get_required_owner_authorities( op, owner_auths ); + operation_get_required_authorities( op, other_auths ); for( auto id : active_auths ) - { FC_ASSERT(verify_authority(id(db()), authority::active) || verify_authority(id(db()), authority::owner), "", ("id", id)); - } + for( auto id : owner_auths ) - { FC_ASSERT(verify_authority(id(db()), authority::owner), "", ("id", id)); - } + + for( const auto& auth : other_auths ) + FC_ASSERT(trx_state->check_authority(auth)); + } FC_CAPTURE_AND_RETHROW( (op) ) } void generic_evaluator::verify_authority_accounts( const authority& a )const diff --git a/libraries/chain/include/graphene/chain/operations.hpp b/libraries/chain/include/graphene/chain/operations.hpp index 300dfdef..f5d0d6f5 100644 --- a/libraries/chain/include/graphene/chain/operations.hpp +++ b/libraries/chain/include/graphene/chain/operations.hpp @@ -122,7 +122,6 @@ namespace graphene { namespace chain { flat_set required_auths; account_id_type fee_payer()const { return fee_paying_account; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; share_type calculate_fee(const fee_schedule_type& k)const; void validate()const; @@ -145,7 +144,6 @@ namespace graphene { namespace chain { asset total_claimed; account_id_type fee_payer()const { return deposit_to_account; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; share_type calculate_fee(const fee_schedule_type& k)const { return 0; } void validate()const; @@ -177,7 +175,6 @@ namespace graphene { namespace chain { account_object::options_type options; account_id_type fee_payer()const { return registrar; } - void get_required_auth(flat_set& active_auth_set , flat_set&)const; void validate()const; share_type calculate_fee( const fee_schedule_type& k )const; @@ -223,7 +220,6 @@ namespace graphene { namespace chain { uint8_t new_listing; account_id_type fee_payer()const { return authorizing_account; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const { FC_ASSERT( fee.amount >= 0 ); FC_ASSERT(new_listing < 0x4); } share_type calculate_fee(const fee_schedule_type& k)const { return k.account_whitelist_fee; } @@ -245,14 +241,13 @@ namespace graphene { namespace chain { /// New owner authority. If set, this operation requires owner authority to execute. optional owner; - /// New active authority. If set, this operation requires owner authority to execute. + /// New active authority. If set, this operation requires owner authority to execute: TODO: why? optional active; /// New account options optional new_options; account_id_type fee_payer()const { return account; } - void get_required_auth(flat_set& active_auth_set , flat_set& owner_auth_set)const; void validate()const; share_type calculate_fee( const fee_schedule_type& k )const; void get_balance_delta( balance_accumulator& acc, const operation_result& result = asset())const { acc.adjust( fee_payer(), -fee ); } @@ -280,8 +275,6 @@ namespace graphene { namespace chain { bool upgrade_to_lifetime_member = false; account_id_type fee_payer()const { return account_to_upgrade; } - void get_required_auth(flat_set& active_auth_set , flat_set&)const - { active_auth_set.insert(account_to_upgrade); } void validate()const; share_type calculate_fee( const fee_schedule_type& k )const; void get_balance_delta( balance_accumulator& acc, const operation_result& = asset())const { acc.adjust( fee_payer(), -fee ); } @@ -307,7 +300,6 @@ namespace graphene { namespace chain { account_id_type new_owner; account_id_type fee_payer()const { return account_id; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee( const fee_schedule_type& k )const; @@ -329,7 +321,6 @@ namespace graphene { namespace chain { string url; account_id_type fee_payer()const { return delegate_account; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee( const fee_schedule_type& k )const; void get_balance_delta( balance_accumulator& acc, const operation_result& result = asset())const { acc.adjust( fee_payer(), -fee ); } @@ -352,7 +343,6 @@ namespace graphene { namespace chain { secret_hash_type initial_secret; account_id_type fee_payer()const { return witness_account; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; @@ -372,7 +362,6 @@ namespace graphene { namespace chain { share_type amount; account_id_type fee_payer()const { return to_account; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; @@ -400,8 +389,6 @@ namespace graphene { namespace chain { chain_parameters new_parameters; account_id_type fee_payer()const { return account_id_type(); } - void get_required_auth(flat_set& active_auth_set, flat_set&)const - { active_auth_set.insert(account_id_type()); } void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta( balance_accumulator& acc, const operation_result& result = asset())const { acc.adjust( fee_payer(), -fee ); } @@ -494,7 +481,6 @@ namespace graphene { namespace chain { optional memo; account_id_type fee_payer()const { return from; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta( balance_accumulator& acc, const operation_result& result = asset())const @@ -528,7 +514,6 @@ namespace graphene { namespace chain { optional memo; account_id_type fee_payer()const { return issuer; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta( balance_accumulator& acc, const operation_result& result = asset())const @@ -566,7 +551,6 @@ namespace graphene { namespace chain { bool is_prediction_market = false; account_id_type fee_payer()const { return issuer; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee( const fee_schedule_type& k )const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const { acc.adjust(fee_payer(), -fee); } @@ -590,7 +574,6 @@ namespace graphene { namespace chain { price settle_price; account_id_type fee_payer()const { return issuer; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const { acc.adjust(fee_payer(), -fee); } @@ -618,7 +601,6 @@ namespace graphene { namespace chain { asset amount; account_id_type fee_payer()const { return account; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const @@ -639,7 +621,6 @@ namespace graphene { namespace chain { share_type amount; ///< core asset account_id_type fee_payer()const { return from_account; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const @@ -679,7 +660,6 @@ namespace graphene { namespace chain { asset_object::asset_options new_options; account_id_type fee_payer()const { return issuer; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const { acc.adjust(fee_payer(), -fee); } @@ -707,7 +687,6 @@ namespace graphene { namespace chain { asset_object::bitasset_options new_options; account_id_type fee_payer()const { return issuer; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const { acc.adjust(fee_payer(), -fee); } @@ -738,8 +717,6 @@ namespace graphene { namespace chain { flat_set new_feed_producers; account_id_type fee_payer()const { return issuer; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const - { active_auth_set.insert(fee_payer()); } void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const @@ -770,7 +747,6 @@ namespace graphene { namespace chain { price_feed feed; account_id_type fee_payer()const { return publisher; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee( const fee_schedule_type& k )const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const { acc.adjust(fee_payer(), -fee); } @@ -791,7 +767,6 @@ namespace graphene { namespace chain { optional memo; account_id_type fee_payer()const { return issuer; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const @@ -811,7 +786,6 @@ namespace graphene { namespace chain { asset amount_to_reserve; account_id_type fee_payer()const { return payer; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const @@ -860,7 +834,6 @@ namespace graphene { namespace chain { std::make_pair(min_to_receive.asset_id, amount_to_sell.asset_id); } account_id_type fee_payer()const { return seller; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; price get_price()const { return amount_to_sell / min_to_receive; } @@ -887,7 +860,6 @@ namespace graphene { namespace chain { account_id_type fee_paying_account; account_id_type fee_payer()const { return fee_paying_account; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const @@ -918,7 +890,6 @@ namespace graphene { namespace chain { asset delta_debt; ///< the amount of the debt to be paid off, may be negative to issue new debt account_id_type fee_payer()const { return funding_account; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const @@ -983,7 +954,6 @@ namespace graphene { namespace chain { static proposal_create_operation genesis_proposal(const class database& db); account_id_type fee_payer()const { return fee_paying_account; } - void get_required_auth( flat_set& active_auth_set, flat_set& )const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const @@ -1021,7 +991,6 @@ namespace graphene { namespace chain { flat_set key_approvals_to_remove; account_id_type fee_payer()const { return fee_paying_account; } - void get_required_auth(flat_set& active_auth_set, flat_set& owner_auth_set)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const { acc.adjust(fee_payer(), -fee); } @@ -1046,7 +1015,6 @@ namespace graphene { namespace chain { proposal_id_type proposal; account_id_type fee_payer()const { return fee_paying_account; } - void get_required_auth(flat_set& active_auth_set, flat_set& owner_auth_set)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const { acc.adjust(fee_payer(), -fee); } @@ -1076,8 +1044,6 @@ namespace graphene { namespace chain { std::make_pair( receives.asset_id, pays.asset_id ); } account_id_type fee_payer()const { return account_id; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const - { active_auth_set.insert(fee_payer()); } void validate()const { FC_ASSERT( !"virtual operation" ); } share_type calculate_fee(const fee_schedule_type& k)const // This is a virtual operation; there is no fee @@ -1124,7 +1090,6 @@ namespace graphene { namespace chain { time_point_sec period_start_time; account_id_type fee_payer()const { return withdraw_from_account; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const { acc.adjust(fee_payer(), -fee); } @@ -1160,7 +1125,6 @@ namespace graphene { namespace chain { uint32_t periods_until_expiration; account_id_type fee_payer()const { return withdraw_from_account; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const { acc.adjust(fee_payer(), -fee); } @@ -1195,7 +1159,6 @@ namespace graphene { namespace chain { optional memo; account_id_type fee_payer()const { return withdraw_to_account; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const @@ -1225,7 +1188,6 @@ namespace graphene { namespace chain { withdraw_permission_id_type withdrawal_permission; account_id_type fee_payer()const { return withdraw_from_account; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const @@ -1278,7 +1240,6 @@ namespace graphene { namespace chain { vesting_policy_initializer policy; account_id_type fee_payer()const { return creator; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const @@ -1306,7 +1267,6 @@ namespace graphene { namespace chain { asset amount; account_id_type fee_payer()const { return owner; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const @@ -1356,7 +1316,6 @@ namespace graphene { namespace chain { worker_initializer initializer; account_id_type fee_payer()const { return owner; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const @@ -1383,7 +1342,6 @@ namespace graphene { namespace chain { vector data; account_id_type fee_payer()const { return payer; } - void get_required_auth(flat_set& active_auth_set, flat_set&)const; void validate()const; share_type calculate_fee(const fee_schedule_type& k)const; void get_balance_delta(balance_accumulator& acc, const operation_result& result = asset())const @@ -1440,6 +1398,18 @@ namespace graphene { namespace chain { /// @} // operations group + + /** + * Appends required authorites to the result vector. The authorities appended are not the + * same as those returned by get_required_auth + * + * @return a set of required authorities for @ref op + */ + void operation_get_required_authorities( const operation& op, vector& result ); + void operation_get_required_active_authorities( const operation& op, flat_set& result ); + void operation_get_required_owner_authorities( const operation& op, flat_set& result ); + void operation_validate( const operation& op ); + /** * Used to track the result of applying an operation and when it was applied. */ @@ -1452,40 +1422,6 @@ namespace graphene { namespace chain { uint16_t op_num; }; - /** - * @brief Used to find accounts which must sign off on operations in a polymorphic manner - */ - struct operation_get_required_auths - { - flat_set& active_auth_set; - flat_set& owner_auth_set; - operation_get_required_auths(flat_set& active_auth_set, - flat_set& owner_auth_set) - : active_auth_set(active_auth_set), - owner_auth_set(owner_auth_set) - {} - typedef void result_type; - template - void operator()(const T& v)const - { - v.get_required_auth(active_auth_set, owner_auth_set); -#ifndef NDEBUG - if( !(active_auth_set.count(v.fee_payer()) || owner_auth_set.count(v.fee_payer())) ) - elog("Fee payer not in required auths on ${op}", ("op", fc::get_typename::name())); - assert(active_auth_set.count(v.fee_payer()) || owner_auth_set.count(v.fee_payer())); -#endif - } - }; - - /** - * @brief Used to validate operations in a polymorphic manner - */ - struct operation_validator - { - typedef void result_type; - template - void operator()( const T& v )const { v.validate(); } - }; /** * @brief Used to calculate fees in a polymorphic manner @@ -1551,11 +1487,7 @@ namespace graphene { namespace chain { public: op_wrapper(const operation& op = operation()):op(op){} operation op; - - void validate()const { op.visit( operation_validator() ); } - void get_required_auth(flat_set& active, flat_set& owner) { - op.visit(operation_get_required_auths(active, owner)); - } + void validate()const; asset set_fee( const fee_schedule_type& k ) { return op.visit( operation_set_fee( k ) ); } share_type calculate_fee( const fee_schedule_type& k )const { return op.visit( operation_calculate_fee( k ) ); } }; diff --git a/libraries/chain/include/graphene/chain/transaction.hpp b/libraries/chain/include/graphene/chain/transaction.hpp index b8ffc1e9..a377556b 100644 --- a/libraries/chain/include/graphene/chain/transaction.hpp +++ b/libraries/chain/include/graphene/chain/transaction.hpp @@ -114,6 +114,8 @@ namespace graphene { namespace chain { op.visit( std::forward( visitor ) ); } + void get_required_authorities( flat_set& active, flat_set& owner, vector& other ); + protected: // Intentionally unreflected: does not go on wire optional block_id_cache; diff --git a/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp b/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp index 3ae0b140..83c3e5a7 100644 --- a/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp +++ b/libraries/chain/include/graphene/chain/transaction_evaluation_state.hpp @@ -38,6 +38,10 @@ namespace graphene { namespace chain { 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 { FC_ASSERT( _db ); return *_db; } bool signed_by(const public_key_type& k); diff --git a/libraries/chain/operations.cpp b/libraries/chain/operations.cpp index 175e3fa2..d6240b45 100644 --- a/libraries/chain/operations.cpp +++ b/libraries/chain/operations.cpp @@ -216,14 +216,6 @@ share_type account_update_operation::calculate_fee( const fee_schedule_type& sch core_fee_required += schedule.total_data_fee(new_options->votes); return core_fee_required; } -void account_update_operation::get_required_auth(flat_set& active_auth_set, - flat_set& owner_auth_set) const -{ - if( owner || active ) - owner_auth_set.insert(account); - else - active_auth_set.insert(account); -} void account_update_operation::validate()const { @@ -284,11 +276,6 @@ share_type override_transfer_operation::calculate_fee( const fee_schedule_type& } -void account_create_operation::get_required_auth(flat_set& active_auth_set, - flat_set&) const -{ - active_auth_set.insert(registrar); -} void account_create_operation::validate()const { @@ -315,11 +302,6 @@ void asset_publish_feed_operation::validate()const feed.validate(); } -void transfer_operation::get_required_auth(flat_set& active_auth_set, - flat_set&) const -{ - active_auth_set.insert( from ); -} void transfer_operation::validate()const { @@ -328,12 +310,6 @@ void transfer_operation::validate()const FC_ASSERT( amount.amount > 0 ); } -void override_transfer_operation::get_required_auth(flat_set& active_auth_set, - flat_set&) const -{ - active_auth_set.insert( issuer ); -} - void override_transfer_operation::validate()const { FC_ASSERT( fee.amount >= 0 ); @@ -342,10 +318,6 @@ void override_transfer_operation::validate()const FC_ASSERT( issuer != from ); } -void asset_create_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(issuer); -} void asset_create_operation::validate()const { @@ -373,10 +345,6 @@ asset_update_operation::asset_update_operation(const asset_object& old) new_options = old.options; } -void asset_update_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(issuer); -} void asset_update_operation::validate()const { @@ -394,10 +362,6 @@ share_type asset_update_operation::calculate_fee(const fee_schedule_type& k)cons return k.asset_update_fee + k.total_data_fee(new_options); } -void asset_reserve_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(payer); -} void asset_reserve_operation::validate()const { @@ -411,10 +375,6 @@ share_type asset_reserve_operation::calculate_fee(const fee_schedule_type& k)con return k.asset_reserve_fee; } -void asset_issue_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(issuer); -} void asset_issue_operation::validate()const { @@ -434,10 +394,6 @@ share_type delegate_create_operation::calculate_fee( const fee_schedule_type& k return k.delegate_create_fee + k.total_data_fee(url); } -void delegate_create_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(delegate_account); -} void delegate_create_operation::validate()const { @@ -445,10 +401,6 @@ void delegate_create_operation::validate()const FC_ASSERT(url.size() < GRAPHENE_MAX_URL_LENGTH ); } -void asset_fund_fee_pool_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(from_account); -} void asset_fund_fee_pool_operation::validate() const { @@ -462,10 +414,6 @@ share_type asset_fund_fee_pool_operation::calculate_fee(const fee_schedule_type& return k.asset_fund_fee_pool_fee; } -void limit_order_create_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(seller); -} void limit_order_create_operation::validate()const { @@ -480,10 +428,6 @@ share_type limit_order_create_operation::calculate_fee(const fee_schedule_type& return k.limit_order_create_fee; } -void limit_order_cancel_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(fee_paying_account); -} void limit_order_cancel_operation::validate()const { @@ -495,10 +439,6 @@ share_type limit_order_cancel_operation::calculate_fee(const fee_schedule_type& return k.get_extended_fee(fee_schedule_type::limit_order_cancel_fee_id); } -void call_order_update_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(funding_account); -} void call_order_update_operation::validate()const { try { @@ -522,10 +462,6 @@ proposal_create_operation proposal_create_operation::genesis_proposal(const data return op; } -void proposal_create_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(fee_paying_account); -} void proposal_create_operation::validate() const { @@ -538,19 +474,6 @@ share_type proposal_create_operation::calculate_fee(const fee_schedule_type& k) return k.proposal_create_fee + k.total_data_fee(proposed_ops); } -void proposal_update_operation::get_required_auth(flat_set& active_auth_set, - flat_set& owner_auth_set) const -{ - active_auth_set.insert(fee_paying_account); - for( auto id : active_approvals_to_add ) - active_auth_set.insert(id); - for( auto id : active_approvals_to_remove ) - active_auth_set.insert(id); - for( auto id : owner_approvals_to_add ) - owner_auth_set.insert(id); - for( auto id : owner_approvals_to_remove ) - owner_auth_set.insert(id); -} void proposal_update_operation::validate() const { @@ -585,14 +508,6 @@ share_type proposal_update_operation::calculate_fee(const fee_schedule_type& k) key_approvals_to_remove); } -void proposal_delete_operation::get_required_auth(flat_set& active_auth_set, - flat_set& owner_auth_set) const -{ - if( using_owner_authority ) - owner_auth_set.insert(fee_paying_account); - else - active_auth_set.insert(fee_paying_account); -} share_type proposal_delete_operation::calculate_fee(const fee_schedule_type& k)const { return k.get_extended_fee( fee_schedule_type::proposal_delete_fee_id ); } @@ -601,10 +516,6 @@ void account_transfer_operation::validate()const FC_ASSERT( fee.amount >= 0 ); } -void account_transfer_operation::get_required_auth(flat_set& active_auth_set, flat_set&)const -{ - active_auth_set.insert( account_id ); -} share_type account_transfer_operation::calculate_fee( const fee_schedule_type& k )const { @@ -617,10 +528,6 @@ void proposal_delete_operation::validate() const FC_ASSERT( fee.amount >= 0 ); } -void witness_withdraw_pay_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(to_account); -} void witness_withdraw_pay_operation::validate() const { @@ -633,10 +540,6 @@ share_type witness_withdraw_pay_operation::calculate_fee(const fee_schedule_type return k.witness_withdraw_pay_fee; } -void account_whitelist_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(authorizing_account); -} void global_parameters_update_operation::validate() const { @@ -649,10 +552,6 @@ share_type global_parameters_update_operation::calculate_fee(const fee_schedule_ return k.global_parameters_update_fee; } -void witness_create_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(witness_account); -} void witness_create_operation::validate() const { @@ -665,10 +564,6 @@ share_type witness_create_operation::calculate_fee(const fee_schedule_type& k) c return k.witness_create_fee + k.total_data_fee(url); } -void withdraw_permission_update_operation::get_required_auth(flat_set& active_auth_set, flat_set&)const -{ - active_auth_set.insert( withdraw_from_account ); -} void withdraw_permission_update_operation::validate()const { @@ -684,11 +579,6 @@ share_type withdraw_permission_update_operation::calculate_fee( const fee_schedu return schedule.withdraw_permission_update_fee; } -void withdraw_permission_claim_operation::get_required_auth(flat_set& active_auth_set, flat_set&)const -{ - active_auth_set.insert( withdraw_to_account ); -} - void withdraw_permission_claim_operation::validate()const { FC_ASSERT( withdraw_to_account != withdraw_from_account ); @@ -704,10 +594,6 @@ share_type withdraw_permission_claim_operation::calculate_fee(const fee_schedule return core_fee_required; } -void withdraw_permission_delete_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(withdraw_from_account); -} void withdraw_permission_delete_operation::validate() const { @@ -720,10 +606,6 @@ share_type withdraw_permission_delete_operation::calculate_fee(const fee_schedul return k.get_extended_fee( fee_schedule_type::withdraw_permission_delete_fee_id ); } -void withdraw_permission_create_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(withdraw_from_account); -} void withdraw_permission_create_operation::validate() const { @@ -740,13 +622,6 @@ share_type withdraw_permission_create_operation::calculate_fee(const fee_schedul return k.withdraw_permission_create_fee; } - -void asset_global_settle_operation::get_required_auth(flat_set& active_auth_set, flat_set&)const -{ - active_auth_set.insert( fee_payer() ); - -} - void asset_global_settle_operation::validate()const { FC_ASSERT( fee.amount >= 0 ); @@ -758,10 +633,6 @@ share_type asset_global_settle_operation::calculate_fee(const fee_schedule_type return k.asset_global_settle_fee; } -void asset_settle_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert( account ); -} void asset_settle_operation::validate() const { @@ -775,16 +646,6 @@ share_type asset_settle_operation::calculate_fee(const fee_schedule_type& k) con } -void graphene::chain::asset_publish_feed_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(publisher); -} - -void asset_update_bitasset_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(issuer); -} - void asset_update_bitasset_operation::validate() const { FC_ASSERT( fee.amount >= 0 ); @@ -806,12 +667,6 @@ share_type asset_update_feed_producers_operation::calculate_fee(const fee_schedu return k.asset_update_fee + k.total_data_fee(new_feed_producers); } -void vesting_balance_create_operation::get_required_auth(flat_set& active_auth_set, flat_set&)const -{ - // owner's authorization isn't needed since this is effectively a transfer of value TO the owner - active_auth_set.insert(creator); -} - share_type vesting_balance_create_operation::calculate_fee(const fee_schedule_type& k)const { // We don't want to have custom inspection for each policy type; instead, charge a data fee for big ones @@ -824,11 +679,6 @@ void vesting_balance_create_operation::validate()const FC_ASSERT( amount.amount > 0 ); } -void vesting_balance_withdraw_operation::get_required_auth(flat_set& active_auth_set, flat_set&)const -{ - active_auth_set.insert(owner); -} - void vesting_balance_withdraw_operation::validate()const { FC_ASSERT( fee.amount >= 0 ); @@ -883,10 +733,6 @@ string memo_data::get_message(const fc::ecc::private_key& priv, } } -void custom_operation::get_required_auth(flat_set& active_auth_set, flat_set&)const -{ - active_auth_set.insert(required_auths.begin(), required_auths.end()); -} void custom_operation::validate()const { FC_ASSERT( fee.amount > 0 ); @@ -896,11 +742,6 @@ share_type custom_operation::calculate_fee(const fee_schedule_type& k)const return k.custom_operation_fee + k.total_data_fee(required_auths, data); } -void worker_create_operation::get_required_auth(flat_set& active_auth_set, flat_set&) const -{ - active_auth_set.insert(owner); -} - void worker_create_operation::validate() const { FC_ASSERT(fee.amount >= 0); @@ -974,11 +815,6 @@ void assert_operation::validate()const p.visit( predicate_validator() ); } } -void assert_operation::get_required_auth(flat_set& active_auth_set, flat_set&)const -{ - active_auth_set.insert(fee_paying_account); - active_auth_set.insert(required_auths.begin(), required_auths.end()); -} /** * The fee for assert operations is proportional to their size, @@ -989,16 +825,136 @@ share_type assert_operation::calculate_fee(const fee_schedule_type& k)const return std::max(size_t(1), fc::raw::pack_size(*this) / 1024) * k.assert_op_fee; } -void balance_claim_operation::get_required_auth(flat_set& active_auth_set, flat_set&)const -{ - active_auth_set.insert( fee_payer() ); -} - void balance_claim_operation::validate()const { FC_ASSERT( fee == asset() ); FC_ASSERT( balance_owner_key != public_key_type() ); } +struct required_auth_visitor +{ + typedef void result_type; + + vector& result; + + required_auth_visitor( vector& r ):result(r){} + + /** for most operations this is a no-op */ + template + void operator()(const T& )const {} +}; + +struct required_active_visitor +{ + typedef void result_type; + + flat_set& result; + + required_active_visitor( flat_set& r ):result(r){} + + /** for most operations this is just the fee payer */ + template + void operator()(const T& o)const + { + result.insert( o.fee_payer() ); + } + void operator()(const account_update_operation& o)const + { + /// if owner authority is required, no active authority is required + if( !(o.owner || o.active) ) /// TODO: document why active cannot be updated by active? + result.insert( o.fee_payer() ); + } + void operator()( const proposal_delete_operation& o )const + { + if( !o.using_owner_authority ) + result.insert( o.fee_payer() ); + } + + void operator()( const proposal_update_operation& o )const + { + result.insert( o.fee_payer() ); + for( auto id : o.active_approvals_to_add ) + result.insert(id); + for( auto id : o.active_approvals_to_remove ) + result.insert(id); + } + void operator()( const custom_operation& o )const + { + result.insert( o.required_auths.begin(), o.required_auths.end() ); + } + void operator()( const assert_operation& o )const + { + result.insert( o.fee_payer() ); + result.insert( o.required_auths.begin(), o.required_auths.end() ); + } +}; + +struct required_owner_visitor +{ + typedef void result_type; + + flat_set& result; + + required_owner_visitor( flat_set& r ):result(r){} + + /** for most operations this is a no-op */ + template + void operator()(const T& o)const {} + + void operator()(const account_update_operation& o)const + { + if( o.owner || o.active ) /// TODO: document why active cannot be updated by active? + result.insert( o.account ); + } + + void operator()( const proposal_delete_operation& o )const + { + if( o.using_owner_authority ) + result.insert( o.fee_payer() ); + } + + void operator()( const proposal_update_operation& o )const + { + for( auto id : o.owner_approvals_to_add ) + result.insert(id); + for( auto id : o.owner_approvals_to_remove ) + result.insert(id); + } +}; + + +void operation_get_required_authorities( const operation& op, vector& result ) +{ + op.visit( required_auth_visitor( result ) ); +} +void operation_get_required_active_authorities( const operation& op, flat_set& result ) +{ + op.visit( required_active_visitor( result ) ); +} +void operation_get_required_owner_authorities( const operation& op, flat_set& result ) +{ + op.visit( required_owner_visitor( result ) ); +} + +/** + * @brief Used to validate operations in a polymorphic manner + */ +struct operation_validator +{ + typedef void result_type; + template + void operator()( const T& v )const { v.validate(); } +}; + +void operation_validate( const operation& op ) +{ + op.visit( operation_validator() ); +} + +void op_wrapper::validate()const +{ + operation_validate(op); +} + } } // namespace graphene::chain diff --git a/libraries/chain/proposal_evaluator.cpp b/libraries/chain/proposal_evaluator.cpp index e50854ea..535bf354 100644 --- a/libraries/chain/proposal_evaluator.cpp +++ b/libraries/chain/proposal_evaluator.cpp @@ -36,7 +36,10 @@ void_result proposal_create_evaluator::do_evaluate(const proposal_create_operati // If we're dealing with the genesis authority, make sure this transaction has a sufficient review period. flat_set auths; for( auto& op : o.proposed_ops ) - op.op.visit(operation_get_required_auths(auths, auths)); + { + operation_get_required_active_authorities(op.op, auths); + operation_get_required_owner_authorities(op.op, auths); + } if( auths.find(account_id_type()) != auths.end() ) FC_ASSERT( o.review_period_seconds && *o.review_period_seconds >= global_parameters.genesis_proposal_review_period ); @@ -61,7 +64,13 @@ object_id_type proposal_create_evaluator::do_apply(const proposal_create_operati //Populate the required approval sets flat_set required_active; - _proposed_trx.visit(operation_get_required_auths(required_active, proposal.required_owner_approvals)); + + for( auto& op : _proposed_trx.operations ) + { + operation_get_required_active_authorities(op, required_active); + operation_get_required_owner_authorities(op, proposal.required_owner_approvals); + } + //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(), diff --git a/libraries/chain/transaction.cpp b/libraries/chain/transaction.cpp index c4bbbfd7..c8429508 100644 --- a/libraries/chain/transaction.cpp +++ b/libraries/chain/transaction.cpp @@ -48,7 +48,7 @@ void transaction::validate() const FC_ASSERT( ref_block_num == 0 && ref_block_prefix > 0 ); for( const auto& op : operations ) - op.visit(operation_validator()); + operation_validate(op); } graphene::chain::transaction_id_type graphene::chain::transaction::id() const @@ -88,5 +88,14 @@ void transaction::set_expiration( const block_id_type& reference_block, unsigned relative_expiration = lifetime_intervals; block_id_cache = reference_block; } +void transaction::get_required_authorities( flat_set& active, flat_set& owner, vector& other ) +{ + for( const auto& op : operations ) + { + operation_get_required_active_authorities( op, active ); + operation_get_required_owner_authorities( op, owner ); + operation_get_required_authorities( op, other ); + } +} } } // graphene::chain diff --git a/libraries/chain/transaction_evaluation_state.cpp b/libraries/chain/transaction_evaluation_state.cpp index d810693d..023d809d 100644 --- a/libraries/chain/transaction_evaluation_state.cpp +++ b/libraries/chain/transaction_evaluation_state.cpp @@ -35,21 +35,27 @@ namespace graphene { namespace chain { FC_ASSERT( account.id.instance() != 0 || _is_proposed_trx, "", ("account",account)("is_proposed",_is_proposed_trx) ); - const authority* au = nullptr; + bool valid = false; switch( auth_class ) { case authority::owner: - au = &account.owner; + valid = check_authority( account.owner, auth_class, depth ); break; case authority::active: - au = &account.active; + valid = check_authority( account.active, auth_class, depth ); break; default: FC_ASSERT( false, "Invalid Account Auth Class" ); }; + if( valid ) + approved_by.insert( std::make_pair(account.id, auth_class) ); + return valid; + } + bool transaction_evaluation_state::check_authority( const authority& au, authority::classification auth_class, int depth ) + { uint32_t total_weight = 0; - for( const auto& auth : au->account_auths ) + for( const auto& auth : au.account_auths ) { if( approved_by.find( std::make_pair(auth.first,auth_class) ) != approved_by.end() ) total_weight += auth.second; @@ -68,36 +74,30 @@ namespace graphene { namespace chain { } } } - for( const auto& key : au->key_auths ) + for( const auto& key : au.key_auths ) { if( signed_by( key.first ) ) total_weight += key.second; } - for( const auto& key : au->address_auths ) + for( const auto& key : au.address_auths ) { if( signed_by( key.first ) ) total_weight += key.second; } - if( total_weight >= au->weight_threshold ) - { - approved_by.insert( std::make_pair(account.id, auth_class) ); - return true; - } - return false; + return total_weight >= au.weight_threshold; } bool transaction_evaluation_state::signed_by(const public_key_type& k) { auto itr = _sigs.find(k); - if( itr != _sigs.end() ) - return itr->second = true; - return false; + return itr != _sigs.end() && (itr->second = true); } + bool transaction_evaluation_state::signed_by(const address& k) { for( auto itr = _sigs.begin(); itr != _sigs.end(); ++itr ) - if( itr->first == k ) return true; + if( itr->first == k ) return itr->second = true; return false; } diff --git a/libraries/plugins/account_history/account_history_plugin.cpp b/libraries/plugins/account_history/account_history_plugin.cpp index 6c999176..0c1016d3 100644 --- a/libraries/plugins/account_history/account_history_plugin.cpp +++ b/libraries/plugins/account_history/account_history_plugin.cpp @@ -152,7 +152,10 @@ struct operation_get_impacted_accounts void operator()( const proposal_create_operation& o )const { for( auto op : o.proposed_ops ) - op.op.visit( operation_get_required_auths( _impacted, _impacted ) ); + { + operation_get_required_active_authorities( op.op, _impacted ); + operation_get_required_owner_authorities( op.op, _impacted ); + } } void operator()( const proposal_update_operation& o )const { } @@ -204,7 +207,8 @@ void account_history_plugin_impl::update_account_histories( const signed_block& // get the set of accounts this operation applies to flat_set impacted; - op.op.visit( operation_get_required_auths( impacted, impacted ) ); + operation_get_required_active_authorities( op.op, impacted ); + operation_get_required_owner_authorities( op.op, impacted ); op.op.visit( operation_get_impacted_accounts( oho, _self, impacted ) ); // for each operation this account applies to that is in the config link it into the history diff --git a/libraries/wallet/wallet.cpp b/libraries/wallet/wallet.cpp index 75a6be46..c4953c22 100644 --- a/libraries/wallet/wallet.cpp +++ b/libraries/wallet/wallet.cpp @@ -1332,12 +1332,13 @@ public: { flat_set req_active_approvals; flat_set req_owner_approvals; + vector other_auths; - tx.visit( operation_get_required_auths( req_active_approvals, req_owner_approvals ) ); + tx.get_required_authorities( req_active_approvals, req_owner_approvals, other_auths ); - // TODO: Only sign if the wallet considers ACCOUNTS to be owned. - // Currently the wallet only owns KEYS and will happily sign - // for any account... + for( const auto& auth : other_auths ) + for( const auto& a : auth.account_auths ) + req_active_approvals.insert(a.first); // std::merge lets us de-duplicate account_id's that occur in both // sets, and dump them into a vector (as required by remote_db api) @@ -1347,9 +1348,13 @@ public: req_owner_approvals.begin() , req_owner_approvals.end(), std::back_inserter(v_approving_account_ids)); + /// TODO: fetch the accounts specified via other_auths as well. + vector< optional > approving_account_objects = _remote_db->get_accounts( v_approving_account_ids ); + /// TODO: recursively check one layer deeper in the authority tree for keys + FC_ASSERT( approving_account_objects.size() == v_approving_account_ids.size() ); flat_map< account_id_type, account_object* > approving_account_lut; @@ -1388,6 +1393,11 @@ public: for( const public_key_type& approving_key : v_approving_keys ) approving_key_set.insert( approving_key ); } + for( const authority& a : other_auths ) + { + for( const auto& k : a.key_auths ) + approving_key_set.insert( k.first ); + } tx.set_expiration(get_dynamic_global_properties().head_block_id); @@ -1403,6 +1413,10 @@ public: } tx.sign( *privkey ); } + /// TODO: if transaction has enough signatures to be "valid" don't add any more, + /// there are cases where the wallet may have more keys than strictly necessary and + /// the transaction will be rejected if the transaction validates without requiring + /// all signatures provided } if( broadcast ) diff --git a/tests/tests/authority_tests.cpp b/tests/tests/authority_tests.cpp index 65f9ed27..d0dc5436 100644 --- a/tests/tests/authority_tests.cpp +++ b/tests/tests/authority_tests.cpp @@ -300,13 +300,16 @@ BOOST_AUTO_TEST_CASE( proposed_single_account ) asset nathan_start_balance = db.get_balance(nathan.get_id(), core.get_id()); { flat_set active_set, owner_set; - op.get_required_auth(active_set, owner_set); + operation_get_required_active_authorities(op,active_set); + operation_get_required_owner_authorities(op,owner_set); BOOST_CHECK_EQUAL(active_set.size(), 1); BOOST_CHECK_EQUAL(owner_set.size(), 0); BOOST_CHECK(*active_set.begin() == moneyman.get_id()); active_set.clear(); - op.proposed_ops.front().get_required_auth(active_set, owner_set); + //op.proposed_ops.front().get_required_auth(active_set, owner_set); + operation_get_required_active_authorities( op.proposed_ops.front().op, active_set ); + operation_get_required_owner_authorities( op.proposed_ops.front().op, owner_set ); BOOST_CHECK_EQUAL(active_set.size(), 1); BOOST_CHECK_EQUAL(owner_set.size(), 0); BOOST_CHECK(*active_set.begin() == nathan.id); @@ -965,8 +968,8 @@ BOOST_FIXTURE_TEST_CASE( bogus_signature, database_fixture ) trx.sign(alice_key ); flat_set active_set, owner_set; - xfer_op.get().get_required_auth(active_set, owner_set); - // wdump( (active_set)(owner_set)(alice_key_id) (alice_account_object) ); + vector others; + trx.get_required_authorities( active_set, owner_set, others ); PUSH_TX( db, trx, skip ); diff --git a/tests/tests/operation_tests.cpp b/tests/tests/operation_tests.cpp index e32e35bd..e61d0f89 100644 --- a/tests/tests/operation_tests.cpp +++ b/tests/tests/operation_tests.cpp @@ -1088,8 +1088,6 @@ BOOST_AUTO_TEST_CASE( limit_order_fill_or_kill ) BOOST_AUTO_TEST_CASE( fill_order ) { try { fill_order_operation o; - flat_set auths; - o.get_required_auth(auths, auths); BOOST_CHECK_THROW(o.validate(), fc::exception); o.calculate_fee(db.current_fee_schedule()); } FC_LOG_AND_RETHROW() }