Skip to content

parallel-exec: validateReadImpl recursive cross-validate probes are inert no-ops #21318

@mh0lt

Description

@mh0lt

Context

Raised from a Copilot review comment on #21294 (execution/state/versionmap.go, validateReadImpl).

The soft spot

In validateReadImpl's MVReadResultDone handling:

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 execution: fix parallel-exec SD-recreate flake + validator false positives #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.

Metadata

Metadata

Assignees

Type

No type
No fields configured for issues without a type.

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions