rpc/jsonrpc: build executionWitness access set from BAL for Amsterdam blocks#21497
Draft
awskii wants to merge 11 commits into
Draft
rpc/jsonrpc: build executionWitness access set from BAL for Amsterdam blocks#21497awskii wants to merge 11 commits into
awskii wants to merge 11 commits into
Conversation
…gApi harness Verifies the three pieces Task 1 depends on compose in one process: - AllProtocolChanges (Amsterdam@genesis) chain via ExecModuleTester produces and persists a BAL through InsertChain - statecfg.EnableHistoricalCommitment + WriteDBCommitmentHistoryEnabled satisfies debug_executionWitness's commitment-history gate - DebugAPIImpl built from newBaseApiForTest reaches ExecutionWitness on the Amsterdam block No new wiring needed for Task 1. Recipe and decisions recorded in the plan.
Adds TestExecutionWitnessAmsterdamBAL and TestExecutionWitnessAmsterdamActivationBlock, plus a tiny recordingStateConstructedHookForTest seam in debug_execution_witness.go so tests can observe whether the re-exec path was taken without adding a production counter. Both tests currently fail with state-root mismatch (the existing re-exec path produces wrong roots for Amsterdam blocks) — RED for the right reason; the BAL branch (Tasks 2/3) will green them. Absorbs the Task-0 constructability spike into debug_witness_bal_test.go, extracting the harness as setupAmsterdamBALHarness per the Task-0 decision to keep one Amsterdam test surface.
Split into a pure mapping (accessedStateFromBAL) plus a DB-touching wrapper (buildAccessedStateFromBAL) so the address/storage/code shaping is unit-testable without spinning up a temporal tx. Six tests cover: empty BAL, addresses + storage reads∪changes dedup, SystemAddress preserved as-is, code over- approximation with EOA skip, shared-code dedup in SortedCodes, and the pruned BAL distinct error via a never-written block hash. The IsAmsterdam branch in ExecutionWitness lands in Task 3; Task 1's harness tests remain RED for now and flip green once the branch is wired.
…mbing) Wraps the existing RecordingState re-exec path in an `else`; the new `if chainConfig.IsAmsterdam(header.Time)` branch calls `buildAccessedStateFromBAL` and leaves `accessedBlockHashes` empty so `collectAccessedHeaders` emits parent-only for Amsterdam. The shared `detectCollapseSiblings` / `buildWitnessTrie` / `verifyWitnessStateless` calls are untouched. Pre-Amsterdam `git diff -w` shows only the new `if` plus the hoisting of `fullEngine`, `header`, and `accessedBlockHashes` declarations needed by both paths. Amsterdam tests keep the path-assertion (hookCount == 0) live but t.Skip the verifyWitnessStateless-passes assertion pending Change 4 (Amsterdam commitment-pipeline divergence — Risk 1). The skip carries the tracking link per the explicit user override of the no-skip rule.
Mark Task 4 verification gates complete in the plan: 6 BAL mapping unit tests green, Amsterdam path assertion green (hookCount == 0 before t.Skip), pre-Amsterdam TestExecutionWitness sub-cases unchanged, Amsterdam skip messages reference the Change 4 plan path, make lint clean, make erigon integration builds, and go test ./rpc/jsonrpc/... clean.
Captures Risk 1 findings from witness-bal-access-set: detectCollapseSiblings.ComputeCommitment diverges from block.Root() for Amsterdam blocks under both BAL and force-routed re-exec paths. Lists debug observations and sketches investigation entry points so the skipped Amsterdam witness tests can be unblocked.
All tasks 0-5 complete. No CLAUDE.md update needed — work followed existing fork-gate + provider patterns.
Source comments should not carry PR/plan/review references per the project comment style guide.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds the
debug_executionWitnessaccess set from the persisted Block-Level Access List (EIP-7928) for Amsterdam blocks instead of re-executing the block. Pre-Amsterdam blocks keep the existing re-execution path byte-for-byte. Draft because end-to-end witness verification is blocked on an Amsterdam commitment-pipeline divergence (Change 4 — stub plan committed here).Deliverable 3 of 3:
Why
The witness is a proof against parent state, built from commitment history; the post-state is already in the DB. Re-execution exists purely to recover the access set. For Amsterdam, EIP-7928 already persists that access set in the BAL, consensus-validated against
header.blockAccessListHash. Consuming it removes the hand-rolled execution loop that can drift from canonical execution, and is the correct way to support Amsterdam witnesses — the serial re-exec path cannot even build a BAL.What's in this PR
accessedStateFromBAL(bal, readCode)— pure mapping fromtypes.BlockAccessListto the existingaccessedStateshape (addresses; storage =StorageReads ∪ StorageChanges[].Slot; code = over-approximation: include every BAL address with non-empty parent-state code).buildAccessedStateFromBAL(ctx, tx, blockHash, blockNum, parentTxNum)— DB wrapper:rawdb.ReadBlockAccessListBytes+types.DecodeBlockAccessListBytes+state.NewHistoryReaderV3(tx, parentTxNum).ReadAccountCode(addr). Distinguishes pruned (nil bytes → error) from legitimately empty (zero-entry list).ExecutionWitnessbranches onchainConfig.IsAmsterdam(header.Time) && !chainConfig.IsEIPDisabled(7928):result.Headers = [parent](EIP-2935 BLOCKHASH is served from history-contract state captured inBAL.StorageReads).Downstream shared backend (
detectCollapseSiblings,buildWitnessTrie,verifyWitnessStateless) untouched.Out of scope
State/Codesmultiset matching with Geth/EEST) — blocked on Change 4 (commitment-pipeline divergence reproducible under both BAL and re-exec paths).Plan:
docs/plans/completed/20260527-witness-bal-access-set.md.