erigontech / erigontech/erigon
parallel-exec: validateReadImpl recursive cross-validate probes are inert no-ops
- Dominant language
- Go
- Stars
- 3.6k
- Forks
- 1.5k
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 455
Description
## Context
Raised from a Copilot review comment on #21294 (`execution/state/versionmap.go`, `validateReadImpl`).
## The soft spot
In `validateReadImpl`'s `MVReadResultDone` handling:
```go
if recursive && readVal == nil {
// Synthetic probe — outer entry's own validation covers it.
}
```
The `MVReadResultNone` branch cross-validates an account-field read by recursively calling `validateReadImpl` for `AddressPath` and `SelfDestructPath` with `readVal=nil, recursive=true`. When such a recursive probe finds a `Done` entry, the no-op above makes it return `VersionValid` — so the recursive cross-validate probes are effectively **inert**: they cannot, on their own, invalidate a read on the basis of a prior account create / self-destruct.
The `recursive` gate was added (commit `b23da1a90f`) because the un-gated recursive probe produced false *positives*; a naive un-gate over-fires on every in-block-created account (each gets a `SelfDestructPath=false` Done entry).
## Why this is currently sound (not a live bug)
Account-lifecycle detection does not actually depend on the recursive probe:
- **Recording side** — a per-account-field read of a self-destructed account records an explicit `SelfDestructPath` dependency (rather than a bare `StorageRead`), which is validated directly. (Strengthened by the follow-up to #21294.)
- The `path == AddressPath` branch keeps an explicit, ungated `IncarnationPath` Done check — `IncarnationPath` is the precise create/destruct signal.
- The `MVReadResultDone` SD-staleness check (revival-aware) catches later self-destructs.
Empirically, the SD/account-lifecycle-heavy suites are green (EEST devnet/Amsterdam EIP-7928, stable, `-race`).
## Ask
Revisit `validateReadImpl`'s cross-validation as part of the parallel-exec validator / `skipCheck` cleanup: either make the recursive probes carry meaningful (revival-aware) lifecycle detection, or remove them as dead code and rely explicitly on the recording-side dependencies + `IncarnationPath` + SD-staleness checks. The recursive probe should not silently look like a safety net while being a no-op.
Contributor guide
Assessment
This issue has not been assessed yet.