Pre-existing defect noticed while reviewing #21293 (since split — the cache piece is #22532, which does not touch this function apart from incidentally fixing its code-domain leg, see below). It becomes user-relevant once #22532 revives the Coherent cache for the standalone rpcdaemon and #22269 flips --state.cache to 128MB: erigon_cacheCheck is the diagnostic people will run when investigating cache behavior, and today it false-positives, clears a healthy cache as a side effect, and reports contradictory output while doing so.
What the endpoint is supposed to do
erigon_cacheCheck → Coherent.ValidateCurrentRoot walks the latest cache root, re-reads every key from the caller's tx via tx.GetLatest(domain, key), reports mismatches in StateKeysOutOfSync/CodeKeysOutOfSync, and clears the cache if anything mismatched.
Defect A: one clone, drained by the first pass, compared under the wrong domain
The state root's btree holds both account entries (20-byte keys) and storage entries (52-byte addr+location keys) in a single tree — by design. But compare drains its input via PopMax() until empty, and it is called twice on the same clone:
compare(cache, kv.AccountsDomain) drains the entire tree — storage entries included — checking every key against the Accounts domain. A 52-byte storage key looked up in Accounts returns empty, so every storage entry with a non-empty cached value "mismatches": all of them are reported and clearCache latches true.compare(cache, kv.StorageDomain) then runs on the already-drained clone — a complete no-op returning an empty slice.result.StateKeysOutOfSync = keys after the second pass overwrites the first pass's findings with that empty slice.
Net effect with WithStorage: true (the default): any root containing at least one storage entry makes erigon_cacheCheck falsely conclude the cache is out of sync, clear the real latest root (dropping all retained warmth), and return the contradiction CacheCleared: true with an empty StateKeysOutOfSync.
The code-domain leg has the same disease on main for a different reason: code entries are keyed by keccak(code), which can never match the address-keyed E3 CodeDomain, so every cached code entry false-mismatches too. #22532 re-keys code entries by address, which incidentally makes the code comparison meaningful — the accounts/storage legs above remain broken after it.
Defect B: no guard for the cache being legitimately ahead
The version guard only returns early when the tx is ahead of the cache (stateID > c.latestStateVersionID). Under #22532's announce semantics the reverse is a normal, expected state: state-change batches are dispatched pre-commit and announce the post-commit PlainStateVersion, so during every commit window the latest root is keyed N+1 while every committed tx reads N. A cacheCheck call landing in that window compares the freshly-fed N+1 batch entries against N-state — legitimate entries "mismatch" and the root that post-commit readers were about to use gets cleared. With foreground commit the window is milliseconds; with --fcu.background.commit it is the entire commit duration. The comparison is only meaningful when stateID == c.latestStateVersionID.
Impact
Not a coherency bug — clearing the cache is always safe, readers fall back to their tx snapshot — so nothing wrong is ever served. It is a broken diagnostic with destructive side effects: it lies about sync state and evicts a warm cache each time it is called. ValidateCurrentRoot has no test coverage.
Suggested fix
- Single compare pass that dispatches the domain by key length (20 → Accounts, 52 → Storage) instead of two passes over one drained clone; accumulating rather than overwriting the result slices then becomes moot.
- Gate the comparison on
stateID == c.latestStateVersionID and report a distinct "cache ahead of this tx, retry after commit" indication otherwise (extending LatestStateBehind or adding a sibling field).
Test shape (red first): feed a batch with account + storage + code entries at the tx's version, call ValidateCurrentRoot, assert nothing is reported out of sync and the cache is not cleared — this fails today on the storage leg. Add a genuinely-stale-entry case asserting detection still works, and a cache-ahead case asserting the new early-return.
Sibling kvcache follow-up from the same review: #22527.
Pre-existing defect noticed while reviewing #21293 (since split — the cache piece is #22532, which does not touch this function apart from incidentally fixing its code-domain leg, see below). It becomes user-relevant once #22532 revives the
Coherentcache for the standalone rpcdaemon and #22269 flips--state.cacheto128MB:erigon_cacheCheckis the diagnostic people will run when investigating cache behavior, and today it false-positives, clears a healthy cache as a side effect, and reports contradictory output while doing so.What the endpoint is supposed to do
erigon_cacheCheck→Coherent.ValidateCurrentRootwalks the latest cache root, re-reads every key from the caller's tx viatx.GetLatest(domain, key), reports mismatches inStateKeysOutOfSync/CodeKeysOutOfSync, and clears the cache if anything mismatched.Defect A: one clone, drained by the first pass, compared under the wrong domain
The state root's btree holds both account entries (20-byte keys) and storage entries (52-byte
addr+locationkeys) in a single tree — by design. Butcomparedrains its input viaPopMax()until empty, and it is called twice on the same clone:compare(cache, kv.AccountsDomain)drains the entire tree — storage entries included — checking every key against the Accounts domain. A 52-byte storage key looked up in Accounts returns empty, so every storage entry with a non-empty cached value "mismatches": all of them are reported andclearCachelatches true.compare(cache, kv.StorageDomain)then runs on the already-drained clone — a complete no-op returning an empty slice.result.StateKeysOutOfSync = keysafter the second pass overwrites the first pass's findings with that empty slice.Net effect with
WithStorage: true(the default): any root containing at least one storage entry makeserigon_cacheCheckfalsely conclude the cache is out of sync, clear the real latest root (dropping all retained warmth), and return the contradictionCacheCleared: truewith an emptyStateKeysOutOfSync.The code-domain leg has the same disease on main for a different reason: code entries are keyed by
keccak(code), which can never match the address-keyed E3CodeDomain, so every cached code entry false-mismatches too. #22532 re-keys code entries by address, which incidentally makes the code comparison meaningful — the accounts/storage legs above remain broken after it.Defect B: no guard for the cache being legitimately ahead
The version guard only returns early when the tx is ahead of the cache (
stateID > c.latestStateVersionID). Under #22532's announce semantics the reverse is a normal, expected state: state-change batches are dispatched pre-commit and announce the post-commitPlainStateVersion, so during every commit window the latest root is keyedN+1while every committed tx readsN. AcacheCheckcall landing in that window compares the freshly-fedN+1batch entries againstN-state — legitimate entries "mismatch" and the root that post-commit readers were about to use gets cleared. With foreground commit the window is milliseconds; with--fcu.background.commitit is the entire commit duration. The comparison is only meaningful whenstateID == c.latestStateVersionID.Impact
Not a coherency bug — clearing the cache is always safe, readers fall back to their tx snapshot — so nothing wrong is ever served. It is a broken diagnostic with destructive side effects: it lies about sync state and evicts a warm cache each time it is called.
ValidateCurrentRoothas no test coverage.Suggested fix
stateID == c.latestStateVersionIDand report a distinct "cache ahead of this tx, retry after commit" indication otherwise (extendingLatestStateBehindor adding a sibling field).Test shape (red first): feed a batch with account + storage + code entries at the tx's version, call
ValidateCurrentRoot, assert nothing is reported out of sync and the cache is not cleared — this fails today on the storage leg. Add a genuinely-stale-entry case asserting detection still works, and a cache-ahead case asserting the new early-return.Sibling kvcache follow-up from the same review: #22527.