x/auction: types validation (#500)

x/auction: types validation

x/auction/types/auctions.go

@@ -1,7 +1,9 @@
 package types
 
 import (
+	"errors"
 	"fmt"
+	"strings"
 	"time"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -66,15 +68,26 @@ func (a BaseAuction) GetType() string { return "base" }
 
 // Validate verifies that the auction end time is before max end time
 func (a BaseAuction) Validate() error {
-	if a.EndTime.After(a.MaxEndTime) {
+	// ID can be 0 for surplus, debt and collateral auctions
-		return fmt.Errorf("MaxEndTime < EndTime (%s < %s)", a.MaxEndTime, a.EndTime)
+	if strings.TrimSpace(a.Initiator) == "" {
+		return errors.New("auction initiator cannot be blank")
 	}
 	if !a.Lot.IsValid() {
 		return fmt.Errorf("invalid lot: %s", a.Lot)
 	}
+	// NOTE: bidder can be empty for Surplus and Collateral auctions
+	if !a.Bidder.Empty() && len(a.Bidder) != sdk.AddrLen {
+		return fmt.Errorf("the expected bidder address length is %d, actual length is %d", sdk.AddrLen, len(a.Bidder))
+	}
 	if !a.Bid.IsValid() {
 		return fmt.Errorf("invalid bid: %s", a.Bid)
 	}
+	if a.EndTime.IsZero() || a.MaxEndTime.IsZero() {
+		return errors.New("end time cannot be zero")
+	}
+	if a.EndTime.After(a.MaxEndTime) {
+		return fmt.Errorf("MaxEndTime < EndTime (%s < %s)", a.MaxEndTime, a.EndTime)
+	}
 	return nil
 }
 
@@ -154,6 +167,7 @@ func (a DebtAuction) GetModuleAccountCoins() sdk.Coins {
 // GetPhase returns the direction of a debt auction, which never changes.
 func (a DebtAuction) GetPhase() string { return "reverse" }
 
+// Validate validates the DebtAuction fields values.
 func (a DebtAuction) Validate() error {
 	if !a.CorrespondingDebt.IsValid() {
 		return fmt.Errorf("invalid corresponding debt: %s", a.CorrespondingDebt)
@@ -175,7 +189,8 @@ func NewDebtAuction(buyerModAccName string, bid sdk.Coin, initialLot sdk.Coin, e
 			Bid:             bid,                                      // amount that the buyer is buying - doesn't change over course of auction
 			HasReceivedBids: false,                                    // new auctions don't have any bids
 			EndTime:         endTime,
-			MaxEndTime:      endTime},
+			MaxEndTime:      endTime,
+		},
 		CorrespondingDebt: debt,
 	}
 	return auction
@@ -221,6 +236,7 @@ func (a CollateralAuction) GetPhase() string {
 	return "forward"
 }
 
+// Validate validates the CollateralAuction fields values.
 func (a CollateralAuction) Validate() error {
 	if !a.CorrespondingDebt.IsValid() {
 		return fmt.Errorf("invalid corresponding debt: %s", a.CorrespondingDebt)
@@ -292,18 +308,28 @@ func (wa WeightedAddresses) Validate() error {
 	if len(wa.Weights) < 1 {
 		return fmt.Errorf("must be at least 1 weighted address")
 	}
+
 	if len(wa.Addresses) != len(wa.Weights) {
 		return fmt.Errorf("number of addresses doesn't match number of weights, %d ≠ %d", len(wa.Addresses), len(wa.Weights))
 	}
+
 	totalWeight := sdk.ZeroInt()
-	for _, w := range wa.Weights {
+	for i := range wa.Addresses {
-		if w.IsNegative() {
+		if wa.Addresses[i].Empty() {
-			return fmt.Errorf("weights contain a negative amount: %s", w)
+			return fmt.Errorf("address %d cannot be empty", i)
 		}
-		totalWeight = totalWeight.Add(w)
+		if len(wa.Addresses[i]) != sdk.AddrLen {
+			return fmt.Errorf("address %d has an invalid length: expected %d, got %d", i, sdk.AddrLen, len(wa.Addresses[i]))
 		}
+		if wa.Weights[i].IsNegative() {
+			return fmt.Errorf("weight %d contains a negative amount: %s", i, wa.Weights[i])
+		}
+		totalWeight = totalWeight.Add(wa.Weights[i])
+	}
+
 	if !totalWeight.IsPositive() {
 		return fmt.Errorf("total weight must be positive")
 	}
+
 	return nil
 }

x/auction/types/auctions_test.go

@@ -1,7 +1,6 @@
 package types
 
 import (
-	"fmt"
 	"testing"
 	"time"
 
@@ -21,10 +20,14 @@ const (
 	TestDebtAmount2         = 15
 	TestExtraEndTime        = 10000
 	TestAuctionID           = 9999123
-	TestAccAddress1         = "kava1qcfdf69js922qrdr4yaww3ax7gjml6pd39p8lj"
+	testAccAddress1         = "kava1qcfdf69js922qrdr4yaww3ax7gjml6pd39p8lj"
-	TestAccAddress2         = "kava1pdfav2cjhry9k79nu6r8kgknnjtq6a7rcr0qlr"
+	testAccAddress2         = "kava1pdfav2cjhry9k79nu6r8kgknnjtq6a7rcr0qlr"
 )
 
+func init() {
+	sdk.GetConfig().SetBech32PrefixForAccount("kava", "kava"+sdk.PrefixPublic)
+}
+
 func d(amount string) sdk.Dec               { return sdk.MustNewDecFromStr(amount) }
 func c(denom string, amount int64) sdk.Coin { return sdk.NewInt64Coin(denom, amount) }
 func i(n int64) sdk.Int                     { return sdk.NewInt(n) }
@@ -36,74 +39,337 @@ func is(ns ...int64) (is []sdk.Int) {
 }
 
 func TestNewWeightedAddresses(t *testing.T) {
+	addr1, err := sdk.AccAddressFromBech32(testAccAddress1)
+	require.NoError(t, err)
+
+	addr2, err := sdk.AccAddressFromBech32(testAccAddress2)
+	require.NoError(t, err)
 
 	tests := []struct {
 		name      string
 		addresses []sdk.AccAddress
 		weights   []sdk.Int
-		expectedErr error
+		expPass   bool
 	}{
 		{
-			name: "normal",
+			"normal",
-			addresses: []sdk.AccAddress{
+			[]sdk.AccAddress{addr1, addr2},
-				sdk.AccAddress([]byte(TestAccAddress1)),
+			[]sdk.Int{sdk.NewInt(6), sdk.NewInt(8)},
-				sdk.AccAddress([]byte(TestAccAddress2)),
+			true,
-			},
-			weights:     is(6, 8),
-			expectedErr: nil,
 		},
 		{
-			name: "mismatched",
+			"empty address",
-			addresses: []sdk.AccAddress{
+			[]sdk.AccAddress{nil, nil},
-				sdk.AccAddress([]byte(TestAccAddress1)),
+			[]sdk.Int{sdk.NewInt(6), sdk.NewInt(8)},
-				sdk.AccAddress([]byte(TestAccAddress2)),
+			false,
-			},
-			weights:     is(6),
-			expectedErr: fmt.Errorf("number of addresses doesn't match number of weights, %d ≠ %d", 2, 1),
 		},
 		{
-			name: "negativeWeight",
+			"mismatched",
-			addresses: []sdk.AccAddress{
+			[]sdk.AccAddress{addr1, addr2},
-				sdk.AccAddress([]byte(TestAccAddress1)),
+			[]sdk.Int{sdk.NewInt(6)},
-				sdk.AccAddress([]byte(TestAccAddress2)),
+			false,
-			},
-			weights:     is(6, -8),
-			expectedErr: fmt.Errorf("weights contain a negative amount: %s", i(-8)),
 		},
 		{
-			name: "zero total weights",
+			"negative weight",
-			addresses: []sdk.AccAddress{
+			[]sdk.AccAddress{addr1, addr2},
-				sdk.AccAddress([]byte(TestAccAddress1)),
+			is(6, -8),
-				sdk.AccAddress([]byte(TestAccAddress2)),
+			false,
-			},
-			weights:     is(0, 0),
-			expectedErr: fmt.Errorf("total weight must be positive"),
 		},
 		{
-			name:        "no weights",
+			"zero weight",
-			addresses:   nil,
+			[]sdk.AccAddress{addr1, addr2},
-			weights:     nil,
+			is(0, 0),
-			expectedErr: fmt.Errorf("must be at least 1 weighted address"),
+			false,
 		},
 	}
 
 	// Run NewWeightedAdresses tests
 	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
 		// Attempt to instantiate new WeightedAddresses
 		weightedAddresses, err := NewWeightedAddresses(tc.addresses, tc.weights)
 
-			if tc.expectedErr != nil {
+		if tc.expPass {
-				// Confirm the error
-				require.EqualError(t, err, tc.expectedErr.Error())
-			} else {
 			require.NoError(t, err)
-				// Check addresses, weights
 			require.Equal(t, tc.addresses, weightedAddresses.Addresses)
 			require.Equal(t, tc.weights, weightedAddresses.Weights)
+		} else {
+			require.Error(t, err)
+		}
+	}
 }
 
-		})
+func TestBaseAuctionValidate(t *testing.T) {
+	addr1, err := sdk.AccAddressFromBech32(testAccAddress1)
+	require.NoError(t, err)
+
+	now := time.Now()
+
+	tests := []struct {
+		msg     string
+		auction BaseAuction
+		expPass bool
+	}{
+		{
+			"valid auction",
+			BaseAuction{
+				ID:              1,
+				Initiator:       testAccAddress1,
+				Lot:             c("kava", 1),
+				Bidder:          addr1,
+				Bid:             c("kava", 1),
+				EndTime:         now,
+				MaxEndTime:      now,
+				HasReceivedBids: true,
+			},
+			true,
+		},
+		{
+			"blank initiator",
+			BaseAuction{
+				ID:        1,
+				Initiator: "",
+			},
+			false,
+		},
+		{
+			"invalid lot",
+			BaseAuction{
+				ID:        1,
+				Initiator: testAccAddress1,
+				Lot:       sdk.Coin{Denom: "DENOM", Amount: sdk.NewInt(1)},
+			},
+			false,
+		},
+		{
+			"empty bidder",
+			BaseAuction{
+				ID:        1,
+				Initiator: testAccAddress1,
+				Lot:       c("kava", 1),
+				Bidder:    sdk.AccAddress{},
+			},
+			false,
+		},
+		{
+			"invalid bidder",
+			BaseAuction{
+				ID:        1,
+				Initiator: testAccAddress1,
+				Lot:       c("kava", 1),
+				Bidder:    addr1[:10],
+			},
+			false,
+		},
+		{
+			"invalid bid",
+			BaseAuction{
+				ID:        1,
+				Initiator: testAccAddress1,
+				Lot:       c("kava", 1),
+				Bidder:    addr1,
+				Bid:       sdk.Coin{Denom: "DENOM", Amount: sdk.NewInt(1)},
+			},
+			false,
+		},
+		{
+			"invalid end time",
+			BaseAuction{
+				ID:        1,
+				Initiator: testAccAddress1,
+				Lot:       c("kava", 1),
+				Bidder:    addr1,
+				Bid:       c("kava", 1),
+				EndTime:   time.Unix(0, 0),
+			},
+			false,
+		},
+		{
+			"max end time > endtime",
+			BaseAuction{
+				ID:         1,
+				Initiator:  testAccAddress1,
+				Lot:        c("kava", 1),
+				Bidder:     addr1,
+				Bid:        c("kava", 1),
+				EndTime:    now.Add(time.Minute),
+				MaxEndTime: now,
+			},
+			false,
+		},
+	}
+
+	for _, tc := range tests {
+
+		err := tc.auction.Validate()
+
+		if tc.expPass {
+			require.NoError(t, err, tc.msg)
+		} else {
+			require.Error(t, err, tc.msg)
+		}
+	}
+}
+
+func TestDebtAuctionValidate(t *testing.T) {
+	addr1, err := sdk.AccAddressFromBech32(testAccAddress1)
+	require.NoError(t, err)
+
+	now := time.Now()
+
+	tests := []struct {
+		msg     string
+		auction DebtAuction
+		expPass bool
+	}{
+		{
+			"valid auction",
+			DebtAuction{
+				BaseAuction: BaseAuction{
+					ID:              1,
+					Initiator:       testAccAddress1,
+					Lot:             c("kava", 1),
+					Bidder:          addr1,
+					Bid:             c("kava", 1),
+					EndTime:         now,
+					MaxEndTime:      now,
+					HasReceivedBids: true,
+				},
+				CorrespondingDebt: c("kava", 1),
+			},
+			true,
+		},
+		{
+			"invalid corresponding debt",
+			DebtAuction{
+				BaseAuction: BaseAuction{
+					ID:              1,
+					Initiator:       testAccAddress1,
+					Lot:             c("kava", 1),
+					Bidder:          addr1,
+					Bid:             c("kava", 1),
+					EndTime:         now,
+					MaxEndTime:      now,
+					HasReceivedBids: true,
+				},
+				CorrespondingDebt: sdk.Coin{Denom: "DENOM", Amount: sdk.NewInt(1)},
+			},
+			false,
+		},
+	}
+
+	for _, tc := range tests {
+
+		err := tc.auction.Validate()
+
+		if tc.expPass {
+			require.NoError(t, err, tc.msg)
+		} else {
+			require.Error(t, err, tc.msg)
+		}
+	}
+}
+
+func TestCollateralAuctionValidate(t *testing.T) {
+	addr1, err := sdk.AccAddressFromBech32(testAccAddress1)
+	require.NoError(t, err)
+
+	now := time.Now()
+
+	tests := []struct {
+		msg     string
+		auction CollateralAuction
+		expPass bool
+	}{
+		{
+			"valid auction",
+			CollateralAuction{
+				BaseAuction: BaseAuction{
+					ID:              1,
+					Initiator:       testAccAddress1,
+					Lot:             c("kava", 1),
+					Bidder:          addr1,
+					Bid:             c("kava", 1),
+					EndTime:         now,
+					MaxEndTime:      now,
+					HasReceivedBids: true,
+				},
+				CorrespondingDebt: c("kava", 1),
+				MaxBid:            c("kava", 1),
+				LotReturns: WeightedAddresses{
+					Addresses: []sdk.AccAddress{addr1},
+					Weights:   []sdk.Int{sdk.NewInt(1)},
+				},
+			},
+			true,
+		},
+		{
+			"invalid corresponding debt",
+			CollateralAuction{
+				BaseAuction: BaseAuction{
+					ID:              1,
+					Initiator:       testAccAddress1,
+					Lot:             c("kava", 1),
+					Bidder:          addr1,
+					Bid:             c("kava", 1),
+					EndTime:         now,
+					MaxEndTime:      now,
+					HasReceivedBids: true,
+				},
+				CorrespondingDebt: sdk.Coin{Denom: "DENOM", Amount: sdk.NewInt(1)},
+			},
+			false,
+		},
+		{
+			"invalid max bid",
+			CollateralAuction{
+				BaseAuction: BaseAuction{
+					ID:              1,
+					Initiator:       testAccAddress1,
+					Lot:             c("kava", 1),
+					Bidder:          addr1,
+					Bid:             c("kava", 1),
+					EndTime:         now,
+					MaxEndTime:      now,
+					HasReceivedBids: true,
+				},
+				CorrespondingDebt: c("kava", 1),
+				MaxBid:            sdk.Coin{Denom: "DENOM", Amount: sdk.NewInt(1)},
+			},
+			false,
+		},
+		{
+			"invalid lot returns",
+			CollateralAuction{
+				BaseAuction: BaseAuction{
+					ID:              1,
+					Initiator:       testAccAddress1,
+					Lot:             c("kava", 1),
+					Bidder:          addr1,
+					Bid:             c("kava", 1),
+					EndTime:         now,
+					MaxEndTime:      now,
+					HasReceivedBids: true,
+				},
+				CorrespondingDebt: c("kava", 1),
+				MaxBid:            c("kava", 1),
+				LotReturns: WeightedAddresses{
+					Addresses: []sdk.AccAddress{nil},
+					Weights:   []sdk.Int{sdk.NewInt(1)},
+				},
+			},
+			false,
+		},
+	}
+
+	for _, tc := range tests {
+
+		err := tc.auction.Validate()
+
+		if tc.expPass {
+			require.NoError(t, err, tc.msg)
+		} else {
+			require.Error(t, err, tc.msg)
+		}
 	}
 }
 
@@ -170,8 +436,8 @@ func TestNewDebtAuction(t *testing.T) {
 func TestNewCollateralAuction(t *testing.T) {
 	// Set up WeightedAddresses
 	addresses := []sdk.AccAddress{
-		sdk.AccAddress([]byte(TestAccAddress1)),
+		sdk.AccAddress([]byte(testAccAddress1)),
-		sdk.AccAddress([]byte(TestAccAddress2)),
+		sdk.AccAddress([]byte(testAccAddress2)),
 	}
 
 	weights := []sdk.Int{

x/auction/types/msg.go

@@ -1,6 +1,7 @@
 package types
 
 import (
+	"errors"
 	"fmt"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -34,9 +35,15 @@ func (msg MsgPlaceBid) Type() string { return "place_bid" }
 
 // ValidateBasic does a simple validation check that doesn't require access to state.
 func (msg MsgPlaceBid) ValidateBasic() error {
+	if msg.AuctionID == 0 {
+		return errors.New("auction id cannot be zero")
+	}
 	if msg.Bidder.Empty() {
 		return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "bidder address cannot be empty")
 	}
+	if len(msg.Bidder) != sdk.AddrLen {
+		return fmt.Errorf("the expected bidder address length is %d, actual length is %d", sdk.AddrLen, len(msg.Bidder))
+	}
 	if !msg.Amount.IsValid() {
 		return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, "bid amount %s", msg.Amount)
 	}

x/auction/types/msg_test.go

@@ -9,33 +9,51 @@ import (
 )
 
 func TestMsgPlaceBid_ValidateBasic(t *testing.T) {
-	addr := sdk.AccAddress([]byte("someName"))
+	addr, err := sdk.AccAddressFromBech32(testAccAddress1)
+	require.NoError(t, err)
+
 	tests := []struct {
 		name       string
 		msg        MsgPlaceBid
 		expectPass bool
 	}{
-		{"normal",
+		{
+			"normal",
+			NewMsgPlaceBid(1, addr, c("token", 10)),
+			true,
+		},
+		{
+			"zero id",
 			NewMsgPlaceBid(0, addr, c("token", 10)),
-			true},
+			false,
-		{"emptyAddr",
+		},
-			NewMsgPlaceBid(0, sdk.AccAddress{}, c("token", 10)),
+		{
-			false},
+			"empty address ",
-		{"negativeAmount",
+			NewMsgPlaceBid(1, nil, c("token", 10)),
-			NewMsgPlaceBid(0, addr, sdk.Coin{Denom: "token", Amount: sdk.NewInt(-10)}),
+			false,
-			false},
+		},
-		{"zeroAmount",
+		{
-			NewMsgPlaceBid(0, addr, c("token", 0)),
+			"invalid address",
-			true},
+			NewMsgPlaceBid(1, addr[:10], c("token", 10)),
+			false,
+		},
+		{
+			"negative amount",
+			NewMsgPlaceBid(1, addr, sdk.Coin{Denom: "token", Amount: sdk.NewInt(-10)}),
+			false,
+		},
+		{
+			"zero amount",
+			NewMsgPlaceBid(1, addr, c("token", 0)),
+			true,
+		},
 	}
 
 	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
 		if tc.expectPass {
-				require.NoError(t, tc.msg.ValidateBasic())
+			require.NoError(t, tc.msg.ValidateBasic(), tc.name)
 		} else {
-				require.Error(t, tc.msg.ValidateBasic())
+			require.Error(t, tc.msg.ValidateBasic(), tc.name)
-			}
+		}
-		})
 	}
 }

x/incentive/types/genesis.go

@@ -56,8 +56,8 @@ func (gs GenesisState) Validate() error {
 	if err := gs.Params.Validate(); err != nil {
 		return err
 	}
-	if gs.PreviousBlockTime.Equal(time.Time{}) {
+	if gs.PreviousBlockTime.IsZero() {
-		return fmt.Errorf("previous block time not set")
+		return fmt.Errorf("previous block time not set or zero")
 	}
 	return nil
 }

x/incentive/types/msg.go

@@ -1,9 +1,6 @@
 package types
 
 import (
-	"errors"
-	"strings"
-
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
@@ -36,10 +33,7 @@ func (msg MsgClaimReward) ValidateBasic() error {
 	if msg.Sender.Empty() {
 		return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "sender address cannot be empty")
 	}
-	if strings.TrimSpace(msg.Denom) == "" {
+	return sdk.ValidateDenom(msg.Denom)
-		return errors.New("invalid (empty) denom")
-	}
-	return nil
 }
 
 // GetSignBytes gets the canonical byte representation of the Msg.

x/validator-vesting/types/genesis.go

@@ -2,7 +2,7 @@ package types
 
 import (
 	"bytes"
-	"fmt"
+	"errors"
 	"time"
 
 	tmtime "github.com/tendermint/tendermint/types/time"
@@ -39,8 +39,8 @@ func (data GenesisState) IsEmpty() bool {
 
 // ValidateGenesis returns nil because accounts are validated by auth
 func ValidateGenesis(data GenesisState) error {
-	if data.PreviousBlockTime.Unix() < 0 {
+	if data.PreviousBlockTime.IsZero() {
-		return fmt.Errorf("Previous block time should be positive, is set to %v", data.PreviousBlockTime.Unix())
+		return errors.New("previous block time cannot be zero")
 	}
 	return nil
 }