Skip to content

docs: apply review feedback to blobstream rollups#1541

Merged
jcstein merged 1 commit intomainfrom
review-comments
Apr 23, 2024
Merged

docs: apply review feedback to blobstream rollups#1541
jcstein merged 1 commit intomainfrom
review-comments

Conversation

@rach-id
Copy link
Copy Markdown
Member

@rach-id rach-id commented Apr 23, 2024

Overview

Applies PR review feedback.

Summary by CodeRabbit

  • Documentation
    • Updated the Blobstream rollups documentation to clarify the shift from "share commitment" to "blob share commitment" and the need for new tooling to support this approach.

@rach-id rach-id requested review from adlerjohn and jcstein April 23, 2024 18:31
@rach-id rach-id self-assigned this Apr 23, 2024
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 23, 2024

Walkthrough

The recent update transitions from using "share commitment" to "blob share commitment" for constructing Blobstream rollups. This change enhances the clarity and efficiency of rollup construction, signaling a shift in methodology and the need for new supporting tools.

Changes

File Path Change Summary
developers/.../blobstream-rollups.md Updated the construction method from "share commitment" to "blob share commitment" for Blobstream rollups.

🐇💻✨
In the realm of code, where the bits entwine,
A rabbit hopped through, leaving changes so fine.
From share to blob share, the commitments do shift,
In rollups they trust, through data they sift.
Hooray for the changes, the rabbit did cheer,
For cleaner, clearer paths now appear!


Recent Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between f89b01a and 4ad2c6c.
Files selected for processing (1)
  • developers/blobstream-rollups.md (11 hunks)
Additional Context Used
LanguageTool (221)
developers/blobstream-rollups.md (221)

Near line 5: Possible spelling mistake found.
Context: ... use Blobstream. --- # Introduction to Blobstream rollups [Blobstream](https://blog.cele...


Near line 5: Possible spelling mistake found.
Context: ...ream. --- # Introduction to Blobstream rollups [Blobstream](https://blog.celestia.org...


Near line 7: Possible spelling mistake found.
Context: ... # Introduction to Blobstream rollups [Blobstream](https://blog.celestia.org/introducing-...


