execution: parallel-only ExecV3 + ephemeral single-block replay harness

[mh0lt]

Goals

1. Let ephemeral execution use the parallel path. Today ephemeral execution
   (ExecuteBlockEphemerally) is serial-only and separate from the parallel
   ExecV3, so single-block replays can't exercise (or catch bugs in) the
   parallel executor. This makes the parallel path drivable for a discrete
   single block.
2. Rationalize ExecV3 to be parallel-only without removing serial exec.
   ExecV3 sheds its StageState/Unwinder coupling and becomes a
   stage-independent parallel core; the legacy serial path is kept but moves to
   execV3Serial, now managed by the stage process (SpawnExecuteBlocksStage),
   which also owns SeekCommitment/restoreTxNum/progress-update and the bad-block
   unwind.

Why: to allow discrete, repeatable block testing on the real parallel
execution path — in particular replaying outlier blocks captured from tip
processing offline, for debugging and profiling, without standing up a node.

What's here

Extraction (execution/stagedsync)

• ExecV3 is parallel-only, u/StageState-free; returns an outcome the stage
  wrapper uses for progress update + unwind.
• Bad-block unwind routing hoisted to SpawnExecuteBlocksStage as
  unwindOnExecError (+ TestUnwindOnExecError, 8 cases).
• Serial path preserved as execV3Serial, called by the stage.
• Seams for ephemeral replay: a blockSource for block input;
  execctx.WithMemBatch to back SharedDomains with a supplied in-memory batch;
  DISCARD_COMMITMENT nils the commit stream so the parallel path runs
  exec-only (calculator idles, apply-loop exit is driven by exec, not the trie
  root).
• Fixes a latent nil-notifications deref in the parallel apply loop.

Ephemeral harness (execution/tests/blockreplay)

• Replays a captured block through parallel ExecV3 with no MDBX behind the
  state (witness mem batch + in-memory FullBlockReader, commitment off).
• Fixtures capture inputs and the block's authoritative canonical outputs
  (post-state read from the committed chain at end-of-block — never re-derived
  by an executor, since serial is being decommissioned). The replay's post-state
  is checked against them (data, not the trie root; receipts/gas/bloom are
  already validated by the executor).
• integration capture_block --block=N produces fixtures from a node.

Testing

• TestUnwindOnExecError (pins the hoisted unwind routing; mutation-verified).
• blockreplay package tests (capture/replay round-trip, seed readback,
  authoritative-outputs presence) and BenchmarkEphemeralParallelReplay
  (validates the parallel replay's post-state against the canonical outputs each
  iteration). The parallel replay of mainnet 25604144 passes at ~88ms/op.
• Full stagedsync + execmodule suites green; make lint clean.

Notes for review

• DISCARD_COMMITMENT default-false leaves the staged consensus path unchanged
  (the two prior behaviours are exactly preserved).
• The sample fixture testdata/block-25604144.gob is ~3.4 MB. Fixture-backed
  tests skip gracefully if it's absent; happy to relocate it to a downloaded
  asset if preferred rather than commit a binary of that size.
• Follow-ups (not in this PR): fully disable the commitment subscription
  (this PR idles it), a top-down hoist of mem-batch construction out of
  tx.Debug(), and wiring the mem-batch Flush for consumers that need outbound
  post-state.

[Copilot]

Pull request overview

This PR refactors Erigon’s execution stage so ExecV3 becomes a stage-independent, parallel-only execution core, and adds an ephemeral single-block replay harness (with capture tooling) to exercise the parallel path offline against captured real-chain blocks.

Changes:

• Hoists stage coordination (range resolution, progress update, bad-block unwind routing) out of ExecV3 into SpawnExecuteBlocksStage, while keeping the legacy serial path as execV3Serial.
• Introduces a block replay fixture format + capture/replay APIs and tests, including a DB-free FullBlockReader and authoritative post-state output checking.
• Adds an integration command (capture_block) to generate fixtures from a running node/datadir.

Reviewed changes

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

Show a summary per file

