Skip to content

docs: address documentation audit findings from issue #281#289

Merged
RealiCZ merged 4 commits into
mainfrom
agent/issue-281
May 7, 2026
Merged

docs: address documentation audit findings from issue #281#289
RealiCZ merged 4 commits into
mainfrom
agent/issue-281

Conversation

@TroublorBot
Copy link
Copy Markdown
Collaborator

@TroublorBot TroublorBot commented May 6, 2026

Generated by engineer-agent — review carefully before merging.

Summary

Addresses the documentation audit findings from issue #281 across specs and related docs.

Fixes #281

@TroublorBot
docs: address audit findings from issue #281
9b630a4 
Apply documentation audit fixes covering correctness, freshness, and readability:

- Oracle v2.0.0 code hash corrected to match source artifacts (C-001).
- Rex5 unstable content moved into <details> blocks across keyless-deploy,
  oracle, system-tx, evm/overview, evm/resource-accounting (R-001..R-004,
  C-005); affected pages reverted to spec: Rex4 in frontmatter.
- Rex5 behavioral summary added to hardfork-spec.md, upgrades/overview.md,
  and spec/overview.md spec progression (C-002, C-003, R-007, R-008).
- Glossary updated: spec progression includes REX5, Detained limit reordered
  with Rex4+ relative-cap definition leading, MEGA_SYSTEM_ADDRESS Rex5 bullet
  marked _(Rex5, unstable)_ (R-009, R-010, R-015).
- SequencerRegistry doc gains v1.0.0 code hash and ActivationBlockTooLarge()
  error in interface and Change Scheduling rule (F-002).
- KeylessDeploy doc gains Rex5 trailing-bytes rejection rule in <details>
  block plus Spec History entry (F-001, R-001).
- rex5.md adds the trailing-bytes change to What Changed; References now
  point to spec pages instead of crate file paths (F-001, R-014).
- hardfork-spec.md drops MegaHardfork/MegaSpecId reference-impl mentions and
  rewrites Backward Compatibility bullets in plain English (R-005, R-006).
- block-environment.md uses relative link to gas-detention spec (R-012).
- mega-access-control.md and mega-limit-control.md drop the Source: line
  pointing at Solidity files (R-013).
- AGENTS.md test-organization list now includes rex5/ (C-004).

Generated-by: engineer-agent
@TroublorBot TroublorBot added the agent Generated by AI agents label May 6, 2026
@TroublorBot TroublorBot self-assigned this May 6, 2026
@TroublorBot TroublorBot mentioned this pull request May 6, 2026
Closed
@Troublor Troublor added api:unchanged No change to the public interface or API comp:doc Changes in the documentation spec:unstable Changes to the unstable spec (currently REX5) labels May 6, 2026
@TroublorBot
Copy link
Copy Markdown
Collaborator Author

TroublorBot commented May 6, 2026 •
edited
Loading

Labels updated. The PR now has spec:unchanged, comp:doc, api:unchanged, and agent — matching the reviewer's suggestion. No code changes or commits needed.

@Troublor Troublor added spec:unchanged No change to any `mega-evm`'s behavior and removed spec:unstable Changes to the unstable spec (currently REX5) labels May 6, 2026
@Troublor
Copy link
Copy Markdown
Collaborator

Troublor commented May 6, 2026

@claude review this PR

@TroublorBot
Copy link
Copy Markdown
Collaborator Author

TroublorBot commented May 6, 2026 •
edited
Loading

Overall the PR looks correct and well-targeted. All audit findings from issue #281 are addressed: code hashes corrected, Rex5 unstable content properly wrapped in <details> blocks, spec progressions updated, glossary entries reordered, and internal references cleaned up.

One issue I found during review and fixed in a follow-up commit:

sequencer-registry.md — cancel exception missing from ActivationBlockTooLarge prose

