fix(l1): eth_call/eth_estimateGas tolerate a missing header slot_number

[ilitteri]

Motivation

eth_call, eth_estimateGas, and eth_createAccessList fail with
slot_number must be present in Amsterdam+ blocks on any chain whose block
headers carry no slot_number and run Amsterdam-or-later rules — even
though the very same blocks import and execute fine.

Scope, to be precise (the current live fork is Fusaka/Osaka, which is
pre-Amsterdam, so this does not affect mainnet or the default local
genesis today):

• Reachable only when fork >= Amsterdam. Pre-Amsterdam chains — mainnet at
  Fusaka, the default fixtures/genesis/l1.json (osakaTime: 0, no
  amsterdamTime), stock --dev — take the else branch, which already
  defaults the slot to 0. Unaffected.
• It bites Amsterdam-activated configs whose blocks are produced without a V4
  engine call, so the header gets no slot: e.g. --dev /
  engine_forkchoiceUpdatedV3-driven production against an
  Amsterdam-activated genesis such as fixtures/genesis/l1-bal.json
  (amsterdamTime: 0), and Amsterdam+ testnets driven by a consensus client
  that hasn't adopted forkchoiceUpdatedV4. On these, wallet flows break
  (gas estimation before send) while transactions still execute.

Description

env_from_generic (the environment builder behind the simulation RPCs) was
the only environment constructor that treated a missing slot_number as an
error on Amsterdam+. The block-execution environment and setup_env both
default it to 0, so the chain accepts, executes, and re-validates such blocks
with SLOTNUM reading 0 — only simulation refused. Simulation should never be
stricter than execution: its job is to predict what execution does.

This aligns env_from_generic with the other two constructors: a missing
slot_number defaults to 0 (L2 keeps its explicit 0). Behavior below
Amsterdam is unchanged (that path already defaulted to 0). On a properly
operated Amsterdam+ network the change is also invisible — the slot is a
required field of PayloadAttributesV4 on the build side and
engine_newPayloadV5 rejects payloads without it on the import side, so
canonical Amsterdam+ headers always carry Some(slot); the removed error path
was only reachable in the header-has-no-slot case above.

Adds two regression tests (Amsterdam genesis): simulation succeeds on a header
without a slot number, and with one.

Checklist

• Updated STORE_SCHEMA_VERSION (crates/storage/lib.rs) if the PR includes breaking changes to the Store requiring a re-sync. — N/A, no store changes.

[github-actions]

🤖 Kimi Code Review

The PR correctly fixes a divergence between block execution and simulation (eth_call/eth_estimateGas) environments. The change is safe and aligns with Ethereum consensus behavior.

crates/vm/backends/levm/mod.rs (lines 3203-3217)

The logic simplification is correct. By removing the strict Fork::Amsterdam check that returned an error for missing slot_number, simulation now mirrors execution behavior:

• Safety: Defaulting to U256::zero() for missing slots is consensus-safe because the execution path already does this (SLOTNUM reads 0 when the header lacks the field). The opcode only exists post-Amsterdam, so a 0 default is the expected behavior for pre-V4 Engine API compatibility.
• Consistency: L1 simulation now tolerates the same None headers that L2 already handled and that execution accepts.

test/tests/levm/simulation_env_tests.rs

Well-structured regression tests. Minor notes:

1. Lines 33-38: The synchronous tokio runtime creation is acceptable for isolated tests, but consider using #[tokio::test] and making amsterdam_db_and_header async if this pattern grows, to avoid runtime instantiation overhead.
2. Line 26: const SENDER definition is clean.
3. Lines 44-53: Header construction correctly activates all forks (timestamp 1 > fork time 0), ensuring config.fork >= Fork::Amsterdam in the VM.

test/tests/levm/mod.rs

Module inclusion is correct.

Overall: The fix is precise—simulation must never be stricter than execution. The tests verify both the regression case (missing slot) and existing behavior (present slot). No security issues identified.

---

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

[github-actions]

🤖 Codex Code Review

1. test/tests/levm/simulation_env_tests.rs:80 and test/tests/levm/simulation_env_tests.rs:95 do not actually validate the SLOTNUM behavior this PR is changing. Both tests only assert that a plain transfer returns Ok, so they would still pass even if env_from_generic() ignored slot_number entirely. Since the production change is about Amsterdam slot_number wiring into the execution environment, these should execute bytecode that reads SLOTNUM and assert the returned value is 0 for None and 1234 when present.

