chore: cherry-picks for v0.11.1 (#1119)

Cherry-pick two recent changes to the v0.11.x release branch so that we can create a v0.11.1 release and upgrade to that in celestia-node. `share.RawData()` is needed to complete the celestia-node side of celestiaorg/celestia-node#1448. The rename to sequence len was also cherry-picked to avoid merge conflicts Co-authored-by: Callum Waters <[email protected]>
celestiaorg · Dec 14, 2022 · a1eb594 · a1eb594
1 parent 281a352
commit a1eb594
Show file tree

Hide file tree

Showing 6 changed files with 259 additions and 38 deletions.
diff --git a/docs/architecture/adr-007-universal-share-prefix.md b/docs/architecture/adr-007-universal-share-prefix.md
@@ -36,10 +36,10 @@ The current share format poses multiple challenges:
 
 Introduce a universal share encoding that applies to both compact and sparse shares:
 
-- First share of sequence:<br>`namespace_id (8 bytes) | info (1 byte) | data length (varint 1 to 10 bytes) | data`
+- First share of sequence:<br>`namespace_id (8 bytes) | info (1 byte) | sequence length (varint 1 to 10 bytes) | data`
 - Contiguous share of sequence:<br>`namespace_id (8 bytes) | info (1 byte) | data`
 
-Compact shares have the added constraint: the first byte of `data` in each share is a reserved byte so the format is:<br>`namespace_id (8 bytes) | info (1 byte) | data length (varint 1 to 10 bytes) | reserved (1 byte) | data` and every unit in the compact share `data` is prefixed with a `unit length (varint 1 to 10 bytes)`.
+Compact shares have the added constraint: the first byte of `data` in each share is a reserved byte so the format is:<br>`namespace_id (8 bytes) | info (1 byte) | sequence length (varint 1 to 10 bytes) | reserved (1 byte) | data` and every unit in the compact share `data` is prefixed with a `unit length (varint 1 to 10 bytes)`.
 
 Where `info (1 byte)` is a byte with the following structure:
 

diff --git a/pkg/shares/share_merging.go b/pkg/shares/share_merging.go
@@ -209,7 +209,7 @@ func (s ShareSequence) validSequenceLength() error {
 // firstShare and returns the number of shares needed to store a sequence of
 // that length.
 func numberOfSharesNeeded(firstShare Share) (sharesUsed int, err error) {
-	sequenceLength, err := firstShare.SequenceLength()
+	sequenceLength, _, err := firstShare.SequenceLen()
 	if err != nil {
 		return 0, err
 	}

diff --git a/pkg/shares/shares.go b/pkg/shares/shares.go
@@ -28,26 +28,61 @@ func (s Share) NamespaceID() namespace.ID {
 
 func (s Share) InfoByte() (InfoByte, error) {
 	if len(s) < appconsts.NamespaceSize+appconsts.ShareInfoBytes {
-		panic(fmt.Sprintf("share %s is too short to contain an info byte", s))
+		return 0, fmt.Errorf("share %s is too short to contain an info byte", s)
 	}
 	// the info byte is the first byte after the namespace ID
 	unparsed := s[appconsts.NamespaceSize]
 	return ParseInfoByte(unparsed)
 }
 
-func (s Share) SequenceLength() (uint64, error) {
-	infoByte, err := s.InfoByte()
+// SequenceLen returns the value of the sequence length varint, the number of
+// bytes occupied by the sequence length varint, and optionally an error. It
+// returns 0, 0, nil if this is a continuation share (i.e. doesn't contain a
+// sequence length).
+func (s Share) SequenceLen() (len uint64, numBytes int, err error) {
+	isSequenceStart, err := s.isSequenceStart()
 	if err != nil {
-		return 0, err
-	}
-	if !infoByte.IsSequenceStart() {
-		return 0, fmt.Errorf("share %s is not a sequence start", s)
+		return 0, 0, err
 	}
-	if s.isCompactShare() && len(s) < appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareSequenceLengthBytes {
-		return 0, fmt.Errorf("compact share %s is too short to contain sequence length", s)
+	if !isSequenceStart {
+		return 0, 0, nil
 	}
+
 	reader := bytes.NewReader(s[appconsts.NamespaceSize+appconsts.ShareInfoBytes:])
-	return binary.ReadUvarint(reader)
+	len, err = binary.ReadUvarint(reader)
+	if err != nil {
+		return 0, 0, err
+	}
+
+	if s.isCompactShare() {
+		return len, appconsts.FirstCompactShareSequenceLengthBytes, nil
+	}
+	return len, numberOfBytesVarint(len), nil
+}
+
+// RawData returns the raw share data. The raw share data does not contain the
+// namespace ID, info byte, or sequence length. It does contain the reserved
+// bytes for compact shares.
+func (s Share) RawData() (rawData []byte, err error) {
+	_, numSequenceLengthBytes, err := s.SequenceLen()
+	if err != nil {
+		return rawData, err
+	}
+
+	rawDataStartIndex := appconsts.NamespaceSize + appconsts.ShareInfoBytes + numSequenceLengthBytes
+	if len(s) < rawDataStartIndex {
+		return rawData, fmt.Errorf("share %s is too short to contain raw data", s)
+	}
+
+	return s[rawDataStartIndex:], nil
+}
+
+func (s Share) isSequenceStart() (bool, error) {
+	infoByte, err := s.InfoByte()
+	if err != nil {
+		return false, err
+	}
+	return infoByte.IsSequenceStart(), nil
 }
 
 // isCompactShare returns true if this share is a compact share.
@@ -70,3 +105,9 @@ func FromBytes(bytes [][]byte) (shares []Share) {
 	}
 	return shares
 }
+
+// numberOfBytesVarint calculates the number of bytes needed to write a varint of n
+func numberOfBytesVarint(n uint64) (numberOfBytes int) {
+	buf := make([]byte, binary.MaxVarintLen64)
+	return binary.PutUvarint(buf, n)
+}
diff --git a/pkg/shares/shares_test.go b/pkg/shares/shares_test.go
@@ -318,3 +318,183 @@ func generateRandomMessage(size int) coretypes.Message {
 	}
 	return msg
 }
+
+func TestSequenceLen(t *testing.T) {
+	type testCase struct {
+		name         string
+		share        Share
+		wantLen      uint64
+		wantNumBytes int
+		wantErr      bool
+	}
+	firstShare := []byte{
+		1, 1, 1, 1, 1, 1, 1, 1, // namespace
+		1,           // info byte
+		10, 0, 0, 0, // sequence len
+		1, 2, 3, 4, 5, 6, 7, 8, 9, 10, // data
+	}
+	firstShareWithLongSequence := []byte{
+		1, 1, 1, 1, 1, 1, 1, 1, // namespace
+		1,            // info byte
+		195, 2, 0, 0, // sequence len
+	}
+	continuationShare := []byte{
+		1, 1, 1, 1, 1, 1, 1, 1, // namespace
+		0,  // info byte
+		10, // sequence len
+	}
+	compactShare := []byte{
+		0, 0, 0, 0, 0, 0, 0, 1, // namespace
+		1,           // info byte
+		10, 0, 0, 0, // sequence len
+	}
+	noInfoByte := []byte{
+		0, 0, 0, 0, 0, 0, 0, 1, // namespace
+	}
+	noSequenceLen := []byte{
+		0, 0, 0, 0, 0, 0, 0, 1, // namespace
+		1, // info byte
+	}
+	testCases := []testCase{
+		{
+			name:         "first share",
+			share:        firstShare,
+			wantLen:      10,
+			wantNumBytes: 1,
+			wantErr:      false,
+		},
+		{
+			name:         "first share with long sequence",
+			share:        firstShareWithLongSequence,
+			wantLen:      323,
+			wantNumBytes: 2,
+			wantErr:      false,
+		},
+		{
+			name:         "continuation share",
+			share:        continuationShare,
+			wantLen:      0,
+			wantNumBytes: 0,
+			wantErr:      false,
+		},
+		{
+			name:         "compact share",
+			share:        compactShare,
+			wantLen:      10,
+			wantNumBytes: 4,
+			wantErr:      false,
+		},
+		{
+			name:         "no info byte returns error",
+			share:        noInfoByte,
+			wantLen:      0,
+			wantNumBytes: 0,
+			wantErr:      true,
+		},
+		{
+			name:         "no sequence len returns error",
+			share:        noSequenceLen,
+			wantLen:      0,
+			wantNumBytes: 0,
+			wantErr:      true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			len, numBytes, err := tc.share.SequenceLen()
+
+			if tc.wantErr {
+				assert.Error(t, err)
+				return
+			}
+			assert.Equal(t, tc.wantLen, len)
+			assert.Equal(t, tc.wantNumBytes, numBytes)
+		})
+	}
+}
+
+func TestRawData(t *testing.T) {
+	type testCase struct {
+		name    string
+		share   Share
+		want    []byte
+		wantErr bool
+	}
+	firstSparseShare := []byte{
+		1, 1, 1, 1, 1, 1, 1, 1, // namespace
+		1,                             // info byte
+		10,                            // sequence len
+		1, 2, 3, 4, 5, 6, 7, 8, 9, 10, // data
+	}
+	continuationSparseShare := []byte{
+		1, 1, 1, 1, 1, 1, 1, 1, // namespace
+		0,                             // info byte
+		1, 2, 3, 4, 5, 6, 7, 8, 9, 10, // data
+	}
+	firstCompactShare := []byte{
+		0, 0, 0, 0, 0, 0, 0, 1, // namespace
+		1,           // info byte
+		10, 0, 0, 0, // sequence len
+		15, 0, // reserved bytes
+		1, 2, 3, 4, 5, 6, 7, 8, 9, 10, // data
+	}
+	continuationCompactShare := []byte{
+		0, 0, 0, 0, 0, 0, 0, 1, // namespace
+		0,    // info byte
+		0, 0, // reserved bytes
+		1, 2, 3, 4, 5, 6, 7, 8, 9, 10, // data
+	}
+	noSequenceLen := []byte{
+		0, 0, 0, 0, 0, 0, 0, 1, // namespace
+		1, // info byte
+	}
+	notEnoughSequenceLenBytes := []byte{
+		0, 0, 0, 0, 0, 0, 0, 1, // namespace
+		1,        // info byte
+		10, 0, 0, // sequence len
+	}
+	testCases := []testCase{
+		{
+			name:  "first sparse share",
+			share: firstSparseShare,
+			want:  []byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10},
+		},
+		{
+			name:  "continuation sparse share",
+			share: continuationSparseShare,
+			want:  []byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10},
+		},
+		{
+			name:  "first compact share",
+			share: firstCompactShare,
+			want:  []byte{15, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10},
+		},
+		{
+			name:  "continuation compact share",
+			share: continuationCompactShare,
+			want:  []byte{0, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10},
+		},
+		{
+			name:    "no sequence len returns error",
+			share:   noSequenceLen,
+			wantErr: true,
+		},
+		{
+			name:    "not enough sequence len bytes returns error",
+			share:   notEnoughSequenceLenBytes,
+			wantErr: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			rawData, err := tc.share.RawData()
+			if tc.wantErr {
+				assert.Error(t, err)
+				return
+			}
+			assert.Equal(t, tc.want, rawData)
+		})
+	}
+}
diff --git a/pkg/shares/split_compact_shares.go b/pkg/shares/split_compact_shares.go
@@ -28,12 +28,12 @@ func NewCompactShareSplitter(ns namespace.ID, version uint8) *CompactShareSplitt
 	if err != nil {
 		panic(err)
 	}
