Conversation
There was a problem hiding this comment.
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 |
| ## 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.
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.
There was a problem hiding this comment.
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:Details
❓ Verification inconclusive
Verify port consistency between example and port table
The CLI example uses port
9098forrpc.grpc_laddr, but the ports table incelestia-app.mdlists 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.mdandcelestia-app.mdIt 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 |
|
We'll also need to bump the version specified in in install step for arabica but IIRC we can do that with https://github.com/celestiaorg/docs/actions/workflows/latest_tags.yml workflow after we create v4.0.0-arabica so I'll merge this as-is. |
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)