erigontech / erigontech/erigon

rpc/jsonrpc: erigon_cacheCheck never validates storage and flags spurious mismatches (kvcache.ValidateCurrentRoot)

Open
#22,277 1 comment 0 reactions 1 assignee Claimed by @yperbasis View on GitHub
RPC
Dominant language
Go
Stars
3.6k
Forks
1.5k
Avg merge
1d 16h
Merged PRs (30d)
455

Description

`kvcache.Coherent.ValidateCurrentRoot` (wired to the `erigon_cacheCheck` RPC via `rpc/jsonrpc/erigon_cache_check.go`) has three compounding bugs:

1. **The state clone is validated twice, the second time empty.** `compare()` drains the cloned btree with `PopMax`; the first call `compare(cache, kv.AccountsDomain)` empties it, so the follow-up `compare(cache, kv.StorageDomain)` iterates nothing — storage entries are never actually validated against `StorageDomain`.
2. **Storage keys are validated against the wrong domain.** The root's state btree holds both account keys (20 bytes) and storage keys (52 bytes: address+location). The first pass checks all of them against `AccountsDomain`, so every cached storage entry compares against an empty read and is flagged out-of-sync — and `clearCache` then wipes a healthy cache.
3. **Results are overwritten.** `result.StateKeysOutOfSync = keys` is assigned by both passes, so the second (empty) pass masks whatever the first pass found.

Fix shape: dispatch each popped key by shape (20 bytes → `AccountsDomain`, otherwise `StorageDomain`) in a single pass over the state clone, keep the code-cache pass as is, and append rather than overwrite the out-of-sync lists.

Pre-existing (not introduced by the FCU work), but the `--state.cache` `128MB` default in #22269 makes Coherent — and therefore this endpoint's broken path — reachable on every standalone rpcdaemon by default (#22532 carries the Coherent-cache fixes).

---

**Addendum (merged from duplicate #22530):**

4. **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 state: 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` entries against `N`-state — legitimate entries "mismatch" and the root gets cleared. Milliseconds with foreground commit; the whole commit duration with `--fcu.background.commit`. The comparison is only meaningful when `stateID == c.latestStateVersionID`; otherwise report a distinct "cache ahead, retry after commit" indication.

Point 2 also had a code-domain sibling on main — code entries keyed by `keccak(code)` can never match the address-keyed E3 `CodeDomain` — which #22532 incidentally fixes by re-keying code entries by address.

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 (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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.