diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index eb60ec697d..baaba384e8 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -4205,7 +4205,7 @@ struct controller_impl { block_state_ptr claimed_bsp = fork_db_fetch_bsp_on_branch_by_num( bsp_in->previous(), qc_ext.qc.block_num ); if( !claimed_bsp ) { - dlog("qc not found in forkdb, qc: ${qc} for block ${bn} ${id}, previous ${p}", + dlog("block state of claimed qc not found in forkdb, qc: ${qc} for block ${bn} ${id}, previous ${p}", ("qc", qc_ext.qc.to_qc_claim())("bn", bsp_in->block_num())("id", bsp_in->id())("p", bsp_in->previous())); return; } diff --git a/unittests/savanna_cluster.cpp b/unittests/savanna_cluster.cpp index 0db47d75c9..f6a59afaea 100644 --- a/unittests/savanna_cluster.cpp +++ b/unittests/savanna_cluster.cpp @@ -6,6 +6,7 @@ node_t::node_t(size_t node_idx, cluster_t& cluster, setup_policy policy /* = set : tester(policy) , _node_idx(node_idx) , _last_vote({}, false) + , _cluster(cluster) { // since we are creating forks, finalizers may be locked on another fork and unable to vote. @@ -57,4 +58,10 @@ node_t::node_t(size_t node_idx, cluster_t& cluster, setup_policy policy /* = set node_t::~node_t() {} -} \ No newline at end of file +void node_t::propagate_delayed_votes_to(const node_t& to) { + for (auto& vote : _delayed_votes) + if (to.is_open()) + to.control->process_vote_message(++_cluster._connection_id, vote); +} + +} diff --git a/unittests/savanna_cluster.hpp b/unittests/savanna_cluster.hpp index 002a5b560b..8c867cbe4c 100644 --- a/unittests/savanna_cluster.hpp +++ b/unittests/savanna_cluster.hpp @@ -116,6 +116,8 @@ namespace savanna_cluster { std::function _accepted_block_cb; std::function _voted_block_cb; + cluster_t& _cluster; + public: node_t(size_t node_idx, cluster_t& cluster, setup_policy policy = setup_policy::none); @@ -127,6 +129,8 @@ namespace savanna_cluster { size_t& vote_delay() { return _vote_delay; } + void propagate_delayed_votes_to(const node_t& to); + const vote_t& last_vote() const { return _last_vote; } void set_node_finalizers(std::span names) { @@ -545,7 +549,8 @@ namespace savanna_cluster { // ------------------- void print(const char* name, const signed_block_ptr& b) const { if (_debug_mode) - std::cout << name << " (" << b->block_num() << ") timestamp = " << b->timestamp.slot << ", id = " << b->calculate_id().str().substr(8, 16) + std::cout << name << " (" << b->block_num() << ") timestamp = " << b->timestamp.slot + << ", id = " << b->calculate_id().str().substr(8, 16) << ", previous = " << b->previous.str().substr(8, 16) << '\n'; } @@ -563,14 +568,14 @@ namespace savanna_cluster { peers_t _peers; size_t _num_nodes; bool _shutting_down {false}; + uint32_t _connection_id = 0; friend node_t; void dispatch_vote_to_peers(size_t node_idx, skip_self_t skip_self, const vote_message_ptr& msg) { - static uint32_t connection_id = 0; for_each_peer(node_idx, skip_self, [&](node_t& n) { if (n.is_open()) - n.control->process_vote_message(++connection_id, msg); + n.control->process_vote_message(++_connection_id, msg); }); } diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index 4b65b76ff9..fff7956d80 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -592,7 +592,6 @@ BOOST_FIXTURE_TEST_CASE(validate_qc_requiring_finalizer_policies, savanna_cluste } FC_LOG_AND_RETHROW() - // ---------------------------------------------------------------------------------------------------- // For issue #694, we need to change the finality core of the `block_header_state`, but we want to // ensure that this doesn't create a consensus incompatibility with Spring 1.0.0, so the blocks created @@ -688,5 +687,108 @@ BOOST_FIXTURE_TEST_CASE(verify_spring_1_0_block_compatibitity, savanna_cluster:: } FC_LOG_AND_RETHROW() +/* ----------------------------------------------------------------------------------------------------- + Finality advancing past block claimed on alternate branch + ========================================================= + +Time: t1 t2 t3 t4 t5 t6 t7 +Blocks: + B0 <--- B1 <--- B2 <--- B3 <-|- B4 <--- B5 + | + \----------------- B6 <--- B7 +QC claim: + Strong Strong Strong Strong Strong No QC + B0 B1 B3 B4 B2 achieved + +Vote: Strong Strong Strong Weak - + + ^ + | + validating those weak votes on b2 + would fail if nodes have received b4 and b5 + which advanced lib to b3 + + - Node D is isolated and has not seen B3, B4, and B5 + - it received B3 via push_block, (so it can make it its head and produce a child of B3), but has not + received votes on b3 (only on b2), so b6 includes a strong QC on b2. + - when b6 is pushed to A, B and C, finalizers of A, B, and C are unable to vote on it, because they + are locked on B4, + -> liveness check fails because: `B6' s core.latest_qc_block_timestamp() < fsi.lock.timestamp` + because `B2 timestamp < B4 timestamp`. + -> safety check fails because `B6` does not extend `B4`. +--------------------------------------------------------------------------------------------------------*/ +BOOST_FIXTURE_TEST_CASE(finality_advancing_past_block_claimed_on_alternate_branch, savanna_cluster::cluster_t) try { + using namespace savanna_cluster; + auto& A=_nodes[0]; auto& B=_nodes[1]; auto& C=_nodes[2]; auto& D=_nodes[3]; + + // _debug_mode = true; + + auto b0 = A.produce_block(); + print("b0", b0); + + signed_block_ptr b1, b2; + { + // delay votes from B and C (can't delay on A as A produces), so b2 will not include a QC on B1 + fc::scoped_set_value tmp_B(B.vote_delay(), 1); + fc::scoped_set_value tmp_C(C.vote_delay(), 1); + + b1 = A.produce_block(); + print("b1", b1); + BOOST_REQUIRE_EQUAL(qc_s(qc(b1)), strong_qc(b0)); // b1 claims a strong QC on b0 + + b2 = A.produce_block(); + print("b2", b2); + BOOST_REQUIRE(!qc(b2)); // b2 should not include a QC (votes on b1 delayed) + } + + B.propagate_delayed_votes_to(D); // propagate votes on b2 to D, so it can form a QC on b2 + C.propagate_delayed_votes_to(D); // which will be included in b6 + + const std::vector partition {3}; // partition D so that it doesn't see b3, b4 and b5 + set_partition(partition); // and don't vote on it + + auto b3 = A.produce_block(); + print("b3", b3); + BOOST_REQUIRE_EQUAL(qc_s(qc(b3)), strong_qc(b1)); // b3 claims a strong QC on b1 (votes on b2 delayed) + + D.push_block(b3); // we want D to see b3, but not receive the votes on + // b3, so that when it produces b6, b6 will have a + // qc claim on b2 + + auto b4 = A.produce_block(); + print("b4", b4); + BOOST_REQUIRE_EQUAL(qc_s(qc(b4)), strong_qc(b3)); // b4 claims a strong QC on b3 + + auto b5 = A.produce_block(); + print("b5", b5); + BOOST_REQUIRE_EQUAL(qc_s(qc(b5)), strong_qc(b4)); // b5 claims a strong QC on b4 + + + set_partition({}); // remove partition so all nodes can vote on b6 and above + + auto b6 = D.produce_block(_block_interval_us * 3); // D (who has not seen b4 and b5) produces b6 + // b6 has a higher timestamp than b5 + print("b6", b6); + BOOST_REQUIRE_EQUAL(b6->previous, b3->calculate_id()); + BOOST_REQUIRE(!!qc(b6)); // b6 should include a QC + BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), strong_qc(b2)); // b6 claims a strong QC on b2 + + // --------------------------------------------------------------------------------------------------- + // After voting on `b5` (which makes `b3` final), the finalizers who voted on `b5` are locked on `b4`, + // and therefore cannot vote on `b6`: + // + // - liveness check fails because: `b6' s core.latest_qc_block_timestamp() < fsi.lock.timestamp` + // because `b2 timestamp < b4 timestamp`. + // - safety check fails because `b6` does not extend `b4`. + // + // As a result, we don't expect the next block (b7) to include a QC + // --------------------------------------------------------------------------------------------------- + + auto b7 = D.produce_block(); // D produces a block. It still has not seen b4 and b5. + print("b7", b7); + BOOST_REQUIRE(!qc(b7)); // b7 should not include a QC + +} FC_LOG_AND_RETHROW() + BOOST_AUTO_TEST_SUITE_END()