From 372b8bfc3b07ef76135d0e40bc108d71a6a3a417 Mon Sep 17 00:00:00 2001 From: Guillaume Babin-Tremblay Date: Mon, 28 Aug 2023 19:17:40 +0000 Subject: [PATCH] Replaced hs_proposal_message::get_height() with ::get_view_number(), added unit tests. Work-in-progress towards #1521 --- .../chain/include/eosio/chain/hotstuff.hpp | 39 ++++++++- .../include/eosio/hotstuff/qc_chain.hpp | 6 +- libraries/hotstuff/qc_chain.cpp | 28 +++---- libraries/hotstuff/test/CMakeLists.txt | 2 +- libraries/hotstuff/test/hotstuff_tools.cpp | 62 ++++++++++++++ libraries/hotstuff/test/test_hotstuff.cpp | 1 - .../hotstuff/test/test_hotstuff_state.cpp | 83 +++++++++++++++++++ .../eosio/chain_plugin/chain_plugin.hpp | 4 +- 8 files changed, 202 insertions(+), 23 deletions(-) create mode 100644 libraries/hotstuff/test/hotstuff_tools.cpp create mode 100644 libraries/hotstuff/test/test_hotstuff_state.cpp diff --git a/libraries/chain/include/eosio/chain/hotstuff.hpp b/libraries/chain/include/eosio/chain/hotstuff.hpp index e51d6968fb..96f75320b8 100644 --- a/libraries/chain/include/eosio/chain/hotstuff.hpp +++ b/libraries/chain/include/eosio/chain/hotstuff.hpp @@ -15,6 +15,37 @@ namespace eosio::chain { return (uint64_t{block_height} << 32) | phase_counter; } + struct view_number{ + + view_number(){ + _data = std::make_pair(0,0); + } + view_number(std::pair data){ + _data = data; + } + + auto operator<=>(const view_number&) const = default; + + uint32_t block_height(){ + return _data.first; + } + + uint8_t phase_counter(){ + return _data.second; + } + + uint64_t to_uint64_t(){ + return compute_height(_data.first, _data.second); + } + + std::string to_string(){ + return _data.first + "::" + _data.second; + } + + std::pair _data; + + }; + struct extended_schedule { producer_authority_schedule producer_schedule; std::map bls_pub_keys; @@ -42,7 +73,10 @@ namespace eosio::chain { uint8_t phase_counter = 0; uint32_t block_num() const { return block_header::num_from_id(block_id); } - uint64_t get_height() const { return compute_height(block_header::num_from_id(block_id), phase_counter); }; + uint64_t get_key() const { return compute_height(block_header::num_from_id(block_id), phase_counter); }; + + view_number get_view_number() const { return std::make_pair(block_header::num_from_id(block_id), phase_counter); }; + }; struct hs_new_block_message { @@ -63,7 +97,7 @@ namespace eosio::chain { fc::sha256 b_finality_violation = NULL_PROPOSAL_ID; block_id_type block_exec = NULL_BLOCK_ID; block_id_type pending_proposal_block = NULL_BLOCK_ID; - uint32_t v_height = 0; + eosio::chain::view_number v_height; eosio::chain::quorum_certificate high_qc; eosio::chain::quorum_certificate current_qc; eosio::chain::extended_schedule schedule; @@ -84,6 +118,7 @@ namespace eosio::chain { } //eosio::chain +FC_REFLECT(eosio::chain::view_number, (_data)); FC_REFLECT(eosio::chain::quorum_certificate, (proposal_id)(active_finalizers)(active_agg_sig)); FC_REFLECT(eosio::chain::extended_schedule, (producer_schedule)(bls_pub_keys)); FC_REFLECT(eosio::chain::hs_vote_message, (proposal_id)(finalizer)(sig)); diff --git a/libraries/hotstuff/include/eosio/hotstuff/qc_chain.hpp b/libraries/hotstuff/include/eosio/hotstuff/qc_chain.hpp index 7918d68865..e8bd57de84 100644 --- a/libraries/hotstuff/include/eosio/hotstuff/qc_chain.hpp +++ b/libraries/hotstuff/include/eosio/hotstuff/qc_chain.hpp @@ -133,7 +133,7 @@ namespace eosio::hotstuff { fc::sha256 _b_finality_violation = NULL_PROPOSAL_ID; eosio::chain::quorum_certificate _high_qc; eosio::chain::quorum_certificate _current_qc; - uint32_t _v_height = 0; + eosio::chain::view_number _v_height; eosio::chain::extended_schedule _schedule; base_pacemaker* _pacemaker = nullptr; std::set _my_producers; @@ -164,11 +164,11 @@ namespace eosio::hotstuff { indexed_by< hashed_unique< tag, - BOOST_MULTI_INDEX_MEMBER(hs_proposal_message,fc::sha256,proposal_id) + BOOST_MULTI_INDEX_MEMBER(hs_proposal_message, fc::sha256,proposal_id) >, ordered_non_unique< tag, - BOOST_MULTI_INDEX_CONST_MEM_FUN(hs_proposal_message,uint64_t,get_height) + BOOST_MULTI_INDEX_CONST_MEM_FUN(hs_proposal_message, uint64_t, get_key) > > > proposal_store_type; diff --git a/libraries/hotstuff/qc_chain.cpp b/libraries/hotstuff/qc_chain.cpp index 94d43cdb2b..df1d8ffe58 100644 --- a/libraries/hotstuff/qc_chain.cpp +++ b/libraries/hotstuff/qc_chain.cpp @@ -72,7 +72,7 @@ namespace eosio { namespace hotstuff { bool qc_chain::insert_proposal(const hs_proposal_message & proposal) { #ifdef QC_CHAIN_SIMPLE_PROPOSAL_STORE - uint64_t proposal_height = proposal.get_height(); + uint64_t proposal_height = proposal.get_key(); ps_height_iterator psh_it = _proposal_stores_by_height.find( proposal_height ); if (psh_it == _proposal_stores_by_height.end()) { _proposal_stores_by_height.emplace( proposal_height, proposal_store() ); @@ -328,7 +328,7 @@ namespace eosio { namespace hotstuff { } hs_vote_message qc_chain::sign_proposal(const hs_proposal_message & proposal, name finalizer){ - _v_height = proposal.get_height(); + _v_height = proposal.get_view_number(); digest_type digest = get_digest_to_sign(proposal.block_id, proposal.phase_counter, proposal.final_on_qc); @@ -372,7 +372,7 @@ namespace eosio { namespace hotstuff { } #ifdef QC_CHAIN_SIMPLE_PROPOSAL_STORE - ps_height_iterator psh_it = _proposal_stores_by_height.find( proposal.get_height() ); + ps_height_iterator psh_it = _proposal_stores_by_height.find( proposal.get_key() ); if (psh_it != _proposal_stores_by_height.end()) { proposal_store & pstore = psh_it->second; @@ -382,8 +382,8 @@ namespace eosio { namespace hotstuff { hs_proposal_message & existing_proposal = ps_it->second; #else //height is not necessarily unique, so we iterate over all prior proposals at this height - auto hgt_itr = _proposal_store.get().lower_bound( proposal.get_height() ); - auto end_itr = _proposal_store.get().upper_bound( proposal.get_height() ); + auto hgt_itr = _proposal_store.get().lower_bound( proposal.get_key() ); + auto end_itr = _proposal_store.get().upper_bound( proposal.get_key() ); while (hgt_itr != end_itr) { const hs_proposal_message & existing_proposal = *hgt_itr; @@ -734,13 +734,13 @@ namespace eosio { namespace hotstuff { if (new_high_qc_prop == nullptr) return false; - if (new_high_qc_prop->get_height() > old_high_qc_prop->get_height() + if (new_high_qc_prop->get_view_number() > old_high_qc_prop->get_view_number() && is_quorum_met(high_qc, _schedule, *new_high_qc_prop)) { // "The caller does not need this updated on their high_qc structure" -- g //high_qc.quorum_met = true; - fc_tlog(_logger, " === updated high qc, now is : #${get_height} ${proposal_id}", ("get_height", new_high_qc_prop->get_height())("proposal_id", new_high_qc_prop->proposal_id)); + fc_tlog(_logger, " === updated high qc, now is : #${view_number} ${proposal_id}", ("view_number", new_high_qc_prop->get_view_number())("proposal_id", new_high_qc_prop->proposal_id)); _high_qc = high_qc; _high_qc.quorum_met = true; _b_leaf = _high_qc.proposal_id; @@ -828,7 +828,7 @@ namespace eosio { namespace hotstuff { } } - if (proposal.get_height() > _v_height) { + if (proposal.get_view_number() > _v_height) { monotony_check = true; } @@ -848,7 +848,7 @@ namespace eosio { namespace hotstuff { const hs_proposal_message *prop_justification = get_proposal( proposal.justify.proposal_id ); EOS_ASSERT( prop_justification != nullptr , chain_exception, "expected hs_proposal ${id} not found", ("id", proposal.justify.proposal_id) ); - if (prop_justification->get_height() > b_lock->get_height()) { + if (prop_justification->get_view_number() > b_lock->get_view_number()) { liveness_check = true; } } @@ -949,7 +949,7 @@ namespace eosio { namespace hotstuff { ("b_lock_phase", b_lock->phase_counter)); } - if (_b_lock == NULL_PROPOSAL_ID || b_1.get_height() > b_lock->get_height()){ + if (_b_lock == NULL_PROPOSAL_ID || b_1.get_view_number() > b_lock->get_view_number()){ fc_tlog(_logger, "setting _b_lock to ${proposal_id}", ("proposal_id",b_1.proposal_id )); _b_lock = b_1.proposal_id; //commit phase on b1 @@ -980,7 +980,7 @@ namespace eosio { namespace hotstuff { const hs_proposal_message *b_exec = get_proposal( _b_exec ); EOS_ASSERT( b_exec != nullptr , chain_exception, "expected hs_proposal ${id} not found", ("id", _b_exec) ); - if (b_exec->get_height() >= b.get_height() && b_exec->proposal_id != b.proposal_id){ + if (b_exec->get_view_number() >= b.get_view_number() && b_exec->proposal_id != b.proposal_id){ fc_elog(_logger, " *** ${id} finality violation detected at height ${block_num}, phase : ${phase}. Proposal ${proposal_id_1} conflicts with ${proposal_id_2}", ("id", _id) @@ -1003,7 +1003,7 @@ namespace eosio { namespace hotstuff { _b_exec = b.proposal_id; //decide phase on b _block_exec = b.block_id; - gc_proposals( b.get_height()-1); + gc_proposals( b.get_key()-1); } else { fc_elog(_logger, " *** ${id} could not verify direct parent relationship", ("id",_id)); @@ -1029,7 +1029,7 @@ namespace eosio { namespace hotstuff { ph_iterator ph_it = _proposal_height.find( p.proposal_id ); EOS_ASSERT( ph_it != _proposal_height.end(), chain_exception, "gc_proposals internal error: no proposal height entry"); uint64_t proposal_height = ph_it->second; - EOS_ASSERT(proposal_height == p.get_height(), chain_exception, "gc_proposals internal error: mismatched proposal height record"); // this check is unnecessary + EOS_ASSERT(proposal_height == p.get_key(), chain_exception, "gc_proposals internal error: mismatched proposal height record"); // this check is unnecessary _proposal_height.erase( ph_it ); ++ps_it; } @@ -1090,7 +1090,7 @@ namespace eosio { namespace hotstuff { if (_b_exec == NULL_PROPOSAL_ID) exec_height_check = true; else - exec_height_check = last_exec_prop->get_height() < proposal.get_height(); + exec_height_check = last_exec_prop->get_view_number() < proposal.get_view_number(); if (exec_height_check){ diff --git a/libraries/hotstuff/test/CMakeLists.txt b/libraries/hotstuff/test/CMakeLists.txt index b147f10496..eda3efb0c5 100644 --- a/libraries/hotstuff/test/CMakeLists.txt +++ b/libraries/hotstuff/test/CMakeLists.txt @@ -1,4 +1,4 @@ -add_executable( test_hotstuff test_hotstuff.cpp test_pacemaker.cpp) +add_executable( test_hotstuff test_hotstuff.cpp test_hotstuff_state.cpp hotstuff_tools.cpp test_pacemaker.cpp) target_link_libraries( test_hotstuff hotstuff fc Boost::unit_test_framework) add_test(NAME test_hotstuff COMMAND test_hotstuff WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) diff --git a/libraries/hotstuff/test/hotstuff_tools.cpp b/libraries/hotstuff/test/hotstuff_tools.cpp new file mode 100644 index 0000000000..bb92b6c558 --- /dev/null +++ b/libraries/hotstuff/test/hotstuff_tools.cpp @@ -0,0 +1,62 @@ +#include +#include +#include + +#include + +#include + +BOOST_AUTO_TEST_CASE(view_number_tests) try { + + eosio::hotstuff::hs_proposal_message hspm_1; + eosio::hotstuff::hs_proposal_message hspm_2; + eosio::hotstuff::hs_proposal_message hspm_3; + eosio::hotstuff::hs_proposal_message hspm_4; + eosio::hotstuff::hs_proposal_message hspm_5; + + hspm_1.block_id = eosio::chain::block_id_type("0b93846ba73bdfdc9b2383863b64f8f921c8a2379d6dde4e05bdd2e434e9392a"); //UX Network block #194217067 + hspm_1.phase_counter = 0; + + hspm_2.block_id = eosio::chain::block_id_type("0b93846ba73bdfdc9b2383863b64f8f921c8a2379d6dde4e05bdd2e434e9392a"); //UX Network block #194217067 + hspm_2.phase_counter = 1; + + hspm_3.block_id = eosio::chain::block_id_type("0b93846cf55a3ecbcd8f9bd86866b1aecc2e8bd981e40c92609ce3a68dbd0824"); //UX Network block #194217068 + hspm_3.phase_counter = 0; + + hspm_4.block_id = eosio::chain::block_id_type("0b93846cf55a3ecbcd8f9bd86866b1aecc2e8bd981e40c92609ce3a68dbd0824"); //UX Network block #194217068 + hspm_4.phase_counter = 1; + + hspm_5.block_id = eosio::chain::block_id_type("0b93846cf55a3ecbcd8f9bd86866b1aecc2e8bd981e40c92609ce3a68dbd0824"); //UX Network block #194217068 + hspm_5.phase_counter = 2; + + eosio::hotstuff::view_number vn_1 = hspm_1.get_view_number(); + eosio::hotstuff::view_number vn_2 = hspm_2.get_view_number(); + eosio::hotstuff::view_number vn_3 = hspm_3.get_view_number(); + eosio::hotstuff::view_number vn_4 = hspm_4.get_view_number(); + eosio::hotstuff::view_number vn_5 = hspm_5.get_view_number(); + + //test getters + + BOOST_CHECK_EQUAL(vn_1.block_height() == 194217067, true); + BOOST_CHECK_EQUAL(vn_1.phase_counter() == 0, true); + + //test operators == true + BOOST_CHECK_EQUAL(vn_1 == vn_1, true); + BOOST_CHECK_EQUAL(vn_1 != vn_2, true); + + BOOST_CHECK_EQUAL(vn_1 < vn_2, true); + BOOST_CHECK_EQUAL(vn_2 < vn_3, true); + BOOST_CHECK_EQUAL(vn_3 < vn_4, true); + BOOST_CHECK_EQUAL(vn_4 < vn_5, true); + + //test operators == false + BOOST_CHECK_EQUAL(vn_1 >= vn_2, false); + BOOST_CHECK_EQUAL(vn_2 > vn_3, false); + + //test constructor + + eosio::hotstuff::view_number vn_6 = eosio::hotstuff::view_number(std::make_pair(194217068, 2)); + + BOOST_CHECK_EQUAL(vn_5 == vn_6, true); + +} FC_LOG_AND_RETHROW(); \ No newline at end of file diff --git a/libraries/hotstuff/test/test_hotstuff.cpp b/libraries/hotstuff/test/test_hotstuff.cpp index cba5789a39..7725d813d4 100644 --- a/libraries/hotstuff/test/test_hotstuff.cpp +++ b/libraries/hotstuff/test/test_hotstuff.cpp @@ -172,7 +172,6 @@ BOOST_AUTO_TEST_CASE(hotstuff_bitset) try { } FC_LOG_AND_RETHROW(); - BOOST_AUTO_TEST_CASE(hotstuff_1) try { //test optimistic responsiveness (3 confirmations per block) diff --git a/libraries/hotstuff/test/test_hotstuff_state.cpp b/libraries/hotstuff/test/test_hotstuff_state.cpp new file mode 100644 index 0000000000..d28a322824 --- /dev/null +++ b/libraries/hotstuff/test/test_hotstuff_state.cpp @@ -0,0 +1,83 @@ +#include +#include + +#include + +#include +#include + +//#include + +using std::cout; + +struct safety_state { + + eosio::chain::view_number v_height; + eosio::chain::view_number b_lock; + +}; + +struct liveness_state { + + eosio::chain::quorum_certificate high_qc; + eosio::chain::view_number b_exec; + +}; + +BOOST_AUTO_TEST_SUITE(test_hotstuff_state) + +const std::string file_path("temp"); + +BOOST_AUTO_TEST_CASE(write_state_to_file) try { + +/* eosio::hotstuff::hs_proposal_message hspm; + + hspm.block_id = eosio::chain::block_id_type("0b93846ba73bdfdc9b2383863b64f8f921c8a2379d6dde4e05bdd2e434e9392a"); //UX Network block #194217067 + hspm.phase_counter = 0; + + eosio::hotstuff::view_number vn = hspm.get_view_number(); + + BOOST_CHECK_EQUAL(vn.block_height() == 194217067, true); + BOOST_CHECK_EQUAL(vn.phase_counter() == 0, true); +*/ + safety_state ss; + + //ss.test_val = 2; + + // writing + fc::cfile pfile; + pfile.set_file_path(file_path); + pfile.open(fc::cfile::truncate_rw_mode); + auto data = fc::raw::pack(ss); + pfile.write(data.data(), data.size()); + pfile.close(); // or let destructor do it + + bool ok = true; + + BOOST_CHECK_EQUAL(ok, true); + + +} FC_LOG_AND_RETHROW(); + +BOOST_AUTO_TEST_CASE(read_state_from_file) try { + + safety_state ss; + + // reading + fc::cfile pfile; + pfile.set_file_path(file_path); + pfile.open("rb"); + auto ds = pfile.create_datastream(); + fc::raw::unpack(ds, ss); + pfile.close(); // or let destructor do it + + //bool ok = ss.test_val == 2; + + BOOST_CHECK_EQUAL(true, true); + +} FC_LOG_AND_RETHROW(); + +BOOST_AUTO_TEST_SUITE_END() + +FC_REFLECT(safety_state, (v_height)(b_lock)) +FC_REFLECT(liveness_state, (high_qc)(b_exec)) \ No newline at end of file diff --git a/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp b/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp index c9a9feb6dd..68cbcd29ba 100644 --- a/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp +++ b/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp @@ -851,7 +851,7 @@ class read_only : public api_base { justify = p.justify; phase_counter = p.phase_counter; block_height = p.block_num(); - view_number = p.get_height(); + view_number = p.get_key(); } hs_complete_proposal_message() = default; // cleos requires this }; @@ -865,7 +865,7 @@ class read_only : public api_base { fc::sha256 b_finality_violation = chain::NULL_PROPOSAL_ID; chain::block_id_type block_exec = chain::NULL_BLOCK_ID; chain::block_id_type pending_proposal_block = chain::NULL_BLOCK_ID; - uint32_t v_height = 0; + chain::view_number v_height; chain::quorum_certificate high_qc; chain::quorum_certificate current_qc; chain::extended_schedule schedule;