[pull] master from bitcoin:master

contrib/tracing/README.md

@@ -246,10 +246,10 @@ $ python3 contrib/tracing/log_utxocache_flush.py $(pidof bitcoind)
 
 ```
 Logging utxocache flushes. Ctrl-C to end...
-Duration (µs)   Mode       Coins Count     Memory Usage    Prune
-730451          IF_NEEDED  22990           3323.54 kB      True
-637657          ALWAYS     122320          17124.80 kB     False
-81349           ALWAYS     0               1383.49 kB      False
+Duration (µs)   Mode         Coins Count     Memory Usage    Flush for Prune
+2556340         IF_NEEDED    2899141         394844.34 kB    False
+2005788         FORCE_FLUSH  2238117         310189.68 kB    False
+2685            FORCE_FLUSH  0               262.24 kB       False
 ```
 
 ### log_utxos.bt

contrib/tracing/log_utxocache_flush.py

@@ -10,7 +10,7 @@
 """Example logging Bitcoin Core utxo set cache flushes utilizing
     the utxocache:flush tracepoint."""
 
-# USAGE:  ./contrib/tracing/log_utxocache_flush.py path/to/bitcoind
+# USAGE:  ./contrib/tracing/log_utxocache_flush.py <pid of bitcoind>
 
 # BCC: The C program to be compiled to an eBPF program (by BCC) and loaded into
 # a sandboxed Linux kernel VM.
@@ -45,7 +45,8 @@
     'NONE',
     'IF_NEEDED',
     'PERIODIC',
-    'ALWAYS'
+    'FORCE_FLUSH',
+    'FORCE_SYNC',
 ]
 
 
@@ -61,7 +62,7 @@ class Data(ctypes.Structure):
 
 
 def print_event(event):
