[r3.4] execution/stagedsync: find diffset by actually-executed hash on unwind

[JkLondon]

Summary

Fixes a state-leak bug in unwindExec3 that surfaces as a gas used mismatch and Cannot update chain head after any reorg at the tip whose unwound block deployed a contract via CREATE/CREATE2 to a counterfactual address (safe-wallets, EIP-1167 clone factories, ERC-4337 accounts, deterministic deployers). Reproduced on hoodi at block 2 789 993 on a node running release/3.4 — local sidechain head 0x706b073b…, canonical 0x72c42d17… / 0xc4c75456…, divergence +72 524 gas in tx 0x73652205af0b85e7… (idx 7) which goes from status=1, gasUsed=171 004 on canonical to status=0, gasUsed=243 528 (consume-all-gas) locally.

This is not a cherry-pick of a merged PR. The fix originated on release/3.4 after diagnosing the hoodi snapshotter incident on 2026-05-11. The corresponding bug is also present on main (see "Cherry-pick to main" below).

Root cause

unwindExec3 looks up the per-block DomainEntryDiff set via doms.GetDiffset(rwTx, br.CanonicalHash(currentBlock), currentBlock). The diffset, however, is keyed by the hash of the block that was actually executed, not by whatever the current canonical hash happens to be at unwind time.

The headers stage processes a reorg in two steps inside a single transaction (execution/stagedsync/stage_headers.go):

1. u.UnwindTo(unwindTo, …) — schedules an unwind for subsequent stages.
2. fixCanonicalChain(…) — overwrites kv.HeaderCanonical to point at the new chain.

The canonical pointer flip happens before the execution-stage unwind actually runs. When unwindExec3 then asks for br.CanonicalHash(currentBlock) it gets the new canonical hash, but the diffset for that height is sitting in ChangeSets3 under the previous (now sidechain) hash that was actually applied.

The pre-existing fallback at stage_execute.go:191-204 only handled the case where CanonicalHash returns !ok (no canonical pointer at this height — meaning we executed a block that was never marked canonical). It did not handle the case where CanonicalHash returns the new canonical and the diffset is missing under that new hash. In that case the code treated the miss as the "first block in traversal, nothing to unwind yet" edge case and silently continued.

The downstream effect is severe:

• changeSet stays nil → unwindExec3State gets nil → the loop that collects state changes into stateChanges is skipped → sd.Unwind(txUnwindTo, nil) → sd.unwindChangesetRaw = nil.
• At TemporalMemBatch.Flush the guard if sd.unwindChangesetRaw != nil (temporal_mem_batch.go:596-606) skips AggregatorRoTx.Unwind entirely.
• No empty-tombstones are written into the latest AccountsDomain / CodeDomain rows for the contracts deployed in the unwound sidechain block. The previous tombstone-restore fix #20627 / e1526bf is therefore bypassed entirely — its []byte{} tombstone write never gets a chance to run because the diffset that drives it is "not found".

The execution stage progress is then rolled back through the staged-sync machinery (Headers, Bodies, Senders reset), but the domain (latest state) tables in MDBX still reflect the sidechain. On the next forward execution the canonical chain re-attempts the same CREATE2 against the now-phantom contract. EIP-684 / EIP-1014 collision rules consume all remaining gas of that tx — exactly what shows up in logs:

[WARN] gas used mismatch  block=2789993  header=4279886  execution=4352410  diff=72524  txCount=34  receiptCount=34
  tx gas detail  block=2789993 txIdx=7 txHash=0x73652205af0b85e7 gasUsed=243528 ... status=0

