From b4dc733a732ac1259b5c8cdd9b5fa07b4995c214 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Fri, 13 Sep 2024 16:07:59 -0400 Subject: [PATCH 01/49] add block_header_extension types to ship abi --- libraries/state_history/abi.cpp | 59 ++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/libraries/state_history/abi.cpp b/libraries/state_history/abi.cpp index 095b150886..fb5cf7f3fe 100644 --- a/libraries/state_history/abi.cpp +++ b/libraries/state_history/abi.cpp @@ -588,6 +588,13 @@ extern const char* const state_history_plugin_abi = R"({ { "type": "uint32", "name": "account_net_usage_average_window" } ] }, + { + "name": "finalizer_authority", "fields": [ + { "name": "description", "type": "string" }, + { "name": "weight", "type": "uint64" }, + { "name": "public_key", "type": "bytes" } + ] + }, { "name": "finalizer_authority_with_string_key", "fields": [ { "name": "description", "type": "string" }, @@ -616,6 +623,55 @@ extern const char* const state_history_plugin_abi = R"({ { "name": "pending_finalizer_policy", "type": "finalizer_policy_with_string_key?" }, { "name": "last_pending_finalizer_policy_generation", "type": "uint32" } ] + }, + { + "name": "protocol_feature_activation_extension", "fields": [ + { "name": "protocol_features", "type": "checksum256[]" } + ] + }, + { + "name": "producer_schedule_change_extension", "base": "producer_authority_schedule", "fields": [] + }, + { + "name": "qc_claim", "fields": [ + { "name": "block_num", "type": "uint32" }, + { "name": "is_strong_qc", "type": "bool" } + ] + }, + { + "name": "insert_finalizer_policy_index_pair", "fields": [ + { "name": "index", "type": "uint16" }, + { "name": "value", "type": "finalizer_authority" } + ] + }, + { + "name": "finalizer_policy_diff", "fields": [ + { "name": "generation", "type": "uint32" }, + { "name": "threshold", "type": "uint64" }, + { "name": "remove_indexes", "type": "uint16[]" }, + { "name": "insert_indexes", "type": "insert_finalizer_policy_index_pair[]" } + ] + }, + { + "name": "insert_proposer_policy_index_pair", "fields": [ + { "name": "index", "type": "uint16" }, + { "name": "value", "type": "producer_authority" } + ] + }, + { + "name": "proposer_policy_diff", "fields": [ + { "name": "version", "type": "uint32" }, + { "name": "proposal_time", "type": "block_timestamp_type" }, + { "name": "remove_indexes", "type": "uint16[]" }, + { "name": "insert_indexes", "type": "insert_proposer_policy_index_pair[]" } + ] + }, + { + "name": "finality_extension", "fields": [ + { "name": "qc_claim", "type": "qc_claim" }, + { "name": "new_finalizer_policy_diff", "type": "finalizer_policy_diff?" }, + { "name": "new_proposer_policy_diff", "type": "proposer_policy_diff?" } + ] } ], "types": [ @@ -657,7 +713,8 @@ extern const char* const state_history_plugin_abi = R"({ { "name": "resource_limits_ratio", "types": ["resource_limits_ratio_v0"] }, { "name": "elastic_limit_parameters", "types": ["elastic_limit_parameters_v0"] }, { "name": "resource_limits_config", "types": ["resource_limits_config_v0"] }, - { "name": "block_signing_authority", "types": ["block_signing_authority_v0"] } + { "name": "block_signing_authority", "types": ["block_signing_authority_v0"] }, + { "name": "block_header_extension", "types": ["protocol_feature_activation_extension", "producer_schedule_change_extension", "finality_extension"] } ], "tables": [ { "name": "account", "type": "account", "key_names": ["name"] }, From ca100fb2d3c32d957e8431672d2b32cbf0527e4a Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Sat, 14 Sep 2024 21:37:58 -0400 Subject: [PATCH 02/49] Add read_serialized_block_by_num() and read_serialized_block_by_id() --- libraries/chain/block_log.cpp | 55 +++++++++++++++++++ .../chain/include/eosio/chain/block_log.hpp | 7 ++- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index b83d7599cc..11d51d2448 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -485,6 +485,7 @@ namespace eosio { namespace chain { virtual void flush() = 0; virtual signed_block_ptr read_block_by_num(uint32_t block_num) = 0; + virtual std::vector read_serialized_block_by_num(uint32_t block_num) = 0; virtual std::optional read_block_header_by_num(uint32_t block_num) = 0; virtual uint32_t version() const = 0; @@ -518,6 +519,7 @@ namespace eosio { namespace chain { void flush() final {} signed_block_ptr read_block_by_num(uint32_t block_num) final { return {}; }; + std::vector read_serialized_block_by_num(uint32_t block_num) final { return {}; }; std::optional read_block_header_by_num(uint32_t block_num) final { return {}; }; uint32_t version() const final { return 0; } @@ -615,6 +617,54 @@ namespace eosio { namespace chain { FC_LOG_AND_RETHROW() } + std::vector read_serialized_block_by_num(uint32_t block_num) final { + try { + uint64_t pos = get_block_pos(block_num); + + if (pos == block_log::npos || !head) { + // Rare case. No need to write special code for it. + // Just use the regular retry_read_block_by_num and then serialize. + return fc::raw::pack(*(retry_read_block_by_num(block_num))); + } + + uint64_t block_size = 0; + uint32_t head_block_num = block_header::num_from_id(head->id); + + if (block_num < head_block_num) { + // current block is not the last block in the log file. + uint64_t next_block_pos = get_block_pos(block_num + 1); + if (next_block_pos == block_log::npos) { + wlog("no position found for block ${n}", ("n", block_num + 1)); + return {}; + } + if (next_block_pos <= pos) { + wlog("next block position ${nn} should be greater than current block position ${cn}", ("nn", next_block_pos)("cn", pos)); + return {}; + } + + block_size = next_block_pos - pos - 8; + } else if (block_num == head_block_num) { + // current block is the last block in the file. + auto log_size = std::filesystem::file_size(block_file.get_file_path()); + block_size = log_size - pos - 8; + } else { + wlog("read_serialized_block_by_num block_num LHUANG_ERR > head_block_num"); + EOS_ASSERT(false, block_log_exception, + "Current block_num ${n} was greater than head_block_num ${h}", ("n", block_num)("h", head_block_num)); + return {}; + } + + std::vector buff; + buff.resize(block_size); + + block_file.seek(pos); + block_file.read(buff.data(), block_size); + + return buff; + } + FC_LOG_AND_RETHROW() + } + std::optional read_block_header_by_num(uint32_t block_num) final { try { uint64_t pos = get_block_pos(block_num); @@ -1230,6 +1280,11 @@ namespace eosio { namespace chain { return my->read_block_by_num(block_num); } + std::vector block_log::read_serialized_block_by_num(uint32_t block_num) const { + std::lock_guard g(my->mtx); + return my->read_serialized_block_by_num(block_num); + } + std::optional block_log::read_block_header_by_num(uint32_t block_num) const { std::lock_guard g(my->mtx); return my->read_block_header_by_num(block_num); diff --git a/libraries/chain/include/eosio/chain/block_log.hpp b/libraries/chain/include/eosio/chain/block_log.hpp index be539a5ddd..0bdec0fba4 100644 --- a/libraries/chain/include/eosio/chain/block_log.hpp +++ b/libraries/chain/include/eosio/chain/block_log.hpp @@ -53,7 +53,8 @@ namespace eosio { namespace chain { void reset( const genesis_state& gs, const signed_block_ptr& genesis_block ); void reset( const chain_id_type& chain_id, uint32_t first_block_num ); - signed_block_ptr read_block_by_num(uint32_t block_num)const; + signed_block_ptr read_block_by_num(uint32_t block_num)const; + std::vector read_serialized_block_by_num(uint32_t block_num)const; std::optional read_block_header_by_num(uint32_t block_num)const; std::optional read_block_id_by_num(uint32_t block_num)const; @@ -61,6 +62,10 @@ namespace eosio { namespace chain { return read_block_by_num(block_header::num_from_id(id)); } + std::vector read_serialized_block_by_id(const block_id_type& id)const { + return read_serialized_block_by_num(block_header::num_from_id(id)); + } + /** * Return offset of block in file, or block_log::npos if it does not exist. */ From d1ef5cfc4d4e9ccf6567abc7467bdddcd1daefee Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Sun, 15 Sep 2024 10:33:36 -0400 Subject: [PATCH 03/49] Add fetch_serialized_block_by_number() and fetch_serialized_block_by_id() to controller --- libraries/chain/controller.cpp | 15 +++++++++++++++ .../chain/include/eosio/chain/controller.hpp | 5 +++++ 2 files changed, 20 insertions(+) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 1cb4f7eb81..18d45a108b 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -5513,6 +5513,13 @@ signed_block_ptr controller::fetch_block_by_id( const block_id_type& id )const { return signed_block_ptr(); } +std::vector controller::fetch_serialized_block_by_id( const block_id_type& id )const { try { + auto sb_ptr = my->fork_db_fetch_block_by_id(id); + if( sb_ptr ) return fc::raw::pack(*sb_ptr); + + return my->blog.read_serialized_block_by_id(id); +} FC_CAPTURE_AND_RETHROW( (id) ) } + bool controller::block_exists(const block_id_type& id) const { bool exists = my->fork_db_block_exists(id); if( exists ) return true; @@ -5542,6 +5549,14 @@ signed_block_ptr controller::fetch_block_by_number( uint32_t block_num )const { return my->blog.read_block_by_num(block_num); } FC_CAPTURE_AND_RETHROW( (block_num) ) } +vector controller::fetch_serialized_block_by_number( uint32_t block_num )const { try { + if (signed_block_ptr b = my->fork_db_fetch_block_on_best_branch_by_num(block_num)) { + return fc::raw::pack(*b); + } + + return my->blog.read_serialized_block_by_num(block_num); +} FC_CAPTURE_AND_RETHROW( (block_num) ) } + std::optional controller::fetch_block_header_by_number( uint32_t block_num )const { try { auto b = my->fork_db_fetch_block_on_best_branch_by_num(block_num); if (b) diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index 599800e753..0025220b3c 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -320,6 +320,11 @@ namespace eosio::chain { signed_block_ptr fetch_block_by_number( uint32_t block_num )const; // thread-safe signed_block_ptr fetch_block_by_id( const block_id_type& id )const; + // thread-safe, retrieves signed block in serialized form, used by net_plugin + // to avoid deserialization and serialization + vector fetch_serialized_block_by_number( uint32_t block_num )const; + // thread-safe + vector fetch_serialized_block_by_id( const block_id_type& id )const; // thread-safe bool block_exists(const block_id_type& id) const; bool validated_block_exists(const block_id_type& id) const; From e65a9b62a7729e09934b5658194ec87be25e5f5a Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Sun, 15 Sep 2024 14:14:06 -0400 Subject: [PATCH 04/49] Update net_plugin for changes required to handle serialized blocks --- plugins/net_plugin/net_plugin.cpp | 51 +++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 62e111ecfc..7b9d6e7dde 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -1071,7 +1071,7 @@ namespace eosio { void blk_send(const block_id_type& blkid); void enqueue( const net_message &msg ); - size_t enqueue_block( const signed_block_ptr& sb, bool to_sync_queue = false); + size_t enqueue_block( const std::vector& sb, bool to_sync_queue = false); void enqueue_buffer( const std::shared_ptr>& send_buffer, go_away_reason close_after_send, bool to_sync_queue = false); @@ -1572,16 +1572,16 @@ namespace eosio { void connection::blk_send( const block_id_type& blkid ) { try { controller& cc = my_impl->chain_plug->chain(); - signed_block_ptr b = cc.fetch_block_by_id( blkid ); // thread-safe - if( b ) { - peer_dlog( this, "fetch_block_by_id num ${n}", ("n", b->block_num()) ); + std::vector b = cc.fetch_serialized_block_by_id( blkid ); // thread-safe + if( !b.empty() ) { + peer_dlog( this, "fetch_serialized_block_by_id num ${n}", ("n", block_header::num_from_id(blkid)) ); enqueue_block( b ); } else { peer_ilog( this, "fetch block by id returned null, id ${id}", ("id", blkid) ); } } catch( const assert_exception& ex ) { // possible corrupted block log - peer_elog( this, "caught assert on fetch_block_by_id, ${ex}, id ${id}", ("ex", ex.to_string())("id", blkid) ); + peer_elog( this, "caught assert on fetch_serialized_block_by_id, ${ex}, id ${id}", ("ex", ex.to_string())("id", blkid) ); } catch( ... ) { peer_elog( this, "caught other exception fetching block id ${id}", ("id", blkid) ); } @@ -1754,11 +1754,11 @@ namespace eosio { uint32_t num = peer_requested->last + 1; controller& cc = my_impl->chain_plug->chain(); - signed_block_ptr sb; + std::vector sb; try { - sb = cc.fetch_block_by_number( num ); // thread-safe + sb = cc.fetch_serialized_block_by_number( num ); // thread-safe } FC_LOG_AND_DROP(); - if( sb ) { + if( !sb.empty() ) { // Skip transmitting block this loop if threshold exceeded if (block_sync_send_start == 0ns) { // start of enqueue blocks block_sync_send_start = get_time(); @@ -1846,6 +1846,25 @@ namespace eosio { return send_buffer; } + static send_buffer_type create_send_buffer_from_serialized_block( const std::vector& v ) { + static_assert( signed_block_which == fc::get_index() ); + + // match net_message static_variant pack + const uint32_t which_size = fc::raw::pack_size( unsigned_int( signed_block_which ) ); + const uint32_t payload_size = which_size + v.size(); + + const char* const header = reinterpret_cast(&payload_size); // avoid variable size encoding of uint32_t + const size_t buffer_size = message_header_size + payload_size; + + auto send_buffer = std::make_shared>( buffer_size ); + fc::datastream ds( send_buffer->data(), buffer_size ); + ds.write( header, message_header_size ); + fc::raw::pack( ds, unsigned_int( signed_block_which ) ); + ds.write( v.data(), v.size() ); + + return send_buffer; + } + }; struct block_buffer_factory : public buffer_factory { @@ -1858,6 +1877,13 @@ namespace eosio { return send_buffer; } + const send_buffer_type& get_send_buffer( const std::vector& sb ) { + if( !send_buffer ) { + send_buffer = create_send_buffer( sb ); + } + return send_buffer; + } + private: static std::shared_ptr> create_send_buffer( const signed_block_ptr& sb ) { @@ -1867,6 +1893,12 @@ namespace eosio { fc_dlog( logger, "sending block ${bn}", ("bn", sb->block_num()) ); return buffer_factory::create_send_buffer( signed_block_which, *sb ); } + + static std::shared_ptr> create_send_buffer( const std::vector& ssb ) { // ssb: serialized signed block + // this implementation is to avoid copy of signed_block to net_message + // matches which of net_message for signed_block + return buffer_factory::create_send_buffer_from_serialized_block( ssb ); + } }; struct trx_buffer_factory : public buffer_factory { @@ -1905,8 +1937,7 @@ namespace eosio { } // called from connection strand - size_t connection::enqueue_block( const signed_block_ptr& b, bool to_sync_queue) { - peer_dlog( this, "enqueue block ${num}", ("num", b->block_num()) ); + size_t connection::enqueue_block( const std::vector& b, bool to_sync_queue) { verify_strand_in_this_thread( strand, __func__, __LINE__ ); block_buffer_factory buff_factory; From 81ef71843c7a7970f8b728a0167246979a06da6c Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Mon, 16 Sep 2024 11:21:35 -0400 Subject: [PATCH 05/49] Verify block number or block ID before returning serialized signed block --- libraries/chain/block_log.cpp | 76 ++++++++++++++----- .../chain/include/eosio/chain/block_log.hpp | 5 +- 2 files changed, 59 insertions(+), 22 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 11d51d2448..008db878ae 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -197,6 +197,23 @@ namespace eosio { namespace chain { return bh; } + uint32_t get_block_num_at(fc::datastream& file, uint64_t position) { + // to derive blknum_offset==14 see block_header.hpp and note on disk struct is packed + // block_timestamp_type timestamp; //bytes 0:3 + // account_name producer; //bytes 4:11 + // uint16_t confirmed; //bytes 12:13 + // block_id_type previous; //bytes 14:45, low 4 bytes is big endian block number + // of previous block + + file.seek_end(0); + EOS_ASSERT(position <= file.tellp(), block_log_exception, "Invalid block position ${position}", + ("position", position)); + + int blknum_offset = 14; + uint32_t prev_block_num = read_data_at(file, position + blknum_offset); + return fc::endian_reverse_u32(prev_block_num) + 1; + } + /// Provide the read only view of the blocks.log file class block_log_data : public chain::log_data_base { block_log_preamble preamble; @@ -238,19 +255,7 @@ namespace eosio { namespace chain { } uint32_t block_num_at(uint64_t position) { - // to derive blknum_offset==14 see block_header.hpp and note on disk struct is packed - // block_timestamp_type timestamp; //bytes 0:3 - // account_name producer; //bytes 4:11 - // uint16_t confirmed; //bytes 12:13 - // block_id_type previous; //bytes 14:45, low 4 bytes is big endian block number - // of previous block - - EOS_ASSERT(position <= size(), block_log_exception, "Invalid block position ${position}", - ("position", position)); - - int blknum_offset = 14; - uint32_t prev_block_num = read_data_at(file, position + blknum_offset); - return fc::endian_reverse_u32(prev_block_num) + 1; + return get_block_num_at(file, position); } auto& ro_stream_at(uint64_t pos) { @@ -474,6 +479,12 @@ namespace eosio { namespace chain { }; std::optional head; + enum class verify_block_num_t { + yes, // Verify block number when read a serialized block. + no // Does not verify block number when read a serialized block. + // This is for the case where block id is vefified seperately. + }; + virtual ~block_log_impl() = default; virtual uint32_t first_block_num() = 0; @@ -485,7 +496,7 @@ namespace eosio { namespace chain { virtual void flush() = 0; virtual signed_block_ptr read_block_by_num(uint32_t block_num) = 0; - virtual std::vector read_serialized_block_by_num(uint32_t block_num) = 0; + virtual std::vector read_serialized_block_by_num(uint32_t block_num, verify_block_num_t verify_block_num) = 0; virtual std::optional read_block_header_by_num(uint32_t block_num) = 0; virtual uint32_t version() const = 0; @@ -519,7 +530,7 @@ namespace eosio { namespace chain { void flush() final {} signed_block_ptr read_block_by_num(uint32_t block_num) final { return {}; }; - std::vector read_serialized_block_by_num(uint32_t block_num) final { return {}; }; + std::vector read_serialized_block_by_num(uint32_t block_num, verify_block_num_t verify_block_num) final { return {}; }; std::optional read_block_header_by_num(uint32_t block_num) final { return {}; }; uint32_t version() const final { return 0; } @@ -617,7 +628,7 @@ namespace eosio { namespace chain { FC_LOG_AND_RETHROW() } - std::vector read_serialized_block_by_num(uint32_t block_num) final { + std::vector read_serialized_block_by_num(uint32_t block_num, verify_block_num_t verify_block_num) final { try { uint64_t pos = get_block_pos(block_num); @@ -627,6 +638,15 @@ namespace eosio { namespace chain { return fc::raw::pack(*(retry_read_block_by_num(block_num))); } + if (verify_block_num == verify_block_num_t::yes) { + auto bnum = get_block_num_at(block_file, pos); + if (block_num != bnum) { + wlog("Wrong block num was read from block log. expected block_num: ${n}, returned block num: ${bnum}", + ("n", block_num)("bnum", bnum)); + return {}; + } + } + uint64_t block_size = 0; uint32_t head_block_num = block_header::num_from_id(head->id); @@ -648,7 +668,6 @@ namespace eosio { namespace chain { auto log_size = std::filesystem::file_size(block_file.get_file_path()); block_size = log_size - pos - 8; } else { - wlog("read_serialized_block_by_num block_num LHUANG_ERR > head_block_num"); EOS_ASSERT(false, block_log_exception, "Current block_num ${n} was greater than head_block_num ${h}", ("n", block_num)("h", head_block_num)); return {}; @@ -1282,7 +1301,28 @@ namespace eosio { namespace chain { std::vector block_log::read_serialized_block_by_num(uint32_t block_num) const { std::lock_guard g(my->mtx); - return my->read_serialized_block_by_num(block_num); + return my->read_serialized_block_by_num(block_num, detail::block_log_impl::verify_block_num_t::yes); + } + + std::vector block_log::read_serialized_block_by_id(const block_id_type& id) const { + std::lock_guard g(my->mtx); + + auto block_num = block_header::num_from_id(id); + + std::optional h = my->read_block_header_by_num(block_num); + if (h) { + auto returd_id = h->calculate_id(); + if (returd_id != id) { + wlog("wrong block id was read from block log, expected id: ${id}, returned id: ${rid}", + ("id", id)("rid", returd_id)); + return {}; + } + } else { + wlog("no header found for block id: ${id}, block_num: ${n}", ("id", id)("n", block_num)); + return {}; + } + + return my->read_serialized_block_by_num(block_num, detail::block_log_impl::verify_block_num_t::no); } std::optional block_log::read_block_header_by_num(uint32_t block_num) const { diff --git a/libraries/chain/include/eosio/chain/block_log.hpp b/libraries/chain/include/eosio/chain/block_log.hpp index 0bdec0fba4..e5cd3ae78a 100644 --- a/libraries/chain/include/eosio/chain/block_log.hpp +++ b/libraries/chain/include/eosio/chain/block_log.hpp @@ -55,6 +55,7 @@ namespace eosio { namespace chain { signed_block_ptr read_block_by_num(uint32_t block_num)const; std::vector read_serialized_block_by_num(uint32_t block_num)const; + std::vector read_serialized_block_by_id(const block_id_type& id)const; std::optional read_block_header_by_num(uint32_t block_num)const; std::optional read_block_id_by_num(uint32_t block_num)const; @@ -62,10 +63,6 @@ namespace eosio { namespace chain { return read_block_by_num(block_header::num_from_id(id)); } - std::vector read_serialized_block_by_id(const block_id_type& id)const { - return read_serialized_block_by_num(block_header::num_from_id(id)); - } - /** * Return offset of block in file, or block_log::npos if it does not exist. */ From 907dcdc1225273523daf6a1a48e80fcb57a45be4 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Mon, 16 Sep 2024 11:31:19 -0400 Subject: [PATCH 06/49] Do not hardcode size of block position field in block log file --- libraries/chain/block_log.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 008db878ae..1901573523 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -633,8 +633,8 @@ namespace eosio { namespace chain { uint64_t pos = get_block_pos(block_num); if (pos == block_log::npos || !head) { - // Rare case. No need to write special code for it. - // Just use the regular retry_read_block_by_num and then serialize. + // Rare case. No need a special code for it. + // Just fall back to the regular `retry_read_block_by_num` and then serialize. return fc::raw::pack(*(retry_read_block_by_num(block_num))); } @@ -649,6 +649,7 @@ namespace eosio { namespace chain { uint64_t block_size = 0; uint32_t head_block_num = block_header::num_from_id(head->id); + constexpr uint32_t block_pos_size = sizeof(uint64_t); // size of block position field in the block log file if (block_num < head_block_num) { // current block is not the last block in the log file. @@ -662,11 +663,11 @@ namespace eosio { namespace chain { return {}; } - block_size = next_block_pos - pos - 8; + block_size = next_block_pos - pos - block_pos_size; } else if (block_num == head_block_num) { // current block is the last block in the file. auto log_size = std::filesystem::file_size(block_file.get_file_path()); - block_size = log_size - pos - 8; + block_size = log_size - pos - block_pos_size; } else { EOS_ASSERT(false, block_log_exception, "Current block_num ${n} was greater than head_block_num ${h}", ("n", block_num)("h", head_block_num)); From 5211dea7638e61f3f9d0d4b3a6fd5a92eb3c412b Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Mon, 16 Sep 2024 15:12:02 -0400 Subject: [PATCH 07/49] Use the return of retry_read_block_by_num only if it is not nullptr --- libraries/chain/block_log.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 1901573523..3cb573b70e 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -635,7 +635,12 @@ namespace eosio { namespace chain { if (pos == block_log::npos || !head) { // Rare case. No need a special code for it. // Just fall back to the regular `retry_read_block_by_num` and then serialize. - return fc::raw::pack(*(retry_read_block_by_num(block_num))); + auto ret = retry_read_block_by_num(block_num); + if (ret) { + return fc::raw::pack(*ret); + } else { + return {}; + } } if (verify_block_num == verify_block_num_t::yes) { From 308fe2567153632c7e5dc3738df7baf2d8063dd2 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Tue, 17 Sep 2024 12:23:11 -0400 Subject: [PATCH 08/49] bump abieos submod to branch w/ ship updates --- tests/abieos | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/abieos b/tests/abieos index 5bd9a54824..ae78105e01 160000 --- a/tests/abieos +++ b/tests/abieos @@ -1 +1 @@ -Subproject commit 5bd9a5482411e1a3054792f4210ce2bebe29d9a5 +Subproject commit ae78105e015dae0dc10466018f8d7c3eae455285 From 3dc089683ea5e8ff010759e71a59ca503e5da756 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Tue, 17 Sep 2024 13:41:18 -0400 Subject: [PATCH 09/49] bump abieos submod to branch w/ ship updates & gcc fix --- tests/abieos | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/abieos b/tests/abieos index ae78105e01..d2586071be 160000 --- a/tests/abieos +++ b/tests/abieos @@ -1 +1 @@ -Subproject commit ae78105e015dae0dc10466018f8d7c3eae455285 +Subproject commit d2586071be1b3383ac6c38a8beec91a0901ea458 From b997d303aedd98238d520902cfd1786d1d19cd59 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Wed, 18 Sep 2024 13:15:26 -0400 Subject: [PATCH 10/49] Make EOS_ASSERT in get_block_num_at stricter --- libraries/chain/block_log.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 3cb573b70e..b2b8a0c5a9 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -206,10 +206,12 @@ namespace eosio { namespace chain { // of previous block file.seek_end(0); - EOS_ASSERT(position <= file.tellp(), block_log_exception, "Invalid block position ${position}", - ("position", position)); + int blknum_offset = 14; + + EOS_ASSERT(position + blknum_offset + sizeof(uint32_t) <= file.tellp(), block_log_exception, + "Read outside of file: position ${position}, blknum_offset ${o}, file size ${s}", + ("position", position)("o", blknum_offset)("s", file.tellp())); - int blknum_offset = 14; uint32_t prev_block_num = read_data_at(file, position + blknum_offset); return fc::endian_reverse_u32(prev_block_num) + 1; } From 0d0e55bacdbdf1faf08b9eb284ab8a3a873c8a68 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Wed, 18 Sep 2024 13:22:12 -0400 Subject: [PATCH 11/49] Improve comments and rename a variable --- libraries/chain/block_log.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index b2b8a0c5a9..b28232e794 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -482,9 +482,9 @@ namespace eosio { namespace chain { std::optional head; enum class verify_block_num_t { - yes, // Verify block number when read a serialized block. - no // Does not verify block number when read a serialized block. - // This is for the case where block id is vefified seperately. + yes, // Verify the block number when reading a serialized block. + no // Do not verify the block number when read a serialized block. + // This is for the case where the block id is vefified seperately. }; virtual ~block_log_impl() = default; @@ -1319,10 +1319,10 @@ namespace eosio { namespace chain { std::optional h = my->read_block_header_by_num(block_num); if (h) { - auto returd_id = h->calculate_id(); - if (returd_id != id) { - wlog("wrong block id was read from block log, expected id: ${id}, returned id: ${rid}", - ("id", id)("rid", returd_id)); + auto read_id = h->calculate_id(); + if (read_id != id) { + wlog("wrong block id was read from block log, expected id: ${id}, read id: ${rid}", + ("id", id)("rid", read_id)); return {}; } } else { From c39f4f145867b03f46aaf7162782fffce489a9ef Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Wed, 18 Sep 2024 19:38:34 -0400 Subject: [PATCH 12/49] Remove the code to validate block number and block ID in a serialized block --- libraries/chain/block_log.cpp | 41 +++++------------------------------ 1 file changed, 5 insertions(+), 36 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index b28232e794..16bd66a8dd 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -481,12 +481,6 @@ namespace eosio { namespace chain { }; std::optional head; - enum class verify_block_num_t { - yes, // Verify the block number when reading a serialized block. - no // Do not verify the block number when read a serialized block. - // This is for the case where the block id is vefified seperately. - }; - virtual ~block_log_impl() = default; virtual uint32_t first_block_num() = 0; @@ -498,7 +492,7 @@ namespace eosio { namespace chain { virtual void flush() = 0; virtual signed_block_ptr read_block_by_num(uint32_t block_num) = 0; - virtual std::vector read_serialized_block_by_num(uint32_t block_num, verify_block_num_t verify_block_num) = 0; + virtual std::vector read_serialized_block_by_num(uint32_t block_num) = 0; virtual std::optional read_block_header_by_num(uint32_t block_num) = 0; virtual uint32_t version() const = 0; @@ -532,7 +526,7 @@ namespace eosio { namespace chain { void flush() final {} signed_block_ptr read_block_by_num(uint32_t block_num) final { return {}; }; - std::vector read_serialized_block_by_num(uint32_t block_num, verify_block_num_t verify_block_num) final { return {}; }; + std::vector read_serialized_block_by_num(uint32_t block_num) final { return {}; }; std::optional read_block_header_by_num(uint32_t block_num) final { return {}; }; uint32_t version() const final { return 0; } @@ -630,7 +624,7 @@ namespace eosio { namespace chain { FC_LOG_AND_RETHROW() } - std::vector read_serialized_block_by_num(uint32_t block_num, verify_block_num_t verify_block_num) final { + std::vector read_serialized_block_by_num(uint32_t block_num) final { try { uint64_t pos = get_block_pos(block_num); @@ -645,15 +639,6 @@ namespace eosio { namespace chain { } } - if (verify_block_num == verify_block_num_t::yes) { - auto bnum = get_block_num_at(block_file, pos); - if (block_num != bnum) { - wlog("Wrong block num was read from block log. expected block_num: ${n}, returned block num: ${bnum}", - ("n", block_num)("bnum", bnum)); - return {}; - } - } - uint64_t block_size = 0; uint32_t head_block_num = block_header::num_from_id(head->id); constexpr uint32_t block_pos_size = sizeof(uint64_t); // size of block position field in the block log file @@ -1309,28 +1294,12 @@ namespace eosio { namespace chain { std::vector block_log::read_serialized_block_by_num(uint32_t block_num) const { std::lock_guard g(my->mtx); - return my->read_serialized_block_by_num(block_num, detail::block_log_impl::verify_block_num_t::yes); + return my->read_serialized_block_by_num(block_num); } std::vector block_log::read_serialized_block_by_id(const block_id_type& id) const { std::lock_guard g(my->mtx); - - auto block_num = block_header::num_from_id(id); - - std::optional h = my->read_block_header_by_num(block_num); - if (h) { - auto read_id = h->calculate_id(); - if (read_id != id) { - wlog("wrong block id was read from block log, expected id: ${id}, read id: ${rid}", - ("id", id)("rid", read_id)); - return {}; - } - } else { - wlog("no header found for block id: ${id}, block_num: ${n}", ("id", id)("n", block_num)); - return {}; - } - - return my->read_serialized_block_by_num(block_num, detail::block_log_impl::verify_block_num_t::no); + return my->read_serialized_block_by_num(block_header::num_from_id(id)); } std::optional block_log::read_block_header_by_num(uint32_t block_num) const { From 12c63d88f239842c17f29900858c42d473d0821c Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Wed, 18 Sep 2024 19:42:30 -0400 Subject: [PATCH 13/49] Revert the change of moving block_num_at out of block_log_data class --- libraries/chain/block_log.cpp | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 16bd66a8dd..7378173f7e 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -197,24 +197,6 @@ namespace eosio { namespace chain { return bh; } - uint32_t get_block_num_at(fc::datastream& file, uint64_t position) { - // to derive blknum_offset==14 see block_header.hpp and note on disk struct is packed - // block_timestamp_type timestamp; //bytes 0:3 - // account_name producer; //bytes 4:11 - // uint16_t confirmed; //bytes 12:13 - // block_id_type previous; //bytes 14:45, low 4 bytes is big endian block number - // of previous block - - file.seek_end(0); - int blknum_offset = 14; - - EOS_ASSERT(position + blknum_offset + sizeof(uint32_t) <= file.tellp(), block_log_exception, - "Read outside of file: position ${position}, blknum_offset ${o}, file size ${s}", - ("position", position)("o", blknum_offset)("s", file.tellp())); - - uint32_t prev_block_num = read_data_at(file, position + blknum_offset); - return fc::endian_reverse_u32(prev_block_num) + 1; - } /// Provide the read only view of the blocks.log file class block_log_data : public chain::log_data_base { @@ -257,7 +239,22 @@ namespace eosio { namespace chain { } uint32_t block_num_at(uint64_t position) { - return get_block_num_at(file, position); + // to derive blknum_offset==14 see block_header.hpp and note on disk struct is packed + // block_timestamp_type timestamp; //bytes 0:3 + // account_name producer; //bytes 4:11 + // uint16_t confirmed; //bytes 12:13 + // block_id_type previous; //bytes 14:45, low 4 bytes is big endian block number + // of previous block + + file.seek_end(0); + int blknum_offset = 14; + + EOS_ASSERT(position + blknum_offset + sizeof(uint32_t) <= file.tellp(), block_log_exception, + "Read outside of file: position ${position}, blknum_offset ${o}, file size ${s}", + ("position", position)("o", blknum_offset)("s", file.tellp())); + + uint32_t prev_block_num = read_data_at(file, position + blknum_offset); + return fc::endian_reverse_u32(prev_block_num) + 1; } auto& ro_stream_at(uint64_t pos) { From 543a1a57bbd409a3a044a322ea31bd58b220e7e1 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Wed, 18 Sep 2024 19:58:25 -0400 Subject: [PATCH 14/49] Streamline error handling --- libraries/chain/block_log.cpp | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 7378173f7e..bb2bb1e650 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -636,31 +636,31 @@ namespace eosio { namespace chain { } } - uint64_t block_size = 0; uint32_t head_block_num = block_header::num_from_id(head->id); + EOS_ASSERT(block_num <= head_block_num, block_log_exception, + "Current block_num ${n} was greater than head_block_num ${h}", + ("n", block_num)("h", head_block_num)); + + uint64_t block_size = 0; constexpr uint32_t block_pos_size = sizeof(uint64_t); // size of block position field in the block log file if (block_num < head_block_num) { // current block is not the last block in the log file. uint64_t next_block_pos = get_block_pos(block_num + 1); - if (next_block_pos == block_log::npos) { - wlog("no position found for block ${n}", ("n", block_num + 1)); - return {}; - } - if (next_block_pos <= pos) { - wlog("next block position ${nn} should be greater than current block position ${cn}", ("nn", next_block_pos)("cn", pos)); - return {}; - } + + EOS_ASSERT(next_block_pos != block_log::npos, block_log_exception, + "no position found for block ${n}", ("n", block_num + 1)); + EOS_ASSERT(next_block_pos > pos, block_log_exception, + "next block position ${nn} should be greater than current block position ${cn}", + ("nn", next_block_pos)("cn", pos)); block_size = next_block_pos - pos - block_pos_size; - } else if (block_num == head_block_num) { + } else { + assert (block_num == head_block_num); + // current block is the last block in the file. auto log_size = std::filesystem::file_size(block_file.get_file_path()); block_size = log_size - pos - block_pos_size; - } else { - EOS_ASSERT(false, block_log_exception, - "Current block_num ${n} was greater than head_block_num ${h}", ("n", block_num)("h", head_block_num)); - return {}; } std::vector buff; From cb0112ffbaa39652e8c36d5492850050d0ffef17 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Wed, 18 Sep 2024 21:16:04 -0400 Subject: [PATCH 15/49] More EOS_ASSERTs --- libraries/chain/block_log.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index bb2bb1e650..8daa35f888 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -650,17 +650,21 @@ namespace eosio { namespace chain { EOS_ASSERT(next_block_pos != block_log::npos, block_log_exception, "no position found for block ${n}", ("n", block_num + 1)); - EOS_ASSERT(next_block_pos > pos, block_log_exception, - "next block position ${nn} should be greater than current block position ${cn}", - ("nn", next_block_pos)("cn", pos)); + EOS_ASSERT(next_block_pos > pos + block_pos_size, block_log_exception, + "next block position ${np} should be greater than current block position ${p} plus block position field size ${bps}", + ("np", next_block_pos)("p", pos)("bps", block_pos_size)); block_size = next_block_pos - pos - block_pos_size; } else { + // current block is the last block in the file. assert (block_num == head_block_num); - // current block is the last block in the file. - auto log_size = std::filesystem::file_size(block_file.get_file_path()); - block_size = log_size - pos - block_pos_size; + auto file_size = std::filesystem::file_size(block_file.get_file_path()); + EOS_ASSERT(file_size > pos + block_pos_size, block_log_exception, + "block log file size ${fs} should be greater than current block position ${p} plus block position field size ${bps}", + ("fs", file_size)("p", pos)("bps", block_pos_size)); + + block_size = file_size - pos - block_pos_size; } std::vector buff; From 55994b0274dd5de14e5c04df6ff5d195ece83611 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 19 Sep 2024 07:47:10 -0500 Subject: [PATCH 16/49] GH-612 Remove dead code --- plugins/net_plugin/net_plugin.cpp | 37 +++++-------------------------- 1 file changed, 6 insertions(+), 31 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 2a6be27e53..c7ea797598 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -1060,7 +1060,6 @@ namespace eosio { void blk_send_branch( const block_id_type& msg_head_id ); void blk_send_branch( uint32_t msg_head_num, uint32_t lib_num, uint32_t head_num ); - void blk_send(const block_id_type& blkid); void enqueue( const net_message &msg ); size_t enqueue_block( const std::vector& sb, bool to_sync_queue = false); @@ -1560,25 +1559,6 @@ namespace eosio { } } - // called from connection strand - void connection::blk_send( const block_id_type& blkid ) { - try { - controller& cc = my_impl->chain_plug->chain(); - std::vector b = cc.fetch_serialized_block_by_id( blkid ); // thread-safe - if( !b.empty() ) { - peer_dlog( this, "fetch_serialized_block_by_id num ${n}", ("n", block_header::num_from_id(blkid)) ); - enqueue_block( b ); - } else { - peer_ilog( this, "fetch block by id returned null, id ${id}", ("id", blkid) ); - } - } catch( const assert_exception& ex ) { - // possible corrupted block log - peer_elog( this, "caught assert on fetch_serialized_block_by_id, ${ex}, id ${id}", ("ex", ex.to_string())("id", blkid) ); - } catch( ... ) { - peer_elog( this, "caught other exception fetching block id ${id}", ("id", blkid) ); - } - } - void connection::send_handshake() { if (closed()) return; @@ -3694,11 +3674,9 @@ namespace eosio { blk_send_branch( msg.req_blocks.ids.empty() ? block_id_type() : msg.req_blocks.ids.back() ); break; case normal : - peer_dlog( this, "received request_message:normal" ); - if( !msg.req_blocks.ids.empty() ) { - blk_send( msg.req_blocks.ids.back() ); - } - break; + peer_wlog( this, "Invalid request_message, req_blocks.mode = normal" ); + close(); + return; default:; } @@ -3712,12 +3690,9 @@ namespace eosio { } // no break case normal : - if( !msg.req_trx.ids.empty() ) { - peer_wlog( this, "Invalid request_message, req_trx.ids.size ${s}", ("s", msg.req_trx.ids.size()) ); - close(); - return; - } - break; + peer_wlog( this, "Invalid request_message, req_trx.mode = normal" ); + close(); + return; default:; } } From 246ccfbb4888e02cfa81f96ab97e6ad332de7923 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 19 Sep 2024 08:23:02 -0500 Subject: [PATCH 17/49] GH-612 Missed the no break. --- plugins/net_plugin/net_plugin.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index c7ea797598..76839be79e 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -3688,11 +3688,15 @@ namespace eosio { if( msg.req_blocks.mode == none ) { peer_syncing_from_us = false; } - // no break + if( !msg.req_trx.ids.empty() ) { + peer_wlog( this, "Invalid request_message, req_trx.mode=none, req_trx.ids.size ${s}", ("s", msg.req_trx.ids.size()) ); + close(); + } + break; case normal : - peer_wlog( this, "Invalid request_message, req_trx.mode = normal" ); + peer_wlog( this, "Invalid request_message, req_trx.mode=normal" ); close(); - return; + break; default:; } } From eaa6161347843a01726f6cb95b339b605c8b72f8 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Thu, 19 Sep 2024 15:01:16 -0400 Subject: [PATCH 18/49] Refactor to handle split block log files --- libraries/chain/block_log.cpp | 115 ++++++++++-------- .../chain/include/eosio/chain/log_catalog.hpp | 58 +++++++++ 2 files changed, 123 insertions(+), 50 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 8daa35f888..23f2c98119 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -185,6 +185,16 @@ namespace eosio { namespace chain { return block; } + template + std::vector read_serialized_block(Stream&& ds, uint64_t block_size) { + std::vector buff; + buff.resize(block_size); + + ds.read(buff.data(), block_size); + + return buff; + } + template signed_block_header read_block_header(Stream&& ds, uint32_t expect_block_num) { signed_block_header bh; @@ -568,6 +578,7 @@ namespace eosio { namespace chain { virtual uint32_t working_block_file_first_block_num() { return preamble.first_block_num; } virtual void post_append(uint64_t pos) {} virtual signed_block_ptr retry_read_block_by_num(uint32_t block_num) { return {}; } + virtual std::vector retry_read_serialized_block_by_num(uint32_t block_num) { return {}; } virtual std::optional retry_read_block_header_by_num(uint32_t block_num) { return {}; } void append(const signed_block_ptr& b, const block_id_type& id, @@ -609,6 +620,45 @@ namespace eosio { namespace chain { return pos; } + block_pos_size_t get_block_position_and_size(uint32_t block_num) { + uint64_t pos = get_block_pos(block_num); + + if (pos == block_log::npos) { + return block_pos_size_t {.position = block_log::npos, .size = 0}; + } + + assert(head); + uint32_t last_block_num = block_header::num_from_id(head->id); + assert(block_num <= last_block_num); + + uint64_t block_size = 0; + constexpr uint32_t block_pos_size = sizeof(uint64_t); // size of block position field in the block log file + + if (block_num < last_block_num) { + // current block is not the last block in the log file. + uint64_t next_block_pos = get_block_pos(block_num + 1); + + EOS_ASSERT(next_block_pos != block_log::npos, block_log_exception, + "no position found for block ${n}", ("n", block_num + 1)); + EOS_ASSERT(next_block_pos > pos + block_pos_size, block_log_exception, + "next block position ${np} should be greater than current block position ${p} plus block position field size ${bps}", + ("np", next_block_pos)("p", pos)("bps", block_pos_size)); + + block_size = next_block_pos - pos - block_pos_size; + } else { + // current block is the last block in the file. + + auto file_size = std::filesystem::file_size(block_file.get_file_path()); + EOS_ASSERT(file_size > pos + block_pos_size, block_log_exception, + "block log file size ${fs} should be greater than current block position ${p} plus block position field size ${bps}", + ("fs", file_size)("p", pos)("bps", block_pos_size)); + + block_size = file_size - pos - block_pos_size; + } + + return block_pos_size_t {.position = pos, .size = block_size}; + } + signed_block_ptr read_block_by_num(uint32_t block_num) final { try { uint64_t pos = get_block_pos(block_num); @@ -623,57 +673,12 @@ namespace eosio { namespace chain { std::vector read_serialized_block_by_num(uint32_t block_num) final { try { - uint64_t pos = get_block_pos(block_num); - - if (pos == block_log::npos || !head) { - // Rare case. No need a special code for it. - // Just fall back to the regular `retry_read_block_by_num` and then serialize. - auto ret = retry_read_block_by_num(block_num); - if (ret) { - return fc::raw::pack(*ret); - } else { - return {}; - } - } - - uint32_t head_block_num = block_header::num_from_id(head->id); - EOS_ASSERT(block_num <= head_block_num, block_log_exception, - "Current block_num ${n} was greater than head_block_num ${h}", - ("n", block_num)("h", head_block_num)); - - uint64_t block_size = 0; - constexpr uint32_t block_pos_size = sizeof(uint64_t); // size of block position field in the block log file - - if (block_num < head_block_num) { - // current block is not the last block in the log file. - uint64_t next_block_pos = get_block_pos(block_num + 1); - - EOS_ASSERT(next_block_pos != block_log::npos, block_log_exception, - "no position found for block ${n}", ("n", block_num + 1)); - EOS_ASSERT(next_block_pos > pos + block_pos_size, block_log_exception, - "next block position ${np} should be greater than current block position ${p} plus block position field size ${bps}", - ("np", next_block_pos)("p", pos)("bps", block_pos_size)); - - block_size = next_block_pos - pos - block_pos_size; - } else { - // current block is the last block in the file. - assert (block_num == head_block_num); - - auto file_size = std::filesystem::file_size(block_file.get_file_path()); - EOS_ASSERT(file_size > pos + block_pos_size, block_log_exception, - "block log file size ${fs} should be greater than current block position ${p} plus block position field size ${bps}", - ("fs", file_size)("p", pos)("bps", block_pos_size)); - - block_size = file_size - pos - block_pos_size; + block_pos_size_t pos_size = get_block_position_and_size(block_num); + if (pos_size.position != block_log::npos) { + block_file.seek(pos_size.position); + return read_serialized_block(block_file, pos_size.size); } - - std::vector buff; - buff.resize(block_size); - - block_file.seek(pos); - block_file.read(buff.data(), block_size); - - return buff; + return retry_read_serialized_block_by_num(block_num); } FC_LOG_AND_RETHROW() } @@ -1095,6 +1100,16 @@ namespace eosio { namespace chain { return {}; } + std::vector retry_read_serialized_block_by_num(uint32_t block_num) final { + uint64_t block_size = 0; + + auto ds = catalog.ro_stream_and_size_for_block(block_num, block_size); + if (ds) { + return read_serialized_block(*ds, block_size); + } + return {}; + } + std::optional retry_read_block_header_by_num(uint32_t block_num) final { auto ds = catalog.ro_stream_for_block(block_num); if (ds) diff --git a/libraries/chain/include/eosio/chain/log_catalog.hpp b/libraries/chain/include/eosio/chain/log_catalog.hpp index 70a60574f7..6560721f88 100644 --- a/libraries/chain/include/eosio/chain/log_catalog.hpp +++ b/libraries/chain/include/eosio/chain/log_catalog.hpp @@ -31,6 +31,11 @@ struct null_verifier { void verify(const LogData&, const std::filesystem::path&) {} }; +struct block_pos_size_t { + uint64_t position = 0; // start position of the block in block log + uint64_t size = 0; // size of the block +}; + template struct log_catalog { using block_num_t = uint32_t; @@ -159,6 +164,50 @@ struct log_catalog { } } + block_pos_size_t block_position_size(uint32_t block_num) { + assert(block_num >= log_data.first_block_num()); + uint64_t pos = log_index.nth_block_position(block_num - log_data.first_block_num()); + + constexpr uint32_t block_pos_size = sizeof(uint64_t); + uint64_t block_size = 0; + assert(block_num <= log_data.last_block_num()); + if (block_num < log_data.last_block_num()) { + uint64_t next_block_pos = log_index.nth_block_position(block_num + 1 - log_data.first_block_num()); + block_size = next_block_pos - pos - block_pos_size; + } else { + block_size = log_data.size() - pos - block_pos_size; + } + + return block_pos_size_t { .position = pos, .size = block_size }; + } + + std::optional get_block_position_and_size(uint32_t block_num) { + try { + if (active_index != npos) { + auto active_item = std::next(collection.begin(), active_index); + if (active_item->first <= block_num && block_num <= active_item->second.last_block_num) { + return block_position_size(block_num); + } + } + if (block_num < first_block_num()) + return {}; + + auto it = --collection.upper_bound(block_num); + + if (block_num <= it->second.last_block_num) { + auto name = it->second.filename_base; + log_data.open(name.replace_extension("log")); + log_index.open(name.replace_extension("index")); + active_index = std::distance(collection.begin(), it); + return block_position_size(block_num); + } + return {}; + } catch (...) { + active_index = npos; + return {}; + } + } + fc::datastream* ro_stream_for_block(uint32_t block_num) { auto pos = get_block_position(block_num); if (pos) { @@ -176,6 +225,15 @@ struct log_catalog { return {}; } + fc::datastream* ro_stream_and_size_for_block(uint32_t block_num, uint64_t& block_size) { + auto pos_size = get_block_position_and_size(block_num); + if (pos_size) { + block_size = pos_size->size; + return &log_data.ro_stream_at(pos_size->position); + } + return nullptr; + } + std::optional id_for_block(uint32_t block_num) { auto pos = get_block_position(block_num); if (pos) { From 82a0c77891f385c33345124571c3e20113419c57 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Thu, 19 Sep 2024 17:22:13 -0400 Subject: [PATCH 19/49] Add new testcase `finalizer_safety_file_serialization_unchanged` --- unittests/finalizer_tests.cpp | 60 +++++++++++++++++++++++------------ 1 file changed, 40 insertions(+), 20 deletions(-) diff --git a/unittests/finalizer_tests.cpp b/unittests/finalizer_tests.cpp index e44f228138..19f42ad207 100644 --- a/unittests/finalizer_tests.cpp +++ b/unittests/finalizer_tests.cpp @@ -228,32 +228,31 @@ BOOST_AUTO_TEST_CASE( finalizer_safety_file_io ) try { } FC_LOG_AND_RETHROW() +namespace fs = std::filesystem; -BOOST_AUTO_TEST_CASE( finalizer_safety_file_versioning ) try { - namespace fs = std::filesystem; - - fs::path test_data_path { UNITTEST_TEST_DATA_DIR }; - auto fsi_reference_dir = test_data_path / "fsi"; +void create_fsi_reference(my_finalizers_t& fset) { + std::vector keys = create_keys(3); + std::vector fsi = create_random_fsi(3); - auto create_fsi_reference = [&](my_finalizers_t& fset) { - std::vector keys = create_keys(3); - std::vector fsi = create_random_fsi(3); + bls_pub_priv_key_map_t local_finalizers = create_local_finalizers<0, 1, 2>(keys); + fset.set_keys(local_finalizers); + set_fsi(fset, keys, fsi); +} - bls_pub_priv_key_map_t local_finalizers = create_local_finalizers<0, 1, 2>(keys); - fset.set_keys(local_finalizers); - set_fsi(fset, keys, fsi); - }; +void create_fsi_reference_file(const fs::path& safety_file_path) { + my_finalizers_t fset{safety_file_path}; + create_fsi_reference(fset); + fset.save_finalizer_safety_info(); +} - auto create_fsi_reference_file = [&](const fs::path& safety_file_path) { - my_finalizers_t fset{safety_file_path}; - create_fsi_reference(fset); - fset.save_finalizer_safety_info(); - }; +auto mk_versioned_fsi_file_path(uint32_t v) { + fs::path test_data_path { UNITTEST_TEST_DATA_DIR }; + auto fsi_reference_dir = test_data_path / "fsi"; - auto mk_versioned_fsi_file_path = [&](uint32_t v) { - return fsi_reference_dir / ("safety_v"s + std::to_string(v) + ".dat"); - }; + return fsi_reference_dir / ("safety_v"s + std::to_string(v) + ".dat"); +} +BOOST_AUTO_TEST_CASE( finalizer_safety_file_versioning ) try { auto current_version = my_finalizers_t::current_safety_file_version; // run this unittest with the option `-- --save-fsi-ref` to save ref file for the current version. @@ -307,5 +306,26 @@ BOOST_AUTO_TEST_CASE( finalizer_safety_file_versioning ) try { } FC_LOG_AND_RETHROW() +// Verify that we have not changed the fsi file serialiization +// ------------------------------------------------------------ +BOOST_AUTO_TEST_CASE( finalizer_safety_file_serialization_unchanged ) try { + auto current_version = my_finalizers_t::current_safety_file_version; + auto ref_path = mk_versioned_fsi_file_path(current_version); // the saved file for current_version + + fc::temp_directory tempdir; + auto tmp_path = tempdir.path() / "new_safety.dat"; + create_fsi_reference_file(tmp_path); // save a new file in tmp_path + + auto read_file = [](const fs::path& path) { + std::ifstream t("file.txt"); + std::stringstream buffer; + buffer << t.rdbuf(); + return buffer; + }; + + BOOST_REQUIRE(read_file(ref_path).view() == read_file(tmp_path).view()); + +} FC_LOG_AND_RETHROW() + BOOST_AUTO_TEST_SUITE_END() From d25f119d03cf055d48c322bbb611470aded3fab4 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Thu, 19 Sep 2024 17:54:43 -0400 Subject: [PATCH 20/49] Remove read_serialized_block_by_id() which is not longer needed --- libraries/chain/block_log.cpp | 5 ----- libraries/chain/controller.cpp | 7 ------- libraries/chain/include/eosio/chain/block_log.hpp | 1 - libraries/chain/include/eosio/chain/controller.hpp | 2 -- 4 files changed, 15 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 23f2c98119..a383937f4b 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -1313,11 +1313,6 @@ namespace eosio { namespace chain { return my->read_serialized_block_by_num(block_num); } - std::vector block_log::read_serialized_block_by_id(const block_id_type& id) const { - std::lock_guard g(my->mtx); - return my->read_serialized_block_by_num(block_header::num_from_id(id)); - } - std::optional block_log::read_block_header_by_num(uint32_t block_num) const { std::lock_guard g(my->mtx); return my->read_block_header_by_num(block_num); diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index cf5a4ef393..cd1152d617 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -5514,13 +5514,6 @@ signed_block_ptr controller::fetch_block_by_id( const block_id_type& id )const { return signed_block_ptr(); } -std::vector controller::fetch_serialized_block_by_id( const block_id_type& id )const { try { - auto sb_ptr = my->fork_db_fetch_block_by_id(id); - if( sb_ptr ) return fc::raw::pack(*sb_ptr); - - return my->blog.read_serialized_block_by_id(id); -} FC_CAPTURE_AND_RETHROW( (id) ) } - bool controller::block_exists(const block_id_type& id) const { bool exists = my->fork_db_block_exists(id); if( exists ) return true; diff --git a/libraries/chain/include/eosio/chain/block_log.hpp b/libraries/chain/include/eosio/chain/block_log.hpp index e5cd3ae78a..e933cb7920 100644 --- a/libraries/chain/include/eosio/chain/block_log.hpp +++ b/libraries/chain/include/eosio/chain/block_log.hpp @@ -55,7 +55,6 @@ namespace eosio { namespace chain { signed_block_ptr read_block_by_num(uint32_t block_num)const; std::vector read_serialized_block_by_num(uint32_t block_num)const; - std::vector read_serialized_block_by_id(const block_id_type& id)const; std::optional read_block_header_by_num(uint32_t block_num)const; std::optional read_block_id_by_num(uint32_t block_num)const; diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index 0025220b3c..da895f0bc6 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -324,8 +324,6 @@ namespace eosio::chain { // to avoid deserialization and serialization vector fetch_serialized_block_by_number( uint32_t block_num )const; // thread-safe - vector fetch_serialized_block_by_id( const block_id_type& id )const; - // thread-safe bool block_exists(const block_id_type& id) const; bool validated_block_exists(const block_id_type& id) const; // thread-safe, retrieves block according to fork db best branch which can change at any moment From 0a93311a69d4ce445ddfc817f7c4d70dbc80cc26 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Thu, 19 Sep 2024 20:06:33 -0400 Subject: [PATCH 21/49] Append serialized block to a passed in buffer to save copying --- libraries/chain/block_log.cpp | 36 ++++++++++--------- libraries/chain/controller.cpp | 14 ++++++-- .../chain/include/eosio/chain/block_log.hpp | 3 +- .../chain/include/eosio/chain/controller.hpp | 6 ++-- plugins/net_plugin/net_plugin.cpp | 2 +- 5 files changed, 36 insertions(+), 25 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index a383937f4b..647a7e8801 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -186,13 +186,13 @@ namespace eosio { namespace chain { } template - std::vector read_serialized_block(Stream&& ds, uint64_t block_size) { - std::vector buff; - buff.resize(block_size); + void read_serialized_block(Stream&& ds, uint64_t block_size, std::vector& buff) { + assert(buff.empty()); - ds.read(buff.data(), block_size); - - return buff; + try { + buff.resize(block_size); + ds.read(buff.data(), block_size); // append to buff + } catch (...) { buff.clear(); } // restore buff to its original state } template @@ -499,7 +499,7 @@ namespace eosio { namespace chain { virtual void flush() = 0; virtual signed_block_ptr read_block_by_num(uint32_t block_num) = 0; - virtual std::vector read_serialized_block_by_num(uint32_t block_num) = 0; + virtual void read_serialized_block_by_num(uint32_t block_num, std::vector& buff) = 0; virtual std::optional read_block_header_by_num(uint32_t block_num) = 0; virtual uint32_t version() const = 0; @@ -533,7 +533,7 @@ namespace eosio { namespace chain { void flush() final {} signed_block_ptr read_block_by_num(uint32_t block_num) final { return {}; }; - std::vector read_serialized_block_by_num(uint32_t block_num) final { return {}; }; + void read_serialized_block_by_num(uint32_t block_num, std::vector& buff) final {}; std::optional read_block_header_by_num(uint32_t block_num) final { return {}; }; uint32_t version() const final { return 0; } @@ -578,7 +578,7 @@ namespace eosio { namespace chain { virtual uint32_t working_block_file_first_block_num() { return preamble.first_block_num; } virtual void post_append(uint64_t pos) {} virtual signed_block_ptr retry_read_block_by_num(uint32_t block_num) { return {}; } - virtual std::vector retry_read_serialized_block_by_num(uint32_t block_num) { return {}; } + virtual void retry_read_serialized_block_by_num(uint32_t block_num, std::vector& buff) {} virtual std::optional retry_read_block_header_by_num(uint32_t block_num) { return {}; } void append(const signed_block_ptr& b, const block_id_type& id, @@ -671,14 +671,16 @@ namespace eosio { namespace chain { FC_LOG_AND_RETHROW() } - std::vector read_serialized_block_by_num(uint32_t block_num) final { + void read_serialized_block_by_num(uint32_t block_num, std::vector& buff) final { try { block_pos_size_t pos_size = get_block_position_and_size(block_num); if (pos_size.position != block_log::npos) { block_file.seek(pos_size.position); - return read_serialized_block(block_file, pos_size.size); + read_serialized_block(block_file, pos_size.size, buff); + + return; } - return retry_read_serialized_block_by_num(block_num); + retry_read_serialized_block_by_num(block_num, buff); } FC_LOG_AND_RETHROW() } @@ -1100,14 +1102,14 @@ namespace eosio { namespace chain { return {}; } - std::vector retry_read_serialized_block_by_num(uint32_t block_num) final { + void retry_read_serialized_block_by_num(uint32_t block_num, std::vector& buff) final { uint64_t block_size = 0; auto ds = catalog.ro_stream_and_size_for_block(block_num, block_size); if (ds) { - return read_serialized_block(*ds, block_size); + read_serialized_block(*ds, block_size, buff); + return; } - return {}; } std::optional retry_read_block_header_by_num(uint32_t block_num) final { @@ -1308,9 +1310,9 @@ namespace eosio { namespace chain { return my->read_block_by_num(block_num); } - std::vector block_log::read_serialized_block_by_num(uint32_t block_num) const { + void block_log::read_serialized_block_by_num(uint32_t block_num, std::vector& buff) const { std::lock_guard g(my->mtx); - return my->read_serialized_block_by_num(block_num); + my->read_serialized_block_by_num(block_num, buff); } std::optional block_log::read_block_header_by_num(uint32_t block_num) const { diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index cd1152d617..23881df441 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -5543,12 +5543,20 @@ signed_block_ptr controller::fetch_block_by_number( uint32_t block_num )const { return my->blog.read_block_by_num(block_num); } FC_CAPTURE_AND_RETHROW( (block_num) ) } -vector controller::fetch_serialized_block_by_number( uint32_t block_num )const { try { +void controller::fetch_serialized_block_by_number( uint32_t block_num, std::vector& buff )const { try { if (signed_block_ptr b = my->fork_db_fetch_block_on_best_branch_by_num(block_num)) { - return fc::raw::pack(*b); + try { + assert(buff.empty()); + const auto sz = fc::raw::pack_size(*b); + buff.resize(sz); + fc::datastream ds(buff.data(), sz); // append to buff + fc::raw::pack(ds, *b); + } catch (...) { buff.clear(); } // restore buff to its original state + + return; } - return my->blog.read_serialized_block_by_num(block_num); + my->blog.read_serialized_block_by_num(block_num, buff); } FC_CAPTURE_AND_RETHROW( (block_num) ) } std::optional controller::fetch_block_header_by_number( uint32_t block_num )const { try { diff --git a/libraries/chain/include/eosio/chain/block_log.hpp b/libraries/chain/include/eosio/chain/block_log.hpp index e933cb7920..b342ea8eaa 100644 --- a/libraries/chain/include/eosio/chain/block_log.hpp +++ b/libraries/chain/include/eosio/chain/block_log.hpp @@ -54,7 +54,8 @@ namespace eosio { namespace chain { void reset( const chain_id_type& chain_id, uint32_t first_block_num ); signed_block_ptr read_block_by_num(uint32_t block_num)const; - std::vector read_serialized_block_by_num(uint32_t block_num)const; + // Append into `buff` to avoid copying. Assume `buff` is an empty vector. + void read_serialized_block_by_num(uint32_t block_num, std::vector& buff)const; std::optional read_block_header_by_num(uint32_t block_num)const; std::optional read_block_id_by_num(uint32_t block_num)const; diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index da895f0bc6..988700b4fc 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -320,9 +320,9 @@ namespace eosio::chain { signed_block_ptr fetch_block_by_number( uint32_t block_num )const; // thread-safe signed_block_ptr fetch_block_by_id( const block_id_type& id )const; - // thread-safe, retrieves signed block in serialized form, used by net_plugin - // to avoid deserialization and serialization - vector fetch_serialized_block_by_number( uint32_t block_num )const; + // thread-safe, retrieves serialized signed block and appends it to `buff` (to avoid copying). + // Assume `buff` is an empty vector. + void fetch_serialized_block_by_number( uint32_t block_num, std::vector& buff )const; // thread-safe bool block_exists(const block_id_type& id) const; bool validated_block_exists(const block_id_type& id) const; diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 76839be79e..5aa8abe0b6 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -1728,7 +1728,7 @@ namespace eosio { controller& cc = my_impl->chain_plug->chain(); std::vector sb; try { - sb = cc.fetch_serialized_block_by_number( num ); // thread-safe + cc.fetch_serialized_block_by_number( num, sb ); // thread-safe } FC_LOG_AND_DROP(); if( !sb.empty() ) { // Skip transmitting block this loop if threshold exceeded From 0f0b1f628fae5fbbd6dddddbac203957fb96b177 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Thu, 19 Sep 2024 20:21:45 -0400 Subject: [PATCH 22/49] Revert size() change in block_num_at --- libraries/chain/block_log.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 647a7e8801..3ec57d18fb 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -256,12 +256,11 @@ namespace eosio { namespace chain { // block_id_type previous; //bytes 14:45, low 4 bytes is big endian block number // of previous block - file.seek_end(0); int blknum_offset = 14; - EOS_ASSERT(position + blknum_offset + sizeof(uint32_t) <= file.tellp(), block_log_exception, + EOS_ASSERT(position + blknum_offset + sizeof(uint32_t) <= size(), block_log_exception, "Read outside of file: position ${position}, blknum_offset ${o}, file size ${s}", - ("position", position)("o", blknum_offset)("s", file.tellp())); + ("position", position)("o", blknum_offset)("s", size())); uint32_t prev_block_num = read_data_at(file, position + blknum_offset); return fc::endian_reverse_u32(prev_block_num) + 1; From 53fe9a18c649caa18f8e3a928e5892ed81b328d6 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Thu, 19 Sep 2024 20:23:45 -0400 Subject: [PATCH 23/49] remove an unnecessary EOS_ASSERT --- libraries/chain/block_log.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 3ec57d18fb..071917f34f 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -637,8 +637,6 @@ namespace eosio { namespace chain { // current block is not the last block in the log file. uint64_t next_block_pos = get_block_pos(block_num + 1); - EOS_ASSERT(next_block_pos != block_log::npos, block_log_exception, - "no position found for block ${n}", ("n", block_num + 1)); EOS_ASSERT(next_block_pos > pos + block_pos_size, block_log_exception, "next block position ${np} should be greater than current block position ${p} plus block position field size ${bps}", ("np", next_block_pos)("p", pos)("bps", block_pos_size)); From 2f2fbada1853fee3e28c06f6c395b720262e1176 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 20 Sep 2024 11:18:40 -0400 Subject: [PATCH 24/49] Add second test for issue #764. --- unittests/finalizer_tests.cpp | 49 +++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 8 deletions(-) diff --git a/unittests/finalizer_tests.cpp b/unittests/finalizer_tests.cpp index 19f42ad207..a66c2d1f82 100644 --- a/unittests/finalizer_tests.cpp +++ b/unittests/finalizer_tests.cpp @@ -245,13 +245,20 @@ void create_fsi_reference_file(const fs::path& safety_file_path) { fset.save_finalizer_safety_info(); } -auto mk_versioned_fsi_file_path(uint32_t v) { +fs::path mk_versioned_fsi_file_path(uint32_t v) { fs::path test_data_path { UNITTEST_TEST_DATA_DIR }; auto fsi_reference_dir = test_data_path / "fsi"; return fsi_reference_dir / ("safety_v"s + std::to_string(v) + ".dat"); } +std::stringstream read_file(const fs::path& path) { + std::ifstream t("file.txt"); + std::stringstream buffer; + buffer << t.rdbuf(); + return buffer; +} + BOOST_AUTO_TEST_CASE( finalizer_safety_file_versioning ) try { auto current_version = my_finalizers_t::current_safety_file_version; @@ -316,16 +323,42 @@ BOOST_AUTO_TEST_CASE( finalizer_safety_file_serialization_unchanged ) try { auto tmp_path = tempdir.path() / "new_safety.dat"; create_fsi_reference_file(tmp_path); // save a new file in tmp_path - auto read_file = [](const fs::path& path) { - std::ifstream t("file.txt"); - std::stringstream buffer; - buffer << t.rdbuf(); - return buffer; - }; - BOOST_REQUIRE(read_file(ref_path).view() == read_file(tmp_path).view()); } FC_LOG_AND_RETHROW() +// Verify that the current version of safety.dat file committed to the repo can be loaded on +// nodeos startup (it is not saved until we actually vote, and voting would change the fsi). +// ----------------------------------------------------------------------------------------- +BOOST_AUTO_TEST_CASE( finalizer_safety_file_serialization_io ) try { + fc::temp_directory tempdir; + auto [cfg, genesis_state] = tester::default_config(tempdir); + + fs::path tmp_path = cfg.finalizers_dir / config::safety_filename; + + auto current_version = my_finalizers_t::current_safety_file_version; + fs::path ref_path = mk_versioned_fsi_file_path(current_version); // the saved file for current_version + + tester t( tempdir, true ); + + fs::create_directory(cfg.finalizers_dir); + fs::copy_file(ref_path, tmp_path, fs::copy_options::none); + auto initial_time = fs::last_write_time(tmp_path); + + std::this_thread::sleep_for(std::chrono::milliseconds{100}); + + // set finalizer, so that the file is overwritten. set the last one so that order is unchanged. + std::vector keys = create_keys(3); + bls_pub_priv_key_map_t local_finalizer_keys; + local_finalizer_keys[keys.back().pubkey_str] = keys.back().privkey_str; + t.control->set_node_finalizer_keys(local_finalizer_keys); + + // Since we didn't vote, the file time should not have changed. + auto last_time = fs::last_write_time(tmp_path); + BOOST_REQUIRE(last_time == initial_time); + +} FC_LOG_AND_RETHROW() + + BOOST_AUTO_TEST_SUITE_END() From 5cd4014cf73e32f0b14e4625e27080d8f33bef8e Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 20 Sep 2024 13:09:08 -0400 Subject: [PATCH 25/49] Fix file comparison and use `fc::read_file_contents` --- unittests/finalizer_tests.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/unittests/finalizer_tests.cpp b/unittests/finalizer_tests.cpp index a66c2d1f82..e3d0f6f40a 100644 --- a/unittests/finalizer_tests.cpp +++ b/unittests/finalizer_tests.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include using namespace eosio; @@ -252,11 +253,10 @@ fs::path mk_versioned_fsi_file_path(uint32_t v) { return fsi_reference_dir / ("safety_v"s + std::to_string(v) + ".dat"); } -std::stringstream read_file(const fs::path& path) { - std::ifstream t("file.txt"); - std::stringstream buffer; - buffer << t.rdbuf(); - return buffer; +std::string read_file(const fs::path& path) { + std::string res; + fc::read_file_contents(path, res); + return res; } BOOST_AUTO_TEST_CASE( finalizer_safety_file_versioning ) try { @@ -323,7 +323,7 @@ BOOST_AUTO_TEST_CASE( finalizer_safety_file_serialization_unchanged ) try { auto tmp_path = tempdir.path() / "new_safety.dat"; create_fsi_reference_file(tmp_path); // save a new file in tmp_path - BOOST_REQUIRE(read_file(ref_path).view() == read_file(tmp_path).view()); + BOOST_REQUIRE(read_file(ref_path) == read_file(tmp_path)); } FC_LOG_AND_RETHROW() From ea5d97d92eaed7d6680f79d34652e30989f206d0 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 20 Sep 2024 13:41:49 -0400 Subject: [PATCH 26/49] Sleep for the minimum period of `std::chrono::file_clock` --- unittests/finalizer_tests.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/unittests/finalizer_tests.cpp b/unittests/finalizer_tests.cpp index e3d0f6f40a..5ef8e658f2 100644 --- a/unittests/finalizer_tests.cpp +++ b/unittests/finalizer_tests.cpp @@ -346,7 +346,13 @@ BOOST_AUTO_TEST_CASE( finalizer_safety_file_serialization_io ) try { fs::copy_file(ref_path, tmp_path, fs::copy_options::none); auto initial_time = fs::last_write_time(tmp_path); - std::this_thread::sleep_for(std::chrono::milliseconds{100}); + // sleep for one period of the file clock + // -------------------------------------- + using file_clock = std::chrono::file_clock; + auto one_period = file_clock::duration(1); + auto sleep_duration = std::chrono::duration_cast(one_period); + + std::this_thread::sleep_for(sleep_duration); // set finalizer, so that the file is overwritten. set the last one so that order is unchanged. std::vector keys = create_keys(3); From 8f4ac9f4bfbae2baae0042cfab63757e6ffbb7e5 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 20 Sep 2024 13:49:26 -0400 Subject: [PATCH 27/49] Add function `sleep_for_n_file_clock_periods` and use in other location. --- unittests/finalizer_tests.cpp | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/unittests/finalizer_tests.cpp b/unittests/finalizer_tests.cpp index 5ef8e658f2..d0fc4ee5f5 100644 --- a/unittests/finalizer_tests.cpp +++ b/unittests/finalizer_tests.cpp @@ -96,6 +96,16 @@ void set_fsi(my_finalizers_t& fset, const std::vector& keys, const F ((fset.set_fsi(keys[I].pubkey, fsi[I])), ...); } +// sleep for n periods of the file clock +// -------------------------------------- +void sleep_for_n_file_clock_periods(uint32_t n) { + using file_clock = std::chrono::file_clock; + auto n_periods = file_clock::duration(n); + auto sleep_duration = std::chrono::duration_cast(n_periods); + + std::this_thread::sleep_for(sleep_duration); +} + BOOST_AUTO_TEST_SUITE(finalizer_tests) BOOST_AUTO_TEST_CASE( basic_finalizer_safety_file_io ) try { @@ -294,7 +304,8 @@ BOOST_AUTO_TEST_CASE( finalizer_safety_file_versioning ) try { auto ref_path = mk_versioned_fsi_file_path(i); auto copy_path = tempdir.path() / ref_path.filename(); fs::copy_file(ref_path, copy_path, fs::copy_options::none); - std::this_thread::sleep_for(std::chrono::milliseconds{10}); + + sleep_for_n_file_clock_periods(2); // first load the reference file in the old format, and then save it in the new version format // ------------------------------------------------------------------------------------------- @@ -346,13 +357,7 @@ BOOST_AUTO_TEST_CASE( finalizer_safety_file_serialization_io ) try { fs::copy_file(ref_path, tmp_path, fs::copy_options::none); auto initial_time = fs::last_write_time(tmp_path); - // sleep for one period of the file clock - // -------------------------------------- - using file_clock = std::chrono::file_clock; - auto one_period = file_clock::duration(1); - auto sleep_duration = std::chrono::duration_cast(one_period); - - std::this_thread::sleep_for(sleep_duration); + sleep_for_n_file_clock_periods(2); // set finalizer, so that the file is overwritten. set the last one so that order is unchanged. std::vector keys = create_keys(3); From e99b7f9c27f4bbd99793a338f560864c27428fe9 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 20 Sep 2024 14:04:49 -0400 Subject: [PATCH 28/49] Fix compilation warnings: In file included from ../plugins/producer_plugin/test/test_block_timing_util.cpp:3: 1153 ../plugins/producer_plugin/include/eosio/producer_plugin/block_timing_util.hpp: In member function 'void block_timing_util::test_calculate_producer_wake_up_time::test_method()': 1154 ../plugins/producer_plugin/include/eosio/producer_plugin/block_timing_util.hpp:132:18: warning: '.std::optional::.std::_Optional_base::_M_payload.std::_Optional_payload::.std::_Optional_payload_base::_M_payload.std::_Optional_payload_base::_Storage::_M_value.fc::time_point::elapsed.fc::microseconds::_count' may be used uninitialized in this function [-Wmaybe-uninitialized] 1155 132 | return {}; 1156 | ^ --- .../test/test_block_timing_util.cpp | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/plugins/producer_plugin/test/test_block_timing_util.cpp b/plugins/producer_plugin/test/test_block_timing_util.cpp index 1c9108710e..dd53c809a7 100644 --- a/plugins/producer_plugin/test/test_block_timing_util.cpp +++ b/plugins/producer_plugin/test/test_block_timing_util.cpp @@ -151,7 +151,7 @@ BOOST_AUTO_TEST_CASE(test_calculate_producer_wake_up_time) { for (uint32_t i = 0; i < static_cast(config::producer_repetitions * active_schedule.size() * 3); ++i) { // 3 rounds to test boundaries block_timestamp_type block_timestamp(prod_round_1st_block_slot + i); auto block_time = block_timestamp.to_time_point(); - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), block_time); + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), std::optional{block_time}); } } { // We have all producers in active_schedule configured, we should produce every block @@ -161,7 +161,7 @@ BOOST_AUTO_TEST_CASE(test_calculate_producer_wake_up_time) { for (uint32_t i = 0; i < static_cast(config::producer_repetitions * active_schedule.size() * 3); ++i) { // 3 rounds to test boundaries block_timestamp_type block_timestamp(prod_round_1st_block_slot + i); auto block_time = block_timestamp.to_time_point(); - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), block_time); + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), std::optional{block_time}); } } { // We have all producers in active_schedule of 21 (plus a couple of extra producers configured), we should produce every block @@ -179,7 +179,7 @@ BOOST_AUTO_TEST_CASE(test_calculate_producer_wake_up_time) { for (uint32_t i = 0; i < static_cast(config::producer_repetitions * active_schedule.size() * 3); ++i) { // 3 rounds to test boundaries block_timestamp_type block_timestamp(prod_round_1st_block_slot + i); auto block_time = block_timestamp.to_time_point(); - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), block_time); + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), std::optional{block_time}); } } { // Tests for when we only have a subset of all active producers, we do not produce all blocks, only produce blocks for our round @@ -194,48 +194,48 @@ BOOST_AUTO_TEST_CASE(test_calculate_producer_wake_up_time) { std::set producers = { "initb"_n }; block_timestamp_type block_timestamp(prod_round_1st_block_slot); auto expected_block_time = block_timestamp_type(prod_round_1st_block_slot + config::producer_repetitions).to_time_point(); - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot-1}, producers, active_schedule, empty_watermarks), expected_block_time); // same - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot+config::producer_repetitions-1}, producers, active_schedule, empty_watermarks), expected_block_time); // same - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot+config::producer_repetitions-2}, producers, active_schedule, empty_watermarks), expected_block_time); // same - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot+config::producer_repetitions-3}, producers, active_schedule, empty_watermarks), expected_block_time); // same + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot-1}, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); // same + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot+config::producer_repetitions-1}, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); // same + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot+config::producer_repetitions-2}, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); // same + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot+config::producer_repetitions-3}, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); // same // current which gives same expected - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot+config::producer_repetitions}, producers, active_schedule, empty_watermarks), expected_block_time); + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot+config::producer_repetitions}, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); expected_block_time += fc::microseconds(config::block_interval_us); - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot+config::producer_repetitions+1}, producers, active_schedule, empty_watermarks), expected_block_time); + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot+config::producer_repetitions+1}, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); // inita is first in the schedule, prod_round_1st_block_slot is block time of the first block, so will return the next block time as that is when current should be produced producers = std::set{ "inita"_n }; block_timestamp = block_timestamp_type{prod_round_1st_block_slot}; expected_block_time = block_timestamp.to_time_point(); - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot-1}, producers, active_schedule, empty_watermarks), expected_block_time); // same - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot-2}, producers, active_schedule, empty_watermarks), expected_block_time); // same - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot-3}, producers, active_schedule, empty_watermarks), expected_block_time); // same + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot-1}, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); // same + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot-2}, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); // same + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp_type{block_timestamp.slot-3}, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); // same for (size_t i = 0; i < config::producer_repetitions; ++i) { expected_block_time = block_timestamp_type(prod_round_1st_block_slot+i).to_time_point(); - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), expected_block_time); + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); block_timestamp = block_timestamp.next(); } expected_block_time = block_timestamp.to_time_point(); - BOOST_CHECK_NE(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), expected_block_time); // end of round, so not the next + BOOST_CHECK_NE(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); // end of round, so not the next // initc is third in the schedule, verify its wake-up time is as expected producers = std::set{ "initc"_n }; block_timestamp = block_timestamp_type(prod_round_1st_block_slot); // expect 2*producer_repetitions since we expect wake-up time to be after the first two rounds expected_block_time = block_timestamp_type(prod_round_1st_block_slot + 2*config::producer_repetitions).to_time_point(); - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), expected_block_time); + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); // inith, initk - configured for 2 of the 21 producers. inith is 8th in schedule, initk is 11th in schedule producers = std::set{ "inith"_n, "initk"_n }; block_timestamp = block_timestamp_type(prod_round_1st_block_slot); // expect to produce after 7 rounds since inith is 8th expected_block_time = block_timestamp_type(prod_round_1st_block_slot + 7*config::producer_repetitions).to_time_point(); - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), expected_block_time); + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); // give it a time after inith otherwise would return inith time block_timestamp = block_timestamp_type(prod_round_1st_block_slot + 8*config::producer_repetitions); // after inith round // expect to produce after 10 rounds since inith is 11th expected_block_time = block_timestamp_type(prod_round_1st_block_slot + 10*config::producer_repetitions).to_time_point(); - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), expected_block_time); + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); // cpu_effort at 50%, initc constexpr fc::microseconds half_cpu_effort = fc::microseconds{eosio::chain::config::block_interval_us / 2u}; @@ -243,18 +243,18 @@ BOOST_AUTO_TEST_CASE(test_calculate_producer_wake_up_time) { block_timestamp = block_timestamp_type(prod_round_1st_block_slot); expected_block_time = block_timestamp_type(prod_round_1st_block_slot + 2*config::producer_repetitions).to_time_point(); // first in round is not affected by cpu effort - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(half_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), expected_block_time); + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(half_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); block_timestamp = block_timestamp_type(prod_round_1st_block_slot + 2*config::producer_repetitions + 1); // second in round is 50% sooner expected_block_time = block_timestamp.to_time_point(); expected_block_time -= fc::microseconds(half_cpu_effort); - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(half_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), expected_block_time); + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(half_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); // third in round is 2*50% sooner block_timestamp = block_timestamp_type(prod_round_1st_block_slot + 2*config::producer_repetitions + 2); // second in round is 50% sooner expected_block_time = block_timestamp.to_time_point(); expected_block_time -= fc::microseconds(2*half_cpu_effort.count()); - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(half_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), expected_block_time); + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(half_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); } { // test watermark std::vector active_schedule{ // 21 @@ -270,15 +270,15 @@ BOOST_AUTO_TEST_CASE(test_calculate_producer_wake_up_time) { // initc, with no watermarks producers = std::set{ "initc"_n }; auto expected_block_time = block_timestamp_type(prod_round_1st_block_slot + 2*config::producer_repetitions).to_time_point(); // without watermark - BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), expected_block_time); + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, empty_watermarks), std::optional{expected_block_time}); // add watermark at first block, first block should not be allowed, wake-up time should be after first block of initc prod_watermarks.consider_new_watermark("initc"_n, 2, block_timestamp_type((prod_round_1st_block_slot + 2*config::producer_repetitions + 1))); // +1 since watermark is in block production time expected_block_time = block_timestamp_type(prod_round_1st_block_slot + 2*config::producer_repetitions + 1).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); + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, prod_watermarks), std::optional{expected_block_time}); // add watermark at first 2 blocks, first & second block should not be allowed, wake-up time should be after second block of initc prod_watermarks.consider_new_watermark("initc"_n, 2, block_timestamp_type((prod_round_1st_block_slot + 2*config::producer_repetitions + 1 + 1))); 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); + BOOST_CHECK_EQUAL(calculate_producer_wake_up_time(full_cpu_effort, 2, block_timestamp, producers, active_schedule, prod_watermarks), std::optional{expected_block_time}); } { // actual example that caused multiple start blocks producer_watermarks prod_watermarks; From 55c7136c64d228468aa281e7a6aee7ac312b543a Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 20 Sep 2024 14:10:00 -0400 Subject: [PATCH 29/49] Fix compilation warning. --- libraries/testing/tester.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/testing/tester.cpp b/libraries/testing/tester.cpp index 4f9068ad0b..ffcc254dd9 100644 --- a/libraries/testing/tester.cpp +++ b/libraries/testing/tester.cpp @@ -375,7 +375,7 @@ namespace eosio::testing { // only block number is signaled, in forking tests will get the same block number more than once. }); [[maybe_unused]] auto accepted_block_header_connection = control->accepted_block_header().connect([this](const block_signal_params& t) { - const auto& [block, id] = t; + [[maybe_unused]] const auto& [block, id] = t; assert(block); assert(_check_signal(id, block_signal::accepted_block_header)); }); From 1340e3b053d2902bec9443dfe57497131824699f Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Fri, 20 Sep 2024 15:14:55 -0400 Subject: [PATCH 30/49] Revert back to return std::vector --- libraries/chain/block_log.cpp | 36 +++++++++---------- libraries/chain/controller.cpp | 14 ++------ .../chain/include/eosio/chain/block_log.hpp | 3 +- .../chain/include/eosio/chain/controller.hpp | 5 ++- plugins/net_plugin/net_plugin.cpp | 2 +- 5 files changed, 24 insertions(+), 36 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 071917f34f..7dc99d8ab5 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -186,13 +186,13 @@ namespace eosio { namespace chain { } template - void read_serialized_block(Stream&& ds, uint64_t block_size, std::vector& buff) { - assert(buff.empty()); + std::vector read_serialized_block(Stream&& ds, uint64_t block_size) { + std::vector buff; + buff.resize(block_size); - try { - buff.resize(block_size); - ds.read(buff.data(), block_size); // append to buff - } catch (...) { buff.clear(); } // restore buff to its original state + ds.read(buff.data(), block_size); + + return buff; } template @@ -498,7 +498,7 @@ namespace eosio { namespace chain { virtual void flush() = 0; virtual signed_block_ptr read_block_by_num(uint32_t block_num) = 0; - virtual void read_serialized_block_by_num(uint32_t block_num, std::vector& buff) = 0; + virtual std::vector read_serialized_block_by_num(uint32_t block_num) = 0; virtual std::optional read_block_header_by_num(uint32_t block_num) = 0; virtual uint32_t version() const = 0; @@ -532,7 +532,7 @@ namespace eosio { namespace chain { void flush() final {} signed_block_ptr read_block_by_num(uint32_t block_num) final { return {}; }; - void read_serialized_block_by_num(uint32_t block_num, std::vector& buff) final {}; + std::vector read_serialized_block_by_num(uint32_t block_num) final { return {}; }; std::optional read_block_header_by_num(uint32_t block_num) final { return {}; }; uint32_t version() const final { return 0; } @@ -577,7 +577,7 @@ namespace eosio { namespace chain { virtual uint32_t working_block_file_first_block_num() { return preamble.first_block_num; } virtual void post_append(uint64_t pos) {} virtual signed_block_ptr retry_read_block_by_num(uint32_t block_num) { return {}; } - virtual void retry_read_serialized_block_by_num(uint32_t block_num, std::vector& buff) {} + virtual std::vector retry_read_serialized_block_by_num(uint32_t block_num) { return {}; } virtual std::optional retry_read_block_header_by_num(uint32_t block_num) { return {}; } void append(const signed_block_ptr& b, const block_id_type& id, @@ -668,16 +668,14 @@ namespace eosio { namespace chain { FC_LOG_AND_RETHROW() } - void read_serialized_block_by_num(uint32_t block_num, std::vector& buff) final { + std::vector read_serialized_block_by_num(uint32_t block_num) final { try { block_pos_size_t pos_size = get_block_position_and_size(block_num); if (pos_size.position != block_log::npos) { block_file.seek(pos_size.position); - read_serialized_block(block_file, pos_size.size, buff); - - return; + return read_serialized_block(block_file, pos_size.size); } - retry_read_serialized_block_by_num(block_num, buff); + return retry_read_serialized_block_by_num(block_num); } FC_LOG_AND_RETHROW() } @@ -1099,14 +1097,14 @@ namespace eosio { namespace chain { return {}; } - void retry_read_serialized_block_by_num(uint32_t block_num, std::vector& buff) final { + std::vector retry_read_serialized_block_by_num(uint32_t block_num) final { uint64_t block_size = 0; auto ds = catalog.ro_stream_and_size_for_block(block_num, block_size); if (ds) { - read_serialized_block(*ds, block_size, buff); - return; + return read_serialized_block(*ds, block_size); } + return {}; } std::optional retry_read_block_header_by_num(uint32_t block_num) final { @@ -1307,9 +1305,9 @@ namespace eosio { namespace chain { return my->read_block_by_num(block_num); } - void block_log::read_serialized_block_by_num(uint32_t block_num, std::vector& buff) const { + std::vector block_log::read_serialized_block_by_num(uint32_t block_num) const { std::lock_guard g(my->mtx); - my->read_serialized_block_by_num(block_num, buff); + return my->read_serialized_block_by_num(block_num); } std::optional block_log::read_block_header_by_num(uint32_t block_num) const { diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 7b02883c22..6e7f4615ba 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -5543,20 +5543,12 @@ signed_block_ptr controller::fetch_block_by_number( uint32_t block_num )const { return my->blog.read_block_by_num(block_num); } FC_CAPTURE_AND_RETHROW( (block_num) ) } -void controller::fetch_serialized_block_by_number( uint32_t block_num, std::vector& buff )const { try { +std::vector controller::fetch_serialized_block_by_number( uint32_t block_num)const { try { if (signed_block_ptr b = my->fork_db_fetch_block_on_best_branch_by_num(block_num)) { - try { - assert(buff.empty()); - const auto sz = fc::raw::pack_size(*b); - buff.resize(sz); - fc::datastream ds(buff.data(), sz); // append to buff - fc::raw::pack(ds, *b); - } catch (...) { buff.clear(); } // restore buff to its original state - - return; + return fc::raw::pack(*b); } - my->blog.read_serialized_block_by_num(block_num, buff); + return my->blog.read_serialized_block_by_num(block_num); } FC_CAPTURE_AND_RETHROW( (block_num) ) } std::optional controller::fetch_block_header_by_number( uint32_t block_num )const { try { diff --git a/libraries/chain/include/eosio/chain/block_log.hpp b/libraries/chain/include/eosio/chain/block_log.hpp index b342ea8eaa..e933cb7920 100644 --- a/libraries/chain/include/eosio/chain/block_log.hpp +++ b/libraries/chain/include/eosio/chain/block_log.hpp @@ -54,8 +54,7 @@ namespace eosio { namespace chain { void reset( const chain_id_type& chain_id, uint32_t first_block_num ); signed_block_ptr read_block_by_num(uint32_t block_num)const; - // Append into `buff` to avoid copying. Assume `buff` is an empty vector. - void read_serialized_block_by_num(uint32_t block_num, std::vector& buff)const; + std::vector read_serialized_block_by_num(uint32_t block_num)const; std::optional read_block_header_by_num(uint32_t block_num)const; std::optional read_block_id_by_num(uint32_t block_num)const; diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index e057d52f2e..e53b492c1c 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -320,9 +320,8 @@ namespace eosio::chain { signed_block_ptr fetch_block_by_number( uint32_t block_num )const; // thread-safe signed_block_ptr fetch_block_by_id( const block_id_type& id )const; - // thread-safe, retrieves serialized signed block and appends it to `buff` (to avoid copying). - // Assume `buff` is an empty vector. - void fetch_serialized_block_by_number( uint32_t block_num, std::vector& buff )const; + // thread-safe, retrieves serialized signed block + std::vector fetch_serialized_block_by_number( uint32_t block_num)const; // thread-safe bool block_exists(const block_id_type& id) const; bool validated_block_exists(const block_id_type& id) const; diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index bebc4464ba..749d92d892 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -1729,7 +1729,7 @@ namespace eosio { controller& cc = my_impl->chain_plug->chain(); std::vector sb; try { - cc.fetch_serialized_block_by_number( num, sb ); // thread-safe + sb = cc.fetch_serialized_block_by_number( num ); // thread-safe } FC_LOG_AND_DROP(); if( !sb.empty() ) { // Skip transmitting block this loop if threshold exceeded From 9120c4876df1f0aa18d8662d490596312966b539 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Fri, 20 Sep 2024 16:35:28 -0400 Subject: [PATCH 31/49] Fix merge issue --- libraries/testing/tester.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/testing/tester.cpp b/libraries/testing/tester.cpp index ffcc254dd9..0382e374e8 100644 --- a/libraries/testing/tester.cpp +++ b/libraries/testing/tester.cpp @@ -374,7 +374,7 @@ namespace eosio::testing { [[maybe_unused]] auto block_start_connection = control->block_start().connect([](block_num_type num) { // only block number is signaled, in forking tests will get the same block number more than once. }); - [[maybe_unused]] auto accepted_block_header_connection = control->accepted_block_header().connect([this](const block_signal_params& t) { + [[maybe_unused]] auto accepted_block_header_connection = control->accepted_block_header().connect([&](const block_signal_params& t) { [[maybe_unused]] const auto& [block, id] = t; assert(block); assert(_check_signal(id, block_signal::accepted_block_header)); From 907d9b04a3a8ce8fa4f7306f96b7bbffc55a75e3 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Fri, 20 Sep 2024 16:55:30 -0400 Subject: [PATCH 32/49] Use EOS_ASSERT for --- libraries/chain/block_log.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 7dc99d8ab5..ad4bcdc4c5 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -628,7 +628,9 @@ namespace eosio { namespace chain { assert(head); uint32_t last_block_num = block_header::num_from_id(head->id); - assert(block_num <= last_block_num); + EOS_ASSERT(block_num <= last_block_num, block_log_exception, + "block_num ${bn} should not be greater than last_block_num ${lbn}", + ("bn", block_num)("lbn", last_block_num)); uint64_t block_size = 0; constexpr uint32_t block_pos_size = sizeof(uint64_t); // size of block position field in the block log file From 1888501ea046e22d05b61b957ac327eb4fa90ec6 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Fri, 20 Sep 2024 17:14:36 -0400 Subject: [PATCH 33/49] use seek_end(0) and tellp to find file size for the already openned block log file --- libraries/chain/block_log.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index ad4bcdc4c5..ec92f26729 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -647,7 +647,8 @@ namespace eosio { namespace chain { } else { // current block is the last block in the file. - auto file_size = std::filesystem::file_size(block_file.get_file_path()); + block_file.seek_end(0); + auto file_size = block_file.tellp(); EOS_ASSERT(file_size > pos + block_pos_size, block_log_exception, "block log file size ${fs} should be greater than current block position ${p} plus block position field size ${bps}", ("fs", file_size)("p", pos)("bps", block_pos_size)); From a9ae2d86dda668baa71213ec7fcd79267eb1ed1a Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Sat, 21 Sep 2024 07:54:47 -0400 Subject: [PATCH 34/49] Use structure binding --- libraries/chain/block_log.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index ec92f26729..946e4764b8 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -673,10 +673,10 @@ namespace eosio { namespace chain { std::vector read_serialized_block_by_num(uint32_t block_num) final { try { - block_pos_size_t pos_size = get_block_position_and_size(block_num); - if (pos_size.position != block_log::npos) { - block_file.seek(pos_size.position); - return read_serialized_block(block_file, pos_size.size); + auto [ position, size ] = get_block_position_and_size(block_num); + if (position != block_log::npos) { + block_file.seek(position); + return read_serialized_block(block_file, size); } return retry_read_serialized_block_by_num(block_num); } From b40b50d2fc905d5c59880ef9731114ee3920dfa8 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Sun, 22 Sep 2024 08:26:57 -0400 Subject: [PATCH 35/49] Simplify get_block_position_and_size --- .../chain/include/eosio/chain/log_catalog.hpp | 42 ++++--------------- 1 file changed, 9 insertions(+), 33 deletions(-) diff --git a/libraries/chain/include/eosio/chain/log_catalog.hpp b/libraries/chain/include/eosio/chain/log_catalog.hpp index 6560721f88..332bd2a0b0 100644 --- a/libraries/chain/include/eosio/chain/log_catalog.hpp +++ b/libraries/chain/include/eosio/chain/log_catalog.hpp @@ -164,48 +164,24 @@ struct log_catalog { } } - block_pos_size_t block_position_size(uint32_t block_num) { - assert(block_num >= log_data.first_block_num()); - uint64_t pos = log_index.nth_block_position(block_num - log_data.first_block_num()); + std::optional get_block_position_and_size(uint32_t block_num) { + std::optional pos = get_block_position(block_num); + + if (!pos) { + return {}; + } constexpr uint32_t block_pos_size = sizeof(uint64_t); uint64_t block_size = 0; assert(block_num <= log_data.last_block_num()); if (block_num < log_data.last_block_num()) { uint64_t next_block_pos = log_index.nth_block_position(block_num + 1 - log_data.first_block_num()); - block_size = next_block_pos - pos - block_pos_size; + block_size = next_block_pos - *pos - block_pos_size; } else { - block_size = log_data.size() - pos - block_pos_size; + block_size = log_data.size() - *pos - block_pos_size; } - return block_pos_size_t { .position = pos, .size = block_size }; - } - - std::optional get_block_position_and_size(uint32_t block_num) { - try { - if (active_index != npos) { - auto active_item = std::next(collection.begin(), active_index); - if (active_item->first <= block_num && block_num <= active_item->second.last_block_num) { - return block_position_size(block_num); - } - } - if (block_num < first_block_num()) - return {}; - - auto it = --collection.upper_bound(block_num); - - if (block_num <= it->second.last_block_num) { - auto name = it->second.filename_base; - log_data.open(name.replace_extension("log")); - log_index.open(name.replace_extension("index")); - active_index = std::distance(collection.begin(), it); - return block_position_size(block_num); - } - return {}; - } catch (...) { - active_index = npos; - return {}; - } + return block_pos_size_t { .position = *pos, .size = block_size }; } fc::datastream* ro_stream_for_block(uint32_t block_num) { From 1075d83aa97ca06c27593b577486583324197a32 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Sun, 22 Sep 2024 08:43:10 -0400 Subject: [PATCH 36/49] add back peer_dlog in enqueue_block --- plugins/net_plugin/net_plugin.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 749d92d892..40467a49c9 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -1062,7 +1062,7 @@ namespace eosio { void blk_send_branch( uint32_t msg_head_num, uint32_t lib_num, uint32_t head_num ); void enqueue( const net_message &msg ); - size_t enqueue_block( const std::vector& sb, bool to_sync_queue = false); + size_t enqueue_block( const std::vector& sb, uint32_t block_num, bool to_sync_queue = false); void enqueue_buffer( const std::shared_ptr>& send_buffer, go_away_reason close_after_send, bool to_sync_queue = false); @@ -1750,7 +1750,7 @@ namespace eosio { } } block_sync_throttling = false; - auto sent = enqueue_block( sb, true ); + auto sent = enqueue_block( sb, num, true ); block_sync_total_bytes_sent += sent; block_sync_frame_bytes_sent += sent; ++peer_requested->last; @@ -1910,7 +1910,8 @@ namespace eosio { } // called from connection strand - size_t connection::enqueue_block( const std::vector& b, bool to_sync_queue) { + size_t connection::enqueue_block( const std::vector& b, uint32_t block_num, bool to_sync_queue ) { + peer_dlog( this, "enqueue block ${num}", ("num", block_num) ); verify_strand_in_this_thread( strand, __func__, __LINE__ ); block_buffer_factory buff_factory; From 692c1d06b03ef12a85bbc58bfcbc1f4636fdff05 Mon Sep 17 00:00:00 2001 From: Lin Huang Date: Mon, 23 Sep 2024 13:57:08 -0400 Subject: [PATCH 37/49] Add tests for get_serialized_block_by_number --- unittests/block_log_get_block_tests.cpp | 112 ++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 unittests/block_log_get_block_tests.cpp diff --git a/unittests/block_log_get_block_tests.cpp b/unittests/block_log_get_block_tests.cpp new file mode 100644 index 0000000000..61019859af --- /dev/null +++ b/unittests/block_log_get_block_tests.cpp @@ -0,0 +1,112 @@ +#include +#include // for fc_exception_message_contains + +#include + +#include + +using namespace eosio::chain; +using namespace eosio::testing; + +struct block_log_get_block_fixture { + block_log_get_block_fixture() { + block_dir = dir.path(); + + log.emplace(block_dir); + + log->reset(genesis_state(), std::make_shared()); + BOOST_REQUIRE_EQUAL(log->first_block_num(), 1u); + BOOST_REQUIRE_EQUAL(log->head()->block_num(), 1u); + + for(uint32_t i = 2; i < last_block_num + 1; ++i) { + auto p = std::make_shared(); + p->previous._hash[0] = fc::endian_reverse_u32(i-1); + log->append(p, p->calculate_id()); + } + BOOST_REQUIRE_EQUAL(log->head()->block_num(), last_block_num); + }; + + void test_read_serialized_block(const block_log& blog, uint32_t block_num) { + // read the serialized block + auto serialized_block = blog.read_serialized_block_by_num(block_num); + + // the serialized block can be deserialized + BOOST_REQUIRE_NO_THROW(fc::raw::unpack(serialized_block)); + + // read the signed block by regular read_block_by_num + signed_block_ptr block = blog.read_block_by_num(block_num); + BOOST_REQUIRE(block); + + // the serialized block should match the signed block's serialized form + BOOST_REQUIRE(serialized_block == fc::raw::pack(*block)); + } + + fc::temp_directory dir; + std::filesystem::path block_dir; + std::optional log; + static constexpr uint32_t last_block_num = 50; +}; + +BOOST_AUTO_TEST_SUITE(block_log_get_block_tests) + +BOOST_FIXTURE_TEST_CASE(basic_block_log, block_log_get_block_fixture) try { + // test reading a non-last-block + test_read_serialized_block(*log, last_block_num - 2); + + // test reading last block + test_read_serialized_block(*log, last_block_num); +} FC_LOG_AND_RETHROW() + +BOOST_FIXTURE_TEST_CASE(splitted_block_log, block_log_get_block_fixture) try { + uint32_t stride = last_block_num / 2; + auto retained_dir = block_dir / "retained"; + + block_log::split_blocklog(block_dir, retained_dir, stride); + + std::filesystem::remove(block_dir / "blocks.log"); + std::filesystem::remove(block_dir / "blocks.index"); + + block_log blog(block_dir, partitioned_blocklog_config{ .retained_dir = retained_dir }); + + // test reading a block in the first partitioned log + test_read_serialized_block(blog, stride - 1); + + // test reading a block in the second partitioned log + test_read_serialized_block(blog, stride + 1); +} FC_LOG_AND_RETHROW() + +BOOST_FIXTURE_TEST_CASE(nonexisting_block_num, block_log_get_block_fixture) try { + // read a non-existing block + auto serialized_block = log->read_serialized_block_by_num(last_block_num + 1); + + // should return an empty vector of char + BOOST_REQUIRE(serialized_block.empty()); +} FC_LOG_AND_RETHROW() + +BOOST_FIXTURE_TEST_CASE(corrupted_next_block_position, block_log_get_block_fixture) try { + // intentionally modify block position for next block (which is the last block) + uint64_t bad_pos = sizeof(uint64_t) * (last_block_num); + fc::datastream index_file; + index_file.set_file_path(block_dir / "blocks.index"); + index_file.open(fc::cfile::update_rw_mode); + index_file.seek_end(-sizeof(uint64_t)); + index_file.write((char*)&bad_pos, sizeof(bad_pos)); + index_file.flush(); + index_file.close(); + + BOOST_CHECK_EXCEPTION(log->read_serialized_block_by_num(last_block_num - 1), + block_log_exception, + fc_exception_message_contains("next block position")); +} FC_LOG_AND_RETHROW() + +BOOST_FIXTURE_TEST_CASE(corrupted_file_size, block_log_get_block_fixture) try { + // corrupt file size by truncating it + auto new_size = log->get_block_pos(last_block_num) + sizeof(uint64_t); + std::filesystem::resize_file(block_dir / "blocks.log", new_size); + + BOOST_CHECK_EXCEPTION(log->read_serialized_block_by_num(last_block_num), + block_log_exception, + fc_exception_message_contains("block log file size")); +} FC_LOG_AND_RETHROW() + +BOOST_AUTO_TEST_SUITE_END() From b33ec51f5cb4c659105939492f8658927c409fb0 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Mon, 23 Sep 2024 17:17:04 -0400 Subject: [PATCH 38/49] bump abieos submodule to main's head w/ ship changes --- tests/abieos | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/abieos b/tests/abieos index d2586071be..4663982fc2 160000 --- a/tests/abieos +++ b/tests/abieos @@ -1 +1 @@ -Subproject commit d2586071be1b3383ac6c38a8beec91a0901ea458 +Subproject commit 4663982fc2c61d85a5d56e2277862ee46699ac3b From 52f5026e5ea4690be70e2ad524492634dc21d5bd Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 23 Sep 2024 16:19:42 -0500 Subject: [PATCH 39/49] GH-789 Add verification to startup/catchup test that node does not sync too far ahead. Also increase number of blocks produced at start so that nodes are in lib catchup during sync. Verify all nodes are able to sync without a block log. --- tests/TestHarness/Node.py | 21 +++++++++++ tests/nodeos_startup_catchup.py | 67 ++++++++++++++++++++++++--------- 2 files changed, 70 insertions(+), 18 deletions(-) diff --git a/tests/TestHarness/Node.py b/tests/TestHarness/Node.py index 0ccedfda0c..8cfab82bac 100644 --- a/tests/TestHarness/Node.py +++ b/tests/TestHarness/Node.py @@ -687,6 +687,27 @@ def findInLog(self, searchStr): return True return False + def linesInLog(self, searchStr): + dataDir=Utils.getNodeDataDir(self.nodeId) + files=Node.findStderrFiles(dataDir) + lines=[] + for file in files: + with open(file, 'r') as f: + for line in f: + if searchStr in line: + lines.append(line) + return lines + + def countInLog(self, searchStr) -> int: + dataDir=Utils.getNodeDataDir(self.nodeId) + files=Node.findStderrFiles(dataDir) + count = 0 + for file in files: + with open(file, 'r') as f: + contents = f.read() + count += contents.count(searchStr) + return count + # verify only one or two 'Starting block' per block number unless block is restarted def verifyStartingBlockMessages(self): dataDir=Utils.getNodeDataDir(self.nodeId) diff --git a/tests/nodeos_startup_catchup.py b/tests/nodeos_startup_catchup.py index feca64f4dc..669e1d7233 100755 --- a/tests/nodeos_startup_catchup.py +++ b/tests/nodeos_startup_catchup.py @@ -135,9 +135,9 @@ def waitForNodeStarted(node): transactionsPerBlock=targetTpsPerGenerator*trxGeneratorCnt*timePerBlock/1000 steadyStateWait=20 startBlockNum=blockNum+steadyStateWait - numBlocks=20 + numBlocks=400 endBlockNum=startBlockNum+numBlocks - waitForBlock(node0, endBlockNum) + waitForBlock(node0, endBlockNum, timeout=numBlocks) steadyStateWindowTrxs=0 steadyStateAvg=0 steadyStateWindowBlks=0 @@ -182,23 +182,18 @@ def waitForNodeStarted(node): Print("Shutdown catchup node and validate exit code") catchupNode.interruptAndVerifyExitStatus(60) - # every other catchup make a lib catchup - if catchup_num % 2 == 0: - Print(f"Wait for producer to advance lib past head of catchup {catchupHead}") - # catchupHead+5 to allow for advancement of head during shutdown of catchupNode - waitForBlock(node0, catchupHead+5, timeout=twoRoundsTimeout*2, blockType=BlockType.lib) + Print(f"Wait for producer to advance lib past head of catchup {catchupHead}") + # catchupHead+5 to allow for advancement of head during shutdown of catchupNode + waitForBlock(node0, catchupHead+5, timeout=twoRoundsTimeout*2, blockType=BlockType.lib) Print("Restart catchup node") - addSwapFlags = None - if catchup_num % 3 == 0: - addSwapFlags = {"--block-log-retain-blocks": "0", "--delete-all": ""} - catchupNode.relaunch(skipGenesis=False, addSwapFlags=addSwapFlags) + catchupNode.relaunch(skipGenesis=False) waitForNodeStarted(catchupNode) lastCatchupLibNum=lib(catchupNode) Print("Verify catchup node is advancing") # verify catchup node is advancing to producer - waitForBlock(catchupNode, lastCatchupLibNum+1, timeout=twoRoundsTimeout, blockType=BlockType.lib) + waitForBlock(catchupNode, lastLibNum, timeout=twoRoundsTimeout, blockType=BlockType.lib) Print("Verify producer is still advancing LIB") lastLibNum=lib(node0) @@ -209,20 +204,56 @@ def waitForNodeStarted(node): # verify catchup node is advancing to producer waitForBlock(catchupNode, lastLibNum, timeout=(numBlocksToCatchup/2 + 60), blockType=BlockType.lib) catchupNode.interruptAndVerifyExitStatus(60) + + Print("Verify catchup without a block log") + addSwapFlags = {"--block-log-retain-blocks": "0", "--delete-all": ""} + catchupNode.relaunch(skipGenesis=False, addSwapFlags=addSwapFlags) + waitForNodeStarted(catchupNode) + lastCatchupLibNum=lib(catchupNode) + + Print("Verify catchup node is advancing without block log") + # verify catchup node is advancing to producer + waitForBlock(catchupNode, lastLibNum+1, timeout=twoRoundsTimeout, blockType=BlockType.lib) + catchupNode.interruptAndVerifyExitStatus(60) catchupNode.popenProc=None - logFile = Utils.getNodeDataDir(catchupNodeNum) + "/stderr.txt" - f = open(logFile) - contents = f.read() + # Verify not syncing ahead of sync-fetch-span + sync_fetch_span = 1000 # default + irreversible = False + if catchupNode.nodeId in specificExtraNodeosArgs: + m = re.search(r"sync-fetch-span (\d+)", specificExtraNodeosArgs[catchupNode.nodeId]) + if m is not None: + sync_fetch_span = int(m.group(1)) + irreversible = re.search(r"irreversible", specificExtraNodeosArgs[catchupNode.nodeId]) is not None + Print(f"Verify request span for sync-fetch-span {sync_fetch_span} of {catchupNode.data_dir}") + lines = catchupNode.linesInLog("requesting range") + for line in lines: + m = re.search(r"requesting range (\d+) to (\d+), fhead (\d+), lib (\d+)", line) + if m is not None: + startBlockNum=int(m.group(1)) + endBlockNum=int(m.group(2)) + fhead=int(m.group(3)) + libNum=int(m.group(4)) + if endBlockNum-startBlockNum > sync_fetch_span: + errorExit(f"Requested range exceeds sync-fetch-span {sync_fetch_span}: {line}") + if irreversible: + # for now just use a larger tolerance, later when the logs include calculated lib this can be more precise + # See https://github.com/AntelopeIO/spring/issues/806 + if endBlockNum > fhead and fhead > libNum and endBlockNum - fhead > (sync_fetch_span*10): + errorExit(f"Requested range too far head of fork head {fhead} in irreversible mode, sync-fetch-span {sync_fetch_span}: {line}") + else: + if endBlockNum > fhead and fhead > libNum and endBlockNum - fhead > (sync_fetch_span*2-1): + errorExit(f"Requested range too far head of fork head {fhead} sync-fetch-span {sync_fetch_span}: {line}") + # See https://github.com/AntelopeIO/spring/issues/81 for fix to reduce the number of expected unlinkable blocks # Test verifies LIB is advancing, check to see that not too many unlinkable block exceptions are generated # while syncing up to head. - numUnlinkable = contents.count("3030001 unlinkable_block_exception: Unlinkable block") + numUnlinkable = catchupNode.countInLog("unlinkable_block") numUnlinkableAllowed = 500 - Print(f"Node{catchupNodeNum} has {numUnlinkable} unlinkable_block_exception in {logFile}") + Print(f"Node{catchupNodeNum} has {numUnlinkable} unlinkable_block in {catchupNode.data_dir}") if numUnlinkable > numUnlinkableAllowed: errorExit(f"Node{catchupNodeNum} has {numUnlinkable} which is more than the configured " - f"allowed {numUnlinkableAllowed} unlinkable blocks: {logFile}.") + f"allowed {numUnlinkableAllowed} unlinkable blocks: {catchupNode.data_dir}.") testSuccessful=True From 41df1c5388fc0028d07c2242afde3e5eeb854333 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 23 Sep 2024 16:21:11 -0500 Subject: [PATCH 40/49] GH-789 Do not allow sync ahead until blocks are in the forkdb to evaluate correctly. --- plugins/net_plugin/net_plugin.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index a77e19cc57..cf827c963a 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2192,13 +2192,15 @@ namespace eosio { controller& cc = my_impl->chain_plug->chain(); if (cc.get_read_mode() == db_read_mode::IRREVERSIBLE) { auto forkdb_head = cc.fork_db_head(); - auto calculated_lib = forkdb_head.irreversible_blocknum(); - auto num_blocks_that_can_be_applied = calculated_lib > head_num ? calculated_lib - head_num : 0; - if (num_blocks_that_can_be_applied < sync_fetch_span) { - if (head_num ) - fc_ilog(logger, "sync ahead allowed past sync-fetch-span ${sp}, block ${bn} for paused LIB ${l}, chain_lib ${cl}, forkdb size ${s}", - ("bn", blk_num)("sp", sync_fetch_span)("l", calculated_lib)("cl", head_num)("s", cc.fork_db_size())); - return true; + if (forkdb_head.block_num() >= blk_num) { + auto calculated_lib = forkdb_head.irreversible_blocknum(); + auto num_blocks_that_can_be_applied = calculated_lib > head_num ? calculated_lib - head_num : 0; + if (num_blocks_that_can_be_applied < sync_fetch_span) { + if (head_num ) + fc_ilog(logger, "sync ahead allowed past sync-fetch-span ${sp}, block ${bn} for paused LIB ${l}, chain_lib ${cl}, forkdb size ${s}", + ("bn", blk_num)("sp", sync_fetch_span)("l", calculated_lib)("cl", head_num)("s", cc.fork_db_size())); + return true; + } } } From 71a142e3aaf27ecd2644918fdd5db37ad27a5f1f Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 24 Sep 2024 06:55:24 -0500 Subject: [PATCH 41/49] GH-789 Use offset instead of absolute check --- plugins/net_plugin/net_plugin.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index cf827c963a..c39b8bbb73 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2192,15 +2192,15 @@ namespace eosio { controller& cc = my_impl->chain_plug->chain(); if (cc.get_read_mode() == db_read_mode::IRREVERSIBLE) { auto forkdb_head = cc.fork_db_head(); - if (forkdb_head.block_num() >= blk_num) { - auto calculated_lib = forkdb_head.irreversible_blocknum(); - auto num_blocks_that_can_be_applied = calculated_lib > head_num ? calculated_lib - head_num : 0; - if (num_blocks_that_can_be_applied < sync_fetch_span) { - if (head_num ) - fc_ilog(logger, "sync ahead allowed past sync-fetch-span ${sp}, block ${bn} for paused LIB ${l}, chain_lib ${cl}, forkdb size ${s}", - ("bn", blk_num)("sp", sync_fetch_span)("l", calculated_lib)("cl", head_num)("s", cc.fork_db_size())); - return true; - } + auto calculated_lib = forkdb_head.irreversible_blocknum(); + auto num_blocks_that_can_be_applied = calculated_lib > head_num ? calculated_lib - head_num : 0; + // add blocks that can potentially be applied as they are not in the forkdb yet + num_blocks_that_can_be_applied += blk_num > forkdb_head.block_num() ? blk_num - forkdb_head.block_num() : 0; + if (num_blocks_that_can_be_applied < sync_fetch_span) { + if (head_num ) + fc_ilog(logger, "sync ahead allowed past sync-fetch-span ${sp}, block ${bn} for paused LIB ${l}, chain_lib ${cl}, forkdb size ${s}", + ("bn", blk_num)("sp", sync_fetch_span)("l", calculated_lib)("cl", head_num)("s", cc.fork_db_size())); + return true; } } From 32c5b87bcb4ad4e420967f11d03762f4159f0976 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 24 Sep 2024 09:21:01 -0500 Subject: [PATCH 42/49] GH-815 Add a test that kills ship clients and make sure nodeos is unaffected --- tests/CMakeLists.txt | 3 + tests/ship_kill_client_test.py | 134 +++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+) create mode 100755 tests/ship_kill_client_test.py diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index be8d73d774..a5ae9f0aa6 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -55,6 +55,7 @@ configure_file(${CMAKE_CURRENT_SOURCE_DIR}/cli_test.py ${CMAKE_CURRENT_BINARY_DI configure_file(${CMAKE_CURRENT_SOURCE_DIR}/ship_reqs_across_svnn_test.py ${CMAKE_CURRENT_BINARY_DIR}/ship_reqs_across_svnn_test.py COPYONLY) configure_file(${CMAKE_CURRENT_SOURCE_DIR}/ship_test.py ${CMAKE_CURRENT_BINARY_DIR}/ship_test.py COPYONLY) configure_file(${CMAKE_CURRENT_SOURCE_DIR}/ship_streamer_test.py ${CMAKE_CURRENT_BINARY_DIR}/ship_streamer_test.py COPYONLY) +configure_file(${CMAKE_CURRENT_SOURCE_DIR}/ship_kill_client_test.py ${CMAKE_CURRENT_BINARY_DIR}/ship_kill_client_test.py COPYONLY) configure_file(${CMAKE_CURRENT_SOURCE_DIR}/bridge_for_fork_test_shape.json ${CMAKE_CURRENT_BINARY_DIR}/bridge_for_fork_test_shape.json COPYONLY) configure_file(${CMAKE_CURRENT_SOURCE_DIR}/lib_advance_test.py ${CMAKE_CURRENT_BINARY_DIR}/lib_advance_test.py COPYONLY) configure_file(${CMAKE_CURRENT_SOURCE_DIR}/http_plugin_test.py ${CMAKE_CURRENT_BINARY_DIR}/http_plugin_test.py COPYONLY) @@ -187,6 +188,8 @@ add_test(NAME ship_streamer_if_test COMMAND tests/ship_streamer_test.py -v --num set_property(TEST ship_streamer_if_test PROPERTY LABELS long_running_tests) add_test(NAME ship_streamer_if_fetch_finality_data_test COMMAND tests/ship_streamer_test.py -v --num-clients 10 --activate-if --finality-data-history ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST ship_streamer_if_fetch_finality_data_test PROPERTY LABELS long_running_tests) +add_test(NAME ship_kill_client_test COMMAND tests/ship_kill_client_test.py -v --num-clients 10 ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) +set_property(TEST ship_kill_client_test PROPERTY LABELS nonparallelizable_tests) add_test(NAME p2p_dawn515_test COMMAND tests/p2p_tests/dawn_515/test.sh WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST p2p_dawn515_test PROPERTY LABELS nonparallelizable_tests) diff --git a/tests/ship_kill_client_test.py b/tests/ship_kill_client_test.py new file mode 100755 index 0000000000..9a28718900 --- /dev/null +++ b/tests/ship_kill_client_test.py @@ -0,0 +1,134 @@ +#!/usr/bin/env python3 + +import time +import json +import os +import shutil +import signal +import sys + +from TestHarness import Account, Cluster, TestHelper, Utils, WalletMgr +from TestHarness.TestHelper import AppArgs + +############################################################### +# ship_kill_client_test +# +# Setup a nodeos with SHiP (state_history_plugin). +# Connect a number of clients and then kill the clients and shutdown nodoes. +# nodeos should exit cleanly and not hang or SEGfAULT. +# +############################################################### + +Print=Utils.Print + +appArgs = AppArgs() +extraArgs = appArgs.add(flag="--num-clients", type=int, help="How many ship_streamers should be started", default=1) +args = TestHelper.parse_args({"--activate-if","--dump-error-details","--keep-logs","-v","--leave-running","--unshared"}, applicationSpecificArgs=appArgs) + +Utils.Debug=args.v +cluster=Cluster(unshared=args.unshared, keepRunning=args.leave_running, keepLogs=args.keep_logs) +activateIF=args.activate_if +dumpErrorDetails=args.dump_error_details +walletPort=TestHelper.DEFAULT_WALLET_PORT + +totalProducerNodes=2 +totalNonProducerNodes=1 +totalNodes=totalProducerNodes+totalNonProducerNodes + +walletMgr=WalletMgr(True, port=walletPort) +testSuccessful=False + +WalletdName=Utils.EosWalletName +shipTempDir=None + +try: + TestHelper.printSystemInfo("BEGIN") + + cluster.setWalletMgr(walletMgr) + Print("Stand up cluster") + + shipNodeNum = 2 + specificExtraNodeosArgs={} + specificExtraNodeosArgs[shipNodeNum]="--plugin eosio::state_history_plugin --trace-history --chain-state-history --finality-data-history --state-history-stride 200 --plugin eosio::net_api_plugin --plugin eosio::producer_api_plugin " + + if cluster.launch(pnodes=totalProducerNodes, loadSystemContract=False, + totalNodes=totalNodes, totalProducers=totalProducerNodes, activateIF=activateIF, biosFinalizer=False, + specificExtraNodeosArgs=specificExtraNodeosArgs) is False: + Utils.cmdError("launcher") + Utils.errorExit("Failed to stand up cluster.") + + # verify nodes are in sync and advancing + cluster.waitOnClusterSync(blockAdvancing=5) + Print("Cluster in Sync") + + prodNode0 = cluster.getNode(0) + nonprodNode = cluster.getNode(1) + shipNode = cluster.getNode(shipNodeNum) + + # cluster.waitOnClusterSync(blockAdvancing=3) + start_block_num = shipNode.getBlockNum() + + #verify nodes are in sync and advancing + cluster.waitOnClusterSync(blockAdvancing=3) + Print("Shutdown unneeded bios node") + cluster.biosNode.kill(signal.SIGTERM) + + Print("Configure and launch txn generators") + targetTpsPerGenerator = 10 + testTrxGenDurationSec=60*60 + numTrxGenerators=2 + cluster.launchTrxGenerators(contractOwnerAcctName=cluster.eosioAccount.name, acctNamesList=[cluster.defproduceraAccount.name, cluster.defproducerbAccount.name], + acctPrivKeysList=[cluster.defproduceraAccount.activePrivateKey,cluster.defproducerbAccount.activePrivateKey], nodeId=nonprodNode.nodeId, + tpsPerGenerator=targetTpsPerGenerator, numGenerators=numTrxGenerators, durationSec=testTrxGenDurationSec, + waitToComplete=False) + + status = cluster.waitForTrxGeneratorsSpinup(nodeId=nonprodNode.nodeId, numGenerators=numTrxGenerators) + assert status is not None and status is not False, "ERROR: Failed to spinup Transaction Generators" + + nonprodNode.waitForProducer("defproducera") + + block_range = 100000 # we are going to kill the client, so just make this a huge number + end_block_num = start_block_num + block_range + + shipClient = "tests/ship_streamer" + cmd = f"{shipClient} --start-block-num {start_block_num} --end-block-num {end_block_num} --fetch-block --fetch-traces --fetch-deltas --fetch-finality-data" + if Utils.Debug: Utils.Print(f"cmd: {cmd}") + clients = [] + files = [] + shipTempDir = os.path.join(Utils.DataDir, "ship") + os.makedirs(shipTempDir, exist_ok = True) + shipClientFilePrefix = os.path.join(shipTempDir, "client") + + starts = [] + for i in range(0, args.num_clients): + start = time.perf_counter() + outFile = open(f"{shipClientFilePrefix}{i}.out", "w") + errFile = open(f"{shipClientFilePrefix}{i}.err", "w") + Print(f"Start client {i}") + popen=Utils.delayedCheckOutput(cmd, stdout=outFile, stderr=errFile) + starts.append(time.perf_counter()) + clients.append((popen, cmd)) + files.append((outFile, errFile)) + Print(f"Client {i} started, Ship node head is: {shipNode.getBlockNum()}") + + + # allow time for all clients to connect + shipNode.waitForHeadToAdvance(5) + shipNode.waitForLibToAdvance() + + Print(f"Kill all {args.num_clients} clients") + for index, (popen, _), (out, err), start in zip(range(len(clients)), clients, files, starts): + popen.kill() + + shipNode.kill(signal.SIGTERM) + assert not shipNode.verifyAlive(), "ship node did not shutdown" + + testSuccessful = True +finally: + TestHelper.shutdown(cluster, walletMgr, testSuccessful=testSuccessful, dumpErrorDetails=dumpErrorDetails) + if shipTempDir is not None: + if testSuccessful and not args.keep_logs: + shutil.rmtree(shipTempDir, ignore_errors=True) + +errorCode = 0 if testSuccessful else 1 +exit(errorCode) From c09ec2a7a3881221ce7fae677bce93f14ba950ce Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 24 Sep 2024 09:50:08 -0500 Subject: [PATCH 43/49] GH-815 Attempt to make test fail more often by increasing number of clients --- tests/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index a5ae9f0aa6..0f5c17d10d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -188,7 +188,7 @@ add_test(NAME ship_streamer_if_test COMMAND tests/ship_streamer_test.py -v --num set_property(TEST ship_streamer_if_test PROPERTY LABELS long_running_tests) add_test(NAME ship_streamer_if_fetch_finality_data_test COMMAND tests/ship_streamer_test.py -v --num-clients 10 --activate-if --finality-data-history ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST ship_streamer_if_fetch_finality_data_test PROPERTY LABELS long_running_tests) -add_test(NAME ship_kill_client_test COMMAND tests/ship_kill_client_test.py -v --num-clients 10 ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) +add_test(NAME ship_kill_client_test COMMAND tests/ship_kill_client_test.py -v --activate-if --num-clients 20 ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST ship_kill_client_test PROPERTY LABELS nonparallelizable_tests) add_test(NAME p2p_dawn515_test COMMAND tests/p2p_tests/dawn_515/test.sh WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) From 6350739b577f5bbcab03189c94779a6646ab4526 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 24 Sep 2024 10:43:14 -0500 Subject: [PATCH 44/49] GH-815 shutdown nodeos during shutdown of clients --- tests/ship_kill_client_test.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tests/ship_kill_client_test.py b/tests/ship_kill_client_test.py index 9a28718900..039d3dd46a 100755 --- a/tests/ship_kill_client_test.py +++ b/tests/ship_kill_client_test.py @@ -99,14 +99,12 @@ os.makedirs(shipTempDir, exist_ok = True) shipClientFilePrefix = os.path.join(shipTempDir, "client") - starts = [] for i in range(0, args.num_clients): start = time.perf_counter() outFile = open(f"{shipClientFilePrefix}{i}.out", "w") errFile = open(f"{shipClientFilePrefix}{i}.err", "w") Print(f"Start client {i}") popen=Utils.delayedCheckOutput(cmd, stdout=outFile, stderr=errFile) - starts.append(time.perf_counter()) clients.append((popen, cmd)) files.append((outFile, errFile)) Print(f"Client {i} started, Ship node head is: {shipNode.getBlockNum()}") @@ -116,12 +114,12 @@ shipNode.waitForHeadToAdvance(5) shipNode.waitForLibToAdvance() - Print(f"Kill all {args.num_clients} clients") - for index, (popen, _), (out, err), start in zip(range(len(clients)), clients, files, starts): + Print(f"Kill all {args.num_clients} clients and ship node") + for index, (popen, _) in zip(range(len(clients)), clients): popen.kill() - - shipNode.kill(signal.SIGTERM) - assert not shipNode.verifyAlive(), "ship node did not shutdown" + if index == len(clients)/2: + shipNode.kill(signal.SIGTERM) + assert not shipNode.verifyAlive(), "ship node did not shutdown" testSuccessful = True finally: From 80d5aef183faf51d8cedfc43c38b8804e4f33e99 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 24 Sep 2024 11:16:39 -0500 Subject: [PATCH 45/49] GH-815 Use state_history_plugin thread for socket --- plugins/state_history_plugin/state_history_plugin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/state_history_plugin/state_history_plugin.cpp b/plugins/state_history_plugin/state_history_plugin.cpp index 89eb78b9d6..f8f3e6b484 100644 --- a/plugins/state_history_plugin/state_history_plugin.cpp +++ b/plugins/state_history_plugin/state_history_plugin.cpp @@ -101,7 +101,7 @@ struct state_history_plugin_impl { void create_listener(const std::string& address) { const boost::posix_time::milliseconds accept_timeout(200); // connections set must only be modified by main thread; run listener on main thread to avoid needing another post() - fc::create_listener(app().get_io_service(), _log, accept_timeout, address, "", [this](Protocol::socket&& socket) { + fc::create_listener(thread_pool.get_executor(), _log, accept_timeout, address, "", [this](Protocol::socket&& socket) { catch_and_log([this, &socket]() { connections.emplace(new session(std::move(socket), boost::asio::make_strand(thread_pool.get_executor()), chain_plug->chain(), trace_log, chain_state_log, finality_data_log, From 49492dced540eb14def5926535166360f87796de Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 24 Sep 2024 12:49:02 -0500 Subject: [PATCH 46/49] GH-815 Modify connections on the main thread. Use explicit strand for ship thread. --- .../eosio/state_history_plugin/session.hpp | 2 +- .../state_history_plugin.cpp | 33 ++++++++++--------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/plugins/state_history_plugin/include/eosio/state_history_plugin/session.hpp b/plugins/state_history_plugin/include/eosio/state_history_plugin/session.hpp index 572c2d3ec6..324c5db818 100644 --- a/plugins/state_history_plugin/include/eosio/state_history_plugin/session.hpp +++ b/plugins/state_history_plugin/include/eosio/state_history_plugin/session.hpp @@ -327,7 +327,7 @@ class session final : public session_base { std::optional& chain_state_log; std::optional& finality_data_log; - GetBlockID get_block_id; + GetBlockID get_block_id; // call from main app thread GetBlock get_block; ///these items might be used on either the strand or main thread diff --git a/plugins/state_history_plugin/state_history_plugin.cpp b/plugins/state_history_plugin/state_history_plugin.cpp index f8f3e6b484..a64d29ff6b 100644 --- a/plugins/state_history_plugin/state_history_plugin.cpp +++ b/plugins/state_history_plugin/state_history_plugin.cpp @@ -101,21 +101,24 @@ struct state_history_plugin_impl { void create_listener(const std::string& address) { const boost::posix_time::milliseconds accept_timeout(200); // connections set must only be modified by main thread; run listener on main thread to avoid needing another post() - fc::create_listener(thread_pool.get_executor(), _log, accept_timeout, address, "", [this](Protocol::socket&& socket) { - catch_and_log([this, &socket]() { - connections.emplace(new session(std::move(socket), boost::asio::make_strand(thread_pool.get_executor()), chain_plug->chain(), - trace_log, chain_state_log, finality_data_log, - [this](const chain::block_num_type block_num) { - return get_block_id(block_num); - }, - [this](const chain::block_id_type& block_id) { - return chain_plug->chain().fetch_block_by_id(block_id); - }, - [this](session_base* conn) { - boost::asio::post(app().get_io_service(), [conn, this]() { - connections.erase(connections.find(conn)); - }); - }, _log)); + auto strand = boost::asio::make_strand(thread_pool.get_executor()); + fc::create_listener(strand, _log, accept_timeout, address, "", [this, strand](Protocol::socket&& socket) { + boost::asio::post(app().get_io_service(), [this, strand, socket{std::move(socket)}]() mutable { + catch_and_log([this, &socket, &strand]() { + connections.emplace(new session(std::move(socket), std::move(strand), chain_plug->chain(), + trace_log, chain_state_log, finality_data_log, + [this](const chain::block_num_type block_num) { + return get_block_id(block_num); + }, + [this](const chain::block_id_type& block_id) { + return chain_plug->chain().fetch_block_by_id(block_id); + }, + [this](session_base* conn) { + boost::asio::post(app().get_io_service(), [conn, this]() { + connections.erase(connections.find(conn)); + }); + }, _log)); + }); }); }); } From 9736f6f06b2da7d78c17ca9fd24bfc463cdadb23 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 24 Sep 2024 12:58:17 -0500 Subject: [PATCH 47/49] GH-815 Make the test always run in Savanna mode --- tests/CMakeLists.txt | 2 +- tests/ship_kill_client_test.py | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0f5c17d10d..a99d2b7d96 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -188,7 +188,7 @@ add_test(NAME ship_streamer_if_test COMMAND tests/ship_streamer_test.py -v --num set_property(TEST ship_streamer_if_test PROPERTY LABELS long_running_tests) add_test(NAME ship_streamer_if_fetch_finality_data_test COMMAND tests/ship_streamer_test.py -v --num-clients 10 --activate-if --finality-data-history ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST ship_streamer_if_fetch_finality_data_test PROPERTY LABELS long_running_tests) -add_test(NAME ship_kill_client_test COMMAND tests/ship_kill_client_test.py -v --activate-if --num-clients 20 ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) +add_test(NAME ship_kill_client_test COMMAND tests/ship_kill_client_test.py -v --num-clients 20 ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST ship_kill_client_test PROPERTY LABELS nonparallelizable_tests) add_test(NAME p2p_dawn515_test COMMAND tests/p2p_tests/dawn_515/test.sh WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) diff --git a/tests/ship_kill_client_test.py b/tests/ship_kill_client_test.py index 039d3dd46a..a04932fee5 100755 --- a/tests/ship_kill_client_test.py +++ b/tests/ship_kill_client_test.py @@ -23,11 +23,10 @@ appArgs = AppArgs() extraArgs = appArgs.add(flag="--num-clients", type=int, help="How many ship_streamers should be started", default=1) -args = TestHelper.parse_args({"--activate-if","--dump-error-details","--keep-logs","-v","--leave-running","--unshared"}, applicationSpecificArgs=appArgs) +args = TestHelper.parse_args({"--dump-error-details","--keep-logs","-v","--leave-running","--unshared"}, applicationSpecificArgs=appArgs) Utils.Debug=args.v cluster=Cluster(unshared=args.unshared, keepRunning=args.leave_running, keepLogs=args.keep_logs) -activateIF=args.activate_if dumpErrorDetails=args.dump_error_details walletPort=TestHelper.DEFAULT_WALLET_PORT @@ -52,7 +51,7 @@ specificExtraNodeosArgs[shipNodeNum]="--plugin eosio::state_history_plugin --trace-history --chain-state-history --finality-data-history --state-history-stride 200 --plugin eosio::net_api_plugin --plugin eosio::producer_api_plugin " if cluster.launch(pnodes=totalProducerNodes, loadSystemContract=False, - totalNodes=totalNodes, totalProducers=totalProducerNodes, activateIF=activateIF, biosFinalizer=False, + totalNodes=totalNodes, totalProducers=totalProducerNodes, activateIF=True, biosFinalizer=False, specificExtraNodeosArgs=specificExtraNodeosArgs) is False: Utils.cmdError("launcher") Utils.errorExit("Failed to stand up cluster.") From 264151513fd0d03a47273a26a918350a2798f7c1 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 24 Sep 2024 13:19:25 -0500 Subject: [PATCH 48/49] GH-815 Revert usage of strand. Needs to be done in fc::create_listener --- plugins/state_history_plugin/state_history_plugin.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/plugins/state_history_plugin/state_history_plugin.cpp b/plugins/state_history_plugin/state_history_plugin.cpp index a64d29ff6b..df6b839a31 100644 --- a/plugins/state_history_plugin/state_history_plugin.cpp +++ b/plugins/state_history_plugin/state_history_plugin.cpp @@ -100,12 +100,11 @@ struct state_history_plugin_impl { template void create_listener(const std::string& address) { const boost::posix_time::milliseconds accept_timeout(200); - // connections set must only be modified by main thread; run listener on main thread to avoid needing another post() - auto strand = boost::asio::make_strand(thread_pool.get_executor()); - fc::create_listener(strand, _log, accept_timeout, address, "", [this, strand](Protocol::socket&& socket) { - boost::asio::post(app().get_io_service(), [this, strand, socket{std::move(socket)}]() mutable { - catch_and_log([this, &socket, &strand]() { - connections.emplace(new session(std::move(socket), std::move(strand), chain_plug->chain(), + // connections set must only be modified by main thread; run listener on ship thread so sockets use default executor of the ship thread + fc::create_listener(thread_pool.get_executor(), _log, accept_timeout, address, "", [this](Protocol::socket&& socket) { + boost::asio::post(app().get_io_service(), [this, socket{std::move(socket)}]() mutable { + catch_and_log([this, &socket]() { + connections.emplace(new session(std::move(socket), boost::asio::make_strand(thread_pool.get_executor()), chain_plug->chain(), trace_log, chain_state_log, finality_data_log, [this](const chain::block_num_type block_num) { return get_block_id(block_num); From 6df1c1182f23303f85524ad520e4b49572ec6155 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 24 Sep 2024 14:02:46 -0500 Subject: [PATCH 49/49] GH-815 Cleanup test --- tests/ship_kill_client_test.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/ship_kill_client_test.py b/tests/ship_kill_client_test.py index a04932fee5..a050271063 100755 --- a/tests/ship_kill_client_test.py +++ b/tests/ship_kill_client_test.py @@ -30,6 +30,7 @@ dumpErrorDetails=args.dump_error_details walletPort=TestHelper.DEFAULT_WALLET_PORT +# simpler to have two producer nodes then setup different accounts for trx generator totalProducerNodes=2 totalNonProducerNodes=1 totalNodes=totalProducerNodes+totalNonProducerNodes @@ -61,7 +62,7 @@ Print("Cluster in Sync") prodNode0 = cluster.getNode(0) - nonprodNode = cluster.getNode(1) + prodNode1 = cluster.getNode(1) shipNode = cluster.getNode(shipNodeNum) # cluster.waitOnClusterSync(blockAdvancing=3) @@ -77,14 +78,14 @@ testTrxGenDurationSec=60*60 numTrxGenerators=2 cluster.launchTrxGenerators(contractOwnerAcctName=cluster.eosioAccount.name, acctNamesList=[cluster.defproduceraAccount.name, cluster.defproducerbAccount.name], - acctPrivKeysList=[cluster.defproduceraAccount.activePrivateKey,cluster.defproducerbAccount.activePrivateKey], nodeId=nonprodNode.nodeId, + acctPrivKeysList=[cluster.defproduceraAccount.activePrivateKey,cluster.defproducerbAccount.activePrivateKey], nodeId=prodNode1.nodeId, tpsPerGenerator=targetTpsPerGenerator, numGenerators=numTrxGenerators, durationSec=testTrxGenDurationSec, waitToComplete=False) - status = cluster.waitForTrxGeneratorsSpinup(nodeId=nonprodNode.nodeId, numGenerators=numTrxGenerators) + status = cluster.waitForTrxGeneratorsSpinup(nodeId=prodNode1.nodeId, numGenerators=numTrxGenerators) assert status is not None and status is not False, "ERROR: Failed to spinup Transaction Generators" - nonprodNode.waitForProducer("defproducera") + prodNode1.waitForProducer("defproducera") block_range = 100000 # we are going to kill the client, so just make this a huge number end_block_num = start_block_num + block_range @@ -99,7 +100,6 @@ shipClientFilePrefix = os.path.join(shipTempDir, "client") for i in range(0, args.num_clients): - start = time.perf_counter() outFile = open(f"{shipClientFilePrefix}{i}.out", "w") errFile = open(f"{shipClientFilePrefix}{i}.err", "w") Print(f"Start client {i}")