-    print("%-15d %-10s %-15d %-15s %-8s" % (
+    print("%-15d %-12s %-15d %-15s %-8s" % (
         event.duration,
         FLUSH_MODES[event.mode],
         event.coins_count,
@@ -88,7 +89,7 @@ def handle_flush(_, data, size):
 
     b["flush"].open_perf_buffer(handle_flush)
     print("Logging utxocache flushes. Ctrl-C to end...")
-    print("%-15s %-10s %-15s %-15s %-8s" % ("Duration (µs)", "Mode",
+    print("%-15s %-12s %-15s %-15s %-8s" % ("Duration (µs)", "Mode",
                                             "Coins Count", "Memory Usage",
                                             "Flush for Prune"))
 

doc/tracing.md

@@ -185,8 +185,8 @@ Is called *after* the in-memory UTXO cache is flushed.
 
 Arguments passed:
 1. Time it took to flush the cache microseconds as `int64`
-2. Flush state mode as `uint32`. It's an enumerator class with values `0`
-   (`NONE`), `1` (`IF_NEEDED`), `2` (`PERIODIC`), `3` (`ALWAYS`)
+2. Flush state mode as `uint32`. It's an enumerator class with values
+   `0` (`NONE`), `1` (`IF_NEEDED`), `2` (`PERIODIC`), `3` (`FORCE_FLUSH`), `4` (`FORCE_SYNC`)
 3. Cache size (number of coins) before the flush as `uint64`
 4. Cache memory usage in bytes as `uint64`
 5. If pruning caused the flush as `bool`

src/coins.cpp

@@ -7,6 +7,7 @@
 #include <consensus/consensus.h>
 #include <logging.h>
 #include <random.h>
+#include <uint256.h>
 #include <util/trace.h>
 
 TRACEPOINT_SEMAPHORE(utxocache, add);
@@ -247,15 +248,15 @@ void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& ha
             }
         }
     }
-    hashBlock = hashBlockIn;
+    SetBestBlock(hashBlockIn);
 }
 
-void CCoinsViewCache::Flush(bool will_reuse_cache)
+void CCoinsViewCache::Flush(bool reallocate_cache)
 {
     auto cursor{CoinsViewCacheCursor(m_sentinel, cacheCoins, /*will_erase=*/true)};
     base->BatchWrite(cursor, hashBlock);
     cacheCoins.clear();
-    if (will_reuse_cache) {
+    if (reallocate_cache) {
         ReallocateCache();
     }
     cachedCoinsUsage = 0;
@@ -271,6 +272,13 @@ void CCoinsViewCache::Sync()
     }
 }
 
+void CCoinsViewCache::Reset() noexcept
+{
+    cacheCoins.clear();
+    cachedCoinsUsage = 0;
+    SetBestBlock(uint256::ZERO);
+}
+
 void CCoinsViewCache::Uncache(const COutPoint& hash)
 {
     CCoinsMap::iterator it = cacheCoins.find(hash);

src/coins.h

@@ -6,6 +6,7 @@
 #ifndef BITCOIN_COINS_H
 #define BITCOIN_COINS_H
 
+#include <attributes.h>
 #include <compressor.h>
 #include <core_memusage.h>
 #include <memusage.h>
@@ -369,6 +370,12 @@ class CCoinsViewCache : public CCoinsViewBacked
     /* Cached dynamic memory usage for the inner Coin objects. */
     mutable size_t cachedCoinsUsage{0};
 
+    /**
+     * Discard all modifications made to this cache without flushing to the base view.
+     * This can be used to efficiently reuse a cache instance across multiple operations.
+     */
+    void Reset() noexcept;
+
 public:
     CCoinsViewCache(CCoinsView *baseIn, bool deterministic = false);
 
@@ -432,10 +439,10 @@ class CCoinsViewCache : public CCoinsViewBacked
      * Push the modifications applied to this cache to its base and wipe local state.
      * Failure to call this method or Sync() before destruction will cause the changes
      * to be forgotten.
-     * If will_reuse_cache is false, the cache will retain the same memory footprint
+     * If reallocate_cache is false, the cache will retain the same memory footprint
      * after flushing and should be destroyed to deallocate.
      */
-    void Flush(bool will_reuse_cache = true);
+    void Flush(bool reallocate_cache = true);
 
     /**
      * Push the modifications applied to this cache to its base while retaining
@@ -470,6 +477,25 @@ class CCoinsViewCache : public CCoinsViewBacked
     //! Run an internal sanity check on the cache data structure. */
     void SanityCheck() const;
 
+    class ResetGuard
+    {
+    private:
+        friend CCoinsViewCache;
+        CCoinsViewCache& m_cache;
+        explicit ResetGuard(CCoinsViewCache& cache LIFETIMEBOUND) noexcept : m_cache{cache} {}
+
+    public:
+        ResetGuard(const ResetGuard&) = delete;
+        ResetGuard& operator=(const ResetGuard&) = delete;
+        ResetGuard(ResetGuard&&) = delete;
+        ResetGuard& operator=(ResetGuard&&) = delete;
+
+        ~ResetGuard() { m_cache.Reset(); }
+    };
+
+    //! Create a scoped guard that will call `Reset()` on this cache when it goes out of scope.
+    [[nodiscard]] ResetGuard CreateResetGuard() noexcept { return ResetGuard{*this}; }
+
 private:
     /**
      * @note this is marked const, but may actually append to `cacheCoins`, increasing

src/net_processing.cpp

@@ -1406,11 +1406,13 @@ void PeerManagerImpl::FindNextBlocksToDownload(const Peer& peer, unsigned int co
         return;
     }
 
-    // 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,
+    // When syncing with AssumeUtxo and the snapshot has not yet been validated,
+    // abort downloading blocks from peers that don't have the snapshot block in their best chain.
+    // We can't reorg to this chain due to missing undo data until validation completes,
     // so downloading blocks from it would be futile.
     const CBlockIndex* snap_base{m_chainman.CurrentChainstate().SnapshotBase()};
-    if (snap_base && state->pindexBestKnownBlock->GetAncestor(snap_base->nHeight) != snap_base) {
+    if (snap_base && m_chainman.CurrentChainstate().m_assumeutxo == Assumeutxo::UNVALIDATED &&
+        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;
     }

src/rpc/blockchain.cpp

@@ -1043,7 +1043,7 @@ static RPCHelpMan gettxoutsetinfo()
     NodeContext& node = EnsureAnyNodeContext(request.context);
     ChainstateManager& chainman = EnsureChainman(node);
     Chainstate& active_chainstate = chainman.ActiveChainstate();
-    active_chainstate.ForceFlushStateToDisk();
+    active_chainstate.ForceFlushStateToDisk(/*wipe_cache=*/false);
 
     CCoinsView* coins_view;
     BlockManager* blockman;
@@ -2383,7 +2383,7 @@ static RPCHelpMan scantxoutset()
             ChainstateManager& chainman = EnsureChainman(node);
             LOCK(cs_main);
             Chainstate& active_chainstate = chainman.ActiveChainstate();
-            active_chainstate.ForceFlushStateToDisk();
+            active_chainstate.ForceFlushStateToDisk(/*wipe_cache=*/false);
             pcursor = CHECK_NONFATAL(active_chainstate.CoinsDB().Cursor());
             tip = CHECK_NONFATAL(active_chainstate.m_chain.Tip());
         }
@@ -3200,7 +3200,7 @@ PrepareUTXOSnapshot(
         //
         AssertLockHeld(::cs_main);
 
-        chainstate.ForceFlushStateToDisk();
+        chainstate.ForceFlushStateToDisk(/*wipe_cache=*/false);
 
         maybe_stats = GetUTXOStats(&chainstate.CoinsDB(), chainstate.m_blockman, CoinStatsHashType::HASH_SERIALIZED, interruption_point);
         if (!maybe_stats) {

src/test/chainstate_write_tests.cpp

@@ -82,7 +82,7 @@ BOOST_FIXTURE_TEST_CASE(write_during_multiblock_activation, TestChain100Setup)
     BOOST_CHECK_EQUAL(second_from_tip->pprev, chainstate.m_chain.Tip());
 
     // Set m_next_write to current time
-    chainstate.FlushStateToDisk(state_dummy, FlushStateMode::ALWAYS);
+    chainstate.FlushStateToDisk(state_dummy, FlushStateMode::FORCE_FLUSH);
     m_node.validation_signals->SyncWithValidationInterfaceQueue();
     // The periodic flush interval is between 50 and 70 minutes (inclusive)
     // The next call to a PERIODIC write will flush

src/test/coins_tests.cpp

@@ -1116,4 +1116,47 @@ BOOST_AUTO_TEST_CASE(ccoins_emplace_duplicate_keeps_usage_balanced)
     BOOST_CHECK(cache.AccessCoin(outpoint) == coin1);
 }
 
+BOOST_AUTO_TEST_CASE(ccoins_reset_guard)
+{
+    CCoinsViewTest root{m_rng};
+    CCoinsViewCache root_cache{&root};
+    uint256 base_best_block{m_rng.rand256()};
+    root_cache.SetBestBlock(base_best_block);
+    root_cache.Flush();
+
+    CCoinsViewCache cache{&root};
+
+    const COutPoint outpoint{Txid::FromUint256(m_rng.rand256()), m_rng.rand32()};
+
+    const Coin coin{CTxOut{m_rng.randrange(10), CScript{} << m_rng.randbytes(CScriptBase::STATIC_SIZE + 1)}, 1, false};
+    cache.EmplaceCoinInternalDANGER(COutPoint{outpoint}, Coin{coin});
+
+    uint256 cache_best_block{m_rng.rand256()};
+    cache.SetBestBlock(cache_best_block);
+
+    {
+        const auto reset_guard{cache.CreateResetGuard()};
+        BOOST_CHECK(cache.AccessCoin(outpoint) == coin);
+        BOOST_CHECK(!cache.AccessCoin(outpoint).IsSpent());
+        BOOST_CHECK_EQUAL(cache.GetCacheSize(), 1);
+        BOOST_CHECK_EQUAL(cache.GetBestBlock(), cache_best_block);
+        BOOST_CHECK(!root_cache.HaveCoinInCache(outpoint));
+    }
+
+    BOOST_CHECK(cache.AccessCoin(outpoint).IsSpent());
+    BOOST_CHECK_EQUAL(cache.GetCacheSize(), 0);
+    BOOST_CHECK_EQUAL(cache.GetBestBlock(), base_best_block);
+    BOOST_CHECK(!root_cache.HaveCoinInCache(outpoint));
+
+    // Using a reset guard again is idempotent
+    {
+        const auto reset_guard{cache.CreateResetGuard()};
+    }
+
+    BOOST_CHECK(cache.AccessCoin(outpoint).IsSpent());
+    BOOST_CHECK_EQUAL(cache.GetCacheSize(), 0);
+    BOOST_CHECK_EQUAL(cache.GetBestBlock(), base_best_block);
+    BOOST_CHECK(!root_cache.HaveCoinInCache(outpoint));
+}
+
 BOOST_AUTO_TEST_SUITE_END()

src/test/fuzz/coins_view.cpp

@@ -74,7 +74,7 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend
                 }
             },
             [&] {
-                coins_view_cache.Flush(/*will_reuse_cache=*/fuzzed_data_provider.ConsumeBool());
+                coins_view_cache.Flush(/*reallocate_cache=*/fuzzed_data_provider.ConsumeBool());
             },
             [&] {
                 coins_view_cache.Sync();
@@ -85,6 +85,20 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend
                 if (is_db && best_block.IsNull()) best_block = uint256::ONE;
                 coins_view_cache.SetBestBlock(best_block);
             },
