Merge pull request #4951 from wanderingbort/feature/fix-double-sig-check

Remove the redundant signature recovery and hash

libraries/chain/block_header_state.cpp

@@ -177,6 +177,7 @@ namespace eosio { namespace chain {
     result.header.producer_signature = h.producer_signature;
     result.id                        = result.header.id();
 
+    // ASSUMPTION FROM controller_impl::apply_block = all untrusted blocks will have their signatures pre-validated here
     if( !trust ) {
        EOS_ASSERT( result.block_signing_key == result.signee(), wrong_signing_key, "block not signed by expected key",
                   ("result.block_signing_key", result.block_signing_key)("signee", result.signee() ) );
@@ -225,12 +226,10 @@ namespace eosio { namespace chain {
      return digest_type::hash( std::make_pair(header_bmroot, pending_schedule_hash) );
   }
 
-  void block_header_state::sign( const std::function<signature_type(const digest_type&)>& signer, bool trust ) {
+  void block_header_state::sign( const std::function<signature_type(const digest_type&)>& signer ) {
      auto d = sig_digest();
      header.producer_signature = signer( d );
-     if( !trust ) {
-        EOS_ASSERT( block_signing_key == fc::crypto::public_key( header.producer_signature, d ), wrong_signing_key, "block is signed with unexpected key" );
-     }
+     EOS_ASSERT( block_signing_key == fc::crypto::public_key( header.producer_signature, d ), wrong_signing_key, "block is signed with unexpected key" );
   }
 
   public_key_type block_header_state::signee()const {

libraries/chain/controller.cpp

@@ -231,7 +231,7 @@ struct controller_impl {
             std::cerr<< "\n";
             ilog( "${n} reversible blocks replayed", ("n",rev) );
             auto end = fc::time_point::now();
-            ilog( "replayed ${n} blocks in seconds, ${mspb} ms/block",
+            ilog( "replayed ${n} blocks in ${duration} seconds, ${mspb} ms/block",
                   ("n", head->block_num)("duration", (end-start).count()/1000000)
                   ("mspb", ((end-start).count()/1000.0)/head->block_num)        );
             std::cerr<< "\n";
@@ -828,10 +828,10 @@ struct controller_impl {
 
 
 
-   void sign_block( const std::function<signature_type( const digest_type& )>& signer_callback, bool trust  ) {
+   void sign_block( const std::function<signature_type( const digest_type& )>& signer_callback  ) {
       auto p = pending->_pending_block_state;
 
-      p->sign( signer_callback, false); //trust );
+      p->sign( signer_callback );
 
       static_cast<signed_block_header&>(*p->block) = p->header;
    } /// sign_block
@@ -877,11 +877,21 @@ struct controller_impl {
          }
 
          finalize_block();
-         sign_block( [&]( const auto& ){ return b->producer_signature; }, false ); //trust );
 
-         // this is implied by the signature passing
-         //FC_ASSERT( b->id() == pending->_pending_block_state->block->id(),
-         //           "applying block didn't produce expected block id" );
+         // this implicitly asserts that all header fields (less the signature) are identical
+         EOS_ASSERT(b->id() == pending->_pending_block_state->header.id(),
+                   block_validate_exception, "Block ID does not match",
+                   ("producer_block_id",b->id())("validator_block_id",pending->_pending_block_state->header.id()));
+
+         // We need to fill out the pending block state's block because that gets serialized in the reversible block log
+         // in the future we can optimize this by serializing the original and not the copy
+
+         // we can always trust this signature because,
+         //   - prior to apply_block, we call fork_db.add which does a signature check IFF the block is untrusted
+         //   - OTHERWISE the block is trusted and therefore we trust that the signature is valid
+         // Also, as ::sign_block does not lazily calculate the digest of the block, we can just short-circuit to save cycles
+         pending->_pending_block_state->header.producer_signature = b->producer_signature;
+         static_cast<signed_block_header&>(*pending->_pending_block_state->block) =  pending->_pending_block_state->header;
 
          commit_block(false);
          return;
@@ -1274,7 +1284,7 @@ void controller::finalize_block() {
 }
 
 void controller::sign_block( const std::function<signature_type( const digest_type& )>& signer_callback ) {
-   my->sign_block( signer_callback, false /* don't trust */);
+   my->sign_block( signer_callback );
 }
 
 void controller::commit_block() {

libraries/chain/include/eosio/chain/block_header_state.hpp

@@ -49,7 +49,7 @@ struct block_header_state {
     producer_key         get_scheduled_producer( block_timestamp_type t )const;
     const block_id_type& prev()const { return header.previous; }
     digest_type          sig_digest()const;
-    void                 sign( const std::function<signature_type(const digest_type&)>& signer, bool trust = false );
+    void                 sign( const std::function<signature_type(const digest_type&)>& signer );
     public_key_type      signee()const;
 };