feat!: max namespace version for parity namespace (#1590)

Closes #1587 Motivated by an effort to bump celestia-node's dependency on celestia-app. Will cut a new v0.14.x RC after this merges and also cherry-pick this to main. Can split this into multiple PRs if requested. 1. Export `NewShare` 2. Modify parity namespace to use the max namespace version so that the NMT feature `ignoreMaxNamespace` works as expected Follow-ups: - #1610 (not blocking the upgrade so spinning out)
celestiaorg · Apr 13, 2023 · 0a7a0a4 · 0a7a0a4
1 parent d916dfe
commit 0a7a0a4
Show file tree

Hide file tree

Showing 13 changed files with 49 additions and 18 deletions.
diff --git a/pkg/da/data_availability_header_test.go b/pkg/da/data_availability_header_test.go
@@ -26,7 +26,7 @@ func TestNilDataAvailabilityHeaderHashDoesntCrash(t *testing.T) {
 
 func TestMinDataAvailabilityHeader(t *testing.T) {
 	dah := MinDataAvailabilityHeader()
-	expectedHash := []byte{0xad, 0x23, 0x6a, 0x2e, 0x4c, 0x5f, 0xca, 0x6c, 0xdb, 0xae, 0x5d, 0x5e, 0xdf, 0x79, 0xe8, 0x8e, 0x84, 0xc5, 0x2e, 0xed, 0x62, 0xeb, 0xd0, 0xb6, 0x5d, 0x18, 0xb2, 0x7c, 0x32, 0xa8, 0xbc, 0x58}
+	expectedHash := []byte{0xd3, 0x27, 0xf4, 0x4e, 0x52, 0x55, 0x21, 0xa6, 0x1b, 0x83, 0x65, 0x20, 0xd1, 0x96, 0xa2, 0xd2, 0x2c, 0x1d, 0x48, 0x67, 0xdb, 0xde, 0x3b, 0x1, 0xb, 0xfb, 0xa0, 0xef, 0x76, 0xed, 0x30, 0x27}
 	require.Equal(t, expectedHash, dah.hash)
 	require.NoError(t, dah.ValidateBasic())
 }
@@ -42,13 +42,13 @@ func TestNewDataAvailabilityHeader(t *testing.T) {
 	tests := []test{
 		{
 			name:         "typical",
-			expectedHash: []byte{0xeb, 0xfd, 0xb5, 0xc5, 0x52, 0x59, 0xd6, 0xe, 0x72, 0x6b, 0xde, 0x58, 0x7, 0x9a, 0x58, 0xd2, 0x18, 0x7b, 0xc9, 0x44, 0x7, 0x6e, 0xbe, 0x74, 0x47, 0x67, 0x45, 0xa3, 0xb7, 0x3a, 0x52, 0x47},
+			expectedHash: []byte{0x5b, 0x27, 0x3e, 0x3a, 0x5d, 0x9e, 0x90, 0x25, 0x58, 0x21, 0xb7, 0xe0, 0x4d, 0x4b, 0xaa, 0xde, 0x37, 0xa6, 0x6f, 0xcc, 0xd, 0x16, 0x6f, 0x9e, 0xe0, 0x7f, 0xbe, 0x8, 0xb4, 0x41, 0xc8, 0xa6},
 			squareSize:   2,
 			shares:       generateShares(4),
 		},
 		{
 			name:         "max square size",
-			expectedHash: []byte{0x48, 0x28, 0xa9, 0xef, 0x79, 0xc2, 0x12, 0x12, 0xc, 0x53, 0x83, 0x27, 0x55, 0x7d, 0x42, 0xdd, 0x64, 0x74, 0xad, 0x4e, 0x82, 0xcb, 0xa0, 0x43, 0xed, 0x14, 0x2, 0x54, 0x0, 0x3b, 0xf6, 0x11},
+			expectedHash: []byte{0xce, 0x5c, 0xf3, 0xc9, 0x15, 0xeb, 0xbf, 0xb0, 0x67, 0xe1, 0xa5, 0x97, 0x35, 0xf3, 0x25, 0x7b, 0x1c, 0x47, 0x74, 0x1f, 0xec, 0x6a, 0x33, 0x19, 0x7f, 0x8f, 0xc2, 0x4a, 0xe, 0xe2, 0xbe, 0x73},
 			squareSize:   appconsts.DefaultMaxSquareSize,
 			shares:       generateShares(appconsts.DefaultMaxSquareSize * appconsts.DefaultMaxSquareSize),
 		},

diff --git a/pkg/namespace/consts.go b/pkg/namespace/consts.go
@@ -2,6 +2,7 @@ package namespace
 
 import (
 	"bytes"
+	"math"
 
 	"github.com/celestiaorg/celestia-app/pkg/appconsts"
 )
@@ -59,5 +60,8 @@ var (
 	TailPaddingNamespace = MustNewV0([]byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFE})
 
 	// ParitySharesNamespace is the namespace reserved for erasure coded data.
-	ParitySharesNamespace = MustNewV0([]byte{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF})
+	ParitySharesNamespace = Namespace{
+		Version: math.MaxUint8,
+		ID:      bytes.Repeat([]byte{0xFF}, NamespaceIDSize),
+	}
 )
diff --git a/pkg/namespace/random_namespace.go b/pkg/namespace/random_namespace.go
@@ -0,0 +1,18 @@
+package namespace
+
+import tmrand "github.com/tendermint/tendermint/libs/rand"
+
+func RandomNamespace() Namespace {
+	for {
+		id := RandomVerzionZeroID()
+		namespace, err := New(NamespaceVersionZero, id)
+		if err != nil {
+			continue
+		}
+		return namespace
+	}
+}
+
+func RandomVerzionZeroID() []byte {
+	return append(NamespaceVersionZeroPrefix, tmrand.Bytes(NamespaceVersionZeroIDSize)...)
+}
diff --git a/pkg/proof/proof.go b/pkg/proof/proof.go
@@ -136,7 +136,11 @@ func NewShareInclusionProof(
 	// get the extended rows containing the shares.
 	rows := make([][]shares.Share, endRow-startRow+1)
 	for i := startRow; i <= endRow; i++ {
-		rows[i-startRow] = shares.FromBytes(eds.Row(uint(i)))
+		shares, err := shares.FromBytes(eds.Row(uint(i)))
+		if err != nil {
+			return types.ShareProof{}, err
+		}
+		rows[i-startRow] = shares
 	}
 
 	var shareProofs []*tmproto.NMTProof //nolint:prealloc

diff --git a/pkg/shares/compact_shares_test.go b/pkg/shares/compact_shares_test.go
@@ -172,7 +172,7 @@ func Test_parseCompactSharesErrors(t *testing.T) {
 	shareWithUnsupportedShareVersionBytes := rawShares[0]
 	shareWithUnsupportedShareVersionBytes[appconsts.NamespaceSize] = byte(infoByte)
 
-	shareWithUnsupportedShareVersion, err := newShare(shareWithUnsupportedShareVersionBytes)
+	shareWithUnsupportedShareVersion, err := NewShare(shareWithUnsupportedShareVersionBytes)
 	if err != nil {
 		t.Fatal(err)
 	}

diff --git a/pkg/shares/parse_sparse_shares_test.go b/pkg/shares/parse_sparse_shares_test.go
@@ -105,7 +105,7 @@ func Test_parseSparseSharesErrors(t *testing.T) {
 	rawShare = append(rawShare, namespace.ID{1, 1, 1, 1, 1, 1, 1, 1}...)
 	rawShare = append(rawShare, byte(infoByte))
 	rawShare = append(rawShare, bytes.Repeat([]byte{0}, appconsts.ShareSize-len(rawShare))...)
-	share, err := newShare(rawShare)
+	share, err := NewShare(rawShare)
 	if err != nil {
 		t.Fatal(err)
 	}

diff --git a/pkg/shares/share_builder.go b/pkg/shares/share_builder.go
@@ -77,7 +77,7 @@ func (b *Builder) AddData(rawData []byte) (rawDataLeftOver []byte) {
 }
 
 func (b *Builder) Build() (*Share, error) {
-	return newShare(b.rawShareData)
+	return NewShare(b.rawShareData)
 }
 
 // IsEmptyShare returns true if no data has been written to the share

diff --git a/pkg/shares/shares.go b/pkg/shares/shares.go
@@ -30,7 +30,7 @@ func (s *Share) InfoByte() (InfoByte, error) {
 	return ParseInfoByte(unparsed)
 }
 
-func newShare(data []byte) (*Share, error) {
+func NewShare(data []byte) (*Share, error) {
 	if err := validateSize(data); err != nil {
 		return nil, err
 	}
@@ -200,10 +200,13 @@ func ToBytes(shares []Share) (bytes [][]byte) {
 	return bytes
 }
 
-func FromBytes(bytes [][]byte) (shares []Share) {
-	shares = make([]Share, len(bytes))
-	for i, b := range bytes {
-		shares[i] = Share{data: b}
+func FromBytes(bytes [][]byte) (shares []Share, err error) {
+	for _, b := range bytes {
+		share, err := NewShare(b)
+		if err != nil {
+			return nil, err
+		}
+		shares = append(shares, *share)
 	}
-	return shares
+	return shares, nil
 }
diff --git a/specs/src/specs/consensus.md b/specs/src/specs/consensus.md
@@ -47,7 +47,7 @@
 | `RESERVED_PADDING_NAMESPACE`        | `Namespace` | `0x0000000000000000000000000000000000000000000000000000000000000000FF` | Padding after all reserved namespaces but before blobs.                                              |
 | `MAX_RESERVED_NAMESPACE`            | `Namespace` | `0x0000000000000000000000000000000000000000000000000000000000000000FF` | Max reserved namespace is lexicographically the largest namespace that is reserved for protocol use. |
 | `TAIL_PADDING_NAMESPACE`            | `Namespace` | `0x00000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFE` | Tail padding for messages: padding after all messages to fill up the original data square.           |
-| `PARITY_SHARE_NAMESPACE`            | `Namespace` | `0x00000000000000000000000000000000000000000000000000FFFFFFFFFFFFFFFF` | Parity shares: extended shares in the available data matrix.                                         |
+| `PARITY_SHARE_NAMESPACE`            | `Namespace` | `0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF` | Parity shares: extended shares in the available data matrix.                                         |
 
 ### Rewards and Penalties
 

diff --git a/x/blob/README.md b/x/blob/README.md
@@ -60,7 +60,7 @@ The blob module emits the following events:
 celestia-app tx blob PayForBlobs <hex encoded namespace> <hex encoded data> [flags]
 ```
 
-For submitting PFB transaction via a light client's rpc, see [celestia-node's documention](https://docs.celestia.org/developers/node-api/#post-submit_pfd).
+For submitting PFB transaction via a light client's rpc, see [celestia-node's documention](https://docs.celestia.org/developers/rpc-tutorial/#submitpayforblob-arguments).
 
 While not directly supported, the steps in the [`SubmitPayForBlob`](https://github.com/celestiaorg/celestia-app/blob/a82110a281bf9ee95a9bf9f0492e5d091371ff0b/x/blob/payforblob.go) function can be reverse engineered to submit blobs programmatically.
 

diff --git a/x/blob/types/errors.go b/x/blob/types/errors.go
@@ -33,4 +33,5 @@ var (
 	ErrNoShareVersions                = errors.Register(ModuleName, 11133, "no share versions provided")
 	ErrNoBlobSizes                    = errors.Register(ModuleName, 11134, "no blob sizes provided")
 	ErrNoShareCommitments             = errors.Register(ModuleName, 11135, "no share commitments provided")
+	ErrInvalidNamespace               = errors.Register(ModuleName, 11136, "invalid namespace")
 )
diff --git a/x/blob/types/payforblob.go b/x/blob/types/payforblob.go
@@ -5,6 +5,7 @@ import (
 	fmt "fmt"
 	math "math"
 
+	"cosmossdk.io/errors"
 	"github.com/celestiaorg/celestia-app/pkg/appconsts"
 	appns "github.com/celestiaorg/celestia-app/pkg/namespace"
 	appshares "github.com/celestiaorg/celestia-app/pkg/shares"
@@ -104,7 +105,7 @@ func (msg *MsgPayForBlobs) ValidateBasic() error {
 	for _, namespace := range msg.Namespaces {
 		ns, err := appns.From(namespace)
 		if err != nil {
-			return err
+			return errors.Wrap(ErrInvalidNamespace, err.Error())
 		}
 		err = ValidateBlobNamespaceID(ns)
 		if err != nil {

diff --git a/x/blob/types/payforblob_test.go b/x/blob/types/payforblob_test.go
@@ -176,7 +176,7 @@ func TestValidateBasic(t *testing.T) {
 		{
 			name:    "parity shares namespace id",
 			msg:     paritySharesMsg,
-			wantErr: ErrParitySharesNamespace,
+			wantErr: ErrInvalidNamespace,
 		},
 		{
 			name:    "tail padding namespace id",