test(l1): bump Amsterdam fixtures to glamsterdam-devnet-8 - #7094
Conversation
EIP-2780 now prices a value transfer with a single TX_VALUE_COST of 6000 covering both the recipient balance write and the transfer log, and a contract creation carrying value pays no value charge at all because CREATE_ACCESS already covers the balance write. Create-with-value drops from 24756 to 23000; the calldata-floor anchor follows since it shares recipient_regular_gas.
TX_ACCESS_LIST_ADDRESS and TX_ACCESS_LIST_STORAGE_KEY become the cold access minus WARM_ACCESS (2900), so prepaying an entry is gas neutral with the cold access it replaces. EIP-2930's 100 discount is not restored.
… cap The gate is flat: each dimension measures the tx's worst-case contribution against its remaining budget, with no intrinsic subtraction and no credit for the top-frame state charge. Only the execution dimension is capped at TX_MAX_GAS_LIMIT. Also cover the cap on max(intrinsic, calldata floor): a floor above TX_MAX_GAS_LIMIT is rejected no matter how large the gas limit is, since excess gas limit becomes state-gas reservoir.
Point the shared .fixtures_url_amsterdam, the hive config (fixtures + eels_commit) and AMSTERDAM_FIXTURES_BRANCH at devnet-8. Clear vectors/eest/for_amsterdam before overlaying the bundle: fixtures renamed between devnet releases (EIP-8037 regular gas -> execution gas) otherwise survived as stale leftovers and ran against the client. Retarget the stateless Amsterdam+ skip at the real cause: tests-zkevm@v0.6.2 is filled against devnet-7 and its gas expectations no longer match a devnet-8 client.
The engine harness dropped `slotNumber` when rebuilding a fixture's genesis, so a chain starting at a non-zero slot got a genesis hash that did not match the fixture's. The initial FCU then pointed at an unknown head and returned SYNCING instead of VALID.
|
🤖 Kimi Code ReviewReview: Amsterdam devnet-8 gas repricing & payload validation The PR correctly updates the client for the Amsterdam (Glamsterdam) devnet-8 specification, specifically implementing the EIP-2780 and EIP-8038 gas schedule changes and hardening payload validation. No critical issues found. Correctness & Consensus SafetyGas Accounting (crates/vm/levm/src/gas_cost.rs)
Engine API Validation (crates/networking/rpc/engine/payload.rs)
TestingEIP-8037 2D Gas Gate (test/tests/blockchain/eip8037_block_gate_tests.rs)
EIP-2780 & EIP-8038 Unit Tests
Infrastructure & ToolingMakefile Safety (tooling/ef_tests/*/Makefile)
CI/CD (.github/workflows/)
Documentation
Minor Suggestions
SummaryThe PR is ready to merge. The gas repricings are arithmetically correct, the payload validation hardening prevents cross-fork confusion, and the test coverage adequately exercises the boundary conditions of the new 2D gas gate. Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
Greptile SummaryThe PR updates Amsterdam testing to glamsterdam-devnet-8 and aligns execution, Engine API validation, fixture extraction, and CI coverage with the new bundle.
Confidence Score: 5/5The PR appears safe to merge with no actionable changed-code failures identified. Engine payload versioning, gas repricing, and fixture overlay dependencies are internally consistent, while the remaining Amsterdam coverage gaps are pre-existing and explicitly documented.
|
| Filename | Overview |
|---|---|
| crates/networking/rpc/engine/payload.rs | Splits common Cancun payload validation from version-specific Amsterdam field checks so V1–V4 reject and V5 requires the new fields. |
| crates/vm/levm/src/gas_cost.rs | Updates Amsterdam value-transfer and access-list gas prices consistently across shared intrinsic and calldata-floor callers. |
| tooling/ef_tests/blockchain/Makefile | Replaces a single-directory target with a clean overlay stamp covering regular and fork-activation Amsterdam fixtures. |
| tooling/ef_tests/engine/Makefile | Applies the same repeatable overlay strategy to Engine fixtures. |
| tooling/ef_tests/state/Makefile | Cleans stale Amsterdam state fixtures and wires the overlay into the primary LEVM and CI targets. |
| tooling/ef_tests/engine/src/fixture.rs | Propagates fixture genesis slot numbers into generated genesis JSON. |
| test/tests/blockchain/eip8037_block_gate_tests.rs | Adds focused tests for independent two-dimensional block gas limits and the execution-only transaction cap. |
| .github/workflows/daily_hive_report.yaml | Selects Amsterdam fixture bundles using an explicit matrix flag while broadening fork-boundary matching. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Bundle["glamsterdam-devnet-8 bundle"] --> Overlay["Clean Amsterdam fixture overlay"]
Overlay --> State["State tests"]
Overlay --> Blockchain["Blockchain tests"]
Overlay --> Engine["Engine tests"]
Engine --> Versions{"newPayload version"}
Versions -->|"V1-V4"| Reject["Reject slotNumber and blockAccessList"]
Versions -->|"V5"| Require["Require slotNumber and blockAccessList"]
Blockchain --> Gas["Amsterdam execution"]
State --> Gas
Gas --> E2780["EIP-2780 value-transfer pricing"]
Gas --> E8038["EIP-8038 access-list pricing"]
Reviews (1): Last reviewed commit: "ci(l1): sweep the Amsterdam fork transit..." | Re-trigger Greptile
Lines of code reportTotal lines added: Detailed view |
🤖 Codex Code Review
The runtime changes themselves looked coherent on review: the pre-Amsterdam payload field rejection in I couldn’t run targeted Automated review by OpenAI Codex · gpt-5.4 · custom prompt |
Building the mainnet vectors runs the clean target, which deleted the already-downloaded Amsterdam bundle before it could be extracted. Make the Amsterdam download order-only on the vectors dir so it happens after the clean. The state suite also deleted its own prerequisite tarball, so every invocation re-triggered the clean and re-downloaded from scratch. That wiped the Amsterdam fixtures that `download-evm-ef-tests` had just extracted, leaving the CI run with no Amsterdam coverage. Keep the tarball and depend on `amsterdam-vectors` explicitly. Note the mapper is verified against tests-glamsterdam-devnet@v8.0.0.
Restate the implementation status against the devnet-8 EIP set, drop the completed bal-devnet-7 work items, and replace them with what is actually outstanding: EIP-8070 hive coverage, a devnet-8 zkEVM bundle, eth_simulateV1 and EIP-8189.
The Amsterdam bundle contributes two subtrees, but only `for_amsterdam` was overlaid onto `vectors/eest`. `for_bpo2toamsterdamattime15k` is the only fork-activation set targeting Amsterdam and exists nowhere else, so 39 blockchain and 40 engine fixtures spanning 13 EIPs never ran. Drive both subtrees from a list and track the overlay with a stamp file, so the target no longer claims to produce just one directory.
…versions engine_newPayloadV1..V4 accepted an ExecutionPayload carrying `slotNumber` or `blockAccessList`, which are defined only from V5. A pre-fork block with a slot number was executed instead of being rejected with -32602. V3 previously doubled as both the pre-Amsterdam shape and the base of the Amsterdam one, so split the shared Cancun-onwards field checks out and let V5 keep requiring what the earlier versions now forbid.
The fork-based limits matched `fork_Amsterdam`, which does not appear in `fork_BPO2ToAmsterdamAtTime15k`, so the daily Amsterdam runs saw none of the fork-activation fixtures. Match `fork_.*Amsterdam` instead. That limit no longer carries a literal fork name, so select the Amsterdam bundle from an explicit matrix flag rather than a substring of the limit; matching on the limit would silently fall back to the mainnet bundle, which ships no Amsterdam fixtures. Widen the EIP-list limits to the full Amsterdam set.
53631af to
e283567
Compare
Benchmark Results ComparisonNo significant difference was registered for any benchmark run. Detailed ResultsBenchmark Results: BubbleSort
Benchmark Results: ERC20Approval
Benchmark Results: ERC20Mint
Benchmark Results: ERC20Transfer
Benchmark Results: Factorial
Benchmark Results: FactorialRecursive
Benchmark Results: Fibonacci
Benchmark Results: FibonacciRecursive
Benchmark Results: ManyHashes
Benchmark Results: MstoreBench
Benchmark Results: Push
Benchmark Results: SstoreBench_no_opt
|
EF Tests Comparison
|
|
On the Codex point about The stateless harness reads a different corpus. And the zkevm extraction is narrowed to one subtree: The ordering observation itself is correct and intentional: |
Amsterdam declares no blob params of its own, so per EIP-7892 the params in force are those of the highest activated BPO entry. Swapping to BPO2's entry whenever Amsterdam is active picked target 14 / max 21 on a chain that only scheduled BPO1, which rejects a parent that spent between 10 and 14 blobs: the EIP-7918 target check returns 0 where the network takes the reserve-price branch.
get_blob_schedule_for_fork read the entry named after the fork, so a named fork that declares no entry of its own reported no blob schedule at all. Resolve through the fork's activation timestamp instead, which reuses the inheritance rules already in get_fork_blob_schedule.
The light variant is a two-participant Amsterdam blob-schedule repro: BPO1 is the only scheduled BPO, so Amsterdam inherits target 10 / max 15.
EIP-8038's revised gas schedule lowers COLD_STORAGE_ACCESS to 2100 and raises ACCOUNT_WRITE to 9000, which moves CALL_VALUE, CREATE_ACCESS and the storage clear refund with it. Derive those three from their inputs so the next repricing is a one-line change.
| } | ||
|
|
||
| /// Per-storage-key access-list cost. EIP-8038 raises this from 1900 to 3000 at Amsterdam. | ||
| /// Per-storage-key access-list cost. EIP-8038 raises this from 1900 to 2900 at Amsterdam. |
There was a problem hiding this comment.
This says 2900, but the constant it documents evaluates to 2000.
ACCESS_LIST_STORAGE_KEY_COST_AMSTERDAM = COLD_STORAGE_ACCESS_AMSTERDAM - WARM_ADDRESS_ACCESS_COST
= 2100 - 100
= 2000
The 2900 figure is the address case, where COLD_ACCOUNT_ACCESS_AMSTERDAM is 3000 — that line above is correct. Both comments were moved from 3000 to 2900 together, but the storage-key one also needed recomputing because this PR drops COLD_STORAGE_ACCESS_AMSTERDAM from 3000 to 2100. So the value is right and the formula is applied consistently; only the number in the prose is stale.
Worth fixing rather than leaving, because on a gas constant the comment is what a cross-client check reads. Someone diffing ethrex against the EIP or another client sees 2900, finds 2000 in the fixtures, and has to work out which of the two is lying. 1900 to 2000 keeps the sentence shape.
Same paragraph is also the one place COLD_STORAGE_ACCESS_AMSTERDAM: 3000 -> 2100 shows up — the PR description covers the EIP-2780 and access-list moves but not that one, nor ACCOUNT_WRITE 8000 -> 9000, CALL_VALUE -> 11300 or CREATE_ACCESS -> 12000. All plausibly EIP-8038 repricing, but a reader reconciling against devnet-8 would have an easier time if the description listed them.
| pub const COLD_STORAGE_ACCESS_AMSTERDAM: u64 = 3000; | ||
| pub const ACCESS_LIST_ADDRESS_COST_AMSTERDAM: u64 = 3000; | ||
| pub const ACCESS_LIST_STORAGE_KEY_COST_AMSTERDAM: u64 = 3000; | ||
| pub const COLD_STORAGE_ACCESS_AMSTERDAM: u64 = 2100; |
There was a problem hiding this comment.
The title is test(l1): bump Amsterdam fixtures to glamsterdam-devnet-8, but this hunk changes the Amsterdam gas schedule and engine/payload.rs changes which payload versions accept slotNumber/blockAccessList. Only 2 of the 30 files are fixtures.
Flagging the type specifically because it outlives the PR: on a squash merge the title becomes the commit subject in main, so the durable record of a consensus gas change reads test(l1). Anyone later scanning history or generating release notes for behaviour changes filters it out. A stale description is ephemeral and I would not raise it; a stale type is not.
fix(l1) looks right — the body frames these as conformance corrections against devnet-8, and the engine half fixes payloads that were wrongly accepted.
3 participants
Bumps the Amsterdam fixtures from
tests-glamsterdam-devnet@v7.2.0tov8.0.0and makes the client pass them.Gas schedule
devnet-8 moved two of the repricing EIPs:
TX_VALUE_COSTof 6000 covering both the recipient balance write and the EIP-7708 transfer log, replacing the 4244 + 1756 split. A creation carrying value pays no value charge at all, sinceCREATE_ACCESSalready covers the balance write — create-with-value drops from 24756 to 23000, and the calldata-floor anchor follows because it sharesrecipient_regular_gas.WARM_ACCESS, i.e. 2900 instead of 3000, so prepaying an entry is gas neutral with the cold access it replaces. EIP-2930's extra 100 discount is not restored.Engine API
engine_newPayloadV1..V4accepted payloads carryingslotNumberorblockAccessList, which are defined only from V5. A pre-fork block with a slot number was executed instead of being rejected with-32602. V3 previously doubled as both the pre-Amsterdam shape and the base of the Amsterdam one, so the shared Cancun-onwards field checks are split out and V5 keeps requiring what the earlier versions now forbid.Fixtures and CI
.fixtures_url_amsterdam,.github/config/hive/amsterdam.yaml(fixtures+eels_commit) andAMSTERDAM_FIXTURES_BRANCHall point at devnet-8.vectors/eest, so the subtrees are cleared before extracting. Fixtures renamed between devnet releases (EIP-8037 regular gas → execution gas) otherwise survived as stale leftovers and ran against the client.for_bpo2toamsterdamattime15k. It holds the fork-activation fixtures — a header field appearing mid-chain, the gas schedule switching between blocks — and exists only in this bundle, so omitting it left the whole fork boundary untested. The hive limits becomefork_.*Amsterdamto sweep them too, and the daily report keys the Amsterdam bundle off an explicit flag rather than a substring of the limit, which no longer contains a literal fork name.clean-vectorsand would otherwise wipe the tarball between the curl and the extract. The state suite keeps its tarball, hence the new.gitignoreentry. This is what recovers the Amsterdam state fixtures noted below — previously a second clean destroyed them before the tests ran.run-evm-ef-tests-ciandtest-levmnow depend onamsterdam-vectors, so they cover Amsterdam without a separate download step.tests-zkevm@v0.6.2is filled against devnet-7 and its gas expectations no longer match a devnet-8 client. Documented indocs/known_issues.md.New tests pin the EIP-8037 two-dimensional inclusion gate and the per-tx execution-gas cap, including the cap on
max(intrinsic, calldata floor).Verification
clean-vectors: 10993 tests / 75478 fixtures, 0 failed.cargo fmt --checkandcargo clippy --all-targets -D warningsclean.Notes for review
mainthe state suite'sRun testsstep re-triggersclean-evm-ef-tests, because the$(VECTORS_DIR)rule deletes the tarball and make then rebuilds it, wipingvectors/and with it the Amsterdam fixtures theDownload EF Testsstep had just extracted.mainwas running zero Amsterdam state fixtures. The idempotency change fixes that, so they now actually execute.EF Tests (no_std crypto)job pulls no Amsterdam fixtures at all, so the no_std crypto path has zero Amsterdam coverage. Pre-existing onmain, left alone here to keep this PR scoped; worth its own change.