feat: throw error when parsing unsupported share version #837
Conversation
| // intermediate state roots, or evidence. The returned [][]byte do not have | ||
| // namespaces, info bytes, data length delimiter, or unit length | ||
| // delimiters and are ready to be unmarshalled | ||
| func parseCompactShares(shares [][]byte) (data [][]byte, err error) { |
There was a problem hiding this comment.
[Non Blocking][Question]
Where is this actually called? I found it being used in func Merge(eds...), but I don't see this latter being used anywhere
There was a problem hiding this comment.
Where is this actually called?
- https://github.com/rootulp/celestia-app/blob/24ddfce6c6b523f4e0dc5de6dd780bc3ad5ed272/pkg/shares/share_merging.go#L80
- https://github.com/rootulp/celestia-app/blob/24ddfce6c6b523f4e0dc5de6dd780bc3ad5ed272/pkg/shares/share_merging.go#L80
which get called from func Merge(eds...) like you said. I also don't see Merge used anywhere outside tests. Couldn't find its usage in celestia-node either
There was a problem hiding this comment.
As far as I understand, this was moved from core to app as it was mainly used in app. And, this was done prior to non-interactive defaults as intended to be used by that.
I don't want to mention Evan as he's on vacation and if I mention him, he will respond :D
There was a problem hiding this comment.
Agreed. It seems non-blocking to find usage of this Merge(eds...) function so I think this can merge irrespective of investigating that further
There was a problem hiding this comment.
Should we create an issue for that so that we remmeber it? or just keep it in mind until Evan is back?
I am fine with any. Approooving
Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>
…#837) Closes celestiaorg#830 specifically: > Alternatively, if we don't want this to be a ProcessProposal validity rule, we can enforce this type of check when we parse the data square back into block data (e.g. [here](https://github.com/celestiaorg/celestia-app/blob/6b86e91eea1063c27c1ae461eddbe505f778df08/pkg/shares/parse_compact_shares.go#L14)) Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>
Closes #830 specifically: