You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
After discussing with @Wondertan, we realized that https://github.com/celestiaorg/celestia-node/issues/183 can be tackled without modifying the NMT library at all (and instead by modifying this [visitor](https://github.com/celestiaorg/celestia-node/blob/ed4cdf03f2bd35c30d5b9874d4bc40bea6f5a061/ipld/nmt_adder.go#L34)). 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 PrefixedData type entirely. But it might make sense to do this as part of a somewhat larger refactoring (can/should be split into several PRs) where we also tackle #15. In it's current form and because of the usage of this interface we still need to do a copy which merges the nid and the data. Hlib convinced me that it is better to hide this copying inside of the NMT though. Currently, it happens in the wrapper which is bad usability. Especially as Hlib put it:
The reason is; right now, we can avoid storing unnecessary data, but it's hacky. Meaning that on Push we need to prepend, and on Visit remove that we prepend so we don't store it
I still think the suggested change makes sense. It would also allow us to delete the
PrefixedData
type entirely. But it might make sense to do this as part of a somewhat larger refactoring (can/should be split into several PRs) where we also tackle #15. In it's current form and because of the usage of this interface we still need to do a copy which merges the nid and the data. Hlib convinced me that it is better to hide this copying inside of the NMT though. Currently, it happens in the wrapper which is bad usability. Especially as Hlib put it:Originally posted by @liamsi in #55 (comment)
The text was updated successfully, but these errors were encountered: