error handling for validate basic (#300)

x/payment/types/errors.go

@@ -6,7 +6,12 @@ import (
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
-// x/payment module sentinel errors
 var (
-	ErrSample = sdkerrors.Register(ModuleName, 1100, "sample error")
+	ErrReservedNamespace              = sdkerrors.Register(ModuleName, 11110, "cannot use reserved namespace IDs")
+	ErrInvalidNamespaceLen            = sdkerrors.Register(ModuleName, 11111, "invalid namespace length")
+	ErrInvalidDataSize                = sdkerrors.Register(ModuleName, 11112, "data must be multiple of shareSize")
+	ErrDeclaredActualDataSizeMismatch = sdkerrors.Register(ModuleName, 11113, "declared data size does not match actual size")
+	ErrCommittedSquareSizeNotPowOf2   = sdkerrors.Register(ModuleName, 11114, "committed to invalid square size: must be power of two")
+	ErrCalculateCommit                = sdkerrors.Register(ModuleName, 11115, "unexpected error calculating commit for share")
+	ErrInvalidShareCommit             = sdkerrors.Register(ModuleName, 11116, "invalid commit for share")
 )

x/payment/types/payformessage_test.go

@@ -11,7 +11,6 @@ import (
 	authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"github.com/tendermint/tendermint/pkg/consts"
 )
 
 func TestMountainRange(t *testing.T) {
@@ -75,34 +74,34 @@ func TestNextPowerOf2(t *testing.T) {
 	}
 }
 
-func TestPowerOf2 (t *testing.T) {
+func TestPowerOf2(t *testing.T) {
 	type test struct {
-		input uint64
+		input    uint64
 		expected bool
 	}
-	tests := []test {
+	tests := []test{
 		{
-			input: 1,
+			input:    1,
 			expected: true,
 		},
 		{
-			input: 2,
+			input:    2,
 			expected: true,
 		},
 		{
-			input: 256,
+			input:    256,
 			expected: true,
 		},
 		{
-			input: 3,
+			input:    3,
 			expected: false,
 		},
 		{
-			input: 79,
+			input:    79,
 			expected: false,
 		},
 		{
-			input: 0,
+			input:    0,
 			expected: false,
 		},
 	}
@@ -257,105 +256,6 @@ func TestSignMalleatedTxs(t *testing.T) {
 	}
 }
 
-func TestWirePayForMessage_ValidateBasic(t *testing.T) {
-	type test struct {
-		name      string
-		msg       *MsgWirePayForMessage
-		expectErr bool
-		errStr    string
-	}
-
-	// valid pfm
-	validMsg := validWirePayForMessage(t)
-
-	// pfm with bad ns id
-	badIDMsg := validWirePayForMessage(t)
-	badIDMsg.MessageNameSpaceId = []byte{1, 2, 3, 4, 5, 6, 7}
-
-	// pfm that uses reserved ns id
-	reservedMsg := validWirePayForMessage(t)
-	reservedMsg.MessageNameSpaceId = []byte{0, 0, 0, 0, 0, 0, 0, 100}
-
-	// pfm that has a wrong msg size
-	invalidMsgSizeMsg := validWirePayForMessage(t)
-	invalidMsgSizeMsg.Message = bytes.Repeat([]byte{1}, consts.ShareSize-20)
-
-	// pfm that has a wrong msg size
-	invalidDeclaredMsgSizeMsg := validWirePayForMessage(t)
-	invalidDeclaredMsgSizeMsg.MessageSize = 999
-
-	// pfm with bad commitment
-	badCommitMsg := validWirePayForMessage(t)
-	badCommitMsg.MessageShareCommitment[0].ShareCommitment = []byte{1, 2, 3, 4}
-
-	// pfm that has invalid square size (not power of 2)
-	invalidSquareSizeMsg := validWirePayForMessage(t)
-	invalidSquareSizeMsg.MessageShareCommitment[0].K = 15
-
-	// pfm that has a different power of 2 square size
-	badSquareSizeMsg := validWirePayForMessage(t)
-	badSquareSizeMsg.MessageShareCommitment[0].K = 4
-
-	tests := []test{
-		{
-			name: "valid msg",
-			msg:  validMsg,
-		},
-		{
-			name:      "bad ns ID",
-			msg:       badIDMsg,
-			expectErr: true,
-			errStr:    "invalid namespace length",
-		},
-		{
-			name:      "reserved ns id",
-			msg:       reservedMsg,
-			expectErr: true,
-			errStr:    "uses a reserved namesapce ID",
-		},
-		{
-			name:      "invalid msg size",
-			msg:       invalidMsgSizeMsg,
-			expectErr: true,
-			errStr:    "Share message must be divisible",
-		},
-		{
-			name:      "bad declared message size",
-			msg:       invalidDeclaredMsgSizeMsg,
-			expectErr: true,
-			errStr:    "Declared Message size does not match actual Message size",
-		},
-		{
-			name:      "bad commitment",
-			msg:       badCommitMsg,
-			expectErr: true,
-			errStr:    "invalid commit for square size",
-		},
-		{
-			name:      "invalid square size",
-			msg:       invalidSquareSizeMsg,
-			expectErr: true,
-			errStr:    fmt.Sprintf("invalid square size, the size must be power of 2: %d", invalidSquareSizeMsg.MessageShareCommitment[0].K),
-		},
-		{
-			name:      "wrong but valid square size",
-			msg:       badSquareSizeMsg,
-			expectErr: true,
-			errStr:    fmt.Sprintf("invalid commit for square size %d", badSquareSizeMsg.MessageShareCommitment[0].K),
-		},
-	}
-
-	for _, tt := range tests {
-		err := tt.msg.ValidateBasic()
-		if tt.expectErr {
-			require.NotNil(t, err, tt.name)
-			require.Contains(t, err.Error(), tt.errStr, tt.name)
-			continue
-		}
-		require.NoError(t, err, tt.name)
-	}
-}
-
 func TestProcessMessage(t *testing.T) {
 	type test struct {
 		name      string

x/payment/types/wirepayformessage.go

@@ -2,7 +2,6 @@ package types
 
 import (
 	"bytes"
-	"errors"
 	fmt "fmt"
 
 	sdkclient "github.com/cosmos/cosmos-sdk/client"
@@ -72,8 +71,7 @@ func (msg *MsgWirePayForMessage) ValidateBasic() error {
 
 	// ensure that the namespace id is of length == NamespaceIDSize
 	if nsLen := len(msg.GetMessageNameSpaceId()); nsLen != NamespaceIDSize {
-		return fmt.Errorf(
-			"invalid namespace length: got %d wanted %d",
+		return ErrInvalidNamespaceLen.Wrapf("got: %d want: %d",
 			nsLen,
 			NamespaceIDSize,
 		)
@@ -83,38 +81,42 @@ func (msg *MsgWirePayForMessage) ValidateBasic() error {
 		return sdkerrors.ErrInvalidAddress.Wrapf("invalid 'from' address: %s", err)
 	}
 
-	// ensure that the included message is evenly divisble into shares
+	// ensure that the included message is evenly divisible into shares
 	if msgMod := uint64(len(msg.GetMessage())) % ShareSize; msgMod != 0 {
-		return fmt.Errorf("Share message must be divisible by %d", ShareSize)
+		return ErrInvalidDataSize.Wrapf(
+			"shareSize: %d, data length: %d",
+			len(msg.Message),
+			ShareSize,
+		)
 	}
 
 	// make sure that the message size matches the actual size of the message
 	if msg.MessageSize != uint64(len(msg.Message)) {
-		return fmt.Errorf(
-			"Declared Message size does not match actual Message size, %d vs %d",
+		return ErrDeclaredActualDataSizeMismatch.Wrapf(
+			"declared: %d vs actual: %d",
 			msg.MessageSize,
 			len(msg.Message),
 		)
 	}
 
 	// ensure that a reserved namespace is not used
 	if bytes.Compare(msg.GetMessageNameSpaceId(), consts.MaxReservedNamespace) < 1 {
-		return errors.New("message is not valid: uses a reserved namesapce ID")
+		return ErrReservedNamespace.Wrapf("got namespace: %x, want: > %x", msg.GetMessageNameSpaceId(), consts.MaxReservedNamespace)
 	}
 
-	for _, commit := range msg.MessageShareCommitment {
+	for idx, commit := range msg.MessageShareCommitment {
 		// check that each commit is valid
 		if !powerOf2(commit.K) {
-			return fmt.Errorf("invalid square size, the size must be power of 2: %d", commit.K)
+			return ErrCommittedSquareSizeNotPowOf2.Wrapf("committed to square size: %d", commit.K)
 		}
 
 		calculatedCommit, err := CreateCommitment(commit.K, msg.GetMessageNameSpaceId(), msg.Message)
 		if err != nil {
-			return err
+			return ErrCalculateCommit.Wrap(err.Error())
 		}
 
-		if string(calculatedCommit) != string(commit.ShareCommitment) {
-			return fmt.Errorf("invalid commit for square size %d", commit.K)
+		if !bytes.Equal(calculatedCommit, commit.ShareCommitment) {
+			return ErrInvalidShareCommit.Wrapf("for square size %d and commit number %v", commit.K, idx)
 		}
 	}
 
@@ -212,4 +214,4 @@ func ProcessWirePayForMessage(msg *MsgWirePayForMessage, squareSize uint64) (*tm
 	}
 
 	return &coreMsg, pfm, shareCommit.Signature, nil
-}
+}

x/payment/types/wirepayformessage_test.go

@@ -0,0 +1,105 @@
+package types
+
+import (
+	"bytes"
+	"testing"
+
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	"github.com/stretchr/testify/assert"
+	"github.com/tendermint/tendermint/pkg/consts"
+)
+
+func TestWirePayForMessage_ValidateBasic(t *testing.T) {
+	type test struct {
+		name    string
+		msg     *MsgWirePayForMessage
+		wantErr *sdkerrors.Error
+	}
+
+	// valid pfm
+	validMsg := validWirePayForMessage(t)
+
+	// pfm with bad ns id
+	badIDMsg := validWirePayForMessage(t)
+	badIDMsg.MessageNameSpaceId = []byte{1, 2, 3, 4, 5, 6, 7}
+
+	// pfm that uses reserved ns id
+	reservedMsg := validWirePayForMessage(t)
+	reservedMsg.MessageNameSpaceId = []byte{0, 0, 0, 0, 0, 0, 0, 100}
+
+	// pfm that has a wrong msg size
+	invalidMsgSizeMsg := validWirePayForMessage(t)
+	invalidMsgSizeMsg.Message = bytes.Repeat([]byte{1}, consts.ShareSize-20)
+
+	// pfm that has a wrong msg size
+	invalidDeclaredMsgSizeMsg := validWirePayForMessage(t)
+	invalidDeclaredMsgSizeMsg.MessageSize = 999
+
+	// pfm with bad commitment
+	badCommitMsg := validWirePayForMessage(t)
+	badCommitMsg.MessageShareCommitment[0].ShareCommitment = []byte{1, 2, 3, 4}
+
+	// pfm that has invalid square size (not power of 2)
+	invalidSquareSizeMsg := validWirePayForMessage(t)
+	invalidSquareSizeMsg.MessageShareCommitment[0].K = 15
+
+	// pfm that has a different power of 2 square size
+	badSquareSizeMsg := validWirePayForMessage(t)
+	badSquareSizeMsg.MessageShareCommitment[0].K = 4
+
+	tests := []test{
+		{
+			name:    "valid msg",
+			msg:     validMsg,
+			wantErr: nil,
+		},
+		{
+			name:    "bad ns ID",
+			msg:     badIDMsg,
+			wantErr: ErrInvalidNamespaceLen,
+		},
+		{
+			name:    "reserved ns id",
+			msg:     reservedMsg,
+			wantErr: ErrReservedNamespace,
+		},
+		{
+			name:    "invalid msg size",
+			msg:     invalidMsgSizeMsg,
+			wantErr: ErrInvalidDataSize,
+		},
+		{
+			name:    "bad declared message size",
+			msg:     invalidDeclaredMsgSizeMsg,
+			wantErr: ErrDeclaredActualDataSizeMismatch,
+		},
+		{
+			name:    "bad commitment",
+			msg:     badCommitMsg,
+			wantErr: ErrCommittedSquareSizeNotPowOf2,
+		},
+		{
+			name:    "invalid square size",
+			msg:     invalidSquareSizeMsg,
+			wantErr: ErrCommittedSquareSizeNotPowOf2,
+		},
+		{
+			name:    "wrong but valid square size",
+			msg:     badSquareSizeMsg,
+			wantErr: ErrInvalidShareCommit,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := tt.msg.ValidateBasic()
+			if tt.wantErr != nil {
+				assert.ErrorAs(t, err, tt.wantErr)
+				space, code, log := sdkerrors.ABCIInfo(err, false)
+				assert.Equal(t, tt.wantErr.Codespace(), space)
+				assert.Equal(t, tt.wantErr.ABCICode(), code)
+				t.Log(log)
+			}
+		})
+	}
+}