File	Description
execution/tests/blockreplay/witnessmembatch.go	Adds a witness-backed TemporalMemBatch seam for SharedDomains reads/writes during replay.
execution/tests/blockreplay/witness_domains.go	Builds witness-backed SharedDomains using the injected mem-batch seam.
execution/tests/blockreplay/seed.go	Seeds fixture pre-state into real domains using the production writer.
execution/tests/blockreplay/seed_test.go	Verifies seeded pre-state reads back through SharedDomains correctly.
execution/tests/blockreplay/rlp.go	RLP encode/decode helpers for blocks/headers in fixtures.
execution/tests/blockreplay/replay_bench_test.go	Adds replay test + benchmark harness for a mainnet fixture.
execution/tests/blockreplay/outputs.go	Defines authoritative outputs capture, collection, and diffing for replay verification.
execution/tests/blockreplay/outputs_test.go	Pins that fixtures contain authoritative captured outputs (incl. coinbase credit).
execution/tests/blockreplay/memreader.go	Implements a DB-free FullBlockReader backed by fixture data.
execution/tests/blockreplay/harness.go	Implements fixture capture (from canonical history) and DB-free replay execution.
execution/tests/blockreplay/decode.go	Fixture decode helpers (block, parent header, senders list).
execution/tests/blockreplay/blockreplay.go	Defines fixture structures and in-mem/recording state readers.
execution/tests/blockreplay/blockreplay_test.go	End-to-end capture→save→load→replay round-trip test.
execution/stagedsync/stage_execute.go	Moves range resolution/progress update/unwind routing to the stage boundary; calls new ExecV3 API.
execution/stagedsync/stage_execute_unwind_routing_test.go	Adds test coverage for the new unwind routing logic.
execution/stagedsync/exec3.go	Refactors ExecV3 into a parallel-only core + introduces execRange/execV3Outcome; adds execV3Finalize.
execution/stagedsync/exec3_parallel.go	Updates parallel executor to report implicated bad block info and supports exec-only mode (discard commitment).
execution/stagedsync/exec3_ephemeral_test.go	Adds benchmark that replays a captured block via parallel ExecV3 using witness-backed domains.
execution/stagedsync/exec3_blocksource.go	Introduces blockSource abstraction to drive execution from DB or an ephemeral source.
db/state/execctx/options.go	Adds WithMemBatch option to inject a custom in-memory batch into SharedDomains.
db/state/execctx/domain_shared.go	Wires WithMemBatch into NewSharedDomains mem-batch construction.
cmd/integration/commands/stages.go	Adds capture_block integration command to emit replay fixtures from a node/datadir.

---

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

[mh0lt]

Addressed — Capture now returns an explicit error for block 0 (harness.go: if blockNum == 0 { return … "cannot capture genesis block (0): it has no parent" }) before the blockNum-1 parent read, so no MaxUint64 underflow.

[mh0lt]

Addressed — NewMemBlockReader returns an error for block 0 (memreader.go: if block.NumberU64() == 0 { return nil, … "cannot replay genesis block (0)" }) before computing parentN, so the NumberU64()-1 underflow is unreachable.

[mh0lt]

Addressed — Outputs.Diff now prints codehash in the account-mismatch message (got {… codehash=%x …} want {… codehash=%x …}), so a codehash-only divergence is no longer masked.

[mh0lt]

Addressed — the executor no longer calls restoreTxNum when blockSrc == nil; range resolution (SeekCommitment + restoreTxNum) is owned by the caller and consumed by the executor (see the contract note in exec3_parallel.go). No second TxNums/index DB pass.

[yperbasis]

plz fix lint

[yperbasis]

Requesting changes for one production unwind bug and several replay-correctness gaps. Existing focused package tests and a one-iteration replay benchmark pass, but targeted repros confirm the unwind, witness prefix/deletion, and output-oracle defects described inline.

[yperbasis]

[P1] Unwind the recorded failed block, not lastHeader. When a worker produces blockResult.Err, the apply loop records failedBlock/failedHash but does not update lastHeader; that still points to the previous successfully validated block. If block N fails, this branch therefore unwinds to N-2 and marks valid block N-1 as bad. If the first block in the batch fails, lastHeader is nil and no unwind is scheduled. Please use out.failedBlock-1 and out.failedHash for recorded failures, with a fallback only for failure paths that lack those fields. A focused repro expecting unwind point 19 currently gets 18.

[mh0lt]

Addressed — the unwind now targets the implicated block, independent of lastHeader: execV3Outcome.failedBlock/failedHash drives UnwindTo(out.failedBlock-1, BadBlock(out.failedHash, execErr)) (and handleIncorrectRootHashError(out.failedBlock, out.failedHash, …) for the trie-root case) in stage_execute.go. Covered by stage_execute_unwind_routing_test.go, including the case where the batch’s first block fails so lastHeader is nil but failedBlock is recorded — the unwind is still scheduled to failedBlock-1.

[yperbasis]

[P1] Prefix operations must include witness state. This wrapper overlays only GetLatest; SharedDomains.HasPrefix and IteratePrefix dispatch to the embedded delegate, which contains none of the values loaded into w.witness. As a result, HasStorage returns false for an account with captured storage, and account deletion via DomainDelPrefix leaves old witness slots readable later in the replay. I reproduced both cases. Please implement overlay-aware prefix operations or seed the delegate itself.

[mh0lt]

Addressed — HasPrefix, IteratePrefix, and HasPrefixInRAM are now overlay-aware. prefixKeys merges the delegate’s keys with the witness pre-state and drops post-Seal-deleted / empty-value keys (witnessmembatch.go). So HasStorage returns true for an account with captured witness storage, and a slot cleared via DomainDelPrefix is no longer readable later in the replay. Both reproducers you cited now pass.

