From 32213ed56c346dc599653eda778aa100a70fde26 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Mon, 11 May 2020 18:56:28 -0400 Subject: [PATCH 1/8] x/auction: types validation --- x/auction/types/auctions.go | 45 ++++++++++++++++++++++++++++++++---- x/auction/types/msg.go | 7 ++++++ x/incentive/types/genesis.go | 4 ++-- 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/x/auction/types/auctions.go b/x/auction/types/auctions.go index 2e06b86e..37e797f5 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,39 @@ 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) + if a.ID == 0 { + return errors.New("auction id cannot be zero") + } + 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) } + if a.Bidder.Empty() { + return errors.New("auction bidder cannot be empty") + } + if 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 len(a.Bidder.Bytes()) != sdk.AddrLen { + return errors.New("auction bidder cannot be empty") + } 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) + } + if a.HasReceivedBids && !a.Bid.IsPositive() { + return fmt.Errorf("cannot have a zero value bid when auction has a true HasReceivedBids flag value") + } + if !a.HasReceivedBids && a.Bid.IsPositive() { + return fmt.Errorf("cannot have a positive value bid when auction has a false HasReceivedBids flag value") + } return nil } @@ -154,6 +180,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) @@ -221,6 +248,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,9 +320,16 @@ func (wa WeightedAddresses) Validate() error { 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)) } - 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) + } + if len(wa.Addresses[i]) != sdk.AddrLen { + return fmt.Errorf("address %d has an invalid length: expectd %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]) } } return nil 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/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 } From e3aad2306b5873e4a55fd7488969b1b6d40c40bc Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Mon, 11 May 2020 19:10:14 -0400 Subject: [PATCH 2/8] other validations --- x/incentive/types/msg.go | 8 +------- x/validator-vesting/types/genesis.go | 6 +++--- 2 files changed, 4 insertions(+), 10 deletions(-) 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 } From 71742d2eefcb544d55478c9b867b2283528ba571 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Tue, 12 May 2020 16:32:18 -0400 Subject: [PATCH 3/8] auction tests --- x/auction/types/auctions.go | 8 +- x/auction/types/auctions_test.go | 390 +++++++++++++++++++++++++++---- 2 files changed, 343 insertions(+), 55 deletions(-) diff --git a/x/auction/types/auctions.go b/x/auction/types/auctions.go index 37e797f5..53574412 100644 --- a/x/auction/types/auctions.go +++ b/x/auction/types/auctions.go @@ -83,9 +83,6 @@ func (a BaseAuction) Validate() error { if 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 len(a.Bidder.Bytes()) != sdk.AddrLen { - return errors.New("auction bidder cannot be empty") - } if !a.Bid.IsValid() { return fmt.Errorf("invalid bid: %s", a.Bid) } @@ -202,7 +199,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 @@ -326,7 +324,7 @@ func (wa WeightedAddresses) Validate() error { return fmt.Errorf("address %d cannot be empty", i) } if len(wa.Addresses[i]) != sdk.AddrLen { - return fmt.Errorf("address %d has an invalid length: expectd %d, got %d", i, sdk.AddrLen, len(wa.Addresses[i])) + 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]) diff --git a/x/auction/types/auctions_test.go b/x/auction/types/auctions_test.go index 49953cbe..e19e7423 100644 --- a/x/auction/types/auctions_test.go +++ b/x/auction/types/auctions_test.go @@ -20,11 +20,20 @@ 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 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 @@ -34,69 +43,350 @@ func TestNewWeightedAddresses(t *testing.T) { }{ { "normal", - []sdk.AccAddress{ - sdk.AccAddress([]byte(TestAccAddress1)), - sdk.AccAddress([]byte(TestAccAddress2)), - }, - []sdk.Int{ - sdk.NewInt(6), - sdk.NewInt(8), - }, + []sdk.AccAddress{addr1, addr2}, + []sdk.Int{sdk.NewInt(6), sdk.NewInt(8)}, true, }, + { + "empty address", + []sdk.AccAddress{nil, nil}, + []sdk.Int{sdk.NewInt(6), sdk.NewInt(8)}, + false, + }, { "mismatched", - []sdk.AccAddress{ - sdk.AccAddress([]byte(TestAccAddress1)), - sdk.AccAddress([]byte(TestAccAddress2)), - }, - []sdk.Int{ - sdk.NewInt(6), - }, + []sdk.AccAddress{addr1, addr2}, + []sdk.Int{sdk.NewInt(6)}, false, }, { "negativeWeight", - []sdk.AccAddress{ - sdk.AccAddress([]byte(TestAccAddress1)), - sdk.AccAddress([]byte(TestAccAddress2)), - }, - []sdk.Int{ - sdk.NewInt(6), - sdk.NewInt(-8), - }, + []sdk.AccAddress{addr1, addr2}, + []sdk.Int{sdk.NewInt(6), sdk.NewInt(-8)}, 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.expectpass { - // Confirm there is no error - require.Nil(t, err) + if tc.expectpass { + require.NoError(t, err) + require.Equal(t, tc.addresses, weightedAddresses.Addresses) + require.Equal(t, tc.weights, weightedAddresses.Weights) + } else { + require.Error(t, err) + } + } +} - // Check addresses, weights - require.Equal(t, tc.addresses, weightedAddresses.Addresses) - require.Equal(t, tc.weights, weightedAddresses.Weights) - } else { - // Confirm that there is an error - require.NotNil(t, err) +func TestBaseAuctionValidate(t *testing.T) { + addr1, err := sdk.AccAddressFromBech32(testAccAddress1) + require.NoError(t, err) - switch tc.name { - case "mismatched": - require.Contains(t, err.Error(), "number of addresses doesn't match number of weights") - case "negativeWeight": - require.Contains(t, err.Error(), "weights contain a negative amount") - default: - // Unexpected error state - t.Fail() - } - } - }) + 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, + }, + { + "0 id", + BaseAuction{ID: 0}, + false, + }, + { + "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: nil, + }, + 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, + }, + { + "zero bid with received bids", + BaseAuction{ + ID: 1, + Initiator: testAccAddress1, + Lot: c("kava", 1), + Bidder: addr1, + Bid: c("kava", 0), + EndTime: now, + MaxEndTime: now, + HasReceivedBids: true, + }, + false, + }, + { + "positive bid without receiving bids", + BaseAuction{ + ID: 1, + Initiator: testAccAddress1, + Lot: c("kava", 1), + Bidder: addr1, + Bid: c("kava", 1), + EndTime: now, + MaxEndTime: now, + HasReceivedBids: false, + }, + 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) + } } } @@ -163,8 +453,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{ From 2327b01ed4ffdec2a173ec0d7809dab1ebd49040 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Tue, 12 May 2020 16:37:08 -0400 Subject: [PATCH 4/8] message test --- x/auction/types/msg_test.go | 56 ++++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/x/auction/types/msg_test.go b/x/auction/types/msg_test.go index 7ac2909c..948ac22b 100644 --- a/x/auction/types/msg_test.go +++ b/x/auction/types/msg_test.go @@ -9,34 +9,52 @@ 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) + } } } From 1a46b3fa2bb6c5a16d4344d5905edcc0eb000333 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Wed, 13 May 2020 09:24:17 -0400 Subject: [PATCH 5/8] fix tests --- x/auction/types/auctions.go | 9 ++++----- x/auction/types/auctions_test.go | 7 +------ 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/x/auction/types/auctions.go b/x/auction/types/auctions.go index 53574412..021c8561 100644 --- a/x/auction/types/auctions.go +++ b/x/auction/types/auctions.go @@ -68,19 +68,18 @@ 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.ID == 0 { - return errors.New("auction id cannot be zero") - } + // 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) } - if a.Bidder.Empty() { + // NOTE: bidder can be nil for Surplus and Collateral auctions + if a.Bidder != nil && a.Bidder.Empty() { return errors.New("auction bidder cannot be empty") } - if len(a.Bidder) != sdk.AddrLen { + if a.Bidder != nil && 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() { diff --git a/x/auction/types/auctions_test.go b/x/auction/types/auctions_test.go index e19e7423..decb2cc7 100644 --- a/x/auction/types/auctions_test.go +++ b/x/auction/types/auctions_test.go @@ -107,11 +107,6 @@ func TestBaseAuctionValidate(t *testing.T) { }, true, }, - { - "0 id", - BaseAuction{ID: 0}, - false, - }, { "blank initiator", BaseAuction{ @@ -135,7 +130,7 @@ func TestBaseAuctionValidate(t *testing.T) { ID: 1, Initiator: testAccAddress1, Lot: c("kava", 1), - Bidder: nil, + Bidder: sdk.AccAddress{}, }, false, }, From 9932169a6786e2a58ef9bc5d9c4195246c9be8b0 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Wed, 13 May 2020 12:05:55 -0400 Subject: [PATCH 6/8] sim fixes --- x/auction/types/auctions.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/x/auction/types/auctions.go b/x/auction/types/auctions.go index 439b64e8..fe9ec321 100644 --- a/x/auction/types/auctions.go +++ b/x/auction/types/auctions.go @@ -68,18 +68,15 @@ func (a BaseAuction) GetType() string { return "base" } // Validate verifies that the auction end time is before max end time func (a BaseAuction) Validate() error { - // ID can be 0 for surplus, Debt and collateral auctions + // 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 nil for Surplus and Collateral auctions - if a.Bidder != nil && a.Bidder.Empty() { - return errors.New("auction bidder cannot be empty") - } - if a.Bidder != nil && len(a.Bidder) != sdk.AddrLen { + // 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() { @@ -196,7 +193,7 @@ func NewDebtAuction(buyerModAccName string, bid sdk.Coin, initialLot sdk.Coin, e Lot: initialLot, Bidder: supply.NewModuleAddress(buyerModAccName), // send proceeds from the first bid to the buyer. Bid: bid, // amount that the buyer is buying - doesn't change over course of auction - HasReceivedBids: false, // new auctions don't have any bids + HasReceivedBids: true, // new auctions don't have any bids EndTime: endTime, MaxEndTime: endTime, }, From 6a07def455f906ff948734754ba163edc37c14ef Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Fri, 15 May 2020 12:43:52 -0400 Subject: [PATCH 7/8] remove test case --- x/auction/types/auctions.go | 8 +------- x/auction/types/auctions_test.go | 28 ---------------------------- 2 files changed, 1 insertion(+), 35 deletions(-) diff --git a/x/auction/types/auctions.go b/x/auction/types/auctions.go index fe9ec321..ffdbcac4 100644 --- a/x/auction/types/auctions.go +++ b/x/auction/types/auctions.go @@ -88,12 +88,6 @@ func (a BaseAuction) Validate() error { if a.EndTime.After(a.MaxEndTime) { return fmt.Errorf("MaxEndTime < EndTime (%s < %s)", a.MaxEndTime, a.EndTime) } - if a.HasReceivedBids && !a.Bid.IsPositive() { - return fmt.Errorf("cannot have a zero value bid when auction has a true HasReceivedBids flag value") - } - if !a.HasReceivedBids && a.Bid.IsPositive() { - return fmt.Errorf("cannot have a positive value bid when auction has a false HasReceivedBids flag value") - } return nil } @@ -193,7 +187,7 @@ func NewDebtAuction(buyerModAccName string, bid sdk.Coin, initialLot sdk.Coin, e Lot: initialLot, Bidder: supply.NewModuleAddress(buyerModAccName), // send proceeds from the first bid to the buyer. Bid: bid, // amount that the buyer is buying - doesn't change over course of auction - HasReceivedBids: true, // new auctions don't have any bids + HasReceivedBids: true, EndTime: endTime, MaxEndTime: endTime, }, diff --git a/x/auction/types/auctions_test.go b/x/auction/types/auctions_test.go index 0a1cdd60..b857fc18 100644 --- a/x/auction/types/auctions_test.go +++ b/x/auction/types/auctions_test.go @@ -196,34 +196,6 @@ func TestBaseAuctionValidate(t *testing.T) { }, false, }, - { - "zero bid with received bids", - BaseAuction{ - ID: 1, - Initiator: testAccAddress1, - Lot: c("kava", 1), - Bidder: addr1, - Bid: c("kava", 0), - EndTime: now, - MaxEndTime: now, - HasReceivedBids: true, - }, - false, - }, - { - "positive bid without receiving bids", - BaseAuction{ - ID: 1, - Initiator: testAccAddress1, - Lot: c("kava", 1), - Bidder: addr1, - Bid: c("kava", 1), - EndTime: now, - MaxEndTime: now, - HasReceivedBids: false, - }, - false, - }, } for _, tc := range tests { From 5069aebda704854d5f09c6265c0f32e11d737be8 Mon Sep 17 00:00:00 2001 From: Federico Kunze Date: Fri, 15 May 2020 12:49:54 -0400 Subject: [PATCH 8/8] minor fix --- x/auction/types/auctions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auction/types/auctions.go b/x/auction/types/auctions.go index ffdbcac4..7af97e6d 100644 --- a/x/auction/types/auctions.go +++ b/x/auction/types/auctions.go @@ -187,7 +187,7 @@ func NewDebtAuction(buyerModAccName string, bid sdk.Coin, initialLot sdk.Coin, e Lot: initialLot, Bidder: supply.NewModuleAddress(buyerModAccName), // send proceeds from the first bid to the buyer. Bid: bid, // amount that the buyer is buying - doesn't change over course of auction - HasReceivedBids: true, + HasReceivedBids: false, // new auctions don't have any bids EndTime: endTime, MaxEndTime: endTime, },