You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Raised from a Copilot review comment on #21294 (execution/state/versionmap.go, validateReadImpl).
The soft spot
In validateReadImpl's MVReadResultDone handling:
ifrecursive&&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:
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.
Context
Raised from a Copilot review comment on #21294 (
execution/state/versionmap.go,validateReadImpl).The soft spot
In
validateReadImpl'sMVReadResultDonehandling:The
MVReadResultNonebranch cross-validates an account-field read by recursively callingvalidateReadImplforAddressPathandSelfDestructPathwithreadVal=nil, recursive=true. When such a recursive probe finds aDoneentry, the no-op above makes it returnVersionValid— 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
recursivegate was added (commitb23da1a90f) because the un-gated recursive probe produced false positives; a naive un-gate over-fires on every in-block-created account (each gets aSelfDestructPath=falseDone entry).Why this is currently sound (not a live bug)
Account-lifecycle detection does not actually depend on the recursive probe:
SelfDestructPathdependency (rather than a bareStorageRead), which is validated directly. (Strengthened by the follow-up to execution: fix parallel-exec SD-recreate flake + validator false positives #21294.)path == AddressPathbranch keeps an explicit, ungatedIncarnationPathDone check —IncarnationPathis the precise create/destruct signal.MVReadResultDoneSD-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 /skipCheckcleanup: 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.