diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 5615b8091b..10da42217d 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -4232,11 +4232,11 @@ struct controller_impl { assert(!verify_qc_future.valid()); } - bool best_head = fork_db.add(bsp, ignore_duplicate_t::yes); + fork_db_add_t add_result = fork_db.add(bsp, ignore_duplicate_t::yes); if constexpr (is_proper_savanna_block) vote_processor.notify_new_block(async_aggregation); - return controller::accepted_block_result{best_head, block_handle{std::move(bsp)}}; + return controller::accepted_block_result{add_result, block_handle{std::move(bsp)}}; } // thread safe, expected to be called from thread other than the main thread diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index dbcadb8f92..d49f6c38bb 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -94,7 +94,7 @@ namespace eosio::chain { 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 ); - bool add_impl( const bsp_t& n, ignore_duplicate_t ignore_duplicate, bool validate, validator_t& validator ); + fork_db_add_t add_impl( const bsp_t& n, ignore_duplicate_t ignore_duplicate, bool validate, validator_t& validator ); bool is_valid() const; bsp_t get_block_impl( const block_id_type& id, include_root_t include_root = include_root_t::no ) const; @@ -240,8 +240,8 @@ namespace eosio::chain { } template - bool fork_database_impl::add_impl(const bsp_t& n, ignore_duplicate_t ignore_duplicate, - bool validate, validator_t& validator) { + fork_db_add_t fork_database_impl::add_impl(const bsp_t& n, ignore_duplicate_t ignore_duplicate, + bool validate, validator_t& validator) { EOS_ASSERT( root, fork_database_exception, "root not yet set" ); EOS_ASSERT( n, fork_database_exception, "attempt to add null block state" ); @@ -277,15 +277,25 @@ namespace eosio::chain { EOS_RETHROW_EXCEPTIONS( fork_database_exception, "serialized fork database is incompatible with configured protocol features" ) } + auto prev_head = head_impl(include_root_t::yes); + auto inserted = index.insert(n); EOS_ASSERT(ignore_duplicate == ignore_duplicate_t::yes || inserted.second, fork_database_exception, "duplicate block added: ${id}", ("id", n->id())); - return inserted.second && n == head_impl(include_root_t::no); + if (!inserted.second) + return fork_db_add_t::duplicate; + const bool new_head = n == head_impl(include_root_t::no); + if (new_head && n->previous() == prev_head->id()) + return fork_db_add_t::appended_to_head; + if (new_head) + return fork_db_add_t::fork_switch; + + return fork_db_add_t::added; } template - bool fork_database_t::add( const bsp_t& n, ignore_duplicate_t ignore_duplicate ) { + fork_db_add_t fork_database_t::add( const bsp_t& n, ignore_duplicate_t ignore_duplicate ) { std::lock_guard g( my->mtx ); return my->add_impl(n, ignore_duplicate, false, [](block_timestamp_type timestamp, diff --git a/libraries/chain/include/eosio/chain/controller.hpp b/libraries/chain/include/eosio/chain/controller.hpp index daf151ae2a..7eaaa2759f 100644 --- a/libraries/chain/include/eosio/chain/controller.hpp +++ b/libraries/chain/include/eosio/chain/controller.hpp @@ -96,6 +96,7 @@ namespace eosio::chain { using resource_limits::resource_limits_manager; using apply_handler = std::function; + enum class fork_db_add_t; using forked_callback_t = std::function; // lookup transaction_metadata via supplied function to avoid re-creation @@ -235,7 +236,7 @@ namespace eosio::chain { void set_async_aggregation(async_t val); struct accepted_block_result { - const bool is_new_best_head = false; // true if new best head + const fork_db_add_t add_result; std::optional block; // empty optional if block is unlinkable }; // thread-safe diff --git a/libraries/chain/include/eosio/chain/fork_database.hpp b/libraries/chain/include/eosio/chain/fork_database.hpp index fef3b12b68..9c841c2b8f 100644 --- a/libraries/chain/include/eosio/chain/fork_database.hpp +++ b/libraries/chain/include/eosio/chain/fork_database.hpp @@ -12,6 +12,7 @@ namespace eosio::chain { using block_branch_t = std::vector; enum class ignore_duplicate_t { no, yes }; enum class include_root_t { no, yes }; + enum class fork_db_add_t { duplicate, added, appended_to_head, fork_switch }; // Used for logging of comparison values used for best fork determination std::string log_fork_comparison(const block_state& bs); @@ -67,9 +68,14 @@ namespace eosio::chain { /** * Add block state to fork database. * Must link to existing block in fork database or the root. - * @return true if n becomes the new best head (and was not the best head before) + * @returns duplicate - already added and ignore_duplicate=true + * added - inserted into an existing branch or started a new branch, not best branch + * appended_to_head - new best head of current best branch + * fork_switch - new best head of new branch, fork switch to new branch + * @throws unlinkable_block_exception - unlinkable to any branch + * @throws fork_database_exception - no root, n is nullptr, protocol feature error, duplicate when ignore_duplicate=false */ - bool add( const bsp_t& n, ignore_duplicate_t ignore_duplicate ); + fork_db_add_t add( const bsp_t& n, ignore_duplicate_t ignore_duplicate ); void remove( const block_id_type& id ); @@ -306,3 +312,6 @@ namespace eosio::chain { static constexpr uint32_t max_supported_version = 3; }; } /// eosio::chain + +FC_REFLECT_ENUM( eosio::chain::fork_db_add_t, + (duplicate)(added)(appended_to_head)(fork_switch) ) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index 4685f0a8b5..900b47e85b 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -3722,7 +3723,7 @@ namespace eosio { } // this will return empty optional if block is not linkable controller::accepted_block_result abh = cc.accept_block( id, ptr ); - best_head = abh.is_new_best_head; + best_head = abh.add_result == fork_db_add_t::appended_to_head || abh.add_result == fork_db_add_t::fork_switch; obh = std::move(abh.block); unlinkable = !obh; close_mode = sync_manager::closing_mode::handshake; diff --git a/plugins/test_control_plugin/test_control_plugin.cpp b/plugins/test_control_plugin/test_control_plugin.cpp index 8644e56fa9..0423fdd0cb 100644 --- a/plugins/test_control_plugin/test_control_plugin.cpp +++ b/plugins/test_control_plugin/test_control_plugin.cpp @@ -1,4 +1,5 @@ #include +#include namespace eosio { @@ -160,9 +161,9 @@ void test_control_plugin_impl::swap_action_in_block(const chain::signed_block_pt copy_b->producer_signature = _swap_on_options.blk_priv_key.sign(copy_b->calculate_id()); // will be processed on the next start_block if is_new_best_head - const auto&[is_new_best_head, bh] = _chain.accept_block(copy_b->calculate_id(), copy_b); - ilog("Swapped action ${f} to ${t}, is_new_best_head ${bh}, block ${bn}", - ("f", _swap_on_options.from)("t", _swap_on_options.to)("bh", is_new_best_head)("bn", bh ? bh->block_num() : 0)); + const auto&[add_result, bh] = _chain.accept_block(copy_b->calculate_id(), copy_b); + ilog("Swapped action ${f} to ${t}, add_result ${a}, block ${bn}", + ("f", _swap_on_options.from)("t", _swap_on_options.to)("a", add_result)("bn", bh ? bh->block_num() : 0)); reset_swap_action(); } diff --git a/unittests/fork_db_tests.cpp b/unittests/fork_db_tests.cpp index 6ba25923b0..ac56827cec 100644 --- a/unittests/fork_db_tests.cpp +++ b/unittests/fork_db_tests.cpp @@ -7,8 +7,9 @@ namespace eosio::chain { +uint32_t nonce = 0; + inline block_id_type make_block_id(block_num_type block_num) { - static uint32_t nonce = 0; ++nonce; block_id_type id = fc::sha256::hash(std::to_string(block_num) + "-" + std::to_string(nonce)); id._hash[0] &= 0xffffffff00000000; @@ -57,26 +58,38 @@ using namespace eosio::chain; struct generate_fork_db_state { generate_fork_db_state() { fork_db.reset_root(root); - fork_db.add(bsp11a, ignore_duplicate_t::no); - fork_db.add(bsp11b, ignore_duplicate_t::no); - fork_db.add(bsp11c, ignore_duplicate_t::no); - fork_db.add(bsp12a, ignore_duplicate_t::no); - fork_db.add(bsp13a, ignore_duplicate_t::no); - fork_db.add(bsp12b, ignore_duplicate_t::no); - fork_db.add(bsp12bb, ignore_duplicate_t::no); - fork_db.add(bsp12bbb, ignore_duplicate_t::no); - fork_db.add(bsp12c, ignore_duplicate_t::no); - fork_db.add(bsp13b, ignore_duplicate_t::no); - fork_db.add(bsp13bb, ignore_duplicate_t::no); - fork_db.add(bsp13bbb, ignore_duplicate_t::no); - fork_db.add(bsp14b, ignore_duplicate_t::no); - fork_db.add(bsp13c, ignore_duplicate_t::no); + BOOST_TEST((fork_db.add(bsp11a, ignore_duplicate_t::no) == fork_db_add_t::appended_to_head)); + BOOST_TEST((fork_db.add(bsp11b, ignore_duplicate_t::no) == fork_db_add_t::added)); + BOOST_TEST((fork_db.add(bsp11c, ignore_duplicate_t::no) == fork_db_add_t::added)); + BOOST_TEST((fork_db.add(bsp12a, ignore_duplicate_t::no) == fork_db_add_t::appended_to_head)); + BOOST_TEST((fork_db.add(bsp13a, ignore_duplicate_t::no) == fork_db_add_t::appended_to_head)); + BOOST_TEST((fork_db.add(bsp12b, ignore_duplicate_t::no) == fork_db_add_t::added)); + BOOST_TEST((fork_db.add(bsp12bb, ignore_duplicate_t::no) == fork_db_add_t::added)); + BOOST_TEST((fork_db.add(bsp12bbb, ignore_duplicate_t::no) == fork_db_add_t::added)); + BOOST_TEST((fork_db.add(bsp12c, ignore_duplicate_t::no) == fork_db_add_t::added)); + BOOST_TEST((fork_db.add(bsp13b, ignore_duplicate_t::no) == fork_db_add_t::fork_switch)); + + // no fork_switch, because id is less + BOOST_TEST(bsp13bb->latest_qc_block_timestamp() == bsp13b->latest_qc_block_timestamp()); + BOOST_TEST(bsp13bb->timestamp() == bsp13b->timestamp()); + BOOST_TEST(bsp13bb->id() < bsp13b->id()); + BOOST_TEST((fork_db.add(bsp13bb, ignore_duplicate_t::no) == fork_db_add_t::added)); + + // fork_switch by id, everything else is the same + BOOST_TEST(bsp13bbb->latest_qc_block_timestamp() == bsp13b->latest_qc_block_timestamp()); + BOOST_TEST(bsp13bbb->timestamp() == bsp13b->timestamp()); + BOOST_TEST(bsp13bbb->id() > bsp13b->id()); + BOOST_TEST((fork_db.add(bsp13bbb, ignore_duplicate_t::no) == fork_db_add_t::fork_switch)); + + BOOST_TEST((fork_db.add(bsp14b, ignore_duplicate_t::no) == fork_db_add_t::fork_switch)); + BOOST_TEST((fork_db.add(bsp13c, ignore_duplicate_t::no) == fork_db_add_t::added)); } fork_database_if_t fork_db; // Setup fork database with blocks based on a root of block 10 // Add a number of forks in the fork database + bool reset_nonce = [&]() { nonce = 0; return true; }(); block_state_ptr root = test_block_state_accessor::make_genesis_block_state(); block_state_ptr bsp11a = test_block_state_accessor::make_unique_block_state(11, root); block_state_ptr bsp12a = test_block_state_accessor::make_unique_block_state(12, bsp11a); @@ -112,12 +125,12 @@ BOOST_FIXTURE_TEST_CASE(add_remove_test, generate_fork_db_state) try { BOOST_TEST(!fork_db.get_block(bsp12b->id())); BOOST_TEST(!fork_db.get_block(bsp13b->id())); BOOST_TEST(!fork_db.get_block(bsp14b->id())); - BOOST_TEST(!fork_db.add(bsp12b, ignore_duplicate_t::no)); // will throw if already exists + BOOST_TEST((fork_db.add(bsp12b, ignore_duplicate_t::no) == fork_db_add_t::added)); // will throw if already exists // 13b not the best branch because 13c has higher timestamp - BOOST_TEST(!fork_db.add(bsp13b, ignore_duplicate_t::no)); // will throw if already exists + BOOST_TEST((fork_db.add(bsp13b, ignore_duplicate_t::no) == fork_db_add_t::added)); // will throw if already exists // 14b has a higher timestamp than 13c - BOOST_TEST(fork_db.add(bsp14b, ignore_duplicate_t::no)); // will throw if already exists - BOOST_TEST(!fork_db.add(bsp14b, ignore_duplicate_t::yes)); + BOOST_TEST((fork_db.add(bsp14b, ignore_duplicate_t::no) == fork_db_add_t::fork_switch)); // will throw if already exists + BOOST_TEST((fork_db.add(bsp14b, ignore_duplicate_t::yes) == fork_db_add_t::duplicate)); // test search BOOST_TEST(fork_db.search_on_branch( bsp13bb->id(), 11) == bsp11b); @@ -143,7 +156,7 @@ BOOST_FIXTURE_TEST_CASE(add_remove_test, generate_fork_db_state) try { BOOST_TEST(branch[1] == bsp11a); auto bsp14c = test_block_state_accessor::make_unique_block_state(14, bsp13c); // should be best branch - BOOST_TEST(fork_db.add(bsp14c, ignore_duplicate_t::yes)); + BOOST_TEST((fork_db.add(bsp14c, ignore_duplicate_t::yes) == fork_db_add_t::fork_switch)); // test fetch branch when lib is greater than head branch = fork_db.fetch_branch(bsp13b->id(), bsp12a->id());