The PR added normative text stating that activationBlock exceeding type(uint96).max MUST revert with ActivationBlockTooLarge(). However, the Solidity source handles the cancel case (activationBlock == type(uint256).max) before reaching that check and exits early — so type(uint256).max successfully cancels rather than reverting. Without documenting this order, a reader would conclude that the documented cancel pattern (type(uint256).max) would fail. Fixed in commit b375767 by describing the cancellation path first.

Everything else looks correct. Prettier passes.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 6, 2026 •
edited
Loading

Claude finished @Troublor's task in 4m 1s —— View job

PR Review

Tasks:

  • Gather context and understand the request
  • Review git diff for all changed files
  • Verify documentation claims against source code
  • Post comprehensive review feedback

Summary

This PR addresses documentation audit findings from issue #281. All changes are documentation-only (15 files, no source code changes). The overall quality is high and the factual claims checked out against the source. One correctness fix is notable: the Oracle v2.0.0 code hash was wrong and is now corrected.

Verified Correct ✅

File Change Verified
AGENTS.md Added rex5/ to test directory list ls crates/mega-evm/tests/ confirms it exists
block-environment.md Absolute GitBook URL → relative .md path docs/spec/evm/gas-detention.md exists
oracle.md Code hash 0xcdc4cffc… → 0x71a65239… Matches crates/system-contracts/src/generated/oracle_artifacts.rs V2_0_0_CODE_HASH
sequencer-registry.md Code hash 0x2dd91bc3… added Matches sequencer_registry_artifacts.rs V1_0_0_CODE_HASH
sequencer-registry.md Errors added to interface (FutureBlock, BeforeInitialBlock, NotAdmin, ZeroAddress, InvalidActivationBlock, ActivationBlockTooLarge) All confirmed in ISequencerRegistry.sol
keyless-deploy.md MalformedEncoding() trailing-bytes rejection Confirmed in crates/mega-evm/src/sandbox/error.rs and sandbox/tx.rs
evm/overview.md SequencerRegistry address 0x6342…0006 Confirmed in system/sequencer_registry.rs:41
glossary.md Detention cap: absolute (pre-Rex4) → relative (Rex4+) semantics clarified Confirmed in constants.rs:74-84, limit/compute_gas.rs:71-73
hardfork-spec.md Rex5 content list replaces "No behavioral changes defined yet" All 5 items confirmed in source/docs
resource-accounting.md, oracle.md, system-tx.md spec: Rex5 → spec: Rex4 (Rex5 content moved to <details> blocks) Appropriate: page stable content is Rex4, Rex5 additions are unstable

Issues Found

1. Confusing wording in system-tx.md (minor)

In the Rex5 <details> block (system-tx.md):

<details>
<summary>Rex5 (unstable): Dynamic system address resolution</summary>

`MEGA_SYSTEM_ADDRESS` MUST be resolved per block from ...
The fixed-constant resolution above no longer applies.  ← "above" is outside the collapsed block

A reader who opens only the details block loses the "above" reference. Consider rephrasing to:

The fixed constant 0xA887dCB9D5f39Ef79272801d05Abdf707CFBbD1d is replaced by dynamic resolution from SequencerRegistry.

Fix this →

2. sequencer-registry.md — Version header inconsistency (pre-existing, minor)

The Bytecode section uses plain text Version 1.0.0 (line 33) rather than the #### heading level used in oracle.md (#### Version 1.0.0, #### Version 1.1.0, etc.). This wasn't introduced by this PR, but it could be fixed as part of this cleanup.

3. hardfork-spec.md — Normative MUST removed (worth confirming intentional)

The diff weakens:

-Implementations MUST gate spec-specific behavior on the active spec.
+Implementations gate spec-specific behavior on the active spec.

