From 8dd9200fc9b0d263f8f75943ce581a925d061378 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Sat, 24 Jan 2026 13:58:46 -0500 Subject: [PATCH 1/5] 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 7f2ffc38efa..2afbbbff38c 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 6da53829996..beb3bb37a35 100644 --- a/src/coins.h +++ b/src/coins.h @@ -376,6 +376,12 @@ protected: /* 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 2/5] 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 2afbbbff38c..449adcde03c 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 3/5] 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 beb3bb37a35..f85ea5c9a37 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 @@ public: //! 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 6396fce60ac..344db5bb0bf 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 09595678ad9..ed1e4078dd3 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 f57c25210e3..6894917ecd4 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 4/5] 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 b6da6d2d8d6..b505771e532 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 e4b1e555bdd..c5e29ab62f8 100644 --- a/src/validation.h +++ b/src/validation.h @@ -488,6 +488,10 @@ public: //! 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 5/5] 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 449adcde03c..6f9b5ed1e36 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 f85ea5c9a37..d34196e522b 100644 --- a/src/coins.h +++ b/src/coins.h @@ -446,10 +446,10 @@ public: * 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 ed1e4078dd3..699a45e2c49 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 6894917ecd4..5400cdf1010 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 b505771e532..b1a3ddd25da 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; }