From 44e86d581f9b79c56c97a0510ec4039de9d4f7c5 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Mon, 22 Dec 2025 17:47:02 -0500 Subject: [PATCH 01/18] validation: reuse connect block coins view Co-authored-by: l0rinc --- src/coins.cpp | 7 +++++++ src/coins.h | 3 +++ src/test/coins_tests.cpp | 24 ++++++++++++++++++++++++ src/test/fuzz/coins_view.cpp | 5 +++++ src/test/fuzz/coinscache_sim.cpp | 6 ++++++ src/validation.cpp | 7 +++++-- src/validation.h | 4 ++++ 7 files changed, 54 insertions(+), 2 deletions(-) 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..e239cfa0a536 100644 --- a/src/coins.h +++ b/src/coins.h @@ -452,6 +452,9 @@ class CCoinsViewCache : public CCoinsViewBacked */ void Sync(); + //! Wipe local state. + void Reset() noexcept; + /** * Removes the UTXO with the given outpoint from the cache, if it is * not modified. diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 6396fce60ac3..319d0dedc5ac 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -1120,4 +1120,28 @@ BOOST_AUTO_TEST_CASE(ccoins_emplace_duplicate_keeps_usage_balanced) BOOST_CHECK(cache.AccessCoin(outpoint) == coin1); } +BOOST_AUTO_TEST_CASE(ccoins_reset) +{ + CCoinsView root; + CCoinsViewCacheTest 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}); + cache.SetBestBlock(uint256::ONE); + cache.SelfTest(); + + BOOST_CHECK(cache.AccessCoin(outpoint) == coin); + BOOST_CHECK(!cache.AccessCoin(outpoint).IsSpent()); + BOOST_CHECK_EQUAL(cache.GetCacheSize(), 1); + BOOST_CHECK_EQUAL(cache.GetBestBlock(), uint256::ONE); + + cache.Reset(); + + BOOST_CHECK(cache.AccessCoin(outpoint).IsSpent()); + BOOST_CHECK_EQUAL(cache.GetCacheSize(), 0); + BOOST_CHECK_EQUAL(cache.GetBestBlock(), uint256::ZERO); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index 09595678ad98..a33f43475a2b 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -85,6 +85,11 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend if (is_db && best_block.IsNull()) best_block = uint256::ONE; coins_view_cache.SetBestBlock(best_block); }, + [&] { + coins_view_cache.Reset(); + // Set best block hash to non-null to satisfy the assertion in CCoinsViewDB::BatchWrite(). + if (is_db) coins_view_cache.SetBestBlock(uint256::ONE); + }, [&] { 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..835df2bf9598 100644 --- a/src/test/fuzz/coinscache_sim.cpp +++ b/src/test/fuzz/coinscache_sim.cpp @@ -401,6 +401,12 @@ FUZZ_TARGET(coinscache_sim) caches.back()->Sync(); }, + [&]() { // Reset. + sim_caches[caches.size()].Wipe(); + // Apply to real caches. + caches.back()->Reset(); + }, + [&]() { // GetCacheSize (void)caches.back()->GetCacheSize(); }, diff --git a/src/validation.cpp b/src/validation.cpp index b6da6d2d8d6f..e06ba2a36d9c 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,7 @@ bool Chainstate::ConnectTip( LogDebug(BCLog::BENCH, " - Load block from disk: %.2fms\n", Ticks(time_2 - time_1)); { - CCoinsViewCache view(&CoinsTip()); + auto& view{*m_coins_views->m_connect_block_view}; 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); @@ -3107,6 +3108,7 @@ bool Chainstate::ConnectTip( if (state.IsInvalid()) InvalidBlockFound(pindexNew, state); LogError("%s: ConnectBlock %s failed, %s\n", __func__, pindexNew->GetBlockHash().ToString(), state.ToString()); + view.Reset(); return false; } time_3 = SteadyClock::now(); @@ -3116,7 +3118,8 @@ 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 + view.Reset(); } 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..b8ffeb88cf01 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); + //! Used as an empty view that is only passed into ConnectBlock to help speed up block validation, + //! as well as not pollute the underlying cache with newly created coins in case the block is invalid. + 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 5b16cf977cda4ee138a7ea9c2087ddeefbc5ea6f Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Mon, 22 Dec 2025 19:19:19 -0500 Subject: [PATCH 02/18] coins: don't mutate main cache when connecting block Co-authored-by: l0rinc --- src/coins.cpp | 14 +++ src/coins.h | 5 +- src/coinsviewcacheasync.h | 40 +++++++ src/test/CMakeLists.txt | 1 + src/test/coinsviewcacheasync_tests.cpp | 155 +++++++++++++++++++++++++ src/validation.cpp | 2 +- src/validation.h | 3 +- 7 files changed, 217 insertions(+), 3 deletions(-) create mode 100644 src/coinsviewcacheasync.h create mode 100644 src/test/coinsviewcacheasync_tests.cpp diff --git a/src/coins.cpp b/src/coins.cpp index 2afbbbff38c2..d05fe7c8cfee 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -38,6 +38,20 @@ 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::FetchCoinWithoutMutating(const COutPoint& outpoint) const noexcept +{ + // Walk up the chain of caches, returning on first entry that exists + const CCoinsView* view{base}; + while (const auto* cache{dynamic_cast(view)}) { + auto it{cache->cacheCoins.find(outpoint)}; + if (it != cache->cacheCoins.end()) { + return !it->second.coin.IsSpent() ? std::optional{it->second.coin} : std::nullopt; + } + view = cache->base; + } + return view->GetCoin(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) diff --git a/src/coins.h b/src/coins.h index e239cfa0a536..9cb3671c12a7 100644 --- a/src/coins.h +++ b/src/coins.h @@ -376,6 +376,9 @@ class CCoinsViewCache : public CCoinsViewBacked /* Cached dynamic memory usage for the inner Coin objects. */ mutable size_t cachedCoinsUsage{0}; + //! Get the coin from base but do not access or mutate cacheCoins. + std::optional FetchCoinWithoutMutating(const COutPoint& outpoint) const noexcept; + public: CCoinsViewCache(CCoinsView *baseIn, bool deterministic = false); @@ -485,7 +488,7 @@ class CCoinsViewCache : public CCoinsViewBacked * @note this is marked const, but may actually append to `cacheCoins`, increasing * memory usage. */ - CCoinsMap::iterator FetchCoin(const COutPoint &outpoint) const; + virtual CCoinsMap::iterator FetchCoin(const COutPoint &outpoint) const; }; //! Utility function to add all of a transaction's outputs to a cache. diff --git a/src/coinsviewcacheasync.h b/src/coinsviewcacheasync.h new file mode 100644 index 000000000000..6b5dee91c967 --- /dev/null +++ b/src/coinsviewcacheasync.h @@ -0,0 +1,40 @@ +// 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. + +#ifndef BITCOIN_COINSVIEWCACHEASYNC_H +#define BITCOIN_COINSVIEWCACHEASYNC_H + +#include +#include + +/** + * CCoinsViewCache subclass that does not call GetCoin on base via FetchCoin, so base will not be mutated before Flush. + * Only used in ConnectBlock to pass as an ephemeral view that can be reset if the block is invalid. + * It provides the same interface as CCoinsViewCache, overriding the FetchCoin private method. + */ +class CoinsViewCacheAsync : public CCoinsViewCache +{ +private: + CCoinsMap::iterator FetchCoin(const COutPoint& outpoint) const override + { + auto [ret, inserted]{cacheCoins.try_emplace(outpoint)}; + if (!inserted) return ret; + + // TODO: Remove spent checks once we no longer return spent coins in coinscache_sim CoinsViewBottom. + if (auto coin{FetchCoinWithoutMutating(outpoint)}; coin && !coin->IsSpent()) { + ret->second.coin = std::move(*coin); + } else { + cacheCoins.erase(ret); + return cacheCoins.end(); + } + + cachedCoinsUsage += ret->second.coin.DynamicMemoryUsage(); + return ret; + } + +public: + using CCoinsViewCache::CCoinsViewCache; +}; + +#endif // BITCOIN_COINSVIEWCACHEASYNC_H diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 83cb989aa9b1..96c95cc4571c 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -32,6 +32,7 @@ add_executable(test_bitcoin cluster_linearize_tests.cpp coins_tests.cpp coinscachepair_tests.cpp + coinsviewcacheasync_tests.cpp coinstatsindex_tests.cpp common_url_tests.cpp compress_tests.cpp diff --git a/src/test/coinsviewcacheasync_tests.cpp b/src/test/coinsviewcacheasync_tests.cpp new file mode 100644 index 000000000000..122eaa8462ce --- /dev/null +++ b/src/test/coinsviewcacheasync_tests.cpp @@ -0,0 +1,155 @@ +// 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 +#include + +BOOST_AUTO_TEST_SUITE(coinsviewcacheasync_tests) + +static CBlock CreateBlock() noexcept +{ + static constexpr auto NUM_TXS{100}; + CBlock block; + CMutableTransaction coinbase; + coinbase.vin.emplace_back(); + block.vtx.push_back(MakeTransactionRef(coinbase)); + + Txid prevhash{Txid::FromUint256(uint256{1})}; + + for (const auto i : std::views::iota(1, NUM_TXS)) { + CMutableTransaction tx; + Txid txid{Txid::FromUint256(uint256(i))}; + tx.vin.emplace_back(txid, 0); + prevhash = tx.GetHash(); + 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); +} + +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}; + CoinsViewCacheAsync view{&main_cache}; + for (auto i{0}; i < 3; ++i) { + 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.Reset(); + } +} + +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); + CoinsViewCacheAsync view{&main_cache}; + for (auto i{0}; i < 3; ++i) { + CheckCache(block, view); + view.Reset(); + } +} + +// 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); + CoinsViewCacheAsync view{&main_cache}; + for (auto i{0}; i < 3; ++i) { + for (const auto& tx : block.vtx) { + for (const auto& in : tx->vin) { + const auto& c{view.AccessCoin(in.prevout)}; + BOOST_CHECK(c.IsSpent()); + } + } + // Coins are not added to the view, even though they exist unspent in the parent db + BOOST_CHECK_EQUAL(view.GetCacheSize(), 0); + view.Reset(); + } +} + +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}; + CoinsViewCacheAsync view{&main_cache}; + for (auto i{0}; i < 3; ++i) { + 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_EQUAL(view.GetCacheSize(), 0); + view.Reset(); + } +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/validation.cpp b/src/validation.cpp index e06ba2a36d9c..ab975316974c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1861,7 +1861,7 @@ void CoinsViews::InitCache() { AssertLockHeld(::cs_main); m_cacheview = std::make_unique(&m_catcherview); - m_connect_block_view = std::make_unique(&*m_cacheview); + m_connect_block_view = std::make_unique(&*m_cacheview); } Chainstate::Chainstate( diff --git a/src/validation.h b/src/validation.h index b8ffeb88cf01..38762d73a6d3 100644 --- a/src/validation.h +++ b/src/validation.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -490,7 +491,7 @@ class CoinsViews { //! Used as an empty view that is only passed into ConnectBlock to help speed up block validation, //! as well as not pollute the underlying cache with newly created coins in case the block is invalid. - std::unique_ptr m_connect_block_view GUARDED_BY(cs_main); + 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 From 0a462e55e9b6fdaf5134aa3601fc7775918ff264 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Fri, 26 Dec 2025 11:13:59 -0500 Subject: [PATCH 03/18] fuzz: pass coins_view_cache to TestCoinsView in coins_view --- src/test/fuzz/coins_view.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index a33f43475a2b..ec5fa1984e08 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -42,11 +42,10 @@ void initialize_coins_view() static const auto testing_setup = MakeNoLogFileContext<>(); } -void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend_coins_view, bool is_db) +void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& coins_view_cache, CCoinsView& backend_coins_view, bool is_db) { bool good_data{true}; - CCoinsViewCache coins_view_cache{&backend_coins_view, /*deterministic=*/true}; if (is_db) coins_view_cache.SetBestBlock(uint256::ONE); COutPoint random_out_point; Coin random_coin; @@ -303,7 +302,8 @@ FUZZ_TARGET(coins_view, .init = initialize_coins_view) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; CCoinsView backend_coins_view; - TestCoinsView(fuzzed_data_provider, backend_coins_view, /*is_db=*/false); + CCoinsViewCache coins_view_cache{&backend_coins_view, /*deterministic=*/true}; + TestCoinsView(fuzzed_data_provider, coins_view_cache, backend_coins_view, /*is_db=*/false); } FUZZ_TARGET(coins_view_db, .init = initialize_coins_view) @@ -314,6 +314,7 @@ FUZZ_TARGET(coins_view_db, .init = initialize_coins_view) .cache_bytes = 1_MiB, .memory_only = true, }; - CCoinsViewDB coins_db{std::move(db_params), CoinsViewOptions{}}; - TestCoinsView(fuzzed_data_provider, coins_db, /*is_db=*/true); + CCoinsViewDB backend_coins_view{std::move(db_params), CoinsViewOptions{}}; + CCoinsViewCache coins_view_cache{&backend_coins_view, /*deterministic=*/true}; + TestCoinsView(fuzzed_data_provider, coins_view_cache, backend_coins_view, /*is_db=*/true); } From bd6ec0c82973c32e8820b45d581cc1ebc9db841f Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Fri, 26 Dec 2025 11:12:07 -0500 Subject: [PATCH 04/18] validation: fetch all inputs of block before connecting block --- src/coins.h | 2 +- src/coinsviewcacheasync.h | 106 +++++++++++++++++++++++-- src/test/coinsviewcacheasync_tests.cpp | 23 ++++++ src/test/fuzz/coins_view.cpp | 99 ++++++++++++++++++++++- src/test/fuzz/coinscache_sim.cpp | 40 +++++++++- src/validation.cpp | 1 + 6 files changed, 256 insertions(+), 15 deletions(-) diff --git a/src/coins.h b/src/coins.h index 9cb3671c12a7..2e6564b4e28b 100644 --- a/src/coins.h +++ b/src/coins.h @@ -456,7 +456,7 @@ class CCoinsViewCache : public CCoinsViewBacked void Sync(); //! Wipe local state. - void Reset() noexcept; + virtual void Reset() noexcept; /** * Removes the UTXO with the given outpoint from the cache, if it is diff --git a/src/coinsviewcacheasync.h b/src/coinsviewcacheasync.h index 6b5dee91c967..b37048526732 100644 --- a/src/coinsviewcacheasync.h +++ b/src/coinsviewcacheasync.h @@ -5,28 +5,98 @@ #ifndef BITCOIN_COINSVIEWCACHEASYNC_H #define BITCOIN_COINSVIEWCACHEASYNC_H +#include #include +#include #include +#include +#include +#include +#include +#include + /** - * CCoinsViewCache subclass that does not call GetCoin on base via FetchCoin, so base will not be mutated before Flush. + * CCoinsViewCache subclass that fetches all block inputs before ConnectBlock without mutating the base cache. * Only used in ConnectBlock to pass as an ephemeral view that can be reset if the block is invalid. - * It provides the same interface as CCoinsViewCache, overriding the FetchCoin private method. + * It provides the same interface as CCoinsViewCache, overriding the FetchCoin private method and Reset. + * It adds an additional StartFetching method to provide the block. */ class CoinsViewCacheAsync : public CCoinsViewCache { private: + //! The latest input not yet being fetched. + mutable uint32_t m_input_head{0}; + //! The latest input not yet accessed by a consumer. + mutable uint32_t m_input_tail{0}; + + //! The inputs of the block which is being fetched. + struct InputToFetch { + //! The outpoint of the input to fetch. + const COutPoint& outpoint; + //! The coin that will be fetched. + std::optional coin{std::nullopt}; + + /** + * We only move when m_inputs reallocates during setup. + * We never move after work begins, so we don't have to copy other members. + */ + InputToFetch(InputToFetch&& other) noexcept : outpoint{other.outpoint} {} + explicit InputToFetch(const COutPoint& o LIFETIMEBOUND) noexcept : outpoint{o} {} + }; + mutable std::vector m_inputs{}; + + /** + * Claim and fetch the next input in the queue. + * + * @return true if there are more inputs in the queue to fetch + * @return false if there are no more inputs in the queue to fetch + */ + bool ProcessInput() const noexcept + { + const auto i{m_input_head++}; + if (i >= m_inputs.size()) [[unlikely]] return false; + + auto& input{m_inputs[i]}; + if (auto coin{FetchCoinWithoutMutating(input.outpoint)}) [[likely]] input.coin.emplace(std::move(*coin)); + return true; + } + + //! Get the index in m_inputs for the given outpoint. Advances m_input_tail if found. + std::optional GetInputIndex(const COutPoint& outpoint) const noexcept + { + // This assumes ConnectBlock accesses all inputs in the same order as they are added to m_inputs + // in StartFetching. Some outpoints are not accessed because they are created by the block, so we scan until we + // come across the requested input. We advance the tail since the input will be cached and not accessed through + // this method again. + for (const auto i : std::views::iota(m_input_tail, m_inputs.size())) [[likely]] { + if (m_inputs[i].outpoint == outpoint) { + m_input_tail = i + 1; + return i; + } + } + return std::nullopt; + } + CCoinsMap::iterator FetchCoin(const COutPoint& outpoint) const override { auto [ret, inserted]{cacheCoins.try_emplace(outpoint)}; if (!inserted) return ret; - // TODO: Remove spent checks once we no longer return spent coins in coinscache_sim CoinsViewBottom. - if (auto coin{FetchCoinWithoutMutating(outpoint)}; coin && !coin->IsSpent()) { - ret->second.coin = std::move(*coin); - } else { - cacheCoins.erase(ret); - return cacheCoins.end(); + if (const auto i{GetInputIndex(outpoint)}) [[likely]] { + auto& input{m_inputs[*i]}; + if (input.coin) [[likely]] ret->second.coin = std::move(*input.coin); + } + + if (ret->second.coin.IsSpent()) [[unlikely]] { + // We will only get in here for BIP30 checks or a block with missing or spent inputs. + // TODO: Remove spent checks once we no longer return spent coins in coinscache_sim CoinsViewBottom. + if (auto coin{FetchCoinWithoutMutating(outpoint)}; coin && !coin->IsSpent()) { + ret->second.coin = std::move(*coin); + } else { + cacheCoins.erase(ret); + return cacheCoins.end(); + } } cachedCoinsUsage += ret->second.coin.DynamicMemoryUsage(); @@ -34,6 +104,26 @@ class CoinsViewCacheAsync : public CCoinsViewCache } public: + //! Fetch all block inputs. + void StartFetching(const CBlock& block) noexcept + { + // Loop through the inputs of the block and set them in the queue. + for (const auto& tx : block.vtx | std::views::drop(1)) [[likely]] { + for (const auto& input : tx->vin) [[likely]] m_inputs.emplace_back(input.prevout); + } + // Don't start if there's nothing to fetch. + if (m_inputs.empty()) [[unlikely]] return; + while (ProcessInput()) [[likely]] {} + } + + void Reset() noexcept override + { + m_inputs.clear(); + m_input_head = 0; + m_input_tail = 0; + CCoinsViewCache::Reset(); + } + using CCoinsViewCache::CCoinsViewCache; }; diff --git a/src/test/coinsviewcacheasync_tests.cpp b/src/test/coinsviewcacheasync_tests.cpp index 122eaa8462ce..b5a8f6dfbe45 100644 --- a/src/test/coinsviewcacheasync_tests.cpp +++ b/src/test/coinsviewcacheasync_tests.cpp @@ -86,6 +86,7 @@ BOOST_AUTO_TEST_CASE(fetch_inputs_from_db) CCoinsViewCache main_cache{&db}; CoinsViewCacheAsync view{&main_cache}; for (auto i{0}; i < 3; ++i) { + view.StartFetching(block); CheckCache(block, view); // Check that no coins have been moved up to main cache from db for (const auto& tx : block.vtx) { @@ -105,6 +106,7 @@ BOOST_AUTO_TEST_CASE(fetch_inputs_from_cache) PopulateView(block, main_cache); CoinsViewCacheAsync view{&main_cache}; for (auto i{0}; i < 3; ++i) { + view.StartFetching(block); CheckCache(block, view); view.Reset(); } @@ -122,6 +124,7 @@ BOOST_AUTO_TEST_CASE(fetch_no_double_spend) PopulateView(block, main_cache, /*spent=*/true); CoinsViewCacheAsync view{&main_cache}; for (auto i{0}; i < 3; ++i) { + view.StartFetching(block); for (const auto& tx : block.vtx) { for (const auto& in : tx->vin) { const auto& c{view.AccessCoin(in.prevout)}; @@ -141,6 +144,7 @@ BOOST_AUTO_TEST_CASE(fetch_no_inputs) CCoinsViewCache main_cache{&db}; CoinsViewCacheAsync view{&main_cache}; for (auto i{0}; i < 3; ++i) { + view.StartFetching(block); for (const auto& tx : block.vtx) { for (const auto& in : tx->vin) { const auto& c{view.AccessCoin(in.prevout)}; @@ -152,4 +156,23 @@ BOOST_AUTO_TEST_CASE(fetch_no_inputs) } } +// Access coin that is not a block's input +BOOST_AUTO_TEST_CASE(access_non_input_coin) +{ + const auto block{CreateBlock()}; + CCoinsViewDB db{{.path = "", .cache_bytes = 1_MiB, .memory_only = true}, {}}; + CCoinsViewCache main_cache{&db}; + Coin coin{}; + coin.out.nValue = 1; + const COutPoint outpoint{Txid::FromUint256(uint256::ZERO), 0}; + main_cache.EmplaceCoinInternalDANGER(COutPoint{Txid::FromUint256(uint256::ZERO), 0}, std::move(coin)); + CoinsViewCacheAsync view{&main_cache}; + for (auto i{0}; i < 3; ++i) { + view.StartFetching(block); + const auto& accessed_coin{view.AccessCoin(outpoint)}; + BOOST_CHECK(!accessed_coin.IsSpent()); + view.Reset(); + } +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index ec5fa1984e08..47b8f0947b64 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -3,12 +3,15 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include #include #include +#include #include +#include #include