backport: assumeutxo M3 — background validation completion and snapshot promotion

doc/design/assumeutxo.md

@@ -3,9 +3,9 @@
 Assumeutxo is a feature that allows fast bootstrapping of a validating dashd
 instance with a very similar security model to assumevalid.
 
-The RPC commands `dumptxoutset` and `loadtxoutset` are used to respectively generate
-and load UTXO snapshots. The utility script `./contrib/devtools/utxo_snapshot.sh` may
-be of use.
+The RPC commands `dumptxoutset` and `loadtxoutset` (yet to be merged) are used to
+respectively generate and load UTXO snapshots. The utility script
+`./contrib/devtools/utxo_snapshot.sh` may be of use.
 
 ## General background
 
@@ -17,14 +17,9 @@ be of use.
 
 - A new block index `nStatus` flag is introduced, `BLOCK_ASSUMED_VALID`, to mark block
   index entries that are required to be assumed-valid by a chainstate created
-  from a UTXO snapshot. This flag is mostly used as a way to modify certain
+  from a UTXO snapshot. This flag is used as a way to modify certain
   CheckBlockIndex() logic to account for index entries that are pending validation by a
-  chainstate running asynchronously in the background. We also use this flag to control
-  which index entries are added to setBlockIndexCandidates during LoadBlockIndex().
-
-- Indexing implementations via BaseIndex can no longer assume that indexation happens
-  sequentially, since background validation chainstates can submit BlockConnected
-  events out of order with the active chain.
+  chainstate running asynchronously in the background.
 
 - The concept of UTXO snapshots is treated as an implementation detail that lives
   behind the ChainstateManager interface. The external presentation of the changes
@@ -76,9 +71,15 @@ original chainstate remains in use as active.
 
 Once the snapshot chainstate is loaded and validated, it is promoted to active
 chainstate and a sync to tip begins. A new chainstate directory is created in the
-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()`).
+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()`).
+
+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
+the snapshot chainstate on subsequent inits. Otherwise, the directory is a normal
+leveldb database.
 
 |    |    |
 | ---------- | ----------- |
@@ -88,7 +89,7 @@ active on node startup (via `DetectSnapshotChainstate()`).
 The snapshot begins to sync to tip from its base block, technically in parallel with
 the original chainstate, but it is given priority during block download and is
 allocated most of the cache (see `MaybeRebalanceCaches()` and usages) as our chief
-consideration is getting to network tip.
+goal is getting to network tip.
 
 **Failure consideration:** if shutdown happens at any point during this phase, both
 chainstates will be detected during the next init and the process will resume.
@@ -107,33 +108,36 @@ 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_stop_use` (not yet
-committed - see bitcoin#15606), in `CompleteSnapshotValidation()`, 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`.
-
-The background chainstate data lingers on disk until shutdown, when in
-`ChainstateManager::Reset()`, the background chainstate is cleaned up with
-`ValidatedSnapshotShutdownCleanup()`, which renames the `chainstate_[hash]` datadir as
-`chainstate`.
+chainstate, we stop use of the background chainstate by setting `m_disabled`, in
+`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`. In Dash, completion additionally compares the
+deterministic masternode-list hash the background chainstate derived at the base block
+against the hash recorded at snapshot activation, and the EvoDB best-block markers
+against both chainstates' coins tips; any divergence fails completion with
+`EVO_STATE_MISMATCH` and quarantines the snapshot exactly like a UTXO hash mismatch.
 
 |    |    |
 | ---------- | ----------- |
-| number of chainstates | 2 (ibd has `m_stop_use=true`) |
+| number of chainstates | 2 (ibd has `m_disabled=true`) |
 | active chainstate | snapshot |
 
-**Failure consideration:** if dashd unexpectedly halts after `m_stop_use` is set on
-the background chainstate but before `CompleteSnapshotValidation()` can finish, the
-need to complete snapshot validation will be detected on subsequent init by
-`ChainstateManager::CheckForUncleanShutdown()`.
+The background chainstate data lingers on disk until the program is restarted.
 
 ### Dashd restarts sometime after snapshot validation has completed
 
