Conversation
|
I decided to merge the whole spec into CIP instead of maintaining a separate doc. Having it externally seems cumbersome and prone to breaks. We would have to cross-link various required sections from CIP to the actual step. Instead, I will link celestia-node specs to CIP. In the worst case, we will copy the documents if that does not work for some reason. |
9a2ea7a to
c5424dd
Compare
|
After spending a few hours, I was able to nicely integrate the CIP into specs, so there is no need to do any copying |
|
sharing this on github as well, I think it would be good if we align this CIP and the discussion from rsmt2d . based on what i can tell, it already is |
|
Connecting previous discussion from celestiaorg/celestia-node#3205 |
jcstein
left a comment
There was a problem hiding this comment.
left some comments and committable suggestions
ebuchman
left a comment
There was a problem hiding this comment.
Just a first pass to clarify some things
| ```text | ||
| RowID { | ||
| Height: u64; | ||
| RowIndex: u16; |
There was a problem hiding this comment.
is this 16 bit limit enforced elsewhere in the core protocol?
There was a problem hiding this comment.
Not sure I understand the question.
There was a problem hiding this comment.
Oh I mean, is there a limit on how many rows there can be enforced somewhere else in the protocol?
There was a problem hiding this comment.
Ah not that I would be aware of. The u16 is chosen because it maxes at 2TB squares with 512shares, which is more than enough. Might be worth clarifying that as well in the spec.
There was a problem hiding this comment.
cc @rootulp, do we have any row number limits defined in core?
There was a problem hiding this comment.
Yea! The max row number is equivalent to 2 * the max effective square size because the row number seems applicable to the extended data square. The max effective square size is currently 64 but it could be bumped to 128 via a governance proposal.
Related table comparing original data square size and extended data square size: https://gist.github.com/rootulp/bbf10f6e9cf114816aaa994eb64b63a4
There was a problem hiding this comment.
leaving this discussion open to migrate into the forum
ac6e206 to
2d74d8c
Compare
rootulp
left a comment
There was a problem hiding this comment.
Re-reviewed the first half.
…re rules to the spec
Co-authored-by: Josh Stein <46639943+jcstein@users.noreply.github.com>
Co-authored-by: Rootul P <rootulp@gmail.com>
bc34ac3 to
0324e1b
Compare
|
the only remaining step is to link these 3 lines to the correct serialization heading, as there are now 2 different headings. before, there were 2 headings with the same content: "serialization" additional context, we cannot have a link that goes back to 2 headings. it just will not work. how would a link direct you to a heading, if there are 2 identical headings? |
Uh oh!
There was an error while loading. Please reload this page.