Merge pull request #1251 from bitshares/trx-signees

Improve block generation performance

libraries/chain/db_block.cpp

@@ -448,7 +448,7 @@ signed_block database::_generate_block(
       FC_ASSERT( fc::raw::pack_size(pending_block) <= get_global_properties().parameters.maximum_block_size );
    }
 
-   push_block( pending_block, skip );
+   push_block( pending_block, skip | skip_transaction_signatures ); // skip authority check when pushing self-generated blocks
 
    return pending_block;
 } FC_CAPTURE_AND_RETHROW( (witness_id) ) }

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

@@ -165,12 +165,30 @@ namespace graphene { namespace chain {
          uint32_t max_recursion = GRAPHENE_MAX_SIG_CHECK_DEPTH
          ) const;
 
-      flat_set<public_key_type> get_signature_keys( const chain_id_type& chain_id )const;
+      /**
+       * @brief Extract public keys from signatures with given chain ID.
+       * @param chain_id A chain ID
+       * @return Public keys
+       * @note If @ref signees is empty, E.G. when it's the first time calling
+       *       this function for the signed transaction, public keys will be
+       *       extracted with given chain ID, and be stored into the mutable
+       *       @ref signees field, then @ref signees will be returned;
+       *       otherwise, the @ref chain_id parameter will be ignored, and
+       *       @ref signees will be returned directly.
+       */
+      const flat_set<public_key_type>& get_signature_keys( const chain_id_type& chain_id )const;
 
+      /** Signatures */
       vector<signature_type> signatures;
 
-      /// Removes all operations and signatures
-      void clear() { operations.clear(); signatures.clear(); }
+      /** Public keys extracted from signatures */
+      mutable flat_set<public_key_type> signees;
+
+      /// Removes all operations, signatures and signees
+      void clear() { operations.clear(); signatures.clear(); signees.clear(); }
+
+      /// Removes all signatures and signees
+      void clear_signatures() { signatures.clear(); signees.clear(); }
    };
 
    void verify_authority( const vector<operation>& ops, const flat_set<public_key_type>& sigs,
@@ -209,5 +227,6 @@ namespace graphene { namespace chain {
 } } // graphene::chain
 
 FC_REFLECT( graphene::chain::transaction, (ref_block_num)(ref_block_prefix)(expiration)(operations)(extensions) )
+// Note: not reflecting signees field for backward compatibility; in addition, it should not be in p2p messages
 FC_REFLECT_DERIVED( graphene::chain::signed_transaction, (graphene::chain::transaction), (signatures) )
 FC_REFLECT_DERIVED( graphene::chain::processed_transaction, (graphene::chain::signed_transaction), (operation_results) )

libraries/chain/protocol/transaction.cpp

@@ -72,6 +72,7 @@ const signature_type& graphene::chain::signed_transaction::sign(const private_ke
 {
    digest_type h = sig_digest( chain_id );
    signatures.push_back(key.sign_compact(h));
+   signees.clear(); // Clear signees since it may be inconsistent after added a new signature
    return signatures.back();
 }
 
@@ -302,22 +303,27 @@ void verify_authority( const vector<operation>& ops, const flat_set<public_key_t
 } FC_CAPTURE_AND_RETHROW( (ops)(sigs) ) }
 
 
-flat_set<public_key_type> signed_transaction::get_signature_keys( const chain_id_type& chain_id )const
+const flat_set<public_key_type>& signed_transaction::get_signature_keys( const chain_id_type& chain_id )const
 { try {
-   auto d = sig_digest( chain_id );
-   flat_set<public_key_type> result;
-   for( const auto&  sig : signatures )
+   // Strictly we should check whether the given chain ID is same as the one used to initialize the `signees` field.
+   // However, we don't pass in another chain ID so far, for better performance, we skip the check.
+   if( signees.empty() && !signatures.empty() )
    {
-      GRAPHENE_ASSERT(
-         result.insert( fc::ecc::public_key(sig,d) ).second,
-         tx_duplicate_sig,
-         "Duplicate Signature detected" );
+      auto d = sig_digest( chain_id );
+      flat_set<public_key_type> result;
+      for( const auto&  sig : signatures )
+      {
+         GRAPHENE_ASSERT(
+            result.insert( fc::ecc::public_key(sig,d) ).second,
+            tx_duplicate_sig,
+            "Duplicate Signature detected" );
+      }
+      signees = std::move( result );
    }
-   return result;
+   return signees;
 } FC_CAPTURE_AND_RETHROW() }
 
 
-
 set<public_key_type> signed_transaction::get_required_signatures(
    const chain_id_type& chain_id,
    const flat_set<public_key_type>& available_keys,
@@ -330,7 +336,7 @@ set<public_key_type> signed_transaction::get_required_signatures(
    vector<authority> other;
    get_required_authorities( required_active, required_owner, other );
 
-   flat_set<public_key_type> signature_keys = get_signature_keys( chain_id );
+   const flat_set<public_key_type>& signature_keys = get_signature_keys( chain_id );
    sign_state s( signature_keys, get_active, available_keys );
    s.max_recursion = max_recursion_depth;
 

libraries/wallet/wallet.cpp

@@ -1912,7 +1912,7 @@ class wallet_api_impl
       owned_keys.reserve( pks.size() );
       std::copy_if( pks.begin(), pks.end(), std::inserter(owned_keys, owned_keys.end()),
                     [this](const public_key_type& pk){ return _keys.find(pk) != _keys.end(); } );
-      tx.signatures.clear();
+      tx.clear_signatures();
       set<public_key_type> approving_key_set = _remote_db->get_required_signatures( tx, owned_keys );
 
       auto dyn_props = get_dynamic_global_properties();
@@ -1931,7 +1931,7 @@ class wallet_api_impl
       for (;;)
       {
          tx.set_expiration( dyn_props.time + fc::seconds(30 + expiration_time_offset) );
-         tx.signatures.clear();
+         tx.clear_signatures();
 
          for( const public_key_type& key : approving_key_set )
             tx.sign( get_private_key(key), _chain_id );
@@ -2062,7 +2062,6 @@ class wallet_api_impl
       trx.operations = {op};
       set_operation_fees( trx, _remote_db->get_global_properties().parameters.current_fees);
       trx.validate();
-      idump((broadcast));
 
       return sign_transaction(trx, broadcast);
    }
@@ -2087,7 +2086,6 @@ class wallet_api_impl
       trx.operations = {op};
       set_operation_fees( trx, _remote_db->get_global_properties().parameters.current_fees);
       trx.validate();
-      idump((broadcast));
 
       return sign_transaction(trx, broadcast);
    }

tests/common/database_fixture.cpp

@@ -635,7 +635,6 @@ const account_object& database_fixture::create_account(
       trx.validate();
 
       processed_transaction ptx = db.push_transaction(trx, ~0);
-      //wdump( (ptx) );
       const account_object& result = db.get<account_object>(ptx.operation_results[0].get<object_id_type>());
       trx.operations.clear();
       return result;
@@ -719,7 +718,6 @@ const limit_order_object* database_fixture::create_sell_order( const account_obj
                                                 const time_point_sec order_expiration,
                                                 const price& fee_core_exchange_rate )
 {
-   //wdump((amount)(recv));
    limit_order_create_operation buy_order;
    buy_order.seller = user.id;
    buy_order.amount_to_sell = amount;
@@ -731,7 +729,6 @@ const limit_order_object* database_fixture::create_sell_order( const account_obj
    auto processed = db.push_transaction(trx, ~0);
    trx.operations.clear();
    verify_asset_supplies(db);
-   //wdump((processed));
    return db.find<limit_order_object>( processed.operation_results[0].get<object_id_type>() );
 }