-When dashd initializes again, what began as the snapshot chainstate is now
-indistinguishable from a chainstate that has been built from the traditional IBD
-process, and will be initialized as such.
+After a shutdown and subsequent restart, `LoadChainstate()` cleans up the background
+chainstate with `ValidatedSnapshotCleanup()`, which renames the `chainstate_snapshot`
+datadir as `chainstate` and removes the now unnecessary background chainstate data.
 
 |    |    |
 | ---------- | ----------- |
 | number of chainstates | 1 |
-| active chainstate | ibd |
+| active chainstate | ibd (was snapshot, but is now fully validated) |
+
+What began as the snapshot chainstate is now indistinguishable from a chainstate that
+has been built from the traditional IBD process, and will be initialized as such.
+
+A file will be left in `chainstate/base_blockhash`, which indicates that the
+chainstate, even though now fully validated, was originally started from a snapshot
+with the corresponding base blockhash.

src/bench/load_external.cpp

@@ -48,14 +48,13 @@ static void LoadExternalBlockFile(benchmark::Bench& bench)
         fclose(file);
     }
 
-    Chainstate& chainstate{testing_setup->m_node.chainman->ActiveChainstate()};
     std::multimap<uint256, FlatFilePos> blocks_with_unknown_parent;
     FlatFilePos pos;
     bench.run([&] {
         // "rb" is "binary, O_RDONLY", positioned to the start of the file.
         // The file will be closed by LoadExternalBlockFile().
         FILE* file{fsbridge::fopen(blkfile, "rb")};
-        chainstate.LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent);
+        testing_setup->m_node.chainman->LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent);
     });
     fs::remove(blkfile);
 }

src/chain.h

