backport: assumeutxo M3 — crash-safe background completion (bitcoin#25740, #27862, #28050, #27746)

ci/dash/lint-tidy.sh

@@ -90,6 +90,7 @@ iwyu_tool.py \
   "src/init" \
   "src/kernel" \
   "src/node/chainstate.cpp" \
+  "src/node/mempool_args.cpp" \
   "src/node/minisketchwrapper.cpp" \
   "src/policy/feerate.cpp" \
   "src/policy/packages.cpp" \

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/Makefile.am

@@ -324,7 +324,6 @@ BITCOIN_CORE_H = \
   masternode/payments.h \
   masternode/sync.h \
   masternode/utils.h \
-  mempool_args.h \
   memusage.h \
   merkleblock.h \
   messagesigner.h \
@@ -346,6 +345,7 @@ BITCOIN_CORE_H = \
   node/context.h \
   node/eviction.h \
   node/interface_ui.h \
+  node/mempool_args.h \
   node/mempool_persist_args.h \
   node/miner.h \
   node/minisketchwrapper.h \
@@ -600,7 +600,6 @@ libbitcoin_node_a_SOURCES = \
   masternode/payments.cpp \
   masternode/sync.cpp \
   masternode/utils.cpp \
-  mempool_args.cpp \
   net.cpp \
   net_processing.cpp \
   netfulfilledman.cpp \
@@ -614,6 +613,7 @@ libbitcoin_node_a_SOURCES = \
   node/eviction.cpp \
   node/interface_ui.cpp \
   node/interfaces.cpp \
+  node/mempool_args.cpp \
   node/mempool_persist_args.cpp \
   node/miner.cpp \
   node/minisketchwrapper.cpp \

src/Makefile.bench.include

@@ -31,6 +31,7 @@ bench_bench_dash_SOURCES = \
   bench/crypto_hash.cpp \
   bench/data.cpp \
   bench/data.h \
+  bench/descriptors.cpp \
   bench/duplicate_inputs.cpp \
   bench/ecdsa.cpp \
   bench/ellswift.cpp \

src/Makefile.test_fuzz.include

@@ -12,7 +12,7 @@ TEST_FUZZ_H = \
     test/fuzz/FuzzedDataProvider.h \
     test/fuzz/util.h \
     test/util/mining.h \
-    test/fuzz/mempool_utils.h \
+    test/fuzz/util/mempool.h \
     test/fuzz/util/net.h
 
 libtest_fuzz_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BOOST_CPPFLAGS)
@@ -21,5 +21,6 @@ libtest_fuzz_a_SOURCES = \
   test/fuzz/fuzz.cpp \
   test/util/mining.cpp \
   test/fuzz/util.cpp \
+  test/fuzz/util/mempool.cpp \
   test/fuzz/util/net.cpp \
   $(TEST_FUZZ_H)

src/bench/descriptors.cpp

