From 78bbc62f0844b2fcbbc690e93327ce62c52493c7 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 22 Mar 2024 15:18:06 -0500 Subject: [PATCH 1/9] GH-2286 Add an irreversible node to test --- tests/transition_to_if.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/transition_to_if.py b/tests/transition_to_if.py index 0263fb44f0..5966083c6b 100755 --- a/tests/transition_to_if.py +++ b/tests/transition_to_if.py @@ -22,7 +22,7 @@ topo=args.s debug=args.v prod_count = 1 # per node prod count -total_nodes=pnodes +total_nodes=pnodes+1 dumpErrorDetails=args.dump_error_details Utils.Debug=debug @@ -41,8 +41,9 @@ numTrxGenerators=2 Print("Stand up cluster") # For now do not load system contract as it does not support setfinalizer + specificExtraNodeosArgs = { 4: "--read-mode irreversible"} if cluster.launch(pnodes=pnodes, totalNodes=total_nodes, prodCount=prod_count, maximumP2pPerHost=total_nodes+numTrxGenerators, topo=topo, delay=delay, loadSystemContract=False, - activateIF=False) is False: + activateIF=False, specificExtraNodeosArgs=specificExtraNodeosArgs) is False: errorExit("Failed to stand up eos cluster.") assert cluster.biosNode.getInfo(exitOnError=True)["head_block_producer"] != "eosio", "launch should have waited for production to change" @@ -63,7 +64,9 @@ assert cluster.biosNode.waitForLibToAdvance(), "Lib should advance after instant finality activated" assert cluster.biosNode.waitForProducer("defproducera"), "Did not see defproducera" assert cluster.biosNode.waitForHeadToAdvance(blocksToAdvance=13) # into next producer - assert cluster.biosNode.waitForLibToAdvance(), "Lib stopped advancing" + assert cluster.biosNode.waitForLibToAdvance(), "Lib stopped advancing on biosNode" + assert cluster.getNode(1).waitForLibToAdvance(), "Lib stopped advancing on Node 1" + assert cluster.getNode(4).waitForLibToAdvance(), "Lib stopped advancing on Node 4, irreversible node" info = cluster.biosNode.getInfo(exitOnError=True) assert (info["head_block_num"] - info["last_irreversible_block_num"]) < 9, "Instant finality enabled LIB diff should be small" @@ -76,8 +79,9 @@ # should take effect in first block of defproducerg slot (so defproducerh) assert cluster.setProds(["defproducerb", "defproducerh", "defproducerm", "defproducerr"]), "setprods failed" setProdsBlockNum = cluster.biosNode.getBlockNum() - cluster.biosNode.waitForBlock(setProdsBlockNum+12+12+1) + assert cluster.biosNode.waitForBlock(setProdsBlockNum+12+12+1), "Block of new producers not reached" assert cluster.biosNode.getInfo(exitOnError=True)["head_block_producer"] == "defproducerh", "setprods should have taken effect" + assert cluster.getNode(4).waitForBlock(setProdsBlockNum + 12 + 12 + 1), "Block of new producers not reached on irreversible node" testSuccessful=True finally: From b9ac212f9f619bc5499470c6ec02c8c2a875ff4e Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 22 Mar 2024 15:18:39 -0500 Subject: [PATCH 2/9] GH-2161 Uncomment tests. --- tests/CMakeLists.txt | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 4a12e8a90b..e9da8bb115 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -219,7 +219,6 @@ add_test(NAME keosd_auto_launch_test COMMAND tests/keosd_auto_launch_test.py WOR set_property(TEST keosd_auto_launch_test PROPERTY LABELS nonparallelizable_tests) add_test(NAME nodeos_snapshot_diff_test COMMAND tests/nodeos_snapshot_diff_test.py -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST nodeos_snapshot_diff_test PROPERTY LABELS nonparallelizable_tests) -# requires https://github.com/AntelopeIO/leap/issues/1558 add_test(NAME nodeos_snapshot_diff_if_test COMMAND tests/nodeos_snapshot_diff_test.py -v --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST nodeos_snapshot_diff_if_test PROPERTY LABELS nonparallelizable_tests) add_test(NAME nodeos_snapshot_forked_test COMMAND tests/nodeos_snapshot_forked_test.py -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) @@ -287,15 +286,13 @@ set_property(TEST nodeos_under_min_avail_ram_if_lr_test PROPERTY LABELS long_run add_test(NAME nodeos_irreversible_mode_lr_test COMMAND tests/nodeos_irreversible_mode_test.py -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST nodeos_irreversible_mode_lr_test PROPERTY LABELS long_running_tests) -# requires https://github.com/AntelopeIO/leap/issues/2286 -#add_test(NAME nodeos_irreversible_mode_if_lr_test COMMAND tests/nodeos_irreversible_mode_test.py -v --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) -#set_property(TEST nodeos_irreversible_mode_if_lr_test PROPERTY LABELS long_running_tests) +add_test(NAME nodeos_irreversible_mode_if_lr_test COMMAND tests/nodeos_irreversible_mode_test.py -v --activate-if ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) +set_property(TEST nodeos_irreversible_mode_if_lr_test PROPERTY LABELS long_running_tests) add_test(NAME nodeos_read_terminate_at_block_lr_test COMMAND tests/nodeos_read_terminate_at_block_test.py -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST nodeos_read_terminate_at_block_lr_test PROPERTY LABELS long_running_tests) -# requires https://github.com/AntelopeIO/leap/issues/2057 because running in irreversible mode currently switches different than non-irreversible mode -#add_test(NAME nodeos_read_terminate_at_block_if_lr_test COMMAND tests/nodeos_read_terminate_at_block_test.py --activate-if -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) -#set_property(TEST nodeos_read_terminate_at_block_if_lr_test PROPERTY LABELS long_running_tests) +add_test(NAME nodeos_read_terminate_at_block_if_lr_test COMMAND tests/nodeos_read_terminate_at_block_test.py --activate-if -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) +set_property(TEST nodeos_read_terminate_at_block_if_lr_test PROPERTY LABELS long_running_tests) add_test(NAME liveness_test COMMAND tests/liveness_test.py -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST liveness_test PROPERTY LABELS nonparallelizable_tests) From be96431d2348fb2ed4e9066334a32fed6f8a4e7d Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 22 Mar 2024 15:19:04 -0500 Subject: [PATCH 3/9] GH-2161 For LIB to advance need to make bios not a finalizer --- tests/nodeos_irreversible_mode_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/nodeos_irreversible_mode_test.py b/tests/nodeos_irreversible_mode_test.py index dd60f6e1f9..144e8793c6 100755 --- a/tests/nodeos_irreversible_mode_test.py +++ b/tests/nodeos_irreversible_mode_test.py @@ -166,6 +166,7 @@ def relaunchNode(node: Node, chainArg="", addSwapFlags=None, relaunchAssertMessa totalNodes=totalNodes, pnodes=1, activateIF=activateIF, + biosFinalizer=False, topo="mesh", specificExtraNodeosArgs={ 0:"--enable-stale-production", From 988261e14eb74d52609528eebfd3d7bb9eefdfb2 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 22 Mar 2024 15:19:23 -0500 Subject: [PATCH 4/9] GH-2161 Add better log statement --- plugins/net_plugin/net_plugin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/net_plugin/net_plugin.cpp b/plugins/net_plugin/net_plugin.cpp index ccef14248f..2af7109e0e 100644 --- a/plugins/net_plugin/net_plugin.cpp +++ b/plugins/net_plugin/net_plugin.cpp @@ -3854,7 +3854,7 @@ namespace eosio { fc::microseconds age( fc::time_point::now() - block->timestamp); fc_dlog( logger, "received signed_block: #${n} block age in secs = ${age}, connection ${cid}, ${v}, lib #${lib}", - ("n", blk_num)("age", age.to_seconds())("cid", c->connection_id)("v", obt ? "pre-validated" : "validation pending")("lib", lib) ); + ("n", blk_num)("age", age.to_seconds())("cid", c->connection_id)("v", obt ? "header validated" : "header validation pending")("lib", lib) ); go_away_reason reason = no_reason; bool accepted = false; From 32258f4c8bdce62809e591080c47cd47da0d4dcb Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Fri, 22 Mar 2024 16:05:50 -0500 Subject: [PATCH 5/9] GH-2161 Add support for transition to savanna in irreversible mode --- libraries/chain/controller.cpp | 68 +++++++++++++++---- libraries/chain/fork_database.cpp | 10 +++ .../include/eosio/chain/fork_database.hpp | 4 ++ plugins/producer_plugin/producer_plugin.cpp | 3 + 4 files changed, 71 insertions(+), 14 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index a2d642aaec..eb5f135223 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1246,6 +1246,41 @@ struct controller_impl { } } + template + void apply_irreversible_block(ForkDB& forkdb, const BSP& bsp) { + if( read_mode == db_read_mode::IRREVERSIBLE ) { + controller::block_report br; + if constexpr (std::is_same_v>) { + // before transition to savanna + apply_block(br, bsp, controller::block_status::complete, trx_meta_cache_lookup{}); + } else { + if (bsp->block->is_proper_svnn_block()) { + // if chain_head is legacy, update to non-legacy chain_head, this is needed so that the correct block_state is created in apply_block + if (std::holds_alternative(chain_head.internal())) { + chain_head = block_handle{forkdb.get_block(bsp->previous(), include_root_t::yes)}; + } + apply_block(br, bsp, controller::block_status::complete, trx_meta_cache_lookup{}); + } else { + // only called during transition when not a proper savanna block + fork_db.apply_l([&](const auto& forkdb_l) { + block_state_legacy_ptr legacy = forkdb_l.get_block(bsp->id()); + fork_db.switch_to_legacy(); // apply block uses to know what types to create + fc::scoped_exit> e([&]{fork_db.switch_to_both();}); + apply_block(br, legacy, controller::block_status::complete, trx_meta_cache_lookup{}); + // irreversible apply was just done, calculate new_valid here instead of in transition_to_savanna() + assert(legacy->action_receipt_digests); + auto action_mroot = calculate_merkle(*legacy->action_receipt_digests); + // Create the valid structure for producing + auto prev = forkdb.get_block(legacy->previous(), include_root_t::yes); + assert(prev); + bsp->valid = prev->new_valid(*bsp, action_mroot); + forkdb.add(bsp, bsp->is_valid() ? mark_valid_t::yes : mark_valid_t::no, ignore_duplicate_t::yes); + }); + } + } + } + } + void transition_to_savanna() { assert(chain_head.header().contains_header_extension(instant_finality_extension::extension_id())); // copy head branch branch from legacy forkdb legacy to savanna forkdb @@ -1253,7 +1288,7 @@ struct controller_impl { block_state_legacy_ptr legacy_root; fork_db.apply_l([&](const auto& forkdb) { legacy_root = forkdb.root(); - legacy_branch = forkdb.fetch_branch(forkdb.head()->id()); + legacy_branch = forkdb.fetch_branch(fork_db_head(forkdb, irreversible_mode())->id()); }); assert(!!legacy_root); @@ -1263,6 +1298,7 @@ struct controller_impl { fork_db.switch_from_legacy(new_root); fork_db.apply_s([&](auto& forkdb) { block_state_ptr prev = forkdb.root(); + assert(prev); for (auto bitr = legacy_branch.rbegin(); bitr != legacy_branch.rend(); ++bitr) { const bool skip_validate_signee = true; // validated already auto new_bsp = std::make_shared( @@ -1270,25 +1306,29 @@ struct controller_impl { (*bitr)->block, protocol_features.get_protocol_feature_set(), validator_t{}, skip_validate_signee); - // legacy_branch is from head, all should be validated - assert((*bitr)->action_receipt_digests); - auto action_mroot = calculate_merkle(*(*bitr)->action_receipt_digests); - // Create the valid structure for producing - new_bsp->valid = prev->new_valid(*new_bsp, action_mroot); + // legacy_branch is from head, all will be validated unless irreversible_mode(), + // IRREVERSIBLE applies (validates) blocks when irreversible, new_valid will be done after apply in log_irreversible + assert(read_mode == db_read_mode::IRREVERSIBLE || (*bitr)->action_receipt_digests); + if ((*bitr)->action_receipt_digests) { + auto action_mroot = calculate_merkle(*(*bitr)->action_receipt_digests); + // Create the valid structure for producing + new_bsp->valid = prev->new_valid(*new_bsp, action_mroot); + } forkdb.add(new_bsp, (*bitr)->is_valid() ? mark_valid_t::yes : mark_valid_t::no, ignore_duplicate_t::yes); prev = new_bsp; } assert(read_mode == db_read_mode::IRREVERSIBLE || forkdb.head()->id() == legacy_branch.front()->id()); - chain_head = block_handle{forkdb.head()}; + if (read_mode != db_read_mode::IRREVERSIBLE) + chain_head = block_handle{forkdb.head()}; + ilog("Transition to instant finality happening after block ${b}, First IF Proper Block ${pb}", ("b", prev->block_num())("pb", prev->block_num()+1)); }); - ilog("Transition to instant finality happening after block ${b}, First IF Proper Block ${pb}", ("b", chain_head.block_num())("pb", chain_head.block_num()+1)); { // If Leap started at a block prior to the IF transition, it needs to provide a default safety // information for those finalizers that don't already have one. This typically should be done when // we create the non-legacy fork_db, as from this point we may need to cast votes to participate // to the IF consensus. See https://github.com/AntelopeIO/leap/issues/2070#issuecomment-1941901836 - auto start_block = chain_head; + auto start_block = chain_head; // doesn't matter this is not updated for IRREVERSIBLE, can be in irreversible mode and be a finalizer auto lib_block = chain_head; my_finalizers.set_default_safety_information( finalizer_safety_information{ .last_vote_range_start = block_timestamp_type(0), @@ -1345,10 +1385,7 @@ struct controller_impl { auto it = v.begin(); for( auto bitr = branch.rbegin(); bitr != branch.rend() && should_process(*bitr); ++bitr ) { - if( read_mode == db_read_mode::IRREVERSIBLE ) { - controller::block_report br; - apply_block( br, *bitr, controller::block_status::complete, trx_meta_cache_lookup{} ); - } + apply_irreversible_block(forkdb, *bitr); emit( irreversible_block, std::tie((*bitr)->block, (*bitr)->id()) ); @@ -1366,8 +1403,9 @@ struct controller_impl { // Do not advance irreversible past IF Genesis Block break; } - } else if ((*bitr)->block->is_proper_svnn_block()) { + } else if ((*bitr)->block->is_proper_svnn_block() && fork_db.version_in_use() == fork_database::in_use_t::both) { fork_db.switch_to_savanna(); + break; } } } catch( std::exception& ) { @@ -3057,6 +3095,7 @@ struct controller_impl { if (s != controller::block_status::irreversible) { auto add_completed_block = [&](auto& forkdb) { + assert(std::holds_alternative>(cb.bsp.internal())); const auto& bsp = std::get>(cb.bsp.internal()); if( s == controller::block_status::incomplete ) { forkdb.add( bsp, mark_valid_t::yes, ignore_duplicate_t::no ); @@ -3084,6 +3123,7 @@ struct controller_impl { if( s == controller::block_status::incomplete ) { fork_db.apply_s([&](auto& forkdb) { + assert(std::holds_alternative>(cb.bsp.internal())); const auto& bsp = std::get>(cb.bsp.internal()); uint16_t if_ext_id = instant_finality_extension::extension_id(); diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index 5bafbd03e9..57c9fae4b9 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -804,6 +804,16 @@ namespace eosio::chain { in_use = in_use_t::savanna; } + // only called from the main thread + void fork_database::switch_to_legacy() { + in_use = in_use_t::legacy; + } + + // only called from the main thread + void fork_database::switch_to_both() { + in_use = in_use_t::both; + } + block_branch_t fork_database::fetch_branch_from_head() const { return apply([&](auto& forkdb) { return forkdb.fetch_block_branch(forkdb.head()->id()); diff --git a/libraries/chain/include/eosio/chain/fork_database.hpp b/libraries/chain/include/eosio/chain/fork_database.hpp index 4795cf5fa8..12742b2fae 100644 --- a/libraries/chain/include/eosio/chain/fork_database.hpp +++ b/libraries/chain/include/eosio/chain/fork_database.hpp @@ -157,6 +157,10 @@ namespace eosio::chain { // switches to using both legacy and savanna during transition void switch_from_legacy(const block_state_ptr& root); void switch_to_savanna(); + // used in irreversible mode + void switch_to_legacy(); + // used in irreversible mode + void switch_to_both(); in_use_t version_in_use() const { return in_use.load(); } diff --git a/plugins/producer_plugin/producer_plugin.cpp b/plugins/producer_plugin/producer_plugin.cpp index 8edbb8b55d..f43668489d 100644 --- a/plugins/producer_plugin/producer_plugin.cpp +++ b/plugins/producer_plugin/producer_plugin.cpp @@ -1347,6 +1347,9 @@ void producer_plugin_impl::plugin_startup() { EOS_ASSERT(_producers.empty() || chain.get_read_mode() != chain::db_read_mode::IRREVERSIBLE, plugin_config_exception, "node cannot have any producer-name configured because block production is impossible when read_mode is \"irreversible\""); + EOS_ASSERT(_finalizer_keys.empty() || chain.get_read_mode() != chain::db_read_mode::IRREVERSIBLE, plugin_config_exception, + "node cannot have any finalizers configured because finalization is impossible when read_mode is \"irreversible\""); + EOS_ASSERT(_producers.empty() || chain.get_validation_mode() == chain::validation_mode::FULL, plugin_config_exception, "node cannot have any producer-name configured because block production is not safe when validation_mode is not \"full\""); From 1b7351d4e2e22e96fd9166b9334b25410f424101 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 25 Mar 2024 17:04:47 -0500 Subject: [PATCH 6/9] GH-2286 Add message to assert. --- tests/transition_to_if.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/transition_to_if.py b/tests/transition_to_if.py index 5966083c6b..3df75ac27a 100755 --- a/tests/transition_to_if.py +++ b/tests/transition_to_if.py @@ -63,7 +63,7 @@ assert cluster.biosNode.waitForLibToAdvance(), "Lib should advance after instant finality activated" assert cluster.biosNode.waitForProducer("defproducera"), "Did not see defproducera" - assert cluster.biosNode.waitForHeadToAdvance(blocksToAdvance=13) # into next producer + assert cluster.biosNode.waitForHeadToAdvance(blocksToAdvance=13), "Head did not advance 13 blocks to next producer" assert cluster.biosNode.waitForLibToAdvance(), "Lib stopped advancing on biosNode" assert cluster.getNode(1).waitForLibToAdvance(), "Lib stopped advancing on Node 1" assert cluster.getNode(4).waitForLibToAdvance(), "Lib stopped advancing on Node 4, irreversible node" From 6f47ddb56790051973dd3c4fbfac70a386529b62 Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Mon, 25 Mar 2024 17:05:20 -0500 Subject: [PATCH 7/9] GH-2886 Create method to remove duplicate code --- libraries/chain/controller.cpp | 78 ++++++++++++++++++---------------- 1 file changed, 42 insertions(+), 36 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index eb5f135223..242f563586 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1248,39 +1248,53 @@ struct controller_impl { template void apply_irreversible_block(ForkDB& forkdb, const BSP& bsp) { - if( read_mode == db_read_mode::IRREVERSIBLE ) { - controller::block_report br; - if constexpr (std::is_same_v>) { - // before transition to savanna + if (read_mode != db_read_mode::IRREVERSIBLE) + return; + controller::block_report br; + if constexpr (std::is_same_v>) { + // before transition to savanna + apply_block(br, bsp, controller::block_status::complete, trx_meta_cache_lookup{}); + } else { + assert(bsp->block); + if (bsp->block->is_proper_svnn_block()) { + // if chain_head is legacy, update to non-legacy chain_head, this is needed so that the correct block_state is created in apply_block + if (std::holds_alternative(chain_head.internal())) { + auto prev = forkdb.get_block(bsp->previous(), include_root_t::yes); + assert(prev); + chain_head = block_handle{prev}; + } apply_block(br, bsp, controller::block_status::complete, trx_meta_cache_lookup{}); } else { - if (bsp->block->is_proper_svnn_block()) { - // if chain_head is legacy, update to non-legacy chain_head, this is needed so that the correct block_state is created in apply_block - if (std::holds_alternative(chain_head.internal())) { - chain_head = block_handle{forkdb.get_block(bsp->previous(), include_root_t::yes)}; - } - apply_block(br, bsp, controller::block_status::complete, trx_meta_cache_lookup{}); - } else { - // only called during transition when not a proper savanna block - fork_db.apply_l([&](const auto& forkdb_l) { - block_state_legacy_ptr legacy = forkdb_l.get_block(bsp->id()); - fork_db.switch_to_legacy(); // apply block uses to know what types to create - fc::scoped_exit> e([&]{fork_db.switch_to_both();}); - apply_block(br, legacy, controller::block_status::complete, trx_meta_cache_lookup{}); - // irreversible apply was just done, calculate new_valid here instead of in transition_to_savanna() - assert(legacy->action_receipt_digests); - auto action_mroot = calculate_merkle(*legacy->action_receipt_digests); - // Create the valid structure for producing - auto prev = forkdb.get_block(legacy->previous(), include_root_t::yes); - assert(prev); - bsp->valid = prev->new_valid(*bsp, action_mroot); - forkdb.add(bsp, bsp->is_valid() ? mark_valid_t::yes : mark_valid_t::no, ignore_duplicate_t::yes); - }); - } + // only called during transition when not a proper savanna block + fork_db.apply_l([&](const auto& forkdb_l) { + block_state_legacy_ptr legacy = forkdb_l.get_block(bsp->id()); + fork_db.switch_to_legacy(); // apply block uses to know what types to create + fc::scoped_exit> e([&]{fork_db.switch_to_both();}); + apply_block(br, legacy, controller::block_status::complete, trx_meta_cache_lookup{}); + // irreversible apply was just done, calculate new_valid here instead of in transition_to_savanna() + assert(legacy->action_receipt_digests); + auto prev = forkdb.get_block(legacy->previous(), include_root_t::yes); + assert(prev); + transition_add_to_savanna_fork_db(forkdb, legacy, bsp, prev); + }); } } } + void transition_add_to_savanna_fork_db(fork_database_if_t& forkdb, + const block_state_legacy_ptr& legacy, const block_state_ptr& new_bsp, + const block_state_ptr& prev) { + // legacy_branch is from head, all will be validated unless irreversible_mode(), + // IRREVERSIBLE applies (validates) blocks when irreversible, new_valid will be done after apply in log_irreversible + assert(read_mode == db_read_mode::IRREVERSIBLE || legacy->action_receipt_digests); + if (legacy->action_receipt_digests) { + auto action_mroot = calculate_merkle(*legacy->action_receipt_digests); + // Create the valid structure for producing + new_bsp->valid = prev->new_valid(*new_bsp, action_mroot); + } + forkdb.add(new_bsp, legacy->is_valid() ? mark_valid_t::yes : mark_valid_t::no, ignore_duplicate_t::yes); + } + void transition_to_savanna() { assert(chain_head.header().contains_header_extension(instant_finality_extension::extension_id())); // copy head branch branch from legacy forkdb legacy to savanna forkdb @@ -1306,15 +1320,7 @@ struct controller_impl { (*bitr)->block, protocol_features.get_protocol_feature_set(), validator_t{}, skip_validate_signee); - // legacy_branch is from head, all will be validated unless irreversible_mode(), - // IRREVERSIBLE applies (validates) blocks when irreversible, new_valid will be done after apply in log_irreversible - assert(read_mode == db_read_mode::IRREVERSIBLE || (*bitr)->action_receipt_digests); - if ((*bitr)->action_receipt_digests) { - auto action_mroot = calculate_merkle(*(*bitr)->action_receipt_digests); - // Create the valid structure for producing - new_bsp->valid = prev->new_valid(*new_bsp, action_mroot); - } - forkdb.add(new_bsp, (*bitr)->is_valid() ? mark_valid_t::yes : mark_valid_t::no, ignore_duplicate_t::yes); + transition_add_to_savanna_fork_db(forkdb, *bitr, new_bsp, prev); prev = new_bsp; } assert(read_mode == db_read_mode::IRREVERSIBLE || forkdb.head()->id() == legacy_branch.front()->id()); From e92e4ad87a7ebb9a408a28db342dd8cd698ffcbc Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 26 Mar 2024 17:03:21 -0500 Subject: [PATCH 8/9] GH-2286 Misc cleanup --- libraries/chain/controller.cpp | 16 ++++++++-------- libraries/chain/fork_database.cpp | 15 --------------- .../chain/include/eosio/chain/fork_database.hpp | 6 +----- 3 files changed, 9 insertions(+), 28 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 242f563586..6ba03268bd 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1257,23 +1257,23 @@ struct controller_impl { } else { assert(bsp->block); if (bsp->block->is_proper_svnn_block()) { - // if chain_head is legacy, update to non-legacy chain_head, this is needed so that the correct block_state is created in apply_block - if (std::holds_alternative(chain_head.internal())) { - auto prev = forkdb.get_block(bsp->previous(), include_root_t::yes); + apply_l(chain_head, [&](const auto&) { + // if chain_head is legacy, update to non-legacy chain_head, this is needed so that the correct block_state is created in apply_block + block_state_ptr prev = forkdb.get_block(bsp->previous(), include_root_t::yes); assert(prev); chain_head = block_handle{prev}; - } + }); apply_block(br, bsp, controller::block_status::complete, trx_meta_cache_lookup{}); } else { // only called during transition when not a proper savanna block fork_db.apply_l([&](const auto& forkdb_l) { block_state_legacy_ptr legacy = forkdb_l.get_block(bsp->id()); - fork_db.switch_to_legacy(); // apply block uses to know what types to create - fc::scoped_exit> e([&]{fork_db.switch_to_both();}); + fork_db.switch_to(fork_database::in_use_t::legacy); // apply block uses to know what types to create + fc::scoped_exit> e([&]{fork_db.switch_to(fork_database::in_use_t::both);}); apply_block(br, legacy, controller::block_status::complete, trx_meta_cache_lookup{}); // irreversible apply was just done, calculate new_valid here instead of in transition_to_savanna() assert(legacy->action_receipt_digests); - auto prev = forkdb.get_block(legacy->previous(), include_root_t::yes); + 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); }); @@ -1410,7 +1410,7 @@ struct controller_impl { break; } } else if ((*bitr)->block->is_proper_svnn_block() && fork_db.version_in_use() == fork_database::in_use_t::both) { - fork_db.switch_to_savanna(); + fork_db.switch_to(fork_database::in_use_t::savanna); break; } } diff --git a/libraries/chain/fork_database.cpp b/libraries/chain/fork_database.cpp index 57c9fae4b9..5cda182450 100644 --- a/libraries/chain/fork_database.cpp +++ b/libraries/chain/fork_database.cpp @@ -799,21 +799,6 @@ namespace eosio::chain { } } - // only called from the main thread - void fork_database::switch_to_savanna() { - in_use = in_use_t::savanna; - } - - // only called from the main thread - void fork_database::switch_to_legacy() { - in_use = in_use_t::legacy; - } - - // only called from the main thread - void fork_database::switch_to_both() { - in_use = in_use_t::both; - } - block_branch_t fork_database::fetch_branch_from_head() const { return apply([&](auto& forkdb) { return forkdb.fetch_block_branch(forkdb.head()->id()); diff --git a/libraries/chain/include/eosio/chain/fork_database.hpp b/libraries/chain/include/eosio/chain/fork_database.hpp index 12742b2fae..4f26eb547a 100644 --- a/libraries/chain/include/eosio/chain/fork_database.hpp +++ b/libraries/chain/include/eosio/chain/fork_database.hpp @@ -156,11 +156,7 @@ namespace eosio::chain { // switches to using both legacy and savanna during transition void switch_from_legacy(const block_state_ptr& root); - void switch_to_savanna(); - // used in irreversible mode - void switch_to_legacy(); - // used in irreversible mode - void switch_to_both(); + void switch_to(in_use_t v) { in_use = v; } in_use_t version_in_use() const { return in_use.load(); } From dc029db38129c86e94c1a33f057f95ee061f365b Mon Sep 17 00:00:00 2001 From: Kevin Heifner Date: Tue, 26 Mar 2024 17:29:34 -0500 Subject: [PATCH 9/9] GH-2286 Simplify by adding fork_db_head_or_pending() --- libraries/chain/controller.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 6ba03268bd..ba87e9e668 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -984,8 +984,8 @@ struct controller_impl { } template - typename ForkDB::bsp_t fork_db_head(const ForkDB& forkdb, bool irreversible_mode) const { - if (irreversible_mode) { + typename ForkDB::bsp_t fork_db_head_or_pending(const ForkDB& forkdb) const { + if (irreversible_mode()) { // When in IRREVERSIBLE mode fork_db blocks are marked valid when they become irreversible so that // fork_db.head() returns irreversible block // Use pending_head since this method should return the chain head and not last irreversible. @@ -997,17 +997,17 @@ struct controller_impl { uint32_t fork_db_head_block_num() const { return fork_db.apply( - [&](const auto& forkdb) { return fork_db_head(forkdb, irreversible_mode())->block_num(); }); + [&](const auto& forkdb) { return fork_db_head_or_pending(forkdb)->block_num(); }); } block_id_type fork_db_head_block_id() const { return fork_db.apply( - [&](const auto& forkdb) { return fork_db_head(forkdb, irreversible_mode())->id(); }); + [&](const auto& forkdb) { return fork_db_head_or_pending(forkdb)->id(); }); } uint32_t fork_db_head_irreversible_blocknum() const { return fork_db.apply( - [&](const auto& forkdb) { return fork_db_head(forkdb, irreversible_mode())->irreversible_blocknum(); }); + [&](const auto& forkdb) { return fork_db_head_or_pending(forkdb)->irreversible_blocknum(); }); } // --------------- access fork_db root ---------------------------------------------------------------------- @@ -1302,7 +1302,7 @@ struct controller_impl { block_state_legacy_ptr legacy_root; fork_db.apply_l([&](const auto& forkdb) { legacy_root = forkdb.root(); - legacy_branch = forkdb.fetch_branch(fork_db_head(forkdb, irreversible_mode())->id()); + legacy_branch = forkdb.fetch_branch(fork_db_head_or_pending(forkdb)->id()); }); assert(!!legacy_root); @@ -1371,7 +1371,7 @@ struct controller_impl { bool savanna_transistion_required = false; auto mark_branch_irreversible = [&, this](auto& forkdb) { auto branch = (if_lib_num > 0) ? forkdb.fetch_branch( if_irreversible_block_id, new_lib_num) - : forkdb.fetch_branch( fork_db_head(forkdb, irreversible_mode())->id(), new_lib_num ); + : forkdb.fetch_branch( fork_db_head_or_pending(forkdb)->id(), new_lib_num ); try { auto should_process = [&](auto& bsp) { // Only make irreversible blocks that have been validated. Blocks in the fork database may not be on our current best head