coins: make cache freshness imply dirtiness and remove invalid test states #34864

1. 
   l0rinc commented at 12:01 pm on March 19, 2026: contributor

   Problem

   The coins cache implementation still carries logic and representation for states that production code does not create, especially standalone fresh entries and BatchWrite() inputs that are not dirty. The unit tests and fuzz harnesses were also constructing those states directly.

   That reduces confidence in the results: instead of checking realistic cache behavior, parts of the test suite are checking behavior around impossible states. It also becomes a recurring source of test-only failures whenever production invariants are tightened.

   Fix

   This series first makes the production invariant explicit by setting freshness through the same method that marks entries dirty. That makes the dependency clear at the call sites instead of relying on a separate freshness step.

   With that in place, the series updates the tests so they stop constructing fresh-only states before the stricter BatchWrite() checks land. It then asserts that every cache entry reaching BatchWrite() is dirty, rejects spent-and-fresh BatchWrite() entries as logic errors, removes the bare freshness helper, and simplifies the internal representation so dirtiness is derived from linked-list membership instead of a stored flag bit.

   The final step makes the dirty-marking API take the resulting freshness explicitly. That avoids callers accidentally preserving an old fresh state when they mean to clear it.

   Notes

   This revives Andrew’s earlier work in #30673 and #33018.

   A spent-and-fresh entry is not just unusual, it is inconsistent with how the cache works. If a fresh entry is spent, it should be erased locally because the parent view never knew it existed. Writing it upward as spent is therefore a logic error, and the series makes that boundary explicit in BatchWrite().

   Another subtle point is that making freshness explicit changes the contract of the dirty-marking helper. The caller must pass the resulting freshness of the entry, not just whether the current operation introduced freshness. That matters in paths where an entry may already be fresh before the current update.

   Finally, once fresh-only states are removed from tests, dirtiness can be derived from linked-list membership without losing information. That lets the cache entry store only whether an entry is fresh, while the linked list itself becomes the source of truth for whether an entry is dirty.

2. coins: pass freshness through `SetDirty()` in production

   Production code already knows when a dirty entry should also be fresh.
   Thread that state through `SetDirty(..., fresh)` in `AddCoin()` and the inserted-entry path in `BatchWrite()` instead of setting `FRESH` in a second step.
   
   This sets up the later cleanup, making the production invariant explicit before the tests are adjusted, and shows that bare `FRESH` entries are only coming from test code.
   
   Co-authored-by: Lőrinc <pap.lorinc@gmail.com>

   745d974961
3. test: remove fresh-only states from `coins_tests`

   Production code no longer introduces standalone `FRESH` entries.
   Remove the `State::FRESH` cases from `coins_tests.cpp` so the unit tests stop constructing cache states that production code cannot reach.
   
   This keeps the test helper aligned with the reachable cache states before the following `BatchWrite()` cleanup asserts that every cursor entry is dirty.
   
   Co-authored-by: Lőrinc <pap.lorinc@gmail.com>

   1fd542f28a
4. coins: assume `BatchWrite()` cursor entries are `DIRTY`

   Now that production code no longer creates fresh-only linked-list entries, every cursor entry reaching `BatchWrite()` is already dirty.
   Replace the old skip logic with assertions and unconditional processing in the cache, db, test, and fuzz backends.
   
   Simplify the coin DB log line to match the new invariant.
   
   Co-authored-by: Lőrinc <pap.lorinc@gmail.com>

   c4ddb1cebb
5. coins: reject spent `FRESH` entries in `BatchWrite()`

   A spent `FRESH` entry is the remaining impossible `BatchWrite()` state.
   Spending a fresh entry erases it instead of writing it to the parent, so `CCoinsViewCache::BatchWrite()` should treat that case as a logic error.
   
   Update the `ccoins_write` table to cover the failure explicitly.
   
   Co-authored-by: Lőrinc <pap.lorinc@gmail.com>

   e78784a0bc
6. coins: remove the bare `SetFresh()` helper

   The earlier commits make the production invariant explicit and enforce the impossible `BatchWrite()` states.
   Remove the bare `SetFresh()` helper and update the unit tests and the `coins_view` fuzz harness to use `SetDirty(..., fresh)` directly.
   
   This drops the remaining fresh-only test states so later cleanups can stop modeling a standalone `FRESH` flag.
   
   Co-authored-by: Lőrinc <pap.lorinc@gmail.com>

   3f025fdc8e
7. coins: inline `AddFlags()` into `SetDirty()`

   The next commit changes how `CCoinsCacheEntry` stores dirtiness and freshness.
   Move the linked-list update logic into `SetDirty()` first so that the representation change only needs to touch one helper and `SetClean()`.
   
   Co-authored-by: Lőrinc <pap.lorinc@gmail.com>

   9073d71e91
8. coins: derive `DIRTY` from linked list membership

   `CCoinsCacheEntry` only needs to store whether an entry is fresh.
   Now that `AddFlags()` is inlined into `SetDirty()`, dirtiness can be derived from membership in the dirty-entry list instead of a separate bit in `m_flags`.
   
   Make `SetDirty()` take the resulting freshness explicitly so callers cannot accidentally preserve an old fresh state by passing `false`.
   Replace `m_flags` with `m_fresh`, derive `IsDirty()` from the linked-list pointers, and update the nearby comments, docstrings, and pair tests to match.
   
   Co-authored-by: Lőrinc <pap.lorinc@gmail.com>

   5c30d62df4
9. DrahtBot added the label UTXO Db and Indexes on Mar 19, 2026
10. 
    DrahtBot commented at 12:02 pm on March 19, 2026: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Reviews

    See the guideline for information on the review process. A summary of reviews will appear here.

Contributors
l0rinc DrahtBot

Labels
UTXO Db and Indexes