Merge pull request #139 from peerplays-network/feature/GRPH-94

Improve block generation performance

libraries/chain/db_block.cpp

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

@@ -71,6 +71,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();
 }
 
@@ -297,22 +298,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,
@@ -325,8 +331,8 @@ set<public_key_type> signed_transaction::get_required_signatures(
    vector<authority> other;
    get_required_authorities( required_active, required_owner, other );
 
-
-   sign_state s(get_signature_keys( chain_id ),get_active,available_keys);
+   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;
 
    for( const auto& auth : other )

libraries/wallet/wallet.cpp

@@ -2194,6 +2194,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.clear_signatures();
       set<public_key_type> approving_key_set = _remote_db->get_required_signatures(tx, owned_keys);
 
       auto dyn_props = get_dynamic_global_properties();
@@ -2211,8 +2212,8 @@ class wallet_api_impl
       uint32_t expiration_time_offset = 0;
       for (;;)
       {
-         tx.set_expiration(dyn_props.time + fc::seconds(30 + expiration_time_offset));
-         tx.signatures.clear();
+         tx.set_expiration( dyn_props.time + fc::seconds(30 + expiration_time_offset) );
+         tx.clear_signatures();
 
          for (const public_key_type &key : approving_key_set)
             tx.sign(get_private_key(key), _chain_id);
@@ -2293,7 +2294,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

@@ -699,7 +699,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;
@@ -769,7 +768,6 @@ const limit_order_object*database_fixture::create_sell_order(account_id_type use
 
 const limit_order_object* database_fixture::create_sell_order( const account_object& user, const asset& amount, const asset& recv )
 {
-   //wdump((amount)(recv));
    limit_order_create_operation buy_order;
    buy_order.seller = user.id;
    buy_order.amount_to_sell = amount;
@@ -780,7 +778,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>() );
 }
 

tests/tests/authority_tests.cpp

@@ -84,8 +84,7 @@ BOOST_AUTO_TEST_CASE( any_two_of_three )
          trx.operations.push_back(op);
          sign(trx, nathan_key1);
          PUSH_TX( db, trx, database::skip_transaction_dupe_check );
-         trx.operations.clear();
-         trx.signatures.clear();
+         trx.clear();
       } FC_CAPTURE_AND_RETHROW ((nathan.active))
 
       transfer_operation op;
@@ -99,19 +98,19 @@ BOOST_AUTO_TEST_CASE( any_two_of_three )
       PUSH_TX( db, trx, database::skip_transaction_dupe_check );
       BOOST_CHECK_EQUAL(get_balance(nathan, core), static_cast<int64_t>(old_balance - 500));
 
-      trx.signatures.clear();
+      trx.clear_signatures();
       sign(trx, nathan_key2);
       sign(trx, nathan_key3);
       PUSH_TX( db, trx, database::skip_transaction_dupe_check );
       BOOST_CHECK_EQUAL(get_balance(nathan, core), static_cast<int64_t>(old_balance - 1000));
 
-      trx.signatures.clear();
+      trx.clear_signatures();
       sign(trx, nathan_key1);
       sign(trx, nathan_key3);
       PUSH_TX( db, trx, database::skip_transaction_dupe_check );
       BOOST_CHECK_EQUAL(get_balance(nathan, core), static_cast<int64_t>(old_balance - 1500));
 
-      trx.signatures.clear();
+      trx.clear_signatures();
       //sign(trx, fc::ecc::private_key::generate());
       sign(trx,nathan_key3);
       GRAPHENE_CHECK_THROW(PUSH_TX( db, trx, database::skip_transaction_dupe_check ), fc::exception);
@@ -156,7 +155,7 @@ BOOST_AUTO_TEST_CASE( recursive_accounts )
       BOOST_TEST_MESSAGE( "Attempting to transfer with parent1 signature, should fail" );
       sign(trx,parent1_key);
       GRAPHENE_CHECK_THROW(PUSH_TX( db, trx, database::skip_transaction_dupe_check ), fc::exception);
-      trx.signatures.clear();
+      trx.clear_signatures();
 
       BOOST_TEST_MESSAGE( "Attempting to transfer with parent2 signature, should fail" );
       sign(trx,parent2_key);
@@ -166,8 +165,7 @@ BOOST_AUTO_TEST_CASE( recursive_accounts )
       sign(trx,parent1_key);
       PUSH_TX( db, trx, database::skip_transaction_dupe_check );
       BOOST_CHECK_EQUAL(get_balance(child, core), static_cast<int64_t>(old_balance - 500));
-      trx.operations.clear();
-      trx.signatures.clear();
+      trx.clear();
 
       BOOST_TEST_MESSAGE( "Adding a key for the child that can override parents" );
       fc::ecc::private_key child_key = fc::ecc::private_key::generate();
@@ -181,8 +179,7 @@ BOOST_AUTO_TEST_CASE( recursive_accounts )
          sign(trx,parent2_key);
          PUSH_TX( db, trx, database::skip_transaction_dupe_check );
          BOOST_REQUIRE_EQUAL(child.active.num_auths(), 3u);
-         trx.operations.clear();
-         trx.signatures.clear();
+         trx.clear();
       }
 
       op.from = child.id;
