From 7b0f1e7a632508dbc332c6ab1fdba02a6dbd5d03 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 23 Aug 2024 17:01:45 -0400 Subject: [PATCH 01/19] Start on `gh_534_liveness_issue` testcase --- unittests/savanna_misc_tests.cpp | 48 ++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index 2d60fa2112..7b49c39824 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -53,9 +53,9 @@ BOOST_FIXTURE_TEST_CASE(snapshot_startup_with_forkdb, savanna_cluster::cluster_t +-----+ S +-----+ S +-----+ no +-----+ W +-----+ S +-----+ A produces <----| b0 |<-----| b1 |<-----------| b3 |<-------+ b4 |<-----| b5 |<----| b6 |<------- +-----+ +-----+ +-----+ claim +-----+ +-----+ +-----+ - ^ - | +-----+ -D produces +------| b2 | + ^ + | +-----+ +D produces +--------------------| b2 | S +-----+ */ @@ -149,4 +149,46 @@ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, savanna_cluster::cluster_t) try { } FC_LOG_AND_RETHROW() + +// ----------------------------------------------------------------------------------------------------- +// see https://github.com/AntelopeIO/spring/issues/621 explaining the issue that this test demonstrates. +// +// The fix in https://github.com/AntelopeIO/spring/issues/534 for the weak masking issue respected a more +// conservative version of rule 2. This solved the safety concerns due to the weak masking issue, but it +// was unnecessarily restrictive with respect to liveness. +// +// As a consequence if this liveness issue, finalizers may be stuck voting weak if the QC is not formed +// quickly enough. +// +// This testcase fails prior to https://github.com/AntelopeIO/spring/issues/621 being fixed. +// ----------------------------------------------------------------------------------------------------- +BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { + auto& A=_nodes[0]; auto& B=_nodes[1]; auto& C=_nodes[2]; auto& D=_nodes[3]; + using vote_t = savanna_cluster::node_t::vote_t; + _debug_mode = true; + + auto b0 = A.produce_blocks(2); // receives strong votes from all finalizers + auto b1 = A.produce_blocks(2); // receives strong votes from all finalizers + auto b2 = A.produce_blocks(2); // receives strong votes from all finalizers + print("b1", b1); + print("b2", b2); + + // partition D out. D will be used to produce blocks on an alternative fork. + // We will have 3 finalizers voting which is enough to reach QCs + // ------------------------------------------------------------------------- + const std::vector partition {3}; + set_partition(partition); + + auto b3 = D.produce_block(); // produce a block on D + print("b3", b3); + + push_block(1, b3); // push block to A, B and C, should receive strong votes + BOOST_REQUIRE_EQUAL(B.last_vote, vote_t(b3, true)); + BOOST_REQUIRE_EQUAL(C.last_vote, vote_t(b3, true)); + BOOST_REQUIRE_EQUAL(A.last_vote, vote_t(b3, true)); + BOOST_REQUIRE_EQUAL(qc_s(qc(b3)), qc_s(b1, true)); // b3 should include a strong qc on b1 + +} FC_LOG_AND_RETHROW() + + BOOST_AUTO_TEST_SUITE_END() From 3a9d896c08d48f86b16074656a2b91de670235cd Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 23 Aug 2024 18:06:51 -0400 Subject: [PATCH 02/19] Some progress on `gh_534_liveness_issue` testcase --- unittests/savanna_misc_tests.cpp | 47 +++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 7 deletions(-) diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index 7b49c39824..d6dee2af6f 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -167,11 +167,12 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { using vote_t = savanna_cluster::node_t::vote_t; _debug_mode = true; - auto b0 = A.produce_blocks(2); // receives strong votes from all finalizers - auto b1 = A.produce_blocks(2); // receives strong votes from all finalizers - auto b2 = A.produce_blocks(2); // receives strong votes from all finalizers + auto b0 = A.produce_block(); // receives strong votes from all finalizers + auto b1 = A.produce_block(); // receives strong votes from all finalizers + auto b2 = A.produce_block(); // receives strong votes from all finalizers print("b1", b1); print("b2", b2); + BOOST_REQUIRE_EQUAL(A.lib_number, b0->block_num()); // partition D out. D will be used to produce blocks on an alternative fork. // We will have 3 finalizers voting which is enough to reach QCs @@ -179,14 +180,46 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { const std::vector partition {3}; set_partition(partition); - auto b3 = D.produce_block(); // produce a block on D + auto b3 = D.produce_block(); // produce a block on D print("b3", b3); - push_block(1, b3); // push block to A, B and C, should receive strong votes + const std::vector tmp_partition {0}; // we temporarily separate A (before pushing b3) + set_partition(tmp_partition); // because we don't want A to see the block produced by D (b3) + // otherwise it will switch forks and build its next block (b4) + // on top of it + + push_block(1, b3); // push block to A, B and C, should receive strong votes + BOOST_REQUIRE_EQUAL(A.last_vote, vote_t(b2, true)); BOOST_REQUIRE_EQUAL(B.last_vote, vote_t(b3, true)); BOOST_REQUIRE_EQUAL(C.last_vote, vote_t(b3, true)); - BOOST_REQUIRE_EQUAL(A.last_vote, vote_t(b3, true)); - BOOST_REQUIRE_EQUAL(qc_s(qc(b3)), qc_s(b1, true)); // b3 should include a strong qc on b1 + BOOST_REQUIRE_EQUAL(D.last_vote, vote_t(b3, true)); + BOOST_REQUIRE_EQUAL(qc_s(qc(b3)), qc_s(b2, true)); // b3 should include a strong qc on b2 + BOOST_REQUIRE_EQUAL(B.lib_number, b1->block_num()); // don't use A.lib_number as A is partitioned by itself + // so it didn't see b3 and its enclosed QC. + + set_partition(partition); // restore our original partition {A, B, C} and {D} + + // from now on, to reproduce the scenario where votes are delayed, so the QC we receive don't + // claim the parent block, but an ancestor, we need to artificially delay propagating the votes. + // --------------------------------------------------------------------------------------------- + + // TODO - implement vote propagation delay and fix accordingly below checks. + + auto b4 = A.produce_block(_block_interval_us * 2); // receives weak votes from {B, C}. + print("b4", b4); + BOOST_REQUIRE_EQUAL(A.last_vote, vote_t(b4, true)); // A votes strong because it didn't see (and vote on) B3 + BOOST_REQUIRE_EQUAL(B.last_vote, vote_t(b4, false)); + BOOST_REQUIRE_EQUAL(C.last_vote, vote_t(b4, false)); + BOOST_REQUIRE_EQUAL(qc_s(qc(b4)), qc_s(b2, true)); // b4 should include a strong qc on b2 + BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); + + auto b5 = A.produce_block(_block_interval_us * 2); // receives weak votes from {B, C}. + print("b5", b5); + BOOST_REQUIRE_EQUAL(A.last_vote, vote_t(b5, true)); // A votes strong because it didn't see (and vote on) B3 + BOOST_REQUIRE_EQUAL(B.last_vote, vote_t(b5, false)); + BOOST_REQUIRE_EQUAL(C.last_vote, vote_t(b5, false)); + BOOST_REQUIRE_EQUAL(qc_s(qc(b5)), qc_s(b2, true)); // b5 should include a strong qc on b2 + BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); } FC_LOG_AND_RETHROW() From baad9f29da3c72e6472a61eb3178a8400f0aacba Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Mon, 26 Aug 2024 09:14:27 -0400 Subject: [PATCH 03/19] Minor cleanup --- libraries/testing/tester.cpp | 3 +- unittests/savanna_cluster.cpp | 20 ++-- unittests/savanna_cluster.hpp | 105 ++++++++++--------- unittests/savanna_finalizer_policy_tests.cpp | 12 +-- unittests/savanna_misc_tests.cpp | 56 +++++----- 5 files changed, 101 insertions(+), 95 deletions(-) diff --git a/libraries/testing/tester.cpp b/libraries/testing/tester.cpp index 03d6a7814c..ee73a8b57e 100644 --- a/libraries/testing/tester.cpp +++ b/libraries/testing/tester.cpp @@ -1304,7 +1304,8 @@ namespace eosio::testing { auto [privkey, pubkey, pop] = get_bls_key(name); local_finalizer_keys[pubkey.to_string()] = privkey.to_string(); } - control->set_node_finalizer_keys(local_finalizer_keys); + if (control) + control->set_node_finalizer_keys(local_finalizer_keys); } base_tester::set_finalizers_output_t base_tester::set_active_finalizers(std::span names) { diff --git a/unittests/savanna_cluster.cpp b/unittests/savanna_cluster.cpp index 279ba7f585..791b11c1a6 100644 --- a/unittests/savanna_cluster.cpp +++ b/unittests/savanna_cluster.cpp @@ -4,12 +4,12 @@ namespace savanna_cluster { node_t::node_t(size_t node_idx, cluster_t& cluster, setup_policy policy /* = setup_policy::none */) : tester(policy) - , node_idx(node_idx) { + , _node_idx(node_idx) { // since we are creating forks, finalizers may be locked on another fork and unable to vote. do_check_for_votes(false); - voted_block_cb = [&, node_idx](const eosio::chain::vote_signal_params& v) { + _voted_block_cb = [&, node_idx](const eosio::chain::vote_signal_params& v) { // no mutex needed because controller is set in tester (via `disable_async_voting(true)`) // to vote (and emit the `voted_block` signal) synchronously. // -------------------------------------------------------------------------------------- @@ -17,15 +17,15 @@ node_t::node_t(size_t node_idx, cluster_t& cluster, setup_policy policy /* = set if (status == vote_result_t::success) { vote_message_ptr vote_msg = std::get<2>(v); - last_vote = vote_t(vote_msg); - if (propagate_votes) - cluster.dispatch_vote_to_peers(node_idx, skip_self_t::yes, std::get<2>(v)); + _last_vote = vote_t(vote_msg); + if (_propagate_votes) + cluster.dispatch_vote_to_peers(node_idx, skip_self_t::yes, vote_msg); } }; // called on `commit_block`, for both blocks received from `push_block` and produced blocks - accepted_block_cb = [&, node_idx](const eosio::chain::block_signal_params& p) { - if (!pushing_a_block) { + _accepted_block_cb = [&, node_idx](const eosio::chain::block_signal_params& p) { + if (!_pushing_a_block) { // we want to propagate only blocks we produce, not the ones we receive from the network auto& b = std::get<0>(p); cluster.push_block_to_peers(node_idx, skip_self_t::yes, b); @@ -33,9 +33,9 @@ node_t::node_t(size_t node_idx, cluster_t& cluster, setup_policy policy /* = set }; auto node_initialization_fn = [&, node_idx]() { - [[maybe_unused]] auto _a = control->voted_block().connect(voted_block_cb); - [[maybe_unused]] auto _b = control->accepted_block().connect(accepted_block_cb); - tester::set_node_finalizers(node_finalizers); + [[maybe_unused]] auto _a = control->voted_block().connect(_voted_block_cb); + [[maybe_unused]] auto _b = control->accepted_block().connect(_accepted_block_cb); + tester::set_node_finalizers(_node_finalizers); cluster.get_new_blocks_from_peers(node_idx); }; diff --git a/unittests/savanna_cluster.hpp b/unittests/savanna_cluster.hpp index 7c5ac49676..0343865ef4 100644 --- a/unittests/savanna_cluster.hpp +++ b/unittests/savanna_cluster.hpp @@ -51,34 +51,47 @@ namespace savanna_cluster { vector privkeys; }; - // ---------------------------------------------------------------------------- - class node_t : public tester { - private: - size_t node_idx; - bool pushing_a_block{false}; + // two classes for comparisons in BOOST_REQUIRE_EQUAL + // -------------------------------------------------- + struct vote_t { + vote_t() : strong(false) {} + explicit vote_t(const vote_message_ptr& p) : id(p->block_id), strong(p->strong) {} + explicit vote_t(const signed_block_ptr& p, bool strong) : id(p->calculate_id()), strong(strong) {} - std::function accepted_block_cb; - std::function voted_block_cb; + friend std::ostream& operator<<(std::ostream& s, const vote_t& v) { + s << "vote_t(" << v.id.str().substr(8, 16) << ", " << (v.strong ? "strong" : "weak") << ")"; + return s; + } + bool operator==(const vote_t&) const = default; - public: - struct vote_t { - vote_t() : strong(false) {} - explicit vote_t(const vote_message_ptr& p) : id(p->block_id), strong(p->strong) {} - explicit vote_t(const signed_block_ptr& p, bool strong) : id(p->calculate_id()), strong(strong) {} - - friend std::ostream& operator<<(std::ostream& s, const vote_t& v) { - s << "vote_t(" << v.id.str().substr(8, 16) << ", " << (v.strong ? "strong" : "weak") << ")"; - return s; - } - bool operator==(const vote_t&) const = default; + block_id_type id; + bool strong; + }; + + struct qc_s { + explicit qc_s(const signed_block_ptr& p, bool strong) : block_num(p->block_num()), strong(strong) {} + explicit qc_s(const std::optional& qc) : block_num(qc->block_num), strong(qc->is_strong()) {} + + friend std::ostream& operator<<(std::ostream& s, const qc_s& v) { + s << "qc_s(" << v.block_num << ", " << (v.strong ? "strong" : "weak") << ")"; + return s; + } + bool operator==(const qc_s&) const = default; - block_id_type id; - bool strong; - }; + uint32_t block_num; // claimed block + bool strong; + }; - bool propagate_votes{true}; - vote_t last_vote; - std::vector node_finalizers; + // ---------------------------------------------------------------------------- + class node_t : public tester { + private: + size_t _node_idx; + bool _pushing_a_block{false}; + bool _propagate_votes{true}; + vote_t _last_vote; + std::vector _node_finalizers; + std::function _accepted_block_cb; + std::function _voted_block_cb; public: node_t(size_t node_idx, cluster_t& cluster, setup_policy policy = setup_policy::none); @@ -87,9 +100,13 @@ namespace savanna_cluster { node_t(node_t&&) = default; + bool& propagate_votes() { return _propagate_votes; } + + const vote_t& last_vote() const { return _last_vote; } + void set_node_finalizers(std::span names) { - node_finalizers = std::vector{ names.begin(), names.end() }; - tester::set_node_finalizers(node_finalizers); + _node_finalizers = std::vector{ names.begin(), names.end() }; + tester::set_node_finalizers(_node_finalizers); } void transition_to_savanna(std::span finalizer_policy_names) { @@ -167,8 +184,8 @@ namespace savanna_cluster { void push_block(const signed_block_ptr& b) { if (is_open() && !fetch_block_by_id(b->calculate_id())) { - assert(!pushing_a_block); - fc::scoped_set_value set_pushing_a_block(pushing_a_block, true); + assert(!_pushing_a_block); + fc::scoped_set_value set_pushing_a_block(_pushing_a_block, true); tester::push_block(b); } } @@ -189,19 +206,19 @@ namespace savanna_cluster { } std::string snapshot() const { - dlog("node ${i} - taking snapshot", ("i", node_idx)); + dlog("node ${i} - taking snapshot", ("i", _node_idx)); auto writer = buffered_snapshot_suite::get_writer(); control->write_snapshot(writer); return buffered_snapshot_suite::finalize(writer); } void open_from_snapshot(const std::string& snapshot) { - dlog("node ${i} - restoring from snapshot", ("i", node_idx)); + dlog("node ${i} - restoring from snapshot", ("i", _node_idx)); open(buffered_snapshot_suite::get_reader(snapshot)); } std::vector save_fsi() const { - dlog("node ${i} - saving fsi", ("i", node_idx)); + dlog("node ${i} - saving fsi", ("i", _node_idx)); auto finalizer_path = get_fsi_path(); std::ifstream file(finalizer_path.generic_string(), std::ios::binary | std::ios::ate); std::streamsize size = file.tellg(); @@ -214,7 +231,7 @@ namespace savanna_cluster { } void overwrite_fsi(const std::vector& fsi) const { - dlog("node ${i} - overwriting fsi", ("i", node_idx)); + dlog("node ${i} - overwriting fsi", ("i", _node_idx)); auto finalizer_path = get_fsi_path(); std::ofstream file(finalizer_path.generic_string(), std::ios::binary); assert(!fsi.empty()); @@ -222,13 +239,13 @@ namespace savanna_cluster { } void remove_fsi() { - dlog("node ${i} - removing fsi", ("i", node_idx)); + dlog("node ${i} - removing fsi", ("i", _node_idx)); remove_all(get_fsi_path()); } void remove_state() { auto state_path = cfg.state_dir; - dlog("node ${i} - removing state data from: ${state_path}", ("i", node_idx)("${state_path}", state_path)); + dlog("node ${i} - removing state data from: ${state_path}", ("i", _node_idx)("${state_path}", state_path)); remove_all(state_path); fs::create_directories(state_path); } @@ -246,7 +263,7 @@ namespace savanna_cluster { for (auto const& dir_entry : std::filesystem::directory_iterator{path}) { auto path = dir_entry.path(); if (path.filename().generic_string() != "reversible") { - dlog("node ${i} - removing : ${path}", ("i", node_idx)("${path}", path)); + dlog("node ${i} - removing : ${path}", ("i", _node_idx)("${path}", path)); remove_all(path); } } @@ -259,7 +276,7 @@ namespace savanna_cluster { void remove_blocks(bool rm_blocks_log) { auto reversible_path = cfg.blocks_dir / config::reversible_blocks_dir_name; auto& path = rm_blocks_log ? cfg.blocks_dir : reversible_path; - dlog("node ${i} - removing : ${path}", ("i", node_idx)("${path}", path)); + dlog("node ${i} - removing : ${path}", ("i", _node_idx)("${path}", path)); remove_all(path); fs::create_directories(reversible_path); } @@ -471,22 +488,6 @@ namespace savanna_cluster { size_t num_nodes() const { return _num_nodes; } - // Class for comparisons in BOOST_REQUIRE_EQUAL - // -------------------------------------------- - struct qc_s { - explicit qc_s(const signed_block_ptr& p, bool strong) : block_num(p->block_num()), strong(strong) {} - explicit qc_s(const std::optional& qc) : block_num(qc->block_num), strong(qc->is_strong()) {} - - friend std::ostream& operator<<(std::ostream& s, const qc_s& v) { - s << "qc_s(" << v.block_num << ", " << (v.strong ? "strong" : "weak") << ")"; - return s; - } - bool operator==(const qc_s&) const = default; - - uint32_t block_num; // claimed block - bool strong; - }; - static qc_claim_t qc_claim(const signed_block_ptr& b) { return b->extract_header_extension().qc_claim; } diff --git a/unittests/savanna_finalizer_policy_tests.cpp b/unittests/savanna_finalizer_policy_tests.cpp index 4af240c9c2..1cd034bfa5 100644 --- a/unittests/savanna_finalizer_policy_tests.cpp +++ b/unittests/savanna_finalizer_policy_tests.cpp @@ -38,7 +38,7 @@ BOOST_FIXTURE_TEST_CASE(policy_change, savanna_cluster::cluster_t) try { // -------------------------------------------------------------------------------------------------- B.close(); // update `B.node_finalizers` with the new key so that B can vote both on the active and pending policy - B.node_finalizers = std::vector{ _fin_keys[1], _fin_keys[num_nodes()] }; // see node_t::node_t + B.set_node_finalizers(std::vector{ _fin_keys[1], _fin_keys[num_nodes()] }); // see node_t::node_t B.open(); // quick sanity check @@ -144,8 +144,8 @@ BOOST_FIXTURE_TEST_CASE(policy_change_reduce_threshold_replace_all_keys, savanna // ----------------------------------------------------------------------------------------------- A.close(); B.close(); - A.node_finalizers = std::vector{ _fin_keys[0], _fin_keys[num_nodes()] }; - B.node_finalizers = std::vector{ _fin_keys[1], _fin_keys[num_nodes() + 1] }; + A.set_node_finalizers(std::vector{ _fin_keys[0], _fin_keys[num_nodes()] }); + B.set_node_finalizers(std::vector{ _fin_keys[1], _fin_keys[num_nodes() + 1] }); A.open(); B.open(); @@ -206,9 +206,9 @@ BOOST_FIXTURE_TEST_CASE(policy_change_restart_from_snapshot, savanna_cluster::cl A.close(); B.close(); C.close(); - A.node_finalizers = std::vector{ _fin_keys[0], _fin_keys[num_nodes()] }; - B.node_finalizers = std::vector{ _fin_keys[1], _fin_keys[num_nodes() + 1] }; - C.node_finalizers = std::vector{ _fin_keys[2], _fin_keys[num_nodes() + 2] }; + A.set_node_finalizers(std::vector{ _fin_keys[0], _fin_keys[num_nodes()] }); + B.set_node_finalizers(std::vector{ _fin_keys[1], _fin_keys[num_nodes() + 1] }); + C.set_node_finalizers(std::vector{ _fin_keys[2], _fin_keys[num_nodes() + 2] }); A.open(); B.open(); C.open(); diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index a1ca4e8871..1cb2bbec6f 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -61,7 +61,9 @@ D produces +--------------------| b2 | */ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, savanna_cluster::cluster_t) try { auto& A=_nodes[0]; auto& B=_nodes[1]; auto& C=_nodes[2]; auto& D=_nodes[3]; - using vote_t = savanna_cluster::node_t::vote_t; + using vote_t = savanna_cluster::vote_t; + using qc_s = savanna_cluster::qc_s; + //_debug_mode = true; auto b0 = A.produce_blocks(2); // receives strong votes from all finalizers @@ -87,9 +89,9 @@ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, savanna_cluster::cluster_t) try { // on top of it push_block(1, b2); // push block to B and C, should receive weak votes - BOOST_REQUIRE_EQUAL(B.last_vote, vote_t(b2, false)); - BOOST_REQUIRE_EQUAL(C.last_vote, vote_t(b2, false)); - BOOST_REQUIRE_EQUAL(A.last_vote, vote_t(b1, true));// A should not have seen b2, and therefore not voted on it + BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b2, false)); + BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b2, false)); + BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b1, true));// A should not have seen b2, and therefore not voted on it BOOST_REQUIRE_EQUAL(qc_s(qc(b2)), qc_s(b0, true)); // b2 should include a strong qc on b0 @@ -98,16 +100,16 @@ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, savanna_cluster::cluster_t) try { signed_block_ptr b3; { - fc::scoped_set_value tmp(B.propagate_votes, false); // temporarily prevent B from broadcasting its votes) + fc::scoped_set_value tmp(B.propagate_votes(), false);// temporarily prevent B from broadcasting its votes) // so A won't receive them and form a QC on b3 b3 = A.produce_block(_block_interval_us * 2); // A will see its own strong vote on b3, and C's weak vote // (not a quorum) // because B doesn't propagate and D is partitioned away print("b3", b3); - BOOST_REQUIRE_EQUAL(A.last_vote, vote_t(b3, true)); // A didn't vote on b2 so it can vote strong - BOOST_REQUIRE_EQUAL(B.last_vote, vote_t(b3, false)); // but B and C have to vote weak. - BOOST_REQUIRE_EQUAL(C.last_vote, vote_t(b3, false)); // C did vote, but we turned vote propagation off so + BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b3, true)); // A didn't vote on b2 so it can vote strong + BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b3, false)); // but B and C have to vote weak. + BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b3, false)); // C did vote, but we turned vote propagation off so // A will never see C's vote BOOST_REQUIRE_EQUAL(qc_s(qc(b3)), qc_s(b1, true)); // b3 should include a strong qc on b1 } @@ -119,9 +121,9 @@ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, savanna_cluster::cluster_t) try { // and should include a strong QC claim on b1 (repeated) // since we don't have enough votes to form a QC on b3 print("b4", b4); - BOOST_REQUIRE_EQUAL(A.last_vote, vote_t(b4, true)); - BOOST_REQUIRE_EQUAL(B.last_vote, vote_t(b4, false)); - BOOST_REQUIRE_EQUAL(C.last_vote, vote_t(b4, false)); + BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b4, true)); + BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b4, false)); + BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b4, false)); BOOST_REQUIRE_EQUAL(qc_claim(b3), qc_claim(b4)); // A didn't form a QC on b3, so b4 should repeat b3's claim BOOST_REQUIRE(!qc(b4)); // b4 should not have a QC extension (no new QC formed on b3) @@ -132,8 +134,8 @@ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, savanna_cluster::cluster_t) try { // weak QC on b4, which itself had a strong QC on b1. // Upon receiving a strong QC on b5, b4 will be final print("b5", b5); - BOOST_REQUIRE_EQUAL(A.last_vote, vote_t(b5, true)); - BOOST_REQUIRE_EQUAL(B.last_vote, vote_t(b5, true)); + BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b5, true)); + BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b5, true)); BOOST_REQUIRE_EQUAL(qc_s(qc(b5)), qc_s(b4, false)); // b5 should include a weak qc on b4 BOOST_REQUIRE_EQUAL(A.lib_number, b0->block_num()); @@ -142,8 +144,8 @@ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, savanna_cluster::cluster_t) try { print("b6", b6); BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), qc_s(b5, true)); // b6 should include a strong qc on b5 - BOOST_REQUIRE_EQUAL(A.last_vote, vote_t(b6, true)); - BOOST_REQUIRE_EQUAL(B.last_vote, vote_t(b6, true)); + BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b6, true)); + BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b6, true)); BOOST_REQUIRE_EQUAL(A.lib_number, b4->block_num()); @@ -164,7 +166,9 @@ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, savanna_cluster::cluster_t) try { // ----------------------------------------------------------------------------------------------------- BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { auto& A=_nodes[0]; auto& B=_nodes[1]; auto& C=_nodes[2]; auto& D=_nodes[3]; - using vote_t = savanna_cluster::node_t::vote_t; + using vote_t = savanna_cluster::vote_t; + using qc_s = savanna_cluster::qc_s; + _debug_mode = true; auto b0 = A.produce_block(); // receives strong votes from all finalizers @@ -189,10 +193,10 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { // on top of it push_block(1, b3); // push block to A, B and C, should receive strong votes - BOOST_REQUIRE_EQUAL(A.last_vote, vote_t(b2, true)); - BOOST_REQUIRE_EQUAL(B.last_vote, vote_t(b3, true)); - BOOST_REQUIRE_EQUAL(C.last_vote, vote_t(b3, true)); - BOOST_REQUIRE_EQUAL(D.last_vote, vote_t(b3, true)); + BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b2, true)); + BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b3, true)); + BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b3, true)); + BOOST_REQUIRE_EQUAL(D.last_vote(), vote_t(b3, true)); BOOST_REQUIRE_EQUAL(qc_s(qc(b3)), qc_s(b2, true)); // b3 should include a strong qc on b2 BOOST_REQUIRE_EQUAL(B.lib_number, b1->block_num()); // don't use A.lib_number as A is partitioned by itself // so it didn't see b3 and its enclosed QC. @@ -207,17 +211,17 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { auto b4 = A.produce_block(_block_interval_us * 2); // receives weak votes from {B, C}. print("b4", b4); - BOOST_REQUIRE_EQUAL(A.last_vote, vote_t(b4, true)); // A votes strong because it didn't see (and vote on) B3 - BOOST_REQUIRE_EQUAL(B.last_vote, vote_t(b4, false)); - BOOST_REQUIRE_EQUAL(C.last_vote, vote_t(b4, false)); + BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b4, true)); // A votes strong because it didn't see (and vote on) B3 + BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b4, false)); + BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b4, false)); BOOST_REQUIRE_EQUAL(qc_s(qc(b4)), qc_s(b2, true)); // b4 should include a strong qc on b2 BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); auto b5 = A.produce_block(_block_interval_us * 2); // receives weak votes from {B, C}. print("b5", b5); - BOOST_REQUIRE_EQUAL(A.last_vote, vote_t(b5, true)); // A votes strong because it didn't see (and vote on) B3 - BOOST_REQUIRE_EQUAL(B.last_vote, vote_t(b5, false)); - BOOST_REQUIRE_EQUAL(C.last_vote, vote_t(b5, false)); + BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b5, true)); // A votes strong because it didn't see (and vote on) B3 + BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b5, false)); + BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b5, false)); BOOST_REQUIRE_EQUAL(qc_s(qc(b5)), qc_s(b2, true)); // b5 should include a strong qc on b2 BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); From 040d4b25aefe6172feff89107caed23c2d80b56b Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Mon, 26 Aug 2024 10:03:44 -0400 Subject: [PATCH 04/19] Implement vote delay for `savanna_cluster` --- unittests/savanna_cluster.cpp | 14 ++++++++++++-- unittests/savanna_cluster.hpp | 8 +++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/unittests/savanna_cluster.cpp b/unittests/savanna_cluster.cpp index 791b11c1a6..8596d0367f 100644 --- a/unittests/savanna_cluster.cpp +++ b/unittests/savanna_cluster.cpp @@ -18,8 +18,18 @@ node_t::node_t(size_t node_idx, cluster_t& cluster, setup_policy policy /* = set if (status == vote_result_t::success) { vote_message_ptr vote_msg = std::get<2>(v); _last_vote = vote_t(vote_msg); - if (_propagate_votes) - cluster.dispatch_vote_to_peers(node_idx, skip_self_t::yes, vote_msg); + + if (_propagate_votes) { + if (_vote_delay) + _delayed_votes.push_back(std::move(vote_msg)); + while (_delayed_votes.size() > _vote_delay) { + vote_message_ptr vote = _delayed_votes.front(); + _delayed_votes.erase(_delayed_votes.cbegin()); + cluster.dispatch_vote_to_peers(node_idx, skip_self_t::yes, vote); + } + if (!_vote_delay) + cluster.dispatch_vote_to_peers(node_idx, skip_self_t::yes, vote_msg); + } } }; diff --git a/unittests/savanna_cluster.hpp b/unittests/savanna_cluster.hpp index 0343865ef4..c8dd02bbde 100644 --- a/unittests/savanna_cluster.hpp +++ b/unittests/savanna_cluster.hpp @@ -87,9 +87,13 @@ namespace savanna_cluster { private: size_t _node_idx; bool _pushing_a_block{false}; - bool _propagate_votes{true}; + bool _propagate_votes{true}; // if false, votes are dropped vote_t _last_vote; std::vector _node_finalizers; + + size_t _vote_delay{0}; // delay vote propagation by this much + std::vector _delayed_votes; + std::function _accepted_block_cb; std::function _voted_block_cb; @@ -102,6 +106,8 @@ namespace savanna_cluster { bool& propagate_votes() { return _propagate_votes; } + size_t& vote_delay() { return _vote_delay; } + const vote_t& last_vote() const { return _last_vote; } void set_node_finalizers(std::span names) { From 0391637b2acd18691acaac519db0674f8949c854 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Mon, 26 Aug 2024 10:04:19 -0400 Subject: [PATCH 05/19] Finish `gh_534_liveness_issue` testcase --- unittests/savanna_misc_tests.cpp | 39 ++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index 1cb2bbec6f..aba06a6237 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -207,24 +207,49 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { // claim the parent block, but an ancestor, we need to artificially delay propagating the votes. // --------------------------------------------------------------------------------------------- - // TODO - implement vote propagation delay and fix accordingly below checks. + fc::scoped_set_value tmp(B.vote_delay(), 1); // delaying just B's votes should be enough to delay QCs auto b4 = A.produce_block(_block_interval_us * 2); // receives weak votes from {B, C}. print("b4", b4); - BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b4, true)); // A votes strong because it didn't see (and vote on) B3 - BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b4, false)); + BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b4, true)); // A votes strong because it didn't see (and vote on) B3 + BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b4, false)); // B's last vote even if it wasn't propagated BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b4, false)); - BOOST_REQUIRE_EQUAL(qc_s(qc(b4)), qc_s(b2, true)); // b4 should include a strong qc on b2 + BOOST_REQUIRE_EQUAL(qc_s(qc(b4)), qc_s(b2, true)); // b4 should include a strong qc on b2 BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - auto b5 = A.produce_block(_block_interval_us * 2); // receives weak votes from {B, C}. + auto b5 = A.produce_block(_block_interval_us * 2); // receives weak votes from {B, C}. print("b5", b5); - BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b5, true)); // A votes strong because it didn't see (and vote on) B3 + BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b5, true)); // A votes strong because it didn't see (and vote on) B3 BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b5, false)); BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b5, false)); - BOOST_REQUIRE_EQUAL(qc_s(qc(b5)), qc_s(b2, true)); // b5 should include a strong qc on b2 + BOOST_REQUIRE(!qc(b5)); // Because B's vote was delayed, b5 should not have a QC BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); + auto b6 = A.produce_block(_block_interval_us * 2); // receives strong votes from {A, B, C}. + print("b6", b6); + BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b6, true)); // A votes strong because it didn't see (and vote on) B3 + BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b6, true)); // with issue #627 fix, should start voting strong again + BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b6, true)); // with issue #627 fix, should start voting strong again + BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), qc_s(b4, false)); // Because B's vote was delayed, b6 has a weak QC on b4 + BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); + + auto b7 = A.produce_block(_block_interval_us * 2); // receives strong votes from {A, B, C}. + print("b7", b7); + BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b7, true)); + BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b7, true)); + BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b7, true)); + BOOST_REQUIRE_EQUAL(qc_s(qc(b7)), qc_s(b5, false)); // Because B's vote was delayed, b7 has a weak QC on b5 + BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); + + auto b8 = A.produce_block(_block_interval_us * 2); // receives strong votes from {A, B, C}. + print("b8", b8); + BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b8, true)); + BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b8, true)); + BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b8, true)); + BOOST_REQUIRE_EQUAL(qc_s(qc(b8)), qc_s(b6, true)); // Because of the strong votes on b6, b8 has a strong QC on b6 + BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); + + } FC_LOG_AND_RETHROW() From 74d16682b811abdcbe95cc237074ab88ae339f77 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Mon, 26 Aug 2024 10:24:15 -0400 Subject: [PATCH 06/19] Validate testcase `gh_534_liveness_issue` with fsi changes. --- unittests/savanna_misc_tests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index aba06a6237..393147e5d2 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -169,7 +169,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { using vote_t = savanna_cluster::vote_t; using qc_s = savanna_cluster::qc_s; - _debug_mode = true; + //_debug_mode = true; auto b0 = A.produce_block(); // receives strong votes from all finalizers auto b1 = A.produce_block(); // receives strong votes from all finalizers @@ -247,7 +247,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b8, true)); BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b8, true)); BOOST_REQUIRE_EQUAL(qc_s(qc(b8)), qc_s(b6, true)); // Because of the strong votes on b6, b8 has a strong QC on b6 - BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); + BOOST_REQUIRE_EQUAL(A.lib_number, b4->block_num()); } FC_LOG_AND_RETHROW() From bf5f7cdd1a8194e17a6c8f3713b733541e6b3ba7 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Mon, 26 Aug 2024 10:26:41 -0400 Subject: [PATCH 07/19] Comment out new testcase as it fails with current fsi implementation --- unittests/savanna_misc_tests.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index 393147e5d2..1a7d65c311 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -165,12 +165,12 @@ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, savanna_cluster::cluster_t) try { // This testcase fails prior to https://github.com/AntelopeIO/spring/issues/621 being fixed. // ----------------------------------------------------------------------------------------------------- BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { +#if 0 auto& A=_nodes[0]; auto& B=_nodes[1]; auto& C=_nodes[2]; auto& D=_nodes[3]; using vote_t = savanna_cluster::vote_t; using qc_s = savanna_cluster::qc_s; //_debug_mode = true; - auto b0 = A.produce_block(); // receives strong votes from all finalizers auto b1 = A.produce_block(); // receives strong votes from all finalizers auto b2 = A.produce_block(); // receives strong votes from all finalizers @@ -248,8 +248,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b8, true)); BOOST_REQUIRE_EQUAL(qc_s(qc(b8)), qc_s(b6, true)); // Because of the strong votes on b6, b8 has a strong QC on b6 BOOST_REQUIRE_EQUAL(A.lib_number, b4->block_num()); - - +#endif } FC_LOG_AND_RETHROW() From 1d19e44ec9a10e454a7c8316db2aeade2369ad29 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Mon, 26 Aug 2024 10:59:54 -0400 Subject: [PATCH 08/19] Add big block comment copied from issue #621. --- unittests/savanna_misc_tests.cpp | 69 ++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index 1a7d65c311..5be47dbc97 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -164,6 +164,75 @@ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, savanna_cluster::cluster_t) try { // // This testcase fails prior to https://github.com/AntelopeIO/spring/issues/621 being fixed. // ----------------------------------------------------------------------------------------------------- +/* ----------------------------------------------------------------------------------------------------- + testcase + -------- +Time: t1 t2 t3 t4 t5 t6 t7 t8 +Blocks: + B0 <--- B1 <--- B2 <-|- B3 + | + \--------- B4 <--- B5 <--- B6 <--- B7 <--- B8 +QC claim: + Strong Strong Strong Strong Strong Weak Weak Strong + B0 B1 B2 B2 B2 B4 B5 B6 + +Vote: Strong Strong Strong Weak Weak Strong Strong Strong + + + +In the above example, things are moving along normally until time t4 when a microfork occurs. +Instead of building block B4 off of block B3, the producer builds block B4 off of block B2. +And then going forward, for some reason, it takes slightly longer for votes to propagate that a +QC on a block cannot be formed in time to be included in the very next block; instead the QC goes +in the block after. + +The finalizer of interest is voting on all of the blocks as they come. For this example, it is +sufficient to only have one finalizer. The first time the finalizer is forced to vote weak is on +block B4. As the other blocks continue to build on that new branch, it votes on them appropriately +and the producer collects the vote and forms a QC as soon as it can, which always remains one block +late. The finalizer should begin voting strong again starting with block B6. However, prior to the +changes described in this issue, the finalizer would remain stuck voting weak indefinitely. + +The expected state of the fsi record for the finalizer after each vote is provided below. It also +records what the new LIB should be after processing the block. In addition to checking that the blocks +have the claims as required above and the LIB as noted below, the test should also check that the fsi +record after each vote is as expected below. + +Finalizer fsi after voting strong on block B2 (LIB B0): +last_vote: B2 +lock: B1 +other_branch_latest_time: empty + +Finalizer fsi after voting strong on block B3 (LIB B1): +last_vote: B3 +lock: B2 +other_branch_latest_time: empty + +Finalizer fsi after voting weak on block B4 (LIB B1): +last_vote: B4 +lock: B2 +other_branch_latest_time: t3 + +Finalizer fsi after voting weak on block B5 (LIB B1): +last_vote: B5 +lock: B2 +other_branch_latest_time: t3 + +Finalizer fsi after voting strong on block B6 (LIB B1): +last_vote: B6 +lock: B4 +other_branch_latest_time: empty + +Finalizer fsi after voting strong on block B7 (LIB B1): +last_vote: B7 +lock: B5 +other_branch_latest_time: empty + +Finalizer fsi after voting strong on block B8 (LIB B4): +last_vote: B8 +lock: B6 +other_branch_latest_time: empty +--------------------------------------------------------------------------------------------------------- */ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { #if 0 auto& A=_nodes[0]; auto& B=_nodes[1]; auto& C=_nodes[2]; auto& D=_nodes[3]; From b8d832c2a5bb0291b1d291062944cf67b70b07b8 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Mon, 26 Aug 2024 19:40:35 -0400 Subject: [PATCH 09/19] Address PR comment. --- libraries/testing/tester.cpp | 3 +-- unittests/savanna_cluster.hpp | 5 ++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/libraries/testing/tester.cpp b/libraries/testing/tester.cpp index ee73a8b57e..03d6a7814c 100644 --- a/libraries/testing/tester.cpp +++ b/libraries/testing/tester.cpp @@ -1304,8 +1304,7 @@ namespace eosio::testing { auto [privkey, pubkey, pop] = get_bls_key(name); local_finalizer_keys[pubkey.to_string()] = privkey.to_string(); } - if (control) - control->set_node_finalizer_keys(local_finalizer_keys); + control->set_node_finalizer_keys(local_finalizer_keys); } base_tester::set_finalizers_output_t base_tester::set_active_finalizers(std::span names) { diff --git a/unittests/savanna_cluster.hpp b/unittests/savanna_cluster.hpp index c8dd02bbde..1945b08b9b 100644 --- a/unittests/savanna_cluster.hpp +++ b/unittests/savanna_cluster.hpp @@ -112,7 +112,10 @@ namespace savanna_cluster { void set_node_finalizers(std::span names) { _node_finalizers = std::vector{ names.begin(), names.end() }; - tester::set_node_finalizers(_node_finalizers); + if (control) { + // node is "open", se we can update the tester immediately + tester::set_node_finalizers(_node_finalizers); + } } void transition_to_savanna(std::span finalizer_policy_names) { From 8f383b67672de5e6a6e507d5937f798f45fcc70c Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Tue, 27 Aug 2024 07:46:12 -0400 Subject: [PATCH 10/19] Address PR comments, incl. not skipping slots for b5 to b8. --- unittests/savanna_misc_tests.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index 5be47dbc97..7e3eea3d5f 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -261,7 +261,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { // otherwise it will switch forks and build its next block (b4) // on top of it - push_block(1, b3); // push block to A, B and C, should receive strong votes + push_block(1, b3); // push block to B and C, should receive strong votes BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b2, true)); BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b3, true)); BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b3, true)); @@ -278,7 +278,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { fc::scoped_set_value tmp(B.vote_delay(), 1); // delaying just B's votes should be enough to delay QCs - auto b4 = A.produce_block(_block_interval_us * 2); // receives weak votes from {B, C}. + auto b4 = A.produce_block(_block_interval_us * 2); // b4 skips a slot. receives weak votes from {B, C}. print("b4", b4); BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b4, true)); // A votes strong because it didn't see (and vote on) B3 BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b4, false)); // B's last vote even if it wasn't propagated @@ -286,7 +286,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(qc_s(qc(b4)), qc_s(b2, true)); // b4 should include a strong qc on b2 BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - auto b5 = A.produce_block(_block_interval_us * 2); // receives weak votes from {B, C}. + auto b5 = A.produce_block(); // receives weak votes from {B, C}. print("b5", b5); BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b5, true)); // A votes strong because it didn't see (and vote on) B3 BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b5, false)); @@ -294,7 +294,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE(!qc(b5)); // Because B's vote was delayed, b5 should not have a QC BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - auto b6 = A.produce_block(_block_interval_us * 2); // receives strong votes from {A, B, C}. + auto b6 = A.produce_block(); // receives strong votes from {A, B, C}. print("b6", b6); BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b6, true)); // A votes strong because it didn't see (and vote on) B3 BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b6, true)); // with issue #627 fix, should start voting strong again @@ -302,7 +302,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), qc_s(b4, false)); // Because B's vote was delayed, b6 has a weak QC on b4 BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - auto b7 = A.produce_block(_block_interval_us * 2); // receives strong votes from {A, B, C}. + auto b7 = A.produce_block(); // receives strong votes from {A, B, C}. print("b7", b7); BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b7, true)); BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b7, true)); @@ -310,7 +310,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(qc_s(qc(b7)), qc_s(b5, false)); // Because B's vote was delayed, b7 has a weak QC on b5 BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - auto b8 = A.produce_block(_block_interval_us * 2); // receives strong votes from {A, B, C}. + auto b8 = A.produce_block(); // receives strong votes from {A, B, C}. print("b8", b8); BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b8, true)); BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b8, true)); From 3a5f697a8f8940c70ba1f5b9d685404e04054f3f Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Tue, 27 Aug 2024 08:20:20 -0400 Subject: [PATCH 11/19] Make the checks a little cleaner by using `strong_qc/weak_qc` and same for `vote` --- unittests/savanna_cluster.cpp | 6 ++++-- unittests/savanna_cluster.hpp | 21 ++++++++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/unittests/savanna_cluster.cpp b/unittests/savanna_cluster.cpp index 8596d0367f..0db47d75c9 100644 --- a/unittests/savanna_cluster.cpp +++ b/unittests/savanna_cluster.cpp @@ -4,7 +4,9 @@ namespace savanna_cluster { node_t::node_t(size_t node_idx, cluster_t& cluster, setup_policy policy /* = setup_policy::none */) : tester(policy) - , _node_idx(node_idx) { + , _node_idx(node_idx) + , _last_vote({}, false) +{ // since we are creating forks, finalizers may be locked on another fork and unable to vote. do_check_for_votes(false); @@ -17,7 +19,7 @@ node_t::node_t(size_t node_idx, cluster_t& cluster, setup_policy policy /* = set if (status == vote_result_t::success) { vote_message_ptr vote_msg = std::get<2>(v); - _last_vote = vote_t(vote_msg); + _last_vote = vote_t(vote_msg->block_id, vote_msg->strong); if (_propagate_votes) { if (_vote_delay) diff --git a/unittests/savanna_cluster.hpp b/unittests/savanna_cluster.hpp index 1945b08b9b..0827986d72 100644 --- a/unittests/savanna_cluster.hpp +++ b/unittests/savanna_cluster.hpp @@ -54,10 +54,6 @@ namespace savanna_cluster { // two classes for comparisons in BOOST_REQUIRE_EQUAL // -------------------------------------------------- struct vote_t { - vote_t() : strong(false) {} - explicit vote_t(const vote_message_ptr& p) : id(p->block_id), strong(p->strong) {} - explicit vote_t(const signed_block_ptr& p, bool strong) : id(p->calculate_id()), strong(strong) {} - friend std::ostream& operator<<(std::ostream& s, const vote_t& v) { s << "vote_t(" << v.id.str().substr(8, 16) << ", " << (v.strong ? "strong" : "weak") << ")"; return s; @@ -68,8 +64,16 @@ namespace savanna_cluster { bool strong; }; + struct strong_vote : public vote_t { + explicit strong_vote(const signed_block_ptr& p) : vote_t(p->calculate_id(), true) {} + }; + struct weak_vote : public vote_t { + explicit weak_vote(const signed_block_ptr& p) : vote_t(p->calculate_id(), false) {} + }; + + struct qc_s { - explicit qc_s(const signed_block_ptr& p, bool strong) : block_num(p->block_num()), strong(strong) {} + explicit qc_s(uint32_t block_num, bool strong) : block_num(block_num), strong(strong) {} explicit qc_s(const std::optional& qc) : block_num(qc->block_num), strong(qc->is_strong()) {} friend std::ostream& operator<<(std::ostream& s, const qc_s& v) { @@ -82,6 +86,13 @@ namespace savanna_cluster { bool strong; }; + struct strong_qc : public qc_s { + explicit strong_qc(const signed_block_ptr& p) : qc_s(p->block_num(), true) {} + }; + struct weak_qc : public qc_s { + explicit weak_qc(const signed_block_ptr& p) : qc_s(p->block_num(), false) {} + }; + // ---------------------------------------------------------------------------- class node_t : public tester { private: From 9153e40866f97cd451f6226efa4fadca43dcdd6e Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Tue, 27 Aug 2024 08:22:17 -0400 Subject: [PATCH 12/19] Forgot this file! --- unittests/savanna_misc_tests.cpp | 104 +++++++++++++++---------------- 1 file changed, 51 insertions(+), 53 deletions(-) diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index 7e3eea3d5f..bbd4f99c30 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -60,9 +60,8 @@ D produces +--------------------| b2 | */ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, 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]; - using vote_t = savanna_cluster::vote_t; - using qc_s = savanna_cluster::qc_s; //_debug_mode = true; @@ -89,14 +88,14 @@ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, savanna_cluster::cluster_t) try { // on top of it push_block(1, b2); // push block to B and C, should receive weak votes - BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b2, false)); - BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b2, false)); - BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b1, true));// A should not have seen b2, and therefore not voted on it + BOOST_REQUIRE_EQUAL(B.last_vote(), weak_vote(b2)); + BOOST_REQUIRE_EQUAL(C.last_vote(), weak_vote(b2)); + BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b1));// A should not have seen b2, and therefore not voted on it - BOOST_REQUIRE_EQUAL(qc_s(qc(b2)), qc_s(b0, true)); // b2 should include a strong qc on b0 + BOOST_REQUIRE_EQUAL(qc_s(qc(b2)), strong_qc(b0)); // b2 should include a strong qc on b0 - set_partition(partition); // restore our original partition {A, B, C} and {D} + set_partition(partition); // restore our original partition {A, B, C} and {D} signed_block_ptr b3; { @@ -107,11 +106,11 @@ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, savanna_cluster::cluster_t) try { // (not a quorum) // because B doesn't propagate and D is partitioned away print("b3", b3); - BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b3, true)); // A didn't vote on b2 so it can vote strong - BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b3, false)); // but B and C have to vote weak. - BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b3, false)); // C did vote, but we turned vote propagation off so + BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b3)); // A didn't vote on b2 so it can vote strong + BOOST_REQUIRE_EQUAL(B.last_vote(), weak_vote(b3)); // but B and C have to vote weak. + BOOST_REQUIRE_EQUAL(C.last_vote(), weak_vote(b3)); // C did vote, but we turned vote propagation off so // A will never see C's vote - BOOST_REQUIRE_EQUAL(qc_s(qc(b3)), qc_s(b1, true)); // b3 should include a strong qc on b1 + BOOST_REQUIRE_EQUAL(qc_s(qc(b3)), strong_qc(b1)); // b3 should include a strong qc on b1 } BOOST_REQUIRE_EQUAL(A.lib_number, b0->block_num()); @@ -121,9 +120,9 @@ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, savanna_cluster::cluster_t) try { // and should include a strong QC claim on b1 (repeated) // since we don't have enough votes to form a QC on b3 print("b4", b4); - BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b4, true)); - BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b4, false)); - BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b4, false)); + BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b4)); + BOOST_REQUIRE_EQUAL(B.last_vote(), weak_vote(b4)); + BOOST_REQUIRE_EQUAL(C.last_vote(), weak_vote(b4)); BOOST_REQUIRE_EQUAL(qc_claim(b3), qc_claim(b4)); // A didn't form a QC on b3, so b4 should repeat b3's claim BOOST_REQUIRE(!qc(b4)); // b4 should not have a QC extension (no new QC formed on b3) @@ -134,18 +133,18 @@ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, savanna_cluster::cluster_t) try { // weak QC on b4, which itself had a strong QC on b1. // Upon receiving a strong QC on b5, b4 will be final print("b5", b5); - BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b5, true)); - BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b5, true)); - BOOST_REQUIRE_EQUAL(qc_s(qc(b5)), qc_s(b4, false)); // b5 should include a weak qc on b4 + BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b5)); + BOOST_REQUIRE_EQUAL(B.last_vote(), strong_vote(b5)); + BOOST_REQUIRE_EQUAL(qc_s(qc(b5)), weak_qc(b4)); // b5 should include a weak qc on b4 BOOST_REQUIRE_EQUAL(A.lib_number, b0->block_num()); auto b6 = A.produce_block(); // should include a strong QC on b5, b1 should be final print("b6", b6); - BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), qc_s(b5, true)); // b6 should include a strong qc on b5 + BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), strong_qc(b5)); // b6 should include a strong qc on b5 - BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b6, true)); - BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b6, true)); + BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b6)); + BOOST_REQUIRE_EQUAL(B.last_vote(), strong_vote(b6)); BOOST_REQUIRE_EQUAL(A.lib_number, b4->block_num()); @@ -235,9 +234,8 @@ other_branch_latest_time: empty --------------------------------------------------------------------------------------------------------- */ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { #if 0 + using namespace savanna_cluster; auto& A=_nodes[0]; auto& B=_nodes[1]; auto& C=_nodes[2]; auto& D=_nodes[3]; - using vote_t = savanna_cluster::vote_t; - using qc_s = savanna_cluster::qc_s; //_debug_mode = true; auto b0 = A.produce_block(); // receives strong votes from all finalizers @@ -262,11 +260,11 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { // on top of it push_block(1, b3); // push block to B and C, should receive strong votes - BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b2, true)); - BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b3, true)); - BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b3, true)); - BOOST_REQUIRE_EQUAL(D.last_vote(), vote_t(b3, true)); - BOOST_REQUIRE_EQUAL(qc_s(qc(b3)), qc_s(b2, true)); // b3 should include a strong qc on b2 + BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b2)); + BOOST_REQUIRE_EQUAL(B.last_vote(), strong_vote(b3)); + BOOST_REQUIRE_EQUAL(C.last_vote(), strong_vote(b3)); + BOOST_REQUIRE_EQUAL(D.last_vote(), strong_vote(b3)); + BOOST_REQUIRE_EQUAL(qc_s(qc(b3)), strong_qc(b2)); // b3 should include a strong qc on b2 BOOST_REQUIRE_EQUAL(B.lib_number, b1->block_num()); // don't use A.lib_number as A is partitioned by itself // so it didn't see b3 and its enclosed QC. @@ -276,46 +274,46 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { // claim the parent block, but an ancestor, we need to artificially delay propagating the votes. // --------------------------------------------------------------------------------------------- - fc::scoped_set_value tmp(B.vote_delay(), 1); // delaying just B's votes should be enough to delay QCs + fc::scoped_set_value tmp(B.vote_delay(), 1); // delaying just B's votes should be enough to delay QCs - auto b4 = A.produce_block(_block_interval_us * 2); // b4 skips a slot. receives weak votes from {B, C}. + auto b4 = A.produce_block(_block_interval_us * 2); // b4 skips a slot. receives weak votes from {B, C}. print("b4", b4); - BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b4, true)); // A votes strong because it didn't see (and vote on) B3 - BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b4, false)); // B's last vote even if it wasn't propagated - BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b4, false)); - BOOST_REQUIRE_EQUAL(qc_s(qc(b4)), qc_s(b2, true)); // b4 should include a strong qc on b2 + BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b4)); // A votes strong because it didn't see (and vote on) B3 + BOOST_REQUIRE_EQUAL(B.last_vote(), weak_vote(b4)); // B's last vote even if it wasn't propagated + BOOST_REQUIRE_EQUAL(C.last_vote(), weak_vote(b4)); + BOOST_REQUIRE_EQUAL(qc_s(qc(b4)), strong_qc(b2)); // b4 should include a strong qc on b2 BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - auto b5 = A.produce_block(); // receives weak votes from {B, C}. + auto b5 = A.produce_block(); // receives weak votes from {B, C}. print("b5", b5); - BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b5, true)); // A votes strong because it didn't see (and vote on) B3 - BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b5, false)); - BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b5, false)); - BOOST_REQUIRE(!qc(b5)); // Because B's vote was delayed, b5 should not have a QC + BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b5)); // A votes strong because it didn't see (and vote on) B3 + BOOST_REQUIRE_EQUAL(B.last_vote(), weak_vote(b5)); + BOOST_REQUIRE_EQUAL(C.last_vote(), weak_vote(b5)); + BOOST_REQUIRE(!qc(b5)); // Because B's vote was delayed, b5 should not have a QC BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - auto b6 = A.produce_block(); // receives strong votes from {A, B, C}. + auto b6 = A.produce_block(); // receives strong votes from {A, B, C}. print("b6", b6); - BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b6, true)); // A votes strong because it didn't see (and vote on) B3 - BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b6, true)); // with issue #627 fix, should start voting strong again - BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b6, true)); // with issue #627 fix, should start voting strong again - BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), qc_s(b4, false)); // Because B's vote was delayed, b6 has a weak QC on b4 + BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b6)); // A votes strong because it didn't see (and vote on) B3 + BOOST_REQUIRE_EQUAL(B.last_vote(), strong_vote(b6)); // with issue #627 fix, should start voting strong again + BOOST_REQUIRE_EQUAL(C.last_vote(), strong_vote(b6)); // with issue #627 fix, should start voting strong again + BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), weak_qc(b4)); // Because B's vote was delayed, b6 has a weak QC on b4 BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - auto b7 = A.produce_block(); // receives strong votes from {A, B, C}. + auto b7 = A.produce_block(); // receives strong votes from {A, B, C}. print("b7", b7); - BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b7, true)); - BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b7, true)); - BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b7, true)); - BOOST_REQUIRE_EQUAL(qc_s(qc(b7)), qc_s(b5, false)); // Because B's vote was delayed, b7 has a weak QC on b5 + BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b7)); + BOOST_REQUIRE_EQUAL(B.last_vote(), strong_vote(b7)); + BOOST_REQUIRE_EQUAL(C.last_vote(), strong_vote(b7)); + BOOST_REQUIRE_EQUAL(qc_s(qc(b7)), weak_qc(b5)); // Because B's vote was delayed, b7 has a weak QC on b5 BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - auto b8 = A.produce_block(); // receives strong votes from {A, B, C}. + auto b8 = A.produce_block(); // receives strong votes from {A, B, C}. print("b8", b8); - BOOST_REQUIRE_EQUAL(A.last_vote(), vote_t(b8, true)); - BOOST_REQUIRE_EQUAL(B.last_vote(), vote_t(b8, true)); - BOOST_REQUIRE_EQUAL(C.last_vote(), vote_t(b8, true)); - BOOST_REQUIRE_EQUAL(qc_s(qc(b8)), qc_s(b6, true)); // Because of the strong votes on b6, b8 has a strong QC on b6 + BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b8)); + BOOST_REQUIRE_EQUAL(B.last_vote(), strong_vote(b8)); + BOOST_REQUIRE_EQUAL(C.last_vote(), strong_vote(b8)); + BOOST_REQUIRE_EQUAL(qc_s(qc(b8)), strong_qc(b6)); // Because of the strong votes on b6, b8 has a strong QC on b6 BOOST_REQUIRE_EQUAL(A.lib_number, b4->block_num()); #endif } FC_LOG_AND_RETHROW() From 511c2a1a4219324dbde51f2af1a8656451997f35 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Tue, 27 Aug 2024 08:29:05 -0400 Subject: [PATCH 13/19] Remove `#ifdef`, align comments. --- unittests/savanna_misc_tests.cpp | 40 +++++++++++++++----------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index bbd4f99c30..dcbd531181 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -115,10 +115,10 @@ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(A.lib_number, b0->block_num()); - // Now B broadcasts its votes again, so - auto b4 = A.produce_block(); // b4 should receive 3 weak votes from A, B and C - // and should include a strong QC claim on b1 (repeated) - // since we don't have enough votes to form a QC on b3 + // Now B broadcasts its votes again, so + auto b4 = A.produce_block(); // b4 should receive 3 weak votes from A, B and C + // and should include a strong QC claim on b1 (repeated) + // since we don't have enough votes to form a QC on b3 print("b4", b4); BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b4)); BOOST_REQUIRE_EQUAL(B.last_vote(), weak_vote(b4)); @@ -135,13 +135,13 @@ BOOST_FIXTURE_TEST_CASE(weak_masking_issue, savanna_cluster::cluster_t) try { print("b5", b5); BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b5)); BOOST_REQUIRE_EQUAL(B.last_vote(), strong_vote(b5)); - BOOST_REQUIRE_EQUAL(qc_s(qc(b5)), weak_qc(b4)); // b5 should include a weak qc on b4 + BOOST_REQUIRE_EQUAL(qc_s(qc(b5)), weak_qc(b4)); // b5 should include a weak qc on b4 BOOST_REQUIRE_EQUAL(A.lib_number, b0->block_num()); auto b6 = A.produce_block(); // should include a strong QC on b5, b1 should be final print("b6", b6); - BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), strong_qc(b5)); // b6 should include a strong qc on b5 + BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), strong_qc(b5)); // b6 should include a strong qc on b5 BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b6)); BOOST_REQUIRE_EQUAL(B.last_vote(), strong_vote(b6)); @@ -233,14 +233,13 @@ lock: B6 other_branch_latest_time: empty --------------------------------------------------------------------------------------------------------- */ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { -#if 0 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(); // receives strong votes from all finalizers - auto b1 = A.produce_block(); // receives strong votes from all finalizers - auto b2 = A.produce_block(); // receives strong votes from all finalizers + auto b0 = A.produce_block(); // receives strong votes from all finalizers + auto b1 = A.produce_block(); // receives strong votes from all finalizers + auto b2 = A.produce_block(); // receives strong votes from all finalizers print("b1", b1); print("b2", b2); BOOST_REQUIRE_EQUAL(A.lib_number, b0->block_num()); @@ -251,24 +250,24 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { const std::vector partition {3}; set_partition(partition); - auto b3 = D.produce_block(); // produce a block on D + auto b3 = D.produce_block(); // produce a block on D print("b3", b3); - const std::vector tmp_partition {0}; // we temporarily separate A (before pushing b3) - set_partition(tmp_partition); // because we don't want A to see the block produced by D (b3) - // otherwise it will switch forks and build its next block (b4) - // on top of it + const std::vector tmp_partition {0}; // we temporarily separate A (before pushing b3) + set_partition(tmp_partition); // because we don't want A to see the block produced by D (b3) + // otherwise it will switch forks and build its next block (b4) + // on top of it - push_block(1, b3); // push block to B and C, should receive strong votes + push_block(1, b3); // push block to B and C, should receive strong votes BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b2)); BOOST_REQUIRE_EQUAL(B.last_vote(), strong_vote(b3)); BOOST_REQUIRE_EQUAL(C.last_vote(), strong_vote(b3)); BOOST_REQUIRE_EQUAL(D.last_vote(), strong_vote(b3)); - BOOST_REQUIRE_EQUAL(qc_s(qc(b3)), strong_qc(b2)); // b3 should include a strong qc on b2 - BOOST_REQUIRE_EQUAL(B.lib_number, b1->block_num()); // don't use A.lib_number as A is partitioned by itself - // so it didn't see b3 and its enclosed QC. + BOOST_REQUIRE_EQUAL(qc_s(qc(b3)), strong_qc(b2)); // b3 should include a strong qc on b2 + BOOST_REQUIRE_EQUAL(B.lib_number, b1->block_num()); // don't use A.lib_number as A is partitioned by itself + // so it didn't see b3 and its enclosed QC. - set_partition(partition); // restore our original partition {A, B, C} and {D} + set_partition(partition); // restore our original partition {A, B, C} and {D} // from now on, to reproduce the scenario where votes are delayed, so the QC we receive don't // claim the parent block, but an ancestor, we need to artificially delay propagating the votes. @@ -315,7 +314,6 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(C.last_vote(), strong_vote(b8)); BOOST_REQUIRE_EQUAL(qc_s(qc(b8)), strong_qc(b6)); // Because of the strong votes on b6, b8 has a strong QC on b6 BOOST_REQUIRE_EQUAL(A.lib_number, b4->block_num()); -#endif } FC_LOG_AND_RETHROW() From d3bb8156281e86d6815a42c1202b64fa5fcd56b7 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Tue, 27 Aug 2024 14:35:37 -0400 Subject: [PATCH 14/19] Add `fsi` checks. --- libraries/chain/controller.cpp | 4 ++ .../chain/include/eosio/chain/controller.hpp | 3 ++ .../eosio/chain/finality/finalizer.hpp | 2 +- unittests/savanna_cluster.hpp | 9 +++++ unittests/savanna_misc_tests.cpp | 37 +++++++++++++------ 5 files changed, 43 insertions(+), 12 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index a0f4c18293..987c229cb0 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -5986,6 +5986,10 @@ bool controller::is_node_finalizer_key(const bls_public_key& key) const { return my->my_finalizers.contains(key); } +const my_finalizers_t& controller::get_node_finalizers() const { + return my->my_finalizers; +} + /// Protocol feature activation handlers: template<> diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index 94de0b7f83..3e0528aea0 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -9,6 +9,7 @@ #include #include #include +#include #include @@ -452,6 +453,8 @@ namespace eosio::chain { // is the bls key a registered finalizer key of this node, thread safe bool is_node_finalizer_key(const bls_public_key& key) const; + const my_finalizers_t& get_node_finalizers() const; // used for tests (purpose is inspecting fsi) + private: friend class apply_context; friend class transaction_context; diff --git a/libraries/chain/include/eosio/chain/finality/finalizer.hpp b/libraries/chain/include/eosio/chain/finality/finalizer.hpp index 1ca4c90a2d..e43139dca3 100644 --- a/libraries/chain/include/eosio/chain/finality/finalizer.hpp +++ b/libraries/chain/include/eosio/chain/finality/finalizer.hpp @@ -181,7 +181,7 @@ namespace eosio::chain { fsi_map load_finalizer_safety_info(); // for testing purposes only, not thread safe - const fsi_t& get_fsi(const bls_public_key& k) { return finalizers[k].fsi; } + const fsi_t& get_fsi(const bls_public_key& k) const { return finalizers.at(k).fsi; } void set_fsi(const bls_public_key& k, const fsi_t& fsi) { finalizers[k].fsi = fsi; } private: diff --git a/unittests/savanna_cluster.hpp b/unittests/savanna_cluster.hpp index 0827986d72..f5109d8454 100644 --- a/unittests/savanna_cluster.hpp +++ b/unittests/savanna_cluster.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -22,6 +23,7 @@ namespace savanna_cluster { using block_header = eosio::chain::block_header; using tester = eosio::testing::tester; using setup_policy = eosio::testing::setup_policy; + using fsi_t = eosio::chain::finalizer_safety_information; class cluster_t; @@ -289,6 +291,13 @@ namespace savanna_cluster { } } + const fsi_t& get_fsi(size_t idx = 0) const { + assert(control); + assert(idx < _node_finalizers.size()); + auto [privkey, pubkey, pop] = get_bls_key(_node_finalizers[idx]); + return control->get_node_finalizers().get_fsi(pubkey); + } + private: // always removes reversible data (`blocks/reversible`) // optionally remove the blocks log as well by deleting the whole `blocks` directory diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index dcbd531181..f8e1bc633e 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -250,6 +250,14 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { const std::vector partition {3}; set_partition(partition); + auto check_fsi = [&](const node_t& node, const signed_block_ptr& last_vote, const signed_block_ptr& lock, + block_timestamp_type other_branch_latest_time) { + const fsi_t& fsi = node.get_fsi(); + BOOST_REQUIRE_EQUAL(fsi.last_vote.block_id, last_vote->calculate_id()); + BOOST_REQUIRE_EQUAL(fsi.lock.block_id, lock->calculate_id()); + BOOST_REQUIRE_EQUAL(fsi.other_branch_latest_time, other_branch_latest_time); + }; + auto b3 = D.produce_block(); // produce a block on D print("b3", b3); @@ -263,9 +271,10 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(B.last_vote(), strong_vote(b3)); BOOST_REQUIRE_EQUAL(C.last_vote(), strong_vote(b3)); BOOST_REQUIRE_EQUAL(D.last_vote(), strong_vote(b3)); - BOOST_REQUIRE_EQUAL(qc_s(qc(b3)), strong_qc(b2)); // b3 should include a strong qc on b2 - BOOST_REQUIRE_EQUAL(B.lib_number, b1->block_num()); // don't use A.lib_number as A is partitioned by itself + BOOST_REQUIRE_EQUAL(qc_s(qc(b3)), strong_qc(b2)); // b3 should include a strong qc on b2 + BOOST_REQUIRE_EQUAL(B.lib_number, b1->block_num()); // don't use A.lib_number as A is partitioned by itself // so it didn't see b3 and its enclosed QC. + check_fsi(B, b3, b2, {}); set_partition(partition); // restore our original partition {A, B, C} and {D} @@ -280,8 +289,9 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b4)); // A votes strong because it didn't see (and vote on) B3 BOOST_REQUIRE_EQUAL(B.last_vote(), weak_vote(b4)); // B's last vote even if it wasn't propagated BOOST_REQUIRE_EQUAL(C.last_vote(), weak_vote(b4)); - BOOST_REQUIRE_EQUAL(qc_s(qc(b4)), strong_qc(b2)); // b4 should include a strong qc on b2 - BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); + BOOST_REQUIRE_EQUAL(qc_s(qc(b4)), strong_qc(b2)); // b4 should include a strong qc on b2 + BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); + check_fsi(B, b4, b2, b3->timestamp); auto b5 = A.produce_block(); // receives weak votes from {B, C}. print("b5", b5); @@ -289,31 +299,36 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(B.last_vote(), weak_vote(b5)); BOOST_REQUIRE_EQUAL(C.last_vote(), weak_vote(b5)); BOOST_REQUIRE(!qc(b5)); // Because B's vote was delayed, b5 should not have a QC - BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); + BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); + check_fsi(B, b5, b2, b3->timestamp); auto b6 = A.produce_block(); // receives strong votes from {A, B, C}. print("b6", b6); BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b6)); // A votes strong because it didn't see (and vote on) B3 BOOST_REQUIRE_EQUAL(B.last_vote(), strong_vote(b6)); // with issue #627 fix, should start voting strong again BOOST_REQUIRE_EQUAL(C.last_vote(), strong_vote(b6)); // with issue #627 fix, should start voting strong again - BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), weak_qc(b4)); // Because B's vote was delayed, b6 has a weak QC on b4 - BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); + BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), weak_qc(b4)); // Because B's vote was delayed, b6 has a weak QC on b4 + BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); + check_fsi(B, b6, b4, {}); auto b7 = A.produce_block(); // receives strong votes from {A, B, C}. print("b7", b7); BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b7)); BOOST_REQUIRE_EQUAL(B.last_vote(), strong_vote(b7)); BOOST_REQUIRE_EQUAL(C.last_vote(), strong_vote(b7)); - BOOST_REQUIRE_EQUAL(qc_s(qc(b7)), weak_qc(b5)); // Because B's vote was delayed, b7 has a weak QC on b5 - BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); + BOOST_REQUIRE_EQUAL(qc_s(qc(b7)), weak_qc(b5)); // Because B's vote was delayed, b7 has a weak QC on b5 + BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); + check_fsi(B, b7, b5, {}); auto b8 = A.produce_block(); // receives strong votes from {A, B, C}. print("b8", b8); BOOST_REQUIRE_EQUAL(A.last_vote(), strong_vote(b8)); BOOST_REQUIRE_EQUAL(B.last_vote(), strong_vote(b8)); BOOST_REQUIRE_EQUAL(C.last_vote(), strong_vote(b8)); - BOOST_REQUIRE_EQUAL(qc_s(qc(b8)), strong_qc(b6)); // Because of the strong votes on b6, b8 has a strong QC on b6 - BOOST_REQUIRE_EQUAL(A.lib_number, b4->block_num()); + BOOST_REQUIRE_EQUAL(qc_s(qc(b8)), strong_qc(b6)); // Because of the strong votes on b6, b8 has a strong QC on b6 + BOOST_REQUIRE_EQUAL(A.lib_number, b4->block_num()); + check_fsi(B, b8, b6, {}); + } FC_LOG_AND_RETHROW() From 4d5f1cb4445699bb4ef0bb72c4df6fcdc8601e0e Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Tue, 27 Aug 2024 14:43:47 -0400 Subject: [PATCH 15/19] Make `check_fsi` checks a bit clearer. --- unittests/savanna_misc_tests.cpp | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index f8e1bc633e..94f99e8b9b 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -250,12 +250,16 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { const std::vector partition {3}; set_partition(partition); - auto check_fsi = [&](const node_t& node, const signed_block_ptr& last_vote, const signed_block_ptr& lock, - block_timestamp_type other_branch_latest_time) { + struct fsi_expect { + const signed_block_ptr& last_vote; + const signed_block_ptr& lock; + block_timestamp_type other_branch_latest_time; + }; + auto check_fsi = [&](const node_t& node, fsi_expect expected) { const fsi_t& fsi = node.get_fsi(); - BOOST_REQUIRE_EQUAL(fsi.last_vote.block_id, last_vote->calculate_id()); - BOOST_REQUIRE_EQUAL(fsi.lock.block_id, lock->calculate_id()); - BOOST_REQUIRE_EQUAL(fsi.other_branch_latest_time, other_branch_latest_time); + BOOST_REQUIRE_EQUAL(fsi.last_vote.block_id, expected.last_vote->calculate_id()); + BOOST_REQUIRE_EQUAL(fsi.lock.block_id, expected.lock->calculate_id()); + BOOST_REQUIRE_EQUAL(fsi.other_branch_latest_time, expected.other_branch_latest_time); }; auto b3 = D.produce_block(); // produce a block on D @@ -274,7 +278,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(qc_s(qc(b3)), strong_qc(b2)); // b3 should include a strong qc on b2 BOOST_REQUIRE_EQUAL(B.lib_number, b1->block_num()); // don't use A.lib_number as A is partitioned by itself // so it didn't see b3 and its enclosed QC. - check_fsi(B, b3, b2, {}); + check_fsi(B, {.last_vote = b3, .lock = b2, .other_branch_latest_time = {}}); set_partition(partition); // restore our original partition {A, B, C} and {D} @@ -291,7 +295,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(C.last_vote(), weak_vote(b4)); BOOST_REQUIRE_EQUAL(qc_s(qc(b4)), strong_qc(b2)); // b4 should include a strong qc on b2 BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - check_fsi(B, b4, b2, b3->timestamp); + check_fsi(B, fsi_expect{.last_vote = b4, .lock = b2, .other_branch_latest_time = b3->timestamp }); auto b5 = A.produce_block(); // receives weak votes from {B, C}. print("b5", b5); @@ -300,7 +304,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(C.last_vote(), weak_vote(b5)); BOOST_REQUIRE(!qc(b5)); // Because B's vote was delayed, b5 should not have a QC BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - check_fsi(B, b5, b2, b3->timestamp); + check_fsi(B, fsi_expect{.last_vote = b5, .lock = b2, .other_branch_latest_time = b3->timestamp }); auto b6 = A.produce_block(); // receives strong votes from {A, B, C}. print("b6", b6); @@ -309,7 +313,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(C.last_vote(), strong_vote(b6)); // with issue #627 fix, should start voting strong again BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), weak_qc(b4)); // Because B's vote was delayed, b6 has a weak QC on b4 BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - check_fsi(B, b6, b4, {}); + check_fsi(B, fsi_expect{.last_vote = b6, .lock = b4, .other_branch_latest_time = {}}); auto b7 = A.produce_block(); // receives strong votes from {A, B, C}. print("b7", b7); @@ -318,7 +322,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(C.last_vote(), strong_vote(b7)); BOOST_REQUIRE_EQUAL(qc_s(qc(b7)), weak_qc(b5)); // Because B's vote was delayed, b7 has a weak QC on b5 BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - check_fsi(B, b7, b5, {}); + check_fsi(B, fsi_expect{.last_vote = b7, .lock = b5, .other_branch_latest_time = {}}); auto b8 = A.produce_block(); // receives strong votes from {A, B, C}. print("b8", b8); @@ -327,7 +331,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(C.last_vote(), strong_vote(b8)); BOOST_REQUIRE_EQUAL(qc_s(qc(b8)), strong_qc(b6)); // Because of the strong votes on b6, b8 has a strong QC on b6 BOOST_REQUIRE_EQUAL(A.lib_number, b4->block_num()); - check_fsi(B, b8, b6, {}); + check_fsi(B, fsi_expect{.last_vote = b8, .lock = b6, .other_branch_latest_time = {}}); } FC_LOG_AND_RETHROW() From 81df89660a81cedc5c2830e728171f9f654139c6 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Tue, 27 Aug 2024 15:58:42 -0400 Subject: [PATCH 16/19] Make `get_node_finalizers` private and fix test. --- libraries/chain/include/eosio/chain/controller.hpp | 8 +++++++- unittests/finalizer_tests.cpp | 3 +-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index 3e0528aea0..599800e753 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -23,6 +23,10 @@ namespace boost::asio { class thread_pool; } +namespace savanna_cluster { + class node_t; +} + namespace eosio::vm { class wasm_allocator; } namespace eosio::chain { @@ -453,11 +457,13 @@ namespace eosio::chain { // is the bls key a registered finalizer key of this node, thread safe bool is_node_finalizer_key(const bls_public_key& key) const; - const my_finalizers_t& get_node_finalizers() const; // used for tests (purpose is inspecting fsi) private: + const my_finalizers_t& get_node_finalizers() const; // used for tests (purpose is inspecting fsi). + friend class apply_context; friend class transaction_context; + friend class savanna_cluster::node_t; chainbase::database& mutable_db()const; diff --git a/unittests/finalizer_tests.cpp b/unittests/finalizer_tests.cpp index e78c66a81f..9ed35fb4ee 100644 --- a/unittests/finalizer_tests.cpp +++ b/unittests/finalizer_tests.cpp @@ -162,8 +162,7 @@ BOOST_AUTO_TEST_CASE( corrupt_finalizer_safety_file ) try { finalizer_safety_exception); // make sure the safety info for our finalizer that we saved above is restored correctly - BOOST_CHECK_NE(fset.get_fsi(k.pubkey), fsi); - BOOST_CHECK_EQUAL(fset.get_fsi(k.pubkey), fsi_t()); + BOOST_CHECK(!fset.contains(k.pubkey)); } } FC_LOG_AND_RETHROW() From 7db60833da95097eab410d740792f849cb7e979a Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Tue, 27 Aug 2024 16:50:52 -0400 Subject: [PATCH 17/19] Add `Boost::crc` as a controller dependency. --- libraries/chain/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chain/CMakeLists.txt b/libraries/chain/CMakeLists.txt index 1b0dab0194..b35cae82cb 100644 --- a/libraries/chain/CMakeLists.txt +++ b/libraries/chain/CMakeLists.txt @@ -157,7 +157,7 @@ add_library(Boost::numeric_ublas ALIAS boost_numeric_ublas) target_link_libraries( eosio_chain PUBLIC bn256 fc chainbase eosio_rapidjson Logging IR WAST WASM softfloat builtins ${CHAIN_EOSVM_LIBRARIES} ${LLVM_LIBS} ${CHAIN_RT_LINKAGE} Boost::signals2 Boost::hana Boost::property_tree Boost::multi_index Boost::asio Boost::lockfree - Boost::assign Boost::accumulators Boost::rational + Boost::assign Boost::accumulators Boost::rational Boost::crc ) target_include_directories( eosio_chain PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/include" "${CMAKE_CURRENT_BINARY_DIR}/include" From 9758307792696fe9ae47c814b181ebc7e14893fd Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Tue, 27 Aug 2024 16:58:49 -0400 Subject: [PATCH 18/19] Move `check_fsi()` to `savanna_cluster` so it can be re-used. --- unittests/savanna_cluster.hpp | 13 +++++++++++++ unittests/savanna_misc_tests.cpp | 24 ++++++------------------ 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/unittests/savanna_cluster.hpp b/unittests/savanna_cluster.hpp index f5109d8454..20e2c448c2 100644 --- a/unittests/savanna_cluster.hpp +++ b/unittests/savanna_cluster.hpp @@ -95,6 +95,12 @@ namespace savanna_cluster { explicit weak_qc(const signed_block_ptr& p) : qc_s(p->block_num(), false) {} }; + struct fsi_expect { + const signed_block_ptr& last_vote; + const signed_block_ptr& lock; + block_timestamp_type other_branch_latest_time; + }; + // ---------------------------------------------------------------------------- class node_t : public tester { private: @@ -298,6 +304,13 @@ namespace savanna_cluster { return control->get_node_finalizers().get_fsi(pubkey); } + void check_fsi(const fsi_expect& expected) { + const fsi_t& fsi = get_fsi(); + BOOST_REQUIRE_EQUAL(fsi.last_vote.block_id, expected.last_vote->calculate_id()); + BOOST_REQUIRE_EQUAL(fsi.lock.block_id, expected.lock->calculate_id()); + BOOST_REQUIRE_EQUAL(fsi.other_branch_latest_time, expected.other_branch_latest_time); + } + private: // always removes reversible data (`blocks/reversible`) // optionally remove the blocks log as well by deleting the whole `blocks` directory diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index 94f99e8b9b..e77b9c4b68 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -250,18 +250,6 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { const std::vector partition {3}; set_partition(partition); - struct fsi_expect { - const signed_block_ptr& last_vote; - const signed_block_ptr& lock; - block_timestamp_type other_branch_latest_time; - }; - auto check_fsi = [&](const node_t& node, fsi_expect expected) { - const fsi_t& fsi = node.get_fsi(); - BOOST_REQUIRE_EQUAL(fsi.last_vote.block_id, expected.last_vote->calculate_id()); - BOOST_REQUIRE_EQUAL(fsi.lock.block_id, expected.lock->calculate_id()); - BOOST_REQUIRE_EQUAL(fsi.other_branch_latest_time, expected.other_branch_latest_time); - }; - auto b3 = D.produce_block(); // produce a block on D print("b3", b3); @@ -278,7 +266,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(qc_s(qc(b3)), strong_qc(b2)); // b3 should include a strong qc on b2 BOOST_REQUIRE_EQUAL(B.lib_number, b1->block_num()); // don't use A.lib_number as A is partitioned by itself // so it didn't see b3 and its enclosed QC. - check_fsi(B, {.last_vote = b3, .lock = b2, .other_branch_latest_time = {}}); + B.check_fsi({.last_vote = b3, .lock = b2, .other_branch_latest_time = {}}); set_partition(partition); // restore our original partition {A, B, C} and {D} @@ -295,7 +283,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(C.last_vote(), weak_vote(b4)); BOOST_REQUIRE_EQUAL(qc_s(qc(b4)), strong_qc(b2)); // b4 should include a strong qc on b2 BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - check_fsi(B, fsi_expect{.last_vote = b4, .lock = b2, .other_branch_latest_time = b3->timestamp }); + B.check_fsi(fsi_expect{.last_vote = b4, .lock = b2, .other_branch_latest_time = b3->timestamp }); auto b5 = A.produce_block(); // receives weak votes from {B, C}. print("b5", b5); @@ -304,7 +292,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(C.last_vote(), weak_vote(b5)); BOOST_REQUIRE(!qc(b5)); // Because B's vote was delayed, b5 should not have a QC BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - check_fsi(B, fsi_expect{.last_vote = b5, .lock = b2, .other_branch_latest_time = b3->timestamp }); + B.check_fsi(fsi_expect{.last_vote = b5, .lock = b2, .other_branch_latest_time = b3->timestamp }); auto b6 = A.produce_block(); // receives strong votes from {A, B, C}. print("b6", b6); @@ -313,7 +301,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(C.last_vote(), strong_vote(b6)); // with issue #627 fix, should start voting strong again BOOST_REQUIRE_EQUAL(qc_s(qc(b6)), weak_qc(b4)); // Because B's vote was delayed, b6 has a weak QC on b4 BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - check_fsi(B, fsi_expect{.last_vote = b6, .lock = b4, .other_branch_latest_time = {}}); + B.check_fsi(fsi_expect{.last_vote = b6, .lock = b4, .other_branch_latest_time = {}}); auto b7 = A.produce_block(); // receives strong votes from {A, B, C}. print("b7", b7); @@ -322,7 +310,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(C.last_vote(), strong_vote(b7)); BOOST_REQUIRE_EQUAL(qc_s(qc(b7)), weak_qc(b5)); // Because B's vote was delayed, b7 has a weak QC on b5 BOOST_REQUIRE_EQUAL(A.lib_number, b1->block_num()); - check_fsi(B, fsi_expect{.last_vote = b7, .lock = b5, .other_branch_latest_time = {}}); + B.check_fsi(fsi_expect{.last_vote = b7, .lock = b5, .other_branch_latest_time = {}}); auto b8 = A.produce_block(); // receives strong votes from {A, B, C}. print("b8", b8); @@ -331,7 +319,7 @@ BOOST_FIXTURE_TEST_CASE(gh_534_liveness_issue, savanna_cluster::cluster_t) try { BOOST_REQUIRE_EQUAL(C.last_vote(), strong_vote(b8)); BOOST_REQUIRE_EQUAL(qc_s(qc(b8)), strong_qc(b6)); // Because of the strong votes on b6, b8 has a strong QC on b6 BOOST_REQUIRE_EQUAL(A.lib_number, b4->block_num()); - check_fsi(B, fsi_expect{.last_vote = b8, .lock = b6, .other_branch_latest_time = {}}); + B.check_fsi(fsi_expect{.last_vote = b8, .lock = b6, .other_branch_latest_time = {}}); } FC_LOG_AND_RETHROW() From cd44dc762df4593d3e30a9be7b44b7b1d0d40094 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Tue, 27 Aug 2024 17:46:05 -0400 Subject: [PATCH 19/19] Add `Boost::crc` to `EosioTester.cmake.in` and `EosioTesterBuild.cmake.in` --- CMakeModules/EosioTester.cmake.in | 1 + CMakeModules/EosioTesterBuild.cmake.in | 1 + libraries/chain/CMakeLists.txt | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CMakeModules/EosioTester.cmake.in b/CMakeModules/EosioTester.cmake.in index 09ce12f59e..a4600d6f63 100644 --- a/CMakeModules/EosioTester.cmake.in +++ b/CMakeModules/EosioTester.cmake.in @@ -99,6 +99,7 @@ target_link_libraries(EosioChain INTERFACE Boost::interprocess Boost::asio Boost::beast + Boost::crc Boost::signals2 Boost::iostreams "-lz" # Needed by Boost iostreams diff --git a/CMakeModules/EosioTesterBuild.cmake.in b/CMakeModules/EosioTesterBuild.cmake.in index 60371ebe87..db7965dccb 100644 --- a/CMakeModules/EosioTesterBuild.cmake.in +++ b/CMakeModules/EosioTesterBuild.cmake.in @@ -96,6 +96,7 @@ target_link_libraries(EosioChain INTERFACE Boost::interprocess Boost::asio Boost::beast + Boost::crc Boost::signals2 Boost::iostreams "-lz" # Needed by Boost iostreams diff --git a/libraries/chain/CMakeLists.txt b/libraries/chain/CMakeLists.txt index b35cae82cb..1b0dab0194 100644 --- a/libraries/chain/CMakeLists.txt +++ b/libraries/chain/CMakeLists.txt @@ -157,7 +157,7 @@ add_library(Boost::numeric_ublas ALIAS boost_numeric_ublas) target_link_libraries( eosio_chain PUBLIC bn256 fc chainbase eosio_rapidjson Logging IR WAST WASM softfloat builtins ${CHAIN_EOSVM_LIBRARIES} ${LLVM_LIBS} ${CHAIN_RT_LINKAGE} Boost::signals2 Boost::hana Boost::property_tree Boost::multi_index Boost::asio Boost::lockfree - Boost::assign Boost::accumulators Boost::rational Boost::crc + Boost::assign Boost::accumulators Boost::rational ) target_include_directories( eosio_chain PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/include" "${CMAKE_CURRENT_BINARY_DIR}/include"