Skip to content

Commit

Permalink
fix: share shouldn't validate namespace versions
Browse files Browse the repository at this point in the history
  • Loading branch information
cmwaters committed Jul 26, 2024
1 parent 384e43d commit bdd4e62
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 97 deletions.
42 changes: 3 additions & 39 deletions share/compact_shares_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func TestCompactShareSplitter(t *testing.T) {
shares, err := css.Export()
require.NoError(t, err)

resTxs, err := parseCompactShares(shares, SupportedShareVersions)
resTxs, err := parseCompactShares(shares)
require.NoError(t, err)

assert.Equal(t, txs, resTxs)
Expand Down Expand Up @@ -80,7 +80,7 @@ func Test_processCompactShares(t *testing.T) {
shares, _, err := splitTxs(txs)
require.NoError(t, err)

parsedTxs, err := parseCompactShares(shares, SupportedShareVersions)
parsedTxs, err := parseCompactShares(shares)
if err != nil {
t.Error(err)
}
Expand All @@ -97,7 +97,7 @@ func Test_processCompactShares(t *testing.T) {

txShares, _, err := splitTxs(txs)
require.NoError(t, err)
parsedTxs, err := parseCompactShares(txShares, SupportedShareVersions)
parsedTxs, err := parseCompactShares(txShares)
if err != nil {
t.Error(err)
}
Expand Down Expand Up @@ -212,42 +212,6 @@ func TestContiguousCompactShareContainsInfoByte(t *testing.T) {
assert.Equal(t, byte(want), infoByte)
}

func Test_parseCompactSharesErrors(t *testing.T) {
type testCase struct {
name string
shares []Share
}

txs := generateRandomTxs(2, ContinuationCompactShareContentSize*4)
txShares, _, err := splitTxs(txs)
require.NoError(t, err)
rawShares := ToBytes(txShares)

unsupportedShareVersion := 5
infoByte, _ := NewInfoByte(uint8(unsupportedShareVersion), true)
shareWithUnsupportedShareVersionBytes := rawShares[0]
shareWithUnsupportedShareVersionBytes[NamespaceSize] = byte(infoByte)

shareWithUnsupportedShareVersion, err := NewShare(shareWithUnsupportedShareVersionBytes)
if err != nil {
t.Fatal(err)
}

testCases := []testCase{
{
"share with unsupported share version",
[]Share{*shareWithUnsupportedShareVersion},
},
}

for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
_, err := parseCompactShares(tt.shares, SupportedShareVersions)
assert.Error(t, err)
})
}
}