@@ -98,10 +98,10 @@ enum BlockStatus : uint32_t {
     BLOCK_VALID_TRANSACTIONS =    3,
 
     //! Outputs do not overspend inputs, no double spends, coinbase output ok, no immature coinbase spends, BIP30.
-    //! Implies all parents are also at least CHAIN.
+    //! Implies all parents are either at least VALID_CHAIN, or are ASSUMED_VALID
     BLOCK_VALID_CHAIN        =    4,
 
-    //! Scripts & signatures ok. Implies all parents are also at least SCRIPTS.
+    //! Scripts & signatures ok. Implies all parents are either at least VALID_SCRIPTS, or are ASSUMED_VALID.
     BLOCK_VALID_SCRIPTS      =    5,
 
     //! All validity bits.
@@ -119,10 +119,18 @@ enum BlockStatus : uint32_t {
     BLOCK_CONFLICT_CHAINLOCK =   128, //!< conflicts with chainlock system
 
     /**
-     * If set, this indicates that the block index entry is assumed-valid.
-     * Certain diagnostics will be skipped in e.g. CheckBlockIndex().
-     * It almost certainly means that the block's full validation is pending
-     * on a background chainstate. See `doc/design/assumeutxo.md`.
+     * If ASSUMED_VALID is set, it means that this block has not been validated
+     * and has validity status less than VALID_SCRIPTS. Also that it may have
+     * descendant blocks with VALID_SCRIPTS set, because they can be validated
+     * based on an assumeutxo snapshot.
+     *
+     * When an assumeutxo snapshot is loaded, the ASSUMED_VALID flag is added to
+     * unvalidated blocks at the snapshot height and below. Then, as the background
+     * validation progresses, and these blocks are validated, the ASSUMED_VALID
+     * flags are removed. See `doc/design/assumeutxo.md` for details.
+     *
+     * This flag is only used to implement checks in CheckBlockIndex() and
+     * should not be used elsewhere.
      */
     BLOCK_ASSUMED_VALID      =   256,
 };

src/evo/chainhelper.cpp

@@ -7,14 +7,17 @@
 #include <chainlock/chainlock.h>
 #include <chainparams.h>
 #include <evo/creditpool.h>
+#include <evo/deterministicmns.h>
 #include <evo/mnhftx.h>
 #include <evo/specialtxman.h>
 #include <governance/superblock.h>
+#include <hash.h>
 #include <instantsend/instantsend.h>
 #include <instantsend/lock.h>
 #include <logging.h>
 #include <masternode/payments.h>
 #include <masternode/sync.h>
+#include <util/check.h>
 
 CChainstateHelper::CChainstateHelper(CEvoDB& evodb, CDeterministicMNManager& dmnman, const CMasternodeSync& mn_sync,
                                      llmq::CInstantSendManager& isman, llmq::CQuorumBlockProcessor& qblockman,
@@ -23,6 +26,7 @@ CChainstateHelper::CChainstateHelper(CEvoDB& evodb, CDeterministicMNManager& dmn
                                      const llmq::CQuorumManager& qman) :
     isman{isman},
     mn_sync{mn_sync},
+    m_dmnman{dmnman},
     credit_pool_manager{std::make_unique<CCreditPoolManager>(evodb, chainman)},
     m_chainlocks{chainlocks},
     ehf_manager{std::make_unique<CMNHFManager>(evodb, chainman)},
@@ -60,6 +64,11 @@ bool CChainstateHelper::HasChainLock(int nHeight, const uint256& blockHash) cons
 
 int32_t CChainstateHelper::GetBestChainLockHeight() const { return m_chainlocks.GetBestChainLockHeight(); }
 
+uint256 CChainstateHelper::GetDeterministicMNListHash(const CBlockIndex* pindex) const
+{
+    return SerializeHash(m_dmnman.GetListForBlock(Assert(pindex)));
+}
+
 /** Passthrough functions to CCreditPoolManager */
 CCreditPool CChainstateHelper::GetCreditPool(const CBlockIndex* const pindex)
 {

src/evo/chainhelper.h

@@ -42,6 +42,7 @@ class CChainstateHelper
 private:
     llmq::CInstantSendManager& isman;
     const CMasternodeSync& mn_sync;
+    CDeterministicMNManager& m_dmnman;
 
 public:
     const std::unique_ptr<CCreditPoolManager> credit_pool_manager;
@@ -69,6 +70,9 @@ class CChainstateHelper
     bool HasChainLock(int nHeight, const uint256& blockHash) const;
     int32_t GetBestChainLockHeight() const;
 
+    /** Return a canonical hash of the deterministic MN list derived at a block. */
+    uint256 GetDeterministicMNListHash(const CBlockIndex* pindex) const;
+
     /** Passthrough functions to CCreditPoolManager */
     CCreditPool GetCreditPool(const CBlockIndex* const pindex);
 

src/evo/deterministicmns.cpp

@@ -814,8 +814,8 @@ CDeterministicMNList CDeterministicMNManager::GetListForBlockInternal(gsl::not_n
                 // cached chain) bootstraps an empty list here and rebuilds via
                 // ProcessBlock from that point on.
                 throw BlockDataUnavailableError(strprintf(
-                    "CDeterministicMNManager::%s -- masternode list diff for block %s is not available (pruned or below an unvalidated snapshot base)",
-                    __func__, pindex->GetBlockHash().ToString()));
+                    "CDeterministicMNManager::%s -- masternode list diff for block %s %s",
+                    __func__, pindex->GetBlockHash().ToString(), BLOCK_DATA_UNAVAILABLE_SUFFIX));
             }
             // no snapshot and no diff on disk means that it's the initial snapshot
             m_initial_snapshot_index = pindex;

src/evo/deterministicmns.h

@@ -26,6 +26,7 @@
 #include <limits>
 #include <numeric>
 #include <stdexcept>
+#include <string_view>
 #include <unordered_map>
 #include <utility>
 #include <vector>
@@ -685,12 +686,21 @@ struct MNListUpdates
     CDeterministicMNListDiff diff;
 };
 
