From 2ee7f9b259059d59e127852ea898b58183604b46 Mon Sep 17 00:00:00 2001 From: Andrew Toth Date: Sat, 3 Jan 2026 20:28:55 +0100 Subject: [PATCH] coins: assume `GetCoin` only returns unspent coins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `CCoinsViewCache::FetchCoin()` had special handling for a spent `Coin` returned by the parent view. Production parents (`CCoinsViewCache` and `CCoinsViewDB`) do not return spent coins, so this path is unreachable. Replace it with an `Assume(!coin.IsSpent())`, drop outdated documentation about spent+FRESH cache entries, and simplify `SanityCheck()` to assert the remaining possible state invariants. This is safe because it does not change behavior for valid backends and will fail fast if the `GetCoin()` contract is violated. Co-authored-by: Lőrinc --- src/coins.cpp | 26 +++++++++++--------------- src/coins.h | 7 ------- 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/src/coins.cpp b/src/coins.cpp index 7f2ffc38efa..ccd6d605ba6 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -55,10 +55,7 @@ CCoinsMap::iterator CCoinsViewCache::FetchCoin(const COutPoint &outpoint) const if (auto coin{base->GetCoin(outpoint)}) { ret->second.coin = std::move(*coin); cachedCoinsUsage += ret->second.coin.DynamicMemoryUsage(); - if (ret->second.coin.IsSpent()) { // TODO GetCoin cannot return spent coins - // The parent only has an empty entry for this outpoint; we can consider our version as fresh. - CCoinsCacheEntry::SetFresh(*ret, m_sentinel); - } + Assert(!ret->second.coin.IsSpent()); } else { cacheCoins.erase(ret); return cacheCoins.end(); @@ -277,7 +274,7 @@ void CCoinsViewCache::Sync() void CCoinsViewCache::Uncache(const COutPoint& hash) { CCoinsMap::iterator it = cacheCoins.find(hash); - if (it != cacheCoins.end() && !it->second.IsDirty() && !it->second.IsFresh()) { + if (it != cacheCoins.end() && !it->second.IsDirty()) { cachedCoinsUsage -= it->second.coin.DynamicMemoryUsage(); TRACEPOINT(utxocache, uncache, hash.hash.data(), @@ -318,20 +315,19 @@ void CCoinsViewCache::ReallocateCache() void CCoinsViewCache::SanityCheck() const { size_t recomputed_usage = 0; - size_t count_flagged = 0; + size_t count_dirty = 0; for (const auto& [_, entry] : cacheCoins) { - unsigned attr = 0; - if (entry.IsDirty()) attr |= 1; - if (entry.IsFresh()) attr |= 2; - if (entry.coin.IsSpent()) attr |= 4; - // Only 5 combinations are possible. - assert(attr != 2 && attr != 4 && attr != 7); + if (entry.coin.IsSpent()) { + assert(entry.IsDirty() && !entry.IsFresh()); // A spent coin must be dirty and cannot be fresh + } else { + assert(entry.IsDirty() || !entry.IsFresh()); // An unspent coin must not be fresh if not dirty + } // Recompute cachedCoinsUsage. recomputed_usage += entry.coin.DynamicMemoryUsage(); // Count the number of entries we expect in the linked list. - if (entry.IsDirty() || entry.IsFresh()) ++count_flagged; + if (entry.IsDirty()) ++count_dirty; } // Iterate over the linked list of flagged entries. size_t count_linked = 0; @@ -340,11 +336,11 @@ void CCoinsViewCache::SanityCheck() const assert(it->second.Next()->second.Prev() == it); assert(it->second.Prev()->second.Next() == it); // Verify they are actually flagged. - assert(it->second.IsDirty() || it->second.IsFresh()); + assert(it->second.IsDirty()); // Count the number of entries actually in the list. ++count_linked; } - assert(count_linked == count_flagged); + assert(count_linked == count_dirty); assert(recomputed_usage == cachedCoinsUsage); } diff --git a/src/coins.h b/src/coins.h index 6da53829996..66d712be34d 100644 --- a/src/coins.h +++ b/src/coins.h @@ -102,7 +102,6 @@ using CoinsCachePair = std::pair; * - unspent, FRESH, DIRTY (e.g. a new coin created in the cache) * - unspent, not FRESH, DIRTY (e.g. a coin changed in the cache during a reorg) * - unspent, not FRESH, not DIRTY (e.g. an unspent coin fetched from the parent cache) - * - spent, FRESH, not DIRTY (e.g. a spent coin fetched from the parent cache) * - spent, not FRESH, DIRTY (e.g. a coin is spent and spentness needs to be flushed to the parent) */ struct CCoinsCacheEntry @@ -117,12 +116,6 @@ private: * the parent cache for batch writing. This is a performance optimization * compared to giving all entries in the cache to the parent and having the * parent scan for only modified entries. - * - * FRESH-but-not-DIRTY coins can not occur in practice, since that would - * mean a spent coin exists in the parent CCoinsView and not in the child - * CCoinsViewCache. Nevertheless, if a spent coin is retrieved from the - * parent cache, the FRESH-but-not-DIRTY coin will be tracked by the linked - * list and deleted when Sync or Flush is called on the CCoinsViewCache. */ CoinsCachePair* m_prev{nullptr}; CoinsCachePair* m_next{nullptr};