[yperbasis]

[P1] Validate the replay's actual write set. got is constructed exclusively by iterating keys in want, so an account, storage slot, or code entry written only by parallel execution is never read and cannot appear in Diff. Because commitment is disabled, receipts and gas do not detect a state-only extra write, allowing the benchmark to report verified-correct execution for incorrect state. Please compare the replay changeset/write-key set with the reference key set before comparing values.

[mh0lt]

Addressed — the replay’s actual write-set is now validated. witnessMemBatch records every post-Seal domain write and writeSetDiff flags any account/storage/code key the replay wrote that is absent from want and whose post value diverges from the witness base (a state-only extra write commitment-off replay could not otherwise see). Wired into verify as writeSet.Diff(expected) (exec3_ephemeral_test.go), alongside the existing expected.Diff(got).

[yperbasis]

[P2] Capture and feed the block access list. The DB-backed source reads the BAL sidecar and passes it into state.NewVersionMap, but this source always returns nil. Amsterdam or experimental-BAL fixtures are therefore replayed through the OCC/no-BAL scheduling path rather than the production BAL-driven path, so scheduler behavior and performance outliers cannot be reproduced faithfully. Please serialize the sidecar in the fixture and return it here.

[mh0lt]

Addressed — the block source now captures and feeds the BAL: fx.BAL() populates singleBlockSource.bal, which next() returns (exec3_ephemeral_test.go), so Amsterdam / experimental-BAL fixtures get a non-nil BAL into NewVersionMap rather than always nil.

[yperbasis]

[P2] Include account incarnation in the comparison. acctData captures Incarnation and the witness seeding path uses it, but this condition ignores it. A wrong incarnation after contract recreation or self-destruct therefore produces a clean diff even though subsequent storage/account behavior can diverge. Please compare and report Incarnation as well.

[mh0lt]

Addressed — Outputs.Diff now compares Incarnation (in the account-mismatch condition and message), and CollectOutputs/toAcctData capture it. A wrong incarnation after contract recreation or self-destruct now surfaces as a diff.

[yperbasis]

[P2] Missing committed fixtures must fail, not skip. This fixture is checked into the PR and these tests are intended to validate it, so silently skipping when it disappears masks packaging or accidental-deletion regressions. It also falls outside the repository's allowed test-skip cases. Please let Load/require.NoError fail the test instead.

[mh0lt]

Addressed — a missing committed fixture now fails (b.Fatal / require.NoError) instead of skipping, so packaging or accidental-deletion regressions surface rather than silently passing.

[Copilot]

Pull request overview

Copilot reviewed 23 out of 24 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (5)

execution/tests/blockreplay/witness_domains.go:29

• This Diff() docstring claims it reports both extra and missing writes and is empty only on an exact key-set match, but the implementation delegates to writeSetDiff(), which only reports extra state-changing writes outside the reference set (and intentionally does not report missing reference keys). Please align the comment with the actual behavior to avoid confusing test failures and future callers.

// Diff returns the key-set differences between the replay's write-set and want,
// in both directions (extra and missing writes). Empty when they match exactly.
func (ws *WitnessWriteSet) Diff(want *Outputs) []string { return ws.mem.writeSetDiff(want) }

execution/tests/blockreplay/memreader.go:78

• In memBlockReader.headerAt, number-1 underflows when number==0. BLOCKHASH can legitimately request header 0 for low-number blocks, so it’s better to guard the number-1 lookup to avoid relying on wraparound semantics (and accidental MaxUint64 map lookups).

		h := &types.Header{Number: *uint256.NewInt(number)}
		if prev, ok := r.ancestors[number-1]; ok {
			h.ParentHash = prev
		}

execution/tests/blockreplay/witness_domains.go:25

• The WitnessWriteSet comment says the verifier can require the replay write-set to equal the reference key set exactly, but writeSetDiff intentionally does not enforce exact key-set equality (it filters out reference no-op writes and focuses on extra state-changing writes). This comment is misleading for readers of the API.

This issue also appears on line 27 of the same file.

// caller runs the parallel executor against it and must never Flush.
// WitnessWriteSet exposes the replay's post-Seal write-set so the ephemeral
// verifier can require it to equal the reference output key set exactly.
type WitnessWriteSet struct{ mem *witnessMemBatch }

execution/stagedsync/exec3_ephemeral_test.go:164

• The verify() comment says it requires the replay write-set to equal the reference key set exactly (extra or missing writes fail), but WitnessWriteSet.Diff only flags extra state-changing writes outside the reference set. This mismatch makes the harness contract unclear.

// verify checks the post-state (Flush -> outputs read via the domains) against
// the authoritative canonical outputs — the data, not the trie root. It first
// requires the replay's write-set to equal the reference key set exactly (an
// extra or missing account/storage/code write fails), then compares values —
// so a state-only extra write commitment-off replay can't otherwise see is
// caught.

