fuzz: Speed up utxo_snapshot by lazy re-init

The re-init is expensive, so skip it if there is no need.

Also, add an even faster fuzz target utxo_snapshot_invalid, which does
not need any re-init at all.

3 files changed, 114 insertions, 36 deletions

diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp
index 7401a613b7..0a0896650f 100644
--- a/src/test/fuzz/utxo_snapshot.cpp
+++ b/src/test/fuzz/utxo_snapshot.cpp
@@ -1,45 +1,78 @@
-// Copyright (c) 2021-2022 The Bitcoin Core developers
+// Copyright (c) 2021-present 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 <chain.h>
 #include <chainparams.h>
+#include <coins.h>
+#include <consensus/consensus.h>
 #include <consensus/validation.h>
+#include <node/blockstorage.h>
 #include <node/utxo_snapshot.h>
+#include <primitives/block.h>
+#include <primitives/transaction.h>
+#include <serialize.h>
+#include <span.h>
+#include <streams.h>
+#include <sync.h>
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
 #include <test/fuzz/util.h>
 #include <test/util/mining.h>
 #include <test/util/setup_common.h>
-#include <util/chaintype.h>
+#include <uint256.h>
+#include <util/check.h>
 #include <util/fs.h>
+#include <util/result.h>
 #include <validation.h>
-#include <validationinterface.h>
+
+#include <cstdint>
+#include <functional>
+#include <ios>
+#include <memory>
+#include <optional>
+#include <vector>
 
 using node::SnapshotMetadata;
 
 namespace {
 
 const std::vector<std::shared_ptr<CBlock>>* g_chain;
+TestingSetup* g_setup;
 
+template <bool INVALID>
 void initialize_chain()
 {
     const auto params{CreateChainParams(ArgsManager{}, ChainType::REGTEST)};
     static const auto chain{CreateBlockChain(2 * COINBASE_MATURITY, *params)};
     g_chain = &chain;
+    static const auto setup{
+        MakeNoLogFileContext<TestingSetup>(ChainType::REGTEST,
+                                           TestOpts{
+                                               .setup_net = false,
+                                               .setup_validation_interface = false,
+                                           }),
+    };
+    if constexpr (INVALID) {
+        auto& chainman{*setup->m_node.chainman};
+        for (const auto& block : chain) {
+            BlockValidationState dummy;
+            bool processed{chainman.ProcessNewBlockHeaders({*block}, true, dummy)};
+            Assert(processed);
+            const auto* index{WITH_LOCK(::cs_main, return chainman.m_blockman.LookupBlockIndex(block->GetHash()))};
+            Assert(index);
+        }
+    }
+    g_setup = setup.get();
 }
 
-FUZZ_TARGET(utxo_snapshot, .init = initialize_chain)
+template <bool INVALID>
+void utxo_snapshot_fuzz(FuzzBufferType buffer)
 {
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
-    std::unique_ptr<const TestingSetup> setup{
-        MakeNoLogFileContext<const TestingSetup>(
-            ChainType::REGTEST,
-            TestOpts{
-                .setup_net = false,
-                .setup_validation_interface = false,
-            })};
-    const auto& node = setup->m_node;
-    auto& chainman{*node.chainman};
+    auto& setup{*g_setup};
+    bool dirty_chainman{false}; // Re-use the global chainman, but reset it when it is dirty
+    auto& chainman{*setup.m_node.chainman};
 
     const auto snapshot_path = gArgs.GetDataDirNet() / "fuzzed_snapshot.dat";
 
@@ -74,6 +107,16 @@ FUZZ_TARGET(utxo_snapshot, .init = initialize_chain)
                 height++;
             }
         }
+        if constexpr (INVALID) {
+            // Append an invalid coin to ensure invalidity. This error will be
+            // detected late in PopulateAndValidateSnapshot, and allows the
+            // INVALID fuzz target to reach more potential code coverage.
+            const auto& coinbase{g_chain->back()->vtx.back()};
+            outfile << coinbase->GetHash();
+            WriteCompactSize(outfile, 1);   // number of coins for the hash
+            WriteCompactSize(outfile, 999); // index of coin
+            outfile << Coin{coinbase->vout[0], /*nHeightIn=*/999, /*fCoinBaseIn=*/0};
+        }
     }
 
     const auto ActivateFuzzedSnapshot{[&] {
@@ -89,12 +132,16 @@ FUZZ_TARGET(utxo_snapshot, .init = initialize_chain)
     }};
 
     if (fuzzed_data_provider.ConsumeBool()) {
-        for (const auto& block : *g_chain) {
-            BlockValidationState dummy;
-            bool processed{chainman.ProcessNewBlockHeaders({*block}, true, dummy)};
-            Assert(processed);
-            const auto* index{WITH_LOCK(::cs_main, return chainman.m_blockman.LookupBlockIndex(block->GetHash()))};
-            Assert(index);
+        // Consume the bool, but skip the code for the INVALID fuzz target
+        if constexpr (!INVALID) {
+            for (const auto& block : *g_chain) {
+                BlockValidationState dummy;
+                bool processed{chainman.ProcessNewBlockHeaders({*block}, true, dummy)};
+                Assert(processed);
+                const auto* index{WITH_LOCK(::cs_main, return chainman.m_blockman.LookupBlockIndex(block->GetHash()))};
+                Assert(index);
+            }
+            dirty_chainman = true;
         }
     }
 
@@ -118,11 +165,35 @@ FUZZ_TARGET(utxo_snapshot, .init = initialize_chain)
             }
         }
         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
     Assert(!ActivateFuzzedSnapshot());
