Skip to content

fix(l1): support Amsterdam block production in dev mode#6731

Open
edg-l wants to merge 2 commits into
mainfrom
fix/dev-newpayload-v5
Open

fix(l1): support Amsterdam block production in dev mode#6731
edg-l wants to merge 2 commits into
mainfrom
fix/dev-newpayload-v5

Conversation

@edg-l
Copy link
Copy Markdown
Contributor

@edg-l edg-l commented May 27, 2026

Dev mode fetched payloads via engine_getPayloadV5 (which returns the EIP-7928 Block Access List) but submitted them via engine_newPayloadV4. V4 rejects any payload carrying a block_access_list field and returns -38005 Unsupported fork for Amsterdam-timestamped blocks, so dev mode could not produce a single Amsterdam block.

Added engine_new_payload_v5 to EngineClient in crates/networking/rpc/clients/auth/mod.rs. The BAL travels inside execution_payload.block_access_list, so no extra argument is needed. block_producer.rs now routes BAL-carrying payloads to V5 and falls back to V4 for pre-Amsterdam blocks.

Discovered while verifying #6678 (BAL Prometheus metrics): bal_* metrics were never populated because dev mode failed every slot with engine_newPayloadV4: block_access_list not allowed. After this fix, a dev node with an Amsterdam-active genesis produces blocks and bal_* metrics populate as expected.

Genesis fixture

Also adds fixtures/genesis/l1-bal.json, an Amsterdam-active dev genesis (mirrors l1.json with amsterdamTime: 0 plus osaka/amsterdam blob schedules), so dev mode can produce BAL-carrying blocks out of the box:

ethrex --dev --network fixtures/genesis/l1-bal.json --datadir memory --metrics

Test plan

  • cargo check -p ethrex-rpc -p ethrex-dev builds clean
  • ethrex --dev --network fixtures/genesis/l1-bal.json --metrics: previously failed every slot with engine_newPayloadV4: block_access_list not allowed; now produces Amsterdam blocks each slot

The dev block producer fetched payloads via engine_getPayloadV5 (which
returns the Block Access List) but submitted them via engine_newPayloadV4,
which rejects the BAL field and -38005s on Amsterdam timestamps. Add an
engine_new_payload_v5 method to EngineClient and route the producer to it
when the payload carries a BAL, keeping V4 for pre-Amsterdam blocks.
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 27, 2026

⚠️ Known Issues — intentionally skipped tests

Source: docs/known_issues.md

Known Issues

Tests intentionally excluded from CI. Source of truth for the Known
Issues
section the L1 workflow appends to each ef-tests job summary
and posts as a sticky PR comment.

EF Tests — Stateless coverage narrowed to EIP-8025 optional-proofs

make -C tooling/ef_tests/blockchain test calls test-stateless-zkevm
instead of test-stateless. The zkevm@v0.3.3 fixtures are filled against
bal@v5.6.1, out of sync with current bal spec; the broad target trips ~549
fixtures. Re-broaden once the zkevm bundle is regenerated.

Why and resolution path

PR #6527 broadened
test-stateless to extract the entire for_amsterdam/ tree from the
zkevm bundle and run all of it under --features stateless; combined with
this branch's bal-devnet-7 semantics that scope produces ~549
GasUsedMismatch / ReceiptsRootMismatch /
BlockAccessListHashMismatch failures.

test-stateless-zkevm filters cargo to the eip8025_optional_proofs
suite, which still validates the stateless harness without the bal-version
mismatch.

Re-broaden by switching test: back to test-stateless in
tooling/ef_tests/blockchain/Makefile once the zkevm bundle is regenerated
against the current bal spec.

@github-actions github-actions Bot added the L1 Ethereum client label May 27, 2026
l1-bal.json mirrors l1.json with amsterdamTime=0 (plus osaka/amsterdam
blob schedules) so dev mode can produce EIP-7928 BAL-carrying blocks:
  ethrex --dev --network fixtures/genesis/l1-bal.json
