docs: explain why using tx-hash to build a blobstream rollup is not good #1644
Conversation
WalkthroughThe recent changes enhance the document with a new FAQ section addressing the use of Celestia transaction hashes for referencing rollup data. It outlines the complexities and inefficiencies of this approach, recommending alternative methods like the sequence of spans. By detailing the transaction verification process, the changes provide valuable insights that improve understanding of rollup data management. 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 Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (10)
developers/blobstream-rollups.md (10)
471-471: Break down long lines for better readability.The line exceeds the recommended length of 80 characters.
- A transaction proof in Celestia goes back to providing an inclusion proof of the shares containing the transaction. + A transaction proof in Celestia goes back to providing an inclusion proof + of the shares containing the transaction.Tools
Markdownlint
471-471: Expected: 80; Actual: 115
Line length(MD013, line-length)
472-472: Break down long lines for better readability.The line exceeds the recommended length of 80 characters.
- This means if the transaction hash is used to reference data in a Celestia block, the rollup verification mechanism should + This means if the transaction hash is used to reference data in a Celestia block, + the rollup verification mechanism shouldTools
Markdownlint
472-472: Expected: 80; Actual: 122
Line length(MD013, line-length)
475-475: Break down long lines for better readability.The line exceeds the recommended length of 80 characters.
- Verify an inclusion proof of the shares comprising the transaction up to the data root tuple root + Verify an inclusion proof of the shares comprising the transaction + up to the data root tuple rootTools
Markdownlint
475-475: Expected: 80; Actual: 99
Line length(MD013, line-length)
476-476: Break down long lines for better readability.The line exceeds the recommended length of 80 characters.
- Decode those shares and parse the transaction, then hash its components to generate the transaction hash + Decode those shares and parse the transaction, then hash its components + to generate the transaction hashTools
Markdownlint
476-476: Expected: 80; Actual: 106
Line length(MD013, line-length)
479-479: Break down long lines for better readability.The line exceeds the recommended length of 80 characters.
- At this level, the transaction hash is authenticated and the verification contract has the shares of the transaction. + At this level, the transaction hash is authenticated + and the verification contract has the shares of the transaction.Tools
LanguageTool
[uncategorized] ~479-~479: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...l, the transaction hash is authenticated and the verification contract has the share...(COMMA_COMPOUND_SENTENCE_2)
Markdownlint
479-479: Expected: 80; Actual: 90
Line length(MD013, line-length)
481-481: Break down long lines for better readability.The line exceeds the recommended length of 80 characters.
- Then, the verification contract needs to take the share commitment from the parsed transaction + Then, the verification contract needs to take the share commitment + from the parsed transactionTools
Markdownlint
481-481: Expected: 80; Actual: 94
Line length(MD013, line-length)
482-482: Break down long lines for better readability.The line exceeds the recommended length of 80 characters.
- and follow the steps outlined in the [blob share commitment](#blob-share-commitment) section. + and follow the steps outlined in the + [blob share commitment](#blob-share-commitment) section.Tools
Markdownlint
482-482: Expected: 80; Actual: 93
Line length(MD013, line-length)
485-485: Break down long lines for better readability.The line exceeds the recommended length of 80 characters.
- As observed, using the transaction hash is expensive and doesn't yield any advantages over using the blob share commitment, which in turn is more expensive than using the sequence of spans. + As observed, using the transaction hash is expensive and doesn't yield any advantages + over using the blob share commitment, which in turn is more expensive + than using the sequence of spans.Tools
Markdownlint
485-485: Expected: 80; Actual: 103
Line length(MD013, line-length)
487-487: Break down long lines for better readability.The line exceeds the recommended length of 80 characters.
- So, unless there are more reasons to use the transaction hash to reference the rollup data, the sequence of spans approach remains better. + So, unless there are more reasons to use the transaction hash to reference the rollup data, + the sequence of spans approach remains better.Tools
Markdownlint
487-487: Expected: 80; Actual: 107
Line length(MD013, line-length)
479-479: Add a comma before "and" to connect two independent clauses.Use a comma before “and” if it connects two independent clauses.
- At this level, the transaction hash is authenticated and the verification contract has the shares of the transaction. + At this level, the transaction hash is authenticated, and the verification contract has the shares of the transaction.Tools
LanguageTool
[uncategorized] ~479-~479: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...l, the transaction hash is authenticated and the verification contract has the share...(COMMA_COMPOUND_SENTENCE_2)
Markdownlint
479-479: Expected: 80; Actual: 90
Line length(MD013, line-length)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- developers/blobstream-rollups.md (1 hunks)
Additional context used
LanguageTool
developers/blobstream-rollups.md
[uncategorized] ~479-~479: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...l, the transaction hash is authenticated and the verification contract has the share...(COMMA_COMPOUND_SENTENCE_2)
Markdownlint
developers/blobstream-rollups.md
471-471: Expected: 80; Actual: 115
Line length(MD013, line-length)
472-472: Expected: 80; Actual: 122
Line length(MD013, line-length)
475-475: Expected: 80; Actual: 99
Line length(MD013, line-length)
476-476: Expected: 80; Actual: 106
Line length(MD013, line-length)
477-477: Expected: 80; Actual: 87
Line length(MD013, line-length)
479-479: Expected: 80; Actual: 90
Line length(MD013, line-length)
481-481: Expected: 80; Actual: 94
Line length(MD013, line-length)
482-482: Expected: 80; Actual: 93
Line length(MD013, line-length)
485-485: Expected: 80; Actual: 103
Line length(MD013, line-length)
487-487: Expected: 80; Actual: 107
Line length(MD013, line-length)
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (3)
developers/blobstream-rollups.md (3)
479-479: Add a comma before "and".Use a comma before “and” if it connects two independent clauses.
- At this level, the transaction hash is authenticated and the verification contract has the + At this level, the transaction hash is authenticated, and the verification contract has theTools
LanguageTool
[uncategorized] ~479-~479: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...l, the transaction hash is authenticated and the verification contract has the share...(COMMA_COMPOUND_SENTENCE_2)
Markdownlint
479-479: Expected: 80; Actual: 90
Line length(MD013, line-length)
471-472: Break long lines for better readability.Several lines exceed the recommended length of 80 characters. Consider breaking them into shorter lines.
- A transaction proof in Celestia goes back to providing an inclusion proof of the shares containing the transaction. + A transaction proof in Celestia goes back to providing an inclusion proof of the shares + containing the transaction. - This means if the transaction hash is used to reference data in a Celestia block, the rollup verification mechanism should + This means if the transaction hash is used to reference data in a Celestia block, the rollup + verification mechanism should - Verify an inclusion proof of the shares comprising the transaction up to the data root tuple root + Verify an inclusion proof of the shares comprising the transaction up to the data root + tuple root - Decode those shares and parse the transaction, then hash its components to generate the transaction hash + Decode those shares and parse the transaction, then hash its components to generate the + transaction hash - Verify that the generated transaction hash matches the one used to reference the data + Verify that the generated transaction hash matches the one used to reference the data - shares of the transaction. Then, the verification contract needs to take the share commitment from the parsed transaction + shares of the transaction. Then, the verification contract needs to take the share + commitment from the parsed transaction - and follow the steps outlined in the [blob share commitment](#blob-share-commitment) section. + and follow the steps outlined in the + [blob share commitment](#blob-share-commitment) section. - As observed, using the transaction hash is expensive and doesn't yield any advantages over using the blob share commitment, which in turn is more expensive than using the sequence of spans. + As observed, using the transaction hash is expensive and doesn't yield any advantages over + using the blob share commitment, which in turn is more expensive than using the sequence + of spans. - So, unless there are more reasons to use the transaction hash to reference the rollup data, the sequence of spans approach remains better. + So, unless there are more reasons to use the transaction hash to reference the rollup data, + the sequence of spans approach remains better.Also applies to: 475-477, 479-479, 481-482, 485-485, 487-487
Tools
Markdownlint
471-471: Expected: 80; Actual: 115
Line length(MD013, line-length)
472-472: Expected: 80; Actual: 122
Line length(MD013, line-length)
468-469: Clarify the statement.The sentence can be made clearer by explicitly stating what is not recommended.
- However, in Celestia, referencing the data using the transaction hash is not recommended. + However, in Celestia, referencing the rollup data using the transaction hash is not recommended.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- developers/blobstream-rollups.md (1 hunks)
Additional context used
LanguageTool
developers/blobstream-rollups.md
[uncategorized] ~479-~479: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...l, the transaction hash is authenticated and the verification contract has the share...(COMMA_COMPOUND_SENTENCE_2)
Markdownlint
developers/blobstream-rollups.md
471-471: Expected: 80; Actual: 115
Line length(MD013, line-length)
472-472: Expected: 80; Actual: 122
Line length(MD013, line-length)
475-475: Expected: 80; Actual: 99
Line length(MD013, line-length)
476-476: Expected: 80; Actual: 106
Line length(MD013, line-length)
477-477: Expected: 80; Actual: 87
Line length(MD013, line-length)
479-479: Expected: 80; Actual: 90
Line length(MD013, line-length)
481-481: Expected: 80; Actual: 94
Line length(MD013, line-length)
482-482: Expected: 80; Actual: 93
Line length(MD013, line-length)
485-485: Expected: 80; Actual: 103
Line length(MD013, line-length)
487-487: Expected: 80; Actual: 107
Line length(MD013, line-length)
Additional comments not posted (2)
developers/blobstream-rollups.md (2)
464-465: LGTM!The addition of the FAQ header is appropriate.
466-466: LGTM!The addition of the section header is appropriate.
Overview
Explain a frequent question of why we shouldn't build rollups using Blobstream and reference the data using a transaction hash
Summary by CodeRabbit