Define a ParseShares API

[rootulp]

Context

celestia-node currently parses shares depending on the type of share (see here). Universal share encoding should enable us to expose one API where any type of share can be parsed. As a result, it may be possible to stop exporting

celestia-app/pkg/shares/share_merging.go

Line 125 in f67bb6b

func ParseMsgs(shares [][]byte) (coretypes.Messages, error) {

celestia-app/pkg/shares/share_merging.go

Line 78 in f67bb6b

func ParseTxs(shares [][]byte) (coretypes.Txs, error) {

celestia-app/pkg/shares/share_merging.go

Line 95 in f67bb6b

func ParseEvd(shares [][]byte) (coretypes.EvidenceData, error) {

[evan-forbes]

I still think we will need to export these functions, but also expose a ParseShares api. Perhaps something like this.

type ShareSet [][]byte

func ParseShares(shares [][]byte) []ShareSet

then we can parse depending on namespace

[rootulp]

1. Does this proposal imply an additional mechanism to convert from []ShareSet => coretypes.Messages, coretypes.Txs, or coretypes.EvidenceData?
2. Is the type ShareSet meant to describe that shares in this set are of a particular type. In other words, all share in this ShareSet are coretypes.Txs?

One alternative proposal that comes to mind:

func ParseShares(shares [][]byte) (coretypes.Messages, coretypes.Txs, coretypes.EvidenceData, error)

However this has the disadvantage that the return signature must be updated if/when we introduce more types.

[evan-forbes]

ParseShares should perform only the universal shares parsing logic, which means splitting up some set of data using only the namespace and the message start byte.

Now we have some universal and purposefully ambiguous (potentially called a ShareSet) data that we can do something with. If we're celestia-node, and we just want to serve some complete set of shares, then we can do that, if we actually want to parse the shares into a data structure, then we can do that using the namespace and the version portion of the info byte.

[evan-forbes]

If we are wanting to parse the entire square into block data, then we already have the Merge function to do that.

that being said, we can slightly refactor that function and simplify the merging logic now that we have this extra info.

[rootulp]

ParseShares should perform only the universal shares parsing logic, which means splitting up some set of data using only the namespace and the message start byte.

Agreed. How about?

// ShareSequence represents a contiguous sequence of shares that are part of the
// same namespace and message. For compact shares, one share sequence exists per
// reserved namespace. For sparse shares, one share sequence exists per message.
type ShareSequence struct {
	ID     namespace.ID
	Shares []Share
}

func ParseShares(shares [][]byte) []ShareSequence {
	// TODO
	return []ShareSequence{}
}

[evan-forbes]

docs are good btw.
I don't have any qualms with using word sequence over set, but why use NamespacedShares? isn't one of our goals to get rid of NamespaceShares? #721

In the past this has just felt like fighting the type system with middleware.

[rootulp]

sequence over set

Sounds good. I opted for sequence to indicate that the order of the shares in this struct are important and I think set conveys lack of ordering.

isn't one of our goals to get rid of NamespaceShares?

Sorry I forgot about #721 but just commented there. I'm not strongly opinionated on keeping vs removing the NamespaceShares type. If we remove, we can definitely remove NamespaceShares from the ShareSequence struct and use []Share instead.

[rootulp]

Opted to remove the NamespaceShares type so I updated the proposed API in #723 (comment)

The implementation of this API should split the provided shares into sequences based on the last bit in the info byte (a.k.a the message start indicator). It is possible to rename message start indicator to sequence start indicator given this API is public and "sequence start" is more accurate given this applies to non-message shares.