-	placeholderDataLength := make([]byte, appconsts.FirstCompactShareSequenceLengthBytes)
+	placeholderSequenceLen := make([]byte, appconsts.FirstCompactShareSequenceLengthBytes)
 	placeholderReservedBytes := make([]byte, appconsts.CompactShareReservedBytes)
 
 	pendingShare = append(pendingShare, ns...)
 	pendingShare = append(pendingShare, byte(infoByte))
-	pendingShare = append(pendingShare, placeholderDataLength...)
+	pendingShare = append(pendingShare, placeholderSequenceLen...)
 	pendingShare = append(pendingShare, placeholderReservedBytes...)
 	return &CompactShareSplitter{pendingShare: pendingShare, namespace: ns}
 }
@@ -123,35 +123,35 @@ func (css *CompactShareSplitter) Export() []Share {
 		css.shares = append(css.shares, css.pendingShare)
 	}
 
-	dataLengthVarint := css.dataLengthVarint(bytesOfPadding)
-	css.writeDataLengthVarintToFirstShare(dataLengthVarint)
+	sequenceLenVarint := css.sequenceLenVarint(bytesOfPadding)
+	css.writeSequenceLenVarintToFirstShare(sequenceLenVarint)
 	return css.shares
 }
 
-// dataLengthVarint returns a varint of the data length written to this compact
+// sequenceLenVarint returns a varint of the sequence length written to this compact
 // share splitter.
