refactor: Pass chainstate parameters to MaybeCompleteSnapshotValidation

Remove hardcoded references to m_ibd_chainstate and m_snapshot_chainstate so
MaybeCompleteSnapshotValidation function can be simpler and focus on validating
the snapshot without dealing with internal ChainstateManager states.

This is a step towards being able to validate the snapshot outside of
ActivateBestChain loop so cs_main is not locked for minutes when the snapshot
block is connected.

doc/design/assumeutxo.md

@@ -63,7 +63,7 @@ chainstate and a sync to tip begins. A new chainstate directory is created in th
 datadir for the snapshot chainstate called `chainstate_snapshot`.
 
 When this directory is present in the datadir, the snapshot chainstate will be detected
-and loaded as active on node startup (via `DetectSnapshotChainstate()`).
+and loaded as active on node startup (via `LoadAssumeutxoChainstate()`).
 
 A special file is created within that directory, `base_blockhash`, which contains the
 serialized `uint256` of the base block of the snapshot. This is used to reinitialize

src/node/chainstate.cpp

@@ -169,15 +169,16 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
     Chainstate& validated_cs{chainman.InitializeChainstate(options.mempool)};
 
     // Load a chain created from a UTXO snapshot, if any exist.
-    bool has_snapshot = chainman.DetectSnapshotChainstate();
+    Chainstate* assumeutxo_cs{chainman.LoadAssumeutxoChainstate()};
 
