refactor: Add ChainstateManager::ValidatedChainstate() method
What changed, and why it matters
This is a simple internal code cleanup in Bitcoin Core. It renames and slightly rewrites a helper function used to pick which copy of the blockchain data indexes should look at. There is no change in behavior and no security issue visible in the commit.
No action required. This is a non-security refactor.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors ChainstateManager by replacing GetChainstateForIndexing() with a new inline method ValidatedChainstate(). The new method iterates over CurrentChainstate() and HistoricalChainstate(), returning the one whose m_assumeutxo state is Assumeutxo::VALIDATED, and aborts if none is found. The old method selected m_ibd_chainstate if multiple chainstates existed, otherwise m_active_chainstate. The commit message explicitly states ‘no change in behavior.’ All call sites are updated to use the new accessor. No functional, consensus, or security-relevant change is evident.
Changed components
src/validation.hsrc/validation.cppsrc/index/base.cppsrc/init.cppInspect captured patch +13 / −20
diff --git a/src/index/base.cpp b/src/index/base.cpp
index 6d424804..a94b4ca7 100644
--- a/src/index/base.cpp
+++ b/src/index/base.cpp
@@ -109,7 +109,7 @@ bool BaseIndex::Init()
// m_chainstate member gives indexing code access to node internals. It is
// removed in followup https://github.com/bitcoin/bitcoin/pull/24230
m_chainstate = WITH_LOCK(::cs_main,
- return &m_chain->context()->chainman->GetChainstateForIndexing());
+ return &m_chain->context()->chainman->ValidatedChainstate());
// Register to validation interface before setting the 'm_synced' flag, so that
// callbacks are not missed once m_synced is true.
m_chain->context()->validation_signals->RegisterValidationInterface(this);
diff --git a/src/init.cpp b/src/init.cpp
index 593467e3..9c710d69 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -2229,7 +2229,7 @@ bool StartIndexBackgroundSync(NodeContext& node)
std::optional<const CBlockIndex*> indexes_start_block;
std::string older_index_name;
ChainstateManager& chainman = *Assert(node.chainman);
- const Chainstate& chainstate = WITH_LOCK(::cs_main, return chainman.GetChainstateForIndexing());
+ const Chainstate& chainstate = WITH_LOCK(::cs_main, return chainman.ValidatedChainstate());
const CChain& index_chain = chainstate.m_chain;
for (auto index : node.indexes) {
diff --git a/src/validation.cpp b/src/validation.cpp
index fabff80c..da60efbc 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -6437,14 +6437,6 @@ bool ChainstateManager::ValidatedSnapshotCleanup(Chainstate& validated_cs, Chain
return true;
}
-Chainstate& ChainstateManager::GetChainstateForIndexing()
-{
- // We can't always return `m_ibd_chainstate` because after background validation
- // has completed, `m_snapshot_chainstate == m_active_chainstate`, but it can be
- // indexed.
- return (this->GetAll().size() > 1) ? *m_ibd_chainstate : *m_active_chainstate;
-}
-
std::pair<int, int> ChainstateManager::GetPruneRange(const Chainstate& chainstate, int last_height_can_prune)
{
if (chainstate.m_chain.Height() <= 0) {
diff --git a/src/validation.h b/src/validation.h
index cafe2493..464dcbd0 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -1153,6 +1153,17 @@ public:
return cs && cs->m_target_blockhash && !cs->m_target_utxohash ? cs : nullptr;
}
+ //! Return fully validated chainstate that should be used for indexing, to
+ //! support indexes that need to index blocks in order and can't start from
+ //! the snapshot block.
+ Chainstate& ValidatedChainstate() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex())
+ {
+ for (auto* cs : {&CurrentChainstate(), HistoricalChainstate()}) {
+ if (cs && cs->m_assumeutxo == Assumeutxo::VALIDATED) return *cs;
+ }
+ abort();
+ }
+
//! Alternatives to CurrentChainstate() used by older code to query latest
//! chainstate information without locking cs_main. Newer code should avoid
//! querying ChainstateManager and use Chainstate objects directly, or
@@ -1344,16 +1355,6 @@ public:
//! @sa node/chainstate:LoadChainstate()
bool ValidatedSnapshotCleanup(Chainstate& validated_cs, Chainstate& unvalidated_cs) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
- //! @returns the chainstate that indexes should consult when ensuring that an
- //! index is synced with a chain where we can expect block index entries to have
- //! BLOCK_HAVE_DATA beneath the tip.
- //!
- //! In other words, give us the chainstate for which we can reasonably expect
- //! that all blocks beneath the tip have been indexed. In practice this means
- //! when using an assumed-valid chainstate based upon a snapshot, return only the
- //! fully validated chain.
- Chainstate& GetChainstateForIndexing() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
-
//! Return the [start, end] (inclusive) of block heights we can prune.
//!
//! start > end is possible, meaning no blocks can be pruned.
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.