-
Notifications
You must be signed in to change notification settings - Fork 130
docs(l2): improve L2 docs quality #5523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR improves the quality and usability of L2 deployment documentation based on external user feedback. The changes make the documentation more accessible by switching from cargo build to cargo install, clarify the relationship between build-time and deploy-time proving system configuration, add crucial gas limit tuning information, and reorganize the Aligned integration documentation to be more discoverable within the deployment section.
Key changes:
- Replaced
cargo buildwithcargo installacross all deployment documentation to simplify binary installation and ensure it's in$PATH - Added documentation for gas limit configuration parameters (
--block-producer.block-gas-limitand--committer.batch-gas-limit) - Clarified that enabling multiple proving systems (e.g., both SP1 and RISC0) at deploy time requires all selected proofs for settlement
- Moved Aligned integration documentation from
docs/l2/aligned_integration.mdtodocs/l2/deployment/aligned.mdfor better organization
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/l2/deployment/vanilla.md | Added clarification about multi-proof requirements and gas limit configuration documentation |
| docs/l2/deployment/validium.md | Added clarification about multi-proof requirements and gas limit configuration documentation |
| docs/l2/deployment/prover/overview.md | Changed from cargo build to cargo install, added installation path info, and clarified proving system requirements |
| docs/l2/deployment/overview.md | Changed from cargo build to cargo install, added installation path info, and clarified proving system requirements |
| docs/l2/deployment/aligned.md | New file with Aligned integration guide (moved from aligned_integration.md) with updated commands using cargo install |
| docs/l2/deployment/README.md | Added link to new Aligned documentation location in deployment section |
| docs/l2/aligned_integration.md | Removed file (content moved to deployment/aligned.md) |
| docs/SUMMARY.md | Updated navigation to reflect new Aligned documentation location under deployment section |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| > [!NOTE] | ||
| > This command requires the `COMPILE_CONTRACTS` env variable to be set, as the deployer needs the SDK to embed the proxy bytecode. | ||
| > In this step we are initializing the `OnChainProposer` contract with the `ALIGNED_PROOF_AGGREGATOR_SERVICE_ADDRESS` and skipping the rest of verifiers; you can find the address for the aligned aggregator service [here](https://docs.alignedlayer.com/guides/7_contract_addresses). |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo: "initiallizing" should be "initializing".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It says initializing...
| ```shell | ||
| # For SP1 CPU proving (very slow, not recommended) | ||
| cargo build --release --bin ethrex --features l2,l2-sql,sp1 | ||
| cargo install --path cmd/ethrex --bin ethrex --features l2,l2-sql,sp1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--locked is missing for all the install commands.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, added --locked to every cargo install I saw 9929ce3
ilitteri
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- All the
cargo installcommands are missing the--lockedflag. - Some Copilot suggestions are worth applying.
Motivation
Description