execution/tests/blockreplay/replay_bench_test.go:23

• PR description says fixture-backed tests "skip gracefully if it's absent", but this helper hard-fails when the fixture file is missing (and explicitly states it won’t skip). Please align either the PR description or the test behavior so CI expectations are clear (especially given the fixture is a multi-MB binary).

	// The fixture is committed to testdata/; a missing file is a packaging
	// regression, so fail loudly rather than skip (skips also fall outside the
	// repo's allowed test-skip cases).
	p := filepath.Join("testdata", "block-"+block+".gob")

[yperbasis]

Combined review (own findings + Codex + Copilot), verified against 2ead3c0 — build and the new tests were run locally; note CI has not run on this branch at all.

P1

Exec-only mode deadlocks bulk parallel replays. With DISCARD_COMMITMENT=true, commitResults is nil, so the calculator loop (for in != nil) exits immediately and never drains blockRequests — but blockRequests is still created whenever dbg.BALDrivenCommitment is on (default true, all chains). executeBlocks sends into it with a blocking select, so once a batch exceeds the 2048 buffer the exec loop blocks forever, the apply loop keeps waiting on applyResults, and nothing cancels the context. LoopBlockLimit defaults to 0, so bulk exec-only replay via stage_exec hangs. Gate blockRequests creation on the commit stream being live.

BenchmarkEphemeralParallelReplay breaks make test-bench and would eject the PR from the merge queue. test-bench runs -run='^$' -bench=. -benchtime=1x -short without DISCARD_COMMITMENT, and ci-gate calls it in merge_group; reproduced locally — b.Fatal at exec3_ephemeral_test.go:185. Since test-bench doesn't auto-run on PRs, the failure would first surface in the queue. The flag is read at package init, so the benchmark cannot self-enable it. Suggested fix for this and the previous point together: make exec-only an explicit option (an ExecuteBlockCfg field or ExecV3 parameter) instead of reading the env inside execImpl, and map DISCARD_COMMITMENT onto it at the stage boundary; the benchmark then sets the option directly.

HasStorage is not faithfully captured — CREATE/CREATE2 collision behavior can change at replay. recordingReader.HasStorage delegates without recording, while replay infers presence from captured slots. This flips both ways: storage that exists but was never slot-read answers false at replay (affects both Replay() and the witness-domains path), and a recorded zero-value read makes absent storage answer true on the serial path (inMemReader.HasStorage counts zero-valued entries; the witness path is immune because the production Writer drops zero writes). The answer feeds the EIP-684/7610 collision check (evm.go:524 falls through to stateReader.HasStorage). Capture a per-address HasStorage result in the fixture and serve it on both paths; the committed fixture then needs recapture (gob decodes the missing field as nil — handle nil or recapture).

P2

Deleted-account substate escapes verification. The blanket continue in writeSetDiff for keys under a want.Deleted account (witnessmembatch.go:208) also waives live-value writes, and CollectOutputs never scans deleted accounts — a replay that leaves or adds live storage/code under a deleted account verifies clean. Instead of skipping, require the post-value to be a tombstone and flag survivors; optionally also assert HasPrefix(deletedAddr) is false after replay.

Rebase + CI run needed. The branch is CONFLICTING and 70 commits behind, with conflicts exactly in the four refactored files; #22759 (de-variadic commitResults) and #22717 (BAL move) touch the same seams, so please re-verify behavior preservation across the rebase.

P3

Stale docstrings/description. The WitnessWriteSet type comment, its Diff docstring, and verify() in exec3_ephemeral_test.go still claim exact write-set matching, while writeSetDiff deliberately reports only extra state-changing writes (missing keys are caught by the value diff). The PR body also says fixture tests "skip gracefully if absent" — they now correctly fail loudly.

Call out the DISCARD_COMMITMENT semantic change. The parallel path previously computed and checked commitment even under the flag; now it is genuinely disabled, including under fork validation. Consistent with the serial path, but worth stating in the description as intentional.

captureAncestors swallows CanonicalHash errors, leaving holes that later surface as BLOCKHASH-returns-zero divergence at replay. Log or fail the capture.

[mh0lt]

Rebased onto main and addressed the review — pushed 6b86751cdf (build + make lint clean, blockreplay + stagedsync tests green, benchmark verified below).

