refactor: Delete ChainstateManager::IsSnapshotActive() method
What changed, and why it matters
This commit is a straightforward internal code cleanup. It removes a helper function called IsSnapshotActive() that was used in only one production spot and several tests, replacing it with a direct check of whether the current chainstate came from a snapshot. The change does not alter what the program does; it only makes the code clearer and fixes an outdated comment. There is no indication this affects user funds, network consensus, or introduces a security bug.
No security action required. Review as normal code-quality refactor if desired.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch deletes ChainstateManager::IsSnapshotActive() and updates its callers to check chainman.CurrentChainstate().m_from_snapshot_blockhash directly under the cs_main lock. The hasAssumedValidChain() interface method is updated accordingly, and its documentation is corrected to note the condition remains true after snapshot validation until restart. The behavior is preserved; this is a pure refactor with no functional change to snapshot activation, validation, or cache rebalancing logic.
Changed components
src/validation.cppsrc/validation.hsrc/node/interfaces.cppsrc/node/chainstate.cppsrc/interfaces/chain.hsrc/test/validation_chainstate_tests.cppsrc/test/validation_chainstatemanager_tests.cppInspect captured patch +15 / −24
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
index 17417218..686da67b 100644
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@@ -391,7 +391,9 @@ public:
//! removed transactions and already added new transactions.
virtual void requestMempoolTransactions(Notifications& notifications) = 0;
- //! Return true if an assumed-valid chain is in use.
+ //! Return true if an assumed-valid snapshot is in use. Note that this
+ //! returns true even after the snapshot is validated, until the next node
+ //! restart.
virtual bool hasAssumedValidChain() = 0;
//! Get internal node context. Useful for testing, but not
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
index bba518d1..a3010898 100644
--- a/src/node/chainstate.cpp
+++ b/src/node/chainstate.cpp
@@ -210,7 +210,6 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
// ChainstateManager::ResetChainstates(), reinitialize them here without
// duplicating the blockindex work above.
assert(chainman.GetAll().empty());
- assert(!chainman.IsSnapshotActive());
assert(!chainman.IsSnapshotValidated());
chainman.InitializeChainstate(options.mempool);
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index dfcf6629..5113f44f 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -845,7 +845,8 @@ public:
}
bool hasAssumedValidChain() override
{
- return chainman().IsSnapshotActive();
+ LOCK(::cs_main);
+ return bool{chainman().CurrentChainstate().m_from_snapshot_blockhash};
}
NodeContext* context() override { return &m_node; }
diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp
index b4aeb5b3..e823193a 100644
--- a/src/test/validation_chainstate_tests.cpp
+++ b/src/test/validation_chainstate_tests.cpp
@@ -95,7 +95,7 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup)
this, NoMalleation, /*reset_chainstate=*/ true));
// Ensure our active chain is the snapshot chainstate.
- BOOST_CHECK(WITH_LOCK(::cs_main, return chainman.IsSnapshotActive()));
+ BOOST_CHECK(WITH_LOCK(::cs_main, return chainman.CurrentChainstate().m_from_snapshot_blockhash));
curr_tip = get_notify_tip();
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index 036089c3..5d0bc907 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -49,7 +49,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager, TestChain100Setup)
Chainstate& c1 = manager.ActiveChainstate();
chainstates.push_back(&c1);
- BOOST_CHECK(!manager.IsSnapshotActive());
+ BOOST_CHECK(WITH_LOCK(::cs_main, return !manager.CurrentChainstate().m_from_snapshot_blockhash));
BOOST_CHECK(WITH_LOCK(::cs_main, return !manager.IsSnapshotValidated()));
auto all = manager.GetAll();
BOOST_CHECK_EQUAL_COLLECTIONS(all.begin(), all.end(), chainstates.begin(), chainstates.end());
@@ -84,7 +84,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager, TestChain100Setup)
BOOST_CHECK(c2.ActivateBestChain(_, nullptr));
BOOST_CHECK_EQUAL(manager.SnapshotBlockhash().value(), snapshot_blockhash);
- BOOST_CHECK(manager.IsSnapshotActive());
+ BOOST_CHECK(WITH_LOCK(::cs_main, return manager.CurrentChainstate().m_from_snapshot_blockhash));
BOOST_CHECK(WITH_LOCK(::cs_main, return !manager.IsSnapshotValidated()));
BOOST_CHECK_EQUAL(&c2, &manager.ActiveChainstate());
BOOST_CHECK(&c1 != &manager.ActiveChainstate());
@@ -182,10 +182,9 @@ struct SnapshotTestSetup : TestChain100Setup {
{
ChainstateManager& chainman = *Assert(m_node.chainman);
- BOOST_CHECK(!chainman.IsSnapshotActive());
-
{
LOCK(::cs_main);
+ BOOST_CHECK(!chainman.CurrentChainstate().m_from_snapshot_blockhash);
BOOST_CHECK(!chainman.IsSnapshotValidated());
BOOST_CHECK(!node::FindAssumeutxoChainstateDir(chainman.m_options.datadir));
}
@@ -569,7 +568,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup)
BOOST_CHECK(fs::exists(snapshot_chainstate_dir));
BOOST_CHECK_EQUAL(snapshot_chainstate_dir, gArgs.GetDataDirNet() / "chainstate_snapshot");
- BOOST_CHECK(chainman.IsSnapshotActive());
+ BOOST_CHECK(WITH_LOCK(::cs_main, return chainman.CurrentChainstate().m_from_snapshot_blockhash));
const uint256 snapshot_tip_hash = WITH_LOCK(chainman.GetMutex(),
return chainman.ActiveTip()->GetBlockHash());
@@ -611,7 +610,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup)
BOOST_CHECK_EQUAL(chainstates[0]->m_chain.Height(), 109);
BOOST_CHECK_EQUAL(chainstates[1]->m_chain.Height(), 210);
- BOOST_CHECK(chainman_restarted.IsSnapshotActive());
+ BOOST_CHECK(chainman_restarted.CurrentChainstate().m_from_snapshot_blockhash);
BOOST_CHECK(!chainman_restarted.IsSnapshotValidated());
BOOST_CHECK_EQUAL(chainman_restarted.ActiveTip()->GetBlockHash(), snapshot_tip_hash);
@@ -650,7 +649,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup
BOOST_CHECK(fs::exists(snapshot_chainstate_dir));
BOOST_CHECK_EQUAL(snapshot_chainstate_dir, gArgs.GetDataDirNet() / "chainstate_snapshot");
- BOOST_CHECK(chainman.IsSnapshotActive());
+ BOOST_CHECK(WITH_LOCK(::cs_main, return chainman.CurrentChainstate().m_from_snapshot_blockhash));
const uint256 snapshot_tip_hash = WITH_LOCK(chainman.GetMutex(),
return chainman.ActiveTip()->GetBlockHash());
@@ -658,7 +657,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup
BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::SUCCESS);
WITH_LOCK(::cs_main, BOOST_CHECK(chainman.IsSnapshotValidated()));
- BOOST_CHECK(chainman.IsSnapshotActive());
+ BOOST_CHECK(WITH_LOCK(::cs_main, return chainman.CurrentChainstate().m_from_snapshot_blockhash));
// Cache should have been rebalanced and reallocated to the "only" remaining
// chainstate.
@@ -699,7 +698,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup
{
LOCK(chainman_restarted.GetMutex());
BOOST_CHECK_EQUAL(chainman_restarted.GetAll().size(), 1);
- BOOST_CHECK(!chainman_restarted.IsSnapshotActive());
+ BOOST_CHECK(!chainman_restarted.CurrentChainstate().m_from_snapshot_blockhash);
BOOST_CHECK(!chainman_restarted.IsSnapshotValidated());
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);
@@ -771,7 +770,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion_hash_mismatch, Sna
{
LOCK(::cs_main);
BOOST_CHECK_EQUAL(chainman_restarted.GetAll().size(), 1);
- BOOST_CHECK(!chainman_restarted.IsSnapshotActive());
+ BOOST_CHECK(!chainman_restarted.CurrentChainstate().m_from_snapshot_blockhash);
BOOST_CHECK(!chainman_restarted.IsSnapshotValidated());
BOOST_CHECK_EQUAL(chainman_restarted.ActiveHeight(), 210);
}
diff --git a/src/validation.cpp b/src/validation.cpp
index c229851e..ff868911 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -6179,12 +6179,6 @@ Chainstate& ChainstateManager::ActiveChainstate() const
return *m_active_chainstate;
}
-bool ChainstateManager::IsSnapshotActive() const
-{
- LOCK(::cs_main);
- return m_snapshot_chainstate && m_active_chainstate == m_snapshot_chainstate.get();
-}
-
void ChainstateManager::MaybeRebalanceCaches()
{
AssertLockHeld(::cs_main);
diff --git a/src/validation.h b/src/validation.h
index 0f7fa2cb..30d3c2ae 100644
--- a/src/validation.h
+++ b/src/validation.h
@@ -1189,10 +1189,6 @@ public:
*/
mutable VersionBitsCache m_versionbitscache;
- //! @returns true if a snapshot-based chainstate is in use. Also implies
- //! that a background validation chainstate is also in use.
- bool IsSnapshotActive() const;
-
std::optional<uint256> SnapshotBlockhash() const;
//! Is there a snapshot in use and has it been fully validated?
Why this scored 14/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.