coins: Keep track of number of dirty entries in `CCoinsViewCache`
What changed, and why it matters
This Bitcoin Core change adds an internal counter that tracks how many unspent-coin cache entries are 'dirty' (modified but not yet saved to disk). It is a correctness and accounting improvement, not a fix for an active security flaw. The change helps flush warnings and disk-space checks reflect reality after a recent feature that allows partial, non-wiping flushes. No exploit or vulnerability is described in the commit.
No security action required. Treat as normal code-quality/refactoring review. If auditing, verify that m_dirty_count is correctly maintained across all SetDirty/erase/clear paths and that SanityCheck() catches inconsistencies in debug builds.
Security signals we found
Internal cache accounting change with no consensus or network impact
Adds Assume(m_dirty_count == 0) after Flush/Sync and assert in SanityCheck
No mention of vulnerability, exploit, bug, crash, or DoS in commit message or diff
Test-only additions verify dirty count invariants
Evidence from the diff
The commit introduces m_dirty_count to CCoinsViewCache and threads it through CoinsViewCacheCursor. It increments when entries are marked dirty, decrements on removal/cleaning, and is asserted to match the linked-list count in SanityCheck(). The motivation is that after non-wiping flushes (#28280/#28233), total cache size is a poor proxy for dirty entries, so flush-trigger logic needs an accurate dirty count. The diff is purely internal accounting; it does not change consensus rules, network behavior, or cryptographic handling.
Changed components
src/coins.cppsrc/coins.hsrc/test/coins_tests.cppsrc/test/fuzz/coins_view.cppInspect captured patch +48 / −11
diff --git a/src/coins.cpp b/src/coins.cpp
index fc33c521..1bc01d20 100644
--- a/src/coins.cpp
+++ b/src/coins.cpp
@@ -98,10 +98,12 @@ void CCoinsViewCache::AddCoin(const COutPoint &outpoint, Coin&& coin, bool possi
fresh = !it->second.IsDirty();
}
if (!inserted) {
+ m_dirty_count -= it->second.IsDirty();
cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
}
it->second.coin = std::move(coin);
CCoinsCacheEntry::SetDirty(*it, m_sentinel);
+ ++m_dirty_count;
if (fresh) CCoinsCacheEntry::SetFresh(*it, m_sentinel);
cachedCoinsUsage += it->second.coin.DynamicMemoryUsage();
TRACEPOINT(utxocache, add,
@@ -117,6 +119,7 @@ void CCoinsViewCache::EmplaceCoinInternalDANGER(COutPoint&& outpoint, Coin&& coi
auto [it, inserted] = cacheCoins.try_emplace(std::move(outpoint), std::move(coin));
if (inserted) {
CCoinsCacheEntry::SetDirty(*it, m_sentinel);
+ ++m_dirty_count;
cachedCoinsUsage += mem_usage;
}
}
@@ -135,6 +138,7 @@ void AddCoins(CCoinsViewCache& cache, const CTransaction &tx, int nHeight, bool
bool CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) {
CCoinsMap::iterator it = FetchCoin(outpoint);
if (it == cacheCoins.end()) return false;
+ m_dirty_count -= it->second.IsDirty();
cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage();
TRACEPOINT(utxocache, spent,
outpoint.hash.data(),
@@ -149,6 +153,7 @@ bool CCoinsViewCache::SpendCoin(const COutPoint &outpoint, Coin* moveout) {
cacheCoins.erase(it);
} else {
CCoinsCacheEntry::SetDirty(*it, m_sentinel);
+ ++m_dirty_count;
it->second.coin.Clear();
}
return true;
@@ -207,8 +212,9 @@ void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& ha
} else {
entry.coin = it->second.coin;
}
- cachedCoinsUsage += entry.coin.DynamicMemoryUsage();
CCoinsCacheEntry::SetDirty(*itUs, m_sentinel);
+ ++m_dirty_count;
+ cachedCoinsUsage += entry.coin.DynamicMemoryUsage();
// We can mark it FRESH in the parent if it was FRESH in the child
// Otherwise it might have just been flushed from the parent's cache
// and already exist in the grandparent
@@ -227,6 +233,7 @@ void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& ha
if (itUs->second.IsFresh() && it->second.coin.IsSpent()) {
// The grandparent cache does not have an entry, and the coin
// has been spent. We can just delete it from the parent cache.
+ m_dirty_count -= itUs->second.IsDirty();
cachedCoinsUsage -= itUs->second.coin.DynamicMemoryUsage();
cacheCoins.erase(itUs);
} else {
@@ -240,7 +247,10 @@ void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& ha
itUs->second.coin = it->second.coin;
}
cachedCoinsUsage += itUs->second.coin.DynamicMemoryUsage();
- CCoinsCacheEntry::SetDirty(*itUs, m_sentinel);
+ if (!itUs->second.IsDirty()) {
+ CCoinsCacheEntry::SetDirty(*itUs, m_sentinel);
+ ++m_dirty_count;
+ }
// NOTE: It isn't safe to mark the coin as FRESH in the parent
// cache. If it already existed and was spent in the parent
// cache then marking it FRESH would prevent that spentness
@@ -253,8 +263,9 @@ void CCoinsViewCache::BatchWrite(CoinsViewCacheCursor& cursor, const uint256& ha
void CCoinsViewCache::Flush(bool reallocate_cache)
{
- auto cursor{CoinsViewCacheCursor(m_sentinel, cacheCoins, /*will_erase=*/true)};
+ auto cursor{CoinsViewCacheCursor(m_dirty_count, m_sentinel, cacheCoins, /*will_erase=*/true)};
base->BatchWrite(cursor, hashBlock);
+ Assume(m_dirty_count == 0);
cacheCoins.clear();
if (reallocate_cache) {
ReallocateCache();
@@ -264,8 +275,9 @@ void CCoinsViewCache::Flush(bool reallocate_cache)
void CCoinsViewCache::Sync()
{
- auto cursor{CoinsViewCacheCursor(m_sentinel, cacheCoins, /*will_erase=*/false)};
+ auto cursor{CoinsViewCacheCursor(m_dirty_count, m_sentinel, cacheCoins, /*will_erase=*/false)};
base->BatchWrite(cursor, hashBlock);
+ Assume(m_dirty_count == 0);
if (m_sentinel.second.Next() != &m_sentinel) {
/* BatchWrite must clear flags of all entries */
throw std::logic_error("Not all unspent flagged entries were cleared");
@@ -276,6 +288,7 @@ void CCoinsViewCache::Reset() noexcept
{
cacheCoins.clear();
cachedCoinsUsage = 0;
+ m_dirty_count = 0;
SetBestBlock(uint256::ZERO);
}
@@ -348,7 +361,7 @@ void CCoinsViewCache::SanityCheck() const
// Count the number of entries actually in the list.
++count_linked;
}
- assert(count_linked == count_dirty);
+ assert(count_dirty == count_linked && count_dirty == m_dirty_count);
assert(recomputed_usage == cachedCoinsUsage);
}
diff --git a/src/coins.h b/src/coins.h
index 4b39c0ba..9efd23c5 100644
--- a/src/coins.h
+++ b/src/coins.h
@@ -265,10 +265,11 @@ struct CoinsViewCacheCursor
//! This is an optimization compared to erasing all entries as the cursor iterates them when will_erase is set.
//! Calling CCoinsMap::clear() afterwards is faster because a CoinsCachePair cannot be coerced back into a
//! CCoinsMap::iterator to be erased, and must therefore be looked up again by key in the CCoinsMap before being erased.
- CoinsViewCacheCursor(CoinsCachePair& sentinel LIFETIMEBOUND,
+ CoinsViewCacheCursor(size_t& dirty_count LIFETIMEBOUND,
+ CoinsCachePair& sentinel LIFETIMEBOUND,
CCoinsMap& map LIFETIMEBOUND,
bool will_erase) noexcept
- : m_sentinel(sentinel), m_map(map), m_will_erase(will_erase) {}
+ : m_dirty_count(dirty_count), m_sentinel(sentinel), m_map(map), m_will_erase(will_erase) {}
inline CoinsCachePair* Begin() const noexcept { return m_sentinel.second.Next(); }
inline CoinsCachePair* End() const noexcept { return &m_sentinel; }
@@ -277,6 +278,7 @@ struct CoinsViewCacheCursor
inline CoinsCachePair* NextAndMaybeErase(CoinsCachePair& current) noexcept
{
const auto next_entry{current.second.Next()};
+ m_dirty_count -= current.second.IsDirty();
// If we are not going to erase the cache, we must still erase spent entries.
// Otherwise, clear the state of the entry.
if (!m_will_erase) {
@@ -292,6 +294,7 @@ struct CoinsViewCacheCursor
inline bool WillErase(CoinsCachePair& current) const noexcept { return m_will_erase || current.second.coin.IsSpent(); }
private:
+ size_t& m_dirty_count;
CoinsCachePair& m_sentinel;
CCoinsMap& m_map;
bool m_will_erase;
@@ -369,6 +372,8 @@ protected:
/* Cached dynamic memory usage for the inner Coin objects. */
mutable size_t cachedCoinsUsage{0};
+ /* Running count of dirty Coin cache entries. */
+ mutable size_t m_dirty_count{0};
/**
* Discard all modifications made to this cache without flushing to the base view.
@@ -458,9 +463,12 @@ public:
*/
void Uncache(const COutPoint &outpoint);
- //! Calculate the size of the cache (in number of transaction outputs)
+ //! Size of the cache (in number of transaction outputs)
unsigned int GetCacheSize() const;
+ //! Number of dirty cache entries (transaction outputs)
+ size_t GetDirtyCount() const noexcept { return m_dirty_count; }
+
//! Calculate the size of the cache (in bytes)
size_t DynamicMemoryUsage() const;
diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp
index d5219fce..92935ba6 100644
--- a/src/test/coins_tests.cpp
+++ b/src/test/coins_tests.cpp
@@ -95,6 +95,7 @@ public:
CCoinsMap& map() const { return cacheCoins; }
CoinsCachePair& sentinel() const { return m_sentinel; }
size_t& usage() const { return cachedCoinsUsage; }
+ size_t& dirty() const { return m_dirty_count; }
};
} // namespace
@@ -656,8 +657,10 @@ static void WriteCoinsViewEntry(CCoinsView& view, const MaybeCoin& cache_coin)
CCoinsMapMemoryResource resource;
CCoinsMap map{0, CCoinsMap::hasher{}, CCoinsMap::key_equal{}, &resource};
if (cache_coin) InsertCoinsMapEntry(map, sentinel, *cache_coin);
- auto cursor{CoinsViewCacheCursor(sentinel, map, /*will_erase=*/true)};
+ size_t dirty_count{cache_coin && cache_coin->IsDirty()};
+ auto cursor{CoinsViewCacheCursor(dirty_count, sentinel, map, /*will_erase=*/true)};
view.BatchWrite(cursor, {});
+ BOOST_CHECK_EQUAL(dirty_count, 0U);
}
class SingleEntryCacheTest
@@ -667,7 +670,10 @@ public:
{
auto base_cache_coin{base_value == ABSENT ? MISSING : CoinEntry{base_value, CoinEntry::State::DIRTY}};
WriteCoinsViewEntry(base, base_cache_coin);
- if (cache_coin) cache.usage() += InsertCoinsMapEntry(cache.map(), cache.sentinel(), *cache_coin);
+ if (cache_coin) {
+ cache.usage() += InsertCoinsMapEntry(cache.map(), cache.sentinel(), *cache_coin);
+ cache.dirty() += cache_coin->IsDirty();
+ }
}
CCoinsView root;
@@ -1128,6 +1134,7 @@ BOOST_AUTO_TEST_CASE(ccoins_reset_guard)
const Coin coin{CTxOut{m_rng.randrange(10), CScript{} << m_rng.randbytes(CScriptBase::STATIC_SIZE + 1)}, 1, false};
cache.EmplaceCoinInternalDANGER(COutPoint{outpoint}, Coin{coin});
+ BOOST_CHECK_EQUAL(cache.GetDirtyCount(), 1U);
uint256 cache_best_block{m_rng.rand256()};
cache.SetBestBlock(cache_best_block);
@@ -1137,12 +1144,14 @@ BOOST_AUTO_TEST_CASE(ccoins_reset_guard)
BOOST_CHECK(cache.AccessCoin(outpoint) == coin);
BOOST_CHECK(!cache.AccessCoin(outpoint).IsSpent());
BOOST_CHECK_EQUAL(cache.GetCacheSize(), 1);
+ BOOST_CHECK_EQUAL(cache.GetDirtyCount(), 1);
BOOST_CHECK_EQUAL(cache.GetBestBlock(), cache_best_block);
BOOST_CHECK(!root_cache.HaveCoinInCache(outpoint));
}
BOOST_CHECK(cache.AccessCoin(outpoint).IsSpent());
BOOST_CHECK_EQUAL(cache.GetCacheSize(), 0);
+ BOOST_CHECK_EQUAL(cache.GetDirtyCount(), 0);
BOOST_CHECK_EQUAL(cache.GetBestBlock(), base_best_block);
BOOST_CHECK(!root_cache.HaveCoinInCache(outpoint));
@@ -1153,8 +1162,13 @@ BOOST_AUTO_TEST_CASE(ccoins_reset_guard)
BOOST_CHECK(cache.AccessCoin(outpoint).IsSpent());
BOOST_CHECK_EQUAL(cache.GetCacheSize(), 0);
+ BOOST_CHECK_EQUAL(cache.GetDirtyCount(), 0U);
BOOST_CHECK_EQUAL(cache.GetBestBlock(), base_best_block);
BOOST_CHECK(!root_cache.HaveCoinInCache(outpoint));
+
+ // Flush should be a no-op after reset.
+ cache.Flush();
+ BOOST_CHECK_EQUAL(cache.GetDirtyCount(), 0U);
}
BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp
index 699a45e2..ec10dbb3 100644
--- a/src/test/fuzz/coins_view.cpp
+++ b/src/test/fuzz/coins_view.cpp
@@ -139,6 +139,7 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend
[&] {
CoinsCachePair sentinel{};
sentinel.second.SelfRef(sentinel);
+ size_t dirty_count{0};
CCoinsMapMemoryResource resource;
CCoinsMap coins_map{0, SaltedOutpointHasher{/*deterministic=*/true}, CCoinsMap::key_equal{}, &resource};
LIMITED_WHILE(good_data && fuzzed_data_provider.ConsumeBool(), 10'000)
@@ -159,10 +160,11 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend
auto it{coins_map.emplace(random_out_point, std::move(coins_cache_entry)).first};
if (dirty) CCoinsCacheEntry::SetDirty(*it, sentinel);
if (fresh) CCoinsCacheEntry::SetFresh(*it, sentinel);
+ dirty_count += dirty;
}
bool expected_code_path = false;
try {
- auto cursor{CoinsViewCacheCursor(sentinel, coins_map, /*will_erase=*/true)};
+ auto cursor{CoinsViewCacheCursor(dirty_count, sentinel, coins_map, /*will_erase=*/true)};
uint256 best_block{coins_view_cache.GetBestBlock()};
if (fuzzed_data_provider.ConsumeBool()) best_block = ConsumeUInt256(fuzzed_data_provider);
// Set best block hash to non-null to satisfy the assertion in CCoinsViewDB::BatchWrite().
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.