GH-1062 Modify signed_block to hold the packed version of the signed_…

…block so it does not need to be re-packed when needed for the block log or for P2P. Refactor signed_block construction so that it is harder to use incorrectly. It is necessary to always store the packed block in case it is needed. Specialize signed_block unpack so that it always fills in the packed_block of signed_block.

libraries/chain/block_log.cpp

@@ -661,10 +661,11 @@ namespace eosio { namespace chain {
 
          signed_block_ptr read_block_by_num(uint32_t block_num) final {
             try {
-               uint64_t pos = get_block_pos(block_num);
+               auto [ pos, size ] = get_block_position_and_size(block_num);
                if (pos != block_log::npos) {
                   block_file.seek(pos);
-                  return read_block(block_file, block_num);
+                  fc::datastream_mirror ds(block_file, size);
+                  return read_block(ds, block_num);
                }
                return retry_read_block_by_num(block_num);
             }
@@ -801,7 +802,7 @@ namespace eosio { namespace chain {
 
          void reset(const genesis_state& gs, const signed_block_ptr& first_block) override {
             this->reset(1, gs, default_initial_version);
-            this->append(first_block, first_block->calculate_id(), fc::raw::pack(*first_block));
+            this->append(first_block, first_block->calculate_id(), first_block->packed_signed_block());
          }
 
          void reset(const chain_id_type& chain_id, uint32_t first_block_num) override {
@@ -1094,9 +1095,13 @@ namespace eosio { namespace chain {
          }
 
          signed_block_ptr retry_read_block_by_num(uint32_t block_num) final {
-            auto ds = catalog.ro_stream_for_block(block_num);
-            if (ds)
-               return read_block(*ds, block_num);
+            uint64_t block_size = 0;
+
+            auto ds = catalog.ro_stream_and_size_for_block(block_num, block_size);
+            if (ds) {
+               fc::datastream_mirror dsm(*ds, block_size);
+               return read_block(dsm, block_num);
+            }
             return {};
          }
 
@@ -1277,9 +1282,8 @@ namespace eosio { namespace chain {
    }
 
    void block_log::append(const signed_block_ptr& b, const block_id_type& id) {
-      std::vector<char> packed_block = fc::raw::pack(*b);
       std::lock_guard g(my->mtx);
-      my->append(b, id, packed_block);
+      my->append(b, id, b->packed_signed_block());
    }
 
    void block_log::append(const signed_block_ptr& b, const block_id_type& id, const std::vector<char>& packed_block) {

libraries/chain/block_state.cpp

@@ -114,7 +114,7 @@ block_state::block_state(const block_header_state&                bhs,
    , cached_trxs(std::move(trx_metas))
    , action_mroot(action_mroot)
 {
-   mutable_signed_block_ptr new_block = std::make_shared<signed_block>(signed_block_header{bhs.header});
+   mutable_block_ptr new_block = signed_block::create_mutable_block(signed_block_header{bhs.header});
    new_block->transactions = std::move(trx_receipts);
 
    if( qc ) {
@@ -125,7 +125,7 @@ block_state::block_state(const block_header_state&                bhs,
 
    sign(*new_block, block_id, signer, valid_block_signing_authority);
 
-   block = std::move(new_block);
+   block = signed_block::create_signed_block(std::move(new_block));
 }
 
 // Used for transition from dpos to Savanna.

libraries/chain/block_state_legacy.cpp

@@ -64,15 +64,15 @@ namespace eosio::chain {
    {}
 
    block_state_legacy::block_state_legacy( pending_block_header_state_legacy&& cur,
-                                           mutable_signed_block_ptr&& b,
+                                           mutable_block_ptr&& b,
                                            deque<transaction_metadata_ptr>&& trx_metas,
                                            const std::optional<digests_t>& action_receipt_digests_savanna,
                                            const protocol_feature_set& pfs,
                                            const validator_t& validator,
                                            const signer_callback_type& signer
                            )
    :block_header_state_legacy( inject_additional_signatures( std::move(cur), *b, pfs, validator, signer ) )
-   ,block( std::move(b) )
+   ,block( signed_block::create_signed_block(std::move(b)) )
    ,_pub_keys_recovered( true ) // called by produce_block so signature recovery of trxs must have been done
    ,_cached_trxs( std::move(trx_metas) )
    ,action_mroot_savanna( action_receipt_digests_savanna ? std::optional<digest_type>(calculate_merkle(*action_receipt_digests_savanna)) : std::nullopt )

libraries/chain/controller.cpp

@@ -231,7 +231,7 @@ struct assembled_block {
       block_id_type                     id;
       pending_block_header_state_legacy pending_block_header_state;
       deque<transaction_metadata_ptr>   trx_metas;
-      mutable_signed_block_ptr          unsigned_block;
+      mutable_block_ptr                 unsigned_block;
 
       // if the unsigned_block pre-dates block-signing authorities this may be present.
       std::optional<producer_authority_schedule> new_producer_authority_cache;
@@ -728,7 +728,7 @@ struct building_block {
                }
 
                // in dpos, we create a signed_block here. In IF mode, we do it later (when we are ready to sign it)
-               auto block_ptr = std::make_shared<signed_block>(bb.pending_block_header_state.make_block_header(
+               auto block_ptr = signed_block::create_mutable_block(bb.pending_block_header_state.make_block_header(
                   transaction_mroot, action_mroot, bb.new_pending_producer_schedule, std::move(new_finalizer_policy),
                   vector<digest_type>(bb.new_protocol_feature_activations), pfs));
 
@@ -1567,20 +1567,8 @@ struct controller_impl {
                return irreversible_mode() || bsp->is_valid();
             };
 
-            using packed_block_future = std::future<std::vector<char>>;
-            std::vector<packed_block_future> v;
-            if (!irreversible_mode()) {
-               v.reserve( branch.size() );
-               for( auto bitr = branch.rbegin(); bitr != branch.rend() && should_process(*bitr); ++bitr ) {
-                  v.emplace_back( post_async_task( thread_pool.get_executor(), [b=(*bitr)->block]() { return fc::raw::pack(*b); } ) );
-               }
-            }
-            auto it = v.begin();
-
             for( auto bitr = branch.rbegin(); bitr != branch.rend() && should_process(*bitr); ++bitr ) {
-               packed_block_future f;
                if (irreversible_mode()) {
-                  f = post_async_task( thread_pool.get_executor(), [b=(*bitr)->block]() { return fc::raw::pack(*b); } );
                   result = apply_irreversible_block(fork_db, *bitr);
                   if (result != controller::apply_blocks_result::complete)
                      break;
@@ -1590,7 +1578,7 @@ struct controller_impl {
 
                // blog.append could fail due to failures like running out of space.
                // Do it before commit so that in case it throws, DB can be rolled back.
-               blog.append( (*bitr)->block, (*bitr)->id(), irreversible_mode() ? f.get() : it++->get() );
+               blog.append( (*bitr)->block, (*bitr)->id(), (*bitr)->block->packed_signed_block() );
 
                db.commit( (*bitr)->block_num() );
                root_id = (*bitr)->id();
@@ -1659,7 +1647,7 @@ struct controller_impl {
       auto head = std::make_shared<block_state_legacy>();
       static_cast<block_header_state_legacy&>(*head) = genheader;
       head->activated_protocol_features = std::make_shared<protocol_feature_activation_set>(); // no activated protocol features in genesis
-      head->block = std::make_shared<signed_block>(genheader.header);
+      head->block = signed_block::create_signed_block(signed_block::create_mutable_block(genheader.header));
       chain_head = block_handle{head};
 
       db.set_revision( chain_head.block_num() );
@@ -5531,7 +5519,7 @@ signed_block_ptr controller::fetch_block_by_number( uint32_t block_num )const  {
 
 std::vector<char> controller::fetch_serialized_block_by_number( uint32_t block_num)const  { try {
    if (signed_block_ptr b = my->fork_db_fetch_block_on_best_branch_by_num(block_num)) {
-      return fc::raw::pack(*b);
+      return b->packed_signed_block();
    }
 
    return my->blog.read_serialized_block_by_num(block_num);

libraries/chain/deep_mind.cpp

@@ -91,7 +91,6 @@ namespace eosio::chain {
    {
       assert(b);
       assert(active_proposer_policy);
-      auto packed_blk = fc::raw::pack(*b);
       auto finality_data = fc::raw::pack(fd);
       auto packed_proposer_policy = fc::raw::pack(*active_proposer_policy);
       auto packed_finalizer_policy = fc::raw::pack(active_finalizer_policy);
@@ -100,7 +99,7 @@ namespace eosio::chain {
          ("id", id)
          ("num", b->block_num())
          ("lib", lib)
-         ("blk", fc::to_hex(packed_blk))
+         ("blk", fc::to_hex(b->packed_signed_block()))
          ("fd", fc::to_hex(finality_data))
          ("pp", fc::to_hex(packed_proposer_policy))
          ("fp", fc::to_hex(packed_finalizer_policy))

libraries/chain/include/eosio/chain/block.hpp

@@ -85,21 +85,26 @@ namespace eosio { namespace chain {
 
    using block_extension = block_extension_types::block_extension_t;
 
+   using signed_block_ptr = std::shared_ptr<const signed_block>;
+   using mutable_block_ptr = std::unique_ptr<signed_block>;
+
    /**
     */
    struct signed_block : public signed_block_header{
    private:
       signed_block( const signed_block& ) = default;
+      explicit signed_block( const signed_block_header& h ):signed_block_header(h){}
    public:
       signed_block() = default;
-      explicit signed_block( const signed_block_header& h ):signed_block_header(h){}
       signed_block( signed_block&& ) = default;
       signed_block& operator=(const signed_block&) = delete;
       signed_block& operator=(signed_block&&) = default;
-      signed_block clone() const { return *this; }
+      mutable_block_ptr clone() const { return std::unique_ptr<signed_block>(new signed_block(*this)); }
+      static mutable_block_ptr create_mutable_block(const signed_block_header& h) { return std::unique_ptr<signed_block>(new signed_block(h)); }
+      static signed_block_ptr  create_signed_block(mutable_block_ptr&& b) { b->pack(); return b; }
 
       deque<transaction_receipt>   transactions; /// new or generated transactions
-      extensions_type               block_extensions;
+      extensions_type              block_extensions;
 
       flat_multimap<uint16_t, block_extension> validate_and_extract_extensions()const;
       std::optional<block_extension> extract_extension(uint16_t extension_id)const;
@@ -108,9 +113,17 @@ namespace eosio { namespace chain {
          return std::get<Ext>(*extract_extension(Ext::extension_id()));
       }
       bool contains_extension(uint16_t extension_id)const;
+
+      const bytes& packed_signed_block() const { assert(!packed_block.empty()); return packed_block; }
+
+   private:
+      friend struct block_state;
+      friend struct block_state_legacy;
+      template<typename Stream> friend void fc::raw::unpack(Stream& s, eosio::chain::signed_block& v);
+      void pack() { packed_block = fc::raw::pack( *this ); }
+
+      bytes packed_block; // packed this
    };
-   using signed_block_ptr = std::shared_ptr<const signed_block>;
-   using mutable_signed_block_ptr = std::shared_ptr<signed_block>;
 
    struct producer_confirmation {
       block_id_type   block_id;
@@ -129,3 +142,20 @@ FC_REFLECT_DERIVED(eosio::chain::transaction_receipt, (eosio::chain::transaction
 FC_REFLECT(eosio::chain::additional_block_signatures_extension, (signatures));
 FC_REFLECT(eosio::chain::quorum_certificate_extension, (qc));
 FC_REFLECT_DERIVED(eosio::chain::signed_block, (eosio::chain::signed_block_header), (transactions)(block_extensions) )
+
+namespace fc::raw {
+   template <typename Stream>
+   void unpack(Stream& s, eosio::chain::signed_block& v) {
+      try {
+         if constexpr (requires { s.extract_mirror(); }) {
+            fc::reflector<eosio::chain::signed_block>::visit( fc::raw::detail::unpack_object_visitor<Stream, eosio::chain::signed_block>( v, s ) );
+            v.packed_block = s.extract_mirror();
+         } else {
+            fc::datastream_mirror<Stream> ds(s, sizeof(eosio::chain::signed_block) + 4096);
+            fc::reflector<eosio::chain::signed_block>::visit( fc::raw::detail::unpack_object_visitor<fc::datastream_mirror<Stream>, eosio::chain::signed_block>( v, ds ) );
+            v.packed_block = ds.extract_mirror();
+         }
+      } FC_RETHROW_EXCEPTIONS(warn, "error unpacking signed_block")
+   }
+}
+

libraries/chain/include/eosio/chain/block_state_legacy.hpp

@@ -21,7 +21,7 @@ namespace eosio::chain {
                  );
 
       block_state_legacy( pending_block_header_state_legacy&& cur,
-                          mutable_signed_block_ptr&& b, // unsigned block
+                          mutable_block_ptr&& b, // unsigned block
                           deque<transaction_metadata_ptr>&& trx_metas,
                           const std::optional<digests_t>& action_receipt_digests_savanna,
                           const protocol_feature_set& pfs,

libraries/libfc/include/fc/io/raw_fwd.hpp

@@ -13,6 +13,10 @@
 #include <variant>
 #include <filesystem>
 
+namespace eosio::chain {
+   struct signed_block;
+}
+
 namespace fc {
    class time_point;
    class time_point_sec;
@@ -32,6 +36,8 @@ namespace fc {
     template<typename T>
     inline size_t pack_size(  const T& v );
 
+    template <typename Stream> void unpack(Stream& s, eosio::chain::signed_block& v);
+
     template<typename Stream, typename Storage> inline void pack( Stream& s, const fc::fixed_string<Storage>& u );
     template<typename Stream, typename Storage> inline void unpack( Stream& s, fc::fixed_string<Storage>& u );
 

plugins/net_plugin/net_plugin.cpp

@@ -15,6 +15,7 @@
 #include <fc/network/message_buffer.hpp>
 #include <fc/io/json.hpp>
 #include <fc/io/raw.hpp>
+#include <fc/io/datastream.hpp>
 #include <fc/variant_object.hpp>
 #include <fc/crypto/rand.hpp>
 #include <fc/exception/exception.hpp>
@@ -3190,8 +3191,10 @@ namespace eosio {
             return true;
       }
 
-      auto ds = pending_message_buffer.create_datastream();
-      fc::raw::unpack( ds, which );
+      auto mb_ds = pending_message_buffer.create_datastream();
+      fc::raw::unpack( mb_ds, which );
+
+      fc::datastream_mirror ds(mb_ds, message_length);
       shared_ptr<signed_block> ptr = std::make_shared<signed_block>();
       fc::raw::unpack( ds, *ptr );
 

plugins/test_control_plugin/test_control_plugin.cpp

@@ -126,7 +126,7 @@ void test_control_plugin_impl::swap_action_in_block(const chain::signed_block_pt
       return;
    }
 
-   auto copy_b = std::make_shared<chain::signed_block>(b->clone());
+   auto copy_b = b->clone();
    copy_b->previous = b->calculate_id();
    copy_b->block_extensions.clear(); // remove QC extension since header will claim same as previous block
    copy_b->timestamp = b->timestamp.next();
@@ -159,12 +159,13 @@ void test_control_plugin_impl::swap_action_in_block(const chain::signed_block_pt
    copy_b->transaction_mroot = chain::calculate_merkle( std::move(trx_digests) );
    // Re-sign the block
    copy_b->producer_signature = _swap_on_options.blk_priv_key.sign(copy_b->calculate_id());
+   auto copy_b_signed = signed_block::create_signed_block(std::move(copy_b));
 
    // will be processed on the next start_block if is_new_best_head
-   const auto&[add_result, bh] = _chain.accept_block(copy_b->calculate_id(), copy_b);
+   const auto&[add_result, bh] = _chain.accept_block(copy_b_signed->calculate_id(), copy_b_signed);
    ilog("Swapped action ${f} to ${t}, add_result ${a}, block ${bn}",
         ("f", _swap_on_options.from)("t", _swap_on_options.to)("a", add_result)("bn", bh ? bh->block_num() : 0));
-   app().find_plugin<net_plugin>()->broadcast_block(copy_b, copy_b->calculate_id());
+   app().find_plugin<net_plugin>()->broadcast_block(copy_b_signed, copy_b_signed->calculate_id());
    if (_swap_on_options.shutdown)
       app().quit();
    reset_swap_action();

unittests/block_log_extract.cpp

@@ -11,7 +11,7 @@ using namespace eosio::chain;
 struct block_log_extract_fixture {
    block_log_extract_fixture() {
       log.emplace(dir.path());
-      log->reset(genesis_state(), std::make_shared<signed_block>());
+      log->reset(genesis_state(), signed_block::create_signed_block(signed_block::create_mutable_block({})));
       BOOST_REQUIRE_EQUAL(log->first_block_num(), 1u);
       BOOST_REQUIRE_EQUAL(log->head()->block_num(), 1u);
       for(uint32_t i = 2; i < 13; ++i) {
@@ -21,9 +21,10 @@ struct block_log_extract_fixture {
    };
 
    void add(uint32_t index) {
-      auto p = std::make_shared<signed_block>();
+      auto p = signed_block::create_mutable_block({});
       p->previous._hash[0] = fc::endian_reverse_u32(index-1);
-      log->append(p, p->calculate_id());
+      auto sp = signed_block::create_signed_block(std::move(p));
+      log->append(sp, sp->calculate_id());
    }
 
    static void rename_blocks_files(std::filesystem::path dir) {

unittests/block_log_get_block_tests.cpp

@@ -14,14 +14,15 @@ struct block_log_get_block_fixture {
 
       log.emplace(block_dir);
 
-      log->reset(genesis_state(), std::make_shared<signed_block>());
+      log->reset(genesis_state(), signed_block::create_signed_block(signed_block::create_mutable_block({})));
       BOOST_REQUIRE_EQUAL(log->first_block_num(), 1u);
       BOOST_REQUIRE_EQUAL(log->head()->block_num(), 1u);
 
       for(uint32_t i = 2; i < last_block_num + 1; ++i) {
-         auto p = std::make_shared<signed_block>();
+         auto p = signed_block::create_mutable_block({});
          p->previous._hash[0] = fc::endian_reverse_u32(i-1);
-         log->append(p, p->calculate_id());
+         auto sp = signed_block::create_signed_block(std::move(p));
+         log->append(sp, sp->calculate_id());
       }
       BOOST_REQUIRE_EQUAL(log->head()->block_num(), last_block_num);
    };