refactor: Delete ChainstateManager::IsSnapshotValidated() method

ryanofsky · ryanofsky · commit ee35250683ab · 2025-12-12T06:49:59.000-04:00
IsSnapshotValidated() is only called one place outside of tests, and is use
redundantly in some tests, asserting that a snapshot is not validated when a
snapshot chainstate does not even exist. Simplify by dropping the method and
checking Chainstate m_assumeutxo field directly.
diff --git 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.IsSnapshotValidated());
 
         chainman.InitializeChainstate(options.mempool);
 
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
@@ -50,7 +50,6 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager, TestChain100Setup)
     chainstates.push_back(&c1);
 
     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());
 
@@ -85,7 +84,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager, TestChain100Setup)
 
     BOOST_CHECK_EQUAL(manager.SnapshotBlockhash().value(), snapshot_blockhash);
     BOOST_CHECK(WITH_LOCK(::cs_main, return manager.CurrentChainstate().m_from_snapshot_blockhash));
-    BOOST_CHECK(WITH_LOCK(::cs_main, return !manager.IsSnapshotValidated()));
+    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();
@@ -185,7 +184,6 @@ struct SnapshotTestSetup : TestChain100Setup {
         {
             LOCK(::cs_main);
             BOOST_CHECK(!chainman.CurrentChainstate().m_from_snapshot_blockhash);
-            BOOST_CHECK(!chainman.IsSnapshotValidated());
             BOOST_CHECK(!node::FindAssumeutxoChainstateDir(chainman.m_options.datadir));
         }
 
@@ -611,7 +609,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup)
         BOOST_CHECK_EQUAL(chainstates[1]->m_chain.Height(), 210);
 
         BOOST_CHECK(chainman_restarted.CurrentChainstate().m_from_snapshot_blockhash);
-        BOOST_CHECK(!chainman_restarted.IsSnapshotValidated());
+        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);
@@ -656,7 +654,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup
     res = WITH_LOCK(::cs_main, return chainman.MaybeValidateSnapshot(validated_cs, active_cs));
     BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::SUCCESS);
 
-    WITH_LOCK(::cs_main, BOOST_CHECK(chainman.IsSnapshotValidated()));
+    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));
 
     // Cache should have been rebalanced and reallocated to the "only" remaining
@@ -699,7 +697,6 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup
         LOCK(chainman_restarted.GetMutex());
         BOOST_CHECK_EQUAL(chainman_restarted.GetAll().size(), 1);
         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 +768,6 @@ 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.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
@@ -6367,7 +6367,7 @@ void ChainstateManager::RecalculateBestHeader()
 bool ChainstateManager::ValidatedSnapshotCleanup(Chainstate& validated_cs, Chainstate& unvalidated_cs)
 {
     AssertLockHeld(::cs_main);
-    if (!this->IsSnapshotValidated()) {
+    if (unvalidated_cs.m_assumeutxo != Assumeutxo::VALIDATED) {
         // No need to clean up.
         return false;
     }
diff --git a/src/validation.h b/src/validation.h
@@ -1191,12 +1191,6 @@ class ChainstateManager
 
     std::optional<uint256> SnapshotBlockhash() const;
 
-    //! Is there a snapshot in use and has it been fully validated?
-    bool IsSnapshotValidated() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
-    {
-        return m_snapshot_chainstate && m_snapshot_chainstate->m_assumeutxo == Assumeutxo::VALIDATED;
-    }
-
     /** Check whether we are doing an initial block download (synchronizing from disk or network) */
     bool IsInitialBlockDownload() const;
 

Original file line number	Diff line number	Diff line change
`@@ -6367,7 +6367,7 @@ void ChainstateManager::RecalculateBestHeader()`
`6367`	`6367`	`bool ChainstateManager::ValidatedSnapshotCleanup(Chainstate& validated_cs, Chainstate& unvalidated_cs)`
`6368`	`6368`	`{`
`6369`	`6369`	`AssertLockHeld(::cs_main);`
`6370`		`- if (!this->IsSnapshotValidated()) {`
	`6370`	`+ if (unvalidated_cs.m_assumeutxo != Assumeutxo::VALIDATED) {`
`6371`	`6371`	`// No need to clean up.`
`6372`	`6372`	`return false;`
`6373`	`6373`	`}`