+            [&] {
+                {
+                    const auto reset_guard{coins_view_cache.CreateResetGuard()};
+                }
+                // Set best block hash to non-null to satisfy the assertion in CCoinsViewDB::BatchWrite().
+                if (is_db) {
+                    const uint256 best_block{ConsumeUInt256(fuzzed_data_provider)};
+                    if (best_block.IsNull()) {
+                        good_data = false;
+                        return;
+                    }
+                    coins_view_cache.SetBestBlock(best_block);
+                }
+            },
             [&] {
                 Coin move_to;
                 (void)coins_view_cache.SpendCoin(random_out_point, fuzzed_data_provider.ConsumeBool() ? &move_to : nullptr);

src/test/fuzz/coinscache_sim.cpp

@@ -388,7 +388,7 @@ FUZZ_TARGET(coinscache_sim)
                 // Apply to simulation data.
                 flush();
                 // Apply to real caches.
-                caches.back()->Flush(/*will_reuse_cache=*/provider.ConsumeBool());
+                caches.back()->Flush(/*reallocate_cache=*/provider.ConsumeBool());
             },
 
             [&]() { // Sync.
@@ -398,6 +398,14 @@ FUZZ_TARGET(coinscache_sim)
                 caches.back()->Sync();
             },
 
+            [&]() { // Reset.
+                sim_caches[caches.size()].Wipe();
+                // Apply to real caches.
+                {
+                    const auto reset_guard{caches.back()->CreateResetGuard()};
+                }
+            },
+
             [&]() { // GetCacheSize
                 (void)caches.back()->GetCacheSize();
             },

