From bf3b284b0c3c4951b7f655c65f0c5d79e7a7da43 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 14 Sep 2023 10:20:36 -0500 Subject: [PATCH 01/65] GH-1523 GH-1631 Transition from dpos to hotstuff Producer schedule changes under hotstuff in next,next producer round. --- libraries/chain/block_header_state.cpp | 199 ++++++++++-------- libraries/chain/block_state.cpp | 3 +- libraries/chain/controller.cpp | 46 ++-- libraries/chain/fork_database.cpp | 5 + .../include/eosio/chain/block_header.hpp | 6 + .../eosio/chain/block_header_state.hpp | 34 ++- .../chain/include/eosio/chain/block_state.hpp | 1 + .../chain/include/eosio/chain/controller.hpp | 4 + libraries/hotstuff/chain_pacemaker.cpp | 4 + plugins/producer_plugin/producer_plugin.cpp | 17 +- 10 files changed, 207 insertions(+), 112 deletions(-) diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index 65fa92be3f..1d55254d6e 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -15,6 +15,12 @@ namespace eosio { namespace chain { const auto& protocol_features = pfa->protocol_features; return digest && protocol_features.find(*digest) != protocol_features.end(); } + + uint32_t get_next_next_round_block_num( block_timestamp_type t, uint32_t block_num ) { + auto index = t.slot % config::producer_repetitions; + // remainder of current + next round + return block_num + config::producer_repetitions - index + config::producer_repetitions; + } } producer_authority block_header_state::get_scheduled_producer( block_timestamp_type t )const { @@ -38,6 +44,7 @@ namespace eosio { namespace chain { } pending_block_header_state block_header_state::next( block_timestamp_type when, + bool hotstuff_activated, uint16_t num_prev_blocks_to_confirm )const { pending_block_header_state result; @@ -50,18 +57,9 @@ namespace eosio { namespace chain { auto proauth = get_scheduled_producer(when); - auto itr = producer_to_last_produced.find( proauth.producer_name ); - if( itr != producer_to_last_produced.end() ) { - EOS_ASSERT( itr->second < (block_num+1) - num_prev_blocks_to_confirm, producer_double_confirm, - "producer ${prod} double-confirming known range", - ("prod", proauth.producer_name)("num", block_num+1) - ("confirmed", num_prev_blocks_to_confirm)("last_produced", itr->second) ); - } - result.block_num = block_num + 1; result.previous = id; result.timestamp = when; - result.confirmed = num_prev_blocks_to_confirm; result.active_schedule_version = active_schedule.version; result.prev_activated_protocol_features = activated_protocol_features; @@ -72,103 +70,129 @@ namespace eosio { namespace chain { result.blockroot_merkle = blockroot_merkle; result.blockroot_merkle.append( id ); - /// grow the confirmed count - static_assert(std::numeric_limits::max() >= (config::max_producers * 2 / 3) + 1, "8bit confirmations may not be able to hold all of the needed confirmations"); - - // This uses the previous block active_schedule because thats the "schedule" that signs and therefore confirms _this_ block - auto num_active_producers = active_schedule.producers.size(); - uint32_t required_confs = (uint32_t)(num_active_producers * 2 / 3) + 1; + result.prev_pending_schedule = pending_schedule; + + if (hotstuff_activated) { + result.confirmed = hs_block_confirmed; + result.dpos_proposed_irreversible_blocknum = 0; + // fork_database will prefer hotstuff blocks over dpos blocks + result.dpos_irreversible_blocknum = hs_dpos_irreversible_blocknum; + // Change to active on the next().next() producer block_num + if( pending_schedule.schedule.producers.size() && + block_num >= detail::get_next_next_round_block_num(when, pending_schedule.schedule_lib_num)) { + result.active_schedule = pending_schedule.schedule; + result.was_pending_promoted = true; + } else { + result.active_schedule = active_schedule; + } - if( confirm_count.size() < config::maximum_tracked_dpos_confirmations ) { - result.confirm_count.reserve( confirm_count.size() + 1 ); - result.confirm_count = confirm_count; - result.confirm_count.resize( confirm_count.size() + 1 ); - result.confirm_count.back() = (uint8_t)required_confs; } else { - result.confirm_count.resize( confirm_count.size() ); - memcpy( &result.confirm_count[0], &confirm_count[1], confirm_count.size() - 1 ); - result.confirm_count.back() = (uint8_t)required_confs; - } + auto itr = producer_to_last_produced.find( proauth.producer_name ); + if( itr != producer_to_last_produced.end() ) { + EOS_ASSERT( itr->second < (block_num+1) - num_prev_blocks_to_confirm, producer_double_confirm, + "producer ${prod} double-confirming known range", + ("prod", proauth.producer_name)("num", block_num+1) + ("confirmed", num_prev_blocks_to_confirm)("last_produced", itr->second) ); + } - auto new_dpos_proposed_irreversible_blocknum = dpos_proposed_irreversible_blocknum; - - int32_t i = (int32_t)(result.confirm_count.size() - 1); - uint32_t blocks_to_confirm = num_prev_blocks_to_confirm + 1; /// confirm the head block too - while( i >= 0 && blocks_to_confirm ) { - --result.confirm_count[i]; - //idump((confirm_count[i])); - if( result.confirm_count[i] == 0 ) - { - uint32_t block_num_for_i = result.block_num - (uint32_t)(result.confirm_count.size() - 1 - i); - new_dpos_proposed_irreversible_blocknum = block_num_for_i; - //idump((dpos2_lib)(block_num)(dpos_irreversible_blocknum)); - - if (i == static_cast(result.confirm_count.size() - 1)) { - result.confirm_count.resize(0); - } else { - memmove( &result.confirm_count[0], &result.confirm_count[i + 1], result.confirm_count.size() - i - 1); - result.confirm_count.resize( result.confirm_count.size() - i - 1 ); - } + result.confirmed = num_prev_blocks_to_confirm; - break; - } - --i; - --blocks_to_confirm; - } + /// grow the confirmed count + static_assert(std::numeric_limits::max() >= (config::max_producers * 2 / 3) + 1, "8bit confirmations may not be able to hold all of the needed confirmations"); + + // This uses the previous block active_schedule because thats the "schedule" that signs and therefore confirms _this_ block + auto num_active_producers = active_schedule.producers.size(); + uint32_t required_confs = (uint32_t)(num_active_producers * 2 / 3) + 1; + + if( confirm_count.size() < config::maximum_tracked_dpos_confirmations ) { + result.confirm_count.reserve( confirm_count.size() + 1 ); + result.confirm_count = confirm_count; + result.confirm_count.resize( confirm_count.size() + 1 ); + result.confirm_count.back() = (uint8_t)required_confs; + } else { + result.confirm_count.resize( confirm_count.size() ); + memcpy( &result.confirm_count[0], &confirm_count[1], confirm_count.size() - 1 ); + result.confirm_count.back() = (uint8_t)required_confs; + } - result.dpos_proposed_irreversible_blocknum = new_dpos_proposed_irreversible_blocknum; - result.dpos_irreversible_blocknum = calc_dpos_last_irreversible( proauth.producer_name ); + auto new_dpos_proposed_irreversible_blocknum = dpos_proposed_irreversible_blocknum; + + int32_t i = (int32_t)(result.confirm_count.size() - 1); + uint32_t blocks_to_confirm = num_prev_blocks_to_confirm + 1; /// confirm the head block too + while( i >= 0 && blocks_to_confirm ) { + --result.confirm_count[i]; + //idump((confirm_count[i])); + if( result.confirm_count[i] == 0 ) + { + uint32_t block_num_for_i = result.block_num - (uint32_t)(result.confirm_count.size() - 1 - i); + new_dpos_proposed_irreversible_blocknum = block_num_for_i; + //idump((dpos2_lib)(block_num)(dpos_irreversible_blocknum)); + + if (i == static_cast(result.confirm_count.size() - 1)) { + result.confirm_count.resize(0); + } else { + memmove( &result.confirm_count[0], &result.confirm_count[i + 1], result.confirm_count.size() - i - 1); + result.confirm_count.resize( result.confirm_count.size() - i - 1 ); + } - result.prev_pending_schedule = pending_schedule; + break; + } + --i; + --blocks_to_confirm; + } - if( pending_schedule.schedule.producers.size() && - result.dpos_irreversible_blocknum >= pending_schedule.schedule_lib_num ) - { - result.active_schedule = pending_schedule.schedule; + result.dpos_proposed_irreversible_blocknum = new_dpos_proposed_irreversible_blocknum; + result.dpos_irreversible_blocknum = calc_dpos_last_irreversible( proauth.producer_name ); + + if( pending_schedule.schedule.producers.size() && + result.dpos_irreversible_blocknum >= pending_schedule.schedule_lib_num ) + { + result.active_schedule = pending_schedule.schedule; - flat_map new_producer_to_last_produced; + flat_map new_producer_to_last_produced; - for( const auto& pro : result.active_schedule.producers ) { - if( pro.producer_name == proauth.producer_name ) { - new_producer_to_last_produced[pro.producer_name] = result.block_num; - } else { - auto existing = producer_to_last_produced.find( pro.producer_name ); - if( existing != producer_to_last_produced.end() ) { - new_producer_to_last_produced[pro.producer_name] = existing->second; + for( const auto& pro : result.active_schedule.producers ) { + if( pro.producer_name == proauth.producer_name ) { + new_producer_to_last_produced[pro.producer_name] = result.block_num; } else { - new_producer_to_last_produced[pro.producer_name] = result.dpos_irreversible_blocknum; + auto existing = producer_to_last_produced.find( pro.producer_name ); + if( existing != producer_to_last_produced.end() ) { + new_producer_to_last_produced[pro.producer_name] = existing->second; + } else { + new_producer_to_last_produced[pro.producer_name] = result.dpos_irreversible_blocknum; + } } } - } - new_producer_to_last_produced[proauth.producer_name] = result.block_num; + new_producer_to_last_produced[proauth.producer_name] = result.block_num; - result.producer_to_last_produced = std::move( new_producer_to_last_produced ); + result.producer_to_last_produced = std::move( new_producer_to_last_produced ); - flat_map new_producer_to_last_implied_irb; + flat_map new_producer_to_last_implied_irb; - for( const auto& pro : result.active_schedule.producers ) { - if( pro.producer_name == proauth.producer_name ) { - new_producer_to_last_implied_irb[pro.producer_name] = dpos_proposed_irreversible_blocknum; - } else { - auto existing = producer_to_last_implied_irb.find( pro.producer_name ); - if( existing != producer_to_last_implied_irb.end() ) { - new_producer_to_last_implied_irb[pro.producer_name] = existing->second; + for( const auto& pro : result.active_schedule.producers ) { + if( pro.producer_name == proauth.producer_name ) { + new_producer_to_last_implied_irb[pro.producer_name] = dpos_proposed_irreversible_blocknum; } else { - new_producer_to_last_implied_irb[pro.producer_name] = result.dpos_irreversible_blocknum; + auto existing = producer_to_last_implied_irb.find( pro.producer_name ); + if( existing != producer_to_last_implied_irb.end() ) { + new_producer_to_last_implied_irb[pro.producer_name] = existing->second; + } else { + new_producer_to_last_implied_irb[pro.producer_name] = result.dpos_irreversible_blocknum; + } } } - } - result.producer_to_last_implied_irb = std::move( new_producer_to_last_implied_irb ); + result.producer_to_last_implied_irb = std::move( new_producer_to_last_implied_irb ); - result.was_pending_promoted = true; - } else { - result.active_schedule = active_schedule; - result.producer_to_last_produced = producer_to_last_produced; - result.producer_to_last_produced[proauth.producer_name] = result.block_num; - result.producer_to_last_implied_irb = producer_to_last_implied_irb; - result.producer_to_last_implied_irb[proauth.producer_name] = dpos_proposed_irreversible_blocknum; - } + result.was_pending_promoted = true; + } else { + result.active_schedule = active_schedule; + result.producer_to_last_produced = producer_to_last_produced; + result.producer_to_last_produced[proauth.producer_name] = result.block_num; + result.producer_to_last_implied_irb = producer_to_last_implied_irb; + result.producer_to_last_implied_irb[proauth.producer_name] = dpos_proposed_irreversible_blocknum; + } + } // !hotstuff_activated return result; } @@ -385,12 +409,13 @@ namespace eosio { namespace chain { const signed_block_header& h, vector&& _additional_signatures, const protocol_feature_set& pfs, + bool hotstuff_activated, const std::function&, const vector& )>& validator, bool skip_validate_signee )const { - return next( h.timestamp, h.confirmed ).finish_next( h, std::move(_additional_signatures), pfs, validator, skip_validate_signee ); + return next( h.timestamp, hotstuff_activated, h.confirmed ).finish_next( h, std::move(_additional_signatures), pfs, validator, skip_validate_signee ); } digest_type block_header_state::sig_digest()const { diff --git a/libraries/chain/block_state.cpp b/libraries/chain/block_state.cpp index fd614a6118..c1dbdf122c 100644 --- a/libraries/chain/block_state.cpp +++ b/libraries/chain/block_state.cpp @@ -77,12 +77,13 @@ namespace eosio { namespace chain { block_state::block_state( const block_header_state& prev, signed_block_ptr b, const protocol_feature_set& pfs, + bool hotstuff_activated, const std::function&, const vector& )>& validator, bool skip_validate_signee ) - :block_header_state( prev.next( *b, extract_additional_signatures(b, pfs, prev.activated_protocol_features), pfs, validator, skip_validate_signee ) ) + :block_header_state( prev.next( *b, extract_additional_signatures(b, pfs, prev.activated_protocol_features), pfs, hotstuff_activated, validator, skip_validate_signee ) ) ,block( std::move(b) ) {} diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index e4c6f89bb0..2e0d282d36 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -116,9 +116,10 @@ class maybe_session { struct building_block { building_block( const block_header_state& prev, block_timestamp_type when, + bool hotstuff_activated, uint16_t num_prev_blocks_to_confirm, const vector& new_protocol_feature_activations ) - :_pending_block_header_state( prev.next( when, num_prev_blocks_to_confirm ) ) + :_pending_block_header_state( prev.next( when, hotstuff_activated, num_prev_blocks_to_confirm ) ) ,_new_protocol_feature_activations( new_protocol_feature_activations ) ,_trx_mroot_or_receipt_digests( digests_t{} ) {} @@ -152,10 +153,11 @@ using block_stage_type = std::variant& new_protocol_feature_activations ) :_db_session( std::move(s) ) - ,_block_stage( building_block( prev, when, num_prev_blocks_to_confirm, new_protocol_feature_activations ) ) + ,_block_stage( building_block( prev, when, hotstuff_activated, num_prev_blocks_to_confirm, new_protocol_feature_activations ) ) {} maybe_session _db_session; @@ -235,6 +237,7 @@ struct controller_impl { std::optional pending; block_state_ptr head; fork_database fork_db; + std::atomic hs_irreversible_block_num{0}; resource_limits_manager resource_limits; subjective_billing subjective_bill; authorization_manager authorization; @@ -431,11 +434,13 @@ struct controller_impl { } const auto fork_head = fork_db_head(); + const uint32_t hs_lib = hs_irreversible_block_num; + const uint32_t new_lib = hs_lib > 0 ? hs_lib : fork_head->dpos_irreversible_blocknum; - if( fork_head->dpos_irreversible_blocknum <= lib_num ) + if( new_lib <= lib_num ) return; - auto branch = fork_db.fetch_branch( fork_head->id, fork_head->dpos_irreversible_blocknum ); + auto branch = fork_db.fetch_branch( fork_head->id, new_lib ); try { std::vector>> v; @@ -468,7 +473,7 @@ struct controller_impl { throw; } - //db.commit( fork_head->dpos_irreversible_blocknum ); // redundant + //db.commit( new_lib ); // redundant if( root_id != fork_db.root()->id ) { branch.emplace_back(fork_db.root()); @@ -1699,6 +1704,9 @@ struct controller_impl { { EOS_ASSERT( !pending, block_validate_exception, "pending block already exists" ); + // can change during start_block, so use same value throughout; although the transition from 0 to >0 cannot happen during start_block + uint32_t hs_lib = hs_irreversible_block_num.load(); + emit( self.block_start, head->block_num + 1 ); // at block level, no transaction specific logging is possible @@ -1716,9 +1724,9 @@ struct controller_impl { EOS_ASSERT( db.revision() == head->block_num, database_exception, "db revision is not on par with head block", ("db.revision()", db.revision())("controller_head_block", head->block_num)("fork_db_head_block", fork_db.head()->block_num) ); - pending.emplace( maybe_session(db), *head, when, confirm_block_count, new_protocol_feature_activations ); + pending.emplace( maybe_session(db), *head, when, hs_lib > 0, confirm_block_count, new_protocol_feature_activations ); } else { - pending.emplace( maybe_session(), *head, when, confirm_block_count, new_protocol_feature_activations ); + pending.emplace( maybe_session(), *head, when, hs_lib > 0, confirm_block_count, new_protocol_feature_activations ); } pending->_block_status = s; @@ -1800,22 +1808,23 @@ struct controller_impl { const auto& gpo = self.get_global_properties(); if( gpo.proposed_schedule_block_num && // if there is a proposed schedule that was proposed in a block ... - ( *gpo.proposed_schedule_block_num <= pbhs.dpos_irreversible_blocknum ) && // ... that has now become irreversible ... + ( hs_lib > 0 || *gpo.proposed_schedule_block_num <= pbhs.dpos_irreversible_blocknum ) && // ... that has now become irreversible or hotstuff activated... pbhs.prev_pending_schedule.schedule.producers.size() == 0 // ... and there was room for a new pending schedule prior to any possible promotion ) { - // Promote proposed schedule to pending schedule. + // Promote proposed schedule to pending schedule; happens in next block after hotstuff activated + EOS_ASSERT( gpo.proposed_schedule.version == pbhs.active_schedule_version + 1, + producer_schedule_exception, "wrong producer schedule version specified" ); + + std::get(pending->_block_stage)._new_pending_producer_schedule = producer_authority_schedule::from_shared(gpo.proposed_schedule); + if( !replaying ) { ilog( "promoting proposed schedule (set in block ${proposed_num}) to pending; current block: ${n} lib: ${lib} schedule: ${schedule} ", ("proposed_num", *gpo.proposed_schedule_block_num)("n", pbhs.block_num) - ("lib", pbhs.dpos_irreversible_blocknum) - ("schedule", producer_authority_schedule::from_shared(gpo.proposed_schedule) ) ); + ("lib", hs_lib > 0 ? hs_lib : pbhs.dpos_irreversible_blocknum) + ("schedule", std::get(pending->_block_stage)._new_pending_producer_schedule ) ); } - EOS_ASSERT( gpo.proposed_schedule.version == pbhs.active_schedule_version + 1, - producer_schedule_exception, "wrong producer schedule version specified" ); - - std::get(pending->_block_stage)._new_pending_producer_schedule = producer_authority_schedule::from_shared(gpo.proposed_schedule); db.modify( gpo, [&]( auto& gp ) { gp.proposed_schedule_block_num = std::optional(); gp.proposed_schedule.version=0; @@ -2212,6 +2221,7 @@ struct controller_impl { prev, b, protocol_features.get_protocol_feature_set(), + b->confirmed == hs_block_confirmed, // is hotstuff enabled for block [this]( block_timestamp_type timestamp, const flat_set& cur_features, const vector& new_features ) @@ -2318,6 +2328,7 @@ struct controller_impl { *head, b, protocol_features.get_protocol_feature_set(), + b->confirmed == hs_block_confirmed, // is hotstuff enabled for block [this]( block_timestamp_type timestamp, const flat_set& cur_features, const vector& new_features ) @@ -3168,6 +3179,11 @@ std::optional controller::pending_producer_block_id()const { return my->pending->_producer_block_id; } +void controller::set_hs_irreversible_block_num(uint32_t block_num) { + // needs to be set by qc_chain at startup and as irreversible changes + my->hs_irreversible_block_num = block_num; +} + uint32_t controller::last_irreversible_block_num() const { return my->fork_db.root()->block_num; } diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index 57bab920b2..52d6fcf397 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -42,6 +42,7 @@ namespace eosio { namespace chain { ordered_unique< tag, composite_key< block_state, global_fun, + // see first_preferred comment member, member, member @@ -57,6 +58,10 @@ namespace eosio { namespace chain { > fork_multi_index_type; bool first_preferred( const block_header_state& lhs, const block_header_state& rhs ) { + // dpos_irreversible_blocknum == std::numeric_limits::max() after hotstuff activation + // hotstuff block considered preferred over dpos + // hotstuff blocks compared by block_num as both lhs & rhs dpos_irreversible_blocknum is max uint32_t + // This can be simplified in a future release that assumes hotstuff already activated return std::tie( lhs.dpos_irreversible_blocknum, lhs.block_num ) > std::tie( rhs.dpos_irreversible_blocknum, rhs.block_num ); } diff --git a/libraries/chain/include/eosio/chain/block_header.hpp b/libraries/chain/include/eosio/chain/block_header.hpp index 8bb7976b76..b6dac4e2b1 100644 --- a/libraries/chain/include/eosio/chain/block_header.hpp +++ b/libraries/chain/include/eosio/chain/block_header.hpp @@ -25,6 +25,9 @@ namespace eosio { namespace chain { using block_header_extension = block_header_extension_types::block_header_extension_t; + // totem for block_header.confirmed that indicates hotstuff consensus is active + constexpr uint16_t hs_block_confirmed = std::numeric_limits::max(); + struct block_header { block_timestamp_type timestamp; @@ -38,6 +41,9 @@ namespace eosio { namespace chain { * No producer should sign a block with overlapping ranges or it is proof of byzantine * behavior. When producing a block a producer is always confirming at least the block he * is building off of. A producer cannot confirm "this" block, only prior blocks. + * + * After hotstuff activation a producer can no longer confirm blocks only propose them; + * confirmed will be std::numeric_limits::max() after hotstuff activation. */ uint16_t confirmed = 1; diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index 060c1ee943..d70390b90c 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -48,6 +48,10 @@ using signer_callback_type = std::function(const dig struct block_header_state; +// totem for dpos_irreversible_blocknum after hotstuff is activated +// This value implicitly means that fork_database will prefer hotstuff blocks over dpos blocks +constexpr uint32_t hs_dpos_irreversible_blocknum = std::numeric_limits::max(); + namespace detail { struct block_header_state_common { uint32_t block_num = 0; @@ -115,6 +119,33 @@ struct pending_block_header_state : public detail::block_header_state_common { /** * @struct block_header_state + * + * Algorithm for producer schedule change (pre-hostuff) + * privileged contract -> set_proposed_producers(producers) -> + * global_property_object.proposed_schedule_block_num = current_block_num + * global_property_object.proposed_schedule = producers + * + * start_block -> (global_property_object.proposed_schedule_block_num == dpos_lib) + * building_block._new_pending_producer_schedule = producers + * + * finalize_block -> + * block_header.extensions.wtmsig_block_signatures = producers + * block_header.new_producers = producers + * + * create_block_state -> + * block_state.schedule_lib_num = current_block_num + * block_state.pending_schedule.schedule = producers + * + * start_block -> + * block_state.prev_pending_schedule = pending_schedule (producers) + * if (pending_schedule.schedule_lib_num == dpos_lib) + * block_state.active_schedule = pending_schedule + * block_state.was_pending_promoted = true + * block_state.pending_schedule.clear() // doesn't get copied from previous + * else + * block_state.pending_schedule = prev_pending_schedule + * + * * @brief defines the minimum state necessary to validate transaction headers */ struct block_header_state : public detail::block_header_state_common { @@ -136,11 +167,12 @@ struct block_header_state : public detail::block_header_state_common { explicit block_header_state( legacy::snapshot_block_header_state_v2&& snapshot ); - pending_block_header_state next( block_timestamp_type when, uint16_t num_prev_blocks_to_confirm )const; + pending_block_header_state next( block_timestamp_type when, bool hotstuff_activated, uint16_t num_prev_blocks_to_confirm )const; block_header_state next( const signed_block_header& h, vector&& additional_signatures, const protocol_feature_set& pfs, + bool hotstuff_activated, const std::function&, const vector& )>& validator, diff --git a/libraries/chain/include/eosio/chain/block_state.hpp b/libraries/chain/include/eosio/chain/block_state.hpp index 4772094739..c8338a12fb 100644 --- a/libraries/chain/include/eosio/chain/block_state.hpp +++ b/libraries/chain/include/eosio/chain/block_state.hpp @@ -11,6 +11,7 @@ namespace eosio { namespace chain { block_state( const block_header_state& prev, signed_block_ptr b, const protocol_feature_set& pfs, + bool hotstuff_activated, const std::function&, const vector& )>& validator, diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index b86cba8a06..5db2b69495 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -239,6 +239,10 @@ namespace eosio { namespace chain { const producer_authority_schedule& pending_producers()const; std::optional proposed_producers()const; + // Called by qc_chain to indicate the current irreversible block num + // After hotstuff is activated, this should be called on startup by qc_chain + void set_hs_irreversible_block_num(uint32_t block_num); + uint32_t last_irreversible_block_num() const; block_id_type last_irreversible_block_id() const; time_point last_irreversible_block_time() const; diff --git a/libraries/hotstuff/chain_pacemaker.cpp b/libraries/hotstuff/chain_pacemaker.cpp index e034d9b1fc..32fbd63984 100644 --- a/libraries/hotstuff/chain_pacemaker.cpp +++ b/libraries/hotstuff/chain_pacemaker.cpp @@ -232,6 +232,10 @@ namespace eosio { namespace hotstuff { std::optional ext = blk->block->extract_header_extension(hs_finalizer_set_extension::extension_id()); if (ext) { std::scoped_lock g( _chain_state_mutex ); + if (_active_finalizer_set.generation == 0) { + // switching from dpos to hotstuff, all nodes will switch at same block height + _chain->set_hs_irreversible_block_num(blk->block_num); // can be any value <= dpos lib + } _active_finalizer_set = std::move(std::get(*ext)); } } diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 757b930303..2af83154d0 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -765,18 +765,16 @@ class producer_plugin_impl : public std::enable_shared_from_thistimestamp < fc::minutes(5) || (blk_num % 1000 == 0)) { ilog("Received block ${id}... #${n} @ ${t} signed by ${p} " - "[trxs: ${count}, lib: ${lib}, confirmed: ${confs}, net: ${net}, cpu: ${cpu}, elapsed: ${elapsed}, time: ${time}, latency: " - "${latency} ms]", + "[trxs: ${count}, lib: ${lib}, net: ${net}, cpu: ${cpu}, elapsed: ${elapsed}, time: ${time}, latency: ${latency} ms]", ("p", block->producer)("id", id.str().substr(8, 16))("n", blk_num)("t", block->timestamp) ("count", block->transactions.size())("lib", chain.last_irreversible_block_num()) - ("confs", block->confirmed)("net", br.total_net_usage)("cpu", br.total_cpu_usage_us) + ("net", br.total_net_usage)("cpu", br.total_cpu_usage_us) ("elapsed", br.total_elapsed_time)("time", br.total_time)("latency", (now - block->timestamp).count() / 1000)); if (chain.get_read_mode() != db_read_mode::IRREVERSIBLE && hbs->id != id && hbs->block != nullptr) { // not applied to head ilog("Block not applied to head ${id}... #${n} @ ${t} signed by ${p} " - "[trxs: ${count}, dpos: ${dpos}, confirmed: ${confs}, net: ${net}, cpu: ${cpu}, elapsed: ${elapsed}, time: ${time}, " - "latency: ${latency} ms]", + "[trxs: ${count}, lib: ${lib}, net: ${net}, cpu: ${cpu}, elapsed: ${elapsed}, time: ${time}, latency: ${latency} ms]", ("p", hbs->block->producer)("id", hbs->id.str().substr(8, 16))("n", hbs->block_num)("t", hbs->block->timestamp) - ("count", hbs->block->transactions.size())("dpos", hbs->dpos_irreversible_blocknum)("confs", hbs->block->confirmed) + ("count", hbs->block->transactions.size())("lib", chain.last_irreversible_block_num()) ("net", br.total_net_usage)("cpu", br.total_cpu_usage_us)("elapsed", br.total_elapsed_time)("time", br.total_time) ("latency", (now - hbs->block->timestamp).count() / 1000)); } @@ -1911,11 +1909,11 @@ producer_plugin_impl::start_block_result producer_plugin_impl::start_block() { try { uint16_t blocks_to_confirm = 0; - if (in_producing_mode()) { + if (in_producing_mode() && hbs->dpos_irreversible_blocknum != hs_dpos_irreversible_blocknum) { // determine how many blocks this producer can confirm // 1) if it is not a producer from this node, assume no confirmations (we will discard this block anyway) // 2) if it is a producer on this node that has never produced, the conservative approach is to assume no - // confirmations to make sure we don't double sign after a crash TODO: make these watermarks durable? + // confirmations to make sure we don't double sign after a crash // 3) if it is a producer on this node where this node knows the last block it produced, safely set it -UNLESS- // 4) the producer on this node's last watermark is higher (meaning on a different fork) if (current_watermark) { @@ -2857,6 +2855,8 @@ void producer_plugin_impl::switch_to_read_window() { _time_tracker.pause(); + fc_dlog(_log, "switch to read"); + // we are in write window, so no read-only trx threads are processing transactions. if (app().executor().read_only_queue().empty()) { // no read-only tasks to process. stay in write window start_write_window(); // restart write window timer for next round @@ -2881,6 +2881,7 @@ void producer_plugin_impl::switch_to_read_window() { _ro_exec_tasks_fut.emplace_back(post_async_task( _ro_thread_pool.get_executor(), [self = this, pending_block_num]() { return self->read_only_execution_task(pending_block_num); })); } + fc_dlog(_log, "read window"); auto expire_time = boost::posix_time::microseconds(_ro_read_window_time_us.count()); _ro_timer.expires_from_now(expire_time); From 718e17ddbbece5244630ea9b90a34a969c029bae Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 14 Sep 2023 13:13:30 -0500 Subject: [PATCH 02/65] GH-1523 GH-1631 Transition from dpos to hotstuff Add comment to schedule_lib_num. Do not want to rename as to not break clients that expect schedule_lib_num name. --- libraries/chain/include/eosio/chain/block_header_state.hpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/libraries/chain/include/eosio/chain/block_header_state.hpp b/libraries/chain/include/eosio/chain/block_header_state.hpp index d70390b90c..8fbdd6e57a 100644 --- a/libraries/chain/include/eosio/chain/block_header_state.hpp +++ b/libraries/chain/include/eosio/chain/block_header_state.hpp @@ -67,7 +67,10 @@ namespace detail { }; struct schedule_info { - uint32_t schedule_lib_num = 0; /// last irr block num + // schedule_lib_num is compared with dpos lib, but the value is actually current block at time of pending + // After hotstuff is activated, schedule_lib_num is compared to next().next() round for determination of + // changing from pending to active. + uint32_t schedule_lib_num = 0; /// block_num of pending digest_type schedule_hash; producer_authority_schedule schedule; }; @@ -133,7 +136,7 @@ struct pending_block_header_state : public detail::block_header_state_common { * block_header.new_producers = producers * * create_block_state -> - * block_state.schedule_lib_num = current_block_num + * block_state.schedule_lib_num = current_block_num (note this should be named schedule_block_num) * block_state.pending_schedule.schedule = producers * * start_block -> From 0b094688b0b4c6a2b2e15effc126a1a8b0aa6084 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 14 Sep 2023 13:15:36 -0500 Subject: [PATCH 03/65] Remove temp debug output --- plugins/producer_plugin/producer_plugin.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 2af83154d0..dc48f03d47 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -2855,8 +2855,6 @@ void producer_plugin_impl::switch_to_read_window() { _time_tracker.pause(); - fc_dlog(_log, "switch to read"); - // we are in write window, so no read-only trx threads are processing transactions. if (app().executor().read_only_queue().empty()) { // no read-only tasks to process. stay in write window start_write_window(); // restart write window timer for next round @@ -2881,7 +2879,6 @@ void producer_plugin_impl::switch_to_read_window() { _ro_exec_tasks_fut.emplace_back(post_async_task( _ro_thread_pool.get_executor(), [self = this, pending_block_num]() { return self->read_only_execution_task(pending_block_num); })); } - fc_dlog(_log, "read window"); auto expire_time = boost::posix_time::microseconds(_ro_read_window_time_us.count()); _ro_timer.expires_from_now(expire_time); From 4b6928ad7f757391fe9d89e6f9c20c0cad97c14c Mon Sep 17 00:00:00 2001 From: fcecin Date: Mon, 25 Sep 2023 10:58:21 -0300 Subject: [PATCH 04/65] Implement message propagation - propagate proposals if they are added by the node to its proposal store - non-leaders propagate votes of known proposals that haven't been propagated by them yet - added structure to track seen votes for each proposal (GC'd together with proposals by height) - propagate new view messages that have a High QC that's newer than what we had locally - enabled basic propagation test - added star topology propagation test - added ring topology propagation test - documented that new block messages are not propagated - removed topology restriction for new block messages at test_pacemaker - removed hop count from hotstuff_test_handler::dispatch(); will dispatch until network goes quiet Merging both multi-indexes declared in the qc_chain is possible (makes proposal height gc 2x faster) but I'd leave that for later to minimize merge conflicts. --- .../include/eosio/hotstuff/qc_chain.hpp | 31 ++ libraries/hotstuff/qc_chain.cpp | 57 ++- libraries/hotstuff/test/test_hotstuff.cpp | 341 ++++++++++++++++-- libraries/hotstuff/test/test_pacemaker.cpp | 3 +- 4 files changed, 400 insertions(+), 32 deletions(-) diff --git a/libraries/hotstuff/include/eosio/hotstuff/qc_chain.hpp b/libraries/hotstuff/include/eosio/hotstuff/qc_chain.hpp index 6169b1efa5..eeb23efa4d 100644 --- a/libraries/hotstuff/include/eosio/hotstuff/qc_chain.hpp +++ b/libraries/hotstuff/include/eosio/hotstuff/qc_chain.hpp @@ -88,6 +88,12 @@ namespace eosio::hotstuff { bool quorum_met = false; // not serialized across network }; + struct seen_votes { + fc::sha256 proposal_id; // id of proposal being voted on + uint64_t height; // height of the proposal (for GC) + std::set finalizers; // finalizers that have voted on the proposal + }; + // Concurrency note: qc_chain is a single-threaded and lock-free decision engine. // All thread synchronization, if any, is external. class qc_chain { @@ -113,6 +119,10 @@ namespace eosio::hotstuff { void on_hs_vote_msg(const uint32_t connection_id, const hs_vote_message& msg); void on_hs_proposal_msg(const uint32_t connection_id, const hs_proposal_message& msg); void on_hs_new_view_msg(const uint32_t connection_id, const hs_new_view_message& msg); + + // NOTE: The hotstuff New Block message is not ever propagated (multi-hop) by this method. + // Unit tests do not use network topology emulation for this message. + // The live network does not actually dispatch this message to the wire; this is a local callback. void on_hs_new_block_msg(const uint32_t connection_id, const hs_new_block_message& msg); private: @@ -246,6 +256,27 @@ namespace eosio::hotstuff { proposal_store_type _proposal_store; //internal proposals store #endif + + // Possible optimization: merge _proposal_store and _seen_votes_store. + // Store a struct { set seen_votes; hs_proposal_message p; } in the (now single) multi-index. + struct by_seen_votes_proposal_id{}; + struct by_seen_votes_proposal_height{}; + typedef multi_index_container< + seen_votes, + indexed_by< + hashed_unique< + tag, + BOOST_MULTI_INDEX_MEMBER(seen_votes,fc::sha256,proposal_id) + >, + ordered_non_unique< + tag, + BOOST_MULTI_INDEX_MEMBER(seen_votes,uint64_t,height) + > + > + > seen_votes_store_type; + + // given a height, store a map of proposal IDs at that height and the seen votes for it + seen_votes_store_type _seen_votes_store; }; } /// eosio::hotstuff diff --git a/libraries/hotstuff/qc_chain.cpp b/libraries/hotstuff/qc_chain.cpp index 99018fb36b..27a4eef6fb 100644 --- a/libraries/hotstuff/qc_chain.cpp +++ b/libraries/hotstuff/qc_chain.cpp @@ -418,6 +418,10 @@ namespace eosio::hotstuff { //update internal state update(proposal); + //propagate this proposal since it was new to us + if (! am_i_leader()) + send_hs_proposal_msg(connection_id, proposal); + for (auto &msg : msgs) { send_hs_vote_msg( std::nullopt, msg ); } @@ -437,23 +441,45 @@ namespace eosio::hotstuff { bool am_leader = am_i_leader(); - if (!am_leader) - return; - fc_tlog(_logger, " === Process vote from ${finalizer} : current bitset ${value}" , - ("finalizer", vote.finalizer)("value", _current_qc.get_active_finalizers_string())); - // only leader need to take action on votes - if (vote.proposal_id != _current_qc.get_proposal_id()) { - send_hs_message_warning(connection_id, hs_message_warning::discarded); // example; to be tuned to actual need - return; - } + if (am_leader) { + if (vote.proposal_id != _current_qc.get_proposal_id()) { + send_hs_message_warning(connection_id, hs_message_warning::discarded); // example; to be tuned to actual need + return; + } + } const hs_proposal_message *p = get_proposal( vote.proposal_id ); if (p == nullptr) { - fc_elog(_logger, " *** ${id} couldn't find proposal, vote : ${vote}", ("id",_id)("vote", vote)); + if (am_leader) + fc_elog(_logger, " *** ${id} couldn't find proposal, vote : ${vote}", ("id",_id)("vote", vote)); send_hs_message_warning(connection_id, hs_message_warning::discarded); // example; to be tuned to actual need return; } + // if not leader, check message propagation and quit + if (! am_leader) { + seen_votes_store_type::nth_index<0>::type::iterator itr = _seen_votes_store.get().find( p->proposal_id ); + bool propagate = false; + if (itr == _seen_votes_store.get().end()) { + seen_votes sv = { p->proposal_id, p->get_height(), { vote.finalizer } }; + _seen_votes_store.insert(sv); + propagate = true; + } else { + _seen_votes_store.get().modify(itr, [&](seen_votes& sv) { + if (sv.finalizers.count(vote.finalizer) == 0) { + sv.finalizers.insert(vote.finalizer); + propagate = true; + } + }); + } + if (propagate) + send_hs_vote_msg(connection_id, vote); + return; + } + + fc_tlog(_logger, " === Process vote from ${finalizer} : current bitset ${value}" , + ("finalizer", vote.finalizer)("value", _current_qc.get_active_finalizers_string())); + bool quorum_met = _current_qc.is_quorum_met(); //check if quorum already met // If quorum is already met, we don't need to do anything else. Otherwise, we aggregate the signature. @@ -526,6 +552,11 @@ namespace eosio::hotstuff { auto increment_version = fc::make_scoped_exit([this]() { ++_state_version; }); if (!update_high_qc(quorum_certificate{msg.high_qc})) { increment_version.cancel(); + } else { + // Always propagate a view that's newer than ours. + // If it's not newer, then we have already propagated ours. + // If the recipient doesn't think ours is newer, it has already propagated its own, and so on. + send_hs_new_view_msg(connection_id, msg); } } @@ -988,6 +1019,12 @@ namespace eosio::hotstuff { void qc_chain::gc_proposals(uint64_t cutoff){ //fc_tlog(_logger, " === garbage collection on old data"); + auto sv_end_itr = _seen_votes_store.get().upper_bound(cutoff); + while (_seen_votes_store.get().begin() != sv_end_itr){ + auto itr = _seen_votes_store.get().begin(); + _seen_votes_store.get().erase(itr); + } + #ifdef QC_CHAIN_SIMPLE_PROPOSAL_STORE ps_height_iterator psh_it = _proposal_stores_by_height.begin(); while (psh_it != _proposal_stores_by_height.end()) { diff --git a/libraries/hotstuff/test/test_hotstuff.cpp b/libraries/hotstuff/test/test_hotstuff.cpp index d128cbf0a0..85e51f4dcb 100644 --- a/libraries/hotstuff/test/test_hotstuff.cpp +++ b/libraries/hotstuff/test/test_hotstuff.cpp @@ -129,13 +129,16 @@ class hotstuff_test_handler { std::cout << "\n"; } - void dispatch(test_pacemaker& tpm, int hops, test_pacemaker::hotstuff_message_index msg_type, const std::string& memo = "") { - for (int i=0;isecond->get_state(fs_bpa); BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); @@ -1067,9 +1070,9 @@ BOOST_AUTO_TEST_CASE(hotstuff_7) try { BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); - ht.dispatch(tpm, 2, test_pacemaker::hs_vote); //send votes on proposal (prepareQC on first block) + ht.dispatch(tpm, test_pacemaker::hs_vote); //send votes on proposal (prepareQC on first block) - ht.dispatch(tpm, 2, test_pacemaker::hs_proposal); //send proposal to replicas (precommit on first block) + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (precommit on first block) qcc_bpa->second->get_state(fs_bpa); BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); @@ -1077,9 +1080,9 @@ BOOST_AUTO_TEST_CASE(hotstuff_7) try { BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); - ht.dispatch(tpm, 2, test_pacemaker::hs_vote); //propagating votes on new proposal (precommitQC on first block) + ht.dispatch(tpm, test_pacemaker::hs_vote); //propagating votes on new proposal (precommitQC on first block) - ht.dispatch(tpm, 2, test_pacemaker::hs_proposal); //send proposal to replicas (commit on first block) + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (commit on first block) qcc_bpa->second->get_state(fs_bpa); BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); @@ -1089,9 +1092,9 @@ BOOST_AUTO_TEST_CASE(hotstuff_7) try { tpm.set_next_leader("bpb"_n); //leader is set to rotate on next block - ht.dispatch(tpm, 2, test_pacemaker::hs_vote); //propagating votes on new proposal (commitQC on first block) + ht.dispatch(tpm, test_pacemaker::hs_vote); //propagating votes on new proposal (commitQC on first block) - ht.dispatch(tpm, 2, test_pacemaker::hs_proposal); //send proposal to replicas (decide on first block) + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (decide on first block) qcc_bpa->second->get_state(fs_bpa); BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("487e5fcbf2c515618941291ae3b6dcebb68942983d8ac3f61c4bdd9901dadbe7")); @@ -1099,7 +1102,7 @@ BOOST_AUTO_TEST_CASE(hotstuff_7) try { BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); - ht.dispatch(tpm, 2, test_pacemaker::hs_vote); //propagating votes on new proposal (decide on first block) + ht.dispatch(tpm, test_pacemaker::hs_vote); //propagating votes on new proposal (decide on first block) tpm.set_proposer("bpb"_n); //leader has rotated tpm.set_leader("bpb"_n); @@ -1108,7 +1111,7 @@ BOOST_AUTO_TEST_CASE(hotstuff_7) try { tpm.beat(); //produce second block and associated proposal - ht.dispatch(tpm, 2, test_pacemaker::hs_proposal); //send proposal to replicas (prepare on second block) + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (prepare on second block) qcc_bpb->second->get_state(fs_bpb); BOOST_CHECK_EQUAL(fs_bpb.b_leaf.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); @@ -1116,9 +1119,9 @@ BOOST_AUTO_TEST_CASE(hotstuff_7) try { BOOST_CHECK_EQUAL(fs_bpb.b_lock.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); BOOST_CHECK_EQUAL(fs_bpb.b_exec.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); - ht.dispatch(tpm, 2, test_pacemaker::hs_vote); //send votes on proposal (prepareQC on second block) + ht.dispatch(tpm, test_pacemaker::hs_vote); //send votes on proposal (prepareQC on second block) - ht.dispatch(tpm, 2, test_pacemaker::hs_proposal); //send proposal to replicas (precommit on second block) + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (precommit on second block) qcc_bpb->second->get_state(fs_bpb); BOOST_CHECK_EQUAL(fs_bpb.b_leaf.str(), std::string("6462add7d157da87931c859cb689f722003a20f30c0f1408d11b872020903b85")); @@ -1126,9 +1129,9 @@ BOOST_AUTO_TEST_CASE(hotstuff_7) try { BOOST_CHECK_EQUAL(fs_bpb.b_lock.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); BOOST_CHECK_EQUAL(fs_bpb.b_exec.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); - ht.dispatch(tpm, 2, test_pacemaker::hs_vote); //propagating votes on new proposal (precommitQC on second block) + ht.dispatch(tpm, test_pacemaker::hs_vote); //propagating votes on new proposal (precommitQC on second block) - ht.dispatch(tpm, 2, test_pacemaker::hs_proposal); //send proposal to replicas (commit on second block) + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (commit on second block) qcc_bpb->second->get_state(fs_bpb); BOOST_CHECK_EQUAL(fs_bpb.b_leaf.str(), std::string("fd77164bf3898a6a8f27ccff440d17ef6870e75c368fcc93b969066cec70939c")); @@ -1136,9 +1139,9 @@ BOOST_AUTO_TEST_CASE(hotstuff_7) try { BOOST_CHECK_EQUAL(fs_bpb.b_lock.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); BOOST_CHECK_EQUAL(fs_bpb.b_exec.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); - ht.dispatch(tpm, 2, test_pacemaker::hs_vote); //propagating votes on new proposal (commitQC on second block) + ht.dispatch(tpm, test_pacemaker::hs_vote); //propagating votes on new proposal (commitQC on second block) - ht.dispatch(tpm, 2, test_pacemaker::hs_proposal); //send proposal to replicas (decide on second block) + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (decide on second block) qcc_bpb->second->get_state(fs_bpb); BOOST_CHECK_EQUAL(fs_bpb.b_leaf.str(), std::string("89f468a127dbadd81b59076067238e3e9c313782d7d83141b16d9da4f2c2b078")); @@ -1301,5 +1304,301 @@ BOOST_AUTO_TEST_CASE(hotstuff_8) try { } FC_LOG_AND_RETHROW(); +BOOST_AUTO_TEST_CASE(hotstuff_9) try { + + //test leader rotation with a star toplogy (message propagation test) + + test_pacemaker tpm; + for (size_t i=0; isecond->get_state(fs_bpa); + auto qcc_bpb = std::find_if(ht._qc_chains.begin(), ht._qc_chains.end(), [&](const auto& q){ return q.first == "bpb"_n; }); + finalizer_state fs_bpb; + qcc_bpb->second->get_state(fs_bpb); + auto qcc_bpc = std::find_if(ht._qc_chains.begin(), ht._qc_chains.end(), [&](const auto& q){ return q.first == "bpc"_n; }); + finalizer_state fs_bpc; + qcc_bpc->second->get_state(fs_bpc); + + tpm.set_current_block_id(ids[0]); //first block + + tpm.beat(); //produce first block and associated proposal + + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (prepare on first block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + + ht.dispatch(tpm, test_pacemaker::hs_vote); //send votes on proposal (prepareQC on first block) + + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (precommit on first block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + + ht.dispatch(tpm, test_pacemaker::hs_vote); //propagating votes on new proposal (precommitQC on first block) + + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (commit on first block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); //4b4 + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f"));//a250 + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8"));//00 + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + + tpm.set_next_leader("bpb"_n); //leader is set to rotate on next block + + ht.dispatch(tpm, test_pacemaker::hs_vote); //propagating votes on new proposal (commitQC on first block) + + ht.dispatch(tpm, test_pacemaker::hs_new_view); + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (decide on first block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("487e5fcbf2c515618941291ae3b6dcebb68942983d8ac3f61c4bdd9901dadbe7")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + + ht.dispatch(tpm, test_pacemaker::hs_new_view); + ht.dispatch(tpm, test_pacemaker::hs_vote); //propagating votes on new proposal (decide on first block) + + tpm.set_proposer("bpb"_n); //leader has rotated + tpm.set_leader("bpb"_n); + + tpm.set_current_block_id(ids[1]); //second block + + tpm.beat(); //produce second block and associated proposal + + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (prepare on second block) + + qcc_bpb->second->get_state(fs_bpb); + BOOST_CHECK_EQUAL(fs_bpb.b_leaf.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + BOOST_CHECK_EQUAL(fs_bpb.high_qc.proposal_id.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + BOOST_CHECK_EQUAL(fs_bpb.b_lock.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + BOOST_CHECK_EQUAL(fs_bpb.b_exec.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + + ht.dispatch(tpm, test_pacemaker::hs_vote); //send votes on proposal (prepareQC on second block) + + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (precommit on second block) + + qcc_bpb->second->get_state(fs_bpb); + BOOST_CHECK_EQUAL(fs_bpb.b_leaf.str(), std::string("6462add7d157da87931c859cb689f722003a20f30c0f1408d11b872020903b85")); + BOOST_CHECK_EQUAL(fs_bpb.high_qc.proposal_id.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + BOOST_CHECK_EQUAL(fs_bpb.b_lock.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + BOOST_CHECK_EQUAL(fs_bpb.b_exec.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + + ht.dispatch(tpm, test_pacemaker::hs_vote); //propagating votes on new proposal (precommitQC on second block) + + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (commit on second block) + + qcc_bpb->second->get_state(fs_bpb); + BOOST_CHECK_EQUAL(fs_bpb.b_leaf.str(), std::string("fd77164bf3898a6a8f27ccff440d17ef6870e75c368fcc93b969066cec70939c")); + BOOST_CHECK_EQUAL(fs_bpb.high_qc.proposal_id.str(), std::string("6462add7d157da87931c859cb689f722003a20f30c0f1408d11b872020903b85")); + BOOST_CHECK_EQUAL(fs_bpb.b_lock.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + BOOST_CHECK_EQUAL(fs_bpb.b_exec.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + + ht.dispatch(tpm, test_pacemaker::hs_vote); //propagating votes on new proposal (commitQC on second block) + + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (decide on second block) + + qcc_bpb->second->get_state(fs_bpb); + BOOST_CHECK_EQUAL(fs_bpb.b_leaf.str(), std::string("89f468a127dbadd81b59076067238e3e9c313782d7d83141b16d9da4f2c2b078")); + BOOST_CHECK_EQUAL(fs_bpb.high_qc.proposal_id.str(), std::string("fd77164bf3898a6a8f27ccff440d17ef6870e75c368fcc93b969066cec70939c")); + BOOST_CHECK_EQUAL(fs_bpb.b_lock.str(), std::string("6462add7d157da87931c859cb689f722003a20f30c0f1408d11b872020903b85")); + BOOST_CHECK_EQUAL(fs_bpb.b_exec.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + + //check bpa as well + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("fd77164bf3898a6a8f27ccff440d17ef6870e75c368fcc93b969066cec70939c")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("6462add7d157da87931c859cb689f722003a20f30c0f1408d11b872020903b85")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + + //check bpc as well + qcc_bpc->second->get_state(fs_bpc); + BOOST_CHECK_EQUAL(fs_bpc.high_qc.proposal_id.str(), std::string("fd77164bf3898a6a8f27ccff440d17ef6870e75c368fcc93b969066cec70939c")); + BOOST_CHECK_EQUAL(fs_bpc.b_lock.str(), std::string("6462add7d157da87931c859cb689f722003a20f30c0f1408d11b872020903b85")); + BOOST_CHECK_EQUAL(fs_bpc.b_exec.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + + BOOST_CHECK_EQUAL(fs_bpa.b_finality_violation.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + +} FC_LOG_AND_RETHROW(); + +BOOST_AUTO_TEST_CASE(hotstuff_10) try { + + //test leader rotation with a ring topology (message propagation test) + + test_pacemaker tpm; + + // zigzag to separate bpa, bpb and bpc. + // cut connections 11,1 *and* 10,0 to see the test fail. + // turning the ring into a line by cutting just one connection is not enough to fail the test. + tpm.connect( { unique_replicas[ 0], unique_replicas[11] } ); + tpm.connect( { unique_replicas[11], unique_replicas[ 1] } ); //cut this to fail (1 of 2) + tpm.connect( { unique_replicas[ 1], unique_replicas[12] } ); + tpm.connect( { unique_replicas[12], unique_replicas[ 2] } ); + tpm.connect( { unique_replicas[ 2], unique_replicas[13] } ); + tpm.connect( { unique_replicas[13], unique_replicas[ 3] } ); + tpm.connect( { unique_replicas[ 3], unique_replicas[14] } ); + tpm.connect( { unique_replicas[14], unique_replicas[ 4] } ); + tpm.connect( { unique_replicas[ 4], unique_replicas[15] } ); + tpm.connect( { unique_replicas[15], unique_replicas[ 5] } ); + tpm.connect( { unique_replicas[ 5], unique_replicas[16] } ); + tpm.connect( { unique_replicas[16], unique_replicas[ 6] } ); + tpm.connect( { unique_replicas[ 6], unique_replicas[17] } ); + tpm.connect( { unique_replicas[17], unique_replicas[ 7] } ); + tpm.connect( { unique_replicas[ 7], unique_replicas[18] } ); + tpm.connect( { unique_replicas[18], unique_replicas[ 8] } ); + tpm.connect( { unique_replicas[ 8], unique_replicas[19] } ); + tpm.connect( { unique_replicas[19], unique_replicas[ 9] } ); + tpm.connect( { unique_replicas[ 9], unique_replicas[20] } ); + tpm.connect( { unique_replicas[20], unique_replicas[10] } ); + tpm.connect( { unique_replicas[10], unique_replicas[ 0] } ); //cut this to fail (2 of 2) + + hotstuff_test_handler ht; + + ht.initialize_qc_chains(tpm, unique_replicas); + + tpm.set_proposer("bpa"_n); + tpm.set_leader("bpa"_n); + tpm.set_next_leader("bpa"_n); + tpm.set_finalizers(unique_replicas); + + auto qcc_bpa = std::find_if(ht._qc_chains.begin(), ht._qc_chains.end(), [&](const auto& q){ return q.first == "bpa"_n; }); + finalizer_state fs_bpa; + qcc_bpa->second->get_state(fs_bpa); + auto qcc_bpb = std::find_if(ht._qc_chains.begin(), ht._qc_chains.end(), [&](const auto& q){ return q.first == "bpb"_n; }); + finalizer_state fs_bpb; + qcc_bpb->second->get_state(fs_bpb); + auto qcc_bpc = std::find_if(ht._qc_chains.begin(), ht._qc_chains.end(), [&](const auto& q){ return q.first == "bpc"_n; }); + finalizer_state fs_bpc; + qcc_bpc->second->get_state(fs_bpc); + + tpm.set_current_block_id(ids[0]); //first block + + tpm.beat(); //produce first block and associated proposal + + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (prepare on first block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + + ht.dispatch(tpm, test_pacemaker::hs_vote); //send votes on proposal (prepareQC on first block) + + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (precommit on first block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + + ht.dispatch(tpm, test_pacemaker::hs_vote); //propagating votes on new proposal (precommitQC on first block) + + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (commit on first block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + + tpm.set_next_leader("bpb"_n); //leader is set to rotate on next block + + ht.dispatch(tpm, test_pacemaker::hs_vote); //propagating votes on new proposal (commitQC on first block) + + ht.dispatch(tpm, test_pacemaker::hs_new_view); + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (decide on first block) + + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.b_leaf.str(), std::string("487e5fcbf2c515618941291ae3b6dcebb68942983d8ac3f61c4bdd9901dadbe7")); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + + ht.dispatch(tpm, test_pacemaker::hs_new_view); + ht.dispatch(tpm, test_pacemaker::hs_vote); //propagating votes on new proposal (decide on first block) + + tpm.set_proposer("bpb"_n); //leader has rotated + tpm.set_leader("bpb"_n); + + tpm.set_current_block_id(ids[1]); //second block + + tpm.beat(); //produce second block and associated proposal + + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (prepare on second block) + + qcc_bpb->second->get_state(fs_bpb); + BOOST_CHECK_EQUAL(fs_bpb.b_leaf.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + BOOST_CHECK_EQUAL(fs_bpb.high_qc.proposal_id.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + BOOST_CHECK_EQUAL(fs_bpb.b_lock.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + BOOST_CHECK_EQUAL(fs_bpb.b_exec.str(), std::string("a252070cd26d3b231ab2443b9ba97f57fc72e50cca04a020952e45bc7e2d27a8")); + + ht.dispatch(tpm, test_pacemaker::hs_vote); //send votes on proposal (prepareQC on second block) + + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (precommit on second block) + + qcc_bpb->second->get_state(fs_bpb); + BOOST_CHECK_EQUAL(fs_bpb.b_leaf.str(), std::string("6462add7d157da87931c859cb689f722003a20f30c0f1408d11b872020903b85")); + BOOST_CHECK_EQUAL(fs_bpb.high_qc.proposal_id.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + BOOST_CHECK_EQUAL(fs_bpb.b_lock.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + BOOST_CHECK_EQUAL(fs_bpb.b_exec.str(), std::string("4b43fb144a8b5e874777f61f3b37d7a8b06c33fbc48db464ce0e8788ff4edb4f")); + + ht.dispatch(tpm, test_pacemaker::hs_vote); //propagating votes on new proposal (precommitQC on second block) + + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (commit on second block) + + qcc_bpb->second->get_state(fs_bpb); + BOOST_CHECK_EQUAL(fs_bpb.b_leaf.str(), std::string("fd77164bf3898a6a8f27ccff440d17ef6870e75c368fcc93b969066cec70939c")); + BOOST_CHECK_EQUAL(fs_bpb.high_qc.proposal_id.str(), std::string("6462add7d157da87931c859cb689f722003a20f30c0f1408d11b872020903b85")); + BOOST_CHECK_EQUAL(fs_bpb.b_lock.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + BOOST_CHECK_EQUAL(fs_bpb.b_exec.str(), std::string("aedf8bb1ee70bd6e743268f7fe0f8171418aa43a68bb9c6e7329ffa856896c09")); + + ht.dispatch(tpm, test_pacemaker::hs_vote); //propagating votes on new proposal (commitQC on second block) + + ht.dispatch(tpm, test_pacemaker::hs_proposal); //send proposal to replicas (decide on second block) + + qcc_bpb->second->get_state(fs_bpb); + BOOST_CHECK_EQUAL(fs_bpb.b_leaf.str(), std::string("89f468a127dbadd81b59076067238e3e9c313782d7d83141b16d9da4f2c2b078")); + BOOST_CHECK_EQUAL(fs_bpb.high_qc.proposal_id.str(), std::string("fd77164bf3898a6a8f27ccff440d17ef6870e75c368fcc93b969066cec70939c")); + BOOST_CHECK_EQUAL(fs_bpb.b_lock.str(), std::string("6462add7d157da87931c859cb689f722003a20f30c0f1408d11b872020903b85")); + BOOST_CHECK_EQUAL(fs_bpb.b_exec.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + + //check bpa as well + qcc_bpa->second->get_state(fs_bpa); + BOOST_CHECK_EQUAL(fs_bpa.high_qc.proposal_id.str(), std::string("fd77164bf3898a6a8f27ccff440d17ef6870e75c368fcc93b969066cec70939c")); + BOOST_CHECK_EQUAL(fs_bpa.b_lock.str(), std::string("6462add7d157da87931c859cb689f722003a20f30c0f1408d11b872020903b85")); + BOOST_CHECK_EQUAL(fs_bpa.b_exec.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + + //check bpc as well + qcc_bpc->second->get_state(fs_bpc); + BOOST_CHECK_EQUAL(fs_bpc.high_qc.proposal_id.str(), std::string("fd77164bf3898a6a8f27ccff440d17ef6870e75c368fcc93b969066cec70939c")); + BOOST_CHECK_EQUAL(fs_bpc.b_lock.str(), std::string("6462add7d157da87931c859cb689f722003a20f30c0f1408d11b872020903b85")); + BOOST_CHECK_EQUAL(fs_bpc.b_exec.str(), std::string("1511035fdcbabdc5e272a3ac19356536252884ed77077cf871ae5029a7502279")); + + BOOST_CHECK_EQUAL(fs_bpa.b_finality_violation.str(), std::string("0000000000000000000000000000000000000000000000000000000000000000")); + +} FC_LOG_AND_RETHROW(); + + BOOST_AUTO_TEST_SUITE_END() diff --git a/libraries/hotstuff/test/test_pacemaker.cpp b/libraries/hotstuff/test/test_pacemaker.cpp index f285dc899f..0491a67639 100644 --- a/libraries/hotstuff/test/test_pacemaker.cpp +++ b/libraries/hotstuff/test/test_pacemaker.cpp @@ -232,7 +232,8 @@ namespace eosio::hotstuff { void test_pacemaker::on_hs_new_block_msg(const hs_new_block_message& msg, name id) { for (const auto& [qcc_name, qcc_ptr] : _qcc_store) { - if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name) && is_connected(id, qcc_ptr->get_id_i())) + // New Block msg is not propagated by qc_chain, so it has to go to everyone (no is_connected() check) + if (qcc_ptr->get_id_i() != id && is_qc_chain_active(qcc_name)) qcc_ptr->on_hs_new_block_msg(0, msg); } } From 577a7cd4f9b1c5dd70c02f53fc9bfbd1338d3f26 Mon Sep 17 00:00:00 2001 From: fcecin Date: Mon, 25 Sep 2023 14:03:56 -0300 Subject: [PATCH 05/65] Fix new-view message infinite propagation loop in some testcases --- libraries/hotstuff/qc_chain.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libraries/hotstuff/qc_chain.cpp b/libraries/hotstuff/qc_chain.cpp index 27a4eef6fb..500f36715f 100644 --- a/libraries/hotstuff/qc_chain.cpp +++ b/libraries/hotstuff/qc_chain.cpp @@ -729,7 +729,12 @@ namespace eosio::hotstuff { _b_leaf = _high_qc.get_proposal_id(); fc_tlog(_logger, " === ${id} _b_leaf updated (update_high_qc) : ${proposal_id}", ("proposal_id", _high_qc.get_proposal_id())("id", _id)); - return true; + + // avoid looping message propagation when receiving a new-view message with a high_qc.get_proposal_id().empty(). + // not sure if high_qc.get_proposal_id().empty() + _high_qc.get_proposal_id().empty() is something that actually ever happens in the real world. + // not sure if high_qc.get_proposal_id().empty() should be tested and always rejected (return false + no _high_qc / _b_leaf update). + // if this returns false, we won't update the get_finality_status information, but I don't think we care about that at all. + return !high_qc.get_proposal_id().empty(); } else { const hs_proposal_message *old_high_qc_prop = get_proposal( _high_qc.get_proposal_id() ); const hs_proposal_message *new_high_qc_prop = get_proposal( high_qc.get_proposal_id() ); From 33b23e6304ff8685d7c4a95eece04bc050b255d6 Mon Sep 17 00:00:00 2001 From: fcecin Date: Tue, 3 Oct 2023 15:47:36 -0300 Subject: [PATCH 06/65] Delete commented-out code --- libraries/hotstuff/qc_chain.cpp | 9 --------- 1 file changed, 9 deletions(-) diff --git a/libraries/hotstuff/qc_chain.cpp b/libraries/hotstuff/qc_chain.cpp index 2359d4c0dd..59ef5d09f1 100644 --- a/libraries/hotstuff/qc_chain.cpp +++ b/libraries/hotstuff/qc_chain.cpp @@ -405,7 +405,6 @@ namespace eosio::hotstuff { bool am_leader = am_i_leader(); -//<<<<<<< HEAD if (am_leader) { if (vote.proposal_id != _current_qc.get_proposal_id()) { send_hs_message_warning(connection_id, hs_message_warning::discarded); // example; to be tuned to actual need @@ -417,14 +416,6 @@ namespace eosio::hotstuff { if (p == nullptr) { if (am_leader) fc_elog(_logger, " *** ${id} couldn't find proposal, vote : ${vote}", ("id",_id)("vote", vote)); -//======= -// if (!am_leader) -// return; -// fc_tlog(_logger, " === Process vote from ${finalizer_key} : current bitset ${value}" , -// ("finalizer_key", vote.finalizer_key)("value", _current_qc.get_active_finalizers_string())); -// // only leader need to take action on votes -// if (vote.proposal_id != _current_qc.get_proposal_id()) { -//>>>>>>> origin/hotstuff_integration send_hs_message_warning(connection_id, hs_message_warning::discarded); // example; to be tuned to actual need return; } From fd61028a67148b447faf891571679008b69802fa Mon Sep 17 00:00:00 2001 From: fcecin Date: Wed, 18 Oct 2023 17:00:16 -0300 Subject: [PATCH 07/65] Improve seen-votes GC --- libraries/hotstuff/qc_chain.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/libraries/hotstuff/qc_chain.cpp b/libraries/hotstuff/qc_chain.cpp index 59ef5d09f1..17e94e6485 100644 --- a/libraries/hotstuff/qc_chain.cpp +++ b/libraries/hotstuff/qc_chain.cpp @@ -988,11 +988,8 @@ namespace eosio::hotstuff { void qc_chain::gc_proposals(uint64_t cutoff){ //fc_tlog(_logger, " === garbage collection on old data"); - auto sv_end_itr = _seen_votes_store.get().upper_bound(cutoff); - while (_seen_votes_store.get().begin() != sv_end_itr){ - auto itr = _seen_votes_store.get().begin(); - _seen_votes_store.get().erase(itr); - } + auto& seen_votes_index = _seen_votes_store.get(); + seen_votes_index.erase(seen_votes_index.begin(), seen_votes_index.upper_bound(cutoff)); #ifdef QC_CHAIN_SIMPLE_PROPOSAL_STORE ps_height_iterator psh_it = _proposal_stores_by_height.begin(); From db98dbc5814accf4e9936d0c34ee65ef6e6832fc Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 24 Oct 2023 11:52:39 -0500 Subject: [PATCH 08/65] GH-1690 Socket should only be accessed from thread-pool thread. Use async_write instead of send. Wait for all write callbacks to finish before exit. --- tests/trx_generator/trx_provider.cpp | 31 +++++++++++++++++++++++----- tests/trx_generator/trx_provider.hpp | 15 +++++++++++--- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/tests/trx_generator/trx_provider.cpp b/tests/trx_generator/trx_provider.cpp index 190c6b4632..7ace099ac3 100644 --- a/tests/trx_generator/trx_provider.cpp +++ b/tests/trx_generator/trx_provider.cpp @@ -73,15 +73,36 @@ namespace eosio::testing { } void p2p_connection::disconnect() { - ilog("Closing socket."); - _p2p_socket.close(); - ilog("Socket closed."); + int max = 30; + int waited = 0; + while (_sent.load() != _sent_callback_num.load() && waited < max) { + ilog("disconnect waiting on ack - sent ${s} | acked ${a} | waited ${w}", + ("s", _sent.load())("a", _sent_callback_num.load())("w", waited)); + sleep(1); + ++waited; + } + if (waited == max) { + elog("disconnect failed to receive all acks in time - sent ${s} | acked ${a} | waited ${w}", + ("s", _sent.load())("a", _sent_callback_num.load())("w", waited)); + } } void p2p_connection::send_transaction(const chain::packed_transaction& trx) { send_buffer_type msg = create_send_buffer(trx); - _p2p_socket.send(boost::asio::buffer(*msg)); - trx_acknowledged(trx.id(), fc::time_point::min()); //using min to identify ack time as not applicable for p2p + + ++_sent; + _strand.post( [this, msg{std::move(msg)}, id{trx.id()}]() { + boost::asio::async_write( _p2p_socket, boost::asio::buffer(*msg), + boost::asio::bind_executor( _strand, [this, msg, id]( boost::system::error_code ec, std::size_t w ) { + if (ec) { + elog("async write failure: ${e}", ("e", ec.message())); + } else { + trx_acknowledged(id, fc::time_point::min()); //using min to identify ack time as not applicable for p2p + } + ++_sent_callback_num; + }) + ); + } ); } acked_trx_trace_info p2p_connection::get_acked_trx_trace_info(const eosio::chain::transaction_id_type& trx_id) { diff --git a/tests/trx_generator/trx_provider.hpp b/tests/trx_generator/trx_provider.hpp index f11cb13f09..b53536ca74 100644 --- a/tests/trx_generator/trx_provider.hpp +++ b/tests/trx_generator/trx_provider.hpp @@ -2,9 +2,13 @@ #include #include -#include -#include #include + +#include + +#include +#include + #include #include #include @@ -104,10 +108,15 @@ namespace eosio::testing { struct p2p_connection : public provider_connection { boost::asio::ip::tcp::socket _p2p_socket; + boost::asio::io_context::strand _strand; + std::atomic _sent_callback_num{0}; + std::atomic _sent{0}; + explicit p2p_connection(const provider_base_config& provider_config) : provider_connection(provider_config) - , _p2p_socket(_connection_thread_pool.get_executor()) {} + , _p2p_socket(_connection_thread_pool.get_executor()) + , _strand(_connection_thread_pool.get_executor()){} void send_transaction(const chain::packed_transaction& trx) final; From 82e214634c45109c94b4fab813566d128c2ed88d Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 24 Oct 2023 11:54:00 -0500 Subject: [PATCH 09/65] GH-1690 Need to wait over time of run, so offset from startBlock. Make --print-missing-transactions=True the default as it is useful when there are missing trxs. --- tests/PerformanceHarness/log_reader.py | 2 +- tests/PerformanceHarness/performance_test_basic.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/PerformanceHarness/log_reader.py b/tests/PerformanceHarness/log_reader.py index ef68d4b4d5..388596c4b0 100644 --- a/tests/PerformanceHarness/log_reader.py +++ b/tests/PerformanceHarness/log_reader.py @@ -39,7 +39,7 @@ class TpsTestConfig: numTrxGensUsed: int = 0 targetTpsPerGenList: List[int] = field(default_factory=list) quiet: bool = False - printMissingTransactions: bool=False + printMissingTransactions: bool=True @dataclass class stats(): diff --git a/tests/PerformanceHarness/performance_test_basic.py b/tests/PerformanceHarness/performance_test_basic.py index 0d56329985..47d01bb242 100755 --- a/tests/PerformanceHarness/performance_test_basic.py +++ b/tests/PerformanceHarness/performance_test_basic.py @@ -171,7 +171,7 @@ class PtbConfig: quiet: bool=False delPerfLogs: bool=False expectedTransactionsSent: int = field(default_factory=int, init=False) - printMissingTransactions: bool=False + printMissingTransactions: bool=True userTrxDataFile: Path=None endpointMode: str="p2p" apiEndpoint: str=None @@ -495,7 +495,7 @@ def configureConnections(): scrapeTrxGenTrxSentDataLogs(trxSent, self.trxGenLogDirPath, self.ptbConfig.quiet) if len(trxSent) != self.ptbConfig.expectedTransactionsSent: print(f"ERROR: Transactions generated: {len(trxSent)} does not match the expected number of transactions: {self.ptbConfig.expectedTransactionsSent}") - blocksToWait = 2 * self.ptbConfig.testTrxGenDurationSec + 10 + blocksToWait = self.data.startBlock + (2 * self.ptbConfig.testTrxGenDurationSec) + 10 trxNotFound = self.validationNode.waitForTransactionsInBlockRange(trxSent, self.data.startBlock, blocksToWait) self.data.ceaseBlock = self.validationNode.getHeadBlockNum() @@ -733,7 +733,7 @@ def _createBaseArgumentParser(defEndpointApiDef: str, defProdNodeCnt: int, defVa ptbBaseParserGroup.add_argument("--save-state", help=argparse.SUPPRESS if suppressHelp else "Whether to save node state. (Warning: large disk usage)", action='store_true') ptbBaseParserGroup.add_argument("--quiet", help=argparse.SUPPRESS if suppressHelp else "Whether to quiet printing intermediate results and reports to stdout", action='store_true') ptbBaseParserGroup.add_argument("--prods-enable-trace-api", help=argparse.SUPPRESS if suppressHelp else "Determines whether producer nodes should have eosio::trace_api_plugin enabled", action='store_true') - ptbBaseParserGroup.add_argument("--print-missing-transactions", help=argparse.SUPPRESS if suppressHelp else "Toggles if missing transactions are be printed upon test completion.", action='store_true') + ptbBaseParserGroup.add_argument("--print-missing-transactions", type=bool, help=argparse.SUPPRESS if suppressHelp else "Toggles if missing transactions are be printed upon test completion.", default=True) ptbBaseParserGroup.add_argument("--account-name", type=str, help=argparse.SUPPRESS if suppressHelp else "Name of the account to create and assign a contract to", default="eosio") ptbBaseParserGroup.add_argument("--contract-dir", type=str, help=argparse.SUPPRESS if suppressHelp else "Path to contract dir", default="unittests/contracts/eosio.system") ptbBaseParserGroup.add_argument("--wasm-file", type=str, help=argparse.SUPPRESS if suppressHelp else "WASM file name for contract", default="eosio.system.wasm") From 339f5424659ee231c652de9e394d48a6bf5d659e Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 24 Oct 2023 13:28:21 -0500 Subject: [PATCH 10/65] GH-1690 Changed waitForTransactionsInBlockRange to use a start/end instead of start/offset. Pass in the start of empty blocks as the end of the range. --- tests/PerformanceHarness/performance_test_basic.py | 4 ++-- tests/TestHarness/Node.py | 9 ++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/PerformanceHarness/performance_test_basic.py b/tests/PerformanceHarness/performance_test_basic.py index 47d01bb242..a6fc881057 100755 --- a/tests/PerformanceHarness/performance_test_basic.py +++ b/tests/PerformanceHarness/performance_test_basic.py @@ -491,12 +491,12 @@ def configureConnections(): completedRun = True # Get stats after transaction generation stops + endBlock = self.waitForEmptyBlocks(self.validationNode, self.emptyBlockGoal) trxSent = {} scrapeTrxGenTrxSentDataLogs(trxSent, self.trxGenLogDirPath, self.ptbConfig.quiet) if len(trxSent) != self.ptbConfig.expectedTransactionsSent: print(f"ERROR: Transactions generated: {len(trxSent)} does not match the expected number of transactions: {self.ptbConfig.expectedTransactionsSent}") - blocksToWait = self.data.startBlock + (2 * self.ptbConfig.testTrxGenDurationSec) + 10 - trxNotFound = self.validationNode.waitForTransactionsInBlockRange(trxSent, self.data.startBlock, blocksToWait) + trxNotFound = self.validationNode.waitForTransactionsInBlockRange(trxSent, self.data.startBlock, endBlock) self.data.ceaseBlock = self.validationNode.getHeadBlockNum() return PerformanceTestBasic.PtbTpsTestResult(completedRun=completedRun, numGeneratorsUsed=tpsTrxGensConfig.numGenerators, diff --git a/tests/TestHarness/Node.py b/tests/TestHarness/Node.py index b529990b6f..1a166de7e5 100644 --- a/tests/TestHarness/Node.py +++ b/tests/TestHarness/Node.py @@ -161,19 +161,18 @@ def checkBlockForTransactions(self, transIds, blockNum): transIds.pop(self.fetchTransactionFromTrace(trx)) return transIds - def waitForTransactionsInBlockRange(self, transIds, startBlock=2, maxFutureBlocks=0): + def waitForTransactionsInBlockRange(self, transIds, startBlock, endBlock): nextBlockToProcess = startBlock - overallEndBlock = startBlock + maxFutureBlocks while len(transIds) > 0: currentLoopEndBlock = self.getHeadBlockNum() - if currentLoopEndBlock > overallEndBlock: - currentLoopEndBlock = overallEndBlock + if currentLoopEndBlock > endBlock: + currentLoopEndBlock = endBlock for blockNum in range(nextBlockToProcess, currentLoopEndBlock + 1): transIds = self.checkBlockForTransactions(transIds, blockNum) if len(transIds) == 0: return transIds nextBlockToProcess = currentLoopEndBlock + 1 - if currentLoopEndBlock == overallEndBlock: + if currentLoopEndBlock == endBlock: Utils.Print("ERROR: Transactions were missing upon expiration of waitOnblockTransactions") break self.waitForHeadToAdvance() From bfe2a4c93918ac1665bffcd35e3ad6d8ec3fbdd7 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 25 Oct 2023 12:19:34 -0500 Subject: [PATCH 11/65] GH-1690 Add backwards compatibility for `--produce-block-offset-ms 0` for performance harness --- tests/TestHarness/Cluster.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/TestHarness/Cluster.py b/tests/TestHarness/Cluster.py index fc0a6b055a..cc954ba36a 100644 --- a/tests/TestHarness/Cluster.py +++ b/tests/TestHarness/Cluster.py @@ -461,6 +461,11 @@ def connectGroup(group, producerNodes, bridgeNodes) : if "--plugin eosio::history_api_plugin" in args: argsArr.append("--is-nodeos-v2") break + + # Handle common case of specifying no block offset for older versions + if "v2" in self.nodeosVers or "v3" in self.nodeosVers or "v4" in self.nodeosVers: + argsArr = list(map(lambda st: str.replace(st, "--produce-block-offset-ms 0", "--last-block-time-offset-us 0 --last-block-cpu-effort-percent 100"), argsArr)) + Cluster.__LauncherCmdArr = argsArr.copy() launcher = cluster_generator(argsArr) From 6788f441caf042554b6d4f8704651ae7abd81499 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Thu, 26 Oct 2023 15:06:26 -0400 Subject: [PATCH 12/65] don't use zstd for .deb packages yet --- package.cmake | 3 --- 1 file changed, 3 deletions(-) diff --git a/package.cmake b/package.cmake index c4847f6aef..00bbffeddb 100644 --- a/package.cmake +++ b/package.cmake @@ -46,9 +46,6 @@ set(CPACK_PACKAGE_HOMEPAGE_URL "https://github.com/AntelopeIO/leap") set(CPACK_DEBIAN_PACKAGE_SHLIBDEPS ON) set(CPACK_DEBIAN_BASE_PACKAGE_SECTION "utils") -if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.22) - set(CPACK_DEBIAN_COMPRESSION_TYPE "zstd") -endif() set(CPACK_DEBIAN_PACKAGE_CONFLICTS "eosio, mandel") set(CPACK_RPM_PACKAGE_CONFLICTS "eosio, mandel") From b79d98dd91bd1b8d03ffbc337234a67ee38ec814 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 27 Oct 2023 08:54:03 -0500 Subject: [PATCH 13/65] GH-1690 Not allowed to write to socket until previous write completes. --- tests/trx_generator/trx_provider.cpp | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/tests/trx_generator/trx_provider.cpp b/tests/trx_generator/trx_provider.cpp index 7ace099ac3..eb946245f9 100644 --- a/tests/trx_generator/trx_provider.cpp +++ b/tests/trx_generator/trx_provider.cpp @@ -92,16 +92,9 @@ namespace eosio::testing { ++_sent; _strand.post( [this, msg{std::move(msg)}, id{trx.id()}]() { - boost::asio::async_write( _p2p_socket, boost::asio::buffer(*msg), - boost::asio::bind_executor( _strand, [this, msg, id]( boost::system::error_code ec, std::size_t w ) { - if (ec) { - elog("async write failure: ${e}", ("e", ec.message())); - } else { - trx_acknowledged(id, fc::time_point::min()); //using min to identify ack time as not applicable for p2p - } - ++_sent_callback_num; - }) - ); + boost::asio::write(_p2p_socket, boost::asio::buffer(*msg)); + trx_acknowledged(id, fc::time_point::min()); //using min to identify ack time as not applicable for p2p + ++_sent_callback_num; } ); } From e531f8b1a57aedb4ad1db9e3c12e7ce7eb9699e9 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 30 Oct 2023 21:20:28 -0500 Subject: [PATCH 14/65] GH-1837 Simplify calculate_producing_block_deadline to a simple calculation not based on now --- .../eosio/producer_plugin/block_timing_util.hpp | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/plugins/producer_plugin/include/eosio/producer_plugin/block_timing_util.hpp b/plugins/producer_plugin/include/eosio/producer_plugin/block_timing_util.hpp index 27ba83ff6c..c077af1760 100644 --- a/plugins/producer_plugin/include/eosio/producer_plugin/block_timing_util.hpp +++ b/plugins/producer_plugin/include/eosio/producer_plugin/block_timing_util.hpp @@ -55,16 +55,7 @@ namespace block_timing_util { } inline fc::time_point calculate_producing_block_deadline(fc::microseconds cpu_effort, chain::block_timestamp_type block_time) { - auto estimated_deadline = production_round_block_start_time(cpu_effort, block_time) + cpu_effort; - auto now = fc::time_point::now(); - if (estimated_deadline > now) { - return estimated_deadline; - } else { - // This could only happen when the producer stop producing and then comes back alive in the middle of its own - // production round. In this case, we just use the hard deadline. - const auto hard_deadline = block_time.to_time_point() - fc::microseconds(chain::config::block_interval_us - cpu_effort.count()); - return std::min(hard_deadline, now + cpu_effort); - } + return production_round_block_start_time(cpu_effort, block_time) + cpu_effort; } namespace detail { From 2bc20816828784f04f3a66cf9d401884f748ab90 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 30 Oct 2023 21:21:37 -0500 Subject: [PATCH 15/65] GH-1837 Add and revise tests for calculate_producing_block_deadline --- .../test/test_block_timing_util.cpp | 32 ++++++++++++++++--- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/plugins/producer_plugin/test/test_block_timing_util.cpp b/plugins/producer_plugin/test/test_block_timing_util.cpp index a3a43452e8..066cc505b3 100644 --- a/plugins/producer_plugin/test/test_block_timing_util.cpp +++ b/plugins/producer_plugin/test/test_block_timing_util.cpp @@ -51,11 +51,12 @@ BOOST_AUTO_TEST_CASE(test_calculate_block_deadline) { { // Scenario 2: // In producing mode and it is already too late to meet the optimized deadlines, - // the returned deadline can never be later than the hard deadlines. + // the returned deadline can never be later than the next block deadline. + // now is not taken into account, but leaving set_now in to verify it has no affect. auto second_block_time = eosio::chain::block_timestamp_type(production_round_1st_block_slot + 1); fc::mock_time_traits::set_now(second_block_time.to_time_point() - fc::milliseconds(200)); - auto second_block_hard_deadline = second_block_time.to_time_point() - fc::milliseconds(100); + auto second_block_hard_deadline = second_block_time.to_time_point() - fc::milliseconds(200); BOOST_CHECK_EQUAL(calculate_producing_block_deadline(cpu_effort, second_block_time), second_block_hard_deadline); // use previous deadline as now @@ -76,21 +77,42 @@ BOOST_AUTO_TEST_CASE(test_calculate_block_deadline) { fc::mock_time_traits::set_now(seventh_block_time.to_time_point() - fc::milliseconds(500)); BOOST_CHECK_EQUAL(calculate_producing_block_deadline(cpu_effort, seventh_block_time), - seventh_block_time.to_time_point() - fc::milliseconds(100)); + seventh_block_time.to_time_point() - fc::milliseconds(700)); // use previous deadline as now fc::mock_time_traits::set_now(seventh_block_time.to_time_point() - fc::milliseconds(100)); auto eighth_block_time = eosio::chain::block_timestamp_type(production_round_1st_block_slot + 7); BOOST_CHECK_EQUAL(calculate_producing_block_deadline(cpu_effort, eighth_block_time), - eighth_block_time.to_time_point() - fc::milliseconds(200)); + eighth_block_time.to_time_point() - fc::milliseconds(800)); // use previous deadline as now fc::mock_time_traits::set_now(eighth_block_time.to_time_point() - fc::milliseconds(200)); auto ninth_block_time = eosio::chain::block_timestamp_type(production_round_1st_block_slot + 8); BOOST_CHECK_EQUAL(calculate_producing_block_deadline(cpu_effort, ninth_block_time), - ninth_block_time.to_time_point() - fc::milliseconds(300)); + ninth_block_time.to_time_point() - fc::milliseconds(900)); + } + { + // Scenario: time has passed for block to be produced + // Ask for a deadline for current block but its deadline has already passed + auto default_cpu_effort = fc::microseconds(block_interval.count() - (450000/12)); // 462,500 + // with the default_cpu_effort, the first block deadline would be 29.9625 (29.500 + 0.4625) + fc::mock_time_traits::set_now(fc::time_point::from_iso_string("2023-10-29T13:40:29.963")); // past first block interval + auto first_block_time = eosio::chain::block_timestamp_type(fc::time_point::from_iso_string("2023-10-29T13:40:30.000")); + // pass the first block deadline + fc::time_point block_deadline = calculate_producing_block_deadline(default_cpu_effort, first_block_time); + // not equal since block_deadline is 29.962500 + BOOST_CHECK_NE(block_deadline, fc::time_point::from_iso_string("2023-10-29T13:40:29.962")); + BOOST_CHECK_EQUAL(block_deadline.to_iso_string(), fc::time_point::from_iso_string("2023-10-29T13:40:29.962").to_iso_string()); // truncates + + // second block has passed + fc::mock_time_traits::set_now(fc::time_point::from_iso_string("2023-10-29T13:40:30.426")); // past second block interval + auto second_block_time = eosio::chain::block_timestamp_type(fc::time_point::from_iso_string("2023-10-29T13:40:30.500")); + // pass the second block deadline + block_deadline = calculate_producing_block_deadline(default_cpu_effort, second_block_time); + // 29.962500 + (450000/12) = 30.425 + BOOST_CHECK_EQUAL(block_deadline, fc::time_point::from_iso_string("2023-10-29T13:40:30.425")); } } From 4fb6c11f18a755d5b899418af15960db7415f214 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 30 Oct 2023 21:22:08 -0500 Subject: [PATCH 16/65] GH-1837 Add test to verify only one start block per block unless interrupted --- plugins/producer_plugin/producer_plugin.cpp | 2 +- tests/TestHarness/Node.py | 29 +++++++++++++++++++++ tests/distributed-transactions-test.py | 5 ++++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index c610d9b5b6..4d4c7d4741 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -772,9 +772,9 @@ class producer_plugin_impl : public std::enable_shared_from_thischain().head_block_num() + 1)); // abort the pending block abort_block(); - schedule_production_loop(); } diff --git a/tests/TestHarness/Node.py b/tests/TestHarness/Node.py index b529990b6f..41afbd43da 100644 --- a/tests/TestHarness/Node.py +++ b/tests/TestHarness/Node.py @@ -550,6 +550,35 @@ def findInLog(self, searchStr): return True return False + # verify only one 'Starting block' per block number unless block is restarted + def verifyOnlyOneStartingBlock(self): + dataDir=Utils.getNodeDataDir(self.nodeId) + files=Node.findStderrFiles(dataDir) + for f in files: + blockNumbers = set() + duplicatesFound = False + lastRestartBlockNum = 0 + + with open(f, 'r') as file: + for line in file: + match = re.match(r".*Restarting exhausted speculative block #(\d+)", line) + if match: + lastRestartBlockNum = match.group(1) + if re.match(r".*unlinkable_block_exception", line): + lastRestartBlockNum = blockNumber + match = re.match(r".*Starting block #(\d+)", line) + if match: + blockNumber = match.group(1) + if blockNumber != lastRestartBlockNum and blockNumber in blockNumbers: + print(f"Duplicate Staring block found: {blockNumber} in {f}") + duplicatesFound = True + blockNumbers.add(blockNumber) + + if duplicatesFound: + return False + + return True + def analyzeProduction(self, specificBlockNum=None, thresholdMs=500): dataDir=Utils.getNodeDataDir(self.nodeId) files=Node.findStderrFiles(dataDir) diff --git a/tests/distributed-transactions-test.py b/tests/distributed-transactions-test.py index d95dbf64df..62df717e2a 100755 --- a/tests/distributed-transactions-test.py +++ b/tests/distributed-transactions-test.py @@ -111,6 +111,11 @@ cluster.compareBlockLogs() + # verify only one start block per block unless interrupted + for node in cluster.getAllNodes(): + if not node.verifyOnlyOneStartingBlock(): + errorExit("Found more than one Starting block in logs") + testSuccessful=True finally: TestHelper.shutdown(cluster, walletMgr, testSuccessful, dumpErrorDetails) From 4a9dd03485754d97a89e7772df99689418b2e315 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 31 Oct 2023 10:30:10 -0500 Subject: [PATCH 17/65] GH-1837 Do not start a speculative block that will immediately be restarted --- plugins/producer_plugin/producer_plugin.cpp | 11 +++++++++-- .../producer_plugin/test/test_block_timing_util.cpp | 7 +++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 4d4c7d4741..e5fcf20dd2 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -1829,8 +1829,6 @@ producer_plugin_impl::start_block_result producer_plugin_impl::start_block() { last_start_block_time = now; } - // create speculative blocks at regular intervals, so we create blocks with "current" block time - _pending_block_deadline = block_timing_util::calculate_producing_block_deadline(_produce_block_cpu_effort, block_time); if (in_producing_mode()) { uint32_t production_round_index = block_timestamp_type(block_time).slot % chain::config::producer_repetitions; if (production_round_index == 0) { @@ -1844,6 +1842,15 @@ producer_plugin_impl::start_block_result producer_plugin_impl::start_block() { } } + // create speculative blocks at regular intervals, so we create blocks with "current" block time + _pending_block_deadline = block_timing_util::calculate_producing_block_deadline(_produce_block_cpu_effort, block_time); + if (in_speculating_mode()) { // if we are producing, then produce block even if deadline has pasted + // speculative block, no reason to start a block that will immediately be re-started, set deadline in the future + // a block should come in during this time, if not then just keep creating the block every produce_block_cpu_effort + if (now + fc::milliseconds(config::block_interval_ms/10) > _pending_block_deadline) { + _pending_block_deadline = now + _produce_block_cpu_effort; + } + } const auto& preprocess_deadline = _pending_block_deadline; fc_dlog(_log, "Starting block #${n} at ${time} producer ${p}", ("n", pending_block_num)("time", now)("p", scheduled_producer.producer_name)); diff --git a/plugins/producer_plugin/test/test_block_timing_util.cpp b/plugins/producer_plugin/test/test_block_timing_util.cpp index 066cc505b3..8943a6ea3f 100644 --- a/plugins/producer_plugin/test/test_block_timing_util.cpp +++ b/plugins/producer_plugin/test/test_block_timing_util.cpp @@ -114,6 +114,13 @@ BOOST_AUTO_TEST_CASE(test_calculate_block_deadline) { // 29.962500 + (450000/12) = 30.425 BOOST_CHECK_EQUAL(block_deadline, fc::time_point::from_iso_string("2023-10-29T13:40:30.425")); } + { + // Real scenario that caused multiple start blocks + auto default_cpu_effort = fc::microseconds(block_interval.count() - (450000/12)); // 462,500 + auto block_time = eosio::chain::block_timestamp_type(fc::time_point::from_iso_string("2023-10-31T13:37:35.000")); + fc::time_point block_deadline = calculate_producing_block_deadline(default_cpu_effort, block_time); + BOOST_CHECK_EQUAL(block_deadline.to_iso_string(), fc::time_point::from_iso_string("2023-10-31T13:37:34.587").to_iso_string()); + } } BOOST_AUTO_TEST_CASE(test_calculate_producer_wake_up_time) { From b6fd7e16ef49c7ec33b0a5560c544251545cc6c5 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Tue, 31 Oct 2023 13:26:35 -0400 Subject: [PATCH 18/65] disable eosvmoc subjective limits in unit tests --- .../webassembly/eos-vm-oc/code_cache.hpp | 1 + .../chain/webassembly/eos-vm-oc/config.hpp | 12 ++++++++++ .../webassembly/eos-vm-oc/ipc_protocol.hpp | 3 ++- .../runtimes/eos-vm-oc/LLVMJIT.cpp | 10 ++++++--- .../webassembly/runtimes/eos-vm-oc/LLVMJIT.h | 2 +- .../runtimes/eos-vm-oc/code_cache.cpp | 7 +++--- .../runtimes/eos-vm-oc/compile_monitor.cpp | 6 ++--- .../runtimes/eos-vm-oc/compile_trampoline.cpp | 22 +++++++++++++------ libraries/testing/tester.cpp | 11 ++++++++++ 9 files changed, 56 insertions(+), 18 deletions(-) diff --git a/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/code_cache.hpp b/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/code_cache.hpp index d753a9dcaa..f638dc8751 100644 --- a/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/code_cache.hpp +++ b/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/code_cache.hpp @@ -69,6 +69,7 @@ class code_cache_base { code_cache_index _cache_index; const chainbase::database& _db; + eosvmoc::config _eosvmoc_config; std::filesystem::path _cache_file_path; int _cache_fd; diff --git a/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp b/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp index b3f2563831..1de0293bd0 100644 --- a/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp +++ b/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp @@ -7,11 +7,23 @@ #include +#include + namespace eosio { namespace chain { namespace eosvmoc { struct config { uint64_t cache_size = 1024u*1024u*1024u; uint64_t threads = 1u; + + // subjective limits. + // nodeos uses the default values. libtester sets them as required. + rlimit cpu_limits {20u, 20u}; + rlimit vm_limits {512u*1024u*1024u, 512u*1024u*1024u}; + uint64_t stack_size_limit {16u*1024u}; + size_t generated_code_size_limit {16u*1024u*1024u}; }; }}} + +FC_REFLECT(rlimit, (rlim_cur)(rlim_max)) +FC_REFLECT(eosio::chain::eosvmoc::config, (cache_size)(threads)(cpu_limits)(vm_limits)(stack_size_limit)(generated_code_size_limit)) diff --git a/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/ipc_protocol.hpp b/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/ipc_protocol.hpp index 2674a2dbd4..59fb7f10fb 100644 --- a/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/ipc_protocol.hpp +++ b/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/ipc_protocol.hpp @@ -22,6 +22,7 @@ struct code_tuple { struct compile_wasm_message { code_tuple code; + eosvmoc::config eosvmoc_config; //Two sent fd: 1) communication socket for result, 2) the wasm to compile }; @@ -62,7 +63,7 @@ using eosvmoc_message = std::variantcode); } - instantiated_code instantiateModule(const IR::Module& module) + instantiated_code instantiateModule(const IR::Module& module, uint64_t stack_size_limit, size_t generated_code_size_limit) { static bool inited; if(!inited) { @@ -315,13 +315,17 @@ namespace LLVMJIT WAVM_ASSERT_THROW(!!c); ++num_functions_stack_size_found; - if(stack_size > 16u*1024u) + // enforce stack_size_limit only when it is not max (libtester may + // disable it by setting it to max). + if(stack_size_limit != std::numeric_limits::max() && stack_size > stack_size_limit) _exit(1); } } if(num_functions_stack_size_found != module.functions.defs.size()) _exit(1); - if(jitModule->final_pic_code.size() >= 16u*1024u*1024u) + // enforce generated_code_size_limit only when it is not max (libtester may + // disable it by setting it to max). + if(generated_code_size_limit != std::numeric_limits::max() && jitModule->final_pic_code.size() >= generated_code_size_limit) _exit(1); instantiated_code ret; diff --git a/libraries/chain/webassembly/runtimes/eos-vm-oc/LLVMJIT.h b/libraries/chain/webassembly/runtimes/eos-vm-oc/LLVMJIT.h index 13e2510195..0cd022ce3b 100644 --- a/libraries/chain/webassembly/runtimes/eos-vm-oc/LLVMJIT.h +++ b/libraries/chain/webassembly/runtimes/eos-vm-oc/LLVMJIT.h @@ -15,6 +15,6 @@ struct instantiated_code { }; namespace LLVMJIT { - instantiated_code instantiateModule(const IR::Module& module); + instantiated_code instantiateModule(const IR::Module& module, uint64_t stack_size_limit, size_t generated_code_size_limit); } }}} diff --git a/libraries/chain/webassembly/runtimes/eos-vm-oc/code_cache.cpp b/libraries/chain/webassembly/runtimes/eos-vm-oc/code_cache.cpp index 7c48b5b079..e4a23686e8 100644 --- a/libraries/chain/webassembly/runtimes/eos-vm-oc/code_cache.cpp +++ b/libraries/chain/webassembly/runtimes/eos-vm-oc/code_cache.cpp @@ -126,7 +126,7 @@ const code_descriptor* const code_cache_async::get_descriptor_for_code(bool high _outstanding_compiles_and_poison.emplace(*nextup, false); std::vector fds_to_pass; fds_to_pass.emplace_back(memfd_for_bytearray(codeobject->code)); - FC_ASSERT(write_message_with_fds(_compile_monitor_write_socket, compile_wasm_message{ *nextup }, fds_to_pass), "EOS VM failed to communicate to OOP manager"); + FC_ASSERT(write_message_with_fds(_compile_monitor_write_socket, compile_wasm_message{ *nextup, _eosvmoc_config }, fds_to_pass), "EOS VM failed to communicate to OOP manager"); --count_processed; } _queued_compiles.erase(nextup); @@ -179,7 +179,7 @@ const code_descriptor* const code_cache_async::get_descriptor_for_code(bool high _outstanding_compiles_and_poison.emplace(ct, false); std::vector fds_to_pass; fds_to_pass.emplace_back(memfd_for_bytearray(codeobject->code)); - write_message_with_fds(_compile_monitor_write_socket, compile_wasm_message{ ct }, fds_to_pass); + write_message_with_fds(_compile_monitor_write_socket, compile_wasm_message{ ct, _eosvmoc_config }, fds_to_pass); failure = get_cd_failure::temporary; // Compile might not be done yet return nullptr; } @@ -211,7 +211,7 @@ const code_descriptor* const code_cache_sync::get_descriptor_for_code_sync(const std::vector fds_to_pass; fds_to_pass.emplace_back(memfd_for_bytearray(codeobject->code)); - write_message_with_fds(_compile_monitor_write_socket, compile_wasm_message{ {code_id, vm_version} }, fds_to_pass); + write_message_with_fds(_compile_monitor_write_socket, compile_wasm_message{ {code_id, vm_version}, _eosvmoc_config }, fds_to_pass); auto [success, message, fds] = read_message_with_fds(_compile_monitor_read_socket); EOS_ASSERT(success, wasm_execution_error, "failed to read response from monitor process"); EOS_ASSERT(std::holds_alternative(message), wasm_execution_error, "unexpected response from monitor process"); @@ -226,6 +226,7 @@ const code_descriptor* const code_cache_sync::get_descriptor_for_code_sync(const code_cache_base::code_cache_base(const std::filesystem::path& data_dir, const eosvmoc::config& eosvmoc_config, const chainbase::database& db) : _db(db), + _eosvmoc_config(eosvmoc_config), _cache_file_path(data_dir/"code_cache.bin") { static_assert(sizeof(allocator_t) <= header_offset, "header offset intersects with allocator"); diff --git a/libraries/chain/webassembly/runtimes/eos-vm-oc/compile_monitor.cpp b/libraries/chain/webassembly/runtimes/eos-vm-oc/compile_monitor.cpp index 01ae7ecaba..92b55da717 100644 --- a/libraries/chain/webassembly/runtimes/eos-vm-oc/compile_monitor.cpp +++ b/libraries/chain/webassembly/runtimes/eos-vm-oc/compile_monitor.cpp @@ -71,7 +71,7 @@ struct compile_monitor_session { connection_dead_signal(); return; } - kick_compile_off(compile.code, std::move(fds[0])); + kick_compile_off(compile.code, compile.eosvmoc_config, std::move(fds[0])); }, [&](const evict_wasms_message& evict) { for(const code_descriptor& cd : evict.codes) { @@ -90,7 +90,7 @@ struct compile_monitor_session { }); } - void kick_compile_off(const code_tuple& code_id, wrapped_fd&& wasm_code) { + void kick_compile_off(const code_tuple& code_id, const eosvmoc::config& eosvmoc_config, wrapped_fd&& wasm_code) { //prepare a requst to go out to the trampoline int socks[2]; socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC, 0, socks); @@ -100,7 +100,7 @@ struct compile_monitor_session { fds_pass_to_trampoline.emplace_back(socks[1]); fds_pass_to_trampoline.emplace_back(std::move(wasm_code)); - eosvmoc_message trampoline_compile_request = compile_wasm_message{code_id}; + eosvmoc_message trampoline_compile_request = compile_wasm_message{code_id, eosvmoc_config}; if(write_message_with_fds(_trampoline_socket, trampoline_compile_request, fds_pass_to_trampoline) == false) { wasm_compilation_result_message reply{code_id, compilation_result_unknownfailure{}, _allocator->get_free_memory()}; write_message_with_fds(_nodeos_instance_socket, reply); diff --git a/libraries/chain/webassembly/runtimes/eos-vm-oc/compile_trampoline.cpp b/libraries/chain/webassembly/runtimes/eos-vm-oc/compile_trampoline.cpp index b2f16a6d87..de506119f2 100644 --- a/libraries/chain/webassembly/runtimes/eos-vm-oc/compile_trampoline.cpp +++ b/libraries/chain/webassembly/runtimes/eos-vm-oc/compile_trampoline.cpp @@ -17,7 +17,7 @@ using namespace IR; namespace eosio { namespace chain { namespace eosvmoc { -void run_compile(wrapped_fd&& response_sock, wrapped_fd&& wasm_code) noexcept { //noexcept; we'll just blow up if anything tries to cross this boundry +void run_compile(wrapped_fd&& response_sock, wrapped_fd&& wasm_code, uint64_t stack_size_limit, size_t generated_code_size_limit) noexcept { //noexcept; we'll just blow up if anything tries to cross this boundry std::vector wasm = vector_for_memfd(wasm_code); //ideally we catch exceptions and sent them upstream as strings for easier reporting @@ -30,7 +30,7 @@ void run_compile(wrapped_fd&& response_sock, wrapped_fd&& wasm_code) noexcept { wasm_injections::wasm_binary_injection injector(module); injector.inject(); - instantiated_code code = LLVMJIT::instantiateModule(module); + instantiated_code code = LLVMJIT::instantiateModule(module, stack_size_limit, generated_code_size_limit); code_compilation_result_message result_message; @@ -165,16 +165,24 @@ void run_compile_trampoline(int fd) { prctl(PR_SET_NAME, "oc-compile"); prctl(PR_SET_PDEATHSIG, SIGKILL); - struct rlimit cpu_limits = {20u, 20u}; - setrlimit(RLIMIT_CPU, &cpu_limits); + const auto& conf = std::get(message).eosvmoc_config; - struct rlimit vm_limits = {512u*1024u*1024u, 512u*1024u*1024u}; - setrlimit(RLIMIT_AS, &vm_limits); + // enforce cpu_limits only when it is not RLIM_INFINITY (libtester may + // disable it by setting it to RLIM_INFINITY). + if(conf.cpu_limits.rlim_cur != RLIM_INFINITY) { + setrlimit(RLIMIT_CPU, &conf.cpu_limits); + } + + // enforce vm_limits only when it is not RLIM_INFINITY (libtester may + // disable it by setting it to RLIM_INFINITY). + if(conf.vm_limits.rlim_cur != RLIM_INFINITY) { + setrlimit(RLIMIT_AS, &conf.vm_limits); + } struct rlimit core_limits = {0u, 0u}; setrlimit(RLIMIT_CORE, &core_limits); - run_compile(std::move(fds[0]), std::move(fds[1])); + run_compile(std::move(fds[0]), std::move(fds[1]), conf.stack_size_limit, conf.generated_code_size_limit); _exit(0); } else if(pid == -1) diff --git a/libraries/testing/tester.cpp b/libraries/testing/tester.cpp index 8a9b7a0ad6..2db0b28f11 100644 --- a/libraries/testing/tester.cpp +++ b/libraries/testing/tester.cpp @@ -317,6 +317,17 @@ namespace eosio { namespace testing { } } + // libtester does not enforce EOSVMOC limits for all tests except those in + // eosvmoc_limits_test.cpp, where one or more of the limits are set to + // max to indicate the limits are enforced. + if (cfg.eosvmoc_config.cpu_limits.rlim_cur != RLIM_INFINITY && cfg.eosvmoc_config.vm_limits.rlim_cur != RLIM_INFINITY && cfg.eosvmoc_config.stack_size_limit != std::numeric_limits::max() && cfg.eosvmoc_config.generated_code_size_limit != std::numeric_limits::max()) { + // set to inifinity or max such that they are not enforced. + cfg.eosvmoc_config.cpu_limits.rlim_cur = RLIM_INFINITY; + cfg.eosvmoc_config.vm_limits.rlim_cur = RLIM_INFINITY; + cfg.eosvmoc_config.stack_size_limit = std::numeric_limits::max(); + cfg.eosvmoc_config.generated_code_size_limit = std::numeric_limits::max(); + } + control.reset( new controller(cfg, std::move(pfs), *expected_chain_id) ); control->add_indices(); if (lambda) lambda(); From c012b146713eafa5a038df854aca4ea79c7258e3 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Tue, 31 Oct 2023 13:27:20 -0400 Subject: [PATCH 19/65] add eosvmoc limits tests --- unittests/eosvmoc_limits_tests.cpp | 139 +++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) create mode 100644 unittests/eosvmoc_limits_tests.cpp diff --git a/unittests/eosvmoc_limits_tests.cpp b/unittests/eosvmoc_limits_tests.cpp new file mode 100644 index 0000000000..a0d096cead --- /dev/null +++ b/unittests/eosvmoc_limits_tests.cpp @@ -0,0 +1,139 @@ +#ifdef EOSIO_EOS_VM_OC_RUNTIME_ENABLED + +#include +#include +#include + +using namespace eosio; +using namespace eosio::chain; +using namespace eosio::testing; +using mvo = fc::mutable_variant_object; + +BOOST_AUTO_TEST_SUITE(eosvmoc_limits_tests) + +// common routine to verify wasm_execution_error is raised when a resource +// limit specified in eosvmoc_config is reached +void limit_violated_test(const eosvmoc::config eosvmoc_config) { + fc::temp_directory tempdir; + + constexpr bool use_genesis = true; + validating_tester chain( + tempdir, + [&](controller::config& cfg) { + cfg.eosvmoc_config = eosvmoc_config; + }, + use_genesis + ); + + chain.create_accounts({"eosio.token"_n}); + chain.set_code("eosio.token"_n, test_contracts::eosio_token_wasm()); + chain.set_abi("eosio.token"_n, test_contracts::eosio_token_abi()); + + if (chain.control->is_eos_vm_oc_enabled()) { + BOOST_CHECK_EXCEPTION( + chain.push_action( "eosio.token"_n, "create"_n, "eosio.token"_n, mvo() + ( "issuer", "eosio.token" ) + ( "maximum_supply", "1000000.00 TOK" )), + eosio::chain::wasm_execution_error, + [](const eosio::chain::wasm_execution_error& e) { + return expect_assert_message(e, "failed to compile wasm"); + } + ); + } else { + chain.push_action( "eosio.token"_n, "create"_n, "eosio.token"_n, mvo() + ( "issuer", "eosio.token" ) + ( "maximum_supply", "1000000.00 TOK" ) + ); + } +} + +// common routine to verify no wasm_execution_error is raised +// because limits specified in eosvmoc_config are not reached +void limit_not_violated_test(const eosvmoc::config eosvmoc_config) { + fc::temp_directory tempdir; + + constexpr bool use_genesis = true; + validating_tester chain( + tempdir, + [&](controller::config& cfg) { + cfg.eosvmoc_config = eosvmoc_config; + }, + use_genesis + ); + + chain.create_accounts({"eosio.token"_n}); + chain.set_code("eosio.token"_n, test_contracts::eosio_token_wasm()); + chain.set_abi("eosio.token"_n, test_contracts::eosio_token_abi()); + + chain.push_action( "eosio.token"_n, "create"_n, "eosio.token"_n, mvo() + ( "issuer", "eosio.token" ) + ( "maximum_supply", "1000000.00 TOK" ) + ); +} + +// test libtester does not enforce limits +BOOST_AUTO_TEST_CASE( default_limits ) { try { + eosvmoc::config eosvmoc_config; + limit_not_violated_test(eosvmoc_config); +} FC_LOG_AND_RETHROW() } + +// test VM limits are checked +BOOST_AUTO_TEST_CASE( vm_limits ) { try { + eosvmoc::config eosvmoc_config; + + // disable non-tested limits + eosvmoc_config.cpu_limits.rlim_cur = RLIM_INFINITY; + eosvmoc_config.stack_size_limit = std::numeric_limits::max(); + eosvmoc_config.generated_code_size_limit = std::numeric_limits::max(); + + // set vm_limit to a small value such that it is exceeded + eosvmoc_config.vm_limits.rlim_cur = 64u*1024u*1024u; + limit_violated_test(eosvmoc_config); + + // set vm_limit to a large value such that it is not exceeded + eosvmoc_config.vm_limits.rlim_cur = 128u*1024u*1024u; + limit_not_violated_test(eosvmoc_config); +} FC_LOG_AND_RETHROW() } + +// test stack size limit is checked +BOOST_AUTO_TEST_CASE( stack_limit ) { try { + eosvmoc::config eosvmoc_config; + + // disable non-tested limits + eosvmoc_config.cpu_limits.rlim_cur = RLIM_INFINITY; + eosvmoc_config.vm_limits.rlim_cur = RLIM_INFINITY; + eosvmoc_config.generated_code_size_limit = std::numeric_limits::max(); + + // The larget stack size in the test is 104. + // Set stack_size_limit to highest value to verify wasm_execution_error is raised + eosvmoc_config.stack_size_limit = 103; + limit_violated_test(eosvmoc_config); + + // set stack_size_limit to lowest value to verify wasm_execution_error is not raised + eosvmoc_config.stack_size_limit = 104; + limit_not_violated_test(eosvmoc_config); +} FC_LOG_AND_RETHROW() } + +// test generated code size limit is checked +BOOST_AUTO_TEST_CASE( generated_code_size_limit ) { try { + eosvmoc::config eosvmoc_config; + + // disable non-tested limits + eosvmoc_config.cpu_limits.rlim_cur = RLIM_INFINITY; + eosvmoc_config.vm_limits.rlim_cur = RLIM_INFINITY; + eosvmoc_config.stack_size_limit = std::numeric_limits::max(); + + // The sgenerated code size in the test is 36856. + // Set generated_code_size_limit to highest value to verify wasm_execution_error + // is raised + eosvmoc_config.generated_code_size_limit = 36856; + limit_violated_test(eosvmoc_config); + + // set generated_code_size_limit to lowest value to verify wasm_execution_error is not raised + eosvmoc_config.generated_code_size_limit = 36857; + limit_not_violated_test(eosvmoc_config); +} FC_LOG_AND_RETHROW() } + +BOOST_AUTO_TEST_SUITE_END() + +#endif From 1dac3fb540c66dabb4991ff79b57955771a18533 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 31 Oct 2023 13:43:29 -0500 Subject: [PATCH 20/65] GH-1837 Use full cpu effort for calculated wake up time for producer --- plugins/producer_plugin/producer_plugin.cpp | 2 +- .../test/test_block_timing_util.cpp | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index e5fcf20dd2..7b1a4e124e 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -2495,7 +2495,7 @@ void producer_plugin_impl::schedule_production_loop() { chain::controller& chain = chain_plug->chain(); fc_dlog(_log, "Speculative Block Created; Scheduling Speculative/Production Change"); EOS_ASSERT(chain.is_building_block(), missing_pending_block_state, "speculating without pending_block_state"); - auto wake_time = block_timing_util::calculate_producer_wake_up_time(_produce_block_cpu_effort, chain.pending_block_num(), chain.pending_block_timestamp(), + auto wake_time = block_timing_util::calculate_producer_wake_up_time(fc::microseconds{config::block_interval_us}, chain.pending_block_num(), chain.pending_block_timestamp(), _producers, chain.head_block_state()->active_schedule.producers, _producer_watermarks); schedule_delayed_production_loop(weak_from_this(), wake_time); diff --git a/plugins/producer_plugin/test/test_block_timing_util.cpp b/plugins/producer_plugin/test/test_block_timing_util.cpp index 8943a6ea3f..6ba697ab6d 100644 --- a/plugins/producer_plugin/test/test_block_timing_util.cpp +++ b/plugins/producer_plugin/test/test_block_timing_util.cpp @@ -277,6 +277,24 @@ BOOST_AUTO_TEST_CASE(test_calculate_producer_wake_up_time) { expected_block_time = block_timestamp_type(prod_round_1st_block_slot + 2*config::producer_repetitions + 2).to_time_point(); // with watermark, wait until next BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, prod_watermarks), expected_block_time); } + { // actual example that caused multiple start blocks + producer_watermarks prod_watermarks; + std::set producers = { + "inita"_n, "initb"_n, "initc"_n, "initd"_n, "inite"_n, "initf"_n, "initg"_n, "p1"_n, + "inith"_n, "initi"_n, "initj"_n, "initk"_n, "initl"_n, "initm"_n, "initn"_n, + "inito"_n, "initp"_n, "initq"_n, "initr"_n, "inits"_n, "initt"_n, "initu"_n, "p2"_n + }; + std::vector active_schedule{ + {"inita"_n}, {"initb"_n}, {"initc"_n}, {"initd"_n}, {"inite"_n}, {"initf"_n}, {"initg"_n}, + {"inith"_n}, {"initi"_n}, {"initj"_n}, {"initk"_n}, {"initl"_n}, {"initm"_n}, {"initn"_n}, + {"inito"_n}, {"initp"_n}, {"initq"_n}, {"initr"_n}, {"inits"_n}, {"initt"_n}, {"initu"_n} + }; + auto default_cpu_effort = fc::microseconds(block_interval.count() - (450000/12)); // 462,500 + auto wake_time = calculate_producer_wake_up_time(default_cpu_effort, 106022362, eosio::chain::block_timestamp_type(fc::time_point::from_iso_string("2023-10-31T16:06:41.000")), + producers, active_schedule, prod_watermarks); + BOOST_REQUIRE(!!wake_time); + BOOST_CHECK_EQUAL(wake_time->to_iso_string(), fc::time_point::from_iso_string("2023-10-31T16:06:40.587").to_iso_string()); + } } From 79dccc525d8bc04722282ab110a0d324c927de51 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 31 Oct 2023 15:14:43 -0500 Subject: [PATCH 21/65] GH-1837 Allow a couple of start blocks per block as forks cause multiple --- tests/TestHarness/Node.py | 9 ++++++--- tests/distributed-transactions-test.py | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/TestHarness/Node.py b/tests/TestHarness/Node.py index 41afbd43da..d8a8ca1a77 100644 --- a/tests/TestHarness/Node.py +++ b/tests/TestHarness/Node.py @@ -550,12 +550,13 @@ def findInLog(self, searchStr): return True return False - # verify only one 'Starting block' per block number unless block is restarted - def verifyOnlyOneStartingBlock(self): + # verify only one or two 'Starting block' per block number unless block is restarted + def verifyStartingBlockMessages(self): dataDir=Utils.getNodeDataDir(self.nodeId) files=Node.findStderrFiles(dataDir) for f in files: blockNumbers = set() + duplicateBlockNumbers = set() duplicatesFound = False lastRestartBlockNum = 0 @@ -569,9 +570,11 @@ def verifyOnlyOneStartingBlock(self): match = re.match(r".*Starting block #(\d+)", line) if match: blockNumber = match.group(1) - if blockNumber != lastRestartBlockNum and blockNumber in blockNumbers: + if blockNumber != lastRestartBlockNum and blockNumber in duplicateBlockNumbers: print(f"Duplicate Staring block found: {blockNumber} in {f}") duplicatesFound = True + if blockNumber != lastRestartBlockNum and blockNumber in blockNumbers: + duplicateBlockNumbers.add(blockNumber) blockNumbers.add(blockNumber) if duplicatesFound: diff --git a/tests/distributed-transactions-test.py b/tests/distributed-transactions-test.py index 62df717e2a..985ad83168 100755 --- a/tests/distributed-transactions-test.py +++ b/tests/distributed-transactions-test.py @@ -113,7 +113,7 @@ # verify only one start block per block unless interrupted for node in cluster.getAllNodes(): - if not node.verifyOnlyOneStartingBlock(): + if not node.verifyStartingBlockMessages(): errorExit("Found more than one Starting block in logs") testSuccessful=True From 5682bd4c0932bc234ac221834b9aaeb0eee54c7a Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 31 Oct 2023 15:59:29 -0500 Subject: [PATCH 22/65] GH-1837 Verify wake time is in the future --- plugins/producer_plugin/producer_plugin.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 7b1a4e124e..991094c69e 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -2498,6 +2498,10 @@ void producer_plugin_impl::schedule_production_loop() { auto wake_time = block_timing_util::calculate_producer_wake_up_time(fc::microseconds{config::block_interval_us}, chain.pending_block_num(), chain.pending_block_timestamp(), _producers, chain.head_block_state()->active_schedule.producers, _producer_watermarks); + if (wake_time && fc::time_point::now() > *wake_time) { + // if wake time has already passed then use the block deadline instead + wake_time = _pending_block_deadline; + } schedule_delayed_production_loop(weak_from_this(), wake_time); } else { fc_dlog(_log, "Speculative Block Created"); From 87260ae815b33fc6c87ddbf08873f740bb171609 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 1 Nov 2023 07:02:08 -0500 Subject: [PATCH 23/65] GH-1837 Fix spelling --- plugins/producer_plugin/producer_plugin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 991094c69e..679a2b82a4 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -1844,7 +1844,7 @@ producer_plugin_impl::start_block_result producer_plugin_impl::start_block() { // create speculative blocks at regular intervals, so we create blocks with "current" block time _pending_block_deadline = block_timing_util::calculate_producing_block_deadline(_produce_block_cpu_effort, block_time); - if (in_speculating_mode()) { // if we are producing, then produce block even if deadline has pasted + if (in_speculating_mode()) { // if we are producing, then produce block even if deadline has passed // speculative block, no reason to start a block that will immediately be re-started, set deadline in the future // a block should come in during this time, if not then just keep creating the block every produce_block_cpu_effort if (now + fc::milliseconds(config::block_interval_ms/10) > _pending_block_deadline) { From 6e275cd8ecd044a7360f6a470f41cbae9dd7f0cf Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 1 Nov 2023 07:40:18 -0500 Subject: [PATCH 24/65] GH-1690 Recover keys on the chain thread pool --- plugins/producer_plugin/producer_plugin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index c610d9b5b6..6e27b7427f 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -802,7 +802,7 @@ class producer_plugin_impl : public std::enable_shared_from_this Date: Wed, 1 Nov 2023 08:49:10 -0500 Subject: [PATCH 25/65] GH-1837 Add comments --- plugins/producer_plugin/producer_plugin.cpp | 1 + plugins/producer_plugin/test/test_block_timing_util.cpp | 3 +++ 2 files changed, 4 insertions(+) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 679a2b82a4..df4ce39ddd 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -772,6 +772,7 @@ class producer_plugin_impl : public std::enable_shared_from_thischain().head_block_num() + 1)); // abort the pending block abort_block(); diff --git a/plugins/producer_plugin/test/test_block_timing_util.cpp b/plugins/producer_plugin/test/test_block_timing_util.cpp index 6ba697ab6d..1c9108710e 100644 --- a/plugins/producer_plugin/test/test_block_timing_util.cpp +++ b/plugins/producer_plugin/test/test_block_timing_util.cpp @@ -76,6 +76,7 @@ BOOST_AUTO_TEST_CASE(test_calculate_block_deadline) { auto seventh_block_time = eosio::chain::block_timestamp_type(production_round_1st_block_slot + 6); fc::mock_time_traits::set_now(seventh_block_time.to_time_point() - fc::milliseconds(500)); + // 7th block where cpu effort is 100ms less per block BOOST_CHECK_EQUAL(calculate_producing_block_deadline(cpu_effort, seventh_block_time), seventh_block_time.to_time_point() - fc::milliseconds(700)); @@ -83,6 +84,7 @@ BOOST_AUTO_TEST_CASE(test_calculate_block_deadline) { fc::mock_time_traits::set_now(seventh_block_time.to_time_point() - fc::milliseconds(100)); auto eighth_block_time = eosio::chain::block_timestamp_type(production_round_1st_block_slot + 7); + // 8th block where cpu effort is 100ms less per block BOOST_CHECK_EQUAL(calculate_producing_block_deadline(cpu_effort, eighth_block_time), eighth_block_time.to_time_point() - fc::milliseconds(800)); @@ -90,6 +92,7 @@ BOOST_AUTO_TEST_CASE(test_calculate_block_deadline) { fc::mock_time_traits::set_now(eighth_block_time.to_time_point() - fc::milliseconds(200)); auto ninth_block_time = eosio::chain::block_timestamp_type(production_round_1st_block_slot + 8); + // 9th block where cpu effort is 100ms less per block BOOST_CHECK_EQUAL(calculate_producing_block_deadline(cpu_effort, ninth_block_time), ninth_block_time.to_time_point() - fc::milliseconds(900)); } From 7e34c672415dcf4a64e6ae0bd0098fdf36c120d2 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 1 Nov 2023 08:49:35 -0500 Subject: [PATCH 26/65] GH-1837 Cleanup --- tests/TestHarness/Node.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/TestHarness/Node.py b/tests/TestHarness/Node.py index d8a8ca1a77..fb2f70e0a5 100644 --- a/tests/TestHarness/Node.py +++ b/tests/TestHarness/Node.py @@ -557,30 +557,30 @@ def verifyStartingBlockMessages(self): for f in files: blockNumbers = set() duplicateBlockNumbers = set() - duplicatesFound = False + threeStartsFound = False lastRestartBlockNum = 0 + blockNumber = 0 with open(f, 'r') as file: for line in file: match = re.match(r".*Restarting exhausted speculative block #(\d+)", line) if match: lastRestartBlockNum = match.group(1) + continue if re.match(r".*unlinkable_block_exception", line): lastRestartBlockNum = blockNumber + continue match = re.match(r".*Starting block #(\d+)", line) if match: blockNumber = match.group(1) if blockNumber != lastRestartBlockNum and blockNumber in duplicateBlockNumbers: print(f"Duplicate Staring block found: {blockNumber} in {f}") - duplicatesFound = True + threeStartsFound = True if blockNumber != lastRestartBlockNum and blockNumber in blockNumbers: duplicateBlockNumbers.add(blockNumber) blockNumbers.add(blockNumber) - if duplicatesFound: - return False - - return True + return not threeStartsFound def analyzeProduction(self, specificBlockNum=None, thresholdMs=500): dataDir=Utils.getNodeDataDir(self.nodeId) From bed5a26bac087e1b77a9b1988b5841e870e1a4a5 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 1 Nov 2023 10:52:19 -0500 Subject: [PATCH 27/65] GH-1690 Report values used in comparison --- tests/trx_generator/trx_provider.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/trx_generator/trx_provider.cpp b/tests/trx_generator/trx_provider.cpp index eb946245f9..4ff29007fd 100644 --- a/tests/trx_generator/trx_provider.cpp +++ b/tests/trx_generator/trx_provider.cpp @@ -75,9 +75,11 @@ namespace eosio::testing { void p2p_connection::disconnect() { int max = 30; int waited = 0; - while (_sent.load() != _sent_callback_num.load() && waited < max) { + for (uint64_t sent = _sent.load(), sent_callback_num = _sent_callback_num.load(); + sent != sent_callback_num && waited < max; + sent = _sent.load(), sent_callback_num = _sent_callback_num.load()) { ilog("disconnect waiting on ack - sent ${s} | acked ${a} | waited ${w}", - ("s", _sent.load())("a", _sent_callback_num.load())("w", waited)); + ("s", sent)("a", sent_callback_num)("w", waited)); sleep(1); ++waited; } From 2717d0a135c015d36e7dec855f590989d298cd79 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Wed, 1 Nov 2023 13:42:16 -0400 Subject: [PATCH 28/65] globally --catch_system_errors=no --- CMakeLists.txt | 4 ++++ tests/CMakeLists.txt | 2 +- unittests/CMakeLists.txt | 4 ++-- unittests/wasm-spec-tests/generated-tests/CMakeLists.txt | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ff2708fadb..08441b0116 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -175,6 +175,10 @@ endif() # do not attempt to use an external OpenSSL in any manner set(CMAKE_DISABLE_FIND_PACKAGE_OpenSSL On) +# many tests require handling of signals themselves and even when they don't we'd like for them to generate a core dump, this +# is effectively --catch_system_errors=no broadly across all tests +add_compile_definitions(BOOST_TEST_DEFAULTS_TO_CORE_DUMP) + add_subdirectory( libraries ) add_subdirectory( plugins ) add_subdirectory( programs ) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index fb26b0c350..182c68614f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -81,7 +81,7 @@ else() endif() #To run plugin_test with all log from blockchain displayed, put --verbose after --, i.e. plugin_test -- --verbose -add_test(NAME plugin_test COMMAND plugin_test --report_level=detailed --color_output --catch_system_errors=no) +add_test(NAME plugin_test COMMAND plugin_test --report_level=detailed --color_output) add_test(NAME nodeos_sanity_test COMMAND tests/nodeos_run_test.py -v --sanity-test ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST nodeos_sanity_test PROPERTY LABELS nonparallelizable_tests) diff --git a/unittests/CMakeLists.txt b/unittests/CMakeLists.txt index f85ff7cdeb..c1e3eed7a2 100644 --- a/unittests/CMakeLists.txt +++ b/unittests/CMakeLists.txt @@ -85,7 +85,7 @@ target_include_directories( unit_test PUBLIC ${CMAKE_SOURCE_DIR}/plugins/chain_interface/include) ### MARK TEST SUITES FOR EXECUTION ### -add_test(NAME protocol_feature_digest_unit_test COMMAND unit_test --run_test=protocol_feature_digest_tests --report_level=detailed --color_output --catch_system_errors=no) +add_test(NAME protocol_feature_digest_unit_test COMMAND unit_test --run_test=protocol_feature_digest_tests --report_level=detailed --color_output) set(ctest_tests "protocol_feature_digest_tests") foreach(TEST_SUITE ${UNIT_TESTS}) # create an independent target for each test suite execute_process(COMMAND sh -c "grep -E 'BOOST_AUTO_TEST_SUITE\\s*[(]' '${TEST_SUITE}' | grep -vE '//.*BOOST_AUTO_TEST_SUITE\\s*[(]' | cut -d ')' -f 1 | cut -d '(' -f 2" OUTPUT_VARIABLE SUITE_NAME OUTPUT_STRIP_TRAILING_WHITESPACE) # get the test suite name from the *.cpp file @@ -93,7 +93,7 @@ foreach(TEST_SUITE ${UNIT_TESTS}) # create an independent target for each test s execute_process(COMMAND sh -c "echo ${SUITE_NAME} | sed -e 's/s$//' | sed -e 's/_test$//'" OUTPUT_VARIABLE TRIMMED_SUITE_NAME OUTPUT_STRIP_TRAILING_WHITESPACE) # trim "_test" or "_tests" from the end of ${SUITE_NAME} # to run unit_test with all log from blockchain displayed, put "--verbose" after "--", i.e. "unit_test -- --verbose" foreach(RUNTIME ${EOSIO_WASM_RUNTIMES}) - add_test(NAME ${TRIMMED_SUITE_NAME}_unit_test_${RUNTIME} COMMAND unit_test --run_test=${SUITE_NAME} --report_level=detailed --color_output --catch_system_errors=no -- --${RUNTIME}) + add_test(NAME ${TRIMMED_SUITE_NAME}_unit_test_${RUNTIME} COMMAND unit_test --run_test=${SUITE_NAME} --report_level=detailed --color_output -- --${RUNTIME}) # build list of tests to run during coverage testing if(ctest_tests) string(APPEND ctest_tests "|") diff --git a/unittests/wasm-spec-tests/generated-tests/CMakeLists.txt b/unittests/wasm-spec-tests/generated-tests/CMakeLists.txt index 87225c902c..25169702fb 100644 --- a/unittests/wasm-spec-tests/generated-tests/CMakeLists.txt +++ b/unittests/wasm-spec-tests/generated-tests/CMakeLists.txt @@ -24,7 +24,7 @@ foreach(TEST_SUITE ${WASM_TESTS}) # create an independent target for each test s foreach(RUNTIME ${EOSIO_WASM_RUNTIMES}) get_test_property(${SN}_unit_test_${RUNTIME} LABELS TEST_LABEL) if ("NOTFOUND" STREQUAL "${TEST_LABEL}") # prevent duplicates - add_test(NAME ${SN}_unit_test_${RUNTIME} COMMAND wasm_spec_test --run_test=${SN} --report_level=detailed --color_output --catch_system_errors=no -- --${RUNTIME}) + add_test(NAME ${SN}_unit_test_${RUNTIME} COMMAND wasm_spec_test --run_test=${SN} --report_level=detailed --color_output -- --${RUNTIME}) set_property(TEST ${SN}_unit_test_${RUNTIME} PROPERTY LABELS wasm_spec_tests) # build list of tests to run during coverage testing if(ctest_tests) From c56dfa5f495cccaddee9a7dab040ea06cbdeb3ab Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Thu, 26 Oct 2023 10:02:20 -0400 Subject: [PATCH 29/65] upgrade to actions/checkout@v4 --- .github/workflows/build.yaml | 12 ++++++------ .github/workflows/build_base.yaml | 2 +- .github/workflows/ph_backward_compatibility.yaml | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 4dccdddd5b..45ab9e057b 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -89,7 +89,7 @@ jobs: runs-on: ubuntu-latest container: ${{fromJSON(needs.platform-cache.outputs.platforms)[matrix.platform].image}} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: recursive - name: Download builddir @@ -140,7 +140,7 @@ jobs: image: ${{fromJSON(needs.platform-cache.outputs.platforms)[matrix.cfg.base].image}} options: --security-opt seccomp=unconfined steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Download builddir uses: actions/download-artifact@v3 with: @@ -168,7 +168,7 @@ jobs: - cfg: {name: 'ubuntu22repro', base: 'ubuntu22', builddir: 'reproducible'} runs-on: ["self-hosted", "enf-x86-midtier"] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Download builddir uses: actions/download-artifact@v3 with: @@ -201,7 +201,7 @@ jobs: - cfg: {name: 'ubuntu22repro', base: 'ubuntu22', builddir: 'reproducible'} runs-on: ["self-hosted", "enf-x86-lowtier"] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Download builddir uses: actions/download-artifact@v3 with: @@ -243,7 +243,7 @@ jobs: # LEAP - if: ${{ matrix.test != 'deb-install' }} name: Clone leap - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: submodules: recursive - if: ${{ matrix.test != 'deb-install' }} @@ -296,7 +296,7 @@ jobs: # Reference Contracts - name: checkout eos-system-contracts - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: repository: eosnetworkfoundation/eos-system-contracts path: eos-system-contracts diff --git a/.github/workflows/build_base.yaml b/.github/workflows/build_base.yaml index 5e6639f968..ec0b70f95b 100644 --- a/.github/workflows/build_base.yaml +++ b/.github/workflows/build_base.yaml @@ -30,7 +30,7 @@ jobs: runs-on: ["self-hosted", "enf-x86-beefy"] container: ${{fromJSON(inputs.platforms)[matrix.platform].image}} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: recursive - name: Build diff --git a/.github/workflows/ph_backward_compatibility.yaml b/.github/workflows/ph_backward_compatibility.yaml index 4633108821..26564792f2 100644 --- a/.github/workflows/ph_backward_compatibility.yaml +++ b/.github/workflows/ph_backward_compatibility.yaml @@ -46,7 +46,7 @@ jobs: image: ${{fromJSON(needs.platform-cache.outputs.platforms)[matrix.platform].image}} options: --security-opt seccomp=unconfined steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Download builddir uses: actions/download-artifact@v3 with: From ec227851eafb9edd906253b62eb449ff95a62a4b Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Thu, 26 Oct 2023 10:03:36 -0400 Subject: [PATCH 30/65] upgrade parallel-ctest-containers action to node20 --- .github/actions/parallel-ctest-containers/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/parallel-ctest-containers/action.yml b/.github/actions/parallel-ctest-containers/action.yml index c5e58db6eb..c4a8573e11 100644 --- a/.github/actions/parallel-ctest-containers/action.yml +++ b/.github/actions/parallel-ctest-containers/action.yml @@ -12,5 +12,5 @@ inputs: test-timeout: required: true runs: - using: 'node16' + using: 'node20' main: 'dist/index.mjs' From 72e0e0d07517c17dd76d312051cbe7cce05a3b9f Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Thu, 26 Oct 2023 10:12:06 -0400 Subject: [PATCH 31/65] upload core files from failed tests --- .github/workflows/build.yaml | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 45ab9e057b..8c42f6dc30 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -138,7 +138,7 @@ jobs: runs-on: ["self-hosted", "enf-x86-hightier"] container: image: ${{fromJSON(needs.platform-cache.outputs.platforms)[matrix.cfg.base].image}} - options: --security-opt seccomp=unconfined + options: --security-opt seccomp=unconfined --mount type=bind,source=/var/lib/systemd/coredump,target=/cores steps: - uses: actions/checkout@v4 - name: Download builddir @@ -152,6 +152,13 @@ jobs: zstdcat build.tar.zst | tar x cd build ctest --output-on-failure -j $(nproc) -LE "(nonparallelizable_tests|long_running_tests)" --timeout 420 + - name: Upload core files from failed tests + uses: actions/upload-artifact@v3 + if: failure() + with: + name: ${{matrix.cfg.name}}-tests-logs + if-no-files-found: ignore + path: /cores - name: Check CPU Features run: awk 'BEGIN {err = 1} /bmi2/ && /adx/ {err = 0} END {exit err}' /proc/cpuinfo @@ -181,12 +188,17 @@ jobs: log-tarball-prefix: ${{matrix.cfg.name}} tests-label: nonparallelizable_tests test-timeout: 420 + - name: Export core dumps + run: docker run --mount type=bind,source=/var/lib/systemd/coredump,target=/cores alpine sh -c 'tar -C /cores/ -c .' | tar x + if: failure() - name: Upload logs from failed tests uses: actions/upload-artifact@v3 if: failure() with: name: ${{matrix.cfg.name}}-np-logs - path: '*-logs.tar.gz' + path: | + *-logs.tar.gz + core*.zst lr-tests: name: LR Tests (${{matrix.cfg.name}}) @@ -214,12 +226,17 @@ jobs: log-tarball-prefix: ${{matrix.cfg.name}} tests-label: long_running_tests test-timeout: 1800 + - name: Export core dumps + run: docker run --mount type=bind,source=/var/lib/systemd/coredump,target=/cores alpine sh -c 'tar -C /cores/ -c .' | tar x + if: failure() - name: Upload logs from failed tests uses: actions/upload-artifact@v3 if: failure() with: name: ${{matrix.cfg.name}}-lr-logs - path: '*-logs.tar.gz' + path: | + *-logs.tar.gz + core*.zst libtester-tests: name: libtester tests From e4df534c675e8133b257a783cde4326c76618c93 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Wed, 1 Nov 2023 19:51:47 -0400 Subject: [PATCH 32/65] incorporate review comments --- .../chain/webassembly/eos-vm-oc/config.hpp | 6 ++++-- .../webassembly/runtimes/eos-vm-oc/LLVMJIT.cpp | 8 ++------ unittests/eosvmoc_limits_tests.cpp | 17 ++++++++--------- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp b/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp index 1de0293bd0..fcda6a368d 100644 --- a/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp +++ b/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp @@ -15,8 +15,10 @@ struct config { uint64_t cache_size = 1024u*1024u*1024u; uint64_t threads = 1u; - // subjective limits. - // nodeos uses the default values. libtester sets them as required. + // subjective limits for OC compilation. + // nodeos enforces the limits by the default values. + // libtester disables the limits in all tests, except enforces the limits + // in the tests in unittests/eosvmoc_limits_tests.cpp. rlimit cpu_limits {20u, 20u}; rlimit vm_limits {512u*1024u*1024u, 512u*1024u*1024u}; uint64_t stack_size_limit {16u*1024u}; diff --git a/libraries/chain/webassembly/runtimes/eos-vm-oc/LLVMJIT.cpp b/libraries/chain/webassembly/runtimes/eos-vm-oc/LLVMJIT.cpp index b0b9645b6e..acac1f3582 100644 --- a/libraries/chain/webassembly/runtimes/eos-vm-oc/LLVMJIT.cpp +++ b/libraries/chain/webassembly/runtimes/eos-vm-oc/LLVMJIT.cpp @@ -315,17 +315,13 @@ namespace LLVMJIT WAVM_ASSERT_THROW(!!c); ++num_functions_stack_size_found; - // enforce stack_size_limit only when it is not max (libtester may - // disable it by setting it to max). - if(stack_size_limit != std::numeric_limits::max() && stack_size > stack_size_limit) + if(stack_size > stack_size_limit) _exit(1); } } if(num_functions_stack_size_found != module.functions.defs.size()) _exit(1); - // enforce generated_code_size_limit only when it is not max (libtester may - // disable it by setting it to max). - if(generated_code_size_limit != std::numeric_limits::max() && jitModule->final_pic_code.size() >= generated_code_size_limit) + if(jitModule->final_pic_code.size() >= generated_code_size_limit) _exit(1); instantiated_code ret; diff --git a/unittests/eosvmoc_limits_tests.cpp b/unittests/eosvmoc_limits_tests.cpp index a0d096cead..64c4007dc3 100644 --- a/unittests/eosvmoc_limits_tests.cpp +++ b/unittests/eosvmoc_limits_tests.cpp @@ -13,7 +13,7 @@ BOOST_AUTO_TEST_SUITE(eosvmoc_limits_tests) // common routine to verify wasm_execution_error is raised when a resource // limit specified in eosvmoc_config is reached -void limit_violated_test(const eosvmoc::config eosvmoc_config) { +void limit_violated_test(const eosvmoc::config& eosvmoc_config) { fc::temp_directory tempdir; constexpr bool use_genesis = true; @@ -49,7 +49,7 @@ void limit_violated_test(const eosvmoc::config eosvmoc_config) { // common routine to verify no wasm_execution_error is raised // because limits specified in eosvmoc_config are not reached -void limit_not_violated_test(const eosvmoc::config eosvmoc_config) { +void limit_not_violated_test(const eosvmoc::config& eosvmoc_config) { fc::temp_directory tempdir; constexpr bool use_genesis = true; @@ -104,12 +104,12 @@ BOOST_AUTO_TEST_CASE( stack_limit ) { try { eosvmoc_config.vm_limits.rlim_cur = RLIM_INFINITY; eosvmoc_config.generated_code_size_limit = std::numeric_limits::max(); - // The larget stack size in the test is 104. - // Set stack_size_limit to highest value to verify wasm_execution_error is raised + // The stack size of the compiled WASM in the test is 104. + // Set stack_size_limit one less than the actual needed stack size eosvmoc_config.stack_size_limit = 103; limit_violated_test(eosvmoc_config); - // set stack_size_limit to lowest value to verify wasm_execution_error is not raised + // set stack_size_limit to the actual needed stack size eosvmoc_config.stack_size_limit = 104; limit_not_violated_test(eosvmoc_config); } FC_LOG_AND_RETHROW() } @@ -123,13 +123,12 @@ BOOST_AUTO_TEST_CASE( generated_code_size_limit ) { try { eosvmoc_config.vm_limits.rlim_cur = RLIM_INFINITY; eosvmoc_config.stack_size_limit = std::numeric_limits::max(); - // The sgenerated code size in the test is 36856. - // Set generated_code_size_limit to highest value to verify wasm_execution_error - // is raised + // The generated code size of the compiled WASM in the test is 36856. + // Set generated_code_size_limit to the actual generated code size eosvmoc_config.generated_code_size_limit = 36856; limit_violated_test(eosvmoc_config); - // set generated_code_size_limit to lowest value to verify wasm_execution_error is not raised + // Set generated_code_size_limit to one above the actual generated code size eosvmoc_config.generated_code_size_limit = 36857; limit_not_violated_test(eosvmoc_config); } FC_LOG_AND_RETHROW() } From db817f794e1a6dd9cfc3b4a1050c12cbd7517b18 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 2 Nov 2023 10:29:02 -0500 Subject: [PATCH 33/65] GH-1690 Remove unneeded include --- tests/trx_generator/trx_provider.hpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/trx_generator/trx_provider.hpp b/tests/trx_generator/trx_provider.hpp index b53536ca74..f5685fe06b 100644 --- a/tests/trx_generator/trx_provider.hpp +++ b/tests/trx_generator/trx_provider.hpp @@ -4,8 +4,6 @@ #include #include -#include - #include #include From b0f567d4be75caed0e42f30a778f46285ac8d737 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 2 Nov 2023 10:29:24 -0500 Subject: [PATCH 34/65] GH-1690 Add better description --- tests/PerformanceHarness/performance_test_basic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PerformanceHarness/performance_test_basic.py b/tests/PerformanceHarness/performance_test_basic.py index a6fc881057..a96f24ea83 100755 --- a/tests/PerformanceHarness/performance_test_basic.py +++ b/tests/PerformanceHarness/performance_test_basic.py @@ -733,7 +733,7 @@ def _createBaseArgumentParser(defEndpointApiDef: str, defProdNodeCnt: int, defVa ptbBaseParserGroup.add_argument("--save-state", help=argparse.SUPPRESS if suppressHelp else "Whether to save node state. (Warning: large disk usage)", action='store_true') ptbBaseParserGroup.add_argument("--quiet", help=argparse.SUPPRESS if suppressHelp else "Whether to quiet printing intermediate results and reports to stdout", action='store_true') ptbBaseParserGroup.add_argument("--prods-enable-trace-api", help=argparse.SUPPRESS if suppressHelp else "Determines whether producer nodes should have eosio::trace_api_plugin enabled", action='store_true') - ptbBaseParserGroup.add_argument("--print-missing-transactions", type=bool, help=argparse.SUPPRESS if suppressHelp else "Toggles if missing transactions are be printed upon test completion.", default=True) + ptbBaseParserGroup.add_argument("--print-missing-transactions", type=bool, help=argparse.SUPPRESS if suppressHelp else "Print missing transactions upon test completion.", default=True) ptbBaseParserGroup.add_argument("--account-name", type=str, help=argparse.SUPPRESS if suppressHelp else "Name of the account to create and assign a contract to", default="eosio") ptbBaseParserGroup.add_argument("--contract-dir", type=str, help=argparse.SUPPRESS if suppressHelp else "Path to contract dir", default="unittests/contracts/eosio.system") ptbBaseParserGroup.add_argument("--wasm-file", type=str, help=argparse.SUPPRESS if suppressHelp else "WASM file name for contract", default="eosio.system.wasm") From 0e090f0103c622f43f023a854b96acee9717d70e Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Wed, 1 Nov 2023 13:42:16 -0400 Subject: [PATCH 35/65] globally --catch_system_errors=no --- CMakeLists.txt | 4 ++++ tests/CMakeLists.txt | 2 +- unittests/CMakeLists.txt | 4 ++-- unittests/wasm-spec-tests/generated-tests/CMakeLists.txt | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ff2708fadb..08441b0116 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -175,6 +175,10 @@ endif() # do not attempt to use an external OpenSSL in any manner set(CMAKE_DISABLE_FIND_PACKAGE_OpenSSL On) +# many tests require handling of signals themselves and even when they don't we'd like for them to generate a core dump, this +# is effectively --catch_system_errors=no broadly across all tests +add_compile_definitions(BOOST_TEST_DEFAULTS_TO_CORE_DUMP) + add_subdirectory( libraries ) add_subdirectory( plugins ) add_subdirectory( programs ) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index fb26b0c350..182c68614f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -81,7 +81,7 @@ else() endif() #To run plugin_test with all log from blockchain displayed, put --verbose after --, i.e. plugin_test -- --verbose -add_test(NAME plugin_test COMMAND plugin_test --report_level=detailed --color_output --catch_system_errors=no) +add_test(NAME plugin_test COMMAND plugin_test --report_level=detailed --color_output) add_test(NAME nodeos_sanity_test COMMAND tests/nodeos_run_test.py -v --sanity-test ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST nodeos_sanity_test PROPERTY LABELS nonparallelizable_tests) diff --git a/unittests/CMakeLists.txt b/unittests/CMakeLists.txt index f85ff7cdeb..c1e3eed7a2 100644 --- a/unittests/CMakeLists.txt +++ b/unittests/CMakeLists.txt @@ -85,7 +85,7 @@ target_include_directories( unit_test PUBLIC ${CMAKE_SOURCE_DIR}/plugins/chain_interface/include) ### MARK TEST SUITES FOR EXECUTION ### -add_test(NAME protocol_feature_digest_unit_test COMMAND unit_test --run_test=protocol_feature_digest_tests --report_level=detailed --color_output --catch_system_errors=no) +add_test(NAME protocol_feature_digest_unit_test COMMAND unit_test --run_test=protocol_feature_digest_tests --report_level=detailed --color_output) set(ctest_tests "protocol_feature_digest_tests") foreach(TEST_SUITE ${UNIT_TESTS}) # create an independent target for each test suite execute_process(COMMAND sh -c "grep -E 'BOOST_AUTO_TEST_SUITE\\s*[(]' '${TEST_SUITE}' | grep -vE '//.*BOOST_AUTO_TEST_SUITE\\s*[(]' | cut -d ')' -f 1 | cut -d '(' -f 2" OUTPUT_VARIABLE SUITE_NAME OUTPUT_STRIP_TRAILING_WHITESPACE) # get the test suite name from the *.cpp file @@ -93,7 +93,7 @@ foreach(TEST_SUITE ${UNIT_TESTS}) # create an independent target for each test s execute_process(COMMAND sh -c "echo ${SUITE_NAME} | sed -e 's/s$//' | sed -e 's/_test$//'" OUTPUT_VARIABLE TRIMMED_SUITE_NAME OUTPUT_STRIP_TRAILING_WHITESPACE) # trim "_test" or "_tests" from the end of ${SUITE_NAME} # to run unit_test with all log from blockchain displayed, put "--verbose" after "--", i.e. "unit_test -- --verbose" foreach(RUNTIME ${EOSIO_WASM_RUNTIMES}) - add_test(NAME ${TRIMMED_SUITE_NAME}_unit_test_${RUNTIME} COMMAND unit_test --run_test=${SUITE_NAME} --report_level=detailed --color_output --catch_system_errors=no -- --${RUNTIME}) + add_test(NAME ${TRIMMED_SUITE_NAME}_unit_test_${RUNTIME} COMMAND unit_test --run_test=${SUITE_NAME} --report_level=detailed --color_output -- --${RUNTIME}) # build list of tests to run during coverage testing if(ctest_tests) string(APPEND ctest_tests "|") diff --git a/unittests/wasm-spec-tests/generated-tests/CMakeLists.txt b/unittests/wasm-spec-tests/generated-tests/CMakeLists.txt index 87225c902c..25169702fb 100644 --- a/unittests/wasm-spec-tests/generated-tests/CMakeLists.txt +++ b/unittests/wasm-spec-tests/generated-tests/CMakeLists.txt @@ -24,7 +24,7 @@ foreach(TEST_SUITE ${WASM_TESTS}) # create an independent target for each test s foreach(RUNTIME ${EOSIO_WASM_RUNTIMES}) get_test_property(${SN}_unit_test_${RUNTIME} LABELS TEST_LABEL) if ("NOTFOUND" STREQUAL "${TEST_LABEL}") # prevent duplicates - add_test(NAME ${SN}_unit_test_${RUNTIME} COMMAND wasm_spec_test --run_test=${SN} --report_level=detailed --color_output --catch_system_errors=no -- --${RUNTIME}) + add_test(NAME ${SN}_unit_test_${RUNTIME} COMMAND wasm_spec_test --run_test=${SN} --report_level=detailed --color_output -- --${RUNTIME}) set_property(TEST ${SN}_unit_test_${RUNTIME} PROPERTY LABELS wasm_spec_tests) # build list of tests to run during coverage testing if(ctest_tests) From 6286f84bd5bc2cf56e0a507a72e1fc0c315bdcbb Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Thu, 26 Oct 2023 10:02:20 -0400 Subject: [PATCH 36/65] upgrade to actions/checkout@v4 --- .github/workflows/build.yaml | 12 ++++++------ .github/workflows/build_base.yaml | 2 +- .github/workflows/ph_backward_compatibility.yaml | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 4dccdddd5b..45ab9e057b 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -89,7 +89,7 @@ jobs: runs-on: ubuntu-latest container: ${{fromJSON(needs.platform-cache.outputs.platforms)[matrix.platform].image}} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: recursive - name: Download builddir @@ -140,7 +140,7 @@ jobs: image: ${{fromJSON(needs.platform-cache.outputs.platforms)[matrix.cfg.base].image}} options: --security-opt seccomp=unconfined steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Download builddir uses: actions/download-artifact@v3 with: @@ -168,7 +168,7 @@ jobs: - cfg: {name: 'ubuntu22repro', base: 'ubuntu22', builddir: 'reproducible'} runs-on: ["self-hosted", "enf-x86-midtier"] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Download builddir uses: actions/download-artifact@v3 with: @@ -201,7 +201,7 @@ jobs: - cfg: {name: 'ubuntu22repro', base: 'ubuntu22', builddir: 'reproducible'} runs-on: ["self-hosted", "enf-x86-lowtier"] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Download builddir uses: actions/download-artifact@v3 with: @@ -243,7 +243,7 @@ jobs: # LEAP - if: ${{ matrix.test != 'deb-install' }} name: Clone leap - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: submodules: recursive - if: ${{ matrix.test != 'deb-install' }} @@ -296,7 +296,7 @@ jobs: # Reference Contracts - name: checkout eos-system-contracts - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: repository: eosnetworkfoundation/eos-system-contracts path: eos-system-contracts diff --git a/.github/workflows/build_base.yaml b/.github/workflows/build_base.yaml index 5e6639f968..ec0b70f95b 100644 --- a/.github/workflows/build_base.yaml +++ b/.github/workflows/build_base.yaml @@ -30,7 +30,7 @@ jobs: runs-on: ["self-hosted", "enf-x86-beefy"] container: ${{fromJSON(inputs.platforms)[matrix.platform].image}} steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 with: submodules: recursive - name: Build diff --git a/.github/workflows/ph_backward_compatibility.yaml b/.github/workflows/ph_backward_compatibility.yaml index 4633108821..26564792f2 100644 --- a/.github/workflows/ph_backward_compatibility.yaml +++ b/.github/workflows/ph_backward_compatibility.yaml @@ -46,7 +46,7 @@ jobs: image: ${{fromJSON(needs.platform-cache.outputs.platforms)[matrix.platform].image}} options: --security-opt seccomp=unconfined steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Download builddir uses: actions/download-artifact@v3 with: From f2ba2481a7345de7cf564ac56245fe69a4b9ecdb Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Thu, 26 Oct 2023 10:03:36 -0400 Subject: [PATCH 37/65] upgrade parallel-ctest-containers action to node20 --- .github/actions/parallel-ctest-containers/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/parallel-ctest-containers/action.yml b/.github/actions/parallel-ctest-containers/action.yml index c5e58db6eb..c4a8573e11 100644 --- a/.github/actions/parallel-ctest-containers/action.yml +++ b/.github/actions/parallel-ctest-containers/action.yml @@ -12,5 +12,5 @@ inputs: test-timeout: required: true runs: - using: 'node16' + using: 'node20' main: 'dist/index.mjs' From 492c7bd22b0ee1414a027c3ace290ac8d57b7b7c Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Thu, 26 Oct 2023 10:12:06 -0400 Subject: [PATCH 38/65] upload core files from failed tests --- .github/workflows/build.yaml | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 45ab9e057b..8c42f6dc30 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -138,7 +138,7 @@ jobs: runs-on: ["self-hosted", "enf-x86-hightier"] container: image: ${{fromJSON(needs.platform-cache.outputs.platforms)[matrix.cfg.base].image}} - options: --security-opt seccomp=unconfined + options: --security-opt seccomp=unconfined --mount type=bind,source=/var/lib/systemd/coredump,target=/cores steps: - uses: actions/checkout@v4 - name: Download builddir @@ -152,6 +152,13 @@ jobs: zstdcat build.tar.zst | tar x cd build ctest --output-on-failure -j $(nproc) -LE "(nonparallelizable_tests|long_running_tests)" --timeout 420 + - name: Upload core files from failed tests + uses: actions/upload-artifact@v3 + if: failure() + with: + name: ${{matrix.cfg.name}}-tests-logs + if-no-files-found: ignore + path: /cores - name: Check CPU Features run: awk 'BEGIN {err = 1} /bmi2/ && /adx/ {err = 0} END {exit err}' /proc/cpuinfo @@ -181,12 +188,17 @@ jobs: log-tarball-prefix: ${{matrix.cfg.name}} tests-label: nonparallelizable_tests test-timeout: 420 + - name: Export core dumps + run: docker run --mount type=bind,source=/var/lib/systemd/coredump,target=/cores alpine sh -c 'tar -C /cores/ -c .' | tar x + if: failure() - name: Upload logs from failed tests uses: actions/upload-artifact@v3 if: failure() with: name: ${{matrix.cfg.name}}-np-logs - path: '*-logs.tar.gz' + path: | + *-logs.tar.gz + core*.zst lr-tests: name: LR Tests (${{matrix.cfg.name}}) @@ -214,12 +226,17 @@ jobs: log-tarball-prefix: ${{matrix.cfg.name}} tests-label: long_running_tests test-timeout: 1800 + - name: Export core dumps + run: docker run --mount type=bind,source=/var/lib/systemd/coredump,target=/cores alpine sh -c 'tar -C /cores/ -c .' | tar x + if: failure() - name: Upload logs from failed tests uses: actions/upload-artifact@v3 if: failure() with: name: ${{matrix.cfg.name}}-lr-logs - path: '*-logs.tar.gz' + path: | + *-logs.tar.gz + core*.zst libtester-tests: name: libtester tests From d1ab313a1484d47844e1e908e4bb51ad7e2a6719 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 2 Nov 2023 14:29:01 -0500 Subject: [PATCH 39/65] GH-1690 Remove thread hop to producer thread pool and go straight to app thread --- .../eosio/chain/transaction_metadata.hpp | 6 + libraries/chain/transaction_metadata.cpp | 26 +++-- plugins/producer_plugin/producer_plugin.cpp | 105 ++++++++++-------- 3 files changed, 80 insertions(+), 57 deletions(-) diff --git a/libraries/chain/include/eosio/chain/transaction_metadata.hpp b/libraries/chain/include/eosio/chain/transaction_metadata.hpp index 5e585df6e7..7105f95b28 100644 --- a/libraries/chain/include/eosio/chain/transaction_metadata.hpp +++ b/libraries/chain/include/eosio/chain/transaction_metadata.hpp @@ -83,6 +83,12 @@ class transaction_metadata { start_recover_keys( packed_transaction_ptr trx, boost::asio::io_context& thread_pool, const chain_id_type& chain_id, fc::microseconds time_limit, trx_type t, uint32_t max_variable_sig_size = UINT32_MAX ); + /// Thread safe. + /// @returns transaction_metadata_ptr or throws + static transaction_metadata_ptr + recover_keys( packed_transaction_ptr trx, + const chain_id_type& chain_id, fc::microseconds time_limit, + trx_type t, uint32_t max_variable_sig_size = UINT32_MAX ); /// @returns constructed transaction_metadata with no key recovery (sig_cpu_usage=0, recovered_pub_keys=empty) static transaction_metadata_ptr diff --git a/libraries/chain/transaction_metadata.cpp b/libraries/chain/transaction_metadata.cpp index f33e33192b..d49819dd16 100644 --- a/libraries/chain/transaction_metadata.cpp +++ b/libraries/chain/transaction_metadata.cpp @@ -12,15 +12,23 @@ recover_keys_future transaction_metadata::start_recover_keys( packed_transaction uint32_t max_variable_sig_size ) { return post_async_task( thread_pool, [trx{std::move(trx)}, chain_id, time_limit, t, max_variable_sig_size]() mutable { - fc::time_point deadline = time_limit == fc::microseconds::maximum() ? - fc::time_point::maximum() : fc::time_point::now() + time_limit; - check_variable_sig_size( trx, max_variable_sig_size ); - const signed_transaction& trn = trx->get_signed_transaction(); - flat_set recovered_pub_keys; - fc::microseconds cpu_usage = trn.get_signature_keys( chain_id, deadline, recovered_pub_keys ); - return std::make_shared( private_type(), std::move( trx ), cpu_usage, std::move( recovered_pub_keys ), t ); - } - ); + return recover_keys( std::move(trx), chain_id, time_limit, t, max_variable_sig_size ); + }); +} + +transaction_metadata_ptr transaction_metadata::recover_keys( packed_transaction_ptr trx, + const chain_id_type& chain_id, + fc::microseconds time_limit, + trx_type t, + uint32_t max_variable_sig_size ) +{ + fc::time_point deadline = time_limit == fc::microseconds::maximum() ? + fc::time_point::maximum() : fc::time_point::now() + time_limit; + check_variable_sig_size( trx, max_variable_sig_size ); + const signed_transaction& trn = trx->get_signed_transaction(); + flat_set recovered_pub_keys; + fc::microseconds cpu_usage = trn.get_signature_keys( chain_id, deadline, recovered_pub_keys ); + return std::make_shared( private_type(), std::move( trx ), cpu_usage, std::move( recovered_pub_keys ), t ); } size_t transaction_metadata::get_estimated_size() const { diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index df4ce39ddd..3635b34c47 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -397,13 +397,12 @@ class producer_plugin_impl : public std::enable_shared_from_this& response) { @@ -825,38 +817,57 @@ class producer_plugin_impl : public std::enable_shared_from_this_time_tracker.add_idle_time(start); - auto trx_tracker = self->_time_tracker.start_trx(is_transient, start); - fc_tlog(_log, "Time since last trx: ${t}us", ("t", idle_time)); - - auto exception_handler = - [self, is_transient, &next, trx{std::move(trx)}, &start](fc::exception_ptr ex) { - self->log_trx_results(trx, nullptr, ex, 0, start, is_transient); - next(std::move(ex)); - }; - try { - auto result = future.get(); - if (!self->process_incoming_transaction_async(result, api_trx, return_failure_traces, trx_tracker, next)) { - if (self->in_producing_mode()) { - self->schedule_maybe_produce_block(true); - } else { - self->restart_speculative_block(); - } - } - } - CATCH_AND_CALL(exception_handler); - }); - } - }); + boost::asio::post( chain.get_thread_pool(), + [this, trx{trx}, &chain, time_limit{max_trx_cpu_usage}, trx_type, + api_trx, is_transient, next{std::move(next)}, return_failure_traces]() mutable { + + transaction_metadata_ptr trx_meta; + try { + trx_meta = transaction_metadata::recover_keys( trx, chain.get_chain_id(), time_limit, trx_type, chain.configured_subjective_signature_length_limit() ); + } catch(...) { + // use read_write when read is likely fine; this maintains previous behavior of next() always being called from the main thread + app().executor().post(priority::low, exec_queue::read_write, + [ex_ptr = std::current_exception(), this, trx{std::move(trx)}, is_transient, next{std::move(next)}]() { + auto start = fc::time_point::now(); + auto idle_time = this->_time_tracker.add_idle_time(start); + auto trx_tracker = this->_time_tracker.start_trx(is_transient, start); + fc_tlog(_log, "Time since last trx: ${t}us", ("t", idle_time)); + auto ex_handler = + [this, is_transient, &next, &trx](fc::exception_ptr ex) { + this->log_trx_results(trx, nullptr, ex, 0, is_transient); + next(std::move(ex)); + }; + try { + std::rethrow_exception(ex_ptr); + } CATCH_AND_CALL(ex_handler) + }); + return; + } + + app().executor().post(priority::low, exec_queue::read_write, + [this, trx_meta{std::move(trx_meta)}, api_trx, is_transient, next{std::move(next)}, return_failure_traces]() mutable { + auto start = fc::time_point::now(); + auto idle_time = this->_time_tracker.add_idle_time(start); + auto trx_tracker = this->_time_tracker.start_trx(is_transient, start); + fc_tlog(_log, "Time since last trx: ${t}us", ("t", idle_time)); + + auto exception_handler = + [this, is_transient, &next, &trx{trx_meta->packed_trx()}](fc::exception_ptr ex) { + this->log_trx_results(trx, nullptr, ex, 0, is_transient); + next(std::move(ex)); + }; + try { + if (!this->process_incoming_transaction_async(trx_meta, api_trx, return_failure_traces, trx_tracker, next)) { + if (this->in_producing_mode()) { + this->schedule_maybe_produce_block(true); + } else { + this->restart_speculative_block(); + } + } + } + CATCH_AND_CALL(exception_handler); + }); + } ); } bool process_incoming_transaction_async(const transaction_metadata_ptr& trx, @@ -2043,22 +2054,20 @@ inline std::string get_detailed_contract_except_info(const packed_transaction_pt } void producer_plugin_impl::log_trx_results(const transaction_metadata_ptr& trx, - const transaction_trace_ptr& trace, - const fc::time_point& start) { + const transaction_trace_ptr& trace) { uint32_t billed_cpu_time_us = (trace && trace->receipt) ? trace->receipt->cpu_usage_us : 0; - log_trx_results(trx->packed_trx(), trace, nullptr, billed_cpu_time_us, start, trx->is_transient()); + log_trx_results(trx->packed_trx(), trace, nullptr, billed_cpu_time_us, trx->is_transient()); } void producer_plugin_impl::log_trx_results(const transaction_metadata_ptr& trx, const fc::exception_ptr& except_ptr) { uint32_t billed_cpu_time_us = trx ? trx->billed_cpu_time_us : 0; - log_trx_results(trx->packed_trx(), nullptr, except_ptr, billed_cpu_time_us, fc::time_point::now(), trx->is_transient()); + log_trx_results(trx->packed_trx(), nullptr, except_ptr, billed_cpu_time_us, trx->is_transient()); } void producer_plugin_impl::log_trx_results(const packed_transaction_ptr& trx, const transaction_trace_ptr& trace, const fc::exception_ptr& except_ptr, uint32_t billed_cpu_us, - const fc::time_point& start, bool is_transient) { chain::controller& chain = chain_plug->chain(); @@ -2226,7 +2235,7 @@ producer_plugin_impl::handle_push_result(const transaction_metadata_ptr& if (!disable_subjective_enforcement) // subjectively bill failure when producing since not in objective cpu account billing subjective_bill.subjective_bill_failure(first_auth, trace->elapsed, fc::time_point::now()); - log_trx_results(trx, trace, start); + log_trx_results(trx, trace); // this failed our configured maximum transaction time, we don't want to replay it fc_tlog(_log, "Failed ${c} trx, auth: ${a}, prev billed: ${p}us, ran: ${r}us, id: ${id}, except: ${e}", ("c", e.code())("a", first_auth)("p", prev_billed_cpu_time_us)("r", end - start)("id", trx->id())("e", e)); @@ -2245,7 +2254,7 @@ producer_plugin_impl::handle_push_result(const transaction_metadata_ptr& } else { fc_tlog(_log, "Subjective bill for success ${a}: ${b} elapsed ${t}us, time ${r}us", ("a", first_auth)("b", sub_bill)("t", trace->elapsed)("r", end - start)); - log_trx_results(trx, trace, start); + log_trx_results(trx, trace); // if producing then trx is in objective cpu account billing if (!disable_subjective_enforcement && !in_producing_mode()) { subjective_bill.subjective_bill(trx->id(), trx->packed_trx()->expiration(), first_auth, trace->elapsed); From 3096b0056e5fb3fc7736a8574a96c8a50a8d217f Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 2 Nov 2023 14:38:57 -0500 Subject: [PATCH 40/65] GH-1690 Remove --producer-threads option --- .../03_plugins/producer_plugin/index.md | 2 -- plugins/producer_plugin/producer_plugin.cpp | 14 -------- tests/PerformanceHarness/README.md | 36 ++----------------- tests/PerformanceHarness/performance_test.py | 21 +---------- .../performance_test_basic.py | 3 +- tests/PerformanceHarnessScenarioRunner.py | 1 - tests/p2p_high_latency_test.py | 2 +- 7 files changed, 6 insertions(+), 73 deletions(-) diff --git a/docs/01_nodeos/03_plugins/producer_plugin/index.md b/docs/01_nodeos/03_plugins/producer_plugin/index.md index 3d36b24f04..a53a3bc466 100644 --- a/docs/01_nodeos/03_plugins/producer_plugin/index.md +++ b/docs/01_nodeos/03_plugins/producer_plugin/index.md @@ -111,8 +111,6 @@ Config Options for eosio::producer_plugin: --disable-subjective-api-billing arg (=1) Disable subjective CPU billing for API transactions - --producer-threads arg (=2) Number of worker threads in producer - thread pool --snapshots-dir arg (="snapshots") the location of the snapshots directory (absolute path or relative to application data dir) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 3635b34c47..7376bc00f6 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -494,8 +494,6 @@ class producer_plugin_impl : public std::enable_shared_from_this _thread_pool; std::atomic _max_transaction_time_ms; // modified by app thread, read by net_plugin thread pool std::atomic _received_block{0}; // modified by net_plugin thread pool fc::microseconds _max_irreversible_block_age_us; @@ -1077,8 +1075,6 @@ void producer_plugin::set_program_options( "Disable subjective CPU billing for P2P transactions") ("disable-subjective-api-billing", bpo::value()->default_value(true), "Disable subjective CPU billing for API transactions") - ("producer-threads", bpo::value()->default_value(my->_thread_pool_size), - "Number of worker threads in producer thread pool") ("snapshots-dir", bpo::value()->default_value("snapshots"), "the location of the snapshots directory (absolute path or relative to application data dir)") ("read-only-threads", bpo::value(), @@ -1196,9 +1192,6 @@ void producer_plugin_impl::plugin_initialize(const boost::program_options::varia ilog("Subjective CPU billing of API trxs disabled "); } - _thread_pool_size = options.at("producer-threads").as(); - EOS_ASSERT(_thread_pool_size > 0, plugin_config_exception, "producer-threads ${num} must be greater than 0", ("num", _thread_pool_size)); - if (options.count("snapshots-dir")) { auto sd = options.at("snapshots-dir").as(); if (sd.is_relative()) { @@ -1324,12 +1317,6 @@ void producer_plugin_impl::plugin_startup() { try { ilog("producer plugin: plugin_startup() begin"); - _thread_pool.start(_thread_pool_size, [](const fc::exception& e) { - fc_elog(_log, "Exception in producer thread pool, exiting: ${e}", ("e", e.to_detail_string())); - app().quit(); - }); - - chain::controller& chain = chain_plug->chain(); EOS_ASSERT(_producers.empty() || chain.get_read_mode() != chain::db_read_mode::IRREVERSIBLE, plugin_config_exception, "node cannot have any producer-name configured because block production is impossible when read_mode is \"irreversible\""); @@ -1409,7 +1396,6 @@ void producer_plugin_impl::plugin_shutdown() { _ro_timer.cancel(ec); app().executor().stop(); _ro_thread_pool.stop(); - _thread_pool.stop(); _unapplied_transactions.clear(); app().executor().post(0, [me = shared_from_this()]() {}); // keep my pointer alive until queue is drained diff --git a/tests/PerformanceHarness/README.md b/tests/PerformanceHarness/README.md index 42ff944ef0..0b54191863 100644 --- a/tests/PerformanceHarness/README.md +++ b/tests/PerformanceHarness/README.md @@ -371,7 +371,6 @@ Operational Modes: ``` usage: PerformanceHarnessScenarioRunner.py findMax testBpOpMode [--skip-tps-test] - [--calc-producer-threads {none,lmax,full}] [--calc-chain-threads {none,lmax,full}] [--calc-net-threads {none,lmax,full}] [--del-test-report] @@ -398,22 +397,6 @@ Performance Harness: --skip-tps-test Determines whether to skip the max TPS measurement tests - --calc-producer-threads {none,lmax,full} - Determines whether to calculate number of worker - threads to use in producer thread pool ("none", - "lmax", or "full"). In "none" mode, the default, no - calculation will be attempted and the configured - --producer-threads value will be used. In "lmax" mode, - producer threads will incrementally be tested, - starting at plugin default, until the performance rate - ceases to increase with the addition of additional - threads. In "full" mode producer threads will - incrementally be tested from plugin default..num - logical processors, recording each performance and - choosing the local max performance (same value as - would be discovered in "lmax" mode). Useful for - graphing the full performance impact of each available - thread. --calc-chain-threads {none,lmax,full} Determines whether to calculate number of worker threads to use in chain thread pool ("none", "lmax", @@ -505,7 +488,6 @@ usage: PerformanceHarnessScenarioRunner.py findMax testBpOpMode overrideBasicTes [--net-threads NET_THREADS] [--disable-subjective-billing DISABLE_SUBJECTIVE_BILLING] [--produce-block-offset-ms PRODUCE_BLOCK_OFFSET_MS] - [--producer-threads PRODUCER_THREADS] [--read-only-write-window-time-us READ_ONLY_WRITE_WINDOW_TIME_US] [--read-only-read-window-time-us READ_ONLY_READ_WINDOW_TIME_US] [--http-max-in-flight-requests HTTP_MAX_IN_FLIGHT_REQUESTS] @@ -582,8 +564,6 @@ Performance Test Basic Base: --produce-block-offset-ms PRODUCE_BLOCK_OFFSET_MS The number of milliseconds early the last block of a production round should be produced. - --producer-threads PRODUCER_THREADS - Number of worker threads in producer thread pool --read-only-write-window-time-us READ_ONLY_WRITE_WINDOW_TIME_US Time in microseconds the write window lasts. --read-only-read-window-time-us READ_ONLY_READ_WINDOW_TIME_US @@ -665,7 +645,6 @@ The following classes and scripts are typically used by the Performance Harness [--net-threads NET_THREADS] [--disable-subjective-billing DISABLE_SUBJECTIVE_BILLING] [--produce-block-offset-ms PRODUCE_BLOCK_OFFSET_MS] - [--producer-threads PRODUCER_THREADS] [--http-max-in-flight-requests HTTP_MAX_IN_FLIGHT_REQUESTS] [--http-max-response-time-ms HTTP_MAX_RESPONSE_TIME_MS] [--http-max-bytes-in-flight-mb HTTP_MAX_BYTES_IN_FLIGHT_MB] @@ -746,8 +725,6 @@ Performance Test Basic Base: --produce-block-offset-ms PRODUCE_BLOCK_OFFSET_MS The number of milliseconds early the last block of a production round should be produced. - --producer-threads PRODUCER_THREADS - Number of worker threads in producer thread pool (default: 2) --http-max-in-flight-requests HTTP_MAX_IN_FLIGHT_REQUESTS Maximum number of requests http_plugin should use for processing http requests. 429 error response when exceeded. -1 for unlimited (default: -1) --http-max-response-time-ms HTTP_MAX_RESPONSE_TIME_MS @@ -879,7 +856,7 @@ The Performance Harness Scenario Runner, through the `PerformanceTest` and `Perf Command used to run test and generate report: ``` bash -./tests/PerformanceHarnessScenarioRunner.py findMax testBpOpMode --test-iteration-duration-sec 10 --final-iterations-duration-sec 30 --calc-producer-threads lmax --calc-chain-threads lmax --calc-net-threads lmax +./tests/PerformanceHarnessScenarioRunner.py findMax testBpOpMode --test-iteration-duration-sec 10 --final-iterations-duration-sec 30 --calc-chain-threads lmax --calc-net-threads lmax ``` ### Report Breakdown @@ -1255,7 +1232,7 @@ Finally, the full detail test report for each of the determined max TPS throughp "analysisFinish": "2023-08-18T17:39:08.002767" }, "args": { - "rawCmdLine ": "tests/PerformanceHarnessScenarioRunner.py findMax testBpOpMode --test-iteration-duration-sec 10 --final-iterations-duration-sec 30 --calc-producer-threads lmax --calc-chain-threads lmax --calc-net-threads lmax", + "rawCmdLine ": "tests/PerformanceHarnessScenarioRunner.py findMax testBpOpMode --test-iteration-duration-sec 10 --final-iterations-duration-sec 30 --calc-chain-threads lmax --calc-net-threads lmax", "dumpErrorDetails": false, "delay": 1, "nodesFile": null, @@ -1621,9 +1598,6 @@ Finally, the full detail test report for each of the determined max TPS throughp "disableSubjectiveApiBilling": true, "_disableSubjectiveApiBillingNodeosDefault": 1, "_disableSubjectiveApiBillingNodeosArg": "--disable-subjective-api-billing", - "producerThreads": 2, - "_producerThreadsNodeosDefault": 2, - "_producerThreadsNodeosArg": "--producer-threads", "snapshotsDir": null, "_snapshotsDirNodeosDefault": "\"snapshots\"", "_snapshotsDirNodeosArg": "--snapshots-dir", @@ -1771,7 +1745,6 @@ Finally, the full detail test report for each of the determined max TPS throughp "quiet": false, "logDirRoot": ".", "skipTpsTests": false, - "calcProducerThreads": "lmax", "calcChainThreads": "lmax", "calcNetThreads": "lmax", "userTrxDataFile": null, @@ -1915,7 +1888,7 @@ The Performance Test Basic generates, by default, a report that details results } }, "args": { - "rawCmdLine ": "tests/PerformanceHarnessScenarioRunner.py findMax testBpOpMode --test-iteration-duration-sec 10 --final-iterations-duration-sec 30 --calc-producer-threads lmax --calc-chain-threads lmax --calc-net-threads lmax", + "rawCmdLine ": "tests/PerformanceHarnessScenarioRunner.py findMax testBpOpMode --test-iteration-duration-sec 10 --final-iterations-duration-sec 30 --calc-chain-threads lmax --calc-net-threads lmax", "dumpErrorDetails": false, "delay": 1, "nodesFile": null, @@ -2281,9 +2254,6 @@ The Performance Test Basic generates, by default, a report that details results "disableSubjectiveApiBilling": true, "_disableSubjectiveApiBillingNodeosDefault": 1, "_disableSubjectiveApiBillingNodeosArg": "--disable-subjective-api-billing", - "producerThreads": 2, - "_producerThreadsNodeosDefault": 2, - "_producerThreadsNodeosArg": "--producer-threads", "snapshotsDir": null, "_snapshotsDirNodeosDefault": "\"snapshots\"", "_snapshotsDirNodeosArg": "--snapshots-dir", diff --git a/tests/PerformanceHarness/performance_test.py b/tests/PerformanceHarness/performance_test.py index 489521164e..63f05e2bab 100755 --- a/tests/PerformanceHarness/performance_test.py +++ b/tests/PerformanceHarness/performance_test.py @@ -43,7 +43,6 @@ class PtConfig: quiet: bool=False logDirRoot: Path=Path(".") skipTpsTests: bool=False - calcProducerThreads: str="none" calcChainThreads: str="none" calcNetThreads: str="none" userTrxDataFile: Path=None @@ -368,18 +367,6 @@ def runTest(self): self.testDirsCleanup() self.testDirsSetup() - prodResults = None - if self.ptConfig.calcProducerThreads != "none": - print(f"Performing Producer Thread Optimization Tests") - if self.ptConfig.calcProducerThreads == "full": - optType = PerformanceTest.PluginThreadOptRunType.FULL - else: - optType = PerformanceTest.PluginThreadOptRunType.LOCAL_MAX - prodResults = self.optimizePluginThreadCount(optPlugin=PerformanceTest.PluginThreadOpt.PRODUCER, optType=optType, - minThreadCount=self.clusterConfig.extraNodeosArgs.producerPluginArgs._producerThreadsNodeosDefault) - print(f"Producer Thread Optimization results: {prodResults}") - self.clusterConfig.extraNodeosArgs.producerPluginArgs.threads = prodResults.recommendedThreadCount - chainResults = None if self.ptConfig.calcChainThreads != "none": print(f"Performing Chain Thread Optimization Tests") @@ -413,7 +400,7 @@ def runTest(self): self.testsFinish = datetime.utcnow() - self.report = self.createReport(producerThreadResult=prodResults, chainThreadResult=chainResults, netThreadResult=netResults, tpsTestResult=tpsTestResult, nodeosVers=self.clusterConfig.nodeosVers) + self.report = self.createReport(chainThreadResult=chainResults, netThreadResult=netResults, tpsTestResult=tpsTestResult, nodeosVers=self.clusterConfig.nodeosVers) jsonReport = JsonReportHandler.reportAsJSON(self.report) if not self.ptConfig.quiet: @@ -439,12 +426,6 @@ def createPtParser(suppressHelp:bool=False): ptGrpDescription="Performance Harness testing configuration items." ptParserGroup = ptParser.add_argument_group(title=None if suppressHelp else ptGrpTitle, description=None if suppressHelp else ptGrpDescription) ptParserGroup.add_argument("--skip-tps-test", help=argparse.SUPPRESS if suppressHelp else "Determines whether to skip the max TPS measurement tests", action='store_true') - ptParserGroup.add_argument("--calc-producer-threads", type=str, help=argparse.SUPPRESS if suppressHelp else "Determines whether to calculate number of worker threads to use in producer thread pool (\"none\", \"lmax\", or \"full\"). \ - In \"none\" mode, the default, no calculation will be attempted and the configured --producer-threads value will be used. \ - In \"lmax\" mode, producer threads will incrementally be tested, starting at plugin default, until the performance rate ceases to increase with the addition of additional threads. \ - In \"full\" mode producer threads will incrementally be tested from plugin default..num logical processors, recording each performance and choosing the local max performance (same value as would be discovered in \"lmax\" mode). \ - Useful for graphing the full performance impact of each available thread.", - choices=["none", "lmax", "full"], default="none") ptParserGroup.add_argument("--calc-chain-threads", type=str, help=argparse.SUPPRESS if suppressHelp else "Determines whether to calculate number of worker threads to use in chain thread pool (\"none\", \"lmax\", or \"full\"). \ In \"none\" mode, the default, no calculation will be attempted and the configured --chain-threads value will be used. \ In \"lmax\" mode, producer threads will incrementally be tested, starting at plugin default, until the performance rate ceases to increase with the addition of additional threads. \ diff --git a/tests/PerformanceHarness/performance_test_basic.py b/tests/PerformanceHarness/performance_test_basic.py index a96f24ea83..6ff6345b5f 100755 --- a/tests/PerformanceHarness/performance_test_basic.py +++ b/tests/PerformanceHarness/performance_test_basic.py @@ -664,7 +664,7 @@ def setupClusterConfig(args) -> ClusterConfig: producerPluginArgs = ProducerPluginArgs(disableSubjectiveApiBilling=args.disable_subjective_billing, disableSubjectiveP2pBilling=args.disable_subjective_billing, produceBlockOffsetMs=args.produce_block_offset_ms, - producerThreads=args.producer_threads, maxTransactionTime=-1, + maxTransactionTime=-1, readOnlyWriteWindowTimeUs=args.read_only_write_window_time_us, readOnlyReadWindowTimeUs=args.read_only_read_window_time_us) httpPluginArgs = HttpPluginArgs(httpMaxBytesInFlightMb=args.http_max_bytes_in_flight_mb, httpMaxInFlightRequests=args.http_max_in_flight_requests, @@ -721,7 +721,6 @@ def _createBaseArgumentParser(defEndpointApiDef: str, defProdNodeCnt: int, defVa ptbBaseParserGroup.add_argument("--net-threads", type=int, help=argparse.SUPPRESS if suppressHelp else "Number of worker threads in net_plugin thread pool", default=4) ptbBaseParserGroup.add_argument("--disable-subjective-billing", type=bool, help=argparse.SUPPRESS if suppressHelp else "Disable subjective CPU billing for API/P2P transactions", default=True) ptbBaseParserGroup.add_argument("--produce-block-offset-ms", type=int, help=argparse.SUPPRESS if suppressHelp else "The minimum time to reserve at the end of a production round for blocks to propagate to the next block producer.", default=0) - ptbBaseParserGroup.add_argument("--producer-threads", type=int, help=argparse.SUPPRESS if suppressHelp else "Number of worker threads in producer thread pool", default=2) ptbBaseParserGroup.add_argument("--read-only-write-window-time-us", type=int, help=argparse.SUPPRESS if suppressHelp else "Time in microseconds the write window lasts.", default=200000) ptbBaseParserGroup.add_argument("--read-only-read-window-time-us", type=int, help=argparse.SUPPRESS if suppressHelp else "Time in microseconds the read window lasts.", default=60000) ptbBaseParserGroup.add_argument("--http-max-in-flight-requests", type=int, help=argparse.SUPPRESS if suppressHelp else "Maximum number of requests http_plugin should use for processing http requests. 429 error response when exceeded. -1 for unlimited", default=-1) diff --git a/tests/PerformanceHarnessScenarioRunner.py b/tests/PerformanceHarnessScenarioRunner.py index a172c5e3a3..51d6123df5 100755 --- a/tests/PerformanceHarnessScenarioRunner.py +++ b/tests/PerformanceHarnessScenarioRunner.py @@ -82,7 +82,6 @@ def main(): quiet=args.quiet, logDirRoot=Path("."), skipTpsTests=args.skip_tps_test, - calcProducerThreads=args.calc_producer_threads, calcChainThreads=args.calc_chain_threads, calcNetThreads=args.calc_net_threads, userTrxDataFile=Path(args.user_trx_data_file) if args.user_trx_data_file is not None else None, diff --git a/tests/p2p_high_latency_test.py b/tests/p2p_high_latency_test.py index 2b8028209c..d6326d4fe9 100644 --- a/tests/p2p_high_latency_test.py +++ b/tests/p2p_high_latency_test.py @@ -68,7 +68,7 @@ def exec(cmd): try: TestHelper.printSystemInfo("BEGIN") - traceNodeosArgs=" --plugin eosio::producer_plugin --produce-block-offset-ms 0 --producer-threads 1 --plugin eosio::net_plugin --net-threads 1" + traceNodeosArgs=" --plugin eosio::producer_plugin --produce-block-offset-ms 0 --plugin eosio::net_plugin --net-threads 1" if cluster.launch(pnodes=1, totalNodes=totalNodes, totalProducers=1, specificExtraNodeosArgs=specificExtraNodeosArgs, extraNodeosArgs=traceNodeosArgs) is False: Utils.cmdError("launcher") Utils.errorExit("Failed to stand up eos cluster.") From 32256ab5f404b9b960ef16a046e438fbc6ded52e Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 2 Nov 2023 15:16:44 -0500 Subject: [PATCH 41/65] GH-1690 Fix GCC compiler error --- plugins/producer_plugin/producer_plugin.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 7376bc00f6..ad927b67a7 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -850,8 +850,8 @@ class producer_plugin_impl : public std::enable_shared_from_thispacked_trx()}](fc::exception_ptr ex) { - this->log_trx_results(trx, nullptr, ex, 0, is_transient); + [this, is_transient, &next, &trx_meta](fc::exception_ptr ex) { + this->log_trx_results(trx_meta->packed_trx(), nullptr, ex, 0, is_transient); next(std::move(ex)); }; try { From acf9e9588d6b2e8871c2f2f5a085666e0d8d342b Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 6 Nov 2023 08:01:22 -0600 Subject: [PATCH 42/65] GH-1690 Remove this-> syntax --- plugins/producer_plugin/producer_plugin.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index ad927b67a7..73f3287816 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -827,12 +827,12 @@ class producer_plugin_impl : public std::enable_shared_from_this_time_tracker.add_idle_time(start); - auto trx_tracker = this->_time_tracker.start_trx(is_transient, start); + auto idle_time = _time_tracker.add_idle_time(start); + auto trx_tracker = _time_tracker.start_trx(is_transient, start); fc_tlog(_log, "Time since last trx: ${t}us", ("t", idle_time)); auto ex_handler = [this, is_transient, &next, &trx](fc::exception_ptr ex) { - this->log_trx_results(trx, nullptr, ex, 0, is_transient); + log_trx_results(trx, nullptr, ex, 0, is_transient); next(std::move(ex)); }; try { @@ -845,21 +845,21 @@ class producer_plugin_impl : public std::enable_shared_from_this_time_tracker.add_idle_time(start); - auto trx_tracker = this->_time_tracker.start_trx(is_transient, start); + auto idle_time = _time_tracker.add_idle_time(start); + auto trx_tracker = _time_tracker.start_trx(is_transient, start); fc_tlog(_log, "Time since last trx: ${t}us", ("t", idle_time)); auto exception_handler = [this, is_transient, &next, &trx_meta](fc::exception_ptr ex) { - this->log_trx_results(trx_meta->packed_trx(), nullptr, ex, 0, is_transient); + log_trx_results(trx_meta->packed_trx(), nullptr, ex, 0, is_transient); next(std::move(ex)); }; try { - if (!this->process_incoming_transaction_async(trx_meta, api_trx, return_failure_traces, trx_tracker, next)) { - if (this->in_producing_mode()) { - this->schedule_maybe_produce_block(true); + if (!process_incoming_transaction_async(trx_meta, api_trx, return_failure_traces, trx_tracker, next)) { + if (in_producing_mode()) { + schedule_maybe_produce_block(true); } else { - this->restart_speculative_block(); + restart_speculative_block(); } } } From 8d428ebfd5d9ebbbcb192a3b8b88c45b9bb9f0f7 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 6 Nov 2023 10:19:30 -0600 Subject: [PATCH 43/65] GH-1690 Cleanup to be more readable --- plugins/producer_plugin/producer_plugin.cpp | 102 ++++++++++---------- 1 file changed, 52 insertions(+), 50 deletions(-) diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 73f3287816..8e6b207401 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -795,7 +795,6 @@ class producer_plugin_impl : public std::enable_shared_from_thischain(); const auto max_trx_time_ms = (trx_type == transaction_metadata::trx_type::read_only) ? -1 : _max_transaction_time_ms.load(); fc::microseconds max_trx_cpu_usage = max_trx_time_ms < 0 ? fc::microseconds::maximum() : fc::milliseconds(max_trx_time_ms); @@ -815,57 +814,60 @@ class producer_plugin_impl : public std::enable_shared_from_thischain().get_thread_pool(), // use chain thread pool for key recovery + [this, trx{trx}, time_limit{max_trx_cpu_usage}, trx_type, is_transient, next{std::move(next)}, api_trx, return_failure_traces]() mutable { + + chain::controller& chain = chain_plug->chain(); + transaction_metadata_ptr trx_meta; + try { + trx_meta = transaction_metadata::recover_keys(trx, chain.get_chain_id(), time_limit, trx_type, + chain.configured_subjective_signature_length_limit()); + } catch (...) { + // use read_write when read is likely fine; maintains previous behavior of next() always being called from the main thread + app().executor().post( + priority::low, exec_queue::read_write, + [this, ex_ptr{std::current_exception()}, trx{std::move(trx)}, is_transient, next{std::move(next)}]() { + auto start = fc::time_point::now(); + auto idle_time = _time_tracker.add_idle_time(start); + auto trx_tracker = _time_tracker.start_trx(is_transient, start); + fc_tlog(_log, "Time since last trx: ${t}us", ("t", idle_time)); + auto ex_handler = [this, is_transient, &next, &trx](fc::exception_ptr ex) { + log_trx_results(trx, nullptr, ex, 0, is_transient); + next(std::move(ex)); + }; + try { + std::rethrow_exception(ex_ptr); + } CATCH_AND_CALL(ex_handler) + }); + return; + } + + // key recovery complete, continue execution on the main thread + app().executor().post( + priority::low, exec_queue::read_write, + [this, trx_meta{std::move(trx_meta)}, is_transient, next{std::move(next)}, api_trx, return_failure_traces]() { + auto start = fc::time_point::now(); + auto idle_time = _time_tracker.add_idle_time(start); + auto trx_tracker = _time_tracker.start_trx(is_transient, start); + fc_tlog(_log, "Time since last trx: ${t}us", ("t", idle_time)); + + auto exception_handler = [this, is_transient, &next, &trx_meta](fc::exception_ptr ex) { + log_trx_results(trx_meta->packed_trx(), nullptr, ex, 0, is_transient); + next(std::move(ex)); + }; try { - trx_meta = transaction_metadata::recover_keys( trx, chain.get_chain_id(), time_limit, trx_type, chain.configured_subjective_signature_length_limit() ); - } catch(...) { - // use read_write when read is likely fine; this maintains previous behavior of next() always being called from the main thread - app().executor().post(priority::low, exec_queue::read_write, - [ex_ptr = std::current_exception(), this, trx{std::move(trx)}, is_transient, next{std::move(next)}]() { - auto start = fc::time_point::now(); - auto idle_time = _time_tracker.add_idle_time(start); - auto trx_tracker = _time_tracker.start_trx(is_transient, start); - fc_tlog(_log, "Time since last trx: ${t}us", ("t", idle_time)); - auto ex_handler = - [this, is_transient, &next, &trx](fc::exception_ptr ex) { - log_trx_results(trx, nullptr, ex, 0, is_transient); - next(std::move(ex)); - }; - try { - std::rethrow_exception(ex_ptr); - } CATCH_AND_CALL(ex_handler) - }); - return; + if (!process_incoming_transaction_async(trx_meta, api_trx, return_failure_traces, trx_tracker, next)) { + if (in_producing_mode()) { + schedule_maybe_produce_block(true); + } else { + restart_speculative_block(); + } + } } - - app().executor().post(priority::low, exec_queue::read_write, - [this, trx_meta{std::move(trx_meta)}, api_trx, is_transient, next{std::move(next)}, return_failure_traces]() mutable { - auto start = fc::time_point::now(); - auto idle_time = _time_tracker.add_idle_time(start); - auto trx_tracker = _time_tracker.start_trx(is_transient, start); - fc_tlog(_log, "Time since last trx: ${t}us", ("t", idle_time)); - - auto exception_handler = - [this, is_transient, &next, &trx_meta](fc::exception_ptr ex) { - log_trx_results(trx_meta->packed_trx(), nullptr, ex, 0, is_transient); - next(std::move(ex)); - }; - try { - if (!process_incoming_transaction_async(trx_meta, api_trx, return_failure_traces, trx_tracker, next)) { - if (in_producing_mode()) { - schedule_maybe_produce_block(true); - } else { - restart_speculative_block(); - } - } - } - CATCH_AND_CALL(exception_handler); - }); - } ); + CATCH_AND_CALL(exception_handler); + }); + }); } bool process_incoming_transaction_async(const transaction_metadata_ptr& trx, From 5bb7313ae324353bdf2c15959194fd4286527b51 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 6 Nov 2023 11:53:11 -0600 Subject: [PATCH 44/65] GH-1858 Add better error handling --- tests/trx_generator/trx_provider.cpp | 28 ++++++++++++++++++++-------- tests/trx_generator/trx_provider.hpp | 1 + 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/tests/trx_generator/trx_provider.cpp b/tests/trx_generator/trx_provider.cpp index 4ff29007fd..301a817c4c 100644 --- a/tests/trx_generator/trx_provider.cpp +++ b/tests/trx_generator/trx_provider.cpp @@ -109,16 +109,21 @@ namespace eosio::testing { void http_connection::disconnect() { int max = 30; int waited = 0; - while (_sent.load() != _acknowledged.load() && waited < max) { - ilog("http_connection::disconnect waiting on ack - sent ${s} | acked ${a} | waited ${w}", - ("s", _sent.load())("a", _acknowledged.load())("w", waited)); + for (uint64_t sent = _sent.load(), acknowledged = _acknowledged.load(); + sent != acknowledged && waited < max; + sent = _sent.load(), acknowledged = _acknowledged.load()) { + ilog("disconnect waiting on ack - sent ${s} | acked ${a} | waited ${w}", + ("s", sent)("a", acknowledged)("w", waited)); sleep(1); ++waited; } if (waited == max) { - elog("http_connection::disconnect failed to receive all acks in time - sent ${s} | acked ${a} | waited ${w}", + elog("disconnect failed to receive all acks in time - sent ${s} | acked ${a} | waited ${w}", ("s", _sent.load())("a", _acknowledged.load())("w", waited)); } + if (_errors.load()) { + elog("${n} errors reported during http calls, see logs", ("n", _errors.load())); + } } bool http_connection::needs_response_trace_info() { @@ -151,10 +156,17 @@ namespace eosio::testing { trx_acknowledged(trx_id, fc::time_point::now()); if (ec) { elog("http error: ${c}: ${m}", ("c", ec.value())("m", ec.message())); - throw std::runtime_error(ec.message()); + ++_errors; + return; } if (this->needs_response_trace_info() && response.result() == boost::beast::http::status::ok) { + bool exception = false; + auto exception_handler = [this, &response, &exception](const fc::exception_ptr& ex) { + elog("Fail to parse JSON from string: ${string}", ("string", response.body())); + ++_errors; + exception = true; + }; try { fc::variant resp_json = fc::json::from_string(response.body()); if (resp_json.is_object() && resp_json.get_object().contains("processed")) { @@ -186,9 +198,9 @@ namespace eosio::testing { elog("async_http_request Transaction failed, transaction not processed: ${string}", ("string", response.body())); } - } - EOS_RETHROW_EXCEPTIONS(chain::json_parse_exception, "Fail to parse JSON from string: ${string}", - ("string", response.body())); + } CATCH_AND_CALL(exception_handler) + if (exception) + return; } if (!(response.result() == boost::beast::http::status::accepted || diff --git a/tests/trx_generator/trx_provider.hpp b/tests/trx_generator/trx_provider.hpp index f5685fe06b..df8fa3049a 100644 --- a/tests/trx_generator/trx_provider.hpp +++ b/tests/trx_generator/trx_provider.hpp @@ -88,6 +88,7 @@ namespace eosio::testing { std::atomic _acknowledged{0}; std::atomic _sent{0}; + std::atomic _errors{0}; explicit http_connection(const provider_base_config& provider_config) : provider_connection(provider_config) {} From eecd4c5dfb819dfa3af65147024b1f09abe92562 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 6 Nov 2023 11:53:37 -0600 Subject: [PATCH 45/65] GH-1858 Use one strand --- tests/trx_generator/http_client_async.hpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/trx_generator/http_client_async.hpp b/tests/trx_generator/http_client_async.hpp index 7aad239e5c..4fc754c922 100644 --- a/tests/trx_generator/http_client_async.hpp +++ b/tests/trx_generator/http_client_async.hpp @@ -4,6 +4,9 @@ // libs/beast/example/http/client/async/http_client_async.cpp // with minimum modification and yet reusable. // +// Updated to use strand like in the boost example +// libs/beast/example/http/client/crawl/http_crawl.cpp +// // // Copyright (c) 2016-2019 Vinnie Falco (vinnie dot falco at gmail dot com) // @@ -48,6 +51,7 @@ inline void fail(beast::error_code ec, char const* what) { std::cerr << what << // Performs an HTTP GET and prints the response class session : public std::enable_shared_from_this { + net::strand ex_; tcp::resolver resolver_; beast::tcp_stream stream_; beast::flat_buffer buffer_; // (Must persist between reads) @@ -59,8 +63,9 @@ class session : public std::enable_shared_from_this { // Objects are constructed with a strand to // ensure that handlers do not execute concurrently. explicit session(net::io_context& ioc, const response_callback_t& response_callback) - : resolver_(net::make_strand(ioc)) - , stream_(net::make_strand(ioc)) + : ex_(net::make_strand(ioc.get_executor())) + , resolver_(ex_) + , stream_(ex_) , response_callback_(response_callback) {} // Start the asynchronous operation From 6c671fd54d517ab62e2e890ea2496b208a83c90f Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Mon, 6 Nov 2023 16:42:26 -0500 Subject: [PATCH 46/65] wait-for-exact-target not wait-for-exact-target-workflow --- .github/workflows/release.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index dfcd19869d..1eb8f20fdc 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -21,7 +21,7 @@ jobs: file: 'leap-dev.*amd64.deb' target: ${{github.sha}} artifact-name: leap-dev-ubuntu20-amd64 - wait-for-exact-target-workflow: true + wait-for-exact-target: true - name: Get ubuntu22 leap-dev.deb uses: AntelopeIO/asset-artifact-download-action@v3 with: @@ -30,7 +30,7 @@ jobs: file: 'leap-dev.*amd64.deb' target: ${{github.sha}} artifact-name: leap-dev-ubuntu22-amd64 - wait-for-exact-target-workflow: true + wait-for-exact-target: true - name: Create Dockerfile run: | cat < Dockerfile From 1b0cebae0a7cb9c99a905b4d84de02c8bc20bd3e Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Tue, 7 Nov 2023 13:59:13 -0500 Subject: [PATCH 47/65] use tester's default_config, use std::optional for limits. remove hard/soft cpu and vm limits --- .../chain/webassembly/eos-vm-oc/config.hpp | 10 ++--- .../runtimes/eos-vm-oc/compile_trampoline.cpp | 22 ++++++----- .../testing/include/eosio/testing/tester.hpp | 7 ++++ libraries/testing/tester.cpp | 11 ------ unittests/eosvmoc_limits_tests.cpp | 39 +++++++++---------- 5 files changed, 43 insertions(+), 46 deletions(-) diff --git a/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp b/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp index fcda6a368d..b8da19ecfb 100644 --- a/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp +++ b/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp @@ -19,13 +19,13 @@ struct config { // nodeos enforces the limits by the default values. // libtester disables the limits in all tests, except enforces the limits // in the tests in unittests/eosvmoc_limits_tests.cpp. - rlimit cpu_limits {20u, 20u}; - rlimit vm_limits {512u*1024u*1024u, 512u*1024u*1024u}; - uint64_t stack_size_limit {16u*1024u}; - size_t generated_code_size_limit {16u*1024u*1024u}; + std::optional cpu_limit {20u}; + std::optional vm_limit {512u*1024u*1024u}; + std::optional stack_size_limit {16u*1024u}; + std::optional generated_code_size_limit {16u*1024u*1024u}; }; }}} FC_REFLECT(rlimit, (rlim_cur)(rlim_max)) -FC_REFLECT(eosio::chain::eosvmoc::config, (cache_size)(threads)(cpu_limits)(vm_limits)(stack_size_limit)(generated_code_size_limit)) +FC_REFLECT(eosio::chain::eosvmoc::config, (cache_size)(threads)(cpu_limit)(vm_limit)(stack_size_limit)(generated_code_size_limit)) diff --git a/libraries/chain/webassembly/runtimes/eos-vm-oc/compile_trampoline.cpp b/libraries/chain/webassembly/runtimes/eos-vm-oc/compile_trampoline.cpp index de506119f2..11467afd27 100644 --- a/libraries/chain/webassembly/runtimes/eos-vm-oc/compile_trampoline.cpp +++ b/libraries/chain/webassembly/runtimes/eos-vm-oc/compile_trampoline.cpp @@ -167,22 +167,26 @@ void run_compile_trampoline(int fd) { const auto& conf = std::get(message).eosvmoc_config; - // enforce cpu_limits only when it is not RLIM_INFINITY (libtester may - // disable it by setting it to RLIM_INFINITY). - if(conf.cpu_limits.rlim_cur != RLIM_INFINITY) { - setrlimit(RLIMIT_CPU, &conf.cpu_limits); + // enforce cpu limit only when it is set + // (libtester may disable it) + if(conf.cpu_limit) { + struct rlimit cpu_limit = {*conf.cpu_limit, *conf.cpu_limit}; + setrlimit(RLIMIT_CPU, &cpu_limit); } - // enforce vm_limits only when it is not RLIM_INFINITY (libtester may - // disable it by setting it to RLIM_INFINITY). - if(conf.vm_limits.rlim_cur != RLIM_INFINITY) { - setrlimit(RLIMIT_AS, &conf.vm_limits); + // enforce vm limit only when it is set + // (libtester may disable it) + if(conf.vm_limit) { + struct rlimit vm_limit = {*conf.vm_limit, *conf.vm_limit}; + setrlimit(RLIMIT_AS, &vm_limit); } struct rlimit core_limits = {0u, 0u}; setrlimit(RLIMIT_CORE, &core_limits); - run_compile(std::move(fds[0]), std::move(fds[1]), conf.stack_size_limit, conf.generated_code_size_limit); + uint64_t stack_size_limit = conf.stack_size_limit ? *conf.stack_size_limit : std::numeric_limits::max(); + size_t generated_code_size_limit = conf.generated_code_size_limit ? * conf.generated_code_size_limit : std::numeric_limits::max(); + run_compile(std::move(fds[0]), std::move(fds[1]), stack_size_limit, generated_code_size_limit); _exit(0); } else if(pid == -1) diff --git a/libraries/testing/include/eosio/testing/tester.hpp b/libraries/testing/include/eosio/testing/tester.hpp index 5389dc076c..ef559ad985 100644 --- a/libraries/testing/include/eosio/testing/tester.hpp +++ b/libraries/testing/include/eosio/testing/tester.hpp @@ -406,6 +406,13 @@ namespace eosio { namespace testing { cfg.contracts_console = true; cfg.eosvmoc_config.cache_size = 1024*1024*8; + // don't enforce OC compilation subject limits for tests, + // particularly EOS EVM tests may run over those limits + cfg.eosvmoc_config.cpu_limit.reset(); + cfg.eosvmoc_config.vm_limit.reset(); + cfg.eosvmoc_config.stack_size_limit.reset(); + cfg.eosvmoc_config.generated_code_size_limit.reset(); + // don't use auto tier up for tests, since the point is to test diff vms cfg.eosvmoc_tierup = chain::wasm_interface::vm_oc_enable::oc_none; diff --git a/libraries/testing/tester.cpp b/libraries/testing/tester.cpp index 2db0b28f11..8a9b7a0ad6 100644 --- a/libraries/testing/tester.cpp +++ b/libraries/testing/tester.cpp @@ -317,17 +317,6 @@ namespace eosio { namespace testing { } } - // libtester does not enforce EOSVMOC limits for all tests except those in - // eosvmoc_limits_test.cpp, where one or more of the limits are set to - // max to indicate the limits are enforced. - if (cfg.eosvmoc_config.cpu_limits.rlim_cur != RLIM_INFINITY && cfg.eosvmoc_config.vm_limits.rlim_cur != RLIM_INFINITY && cfg.eosvmoc_config.stack_size_limit != std::numeric_limits::max() && cfg.eosvmoc_config.generated_code_size_limit != std::numeric_limits::max()) { - // set to inifinity or max such that they are not enforced. - cfg.eosvmoc_config.cpu_limits.rlim_cur = RLIM_INFINITY; - cfg.eosvmoc_config.vm_limits.rlim_cur = RLIM_INFINITY; - cfg.eosvmoc_config.stack_size_limit = std::numeric_limits::max(); - cfg.eosvmoc_config.generated_code_size_limit = std::numeric_limits::max(); - } - control.reset( new controller(cfg, std::move(pfs), *expected_chain_id) ); control->add_indices(); if (lambda) lambda(); diff --git a/unittests/eosvmoc_limits_tests.cpp b/unittests/eosvmoc_limits_tests.cpp index 64c4007dc3..4bf3e0cff2 100644 --- a/unittests/eosvmoc_limits_tests.cpp +++ b/unittests/eosvmoc_limits_tests.cpp @@ -71,27 +71,34 @@ void limit_not_violated_test(const eosvmoc::config& eosvmoc_config) { ); } -// test libtester does not enforce limits -BOOST_AUTO_TEST_CASE( default_limits ) { try { +// test all limits are not set for tests +BOOST_AUTO_TEST_CASE( limits_not_set ) { try { + validating_tester chain; + auto& cfg = chain.get_config(); + + BOOST_REQUIRE(cfg.eosvmoc_config.cpu_limit == std::nullopt); + BOOST_REQUIRE(cfg.eosvmoc_config.vm_limit == std::nullopt); + BOOST_REQUIRE(cfg.eosvmoc_config.stack_size_limit == std::nullopt); + BOOST_REQUIRE(cfg.eosvmoc_config.generated_code_size_limit == std::nullopt); +} FC_LOG_AND_RETHROW() } + +// test limits are not enforced unless limits in eosvmoc_config +// are modified +BOOST_AUTO_TEST_CASE( limits_not_enforced ) { try { eosvmoc::config eosvmoc_config; limit_not_violated_test(eosvmoc_config); } FC_LOG_AND_RETHROW() } -// test VM limits are checked -BOOST_AUTO_TEST_CASE( vm_limits ) { try { +// test VM limit are checked +BOOST_AUTO_TEST_CASE( vm_limit ) { try { eosvmoc::config eosvmoc_config; - // disable non-tested limits - eosvmoc_config.cpu_limits.rlim_cur = RLIM_INFINITY; - eosvmoc_config.stack_size_limit = std::numeric_limits::max(); - eosvmoc_config.generated_code_size_limit = std::numeric_limits::max(); - // set vm_limit to a small value such that it is exceeded - eosvmoc_config.vm_limits.rlim_cur = 64u*1024u*1024u; + eosvmoc_config.vm_limit = 64u*1024u*1024u; limit_violated_test(eosvmoc_config); // set vm_limit to a large value such that it is not exceeded - eosvmoc_config.vm_limits.rlim_cur = 128u*1024u*1024u; + eosvmoc_config.vm_limit = 128u*1024u*1024u; limit_not_violated_test(eosvmoc_config); } FC_LOG_AND_RETHROW() } @@ -99,11 +106,6 @@ BOOST_AUTO_TEST_CASE( vm_limits ) { try { BOOST_AUTO_TEST_CASE( stack_limit ) { try { eosvmoc::config eosvmoc_config; - // disable non-tested limits - eosvmoc_config.cpu_limits.rlim_cur = RLIM_INFINITY; - eosvmoc_config.vm_limits.rlim_cur = RLIM_INFINITY; - eosvmoc_config.generated_code_size_limit = std::numeric_limits::max(); - // The stack size of the compiled WASM in the test is 104. // Set stack_size_limit one less than the actual needed stack size eosvmoc_config.stack_size_limit = 103; @@ -118,11 +120,6 @@ BOOST_AUTO_TEST_CASE( stack_limit ) { try { BOOST_AUTO_TEST_CASE( generated_code_size_limit ) { try { eosvmoc::config eosvmoc_config; - // disable non-tested limits - eosvmoc_config.cpu_limits.rlim_cur = RLIM_INFINITY; - eosvmoc_config.vm_limits.rlim_cur = RLIM_INFINITY; - eosvmoc_config.stack_size_limit = std::numeric_limits::max(); - // The generated code size of the compiled WASM in the test is 36856. // Set generated_code_size_limit to the actual generated code size eosvmoc_config.generated_code_size_limit = 36856; From 80de19e1df5ea14cb26eb532787b888749f639cb Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 7 Nov 2023 14:51:32 -0600 Subject: [PATCH 48/65] GH-1523 add additional comments --- libraries/chain/block_header_state.cpp | 13 ++++++++----- libraries/chain/controller.cpp | 1 + libraries/hotstuff/chain_pacemaker.cpp | 1 + plugins/producer_plugin/producer_plugin.cpp | 2 +- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/libraries/chain/block_header_state.cpp b/libraries/chain/block_header_state.cpp index 1d55254d6e..c1b8992776 100644 --- a/libraries/chain/block_header_state.cpp +++ b/libraries/chain/block_header_state.cpp @@ -17,9 +17,9 @@ namespace eosio { namespace chain { } uint32_t get_next_next_round_block_num( block_timestamp_type t, uint32_t block_num ) { - auto index = t.slot % config::producer_repetitions; - // remainder of current + next round - return block_num + config::producer_repetitions - index + config::producer_repetitions; + auto index = t.slot % config::producer_repetitions; // current index in current round + // (increment to the end of this round ) + next round + return block_num + (config::producer_repetitions - index) + config::producer_repetitions; } } @@ -43,6 +43,9 @@ namespace eosio { namespace chain { return blocknums[ index ]; } + // create pending_block_header_state from this for `when` + // If hotstuff_activated then use new consensus values and simpler active schedule update. + // If notstuff is not activated then use previous pre-hotstuff consensus logic. pending_block_header_state block_header_state::next( block_timestamp_type when, bool hotstuff_activated, uint16_t num_prev_blocks_to_confirm )const @@ -55,14 +58,14 @@ namespace eosio { namespace chain { (when = header.timestamp).slot++; } - auto proauth = get_scheduled_producer(when); - result.block_num = block_num + 1; result.previous = id; result.timestamp = when; result.active_schedule_version = active_schedule.version; result.prev_activated_protocol_features = activated_protocol_features; + auto proauth = get_scheduled_producer(when); + result.valid_block_signing_authority = proauth.authority; result.producer = proauth.producer_name; result.last_proposed_finalizer_set_generation = last_proposed_finalizer_set_generation; diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 4cde143371..f65ad54d60 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -3185,6 +3185,7 @@ std::optional controller::pending_producer_block_id()const { void controller::set_hs_irreversible_block_num(uint32_t block_num) { // needs to be set by qc_chain at startup and as irreversible changes + assert(block_num > 0); my->hs_irreversible_block_num = block_num; } diff --git a/libraries/hotstuff/chain_pacemaker.cpp b/libraries/hotstuff/chain_pacemaker.cpp index 43eeb49670..eac294893a 100644 --- a/libraries/hotstuff/chain_pacemaker.cpp +++ b/libraries/hotstuff/chain_pacemaker.cpp @@ -234,6 +234,7 @@ namespace eosio { namespace hotstuff { std::scoped_lock g( _chain_state_mutex ); if (_active_finalizer_set.generation == 0) { // switching from dpos to hotstuff, all nodes will switch at same block height + // block header extension is set in finalize_block to value set by host function set_finalizers _chain->set_hs_irreversible_block_num(blk->block_num); // can be any value <= dpos lib } _active_finalizer_set = std::move(std::get(*ext)); diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 2c592610d5..4dd8ec631e 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -1862,7 +1862,7 @@ producer_plugin_impl::start_block_result producer_plugin_impl::start_block() { try { uint16_t blocks_to_confirm = 0; - if (in_producing_mode() && hbs->dpos_irreversible_blocknum != hs_dpos_irreversible_blocknum) { + if (in_producing_mode() && hbs->dpos_irreversible_blocknum != hs_dpos_irreversible_blocknum) { // only if hotstuff not enabled // determine how many blocks this producer can confirm // 1) if it is not a producer from this node, assume no confirmations (we will discard this block anyway) // 2) if it is a producer on this node that has never produced, the conservative approach is to assume no From 9f94a40f236835b965459ab88b46490d14874afd Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Tue, 7 Nov 2023 15:59:44 -0500 Subject: [PATCH 49/65] remove unused FC_REFLECT for rlimit --- .../chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp b/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp index b8da19ecfb..43f9efede2 100644 --- a/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp +++ b/libraries/chain/include/eosio/chain/webassembly/eos-vm-oc/config.hpp @@ -27,5 +27,4 @@ struct config { }}} -FC_REFLECT(rlimit, (rlim_cur)(rlim_max)) FC_REFLECT(eosio::chain::eosvmoc::config, (cache_size)(threads)(cpu_limit)(vm_limit)(stack_size_limit)(generated_code_size_limit)) From f98dcce373c8facac4f3d318ca12345d288ba07e Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 8 Nov 2023 08:50:16 -0500 Subject: [PATCH 50/65] [5.0] Fix chainbase issue when shrinking the database size. --- libraries/chainbase | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chainbase b/libraries/chainbase index 7615ddab28..1e0d680c78 160000 --- a/libraries/chainbase +++ b/libraries/chainbase @@ -1 +1 @@ -Subproject commit 7615ddab287e06fd31f800e66fe39b3a19320ec8 +Subproject commit 1e0d680c78bb1238e6cc5af477ac5ad0f6154712 From 96fc5334dd7d3df201baabe614e481239040d2ce Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 8 Nov 2023 10:38:33 -0500 Subject: [PATCH 51/65] Update chainbase submodule to tip of main branch (including chainbase #29) --- libraries/chainbase | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chainbase b/libraries/chainbase index 1e0d680c78..91a7398e60 160000 --- a/libraries/chainbase +++ b/libraries/chainbase @@ -1 +1 @@ -Subproject commit 1e0d680c78bb1238e6cc5af477ac5ad0f6154712 +Subproject commit 91a7398e60522ab7cc6433c0c56aa8daf297deac From 11df1245ae7fff472f3119bd69b55cf230f3dad1 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 8 Nov 2023 12:13:47 -0600 Subject: [PATCH 52/65] GH-1871 Make sure a unique_conn_node_id is always provided to prometheus --- plugins/net_plugin/net_plugin.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 154d5c23f3..0f159cf4bf 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -4765,9 +4765,18 @@ namespace eosio { ++num_peers; } fc::unique_lock g_conn(c->conn_mtx); + if (c->unique_conn_node_id.empty()) { // still connecting, use temp id so that non-connected peers are reported + if (!c->p2p_address.empty()) { + c->unique_conn_node_id = fc::sha256::hash(c->p2p_address).str().substr(0, 7); + } else if (!c->remote_endpoint_ip.empty()) { + c->unique_conn_node_id = fc::sha256::hash(c->remote_endpoint_ip).str().substr(0, 7); + } else { + c->unique_conn_node_id = fc::sha256::hash(std::to_string(c->connection_id)).str().substr(0, 7); + } + } + std::string conn_node_id = c->unique_conn_node_id; boost::asio::ip::address_v6::bytes_type addr = c->remote_endpoint_ip_array; std::string p2p_addr = c->p2p_address; - std::string conn_node_id = c->unique_conn_node_id; g_conn.unlock(); per_connection.peers.emplace_back( net_plugin::p2p_per_connection_metrics::connection_metric{ From 781c36128550f68827d38f74eaefdb0cbce4aa0a Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 8 Nov 2023 12:30:38 -0600 Subject: [PATCH 53/65] GH-1878 Add test cases for mapped_private --- tests/db_modes_test.sh | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/db_modes_test.sh b/tests/db_modes_test.sh index d9143a2e77..6b5fd20fe3 100755 --- a/tests/db_modes_test.sh +++ b/tests/db_modes_test.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash -# This test is intended to verify that switching between DB modes "just works". Addtionally -# it tries to make sure the dirty bit behaves as expected even in heap mode. +# This test is intended to verify that switching between DB modes "just works". Additionally +# it tries to make sure the dirty bit behaves as expected even in heap and mapped_private mode. set -euo pipefail @@ -79,6 +79,10 @@ run_expect_failure() { run_expect_success --delete-all-blocks #use previous DB with heap mode run_expect_success --database-map-mode heap +#use previous DB with mapped_private mode +run_expect_success --database-map-mode mapped_private +#use previous DB with heap mode +run_expect_success --database-map-mode heap #test lock mode if enabled if (( $TEST_LOCKED_MODE == 1 )); then run_expect_success --database-map-mode locked @@ -94,11 +98,15 @@ run_and_kill run_expect_failure #should also still be dirty in heap mode run_expect_failure --database-map-mode heap +#should also still be dirty in mapped_private mode +run_expect_failure --database-map-mode mapped_private #start over again! but this time start with heap mode run_expect_success --delete-all-blocks --database-map-mode heap #Then switch back to mapped run_expect_success +#Then switch back to mapped_private +run_expect_success --database-map-mode mapped_private #try killing it while in heap mode run_and_kill --database-map-mode heap #should be dirty if we run in either mode node From ccf7356a4abc291b5bdfab9c211ca92704c9c3f1 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 8 Nov 2023 13:42:28 -0600 Subject: [PATCH 54/65] GH-1878 Update test condition now that id can't be empty --- tests/p2p_sync_throttle_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/p2p_sync_throttle_test.py b/tests/p2p_sync_throttle_test.py index 421d411243..d05325f4c9 100755 --- a/tests/p2p_sync_throttle_test.py +++ b/tests/p2p_sync_throttle_test.py @@ -144,7 +144,7 @@ def extractPrometheusMetric(connID: str, metric: str, text: str): time.sleep(0.5) continue Print('Throttling Node Start State') - throttlingNodePortMap = {port: id for id, port in connPorts if id != '' and port != '9877'} + throttlingNodePortMap = {port: id for id, port in connPorts if port != '0' and port != '9877'} throttlingNodeConnId = next(iter(throttlingNodePortMap.values())) # 9879 startSyncThrottlingBytesSent = extractPrometheusMetric(throttlingNodeConnId, 'block_sync_bytes_sent', @@ -181,7 +181,7 @@ def extractPrometheusMetric(connID: str, metric: str, text: str): errorLimit -= 1 continue Print('Throttled Node Start State') - throttledNodePortMap = {port: id for id, port in connPorts if id != ''} + throttledNodePortMap = {port: id for id, port in connPorts if port != '0'} throttledNodeConnId = next(iter(throttledNodePortMap.values())) # 9878 Print(throttledNodeConnId) startSyncThrottledBytesReceived = extractPrometheusMetric(throttledNodeConnId, From 0ad72734d5174581e62215f3836f70c634ef1196 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 10 Nov 2023 06:49:01 -0600 Subject: [PATCH 55/65] Revert "GH-1461 Update base64 from upstream" This reverts commit 0842972b1edad772e03d12973d7e7b16e6845d47. --- libraries/libfc/include/fc/crypto/base64.hpp | 9 +- libraries/libfc/src/crypto/base64.cpp | 392 +++++-------------- libraries/libfc/test/test_base64.cpp | 134 +------ 3 files changed, 100 insertions(+), 435 deletions(-) diff --git a/libraries/libfc/include/fc/crypto/base64.hpp b/libraries/libfc/include/fc/crypto/base64.hpp index d57e2d11e5..9559214df1 100644 --- a/libraries/libfc/include/fc/crypto/base64.hpp +++ b/libraries/libfc/include/fc/crypto/base64.hpp @@ -1,15 +1,14 @@ #pragma once #include -#include namespace fc { std::string base64_encode(unsigned char const* bytes_to_encode, unsigned int in_len); inline std::string base64_encode(char const* bytes_to_encode, unsigned int in_len) { return base64_encode( (unsigned char const*)bytes_to_encode, in_len); } -std::string base64_encode( const std::string_view& enc ); -std::string base64_decode( const std::string_view& encoded_string); +std::string base64_encode( const std::string& enc ); +std::string base64_decode( const std::string& encoded_string); std::string base64url_encode(unsigned char const* bytes_to_encode, unsigned int in_len); inline std::string base64url_encode(char const* bytes_to_encode, unsigned int in_len) { return base64url_encode( (unsigned char const*)bytes_to_encode, in_len); } -std::string base64url_encode( const std::string_view& enc ); -std::string base64url_decode( const std::string_view& encoded_string); +std::string base64url_encode( const std::string& enc ); +std::string base64url_decode( const std::string& encoded_string); } // namespace fc diff --git a/libraries/libfc/src/crypto/base64.cpp b/libraries/libfc/src/crypto/base64.cpp index d5369dcbad..5ff4e68290 100644 --- a/libraries/libfc/src/crypto/base64.cpp +++ b/libraries/libfc/src/crypto/base64.cpp @@ -1,14 +1,10 @@ -/* +#include +#include +#include +/* base64.cpp and base64.h - base64 encoding and decoding with C++. - More information at - https://renenyffenegger.ch/notes/development/Base64/Encoding-and-decoding-base-64-with-cpp - https://github.com/ReneNyffenegger/cpp-base64 - - Version: 2.rc.09 (release candidate) - - Copyright (C) 2004-2017, 2020-2022 René Nyffenegger + Copyright (C) 2004-2008 René Nyffenegger This source code is provided 'as-is', without any express or implied warranty. In no event will the author be held liable for any damages @@ -32,332 +28,134 @@ */ -#include -#include - -#include -#include -#if __cplusplus >= 201703L -#include -#endif // __cplusplus >= 201703L - namespace fc { -// base64.hpp -// Added template return type - -std::string base64_encode (std::string const& s, bool url = false); -std::string base64_encode_pem (std::string const& s); -std::string base64_encode_mime(std::string const& s); - -std::string base64_decode(std::string const& s, bool remove_linebreaks = false); -std::string base64_encode(unsigned char const*, size_t len, bool url = false); - -#if __cplusplus >= 201703L -// -// Interface with std::string_view rather than const std::string& -// Requires C++17 -// Provided by Yannic Bonenberger (https://github.com/Yannic) -// -std::string base64_encode (std::string_view s, bool url = false); -std::string base64_encode_pem (std::string_view s); -std::string base64_encode_mime(std::string_view s); - -std::string base64_decode(std::string_view s, bool remove_linebreaks = false); -#endif // __cplusplus >= 201703L - -// base64.cpp - -// Includes performance improvement from unmerged PR: https://github.com/ReneNyffenegger/cpp-base64/pull/27 - - // - // Depending on the url parameter in base64_chars, one of - // two sets of base64 characters needs to be chosen. - // They differ in their last two characters. - // -static const char* base64_chars[2] = { - "ABCDEFGHIJKLMNOPQRSTUVWXYZ" - "abcdefghijklmnopqrstuvwxyz" - "0123456789" - "+/", - - "ABCDEFGHIJKLMNOPQRSTUVWXYZ" - "abcdefghijklmnopqrstuvwxyz" - "0123456789" - "-_"}; - -static const unsigned char from_base64_chars[256] = { - 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, - 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, - 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 62, 64, 62, 64, 63, - 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 64, 64, 64, 64, 64, 64, - 64, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, - 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 64, 64, 64, 64, 63, - 64, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, - 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 64, 64, 64, 64, 64, - 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, - 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, - 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, - 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, - 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, - 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, - 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, - 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64, 64 -}; - -static unsigned int pos_of_char(const unsigned char chr) { - // - // Return the position of chr within base64_encode() - // - - auto c = from_base64_chars[chr]; - if (c != 64) return c; - - // - // 2020-10-23: Throw std::exception rather than const char* - //(Pablo Martin-Gomez, https://github.com/Bouska) - // - // throw std::runtime_error("Input is not valid base64-encoded data."); - - // - // FC_ASSERT instead of throw runtime_error - // - FC_ASSERT(false, "encountered non-base64 character"); -} - -static std::string insert_linebreaks(std::string str, size_t distance) { - // - // Provided by https://github.com/JomaCorpFX, adapted by me. - // - if (!str.length()) { - return std::string{}; - } - - size_t pos = distance; +static constexpr char base64_chars[] = + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "abcdefghijklmnopqrstuvwxyz" + "0123456789+/"; +static constexpr char base64url_chars[] = + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "abcdefghijklmnopqrstuvwxyz" + "0123456789-_"; - while (pos < str.size()) { - str.insert(pos, "\n"); - pos += distance + 1; - } - - return str; -} - -template -static std::string encode_with_line_breaks(String s) { - return insert_linebreaks(base64_encode(s, false), line_length); -} - -template -static std::string encode_pem(String s) { - return encode_with_line_breaks(s); -} - -template -static std::string encode_mime(String s) { - return encode_with_line_breaks(s); -} +static_assert(sizeof(base64_chars) == sizeof(base64url_chars), "base64 and base64url must have the same amount of chars"); -template -static std::string encode(String s, bool url) { - return base64_encode(reinterpret_cast(s.data()), s.length(), url); +static inline void throw_on_nonbase64(unsigned char c, const char* const b64_chars) { + FC_ASSERT(isalnum(c) || (c == b64_chars[sizeof(base64_chars)-3]) || (c == b64_chars[sizeof(base64_chars)-2]), "encountered non-base64 character"); } -std::string base64_encode(unsigned char const* bytes_to_encode, size_t in_len, bool url) { - - size_t len_encoded = (in_len +2) / 3 * 4; - - unsigned char trailing_char = url ? '.' : '='; - - // - // Choose set of base64 characters. They differ - // for the last two positions, depending on the url - // parameter. - // A bool (as is the parameter url) is guaranteed - // to evaluate to either 0 or 1 in C++ therefore, - // the correct character set is chosen by subscripting - // base64_chars with url. - // - const char* base64_chars_ = base64_chars[url]; +std::string base64_encode_impl(unsigned char const* bytes_to_encode, unsigned int in_len, const char* const b64_chars) { - std::string ret; - ret.reserve(len_encoded); + std::string ret; + int i = 0; + int j = 0; + unsigned char char_array_3[3]; + unsigned char char_array_4[4]; - unsigned int pos = 0; + while (in_len--) { + char_array_3[i++] = *(bytes_to_encode++); + if (i == 3) { + char_array_4[0] = (char_array_3[0] & 0xfc) >> 2; + char_array_4[1] = ((char_array_3[0] & 0x03) << 4) + ((char_array_3[1] & 0xf0) >> 4); + char_array_4[2] = ((char_array_3[1] & 0x0f) << 2) + ((char_array_3[2] & 0xc0) >> 6); + char_array_4[3] = char_array_3[2] & 0x3f; - while (pos < in_len) { - ret.push_back(base64_chars_[(bytes_to_encode[pos + 0] & 0xfc) >> 2]); - - if (pos+1 < in_len) { - ret.push_back(base64_chars_[((bytes_to_encode[pos + 0] & 0x03) << 4) + ((bytes_to_encode[pos + 1] & 0xf0) >> 4)]); - - if (pos+2 < in_len) { - ret.push_back(base64_chars_[((bytes_to_encode[pos + 1] & 0x0f) << 2) + ((bytes_to_encode[pos + 2] & 0xc0) >> 6)]); - ret.push_back(base64_chars_[ bytes_to_encode[pos + 2] & 0x3f]); - } - else { - ret.push_back(base64_chars_[(bytes_to_encode[pos + 1] & 0x0f) << 2]); - ret.push_back(trailing_char); - } - } - else { - - ret.push_back(base64_chars_[(bytes_to_encode[pos + 0] & 0x03) << 4]); - ret.push_back(trailing_char); - ret.push_back(trailing_char); - } - - pos += 3; + for(i = 0; (i <4) ; i++) + ret += b64_chars[char_array_4[i]]; + i = 0; } + } + if (i) + { + for(j = i; j < 3; j++) + char_array_3[j] = '\0'; - return ret; -} - -template -static std::string decode(String const& encoded_string, bool remove_linebreaks) { - // - // decode(…) is templated so that it can be used with String = const std::string& - // or std::string_view (requires at least C++17) - // - - if (encoded_string.empty()) return std::string{}; - - if (remove_linebreaks) { + char_array_4[0] = (char_array_3[0] & 0xfc) >> 2; + char_array_4[1] = ((char_array_3[0] & 0x03) << 4) + ((char_array_3[1] & 0xf0) >> 4); + char_array_4[2] = ((char_array_3[1] & 0x0f) << 2) + ((char_array_3[2] & 0xc0) >> 6); + char_array_4[3] = char_array_3[2] & 0x3f; - std::string copy(encoded_string); + for (j = 0; (j < i + 1); j++) + ret += b64_chars[char_array_4[j]]; - copy.erase(std::remove(copy.begin(), copy.end(), '\n'), copy.end()); + while((i++ < 3)) + ret += '='; - return base64_decode(copy, false); - } + } - size_t length_of_string = encoded_string.length(); - size_t pos = 0; - - // - // The approximate length (bytes) of the decoded string might be one or - // two bytes smaller, depending on the amount of trailing equal signs - // in the encoded string. This approximation is needed to reserve - // enough space in the string to be returned. - // - size_t approx_length_of_decoded_string = length_of_string / 4 * 3; - std::string ret; - ret.reserve(approx_length_of_decoded_string); - - while (pos < length_of_string) { - // - // Iterate over encoded input string in chunks. The size of all - // chunks except the last one is 4 bytes. - // - // The last chunk might be padded with equal signs or dots - // in order to make it 4 bytes in size as well, but this - // is not required as per RFC 2045. - // - // All chunks except the last one produce three output bytes. - // - // The last chunk produces at least one and up to three bytes. - // - - size_t pos_of_char_1 = pos_of_char(encoded_string.at(pos+1) ); - - // - // Emit the first output byte that is produced in each chunk: - // - ret.push_back(static_cast( ( (pos_of_char(encoded_string.at(pos+0)) ) << 2 ) + ( (pos_of_char_1 & 0x30 ) >> 4))); - - if ( ( pos + 2 < length_of_string ) && // Check for data that is not padded with equal signs (which is allowed by RFC 2045) - encoded_string.at(pos+2) != '=' && - encoded_string.at(pos+2) != '.' // accept URL-safe base 64 strings, too, so check for '.' also. - ) - { - // - // Emit a chunk's second byte (which might not be produced in the last chunk). - // - unsigned int pos_of_char_2 = pos_of_char(encoded_string.at(pos+2) ); - ret.push_back(static_cast( (( pos_of_char_1 & 0x0f) << 4) + (( pos_of_char_2 & 0x3c) >> 2))); - - if ( ( pos + 3 < length_of_string ) && - encoded_string.at(pos+3) != '=' && - encoded_string.at(pos+3) != '.' - ) - { - // - // Emit a chunk's third byte (which might not be produced in the last chunk). - // - ret.push_back(static_cast( ( (pos_of_char_2 & 0x03 ) << 6 ) + pos_of_char(encoded_string.at(pos+3)) )); - } - } - - pos += 4; - } + return ret; - return ret; } -std::string base64_decode(std::string const& s, bool remove_linebreaks) { - return decode(s, remove_linebreaks); +std::string base64_encode(unsigned char const* bytes_to_encode, unsigned int in_len) { + return base64_encode_impl(bytes_to_encode, in_len, base64_chars); } -std::string base64_encode(std::string const& s, bool url) { - return encode(s, url); +std::string base64_encode( const std::string& enc ) { + char const* s = enc.c_str(); + return base64_encode( (unsigned char const*)s, enc.size() ); } -std::string base64_encode_pem (std::string const& s) { - return encode_pem(s); +std::string base64url_encode(unsigned char const* bytes_to_encode, unsigned int in_len) { + return base64_encode_impl(bytes_to_encode, in_len, base64url_chars); } -std::string base64_encode_mime(std::string const& s) { - return encode_mime(s); +std::string base64url_encode( const std::string& enc ) { + char const* s = enc.c_str(); + return base64url_encode( (unsigned char const*)s, enc.size() ); } -#if __cplusplus >= 201703L -// -// Interface with std::string_view rather than const std::string& -// Requires C++17 -// Provided by Yannic Bonenberger (https://github.com/Yannic) -// +std::string base64_decode_impl(std::string const& encoded_string, const char* const b64_chars) { + int in_len = encoded_string.size(); + int i = 0; + int j = 0; + int in_ = 0; + unsigned char char_array_4[4], char_array_3[3]; + std::string ret; -std::string base64_encode(std::string_view s, bool url) { - return encode(s, url); -} + while (in_len-- && encoded_string[in_] != '=') { + throw_on_nonbase64(encoded_string[in_], b64_chars); + char_array_4[i++] = encoded_string[in_]; in_++; + if (i ==4) { + for (i = 0; i <4; i++) + char_array_4[i] = strchr(b64_chars, char_array_4[i]) - b64_chars; -std::string base64_encode_pem(std::string_view s) { - return encode_pem(s); -} + char_array_3[0] = (char_array_4[0] << 2) + ((char_array_4[1] & 0x30) >> 4); + char_array_3[1] = ((char_array_4[1] & 0xf) << 4) + ((char_array_4[2] & 0x3c) >> 2); + char_array_3[2] = ((char_array_4[2] & 0x3) << 6) + char_array_4[3]; -std::string base64_encode_mime(std::string_view s) { - return encode_mime(s); -} + for (i = 0; (i < 3); i++) + ret += char_array_3[i]; + i = 0; + } + } -std::string base64_decode(std::string_view s, bool remove_linebreaks) { - return decode(s, remove_linebreaks); -} + if (i) { + for (j = i; j <4; j++) + char_array_4[j] = 0; -#endif // __cplusplus >= 201703L + for (j = 0; j <4; j++) + char_array_4[j] = strchr(b64_chars, char_array_4[j]) - b64_chars; -// end base64.cpp + char_array_3[0] = (char_array_4[0] << 2) + ((char_array_4[1] & 0x30) >> 4); + char_array_3[1] = ((char_array_4[1] & 0xf) << 4) + ((char_array_4[2] & 0x3c) >> 2); + char_array_3[2] = ((char_array_4[2] & 0x3) << 6) + char_array_4[3]; -// fc interface + for (j = 0; (j < i - 1); j++) ret += char_array_3[j]; + } -std::string base64_encode(unsigned char const* bytes_to_encode, unsigned int in_len) { - return base64_encode(bytes_to_encode, in_len, false); -} -std::string base64_encode(const std::string_view& enc) { - return base64_encode(enc, false); -} -std::string base64_decode(const std::string_view& encoded_string) { - return base64_decode(encoded_string, false); + return ret; } -std::string base64url_encode(unsigned char const* bytes_to_encode, unsigned int in_len) { - return base64_encode(bytes_to_encode, in_len, true); +std::string base64_decode(std::string const& encoded_string) { + return base64_decode_impl(encoded_string, base64_chars); } -std::string base64url_encode(const std::string_view& enc) { - return base64_encode(enc, true); -} -std::string base64url_decode(const std::string_view& encoded_string) { - return base64_decode(encoded_string, true); + +std::string base64url_decode(std::string const& encoded_string) { + return base64_decode_impl(encoded_string, base64url_chars); } } // namespace fc + diff --git a/libraries/libfc/test/test_base64.cpp b/libraries/libfc/test/test_base64.cpp index 38fb4e8cf8..ff6ac6a0ec 100644 --- a/libraries/libfc/test/test_base64.cpp +++ b/libraries/libfc/test/test_base64.cpp @@ -40,9 +40,7 @@ BOOST_AUTO_TEST_CASE(base64dec_extraequals) try { auto input = "YWJjMTIzJCYoKSc/tPUB+n5h========="s; auto expected_output = "abc123$&()'?\xb4\xf5\x01\xfa~a"s; - BOOST_CHECK_EXCEPTION(base64_decode(input), fc::exception, [](const fc::exception& e) { - return e.to_detail_string().find("encountered non-base64 character") != std::string::npos; - }); + BOOST_CHECK_EQUAL(expected_output, base64_decode(input)); } FC_LOG_AND_RETHROW(); BOOST_AUTO_TEST_CASE(base64dec_bad_stuff) try { @@ -53,134 +51,4 @@ BOOST_AUTO_TEST_CASE(base64dec_bad_stuff) try { }); } FC_LOG_AND_RETHROW(); -// tests from https://github.com/ReneNyffenegger/cpp-base64/blob/master/test.cpp -BOOST_AUTO_TEST_CASE(base64_cpp_base64_tests) try { - // - // Note: this file must be encoded in UTF-8 - // for the following test, otherwise, the test item - // fails. - // - const std::string orig = - "René Nyffenegger\n" - "http://www.renenyffenegger.ch\n" - "passion for data\n"; - - std::string encoded = base64_encode(reinterpret_cast(orig.c_str()), orig.length()); - std::string decoded = base64_decode(encoded); - - BOOST_CHECK_EQUAL(encoded, "UmVuw6kgTnlmZmVuZWdnZXIKaHR0cDovL3d3dy5yZW5lbnlmZmVuZWdnZXIuY2gKcGFzc2lvbiBmb3IgZGF0YQo="); - BOOST_CHECK_EQUAL(decoded, orig); - - // Test all possibilites of fill bytes (none, one =, two ==) - // References calculated with: https://www.base64encode.org/ - - std::string rest0_original = "abc"; - std::string rest0_reference = "YWJj"; - - std::string rest0_encoded = base64_encode(reinterpret_cast(rest0_original.c_str()), - rest0_original.length()); - std::string rest0_decoded = base64_decode(rest0_encoded); - - BOOST_CHECK_EQUAL(rest0_decoded, rest0_original); - BOOST_CHECK_EQUAL(rest0_reference, rest0_encoded); - - std::string rest1_original = "abcd"; - std::string rest1_reference = "YWJjZA=="; - - std::string rest1_encoded = base64_encode(reinterpret_cast(rest1_original.c_str()), - rest1_original.length()); - std::string rest1_decoded = base64_decode(rest1_encoded); - - BOOST_CHECK_EQUAL(rest1_decoded, rest1_original); - BOOST_CHECK_EQUAL(rest1_reference, rest1_encoded); - - std::string rest2_original = "abcde"; - std::string rest2_reference = "YWJjZGU="; - - std::string rest2_encoded = base64_encode(reinterpret_cast(rest2_original.c_str()), - rest2_original.length()); - std::string rest2_decoded = base64_decode(rest2_encoded); - - BOOST_CHECK_EQUAL(rest2_decoded, rest2_original); - BOOST_CHECK_EQUAL(rest2_reference, rest2_encoded); - - // -------------------------------------------------------------- - // - // Data that is 17 bytes long requires one padding byte when - // base-64 encoded. Such an encoded string could not correctly - // be decoded when encoded with «url semantics». This bug - // was discovered by https://github.com/kosniaz. The following - // test checks if this bug was fixed: - // - std::string a17_orig = "aaaaaaaaaaaaaaaaa"; - std::string a17_encoded = base64_encode(a17_orig); - std::string a17_encoded_url = base64url_encode(a17_orig); - - BOOST_CHECK_EQUAL(a17_encoded, "YWFhYWFhYWFhYWFhYWFhYWE="); - BOOST_CHECK_EQUAL(a17_encoded_url, "YWFhYWFhYWFhYWFhYWFhYWE."); - BOOST_CHECK_EQUAL(base64_decode(a17_encoded_url), a17_orig); - BOOST_CHECK_EQUAL(base64_decode(a17_encoded), a17_orig); - - // -------------------------------------------------------------- - - // characters 63 and 64 / URL encoding - - std::string s_6364 = "\x03" "\xef" "\xff" "\xf9"; - - std::string s_6364_encoded = base64_encode(s_6364); - std::string s_6364_encoded_url = base64url_encode(s_6364); - - BOOST_CHECK_EQUAL(s_6364_encoded, "A+//+Q=="); - BOOST_CHECK_EQUAL(s_6364_encoded_url, "A-__-Q.."); - BOOST_CHECK_EQUAL(base64_decode(s_6364_encoded), s_6364); - BOOST_CHECK_EQUAL(base64_decode(s_6364_encoded_url), s_6364); - - // ---------------------------------------------- - - std::string unpadded_input = "YWJjZGVmZw"; // Note the 'missing' "==" - std::string unpadded_decoded = base64_decode(unpadded_input); - BOOST_CHECK_EQUAL(unpadded_decoded, "abcdefg"); - - unpadded_input = "YWJjZGU"; // Note the 'missing' "=" - unpadded_decoded = base64_decode(unpadded_input); - BOOST_CHECK_EQUAL(unpadded_decoded, "abcde"); - - unpadded_input = ""; - unpadded_decoded = base64_decode(unpadded_input); - BOOST_CHECK_EQUAL(unpadded_decoded, ""); - - unpadded_input = "YQ"; - unpadded_decoded = base64_decode(unpadded_input); - BOOST_CHECK_EQUAL(unpadded_decoded, "a"); - - unpadded_input = "YWI"; - unpadded_decoded = base64_decode(unpadded_input); - BOOST_CHECK_EQUAL(unpadded_decoded, "ab"); - - // -------------------------------------------------------------- - // - // 2022-11-01 - // Replace - // encoded_string[…] with encoded_sring.at(…) - // in - // decode() - // - std::string not_null_terminated = std::string(1, 'a'); - BOOST_CHECK_THROW(base64_decode(not_null_terminated), std::out_of_range); - - // -------------------------------------------------------------- - // - // Test the string_view interface (which required C++17) - // - std::string_view sv_orig = "foobarbaz"; - std::string sv_encoded = base64_encode(sv_orig); - - BOOST_CHECK_EQUAL(sv_encoded, "Zm9vYmFyYmF6"); - - std::string sv_decoded = base64_decode(sv_encoded); - - BOOST_CHECK_EQUAL(sv_decoded, sv_orig); - -} FC_LOG_AND_RETHROW(); - BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file From b0c12963b23c664327cea56f9619dc3779c1e8d1 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 10 Nov 2023 07:14:54 -0600 Subject: [PATCH 56/65] Revert "GH-1461 Do not add or expect `=` at end of base64 encoded strings." This reverts commit 5c2f5077f5885db6f100a76ee9b17b72381526ef. --- libraries/libfc/src/variant.cpp | 7 +++---- libraries/libfc/test/variant/test_variant.cpp | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/libraries/libfc/src/variant.cpp b/libraries/libfc/src/variant.cpp index 56705ad18e..da1e648da0 100644 --- a/libraries/libfc/src/variant.cpp +++ b/libraries/libfc/src/variant.cpp @@ -490,7 +490,7 @@ std::string variant::as_string()const return *reinterpret_cast(this) ? "true" : "false"; case blob_type: if( get_blob().data.size() ) - return base64_encode( get_blob().data.data(), get_blob().data.size() ); + return base64_encode( get_blob().data.data(), get_blob().data.size() ) + "="; return std::string(); case null_type: return std::string(); @@ -533,11 +533,10 @@ blob variant::as_blob()const { const std::string& str = get_string(); if( str.size() == 0 ) return blob(); - try { + if( str.back() == '=' ) + { std::string b64 = base64_decode( get_string() ); return blob( { std::vector( b64.begin(), b64.end() ) } ); - } catch(const std::exception&) { - // unable to decode, just return the raw chars } return blob( { std::vector( str.begin(), str.end() ) } ); } diff --git a/libraries/libfc/test/variant/test_variant.cpp b/libraries/libfc/test/variant/test_variant.cpp index cfef4e50be..827b420ed0 100644 --- a/libraries/libfc/test/variant/test_variant.cpp +++ b/libraries/libfc/test/variant/test_variant.cpp @@ -88,7 +88,7 @@ BOOST_AUTO_TEST_CASE(variant_format_string_limited) const string target_result = format_prefix + a_short_list + " " + "{" + "\"b\":\"" + b_short_list + "\",\"c\":\"" + c_short_list + "\"}" + " " + "[\"" + d_short_list + "\",\"" + e_short_list + "\"]" + " " + - base64_encode( a_blob.data.data(), a_blob.data.size() ) + " " + + base64_encode( a_blob.data.data(), a_blob.data.size() ) + "=" + " " + g_short_list; BOOST_CHECK_EQUAL( result, target_result); From 481b936cf9de29e3a3ea59baad4503f239aaeb15 Mon Sep 17 00:00:00 2001 From: fcecin Date: Fri, 10 Nov 2023 13:36:04 -0300 Subject: [PATCH 57/65] Fix potential bug with proposal propagation --- libraries/hotstuff/qc_chain.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libraries/hotstuff/qc_chain.cpp b/libraries/hotstuff/qc_chain.cpp index 17e94e6485..92eda18f96 100644 --- a/libraries/hotstuff/qc_chain.cpp +++ b/libraries/hotstuff/qc_chain.cpp @@ -383,8 +383,7 @@ namespace eosio::hotstuff { update(proposal); //propagate this proposal since it was new to us - if (! am_i_leader()) - send_hs_proposal_msg(connection_id, proposal); + send_hs_proposal_msg(connection_id, proposal); for (auto &msg : msgs) { send_hs_vote_msg( std::nullopt, msg ); From 09e7fc2bd5a57d868e31022e646b207af4cdbc81 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 13 Nov 2023 08:11:54 -0600 Subject: [PATCH 58/65] GH-1461 Make WalletMgr honor --leave-running & --keep-logs --- tests/nodeos_run_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/nodeos_run_test.py b/tests/nodeos_run_test.py index 59b6fa68d9..8919ccce50 100755 --- a/tests/nodeos_run_test.py +++ b/tests/nodeos_run_test.py @@ -41,7 +41,7 @@ errFileName=f"{cluster.nodeosLogPath}/node_00/stderr.txt" if args.error_log_path: errFileName=args.error_log_path -walletMgr=WalletMgr(True, port=walletPort) +walletMgr=WalletMgr(True, port=walletPort, keepRunning=args.leave_running, keepLogs=args.keep_logs) testSuccessful=False dontBootstrap=sanityTest # intent is to limit the scope of the sanity test to just verifying that nodes can be started From 00b2f993f305ecb7510c927eb87c91bcdb730ac1 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 13 Nov 2023 08:13:36 -0600 Subject: [PATCH 59/65] GH-1461 Remove addition of invalid `=` character --- libraries/libfc/src/variant.cpp | 17 ++-- libraries/libfc/test/variant/test_variant.cpp | 78 ++++++++++++++++++- 2 files changed, 87 insertions(+), 8 deletions(-) diff --git a/libraries/libfc/src/variant.cpp b/libraries/libfc/src/variant.cpp index da1e648da0..b882a35ed9 100644 --- a/libraries/libfc/src/variant.cpp +++ b/libraries/libfc/src/variant.cpp @@ -490,7 +490,7 @@ std::string variant::as_string()const return *reinterpret_cast(this) ? "true" : "false"; case blob_type: if( get_blob().data.size() ) - return base64_encode( get_blob().data.data(), get_blob().data.size() ) + "="; + return base64_encode( get_blob().data.data(), get_blob().data.size() ); return std::string(); case null_type: return std::string(); @@ -533,10 +533,14 @@ blob variant::as_blob()const { const std::string& str = get_string(); if( str.size() == 0 ) return blob(); - if( str.back() == '=' ) - { - std::string b64 = base64_decode( get_string() ); - return blob( { std::vector( b64.begin(), b64.end() ) } ); + try { + // pre-5.0 versions of variant added `=` to end of base64 encoded string in as_string() above. + // fc version of base64_decode allows for extra `=` at the end of the string. + // Other base64 decoders will not accept the extra `=`. + std::vector b64 = base64_decode( str ); + return blob( { std::move(b64) } ); + } catch(const std::exception&) { + // unable to decode, return the raw chars } return blob( { std::vector( str.begin(), str.end() ) } ); } @@ -758,8 +762,7 @@ void to_variant( const blob& b, variant& v ) { } void from_variant( const variant& v, blob& b ) { - std::string _s = base64_decode(v.as_string()); - b.data = std::vector(_s.begin(), _s.end()); + b.data = base64_decode(v.as_string()); } void to_variant( const UInt<8>& n, variant& v ) { v = uint64_t(n); } diff --git a/libraries/libfc/test/variant/test_variant.cpp b/libraries/libfc/test/variant/test_variant.cpp index 827b420ed0..8be5d99232 100644 --- a/libraries/libfc/test/variant/test_variant.cpp +++ b/libraries/libfc/test/variant/test_variant.cpp @@ -88,11 +88,87 @@ BOOST_AUTO_TEST_CASE(variant_format_string_limited) const string target_result = format_prefix + a_short_list + " " + "{" + "\"b\":\"" + b_short_list + "\",\"c\":\"" + c_short_list + "\"}" + " " + "[\"" + d_short_list + "\",\"" + e_short_list + "\"]" + " " + - base64_encode( a_blob.data.data(), a_blob.data.size() ) + "=" + " " + + base64_encode( a_blob.data.data(), a_blob.data.size() ) + " " + g_short_list; BOOST_CHECK_EQUAL( result, target_result); BOOST_CHECK_LT(result.size(), 1024 + 3 * mu.size()); } } + +BOOST_AUTO_TEST_CASE(variant_blob) +{ + // Some test cases from https://github.com/ReneNyffenegger/cpp-base64 + { + std::string a17_orig = "aaaaaaaaaaaaaaaaa"; + std::string a17_encoded = "YWFhYWFhYWFhYWFhYWFhYWE="; + fc::mutable_variant_object mu; + mu("blob", blob{{a17_orig.begin(), a17_orig.end()}}); + mu("str", a17_encoded); + + BOOST_CHECK_EQUAL(mu["blob"].as_string(), a17_encoded); + std::vector b64 = mu["str"].as_blob().data; + std::string_view b64_str(b64.data(), b64.size()); + BOOST_CHECK_EQUAL(b64_str, a17_orig); + } + { + std::string s_6364 = "\x03" "\xef" "\xff" "\xf9"; + std::string s_6364_encoded = "A+//+Q=="; + fc::mutable_variant_object mu; + mu("blob", blob{{s_6364.begin(), s_6364.end()}}); + mu("str", s_6364_encoded); + + BOOST_CHECK_EQUAL(mu["blob"].as_string(), s_6364_encoded); + std::vector b64 = mu["str"].as_blob().data; + std::string_view b64_str(b64.data(), b64.size()); + BOOST_CHECK_EQUAL(b64_str, s_6364); + } + { + std::string org = "abc"; + std::string encoded = "YWJj"; + + fc::mutable_variant_object mu; + mu("blob", blob{{org.begin(), org.end()}}); + mu("str", encoded); + + BOOST_CHECK_EQUAL(mu["blob"].as_string(), encoded); + std::vector b64 = mu["str"].as_blob().data; + std::string_view b64_str(b64.data(), b64.size()); + BOOST_CHECK_EQUAL(b64_str, org); + } +} + +BOOST_AUTO_TEST_CASE(variant_blob_backwards_compatibility) +{ + // pre-5.0 variant would add an additional `=` as a flag that the blob data was base64 encoded + // verify variant can process encoded data with the extra `=` + { + std::string a17_orig = "aaaaaaaaaaaaaaaaa"; + std::string a17_encoded = "YWFhYWFhYWFhYWFhYWFhYWE="; + std::string a17_encoded_old = a17_encoded + '='; + fc::mutable_variant_object mu; + mu("blob", blob{{a17_orig.begin(), a17_orig.end()}}); + mu("str", a17_encoded_old); + + BOOST_CHECK_EQUAL(mu["blob"].as_string(), a17_encoded); + std::vector b64 = mu["str"].as_blob().data; + std::string_view b64_str(b64.data(), b64.size()); + BOOST_CHECK_EQUAL(b64_str, a17_orig); + } + { + std::string org = "abc"; + std::string encoded = "YWJj"; + std::string encoded_old = encoded + '='; + + fc::mutable_variant_object mu; + mu("blob", blob{{org.begin(), org.end()}}); + mu("str", encoded_old); + + BOOST_CHECK_EQUAL(mu["blob"].as_string(), encoded); + std::vector b64 = mu["str"].as_blob().data; + std::string_view b64_str(b64.data(), b64.size()); + BOOST_CHECK_EQUAL(b64_str, org); + } +} + BOOST_AUTO_TEST_SUITE_END() From 7b43a4a46e7d0dd890dec140b46263df19cdeca6 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 13 Nov 2023 08:14:23 -0600 Subject: [PATCH 60/65] GH-1461 Optimize base64_decode for our use cases - Use string_view and return vector instead of string --- .../chain/include/eosio/chain/database_utils.hpp | 4 ++-- libraries/libfc/include/fc/crypto/base64.hpp | 6 ++++-- libraries/libfc/src/crypto/base64.cpp | 13 +++++++------ libraries/libfc/src/crypto/bigint.cpp | 4 ++-- libraries/libfc/src/crypto/elliptic_webauthn.cpp | 2 +- libraries/libfc/test/test_base64.cpp | 12 +++++++++--- 6 files changed, 25 insertions(+), 16 deletions(-) diff --git a/libraries/chain/include/eosio/chain/database_utils.hpp b/libraries/chain/include/eosio/chain/database_utils.hpp index 0ee937e6e1..a3d6948080 100644 --- a/libraries/chain/include/eosio/chain/database_utils.hpp +++ b/libraries/chain/include/eosio/chain/database_utils.hpp @@ -182,8 +182,8 @@ namespace fc { inline void from_variant( const variant& v, eosio::chain::shared_blob& b ) { - std::string _s = base64_decode(v.as_string()); - b = eosio::chain::shared_blob(_s.begin(), _s.end(), b.get_allocator()); + std::vector b64 = base64_decode(v.as_string()); + b = eosio::chain::shared_blob(b64.begin(), b64.end(), b.get_allocator()); } template diff --git a/libraries/libfc/include/fc/crypto/base64.hpp b/libraries/libfc/include/fc/crypto/base64.hpp index 9559214df1..34dd35ad0b 100644 --- a/libraries/libfc/include/fc/crypto/base64.hpp +++ b/libraries/libfc/include/fc/crypto/base64.hpp @@ -1,14 +1,16 @@ #pragma once #include +#include +#include namespace fc { std::string base64_encode(unsigned char const* bytes_to_encode, unsigned int in_len); inline std::string base64_encode(char const* bytes_to_encode, unsigned int in_len) { return base64_encode( (unsigned char const*)bytes_to_encode, in_len); } std::string base64_encode( const std::string& enc ); -std::string base64_decode( const std::string& encoded_string); +std::vector base64_decode( std::string_view encoded_string); std::string base64url_encode(unsigned char const* bytes_to_encode, unsigned int in_len); inline std::string base64url_encode(char const* bytes_to_encode, unsigned int in_len) { return base64url_encode( (unsigned char const*)bytes_to_encode, in_len); } std::string base64url_encode( const std::string& enc ); -std::string base64url_decode( const std::string& encoded_string); +std::vector base64url_decode( std::string_view encoded_string); } // namespace fc diff --git a/libraries/libfc/src/crypto/base64.cpp b/libraries/libfc/src/crypto/base64.cpp index 5ff4e68290..ae6669ffdd 100644 --- a/libraries/libfc/src/crypto/base64.cpp +++ b/libraries/libfc/src/crypto/base64.cpp @@ -107,13 +107,14 @@ std::string base64url_encode( const std::string& enc ) { return base64url_encode( (unsigned char const*)s, enc.size() ); } -std::string base64_decode_impl(std::string const& encoded_string, const char* const b64_chars) { +std::vector base64_decode_impl(std::string_view encoded_string, const char* const b64_chars) { int in_len = encoded_string.size(); int i = 0; int j = 0; int in_ = 0; unsigned char char_array_4[4], char_array_3[3]; - std::string ret; + std::vector ret; + ret.reserve(in_len / 4 * 3); while (in_len-- && encoded_string[in_] != '=') { throw_on_nonbase64(encoded_string[in_], b64_chars); @@ -127,7 +128,7 @@ std::string base64_decode_impl(std::string const& encoded_string, const char* co char_array_3[2] = ((char_array_4[2] & 0x3) << 6) + char_array_4[3]; for (i = 0; (i < 3); i++) - ret += char_array_3[i]; + ret.push_back(char_array_3[i]); i = 0; } } @@ -143,17 +144,17 @@ std::string base64_decode_impl(std::string const& encoded_string, const char* co char_array_3[1] = ((char_array_4[1] & 0xf) << 4) + ((char_array_4[2] & 0x3c) >> 2); char_array_3[2] = ((char_array_4[2] & 0x3) << 6) + char_array_4[3]; - for (j = 0; (j < i - 1); j++) ret += char_array_3[j]; + for (j = 0; (j < i - 1); j++) ret.push_back(char_array_3[j]); } return ret; } -std::string base64_decode(std::string const& encoded_string) { +std::vector base64_decode(std::string_view encoded_string) { return base64_decode_impl(encoded_string, base64_chars); } -std::string base64url_decode(std::string const& encoded_string) { +std::vector base64url_decode(std::string_view encoded_string) { return base64_decode_impl(encoded_string, base64url_chars); } diff --git a/libraries/libfc/src/crypto/bigint.cpp b/libraries/libfc/src/crypto/bigint.cpp index 3e7ab67047..8574fa7022 100644 --- a/libraries/libfc/src/crypto/bigint.cpp +++ b/libraries/libfc/src/crypto/bigint.cpp @@ -221,8 +221,8 @@ namespace fc { else { std::string b64 = v.as_string(); - std::string bin = base64_decode(b64); - bi = bigint(bin.c_str(), bin.size() ); + std::vector bin = base64_decode(b64); + bi = bigint(bin.data(), bin.size() ); } } diff --git a/libraries/libfc/src/crypto/elliptic_webauthn.cpp b/libraries/libfc/src/crypto/elliptic_webauthn.cpp index ae08f5e5fc..de635b09b4 100644 --- a/libraries/libfc/src/crypto/elliptic_webauthn.cpp +++ b/libraries/libfc/src/crypto/elliptic_webauthn.cpp @@ -225,7 +225,7 @@ public_key::public_key(const signature& c, const fc::sha256& digest, bool) { FC_ASSERT(handler.found_type == "webauthn.get", "webauthn signature type not an assertion"); - std::string challenge_bytes = fc::base64url_decode(handler.found_challenge); + std::vector challenge_bytes = fc::base64url_decode(handler.found_challenge); FC_ASSERT(fc::sha256(challenge_bytes.data(), challenge_bytes.size()) == digest, "Wrong webauthn challenge"); char required_origin_scheme[] = "https://"; diff --git a/libraries/libfc/test/test_base64.cpp b/libraries/libfc/test/test_base64.cpp index ff6ac6a0ec..526d7c75d0 100644 --- a/libraries/libfc/test/test_base64.cpp +++ b/libraries/libfc/test/test_base64.cpp @@ -26,21 +26,27 @@ BOOST_AUTO_TEST_CASE(base64dec) try { auto input = "YWJjMTIzJCYoKSc/tPUB+n5h"s; auto expected_output = "abc123$&()'?\xb4\xf5\x01\xfa~a"s; - BOOST_CHECK_EQUAL(expected_output, base64_decode(input)); + std::vector b64 = base64_decode(input); + std::string b64_str(b64.begin(), b64.end()); + BOOST_CHECK_EQUAL(expected_output, b64_str); } FC_LOG_AND_RETHROW(); BOOST_AUTO_TEST_CASE(base64urldec) try { auto input = "YWJjMTIzJCYoKSc_tPUB-n5h"s; auto expected_output = "abc123$&()'?\xb4\xf5\x01\xfa~a"s; - BOOST_CHECK_EQUAL(expected_output, base64url_decode(input)); + std::vector b64 = base64url_decode(input); + std::string b64_str(b64.begin(), b64.end()); + BOOST_CHECK_EQUAL(expected_output, b64_str); } FC_LOG_AND_RETHROW(); BOOST_AUTO_TEST_CASE(base64dec_extraequals) try { auto input = "YWJjMTIzJCYoKSc/tPUB+n5h========="s; auto expected_output = "abc123$&()'?\xb4\xf5\x01\xfa~a"s; - BOOST_CHECK_EQUAL(expected_output, base64_decode(input)); + std::vector b64 = base64_decode(input); + std::string b64_str(b64.begin(), b64.end()); + BOOST_CHECK_EQUAL(expected_output, b64_str); } FC_LOG_AND_RETHROW(); BOOST_AUTO_TEST_CASE(base64dec_bad_stuff) try { From 3ccf0235e5ed0e0d10798fac9213f1c8b95b6614 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 13 Nov 2023 12:19:30 -0600 Subject: [PATCH 61/65] GH-1461 Removed unneeded explicit type --- libraries/libfc/src/variant.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/libfc/src/variant.cpp b/libraries/libfc/src/variant.cpp index b882a35ed9..326a507d08 100644 --- a/libraries/libfc/src/variant.cpp +++ b/libraries/libfc/src/variant.cpp @@ -538,7 +538,7 @@ blob variant::as_blob()const // fc version of base64_decode allows for extra `=` at the end of the string. // Other base64 decoders will not accept the extra `=`. std::vector b64 = base64_decode( str ); - return blob( { std::move(b64) } ); + return { std::move(b64) }; } catch(const std::exception&) { // unable to decode, return the raw chars } From 6e50f2dd2242a74a2a2dd102db898f39ef0cc87f Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Mon, 13 Nov 2023 16:50:22 -0500 Subject: [PATCH 62/65] Add dynamic check for pagemap support --- libraries/chainbase | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chainbase b/libraries/chainbase index 91a7398e60..fcf4e7ccb7 160000 --- a/libraries/chainbase +++ b/libraries/chainbase @@ -1 +1 @@ -Subproject commit 91a7398e60522ab7cc6433c0c56aa8daf297deac +Subproject commit fcf4e7ccb721dcadb1008376485c30db13118629 From f7c449f23fffa33d247a6808de935dabc1932e7a Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Mon, 13 Nov 2023 16:53:51 -0500 Subject: [PATCH 63/65] Update chainbase to tip of main branch --- libraries/chainbase | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/chainbase b/libraries/chainbase index fcf4e7ccb7..d3dba51a00 160000 --- a/libraries/chainbase +++ b/libraries/chainbase @@ -1 +1 @@ -Subproject commit fcf4e7ccb721dcadb1008376485c30db13118629 +Subproject commit d3dba51a0026f81643de40822a3fe5f01c7434a8 From b40b13f80b5e48ccc3cebadd5ef24372b4e0f27f Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 13 Nov 2023 16:54:41 -0600 Subject: [PATCH 64/65] GH-1461 Simplify tests --- libraries/libfc/test/test_base64.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/libraries/libfc/test/test_base64.cpp b/libraries/libfc/test/test_base64.cpp index 526d7c75d0..d6a59e628b 100644 --- a/libraries/libfc/test/test_base64.cpp +++ b/libraries/libfc/test/test_base64.cpp @@ -27,8 +27,7 @@ BOOST_AUTO_TEST_CASE(base64dec) try { auto expected_output = "abc123$&()'?\xb4\xf5\x01\xfa~a"s; std::vector b64 = base64_decode(input); - std::string b64_str(b64.begin(), b64.end()); - BOOST_CHECK_EQUAL(expected_output, b64_str); + BOOST_CHECK_EQUAL(expected_output, std::string_view(b64.begin(), b64.end())); } FC_LOG_AND_RETHROW(); BOOST_AUTO_TEST_CASE(base64urldec) try { @@ -36,8 +35,7 @@ BOOST_AUTO_TEST_CASE(base64urldec) try { auto expected_output = "abc123$&()'?\xb4\xf5\x01\xfa~a"s; std::vector b64 = base64url_decode(input); - std::string b64_str(b64.begin(), b64.end()); - BOOST_CHECK_EQUAL(expected_output, b64_str); + BOOST_CHECK_EQUAL(expected_output, std::string_view(b64.begin(), b64.end())); } FC_LOG_AND_RETHROW(); BOOST_AUTO_TEST_CASE(base64dec_extraequals) try { @@ -45,8 +43,7 @@ BOOST_AUTO_TEST_CASE(base64dec_extraequals) try { auto expected_output = "abc123$&()'?\xb4\xf5\x01\xfa~a"s; std::vector b64 = base64_decode(input); - std::string b64_str(b64.begin(), b64.end()); - BOOST_CHECK_EQUAL(expected_output, b64_str); + BOOST_CHECK_EQUAL(expected_output, std::string_view(b64.begin(), b64.end())); } FC_LOG_AND_RETHROW(); BOOST_AUTO_TEST_CASE(base64dec_bad_stuff) try { From 7b22e57e623d07e15946ff7b36cd80e04f332c95 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 17 Nov 2023 09:51:10 -0600 Subject: [PATCH 65/65] GH-1523 add local variable hs_active to make code clearer --- libraries/chain/controller.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index f65ad54d60..1351bce3aa 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1713,8 +1713,9 @@ struct controller_impl { { EOS_ASSERT( !pending, block_validate_exception, "pending block already exists" ); - // can change during start_block, so use same value throughout; although the transition from 0 to >0 cannot happen during start_block + // can change during start_block, so use same value throughout uint32_t hs_lib = hs_irreversible_block_num.load(); + const bool hs_active = hs_lib > 0; // the transition from 0 to >0 cannot happen during start_block emit( self.block_start, head->block_num + 1 ); @@ -1733,9 +1734,9 @@ struct controller_impl { EOS_ASSERT( db.revision() == head->block_num, database_exception, "db revision is not on par with head block", ("db.revision()", db.revision())("controller_head_block", head->block_num)("fork_db_head_block", fork_db.head()->block_num) ); - pending.emplace( maybe_session(db), *head, when, hs_lib > 0, confirm_block_count, new_protocol_feature_activations ); + pending.emplace( maybe_session(db), *head, when, hs_active, confirm_block_count, new_protocol_feature_activations ); } else { - pending.emplace( maybe_session(), *head, when, hs_lib > 0, confirm_block_count, new_protocol_feature_activations ); + pending.emplace( maybe_session(), *head, when, hs_active, confirm_block_count, new_protocol_feature_activations ); } pending->_block_status = s; @@ -1817,7 +1818,7 @@ struct controller_impl { const auto& gpo = self.get_global_properties(); if( gpo.proposed_schedule_block_num && // if there is a proposed schedule that was proposed in a block ... - ( hs_lib > 0 || *gpo.proposed_schedule_block_num <= pbhs.dpos_irreversible_blocknum ) && // ... that has now become irreversible or hotstuff activated... + ( hs_active || *gpo.proposed_schedule_block_num <= pbhs.dpos_irreversible_blocknum ) && // ... that has now become irreversible or hotstuff activated... pbhs.prev_pending_schedule.schedule.producers.size() == 0 // ... and there was room for a new pending schedule prior to any possible promotion ) { @@ -1830,7 +1831,7 @@ struct controller_impl { if( !replaying ) { ilog( "promoting proposed schedule (set in block ${proposed_num}) to pending; current block: ${n} lib: ${lib} schedule: ${schedule} ", ("proposed_num", *gpo.proposed_schedule_block_num)("n", pbhs.block_num) - ("lib", hs_lib > 0 ? hs_lib : pbhs.dpos_irreversible_blocknum) + ("lib", hs_active ? hs_lib : pbhs.dpos_irreversible_blocknum) ("schedule", std::get(pending->_block_stage)._new_pending_producer_schedule ) ); }