From 69b01af0eb9017a6ae7ca3134c9dcf89e74dbfa8 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Wed, 24 Dec 2025 09:00:10 +0200 Subject: [PATCH 1/6] coins: add PeekCoin() Introduce a helper to look up a Coin through a stack of CCoinsViewCache layers without populating parent caches. This is useful for ephemeral views (e.g. during ConnectBlock) that want to avoid polluting CoinsTip() when validating invalid blocks. Co-authored-by: l0rinc Co-authored-by: Pieter Wuille Co-authored-by: Ryan Ofsky --- src/coins.cpp | 15 +++++++++++++++ src/coins.h | 9 +++++++++ src/test/coins_tests.cpp | 21 +++++++++++++++++++++ src/test/fuzz/coinscache_sim.cpp | 4 +++- 4 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/coins.cpp b/src/coins.cpp index a3bc369de30..8d51737deb8 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -15,6 +15,7 @@ TRACEPOINT_SEMAPHORE(utxocache, spent); TRACEPOINT_SEMAPHORE(utxocache, uncache); std::optional CCoinsView::GetCoin(const COutPoint& outpoint) const { return std::nullopt; } +std::optional CCoinsView::PeekCoin(const COutPoint& outpoint) const { return GetCoin(outpoint); } uint256 CCoinsView::GetBestBlock() const { return uint256(); } std::vector CCoinsView::GetHeadBlocks() const { return std::vector(); } void CCoinsView::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& hashBlock) @@ -31,6 +32,7 @@ bool CCoinsView::HaveCoin(const COutPoint &outpoint) const CCoinsViewBacked::CCoinsViewBacked(CCoinsView *viewIn) : base(viewIn) { } std::optional CCoinsViewBacked::GetCoin(const COutPoint& outpoint) const { return base->GetCoin(outpoint); } +std::optional CCoinsViewBacked::PeekCoin(const COutPoint& outpoint) const { return base->PeekCoin(outpoint); } bool CCoinsViewBacked::HaveCoin(const COutPoint &outpoint) const { return base->HaveCoin(outpoint); } uint256 CCoinsViewBacked::GetBestBlock() const { return base->GetBestBlock(); } std::vector CCoinsViewBacked::GetHeadBlocks() const { return base->GetHeadBlocks(); } @@ -39,6 +41,14 @@ void CCoinsViewBacked::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& h std::unique_ptr CCoinsViewBacked::Cursor() const { return base->Cursor(); } size_t CCoinsViewBacked::EstimateSize() const { return base->EstimateSize(); } +std::optional CCoinsViewCache::PeekCoin(const COutPoint& outpoint) const +{ + if (auto it{cacheCoins.find(outpoint)}; it != cacheCoins.end()) { + return it->second.coin.IsSpent() ? std::nullopt : std::optional{it->second.coin}; + } + return base->PeekCoin(outpoint); +} + CCoinsViewCache::CCoinsViewCache(CCoinsView* baseIn, bool deterministic) : CCoinsViewBacked(baseIn), m_deterministic(deterministic), cacheCoins(0, SaltedOutpointHasher(/*deterministic=*/deterministic), CCoinsMap::key_equal{}, &m_cache_coins_memory_resource) @@ -393,3 +403,8 @@ bool CCoinsViewErrorCatcher::HaveCoin(const COutPoint& outpoint) const { return ExecuteBackedWrapper([&]() { return CCoinsViewBacked::HaveCoin(outpoint); }, m_err_callbacks); } + +std::optional CCoinsViewErrorCatcher::PeekCoin(const COutPoint& outpoint) const +{ + return ExecuteBackedWrapper>([&]() { return CCoinsViewBacked::PeekCoin(outpoint); }, m_err_callbacks); +} diff --git a/src/coins.h b/src/coins.h index 4b39c0bacd2..36516ef7038 100644 --- a/src/coins.h +++ b/src/coins.h @@ -302,9 +302,15 @@ class CCoinsView { public: //! Retrieve the Coin (unspent transaction output) for a given outpoint. + //! May populate the cache. Use PeekCoin() to perform a non-caching lookup. virtual std::optional GetCoin(const COutPoint& outpoint) const; + //! Retrieve the Coin (unspent transaction output) for a given outpoint, without caching results. + //! Does not populate the cache. Use GetCoin() to cache the result. + virtual std::optional PeekCoin(const COutPoint& outpoint) const; + //! Just check whether a given outpoint is unspent. + //! May populate the cache. Use PeekCoin() to perform a non-caching lookup. virtual bool HaveCoin(const COutPoint &outpoint) const; //! Retrieve the block hash whose state this CCoinsView currently represents @@ -340,6 +346,7 @@ protected: public: CCoinsViewBacked(CCoinsView *viewIn); std::optional GetCoin(const COutPoint& outpoint) const override; + std::optional PeekCoin(const COutPoint& outpoint) const override; bool HaveCoin(const COutPoint &outpoint) const override; uint256 GetBestBlock() const override; std::vector GetHeadBlocks() const override; @@ -386,6 +393,7 @@ public: // Standard CCoinsView methods std::optional GetCoin(const COutPoint& outpoint) const override; + std::optional PeekCoin(const COutPoint& outpoint) const override; bool HaveCoin(const COutPoint &outpoint) const override; uint256 GetBestBlock() const override; void SetBestBlock(const uint256 &hashBlock); @@ -536,6 +544,7 @@ public: std::optional GetCoin(const COutPoint& outpoint) const override; bool HaveCoin(const COutPoint &outpoint) const override; + std::optional PeekCoin(const COutPoint& outpoint) const override; private: /** A list of callbacks to execute upon leveldb read error. */ diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 4d6ee6613ec..6ac9d485f3a 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -1159,4 +1159,25 @@ BOOST_AUTO_TEST_CASE(ccoins_reset_guard) BOOST_CHECK(!root_cache.HaveCoinInCache(outpoint)); } +BOOST_AUTO_TEST_CASE(ccoins_peekcoin) +{ + CCoinsViewTest base{m_rng}; + + // Populate the base view with a coin. + const COutPoint outpoint{Txid::FromUint256(m_rng.rand256()), m_rng.rand32()}; + const Coin coin{CTxOut{m_rng.randrange(10), CScript{}}, 1, false}; + { + CCoinsViewCache cache{&base}; + cache.AddCoin(outpoint, Coin{coin}, /*possible_overwrite=*/false); + cache.Flush(); + } + + // Verify PeekCoin can read through the cache stack without mutating the intermediate cache. + CCoinsViewCacheTest main_cache{&base}; + const auto fetched{main_cache.PeekCoin(outpoint)}; + BOOST_CHECK(fetched.has_value()); + BOOST_CHECK(*fetched == coin); + BOOST_CHECK(!main_cache.HaveCoinInCache(outpoint)); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/coinscache_sim.cpp b/src/test/fuzz/coinscache_sim.cpp index c46c91dc4c1..b1b2842300d 100644 --- a/src/test/fuzz/coinscache_sim.cpp +++ b/src/test/fuzz/coinscache_sim.cpp @@ -257,7 +257,9 @@ FUZZ_TARGET(coinscache_sim) // Look up in simulation data. auto sim = lookup(outpointidx); // Look up in real caches. - auto realcoin = caches.back()->GetCoin(data.outpoints[outpointidx]); + auto realcoin = provider.ConsumeBool() ? + caches.back()->PeekCoin(data.outpoints[outpointidx]) : + caches.back()->GetCoin(data.outpoints[outpointidx]); // Compare results. if (!sim.has_value()) { assert(!realcoin); From 67c0d1798e6147f48d4bafc2c9e5ff30f2a62340 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Wed, 11 Feb 2026 20:28:51 -0500 Subject: [PATCH 2/6] coins: introduce CoinsViewOverlay Introduce `CoinsViewOverlay`, a `CCoinsViewCache` subclass that reads coins without mutating the underlying cache via `FetchCoin()`. Use `PeekCoin()` to look up a Coin through a stack of `CCoinsViewCache` layers without populating parent caches. This prevents the main cache from caching inputs pulled from disk for a block that has not yet been fully validated. Once `Flush()` is called on the view, these inputs will be added as spent to `coinsCache` in the main cache via `BatchWrite()`. This is the foundation for async input fetching, where worker threads must not mutate shared state. Co-authored-by: l0rinc --- src/coins.cpp | 7 +- src/coins.h | 24 ++++ src/test/CMakeLists.txt | 1 + src/test/coinsviewoverlay_tests.cpp | 165 ++++++++++++++++++++++++++++ src/test/fuzz/coinscache_sim.cpp | 6 +- 5 files changed, 201 insertions(+), 2 deletions(-) create mode 100644 src/test/coinsviewoverlay_tests.cpp diff --git a/src/coins.cpp b/src/coins.cpp index 8d51737deb8..3385922f24a 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -60,10 +60,15 @@ size_t CCoinsViewCache::DynamicMemoryUsage() const { return memusage::DynamicUsage(cacheCoins) + cachedCoinsUsage; } +std::optional CCoinsViewCache::FetchCoinFromBase(const COutPoint& outpoint) const +{ + return base->GetCoin(outpoint); +} + CCoinsMap::iterator CCoinsViewCache::FetchCoin(const COutPoint &outpoint) const { const auto [ret, inserted] = cacheCoins.try_emplace(outpoint); if (inserted) { - if (auto coin{base->GetCoin(outpoint)}) { + if (auto coin{FetchCoinFromBase(outpoint)}) { ret->second.coin = std::move(*coin); cachedCoinsUsage += ret->second.coin.DynamicMemoryUsage(); Assert(!ret->second.coin.IsSpent()); diff --git a/src/coins.h b/src/coins.h index 36516ef7038..a2768298a9c 100644 --- a/src/coins.h +++ b/src/coins.h @@ -383,6 +383,9 @@ protected: */ void Reset() noexcept; + /* Fetch the coin from base. Used for cache misses in FetchCoin. */ + virtual std::optional FetchCoinFromBase(const COutPoint& outpoint) const; + public: CCoinsViewCache(CCoinsView *baseIn, bool deterministic = false); @@ -512,6 +515,27 @@ private: CCoinsMap::iterator FetchCoin(const COutPoint &outpoint) const; }; +/** + * CCoinsViewCache overlay that avoids populating/mutating parent cache layers on cache misses. + * + * This is achieved by fetching coins from the base view using PeekCoin() instead of GetCoin(), + * so intermediate CCoinsViewCache layers are not filled. + * + * Used during ConnectBlock() as an ephemeral, resettable top-level view that is flushed only + * on success, so invalid blocks don't pollute the underlying cache. + */ +class CoinsViewOverlay : public CCoinsViewCache +{ +private: + std::optional FetchCoinFromBase(const COutPoint& outpoint) const override + { + return base->PeekCoin(outpoint); + } + +public: + using CCoinsViewCache::CCoinsViewCache; +}; + //! Utility function to add all of a transaction's outputs to a cache. //! When check is false, this assumes that overwrites are only possible for coinbase transactions. //! When check is true, the underlying view may be queried to determine whether an addition is diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 2466a483921..61c3a71bfb0 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -33,6 +33,7 @@ add_executable(test_bitcoin coins_tests.cpp coinscachepair_tests.cpp coinstatsindex_tests.cpp + coinsviewoverlay_tests.cpp common_url_tests.cpp compress_tests.cpp crypto_tests.cpp diff --git a/src/test/coinsviewoverlay_tests.cpp b/src/test/coinsviewoverlay_tests.cpp new file mode 100644 index 00000000000..6b20b31211a --- /dev/null +++ b/src/test/coinsviewoverlay_tests.cpp @@ -0,0 +1,165 @@ +// Copyright (c) The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include +#include +#include + +BOOST_AUTO_TEST_SUITE(coinsviewoverlay_tests) + +namespace { + +CBlock CreateBlock() noexcept +{ + static constexpr auto NUM_TXS{100}; + CBlock block; + CMutableTransaction coinbase; + coinbase.vin.emplace_back(); + block.vtx.push_back(MakeTransactionRef(coinbase)); + + for (const auto i : std::views::iota(1, NUM_TXS)) { + CMutableTransaction tx; + Txid txid{Txid::FromUint256(uint256(i))}; + tx.vin.emplace_back(txid, 0); + block.vtx.push_back(MakeTransactionRef(tx)); + } + + return block; +} + +void PopulateView(const CBlock& block, CCoinsView& view, bool spent = false) +{ + CCoinsViewCache cache{&view}; + cache.SetBestBlock(uint256::ONE); + + for (const auto& tx : block.vtx | std::views::drop(1)) { + for (const auto& in : tx->vin) { + Coin coin{}; + if (!spent) coin.out.nValue = 1; + cache.EmplaceCoinInternalDANGER(COutPoint{in.prevout}, std::move(coin)); + } + } + + cache.Flush(); +} + +void CheckCache(const CBlock& block, const CCoinsViewCache& cache) +{ + uint32_t counter{0}; + + for (const auto& tx : block.vtx) { + if (tx->IsCoinBase()) { + BOOST_CHECK(!cache.HaveCoinInCache(tx->vin[0].prevout)); + } else { + for (const auto& in : tx->vin) { + const auto& outpoint{in.prevout}; + const auto& first{cache.AccessCoin(outpoint)}; + const auto& second{cache.AccessCoin(outpoint)}; + BOOST_CHECK_EQUAL(&first, &second); + ++counter; + BOOST_CHECK(cache.HaveCoinInCache(outpoint)); + } + } + } + BOOST_CHECK_EQUAL(cache.GetCacheSize(), counter); +} + +} // namespace + +BOOST_AUTO_TEST_CASE(fetch_inputs_from_db) +{ + const auto block{CreateBlock()}; + CCoinsViewDB db{{.path = "", .cache_bytes = 1_MiB, .memory_only = true}, {}}; + PopulateView(block, db); + CCoinsViewCache main_cache{&db}; + CoinsViewOverlay view{&main_cache}; + const auto& outpoint{block.vtx[1]->vin[0].prevout}; + + BOOST_CHECK(view.HaveCoin(outpoint)); + BOOST_CHECK(view.GetCoin(outpoint).has_value()); + BOOST_CHECK(!main_cache.HaveCoinInCache(outpoint)); + + CheckCache(block, view); + // Check that no coins have been moved up to main cache from db + for (const auto& tx : block.vtx) { + for (const auto& in : tx->vin) { + BOOST_CHECK(!main_cache.HaveCoinInCache(in.prevout)); + } + } + + view.SetBestBlock(uint256::ONE); + BOOST_CHECK(view.SpendCoin(outpoint)); + view.Flush(); + BOOST_CHECK(!main_cache.PeekCoin(outpoint).has_value()); +} + +BOOST_AUTO_TEST_CASE(fetch_inputs_from_cache) +{ + const auto block{CreateBlock()}; + CCoinsViewDB db{{.path = "", .cache_bytes = 1_MiB, .memory_only = true}, {}}; + CCoinsViewCache main_cache{&db}; + PopulateView(block, main_cache); + CoinsViewOverlay view{&main_cache}; + CheckCache(block, view); + + const auto& outpoint{block.vtx[1]->vin[0].prevout}; + view.SetBestBlock(uint256::ONE); + BOOST_CHECK(view.SpendCoin(outpoint)); + view.Flush(); + BOOST_CHECK(!main_cache.PeekCoin(outpoint).has_value()); +} + +// Test for the case where a block spends coins that are spent in the cache, but +// the spentness has not been flushed to the db. +BOOST_AUTO_TEST_CASE(fetch_no_double_spend) +{ + const auto block{CreateBlock()}; + CCoinsViewDB db{{.path = "", .cache_bytes = 1_MiB, .memory_only = true}, {}}; + PopulateView(block, db); + CCoinsViewCache main_cache{&db}; + // Add all inputs as spent already in cache + PopulateView(block, main_cache, /*spent=*/true); + CoinsViewOverlay view{&main_cache}; + for (const auto& tx : block.vtx) { + for (const auto& in : tx->vin) { + const auto& c{view.AccessCoin(in.prevout)}; + BOOST_CHECK(c.IsSpent()); + BOOST_CHECK(!view.HaveCoin(in.prevout)); + BOOST_CHECK(!view.GetCoin(in.prevout)); + } + } + // Coins are not added to the view, even though they exist unspent in the parent db + BOOST_CHECK_EQUAL(view.GetCacheSize(), 0); +} + +BOOST_AUTO_TEST_CASE(fetch_no_inputs) +{ + const auto block{CreateBlock()}; + CCoinsViewDB db{{.path = "", .cache_bytes = 1_MiB, .memory_only = true}, {}}; + CCoinsViewCache main_cache{&db}; + CoinsViewOverlay view{&main_cache}; + for (const auto& tx : block.vtx) { + for (const auto& in : tx->vin) { + const auto& c{view.AccessCoin(in.prevout)}; + BOOST_CHECK(c.IsSpent()); + BOOST_CHECK(!view.HaveCoin(in.prevout)); + BOOST_CHECK(!view.GetCoin(in.prevout)); + } + } + BOOST_CHECK_EQUAL(view.GetCacheSize(), 0); +} + +BOOST_AUTO_TEST_SUITE_END() + diff --git a/src/test/fuzz/coinscache_sim.cpp b/src/test/fuzz/coinscache_sim.cpp index b1b2842300d..c8534e4f60c 100644 --- a/src/test/fuzz/coinscache_sim.cpp +++ b/src/test/fuzz/coinscache_sim.cpp @@ -374,7 +374,11 @@ FUZZ_TARGET(coinscache_sim) [&]() { // Add a cache level (if not already at the max). if (caches.size() != MAX_CACHES) { // Apply to real caches. - caches.emplace_back(new CCoinsViewCache(&*caches.back(), /*deterministic=*/true)); + if (provider.ConsumeBool()) { + caches.emplace_back(new CCoinsViewCache(&*caches.back(), /*deterministic=*/true)); + } else { + caches.emplace_back(new CoinsViewOverlay(&*caches.back(), /*deterministic=*/true)); + } // Apply to simulation data. sim_caches[caches.size()].Wipe(); } From 73e99a59665551243d6dbe03a0e9baa9cab046b9 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Wed, 11 Feb 2026 20:30:56 -0500 Subject: [PATCH 3/6] coins: don't mutate main cache when connecting block Use `CoinsViewOverlay` when connecting blocks in `ConnectTip`. Add a new integration test to verify that using CoinsViewOverlay does not mutate the main cache during validation for an invalid block. Co-authored-by: l0rinc --- src/test/validation_chainstate_tests.cpp | 26 ++++++++++++++++++++++++ src/validation.cpp | 2 +- src/validation.h | 5 +++-- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 9e2c7109778..a4a81bbae5f 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -3,10 +3,12 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. // #include +#include #include #include #include #include +#include