src/test/fuzz/utxo_snapshot.cpp

@@ -57,7 +57,7 @@ void sanity_check_snapshot()
     // Connect the chain to the tmp chainman and sanity check the chainparams snapshot values.
     LOCK(cs_main);
     auto& cs{node.chainman->ActiveChainstate()};
-    cs.ForceFlushStateToDisk();
+    cs.ForceFlushStateToDisk(/*wipe_cache=*/false);
     const auto stats{*Assert(kernel::ComputeUTXOStats(kernel::CoinStatsHashType::HASH_SERIALIZED, &cs.CoinsDB(), node.chainman->m_blockman))};
     const auto cp_au_data{*Assert(node.chainman->GetParams().AssumeutxoForHeight(2 * COINBASE_MATURITY))};
     Assert(stats.nHeight == cp_au_data.height);

src/test/fuzz/utxo_total_supply.cpp

@@ -87,9 +87,9 @@ FUZZ_TARGET(utxo_total_supply)
         tx.vin.emplace_back(txo.first);
         tx.vout.emplace_back(txo.second.nValue, txo.second.scriptPubKey); // "Forward" coin with no fee
     };
-    const auto UpdateUtxoStats = [&]() {
+    const auto UpdateUtxoStats = [&](bool wipe_cache) {
         LOCK(chainman.GetMutex());
-        chainman.ActiveChainstate().ForceFlushStateToDisk();
+        chainman.ActiveChainstate().ForceFlushStateToDisk(wipe_cache);
         utxo_stats = std::move(
             *Assert(kernel::ComputeUTXOStats(kernel::CoinStatsHashType::NONE, &chainman.ActiveChainstate().CoinsDB(), chainman.m_blockman, {})));
         // Check that miner can't print more money than they are allowed to
@@ -99,7 +99,7 @@ FUZZ_TARGET(utxo_total_supply)
 
     // Update internal state to chain tip
     StoreLastTxo();
-    UpdateUtxoStats();
+    UpdateUtxoStats(/*wipe_cache=*/fuzzed_data_provider.ConsumeBool());
     assert(ActiveHeight() == 0);
     // Get at which height we duplicate the coinbase
     // Assuming that the fuzzer will mine relatively short chains (less than 200 blocks), we want the duplicate coinbase to be not too high.
@@ -124,7 +124,7 @@ FUZZ_TARGET(utxo_total_supply)
     circulation += GetBlockSubsidy(ActiveHeight(), Params().GetConsensus());
 
     assert(ActiveHeight() == 1);
-    UpdateUtxoStats();
+    UpdateUtxoStats(/*wipe_cache=*/fuzzed_data_provider.ConsumeBool());
     current_block = PrepareNextBlock();
     StoreLastTxo();
 
@@ -163,7 +163,7 @@ FUZZ_TARGET(utxo_total_supply)
                     circulation += GetBlockSubsidy(ActiveHeight(), Params().GetConsensus());
                 }
 
-                UpdateUtxoStats();
+                UpdateUtxoStats(/*wipe_cache=*/fuzzed_data_provider.ConsumeBool());
 
                 if (!was_valid) {
                     // utxo stats must not change