From a1d4ecc1c58fcd5e6f828eb0c6b40627e4bc8fcc Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Wed, 5 Jun 2024 14:50:14 -0500 Subject: [PATCH 01/15] GH-224 Send handshake when in lib_catchup and verify catchup is called. Should avoid peer thinking the peer is still syncing from it. --- plugins/net_plugin/net_plugin.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 8ffb140aa6..e6c401f08a 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2393,8 +2393,10 @@ namespace eosio { ("ne", sync_next_expected_num)("id", id.str().substr( 8, 16 )) ); } auto chain_info = my_impl->get_chain_info(); - if( sync_state == lib_catchup || num < chain_info.lib_num ) + if( sync_state == lib_catchup || num < chain_info.lib_num ) { + c->send_handshake(); return false; + } set_state( head_catchup ); { fc::lock_guard g_conn( c->conn_mtx ); From 41568a571838637c09bd78735f9681943f2cd5d1 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 6 Jun 2024 12:57:05 -0500 Subject: [PATCH 02/15] GH-216 Do not advance root if irreversible blocks do not apply --- libraries/chain/controller.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index fd5785e310..271a12e6fc 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1460,8 +1460,10 @@ struct controller_impl { auto it = v.begin(); for( auto bitr = branch.rbegin(); bitr != branch.rend() && should_process(*bitr); ++bitr ) { - if (!apply_irreversible_block(forkdb, *bitr)) + if (!apply_irreversible_block(forkdb, *bitr)) { + root_id = forkdb.root()->id(); break; + } emit( irreversible_block, std::tie((*bitr)->block, (*bitr)->id()), __FILE__, __LINE__ ); From 12dd21cf9a94a044cb41f3b35929bae60902ea0f Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 6 Jun 2024 14:18:03 -0500 Subject: [PATCH 03/15] GH-216 EOS_ASSERT to include which forkdb is being read --- libraries/chain/fork_database.cpp | 26 +++++++++---------- .../include/eosio/chain/fork_database.hpp | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index aaf03df51b..3ec511c7b9 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -121,7 +121,7 @@ namespace eosio::chain { explicit fork_database_impl() = default; - void open_impl( const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator ); + void open_impl( const char* desc, const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator ); void close_impl( std::ofstream& out ); void add_impl( const bsp_t& n, mark_valid_t mark_valid, ignore_duplicate_t ignore_duplicate, bool validate, validator_t& validator ); bool is_valid() const; @@ -153,13 +153,13 @@ namespace eosio::chain { fork_database_t::~fork_database_t() = default; // close is performed in fork_database::~fork_database() template - void fork_database_t::open( const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator ) { + void fork_database_t::open( const char* desc, const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator ) { std::lock_guard g( my->mtx ); - my->open_impl( fork_db_file, ds, validator ); + my->open_impl( desc, fork_db_file, ds, validator ); } template - void fork_database_impl::open_impl( const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator ) { + void fork_database_impl::open_impl( const char* desc, const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator ) { bsp_t _root = std::make_shared(); fc::raw::unpack( ds, *_root ); reset_root_impl( _root ); @@ -182,20 +182,20 @@ namespace eosio::chain { if (!head) std::filesystem::remove( fork_db_file ); EOS_ASSERT( head, fork_database_exception, - "could not find head while reconstructing fork database from file; " + "could not find head while reconstructing fork database from file; ${d} " "'${filename}' is likely corrupted and has been removed", - ("filename", fork_db_file) ); + ("d", desc)("filename", fork_db_file) ); } auto candidate = index.template get().begin(); if( candidate == index.template get().end() || !bs_accessor_t::is_valid(**candidate) ) { EOS_ASSERT( head->id() == root->id(), fork_database_exception, - "head not set to root despite no better option available; '${filename}' is likely corrupted", - ("filename", fork_db_file) ); + "head not set to root despite no better option available; ${d} '${filename}' is likely corrupted", + ("d", desc)("filename", fork_db_file) ); } else { EOS_ASSERT( !first_preferred( **candidate, *head ), fork_database_exception, - "head not set to best available option available; '${filename}' is likely corrupted", - ("filename", fork_db_file) ); + "head not set to the best available option available; ${d} '${filename}' is likely corrupted", + ("d", desc)("filename", fork_db_file) ); } } @@ -786,7 +786,7 @@ namespace eosio::chain { { // ---------- pre-Savanna format. Just a single fork_database_l ---------------- in_use = in_use_t::legacy; - fork_db_l.open(fork_db_file, ds, validator); + fork_db_l.open("legacy", fork_db_file, ds, validator); break; } @@ -800,13 +800,13 @@ namespace eosio::chain { bool legacy_valid { false }; fc::raw::unpack( ds, legacy_valid ); if (legacy_valid) { - fork_db_l.open(fork_db_file, ds, validator); + fork_db_l.open("legacy", fork_db_file, ds, validator); } bool savanna_valid { false }; fc::raw::unpack( ds, savanna_valid ); if (savanna_valid) { - fork_db_s.open(fork_db_file, ds, validator); + fork_db_s.open("savanna", fork_db_file, ds, validator); } break; } diff --git a/libraries/chain/include/eosio/chain/fork_database.hpp b/libraries/chain/include/eosio/chain/fork_database.hpp index 981967eb97..139ee0c89b 100644 --- a/libraries/chain/include/eosio/chain/fork_database.hpp +++ b/libraries/chain/include/eosio/chain/fork_database.hpp @@ -46,7 +46,7 @@ namespace eosio::chain { explicit fork_database_t(); ~fork_database_t(); - void open( const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator ); + void open( const char* desc, const std::filesystem::path& fork_db_file, fc::cfile_datastream& ds, validator_t& validator ); void close( std::ofstream& out ); bsp_t get_block( const block_id_type& id, include_root_t include_root = include_root_t::no ) const; From c0851d7d3fcdc5e1d3aa2933800d40374d930c3a Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 6 Jun 2024 14:18:34 -0500 Subject: [PATCH 04/15] GH-216 Add to forkdb as it expects root != head --- libraries/chain/controller.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 271a12e6fc..e1f987e49e 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1323,15 +1323,17 @@ struct controller_impl { return fork_db.apply_l([&](const auto& forkdb_l) { block_state_legacy_ptr legacy = forkdb_l.get_block(bsp->id()); fork_db.switch_to(fork_database::in_use_t::legacy); // apply block uses to know what types to create + block_state_ptr prev = forkdb.get_block(legacy->previous(), include_root_t::yes); + assert(prev); if( apply_block(br, legacy, controller::block_status::complete, trx_meta_cache_lookup{}) ) { fc::scoped_exit> e([&]{fork_db.switch_to(fork_database::in_use_t::both);}); // irreversible apply was just done, calculate new_valid here instead of in transition_to_savanna() assert(legacy->action_mroot_savanna); - block_state_ptr prev = forkdb.get_block(legacy->previous(), include_root_t::yes); - assert(prev); transition_add_to_savanna_fork_db(forkdb, legacy, bsp, prev); return true; } + // add to forkdb as it expects root != head + transition_add_to_savanna_fork_db(forkdb, legacy, bsp, prev); fork_db.switch_to(fork_database::in_use_t::legacy); return false; }); @@ -1460,10 +1462,8 @@ struct controller_impl { auto it = v.begin(); for( auto bitr = branch.rbegin(); bitr != branch.rend() && should_process(*bitr); ++bitr ) { - if (!apply_irreversible_block(forkdb, *bitr)) { - root_id = forkdb.root()->id(); + if (!apply_irreversible_block(forkdb, *bitr)) break; - } emit( irreversible_block, std::tie((*bitr)->block, (*bitr)->id()), __FILE__, __LINE__ ); From cf18c4f73b4008b977fd67b196d136290e2cede4 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Thu, 6 Jun 2024 16:17:45 -0500 Subject: [PATCH 05/15] GH-216 Fix add with mark_valid and ignore_duplicate --- libraries/chain/fork_database.cpp | 19 ++++++------- unittests/fork_db_tests.cpp | 44 ++++++++++++++++++++----------- 2 files changed, 39 insertions(+), 24 deletions(-) diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index 3ec511c7b9..dd77302b89 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -354,18 +354,19 @@ namespace eosio::chain { EOS_RETHROW_EXCEPTIONS( fork_database_exception, "serialized fork database is incompatible with configured protocol features" ) } - if (mark_valid == mark_valid_t::yes) - bs_accessor_t::set_valid(*n, true); - auto inserted = index.insert(n); - if( !inserted.second ) { - if( ignore_duplicate == ignore_duplicate_t::yes ) return; - EOS_THROW( fork_database_exception, "duplicate block added", ("id", n->id()) ); + if( !inserted.second && ignore_duplicate != ignore_duplicate_t::yes ) { + EOS_THROW(fork_database_exception, "duplicate block added", ("id", n->id())); } - auto candidate = index.template get().begin(); - if( bs_accessor_t::is_valid(**candidate) ) { - head = *candidate; + if (mark_valid == mark_valid_t::yes) { + mark_valid_impl(n); + // mark_valid_impl updates head + } else { + auto candidate = index.template get().begin(); + if( bs_accessor_t::is_valid(**candidate) ) { + head = *candidate; + } } } diff --git a/unittests/fork_db_tests.cpp b/unittests/fork_db_tests.cpp index b9d024aa57..27b9c04cc7 100644 --- a/unittests/fork_db_tests.cpp +++ b/unittests/fork_db_tests.cpp @@ -40,6 +40,10 @@ struct block_state_accessor { bsp->core = prev->core.next(parent_block, prev->core.latest_qc_claim()); return bsp; } + + static void reset_valid(block_state_ptr& bsp) { + bsp->validated.store(false); + } }; } // namespace eosio::chain @@ -72,21 +76,24 @@ BOOST_AUTO_TEST_CASE(add_remove_test) try { // keep track of all those added for easy verification std::vector all { bsp11a, bsp12a, bsp13a, bsp11b, bsp12b, bsp12bb, bsp12bbb, bsp13b, bsp13bb, bsp13bbb, bsp14b, bsp11c, bsp12c, bsp13c }; - forkdb.reset_root(root); - forkdb.add(bsp11a, mark_valid_t::no, ignore_duplicate_t::no); - forkdb.add(bsp11b, mark_valid_t::no, ignore_duplicate_t::no); - forkdb.add(bsp11c, mark_valid_t::no, ignore_duplicate_t::no); - forkdb.add(bsp12a, mark_valid_t::no, ignore_duplicate_t::no); - forkdb.add(bsp13a, mark_valid_t::no, ignore_duplicate_t::no); - forkdb.add(bsp12b, mark_valid_t::no, ignore_duplicate_t::no); - forkdb.add(bsp12bb, mark_valid_t::no, ignore_duplicate_t::no); - forkdb.add(bsp12bbb, mark_valid_t::no, ignore_duplicate_t::no); - forkdb.add(bsp12c, mark_valid_t::no, ignore_duplicate_t::no); - forkdb.add(bsp13b, mark_valid_t::no, ignore_duplicate_t::no); - forkdb.add(bsp13bb, mark_valid_t::no, ignore_duplicate_t::no); - forkdb.add(bsp13bbb, mark_valid_t::no, ignore_duplicate_t::no); - forkdb.add(bsp14b, mark_valid_t::no, ignore_duplicate_t::no); - forkdb.add(bsp13c, mark_valid_t::no, ignore_duplicate_t::no); + auto reset = [&]() { + forkdb.reset_root(root); + forkdb.add(bsp11a, mark_valid_t::no, ignore_duplicate_t::no); + forkdb.add(bsp11b, mark_valid_t::no, ignore_duplicate_t::no); + forkdb.add(bsp11c, mark_valid_t::no, ignore_duplicate_t::no); + forkdb.add(bsp12a, mark_valid_t::no, ignore_duplicate_t::no); + forkdb.add(bsp13a, mark_valid_t::no, ignore_duplicate_t::no); + forkdb.add(bsp12b, mark_valid_t::no, ignore_duplicate_t::no); + forkdb.add(bsp12bb, mark_valid_t::no, ignore_duplicate_t::no); + forkdb.add(bsp12bbb, mark_valid_t::no, ignore_duplicate_t::no); + forkdb.add(bsp12c, mark_valid_t::no, ignore_duplicate_t::no); + forkdb.add(bsp13b, mark_valid_t::no, ignore_duplicate_t::no); + forkdb.add(bsp13bb, mark_valid_t::no, ignore_duplicate_t::no); + forkdb.add(bsp13bbb, mark_valid_t::no, ignore_duplicate_t::no); + forkdb.add(bsp14b, mark_valid_t::no, ignore_duplicate_t::no); + forkdb.add(bsp13c, mark_valid_t::no, ignore_duplicate_t::no); + }; + reset(); // test get_block for (auto& i : all) { @@ -128,6 +135,13 @@ BOOST_AUTO_TEST_CASE(add_remove_test) try { BOOST_TEST(branch[0] == bsp12a); BOOST_TEST(branch[1] == bsp11a); + // test mark valid via add + block_state_accessor::reset_valid(bsp13a); + reset(); + BOOST_TEST(forkdb.head()->id() == root->id()); + forkdb.add(bsp13a, mark_valid_t::yes, ignore_duplicate_t::yes); + BOOST_TEST(forkdb.head()->id() == bsp13a->id()); + } FC_LOG_AND_RETHROW(); BOOST_AUTO_TEST_SUITE_END() From dab1a4857a4470560798d9bcb5425fdb72247294 Mon Sep 17 00:00:00 2001 From: Matt Witherspoon <32485495+spoonincode@users.noreply.github.com> Date: Thu, 6 Jun 2024 23:43:05 -0400 Subject: [PATCH 06/15] add rest of finality_data to ship abi --- libraries/state_history/abi.cpp | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/libraries/state_history/abi.cpp b/libraries/state_history/abi.cpp index 5cffe16fbf..bc05b7364f 100644 --- a/libraries/state_history/abi.cpp +++ b/libraries/state_history/abi.cpp @@ -572,13 +572,29 @@ 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_policy", "fields": [ + { "name": "generation", "type": "uint32" }, + { "name": "threshold", "type": "uint64" }, + { "name": "finalizers", "type": "finalizer_authority[]" } + ] + }, { "name": "finality_data", "fields": [ { "name": "major_version", "type": "uint32" }, { "name": "minor_version", "type": "uint32" }, { "name": "active_finalizer_policy_generation", "type": "uint32" }, + { "name": "final_on_strong_qc_block_num", "type": "uint32" }, { "name": "action_mroot", "type": "checksum256" }, - { "name": "base_digest", "type": "checksum256" } + { "name": "base_digest", "type": "checksum256" }, + { "name": "proposed_finalizer_policy", "type": "finalizer_policy?" } ] } ], From 981baeb0b93088bbc15cd2943435b7dc54ec7953 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 7 Jun 2024 07:13:45 -0500 Subject: [PATCH 07/15] GH-246 Allow time for blocks to reach and be processed by irreversible snapshot node --- tests/nodeos_chainbase_allocation_test.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/nodeos_chainbase_allocation_test.py b/tests/nodeos_chainbase_allocation_test.py index cfc65f7197..afc350e624 100755 --- a/tests/nodeos_chainbase_allocation_test.py +++ b/tests/nodeos_chainbase_allocation_test.py @@ -79,6 +79,9 @@ def isSetProdsBlockNumIrr(): producerNode.kill(signal.SIGTERM) + # wait for all blocks to arrive and be processed by irrNode + time.sleep(1) + # Create the snapshot and rename it to avoid name conflict later on res = irrNode.createSnapshot() beforeShutdownSnapshotPath = res["payload"]["snapshot_name"] From 462d7cde019367937b078c8546d56af88accc6b0 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 7 Jun 2024 07:30:12 -0500 Subject: [PATCH 08/15] GH-246 Increase sleep to 3 seconds to provide plenty of time for blocks to arrive --- tests/nodeos_chainbase_allocation_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/nodeos_chainbase_allocation_test.py b/tests/nodeos_chainbase_allocation_test.py index afc350e624..23887dbfba 100755 --- a/tests/nodeos_chainbase_allocation_test.py +++ b/tests/nodeos_chainbase_allocation_test.py @@ -80,7 +80,7 @@ def isSetProdsBlockNumIrr(): producerNode.kill(signal.SIGTERM) # wait for all blocks to arrive and be processed by irrNode - time.sleep(1) + time.sleep(3) # Create the snapshot and rename it to avoid name conflict later on res = irrNode.createSnapshot() From fbec433b6949ef35d4f4322228a3ea7fd89865c5 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 7 Jun 2024 08:25:49 -0500 Subject: [PATCH 09/15] GH-216 Optimize mark_valid --- libraries/chain/fork_database.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index dd77302b89..da58baeffd 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -356,18 +356,22 @@ namespace eosio::chain { auto inserted = index.insert(n); if( !inserted.second && ignore_duplicate != ignore_duplicate_t::yes ) { - EOS_THROW(fork_database_exception, "duplicate block added", ("id", n->id())); + EOS_THROW(fork_database_exception, "duplicate block added: ${id}", ("id", n->id())); } if (mark_valid == mark_valid_t::yes) { - mark_valid_impl(n); - // mark_valid_impl updates head - } else { - auto candidate = index.template get().begin(); - if( bs_accessor_t::is_valid(**candidate) ) { - head = *candidate; + // if just inserted and was inserted already valid then no update needed + if (!inserted.second || !bs_accessor_t::is_valid(*n)) { + index.modify( inserted.first, []( auto& i ) { + bs_accessor_t::set_valid(*i, true); + } ); } } + + auto candidate = index.template get().begin(); + if (bs_accessor_t::is_valid(**candidate)) { + head = *candidate; + } } template From a2d5d701d2d950ce2f423bf4800ff23927090390 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 7 Jun 2024 08:43:14 -0500 Subject: [PATCH 10/15] GH-216 Simplify by using EOS_ASSERT --- libraries/chain/fork_database.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index da58baeffd..4cc12cbaf0 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -355,9 +355,8 @@ namespace eosio::chain { } auto inserted = index.insert(n); - if( !inserted.second && ignore_duplicate != ignore_duplicate_t::yes ) { - EOS_THROW(fork_database_exception, "duplicate block added: ${id}", ("id", n->id())); - } + EOS_ASSERT(ignore_duplicate == ignore_duplicate_t::yes || inserted.second, fork_database_exception, + "duplicate block added: ${id}", ("id", n->id())); if (mark_valid == mark_valid_t::yes) { // if just inserted and was inserted already valid then no update needed From eae79cde6d8b41f82967fdcb33f4668c92750e74 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 7 Jun 2024 12:54:35 -0500 Subject: [PATCH 11/15] Update to 1.0.0 beta2.1 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f8aa3a2738..df88ab04f3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -16,7 +16,7 @@ set( CXX_STANDARD_REQUIRED ON) set(VERSION_MAJOR 1) set(VERSION_MINOR 0) set(VERSION_PATCH 0) -set(VERSION_SUFFIX beta2.0) +set(VERSION_SUFFIX beta2.1) if(VERSION_SUFFIX) set(VERSION_FULL "${VERSION_MAJOR}.${VERSION_MINOR}.${VERSION_PATCH}-${VERSION_SUFFIX}") From 74cf5143f3100bf584fc9bf440e0b1ec5cb1921c Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 10 Jun 2024 08:56:05 -0500 Subject: [PATCH 12/15] GH-259 Remove unused startup time. --- libraries/chain/controller.cpp | 2 +- .../include/eosio/chain/finality/finalizer.hpp | 6 ++---- unittests/finalizer_tests.cpp | 16 ++++++++-------- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index e1f987e49e..78ef227620 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1232,7 +1232,7 @@ struct controller_impl { chain_id( chain_id ), read_mode( cfg.read_mode ), thread_pool(), - my_finalizers(fc::time_point::now(), cfg.finalizers_dir / "safety.dat"), + my_finalizers(cfg.finalizers_dir / "safety.dat"), wasmif( conf.wasm_runtime, conf.eosvmoc_tierup, db, conf.state_dir, conf.eosvmoc_config, !conf.profile_accounts.empty() ) { assert(cfg.chain_thread_pool_size > 0); diff --git a/libraries/chain/include/eosio/chain/finality/finalizer.hpp b/libraries/chain/include/eosio/chain/finality/finalizer.hpp index eb965b2ddc..01b0e4ddab 100644 --- a/libraries/chain/include/eosio/chain/finality/finalizer.hpp +++ b/libraries/chain/include/eosio/chain/finality/finalizer.hpp @@ -71,7 +71,6 @@ namespace eosio::chain { using fsi_map = std::map; private: - const block_timestamp_type t_startup; // nodeos startup time, used for default safety_information const std::filesystem::path persist_file_path; // where we save the safety data mutable std::mutex mtx; mutable fc::datastream persist_file; // we want to keep the file open for speed @@ -81,9 +80,8 @@ namespace eosio::chain { mutable bool inactive_safety_info_written{false}; public: - my_finalizers_t(block_timestamp_type startup_time, const std::filesystem::path& persist_file_path) - : t_startup(startup_time) - , persist_file_path(persist_file_path) + explicit my_finalizers_t(const std::filesystem::path& persist_file_path) + : persist_file_path(persist_file_path) {} template // thread safe diff --git a/unittests/finalizer_tests.cpp b/unittests/finalizer_tests.cpp index b7c4c2e782..3370dfa987 100644 --- a/unittests/finalizer_tests.cpp +++ b/unittests/finalizer_tests.cpp @@ -90,7 +90,7 @@ BOOST_AUTO_TEST_CASE( basic_finalizer_safety_file_io ) try { bls_pub_priv_key_map_t local_finalizers = { { k.pubkey_str, k.privkey_str } }; { - my_finalizers_t fset{block_timestamp_type{}, safety_file_path}; + my_finalizers_t fset{safety_file_path}; fset.set_keys(local_finalizers); fset.set_fsi(k.pubkey, fsi); @@ -101,7 +101,7 @@ BOOST_AUTO_TEST_CASE( basic_finalizer_safety_file_io ) try { } { - my_finalizers_t fset{block_timestamp_type{}, safety_file_path}; + my_finalizers_t fset{safety_file_path}; fset.set_keys(local_finalizers); // that's when the finalizer safety file is read // make sure the safety info for our finalizer that we saved above is restored correctly @@ -123,7 +123,7 @@ BOOST_AUTO_TEST_CASE( corrupt_finalizer_safety_file ) try { bls_pub_priv_key_map_t local_finalizers = { { k.pubkey_str, k.privkey_str } }; { - my_finalizers_t fset{block_timestamp_type{}, safety_file_path}; + my_finalizers_t fset{safety_file_path}; fset.set_keys(local_finalizers); fset.set_fsi(k.pubkey, fsi); @@ -140,7 +140,7 @@ BOOST_AUTO_TEST_CASE( corrupt_finalizer_safety_file ) try { } { - my_finalizers_t fset{block_timestamp_type{}, safety_file_path}; + my_finalizers_t fset{safety_file_path}; BOOST_REQUIRE_THROW(fset.set_keys(local_finalizers), // that's when the finalizer safety file is read finalizer_safety_exception); @@ -159,7 +159,7 @@ BOOST_AUTO_TEST_CASE( finalizer_safety_file_io ) try { std::vector keys = create_keys(10); { - my_finalizers_t fset{block_timestamp_type{}, safety_file_path}; + my_finalizers_t fset{safety_file_path}; bls_pub_priv_key_map_t local_finalizers = create_local_finalizers<1, 3, 5, 6>(keys); fset.set_keys(local_finalizers); @@ -171,7 +171,7 @@ BOOST_AUTO_TEST_CASE( finalizer_safety_file_io ) try { } { - my_finalizers_t fset{block_timestamp_type{}, safety_file_path}; + my_finalizers_t fset{safety_file_path}; bls_pub_priv_key_map_t local_finalizers = create_local_finalizers<3>(keys); fset.set_keys(local_finalizers); @@ -186,7 +186,7 @@ BOOST_AUTO_TEST_CASE( finalizer_safety_file_io ) try { } { - my_finalizers_t fset{block_timestamp_type{}, safety_file_path}; + my_finalizers_t fset{safety_file_path}; bls_pub_priv_key_map_t local_finalizers = create_local_finalizers<3>(keys); fset.set_keys(local_finalizers); @@ -197,7 +197,7 @@ BOOST_AUTO_TEST_CASE( finalizer_safety_file_io ) try { // even though we didn't activate finalizers 1, 5, or 6 in the prior test, and we wrote the safety file, // make sure we have not lost the fsi that was set originally for these finalizers. { - my_finalizers_t fset{block_timestamp_type{}, safety_file_path}; + my_finalizers_t fset{safety_file_path}; bls_pub_priv_key_map_t local_finalizers = create_local_finalizers<1, 5, 6>(keys); fset.set_keys(local_finalizers); From d4d068ad277daadd3d251a4e37178baf64594597 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 10 Jun 2024 08:58:46 -0500 Subject: [PATCH 13/15] GH-259 Do not broadcast a vote if syncing --- plugins/net_plugin/net_plugin.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index e6c401f08a..b83fad8ca2 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -2705,6 +2705,9 @@ namespace eosio { } void dispatch_manager::bcast_vote_msg( uint32_t exclude_peer, send_buffer_type msg ) { + if (my_impl->sync_master->syncing_from_peer()) + return; + my_impl->connections.for_each_block_connection( [exclude_peer, msg{std::move(msg)}]( auto& cp ) { if( !cp->current() ) return true; if( cp->connection_id == exclude_peer ) return true; From c90b91e7853a4610e5163916c9805e3474808469 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 10 Jun 2024 09:58:14 -0500 Subject: [PATCH 14/15] GH-259 only vote when blocks are within 30 seconds of now --- libraries/chain/controller.cpp | 3 +-- .../include/eosio/chain/finality/finalizer.hpp | 15 ++++++++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 78ef227620..47e34bcb03 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -3701,8 +3701,7 @@ struct controller_impl { return; // Each finalizer configured on the node which is present in the active finalizer policy may create and sign a vote. - my_finalizers.maybe_vote( - *bsp->active_finalizer_policy, bsp, bsp->strong_digest, [&](const vote_message_ptr& vote) { + my_finalizers.maybe_vote(bsp, [&](const vote_message_ptr& vote) { // net plugin subscribed to this signal. it will broadcast the vote message on receiving the signal emit(voted_block, std::tuple{uint32_t{0}, vote_status::success, std::cref(vote)}, __FILE__, __LINE__); diff --git a/libraries/chain/include/eosio/chain/finality/finalizer.hpp b/libraries/chain/include/eosio/chain/finality/finalizer.hpp index 01b0e4ddab..174f7a21b7 100644 --- a/libraries/chain/include/eosio/chain/finality/finalizer.hpp +++ b/libraries/chain/include/eosio/chain/finality/finalizer.hpp @@ -78,6 +78,7 @@ namespace eosio::chain { fsi_map inactive_safety_info; // loaded at startup, not mutated afterwards fsi_t default_fsi = fsi_t::unset_fsi(); // default provided at spring startup mutable bool inactive_safety_info_written{false}; + bool enable_voting = false; public: explicit my_finalizers_t(const std::filesystem::path& persist_file_path) @@ -85,14 +86,14 @@ namespace eosio::chain { {} template // thread safe - void maybe_vote(const finalizer_policy& fin_pol, - const block_state_ptr& bsp, - const digest_type& digest, - F&& process_vote) { + void maybe_vote(const block_state_ptr& bsp, F&& process_vote) { if (finalizers.empty()) return; + assert(bsp->active_finalizer_policy); + const auto& fin_pol = *bsp->active_finalizer_policy; + std::vector votes; votes.reserve(finalizers.size()); @@ -100,10 +101,14 @@ namespace eosio::chain { // Would require making sure that only the latest is ever written to the file and that the file access was protected separately. std::unique_lock g(mtx); + if (!enable_voting && bsp->timestamp() < fc::time_point::now() - fc::seconds(30)) + return; + enable_voting = true; + // first accumulate all the votes for (const auto& f : fin_pol.finalizers) { if (auto it = finalizers.find(f.public_key); it != finalizers.end()) { - vote_message_ptr vote_msg = it->second.maybe_vote(it->first, bsp, digest); + vote_message_ptr vote_msg = it->second.maybe_vote(it->first, bsp, bsp->strong_digest); if (vote_msg) votes.push_back(std::move(vote_msg)); } From 7b47b0e74db6e4275ea0b7b9657f8f664d1084fd Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 10 Jun 2024 11:01:46 -0500 Subject: [PATCH 15/15] GH-259 Start unittests with voting enabled --- libraries/chain/controller.cpp | 8 ++++---- libraries/chain/finality/finalizer.cpp | 4 +++- .../chain/include/eosio/chain/controller.hpp | 2 +- .../include/eosio/chain/finality/finalizer.hpp | 4 +++- libraries/testing/tester.cpp | 4 ++-- unittests/finalizer_tests.cpp | 16 ++++++++-------- 6 files changed, 21 insertions(+), 17 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 47e34bcb03..fee2fd8910 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -4611,8 +4611,8 @@ struct controller_impl { wasmif.code_block_num_last_used(code_hash, vm_type, vm_version, block_num); } - void set_node_finalizer_keys(const bls_pub_priv_key_map_t& finalizer_keys) { - my_finalizers.set_keys(finalizer_keys); + void set_node_finalizer_keys(const bls_pub_priv_key_map_t& finalizer_keys, bool enable_immediate_voting) { + my_finalizers.set_keys(finalizer_keys, enable_immediate_voting); } bool irreversible_mode() const { return read_mode == db_read_mode::IRREVERSIBLE; } @@ -5762,8 +5762,8 @@ void controller::code_block_num_last_used(const digest_type& code_hash, uint8_t return my->code_block_num_last_used(code_hash, vm_type, vm_version, block_num); } -void controller::set_node_finalizer_keys(const bls_pub_priv_key_map_t& finalizer_keys) { - my->set_node_finalizer_keys(finalizer_keys); +void controller::set_node_finalizer_keys(const bls_pub_priv_key_map_t& finalizer_keys, bool enable_immediate_voting) { + my->set_node_finalizer_keys(finalizer_keys, enable_immediate_voting); } /// Protocol feature activation handlers: diff --git a/libraries/chain/finality/finalizer.cpp b/libraries/chain/finality/finalizer.cpp index 35100dd805..c38d251830 100644 --- a/libraries/chain/finality/finalizer.cpp +++ b/libraries/chain/finality/finalizer.cpp @@ -200,7 +200,7 @@ my_finalizers_t::fsi_map my_finalizers_t::load_finalizer_safety_info() { } // ---------------------------------------------------------------------------------------- -void my_finalizers_t::set_keys(const std::map& finalizer_keys) { +void my_finalizers_t::set_keys(const std::map& finalizer_keys, bool enable_immediate_voting) { if (finalizer_keys.empty()) return; @@ -226,6 +226,8 @@ void my_finalizers_t::set_keys(const std::map& finaliz // now only inactive finalizers remain in safety_info => move it to inactive_safety_info inactive_safety_info = std::move(safety_info); + + enable_voting = enable_immediate_voting; } diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index af105400fd..0616ff1aa2 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -410,7 +410,7 @@ namespace eosio::chain { void set_to_read_window(); bool is_write_window() const; void code_block_num_last_used(const digest_type& code_hash, uint8_t vm_type, uint8_t vm_version, uint32_t block_num); - void set_node_finalizer_keys(const bls_pub_priv_key_map_t& finalizer_keys); + void set_node_finalizer_keys(const bls_pub_priv_key_map_t& finalizer_keys, bool enable_immediate_voting = false); private: friend class apply_context; diff --git a/libraries/chain/include/eosio/chain/finality/finalizer.hpp b/libraries/chain/include/eosio/chain/finality/finalizer.hpp index 174f7a21b7..71eca79d42 100644 --- a/libraries/chain/include/eosio/chain/finality/finalizer.hpp +++ b/libraries/chain/include/eosio/chain/finality/finalizer.hpp @@ -130,7 +130,9 @@ namespace eosio::chain { return std::ranges::all_of(std::views::keys(finalizers), std::forward(f)); } - void set_keys(const std::map& finalizer_keys); // only call on startup + /// only call on startup + /// @param enable_immediate_voting if true enable immediate voting on startup (for testing) + void set_keys(const std::map& finalizer_keys, bool enable_immediate_voting); void set_default_safety_information(const fsi_t& fsi); // following two member functions could be private, but are used in testing, not thread safe diff --git a/libraries/testing/tester.cpp b/libraries/testing/tester.cpp index e36818f00b..51a40591c3 100644 --- a/libraries/testing/tester.cpp +++ b/libraries/testing/tester.cpp @@ -1253,7 +1253,7 @@ namespace eosio::testing { ("pop", pop.to_string())); } - control->set_node_finalizer_keys(local_finalizer_keys); + control->set_node_finalizer_keys(local_finalizer_keys, true); fc::mutable_variant_object fin_policy_variant; fin_policy_variant("threshold", input.threshold); @@ -1271,7 +1271,7 @@ namespace eosio::testing { auto [privkey, pubkey, pop] = get_bls_key(name); local_finalizer_keys[pubkey.to_string()] = privkey.to_string(); } - control->set_node_finalizer_keys(local_finalizer_keys); + control->set_node_finalizer_keys(local_finalizer_keys, true); } base_tester::set_finalizers_output_t base_tester::set_active_finalizers(std::span names) { diff --git a/unittests/finalizer_tests.cpp b/unittests/finalizer_tests.cpp index 3370dfa987..7a7b27c2c2 100644 --- a/unittests/finalizer_tests.cpp +++ b/unittests/finalizer_tests.cpp @@ -91,7 +91,7 @@ BOOST_AUTO_TEST_CASE( basic_finalizer_safety_file_io ) try { { my_finalizers_t fset{safety_file_path}; - fset.set_keys(local_finalizers); + fset.set_keys(local_finalizers, false); fset.set_fsi(k.pubkey, fsi); fset.save_finalizer_safety_info(); @@ -102,7 +102,7 @@ BOOST_AUTO_TEST_CASE( basic_finalizer_safety_file_io ) try { { my_finalizers_t fset{safety_file_path}; - fset.set_keys(local_finalizers); // that's when the finalizer safety file is read + fset.set_keys(local_finalizers, false); // that's when the finalizer safety file is read // make sure the safety info for our finalizer that we saved above is restored correctly BOOST_CHECK_EQUAL(fset.get_fsi(k.pubkey), fsi); @@ -124,7 +124,7 @@ BOOST_AUTO_TEST_CASE( corrupt_finalizer_safety_file ) try { { my_finalizers_t fset{safety_file_path}; - fset.set_keys(local_finalizers); + fset.set_keys(local_finalizers, false); fset.set_fsi(k.pubkey, fsi); fset.save_finalizer_safety_info(); @@ -141,7 +141,7 @@ BOOST_AUTO_TEST_CASE( corrupt_finalizer_safety_file ) try { { my_finalizers_t fset{safety_file_path}; - BOOST_REQUIRE_THROW(fset.set_keys(local_finalizers), // that's when the finalizer safety file is read + BOOST_REQUIRE_THROW(fset.set_keys(local_finalizers, false), // that's when the finalizer safety file is read finalizer_safety_exception); // make sure the safety info for our finalizer that we saved above is restored correctly @@ -161,7 +161,7 @@ BOOST_AUTO_TEST_CASE( finalizer_safety_file_io ) try { { my_finalizers_t fset{safety_file_path}; bls_pub_priv_key_map_t local_finalizers = create_local_finalizers<1, 3, 5, 6>(keys); - fset.set_keys(local_finalizers); + fset.set_keys(local_finalizers, false); set_fsi(fset, keys, fsi); fset.save_finalizer_safety_info(); @@ -173,7 +173,7 @@ BOOST_AUTO_TEST_CASE( finalizer_safety_file_io ) try { { my_finalizers_t fset{safety_file_path}; bls_pub_priv_key_map_t local_finalizers = create_local_finalizers<3>(keys); - fset.set_keys(local_finalizers); + fset.set_keys(local_finalizers, false); // make sure the safety info for our finalizer that we saved above is restored correctly BOOST_CHECK_EQUAL(fset.get_fsi(keys[3].pubkey), fsi[3]); @@ -188,7 +188,7 @@ BOOST_AUTO_TEST_CASE( finalizer_safety_file_io ) try { { my_finalizers_t fset{safety_file_path}; bls_pub_priv_key_map_t local_finalizers = create_local_finalizers<3>(keys); - fset.set_keys(local_finalizers); + fset.set_keys(local_finalizers, false); // make sure the safety info for our finalizer that we saved above is restored correctly BOOST_CHECK_EQUAL(fset.get_fsi(keys[3].pubkey), fsi[4]); @@ -199,7 +199,7 @@ BOOST_AUTO_TEST_CASE( finalizer_safety_file_io ) try { { my_finalizers_t fset{safety_file_path}; bls_pub_priv_key_map_t local_finalizers = create_local_finalizers<1, 5, 6>(keys); - fset.set_keys(local_finalizers); + fset.set_keys(local_finalizers, false); // make sure the safety info for our previously inactive finalizer was preserved BOOST_CHECK_EQUAL(fset.get_fsi(keys[1].pubkey), fsi[1]);