Skip to content
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

Consider treating namespaced padding as blobs or sequences #783

Closed
evan-forbes opened this issue Sep 24, 2022 · 5 comments · Fixed by #1341
Closed

Consider treating namespaced padding as blobs or sequences #783

evan-forbes opened this issue Sep 24, 2022 · 5 comments · Fixed by #1341
Assignees

Comments

@evan-forbes
Copy link
Member

evan-forbes commented Sep 24, 2022

With the non-interactive defaults, we introduced padding between messages. Per the original specs Caveats section of Message Layout Rational, these are described as

[Namespaced padded shares] have a value of zero and a namespace ID equal to the preceding message's. Since their value is known, they can be omitted from NMT proofs of all shares of a given namespace ID.

We should consider updating the implementation and specs to treat the each section of padding as individual messages. This will prepend an info byte along with a length delimiter.

edit: While the added bytes would break the second property,

Since their value is known, they can be omitted from NMT proofs of all shares of a given namespace ID.

this might mean that light clients could skip these shares entirely when parsing a namespace.

@musalbas
Copy link
Member

musalbas commented Sep 24, 2022

Afaik this would only break the second property for the first padding share, as it has different message lengths. However, the length of the padding should also be transmitted in the p2p layer regardless of this change, so I think it doesn't break the second property at all.

@evan-forbes evan-forbes changed the title Consider treating namespaced padding as messages Consider treating namespaced padding as blobs or sequences Jan 18, 2023
@rootulp
Copy link
Collaborator

rootulp commented Feb 3, 2023

this might mean that light clients could skip these shares entirely when parsing a namespace.

In order for this to be the case, additional metadata must be added to the share so that a client can differentiate between a sequence of namespaced padding shares and a sequence of shares with 0 data. See #1136

@musalbas
Copy link
Member

musalbas commented Feb 6, 2023

Assuming that all padding shares have a start indicator of 1, and data len of 0, then shouldn't you be able to differentiate padding shares from non-padding shares, because only padding shares can have a start indicator of 1 and data len of 0?

@rootulp
Copy link
Collaborator

rootulp commented Feb 6, 2023

Good point. celestia-app denies blob sizes of 0 here so it is possible to treat a share with a sequence start indicator of 1 and a sequence length of 0 as a namespaced padding share.

Pros:

  • an additional reserved bit is not needed

Cons:

  • prevents us from writing a sequence length for the number of padding bytes that follow to the first namespaced padded share. I briefly considered that option so that clients could read the first namespaced padded share and learn how many padding shares followed and potentially skip downloading them. An alternative optimization with similar benefit may be possible because a namespaced padded share has a variable namespace id and constant contents.

rootulp added a commit that referenced this issue Feb 7, 2023
Implements the proposal in
#783 (comment).

Closes #1136
Closes #783
Opens #1344

---------

Co-authored-by: Callum Waters <cmwaters19@gmail.com>
@musalbas
Copy link
Member

musalbas commented Feb 7, 2023

prevents us from writing a sequence length for the number of padding bytes that follow to the first namespaced padded share. I briefly considered that option so that clients could read the first namespaced padded share and learn how many padding shares followed and potentially skip downloading them. An alternative optimization with similar benefit may be possible because a namespaced padded share has a variable namespace id and constant contents.

I think they can learn that using the method described in: #1136 (comment) (cc @adlerjohn)

Consider a model where a full storage node does not send padding shares to a rollup light node over the wire, but simply tells them "hey, shares between index x to y are padding, and here's the sub-tree root of those shares". The light node can then recompute the sub-tree root of those padding shares itself, to authenticate that statement.

@rootulp rootulp self-assigned this Feb 7, 2023
evan-forbes pushed a commit that referenced this issue Feb 27, 2023
Implements the proposal in
#783 (comment).

Closes #1136
Closes #783
Opens #1344

---------

Co-authored-by: Callum Waters <cmwaters19@gmail.com>
cmwaters added a commit to celestiaorg/go-square that referenced this issue Dec 14, 2023
0xchainlover pushed a commit to celestia-org/celestia-app that referenced this issue Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants