From 06172ef0d501742b22dfb56290616184fb585404 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Fri, 20 Feb 2026 15:48:21 +0100 Subject: [PATCH 1/7] refactor: rename `hashBlock` to `m_block_hash` to avoid shadowing --- src/coins.cpp | 12 ++++++------ src/coins.h | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 25b1ead0c1d..5104b558f0d 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -196,13 +196,13 @@ 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; + m_block_hash = hashBlockIn; } void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& hashBlockIn) @@ -279,7 +279,7 @@ void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& ha 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 +291,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..c0bf3c5a174 100644 --- a/src/coins.h +++ b/src/coins.h @@ -374,7 +374,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; From 38a99f334446d957f70255fc7f49667264ec43bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sun, 15 Feb 2026 15:24:53 +0100 Subject: [PATCH 2/7] scripted-diff: normalize `CCoinsView` naming Standardize coins view naming with a mechanical rename pass. This keeps subsequent commits focused on the interface and behavioral changes. Co-authored-by: Andrew Toth -BEGIN VERIFY SCRIPT- git grep -qE '\bin_base\b|\bin_view\b|\bin_block_hash\b|\bblock_hash\b' -- src/coins.cpp src/coins.h src/txdb.cpp src/txdb.h src/test/coins_tests.cpp && { echo "Error: target names already exist in scoped files"; exit 1; } perl -pi -e ' s/\bbaseIn\b/in_base/g; s/\bhashBlockIn\b/in_block_hash/g; s/\bhashBlock\b/block_hash/g; s/\bviewIn\b/in_view/g; ' src/coins.cpp src/coins.h src/txdb.cpp src/txdb.h src/test/coins_tests.cpp -END VERIFY SCRIPT- --- src/coins.cpp | 20 ++++++++++---------- src/coins.h | 20 ++++++++++---------- src/test/coins_tests.cpp | 8 ++++---- src/txdb.cpp | 20 ++++++++++---------- src/txdb.h | 2 +- 5 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 5104b558f0d..444516ea3dc 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -18,7 +18,7 @@ std::optional CCoinsView::GetCoin(const COutPoint& outpoint) const { retur 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) +void CCoinsView::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) { for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) { } } @@ -30,14 +30,14 @@ bool CCoinsView::HaveCoin(const COutPoint &outpoint) const return GetCoin(outpoint).has_value(); } -CCoinsViewBacked::CCoinsViewBacked(CCoinsView *viewIn) : base(viewIn) { } +CCoinsViewBacked::CCoinsViewBacked(CCoinsView *in_view) : base(in_view) { } 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); } +void CCoinsViewBacked::SetBackend(CCoinsView &in_view) { base = &in_view; } +void CCoinsViewBacked::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) { base->BatchWrite(cursor, block_hash); } std::unique_ptr CCoinsViewBacked::Cursor() const { return base->Cursor(); } size_t CCoinsViewBacked::EstimateSize() const { return base->EstimateSize(); } @@ -49,8 +49,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); @@ -201,11 +201,11 @@ uint256 CCoinsViewCache::GetBestBlock() const { return m_block_hash; } -void CCoinsViewCache::SetBestBlock(const uint256 &hashBlockIn) { - m_block_hash = 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,7 +273,7 @@ void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& ha } } } - SetBestBlock(hashBlockIn); + SetBestBlock(in_block_hash); } void CCoinsViewCache::Flush(bool reallocate_cache) diff --git a/src/coins.h b/src/coins.h index c0bf3c5a174..6cbcb5c9507 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; }; /** @@ -330,7 +330,7 @@ public: //! 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); //! Get a cursor to iterate over the whole state virtual std::unique_ptr Cursor() const; @@ -350,14 +350,14 @@ protected: CCoinsView *base; public: - CCoinsViewBacked(CCoinsView *viewIn); + CCoinsViewBacked(CCoinsView *in_view); 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; + void SetBackend(CCoinsView &in_view); + void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) override; std::unique_ptr Cursor() const override; size_t EstimateSize() const override; }; @@ -395,7 +395,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. @@ -407,8 +407,8 @@ public: std::optional PeekCoin(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."); } diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 8321bf6a18c..f8de7f5d3af 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -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; } }; @@ -910,7 +910,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(); diff --git a/src/txdb.cpp b/src/txdb.cpp index 2c39cf2767b..b7f9fdf1bb2 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -97,22 +97,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 +122,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 +154,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 +174,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..b8c06c2e3e0 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -44,7 +44,7 @@ public: 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. From a9f92e3497529b180d723cc4ef71ed37662957f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Fri, 20 Feb 2026 11:19:13 +0100 Subject: [PATCH 3/7] refactor: normalize CCoinsView whitespace and signatures Let's get these out of the way to simplify riskier followup commits --- src/coins.cpp | 14 ++++++++------ src/coins.h | 22 +++++++++++----------- src/txdb.cpp | 5 +++-- src/txdb.h | 2 +- 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 444516ea3dc..87bbdcbfa9e 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -25,18 +25,18 @@ void CCoinsView::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_h std::unique_ptr CCoinsView::Cursor() const { return nullptr; } -bool CCoinsView::HaveCoin(const COutPoint &outpoint) const +bool CCoinsView::HaveCoin(const COutPoint& outpoint) const { return GetCoin(outpoint).has_value(); } -CCoinsViewBacked::CCoinsViewBacked(CCoinsView *in_view) : base(in_view) { } +CCoinsViewBacked::CCoinsViewBacked(CCoinsView* in_view) : base(in_view) { } 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); } +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 &in_view) { base = &in_view; } +void CCoinsViewBacked::SetBackend(CCoinsView& in_view) { base = &in_view; } void CCoinsViewBacked::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) { base->BatchWrite(cursor, block_hash); } std::unique_ptr CCoinsViewBacked::Cursor() const { return base->Cursor(); } size_t CCoinsViewBacked::EstimateSize() const { return base->EstimateSize(); } @@ -185,7 +185,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()); } @@ -201,7 +202,8 @@ uint256 CCoinsViewCache::GetBestBlock() const { return m_block_hash; } -void CCoinsViewCache::SetBestBlock(const uint256 &in_block_hash) { +void CCoinsViewCache::SetBestBlock(const uint256& in_block_hash) +{ m_block_hash = in_block_hash; } diff --git a/src/coins.h b/src/coins.h index 6cbcb5c9507..68d469383e1 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 &in_block_hash): block_hash(in_block_hash) {} + CCoinsViewCursor(const uint256& in_block_hash) : block_hash(in_block_hash) {} virtual ~CCoinsViewCursor() = default; virtual bool GetKey(COutPoint &key) const = 0; @@ -239,7 +239,7 @@ public: virtual void Next() = 0; //! Get best block at the time this cursor was created - const uint256 &GetBestBlock() const { return block_hash; } + const uint256& GetBestBlock() const { return block_hash; } private: uint256 block_hash; }; @@ -317,7 +317,7 @@ public: //! 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; //! Retrieve the block hash whose state this CCoinsView currently represents virtual uint256 GetBestBlock() const; @@ -347,16 +347,16 @@ public: class CCoinsViewBacked : public CCoinsView { protected: - CCoinsView *base; + CCoinsView* base; public: - CCoinsViewBacked(CCoinsView *in_view); + CCoinsViewBacked(CCoinsView* in_view); 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; std::vector GetHeadBlocks() const override; - void SetBackend(CCoinsView &in_view); + void SetBackend(CCoinsView& in_view); void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) override; std::unique_ptr Cursor() const override; size_t EstimateSize() const override; @@ -395,7 +395,7 @@ protected: virtual std::optional FetchCoinFromBase(const COutPoint& outpoint) const; public: - CCoinsViewCache(CCoinsView *in_base, 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,9 +405,9 @@ 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 &block_hash); + 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 +578,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/txdb.cpp b/src/txdb.cpp index b7f9fdf1bb2..592b739ba96 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -78,7 +78,8 @@ std::optional CCoinsViewDB::GetCoin(const COutPoint& outpoint) const return std::nullopt; } -bool CCoinsViewDB::HaveCoin(const COutPoint &outpoint) const { +bool CCoinsViewDB::HaveCoin(const COutPoint& outpoint) const +{ return m_db->Exists(CoinEntry(&outpoint)); } @@ -174,7 +175,7 @@ class CCoinsViewDBCursor: public CCoinsViewCursor public: // Prefer using CCoinsViewDB::Cursor() since we want to perform some // cache warmup on instantiation. - CCoinsViewDBCursor(CDBIterator* pcursorIn, const uint256&in_block_hash): + CCoinsViewDBCursor(CDBIterator* pcursorIn, const uint256& in_block_hash): CCoinsViewCursor(in_block_hash), pcursor(pcursorIn) {} ~CCoinsViewDBCursor() = default; diff --git a/src/txdb.h b/src/txdb.h index b8c06c2e3e0..70618ef547d 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -41,7 +41,7 @@ public: explicit CCoinsViewDB(DBParams db_params, CoinsViewOptions options); std::optional GetCoin(const COutPoint& outpoint) const override; - bool HaveCoin(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& block_hash) override; From 90c635c01cf046b843b6f9e89916319e555151c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sun, 15 Feb 2026 15:15:31 +0100 Subject: [PATCH 4/7] fuzz: keep backend assertions aligned to active backend `TestCoinsView` switches the `CCoinsViewCache` backend during fuzzing and then queries the backend for cross-checks. Pass the backend as a `CCoinsView*` (to make it relocatable) and retarget it when toggling between the original backend and a local empty `CCoinsView` so assertions always refer to the active backend. This will be switched to a singleton in the next commit. Note that the previous slice-assignment (`backend_coins_view = CCoinsView{}`) was a silent noop for the db target, it only copied base-class members (none) without changing the vtable, so the backend was never actually switched. The pointer approach makes the switch real for both targets, which revealed that when restoring the original backend after the empty one, the cache must be reset first to avoid carrying FRESH flags that were valid relative to the empty backend but invalid relative to the original (which may already contain those coins). Co-authored-by: Anthony Towns Co-authored-by: Andrew Toth Co-authored-by: Ryan Ofsky Co-authored-by: marcofleon --- src/test/fuzz/coins_view.cpp | 45 ++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp index 9ef9e47372e..5af5996fa6a 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, bool is_db) { bool good_data{true}; + auto* original_backend{backend_coins_view}; + CCoinsView coins_view_empty{}; 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 : &coins_view_empty; + 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. @@ -347,7 +352,7 @@ 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); + TestCoinsView(fuzzed_data_provider, coins_view_cache, &backend_coins_view, /*is_db=*/false); } FUZZ_TARGET(coins_view_db, .init = initialize_coins_view) @@ -360,7 +365,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, /*is_db=*/true); } // Creates a CoinsViewOverlay and a MutationGuardCoinsViewCache as the base. @@ -373,5 +378,5 @@ FUZZ_TARGET(coins_view_overlay, .init = initialize_coins_view) CCoinsView backend_base_coins_view; MutationGuardCoinsViewCache backend_cache{&backend_base_coins_view, /*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, /*is_db=*/false); } From b637566c8d08d4007c9413dbbb8171d20e3cfad5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sun, 15 Feb 2026 15:16:27 +0100 Subject: [PATCH 5/7] coins: add explicit `CoinsViewEmpty` noop backend Introduce `CoinsViewEmpty` as an explicit no-op `CCoinsView` implementation, and define its singleton accessor out of line in `coins.cpp` to avoid `-Wunique-object-duplication` in shared-library builds.` Use it at call sites that intentionally want a no-op backend instead of constructing anonymous placeholder views. `CCoinsViewTest` and `CoinsViewBottom` now inherit defaults from `CoinsViewEmpty` (e.g. the unused `EstimateSize()`, which now returns 0). Co-authored-by: Ryan Ofsky --- src/bench/ccoins_caching.cpp | 3 +-- src/bitcoin-tx.cpp | 3 +-- src/coins.cpp | 6 ++++++ src/coins.h | 12 ++++++++++++ src/node/psbt.cpp | 3 +-- src/rpc/rawtransaction.cpp | 5 ++--- src/test/coins_tests.cpp | 13 +++++-------- src/test/fuzz/coins_view.cpp | 18 ++++++++---------- src/test/fuzz/coinscache_sim.cpp | 7 +------ src/test/fuzz/transaction.cpp | 3 +-- src/test/script_p2sh_tests.cpp | 3 +-- src/test/sigopcount_tests.cpp | 3 +-- src/test/transaction_tests.cpp | 15 +++++---------- src/test/util/setup_common.cpp | 3 +-- src/validation.cpp | 20 ++++++++------------ 15 files changed, 54 insertions(+), 63 deletions(-) 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 87bbdcbfa9e..e5deef34348 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -41,6 +41,12 @@ void CCoinsViewBacked::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& b std::unique_ptr CCoinsViewBacked::Cursor() const { return base->Cursor(); } size_t CCoinsViewBacked::EstimateSize() const { return base->EstimateSize(); } +CoinsViewEmpty& CoinsViewEmpty::Get() +{ + static CoinsViewEmpty instance; + return instance; +} + std::optional CCoinsViewCache::PeekCoin(const COutPoint& outpoint) const { if (auto it{cacheCoins.find(outpoint)}; it != cacheCoins.end()) { diff --git a/src/coins.h b/src/coins.h index 68d469383e1..876c5ed8b20 100644 --- a/src/coins.h +++ b/src/coins.h @@ -342,6 +342,18 @@ public: virtual size_t EstimateSize() const { return 0; } }; +/** Noop coins view. */ +class CoinsViewEmpty : public CCoinsView +{ +protected: + CoinsViewEmpty() = default; + +public: + static CoinsViewEmpty& Get(); + + CoinsViewEmpty(const CoinsViewEmpty&) = delete; + CoinsViewEmpty& operator=(const CoinsViewEmpty&) = delete; +}; /** CCoinsView backed by another CCoinsView */ class CCoinsViewBacked : public CCoinsView 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 f8de7f5d3af..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 { @@ -677,8 +677,7 @@ public: } } - CCoinsView root; - CCoinsViewCacheTest base{&root}; + CCoinsViewCacheTest base{&CoinsViewEmpty::Get()}; CCoinsViewCacheTest cache{&base}; }; @@ -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 5af5996fa6a..c11581d2d3c 100644 --- a/src/test/fuzz/coins_view.cpp +++ b/src/test/fuzz/coins_view.cpp @@ -90,11 +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}; - CCoinsView coins_view_empty{}; if (is_db) coins_view_cache.SetBestBlock(uint256::ONE); COutPoint random_out_point; @@ -158,7 +158,7 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& co // Reset() clears the best block; db backends require a non-null hash. if (is_db) coins_view_cache.SetBestBlock(uint256::ONE); } - backend_coins_view = use_original_backend ? original_backend : &coins_view_empty; + backend_coins_view = use_original_backend ? original_backend : &CoinsViewEmpty::Get(); coins_view_cache.SetBackend(*backend_coins_view); }, [&] { @@ -350,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) @@ -365,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. @@ -375,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..238281321f5 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)) { 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/validation.cpp b/src/validation.cpp index 2b0de23db3b..e15afd6c1f9 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()); From 86296f276d10762d1db2d3a919492d2b34f6a72b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Fri, 20 Feb 2026 14:19:58 +0100 Subject: [PATCH 6/7] coins: make `CCoinsView` methods pure virtual `CCoinsView` provided default no-op implementations, which allowed constructing a bare view and silently getting dummy behavior. Make all interface methods pure virtual and remove the legacy default definitions from `coins.cpp` so callers must choose an explicit implementation. Move the virtual destructor to the beginning to avoid mixing it between the methods. No-op backing behavior remains available via `CoinsViewEmpty`. --- src/coins.cpp | 16 ------------- src/coins.h | 40 +++++++++++++++++++++----------- src/test/fuzz/coinscache_sim.cpp | 2 +- src/txdb.cpp | 5 ++++ src/txdb.h | 1 + src/txmempool.h | 2 +- 6 files changed, 34 insertions(+), 32 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index e5deef34348..2eadfde95d6 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -14,22 +14,6 @@ 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& block_hash) -{ - for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) { } -} - -std::unique_ptr CCoinsView::Cursor() const { return nullptr; } - -bool CCoinsView::HaveCoin(const COutPoint& outpoint) const -{ - return GetCoin(outpoint).has_value(); -} - CCoinsViewBacked::CCoinsViewBacked(CCoinsView* in_view) : base(in_view) { } std::optional CCoinsViewBacked::GetCoin(const COutPoint& outpoint) const { return base->GetCoin(outpoint); } std::optional CCoinsViewBacked::PeekCoin(const COutPoint& outpoint) const { return base->PeekCoin(outpoint); } diff --git a/src/coins.h b/src/coins.h index 876c5ed8b20..c18f7e10f93 100644 --- a/src/coins.h +++ b/src/coins.h @@ -303,43 +303,43 @@ 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& block_hash); + 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. */ @@ -353,6 +353,18 @@ public: 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 */ diff --git a/src/test/fuzz/coinscache_sim.cpp b/src/test/fuzz/coinscache_sim.cpp index 238281321f5..9d41a6c058b 100644 --- a/src/test/fuzz/coinscache_sim.cpp +++ b/src/test/fuzz/coinscache_sim.cpp @@ -247,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/txdb.cpp b/src/txdb.cpp index 592b739ba96..a098faa44bc 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -78,6 +78,11 @@ std::optional CCoinsViewDB::GetCoin(const COutPoint& outpoint) const return std::nullopt; } +std::optional CCoinsViewDB::PeekCoin(const COutPoint& outpoint) const +{ + return GetCoin(outpoint); +} + bool CCoinsViewDB::HaveCoin(const COutPoint& outpoint) const { return m_db->Exists(CoinEntry(&outpoint)); diff --git a/src/txdb.h b/src/txdb.h index 70618ef547d..b19b312a4b6 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -41,6 +41,7 @@ public: explicit CCoinsViewDB(DBParams db_params, CoinsViewOptions options); 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; 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. */ From 8783cc8056db3adcec85be01683d716aea4d5130 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Fri, 20 Feb 2026 15:41:43 +0100 Subject: [PATCH 7/7] refactor: inline `CCoinsViewBacked` implementation `CCoinsViewBacked` is a simple delegating wrapper around another `CCoinsView`. Inline its one-line overrides in `coins.h` so the view hierarchy can be read without jumping between `coins.h` and `coins.cpp`. --- src/coins.cpp | 11 ----------- src/coins.h | 22 ++++++++++++---------- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 2eadfde95d6..c403e006c85 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -14,17 +14,6 @@ TRACEPOINT_SEMAPHORE(utxocache, add); TRACEPOINT_SEMAPHORE(utxocache, spent); TRACEPOINT_SEMAPHORE(utxocache, uncache); -CCoinsViewBacked::CCoinsViewBacked(CCoinsView* in_view) : base(in_view) { } -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& in_view) { base = &in_view; } -void CCoinsViewBacked::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) { base->BatchWrite(cursor, block_hash); } -std::unique_ptr CCoinsViewBacked::Cursor() const { return base->Cursor(); } -size_t CCoinsViewBacked::EstimateSize() const { return base->EstimateSize(); } - CoinsViewEmpty& CoinsViewEmpty::Get() { static CoinsViewEmpty instance; diff --git a/src/coins.h b/src/coins.h index c18f7e10f93..e1f1bee757c 100644 --- a/src/coins.h +++ b/src/coins.h @@ -374,16 +374,18 @@ protected: CCoinsView* base; public: - CCoinsViewBacked(CCoinsView* in_view); - 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& in_view); - void BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash) 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(); } };