Merge bitcoin/bitcoin#30214: refactor: Improve assumeutxo state representation

82be652e40 doc: Improve ChainstateManager documentation, use consistent terms (Ryan Ofsky)
af455dcb39 refactor: Simplify pruning functions (TheCharlatan)
ae85c495f1 refactor: Delete ChainstateManager::GetAll() method (Ryan Ofsky)
6a572dbda9 refactor: Add ChainstateManager::ActivateBestChains() method (Ryan Ofsky)
491d827d52 refactor: Add ChainstateManager::m_chainstates member (Ryan Ofsky)
e514fe6116 refactor: Delete ChainstateManager::SnapshotBlockhash() method (Ryan Ofsky)
ee35250683 refactor: Delete ChainstateManager::IsSnapshotValidated() method (Ryan Ofsky)
d9e82299fc refactor: Delete ChainstateManager::IsSnapshotActive() method (Ryan Ofsky)
4dfe383912 refactor: Convert ChainstateRole enum to struct (Ryan Ofsky)
352ad27fc1 refactor: Add ChainstateManager::ValidatedChainstate() method (Ryan Ofsky)
a229cb9477 refactor: Add ChainstateManager::CurrentChainstate() method (Ryan Ofsky)
a9b7f5614c refactor: Add Chainstate::StoragePath() method (Ryan Ofsky)
840bd2ef23 refactor: Pass chainstate parameters to MaybeCompleteSnapshotValidation (Ryan Ofsky)
1598a15aed refactor: Deduplicate Chainstate activation code (Ryan Ofsky)
9fe927b6d6 refactor: Add Chainstate m_assumeutxo and m_target_utxohash members (Ryan Ofsky)
6082c84713 refactor: Add Chainstate::m_target_blockhash member (Ryan Ofsky)
de00e87548 test: Fix broken chainstatemanager_snapshot_init check (Ryan Ofsky)

Pull request description:

  This PR contains the first part of #28608, which tries to make assumeutxo code more maintainable, and improve it by not locking `cs_main` for a long time when the snapshot block is connected, and by deleting the snapshot validation chainstate when it is no longer used, instead of waiting until the next restart.

  The changes in this PR are just refactoring. They make `Chainstate` objects self-contained, so for example, it is possible to determine what blocks to connect to a chainstate without querying `ChainstateManager`, and to determine whether a Chainstate is validated without basing it on inferences like `&cs != &ActiveChainstate()` or `GetAll().size() == 1`.

  The PR also tries to make assumeutxo terminology less confusing, using "current chainstate" to refer to the chainstate targeting the current network tip, and "historical chainstate" to refer to the chainstate downloading old blocks and validating the assumeutxo snapshot. It removes uses of the terms "active chainstate," "usable chainstate," "disabled chainstate," "ibd chainstate," and "snapshot chainstate" which are confusing for various reasons.

ACKs for top commit:
  maflcko:
    re-review ACK 82be652e40 🕍
  fjahr:
    re-ACK 82be652e40
  sedited:
    Re-ACK 82be652e40

Tree-SHA512: 81c67abba9fc5bb170e32b7bf8a1e4f7b5592315b4ef720be916d5f1f5a7088c0c59cfb697744dd385552f58aa31ee36176bae6a6e465723e65861089a1252e5

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
@@ -97,14 +97,13 @@ sequentially.
 ### Background chainstate hits snapshot base block
 
 Once the tip of the background chainstate hits the base block of the snapshot
-chainstate, we stop use of the background chainstate by setting `m_disabled`, in
+chainstate, we hash the
-`MaybeCompleteSnapshotValidation()`, which is checked in `ActivateBestChain()`). We hash the
 background chainstate's UTXO set contents and ensure it matches the compiled value in
 `CMainParams::m_assumeutxo_data`.
 
 |    |    |
 | ---------- | ----------- |
-| number of chainstates | 2 (ibd has `m_disabled=true`) |
+| number of chainstates | 2 |
 | active chainstate | snapshot |
 
 The background chainstate data lingers on disk until the program is restarted.

src/bench/wallet_create_tx.cpp

@@ -11,6 +11,7 @@
 #include <consensus/merkle.h>
 #include <interfaces/chain.h>
 #include <kernel/chain.h>
+#include <kernel/types.h>
 #include <node/blockstorage.h>
 #include <outputtype.h>
 #include <policy/feerate.h>
@@ -39,6 +40,7 @@
 #include <utility>
 #include <vector>
 
+using kernel::ChainstateRole;
 using wallet::CWallet;
 using wallet::CreateMockableWalletDatabase;
 using wallet::WALLET_FLAG_DESCRIPTORS;
