docs: specify compute gas accounting and pin it with a cross-spec test harness - #358
docs: specify compute gas accounting and pin it with a cross-spec test harness#358RealiCZ wants to merge 18 commits into
Conversation
…t harness Compute gas had no real specification: roughly 40 lines split across resource-accounting.md and dual-gas-model.md, saying little beyond "same as vanilla EVM gas". The implementation records it at nine distinct sites, and the measurement itself changes across five specs. Add docs/spec/evm/compute-gas.md as the authoritative page. It defines compute gas as a derived measurement of inherited EVM gas rather than a gas schedule of its own: the measurement window and its three exclusions (storage gas, forwarded child gas, refunds), the per-opcode metering classes with per-spec assignment, every non-opcode recording site, the transaction and per-call-frame limits, and the exceed behaviour. The recording sites are closed by an accounting identity so a missing or extra site does not balance. Migrate the compute-gas content out of resource-accounting.md and the metering amount out of dual-gas-model.md, leaving pointers, so there is a single normative source. Correct dual-gas-model.md's total-gas equation, which was stated as an identity but does not hold once an EVM refund is settled. Add crates/mega-evm/tests/compute_gas: a golden snapshot of 32 programs across all nine specs, plus seven tests asserting the rules behind the numbers rather than the numbers alone. Stable-spec rows must never move; a diff there is a replay-breaking regression.
Claude review status
✅ Review clean Last reviewed: New this round: 0 finding(s), 0 question(s) · Resolved this round: 0 · Open questions: 0 |
🧬 Mutation testing — ✅ PASSNothing to test — no mutants were generated on the changed lines. |
🧬 Mutation testing — ✅ PASSNothing to test — no mutants were generated (1 unviable, 0 timed out). |
RealiCZ
added
spec:unchanged
Codecov Report✅ All modified and coverable lines are covered by tests. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
… answers Clippy's doc_markdown lint is an error in this workspace: bare MegaETH, MiniRex, and MegaLimitControl in doc comments need backticks. Separate each Rationale question and each Security Considerations condition from its body with a blank line, so they render as distinct paragraphs rather than running together on one line.
The Class Assignment section gave only the MiniRex base plus six rows of forward deltas, so answering "what class is SLOAD under the spec this page describes" required replaying the whole inheritance chain by hand. The page declares spec: Rex5, so the current assignment belongs in the page as a table, not as something the reader derives. Lead with the current (Rex5/Rex6) assignment, and keep the base plus per-spec deltas below it under a subheading, since replay implementations need every spec's assignment. Note separately that the CALL family and SELFDESTRUCT keep their own classes while additionally carrying a beneficiary volatile-access guard, which the class table alone did not convey.
The page restated rules that other pages already own, creating a second normative source for each. Replace with pointers: - Per-call-frame budget derivation is specified for all four resource dimensions in resource-limits.md; keep only the compute-gas-specific facts (which pre-frame usage the top-level budget deducts, the check ordering, and why the budget is a guardrail rather than a guarantee). - The detained-limit update rule, cap combination, and per-transaction reset are specified in gas-detention.md; keep only how the resulting limit applies. - Drop the Detention Reset subsection, which restated gas-detention.md in full. Fix references left stale by earlier edits: the precompile prose still spoke of "Case 2" and "Cases 2 and 3" after the numbered list became if/else pseudocode, and the effective-gas-limit formula referenced a remaining() function whose definition had been removed. Bind intrinsic_recorded to its subsection. Demote the one fifth-level heading; every other spec page stops at four.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09f4eb4c75
ℹ️ 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".
Window boundaries contradicted the storage-gas exclusion: the boundary rule demanded the window open before any EVM gas movement attributable to the opcode, while the exclusion explicitly permits opening it after the storage-gas charge. Restate the rule as covering the opcode's compute work, and name the storage-gas charge as the one movement that may fall on either side. The dual-gas-model receipt rule was still wrong after the refund correction. Refunds are not the only thing separating the receipt from the tracked counters: gas consumed by an operation whose measurement window never closes is deliberately unrecorded yet still deducted, so the receipt can exceed the tracked total as well as fall below it. State both effects and require the receipt be derived from the consumed budget rather than from the counters. Three narrower corrections: - A frame-local exceed was said not to fail the transaction. That holds only for nested frames; the top-level frame has a budget too, and exceeding it makes the transaction's own result the revert. - Both keyless-deploy exceed branches were said to report InsufficientComputeGas. Only the frame-local branch does; the transaction-level branch halts with OutOfGas and empty output. - The per-call-frame budget omitted Rex4's storage-stipend cap, without which a Rex4 child frame can spend the system-granted stipend on computation. Narrow the test corpus's completeness claim to the recording sites reachable from a plain transaction, and point at the suite that covers KeylessDeploy. Record in the Rationale why this page keeps per-spec behavior in the main specification instead of relocating it to the Spec History, as the documentation conventions otherwise require.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02bc3d11d1
ℹ️ 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".
forwarded_child_gas did not say which child gas limit it means. Under Rex4 the storage gas stipend is granted by inflating the child's limit after the parent's measurement window has closed, so an implementation reading child.gas_limit as the limit the child receives subtracts an extra 23,000 and under-records the parent. State that the pre-inflation value applies on every spec. CLZ was listed in the Plain class. It requires a base revision later than the one MegaETH inherits, so it halts before its window closes and records nothing on every current spec; listing it as metered would lead an independent implementation to enable an unavailable opcode during stable-spec replay. Move it to Untracked with the reason. Code-deposit recording was stated unconditionally. The deposit does not happen when the runtime code is oversized, starts with the EIP-3541 reserved byte, or cannot be paid for, and nothing is recorded in those cases. State the conditions, which hold on every spec. Restate the interception requirement as the invariant it is meant to express — budgets seen by later frames are unchanged — rather than mandating a push/pop tracker strategy, which the documentation conventions exclude.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30ed0e4dc8
ℹ️ 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".
Most of these are residue from earlier repairs — each patch edited one passage while the page cross-references itself heavily. Contradictions with other pages: - resource-accounting.md still defined compute gas as the cumulative gas consumed by EVM execution, which contradicts the single-record rule: gas consumed by an operation whose window never closes is deliberately not recorded. - precompiles.md required the KZG precompile to charge exactly its fixed cost whenever the supplied gas suffices. That holds on a successful return only; on a failure after the precompile runs, the caller loses the whole forwarded limit. The snapshot shows 200,000 forwarded and 200,000 burned. - The canonical metering order's final step read as deferring non-compute recording until after compute-limit enforcement, while SELFDESTRUCT records those dimensions before its body runs. The step fixes when an exceed is acted on, not when usage is recorded; say so. Residue from earlier edits: - Adding a leading "no child frame" branch to forwarded_child_gas made the prose's "second branch" point at the CALL_STIPEND case, sending CREATE and CREATE2 to a branch that subtracts a stipend they never receive. - The accounting identity claimed every term is a subsection below it; the per-opcode windows are defined above, in Measurement Window. - Two test doc comments still called the KZG rule "case 2" after the numbered list became if/else pseudocode. - The oversized-CREATE2 corpus comment attributed Rex5's eager recording to every pre-Rex6 spec, contradicting both the specification and the test's own assertion that MiniRex-Rex4 skip it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0932e530bd
ℹ️ 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".
![mega-maxwell[bot]](https://avatars.githubusercontent.com/u/138558126?s=60&v=4){kind=small}
There was a problem hiding this comment.
❓ Review complete — 1 open question(s)
Reviewed 30ed0e4d..0932e530.
Incremental doc-only round: KZG failure semantics, compute-gas tracking definition, step-5 surface/act wording, CALL_STIPEND branch rename, test comment cleanup. Four prior rounds all clean; no prior findings or questions.
Open questions — answer them in a reply on this PR. Each one is marked answered here once a later review round confirms the answer, so this list stays current:
✅ **Answered** — test_selfdestruct_storage_surcharge_stays_out_of_compute_gas uses MULTIPLIER=4 (surcharge=75,000) and the main.rs module comment documents that min-bucket cases are covered by corpus while non-unit-bucket cases are covered by claim tests.
- The SELFDESTRUCT corpus cases all use
EmptyExternalEnv(SALT multiplier=1, so Rex5 empty-beneficiary surcharge=0). A regression routing this surcharge into compute-gas would be invisible in the snapshot. Is the missing non-unit-bucket SELFDESTRUCT case a deliberate scope choice or an untracked gap? - Why it matters: The snapshot is the primary regression net for compute-gas recording. A recording error on the Rex5 SELFDESTRUCT empty-beneficiary path would be invisible until a non-trivial SALT environment is exercised.
- How to verify: Check whether
MemoryDatabasesupports a non-unit SALT bucket multiplier; if so, add aselfdestruct_to_empty_nonmin_bucketcorpus entry and confirm Rex4 and Rex5 produce identical compute-gas rows.
Fixes the remaining review findings, most of which are contradictions between pages that this work surfaced rather than introduced. Receipt and total gas. The receipt is not the sum of the two metered counters. Three effects separate them: refunds lower it, the calldata floor can raise it, and gas the transaction pays for but neither counter records raises it. That third quantity now has a name and an enumeration of its three sources — an operation halting before its measurement window closes, the gap between the gas limit forwarded into a failing precompile and the compute gas recorded for it, and pre-Rex6 forwarded gas that a compute-gas halt never returns. "Total gas" keeps its original meaning, since six passages across four pages depend on it; the distinctions are layered underneath instead. Interception. Interception fires at frame initialization, not after a call opcode: a transaction whose recipient is a system contract has no initiating opcode, and KeylessDeploy must be intercepted on exactly that path. The opcode-level accounting statements are now conditioned on an opcode existing, in the specification, the gas semantics, and the rationale alike. The generic "an interception records no compute gas" rule is scoped to interceptors that do not meter themselves, with KeylessDeploy named as the exception it always was. Resource limits. The blanket rule that a transaction-level limit exceed preserves remaining gas now records the keyless-deploy exception, which rescues only from Rex6. Intrinsic gas. Contract-creation transactions carry the inherited creation surcharge and the EIP-3860 initcode-word charge; both belong to the recorded intrinsic compute gas. SELFDESTRUCT. It records all three non-compute dimensions before its body runs, so it evaluates all four together afterwards. The reported dimension follows the fixed order, which decides in both directions: data size and key-value updates outrank compute gas, compute gas outranks state growth. Tests. Adds a claim test for the Rex5 empty-beneficiary storage surcharge, which the snapshot corpus cannot reach because it runs entirely against minimum-capacity SALT buckets, and corrects the corpus's coverage claim and the snapshot header's description of gas_used accordingly.
A local `GAS` constant shadowed the `GAS` opcode byte imported at the top of the file, giving one identifier two meanings; rename it to FORWARDED_GAS. Four assertions subtracted one u64 reading from another before comparing. A regression that inverted the expected ordering panicked on arithmetic overflow instead of printing the authored message with both values, which is exactly when that message is needed. Route all four through checked_sub with a diagnostic. Verified by inverting the SELFDESTRUCT fixture, which now reports "Rex5 must not charge less total gas than Rex4 (Rex4=167603 Rex5=92603)".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec7e990bd8
ℹ️ 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".
…condition The unmetered-gas list has now been found incomplete three times — first two sources, then three, now the pre-Rex6 KeylessDeploy full-spend halt, which forfeits the sender's whole envelope and is none of the listed cases. The problem is the framing, not the count: an exhaustive enumeration of "gas in neither counter" invites exactly this. Define the quantity as a remainder of the gas actually charged, require nodes to derive it that way rather than by summing, and let the list be what produces it. The KeylessDeploy case joins it. Separately, the forwarded-gas condition asked whether a child frame was spawned. The exclusion is decided by what the opcode produced, not by what frame initialization later did with it: a pending child call can resolve as a precompile, a system-contract interception, or a too-deep result, with no child EVM frame ever executing, and the parent has parted with the gas in every one of those cases. Ask about the pending call instead, and say so.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c227975d95
ℹ️ 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 decomposition presumed the counters could only undercount the charge. They can also overcount it: from Rex5 a value-transferring CALL or CALLCODE records the inherited CALL_STIPEND in the parent's compute gas even though the EVM hands it to the callee without deducting it from the caller. The checked-in Rex5 call_value_to_existing row is the counterexample — compute 32,621 plus the 39,000 storage intrinsic is 71,621 against a 69,321 receipt, with no refund and no floor involved, so no non-negative remainder satisfies the formula. This is the second consecutive round where the formalism rather than the content was wrong: last round an exhaustive list, this round an equation. The counters and the charge are separate accounts kept for different purposes, and each formula written to relate them has been falsified by another mechanism. Drop the equation. State that no equation relates them, that the charge comes from the gas meter, and give the mechanisms that push the totals in each direction.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d0a3dcb9c
ℹ️ 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".
flyq
left a comment
flyq
left a comment
There was a problem hiding this comment.
Review: PR #358 — compute gas specification + cross-spec test harness
Overview
The PR adds docs/spec/evm/compute-gas.md (550 lines) as the single normative source for compute-gas accounting, converts the scattered content in resource-accounting.md and dual-gas-model.md into pointers, corrects the previously wrong receipt identity (gas_used ≠ compute + storage once refunds settle), and pins the whole thing with a new test target: a 32-program × 9-spec golden snapshot plus 8 rule-assertion tests. No src/ changes — the spec:unchanged / api:unchanged labels are accurate.
Verdict: this is a high-quality PR and I found no correctness errors in it. I independently verified the page's normative claims against the implementation (three parallel deep-verification passes covering ~30 claim clusters), ran the new tests, and checked every cross-page reference. Everything confirmed. The remaining findings below are letter-level wording issues and coverage suggestions — none replay-affecting, none blocking.
Specification correctness (verified against implementation)
All confirmed, each traced to code:
- Measurement window —
recorded = (gas_before − gas_after) − storage_gas_charged − forwarded_child_gaswith saturating subtraction matchesrecord_storage_compute_gas!(instructions.rs:464-514) and the plain wrappers; every abort path (stack underflow, storage-charge OOG, inner-instruction error) skips the trailing record exactly as the single-record rule states. - Forwarded-gas exclusion — the three-branch pseudocode matches
instructions.rs:473-493, including the Rex5CALL_STIPENDsubtraction gated on scheme+value, and the pre-inflation claim holds: Rex4 stipend inflation happens inbefore_frame_init, strictly after the parent's window closes. - MiniRex forwarding quirk — real: the mini_rex table wires only
CALLthroughforward_gas_ext(instructions.rs:691-694); Rex swaps all three. - CREATE2 window movement — MiniRex–Rex4 second window skipped on inner failure (
instructions.rs:1740-1743), Rex5 eager record (:1632-1636), Rex6 single window, and Rex6 does halt on oversized initcode before any expansion (:1613-1626). - Non-opcode sites — intrinsic records revm's pristine
initial_gasbefore the MegaETH storage additions (execution.rs:697-719); the Rex5+ precompile formula, the structural KZG branch condition (a limit comparison, not an error-variant match —precompiles.rs:234-251), and the 100,000 override atprecompiles.rs:98all match; keyless overhead is charged from Rex2 but recorded only from Rex3 (sandbox/execution.rs:174-181), rescue Rex6-gated (:200-202). - Exhaustiveness — the accounting identity's site list is complete: every mutation of the live compute counter routes through the five listed sites. Notably, the Rex6 post-execution fee-reward accounting merges data-size/KV/state-growth only, no compute gas (
execution.rs:952-955), so the "MUST NOT record at any other site" claim survives. - Limits & exceed — 1B/200M limits, strict
>at both levels, 98/100 frame share, Rex4-only pre-inflation cap, frame-check-before-tx-check with no short-circuit,MegaLimitExceeded(uint8,uint64)frame revert, rescue excluding the stipend portion, and the detention halt's distinctVolatileDataAccessOutOfGasvariant all match. - Class assignment — the MiniRex base + per-spec override tables match the instruction tables exactly, including the SELFDESTRUCT reported-dimension priority (data size → KV → compute → state growth,
limit.rs:358-380) and the Rex1 detention reset (compute_gas.rs:172-180).
The PR body's two self-reported fixes (KZG 100,000; keyless rescue matrix) are correctly incorporated, and the test that hard-codes 100,000 instead of importing it is the right pinning choice.
Findings (minor, worth a follow-up commit)
- Rex6 window-boundary claim doesn't literally match SELFDESTRUCT —
compute-gas.md:73says every storage-affecting opcode's window "MUST open before the opcode's storage-gas charge", but Rex6 SELFDESTRUCT still charges storage gas (instructions.rs:2033) before its window opens (:2297). The amounts are identical under the page's own two-form equivalence rule, so this is unobservable — but a normative MUST that mega-evm itself doesn't literally satisfy invites a false "non-compliance" report from a future auditor. Suggest carving SELFDESTRUCT out or restating the Rex6 block in terms of recorded amounts. - Keyless frame-local branch is currently unreachable —
compute-gas.md:415specifies the frame-localInsufficientComputeGasrevert, but KeylessDeploy interception is depth-0-only (intercept.rs:301) and the tracker frame stack is empty there, socheck_limitcan only produce a transaction-level exceed today. The code branch exists as written (defensive), so the spec isn't wrong, but a one-line hint that only the transaction-level branch is reachable on current dispatch would save implementers hunting for a repro. - "Current frame's remaining budget" is ambiguous — in the Rex5+ precompile bound (
compute-gas.md:281), the frame in question is the precompile invocation's own pushed frame (the forwarded 98/100 share), not the caller's frame. Worth one clarifying clause; an implementer bounding by the caller's frame would compute a different cap. - Convention deviation should be explicitly blessed —
docs/spec/AGENTS.mdsays main content describes the latest stable spec only and unstable behavior goes in<details>. The page argues its cross-spec structure well in Rationale, but the Rex6 rescue rule additionally sits in main prose (compute-gas.md:401,:418-421;resource-limits.md:101-103) whilekeyless-deploy.mdwraps the same content in a<details>block — inconsistent placement for identical content. Either wrap those or codify this page's exception inAGENTS.md. (Related nit: two{% hint info %}blocks carry normative MUST NOTs; AGENTS.md reserves hints for non-normative notes.)
Test harness
- All 12 tests pass locally;
cargo fmt,clippy(only the pre-existing MSRV-config warning), and Prettier on docs are clean. Cargo auto-discoverstests/compute_gas/main.rs; no manifest change needed. - The claim tests are well-designed: differential pairs isolate exactly one variable (byte-identical programs, same push widths), each
CALL_STIPENDcondition gets a negative case, and the KZG constant is deliberately hard-coded so an implementation change fails instead of being followed. - I spot-checked the snapshot arithmetically: intrinsic 21000/60000, the +2,300 Rex5 stipend deltas, the +100,000 Rex5 KZG delta, the refund row, pre-Rex2
InvalidFEOpcodeSELFDESTRUCT rows, and the Rex5 eager-expansion value 763,907 (which works out exactly: 21,012 + quadratic expansion of 18,750 words). Rows are internally consistent with the stated rules. - Coverage gap worth closing: the corpus contains no case that actually halts on a compute-gas or detention limit, yet halt position ("which limit is reached first, at which opcode") is the page's stated reason window placement is consensus-visible. A corpus entry that trips gas detention with default limits (volatile read, then a burn loop) would pin the recorded-at-halt amount and the rescue arithmetic in the snapshot. Similarly, per-frame-exceed (
MegaLimitExceeded) recording isn't exercised here. Fine as a follow-up. - Nit:
main.rs:103-196has three near-identicaltransact*helpers; a single core function with the envs/output variants layered on would drop ~50 lines. Optional.
Docs quality
- Structure follows the spec-page template exactly (frontmatter with
spec: Rex5, Motivation → Specification → Constants → Rationale → Security Considerations → Spec History); every constant referenced in the body appears in the Constants table; all 14 cross-page anchors I checked resolve;SUMMARY.mdplacement is sensible. - Cross-page consistency is genuinely good: the Spec History bullets match the upgrade pages (including the subtle Rex2-introduce vs Rex3-record split for the keyless overhead),
keyless-deploy.md/resource-limits.md/glossary.mdall agree with the new normative source, and no page still claims the old receipt identity. - The
dual-gas-model.mdreceipt correction is right, and the metering-order edits sit properly inside the Rex6<details>block.
Recommendation
Approve. Items 1–3 are small wording fixes I'd fold in before merge since this page will be treated as normative; item 4 and the halt-path corpus case can be follow-ups.
Documentation ImpactThis PR adds a new spec page, Agent / Skill Files
These updates can be included in this PR or in a follow-up. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2e9d42e11
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
mega-evm/docs/spec/evm/dual-gas-model.md
Lines 93 to 94 in bc8e31a
Align operand validation with the actual precharge order
For Rex6 malformed storage opcodes, this ordering does not match the handlers: storage_gas_ext::log validates only the offset/length pair, charges LOG storage gas, and then lets the raw opcode validate the remaining topic operands. A LOG1 stack containing only offset and a sufficiently large length can therefore halt with OutOfGas during precharge instead of the StackUnderflow required here; the CALL wrappers similarly inspect only target/value before charging. Either make the handlers validate the complete operand stack first or narrow this rule to the operands needed for precharging, otherwise implementations can diverge on the halt reason.
AGENTS.md reference: docs/spec/AGENTS.md:L113-L115
ℹ️ 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".
…d by a CALLCODE arc test
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4563dd25cb
ℹ️ 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".
…a direct-transaction test
2 participants
Summary
Compute gas is one of the two dimensions of the dual gas model and has its own per-transaction limit, but it had no real specification — roughly 40 lines split across
resource-accounting.mdanddual-gas-model.md, saying little beyond "same as vanilla EVM gas". Meanwhile the implementation records it at nine distinct sites, and the measurement itself changes across five specs.This PR adds
docs/spec/evm/compute-gas.mdas the authoritative page, migrates the scattered content into it, and adds a test harness that pins the specification against the implementation.The motivating use case is the upcoming per-opcode metering hot-path optimisation: that work must preserve recorded compute gas byte-for-byte on every frozen spec, and until now there was neither a written contract nor a regression net for it.
The specification
Compute gas is defined as a derived measurement of the EVM gas inherited from Optimism Isthmus / Ethereum Prague, not as a gas schedule of its own. Restating 256 opcode costs would duplicate the inherited spec, rot on every upstream bump, and still leave the consensus-critical question unanswered — how much of an operation's inherited cost is recorded, and when.
What the page defines instead:
recorded = (gas_before − gas_after) − storage_gas_charged − forwarded_child_gas, with the window boundaries stated normatively. The opening point is consensus-visible: it determines how much has been recorded when an opcode halts partway through, and therefore which limit is reached first.CALL_STIPENDadjustment and the frozen MiniRex quirk where three CALL-family opcodes are not subject to the 98/100 cap), and refunds.What moved
The compute-gas section of
resource-accounting.mdand the metering amount fromdual-gas-model.mdare replaced by pointers, so there is a single normative source.dual-gas-model.mdkeeps the metering order, which is genuinely a storage↔compute interaction.dual-gas-model.md's total-gas equation is corrected: it was stated as an identity between the receipt'sgas_usedandcompute + storage, which does not hold once an EVM refund is settled.The test harness
crates/mega-evm/tests/compute_gascontains two layers:A golden snapshot — 32 programs covering every metering class and every non-opcode recording site, executed under all nine specs, with the resulting compute-gas readings checked in. Any change to a recorded amount on any spec surfaces as a diff. Regenerate with
UPDATE_COMPUTE_GAS_SNAPSHOT=1 cargo test -p mega-evm --test compute_gas.Seven rule assertions — the snapshot records numbers but not why they are what they are; a refactor that preserves a total while breaking the underlying rule would still pass it. These tests assert the rules directly: the
CALL_STIPENDexclusion (with a negative case for each of its three conditions), the CREATE2 memory-expansion window split, the KZG fixed cost, interception costing zero, refund exclusion, the frozen MiniRex forwarding quirk, and Equivalence recording nothing.Reviewers should read the snapshot diff as part of the review. Rows for Equivalence through Rex5 must never change — a diff there is a replay-breaking regression, not a value to bless.
Verification
The specification was checked against the implementation by two independent automated reviews, which found and this PR fixes:
precompiles.rs:98), asprecompiles.md:39already documented. An implementer following the new page alone would have disagreed with the chain on every block touching0x0a.Both errors survived the initial test suite because the corpus contained no KZG case and no keyless-deploy case. The corpus now covers KZG, and the rule is pinned by a test that hard-codes 100,000 rather than importing it, so a change to the implementation constant fails rather than being silently followed.
Local gate:
cargo sort,cargo fmt --all --check,cargo clippy --workspace --lib --examples --tests --benches --all-features --locked,cargo test --workspace, andmise run lint(lychee + markdownlint + prettier) all pass.