-
Notifications
You must be signed in to change notification settings - Fork 43
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
api: Push
for data/shares with decoupled namespace
#55
Comments
Push
for data/shares with decoupled namespace
This is interesting. In the past the Push method used to have these already separated: Line 254 in 6e8a6a5
We can revert to that! Back then there was a lot of discussions around serialzation (of course there always is 🙈 ) and I think (not sure anymore) the type aliases and the API was original meant to be able to take in a type that defines its own serialzation. IMO, the tree should just take bytes anyways and serialzation is sth that should happen before and not inside the tree or anything like that. |
After discussing with @Wondertan, we realized that celestiaorg/celestia-node#183 can be tackled without modifying the NMT library at all (and instead by modifying this visitor). So we could put this on the back burner for now. I still think the suggested change makes sense. It would also allow us to delete the
|
Currently,
tree.Push
requires passingnamespace.PrefixedData
, which is a byte slice prependingnamespace.ID
. However, there is a use case where we don't need to push data that is not prepended with anamespace.ID
, and it can be passed as a separate field. This is the case where we push parity data generated via rsmt2d. Currently, we prepend parity namespace to the parity shares, which is not required. This is one of the issues causing us to store an additional 8 bytes of parity namespace per parity share via NMTWrapper, as outlined in celestiaorg/celestia-node#183. I propose adding an additional method likePushWith
to thenmt.Tree
to allow passing decoupled namespace from the share data to allow mentioned disk usage optimization.The text was updated successfully, but these errors were encountered: