Skip to content

feat(l1): support frames-devnet-0 (tests-frames-devnet@v0.0.0) - #7171

Closed
ilitteri wants to merge 11 commits into
mainfrom
conformance/frames/tests-frames-devnet-v0.0.0
Closed

feat(l1): support frames-devnet-0 (tests-frames-devnet@v0.0.0)#7171
ilitteri wants to merge 11 commits into
mainfrom
conformance/frames/tests-frames-devnet-v0.0.0

Conversation

@ilitteri

@ilitteri ilitteri commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Motivation

First main-branch test track for the frames stream. Until now the EIP-8141 spec-test work lived only on the frames-devnet-0 dev 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 Bogota pseudo-fork (Amsterdam + EIP-8141, activated by bogotaTime — ethrex's Fork::Hegota), so the new for_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 at network: Bogota.

Track wiring, following the Amsterdam overlay's shape:

  • tooling/ef_tests/.fixtures_url_frames pins the release (fixtures_frames-devnet.tar.gz).
  • make frames-vectors overlays for_bogota/ onto vectors/eest, keyed on the release URL like the Amsterdam overlay, fetching through the retrying download.sh. download-test-vectors and test-levm depend 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.
  • Harness: a Bogota fork 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} and the three overflow names, which surface at RLP decoding because the fixture values do not fit the fields (GASPRICE_OVERFLOW/PRIORITY_OVERFLOW: 33-byte fees on type-0x06 transactions, beyond the 2**256 bound EIP-8141 puts on fees and their U256 fields; GASLIMIT_PRICE_PRODUCT_OVERFLOW: a legacy-tx price beyond u64). The two-pass parallel BAL check now runs for Amsterdam and later rather than Amsterdam exactly.
  • Hive: make run-hive-eels-frames{,-rlp,-quick} plus patch-hive-frames-fork, which adds bogotaTime to 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).

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:

  • Frame entry access charge (dev 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.
  • BAL reconciliation when a frame reverts on its own (dev 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).
  • EIP-8037 NEW_ACCOUNT state cost when a frame revives a dead target (dev 8817d7911): matches EELS charge_value_transfer_to_non_alive_account at frame entry, found as a gasUsed chain split against Nethermind.
  • Run a frame's target through the EVM unless it is a codeless VERIFY target (dev 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.
  • Frame-tx fee fields widened to U256 (dev 999bb95dd): EIP-8141 bounds the fee fields at 2256 and the release's admission_constraints[max_cost_within_bound] uses the room (a 2232 max_fee_per_gas on a transaction that must be valid); ethrex's u64 fields could not decode it. The two shared accessors widen with the fields; GenericTransaction and the feeHistory reward still narrow, saturating rather than panicking. The L2 fee-arithmetic test narrows at its read site (dev c2161f3f2).
  • Clippy conformance on the entry charge (saturating_add, dev 8ce49c97b) and .gitignore for *.tar.gz.part partial downloads (dev 1d8a5ed58).

Deliberately left on the dev branch (not needed by any released fixture):

  • Post-v0.0.0 EIP revisions (dev 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.
  • Mempool-admission work (validation-prefix SLOTNUM ban, post-prefix rules, replacement/eviction, sponsored-VERIFY targeting) and RPC receipt to reporting: invisible to blockchain fixtures.
  • The pre-release local uv fill pipeline and the frames-devnet-0.yaml kurtosis config (depends on the dev-only glamsterdam-devnet-8 base config).

STORE_SCHEMA_VERSION is 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.com and leastauthority.com joined lychee.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,137 for_bogota (isolated -- for_bogota re-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.

ilitteri and others added 8 commits August 20, 2026 12:49
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.
@ilitteri
ilitteri requested a review from a team as a code owner August 20, 2026 16:46
@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown

⚠️ Known Issues — intentionally skipped tests

Source: docs/known_issues.md

rpc-compat log-bearing cases excluded

Where: KNOWN_EXCLUDED_TESTS in .github/scripts/check-hive-results.sh counts out
eight hive rpc-compat cases — the four eth_getLogs cases, eth_getBlockReceipts/get-block-receipts-latest,
and three eth_getTransactionReceipt cases. They are exactly the cases whose recorded
response contains at least one log object; every case with an empty log array still runs.
Note this leaves eth_getLogs with no rpc-compat coverage at all, since all four of its
cases are in the set.

Why: ethrex populates blockTimestamp on log objects, as geth, besu, nethermind, reth
and erigon all do. hive's rpc-compat compares responses byte-exactly (jsondiff.FullMatch;
the lenient checkJSONStructure path applies only to cases upstream marks speconly), and
the corpus is pinned to execution-apis d08382ae (2025-02-10), whose recordings predate the
field — it entered the schema in execution-apis#639 and the fixtures in #846 (2026-07-22).
So the extra key cannot match, and this is a property of the pin rather than of the response.

The pin cannot move, and this is not temporary. The pin sits one commit before
execution-apis#627, which moved the test chain to a pre-merge genesis: the current corpus has
~36 proof-of-work blocks before its terminal total difficulty. ethrex does not support
pre-merge chains and will not, so importing that chain.rlp fails at block 1 —
validate_block_header has no pre-London base-fee path. Every revision carrying
blockTimestamp in its fixtures also carries that chain, so there is no revision that
satisfies both. Nor can the corpus be patched locally: rpc-compat's Dockerfile clones
ethereum/execution-apis by hard-coded URL, so the branch buildarg cannot point at a fork.

Coverage: the field itself is pinned by
block_timestamp_is_on_the_log_and_not_on_the_receipt in
crates/networking/rpc/types/receipt.rs, which asserts it is present on each log and absent
from the receipt level.

Removal: delete the entries if ethrex ever gains pre-merge chain import, or if upstream
marks these cases speconly so they are type-checked instead of compared byte-for-byte.


The stateless schema id does not identify the encoding

Where: STATELESS_INPUT_SCHEMA_ID in crates/common/types/stateless_ssz.rs.

Upstream keeps the stateless input schema id at 0x1501
(fork_index 0x15 << 8 | revision 0x01) across incompatible body changes. Three
encodings have now shipped under it: tests-zkevm@v0.6.2, then #3248 + #3278,
then #3356, which moved state, codes and public_keys from SszList to
ProgressiveList. ethrex speaks the last one.

The consequence is that the 2-byte prefix cannot be used to detect a stale or
mismatched bundle. A wrong-dialect input is accepted by the id check and then
fails later — in SSZ decode, or on a root that does not match — rather than being
rejected up front for what it is. only_amsterdam_schema_id_decodes therefore
proves less than its name suggests.

Worth raising upstream: a revision field that does not move across a body change
provides no version negotiation at all.


ZisK guest program hash changes with the unsync_cell gate

Where: crates/common/types/block.rs, transaction.rs.

The gate on the single-threaded unsync_cell::OnceCell moved from
all(feature = "eip-8025", target_arch = "riscv64") to
all(feature = "zisk", target_arch = "riscv64") when the eip-8025 feature was removed.

The guest ELFs were previously built --features "<zkvm>-build-elf,ci", which never enabled
eip-8025, so they compiled the atomic once_cell variant. bin/zisk/Cargo.toml does enable
ethrex-common/zisk, so the ZisK guest now compiles the unsafe impl Sync cell instead.
That changes the ELF bytes and therefore the program hash and verification key.

This is intended (the guest is single-threaded, so the unsync cell is sound and cheaper), but it
is a VK change rather than a no-op refactor, and the diffstat presents it as a file rename
(eip8025_cell.rsunsync_cell.rs). Anyone pinning a ZisK VK across this change must
re-register it. The stateless-validator crate now forwards ethrex-common/zisk from its own
zisk feature so the two ZisK guests do not disagree on the cell type.


Release signing key is an unprotected repository secret

Where: .github/workflows/tag_release.yaml.

MINISIGN_SECRET_KEY is a plain repository secret. There is no environment: on
finalize-release or dry-run-release-assets, and gh api repos/lambdaclass/ethrex/rulesets
shows only branch-targeted rulesets, so the github.ref_type == 'tag' condition is a workflow
check rather than an enforced boundary: anyone who can push a tag can reach the signing key.

This is a repository-settings change, not a code change, so it is recorded here rather than
fixed in the tree. Recommended:

  1. Move MINISIGN_SECRET_KEY / MINISIGN_PASSWORD into a GitHub Environment with required
    reviewers, and add environment: to the two jobs that sign.
  2. Add a ruleset targeting refs/tags/v* restricting who may create release tags.

Until then, the compromise of that key is silent and durable: signatures would still verify
against the committed .github/minisign.pub.

@github-actions github-actions Bot added the L1 Ethereum client label Aug 20, 2026
@github-actions

Copy link
Copy Markdown

🤖 Kimi Code Review

This PR implements EIP-8141 (Frame Transactions) support with U256 fee fields and correct frame-entry gas accounting. The changes are substantial but well-structured.

Correctness & Security

  1. U256 Fee Field Handling (crates/common/types/transaction.rs:1468-1495, crates/vm/levm/src/vm.rs:1579-1585)
    The widening of max_fee_per_gas and max_priority_fee_per_gas to U256 for Frame transactions (while keeping other types as u64) is correctly implemented. The Option<U256> return types prevent mixing with legacy transaction logic, and saturating conversions in test fixtures (tooling/ef_tests/blockchain/types.rs:603-620) safely handle values exceeding u64 bounds.

  2. Frame Entry Gas Accounting (crates/vm/levm/src/vm.rs:1798-1893)
    The logic correctly charges cold/warm access costs and EIP-8037 NEW_ACCOUNT state costs from the frame's own gas limit before execution begins. The runs_default_verify_code check properly exempts VERIFY frames with no code from EVM entry charges while ensuring SENDER/DEFAULT frames to empty accounts still pay.

  3. BAL Checkpoint Consistency (crates/vm/levm/src/vm.rs:1810, 2146-2155)
    The introduction of frame_bal_checkpoint and restoration on frame revert correctly reconciles the Block Access List recorder with state reverts. This prevents a critical consensus bug where reverted frames would leave stale write records in the BAL.

  4. EIP-7702 Delegation Handling (crates/vm/levm/src/vm.rs:1822-1870)
    The "peek first, warm after affordability check" pattern correctly prevents unaffordable frames from touching the delegatee address, matching the EIP-7928 witness requirements. The separation of target_bytecode and delegation resolution is architecturally sound.

Performance & Maintainability

  • Memory: BAL checkpointing clones the recorder state on every frame entry (vm.rs:1810). While acceptable for typical frame counts (expected to be low in practice), consider documenting this allocation in performance-critical paths.
  • Readability: The extensive comments in vm.rs explaining the frame entry flow and BAL reconciliation are excellent and necessary given the complexity of EIP-8141's execution rules.

Minor Observations

  • Fee Market RPC (crates/networking/rpc/eth/fee_market.rs:261-271): The unwrap_or(u64::MAX) clamping for Frame transaction effective priority fees in percentile calculations is safe per the PR rationale (unaffordable values), but document this explicitly in the RPC spec comments to avoid confusion.

  • Test Coverage: The addition of sender_frame_reviving_a_dead_target_pays_new_account and reverted_frame_refiles_its_writes_as_reads_in_the_bal tests covers the specific edge cases of EIP-8037 revival costs and BAL atomicity. Good.

Verdict: The PR correctly implements the EIP-8141 specification regarding U256 bounds, frame-entry access costs, and state isolation. The BAL checkpoint logic is critical for consensus correctness and appears sound. Approved with the minor suggestion to document the RPC fee clamping behavior.


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

@github-actions

Copy link
Copy Markdown

Lines of code report

Total lines added: 73
Total lines removed: 0
Total lines changed: 73

Detailed view
+-------------------------------------------------------+-------+------+
| File                                                  | Lines | Diff |
+-------------------------------------------------------+-------+------+
| ethrex/crates/common/types/transaction.rs             | 5071  | +2   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/eth/fee_market.rs        | 239   | +4   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/vm/backends/levm/mod.rs                 | 3796  | +1   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/opcode_handlers/frame_tx.rs | 507   | +2   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/vm.rs                       | 2247  | +64  |
+-------------------------------------------------------+-------+------+

@github-actions

Copy link
Copy Markdown

🤖 Codex Code Review

Findings:

  1. eth_feeHistory now silently truncates valid frame-tx tips above u64::MAX. The new frame fee fields are full U256 (transaction.rs, transaction.rs), but calculate_percentiles_for_block() clamps a frame tx’s effective priority fee with u64::try_from(...).unwrap_or(u64::MAX) (fee_market.rs). That makes the reward output incorrect for any included frame tx whose paid tip exceeds 64 bits. The comment there is also relying on a false assumption: balances are U256, so such fees are not inherently “unaffordable”. This should return exact hex quantities, not a saturated sentinel.

  2. The EF blockchain harness still treats GASPRICE_OVERFLOW / PRIORITY_OVERFLOW as “must fail during RLP decoding”, but this PR changed frame tx fee fields to decode as U256. The expectation mapping was added in deserialize.rs, and the fast-path only recognizes it through exception_in_rlp_decoding() (test_runner.rs). Once type-6 fee fields no longer overflow decode, exception_is_expected() has no FeeOverflow arm at all (test_runner.rs). Result: any Bogota fixture expecting those exceptions will now be reported as a test failure even if ethrex rejects the tx later for the right reason.

  3. The new EF fixture conversion for frame txs can still panic on malformed inputs instead of reporting the expected invalid-frame error. In types.rs, mode, flags, and scheme are converted with try_into().unwrap(). If a frames-devnet fixture uses out-of-range integers for one of those fields, the harness aborts during JSON conversion instead of cleanly exercising TYPE_6_INVALID_FRAME_FORMAT. That is especially brittle in the exact malformed-input area this PR is adding coverage for.

I couldn’t run cargo test here because rustup tried to write under /home/runner/.rustup, which is read-only in this sandbox. Aside from the points above, the U256 widening itself looks consistent through the main LEVM/mempool call sites.


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

@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown

Benchmark Results Comparison

No significant difference was registered for any benchmark run.

Detailed Results

Benchmark Results: BubbleSort

Command Mean [s] Min [s] Max [s] Relative
main_revm_BubbleSort 2.807 ± 0.016 2.788 2.839 1.12 ± 0.01
main_levm_BubbleSort 2.506 ± 0.015 2.491 2.533 1.00
pr_revm_BubbleSort 2.799 ± 0.008 2.792 2.821 1.12 ± 0.01
pr_levm_BubbleSort 2.509 ± 0.038 2.491 2.617 1.00 ± 0.02

Benchmark Results: ERC20Approval

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ERC20Approval 912.9 ± 9.0 904.4 929.9 1.00
main_levm_ERC20Approval 948.6 ± 9.0 937.4 968.5 1.04 ± 0.01
pr_revm_ERC20Approval 921.3 ± 17.6 903.5 956.6 1.01 ± 0.02
pr_levm_ERC20Approval 943.5 ± 6.0 937.1 957.7 1.03 ± 0.01

Benchmark Results: ERC20Mint

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ERC20Mint 119.2 ± 1.3 118.1 122.2 1.00
main_levm_ERC20Mint 145.9 ± 1.1 144.9 148.3 1.22 ± 0.02
pr_revm_ERC20Mint 119.3 ± 1.9 118.0 124.4 1.00 ± 0.02
pr_levm_ERC20Mint 146.2 ± 0.7 145.2 147.9 1.23 ± 0.02

Benchmark Results: ERC20Transfer

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ERC20Transfer 218.0 ± 4.4 213.5 225.3 1.01 ± 0.02
main_levm_ERC20Transfer 240.1 ± 1.6 237.9 243.4 1.11 ± 0.02
pr_revm_ERC20Transfer 215.6 ± 2.8 213.7 222.5 1.00
pr_levm_ERC20Transfer 239.4 ± 2.1 237.3 244.6 1.11 ± 0.02

Benchmark Results: Factorial

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Factorial 185.7 ± 7.4 181.3 205.3 1.01 ± 0.05
main_levm_Factorial 207.6 ± 4.9 204.5 219.0 1.13 ± 0.04
pr_revm_Factorial 184.0 ± 5.5 181.2 199.7 1.00
pr_levm_Factorial 205.3 ± 1.0 204.2 207.4 1.12 ± 0.03

Benchmark Results: FactorialRecursive

Command Mean [s] Min [s] Max [s] Relative
main_revm_FactorialRecursive 1.331 ± 0.063 1.171 1.409 1.00
main_levm_FactorialRecursive 6.605 ± 0.047 6.554 6.707 4.96 ± 0.24
pr_revm_FactorialRecursive 1.337 ± 0.026 1.295 1.364 1.00 ± 0.05
pr_levm_FactorialRecursive 6.629 ± 0.067 6.517 6.762 4.98 ± 0.24

Benchmark Results: Fibonacci

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Fibonacci 154.3 ± 2.1 152.6 159.7 1.00
main_levm_Fibonacci 179.1 ± 1.3 176.5 181.6 1.16 ± 0.02
pr_revm_Fibonacci 154.8 ± 2.3 152.1 159.5 1.00 ± 0.02
pr_levm_Fibonacci 179.8 ± 2.8 176.2 183.8 1.17 ± 0.02

Benchmark Results: FibonacciRecursive

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_FibonacciRecursive 721.4 ± 5.0 715.7 731.9 1.25 ± 0.01
main_levm_FibonacciRecursive 586.4 ± 15.7 576.1 629.9 1.01 ± 0.03
pr_revm_FibonacciRecursive 720.5 ± 7.9 708.0 730.6 1.24 ± 0.01
pr_levm_FibonacciRecursive 579.3 ± 2.6 576.5 585.0 1.00

Benchmark Results: ManyHashes

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_ManyHashes 8.7 ± 1.5 8.0 13.0 1.00
main_levm_ManyHashes 9.4 ± 0.7 8.8 10.8 1.08 ± 0.21
pr_revm_ManyHashes 9.0 ± 2.0 7.9 14.2 1.03 ± 0.29
pr_levm_ManyHashes 9.5 ± 0.9 8.9 12.0 1.09 ± 0.22

Benchmark Results: MstoreBench

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_MstoreBench 260.1 ± 0.9 258.0 260.9 1.50 ± 0.01
main_levm_MstoreBench 177.5 ± 7.7 171.3 195.2 1.02 ± 0.04
pr_revm_MstoreBench 260.7 ± 1.6 258.5 263.4 1.50 ± 0.02
pr_levm_MstoreBench 173.7 ± 1.5 170.9 175.7 1.00

Benchmark Results: Push

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_Push 242.9 ± 2.7 241.1 250.4 1.23 ± 0.05
main_levm_Push 197.4 ± 6.2 193.9 214.4 1.00 ± 0.05
pr_revm_Push 243.4 ± 2.5 240.8 248.3 1.23 ± 0.05
pr_levm_Push 197.1 ± 7.4 193.8 217.1 1.00

Benchmark Results: SstoreBench_no_opt

Command Mean [ms] Min [ms] Max [ms] Relative
main_revm_SstoreBench_no_opt 140.8 ± 1.9 139.0 144.9 1.48 ± 0.05
main_levm_SstoreBench_no_opt 95.3 ± 3.0 93.8 103.9 1.00
pr_revm_SstoreBench_no_opt 140.8 ± 3.3 138.9 149.8 1.48 ± 0.06
pr_levm_SstoreBench_no_opt 95.6 ± 4.6 93.8 108.8 1.00 ± 0.06

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.
@ilitteri ilitteri closed this Aug 21, 2026
@ilitteri
ilitteri deleted the conformance/frames/tests-frames-devnet-v0.0.0 branch August 21, 2026 01:13
@ilitteri

Copy link
Copy Markdown
Collaborator Author

Superseded by #7183: renaming the head branch closed this PR (GitHub closes PRs whose head branch is renamed); the same commits continue there under the new branch name.

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

Labels

L1 Ethereum client

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant