Page MenuHomePhabricator

coins/refactor: enforce GetCoin() returns only unspent coins
ClosedPublic

Authored by PiRK on Jun 11 2026, 14:28.

Details

Summary

txdb: assert CCoinsViewDB::GetCoin only returns unspent coins

The chainstate UTXO database only stores unspent outputs; spent entries are removed.

Assert after reading a Coin so corruption or misuse cannot propagate a spent coin through the GetCoin() interface.

test: do not return spent coins from CCoinsViewTest::GetCoin

Production GetCoin() implementations only return unspent coins.

Update the CCoinsView test backend to match that contract, so tests stop exercising cache states that cannot occur with CCoinsViewCache or CCoinsViewDB.

fuzz: keep coinscache_sim backend free of spent coins

CoinsViewBottom roughly simulates a memory-backed CCoinsViewDB, which never stores spent coins.

Stop returning spent coins from GetCoin(), erase spent entries in BatchWrite(), and tighten comparisons to expect std::nullopt when the simulator has no coin.

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 is a backport of core#34207
https://github.com/bitcoin/bitcoin/pull/34207/changes/2ee7f9b259059d59e127852ea898b58183604b46

Test Plan

ninja all check check-functional-extended

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK requested review of this revision.Jun 11 2026, 14:28
Fabien requested changes to this revision.Jun 11 2026, 15:04
Fabien added a subscriber: Fabien.

We still have a GetSpentCoins function that relied on GetCoin, how does this affect this behavior?

This revision now requires changes to proceed.Jun 11 2026, 15:04

We still have a GetSpentCoins function that relied on GetCoin, how does this affect this behavior?

AFAICT we mainly use GetSpentCoins for the TransactionAddedToMempool signal, which happens before the coins are considered "spent" by the mempool coins view.

I'm still trying to understand the state of the coins view in net_processing in the avalanche invalidation case. I wonder if moving the GetSpentCoins call before m_mempool.removeRecursive in D20102 wasn't a mistake.

PiRK requested review of this revision.Jun 12 2026, 10:09

The current behavior is unchanged, the mempool coin view keeps all new txos until the transactions are no longer in the mempool, and the underlying db does not consider the coins spent until they are mined..

This revision is now accepted and ready to land.Jun 12 2026, 10:51