diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 73e77080a2..83fac5b6f4 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -3473,7 +3473,12 @@ struct controller_impl { } bool is_head_descendant_of_pending_lib() const { - return fork_db_.is_descendant_of_pending_savanna_lib(chain_head.id()); + // True if pending_savanna_lib is on the chain from chain_head back to fork_db root: pending_savanna_lib is an + // ancestor of chain_head, or is chain_head itself. Answered from the head's finality_core, which tracks the + // canonical block_ref at each height across [last_final, head). No fork_db walk or mutex required. + const auto [pending_id, pending_timestamp] = fork_db_.pending_savanna_lib(); + if (chain_head.id() == pending_id) return true; + return chain_head.extends(pending_id); } void set_savanna_lib(const block_id_type& id, block_timestamp_type timestamp) { diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index 853b4b2b1b..88f42a238f 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -89,7 +89,6 @@ namespace sysio::chain { void remove_impl( block_num_type block_num ); bsp_t head_impl(include_root_t include_root) const; bool set_pending_savanna_lib_impl(const block_id_type& id, block_timestamp_type timestamp); - bool is_descendant_of_pending_savanna_lib_impl(const block_id_type& id) const; bool is_descendant_of_impl(const block_id_type& ancestor, const block_id_type& descendant) const; 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; @@ -368,19 +367,6 @@ namespace sysio::chain { return false; } - template - bool fork_database_type::is_descendant_of_pending_savanna_lib( const block_id_type& id ) const { - std::lock_guard g( my->mtx ); - return my->is_descendant_of_pending_savanna_lib_impl(id); - } - - template - bool fork_database_impl::is_descendant_of_pending_savanna_lib_impl(const block_id_type& id) const { - if (pending_savanna_lib_id == id) - return true; - return is_descendant_of_impl(pending_savanna_lib_id, id); - } - template bool fork_database_type::is_descendant_of(const block_id_type& ancestor, const block_id_type& descendant) const { std::lock_guard g( my->mtx ); diff --git a/libraries/chain/include/sysio/chain/block_handle.hpp b/libraries/chain/include/sysio/chain/block_handle.hpp index 99542cefb7..6ca6fc118b 100644 --- a/libraries/chain/include/sysio/chain/block_handle.hpp +++ b/libraries/chain/include/sysio/chain/block_handle.hpp @@ -38,6 +38,12 @@ struct block_handle { void write(const std::filesystem::path& state_file); bool read(const std::filesystem::path& state_file); + // Returns true if `id` is a strict ancestor of this block within the finality_core's tracking range + // (block_num in [last_final_block_num, current_block_num)). A block does not extend itself. + bool extends(const block_id_type& id) const { + return _bsp && _bsp->core.extends(id); + } + // Returns true iff this block carries a strong QC whose target is not in `head_handle`'s ancestry. Under Savanna's // strong-vote locking, finalizers locked on the QC target cannot later vote on any branch that does not extend it, // so a head whose branch does not include the QC target can never be covered by a future QC; it is permanently @@ -77,11 +83,6 @@ struct block_handle { // Head's branch and the QC target are incompatible; locked out. return true; } - - // Returns true if `id` is in this block's ancestry (or is this block itself within the finality_core's tracking range). - bool extends(const block_id_type& id) const { - return _bsp && _bsp->core.extends(id); - } }; } // namespace sysio::chain diff --git a/libraries/chain/include/sysio/chain/fork_database.hpp b/libraries/chain/include/sysio/chain/fork_database.hpp index 60a2c87521..0658e3ff29 100644 --- a/libraries/chain/include/sysio/chain/fork_database.hpp +++ b/libraries/chain/include/sysio/chain/fork_database.hpp @@ -115,11 +115,6 @@ namespace sysio::chain { std::pair pending_savanna_lib() const; bool set_pending_savanna_lib( const block_id_type& id, block_timestamp_type timestamp ); - /** - * @return true if id is built on top of pending savanna lib or id == pending_savanna_lib - */ - bool is_descendant_of_pending_savanna_lib( const block_id_type& id ) const; - /** * @param ancestor the id of a possible ancestor block * @param descendant the id of a possible descendant block diff --git a/plugins/producer_plugin/src/producer_plugin.cpp b/plugins/producer_plugin/src/producer_plugin.cpp index a1f38970cc..ad172b0c83 100644 --- a/plugins/producer_plugin/src/producer_plugin.cpp +++ b/plugins/producer_plugin/src/producer_plugin.cpp @@ -943,13 +943,14 @@ class producer_plugin_impl : public std::enable_shared_from_thischain(); + const block_handle fhead = chain.fork_db_head(); + // While producing our own block, normally defer applying incoming blocks to avoid disrupting // production mid-block. Exception: if the fork-database best head carries a strong QC for a // block not in our applied head's ancestry, our head's branch can no longer form a QC that // wins fork-choice -- continuing to produce on it is pointless and the resulting blocks would // be orphaned at the next fork switch. In that case fall through and apply blocks now. if (in_producing_mode()) { - const block_handle fhead = chain.fork_db_head(); if (!fhead.locks_out_branch_of(chain.head())) { fc_ilog(_log, "producing, fork database head at: #{} id: {}", fhead.block_num(), fhead.id()); @@ -963,7 +964,7 @@ class producer_plugin_impl : public std::enable_shared_from_this false. finality_core::extends requires block_num < current_block_num, so a block does not extend itself. + BOOST_TEST(!h_bsp13a.extends(bsp13a->id())); + + // Strict ancestors on the same branch -> true. + BOOST_TEST(h_bsp13a.extends(bsp12a->id())); + BOOST_TEST(h_bsp13a.extends(bsp11a->id())); + BOOST_TEST(h_bsp13a.extends(root->id())); + + // Sibling branches -> false (block_num is in tracking range but block_id differs). + BOOST_TEST(!h_bsp13a.extends(bsp11b->id())); + BOOST_TEST(!h_bsp13a.extends(bsp12b->id())); + + // block_num below the tracking range (< last_final_block_num) -> false. Catches a regression where extends + // widens its lower bound: a synthetic id at block_num 9 sits below root's 10 and must never be accepted. + BOOST_TEST(!h_bsp13a.extends(make_block_id(9))); + + // block_num at or above current_block_num on a sibling -> false. Catches a regression where extends widens + // its upper bound: bsp13b is at block 13 (== current_block_num of bsp13a) and bsp14b is at block 14 (>), + // both on branch b; neither must be reported as extended. + BOOST_TEST(!h_bsp13a.extends(bsp13b->id())); + BOOST_TEST(!h_bsp13a.extends(bsp14b->id())); + + // Default-constructed (null _bsp) -> false. Without the null guard this would crash. + BOOST_TEST(!block_handle{}.extends(root->id())); + +} FC_LOG_AND_RETHROW(); + // Tests for block_handle::locks_out_branch_of() // ---------------------------------------------- // Build two branches of equal length sharing a common root, attach blocks with @@ -324,12 +357,164 @@ BOOST_AUTO_TEST_CASE(locks_out_branch_of_test) try { BOOST_TEST(!h13a_weak.locks_out_branch_of(h12b)); BOOST_TEST(!h13a_weak.locks_out_branch_of(h11b)); + // Head higher than `this`, on the same branch (head extends this) -> not locked out. The first extends check + // (_bsp->core.extends(head_id)) returns false because head_id is past this->current_block_num, but the qc_target + // check then catches that head extends qc_target. + block_state_ptr bsp14a_on_strong = test_block_state_accessor::make_unique_block_state(14, bsp13a_strong); + block_handle h14a_on_strong{bsp14a_on_strong}; + BOOST_TEST(!h13a_strong.locks_out_branch_of(h14a_on_strong)); + + // Head higher than `this`, on a different branch -> still locked out (head doesn't extend qc_target). + block_state_ptr bsp14b_on_strong = test_block_state_accessor::make_unique_block_state(14, bsp13b_strong); + block_handle h14b_on_strong{bsp14b_on_strong}; + BOOST_TEST(h13a_strong.locks_out_branch_of(h14b_on_strong)); + // `this` is the genesis block: create_core_for_genesis_block yields latest_qc_claim().is_strong_qc == false, // so the strong-QC early return fires and no head is ever reported as locked out. BOOST_TEST(!h_root.locks_out_branch_of(h12b)); } FC_LOG_AND_RETHROW(); +// Tests for is_head_descendant_of_pending_lib, exercised via its underlying +// helper: head.id() == pending_savanna_lib_id || head_bsp->core.extends(pending_savanna_lib_id). +// `set_pending_savanna_lib` is monotonic in block_num, so each case uses a fresh +// fixture whose pending_lib begins unset. + +namespace { + bool extends_or_equal(const block_state_ptr& head_bsp, const block_id_type& pending_lib_id) { + if (head_bsp->id() == pending_lib_id) return true; + return head_bsp->core.extends(pending_lib_id); + } + + void check(fork_database_t& fork_db, + const block_state_ptr& pending_lib, + const std::vector& heads, + const std::vector& expected_true) { + fork_db.set_pending_savanna_lib(pending_lib->id(), pending_lib->timestamp()); + const auto pid = pending_lib->id(); + std::set true_set; + for (const auto& b : expected_true) true_set.insert(b->id()); + for (const auto& h : heads) { + const bool expected = true_set.count(h->id()) > 0; + BOOST_TEST_CONTEXT("pending_lib=#" << pending_lib->block_num() << " " << pending_lib->id().str().substr(8, 8) + << ", head=#" << h->block_num() << " " << h->id().str().substr(8, 8)) { + BOOST_TEST(extends_or_equal(h, pid) == expected); + } + } + } +} // anonymous + +BOOST_FIXTURE_TEST_CASE(pending_lib_eq_root, generate_fork_db_state) try { + // pending_lib at the genesis root -- every block is a descendant (or root itself) + const std::vector heads = {root, bsp11a, bsp12a, bsp13a, bsp11b, bsp12b, bsp13b, bsp14b, + bsp11c, bsp12c, bsp13c, bsp12bb, bsp13bb, bsp13bbb, bsp12bbb}; + check(fork_db, root, heads, heads); +} FC_LOG_AND_RETHROW(); + +BOOST_FIXTURE_TEST_CASE(pending_lib_at_fork_point_11a, generate_fork_db_state) try { + // pending_lib at bsp11a -- only branch a's blocks (and bsp11a itself) extend it + const std::vector heads = {root, bsp11a, bsp12a, bsp13a, bsp11b, bsp12b, bsp13b, bsp14b, + bsp11c, bsp12c, bsp13c, bsp12bb, bsp13bb, bsp13bbb, bsp12bbb}; + check(fork_db, bsp11a, heads, {bsp11a, bsp12a, bsp13a}); +} FC_LOG_AND_RETHROW(); + +BOOST_FIXTURE_TEST_CASE(pending_lib_on_branch_a, generate_fork_db_state) try { + // pending_lib at bsp12a on branch a -- only bsp12a and bsp13a extend it + const std::vector heads = {root, bsp11a, bsp12a, bsp13a, bsp11b, bsp12b, bsp13b, bsp14b, + bsp11c, bsp12c, bsp13c, bsp12bb, bsp13bb, bsp13bbb, bsp12bbb}; + check(fork_db, bsp12a, heads, {bsp12a, bsp13a}); +} FC_LOG_AND_RETHROW(); + +BOOST_FIXTURE_TEST_CASE(pending_lib_on_subbranch_bb, generate_fork_db_state) try { + // pending_lib at bsp12bb on the bb sub-branch -- only bsp12bb, bsp13bb, bsp13bbb extend it + const std::vector heads = {root, bsp11a, bsp12a, bsp13a, bsp11b, bsp12b, bsp13b, bsp14b, + bsp11c, bsp12c, bsp13c, bsp12bb, bsp13bb, bsp13bbb, bsp12bbb}; + check(fork_db, bsp12bb, heads, {bsp12bb, bsp13bb, bsp13bbb}); +} FC_LOG_AND_RETHROW(); + +BOOST_FIXTURE_TEST_CASE(pending_lib_at_deepest_14b, generate_fork_db_state) try { + // pending_lib at bsp14b -- only bsp14b itself qualifies (nothing descends from it) + const std::vector heads = {root, bsp11a, bsp12a, bsp13a, bsp11b, bsp12b, bsp13b, bsp14b, + bsp11c, bsp12c, bsp13c, bsp12bb, bsp13bb, bsp13bbb, bsp12bbb}; + check(fork_db, bsp14b, heads, {bsp14b}); +} FC_LOG_AND_RETHROW(); + +// Direct controller-level test of is_head_descendant_of_pending_lib. The pending_lib_* fixture tests above verify +// the underlying logic via a local mirror across many (head, pending_lib) combinations on a synthetic fork_db; +// this one drives a real controller through repeated produce_blocks + set_savanna_lib steps to exercise each +// branch of the controller method and catch wiring bugs (wrong destructured field, swapped comparison, wrong +// fork_db accessor). set_savanna_lib is monotonic in block_num, so pending_lib only advances forward across +// stages; head is advanced via produce_blocks between stages to expose new (head, pending_lib) pairs. +BOOST_AUTO_TEST_CASE(controller_is_head_descendant_of_pending_lib_test) try { + nonce = 0; + // setup_policy::none keeps the test light and avoids contract setup; QCs do not form here so pending_lib does + // not advance on its own. + sysio::testing::tester c{sysio::testing::setup_policy::none}; + c.produce_blocks(5); + + auto& chain = *c.control; + auto id_at = [&](uint32_t bn) { + auto id = chain.chain_block_id_for_num(bn); + BOOST_REQUIRE(id.has_value()); + return *id; + }; + auto ts_at = [&](uint32_t bn) { + auto sb = chain.fetch_block_by_number(bn); + BOOST_REQUIRE(sb); + return sb->timestamp; + }; + + // Stage 1 -- ancestor branch, distant: pending_lib at an early block (block 2), head several blocks ahead. + const uint32_t early_bn = 2; + chain.set_savanna_lib(id_at(early_bn), ts_at(early_bn)); + BOOST_REQUIRE_GT(chain.head().block_num(), early_bn); + BOOST_TEST(chain.is_head_descendant_of_pending_lib()); + + // Stage 2 -- ancestor branch, closer: advance head, then set pending_lib to a more recent ancestor. + c.produce_blocks(2); + const uint32_t closer_bn = 4; + chain.set_savanna_lib(id_at(closer_bn), ts_at(closer_bn)); + BOOST_TEST(chain.is_head_descendant_of_pending_lib()); + + // Stage 3 -- equality branch: pending_lib set to current head; chain_head.id() == pending_id short-circuit. + const auto h_eq = chain.head(); + chain.set_savanna_lib(h_eq.id(), h_eq.timestamp()); + BOOST_TEST(chain.is_head_descendant_of_pending_lib()); + + // Stage 4 -- ancestor branch, after head advances past the equality-pinned pending_lib. pending_lib stays where + // it was (no further set_savanna_lib call); head's finality_core must still recognize the now-older lib. + c.produce_blocks(3); + BOOST_REQUIRE_GT(chain.head().block_num(), h_eq.block_num()); + BOOST_TEST(chain.is_head_descendant_of_pending_lib()); + + // Stage 5 -- ancestor branch, immediate parent: pending_lib at head - 1. + const uint32_t parent_bn = chain.head().block_num() - 1; + chain.set_savanna_lib(id_at(parent_bn), ts_at(parent_bn)); + BOOST_TEST(chain.is_head_descendant_of_pending_lib()); + + // Stage 6 -- equality branch at the latest head. + const auto h_eq2 = chain.head(); + chain.set_savanna_lib(h_eq2.id(), h_eq2.timestamp()); + BOOST_TEST(chain.is_head_descendant_of_pending_lib()); + + // Stage 7 -- negative branch, pending_lib past head: fabricated id at head.block_num()+1. set_savanna_lib only + // accepts forward movement, but does not require new_lib <= head.block_num. head can neither equal nor extend + // an id whose block_num is at or past head's current_block_num. + const auto h_neg = chain.head(); + const block_id_type fake_above = make_block_id(h_neg.block_num() + 1); + chain.set_savanna_lib(fake_above, h_neg.timestamp()); + BOOST_TEST(!chain.is_head_descendant_of_pending_lib()); + + // Stage 8 -- negative branch, pending_lib at a block_num head has now caught up to but with a synthetic id that + // does not match the real block at that height. extends() finds a ref at fake_above's block_num but the ids + // differ, so the answer is still false. Distinct from stage 7 in that head's tracking range now covers the + // pending_lib's block_num; this pins the "different id at same height" branch of finality_core::extends. + c.produce_blocks(5); + const uint32_t fake_bn = block_header::num_from_id(fake_above); + BOOST_REQUIRE_GT(chain.head().block_num(), fake_bn); + BOOST_TEST(!chain.is_head_descendant_of_pending_lib()); +} FC_LOG_AND_RETHROW(); + // Tests that locks_out_branch_of returns false when the QC target is in head's ancestry but is older than head's // last_final_block_num, and therefore outside head's finality_core tracking range. Branches share an intermediate // ancestor (not genesis), and head's branch's lib has advanced past that ancestor via a chain of strong QCs. diff --git a/unittests/forked_tests_savanna.cpp b/unittests/forked_tests_savanna.cpp index 774b21831f..34916d95a9 100644 --- a/unittests/forked_tests_savanna.cpp +++ b/unittests/forked_tests_savanna.cpp @@ -813,4 +813,69 @@ BOOST_FIXTURE_TEST_CASE( fork_switch_dedup_savanna, savanna_cluster::cluster_t ) } FC_LOG_AND_RETHROW() +// Validates the precondition + apply behavior underpinning producer_plugin's fork_db_ahead_on_same_chain +// code path (plugins/producer_plugin/src/producer_plugin.cpp). When chain.head() is behind fork_db_head() +// on the same chain -- e.g. blocks accepted into fork_db via on_incoming_block but apply was deferred -- +// the lambda's predicate (fork_db_head().extends(head.id())) returns true, and apply_blocks must catch +// head up to fork_db_head. The interrupt_trx_test Python integration test exercises the full producing +// flow end-to-end; this C++ test localizes the predicate + apply pair on a real controller without the +// producer_plugin in the loop, so a regression points at the controller boundary directly. +// +// Mechanism: partition node D off the cluster so the automatic block propagation does not deliver blocks +// to it; produce blocks on node A; manually feed those blocks into D's fork_db via accept_block (which +// does NOT apply -- mirroring on_incoming_block before start_block has a chance to drain the queue); +// then check the predicate and run apply_blocks to verify catch-up. +BOOST_FIXTURE_TEST_CASE(fork_db_ahead_of_head_on_same_chain, savanna_cluster::cluster_t) try { + auto& A = _nodes[0]; + auto& D = _nodes[3]; + + const auto pre_head = D.head(); + const uint32_t pre_bn = pre_head.block_num(); + + // Isolate D so the cluster's signal-driven block propagation doesn't auto-apply blocks A produces. + set_partition({&D}); + + // Produce 2 blocks on A. With D partitioned, nodes 0/1/2 keep a 3-of-4 quorum so QCs still form and + // A's chain advances; D stays at pre_head. The gap is intentionally kept to num_chains_to_final so + // fork_db_head's finality_core still tracks pre_head's block_num in its [last_final, current) range: + // a 3-block gap would push LIB past pre_head and take it out of range, which is the case that + // !is_head_descendant_of_pending_lib() (the sibling condition in the producer_plugin loop) is + // responsible for, not fork_db_ahead_on_same_chain. + A.produce_blocks(2); + const uint32_t src_fhead_bn = A.fork_db_head().block_num(); + BOOST_REQUIRE_EQUAL(src_fhead_bn, pre_bn + 2); + BOOST_REQUIRE_EQUAL(D.head().block_num(), pre_bn); + + // Feed the 3 new blocks into D's fork_db via accept_block -- bypassing node_t::push_block (which would + // call tester::push_block -> apply_blocks). Each block must link cleanly because D's pre-partition + // chain history is identical to A's. + for (uint32_t bn = pre_bn + 1; bn <= src_fhead_bn; ++bn) { + auto sb = A.control->fetch_block_by_number(bn); + BOOST_REQUIRE(sb); + auto accepted = D.control->accept_block(sb->calculate_id(), sb); + BOOST_REQUIRE(accepted.block.has_value()); + } + + // Precondition: head is behind fork_db_head on the same chain. + const auto tgt_head = D.head(); + const auto tgt_fhead = D.fork_db_head(); + BOOST_REQUIRE_EQUAL(tgt_head.block_num(), pre_bn); + BOOST_REQUIRE_EQUAL(tgt_fhead.block_num(), src_fhead_bn); + BOOST_REQUIRE_EQUAL(tgt_head.id(), pre_head.id()); + + // The exact predicate from producer_plugin's fork_db_ahead_on_same_chain lambda. + BOOST_TEST(tgt_fhead.extends(tgt_head.id())); + + // apply_blocks must catch head up to fork_db_head -- the action the new producer_plugin loop drives + // when the predicate fires. + auto result = D.control->apply_blocks({}, {}); + BOOST_REQUIRE(result.status == controller::apply_blocks_result_t::status_t::complete); + BOOST_REQUIRE_EQUAL(D.head().block_num(), src_fhead_bn); + BOOST_REQUIRE_EQUAL(D.head().id(), tgt_fhead.id()); + + // After catch-up the predicate no longer fires (block_handle::extends does not include self). + BOOST_TEST(!D.fork_db_head().extends(D.head().id())); + +} FC_LOG_AND_RETHROW() + BOOST_AUTO_TEST_SUITE_END()