243528 is the tx gas limit; 171004 is what canonical reference RPC (https://rpc.hoodi.ethpandaops.io) reports for the same tx; 243528 − 171004 = 72524, matching the entire block-level mismatch.

Fix

Localised to one function. Does not require any of the parallel-commitment / SD-recreate infrastructure that landed in main (bfa03df625, 95d96627ce, etc.) — the diffset lookup is the only place that was making an incorrect assumption about canonical-hash stability across stages.

New helper findExecutedDiffsetAtHeight in execution/stagedsync/stage_execute.go:

1. Tries the diffset under the current canonical hash (previous behaviour for the common case).
2. If !ok, walks every header stored at this height via rawdb.ReadHeadersByNumber and tries each one. The header whose diffset is present is by construction the block that was actually executed.
3. Returns the recovered diffset together with the executedHash for the caller.

unwindExec3 is updated to:

• Use the helper instead of the inline lookup.
• Track lastExecHash from the helper-returned executedHash for the topmost unwound block (u.CurrentBlockNumber), rather than re-reading br.CanonicalHash(u.CurrentBlockNumber) afterwards (which would suffer the same canonical-flip miscompare).
• Pass that hash to unwindExec3State, which forwards it to stateCache.RevertWithDiffset(changeset, lastExecutedBlockHash, unwindToHash). The cache's RevertWithDiffset compares its own stored blockHash against revertFromHash — with the corrected hash, surgical eviction works and the cache no longer degrades to a full clear after every tip reorg.

The fallback search is bounded by the number of headers stored at a given height (effectively 1 in the common case, 2 in a 1-deep reorg, and so on). Order of headers returned by ReadHeadersByNumber is by storage prefix, but only one of them has a stored diffset, so the search is deterministic.

Regression test

execution/stagedsync/stage_execute_unwind_test.go covers three phases:

1. Canonical hash points to the actually-executed block — direct hit, executedHash = hOld.
2. Headers stage re-canonicalises the height to a different hash (hNew) — the test first asserts that the pre-fix direct doms.GetDiffset(tx, hNew, height) returns !ok (documenting the bug), then asserts that findExecutedDiffsetAtHeight recovers via fallback and still returns executedHash = hOld with the original diff list intact.
3. A height with no stored diffset under any known header — found=false, no spurious matches.

Test runs in ~100 ms and exercises the helper against a real mdbx + Aggregator + temporal.New stack with a freezeblocks.BlockReader (no snapshots required, the unwind range is at the tip).

Limitations / what this does NOT cover

• Multiple distinct executed sidechains at the same height. If a node reorg'd back-and-forth and ChangeSets3 ends up holding diffsets for more than one block at the same height, the fallback returns the first non-canonical hash whose diffset is present. In practice the staged-sync unwind path immediately invalidates the stored diffset of any unwound block (via the subsequent forward execution), so this is rare; but a fully robust fix would persist a separate execution-applied head hash per height. Out of scope here.
• The race itself (headers stage rewriting kv.HeaderCanonical before execution stage unwind runs) is unchanged. This PR makes the execution-stage unwind robust against that ordering rather than restructuring stage interaction.

How to verify on a stuck node

The runbook for an already-affected datadir (no rebuild required, snapshots are not contaminated because the phantom lives only in the MDBX latest domain tables at the tip) is:

sudo systemctl stop erigon
./build/bin/integration reset_state --datadir=<DD> --chain=hoodi
sudo systemctl start erigon

With the fix in place, future reorgs at the tip that involve CREATE2-to-counterfactual transactions will unwind cleanly and the same scenario will not recur.

Test plan

• go test -count=1 -short ./execution/stagedsync/... ./db/state/... — green.
• New TestFindExecutedDiffsetAtHeight_FallsBackAfterCanonicalReorg — green; embeds a sanity-assert that the pre-fix direct lookup misses (so the test fails if the fallback is reverted).
• Manual: run on a node holding the hoodi-2789993 datadir state; after reset_state and restart, block 2 789 993 must be accepted with hash 0xc4c754565911f13b96a841782095a21a32806f927307ffa71695ecd99801c236.
• CI: full unit-test + lint pipeline.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Fixes execution-stage unwind robustness during tip reorgs by ensuring the per-block diffset is located using the hash of the actually executed block, not the possibly-updated canonical hash, preventing missed unwinds that can leave “phantom” state and trigger downstream gas used mismatch / head update failures.

Changes:

• Added findExecutedDiffsetAtHeight to recover the correct ChangeSets3 diffset even after headers-stage canonical pointer flips.
• Updated unwindExec3 to use the recovered diffset and pass the executed hash through to state-cache revert logic.
• Added a regression test covering canonical reorg fallback behavior and genuine-not-found behavior.

Reviewed changes

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

File	Description
execution/stagedsync/stage_execute.go	Adds executed-diffset discovery helper and integrates it into unwindExec3 to unwind the correct applied state.
execution/stagedsync/stage_execute_unwind_test.go	Adds regression coverage for diffset lookup after canonical reorg and not-found cases.

---

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

[AskAlexSharov]

@JkLondon is it important?

[JkLondon]

yep, will fix

[yperbasis]

Overview

Fixes a state-leak bug where unwindExec3 fails to locate the per-block DomainEntryDiff set after a tip reorg, because the diffset is keyed by the actually-executed block hash while br.CanonicalHash() returns whatever canonical now points to. The miss caused sd.unwindChangesetRaw to stay nil, skipping AggregatorRoTx.Unwind and leaving "phantom" deployed contracts in the latest-state domain tables. The next forward execution of the canonical chain re-attempted the same CREATE2 and consumed the entire gas limit (EIP-684 collision), surfacing as the gas used mismatch on hoodi block 2,789,993.

The fix introduces a helper findExecutedDiffsetAtHeight that tries the canonical hash first and falls back to walking every header stored at the height.

What works well

• Surgical and localised. Only changes unwindExec3; no scope-creep into the broader staged-sync interaction.
• Preserves existing edge-case logic. The "first iteration: nothing executed yet → continue" branch (if changeSet == nil { continue }) is retained, and the helper's not-found return cooperates with it correctly.
• Good regression test. Three phases cover (1) direct hit, (2) canonical-flipped fallback — with an explicit sanity assert that the pre-fix code path (doms.GetDiffset directly under new canonical) misses, so the test fails if the fallback is reverted, and (3) genuinely-not-found.
• Test runs in ~1s and uses a real mdbx + Aggregator + temporal.New stack — no mocks.
• The Copilot finding on lastExecHash capture has already been fixed in the follow-up commit 37a846c30, by moving the assignment into the changeSet == nil branch so it captures the first found diffset rather than the first loop iteration. Important — otherwise a deeper unwind whose top blocks have no diffset would have silently degraded to a full cache clear.

Issues / suggestions

1. PR-description root cause is mis-attributed (docs only, not code)

The description claims the canonical pointer flip happens in stage_headers.go via fixCanonicalChain after u.UnwindTo. Looking at the actual code (stage_headers.go:280-291):

if headerInserter.GetHighest() != 0 {
    if !headerInserter.Unwind() {                 // ← only NON-unwind path
        if err = fixCanonicalChain(...); err != nil { ... }
    }
    ...
}

fixCanonicalChain is gated by !headerInserter.Unwind(), so it does not run when an unwind is scheduled. The mechanism that actually flips canonical-before-unwind on hoodi (PoS) is addAndVerifyBlockStep in stageloop.go:560-605, which unconditionally WriteCanonicalHash(currentHash, currentHeight) every time NewPayload arrives — so a second payload at the same height as a previously-executed block flips the canonical pointer before the eventual forkchoice-triggered unwind runs.

The diagnosis of the actual bug is correct; only the explanation of how it gets triggered is off. Consider updating the PR body so the runbook references the right code path.

2. Doc comment in helper slightly overstates the loop's edge case (stage_execute.go:175-178)

"…the surrounding loop handles by continuing past the first iteration only."

The continue actually fires on every iteration where changeSet == nil && !found, not just the very first one. With a deep enough unwind range whose top N blocks were never executed, several iterations may be skipped before the first diffset is encountered. Re-word to "while no diffset has been found yet" or similar.

3. Minor: ReadHeadersByNumber is called twice on the rare !ok + single-header + miss path

When br.CanonicalHash() returns !ok and exactly one header is stored, the helper sets currentHash = nonCanonicalHeaders[0].Hash(), then falls through to doms.GetDiffset. If that misses, the post-fallback rawdb.ReadHeadersByNumber(rwTx, currentBlock) re-reads the same single header and skips it. Functionally correct, just two cursor scans where one would do. Low priority.

4. Cache-eviction call in unwindExec3State still uses raw canonical (stage_execute.go:359)

unwindToHash, err := rawdb.ReadCanonicalHash(tx, blockUnwindTo)

This is for the target hash (the unwind point), and at unwind time the unwind point's canonical hash is still the original (new canonicals are only written for blocks above the unwind point), so this is safe today. Worth a one-line comment noting why this lookup is robust by construction, since the surrounding diffset lookup needed the elaborate fallback.

5. Test coverage gap (nice-to-have)

The regression test doesn't exercise the !ok && len(headers) > 1 ambiguity-error path or the multi-sidechain-at-same-height limitation that the PR explicitly calls out. Not blocking — but a 3rd phase that confirms found=false plus a specific error string when two non-canonical headers both have diffsets would lock in the documented behaviour.

Correctness / safety

• The fallback is deterministic in the common case (≤2 headers per height, only one has a stored diffset). With multiple stored diffsets at one height the helper returns the first match by ReadHeadersByNumber's storage-prefix order — the PR explicitly accepts this in "Limitations" and the practical rarity makes it a reasonable trade-off for a backport.
• The error-message change in unwindExec3 ("domains.GetDiffset(%d, %s): not found" → "…not found under any known header hash") is a small behaviour change to observability but more accurate (the old message printed the hash we tried, which is now misleading after the fallback).
• No new locking or transaction-scope changes; the helper takes the same rwTx as the caller.

Local verification

$ go test -count=1 -short -run TestFindExecutedDiffsetAtHeight ./execution/stagedsync/...
ok  	github.com/erigontech/erigon/execution/stagedsync	1.020s
$ go test -count=1 -short ./execution/stagedsync/
ok  	github.com/erigontech/erigon/execution/stagedsync	1.257s

Recommendation

Approving. Please tighten the PR description's root-cause section (point 1 — the stage_headers.go story doesn't match the gated code path; the PoS addAndVerifyBlockStep flow is the real trigger). The other suggestions (comment wording, double-read, additional test phase) are non-blocking polish.

The "Cherry-pick to main" section referenced in the body appears to be missing — worth confirming a tracking PR exists, since the same bug pattern is present on main (verified by reading git show main:execution/stagedsync/stage_execute.go).