From 0067abe153298ce9f14262a15533033e6e907f2b Mon Sep 17 00:00:00 2001 From: stringintech Date: Thu, 18 Dec 2025 02:04:45 +0330 Subject: [PATCH 1/9] p2p: Allow block downloads from peers without snapshot block after assumeutxo validation After assumeutxo background validation completes, allow block downloads from peers that don't have the snapshot block in their best chain. Previously, these peers were skipped until restart because `m_chainman.CurrentChainstate().SnapshotBase()` continued returning non-null even after validation finished. Add `m_chainman.CurrentChainstate().m_assumeutxo == Assumeutxo::UNVALIDATED` check to only apply the restriction while background validation is ongoing. --- src/net_processing.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 51dcadad6f88..a3707d3b5f4c 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1385,11 +1385,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; } From 7099e93d0a80c65a547131d7bab977b09573310c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sat, 1 Nov 2025 20:58:25 +0100 Subject: [PATCH 2/9] refactor: rename `FlushStateMode::ALWAYS` to `FORCE_FLUSH` This prepares the addition of `FORCE_SYNC`. `empty_cache` in `FlushStateToDisk` was moved up to be reusable and `FlushStateMode::FORCE_FLUSH` was used as a placeholder before we properly split the two new states. `log_utxocache_flush.py` was regenerated and the alignment adjusted for the wider `FlushStateMode` values. Co-authored-by: Pieter Wuille Co-authored-by: optout <13562139+optout21@users.noreply.github.com> --- contrib/tracing/README.md | 8 ++++---- contrib/tracing/log_utxocache_flush.py | 8 ++++---- doc/tracing.md | 4 ++-- src/test/chainstate_write_tests.cpp | 2 +- src/validation.cpp | 10 +++++----- src/validation.h | 6 +++--- test/functional/interface_usdt_utxocache.py | 6 +++--- 7 files changed, 22 insertions(+), 22 deletions(-) diff --git a/contrib/tracing/README.md b/contrib/tracing/README.md index 252053e7b875..f5095d6ba80c 100644 --- a/contrib/tracing/README.md +++ b/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 diff --git a/contrib/tracing/log_utxocache_flush.py b/contrib/tracing/log_utxocache_flush.py index 230b38e975b0..bcbc79dfb30d 100755 --- a/contrib/tracing/log_utxocache_flush.py +++ b/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 # BCC: The C program to be compiled to an eBPF program (by BCC) and loaded into # a sandboxed Linux kernel VM. @@ -45,7 +45,7 @@ 'NONE', 'IF_NEEDED', 'PERIODIC', - 'ALWAYS' + 'FORCE_FLUSH', ] @@ -61,7 +61,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 +88,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")) diff --git a/doc/tracing.md b/doc/tracing.md index 927fd34b5536..e0e96b044eb8 100644 --- a/doc/tracing.md +++ b/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`), 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` diff --git a/src/test/chainstate_write_tests.cpp b/src/test/chainstate_write_tests.cpp index 5c4592277638..4a79cd8a4639 100644 --- a/src/test/chainstate_write_tests.cpp +++ b/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 diff --git a/src/validation.cpp b/src/validation.cpp index b6da6d2d8d6f..c8d326e58029 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2795,8 +2795,9 @@ bool Chainstate::FlushStateToDisk( bool fCacheCritical = mode == FlushStateMode::IF_NEEDED && cache_state >= CoinsCacheSizeState::CRITICAL; // It's been a while since we wrote the block index and chain state to disk. Do this frequently, so we don't need to redownload or reindex after a crash. bool fPeriodicWrite = mode == FlushStateMode::PERIODIC && nNow >= m_next_write; + const auto empty_cache{(mode == FlushStateMode::FORCE_FLUSH) || fCacheLarge || fCacheCritical}; // Combine all conditions that result in a write to disk. - bool should_write = (mode == FlushStateMode::ALWAYS) || fCacheLarge || fCacheCritical || fPeriodicWrite || fFlushForPrune; + bool should_write = empty_cache || fPeriodicWrite || fFlushForPrune; // Write blocks, block index and best chain related state to disk. if (should_write) { LogDebug(BCLog::COINDB, "Writing chainstate to disk: flush mode=%s, prune=%d, large=%d, critical=%d, periodic=%d", @@ -2844,7 +2845,6 @@ 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}; empty_cache ? CoinsTip().Flush() : CoinsTip().Sync(); full_flush_completed = true; TRACEPOINT(utxocache, flush, @@ -2874,7 +2874,7 @@ bool Chainstate::FlushStateToDisk( void Chainstate::ForceFlushStateToDisk() { BlockValidationState state; - if (!this->FlushStateToDisk(state, FlushStateMode::ALWAYS)) { + if (!this->FlushStateToDisk(state, FlushStateMode::FORCE_FLUSH)) { LogWarning("Failed to force flush state (%s)", state.ToString()); } } @@ -5546,7 +5546,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; } @@ -5992,7 +5992,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); diff --git a/src/validation.h b/src/validation.h index e4b1e555bdde..fb96de4e2eb3 100644 --- a/src/validation.h +++ b/src/validation.h @@ -457,12 +457,12 @@ enum DisconnectResult class ConnectTrace; /** @see Chainstate::FlushStateToDisk */ -inline constexpr std::array FlushStateModeNames{"NONE", "IF_NEEDED", "PERIODIC", "ALWAYS"}; +inline constexpr std::array FlushStateModeNames{"NONE", "IF_NEEDED", "PERIODIC", "FORCE_FLUSH"}; enum class FlushStateMode: uint8_t { NONE, IF_NEEDED, PERIODIC, - ALWAYS + FORCE_FLUSH, }; /** @@ -735,7 +735,7 @@ class Chainstate FlushStateMode mode, int nManualPruneHeight = 0); - //! Unconditionally flush all changes to disk. + //! Flush all changes to disk. void ForceFlushStateToDisk(); //! Prune blockfiles from the disk if necessary and then flush chainstate changes diff --git a/test/functional/interface_usdt_utxocache.py b/test/functional/interface_usdt_utxocache.py index fe7f7e3adb6a..bbdb8b9a6b67 100755 --- a/test/functional/interface_usdt_utxocache.py +++ b/test/functional/interface_usdt_utxocache.py @@ -109,7 +109,7 @@ 0: "NONE", 1: "IF_NEEDED", 2: "PERIODIC", - 3: "ALWAYS", + 3: "FORCE_FLUSH", } @@ -389,8 +389,8 @@ def handle_utxocache_flush(_, data, __): # 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) From c6ca2b85a3e6e73674e210aee4ed69c4af2848e4 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 8 Aug 2024 09:56:53 -0400 Subject: [PATCH 3/9] validation: do not wipe utxo cache for stats/scans/snapshots Since #28280, the cost of a non-wiping sync of the UTXO cache is only proportional to the number of dirty entries, rather than proportional to the size of the entire cache. Because of that, there is no reason to perform a wiping flush in case the contents of the cache is still useful. Split the FlushStateMode::ALWAYS mode into a FORCE_SYNC (non-wiping) and a FORCE_FLUSH (wiping), and then use the former in scantxoutset, gettxoutsetinfo, snapshot creation. Co-authored-by: l0rinc Co-authored-by: cedwies <141683552+cedwies@users.noreply.github.com> --- contrib/tracing/log_utxocache_flush.py | 1 + doc/tracing.md | 2 +- src/rpc/blockchain.cpp | 6 +++--- src/test/fuzz/utxo_snapshot.cpp | 2 +- src/test/fuzz/utxo_total_supply.cpp | 10 +++++----- src/validation.cpp | 6 +++--- src/validation.h | 5 +++-- test/functional/interface_usdt_utxocache.py | 5 +++-- 8 files changed, 20 insertions(+), 17 deletions(-) diff --git a/contrib/tracing/log_utxocache_flush.py b/contrib/tracing/log_utxocache_flush.py index bcbc79dfb30d..fd35e7f69c7c 100755 --- a/contrib/tracing/log_utxocache_flush.py +++ b/contrib/tracing/log_utxocache_flush.py @@ -46,6 +46,7 @@ 'IF_NEEDED', 'PERIODIC', 'FORCE_FLUSH', + 'FORCE_SYNC', ] diff --git a/doc/tracing.md b/doc/tracing.md index e0e96b044eb8..f2599708f943 100644 --- a/doc/tracing.md +++ b/doc/tracing.md @@ -186,7 +186,7 @@ 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` (`FORCE_FLUSH`), + `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` diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 763de8368902..4783f37ae596 100644 --- a/src/rpc/blockchain.cpp +++ b/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) { diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp index e6ae29a873fc..f3ab3554656a 100644 --- a/src/test/fuzz/utxo_snapshot.cpp +++ b/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); diff --git a/src/test/fuzz/utxo_total_supply.cpp b/src/test/fuzz/utxo_total_supply.cpp index d27ca3470b41..e23fe74492b8 100644 --- a/src/test/fuzz/utxo_total_supply.cpp +++ b/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 diff --git a/src/validation.cpp b/src/validation.cpp index c8d326e58029..f0ac366134e1 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2797,7 +2797,7 @@ bool Chainstate::FlushStateToDisk( bool fPeriodicWrite = mode == FlushStateMode::PERIODIC && nNow >= m_next_write; const auto empty_cache{(mode == FlushStateMode::FORCE_FLUSH) || fCacheLarge || fCacheCritical}; // Combine all conditions that result in a write to disk. - bool should_write = empty_cache || fPeriodicWrite || fFlushForPrune; + bool should_write = (mode == FlushStateMode::FORCE_SYNC) || empty_cache || fPeriodicWrite || fFlushForPrune; // Write blocks, block index and best chain related state to disk. if (should_write) { LogDebug(BCLog::COINDB, "Writing chainstate to disk: flush mode=%s, prune=%d, large=%d, critical=%d, periodic=%d", @@ -2871,10 +2871,10 @@ bool Chainstate::FlushStateToDisk( return true; } -void Chainstate::ForceFlushStateToDisk() +void Chainstate::ForceFlushStateToDisk(bool wipe_cache) { BlockValidationState state; - if (!this->FlushStateToDisk(state, FlushStateMode::FORCE_FLUSH)) { + if (!this->FlushStateToDisk(state, wipe_cache ? FlushStateMode::FORCE_FLUSH : FlushStateMode::FORCE_SYNC)) { LogWarning("Failed to force flush state (%s)", state.ToString()); } } diff --git a/src/validation.h b/src/validation.h index fb96de4e2eb3..b666fe6b0a1a 100644 --- a/src/validation.h +++ b/src/validation.h @@ -457,12 +457,13 @@ enum DisconnectResult class ConnectTrace; /** @see Chainstate::FlushStateToDisk */ -inline constexpr std::array FlushStateModeNames{"NONE", "IF_NEEDED", "PERIODIC", "FORCE_FLUSH"}; +inline constexpr std::array FlushStateModeNames{"NONE", "IF_NEEDED", "PERIODIC", "FORCE_FLUSH", "FORCE_SYNC"}; enum class FlushStateMode: uint8_t { NONE, IF_NEEDED, PERIODIC, FORCE_FLUSH, + FORCE_SYNC, }; /** @@ -736,7 +737,7 @@ class Chainstate int nManualPruneHeight = 0); //! Flush all changes to disk. - void ForceFlushStateToDisk(); + void ForceFlushStateToDisk(bool wipe_cache = true); //! 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 bbdb8b9a6b67..da90790d70d7 100755 --- a/test/functional/interface_usdt_utxocache.py +++ b/test/functional/interface_usdt_utxocache.py @@ -110,6 +110,7 @@ 1: "IF_NEEDED", 2: "PERIODIC", 3: "FORCE_FLUSH", + 4: "FORCE_SYNC", } @@ -385,7 +386,7 @@ def handle_utxocache_flush(_, data, __): bpf["utxocache_flush"].open_perf_buffer(handle_utxocache_flush) self.log.info("stop the node to flush the UTXO cache") - UTXOS_IN_CACHE = 2 # might need to be changed if the earlier tests are modified + UTXOS_IN_CACHE = 3 # might need to be changed if the earlier tests are modified # 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. @@ -415,7 +416,7 @@ def handle_utxocache_flush(_, data, __): bpf["utxocache_flush"].open_perf_buffer(handle_utxocache_flush) self.log.info("prune blockchain to trigger a flush for pruning") - expected_flushes.append({"mode": "NONE", "for_prune": True, "size": 0}) + expected_flushes.append({"mode": "NONE", "for_prune": True, "size": BLOCKS_TO_MINE}) self.nodes[0].pruneblockchain(315) bpf.perf_buffer_poll(timeout=500) From 7d9e1a810239a65a153c35f0f94490560441db49 Mon Sep 17 00:00:00 2001 From: stringintech Date: Thu, 18 Dec 2025 02:06:57 +0330 Subject: [PATCH 4/9] test: Verify peer usage after assumeutxo validation completes Add test coverage to ensure peers without the snapshot block in their chain can be used for block downloads after background validation completes. The test fails without the fix in the previous commit. --- test/functional/feature_assumeutxo.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index 20ebd823d13d..7420dfe02a01 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -347,6 +347,29 @@ def test_sync_from_assumeutxo_node(self, snapshot): assert 'NETWORK' in ibd_node.getpeerinfo()[0]['servicesnames'] self.sync_blocks(nodes=(ibd_node, snapshot_node)) + def test_sync_to_most_work_chain_after_background_validation(self): + """ + After background validation completes, node should be able + to download and process blocks from peers without the snapshot block in their chain. + """ + self.log.info("Testing sync to the most-work chain without the snapshot block after background validation") + + forking_node = self.nodes[0] + snapshot_node = self.nodes[2] # Has already completed background validation + + self.log.info("Forking node switches to an alternative chain that forks one block before the snapshot block") + fork_point = SNAPSHOT_BASE_HEIGHT - 1 + forking_node_old_height = forking_node.getblockcount() + forking_node_old_chainwork = int(forking_node.getblockchaininfo()['chainwork'], 16) + forking_node.invalidateblock(forking_node.getblockhash(fork_point + 1)) + + self.log.info("Mine one more block than original chain to make the new chain have most work") + self.generate(forking_node, nblocks=(forking_node_old_height - fork_point) + 1, sync_fun=self.no_op) + assert int(forking_node.getblockchaininfo()['chainwork'], 16) > forking_node_old_chainwork + + self.log.info("Snapshot node should reorg to the most-work chain without the snapshot block") + self.sync_blocks(nodes=(snapshot_node, forking_node)) + def assert_only_network_limited_service(self, node): node_services = node.getnetworkinfo()['localservicesnames'] assert 'NETWORK' not in node_services @@ -775,6 +798,8 @@ def check_tx_counts(final: bool) -> None: # The following test cleans node2 and node3 chain directories. self.test_sync_from_assumeutxo_node(snapshot=dump_output) + self.test_sync_to_most_work_chain_after_background_validation() + @dataclass class Block: hash: str From 8dd9200fc9b0d263f8f75943ce581a925d061378 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Sat, 24 Jan 2026 13:58:46 -0500 Subject: [PATCH 5/9] coins: add Reset on CCoinsViewCache Add a Reset() method to CCoinsViewCache that clears cacheCoins, cachedCoinsUsage, and hashBlock without flushing to the base view. Co-authored-by: l0rinc Co-authored-by: sedited --- src/coins.cpp | 7 +++++++ src/coins.h | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/src/coins.cpp b/src/coins.cpp index 7f2ffc38efa0..2afbbbff38c2 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -274,6 +274,13 @@ void CCoinsViewCache::Sync() } } +void CCoinsViewCache::Reset() noexcept +{ + cacheCoins.clear(); + cachedCoinsUsage = 0; + hashBlock.SetNull(); +} + void CCoinsViewCache::Uncache(const COutPoint& hash) { CCoinsMap::iterator it = cacheCoins.find(hash); diff --git a/src/coins.h b/src/coins.h index 6da53829996d..beb3bb37a35c 100644 --- a/src/coins.h +++ b/src/coins.h @@ -376,6 +376,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); From 041758f5eda5725daad4ae20f66c7d19ba02d063 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Sun, 25 Jan 2026 18:33:25 -0500 Subject: [PATCH 6/9] coins: use hashBlock setter internally for CCoinsViewCache methods Co-authored-by: l0rinc --- src/coins.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 2afbbbff38c2..449adcde03c2 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include TRACEPOINT_SEMAPHORE(utxocache, add); @@ -250,7 +251,7 @@ void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& ha } } } - hashBlock = hashBlockIn; + SetBestBlock(hashBlockIn); } void CCoinsViewCache::Flush(bool will_reuse_cache) @@ -278,7 +279,7 @@ void CCoinsViewCache::Reset() noexcept { cacheCoins.clear(); cachedCoinsUsage = 0; - hashBlock.SetNull(); + SetBestBlock(uint256::ZERO); } void CCoinsViewCache::Uncache(const COutPoint& hash) From 8fb6043231ea396aaa1165b36b082c89e10fcafd Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Sat, 24 Jan 2026 13:59:54 -0500 Subject: [PATCH 7/9] coins: introduce CCoinsViewCache::ResetGuard CCoinsViewCache::CreateResetGuard returns a guard that calls Reset on the cache when the guard goes out of scope. This RAII pattern ensures the cache is always properly reset when it leaves current scope. Co-authored-by: l0rinc Co-authored-by: sedited --- src/coins.h | 20 +++++++++++++++ src/test/coins_tests.cpp | 43 ++++++++++++++++++++++++++++++++ src/test/fuzz/coins_view.cpp | 14 +++++++++++ src/test/fuzz/coinscache_sim.cpp | 8 ++++++ 4 files changed, 85 insertions(+) diff --git a/src/coins.h b/src/coins.h index beb3bb37a35c..f85ea5c9a377 100644 --- a/src/coins.h +++ b/src/coins.h @@ -6,6 +6,7 @@ #ifndef BITCOIN_COINS_H #define BITCOIN_COINS_H +#include #include #include #include @@ -483,6 +484,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 diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 6396fce60ac3..344db5bb0bf5 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -1120,4 +1120,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() diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index 09595678ad98..ed1e4078dd30 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -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); diff --git a/src/test/fuzz/coinscache_sim.cpp b/src/test/fuzz/coinscache_sim.cpp index f57c25210e3c..6894917ecd4f 100644 --- a/src/test/fuzz/coinscache_sim.cpp +++ b/src/test/fuzz/coinscache_sim.cpp @@ -401,6 +401,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(); }, From 44b4ee194d3bdccd86cf5e151b2fc1479aabbb6c Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Sat, 24 Jan 2026 14:02:43 -0500 Subject: [PATCH 8/9] validation: reuse same CCoinsViewCache for every ConnectBlock call Add m_connect_block_view to ChainState's CoinsViews. Call CreateResetGuard inside ConnectTip to ensure the view is Reset after each block, avoiding repeated memory allocations. Co-authored-by: l0rinc --- src/validation.cpp | 6 ++++-- src/validation.h | 4 ++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index b6da6d2d8d6f..b505771e532b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1861,6 +1861,7 @@ void CoinsViews::InitCache() { AssertLockHeld(::cs_main); m_cacheview = std::make_unique(&m_catcherview); + m_connect_block_view = std::make_unique(&*m_cacheview); } Chainstate::Chainstate( @@ -3098,7 +3099,8 @@ bool Chainstate::ConnectTip( LogDebug(BCLog::BENCH, " - Load block from disk: %.2fms\n", Ticks(time_2 - time_1)); { - CCoinsViewCache view(&CoinsTip()); + CCoinsViewCache& view{*m_coins_views->m_connect_block_view}; + const auto reset_guard{view.CreateResetGuard()}; bool rv = ConnectBlock(*block_to_connect, state, pindexNew, view); if (m_chainman.m_options.signals) { m_chainman.m_options.signals->BlockChecked(block_to_connect, state); @@ -3116,7 +3118,7 @@ bool Chainstate::ConnectTip( Ticks(time_3 - time_2), Ticks(m_chainman.time_connect_total), Ticks(m_chainman.time_connect_total) / m_chainman.num_blocks_total); - view.Flush(/*will_reuse_cache=*/false); // local CCoinsViewCache goes out of scope + view.Flush(/*will_reuse_cache=*/false); // No need to reallocate since it only has capacity for 1 block } const auto time_4{SteadyClock::now()}; m_chainman.time_flush += time_4 - time_3; diff --git a/src/validation.h b/src/validation.h index e4b1e555bdde..c5e29ab62f86 100644 --- a/src/validation.h +++ b/src/validation.h @@ -488,6 +488,10 @@ class CoinsViews { //! can fit per the dbcache setting. std::unique_ptr m_cacheview GUARDED_BY(cs_main); + //! Temporary CCoinsViewCache layered on top of m_cacheview and passed to ConnectBlock(). + //! Reset between calls and flushed only on success, so invalid blocks don't pollute the underlying cache. + std::unique_ptr m_connect_block_view GUARDED_BY(cs_main); + //! This constructor initializes CCoinsViewDB and CCoinsViewErrorCatcher instances, but it //! *does not* create a CCoinsViewCache instance by default. This is done separately because the //! presence of the cache has implications on whether or not we're allowed to flush the cache's From 3e0fd0e4ddd894f0e7db1772f10ceaa1dddfb951 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Thu, 15 Jan 2026 16:05:16 -0500 Subject: [PATCH 9/9] refactor: rename will_reuse_cache to reallocate_cache More accurately reflects the purpose of the parameter, since we will keep reusing the cache but don't want to reallocate it. --- src/coins.cpp | 4 ++-- src/coins.h | 4 ++-- src/test/fuzz/coins_view.cpp | 2 +- src/test/fuzz/coinscache_sim.cpp | 2 +- src/validation.cpp | 6 +++--- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 449adcde03c2..6f9b5ed1e360 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -254,12 +254,12 @@ void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& ha 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; diff --git a/src/coins.h b/src/coins.h index f85ea5c9a377..d34196e522b7 100644 --- a/src/coins.h +++ b/src/coins.h @@ -446,10 +446,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 diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index ed1e4078dd30..699a45e2c490 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/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(); diff --git a/src/test/fuzz/coinscache_sim.cpp b/src/test/fuzz/coinscache_sim.cpp index 6894917ecd4f..5400cdf10104 100644 --- a/src/test/fuzz/coinscache_sim.cpp +++ b/src/test/fuzz/coinscache_sim.cpp @@ -391,7 +391,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. diff --git a/src/validation.cpp b/src/validation.cpp index b505771e532b..b1a3ddd25da0 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2983,7 +2983,7 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra LogError("DisconnectTip(): DisconnectBlock %s failed\n", pindexDelete->GetBlockHash().ToString()); return false; } - view.Flush(/*will_reuse_cache=*/false); // local CCoinsViewCache goes out of scope + view.Flush(/*reallocate_cache=*/false); // local CCoinsViewCache goes out of scope } LogDebug(BCLog::BENCH, "- Disconnect block: %.2fms\n", Ticks(SteadyClock::now() - time_start)); @@ -3118,7 +3118,7 @@ bool Chainstate::ConnectTip( Ticks(time_3 - time_2), Ticks(m_chainman.time_connect_total), Ticks(m_chainman.time_connect_total) / m_chainman.num_blocks_total); - view.Flush(/*will_reuse_cache=*/false); // No need to reallocate since it only has capacity for 1 block + view.Flush(/*reallocate_cache=*/false); // No need to reallocate since it only has capacity for 1 block } const auto time_4{SteadyClock::now()}; m_chainman.time_flush += time_4 - time_3; @@ -4921,7 +4921,7 @@ bool Chainstate::ReplayBlocks() } cache.SetBestBlock(pindexNew->GetBlockHash()); - cache.Flush(/*will_reuse_cache=*/false); // local CCoinsViewCache goes out of scope + cache.Flush(/*reallocate_cache=*/false); // local CCoinsViewCache goes out of scope m_chainman.GetNotifications().progress(bilingual_str{}, 100, false); return true; }