-    if (has_snapshot && options.wipe_chainstate_db) {
+    if (assumeutxo_cs && options.wipe_chainstate_db) {
         // Reset chainstate target to network tip instead of snapshot block.
         validated_cs.SetTargetBlock(nullptr);
         LogInfo("[snapshot] deleting snapshot chainstate due to reindexing");
         if (!chainman.DeleteSnapshotChainstate()) {
             return {ChainstateLoadStatus::FAILURE_FATAL, Untranslated("Couldn't remove snapshot chainstate.")};
         }
+        assumeutxo_cs = nullptr;
     }
 
     auto [init_status, init_error] = CompleteChainstateInitialization(chainman, options);
@@ -193,7 +194,9 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
     // snapshot is actually validated? Because this entails unusual
     // filesystem operations to move leveldb data directories around, and that seems
     // too risky to do in the middle of normal runtime.
-    auto snapshot_completion = chainman.MaybeCompleteSnapshotValidation();
+    auto snapshot_completion{assumeutxo_cs
+                             ? chainman.MaybeValidateSnapshot(validated_cs, *assumeutxo_cs)
+                             : SnapshotCompletionResult::SKIPPED};
 
     if (snapshot_completion == SnapshotCompletionResult::SKIPPED) {
         // do nothing; expected case

src/node/utxo_snapshot.cpp

@@ -81,7 +81,7 @@ std::optional<uint256> ReadSnapshotBaseBlockhash(fs::path chaindir)
     return base_blockhash;
 }
 
-std::optional<fs::path> FindSnapshotChainstateDir(const fs::path& data_dir)
+std::optional<fs::path> FindAssumeutxoChainstateDir(const fs::path& data_dir)
 {
     fs::path possible_dir =
         data_dir / fs::u8path(strprintf("chainstate%s", SNAPSHOT_CHAINSTATE_SUFFIX));

src/node/utxo_snapshot.h

@@ -125,7 +125,7 @@ constexpr std::string_view SNAPSHOT_CHAINSTATE_SUFFIX = "_snapshot";
 
 
 //! Return a path to the snapshot-based chainstate dir, if one exists.
-std::optional<fs::path> FindSnapshotChainstateDir(const fs::path& data_dir);
+std::optional<fs::path> FindAssumeutxoChainstateDir(const fs::path& data_dir);
 
 } // namespace node
 

src/test/fuzz/utxo_snapshot.cpp

@@ -191,7 +191,7 @@ void utxo_snapshot_fuzz(FuzzBufferType buffer)
             const auto* index{chainman.m_blockman.LookupBlockIndex(block->GetHash())};
             Assert(index);
             Assert(index->nTx == 0);
-            if (index->nHeight == chainman.GetSnapshotBaseHeight()) {
+            if (index->nHeight == chainman.ActiveChainstate().SnapshotBase()->nHeight) {
                 auto params{chainman.GetParams().AssumeutxoForHeight(index->nHeight)};
                 Assert(params.has_value());
                 Assert(params.value().m_chain_tx_count == index->m_chain_tx_count);

src/test/validation_chainstatemanager_tests.cpp

@@ -187,7 +187,7 @@ struct SnapshotTestSetup : TestChain100Setup {
         {
             LOCK(::cs_main);
             BOOST_CHECK(!chainman.IsSnapshotValidated());
-            BOOST_CHECK(!node::FindSnapshotChainstateDir(chainman.m_options.datadir));
+            BOOST_CHECK(!node::FindAssumeutxoChainstateDir(chainman.m_options.datadir));
         }
 
         size_t initial_size;
@@ -240,7 +240,7 @@ struct SnapshotTestSetup : TestChain100Setup {
                 auto_infile >> coin;
         }));
 
-        BOOST_CHECK(!node::FindSnapshotChainstateDir(chainman.m_options.datadir));
+        BOOST_CHECK(!node::FindAssumeutxoChainstateDir(chainman.m_options.datadir));
 
         BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot(
             this, [](AutoFile& auto_infile, SnapshotMetadata& metadata) {
@@ -264,7 +264,7 @@ struct SnapshotTestSetup : TestChain100Setup {
         }));
 
         BOOST_REQUIRE(CreateAndActivateUTXOSnapshot(this));
-        BOOST_CHECK(fs::exists(*node::FindSnapshotChainstateDir(chainman.m_options.datadir)));
+        BOOST_CHECK(fs::exists(*node::FindAssumeutxoChainstateDir(chainman.m_options.datadir)));
 
         // Ensure our active chain is the snapshot chainstate.
         BOOST_CHECK(!chainman.ActiveChainstate().m_from_snapshot_blockhash->IsNull());
@@ -277,7 +277,7 @@ struct SnapshotTestSetup : TestChain100Setup {
         {
             LOCK(::cs_main);
 
-            fs::path found = *node::FindSnapshotChainstateDir(chainman.m_options.datadir);
+            fs::path found = *node::FindAssumeutxoChainstateDir(chainman.m_options.datadir);
 
             // Note: WriteSnapshotBaseBlockhash() is implicitly tested above.
             BOOST_CHECK_EQUAL(
@@ -565,7 +565,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup)
 
     this->SetupSnapshot();
 
-    fs::path snapshot_chainstate_dir = *node::FindSnapshotChainstateDir(chainman.m_options.datadir);
+    fs::path snapshot_chainstate_dir = *node::FindAssumeutxoChainstateDir(chainman.m_options.datadir);
     BOOST_CHECK(fs::exists(snapshot_chainstate_dir));
     BOOST_CHECK_EQUAL(snapshot_chainstate_dir, gArgs.GetDataDirNet() / "chainstate_snapshot");
 
@@ -639,13 +639,14 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup
 
     ChainstateManager& chainman = *Assert(m_node.chainman);
     Chainstate& active_cs = chainman.ActiveChainstate();
+    Chainstate& validated_cs{*Assert(WITH_LOCK(cs_main, return chainman.HistoricalChainstate()))};
     auto tip_cache_before_complete = active_cs.m_coinstip_cache_size_bytes;
     auto db_cache_before_complete = active_cs.m_coinsdb_cache_size_bytes;
 
     SnapshotCompletionResult res;
     m_node.notifications->m_shutdown_on_fatal_error = false;
 
-    fs::path snapshot_chainstate_dir = *node::FindSnapshotChainstateDir(chainman.m_options.datadir);
+    fs::path snapshot_chainstate_dir = *node::FindAssumeutxoChainstateDir(chainman.m_options.datadir);
     BOOST_CHECK(fs::exists(snapshot_chainstate_dir));
     BOOST_CHECK_EQUAL(snapshot_chainstate_dir, gArgs.GetDataDirNet() / "chainstate_snapshot");
 
@@ -653,7 +654,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup
     const uint256 snapshot_tip_hash = WITH_LOCK(chainman.GetMutex(),
         return chainman.ActiveTip()->GetBlockHash());
 
-    res = WITH_LOCK(::cs_main, return chainman.MaybeCompleteSnapshotValidation());
+    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()));
@@ -669,7 +670,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup
     BOOST_CHECK_EQUAL(all_chainstates[0], &active_cs);
 
     // Trying completion again should return false.
-    res = WITH_LOCK(::cs_main, return chainman.MaybeCompleteSnapshotValidation());
+    res = WITH_LOCK(::cs_main, return chainman.MaybeValidateSnapshot(validated_cs, active_cs));
     BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::SKIPPED);
 
     // The invalid snapshot path should not have been used.
@@ -720,6 +721,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion_hash_mismatch, Sna
 {
     auto chainstates = this->SetupSnapshot();
     Chainstate& validation_chainstate = *std::get<0>(chainstates);
+    Chainstate& unvalidated_cs = *std::get<1>(chainstates);
     ChainstateManager& chainman = *Assert(m_node.chainman);
     SnapshotCompletionResult res;
     m_node.notifications->m_shutdown_on_fatal_error = false;
@@ -740,7 +742,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion_hash_mismatch, Sna
 
     {
         ASSERT_DEBUG_LOG("failed to validate the -assumeutxo snapshot state");
-        res = WITH_LOCK(::cs_main, return chainman.MaybeCompleteSnapshotValidation());
+        res = WITH_LOCK(::cs_main, return chainman.MaybeValidateSnapshot(validation_chainstate, unvalidated_cs));
         BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::HASH_MISMATCH);
     }
 

src/validation.cpp

@@ -3165,11 +3165,17 @@ bool Chainstate::ConnectTip(
              Ticks<SecondsDouble>(m_chainman.time_total),
              Ticks<MillisecondsDouble>(m_chainman.time_total) / m_chainman.num_blocks_total);
 
-    // If we are the background validation chainstate, check to see if we are done
+    // See if this chainstate has reached a target block and can be used to
-    // validating the snapshot (i.e. our tip has reached the snapshot's base block).
+    // validate an assumeutxo snapshot. If it can, hashing the UTXO database
-    if (this != &m_chainman.ActiveChainstate()) {
+    // will be slow, and cs_main could remain locked here for several minutes.
-        m_chainman.MaybeCompleteSnapshotValidation();
+    // If the snapshot is validated, the UTXO hash will be saved to
-    }
+    // this->m_target_utxohash, causing HistoricalChainstate() to return null
+    // and this chainstate to no longer be used. ActivateBestChain() will also
+    // stop connecting blocks to this chainstate because this->ReachedTarget()
+    // will be true and this->setBlockIndexCandidates will not have additional
+    // blocks.
+    Chainstate& current_cs{*Assert(m_chainman.m_active_chainstate)};
+    m_chainman.MaybeValidateSnapshot(*this, current_cs);
 
     connectTrace.BlockConnected(pindexNew, std::move(block_to_connect));
     return true;
@@ -3527,7 +3533,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr<
             // the target block.
             //
             // This cannot be done while holding cs_main (within
-            // MaybeCompleteSnapshotValidation) or a cs_main deadlock will occur.
+            // MaybeValidateSnapshot) or a cs_main deadlock will occur.
             if (m_chainman.snapshot_download_completed) {
                 m_chainman.snapshot_download_completed();
             }
@@ -5761,7 +5767,7 @@ util::Result<CBlockIndex*> ChainstateManager::ActivateSnapshot(
 
         // PopulateAndValidateSnapshot can return (in error) before the leveldb datadir
         // has been created, so only attempt removal if we got that far.
-        if (auto snapshot_datadir = node::FindSnapshotChainstateDir(m_options.datadir)) {
+        if (auto snapshot_datadir = node::FindAssumeutxoChainstateDir(m_options.datadir)) {
             // We have to destruct leveldb::DB in order to release the db lock, otherwise
             // DestroyDB() (in DeleteCoinsDBFromDisk()) will fail. See `leveldb::~DBImpl()`.
             // Destructing the chainstate (and so resetting the coinsviews object) does this.
@@ -6037,45 +6043,35 @@ util::Result<void> ChainstateManager::PopulateAndValidateSnapshot(
 }
 
 // Currently, this function holds cs_main for its duration, which could be for
-// multiple minutes due to the ComputeUTXOStats call. This hold is necessary
+// multiple minutes due to the ComputeUTXOStats call. Holding cs_main used to be
-// because we need to avoid advancing the background validation chainstate
+// necessary (before d43a1f1a2fa3) to avoid advancing validated_cs farther than
-// farther than the snapshot base block - and this function is also invoked
+// its target block. Now it should be possible to avoid this, but simply
-// from within ConnectTip, i.e. from within ActivateBestChain, so cs_main is
+// releasing cs_main here would not be possible because this function is invoked
-// held anyway.
+// by ConnectTip within ActivateBestChain.
 //
-// Eventually (TODO), we could somehow separate this function's runtime from
+// Eventually (TODO) it would be better to call this function outside of
-// maintenance of the active chain, but that will either require
+// ActivateBestChain, on a separate thread that should not require cs_main to
-//
+// hash, because the UTXO set is only hashed after the historical chainstate
-//  (i) setting `m_disabled` immediately and ensuring all chainstate accesses go
+// reaches its target block and is no longer changing.
-//      through IsUsable() checks, or
+SnapshotCompletionResult ChainstateManager::MaybeValidateSnapshot(Chainstate& validated_cs, Chainstate& unvalidated_cs)
-//
-//  (ii) giving each chainstate its own lock instead of using cs_main for everything.
-SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation()
 {
     AssertLockHeld(cs_main);
 
-    // If a snapshot does not need to be validated...
+    // If the snapshot does not need to be validated...
-    if (m_ibd_chainstate.get() == &this->ActiveChainstate() ||
+    if (unvalidated_cs.m_assumeutxo != Assumeutxo::UNVALIDATED ||
             // Or if either chainstate is unusable...
-            !m_snapshot_chainstate ||
+            !unvalidated_cs.m_from_snapshot_blockhash ||
-            m_snapshot_chainstate->m_assumeutxo != Assumeutxo::UNVALIDATED ||
+            validated_cs.m_assumeutxo != Assumeutxo::VALIDATED ||
-            !m_snapshot_chainstate->m_from_snapshot_blockhash ||
+            !validated_cs.m_chain.Tip() ||
-            !m_ibd_chainstate ||
-            m_ibd_chainstate->m_assumeutxo != Assumeutxo::VALIDATED ||
-            !m_ibd_chainstate->m_chain.Tip() ||
             // Or the validated chainstate is not targeting the snapshot block...
-            !m_ibd_chainstate->m_target_blockhash ||
+            !validated_cs.m_target_blockhash ||
-            *m_ibd_chainstate->m_target_blockhash != *m_snapshot_chainstate->m_from_snapshot_blockhash ||
+            *validated_cs.m_target_blockhash != *unvalidated_cs.m_from_snapshot_blockhash ||
             // Or the validated chainstate has not reached the snapshot block yet...
-            !m_ibd_chainstate->ReachedTarget()) {
+            !validated_cs.ReachedTarget()) {
-       // Then a snapshot cannot be validated and there is nothing to do.
+       // Then the snapshot cannot be validated and there is nothing to do.
        return SnapshotCompletionResult::SKIPPED;
     }
-    const int snapshot_tip_height = this->ActiveHeight();
+    assert(validated_cs.TargetBlock() == validated_cs.m_chain.Tip());
-    const int snapshot_base_height = *Assert(this->GetSnapshotBaseHeight());
-    const CBlockIndex& index_new = *Assert(m_ibd_chainstate->m_chain.Tip());
-    assert(SnapshotBlockhash());
-    uint256 snapshot_blockhash = *Assert(SnapshotBlockhash());
 
     auto handle_invalid_snapshot = [&]() EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
         bilingual_str user_error = strprintf(_(
@@ -6090,19 +6086,20 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation()
             "Please report this incident to %s, including how you obtained the snapshot. "
             "The invalid snapshot chainstate will be left on disk in case it is "
             "helpful in diagnosing the issue that caused this error."),
-            CLIENT_NAME, snapshot_tip_height, snapshot_base_height, snapshot_base_height, CLIENT_BUGREPORT
+            CLIENT_NAME, unvalidated_cs.m_chain.Height(),
-        );
+            validated_cs.m_chain.Height(),
+            validated_cs.m_chain.Height(), CLIENT_BUGREPORT);
 
         LogError("[snapshot] !!! %s\n", user_error.original);
         LogError("[snapshot] deleting snapshot, reverting to validated chain, and stopping node\n");
 
         // Reset chainstate target to network tip instead of snapshot block.
-        m_ibd_chainstate->SetTargetBlock(nullptr);
+        validated_cs.SetTargetBlock(nullptr);
 
-        m_active_chainstate = m_ibd_chainstate.get();
+        m_active_chainstate = &validated_cs;
-        m_snapshot_chainstate->m_assumeutxo = Assumeutxo::INVALID;
+        unvalidated_cs.m_assumeutxo = Assumeutxo::INVALID;
 
-        auto rename_result = m_snapshot_chainstate->InvalidateCoinsDBOnDisk();
+        auto rename_result = unvalidated_cs.InvalidateCoinsDBOnDisk();
         if (!rename_result) {
             user_error += Untranslated("\n") + util::ErrorString(rename_result);
         }
@@ -6110,34 +6107,25 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation()
         GetNotifications().fatalError(user_error);
     };
 
-    assert(index_new.GetBlockHash() == snapshot_blockhash);
+    CCoinsViewDB& validated_coins_db = validated_cs.CoinsDB();
-    assert(index_new.nHeight == snapshot_base_height);
+    validated_cs.ForceFlushStateToDisk();
 
-    int curr_height = m_ibd_chainstate->m_chain.Height();
+    const auto& maybe_au_data = m_options.chainparams.AssumeutxoForHeight(validated_cs.m_chain.Height());
-
-    assert(snapshot_base_height == curr_height);
-    assert(snapshot_base_height == index_new.nHeight);
-    assert(this->GetAll().size() == 2);
-
-    CCoinsViewDB& ibd_coins_db = m_ibd_chainstate->CoinsDB();
-    m_ibd_chainstate->ForceFlushStateToDisk();
-
-    const auto& maybe_au_data = m_options.chainparams.AssumeutxoForHeight(curr_height);
     if (!maybe_au_data) {
         LogWarning("[snapshot] assumeutxo data not found for height "
-            "(%d) - refusing to validate snapshot", curr_height);
+            "(%d) - refusing to validate snapshot", validated_cs.m_chain.Height());
         handle_invalid_snapshot();
         return SnapshotCompletionResult::MISSING_CHAINPARAMS;
     }
 
     const AssumeutxoData& au_data = *maybe_au_data;
-    std::optional<CCoinsStats> maybe_ibd_stats;
+    std::optional<CCoinsStats> validated_cs_stats;
     LogInfo("[snapshot] computing UTXO stats for background chainstate to validate "
         "snapshot - this could take a few minutes");
     try {
-        maybe_ibd_stats = ComputeUTXOStats(
+        validated_cs_stats = ComputeUTXOStats(
             CoinStatsHashType::HASH_SERIALIZED,
-            &ibd_coins_db,
+            &validated_coins_db,
             m_blockman,
             [&interrupt = m_interrupt] { SnapshotUTXOHashBreakpoint(interrupt); });
     } catch (StopHashingException const&) {
@@ -6145,7 +6133,7 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation()
     }
 
     // XXX note that this function is slow and will hold cs_main for potentially minutes.
-    if (!maybe_ibd_stats) {
+    if (!validated_cs_stats) {
         LogWarning("[snapshot] failed to generate stats for validation coins db");
         // While this isn't a problem with the snapshot per se, this condition
         // prevents us from validating the snapshot, so we should shut down and let the
@@ -6153,7 +6141,6 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation()
         handle_invalid_snapshot();
         return SnapshotCompletionResult::STATS_FAILED;
     }
-    const auto& ibd_stats = *maybe_ibd_stats;
 
     // Compare the background validation chainstate's UTXO set hash against the hard-coded
     // assumeutxo hash we expect.
@@ -6161,19 +6148,19 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation()
     // TODO: For belt-and-suspenders, we could cache the UTXO set
     // hash for the snapshot when it's loaded in its chainstate's leveldb. We could then
     // reference that here for an additional check.
-    if (AssumeutxoHash{ibd_stats.hashSerialized} != au_data.hash_serialized) {
+    if (AssumeutxoHash{validated_cs_stats->hashSerialized} != au_data.hash_serialized) {
         LogWarning("[snapshot] hash mismatch: actual=%s, expected=%s",
-            ibd_stats.hashSerialized.ToString(),
+            validated_cs_stats->hashSerialized.ToString(),
             au_data.hash_serialized.ToString());
         handle_invalid_snapshot();
         return SnapshotCompletionResult::HASH_MISMATCH;
     }
 
     LogInfo("[snapshot] snapshot beginning at %s has been fully validated",
-        snapshot_blockhash.ToString());
+        unvalidated_cs.m_from_snapshot_blockhash->ToString());
 
-    m_snapshot_chainstate->m_assumeutxo = Assumeutxo::VALIDATED;
+    unvalidated_cs.m_assumeutxo = Assumeutxo::VALIDATED;
-    m_ibd_chainstate->m_target_utxohash = AssumeutxoHash{ibd_stats.hashSerialized};
+    validated_cs.m_target_utxohash = AssumeutxoHash{validated_cs_stats->hashSerialized};
     this->MaybeRebalanceCaches();
 
     return SnapshotCompletionResult::SUCCESS;
@@ -6260,24 +6247,23 @@ ChainstateManager::~ChainstateManager()
     m_versionbitscache.Clear();
 }
 
-bool ChainstateManager::DetectSnapshotChainstate()
+Chainstate* ChainstateManager::LoadAssumeutxoChainstate()
 {
     assert(!m_snapshot_chainstate);
-    std::optional<fs::path> path = node::FindSnapshotChainstateDir(m_options.datadir);
+    std::optional<fs::path> path = node::FindAssumeutxoChainstateDir(m_options.datadir);
     if (!path) {
-        return false;
+        return nullptr;
     }
     std::optional<uint256> base_blockhash = node::ReadSnapshotBaseBlockhash(*path);
     if (!base_blockhash) {
-        return false;
+        return nullptr;
     }
     LogInfo("[snapshot] detected active snapshot chainstate (%s) - loading",
         fs::PathToString(*path));
 
     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 &this->AddChainstate(std::move(snapshot_chainstate));
-    return true;
 }
 
 Chainstate& ChainstateManager::AddChainstate(std::unique_ptr<Chainstate> chainstate)
@@ -6337,7 +6323,7 @@ util::Result<void> Chainstate::InvalidateCoinsDBOnDisk()
 
     // The invalid snapshot datadir is simply moved and not deleted because we may
     // want to do forensics later during issue investigation. The user is instructed
-    // accordingly in MaybeCompleteSnapshotValidation().
+    // accordingly in MaybeValidateSnapshot().
     try {
         fs::rename(snapshot_datadir, invalid_path);
     } catch (const fs::filesystem_error& e) {
@@ -6362,7 +6348,7 @@ bool ChainstateManager::DeleteSnapshotChainstate()
     Assert(m_snapshot_chainstate);
     Assert(m_ibd_chainstate);
 
-    fs::path snapshot_datadir = Assert(node::FindSnapshotChainstateDir(m_options.datadir)).value();
+    fs::path snapshot_datadir = Assert(node::FindAssumeutxoChainstateDir(m_options.datadir)).value();
     if (!DeleteCoinsDBFromDisk(snapshot_datadir, /*is_snapshot=*/ true)) {
         LogError("Deletion of %s failed. Please remove it manually to continue reindexing.",
                   fs::PathToString(snapshot_datadir));
@@ -6384,12 +6370,6 @@ const CBlockIndex* ChainstateManager::GetSnapshotBaseBlock() const
     return m_active_chainstate ? m_active_chainstate->SnapshotBase() : nullptr;
 }
 
-std::optional<int> ChainstateManager::GetSnapshotBaseHeight() const
-{
-    const CBlockIndex* base = this->GetSnapshotBaseBlock();
-    return base ? std::make_optional(base->nHeight) : std::nullopt;
-}
-
 void ChainstateManager::RecalculateBestHeader()
 {
     AssertLockHeld(cs_main);

src/validation.h

@@ -1131,14 +1131,15 @@ public:
     [[nodiscard]] util::Result<CBlockIndex*> ActivateSnapshot(
         AutoFile& coins_file, const node::SnapshotMetadata& metadata, bool in_memory);
 
-    //! Once the background validation chainstate has reached the height which
+    //! Try to validate an assumeutxo snapshot by using a validated historical
-    //! is the base of the UTXO snapshot in use, compare its coins to ensure
+    //! chainstate targeted at the snapshot block. When the target block is
-    //! they match those expected by the snapshot.
+    //! reached, the UTXO hash is computed and saved to
-    //!
+    //! `validated_cs.m_target_utxohash`, and `unvalidated_cs.m_assumeutxo` will
-    //! If the coins match (expected), then mark the validation chainstate for
+    //! be updated from UNVALIDATED to either VALIDATED or INVALID depending on
-    //! deletion and continue using the snapshot chainstate as active.
+    //! whether the hash matches. The INVALID case should not happen in practice
-    //! Otherwise, revert to using the ibd chainstate and shutdown.
+    //! because the software should refuse to load unrecognized snapshots, but
-    SnapshotCompletionResult MaybeCompleteSnapshotValidation() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+    //! if it does happen, it is a fatal error.
+    SnapshotCompletionResult MaybeValidateSnapshot(Chainstate& validated_cs, Chainstate& unvalidated_cs) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
     //! Returns nullptr if no snapshot has been loaded.
     const CBlockIndex* GetSnapshotBaseBlock() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
@@ -1312,8 +1313,9 @@ public:
     void ReportHeadersPresync(const arith_uint256& work, int64_t height, int64_t timestamp);
 
     //! When starting up, search the datadir for a chainstate based on a UTXO
-    //! snapshot that is in the process of being validated.
+    //! snapshot that is in the process of being validated and load it if found.
-    bool DetectSnapshotChainstate() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+    //! Return pointer to the Chainstate if it is loaded.
+    Chainstate* LoadAssumeutxoChainstate() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
     //! Add new chainstate.
     Chainstate& AddChainstate(std::unique_ptr<Chainstate> chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
@@ -1351,10 +1353,6 @@ public:
     std::pair<int, int> GetPruneRange(
         const Chainstate& chainstate, int last_height_can_prune) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
-    //! Return the height of the base block of the snapshot in use, if one exists, else
-    //! nullopt.
-    std::optional<int> GetSnapshotBaseHeight() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
-
     //! Get range of historical blocks to download.
     std::optional<std::pair<const CBlockIndex*, const CBlockIndex*>> GetHistoricalBlockRange() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);