From 7fc69ac30feb34af28e0554eb419180474492222 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 10 Apr 2024 18:57:13 -0500 Subject: [PATCH 01/40] GH-3 Move vote processing off net threads into a dedicated thread pool --- libraries/chain/controller.cpp | 33 ++- .../eosio/chain/block_header_state.hpp | 2 +- .../chain/include/eosio/chain/controller.hpp | 8 +- .../include/eosio/chain/hotstuff/hotstuff.hpp | 14 +- .../chain/unapplied_transaction_queue.hpp | 1 - .../include/eosio/chain/vote_processor.hpp | 216 ++++++++++++++++++ .../libfc/include/fc/crypto/bls_signature.hpp | 7 + libraries/testing/tester.cpp | 2 +- plugins/net_plugin/net_plugin.cpp | 70 +++--- unittests/finality_test_cluster.cpp | 47 +++- unittests/finality_test_cluster.hpp | 6 + unittests/finality_tests.cpp | 3 +- 12 files changed, 338 insertions(+), 71 deletions(-) create mode 100644 libraries/chain/include/eosio/chain/vote_processor.hpp diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 613a7fb16b..23f60eef3d 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include @@ -946,7 +947,9 @@ struct controller_impl { signal accepted_block; signal irreversible_block; signal)> applied_transaction; - signal voted_block; + signal voted_block; + + vote_processor_t vote_processor{fork_db, voted_block}; int64_t set_proposed_producers( vector producers ); int64_t set_proposed_producers_legacy( vector producers ); @@ -1195,6 +1198,7 @@ struct controller_impl { elog( "Exception in chain thread pool, exiting: ${e}", ("e", e.to_detail_string()) ); if( shutdown ) shutdown(); } ); + vote_processor.start(4); set_activation_handler(); set_activation_handler(); @@ -1214,6 +1218,7 @@ struct controller_impl { irreversible_block.connect([this](const block_signal_params& t) { const auto& [ block, id] = t; wasmif.current_lib(block->block_num()); + vote_processor.notify_lib(block->block_num()); }); @@ -3552,19 +3557,8 @@ struct controller_impl { // called from net threads and controller's thread pool - vote_status process_vote_message( const vote_message& vote ) { - // only aggregate votes on proper if blocks - auto aggregate_vote = [&vote](auto& forkdb) -> vote_status { - auto bsp = forkdb.get_block(vote.block_id); - if (bsp && bsp->block->is_proper_svnn_block()) { - return bsp->aggregate_vote(vote); - } - return vote_status::unknown_block; - }; - auto aggregate_vote_legacy = [](auto&) -> vote_status { - return vote_status::unknown_block; - }; - return fork_db.apply(aggregate_vote_legacy, aggregate_vote); + void process_vote_message( uint32_t connection_id, const vote_message& vote ) { + vote_processor.process_vote_message(connection_id, vote); } bool node_has_voted_if_finalizer(const block_id_type& id) const { @@ -3593,11 +3587,10 @@ struct controller_impl { my_finalizers.maybe_vote( *bsp->active_finalizer_policy, bsp, bsp->strong_digest, [&](const vote_message& vote) { // net plugin subscribed to this signal. it will broadcast the vote message on receiving the signal - emit(voted_block, vote); + emit(voted_block, std::tuple{uint32_t{0}, vote_status::success, std::cref(vote)}); // also aggregate our own vote into the pending_qc for this block. - boost::asio::post(thread_pool.get_executor(), - [control = this, vote]() { control->process_vote_message(vote); }); + process_vote_message(0, vote); }); } @@ -5254,8 +5247,8 @@ void controller::set_proposed_finalizers( finalizer_policy&& fin_pol ) { } // called from net threads -vote_status controller::process_vote_message( const vote_message& vote ) { - return my->process_vote_message( vote ); +void controller::process_vote_message( uint32_t connection_id, const vote_message& vote ) { + my->process_vote_message( connection_id, vote ); }; bool controller::node_has_voted_if_finalizer(const block_id_type& id) const { @@ -5538,7 +5531,7 @@ signal& controller::accepted_block_header() { signal& controller::accepted_block() { return my->accepted_block; } signal& controller::irreversible_block() { return my->irreversible_block; } signal)>& controller::applied_transaction() { return my->applied_transaction; } -signal& controller::voted_block() { return my->voted_block; } +signal& controller::voted_block() { return my->voted_block; } chain_id_type controller::extract_chain_id(snapshot_reader& snapshot) { chain_snapshot_header header; diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index 02291c3547..0e9566dd3e 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -78,7 +78,7 @@ struct block_header_state { digest_type compute_finality_digest() const; // Returns true if the block is a Proper Savanna Block - bool is_proper_svnn_block() const; + bool is_proper_svnn_block() const { return header.is_proper_svnn_block(); } // block descending from this need the provided qc in the block extension bool is_needed(const qc_claim_t& qc_claim) const { diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index d2ac81dc32..14433df84d 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -58,6 +58,7 @@ namespace eosio::chain { using trx_meta_cache_lookup = std::function; using block_signal_params = std::tuple; + using vote_signal_params = std::tuple; enum class db_read_mode { HEAD, @@ -326,7 +327,7 @@ namespace eosio::chain { // called by host function set_finalizers void set_proposed_finalizers( finalizer_policy&& fin_pol ); // called from net threads - vote_status process_vote_message( const vote_message& msg ); + void process_vote_message( uint32_t connection_id, const vote_message& msg ); // thread safe, for testing bool node_has_voted_if_finalizer(const block_id_type& id) const; @@ -373,9 +374,8 @@ namespace eosio::chain { signal& accepted_block(); signal& irreversible_block(); signal)>& applied_transaction(); - - // Unlike other signals, voted_block can be signaled from other threads than the main thread. - signal& voted_block(); + // Unlike other signals, voted_block is signaled from other threads than the main thread. + signal& voted_block(); const apply_handler* find_apply_handler( account_name contract, scope_name scope, action_name act )const; wasm_interface& get_wasm_interface(); diff --git a/libraries/chain/include/eosio/chain/hotstuff/hotstuff.hpp b/libraries/chain/include/eosio/chain/hotstuff/hotstuff.hpp index b54f8d7416..f6e2d4297f 100644 --- a/libraries/chain/include/eosio/chain/hotstuff/hotstuff.hpp +++ b/libraries/chain/include/eosio/chain/hotstuff/hotstuff.hpp @@ -25,14 +25,18 @@ namespace eosio::chain { bool strong{false}; bls_public_key finalizer_key; bls_signature sig; + + auto operator<=>(const vote_message&) const = default; + bool operator==(const vote_message&) const = default; }; enum class vote_status { success, - duplicate, - unknown_public_key, - invalid_signature, - unknown_block + duplicate, // duplicate vote, expected as votes arrive on multiple connections + unknown_public_key, // public key is invalid, indicates invalid vote + invalid_signature, // signature is invalid, indicates invalid vote + unknown_block, // block not available, possibly less than LIB, or too far in the future + max_exceeded // received too many votes for a connection }; using bls_public_key = fc::crypto::blslib::bls_public_key; @@ -159,7 +163,7 @@ namespace eosio::chain { FC_REFLECT(eosio::chain::vote_message, (block_id)(strong)(finalizer_key)(sig)); -FC_REFLECT_ENUM(eosio::chain::vote_status, (success)(duplicate)(unknown_public_key)(invalid_signature)(unknown_block)) +FC_REFLECT_ENUM(eosio::chain::vote_status, (success)(duplicate)(unknown_public_key)(invalid_signature)(unknown_block)(max_exceeded)) FC_REFLECT(eosio::chain::valid_quorum_certificate, (_strong_votes)(_weak_votes)(_sig)); FC_REFLECT(eosio::chain::pending_quorum_certificate, (_valid_qc)(_quorum)(_max_weak_sum_before_weak_final)(_state)(_strong_sum)(_weak_sum)(_weak_votes)(_strong_votes)); FC_REFLECT_ENUM(eosio::chain::pending_quorum_certificate::state_t, (unrestricted)(restricted)(weak_achieved)(weak_final)(strong)); diff --git a/libraries/chain/include/eosio/chain/unapplied_transaction_queue.hpp b/libraries/chain/include/eosio/chain/unapplied_transaction_queue.hpp index e1231bedcb..7c73856773 100644 --- a/libraries/chain/include/eosio/chain/unapplied_transaction_queue.hpp +++ b/libraries/chain/include/eosio/chain/unapplied_transaction_queue.hpp @@ -2,7 +2,6 @@ #include #include -#include #include #include diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp new file mode 100644 index 0000000000..a2492fbc38 --- /dev/null +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -0,0 +1,216 @@ +#pragma once + +#include + +#include +#include +#include +#include +#include + +namespace eosio { namespace chain { + +/** + * Process votes in a dedicated thread pool. + */ +class vote_processor_t { + static constexpr size_t max_votes_per_connection = 2500; // 3000 is less than 1MB per connection + static constexpr std::chrono::milliseconds block_wait_time{10}; + + struct by_block_num; + struct by_connection; + struct by_vote; + + struct vote { + uint32_t connection_id; + vote_message msg; + + const block_id_type& id() const { return msg.block_id; } + block_num_type block_num() const { return block_header::num_from_id(msg.block_id); } + }; + + using vote_ptr = std::shared_ptr; + using vote_signal_type = decltype(controller({},chain_id_type::empty_chain_id()).voted_block()); + + typedef multi_index_container< vote_ptr, + indexed_by< + ordered_non_unique, + composite_key, + const_mem_fun + >, composite_key_compare< std::greater<>, sha256_less > // greater for block_num + >, + ordered_non_unique< tag, member >, + ordered_unique< tag, member > + > + > vote_index_type; + + fork_database& fork_db; + std::mutex mtx; + std::condition_variable cv; + vote_index_type index; + // connection, count of messages + std::map num_messages; + std::atomic lib{0}; + std::atomic stopped{false}; + vote_signal_type& vote_signal; + named_thread_pool thread_pool; + +private: + template + void emit( const Signal& s, Arg&& a ) { + try { + s(std::forward(a)); + } catch (std::bad_alloc& e) { + wlog( "std::bad_alloc: ${w}", ("w", e.what()) ); + throw e; + } catch (boost::interprocess::bad_alloc& e) { + wlog( "boost::interprocess::bad alloc: ${w}", ("w", e.what()) ); + throw e; + } catch ( controller_emit_signal_exception& e ) { + wlog( "controller_emit_signal_exception: ${details}", ("details", e.to_detail_string()) ); + throw e; + } catch ( fc::exception& e ) { + wlog( "fc::exception: ${details}", ("details", e.to_detail_string()) ); + } catch ( std::exception& e ) { + wlog( "std::exception: ${details}", ("details", e.what()) ); + } catch ( ... ) { + wlog( "signal handler threw exception" ); + } + } + + void emit(uint32_t connection_id, vote_status status, const vote_message& msg) { + if (connection_id != 0) { // this nodes vote was already signaled + emit( vote_signal, std::tuple{connection_id, status, std::cref(msg)} ); + } + } + + void remove_connection(uint32_t connection_id) { + auto& idx = index.get(); + idx.erase(idx.lower_bound(connection_id), idx.upper_bound(connection_id)); + } + + void remove_before_lib() { + auto& idx = index.get(); + idx.erase(idx.lower_bound(lib.load()), idx.end()); // descending + } + + bool remove_all_for_block(auto& idx, auto& it, const block_id_type& id) { + while (it != idx.end() && (*it)->id() == id) { + it = idx.erase(it); + } + return it == idx.end(); + } + + bool skip_all_for_block(auto& idx, auto& it, const block_id_type& id) { + while (it != idx.end() && (*it)->id() == id) { + ++it; + } + return it == idx.end(); + } + +public: + explicit vote_processor_t(fork_database& forkdb, vote_signal_type& vote_signal) + : fork_db(forkdb) + , vote_signal(vote_signal) {} + + ~vote_processor_t() { + stopped = true; + std::lock_guard g(mtx); + cv.notify_one(); + } + + void start(size_t num_threads) { + assert(num_threads > 1); // need at least two as one is used for coordinatation + thread_pool.start( num_threads, []( const fc::exception& e ) { + elog( "Exception in vote processor thread pool, exiting: ${e}", ("e", e.to_detail_string()) ); + } ); + + // one coordinator thread + boost::asio::post(thread_pool.get_executor(), [&]() { + block_id_type not_in_forkdb_id{}; + while (!stopped) { + std::unique_lock g(mtx); + cv.wait(g, [&]() { + if (!index.empty() || stopped) + return true; + return false; + }); + if (stopped) + break; + remove_before_lib(); + if (index.empty()) + continue; + auto& idx = index.get(); + if (auto i = idx.begin(); i != idx.end() && not_in_forkdb_id == (*i)->id()) { // same block as last while loop + g.unlock(); + std::this_thread::sleep_for(block_wait_time); + g.lock(); + } + for (auto i = idx.begin(); i != idx.end();) { + auto& vt = *i; + block_state_ptr bsp = fork_db.apply_s([&](const auto& forkdb) { + return forkdb.get_block(vt->id()); + }); + if (bsp) { + if (!bsp->is_proper_svnn_block()) { + if (remove_all_for_block(idx, i, bsp->id())) + break; + continue; + } + auto iter_of_bsp = i; + std::vector to_process; + to_process.reserve(std::min(21u, idx.size())); // increase if we increase # of finalizers from 21 + for(; i != idx.end() && bsp->id() == (*i)->id(); ++i) { + // although it is the highest contention on block state pending mutex posting all of the same bsp, + // the highest priority is processing votes for this block state. + to_process.push_back(*i); + } + bool should_break = remove_all_for_block(idx, iter_of_bsp, bsp->id()); + g.unlock(); // do not hold lock when posting + for (auto& vptr : to_process) { + boost::asio::post(thread_pool.get_executor(), [this, bsp, vptr=std::move(vptr)]() { + vote_status s = bsp->aggregate_vote(vptr->msg); + emit(vptr->connection_id, s, vptr->msg); + }); + } + if (should_break) + break; + } else { + not_in_forkdb_id = vt->id(); + if (skip_all_for_block(idx, i, (*i)->id())) + break; + } + } + } + dlog("Exiting vote processor coordinator thread"); + }); + } + + void notify_lib(block_num_type block_num) { + lib = block_num; + } + + void process_vote_message(uint32_t connection_id, const vote_message& msg) { + vote_ptr vptr = std::make_shared(vote{.connection_id = connection_id, .msg = msg}); + boost::asio::post(thread_pool.get_executor(), [this, connection_id, msg] { + std::unique_lock g(mtx); + if (++num_messages[connection_id] > max_votes_per_connection) { + // consider the connection invalid, remove all votes + remove_connection(connection_id); + g.unlock(); + + elog("Exceeded max votes per connection for ${c}", ("c", connection_id)); + emit(connection_id, vote_status::max_exceeded, msg); + } else if (block_header::num_from_id(msg.block_id) < lib.load(std::memory_order_relaxed)) { + // ignore + } else { + index.insert(std::make_shared(vote{.connection_id = connection_id, .msg = msg})); + cv.notify_one(); + } + }); + } + +}; + +} } //eosio::chain diff --git a/libraries/libfc/include/fc/crypto/bls_signature.hpp b/libraries/libfc/include/fc/crypto/bls_signature.hpp index d8c2191d4e..ebf0390f1e 100644 --- a/libraries/libfc/include/fc/crypto/bls_signature.hpp +++ b/libraries/libfc/include/fc/crypto/bls_signature.hpp @@ -44,6 +44,13 @@ namespace fc::crypto::blslib { return _jacobian_montgomery_le.equal(sig._jacobian_montgomery_le); } + auto operator<=>(const bls_signature& rhs) const { + return _affine_non_montgomery_le <=> rhs._affine_non_montgomery_le; + } + auto operator==(const bls_signature& rhs) const { + return _affine_non_montgomery_le == rhs._affine_non_montgomery_le; + } + template friend T& operator<<(T& ds, const bls_signature& sig) { // Serialization as variable length array when it is stored as a fixed length array. This makes for easier deserialization by external tools diff --git a/libraries/testing/tester.cpp b/libraries/testing/tester.cpp index 719851a767..e9ccf4efa4 100644 --- a/libraries/testing/tester.cpp +++ b/libraries/testing/tester.cpp @@ -495,7 +495,7 @@ namespace eosio { namespace testing { // wait for this node's vote to be processed size_t retrys = 200; while (!c.node_has_voted_if_finalizer(c.head_block_id()) && --retrys) { - std::this_thread::sleep_for(std::chrono::milliseconds(1)); + std::this_thread::sleep_for(std::chrono::milliseconds(10)); } FC_ASSERT(retrys, "Never saw this nodes vote processed before timeout"); } diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 9d4c0f5b16..4bdf231f61 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -305,7 +305,7 @@ namespace eosio { bool have_txn( const transaction_id_type& tid ) const; void expire_txns(); - void bcast_vote_msg( const std::optional& exclude_peer, send_buffer_type msg ); + void bcast_vote_msg( uint32_t exclude_peer, send_buffer_type msg ); void add_unlinkable_block( signed_block_ptr b, const block_id_type& id ) { std::optional rm_blk_id = unlinkable_block_cache.add_unlinkable_block(std::move(b), id); @@ -529,12 +529,12 @@ namespace eosio { void on_accepted_block_header( const signed_block_ptr& block, const block_id_type& id ); void on_accepted_block(); - void on_voted_block ( const vote_message& vote ); + void on_voted_block ( uint32_t connection_id, vote_status stauts, const vote_message& vote ); void transaction_ack(const std::pair&); void on_irreversible_block( const block_id_type& id, uint32_t block_num ); - void bcast_vote_message( const std::optional& exclude_peer, const chain::vote_message& msg ); + void bcast_vote_message( uint32_t exclude_peer, const chain::vote_message& msg ); void warn_message( uint32_t sender_peer, const chain::hs_message_warning& code ); void start_conn_timer(boost::asio::steady_timer::duration du, std::weak_ptr from_connection); @@ -2666,10 +2666,10 @@ namespace eosio { } ); } - void dispatch_manager::bcast_vote_msg( const std::optional& exclude_peer, send_buffer_type msg ) { + void dispatch_manager::bcast_vote_msg( uint32_t exclude_peer, send_buffer_type msg ) { my_impl->connections.for_each_block_connection( [exclude_peer, msg{std::move(msg)}]( auto& cp ) { if( !cp->current() ) return true; - if( exclude_peer.has_value() && cp->connection_id == exclude_peer.value() ) return true; + if( cp->connection_id == exclude_peer ) return true; cp->strand.post( [cp, msg]() { if (cp->protocol_version >= proto_instant_finality) { peer_dlog(cp, "sending vote msg"); @@ -3713,24 +3713,7 @@ namespace eosio { ("bn", block_header::num_from_id(msg.block_id))("id", msg.block_id.str().substr(8,16)) ("v", msg.strong ? "strong" : "weak")("k", msg.finalizer_key.to_string().substr(8, 16))); controller& cc = my_impl->chain_plug->chain(); - - switch( cc.process_vote_message(msg) ) { - case vote_status::success: - my_impl->bcast_vote_message(connection_id, msg); - break; - case vote_status::unknown_public_key: - case vote_status::invalid_signature: // close peer immediately - close( false ); // do not reconnect after closing - break; - case vote_status::unknown_block: // track the failure - peer_dlog(this, "vote unknown block #${bn}:${id}..", ("bn", block_header::num_from_id(msg.block_id))("id", msg.block_id.str().substr(8,16))); - block_status_monitor_.rejected(); - break; - case vote_status::duplicate: // do nothing - break; - default: - assert(false); // should never happen - } + cc.process_vote_message(connection_id, msg); } size_t calc_trx_size( const packed_transaction_ptr& trx ) { @@ -3996,14 +3979,41 @@ namespace eosio { } // called from other threads including net threads - void net_plugin_impl::on_voted_block(const vote_message& msg) { - fc_dlog(logger, "on voted signal: block #${bn} ${id}.., ${t}, key ${k}..", - ("bn", block_header::num_from_id(msg.block_id))("id", msg.block_id.str().substr(8,16)) + void net_plugin_impl::on_voted_block(uint32_t connection_id, vote_status status, const vote_message& msg) { + fc_dlog(logger, "connection - ${c} on voted signal: ${s} block #${bn} ${id}.., ${t}, key ${k}..", + ("c", connection_id)("s", status)("bn", block_header::num_from_id(msg.block_id))("id", msg.block_id.str().substr(8,16)) ("t", msg.strong ? "strong" : "weak")("k", msg.finalizer_key.to_string().substr(8, 16))); - bcast_vote_message(std::nullopt, msg); + + switch( status ) { + case vote_status::success: + bcast_vote_message(connection_id, msg); + break; + case vote_status::unknown_public_key: + case vote_status::invalid_signature: + case vote_status::max_exceeded: // close peer immediately + my_impl->connections.for_each_connection([connection_id](const connection_ptr& c) { + if (c->connection_id == connection_id) { + c->close( false ); + } + }); + break; + case vote_status::unknown_block: // track the failure + fc_dlog(logger, "connection - ${c} vote unknown block #${bn}:${id}..", + ("c", connection_id)("bn", block_header::num_from_id(msg.block_id))("id", msg.block_id.str().substr(8,16))); + my_impl->connections.for_each_connection([connection_id](const connection_ptr& c) { + if (c->connection_id == connection_id) { + c->block_status_monitor_.rejected(); + } + }); + break; + case vote_status::duplicate: // do nothing + break; + default: + assert(false); // should never happen + } } - void net_plugin_impl::bcast_vote_message( const std::optional& exclude_peer, const chain::vote_message& msg ) { + void net_plugin_impl::bcast_vote_message( uint32_t exclude_peer, const chain::vote_message& msg ) { buffer_factory buff_factory; auto send_buffer = buff_factory.get_send_buffer( msg ); @@ -4420,8 +4430,8 @@ namespace eosio { my->on_irreversible_block( id, block->block_num() ); } ); - cc.voted_block().connect( [my = shared_from_this()]( const vote_message& vote ) { - my->on_voted_block(vote); + cc.voted_block().connect( [my = shared_from_this()]( const vote_signal_params& vote_signal ) { + my->on_voted_block(std::get<0>(vote_signal), std::get<1>(vote_signal), std::get<2>(vote_signal)); } ); } diff --git a/unittests/finality_test_cluster.cpp b/unittests/finality_test_cluster.cpp index 0ea13c13ed..e84340cb44 100644 --- a/unittests/finality_test_cluster.cpp +++ b/unittests/finality_test_cluster.cpp @@ -10,13 +10,20 @@ finality_test_cluster::finality_test_cluster() { produce_and_push_block(); // make setfinalizer irreversible + // node0's votes + node0.node.control->voted_block().connect( [&]( const eosio::chain::vote_signal_params& v ) { + last_vote_status = std::get<1>(v); + last_connection_vote = std::get<0>(v); + }); // collect node1's votes - node1.node.control->voted_block().connect( [&]( const eosio::chain::vote_message& vote ) { - node1.votes.emplace_back(vote); + node1.node.control->voted_block().connect( [&]( const eosio::chain::vote_signal_params& v ) { + std::lock_guard g(node1.votes_mtx); + node1.votes.emplace_back(std::get<2>(v)); }); // collect node2's votes - node2.node.control->voted_block().connect( [&]( const eosio::chain::vote_message& vote ) { - node2.votes.emplace_back(vote); + node2.node.control->voted_block().connect( [&]( const eosio::chain::vote_signal_params& v ) { + std::lock_guard g(node2.votes_mtx); + node2.votes.emplace_back(std::get<2>(v)); }); // form a 3-chain to make LIB advacing on node0 @@ -35,11 +42,24 @@ finality_test_cluster::finality_test_cluster() { // clean up processed votes for (auto& n : nodes) { + std::lock_guard g(n.votes_mtx); n.votes.clear(); n.prev_lib_num = n.node.control->if_irreversible_block_num(); } } +eosio::chain::vote_status finality_test_cluster::wait_on_vote(uint32_t connection_id) { + // wait for this node's vote to be processed + size_t retrys = 200; + while ( (last_connection_vote != connection_id) && --retrys) { + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + } + if (last_connection_vote != connection_id) { + FC_ASSERT(false, "Never received vote"); + } + return last_vote_status; +} + // node0 produces a block and pushes it to node1 and node2 void finality_test_cluster::produce_and_push_block() { auto b = node0.node.produce_block(); @@ -87,8 +107,11 @@ bool finality_test_cluster::node2_lib_advancing() { // node1_votes and node2_votes when starting. bool finality_test_cluster::produce_blocks_and_verify_lib_advancing() { // start from fresh - node1.votes.clear(); - node2.votes.clear(); + { + std::scoped_lock g(node1.votes_mtx, node2.votes_mtx); + node1.votes.clear(); + node2.votes.clear(); + } for (auto i = 0; i < 3; ++i) { produce_and_push_block(); @@ -103,6 +126,7 @@ bool finality_test_cluster::produce_blocks_and_verify_lib_advancing() { } void finality_test_cluster::node1_corrupt_vote_proposal_id() { + std::lock_guard g(node1.votes_mtx); node1_orig_vote = node1.votes[0]; if( node1.votes[0].block_id.data()[0] == 'a' ) { @@ -113,6 +137,7 @@ void finality_test_cluster::node1_corrupt_vote_proposal_id() { } void finality_test_cluster::node1_corrupt_vote_finalizer_key() { + std::lock_guard g(node1.votes_mtx); node1_orig_vote = node1.votes[0]; // corrupt the finalizer_key (manipulate so it is different) @@ -123,6 +148,7 @@ void finality_test_cluster::node1_corrupt_vote_finalizer_key() { } void finality_test_cluster::node1_corrupt_vote_signature() { + std::lock_guard g(node1.votes_mtx); node1_orig_vote = node1.votes[0]; // corrupt the signature @@ -133,6 +159,7 @@ void finality_test_cluster::node1_corrupt_vote_signature() { } void finality_test_cluster::node1_restore_to_original_vote() { + std::lock_guard g(node1.votes_mtx); node1.votes[0] = node1_orig_vote; } @@ -177,6 +204,7 @@ void finality_test_cluster::setup_node(node_info& node, eosio::chain::account_na // send a vote to node0 eosio::chain::vote_status finality_test_cluster::process_vote(node_info& node, size_t vote_index, vote_mode mode) { + std::unique_lock g(node.votes_mtx); FC_ASSERT( vote_index < node.votes.size(), "out of bound index in process_vote" ); auto& vote = node.votes[vote_index]; if( mode == vote_mode::strong ) { @@ -189,8 +217,13 @@ eosio::chain::vote_status finality_test_cluster::process_vote(node_info& node, s // convert the strong digest to weak and sign it vote.sig = node.priv_key.sign(eosio::chain::create_weak_digest(strong_digest)); } + g.unlock(); - return node0.node.control->process_vote_message( vote ); + static uint32_t connection_id = 0; + node0.node.control->process_vote_message( ++connection_id, vote ); + if (eosio::chain::block_header::num_from_id(vote.block_id) > node0.node.control->last_irreversible_block_num()) + return wait_on_vote(connection_id); + return eosio::chain::vote_status::unknown_block; } eosio::chain::vote_status finality_test_cluster::process_vote(node_info& node, vote_mode mode) { diff --git a/unittests/finality_test_cluster.hpp b/unittests/finality_test_cluster.hpp index 97ab1aa4f0..a84b86bb46 100644 --- a/unittests/finality_test_cluster.hpp +++ b/unittests/finality_test_cluster.hpp @@ -78,10 +78,14 @@ class finality_test_cluster { struct node_info { eosio::testing::tester node; uint32_t prev_lib_num{0}; + std::mutex votes_mtx; std::vector votes; fc::crypto::blslib::bls_private_key priv_key; }; + std::atomic last_connection_vote{0}; + std::atomic last_vote_status{}; + std::array nodes; node_info& node0 = nodes[0]; node_info& node1 = nodes[1]; @@ -100,4 +104,6 @@ class finality_test_cluster { // send the latest vote on "node_index" node to node0 eosio::chain::vote_status process_vote(node_info& node, vote_mode mode); + + eosio::chain::vote_status wait_on_vote(uint32_t connection_id); }; diff --git a/unittests/finality_tests.cpp b/unittests/finality_tests.cpp index 6d68774fe5..311d27b33b 100644 --- a/unittests/finality_tests.cpp +++ b/unittests/finality_tests.cpp @@ -502,8 +502,7 @@ BOOST_AUTO_TEST_CASE(unknown_proposal_votes) { try { cluster.node1_corrupt_vote_proposal_id(); // process the corrupted vote - cluster.process_node1_vote(0); - BOOST_REQUIRE(cluster.process_node1_vote(0) == eosio::chain::vote_status::unknown_block); + BOOST_REQUIRE_THROW(cluster.process_node1_vote(0), fc::exception); // throws because it times out waiting on vote cluster.produce_and_push_block(); BOOST_REQUIRE(cluster.node2_lib_advancing()); From c3a9c95934df742d511b4d3ebec278bec3970939 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 10 Apr 2024 19:18:30 -0500 Subject: [PATCH 02/40] GH-3 Track num_messages per connection --- libraries/chain/include/eosio/chain/vote_processor.hpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index a2492fbc38..288bc155e9 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -93,10 +93,12 @@ class vote_processor_t { void remove_before_lib() { auto& idx = index.get(); idx.erase(idx.lower_bound(lib.load()), idx.end()); // descending + // don't decrement num_messages as too many before lib should be considered an error } bool remove_all_for_block(auto& idx, auto& it, const block_id_type& id) { while (it != idx.end() && (*it)->id() == id) { + --num_messages[(*it)->connection_id]; it = idx.erase(it); } return it == idx.end(); @@ -139,8 +141,10 @@ class vote_processor_t { if (stopped) break; remove_before_lib(); - if (index.empty()) + if (index.empty()) { + num_messages.clear(); continue; + } auto& idx = index.get(); if (auto i = idx.begin(); i != idx.end() && not_in_forkdb_id == (*i)->id()) { // same block as last while loop g.unlock(); @@ -196,7 +200,8 @@ class vote_processor_t { boost::asio::post(thread_pool.get_executor(), [this, connection_id, msg] { std::unique_lock g(mtx); if (++num_messages[connection_id] > max_votes_per_connection) { - // consider the connection invalid, remove all votes + // consider the connection invalid, remove all votes of connection + // don't clear num_messages[connection_id] so we keep reporting max_exceeded until index is drained remove_connection(connection_id); g.unlock(); From 6b0c4c405ed7003fbd0ee31cfc31b56cb4137bff Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 10 Apr 2024 19:47:31 -0500 Subject: [PATCH 03/40] GH-3 Make vote processor thread pool size configurable including disabling vote processing --- libraries/chain/controller.cpp | 10 +++++++--- libraries/chain/include/eosio/chain/config.hpp | 1 + .../chain/include/eosio/chain/controller.hpp | 3 ++- plugins/chain_plugin/chain_plugin.cpp | 16 +++++++++++++--- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 23f60eef3d..7cf326008e 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1194,11 +1194,13 @@ struct controller_impl { my_finalizers(fc::time_point::now(), cfg.finalizers_dir / "safety.dat"), wasmif( conf.wasm_runtime, conf.eosvmoc_tierup, db, conf.state_dir, conf.eosvmoc_config, !conf.profile_accounts.empty() ) { - thread_pool.start( cfg.thread_pool_size, [this]( const fc::exception& e ) { + thread_pool.start( cfg.chain_thread_pool_size, [this]( const fc::exception& e ) { elog( "Exception in chain thread pool, exiting: ${e}", ("e", e.to_detail_string()) ); if( shutdown ) shutdown(); } ); - vote_processor.start(4); + if (cfg.vote_thread_pool_size > 0) { + vote_processor.start(cfg.vote_thread_pool_size); + } set_activation_handler(); set_activation_handler(); @@ -3558,7 +3560,9 @@ struct controller_impl { // called from net threads and controller's thread pool void process_vote_message( uint32_t connection_id, const vote_message& vote ) { - vote_processor.process_vote_message(connection_id, vote); + if (conf.vote_thread_pool_size > 0) { + vote_processor.process_vote_message(connection_id, vote); + } } bool node_has_voted_if_finalizer(const block_id_type& id) const { diff --git a/libraries/chain/include/eosio/chain/config.hpp b/libraries/chain/include/eosio/chain/config.hpp index 9dd10a1b85..74af7b59ca 100644 --- a/libraries/chain/include/eosio/chain/config.hpp +++ b/libraries/chain/include/eosio/chain/config.hpp @@ -80,6 +80,7 @@ const static uint16_t default_max_auth_depth = 6; const static uint32_t default_sig_cpu_bill_pct = 50 * percent_1; // billable percentage of signature recovery const static uint32_t default_produce_block_offset_ms = 450; const static uint16_t default_controller_thread_pool_size = 2; +const static uint16_t default_vote_thread_pool_size = 4; const static uint32_t default_max_variable_signature_length = 16384u; const static uint32_t default_max_action_return_value_size = 256; diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index 14433df84d..e4d4285b08 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -88,7 +88,8 @@ namespace eosio::chain { uint64_t state_size = chain::config::default_state_size; uint64_t state_guard_size = chain::config::default_state_guard_size; uint32_t sig_cpu_bill_pct = chain::config::default_sig_cpu_bill_pct; - uint16_t thread_pool_size = chain::config::default_controller_thread_pool_size; + uint16_t chain_thread_pool_size = chain::config::default_controller_thread_pool_size; + uint16_t vote_thread_pool_size = chain::config::default_vote_thread_pool_size; bool read_only = false; bool force_all_checks = false; bool disable_replay_opts = false; diff --git a/plugins/chain_plugin/chain_plugin.cpp b/plugins/chain_plugin/chain_plugin.cpp index c98d503664..526bd18104 100644 --- a/plugins/chain_plugin/chain_plugin.cpp +++ b/plugins/chain_plugin/chain_plugin.cpp @@ -291,6 +291,8 @@ void chain_plugin::set_program_options(options_description& cli, options_descrip "Percentage of actual signature recovery cpu to bill. Whole number percentages, e.g. 50 for 50%") ("chain-threads", bpo::value()->default_value(config::default_controller_thread_pool_size), "Number of worker threads in controller thread pool") + ("vote-threads", bpo::value()->default_value(config::default_vote_thread_pool_size), + "Number of worker threads in vote processor thread pool. Voting disabled if set to 0 (votes are not propagatged on P2P network).") ("contracts-console", bpo::bool_switch()->default_value(false), "print contract's output to console") ("deep-mind", bpo::bool_switch()->default_value(false), @@ -632,9 +634,17 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) { } if( options.count( "chain-threads" )) { - chain_config->thread_pool_size = options.at( "chain-threads" ).as(); - EOS_ASSERT( chain_config->thread_pool_size > 0, plugin_config_exception, - "chain-threads ${num} must be greater than 0", ("num", chain_config->thread_pool_size) ); + chain_config->chain_thread_pool_size = options.at( "chain-threads" ).as(); + EOS_ASSERT( chain_config->chain_thread_pool_size > 0, plugin_config_exception, + "chain-threads ${num} must be greater than 0", ("num", chain_config->chain_thread_pool_size) ); + } + + if( options.count( "vote-threads" )) { + chain_config->vote_thread_pool_size = options.at( "vote-threads" ).as(); + EOS_ASSERT( chain_config->vote_thread_pool_size > 1 || chain_config->vote_thread_pool_size == 0, plugin_config_exception, + "vote-threads ${num} must be greater than 1 or 0. " + "Voting disabled if set to 0 (votes are not propagatged on P2P network).", + ("num", chain_config->vote_thread_pool_size) ); } chain_config->sig_cpu_bill_pct = options.at("signature-cpu-billable-pct").as(); From fba556145c0dabb39429151e0b9748bdc4b49cb0 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 10 Apr 2024 20:20:31 -0500 Subject: [PATCH 04/40] GH-3 Fix issue with not re-locking after unlock --- libraries/chain/include/eosio/chain/vote_processor.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index 288bc155e9..90719df5d4 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -180,6 +180,8 @@ class vote_processor_t { } if (should_break) break; + g.lock(); + i = idx.begin(); } else { not_in_forkdb_id = vt->id(); if (skip_all_for_block(idx, i, (*i)->id())) From 1dcf7c1619554eb40bfda94debe5f213781d5ccf Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 10 Apr 2024 20:20:56 -0500 Subject: [PATCH 05/40] GH-3 Shutdown nodeos on vote thread pool exception --- libraries/chain/controller.cpp | 5 ++++- libraries/chain/include/eosio/chain/vote_processor.hpp | 6 ++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 7cf326008e..8b7f0e9e13 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1199,7 +1199,10 @@ struct controller_impl { if( shutdown ) shutdown(); } ); if (cfg.vote_thread_pool_size > 0) { - vote_processor.start(cfg.vote_thread_pool_size); + vote_processor.start(cfg.vote_thread_pool_size, [this]( const fc::exception& e ) { + elog( "Exception in vote thread pool, exiting: ${e}", ("e", e.to_detail_string()) ); + if( shutdown ) shutdown(); + } ); } set_activation_handler(); diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index 90719df5d4..337162b65b 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -122,11 +122,9 @@ class vote_processor_t { cv.notify_one(); } - void start(size_t num_threads) { + void start(size_t num_threads, decltype(thread_pool)::on_except_t&& on_except) { assert(num_threads > 1); // need at least two as one is used for coordinatation - thread_pool.start( num_threads, []( const fc::exception& e ) { - elog( "Exception in vote processor thread pool, exiting: ${e}", ("e", e.to_detail_string()) ); - } ); + thread_pool.start( num_threads, std::move(on_except)); // one coordinator thread boost::asio::post(thread_pool.get_executor(), [&]() { From 295f7d4b2995073191fd2101beedcb1f278cdc90 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 11 Apr 2024 13:45:44 -0500 Subject: [PATCH 06/40] GH-3 Add unittest for vote_processor. Modify vote_processor to make it easier to test. --- libraries/chain/controller.cpp | 7 +- .../include/eosio/chain/block_header.hpp | 2 +- .../include/eosio/chain/vote_processor.hpp | 29 ++- unittests/subjective_billing_tests.cpp | 2 +- unittests/vote_processor_tests.cpp | 228 ++++++++++++++++++ 5 files changed, 255 insertions(+), 13 deletions(-) create mode 100644 unittests/vote_processor_tests.cpp diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 8b7f0e9e13..18a197ac51 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -949,7 +949,12 @@ struct controller_impl { signal)> applied_transaction; signal voted_block; - vote_processor_t vote_processor{fork_db, voted_block}; + vote_processor_t vote_processor{voted_block, + [this](const block_id_type& id) -> block_state_ptr { + return fork_db.apply_s([&](const auto& forkdb) { + return forkdb.get_block(id); + }); + }}; int64_t set_proposed_producers( vector producers ); int64_t set_proposed_producers_legacy( vector producers ); diff --git a/libraries/chain/include/eosio/chain/block_header.hpp b/libraries/chain/include/eosio/chain/block_header.hpp index 514879ccd0..223a3e526d 100644 --- a/libraries/chain/include/eosio/chain/block_header.hpp +++ b/libraries/chain/include/eosio/chain/block_header.hpp @@ -90,7 +90,7 @@ namespace eosio::chain { // When block header is validated in block_header_state's next(), // it is already validate if schedule_version == proper_svnn_schedule_version, // finality extension must exist. - bool is_proper_svnn_block() const { return ( schedule_version == proper_svnn_schedule_version ); } + bool is_proper_svnn_block() const { return ( schedule_version == proper_svnn_schedule_version ); } header_extension_multimap validate_and_extract_header_extensions()const; std::optional extract_header_extension(uint16_t extension_id)const; diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index 337162b65b..f718e743da 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -32,7 +33,7 @@ class vote_processor_t { using vote_ptr = std::shared_ptr; using vote_signal_type = decltype(controller({},chain_id_type::empty_chain_id()).voted_block()); - typedef multi_index_container< vote_ptr, + using vote_index_type = boost::multi_index_container< vote_ptr, indexed_by< ordered_non_unique, composite_key, member >, ordered_unique< tag, member > > - > vote_index_type; + >; + + using fetch_block_func_t = std::function; + + vote_signal_type& vote_signal; + fetch_block_func_t fetch_block_func; - fork_database& fork_db; std::mutex mtx; std::condition_variable cv; vote_index_type index; // connection, count of messages std::map num_messages; + std::atomic lib{0}; std::atomic stopped{false}; - vote_signal_type& vote_signal; named_thread_pool thread_pool; private: @@ -112,9 +117,10 @@ class vote_processor_t { } public: - explicit vote_processor_t(fork_database& forkdb, vote_signal_type& vote_signal) - : fork_db(forkdb) - , vote_signal(vote_signal) {} + explicit vote_processor_t(vote_signal_type& vote_signal, fetch_block_func_t&& get_block) + : vote_signal(vote_signal) + , fetch_block_func(get_block) + {} ~vote_processor_t() { stopped = true; @@ -122,6 +128,11 @@ class vote_processor_t { cv.notify_one(); } + size_t size() { + std::lock_guard g(mtx); + return index.size(); + } + void start(size_t num_threads, decltype(thread_pool)::on_except_t&& on_except) { assert(num_threads > 1); // need at least two as one is used for coordinatation thread_pool.start( num_threads, std::move(on_except)); @@ -151,9 +162,7 @@ class vote_processor_t { } for (auto i = idx.begin(); i != idx.end();) { auto& vt = *i; - block_state_ptr bsp = fork_db.apply_s([&](const auto& forkdb) { - return forkdb.get_block(vt->id()); - }); + block_state_ptr bsp = fetch_block_func(vt->id()); if (bsp) { if (!bsp->is_proper_svnn_block()) { if (remove_all_for_block(idx, i, bsp->id())) diff --git a/unittests/subjective_billing_tests.cpp b/unittests/subjective_billing_tests.cpp index f80e5ea909..4cb25fbaf9 100644 --- a/unittests/subjective_billing_tests.cpp +++ b/unittests/subjective_billing_tests.cpp @@ -1,6 +1,6 @@ #include -#include "eosio/chain/subjective_billing.hpp" +#include #include #include diff --git a/unittests/vote_processor_tests.cpp b/unittests/vote_processor_tests.cpp new file mode 100644 index 0000000000..1873c1ee7b --- /dev/null +++ b/unittests/vote_processor_tests.cpp @@ -0,0 +1,228 @@ +#include + +#include +#include +#include +#include +#include +#include + +namespace std { +std::ostream& operator<<(std::ostream& os, const eosio::chain::vote_message& v) { + os << "vote_message{" << v.block_id << std::endl; + return os; +} +std::ostream& operator<<(std::ostream& os, const eosio::chain::vote_status& v) { + os << fc::reflector::to_string(v) << std::endl; + return os; +} +} + +namespace { + +using namespace eosio; +using namespace eosio::chain; + +block_id_type make_block_id(uint32_t block_num) { + block_id_type block_id; + block_id._hash[0] &= 0xffffffff00000000; + block_id._hash[0] += fc::endian_reverse_u32(block_num); + return block_id; +} + +bls_private_key bls_priv_key_0 = bls_private_key::generate(); +bls_private_key bls_priv_key_1 = bls_private_key::generate(); +bls_private_key bls_priv_key_2 = bls_private_key::generate(); +std::vector bls_priv_keys{bls_priv_key_0, bls_priv_key_1, bls_priv_key_2}; + +auto create_genesis_block_state() { // block 2 + signed_block_ptr block = std::make_shared(); + + block->producer = eosio::chain::config::system_account_name; + auto pub_key = eosio::testing::base_tester::get_public_key( block->producer, "active" ); + + std::vector finalizers; + finalizers.push_back(finalizer_authority{.description = "first", .weight = 1, .public_key = bls_priv_keys.at(0).get_public_key()}); + finalizers.push_back(finalizer_authority{.description = "first", .weight = 1, .public_key = bls_priv_keys.at(1).get_public_key()}); + finalizers.push_back(finalizer_authority{.description = "first", .weight = 1, .public_key = bls_priv_keys.at(2).get_public_key()}); + finalizer_policy new_finalizer_policy{.finalizers = finalizers}; + qc_claim_t initial_if_claim { .block_num = 2, + .is_strong_qc = false }; + emplace_extension(block->header_extensions, instant_finality_extension::extension_id(), + fc::raw::pack(instant_finality_extension{ initial_if_claim, new_finalizer_policy, {} })); + + producer_authority_schedule schedule = { 0, { producer_authority{block->producer, block_signing_authority_v0{ 1, {{pub_key, 1}} } } } }; + auto genesis = std::make_shared(); + genesis->block = block; + genesis->active_finalizer_policy = std::make_shared(new_finalizer_policy); + genesis->block->previous = make_block_id(1); + genesis->active_proposer_policy = std::make_shared(proposer_policy{.proposer_schedule = schedule}); + genesis->core = finality_core::create_core_for_genesis_block(1); + genesis->block_id = genesis->block->calculate_id(); + return genesis; +} + +auto create_test_block_state(const block_state_ptr& prev) { + static block_timestamp_type timestamp; + timestamp = timestamp.next(); // each test block state will be unique + signed_block_ptr block = std::make_shared(prev->block->clone()); + block->producer = eosio::chain::config::system_account_name; + block->previous = prev->id(); + block->timestamp = timestamp; + + auto priv_key = eosio::testing::base_tester::get_private_key( block->producer, "active" ); + auto pub_key = eosio::testing::base_tester::get_public_key( block->producer, "active" ); + + auto sig_digest = digest_type::hash("something"); + block->producer_signature = priv_key.sign( sig_digest ); + + vector signing_keys; + signing_keys.emplace_back( std::move( priv_key ) ); + + auto signer = [&]( digest_type d ) { + std::vector result; + result.reserve(signing_keys.size()); + for (const auto& k: signing_keys) + result.emplace_back(k.sign(d)); + return result; + }; + block_header_state bhs = *prev; + bhs.header = *block; + bhs.header.timestamp = timestamp; + bhs.header.previous = prev->id(); + bhs.header.schedule_version = block_header::proper_svnn_schedule_version; + bhs.block_id = block->calculate_id(); + + auto bsp = std::make_shared(bhs, + deque{}, + deque{}, + std::optional{}, + std::optional{}, + signer, + block_signing_authority_v0{ 1, {{pub_key, 1}} }, + digest_type{}); + + return bsp; +} + +vote_message make_empty_message(const block_id_type& id) { + vote_message vm; + vm.block_id = id; + return vm; +} + +vote_message make_vote_message(const block_state_ptr& bsp) { + vote_message vm; + vm.block_id = bsp->id(); + vm.strong = true; + size_t i = bsp->block_num() % bls_priv_keys.size(); + vm.finalizer_key = bls_priv_keys.at(i).get_public_key(); + vm.sig = bls_priv_keys.at(i).sign({(uint8_t*)bsp->strong_digest.data(), (uint8_t*)bsp->strong_digest.data() + bsp->strong_digest.data_size()}); + return vm; +} + +BOOST_AUTO_TEST_SUITE(vote_processor_tests) + +BOOST_AUTO_TEST_CASE( vote_processor_test ) { + boost::signals2::signal voted_block; + + uint32_t received_connection_id = 0; + vote_status received_vote_status = vote_status::unknown_block; + vote_message received_vote_message{}; + + std::unique_ptr signaled; + std::mutex forkdb_mtx; + std::map forkdb; + auto add_to_forkdb = [&](const block_state_ptr& bsp) { + std::lock_guard g(forkdb_mtx); + forkdb[bsp->id()] = bsp; + }; + + voted_block.connect( [&]( const vote_signal_params& vote_signal ) { + received_connection_id = std::get<0>(vote_signal); + received_vote_status = std::get<1>(vote_signal); + received_vote_message = std::get<2>(vote_signal); + signaled->count_down(); + } ); + + vote_processor_t vp{voted_block, [&](const block_id_type& id) -> block_state_ptr { + std::lock_guard g(forkdb_mtx); + return forkdb[id]; + }}; + vp.start(2, [](const fc::exception& e) { + edump((e)); + BOOST_REQUIRE(false); + }); + + { // empty fork db, block never found, never signaled + vote_message vm1 = make_empty_message(make_block_id(1)); + signaled = std::make_unique(1); + vp.process_vote_message(1, vm1); + for (size_t i = 0; i < 5; ++i) { + BOOST_CHECK(!signaled->try_wait()); // not signaled because no block + std::this_thread::sleep_for(std::chrono::milliseconds{5}); + } + BOOST_CHECK(vp.size() == 1); + // move lib past block + vp.notify_lib(2); + for (size_t i = 0; i < 5; ++i) { + if (vp.size() == 0) break; + std::this_thread::sleep_for(std::chrono::milliseconds{5}); + } + BOOST_CHECK(vp.size() == 0); + } + { // process a valid vote + signaled = std::make_unique(1); + auto gensis = create_genesis_block_state(); + auto bsp = create_test_block_state(gensis); + BOOST_CHECK_EQUAL(bsp->block_num(), 3); + vote_message m1 = make_vote_message(bsp); + add_to_forkdb(bsp); + vp.process_vote_message(1, m1); + // duplicate ignored + vp.process_vote_message(1, m1); + signaled->wait(); + BOOST_CHECK(1 == received_connection_id); + BOOST_CHECK(vote_status::success == received_vote_status); + BOOST_CHECK(m1 == received_vote_message); + } + { // process an invalid signature vote + signaled = std::make_unique(1); + auto gensis = create_genesis_block_state(); + auto bsp = create_test_block_state(gensis); + BOOST_CHECK_EQUAL(bsp->block_num(), 3); + vote_message m1 = make_vote_message(bsp); + m1.strong = false; // signed with strong_digest + add_to_forkdb(bsp); + vp.process_vote_message(1, m1); + signaled->wait(); + BOOST_CHECK(1 == received_connection_id); + BOOST_CHECK(vote_status::invalid_signature == received_vote_status); + BOOST_CHECK(m1 == received_vote_message); + } + { // process two diff block votes + signaled = std::make_unique(2); + auto gensis = create_genesis_block_state(); + auto bsp = create_test_block_state(gensis); + auto bsp2 = create_test_block_state(bsp); + vote_message m1 = make_vote_message(bsp); + vote_message m2 = make_vote_message(bsp2); + vp.process_vote_message(2, m1); + vp.process_vote_message(2, m2); + for (size_t i = 0; i < 5; ++i) { + if (vp.size() == 2) break; + std::this_thread::sleep_for(std::chrono::milliseconds{5}); + } + BOOST_CHECK(vp.size() == 2); + add_to_forkdb(bsp); + add_to_forkdb(bsp2); + signaled->wait(); + BOOST_CHECK(2 == received_connection_id); + BOOST_CHECK(vote_status::success == received_vote_status); + BOOST_CHECK(m1 == received_vote_message || m2 == received_vote_message); + } +} + +BOOST_AUTO_TEST_SUITE_END() + +} From 18c2fcad462ab7e3324b0634ae97981475e2da53 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 11 Apr 2024 14:56:43 -0500 Subject: [PATCH 07/40] GH-3 std::latch not available on all platforms; use std::atomic and for loop instead. --- unittests/vote_processor_tests.cpp | 31 ++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/unittests/vote_processor_tests.cpp b/unittests/vote_processor_tests.cpp index 1873c1ee7b..a726e06de8 100644 --- a/unittests/vote_processor_tests.cpp +++ b/unittests/vote_processor_tests.cpp @@ -5,7 +5,6 @@ #include #include #include -#include namespace std { std::ostream& operator<<(std::ostream& os, const eosio::chain::vote_message& v) { @@ -130,7 +129,7 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { vote_status received_vote_status = vote_status::unknown_block; vote_message received_vote_message{}; - std::unique_ptr signaled; + std::atomic signaled = 0; std::mutex forkdb_mtx; std::map forkdb; auto add_to_forkdb = [&](const block_state_ptr& bsp) { @@ -142,7 +141,7 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { received_connection_id = std::get<0>(vote_signal); received_vote_status = std::get<1>(vote_signal); received_vote_message = std::get<2>(vote_signal); - signaled->count_down(); + ++signaled; } ); vote_processor_t vp{voted_block, [&](const block_id_type& id) -> block_state_ptr { @@ -156,10 +155,9 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { { // empty fork db, block never found, never signaled vote_message vm1 = make_empty_message(make_block_id(1)); - signaled = std::make_unique(1); + signaled = 0; vp.process_vote_message(1, vm1); - for (size_t i = 0; i < 5; ++i) { - BOOST_CHECK(!signaled->try_wait()); // not signaled because no block + for (size_t i = 0; i < 5 && vp.size() < 1; ++i) { std::this_thread::sleep_for(std::chrono::milliseconds{5}); } BOOST_CHECK(vp.size() == 1); @@ -172,7 +170,7 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { BOOST_CHECK(vp.size() == 0); } { // process a valid vote - signaled = std::make_unique(1); + signaled = 0; auto gensis = create_genesis_block_state(); auto bsp = create_test_block_state(gensis); BOOST_CHECK_EQUAL(bsp->block_num(), 3); @@ -181,13 +179,16 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { vp.process_vote_message(1, m1); // duplicate ignored vp.process_vote_message(1, m1); - signaled->wait(); + for (size_t i = 0; i < 5 && signaled.load() < 1; ++i) { + std::this_thread::sleep_for(std::chrono::milliseconds{5}); + } + BOOST_CHECK(signaled.load() == 1); BOOST_CHECK(1 == received_connection_id); BOOST_CHECK(vote_status::success == received_vote_status); BOOST_CHECK(m1 == received_vote_message); } { // process an invalid signature vote - signaled = std::make_unique(1); + signaled = 0; auto gensis = create_genesis_block_state(); auto bsp = create_test_block_state(gensis); BOOST_CHECK_EQUAL(bsp->block_num(), 3); @@ -195,13 +196,16 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { m1.strong = false; // signed with strong_digest add_to_forkdb(bsp); vp.process_vote_message(1, m1); - signaled->wait(); + for (size_t i = 0; i < 5 && signaled.load() < 1; ++i) { + std::this_thread::sleep_for(std::chrono::milliseconds{5}); + } + BOOST_CHECK(signaled.load() == 1); BOOST_CHECK(1 == received_connection_id); BOOST_CHECK(vote_status::invalid_signature == received_vote_status); BOOST_CHECK(m1 == received_vote_message); } { // process two diff block votes - signaled = std::make_unique(2); + signaled = 0; auto gensis = create_genesis_block_state(); auto bsp = create_test_block_state(gensis); auto bsp2 = create_test_block_state(bsp); @@ -216,7 +220,10 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { BOOST_CHECK(vp.size() == 2); add_to_forkdb(bsp); add_to_forkdb(bsp2); - signaled->wait(); + for (size_t i = 0; i < 5 && signaled.load() < 2; ++i) { + std::this_thread::sleep_for(std::chrono::milliseconds{5}); + } + BOOST_CHECK(signaled.load() == 2); BOOST_CHECK(2 == received_connection_id); BOOST_CHECK(vote_status::success == received_vote_status); BOOST_CHECK(m1 == received_vote_message || m2 == received_vote_message); From d5cef78e7ef980a365749bca4ed4c7c97e7c4a38 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 11 Apr 2024 15:32:00 -0500 Subject: [PATCH 08/40] GH-3 ci/cd is slow, allow more time. --- unittests/vote_processor_tests.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/unittests/vote_processor_tests.cpp b/unittests/vote_processor_tests.cpp index a726e06de8..47032fdc82 100644 --- a/unittests/vote_processor_tests.cpp +++ b/unittests/vote_processor_tests.cpp @@ -157,14 +157,13 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { vote_message vm1 = make_empty_message(make_block_id(1)); signaled = 0; vp.process_vote_message(1, vm1); - for (size_t i = 0; i < 5 && vp.size() < 1; ++i) { + for (size_t i = 0; i < 50 && vp.size() < 1; ++i) { std::this_thread::sleep_for(std::chrono::milliseconds{5}); } BOOST_CHECK(vp.size() == 1); // move lib past block vp.notify_lib(2); - for (size_t i = 0; i < 5; ++i) { - if (vp.size() == 0) break; + for (size_t i = 0; i < 50 && vp.size() > 0; ++i) { std::this_thread::sleep_for(std::chrono::milliseconds{5}); } BOOST_CHECK(vp.size() == 0); @@ -179,7 +178,7 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { vp.process_vote_message(1, m1); // duplicate ignored vp.process_vote_message(1, m1); - for (size_t i = 0; i < 5 && signaled.load() < 1; ++i) { + for (size_t i = 0; i < 50 && signaled.load() < 1; ++i) { std::this_thread::sleep_for(std::chrono::milliseconds{5}); } BOOST_CHECK(signaled.load() == 1); @@ -196,7 +195,7 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { m1.strong = false; // signed with strong_digest add_to_forkdb(bsp); vp.process_vote_message(1, m1); - for (size_t i = 0; i < 5 && signaled.load() < 1; ++i) { + for (size_t i = 0; i < 50 && signaled.load() < 1; ++i) { std::this_thread::sleep_for(std::chrono::milliseconds{5}); } BOOST_CHECK(signaled.load() == 1); @@ -220,7 +219,7 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { BOOST_CHECK(vp.size() == 2); add_to_forkdb(bsp); add_to_forkdb(bsp2); - for (size_t i = 0; i < 5 && signaled.load() < 2; ++i) { + for (size_t i = 0; i < 50 && signaled.load() < 2; ++i) { std::this_thread::sleep_for(std::chrono::milliseconds{5}); } BOOST_CHECK(signaled.load() == 2); From da0106b5b0662564dbe01d0e80ba2d66e4462197 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 11 Apr 2024 16:33:02 -0500 Subject: [PATCH 09/40] GH-3 Make test more robust by checking that node has received a handshake --- tests/auto_bp_peering_test.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/auto_bp_peering_test.py b/tests/auto_bp_peering_test.py index 666cbd5536..b7c8dd3011 100755 --- a/tests/auto_bp_peering_test.py +++ b/tests/auto_bp_peering_test.py @@ -113,6 +113,8 @@ def neighbors_in_schedule(name, schedule): for conn in connections["payload"]: peer_addr = conn["peer"] if len(peer_addr) == 0: + if len(conn["last_handshake"]["p2p_address"]) == 0: + continue peer_addr = conn["last_handshake"]["p2p_address"].split()[0] if peer_names[peer_addr] != "bios": peers.append(peer_names[peer_addr]) From bf545e337b8d19004289791b8590637c0f680b46 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 11 Apr 2024 16:39:39 -0500 Subject: [PATCH 10/40] GH-3 Optimization: Don't emit duplicate votes --- libraries/chain/include/eosio/chain/vote_processor.hpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index f718e743da..7786b07406 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -182,7 +182,9 @@ class vote_processor_t { for (auto& vptr : to_process) { boost::asio::post(thread_pool.get_executor(), [this, bsp, vptr=std::move(vptr)]() { vote_status s = bsp->aggregate_vote(vptr->msg); - emit(vptr->connection_id, s, vptr->msg); + if (s != vote_status::duplicate) { // don't bother emitting duplicates + emit(vptr->connection_id, s, vptr->msg); + } }); } if (should_break) From d1fe6369e805837a60a07f5dbc9645cf90e9d0b4 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 11 Apr 2024 17:18:26 -0500 Subject: [PATCH 11/40] GH-3 Fix: num_messages.clear() called before decrement --- libraries/chain/include/eosio/chain/vote_processor.hpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index 7786b07406..8e529edaa6 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -103,7 +103,9 @@ class vote_processor_t { bool remove_all_for_block(auto& idx, auto& it, const block_id_type& id) { while (it != idx.end() && (*it)->id() == id) { - --num_messages[(*it)->connection_id]; + if (auto& num = num_messages[(*it)->connection_id]; num != 0) + --num; + it = idx.erase(it); } return it == idx.end(); From f3ff3e86866caa71e16fbfdaac9218facbcec3c6 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 11 Apr 2024 20:02:09 -0500 Subject: [PATCH 12/40] GH-3 Avoid vote_message copies by using shared_ptr --- libraries/chain/controller.cpp | 6 +- libraries/chain/hotstuff/finalizer.cpp | 8 +-- .../chain/include/eosio/chain/controller.hpp | 4 +- .../eosio/chain/hotstuff/finalizer.hpp | 9 ++- .../include/eosio/chain/hotstuff/hotstuff.hpp | 2 + .../include/eosio/chain/vote_processor.hpp | 49 +++++++------ plugins/net_plugin/net_plugin.cpp | 68 ++++++++++++------- unittests/finality_test_cluster.cpp | 24 +++---- unittests/finality_test_cluster.hpp | 4 +- 9 files changed, 96 insertions(+), 78 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 18a197ac51..0ef3788a9b 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -3567,7 +3567,7 @@ struct controller_impl { // called from net threads and controller's thread pool - void process_vote_message( uint32_t connection_id, const vote_message& vote ) { + void process_vote_message( uint32_t connection_id, const vote_message_ptr& vote ) { if (conf.vote_thread_pool_size > 0) { vote_processor.process_vote_message(connection_id, vote); } @@ -3597,7 +3597,7 @@ struct controller_impl { // Each finalizer configured on the node which is present in the active finalizer policy may create and sign a vote. my_finalizers.maybe_vote( - *bsp->active_finalizer_policy, bsp, bsp->strong_digest, [&](const vote_message& vote) { + *bsp->active_finalizer_policy, bsp, bsp->strong_digest, [&](const vote_message_ptr& vote) { // net plugin subscribed to this signal. it will broadcast the vote message on receiving the signal emit(voted_block, std::tuple{uint32_t{0}, vote_status::success, std::cref(vote)}); @@ -5259,7 +5259,7 @@ void controller::set_proposed_finalizers( finalizer_policy&& fin_pol ) { } // called from net threads -void controller::process_vote_message( uint32_t connection_id, const vote_message& vote ) { +void controller::process_vote_message( uint32_t connection_id, const vote_message_ptr& vote ) { my->process_vote_message( connection_id, vote ); }; diff --git a/libraries/chain/hotstuff/finalizer.cpp b/libraries/chain/hotstuff/finalizer.cpp index af0d3028c2..69f6e10feb 100644 --- a/libraries/chain/hotstuff/finalizer.cpp +++ b/libraries/chain/hotstuff/finalizer.cpp @@ -86,9 +86,9 @@ finalizer::vote_result finalizer::decide_vote(const block_state_ptr& bsp) { } // ---------------------------------------------------------------------------------------- -std::optional finalizer::maybe_vote(const bls_public_key& pub_key, - const block_state_ptr& bsp, - const digest_type& digest) { +vote_message_ptr finalizer::maybe_vote(const bls_public_key& pub_key, + const block_state_ptr& bsp, + const digest_type& digest) { finalizer::vote_decision decision = decide_vote(bsp).decision; if (decision == vote_decision::strong_vote || decision == vote_decision::weak_vote) { bls_signature sig; @@ -99,7 +99,7 @@ std::optional finalizer::maybe_vote(const bls_public_key& pub_key, } else { sig = priv_key.sign({(uint8_t*)digest.data(), (uint8_t*)digest.data() + digest.data_size()}); } - return std::optional{vote_message{ bsp->id(), decision == vote_decision::strong_vote, pub_key, sig }}; + return std::make_shared(bsp->id(), decision == vote_decision::strong_vote, pub_key, sig); } return {}; } diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index e4d4285b08..dcd1a18303 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -58,7 +58,7 @@ namespace eosio::chain { using trx_meta_cache_lookup = std::function; using block_signal_params = std::tuple; - using vote_signal_params = std::tuple; + using vote_signal_params = std::tuple; enum class db_read_mode { HEAD, @@ -328,7 +328,7 @@ namespace eosio::chain { // called by host function set_finalizers void set_proposed_finalizers( finalizer_policy&& fin_pol ); // called from net threads - void process_vote_message( uint32_t connection_id, const vote_message& msg ); + void process_vote_message( uint32_t connection_id, const vote_message_ptr& msg ); // thread safe, for testing bool node_has_voted_if_finalizer(const block_id_type& id) const; diff --git a/libraries/chain/include/eosio/chain/hotstuff/finalizer.hpp b/libraries/chain/include/eosio/chain/hotstuff/finalizer.hpp index 762524a46b..296e11eae0 100644 --- a/libraries/chain/include/eosio/chain/hotstuff/finalizer.hpp +++ b/libraries/chain/include/eosio/chain/hotstuff/finalizer.hpp @@ -61,8 +61,7 @@ namespace eosio::chain { finalizer_safety_information fsi; vote_result decide_vote(const block_state_ptr& bsp); - std::optional maybe_vote(const bls_public_key& pub_key, const block_state_ptr& bsp, - const digest_type& digest); + vote_message_ptr maybe_vote(const bls_public_key& pub_key, const block_state_ptr& bsp, const digest_type& digest); }; // ---------------------------------------------------------------------------------------- @@ -95,7 +94,7 @@ namespace eosio::chain { if (finalizers.empty()) return; - std::vector votes; + std::vector votes; votes.reserve(finalizers.size()); // Possible improvement in the future, look at locking only individual finalizers and releasing the lock for writing the file. @@ -105,9 +104,9 @@ namespace eosio::chain { // first accumulate all the votes for (const auto& f : fin_pol.finalizers) { if (auto it = finalizers.find(f.public_key); it != finalizers.end()) { - std::optional vote_msg = it->second.maybe_vote(it->first, bsp, digest); + vote_message_ptr vote_msg = it->second.maybe_vote(it->first, bsp, digest); if (vote_msg) - votes.push_back(std::move(*vote_msg)); + votes.push_back(std::move(vote_msg)); } } // then save the safety info and, if successful, gossip the votes diff --git a/libraries/chain/include/eosio/chain/hotstuff/hotstuff.hpp b/libraries/chain/include/eosio/chain/hotstuff/hotstuff.hpp index f6e2d4297f..9e8682b905 100644 --- a/libraries/chain/include/eosio/chain/hotstuff/hotstuff.hpp +++ b/libraries/chain/include/eosio/chain/hotstuff/hotstuff.hpp @@ -30,6 +30,8 @@ namespace eosio::chain { bool operator==(const vote_message&) const = default; }; + using vote_message_ptr = std::shared_ptr; + enum class vote_status { success, duplicate, // duplicate vote, expected as votes arrive on multiple connections diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index 8e529edaa6..793705acbc 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -23,17 +23,16 @@ class vote_processor_t { struct by_vote; struct vote { - uint32_t connection_id; - vote_message msg; + uint32_t connection_id; + vote_message_ptr msg; - const block_id_type& id() const { return msg.block_id; } - block_num_type block_num() const { return block_header::num_from_id(msg.block_id); } + const block_id_type& id() const { return msg->block_id; } + block_num_type block_num() const { return block_header::num_from_id(msg->block_id); } }; - using vote_ptr = std::shared_ptr; using vote_signal_type = decltype(controller({},chain_id_type::empty_chain_id()).voted_block()); - using vote_index_type = boost::multi_index_container< vote_ptr, + using vote_index_type = boost::multi_index_container< vote, indexed_by< ordered_non_unique, composite_key >, composite_key_compare< std::greater<>, sha256_less > // greater for block_num >, - ordered_non_unique< tag, member >, - ordered_unique< tag, member > + ordered_non_unique< tag, member > > >; @@ -84,7 +82,7 @@ class vote_processor_t { } } - void emit(uint32_t connection_id, vote_status status, const vote_message& msg) { + void emit(uint32_t connection_id, vote_status status, const vote_message_ptr& msg) { if (connection_id != 0) { // this nodes vote was already signaled emit( vote_signal, std::tuple{connection_id, status, std::cref(msg)} ); } @@ -102,8 +100,8 @@ class vote_processor_t { } bool remove_all_for_block(auto& idx, auto& it, const block_id_type& id) { - while (it != idx.end() && (*it)->id() == id) { - if (auto& num = num_messages[(*it)->connection_id]; num != 0) + while (it != idx.end() && it->id() == id) { + if (auto& num = num_messages[it->connection_id]; num != 0) --num; it = idx.erase(it); @@ -112,7 +110,7 @@ class vote_processor_t { } bool skip_all_for_block(auto& idx, auto& it, const block_id_type& id) { - while (it != idx.end() && (*it)->id() == id) { + while (it != idx.end() && it->id() == id) { ++it; } return it == idx.end(); @@ -157,14 +155,14 @@ class vote_processor_t { continue; } auto& idx = index.get(); - if (auto i = idx.begin(); i != idx.end() && not_in_forkdb_id == (*i)->id()) { // same block as last while loop + if (auto i = idx.begin(); i != idx.end() && not_in_forkdb_id == i->id()) { // same block as last while loop g.unlock(); std::this_thread::sleep_for(block_wait_time); g.lock(); } for (auto i = idx.begin(); i != idx.end();) { auto& vt = *i; - block_state_ptr bsp = fetch_block_func(vt->id()); + block_state_ptr bsp = fetch_block_func(vt.id()); if (bsp) { if (!bsp->is_proper_svnn_block()) { if (remove_all_for_block(idx, i, bsp->id())) @@ -172,20 +170,20 @@ class vote_processor_t { continue; } auto iter_of_bsp = i; - std::vector to_process; + std::vector to_process; to_process.reserve(std::min(21u, idx.size())); // increase if we increase # of finalizers from 21 - for(; i != idx.end() && bsp->id() == (*i)->id(); ++i) { + for(; i != idx.end() && bsp->id() == i->id(); ++i) { // although it is the highest contention on block state pending mutex posting all of the same bsp, // the highest priority is processing votes for this block state. to_process.push_back(*i); } bool should_break = remove_all_for_block(idx, iter_of_bsp, bsp->id()); g.unlock(); // do not hold lock when posting - for (auto& vptr : to_process) { - boost::asio::post(thread_pool.get_executor(), [this, bsp, vptr=std::move(vptr)]() { - vote_status s = bsp->aggregate_vote(vptr->msg); + for (auto& v : to_process) { + boost::asio::post(thread_pool.get_executor(), [this, bsp, v=std::move(v)]() { + vote_status s = bsp->aggregate_vote(*v.msg); if (s != vote_status::duplicate) { // don't bother emitting duplicates - emit(vptr->connection_id, s, vptr->msg); + emit(v.connection_id, s, v.msg); } }); } @@ -194,8 +192,8 @@ class vote_processor_t { g.lock(); i = idx.begin(); } else { - not_in_forkdb_id = vt->id(); - if (skip_all_for_block(idx, i, (*i)->id())) + not_in_forkdb_id = vt.id(); + if (skip_all_for_block(idx, i, i->id())) break; } } @@ -208,8 +206,7 @@ class vote_processor_t { lib = block_num; } - void process_vote_message(uint32_t connection_id, const vote_message& msg) { - vote_ptr vptr = std::make_shared(vote{.connection_id = connection_id, .msg = msg}); + void process_vote_message(uint32_t connection_id, const vote_message_ptr& msg) { boost::asio::post(thread_pool.get_executor(), [this, connection_id, msg] { std::unique_lock g(mtx); if (++num_messages[connection_id] > max_votes_per_connection) { @@ -220,10 +217,10 @@ class vote_processor_t { elog("Exceeded max votes per connection for ${c}", ("c", connection_id)); emit(connection_id, vote_status::max_exceeded, msg); - } else if (block_header::num_from_id(msg.block_id) < lib.load(std::memory_order_relaxed)) { + } else if (block_header::num_from_id(msg->block_id) < lib.load(std::memory_order_relaxed)) { // ignore } else { - index.insert(std::make_shared(vote{.connection_id = connection_id, .msg = msg})); + index.insert(vote{.connection_id = connection_id, .msg = msg}); cv.notify_one(); } }); diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 4bdf231f61..e574153967 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -339,6 +339,7 @@ namespace eosio { constexpr uint32_t signed_block_which = fc::get_index(); // see protocol net_message constexpr uint32_t packed_transaction_which = fc::get_index(); // see protocol net_message + constexpr uint32_t vote_message_which = fc::get_index(); // see protocol net_message class connections_manager { public: @@ -476,6 +477,7 @@ namespace eosio { uint32_t max_nodes_per_host = 1; bool p2p_accept_transactions = true; + bool p2p_accept_votes = true; fc::microseconds p2p_dedup_cache_expire_time_us{}; chain_id_type chain_id; @@ -529,12 +531,12 @@ namespace eosio { void on_accepted_block_header( const signed_block_ptr& block, const block_id_type& id ); void on_accepted_block(); - void on_voted_block ( uint32_t connection_id, vote_status stauts, const vote_message& vote ); + void on_voted_block ( uint32_t connection_id, vote_status stauts, const vote_message_ptr& vote ); void transaction_ack(const std::pair&); void on_irreversible_block( const block_id_type& id, uint32_t block_num ); - void bcast_vote_message( uint32_t exclude_peer, const chain::vote_message& msg ); + void bcast_vote_message( uint32_t exclude_peer, const chain::vote_message_ptr& msg ); void warn_message( uint32_t sender_peer, const chain::hs_message_warning& code ); void start_conn_timer(boost::asio::steady_timer::duration du, std::weak_ptr from_connection); @@ -1005,6 +1007,7 @@ namespace eosio { bool process_next_block_message(uint32_t message_length); bool process_next_trx_message(uint32_t message_length); + bool process_next_vote_message(uint32_t message_length); void update_endpoints(const tcp::endpoint& endpoint = tcp::endpoint()); public: @@ -1105,8 +1108,9 @@ namespace eosio { void handle_message( const signed_block& msg ) = delete; // signed_block_ptr overload used instead void handle_message( const block_id_type& id, signed_block_ptr ptr ); void handle_message( const packed_transaction& msg ) = delete; // packed_transaction_ptr overload used instead - void handle_message( packed_transaction_ptr trx ); - void handle_message( const vote_message& msg ); + void handle_message( const packed_transaction_ptr& trx ); + void handle_message( const vote_message_ptr& msg ); + void handle_message( const vote_message& msg ) = delete; // vote_message_ptr overload used instead // returns calculated number of blocks combined latency uint32_t calc_block_latency(); @@ -1187,12 +1191,6 @@ namespace eosio { peer_dlog( c, "handle sync_request_message" ); c->handle_message( msg ); } - - void operator()( const chain::vote_message& msg ) const { - // continue call to handle_message on connection strand - peer_dlog( c, "handle vote_message" ); - c->handle_message( msg ); - } }; @@ -3077,6 +3075,8 @@ namespace eosio { return process_next_block_message( message_length ); } else if( which == packed_transaction_which ) { return process_next_trx_message( message_length ); + } else if( which == vote_message_which ) { + return process_next_vote_message( message_length ); } else { auto ds = pending_message_buffer.create_datastream(); net_message msg; @@ -3189,7 +3189,7 @@ namespace eosio { auto ds = pending_message_buffer.create_datastream(); unsigned_int which{}; fc::raw::unpack( ds, which ); - shared_ptr ptr = std::make_shared(); + packed_transaction_ptr ptr = std::make_shared(); fc::raw::unpack( ds, *ptr ); if( trx_in_progress_sz > def_max_trx_in_progress_size) { char reason[72]; @@ -3212,7 +3212,26 @@ namespace eosio { return true; } - handle_message( std::move( ptr ) ); + handle_message( ptr ); + return true; + } + + // called from connection strand + bool connection::process_next_vote_message(uint32_t message_length) { + if( !my_impl->p2p_accept_votes ) { + peer_dlog( this, "p2p_accept_votes=false - dropping vote" ); + pending_message_buffer.advance_read_ptr( message_length ); + return true; + } + + auto ds = pending_message_buffer.create_datastream(); + unsigned_int which{}; + fc::raw::unpack( ds, which ); + assert(which == vote_message_which); + vote_message_ptr ptr = std::make_shared(); + fc::raw::unpack( ds, *ptr ); + + handle_message( ptr ); return true; } @@ -3708,10 +3727,10 @@ namespace eosio { } } - void connection::handle_message( const vote_message& msg ) { + void connection::handle_message( const vote_message_ptr& msg ) { peer_dlog(this, "received vote: block #${bn}:${id}.., ${v}, key ${k}..", - ("bn", block_header::num_from_id(msg.block_id))("id", msg.block_id.str().substr(8,16)) - ("v", msg.strong ? "strong" : "weak")("k", msg.finalizer_key.to_string().substr(8, 16))); + ("bn", block_header::num_from_id(msg->block_id))("id", msg->block_id.str().substr(8,16)) + ("v", msg->strong ? "strong" : "weak")("k", msg->finalizer_key.to_string().substr(8, 16))); controller& cc = my_impl->chain_plug->chain(); cc.process_vote_message(connection_id, msg); } @@ -3721,7 +3740,7 @@ namespace eosio { } // called from connection strand - void connection::handle_message( packed_transaction_ptr trx ) { + void connection::handle_message( const packed_transaction_ptr& trx ) { const auto& tid = trx->id(); peer_dlog( this, "received packed_transaction ${id}", ("id", tid) ); @@ -3979,10 +3998,10 @@ namespace eosio { } // called from other threads including net threads - void net_plugin_impl::on_voted_block(uint32_t connection_id, vote_status status, const vote_message& msg) { + void net_plugin_impl::on_voted_block(uint32_t connection_id, vote_status status, const vote_message_ptr& msg) { fc_dlog(logger, "connection - ${c} on voted signal: ${s} block #${bn} ${id}.., ${t}, key ${k}..", - ("c", connection_id)("s", status)("bn", block_header::num_from_id(msg.block_id))("id", msg.block_id.str().substr(8,16)) - ("t", msg.strong ? "strong" : "weak")("k", msg.finalizer_key.to_string().substr(8, 16))); + ("c", connection_id)("s", status)("bn", block_header::num_from_id(msg->block_id))("id", msg->block_id.str().substr(8,16)) + ("t", msg->strong ? "strong" : "weak")("k", msg->finalizer_key.to_string().substr(8, 16))); switch( status ) { case vote_status::success: @@ -3999,7 +4018,7 @@ namespace eosio { break; case vote_status::unknown_block: // track the failure fc_dlog(logger, "connection - ${c} vote unknown block #${bn}:${id}..", - ("c", connection_id)("bn", block_header::num_from_id(msg.block_id))("id", msg.block_id.str().substr(8,16))); + ("c", connection_id)("bn", block_header::num_from_id(msg->block_id))("id", msg->block_id.str().substr(8,16))); my_impl->connections.for_each_connection([connection_id](const connection_ptr& c) { if (c->connection_id == connection_id) { c->block_status_monitor_.rejected(); @@ -4013,13 +4032,13 @@ namespace eosio { } } - void net_plugin_impl::bcast_vote_message( uint32_t exclude_peer, const chain::vote_message& msg ) { + void net_plugin_impl::bcast_vote_message( uint32_t exclude_peer, const chain::vote_message_ptr& msg ) { buffer_factory buff_factory; - auto send_buffer = buff_factory.get_send_buffer( msg ); + auto send_buffer = buff_factory.get_send_buffer( *msg ); fc_dlog(logger, "bcast ${t} vote: block #${bn} ${id}.., ${v}, key ${k}..", - ("t", exclude_peer ? "received" : "our")("bn", block_header::num_from_id(msg.block_id))("id", msg.block_id.str().substr(8,16)) - ("v", msg.strong ? "strong" : "weak")("k", msg.finalizer_key.to_string().substr(8,16))); + ("t", exclude_peer ? "received" : "our")("bn", block_header::num_from_id(msg->block_id))("id", msg->block_id.str().substr(8,16)) + ("v", msg->strong ? "strong" : "weak")("k", msg->finalizer_key.to_string().substr(8,16))); dispatcher.strand.post( [this, exclude_peer, msg{std::move(send_buffer)}]() mutable { dispatcher.bcast_vote_msg( exclude_peer, std::move(msg) ); @@ -4244,6 +4263,7 @@ namespace eosio { resp_expected_period = def_resp_expected_wait; max_nodes_per_host = options.at( "p2p-max-nodes-per-host" ).as(); p2p_accept_transactions = options.at( "p2p-accept-transactions" ).as(); + p2p_accept_votes = options.at("vote-threads").as() != 0; use_socket_read_watermark = options.at( "use-socket-read-watermark" ).as(); keepalive_interval = std::chrono::milliseconds( options.at( "p2p-keepalive-interval-ms" ).as() ); diff --git a/unittests/finality_test_cluster.cpp b/unittests/finality_test_cluster.cpp index e84340cb44..42d65a2221 100644 --- a/unittests/finality_test_cluster.cpp +++ b/unittests/finality_test_cluster.cpp @@ -129,10 +129,10 @@ void finality_test_cluster::node1_corrupt_vote_proposal_id() { std::lock_guard g(node1.votes_mtx); node1_orig_vote = node1.votes[0]; - if( node1.votes[0].block_id.data()[0] == 'a' ) { - node1.votes[0].block_id.data()[0] = 'b'; + if( node1.votes[0]->block_id.data()[0] == 'a' ) { + node1.votes[0]->block_id.data()[0] = 'b'; } else { - node1.votes[0].block_id.data()[0] = 'a'; + node1.votes[0]->block_id.data()[0] = 'a'; } } @@ -141,10 +141,10 @@ void finality_test_cluster::node1_corrupt_vote_finalizer_key() { node1_orig_vote = node1.votes[0]; // corrupt the finalizer_key (manipulate so it is different) - auto g1 = node1.votes[0].finalizer_key.jacobian_montgomery_le(); + auto g1 = node1.votes[0]->finalizer_key.jacobian_montgomery_le(); g1 = bls12_381::aggregate_public_keys(std::array{g1, g1}); auto affine = g1.toAffineBytesLE(bls12_381::from_mont::yes); - node1.votes[0].finalizer_key = fc::crypto::blslib::bls_public_key(affine); + node1.votes[0]->finalizer_key = fc::crypto::blslib::bls_public_key(affine); } void finality_test_cluster::node1_corrupt_vote_signature() { @@ -152,10 +152,10 @@ void finality_test_cluster::node1_corrupt_vote_signature() { node1_orig_vote = node1.votes[0]; // corrupt the signature - auto g2 = node1.votes[0].sig.jacobian_montgomery_le(); + auto g2 = node1.votes[0]->sig.jacobian_montgomery_le(); g2 = bls12_381::aggregate_signatures(std::array{g2, g2}); auto affine = g2.toAffineBytesLE(bls12_381::from_mont::yes); - node1.votes[0].sig = fc::crypto::blslib::bls_signature(affine); + node1.votes[0]->sig = fc::crypto::blslib::bls_signature(affine); } void finality_test_cluster::node1_restore_to_original_vote() { @@ -208,20 +208,20 @@ eosio::chain::vote_status finality_test_cluster::process_vote(node_info& node, s FC_ASSERT( vote_index < node.votes.size(), "out of bound index in process_vote" ); auto& vote = node.votes[vote_index]; if( mode == vote_mode::strong ) { - vote.strong = true; + vote->strong = true; } else { - vote.strong = false; + vote->strong = false; // fetch the strong digest - auto strong_digest = node.node.control->get_strong_digest_by_id(vote.block_id); + auto strong_digest = node.node.control->get_strong_digest_by_id(vote->block_id); // convert the strong digest to weak and sign it - vote.sig = node.priv_key.sign(eosio::chain::create_weak_digest(strong_digest)); + vote->sig = node.priv_key.sign(eosio::chain::create_weak_digest(strong_digest)); } g.unlock(); static uint32_t connection_id = 0; node0.node.control->process_vote_message( ++connection_id, vote ); - if (eosio::chain::block_header::num_from_id(vote.block_id) > node0.node.control->last_irreversible_block_num()) + if (eosio::chain::block_header::num_from_id(vote->block_id) > node0.node.control->last_irreversible_block_num()) return wait_on_vote(connection_id); return eosio::chain::vote_status::unknown_block; } diff --git a/unittests/finality_test_cluster.hpp b/unittests/finality_test_cluster.hpp index a84b86bb46..0c965e6d99 100644 --- a/unittests/finality_test_cluster.hpp +++ b/unittests/finality_test_cluster.hpp @@ -79,7 +79,7 @@ class finality_test_cluster { eosio::testing::tester node; uint32_t prev_lib_num{0}; std::mutex votes_mtx; - std::vector votes; + std::vector votes; fc::crypto::blslib::bls_private_key priv_key; }; @@ -91,7 +91,7 @@ class finality_test_cluster { node_info& node1 = nodes[1]; node_info& node2 = nodes[2]; - eosio::chain::vote_message node1_orig_vote; + eosio::chain::vote_message_ptr node1_orig_vote; // sets up "node_index" node void setup_node(node_info& node, eosio::chain::account_name local_finalizer); From ae02196bab0a6c64d8df895bf60a92a632e92078 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 12 Apr 2024 06:20:52 -0500 Subject: [PATCH 13/40] GH-3 Avoid vote_message copies by using shared_ptr --- unittests/vote_processor_tests.cpp | 32 +++++++++++++++--------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/unittests/vote_processor_tests.cpp b/unittests/vote_processor_tests.cpp index 47032fdc82..60f3964cd9 100644 --- a/unittests/vote_processor_tests.cpp +++ b/unittests/vote_processor_tests.cpp @@ -104,19 +104,19 @@ auto create_test_block_state(const block_state_ptr& prev) { return bsp; } -vote_message make_empty_message(const block_id_type& id) { - vote_message vm; - vm.block_id = id; +vote_message_ptr make_empty_message(const block_id_type& id) { + vote_message_ptr vm = std::make_shared(); + vm->block_id = id; return vm; } -vote_message make_vote_message(const block_state_ptr& bsp) { - vote_message vm; - vm.block_id = bsp->id(); - vm.strong = true; +vote_message_ptr make_vote_message(const block_state_ptr& bsp) { + vote_message_ptr vm = std::make_shared(); + vm->block_id = bsp->id(); + vm->strong = true; size_t i = bsp->block_num() % bls_priv_keys.size(); - vm.finalizer_key = bls_priv_keys.at(i).get_public_key(); - vm.sig = bls_priv_keys.at(i).sign({(uint8_t*)bsp->strong_digest.data(), (uint8_t*)bsp->strong_digest.data() + bsp->strong_digest.data_size()}); + vm->finalizer_key = bls_priv_keys.at(i).get_public_key(); + vm->sig = bls_priv_keys.at(i).sign({(uint8_t*)bsp->strong_digest.data(), (uint8_t*)bsp->strong_digest.data() + bsp->strong_digest.data_size()}); return vm; } @@ -127,7 +127,7 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { uint32_t received_connection_id = 0; vote_status received_vote_status = vote_status::unknown_block; - vote_message received_vote_message{}; + vote_message_ptr received_vote_message{}; std::atomic signaled = 0; std::mutex forkdb_mtx; @@ -154,7 +154,7 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { }); { // empty fork db, block never found, never signaled - vote_message vm1 = make_empty_message(make_block_id(1)); + vote_message_ptr vm1 = make_empty_message(make_block_id(1)); signaled = 0; vp.process_vote_message(1, vm1); for (size_t i = 0; i < 50 && vp.size() < 1; ++i) { @@ -173,7 +173,7 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { auto gensis = create_genesis_block_state(); auto bsp = create_test_block_state(gensis); BOOST_CHECK_EQUAL(bsp->block_num(), 3); - vote_message m1 = make_vote_message(bsp); + vote_message_ptr m1 = make_vote_message(bsp); add_to_forkdb(bsp); vp.process_vote_message(1, m1); // duplicate ignored @@ -191,8 +191,8 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { auto gensis = create_genesis_block_state(); auto bsp = create_test_block_state(gensis); BOOST_CHECK_EQUAL(bsp->block_num(), 3); - vote_message m1 = make_vote_message(bsp); - m1.strong = false; // signed with strong_digest + vote_message_ptr m1 = make_vote_message(bsp); + m1->strong = false; // signed with strong_digest add_to_forkdb(bsp); vp.process_vote_message(1, m1); for (size_t i = 0; i < 50 && signaled.load() < 1; ++i) { @@ -208,8 +208,8 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { auto gensis = create_genesis_block_state(); auto bsp = create_test_block_state(gensis); auto bsp2 = create_test_block_state(bsp); - vote_message m1 = make_vote_message(bsp); - vote_message m2 = make_vote_message(bsp2); + vote_message_ptr m1 = make_vote_message(bsp); + vote_message_ptr m2 = make_vote_message(bsp2); vp.process_vote_message(2, m1); vp.process_vote_message(2, m2); for (size_t i = 0; i < 5; ++i) { From a6e4de8b5594eeba19a237ec0858a17a4ecea416 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 12 Apr 2024 07:41:08 -0500 Subject: [PATCH 14/40] GH-3 Fix tests now that duplicates are not signaled --- unittests/finality_test_cluster.cpp | 17 ++++++++++------- unittests/finality_test_cluster.hpp | 6 +++--- unittests/finality_tests.cpp | 8 ++++---- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/unittests/finality_test_cluster.cpp b/unittests/finality_test_cluster.cpp index 42d65a2221..5f0156591f 100644 --- a/unittests/finality_test_cluster.cpp +++ b/unittests/finality_test_cluster.cpp @@ -48,16 +48,19 @@ finality_test_cluster::finality_test_cluster() { } } -eosio::chain::vote_status finality_test_cluster::wait_on_vote(uint32_t connection_id) { +eosio::chain::vote_status finality_test_cluster::wait_on_vote(uint32_t connection_id, bool duplicate) { // wait for this node's vote to be processed + // duplicates are not signaled size_t retrys = 200; while ( (last_connection_vote != connection_id) && --retrys) { std::this_thread::sleep_for(std::chrono::milliseconds(1)); } - if (last_connection_vote != connection_id) { + if (!duplicate && last_connection_vote != connection_id) { FC_ASSERT(false, "Never received vote"); + } else if (duplicate && last_connection_vote == connection_id) { + FC_ASSERT(false, "Duplicate should not have been signaled"); } - return last_vote_status; + return duplicate ? eosio::chain::vote_status::duplicate : last_vote_status.load(); } // node0 produces a block and pushes it to node1 and node2 @@ -68,8 +71,8 @@ void finality_test_cluster::produce_and_push_block() { } // send node1's vote identified by "vote_index" in the collected votes -eosio::chain::vote_status finality_test_cluster::process_node1_vote(uint32_t vote_index, vote_mode mode) { - return process_vote( node1, vote_index, mode ); +eosio::chain::vote_status finality_test_cluster::process_node1_vote(uint32_t vote_index, vote_mode mode, bool duplicate) { + return process_vote( node1, vote_index, mode, duplicate ); } // send node1's latest vote @@ -203,7 +206,7 @@ void finality_test_cluster::setup_node(node_info& node, eosio::chain::account_na } // send a vote to node0 -eosio::chain::vote_status finality_test_cluster::process_vote(node_info& node, size_t vote_index, vote_mode mode) { +eosio::chain::vote_status finality_test_cluster::process_vote(node_info& node, size_t vote_index, vote_mode mode, bool duplicate) { std::unique_lock g(node.votes_mtx); FC_ASSERT( vote_index < node.votes.size(), "out of bound index in process_vote" ); auto& vote = node.votes[vote_index]; @@ -222,7 +225,7 @@ eosio::chain::vote_status finality_test_cluster::process_vote(node_info& node, s static uint32_t connection_id = 0; node0.node.control->process_vote_message( ++connection_id, vote ); if (eosio::chain::block_header::num_from_id(vote->block_id) > node0.node.control->last_irreversible_block_num()) - return wait_on_vote(connection_id); + return wait_on_vote(connection_id, duplicate); return eosio::chain::vote_status::unknown_block; } diff --git a/unittests/finality_test_cluster.hpp b/unittests/finality_test_cluster.hpp index 0c965e6d99..2aefb153a9 100644 --- a/unittests/finality_test_cluster.hpp +++ b/unittests/finality_test_cluster.hpp @@ -36,7 +36,7 @@ class finality_test_cluster { void produce_and_push_block(); // send node1's vote identified by "index" in the collected votes - eosio::chain::vote_status process_node1_vote(uint32_t vote_index, vote_mode mode = vote_mode::strong); + eosio::chain::vote_status process_node1_vote(uint32_t vote_index, vote_mode mode = vote_mode::strong, bool duplicate = false); // send node1's latest vote eosio::chain::vote_status process_node1_vote(vote_mode mode = vote_mode::strong); @@ -100,10 +100,10 @@ class finality_test_cluster { bool lib_advancing(node_info& node); // send "vote_index" vote on node to node0 - eosio::chain::vote_status process_vote(node_info& node, size_t vote_index, vote_mode mode); + eosio::chain::vote_status process_vote(node_info& node, size_t vote_index, vote_mode mode, bool duplicate = false); // send the latest vote on "node_index" node to node0 eosio::chain::vote_status process_vote(node_info& node, vote_mode mode); - eosio::chain::vote_status wait_on_vote(uint32_t connection_id); + eosio::chain::vote_status wait_on_vote(uint32_t connection_id, bool duplicate); }; diff --git a/unittests/finality_tests.cpp b/unittests/finality_tests.cpp index 311d27b33b..1864d4cf61 100644 --- a/unittests/finality_tests.cpp +++ b/unittests/finality_tests.cpp @@ -462,7 +462,7 @@ BOOST_AUTO_TEST_CASE(delayed_strong_weak_lost_vote) { try { BOOST_REQUIRE(!cluster.node1_lib_advancing()); // The delayed vote arrives - cluster.process_node1_vote(delayed_index); + cluster.process_node1_vote(delayed_index, finality_test_cluster::vote_mode::strong, true); cluster.produce_and_push_block(); BOOST_REQUIRE(!cluster.node2_lib_advancing()); BOOST_REQUIRE(!cluster.node1_lib_advancing()); @@ -481,9 +481,9 @@ BOOST_AUTO_TEST_CASE(duplicate_votes) { try { cluster.produce_and_push_block(); for (auto i = 0; i < 5; ++i) { - cluster.process_node1_vote(i); + cluster.process_node1_vote(i, finality_test_cluster::vote_mode::strong); // vote again to make it duplicate - BOOST_REQUIRE(cluster.process_node1_vote(i) == eosio::chain::vote_status::duplicate); + BOOST_REQUIRE(cluster.process_node1_vote(i, finality_test_cluster::vote_mode::strong, true) == eosio::chain::vote_status::duplicate); cluster.produce_and_push_block(); // verify duplicate votes do not affect LIB advancing @@ -511,7 +511,7 @@ BOOST_AUTO_TEST_CASE(unknown_proposal_votes) { try { // process the original vote. LIB should advance cluster.produce_and_push_block(); - cluster.process_node1_vote(0); + cluster.process_node1_vote(0, finality_test_cluster::vote_mode::strong, true); BOOST_REQUIRE(cluster.produce_blocks_and_verify_lib_advancing()); } FC_LOG_AND_RETHROW() } From 382f6481356aee3dba948f515b35e8c3df4a4f4d Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 12 Apr 2024 07:41:44 -0500 Subject: [PATCH 15/40] GH-3 Fix use of packed_transaction_ptr over shared_ptr --- plugins/net_plugin/net_plugin.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index e574153967..b02740e6e1 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -3189,7 +3189,8 @@ namespace eosio { auto ds = pending_message_buffer.create_datastream(); unsigned_int which{}; fc::raw::unpack( ds, which ); - packed_transaction_ptr ptr = std::make_shared(); + // shared_ptr needed here because packed_transaction_ptr is shared_ptr + std::shared_ptr ptr = std::make_shared(); fc::raw::unpack( ds, *ptr ); if( trx_in_progress_sz > def_max_trx_in_progress_size) { char reason[72]; From 508b58422b5a94e8adff37b43714b6087f0fad6b Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 12 Apr 2024 08:19:40 -0500 Subject: [PATCH 16/40] GH-3 There are three producers A,B,C if the test happens to hit this at time of C production it will not get a block until A turn. Need to wait at least 7 seconds. Used 10 here to provide a bit of buffer. --- tests/nodeos_snapshot_forked_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/nodeos_snapshot_forked_test.py b/tests/nodeos_snapshot_forked_test.py index c61372c1e4..803a667aa8 100755 --- a/tests/nodeos_snapshot_forked_test.py +++ b/tests/nodeos_snapshot_forked_test.py @@ -144,7 +144,7 @@ def getSnapshotsCount(nodeId): while nonProdNode.verifyAlive() and count > 0: # wait on prodNode 0 since it will continue to advance, since defproducera and defproducerb are its producers Print("Wait for next block") - assert prodAB.waitForNextBlock(timeout=6), "Production node AB should continue to advance, even after bridge node is killed" + assert prodAB.waitForNextBlock(timeout=10), "Production node AB should continue to advance, even after bridge node is killed" count -= 1 # schedule a snapshot that should get finalized From 4d70ab325bb84def2ba1348320e9e2045d5b5f66 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 12 Apr 2024 08:23:53 -0500 Subject: [PATCH 17/40] GH-3 Test failed on asan run because it took 55ms to load the WASM. Increase a bit. --- unittests/api_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittests/api_tests.cpp b/unittests/api_tests.cpp index f96fe14ec3..ff6f32fcf6 100644 --- a/unittests/api_tests.cpp +++ b/unittests/api_tests.cpp @@ -1226,7 +1226,7 @@ BOOST_AUTO_TEST_CASE(checktime_pause_block_deadline_not_extended_while_loading_t // WASM load times on my machine was 35ms. // Since checktime only kicks in after WASM is loaded this needs to be large enough to load the WASM, but should be // considerably lower than the 150ms max_transaction_time - BOOST_CHECK_MESSAGE( dur < 50'000, "elapsed " << dur << "us" ); + BOOST_CHECK_MESSAGE( dur < 65'000, "elapsed " << dur << "us" ); BOOST_REQUIRE_MESSAGE( dur < 150'000, "elapsed " << dur << "us" ); // should never fail BOOST_REQUIRE_EQUAL( t.validate(), true ); From 1641a0711248172714375754ed0e680347f85850 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 12 Apr 2024 13:39:37 -0500 Subject: [PATCH 18/40] GH-3 Report known lib instead of fork db root as these log statements used to be reported after the call to log_irreversible, they are called before log_irreversible. To avoid confusion by users report what is the known LIB. --- libraries/chain/controller.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 0ef3788a9b..e79fccfcd8 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1392,6 +1392,12 @@ struct controller_impl { } } + block_num_type latest_known_lib_num() const { + block_id_type irreversible_block_id = if_irreversible_block_id.load(); + block_num_type if_lib_num = block_header::num_from_id(irreversible_block_id); + return if_lib_num > 0 ? if_lib_num : fork_db_head_irreversible_blocknum(); + } + void log_irreversible() { EOS_ASSERT( fork_db_has_root(), fork_database_exception, "fork database not properly initialized" ); @@ -3226,7 +3232,7 @@ struct controller_impl { ilog("Produced block ${id}... #${n} @ ${t} signed by ${p} " "[trxs: ${count}, lib: ${lib}, confirmed: ${confs}, net: ${net}, cpu: ${cpu}, elapsed: ${et}, time: ${tt}]", ("p", new_b->producer)("id", id.str().substr(8, 16))("n", new_b->block_num())("t", new_b->timestamp) - ("count", new_b->transactions.size())("lib", fork_db_root_block_num())("net", br.total_net_usage) + ("count", new_b->transactions.size())("lib", latest_known_lib_num())("net", br.total_net_usage) ("cpu", br.total_cpu_usage_us)("et", br.total_elapsed_time)("tt", br.total_time)("confs", new_b->confirmed)); } @@ -3384,7 +3390,7 @@ struct controller_impl { ilog("Received block ${id}... #${n} @ ${t} signed by ${p} " // "Received" instead of "Applied" so it matches existing log output "[trxs: ${count}, lib: ${lib}, net: ${net}, cpu: ${cpu}, elapsed: ${elapsed}, time: ${time}, latency: ${latency} ms]", ("p", bsp->producer())("id", bsp->id().str().substr(8, 16))("n", bsp->block_num())("t", bsp->timestamp()) - ("count", bsp->block->transactions.size())("lib", fork_db_root_block_num()) + ("count", bsp->block->transactions.size())("lib", latest_known_lib_num()) ("net", br.total_net_usage)("cpu", br.total_cpu_usage_us) ("elapsed", br.total_elapsed_time)("time", br.total_time)("latency", (now - bsp->timestamp()).count() / 1000)); const auto& hb_id = chain_head.id(); @@ -3393,7 +3399,7 @@ struct controller_impl { ilog("Block not applied to head ${id}... #${n} @ ${t} signed by ${p} " "[trxs: ${count}, lib: ${lib}, net: ${net}, cpu: ${cpu}, elapsed: ${elapsed}, time: ${time}, latency: ${latency} ms]", ("p", hb->producer)("id", hb_id.str().substr(8, 16))("n", hb->block_num())("t", hb->timestamp) - ("count", hb->transactions.size())("lib", fork_db_root_block_num()) + ("count", hb->transactions.size())("lib", latest_known_lib_num()) ("net", br.total_net_usage)("cpu", br.total_cpu_usage_us)("elapsed", br.total_elapsed_time)("time", br.total_time) ("latency", (now - hb->timestamp).count() / 1000)); } From 4b4eca60d96cbad631f6b2d15fbc0f86d7d76b80 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 15 Apr 2024 15:00:08 -0500 Subject: [PATCH 19/40] GH-3 Change default --vote-threads to 0 to be disabled by default. EOS_ASSERT if configured as a producer with vote processing disabled. --- libraries/chain/include/eosio/chain/config.hpp | 1 - libraries/chain/include/eosio/chain/controller.hpp | 2 +- plugins/chain_plugin/chain_plugin.cpp | 8 +++++++- .../include/eosio/chain_plugin/chain_plugin.hpp | 2 ++ plugins/producer_plugin/producer_plugin.cpp | 3 +++ tests/TestHarness/Cluster.py | 2 ++ 6 files changed, 15 insertions(+), 3 deletions(-) diff --git a/libraries/chain/include/eosio/chain/config.hpp b/libraries/chain/include/eosio/chain/config.hpp index 74af7b59ca..9dd10a1b85 100644 --- a/libraries/chain/include/eosio/chain/config.hpp +++ b/libraries/chain/include/eosio/chain/config.hpp @@ -80,7 +80,6 @@ const static uint16_t default_max_auth_depth = 6; const static uint32_t default_sig_cpu_bill_pct = 50 * percent_1; // billable percentage of signature recovery const static uint32_t default_produce_block_offset_ms = 450; const static uint16_t default_controller_thread_pool_size = 2; -const static uint16_t default_vote_thread_pool_size = 4; const static uint32_t default_max_variable_signature_length = 16384u; const static uint32_t default_max_action_return_value_size = 256; diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index dcd1a18303..1978b1c21b 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -89,7 +89,7 @@ namespace eosio::chain { uint64_t state_guard_size = chain::config::default_state_guard_size; uint32_t sig_cpu_bill_pct = chain::config::default_sig_cpu_bill_pct; uint16_t chain_thread_pool_size = chain::config::default_controller_thread_pool_size; - uint16_t vote_thread_pool_size = chain::config::default_vote_thread_pool_size; + uint16_t vote_thread_pool_size = 0; bool read_only = false; bool force_all_checks = false; bool disable_replay_opts = false; diff --git a/plugins/chain_plugin/chain_plugin.cpp b/plugins/chain_plugin/chain_plugin.cpp index 526bd18104..324e861ea4 100644 --- a/plugins/chain_plugin/chain_plugin.cpp +++ b/plugins/chain_plugin/chain_plugin.cpp @@ -158,6 +158,7 @@ class chain_plugin_impl { std::filesystem::path state_dir; bool readonly = false; flat_map loaded_checkpoints; + bool accept_votes = false; bool accept_transactions = false; bool api_accept_transactions = true; bool account_queries_enabled = false; @@ -291,7 +292,7 @@ void chain_plugin::set_program_options(options_description& cli, options_descrip "Percentage of actual signature recovery cpu to bill. Whole number percentages, e.g. 50 for 50%") ("chain-threads", bpo::value()->default_value(config::default_controller_thread_pool_size), "Number of worker threads in controller thread pool") - ("vote-threads", bpo::value()->default_value(config::default_vote_thread_pool_size), + ("vote-threads", bpo::value()->default_value(0), "Number of worker threads in vote processor thread pool. Voting disabled if set to 0 (votes are not propagatged on P2P network).") ("contracts-console", bpo::bool_switch()->default_value(false), "print contract's output to console") @@ -645,6 +646,7 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) { "vote-threads ${num} must be greater than 1 or 0. " "Voting disabled if set to 0 (votes are not propagatged on P2P network).", ("num", chain_config->vote_thread_pool_size) ); + accept_votes = chain_config->vote_thread_pool_size > 0; } chain_config->sig_cpu_bill_pct = options.at("signature-cpu-billable-pct").as(); @@ -1232,6 +1234,10 @@ void chain_plugin::enable_accept_transactions() { my->enable_accept_transactions(); } +bool chain_plugin::accept_votes() const { + return my->accept_votes; +} + void chain_plugin_impl::log_guard_exception(const chain::guard_exception&e ) { if (e.code() == chain::database_guard_exception::code_value) { 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 45576abe0e..33441f652c 100644 --- a/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp +++ b/plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp @@ -987,6 +987,8 @@ class chain_plugin : public plugin { // set true by other plugins if any plugin allows transactions bool accept_transactions() const; void enable_accept_transactions(); + // true if vote processing is enabled + bool accept_votes() const; static void handle_guard_exception(const chain::guard_exception& e); diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 91f23c1798..2a339b73ad 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -1338,6 +1338,9 @@ void producer_plugin_impl::plugin_startup() { EOS_ASSERT(_producers.empty() || chain_plug->accept_transactions(), plugin_config_exception, "node cannot have any producer-name configured because no block production is possible with no [api|p2p]-accepted-transactions"); + EOS_ASSERT(_producers.empty() || chain_plug->accept_votes(), plugin_config_exception, + "node cannot have any producer-name configured because --vote-threads is defaulted to 0. Enable vote processing for block production."); + chain.set_node_finalizer_keys(_finalizer_keys); _accepted_block_connection.emplace(chain.accepted_block().connect([this](const block_signal_params& t) { diff --git a/tests/TestHarness/Cluster.py b/tests/TestHarness/Cluster.py index 292adaae21..1f4a11b7e0 100644 --- a/tests/TestHarness/Cluster.py +++ b/tests/TestHarness/Cluster.py @@ -256,6 +256,8 @@ def launch(self, pnodes=1, unstartedNodes=0, totalNodes=1, prodCount=21, topo="m if self.staging: argsArr.append("--nogen") nodeosArgs="" + if "--vote-threads" not in extraNodeosArgs: + nodeosArgs += " --vote-threads 3" if "--max-transaction-time" not in extraNodeosArgs: nodeosArgs += " --max-transaction-time -1" if "--abi-serializer-max-time-ms" not in extraNodeosArgs: From 895cef74edcb2ad6ca8ffd0399d52425fb5c7b00 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 15 Apr 2024 16:47:34 -0500 Subject: [PATCH 20/40] GH-3 --vote-threads needed for all producers --- docs/01_nodeos/02_usage/01_nodeos-configuration.md | 1 + .../03_development-environment/00_local-single-node-testnet.md | 2 +- tests/cli_test.py | 2 +- tests/gelf_test.py | 2 +- tests/p2p_no_listen_test.py | 2 ++ tests/plugin_http_api_test.py | 2 +- tests/resource_monitor_plugin_test.py | 2 +- tests/split_blocklog_replay_test.py | 2 +- tutorials/bios-boot-tutorial/bios-boot-tutorial.py | 1 + 9 files changed, 10 insertions(+), 6 deletions(-) diff --git a/docs/01_nodeos/02_usage/01_nodeos-configuration.md b/docs/01_nodeos/02_usage/01_nodeos-configuration.md index 4db966a433..68ec2db596 100644 --- a/docs/01_nodeos/02_usage/01_nodeos-configuration.md +++ b/docs/01_nodeos/02_usage/01_nodeos-configuration.md @@ -22,6 +22,7 @@ The example below shows a typical usage of `nodeos` when starting a block produc ```sh nodeos \ -e -p eosio \ + --vote-threads 3 \ --data-dir /users/mydir/eosio/data \ --config-dir /users/mydir/eosio/config \ --plugin eosio::producer_plugin \ diff --git a/docs/01_nodeos/02_usage/03_development-environment/00_local-single-node-testnet.md b/docs/01_nodeos/02_usage/03_development-environment/00_local-single-node-testnet.md index e94c037d02..b326868b0f 100644 --- a/docs/01_nodeos/02_usage/03_development-environment/00_local-single-node-testnet.md +++ b/docs/01_nodeos/02_usage/03_development-environment/00_local-single-node-testnet.md @@ -32,7 +32,7 @@ Open one "terminal" window and perform the following steps: Start your own single-node blockchain with this single command: ```sh -nodeos -e -p eosio --plugin eosio::chain_api_plugin --plugin eosio::history_api_plugin +nodeos -e -p eosio --vote-threads 3 --plugin eosio::chain_api_plugin --plugin eosio::history_api_plugin ``` [[info | Nodeos Minimal Options]] diff --git a/tests/cli_test.py b/tests/cli_test.py index 74e730e60c..4505e0dad2 100755 --- a/tests/cli_test.py +++ b/tests/cli_test.py @@ -355,7 +355,7 @@ def abi_file_with_nodeos_test(): os.makedirs(data_dir, exist_ok=True) walletMgr = WalletMgr(True) walletMgr.launch() - cmd = "./programs/nodeos/nodeos -e -p eosio --plugin eosio::trace_api_plugin --trace-no-abis --plugin eosio::producer_plugin --plugin eosio::producer_api_plugin --plugin eosio::chain_api_plugin --plugin eosio::chain_plugin --plugin eosio::http_plugin --access-control-allow-origin=* --http-validate-host=false --max-transaction-time=-1 --resource-monitor-not-shutdown-on-threshold-exceeded " + "--data-dir " + data_dir + " --config-dir " + data_dir + cmd = "./programs/nodeos/nodeos -e -p eosio --vote-threads 2 --plugin eosio::trace_api_plugin --trace-no-abis --plugin eosio::producer_plugin --plugin eosio::producer_api_plugin --plugin eosio::chain_api_plugin --plugin eosio::chain_plugin --plugin eosio::http_plugin --access-control-allow-origin=* --http-validate-host=false --max-transaction-time=-1 --resource-monitor-not-shutdown-on-threshold-exceeded " + "--data-dir " + data_dir + " --config-dir " + data_dir node = Node('localhost', 8888, nodeId, data_dir=Path(data_dir), config_dir=Path(data_dir), cmd=shlex.split(cmd), launch_time=datetime.datetime.now().strftime('%Y_%m_%d_%H_%M_%S'), walletMgr=walletMgr) time.sleep(5) node.waitForBlock(1) diff --git a/tests/gelf_test.py b/tests/gelf_test.py index d0ed7f1888..21746c63cb 100755 --- a/tests/gelf_test.py +++ b/tests/gelf_test.py @@ -86,7 +86,7 @@ def gelfServer(stop): data_dir = Path(Utils.getNodeDataDir(node_id)) config_dir = Path(Utils.getNodeConfigDir(node_id)) -start_nodeos_cmd = shlex.split(f"{Utils.EosServerPath} -e -p eosio --data-dir={data_dir} --config-dir={config_dir}") +start_nodeos_cmd = shlex.split(f"{Utils.EosServerPath} -e -p eosio --vote-threads 2 --data-dir={data_dir} --config-dir={config_dir}") if os.path.exists(data_dir): shutil.rmtree(data_dir) os.makedirs(data_dir) diff --git a/tests/p2p_no_listen_test.py b/tests/p2p_no_listen_test.py index 76b3c76886..d669fcce1b 100755 --- a/tests/p2p_no_listen_test.py +++ b/tests/p2p_no_listen_test.py @@ -33,6 +33,8 @@ '-e', '-p', 'eosio', + '--vote-threads', + '3', '--p2p-listen-endpoint', '', '--plugin', diff --git a/tests/plugin_http_api_test.py b/tests/plugin_http_api_test.py index f9628847cc..687788f76c 100755 --- a/tests/plugin_http_api_test.py +++ b/tests/plugin_http_api_test.py @@ -97,7 +97,7 @@ def startEnv(self) : "--p2p-peer-address localhost:9011 --resource-monitor-not-shutdown-on-threshold-exceeded ") % (self.data_dir, self.config_dir, self.data_dir, "\'*\'", "false") nodeos_flags += category_config.nodeosArgs() - start_nodeos_cmd = ("%s -e -p eosio %s %s ") % (Utils.EosServerPath, nodeos_plugins, nodeos_flags) + start_nodeos_cmd = ("%s -e -p eosio --vote-threads 2 %s %s ") % (Utils.EosServerPath, nodeos_plugins, nodeos_flags) self.nodeos = Node(TestHelper.LOCAL_HOST, TestHelper.DEFAULT_PORT, self.node_id, self.data_dir, self.config_dir, shlex.split(start_nodeos_cmd), walletMgr=self.keosd) time.sleep(self.sleep_s*2) self.nodeos.waitForBlock(1, timeout=30) diff --git a/tests/resource_monitor_plugin_test.py b/tests/resource_monitor_plugin_test.py index 4370d529dc..0036bf188c 100755 --- a/tests/resource_monitor_plugin_test.py +++ b/tests/resource_monitor_plugin_test.py @@ -77,7 +77,7 @@ def prepareDirectories(): def runNodeos(extraNodeosArgs, myTimeout): """Startup nodeos, wait for timeout (before forced shutdown) and collect output.""" if debug: Print("Launching nodeos process.") - cmd="programs/nodeos/nodeos --config-dir rsmStaging/etc -e -p eosio --plugin eosio::chain_api_plugin --data-dir " + dataDir + " " + cmd="programs/nodeos/nodeos --config-dir rsmStaging/etc -e -p eosio --vote-threads 2 --plugin eosio::chain_api_plugin --data-dir " + dataDir + " " cmd=cmd + extraNodeosArgs if debug: Print("cmd: %s" % (cmd)) diff --git a/tests/split_blocklog_replay_test.py b/tests/split_blocklog_replay_test.py index ae7c24ffd8..dc9c8d178e 100755 --- a/tests/split_blocklog_replay_test.py +++ b/tests/split_blocklog_replay_test.py @@ -17,7 +17,7 @@ os.makedirs(config_dir) try: - start_nodeos_cmd = f"{Utils.EosServerPath} -e -p eosio --data-dir={data_dir} --config-dir={config_dir} --blocks-log-stride 10" \ + start_nodeos_cmd = f"{Utils.EosServerPath} -e -p eosio --vote-threads 2 --data-dir={data_dir} --config-dir={config_dir} --blocks-log-stride 10" \ " --plugin=eosio::http_plugin --plugin=eosio::chain_api_plugin --http-server-address=localhost:8888" nodeos.launchCmd(start_nodeos_cmd, node_id) diff --git a/tutorials/bios-boot-tutorial/bios-boot-tutorial.py b/tutorials/bios-boot-tutorial/bios-boot-tutorial.py index 8822546307..474d34c52a 100755 --- a/tutorials/bios-boot-tutorial/bios-boot-tutorial.py +++ b/tutorials/bios-boot-tutorial/bios-boot-tutorial.py @@ -118,6 +118,7 @@ def startNode(nodeIndex, account): ' --p2p-max-nodes-per-host ' + str(maxClients) + ' --enable-stale-production' ' --producer-name ' + account['name'] + + ' --vote-threads 3' ' --signature-provider ' + account['pub'] + '=KEY:' + account['pvt'] + ' --plugin eosio::http_plugin' ' --plugin eosio::chain_api_plugin' From c3bdd8aef515c4eb83825d3e503124ee6aef0df1 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 16 Apr 2024 07:23:45 -0500 Subject: [PATCH 21/40] GH-12 Add vote threads needed for unittests --- libraries/testing/include/eosio/testing/tester.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/testing/include/eosio/testing/tester.hpp b/libraries/testing/include/eosio/testing/tester.hpp index 6ca6dbf922..210ff4b67a 100644 --- a/libraries/testing/include/eosio/testing/tester.hpp +++ b/libraries/testing/include/eosio/testing/tester.hpp @@ -426,6 +426,7 @@ namespace eosio { namespace testing { cfg.state_guard_size = 0; cfg.contracts_console = true; cfg.eosvmoc_config.cache_size = 1024*1024*8; + cfg.vote_thread_pool_size = 3; // don't enforce OC compilation subject limits for tests, // particularly EOS EVM tests may run over those limits From e4010d212f9f1352d9d92518c2d3e4e2260cad6b Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 16 Apr 2024 07:24:01 -0500 Subject: [PATCH 22/40] GH-12 Add vote threads --- tests/test_read_only_trx.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_read_only_trx.cpp b/tests/test_read_only_trx.cpp index 56b684da86..a9ee294106 100644 --- a/tests/test_read_only_trx.cpp +++ b/tests/test_read_only_trx.cpp @@ -108,7 +108,7 @@ void test_trxs_common(std::vector& specific_args, bool test_disable fc::logger::get(DEFAULT_LOGGER).set_log_level(fc::log_level::debug); std::vector argv = { "test", // dummy executible name - "-p", "eosio", "-e", // actual arguments follow + "-p", "eosio", "--vote-threads", "3", "-e", // actual arguments follow "--data-dir", temp_dir_str.c_str(), "--config-dir", temp_dir_str.c_str(), "--max-transaction-time=100", From bf8fe18aa2e7c15c1839ebcadddd8ccb629d2498 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 16 Apr 2024 07:33:26 -0500 Subject: [PATCH 23/40] GH-12 Add vote threads to tests --- plugins/producer_plugin/test/test_disallow_delayed_trx.cpp | 2 +- plugins/producer_plugin/test/test_options.cpp | 2 +- plugins/producer_plugin/test/test_trx_full.cpp | 2 +- tests/db_modes_test.sh | 2 +- tests/test_snapshot_scheduler.cpp | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/producer_plugin/test/test_disallow_delayed_trx.cpp b/plugins/producer_plugin/test/test_disallow_delayed_trx.cpp index 3723970ffc..df850afd48 100644 --- a/plugins/producer_plugin/test/test_disallow_delayed_trx.cpp +++ b/plugins/producer_plugin/test/test_disallow_delayed_trx.cpp @@ -55,7 +55,7 @@ BOOST_AUTO_TEST_CASE(delayed_trx) { fc::logger::get(DEFAULT_LOGGER).set_log_level(fc::log_level::debug); std::vector argv = {"test", "--data-dir", temp_dir_str.c_str(), "--config-dir", temp_dir_str.c_str(), - "-p", "eosio", "-e", "--disable-subjective-p2p-billing=true" }; + "-p", "eosio", "-e", "--vote-threads", "3", "--disable-subjective-p2p-billing=true" }; app->initialize( argv.size(), (char**) &argv[0] ); app->startup(); plugin_promise.set_value( diff --git a/plugins/producer_plugin/test/test_options.cpp b/plugins/producer_plugin/test/test_options.cpp index 3fe429b6a9..072cc27ec5 100644 --- a/plugins/producer_plugin/test/test_options.cpp +++ b/plugins/producer_plugin/test/test_options.cpp @@ -37,7 +37,7 @@ BOOST_AUTO_TEST_CASE(state_dir) { "--data-dir", temp_dir_str.c_str(), "--state-dir", custom_state_dir_str.c_str(), "--config-dir", temp_dir_str.c_str(), - "-p", "eosio", "-e" }; + "-p", "eosio", "--vote-threads", "3", "-e" }; app->initialize( argv.size(), (char**) &argv[0] ); app->startup(); plugin_promise.set_value( {app->find_plugin(), app->find_plugin()} ); diff --git a/plugins/producer_plugin/test/test_trx_full.cpp b/plugins/producer_plugin/test/test_trx_full.cpp index 1a51570515..e827f44d6e 100644 --- a/plugins/producer_plugin/test/test_trx_full.cpp +++ b/plugins/producer_plugin/test/test_trx_full.cpp @@ -112,7 +112,7 @@ BOOST_AUTO_TEST_CASE(producer) { fc::logger::get(DEFAULT_LOGGER).set_log_level(fc::log_level::debug); std::vector argv = {"test", "--data-dir", temp_dir_str.c_str(), "--config-dir", temp_dir_str.c_str(), - "-p", "eosio", "-e", "--disable-subjective-p2p-billing=true" }; + "-p", "eosio", "-e", "--vote-threads", "3", "--disable-subjective-p2p-billing=true" }; app->initialize( argv.size(), (char**) &argv[0] ); app->startup(); plugin_promise.set_value( diff --git a/tests/db_modes_test.sh b/tests/db_modes_test.sh index 6b5fd20fe3..a105ad1634 100755 --- a/tests/db_modes_test.sh +++ b/tests/db_modes_test.sh @@ -32,7 +32,7 @@ EOSIO_STUFF_DIR=$(mktemp -d) trap "rm -rf $EOSIO_STUFF_DIR" EXIT NODEOS_LAUNCH_PARAMS="./programs/nodeos/nodeos --resource-monitor-not-shutdown-on-threshold-exceeded -d $EOSIO_STUFF_DIR --config-dir $EOSIO_STUFF_DIR \ --chain-state-db-size-mb 8 --chain-state-db-guard-size-mb 0 \ --e -peosio" +-e -peosio --vote-threads 3" run_nodeos() { if (( $VERBOSE == 0 )); then diff --git a/tests/test_snapshot_scheduler.cpp b/tests/test_snapshot_scheduler.cpp index a03add72bb..2ebf170dc1 100644 --- a/tests/test_snapshot_scheduler.cpp +++ b/tests/test_snapshot_scheduler.cpp @@ -68,7 +68,7 @@ BOOST_AUTO_TEST_CASE(snapshot_scheduler_test) { fc::logger::get(DEFAULT_LOGGER).set_log_level(fc::log_level::debug); std::vector argv = {"test", "--data-dir", temp.c_str(), "--config-dir", temp.c_str(), - "-p", "eosio", "-e"}; + "-p", "eosio", "--vote-threads", "3", "-e"}; app->initialize(argv.size(), (char**) &argv[0]); app->startup(); From 8bfbe0a355b43de8f656054af0df3a7efd99f45b Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 16 Apr 2024 11:40:42 -0500 Subject: [PATCH 24/40] GH-3 Modify named_thread_pool to be a no-op if given 0 for num_threads. --- libraries/chain/controller.cpp | 11 +++++------ .../chain/include/eosio/chain/thread_utils.hpp | 16 ++++++++++------ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index e79fccfcd8..9e74d85e63 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1199,16 +1199,15 @@ struct controller_impl { my_finalizers(fc::time_point::now(), cfg.finalizers_dir / "safety.dat"), wasmif( conf.wasm_runtime, conf.eosvmoc_tierup, db, conf.state_dir, conf.eosvmoc_config, !conf.profile_accounts.empty() ) { + assert(cfg.chain_thread_pool_size > 0); thread_pool.start( cfg.chain_thread_pool_size, [this]( const fc::exception& e ) { elog( "Exception in chain thread pool, exiting: ${e}", ("e", e.to_detail_string()) ); if( shutdown ) shutdown(); } ); - if (cfg.vote_thread_pool_size > 0) { - vote_processor.start(cfg.vote_thread_pool_size, [this]( const fc::exception& e ) { - elog( "Exception in vote thread pool, exiting: ${e}", ("e", e.to_detail_string()) ); - if( shutdown ) shutdown(); - } ); - } + vote_processor.start(cfg.vote_thread_pool_size, [this]( const fc::exception& e ) { + elog( "Exception in vote thread pool, exiting: ${e}", ("e", e.to_detail_string()) ); + if( shutdown ) shutdown(); + } ); set_activation_handler(); set_activation_handler(); diff --git a/libraries/chain/include/eosio/chain/thread_utils.hpp b/libraries/chain/include/eosio/chain/thread_utils.hpp index b4e4d5a673..3838b380e4 100644 --- a/libraries/chain/include/eosio/chain/thread_utils.hpp +++ b/libraries/chain/include/eosio/chain/thread_utils.hpp @@ -108,7 +108,7 @@ namespace eosio { namespace chain { /// Blocks until all threads are created and completed their init function, or an exception is thrown /// during thread startup or an init function. Exceptions thrown during these stages are rethrown from start() /// but some threads might still have been started. Calling stop() after such a failure is safe. - /// @param num_threads is number of threads spawned + /// @param num_threads is number of threads spawned, if 0 then no threads are spawned and stop() is a no-op. /// @param on_except is the function to call if io_context throws an exception, is called from thread pool thread. /// if an empty function then logs and rethrows exception on thread which will terminate. Not called /// for exceptions during the init function (such exceptions are rethrown from start()) @@ -116,6 +116,8 @@ namespace eosio { namespace chain { /// @throw assert_exception if already started and not stopped. void start( size_t num_threads, on_except_t on_except, init_t init = {} ) { FC_ASSERT( !_ioc_work, "Thread pool already started" ); + if (num_threads == 0) + return; _ioc_work.emplace( boost::asio::make_work_guard( _ioc ) ); _ioc.restart(); _thread_pool.reserve( num_threads ); @@ -142,12 +144,14 @@ namespace eosio { namespace chain { /// destroy work guard, stop io_context, join thread_pool void stop() { - _ioc_work.reset(); - _ioc.stop(); - for( auto& t : _thread_pool ) { - t.join(); + if (_thread_pool.size() > 0) { + _ioc_work.reset(); + _ioc.stop(); + for( auto& t : _thread_pool ) { + t.join(); + } + _thread_pool.clear(); } - _thread_pool.clear(); } private: From 117f02d205ce5f114b150812d6319f2951f74cb2 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 16 Apr 2024 11:57:29 -0500 Subject: [PATCH 25/40] GH-3 Check for stopped in inner loop --- libraries/chain/include/eosio/chain/vote_processor.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index 793705acbc..68f599ab0a 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -161,6 +161,8 @@ class vote_processor_t { g.lock(); } for (auto i = idx.begin(); i != idx.end();) { + if (stopped) + break; auto& vt = *i; block_state_ptr bsp = fetch_block_func(vt.id()); if (bsp) { From 027d2780b4979cdba2f5a56ef1e8e9a8d4211901 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 16 Apr 2024 12:04:46 -0500 Subject: [PATCH 26/40] GH-3 Add better descriptions --- libraries/chain/controller.cpp | 6 +++--- libraries/chain/include/eosio/chain/controller.hpp | 1 + libraries/chain/include/eosio/chain/vote_processor.hpp | 5 ++++- plugins/chain_plugin/chain_plugin.cpp | 4 ++-- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 9e74d85e63..c172e1dc08 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1393,8 +1393,8 @@ struct controller_impl { block_num_type latest_known_lib_num() const { block_id_type irreversible_block_id = if_irreversible_block_id.load(); - block_num_type if_lib_num = block_header::num_from_id(irreversible_block_id); - return if_lib_num > 0 ? if_lib_num : fork_db_head_irreversible_blocknum(); + block_num_type savanna_lib_num = block_header::num_from_id(irreversible_block_id); + return savanna_lib_num > 0 ? savanna_lib_num : fork_db_head_irreversible_blocknum(); } void log_irreversible() { @@ -3606,7 +3606,7 @@ struct controller_impl { // net plugin subscribed to this signal. it will broadcast the vote message on receiving the signal emit(voted_block, std::tuple{uint32_t{0}, vote_status::success, std::cref(vote)}); - // also aggregate our own vote into the pending_qc for this block. + // also aggregate our own vote into the pending_qc for this block, 0 connection_id indicates our own vote process_vote_message(0, vote); }); } diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index 1978b1c21b..490b69b540 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -58,6 +58,7 @@ namespace eosio::chain { using trx_meta_cache_lookup = std::function; using block_signal_params = std::tuple; + // connection_id, vote result status, vote_message processed using vote_signal_params = std::tuple; enum class db_read_mode { diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index 68f599ab0a..47489b207e 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -15,7 +15,9 @@ namespace eosio { namespace chain { * Process votes in a dedicated thread pool. */ class vote_processor_t { - static constexpr size_t max_votes_per_connection = 2500; // 3000 is less than 1MB per connection + // Even 3000 vote structs are less than 1MB per connection. + // 2500 is should never be reached unless a specific connection is sending garbage. + static constexpr size_t max_votes_per_connection = 2500; static constexpr std::chrono::milliseconds block_wait_time{10}; struct by_block_num; @@ -209,6 +211,7 @@ class vote_processor_t { } void process_vote_message(uint32_t connection_id, const vote_message_ptr& msg) { + assert(msg); boost::asio::post(thread_pool.get_executor(), [this, connection_id, msg] { std::unique_lock g(mtx); if (++num_messages[connection_id] > max_votes_per_connection) { diff --git a/plugins/chain_plugin/chain_plugin.cpp b/plugins/chain_plugin/chain_plugin.cpp index 324e861ea4..fcc202ac6b 100644 --- a/plugins/chain_plugin/chain_plugin.cpp +++ b/plugins/chain_plugin/chain_plugin.cpp @@ -293,7 +293,7 @@ void chain_plugin::set_program_options(options_description& cli, options_descrip ("chain-threads", bpo::value()->default_value(config::default_controller_thread_pool_size), "Number of worker threads in controller thread pool") ("vote-threads", bpo::value()->default_value(0), - "Number of worker threads in vote processor thread pool. Voting disabled if set to 0 (votes are not propagatged on P2P network).") + "Number of worker threads in vote processor thread pool. If set to 0, voting disabled, votes are not propagatged on P2P network.") ("contracts-console", bpo::bool_switch()->default_value(false), "print contract's output to console") ("deep-mind", bpo::bool_switch()->default_value(false), @@ -643,7 +643,7 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) { if( options.count( "vote-threads" )) { chain_config->vote_thread_pool_size = options.at( "vote-threads" ).as(); EOS_ASSERT( chain_config->vote_thread_pool_size > 1 || chain_config->vote_thread_pool_size == 0, plugin_config_exception, - "vote-threads ${num} must be greater than 1 or 0. " + "vote-threads ${num} must be greater than 1, or equal to 0 to disable. " "Voting disabled if set to 0 (votes are not propagatged on P2P network).", ("num", chain_config->vote_thread_pool_size) ); accept_votes = chain_config->vote_thread_pool_size > 0; From 8f26a50e4f3864dd5ac348ef5da45b27e410894b Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 16 Apr 2024 13:22:24 -0500 Subject: [PATCH 27/40] GH-3 Default vote-threads to 4 when a block producer --- docs/01_nodeos/02_usage/01_nodeos-configuration.md | 1 - .../00_local-single-node-testnet.md | 2 +- libraries/chain/include/eosio/chain/config.hpp | 1 + libraries/chain/include/eosio/chain/thread_utils.hpp | 1 + .../chain/include/eosio/chain/vote_processor.hpp | 11 +++++++++-- plugins/chain_plugin/chain_plugin.cpp | 11 +++++++---- plugins/producer_plugin/producer_plugin.cpp | 3 --- .../test/test_disallow_delayed_trx.cpp | 2 +- plugins/producer_plugin/test/test_options.cpp | 2 +- plugins/producer_plugin/test/test_trx_full.cpp | 2 +- tests/TestHarness/Cluster.py | 2 -- tests/cli_test.py | 2 +- tests/db_modes_test.sh | 2 +- tests/gelf_test.py | 2 +- tests/p2p_no_listen_test.py | 2 -- tests/plugin_http_api_test.py | 2 +- tests/resource_monitor_plugin_test.py | 2 +- tests/split_blocklog_replay_test.py | 2 +- tests/test_read_only_trx.cpp | 2 +- tests/test_snapshot_scheduler.cpp | 2 +- tutorials/bios-boot-tutorial/bios-boot-tutorial.py | 1 - 21 files changed, 30 insertions(+), 27 deletions(-) diff --git a/docs/01_nodeos/02_usage/01_nodeos-configuration.md b/docs/01_nodeos/02_usage/01_nodeos-configuration.md index 68ec2db596..4db966a433 100644 --- a/docs/01_nodeos/02_usage/01_nodeos-configuration.md +++ b/docs/01_nodeos/02_usage/01_nodeos-configuration.md @@ -22,7 +22,6 @@ The example below shows a typical usage of `nodeos` when starting a block produc ```sh nodeos \ -e -p eosio \ - --vote-threads 3 \ --data-dir /users/mydir/eosio/data \ --config-dir /users/mydir/eosio/config \ --plugin eosio::producer_plugin \ diff --git a/docs/01_nodeos/02_usage/03_development-environment/00_local-single-node-testnet.md b/docs/01_nodeos/02_usage/03_development-environment/00_local-single-node-testnet.md index b326868b0f..e94c037d02 100644 --- a/docs/01_nodeos/02_usage/03_development-environment/00_local-single-node-testnet.md +++ b/docs/01_nodeos/02_usage/03_development-environment/00_local-single-node-testnet.md @@ -32,7 +32,7 @@ Open one "terminal" window and perform the following steps: Start your own single-node blockchain with this single command: ```sh -nodeos -e -p eosio --vote-threads 3 --plugin eosio::chain_api_plugin --plugin eosio::history_api_plugin +nodeos -e -p eosio --plugin eosio::chain_api_plugin --plugin eosio::history_api_plugin ``` [[info | Nodeos Minimal Options]] diff --git a/libraries/chain/include/eosio/chain/config.hpp b/libraries/chain/include/eosio/chain/config.hpp index 9dd10a1b85..74af7b59ca 100644 --- a/libraries/chain/include/eosio/chain/config.hpp +++ b/libraries/chain/include/eosio/chain/config.hpp @@ -80,6 +80,7 @@ const static uint16_t default_max_auth_depth = 6; const static uint32_t default_sig_cpu_bill_pct = 50 * percent_1; // billable percentage of signature recovery const static uint32_t default_produce_block_offset_ms = 450; const static uint16_t default_controller_thread_pool_size = 2; +const static uint16_t default_vote_thread_pool_size = 4; const static uint32_t default_max_variable_signature_length = 16384u; const static uint32_t default_max_action_return_value_size = 256; diff --git a/libraries/chain/include/eosio/chain/thread_utils.hpp b/libraries/chain/include/eosio/chain/thread_utils.hpp index 3838b380e4..81dbb24dbe 100644 --- a/libraries/chain/include/eosio/chain/thread_utils.hpp +++ b/libraries/chain/include/eosio/chain/thread_utils.hpp @@ -143,6 +143,7 @@ namespace eosio { namespace chain { } /// destroy work guard, stop io_context, join thread_pool + /// not thread safe, expected to only be called from thread that called start() void stop() { if (_thread_pool.size() > 0) { _ioc_work.reset(); diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index 47489b207e..8d8417cc16 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -58,7 +58,7 @@ class vote_processor_t { std::map num_messages; std::atomic lib{0}; - std::atomic stopped{false}; + std::atomic stopped{true}; named_thread_pool thread_pool; private: @@ -136,7 +136,10 @@ class vote_processor_t { } void start(size_t num_threads, decltype(thread_pool)::on_except_t&& on_except) { - assert(num_threads > 1); // need at least two as one is used for coordinatation + if (num_threads == 0) + return; + + stopped = false; thread_pool.start( num_threads, std::move(on_except)); // one coordinator thread @@ -210,7 +213,11 @@ class vote_processor_t { lib = block_num; } + /// called from net threads and controller's thread pool + /// msg is ignored vote_processor not start()ed void process_vote_message(uint32_t connection_id, const vote_message_ptr& msg) { + if (stopped) + return; assert(msg); boost::asio::post(thread_pool.get_executor(), [this, connection_id, msg] { std::unique_lock g(mtx); diff --git a/plugins/chain_plugin/chain_plugin.cpp b/plugins/chain_plugin/chain_plugin.cpp index fcc202ac6b..b34039e096 100644 --- a/plugins/chain_plugin/chain_plugin.cpp +++ b/plugins/chain_plugin/chain_plugin.cpp @@ -292,8 +292,8 @@ void chain_plugin::set_program_options(options_description& cli, options_descrip "Percentage of actual signature recovery cpu to bill. Whole number percentages, e.g. 50 for 50%") ("chain-threads", bpo::value()->default_value(config::default_controller_thread_pool_size), "Number of worker threads in controller thread pool") - ("vote-threads", bpo::value()->default_value(0), - "Number of worker threads in vote processor thread pool. If set to 0, voting disabled, votes are not propagatged on P2P network.") + ("vote-threads", bpo::value(), + "Number of worker threads in vote processor thread pool. If set to 0, voting disabled, votes are not propagatged on P2P network. Defaults to 4 on producer nodes.") ("contracts-console", bpo::bool_switch()->default_value(false), "print contract's output to console") ("deep-mind", bpo::bool_switch()->default_value(false), @@ -640,8 +640,11 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) { "chain-threads ${num} must be greater than 0", ("num", chain_config->chain_thread_pool_size) ); } - if( options.count( "vote-threads" )) { - chain_config->vote_thread_pool_size = options.at( "vote-threads" ).as(); + if (options.count("producer-name") || options.count("vote-threads")) { + chain_config->vote_thread_pool_size = options.count("vote-threads") ? options.at("vote-threads").as() : 0; + if (chain_config->vote_thread_pool_size == 0 && options.count("producer-name")) { + chain_config->vote_thread_pool_size = config::default_vote_thread_pool_size; + } EOS_ASSERT( chain_config->vote_thread_pool_size > 1 || chain_config->vote_thread_pool_size == 0, plugin_config_exception, "vote-threads ${num} must be greater than 1, or equal to 0 to disable. " "Voting disabled if set to 0 (votes are not propagatged on P2P network).", diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 2a339b73ad..91f23c1798 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -1338,9 +1338,6 @@ void producer_plugin_impl::plugin_startup() { EOS_ASSERT(_producers.empty() || chain_plug->accept_transactions(), plugin_config_exception, "node cannot have any producer-name configured because no block production is possible with no [api|p2p]-accepted-transactions"); - EOS_ASSERT(_producers.empty() || chain_plug->accept_votes(), plugin_config_exception, - "node cannot have any producer-name configured because --vote-threads is defaulted to 0. Enable vote processing for block production."); - chain.set_node_finalizer_keys(_finalizer_keys); _accepted_block_connection.emplace(chain.accepted_block().connect([this](const block_signal_params& t) { diff --git a/plugins/producer_plugin/test/test_disallow_delayed_trx.cpp b/plugins/producer_plugin/test/test_disallow_delayed_trx.cpp index df850afd48..3723970ffc 100644 --- a/plugins/producer_plugin/test/test_disallow_delayed_trx.cpp +++ b/plugins/producer_plugin/test/test_disallow_delayed_trx.cpp @@ -55,7 +55,7 @@ BOOST_AUTO_TEST_CASE(delayed_trx) { fc::logger::get(DEFAULT_LOGGER).set_log_level(fc::log_level::debug); std::vector argv = {"test", "--data-dir", temp_dir_str.c_str(), "--config-dir", temp_dir_str.c_str(), - "-p", "eosio", "-e", "--vote-threads", "3", "--disable-subjective-p2p-billing=true" }; + "-p", "eosio", "-e", "--disable-subjective-p2p-billing=true" }; app->initialize( argv.size(), (char**) &argv[0] ); app->startup(); plugin_promise.set_value( diff --git a/plugins/producer_plugin/test/test_options.cpp b/plugins/producer_plugin/test/test_options.cpp index 072cc27ec5..3fe429b6a9 100644 --- a/plugins/producer_plugin/test/test_options.cpp +++ b/plugins/producer_plugin/test/test_options.cpp @@ -37,7 +37,7 @@ BOOST_AUTO_TEST_CASE(state_dir) { "--data-dir", temp_dir_str.c_str(), "--state-dir", custom_state_dir_str.c_str(), "--config-dir", temp_dir_str.c_str(), - "-p", "eosio", "--vote-threads", "3", "-e" }; + "-p", "eosio", "-e" }; app->initialize( argv.size(), (char**) &argv[0] ); app->startup(); plugin_promise.set_value( {app->find_plugin(), app->find_plugin()} ); diff --git a/plugins/producer_plugin/test/test_trx_full.cpp b/plugins/producer_plugin/test/test_trx_full.cpp index e827f44d6e..1a51570515 100644 --- a/plugins/producer_plugin/test/test_trx_full.cpp +++ b/plugins/producer_plugin/test/test_trx_full.cpp @@ -112,7 +112,7 @@ BOOST_AUTO_TEST_CASE(producer) { fc::logger::get(DEFAULT_LOGGER).set_log_level(fc::log_level::debug); std::vector argv = {"test", "--data-dir", temp_dir_str.c_str(), "--config-dir", temp_dir_str.c_str(), - "-p", "eosio", "-e", "--vote-threads", "3", "--disable-subjective-p2p-billing=true" }; + "-p", "eosio", "-e", "--disable-subjective-p2p-billing=true" }; app->initialize( argv.size(), (char**) &argv[0] ); app->startup(); plugin_promise.set_value( diff --git a/tests/TestHarness/Cluster.py b/tests/TestHarness/Cluster.py index 1f4a11b7e0..292adaae21 100644 --- a/tests/TestHarness/Cluster.py +++ b/tests/TestHarness/Cluster.py @@ -256,8 +256,6 @@ def launch(self, pnodes=1, unstartedNodes=0, totalNodes=1, prodCount=21, topo="m if self.staging: argsArr.append("--nogen") nodeosArgs="" - if "--vote-threads" not in extraNodeosArgs: - nodeosArgs += " --vote-threads 3" if "--max-transaction-time" not in extraNodeosArgs: nodeosArgs += " --max-transaction-time -1" if "--abi-serializer-max-time-ms" not in extraNodeosArgs: diff --git a/tests/cli_test.py b/tests/cli_test.py index 4505e0dad2..74e730e60c 100755 --- a/tests/cli_test.py +++ b/tests/cli_test.py @@ -355,7 +355,7 @@ def abi_file_with_nodeos_test(): os.makedirs(data_dir, exist_ok=True) walletMgr = WalletMgr(True) walletMgr.launch() - cmd = "./programs/nodeos/nodeos -e -p eosio --vote-threads 2 --plugin eosio::trace_api_plugin --trace-no-abis --plugin eosio::producer_plugin --plugin eosio::producer_api_plugin --plugin eosio::chain_api_plugin --plugin eosio::chain_plugin --plugin eosio::http_plugin --access-control-allow-origin=* --http-validate-host=false --max-transaction-time=-1 --resource-monitor-not-shutdown-on-threshold-exceeded " + "--data-dir " + data_dir + " --config-dir " + data_dir + cmd = "./programs/nodeos/nodeos -e -p eosio --plugin eosio::trace_api_plugin --trace-no-abis --plugin eosio::producer_plugin --plugin eosio::producer_api_plugin --plugin eosio::chain_api_plugin --plugin eosio::chain_plugin --plugin eosio::http_plugin --access-control-allow-origin=* --http-validate-host=false --max-transaction-time=-1 --resource-monitor-not-shutdown-on-threshold-exceeded " + "--data-dir " + data_dir + " --config-dir " + data_dir node = Node('localhost', 8888, nodeId, data_dir=Path(data_dir), config_dir=Path(data_dir), cmd=shlex.split(cmd), launch_time=datetime.datetime.now().strftime('%Y_%m_%d_%H_%M_%S'), walletMgr=walletMgr) time.sleep(5) node.waitForBlock(1) diff --git a/tests/db_modes_test.sh b/tests/db_modes_test.sh index a105ad1634..6b5fd20fe3 100755 --- a/tests/db_modes_test.sh +++ b/tests/db_modes_test.sh @@ -32,7 +32,7 @@ EOSIO_STUFF_DIR=$(mktemp -d) trap "rm -rf $EOSIO_STUFF_DIR" EXIT NODEOS_LAUNCH_PARAMS="./programs/nodeos/nodeos --resource-monitor-not-shutdown-on-threshold-exceeded -d $EOSIO_STUFF_DIR --config-dir $EOSIO_STUFF_DIR \ --chain-state-db-size-mb 8 --chain-state-db-guard-size-mb 0 \ --e -peosio --vote-threads 3" +-e -peosio" run_nodeos() { if (( $VERBOSE == 0 )); then diff --git a/tests/gelf_test.py b/tests/gelf_test.py index 21746c63cb..d0ed7f1888 100755 --- a/tests/gelf_test.py +++ b/tests/gelf_test.py @@ -86,7 +86,7 @@ def gelfServer(stop): data_dir = Path(Utils.getNodeDataDir(node_id)) config_dir = Path(Utils.getNodeConfigDir(node_id)) -start_nodeos_cmd = shlex.split(f"{Utils.EosServerPath} -e -p eosio --vote-threads 2 --data-dir={data_dir} --config-dir={config_dir}") +start_nodeos_cmd = shlex.split(f"{Utils.EosServerPath} -e -p eosio --data-dir={data_dir} --config-dir={config_dir}") if os.path.exists(data_dir): shutil.rmtree(data_dir) os.makedirs(data_dir) diff --git a/tests/p2p_no_listen_test.py b/tests/p2p_no_listen_test.py index d669fcce1b..76b3c76886 100755 --- a/tests/p2p_no_listen_test.py +++ b/tests/p2p_no_listen_test.py @@ -33,8 +33,6 @@ '-e', '-p', 'eosio', - '--vote-threads', - '3', '--p2p-listen-endpoint', '', '--plugin', diff --git a/tests/plugin_http_api_test.py b/tests/plugin_http_api_test.py index 687788f76c..f9628847cc 100755 --- a/tests/plugin_http_api_test.py +++ b/tests/plugin_http_api_test.py @@ -97,7 +97,7 @@ def startEnv(self) : "--p2p-peer-address localhost:9011 --resource-monitor-not-shutdown-on-threshold-exceeded ") % (self.data_dir, self.config_dir, self.data_dir, "\'*\'", "false") nodeos_flags += category_config.nodeosArgs() - start_nodeos_cmd = ("%s -e -p eosio --vote-threads 2 %s %s ") % (Utils.EosServerPath, nodeos_plugins, nodeos_flags) + start_nodeos_cmd = ("%s -e -p eosio %s %s ") % (Utils.EosServerPath, nodeos_plugins, nodeos_flags) self.nodeos = Node(TestHelper.LOCAL_HOST, TestHelper.DEFAULT_PORT, self.node_id, self.data_dir, self.config_dir, shlex.split(start_nodeos_cmd), walletMgr=self.keosd) time.sleep(self.sleep_s*2) self.nodeos.waitForBlock(1, timeout=30) diff --git a/tests/resource_monitor_plugin_test.py b/tests/resource_monitor_plugin_test.py index 0036bf188c..4370d529dc 100755 --- a/tests/resource_monitor_plugin_test.py +++ b/tests/resource_monitor_plugin_test.py @@ -77,7 +77,7 @@ def prepareDirectories(): def runNodeos(extraNodeosArgs, myTimeout): """Startup nodeos, wait for timeout (before forced shutdown) and collect output.""" if debug: Print("Launching nodeos process.") - cmd="programs/nodeos/nodeos --config-dir rsmStaging/etc -e -p eosio --vote-threads 2 --plugin eosio::chain_api_plugin --data-dir " + dataDir + " " + cmd="programs/nodeos/nodeos --config-dir rsmStaging/etc -e -p eosio --plugin eosio::chain_api_plugin --data-dir " + dataDir + " " cmd=cmd + extraNodeosArgs if debug: Print("cmd: %s" % (cmd)) diff --git a/tests/split_blocklog_replay_test.py b/tests/split_blocklog_replay_test.py index dc9c8d178e..ae7c24ffd8 100755 --- a/tests/split_blocklog_replay_test.py +++ b/tests/split_blocklog_replay_test.py @@ -17,7 +17,7 @@ os.makedirs(config_dir) try: - start_nodeos_cmd = f"{Utils.EosServerPath} -e -p eosio --vote-threads 2 --data-dir={data_dir} --config-dir={config_dir} --blocks-log-stride 10" \ + start_nodeos_cmd = f"{Utils.EosServerPath} -e -p eosio --data-dir={data_dir} --config-dir={config_dir} --blocks-log-stride 10" \ " --plugin=eosio::http_plugin --plugin=eosio::chain_api_plugin --http-server-address=localhost:8888" nodeos.launchCmd(start_nodeos_cmd, node_id) diff --git a/tests/test_read_only_trx.cpp b/tests/test_read_only_trx.cpp index a9ee294106..56b684da86 100644 --- a/tests/test_read_only_trx.cpp +++ b/tests/test_read_only_trx.cpp @@ -108,7 +108,7 @@ void test_trxs_common(std::vector& specific_args, bool test_disable fc::logger::get(DEFAULT_LOGGER).set_log_level(fc::log_level::debug); std::vector argv = { "test", // dummy executible name - "-p", "eosio", "--vote-threads", "3", "-e", // actual arguments follow + "-p", "eosio", "-e", // actual arguments follow "--data-dir", temp_dir_str.c_str(), "--config-dir", temp_dir_str.c_str(), "--max-transaction-time=100", diff --git a/tests/test_snapshot_scheduler.cpp b/tests/test_snapshot_scheduler.cpp index 2ebf170dc1..a03add72bb 100644 --- a/tests/test_snapshot_scheduler.cpp +++ b/tests/test_snapshot_scheduler.cpp @@ -68,7 +68,7 @@ BOOST_AUTO_TEST_CASE(snapshot_scheduler_test) { fc::logger::get(DEFAULT_LOGGER).set_log_level(fc::log_level::debug); std::vector argv = {"test", "--data-dir", temp.c_str(), "--config-dir", temp.c_str(), - "-p", "eosio", "--vote-threads", "3", "-e"}; + "-p", "eosio", "-e"}; app->initialize(argv.size(), (char**) &argv[0]); app->startup(); diff --git a/tutorials/bios-boot-tutorial/bios-boot-tutorial.py b/tutorials/bios-boot-tutorial/bios-boot-tutorial.py index 474d34c52a..8822546307 100755 --- a/tutorials/bios-boot-tutorial/bios-boot-tutorial.py +++ b/tutorials/bios-boot-tutorial/bios-boot-tutorial.py @@ -118,7 +118,6 @@ def startNode(nodeIndex, account): ' --p2p-max-nodes-per-host ' + str(maxClients) + ' --enable-stale-production' ' --producer-name ' + account['name'] + - ' --vote-threads 3' ' --signature-provider ' + account['pub'] + '=KEY:' + account['pvt'] + ' --plugin eosio::http_plugin' ' --plugin eosio::chain_api_plugin' From 6cc68200e69600688881241626c6b47ae88d4c31 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 16 Apr 2024 13:50:02 -0500 Subject: [PATCH 28/40] GH-3 Simplify by adding a vote_signal_t type --- libraries/chain/controller.cpp | 16 ++++++++-------- .../chain/include/eosio/chain/controller.hpp | 3 ++- .../chain/include/eosio/chain/vote_processor.hpp | 15 ++++++--------- unittests/vote_processor_tests.cpp | 2 +- 4 files changed, 17 insertions(+), 19 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index c172e1dc08..643ed231ec 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -947,14 +947,14 @@ struct controller_impl { signal accepted_block; signal irreversible_block; signal)> applied_transaction; - signal voted_block; + vote_signal_t voted_block; - vote_processor_t vote_processor{voted_block, - [this](const block_id_type& id) -> block_state_ptr { - return fork_db.apply_s([&](const auto& forkdb) { - return forkdb.get_block(id); - }); - }}; + vote_processor_t vote_processor{voted_block, + [this](const block_id_type& id) -> block_state_ptr { + return fork_db.apply_s([&](const auto& forkdb) { + return forkdb.get_block(id); + }); + }}; int64_t set_proposed_producers( vector producers ); int64_t set_proposed_producers_legacy( vector producers ); @@ -5548,7 +5548,7 @@ signal& controller::accepted_block_header() { signal& controller::accepted_block() { return my->accepted_block; } signal& controller::irreversible_block() { return my->irreversible_block; } signal)>& controller::applied_transaction() { return my->applied_transaction; } -signal& controller::voted_block() { return my->voted_block; } +vote_signal_t& controller::voted_block() { return my->voted_block; } chain_id_type controller::extract_chain_id(snapshot_reader& snapshot) { chain_snapshot_header header; diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index 490b69b540..e8d1a5d1e3 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -60,6 +60,7 @@ namespace eosio::chain { using block_signal_params = std::tuple; // connection_id, vote result status, vote_message processed using vote_signal_params = std::tuple; + using vote_signal_t = signal; enum class db_read_mode { HEAD, @@ -377,7 +378,7 @@ namespace eosio::chain { signal& irreversible_block(); signal)>& applied_transaction(); // Unlike other signals, voted_block is signaled from other threads than the main thread. - signal& voted_block(); + vote_signal_t& voted_block(); const apply_handler* find_apply_handler( account_name contract, scope_name scope, action_name act )const; wasm_interface& get_wasm_interface(); diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index 8d8417cc16..1f53ca6212 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -9,7 +10,7 @@ #include #include -namespace eosio { namespace chain { +namespace eosio::chain { /** * Process votes in a dedicated thread pool. @@ -32,8 +33,6 @@ class vote_processor_t { block_num_type block_num() const { return block_header::num_from_id(msg->block_id); } }; - using vote_signal_type = decltype(controller({},chain_id_type::empty_chain_id()).voted_block()); - using vote_index_type = boost::multi_index_container< vote, indexed_by< ordered_non_unique, @@ -48,7 +47,7 @@ class vote_processor_t { using fetch_block_func_t = std::function; - vote_signal_type& vote_signal; + vote_signal_t& vote_signal; fetch_block_func_t fetch_block_func; std::mutex mtx; @@ -119,7 +118,7 @@ class vote_processor_t { } public: - explicit vote_processor_t(vote_signal_type& vote_signal, fetch_block_func_t&& get_block) + explicit vote_processor_t(vote_signal_t& vote_signal, fetch_block_func_t&& get_block) : vote_signal(vote_signal) , fetch_block_func(get_block) {} @@ -148,9 +147,7 @@ class vote_processor_t { while (!stopped) { std::unique_lock g(mtx); cv.wait(g, [&]() { - if (!index.empty() || stopped) - return true; - return false; + return !index.empty() || stopped; }); if (stopped) break; @@ -240,4 +237,4 @@ class vote_processor_t { }; -} } //eosio::chain +} // namespace eosio::chain diff --git a/unittests/vote_processor_tests.cpp b/unittests/vote_processor_tests.cpp index 60f3964cd9..223e89de83 100644 --- a/unittests/vote_processor_tests.cpp +++ b/unittests/vote_processor_tests.cpp @@ -123,7 +123,7 @@ vote_message_ptr make_vote_message(const block_state_ptr& bsp) { BOOST_AUTO_TEST_SUITE(vote_processor_tests) BOOST_AUTO_TEST_CASE( vote_processor_test ) { - boost::signals2::signal voted_block; + vote_signal_t voted_block; uint32_t received_connection_id = 0; vote_status received_vote_status = vote_status::unknown_block; From 166a5809704dae0597d45b62d343bff72cf38abd Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 16 Apr 2024 14:02:02 -0500 Subject: [PATCH 29/40] GH-3 Use chain pluging accept_votes to init p2p_accept_votes --- plugins/net_plugin/net_plugin.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 2050139595..cb67c5c56a 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -4278,7 +4278,6 @@ namespace eosio { resp_expected_period = def_resp_expected_wait; max_nodes_per_host = options.at( "p2p-max-nodes-per-host" ).as(); p2p_accept_transactions = options.at( "p2p-accept-transactions" ).as(); - p2p_accept_votes = options.at("vote-threads").as() != 0; use_socket_read_watermark = options.at( "use-socket-read-watermark" ).as(); keepalive_interval = std::chrono::milliseconds( options.at( "p2p-keepalive-interval-ms" ).as() ); @@ -4427,6 +4426,8 @@ namespace eosio { "***********************************\n" ); } + p2p_accept_votes = chain_plug->accept_votes(); + std::vector listen_addresses = p2p_addresses; EOS_ASSERT( p2p_addresses.size() == p2p_server_addresses.size(), chain::plugin_config_exception, "" ); From 83434d13d502d1f1012f74b3ec00247594d75c7c Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 16 Apr 2024 14:36:55 -0500 Subject: [PATCH 30/40] GH-3 Add vote-threads to all nodes so bridge nodes process votes --- tests/TestHarness/Cluster.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/TestHarness/Cluster.py b/tests/TestHarness/Cluster.py index 292adaae21..1f4a11b7e0 100644 --- a/tests/TestHarness/Cluster.py +++ b/tests/TestHarness/Cluster.py @@ -256,6 +256,8 @@ def launch(self, pnodes=1, unstartedNodes=0, totalNodes=1, prodCount=21, topo="m if self.staging: argsArr.append("--nogen") nodeosArgs="" + if "--vote-threads" not in extraNodeosArgs: + nodeosArgs += " --vote-threads 3" if "--max-transaction-time" not in extraNodeosArgs: nodeosArgs += " --max-transaction-time -1" if "--abi-serializer-max-time-ms" not in extraNodeosArgs: From 042ce3002eeb8169b2d17e4d238e30956b1dda63 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 17 Apr 2024 14:10:06 -0500 Subject: [PATCH 31/40] GH-3 Simplify vote_processor by processing votes on a first-come-first-serve basis. --- libraries/chain/controller.cpp | 2 + .../include/eosio/chain/vote_processor.hpp | 220 ++++++++++-------- plugins/chain_plugin/chain_plugin.cpp | 5 +- unittests/vote_processor_tests.cpp | 51 ++-- 4 files changed, 160 insertions(+), 118 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 643ed231ec..157f191ea8 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -3765,6 +3765,8 @@ struct controller_impl { if (conf.terminate_at_block == 0 || bsp->block_num() <= conf.terminate_at_block) { forkdb.add(bsp, mark_valid_t::no, ignore_duplicate_t::yes); + if constexpr (savanna_mode) + vote_processor.notify_new_block(); } return block_handle{bsp}; diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index 1f53ca6212..430faec8e2 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -19,14 +19,16 @@ class vote_processor_t { // Even 3000 vote structs are less than 1MB per connection. // 2500 is should never be reached unless a specific connection is sending garbage. static constexpr size_t max_votes_per_connection = 2500; - static constexpr std::chrono::milliseconds block_wait_time{10}; + // If we have not processed a vote in this amount of time, give up on it. + static constexpr fc::microseconds too_old = fc::seconds(5); struct by_block_num; struct by_connection; - struct by_vote; + struct by_last_received; struct vote { uint32_t connection_id; + fc::time_point received; vote_message_ptr msg; const block_id_type& id() const { return msg->block_id; } @@ -35,13 +37,9 @@ class vote_processor_t { using vote_index_type = boost::multi_index_container< vote, indexed_by< - ordered_non_unique, - composite_key, - const_mem_fun - >, composite_key_compare< std::greater<>, sha256_less > // greater for block_num - >, - ordered_non_unique< tag, member > + ordered_non_unique< tag, const_mem_fun, std::greater<> >, + ordered_non_unique< tag, member >, + ordered_non_unique< tag, member > > >; @@ -51,12 +49,14 @@ class vote_processor_t { fetch_block_func_t fetch_block_func; std::mutex mtx; - std::condition_variable cv; vote_index_type index; + block_state_ptr last_bsp; // connection, count of messages std::map num_messages; std::atomic lib{0}; + std::atomic largest_known_block_num{0}; + std::atomic queued_votes{0}; std::atomic stopped{true}; named_thread_pool thread_pool; @@ -83,53 +83,111 @@ class vote_processor_t { } } + // called with unlocked mtx void emit(uint32_t connection_id, vote_status status, const vote_message_ptr& msg) { if (connection_id != 0) { // this nodes vote was already signaled - emit( vote_signal, std::tuple{connection_id, status, std::cref(msg)} ); + if (status != vote_status::duplicate) { // don't bother emitting duplicates + emit( vote_signal, std::tuple{connection_id, status, std::cref(msg)} ); + } } } + // called with locked mtx void remove_connection(uint32_t connection_id) { auto& idx = index.get(); idx.erase(idx.lower_bound(connection_id), idx.upper_bound(connection_id)); } + // called with locked mtx void remove_before_lib() { auto& idx = index.get(); idx.erase(idx.lower_bound(lib.load()), idx.end()); // descending // don't decrement num_messages as too many before lib should be considered an error } - bool remove_all_for_block(auto& idx, auto& it, const block_id_type& id) { - while (it != idx.end() && it->id() == id) { - if (auto& num = num_messages[it->connection_id]; num != 0) - --num; + // called with locked mtx + void remove_too_old() { + auto& idx = index.get(); + fc::time_point vote_too_old = fc::time_point::now() - too_old; + idx.erase(idx.lower_bound(fc::time_point::min()), idx.upper_bound(vote_too_old)); + // don't decrement num_messages as too many that are too old should be considered an error + } - it = idx.erase(it); + // called with locked mtx + void queue_for_later(uint32_t connection_id, const vote_message_ptr& msg) { + fc::time_point now = fc::time_point::now(); + remove_before_lib(); + remove_too_old(); + index.insert(vote{.connection_id = connection_id, .received = now, .msg = msg}); + } + + // called with locked mtx + void process_any_queued_for_later(std::unique_lock& g) { + if (index.empty()) + return; + remove_too_old(); + remove_before_lib(); + auto& idx = index.get(); + std::vector unprocessed; + unprocessed.reserve(std::min(21u, idx.size())); // maybe increase if we increase # of finalizers from 21 + for (auto i = idx.begin(); i != idx.end();) { + if (stopped) + return; + vote v = *i; + idx.erase(i); + auto bsp = get_block(i->msg->block_id, g); + // g is unlocked + if (bsp) { + vote_status s = bsp->aggregate_vote(*v.msg); + emit(v.connection_id, s, v.msg); + + g.lock(); + if (auto& num = num_messages[v.connection_id]; num != 0) + --num; + } else { + unprocessed.push_back(std::move(v)); + g.lock(); + } + i = idx.begin(); // need to update since unlocked in loop + } + for (auto& v : unprocessed) { + index.insert(std::move(v)); } - return it == idx.end(); } - bool skip_all_for_block(auto& idx, auto& it, const block_id_type& id) { - while (it != idx.end() && it->id() == id) { - ++it; + // called with locked mtx, returns with unlocked mtx + block_state_ptr get_block(const block_id_type& id, std::unique_lock& g) { + block_state_ptr bsp; + if (last_bsp && last_bsp->id() == id) { + bsp = last_bsp; } - return it == idx.end(); + g.unlock(); + + if (!bsp) { + bsp = fetch_block_func(id); + if (bsp) { + g.lock(); + last_bsp = bsp; + largest_known_block_num = std::max(bsp->block_num(), largest_known_block_num.load()); + g.unlock(); + } + } + return bsp; } public: explicit vote_processor_t(vote_signal_t& vote_signal, fetch_block_func_t&& get_block) : vote_signal(vote_signal) , fetch_block_func(get_block) - {} + { + assert(get_block); + } ~vote_processor_t() { stopped = true; - std::lock_guard g(mtx); - cv.notify_one(); } - size_t size() { + size_t index_size() { std::lock_guard g(mtx); return index.size(); } @@ -140,98 +198,70 @@ class vote_processor_t { stopped = false; thread_pool.start( num_threads, std::move(on_except)); - - // one coordinator thread - boost::asio::post(thread_pool.get_executor(), [&]() { - block_id_type not_in_forkdb_id{}; - while (!stopped) { - std::unique_lock g(mtx); - cv.wait(g, [&]() { - return !index.empty() || stopped; - }); - if (stopped) - break; - remove_before_lib(); - if (index.empty()) { - num_messages.clear(); - continue; - } - auto& idx = index.get(); - if (auto i = idx.begin(); i != idx.end() && not_in_forkdb_id == i->id()) { // same block as last while loop - g.unlock(); - std::this_thread::sleep_for(block_wait_time); - g.lock(); - } - for (auto i = idx.begin(); i != idx.end();) { - if (stopped) - break; - auto& vt = *i; - block_state_ptr bsp = fetch_block_func(vt.id()); - if (bsp) { - if (!bsp->is_proper_svnn_block()) { - if (remove_all_for_block(idx, i, bsp->id())) - break; - continue; - } - auto iter_of_bsp = i; - std::vector to_process; - to_process.reserve(std::min(21u, idx.size())); // increase if we increase # of finalizers from 21 - for(; i != idx.end() && bsp->id() == i->id(); ++i) { - // although it is the highest contention on block state pending mutex posting all of the same bsp, - // the highest priority is processing votes for this block state. - to_process.push_back(*i); - } - bool should_break = remove_all_for_block(idx, iter_of_bsp, bsp->id()); - g.unlock(); // do not hold lock when posting - for (auto& v : to_process) { - boost::asio::post(thread_pool.get_executor(), [this, bsp, v=std::move(v)]() { - vote_status s = bsp->aggregate_vote(*v.msg); - if (s != vote_status::duplicate) { // don't bother emitting duplicates - emit(v.connection_id, s, v.msg); - } - }); - } - if (should_break) - break; - g.lock(); - i = idx.begin(); - } else { - not_in_forkdb_id = vt.id(); - if (skip_all_for_block(idx, i, i->id())) - break; - } - } - } - dlog("Exiting vote processor coordinator thread"); - }); } + // called from main thread void notify_lib(block_num_type block_num) { lib = block_num; } + // called from net threads + void notify_new_block() { + // would require a mtx lock to check if index is empty, post check to thread_pool + boost::asio::post(thread_pool.get_executor(), [this] { + std::unique_lock g(mtx); + process_any_queued_for_later(g); + }); + } + /// called from net threads and controller's thread pool /// msg is ignored vote_processor not start()ed void process_vote_message(uint32_t connection_id, const vote_message_ptr& msg) { if (stopped) return; assert(msg); + block_num_type msg_block_num = block_header::num_from_id(msg->block_id); + if (msg_block_num <= lib.load(std::memory_order_relaxed)) + return; + ++queued_votes; boost::asio::post(thread_pool.get_executor(), [this, connection_id, msg] { + if (stopped) + return; + auto num_queued_votes = --queued_votes; + bool reset_num_messages = num_queued_votes == 0; // caught up, so clear num_messages + if (block_header::num_from_id(msg->block_id) <= lib.load(std::memory_order_relaxed)) + return; // ignore any votes lower than lib std::unique_lock g(mtx); + if (reset_num_messages) + num_messages.clear(); if (++num_messages[connection_id] > max_votes_per_connection) { - // consider the connection invalid, remove all votes of connection - // don't clear num_messages[connection_id] so we keep reporting max_exceeded until index is drained remove_connection(connection_id); g.unlock(); + // drop, too many from this connection to process, consider connection invalid + // don't clear num_messages[connection_id] so we keep reporting max_exceeded until index is drained elog("Exceeded max votes per connection for ${c}", ("c", connection_id)); emit(connection_id, vote_status::max_exceeded, msg); - } else if (block_header::num_from_id(msg->block_id) < lib.load(std::memory_order_relaxed)) { - // ignore } else { - index.insert(vote{.connection_id = connection_id, .msg = msg}); - cv.notify_one(); + block_state_ptr bsp = get_block(msg->block_id, g); + // g is unlocked + + if (!bsp) { + // queue up for later processing + g.lock(); + queue_for_later(connection_id, msg); + } else { + vote_status s = bsp->aggregate_vote(*msg); + emit(connection_id, s, msg); + + g.lock(); + if (auto& num = num_messages[connection_id]; num != 0) + --num; + + process_any_queued_for_later(g); + } } + }); } diff --git a/plugins/chain_plugin/chain_plugin.cpp b/plugins/chain_plugin/chain_plugin.cpp index b34039e096..193691a051 100644 --- a/plugins/chain_plugin/chain_plugin.cpp +++ b/plugins/chain_plugin/chain_plugin.cpp @@ -644,11 +644,8 @@ void chain_plugin_impl::plugin_initialize(const variables_map& options) { chain_config->vote_thread_pool_size = options.count("vote-threads") ? options.at("vote-threads").as() : 0; if (chain_config->vote_thread_pool_size == 0 && options.count("producer-name")) { chain_config->vote_thread_pool_size = config::default_vote_thread_pool_size; + ilog("Setting vote-threads to ${n} on producing node", ("n", chain_config->vote_thread_pool_size)); } - EOS_ASSERT( chain_config->vote_thread_pool_size > 1 || chain_config->vote_thread_pool_size == 0, plugin_config_exception, - "vote-threads ${num} must be greater than 1, or equal to 0 to disable. " - "Voting disabled if set to 0 (votes are not propagatged on P2P network).", - ("num", chain_config->vote_thread_pool_size) ); accept_votes = chain_config->vote_thread_pool_size > 0; } diff --git a/unittests/vote_processor_tests.cpp b/unittests/vote_processor_tests.cpp index 223e89de83..7c45fab7fb 100644 --- a/unittests/vote_processor_tests.cpp +++ b/unittests/vote_processor_tests.cpp @@ -157,16 +157,17 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { vote_message_ptr vm1 = make_empty_message(make_block_id(1)); signaled = 0; vp.process_vote_message(1, vm1); - for (size_t i = 0; i < 50 && vp.size() < 1; ++i) { + for (size_t i = 0; i < 50 && vp.index_size() < 1; ++i) { std::this_thread::sleep_for(std::chrono::milliseconds{5}); } - BOOST_CHECK(vp.size() == 1); + BOOST_TEST(vp.index_size() == 1); // move lib past block vp.notify_lib(2); - for (size_t i = 0; i < 50 && vp.size() > 0; ++i) { + vp.notify_new_block(); + for (size_t i = 0; i < 50 && vp.index_size() > 0; ++i) { std::this_thread::sleep_for(std::chrono::milliseconds{5}); } - BOOST_CHECK(vp.size() == 0); + BOOST_TEST(vp.index_size() == 0); } { // process a valid vote signaled = 0; @@ -181,10 +182,10 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { for (size_t i = 0; i < 50 && signaled.load() < 1; ++i) { std::this_thread::sleep_for(std::chrono::milliseconds{5}); } - BOOST_CHECK(signaled.load() == 1); - BOOST_CHECK(1 == received_connection_id); - BOOST_CHECK(vote_status::success == received_vote_status); - BOOST_CHECK(m1 == received_vote_message); + BOOST_TEST(signaled.load() == 1); + BOOST_TEST(1 == received_connection_id); + BOOST_TEST(vote_status::success == received_vote_status); + BOOST_TEST(m1 == received_vote_message); } { // process an invalid signature vote signaled = 0; @@ -198,10 +199,10 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { for (size_t i = 0; i < 50 && signaled.load() < 1; ++i) { std::this_thread::sleep_for(std::chrono::milliseconds{5}); } - BOOST_CHECK(signaled.load() == 1); - BOOST_CHECK(1 == received_connection_id); - BOOST_CHECK(vote_status::invalid_signature == received_vote_status); - BOOST_CHECK(m1 == received_vote_message); + BOOST_TEST(signaled.load() == 1); + BOOST_TEST(1 == received_connection_id); + BOOST_TEST(vote_status::invalid_signature == received_vote_status); + BOOST_TEST(m1 == received_vote_message); } { // process two diff block votes signaled = 0; @@ -211,21 +212,33 @@ BOOST_AUTO_TEST_CASE( vote_processor_test ) { vote_message_ptr m1 = make_vote_message(bsp); vote_message_ptr m2 = make_vote_message(bsp2); vp.process_vote_message(2, m1); - vp.process_vote_message(2, m2); + vp.process_vote_message(3, m2); for (size_t i = 0; i < 5; ++i) { - if (vp.size() == 2) break; + if (vp.index_size() == 2) break; std::this_thread::sleep_for(std::chrono::milliseconds{5}); } - BOOST_CHECK(vp.size() == 2); + BOOST_TEST(vp.index_size() == 2); + std::this_thread::sleep_for(std::chrono::milliseconds{5}); // no votes for awhile + BOOST_TEST(signaled.load() == 0); add_to_forkdb(bsp); + vp.notify_new_block(); + for (size_t i = 0; i < 50 && signaled.load() < 2; ++i) { + std::this_thread::sleep_for(std::chrono::milliseconds{5}); + } + BOOST_TEST(signaled.load() == 1); + BOOST_TEST(2 == received_connection_id); + BOOST_TEST(vote_status::success == received_vote_status); + BOOST_CHECK(m1 == received_vote_message); + add_to_forkdb(bsp2); + vp.notify_new_block(); for (size_t i = 0; i < 50 && signaled.load() < 2; ++i) { std::this_thread::sleep_for(std::chrono::milliseconds{5}); } - BOOST_CHECK(signaled.load() == 2); - BOOST_CHECK(2 == received_connection_id); - BOOST_CHECK(vote_status::success == received_vote_status); - BOOST_CHECK(m1 == received_vote_message || m2 == received_vote_message); + BOOST_TEST(signaled.load() == 2); + BOOST_TEST(3 == received_connection_id); + BOOST_TEST(vote_status::success == received_vote_status); + BOOST_CHECK(m2 == received_vote_message); } } From d80def5d428f8272eb013b9d430b593ac9c8c958 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 17 Apr 2024 14:36:29 -0500 Subject: [PATCH 32/40] GH-3 Fix use of iterator after erase --- libraries/chain/include/eosio/chain/vote_processor.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index 430faec8e2..2287774fe9 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -135,7 +135,7 @@ class vote_processor_t { return; vote v = *i; idx.erase(i); - auto bsp = get_block(i->msg->block_id, g); + auto bsp = get_block(v.msg->block_id, g); // g is unlocked if (bsp) { vote_status s = bsp->aggregate_vote(*v.msg); From 317dd61face4e7da7f06ab30ae744622dbdf179a Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 18 Apr 2024 14:55:00 -0500 Subject: [PATCH 33/40] GH-3 Remove unneeded if --- libraries/chain/controller.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 157f191ea8..d7fd7fb948 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -3573,9 +3573,7 @@ struct controller_impl { // called from net threads and controller's thread pool void process_vote_message( uint32_t connection_id, const vote_message_ptr& vote ) { - if (conf.vote_thread_pool_size > 0) { - vote_processor.process_vote_message(connection_id, vote); - } + vote_processor.process_vote_message(connection_id, vote); } bool node_has_voted_if_finalizer(const block_id_type& id) const { From 955a72ae358e56838bc6e9095b3679d78100e9a2 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 18 Apr 2024 14:55:59 -0500 Subject: [PATCH 34/40] GH-3 Move emit to controller.hpp and use in vote_processor --- libraries/chain/controller.cpp | 31 ----------------- .../chain/include/eosio/chain/controller.hpp | 33 ++++++++++++++++++- .../include/eosio/chain/vote_processor.hpp | 24 +------------- 3 files changed, 33 insertions(+), 55 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index d7fd7fb948..696b94f491 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1253,37 +1253,6 @@ struct controller_impl { }); } - /** - * Plugins / observers listening to signals emited might trigger - * errors and throw exceptions. Unless those exceptions are caught it could impact consensus and/or - * cause a node to fork. - * - * If it is ever desirable to let a signal handler bubble an exception out of this method - * a full audit of its uses needs to be undertaken. - * - */ - template - void emit( const Signal& s, Arg&& a ) { - try { - s( std::forward( a )); - } catch (std::bad_alloc& e) { - wlog( "std::bad_alloc: ${w}", ("w", e.what()) ); - throw e; - } catch (boost::interprocess::bad_alloc& e) { - wlog( "boost::interprocess::bad alloc: ${w}", ("w", e.what()) ); - throw e; - } catch ( controller_emit_signal_exception& e ) { - wlog( "controller_emit_signal_exception: ${details}", ("details", e.to_detail_string()) ); - throw e; - } catch ( fc::exception& e ) { - wlog( "fc::exception: ${details}", ("details", e.to_detail_string()) ); - } catch ( std::exception& e ) { - wlog( "std::exception: ${details}", ("details", e.what()) ); - } catch ( ... ) { - wlog( "signal handler threw exception" ); - } - } - void dmlog_applied_transaction(const transaction_trace_ptr& t, const signed_transaction* trx = nullptr) { // dmlog_applied_transaction is called by push_scheduled_transaction // where transient transactions are not possible, and by push_transaction diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index e8d1a5d1e3..8d7c3981ef 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -409,6 +409,37 @@ namespace eosio::chain { chainbase::database& mutable_db()const; std::unique_ptr my; - }; + }; // controller + + /** + * Plugins / observers listening to signals emited might trigger + * errors and throw exceptions. Unless those exceptions are caught it could impact consensus and/or + * cause a node to fork. + * + * If it is ever desirable to let a signal handler bubble an exception out of this method + * a full audit of its uses needs to be undertaken. + * + */ + template + void emit( const Signal& s, Arg&& a, const char* location = "" ) { + try { + s( std::forward( a )); + } catch (std::bad_alloc& e) { + wlog( "${l}std::bad_alloc: ${w}", ("l", location)("w", e.what()) ); + throw e; + } catch (boost::interprocess::bad_alloc& e) { + wlog( "${l}boost::interprocess::bad alloc: ${w}", ("l", location)("w", e.what()) ); + throw e; + } catch ( controller_emit_signal_exception& e ) { + wlog( "${l}controller_emit_signal_exception: ${details}", ("l", location)("details", e.to_detail_string()) ); + throw e; + } catch ( fc::exception& e ) { + wlog( "${l}fc::exception: ${details}", ("l", location)("details", e.to_detail_string()) ); + } catch ( std::exception& e ) { + wlog( "std::exception: ${details}", ("l", location)("details", e.what()) ); + } catch ( ... ) { + wlog( "${l}signal handler threw exception", ("l", location) ); + } + } } /// eosio::chain diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index 2287774fe9..655e65bccf 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -61,33 +61,11 @@ class vote_processor_t { named_thread_pool thread_pool; private: - template - void emit( const Signal& s, Arg&& a ) { - try { - s(std::forward(a)); - } catch (std::bad_alloc& e) { - wlog( "std::bad_alloc: ${w}", ("w", e.what()) ); - throw e; - } catch (boost::interprocess::bad_alloc& e) { - wlog( "boost::interprocess::bad alloc: ${w}", ("w", e.what()) ); - throw e; - } catch ( controller_emit_signal_exception& e ) { - wlog( "controller_emit_signal_exception: ${details}", ("details", e.to_detail_string()) ); - throw e; - } catch ( fc::exception& e ) { - wlog( "fc::exception: ${details}", ("details", e.to_detail_string()) ); - } catch ( std::exception& e ) { - wlog( "std::exception: ${details}", ("details", e.what()) ); - } catch ( ... ) { - wlog( "signal handler threw exception" ); - } - } - // called with unlocked mtx void emit(uint32_t connection_id, vote_status status, const vote_message_ptr& msg) { if (connection_id != 0) { // this nodes vote was already signaled if (status != vote_status::duplicate) { // don't bother emitting duplicates - emit( vote_signal, std::tuple{connection_id, status, std::cref(msg)} ); + chain::emit( vote_signal, std::tuple{connection_id, status, std::cref(msg)}, "vote " ); } } } From 4a759b35fe2c0a58e8a692c74497afe7a967ffc2 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 18 Apr 2024 14:56:25 -0500 Subject: [PATCH 35/40] GH-3 Use same value as default for producer --- tests/TestHarness/Cluster.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/TestHarness/Cluster.py b/tests/TestHarness/Cluster.py index 1f4a11b7e0..40f127368a 100644 --- a/tests/TestHarness/Cluster.py +++ b/tests/TestHarness/Cluster.py @@ -257,7 +257,7 @@ def launch(self, pnodes=1, unstartedNodes=0, totalNodes=1, prodCount=21, topo="m argsArr.append("--nogen") nodeosArgs="" if "--vote-threads" not in extraNodeosArgs: - nodeosArgs += " --vote-threads 3" + nodeosArgs += " --vote-threads 4" if "--max-transaction-time" not in extraNodeosArgs: nodeosArgs += " --max-transaction-time -1" if "--abi-serializer-max-time-ms" not in extraNodeosArgs: From 51503c404c275f94e762891fc2914ae794235c11 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 18 Apr 2024 14:56:43 -0500 Subject: [PATCH 36/40] GH-3 Use unordered_map --- .../chain/include/eosio/chain/vote_processor.hpp | 13 +++++++------ plugins/net_plugin/net_plugin.cpp | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index 655e65bccf..32141a2a2a 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -10,6 +10,8 @@ #include #include +#include + namespace eosio::chain { /** @@ -17,7 +19,7 @@ namespace eosio::chain { */ class vote_processor_t { // Even 3000 vote structs are less than 1MB per connection. - // 2500 is should never be reached unless a specific connection is sending garbage. + // 2500 should never be reached unless a specific connection is sending garbage. static constexpr size_t max_votes_per_connection = 2500; // If we have not processed a vote in this amount of time, give up on it. static constexpr fc::microseconds too_old = fc::seconds(5); @@ -51,8 +53,8 @@ class vote_processor_t { std::mutex mtx; vote_index_type index; block_state_ptr last_bsp; - // connection, count of messages - std::map num_messages; + // connection, count of messages + std::unordered_map num_messages; std::atomic lib{0}; std::atomic largest_known_block_num{0}; @@ -99,7 +101,7 @@ class vote_processor_t { index.insert(vote{.connection_id = connection_id, .received = now, .msg = msg}); } - // called with locked mtx + // called with locked mtx, returns with a locked mutex void process_any_queued_for_later(std::unique_lock& g) { if (index.empty()) return; @@ -107,11 +109,10 @@ class vote_processor_t { remove_before_lib(); auto& idx = index.get(); std::vector unprocessed; - unprocessed.reserve(std::min(21u, idx.size())); // maybe increase if we increase # of finalizers from 21 for (auto i = idx.begin(); i != idx.end();) { if (stopped) return; - vote v = *i; + vote v = std::move(*i); idx.erase(i); auto bsp = get_block(v.msg->block_id, g); // g is unlocked diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index cb67c5c56a..815b2815e9 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -535,7 +535,7 @@ namespace eosio { void on_accepted_block_header( const signed_block_ptr& block, const block_id_type& id ); void on_accepted_block(); - void on_voted_block ( uint32_t connection_id, vote_status stauts, const vote_message_ptr& vote ); + void on_voted_block( uint32_t connection_id, vote_status stauts, const vote_message_ptr& vote ); void transaction_ack(const std::pair&); void on_irreversible_block( const block_id_type& id, uint32_t block_num ); From b694aad3e04027355264271a4fa0c30f858a0a0d Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 18 Apr 2024 15:05:05 -0500 Subject: [PATCH 37/40] GH-3 Add better log message --- libraries/chain/include/eosio/chain/vote_processor.hpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index 32141a2a2a..9b9927d9b9 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -39,7 +39,7 @@ class vote_processor_t { using vote_index_type = boost::multi_index_container< vote, indexed_by< - ordered_non_unique< tag, const_mem_fun, std::greater<> >, + ordered_non_unique< tag, const_mem_fun, std::greater<> >, // decending ordered_non_unique< tag, member >, ordered_non_unique< tag, member > > @@ -213,13 +213,14 @@ class vote_processor_t { std::unique_lock g(mtx); if (reset_num_messages) num_messages.clear(); - if (++num_messages[connection_id] > max_votes_per_connection) { + if (auto& num_msgs = ++num_messages[connection_id]; num_msgs > max_votes_per_connection) { remove_connection(connection_id); g.unlock(); // drop, too many from this connection to process, consider connection invalid // don't clear num_messages[connection_id] so we keep reporting max_exceeded until index is drained - elog("Exceeded max votes per connection for ${c}", ("c", connection_id)); + ilog("Exceeded max votes per connection ${n} > ${max} for ${c}", + ("n", num_msgs)("max", max_votes_per_connection)("c", connection_id)); emit(connection_id, vote_status::max_exceeded, msg); } else { block_state_ptr bsp = get_block(msg->block_id, g); From 344b778f3d4bf21d35ece02bafe54934893efb6e Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 18 Apr 2024 15:11:23 -0500 Subject: [PATCH 38/40] GH-3 Do not clear num_messages if there are votes in the index to be processed --- libraries/chain/include/eosio/chain/vote_processor.hpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index 9b9927d9b9..6b6e41d2f2 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -207,11 +207,10 @@ class vote_processor_t { if (stopped) return; auto num_queued_votes = --queued_votes; - bool reset_num_messages = num_queued_votes == 0; // caught up, so clear num_messages if (block_header::num_from_id(msg->block_id) <= lib.load(std::memory_order_relaxed)) return; // ignore any votes lower than lib std::unique_lock g(mtx); - if (reset_num_messages) + if (num_queued_votes == 0 && index.empty()) // caught up, clear num_messages num_messages.clear(); if (auto& num_msgs = ++num_messages[connection_id]; num_msgs > max_votes_per_connection) { remove_connection(connection_id); From ecc8cd47dd09c2c9c702c7b6ed1bc13c802427ce Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 18 Apr 2024 15:26:40 -0500 Subject: [PATCH 39/40] GH-3 More descriptive emit logs --- libraries/chain/controller.cpp | 32 +++++++++---------- .../chain/include/eosio/chain/controller.hpp | 14 ++++---- .../include/eosio/chain/vote_processor.hpp | 2 +- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 696b94f491..a61bcc1413 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1417,7 +1417,7 @@ struct controller_impl { for( auto bitr = branch.rbegin(); bitr != branch.rend() && should_process(*bitr); ++bitr ) { apply_irreversible_block(forkdb, *bitr); - emit( irreversible_block, std::tie((*bitr)->block, (*bitr)->id()) ); + emit( irreversible_block, std::tie((*bitr)->block, (*bitr)->id()), __FILE__, __LINE__ ); // blog.append could fail due to failures like running out of space. // Do it before commit so that in case it throws, DB can be rolled back. @@ -2537,7 +2537,7 @@ struct controller_impl { pending->_block_report.total_elapsed_time += trace->elapsed; pending->_block_report.total_time += trace->elapsed; dmlog_applied_transaction(trace); - emit( applied_transaction, std::tie(trace, trx->packed_trx()) ); + emit( applied_transaction, std::tie(trace, trx->packed_trx()), __FILE__, __LINE__ ); undo_session.squash(); return trace; } @@ -2602,7 +2602,7 @@ struct controller_impl { trace->account_ram_delta = account_delta( gtrx.payer, trx_removal_ram_delta ); dmlog_applied_transaction(trace); - emit( applied_transaction, std::tie(trace, trx->packed_trx()) ); + emit( applied_transaction, std::tie(trace, trx->packed_trx()), __FILE__, __LINE__ ); trx_context.squash(); undo_session.squash(); @@ -2646,7 +2646,7 @@ struct controller_impl { trace->account_ram_delta = account_delta( gtrx.payer, trx_removal_ram_delta ); trace->elapsed = fc::time_point::now() - start; dmlog_applied_transaction(trace); - emit( applied_transaction, std::tie(trace, trx->packed_trx()) ); + emit( applied_transaction, std::tie(trace, trx->packed_trx()), __FILE__, __LINE__ ); undo_session.squash(); pending->_block_report.total_net_usage += trace->net_usage; if( trace->receipt ) pending->_block_report.total_cpu_usage_us += trace->receipt->cpu_usage_us; @@ -2690,12 +2690,12 @@ struct controller_impl { trace->account_ram_delta = account_delta( gtrx.payer, trx_removal_ram_delta ); dmlog_applied_transaction(trace); - emit( applied_transaction, std::tie(trace, trx->packed_trx()) ); + emit( applied_transaction, std::tie(trace, trx->packed_trx()), __FILE__, __LINE__ ); undo_session.squash(); } else { dmlog_applied_transaction(trace); - emit( applied_transaction, std::tie(trace, trx->packed_trx()) ); + emit( applied_transaction, std::tie(trace, trx->packed_trx()), __FILE__, __LINE__ ); } pending->_block_report.total_net_usage += trace->net_usage; @@ -2834,7 +2834,7 @@ struct controller_impl { } dmlog_applied_transaction(trace, &trn); - emit(applied_transaction, std::tie(trace, trx->packed_trx())); + emit( applied_transaction, std::tie(trace, trx->packed_trx()), __FILE__, __LINE__ ); } } @@ -2878,7 +2878,7 @@ struct controller_impl { if (!trx->is_transient()) { dmlog_applied_transaction(trace); - emit(applied_transaction, std::tie(trace, trx->packed_trx())); + emit( applied_transaction, std::tie(trace, trx->packed_trx()), __FILE__, __LINE__ ); pending->_block_report.total_net_usage += trace->net_usage; if( trace->receipt ) pending->_block_report.total_cpu_usage_us += trace->receipt->cpu_usage_us; @@ -2899,7 +2899,7 @@ struct controller_impl { { EOS_ASSERT( !pending, block_validate_exception, "pending block already exists" ); - emit( block_start, chain_head.block_num() + 1 ); + emit( block_start, chain_head.block_num() + 1, __FILE__, __LINE__ ); // at block level, no transaction specific logging is possible if (auto dm_logger = get_deep_mind_logger(false)) { @@ -3144,7 +3144,7 @@ struct controller_impl { const auto& bsp = std::get>(cb.bsp.internal()); if( s == controller::block_status::incomplete ) { forkdb.add( bsp, mark_valid_t::yes, ignore_duplicate_t::no ); - emit( accepted_block_header, std::tie(bsp->block, bsp->id()) ); + emit( accepted_block_header, std::tie(bsp->block, bsp->id()), __FILE__, __LINE__ ); } else { assert(s != controller::block_status::irreversible); forkdb.mark_valid( bsp ); @@ -3154,7 +3154,7 @@ struct controller_impl { } chain_head = block_handle{cb.bsp}; - emit( accepted_block, std::tie(chain_head.block(), chain_head.id()) ); + emit( accepted_block, std::tie(chain_head.block(), chain_head.id()), __FILE__, __LINE__ ); apply(chain_head, [&](const auto& head) { #warning todo: support deep_mind_logger even when in IF mode @@ -3571,7 +3571,7 @@ struct controller_impl { my_finalizers.maybe_vote( *bsp->active_finalizer_policy, bsp, bsp->strong_digest, [&](const vote_message_ptr& vote) { // net plugin subscribed to this signal. it will broadcast the vote message on receiving the signal - emit(voted_block, std::tuple{uint32_t{0}, vote_status::success, std::cref(vote)}); + emit(voted_block, std::tuple{uint32_t{0}, vote_status::success, std::cref(vote)}, __FILE__, __LINE__); // also aggregate our own vote into the pending_qc for this block, 0 connection_id indicates our own vote process_vote_message(0, vote); @@ -3862,7 +3862,7 @@ struct controller_impl { if constexpr (std::is_same_v>) forkdb.add( bsp, mark_valid_t::no, ignore_duplicate_t::yes ); - emit( accepted_block_header, std::tie(bsp->block, bsp->id()) ); + emit( accepted_block_header, std::tie(bsp->block, bsp->id()), __FILE__, __LINE__ ); }; fork_db.apply(do_accept_block); @@ -3900,7 +3900,7 @@ struct controller_impl { trusted_producer_light_validation = true; }; - emit( accepted_block_header, std::tie(bsp->block, bsp->id()) ); + emit( accepted_block_header, std::tie(bsp->block, bsp->id()), __FILE__, __LINE__ ); if( read_mode != db_read_mode::IRREVERSIBLE ) { if constexpr (std::is_same_v>) @@ -3949,7 +3949,7 @@ struct controller_impl { }); } - emit(accepted_block_header, std::tie(bsp->block, bsp->id())); + emit( accepted_block_header, std::tie(bsp->block, bsp->id()), __FILE__, __LINE__ ); controller::block_report br; if (s == controller::block_status::irreversible) { @@ -3957,7 +3957,7 @@ struct controller_impl { // On replay, log_irreversible is not called and so no irreversible_block signal is emitted. // So emit it explicitly here. - emit(irreversible_block, std::tie(bsp->block, bsp->id())); + emit( irreversible_block, std::tie(bsp->block, bsp->id()), __FILE__, __LINE__ ); if (!skip_db_sessions(s)) { db.commit(bsp->block_num()); diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index 8d7c3981ef..6281e35fb7 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -421,24 +421,24 @@ namespace eosio::chain { * */ template - void emit( const Signal& s, Arg&& a, const char* location = "" ) { + void emit( const Signal& s, Arg&& a, const char* file, uint32_t line ) { try { s( std::forward( a )); } catch (std::bad_alloc& e) { - wlog( "${l}std::bad_alloc: ${w}", ("l", location)("w", e.what()) ); + wlog( "${f}:${l} std::bad_alloc: ${w}", ("f", file)("l", line)("w", e.what()) ); throw e; } catch (boost::interprocess::bad_alloc& e) { - wlog( "${l}boost::interprocess::bad alloc: ${w}", ("l", location)("w", e.what()) ); + wlog( "${f}:${l} boost::interprocess::bad alloc: ${w}", ("f", file)("l", line)("w", e.what()) ); throw e; } catch ( controller_emit_signal_exception& e ) { - wlog( "${l}controller_emit_signal_exception: ${details}", ("l", location)("details", e.to_detail_string()) ); + wlog( "${f}:${l} controller_emit_signal_exception: ${details}", ("f", file)("l", line)("details", e.to_detail_string()) ); throw e; } catch ( fc::exception& e ) { - wlog( "${l}fc::exception: ${details}", ("l", location)("details", e.to_detail_string()) ); + wlog( "${f}:${l} fc::exception: ${details}", ("f", file)("l", line)("details", e.to_detail_string()) ); } catch ( std::exception& e ) { - wlog( "std::exception: ${details}", ("l", location)("details", e.what()) ); + wlog( "std::exception: ${details}", ("f", file)("l", line)("details", e.what()) ); } catch ( ... ) { - wlog( "${l}signal handler threw exception", ("l", location) ); + wlog( "${f}:${l} signal handler threw exception", ("f", file)("l", line) ); } } diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index 6b6e41d2f2..d35cd1ccb3 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -67,7 +67,7 @@ class vote_processor_t { void emit(uint32_t connection_id, vote_status status, const vote_message_ptr& msg) { if (connection_id != 0) { // this nodes vote was already signaled if (status != vote_status::duplicate) { // don't bother emitting duplicates - chain::emit( vote_signal, std::tuple{connection_id, status, std::cref(msg)}, "vote " ); + chain::emit( vote_signal, std::tuple{connection_id, status, std::cref(msg)}, __FILE__, __LINE__ ); } } } From 8d3a83821b9d3e6e0c2fa6a6b67a93f40746dac8 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 18 Apr 2024 15:56:08 -0500 Subject: [PATCH 40/40] GH-3 Fix spelling --- libraries/chain/include/eosio/chain/vote_processor.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chain/include/eosio/chain/vote_processor.hpp b/libraries/chain/include/eosio/chain/vote_processor.hpp index d35cd1ccb3..6db78e286e 100644 --- a/libraries/chain/include/eosio/chain/vote_processor.hpp +++ b/libraries/chain/include/eosio/chain/vote_processor.hpp @@ -39,7 +39,7 @@ class vote_processor_t { using vote_index_type = boost::multi_index_container< vote, indexed_by< - ordered_non_unique< tag, const_mem_fun, std::greater<> >, // decending + ordered_non_unique< tag, const_mem_fun, std::greater<> >, // descending ordered_non_unique< tag, member >, ordered_non_unique< tag, member > >