+/** Sentinel suffix carried by serving-failure messages that report locally
+ *  missing block data (pruned, or below an unvalidated snapshot base) rather
+ *  than peer misbehavior. Every producer must append this verbatim so that
+ *  IsBlockDataUnavailableError() keeps recognizing the condition; matching on
+ *  this constant is what keeps such requests from penalizing the peer. */
+inline constexpr std::string_view BLOCK_DATA_UNAVAILABLE_SUFFIX{
+    "is not available (pruned or below an unvalidated snapshot base)"};
+
 /** Thrown when the masternode list for a block cannot be reconstructed because
  *  the data is not on this node yet (pruned, or below an unvalidated snapshot
  *  base, or pending in another chainstate's unflushed EvoDB overlay). Distinct
  *  from the plain std::runtime_error that CDeterministicMNList::ApplyDiff
  *  raises for genuine local corruption, which must never be swallowed.
- *  The message carries the sentinel matched by IsBlockDataUnavailableError(). */
+ *  The message carries BLOCK_DATA_UNAVAILABLE_SUFFIX, matched by
+ *  IsBlockDataUnavailableError(). */
 class BlockDataUnavailableError : public std::runtime_error
 {
 public:
@@ -741,6 +751,12 @@ class CDeterministicMNManager
     };
     CDeterministicMNList GetListAtChainTip() EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
+    void SetListForBlockForTesting(const CDeterministicMNList& list) EXCLUSIVE_LOCKS_REQUIRED(!cs)
+    {
+        LOCK(cs);
+        mnListsCache.insert_or_assign(list.GetBlockHash(), list);
+    }
+
     // Test if given TX is a ProRegTx which also contains the collateral at index n
     static bool IsProTxWithCollateral(const CTransactionRef& tx, uint32_t n);
 

src/evo/evodb.cpp

@@ -6,6 +6,8 @@
 
 #include <uint256.h>
 
+#include <tuple>
+
 CEvoDBScopedCommitter::CEvoDBScopedCommitter(CEvoDB& _evoDB, EvoDbIdentity identity) :
     evoDB{_evoDB},
     identity{identity}
@@ -90,13 +92,13 @@ void CEvoDB::RollbackCurTransaction(EvoDbIdentity identity)
     active_transaction.reset();
 }
 
-bool CEvoDB::CommitRootTransaction(EvoDbIdentity identity)
+bool CEvoDB::CommitRootTransaction(EvoDbIdentity identity, bool sync)
 {
     LOCK(cs);
     auto& context = GetContext(identity);
     assert(context.cur_transaction.IsClean());
     context.root_transaction.Commit();
-    bool ret = db->WriteBatch(context.root_batch);
+    bool ret = db->WriteBatch(context.root_batch, sync);
     context.root_batch.Clear();
     return ret;
 }
@@ -146,5 +148,93 @@ void CEvoDB::EraseSnapshotMarkers()
     LOCK(cs);
     auto& transaction = GetContext(GetCurrentIdentity()).cur_transaction;
     transaction.Erase(std::make_pair(EVODB_BEST_BLOCK, uint8_t{1}));
+    transaction.Erase(EVODB_SNAPSHOT_MNLIST_HASH);
+    transaction.Erase(EVODB_BACKGROUND_MNLIST_HASH);
     transaction.Erase(EVODB_DUAL_CHAINSTATE);
 }