@@ -195,7 +192,7 @@ BOOST_AUTO_TEST_CASE( recursive_accounts )
       BOOST_TEST_MESSAGE( "Attempting transfer just parent1, should fail" );
       sign(trx, parent1_key);
       GRAPHENE_CHECK_THROW(PUSH_TX( db, trx, database::skip_transaction_dupe_check ), fc::exception);
-      trx.signatures.clear();
+      trx.clear_signatures();
       BOOST_TEST_MESSAGE( "Attempting transfer just parent2, should fail" );
       sign(trx, parent2_key);
       GRAPHENE_CHECK_THROW(PUSH_TX( db, trx, database::skip_transaction_dupe_check ), fc::exception);
@@ -204,14 +201,13 @@ BOOST_AUTO_TEST_CASE( recursive_accounts )
       sign(trx,  parent1_key);
       PUSH_TX( db, trx, database::skip_transaction_dupe_check );
       BOOST_CHECK_EQUAL(get_balance(child, core), static_cast<int64_t>(old_balance - 1000));
-      trx.signatures.clear();
+      trx.clear_signatures();
 
       BOOST_TEST_MESSAGE( "Attempting transfer with just child key, should succeed" );
       sign(trx, child_key);
       PUSH_TX( db, trx, database::skip_transaction_dupe_check );
       BOOST_CHECK_EQUAL(get_balance(child, core), static_cast<int64_t>(old_balance - 1500));
-      trx.operations.clear();
-      trx.signatures.clear();
+      trx.clear();
 
       BOOST_TEST_MESSAGE( "Creating grandparent account, parent1 now requires authority of grandparent" );
       auto grandparent = create_account("grandparent");
@@ -227,16 +223,15 @@ BOOST_AUTO_TEST_CASE( recursive_accounts )
          op.owner = *op.active;
          trx.operations.push_back(op);
          PUSH_TX( db, trx, ~0 );
-         trx.operations.clear();
-         trx.signatures.clear();
+         trx.clear();
       }
 
       BOOST_TEST_MESSAGE( "Attempt to transfer using old parent keys, should fail" );
       trx.operations.push_back(op);
       sign(trx, parent1_key);
       sign(trx, parent2_key);
       GRAPHENE_CHECK_THROW(PUSH_TX( db, trx, database::skip_transaction_dupe_check ), fc::exception);
-      trx.signatures.clear();
+      trx.clear_signatures();
       sign( trx,  parent2_key  );
       sign( trx,  grandparent_key  );
 
@@ -253,8 +248,7 @@ BOOST_AUTO_TEST_CASE( recursive_accounts )
          op.owner = *op.active;
          trx.operations.push_back(op);
          PUSH_TX( db, trx, ~0 );
-         trx.operations.clear();
-         trx.signatures.clear();
+         trx.clear();
       }
 
       BOOST_TEST_MESSAGE( "Create recursion depth failure" );
@@ -265,12 +259,11 @@ BOOST_AUTO_TEST_CASE( recursive_accounts )
       //Fails due to recursion depth.
       GRAPHENE_CHECK_THROW(PUSH_TX( db, trx, database::skip_transaction_dupe_check ), fc::exception);
       BOOST_TEST_MESSAGE( "verify child key can override recursion checks" );
-      trx.signatures.clear();
+      trx.clear_signatures();
       sign(trx,  child_key);
       PUSH_TX( db, trx, database::skip_transaction_dupe_check );
       BOOST_CHECK_EQUAL(get_balance(child, core), static_cast<int64_t>(old_balance - 2500));
