feat: REX6 preparation — merge back the internal REX6 line (spec + REX5 refinements)#348
feat: REX6 preparation — merge back the internal REX6 line (spec + REX5 refinements)#348RealiCZ wants to merge 39 commits into
Conversation
Reconciles the internal REX6 preparation line with public main. Resolution principles: REX<=5 semantics follow main (live behavior), REX6 behavior comes from the internal line, and main's perf/infra work is preserved: - wrap_op_compute_gas keeps main's two-arm split (plain + @frame); the surviving frame-creating compute_gas_ext wrappers use the @frame arm, and the REX6 storage-affecting handlers record through the shared record_storage_compute_gas! primitive. - storage_gas_ext::selfdestruct keeps main's static-frame fast path and all-dims trailing latch; the REX6 existing-beneficiary accounting arm is grafted in. - FrameLimitTracker keeps main's cached net_usage; internal's direct tracker writes are converted to the cache-aware helpers, with a new cache-aware add_parent_discardable for the REX6 parent-lane CREATE charge. - record_compute_gas keeps main's compute-only fast path with the REX6 Exempt sticky short-circuit folded in ahead of the latch protocol. - The shared EIP-7702 authorization scanner keeps internal's REX5/REX6 split with main's BTreeMap simulated-nonce table. - Hardfork schedules come from main's block/chain.rs registry; unknown-chain fallback now activates REX6 (unstable) at genesis. - Docs: main's sealed Rex5 pages and versioned-artifact bytecode format as the base, internal REX6 unstable blocks on top. - Benches: main's Workload/register_all framework with a rex6 row added to the shared SPEC_IDS table.
Merging this PR will not alter performance
Performance Changes
Comparing |
There was a problem hiding this comment.
Review
Scope: 57 files, mostly REX6 additions gated on MegaSpecId::REX6 (unstable) plus REX5 refinements the PR body documents as behavior-preserving. Checked the stable-spec backward-compat contract as the first-order concern (per REVIEW.md): I did not find a path where an existing stable spec (EQUIVALENCE..REX5) changes semantics because of this merge. Test coverage is dense (12 new tests/rex6/* modules totaling ~5.6k lines) and CodSpeed shows no regressions on the shared hot path.
Pre-mortem track (parallel independent review)
Ran a pre-mortem sub-agent to look for likely failure modes and verified each candidate against the code. All candidates resolve to either (a) intentional, tested design tradeoffs already documented in-tree, or (b) follow-ups the PR body's Notes section already lists. Cross-referenced for the record:
- Fee-reward
merge_persistent_usagebypassescheck_limit()—evm/execution.rs:935-940callsmerge_persistent_usageon all four trackers, and each impl (limit/{data_size,kv_update,state_growth}.rs::merge_persistent_usage) only callsadd_tx_persistent(amount)with no follow-up latch. This matches the "post-execution fee-reward accounting vs limit latching" follow-up called out in the PR body's Notes. Block-level accounting still absorbs the writes viaBlockLimiter::post_execution_update, so no live-spec effect; REX6-only. - REX6 EIP-7702
Ok(0)on pre-frame limit exceed —evm/execution.rs:598-621drops the existing-authority refund on skip, explicitly documented in the inline comment and pinned bytest_rex6_authority_state_growth_overflow_forgoes_refund. All-or-nothing is the intended semantic. create_rex6static-frame halt reorder — documented as change #11 indocs/spec/upgrades/rex6.md(only the trace-visible halt reason changes; committed gas/state are identical).best_effort_resolve_eip7702_delegate_addressswallows DB errors —evm/host.rs:263-274,350-359. By design; the doc comment spells out why (a precheck/side-mark must not turn a legitimateStackUnderflowinto a spurious DB error).MegaSpecId::default()→REX6+ CLI--specdefault →Rex6—evm/spec.rs:51,bin/mega-evme/src/common/env.rs:30. Mainnet/testnet schedules don't include REX6 (perblock/chain.rsand the PR body); doc-impact already surfaced separately, so this stays an unstable-only default flip.
No inline comments — pre-mortem verified against the code and own-track review found nothing beyond the items already documented or acknowledged.
🧬 Mutation testing — ✅ PASSDiff mutation score: 100.0% (17/17 viable mutants killed)
No new test gaps introduced by this change. 🎉 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7da79d965
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Skip fee-recipient snapshots for deposit-style transactions (op-revm credits no recipient for them), read fee-recipient balance/emptiness without hydrating bytecode, and skip the EIP-7702 authority scan when a pre-frame limit is already latched — so stateless witnesses only need accounts the transaction actually touches. Each path is pinned by an error-injecting regression test.
🧬 Mutation testing — ❌ FAILDiff mutation score: 100.0% (59/59 viable mutants killed)
Timed-out mutants (inconclusive)Each mutation below never finished within the timeout, so it was not proven caught. Make it terminate (a faster test), kill it, or — if it is a genuine non-terminating/equivalent case — add a justified entry to
Tip: run |
There was a problem hiding this comment.
🧭 Re-review update
- Summary: Incremental scope since prior review at
e7da79d9covers changes tocrates/mega-evm/src/evm/execution.rsandcrates/mega-evm/src/evm/host.rs, paired tests incrates/mega-evm/tests/rex6/eip7702_authority_accounting.rsandcrates/mega-evm/tests/rex6/fee_reward_accounting.rs, and CLI-spec doc syncs. All changes are witness-friendly follow-ups to the Codex feedback (threads already resolved). - The three fixes look correct and are each pinned by an error-injecting regression test:
- Deposit-tx guard in
reward_beneficiary(execution.rs:928) mirrors op-revm's deposit early-return; symmetric with the sandbox-tx path. - New
inspect_account_balance_and_emptinesshelper (host.rs:518) is a non-hydrating sibling ofinspect_account_code_hash; skipscode_by_hashin both branches while still marking the account cold in the vacant branch. - REX6 authority-scan skip in
validate()(execution.rs:690) usesadditional_limit.limit_exceeded()— the same conditionpre_executionatexecution.rs:617gates the auth-list apply on, so validate-time scan and pre-execution apply now agree on the doomed-tx path.
- Deposit-tx guard in
- Pre-mortem: independent sub-agent produced 5 candidates; verifier confirmed 0. PM-001/PM-005 (unstable-spec defaults + unknown-chain fallback) match documented intent (mainnet/testnet schedules pin REX≤5). PM-002 refuted from repo evidence at
evm/instructions.rs:769-770(parent_original_gas_left = gas.remaining() + forwarded_gas) — revm has deducted forwarded gas before the abort erase, so the refund is correct. PM-004 refuted:push_create_framereturnsparent_needs_update=falseat top level, sorecord_parent_discardableis never entered for a top-level CREATE; the sender account-info write is already covered atbefore_tx_start. - Inline comments: 0. Body-only items: 0 blocking.
- Still open from earlier reviews: none. All prior Codex/CodeQL threads are resolved.
Pre-mortem (unverified):
crates/mega-evm/src/limit/frame_limit.rs:197-198,208-209—pop_framerevert branches use plain-=oncached_total_usedandcached_total_refund. The invariant (child.discardable_usage ≤ cached_total_used, child.refund ≤ cached_total_refund) is enforced only by construction and a#[cfg(debug_assertions)]assert_eq!innet_usage()— not by the pop path itself. No failing input under current callers; worth confirming whether promoting these four tosaturating_sub(or a release-mode debug-assert) is desired hardening against future edits, givenadd_parent_discardablenewly writes to the parent slot.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8bf474b97a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Kill the surviving diff-scoped spec-gate mutants: exact-boundary tests for the REX5 delegate-resolution hydration gate, the MiniRex/REX contract-creation formula switch, and both directions of the REX-family unscaled-gas debug asserts (available at REX exactly; should-panic pre-REX).
There was a problem hiding this comment.
🧭 Re-review update
- Scope since prior review at
8bf474b9: a single commit1a0f8548— 14 lines incrates/mega-evm/src/limit/frame_limit.rs. This is exactly the hardening the previous re-review'sPre-mortem (unverified)item requested: pinning the pop-frame cache invariant withdebug_assert!on bothchild.discardable_usage <= cached_total_usedandchild.refund <= cached_total_refund, applied symmetrically to the parent-branch and tx_entry-branch revert paths. - The debug-only assertion is the right severity tier here: the hot path stays branch-free in release, tests with
debug_assertionstrip at the exact pop where a future direct-write mutation bypasses the cache-aware helpers, and the comment explains why "by construction every add to any entry also added to the cache" makes the invariant hold today. The subsequent-=remains a straight subtraction so release-mode underflow would still panic (in debug) — nosaturating_subcover-up. - Pre-mortem: sub-agent produced 6 candidates. All resolve without a new inline comment:
MegaSpecId::default() == REX6and unknown-chain fallback activating REX6 at genesis — both are the documented intentional posture per PR body andAGENTS.md("REX6 is the current unstable spec"); mainnet/testnet schedules do not include REX6.MegaContext::system_addressstaying atMEGA_SYSTEM_ADDRESSon the CLIrun/txpath — explicitly acknowledged as accepted design tradeoff in thecreate_evm_contextdocstring atbin/mega-evme/src/common/env.rs:189-192("these paths do not go through the block executor and do not resolve fromSequencerRegistry"). Block execution correctly resolves viaresolve_system_addressatcrates/mega-evm/src/block/executor.rs:627.FrameLimitTrackercache invariant is now backed by the pop-framedebug_assert!s added in1a0f8548; making the fields private would be a larger refactor and is not warranted here.mark_exempt()also disables gas detention — documented insystem/tx.rs:117-121; whitelist is currently onlyORACLE_CONTRACT_ADDRESS.- REX6 EIP-7702
Ok(0)on pre-frame limit-exceeded (no existing-authority refund) — already covered by prior review; the all-or-nothing semantic is intentional and pinned bytest_rex6_authority_state_growth_overflow_forgoes_refund.
- Inline comments: 0. Body-only items: 0 blocking.
- Still open from earlier reviews: three Codex-authored P1/P2 threads on
limit/frame_limit.rs:362,evm/instructions.rs:1094, andlimit/state_growth.rs:287. These are the REX6-only follow-ups the PR body'sNotessection explicitly carries forward (CREATE pre-frame accounting lifecycle, source-side SELFDESTRUCT marking under volatile access, CREATE-target preload before revm depth/balance checks). Not authored by this workflow, so left as-is.
Pre-mortem: no verifiable high-impact failure path found.
There was a problem hiding this comment.
Re-review update
- Scope since prior review at 85883cb: one commit 1119f39 - three doc-comment lines in crates/mega-evm/src/external/gas.rs (backticks around MiniRex in test docstrings). No functional change.
- All 9 prior review threads are resolved. Nothing to reconcile.
- Pre-mortem: sub-agent produced 5 candidates, none tied to the incremental commit and none newly actionable. PM-001 (REX6 reward_beneficiary writes bypassing per-tx limit re-check) is already listed in the PR body Notes as a tracked REX6-only follow-up. PM-002 through PM-005 are all self-labeled hypothesis with no decisive evidence at HEAD.
- No new inline findings.
- Still open from earlier reviews: three Codex-authored P1/P2 threads on limit/frame_limit.rs, evm/instructions.rs, and limit/state_growth.rs are the REX6-only follow-ups the PR body itself carries forward.
Pre-mortem: no verifiable high-impact failure path found.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1119f399a3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The review-driven fixes and remaining follow-up work move to a dedicated PR so this PR stays a clean, byte-faithful internal->public migration. All fix commits are preserved on the follow-up branch.
Kill the surviving diff-scoped spec-gate mutants: exact-boundary tests for the REX5 delegate-resolution hydration gate, the MiniRex/REX contract-creation formula switch, and both directions of the REX-family unscaled-gas debug asserts (available at REX exactly; should-panic pre-REX).
|
Documentation Impact This PR already adds extensive REX6 documentation, but a few gaps and one artifact remain relative to Spec Documentation
These can be addressed in this PR or a fast follow-up. |
There was a problem hiding this comment.
🧭 Re-review update
- Scope since prior review at
1119f399: one commit304fa1a—revert: reset PR to the faithful migration merge (f736086). Net effect at HEAD: reverts the six commits added on top of the migration merge (deposit-tx guard inreward_beneficiary, non-hydrating_balance_and_emptinesssibling, EIP-7702 auth-scan skip on latched limit, pop-frame cachedebug_assert!s, spec-gate boundary tests, doc-comment fixes). PR body Notes states this is intentional — those fixes move to a dedicated follow-up PR so this PR ships a byte-faithful internal→public migration. - Re-review posture: the tip is now the merge commit itself. Live-spec (
EQUIVALENCE..REX5) semantics still trace tomain; REX6 is unstable and unscheduled on mainnet/testnet percrates/mega-evm/src/block/chain.rs. No new blocking finding surfaced against the migration merge that is not already captured below. - Pre-mortem: sub-agent produced 5 candidates; verifier judged 0 as new confirmed findings. PM-001 (
MegaSpecId::default() == REX6) and PM-002 (reward_beneficiarymerge_persistent_usagebypassescheck_limit()) are both already acknowledged in earlier reviews and in the PR body Notes. PM-004 (best_effort_resolve_eip7702_delegate_addressswallows DB errors) is documented as intentional in its doc comment. PM-005 is a low-confidence hypothesis. PM-003 promoted to the Pre-mortem (unverified) section below. - Inline comments: 0. Body-only items: 0 new; existing follow-ups tracked in the open Codex threads listed below.
Still open from earlier reviews (all Codex-authored, on paths whose fixes were reverted by 304fa1a or never landed on this PR; the PR body explicitly carries these to the follow-up PR):
- P1
evm/execution.rs:800— REX6 EIP-7702 authority-scan loop uses uncheckedinitial_gas += authority_storage_gasafternew_account_storage_gasmay saturate to u64 MAX; can wrap past thegas_limit/ affordability check and admit an unfunded auth list. - P2
evm/execution.rs:824— Deposit-caller emptiness read usesinspect_account(caller, false), which can hydratecode_by_hashon the occupied branch and fail stateless/witness replay for a deposit whose caller is a lazy contract account. - P2
limit/data_size.rs:295— REX6 CREATE parent-lanerecord_parent_discardable(ACCOUNT_INFO_WRITE_SIZE)afterpush_create_framestill markscharged_parent_update, sopop_frame_unwind_parent(false)resetsparent.target_updatedon child revert even though the creator nonce bump survives. A subsequent CREATE / value-CALL at the same parent frame re-charges the creator write.limit/kv_update.rshas the same shape. - P2
system/tx.rs:134—is_system_originatedmatches a deposit-typed tx purely onsource_hash == MEGA_SYSTEM_TRANSACTION_SOURCE_HASH, so an executor input (t8n / replay) that hands in a deposit-typed tx from a non-system caller can triggerLimitCheck::Exemptinon_new_txwithout traversingbefore_runcaller/whitelist gate.
Pre-mortem (unverified):
crates/mega-evm/src/limit/limit.rs(mark_exempt) +crates/mega-evm/src/evm/context.rs:594-609(on_new_txordering) —on_new_txcallsadditional_limit.before_tx_start(&self.inner.tx)(which can latchLimitCheck::ExceedsLimit) beforemark_exempt()unconditionally stampsLimitCheck::Exempt. No caller today can hit this because the exempt path is entered only for txs theis_system_originatedgate accepts; but there is no defense-in-depth (nodebug_assert!that the state wasWithinLimit, no early-return reordering), so a future refactor ofis_system_originated(see the opensystem/tx.rs:134Codex thread) or ofmark_exemptcallers could silently unlock the metering bypass. Worth confirming whether adebug_assert!insidemark_exempt, or reorderingon_new_txto skipbefore_tx_startentirely for system txs, is desired hardening.
Pre-mortem: no verifiable high-impact failure path found.
There was a problem hiding this comment.
Re-review update: no incremental commits since prior review at 996c16a; all 13 prior review threads are resolved. Pre-mortem produced 5 candidates: PM-001 was refuted upon verification (push_create_frame returns false when the frame stack is empty, so record_parent_discardable is never invoked for a top-level CREATE; the tx-caller creator write is already recorded in DataSizeTracker::before_tx_start via add_tx_persistent). PM-002, PM-003, PM-004, PM-005 are all already covered by prior reviews or PR body Notes. No new inline findings. Pre-mortem: no verifiable high-impact failure path found.
Summary
wrap_op_compute_gas!hot-path split, the SLOAD fold, and the cachednet_usagetracker are all preserved; internal's direct tracker writes were converted to the cache-aware helpers (plus a newadd_parent_discardable), and the REX6Exemptshort-circuit is folded into the compute fast path.block/chain.rs: mainnet/testnet do not schedule Rex6; the unknown-chain fallback activates REX6 at genesis.This PR is deliberately scoped to the faithful migration only. Review-driven fixes and remaining REX6 hardening are handled in a dedicated follow-up PR so this one stays a clean, byte-faithful internal→public merge (the branch tip resets to the migration merge commit; the fix commits live on the follow-up branch).
Verification
Notes