Conversation
|
Good structure overall — the separation into normative specs, behavior details, implementation references, and public book pages is clean and maintainable. Three issues to fix before merging: 1. Spec progression diagram mixes hardfork and spec names (blocking)
Showing them in a diagram labeled "Spec Progression" immediately followed by "each newer spec includes all previous behaviors" is wrong — 2.
3. Hardfork→spec example omits MiniRex1
|
ReviewOverall this is a solid refactor: the new style guide is well-defined, the spec restructuring into Abstract / Changes / Invariants / Inheritance / References is a clear improvement over the old numbered-section format, and the companion Four issues worth fixing before merge: Factual error
Missing unstable caveat
Semantic ambiguity in specs
|
|
Overall this PR is well-structured and a significant improvement to the documentation. The three-tier hierarchy (normative spec → behavior details → implementation references) is clear, the STYLE-GUIDE.md is useful, and the book pages are accurate and developer-friendly. A few issues to address: |
|
This is a well-structured docs PR. The spec refactoring (normative/behavior-details/implementation-references split) is clean and the style guide are a valuable addition. A few issues worth addressing. 1. Per-frame limits section scoped only to State Growth ( The "Per-Frame Limits (Rex4+)" subsection appears only under State Growth. But Rex4 adds per-frame budgets for all four dimensions (compute gas, data size, KV updates, state growth). A developer reading only the Data Size or KV Updates sections will miss this entirely. The subsection should be elevated to a top-level section or at minimum cross-referenced from each dimension. 2. "How It Works" steps 4-5 are alternatives, not sequential steps ( The numbered list presents a process flow, but steps 4 and 5 are mutually exclusive spec variants (pre-Rex4 absolute cap vs Rex4 relative cap). Numbering them as consecutive steps implies both execute for the same transaction, which is wrong. 3. Error name mismatch ( Step 4 says "execution halts with 4. The successor (Rex2) is listed before the predecessor (Rex), which is inconsistent with other spec files. Minor: |
|
Review: Good structure overall. The spec style guide is a real improvement. The AGENTS.md correction (removing MINI_REX1/MINI_REX2 from the spec progression) is correct. One factual error repeated in four places: MAX_INITCODE_SIZE should be 548,864 bytes (512 KB + 24 KB), not 536,576. Inline suggestions posted on each location. Also the 21x larger note in book/src/evm/README.md only applies to contract size, not initcode. |
ReviewOverall the structure is clean and the spec rewrites improve clarity significantly. Three issues to address: 1. Removed test links in 2. Forward references in 3. |
|
@claude All three points were discussed in prior review rounds:
No changes needed for this round. |
|
Claude finished @RealiCZ's task in 26s —— View job Acknowledged — all three points are intentional design decisions:
No changes needed. The remaining open issues from earlier rounds (factual fixes like the |
|
@claude Please post a final review summary confirming the current state of the PR. |
|
Claude finished @RealiCZ's task in 3m 49s —— View job Final Review SummaryThis is a well-structured docs PR. The three-tier hierarchy (normative spec / behavior details / implementation references), the STYLE-GUIDE, and the GitBook setup are all solid additions. Issues Resolved ✅All major factual and structural issues raised across review rounds are fixed:
Intentionally Dismissed (acknowledged) ✅
One Remaining Issue
|
Troublor
left a comment
There was a problem hiding this comment.
Review Summary
High-quality documentation restructuring. The three-tier hierarchy (normative spec → behavior details → implementation references) with the separate public-facing book is well-designed.
Key issues raised:
- Current EVM Standard describes Rex4 (unstable) — should reflect Rex3 (latest stable) or clearly warn about unstable content.
- SALT buckets never explained — the multiplier formula is used everywhere but the concept is a black box for developers.
- Gas estimation guidance missing — developers need to know to call
eth_estimateGason a MegaETH endpoint. - SSTORE condition underspecified in normative spec — "zero-to-non-zero" should state the exact triple condition (
original==0 AND present==0 AND new!=0). - LOG revert behavior needs clarification — gas cost is permanent regardless of revert, but data size limit accounting rolls back.
- 98/100 rule migration impact not surfaced for developers porting existing Ethereum contracts.
- "Remaining gas is preserved and refunded" is imprecise — should specify halt (not revert), gas_used < gas_limit in receipt.
- No glossary — MegaETH-specific terms used throughout without formal definitions.
| Storage gas costs scale dynamically based on **SALT bucket capacity**. | ||
| Each account and storage slot maps to a SALT bucket in MegaETH's blockchain state. | ||
|
|
||
| **Formula**: `multiplier = bucket_capacity / MIN_BUCKET_SIZE` | ||
|
|
||
| - When `multiplier = 1` (minimum bucket size): **zero storage gas** — no penalty for fresh storage | ||
| - When `multiplier > 1`: linear scaling based on bucket capacity expansion | ||
|
|
||
| This mechanism prevents state bloat by making storage more expensive in crowded state regions. |
There was a problem hiding this comment.
SALT buckets are used throughout the docs but never explained to developers.
The formula multiplier = bucket_capacity / MIN_BUCKET_SIZE is clear mathematically, but a dapp developer reading this has no idea:
- What IS a SALT bucket? (The concept is never introduced.)
- What determines
bucket_capacity? (State density? Account age? Something configurable?) - Can a developer query their contract's bucket capacity? Is there an RPC or system contract for this?
- What is the practical range of
multipliervalues they should expect? (1–10? 1–1000?)
Without this context, a developer cannot estimate storage gas costs for their contracts. They see 20,000 × (multiplier − 1) and have no idea what multiplier will be.
Suggestion: Add a brief "SALT Buckets" subsection here explaining:
- What determines bucket capacity (conceptually — not implementation details)
- Whether developers can query or predict it
- The practical multiplier range
- Or simply: "Use MegaETH's gas estimation API; do not attempt to compute storage gas manually."
book/src/evm/README.md
Outdated
| | **LOG topic** | 3,750/topic | Per topic, regardless of revert | | ||
| | **LOG data** | 80/byte | Per byte, regardless of revert | |
There was a problem hiding this comment.
"Regardless of revert" is true for the gas cost but not for the data size limit accounting.
Verified against code:
- Storage gas: permanently consumed even if the frame reverts (charged via
gas!()before LOG executes). ✓ "regardless of revert" is correct here. - Data size limit: LOG data is tracked as
discardableinDataSizeTrackerand rolled back when the frame reverts viapop_frame(success=false).
For a dapp developer, the practical question is: "If my subcall reverts, do I still pay for its LOGs?"
- Gas cost: Yes, you still pay.
- Data size budget: No, it does not count against your data size limit.
Suggestion: Clarify the dual behavior, e.g.:
| LOG topic | 3,750/topic | Gas cost is permanent regardless of revert; data size usage is rolled back on revert |
| LOG data | 80/byte | Gas cost is permanent regardless of revert; data size usage is rolled back on revert |
| ## Gas Forwarding (98/100 Rule) | ||
|
|
||
| All CALL-like opcodes (CALL, STATICCALL, DELEGATECALL, CALLCODE) and CREATE/CREATE2 forward at most **98/100** of remaining gas to subcalls, replacing the standard EVM's 63/64 rule. | ||
| This prevents call-depth attacks under MegaETH's high gas limits. |
There was a problem hiding this comment.
This section describes what the 98/100 rule IS, but not what it BREAKS for developers porting existing Ethereum contracts.
Contracts that depend on gas forwarding precision may behave differently on MegaETH:
- Gas-limited subcalls — patterns like
addr.call{gas: X}(...)where X is calculated assuming 63/64 retained by the parent. With 98/100, the parent retains 2/100 instead of 1/64, changing the effective gas available to the caller after the subcall. - Proxy patterns checking
gasleft()— contracts that estimate remaining gas after a subcall (e.g., to decide whether to attempt a fallback) will see different numbers. - Gas stipend assumptions — the 2300 gas stipend for ETH transfers is unchanged, but any contract that computes "how much gas will my subcall actually receive" using the 63/64 formula will be wrong.
The MiniRex Behavior Details companion doc touches this under "Migration Impact", but a dapp developer reading the book would never find it there.
Suggestion: Add a brief developer note here, e.g.:
Migration note: Contracts that compute gas forwarding amounts assuming the standard 63/64 rule (EIP-150) will see different behavior. The parent frame retains 2% instead of ~1.6%, so subcalls receive slightly less gas. Review any patterns that rely on precise gas forwarding calculations.
| Storage gas costs for SSTORE, account creation, and contract creation scale dynamically based on SALT bucket capacity. | ||
|
|
||
| **Formula**: `multiplier = bucket_capacity / MIN_BUCKET_SIZE` | ||
|
|
||
| - At `multiplier = 1` (minimum bucket size): **zero storage gas** — fresh storage is free. | ||
| - At `multiplier > 1`: linear scaling makes crowded state regions more expensive. | ||
|
|
||
| See [Dual Gas Model](dual-gas-model.md) for details. |
There was a problem hiding this comment.
After reading the SALT multiplier section, a developer's immediate question is: "How do I estimate gas for my transactions?"
With dynamic SALT multipliers, storage gas, gas detention, and multidimensional resource limits all affecting the final cost, a developer cannot manually compute gas costs from the formulas alone — the multiplier depends on on-chain state they may not have access to.
Suggestion: Add a brief practical note here, e.g.:
Gas estimation: Call
eth_estimateGason a MegaETH RPC endpoint to get precise gas estimates. The endpoint accounts for SALT multipliers, storage gas, and all resource dimensions. Do not attempt to compute gas costs manually from the formulas above — the dynamic multiplier depends on on-chain SALT bucket state.
specs/MiniRex.md
Outdated
| - In comparison, Standard EVM (EIP-3860): 49,152 bytes (48 KB) | ||
| | Operation | Storage Gas | Condition | | ||
| | --- | --- | --- | | ||
| | **SSTORE (0→non-0)** | 2,000,000 × multiplier | Only zero-to-non-zero transitions | |
There was a problem hiding this comment.
"Only zero-to-non-zero transitions" is ambiguous for a reimplementer.
Verified against code (instructions.rs), the exact condition is:
original_value == 0 AND present_value == 0 AND new_value != 0
This is the "clean zero → nonzero" case only. Critically, if original == 0 but present != 0 (the slot was already written to non-zero earlier in the same tx), no storage gas is charged on re-write — even though the transition from the caller's perspective looks like a zero-to-non-zero write.
A reimplementer reading "zero-to-non-zero" could reasonably implement present == 0 && new != 0 (missing the original == 0 guard), which would overcharge in the original != 0, present == 0, new != 0 case (slot cleared then re-set within the same tx).
Suggestion: State the exact triple condition:
Only when
original_value == 0 AND present_value == 0 AND new_value != 0(using EIP-2200 terminology: original is the value at transaction start, present is the current value in the journal).
book/src/SUMMARY.md
Outdated
| - [Rex1](upgrades/rex1.md) | ||
| - [Rex2](upgrades/rex2.md) | ||
| - [Rex3](upgrades/rex3.md) | ||
| - [Rex4](upgrades/rex4.md) |
There was a problem hiding this comment.
Consider adding a Glossary page to the book.
Terms like "compute gas", "storage gas", "SALT bucket", "volatile data", "gas detention", "multiplier", "frame-local", "resource dimension", "detained limit" are used throughout the docs but never formally defined in one place.
For a dapp developer new to MegaETH, these terms appear without introduction — especially "SALT bucket" and "gas detention" which have no equivalent in vanilla Ethereum. A glossary gives them a single place to look up unfamiliar terms without hunting through multiple pages.
For a chain developer reimplementing mega-evm, a glossary establishes canonical terminology — avoiding ambiguity when the same concept is described slightly differently across pages (e.g., "remaining gas is preserved" vs "excess gas is refunded" vs "detained gas is returned").
Suggestion:
## Reference
- [Glossary](glossary.md)
book/src/evm/README.md
Outdated
| # Current EVM Standard | ||
|
|
||
| This page describes the current MegaETH EVM behavior as a single reference. | ||
| It reflects the latest spec (Rex4) and serves as the starting point for developers building on MegaETH. |
There was a problem hiding this comment.
This page describes Rex4 behavior, but Rex4 is the unstable spec (as noted in AGENTS.md and the Rex4 upgrade page itself). Unstable specs are under active development and their semantics may change.
A public-facing developer doc should describe the latest stable spec (Rex3) as the canonical reference. Documenting unstable behavior as "the starting point for developers building on MegaETH" risks developers building against semantics that may change before the spec is finalized.
Rex4-specific features described here that are NOT in Rex3:
- Per-frame resource budgets (§ Per-Frame Resource Budgets)
- Relative gas detention caps (§ Relative Cap)
- MegaAccessControl system contract
- MegaLimitControl system contract
Suggestion: Either
- Rewrite this page to reflect Rex3 (latest stable) and note Rex4 features as "upcoming", or
- Keep Rex4 content but add a clear warning at the top, e.g.: "This page includes features from Rex4, which is currently unstable and subject to change. The latest stable spec is Rex3."
Final ReviewAll issues from prior rounds have been resolved. One item remains from the previous claude summary:
The page opens with 'It reflects the latest spec (Rex4)' with no caveat that Rex4 is the current unstable spec. Everything else is in good shape — the three-tier hierarchy, STYLE-GUIDE, spec rewrites, glossary, gas estimation guidance, SSTORE condition precision, and LOG revert behavior are all correct and well-documented. |
| This page describes the current MegaETH EVM behavior as a single reference. | ||
| It reflects the latest spec (Rex4) and serves as the starting point for developers building on MegaETH. | ||
| For incremental changes introduced by each upgrade, see the [Network Upgrades](../upgrades/README.md) section. | ||
| For deep dives on individual topics, see the linked concept pages below. |
There was a problem hiding this comment.
Rex4 is the current unstable spec (per CLAUDE.md), but this page carries no warning. A developer landing here as their primary EVM reference won't know the semantics may change before activation.
| For deep dives on individual topics, see the linked concept pages below. | |
| For deep dives on individual topics, see the linked concept pages below. | |
| > **Note**: This page reflects Rex4, which is the current unstable specification and is subject to change before activation. |
| - Transaction halts with `OutOfGas` error | ||
| - Remaining gas is **preserved** (not consumed), refunded to sender | ||
| - Transaction **fails** (status=0) but is **still included** in the block | ||
| - Failed transactions still count toward block limits |
There was a problem hiding this comment.
Rex4 introduced a third enforcement tier that's missing here: frame-local violations. When a frame-local budget is exceeded in Rex4+, the frame reverts with MegaLimitExceeded(uint8 kind, uint64 limit) rather than halting the transaction with OutOfGas. The parent frame can catch and continue.
Without this subsection, readers consulting this page in Rex4+ context will have an incomplete and misleading picture of enforcement (the current text implies all violations halt the transaction). resource-accounting.md covers this fully, but readers of resource-limits.md won't know to look there.
Suggest adding a "Frame-Level Violations (Rex4+)" subsection here, or at minimum a forward reference:
| - Failed transactions still count toward block limits | |
| - Failed transactions still count toward block limits | |
| ### Frame-Level Violations (Rex4+) | |
| When a call frame exceeds its frame-local budget: | |
| - That frame **reverts** with `MegaLimitExceeded(uint8 kind, uint64 limit)` — the transaction does **not** halt | |
| - The parent frame can continue executing after the child reverts | |
| - Compute gas consumed by the reverted frame still counts toward the transaction total | |
| Transaction-level limits still apply independently; exceeding them halts the transaction with `OutOfGas`. | |
| See [Resource Accounting](resource-accounting.md) for per-frame budget forwarding details. | |
book/src/evm/README.md
Outdated
| | Max contract | 524,288 bytes (512 KB) | | ||
| | Max initcode | 548,864 bytes (512 KB + 24 KB) | | ||
|
|
||
| The contract size limit is 21× larger than standard Ethereum (24 KB); the initcode limit is ~11× larger (48 KB). |
There was a problem hiding this comment.
The "(48 KB)" parenthetical is ambiguous — it reads as though it's MegaETH's initcode limit, but the table above already shows 548,864 bytes. The intended meaning is that the comparison baseline (standard Ethereum's initcode limit) is 48 KB. Suggest making that explicit:
| The contract size limit is 21× larger than standard Ethereum (24 KB); the initcode limit is ~11× larger (48 KB). | |
| The contract size limit is 21× larger than the standard Ethereum limit (24 KB); the initcode limit is ~11× larger than the standard Ethereum limit (48 KB). |
|
Good docs PR overall. Numbers and constants are accurate against the implementation (verified resource limits, gas detention caps, storage gas formulas, and the AGENTS.md spec progression fix). Two issues worth addressing before merge:
|
| @@ -0,0 +1,195 @@ | |||
| # Overview | |||
|
|
|||
| This page describes the current MegaETH EVM behavior as a single reference. | |||
There was a problem hiding this comment.
This line presents Rex4 as the current reference spec without any stability caveat. CLAUDE.md explicitly marks Rex4 as the current unstable spec under active development.
book/src/upgrades/rex4.md already has the right note — it should also appear here:
| This page describes the current MegaETH EVM behavior as a single reference. | |
| This page describes the current MegaETH EVM behavior as a single reference. | |
| It reflects the latest spec (Rex4), which is currently **unstable** and subject to change before activation. | |
| For incremental changes introduced by each upgrade, see the [Network Upgrades](../upgrades/README.md) section. |
ReviewGood shape overall — the three-tier doc structure (normative spec / behavior details / impl references), STYLE-GUIDE, and GitBook scaffolding are all clean. Most prior feedback has been addressed. One remaining issue:
Everything else looks good to merge once that's addressed. |
Final ReviewAll issues from prior rounds have been resolved, including the previously-flagged remaining item:
All key numbers verified accurate against the implementation (constants.rs): resource limits, storage gas formulas, gas detention caps, frame forwarding ratio (98/100), and the 548,864-byte initcode size fix. The AGENTS.md spec progression correction (removing MINI_REX1/MINI_REX2 from the spec ID list, adding hardfork clarification) is correct. Ready to merge. |
ReviewDocumentation-only PR. Verified numeric constants against implementation — all values match. Internal consistency between spec files, book pages, and impl is good. Spec reformatting follows the new style guide. AGENTS.md fix (removing One gap to address: In Everything else looks good to merge after that fix. |
|
|
||
| ### MegaLimitControl | ||
|
|
||
| You can query your effective remaining compute gas by calling `remainingComputeGas()`. |
There was a problem hiding this comment.
The enableVolatileDataAccess() call and its DisabledByParent() revert are omitted here. Per the Rex4 spec, the interface MUST provide disable, enable, and status-query operations, and calling enable from a descendant when an ancestor owns the restriction MUST revert with DisabledByParent(). That revert is developer-observable and should be documented alongside the disable behaviour.
| You can query your effective remaining compute gas by calling `remainingComputeGas()`. | |
| You can disable volatile data access for your frame and all descendant calls by calling `disableVolatileDataAccess()`. | |
| While disabled, any volatile access reverts immediately with `VolatileDataAccessDisabled(uint8 accessType)` — no gas detention is triggered. | |
| A descendant frame can call `enableVolatileDataAccess()`, but if the restriction was set by an ancestor the call reverts with `DisabledByParent()`. | |
| The restriction ends automatically when the disabling frame returns. |
ReviewDocumentation-only PR. Numbers are accurate against the implementation (verified resource limits, storage gas formulas, gas detention caps, frame forwarding ratio, and the 548,864-byte initcode size fix). Spec doc reformatting follows the new STYLE-GUIDE. One remaining gap before merge:
|
|
|
||
| MegaETH pre-deploys system contracts at well-known addresses: | ||
|
|
||
| | Contract | Address | Purpose | |
There was a problem hiding this comment.
The section only documents disableVolatileDataAccess() — the enable direction is missing. Without it the API reads as one-way, and developers need to know about DisabledByParent() when building contracts that call untrusted subtrees.
| | Contract | Address | Purpose | | |
| You can disable volatile data access for your frame and all descendant calls by calling `disableVolatileDataAccess()`. | |
| While disabled, any volatile access reverts immediately with `VolatileDataAccessDisabled(uint8 accessType)` — no gas detention is triggered. | |
| Call `enableVolatileDataAccess()` to lift the restriction for your frame and its descendants. | |
| A descendant frame cannot re-enable access disabled by an ancestor — attempting to do so reverts with `DisabledByParent()`. | |
| The restriction ends when the disabling frame returns. |
Summary
book/, including EVM, system contracts, and network upgrade pages.specs/STYLE-GUIDE.mdto define normative/informative doc conventions and writing rules.specs/impl/(Behavior Details + Implementation References).Test plan
book/,specs/,.gitbook.yaml).book/src/SUMMARY.md, spec references).