execution/stagedsync: recover CodePath alongside CodeHashPath in parallel normalizeWriteSet

[mh0lt]

Recover CodePath alongside CodeHashPath in the parallel executor's
normalizeWriteSet, so an account can never be persisted with a non-empty
codeHash but no code bytes (codeHash-no-code).

Root cause

On re-execution, SetCode short-circuits (so.Code() already returns the prior
incarnation's bytes, bytes.Equal), so the validated incarnation emits no fresh
CodePath. normalizeWriteSet keeps CodeHashPath (resolved from the version
map / filled from committed state) but the incarnation filter drops the stale
CodePath — leaving a codeHash with no code. A later block that CALLs the
contract executes it as empty and diverges (wrong gas → wrong root). Two ways the
fresh CodePath goes missing:

• CREATE/CREATE2 of an ordinary contract — reproduced on mainnet: a Safe proxy
  deployed via the SafeProxyFactory at block 25291004, CALLed and reverted
  6785 blocks later at 25297789.
• EIP-7702 delegating tx — the delegated sender then fails EIP-3607 as
  "sender not an eoa".

Fix

After the fill-missing pass in normalizeWriteSet: for any address with a
non-empty CodeHashPath in the output but no CodePath (and not
self-destructed), recover the code so code always travels with its hash —

• versionMap hit → the code was written in this block; recover it for any
  contract (genuine in-block code).
• versionMap miss → fall back to committed state via stateReader, bounded to
  7702 designators (stateReader also returns an unchanged contract's existing
  code, and re-emitting that for every touched contract is write amplification with
  no correctness benefit; an unchanged contract's code is already in CodeDomain).

The recovered code is validated to hash to the codeHash it is recovered for
(crypto.Keccak256Hash(code) == codeHash.Value()); a mismatch is skipped, so
recovery never writes code that disagrees with its hash.

Tests

• TestNormalizeWriteSet_CodePathTravelsWithCodeHash — 7702 designator via the
  versionMap.
• TestNormalizeWriteSet_CodePathRecoveredFromStateReader — committed 7702
  designator via the stateReader fallback.
• TestNormalizeWriteSet_CodePathRecoveredForCreatedContract — ordinary CREATE2
  contract (the SafeProxyFactory case).
• TestNormalizeWriteSet_CodePathRecoveryRejectsHashMismatch — code whose keccak
  ≠ the recovered hash is rejected, not emitted.

All fail-without / pass-with the fix on main.

Scope and limitations

Forward-prevention only. This stops new codeHash-no-code from being written;
it cannot repair codeHash-no-code already collated into immutable snapshot .kv
files — that requires a snapshot unwind.

Targets main; a cherry-pick to release/3.5 will follow (not release/3.4,
which does not run parallel execution by default).

[mh0lt]

Update: on-node reproduction + strengthened fix

Running the perf-stack binary on a cold mainnet minimal resync reproduced this bug on re-execution — sender not an eoa at blocks 25277235, 25279079, and 25280960 (different addresses each time). The original recovery did not fire on these ([codepath-recovery] silent), which exposed a gap:

Root cause of the gap: the recovery only re-emitted code present in this tx's versionMap (rr.Version().TxIndex == txIndex). The real failure path is a re-executing 7702 delegation whose code equals the already-committed designator → IBS.SetCode short-circuits (bytes.Equal) → the validated incarnation writes no CodePath, and the prior incarnation's versionMap entry is invalidated on re-exec. So the versionMap has nothing for this tx; the fill-missing loop still fills CodeHashPath from committed state → codeHash persists with no code.

Strengthened fix (7a84cb413d): recover the designator from the versionMap, else fall back to stateReader.ReadAccountCode (mirroring how CodeHashPath is recovered), and gate emission on types.ParseDelegation so only 7702 designators are re-emitted — never ordinary unchanged contract code (no write amplification, no callee misattribution). New TestNormalizeWriteSet_CodePathRecoveredFromStateReader covers the stateReader path.

Scope: this prevents the drop during forward execution. It cannot repair state already collated into immutable snapshots with codeHash-but-no-code — that requires a snapshot unwind (separate work, in development). Full forward validation therefore needs a fresh resync with this fix in place (the existing corrupt datadir can't validate it).

[Copilot]

Pull request overview

Fixes an intermittent parallel-execution consistency gap in normalizeWriteSet where an address can end up with a non-empty CodeHashPath persisted without its companion CodePath, causing EIP-7702 delegated EOAs to be misclassified (surfacing as false EIP-3607 “sender not an eoa” rejections).

Changes:

• Add a recovery pass in normalizeWriteSet to re-emit missing CodePath when a non-empty CodeHashPath is present (and the code parses as an EIP-7702 delegation designator).
• Add unit tests covering both version-map recovery and stateReader fallback recovery scenarios for the dropped-CodePath case.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
execution/stagedsync/exec3_parallel.go	Adds CodePath recovery logic to keep delegation designator code aligned with its emitted CodeHashPath during parallel normalize.
execution/stagedsync/exec3_finalize_test.go	Adds regression tests ensuring CodePath is recovered alongside CodeHashPath for EIP-7702 delegation scenarios.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[yperbasis]

Requesting changes on three points (plus a style one):

1. Verify recovered code against the emitted hash before re-emitting. The hash and the code can come from different sources (vm.Read at ceiling txIndex+1 can return another tx's write; the stateReader current-view races the publish loop's apply progress). On disagreement this persists code that doesn't match the persisted codeHash — and CodeDomain isn't covered by the state root, so a wrong designator target would silently misdirect later execution, worse than today's loud sender not an eoa. A keccak over ≤23 bytes, skipping emission on mismatch, closes this. (Same gap as Copilot's inline comment.)

2. Narrow the trigger. The fill-missing loop emits CodeHashPath for every touched account, so the recovery runs for every touched non-empty-code account on the serialized result-processing path: a full stateReader.ReadAccountCode per storage-touched contract per tx just to discard via ParseDelegation, and a redundant CodePath re-emit for every tx touching a delegated EOA (deduped at DomainPut, but still TouchKey + GetLatest + writeLog per touch — constant on post-Pectra mainnet). Suggest gating recovery on the address having a CodePath/CodeHashPath entry (any incarnation) in the raw writeset: a drop requires this tx to have written code; when the hash comes purely from the fill loop, the code is already committed in CodeDomain (or unrecoverable anyway). With that, recovery should ~never fire in production — so also add a log when it does; a silent safety net hides the upstream scheduler bug it catches (the [codepath-recovery] log mentioned in the PR comments isn't in this diff).

3. Correct the mechanism claims in the comments/test docstrings. The premise of TestNormalizeWriteSet_CodePathTravelsWithCodeHash — "blockIO.WriteSet retains both incarnations' entries (versionMap doesn't clear old)" — is contradicted by the pipeline: RecordWrites replaces the per-tx write set (versionedio.go:1321) and re-exec shrinkage deletes dropped paths from the versionMap (exec3_parallel.go:2331); IBS emits CodePath/CodeHashPath/CodeSizePath atomically. The "short-circuit against an already-committed designator" path also produces no corruption — the committed designator bytes are already in CodeDomain, which is why the narrowed gate loses nothing. The tests are fine as robustness tests of normalizeWriteSet's contract against arbitrary inputs, but the docstrings shouldn't present a mechanism the scheduler prevents. If the trigger is narrowed per point 2, TestNormalizeWriteSet_CodePathRecoveredFromStateReader should assert no emission instead.

4. Comment style: the new multi-paragraph block comments are well over the repo's comment limits — condense to one or two sentences stating the invariant; the scenario forensics belong in the PR description.

[yperbasis]

Core finding

The pipeline can't produce the codeHash-no-code this fix targets, so recovery is a safety net for an invariant that currently holds — not a fix for the reported sender not an eoa.

• IntraBlockState.SetCode emits CodePath+CodeHashPath+CodeSizePath atomically — all three or none (intra_block_state.go:1322-1339).
• RecordWrites replaces the per-tx output, it doesn't accumulate incarnations (versionedio.go:1321).
• Re-exec shrinkage deletes dropped paths from the versionMap (exec3_parallel.go:2335).

So the incarnation filter never sees a lone stale CodePath to drop. A CodeHashPath-without-CodePath in the output can only come from the fill-missing loop, and in both of its sources (a prior tx's versionMap write, or committed state) the code is already in CodeDomain. On a clean datadir no new corruption is created; on a datadir whose snapshots already hold codeHash-no-code, ReadAccountCode returns empty → ParseDelegation fails → recovery is the silent no-op already observed. Defensible only if narrowed so it never fires in production and logs when it does.

Status of the four points (latest 953389bd)

1. Verify recovered code against the emitted hash — ✅ done. emit checks Keccak256Hash(code) == want.Value(); TestNormalizeWriteSet_CodePathRecoveryRejectsHashMismatch covers it.
2. Narrow the trigger + log when it fires — ❌ still iterates the filled codeHashInOutput, so ReadAccountCode runs per touched non-empty-code account per tx and re-emits for every touched delegated EOA; no log added.
3. Correct the false mechanism in the docstrings — ❌ TestNormalizeWriteSet_CodePathTravelsWithCodeHash still claims "blockIO.WriteSet retains both incarnations' entries (versionMap doesn't clear old)", contradicted by versionedio.go:1321 + exec3_parallel.go:2335. If the trigger is narrowed, …RecoveredFromStateReader should assert no emission.
4. Comment style — ❌ the recovery block is still a 4-sentence scenario paragraph, and the test docstrings still embed mainnet block numbers — over the repo's comment limits.

Point 1 is correct hardening, but 2–4 remain open. Holding at changes requested.

[mh0lt]

Addressed yperbasis's remaining three points in 73ae786a76:

Point 2 — narrow trigger + log when fires. Recovery now gates on a codeAddrInRaw set built from the raw writes parameter (CodePath or CodeHashPath, any incarnation). When the codeHash reaches the output via the fill-missing loop alone, the address isn't in the set and recovery skips. log.Warn("[codepath-recovery] …") fires on every emit so the upstream scheduler gap that would justify the safety net is visible if it ever occurs in production.

Point 3 — flipped TestNormalizeWriteSet_CodePathRecoveredFromStateReader. It now asserts countPath(result, state.CodePath) == 0 with the rationale that the committed designator is already in CodeDomain — exactly the case the narrowed gate is designed to skip. The two surviving recovery tests (…TravelsWithCodeHash and …RecoveredForCreatedContract) keep CodePath/CodeHashPath in their raw writesets so the gate lets them through, and the rejection test still asserts no emission on hash mismatch.

Point 4 — comment style. The production-side recovery block is now 3 lines stating the invariant; the test docstrings are 2-3 lines each, no mainnet block numbers, no SetCode/SafeProxyFactory forensics, no "blockIO.WriteSet retains both incarnations" mechanism claim. make lint clean (twice), make erigon clean, the four TestNormalizeWriteSet_CodePath* tests plus the wider TestNormalizeWriteSet|TestCalcFees|TestFlushToUpdates suite still green.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[AskAlexSharov]

The PR branch is based on a pre-isAura version of normalizeWriteSet. Since the branch was authored, main merged a signature change adding isAura bool as an 8th parameter, plus ctx context.Context to execLoopExitCheck and processCommitErr. This causes typecheck failures in calc_state_test.go and exec3_parallel_robustness_test.go.

Fix: rebase onto current main and update the 4 new test calls in exec3_finalize_test.go from 7 to 8 args (append , false for isAura). Rebased version is at alex/fix_21706_lint if you'd like to pull from it.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

[mh0lt]

Addressed the review (current head c68af1f):

yperbasis's four points

1. Verify recovered code against the emitted hash — done; emit skips unless Keccak256Hash(code) == want.Value() (TestNormalizeWriteSet_CodePathRecoveryRejectsHashMismatch).
2. Narrow the trigger + log when it fires — done; recovery is gated on the address having a raw-writeset CodePath/CodeHashPath entry (codeAddrInRaw), so it never runs for accounts whose CodeHashPath came purely from the fill loop. It now logs (log.Debug, this commit — Debug not Warn since it's the per-tx hot path).
3. Correct the mechanism claims / assert-no-emission — done; the docstrings no longer assert the scheduler-prevented mechanism, and the no-raw-code test now asserts no emission (and is renamed …NoCodePathRecoveryWithoutRawCodeWrite).
4. Comment style — done; the recovery block is a 3-line invariant comment and the test docstrings no longer embed mainnet block numbers.

On the core finding — agreed it's a safety net for an invariant the scheduler currently upholds, not a live repro. It's now narrowed so it should never fire in production and logs (Debug) if it ever does, so it surfaces an upstream scheduler regression rather than silently masking one.

Copilot's 3 inline comments also addressed (log level + redundant cast, test rename, non-vacuous hash-mismatch test). Build + make lint clean, the four normalizeWriteSet tests pass.

[mh0lt]

@yperbasis ready for re-review — CI green (run 28036125669), all review threads resolved. The remaining review items were addressed in c68af1f: log level + redundant common.Address cast, the misleading …RecoveredFromStateReader test renamed to …NoCodePathRecoveryWithoutRawCodeWrite, and …RejectsHashMismatch made non-vacuous (asserts a CodeHashPath is present so the 0 CodePath is a genuine rejection). Points 1–4 from your earlier review are all done; the recovery is now a narrowed, logged safety net.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

[yperbasis]

Requesting changes:

• Recovery still fails open: a hash mismatch or ReadAccountCode error leaves CodeHashPath without CodePath and normalization continues. Return an error; the mismatch test should assert failure rather than this inconsistent output.
• The positive tests hand-construct stale cross-incarnation writes that the live pipeline cannot produce: SetCode emits the code trio atomically, RecordWrites replaces transaction output, and re-execution deletes dropped paths. Add an execution-level failing reproduction, or reframe this as invariant hardening and correct the PR claims.
• The three recovery maps are sized from all writes. A 1,024-entry storage-only benchmark regressed from 9,472 to 120,304 B/op, 1 to 16 allocs/op, and about 44 to 50 microseconds/op. Allocate lazily or by code-related entries.

The head also conflicts with main, where normalization has moved to execution/state; this needs a rebase and redesign against the current implementation.