@@ -0,0 +1,30 @@
+// Copyright (c) 2019 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <bench/bench.h>
+#include <key.h>
+#include <script/descriptor.h>
+#include <script/standard.h>
+
+#include <string>
+#include <utility>
+
+static void ExpandDescriptor(benchmark::Bench& bench)
+{
+    const auto desc_str = "sh(multi(15,03669b8afcec803a0d323e9a17f3ea8e68e8abe5a278020a929adbec52421adbd0,0260b2003c386519fc9eadf2b5cf124dd8eea4c4e68d5e154050a9346ea98ce600,0362a74e399c39ed5593852a30147f2959b56bb827dfa3e60e464b02ccf87dc5e8,0261345b53de74a4d721ef877c255429961b7e43714171ac06168d7e08c542a8b8,02da72e8b46901a65d4374fe6315538d8f368557dda3a1dcf9ea903f3afe7314c8,0318c82dd0b53fd3a932d16e0ba9e278fcc937c582d5781be626ff16e201f72286,0297ccef1ef99f9d73dec9ad37476ddb232f1238aff877af19e72ba04493361009,02e502cfd5c3f972fe9a3e2a18827820638f96b6f347e54d63deb839011fd5765d,03e687710f0e3ebe81c1037074da939d409c0025f17eb86adb9427d28f0f7ae0e9,02c04d3a5274952acdbc76987f3184b346a483d43be40874624b29e3692c1df5af,02ed06e0f418b5b43a7ec01d1d7d27290fa15f75771cb69b642a51471c29c84acd,036d46073cbb9ffee90473f3da429abc8de7f8751199da44485682a989a4bebb24,02f5d1ff7c9029a80a4e36b9a5497027ef7f3e73384a4a94fbfe7c4e9164eec8bc,02e41deffd1b7cce11cde209a781adcffdabd1b91c0ba0375857a2bfd9302419f3,02d76625f7956a7fc505ab02556c23ee72d832f1bac391bcd2d3abce5710a13d06))";
+    const std::pair<int64_t, int64_t> range = {0, 1000};
+    FlatSigningProvider provider;
+    std::string error;
+    auto desc = Parse(desc_str, provider, error);
+
+    bench.run([&] {
+        for (int i = range.first; i <= range.second; ++i) {
+            std::vector<CScript> scripts;
+            bool success = desc->Expand(i, provider, scripts, provider);
+            assert(success);
+        }
+    });
+}
+
+BENCHMARK(ExpandDescriptor, benchmark::PriorityLevel::HIGH);

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.cpp

@@ -29,32 +29,33 @@ void CChain::SetTip(CBlockIndex& block)
     }
 }
 