@github-actions
Copy link
Copy Markdown

Lines of code report

Total lines added: 40
Total lines removed: 0
Total lines changed: 40

Detailed view
+--------------------------------------------------+-------+------+
| File                                             | Lines | Diff |
+--------------------------------------------------+-------+------+
| ethrex/crates/blockchain/dev/block_producer.rs   | 135   | +12  |
+--------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/clients/auth/mod.rs | 180   | +28  |
+--------------------------------------------------+-------+------+

@edg-l edg-l marked this pull request as ready for review May 27, 2026 08:34
@edg-l edg-l requested review from a team as code owners May 27, 2026 08:34
@ethrex-project-sync ethrex-project-sync Bot moved this to In Review in ethrex_l1 May 27, 2026
@edg-l
Copy link
Copy Markdown
Contributor Author

edg-l commented May 27, 2026

Smoke test:

ethrex --dev --network fixtures/genesis/l1-bal.json --datadir memory --metrics

Before this fix, every slot failed:

INFO  - Amsterdam: @0
ERROR Failed to produce block: error sending engine_newPayloadV4: Field 'block_access_list not allowed in engine_newPayloadV4' is incorrect or has an unknown format

After:

INFO  - Amsterdam: @0
INFO Produced block 0x7947adbfa320601a04b4b612ba931d41fa55b16fce106cd4d4b8e332edd82856
INFO Produced block 0xa2736186af7210d774228f87c05a46fecc5910687a19495a9a71a0b6cbb4ca32
INFO Produced block 0xec8c17667cf746bd726aa7c0cbcfc5e2d021df6e95cfa7af20f725a1d94af96d
INFO Produced block 0x3dc037335efb1b558b0e9e1ed2a237859ea4382464d8d4db0305ba2501b96b42

BAL-carrying Amsterdam blocks produced each slot, which also unblocks the bal_* metrics in #6678.

@github-actions
Copy link
Copy Markdown

🤖 Kimi Code Review

Review of PR #6731 - Amsterdam (Block Access List) Support

Summary

Adds support for the Amsterdam fork (EIP-7928) by implementing engine_newPayloadV5 and conditionally routing payloads based on the presence of block_access_list.

Code Review

1. crates/blockchain/dev/block_producer.rs (Lines 81-113)

Logic Correctness:

  • The conditional routing based on execution_payload.block_access_list.is_some() is clean and correctly implements the fork-specific behavior described in the comments.
  • Extracting versioned_hashes computation (lines 92-104) eliminates duplication - good refactor.

Potential Issue:

  • Line 99: The check assumes block_access_list is Some for Amsterdam payloads. Ensure the payload builder never produces Some(empty_vec) for pre-Amsterdam forks, as this would incorrectly route to V5. Consider checking is_some_and(|bal| !bal.is_empty()) if empty lists are possible.

2. crates/networking/rpc/clients/auth/mod.rs (Lines 146-177)

Security/Consensus Concern:

  • Lines 161-162: Hardcoding execution_requests: vec![] and raw_bal_hash: None may be incorrect for Amsterdam.
    • If Amsterdam introduces new execution request types (similar to Prague/EIP-6110 deposits), this will fail to propagate them.
    • Verify whether execution_requests should be populated from the payload or if Amsterdam specifically empties them.
    • The comment states the server derives BAL hash from raw bytes, but confirm None is the correct client behavior vs. computing the hash locally.

Code Quality:

  • Line 155: Documentation comment correctly references EIP-7928 and explains the BAL handling.

3. fixtures/genesis/l1-bal.json

Configuration:

  • Fork timestamps (shanghaiTime, cancunTime, pragueTime, osakaTime, amsterdamTime all set to 0) activate all forks at genesis. This is appropriate for a dev fixture but ensure this file is clearly marked as dev-only.
  • Line 34: amsterdam blob schedule duplicates osaka parameters (target: 6, max: 9). Verify this is intentional vs. a copy-paste error if Amsterdam specifies different blob economics.

