-
Notifications
You must be signed in to change notification settings - Fork 378
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
add op stack walkthrough to docs #1789
Conversation
WalkthroughThe pull request updates the "how-to-guides/optimism.md" document to enhance the guidance on deploying an OP Stack rollup with Celestia. It introduces a detailed explanation of the roll-op tool and the integration of op-plasma-celestia with Celestia's Mocha testnet. The guide is divided into two parts: setting up a mock L1 environment and deploying a testnet on the Ethereum Sepolia testnet. Additional instructions for configuration, light node setup, and links to further resources have been included. Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
how-to-guides/optimism.md (5)
14-21
: Consider adding reference to video walkthroughThe PR objectives mention including a video walkthrough (https://www.youtube.com/watch?v=lOLw4uLX644). Consider adding this reference in the introduction section to provide users with a visual learning option.
This guide is in two parts: - First, you'll spin up a mock L1 environment and deploy a devnet that posts data to the Mocha testnet. - In the second part, you'll deploy a testnet that posts data to the Mocha testnet, but this time on a real L1 environment; the Ethereum Sepolia testnet. This will involve setting up a configuration file with the necessary details like Sepolia chain ID, RPC URL, and your deployment keys. +For a visual walkthrough of this process, you can watch our [video guide](https://www.youtube.com/watch?v=lOLw4uLX644).
Line range hint
31-33
: Consider adding system requirementsThe dependency section would benefit from listing system requirements and prerequisites (e.g., minimum hardware specs, OS compatibility, required software versions) before diving into the node setup.
Line range hint
102-119
: Enhance security guidance for key managementThe configuration example includes sensitive information (private keys). Consider:
- Adding strong security warnings
- Recommending the use of environment variables
- Providing guidance on secure key management practices
## Keys +# SECURITY WARNING: Never commit private keys to version control +# Consider using environment variables or a secure secrets manager +# Example using environment variables: +# contract_deployer_key = "${DEPLOYER_KEY}" contract_deployer_account = "0xaddress" contract_deployer_key = "privatekey"
Line range hint
121-121
: Clarify SepoliaETH funding requirementsThe recommendation of "10 SepoliaETH" should be more specific about:
- Exact minimum requirements for different operations
- Estimated costs for typical deployment scenarios
- Ongoing operational costs
External README links need versioning and local content backup
The verification confirms that:
- The external README links are currently valid and accessible
- The guide contains two direct references to the external README for deployment instructions
- These references are the only dependency on the external repository
Recommendations:
- Update the links to include specific commit hashes instead of using
main
branchto[the Alt-DA x Celestia README](https://github.com/celestiaorg/op-plasma-celestia/blob/main/README.md)[the Alt-DA x Celestia README](https://github.com/celestiaorg/op-plasma-celestia/blob/<commit-hash>/README.md)- Include a summarized version of the key deployment steps directly in this guide as a fallback
- Consider implementing automated link checking in CI to detect broken references early
🔗 Analysis chain
Line range hint
64-65
: Verify and future-proof external README linksThe guide relies heavily on external README links for deployment instructions. Consider:
- Adding version/commit hash references to ensure link stability
- Including key deployment steps directly in this guide
- Setting up link checking in the CI pipeline
Also applies to: 82-83
Let me verify the content of the guide and the actual references to the README to ensure a complete verification.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the README links are valid and note the current commit hash gh repo view celestiaorg/op-plasma-celestia --json url,defaultBranchRef | \ jq -r '.defaultBranchRef.name as $branch | .url + "/blob/" + $branch + "/README.md"' | \ xargs curl -s -I | grep "HTTP/"Length of output: 204
Script:
#!/bin/bash # Check the content of the guide and look for README references rg -U "README" how-to-guides/optimism.mdLength of output: 502
Script:
#!/bin/bash # Get the actual content of the lines mentioned in the review sed -n '64,65p;82,83p' how-to-guides/optimism.mdLength of output: 62
Script:
#!/bin/bash # Check if there are any other files referencing the same external repository rg -l "celestiaorg/op-plasma-celestia"Length of output: 64
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.
lgtm, but should include the youtube link
done! thanks for reviewing |
closes #1705
Summary by CodeRabbit
New Features
Documentation