-CBlockLocator CChain::GetLocator(const CBlockIndex *pindex) const {
-    int nStep = 1;
-    std::vector<uint256> vHave;
-    vHave.reserve(32);
-
-    if (!pindex)
-        pindex = Tip();
-    while (pindex) {
-        vHave.push_back(pindex->GetBlockHash());
-        // Stop when we have added the genesis block.
-        if (pindex->nHeight == 0)
-            break;
+std::vector<uint256> LocatorEntries(const CBlockIndex* index)
+{
+    int step = 1;
+    std::vector<uint256> have;
+    if (index == nullptr) return have;
+
+    have.reserve(32);
+    while (index) {
+        have.emplace_back(index->GetBlockHash());
+        if (index->nHeight == 0) break;
         // Exponentially larger steps back, plus the genesis block.
-        int nHeight = std::max(pindex->nHeight - nStep, 0);
-        if (Contains(pindex)) {
-            // Use O(1) CChain index if possible.
-            pindex = (*this)[nHeight];
-        } else {
-            // Otherwise, use O(log n) skiplist.
-            pindex = pindex->GetAncestor(nHeight);
-        }
-        if (vHave.size() > 10)
-            nStep *= 2;
+        int height = std::max(index->nHeight - step, 0);
+        // Use skiplist.
+        index = index->GetAncestor(height);
+        if (have.size() > 10) step *= 2;
     }
+    return have;
+}
 
-    return CBlockLocator(vHave);
+CBlockLocator GetLocator(const CBlockIndex* index)
+{
+    return CBlockLocator{LocatorEntries(index)};
+}
+
+CBlockLocator CChain::GetLocator() const
+{
+    return ::GetLocator(Tip());
 }
 
 const CBlockIndex *CChain::FindFork(const CBlockIndex *pindex) const {

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,
 };
@@ -478,8 +486,8 @@ class CChain
     /** Set/initialize a chain with a given tip. */
     void SetTip(CBlockIndex& block);
 
-    /** Return a CBlockLocator that refers to a block in this chain (by default the tip). */
-    CBlockLocator GetLocator(const CBlockIndex* pindex = nullptr) const;
+    /** Return a CBlockLocator that refers to the tip in of this chain. */
+    CBlockLocator GetLocator() const;
 
     /** Find the last common block between this chain and a block index entry. */
     const CBlockIndex* FindFork(const CBlockIndex* pindex) const;
@@ -488,4 +496,10 @@ class CChain
     CBlockIndex* FindEarliestAtLeast(int64_t nTime, int height) const;
 };
 
+/** Get a locator for a block index entry. */
+CBlockLocator GetLocator(const CBlockIndex* index);
+
+/** Construct a list of hash entries to put in a locator.  */
+std::vector<uint256> LocatorEntries(const CBlockIndex* index);
+
 #endif // BITCOIN_CHAIN_H

src/chainlock/handler.cpp

@@ -47,7 +47,7 @@ ChainlockHandler::ChainlockHandler(chainlock::Chainlocks& chainlocks, Chainstate
     scheduler{std::make_unique<CScheduler>()},
     scheduler_thread{
         std::make_unique<std::thread>(std::thread(util::TraceThread, "cl-schdlr", [&] { scheduler->serviceQueue(); }))},
-    seenChainLocks{SEEN_CHAINLOCKS_RETAINED_SIZE, SEEN_CHAINLOCKS_PRUNE_AFTER_SIZE}
+    seenChainLocks{SEEN_CHAINLOCKS_CUTOFF_SIZE, SEEN_CHAINLOCKS_MAX_SIZE}
 {
 }
 
@@ -89,18 +89,6 @@ size_t ChainlockHandler::SeenChainLockCacheSizeForTesting() const
     return seenChainLocks.size();
 }
 
-size_t ChainlockHandler::SeenChainLockCacheRetainedSizeForTesting() const
-{
-    LOCK(cs);
-    return seenChainLocks.max_size();
-}
-
-size_t ChainlockHandler::SeenChainLockCachePruneAfterSizeForTesting() const
-{
-    LOCK(cs);
-    return seenChainLocks.prune_after_size();
-}
-
 void ChainlockHandler::UpdateTxFirstSeenMap(const Uint256HashSet& tx, const int64_t& time)
 {
     AssertLockNotHeld(cs);

src/chainlock/handler.h

@@ -54,16 +54,8 @@ class ChainlockHandler final : public CValidationInterface
     std::atomic<bool> tryLockChainTipScheduled{false};
     std::atomic<bool> isEnabled{false};
 
-    //! Number of recently seen CLSIG hashes retained once `seenChainLocks` is pruned.
-    static constexpr size_t SEEN_CHAINLOCKS_RETAINED_SIZE{1024};
-    //! Size `seenChainLocks` may grow to before the next insertion prunes it back down to
-    //! SEEN_CHAINLOCKS_RETAINED_SIZE. Pruning partitions every entry, and CLSIG hashes are
-    //! recorded before the signature is verified, so pruning on each insertion past the retained
-    //! size lets a peer turn a stream of unique CLSIG hashes into a stream of full-map partition
-    //! passes under `cs`. Pruning only after twice the retained size amortises that cost over the
-    //! entries dropped in a single batch, at the price of a larger transient cache. The 2x ratio
-    //! matches the default in unordered_lru_cache.
-    static constexpr size_t SEEN_CHAINLOCKS_PRUNE_AFTER_SIZE{2 * SEEN_CHAINLOCKS_RETAINED_SIZE};
+    static constexpr size_t SEEN_CHAINLOCKS_CUTOFF_SIZE{1024};
+    static constexpr size_t SEEN_CHAINLOCKS_MAX_SIZE{2 * SEEN_CHAINLOCKS_CUTOFF_SIZE};
 
     const CBlockIndex* lastNotifyChainLockBlockIndex GUARDED_BY(cs){nullptr};
     Uint256HashMap<std::chrono::seconds> txFirstSeenTime GUARDED_BY(cs);
@@ -86,10 +78,6 @@ class ChainlockHandler final : public CValidationInterface
     bool AlreadyHave(const CInv& inv) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
     void UpdateTxFirstSeenMap(const Uint256HashSet& tx, const int64_t& time) EXCLUSIVE_LOCKS_REQUIRED(!cs);
     size_t SeenChainLockCacheSizeForTesting() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
-    //! Number of entries retained after the seen cache is pruned.
-    size_t SeenChainLockCacheRetainedSizeForTesting() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
-    //! Size the seen cache may grow to before it is pruned back to the retained size.
-    size_t SeenChainLockCachePruneAfterSizeForTesting() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
 
     [[nodiscard]] MessageProcessingResult ProcessNewChainLock(NodeId from, const chainlock::ChainLockSig& clsig,
                                                               const llmq::CQuorumManager& qman,