-      trx.operations.clear();
-      trx.signatures.clear();
+      trx.clear();
 
       BOOST_TEST_MESSAGE( "Verify a cycle fails" );
       {
@@ -280,8 +273,7 @@ BOOST_AUTO_TEST_CASE( recursive_accounts )
          op.owner = *op.active;
          trx.operations.push_back(op);
          PUSH_TX( db, trx, ~0 );
-         trx.operations.clear();
-         trx.signatures.clear();
+         trx.clear();
       }
 
       trx.operations.push_back(op);
@@ -372,7 +364,7 @@ BOOST_AUTO_TEST_CASE( proposed_single_account )
       //committee has no stake in the transaction.
       GRAPHENE_CHECK_THROW(PUSH_TX( db, trx ), fc::exception);
 
-      trx.signatures.clear();
+      trx.clear_signatures();
       pup.active_approvals_to_add.clear();
       pup.active_approvals_to_add.insert(nathan.id);
 
@@ -408,7 +400,7 @@ BOOST_AUTO_TEST_CASE( proposal_failure )
       pop.expiration_time = db.head_block_time() + fc::days(1);
       pop.fee_paying_account = bob_id;
       trx.operations.push_back( pop );
-      trx.signatures.clear();
+      trx.clear_signatures();
       sign( trx, bob_private_key );
       processed_transaction processed = PUSH_TX( db, trx );
       proposal_object prop = db.get<proposal_object>(processed.operation_results.front().get<object_id_type>());
@@ -456,7 +448,7 @@ BOOST_AUTO_TEST_CASE( committee_authority )
    sign(trx, committee_key);
    GRAPHENE_CHECK_THROW(PUSH_TX( db, trx ), graphene::chain::invalid_committee_approval );
 
-   auto _sign = [&] { trx.signatures.clear(); sign( trx, nathan_key ); };
+   auto _sign = [&] { trx.clear_signatures(); sign( trx, nathan_key ); };
 
    proposal_create_operation pop;
    pop.proposed_ops.push_back({trx.operations.front()});
@@ -490,8 +482,7 @@ BOOST_AUTO_TEST_CASE( committee_authority )
 
    BOOST_TEST_MESSAGE( "Checking that the proposal is not authorized to execute" );
    BOOST_REQUIRE(!db.get<proposal_object>(prop.id).is_authorized_to_execute(db));
-   trx.operations.clear();
-   trx.signatures.clear();
+   trx.clear();
    proposal_update_operation uop;
    uop.fee_paying_account = GRAPHENE_TEMP_ACCOUNT;
    uop.proposal = prop.id;
@@ -512,7 +503,7 @@ BOOST_AUTO_TEST_CASE( committee_authority )
    // fails 
    // BOOST_CHECK(db.get<proposal_object>(prop.id).is_authorized_to_execute(db));
 
-   trx.signatures.clear();
+   trx.clear_signatures();
    generate_blocks(*prop.review_period_time);
    uop.key_approvals_to_add.clear();
    uop.key_approvals_to_add.insert(committee_key.get_public_key()); // was 7
@@ -1069,16 +1060,17 @@ BOOST_FIXTURE_TEST_CASE( bogus_signature, database_fixture )
       PUSH_TX( db,  trx, skip  );
 
       trx.operations.push_back( xfer_op );
+      trx.signees.clear(); // signees should be invalidated
       BOOST_TEST_MESSAGE( "Invalidating Alices Signature" );
       // Alice's signature is now invalid
       GRAPHENE_REQUIRE_THROW( PUSH_TX( db,  trx, skip  ), fc::exception );
       // Re-sign, now OK (sig is replaced)
       BOOST_TEST_MESSAGE( "Resign with Alice's Signature" );
-      trx.signatures.clear();
+      trx.clear_signatures();
       sign( trx,  alice_key  );
       PUSH_TX( db,  trx, skip  );
 
-      trx.signatures.clear();
+      trx.clear_signatures();
       trx.operations.pop_back();
       sign( trx,  alice_key  );
       sign( trx,  charlie_key  );
@@ -1127,7 +1119,7 @@ BOOST_FIXTURE_TEST_CASE( voting_account, database_fixture )
       GRAPHENE_CHECK_THROW(PUSH_TX( db, trx ), fc::exception);
       op.new_options->num_committee = 3;
       trx.operations = {op};
-      trx.signatures.clear();
+      trx.clear_signatures();
       sign( trx, vikram_private_key );
       PUSH_TX( db, trx );
       trx.clear();