Recommendations

  1. Add validation in block_producer.rs to ensure Amsterdam payloads (V5) are only sent when the local chain configuration has Amsterdam activated:

    // Pseudocode - verify against actual chain config
    if execution_payload.block_access_list.is_some() && !is_amsterdam_active(timestamp) {
        // Error: payload contains BAL but fork not active
    }
  2. Document in auth/mod.rs why execution_requests is empty for Amsterdam (e.g., "Amsterdam does not introduce new execution request types, unlike Prague").

  3. Consider making the V4/V5 decision based on the fork timestamp from payload.timestamp rather than just field presence, to fail earlier with a clearer error if the EL produces invalid payloads.

Verdict

Approve with minor suggestions. The logic correctly implements the Engine API version routing. Address the hardcoded execution_requests and verify the genesis blob schedule parameters before merging.


Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

@github-actions
Copy link
Copy Markdown

🤖 Codex Code Review

  1. crates/blockchain/dev/block_producer.rs:69 still hard-codes engine_getPayloadV5, so the Amsterdam path is incomplete. In this repo, GetPayloadV6 is the method gated for Amsterdam (crates/networking/rpc/engine/payload.rs:632), while GetPayloadV5 is gated on Osaka (crates/networking/rpc/engine/payload.rs:579). On a chain that enables Amsterdam without also enabling Osaka, block production will fail before it ever reaches the new newPayloadV5 branch. The version selection needs to be made consistently for both getPayload* and newPayload*, ideally from fork/timestamp rather than inferring from the returned payload shape.

  2. crates/networking/rpc/clients/auth/mod.rs:193 advertises an outdated capability set after adding engine_new_payload_v5. The client now calls engine_newPayloadV5, but capabilities() still only reports up to engine_newPayloadV4 and engine_getPayloadV5; it also omits engine_getPayloadV6, even though the server advertises both (crates/networking/rpc/engine/mod.rs:18). If the consensus client uses engine_exchangeCapabilities for negotiation/filtering, this can suppress the exact Amsterdam methods this PR is trying to enable.


Automated review by OpenAI Codex · gpt-5.4 · custom prompt

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 27, 2026

Greptile Summary

This PR fixes Amsterdam block production in dev mode by routing payloads that carry a block_access_list to engine_newPayloadV5 instead of engine_newPayloadV4, which was rejecting them with -38005 Unsupported fork. A companion genesis fixture enables Amsterdam from block 0 for easy local testing.

  • block_producer.rs: Extracts versioned-hash computation before the dispatch branch, then selects V5 or V4 based on execution_payload.block_access_list.is_some().
  • clients/auth/mod.rs: Adds engine_new_payload_v5 mirroring the V4 method; raw_bal_hash is left None in the client struct since it is a server-side parsed field not present in the serialized RPC params.
  • fixtures/genesis/l1-bal.json: New Amsterdam-at-genesis fixture with correct blob schedules for cancun/prague/osaka/amsterdam.

Confidence Score: 4/5

Safe to merge — the routing logic correctly fixes the Amsterdam block production failure, and the only gap is that engine_newPayloadV5 is not declared in the client's advertised capability list.

The core fix (V4→V5 dispatch for BAL-carrying payloads) is correct and well-scoped. The EngineClient::capabilities() list was not updated to include engine_newPayloadV5, leaving the client's self-advertisement inconsistent with both the server's capabilities and its own runtime behavior.

crates/networking/rpc/clients/auth/mod.rs — the capabilities() method needs engine_newPayloadV5 added.

Important Files Changed

