diff --git a/src/bench/ccoins_caching.cpp b/src/bench/ccoins_caching.cpp index 82735b3286e..12ea0760801 100644 --- a/src/bench/ccoins_caching.cpp +++ b/src/bench/ccoins_caching.cpp @@ -26,8 +26,7 @@ static void CCoinsCaching(benchmark::Bench& bench) ECC_Context ecc_context{}; FillableSigningProvider keystore; - CCoinsView coinsDummy; - CCoinsViewCache coins(&coinsDummy); + CCoinsViewCache coins{&CoinsViewEmpty::Get()}; std::vector dummyTransactions = SetupDummyInputs(keystore, coins, {11 * COIN, 50 * COIN, 21 * COIN, 22 * COIN}); diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index d4702a9cf0f..4a891ad8f18 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -586,8 +586,7 @@ static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) // starts as a clone of the raw tx: CMutableTransaction mergedTx{tx}; const CMutableTransaction txv{tx}; - CCoinsView viewDummy; - CCoinsViewCache view(&viewDummy); + CCoinsViewCache view{&CoinsViewEmpty::Get()}; if (!registers.contains("privatekeys")) throw std::runtime_error("privatekeys register variable must be set."); diff --git a/src/coins.cpp b/src/coins.cpp index 25b1ead0c1d..c403e006c85 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -14,33 +14,12 @@ TRACEPOINT_SEMAPHORE(utxocache, add); 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) +CoinsViewEmpty& CoinsViewEmpty::Get() { - for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) { } + static CoinsViewEmpty instance; + return instance; } -std::unique_ptr CCoinsView::Cursor() const { return nullptr; } - -bool CCoinsView::HaveCoin(const COutPoint &outpoint) const -{ - return GetCoin(outpoint).has_value(); -} - -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(); } -void CCoinsViewBacked::SetBackend(CCoinsView &viewIn) { base = &viewIn; } -void CCoinsViewBacked::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& hashBlock) { base->BatchWrite(cursor, hashBlock); } -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()) { @@ -49,8 +28,8 @@ std::optional CCoinsViewCache::PeekCoin(const COutPoint& outpoint) const return base->PeekCoin(outpoint); } -CCoinsViewCache::CCoinsViewCache(CCoinsView* baseIn, bool deterministic) : - CCoinsViewBacked(baseIn), m_deterministic(deterministic), +CCoinsViewCache::CCoinsViewCache(CCoinsView* in_base, bool deterministic) : + CCoinsViewBacked(in_base), m_deterministic(deterministic), cacheCoins(0, SaltedOutpointHasher(/*deterministic=*/deterministic), CCoinsMap::key_equal{}, &m_cache_coins_memory_resource) { m_sentinel.second.SelfRef(m_sentinel); @@ -185,7 +164,8 @@ const Coin& CCoinsViewCache::AccessCoin(const COutPoint &outpoint) const { } } -bool CCoinsViewCache::HaveCoin(const COutPoint &outpoint) const { +bool CCoinsViewCache::HaveCoin(const COutPoint& outpoint) const +{ CCoinsMap::const_iterator it = FetchCoin(outpoint); return (it != cacheCoins.end() && !it->second.coin.IsSpent()); } @@ -196,16 +176,17 @@ bool CCoinsViewCache::HaveCoinInCache(const COutPoint &outpoint) const { } uint256 CCoinsViewCache::GetBestBlock() const { - if (hashBlock.IsNull()) - hashBlock = base->GetBestBlock(); - return hashBlock; + if (m_block_hash.IsNull()) + m_block_hash = base->GetBestBlock(); + return m_block_hash; } -void CCoinsViewCache::SetBestBlock(const uint256 &hashBlockIn) { - hashBlock = hashBlockIn; +void CCoinsViewCache::SetBestBlock(const uint256& in_block_hash) +{ + m_block_hash = in_block_hash; } -void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& hashBlockIn) +void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& in_block_hash) { for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) { if (!it->second.IsDirty()) { // TODO a cursor can only contain dirty entries @@ -273,13 +254,13 @@ void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& ha } } } - SetBestBlock(hashBlockIn); + SetBestBlock(in_block_hash); } void CCoinsViewCache::Flush(bool reallocate_cache) { auto cursor{CoinsViewCacheCursor(m_dirty_count, m_sentinel, cacheCoins, /*will_erase=*/true)}; - base->BatchWrite(cursor, hashBlock); + base->BatchWrite(cursor, m_block_hash); Assume(m_dirty_count == 0); cacheCoins.clear(); if (reallocate_cache) { @@ -291,7 +272,7 @@ void CCoinsViewCache::Flush(bool reallocate_cache) void CCoinsViewCache::Sync() { auto cursor{CoinsViewCacheCursor(m_dirty_count, m_sentinel, cacheCoins, /*will_erase=*/false)}; - base->BatchWrite(cursor, hashBlock); + base->BatchWrite(cursor, m_block_hash); Assume(m_dirty_count == 0); if (m_sentinel.second.Next() != &m_sentinel) { /* BatchWrite must clear flags of all entries */ diff --git a/src/coins.h b/src/coins.h index 08c1886f930..e1f1bee757c 100644 --- a/src/coins.h +++ b/src/coins.h @@ -229,7 +229,7 @@ using CCoinsMapMemoryResource = CCoinsMap::allocator_type::ResourceType; class CCoinsViewCursor { public: - CCoinsViewCursor(const uint256 &hashBlockIn): hashBlock(hashBlockIn) {} + CCoinsViewCursor(const uint256& in_block_hash) : block_hash(in_block_hash) {} virtual ~CCoinsViewCursor() = default; virtual bool GetKey(COutPoint &key) const = 0; @@ -239,9 +239,9 @@ public: virtual void Next() = 0; //! Get best block at the time this cursor was created - const uint256 &GetBestBlock() const { return hashBlock; } + const uint256& GetBestBlock() const { return block_hash; } private: - uint256 hashBlock; + uint256 block_hash; }; /** @@ -303,63 +303,89 @@ private: bool m_will_erase; }; -/** Abstract view on the open txout dataset. */ +/** Pure abstract view on the open txout dataset. */ class CCoinsView { public: + //! As we use CCoinsViews polymorphically, have a virtual destructor + virtual ~CCoinsView() = default; + //! 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; + virtual std::optional GetCoin(const COutPoint& outpoint) const = 0; //! 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; + virtual std::optional PeekCoin(const COutPoint& outpoint) const = 0; //! 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; + virtual bool HaveCoin(const COutPoint& outpoint) const = 0; //! Retrieve the block hash whose state this CCoinsView currently represents - virtual uint256 GetBestBlock() const; + virtual uint256 GetBestBlock() const = 0; //! Retrieve the range of blocks that may have been only partially written. //! If the database is in a consistent state, the result is the empty vector. //! Otherwise, a two-element vector is returned consisting of the new and //! the old block hash, in that order. - virtual std::vector GetHeadBlocks() const; + virtual std::vector GetHeadBlocks() const = 0; //! Do a bulk modification (multiple Coin changes + BestBlock change). //! The passed cursor is used to iterate through the coins. - virtual void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& hashBlock); + virtual void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) = 0; - //! Get a cursor to iterate over the whole state - virtual std::unique_ptr Cursor() const; + //! Get a cursor to iterate over the whole state. Implementations may return nullptr. + virtual std::unique_ptr Cursor() const = 0; - //! As we use CCoinsViews polymorphically, have a virtual destructor - virtual ~CCoinsView() = default; - - //! Estimate database size (0 if not implemented) - virtual size_t EstimateSize() const { return 0; } + //! Estimate database size + virtual size_t EstimateSize() const = 0; }; +/** Noop coins view. */ +class CoinsViewEmpty : public CCoinsView +{ +protected: + CoinsViewEmpty() = default; + +public: + static CoinsViewEmpty& Get(); + + CoinsViewEmpty(const CoinsViewEmpty&) = delete; + CoinsViewEmpty& operator=(const CoinsViewEmpty&) = delete; + + std::optional GetCoin(const COutPoint&) const override { return {}; } + std::optional PeekCoin(const COutPoint& outpoint) const override { return GetCoin(outpoint); } + bool HaveCoin(const COutPoint& outpoint) const override { return !!GetCoin(outpoint); } + uint256 GetBestBlock() const override { return {}; } + std::vector GetHeadBlocks() const override { return {}; } + void BatchWrite(CoinsViewCacheCursor& cursor, const uint256&) override + { + for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) { } + } + std::unique_ptr Cursor() const override { return {}; } + size_t EstimateSize() const override { return 0; } +}; /** CCoinsView backed by another CCoinsView */ class CCoinsViewBacked : public CCoinsView { protected: - CCoinsView *base; + CCoinsView* base; 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; - void SetBackend(CCoinsView &viewIn); - void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& hashBlock) override; - std::unique_ptr Cursor() const override; - size_t EstimateSize() const override; + explicit CCoinsViewBacked(CCoinsView* in_view) : base{Assert(in_view)} {} + + void SetBackend(CCoinsView& in_view) { base = &in_view; } + + std::optional GetCoin(const COutPoint& outpoint) const override { return base->GetCoin(outpoint); } + std::optional PeekCoin(const COutPoint& outpoint) const override { return base->PeekCoin(outpoint); } + bool HaveCoin(const COutPoint& outpoint) const override { return base->HaveCoin(outpoint); } + uint256 GetBestBlock() const override { return base->GetBestBlock(); } + std::vector GetHeadBlocks() const override { return base->GetHeadBlocks(); } + void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) override { base->BatchWrite(cursor, block_hash); } + std::unique_ptr Cursor() const override { return base->Cursor(); } + size_t EstimateSize() const override { return base->EstimateSize(); } }; @@ -374,7 +400,7 @@ protected: * Make mutable so that we can "fill the cache" even from Get-methods * declared as "const". */ - mutable uint256 hashBlock; + mutable uint256 m_block_hash; mutable CCoinsMapMemoryResource m_cache_coins_memory_resource{}; /* The starting sentinel of the flagged entry circular doubly linked list. */ mutable CoinsCachePair m_sentinel; @@ -395,7 +421,7 @@ protected: virtual std::optional FetchCoinFromBase(const COutPoint& outpoint) const; public: - CCoinsViewCache(CCoinsView *baseIn, bool deterministic = false); + CCoinsViewCache(CCoinsView* in_base, bool deterministic = false); /** * By deleting the copy constructor, we prevent accidentally using it when one intends to create a cache on top of a base cache. @@ -405,10 +431,10 @@ 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; + bool HaveCoin(const COutPoint& outpoint) const override; uint256 GetBestBlock() const override; - void SetBestBlock(const uint256 &hashBlock); - void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& hashBlock) override; + void SetBestBlock(const uint256& block_hash); + void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) override; std::unique_ptr Cursor() const override { throw std::logic_error("CCoinsViewCache cursor iteration not supported."); } @@ -578,7 +604,7 @@ public: } std::optional GetCoin(const COutPoint& outpoint) const override; - bool HaveCoin(const COutPoint &outpoint) const override; + bool HaveCoin(const COutPoint& outpoint) const override; std::optional PeekCoin(const COutPoint& outpoint) const override; private: diff --git a/src/node/psbt.cpp b/src/node/psbt.cpp index faedf0b6aab..9d79b42f29a 100644 --- a/src/node/psbt.cpp +++ b/src/node/psbt.cpp @@ -116,8 +116,7 @@ PSBTAnalysis AnalyzePSBT(PartiallySignedTransaction psbtx) // Estimate the size CMutableTransaction mtx(*psbtx.tx); - CCoinsView view_dummy; - CCoinsViewCache view(&view_dummy); + CCoinsViewCache view{&CoinsViewEmpty::Get()}; bool success = true; for (unsigned int i = 0; i < psbtx.tx->vin.size(); ++i) { diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index cae9bb6b7a0..74652c17421 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -623,8 +623,7 @@ static RPCMethod combinerawtransaction() CMutableTransaction mergedTx(txVariants[0]); // Fetch previous transactions (inputs): - CCoinsView viewDummy; - CCoinsViewCache view(&viewDummy); + CCoinsViewCache view{&CoinsViewEmpty::Get()}; { NodeContext& node = EnsureAnyNodeContext(request.context); const CTxMemPool& mempool = EnsureMemPool(node); @@ -638,7 +637,7 @@ static RPCMethod combinerawtransaction() view.AccessCoin(txin.prevout); // Load entries from viewChain into view; can fail. } - view.SetBackend(viewDummy); // switch back to avoid locking mempool for too long + view.SetBackend(CoinsViewEmpty::Get()); // switch back to avoid locking mempool for too long } // Use CTransaction for the constant parts of the diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 8321bf6a18c..2a180f25936 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -38,14 +38,14 @@ bool operator==(const Coin &a, const Coin &b) { a.out == b.out; } -class CCoinsViewTest : public CCoinsView +class CCoinsViewTest : public CoinsViewEmpty { FastRandomContext& m_rng; uint256 hashBestBlock_; std::map map_; public: - CCoinsViewTest(FastRandomContext& rng) : m_rng{rng} {} + explicit CCoinsViewTest(FastRandomContext& rng) : m_rng{rng} {} std::optional GetCoin(const COutPoint& outpoint) const override { @@ -55,7 +55,7 @@ public: uint256 GetBestBlock() const override { return hashBestBlock_; } - void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& hashBlock) override + void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) override { for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)){ if (it->second.IsDirty()) { @@ -67,8 +67,8 @@ public: } } } - if (!hashBlock.IsNull()) - hashBestBlock_ = hashBlock; + if (!block_hash.IsNull()) + hashBestBlock_ = block_hash; } }; @@ -677,8 +677,7 @@ public: } } - CCoinsView root; - CCoinsViewCacheTest base{&root}; + CCoinsViewCacheTest base{&CoinsViewEmpty::Get()}; CCoinsViewCacheTest cache{&base}; }; @@ -910,7 +909,7 @@ void TestFlushBehavior( for (auto i = all_caches.rbegin(); i != all_caches.rend(); ++i) { auto& cache = *i; cache->SanityCheck(); - // hashBlock must be filled before flushing to disk; value is + // block_hash must be filled before flushing to disk; value is // unimportant here. This is normally done during connect/disconnect block. cache->SetBestBlock(m_rng.rand256()); erase ? cache->Flush() : cache->Sync(); @@ -1087,8 +1086,7 @@ BOOST_AUTO_TEST_CASE(coins_resource_is_used) BOOST_AUTO_TEST_CASE(ccoins_addcoin_exception_keeps_usage_balanced) { - CCoinsView root; - CCoinsViewCacheTest cache{&root}; + CCoinsViewCacheTest cache{&CoinsViewEmpty::Get()}; const COutPoint outpoint{Txid::FromUint256(m_rng.rand256()), m_rng.rand32()}; @@ -1105,8 +1103,7 @@ BOOST_AUTO_TEST_CASE(ccoins_addcoin_exception_keeps_usage_balanced) BOOST_AUTO_TEST_CASE(ccoins_emplace_duplicate_keeps_usage_balanced) { - CCoinsView root; - CCoinsViewCacheTest cache{&root}; + CCoinsViewCacheTest cache{&CoinsViewEmpty::Get()}; const COutPoint outpoint{Txid::FromUint256(m_rng.rand256()), m_rng.rand32()}; diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index 9ef9e47372e..c11581d2d3c 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -90,9 +90,11 @@ void initialize_coins_view() static const auto testing_setup = MakeNoLogFileContext<>(); } -void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& coins_view_cache, CCoinsView& backend_coins_view, bool is_db) +void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& coins_view_cache, CCoinsView* backend_coins_view) { + const bool is_db{dynamic_cast(backend_coins_view) != nullptr}; bool good_data{true}; + auto* original_backend{backend_coins_view}; if (is_db) coins_view_cache.SetBestBlock(uint256::ONE); COutPoint random_out_point; @@ -124,15 +126,13 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& co }, [&] { uint256 best_block{ConsumeUInt256(fuzzed_data_provider)}; - // Set best block hash to non-null to satisfy the assertion in CCoinsViewDB::BatchWrite(). + // `CCoinsViewDB::BatchWrite()` requires a non-null best block. 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(). + (void)coins_view_cache.CreateResetGuard(); + // Reset() clears the best block, so reseed db-backed caches. if (is_db) { const uint256 best_block{ConsumeUInt256(fuzzed_data_provider)}; if (best_block.IsNull()) { @@ -150,10 +150,16 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& co coins_view_cache.Uncache(random_out_point); }, [&] { - if (fuzzed_data_provider.ConsumeBool()) { - backend_coins_view = CCoinsView{}; + const bool use_original_backend{fuzzed_data_provider.ConsumeBool()}; + if (use_original_backend && backend_coins_view != original_backend) { + // FRESH flags valid against the empty backend may be invalid + // against the original backend, so reset before restoring it. + (void)coins_view_cache.CreateResetGuard(); + // Reset() clears the best block; db backends require a non-null hash. + if (is_db) coins_view_cache.SetBestBlock(uint256::ONE); } - coins_view_cache.SetBackend(backend_coins_view); + backend_coins_view = use_original_backend ? original_backend : &CoinsViewEmpty::Get(); + coins_view_cache.SetBackend(*backend_coins_view); }, [&] { const std::optional opt_out_point = ConsumeDeserializable(fuzzed_data_provider); @@ -232,13 +238,12 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& co } { - if (is_db) { - std::unique_ptr coins_view_cursor = backend_coins_view.Cursor(); - assert(!!coins_view_cursor); + if (is_db && backend_coins_view == original_backend) { + assert(backend_coins_view->Cursor()); } - (void)backend_coins_view.EstimateSize(); - (void)backend_coins_view.GetBestBlock(); - (void)backend_coins_view.GetHeadBlocks(); + (void)backend_coins_view->EstimateSize(); + (void)backend_coins_view->GetBestBlock(); + (void)backend_coins_view->GetHeadBlocks(); } if (fuzzed_data_provider.ConsumeBool()) { @@ -328,11 +333,11 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& co assert(!exists_using_access_coin && !exists_using_have_coin_in_cache && !exists_using_have_coin); } // If HaveCoin on the backend is true, it must also be on the cache if the coin wasn't spent. - const bool exists_using_have_coin_in_backend = backend_coins_view.HaveCoin(random_out_point); + const bool exists_using_have_coin_in_backend = backend_coins_view->HaveCoin(random_out_point); if (!coin_using_access_coin.IsSpent() && exists_using_have_coin_in_backend) { assert(exists_using_have_coin); } - if (auto coin{backend_coins_view.GetCoin(random_out_point)}) { + if (auto coin{backend_coins_view->GetCoin(random_out_point)}) { assert(exists_using_have_coin_in_backend); // Note we can't assert that `coin_using_get_coin == *coin` because the coin in // the cache may have been modified but not yet flushed. @@ -345,9 +350,8 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& co FUZZ_TARGET(coins_view, .init = initialize_coins_view) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; - CCoinsView backend_coins_view; - CCoinsViewCache coins_view_cache{&backend_coins_view, /*deterministic=*/true}; - TestCoinsView(fuzzed_data_provider, coins_view_cache, backend_coins_view, /*is_db=*/false); + CCoinsViewCache coins_view_cache{&CoinsViewEmpty::Get(), /*deterministic=*/true}; + TestCoinsView(fuzzed_data_provider, coins_view_cache, &CoinsViewEmpty::Get()); } FUZZ_TARGET(coins_view_db, .init = initialize_coins_view) @@ -360,7 +364,7 @@ FUZZ_TARGET(coins_view_db, .init = initialize_coins_view) }; 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); + TestCoinsView(fuzzed_data_provider, coins_view_cache, &backend_coins_view); } // Creates a CoinsViewOverlay and a MutationGuardCoinsViewCache as the base. @@ -370,8 +374,7 @@ FUZZ_TARGET(coins_view_db, .init = initialize_coins_view) FUZZ_TARGET(coins_view_overlay, .init = initialize_coins_view) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; - CCoinsView backend_base_coins_view; - MutationGuardCoinsViewCache backend_cache{&backend_base_coins_view, /*deterministic=*/true}; + MutationGuardCoinsViewCache backend_cache{&CoinsViewEmpty::Get(), /*deterministic=*/true}; CoinsViewOverlay coins_view_cache{&backend_cache, /*deterministic=*/true}; - TestCoinsView(fuzzed_data_provider, coins_view_cache, backend_cache, /*is_db=*/false); + TestCoinsView(fuzzed_data_provider, coins_view_cache, &backend_cache); } diff --git a/src/test/fuzz/coinscache_sim.cpp b/src/test/fuzz/coinscache_sim.cpp index c8534e4f60c..9d41a6c058b 100644 --- a/src/test/fuzz/coinscache_sim.cpp +++ b/src/test/fuzz/coinscache_sim.cpp @@ -139,7 +139,7 @@ struct CacheLevel * * The initial state consists of the empty UTXO set. */ -class CoinsViewBottom final : public CCoinsView +class CoinsViewBottom final : public CoinsViewEmpty { std::map m_data; @@ -153,11 +153,6 @@ public: return std::nullopt; } - uint256 GetBestBlock() const final { return {}; } - std::vector GetHeadBlocks() const final { return {}; } - std::unique_ptr Cursor() const final { return {}; } - size_t EstimateSize() const final { return m_data.size(); } - void BatchWrite(CoinsViewCacheCursor& cursor, const uint256&) final { for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) { @@ -252,7 +247,7 @@ FUZZ_TARGET(coinscache_sim) CallOneOf( provider, - [&]() { // GetCoin + [&]() { // PeekCoin/GetCoin uint32_t outpointidx = provider.ConsumeIntegralInRange(0, NUM_OUTPOINTS - 1); // Look up in simulation data. auto sim = lookup(outpointidx); diff --git a/src/test/fuzz/transaction.cpp b/src/test/fuzz/transaction.cpp index a8b2d2a207d..299db8c10c5 100644 --- a/src/test/fuzz/transaction.cpp +++ b/src/test/fuzz/transaction.cpp @@ -86,8 +86,7 @@ FUZZ_TARGET(transaction, .init = initialize_transaction) (void)RecursiveDynamicUsage(tx); (void)SignalsOptInRBF(tx); - CCoinsView coins_view; - const CCoinsViewCache coins_view_cache(&coins_view); + const CCoinsViewCache coins_view_cache{&CoinsViewEmpty::Get()}; (void)ValidateInputsStandardness(tx, coins_view_cache); (void)IsWitnessStandard(tx, coins_view_cache); diff --git a/src/test/script_p2sh_tests.cpp b/src/test/script_p2sh_tests.cpp index 04a03ca4970..f80090aa2b1 100644 --- a/src/test/script_p2sh_tests.cpp +++ b/src/test/script_p2sh_tests.cpp @@ -277,8 +277,7 @@ BOOST_AUTO_TEST_CASE(switchover) BOOST_AUTO_TEST_CASE(ValidateInputsStandardness) { - CCoinsView coinsDummy; - CCoinsViewCache coins(&coinsDummy); + CCoinsViewCache coins{&CoinsViewEmpty::Get()}; FillableSigningProvider keystore; CKey key[6]; for (int i = 0; i < 6; i++) diff --git a/src/test/sigopcount_tests.cpp b/src/test/sigopcount_tests.cpp index d3fec0f9357..a463f43ea88 100644 --- a/src/test/sigopcount_tests.cpp +++ b/src/test/sigopcount_tests.cpp @@ -116,8 +116,7 @@ BOOST_AUTO_TEST_CASE(GetTxSigOpCost) CMutableTransaction spendingTx; // Create utxo set - CCoinsView coinsDummy; - CCoinsViewCache coins(&coinsDummy); + CCoinsViewCache coins{&CoinsViewEmpty::Get()}; // Create key CKey key = GenerateRandomKey(); CPubKey pubkey = key.GetPubKey(); diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 2bb9e8a475e..19c416c225f 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -392,8 +392,7 @@ BOOST_AUTO_TEST_CASE(basic_transaction_tests) BOOST_AUTO_TEST_CASE(test_Get) { FillableSigningProvider keystore; - CCoinsView coinsDummy; - CCoinsViewCache coins(&coinsDummy); + CCoinsViewCache coins{&CoinsViewEmpty::Get()}; std::vector dummyTransactions = SetupDummyInputs(keystore, coins, {11*CENT, 50*CENT, 21*CENT, 22*CENT}); @@ -749,8 +748,7 @@ BOOST_AUTO_TEST_CASE(test_witness) BOOST_AUTO_TEST_CASE(test_IsStandard) { FillableSigningProvider keystore; - CCoinsView coinsDummy; - CCoinsViewCache coins(&coinsDummy); + CCoinsViewCache coins{&CoinsViewEmpty::Get()}; std::vector dummyTransactions = SetupDummyInputs(keystore, coins, {11*CENT, 50*CENT, 21*CENT, 22*CENT}); @@ -1022,8 +1020,7 @@ BOOST_AUTO_TEST_CASE(test_IsStandard) BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops) { - CCoinsView coins_dummy; - CCoinsViewCache coins(&coins_dummy); + CCoinsViewCache coins{&CoinsViewEmpty::Get()}; CKey key; key.MakeNewKey(true); @@ -1132,8 +1129,7 @@ BOOST_AUTO_TEST_CASE(max_standard_legacy_sigops) BOOST_AUTO_TEST_CASE(checktxinputs_invalid_transactions_test) { auto check_invalid{[](CAmount input_value, CAmount output_value, bool coinbase, int spend_height, TxValidationResult expected_result, std::string_view expected_reason) { - CCoinsView coins_dummy; - CCoinsViewCache inputs(&coins_dummy); + CCoinsViewCache inputs{&CoinsViewEmpty::Get()}; const COutPoint prevout{Txid::FromUint256(uint256::ONE), 0}; inputs.AddCoin(prevout, Coin{{input_value, CScript() << OP_TRUE}, /*nHeightIn=*/1, coinbase}, /*possible_overwrite=*/false); @@ -1181,8 +1177,7 @@ BOOST_AUTO_TEST_CASE(getvalueout_out_of_range_throws) /** Sanity check the return value of SpendsNonAnchorWitnessProg for various output types. */ BOOST_AUTO_TEST_CASE(spends_witness_prog) { - CCoinsView coins_dummy; - CCoinsViewCache coins(&coins_dummy); + CCoinsViewCache coins{&CoinsViewEmpty::Get()}; CKey key; key.MakeNewKey(true); const CPubKey pubkey{key.GetPubKey()}; diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 8b746193f93..5c50553e9f4 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -465,8 +465,7 @@ std::pair TestChain100Setup::CreateValidTransactio keystore.AddKey(input_signing_key); } // - Populate a CoinsViewCache with the unspent output - CCoinsView coins_view; - CCoinsViewCache coins_cache(&coins_view); + CCoinsViewCache coins_cache{&CoinsViewEmpty::Get()}; for (const auto& input_transaction : input_transactions) { AddCoins(coins_cache, *input_transaction.get(), input_height); } diff --git a/src/txdb.cpp b/src/txdb.cpp index 2c39cf2767b..a098faa44bc 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -78,7 +78,13 @@ std::optional CCoinsViewDB::GetCoin(const COutPoint& outpoint) const return std::nullopt; } -bool CCoinsViewDB::HaveCoin(const COutPoint &outpoint) const { +std::optional CCoinsViewDB::PeekCoin(const COutPoint& outpoint) const +{ + return GetCoin(outpoint); +} + +bool CCoinsViewDB::HaveCoin(const COutPoint& outpoint) const +{ return m_db->Exists(CoinEntry(&outpoint)); } @@ -97,22 +103,22 @@ std::vector CCoinsViewDB::GetHeadBlocks() const { return vhashHeadBlocks; } -void CCoinsViewDB::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& hashBlock) +void CCoinsViewDB::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) { CDBBatch batch(*m_db); size_t count = 0; const size_t dirty_count{cursor.GetDirtyCount()}; - assert(!hashBlock.IsNull()); + assert(!block_hash.IsNull()); uint256 old_tip = GetBestBlock(); if (old_tip.IsNull()) { // We may be in the middle of replaying. std::vector old_heads = GetHeadBlocks(); if (old_heads.size() == 2) { - if (old_heads[0] != hashBlock) { + if (old_heads[0] != block_hash) { LogError("The coins database detected an inconsistent state, likely due to a previous crash or shutdown. You will need to restart bitcoind with the -reindex-chainstate or -reindex configuration option.\n"); } - assert(old_heads[0] == hashBlock); + assert(old_heads[0] == block_hash); old_tip = old_heads[1]; } } @@ -122,11 +128,11 @@ void CCoinsViewDB::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& hashB dirty_count, cursor.GetTotalCount()), BCLog::BENCH); // In the first batch, mark the database as being in the middle of a - // transition from old_tip to hashBlock. + // transition from old_tip to block_hash. // A vector is used for future extensibility, as we may want to support // interrupting after partial writes from multiple independent reorgs. batch.Erase(DB_BEST_BLOCK); - batch.Write(DB_HEAD_BLOCKS, Vector(hashBlock, old_tip)); + batch.Write(DB_HEAD_BLOCKS, Vector(block_hash, old_tip)); for (auto it{cursor.Begin()}; it != cursor.End();) { if (it->second.IsDirty()) { @@ -154,9 +160,9 @@ void CCoinsViewDB::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& hashB } } - // In the last batch, mark the database as consistent with hashBlock again. + // In the last batch, mark the database as consistent with block_hash again. batch.Erase(DB_HEAD_BLOCKS); - batch.Write(DB_BEST_BLOCK, hashBlock); + batch.Write(DB_BEST_BLOCK, block_hash); LogDebug(BCLog::COINDB, "Writing final batch of %.2f MiB\n", batch.ApproximateSize() * (1.0 / 1048576.0)); m_db->WriteBatch(batch); @@ -174,8 +180,8 @@ class CCoinsViewDBCursor: public CCoinsViewCursor public: // Prefer using CCoinsViewDB::Cursor() since we want to perform some // cache warmup on instantiation. - CCoinsViewDBCursor(CDBIterator* pcursorIn, const uint256&hashBlockIn): - CCoinsViewCursor(hashBlockIn), pcursor(pcursorIn) {} + CCoinsViewDBCursor(CDBIterator* pcursorIn, const uint256& in_block_hash): + CCoinsViewCursor(in_block_hash), pcursor(pcursorIn) {} ~CCoinsViewDBCursor() = default; bool GetKey(COutPoint &key) const override; diff --git a/src/txdb.h b/src/txdb.h index 248fe43e559..b19b312a4b6 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -41,10 +41,11 @@ public: explicit CCoinsViewDB(DBParams db_params, CoinsViewOptions options); std::optional GetCoin(const COutPoint& outpoint) const override; - bool HaveCoin(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; - void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& hashBlock) override; + void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) override; std::unique_ptr Cursor() const override; //! Whether an unsupported database format is used. diff --git a/src/txmempool.h b/src/txmempool.h index d172c78e860..c4723f89155 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -768,7 +768,7 @@ protected: public: CCoinsViewMemPool(CCoinsView* baseIn, const CTxMemPool& mempoolIn); /** GetCoin, returning whether it exists and is not spent. Also updates m_non_base_coins if the - * coin is not fetched from base. */ + * coin is not fetched from base. May populate the base view on cache misses. */ std::optional GetCoin(const COutPoint& outpoint) const override; /** Add the coins created by this transaction. These coins are only temporarily stored in * m_temp_added and cannot be flushed to the back end. Only used for package validation. */ diff --git a/src/validation.cpp b/src/validation.cpp index ff2fdea8d20..a2b93233b91 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -437,7 +437,7 @@ class MemPoolAccept public: explicit MemPoolAccept(CTxMemPool& mempool, Chainstate& active_chainstate) : m_pool(mempool), - m_view(&m_dummy), + m_view(&CoinsViewEmpty::Get()), m_viewmempool(&active_chainstate.CoinsTip(), m_pool), m_active_chainstate(active_chainstate) { @@ -737,10 +737,6 @@ private: /** When m_view is connected to m_viewmempool as its backend, it can pull coins from the mempool and from the UTXO * set. This is also where temporary coins are stored. */ CCoinsViewMemPool m_viewmempool; - /** When m_view is connected to m_dummy, it can no longer look up coins from the mempool or UTXO set (meaning no disk - * operations happen), but can still return coins it accessed previously. Useful for keeping track of which coins - * were pulled from disk. */ - CCoinsView m_dummy; Chainstate& m_active_chainstate; @@ -867,14 +863,14 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } } - // This is const, but calls into the back end CoinsViews. The CCoinsViewDB at the bottom of the - // hierarchy brings the best block into scope. See CCoinsViewDB::GetBestBlock(). - m_view.GetBestBlock(); + // This is const, but calls into `CCoinsViewCache::GetBestBlock()` to refresh + // the cached best block through `m_viewmempool` after caching inputs. + (void)m_view.GetBestBlock(); - // we have all inputs cached now, so switch back to dummy (to protect - // against bugs where we pull more inputs from disk that miss being added - // to coins_to_uncache) - m_view.SetBackend(m_dummy); + // All required inputs are cached now, so switch m_view to the empty backend. + // This keeps already-fetched cache entries for later checks and prevents new + // backend lookups (which would avoid coins_to_uncache tracking). + m_view.SetBackend(CoinsViewEmpty::Get()); assert(m_active_chainstate.m_blockman.LookupBlockIndex(m_view.GetBestBlock()) == m_active_chainstate.m_chain.Tip());