builder: compute state root directly during block assembly by mh0lt · Pull Request #19707 · erigontech/erigon

mh0lt · 2026-03-06T17:16:30Z

Summary

This PR contains two sets of changes:

1. Builder refactor: compute state root directly during block assembly (#19318)

Remove the ExecV3 re-execution pass from SpawnBuilderExecStage. Instead of building a block with NoopWriter and then re-executing via ExecV3 to compute the state root, write state changes directly to SharedDomains during block assembly and call ComputeCommitment at the end.

Key changes:

block_assembler.go: Added SetStateWriter(w, advanceTxNum) method. When set, ba.writer() returns the real writer instead of NoopWriter. Initialize, AddTransactions, and AssembleBlock all use ba.writer().
exec.go: Uses sd (the parameter from MiningStep, already backed by a MemoryBatch) for execution writes and ComputeCommitment. A separate filterSd handles filterBadTransactions speculative nonce/balance writes, which must not pollute the commitment.
Removed: ~80 lines of ExecV3 orchestration, writeBlockForExecution helper, SendersCfg parameter, parallel-tx unwrapping logic.
stages.go, backend.go, exec_module_tester.go: Removed SendersCfg from BuilderStages call sites.

Why filterSd is needed: filterBadTransactions makes speculative writes (nonce++, balance -= cost) to check transaction validity. If a tx passes the filter but fails during EVM execution (e.g., gas limit exceeded), FinalizeTx is never called, so the speculative write remains. These stale writes would corrupt ComputeCommitment if they went to sd. The separate filterSd isolates this speculation.

2. Varied BAL tests for Phase 1d/1e (#19632, #19633)

Extend the BAL (Block Access List, EIP-7928) test coverage beyond simple ETH transfers to exercise diverse transaction types and multi-transaction blocks:

Phase 1d (Remove ExecV3: Extend BAL test with varied transaction types #19632): Individual tests for contract creation (CodeChange), storage writes via token minting (StorageChange), and multiple transaction types in the same block
Phase 1e (Remove ExecV3: Varied block scenarios (execution-spec-test style) #19633): Execution-spec-test style scenarios with mixed transaction types in a single block, including system transactions at block boundaries

New Tests

Test	What it covers	Status
`TestEngineApiBAlContractCreation`	Deploy Token contract, verify CodeChange in BAL	PASS
`TestEngineApiBAlStorageWrites`	Mint tokens via contract call, verify StorageChange entries	PASS
`TestEngineApiBAlMultiTxBlock`	Transfer + mint in same block, verify per-tx BAL entries	PASS
`TestEngineApiBAlMixedBlock`	Transfer + deploy + Changer.Change() + withdrawals + system txns	PASS
`TestEngineApiBAlSelfDestruct`	Storage writes + self-destruct in same block	PASS

Builder Tests

Test	What it covers	Status
`TestEngineApiBuiltBlockStateMatchesValidation`	Builder state root matches validator	PASS
`TestEngineApiBuiltBlockWithContractDeployAndCall`	Contract deploy + call in built block	PASS
`TestEngineApiBuiltBlockReorgRecovery`	Reorg recovery after built block	PASS

Test plan

go test ./execution/engineapi/... — all 24 tests pass
make test-short — pass
make test-all — pass
make test-all-race — pass
Hive engine: 403 tests, 2 failed (cancun syncing reorgs — pre-existing, within threshold)
Hive RPC: 200 tests, 1 failed (within threshold)
Hive BAL (bal@v5.1.0): 868 tests, 20 failed (selfdestruct logs + fork transition — pre-existing)
CI pipeline

Closes: #19318, #19632, #19633

Add 5 new BAL (Block Access List) tests covering diverse transaction types beyond simple transfers: - TestEngineApiBAlContractCreation: deploy Token contract, verify CodeChange - TestEngineApiBAlStorageWrites: mint tokens, verify StorageChange entries - TestEngineApiBAlMultiTxBlock: transfer + mint in same block - TestEngineApiBAlMixedBlock: transfer + deploy + contract call + withdrawals (known failure: BAL mismatch under parallel execution, see #19668) - TestEngineApiBAlSelfDestruct: storage writes + self-destruct in same block Also add WithWithdrawals option to MockCl for testing CL withdrawal processing in block building. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Remove the ExecV3 re-execution pass from SpawnBuilderExecStage. Instead of building a block with NoopWriter and then re-executing via ExecV3 to compute the state root, write state changes directly to SharedDomains during block assembly and call ComputeCommitment at the end. The sd parameter from MiningStep is already backed by a MemoryBatch, so all writes are ephemeral and discarded when MiningStep returns. A separate filterSd is used for filterBadTransactions speculative nonce/balance writes, which must not pollute the commitment computation. This removes ~80 lines of ExecV3 orchestration, the writeBlockForExecution helper, the SendersCfg parameter, and the parallel-tx unwrapping logic.

When the builder called FinalizeTx after each user transaction, it wrote intermediate state to SharedDomains. If a later system call (e.g. EIP-7002 withdrawal request dequeue) reverted storage slots back to their original values, CommitBlock's blockOriginStorage==dirtyStorage optimization skipped the undo write, leaving stale values in sd.mem and producing a wrong state root. Fix: use NoopWriter for FinalizeTx in Initialize and AddTransactions. Only CommitBlock (in AssembleBlock) uses the real stateWriter. IBS dirty state persists across FinalizeTx calls, so CommitBlock correctly writes all final state without stale intermediate values. Also: - Forward execution requests in MockCl.InsertNewPayload - Isolate filterSd with its own MemoryBatch - Add TestEngineApiBuiltBlockWithWithdrawalRequest

Copilot

Pull request overview

Refactors the block builder execution stage to compute the state root directly from domain writes produced during block assembly (removing the ExecV3 re-execution pass), and expands BAL (Block Access List) engine API test coverage to include more transaction varieties and scenarios.

Changes:

Remove ExecV3 replay from the builder exec stage and compute commitment/state root directly from SharedDomains.
Extend BlockAssembler to optionally accept a real StateWriter (and txNum advancing hook) for domain writes during assembly/finalization.
Add/extend Engine API tests for BAL coverage and builder validation scenarios (including withdrawals / execution requests forwarding).

Reviewed changes

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

Show a summary per file

File	Description
node/eth/backend.go	Removes `StageSendersCfg` from builder stage wiring.
execution/execmodule/execmoduletester/exec_module_tester.go	Updates exec-module tester stage wiring to match removed senders stage.
execution/exec/block_assembler.go	Adds optional state-writer plumbing and txNum-advancing hook for assembly/finalization paths.
execution/builder/builderstages/stages.go	Updates `SpawnBuilderExecStage` call signature after removing senders dependency.
execution/builder/builderstages/exec.go	Implements the “compute commitment directly” builder flow; introduces isolated `filterSd` for filtering speculation.
execution/engineapi/engineapitester/mock_cl.go	Adds withdrawals option and forwards `ExecutionRequests` from `GetPayload` to `NewPayload`.
execution/engineapi/engine_api_builder_test.go	Adds a builder integration test exercising withdrawal-request tx inclusion and validation.
execution/engineapi/engine_api_bal_test.go	Adds additional BAL tests for contract creation, storage writes, multi-tx, mixed blocks, and self-destruct coverage.

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

Copilot · 2026-03-09T08:11:40Z

+	txNum, _, err := sd.SeekCommitment(ctx, tx)
+	if err != nil {
+		return err
+	}
+	sd.SetTxNum(txNum)
+	sd.GetCommitmentContext().SetDeferBranchUpdates(false)
+
+	stateWriter := state.NewWriter(sd.AsPutDel(tx), nil, txNum)
 	stateReader := state.NewReaderV3(sd.AsGetter(tx))


txNum is taken from sd.SeekCommitment() and then reused unchanged for both the stateWriter and ComputeCommitment(). If txNum is expected to advance across block boundaries / tx boundaries (as in other execution paths), this will compute the commitment at the wrong txNum and may diverge from validation. Consider maintaining a local txNum counter (starting from the sought value) and incrementing it for block init / each included tx / finalization, calling stateWriter.SetTxNum(...) (and sd.SetTxNum(...) if needed), and pass the final value to ComputeCommitment().

This is not necessary because: All writes at the same txNum work correctly as last-write-wins in the batch, and ComputeCommitment sees the final state correctly. TestEngineApiBuiltBlockStateMatchesValidation confirms the root hash matches.

No correctness issue. Adding a local txNum here will cause confusion to anyone reading the code as they will believe that it is necessary for correctness where in practise this is not the case as the test shows.

Copilot · 2026-03-09T08:11:40Z

+// Known issue: currently fails with BAL mismatch under parallel execution (#19668).
+func TestEngineApiBAlMixedBlock(t *testing.T) {


This test includes a comment saying it is a known failing case under parallel execution (#19668), but the test is still enabled (only gated by dbg.Exec3Parallel). If the issue is still present, this will make the suite fail/flaky. Either remove/update the comment if the issue is resolved, or explicitly t.Skip/gate the test on the known-broken condition until #19668 is fixed.

Copilot · 2026-03-09T08:11:41Z

+func TestEngineApiBAlContractCreation(t *testing.T) {
+	if !dbg.Exec3Parallel {
+		t.Skip("requires parallel exec")
+	}
+	eat := engineapitester.DefaultEngineApiTester(t)
+	eat.Run(t, func(ctx context.Context, t *testing.T, eat engineapitester.EngineApiTester) {


Test function names use BAl (lowercase l) rather than BAL. For consistency with the acronym used throughout the codebase and the file (BlockAccessList/BAL), consider renaming these new tests (and related helpers if any) to use BAL.

Fixed by: 0928b17

yperbasis

advanceTxNum is always nil — the TxNumAdvancer type and all the if ba.advanceTxNum != nil checks are dead code in this PR. Either remove the mechanism or wire it up. As-is, it's confusing infrastructure that
suggests txNum tracking is needed but isn't actually happening.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

advanceTxNum was always passed as nil in SetStateWriter — the TxNumAdvancer type and all three if ba.advanceTxNum != nil call sites were dead code. Remove the TxNumAdvancer type, the advanceTxNum field on BlockAssembler, the three call sites in Initialize/AddTransactions/AssembleBlock, and simplify SetStateWriter to a single-parameter method. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

mh0lt · 2026-03-09T11:49:28Z

advanceTxNum is always nil — the TxNumAdvancer type and all the if ba.advanceTxNum != nil checks are dead code in this PR. Either remove the mechanism or wire it up. As-is, it's confusing infrastructure that suggests txNum tracking is needed but isn't actually happening.

Fixed by: cf74ee0

Rename TestEngineApiBAlContractCreation, TestEngineApiBAlStorageWrites, TestEngineApiBAlMultiTxBlock, TestEngineApiBAlMixedBlock, and TestEngineApiBAlSelfDestruct to use the correct BAL acronym (uppercase L), consistent with the rest of the codebase. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…allel block exec Three related fixes for parallel execution (EXEC3_PARALLEL=true): 1. exec3_parallel.go: encode txTask.Txs directly for accumulator.StartChange instead of reading via RawTransactions(applyTx). The execLoop's applyTx is a TemporalRo snapshot opened before the staged-sync RwTx commits kv.HeaderCanonical for the block, so RawTransactions returned empty bytes. This caused the txpool to never learn which transactions were mined, leaving stale nonces 0-6 in pending and preventing block 3 from including the remaining txns after a gas-limit overflow in block 2. 2. exec/block_assembler.go: set block-end tx context (ibs.SetTxContext) before FinalizeBlockExecution so that withdrawal/system-call writes are recorded at TxIndex=len(txns), matching the validator path in exec3_parallel.go. 3. engineapitester/engine_api_tester.go: add EIP-4788 beacon root contract to genesis alloc so that block-init storage writes appear in the BAL, enabling TestEngineApiBALMixedBlock to run without a skip. engine_api_bal_test.go: remove now-stale "Known issue" comment and skip guard from TestEngineApiBALMixedBlock. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

## Summary Now that #19707 has merged, ExecV3 is no longer used for block production (the builder uses dedicated `BuilderCreateBlock`/`BuilderExecution`/`BuilderFinish` stages via `BlockAssembler`). This means `isBlockProduction` was always `false` inside ExecV3, making all its guarded code paths dead. - Remove `stages.ModeBlockProduction` constant; the two builder-pipeline callers now use `ModeApplyingBlocks` (the mode label is irrelevant to ExecV3's logic) - Remove `isBlockProduction bool` field from `txExecutor` and all construction sites - Always call `engine.Finalize` in both serial and parallel executors (never `FinalizeAndAssemble`) - Always validate state root and receipt hash — no more mining bypass - Remove `blockProduction bool` param from `ProcessBAL`; drop the BAL hash-assignment branch that was only reachable during block production - Remove `isMining bool` from `BlockPostValidation` and `BlockPostExecutionValidator.Process`; drop the receipt-hash patching branch - Remove unused `isMining bool` param from `NewWorkersPool` - Remove `isBlockProduction bool` param from `computeAndCheckCommitmentV3`; drop the `header.Root` assignment branch Note: `FinalizeBlockExecution` in `block_exec.go` retains its `isMining bool` parameter — it is still legitimately called with `isMining=true` from `BlockAssembler` and `ExecuteBlockEphemerally`. ## Test plan - [x] `make test-all` passes locally 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

mh0lt and others added 2 commits March 6, 2026 14:55

mh0lt requested review from AskAlexSharov, Giulio2002 and yperbasis as code owners March 6, 2026 17:16

mh0lt requested review from shohamc1 and taratorio March 6, 2026 18:36

mh0lt added the tech debt reduction label Mar 6, 2026

mh0lt and others added 4 commits March 6, 2026 18:36

builder: fix gofmt formatting in block_assembler.go

d928ffd

Merge branch 'main' into test/builder-bal-varied-tx

9ba843d

Merge branch 'main' into test/builder-bal-varied-tx

367ff7e

yperbasis requested a review from Copilot March 9, 2026 08:05

Copilot started reviewing on behalf of yperbasis March 9, 2026 08:05 View session

Copilot AI reviewed Mar 9, 2026

View reviewed changes

yperbasis requested changes Mar 9, 2026

View reviewed changes

mh0lt and others added 2 commits March 9, 2026 11:47

Update execution/builder/builderstages/exec.go

a07e2bf

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

yperbasis approved these changes Mar 9, 2026

View reviewed changes

mh0lt force-pushed the test/builder-bal-varied-tx branch from b52b02f to 1657b81 Compare March 9, 2026 14:07

mh0lt merged commit 874916a into main Mar 9, 2026
37 of 39 checks passed

mh0lt deleted the test/builder-bal-varied-tx branch March 9, 2026 16:04

mh0lt mentioned this pull request Mar 9, 2026

Remove unused 'isBlockProduction' from ExecV3 #19760

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

builder: compute state root directly during block assembly#19707

builder: compute state root directly during block assembly#19707
mh0lt merged 10 commits intomainfrom
test/builder-bal-varied-tx

mh0lt commented Mar 6, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

mh0lt Mar 9, 2026

Uh oh!

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

Copilot AI Mar 9, 2026

Uh oh!

mh0lt Mar 9, 2026

Uh oh!

yperbasis left a comment

Uh oh!

mh0lt commented Mar 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		// Known issue: currently fails with BAL mismatch under parallel execution (#19668).
		func TestEngineApiBAlMixedBlock(t *testing.T) {

Conversation

mh0lt commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

1. Builder refactor: compute state root directly during block assembly (#19318)

2. Varied BAL tests for Phase 1d/1e (#19632, #19633)

New Tests

Builder Tests

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

mh0lt Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

mh0lt Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

yperbasis left a comment

Choose a reason for hiding this comment

Uh oh!

mh0lt commented Mar 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mh0lt commented Mar 6, 2026 •

edited

Loading