Filename Overview
crates/blockchain/dev/block_producer.rs Routes Amsterdam payloads (where block_access_list.is_some()) to the new engine_new_payload_v5 and pre-Amsterdam payloads to V4; refactors versioned hash computation out of the match arm.
crates/networking/rpc/clients/auth/mod.rs Adds engine_new_payload_v5 method that wraps NewPayloadV5Request; capabilities() still omits engine_newPayloadV5 despite the client now using it.
fixtures/genesis/l1-bal.json New Amsterdam-active genesis fixture (mirrors l1.json with amsterdamTime: 0 and osaka/amsterdam blob schedules) for testing BAL block production in dev mode.

Sequence Diagram

sequenceDiagram
    participant BP as BlockProducer (dev)
    participant EE as ExecutionEngine (ethrex)

    BP->>EE: engine_forkchoiceUpdatedV3(state, payloadAttrs)
    EE-->>BP: "ForkChoiceResponse { payload_id }"

    BP->>EE: engine_getPayloadV5(payload_id)
    EE-->>BP: "ExecutionPayloadResponse { execution_payload, blobs_bundle }"

    alt execution_payload.block_access_list is Some (Amsterdam+)
        BP->>EE: engine_newPayloadV5(payload, versioned_hashes, parent_beacon_root)
    else block_access_list is None (pre-Amsterdam)
        BP->>EE: engine_newPayloadV4(payload, versioned_hashes, parent_beacon_root)
    end

    EE-->>BP: "PayloadStatus { latest_valid_hash }"
    BP->>BP: "head_block_hash = latest_valid_hash"
Loading

Comments Outside Diff (1)

  1. crates/networking/rpc/clients/auth/mod.rs, line 193-201 (link)

    P2 engine_newPayloadV5 missing from advertised capabilities

    The capabilities() list is sent to the peer during engine_exchangeCapabilities. Any consensus client that gates on this list (or any future external consumer) will not see engine_newPayloadV5 advertised, even though the client now uses it for Amsterdam blocks. The server-side list in engine/mod.rs already includes engine_newPayloadV5, so the client-side list is now out of sync.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: crates/networking/rpc/clients/auth/mod.rs
    Line: 193-201
    
    Comment:
    **`engine_newPayloadV5` missing from advertised capabilities**
    
    The `capabilities()` list is sent to the peer during `engine_exchangeCapabilities`. Any consensus client that gates on this list (or any future external consumer) will not see `engine_newPayloadV5` advertised, even though the client now uses it for Amsterdam blocks. The server-side list in `engine/mod.rs` already includes `engine_newPayloadV5`, so the client-side list is now out of sync.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
crates/networking/rpc/clients/auth/mod.rs:193-201
**`engine_newPayloadV5` missing from advertised capabilities**

The `capabilities()` list is sent to the peer during `engine_exchangeCapabilities`. Any consensus client that gates on this list (or any future external consumer) will not see `engine_newPayloadV5` advertised, even though the client now uses it for Amsterdam blocks. The server-side list in `engine/mod.rs` already includes `engine_newPayloadV5`, so the client-side list is now out of sync.

```suggestion
    fn capabilities() -> Vec<String> {
        vec![
            "engine_exchangeCapabilities".to_owned(),
            "engine_forkchoiceUpdatedV3".to_owned(),
            "engine_getPayloadV4".to_owned(),
            "engine_getPayloadV5".to_owned(),
            "engine_newPayloadV4".to_owned(),
            "engine_newPayloadV5".to_owned(),
        ]
    }
```

Reviews (1): Last reviewed commit: "chore(l1): add Amsterdam-active dev gene..." | Re-trigger Greptile

@github-actions
Copy link
Copy Markdown

🤖 Claude Code Review

Now I have everything I need to write the review.


PR Review: fix(l1): support Amsterdam block production in dev mode

Overview

Dev mode was fetching payloads via engine_getPayloadV5 (which returns EIP-7928 Block Access List data) but submitting them via engine_newPayloadV4. V4 rejects any payload with a block_access_list field with -38005 Unsupported fork, so Amsterdam blocks could never be produced in dev mode. This PR adds engine_new_payload_v5 to EngineClient and routes BAL-carrying payloads to it. Also adds an Amsterdam-active dev genesis fixture.

