Merge pull request #1629 from nathanhourt/issue-210-hardfork

Issue #210: Check required_auths on custom_operation

Author: nathanielhourt

libraries/app/database_api.cpp

@@ -2270,34 +2270,36 @@ set<address> database_api::get_potential_address_signatures( const signed_transa
 
 set<public_key_type> 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<public_key_type> result;
-   trx.get_required_signatures(
-      _db.get_chain_id(),
-      flat_set<public_key_type>(),
-      [&]( 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 = [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 );
+      return &auth;
+   };
+
+   trx.get_required_signatures( _db.get_chain_id(),
+                                flat_set<public_key_type>(),
+                                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<account_id_type> required_active;
    flat_set<account_id_type> required_owner;
    vector<authority> other;
-   trx.get_required_authorities( required_active, required_owner, 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 );
@@ -2307,26 +2309,30 @@ set<public_key_type> database_api_impl::get_potential_signatures( const signed_t
 
 set<address> 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<address> result;
-   trx.get_required_signatures(
-      _db.get_chain_id(),
-      flat_set<public_key_type>(),
-      [&]( 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 = [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;
+      for (const auto& k : auth.get_addresses())
+         result.insert( k );
+      return &auth;
+   };
+
+   trx.get_required_signatures( _db.get_chain_id(),
+                                flat_set<public_key_type>(),
+                                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)
    {

libraries/chain/db_block.cpp

@@ -644,13 +644,12 @@ 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;  };
-      trx.verify_authority( chain_id,
-                            get_active,
-                            get_owner,
-                            allow_non_immediate_owner,
-                            get_global_properties().parameters.max_authority_depth );
+      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()),
+                           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

libraries/chain/db_notify.cpp

@@ -17,6 +17,7 @@
 #include <graphene/chain/vesting_balance_object.hpp>
 #include <graphene/chain/transaction_history_object.hpp>
 #include <graphene/chain/impacted.hpp>
+#include <graphene/chain/hardfork.hpp>
 
 using namespace fc;
 using namespace graphene::chain;
@@ -25,8 +26,13 @@ using namespace graphene::chain;
 struct get_impacted_account_visitor
 {
    flat_set<account_id_type>& _impacted;
-   get_impacted_account_visitor( flat_set<account_id_type>& impact ):_impacted(impact) {}
-   typedef void result_type;
+   bool _ignore_custom_op_reqd_auths;
+
+   get_impacted_account_visitor( flat_set<account_id_type>& 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 )
    {
@@ -154,7 +160,7 @@ struct get_impacted_account_visitor
       _impacted.insert( op.fee_payer() ); // fee_paying_account
       vector<authority> other;
       for( const auto& proposed_op : op.proposed_ops )
-         operation_get_required_authorities( proposed_op.op, _impacted, _impacted, other );
+         operation_get_required_authorities( proposed_op.op, _impacted, _impacted, other, _ignore_custom_op_reqd_auths );
       for( auto& o : other )
          add_authority_accounts( _impacted, o );
    }
@@ -214,6 +220,8 @@ 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() );
    }
    void operator()( const assert_operation& op )
    {
@@ -283,20 +291,17 @@ struct get_impacted_account_visitor
    }
 };
 
-void graphene::chain::operation_get_impacted_accounts( const operation& op, flat_set<account_id_type>& result )
-{
-  get_impacted_account_visitor vtor = get_impacted_account_visitor( result );
+void graphene::chain::operation_get_impacted_accounts( const operation& op, flat_set<account_id_type>& 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<account_id_type>& result )
-{
+void graphene::chain::transaction_get_impacted_accounts( const transaction& tx, flat_set<account_id_type>& result, bool ignore_custom_operation_required_auths ) {
   for( const auto& op : tx.operations )
-    operation_get_impacted_accounts( op, result );
+    operation_get_impacted_accounts( op, result, ignore_custom_operation_required_auths );
 }
 
-void get_relevant_accounts( const object* obj, flat_set<account_id_type>& accounts )
-{
+void get_relevant_accounts( const object* obj, flat_set<account_id_type>& accounts, bool ignore_custom_operation_required_auths ) {
    if( obj->id.space() == protocol_ids )
    {
       switch( (object_type)obj->id.type() )
@@ -342,12 +347,14 @@ void get_relevant_accounts( const object* obj, flat_set<account_id_type>& accoun
         } case proposal_object_type:{
            const auto& aobj = dynamic_cast<const proposal_object*>(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<const operation_history_object*>(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<const withdraw_permission_object*>(obj);
@@ -404,7 +411,8 @@ void get_relevant_accounts( const object* obj, flat_set<account_id_type>& accoun
            } case impl_transaction_history_object_type:{
               const auto& aobj = dynamic_cast<const transaction_history_object*>(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<const blinded_balance_object*>(obj);
@@ -458,6 +466,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 +478,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 +494,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,11 +513,12 @@ 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) ) }

libraries/chain/hardfork.d/CORE_210.hf

@@ -0,0 +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

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<graphene::chain::account_id_type>& result );
+void operation_get_impacted_accounts( const graphene::chain::operation& op,
+                                      fc::flat_set<graphene::chain::account_id_type>& result,
+                                      bool ignore_custom_operation_required_auths );
 
-void transaction_get_impacted_accounts(
-   const graphene::chain::transaction& tx,
-   fc::flat_set<graphene::chain::account_id_type>& result
-   );
+void transaction_get_impacted_accounts( const graphene::chain::transaction& tx,
+                                        fc::flat_set<graphene::chain::account_id_type>& result,
+                                        bool ignore_custom_operation_required_auths );
 
-} } // graphene::app
+} } // graphene::app

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<account_id_type> _required_active_auths;
+         flat_set<account_id_type> _required_owner_auths;
 
          hardfork_visitor_1479 vtor_1479;
    };