feat: revert nitro guide to stable commit and remove placeholder images #1389
Conversation
WalkthroughThe transition involves moving from a specific tagged version ( Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
public/img/nitro-vroom-devnet.pngis excluded by:!**/*.pngpublic/img/nitro-vroom.pngis excluded by:!**/*.png
Files selected for processing (2)
- developers/arbitrum-deploy.md (2 hunks)
- developers/arbitrum-mocha.md (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- developers/arbitrum-deploy.md
Additional comments: 5
developers/arbitrum-mocha.md (5)
- 40-41: The image version for deploying an Arbitrum Nitro rollup to the Mocha testnet has been updated from
v0.12.1tov0.12.4. Ensure that this version change is consistent with the testing outcomes mentioned in the PR objectives, specifically thatv0.12.4has been tested and confirmed to work effectively in the desired configurations.- 38-44: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [40-49]
The addition of
celestia light startcommand with specific flags (--core.ip,--p2p.network,--log.level, and--gateway) in thedocker-compose.yamlfile is a significant change. It's crucial to ensure that these flags are correctly documented and supported by thev0.12.4version ofcelestia-node. Additionally, verify that the network and logging configurations are appropriate for the intended use case in the Mocha testnet environment.
- 4-9: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [53-54]
The update to
NODE_PATHin thetest-node.bashscript to use.celestia-light-{{constants.mochaChainId}}directory aligns with the changes in the docker-compose configuration. This consistency is good, but ensure that theconstants.mochaChainIdis correctly defined and accessible in this script's context to avoid path resolution issues.
- 4-9: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [56-56]
The change in the
export CELESTIA_NODE_AUTH_TOKENcommand to use thecelestia-dacontainer name is aligned with the updated container name in the docker-compose file. This change is necessary for consistency and should work as intended if the container name matches across all scripts and configurations.
- 38-44: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-62]
Overall, the documentation update to reflect the change from
v0.12.1tov0.12.4for deploying an Arbitrum Nitro rollup to the Mocha testnet seems thorough. However, it's crucial to ensure that all changes, including command updates and configuration adjustments, are validated against the actual behavior ofcelestia-nodeversionv0.12.4and the specific requirements of the Mocha testnet environment. Additionally, consider adding more detailed guidance for users on configuring their deployment securely, especially regarding running containers as non-root users.
|
|
||
| # Deploy an Arbitrum rollup to Mocha testnet | ||
|
|
||
|  | ||
|
|
||
| <!-- markdownlint-disable MD033 --> | ||
| <script setup> | ||
| import constants from '/.vitepress/constants/constants.js' |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [50-50]
The warning about not advising to run with user: root permissions in production is important for security best practices. It's good that this caution is included, but it might be beneficial to provide an alternative configuration for users to avoid running as root, especially if this documentation is intended for a wider audience that might include production environments.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [59-60]
The update to the tendermint-rpc URL and the placeholder for <your-10bytenamespace> in the config.ts file are critical for connecting to the correct Tendermint RPC and using a unique namespace. Users should be guided on how to generate or choose their 10-byte namespace if not already provided, ensuring they understand the importance of this value in the context of their deployment.
jcstein
changed the title
|
chef still cooking |
Overview
This PR reverts Nitro docs to a stable commit based on testing below. Testing done on
arm64machine.v0.2.2-no-blobstreamwith v0.12.1v0.2.2-no-blobstreamv0.2.2-no-blobstreamwith v0.12.4v0.2.2-no-blobstream--validationposts (doesn't validate)--validationposts (doesn't validate)Errors posting seem to boil down to connection issues in docker, yet to be resolved:
See preview of modified content
Checklist
Summary by CodeRabbit
Summary by CodeRabbit