+    if constexpr (INVALID) {
+        // Activating the snapshot, or any other action that makes the chainman
+        // "dirty" can and must not happen for the INVALID fuzz target
+        Assert(!dirty_chainman);
+    }
+    if (dirty_chainman) {
+        setup.m_node.chainman.reset();
+        setup.m_make_chainman();
+        setup.LoadVerifyActivateChainstate();
+    }
 }
+
+// There are two fuzz targets:
+//
+// The target 'utxo_snapshot', which allows valid snapshots, but is slow,
+// because it has to reset the chainstate manager on almost all fuzz inputs.
+// Otherwise, a dirty header tree or dirty chainstate could leak from one fuzz
+// input execution into the next, which makes execution non-deterministic.
+//
+// The target 'utxo_snapshot_invalid', which is fast and does not require any
+// expensive state to be reset.
+FUZZ_TARGET(utxo_snapshot /*valid*/, .init = initialize_chain<false>) { utxo_snapshot_fuzz<false>(buffer); }
+FUZZ_TARGET(utxo_snapshot_invalid, .init = initialize_chain<true>) { utxo_snapshot_fuzz<true>(buffer); }
+
 } // namespace
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 2d6bc3c988..cf47d16faf 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -244,24 +244,30 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts)
 
     m_node.notifications = std::make_unique<KernelNotifications>(*Assert(m_node.shutdown), m_node.exit_status, *Assert(m_node.warnings));
 
-    const ChainstateManager::Options chainman_opts{
-        .chainparams = chainparams,
-        .datadir = m_args.GetDataDirNet(),
-        .check_block_index = 1,
-        .notifications = *m_node.notifications,
-        .signals = m_node.validation_signals.get(),
-        .worker_threads_num = 2,
+    m_make_chainman = [this, &chainparams] {
+        Assert(!m_node.chainman);
+        const ChainstateManager::Options chainman_opts{
+            .chainparams = chainparams,
+            .datadir = m_args.GetDataDirNet(),
+            .check_block_index = 1,
+            .notifications = *m_node.notifications,
+            .signals = m_node.validation_signals.get(),
+            .worker_threads_num = 2,
+        };
+        const BlockManager::Options blockman_opts{
+            .chainparams = chainman_opts.chainparams,
+            .blocks_dir = m_args.GetBlocksDirPath(),
+            .notifications = chainman_opts.notifications,
+        };
+        m_node.chainman = std::make_unique<ChainstateManager>(*Assert(m_node.shutdown), chainman_opts, blockman_opts);
+        LOCK(m_node.chainman->GetMutex());
+        m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<BlockTreeDB>(DBParams{
+            .path = m_args.GetDataDirNet() / "blocks" / "index",
+            .cache_bytes = static_cast<size_t>(m_cache_sizes.block_tree_db),
+            .memory_only = true,
+        });
     };
-    const BlockManager::Options blockman_opts{
-        .chainparams = chainman_opts.chainparams,
-        .blocks_dir = m_args.GetBlocksDirPath(),
-        .notifications = chainman_opts.notifications,
-    };
-    m_node.chainman = std::make_unique<ChainstateManager>(*Assert(m_node.shutdown), chainman_opts, blockman_opts);
-    m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<BlockTreeDB>(DBParams{
-        .path = m_args.GetDataDirNet() / "blocks" / "index",
-        .cache_bytes = static_cast<size_t>(m_cache_sizes.block_tree_db),
-        .memory_only = true});
+    m_make_chainman();
 }
 
 ChainTestingSetup::~ChainTestingSetup()
diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h
index 9515f0255e..7d69551516 100644
--- a/src/test/util/setup_common.h
+++ b/src/test/util/setup_common.h
@@ -81,6 +81,7 @@ struct ChainTestingSetup : public BasicTestingSetup {
     node::CacheSizes m_cache_sizes{};
     bool m_coins_db_in_memory{true};
     bool m_block_tree_db_in_memory{true};
+    std::function<void()> m_make_chainman{};
 
     explicit ChainTestingSetup(const ChainType chainType = ChainType::MAIN, TestOpts = {});
     ~ChainTestingSetup();