Near line 7: Unpaired symbol: ‘[’ seems to be missing
Context: ...ction to Blobstream rollups [Blobstream](https://blog.celestia.org/introducing-b...


Near line 9: Possible spelling mistake found.
Context: ...les with the number of users. It allows rollups to post their data on Celestia while pr...


Near line 9: Possible spelling mistake found.
Context: ...It allows rollups to post their data on Celestia while proving their availability in the...


Near line 10: Possible spelling mistake found.
Context: ...while proving their availability in the rollup settlement contract. This document wil...


Near line 12: Possible spelling mistake found.
Context: ...tline a few ways to build optimistic or zk-rollups that post their data to Celestia and us...


Near line 13: Possible spelling mistake found.
Context: ...c or zk-rollups that post their data to Celestia and use Blobstream to prove that data's...


Near line 18: Possible spelling mistake found.
Context: ...constructs that can be used in building Blobstream rollups. Each with its pros and cons an...


Near line 18: Possible spelling mistake found.
Context: ...that can be used in building Blobstream rollups. Each with its pros and cons and the ro...


Near line 18: Possible missing comma found.
Context: ...bstream rollups. Each with its pros and cons and the rollup developer can choose whi...


Near line 18: Possible spelling mistake found.
Context: ...ps. Each with its pros and cons and the rollup developer can choose which one suits th...


Near line 20: Possible spelling mistake found.
Context: ...) method can be used currently to build Blobstream rollups. The [blob share commitment](#b...


Near line 20: Possible spelling mistake found.
Context: ...n be used currently to build Blobstream rollups. The [blob share commitment](#blob-shar...


Near line 26: Possible spelling mistake found.
Context: ...mitment over the data contained in the [MsgPayForBlobs transaction](https://github.com/celesti...


Near line 28: Possible spelling mistake found.
Context: ...g that the corresponding data exists on Celestia efficiently](https://github.com/celesti...


Near line 32: Possible spelling mistake found.
Context: ...o a blob share commitment was posted to Celestia using Blobstream, the following proofs ...


Near line 33: Possible spelling mistake found.
Context: ...commitment was posted to Celestia using Blobstream, the following proofs need to be verifi...


Near line 35: This sentence does not start with an uppercase letter.
Context: ...lowing proofs need to be verified: 1. [share inclusion proof to the blob share commi...


Near line 35: Unpaired symbol: ‘[’ seems to be missing
Context: ...usion proof to the blob share commitment](https://github.com/celestiaorg/celestia...


Near line 36: Possible spelling mistake found.
Context: ...itment-inclusion): meaning creating two merkle proofs: 1. share merkle proof up to ...


Near line 37: Possible spelling mistake found.
Context: ...creating two merkle proofs: 1. share merkle proof up to the [subtree root](https://...


Near line 39: Possible spelling mistake found.
Context: ...onding to that share 2. subtree root merkle proof to the [blob share commitment](ht...


Near line 41: Possible spelling mistake found.
Context: ...t-inclusion-to-data-root): meaning four merkle proofs: 1. [subtree roots merkle pro...


Near line 42: Possible spelling mistake found.
Context: ...our merkle proofs: 1. [subtree roots merkle proofs to the blob share commitment](ht...


Near line 42: Unpaired symbol: ‘[’ seems to be missing
Context: ...rkle proofs to the blob share commitment](https://github.com/celestiaorg/celestia...


Near line 44: Possible spelling mistake found.
Context: ...ee roots are valid 2. [subtree roots merkle proofs up to the row roots](https://git...


Near line 44: Unpaired symbol: ‘[’ seems to be missing
Context: ... roots merkle proofs up to the row roots](https://github.com/celestiaorg/celestia...


Near line 45: Possible spelling mistake found.
Context: ...ee roots belong to a set of rows in the Celestia block 3. [row roots proofs to the da...


Near line 46: Unpaired symbol: ‘[’ seems to be missing
Context: ... 3. [row roots proofs to the data root](https://github.com/celestiaorg/celestia...


Near line 47: Possible spelling mistake found.
Context: ... to prove that those rows belong to the Celestia Block 4. [data root tuple proof to t...


Near line 48: Unpaired symbol: ‘[’ seems to be missing
Context: ... root tuple proof to the data root tuple](https://docs.celestia.org/developers/bl...


Near line 49: Possible spelling mistake found.
Context: ...ommitment-scheme): to prove that the Celestia block referenced by its height and data...


Near line 50: Possible spelling mistake found.
Context: ...t and data root, was committed to by Blobstream. More details on the blob share commit...


Near line 58: Possible spelling mistake found.
Context: ...lob share commitment has been posted to Celestia. :::tip NOTE Generating/verifying blob...


Near line 70: Possible spelling mistake found.
Context: ... These require the ability to parse the protobuf encoded PFBs. In fact, if the rollup p...


Near line 72: Possible spelling mistake found.
Context: ...protobuf encoded PFBs. In fact, if the rollup project has a way to parse the protobuf...


Near line 72: Possible spelling mistake found.
Context: ...e rollup project has a way to parse the protobuf encoded PFB, either in a smart contract...


Near line 73: Possible spelling mistake found.
Context: ...ed PFB, either in a smart contract or a zk-circuit, they will be able to create compact pr...


Near line 74: Possible spelling mistake found.
Context: ...be able to create compact proofs of the rollup data. These proofs will work as follow...


Near line 79: Possible spelling mistake found.
Context: ... PFB commitment to the one saved in the rollup contract - Proving inclusion of the PFB...


Near line 80: Only proper nouns start with an uppercase character (there are exceptions for headlines).
Context: ... the one saved in the rollup contract - Proving inclusion of the PFB to the data root t...


Near line 80: Possible typo: you repeated a whitespace
Context: ...of the PFB to the data root tuple root. This will be a compact proof since we wi...


Near line 82: Possible spelling mistake found.
Context: ... shares regardless of the size of the rollup data. More details on compact proofs c...


Near line 89: Possible spelling mistake found.
Context: ...mmitment: Pros The pros of referencing rollup data using a blob share commitment: - ...


Near line 92: Possible spelling mistake found.
Context: ... to find another way of referencing the rollup data. - If the team has access to proto...


Near line 93: Possible spelling mistake found.
Context: ...ollup data. - If the team has access to protobuf parsing, it allows for compact proof, b...


Near line 98: Possible spelling mistake found.
Context: ... the case of having no way to parse the protobuf PFB encoding. - In the optimistic rollu...


Near line 100: Possible spelling mistake found.
Context: ...tobuf PFB encoding. - In the optimistic rollups construction, defined below, this requi...


Near line 101: Possible spelling mistake found.
Context: ...ed below, this requires waiting for the Celestia block to be committed to by Blobstream ...


Near line 102: Possible typo: you repeated a whitespace
Context: ...aving updating the settlement contract. This might require waiting for a few hou...


Near line 104: Possible spelling mistake found.
Context: ... on each chain, to finally submit the rollup update. Given these limitations, an al...


Near line 111: Possible spelling mistake found.
Context: ...pans An alternative way of referencing rollup data in the rollup settlement contract ...


Near line 111: Possible spelling mistake found.
Context: ...e way of referencing rollup data in the rollup settlement contract is using a sequence...


Near line 114: Possible spelling mistake found.
Context: ...ata pointer that allows pointing to the rollup data inside a Celestia square using its...


Near line 115: Possible spelling mistake found.
Context: ...ws pointing to the rollup data inside a Celestia square using its location inside the sq...


Near line 118: Loose punctuation mark.
Context: ...g the following information: - height: The height of the Celestia block contai...


Near line 118: Possible spelling mistake found.
Context: ...rmation: - height: The height of the Celestia block containing the rollup data. - `st...


Near line 118: Possible spelling mistake found.
Context: ...ht of the Celestia block containing the rollup data. - startIndex: The index of the ...


Near line 119: Loose punctuation mark.
Context: ...ntaining the rollup data. - startIndex: The index of the first share containing...


Near line 119: Possible spelling mistake found.
Context: ...index of the first share containing the rollup data. - dataLen: The number of shares...


Near line 120: Loose punctuation mark.
Context: ... containing the rollup data. - dataLen: The number of shares containing the rol...


Near line 120: Possible spelling mistake found.
Context: ...n: The number of shares containing the rollup data. The startIndexand thedataLe...


Near line 122: Possible spelling mistake found.
Context: ...shares containing the rollup data. The startIndex and the dataLen can be queried from ...


Near line 122: Possible spelling mistake found.
Context: ... rollup data. The startIndex and the dataLen can be queried from Celestia after the...


Near line 122: Possible spelling mistake found.
Context: ...and thedataLen` can be queried from Celestia after the corresponding transaction get...


Near line 126: Possible spelling mistake found.
Context: .../client/verify.go#L70-L85) command. The TxShareRange returns the start and end share of the...


Near line 130: Possible spelling mistake found.
Context: ...a transaction hash. :::tip NOTE If the rollup data is submitted in multiple blocks, t...


Near line 132: Possible spelling mistake found.
Context: ...ith the data only submitted to a single Celestia block. ::: #### Sequence of spans: Pro...


Near line 148: Possible spelling mistake found.
Context: ...refers to data that is not available on Celestia, it is necessary and sufficient to show...


Near line 150: Possible spelling mistake found.
Context: ...sequence of spans doesn't belong to the Celestia block, i.e., the span is out of bounds....


Near line 153: Possible spelling mistake found.
Context: ...ate this proof via generating a binary [Merkle proof](https://github.com/celestiaorg/c...


Near line 154: Possible spelling mistake found.
Context: ...of.go#L19-L31) of any row/column to the Celestia data root. This proof will provide the ...


Near line 158: Possible spelling mistake found.
Context: ...used to calculate the square size. The [computeSquareSizeFromRowProof](https://github.com/celestiaorg/blobstr...


Near line 159: Possible spelling mistake found.
Context: ...AVerifier.sol#L267-L300) method in the [DAVerifier](https://github.com/celestiaorg/blobstr...


Near line 164: Possible spelling mistake found.
Context: ...s. In order words, we will check if the startIndex and the startIndex + dataLen are inc...


Near line 164: Possible spelling mistake found.
Context: ... will check if the startIndex and the startIndex + dataLen are included in the range `[...


Near line 171: Possible spelling mistake found.
Context: ...For the data root, we will use a binary Merkle proof to prove its inclusion in a data ...


Near line 172: Possible spelling mistake found.
Context: ...tuple root that was committed to by the Blobstream smart contract. More on this in the [da...


Near line 180: Possible spelling mistake found.
Context: ...using a blob share commitment, an extra merkle proof is needed to prove inclusion of t...


Near line 188: Possible spelling mistake found.
Context: ...he sequence of spans, i.e., part of the rollup data is done as follows: 1. Prove that...


Near line 190: Possible spelling mistake found.
Context: ... data root tuple is committed to by the Blobstream smart contract: To prove the d...


Near line 193: Possible spelling mistake found.
Context: ...ve the data root is committed to by the Blobstream smart contract, we will need to pro...


Near line 194: Possible spelling mistake found.
Context: ...contract, we will need to provide a Merkle proof of the data root tuple to a data ...


Near line 200: Possible spelling mistake found.
Context: .... Verify inclusion proof of the data to Celestia data root: To prove that the data ...


Near line 203: Possible spelling mistake found.
Context: ... to provide two proofs: a namespace Merkle proof of the data to a row root. Th...


Near line 203: Possible typo: you repeated a whitespace
Context: ...Merkle proof of the data to a row root. This could be done via proving the share...


Near line 205: Possible spelling mistake found.
Context: ...a to the row root using a namespace Merkle proof. And, a binary Merkle proof o...


Near line 205: Possible typo: you repeated a whitespace
Context: ...ow root using a namespace Merkle proof. And, a binary Merkle proof of the row ro...


Near line 206: Possible spelling mistake found.
Context: ...mespace Merkle proof. And, a binary Merkle proof of the row root to the data root....


Near line 208: Possible spelling mistake found.
Context: ...These proofs can be generated using the [ProveShares](https://github.com/celestiaorg/celest...


Near line 217: Possible spelling mistake found.
Context: ... To prove that the data is part of the rollup sequence of spans, we take the auth...


Near line 221: Possible spelling mistake found.
Context: ...of to get the row index in the extended Celestia square and get the index of the sha...


Near line 232: Possible spelling mistake found.
Context: ...ure that the data/shares is part of the rollup data. #### Sequence of spans: Pros - ...


Near line 243: Possible spelling mistake found.
Context: ...nce of spans: Cons None ## Optimistic rollups One type of rollups that can be built ...


Near line 245: Possible spelling mistake found.
Context: ...one ## Optimistic rollups One type of rollups that can be built with Blobstream is op...


Near line 245: Possible spelling mistake found.
Context: ... be built with Blobstream is optimistic rollups. An optimistic rollup is a rollup that ...


Near line 246: Possible spelling mistake found.
Context: ...am is optimistic rollups. An optimistic rollup is a rollup that commits optimistically...


Near line 246: Possible spelling mistake found.
Context: ...stic rollups. An optimistic rollup is a rollup that commits optimistically to a set of...


Near line 249: Possible spelling mistake found.
Context: ...an create fraud proofs to signal that. Celestia allows optimistic rollups to post their...


Near line 250: Possible spelling mistake found.
Context: ...ignal that. Celestia allows optimistic rollups to post their data on its DA layer, and...


Near line 251: Possible spelling mistake found.
Context: ... prove that the data is available using Blobstream. To build an optimistic rollup that us...


Near line 253: Possible spelling mistake found.
Context: ...ing Blobstream. To build an optimistic rollup that uses Celestia as a DA layer, the f...


Near line 253: Possible spelling mistake found.
Context: ...To build an optimistic rollup that uses Celestia as a DA layer, the following constructi...


Near line 256: Possible spelling mistake found.
Context: ...ons can be inspired by. ### Optimistic rollups that use a sequence of spans Optimisti...


Near line 258: Possible spelling mistake found.
Context: ...hat use a sequence of spans Optimistic rollups can post their data in Celestia, then i...


Near line 258: Possible spelling mistake found.
Context: ...timistic rollups can post their data in Celestia, then in the rollup settlement contract...


Near line 258: Possible spelling mistake found.
Context: ...ost their data in Celestia, then in the rollup settlement contract, they can reference...


Near line 260: Possible spelling mistake found.
Context: ...ce of spans](#sequence-of-spans). Then, rollup full nodes can verify if that data is v...


Near line 264: Possible spelling mistake found.
Context: ... transitions fraud proofs (left for the rollup to define), goes back to the following ...


Near line 267: Possible spelling mistake found.
Context: ...he following cases: - Proving that the rollup data is unavailable: this goes back to ...


Near line 270: Possible spelling mistake found.
Context: ...his goes back to: - [Proving that the rollup data is available and is part of the se...


Near line 270: Unpaired symbol: ‘[’ seems to be missing
Context: ...ble and is part of the sequence of spans](#sequence-of-spans-proving-inclusion-of...


Near line 272: Possible spelling mistake found.
Context: ...invalid state transition: this is the rollup logic fraud proofs, and it's left to th...


Near line 272: Possible spelling mistake found.
Context: ...ogic fraud proofs, and it's left to the rollup to define. #### Optimistic rollups tha...


Near line 274: Possible spelling mistake found.
Context: ... the rollup to define. #### Optimistic rollups that use a sequence of spans: Pros - N...


Near line 277: Possible spelling mistake found.
Context: ...nt of submitting the commitments to the rollup settlement contracts - The fraud proofs...


Near line 279: Possible spelling mistake found.
Context: ...or example, a single transaction in the rollup data that was posted to Celestia is fau...


Near line 279: Possible spelling mistake found.
Context: ...n in the rollup data that was posted to Celestia is faulty, only the shares containing t...


Near line 283: Possible spelling mistake found.
Context: ...on chain and verified. #### Optimistic rollups that use a sequence of spans: Cons Non...


Near line 287: Possible spelling mistake found.
Context: ...e of spans: Cons None #### Optimistic rollups that use a sequence of spans: Example ...


Near line 289: Possible spelling mistake found.
Context: ...f spans: Example An example optimistic rollup that uses sequence of spans to referenc...


Near line 290: Possible spelling mistake found.
Context: ... reference its data can be found in the [RollupInclusionProofs](https://github.com/celestiaorg/blobst...


Near line 297: Possible spelling mistake found.
Context: ...queries) documentation. ### Optimistic rollups that use blob share commitments Anothe...


Near line 299: Possible spelling mistake found.
Context: ...are commitments Another way to build a rollup is to replace the sequence of spans wit...


Near line 301: Possible spelling mistake found.
Context: ...nd a blob share commitment. Then, users/rollup full nodes will be able to query that d...


Near line 302: Possible spelling mistake found.
Context: ...query that data and validate it. If the rollup data is not valid, they can create a fr...


Near line 306: Possible spelling mistake found.
Context: ...ed blob share commitment is part of the Celestia block, referenced by its height in the ...


Near line 307: Possible spelling mistake found.
Context: ... height in the moment of submitting the rollup commitments to the settlement contract....


Near line 308: Possible spelling mistake found.
Context: ...ake sure that the commitment is part of Celestia. Otherwise, rollup sequencers can commi...


Near line 309: Possible spelling mistake found.
Context: ...mitment is part of Celestia. Otherwise, rollup sequencers can commit to random blob sh...


Near line 316: Possible spelling mistake found.
Context: ...invalid state transition is part of the rollup data. Alternatively, the rollup settlem...


Near line 317: Possible spelling mistake found.
Context: ... of the rollup data. Alternatively, the rollup settlement contract would need to have ...


Near line 318: Possible spelling mistake found.
Context: ...t would need to have a library to parse protobuf encoded PFBs, as explained in the [comp...


Near line 321: Possible spelling mistake found.
Context: ...pensive proofs. The cost of parsing the protobuf is not included in this analysis and ne...


Near line 321: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...uf is not included in this analysis and needs to be investigated separately. #### Optim...


Near line 324: Possible spelling mistake found.
Context: ...vestigated separately. #### Optimistic rollups that use blob share commitments: Pros ...


Near line 326: Possible spelling mistake found.
Context: ...ob share commitment as the one saved in Celestia which gives access to existing tooling ...


Near line 326: Try using a synonym here to strengthen your writing.
Context: ...ment as the one saved in Celestia which gives access to existing tooling #### Optimi...


Near line 329: Possible spelling mistake found.
Context: ...ss to existing tooling #### Optimistic rollups that use blob share commitments: Cons ...


Near line 331: Possible typo: you repeated a whitespace
Context: ... proofs are expensive in the base case. And if the settlement contract is able t...


Near line 332: As a shorter alternative for ‘able to’, consider using “can”.
Context: ... case. And if the settlement contract is able to parse the PFBs, thorough investigatio...


Near line 333: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...nvestigations of the cost of that would need to be done. ## Zk-rollups Zk-rollups, ak...


Near line 335: Possible spelling mistake found.
Context: ...cost of that would need to be done. ## Zk-rollups Zk-rollups, aka validity rollups, can ...


Near line 336: Possible spelling mistake found.
Context: ... would need to be done. ## Zk-rollups Zk-rollups, aka validity rollups, can also use Cel...


Near line 337: Possible spelling mistake found.
Context: ...## Zk-rollups Zk-rollups, aka validity rollups, can also use Celestia as a DA and Blob...


Near line 337: Possible spelling mistake found.
Context: ...ups, aka validity rollups, can also use Celestia as a DA and Blobstream to verify that t...


Near line 343: Possible spelling mistake found.
Context: ...t. Similar to the optimistic case, the rollup settlement contract can reference the r...


Near line 343: Possible spelling mistake found.
Context: ...p settlement contract can reference the rollup data using either the sequence of spans...


Near line 347: Possible spelling mistake found.
Context: ...will discuss both in this section. ### Zk-rollups that use sequence of spans When submit...


Near line 349: Possible spelling mistake found.
Context: ... When submitting the commitments to the rollup settlement contract, this latter will n...


Near line 352: Possible spelling mistake found.
Context: ... will need to verify the following: 1. Zk-proof of the state transitions, which is left...


Near line 352: Possible spelling mistake found.
Context: ...tate transitions, which is left for the rollup to define. 2. Verify that the sequence ...


Near line 355: Possible spelling mistake found.
Context: ...lusion-proofs.md), i.e., is part of the Celestia block referenced by its height, as desc...


Near line 357: Possible spelling mistake found.
Context: ...nce-of-spans-proof-details) section. 3. Zk-proof of the rollup data to the data root. ...


Near line 357: Possible spelling mistake found.
Context: ...of-details) section. 3. Zk-proof of the rollup data to the data root. The verificat...


Near line 357: Possible typo: you repeated a whitespace
Context: ...of of the rollup data to the data root. The verification process of this should ...


Near line 359: Possible typo: you repeated a whitespace
Context: ...s the correct value that's being saved. The commitment can be the data root and ...


Near line 360: Possible typo: you repeated a whitespace
Context: ...he data root and the sequence of spans. And, when the rollup data is proven insi...


Near line 361: Possible spelling mistake found.
Context: ...the sequence of spans. And, when the rollup data is proven inside the circuit to th...


Near line 362: Possible typo: you repeated a whitespace
Context: ...a root is asserted to be the input one. Similarly, the data's location is assert...


Near line 364: Possible typo: you repeated a whitespace
Context: ...same as the input sequence of spans. These arguments are the ones used in the...


Near line 367: Possible spelling mistake found.
Context: ...ettlement contract can be sure that the rollup data was posted to Celestia, and the se...


Near line 368: Possible spelling mistake found.
Context: ...sure that the rollup data was posted to Celestia, and the sequence of spans references i...


Near line 370: Possible spelling mistake found.
Context: ...of spans references it correctly. #### Zk-rollups that use sequence of spans: Pros - The...


Near line 372: Possible spelling mistake found.
Context: ... Pros - The inclusion proof inside the zk-circuit is a simple proof that uses traditional...


Near line 373: Possible spelling mistake found.
Context: ...is a simple proof that uses traditional merkle tree. In the case of using blob share...


Near line 373: Possible typo: you repeated a whitespace
Context: ...roof that uses traditional merkle tree. In the case of using blob share commitme...


Near line 377: Possible spelling mistake found.
Context: ... expensive to prove are required. #### Zk-rollups that use sequence of spans: Cons None ...


Near line 381: Possible spelling mistake found.
Context: ... use sequence of spans: Cons None ### Zk-rollups that use blob share commitments To use...


Near line 383: Possible spelling mistake found.
Context: ...use blob share commitments to reference rollup data in the zk-rollup settlement contra...


Near line 383: Possible spelling mistake found.
Context: ...itments to reference rollup data in the zk-rollup settlement contract, the zk-circuits ne...


Near line 384: Possible spelling mistake found.
Context: ... the zk-rollup settlement contract, the zk-circuits need to be able to deserialize protobuf...


Near line 384: Possible spelling mistake found.
Context: ...circuits need to be able to deserialize protobuf encoded messages. Alternatively, more i...


Near line 385: Possible spelling mistake found.
Context: ... messages. Alternatively, more involved merkle proofs will need to be verified. #### ...


Near line 387: Possible spelling mistake found.
Context: ... proofs will need to be verified. #### Protobuf deserialization inside a zk-circuit On...


Near line 387: Possible spelling mistake found.
Context: ...ill need to be verified. #### Protobuf deserialization inside a zk-circuit One way of using t...


Near line 387: Possible spelling mistake found.
Context: ... #### Protobuf deserialization inside a zk-circuit One way of using the blob share commit...


Near line 389: Possible spelling mistake found.
Context: ... blob share commitment to reference the rollup data in zk-rollups is via using a proto...


Near line 389: Possible spelling mistake found.
Context: ...mitment to reference the rollup data in zk-rollups is via using a protobuf deserialization...


Near line 390: Possible spelling mistake found.
Context: ...ollup data in zk-rollups is via using a protobuf deserialization library inside the zk-c...


Near line 390: Possible spelling mistake found.
Context: ...a in zk-rollups is via using a protobuf deserialization library inside the zk-circuit. And the ...


Near line 390: Possible spelling mistake found.
Context: ...obuf deserialization library inside the zk-circuit. And the verification would proceed as ...


Near line 393: Possible spelling mistake found.
Context: ...ification would proceed as follows: 1. Zk-proof of the state transitions, which is left...


Near line 393: Possible spelling mistake found.
Context: ...state transitions, which is left to the rollup team to define. 2. Verify that the blob...


Near line 396: Possible spelling mistake found.
Context: ...mmitment-proof-details) section. 3. The zk-proof verifier would take as argument the dat...


Near line 396: Possible typo: you repeated a whitespace
Context: ...ata root and the blob share commitment. Then, inside the circuit, the protobuf e...


Near line 397: Possible spelling mistake found.
Context: ...tment. Then, inside the circuit, the protobuf encoded PFB transaction will be dese...


Near line 404: Possible spelling mistake found.
Context: ... If the above conditions are valid, the rollup settlement contract can be sure that th...


Near line 405: Possible spelling mistake found.
Context: ...ettlement contract can be sure that the rollup data was posted to Celestia and is corr...


Near line 405: Possible spelling mistake found.
Context: ...sure that the rollup data was posted to Celestia and is correctly referenced. #### Zk-r...


Near line 407: Possible spelling mistake found.
Context: ...stia and is correctly referenced. #### Zk-rollups that use blob share commitments: Pros ...


Near line 411: Possible spelling mistake found.
Context: ...lob share commitments: Pros None #### Zk-rollups that use blob share commitments: Cons ...


Near line 413: Possible spelling mistake found.
Context: ...is approach requires having access to a protobuf decoder inside a zk-circuit which is no...


Near line 413: Possible spelling mistake found.
Context: ...g access to a protobuf decoder inside a zk-circuit which is not straightforward to have. ...


Near line 414: Possible typo: you repeated a whitespace
Context: ...t which is not straightforward to have. Also, the relative costs will need to be...


Near line 417: Possible spelling mistake found.
Context: ...ill need to be investigated. ### Heavy merkle proofs usage Similar to [Protobuf dese...


Near line 419: Possible spelling mistake found.
Context: ... Heavy merkle proofs usage Similar to [Protobuf deserialization inside a zk-circuit](#p...


Near line 419: Possible spelling mistake found.
Context: ...rkle proofs usage Similar to [Protobuf deserialization inside a zk-circuit](#protobuf-deserial...


Near line 419: Possible spelling mistake found.
Context: ...r to [Protobuf deserialization inside a zk-circuit](#protobuf-deserialization-inside-a-zk-...


Near line 420: Possible spelling mistake found.
Context: ...serialization-inside-a-zk-circuit), the zk-circuit will proceed to the verification of the...


Near line 421: Possible spelling mistake found.
Context: ... is that instead of parsing the encoded protobuf, the proofs defined under the [blob sha...


Near line 423: Possible spelling mistake found.
Context: ...ion will need to be verified inside the zk-circuit as follows: 1. Zk-proof of the state t...


Near line 425: Possible spelling mistake found.
Context: ...d inside the zk-circuit as follows: 1. Zk-proof of the state transitions, which is left...


Near line 425: Possible spelling mistake found.
Context: ...state transitions, which is left to the rollup team to define. 2. Verify that the blob...


Near line 428: Possible spelling mistake found.
Context: ...mmitment-proof-details) section. 3. The zk-proof verifier would take as argument the dat...


Near line 428: Possible typo: you repeated a whitespace
Context: ...ata root and the blob share commitment. Then, inside the circuit: - It will ver...


Near line 431: Possible spelling mistake found.
Context: ...lob share commitment corresponds to the rollup data. - Verify that the input data root...


Near line 432: Possible typo: you repeated a whitespace
Context: ... commits to that blob share commitment. Check the [blob share commitment proof d...


Near line 436: Possible spelling mistake found.
Context: ...tails Once these proofs are valid, the rollup settlement contract can be sure that th...


Near line 436: Possible spelling mistake found.
Context: ...ettlement contract can be sure that the rollup data was posted to Celestia and is corr...


Near line 437: Possible spelling mistake found.
Context: ...sure that the rollup data was posted to Celestia and is correctly referenced. #### heav...


Near line 439: This sentence does not start with an uppercase letter.
Context: ...stia and is correctly referenced. #### heavy merkle proofs usage: Pros None #### h...


Near line 439: Possible spelling mistake found.
Context: ...nd is correctly referenced. #### heavy merkle proofs usage: Pros None #### heavy me...


Near line 443: Possible spelling mistake found.
Context: ...le proofs usage: Pros None #### heavy merkle proofs usage: Cons - More heavy usage ...


Near line 445: Possible spelling mistake found.
Context: ...oofs usage: Cons - More heavy usage of merkle proofs inside and outside the zk-circui...


Near line 445: Possible spelling mistake found.
Context: ...of merkle proofs inside and outside the zk-circuit. ## Conclusion Given the above detail...


Near line 451: Possible spelling mistake found.
Context: ...al case as explained in the [optimistic rollups that uses a sequence of spans](#optimis...


Near line 453: Possible spelling mistake found.
Context: ...lups-that-use-a-sequence-of-spans) and [zk-rollups that use sequence of spans](#zk-rollups...


Near line 455: Possible spelling mistake found.
Context: ...or greater flexibility. However, if the rollup team has different requirements, then t...


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 23, 2024

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-04-23 19:43 UTC

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Out of diff range and nitpick comments (10)
developers/blobstream-rollups.md (10)

20-20: The note about the current unavailability of the "blob share commitment" method should be highlighted or emphasized to ensure it catches the reader's attention.


61-61: Consider adding a projected timeline or a link to a roadmap for the development of the necessary tooling for generating/verifying blob share commitment proofs. This would provide clarity and manage expectations for the users.


Line range hint 102-102: There is an extra space before "This might require waiting...". It should be removed for consistency.

-   This might require waiting for a few hours, depending on the batches size
+  This might require waiting for a few hours, depending on the batches size

Line range hint 203-203: There is an extra space before "This could be done via proving the shares...". It should be removed to maintain consistent formatting.

-     This could be done via proving the shares that contain the data to the
+    This could be done via proving the shares that contain the data to the

Line range hint 205-205: There is an extra space before "And, a binary Merkle proof of the row root...". It should be removed for consistency.

-     And, a binary Merkle proof of the row root to the data root.
+    And, a binary Merkle proof of the row root to the data root.

Line range hint 357-357: There is an extra space before "The verification process of this should...". It should be removed to maintain consistent formatting.

-    The verification process of this should accept a commitment as input so that
+   The verification process of this should accept a commitment as input so that

Line range hint 359-359: There is an extra space before "The commitment can be the data root and...". It should be removed for consistency.

-    The commitment can be the data root and the sequence of spans.
+   The commitment can be the data root and the sequence of spans.

Line range hint 360-360: There is an extra space before "And, when the rollup data is proven inside...". It should be removed to maintain consistent formatting.

-    And, when the rollup data is proven inside the circuit to the data root,
+   And, when the rollup data is proven inside the circuit to the data root,

Line range hint 362-362: There is an extra space before "Similarly, the data's location is asserted...". It should be removed for consistency.

-    Similarly, the data's location is asserted to be the same as the input
+   Similarly, the data's location is asserted to be the same as the input

Line range hint 364-364: There is an extra space before "These arguments are the ones used in the...". It should be removed to maintain consistent formatting.

-    These arguments are the ones used in the sequence of spans verification in (2).
+   These arguments are the ones used in the sequence of spans verification in (2).

Comment thread developers/blobstream-rollups.md
Comment thread developers/blobstream-rollups.md
Comment thread developers/blobstream-rollups.md
Comment thread developers/blobstream-rollups.md
Comment thread developers/blobstream-rollups.md
Comment thread developers/blobstream-rollups.md
@jcstein jcstein merged commit 2fcf31b into main Apr 23, 2024
@jcstein jcstein deleted the review-comments branch April 23, 2024 19:43
Copy link
Copy Markdown
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants