fix(x/ecocredit): add missing transfer event and add comments

x/ecocredit/marketplace/keeper/features/msg_buy_direct.feature

@@ -343,8 +343,23 @@ Feature: Msg/BuyDirect
 
     Background:
       Given a credit type
+      And alice's address "regen1nzh226hxrsvf4k69sa8v0nfuzx5vgwkczk8j68"
       And bob's address "regen1depk54cuajgkzea6zpgkq36tnjwdzv4ak663u6"
 
+    Scenario: EventTransfer is emitted
+      Given alice created a sell order with id "1"
+      When bob attempts to buy credits with quantity "10"
+      Then expect event transfer with properties
+      """
+      {
+        "sender": "regen1nzh226hxrsvf4k69sa8v0nfuzx5vgwkczk8j68",
+        "recipient": "regen1depk54cuajgkzea6zpgkq36tnjwdzv4ak663u6",
+        "batch_denom": "C01-001-20200101-20210101-001",
+        "tradable_amount": "0",
+        "retired_amount": "10"
+      }
+      """
+
     Scenario: EventRetire is emitted
       Given alice created a sell order with id "1"
       When bob attempts to buy credits with sell order id "1" and retirement reason "offsetting electricity consumption"

x/ecocredit/marketplace/keeper/msg_buy_direct.go

