diff --git a/libraries/libfc/include/fc/scoped_exit.hpp b/libraries/libfc/include/fc/scoped_exit.hpp index 657bfe6a08..d21b24a2a2 100644 --- a/libraries/libfc/include/fc/scoped_exit.hpp +++ b/libraries/libfc/include/fc/scoped_exit.hpp @@ -36,4 +36,41 @@ namespace fc { return scoped_exit( std::forward(c) ); } + // --------------------------------------------------------------------------- + // An object which assigns a value to a variable in its constructor, and resets + // to its previous value in its destructor + // --------------------------------------------------------------------------- + template + class scoped_set_value { + public: + template + [[nodiscard]] scoped_set_value(T& var, V&& val, + bool do_it = true) noexcept(std::is_nothrow_copy_constructible_v && + std::is_nothrow_move_assignable_v) + : _v(var) + , _do_it(do_it) { + if (_do_it) { + _old_value = std::move(_v); + _v = std::forward(val); + } + } + + ~scoped_set_value() { + if (_do_it) + _v = std::move(_old_value); + } + + void dismiss() noexcept { _do_it = false; } + + scoped_set_value(const scoped_set_value&) = delete; + scoped_set_value& operator=(const scoped_set_value&) = delete; + scoped_set_value(scoped_set_value&&) = delete; + scoped_set_value& operator=(scoped_set_value&&) = delete; + void* operator new(std::size_t) = delete; + + private: + T& _v; + T _old_value; + bool _do_it; + }; } diff --git a/unittests/savanna_cluster.cpp b/unittests/savanna_cluster.cpp index 428f9a063a..279ba7f585 100644 --- a/unittests/savanna_cluster.cpp +++ b/unittests/savanna_cluster.cpp @@ -14,8 +14,13 @@ node_t::node_t(size_t node_idx, cluster_t& cluster, setup_policy policy /* = set // to vote (and emit the `voted_block` signal) synchronously. // -------------------------------------------------------------------------------------- vote_result_t status = std::get<1>(v); - if (status == vote_result_t::success) - cluster.dispatch_vote_to_peers(node_idx, skip_self_t::yes, std::get<2>(v)); + + 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)); + } }; // called on `commit_block`, for both blocks received from `push_block` and produced blocks diff --git a/unittests/savanna_cluster.hpp b/unittests/savanna_cluster.hpp index b135a1ca7e..7c5ac49676 100644 --- a/unittests/savanna_cluster.hpp +++ b/unittests/savanna_cluster.hpp @@ -54,13 +54,30 @@ namespace savanna_cluster { // ---------------------------------------------------------------------------- class node_t : public tester { private: - size_t node_idx; - bool pushing_a_block {false }; + size_t node_idx; + bool pushing_a_block{false}; std::function accepted_block_cb; std::function voted_block_cb; 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; + }; + + bool propagate_votes{true}; + vote_t last_vote; std::vector node_finalizers; public: @@ -151,8 +168,7 @@ 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); - pushing_a_block = true; - auto reset_pending_on_exit = fc::make_scoped_exit([this]{ pushing_a_block = false; }); + fc::scoped_set_value set_pushing_a_block(pushing_a_block, true); tester::push_block(b); } } @@ -403,7 +419,7 @@ namespace savanna_cluster { // returns the number of nodes where `lib` has advanced after executing `f` template - size_t num_lib_advancing(F&& f) { + size_t num_lib_advancing(F&& f) const { std::vector libs(_nodes.size()); for (size_t i=0; i<_nodes.size(); ++i) libs[i] = _nodes[i].lib_num(); @@ -455,9 +471,44 @@ 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; + } + + static std::optional qc(const signed_block_ptr& b) { + if (b->contains_extension(quorum_certificate_extension::extension_id())) + return b->extract_extension().qc; + return {}; + } + + // debugging utilities + // ------------------- + void print(const char* name, const signed_block_ptr& b) const { + if (_debug_mode) + std::cout << name << " ts = " << b->timestamp.slot << ", id = " << b->calculate_id().str().substr(8, 16) + << ", previous = " << b->previous.str().substr(8, 16) << '\n'; + } + public: std::vector _nodes; fin_keys_t _fin_keys; + bool _debug_mode{false}; static constexpr fc::microseconds _block_interval_us = fc::milliseconds(eosio::chain::config::block_interval_ms); diff --git a/unittests/savanna_misc_tests.cpp b/unittests/savanna_misc_tests.cpp index bcc48467d0..11285975bb 100644 --- a/unittests/savanna_misc_tests.cpp +++ b/unittests/savanna_misc_tests.cpp @@ -5,6 +5,7 @@ using namespace eosio::testing; BOOST_AUTO_TEST_SUITE(savanna_misc_tests) +// ------------------------------------------------------------------------------------ // Verify that we can restart a node from a snapshot without state or blocks (reversible // or not) // ------------------------------------------------------------------------------------ @@ -21,6 +22,7 @@ BOOST_FIXTURE_TEST_CASE(snapshot_startup_without_forkdb, savanna_cluster::cluste } FC_LOG_AND_RETHROW() +// ------------------------------------------------------------------------------------ // Verify that we cannot restart a node from a snapshot without state and blocks log, // but with a fork database // ------------------------------------------------------------------------------------ @@ -39,5 +41,112 @@ BOOST_FIXTURE_TEST_CASE(snapshot_startup_with_forkdb, savanna_cluster::cluster_t } FC_LOG_AND_RETHROW() +// ----------------------------------------------------------------------------------------------------- +// Test case demonstrating the weak masking issue (see https://github.com/AntelopeIO/spring/issues/534) +// Because the issue is fixed in spring https://github.com/AntelopeIO/spring/pull/537, test must pass +// on all versions past that commit. +// ----------------------------------------------------------------------------------------------------- +/* + S + +------------------------------+ + V | + +-----+ S +-----+ S +-----+ no +-----+ W +-----+ S +-----+ +A produces <----| b0 |<-----| b1 |<-----------| b3 |<-------+ b4 |<-----| b5 |<----| b6 |<------- + +-----+ +-----+ +-----+ claim +-----+ +-----+ +-----+ + ^ + | +-----+ +D produces +--------------------| b2 | + S +-----+ + +*/ +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; + //_debug_mode = true; + + auto b0 = A.produce_blocks(2); // receives strong votes from all finalizers + print("b0", b0); + + // 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 b1 = A.produce_block(); // receives strong votes from 3 finalizers (D partitioned out) + print("b1", b1); + + auto b2 = D.produce_block(_block_interval_us * 2); // produce a `later` block on D + print("b2", b2); + + BOOST_REQUIRE_GT(b2->timestamp.slot, b1->timestamp.slot); + + const std::vector tmp_partition {0}; // we temporarily separate A (before pushing b2) + set_partitions({tmp_partition, partition}); // because we don't want A to see the block produced by D (b2) + // otherwise it will switch forks and build its next block (b3) + // 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(qc_s(qc(b2)), qc_s(b0, true)); // b2 should include a strong qc on b0 + + + set_partition(partition); // restore our original partition {A, B, C} and {D} + + signed_block_ptr b3; + { + 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 + // 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(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 + 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(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) + + BOOST_REQUIRE_EQUAL(A.lib_number, b0->block_num()); + + auto b5 = A.produce_block(); // a weak QC was formed on b4 and is included in b5 + // b5 should receive 3 strong votes (because it has a + // 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.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(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()); + +} FC_LOG_AND_RETHROW() BOOST_AUTO_TEST_SUITE_END()