diff --git a/x/auction/types/auctions.go b/x/auction/types/auctions.go index ca5ea74f..7af97e6d 100644 --- a/x/auction/types/auctions.go +++ b/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) { - return fmt.Errorf("MaxEndTime < EndTime (%s < %s)", a.MaxEndTime, a.EndTime) + // ID can be 0 for surplus, debt and collateral auctions + 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 { - if w.IsNegative() { - return fmt.Errorf("weights contain a negative amount: %s", w) + for i := range wa.Addresses { + if wa.Addresses[i].Empty() { + 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 } diff --git a/x/auction/types/auctions_test.go b/x/auction/types/auctions_test.go index 7494612d..b857fc18 100644 --- a/x/auction/types/auctions_test.go +++ b/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" - TestAccAddress2 = "kava1pdfav2cjhry9k79nu6r8kgknnjtq6a7rcr0qlr" + testAccAddress1 = "kava1qcfdf69js922qrdr4yaww3ax7gjml6pd39p8lj" + 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 + name string + addresses []sdk.AccAddress + weights []sdk.Int + expPass bool }{ { - name: "normal", - addresses: []sdk.AccAddress{ - sdk.AccAddress([]byte(TestAccAddress1)), - sdk.AccAddress([]byte(TestAccAddress2)), - }, - weights: is(6, 8), - expectedErr: nil, + "normal", + []sdk.AccAddress{addr1, addr2}, + []sdk.Int{sdk.NewInt(6), sdk.NewInt(8)}, + true, }, { - name: "mismatched", - addresses: []sdk.AccAddress{ - sdk.AccAddress([]byte(TestAccAddress1)), - sdk.AccAddress([]byte(TestAccAddress2)), - }, - weights: is(6), - expectedErr: fmt.Errorf("number of addresses doesn't match number of weights, %d ≠ %d", 2, 1), + "empty address", + []sdk.AccAddress{nil, nil}, + []sdk.Int{sdk.NewInt(6), sdk.NewInt(8)}, + false, }, { - name: "negativeWeight", - addresses: []sdk.AccAddress{ - sdk.AccAddress([]byte(TestAccAddress1)), - sdk.AccAddress([]byte(TestAccAddress2)), - }, - weights: is(6, -8), - expectedErr: fmt.Errorf("weights contain a negative amount: %s", i(-8)), + "mismatched", + []sdk.AccAddress{addr1, addr2}, + []sdk.Int{sdk.NewInt(6)}, + false, }, { - name: "zero total weights", - addresses: []sdk.AccAddress{ - sdk.AccAddress([]byte(TestAccAddress1)), - sdk.AccAddress([]byte(TestAccAddress2)), - }, - weights: is(0, 0), - expectedErr: fmt.Errorf("total weight must be positive"), + "negative weight", + []sdk.AccAddress{addr1, addr2}, + is(6, -8), + false, }, { - name: "no weights", - addresses: nil, - weights: nil, - expectedErr: fmt.Errorf("must be at least 1 weighted address"), + "zero weight", + []sdk.AccAddress{addr1, addr2}, + is(0, 0), + 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) + // Attempt to instantiate new WeightedAddresses + weightedAddresses, err := NewWeightedAddresses(tc.addresses, tc.weights) - if tc.expectedErr != nil { - // 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) - } + if tc.expPass { + require.NoError(t, err) + 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(TestAccAddress2)), + sdk.AccAddress([]byte(testAccAddress1)), + sdk.AccAddress([]byte(testAccAddress2)), } weights := []sdk.Int{ diff --git a/x/auction/types/msg.go b/x/auction/types/msg.go index 4614f512..ca4ff49e 100644 --- a/x/auction/types/msg.go +++ b/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) } diff --git a/x/auction/types/msg_test.go b/x/auction/types/msg_test.go index 4c5ee977..abef664b 100644 --- a/x/auction/types/msg_test.go +++ b/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}, - {"emptyAddr", - NewMsgPlaceBid(0, sdk.AccAddress{}, c("token", 10)), - false}, - {"negativeAmount", - NewMsgPlaceBid(0, addr, sdk.Coin{Denom: "token", Amount: sdk.NewInt(-10)}), - false}, - {"zeroAmount", - NewMsgPlaceBid(0, addr, c("token", 0)), - true}, + false, + }, + { + "empty address ", + NewMsgPlaceBid(1, nil, c("token", 10)), + false, + }, + { + "invalid address", + 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()) - } else { - require.Error(t, tc.msg.ValidateBasic()) - } - }) + if tc.expectPass { + require.NoError(t, tc.msg.ValidateBasic(), tc.name) + } else { + require.Error(t, tc.msg.ValidateBasic(), tc.name) + } } } diff --git a/x/incentive/types/genesis.go b/x/incentive/types/genesis.go index d73d1b5e..f9514727 100644 --- a/x/incentive/types/genesis.go +++ b/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{}) { - return fmt.Errorf("previous block time not set") + if gs.PreviousBlockTime.IsZero() { + return fmt.Errorf("previous block time not set or zero") } return nil } diff --git a/x/incentive/types/msg.go b/x/incentive/types/msg.go index 446389d5..dc5707a4 100644 --- a/x/incentive/types/msg.go +++ b/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 errors.New("invalid (empty) denom") - } - return nil + return sdk.ValidateDenom(msg.Denom) } // GetSignBytes gets the canonical byte representation of the Msg. diff --git a/x/validator-vesting/types/genesis.go b/x/validator-vesting/types/genesis.go index aef83708..8a4daf8f 100644 --- a/x/validator-vesting/types/genesis.go +++ b/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 { - return fmt.Errorf("Previous block time should be positive, is set to %v", data.PreviousBlockTime.Unix()) + if data.PreviousBlockTime.IsZero() { + return errors.New("previous block time cannot be zero") } return nil }