From 9a99cf9b94085f529c8d77373e174b2835a34233 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 11 Sep 2024 16:58:06 -0400 Subject: [PATCH 1/6] Start work on `finality_advancing_past_block_claimed_on_alternate_branch` testcase --- unittests/savanna_cluster.cpp | 9 +- unittests/savanna_cluster.hpp | 4 + unittests/savanna_misc_tests.cpp | 172 ++++++++++++++++++++++++++++++- 3 files changed, 183 insertions(+), 2 deletions(-) diff --git a/unittests/savanna_cluster.cpp b/unittests/savanna_cluster.cpp index 0db47d75c9..59753acec4 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() { + for (auto& vote : _delayed_votes) + _cluster.dispatch_vote_to_peers(_node_idx, skip_self_t::yes, vote); + _delayed_votes.clear(); +} + +} diff --git a/unittests/savanna_cluster.hpp b/unittests/savanna_cluster.hpp index 002a5b560b..f90a034fd6 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(); + const vote_t& last_vote() const { return _last_vote; } void set_node_finalizers(std::span names) { diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index 4b65b76ff9..35283d25da 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,176 @@ 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 t8 +Blocks: + B0 <--- B1 <--- B2 <--- B3 <-|- B4 <--- B5 + | + \----------------- B6 <--- B7 <--- B8 <--- B9 +QC claim: + Strong Strong Strong Strong Strong Weak Strong Strong + B0 B1 B3 B4 B2 B6 B7 B8 + +Vote: Strong Strong Strong Weak Strong Strong Strong + + ^ + | + 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, qc validation should fail. +--------------------------------------------------------------------------------------------------------*/ +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) + } + + // propagate + + 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 + + + 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 b6 + print("b6", b6); + BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), strong_qc(b2)); // b6 claims a strong QC on b2 + + +#if 0 + auto pending = A.head_pending_finalizer_policy(); + BOOST_REQUIRE(!!pending); // check that we have a pending finalizer policy + auto p2 = pending->generation; // and its generation is higher than the active one + BOOST_REQUIRE_EQUAL(p2, p1 + 1); // b3 has new pending finalizer policy p2 + + const std::vector partition {0}; // partition A so that B, C and D don't see b4 (yet) + set_partition(partition); // and don't vote on it + + 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 + BOOST_REQUIRE_EQUAL(A.lib_number, b2->block_num()); // b4 makes B2 final + pending = A.head_pending_finalizer_policy(); + BOOST_REQUIRE_EQUAL(pending->generation, p2); // b4 has new pending finalizer policy p2 + + auto b5 = A.produce_block(); + print("b5", b5); + BOOST_REQUIRE(!qc(b5)); // b5 doesn't include a new qc (duplicates b4's strong claim on b3) + BOOST_REQUIRE_EQUAL(A.lib_number, b2->block_num()); // finality unchanged stays at b2 + pending = A.head_pending_finalizer_policy(); + BOOST_REQUIRE_EQUAL(pending->generation, p2); // b5 still has new pending finalizer policy p2 + // since finality did not advance + + set_partition({}); // remove partition so A will receive votes on b4 and b5 + + push_block(0, b4); // other nodes receive b4 and vote on it, so A forms a qc on b4 + auto b6 = A.produce_block(); + print("b6", b6); + BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), strong_qc(b4)); // b6 claims a strong QC on b4 + BOOST_REQUIRE_EQUAL(A.lib_number, b3->block_num()); // b6 makes b3 final. + + auto active = A.head_active_finalizer_policy(); + BOOST_REQUIRE_EQUAL(active->generation, p2); // b6 has active finalizer policy p2 + BOOST_REQUIRE(!A.head_pending_finalizer_policy()); // and no pending finalizer policy. + + // At this point, in the Spring 1.0 implementation, policy P2 is lost from the block_header_state + // of B6, which is the source of the problem + + auto b6_snapshot = A.snapshot(); + + push_block(0, b5); + + auto b7 = A.produce_block(); + print("b7", b7); + BOOST_REQUIRE_EQUAL(qc_s(qc(b7)), strong_qc(b5)); // b7 claims a strong QC on b5 + BOOST_REQUIRE_EQUAL(A.lib_number, b3->block_num()); // lib is still b3 + + active = A.head_active_finalizer_policy(); + BOOST_REQUIRE_EQUAL(active->generation, p2); // b7 has active finalizer policy p2 + BOOST_REQUIRE(!A.head_pending_finalizer_policy()); // and no pending finalizer policy. + + push_block(0, b6); // push b6 again as it was unlinkable until the other + // nodes received b5 + + auto b8 = A.produce_block(); + print("b8", b8); + BOOST_REQUIRE_EQUAL(qc_s(qc(b8)), strong_qc(b6)); // b8 claims a strong QC on b6 + BOOST_REQUIRE_EQUAL(A.lib_number, b4->block_num()); // b8 makes B4 final + + active = A.head_active_finalizer_policy(); + BOOST_REQUIRE_EQUAL(active->generation, p2); // b8 has active finalizer policy p2 + BOOST_REQUIRE(!A.head_pending_finalizer_policy()); // and no pending finalizer policy. + + push_block(0, b7); // push b7 and b8 as they were unlinkable until the other + push_block(0, b8); // nodes received b6 + + auto b9 = A.produce_block(); + print("b9", b9); + BOOST_REQUIRE_EQUAL(qc_s(qc(b9)), strong_qc(b8)); // b9 claims a strong QC on b8 + BOOST_REQUIRE_EQUAL(A.lib_number, b6->block_num()); // b9 makes B6 final + + active = A.head_active_finalizer_policy(); + BOOST_REQUIRE_EQUAL(active->generation, p2); // b9 has active finalizer policy p2 + BOOST_REQUIRE(!A.head_pending_finalizer_policy()); // and no pending finalizer policy. + + // restart from b6 snapshot. + // ------------------------- + A.close(); + A.remove_state(); + A.remove_reversible_data_and_blocks_log(); + + set_partition({0}); // partition A so it doesn't receive blocks on `open()` + A.open_from_snapshot(b6_snapshot); + + A.push_block(b7); // when pushing b7, if we try to access any block state + A.push_block(b8); // before b6, we will fail with a `verify_qc_claim` + A.push_block(b9); // exception, which is what will happens until issue + // #694 is addressed. +#endif + +} FC_LOG_AND_RETHROW() + BOOST_AUTO_TEST_SUITE_END() From 4490d6dd8bd4b9209519b49713a4c70ce8df18eb Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Thu, 12 Sep 2024 17:28:18 -0400 Subject: [PATCH 2/6] progress on `finality_advancing_past_block_claimed_on_alternate_branch` testcase. --- libraries/chain/controller.cpp | 2 +- unittests/savanna_cluster.cpp | 6 +- unittests/savanna_cluster.hpp | 9 +-- unittests/savanna_misc_tests.cpp | 102 +++---------------------------- 4 files changed, 19 insertions(+), 100 deletions(-) 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 59753acec4..0a1b438672 100644 --- a/unittests/savanna_cluster.cpp +++ b/unittests/savanna_cluster.cpp @@ -58,10 +58,10 @@ node_t::node_t(size_t node_idx, cluster_t& cluster, setup_policy policy /* = set node_t::~node_t() {} -void node_t::propagate_delayed_votes() { +void node_t::propagate_delayed_votes_to(node_t &to) { for (auto& vote : _delayed_votes) - _cluster.dispatch_vote_to_peers(_node_idx, skip_self_t::yes, vote); - _delayed_votes.clear(); + 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 f90a034fd6..5cf7b3a89f 100644 --- a/unittests/savanna_cluster.hpp +++ b/unittests/savanna_cluster.hpp @@ -129,7 +129,7 @@ namespace savanna_cluster { size_t& vote_delay() { return _vote_delay; } - void propagate_delayed_votes(); + void propagate_delayed_votes_to(node_t &to); const vote_t& last_vote() const { return _last_vote; } @@ -549,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'; } @@ -567,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 35283d25da..7a0d5b2750 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -737,7 +737,8 @@ BOOST_FIXTURE_TEST_CASE(finality_advancing_past_block_claimed_on_alternate_branc BOOST_REQUIRE(!qc(b2)); // b2 should not include a QC (votes on b1 delayed) } - // propagate + 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 @@ -759,102 +760,19 @@ BOOST_FIXTURE_TEST_CASE(finality_advancing_past_block_claimed_on_alternate_branc 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 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 - -#if 0 - auto pending = A.head_pending_finalizer_policy(); - BOOST_REQUIRE(!!pending); // check that we have a pending finalizer policy - auto p2 = pending->generation; // and its generation is higher than the active one - BOOST_REQUIRE_EQUAL(p2, p1 + 1); // b3 has new pending finalizer policy p2 - - const std::vector partition {0}; // partition A so that B, C and D don't see b4 (yet) - set_partition(partition); // and don't vote on it - - 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 - BOOST_REQUIRE_EQUAL(A.lib_number, b2->block_num()); // b4 makes B2 final - pending = A.head_pending_finalizer_policy(); - BOOST_REQUIRE_EQUAL(pending->generation, p2); // b4 has new pending finalizer policy p2 - - auto b5 = A.produce_block(); - print("b5", b5); - BOOST_REQUIRE(!qc(b5)); // b5 doesn't include a new qc (duplicates b4's strong claim on b3) - BOOST_REQUIRE_EQUAL(A.lib_number, b2->block_num()); // finality unchanged stays at b2 - pending = A.head_pending_finalizer_policy(); - BOOST_REQUIRE_EQUAL(pending->generation, p2); // b5 still has new pending finalizer policy p2 - // since finality did not advance - - set_partition({}); // remove partition so A will receive votes on b4 and b5 - - push_block(0, b4); // other nodes receive b4 and vote on it, so A forms a qc on b4 - auto b6 = A.produce_block(); - print("b6", b6); - BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), strong_qc(b4)); // b6 claims a strong QC on b4 - BOOST_REQUIRE_EQUAL(A.lib_number, b3->block_num()); // b6 makes b3 final. - - auto active = A.head_active_finalizer_policy(); - BOOST_REQUIRE_EQUAL(active->generation, p2); // b6 has active finalizer policy p2 - BOOST_REQUIRE(!A.head_pending_finalizer_policy()); // and no pending finalizer policy. - - // At this point, in the Spring 1.0 implementation, policy P2 is lost from the block_header_state - // of B6, which is the source of the problem - - auto b6_snapshot = A.snapshot(); - - push_block(0, b5); - - auto b7 = A.produce_block(); + auto b7 = D.produce_block(); // D produces a block. It still has not seen b4 and b5. print("b7", b7); - BOOST_REQUIRE_EQUAL(qc_s(qc(b7)), strong_qc(b5)); // b7 claims a strong QC on b5 - BOOST_REQUIRE_EQUAL(A.lib_number, b3->block_num()); // lib is still b3 - - active = A.head_active_finalizer_policy(); - BOOST_REQUIRE_EQUAL(active->generation, p2); // b7 has active finalizer policy p2 - BOOST_REQUIRE(!A.head_pending_finalizer_policy()); // and no pending finalizer policy. - - push_block(0, b6); // push b6 again as it was unlinkable until the other - // nodes received b5 - - auto b8 = A.produce_block(); - print("b8", b8); - BOOST_REQUIRE_EQUAL(qc_s(qc(b8)), strong_qc(b6)); // b8 claims a strong QC on b6 - BOOST_REQUIRE_EQUAL(A.lib_number, b4->block_num()); // b8 makes B4 final - - active = A.head_active_finalizer_policy(); - BOOST_REQUIRE_EQUAL(active->generation, p2); // b8 has active finalizer policy p2 - BOOST_REQUIRE(!A.head_pending_finalizer_policy()); // and no pending finalizer policy. - - push_block(0, b7); // push b7 and b8 as they were unlinkable until the other - push_block(0, b8); // nodes received b6 - - auto b9 = A.produce_block(); - print("b9", b9); - BOOST_REQUIRE_EQUAL(qc_s(qc(b9)), strong_qc(b8)); // b9 claims a strong QC on b8 - BOOST_REQUIRE_EQUAL(A.lib_number, b6->block_num()); // b9 makes B6 final - - active = A.head_active_finalizer_policy(); - BOOST_REQUIRE_EQUAL(active->generation, p2); // b9 has active finalizer policy p2 - BOOST_REQUIRE(!A.head_pending_finalizer_policy()); // and no pending finalizer policy. - - // restart from b6 snapshot. - // ------------------------- - A.close(); - A.remove_state(); - A.remove_reversible_data_and_blocks_log(); - - set_partition({0}); // partition A so it doesn't receive blocks on `open()` - A.open_from_snapshot(b6_snapshot); - - A.push_block(b7); // when pushing b7, if we try to access any block state - A.push_block(b8); // before b6, we will fail with a `verify_qc_claim` - A.push_block(b9); // exception, which is what will happens until issue - // #694 is addressed. -#endif + BOOST_REQUIRE(!!qc(b7)); // b7 should include a QC + BOOST_REQUIRE_EQUAL(qc_s(qc(b7)), weak_qc(b6)); // b7 claims a weak QC on b6 } FC_LOG_AND_RETHROW() From 46783db1796b412a5fdc1863b3254b9e1c4504f9 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 13 Sep 2024 15:15:52 -0400 Subject: [PATCH 3/6] Complete testcase for issue #751, although it test something else. --- unittests/savanna_misc_tests.cpp | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index 7a0d5b2750..5b58db67a8 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -711,7 +711,9 @@ Vote: Strong Strong Strong Weak Strong Strong Stron - 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, qc validation should fail. + - when b6 is pushed to A, B and C, qc validation would fail in Spring 1.0.0 because the fork_db + lookup, but succeeds in Spring 1.0.1 as only the finality core of the claiming block is needed + to validate the QC. --------------------------------------------------------------------------------------------------------*/ BOOST_FIXTURE_TEST_CASE(finality_advancing_past_block_claimed_on_alternate_branch, savanna_cluster::cluster_t) try { using namespace savanna_cluster; @@ -769,10 +771,20 @@ BOOST_FIXTURE_TEST_CASE(finality_advancing_past_block_claimed_on_alternate_branc 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 include a QC - BOOST_REQUIRE_EQUAL(qc_s(qc(b7)), weak_qc(b6)); // b7 claims a weak QC on b6 + BOOST_REQUIRE(!qc(b7)); // b7 should include a QC } FC_LOG_AND_RETHROW() From fa553ed7b5da532bdfce560320426c474cbb6f7d Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 13 Sep 2024 16:04:57 -0400 Subject: [PATCH 4/6] Comment out `_debug_mode` --- unittests/savanna_misc_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index 5b58db67a8..4e41fa7325 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -719,7 +719,7 @@ BOOST_FIXTURE_TEST_CASE(finality_advancing_past_block_claimed_on_alternate_branc using namespace savanna_cluster; auto& A=_nodes[0]; auto& B=_nodes[1]; auto& C=_nodes[2]; auto& D=_nodes[3]; - _debug_mode = true; + // _debug_mode = true; auto b0 = A.produce_block(); print("b0", b0); From 41ceebcdd7364e21f84f613c872aa112d1845abe Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 18 Sep 2024 15:13:15 -0400 Subject: [PATCH 5/6] Make parameter const and update whitespace. --- unittests/savanna_cluster.cpp | 2 +- unittests/savanna_cluster.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/unittests/savanna_cluster.cpp b/unittests/savanna_cluster.cpp index 0a1b438672..f6a59afaea 100644 --- a/unittests/savanna_cluster.cpp +++ b/unittests/savanna_cluster.cpp @@ -58,7 +58,7 @@ node_t::node_t(size_t node_idx, cluster_t& cluster, setup_policy policy /* = set node_t::~node_t() {} -void node_t::propagate_delayed_votes_to(node_t &to) { +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 5cf7b3a89f..8c867cbe4c 100644 --- a/unittests/savanna_cluster.hpp +++ b/unittests/savanna_cluster.hpp @@ -129,7 +129,7 @@ namespace savanna_cluster { size_t& vote_delay() { return _vote_delay; } - void propagate_delayed_votes_to(node_t &to); + void propagate_delayed_votes_to(const node_t& to); const vote_t& last_vote() const { return _last_vote; } From 78c1053e75ccde144fc424429c38e4800d760aa9 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 18 Sep 2024 15:13:50 -0400 Subject: [PATCH 6/6] Update outdated PR comments according to review. --- unittests/savanna_misc_tests.cpp | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index 4e41fa7325..fff7956d80 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -691,16 +691,16 @@ BOOST_FIXTURE_TEST_CASE(verify_spring_1_0_block_compatibitity, savanna_cluster:: Finality advancing past block claimed on alternate branch ========================================================= -Time: t1 t2 t3 t4 t5 t6 t7 t8 +Time: t1 t2 t3 t4 t5 t6 t7 Blocks: B0 <--- B1 <--- B2 <--- B3 <-|- B4 <--- B5 | - \----------------- B6 <--- B7 <--- B8 <--- B9 + \----------------- B6 <--- B7 QC claim: - Strong Strong Strong Strong Strong Weak Strong Strong - B0 B1 B3 B4 B2 B6 B7 B8 + Strong Strong Strong Strong Strong No QC + B0 B1 B3 B4 B2 achieved -Vote: Strong Strong Strong Weak Strong Strong Strong +Vote: Strong Strong Strong Weak - ^ | @@ -711,9 +711,11 @@ Vote: Strong Strong Strong Weak Strong Strong Stron - 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, qc validation would fail in Spring 1.0.0 because the fork_db - lookup, but succeeds in Spring 1.0.1 as only the finality core of the claiming block is needed - to validate the QC. + - 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; @@ -784,7 +786,7 @@ BOOST_FIXTURE_TEST_CASE(finality_advancing_past_block_claimed_on_alternate_branc 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 include a QC + BOOST_REQUIRE(!qc(b7)); // b7 should not include a QC } FC_LOG_AND_RETHROW()