The fix is correct and targeted. A few issues below.


Bug: engine_newPayloadV5 missing from client capabilities

File: crates/networking/rpc/clients/auth/mod.rs:193-200

fn capabilities() -> Vec<String> {
    vec![
        "engine_exchangeCapabilities".to_owned(),
        "engine_forkchoiceUpdatedV3".to_owned(),
        "engine_getPayloadV4".to_owned(),
        "engine_getPayloadV5".to_owned(),
        "engine_newPayloadV4".to_owned(),  // <-- engine_newPayloadV5 not advertised
    ]
}

engine_getPayloadV5 is already advertised, but the newly callable engine_newPayloadV5 is not. These are sent during engine_exchangeCapabilities. Any CL (or test harness) that gates V5 dispatch on the advertised capabilities will never route to V5. This should be a one-liner fix.


Bug (pre-existing, now replicated): execution_requests from payload response is silently dropped

File: crates/networking/rpc/clients/auth/mod.rs:162 (and the existing V4 at line 130)

let request = NewPayloadV5Request {
    payload: execution_payload,
    expected_blob_versioned_hashes,
    parent_beacon_block_root,
    execution_requests: vec![],   // always empty
    raw_bal_hash: None,
}

engine_getPayloadV5 returns execution_requests in its response (ExecutionPayloadResponse.execution_requests at types/payload.rs:321), but block_producer.rs never extracts them and the client hardcodes vec![]. On the server side, compute_requests_hash over that empty slice will not match the requests_hash baked into blocks that contain Prague/Amsterdam execution requests (EIP-6110 deposits, EIP-7002/7251). This doesn't trigger in a vanilla dev node with no staking activity, but it is structurally incorrect.

The same bug exists in the V4 path before this PR; this PR replicates it into V5 without fixing it. The fix would be to thread execution_payload_response.execution_requests.unwrap_or_default() through to both call sites.


Minor: log messages lost version information

File: crates/blockchain/dev/block_producer.rs:68, 74

Before:

"engine_newPayloadV4 response: {response:?}"
"Failed to produce block: error sending engine_newPayloadV4: {error}"

After:

"engine_newPayload response: {response:?}"
"Failed to produce block: error sending engine_newPayload: {error}"

The version number is useful for diagnosing exactly which path was taken. Consider including it — either by logging before the branch (tracing::debug!("Using engine_newPayloadV{}", if is_v5 { 5 } else { 4 })) or by having each branch emit its own message.


Observation: BAL presence as a fork detector

File: crates/blockchain/dev/block_producer.rs:102

let new_payload_result = if execution_payload.block_access_list.is_some() {

This is a reasonable heuristic since Amsterdam always includes a BAL field (even as an empty list). It's correct for the current spec. If a future fork ever re-uses V5 without a BAL, or introduces a V6 for Amsterdam, this heuristic would silently misbehave. Worth a brief note in the comment that this relies on Amsterdam always populating block_access_list.


Genesis fixture (fixtures/genesis/l1-bal.json)

The fixture correctly mirrors l1.json with amsterdamTime: 0 added to config and an amsterdam entry added to blobSchedule (matching Osaka's target: 6 / max: 9 values). The alloc section is identical to l1.json. This looks good.


Summary

Severity Finding
Bug engine_newPayloadV5 not in capabilities()
Bug (pre-existing) execution_requests silently dropped; replicated into V5
Minor Log messages lost version number
Nit Fork detection comment could be more explicit

The core fix (routing BAL-carrying payloads to V5) is correct. The capabilities omission should be addressed before merge as it can break CL interoperability. The execution_requests issue is pre-existing but worth tracking.


Automated review by Claude (Anthropic) · sonnet · custom prompt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

3 participants