refactor: Delete ChainstateManager::IsSnapshotActive() method

IsSnapshotActive() method is only called one place outside of tests and
asserts, and is confusing because it returns true even after the snapshot is
fully validated.

The documentation which said this "implies that a background validation
chainstate is also in use" is also incorrect, because after the snapshot is
validated, the background chainstate gets disabled and IsUsable() would return
false.

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

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);

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; }

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();
 

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);
     }

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);

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?