No blocking correctness, security, or gas-accounting issues stood out in the code change itself. The relaxed handling in crates/vm/backends/levm/mod.rs:3214 now matches the existing block-execution path at crates/vm/backends/levm/mod.rs:2367, which is the right consistency property for eth_call/eth_estimateGas.

I could not run the targeted cargo test in this environment because the preconfigured Rust/Cargo homes are read-only and dependency resolution requires a writable cargo directory.

---

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

[github-actions]

🤖 Claude Code Review

Review: fix(l1): eth_call/eth_estimateGas tolerate a missing header slot_number (PR 6961)

Correctness

The core fix is correct and well-justified. I verified the claim in the PR description directly against the code:

• setup_env_with_config (crates/vm/backends/levm/mod.rs:2367-2369), the environment builder used for real block execution, unconditionally defaults slot_number to zero via .unwrap_or(U256::zero()) — with no fork check at all.
• env_from_generic (the simulation path used by eth_call, eth_estimateGas, and eth_createAccessList, confirmed via all three call sites at lines 2473, 2907, and 2905) was the only constructor that special-cased Amsterdam+ and hard-errored on None.
• I confirmed there's no consensus/header-validation rule elsewhere in crates/blockchain or crates/common that requires slot_number to be present for a block to be accepted/executed — the only place that rejects a missing slot is engine_newPayloadV5 (crates/networking/rpc/engine/payload.rs:996), which is one specific import path, not a universal invariant. So the removed error path was indeed reachable for legitimately-imported/executed blocks (e.g., --dev mode via engine_forkchoiceUpdatedV3), making the old behavior a real bug rather than a defense-in-depth check.

The new unified logic:

let slot_number = if let VMType::L2(_) = vm_type {
    U256::zero()
} else {
    header.slot_number.map(U256::from).unwrap_or(U256::zero())
};

now exactly mirrors the execution path's behavior for both L1 and L2, which is the right fix — simulation should never be stricter than execution.

Test coverage

The two added tests (simulation_env_tests.rs) directly exercise LEVM::simulate_tx_from_generic against an Amsterdam-activated chain config, with and without slot_number, and would have failed against the old code with the exact error message cited in the PR description. Good regression coverage, and the setup correctly configures the chain on the same Store instance the VM reads from.

Minor nit

• test/tests/levm/simulation_env_tests.rs:87 and :98: these lines are long (well past the typical 100-char rustfmt width) and will likely get reformatted/wrapped by cargo fmt. Worth running cargo fmt locally before merge if CI enforces formatting.

Summary

Small, well-scoped fix that removes an inconsistency between simulation and execution rather than adding new logic — no new error paths, no gas/consensus semantics changed for actual block execution. No correctness, security, or performance concerns found.

---

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

[greptile-apps]

Greptile Summary

This PR fixes eth_call, eth_estimateGas, and eth_createAccessList failing on any Amsterdam+ chain whose block headers carry no slot_number — including ethrex's own --dev mode driven by engine_forkchoiceUpdatedV3. The root cause was that env_from_generic (the env builder for simulation RPCs) treated a missing slot_number as a hard error on Amsterdam+, while both setup_env and the block-execution env builder already silently defaulted it to zero.

• Core fix (crates/vm/backends/levm/mod.rs): removes the Amsterdam+ slot_number error guard from env_from_generic, replacing it with the same unwrap_or(U256::zero()) default the other two constructors use — making simulation no stricter than execution.
• Tests (test/tests/levm/simulation_env_tests.rs): adds two focused regression tests covering the Amsterdam-without-slot and Amsterdam-with-slot cases via LEVM::simulate_tx_from_generic.
• Minor: the new mod simulation_env_tests declaration in test/tests/levm/mod.rs is inserted out of alphabetical order.

Confidence Score: 5/5

Safe to merge; the change removes a spurious error that made simulation stricter than execution without any functional benefit.

The deleted code path was unreachable on properly-operated Amsterdam+ networks (canonical headers always have a slot via PayloadAttributesV4), and on pre-V4 or dev-mode chains it was erroneously blocking valid simulations. The replacement logic (unwrap_or(U256::zero())) matches what setup_env and the block-execution constructor already do. Two targeted regression tests cover both the None and Some cases.

Only test/tests/levm/mod.rs has a trivial ordering nit; no file requires substantive attention.

Important Files Changed

