-
Notifications
You must be signed in to change notification settings - Fork 53
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
docs: full node guide #462
Conversation
WalkthroughThe pull request modifies the sidebar configuration and the content of the guide on running a full node. The sidebar entry is simplified for clarity, changing the title and URL to focus solely on running a rollup full node. The guide itself is revised to concentrate on setting up a full node alongside a sequencer node, detailing initialization, configuration, and execution steps while removing unnecessary references to multi-node networks. Changes
Possibly related PRs
Suggested labels
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: 0
Outside diff range and nitpick comments (2)
guides/full-node.md (2)
24-37
: Add a comma to improve readability.The instructions for initializing the chain config are clear and easy to follow. The code block correctly shows how to update the
rollkit.toml
file to specify a different config directory for the full node.However, please add a comma after "In order to run a full node" to improve readability:
-In order to run a full node you have to specify a different config directory. +In order to run a full node, you have to specify a different config directory.Tools
LanguageTool
[style] ~25-~25: Consider a shorter alternative to avoid wordiness.
Context: ...he sequencer node. ## Getting started In order to run a full node you have to specify a d...(IN_ORDER_TO_PREMIUM)
[typographical] ~26-~26: It seems that a comma is missing.
Context: ...Getting started In order to run a full node you have to specify a different config ...(IN_ORDER_TO_VB_COMMA)
Line range hint
45-89
: Add commas to improve readability.The instructions for copying the genesis file, setting up the p2p address, and starting the full node are clear and easy to follow. The code blocks correctly show how to perform these tasks.
However, please add commas to improve readability:
- Add a comma before 'and' to connect two independent clauses:
-Notice that we are using the `P2P_ID` environment variable to set the seed node and we expect that the sequencer node is listening for p2p connections on port `26656`. +Notice that we are using the `P2P_ID` environment variable to set the seed node, and we expect that the sequencer node is listening for p2p connections on port `26656`.
- Add a comma after the conjunctive/linking adverb 'Also':
-Also we specify the `--rollkit.aggregator=false` flag to indicate that this node is not an aggregator node. +Also, we specify the `--rollkit.aggregator=false` flag to indicate that this node is not an aggregator node.Tools
LanguageTool
[uncategorized] ~89-~89: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...nvironment variable to set the seed node and we expect that the sequencer node is li...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~89-~89: A comma may be missing after the conjunctive/linking adverb ‘Also’.
Context: ...ng for p2p connections on port26656
. Also we specify the `--rollkit.aggregator=fa...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- .vitepress/config.ts (1 hunks)
- guides/full-node.md (3 hunks)
Files skipped from review due to trivial changes (1)
- .vitepress/config.ts
Additional context used
LanguageTool
guides/full-node.md
[style] ~25-~25: Consider a shorter alternative to avoid wordiness.
Context: ...he sequencer node. ## Getting started In order to run a full node you have to specify a d...(IN_ORDER_TO_PREMIUM)
[typographical] ~26-~26: It seems that a comma is missing.
Context: ...Getting started In order to run a full node you have to specify a different config ...(IN_ORDER_TO_VB_COMMA)
[uncategorized] ~89-~89: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...nvironment variable to set the seed node and we expect that the sequencer node is li...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~89-~89: A comma may be missing after the conjunctive/linking adverb ‘Also’.
Context: ...ng for p2p connections on port26656
. Also we specify the `--rollkit.aggregator=fa...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Additional comments not posted (3)
guides/full-node.md (3)
8-8
: LGTM!The introduction clearly states the purpose of the guide.
22-22
: LGTM!The statement correctly assumes that the user has a running sequencer node, which is a prerequisite for setting up a full node.
39-43
: LGTM!The command to initialize the chain config for the full node is correct and includes the necessary flags.
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: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- .vitepress/config.ts (1 hunks)
- guides/full-node.md (1 hunks)
- guides/overview.md (1 hunks)
Files skipped from review due to trivial changes (1)
- guides/overview.md
Files skipped from review as they are similar to previous changes (1)
- .vitepress/config.ts
Additional context used
Markdownlint
guides/full-node.md
43-43: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
75-75: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (4)
guides/full-node.md (4)
3-6
: LGTM!The introduction provides a clear and concise overview of the purpose and benefits of setting up a full node. It effectively sets the context for the rest of the guide.
7-14
: LGTM!The prerequisites section clearly outlines the requirements for setting up a full node. The specific details, such as port numbers, are helpful for users to ensure they have the necessary components in place before proceeding with the guide.
15-64
: LGTM!The "Setting Up Your Full Node" section provides clear and well-organized step-by-step instructions for configuring and starting the full node. The code snippets and explanations are helpful for users to follow along and understand the purpose of each command and configuration.
Tools
Markdownlint
43-43: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
71-81
: LGTM!The "Verifying Full Node Operation" section provides a helpful example of the output users should expect to see when their full node is successfully connecting to the network and processing blocks. The explanation of the output is clear and informative.
Tools
Markdownlint
75-75: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
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
Outside diff range and nitpick comments (3)
guides/full-node.md (3)
84-86
: Add a comma before 'and' to connect two independent clauses.As suggested by the LanguageTool static analysis tool, please add a comma before 'and' in the sentence:
-If your rollup uses EVM as an execution layar and you see an error like `datadir already used by another process`, it means you have to remove all the state from rollup data directory (`/root/.yourrollup_fn/data/`) and specify a different data directory for the EVM client. +If your rollup uses EVM as an execution layar, and you see an error like `datadir already used by another process`, it means you have to remove all the state from rollup data directory (`/root/.yourrollup_fn/data/`) and specify a different data directory for the EVM client.This helps to connect the two independent clauses properly.
Tools
LanguageTool
[uncategorized] ~85-~85: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ur rollup uses EVM as an execution layar and you see an error like `datadir already ...(COMMA_COMPOUND_SENTENCE)
27-27
: Add a singular genitive ('s) apostrophe to 'lets'.As suggested by the LanguageTool static analysis tool, please add a singular genitive ('s) apostrophe to the word 'lets':
-Initialize the chain config for the full node, lets call it `FullNode` and set the chain ID to your rollup chain ID: +Initialize the chain config for the full node, let's call it `FullNode` and set the chain ID to your rollup chain ID:This corrects the grammatical error in the sentence.
Tools
LanguageTool
[uncategorized] ~27-~27: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...ize the chain config for the full node, lets call itFullNode
and set the chain ID...(AI_HYDRA_LEO_APOSTROPHE_S_XS)
1-91
: Excellent guide for setting up a full node!The guide is well-structured, easy to follow, and covers all the necessary steps for setting up a full node in a Rollkit-based blockchain network. The code snippets provided are clear and include helpful comments.
A few additional suggestions:
- Consider adding a section on the hardware requirements and recommended specifications for running a full node.
- Include information on how to update and maintain the full node over time, such as applying security patches, upgrading the software version, and backing up the data.
- Provide links to additional resources or documentation for further reading and troubleshooting.
Overall, great work on putting together this comprehensive guide!
Tools
LanguageTool
[uncategorized] ~27-~27: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...ize the chain config for the full node, lets call itFullNode
and set the chain ID...(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~85-~85: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ur rollup uses EVM as an execution layar and you see an error like `datadir already ...(COMMA_COMPOUND_SENTENCE)
Markdownlint
43-43: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
77-77: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- guides/full-node.md (1 hunks)
Additional context used
LanguageTool
guides/full-node.md
[uncategorized] ~27-~27: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...ize the chain config for the full node, lets call itFullNode
and set the chain ID...(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~85-~85: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ur rollup uses EVM as an execution layar and you see an error like `datadir already ...(COMMA_COMPOUND_SENTENCE)
Markdownlint
guides/full-node.md
43-43: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
77-77: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (7)
guides/full-node.md (7)
21-25
: LGTM!Using a separate configuration directory for the full node is a good practice to avoid conflicts with the sequencer node.
29-31
: Replace the placeholder chain ID with the actual value.The
--chain-id
flag is set toyour-rollup-chain-id
, which is a placeholder. Please replace it with the actual chain ID of your rollup.
35-37
: Verify the source path of the genesis file.Please ensure that the source path
/root/.yourrollupd/config/genesis.json
correctly points to the genesis file of your sequencer node. Update the path if necessary.
43-45
: The past review comment suggesting the addition of a language specifier for the code block is still valid. Please address it.Tools
Markdownlint
43-43: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
49-51
: Replace the example P2P address with the actual value.The P2P address
12D3KooWJbD9TQoMSSSUyfhHMmgVY3LqCjxYFz8wQ92Qa6DAqtmh
used in theP2P_ID
environment variable is an example. Please replace it with the actual P2P address from your sequencer node's logs.
57-66
: LGTM!The
rollkit start
command and its flags are correctly configured for starting the full node. The ports and addresses are set to avoid conflicts with the sequencer node, and theP2P_ID
environment variable is used to set the seed node.
77-80
: The past review comment suggesting the addition of a language specifier for the code block is still valid. Please address it.Tools
Markdownlint
77-77: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Summary by CodeRabbit
New Features
Bug Fixes
Documentation