From cbc3060d5a9af228d4679bb5a13b8ac32ec352d3 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Sat, 9 Sep 2023 19:17:00 -0400 Subject: [PATCH] More code cleanup and prepare storing `hs_commitment` in block extension --- libraries/chain/controller.cpp | 4 ++ libraries/chain/fork_database.cpp | 2 + .../chain/include/eosio/chain/controller.hpp | 2 + libraries/hotstuff/chain_pacemaker.cpp | 63 ++++++------------- .../eosio/hotstuff/chain_pacemaker.hpp | 8 +-- .../include/eosio/hotstuff/qc_chain.hpp | 12 ++-- libraries/hotstuff/qc_chain.cpp | 39 +++++------- libraries/hotstuff/test/test_pacemaker.cpp | 8 +-- 8 files changed, 56 insertions(+), 82 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 7760ecc01a..e8590b0196 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -2990,6 +2990,10 @@ void controller::commit_block() { my->commit_block(block_status::incomplete); } +void controller::mark_irreversible(block_id_type b) { // called from HotStuff consensus +} + + deque controller::abort_block() { return my->abort_block(); } diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index 794488b25d..16189cf79b 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -573,6 +573,8 @@ namespace eosio { namespace chain { } void fork_database_impl::mark_irreversible_impl( const block_id_type& b ) { + // called by HotStuff consensus to notify `chain` that block b is now irreversible + } block_state_ptr fork_database::get_block(const block_id_type& id)const { diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index 5c16868f97..fe498250e1 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -168,6 +168,8 @@ namespace eosio { namespace chain { block_state_ptr finalize_block( block_report& br, const signer_callback_type& signer_callback ); void sign_block( const signer_callback_type& signer_callback ); void commit_block(); + + void mark_irreversible(block_id_type b); // called from HotStuff consensus // thread-safe std::future create_block_state_future( const block_id_type& id, const signed_block_ptr& b ); diff --git a/libraries/hotstuff/chain_pacemaker.cpp b/libraries/hotstuff/chain_pacemaker.cpp index 6dfaaa25f0..326c9cf4e7 100644 --- a/libraries/hotstuff/chain_pacemaker.cpp +++ b/libraries/hotstuff/chain_pacemaker.cpp @@ -105,7 +105,8 @@ namespace eosio { namespace hotstuff { std::set my_producers, bls_key_map_t finalizer_keys, fc::logger& logger) - : _qc_chain("default"_n, this, std::move(my_producers), std::move(finalizer_keys), logger), + : _chain(chain), + _qc_chain("default"_n, this, std::move(my_producers), std::move(finalizer_keys), logger), _logger(logger) { _accepted_block_connection = chain->accepted_block.connect( [this]( const block_state_ptr& blk ) { @@ -213,6 +214,13 @@ namespace eosio { namespace hotstuff { return n; } + // called from main thread + void chain_pacemaker::on_pre_accepted_block( const signed_block_ptr& blk ) { + std::scoped_lock g( _chain_state_mutex ); + // store _qc_chain.get_last_commitment() into block extension + // is it the right location? What happens if this block does not become final? + } + // called from main thread void chain_pacemaker::on_accepted_block( const block_state_ptr& blk ) { std::scoped_lock g( _chain_state_mutex ); @@ -302,48 +310,15 @@ namespace eosio { namespace hotstuff { // called from net threads void chain_pacemaker::on_hs_msg(const eosio::chain::hs_message &msg) { - std::visit(overloaded{ - [this](const hs_vote_message& m) { on_hs_vote_msg(m); }, - [this](const hs_proposal_message& m) { on_hs_proposal_msg(m); }, - [this](const hs_new_block_message& m) { on_hs_new_block_msg(m); }, - [this](const hs_new_view_message& m) { on_hs_new_view_msg(m); }, - }, msg); - } - - // called from net threads - void chain_pacemaker::on_hs_proposal_msg(const hs_proposal_message& msg) { - csc prof("prop"); - std::lock_guard g( _hotstuff_global_mutex ); - prof.core_in(); - _qc_chain.on_hs_proposal_msg(msg); - prof.core_out(); - } - - // called from net threads - void chain_pacemaker::on_hs_vote_msg(const hs_vote_message& msg) { - csc prof("vote"); - std::lock_guard g( _hotstuff_global_mutex ); - prof.core_in(); - _qc_chain.on_hs_vote_msg(msg); - prof.core_out(); - } - - // called from net threads - void chain_pacemaker::on_hs_new_block_msg(const hs_new_block_message& msg) { - csc prof("nblk"); - std::lock_guard g( _hotstuff_global_mutex ); - prof.core_in(); - _qc_chain.on_hs_new_block_msg(msg); - prof.core_out(); - } - - // called from net threads - void chain_pacemaker::on_hs_new_view_msg(const hs_new_view_message& msg) { - csc prof("view"); - std::lock_guard g( _hotstuff_global_mutex ); - prof.core_in(); - _qc_chain.on_hs_new_view_msg(msg); - prof.core_out(); + auto res = _qc_chain.on_hs_msg(msg); // returns block_id made irrersible (if any) + if (res) { + // todo: problem calling into the main thread from a net_plugin thread + // one solution would be for: + // - `_chain->mark_irreversible` to update an `atomic_shared_ptr` + // - or: Instead of calling into _chain here, we could store the lib block_id into a member variable of chain_pacemaker, + // and call `_chain->mark_irreversible` from on_accepted_block. + // - or: controller::log_irreversible to query `chain_pacemaker` for the lib instead of `fork_head->dpos_irreversible_blocknum` + _chain->mark_irreversible(*res); + } } - }} diff --git a/libraries/hotstuff/include/eosio/hotstuff/chain_pacemaker.hpp b/libraries/hotstuff/include/eosio/hotstuff/chain_pacemaker.hpp index 6b80036bc8..5878485172 100644 --- a/libraries/hotstuff/include/eosio/hotstuff/chain_pacemaker.hpp +++ b/libraries/hotstuff/include/eosio/hotstuff/chain_pacemaker.hpp @@ -48,11 +48,8 @@ namespace eosio::hotstuff { private: void on_accepted_block( const block_state_ptr& blk ); void on_irreversible_block( const block_state_ptr& blk ); - - void on_hs_proposal_msg(const hs_proposal_message& msg); //consensus msg event handler - void on_hs_vote_msg(const hs_vote_message& msg); //confirmation msg event handler - void on_hs_new_view_msg(const hs_new_view_message& msg); //new view msg event handler - void on_hs_new_block_msg(const hs_new_block_message& msg); //new block msg event handler + void on_pre_accepted_block( const signed_block_ptr& blk ); + private: //FIXME/REMOVE: for testing/debugging only @@ -78,6 +75,7 @@ namespace eosio::hotstuff { boost::signals2::scoped_connection _accepted_block_connection; boost::signals2::scoped_connection _irreversible_block_connection; + controller* _chain; qc_chain _qc_chain; std::function bcast_hs_message; diff --git a/libraries/hotstuff/include/eosio/hotstuff/qc_chain.hpp b/libraries/hotstuff/include/eosio/hotstuff/qc_chain.hpp index 2083b977c1..50a0435b84 100644 --- a/libraries/hotstuff/include/eosio/hotstuff/qc_chain.hpp +++ b/libraries/hotstuff/include/eosio/hotstuff/qc_chain.hpp @@ -110,10 +110,7 @@ namespace eosio::hotstuff { void on_beat(); //handler for pacemaker beat() - void on_hs_vote_msg(const hs_vote_message& msg); //vote msg event handler - std::optional on_hs_proposal_msg(const hs_proposal_message& msg); //proposal msg event handler - void on_hs_new_view_msg(const hs_new_view_message& msg); //new view msg event handler - void on_hs_new_block_msg(const hs_new_block_message& msg); //new block msg event handler + std::optional on_hs_msg(const hs_message& msg); private: std::optional process_hs_msg(const hs_message& msg) @@ -122,8 +119,8 @@ namespace eosio::hotstuff { std::visit(overloaded{ [this](const hs_vote_message& m) { process_vote(m); }, [this, &res](const hs_proposal_message& m) { res = process_proposal(m); }, - [this](const hs_new_block_message& m) {}, - [this](const hs_new_view_message& m) {}, + [](const hs_new_block_message& m) {}, + [](const hs_new_view_message& m) {}, }, msg); return res; @@ -178,6 +175,8 @@ namespace eosio::hotstuff { void gc_proposals(uint64_t cutoff); //garbage collection of old proposals + const std::optional& get_last_commitment() const { return _last_commitment; } + #warning remove. bls12-381 key used for testing purposes //todo : remove. bls12-381 key used for testing purposes std::vector _seed = @@ -209,6 +208,7 @@ namespace eosio::hotstuff { std::set _my_producers; chain::bls_key_map_t _my_finalizer_keys; name _id; + std::optional _last_commitment; // thread safety?? mutable std::atomic _state_version = 1; diff --git a/libraries/hotstuff/qc_chain.cpp b/libraries/hotstuff/qc_chain.cpp index 8aded9ec30..e07d3dfaa3 100644 --- a/libraries/hotstuff/qc_chain.cpp +++ b/libraries/hotstuff/qc_chain.cpp @@ -823,25 +823,18 @@ namespace eosio::hotstuff { return final_on_qc_check && monotony_check && (liveness_check || safety_check); } - //on proposal received, called from network thread + //hs_message received, called from network thread //returns highest block_id that was made irreversible (if any) - std::optional qc_chain::on_hs_proposal_msg(const hs_proposal_message& msg) { - return process_proposal(msg); - } - - //on vote received, called from network thread - void qc_chain::on_hs_vote_msg(const hs_vote_message& msg) { - process_vote(msg); - } - - //on new view received, called from network thread - void qc_chain::on_hs_new_view_msg(const hs_new_view_message& msg) { - process_new_view(msg); - } - - //on new block received, called from network thread - void qc_chain::on_hs_new_block_msg(const hs_new_block_message& msg) { - process_new_block(msg); + std::optional qc_chain::on_hs_msg(const hs_message& msg) { + std::optional res; + std::visit(overloaded{ + [this](const hs_vote_message& m) { process_vote(m); }, + [this, &res](const hs_proposal_message& m) { res = process_proposal(m); }, + [this](const hs_new_block_message& m) { process_new_block(m); }, + [this](const hs_new_view_message& m) { process_new_view(m); }, + }, + msg); + return res; } std::optional qc_chain::update(const hs_proposal_message& proposal) { @@ -1038,15 +1031,15 @@ void qc_chain::commit(const hs_commitment& commitment) { } if (!proposal_chain.empty()) { - - // update latest known commitment in controller - // controller->update_hs_commitment(commitment); + _last_commitment = commitment; // will be provided to controller by pacemaker in `on_irreversible_block` // commit all ancestor blocks sequentially first (hence the reverse) for (auto p : boost::adaptors::reverse(proposal_chain)) { - // Execute commands [...] // issue #1522: HotStuff finality should drive LIB in controller - // ??? controller->commit_block(controller::block_status::irreversible, p.block_id); + // no need to do anything here. `qc_chain::update()` will return the `block_id_type` made final + // (which is b.block_id) to `chain_pacemaker`, which will notify the controller. The controller + // will notify `fork_database` so that this block and its ancestors are marked irreversible and + // moved out of `fork_database`. ; } diff --git a/libraries/hotstuff/test/test_pacemaker.cpp b/libraries/hotstuff/test/test_pacemaker.cpp index 3fb293c4c5..4cf1489489 100644 --- a/libraries/hotstuff/test/test_pacemaker.cpp +++ b/libraries/hotstuff/test/test_pacemaker.cpp @@ -168,7 +168,7 @@ namespace eosio::hotstuff { 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) ) - qcc_ptr->on_hs_proposal_msg(msg); + qcc_ptr->on_hs_msg(msg); qc_itr++; } } @@ -179,7 +179,7 @@ namespace eosio::hotstuff { 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) ) - qcc_ptr->on_hs_vote_msg(msg); + qcc_ptr->on_hs_msg(msg); qc_itr++; } } @@ -190,7 +190,7 @@ namespace eosio::hotstuff { 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) ) - qcc_ptr->on_hs_new_block_msg(msg); + qcc_ptr->on_hs_msg(msg); qc_itr++; } } @@ -201,7 +201,7 @@ namespace eosio::hotstuff { 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) ) - qcc_ptr->on_hs_new_view_msg(msg); + qcc_ptr->on_hs_msg(msg); qc_itr++; } }