+
+void CEvoDB::WriteSnapshotBaseMNListHash(const uint256& hash)
+{
+    Write(EVODB_SNAPSHOT_MNLIST_HASH, hash);
+}
+
+bool CEvoDB::ReadSnapshotBaseMNListHash(uint256& hash)
+{
+    // Lifecycle markers are read at rest (completion after both identities'
+    // sync commits, or startup recovery). Read the raw DB so the result cannot
+    // depend on which transaction-less default identity happens to be current.
+    LOCK(cs);
+    return db->Read(EVODB_SNAPSHOT_MNLIST_HASH, hash);
+}
+
+void CEvoDB::WriteBackgroundMNListHash(const uint256& block_hash, const uint256& mn_list_hash)
+{
+    Write(EVODB_BACKGROUND_MNLIST_HASH, std::make_pair(block_hash, mn_list_hash));
+}
+
+bool CEvoDB::ReadBackgroundMNListHash(uint256& block_hash, uint256& mn_list_hash)
+{
+    // See ReadSnapshotBaseMNListHash: at-rest raw read, identity-independent.
+    LOCK(cs);
+    std::pair<uint256, uint256> value;
+    if (!db->Read(EVODB_BACKGROUND_MNLIST_HASH, value)) return false;
+    std::tie(block_hash, mn_list_hash) = value;
+    return true;
+}
+
+bool CEvoDB::PromoteSnapshotMarkers(const uint256& expected_snapshot_tip)
+{
+    LOCK(cs);
+    assert(!active_transaction.has_value());
+    for (const auto& [_, context] : transaction_contexts) {
+        if (!context) continue;
+        assert(context->cur_transaction.IsClean());
+        assert(context->root_transaction.IsClean());
+    }
+
+    const auto snapshot_key = std::make_pair(EVODB_BEST_BLOCK, uint8_t{1});
+    uint256 snapshot_tip;
+    if (!db->Read(snapshot_key, snapshot_tip)) {
+        uint256 normal_tip;
+        const bool already_promoted = db->Read(EVODB_BEST_BLOCK, normal_tip) && normal_tip == expected_snapshot_tip &&
+                                      !db->Exists(EVODB_DUAL_CHAINSTATE) && !db->Exists(EVODB_SNAPSHOT_MNLIST_HASH) &&
+                                      !db->Exists(EVODB_BACKGROUND_MNLIST_HASH);
+        if (already_promoted) m_default_identity = EvoDbIdentity::NORMAL;
+        return already_promoted;
+    }
+    if (snapshot_tip != expected_snapshot_tip) return false;
+
+    CDBBatch batch{*db};
+    batch.Write(EVODB_BEST_BLOCK, snapshot_tip);
+    batch.Erase(snapshot_key);
+    batch.Erase(EVODB_SNAPSHOT_MNLIST_HASH);
+    batch.Erase(EVODB_BACKGROUND_MNLIST_HASH);
+    batch.Erase(EVODB_DUAL_CHAINSTATE);
+    if (!db->WriteBatch(batch, /*fSync=*/true)) return false;
+    // The dual-chainstate run is over: the promoted state is the NORMAL
+    // identity, so transaction-less access must resolve there again.
+    m_default_identity = EvoDbIdentity::NORMAL;
+    return true;
+}
+
+bool CEvoDB::DiscardSnapshotMarkers()
+{
+    LOCK(cs);
+    assert(!active_transaction.has_value());
+    for (const auto& [_, context] : transaction_contexts) {
+        if (!context) continue;
+        assert(context->cur_transaction.IsClean());
+        assert(context->root_transaction.IsClean());
+    }
+
+    CDBBatch batch{*db};
+    batch.Erase(std::make_pair(EVODB_BEST_BLOCK, uint8_t{1}));
+    batch.Erase(EVODB_SNAPSHOT_MNLIST_HASH);
+    batch.Erase(EVODB_BACKGROUND_MNLIST_HASH);
+    batch.Erase(EVODB_DUAL_CHAINSTATE);
+    if (!db->WriteBatch(batch, /*fSync=*/true)) return false;
+    // The snapshot chainstate is gone; transaction-less access must resolve
+    // against the NORMAL identity again.
+    m_default_identity = EvoDbIdentity::NORMAL;
+    return true;
+}