From 1ede4803f1f5dffa55644d908062b52bf71394e7 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 8 Aug 2024 09:56:53 -0400 Subject: [PATCH] validation: do not wipe utxo cache for stats/scans/snapshots --- src/bitcoin-chainstate.cpp | 2 +- src/init.cpp | 4 ++-- src/rpc/blockchain.cpp | 6 +++--- src/test/fuzz/utxo_total_supply.cpp | 10 +++++----- src/test/validation_chainstatemanager_tests.cpp | 2 +- src/validation.cpp | 14 +++++++------- src/validation.h | 5 +++-- test/functional/interface_usdt_utxocache.py | 7 ++++--- 8 files changed, 26 insertions(+), 24 deletions(-) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index ebe013b638..7cac0fe1b7 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -290,7 +290,7 @@ epilogue: LOCK(cs_main); for (Chainstate* chainstate : chainman.GetAll()) { if (chainstate->CanFlushToDisk()) { - chainstate->ForceFlushStateToDisk(); + chainstate->ForceFlushStateToDisk(/*wipe_cache=*/true); chainstate->ResetCoinsViews(); } } diff --git a/src/init.cpp b/src/init.cpp index faaf3353d0..d3d4e1524b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -328,7 +328,7 @@ void Shutdown(NodeContext& node) LOCK(cs_main); for (Chainstate* chainstate : node.chainman->GetAll()) { if (chainstate->CanFlushToDisk()) { - chainstate->ForceFlushStateToDisk(); + chainstate->ForceFlushStateToDisk(/*wipe_cache=*/true); } } } @@ -354,7 +354,7 @@ void Shutdown(NodeContext& node) LOCK(cs_main); for (Chainstate* chainstate : node.chainman->GetAll()) { if (chainstate->CanFlushToDisk()) { - chainstate->ForceFlushStateToDisk(); + chainstate->ForceFlushStateToDisk(/*wipe_cache=*/true); chainstate->ResetCoinsViews(); } } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index b449444aff..eccb04c21a 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -976,7 +976,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; @@ -2279,7 +2279,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()); } @@ -2727,7 +2727,7 @@ UniValue CreateUTXOSnapshot( // LOCK(::cs_main); - chainstate.ForceFlushStateToDisk(); + chainstate.ForceFlushStateToDisk(/*wipe_cache=*/false); maybe_stats = GetUTXOStats(&chainstate.CoinsDB(), chainstate.m_blockman, CoinStatsHashType::HASH_SERIALIZED, node.rpc_interruption_point); if (!maybe_stats) { diff --git a/src/test/fuzz/utxo_total_supply.cpp b/src/test/fuzz/utxo_total_supply.cpp index b0f1a1251a..cd0c3dfd27 100644 --- a/src/test/fuzz/utxo_total_supply.cpp +++ b/src/test/fuzz/utxo_total_supply.cpp @@ -77,9 +77,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=*/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 @@ -89,7 +89,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. @@ -114,7 +114,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(); @@ -153,7 +153,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 diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index cf819f8751..6fc4e21905 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -372,7 +372,7 @@ struct SnapshotTestSetup : TestChain100Setup { { for (Chainstate* cs : chainman.GetAll()) { LOCK(::cs_main); - cs->ForceFlushStateToDisk(); + cs->ForceFlushStateToDisk(/*wipe_cache=*/true); } // Process all callbacks referring to the old manager before wiping it. m_node.validation_signals->SyncWithValidationInterfaceQueue(); diff --git a/src/validation.cpp b/src/validation.cpp index 25da81ae8a..7fa4e326b4 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2869,7 +2869,7 @@ bool Chainstate::FlushStateToDisk( // It's been very long since we flushed the cache. Do this infrequently, to optimize cache usage. bool fPeriodicFlush = mode == FlushStateMode::PERIODIC && nNow > m_last_flush + DATABASE_FLUSH_INTERVAL; // Combine all conditions that result in a full cache flush. - fDoFullFlush = (mode == FlushStateMode::ALWAYS) || fCacheLarge || fCacheCritical || fPeriodicFlush || fFlushForPrune; + fDoFullFlush = (mode == FlushStateMode::FORCE_FLUSH) || (mode == FlushStateMode::FORCE_SYNC) || fCacheLarge || fCacheCritical || fPeriodicFlush || fFlushForPrune; // Write blocks and block index to disk. if (fDoFullFlush || fPeriodicWrite) { // Ensure we can write block index @@ -2917,7 +2917,7 @@ bool Chainstate::FlushStateToDisk( return FatalError(m_chainman.GetNotifications(), state, _("Disk space is too low!")); } // Flush the chainstate (which may refer to block index entries). - const auto empty_cache{(mode == FlushStateMode::ALWAYS) || fCacheLarge || fCacheCritical}; + const auto empty_cache{(mode == FlushStateMode::FORCE_FLUSH) || fCacheLarge || fCacheCritical}; if (empty_cache ? !CoinsTip().Flush() : !CoinsTip().Sync()) { return FatalError(m_chainman.GetNotifications(), state, _("Failed to write to coin database.")); } @@ -2941,10 +2941,10 @@ bool Chainstate::FlushStateToDisk( return true; } -void Chainstate::ForceFlushStateToDisk() +void Chainstate::ForceFlushStateToDisk(bool wipe_cache) { BlockValidationState state; - if (!this->FlushStateToDisk(state, FlushStateMode::ALWAYS)) { + if (!this->FlushStateToDisk(state, wipe_cache ? FlushStateMode::FORCE_FLUSH : FlushStateMode::FORCE_SYNC)) { LogPrintf("%s: failed to flush state (%s)\n", __func__, state.ToString()); } } @@ -5569,7 +5569,7 @@ bool Chainstate::ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size) ret = FlushStateToDisk(state, FlushStateMode::IF_NEEDED); } else { // Otherwise, flush state to disk and deallocate the in-memory coins map. - ret = FlushStateToDisk(state, FlushStateMode::ALWAYS); + ret = FlushStateToDisk(state, FlushStateMode::FORCE_FLUSH); } return ret; } @@ -6038,7 +6038,7 @@ util::Result ChainstateManager::PopulateAndValidateSnapshot( // returns in `ActivateSnapshot()`, when `MaybeRebalanceCaches()` is // called, since we've added a snapshot chainstate and therefore will // have to downsize the IBD chainstate, which will result in a call to - // `FlushStateToDisk(ALWAYS)`. + // `FlushStateToDisk(FORCE_FLUSH)`. } assert(index); @@ -6138,7 +6138,7 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation() assert(this->GetAll().size() == 2); CCoinsViewDB& ibd_coins_db = m_ibd_chainstate->CoinsDB(); - m_ibd_chainstate->ForceFlushStateToDisk(); + m_ibd_chainstate->ForceFlushStateToDisk(/*wipe_cache=*/true); const auto& maybe_au_data = m_options.chainparams.AssumeutxoForHeight(curr_height); if (!maybe_au_data) { diff --git a/src/validation.h b/src/validation.h index f905d6e624..33838ab1d9 100644 --- a/src/validation.h +++ b/src/validation.h @@ -448,7 +448,8 @@ enum class FlushStateMode { NONE, IF_NEEDED, PERIODIC, - ALWAYS + FORCE_SYNC, + FORCE_FLUSH, }; /** @@ -678,7 +679,7 @@ public: int nManualPruneHeight = 0); //! Unconditionally flush all changes to disk. - void ForceFlushStateToDisk(); + void ForceFlushStateToDisk(bool wipe_cache); //! Prune blockfiles from the disk if necessary and then flush chainstate changes //! if we pruned. diff --git a/test/functional/interface_usdt_utxocache.py b/test/functional/interface_usdt_utxocache.py index ad98a3a162..f78a115761 100755 --- a/test/functional/interface_usdt_utxocache.py +++ b/test/functional/interface_usdt_utxocache.py @@ -100,7 +100,8 @@ FLUSHMODE_NAME = { 0: "NONE", 1: "IF_NEEDED", 2: "PERIODIC", - 3: "ALWAYS", + 3: "FORCE_SYNC", + 4: "FORCE_FLUSH", } @@ -368,8 +369,8 @@ class UTXOCacheTracepointTest(BitcoinTestFramework): # A node shutdown causes two flushes. One that flushes UTXOS_IN_CACHE # UTXOs and one that flushes 0 UTXOs. Normally the 0-UTXO-flush is the # second flush, however it can happen that the order changes. - expected_flushes.append({"mode": "ALWAYS", "for_prune": False, "size": UTXOS_IN_CACHE}) - expected_flushes.append({"mode": "ALWAYS", "for_prune": False, "size": 0}) + expected_flushes.append({"mode": "FORCE_FLUSH", "for_prune": False, "size": UTXOS_IN_CACHE}) + expected_flushes.append({"mode": "FORCE_FLUSH", "for_prune": False, "size": 0}) self.stop_node(0) bpf.perf_buffer_poll(timeout=200)