coins: make `CCoinsView` methods pure virtual
What changed, and why it matters
This commit tightens up a key internal interface in Bitcoin Core so that developers can no longer accidentally create a 'dummy' version of the coin database view that silently does nothing. It is a defensive code-quality change, not a fix for an active security bug. The change makes the interface 'pure virtual,' meaning every real implementation must explicitly define all required behavior, and moves the no-op behavior into a clearly named 'CoinsViewEmpty' helper. It also adds a missing explicit implementation for one database-backed view (PeekCoin in CCoinsViewDB).
No urgent action required. Treat as normal code-hardening/maintenance. Review downstream subclasses to ensure all pure virtual methods are implemented; the patch already updates CCoinsViewDB. If backporting, verify that any project-specific subclasses of CCoinsView also implement the now-pure-virtual methods.
Security signals we found
Defensive hardening: interface methods made pure virtual to prevent accidental instantiation of a no-op view
Silent dummy behavior removed from base class and isolated into an explicitly named CoinsViewEmpty
Missing override added: CCoinsViewDB::PeekCoin now explicitly implemented
Comment clarification in CCoinsViewMemPool about base-view cache population
Evidence from the diff
CCoinsView previously provided concrete default no-op implementations for GetCoin, PeekCoin, HaveCoin, GetBestBlock, GetHeadBlocks, BatchWrite, Cursor, and EstimateSize. This allowed a bare CCoinsView instance to be constructed and used, producing silent dummy behavior. The patch makes all of these methods pure virtual (= 0), removes the default definitions from coins.cpp, and moves the virtual destructor to the top of the class. The legacy no-op behavior is preserved in a new explicit CoinsViewEmpty class. CCoinsViewDB gains an explicit PeekCoin override that delegates to GetCoin. A comment in CCoinsViewMemPool is clarified to note that GetCoin may populate the base view on cache misses. The fuzz target comment is updated only cosmetically.
Changed components
src/coins.h (CCoinsView interface, new CoinsViewEmpty)src/coins.cpp (removed default CCoinsView method bodies)src/txdb.h / src/txdb.cpp (CCoinsViewDB::PeekCoin override)src/txmempool.h (CCoinsViewMemPool comment)src/test/fuzz/coinscache_sim.cpp (comment only)Inspect captured patch +34 / −32
diff --git a/src/coins.cpp b/src/coins.cpp
index e5deef34..2eadfde9 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<Coin> CCoinsView::GetCoin(const COutPoint& outpoint) const { return std::nullopt; }
-std::optional<Coin> CCoinsView::PeekCoin(const COutPoint& outpoint) const { return GetCoin(outpoint); }
-uint256 CCoinsView::GetBestBlock() const { return uint256(); }
-std::vector<uint256> CCoinsView::GetHeadBlocks() const { return std::vector<uint256>(); }
-void CCoinsView::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& block_hash)
-{
- for (auto it{cursor.Begin()}; it != cursor.End(); it = cursor.NextAndMaybeErase(*it)) { }
-}
-
-std::unique_ptr<CCoinsViewCursor> 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<Coin> CCoinsViewBacked::GetCoin(const COutPoint& outpoint) const { return base->GetCoin(outpoint); }
std::optional<Coin> CCoinsViewBacked::PeekCoin(const COutPoint& outpoint) const { return base->PeekCoin(outpoint); }
diff --git a/src/coins.h b/src/coins.h
index 876c5ed8..c18f7e10 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<Coin> GetCoin(const COutPoint& outpoint) const;
+ virtual std::optional<Coin> 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<Coin> PeekCoin(const COutPoint& outpoint) const;
+ virtual std::optional<Coin> 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<uint256> GetHeadBlocks() const;
+ virtual std::vector<uint256> 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<CCoinsViewCursor> Cursor() const;
+ //! Get a cursor to iterate over the whole state. Implementations may return nullptr.
+ virtual std::unique_ptr<CCoinsViewCursor> 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<Coin> GetCoin(const COutPoint&) const override { return {}; }
+ std::optional<Coin> 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<uint256> 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<CCoinsViewCursor> 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 23828132..9d41a6c0 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<uint32_t>(0, NUM_OUTPOINTS - 1);
// Look up in simulation data.
auto sim = lookup(outpointidx);
diff --git a/src/txdb.cpp b/src/txdb.cpp
index 592b739b..a098faa4 100644
--- a/src/txdb.cpp
+++ b/src/txdb.cpp
@@ -78,6 +78,11 @@ std::optional<Coin> CCoinsViewDB::GetCoin(const COutPoint& outpoint) const
return std::nullopt;
}
+std::optional<Coin> 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 70618ef5..b19b312a 100644
--- a/src/txdb.h
+++ b/src/txdb.h
@@ -41,6 +41,7 @@ public:
explicit CCoinsViewDB(DBParams db_params, CoinsViewOptions options);
std::optional<Coin> GetCoin(const COutPoint& outpoint) const override;
+ std::optional<Coin> PeekCoin(const COutPoint& outpoint) const override;
bool HaveCoin(const COutPoint& outpoint) const override;
uint256 GetBestBlock() const override;
std::vector<uint256> GetHeadBlocks() const override;
diff --git a/src/txmempool.h b/src/txmempool.h
index d172c78e..c4723f89 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<Coin> 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. */
Why this scored 18/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.