use only one coins field in MsgPlaceBid

x/auction/abci_test.go

@@ -39,7 +39,7 @@ func TestKeeper_EndBlocker(t *testing.T) {
 
 	auctionID, err := keeper.StartForwardReverseAuction(ctx, sellerModName, c("token1", 20), c("token2", 50), returnAddrs, returnWeights)
 	require.NoError(t, err)
-	require.NoError(t, keeper.PlaceBid(ctx, auctionID, buyer, c("token2", 30), c("token1", 20)))
+	require.NoError(t, keeper.PlaceBid(ctx, auctionID, buyer, c("token2", 30)))
 
 	// Run the endblocker, simulating a block time 1ns before auction expiry
 	preExpiryTime := ctx.BlockTime().Add(auction.DefaultBidDuration - 1)

x/auction/client/cli/tx.go

@@ -33,9 +33,9 @@ func GetTxCmd(cdc *codec.Codec) *cobra.Command {
 // GetCmdPlaceBid cli command for creating and modifying cdps.
 func GetCmdPlaceBid(cdc *codec.Codec) *cobra.Command {
 	return &cobra.Command{
-		Use:   "placebid [AuctionID] [Bidder] [Bid] [Lot]",
+		Use:   "placebid [auctionID] [amount]",
 		Short: "place a bid on an auction",
-		Args:  cobra.ExactArgs(4),
+		Args:  cobra.MinimumNArgs(2),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			cliCtx := context.NewCLIContext().WithCodec(cdc)
 			txBldr := auth.NewTxBuilderFromCLI().WithTxEncoder(utils.GetTxEncoder(cdc))
@@ -46,18 +46,13 @@ func GetCmdPlaceBid(cdc *codec.Codec) *cobra.Command {
 				return err
 			}
 
-			bid, err := sdk.ParseCoin(args[2])
+			amt, err := sdk.ParseCoin(args[2])
 			if err != nil {
-				fmt.Printf("invalid bid amount - %s \n", string(args[2]))
+				fmt.Printf("invalid amount - %s \n", string(args[2]))
 				return err
 			}
 
-			lot, err := sdk.ParseCoin(args[3])
+			msg := types.NewMsgPlaceBid(id, cliCtx.GetFromAddress(), amt)
-			if err != nil {
-				fmt.Printf("invalid lot - %s \n", string(args[3]))
-				return err
-			}
-			msg := types.NewMsgPlaceBid(id, cliCtx.GetFromAddress(), bid, lot)
 			err = msg.ValidateBasic()
 			if err != nil {
 				return err

x/auction/client/rest/tx.go

@@ -33,7 +33,7 @@ const (
 
 func registerTxRoutes(cliCtx context.CLIContext, r *mux.Router) {
 	r.HandleFunc(
-		fmt.Sprintf("/auction/bid/{%s}/{%s}/{%s}/{%s}", restAuctionID, restBidder, restBid, restLot), bidHandlerFn(cliCtx)).Methods("PUT")
+		fmt.Sprintf("/auction/bid/{%s}/{%s}/{%s}", restAuctionID, restBidder, restBid), bidHandlerFn(cliCtx)).Methods("PUT")
 }
 
 func bidHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
@@ -44,7 +44,6 @@ func bidHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
 		strAuctionID := vars[restAuctionID]
 		bechBidder := vars[restBidder]
 		strBid := vars[restBid]
-		strLot := vars[restLot]
 
 		auctionID, err := strconv.ParseUint(strAuctionID, 10, 64)
 		if err != nil {
@@ -64,13 +63,7 @@ func bidHandlerFn(cliCtx context.CLIContext) http.HandlerFunc {
 			return
 		}
 
-		lot, err := sdk.ParseCoin(strLot)
+		msg := types.NewMsgPlaceBid(auctionID, bidder, bid)
-		if err != nil {
-			rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
-			return
-		}
-
-		msg := types.NewMsgPlaceBid(auctionID, bidder, bid, lot)
 		if err := msg.ValidateBasic(); err != nil {
 			rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
 			return

x/auction/handler.go

@@ -21,7 +21,7 @@ func NewHandler(keeper Keeper) sdk.Handler {
 
 func handleMsgPlaceBid(ctx sdk.Context, keeper Keeper, msg MsgPlaceBid) sdk.Result {
 
-	err := keeper.PlaceBid(ctx, msg.AuctionID, msg.Bidder, msg.Bid, msg.Lot)
+	err := keeper.PlaceBid(ctx, msg.AuctionID, msg.Bidder, msg.Amount)
 	if err != nil {
 		return err.Result()
 	}

x/auction/keeper/auctions.go

@@ -68,8 +68,7 @@ func (k Keeper) StartForwardReverseAuction(ctx sdk.Context, seller string, lot s
 }
 
 // PlaceBid places a bid on any auction.
-// TODO passing bid and lot is weird when only one needed
+func (k Keeper) PlaceBid(ctx sdk.Context, auctionID uint64, bidder sdk.AccAddress, newAmount sdk.Coin) sdk.Error {
-func (k Keeper) PlaceBid(ctx sdk.Context, auctionID uint64, bidder sdk.AccAddress, bid sdk.Coin, lot sdk.Coin) sdk.Error {
 
 	// get auction from store
 	auction, found := k.GetAuction(ctx, auctionID)
@@ -81,32 +80,26 @@ func (k Keeper) PlaceBid(ctx sdk.Context, auctionID uint64, bidder sdk.AccAddres
 	if ctx.BlockTime().After(auction.GetEndTime()) {
 		return sdk.ErrInternal("auction has closed")
 	}
-	if auction.GetBid().Denom != bid.Denom {
-		return sdk.ErrInternal("bid has incorrect denom")
-	}
-	if auction.GetLot().Denom != lot.Denom {
-		return sdk.ErrInternal("lot has incorrect denom")
-	}
 
 	// place bid
 	var err sdk.Error
 	var a types.Auction
 	switch auc := auction.(type) {
 	case types.ForwardAuction:
-		a, err = k.PlaceBidForward(ctx, auc, bidder, bid)
+		a, err = k.PlaceBidForward(ctx, auc, bidder, newAmount)
 		if err != nil {
 			return err
 		}
 	case types.ReverseAuction:
-		a, err = k.PlaceBidReverse(ctx, auc, bidder, lot)
+		a, err = k.PlaceBidReverse(ctx, auc, bidder, newAmount)
 		if err != nil {
 			return err
 		}
 	case types.ForwardReverseAuction:
 		if !auc.IsReversePhase() {
-			a, err = k.PlaceBidForwardReverseForward(ctx, auc, bidder, bid)
+			a, err = k.PlaceBidForwardReverseForward(ctx, auc, bidder, newAmount)
 		} else {
-			a, err = k.PlaceBidForwardReverseReverse(ctx, auc, bidder, lot)
+			a, err = k.PlaceBidForwardReverseReverse(ctx, auc, bidder, newAmount)
 		}
 		if err != nil {
 			return err
@@ -163,6 +156,9 @@ func (k Keeper) PlaceBidForward(ctx sdk.Context, a types.ForwardAuction, bidder
 // TODO naming
 func (k Keeper) PlaceBidForwardReverseForward(ctx sdk.Context, a types.ForwardReverseAuction, bidder sdk.AccAddress, bid sdk.Coin) (types.ForwardReverseAuction, sdk.Error) {
 	// Validate bid
+	if bid.Denom != a.Bid.Denom {
+		return a, sdk.ErrInternal("bid denom doesn't match auction")
+	}
 	if a.IsReversePhase() {
 		return a, sdk.ErrInternal("auction is not in forward phase")
 	}
@@ -201,6 +197,9 @@ func (k Keeper) PlaceBidForwardReverseForward(ctx sdk.Context, a types.ForwardRe
 
 func (k Keeper) PlaceBidForwardReverseReverse(ctx sdk.Context, a types.ForwardReverseAuction, bidder sdk.AccAddress, lot sdk.Coin) (types.ForwardReverseAuction, sdk.Error) {
 	// Validate bid
+	if lot.Denom != a.Lot.Denom {
+		return a, sdk.ErrInternal("lot denom doesn't match auction")
+	}
 	if !a.IsReversePhase() {
 		return a, sdk.ErrInternal("auction not in reverse phase")
 	}

x/auction/keeper/auctions_test.go

@@ -43,14 +43,14 @@ func TestForwardAuctionBasic(t *testing.T) {
 	tApp.CheckBalance(t, ctx, sellerAddr, cs(c("token1", 80), c("token2", 100)))
 
 	// PlaceBid (bid: 10 token, lot: same as starting)
-	require.NoError(t, keeper.PlaceBid(ctx, auctionID, buyer, c("token2", 10), c("token1", 20))) // bid, lot
+	require.NoError(t, keeper.PlaceBid(ctx, auctionID, buyer, c("token2", 10)))
 	// Check buyer's coins have decreased
 	tApp.CheckBalance(t, ctx, buyer, cs(c("token1", 100), c("token2", 90)))
 	// Check seller's coins have not increased (because proceeds are burned)
 	tApp.CheckBalance(t, ctx, sellerAddr, cs(c("token1", 80), c("token2", 100)))
 
 	// increment bid same bidder
-	err = keeper.PlaceBid(ctx, auctionID, buyer, c("token2", 20), c("token1", 20))
+	err = keeper.PlaceBid(ctx, auctionID, buyer, c("token2", 20))
 	require.NoError(t, err)
 
 	// Close auction at just at auction expiry time
@@ -85,7 +85,7 @@ func TestReverseAuctionBasic(t *testing.T) {
 	tApp.CheckBalance(t, ctx, buyerAddr, nil) // zero coins
 
 	// Place a bid
-	require.NoError(t, keeper.PlaceBid(ctx, 0, seller, c("token1", 20), c("token2", 10))) // bid, lot
+	require.NoError(t, keeper.PlaceBid(ctx, 0, seller, c("token2", 10)))
 	// Check seller's coins have decreased
 	tApp.CheckBalance(t, ctx, seller, cs(c("token1", 80), c("token2", 100)))
 	// Check buyer's coins have increased
@@ -129,7 +129,7 @@ func TestForwardReverseAuctionBasic(t *testing.T) {
 	tApp.CheckBalance(t, ctx, sellerAddr, cs(c("token1", 80), c("token2", 100)))
 
 	// Place a forward bid
-	require.NoError(t, keeper.PlaceBid(ctx, 0, buyer, c("token2", 10), c("token1", 20))) // bid, lot
+	require.NoError(t, keeper.PlaceBid(ctx, 0, buyer, c("token2", 10)))
 	// Check bidder's coins have decreased
 	tApp.CheckBalance(t, ctx, buyer, cs(c("token1", 100), c("token2", 90)))
 	// Check seller's coins have increased
@@ -140,8 +140,8 @@ func TestForwardReverseAuctionBasic(t *testing.T) {
 	}
 
 	// Place a reverse bid
-	require.NoError(t, keeper.PlaceBid(ctx, 0, buyer, c("token2", 50), c("token1", 15))) // first bid up to max bid to switch phases
+	require.NoError(t, keeper.PlaceBid(ctx, 0, buyer, c("token2", 50))) // first bid up to max bid to switch phases
-	require.NoError(t, keeper.PlaceBid(ctx, 0, buyer, c("token2", 50), c("token1", 15)))
+	require.NoError(t, keeper.PlaceBid(ctx, 0, buyer, c("token1", 15)))
 	// Check bidder's coins have decreased
 	tApp.CheckBalance(t, ctx, buyer, cs(c("token1", 100), c("token2", 50)))
 	// Check seller's coins have increased

x/auction/types/msg.go

@@ -9,17 +9,15 @@ var _ sdk.Msg = &MsgPlaceBid{}
 type MsgPlaceBid struct {
 	AuctionID uint64
 	Bidder    sdk.AccAddress // This can be a buyer (who increments bid), or a seller (who decrements lot) TODO rename to be clearer?
-	Bid       sdk.Coin
+	Amount    sdk.Coin       // The new bid or lot to set on the auction
-	Lot       sdk.Coin
 }
 
 // NewMsgPlaceBid returns a new MsgPlaceBid.
-func NewMsgPlaceBid(auctionID uint64, bidder sdk.AccAddress, bid sdk.Coin, lot sdk.Coin) MsgPlaceBid {
+func NewMsgPlaceBid(auctionID uint64, bidder sdk.AccAddress, amt sdk.Coin) MsgPlaceBid {
 	return MsgPlaceBid{
 		AuctionID: auctionID,
 		Bidder:    bidder,
-		Bid:       bid,
+		Amount:    amt,
-		Lot:       lot,
 	}
 }
 
@@ -29,18 +27,14 @@ func (msg MsgPlaceBid) Route() string { return RouterKey }
 // Type returns a human-readable string for the message, intended for utilization within tags.
 func (msg MsgPlaceBid) Type() string { return "place_bid" }
 
-// ValidateBasic does a simple validation check that doesn't require access to any other information.
+// ValidateBasic does a simple validation check that doesn't require access to state.
 func (msg MsgPlaceBid) ValidateBasic() sdk.Error {
 	if msg.Bidder.Empty() {
 		return sdk.ErrInternal("invalid (empty) bidder address")
 	}
-	if msg.Bid.Amount.LT(sdk.ZeroInt()) {
+	if !msg.Amount.IsValid() {
-		return sdk.ErrInternal("invalid (negative) bid amount")
+		return sdk.ErrInternal("invalid bid amount")
 	}
-	if msg.Lot.Amount.LT(sdk.ZeroInt()) {
-		return sdk.ErrInternal("invalid (negative) lot amount")
-	}
-	// TODO check coin denoms
 	return nil
 }
 
@@ -53,4 +47,4 @@ func (msg MsgPlaceBid) GetSignBytes() []byte {
 // GetSigners returns the addresses of signers that must sign.
 func (msg MsgPlaceBid) GetSigners() []sdk.AccAddress {
 	return []sdk.AccAddress{msg.Bidder}
-}
+}

x/auction/types/msg_test.go

@@ -14,19 +14,29 @@ func TestMsgPlaceBid_ValidateBasic(t *testing.T) {
 		msg        MsgPlaceBid
 		expectPass bool
 	}{
-		{"normal", MsgPlaceBid{0, addr, sdk.NewInt64Coin("usdx", 10), sdk.NewInt64Coin("kava", 20)}, true},
+		{"normal",
-		{"emptyAddr", MsgPlaceBid{0, sdk.AccAddress{}, sdk.NewInt64Coin("usdx", 10), sdk.NewInt64Coin("kava", 20)}, false},
+			NewMsgPlaceBid(0, addr, c("token", 10)),
-		{"negativeBid", MsgPlaceBid{0, addr, sdk.Coin{"usdx", sdk.NewInt(-10)}, sdk.NewInt64Coin("kava", 20)}, false},
+			true},
-		{"negativeLot", MsgPlaceBid{0, addr, sdk.NewInt64Coin("usdx", 10), sdk.Coin{"kava", sdk.NewInt(-20)}}, false},
+		{"emptyAddr",
-		{"zerocoins", MsgPlaceBid{0, addr, sdk.NewInt64Coin("usdx", 0), sdk.NewInt64Coin("kava", 0)}, true},
+			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},
 	}
+
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			if tc.expectPass {
-				require.Nil(t, tc.msg.ValidateBasic())
+				require.NoError(t, tc.msg.ValidateBasic())
 			} else {
-				require.NotNil(t, tc.msg.ValidateBasic())
+				require.Error(t, tc.msg.ValidateBasic())
 			}
 		})
 	}
 }
+
+func c(denom string, amount int64) sdk.Coin { return sdk.NewInt64Coin(denom, amount) }