-
Notifications
You must be signed in to change notification settings - Fork 382
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
feat: split consensus node page #1472
feat: split consensus node page #1472
Conversation
Warning Rate Limit Exceeded@jcstein has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 30 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe documentation has been updated to distinguish more clearly between "Full Consensus Node" and "Validator Node" roles within the Celestia network. This includes creating separate guides for each node type, refining existing content to align with this distinction, and updating links and terminology across various documents to ensure consistency and clarity. Changes
Assessment against linked issues
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 (
|
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.
Review Status
Actionable comments generated: 7
Configuration used: CodeRabbit UI
Files selected for processing (11)
- .vitepress/config.ts (1 hunks)
- developers/blobstream-proof-queries.md (1 hunks)
- nodes/arabica-devnet.md (1 hunks)
- nodes/celestia-app-vesting.md (1 hunks)
- nodes/decide-node.md (1 hunks)
- nodes/full-consensus-node.md (3 hunks)
- nodes/instantiate-testnet.md (1 hunks)
- nodes/mainnet.md (1 hunks)
- nodes/mocha-testnet.md (2 hunks)
- nodes/overview.md (1 hunks)
- nodes/validator-node.md (1 hunks)
Additional comments: 25
nodes/instantiate-testnet.md (1)
- 15-15: Updating the link to point to the hardware requirements in the
validator-node.md
document is a helpful change for users looking to set up a validator node. Ensure that the link is correctly formatted and functional, leading users directly to the "Hardware requirements" section of thevalidator-node.md
document.nodes/validator-node.md (6)
- 17-25: The hardware requirements listed for running a validator node are clear and concise. However, it's beneficial to mention that these are minimum recommendations and that actual requirements may vary based on network conditions and validator responsibilities.
Consider adding a note that these are minimum recommendations and actual requirements may vary.
- 32-33: Linking to the setup instructions for a full consensus node is crucial for ensuring that validator node operators have the necessary background setup. Ensure that the link is accurate and leads directly to the relevant section.
- 47-54: The instructions for delegating stake to a validator are clear. However, ensure that users understand the importance of securely managing their validator wallet and the implications of delegating stake.
Consider adding a security note about the importance of securely managing the validator wallet.
- 93-102: Mentioning the optional deployment of a bridge node and its significance to the Celestia network is valuable. It's good to clarify that it's recommended but not required for
celestia-app
. This helps users make informed decisions based on their setup goals.- 143-154: Starting the validator node and creating the validator onchain are critical steps. The instructions are clear, but ensure that users are aware of the financial and security responsibilities that come with being a validator.
Consider adding a note on the financial and security responsibilities of running a validator node.
- 213-222: Encouraging users to submit their validator information to the networks repository is a good practice for community engagement and network health. Ensure that the link to the repository is correct and that guidelines for submission are clear and accessible.
nodes/mocha-testnet.md (2)
- 27-28: The links to the full consensus node and validator node documentation have been updated to point to the correct URLs. This change enhances the navigability and accuracy of the documentation.
- 67-67: The reference to running a local full consensus node has been corrected to include the
.md
extension in the link. This ensures that the link is functional and directs users to the correct documentation page.nodes/arabica-devnet.md (1)
- 62-62: The link to the full consensus node documentation has been updated to include the correct path. This change ensures users are directed to the appropriate resource for setting up a full consensus node in the Arabica devnet context.
nodes/mainnet.md (1)
- 99-100: The links to the Full Consensus Node and Validator Node have been updated to point to their respective new documentation pages. This change aligns with the PR's objective to enhance documentation structure and clarity by providing separate, focused guides for different types of nodes. It's crucial to verify that these links are correctly pointing to the intended markdown files and that the files exist at the specified paths. Additionally, ensure that the naming convention used in the links matches the actual file names to avoid broken links.
Verification successful
The verification process confirms that the links to the Full Consensus Node and Validator Node documentation have been correctly updated and point to the existing markdown files within the 'nodes' directory. This aligns with the PR's objective to enhance documentation structure and clarity.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the existence of the linked markdown files. fd 'full-consensus-node.md' && echo "Full consensus node file exists." fd 'validator-node.md' && echo "Validator node file exists."Length of output: 243
nodes/celestia-app-vesting.md (4)
- 381-388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [10-10]
The term "devnet" is correctly used in the context of a development network, and "testnet" is correctly used for a testing network. The static analysis suggestion to split these terms into "dev net" and "test net" is not applicable here.
- 384-385: Consider adding a brief explanation or summary before the links to the validator and full consensus node setup guides. This would help readers understand why they might want to set up a validator or full consensus node in the context of creating a vesting account on the Mocha testnet.
- 381-388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [312-312]
Please ensure that all links, such as the one to the Mocha testnet documentation, are periodically reviewed and updated if necessary to ensure they remain valid and point to the intended content. This helps maintain the document's usefulness and reliability over time.
- 381-388: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]
Consider adding a table of contents at the beginning of the document for easier navigation. This would be particularly helpful given the document's length and the multiple sections covering different scenarios and optional configurations.
.vitepress/config.ts (1)
- 413-420: The addition of "Full consensus node" and "Validator node" as sub-items under the "Consensus" section in the sidebar is correctly implemented and aligns with the PR objectives to enhance documentation structure and clarity. This change makes the documentation more navigable and user-friendly by clearly distinguishing between the two types of consensus nodes.
developers/blobstream-proof-queries.md (9)
- 11-11: The terminology update from "consensus full node" to "full consensus node" is clear and consistent with the PR objectives to enhance documentation clarity. Good job on standardizing the terminology.
- 11-11: The link provided for the "full consensus node" should be verified to ensure it points to the correct and newly created page as per the PR objectives. This is crucial for maintaining the integrity of the documentation and ensuring users are directed to the appropriate resources.
- 8-14: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [17-19]
The explanation of the use of Celestia's consensus node's RPC to query for proofs is clear and concise. However, it might be beneficial to briefly mention why a validating node is not required for querying proofs, as this could provide additional clarity for readers unfamiliar with the Celestia network's architecture.
- 8-14: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [22-23]
The distinction between a PFB transaction and the actual data blob, and their treatment within the Celestia block, is well explained. This clarification helps in understanding the underlying mechanics of data handling in Celestia blocks, which is essential for comprehending the proof querying process.
- 8-14: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [29-29]
The mention of "Blobstream X" introduces a specific implementation of Blobstream. It would be helpful to include a brief description or a link to more detailed documentation on Blobstream X for readers who may not be familiar with it. This would aid in understanding its role and significance in the context of querying proofs.
- 8-14: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [113-114]
The requirement for a Merkle proof of the data root tuple to a data root tuple root is a critical step in the proof verification process. It might be beneficial to include an example or a more detailed explanation of how to generate or obtain this Merkle proof, especially for readers who might be new to working with Merkle proofs.
- 8-14: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [333-334]
The explanation of the need for two proofs to verify a rollup transaction's inclusion is clear. However, providing a concrete example of a namespace Merkle proof and a binary Merkle proof in action would greatly enhance understanding. Examples help bridge the gap between theory and practical application, making the documentation more accessible to a wider audience.
- 8-14: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [366-366]
The section on converting proofs to be usable in the
DAVerifier
library is highly technical and assumes a significant amount of prior knowledge. While the detailed breakdown is appreciated, a brief introductory paragraph summarizing the goal of this conversion process could make this section more approachable to readers with varying levels of expertise.
- 8-14: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1155-1159]
The conclusion succinctly summarizes the steps involved in verifying the inclusion proof of a transaction and proving that the data root tuple is committed to by the Blobstream X smart contract. This recap is helpful for readers to consolidate their understanding of the process. It might be beneficial to include a call to action or next steps for readers who have successfully followed through the guide, guiding them on how to apply this knowledge in their projects or what to explore next.
before I review, can you please look at the coderabbit suggestions and implement them @Mackenzie-OO7 ? thanks! |
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
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- nodes/full-consensus-node.md (4 hunks)
- nodes/overview.md (2 hunks)
Additional Context Used
Additional comments not posted (5)
nodes/overview.md (2)
14-17
: ,
The previous suggestions regarding link naming and grammatical corrections have been correctly implemented.
29-29
: The grammatical correction from "setup" to "set up" is correctly implemented.nodes/full-consensus-node.md (3)
2-2
: ,
The previous suggestion regarding the capitalization in the description has been correctly implemented.
14-15
: The updates clarify the guide's focus on setting up a full consensus node, enhancing clarity for readers.
42-42
: Correct usage of "Set up" for grammatical correctness.
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: 4
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- nodes/full-consensus-node.md (4 hunks)
Additional Context Used
Additional comments not posted (10)
nodes/full-consensus-node.md (10)
2-2
: The description in the YAML front matter has been updated correctly to reflect the focus of the document.
14-15
: The introduction succinctly outlines the purpose of the guide, focusing on setting up a full consensus node and its role in syncing blockchain history. This aligns well with the PR objectives.
42-42
: The section on setting up dependencies is concise and directs users to another document for detailed instructions, maintaining the focus and readability of this guide.
50-52
: The instructions for setting up the P2P networks are clear and include a step-by-step process. The use of "set up" as a verb is grammatically correct, addressing previous feedback.
1-5
: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [97-97]
The instructions for setting seeds are clear and provide a practical example of modifying the configuration file. This is a crucial step for node connectivity and is well-documented.
1-5
: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [122-123]
The optional configuration for persistent peers is well-explained, including the rationale for when it's advised. This helps users make informed decisions based on their node setup.
1-5
: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [154-156]
The section on connecting a consensus node to a bridge node is relevant and important for users who need to enable transaction indexing and retain all block data. The instructions are clear and actionable.
1-5
: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [211-211]
The explanation of the default syncing mechanism is informative, highlighting the security benefits and potential time investment. This sets realistic expectations for users.
1-5
: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [284-284]
The instruction to start the consensus node is straightforward. The optional note about running
celestia-app
as a background process is a helpful addition for users.
1-5
: > 📝 NOTEThis review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [322-322]
The command to make the RPC service available publicly is correctly formatted, addressing a previous comment. This is crucial for users who wish to configure their node as a public RPC endpoint.
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
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (1)
- nodes/full-consensus-node.md (6 hunks)
Additional Context Used
Additional comments not posted (2)
nodes/full-consensus-node.md (2)
50-52
: The phrase "Now we will set up the P2P Networks" correctly uses "set up" as a verb, addressing a previous grammatical correction. This change enhances the document's professionalism.
60-60
: Adding a comma after "network" improves readability, as suggested in previous feedback. This minor change makes the instructions clearer.
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.
awesome work, thanks @Mackenzie-OO7!
Happy to help! |
* docs: split consensus node page * docs: setup to set up * docs: setup to set up 2 * docs: coderabbit reviews * docs: change to everywhere * docs: add backlink --------- Co-authored-by: jcstein <[email protected]>
…stream (#1489) * docs: add documentation on how to build rollups on Celestia * docs: remove question * docs: one -> value * docs: explain more the proofs + reformating * docs: set ETA * docs: expand on protobuf deserialisation * docs: expand on protobuf deserialisation * docs: rewrite, reorganise, explain * docs: review feedback * docs: grammar * docs: remove new lines * docs: add links to inclusion proofs * docs: update blobstream contract addresses (#1485) Replaces #1482 * ci: test removing the CNAME file for clean up of previews. (#1493) * docs: update blobstream (#1495) * docs: update bubs-testnet.md with new URIs * docs: add status page and bridge * docs: comment out thirdweb bc chain is different now * docs: update gm portal and contract guides for bubs * docs: add mocha and namespace * docs: add bridging context * [GH Action] Update release tags and commit SHAs for arabica (#1497) * [automated GH action] update latest release tags & commit sha (arabica) * Update .vitepress/constants/arabica_versions.js --------- Co-authored-by: jcstein <[email protected]> Co-authored-by: Josh Stein <[email protected]> * [GH Action] Update release tags and commit SHAs for mocha (#1498) * [automated GH action] update latest release tags & commit sha (mocha) * Update .vitepress/constants/mocha_versions.js --------- Co-authored-by: jcstein <[email protected]> Co-authored-by: Josh Stein <[email protected]> * docs: update go constants for arabiac and mocha * docs: update to make install for all types of machines (#1502) * feat: Update node-tutorial.md (#1478) this is live in v0.13.0 i think, cc @vgonkivs for review * feat: split consensus node page (#1472) * docs: split consensus node page * docs: setup to set up * docs: setup to set up 2 * docs: coderabbit reviews * docs: change to everywhere * docs: add backlink --------- Co-authored-by: jcstein <[email protected]> * docs: rollup available link Vitalik (#1505) * docs: rollup link Vitalik * docs: changes follow suggestion for example giving comma * docs: Fix proof query example errors (#1504) * Add missing return statement in toRowProofs function * fix dataRoot slice out of range error * fixes another missing return in toRowProofs * docs: fix blobstream proof queries code (#1506) * docs: initial commit, needs revision * docs: remove old light node config * docs: edits and refactor page * docs: minor copy edit * docs: cleanup for new deployment guide * docs: todos for deployment guide * docs: edit intro * docs: step 1-2 * docs: deploy steps and table * docs: remainder of deploy steps draft * docs: change style of quickstart to Quickstart: Deploy an Orbit rollup * docs: add todos * docs: update contract addresses * docs: using light node, namespace, auth token * docs: add link to arb doc * docs: add todo * docs: resolve build errs * docs: add bridging guide * docs: small revisions * docs: copy edits * docs: deploy revision * docs: fix blobstream address * docs: add alchemy rpc section * docs: note about opening ports in docker * docs: bump celestia-node version * docs: edit l1 to l2 * docs: add newer success logs * feat: add full node page to docs for orbit * feat: write out full node page * docs: more info on validatorfn page * docs: update wasm root * docs: update nitro release * docs: use v2.3.1-rc.1 everywhere for nitro * docs: edit out 'click heres' * fix: links * docs: add eth-rpc variable * docs: coderabbit suggestions * docs: clarify wss * docs: swap steps 6 and 7 for deploy doc * docs: edit wss section * docs: add more context for wss * docs: improve md readabliity in table * docs: update deployui url * docs: update ui url * docs: we'll use in next step * docs: updates * docs: add fn * [GH Action] Update release tags and commit SHAs for mainnet (#1507) * [automated GH action] update latest release tags & commit sha (mainnet) * Update .vitepress/constants/mainnet_versions.js --------- Co-authored-by: jcstein <[email protected]> Co-authored-by: Josh Stein <[email protected]> * docs: update newmetric endpoints Resolves #1501 * docs: update orbit URL * docs: coderabbit suggestions * docs: add op raspberry testnet * [GH Action] Update release tags and commit SHAs for arabica (#1511) * [automated GH action] update latest release tags & commit sha (arabica) * Apply suggestions from code review --------- Co-authored-by: jcstein <[email protected]> Co-authored-by: Josh Stein <[email protected]> * docs: add sepolia -> mocha (#1512) * fix typo in arabica-devnet page (#1513) * [WIP] docs: add audits for blobstream x (#1491) * docs: add audits for blobstream x * docs: add veridise audit * Update rollup-overview.md (#1516) * feat: restructure developers category (#1515) * feat: drafting buttons * fix: color of btn * feat: add note text optional * feat: rename to build-whatever.md * fix: copy * feat: category shuffle * fix: suggestions from code review * feat: add vistara * Apply suggestions from code review * fix: typo * feat: add alt texts * fix: typo * feat: aria-labels * docs: shuffle intro * feat: add celestia-node.sh script * fix: clarify section vs. tutorial * feat: exit if dir change fails * Update mocha_versions.js (#1529) * Update mocha_versions.js * Update mocha_versions.js * Add Grove archival endpoints to `nodes/mainnet.md` (#1525) * Add Grove to nodes/mainnet.md * Apply suggestions from code review Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update nodes/mainnet.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Store image in src code * Add description of archival nodes * Apply suggestions from code review * Apply suggestions from code review * Update nodes/mainnet.md --------- Co-authored-by: joshcs.eth <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * docs: fix typos (#1526) * fix typo * fix typo * fix typo * fix typo * fix typos * fix typo * fix typo * fix typos * fix typos * fix typos * fix typos * fix typo * fix typo * fix typo Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Apply suggestions from code review --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: joshcs.eth <[email protected]> * Update mainnet.md (#1527) Add noders endpoints * Update mocha-testnet.md (#1528) * Update mocha-testnet.md Add noders endpoints * Fixed link to endpoint * Apply suggestions from code review --------- Co-authored-by: joshcs.eth <[email protected]> * fix: veridise path * feat: add to menu * docs: begin lint * docs: remove duplicate headings * docs: clarify duplicate headings * docs: better headings * docs: mention blobstream rollups docs in the blobstream overview --------- Co-authored-by: Josh Stein <[email protected]> Co-authored-by: Matthew Sevey <[email protected]> Co-authored-by: jcstein <[email protected]> Co-authored-by: celestia-bot <[email protected]> Co-authored-by: jcstein <[email protected]> Co-authored-by: Levai Mackenzie Ágbàrà <[email protected]> Co-authored-by: Guspan Tanadi <[email protected]> Co-authored-by: Kassius <[email protected]> Co-authored-by: Ratan Kaliani <[email protected]> Co-authored-by: Callum Waters <[email protected]> Co-authored-by: Alex Beckett <[email protected]> Co-authored-by: mindstyle85 <[email protected]> Co-authored-by: Daniel Olshansky <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: omahs <[email protected]> Co-authored-by: Alex Moskaleu (SeptimA) <[email protected]>
Overview
closes #1428
Checklist
Summary by CodeRabbit