From 01050fc443566f54e2de6285db68001017c4338d Mon Sep 17 00:00:00 2001 From: Huang-Ming Huang Date: Thu, 15 Oct 2026 15:07:41 +0000 Subject: [PATCH 1/2] Fix WIRE-420 restart after interrupted chain initialization Change-Id: Id1d8f43d7ec9ac42bcbc8a544d71e5c94dc6fd27 --- libraries/chain/block_log.cpp | 5 +- libraries/chain/controller.cpp | 15 ++-- .../chaindb/include/chainbase/chainbase.hpp | 2 + .../chainbase/pinnable_mapped_file.hpp | 3 + .../chaindb/src/pinnable_mapped_file.cpp | 2 + unittests/block_log_extract.cpp | 25 +++++++ unittests/restart_chain_tests.cpp | 72 +++++++++++++++++++ 7 files changed, 116 insertions(+), 8 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index eeacf5debf..6dc7ee3dfa 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -1565,7 +1565,10 @@ namespace sysio { namespace chain { }); } - if (first_block_file.empty() && std::filesystem::exists(block_dir / "blocks.log")) { + // Opening a new controller creates an empty active log before chain startup. + // It has no preamble yet; retain the normal fallback to archived segments. + if (first_block_file.empty() && std::filesystem::exists(block_dir / "blocks.log") && + std::filesystem::file_size(block_dir / "blocks.log") > 0) { first_block_file = block_dir / "blocks.log"; } diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index 32c3e75b77..accbb3c38b 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -1595,6 +1595,11 @@ struct controller_impl { void add_indices() { controller_index_set::add_indices(db); + // Persist format identity before another plugin can fail initialization. + // Existing databases must pass the ordinary version checks, even at revision zero. + if (db.was_created() && db.get_index().size() == 0) { + db.create([](auto&) {}); + } kv_database_index_set::add_indices(db); authorization.add_indices(); @@ -1778,9 +1783,7 @@ struct controller_impl { }); db.set_revision( chain_head.block_num() ); - db.create([](const auto& header){ - // nothing to do - }); + validate_db_version(db); // --- Snapshot state validation --- // Singleton enforcement @@ -1867,10 +1870,8 @@ struct controller_impl { } void initialize_database(const genesis_state& genesis) { - // create the database header sigil - db.create([&]( auto& header ){ - // nothing to do for now - }); + // Index registration already persisted the header, including on a prior interrupted launch. + validate_db_version(db); // Initialize block summary index for (int i = 0; i < 0x10000; i++) diff --git a/libraries/chaindb/include/chainbase/chainbase.hpp b/libraries/chaindb/include/chainbase/chainbase.hpp index 89d31359a3..030199478b 100644 --- a/libraries/chaindb/include/chainbase/chainbase.hpp +++ b/libraries/chaindb/include/chainbase/chainbase.hpp @@ -246,6 +246,8 @@ namespace chainbase { database& operator=(const database&) = delete; bool is_read_only() const { return _read_only; } + /// Whether this instance created a new backing file rather than opening existing state. + bool was_created() const { return _db_file.was_created(); } void flush(); struct session { diff --git a/libraries/chaindb/include/chainbase/pinnable_mapped_file.hpp b/libraries/chaindb/include/chainbase/pinnable_mapped_file.hpp index b602069a23..587206e933 100644 --- a/libraries/chaindb/include/chainbase/pinnable_mapped_file.hpp +++ b/libraries/chaindb/include/chainbase/pinnable_mapped_file.hpp @@ -82,6 +82,8 @@ class pinnable_mapped_file { ~pinnable_mapped_file(); segment_manager* get_segment_manager() const { return _segment_manager;} + /// True only when this open created the database file, never inferred from its contents. + bool was_created() const { return _created; } size_t check_memory_and_flush_if_needed(); static ss_allocator_t* get_small_size_allocator(std::byte* seg_mgr); @@ -117,6 +119,7 @@ class pinnable_mapped_file { std::string _database_name; size_t _database_size = 0; bool _writable = false; + bool _created = false; bool _sharable = false; bip::file_mapping _file_mapping; diff --git a/libraries/chaindb/src/pinnable_mapped_file.cpp b/libraries/chaindb/src/pinnable_mapped_file.cpp index c0bd7f5fd2..61a0561683 100644 --- a/libraries/chaindb/src/pinnable_mapped_file.cpp +++ b/libraries/chaindb/src/pinnable_mapped_file.cpp @@ -137,6 +137,7 @@ pinnable_mapped_file::pinnable_mapped_file(const std::filesystem::path& dir, boo _file_mapped_region = bip::mapped_region(_file_mapping, bip::read_write); file_mapped_segment_manager = new ((char*)_file_mapped_region.get_address()+header_size) segment_manager(shared_file_size-header_size); new (_file_mapped_region.get_address()) db_header; + _created = true; } else if(_writable) { auto existing_file_size = std::filesystem::file_size(_data_file_path); @@ -457,6 +458,7 @@ pinnable_mapped_file& pinnable_mapped_file::operator=(pinnable_mapped_file&& o) std::swap(_database_name, o._database_name); std::swap(_database_size, o._database_size); std::swap(_writable, o._writable); + std::swap(_created, o._created); std::swap(_sharable, o._sharable); std::swap(_file_mapping, o._file_mapping); std::swap(_file_mapped_region, o._file_mapped_region); diff --git a/unittests/block_log_extract.cpp b/unittests/block_log_extract.cpp index 984a88d2a5..0321ef797b 100644 --- a/unittests/block_log_extract.cpp +++ b/unittests/block_log_extract.cpp @@ -4,6 +4,7 @@ #include #include +#include #include using namespace sysio::chain; @@ -49,6 +50,30 @@ struct block_log_extract_fixture { BOOST_AUTO_TEST_SUITE(block_log_extraction_tests) +BOOST_FIXTURE_TEST_CASE(empty_active_log_uses_retained_context, block_log_extract_fixture) { + fc::temp_directory active_dir; + fc::temp_directory retained_dir; + { + block_log uninitialized(active_dir.path()); + } + BOOST_CHECK(!block_log::extract_chain_context(active_dir.path(), {})); + // A segment starting after genesis exercises the fallback after the active log. + block_log::extract_block_range(dir.path(), retained_dir.path(), 3, 7); + auto id = block_log::extract_chain_id(active_dir.path(), retained_dir.path()); + BOOST_REQUIRE(id); + BOOST_CHECK(*id == gs.compute_chain_id()); +} + +BOOST_AUTO_TEST_CASE(empty_retained_log_is_rejected) { + fc::temp_directory active_dir; + fc::temp_directory retained_dir; + { + std::ofstream log(retained_dir.path() / "blocks-3-7.log", std::ios::binary); + } + BOOST_CHECK_THROW(block_log::extract_chain_context(active_dir.path(), retained_dir.path()), + std::ios_base::failure); +} + BOOST_FIXTURE_TEST_CASE(extract_from_middle, block_log_extract_fixture) try { fc::temp_directory output_dir; diff --git a/unittests/restart_chain_tests.cpp b/unittests/restart_chain_tests.cpp index 9cd5c72de9..a2e15b9cb4 100644 --- a/unittests/restart_chain_tests.cpp +++ b/unittests/restart_chain_tests.cpp @@ -2,8 +2,10 @@ #include #include +#include #include #include +#include #include #include @@ -87,6 +89,76 @@ class replay_tester : public base_tester { BOOST_AUTO_TEST_SUITE(restart_chain_tests) +/** Reproduce chain initialization followed by a later plugin failure, before chain startup. */ +BOOST_AUTO_TEST_CASE(restart_after_interrupted_plugin_initialization) { + tester reference; + auto cfg = reference.get_config(); + const auto genesis = block_log::extract_genesis_state(cfg.blocks_dir); + BOOST_REQUIRE(genesis); + fc::temp_directory directory; + cfg.state_dir = directory.path() / "state"; + cfg.blocks_dir = directory.path() / "blocks"; + cfg.finalizers_dir = directory.path() / "finalizers"; + for (unsigned attempt = 0; attempt < 2; ++attempt) { + { + controller interrupted(cfg, genesis->compute_chain_id()); + interrupted.add_indices(); + // The Ethereum startup gate used to throw at this lifecycle boundary. + } + BOOST_CHECK(!controller::extract_chain_id_from_db(cfg.state_dir)); + BOOST_REQUIRE(std::filesystem::exists(cfg.blocks_dir / "blocks.log")); + BOOST_CHECK_EQUAL(std::filesystem::file_size(cfg.blocks_dir / "blocks.log"), 0u); + BOOST_CHECK(!block_log::extract_chain_context(cfg.blocks_dir, {})); + } + { + controller restarted(cfg, make_protocol_feature_set(), genesis->compute_chain_id()); + restarted.add_indices(); + restarted.startup([] {}, [] { return false; }, *genesis); + BOOST_CHECK_EQUAL(restarted.db().get_index().size(), 1u); + } + BOOST_REQUIRE(controller::extract_chain_id_from_db(cfg.state_dir)); + BOOST_CHECK(*controller::extract_chain_id_from_db(cfg.state_dir) == genesis->compute_chain_id()); +} + +/** Only an empty active log is uninitialized; partial headers must still fail. */ +BOOST_AUTO_TEST_CASE(restart_rejects_truncated_block_log) { + fc::temp_directory directory; + { + std::ofstream log(directory.path() / "blocks.log", std::ios::binary); + log.put('\0'); + } + BOOST_CHECK_THROW(block_log::extract_chain_context(directory.path(), {}), std::ios_base::failure); +} + +/** Revision zero does not permit incompatible or absent version headers on existing state. */ +BOOST_AUTO_TEST_CASE(restart_rejects_incompatible_or_missing_database_header) { + tester reference; + auto cfg = reference.get_config(); + fc::temp_directory directory; + cfg.state_dir = directory.path() / "state"; + cfg.blocks_dir = directory.path() / "blocks"; + cfg.finalizers_dir = directory.path() / "finalizers"; + { + controller interrupted(cfg, reference.control->get_chain_id()); + interrupted.add_indices(); + } + { + chainbase::database db(cfg.state_dir, chainbase::database::read_write, cfg.state_size); + db.add_index(); + const auto& header = db.get(); + db.modify(header, [](auto& value) { value.version = database_header_object::current_version + 1; }); + } + BOOST_CHECK_THROW(controller::extract_chain_id_from_db(cfg.state_dir), bad_database_version_exception); + { + chainbase::database db(cfg.state_dir, chainbase::database::read_write, cfg.state_size); + db.add_index(); + db.remove(db.get()); + db.add_index(); + db.create([](auto&) {}); + } + BOOST_CHECK_THROW(controller::extract_chain_id_from_db(cfg.state_dir), bad_database_version_exception); +} + BOOST_AUTO_TEST_CASE_TEMPLATE( test_existing_state_without_block_log, T, testers ) { T chain; From d3700f56a90c662ba1e703cc52ff4012844d5529 Mon Sep 17 00:00:00 2001 From: Huang-Ming Huang Date: Tue, 20 Oct 2026 20:19:50 +0000 Subject: [PATCH 2/2] Address PR review feedback on interrupted initialization Change-Id: I947ddc4ef7d132c901082ac5480466cf0438b994 --- libraries/chain/block_log.cpp | 16 +++-- libraries/chain/controller.cpp | 23 ++++--- .../chaindb/include/chainbase/chainbase.hpp | 2 - .../chainbase/pinnable_mapped_file.hpp | 3 - .../chaindb/src/pinnable_mapped_file.cpp | 2 - unittests/block_log_extract.cpp | 15 +++-- unittests/restart_chain_tests.cpp | 63 ++++--------------- 7 files changed, 43 insertions(+), 81 deletions(-) diff --git a/libraries/chain/block_log.cpp b/libraries/chain/block_log.cpp index 6dc7ee3dfa..a4ea1b7d93 100644 --- a/libraries/chain/block_log.cpp +++ b/libraries/chain/block_log.cpp @@ -15,6 +15,12 @@ static_assert(__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__); namespace sysio { namespace chain { + namespace { + constexpr auto empty_active_block_log_message = + "{} is empty: the launch that created it did not complete startup. " + "Rerun once with --delete-all-blocks."; + } + enum versions { initial_version = 1, ///< complete block log from genesis block_x_start_version = 2, ///< adds optional partial block log, cannot be used for replay without snapshot @@ -1565,11 +1571,11 @@ namespace sysio { namespace chain { }); } - // Opening a new controller creates an empty active log before chain startup. - // It has no preamble yet; retain the normal fallback to archived segments. - if (first_block_file.empty() && std::filesystem::exists(block_dir / "blocks.log") && - std::filesystem::file_size(block_dir / "blocks.log") > 0) { - first_block_file = block_dir / "blocks.log"; + const auto active_log = block_dir / "blocks.log"; + if (first_block_file.empty() && std::filesystem::exists(active_log)) { + SYS_ASSERT(std::filesystem::file_size(active_log) > 0, block_log_exception, + empty_active_block_log_message, active_log.generic_string()); + first_block_file = active_log; } if (!first_block_file.empty()) { diff --git a/libraries/chain/controller.cpp b/libraries/chain/controller.cpp index accbb3c38b..c7aa365f3b 100644 --- a/libraries/chain/controller.cpp +++ b/libraries/chain/controller.cpp @@ -51,6 +51,12 @@ namespace sysio::chain { +namespace { +constexpr auto missing_database_header_message = + "State database has no version header: the launch that created it did not complete startup. " + "Rerun once with --delete-all-blocks."; +} + using resource_limits::resource_limits_manager; using controller_index_set = index_set< @@ -1456,7 +1462,7 @@ struct controller_impl { const auto& header_idx = db.get_index().indices().get(); SYS_ASSERT(header_idx.begin() != header_idx.end(), bad_database_version_exception, - "state database version pre-dates versioning, please restore from a compatible snapshot or replay!"); + missing_database_header_message); auto header_itr = header_idx.begin(); header_itr->validate(); @@ -1595,11 +1601,6 @@ struct controller_impl { void add_indices() { controller_index_set::add_indices(db); - // Persist format identity before another plugin can fail initialization. - // Existing databases must pass the ordinary version checks, even at revision zero. - if (db.was_created() && db.get_index().size() == 0) { - db.create([](auto&) {}); - } kv_database_index_set::add_indices(db); authorization.add_indices(); @@ -1783,7 +1784,9 @@ struct controller_impl { }); db.set_revision( chain_head.block_num() ); - validate_db_version(db); + db.create([](const auto& header){ + // nothing to do + }); // --- Snapshot state validation --- // Singleton enforcement @@ -1870,8 +1873,10 @@ struct controller_impl { } void initialize_database(const genesis_state& genesis) { - // Index registration already persisted the header, including on a prior interrupted launch. - validate_db_version(db); + // create the database header sigil + db.create([&]( auto& header ){ + // nothing to do for now + }); // Initialize block summary index for (int i = 0; i < 0x10000; i++) diff --git a/libraries/chaindb/include/chainbase/chainbase.hpp b/libraries/chaindb/include/chainbase/chainbase.hpp index 030199478b..89d31359a3 100644 --- a/libraries/chaindb/include/chainbase/chainbase.hpp +++ b/libraries/chaindb/include/chainbase/chainbase.hpp @@ -246,8 +246,6 @@ namespace chainbase { database& operator=(const database&) = delete; bool is_read_only() const { return _read_only; } - /// Whether this instance created a new backing file rather than opening existing state. - bool was_created() const { return _db_file.was_created(); } void flush(); struct session { diff --git a/libraries/chaindb/include/chainbase/pinnable_mapped_file.hpp b/libraries/chaindb/include/chainbase/pinnable_mapped_file.hpp index 587206e933..b602069a23 100644 --- a/libraries/chaindb/include/chainbase/pinnable_mapped_file.hpp +++ b/libraries/chaindb/include/chainbase/pinnable_mapped_file.hpp @@ -82,8 +82,6 @@ class pinnable_mapped_file { ~pinnable_mapped_file(); segment_manager* get_segment_manager() const { return _segment_manager;} - /// True only when this open created the database file, never inferred from its contents. - bool was_created() const { return _created; } size_t check_memory_and_flush_if_needed(); static ss_allocator_t* get_small_size_allocator(std::byte* seg_mgr); @@ -119,7 +117,6 @@ class pinnable_mapped_file { std::string _database_name; size_t _database_size = 0; bool _writable = false; - bool _created = false; bool _sharable = false; bip::file_mapping _file_mapping; diff --git a/libraries/chaindb/src/pinnable_mapped_file.cpp b/libraries/chaindb/src/pinnable_mapped_file.cpp index 61a0561683..c0bd7f5fd2 100644 --- a/libraries/chaindb/src/pinnable_mapped_file.cpp +++ b/libraries/chaindb/src/pinnable_mapped_file.cpp @@ -137,7 +137,6 @@ pinnable_mapped_file::pinnable_mapped_file(const std::filesystem::path& dir, boo _file_mapped_region = bip::mapped_region(_file_mapping, bip::read_write); file_mapped_segment_manager = new ((char*)_file_mapped_region.get_address()+header_size) segment_manager(shared_file_size-header_size); new (_file_mapped_region.get_address()) db_header; - _created = true; } else if(_writable) { auto existing_file_size = std::filesystem::file_size(_data_file_path); @@ -458,7 +457,6 @@ pinnable_mapped_file& pinnable_mapped_file::operator=(pinnable_mapped_file&& o) std::swap(_database_name, o._database_name); std::swap(_database_size, o._database_size); std::swap(_writable, o._writable); - std::swap(_created, o._created); std::swap(_sharable, o._sharable); std::swap(_file_mapping, o._file_mapping); std::swap(_file_mapped_region, o._file_mapped_region); diff --git a/unittests/block_log_extract.cpp b/unittests/block_log_extract.cpp index 0321ef797b..6c84720d6e 100644 --- a/unittests/block_log_extract.cpp +++ b/unittests/block_log_extract.cpp @@ -50,18 +50,17 @@ struct block_log_extract_fixture { BOOST_AUTO_TEST_SUITE(block_log_extraction_tests) -BOOST_FIXTURE_TEST_CASE(empty_active_log_uses_retained_context, block_log_extract_fixture) { +/** An active log created before interrupted startup has no preamble to read. */ +BOOST_AUTO_TEST_CASE(empty_active_log_reports_interrupted_initialization) { fc::temp_directory active_dir; - fc::temp_directory retained_dir; { block_log uninitialized(active_dir.path()); } - BOOST_CHECK(!block_log::extract_chain_context(active_dir.path(), {})); - // A segment starting after genesis exercises the fallback after the active log. - block_log::extract_block_range(dir.path(), retained_dir.path(), 3, 7); - auto id = block_log::extract_chain_id(active_dir.path(), retained_dir.path()); - BOOST_REQUIRE(id); - BOOST_CHECK(*id == gs.compute_chain_id()); + const auto expected = (active_dir.path() / "blocks.log").generic_string() + + " is empty: the launch that created it did not complete startup. " + "Rerun once with --delete-all-blocks."; + BOOST_REQUIRE_EXCEPTION(block_log::extract_chain_context(active_dir.path(), {}), block_log_exception, + [&](const auto& ex) { return ex.top_message() == expected; }); } BOOST_AUTO_TEST_CASE(empty_retained_log_is_rejected) { diff --git a/unittests/restart_chain_tests.cpp b/unittests/restart_chain_tests.cpp index a2e15b9cb4..d6010e95e7 100644 --- a/unittests/restart_chain_tests.cpp +++ b/unittests/restart_chain_tests.cpp @@ -2,7 +2,6 @@ #include #include -#include #include #include #include @@ -89,38 +88,27 @@ class replay_tester : public base_tester { BOOST_AUTO_TEST_SUITE(restart_chain_tests) -/** Reproduce chain initialization followed by a later plugin failure, before chain startup. */ -BOOST_AUTO_TEST_CASE(restart_after_interrupted_plugin_initialization) { +/** An interrupted initialization leaves a state database without a version header. */ +BOOST_AUTO_TEST_CASE(interrupted_initialization_reports_missing_database_header) { tester reference; auto cfg = reference.get_config(); - const auto genesis = block_log::extract_genesis_state(cfg.blocks_dir); - BOOST_REQUIRE(genesis); fc::temp_directory directory; cfg.state_dir = directory.path() / "state"; cfg.blocks_dir = directory.path() / "blocks"; cfg.finalizers_dir = directory.path() / "finalizers"; - for (unsigned attempt = 0; attempt < 2; ++attempt) { - { - controller interrupted(cfg, genesis->compute_chain_id()); - interrupted.add_indices(); - // The Ethereum startup gate used to throw at this lifecycle boundary. - } - BOOST_CHECK(!controller::extract_chain_id_from_db(cfg.state_dir)); - BOOST_REQUIRE(std::filesystem::exists(cfg.blocks_dir / "blocks.log")); - BOOST_CHECK_EQUAL(std::filesystem::file_size(cfg.blocks_dir / "blocks.log"), 0u); - BOOST_CHECK(!block_log::extract_chain_context(cfg.blocks_dir, {})); - } { - controller restarted(cfg, make_protocol_feature_set(), genesis->compute_chain_id()); - restarted.add_indices(); - restarted.startup([] {}, [] { return false; }, *genesis); - BOOST_CHECK_EQUAL(restarted.db().get_index().size(), 1u); + controller interrupted(cfg, reference.control->get_chain_id()); + interrupted.add_indices(); } - BOOST_REQUIRE(controller::extract_chain_id_from_db(cfg.state_dir)); - BOOST_CHECK(*controller::extract_chain_id_from_db(cfg.state_dir) == genesis->compute_chain_id()); + BOOST_REQUIRE_EXCEPTION(controller::extract_chain_id_from_db(cfg.state_dir), bad_database_version_exception, + [](const auto& ex) { + return ex.top_message() == + "State database has no version header: the launch that created it did not complete startup. " + "Rerun once with --delete-all-blocks."; + }); } -/** Only an empty active log is uninitialized; partial headers must still fail. */ +/** A partially written active log still reports its original read failure. */ BOOST_AUTO_TEST_CASE(restart_rejects_truncated_block_log) { fc::temp_directory directory; { @@ -130,35 +118,6 @@ BOOST_AUTO_TEST_CASE(restart_rejects_truncated_block_log) { BOOST_CHECK_THROW(block_log::extract_chain_context(directory.path(), {}), std::ios_base::failure); } -/** Revision zero does not permit incompatible or absent version headers on existing state. */ -BOOST_AUTO_TEST_CASE(restart_rejects_incompatible_or_missing_database_header) { - tester reference; - auto cfg = reference.get_config(); - fc::temp_directory directory; - cfg.state_dir = directory.path() / "state"; - cfg.blocks_dir = directory.path() / "blocks"; - cfg.finalizers_dir = directory.path() / "finalizers"; - { - controller interrupted(cfg, reference.control->get_chain_id()); - interrupted.add_indices(); - } - { - chainbase::database db(cfg.state_dir, chainbase::database::read_write, cfg.state_size); - db.add_index(); - const auto& header = db.get(); - db.modify(header, [](auto& value) { value.version = database_header_object::current_version + 1; }); - } - BOOST_CHECK_THROW(controller::extract_chain_id_from_db(cfg.state_dir), bad_database_version_exception); - { - chainbase::database db(cfg.state_dir, chainbase::database::read_write, cfg.state_size); - db.add_index(); - db.remove(db.get()); - db.add_index(); - db.create([](auto&) {}); - } - BOOST_CHECK_THROW(controller::extract_chain_id_from_db(cfg.state_dir), bad_database_version_exception); -} - BOOST_AUTO_TEST_CASE_TEMPLATE( test_existing_state_without_block_log, T, testers ) { T chain;