coins: assume GetCoin only returns unspent coins

`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 <pap.lorinc@gmail.com>
This commit is contained in:
Andrew Toth 2026-01-03 20:28:55 +01:00 committed by Lőrinc
parent eec551aaf1
commit 2ee7f9b259
No known key found for this signature in database
GPG Key ID: 669FFF0FFA477A76
2 changed files with 11 additions and 22 deletions

View File

@ -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);
}

View File

@ -102,7 +102,6 @@ using CoinsCachePair = std::pair<const COutPoint, CCoinsCacheEntry>;
* - 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};