@@ -30,7 +30,10 @@ func (k Keeper) BuyDirect(ctx context.Context, req *types.MsgBuyDirect) (*types.
 
 		sellOrder, err := k.stateStore.SellOrderTable().Get(ctx, order.SellOrderId)
 		if err != nil {
-			return nil, sdkerrors.ErrInvalidRequest.Wrapf("%s: sell order with id %d: %s", orderIndex, order.SellOrderId, err.Error())
+			return nil, sdkerrors.ErrInvalidRequest.Wrapf(
+				"%s: sell order with id %d: %s",
+				orderIndex, order.SellOrderId, err,
+			)
 		}
 
 		// check if buyer account is equal to seller account
@@ -56,7 +59,7 @@ func (k Keeper) BuyDirect(ctx context.Context, req *types.MsgBuyDirect) (*types.
 		if err != nil {
 			return nil, err
 		}
-		creditOrderQty, err := math.NewPositiveFixedDecFromString(order.Quantity, ct.Precision)
+		buyQuantity, err := math.NewPositiveFixedDecFromString(order.Quantity, ct.Precision)
 		if err != nil {
 			return nil, sdkerrors.ErrInvalidRequest.Wrapf(
 				"%s: decimal places exceeds precision: quantity: %s, credit type precision: %d",
@@ -67,7 +70,7 @@ func (k Keeper) BuyDirect(ctx context.Context, req *types.MsgBuyDirect) (*types.
 		// check that bid price and ask price denoms match
 		market, err := k.stateStore.MarketTable().Get(ctx, sellOrder.MarketId)
 		if err != nil {
-			return nil, sdkerrors.ErrInvalidRequest.Wrapf("market id %d: %s", sellOrder.MarketId, err.Error())
+			return nil, sdkerrors.ErrInvalidRequest.Wrapf("market id %d: %s", sellOrder.MarketId, err)
 		}
 		if order.BidPrice.Denom != market.BankDenom {
 			return nil, sdkerrors.ErrInvalidRequest.Wrapf(
@@ -90,21 +93,27 @@ func (k Keeper) BuyDirect(ctx context.Context, req *types.MsgBuyDirect) (*types.
 		}
 
 		// check address has the total cost (price per * order quantity)
-		bal := k.bankKeeper.GetBalance(sdkCtx, buyerAcc, order.BidPrice.Denom)
-		cost, err := getTotalCost(sellOrderAskAmount, creditOrderQty)
+		buyerBalance := k.bankKeeper.GetBalance(sdkCtx, buyerAcc, order.BidPrice.Denom)
+		cost, err := getTotalCost(sellOrderAskAmount, buyQuantity)
 		if err != nil {
 			return nil, err
 		}
-		coinCost := sdk.Coin{Amount: cost, Denom: market.BankDenom}
-		if bal.IsLT(coinCost) {
+		totalCost := sdk.Coin{Amount: cost, Denom: market.BankDenom}
+		if buyerBalance.IsLT(totalCost) {
 			return nil, sdkerrors.ErrInsufficientFunds.Wrapf(
 				"%s: quantity: %s, ask price: %s%s, total price: %v, bank balance: %v",
-				orderIndex, order.Quantity, sellOrder.AskAmount, market.BankDenom, coinCost, bal,
+				orderIndex, order.Quantity, sellOrder.AskAmount, market.BankDenom, totalCost, buyerBalance,
 			)
 		}
 
-		// fill the order, updating balances and the sell order in state
-		if err = k.fillOrder(ctx, orderIndex, sellOrder, buyerAcc, creditOrderQty, coinCost, orderOptions{
+		// fillOrder updates seller balance, buyer balance, batch supply, and transfers calculated
+		// total cost from buyer account to seller account.
+		if err = k.fillOrder(ctx, fillOrderParams{
+			orderIndex:   orderIndex,
+			sellOrder:    sellOrder,
+			buyerAcc:     buyerAcc,
+			buyQuantity:  buyQuantity,
+			totalCost:    totalCost,
 			autoRetire:   !order.DisableAutoRetire,
 			batchDenom:   batch.Denom,
 			jurisdiction: order.RetirementJurisdiction,

x/ecocredit/marketplace/keeper/msg_buy_direct_test.go

@@ -160,6 +160,12 @@ func (s *buyDirectSuite) TheBatchSupply(a gocuke.DocString) {
 	require.NoError(s.t, err)
 }
 
+func (s *buyDirectSuite) AlicesAddress(a string) {
+	addr, err := sdk.AccAddressFromBech32(a)
+	require.NoError(s.t, err)
+	s.alice = addr
+}
+
 func (s *buyDirectSuite) BobsAddress(a string) {
 	addr, err := sdk.AccAddressFromBech32(a)
 	require.NoError(s.t, err)
@@ -551,6 +557,18 @@ func (s *buyDirectSuite) ExpectEventBuyDirectWithProperties(a gocuke.DocString)
 	require.NoError(s.t, err)
 }
 
+func (s *buyDirectSuite) ExpectEventTransferWithProperties(a gocuke.DocString) {
+	var event basetypes.EventTransfer
+	err := json.Unmarshal([]byte(a.Content), &event)
+	require.NoError(s.t, err)
+
+	sdkEvent, found := testutil.GetEvent(&event, s.sdkCtx.EventManager().Events())
+	require.True(s.t, found)
+
+	err = testutil.MatchEvent(&event, sdkEvent)
+	require.NoError(s.t, err)
+}
+
 func (s *buyDirectSuite) ExpectEventRetireWithProperties(a gocuke.DocString) {
 	var event basetypes.EventRetire
 	err := json.Unmarshal([]byte(a.Content), &event)

x/ecocredit/marketplace/keeper/utils.go

@@ -14,161 +14,203 @@ import (
 	"github.com/regen-network/regen-ledger/x/ecocredit/v3/server/utils"
 )
 
-// isDenomAllowed checks if the denom is allowed to be used in orders.
+// isDenomAllowed checks if the denom is allowed to be used in sell orders.
 func isDenomAllowed(ctx context.Context, bankDenom string, table api.AllowedDenomTable) (bool, error) {
 	return table.Has(ctx, bankDenom)
 }
 
-type orderOptions struct {
+type fillOrderParams struct {
+	orderIndex   string
+	sellOrder    *api.SellOrder
+	buyerAcc     sdk.AccAddress
+	buyQuantity  math.Dec
+	totalCost    sdk.Coin
 	autoRetire   bool
 	batchDenom   string
 	jurisdiction string
 	reason       string
 }
 
-// fillOrder moves credits and coins according to the order. It will:
-// - update a sell order, removing it if quantity becomes 0 as a result of this purchase.
-// - remove the purchaseQty from the seller's escrowed balance.
-// - add credits to the buyer's tradable/retired address (based on the DisableAutoRetire field).
-// - update the supply accordingly.
-// - send the coins specified in the bid to the seller.
-func (k Keeper) fillOrder(ctx context.Context, orderIndex string, sellOrder *api.SellOrder, buyerAcc sdk.AccAddress, purchaseQty math.Dec,
-	cost sdk.Coin, opts orderOptions) error {
+// fillOrder updates seller balance, buyer balance, batch supply, and transfers calculated total cost
+// from buyer account to seller account.
+func (k Keeper) fillOrder(ctx context.Context, params fillOrderParams) error {
 	sdkCtx := sdk.UnwrapSDKContext(ctx)
-	sellOrderQty, err := math.NewDecFromString(sellOrder.Quantity)
+
+	// get sell order quantity to be checked and/or updated
+	sellOrderQty, err := math.NewDecFromString(params.sellOrder.Quantity)
 	if err != nil {
 		return err
 	}
 
-	switch sellOrderQty.Cmp(purchaseQty) {
+	// If the sell order quantity is less than the purchase quantity, return an error.
+	// If the sell order quantity is equal to the purchase quantity, remove the sell order.
+	// If the sell order quantity is greater than the purchase quantity, subtract the purchase
+	// quantity from the sell order quantity and update the sell order.
+	switch sellOrderQty.Cmp(params.buyQuantity) {
 	case math.LessThan:
 		return sdkerrors.ErrInvalidRequest.Wrapf(
 			"%s: requested quantity: %v, sell order quantity %s",
-			orderIndex, purchaseQty, sellOrder.Quantity,
+			params.orderIndex, params.buyQuantity, params.sellOrder.Quantity,
 		)
 	case math.EqualTo:
-		if err := k.stateStore.SellOrderTable().Delete(ctx, sellOrder); err != nil {
+		if err := k.stateStore.SellOrderTable().Delete(ctx, params.sellOrder); err != nil {
 			return err
 		}
 	case math.GreaterThan:
-		newSellOrderQty, err := sellOrderQty.Sub(purchaseQty)
+		newSellOrderQty, err := sellOrderQty.Sub(params.buyQuantity)
 		if err != nil {
 			return err
 		}
-		sellOrder.Quantity = newSellOrderQty.String()
-		if err = k.stateStore.SellOrderTable().Update(ctx, sellOrder); err != nil {
+		params.sellOrder.Quantity = newSellOrderQty.String()
+		if err = k.stateStore.SellOrderTable().Update(ctx, params.sellOrder); err != nil {
 			return err
 		}
 	}
 
-	// remove the credits from the seller's escrowed balance
-	sellerBal, err := k.baseStore.BatchBalanceTable().Get(ctx, sellOrder.Seller, sellOrder.BatchKey)
+	// calculate and set seller balance escrowed amount (subtract credits)
+	sellerBal, err := k.baseStore.BatchBalanceTable().Get(ctx, params.sellOrder.Seller, params.sellOrder.BatchKey)
 	if err != nil {
 		return err
 	}
 	escrowBal, err := math.NewDecFromString(sellerBal.EscrowedAmount)
 	if err != nil {
 		return err
 	}
-	escrowBal, err = math.SafeSubBalance(escrowBal, purchaseQty)
+	escrowBal, err = math.SafeSubBalance(escrowBal, params.buyQuantity)
 	if err != nil {
 		return err
 	}
 	sellerBal.EscrowedAmount = escrowBal.String()
+
+	// update seller balance with new escrowed amount
 	if err = k.baseStore.BatchBalanceTable().Update(ctx, sellerBal); err != nil {
 		return err
 	}
 
-	// update the buyers balance and the batch supply
-	supply, err := k.baseStore.BatchSupplyTable().Get(ctx, sellOrder.BatchKey)
-	if err != nil {
-		return err
-	}
-	buyerBal, err := utils.GetBalance(ctx, k.baseStore.BatchBalanceTable(), buyerAcc, sellOrder.BatchKey)
+	// get buyer balance to be updated
+	buyerBal, err := utils.GetBalance(ctx, k.baseStore.BatchBalanceTable(), params.buyerAcc, params.sellOrder.BatchKey)
 	if err != nil {
 		return err
 	}
-	// if auto retire is disabled, we move the credits into the buyer's tradable balance.
-	// supply is not updated because supply does not distinguish between tradable and escrowed credits.
-	if !opts.autoRetire {
+
+	// If auto retire is disabled, we update buyer balance tradable amount. We emit a transfer event with
+	// the credit quantity being purchased as the tradable amount. We do not update batch supply because
+	// we do not distinguish between tradable and escrowed credits in batch supply.
+	//
+	// If auto-retire is enabled, we update buyer balance retired amount. We emit a transfer event with the
+	// credit quantity being purchases as the retired amount and a retire event with the credit quantity as
+	// the amount. We also update batch supply to reflect the retirement of the credits.
+	if !params.autoRetire {
+
+		// calculate and set buyer balance tradable amount (add credits)
 		tradableBalance, err := math.NewDecFromString(buyerBal.TradableAmount)
 		if err != nil {
 			return err
 		}
-		tradableBalance, err = math.SafeAddBalance(tradableBalance, purchaseQty)
+		tradableBalance, err = math.SafeAddBalance(tradableBalance, params.buyQuantity)
 		if err != nil {
 			return err
 		}
 		buyerBal.TradableAmount = tradableBalance.String()
+
+		// emit transfer event with purchase quantity as tradable amount
 		if err = sdkCtx.EventManager().EmitTypedEvent(&basetypes.EventTransfer{
-			Sender:         sdk.AccAddress(sellOrder.Seller).String(),
-			Recipient:      buyerAcc.String(),
-			BatchDenom:     opts.batchDenom,
-			TradableAmount: purchaseQty.String(),
-			RetiredAmount:  "0",
+			Sender:         sdk.AccAddress(params.sellOrder.Seller).String(),
+			Recipient:      params.buyerAcc.String(),
+			BatchDenom:     params.batchDenom,
+			TradableAmount: params.buyQuantity.String(),
+			RetiredAmount:  "0", // add zero to prevent empty string
 		}); err != nil {
 			return err
 		}
 	} else {
+
+		// calculate and set buyer balance retired amount (add credits)
 		retiredBalance, err := math.NewDecFromString(buyerBal.RetiredAmount)
 		if err != nil {
 			return err
 		}
-		retiredBalance, err = math.SafeAddBalance(retiredBalance, purchaseQty)
+		retiredBalance, err = math.SafeAddBalance(retiredBalance, params.buyQuantity)
 		if err != nil {
 			return err
 		}
 		buyerBal.RetiredAmount = retiredBalance.String()
 
+		// get batch supply to be updated
+		supply, err := k.baseStore.BatchSupplyTable().Get(ctx, params.sellOrder.BatchKey)
+		if err != nil {
+			return err
+		}
+
+		// calculate and set batch supply tradable amount (subtract credits)
 		supplyTradable, err := math.NewDecFromString(supply.TradableAmount)
 		if err != nil {
 			return err
 		}
-		supplyTradable, err = math.SafeSubBalance(supplyTradable, purchaseQty)
+		supplyTradable, err = math.SafeSubBalance(supplyTradable, params.buyQuantity)
 		if err != nil {
 			return err
 		}
 		supply.TradableAmount = supplyTradable.String()
 
+		// calculate and set batch supply retired amount (add credits)
 		supplyRetired, err := math.NewDecFromString(supply.RetiredAmount)
 		if err != nil {
 			return err
 		}
-		supplyRetired, err = math.SafeAddBalance(supplyRetired, purchaseQty)
+		supplyRetired, err = math.SafeAddBalance(supplyRetired, params.buyQuantity)
 		if err != nil {
 			return err
 		}
 		supply.RetiredAmount = supplyRetired.String()
+
+		// update batch supply with new tradable and retired amount
 		if err = k.baseStore.BatchSupplyTable().Update(ctx, supply); err != nil {
 			return err
 		}
+
+		// emit transfer event with purchase quantity as retired amount
+		if err = sdkCtx.EventManager().EmitTypedEvent(&basetypes.EventTransfer{
+			Sender:         sdk.AccAddress(params.sellOrder.Seller).String(),
+			Recipient:      params.buyerAcc.String(),
+			BatchDenom:     params.batchDenom,
+			TradableAmount: "0", // add zero to prevent empty string
+			RetiredAmount:  params.buyQuantity.String(),
+		}); err != nil {
+			return err
+		}
+
+		// emit retire event with purchase quantity as amount
 		if err = sdkCtx.EventManager().EmitTypedEvent(&basetypes.EventRetire{
-			Owner:        buyerAcc.String(),
-			BatchDenom:   opts.batchDenom,
-			Amount:       purchaseQty.String(),
-			Jurisdiction: opts.jurisdiction,
-			Reason:       opts.reason,
+			Owner:        params.buyerAcc.String(),
+			BatchDenom:   params.batchDenom,
+			Amount:       params.buyQuantity.String(),
+			Jurisdiction: params.jurisdiction,
+			Reason:       params.reason,
 		}); err != nil {
 			return err
 		}
 	}
+
+	// update buyer balance with new tradable or retired amount
 	if err = k.baseStore.BatchBalanceTable().Save(ctx, buyerBal); err != nil {
 		return err
 	}
 
-	return k.bankKeeper.SendCoins(sdkCtx, buyerAcc, sellOrder.Seller, sdk.NewCoins(cost))
+	// send total cost from buyer account to seller account
+	return k.bankKeeper.SendCoins(sdkCtx, params.buyerAcc, params.sellOrder.Seller, sdk.NewCoins(params.totalCost))
 }
 
-// getTotalCost calculates the cost of the order by multiplying the price per credit, and the amount of credits
-// desired in the order.
-func getTotalCost(pricePerCredit sdkmath.Int, amtCredits math.Dec) (sdkmath.Int, error) {
-	unitPrice, err := math.NewPositiveFixedDecFromString(pricePerCredit.String(), amtCredits.NumDecimalPlaces())
+// getTotalCost calculates the total cost of the buy order by multiplying the price per credit specified
+// in the sell order (i.e. the ask amount) and the quantity of credits specified in the buy order.
+func getTotalCost(askAmount sdkmath.Int, buyQuantity math.Dec) (sdkmath.Int, error) {
+	unitPrice, err := math.NewPositiveFixedDecFromString(askAmount.String(), buyQuantity.NumDecimalPlaces())
 	if err != nil {
 		return sdkmath.Int{}, err
 	}
-	cost, err := amtCredits.Mul(unitPrice)
+	totalCost, err := buyQuantity.Mul(unitPrice)
 	if err != nil {
 		return sdkmath.Int{}, err
 	}
-	return cost.SdkIntTrim(), nil
+	return totalCost.SdkIntTrim(), nil
 }