refactor: Delete ChainstateManager::GetAll() method
What changed, and why it matters
This commit is a straightforward internal code cleanup in Bitcoin Core. It removes a helper method called GetAll() and replaces its uses with direct access to an internal list of chainstates. The change also adds a small safety check to ignore chainstates based on invalid snapshots when considering new block candidates. There is no indication this fixes an active security bug or introduces a vulnerability.
No security action required. Treat as normal code maintenance and review for correctness during routine code review.
Security signals we found
No security-relevant signals present in commit message or diff
Refactoring only: removal of helper method and replacement with direct member access
Added defensive early-return for invalid-snapshot chainstates in TryAddBlockIndexCandidate
No mention of CVE, vulnerability, bug fix, or security issue by the vendor
Evidence from the diff
The patch refactors ChainstateManager by deleting the GetAll() method, which previously returned a filtered vector of active Chainstate pointers while holding cs_main. Callers now iterate m_chainstates directly. The filtering logic (skip INVALID assumeutxo and target_utxohash chainstates) is moved explicitly into ActivateBestChains(). Additionally, TryAddBlockIndexCandidate() now early-returns if the chainstate’s m_assumeutxo is INVALID. Other changes update tests and prune-target calculation to use HistoricalChainstate() instead of GetAll().size().
Changed components
src/validation.cppsrc/validation.hsrc/init.cppsrc/kernel/bitcoinkernel.cppsrc/node/blockstorage.cppsrc/node/chainstate.cppsrc/test/util/validation.cppsrc/test/validation_chainstatemanager_tests.cppInspect captured patch +95 / −80
diff --git a/src/init.cpp b/src/init.cpp
index 9c710d69..80055292 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -345,7 +345,7 @@ void Shutdown(NodeContext& node)
// FlushStateToDisk generates a ChainStateFlushed callback, which we should avoid missing
if (node.chainman) {
LOCK(cs_main);
- for (Chainstate* chainstate : node.chainman->GetAll()) {
+ for (const auto& chainstate : node.chainman->m_chainstates) {
if (chainstate->CanFlushToDisk()) {
chainstate->ForceFlushStateToDisk();
}
@@ -371,7 +371,7 @@ void Shutdown(NodeContext& node)
if (node.chainman) {
LOCK(cs_main);
- for (Chainstate* chainstate : node.chainman->GetAll()) {
+ for (const auto& chainstate : node.chainman->m_chainstates) {
if (chainstate->CanFlushToDisk()) {
chainstate->ForceFlushStateToDisk();
chainstate->ResetCoinsViews();
@@ -1873,7 +1873,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
if (chainman.m_blockman.IsPruneMode()) {
if (chainman.m_blockman.m_blockfiles_indexed) {
LOCK(cs_main);
- for (Chainstate* chainstate : chainman.GetAll()) {
+ for (const auto& chainstate : chainman.m_chainstates) {
uiInterface.InitMessage(_("Pruning blockstore…"));
chainstate->PruneAndFlush();
}
diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp
index 5bb1a06b..15812baf 100644
--- a/src/kernel/bitcoinkernel.cpp
+++ b/src/kernel/bitcoinkernel.cpp
@@ -1009,7 +1009,7 @@ void btck_chainstate_manager_destroy(btck_ChainstateManager* chainman)
{
{
LOCK(btck_ChainstateManager::get(chainman).m_chainman->GetMutex());
- for (Chainstate* chainstate : btck_ChainstateManager::get(chainman).m_chainman->GetAll()) {
+ for (const auto& chainstate : btck_ChainstateManager::get(chainman).m_chainman->m_chainstates) {
if (chainstate->CanFlushToDisk()) {
chainstate->ForceFlushStateToDisk();
chainstate->ResetCoinsViews();
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index 0bb22d90..ec95f11b 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -318,9 +318,16 @@ void BlockManager::FindFilesToPrune(
ChainstateManager& chainman)
{
LOCK2(cs_main, cs_LastBlockFile);
- // Distribute our -prune budget over all chainstates.
+ // Compute `target` value with maximum size (in bytes) of blocks below the
+ // `last_prune` height which should be preserved and not pruned. The
+ // `target` value will be derived from the -prune preference provided by the
+ // user. If there is a historical chainstate being used to populate indexes
+ // and validate the snapshot, the target is divided by two so half of the
+ // block storage will be reserved for the historical chainstate, and the
+ // other half will be reserved for the most-work chainstate.
+ const int num_chainstates{chainman.HistoricalChainstate() ? 2 : 1};
const auto target = std::max(
- MIN_DISK_SPACE_FOR_BLOCK_FILES, GetPruneTarget() / chainman.GetAll().size());
+ MIN_DISK_SPACE_FOR_BLOCK_FILES, GetPruneTarget() / num_chainstates);
const uint64_t target_sync_height = chainman.m_best_header->nHeight;
if (chain.m_chain.Height() < 0 || target == 0) {
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
index 59d6de63..4a8f8240 100644
--- a/src/node/chainstate.cpp
+++ b/src/node/chainstate.cpp
@@ -66,8 +66,8 @@ static ChainstateLoadResult CompleteChainstateInitialization(
return {ChainstateLoadStatus::FAILURE, _("Error initializing block database")};
}
- auto is_coinsview_empty = [&](Chainstate* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
- return options.wipe_chainstate_db || chainstate->CoinsTip().GetBestBlock().IsNull();
+ auto is_coinsview_empty = [&](Chainstate& chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
+ return options.wipe_chainstate_db || chainstate.CoinsTip().GetBestBlock().IsNull();
};
assert(chainman.m_total_coinstip_cache > 0);
@@ -78,12 +78,12 @@ static ChainstateLoadResult CompleteChainstateInitialization(
// recalculated by `chainman.MaybeRebalanceCaches()`. The discount factor
// is conservatively chosen such that the sum of the caches does not exceed
// the allowable amount during this temporary initialization state.
- double init_cache_fraction = chainman.GetAll().size() > 1 ? 0.2 : 1.0;
+ double init_cache_fraction = chainman.HistoricalChainstate() ? 0.2 : 1.0;
// At this point we're either in reindex or we've loaded a useful
// block tree into BlockIndex()!
- for (Chainstate* chainstate : chainman.GetAll()) {
+ for (const auto& chainstate : chainman.m_chainstates) {
LogInfo("Initializing chainstate %s", chainstate->ToString());
try {
@@ -117,7 +117,7 @@ static ChainstateLoadResult CompleteChainstateInitialization(
chainstate->InitCoinsCache(chainman.m_total_coinstip_cache * init_cache_fraction);
assert(chainstate->CanFlushToDisk());
- if (!is_coinsview_empty(chainstate)) {
+ if (!is_coinsview_empty(*chainstate)) {
// LoadChainTip initializes the chain based on CoinsTip()'s best block
if (!chainstate->LoadChainTip()) {
return {ChainstateLoadStatus::FAILURE, _("Error initializing block database")};
@@ -126,9 +126,9 @@ static ChainstateLoadResult CompleteChainstateInitialization(
}
}
- auto chainstates{chainman.GetAll()};
+ const auto& chainstates{chainman.m_chainstates};
if (std::any_of(chainstates.begin(), chainstates.end(),
- [](const Chainstate* cs) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return cs->NeedsRedownload(); })) {
+ [](const auto& cs) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return cs->NeedsRedownload(); })) {
return {ChainstateLoadStatus::FAILURE, strprintf(_("Witness data for blocks after height %d requires validation. Please restart with -reindex."),
chainman.GetConsensus().SegwitHeight)};
};
@@ -209,7 +209,7 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
// Because ValidatedSnapshotCleanup() has torn down chainstates with
// ChainstateManager::ResetChainstates(), reinitialize them here without
// duplicating the blockindex work above.
- assert(chainman.GetAll().empty());
+ assert(chainman.m_chainstates.empty());
chainman.InitializeChainstate(options.mempool);
@@ -232,14 +232,14 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, const ChainstateLoadOptions& options)
{
- auto is_coinsview_empty = [&](Chainstate* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
- return options.wipe_chainstate_db || chainstate->CoinsTip().GetBestBlock().IsNull();
+ auto is_coinsview_empty = [&](Chainstate& chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
+ return options.wipe_chainstate_db || chainstate.CoinsTip().GetBestBlock().IsNull();
};
LOCK(cs_main);
- for (Chainstate* chainstate : chainman.GetAll()) {
- if (!is_coinsview_empty(chainstate)) {
+ for (auto& chainstate : chainman.m_chainstates) {
+ if (!is_coinsview_empty(*chainstate)) {
const CBlockIndex* tip = chainstate->m_chain.Tip();
if (tip && tip->nTime > GetTime() + MAX_FUTURE_BLOCK_TIME) {
return {ChainstateLoadStatus::FAILURE, _("The block database contains a block which appears to be from the future. "
diff --git a/src/test/util/validation.cpp b/src/test/util/validation.cpp
index ae8642f9..c199d912 100644
--- a/src/test/util/validation.cpp
+++ b/src/test/util/validation.cpp
@@ -16,8 +16,9 @@ void TestChainstateManager::DisableNextWrite()
struct TestChainstate : public Chainstate {
void ResetNextWrite() { m_next_write = NodeClock::time_point::max() - 1s; }
};
- for (auto* cs : GetAll()) {
- static_cast<TestChainstate*>(cs)->ResetNextWrite();
+ LOCK(::cs_main);
+ for (const auto& cs : m_chainstates) {
+ static_cast<TestChainstate&>(*cs).ResetNextWrite();
}
}
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index 5a1ff8cd..539ae453 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -40,18 +40,19 @@ BOOST_FIXTURE_TEST_SUITE(validation_chainstatemanager_tests, TestingSetup)
BOOST_FIXTURE_TEST_CASE(chainstatemanager, TestChain100Setup)
{
ChainstateManager& manager = *m_node.chainman;
- std::vector<Chainstate*> chainstates;
BOOST_CHECK(WITH_LOCK(::cs_main, return !manager.CurrentChainstate().m_from_snapshot_blockhash));
// Create a legacy (IBD) chainstate.
//
Chainstate& c1 = manager.ActiveChainstate();
- chainstates.push_back(&c1);
BOOST_CHECK(WITH_LOCK(::cs_main, return !manager.CurrentChainstate().m_from_snapshot_blockhash));
- auto all = manager.GetAll();
- BOOST_CHECK_EQUAL_COLLECTIONS(all.begin(), all.end(), chainstates.begin(), chainstates.end());
+ {
+ LOCK(manager.GetMutex());
+ BOOST_CHECK_EQUAL(manager.m_chainstates.size(), 1);
+ BOOST_CHECK_EQUAL(manager.m_chainstates[0].get(), &c1);
+ }
auto& active_chain = WITH_LOCK(manager.GetMutex(), return manager.ActiveChain());
BOOST_CHECK_EQUAL(&active_chain, &c1.m_chain);
@@ -69,7 +70,6 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager, TestChain100Setup)
//
const uint256 snapshot_blockhash = active_tip->GetBlockHash();
Chainstate& c2{WITH_LOCK(::cs_main, return manager.AddChainstate(std::make_unique<Chainstate>(nullptr, manager.m_blockman, manager, snapshot_blockhash)))};
- chainstates.push_back(&c2);
c2.InitCoinsDB(
/*cache_size_bytes=*/1 << 23, /*in_memory=*/true, /*should_wipe=*/false);
{
@@ -86,8 +86,12 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager, TestChain100Setup)
BOOST_CHECK(WITH_LOCK(::cs_main, return manager.CurrentChainstate().m_assumeutxo == Assumeutxo::UNVALIDATED));
BOOST_CHECK_EQUAL(&c2, &manager.ActiveChainstate());
BOOST_CHECK(&c1 != &manager.ActiveChainstate());
- auto all2 = manager.GetAll();
- BOOST_CHECK_EQUAL_COLLECTIONS(all2.begin(), all2.end(), chainstates.begin(), chainstates.end());
+ {
+ LOCK(manager.GetMutex());
+ BOOST_CHECK_EQUAL(manager.m_chainstates.size(), 2);
+ BOOST_CHECK_EQUAL(manager.m_chainstates[0].get(), &c1);
+ BOOST_CHECK_EQUAL(manager.m_chainstates[1].get(), &c2);
+ }
auto& active_chain2 = WITH_LOCK(manager.GetMutex(), return manager.ActiveChain());
BOOST_CHECK_EQUAL(&active_chain2, &c2.m_chain);
@@ -292,7 +296,7 @@ struct SnapshotTestSetup : TestChain100Setup {
LOCK(::cs_main);
int chains_tested{0};
- for (Chainstate* chainstate : chainman.GetAll()) {
+ for (const auto& chainstate : chainman.m_chainstates) {
BOOST_TEST_MESSAGE("Checking coins in " << chainstate->ToString());
CCoinsViewCache& coinscache = chainstate->CoinsTip();
@@ -325,10 +329,10 @@ struct SnapshotTestSetup : TestChain100Setup {
size_t coins_in_background{0};
size_t coins_missing_from_background{0};
- for (Chainstate* chainstate : chainman.GetAll()) {
+ for (const auto& chainstate : chainman.m_chainstates) {
BOOST_TEST_MESSAGE("Checking coins in " << chainstate->ToString());
CCoinsViewCache& coinscache = chainstate->CoinsTip();
- bool is_background = chainstate != &chainman.ActiveChainstate();
+ bool is_background = chainstate.get() != &chainman.ActiveChainstate();
for (CTransactionRef& txn : m_coinbase_txns) {
COutPoint op{txn->GetHash(), 0};
@@ -365,15 +369,17 @@ struct SnapshotTestSetup : TestChain100Setup {
BOOST_TEST_MESSAGE("Simulating node restart");
{
- for (Chainstate* cs : chainman.GetAll()) {
- LOCK(::cs_main);
- cs->ForceFlushStateToDisk();
+ LOCK(chainman.GetMutex());
+ for (const auto& cs : chainman.m_chainstates) {
+ if (cs->CanFlushToDisk()) cs->ForceFlushStateToDisk();
}
+ }
+ {
// Process all callbacks referring to the old manager before wiping it.
m_node.validation_signals->SyncWithValidationInterfaceQueue();
LOCK(::cs_main);
chainman.ResetChainstates();
- BOOST_CHECK_EQUAL(chainman.GetAll().size(), 0);
+ BOOST_CHECK_EQUAL(chainman.m_chainstates.size(), 0);
m_node.notifications = std::make_unique<KernelNotifications>(Assert(m_node.shutdown_request), m_node.exit_status, *Assert(m_node.warnings));
const ChainstateManager::Options chainman_opts{
.chainparams = ::Params(),
@@ -438,17 +444,16 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup)
BOOST_CHECK_EQUAL(assumed_tip->nHeight, 120);
auto reload_all_block_indexes = [&]() {
+ LOCK(chainman.GetMutex());
// For completeness, we also reset the block sequence counters to
// ensure that no state which affects the ranking of tip-candidates is
// retained (even though this isn't strictly necessary).
- WITH_LOCK(::cs_main, return chainman.ResetBlockSequenceCounters());
- for (Chainstate* cs : chainman.GetAll()) {
- LOCK(::cs_main);
+ chainman.ResetBlockSequenceCounters();
+ for (const auto& cs : chainman.m_chainstates) {
cs->ClearBlockIndexCandidates();
BOOST_CHECK(cs->setBlockIndexCandidates.empty());
}
-
- WITH_LOCK(::cs_main, chainman.LoadBlockIndex());
+ chainman.LoadBlockIndex();
};
// Ensure that without any assumed-valid BlockIndex entries, only the current tip is
@@ -565,8 +570,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup)
const uint256 snapshot_tip_hash = WITH_LOCK(chainman.GetMutex(),
return chainman.ActiveTip()->GetBlockHash());
- auto all_chainstates = chainman.GetAll();
- BOOST_CHECK_EQUAL(all_chainstates.size(), 2);
+ BOOST_CHECK_EQUAL(WITH_LOCK(chainman.GetMutex(), return chainman.m_chainstates.size()), 2);
// "Rewind" the background chainstate so that its tip is not at the
// base block of the snapshot - this is so after simulating a node restart,
@@ -591,23 +595,22 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup)
// This call reinitializes the chainstates.
this->LoadVerifyActivateChainstate();
- std::vector<Chainstate*> chainstates;
{
LOCK(chainman_restarted.GetMutex());
- chainstates = chainman_restarted.GetAll();
- BOOST_CHECK_EQUAL(chainstates.size(), 2);
+ BOOST_CHECK_EQUAL(chainman_restarted.m_chainstates.size(), 2);
// Background chainstate has height of 109 not 110 here due to a quirk
// of the LoadVerifyActivate only calling ActivateBestChain on one
// chainstate. The height would be 110 after a real restart, but it's
// fine for this test which is focused on the snapshot chainstate.
- BOOST_CHECK_EQUAL(chainstates[0]->m_chain.Height(), 109);
- BOOST_CHECK_EQUAL(chainstates[1]->m_chain.Height(), 210);
+ BOOST_CHECK_EQUAL(chainman_restarted.m_chainstates[0]->m_chain.Height(), 109);
+ BOOST_CHECK_EQUAL(chainman_restarted.m_chainstates[1]->m_chain.Height(), 210);
BOOST_CHECK(chainman_restarted.CurrentChainstate().m_from_snapshot_blockhash);
BOOST_CHECK(chainman_restarted.CurrentChainstate().m_assumeutxo == Assumeutxo::UNVALIDATED);
BOOST_CHECK_EQUAL(chainman_restarted.ActiveTip()->GetBlockHash(), snapshot_tip_hash);
BOOST_CHECK_EQUAL(chainman_restarted.ActiveHeight(), 210);
+ BOOST_CHECK_EQUAL(chainman_restarted.HistoricalChainstate()->m_chain.Height(), 109);
}
BOOST_TEST_MESSAGE(
@@ -619,9 +622,10 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup)
// Background chainstate should be unaware of new blocks on the snapshot
// chainstate, but the block disconnected above is now reattached.
- BOOST_CHECK_EQUAL(chainstates[0]->m_chain.Height(), 110);
- BOOST_CHECK_EQUAL(chainstates[1]->m_chain.Height(), 220);
- BOOST_CHECK_EQUAL(chainman_restarted.GetAll().size(), 1);
+ BOOST_CHECK_EQUAL(chainman_restarted.m_chainstates.size(), 2);
+ BOOST_CHECK_EQUAL(chainman_restarted.m_chainstates[0]->m_chain.Height(), 110);
+ BOOST_CHECK_EQUAL(chainman_restarted.m_chainstates[1]->m_chain.Height(), 220);
+ BOOST_CHECK_EQUAL(chainman_restarted.HistoricalChainstate(), nullptr);
}
}
@@ -651,16 +655,13 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup
BOOST_CHECK(WITH_LOCK(::cs_main, return chainman.CurrentChainstate().m_assumeutxo == Assumeutxo::VALIDATED));
BOOST_CHECK(WITH_LOCK(::cs_main, return chainman.CurrentChainstate().m_from_snapshot_blockhash));
+ BOOST_CHECK_EQUAL(WITH_LOCK(chainman.GetMutex(), return chainman.HistoricalChainstate()), nullptr);
// Cache should have been rebalanced and reallocated to the "only" remaining
// chainstate.
BOOST_CHECK(active_cs.m_coinstip_cache_size_bytes > tip_cache_before_complete);
BOOST_CHECK(active_cs.m_coinsdb_cache_size_bytes > db_cache_before_complete);
- auto all_chainstates = chainman.GetAll();
- BOOST_CHECK_EQUAL(all_chainstates.size(), 1);
- BOOST_CHECK_EQUAL(all_chainstates[0], &active_cs);
-
// Trying completion again should return false.
res = WITH_LOCK(::cs_main, return chainman.MaybeValidateSnapshot(validated_cs, active_cs));
BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::SKIPPED);
@@ -690,7 +691,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup
{
LOCK(chainman_restarted.GetMutex());
- BOOST_CHECK_EQUAL(chainman_restarted.GetAll().size(), 1);
+ BOOST_CHECK_EQUAL(chainman_restarted.m_chainstates.size(), 1);
BOOST_CHECK(!chainman_restarted.CurrentChainstate().m_from_snapshot_blockhash);
BOOST_CHECK(active_cs2.m_coinstip_cache_size_bytes > tip_cache_before_complete);
BOOST_CHECK(active_cs2.m_coinsdb_cache_size_bytes > db_cache_before_complete);
@@ -737,10 +738,14 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion_hash_mismatch, Sna
BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::HASH_MISMATCH);
}
- auto all_chainstates = chainman.GetAll();
- BOOST_CHECK_EQUAL(all_chainstates.size(), 1);
- BOOST_CHECK_EQUAL(all_chainstates[0], &validation_chainstate);
- BOOST_CHECK_EQUAL(&chainman.ActiveChainstate(), &validation_chainstate);
+ {
+ LOCK(chainman.GetMutex());
+ BOOST_CHECK_EQUAL(chainman.m_chainstates.size(), 2);
+ BOOST_CHECK(chainman.m_chainstates[0]->m_assumeutxo == Assumeutxo::VALIDATED);
+ BOOST_CHECK(!chainman.m_chainstates[0]->SnapshotBase());
+ BOOST_CHECK(chainman.m_chainstates[1]->m_assumeutxo == Assumeutxo::INVALID);
+ BOOST_CHECK(chainman.m_chainstates[1]->SnapshotBase());
+ }
fs::path snapshot_invalid_dir = gArgs.GetDataDirNet() / "chainstate_snapshot_INVALID";
BOOST_CHECK(fs::exists(snapshot_invalid_dir));
@@ -761,7 +766,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion_hash_mismatch, Sna
{
LOCK(::cs_main);
- BOOST_CHECK_EQUAL(chainman_restarted.GetAll().size(), 1);
+ BOOST_CHECK_EQUAL(chainman_restarted.m_chainstates.size(), 1);
BOOST_CHECK(!chainman_restarted.CurrentChainstate().m_from_snapshot_blockhash);
BOOST_CHECK_EQUAL(chainman_restarted.ActiveHeight(), 210);
}
diff --git a/src/validation.cpp b/src/validation.cpp
index cc080d01..72872c5c 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -3811,6 +3811,15 @@ void Chainstate::ResetBlockFailureFlags(CBlockIndex *pindex) {
void Chainstate::TryAddBlockIndexCandidate(CBlockIndex* pindex)
{
AssertLockHeld(cs_main);
+
+ // Do not continue building a chainstate that is based on an invalid
+ // snapshot. This is a belt-and-suspenders type of check because if an
+ // invalid snapshot is loaded, the node will shut down to force a manual
+ // intervention. But it is good to handle this case correctly regardless.
+ if (m_assumeutxo == Assumeutxo::INVALID) {
+ return;
+ }
+
// The block only is a candidate for the most-work-chain if it has the same
// or more work than our current tip.
if (m_chain.Tip() != nullptr && setBlockIndexCandidates.value_comp()(pindex, m_chain.Tip())) {
@@ -3877,7 +3886,7 @@ void ChainstateManager::ReceivedBlockTransactions(const CBlock& block, CBlockInd
}
pindex->m_chain_tx_count = prev_tx_sum(*pindex);
pindex->nSequenceId = nBlockSequenceId++;
- for (Chainstate *c : GetAll()) {
+ for (const auto& c : m_chainstates) {
c->TryAddBlockIndexCandidate(pindex);
}
std::pair<std::multimap<CBlockIndex*, CBlockIndex*>::iterator, std::multimap<CBlockIndex*, CBlockIndex*>::iterator> range = m_blockman.m_blocks_unlinked.equal_range(pindex);
@@ -4980,7 +4989,7 @@ bool ChainstateManager::LoadBlockIndex()
(pindex->IsValid(BLOCK_VALID_TRANSACTIONS) &&
(pindex->HaveNumChainTxs() || pindex->pprev == nullptr))) {
- for (Chainstate* chainstate : GetAll()) {
+ for (const auto& chainstate : m_chainstates) {
chainstate->TryAddBlockIndexCandidate(pindex);
}
}
@@ -5595,18 +5604,6 @@ double ChainstateManager::GuessVerificationProgress(const CBlockIndex* pindex) c
return std::min<double>(pindex->m_chain_tx_count / fTxTotal, 1.0);
}
-std::vector<Chainstate*> ChainstateManager::GetAll()
-{
- LOCK(::cs_main);
- std::vector<Chainstate*> out;
-
- for (const auto& cs : m_chainstates) {
- if (cs && cs->m_assumeutxo != Assumeutxo::INVALID && !cs->m_target_utxohash) out.push_back(cs.get());
- }
-
- return out;
-}
-
Chainstate& ChainstateManager::InitializeChainstate(CTxMemPool* mempool)
{
AssertLockHeld(::cs_main);
@@ -6354,9 +6351,7 @@ bool ChainstateManager::ValidatedSnapshotCleanup(Chainstate& validated_cs, Chain
// The caller of this method will be responsible for reinitializing chainstates
// if they want to continue operation.
this->ResetChainstates();
-
- // No chainstates should be considered usable.
- assert(this->GetAll().size() == 0);
+ assert(this->m_chainstates.size() == 0);
LogInfo("[snapshot] deleting background chainstate directory (now unnecessary) (%s)",
fs::PathToString(validated_path));
@@ -6445,7 +6440,17 @@ util::Result<void> ChainstateManager::ActivateBestChains()
// the chainman unique_ptrs since ABC requires us not to be holding cs_main, so retrieve
// the relevant pointers before the ABC call.
AssertLockNotHeld(cs_main);
- for (Chainstate* chainstate : GetAll()) {
+ std::vector<Chainstate*> chainstates;
+ {
+ LOCK(GetMutex());
+ chainstates.reserve(m_chainstates.size());
+ for (const auto& chainstate : m_chainstates) {
+ if (chainstate && chainstate->m_assumeutxo != Assumeutxo::INVALID && !chainstate->m_target_utxohash) {
+ chainstates.push_back(chainstate.get());
+ }
+ }
+ }
+ for (Chainstate* chainstate : chainstates) {
BlockValidationState state;
if (!chainstate->ActivateBestChain(state, nullptr)) {
LOCK(GetMutex());
diff --git a/src/validation.h b/src/validation.h
index 82fbf04d..e14dac74 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -1081,9 +1081,6 @@ public:
// constructor
Chainstate& InitializeChainstate(CTxMemPool* mempool) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
- //! Get all chainstates currently being used.
- std::vector<Chainstate*> GetAll();
-
//! Construct and activate a Chainstate on the basis of UTXO snapshot data.
//!
//! Steps:
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.