From 4a4ff7150f47810892fc723e9d97cec133da2f72 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 6 Mar 2024 09:44:13 -0500 Subject: [PATCH 1/3] Simplify fork_db `get_block` and root access. Simplify `create_block_state_i` --- libraries/chain/controller.cpp | 46 ++++++------------- libraries/chain/fork_database.cpp | 33 ++++--------- .../include/eosio/chain/fork_database.hpp | 3 +- 3 files changed, 22 insertions(+), 60 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index d3125997cf..dcfc549806 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -3310,43 +3310,23 @@ struct controller_impl { } // thread safe, expected to be called from thread other than the main thread - block_handle create_block_state_i( const block_id_type& id, const signed_block_ptr& b, const block_header_state& prev ) { - // Verify claim made by instant_finality_extension in block header extension and - // quorum_certificate_extension in block extension are valid. - // This is the only place the evaluation is done. - verify_qc_claim(id, b, prev); + template + block_handle create_block_state_i( const block_id_type& id, const signed_block_ptr& b, const BS& prev ) { + constexpr bool savanna_mode = std::is_same_v, block_state>; + if constexpr (savanna_mode) { + // Verify claim made by instant_finality_extension in block header extension and + // quorum_certificate_extension in block extension are valid. + // This is the only place the evaluation is done. + verify_qc_claim(id, b, prev); + } - auto trx_mroot = calculate_trx_merkle( b->transactions, true ); + auto trx_mroot = calculate_trx_merkle( b->transactions, savanna_mode ); EOS_ASSERT( b->transaction_mroot == trx_mroot, block_validate_exception, "invalid block transaction merkle root ${b} != ${c}", ("b", b->transaction_mroot)("c", trx_mroot) ); const bool skip_validate_signee = false; - auto bsp = std::make_shared( - prev, - b, - protocol_features.get_protocol_feature_set(), - [this]( block_timestamp_type timestamp, - const flat_set& cur_features, - const vector& new_features ) - { check_protocol_features( timestamp, cur_features, new_features ); }, - skip_validate_signee - ); - - EOS_ASSERT( id == bsp->id(), block_validate_exception, - "provided id ${id} does not match calculated block id ${bid}", ("id", id)("bid", bsp->id()) ); - - return block_handle{bsp}; - } - - // thread safe, expected to be called from thread other than the main thread - block_handle create_block_state_i( const block_id_type& id, const signed_block_ptr& b, const block_header_state_legacy& prev ) { - auto trx_mroot = calculate_trx_merkle( b->transactions, false ); - EOS_ASSERT( b->transaction_mroot == trx_mroot, block_validate_exception, - "invalid block transaction merkle root ${b} != ${c}", ("b", b->transaction_mroot)("c", trx_mroot) ); - - const bool skip_validate_signee = false; - auto bsp = std::make_shared( + auto bsp = std::make_shared( prev, b, protocol_features.get_protocol_feature_set(), @@ -3371,7 +3351,7 @@ struct controller_impl { auto existing = forkdb.get_block( id ); EOS_ASSERT( !existing, fork_database_exception, "we already know about this block: ${id}", ("id", id) ); - auto prev = forkdb.get_block_header( b->previous ); + auto prev = forkdb.get_block( b->previous, true ); EOS_ASSERT( prev, unlinkable_block_exception, "unlinkable block ${id} previous ${p}", ("id", id)("p", b->previous) ); @@ -3392,7 +3372,7 @@ struct controller_impl { EOS_ASSERT( !existing, fork_database_exception, "we already know about this block: ${id}", ("id", id) ); // previous not found could mean that previous block not applied yet - auto prev = forkdb.get_block_header( b->previous ); + auto prev = forkdb.get_block( b->previous, true ); if( !prev ) return {}; return create_block_state_i( id, b, *prev ); diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index 6d4de818b3..9f2677cfe1 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -128,8 +128,7 @@ namespace eosio::chain { void close_impl( const std::filesystem::path& fork_db_file ); void add_impl( const bsp_t& n, mark_valid_t mark_valid, ignore_duplicate_t ignore_duplicate, bool validate, validator_t& validator ); - bhsp_t get_block_header_impl( const block_id_type& id ) const; - bsp_t get_block_impl( const block_id_type& id ) const; + bsp_t get_block_impl( const block_id_type& id, bool check_root = false ) const; bool block_exists_impl( const block_id_type& id ) const; void reset_root_impl( const bhs_t& root_bhs ); void rollback_head_to_root_impl(); @@ -372,31 +371,12 @@ namespace eosio::chain { root = new_root; } - template - fork_database_t::bhsp_t fork_database_t::get_block_header( const block_id_type& id ) const { - std::lock_guard g( my->mtx ); - return my->get_block_header_impl( id ); - } - - template - fork_database_impl::bhsp_t fork_database_impl::get_block_header_impl( const block_id_type& id ) const { - if( root->id() == id ) { - return root; - } - - auto itr = index.find( id ); - if( itr != index.end() ) - return *itr; - - return bhsp_t(); - } - template void fork_database_impl::add_impl(const bsp_t& n, mark_valid_t mark_valid, ignore_duplicate_t ignore_duplicate, bool validate, validator_t& validator) { EOS_ASSERT( root, fork_database_exception, "root not yet set" ); EOS_ASSERT( n, fork_database_exception, "attempt to add null block state" ); - auto prev_bh = get_block_header_impl( n->previous() ); + auto prev_bh = get_block_impl( n->previous(), true ); EOS_ASSERT( prev_bh, unlinkable_block_exception, "forkdb unlinkable block ${id} previous ${p}", ("id", n->id())("p", n->previous()) ); @@ -684,13 +664,16 @@ namespace eosio::chain { } template - BSP fork_database_t::get_block(const block_id_type& id) const { + BSP fork_database_t::get_block(const block_id_type& id, bool check_root /* = false */) const { std::lock_guard g( my->mtx ); - return my->get_block_impl(id); + return my->get_block_impl(id, check_root); } template - BSP fork_database_impl::get_block_impl(const block_id_type& id) const { + BSP fork_database_impl::get_block_impl(const block_id_type& id, bool check_root /* = false */) const { + if( check_root && root->id() == id ) { + return root; + } auto itr = index.find( id ); if( itr != index.end() ) return *itr; diff --git a/libraries/chain/include/eosio/chain/fork_database.hpp b/libraries/chain/include/eosio/chain/fork_database.hpp index b7f18964aa..f40c2701e7 100644 --- a/libraries/chain/include/eosio/chain/fork_database.hpp +++ b/libraries/chain/include/eosio/chain/fork_database.hpp @@ -51,8 +51,7 @@ namespace eosio::chain { void open( const std::filesystem::path& fork_db_file, validator_t& validator ); void close( const std::filesystem::path& fork_db_file ); - bhsp_t get_block_header( const block_id_type& id ) const; - bsp_t get_block( const block_id_type& id ) const; + bsp_t get_block( const block_id_type& id, bool check_root = false ) const; bool block_exists( const block_id_type& id ) const; /** From a469ffdb98c0aad27c70320b2befea34f5af4fa5 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 6 Mar 2024 10:26:36 -0500 Subject: [PATCH 2/3] Finish updating fork_db to store `block_state` as root. --- libraries/chain/controller.cpp | 2 +- libraries/chain/fork_database.cpp | 23 ++++++++++--------- .../include/eosio/chain/fork_database.hpp | 2 +- unittests/finalizer_vote_tests.cpp | 2 +- unittests/fork_db_tests.cpp | 2 +- 5 files changed, 16 insertions(+), 15 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index dcfc549806..eaa21d159f 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1019,7 +1019,7 @@ struct controller_impl { fork_db.apply([&](auto& forkdb) { std::visit([&](const auto& bsp) { if constexpr (std::is_same_v, std::decay_t>) - forkdb.reset_root(*bsp); + forkdb.reset_root(bsp); }, chain_head.internal()); }); } diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index 9f2677cfe1..8e1f629c46 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -18,6 +18,8 @@ namespace eosio::chain { /** * History: * Version 1: initial version of the new refactored fork database portable format + * Version 2: Savanna version, store either `block_state`, `block_state_legacy` or both versions, + * root is full `block_state`, not just the header. */ struct block_state_accessor { @@ -118,7 +120,7 @@ namespace eosio::chain { std::mutex mtx; fork_multi_index_type index; - bsp_t root; // Only uses the block_header_state portion of block_state + bsp_t root; bsp_t head; const uint32_t magic_number; @@ -130,7 +132,7 @@ namespace eosio::chain { bsp_t get_block_impl( const block_id_type& id, bool check_root = false ) const; bool block_exists_impl( const block_id_type& id ) const; - void reset_root_impl( const bhs_t& root_bhs ); + void reset_root_impl( const bsp_t& root_bs ); void rollback_head_to_root_impl(); void advance_root_impl( const block_id_type& id ); void remove_impl( const block_id_type& id ); @@ -188,8 +190,8 @@ namespace eosio::chain { ("max", fork_database::max_supported_version) ); - bhs_t state; - fc::raw::unpack( ds, state ); + bsp_t state = std::make_shared(); + fc::raw::unpack( ds, *state ); reset_root_impl( state ); unsigned_int size; fc::raw::unpack( ds, size ); @@ -247,7 +249,7 @@ namespace eosio::chain { std::ofstream out( fork_db_file.generic_string().c_str(), std::ios::out | std::ios::binary | std::ofstream::trunc ); fc::raw::pack( out, magic_number ); fc::raw::pack( out, fork_database::max_supported_version ); // write out current version which is always max_supported_version - fc::raw::pack( out, *static_cast(&*root) ); + fc::raw::pack( out, *root ); uint32_t num_blocks_in_fork_db = index.size(); fc::raw::pack( out, unsigned_int{num_blocks_in_fork_db} ); @@ -297,16 +299,15 @@ namespace eosio::chain { } template - void fork_database_t::reset_root( const bhs_t& root_bhs ) { + void fork_database_t::reset_root( const bsp_t& root_bsp ) { std::lock_guard g( my->mtx ); - my->reset_root_impl(root_bhs); + my->reset_root_impl(root_bsp); } template - void fork_database_impl::reset_root_impl( const bhs_t& root_bhs ) { + void fork_database_impl::reset_root_impl( const bsp_t& root_bsp ) { index.clear(); - root = std::make_shared(); - static_cast(*root) = root_bhs; + root = root_bsp; bs_accessor_t::set_valid(*root, true); head = root; } @@ -758,7 +759,7 @@ namespace eosio::chain { fork_db_s = std::make_unique(fork_database_if_t::magic_number); legacy = false; apply_s([&](auto& forkdb) { - forkdb.reset_root(*new_head); + forkdb.reset_root(new_head); }); return block_handle{new_head}; } diff --git a/libraries/chain/include/eosio/chain/fork_database.hpp b/libraries/chain/include/eosio/chain/fork_database.hpp index f40c2701e7..de5f43a4f6 100644 --- a/libraries/chain/include/eosio/chain/fork_database.hpp +++ b/libraries/chain/include/eosio/chain/fork_database.hpp @@ -58,7 +58,7 @@ namespace eosio::chain { * Purges any existing blocks from the fork database and resets the root block_header_state to the provided value. * The head will also be reset to point to the root. */ - void reset_root( const bhs_t& root_bhs ); + void reset_root( const bsp_t& root_bhs ); /** * Removes validated flag from all blocks in fork database and resets head to point to the root. diff --git a/unittests/finalizer_vote_tests.cpp b/unittests/finalizer_vote_tests.cpp index 999f4241cf..06f705a3f6 100644 --- a/unittests/finalizer_vote_tests.cpp +++ b/unittests/finalizer_vote_tests.cpp @@ -123,7 +123,7 @@ struct simulator_t { auto genesis = make_bsp(proposal_t{0, "n0"}, bsp(), finpol); bsp_vec.push_back(genesis); - forkdb.reset_root(*genesis); + forkdb.reset_root(genesis); block_ref genesis_ref(genesis->id(), genesis->timestamp()); my_finalizer.fsi = fsi_t{block_timestamp_type(0), genesis_ref, genesis_ref}; diff --git a/unittests/fork_db_tests.cpp b/unittests/fork_db_tests.cpp index 655b3e9544..05d5bc8ebe 100644 --- a/unittests/fork_db_tests.cpp +++ b/unittests/fork_db_tests.cpp @@ -72,7 +72,7 @@ BOOST_AUTO_TEST_CASE(add_remove_test) try { // keep track of all those added for easy verification std::vector all { bsp11a, bsp12a, bsp13a, bsp11b, bsp12b, bsp12bb, bsp12bbb, bsp13b, bsp13bb, bsp13bbb, bsp14b, bsp11c, bsp12c, bsp13c }; - forkdb.reset_root(*root); + forkdb.reset_root(root); forkdb.add(bsp11a, mark_valid_t::no, ignore_duplicate_t::no); forkdb.add(bsp11b, mark_valid_t::no, ignore_duplicate_t::no); forkdb.add(bsp11c, mark_valid_t::no, ignore_duplicate_t::no); From 2697fd0d26e8ca040944aaf886718e23c1959dd7 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 6 Mar 2024 11:51:34 -0500 Subject: [PATCH 3/3] Use enum instead of `bool` parameter for `get_block`. --- libraries/chain/controller.cpp | 4 ++-- libraries/chain/fork_database.cpp | 12 +++++++----- .../chain/include/eosio/chain/fork_database.hpp | 3 ++- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index eaa21d159f..06bb496d68 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -3351,7 +3351,7 @@ struct controller_impl { auto existing = forkdb.get_block( id ); EOS_ASSERT( !existing, fork_database_exception, "we already know about this block: ${id}", ("id", id) ); - auto prev = forkdb.get_block( b->previous, true ); + auto prev = forkdb.get_block( b->previous, check_root_t::yes ); EOS_ASSERT( prev, unlinkable_block_exception, "unlinkable block ${id} previous ${p}", ("id", id)("p", b->previous) ); @@ -3372,7 +3372,7 @@ struct controller_impl { EOS_ASSERT( !existing, fork_database_exception, "we already know about this block: ${id}", ("id", id) ); // previous not found could mean that previous block not applied yet - auto prev = forkdb.get_block( b->previous, true ); + auto prev = forkdb.get_block( b->previous, check_root_t::yes ); if( !prev ) return {}; return create_block_state_i( id, b, *prev ); diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index 8e1f629c46..2f878bb2fd 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -130,7 +130,7 @@ namespace eosio::chain { void close_impl( const std::filesystem::path& fork_db_file ); void add_impl( const bsp_t& n, mark_valid_t mark_valid, ignore_duplicate_t ignore_duplicate, bool validate, validator_t& validator ); - bsp_t get_block_impl( const block_id_type& id, bool check_root = false ) const; + bsp_t get_block_impl( const block_id_type& id, check_root_t check_root = check_root_t::no ) const; bool block_exists_impl( const block_id_type& id ) const; void reset_root_impl( const bsp_t& root_bs ); void rollback_head_to_root_impl(); @@ -377,7 +377,7 @@ namespace eosio::chain { EOS_ASSERT( root, fork_database_exception, "root not yet set" ); EOS_ASSERT( n, fork_database_exception, "attempt to add null block state" ); - auto prev_bh = get_block_impl( n->previous(), true ); + auto prev_bh = get_block_impl( n->previous(), check_root_t::yes ); EOS_ASSERT( prev_bh, unlinkable_block_exception, "forkdb unlinkable block ${id} previous ${p}", ("id", n->id())("p", n->previous()) ); @@ -665,14 +665,16 @@ namespace eosio::chain { } template - BSP fork_database_t::get_block(const block_id_type& id, bool check_root /* = false */) const { + BSP fork_database_t::get_block(const block_id_type& id, + check_root_t check_root /* = check_root_t::no */) const { std::lock_guard g( my->mtx ); return my->get_block_impl(id, check_root); } template - BSP fork_database_impl::get_block_impl(const block_id_type& id, bool check_root /* = false */) const { - if( check_root && root->id() == id ) { + BSP fork_database_impl::get_block_impl(const block_id_type& id, + check_root_t check_root /* = check_root_t::no */) const { + if( check_root == check_root_t::yes && root->id() == id ) { return root; } auto itr = index.find( id ); diff --git a/libraries/chain/include/eosio/chain/fork_database.hpp b/libraries/chain/include/eosio/chain/fork_database.hpp index de5f43a4f6..faa2e402b9 100644 --- a/libraries/chain/include/eosio/chain/fork_database.hpp +++ b/libraries/chain/include/eosio/chain/fork_database.hpp @@ -11,6 +11,7 @@ namespace eosio::chain { using block_branch_t = std::vector; enum class mark_valid_t { no, yes }; enum class ignore_duplicate_t { no, yes }; + enum class check_root_t { no, yes }; // Used for logging of comparison values used for best fork determination std::string log_fork_comparison(const block_state& bs); @@ -51,7 +52,7 @@ namespace eosio::chain { void open( const std::filesystem::path& fork_db_file, validator_t& validator ); void close( const std::filesystem::path& fork_db_file ); - bsp_t get_block( const block_id_type& id, bool check_root = false ) const; + bsp_t get_block( const block_id_type& id, check_root_t check_root = check_root_t::no ) const; bool block_exists( const block_id_type& id ) const; /**