execution/stagedsync: prune in-RAM overlay when execution unwind is a disk no-op

[JkLondon]

What

Fixes a gas used by execution mismatch that puts the node into an infinite
unwind/retry loop. Observed on the Hoodi snapshotter (e3.4.3, serial batch
execution) stuck on block 3004265:

gas used mismatch block=3004265 header=58278345 execution=58257300 diff=-21045 txCount=45

Root cause

#20625 / #21538 correctly prunes the in-RAM TemporalMemBatch overlay by
txNum inside TemporalMemBatch.Unwind (for accounts and storage). That
prune is reached from unwindExec3.

The gap is one level up, in UnwindExecutionStage:

if u.UnwindPoint >= s.BlockNumber {
    return nil   // early return — unwindExec3 (and thus sd.Unwind) never runs
}

When a block fails its post-execution gas check mid-batch, its writes are
already in the overlay but the block was never committed, so the committed
execution-stage progress s.BlockNumber is at or below the unwind point and
this early return fires. The overlay prune is skipped entirely. Because the
same SharedDomains is reused across the unwind→retry loop inside one
sync.Run, the stale write survives:

• tx19 of 3004265 does a first-time SSTORE to ca5daf64 slot0 → overlay holds
  …a3a34 at tx19's txNum.
• block fails the gas check → UnwindTo(3004264) → UnwindExecutionStage
  no-ops (3004264 ≥ committed progress) → overlay not pruned.
• retry: tx19 reads the stale …a3a34 instead of the committed 0, takes the
  "already initialised" branch, skips the SSTORE_SET (20000 gas) + compute →
  block is 21045 gas short → fails again → loops forever.

The committed DB is correct (eth_getStorageAt ca5daf64 0x0 = 0x0); only the
in-memory overlay was stale, which is why the state root still matches and
only gasUsed diverges.

Fix

In the u.UnwindPoint >= s.BlockNumber branch, still prune the in-RAM overlay
to the unwind boundary (same txNum / sd.Unwind call the non-no-op path
already uses). When the overlay has no uncommitted writes above the unwind
point (the normal case) this is a harmless no-op.

Test

TestUnwindExecutionStage_PrunesUncommittedOverlayWrite seeds the overlay with
a first-time storage write at a txNum belonging to a block above the unwind
point, drives UnwindExecutionStage with u.UnwindPoint >= s.BlockNumber, and
asserts the stale write is gone while a write at/below the unwind point
survives.

• Without the fix (early return restored): FAILS — the overlay still returns
  the stale value (d7549f2a…a3a34, the real bug value).
• With the fix: PASSES.

Verification

• go build ./execution/stagedsync/... ./db/state/... — clean
• go test ./execution/stagedsync/ -short — pass
• TestSharedDomain_Unwind* (the #20625 regression tests) — still pass
• TestFindExecutedDiffsetAtHeight_FallsBackAfterCanonicalReorg — still pass
• gofmt / go vet / golangci-lint run ./execution/stagedsync/ — 0 issues

Notes

Relates to #21681 (the gas used mismatch in 3.4 reports that persist after
#20625). Not auto-closing — the same root cause is the most likely
explanation for those, but they should be confirmed on the snapshotter (incl.
block 25202264) first.

The fix lives in the single execution-unwind chokepoint, so it covers both the
serial and parallel executors. A forward-port to main will be opened
separately.

Co-authored-by: Claude noreply@anthropic.com

[Copilot]

Pull request overview

Fixes an execution-stage unwind edge case where a disk unwind becomes a no-op (u.UnwindPoint >= s.BlockNumber) but the in-RAM SharedDomains/TemporalMemBatch overlay can still contain uncommitted writes from a failed mid-batch execution, causing stale reads and repeated gas-used mismatches/infinite unwind→retry loops.

Changes:

• In UnwindExecutionStage, when the disk unwind would be skipped, still compute the unwind boundary txNum and call doms.Unwind(txNum, nil) to prune uncommitted overlay writes.
• Add a regression test that seeds overlay writes above/below the unwind boundary and asserts only the above-boundary write is pruned in the no-op-disk-unwind path.

Reviewed changes

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

File	Description
execution/stagedsync/stage_execute.go	Ensures in-RAM overlay is pruned even when execution unwind is a disk no-op, preventing stale overlay reads across retry loops.
execution/stagedsync/stage_execute_unwind_test.go	Adds a focused regression test validating overlay pruning occurs in the early-return (disk no-op) unwind branch.

---

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

[AskAlexSharov]

ok. but smells like: this if is for early-return, probably instead of adding biz-logic inside this if - need modify it's condition logic so it "doesn't early return in some cases. Because maybe unwind has (or will have in future) important biz-logic which doesn't exists in current if. For example: it's unclear for me why this if doesn't have u.Done(rwTx) (outer biz-logic has it)

[yperbasis]

Two substantive points and a couple of nits (reviewed statically against the PR branch).

1. Prune boundary is unwindPoint+1, but re-execution resumes from committed progress

The no-op path correctly does not call u.Done (lowering progress to unwindPoint would be wrong — (C, unwindPoint] was never flushed), so the Execution-stage progress stays at the committed value C (= s.BlockNumber). Re-execution then resumes from doms.SeekCommitment (exec3.go:125) = C+1, not unwindPoint+1.

With BatchCommitments=true (the default / snapshotter mode), commitment state is persisted only on the ~20s timer or when the batch fills, so C can sit several blocks below unwindPoint. The prune at Min(unwindPoint+1) then keeps the overlay writes for blocks (C, unwindPoint], but those blocks are re-executed from C+1 and re-read their own stale overlay — the same bug class. It doesn't loop forever (the unwind point walks back to C, then the prune clears everything — ~2 cycles), but it costs an extra partial re-execution and emits spurious "gas used mismatch" warnings for blocks that aren't actually bad. For the reported incident the two boundaries coincide (C == unwindPoint, i.e. one block since the last commit), which is why it works there.

Suggest pruning to the committed boundary instead — Min(ctx, rwTx, s.BlockNumber+1) (or the SeekCommitment txNum). That clears all uncommitted overlay in one step, aligns the prune with the resume point, and can't over-prune (committed state is in the DB).

This interacts with the test: keepKey is at block 7, above committedBlock=5 and below unwindPoint=7, and is asserted to survive ("no over-pruning"). But under resume-from-committed, block 7 is re-executed, so keeping its write is exactly what triggers the extra cycle — adopting the committed boundary flips that assertion.

2. State cache isn't reverted on this path

GetLatest consults sd.stateCache after the overlay (domain_shared.go:407); the disk path calls stateCache.RevertWithDiffset(...), the no-op path doesn't. Mitigated in practice — both executors call ValidateAndPrepare per block (exec3_serial.go:118, exec3_parallel.go:754), which clears the cache on retry, and the snapshotter never attaches a cache (SetStateCache is execmodule-only). So not a hole today, but the fix leans on that implicit clear rather than the explicit revert; worth a note for anyone enabling the cache on other paths that share this chokepoint.

Nits

• doms.SetTxNum(txNum) is overwritten by the next run's SeekCommitment → redundant (harmless; mirrors the disk path).
• The regression test exercises the prune mechanism directly (no state cache, no end-to-end retry loop) — reasonable scope, but it can't catch either point above.