-
Notifications
You must be signed in to change notification settings - Fork 410
docs: app v4 #2021
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?
docs: app v4 #2021
Conversation
WalkthroughThe documentation for running and configuring celestia-app consensus nodes was updated for clarity and to reflect requirements for celestia-app version 4.0.0 and above. Notably, the instructions now specify the mandatory Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Docs
participant celestia-app
User->>Docs: Reads updated build instructions
Docs->>User: Explains multiplexer support in celestia-appd binary
User->>Docs: Reads consensus node startup instructions
Docs->>User: Instructs to set rpc.grpc_laddr for v4.0.0+
User->>celestia-app: Starts node with required flags
Assessment against linked issues
Possibly related PRs
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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.
Pull Request Overview
This PR updates the documentation for celestia-app to reflect changes in app versions and associated configuration details for running a consensus node and installing the binary.
- Update consensus node instructions for celestia-app v4.0.0 by requiring the --rpc.grpc_laddr flag.
- Revise version-specific guidance and improve language consistency in installation instructions.
- Clarify the upgraded binary build process by noting multiplexer support.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
how-to-guides/consensus-node.md | Updated version notes and added the --rpc.grpc_laddr flag for v4.0.0 |
how-to-guides/celestia-app.md | Improved language consistency and clarified build instructions |
@@ -320,13 +320,13 @@ The [Node snapshots guide](/how-to-guides/snapshots.md) provides everything you | |||
|
|||
## Start the consensus node | |||
|
|||
If you are running celestia-app v1.x.x: | |||
If you are running celestia-app >= v4.0.0, the `rpc.grpc_laddr` config option is required. This option can be set via the CLI flag `--rpc.grpc_laddr tcp://0.0.0.0:9098` or in the `config.toml`. |
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.
Consider adding a brief note or table summarizing the version-specific instructions to clearly delineate between v1.x.x, v2.x.x, and v4.0.0 requirements. This will help users quickly identify the correct steps based on their app version.
Copilot uses AI. Check for mistakes.
@@ -57,7 +57,7 @@ Be sure to select the correct network to install the binary for. | |||
|
|||
::: | |||
|
|||
3. Build and install the `celestia-appd` binary: | |||
3. Build and install the `celestia-appd` binary with multiplexer support: |
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.
Consider providing a short explanation or a link to additional documentation on what 'multiplexer support' entails, so users know if they require any further configuration.
3. Build and install the `celestia-appd` binary with multiplexer support: | |
3. Build and install the `celestia-appd` binary with multiplexer support: | |
Multiplexer support allows the `celestia-appd` binary to handle multiple streams of data efficiently. | |
For more details, refer to the [multiplexer support documentation](https://docs.celestia.org/multiplexer-support). |
Copilot uses AI. Check for mistakes.
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: 0
🧹 Nitpick comments (4)
how-to-guides/celestia-app.md (3)
20-21
: Ensure consistent product name casingThe document title and other sections use lowercase
celestia-app
. To maintain consistency, change the capitalized usage here to lowercase:-def Celestia-app is the software that enables you to run +celestia-app is the software that enables you to run
25-26
: Consider using present perfect tense for clarityUsing “have completed” can improve readability:
-def This section of the tutorial assumes you completed the steps in +This section of the tutorial assumes you have completed the steps in
60-60
: Link or explain multiplexer supportGreat addition of multiplexer support! For readers unfamiliar with this concept, consider adding a hyperlink to relevant documentation or a brief parenthetical explanation of what “multiplexer support” entails.
how-to-guides/consensus-node.md (1)
329-329
: Add missing commas for readabilityAdding commas after the introductory clause and before “so” will improve clarity:
- If you are running celestia-app v2.x.x then you'll want to start the node with a `--v2-upgrade-height` that is dependent on the network. + If you are running celestia-app v2.x.x, then you'll want to start the node with a `--v2-upgrade-height` that is dependent on the network. - The `--v2-upgrade-height` flag is only needed during the v2 upgrade height so after your node has executed the upgrade (e.g. you see the log `upgraded from app version 1 to 2`), you don't need to provide this flag for future `celestia-appd start` invocations. + The `--v2-upgrade-height` flag is only needed during the v2 upgrade height, so after your node has executed the upgrade (e.g. you see the log `upgraded from app version 1 to 2`), you don't need to provide this flag for future `celestia-appd start` invocations.🧰 Tools
🪛 LanguageTool
[typographical] ~329-~329: Consider adding a comma.
Context: ... If you are running celestia-app v2.x.x then you'll want to start the node with a `-...(IF_THEN_COMMA)
[uncategorized] ~329-~329: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...only needed during the v2 upgrade height so after your node has executed the upgrad...(COMMA_COMPOUND_SENTENCE_2)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
how-to-guides/celestia-app.md
(2 hunks)how-to-guides/consensus-node.md
(4 hunks)
🧰 Additional context used
🪛 LanguageTool
how-to-guides/consensus-node.md
[typographical] ~329-~329: Consider adding a comma.
Context: ... If you are running celestia-app v2.x.x then you'll want to start the node with a `-...
(IF_THEN_COMMA)
[uncategorized] ~329-~329: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...only needed during the v2 upgrade height so after your node has executed the upgrad...
(COMMA_COMPOUND_SENTENCE_2)
🔇 Additional comments (1)
how-to-guides/consensus-node.md (1)
323-327
:❓ Verification inconclusive
Verify port consistency between example and port table
The CLI example uses port
9098
forrpc.grpc_laddr
, but the ports table incelestia-app.md
lists gRPC on port9090
. Please confirm the correct default port for v4.0.0+ and update the example or the table accordingly.To help verify:
🏁 Script executed:
#!/bin/bash # Search for gRPC port usages across docs rg -n "909[08]" how-to-guides/celestia-app.md how-to-guides/consensus-node.mdLength of output: 609
Confirm default gRPC port alignment between
consensus-node.md
andcelestia-app.md
It looks like the CLI example in how-to-guides/consensus-node.md (lines 323–327) uses port 9098 for
rpc.grpc_laddr
, but the ports table in how-to-guides/celestia-app.md (line 131) lists gRPC on port 9090. Please verify the correct default port for celestia-app v4.0.0+ and update either the example or the table so they match.• how-to-guides/consensus-node.md (lines 323–327):
celestia-appd start --rpc.grpc_laddr tcp://0.0.0.0:9098
• how-to-guides/celestia-app.md (line 131):
| 9090 | HTTP | 0.0.0.0 | gRPC | true | --grpc.address string |
Closes #2018
Ready for review but we don't need to merge it until we release celestia-app v4.0.0-arabica (expected Monday May 5)
Summary by CodeRabbit