simplify ChainstateManager::SnapshotBlockhash() return semantics

Don't return null snapshotblockhash values to avoid caller complexity/confusion.

Author: jamesob

src/chain.h

@@ -163,14 +163,27 @@ class CBlockIndex
 
     //! Number of transactions in this block.
     //! Note: in a potential headers-first mode, this number cannot be relied upon
+    //! Note: this value is faked during UTXO snapshot load to ensure that
+    //! LoadBlockIndex() will load index entries for blocks that we lack data for.
+    //! @sa ActivateSnapshot
     unsigned int nTx{0};
 
     //! (memory only) Number of transactions in the chain up to and including this block.
     //! This value will be non-zero only if and only if transactions for this block and all its parents are available.
     //! Change to 64-bit type when necessary; won't happen before 2030
+    //!
+    //! Note: this value is faked during use of a UTXO snapshot because we don't
+    //! have the underlying block data available during snapshot load.
+    //! @sa AssumeutxoData
+    //! @sa ActivateSnapshot
     unsigned int nChainTx{0};
 
     //! Verification status of this block. See enum BlockStatus
+    //!
+    //! Note: this value is modified to show BLOCK_OPT_WITNESS during UTXO snapshot
+    //! load to avoid the block index being spuriously rewound.
+    //! @sa RewindBlockIndex
+    //! @sa ActivateSnapshot
     uint32_t nStatus{0};
 
     //! block header

src/coins.cpp

@@ -97,6 +97,14 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi
     cachedCoinsUsage += it->second.coin.DynamicMemoryUsage();
 }
 
+void CCoinsViewCache::EmplaceCoinInternalDANGER(COutPoint&& outpoint, Coin&& coin) {
+    cachedCoinsUsage += coin.DynamicMemoryUsage();
+    cacheCoins.emplace(
+        std::piecewise_construct,
+        std::forward_as_tuple(std::move(outpoint)),
+        std::forward_as_tuple(std::move(coin), CCoinsCacheEntry::DIRTY));
+}
+
 void AddCoins(CCoinsViewCache& cache, const CTransaction &tx, int nHeight, bool check_for_overwrite) {
     bool fCoinbase = tx.IsCoinBase();
     const uint256& txid = tx.GetHash();

src/coins.h

@@ -20,6 +20,8 @@
 #include <functional>
 #include <unordered_map>
 
+class ChainstateManager;
+
 /**
  * A UTXO entry.
  *
@@ -125,6 +127,7 @@ struct CCoinsCacheEntry
 
     CCoinsCacheEntry() : flags(0) {}
     explicit CCoinsCacheEntry(Coin&& coin_) : coin(std::move(coin_)), flags(0) {}
+    CCoinsCacheEntry(Coin&& coin_, unsigned char flag) : coin(std::move(coin_)), flags(flag) {}
 };
 
 typedef std::unordered_map<COutPoint, CCoinsCacheEntry, SaltedOutpointHasher> CCoinsMap;
@@ -262,6 +265,15 @@ class CCoinsViewCache : public CCoinsViewBacked
      */
     void AddCoin(const COutPoint& outpoint, Coin&& coin, bool possible_overwrite);
 
+    /**
+     * Emplace a coin into cacheCoins without performing any checks, marking
+     * the emplaced coin as dirty.
+     *
+     * NOT FOR GENERAL USE. Used only when loading coins from a UTXO snapshot.
+     * @sa ChainstateManager::PopulateAndValidateSnapshot()
+     */
+    void EmplaceCoinInternalDANGER(COutPoint&& outpoint, Coin&& coin);
+
     /**
      * Spend a coin. Pass moveto in order to get the deleted data.
      * If no unspent output exists for the passed outpoint, this call

src/test/validation_chainstatemanager_tests.cpp

@@ -28,6 +28,8 @@ BOOST_AUTO_TEST_CASE(chainstatemanager)
     std::vector<CChainState*> chainstates;
     const CChainParams& chainparams = Params();
 
+    BOOST_CHECK(!manager.SnapshotBlockhash().has_value());
+
     // Create a legacy (IBD) chainstate.
     //
     CChainState& c1 = WITH_LOCK(::cs_main, return manager.InitializeChainstate(mempool));
@@ -54,10 +56,17 @@ BOOST_AUTO_TEST_CASE(chainstatemanager)
     auto& validated_cs = manager.ValidatedChainstate();
     BOOST_CHECK_EQUAL(&validated_cs, &c1);
 
+    BOOST_CHECK(!manager.SnapshotBlockhash().has_value());
+
     // Create a snapshot-based chainstate.
     //
-    CChainState& c2 = WITH_LOCK(::cs_main, return manager.InitializeChainstate(mempool, GetRandHash()));
+    const uint256 snapshot_blockhash = GetRandHash();
+    CChainState& c2 = WITH_LOCK(::cs_main, return manager.InitializeChainstate(
+        mempool, snapshot_blockhash));
     chainstates.push_back(&c2);
+
+    BOOST_CHECK_EQUAL(manager.SnapshotBlockhash().value(), snapshot_blockhash);
+
     c2.InitCoinsDB(
         /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
     WITH_LOCK(::cs_main, c2.InitCoinsCache(1 << 23));