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..fd35e7f69c7c 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,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")) diff --git a/doc/tracing.md b/doc/tracing.md index 927fd34b5536..f2599708f943 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`), `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/coins.cpp b/src/coins.cpp index ccd6d605ba6e..a3bc369de302 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include 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); diff --git a/src/coins.h b/src/coins.h index 66d712be34da..4b39c0bacd28 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 @@ -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 diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 16b4735e5ef6..7354eb57e902 100644 --- a/src/net_processing.cpp +++ b/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; } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 7de01ac641a0..a883570f94b4 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/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/test/coins_tests.cpp b/src/test/coins_tests.cpp index 63024054fa6c..4d6ee6613ecb 100644 --- a/src/test/coins_tests.cpp +++ b/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() diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index 09595678ad98..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(); @@ -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 c6102c2a65ce..c46c91dc4c19 100644 --- a/src/test/fuzz/coinscache_sim.cpp +++ b/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(); }, diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp index dddc67a41b78..67290f7d306b 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 3330d261eb9b..0dcae8580b15 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1856,6 +1856,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( @@ -2769,8 +2770,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 = (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", @@ -2818,7 +2820,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, @@ -2845,10 +2846,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)) { LogWarning("Failed to force flush state (%s)", state.ToString()); } } @@ -2956,7 +2957,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)); @@ -3073,7 +3074,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); @@ -3091,7 +3093,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(/*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; @@ -4904,7 +4906,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; } @@ -5531,7 +5533,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; } @@ -5977,7 +5979,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 ee7257b3ba57..8aa8fbb4d3de 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", "ALWAYS"}; +inline constexpr std::array FlushStateModeNames{"NONE", "IF_NEEDED", "PERIODIC", "FORCE_FLUSH", "FORCE_SYNC"}; enum class FlushStateMode: uint8_t { NONE, IF_NEEDED, PERIODIC, - ALWAYS + FORCE_FLUSH, + FORCE_SYNC, }; /** @@ -488,6 +489,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 @@ -735,8 +740,8 @@ class Chainstate FlushStateMode mode, int nManualPruneHeight = 0); - //! Unconditionally flush all changes to disk. - void ForceFlushStateToDisk(); + //! Flush all changes to disk. + 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/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 diff --git a/test/functional/interface_usdt_utxocache.py b/test/functional/interface_usdt_utxocache.py index fe7f7e3adb6a..da90790d70d7 100755 --- a/test/functional/interface_usdt_utxocache.py +++ b/test/functional/interface_usdt_utxocache.py @@ -109,7 +109,8 @@ 0: "NONE", 1: "IF_NEEDED", 2: "PERIODIC", - 3: "ALWAYS", + 3: "FORCE_FLUSH", + 4: "FORCE_SYNC", } @@ -385,12 +386,12 @@ 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. - 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) @@ -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)