If this is an intentional shift from RFC 2119 normative language to informative prose (consistent with the rest of the doc's style), that's fine. But if the intent is to keep a behavioral requirement for node implementors, the MUST should stay. Please confirm.

4. mega-access-control.md / mega-limit-control.md — Source links removed without replacement (minor)

-Source: [`MegaAccessControl.sol`](https://github.com/megaeth-labs/mega-evm/blob/main/crates/system-contracts/contracts/MegaAccessControl.sol)

These links pointed directly to the Solidity source. Removing them is fine if the intent is to keep the spec self-contained, but there is no replacement navigation hint for readers who want to inspect the source. Not blocking, but worth noting.

Overall Assessment

Approve with minor comments. The code hash correction in oracle.md is a material accuracy fix. The pattern of moving unstable Rex5 content into <details> blocks while updating the spec: frontmatter to the last stable spec is applied consistently and correctly. The documentation now correctly reflects what Rex5 introduces.

The four items above are either pre-existing, minor wording suggestions, or a one-line clarification request — none are blocking.

@TroublorBot
docs: clarify cancel exception in sequencer-registry Change Scheduling
b375767 
The cancellation path (activationBlock == type(uint256).max) bypasses the
ActivationBlockTooLarge check in the Solidity source. The previous prose
stated the uint96 size constraint without noting the cancel exception, making
it appear that passing type(uint256).max would revert. Document cancellation
first to make the ordering clear.

Generated-by: engineer-agent
@TroublorBot
Copy link
Copy Markdown
Collaborator Author

TroublorBot commented May 6, 2026 •
edited
Loading

The comment from @claude[bot] is just a "review in progress" status update — the actual review results aren't in yet. There's no actionable feedback to respond to here.

No changes needed at this time.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 6, 2026

LGTM. Code hashes for Oracle v2.0.0 and SequencerRegistry v1.0.0 verified against the generated artifacts in crates/system-contracts/src/generated/. The only substantive issue (cancel-exception ordering in sequencer-registry.md) was already caught and fixed in commit b375767.

@TroublorBot
Copy link
Copy Markdown
Collaborator Author

TroublorBot commented May 6, 2026 •
edited
Loading

The @claude[bot] review is an approval — "LGTM" with no requested changes. No action needed.

@TroublorBot
Copy link
Copy Markdown
Collaborator Author

TroublorBot commented May 6, 2026

🔍 Looking into this...

@TroublorBot
address review feedback: fix details block wording and version heading
1942e40 
- system-tx.md: make Rex5 details block self-contained — replace "above"
  reference with the explicit constant value so the block reads correctly
  when expanded in isolation.
- sequencer-registry.md: promote "Version 1.0.0" to a #### heading and add
  the "A node MUST deploy..." preamble, matching the heading style used in
  oracle.md and other system contract pages.

Generated-by: engineer-agent
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 6, 2026

LGTM. All prior review feedback has been addressed: the system-tx.md details block is now self-contained (explicit constant replaces the dangling "above" reference), the cancel-exception ordering in sequencer-registry.md was fixed in b375767, and rex5.md gained a proper KeylessDeploy trailing-bytes section with correct numbering. No new issues found.

@TroublorBot
Copy link
Copy Markdown
Collaborator Author

TroublorBot commented May 6, 2026 •
edited
Loading

The @claude[bot] comment is another approval — "LGTM" with no new issues found. No action needed.

@RealiCZ RealiCZ merged commit b6bf99b into main May 7, 2026
25 checks passed
@RealiCZ RealiCZ deleted the agent/issue-281 branch May 7, 2026 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Troublor Troublor Troublor approved these changes

@RealiCZ RealiCZ RealiCZ approved these changes

@flyq flyq Awaiting requested review from flyq flyq is a code owner

@vincent-k2026 vincent-k2026 Awaiting requested review from vincent-k2026 vincent-k2026 is a code owner

Assignees

@TroublorBot TroublorBot

Labels

agent Generated by AI agents api:unchanged No change to the public interface or API comp:doc Changes in the documentation spec:unchanged No change to any `mega-evm`'s behavior

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

docs: address documentation audit findings from 2026-05-04

3 participants

@TroublorBot @Troublor @RealiCZ