-func (css *CompactShareSplitter) dataLengthVarint(bytesOfPadding int) []byte {
+func (css *CompactShareSplitter) sequenceLenVarint(bytesOfPadding int) []byte {
 	if css.isEmpty() {
 		return []byte{}
 	}
 
-	// declare and initialize the data length
-	dataLengthVarint := make([]byte, appconsts.FirstCompactShareSequenceLengthBytes)
-	binary.PutUvarint(dataLengthVarint, css.dataLength(bytesOfPadding))
-	zeroPadIfNecessary(dataLengthVarint, appconsts.FirstCompactShareSequenceLengthBytes)
+	// declare and initialize the sequence length
+	sequenceLenVarint := make([]byte, appconsts.FirstCompactShareSequenceLengthBytes)
+	binary.PutUvarint(sequenceLenVarint, css.sequenceLen(bytesOfPadding))
+	zeroPadIfNecessary(sequenceLenVarint, appconsts.FirstCompactShareSequenceLengthBytes)
 
-	return dataLengthVarint
+	return sequenceLenVarint
 }
 
-func (css *CompactShareSplitter) writeDataLengthVarintToFirstShare(dataLengthVarint []byte) {
+func (css *CompactShareSplitter) writeSequenceLenVarintToFirstShare(sequenceLen []byte) {
 	if css.isEmpty() {
 		return
 	}
 
-	// write the data length varint to the first share
+	// write the sequence length varint to the first share
 	firstShare := css.shares[0]
 	for i := 0; i < appconsts.FirstCompactShareSequenceLengthBytes; i++ {
-		firstShare[appconsts.NamespaceSize+appconsts.ShareInfoBytes+i] = dataLengthVarint[i]
+		firstShare[appconsts.NamespaceSize+appconsts.ShareInfoBytes+i] = sequenceLen[i]
 	}
 
 	// replace existing first share with new first share
@@ -199,12 +199,12 @@ func (css *CompactShareSplitter) indexOfReservedBytes() int {
 	return appconsts.NamespaceSize + appconsts.ShareInfoBytes
 }
 
-// dataLength returns the total length in bytes of all units (transactions,
-// intermediate state roots, or evidence) written to this splitter.
-// dataLength does not include the # of bytes occupied by the namespace ID or
-// the share info byte in each share. dataLength does include the reserved
-// byte in each share and the unit length delimiter prefixed to each unit.
-func (css *CompactShareSplitter) dataLength(bytesOfPadding int) uint64 {
+// sequenceLen returns the total length in bytes of all units (transactions or
+// intermediate state roots) written to this splitter. sequenceLen does not
+// include the # of bytes occupied by the namespace ID or the share info byte in
+// each share. sequenceLen does include the reserved byte in each share and the
+// unit length delimiter prefixed to each unit.
+func (css *CompactShareSplitter) sequenceLen(bytesOfPadding int) uint64 {
 	if len(css.shares) == 0 {
 		return 0
 	}
@@ -213,8 +213,8 @@ func (css *CompactShareSplitter) dataLength(bytesOfPadding int) uint64 {
 	}
 
 	continuationSharesCount := len(css.shares) - 1
-	continuationSharesDataLength := uint64(continuationSharesCount) * appconsts.ContinuationCompactShareContentSize
-	return uint64(appconsts.FirstCompactShareContentSize) + continuationSharesDataLength - uint64(bytesOfPadding)
+	continuationSharesSequenceLen := uint64(continuationSharesCount) * appconsts.ContinuationCompactShareContentSize
+	return uint64(appconsts.FirstCompactShareContentSize) + continuationSharesSequenceLen - uint64(bytesOfPadding)
 }
 
 // isEmptyPendingShare returns true if the pending share is empty, false otherwise.

diff --git a/pkg/shares/split_sparse_shares.go b/pkg/shares/split_sparse_shares.go
@@ -39,14 +39,14 @@ func (sss *SparseShareSplitter) RemoveMessage(i int) (int, error) {
 	j := 1
 	initialCount := sss.count
 	if len(sss.shares) > i+1 {
-		msgLen, err := sss.shares[i+1].SequenceLength()
+		sequenceLen, _, err := sss.shares[i+1].SequenceLen()
 		if err != nil {
 			return 0, err
 		}
 		// 0 means that there is padding after the share that we are about to
 		// remove. to remove this padding, we increase j by 1
-		// with the message
-		if msgLen == 0 {
+		// with the blob
+		if sequenceLen == 0 {
 			j++
 			sss.count -= len(sss.shares[j])
 		}