Refactor Authority Checking

transaction_evaluation_state no longer tracks authority validation.
Authority validation is now compeltely independent of the database.

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
 

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) ) }
 

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" )

libraries/chain/include/graphene/chain/protocol/transaction.hpp

@@ -122,11 +122,13 @@ namespace graphene { namespace chain {
        */
       set<public_key_type> get_required_signatures( const flat_set<public_key_type>& available_keys,
                                                     const std::function<const authority*(account_id_type)>& get_active,
-                                                    const std::function<const authority*(account_id_type)>& get_owner
+                                                    const std::function<const authority*(account_id_type)>& get_owner,
+                                                    uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH
                                                     )const;
 
       void verify_authority( const std::function<const authority*(account_id_type)>& get_active,
-                             const std::function<const authority*(account_id_type)>& get_owner  )const;
+                             const std::function<const authority*(account_id_type)>& get_owner,
+                             uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH )const;
 
       flat_set<public_key_type> get_signature_keys()const;
 
@@ -139,6 +141,8 @@ namespace graphene { namespace chain {
    void verify_authority( const vector<operation>& ops, const flat_set<public_key_type>& sigs, 
                           const std::function<const authority*(account_id_type)>& get_active,
                           const std::function<const authority*(account_id_type)>& get_owner,
+                          uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH,
+                          bool allow_committe = false,
                           const flat_set<account_id_type>& active_aprovals = flat_set<account_id_type>(),
                           const flat_set<account_id_type>& owner_approvals = flat_set<account_id_type>());
 

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<pair<object_id_type,authority::classification>> approved_by;
-         */
-
-         /// Used to look up new objects using transaction relative IDs
          vector<operation_result> 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<public_key_type, bool>  _sigs;
          const signed_transaction*        _trx = nullptr;
          database*                        _db = nullptr;
          bool                             _is_proposed_trx = false;

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;

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<public_key_type,bool>   provided_signatures;
       flat_set<account_id_type>        approved_by;
+      uint32_t                         max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH;
 };
 
 
 void verify_authority( const vector<operation>& ops, const flat_set<public_key_type>& sigs, 
                        const std::function<const authority*(account_id_type)>& get_active,
                        const std::function<const authority*(account_id_type)>& get_owner,
+                       uint32_t max_recursion_depth,
+                       bool  allow_committe,
                        const flat_set<account_id_type>& active_aprovals,
                        const flat_set<account_id_type>& owner_approvals )
 { try {
@@ -192,7 +195,12 @@ void verify_authority( const vector<operation>& ops, const flat_set<public_key_t
    for( const auto& op : ops )
       operation_get_required_authorities( op, required_active, required_owner, other );
 
+   if( !allow_committe )
+      GRAPHENE_ASSERT( required_active.find(GRAPHENE_COMMITTEE_ACCOUNT) == required_active.end(),
+                       invalid_committee_approval, "Committee account may only propose transactions" );
+
    sign_state s(sigs,get_active);
+   s.max_recursion = max_recursion_depth;
    for( auto& id : active_aprovals )
       s.approved_by.insert( id );
    for( auto& id : owner_approvals )
@@ -229,7 +237,8 @@ flat_set<public_key_type> signed_transaction::get_signature_keys()const
 
 set<public_key_type> signed_transaction::get_required_signatures( const flat_set<public_key_type>& available_keys,
                                               const std::function<const authority*(account_id_type)>& get_active,
-                                              const std::function<const authority*(account_id_type)>& get_owner )const
+                                              const std::function<const authority*(account_id_type)>& get_owner,
+                                              uint32_t max_recursion_depth )const
 {
    flat_set<account_id_type> required_active;
    flat_set<account_id_type> required_owner;
@@ -238,6 +247,7 @@ set<public_key_type> 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<public_key_type> signed_transaction::get_required_signatures( const flat_set
 
 
 void signed_transaction::verify_authority( const std::function<const authority*(account_id_type)>& get_active,
-                                 const std::function<const authority*(account_id_type)>& get_owner  )const
+                                           const std::function<const authority*(account_id_type)>& 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

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<proposal_object>(PUSH_TX( db, trx ).operation_results.front().get<object_id_type>());
 
@@ -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); };