feat(l1): support frames-devnet-0 (tests-frames-devnet@v0.0.0) - #7183
feat(l1): support frames-devnet-0 (tests-frames-devnet@v0.0.0)#7183ilitteri wants to merge 13 commits into
Conversation
Two halves: the execution rule ethrex was missing, and the harness needed to see it fail. EIP-8141 Rationale: "Cold/warm access costs for the frame's target account are charged within the frame's own gas_limit through the normal EVM warm/cold accounting, not through the per-frame cost." ethrex charged nothing and left the target cold, so a frame read its target for free and every later frame re-paid the cold price for an account an earlier frame had already touched -- gas the receipts trie and the header gasUsed both carry. The charge (and the EIP-7702 delegation access that follows the indicator) now comes out of the frame's own budget, and a frame that cannot afford to be entered forfeits its gas limit without executing. Three details the reference implementation pins, none of them obvious from the EIP text: - A VERIFY frame whose target has no code runs the protocol default code *instead of* an EVM. It builds no gas meter, so it neither pays the entry charge nor leaves its target warm. A SENDER or DEFAULT frame to a codeless account is not special: it runs an EVM over empty code and pays. - The target is warmed inside the frame's substate backup, so a failed frame contributes no warmth to later frames -- the shared journal absorbs a frame's accesses only when it succeeds. - A frame whose sender cannot fund its value never starts and spends nothing, where ethrex charged the full frame gas limit. Entering a frame reads the target account, so the EIP-7928 recorder is told about it too, before the frame runs and regardless of how it ends. Harness side, so `tests/amsterdam/eip8141_frame_transactions` from ethereum/execution-specs#3047 can run: a `Bogota` fork (Amsterdam + hegota_time, the pseudo-fork EELS fills these fixtures with), type-0x06 fixture deserialization including the `"0x"`-as-absent encoding of an implicit frame target and an ARBITRARY signature's signer, and the two TYPE_6_INVALID_{FRAME_FORMAT,SIGNATURE} exception names, whose structural rules ethrex enforces in the decoder. The two-pass parallel BAL check now runs for Amsterdam and later rather than Amsterdam exactly. 32 of the 36 frame-transaction fixtures pass; the 4 that remain are BAL mismatches on frames that revert or halt.
Cherry-picked from the hegota-devnet branch, where it was found by a devnet builder that stopped producing blocks. An EIP-8141 frame that reverts outside an atomic batch rolls back its state changes, its state gas and its logs, but left the EIP-7928 recorder holding the writes it had already recorded. The builder therefore produced a block whose access list disagreed with the state the block contains, and the block failed its own BAL validation on re-execution. Take a recorder checkpoint at frame entry and restore it whenever the frame fails, mirroring what the atomic-batch unroll already does. `restore` re-files a freshly-written slot as a read and leaves `touched_addresses` alone, so every access the frame made is still reported and only the reverted changes go. The write-then-read case is why the slot could vanish from the access list altogether rather than merely being over-reported: `record_storage_read` suppresses a read for a slot that is already written, so dropping the write without re-filing it as a read left the slot in neither storage_changes nor storage_reads. This is the last of the frame-transaction spec-test failures: all 36 fixture files from ethereum/execution-specs#3047 (415 filled cases) now pass. [ported to main: the regression test passes `None` for the stateless_validator parameter VM::new gained on main, the one-line adjustment frames-devnet-0 made in db75e42 when it merged main]
The frame entry charge used `+`, which levm's arithmetic_side_effects lint rejects; both operands are bounded by the frame's gas limit by construction, so `saturating_add` states that without changing behaviour. [ported to main from frames-devnet-0 commit 8ce49c9; that commit also fixed two identity_op sites in the eth/72 cell-request path, which exists only on the devnet branch and stays there]
…d target Found by the ethrex + Nethermind devnet, not by a fixture: the first frame transaction that paid a fresh address split the chain on the header `gasUsed`, ethrex reporting 22_910 against Nethermind's 119_766. EELS charges `charge_value_transfer_to_non_alive_account` inside `create_evm_from_frame`, so a frame whose value transfer revives an account that is not alive pays the NEW_ACCOUNT state cost at entry, out of its own gas limit and before it runs. ethrex charged nothing, so it billed the frame a bare cold access (3_000) where Nethermind billed the whole 100_000 gas limit -- the charge is larger than the limit that frame declared, so on their side the frame never ran at all. The frame's state-gas reservoir starts empty, so there is nothing to draw the charge from and it spills into the frame's execution gas in full. It also lands in the state dimension, after the per-frame baseline, so a frame that fails rolls it back and contributes none of it -- matching a frame that never created the account. No spec-test fixture covers this: every `tests/amsterdam/eip8141_frame_transactions` case that moves value moves it to an account the `pre` state already funds. All 14_900 blockchain fixtures still pass. The two existing tests that transferred to a code-less EOA were written before the charge existed and targeted an unseeded address, which is exactly the case that now costs more than their 50_000-gas frames could pay. They are about the default-code path, so their recipient is now seeded alive, and the revival case gets its own test covering both arms: unaffordable (frame forfeits its limit, account stays dead) and affordable (frame succeeds, billed cold access plus the state charge).
…arget Two consensus bugs, both from the same over-broad branch, both caught by the frame-transaction fixtures added to execution-specs#3047 after this branch was cut. EIP-8141 sends every frame except one through a top-level call: only a VERIFY frame whose resolved target has no code runs the protocol default code instead. ethrex took the default-code path for *any* codeless target, which is indistinguishable for a plain EOA -- empty code returns success having spent nothing -- but wrong for a precompile, whose dispatch happens inside that call. A SENDER or DEFAULT frame targeting one reported success with `gas_used` of zero and no output, where the precompile should have run and been billed. Fixed by branching on the frame's mode and the target's own code rather than on emptiness alone. The second is the order of the entry charges. The delegate of an EIP-7702 target was read before the frame was known to afford that access, so a frame whose gas could not cover it still filed the delegate in the EIP-7928 access list. The receipts cannot contradict that -- an unaffordable designation and a failure inside the delegate's code both forfeit the whole frame gas limit -- so the access list is the only place it shows. `eip7702_peek_delegation` already exists for exactly this ordering and is what the CALL-family handlers use; the frame path now uses it too, and resolves the delegate only once the charge is paid. Repins `.frames_spec_rev` to cd654d9a, which carries the target-resolution and block-access-list cases that caught both. All 44 frame fixtures pass, and 14908 blockchain fixtures overall. [ported to main: the .frames_spec_rev repin is dropped -- the local-fill pipeline never lands on main, since tests-frames-devnet@v0.0.0 ships these fixtures as a release and a later commit pins that instead. The docs rewrite of the frame-warmth section (originally from frames-devnet-0 commit 5b401e9) comes along here because it describes the entry-charge behaviour this commit and its predecessor implement; the old text documented the pre-charge world]
The Makefile fetches the Amsterdam bundle to `$(ARTIFACT).part` first so a failed download cannot truncate the previous one, but `*tests*.tar.gz` does not match the `.part` suffix, so a 698 MB partial was committable -- and was committed, then removed a commit later. GitHub rejects any pushed blob over 100 MB whether or not the tip still references it.
…uite
The `tests-frames-devnet@v0.0.0` release refills the whole suite at the
`Bogota` pseudo-fork, so it re-covers every Amsterdam EIP with frame
transactions active, not only the EIP-8141 tests. 3137 blockchain fixtures,
of which two failed.
EIP-8141 bounds a frame transaction's fee fields at 2**256, and one fixture
uses that room: `admission_constraints[max_cost_within_bound]` names a
`max_fee_per_gas` of 2**232 and expects the transaction to be *valid*, since
`max_cost` still fits. ethrex held both fee fields in `u64`, so it could not
decode the transaction at all and rejected the block another client accepts.
The fields are now `U256`, and the two shared accessors widen with them; most
frame call sites were already wrapping them in `U256::from`, so they lose the
wrapper rather than gain one. `GenericTransaction` and the `feeHistory` reward
still narrow, and saturate rather than call `U256::as_u64`, which panics: both
report a fee a payer must be able to cover, so a value that large cannot
change either answer.
The other failure was the harness: three overflow exception names the release
introduces (`GASPRICE_OVERFLOW`, `PRIORITY_OVERFLOW`, and for a frame
transaction `GASLIMIT_PRICE_PRODUCT_OVERFLOW`) surface at RLP decoding rather
than at validation, because the operands themselves exceed `u64`. The last of
those is already mapped for other transaction types as a validation-time
error, so both surfaces are accepted.
Pipeline: `.fixtures_url_frames` pins the release and `make frames-vectors`
overlays it, keyed on the URL like the Amsterdam overlay, replacing the local
`uv run fill` of an unreleased revision. `make run-hive-eels-frames{,-rlp,-quick}`
run it under hive.
Hive needs one patch to be useful: its ethrex client maps
`HIVE_<FORK>_TIMESTAMP` onto genesis fields and stops at Amsterdam, so the
Bogota timestamp EEST sets reaches the client as nothing, frame transactions
stay pre-fork, and every EIP-8141 test fails while the rest of the Bogota
suite passes -- 24785 passed against 159 failed on the public dashboard, the
159 being exactly the EIP-8141 payload count. `patch-hive-frames-fork` adds
`bogotaTime` to the mapper in the clone, the same way run-hive-build-block
already patches `ethrex.sh`. The one-line fix belongs upstream in
ethereum/hive.
3137/3137 Bogota fixtures, 18001 blockchain fixtures overall, full workspace
suite, clippy -D warnings and fmt clean in both workspaces.
[ported to main: the frames overlay fetches through .github/scripts/download.sh
like the Amsterdam overlay now does, instead of the raw curl-to-.part this
commit carried before main gained the retrying downloader; the RPC-receipt and
mempool replacement/eviction test hunks widened tests that exist only on
frames-devnet-0 and stay there with the mempool rules they cover]
Widening a frame transaction's fee fields to U256 widened the shared accessors with them, which broke the workspace under --features l2: the L2 fee-settlement integration test does its arithmetic in u64. Narrow at the read site, where the assumption that an L2 EIP-1559 transaction the test signs itself fits in u64 is local and checked, rather than widening the arithmetic.
grafana.com answers 403 to non-browser agents and leastauthority.com times out from CI runners; both serve fine in a browser. They failed the Link Check job on pages this branch does not touch (the monitoring docs and the audits index), so add them to the existing bot-blocked-hosts exclude list.
Widening a frame transaction's fee fields to U256 left harness comments attributing every fee-overflow rejection to `u64` fields: the type-0x06 overflow fixtures actually carry 33-byte values that fail the `U256` decode (EIP-8141 bounds fees below 2**256), while every other type still bounds fees at `u64` (the gas_limit * price fixtures carry a 31-byte price). Also stop claiming the type-0x06 decoder enforces every structural frame rule — most such fixtures reject only when the frame executes.
The fee-overflow comment in the blockchain test runner said the oversized
`gas_limit * price` fixtures fail at RLP decoding. Only the legacy-tx one
does (31-byte price, alone beyond `u64`); the frame-tx product-overflow
fixture carries a 2^255 fee that fits `U256`, decodes fine, and is caught
at validation without consulting the decode-tolerance arm — the same
decode-vs-validation split as the high-nonce fixtures.
The EIP-8141 doc's section 5 credited the `warmth/` fixtures with pinning
the unfunded-value rule ("never starts, and spends nothing"). No released
fixture covers that case — the release's affordability fixtures exercise
the entry charge, whose outcome is the opposite (full gas-limit forfeit) —
so cite the reference implementation, which reports gas_used = 0 before it
ever builds a gas meter, and scope the pinned-by sentence to the charge and
warmth-isolation rules.
|
Lines of code reportTotal lines added: Detailed view |
🤖 Kimi Code ReviewReview SummaryThis PR implements EIP-8141 (Frame Transactions) support, introducing a new transaction type with U256-bounded fee fields and complex multi-frame execution semantics. The implementation is thorough and well-tested, with careful attention to gas accounting, state isolation, and EIP-7928 Block Access List (BAL) correctness. Critical Issues1. Potential overflow in gas price calculation
Security & Correctness2. Frame entry gas accounting
3. BAL (Block Access List) isolation 4. Substate isolation Code Quality5. Type safety 6. Test coverage
Minor Suggestions7. Documentation 8. Hive patch workaround VerdictApprove with minor fix required. Address the saturating addition in Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt |
🤖 Claude Code ReviewReview: PR 7183 — frames-devnet-0 (tests-frames-devnet@v0.0.0)This is a large, well-documented spec-conformance PR porting frame-transaction implementation pieces from a dev branch to satisfy the first released EIP-8141 fixture bundle. Overall the engineering quality is high: comments consistently explain why (spec citations, EELS cross-references, prior devnet divergences found against Nethermind), and the changes are backed by extensive new unit tests plus claimed 18,017/18,017 EF fixture passes. Correctness
This is the substantive logic change. I traced the ordering carefully:
I did not find a logic bug here, but flag it as the highest-risk section to re-verify manually since it reimplements gas/warmth/state-dimension accounting across several interacting conditions ( Fee-field widening ( Consistently applied. I grepped every call site of
One inconsistency found (reported via findings): in Security / consensus
Minor/style
Non-code
Overall: solid, well-reasoned port with strong test backing. The one concrete, fixable issue is the Automated review by Claude (Anthropic) · sonnet · custom prompt |
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
|
The blockchain runner tolerated a decode failure for fixtures expecting GASLIMIT_PRICE_PRODUCT_OVERFLOW on the claim that their 31-byte legacy gasPrice does not fit a u64 field. The claim was wrong: LegacyTransaction's gas_price is U256, so the fixture decodes fine and is rejected at execution — the identical fixture already passes exactly that way in the for_amsterdam/for_cancun/for_osaka/for_prague subtrees, none of which has a tolerance for it. The disjunct was dead code for every current fixture and, worse, would have silently absorbed a future decode regression for the five copies of the fixture across five fork subtrees where main today fails loudly. expects_fee_overflow now accepts only FeeOverflow, i.e. GASPRICE_OVERFLOW and PRIORITY_OVERFLOW, whose 33-byte (2^256 or more) type-0x06 fees genuinely cannot decode into the U256 fee fields. The runner comment and the FeeOverflow doc now state the true split. Suite unchanged by the removal: 18,017 passed / 0 failed / 0 ignored.
MegaRedHand
left a comment
MegaRedHand
left a comment
There was a problem hiding this comment.
Let's review the change to U256 for max_fee_per_gas
2 participants
Motivation
First main-branch test track for the frames stream. Until now the EIP-8141 spec-test work lived only on the
frames-devnet-0dev branch; main carries the frame-transaction implementation (#6326, #7004, #7068, #7073, #7089) but nothing runs the upstream fixtures against it. This PR pins the first released bundle,tests-frames-devnet@v0.0.0, and ports from the dev branch exactly the implementation the released fixtures exercise.Description
The release refills the whole suite at the
Bogotapseudo-fork (Amsterdam + EIP-8141, activated bybogotaTime— ethrex'sFork::Hegota), so the newfor_bogota/subtree re-covers every other Amsterdam EIP with frame transactions active, not just the 8141 tests: 3,137 fixture files carrying 24,916 blockchain test cases (counted as top-level case keys), every one atnetwork: Bogota.Track wiring, following the Amsterdam overlay's shape:
tooling/ef_tests/.fixtures_url_framespins the release (fixtures_frames-devnet.tar.gz).make frames-vectorsoverlaysfor_bogota/ontovectors/eest, keyed on the release URL like the Amsterdam overlay, fetching through the retryingdownload.sh.download-test-vectorsandtest-levmdepend on it, so the existing "Run Blockchain EF tests" CI job covers the track with no workflow change — the same way the Amsterdam bundle is covered.Bogotafork config (Amsterdam +hegota_time), type-0x06 fixture deserialization (including the"0x"-as-absent encoding of an implicit frame target and an ARBITRARY signature's signer), and mappings for the exception names the release introduces:TYPE_6_INVALID_{FRAME_FORMAT,SIGNATURE}, plusGASPRICE_OVERFLOW/PRIORITY_OVERFLOW, whose fixtures surface at RLP decoding because their 33-byte type-0x06 fees are 2**256 or more — beyond the bound EIP-8141 puts on fees — and so do not fit theU256fields. (GASLIMIT_PRICE_PRODUCT_OVERFLOWneeded no new mapping and no decode tolerance: its name→exception mapping pre-exists on main, and its fixtures' fee values fitU256— legacygas_priceincluded — so they decode fine and are rejected at execution, exactly as the same fixtures already pass in main's other fork subtrees.) The two-pass parallel BAL check now runs for Amsterdam and later rather than Amsterdam exactly.make run-hive-eels-frames{,-rlp,-quick}pluspatch-hive-frames-fork, which addsbogotaTimeto hive's ethrex genesis mapper in the clone (the mapper stops at Amsterdam upstream; without it every EIP-8141 payload runs pre-fork and fails).Scope note: the bundle also ships
blockchain_tests_engine/,state_tests/, and sync/transaction subtrees; this track overlays the blockchain format only, matching the dev branch's wiring, with the hiveframes/frames-quickworkflows as the release's designated engine-format vehicle. In-process engine/state overlays (each needs a Bogota mapping in its harness) can follow separately.Implementation ported from
frames-devnet-0, each commit keeping the dev branch's reasoning; everything is reachable only through type-0x06 execution or the Hegota-gated frame path, so no pre-Hegota behavior changes:666703940): EIP-8141 Rationale — the target's cold/warm access cost is charged inside the frame's own gas limit, plus the EIP-7702 delegation access behind it. Warmth lands in the frame's substate backup so a failed frame leaves nothing warm; the EIP-7928 recorder learns of the target read regardless of how the frame ends.b24600017): a reverting non-batch frame restores a recorder checkpoint, mirroring the atomic-batch unroll, so the block access list agrees with the state the block carries (write-then-read re-files as a read).8817d7911): matches EELScharge_value_transfer_to_non_alive_accountat frame entry, found as agasUsedchain split against Nethermind.d39f4f547): only a codeless VERIFY target takes the protocol default code; a SENDER/DEFAULT frame to a precompile must run it. Also orders the EIP-7702 delegate read after the entry charge is known affordable (eip7702_peek_delegation), so an unaffordable frame files no delegate in the BAL.U256(dev999bb95dd): EIP-8141 bounds the fee fields at 2256 and the release'sadmission_constraints[max_cost_within_bound]uses the room (a 2232max_fee_per_gason a transaction that must be valid); ethrex'su64fields could not decode it. The two shared accessors widen with the fields;GenericTransactionand thefeeHistoryreward still narrow, saturating rather than panicking. The L2 fee-arithmetic test narrows at its read site (devc2161f3f2).saturating_add, dev8ce49c97b) and.gitignorefor*.tar.gz.partpartial downloads (dev1d8a5ed58).Deliberately left on the dev branch (not needed by any released fixture):
57a44764b,1e4f60cf5): approval-context restore on frame revert, atomic-batch approval-scope static rejection, the relaxed banned-opcode list, payer warming at APPROVE. The released suite passed on the dev branch before these landed; they follow with the next release that fills them.toreporting: invisible to blockchain fixtures.uv fillpipeline and theframes-devnet-0.yamlkurtosis config (depends on the dev-only glamsterdam-devnet-8 base config).STORE_SCHEMA_VERSIONis untouched: nothing here changes a storage codec — the frame-receipt storage encoding and its flush call sites were already on main.CI housekeeping picked up along the way:
grafana.comandleastauthority.comjoinedlychee.toml's bot-blocked-hosts exclude list. Both serve fine in a browser but 403/time out from CI runners, and they were failing the docs Link Check on pages this PR does not touch (the monitoring docs and the audits index).Local test run
make -C tooling/ef_tests/blockchain test-levm(Bogota overlay included): 18,017 / 18,017, 0 ignored — 14,880 pre-existing + 3,137for_bogota(isolated-- for_bogotare-run: 3,137 / 3,137, matching the dev branch's count for this release).make -C tooling/ef_tests/blockchain test-stateless: 3,218 / 3,218.make -C tooling/ef_tests/engine test: 11,020 / 11,020 (75,641 fixture cases), 0 failed.cargo test --workspace --exclude 'ethrex-l2*' --exclude ethrex-prover --exclude ethrex-guest-program: 1,870 / 1,870 across 48 suites.cargo clippy --workspace --all-targets -- -D warnings: clean in both workspaces;cargo fmt --all -- --check: clean in both workspaces.