refactor: Move LoadGenesisBlock to ChainstateManager
What changed, and why it matters
This commit is a straightforward internal code reorganization: it moves the LoadGenesisBlock function from the Chainstate class to the ChainstateManager class, marks its return value as important to check, and slightly rewords two rare error log messages. There is no change to how Bitcoin Core validates blocks, handles transactions, or protects against attacks. It does not fix a security bug or introduce a known vulnerability.
No security action needed. Treat as normal refactoring during code review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch relocates LoadGenesisBlock from Chainstate to ChainstateManager because the function only needs ChainstateManager-level state (params, m_blockman, m_best_header) and no per-chainstate data. Call sites are updated from chainman.ActiveChainstate().LoadGenesisBlock() to chainman.LoadGenesisBlock(). The method is annotated [[nodiscard]], and the one call site that intentionally ignores the return value is cast to (void). Error log strings drop the func prefix because -logsourcelocations already provides it. No consensus, networking, or cryptographic logic is modified.
Changed components
src/validation.cppsrc/validation.hsrc/node/blockstorage.cppsrc/node/chainstate.cppsrc/test/util/chainstate.hsrc/test/validation_chainstate_tests.cppInspect captured patch +17 / −16
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index 8726c741..8771c353 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -1305,7 +1305,7 @@ void ImportBlocks(ChainstateManager& chainman, std::span<const fs::path> import_
chainman.m_blockman.m_blockfiles_indexed = true;
LogInfo("Reindexing finished");
// To avoid ending up in a situation without genesis block, re-try initializing (no-op if reindexing worked):
- chainman.ActiveChainstate().LoadGenesisBlock();
+ (void)chainman.LoadGenesisBlock();
}
// -loadblock=
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
index 1725fe70..c27447b7 100644
--- a/src/node/chainstate.cpp
+++ b/src/node/chainstate.cpp
@@ -62,7 +62,7 @@ static ChainstateLoadResult CompleteChainstateInitialization(
// If we're not mid-reindex (based on disk + args), add a genesis block on disk
// (otherwise we use the one already on disk).
// This is called again in ImportBlocks after the reindex completes.
- if (chainman.m_blockman.m_blockfiles_indexed && !chainman.ActiveChainstate().LoadGenesisBlock()) {
+ if (chainman.m_blockman.m_blockfiles_indexed && !chainman.LoadGenesisBlock()) {
return {ChainstateLoadStatus::FAILURE, _("Error initializing block database")};
}
diff --git a/src/test/util/chainstate.h b/src/test/util/chainstate.h
index 48a3381a..aeec14cc 100644
--- a/src/test/util/chainstate.h
+++ b/src/test/util/chainstate.h
@@ -80,7 +80,7 @@ CreateAndActivateUTXOSnapshot(
node.chainman->ResetChainstates();
node.chainman->InitializeChainstate(node.mempool.get());
Chainstate& chain = node.chainman->ActiveChainstate();
- Assert(chain.LoadGenesisBlock());
+ Assert(node.chainman->LoadGenesisBlock());
// These cache values will be corrected shortly in `MaybeRebalanceCaches`.
chain.InitCoinsDB(1_MiB, /*in_memory=*/true, /*should_wipe=*/false);
chain.InitCoinsCache(1_MiB);
diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp
index 141c67da..bcbb553d 100644
--- a/src/test/validation_chainstate_tests.cpp
+++ b/src/test/validation_chainstate_tests.cpp
@@ -43,7 +43,7 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches)
c1.InitCoinsDB(
/*cache_size_bytes=*/8_MiB, /*in_memory=*/true, /*should_wipe=*/false);
WITH_LOCK(::cs_main, c1.InitCoinsCache(8_MiB));
- BOOST_REQUIRE(c1.LoadGenesisBlock()); // Need at least one block loaded to be able to flush caches
+ BOOST_REQUIRE(manager.LoadGenesisBlock()); // Need at least one block loaded to be able to flush caches
// Add a coin to the in-memory cache, upsize once, then downsize.
{
diff --git a/src/validation.cpp b/src/validation.cpp
index 5341c604..8069f51b 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -4943,30 +4943,30 @@ bool ChainstateManager::LoadBlockIndex()
return true;
}
-bool Chainstate::LoadGenesisBlock()
+bool ChainstateManager::LoadGenesisBlock()
{
LOCK(cs_main);
- const CChainParams& params{m_chainman.GetParams()};
+ const CBlock& genesis_block{GetParams().GenesisBlock()};
// Check whether we're already initialized by checking for genesis in
- // m_blockman.m_block_index. Note that we can't use m_chain here, since it is
+ // m_blockman.m_block_index. Note that we can't use a chainstate's m_chain here, since it is
// set based on the coins db, not the block index db, which is the only
// thing loaded at this point.
- if (m_blockman.m_block_index.contains(params.GenesisBlock().GetHash()))
+ if (m_blockman.m_block_index.contains(genesis_block.GetHash())) {
return true;
+ }
try {
- const CBlock& block = params.GenesisBlock();
- FlatFilePos blockPos{m_blockman.WriteBlock(block, 0)};
+ FlatFilePos blockPos{m_blockman.WriteBlock(genesis_block, 0)};
if (blockPos.IsNull()) {
- LogError("%s: writing genesis block to disk failed\n", __func__);
+ LogError("Writing genesis block to disk failed");
return false;
}
- CBlockIndex* pindex = m_blockman.AddToBlockIndex(block, m_chainman.m_best_header);
- m_chainman.ReceivedBlockTransactions(block, pindex, blockPos);
+ CBlockIndex* pindex{m_blockman.AddToBlockIndex(genesis_block, m_best_header)};
+ ReceivedBlockTransactions(genesis_block, pindex, blockPos);
} catch (const std::runtime_error& e) {
- LogError("%s: failed to write genesis block: %s\n", __func__, e.what());
+ LogError("Failed to write genesis block: %s", e.what());
return false;
}
diff --git a/src/validation.h b/src/validation.h
index 4cb5dc63..2627cdce 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -809,8 +809,6 @@ public:
/** Whether the chain state needs to be redownloaded due to lack of witness data */
[[nodiscard]] bool NeedsRedownload() const EXCLUSIVE_LOCKS_REQUIRED(cs_main);
- /** Ensures we have a genesis block in the block tree, possibly writing one to disk. */
- bool LoadGenesisBlock();
/** Add a block to the candidate set if it has as much work as the current tip. */
void TryAddBlockIndexCandidate(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
@@ -1085,6 +1083,9 @@ public:
//! coins databases. This will be split somehow across chainstates.
size_t m_total_coinsdb_cache{0};
+ /// Ensures a genesis block is in the block tree, possibly writing one to disk.
+ [[nodiscard]] bool LoadGenesisBlock();
+
//! Instantiate a new chainstate.
//!
//! @param[in] mempool The mempool to pass to the chainstate
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.