From 67c0d1798e6147f48d4bafc2c9e5ff30f2a62340 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Wed, 11 Feb 2026 20:28:51 -0500 Subject: [PATCH] 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(); }