func generateRandomlySizedTxs(count, maxSize int) [][]byte {
txs := make([][]byte, count)
for i := 0; i < count; i++ {
Expand Down
14 changes: 10 additions & 4 deletions share/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ type Namespace struct {
data []byte
}

// NewNamespace returns a new namespace with the provided version and id.
// NewNamespace validates the provided version and id and returns a new namespace.
// This should be used for user specified namespaces.
func NewNamespace(version uint8, id []byte) (Namespace, error) {
if err := validate(version, id); err != nil {
if err := IsValidNamespace(version, id); err != nil {
return Namespace{}, err
}

Expand All @@ -38,11 +39,12 @@ func MustNewNamespace(version uint8, id []byte) Namespace {
}

// NewNamespaceFromBytes returns a new namespace from the provided byte slice.
// This is for user specified namespaces.
func NewNamespaceFromBytes(bytes []byte) (Namespace, error) {
if len(bytes) != NamespaceSize {
return Namespace{}, fmt.Errorf("invalid namespace length: %d. Must be %d bytes", len(bytes), NamespaceSize)
}
if err := validate(bytes[VersionIndex], bytes[NamespaceVersionSize:]); err != nil {
if err := IsValidNamespace(bytes[VersionIndex], bytes[NamespaceVersionSize:]); err != nil {
return Namespace{}, err
}

Expand Down Expand Up @@ -90,7 +92,11 @@ func (n Namespace) ID() []byte {
return n.data[NamespaceVersionSize:]
}

func validate(version uint8, id []byte) error {
// IsValidNamespace returns an error if the provided version is not
// supported or the provided id does not meet the requirements
// for the provided version. This should be user for validating
// user specified namespaces
func IsValidNamespace(version uint8, id []byte) error {
err := validateVersionSupported(version)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion share/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
// ParseTxs collects all of the transactions from the shares provided
func ParseTxs(shares []Share) ([][]byte, error) {
// parse the shares
rawTxs, err := parseCompactShares(shares, SupportedShareVersions)
rawTxs, err := parseCompactShares(shares)
if err != nil {
return nil, err
}
Expand Down
19 changes: 1 addition & 18 deletions share/parse_compact_shares.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,11 @@ package share
// an error is returned. The returned data [][]byte does not have namespaces,
// info bytes, data length delimiter, or unit length delimiters and are ready to
// be unmarshalled.
func parseCompactShares(shares []Share, supportedShareVersions []uint8) (data [][]byte, err error) {
func parseCompactShares(shares []Share) (data [][]byte, err error) {
if len(shares) == 0 {
return nil, nil
}

err = validateShareVersions(shares, supportedShareVersions)
if err != nil {
return nil, err
}

rawData, err := extractRawData(shares)
if err != nil {
return nil, err
Expand All @@ -29,18 +24,6 @@ func parseCompactShares(shares []Share, supportedShareVersions []uint8) (data []
return data, nil
}

// validateShareVersions returns an error if the shares contain a share with an
// unsupported share version. Returns nil if all shares contain supported share
// versions.
func validateShareVersions(shares []Share, supportedShareVersions []uint8) error {
for i := 0; i < len(shares); i++ {
if err := shares[i].DoesSupportVersions(supportedShareVersions); err != nil {
return err
}
}
return nil
}

// parseRawData returns the units (transactions, PFB transactions, intermediate
// state roots) contained in raw data by parsing the unit length delimiter
// prefixed to each unit.
Expand Down
32 changes: 0 additions & 32 deletions share/parse_sparse_shares_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,38 +91,6 @@ func Test_parseSparseShares(t *testing.T) {
}
}

func Test_parseSparseSharesErrors(t *testing.T) {
type testCase struct {
name string
shares []Share
}

unsupportedShareVersion := 5
infoByte, _ := NewInfoByte(uint8(unsupportedShareVersion), true)

rawShare := RandomNamespace().Bytes()
rawShare = append(rawShare, byte(infoByte))
rawShare = append(rawShare, bytes.Repeat([]byte{0}, ShareSize-len(rawShare))...)
share, err := NewShare(rawShare)
if err != nil {
t.Fatal(err)
}

tests := []testCase{
{
"share with unsupported share version",
[]Share{*share},
},
}

for _, tt := range tests {
t.Run(tt.name, func(*testing.T) {
_, err := parseSparseShares(tt.shares, SupportedShareVersions)
assert.Error(t, err)
})
}
}

func Test_parseSparseSharesWithNamespacedPadding(t *testing.T) {
sss := NewSparseShareSplitter()
randomSmallBlob := generateRandomBlob(ContinuationSparseShareContentSize / 2)
Expand Down
7 changes: 4 additions & 3 deletions share/shares.go → share/share.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@ func NewShare(data []byte) (*Share, error) {
if err := validateSize(data); err != nil {
return nil, err
}
if err := validate(data[0], data[1:NamespaceSize]); err != nil {
sh := &Share{data}
if err := sh.doesSupportVersions(SupportedShareVersions); err != nil {
return nil, err
}
return &Share{data}, nil
return sh, nil
}

func validateSize(data []byte) error {
Expand Down Expand Up @@ -54,7 +55,7 @@ func (s *Share) Version() uint8 {
}

// DoesSupportVersions checks if the share version is supported
func (s *Share) DoesSupportVersions(supportedShareVersions []uint8) error {
func (s *Share) doesSupportVersions(supportedShareVersions []uint8) error {
ver := s.Version()
if !bytes.Contains(supportedShareVersions, []byte{ver}) {
return fmt.Errorf("unsupported share version %v is not present in the list of supported share versions %v", ver, supportedShareVersions)
Expand Down
23 changes: 23 additions & 0 deletions share/shares_test.go → share/share_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,3 +214,26 @@ func TestIsPadding(t *testing.T) {
})
}
}

func TestUnsupportedShareVersion(t *testing.T) {
unsupportedShareVersion := 5
infoByte, _ := NewInfoByte(uint8(unsupportedShareVersion), true)

rawShare := RandomNamespace().Bytes()
rawShare = append(rawShare, byte(infoByte))
rawShare = append(rawShare, bytes.Repeat([]byte{0}, ShareSize-len(rawShare))...)
_, err := NewShare(rawShare)
require.Error(t, err)
}

func TestShareToBytesAndFromBytes(t *testing.T) {
blobs := []*Blob{generateRandomBlob(580), generateRandomBlob(380), generateRandomBlob(1100)}
SortBlobs(blobs)
shares, err := splitBlobs(blobs...)
require.NoError(t, err)

shareBytes := ToBytes(shares)
reconstructedShares, err := FromBytes(shareBytes)
require.NoError(t, err)
assert.Equal(t, shares, reconstructedShares)
}

0 comments on commit bdd4e62

Please sign in to comment.