Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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: add block validity rules specs #1966
docs: add block validity rules specs #1966
Changes from 7 commits
134b2b4
0555466
f3a5470
51411fa
07c2678
8173c9c
b449608
d0329fd
069bbd7
8a5a7d1
f8660a7
b6ebd26
4cac7a4
c3e19fe
57380e7
f72be81
550e4f5
b36dcc4
89c03ac
88a3ac7
27209c8
9dca098
71cac1e
7ac6ec3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
[question][no change needed]
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.
BEFPs do, and blob inclusion will sooner or later. Not sure when we'll get state fraud proofs
this does bring up a good point, where I feel like all fraud proofs are actually block validity fraud proofs, and perhaps I should try to change the first paragraph to emphasize that
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.
[Suggestion] I see there is a missing link between the block validity rules and the supplied subsections. My suggestion revolves around making this link a bit clear (feel free to rephrase it as you see fit, I got inspiration from the subsections you already included in the specs):
Below is my suggested addition to the text:
In Celestia, the block validity rules encompass two main aspects: the rules governing the validity of individual transactions and the rules dictating how to construct the data hash.
Transaction validity rules: Similar to any normal block, Celestia blocks are composed of a set of transactions. Therefore, the transaction validity rules defined by CometBFT (I guess part of the rules should be in the app not just CometBFT, so please revise it the way you think is correct) form an integral part of the block validity rules in Celestia. (a reference to the appropriate documentation for the specific transaction validity rules.)
Blob transaction: The Blob transaction is unique to Celestia, and its validity rules are covered in the corresponding [specifications](either link the specs or a subsection in this spec). ( I am assuming this is something we need to explain separately, that is why I have put it as a separate item)
Data hash calculation steps: The following steps are specific to the calculation of the data hash:
Encoding/laying out transactions into a data square: Transactions within the block are serialized and organized into a square format. This process involves reorganizing the transactions, converting them into a series of bytes, splitting them into fixed-size shares, arranging them in a square structure, and erasure coding them. All these steps adhere to specific rules, which are also considered part of the block validity rules. (Links to the respective specifications, OR subsections of the current doc, should be provided for further details.)
Constructing the data hash: The data hash is computed from the data square mentioned earlier. Detailed information on the construction of the data hash can be found in the provided specifications OR a subsection in this doc. The correct construction of the data hash is also within the block validity rules.
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.
I like this a lot for the celestia specific rules and have updated accordingly. Note that I have simplified further by deleting a lot of the original descriptions and instead relying on the other portions of the spec that already cover it
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.
[Question] Can you please elaborate on the reason behind including this section as part of the "Block validity rule" specs? It didn't seem to touch on any validation rule? Thanks 🙏
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 good point. I guess this was just a section for block validity rules that were directly related to data availability
this point is usually glossed over in most blockchains, as it is assumed that the data is downloaded entirely in order to verify it
this section describes the difference between consensus nodes and light clients to emphasize that point
and then uses this preface for the reasoning behind the rest of the rules
do you think we should remove this header? While I'm less confident about the header, I do think the three paragraphs after the header are a useful preface
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.
I will attempt to address this when I address the above comment
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.
I understand your point, and it does make sense. I would recommend consolidating all the relevant background into a dedicated section, such as "Background" or "Introduction".
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.
Should these be part of this section. IMO blob inclusion and state fraud proofs are tangential to block validity rules
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.
that's a fair point moved all fraud proofs to their own section c3e19fe