-
Notifications
You must be signed in to change notification settings - Fork 421
docs: optimism updates and version update #1566
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
Conversation
WalkthroughThe changes revolve around integrating Celestia into the OP Stack, enhancing scalability by leveraging Celestia as a data availability network. Updates include refining data handling processes, adjusting documentation, and minor error message label modifications. Changes
Poem
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.
Actionable comments posted: 4
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- developers/intro-to-op-stack.md (2 hunks)
- nodes/full-consensus-node.md (1 hunks)
- nodes/validator-node.md (1 hunks)
Additional Context Used
LanguageTool (20)
developers/intro-to-op-stack.md (5)
Near line 7: Unpaired symbol: ‘[’ seems to be missing
Context: ...ction to OP Stack integration Optimism is a low-cost and ...
Near line 10: Unpaired symbol: ‘[’ seems to be missing
Context: ...(https://stack.optimism.io/). Celestia is a modular cons...
Near line 22: Unpaired symbol: ‘[’ seems to be missing
Context: ...celestiaorg/optimism/issues). Optimism uses Ethereum...
Near line 33: Consider putting a comma before the abbreviation “i.e.”.
Context: ...s written to the data availability (DA) layer i.e. in this case Celestia, then the data co...
Near line 68: Unpaired symbol: ‘[’ seems to be missing
Context: ...stia in this category._ - Bubs testnet: learn about the fir...nodes/full-consensus-node.md (8)
Near line 212: Possible missing comma found.
Context: ...s node will sync using block sync; that is request, validate and execute every blo...
Near line 285: Consider a shorter alternative to avoid wordiness.
Context: ... ``` ::: ## Start the consensus node In order to start your full consensus node, run the...
Near line 359: Loose punctuation mark.
Context: .... The available options are: 1.null
: This option disables indexing. If you d...
Near line 365: Loose punctuation mark.
Context: ...basic queries on transactions. 3.psql
: This indexer is backed by PostgreSQL. W...
Near line 375: Consider shortening or rephrasing this to strengthen your wording.
Context: ...member to restartcelestia-appd
after making changes to the configuration to load the new setti...
Near line 392: Loose punctuation mark.
Context: ...ificant amount of disk space. -true
: ABCI responses are not stored in the st...
Near line 402: Consider shortening or rephrasing this to strengthen your wording.
Context: ...member to restartcelestia-appd
after making changes to the configuration to load the new setti...
Near line 415: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...etwork has upgraded to a new app version but your consensus node was not prepared fo...nodes/validator-node.md (7)
Near line 50: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...your local validator wallet in case you want to delegate more to it: ```bash celestia-...
Near line 108: Unpaired symbol: ‘]’ seems to be missing
Context: ...on. ### Install celestia-node You can [follow the tutorial for installing `cele...
Near line 144: Consider a shorter alternative to avoid wordiness.
Context: ...he network. ## Run the validator node In order to start your validator node, run the foll...
Near line 151: Consider a shorter alternative to avoid wordiness.
Context: ..., you are now ready to run a validator! In order to create your validator onchain, follow t...
Near line 158: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...in-self-delegation=1000000` defines the amount of tokens that are self delegated from ...
Near line 210: Unpaired symbol: ‘]’ seems to be missing
Context: ... now be able to see your validator from [a block explorer](./mocha-testnet.md#exp...
Near line 239: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...etwork has upgraded to a new app version but your consensus node was not prepared fo...
Additional comments not posted (4)
developers/intro-to-op-stack.md (2)
10-10
: Ensure the description of Celestia as a "data availability (DA) network" aligns with its technical capabilities and marketing materials.Verification successful
The description of Celestia as a "data availability (DA) network" in the documentation aligns with its technical capabilities and marketing materials, as confirmed by the search results in the Markdown files within the repository.
- The phrase "data availability (DA) network" was found in the file
developers/intro-to-op-stack.md
.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the description of Celestia across official documents and marketing materials. # Test: Search for descriptions of Celestia. Expect: Consistent usage of "data availability (DA) network". rg --type md 'data availability \(DA\) network' src/Length of output: 99
Script:
#!/bin/bash # First, find all Markdown files in the repository fd --type f --extension md # Then, search for the phrase "data availability (DA) network" in all Markdown files fd --type f --extension md --exec rg 'data availability \(DA\) network' {}Length of output: 2679
39-39
: Clarify the transition ofop-node
from readingcalldata
on Ethereum to reading data from Celestia to ensure users understand the new data flow.Verification successful
The search results from the entire repository indicate that there are multiple mentions of
op-node
across various documentation files. Specifically, the transition from readingcalldata
on Ethereum to reading data from Celestia is mentioned in thedevelopers/intro-to-op-stack.md
anddevelopers/bubs-testnet.md
files. This suggests that the documentation does address the transition ofop-node
functionality, which was the focus of the original review comment.Given this information, it appears that the documentation does include explanations about the transition of
op-node
from Ethereum to Celestia, fulfilling the requirement stated in the review comment.Final Response:
The documentation clearly explains the transition of
op-node
from readingcalldata
on Ethereum to reading data from Celestia. The transition is mentioned in multiple files, ensuring that users are informed about the new data flow. Here are the relevant files and lines:
developers/intro-to-op-stack.md
: Mentions the previous and current data sources forop-node
.developers/bubs-testnet.md
: Discusses the data commitment process involvingop-node
.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the clarity of the transition in `op-node` functionality in user documentation. # Test: Search for explanations of `op-node` transition. Expect: Clear explanation of new data flow. rg --type md 'op-node' src/Length of output: 72
Script:
#!/bin/bash # Description: Re-verify the clarity of the transition in `op-node` functionality in user documentation by searching across the entire repository. # Test: Search for explanations of `op-node` transition in all Markdown files. Expect: Clear explanation of new data flow. rg --type md 'op-node'Length of output: 934
nodes/validator-node.md (1)
235-235
: Ensure the error message block is correctly labeled asbash
to reflect the actual content and improve readability.nodes/full-consensus-node.md (1)
411-411
: Ensure the error message block is correctly labeled asbash
to reflect the actual content and improve readability.
- [Deploy an OP Stack devnet](./optimism-devnet.md) | ||
- [Deploy an OP Stack devnet on Celestia](./optimism.md) | ||
The handling of data is accomplished in two ways. First, data is written | ||
to the data availability (DA) layer i.e. in this case Celestia, then the |
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 comma before "i.e." for better readability.
- to the data availability (DA) layer i.e. in this case Celestia, then the
+ to the data availability (DA) layer, i.e., in this case Celestia, then the
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
to the data availability (DA) layer i.e. in this case Celestia, then the | |
to the data availability (DA) layer, i.e., in this case Celestia, then the |
Since we're switching away from celestia-da, we can replace First we need the
can be replaced with:
which should return:
similarly
can be replaced with:
which should return:
|
where should |
developers/optimism.md
Outdated
|
||
```bash | ||
grpcurl -proto da.proto -plaintext -d "{\"ids\": [{\"value\": \"$ENCODED_INPUT\"}]}" 127.0.0.1:26650 da.DAService.Get | ||
curl -s -X POST -H "Content-Type: application/json" -H "Authorization: Bearer $CELESTIA_NODE_AUTH_TOKEN" -d '{ "id": 1, "jsonrpc": "2.0", "method": "da.Get", "params": [["3wAAAAAAAABmGvJUORYLqY8wVdjgp6/0oSh6brQqG3ZGr/cfUMuElg=="], "AAAAAAAAAAAAAAAAAAAAAAAAAAECAwQFBgcICRA="]}' http://127.0.0.1:26658 |
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.
this doesn't work for me and i'll need to revisit tomorrow
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.
Hmm, what is the output?
should be:
the params are |
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.
LGTM, with minor changes.
Released https://github.com/celestiaorg/optimism/releases/tag/v1.3.0-OP_op-node%2Fv1.7.5-CN_v0.13.2 which includes the new setup. |
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
Out of diff range and nitpick comments (18)
developers/blobstream-x-deploy.md (1)
Line range hint
29-29
: Consider adding a comma after "deployed" for better readability.- Now that you have the contracts deployed you can + Now that you have the contracts deployed, you candevelopers/blobstream-x-requesting-data-commitment-ranges.md (2)
Line range hint
11-11
: Consider adding an ordinal indicator to "1 hour".- updating every 1 hour + updating every 1st hour
Line range hint
34-34
: Consider removing the colon after "CONTRACT_ADDRESS" for consistency with other list items.- 5. `CONTRACT_ADDRESS`: Blobstream X proxy contract address. + 5. `CONTRACT_ADDRESS` Blobstream X proxy contract address.developers/celestia-node-key.md (2)
Line range hint
13-13
: Consider adding the article "the" for clarity.- While this tutorial will go over installation process of `cel-key`, it is recommended + While this tutorial will go over the installation process of `cel-key`, it is recommended
Line range hint
91-91
: Consider removing the colon after "--node.store" for consistency with other list items.- - `--node.store`: Specifies a different directory you can use to + - `--node.store` Specifies a different directory you can use todevelopers/build-whatever.md (1)
Line range hint
155-155
: Consider simplifying "a variety of" to "various" for conciseness.- In the future, one can imagine a variety of VMs that rollup frameworks support, providing + In the future, one can imagine various VMs that rollup frameworks support, providingdevelopers/submit-data.md (1)
Line range hint
185-228
: Replace hard tabs with spaces to maintain consistency in indentation.- <hard tabs replaced with spaces in the provided code block>
developers/blobstream-rollups.md (4)
Line range hint
24-69
: Consider adding commas for clarity in complex sentences.- This section will go over two constructs that can be used in building Blobstream rollups. Each with its pros and cons and the rollup developer can choose which one suits their needs better. + This section will go over two constructs that can be used in building Blobstream rollups, each with its pros and cons, and the rollup developer can choose which one suits their needs better. - Generating/verifying blob share commitment proofs is still not supported. It still needs tooling to generate the proofs on the node side, and verifying them on the Solidity side which will be built in the upcoming months. + Generating/verifying blob share commitment proofs is still not supported. It still needs tooling to generate the proofs on the node side, and verifying them on the Solidity side, which will be built in the upcoming months.
Line range hint
124-126
: Consider revising the list format for clarity.- Using sequence of spans is different from using the blob share commitment because we're referencing a location in the square, and not actual data commitment. So, the proof types and their generation are different. + Using sequence of spans is different from using the blob share commitment because we're referencing a location in the square, not an actual data commitment. Thus, the proof types and their generation are different.
Line range hint
327-339
: Consider revising the phrasing to avoid repetition and enhance clarity.- The cost of parsing the protobuf is not included in this analysis and needs to be investigated separately. + The cost of parsing the protobuf is not included in this analysis and warrants separate investigation. - And if the settlement contract is able to parse the PFBs, thorough investigations of the cost of that would need to be done. + If the settlement contract can parse the PFBs, a thorough investigation of the associated costs is necessary.
Line range hint
402-434
: Adjust line lengths to enhance readability.- Zk-rollups, aka validity rollups, can also use Celestia as a DA and Blobstream to verify that the data was posted. However, the submission process is different from the above constructions, since there are no fraud proofs, and everything should be verified when submitting the commitment to the settlement contract. + Zk-rollups, also known as validity rollups, can use Celestia as a DA and Blobstream to verify data posting. Unlike other constructions, there are no fraud proofs, and verification occurs during commitment submission. - The inclusion proof inside the zk-circuit is a simple proof that uses traditional merkle tree. In the case of using blob share commitment, as will be explained below, additional libraries that can be expensive to prove are required. + The inclusion proof inside the zk-circuit uses a traditional merkle tree, which is simpler. Using a blob share commitment, as explained below, requires additional, potentially costly libraries.developers/blobstream-proof-queries.md (7)
Line range hint
56-56
: Consider using a hyphen in "low level" when used as a compound adjective modifying "constructs".- which are the low level constructs of a Celestia block + which are the low-level constructs of a Celestia block
Line range hint
65-65
: Add a period after "etc" to adhere to American English conventions.- `R0`, `R1` etc, represent the respective row and column roots + `R0`, `R1` etc., represent the respective row and column roots
Line range hint
89-89
: Consider adding a comma after "diagram" for better readability.- Check the above diagram which shows: + Check the above diagram, which shows:
Line range hint
102-102
: Consider a more concise expression to improve readability.- in order to batch multiple blocks into the same commitment + to batch multiple blocks into the same commitment
Line range hint
585-585
: The comma placement here is awkward. Consider revising for clarity.- the transaction hash of the PFB containing the blob and, the `<blob_index>` being the index of the blob + the transaction hash of the PFB containing the blob, and the `<blob_index>` being the index of the blob
Line range hint
803-803
: Add a comma after "type" for better readability.- The `min` and `max` are `Namespace` type which is: + The `min` and `max` are `Namespace` type, which is:
Line range hint
1038-1038
: Remove the repeated word "data".- the block containing the Rollup data data root + the block containing the Rollup data root
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (15)
- .vitepress/config.ts (2 hunks)
- developers/blobstream-proof-queries.md (1 hunks)
- developers/blobstream-rollups.md (1 hunks)
- developers/blobstream-x-deploy.md (1 hunks)
- developers/blobstream-x-requesting-data-commitment-ranges.md (1 hunks)
- developers/bubs-testnet.md (3 hunks)
- developers/build-whatever.md (1 hunks)
- developers/celestia-node-key.md (1 hunks)
- developers/deploy-on-bubs.md (1 hunks)
- developers/ethereum-fallback.md (1 hunks)
- developers/intro-to-op-stack.md (2 hunks)
- developers/optimism-devnet.md (1 hunks)
- developers/optimism.md (6 hunks)
- developers/rollkit.md (1 hunks)
- developers/submit-data.md (1 hunks)
Files skipped from review due to trivial changes (7)
- .vitepress/config.ts
- developers/deploy-on-bubs.md
- developers/ethereum-fallback.md
- developers/intro-to-op-stack.md
- developers/optimism-devnet.md
- developers/optimism.md
- developers/rollkit.md
Additional comments not posted (1)
developers/blobstream-rollups.md (1)
3-8
: Navigation links added correctly.
[Bubs Testnet](https://bubs-sepolia.hub.caldera.xyz/) is a fresh offering from | ||
[Caldera](https://caldera.xyz) with support from Celestia Labs, | ||
built with OP Stack and Celestia, and is dedicated to providing developers with | ||
[Bubs Testnet](https://bubs-sepolia.hub.caldera.xyz/) is a the first |
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.
Correct the grammatical error by choosing either "a" or "the".
- [Bubs Testnet](https://bubs-sepolia.hub.caldera.xyz/) is a the first
+ [Bubs Testnet](https://bubs-sepolia.hub.caldera.xyz/) is the first
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
[Bubs Testnet](https://bubs-sepolia.hub.caldera.xyz/) is a the first | |
[Bubs Testnet](https://bubs-sepolia.hub.caldera.xyz/) is the first |
- [Deploy a smart contract on Bubs testnet](./deploy-on-bubs.md) | ||
- [Deploy a GM Portal dapp on Bubs testnet](./gm-portal-bubs.md) | ||
<!-- - [Deploy a smart contract with Thirdweb](https://thirdweb.com/bubs-testnet) --> | ||
capabilities. |
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.
Ensure the file ends with a single newline character.
capabilities.
+
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
capabilities. | |
capabilities. | |
Overview
Summary by CodeRabbit