GRPH-63 Authorize Transaction Bug

libraries/app/database_api.cpp

@@ -26,6 +26,7 @@
 #include <graphene/chain/get_config.hpp>
 #include <graphene/chain/tournament_object.hpp>
 #include <graphene/chain/account_object.hpp>
+#include <graphene/chain/hardfork.hpp>
 
 #include <fc/bloom_filter.hpp>
 #include <fc/smart_ref_impl.hpp>
@@ -1773,10 +1774,12 @@ set<public_key_type> database_api::get_required_signatures( const signed_transac
 set<public_key_type> database_api_impl::get_required_signatures( const signed_transaction& trx, const flat_set<public_key_type>& available_keys )const
 {
    wdump((trx)(available_keys));
+   bool allow_non_immediate_owner = ( _db.head_block_time() >= HARDFORK_1002_TIME );
    auto result = trx.get_required_signatures( _db.get_chain_id(),
                                        available_keys,
                                        [&]( account_id_type id ){ return &id(_db).active; },
                                        [&]( account_id_type id ){ return &id(_db).owner; },
+                                       allow_non_immediate_owner,
                                        _db.get_global_properties().parameters.max_authority_depth );
    wdump((result));
    return result;
@@ -1794,7 +1797,11 @@ 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
 {
    wdump((trx));
+   auto chain_time = _db.head_block_time();
+   bool allow_non_immediate_owner = ( chain_time >= HARDFORK_1002_TIME );
+
    set<public_key_type> result;
+
    trx.get_required_signatures(
       _db.get_chain_id(),
       flat_set<public_key_type>(),
@@ -1812,15 +1819,28 @@ set<public_key_type> database_api_impl::get_potential_signatures( const signed_t
             result.insert(k);
          return &auth;
       },
+      allow_non_immediate_owner,
       _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 );
+
    wdump((result));
    return result;
 }
 
 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_1002_TIME );
+
    set<address> result;
    trx.get_required_signatures(
       _db.get_chain_id(),
@@ -1839,6 +1859,7 @@ set<address> database_api_impl::get_potential_address_signatures( const signed_t
             result.insert(k);
          return &auth;
       },
+      allow_non_immediate_owner,
       _db.get_global_properties().parameters.max_authority_depth
    );
    return result;
@@ -1851,9 +1872,11 @@ bool database_api::verify_authority( const signed_transaction& trx )const
 
 bool database_api_impl::verify_authority( const signed_transaction& trx )const
 {
+   bool allow_non_immediate_owner = ( _db.head_block_time() >= HARDFORK_1002_TIME );
    trx.verify_authority( _db.get_chain_id(),
                          [&]( account_id_type id ){ return &id(_db).active; },
                          [&]( account_id_type id ){ return &id(_db).owner; },
+                         allow_non_immediate_owner,
                           _db.get_global_properties().parameters.max_authority_depth );
    return true;
 }

libraries/chain/db_block.cpp

@@ -680,9 +680,10 @@ processed_transaction database::_apply_transaction(const signed_transaction& trx
 
    if( !(skip & (skip_transaction_signatures | skip_authority_check) ) )
    {
+      bool allow_non_immediate_owner = ( head_block_time() >= HARDFORK_1002_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, get_global_properties().parameters.max_authority_depth );
+      trx.verify_authority( chain_id, get_active, get_owner, allow_non_immediate_owner, 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/hardfork.d/1002.hf

@@ -0,0 +1,7 @@
+// added transaction size check
+// Bug fix of update commitee member update
+// bitshares-core issue #584 Owner keys of non-immediately required accounts can not authorize a transaction
+// https://github.com/bitshares/bitshares-core/issues/584
+#ifndef HARDFORK_1002_TIME
+#define HARDFORK_1002_TIME (fc::time_point_sec( 1567488600 )) //Tuesday, 3 September 2019 05:30:00 GMT
+#endif

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

@@ -141,13 +141,27 @@ namespace graphene { namespace chain {
          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,
+         bool allow_non_immediate_owner,
          uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH
          )const;
 
+      /**
+       * Checks whether signatures in this signed transaction are sufficient to authorize the transaction.
+       *   Throws an exception when failed.
+       *
+       * @param chain_id the ID of a block chain
+       * @param get_active callback function to retrieve active authorities of a given account
+       * @param get_owner  callback function to retrieve owner authorities of a given account
+       * @param allow_non_immediate_owner whether to allow owner authority of non-immediately
+       *            required accounts to authorize operations in the transaction
+       * @param max_recursion maximum level of recursion when verifying, since an account
+       *            can have another account in active authorities and/or owner authorities
+       */
       void verify_authority(
          const chain_id_type& chain_id,
          const std::function<const authority*(account_id_type)>& get_active,
          const std::function<const authority*(account_id_type)>& get_owner,
+         bool allow_non_immediate_owner,
          uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH )const;
 
       /**
@@ -162,6 +176,7 @@ namespace graphene { namespace chain {
          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,
+         bool allow_non_immediate_owner,
          uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH
          ) const;
 
@@ -171,11 +186,31 @@ namespace graphene { namespace chain {
 
       /// Removes all operations and signatures
       void clear() { operations.clear(); signatures.clear(); }
+
+      /** Removes all signatures */
+      void clear_signatures() { signatures.clear(); }
    };
 
+   /**
+    * Checks whether given public keys and approvals are sufficient to authorize given operations.
+    *   Throws an exception when failed.
+    *
+    * @param ops a vector of operations
+    * @param sigs a set of public keys
+    * @param get_active callback function to retrieve active authorities of a given account
+    * @param get_owner  callback function to retrieve owner authorities of a given account
+    * @param allow_non_immediate_owner whether to allow owner authority of non-immediately
+    *            required accounts to authorize operations
+    * @param max_recursion maximum level of recursion when verifying, since an account
+    *            can have another account in active authorities and/or owner authorities
+    * @param allow_committee whether to allow the special "committee account" to authorize the operations
+    * @param active_approvals accounts that approved the operations with their active authories
+    * @param owner_approvals accounts that approved the operations with their owner authories
+    */
    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,
+                          bool allow_non_immediate_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>(),

libraries/chain/proposal_object.cpp

@@ -24,6 +24,7 @@
 #include <graphene/chain/database.hpp>
 #include <graphene/chain/account_object.hpp>
 #include <graphene/chain/proposal_object.hpp>
+#include <graphene/chain/hardfork.hpp>
 
 namespace graphene { namespace chain {
 
@@ -32,10 +33,12 @@ bool proposal_object::is_authorized_to_execute(database& db) const
    transaction_evaluation_state dry_run_eval(&db);
 
    try {
+      bool allow_non_immediate_owner = ( db.head_block_time() >= HARDFORK_1002_TIME );
       verify_authority( proposed_transaction.operations, 
                         available_key_approvals,
                         [&]( account_id_type id ){ return &id(db).active; },
                         [&]( account_id_type id ){ return &id(db).owner;  },
+                        allow_non_immediate_owner,
                         db.get_global_properties().parameters.max_authority_depth,
                         true, /* allow committeee */
                         available_active_approvals,

libraries/chain/protocol/transaction.cpp

@@ -97,6 +97,8 @@ void transaction::get_required_authorities( flat_set<account_id_type>& active, f
 {
    for( const auto& op : operations )
       operation_get_required_authorities( op, active, owner, other );
+   for( const auto& account : owner )
+      active.erase( account );
 }
 
 
@@ -159,7 +161,7 @@ struct sign_state
       bool check_authority( account_id_type id )
       {
          if( approved_by.find(id) != approved_by.end() ) return true;
-         return check_authority( get_active(id) );
+         return check_authority( get_active(id) ) || ( allow_non_immediate_owner && check_authority( get_owner(id) ) );
       }
 
       /**
@@ -194,7 +196,8 @@ struct sign_state
             {
                if( depth == max_recursion )
                   continue;
-               if( check_authority( get_active( a.first ), depth+1 ) )
+               if( check_authority( get_active( a.first ), depth+1 ) 
+                     || ( allow_non_immediate_owner && check_authority( get_owner( a.first ), depth+1 ) ) )
                {
                   approved_by.insert( a.first );
                   total_weight += a.second;
@@ -225,27 +228,37 @@ struct sign_state
       }
 
       sign_state( const flat_set<public_key_type>& sigs,
-                  const std::function<const authority*(account_id_type)>& a,
+                  const std::function<const authority*(account_id_type)>& active,
+                  const std::function<const authority*(account_id_type)>& owner,
+                  bool allow_owner,
+                  uint32_t max_recursion_depth = GRAPHENE_MAX_SIG_CHECK_DEPTH,
                   const flat_set<public_key_type>& keys = flat_set<public_key_type>() )
-      :get_active(a),available_keys(keys)
+      :  get_active(active),
+         get_owner(owner),
+         allow_non_immediate_owner(allow_owner),
+         max_recursion(max_recursion_depth),
+         available_keys(keys)
       {
          for( const auto& key : sigs )
             provided_signatures[ key ] = false;
          approved_by.insert( GRAPHENE_TEMP_ACCOUNT  );
       }
 
       const std::function<const authority*(account_id_type)>& get_active;
-      const flat_set<public_key_type>&                        available_keys;
+      const std::function<const authority*(account_id_type)>& get_owner;
+      const bool                       allow_non_immediate_owner;
+      const uint32_t                   max_recursion;
+      const flat_set<public_key_type>& available_keys;
 
       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,
+                       bool allow_non_immediate_owner,
                        uint32_t max_recursion_depth,
                        bool  allow_committe,
                        const flat_set<account_id_type>& active_aprovals,
@@ -262,8 +275,7 @@ void verify_authority( const vector<operation>& ops, const flat_set<public_key_t
       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;
+   sign_state s( sigs, get_active, get_owner, allow_non_immediate_owner, max_recursion_depth );
    for( auto& id : active_aprovals )
       s.approved_by.insert( id );
    for( auto& id : owner_approvals )
@@ -318,30 +330,31 @@ 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,
+   bool allow_non_immediate_owner,
    uint32_t max_recursion_depth )const
 {
    flat_set<account_id_type> required_active;
    flat_set<account_id_type> required_owner;
    vector<authority> other;
    get_required_authorities( required_active, required_owner, other );
 
-
-   sign_state s(get_signature_keys( chain_id ),get_active,available_keys);
-   s.max_recursion = max_recursion_depth;
+   const flat_set<public_key_type>& signature_keys = get_signature_keys(chain_id);
+   sign_state s( signature_keys, get_active, get_owner, allow_non_immediate_owner, max_recursion_depth, 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.check_authority( active  ) || s.check_authority( get_owner( active ) );
 
    s.remove_unused_signatures();
 
    set<public_key_type> result;
 
    for( auto& provided_sig : s.provided_signatures )
-      if( available_keys.find( provided_sig.first ) != available_keys.end() )
+      if( available_keys.find( provided_sig.first ) != available_keys.end() 
+            && signature_keys.find( provided_sig.first ) == signature_keys.end() )
          result.insert( provided_sig.first );
 
    return result;
@@ -352,6 +365,7 @@ set<public_key_type> signed_transaction::minimize_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,
+   bool allow_non_immediate_owner,
    uint32_t max_recursion
    ) const
 {
@@ -363,7 +377,7 @@ set<public_key_type> signed_transaction::minimize_required_signatures(
       result.erase( k );
       try
       {
-         graphene::chain::verify_authority( operations, result, get_active, get_owner, max_recursion );
+         graphene::chain::verify_authority( operations, result, get_active, get_owner, allow_non_immediate_owner, max_recursion );
          continue;  // element stays erased if verify_authority is ok
       }
       catch( const tx_missing_owner_auth& e ) {}
@@ -378,9 +392,15 @@ void signed_transaction::verify_authority(
    const chain_id_type& chain_id,
    const std::function<const authority*(account_id_type)>& get_active,
    const std::function<const authority*(account_id_type)>& get_owner,
+   bool allow_non_immediate_owner,
    uint32_t max_recursion )const
 { try {
-   graphene::chain::verify_authority( operations, get_signature_keys( chain_id ), get_active, get_owner, max_recursion );
+   graphene::chain::verify_authority( operations,
+                                      get_signature_keys( chain_id ),
+                                      get_active,
+                                      get_owner,
+                                      allow_non_immediate_owner,
+                                      max_recursion );
 } FC_CAPTURE_AND_RETHROW( (*this) ) }
 
 } } // graphene::chain