From 842e1b9c01398828d5d5e7e01ce6e1f22e2c7f97 Mon Sep 17 00:00:00 2001 From: fcecin Date: Tue, 19 Sep 2023 16:50:56 -0300 Subject: [PATCH 1/7] Add multi-hop network to test_pacemaker - test_pacemaker allows each test to define a specific connection mesh between nodes - added a simple unit test (#7) that can be used to test a solution for #1548 (just needs to introduce a disconnect between nodes) - added support to write multi-hop unit tests (with selective message propagation by type) The hotstuff protocol has "phases", and between two adjacent phases, we are typically using different message types. This allows us to write tests where we loop propagating a specific message type across the network, for as many hops as necessary, until the `dispatch` method on the `test_pacemaker` returns an empty vector, signaling no more message propagation happened. And then, we can start propagating the message type that's expected for the next phase of the protocol (these were generated during propagation of the previous type, but were just retained). All unit tests still use a full connection mesh, which is easily created by calling `test_pacemaker::connect(vector-with-all-node-names)`. The added test is meant to be changed by the inclusion of an explicit disconnect between the two rotating leaders; this can be used as the first test of a solution to message propagation. Closes #1658 --- .../include/eosio/hotstuff/test_pacemaker.hpp | 28 +++- libraries/hotstuff/test/test_hotstuff.cpp | 154 +++++++++++++++++- libraries/hotstuff/test/test_pacemaker.cpp | 98 +++++++---- 3 files changed, 235 insertions(+), 45 deletions(-) diff --git a/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp b/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp index cfa6b05239..2d5fdd057d 100644 --- a/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp +++ b/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp @@ -7,12 +7,19 @@ namespace eosio { namespace hotstuff { class test_pacemaker : public base_pacemaker { public: + using hotstuff_message = std::pair>; + enum hotstuff_message_index { + hs_proposal = 0, + hs_vote = 1, + hs_new_block = 2, + hs_new_view = 3, + hs_all_messages + }; + //class-specific functions bool is_qc_chain_active(const name & qcc_name) { return _qcc_deactivated.find(qcc_name) == _qcc_deactivated.end(); } - using hotstuff_message = std::pair>; - void set_proposer(name proposer); void set_leader(name leader); @@ -27,11 +34,17 @@ namespace eosio { namespace hotstuff { void add_message_to_queue(hotstuff_message msg); + void connect(const std::vector& nodes); + + void disconnect(const std::vector& nodes); + + bool is_connected(name node1, name node2); + void pipe(std::vector messages); - void dispatch(std::string memo, int count); + void dispatch(std::string memo, int count, hotstuff_message_index msg_type = hs_all_messages); - std::vector dispatch(std::string memo); + std::vector dispatch(std::string memo, hotstuff_message_index msg_type = hs_all_messages); void activate(name replica); void deactivate(name replica); @@ -72,9 +85,12 @@ namespace eosio { namespace hotstuff { // qc_chain ids in this set are currently deactivated set _qcc_deactivated; - private: + // network topology: key (node name) is connected to all nodes in the mapped set. + // double mapping, so if _net[a] yields b, then _net[b] yields a. + // this is a filter; messages to self won't happen even if _net[x] yields x. + map> _net; - std::vector _message_queue; + private: name _proposer; name _leader; diff --git a/libraries/hotstuff/test/test_hotstuff.cpp b/libraries/hotstuff/test/test_hotstuff.cpp index 997708fe91..5d1224cb40 100644 --- a/libraries/hotstuff/test/test_hotstuff.cpp +++ b/libraries/hotstuff/test/test_hotstuff.cpp @@ -46,12 +46,6 @@ class hotstuff_test_handler { _qc_chains.clear(); - // These used to be able to break the tests. Might be useful at some point. - _qc_chains.reserve( 100 ); - //_qc_chains.reserve( 10000 ); - //_qc_chains.reserve( 15 ); - //_qc_chains.reserve( replicas.size() ); - for (name r : replicas) { qc_chain *qcc_ptr = new qc_chain(r, &tpm, {r}, {}, hotstuff_logger); std::shared_ptr qcc_shared_ptr(qcc_ptr); @@ -138,6 +132,11 @@ class hotstuff_test_handler { std::cout << "\n"; } + + void dispatch(test_pacemaker& tpm, int hops, test_pacemaker::hotstuff_message_index msg_type, const std::string& memo = "") { + for (int i=0;isecond->get_state(fs_bpa); + auto qcc_bpb = std::find_if(ht._qc_chains.begin(), ht._qc_chains.end(), [&](const auto& q){ return q.first == "bpb"_n; }); + finalizer_state fs_bpb; + qcc_bpb->second->get_state(fs_bpb); + auto qcc_bpc = std::find_if(ht._qc_chains.begin(), ht._qc_chains.end(), [&](const auto& q){ return q.first == "bpc"_n; }); + finalizer_state fs_bpc; + qcc_bpc->second->get_state(fs_bpc); + + tpm.set_current_block_id(ids[0]); //first block + + tpm.beat(); //produce first block and associated proposal + + ht.dispatch(tpm, 2, test_pacemaker::hs_proposal); //send proposal to replicas (prepare on first block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + + ht.dispatch(tpm, 2, test_pacemaker::hs_vote); //send votes on proposal (prepareQC on first block) + + ht.dispatch(tpm, 2, test_pacemaker::hs_proposal); //send proposal to replicas (precommit on first block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + + ht.dispatch(tpm, 2, test_pacemaker::hs_vote); //propagating votes on new proposal (precommitQC on first block) + + ht.dispatch(tpm, 2, test_pacemaker::hs_proposal); //send proposal to replicas (commit on first block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + + tpm.set_next_leader("bpb"_n); //leader is set to rotate on next block + + ht.dispatch(tpm, 2, test_pacemaker::hs_vote); //propagating votes on new proposal (commitQC on first block) + + ht.dispatch(tpm, 2, test_pacemaker::hs_proposal); //send proposal to replicas (decide on first block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("487e5fcbf2c515618941291ae3b6dcebb68942983d8ac3f61c4bdd9901dadbe7")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + + ht.dispatch(tpm, 2, test_pacemaker::hs_vote); //propagating votes on new proposal (decide on first block) + + tpm.set_proposer("bpb"_n); //leader has rotated + tpm.set_leader("bpb"_n); + + tpm.set_current_block_id(ids[1]); //second block + + tpm.beat(); //produce second block and associated proposal + + ht.dispatch(tpm, 2, test_pacemaker::hs_proposal); //send proposal to replicas (prepare on second block) + + qcc_bpb->second->get_state(fs_bpb); + BOOST_CHECK_EQUAL(fs_bpb.b_leaf.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + BOOST_CHECK_EQUAL(fs_bpb.high_qc.proposal_id.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + BOOST_CHECK_EQUAL(fs_bpb.b_lock.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + BOOST_CHECK_EQUAL(fs_bpb.b_exec.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + + ht.dispatch(tpm, 2, test_pacemaker::hs_vote); //send votes on proposal (prepareQC on second block) + + ht.dispatch(tpm, 2, test_pacemaker::hs_proposal); //send proposal to replicas (precommit on second block) + + qcc_bpb->second->get_state(fs_bpb); + BOOST_CHECK_EQUAL(fs_bpb.b_leaf.str(), std::string("6462add7d157da87931c859cb689f722003a20f30c0f1408d11b872020903b85")); + BOOST_CHECK_EQUAL(fs_bpb.high_qc.proposal_id.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + BOOST_CHECK_EQUAL(fs_bpb.b_lock.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + BOOST_CHECK_EQUAL(fs_bpb.b_exec.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + + ht.dispatch(tpm, 2, test_pacemaker::hs_vote); //propagating votes on new proposal (precommitQC on second block) + + ht.dispatch(tpm, 2, test_pacemaker::hs_proposal); //send proposal to replicas (commit on second block) + + qcc_bpb->second->get_state(fs_bpb); + BOOST_CHECK_EQUAL(fs_bpb.b_leaf.str(), std::string("fd77164bf3898a6a8f27ccff440d17ef6870e75c368fcc93b969066cec70939c")); + BOOST_CHECK_EQUAL(fs_bpb.high_qc.proposal_id.str(), std::string("6462add7d157da87931c859cb689f722003a20f30c0f1408d11b872020903b85")); + BOOST_CHECK_EQUAL(fs_bpb.b_lock.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + BOOST_CHECK_EQUAL(fs_bpb.b_exec.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + + ht.dispatch(tpm, 2, test_pacemaker::hs_vote); //propagating votes on new proposal (commitQC on second block) + + ht.dispatch(tpm, 2, test_pacemaker::hs_proposal); //send proposal to replicas (decide on second block) + + qcc_bpb->second->get_state(fs_bpb); + BOOST_CHECK_EQUAL(fs_bpb.b_leaf.str(), std::string("89f468a127dbadd81b59076067238e3e9c313782d7d83141b16d9da4f2c2b078")); + BOOST_CHECK_EQUAL(fs_bpb.high_qc.proposal_id.str(), std::string("fd77164bf3898a6a8f27ccff440d17ef6870e75c368fcc93b969066cec70939c")); + BOOST_CHECK_EQUAL(fs_bpb.b_lock.str(), std::string("6462add7d157da87931c859cb689f722003a20f30c0f1408d11b872020903b85")); + BOOST_CHECK_EQUAL(fs_bpb.b_exec.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + + //check bpa as well + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("fd77164bf3898a6a8f27ccff440d17ef6870e75c368fcc93b969066cec70939c")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("6462add7d157da87931c859cb689f722003a20f30c0f1408d11b872020903b85")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + + //check bpc as well + qcc_bpc->second->get_state(fs_bpc); + BOOST_CHECK_EQUAL(fs_bpc.high_qc.proposal_id.str(), std::string("fd77164bf3898a6a8f27ccff440d17ef6870e75c368fcc93b969066cec70939c")); + BOOST_CHECK_EQUAL(fs_bpc.b_lock.str(), std::string("6462add7d157da87931c859cb689f722003a20f30c0f1408d11b872020903b85")); + BOOST_CHECK_EQUAL(fs_bpc.b_exec.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + + BOOST_CHECK_EQUAL(fs_bpa.b_finality_violation.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + +} FC_LOG_AND_RETHROW(); + BOOST_AUTO_TEST_SUITE_END() diff --git a/libraries/hotstuff/test/test_pacemaker.cpp b/libraries/hotstuff/test/test_pacemaker.cpp index 87aa3822a1..ae4b10f11b 100644 --- a/libraries/hotstuff/test/test_pacemaker.cpp +++ b/libraries/hotstuff/test/test_pacemaker.cpp @@ -31,6 +31,31 @@ namespace eosio::hotstuff { _pending_message_queue.push_back(msg); } + void test_pacemaker::connect(const std::vector& nodes) { + for (auto it1 = nodes.begin(); it1 != nodes.end(); ++it1) { + for (auto it2 = std::next(it1); it2 != nodes.end(); ++it2) { + _net[*it1].insert(*it2); + _net[*it2].insert(*it1); + } + } + } + + void test_pacemaker::disconnect(const std::vector& nodes) { + for (auto it1 = nodes.begin(); it1 != nodes.end(); ++it1) { + for (auto it2 = std::next(it1); it2 != nodes.end(); ++it2) { + _net[*it1].erase(*it2); + _net[*it2].erase(*it1); + } + } + } + + bool test_pacemaker::is_connected(name node1, name node2) { + auto it = _net.find(node1); + if (it == _net.end()) + return false; + return it->second.count(node2) > 0; + } + void test_pacemaker::pipe(std::vector messages) { auto itr = messages.begin(); while (itr != messages.end()) { @@ -39,17 +64,22 @@ namespace eosio::hotstuff { } } - void test_pacemaker::dispatch(std::string memo, int count) { + void test_pacemaker::dispatch(std::string memo, int count, hotstuff_message_index msg_type) { for (int i = 0 ; i < count ; i++) { - this->dispatch(memo); + this->dispatch(memo, msg_type); } } - std::vector test_pacemaker::dispatch(std::string memo) { + std::vector test_pacemaker::dispatch(std::string memo, hotstuff_message_index msg_type) { + + std::vector dispatched_messages; + std::vector kept_messages; - std::vector dispatched_messages = _pending_message_queue; - _message_queue = _pending_message_queue; + std::vector message_queue = _pending_message_queue; + // Need to clear the persisted message queue here because new ones are inserted in + // the loop below as a side-effect of the on_hs...() calls. Messages that are not + // propagated in the loop go into kept_messages and are reinserted after the loop. _pending_message_queue.clear(); size_t proposals_count = 0; @@ -57,37 +87,39 @@ namespace eosio::hotstuff { size_t new_blocks_count = 0; size_t new_views_count = 0; - auto msg_itr = _message_queue.begin(); - while (msg_itr!=_message_queue.end()) { + auto msg_itr = message_queue.begin(); + while (msg_itr != message_queue.end()) { + name sender_id = msg_itr->first; size_t v_index = msg_itr->second.index(); - if (v_index==0) - ++proposals_count; - else if (v_index==1) - ++votes_count; - else if (v_index==2) - ++new_blocks_count; - else if (v_index==3) - ++new_views_count; - else - throw std::runtime_error("unknown message variant"); - - if (msg_itr->second.index() == 0) - on_hs_proposal_msg(std::get(msg_itr->second), msg_itr->first); - else if (msg_itr->second.index() == 1) - on_hs_vote_msg(std::get(msg_itr->second), msg_itr->first); - else if (msg_itr->second.index() == 2) - on_hs_new_block_msg(std::get(msg_itr->second), msg_itr->first); - else if (msg_itr->second.index() == 3) - on_hs_new_view_msg(std::get(msg_itr->second), msg_itr->first); - else - throw std::runtime_error("unknown message variant"); + if (msg_type == hs_all_messages || msg_type == v_index) { + + if (v_index == hs_proposal) { + ++proposals_count; + on_hs_proposal_msg(std::get(msg_itr->second), sender_id); + } else if (v_index == hs_vote) { + ++votes_count; + on_hs_vote_msg(std::get(msg_itr->second), sender_id); + } else if (v_index == hs_new_block) { + ++new_blocks_count; + on_hs_new_block_msg(std::get(msg_itr->second), sender_id); + } else if (v_index == hs_new_view) { + ++new_views_count; + on_hs_new_view_msg(std::get(msg_itr->second), sender_id); + } else { + throw std::runtime_error("unknown message variant"); + } + + dispatched_messages.push_back(*msg_itr); + } else { + kept_messages.push_back(*msg_itr); + } ++msg_itr; } - _message_queue.clear(); + _pending_message_queue.insert(_pending_message_queue.end(), kept_messages.begin(), kept_messages.end()); if (memo != "") { ilog(" === ${memo} : ", ("memo", memo)); @@ -181,7 +213,7 @@ namespace eosio::hotstuff { while (qc_itr != _qcc_store.end()){ const name & qcc_name = qc_itr->first; std::shared_ptr & qcc_ptr = qc_itr->second; - if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) ) + if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) && is_connected(id, qcc_ptr->get_id_i())) qcc_ptr->on_hs_proposal_msg(0, msg); qc_itr++; } @@ -192,7 +224,7 @@ namespace eosio::hotstuff { while (qc_itr != _qcc_store.end()) { const name & qcc_name = qc_itr->first; std::shared_ptr & qcc_ptr = qc_itr->second; - if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) ) + if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) && is_connected(id, qcc_ptr->get_id_i())) qcc_ptr->on_hs_vote_msg(0, msg); qc_itr++; } @@ -203,7 +235,7 @@ namespace eosio::hotstuff { while (qc_itr != _qcc_store.end()) { const name & qcc_name = qc_itr->first; std::shared_ptr & qcc_ptr = qc_itr->second; - if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) ) + if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) && is_connected(id, qcc_ptr->get_id_i())) qcc_ptr->on_hs_new_block_msg(0, msg); qc_itr++; } @@ -214,7 +246,7 @@ namespace eosio::hotstuff { while (qc_itr != _qcc_store.end()){ const name & qcc_name = qc_itr->first; std::shared_ptr & qcc_ptr = qc_itr->second; - if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) ) + if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) && is_connected(id, qcc_ptr->get_id_i())) qcc_ptr->on_hs_new_view_msg(0, msg); qc_itr++; } From 887e4ff65eede13084f634f4037c79285bc794ca Mon Sep 17 00:00:00 2001 From: Fabiana Cecin Date: Wed, 20 Sep 2023 11:52:00 -0300 Subject: [PATCH 2/7] Update libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp Co-authored-by: Gregory Popovitch --- libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp b/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp index 2d5fdd057d..40015a7911 100644 --- a/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp +++ b/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp @@ -40,7 +40,7 @@ namespace eosio { namespace hotstuff { bool is_connected(name node1, name node2); - void pipe(std::vector messages); + void pipe(const std::vector& messages); void dispatch(std::string memo, int count, hotstuff_message_index msg_type = hs_all_messages); From 357dcc4424d2151232b34a9d43078c908a5ca6ce Mon Sep 17 00:00:00 2001 From: Fabiana Cecin Date: Wed, 20 Sep 2023 11:52:38 -0300 Subject: [PATCH 3/7] Update libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp Co-authored-by: Gregory Popovitch --- libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp b/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp index 40015a7911..daf3fce3ba 100644 --- a/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp +++ b/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp @@ -32,7 +32,7 @@ namespace eosio { namespace hotstuff { void set_quorum_threshold(uint32_t threshold); - void add_message_to_queue(hotstuff_message msg); + void add_message_to_queue(const hotstuff_message& msg); void connect(const std::vector& nodes); From 5c6c3112836a93dd2b9f095689183874e53e8fd5 Mon Sep 17 00:00:00 2001 From: fcecin Date: Wed, 20 Sep 2023 12:16:43 -0300 Subject: [PATCH 4/7] Misc fixes - add const& 's - all members private - remove dead code - better loop patterns by @greg7mdp --- .../include/eosio/hotstuff/test_pacemaker.hpp | 6 +-- libraries/hotstuff/test/test_hotstuff.cpp | 4 -- libraries/hotstuff/test/test_pacemaker.cpp | 52 ++++++------------- 3 files changed, 19 insertions(+), 43 deletions(-) diff --git a/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp b/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp index daf3fce3ba..db6ab6ba8b 100644 --- a/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp +++ b/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp @@ -26,7 +26,7 @@ namespace eosio { namespace hotstuff { void set_next_leader(name next_leader); - void set_finalizers(std::vector finalizers); + void set_finalizers(const std::vector& finalizers); void set_current_block_id(block_id_type id); @@ -77,6 +77,8 @@ namespace eosio { namespace hotstuff { void send_hs_message_warning(const uint32_t sender_peer, const chain::hs_message_warning code); + private: + std::vector _pending_message_queue; // qc_chain id to qc_chain object @@ -90,8 +92,6 @@ namespace eosio { namespace hotstuff { // this is a filter; messages to self won't happen even if _net[x] yields x. map> _net; - private: - name _proposer; name _leader; name _next_leader; diff --git a/libraries/hotstuff/test/test_hotstuff.cpp b/libraries/hotstuff/test/test_hotstuff.cpp index 5d1224cb40..456e4d0535 100644 --- a/libraries/hotstuff/test/test_hotstuff.cpp +++ b/libraries/hotstuff/test/test_hotstuff.cpp @@ -88,10 +88,6 @@ class hotstuff_test_handler { std::cout << "\n"; } - void print_msg_queue(test_pacemaker &tpm){ - print_msgs(tpm._pending_message_queue); - } - void print_pm_state(test_pacemaker &tpm){ std::cout << "\n"; std::cout << " leader : " << tpm.get_leader() << "\n"; diff --git a/libraries/hotstuff/test/test_pacemaker.cpp b/libraries/hotstuff/test/test_pacemaker.cpp index ae4b10f11b..5780c5624c 100644 --- a/libraries/hotstuff/test/test_pacemaker.cpp +++ b/libraries/hotstuff/test/test_pacemaker.cpp @@ -15,7 +15,7 @@ namespace eosio::hotstuff { _next_leader = next_leader; }; - void test_pacemaker::set_finalizers(std::vector finalizers) { + void test_pacemaker::set_finalizers(const std::vector& finalizers) { _finalizers = finalizers; }; @@ -27,7 +27,7 @@ namespace eosio::hotstuff { _quorum_threshold = threshold; } - void test_pacemaker::add_message_to_queue(hotstuff_message msg) { + void test_pacemaker::add_message_to_queue(const hotstuff_message& msg) { _pending_message_queue.push_back(msg); } @@ -56,7 +56,7 @@ namespace eosio::hotstuff { return it->second.count(node2) > 0; } - void test_pacemaker::pipe(std::vector messages) { + void test_pacemaker::pipe(const std::vector& messages) { auto itr = messages.begin(); while (itr != messages.end()) { _pending_message_queue.push_back(*itr); @@ -87,36 +87,32 @@ namespace eosio::hotstuff { size_t new_blocks_count = 0; size_t new_views_count = 0; - auto msg_itr = message_queue.begin(); - while (msg_itr != message_queue.end()) { - - name sender_id = msg_itr->first; - size_t v_index = msg_itr->second.index(); + for (const auto& msg_pair : message_queue) { + const auto& [sender_id, msg] = msg_pair; + size_t v_index = msg.index(); if (msg_type == hs_all_messages || msg_type == v_index) { if (v_index == hs_proposal) { ++proposals_count; - on_hs_proposal_msg(std::get(msg_itr->second), sender_id); + on_hs_proposal_msg(std::get(msg), sender_id); } else if (v_index == hs_vote) { ++votes_count; - on_hs_vote_msg(std::get(msg_itr->second), sender_id); + on_hs_vote_msg(std::get(msg), sender_id); } else if (v_index == hs_new_block) { ++new_blocks_count; - on_hs_new_block_msg(std::get(msg_itr->second), sender_id); + on_hs_new_block_msg(std::get(msg), sender_id); } else if (v_index == hs_new_view) { ++new_views_count; - on_hs_new_view_msg(std::get(msg_itr->second), sender_id); + on_hs_new_view_msg(std::get(msg), sender_id); } else { throw std::runtime_error("unknown message variant"); } - dispatched_messages.push_back(*msg_itr); + dispatched_messages.push_back(msg_pair); } else { - kept_messages.push_back(*msg_itr); + kept_messages.push_back(msg_pair); } - - ++msg_itr; } _pending_message_queue.insert(_pending_message_queue.end(), kept_messages.begin(), kept_messages.end()); @@ -209,46 +205,30 @@ namespace eosio::hotstuff { void test_pacemaker::send_hs_message_warning(const uint32_t sender_peer, const chain::hs_message_warning code) { } void test_pacemaker::on_hs_proposal_msg(const hs_proposal_message& msg, name id) { - auto qc_itr = _qcc_store.begin(); - while (qc_itr != _qcc_store.end()){ - const name & qcc_name = qc_itr->first; - std::shared_ptr & qcc_ptr = qc_itr->second; + for (const auto& [qcc_name, qcc_ptr] : _qcc_store) { if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) && is_connected(id, qcc_ptr->get_id_i())) qcc_ptr->on_hs_proposal_msg(0, msg); - qc_itr++; } } void test_pacemaker::on_hs_vote_msg(const hs_vote_message& msg, name id) { - auto qc_itr = _qcc_store.begin(); - while (qc_itr != _qcc_store.end()) { - const name & qcc_name = qc_itr->first; - std::shared_ptr & qcc_ptr = qc_itr->second; + for (const auto& [qcc_name, qcc_ptr] : _qcc_store) { if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) && is_connected(id, qcc_ptr->get_id_i())) qcc_ptr->on_hs_vote_msg(0, msg); - qc_itr++; } } void test_pacemaker::on_hs_new_block_msg(const hs_new_block_message& msg, name id) { - auto qc_itr = _qcc_store.begin(); - while (qc_itr != _qcc_store.end()) { - const name & qcc_name = qc_itr->first; - std::shared_ptr & qcc_ptr = qc_itr->second; + for (const auto& [qcc_name, qcc_ptr] : _qcc_store) { if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) && is_connected(id, qcc_ptr->get_id_i())) qcc_ptr->on_hs_new_block_msg(0, msg); - qc_itr++; } } void test_pacemaker::on_hs_new_view_msg(const hs_new_view_message& msg, name id) { - auto qc_itr = _qcc_store.begin(); - while (qc_itr != _qcc_store.end()){ - const name & qcc_name = qc_itr->first; - std::shared_ptr & qcc_ptr = qc_itr->second; + for (const auto& [qcc_name, qcc_ptr] : _qcc_store) { if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) && is_connected(id, qcc_ptr->get_id_i())) qcc_ptr->on_hs_new_view_msg(0, msg); - qc_itr++; } } From c7645269cc29a0ba5de5a083713ed1b88e200ed2 Mon Sep 17 00:00:00 2001 From: fcecin Date: Sat, 23 Sep 2023 14:43:27 -0300 Subject: [PATCH 5/7] Filter duplicate votes - qc_chain does not aggregate a signature from a finalizer that has already voted on a proposal - added test_pacemaker::duplicate() - added hotstuff_8 unit test to test for duplicate votes This fix is related to #1548 (lack of duplicate filtering only manifests with message propagation) --- .../include/eosio/hotstuff/test_pacemaker.hpp | 2 + libraries/hotstuff/qc_chain.cpp | 8 + libraries/hotstuff/test/test_hotstuff.cpp | 139 ++++++++++++++++++ libraries/hotstuff/test/test_pacemaker.cpp | 12 ++ 4 files changed, 161 insertions(+) diff --git a/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp b/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp index db6ab6ba8b..8efbcf4d65 100644 --- a/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp +++ b/libraries/hotstuff/include/eosio/hotstuff/test_pacemaker.hpp @@ -42,6 +42,8 @@ namespace eosio { namespace hotstuff { void pipe(const std::vector& messages); + void duplicate(hotstuff_message_index msg_type); + void dispatch(std::string memo, int count, hotstuff_message_index msg_type = hs_all_messages); std::vector dispatch(std::string memo, hotstuff_message_index msg_type = hs_all_messages); diff --git a/libraries/hotstuff/qc_chain.cpp b/libraries/hotstuff/qc_chain.cpp index 667a0836cf..955b389e68 100644 --- a/libraries/hotstuff/qc_chain.cpp +++ b/libraries/hotstuff/qc_chain.cpp @@ -462,6 +462,14 @@ namespace eosio::hotstuff { auto increment_version = fc::make_scoped_exit([this]() { ++_state_version; }); const hs_bitset& finalizer_set = _current_qc.get_active_finalizers(); + + // if a finalizer has already aggregated a vote signature for the current QC, just discard this vote + vector finalizers = _pacemaker->get_finalizers(); + for (size_t i=0; isecond->get_state(fs_bpa); + auto qcc_bpb = std::find_if(ht._qc_chains.begin(), ht._qc_chains.end(), [&](const auto& q){ return q.first == "bpb"_n; }); + finalizer_state fs_bpb; + qcc_bpb->second->get_state(fs_bpb); + + ht.print_bp_state("bpa"_n, ""); + + tpm.set_current_block_id(ids[0]); //first block + + tpm.beat(); //produce first block and associated proposal + + tpm.dispatch(""); //send proposal to replicas (prepare on first block) + + ht.print_bp_state("bpa"_n, ""); + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + + // produce duplicate votes: should not fail the test if qc_chain is filtering duplicate votes. + // we cannot use pipe(dispatch()) here because pipe will append the duplicate votes like this to the pending message queue: + // abcdefghijklmnopqrstuabcdefghijklmnopqrstu + // however, after receiving 15 unique votes, the quorum is met and the duplicate votes are discared by the quorum rule. + // tpm.duplicate() will duplicate like this: aabbccddee...ssttuu, which will exercise the duplicate vote filter (bitset test). + tpm.duplicate(test_pacemaker::hs_vote); + + tpm.dispatch(""); //send votes on proposal (prepareQC on first block) + + tpm.dispatch(""); //send proposal to replicas (precommit on first block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + + tpm.dispatch(""); //propagating votes on new proposal (precommitQC on first block) + + tpm.dispatch(""); //send proposal to replicas (commit on first block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + + tpm.dispatch(""); //propagating votes on new proposal (commitQC on first block) + + tpm.dispatch(""); //send proposal to replicas (decide on first block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("487e5fcbf2c515618941291ae3b6dcebb68942983d8ac3f61c4bdd9901dadbe7")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + + tpm.dispatch(""); //propagating votes on new proposal (decide on first block) + + tpm.set_current_block_id(ids[1]); //second block + + tpm.beat(); //produce second block and associated proposal + + tpm.dispatch(""); //send proposal to replicas (prepare on second block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("a8c84b7f9613aebf2ae34f457189d58de95a6b0a50d103a4c9e6405180d6fffb")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("487e5fcbf2c515618941291ae3b6dcebb68942983d8ac3f61c4bdd9901dadbe7")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + + tpm.dispatch(""); //send votes on proposal (prepareQC on second block) + + tpm.dispatch(""); //send proposal to replicas (precommit on second block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("4af7c22e5220a61ac96c35533539e65d398e9f44de4c6e11b5b0279e7a79912f")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("a8c84b7f9613aebf2ae34f457189d58de95a6b0a50d103a4c9e6405180d6fffb")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("487e5fcbf2c515618941291ae3b6dcebb68942983d8ac3f61c4bdd9901dadbe7")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + + tpm.dispatch(""); //propagating votes on new proposal (precommitQC on second block) + + tpm.dispatch(""); //send proposal to replicas (commit on second block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("ab04f499892ad5ebd209d54372fd5c0bda0288410a084b55c70eda40514044f3")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("4af7c22e5220a61ac96c35533539e65d398e9f44de4c6e11b5b0279e7a79912f")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("a8c84b7f9613aebf2ae34f457189d58de95a6b0a50d103a4c9e6405180d6fffb")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("487e5fcbf2c515618941291ae3b6dcebb68942983d8ac3f61c4bdd9901dadbe7")); + + tpm.dispatch(""); //propagating votes on new proposal (commitQC on second block) + + tpm.dispatch(""); //send proposal to replicas (decide on second block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("9eeffb58a16133517d8d2f6f90b8a3420269de3356362677055b225a44a7c151")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("ab04f499892ad5ebd209d54372fd5c0bda0288410a084b55c70eda40514044f3")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("4af7c22e5220a61ac96c35533539e65d398e9f44de4c6e11b5b0279e7a79912f")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("a8c84b7f9613aebf2ae34f457189d58de95a6b0a50d103a4c9e6405180d6fffb")); + + tpm.dispatch(""); //send proposal to replicas (decide on second block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("9eeffb58a16133517d8d2f6f90b8a3420269de3356362677055b225a44a7c151")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("9eeffb58a16133517d8d2f6f90b8a3420269de3356362677055b225a44a7c151")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("4af7c22e5220a61ac96c35533539e65d398e9f44de4c6e11b5b0279e7a79912f")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("a8c84b7f9613aebf2ae34f457189d58de95a6b0a50d103a4c9e6405180d6fffb")); + + //check bpb as well + qcc_bpb->second->get_state(fs_bpb); + BOOST_CHECK_EQUAL(fs_bpb.high_qc.proposal_id.str(), std::string("ab04f499892ad5ebd209d54372fd5c0bda0288410a084b55c70eda40514044f3")); + BOOST_CHECK_EQUAL(fs_bpb.b_lock.str(), std::string("4af7c22e5220a61ac96c35533539e65d398e9f44de4c6e11b5b0279e7a79912f")); + BOOST_CHECK_EQUAL(fs_bpb.b_exec.str(), std::string("a8c84b7f9613aebf2ae34f457189d58de95a6b0a50d103a4c9e6405180d6fffb")); + + BOOST_CHECK_EQUAL(fs_bpa.b_finality_violation.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + +} FC_LOG_AND_RETHROW(); + BOOST_AUTO_TEST_SUITE_END() diff --git a/libraries/hotstuff/test/test_pacemaker.cpp b/libraries/hotstuff/test/test_pacemaker.cpp index 5780c5624c..f1770c54a9 100644 --- a/libraries/hotstuff/test/test_pacemaker.cpp +++ b/libraries/hotstuff/test/test_pacemaker.cpp @@ -70,6 +70,18 @@ namespace eosio::hotstuff { } } + void test_pacemaker::duplicate(hotstuff_message_index msg_type) { + std::vector dup; + for (const auto& msg_pair : _pending_message_queue) { + const auto& [sender_id, msg] = msg_pair; + size_t v_index = msg.index(); + dup.push_back(msg_pair); + if (v_index == msg_type) + dup.push_back(msg_pair); + } + _pending_message_queue = dup; + } + std::vector test_pacemaker::dispatch(std::string memo, hotstuff_message_index msg_type) { std::vector dispatched_messages; From ead5eb47ea3840fa20ed791a1f06e82f1d34f3a0 Mon Sep 17 00:00:00 2001 From: Fabiana Cecin Date: Sat, 23 Sep 2023 16:38:08 -0300 Subject: [PATCH 6/7] Update libraries/hotstuff/qc_chain.cpp Co-authored-by: Gregory Popovitch --- libraries/hotstuff/qc_chain.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/hotstuff/qc_chain.cpp b/libraries/hotstuff/qc_chain.cpp index 955b389e68..99018fb36b 100644 --- a/libraries/hotstuff/qc_chain.cpp +++ b/libraries/hotstuff/qc_chain.cpp @@ -464,7 +464,7 @@ namespace eosio::hotstuff { const hs_bitset& finalizer_set = _current_qc.get_active_finalizers(); // if a finalizer has already aggregated a vote signature for the current QC, just discard this vote - vector finalizers = _pacemaker->get_finalizers(); + const vector& finalizers = _pacemaker->get_finalizers(); for (size_t i=0; i Date: Sat, 23 Sep 2023 16:38:16 -0300 Subject: [PATCH 7/7] Update libraries/hotstuff/test/test_pacemaker.cpp Co-authored-by: Gregory Popovitch --- libraries/hotstuff/test/test_pacemaker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/hotstuff/test/test_pacemaker.cpp b/libraries/hotstuff/test/test_pacemaker.cpp index f1770c54a9..f285dc899f 100644 --- a/libraries/hotstuff/test/test_pacemaker.cpp +++ b/libraries/hotstuff/test/test_pacemaker.cpp @@ -79,7 +79,7 @@ namespace eosio::hotstuff { if (v_index == msg_type) dup.push_back(msg_pair); } - _pending_message_queue = dup; + _pending_message_queue = std::move(dup); } std::vector test_pacemaker::dispatch(std::string memo, hotstuff_message_index msg_type) {