diff --git a/pkg/shares/parse.go b/pkg/shares/parse.go index 13b0f38d1e..6181150bb0 100644 --- a/pkg/shares/parse.go +++ b/pkg/shares/parse.go @@ -35,7 +35,10 @@ func ParseBlobs(shares []Share) ([]coretypes.Blob, error) { return blobList, nil } -func ParseShares(shares []Share) ([]ShareSequence, error) { +// ParseShares parses the shares provided and returns a list of ShareSequences. +// If ignorePadding is true then the returned ShareSequences will not contain +// any padding sequences. +func ParseShares(shares []Share, ignorePadding bool) ([]ShareSequence, error) { sequences := []ShareSequence{} currentSequence := ShareSequence{} @@ -77,5 +80,17 @@ func ParseShares(shares []Share) ([]ShareSequence, error) { } } - return sequences, nil + result := []ShareSequence{} + for _, sequence := range sequences { + isPadding, err := sequence.isPadding() + if err != nil { + return nil, err + } + if ignorePadding && isPadding { + continue + } + result = append(result, sequence) + } + + return result, nil } diff --git a/pkg/shares/parse_test.go b/pkg/shares/parse_test.go index 6a0639e9ec..96eab2aec6 100644 --- a/pkg/shares/parse_test.go +++ b/pkg/shares/parse_test.go @@ -16,16 +16,8 @@ import ( ) func TestParseShares(t *testing.T) { - type testCase struct { - name string - shares []Share - want []ShareSequence - expectErr bool - } - - start := true ns1 := appns.MustNewV0(bytes.Repeat([]byte{1}, appns.NamespaceVersionZeroIDSize)) - namespaceTwo := appns.MustNewV0(bytes.Repeat([]byte{1}, appns.NamespaceVersionZeroIDSize)) + ns2 := appns.MustNewV0(bytes.Repeat([]byte{2}, appns.NamespaceVersionZeroIDSize)) txShares, _, _, err := SplitTxs(generateRandomTxs(2, 1000)) require.NoError(t, err) @@ -33,114 +25,180 @@ func TestParseShares(t *testing.T) { txShareContinuation := txShares[1] blobOneShares, err := SplitBlobs(0, []uint32{}, []types.Blob{generateRandomBlobWithNamespace(ns1, 1000)}, false) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) blobOneStart := blobOneShares[0] blobOneContinuation := blobOneShares[1] - blobTwoShares, err := SplitBlobs(0, []uint32{}, []types.Blob{generateRandomBlobWithNamespace(namespaceTwo, 1000)}, false) - if err != nil { - t.Fatal(err) - } + blobTwoShares, err := SplitBlobs(0, []uint32{}, []types.Blob{generateRandomBlobWithNamespace(ns2, 1000)}, false) + require.NoError(t, err) blobTwoStart := blobTwoShares[0] blobTwoContinuation := blobTwoShares[1] - invalidShare := Share{data: append(generateRawShare(ns1, start, 1), []byte{0}...)} // invalidShare is now longer than the length of a valid share + // invalidShare is longer than the length of a valid share + invalidShare := Share{data: append(generateRawShare(ns1, true, 1), []byte{0}...)} - largeSequenceLen := 1000 // it takes more than one share to store a sequence of 1000 bytes - oneShareWithTooLargeSequenceLen := generateRawShare(ns1, start, uint32(largeSequenceLen)) + // tooLargeSequenceLen is a single share with too large of a sequence len + // because it takes more than one share to store a sequence of 1000 bytes + tooLargeSequenceLen := generateRawShare(ns1, true, uint32(1000)) - shortSequenceLen := 0 - oneShareWithTooShortSequenceLen := generateRawShare(ns1, start, uint32(shortSequenceLen)) + ns1Padding, err := NamespacePaddingShare(ns1) + require.NoError(t, err) + + type testCase struct { + name string + shares []Share + ignorePadding bool + want []ShareSequence + expectErr bool + } tests := []testCase{ { - "empty", - []Share{}, - []ShareSequence{}, - false, + name: "empty", + shares: []Share{}, + ignorePadding: false, + want: []ShareSequence{}, + expectErr: false, }, { - "one transaction share", - []Share{txShareStart}, - []ShareSequence{{Namespace: appns.TxNamespace, Shares: []Share{txShareStart}}}, - false, + name: "one transaction share", + shares: []Share{txShareStart}, + ignorePadding: false, + want: []ShareSequence{{Namespace: appns.TxNamespace, Shares: []Share{txShareStart}}}, + expectErr: false, }, { - "two transaction shares", - []Share{txShareStart, txShareContinuation}, - []ShareSequence{{Namespace: appns.TxNamespace, Shares: []Share{txShareStart, txShareContinuation}}}, - false, + name: "two transaction shares", + shares: []Share{txShareStart, txShareContinuation}, + ignorePadding: false, + want: []ShareSequence{{Namespace: appns.TxNamespace, Shares: []Share{txShareStart, txShareContinuation}}}, + expectErr: false, }, { - "one blob share", - []Share{blobOneStart}, - []ShareSequence{{Namespace: ns1, Shares: []Share{blobOneStart}}}, - false, + name: "one blob share", + shares: []Share{blobOneStart}, + ignorePadding: false, + want: []ShareSequence{{Namespace: ns1, Shares: []Share{blobOneStart}}}, + expectErr: false, }, { - "two blob shares", - []Share{blobOneStart, blobOneContinuation}, - []ShareSequence{{Namespace: ns1, Shares: []Share{blobOneStart, blobOneContinuation}}}, - false, + name: "two blob shares", + shares: []Share{blobOneStart, blobOneContinuation}, + ignorePadding: false, + want: []ShareSequence{{Namespace: ns1, Shares: []Share{blobOneStart, blobOneContinuation}}}, + expectErr: false, }, { - "two blobs with two shares each", - []Share{blobOneStart, blobOneContinuation, blobTwoStart, blobTwoContinuation}, - []ShareSequence{ + name: "two blobs with two shares each", + shares: []Share{blobOneStart, blobOneContinuation, blobTwoStart, blobTwoContinuation}, + ignorePadding: false, + want: []ShareSequence{ {Namespace: ns1, Shares: []Share{blobOneStart, blobOneContinuation}}, - {Namespace: namespaceTwo, Shares: []Share{blobTwoStart, blobTwoContinuation}}, + {Namespace: ns2, Shares: []Share{blobTwoStart, blobTwoContinuation}}, }, - false, + expectErr: false, }, { - "one transaction, one blob", - []Share{txShareStart, blobOneStart}, - []ShareSequence{ + name: "one transaction, one blob", + shares: []Share{txShareStart, blobOneStart}, + ignorePadding: false, + want: []ShareSequence{ {Namespace: appns.TxNamespace, Shares: []Share{txShareStart}}, {Namespace: ns1, Shares: []Share{blobOneStart}}, }, - false, + expectErr: false, }, { - "one transaction, two blobs", - []Share{txShareStart, blobOneStart, blobTwoStart}, - []ShareSequence{ + name: "one transaction, two blobs", + shares: []Share{txShareStart, blobOneStart, blobTwoStart}, + ignorePadding: false, + want: []ShareSequence{ {Namespace: appns.TxNamespace, Shares: []Share{txShareStart}}, {Namespace: ns1, Shares: []Share{blobOneStart}}, - {Namespace: namespaceTwo, Shares: []Share{blobTwoStart}}, + {Namespace: ns2, Shares: []Share{blobTwoStart}}, }, - false, + expectErr: false, + }, + { + name: "one share with invalid size", + shares: []Share{invalidShare}, + ignorePadding: false, + want: []ShareSequence{}, + expectErr: true, + }, + { + name: "blob one start followed by blob two continuation", + shares: []Share{blobOneStart, blobTwoContinuation}, + ignorePadding: false, + want: []ShareSequence{}, + expectErr: true, }, { - "one share with invalid size", - []Share{invalidShare}, - []ShareSequence{}, - true, + name: "one share with too large sequence length", + shares: []Share{{data: tooLargeSequenceLen}}, + ignorePadding: false, + want: []ShareSequence{}, + expectErr: true, }, { - "blob one start followed by blob two continuation", - []Share{blobOneStart, blobTwoContinuation}, - []ShareSequence{}, - true, + name: "tail padding shares", + shares: TailPaddingShares(2), + ignorePadding: false, + want: []ShareSequence{ + { + Namespace: appns.TailPaddingNamespace, + Shares: []Share{TailPaddingShare()}, + }, + { + Namespace: appns.TailPaddingNamespace, + Shares: []Share{TailPaddingShare()}, + }, + }, + expectErr: false, }, { - "one share with too large sequence length", - []Share{{data: oneShareWithTooLargeSequenceLen}}, - []ShareSequence{}, - true, + name: "reserve padding shares", + shares: ReservedPaddingShares(2), + ignorePadding: false, + want: []ShareSequence{ + { + Namespace: appns.ReservedPaddingNamespace, + Shares: []Share{ReservedPaddingShare()}, + }, + { + Namespace: appns.ReservedPaddingNamespace, + Shares: []Share{ReservedPaddingShare()}, + }, + }, + expectErr: false, + }, + { + name: "namespace padding shares", + shares: []Share{ns1Padding, ns1Padding}, + ignorePadding: false, + want: []ShareSequence{ + { + Namespace: ns1, + Shares: []Share{ns1Padding}, + }, + { + Namespace: ns1, + Shares: []Share{ns1Padding}, + }, + }, + expectErr: false, }, { - "one share with too short sequence length", - []Share{{data: oneShareWithTooShortSequenceLen}}, - []ShareSequence{}, - true, + name: "ignores all types of padding shares", + shares: []Share{TailPaddingShare(), ReservedPaddingShare(), ns1Padding}, + ignorePadding: true, + want: []ShareSequence{}, + expectErr: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := ParseShares(tt.shares) + got, err := ParseShares(tt.shares, tt.ignorePadding) if tt.expectErr { assert.Error(t, err) return diff --git a/pkg/shares/share_sequence.go b/pkg/shares/share_sequence.go index bcf5c51250..939f807ed4 100644 --- a/pkg/shares/share_sequence.go +++ b/pkg/shares/share_sequence.go @@ -50,6 +50,14 @@ func (s ShareSequence) validSequenceLen() error { if len(s.Shares) == 0 { return fmt.Errorf("invalid sequence length because share sequence %v has no shares", s) } + isPadding, err := s.isPadding() + if err != nil { + return err + } + if isPadding { + return nil + } + firstShare := s.Shares[0] sharesNeeded, err := numberOfSharesNeeded(firstShare) if err != nil { @@ -62,6 +70,17 @@ func (s ShareSequence) validSequenceLen() error { return nil } +func (s ShareSequence) isPadding() (bool, error) { + if len(s.Shares) != 1 { + return false, nil + } + isPadding, err := s.Shares[0].IsPadding() + if err != nil { + return false, err + } + return isPadding, nil +} + // numberOfSharesNeeded extracts the sequenceLen written to the share // firstShare and returns the number of shares needed to store a sequence of // that length.