Progress bitshares#210: Apply requested changes

libraries/app/database_api.cpp

@@ -2268,65 +2268,71 @@ set<address> database_api::get_potential_address_signatures( const signed_transa
    return my->get_potential_address_signatures( trx );
 }
 
-set<public_key_type> database_api_impl::get_potential_signatures( const signed_transaction& trx )const
+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 = [&](account_id_type id) {
+      const auto& auth = id(_db).active;
+      for (const auto& k : auth.get_keys())
+         result.insert(k);
+      return &auth;
+   };
+   auto get_owner = [&](account_id_type id) {
+      const auto& auth = id(_db).owner;
+      for (const auto& k : auth.get_keys())
+         result.insert(k);
+      return &auth;
+   };
+
+   trx.get_required_signatures(_db.get_chain_id(),
+                               flat_set<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 );
-   for( const auto& auth : other )
-      for( const auto& key : auth.get_keys() )
-         result.insert( key );
+   trx.get_required_authorities(required_active, required_owner, other, ignore_custom_op_reqd_auths);
+   for (const auto& auth : other)
+      for (const auto& key : auth.get_keys())
+         result.insert(key);
 
    return result;
 }
 
-set<address> database_api_impl::get_potential_address_signatures( const signed_transaction& trx )const
+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 = [&](account_id_type id) {
+      const auto& auth = id(_db).active;
+      for (const auto& k : auth.get_addresses())
+         result.insert(k);
+      return &auth;
+   };
+   auto get_owner = [&](account_id_type id) {
+      const auto& auth = id(_db).owner;
+      for (const auto& k : auth.get_addresses())
+         result.insert(k);
+      return &auth;
+   };
+
+   trx.get_required_signatures(_db.get_chain_id(),
+                               flat_set<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

@@ -647,17 +647,8 @@ processed_transaction database::_apply_transaction(const signed_transaction& trx
       auto get_active = [&]( account_id_type id ) { return &id(*this).active; };
       auto get_owner  = [&]( account_id_type id ) { return &id(*this).owner;  };
 
-      // See bitshares-core issue #210 for discussion
-      // The custom_operation::get_required_active_authorities() method initially failed to report the authorities
-      // from the custom_operaton::required_auths field. This was a bug. It's a simple fix in that method, but the
-      // fix is a hardfork, and thus we need a hardfork guard. Since that method cannot access chain time, we must
-      // implement the guard here, and skip the call to get_required_active_authorities() prior to the hardfork.
-      // Therefore, if the head_block_time() is prior to the 210 hardfork, we ignore the required auths specified
-      // by the custom_operation.
-      bool ignore_custom_operation_required_auths = (head_block_time() <= HARDFORK_CORE_210_TIME);
-
       trx.verify_authority(chain_id, get_active, get_owner, allow_non_immediate_owner,
-                           ignore_custom_operation_required_auths,
+                           MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(head_block_time()),
                            get_global_properties().parameters.max_authority_depth);
    }
 

libraries/chain/db_notify.cpp

@@ -25,8 +25,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 )
    {
@@ -153,10 +158,10 @@ 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 );
-      for( auto& o : other )
-         add_authority_accounts( _impacted, o );
+      for (const auto& proposed_op : op.proposed_ops)
+         operation_get_required_authorities(proposed_op.op, _impacted, _impacted, other, _ignore_custom_op_reqd_auths);
+      for (auto& o : other)
+         add_authority_accounts(_impacted, o);
    }
    void operator()( const proposal_update_operation& op )
    {
@@ -213,7 +218,9 @@ struct get_impacted_account_visitor
    }
    void operator()( const custom_operation& op )
    {
-      _impacted.insert( op.fee_payer() ); // payer
+      _impacted.insert(op.fee_payer()); // payer
+      if (!_ignore_custom_op_reqd_auths)
+         _impacted.insert(op.required_auths.begin(), op.required_auths.end());
    }
    void operator()( const assert_operation& op )
    {
@@ -283,20 +290,17 @@ struct get_impacted_account_visitor
    }
 };
 
-void graphene::chain::operation_get_impacted_accounts( const operation& op, flat_set<account_id_type>& result )
-{
-  get_impacted_account_visitor vtor = get_impacted_account_visitor( result );
-  op.visit( vtor );
+void graphene::chain::operation_get_impacted_accounts(const operation& op, flat_set<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 )
-{
-  for( const auto& op : tx.operations )
-    operation_get_impacted_accounts( op, 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, 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 +346,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 +410,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 +465,7 @@ void database::notify_changed_objects()
    if( _undo_db.enabled() ) 
    {
       const auto& head_undo = _undo_db.head();
+      auto chain_time = head_block_time();
 
       // New
       if( !new_objects.empty() )
@@ -469,7 +477,8 @@ void database::notify_changed_objects()
           new_ids.push_back(item);
           auto obj = find_object(item);
           if(obj != nullptr)
-            get_relevant_accounts(obj, new_accounts_impacted);
+            get_relevant_accounts(obj, new_accounts_impacted,
+                                  MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(chain_time));
         }
 
         if( new_ids.size() )
@@ -484,7 +493,8 @@ void database::notify_changed_objects()
         for( const auto& item : head_undo.old_values )
         {
           changed_ids.push_back(item.first);
-          get_relevant_accounts(item.second.get(), changed_accounts_impacted);
+          get_relevant_accounts(item.second.get(), changed_accounts_impacted,
+                                MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(chain_time));
         }
 
         if( changed_ids.size() )
@@ -502,13 +512,14 @@ void database::notify_changed_objects()
           removed_ids.emplace_back( item.first );
           auto obj = item.second.get();
           removed.emplace_back( obj );
-          get_relevant_accounts(obj, removed_accounts_impacted);
+          get_relevant_accounts(obj, removed_accounts_impacted,
+                                MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(chain_time));
         }
 
         if( removed_ids.size() )
-           GRAPHENE_TRY_NOTIFY( removed_objects, removed_ids, removed, removed_accounts_impacted)
+           GRAPHENE_TRY_NOTIFY(removed_objects, removed_ids, removed, removed_accounts_impacted)
       }
    }
-} FC_CAPTURE_AND_LOG( (0) ) }
+} FC_LOG_AND_RETHROW() }
 
 } }

libraries/chain/hardfork.d/CORE_210.hf

@@ -1,4 +1,6 @@
 // #210 Check authorities on custom_operation
 #ifndef HARDFORK_CORE_210_TIME
 #define HARDFORK_CORE_210_TIME (fc::time_point_sec(1893456000)) // Jan 1 00:00:00 2030 (Not yet scheduled)
+// Bugfix: pre-HF 210, custom_operation's required_auths field was ignored.
+#define MUST_IGNORE_CUSTOM_OP_REQD_AUTHS(chain_time) (chain_time <= HARDFORK_CORE_210_TIME)
 #endif

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