Filename	Overview
crates/vm/backends/levm/mod.rs	Removes the Amsterdam+ slot_number error guard from env_from_generic, defaulting to zero like the two other env constructors already do; the change is minimal and correct.
test/tests/levm/simulation_env_tests.rs	Adds two regression tests for the fixed behaviour (Amsterdam header with and without a slot number); test setup is sound.
test/tests/levm/mod.rs	Registers the new test module; simulation_env_tests is inserted out of alphabetical order relative to the surrounding module list.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["eth_call / eth_estimateGas / eth_createAccessList"] --> B["env_from_generic()"]
    B --> C{VMType is L2?}
    C -->|Yes| D["slot_number = 0"]
    C -->|No| E["Read header.slot_number"]
    E --> F{slot_number present?}
    F -->|Yes| G["slot_number = header value"]
    F -->|No - was Error, now 0| H["slot_number = 0"]
    D --> I["Build Environment and run VM"]
    G --> I
    H --> I

    style H fill:#90EE90

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["eth_call / eth_estimateGas / eth_createAccessList"] --> B["env_from_generic()"]
    B --> C{VMType is L2?}
    C -->|Yes| D["slot_number = 0"]
    C -->|No| E["Read header.slot_number"]
    E --> F{slot_number present?}
    F -->|Yes| G["slot_number = header value"]
    F -->|No - was Error, now 0| H["slot_number = 0"]
    D --> I["Build Environment and run VM"]
    G --> I
    H --> I

    style H fill:#90EE90

Loading

Comments Outside Diff (1)

1. test/tests/levm/mod.rs, line 15-27 (link)

   The new module declaration is placed between eip8246_tests and l2_fee_token_ratio_tests, but alphabetically simulation_env_tests (si…) belongs after requests_eip8282_extraction_tests (r…) and before stack_tests (st…), which is the ordering the rest of the list follows.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: test/tests/levm/mod.rs
   Line: 15-27
   
   Comment:
   The new module declaration is placed between `eip8246_tests` and `l2_fee_token_ratio_tests`, but alphabetically `simulation_env_tests` (`si…`) belongs after `requests_eip8282_extraction_tests` (`r…`) and before `stack_tests` (`st…`), which is the ordering the rest of the list follows.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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
test/tests/levm/mod.rs:15-27
The new module declaration is placed between `eip8246_tests` and `l2_fee_token_ratio_tests`, but alphabetically `simulation_env_tests` (`si…`) belongs after `requests_eip8282_extraction_tests` (`r…`) and before `stack_tests` (`st…`), which is the ordering the rest of the list follows.

```suggestion
mod eip8246_tests;
mod l2_fee_token_ratio_tests;
mod l2_fee_token_tests;
mod l2_gas_reservation_tests;
mod l2_hook_tests;
mod l2_privileged_tx_tests;
mod memory_tests;
mod opcode_tracer_tests;
mod precompile_tests;
mod prestate_tracer_tests;
mod requests_eip8282_extraction_tests;
mod simulation_env_tests;
mod stack_tests;
```

_{Reviews (1): Last reviewed commit: "test(l1): eth_call/eth_estimateGas toler..." | Re-trigger Greptile}

[github-actions]

Lines of code report

Total lines added: 0
Total lines removed: 7
Total lines changed: 7

Detailed view

+---------------------------------------+-------+------+
| File                                  | Lines | Diff |
+---------------------------------------+-------+------+
| ethrex/crates/vm/backends/levm/mod.rs | 2835  | -7   |
+---------------------------------------+-------+------+

[Copilot]

Pull request overview

Aligns the LEVM simulation RPC environment builder (eth_call, eth_estimateGas, eth_createAccessList) with the execution path by defaulting a missing slot_number to 0 on Amsterdam+ forks, preventing simulation from failing on chains/headers that legitimately omit the field.

Changes:

• Remove Amsterdam+ slot_number presence validation from env_from_generic, defaulting missing values to 0 (L2 remains forced to 0).
• Add regression tests covering simulation on Amsterdam headers with slot_number: None and Some(_).
• Register the new test module in the LEVM test suite.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
crates/vm/backends/levm/mod.rs	Makes simulation env construction tolerate missing slot_number by defaulting to 0, matching execution behavior.
test/tests/levm/simulation_env_tests.rs	Adds regression tests ensuring simulation succeeds with missing/present slot_number under Amsterdam fork rules.
test/tests/levm/mod.rs	Includes the new simulation environment test module.

---

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

[ElFantasma]

Building on Copilot's callout above: this test's assertion (result.is_ok()) is identical to the first test's, so it would pass regardless of whether the slot value flows through at all — nothing here actually distinguishes Some(1234) from None. Two shapes worth considering: (a) execute a small SLOTNUM-reading contract and assert the returned value equals 1234, which makes the test name honest; or (b) rename to something like simulation_accepts_a_present_slot_number so the name matches what's asserted. Non-blocking.