refactor: Deduplicate Chainstate activation code
What changed, and why it matters
This commit is a straightforward internal code cleanup in Bitcoin Core. It merges two nearly identical blocks of code that set up a snapshot-based chainstate into a single helper method called AddChainstate. There is no change to network rules, consensus logic, wallet behavior, or user-facing functionality. It is purely a refactoring to reduce duplication.
No security action required. Review as normal code-quality refactoring.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change deduplicates initialization logic between ChainstateManager::ActivateSnapshot and ChainstateManager::ActivateExistingSnapshot by moving the common steps (assert no existing snapshot chainstate, assign m_snapshot_chainstate, set the IBD chainstate target blockhash, transfer mempool ownership, update m_active_chainstate) into a new private method AddChainstate. Call sites are updated to construct the Chainstate object and pass it to AddChainstate. The behavior is intended to be functionally equivalent; no security-sensitive logic is added or removed.
Changed components
src/validation.cppsrc/validation.hsrc/test/validation_chainstatemanager_tests.cppInspect captured patch +16 / −32
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index c6ae6088..03cec59c 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -69,7 +69,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager, TestChain100Setup)
// Create a snapshot-based chainstate.
//
const uint256 snapshot_blockhash = active_tip->GetBlockHash();
- Chainstate& c2 = WITH_LOCK(::cs_main, return manager.ActivateExistingSnapshot(snapshot_blockhash));
+ 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);
@@ -135,7 +135,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_rebalance_caches, TestChain100Setup)
// Create a snapshot-based chainstate.
//
CBlockIndex* snapshot_base{WITH_LOCK(manager.GetMutex(), return manager.ActiveChain()[manager.ActiveChain().Height() / 2])};
- Chainstate& c2 = WITH_LOCK(cs_main, return manager.ActivateExistingSnapshot(*snapshot_base->phashBlock));
+ Chainstate& c2{WITH_LOCK(::cs_main, return manager.AddChainstate(std::make_unique<Chainstate>(nullptr, manager.m_blockman, manager, *snapshot_base->phashBlock)))};
chainstates.push_back(&c2);
c2.InitCoinsDB(
/*cache_size_bytes=*/1 << 23, /*in_memory=*/true, /*should_wipe=*/false);
@@ -489,8 +489,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup)
}
// Note: cs2's tip is not set when ActivateExistingSnapshot is called.
- Chainstate& cs2 = WITH_LOCK(::cs_main,
- return chainman.ActivateExistingSnapshot(*assumed_base->phashBlock));
+ Chainstate& cs2{WITH_LOCK(::cs_main, return chainman.AddChainstate(std::make_unique<Chainstate>(nullptr, chainman.m_blockman, chainman, *assumed_base->phashBlock)))};
// Set tip of the fully validated chain to be the validated tip
cs1.m_chain.SetTip(*validated_tip);
diff --git a/src/validation.cpp b/src/validation.cpp
index 4c6432de..9e27da58 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -5796,28 +5796,14 @@ util::Result<CBlockIndex*> ChainstateManager::ActivateSnapshot(
}
}
- assert(!m_snapshot_chainstate);
- m_snapshot_chainstate.swap(snapshot_chainstate);
- const bool chaintip_loaded = m_snapshot_chainstate->LoadChainTip();
+ Chainstate& chainstate{AddChainstate(std::move(snapshot_chainstate))};
+ const bool chaintip_loaded{chainstate.LoadChainTip()};
assert(chaintip_loaded);
-
- // Set snapshot block as the target block for the historical chainstate.
- assert(m_ibd_chainstate.get());
- assert(!m_ibd_chainstate->m_target_blockhash);
- m_ibd_chainstate->SetTargetBlockHash(base_blockhash);
-
- // Transfer possession of the mempool to the snapshot chainstate.
- // Mempool is empty at this point because we're still in IBD.
- Assert(m_active_chainstate->m_mempool->size() == 0);
- Assert(!m_snapshot_chainstate->m_mempool);
- m_snapshot_chainstate->m_mempool = m_active_chainstate->m_mempool;
- m_active_chainstate->m_mempool = nullptr;
- m_active_chainstate = m_snapshot_chainstate.get();
- m_blockman.m_snapshot_height = this->GetSnapshotBaseHeight();
+ m_blockman.m_snapshot_height = Assert(chainstate.SnapshotBase())->nHeight;
LogInfo("[snapshot] successfully activated snapshot %s", base_blockhash.ToString());
LogInfo("[snapshot] (%.2f MB)",
- m_snapshot_chainstate->CoinsTip().DynamicMemoryUsage() / (1000 * 1000));
+ chainstate.CoinsTip().DynamicMemoryUsage() / (1000 * 1000));
this->MaybeRebalanceCaches();
return snapshot_start_block;
@@ -6288,21 +6274,21 @@ bool ChainstateManager::DetectSnapshotChainstate()
LogInfo("[snapshot] detected active snapshot chainstate (%s) - loading",
fs::PathToString(*path));
- this->ActivateExistingSnapshot(*base_blockhash);
+ auto snapshot_chainstate{std::make_unique<Chainstate>(nullptr, m_blockman, *this, base_blockhash)};
+ LogInfo("[snapshot] switching active chainstate to %s", snapshot_chainstate->ToString());
+ this->AddChainstate(std::move(snapshot_chainstate));
return true;
}
-Chainstate& ChainstateManager::ActivateExistingSnapshot(uint256 base_blockhash)
+Chainstate& ChainstateManager::AddChainstate(std::unique_ptr<Chainstate> chainstate)
{
assert(!m_snapshot_chainstate);
- m_snapshot_chainstate =
- std::make_unique<Chainstate>(nullptr, m_blockman, *this, base_blockhash);
- LogInfo("[snapshot] switching active chainstate to %s", m_snapshot_chainstate->ToString());
+ m_snapshot_chainstate = std::move(chainstate);
// Set target block for historical chainstate to snapshot block.
assert(m_ibd_chainstate.get());
assert(!m_ibd_chainstate->m_target_blockhash);
- m_ibd_chainstate->SetTargetBlockHash(base_blockhash);
+ m_ibd_chainstate->SetTargetBlockHash(*Assert(m_snapshot_chainstate->m_from_snapshot_blockhash));
// Transfer possession of the mempool to the chainstate.
// Mempool is empty at this point because we're still in IBD.
diff --git a/src/validation.h b/src/validation.h
index d2d9e419..5e6d11d1 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -1315,16 +1315,15 @@ public:
//! snapshot that is in the process of being validated.
bool DetectSnapshotChainstate() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+ //! Add new chainstate.
+ Chainstate& AddChainstate(std::unique_ptr<Chainstate> chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+
void ResetChainstates() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
//! Remove the snapshot-based chainstate and all on-disk artifacts.
//! Used when reindex{-chainstate} is called during snapshot use.
[[nodiscard]] bool DeleteSnapshotChainstate() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
- //! Switch the active chainstate to one based on a UTXO snapshot that was loaded
- //! previously.
- Chainstate& ActivateExistingSnapshot(uint256 base_blockhash) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
-
//! If we have validated a snapshot chain during this runtime, copy its
//! chainstate directory over to the main `chainstate` location, completing
//! validation of the snapshot.
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.