From b61306dc6519ddd36955159a41c0b5415e039e47 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 10 Jul 2024 11:56:09 -0500 Subject: [PATCH 1/2] GH-333 Move savanna pending lib calculation to forkdb add() --- libraries/chain/block_state.cpp | 8 ++++ libraries/chain/controller.cpp | 38 ++----------------- libraries/chain/fork_database.cpp | 29 ++++++++++++-- .../chain/include/eosio/chain/block_state.hpp | 2 + 4 files changed, 40 insertions(+), 37 deletions(-) diff --git a/libraries/chain/block_state.cpp b/libraries/chain/block_state.cpp index 747fd2f67d..5c07e6dcea 100644 --- a/libraries/chain/block_state.cpp +++ b/libraries/chain/block_state.cpp @@ -298,6 +298,14 @@ void block_state::verify_qc(const valid_quorum_certificate& qc) const { invalid_qc_claim, "signature validation failed" ); } +qc_claim_t block_state::extract_qc_claim() const { + auto itr = header_exts.lower_bound(instant_finality_extension::extension_id()); + if (itr == header_exts.end()) + return {}; + const auto& if_ext = std::get(itr->second); + return if_ext.qc_claim; +} + valid_t block_state::new_valid(const block_header_state& next_bhs, const digest_type& action_mroot, const digest_type& strong_digest) const { assert(valid); assert(next_bhs.core.last_final_block_num() >= core.last_final_block_num()); diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 31a336f50b..d2e9d41e3d 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -3255,23 +3255,6 @@ struct controller_impl { emit( accepted_block, std::tie(chain_head.block(), chain_head.id()), __FILE__, __LINE__ ); if( s == controller::block_status::incomplete ) { - fork_db.apply_s([&](auto& forkdb) { - assert(std::holds_alternative>(cb.bsp.internal())); - const auto& bsp = std::get>(cb.bsp.internal()); - - uint16_t if_ext_id = instant_finality_extension::extension_id(); - assert(bsp->header_exts.count(if_ext_id) > 0); // in all instant_finality block headers - const auto& if_ext = std::get(bsp->header_exts.lower_bound(if_ext_id)->second); - if (if_ext.qc_claim.is_strong_qc) { - // claim has already been verified - auto claimed = forkdb.search_on_branch(bsp->id(), if_ext.qc_claim.block_num); - if (claimed) { - auto& final_on_strong_qc_block_ref = claimed->core.get_block_reference(claimed->core.final_on_strong_qc_block_num); - set_savanna_lib_id(final_on_strong_qc_block_ref.block_id); - } - } - }); - log_irreversible(); } @@ -3937,13 +3920,10 @@ struct controller_impl { // thread safe void integrate_received_qc_to_block(const block_state_ptr& bsp_in) { // extract QC from block extension - const auto& block_exts = bsp_in->block->validate_and_extract_extensions(); - auto qc_ext_id = quorum_certificate_extension::extension_id(); - - if( block_exts.count(qc_ext_id) == 0 ) { + if (!bsp_in->block->contains_extension(quorum_certificate_extension::extension_id())) return; - } - const auto& qc_ext = std::get(block_exts.lower_bound(qc_ext_id)->second); + + auto qc_ext = bsp_in->block->extract_extension(); const auto& received_qc = qc_ext.qc.data; const auto claimed = fetch_bsp_on_branch_by_num( bsp_in->previous(), qc_ext.qc.block_num ); @@ -3966,15 +3946,7 @@ struct controller_impl { dlog("setting valid qc: ${rqc} into claimed block ${bn} ${id}", ("rqc", qc_ext.qc.to_qc_claim())("bn", claimed->block_num())("id", claimed->id())); claimed->set_valid_qc(received_qc); - // advance LIB if QC is strong if( received_qc.is_strong() ) { - // We evaluate a block extension qc and advance lib if strong. - // This is done before evaluating the block. It is possible the block - // will not be valid or forked out. This is safe because the block is - // just acting as a carrier of this info. It doesn't matter if the block - // is actually valid as it simply is used as a network message for this data. - const auto& final_on_strong_qc_block_ref = claimed->core.get_block_reference(claimed->core.final_on_strong_qc_block_num); - set_savanna_lib_id(final_on_strong_qc_block_ref.block_id); // Update finalizer safety information based on vote evidence my_finalizers.maybe_update_fsi(claimed, received_qc); } @@ -4517,9 +4489,7 @@ struct controller_impl { void set_savanna_lib_id(const block_id_type& id) { fork_db.apply_s([&](auto& forkdb) { - if (forkdb.set_pending_savanna_lib_id(id)) { - dlog("set irreversible block ${bn}: ${id}", ("bn", block_header::num_from_id(id))("id", id)); - } + forkdb.set_pending_savanna_lib_id(id); }); } diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index 77980185fb..6acf272f20 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -106,6 +106,7 @@ namespace eosio::chain { void advance_root_impl( const block_id_type& id ); void remove_impl( const block_id_type& id ); bsp_t head_impl(include_root_t include_root) const; + bool set_pending_savanna_lib_id_impl(const block_id_type& id); branch_t fetch_branch_impl( const block_id_type& h, uint32_t trim_after_block_num ) const; block_branch_t fetch_block_branch_impl( const block_id_type& h, uint32_t trim_after_block_num ) const; branch_t fetch_branch_impl( const block_id_type& h, const block_id_type& b ) const; @@ -241,6 +242,22 @@ 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" ); + if constexpr (std::is_same_v) { + auto qc_claim = n->extract_qc_claim(); + if (qc_claim.is_strong_qc) { + // claim has already been verified, update LIB even if unable to verify block + // We evaluate a block extension qc and advance lib if strong. + // This is done before evaluating the block. It is possible the block + // will not be valid or forked out. This is safe because the block is + // just acting as a carrier of this info. It doesn't matter if the block + // is actually valid as it simply is used as a network message for this data. + if (auto claimed = search_on_branch_impl(n->id(), qc_claim.block_num, include_root_t::no)) { + auto& final_on_strong_qc_block_ref = claimed->core.get_block_reference(claimed->core.final_on_strong_qc_block_num); + set_pending_savanna_lib_id_impl(final_on_strong_qc_block_ref.block_id); + } + } + } + auto prev_bh = get_block_impl( n->previous(), include_root_t::yes ); EOS_ASSERT( prev_bh, unlinkable_block_exception, "forkdb unlinkable block ${id} previous ${p}", ("id", n->id())("p", n->previous()) ); @@ -321,11 +338,17 @@ namespace eosio::chain { template bool fork_database_t::set_pending_savanna_lib_id(const block_id_type& id) { - block_num_type new_lib = block_header::num_from_id(id); std::lock_guard g( my->mtx ); - block_num_type old_lib = block_header::num_from_id(my->pending_savanna_lib_id); + return my->set_pending_savanna_lib_id_impl(id); + } + + template + bool fork_database_impl::set_pending_savanna_lib_id_impl(const block_id_type& id) { + block_num_type new_lib = block_header::num_from_id(id); + block_num_type old_lib = block_header::num_from_id(pending_savanna_lib_id); if (new_lib > old_lib) { - my->pending_savanna_lib_id = id; + dlog("set pending savanna lib ${bn}: ${id}", ("bn", block_header::num_from_id(id))("id", id)); + pending_savanna_lib_id = id; return true; } return false; diff --git a/libraries/chain/include/eosio/chain/block_state.hpp b/libraries/chain/include/eosio/chain/block_state.hpp index 87a4d5983c..dc9252672d 100644 --- a/libraries/chain/include/eosio/chain/block_state.hpp +++ b/libraries/chain/include/eosio/chain/block_state.hpp @@ -125,6 +125,8 @@ struct block_state : public block_header_state { // block_header_state provi std::optional get_best_qc() const { return pending_qc.get_best_qc(block_num()); } // thread safe bool valid_qc_is_strong() const { return pending_qc.valid_qc_is_strong(); } // thread safe void set_valid_qc(const valid_quorum_certificate& qc) { pending_qc.set_valid_qc(qc); } + // extract the qc_claim from block header instant_finality_extension + qc_claim_t extract_qc_claim() const; // heuristic for determination if we are syncing or replaying for optimizations bool is_recent() const { From ac888e9899c683bbd397d85dedb79c8e5ebd363e Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 10 Jul 2024 12:35:59 -0500 Subject: [PATCH 2/2] GH-333 Use previous() as block is not in forkdb at this point --- libraries/chain/fork_database.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index 6acf272f20..139844fd58 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -251,7 +251,7 @@ namespace eosio::chain { // will not be valid or forked out. This is safe because the block is // just acting as a carrier of this info. It doesn't matter if the block // is actually valid as it simply is used as a network message for this data. - if (auto claimed = search_on_branch_impl(n->id(), qc_claim.block_num, include_root_t::no)) { + if (auto claimed = search_on_branch_impl(n->previous(), qc_claim.block_num, include_root_t::no)) { auto& final_on_strong_qc_block_ref = claimed->core.get_block_reference(claimed->core.final_on_strong_qc_block_num); set_pending_savanna_lib_id_impl(final_on_strong_qc_block_ref.block_id); }