Rebase. Merged current main. The only textual conflict was exec3.go (execV3Finalize's logPrefix vs main's execStage.LogPrefix() + its //nolint:gocritic) — kept both. Main's #22745 (TxnByIdxInBlock gained an ok bool return) was a silent semantic break: the new memBlockReader implemented the old signature — updated it. #22759 (de-variadic commitResults) and #22717 (BAL move) auto-merged and build+tests pass.

P1 — Exec-only deadlocks bulk parallel replays. Fixed — blockRequests is now created only when the commit stream is live (commitResults != nil), so under exec-only the calculator's early exit can't leave a blocking executeBlocks send wedged past the buffer.

P1 — Benchmark breaks make test-bench. Fixed via the explicit-option route you suggested: exec-only is now an ExecuteBlockCfg.discardCommitment field, initialized from dbg.DiscardCommitment() at the stage boundary and set directly by setupEphemeralReplay. All five dbg.DiscardCommitment() reads now go through the cfg field. The benchmark self-enables it and no longer b.Fatals — verified it passes the test-bench path (-run='^$' -bench=BenchmarkEphemeralParallelReplay -benchtime=1x -short with no env): BenchmarkEphemeralParallelReplay-12 1 63438130 ns/op PASS.

P1 — HasStorage not faithfully captured. Deferred with a note — this one needs a fixture schema change (capture a per-address HasStorage result) and a recapture of the committed fixture via integration capture_block against a real DB, which I can't run here. I'd rather do it as a focused follow-up so the recaptured fixture is real, not hand-forged. Flagging explicitly so it's not lost.

P2 — Deleted-account substate escapes verification. Fixed — writeSetDiff no longer blanket-skips writes under a want.Deleted account; it now requires each to be a tombstone and flags any surviving live slot/code value (live <kind> write under deleted account), which is the only place this is caught since CollectOutputs doesn't rescan deleted accounts.

P3. captureAncestors now fails on a missing/errored canonical hash instead of leaving a BLOCKHASH-diverging hole. Fixed the stale docstrings (WitnessWriteSet, Diff, verify()) to say what writeSetDiff actually does — report extra state-changing writes, with missing keys caught by the value diff. The DISCARD_COMMITMENT semantic change is intentional (the parallel path now genuinely disables commitment, including under fork validation, consistent with the serial path) — I'll add that to the PR description.

Note: as you flagged, CI hadn't run on this branch; it will now on the rebased head.

[yperbasis]

CI is red

[AskAlexSharov]

Reviewed at head d462d857. CI is green and the earlier P1 threads are addressed. Three items need attention — one is a silent revert of merged main work, and two concern the last commit (d462d85), which changed the unwind contract after the P1 thread was answered.

High

1. dbBlockSource.blockAndBAL reverts #22914 on the parallel hot path

execution/stagedsync/exec3_blocksource.go:70

data, err := rawdb.ReadBlockAccessListBytes(s.blockTx, b.Hash(), blockNum)

main calls blockAccessListBytes(blockTx, b, blockNum), which prefers the payload-carried BAL and reads the DB only when header.HasNonEmptyBAL(). #22914 added that gate specifically to "avoid unnecessary database and network work". The new source drops both halves: it ignores block.BlockAccessList() and does a sidecar lookup for every block, including all pre-Amsterdam blocks and blocks whose header commits to an empty BAL.

The merge commit (930ecbc) justifies this as "main's inline blockAccessListBytes loader is redundant here" — it is not; the source reimplements only the DB half.

Worse, blockAccessListBytes (exec3.go:595) is now called only from exec3_bal_test.go. TestBlockAccessListBytes asserts wantReads: 0 for the missing- and empty-commitment cases, so it still passes while guarding code no production path runs.

Fix is a one-liner in blockAndBAL:

data, err := blockAccessListBytes(s.blockTx, b, blockNum)

2. unwindOnExecError docstring describes behaviour the code no longer has

execution/stagedsync/stage_execute.go:452-458

"any other invalid block unwinds to the recorded failed block minus one and marks that block bad"

The body does the opposite — it returns execErr and schedules nothing. d462d85 removed the UnwindTo(out.failedBlock-1, BadBlock(out.failedHash, execErr)) call but left the doc from the previous round.

This also makes the resolution comment on the "Unwind the recorded failed block, not lastHeader" thread stale: it states the unwind now targets out.failedBlock-1, which is no longer the case. Worth reopening that thread so the P1 is not closed against code that was since reverted.

3. Plain-invalid parallel bad block no longer sets a stage unwind point — please confirm each caller owns it

d462d85's message says it "restores main's ExecV3 error routing, which propagates plain-invalid errors and lets the caller drive the unwind". That is accurate for main's serial path, but main's parallel path did unwind: exec3_parallel.go:862-874 on main calls u.UnwindTo(lastHeader-1, BadBlock(lastHeader.Hash(), execErr), rwTx). So this is a behaviour change, not a restoration.

Forkchoice and fork validation do own the unwind, and exec_module.go:701 halts the node on ErrInvalidBlock during ProcessFrozenBlocks, so initial sync is covered. The gap I could not close from the diff is the plain pipeline forward path: SpawnExecuteBlocksStage returns the error, Senders/Bodies stay at the bad block, and nothing sets an unwind point — the loop retries the same block. If that path is unreachable in practice, please say so in the PR body; the eest fix is a good reason for the change, but it deserves more than a code comment.

Medium

4. The unwind-routing tests never exercise a scheduled unwind

execution/stagedsync/stage_execute_unwind_routing_test.go

All ten subtests assert require.Empty(t, u.calls). Both wrong-root cases use failedBlock: 5 with s.BlockNumber: 10, which trips handleIncorrectRootHashError's if blockNumber <= minBlockNum { return nil } guard before the binary search or the UnwindTo. The subtest is named "takes the binary-search path from the implicated block", but nothing verifies the target block — swapping out.failedBlock for out.lastHeader.Number in the production code leaves the suite green.

Please add a case with failedBlock > s.BlockNumber (or extract the target computation) so a regression in which block gets unwound is caught.

5. handleIncorrectRootHashError now runs on the initial cycle too

The !initialCycle and !fail.exec gates from main's parallel path are gone. On the initial cycle main returned fail.err (fatal); now the stage schedules a binary-search unwind and returns nil, so the stage reports success. The commit message says this is deliberate and matches serial, which is fair, but it changes the initial-sync failure mode and interacts with the halt-on-invalid logic in exec_module.go. Worth a QA sync-from-scratch run and a line in the PR body.

6. Two sources of truth for exec-only

cfg.discardCommitment is snapshotted from dbg.DiscardCommitment() in StageExecuteBlocksCfg, but db/state/execctx/domain_shared.go:790 (IsUnfrozenStepEdge) still reads the env directly — and it is reached from execution/state/rw_v3.go:437 in the apply path, independent of the calculator. The ephemeral harness sets cfg.discardCommitment = true without the env, so the two disagree in exactly the configuration this PR introduces. Either thread the flag through, or note why the step-edge checkpoint does not matter on the witness path.

7. The commitResults = nil (exec-only) path has no test coverage

It is a materially different parallel topology: the calculator exits immediately, rootResults closes up front, blockRequests stays nil. The nil-channel handling in deliver and triggerBatchCommitment is correct, but the only thing exercising it is BenchmarkEphemeralParallelReplay, and CI does not run benchmarks. A Test wrapper around one iteration of that body would cover both this and the seam the PR is built for.

Low / nits

• execution/tests/blockreplay/harness.go:211-212: GetHeaderByNumber and GetHeaderByHash return c.parent for any argument, while GetHeader correctly returns nil for a non-parent. Any rule consulting them at a different height gets a silently wrong header. Return nil unless the argument matches the parent.
• execution/tests/blockreplay/blockreplay.go: recordingReader.HasStorage delegates without recording, but inMemReader.HasStorage answers from fx.Storage, which holds only slots the block read. An account with storage but no slot reads flips true to false on replay. The mem-batch path got witness-aware prefix ops in 2ead3c0; the Replay / ExecuteBlockEphemerally path still has the gap.
• ExecV3 stays exported but now takes the unexported execRange and blockSource, so no package outside stagedsync can call it. Either unexport it or export the parameter types.
• cmd/integration imports execution/tests/blockreplay — a tests/ package linked into a shipped binary. Consider moving Capture out of tests/.
• execution/stagedsync/exec3.go:692: stale comment, "the placeholder here uses execRoTx for the serial path fallback". executeBlocks is parallel-only now (sole caller exec3_parallel.go:368) and the closure routes through src.header.
• witnessmembatch.go / outputs.go: sort.Strings to slices.Sort; append([]byte(nil), v...) to bytes.Clone(v) at witnessmembatch.go:62.
• The 3.4 MB gob: you already flagged it. exec3_ephemeral_test.go also reaches across packages via ../tests/blockreplay/testdata/... — if the fixture moves to a downloaded asset, that path moves with it.

What looks good

The execRange / execV3Outcome split is a clean seam. Dropping the second restoreTxNum call in execImpl is correct — it is idempotent on its own output, and main's second call could actually return 0 through the lastTxNum == inputTxNum early return. The nil-notifications guard is a real latent-crash fix. deliver's if ch == nil and triggerBatchCommitment's guard make the nil commit stream safe. Seeding through the production Writer in SeedDomains / NewWitnessDomains rather than hand-encoding is the right call, and sourcing the reference outputs from committed history instead of an executor is exactly right for an oracle.

[taratorio]

Requesting changes for two additional blockers on head 1b6a2849.

The new exec-only benchmark currently panics in the parallel-commitment CI configuration, and multi-batch DISCARD_COMMITMENT execution can resume from an older commitment boundary after committing newer flat state. I reproduced the benchmark panic locally with:

ERIGON_COMMITMENT_PARALLEL=true go test ./execution/stagedsync -run '^$' -bench '^BenchmarkEphemeralParallelReplay$' -benchtime=1x -count=1

The payload-BAL regression and incomplete HasStorage witness are already covered by existing review feedback, so I have not duplicated those comments here.

[taratorio]

[P1] Skip parallel-trie activation for the seeded exec-only witness.

NewWitnessDomains seeds accounts, code, and storage through DomainPut before this call. With ERIGON_COMMITMENT_PARALLEL=true, NewSharedDomains has a pending parallel trie variant and those seed writes leave keys in the sequential update buffer. EnableParaTrieDB then panics with EnableParaTrieDB after touches: keys collected on the sequential buffer would be dropped.

This is the current bench / benchmarks (parallel) CI failure and reproduces locally. Since cfg.discardCommitment means no trie work should run, avoid this setup in that mode, or configure the trie before seeding the witness.

[mh0lt]

Addressed — the trie setup (EnableParaTrieDB/EnableTrieWarmup/SetDeferCommitmentUpdates) is now gated on !cfg.discardCommitment (exec3.go), so exec-only mode never activates the parallel trie over the seeded witness. BenchmarkEphemeralParallelReplay (DISCARD_COMMITMENT=true ERIGON_COMMITMENT_PARALLEL=true) now passes instead of panicking. Commit 0cf5f43.

[taratorio]

[P1] Do not resume exec-only batches from the stored commitment.

With this stream disabled, KeyCommitmentState remains at the pre-run boundary. Execution can still stop at the batch-size limit; the stage then records the newer Execution progress and the loop commits the newer flat state. On the next cycle, SpawnExecuteBlocksStage calls SeekCommitment, which prefers the existing old commitment state over its Execution-progress fallback. The executor therefore starts from the old block again, but over already-advanced state.

In discard mode, derive the resume point from Execution progress plus TxNums, or prevent partial batches from being committed. Please cover the two-cycle case on a DB that already has commitment state.

[mh0lt]

Addressed — resume now derives from Execution progress in discard mode. The decision is extracted into resolveExecResumePoint (stage_execute.go): after SeekCommitment, when cfg.discardCommitment and Execution progress is past the (stale) commitment boundary, it resumes from Execution progress via TxnumReader().Max rather than re-running already-advanced blocks over their own post-state. TestResolveExecResumePoint covers the two-cycle case on a DB that already has commitment state, plus the no-advance and normal-commitment cases where the commitment boundary stays authoritative. Commit 1310340.

[mh0lt]

Merged latest `main` (0c6b079fb9) to clear the conflict that was blocking CI. The only conflict was in `exec3.go`, between this branch relocating BAL feeding into the block source (`src.next → blockAndBAL`) and #22190 adding BAL-feed diagnostics to the old inline decode block.

Resolved semantically rather than take-HEAD (which would have dropped #22190's payload-BAL preference):

• `dbBlockSource.blockAndBAL` now routes through `blockAccessListBytes`, so it prefers the payload-carried BAL and falls back to the DB sidecar — preserving the 0-re-exec behavior (identical to the prior direct-rawdb read when no payload BAL is present).
• The "executing block without a BAL" debug + `TraceBALFeed` diagnostics are re-added in `executeBlocks`, driven by the src-fed `dbBAL`.

Verified post-merge: full build clean, `TestBlockAccessListBytes`, parallel-exec tests, and `BenchmarkEphemeralParallelReplay` (exec-only + para-trie) all pass; `make lint` clean.

[taratorio]

Requesting changes for two execution/replay correctness issues. The focused tests and one-shot replay benchmark pass, but they do not cover these cases.

[taratorio]

[P1] Keep wrong trie roots fatal during the initial cycle

Before this extraction, the parallel apply loop called handleIncorrectRootHashError only when !initialCycle; an initial-cycle root mismatch returned ErrWrongTrieRoot because there is no reorg to recover from. This router now sends every wrong-root error to the handler. The handler returns nil after scheduling an unwind, and it returns nil without an unwind when failedBlock <= s.BlockNumber, so initial sync can stop reporting a fatal state-root mismatch. Please return execErr when s.CurrentSyncCycle.IsInitialCycle and update the initial-cycle test accordingly.

[mh0lt]

Addressed in dbfbabef70 — unwindOnExecError now returns execErr when s.CurrentSyncCycle.IsInitialCycle before routing to handleIncorrectRootHashError, so an initial-cycle wrong trie root stays fatal (no fork to recover from). The initial-cycle routing test now asserts it propagates ErrWrongTrieRoot with zero unwind calls (was asserting the swallowed-nil behavior).

[taratorio]

[P1] Preserve HasStorage query results in the fixture

This delegates the query but never records its result. Replay later derives the answer from len(fx.Storage[address]), which describes slots that happened to be read, not whether the account has any live storage. This gives a false negative when storage exists only in an unread slot and a false positive after reading an absent or zero-valued slot. The EVM uses HasStorage for CREATE collision detection, so replay can take a different execution path. Please record the per-address answer and preserve it in both the in-memory and witness-backed replay paths.

[mh0lt]

Addressed in 18bea18fd6 — the per-address answer is now captured (Fixture.HasStorageResult) at query time in recordingReader.HasStorage, and served in both replay paths: inMemReader.HasStorage returns the recorded value, and the witness path returns it from witnessMemBatch.HasPrefix(StorageDomain, addr) (which ReaderV3.HasStorage queries and uses only the bool). No longer inferred from read slots.

[taratorio]

Pending review feedback on current head 0c6b079fb9

I re-audited the earlier inline threads and review-body findings. Most earlier implementation issues are fixed, but the following previous feedback is still pending. GitHub also still marks all 14 inline threads unresolved; 11 appear implemented and can be resolved after reviewer verification.

Correctness blockers

• Keep wrong trie roots fatal during the initial sync cycle: execution: parallel-only ExecV3 + ephemeral single-block replay harness #22733 (comment)
• Capture and replay the per-address HasStorage answer instead of inferring it from captured slot reads: execution: parallel-only ExecV3 + ephemeral single-block replay harness #22733 (comment). This was raised earlier and explicitly deferred.

Behavior and test coverage

• Reconcile the plain-invalid unwind contract. The unwindOnExecError doc says other invalid blocks unwind, but the implementation returns the error without setting a stage unwind point. The earlier reply saying it now unwinds to failedBlock-1 became stale after the later behavior change. Please verify and document which caller owns each unwind path.
• Add a wrong-root routing test with failedBlock > s.BlockNumber that asserts the actual UnwindTo target. The current routing tests avoid the scheduled-unwind branch and assert zero calls.
• Remove or justify the two exec-only sources of truth: ExecuteBlockCfg.discardCommitment and the direct dbg.DiscardCommitment() read in SharedDomains.IsUnfrozenStepEdge.
• Add a regular test for the exec-only nil-commitResults topology. It is currently exercised only by BenchmarkEphemeralParallelReplay, so normal go test runs do not cover it.
• Make fixtureChainReader.GetHeaderByNumber/GetHeaderByHash validate their arguments instead of returning the parent for every query, and guard the number-1 lookup in memBlockReader.headerAt for block zero.

Documentation and API cleanup

• Update the PR description: the committed fixture fails when absent; it does not “skip gracefully”. The “two prior behaviours are exactly preserved” note also conflicts with the changed initial-cycle wrong-root handling.
• Resolve the exported ExecV3 API taking unexported execRange and blockSource types.
• Move capture code out of execution/tests/blockreplay, or document why the shipped cmd/integration binary imports a tests/ package.
• Remove the stale “serial path fallback” comment in parallel-only executeBlocks; the earlier small slices.Sort/bytes.Clone cleanup suggestions are also still pending.

Already-fixed items such as the exec-only deadlock, benchmark configuration, BAL loading, witness prefix operations, output/write-set checks, para-trie setup, resume point, deleted-account verification, ancestor errors, rebase, and CI are intentionally omitted from this checklist.

unblock

[mh0lt]

Worked through the checklist — all items addressed (head `5cb07c8f56`):

Correctness blockers

• Wrong trie roots fatal in the initial cycle — `dbfbabef70` (routing returns `execErr` when `IsInitialCycle`; test flipped to assert it).
• Capture/replay `HasStorage` instead of inferring from slot reads — `18bea18fd6` (`Fixture.HasStorageResult`, served in both in-memory and witness paths).

Behavior and test coverage

• Reconcile the plain-invalid unwind contract — `0357f86b21` (doc now matches the impl: plain-invalid returns `execErr`, the staged-sync loop that detects `ErrInvalidBlock` owns that unwind; wrong-root paths documented).
• Wrong-root routing test with `failedBlock > s.BlockNumber` asserting the actual `UnwindTo` target — `0357f86b21` (asserts `UnwindTo(10)`).
• Two exec-only sources of truth — `74be0681fd` (cfg is the single source, threaded into `SharedDomains.SetDiscardCommitment`; `IsUnfrozenStepEdge` uses the field, no direct env read).
• Regular test for the nil-`commitResults` topology — `c04b41052e` (`TestEphemeralParallelReplay`; also caught+fixed the fixture path after the move).
• `fixtureChainReader` arg validation + block-0 `number-1` guard — `04051f2f0c`.

Documentation and API cleanup

• PR description corrected (fixture fails when absent, not "skips gracefully"; initial-cycle wrong-root note updated).
• Exported `ExecV3` with unexported types — `89fdbeadad` (unexported to `execV3`; only in-package callers).
• Move capture out of `tests/` — `89fdbeadad` (`execution/tests/blockreplay` → `execution/blockreplay`; `cmd/integration` no longer imports a `tests/` package).
• Stale "serial path fallback" comment — `04051f2f0c`. For the `slices.Sort`/`bytes.Clone` nits: I could not find them in the current inline threads — if still wanted, could you point me at the spot?

CI running on the new head.

alex