@@ -101,7 +103,7 @@ void generateFakeBlock(const CChainParams& params,
 
     // notify wallet
     const auto& pindex = WITH_LOCK(::cs_main, return context.chainman->ActiveChain().Tip());
-    wallet.blockConnected(ChainstateRole::NORMAL, kernel::MakeBlockInfo(pindex, &block));
+    wallet.blockConnected(ChainstateRole{}, kernel::MakeBlockInfo(pindex, &block));
 }
 
 struct PreSelectInputs {

src/bench/wallet_migration.cpp

@@ -6,6 +6,7 @@
 #include <interfaces/chain.h>
 #include <interfaces/wallet.h>
 #include <kernel/chain.h>
+#include <kernel/types.h>
 #include <node/context.h>
 #include <test/util/mining.h>
 #include <test/util/setup_common.h>

src/dbwrapper.cpp

@@ -214,7 +214,7 @@ struct LevelDBContext {
 };
 
 CDBWrapper::CDBWrapper(const DBParams& params)
-    : m_db_context{std::make_unique<LevelDBContext>()}, m_name{fs::PathToString(params.path.stem())}, m_path{params.path}, m_is_memory{params.memory_only}
+    : m_db_context{std::make_unique<LevelDBContext>()}, m_name{fs::PathToString(params.path.stem())}
 {
     DBContext().penv = nullptr;
     DBContext().readoptions.verify_checksums = true;

src/dbwrapper.h

@@ -191,12 +191,6 @@ private:
     //! obfuscation key storage key, null-prefixed to avoid collisions
     inline static const std::string OBFUSCATION_KEY{"\000obfuscate_key", 14}; // explicit size to avoid truncation at leading \0
 
-    //! path to filesystem storage
-    const fs::path m_path;
-
-    //! whether or not the database resides in memory
-    bool m_is_memory;
-
     std::optional<std::string> ReadImpl(std::span<const std::byte> key) const;
     bool ExistsImpl(std::span<const std::byte> key) const;
     size_t EstimateSizeImpl(std::span<const std::byte> key1, std::span<const std::byte> key2) const;
@@ -237,14 +231,6 @@ public:
         WriteBatch(batch, fSync);
     }
 
-    //! @returns filesystem path to the on-disk data.
-    std::optional<fs::path> StoragePath() {
-        if (m_is_memory) {
-            return {};
-        }
-        return m_path;
-    }
-
     template <typename K>
     bool Exists(const K& key) const
     {

src/index/base.cpp

@@ -10,6 +10,7 @@
 #include <interfaces/chain.h>
 #include <interfaces/types.h>
 #include <kernel/chain.h>
+#include <kernel/types.h>
 #include <logging.h>
 #include <node/abort.h>
 #include <node/blockstorage.h>
@@ -42,6 +43,8 @@
 #include <utility>
 #include <vector>
 
+using kernel::ChainstateRole;
+
 constexpr uint8_t DB_BEST_BLOCK{'B'};
 
 constexpr auto SYNC_LOG_INTERVAL{30s};
@@ -109,7 +112,7 @@ bool BaseIndex::Init()
     // m_chainstate member gives indexing code access to node internals. It is
     // removed in followup https://github.com/bitcoin/bitcoin/pull/24230
     m_chainstate = WITH_LOCK(::cs_main,
-        return &m_chain->context()->chainman->GetChainstateForIndexing());
+                             return &m_chain->context()->chainman->ValidatedChainstate());
     // Register to validation interface before setting the 'm_synced' flag, so that
     // callbacks are not missed once m_synced is true.
     m_chain->context()->validation_signals->RegisterValidationInterface(this);
@@ -323,15 +326,13 @@ bool BaseIndex::Rewind(const CBlockIndex* current_tip, const CBlockIndex* new_ti
     return true;
 }
 
-void BaseIndex::BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex)
+void BaseIndex::BlockConnected(const ChainstateRole& role, const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex)
 {
-    // Ignore events from the assumed-valid chain; we will process its blocks
+    // Ignore events from not fully validated chains to avoid out-of-order indexing.
-    // (sequentially) after it is fully verified by the background chainstate. This
-    // is to avoid any out-of-order indexing.
     //
     // TODO at some point we could parameterize whether a particular index can be
     // built out of order, but for now just do the conservative simple thing.
-    if (role == ChainstateRole::ASSUMEDVALID) {
+    if (!role.validated) {
         return;
     }
 
@@ -377,11 +378,10 @@ void BaseIndex::BlockConnected(ChainstateRole role, const std::shared_ptr<const
     }
 }
 
-void BaseIndex::ChainStateFlushed(ChainstateRole role, const CBlockLocator& locator)
+void BaseIndex::ChainStateFlushed(const ChainstateRole& role, const CBlockLocator& locator)
 {
-    // Ignore events from the assumed-valid chain; we will process its blocks
+    // Ignore events from not fully validated chains to avoid out-of-order indexing.
-    // (sequentially) after it is fully verified by the background chainstate.
+    if (!role.validated) {
-    if (role == ChainstateRole::ASSUMEDVALID) {
         return;
     }
 

src/index/base.h

@@ -118,9 +118,9 @@ protected:
     Chainstate* m_chainstate{nullptr};
     const std::string m_name;
 
-    void BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex) override;
+    void BlockConnected(const kernel::ChainstateRole& role, const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex) override;
 
-    void ChainStateFlushed(ChainstateRole role, const CBlockLocator& locator) override;
+    void ChainStateFlushed(const kernel::ChainstateRole& role, const CBlockLocator& locator) override;
 
     /// Return custom notification options for index.
     [[nodiscard]] virtual interfaces::Chain::NotifyOptions CustomOptions() { return {}; }

src/init.cpp

@@ -345,7 +345,7 @@ void Shutdown(NodeContext& node)
     // FlushStateToDisk generates a ChainStateFlushed callback, which we should avoid missing
     if (node.chainman) {
         LOCK(cs_main);
-        for (Chainstate* chainstate : node.chainman->GetAll()) {
+        for (const auto& chainstate : node.chainman->m_chainstates) {
             if (chainstate->CanFlushToDisk()) {
                 chainstate->ForceFlushStateToDisk();
             }
@@ -371,7 +371,7 @@ void Shutdown(NodeContext& node)
 
     if (node.chainman) {
         LOCK(cs_main);
-        for (Chainstate* chainstate : node.chainman->GetAll()) {
+        for (const auto& chainstate : node.chainman->m_chainstates) {
             if (chainstate->CanFlushToDisk()) {
                 chainstate->ForceFlushStateToDisk();
                 chainstate->ResetCoinsViews();
@@ -1877,14 +1877,14 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     if (chainman.m_blockman.IsPruneMode()) {
         if (chainman.m_blockman.m_blockfiles_indexed) {
             LOCK(cs_main);
-            for (Chainstate* chainstate : chainman.GetAll()) {
+            for (const auto& chainstate : chainman.m_chainstates) {
                 uiInterface.InitMessage(_("Pruning blockstore…"));
                 chainstate->PruneAndFlush();
             }
         }
     } else {
         // Prior to setting NODE_NETWORK, check if we can provide historical blocks.
-        if (!WITH_LOCK(chainman.GetMutex(), return chainman.BackgroundSyncInProgress())) {
+        if (!WITH_LOCK(chainman.GetMutex(), return chainman.HistoricalChainstate())) {
             LogInfo("Setting NODE_NETWORK in non-prune mode");
             g_local_services = ServiceFlags(g_local_services | NODE_NETWORK);
         } else {
@@ -2233,7 +2233,7 @@ bool StartIndexBackgroundSync(NodeContext& node)
     std::optional<const CBlockIndex*> indexes_start_block;
     std::string older_index_name;
     ChainstateManager& chainman = *Assert(node.chainman);
-    const Chainstate& chainstate = WITH_LOCK(::cs_main, return chainman.GetChainstateForIndexing());
+    const Chainstate& chainstate = WITH_LOCK(::cs_main, return chainman.ValidatedChainstate());
     const CChain& index_chain = chainstate.m_chain;
 
     for (auto index : node.indexes) {

src/interfaces/chain.h

@@ -30,10 +30,12 @@ class Coin;
 class uint256;
 enum class MemPoolRemovalReason;
 enum class RBFTransactionState;
-enum class ChainstateRole;
 struct bilingual_str;
 struct CBlockLocator;
 struct FeeCalculation;
+namespace kernel {
+struct ChainstateRole;
+} // namespace kernel
 namespace node {
 struct NodeContext;
 } // namespace node
@@ -321,10 +323,10 @@ public:
         virtual ~Notifications() = default;
         virtual void transactionAddedToMempool(const CTransactionRef& tx) {}
         virtual void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) {}
-        virtual void blockConnected(ChainstateRole role, const BlockInfo& block) {}
+        virtual void blockConnected(const kernel::ChainstateRole& role, const BlockInfo& block) {}
         virtual void blockDisconnected(const BlockInfo& block) {}
         virtual void updatedBlockTip() {}
-        virtual void chainStateFlushed(ChainstateRole role, const CBlockLocator& locator) {}
+        virtual void chainStateFlushed(const kernel::ChainstateRole& role, const CBlockLocator& locator) {}
     };
 
     //! Options specifying which chain notifications are required.
@@ -389,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/kernel/bitcoinkernel.cpp

@@ -51,6 +51,7 @@
 #include <utility>
 #include <vector>
 
+using kernel::ChainstateRole;
 using util::ImmediateTaskRunner;
 
 // Define G_TRANSLATION_FUN symbol in libbitcoinkernel library so users of the
@@ -359,7 +360,7 @@ protected:
         }
     }
 
-    void BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex) override
+    void BlockConnected(const ChainstateRole& role, const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex) override
     {
         if (m_cbs.block_connected) {
             m_cbs.block_connected(m_cbs.user_data,
@@ -981,13 +982,9 @@ btck_ChainstateManager* btck_chainstate_manager_create(
             LogError("Failed to verify loaded chain state from your datadir: %s", chainstate_err.original);
             return nullptr;
         }
-
+        if (auto result = chainman->ActivateBestChains(); !result) {
-        for (Chainstate* chainstate : WITH_LOCK(chainman->GetMutex(), return chainman->GetAll())) {
+            LogError("%s", util::ErrorString(result).original);
-            BlockValidationState state;
+            return nullptr;
-            if (!chainstate->ActivateBestChain(state, nullptr)) {
-                LogError("Failed to connect best block: %s", state.ToString());
-                return nullptr;
-            }
         }
     } catch (const std::exception& e) {
         LogError("Failed to load chainstate: %s", e.what());
@@ -1012,7 +1009,7 @@ void btck_chainstate_manager_destroy(btck_ChainstateManager* chainman)
 {
     {
         LOCK(btck_ChainstateManager::get(chainman).m_chainman->GetMutex());
-        for (Chainstate* chainstate : btck_ChainstateManager::get(chainman).m_chainman->GetAll()) {
+        for (const auto& chainstate : btck_ChainstateManager::get(chainman).m_chainman->m_chainstates) {
             if (chainstate->CanFlushToDisk()) {
                 chainstate->ForceFlushStateToDisk();
                 chainstate->ResetCoinsViews();

src/kernel/chain.cpp

@@ -5,11 +5,14 @@
 #include <chain.h>
 #include <interfaces/chain.h>
 #include <kernel/chain.h>
+#include <kernel/types.h>
 #include <sync.h>
 #include <uint256.h>
 
 class CBlock;
 
+using kernel::ChainstateRole;
+
 namespace kernel {
 interfaces::BlockInfo MakeBlockInfo(const CBlockIndex* index, const CBlock* data)
 {
@@ -25,14 +28,15 @@ interfaces::BlockInfo MakeBlockInfo(const CBlockIndex* index, const CBlock* data
     info.data = data;
     return info;
 }
-} // namespace kernel
 
 std::ostream& operator<<(std::ostream& os, const ChainstateRole& role) {
-    switch(role) {
+    if (!role.validated) {
-        case ChainstateRole::NORMAL: os << "normal"; break;
+        os << "assumedvalid";
-        case ChainstateRole::ASSUMEDVALID: os << "assumedvalid"; break;
+    } else if (role.historical) {
-        case ChainstateRole::BACKGROUND: os << "background"; break;
+        os << "background";
-        default: os.setstate(std::ios_base::failbit);
+    } else {
+        os << "normal";
     }
     return os;
 }
+} // namespace kernel

src/kernel/chain.h

@@ -14,26 +14,10 @@ struct BlockInfo;
 } // namespace interfaces
 
 namespace kernel {
+struct ChainstateRole;
 //! Return data from block index.
 interfaces::BlockInfo MakeBlockInfo(const CBlockIndex* block_index, const CBlock* data = nullptr);
-
+std::ostream& operator<<(std::ostream& os, const ChainstateRole& role);
 } // namespace kernel
 
-//! This enum describes the various roles a specific Chainstate instance can take.
-//! Other parts of the system sometimes need to vary in behavior depending on the
-//! existence of a background validation chainstate, e.g. when building indexes.
-enum class ChainstateRole {
-    // Single chainstate in use, "normal" IBD mode.
-    NORMAL,
-
-    // Doing IBD-style validation in the background. Implies use of an assumed-valid
-    // chainstate.
-    BACKGROUND,
-
-    // Active assumed-valid chainstate. Implies use of a background IBD chainstate.
-    ASSUMEDVALID,
-};
-
-std::ostream& operator<<(std::ostream& os, const ChainstateRole& role);
-
 #endif // BITCOIN_KERNEL_CHAIN_H

src/kernel/types.h

@@ -0,0 +1,30 @@
+// Copyright (c) The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+//! @file kernel/types.h is a home for simple enum and struct type definitions
+//! that can be used internally by functions in the libbitcoin_kernel library,
+//! but also used externally by node, wallet, and GUI code.
+//!
+//! This file is intended to define only simple types that do not have external
+//! dependencies. More complicated types should be defined in dedicated header
+//! files.
+
+#ifndef BITCOIN_KERNEL_TYPES_H
+#define BITCOIN_KERNEL_TYPES_H
+
+namespace kernel {
+//! Information about chainstate that notifications are sent from.
+struct ChainstateRole {
+    //! Whether this is a notification from a chainstate that's been fully
+    //! validated starting from the genesis block. False if it is from an
+    //! assumeutxo snapshot chainstate that has not been validated yet.
+    bool validated{true};
+
+    //! Whether this is a historical chainstate downloading old blocks to
+    //! validate an assumeutxo snapshot, not syncing to the network tip.
+    bool historical{false};
+};
+} // namespace kernel
+
+#endif // BITCOIN_KERNEL_TYPES_H

src/net_processing.cpp

@@ -22,7 +22,7 @@
 #include <flatfile.h>
 #include <headerssync.h>
 #include <index/blockfilterindex.h>
-#include <kernel/chain.h>
+#include <kernel/types.h>
 #include <logging.h>
 #include <merkleblock.h>
 #include <net.h>
@@ -85,6 +85,7 @@
 #include <typeinfo>
 #include <utility>
 
+using kernel::ChainstateRole;
 using namespace util::hex_literals;
 
 TRACEPOINT_SEMAPHORE(net, inbound_message);
@@ -507,7 +508,7 @@ public:
     /** Overridden from CValidationInterface. */
     void ActiveTipChange(const CBlockIndex& new_tip, bool) override
         EXCLUSIVE_LOCKS_REQUIRED(!m_tx_download_mutex);
-    void BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected) override
+    void BlockConnected(const ChainstateRole& role, const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected) override
         EXCLUSIVE_LOCKS_REQUIRED(!m_tx_download_mutex);
     void BlockDisconnected(const std::shared_ptr<const CBlock> &block, const CBlockIndex* pindex) override
         EXCLUSIVE_LOCKS_REQUIRED(!m_tx_download_mutex);
@@ -1387,7 +1388,7 @@ void PeerManagerImpl::FindNextBlocksToDownload(const Peer& peer, unsigned int co
     // When we sync with AssumeUtxo and discover the snapshot is not in the peer's best chain, abort:
     // We can't reorg to this chain due to missing undo data until the background sync has finished,
     // so downloading blocks from it would be futile.
-    const CBlockIndex* snap_base{m_chainman.GetSnapshotBaseBlock()};
+    const CBlockIndex* snap_base{m_chainman.CurrentChainstate().SnapshotBase()};
     if (snap_base && state->pindexBestKnownBlock->GetAncestor(snap_base->nHeight) != snap_base) {
         LogDebug(BCLog::NET, "Not downloading blocks from peer=%d, which doesn't have the snapshot block in its best chain.\n", peer.m_id);
         return;
@@ -1946,7 +1947,7 @@ void PeerManagerImpl::ActiveTipChange(const CBlockIndex& new_tip, bool is_ibd)
  * possibly reduce dynamic block stalling timeout.
  */
 void PeerManagerImpl::BlockConnected(
-    ChainstateRole role,
+    const ChainstateRole& role,
     const std::shared_ptr<const CBlock>& pblock,
     const CBlockIndex* pindex)
 {
@@ -1965,8 +1966,8 @@ void PeerManagerImpl::BlockConnected(
     }
 
     // The following task can be skipped since we don't maintain a mempool for
-    // the ibd/background chainstate.
+    // the historical chainstate.
-    if (role == ChainstateRole::BACKGROUND) {
+    if (role.historical) {
         return;
     }
     LOCK(m_tx_download_mutex);
@@ -5921,18 +5922,19 @@ bool PeerManagerImpl::SendMessages(CNode* pto)
                 return std::max(0, MAX_BLOCKS_IN_TRANSIT_PER_PEER - static_cast<int>(state.vBlocksInFlight.size()));
             };
 
-            // If a snapshot chainstate is in use, we want to find its next blocks
+            // If there are multiple chainstates, download blocks for the
-            // before the background chainstate to prioritize getting to network tip.
+            // current chainstate first, to prioritize getting to network tip
+            // before downloading historical blocks.
             FindNextBlocksToDownload(*peer, get_inflight_budget(), vToDownload, staller);
-            if (m_chainman.BackgroundSyncInProgress() && !IsLimitedPeer(*peer)) {
+            auto historical_blocks{m_chainman.GetHistoricalBlockRange()};
-                // If the background tip is not an ancestor of the snapshot block,
+            if (historical_blocks && !IsLimitedPeer(*peer)) {
+                // If the first needed historical block is not an ancestor of the last,
                 // we need to start requesting blocks from their last common ancestor.
-                const CBlockIndex *from_tip = LastCommonAncestor(m_chainman.GetBackgroundSyncTip(), m_chainman.GetSnapshotBaseBlock());
+                const CBlockIndex* from_tip = LastCommonAncestor(historical_blocks->first, historical_blocks->second);
                 TryDownloadingHistoricalBlocks(
                     *peer,
                     get_inflight_budget(),
-                    vToDownload, from_tip,
+                    vToDownload, from_tip, historical_blocks->second);
-                    Assert(m_chainman.GetSnapshotBaseBlock()));
             }
             for (const CBlockIndex *pindex : vToDownload) {
                 uint32_t nFetchFlags = GetFetchFlags(*peer);

src/node/blockstorage.cpp

@@ -12,9 +12,11 @@
 #include <flatfile.h>
 #include <hash.h>
 #include <kernel/blockmanager_opts.h>
+#include <kernel/chain.h>
 #include <kernel/chainparams.h>
 #include <kernel/messagestartchars.h>
 #include <kernel/notifications_interface.h>
+#include <kernel/types.h>
 #include <logging.h>
 #include <pow.h>
 #include <primitives/block.h>
@@ -282,8 +284,7 @@ void BlockManager::PruneOneBlockFile(const int fileNumber)
 void BlockManager::FindFilesToPruneManual(
     std::set<int>& setFilesToPrune,
     int nManualPruneHeight,
-    const Chainstate& chain,
+    const Chainstate& chain)
-    ChainstateManager& chainman)
 {
     assert(IsPruneMode() && nManualPruneHeight > 0);
 
@@ -292,7 +293,7 @@ void BlockManager::FindFilesToPruneManual(
         return;
     }
 
-    const auto [min_block_to_prune, last_block_can_prune] = chainman.GetPruneRange(chain, nManualPruneHeight);
+    const auto [min_block_to_prune, last_block_can_prune] = chain.GetPruneRange(nManualPruneHeight);
 
     int count = 0;
     for (int fileNumber = 0; fileNumber < this->MaxBlockfileNum(); fileNumber++) {
@@ -316,9 +317,16 @@ void BlockManager::FindFilesToPrune(
     ChainstateManager& chainman)
 {
     LOCK2(cs_main, cs_LastBlockFile);
-    // Distribute our -prune budget over all chainstates.
+    // Compute `target` value with maximum size (in bytes) of blocks below the
+    // `last_prune` height which should be preserved and not pruned. The
+    // `target` value will be derived from the -prune preference provided by the
+    // user. If there is a historical chainstate being used to populate indexes
+    // and validate the snapshot, the target is divided by two so half of the
+    // block storage will be reserved for the historical chainstate, and the
+    // other half will be reserved for the most-work chainstate.
+    const int num_chainstates{chainman.HistoricalChainstate() ? 2 : 1};
     const auto target = std::max(
-        MIN_DISK_SPACE_FOR_BLOCK_FILES, GetPruneTarget() / chainman.GetAll().size());
+        MIN_DISK_SPACE_FOR_BLOCK_FILES, GetPruneTarget() / num_chainstates);
     const uint64_t target_sync_height = chainman.m_best_header->nHeight;
 
     if (chain.m_chain.Height() < 0 || target == 0) {
@@ -328,7 +336,7 @@ void BlockManager::FindFilesToPrune(
         return;
     }
 
-    const auto [min_block_to_prune, last_block_can_prune] = chainman.GetPruneRange(chain, last_prune);
+    const auto [min_block_to_prune, last_block_can_prune] = chain.GetPruneRange(last_prune);
 
     uint64_t nCurrentUsage = CalculateCurrentUsage();
     // We don't check to prune until after we've allocated new space for files
@@ -1276,16 +1284,8 @@ void ImportBlocks(ChainstateManager& chainman, std::span<const fs::path> import_
     }
 
     // scan for better chains in the block chain database, that are not yet connected in the active best chain
-
+    if (auto result = chainman.ActivateBestChains(); !result) {
-    // We can't hold cs_main during ActivateBestChain even though we're accessing
+        chainman.GetNotifications().fatalError(util::ErrorString(result));
-    // the chainman unique_ptrs since ABC requires us not to be holding cs_main, so retrieve
-    // the relevant pointers before the ABC call.
-    for (Chainstate* chainstate : WITH_LOCK(::cs_main, return chainman.GetAll())) {
-        BlockValidationState state;
-        if (!chainstate->ActivateBestChain(state, nullptr)) {
-            chainman.GetNotifications().fatalError(strprintf(_("Failed to connect best block (%s)."), state.ToString()));
-            return;
-        }
     }
     // End scope of ImportingNow
 }

src/node/blockstorage.h

@@ -223,8 +223,7 @@ private:
     void FindFilesToPruneManual(
         std::set<int>& setFilesToPrune,
         int nManualPruneHeight,
-        const Chainstate& chain,
+        const Chainstate& chain);
-        ChainstateManager& chainman);
 
     /**
      * Prune block and undo files (blk???.dat and rev???.dat) so that the disk space used is less than a user-defined target.

src/node/chainstate.cpp

@@ -66,8 +66,8 @@ static ChainstateLoadResult CompleteChainstateInitialization(
         return {ChainstateLoadStatus::FAILURE, _("Error initializing block database")};
     }
 
-    auto is_coinsview_empty = [&](Chainstate* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
+    auto is_coinsview_empty = [&](Chainstate& chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
-        return options.wipe_chainstate_db || chainstate->CoinsTip().GetBestBlock().IsNull();
+        return options.wipe_chainstate_db || chainstate.CoinsTip().GetBestBlock().IsNull();
     };
 
     assert(chainman.m_total_coinstip_cache > 0);
@@ -78,12 +78,12 @@ static ChainstateLoadResult CompleteChainstateInitialization(
     // recalculated by `chainman.MaybeRebalanceCaches()`. The discount factor
     // is conservatively chosen such that the sum of the caches does not exceed
     // the allowable amount during this temporary initialization state.
-    double init_cache_fraction = chainman.GetAll().size() > 1 ? 0.2 : 1.0;
+    double init_cache_fraction = chainman.HistoricalChainstate() ? 0.2 : 1.0;
 
     // At this point we're either in reindex or we've loaded a useful
     // block tree into BlockIndex()!
 
-    for (Chainstate* chainstate : chainman.GetAll()) {
+    for (const auto& chainstate : chainman.m_chainstates) {
         LogInfo("Initializing chainstate %s", chainstate->ToString());
 
         try {
@@ -117,7 +117,7 @@ static ChainstateLoadResult CompleteChainstateInitialization(
         chainstate->InitCoinsCache(chainman.m_total_coinstip_cache * init_cache_fraction);
         assert(chainstate->CanFlushToDisk());
 
-        if (!is_coinsview_empty(chainstate)) {
+        if (!is_coinsview_empty(*chainstate)) {
             // LoadChainTip initializes the chain based on CoinsTip()'s best block
             if (!chainstate->LoadChainTip()) {
                 return {ChainstateLoadStatus::FAILURE, _("Error initializing block database")};
@@ -126,9 +126,9 @@ static ChainstateLoadResult CompleteChainstateInitialization(
         }
     }
 
-    auto chainstates{chainman.GetAll()};
+    const auto& chainstates{chainman.m_chainstates};
     if (std::any_of(chainstates.begin(), chainstates.end(),
-                    [](const Chainstate* cs) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return cs->NeedsRedownload(); })) {
+                    [](const auto& cs) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return cs->NeedsRedownload(); })) {
         return {ChainstateLoadStatus::FAILURE, strprintf(_("Witness data for blocks after height %d requires validation. Please restart with -reindex."),
                                                          chainman.GetConsensus().SegwitHeight)};
     };
@@ -166,16 +166,19 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
     chainman.m_total_coinsdb_cache = cache_sizes.coins_db;
 
     // Load the fully validated chainstate.
-    chainman.InitializeChainstate(options.mempool);
+    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()) {
+        if (!chainman.DeleteChainstate(*assumeutxo_cs)) {
             return {ChainstateLoadStatus::FAILURE_FATAL, Untranslated("Couldn't remove snapshot chainstate.")};
         }
+        assumeutxo_cs = nullptr;
     }
 
     auto [init_status, init_error] = CompleteChainstateInitialization(chainman, options);
@@ -191,22 +194,22 @@ 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
     } else if (snapshot_completion == SnapshotCompletionResult::SUCCESS) {
         LogInfo("[snapshot] cleaning up unneeded background chainstate, then reinitializing");
-        if (!chainman.ValidatedSnapshotCleanup()) {
+        if (!chainman.ValidatedSnapshotCleanup(validated_cs, *assumeutxo_cs)) {
             return {ChainstateLoadStatus::FAILURE_FATAL, Untranslated("Background chainstate cleanup failed unexpectedly.")};
         }
 
         // Because ValidatedSnapshotCleanup() has torn down chainstates with
         // ChainstateManager::ResetChainstates(), reinitialize them here without
         // duplicating the blockindex work above.
-        assert(chainman.GetAll().empty());
+        assert(chainman.m_chainstates.empty());
-        assert(!chainman.IsSnapshotActive());
-        assert(!chainman.IsSnapshotValidated());
 
         chainman.InitializeChainstate(options.mempool);
 
@@ -229,14 +232,14 @@ ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSize
 
 ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, const ChainstateLoadOptions& options)
 {
-    auto is_coinsview_empty = [&](Chainstate* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
+    auto is_coinsview_empty = [&](Chainstate& chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
-        return options.wipe_chainstate_db || chainstate->CoinsTip().GetBestBlock().IsNull();
+        return options.wipe_chainstate_db || chainstate.CoinsTip().GetBestBlock().IsNull();
     };
 
     LOCK(cs_main);
 
-    for (Chainstate* chainstate : chainman.GetAll()) {
+    for (auto& chainstate : chainman.m_chainstates) {
-        if (!is_coinsview_empty(chainstate)) {
+        if (!is_coinsview_empty(*chainstate)) {
             const CBlockIndex* tip = chainstate->m_chain.Tip();
             if (tip && tip->nTime > GetTime() + MAX_FUTURE_BLOCK_TIME) {
                 return {ChainstateLoadStatus::FAILURE, _("The block database contains a block which appears to be from the future. "

src/node/interfaces.cpp

@@ -81,6 +81,7 @@ using interfaces::MakeSignalHandler;
 using interfaces::Mining;
 using interfaces::Node;
 using interfaces::WalletLoader;
+using kernel::ChainstateRole;
 using node::BlockAssembler;
 using node::BlockWaitOptions;
 using util::Join;
@@ -461,7 +462,7 @@ public:
     {
         m_notifications->transactionRemovedFromMempool(tx, reason);
     }
-    void BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock>& block, const CBlockIndex* index) override
+    void BlockConnected(const ChainstateRole& role, const std::shared_ptr<const CBlock>& block, const CBlockIndex* index) override
     {
         m_notifications->blockConnected(role, kernel::MakeBlockInfo(index, block.get()));
     }
@@ -473,7 +474,8 @@ public:
     {
         m_notifications->updatedBlockTip();
     }
-    void ChainStateFlushed(ChainstateRole role, const CBlockLocator& locator) override {
+    void ChainStateFlushed(const ChainstateRole& role, const CBlockLocator& locator) override
+    {
         m_notifications->chainStateFlushed(role, locator);
     }
     std::shared_ptr<Chain::Notifications> m_notifications;
@@ -843,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/node/utxo_snapshot.cpp

@@ -25,7 +25,7 @@ bool WriteSnapshotBaseBlockhash(Chainstate& snapshot_chainstate)
     AssertLockHeld(::cs_main);
     assert(snapshot_chainstate.m_from_snapshot_blockhash);
 
-    const std::optional<fs::path> chaindir = snapshot_chainstate.CoinsDB().StoragePath();
+    const std::optional<fs::path> chaindir = snapshot_chainstate.StoragePath();
     assert(chaindir); // Sanity check that chainstate isn't in-memory.
     const fs::path write_to = *chaindir / node::SNAPSHOT_BLOCKHASH_FILENAME;
 
@@ -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/rpc/blockchain.cpp

@@ -3428,7 +3428,7 @@ return RPCHelpMan{
 
     ChainstateManager& chainman = EnsureAnyChainman(request.context);
 
-    auto make_chain_data = [&](const Chainstate& cs, bool validated) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
+    auto make_chain_data = [&](const Chainstate& cs) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
         AssertLockHeld(::cs_main);
         UniValue data(UniValue::VOBJ);
         if (!cs.m_chain.Tip()) {
@@ -3448,17 +3448,16 @@ return RPCHelpMan{
         if (cs.m_from_snapshot_blockhash) {
             data.pushKV("snapshot_blockhash", cs.m_from_snapshot_blockhash->ToString());
         }
-        data.pushKV("validated", validated);
+        data.pushKV("validated", cs.m_assumeutxo == Assumeutxo::VALIDATED);
         return data;
     };
 
     obj.pushKV("headers", chainman.m_best_header ? chainman.m_best_header->nHeight : -1);
-
-    const auto& chainstates = chainman.GetAll();
     UniValue obj_chainstates{UniValue::VARR};
-    for (Chainstate* cs : chainstates) {
+    if (const Chainstate * cs{chainman.HistoricalChainstate()}) {
-      obj_chainstates.push_back(make_chain_data(*cs, !cs->m_from_snapshot_blockhash || chainstates.size() == 1));
+        obj_chainstates.push_back(make_chain_data(*cs));
     }
+    obj_chainstates.push_back(make_chain_data(chainman.CurrentChainstate()));
     obj.pushKV("chainstates", std::move(obj_chainstates));
     return obj;
 }

src/test/chainstate_write_tests.cpp

@@ -8,6 +8,8 @@
 
 #include <boost/test/unit_test.hpp>
 
+using kernel::ChainstateRole;
+
 // Taken from validation.cpp
 static constexpr auto DATABASE_WRITE_INTERVAL_MIN{50min};
 static constexpr auto DATABASE_WRITE_INTERVAL_MAX{70min};
@@ -18,7 +20,7 @@ BOOST_FIXTURE_TEST_CASE(chainstate_write_interval, TestingSetup)
 {
     struct TestSubscriber final : CValidationInterface {
         bool m_did_flush{false};
-        void ChainStateFlushed(ChainstateRole, const CBlockLocator&) override
+        void ChainStateFlushed(const ChainstateRole&, const CBlockLocator&) override
         {
             m_did_flush = true;
         }
@@ -55,7 +57,7 @@ BOOST_FIXTURE_TEST_CASE(write_during_multiblock_activation, TestChain100Setup)
     {
         const CBlockIndex* m_tip{nullptr};
         const CBlockIndex* m_flushed_at_block{nullptr};
-        void ChainStateFlushed(ChainstateRole, const CBlockLocator&) override
+        void ChainStateFlushed(const ChainstateRole&, const CBlockLocator&) override
         {
             m_flushed_at_block = m_tip;
         }

src/test/coinstatsindex_tests.cpp

@@ -6,12 +6,15 @@
 #include <index/coinstatsindex.h>
 #include <interfaces/chain.h>
 #include <kernel/coinstats.h>
+#include <kernel/types.h>
 #include <test/util/setup_common.h>
 #include <test/util/validation.h>
 #include <validation.h>
 
 #include <boost/test/unit_test.hpp>
 
+using kernel::ChainstateRole;
+
 BOOST_AUTO_TEST_SUITE(coinstatsindex_tests)
 
 BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup)
@@ -101,7 +104,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup)
         // Send block connected notification, then stop the index without
         // sending a chainstate flushed notification. Prior to #24138, this
         // would cause the index to be corrupted and fail to reload.
-        ValidationInterfaceTest::BlockConnected(ChainstateRole::NORMAL, index, new_block, new_block_index);
+        ValidationInterfaceTest::BlockConnected(ChainstateRole{}, index, new_block, new_block_index);
         index.Stop();
     }
 

src/test/fuzz/utxo_snapshot.cpp

@@ -110,7 +110,7 @@ void utxo_snapshot_fuzz(FuzzBufferType buffer)
 
     const auto snapshot_path = gArgs.GetDataDirNet() / "fuzzed_snapshot.dat";
 
-    Assert(!chainman.SnapshotBlockhash());
+    Assert(!chainman.ActiveChainstate().m_from_snapshot_blockhash);
 
     {
         AutoFile outfile{fsbridge::fopen(snapshot_path, "wb")};
@@ -183,15 +183,13 @@ void utxo_snapshot_fuzz(FuzzBufferType buffer)
     if (ActivateFuzzedSnapshot()) {
         LOCK(::cs_main);
         Assert(!chainman.ActiveChainstate().m_from_snapshot_blockhash->IsNull());
-        Assert(*chainman.ActiveChainstate().m_from_snapshot_blockhash ==
-               *chainman.SnapshotBlockhash());
         const auto& coinscache{chainman.ActiveChainstate().CoinsTip()};
         for (const auto& block : *g_chain) {
             Assert(coinscache.HaveCoin(COutPoint{block->vtx.at(0)->GetHash(), 0}));
             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);
@@ -202,7 +200,6 @@ void utxo_snapshot_fuzz(FuzzBufferType buffer)
         Assert(g_chain->size() == coinscache.GetCacheSize());
         dirty_chainman = true;
     } else {
-        Assert(!chainman.SnapshotBlockhash());
         Assert(!chainman.ActiveChainstate().m_from_snapshot_blockhash);
     }
     // Snapshot should refuse to load a second time regardless of validity

src/test/util/chainstate.h

@@ -81,7 +81,7 @@ CreateAndActivateUTXOSnapshot(
             Chainstate& chain = node.chainman->ActiveChainstate();
             Assert(chain.LoadGenesisBlock());
             // These cache values will be corrected shortly in `MaybeRebalanceCaches`.
-            chain.InitCoinsDB(1 << 20, true, false, "");
+            chain.InitCoinsDB(1 << 20, /*in_memory=*/true, /*should_wipe=*/false);
             chain.InitCoinsCache(1 << 20);
             chain.CoinsTip().SetBestBlock(gen_hash);
             chain.setBlockIndexCandidates.insert(node.chainman->m_blockman.LookupBlockIndex(gen_hash));

src/test/util/validation.cpp

@@ -9,15 +9,19 @@
 #include <validation.h>
 #include <validationinterface.h>
 
+using kernel::ChainstateRole;
+
 void TestChainstateManager::DisableNextWrite()
 {
     struct TestChainstate : public Chainstate {
         void ResetNextWrite() { m_next_write = NodeClock::time_point::max() - 1s; }
     };
-    for (auto* cs : GetAll()) {
+    LOCK(::cs_main);
-        static_cast<TestChainstate*>(cs)->ResetNextWrite();
+    for (const auto& cs : m_chainstates) {
+        static_cast<TestChainstate&>(*cs).ResetNextWrite();
     }
 }
+
 void TestChainstateManager::ResetIbd()
 {
     m_cached_finished_ibd = false;
@@ -32,10 +36,10 @@ void TestChainstateManager::JumpOutOfIbd()
 }
 
 void ValidationInterfaceTest::BlockConnected(
-        ChainstateRole role,
+    const ChainstateRole& role,
-        CValidationInterface& obj,
+    CValidationInterface& obj,
-        const std::shared_ptr<const CBlock>& block,
+    const std::shared_ptr<const CBlock>& block,
-        const CBlockIndex* pindex)
+    const CBlockIndex* pindex)
 {
     obj.BlockConnected(role, block, pindex);
 }

src/test/util/validation.h

@@ -22,7 +22,7 @@ class ValidationInterfaceTest
 {
 public:
     static void BlockConnected(
-        ChainstateRole role,
+        const kernel::ChainstateRole& role,
         CValidationInterface& obj,
         const std::shared_ptr<const CBlock>& block,
         const CBlockIndex* pindex);

src/test/validation_block_tests.cpp

@@ -19,6 +19,7 @@
 
 #include <thread>
 
+using kernel::ChainstateRole;
 using node::BlockAssembler;
 
 namespace validation_block_tests {
@@ -43,7 +44,7 @@ struct TestSubscriber final : public CValidationInterface {
         BOOST_CHECK_EQUAL(m_expected_tip, pindexNew->GetBlockHash());
     }
 
-    void BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex) override
+    void BlockConnected(const ChainstateRole& role, const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex) override
     {
         BOOST_CHECK_EQUAL(m_expected_tip, block->hashPrevBlock);
         BOOST_CHECK_EQUAL(m_expected_tip, pindex->pprev->GetBlockHash());

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();
 
@@ -107,16 +107,7 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup)
 
     curr_tip = get_notify_tip();
 
-    BOOST_CHECK_EQUAL(chainman.GetAll().size(), 2);
+    Chainstate& background_cs{*Assert(WITH_LOCK(::cs_main, return chainman.HistoricalChainstate()))};
-
-    Chainstate& background_cs{*Assert([&]() -> Chainstate* {
-        for (Chainstate* cs : chainman.GetAll()) {
-            if (cs != &chainman.ActiveChainstate()) {
-                return cs;
-            }
-        }
-        return nullptr;
-    }())};
 
     // Append the first block to the background chain.
     BlockValidationState state;

src/test/validation_chainstatemanager_tests.cpp

@@ -40,19 +40,19 @@ BOOST_FIXTURE_TEST_SUITE(validation_chainstatemanager_tests, TestingSetup)
 BOOST_FIXTURE_TEST_CASE(chainstatemanager, TestChain100Setup)
 {
     ChainstateManager& manager = *m_node.chainman;
-    std::vector<Chainstate*> chainstates;
 
-    BOOST_CHECK(!manager.SnapshotBlockhash().has_value());
+    BOOST_CHECK(WITH_LOCK(::cs_main, return !manager.CurrentChainstate().m_from_snapshot_blockhash));
 
     // Create a legacy (IBD) chainstate.
     //
     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();
+        LOCK(manager.GetMutex());
-    BOOST_CHECK_EQUAL_COLLECTIONS(all.begin(), all.end(), chainstates.begin(), chainstates.end());
+        BOOST_CHECK_EQUAL(manager.m_chainstates.size(), 1);
+        BOOST_CHECK_EQUAL(manager.m_chainstates[0].get(), &c1);
+    }
 
     auto& active_chain = WITH_LOCK(manager.GetMutex(), return manager.ActiveChain());
     BOOST_CHECK_EQUAL(&active_chain, &c1.m_chain);
@@ -64,13 +64,12 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager, TestChain100Setup)
     auto exp_tip = c1.m_chain.Tip();
     BOOST_CHECK_EQUAL(active_tip, exp_tip);
 
-    BOOST_CHECK(!manager.SnapshotBlockhash().has_value());
+    BOOST_CHECK(WITH_LOCK(::cs_main, return !manager.CurrentChainstate().m_from_snapshot_blockhash));
 
     // 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);
     {
@@ -83,13 +82,16 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager, TestChain100Setup)
     BlockValidationState _;
     BOOST_CHECK(c2.ActivateBestChain(_, nullptr));
 
-    BOOST_CHECK_EQUAL(manager.SnapshotBlockhash().value(), snapshot_blockhash);
+    BOOST_CHECK_EQUAL(WITH_LOCK(::cs_main, return *manager.CurrentChainstate().m_from_snapshot_blockhash), snapshot_blockhash);
-    BOOST_CHECK(manager.IsSnapshotActive());
+    BOOST_CHECK(WITH_LOCK(::cs_main, return manager.CurrentChainstate().m_assumeutxo == Assumeutxo::UNVALIDATED));
-    BOOST_CHECK(WITH_LOCK(::cs_main, return !manager.IsSnapshotValidated()));
     BOOST_CHECK_EQUAL(&c2, &manager.ActiveChainstate());
     BOOST_CHECK(&c1 != &manager.ActiveChainstate());
-    auto all2 = manager.GetAll();
+    {
-    BOOST_CHECK_EQUAL_COLLECTIONS(all2.begin(), all2.end(), chainstates.begin(), chainstates.end());
+        LOCK(manager.GetMutex());
+        BOOST_CHECK_EQUAL(manager.m_chainstates.size(), 2);
+        BOOST_CHECK_EQUAL(manager.m_chainstates[0].get(), &c1);
+        BOOST_CHECK_EQUAL(manager.m_chainstates[1].get(), &c2);
+    }
 
     auto& active_chain2 = WITH_LOCK(manager.GetMutex(), return manager.ActiveChain());
     BOOST_CHECK_EQUAL(&active_chain2, &c2.m_chain);
@@ -135,7 +137,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);
@@ -182,12 +184,10 @@ struct SnapshotTestSetup : TestChain100Setup {
     {
         ChainstateManager& chainman = *Assert(m_node.chainman);
 
-        BOOST_CHECK(!chainman.IsSnapshotActive());
-
         {
             LOCK(::cs_main);
-            BOOST_CHECK(!chainman.IsSnapshotValidated());
+            BOOST_CHECK(!chainman.CurrentChainstate().m_from_snapshot_blockhash);
-            BOOST_CHECK(!node::FindSnapshotChainstateDir(chainman.m_options.datadir));
+            BOOST_CHECK(!node::FindAssumeutxoChainstateDir(chainman.m_options.datadir));
         }
 
         size_t initial_size;
@@ -215,7 +215,6 @@ struct SnapshotTestSetup : TestChain100Setup {
         // Snapshot should refuse to load at this height.
         BOOST_REQUIRE(!CreateAndActivateUTXOSnapshot(this));
         BOOST_CHECK(!chainman.ActiveChainstate().m_from_snapshot_blockhash);
-        BOOST_CHECK(!chainman.SnapshotBlockhash());
 
         // Mine 10 more blocks, putting at us height 110 where a valid assumeutxo value can
         // be found.
@@ -240,7 +239,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,25 +263,22 @@ 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());
-        BOOST_CHECK_EQUAL(
-            *chainman.ActiveChainstate().m_from_snapshot_blockhash,
-            *chainman.SnapshotBlockhash());
 
         Chainstate& snapshot_chainstate = chainman.ActiveChainstate();
 
         {
             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(
                 *node::ReadSnapshotBaseBlockhash(found),
-                *chainman.SnapshotBlockhash());
+                *Assert(chainman.CurrentChainstate().m_from_snapshot_blockhash));
         }
 
         const auto& au_data = ::Params().AssumeutxoForHeight(snapshot_height);
@@ -291,7 +287,7 @@ struct SnapshotTestSetup : TestChain100Setup {
         BOOST_CHECK_EQUAL(tip->m_chain_tx_count, au_data->m_chain_tx_count);
 
         // To be checked against later when we try loading a subsequent snapshot.
-        uint256 loaded_snapshot_blockhash{*chainman.SnapshotBlockhash()};
+        uint256 loaded_snapshot_blockhash{*Assert(WITH_LOCK(chainman.GetMutex(), return chainman.CurrentChainstate().m_from_snapshot_blockhash))};
 
         // Make some assertions about the both chainstates. These checks ensure the
         // legacy chainstate hasn't changed and that the newly created chainstate
@@ -300,7 +296,7 @@ struct SnapshotTestSetup : TestChain100Setup {
             LOCK(::cs_main);
             int chains_tested{0};
 
-            for (Chainstate* chainstate : chainman.GetAll()) {
+            for (const auto& chainstate : chainman.m_chainstates) {
                 BOOST_TEST_MESSAGE("Checking coins in " << chainstate->ToString());
                 CCoinsViewCache& coinscache = chainstate->CoinsTip();
 
@@ -333,10 +329,10 @@ struct SnapshotTestSetup : TestChain100Setup {
             size_t coins_in_background{0};
             size_t coins_missing_from_background{0};
 
-            for (Chainstate* chainstate : chainman.GetAll()) {
+            for (const auto& chainstate : chainman.m_chainstates) {
                 BOOST_TEST_MESSAGE("Checking coins in " << chainstate->ToString());
                 CCoinsViewCache& coinscache = chainstate->CoinsTip();
-                bool is_background = chainstate != &chainman.ActiveChainstate();
+                bool is_background = chainstate.get() != &chainman.ActiveChainstate();
 
                 for (CTransactionRef& txn : m_coinbase_txns) {
                     COutPoint op{txn->GetHash(), 0};
@@ -373,15 +369,17 @@ struct SnapshotTestSetup : TestChain100Setup {
 
         BOOST_TEST_MESSAGE("Simulating node restart");
         {
-            for (Chainstate* cs : chainman.GetAll()) {
+            LOCK(chainman.GetMutex());
-                LOCK(::cs_main);
+            for (const auto& cs : chainman.m_chainstates) {
-                cs->ForceFlushStateToDisk();
+                if (cs->CanFlushToDisk()) cs->ForceFlushStateToDisk();
             }
+        }
+        {
             // Process all callbacks referring to the old manager before wiping it.
             m_node.validation_signals->SyncWithValidationInterfaceQueue();
             LOCK(::cs_main);
             chainman.ResetChainstates();
-            BOOST_CHECK_EQUAL(chainman.GetAll().size(), 0);
+            BOOST_CHECK_EQUAL(chainman.m_chainstates.size(), 0);
             m_node.notifications = std::make_unique<KernelNotifications>(Assert(m_node.shutdown_request), m_node.exit_status, *Assert(m_node.warnings));
             const ChainstateManager::Options chainman_opts{
                 .chainparams = ::Params(),
@@ -446,17 +444,16 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup)
     BOOST_CHECK_EQUAL(assumed_tip->nHeight, 120);
 
     auto reload_all_block_indexes = [&]() {
+        LOCK(chainman.GetMutex());
         // For completeness, we also reset the block sequence counters to
         // ensure that no state which affects the ranking of tip-candidates is
         // retained (even though this isn't strictly necessary).
-        WITH_LOCK(::cs_main, return chainman.ResetBlockSequenceCounters());
+        chainman.ResetBlockSequenceCounters();
-        for (Chainstate* cs : chainman.GetAll()) {
+        for (const auto& cs : chainman.m_chainstates) {
-            LOCK(::cs_main);
             cs->ClearBlockIndexCandidates();
             BOOST_CHECK(cs->setBlockIndexCandidates.empty());
         }
-
+        chainman.LoadBlockIndex();
-        WITH_LOCK(::cs_main, chainman.LoadBlockIndex());
     };
 
     // Ensure that without any assumed-valid BlockIndex entries, only the current tip is
@@ -489,8 +486,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,
+    Chainstate& cs2{WITH_LOCK(::cs_main, return chainman.AddChainstate(std::make_unique<Chainstate>(nullptr, chainman.m_blockman, chainman, *assumed_base->phashBlock)))};
-        return chainman.ActivateExistingSnapshot(*assumed_base->phashBlock));
 
     // Set tip of the fully validated chain to be the validated tip
     cs1.m_chain.SetTip(*validated_tip);
@@ -557,7 +553,8 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_loadblockindex, TestChain100Setup)
     BOOST_CHECK_EQUAL(cs2.setBlockIndexCandidates.size(), num_indexes - last_assumed_valid_idx + 1);
 }
 
-//! Ensure that snapshot chainstates initialize properly when found on disk.
+//! Ensure that snapshot chainstate can be loaded when found on disk after a
+//! restart, and that new blocks can be connected to both chainstates.
 BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup)
 {
     ChainstateManager& chainman = *Assert(m_node.chainman);
@@ -565,16 +562,15 @@ 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");
 
-    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());
 
-    auto all_chainstates = chainman.GetAll();
+    BOOST_CHECK_EQUAL(WITH_LOCK(chainman.GetMutex(), return chainman.m_chainstates.size()), 2);
-    BOOST_CHECK_EQUAL(all_chainstates.size(), 2);
 
     // "Rewind" the background chainstate so that its tip is not at the
     // base block of the snapshot - this is so after simulating a node restart,
@@ -591,8 +587,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup)
     BOOST_CHECK_EQUAL(bg_chainstate.m_chain.Height(), 109);
 
     // Test that simulating a shutdown (resetting ChainstateManager) and then performing
-    // chainstate reinitializing successfully cleans up the background-validation
+    // chainstate reinitializing successfully reloads both chainstates.
-    // chainstate data, and we end up with a single chainstate that is at tip.
     ChainstateManager& chainman_restarted = this->SimulateNodeRestart();
 
     BOOST_TEST_MESSAGE("Performing Load/Verify/Activate of chainstate");
@@ -602,12 +597,20 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup)
 
     {
         LOCK(chainman_restarted.GetMutex());
-        BOOST_CHECK_EQUAL(chainman_restarted.GetAll().size(), 2);
+        BOOST_CHECK_EQUAL(chainman_restarted.m_chainstates.size(), 2);
-        BOOST_CHECK(chainman_restarted.IsSnapshotActive());
+        // Background chainstate has height of 109 not 110 here due to a quirk
-        BOOST_CHECK(!chainman_restarted.IsSnapshotValidated());
+        // of the LoadVerifyActivate only calling ActivateBestChain on one
+        // chainstate. The height would be 110 after a real restart, but it's
+        // fine for this test which is focused on the snapshot chainstate.
+        BOOST_CHECK_EQUAL(chainman_restarted.m_chainstates[0]->m_chain.Height(), 109);
+        BOOST_CHECK_EQUAL(chainman_restarted.m_chainstates[1]->m_chain.Height(), 210);
+
+        BOOST_CHECK(chainman_restarted.CurrentChainstate().m_from_snapshot_blockhash);
+        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);
+        BOOST_CHECK_EQUAL(chainman_restarted.HistoricalChainstate()->m_chain.Height(), 109);
     }
 
     BOOST_TEST_MESSAGE(
@@ -618,12 +621,11 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_init, SnapshotTestSetup)
         BOOST_CHECK_EQUAL(chainman_restarted.ActiveHeight(), 220);
 
         // Background chainstate should be unaware of new blocks on the snapshot
-        // chainstate.
+        // chainstate, but the block disconnected above is now reattached.
-        for (Chainstate* cs : chainman_restarted.GetAll()) {
+        BOOST_CHECK_EQUAL(chainman_restarted.m_chainstates.size(), 2);
-            if (cs != &chainman_restarted.ActiveChainstate()) {
+        BOOST_CHECK_EQUAL(chainman_restarted.m_chainstates[0]->m_chain.Height(), 110);
-                BOOST_CHECK_EQUAL(cs->m_chain.Height(), 109);
+        BOOST_CHECK_EQUAL(chainman_restarted.m_chainstates[1]->m_chain.Height(), 220);
-            }
+        BOOST_CHECK_EQUAL(chainman_restarted.HistoricalChainstate(), nullptr);
-        }
     }
 }
 
@@ -633,37 +635,35 @@ 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");
 
-    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());
 
-    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()));
+    BOOST_CHECK(WITH_LOCK(::cs_main, return chainman.CurrentChainstate().m_assumeutxo == Assumeutxo::VALIDATED));
-    BOOST_CHECK(chainman.IsSnapshotActive());
+    BOOST_CHECK(WITH_LOCK(::cs_main, return chainman.CurrentChainstate().m_from_snapshot_blockhash));
+    BOOST_CHECK_EQUAL(WITH_LOCK(chainman.GetMutex(), return chainman.HistoricalChainstate()), nullptr);
 
     // Cache should have been rebalanced and reallocated to the "only" remaining
     // chainstate.
     BOOST_CHECK(active_cs.m_coinstip_cache_size_bytes > tip_cache_before_complete);
     BOOST_CHECK(active_cs.m_coinsdb_cache_size_bytes > db_cache_before_complete);
 
-    auto all_chainstates = chainman.GetAll();
-    BOOST_CHECK_EQUAL(all_chainstates.size(), 1);
-    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.
@@ -691,9 +691,8 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup
 
     {
         LOCK(chainman_restarted.GetMutex());
-        BOOST_CHECK_EQUAL(chainman_restarted.GetAll().size(), 1);
+        BOOST_CHECK_EQUAL(chainman_restarted.m_chainstates.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);
 
@@ -714,6 +713,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;
@@ -734,14 +734,18 @@ 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);
     }
 
-    auto all_chainstates = chainman.GetAll();
+    {
-    BOOST_CHECK_EQUAL(all_chainstates.size(), 1);
+        LOCK(chainman.GetMutex());
-    BOOST_CHECK_EQUAL(all_chainstates[0], &validation_chainstate);
+        BOOST_CHECK_EQUAL(chainman.m_chainstates.size(), 2);
-    BOOST_CHECK_EQUAL(&chainman.ActiveChainstate(), &validation_chainstate);
+        BOOST_CHECK(chainman.m_chainstates[0]->m_assumeutxo == Assumeutxo::VALIDATED);
+        BOOST_CHECK(!chainman.m_chainstates[0]->SnapshotBase());
+        BOOST_CHECK(chainman.m_chainstates[1]->m_assumeutxo == Assumeutxo::INVALID);
+        BOOST_CHECK(chainman.m_chainstates[1]->SnapshotBase());
+    }
 
     fs::path snapshot_invalid_dir = gArgs.GetDataDirNet() / "chainstate_snapshot_INVALID";
     BOOST_CHECK(fs::exists(snapshot_invalid_dir));
@@ -762,9 +766,8 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion_hash_mismatch, Sna
 
     {
         LOCK(::cs_main);
-        BOOST_CHECK_EQUAL(chainman_restarted.GetAll().size(), 1);
+        BOOST_CHECK_EQUAL(chainman_restarted.m_chainstates.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/test/validationinterface_tests.cpp

@@ -8,7 +8,6 @@
 #include <scheduler.h>
 #include <test/util/setup_common.h>
 #include <util/check.h>
-#include <kernel/chain.h>
 #include <validationinterface.h>
 
 #include <atomic>

src/txdb.h

@@ -53,9 +53,6 @@ public:
 
     //! Dynamically alter the underlying leveldb cache size.
     void ResizeCache(size_t new_cache_size) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
-
-    //! @returns filesystem path to on-disk storage or std::nullopt if in memory.
-    std::optional<fs::path> StoragePath() { return m_db->StoragePath(); }
 };
 
 #endif // BITCOIN_TXDB_H

src/validation.h

@@ -58,6 +58,9 @@ class DisconnectedBlockTransactions;
 struct PrecomputedTransactionData;
 struct LockPoints;
 struct AssumeutxoData;
+namespace kernel {
+struct ChainstateRole;
+} // namespace kernel
 namespace node {
 class SnapshotMetadata;
 } // namespace node
@@ -514,6 +517,16 @@ constexpr int64_t LargeCoinsCacheThreshold(int64_t total_space) noexcept
                     total_space - MAX_BLOCK_COINSDB_USAGE_BYTES);
 }
 
+//! Chainstate assumeutxo validity.
+enum class Assumeutxo {
+    //! Every block in the chain has been validated.
+    VALIDATED,
+    //! Blocks after an assumeutxo snapshot have been validated but the snapshot itself has not been validated.
+    UNVALIDATED,
+    //! The assumeutxo snapshot failed validation.
+    INVALID,
+};
+
 /**
  * Chainstate stores and provides an API to update our local knowledge of the
  * current best chain.
@@ -545,22 +558,12 @@ protected:
     //! Manages the UTXO set, which is a reflection of the contents of `m_chain`.
     std::unique_ptr<CoinsViews> m_coins_views;
 
-    //! This toggle exists for use when doing background validation for UTXO
-    //! snapshots.
-    //!
-    //! In the expected case, it is set once the background validation chain reaches the
-    //! same height as the base of the snapshot and its UTXO set is found to hash to
-    //! the expected assumeutxo value. It signals that we should no longer connect
-    //! blocks to the background chainstate. When set on the background validation
-    //! chainstate, it signifies that we have fully validated the snapshot chainstate.
-    //!
-    //! In the unlikely case that the snapshot chainstate is found to be invalid, this
-    //! is set to true on the snapshot chainstate.
-    bool m_disabled GUARDED_BY(::cs_main) {false};
-
     //! Cached result of LookupBlockIndex(*m_from_snapshot_blockhash)
     mutable const CBlockIndex* m_cached_snapshot_base GUARDED_BY(::cs_main){nullptr};
 
+    //! Cached result of LookupBlockIndex(*m_target_blockhash)
+    mutable const CBlockIndex* m_cached_target_block GUARDED_BY(::cs_main){nullptr};
+
     std::optional<const char*> m_last_script_check_reason_logged GUARDED_BY(::cs_main){};
 
 public:
@@ -579,11 +582,14 @@ public:
         ChainstateManager& chainman,
         std::optional<uint256> from_snapshot_blockhash = std::nullopt);
 
+    //! Return path to chainstate leveldb directory.
+    fs::path StoragePath() const;
+
     //! Return the current role of the chainstate. See `ChainstateManager`
     //! documentation for a description of the different types of chainstates.
     //!
     //! @sa ChainstateRole
-    ChainstateRole GetRole() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+    kernel::ChainstateRole GetRole() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
     /**
      * Initialize the CoinsViews UTXO set database management data structures. The in-memory
@@ -594,8 +600,7 @@ public:
     void InitCoinsDB(
         size_t cache_size_bytes,
         bool in_memory,
-        bool should_wipe,
+        bool should_wipe);
-        fs::path leveldb_name = "chainstate");
 
     //! Initialize the in-memory coins cache (to be done after the health of the on-disk database
     //! is verified).
@@ -613,6 +618,11 @@ public:
     //! @see CChain, CBlockIndex.
     CChain m_chain;
 
+    //! Assumeutxo state indicating whether all blocks in the chain were
+    //! validated, or if the chainstate is based on an assumeutxo snapshot and
+    //! the snapshot has not been validated.
+    Assumeutxo m_assumeutxo GUARDED_BY(::cs_main);
+
     /**
      * The blockhash which is the base of the snapshot this chainstate was created from.
      *
@@ -620,6 +630,16 @@ public:
      */
     const std::optional<uint256> m_from_snapshot_blockhash;
 
+    //! Target block for this chainstate. If this is not set, chainstate will
+    //! target the most-work, valid block. If this is set, ChainstateManager
+    //! considers this a "historical" chainstate since it will only contain old
+    //! blocks up to the target block, not newer blocks.
+    std::optional<uint256> m_target_blockhash GUARDED_BY(::cs_main);
+
+    //! Hash of the UTXO set at the target block, computed when the chainstate
+    //! reaches the target block, and null before then.
+    std::optional<AssumeutxoHash> m_target_utxohash GUARDED_BY(::cs_main);
+
     /**
      * The base of the snapshot this chainstate was created from.
      *
@@ -627,6 +647,26 @@ public:
      */
     const CBlockIndex* SnapshotBase() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
+    //! Return target block which chainstate tip is expected to reach, if this
+    //! is a historic chainstate being used to validate a snapshot, or null if
+    //! chainstate targets the most-work block.
+    const CBlockIndex* TargetBlock() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+    //! Set target block for this chainstate. If null, chainstate will target
+    //! the most-work valid block. If non-null chainstate will be a historic
+    //! chainstate and target the specified block.
+    void SetTargetBlock(CBlockIndex* block) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+    //! Set target block for this chainstate using just a block hash. Useful
+    //! when the block database has not been loaded yet.
+    void SetTargetBlockHash(uint256 block_hash) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+
+    //! Return true if chainstate reached target block.
+    bool ReachedTarget() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
+    {
+        const CBlockIndex* target_block{TargetBlock()};
+        assert(!target_block || target_block->GetAncestor(m_chain.Height()) == m_chain.Tip());
+        return target_block && target_block == m_chain.Tip();
+    }
+
     /**
      * The set of all CBlockIndex entries that have as much work as our current
      * tip or more, and transaction data needed to be validated (with
@@ -668,9 +708,6 @@ public:
     //! Destructs all objects related to accessing the UTXO set.
     void ResetCoinsViews() { m_coins_views.reset(); }
 
-    //! Does this chainstate have a UTXO set attached?
-    bool HasCoinsViews() const { return (bool)m_coins_views; }
-
     //! The cache size of the on-disk coins view.
     size_t m_coinsdb_cache_size_bytes{0};
 
@@ -719,9 +756,9 @@ public:
      * validationinterface callback.
      *
      * Note that if this is called while a snapshot chainstate is active, and if
-     * it is called on a background chainstate whose tip has reached the base block
+     * it is called on a validated chainstate whose tip has reached the base
-     * of the snapshot, its execution will take *MINUTES* while it hashes the
+     * block of the snapshot, its execution will take *MINUTES* while it hashes
-     * background UTXO set to verify the assumeutxo value the snapshot was activated
+     * the UTXO set to verify the assumeutxo value the snapshot was activated
      * with. `cs_main` will be held during this time.
      *
      * @returns true unless a system error occurred
@@ -798,6 +835,11 @@ public:
         return m_mempool ? &m_mempool->cs : nullptr;
     }
 
+    //! Return the [start, end] (inclusive) of block heights we can prune.
+    //!
+    //! start > end is possible, meaning no blocks can be pruned.
+    std::pair<int, int> GetPruneRange(int last_height_can_prune) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+
 protected:
     bool ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs);
     bool ConnectTip(
@@ -855,83 +897,38 @@ enum class SnapshotCompletionResult {
     // base block.
     MISSING_CHAINPARAMS,
 
-    // Failed to generate UTXO statistics (to check UTXO set hash) for the background
+    // Failed to generate UTXO statistics (to check UTXO set hash) for the
-    // chainstate.
+    // validated chainstate.
     STATS_FAILED,
 
-    // The UTXO set hash of the background validation chainstate does not match
+    // The UTXO set hash of the validated chainstate does not match the one
-    // the one expected by assumeutxo chainparams.
+    // expected by assumeutxo chainparams.
     HASH_MISMATCH,
-
-    // The blockhash of the current tip of the background validation chainstate does
-    // not match the one expected by the snapshot chainstate.
-    BASE_BLOCKHASH_MISMATCH,
 };
 
 /**
- * Provides an interface for creating and interacting with one or two
+ * Interface for managing multiple \ref Chainstate objects, where each
- * chainstates: an IBD chainstate generated by downloading blocks, and
+ * chainstate is associated with chainstate* subdirectory in the data directory
- * an optional snapshot chainstate loaded from a UTXO snapshot. Managed
+ * and contains a database of UTXOs existing at a different point in history.
- * chainstates can be maintained at different heights simultaneously.
+ * (See \ref Chainstate class for more information.)
  *
- * This class provides abstractions that allow the retrieval of the current
+ * Normally there is exactly one Chainstate, which contains the UTXO set of
- * most-work chainstate ("Active") as well as chainstates which may be in
+ * chain tip if syncing is completed, or the UTXO set the most recent validated
- * background use to validate UTXO snapshots.
+ * block if the initial sync is still in progress.
  *
- * Definitions:
+ * However, if an assumeutxo snapshot is loaded before syncing is completed,
- *
+ * there will be two chainstates. The original fully validated chainstate will
- * *IBD chainstate*: a chainstate whose current state has been "fully"
+ * continue to exist and download new blocks in the background. But the new
- *   validated by the initial block download process.
+ * snapshot which is loaded will become a second chainstate. The second
- *
+ * chainstate will be used as the chain tip for the wallet and RPCs even though
- * *Snapshot chainstate*: a chainstate populated by loading in an
+ * it is only assumed to be valid. When the initial chainstate catches up to the
- *    assumeutxo UTXO snapshot.
+ * snapshot height and confirms that the assumeutxo snapshot is actually valid,
- *
+ * the second chainstate will be marked validated and become the only chainstate
- * *Active chainstate*: the chainstate containing the current most-work
+ * again.
- *    chain. Consulted by most parts of the system (net_processing,
- *    wallet) as a reflection of the current chain and UTXO set.
- *    This may either be an IBD chainstate or a snapshot chainstate.
- *
- * *Background IBD chainstate*: an IBD chainstate for which the
- *    IBD process is happening in the background while use of the
- *    active (snapshot) chainstate allows the rest of the system to function.
  */
 class ChainstateManager
 {
 private:
-    //! The chainstate used under normal operation (i.e. "regular" IBD) or, if
-    //! a snapshot is in use, for background validation.
-    //!
-    //! Its contents (including on-disk data) will be deleted *upon shutdown*
-    //! after background validation of the snapshot has completed. We do not
-    //! free the chainstate contents immediately after it finishes validation
-    //! to cautiously avoid a case where some other part of the system is still
-    //! using this pointer (e.g. net_processing).
-    //!
-    //! Once this pointer is set to a corresponding chainstate, it will not
-    //! be reset until init.cpp:Shutdown().
-    //!
-    //! It is important for the pointer to not be deleted until shutdown,
-    //! because cs_main is not always held when the pointer is accessed, for
-    //! example when calling ActivateBestChain, so there's no way you could
-    //! prevent code from using the pointer while deleting it.
-    std::unique_ptr<Chainstate> m_ibd_chainstate GUARDED_BY(::cs_main);
-
-    //! A chainstate initialized on the basis of a UTXO snapshot. If this is
-    //! non-null, it is always our active chainstate.
-    //!
-    //! Once this pointer is set to a corresponding chainstate, it will not
-    //! be reset until init.cpp:Shutdown().
-    //!
-    //! It is important for the pointer to not be deleted until shutdown,
-    //! because cs_main is not always held when the pointer is accessed, for
-    //! example when calling ActivateBestChain, so there's no way you could
-    //! prevent code from using the pointer while deleting it.
-    std::unique_ptr<Chainstate> m_snapshot_chainstate GUARDED_BY(::cs_main);
-
-    //! Points to either the ibd or snapshot chainstate; indicates our
-    //! most-work chain.
-    Chainstate* m_active_chainstate GUARDED_BY(::cs_main) {nullptr};
-
     CBlockIndex* m_best_invalid GUARDED_BY(::cs_main){nullptr};
 
     /** The last header for which a headerTip notification was issued. */
@@ -968,15 +965,6 @@ private:
     /** Most recent headers presync progress update, for rate-limiting. */
     MockableSteadyClock::time_point m_last_presync_update GUARDED_BY(GetMutex()){};
 
-    //! Return true if a chainstate is considered usable.
-    //!
-    //! This is false when a background validation chainstate has completed its
-    //! validation of an assumed-valid chainstate, or when a snapshot
-    //! chainstate has been found to be invalid.
-    bool IsUsable(const Chainstate* const cs) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
-        return cs && !cs->m_disabled;
-    }
-
     //! A queue for script verifications that have to be performed by worker threads.
     CCheckQueue<CScriptCheck> m_script_check_queue;
 
@@ -1091,9 +1079,6 @@ public:
     //                                  constructor
     Chainstate& InitializeChainstate(CTxMemPool* mempool) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
-    //! Get all chainstates currently being used.
-    std::vector<Chainstate*> GetAll();
-
     //! Construct and activate a Chainstate on the basis of UTXO snapshot data.
     //!
     //! Steps:
@@ -1104,38 +1089,71 @@ public:
     //!   per assumeutxo chain parameters.
     //! - Wait for our headers chain to include the base block of the snapshot.
     //! - "Fast forward" the tip of the new chainstate to the base of the snapshot.
-    //! - Move the new chainstate to `m_snapshot_chainstate` and make it our
+    //! - Construct the new Chainstate and add it to m_chainstates.
-    //!   ChainstateActive().
     [[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.
+    //! Return current chainstate targeting the most-work, network tip.
-    const CBlockIndex* GetSnapshotBaseBlock() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+    Chainstate& CurrentChainstate() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex())
+    {
+        for (auto& cs : m_chainstates) {
+            if (cs && cs->m_assumeutxo != Assumeutxo::INVALID && !cs->m_target_blockhash) return *cs;
+        }
+        abort();
+    }
 
-    //! The most-work chain.
+    //! Return historical chainstate targeting a specific block, if any.
+    Chainstate* HistoricalChainstate() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex())
+    {
+        for (auto& cs : m_chainstates) {
+            if (cs && cs->m_assumeutxo != Assumeutxo::INVALID && cs->m_target_blockhash && !cs->m_target_utxohash) return cs.get();
+        }
+        return nullptr;
+    }
+
+    //! Return fully validated chainstate that should be used for indexing, to
+    //! support indexes that need to index blocks in order and can't start from
+    //! the snapshot block.
+    Chainstate& ValidatedChainstate() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex())
+    {
+        for (auto* cs : {&CurrentChainstate(), HistoricalChainstate()}) {
+            if (cs && cs->m_assumeutxo == Assumeutxo::VALIDATED) return *cs;
+        }
+        abort();
+    }
+
+    //! Remove a chainstate.
+    std::unique_ptr<Chainstate> RemoveChainstate(Chainstate& chainstate) EXCLUSIVE_LOCKS_REQUIRED(GetMutex())
+    {
+        auto it{std::find_if(m_chainstates.begin(), m_chainstates.end(), [&](auto& cs) { return cs.get() == &chainstate; })};
+        if (it != m_chainstates.end()) {
+            auto ret{std::move(*it)};
+            m_chainstates.erase(it);
+            return ret;
+        }
+        return nullptr;
+    }
+
+    //! Alternatives to CurrentChainstate() used by older code to query latest
+    //! chainstate information without locking cs_main. Newer code should avoid
+    //! querying ChainstateManager and use Chainstate objects directly, or
+    //! should use CurrentChainstate() instead.
+    //! @{
     Chainstate& ActiveChainstate() const;
     CChain& ActiveChain() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) { return ActiveChainstate().m_chain; }
     int ActiveHeight() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) { return ActiveChain().Height(); }
     CBlockIndex* ActiveTip() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) { return ActiveChain().Tip(); }
-
+    //! @}
-    //! The state of a background sync (for net processing)
-    bool BackgroundSyncInProgress() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) {
-        return IsUsable(m_snapshot_chainstate.get()) && IsUsable(m_ibd_chainstate.get());
-    }
-
-    //! The tip of the background sync chain
-    const CBlockIndex* GetBackgroundSyncTip() const EXCLUSIVE_LOCKS_REQUIRED(GetMutex()) {
-        return BackgroundSyncInProgress() ? m_ibd_chainstate->m_chain.Tip() : nullptr;
-    }
 
     node::BlockMap& BlockIndex() EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
     {
@@ -1148,18 +1166,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?
-    bool IsSnapshotValidated() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
-    {
-        return m_snapshot_chainstate && m_ibd_chainstate && m_ibd_chainstate->m_disabled;
-    }
-
     /** Check whether we are doing an initial block download (synchronizing from disk or network) */
     bool IsInitialBlockDownload() const;
 
@@ -1293,18 +1299,18 @@ 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);
 
     void ResetChainstates() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
-    //! Remove the snapshot-based chainstate and all on-disk artifacts.
+    //! Remove the chainstate and all on-disk artifacts.
     //! Used when reindex{-chainstate} is called during snapshot use.
-    [[nodiscard]] bool DeleteSnapshotChainstate() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+    [[nodiscard]] bool DeleteChainstate(Chainstate& chainstate) 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
@@ -1315,27 +1321,13 @@ public:
     //! directories are moved or deleted.
     //!
     //! @sa node/chainstate:LoadChainstate()
-    bool ValidatedSnapshotCleanup() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+    bool ValidatedSnapshotCleanup(Chainstate& validated_cs, Chainstate& unvalidated_cs) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
-    //! @returns the chainstate that indexes should consult when ensuring that an
+    //! Get range of historical blocks to download.
-    //!   index is synced with a chain where we can expect block index entries to have
+    std::optional<std::pair<const CBlockIndex*, const CBlockIndex*>> GetHistoricalBlockRange() const EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
-    //!   BLOCK_HAVE_DATA beneath the tip.
-    //!
-    //!   In other words, give us the chainstate for which we can reasonably expect
-    //!   that all blocks beneath the tip have been indexed. In practice this means
-    //!   when using an assumed-valid chainstate based upon a snapshot, return only the
-    //!   fully validated chain.
-    Chainstate& GetChainstateForIndexing() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
-    //! Return the [start, end] (inclusive) of block heights we can prune.
+    //! Call ActivateBestChain() on every chainstate.
-    //!
+    util::Result<void> ActivateBestChains() LOCKS_EXCLUDED(::cs_main);
-    //! start > end is possible, meaning no blocks can be pruned.
-    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);
 
     //! If, due to invalidation / reconsideration of blocks, the previous
     //! best header is no longer valid / guaranteed to be the most-work
@@ -1345,6 +1337,15 @@ public:
     CCheckQueue<CScriptCheck>& GetCheckQueue() { return m_script_check_queue; }
 
     ~ChainstateManager();
+
+    //! List of chainstates. Note: in general, it is not safe to delete
+    //! Chainstate objects once they are added to this list because there is no
+    //! mutex that can be locked to prevent Chainstate pointers from being used
+    //! while they are deleted. (cs_main doesn't work because it is too narrow
+    //! and is released in the middle of Chainstate::ActivateBestChain to let
+    //! notifications be processed. m_chainstate_mutex doesn't work because it
+    //! is not locked at other times when the chainstate is in use.)
+    std::vector<std::unique_ptr<Chainstate>> m_chainstates GUARDED_BY(::cs_main);
 };
 
 /** Deployment* info via ChainstateManager */

src/validationinterface.cpp

@@ -7,9 +7,9 @@
 
 #include <chain.h>
 #include <consensus/validation.h>
-#include <kernel/chain.h>
 #include <kernel/mempool_entry.h>
 #include <kernel/mempool_removal_reason.h>
+#include <kernel/types.h>
 #include <logging.h>
 #include <primitives/block.h>
 #include <primitives/transaction.h>
@@ -21,6 +21,8 @@
 #include <unordered_map>
 #include <utility>
 
+using kernel::ChainstateRole;
+
 /**
  * ValidationSignalsImpl manages a list of shared_ptr<CValidationInterface> callbacks.
  *
@@ -209,7 +211,8 @@ void ValidationSignals::TransactionRemovedFromMempool(const CTransactionRef& tx,
                           RemovalReasonToString(reason));
 }
 
-void ValidationSignals::BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock> &pblock, const CBlockIndex *pindex) {
+void ValidationSignals::BlockConnected(const ChainstateRole& role, const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindex)
+{
     auto event = [role, pblock, pindex, this] {
         m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.BlockConnected(role, pblock, pindex); });
     };
@@ -238,7 +241,8 @@ void ValidationSignals::BlockDisconnected(const std::shared_ptr<const CBlock>& p
                           pindex->nHeight);
 }
 
-void ValidationSignals::ChainStateFlushed(ChainstateRole role, const CBlockLocator &locator) {
+void ValidationSignals::ChainStateFlushed(const ChainstateRole& role, const CBlockLocator& locator)
+{
     auto event = [role, locator, this] {
         m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.ChainStateFlushed(role, locator); });
     };

src/validationinterface.h

@@ -6,7 +6,6 @@
 #ifndef BITCOIN_VALIDATIONINTERFACE_H
 #define BITCOIN_VALIDATIONINTERFACE_H
 
-#include <kernel/chain.h>
 #include <kernel/cs_main.h>
 #include <primitives/transaction.h>
 #include <sync.h>
@@ -17,6 +16,9 @@
 #include <memory>
 #include <vector>
 
+namespace kernel {
+struct ChainstateRole;
+} // namespace kernel
 namespace util {
 class TaskRunnerInterface;
 } // namespace util
@@ -118,7 +120,7 @@ protected:
      *
      * Called on a background thread.
      */
-    virtual void BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock> &block, const CBlockIndex *pindex) {}
+    virtual void BlockConnected(const kernel::ChainstateRole& role, const std::shared_ptr<const CBlock>& block, const CBlockIndex* pindex) {}
     /**
      * Notifies listeners of a block being disconnected
      * Provides the block that was disconnected.
@@ -143,7 +145,7 @@ protected:
      *
      * Called on a background thread.
      */
-    virtual void ChainStateFlushed(ChainstateRole role, const CBlockLocator &locator) {}
+    virtual void ChainStateFlushed(const kernel::ChainstateRole& role, const CBlockLocator& locator) {}
     /**
      * Notifies listeners of a block validation result.
      * If the provided BlockValidationState IsValid, the provided block
@@ -221,9 +223,9 @@ public:
     void TransactionAddedToMempool(const NewMempoolTransactionInfo&, uint64_t mempool_sequence);
     void TransactionRemovedFromMempool(const CTransactionRef&, MemPoolRemovalReason, uint64_t mempool_sequence);
     void MempoolTransactionsRemovedForBlock(const std::vector<RemovedMempoolTransactionInfo>&, unsigned int nBlockHeight);
-    void BlockConnected(ChainstateRole, const std::shared_ptr<const CBlock> &, const CBlockIndex *pindex);
+    void BlockConnected(const kernel::ChainstateRole&, const std::shared_ptr<const CBlock>&, const CBlockIndex* pindex);
     void BlockDisconnected(const std::shared_ptr<const CBlock> &, const CBlockIndex* pindex);
-    void ChainStateFlushed(ChainstateRole, const CBlockLocator &);
+    void ChainStateFlushed(const kernel::ChainstateRole&, const CBlockLocator&);
     void BlockChecked(const std::shared_ptr<const CBlock>&, const BlockValidationState&);
     void NewPoWValidBlock(const CBlockIndex *, const std::shared_ptr<const CBlock>&);
 };

src/wallet/wallet.cpp

@@ -22,8 +22,8 @@
 #include <interfaces/chain.h>
 #include <interfaces/handler.h>
 #include <interfaces/wallet.h>
-#include <kernel/chain.h>
 #include <kernel/mempool_removal_reason.h>
+#include <kernel/types.h>
 #include <key.h>
 #include <key_io.h>
 #include <logging.h>
@@ -87,6 +87,7 @@ using common::AmountErrMsg;
 using common::AmountHighWarn;
 using common::PSBTError;
 using interfaces::FoundBlock;
+using kernel::ChainstateRole;
 using util::ReplaceAll;
 using util::ToString;
 
@@ -1477,9 +1478,9 @@ void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRe
     }
 }
 
-void CWallet::blockConnected(ChainstateRole role, const interfaces::BlockInfo& block)
+void CWallet::blockConnected(const ChainstateRole& role, const interfaces::BlockInfo& block)
 {
-    if (role == ChainstateRole::BACKGROUND) {
+    if (role.historical) {
         return;
     }
     assert(block.data);

src/wallet/wallet.h

@@ -629,7 +629,7 @@ public:
     CWalletTx* AddToWallet(CTransactionRef tx, const TxState& state, const UpdateWalletTxFn& update_wtx=nullptr, bool rescanning_old_block = false);
     bool LoadToWallet(const Txid& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
     void transactionAddedToMempool(const CTransactionRef& tx) override;
-    void blockConnected(ChainstateRole role, const interfaces::BlockInfo& block) override;
+    void blockConnected(const kernel::ChainstateRole& role, const interfaces::BlockInfo& block) override;
     void blockDisconnected(const interfaces::BlockInfo& block) override;
     void updatedBlockTip() override;
     int64_t RescanFromTime(int64_t startTime, const WalletRescanReserver& reserver, bool update);

src/zmq/zmqnotificationinterface.cpp

@@ -5,8 +5,8 @@
 #include <zmq/zmqnotificationinterface.h>
 
 #include <common/args.h>
-#include <kernel/chain.h>
 #include <kernel/mempool_entry.h>
+#include <kernel/types.h>
 #include <logging.h>
 #include <netbase.h>
 #include <primitives/block.h>
@@ -24,6 +24,8 @@
 #include <utility>
 #include <vector>
 
+using kernel::ChainstateRole;
+
 CZMQNotificationInterface::CZMQNotificationInterface() = default;
 
 CZMQNotificationInterface::~CZMQNotificationInterface()
@@ -176,9 +178,9 @@ void CZMQNotificationInterface::TransactionRemovedFromMempool(const CTransaction
     });
 }
 
-void CZMQNotificationInterface::BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected)
+void CZMQNotificationInterface::BlockConnected(const ChainstateRole& role, const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected)
 {
-    if (role == ChainstateRole::BACKGROUND) {
+    if (role.historical) {
         return;
     }
     for (const CTransactionRef& ptx : pblock->vtx) {

src/zmq/zmqnotificationinterface.h

@@ -35,7 +35,7 @@ protected:
     // CValidationInterface
     void TransactionAddedToMempool(const NewMempoolTransactionInfo& tx, uint64_t mempool_sequence) override;
     void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) override;
-    void BlockConnected(ChainstateRole role, const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected) override;
+    void BlockConnected(const kernel::ChainstateRole& role, const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexConnected) override;
     void BlockDisconnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindexDisconnected) override;
     void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override;