fix(x/ecocredit): fix create class fee if not required and update tes…

…ts (#1234)

* fix(x/ecocredit): fix create class fee if not required and update tests

* consistent basket and credit class fee

* consistent basket and credit class fee

* use regen address prefix

* fix imports and format

* address review comments

* revert and simplify

x/ecocredit/server/basket/features/msg_create.feature

@@ -2,10 +2,10 @@ Feature: Msg/Create
 
   A basket can be created:
   - when the basket name is unique
-  - when a minimum basket fee is not set
-  - when the basket fee denom matches a minimum basket fee denom
-  - when the basket fee amount is greater than or equal to a minimum basket fee amount
-  - when the user has a balance greater than or equal to a minimum basket fee amount
+  - when an allowed basket fee is not set and no fee is provided
+  - when the basket fee denom matches an allowed basket fee denom
+  - when the basket fee amount is greater than or equal to an allowed basket fee amount
+  - when the admin balance is greater than or equal to an allowed basket fee amount
   - when the basket includes a credit type that exists
   - when the basket criteria includes credit classes that exist
   - when the basket criteria includes credit classes that match the credit type
@@ -27,66 +27,66 @@ Feature: Msg/Create
       When alice attempts to create a basket with name "NCT"
       Then expect the error "basket with name NCT already exists: unique key violation"
 
-  Rule: The basket fee is not required if a minimum basket fee is not set
+  Rule: The basket fee is not required if an allowed basket fee is not set
 
     Background:
       Given a credit type
 
-    Scenario: basket fee provided
+    Scenario: basket fee provided and allowed basket fee not set
       Given alice has a token balance "20regen"
       When alice attempts to create a basket with fee "20regen"
       Then expect no error
 
-    Scenario: basket fee not provided
+    Scenario: basket fee not provided and allowed basket fee not set
       When alice attempts to create a basket with no fee
       Then expect no error
 
-    # No failing scenario - basket fee is not required if minimum basket fee is not set
+    # no failing scenario - basket fee is not required if allowed basket fee is not set
 
-  Rule: The basket fee must match a minimum basket fee denom
+  Rule: The basket fee must match an allowed basket fee denom
 
     Background:
       Given a credit type
       And alice has a token balance "20regen"
 
-    Scenario: basket fee matches the denom (single fee)
-      Given a minimum basket fee "20regen"
+    Scenario: basket fee matches allowed basket fee denom (single fee)
+      Given allowed basket fee "20regen"
       When alice attempts to create a basket with fee "20regen"
       Then expect no error
 
-    Scenario: basket fee matches the denom (multiple fees)
-      Given minimum basket fees "20regen,20atom"
+    Scenario: basket fee matches allowed basket fee denom (multiple fees)
+      Given allowed basket fee "20regen,20atom"
       When alice attempts to create a basket with fee "20regen"
       Then expect no error
 
-    Scenario: basket fee does not match the denom (single fee)
-      Given a minimum basket fee "20regen"
+    Scenario: basket fee does not match allowed basket fee denom (single fee)
+      Given allowed basket fee "20regen"
       When alice attempts to create a basket with fee "20atom"
-      Then expect the error "minimum fee 20regen, got 20atom: insufficient fee"
+      Then expect the error "fee must be 20regen, got 20atom: insufficient fee"
 
-    Scenario: basket fee does not match the denom (multiple fees)
-      Given minimum basket fees "20regen,20atom"
+    Scenario: basket fee does not match allowed basket fee denom (multiple fees)
+      Given allowed basket fee "20regen,20atom"
       When alice attempts to create a basket with fee "20stake"
-      Then expect the error "minimum fee one of 20atom,20regen, got 20stake: insufficient fee"
+      Then expect the error "fee must be one of 20atom,20regen, got 20stake: insufficient fee"
 
-    Scenario: basket fee not provided (single fee)
-      Given a minimum basket fee "20regen"
+    Scenario: basket fee not provided and allowed basket fee set (single fee)
+      Given allowed basket fee "20regen"
       When alice attempts to create a basket with no fee
-      Then expect the error "minimum fee 20regen, got : insufficient fee"
+      Then expect the error "fee cannot be empty: must be 20regen: insufficient fee"
 
-    Scenario: basket fee not provided (multiple fees)
-      Given minimum basket fees "20regen,20atom"
+    Scenario: basket fee not provided and allowed basket fee set (multiple fees)
+      Given allowed basket fee "20regen,20atom"
       When alice attempts to create a basket with no fee
-      Then expect the error "minimum fee one of 20atom,20regen, got : insufficient fee"
+      Then expect the error "fee cannot be empty: must be one of 20atom,20regen: insufficient fee"
 
-  Rule: The basket fee must be greater than or equal to a minimum basket fee
+  Rule: The basket fee must be greater than or equal to an allowed basket fee
 
     Background:
       Given a credit type
       And alice has a token balance "20regen"
 
-    Scenario Outline: basket fee is greater than or equal to minimum basket fee (single fee)
-      Given a minimum basket fee "20regen"
+    Scenario Outline: basket fee is greater than or equal to allowed basket fee amount (single fee)
+      Given allowed basket fee "20regen"
       When alice attempts to create a basket with fee "<basket-fee>"
       Then expect no error
 
@@ -95,8 +95,8 @@ Feature: Msg/Create
         | greater than | 30regen    |
         | equal to     | 20regen    |
 
-    Scenario Outline: basket fee is greater than or equal to minimum basket fee (multiple fees)
-      Given minimum basket fees "20regen,20atom"
+    Scenario Outline: basket fee is greater than or equal to allowed basket fee amount (multiple fees)
+      Given allowed basket fee "20regen,20atom"
       When alice attempts to create a basket with fee "<basket-fee>"
       Then expect no error
 
@@ -105,23 +105,23 @@ Feature: Msg/Create
         | greater than | 30regen    |
         | equal to     | 20regen    |
 
-    Scenario: basket fee is less than minimum basket fee (single fee)
-      Given a minimum basket fee "20regen"
+    Scenario: basket fee is less than allowed basket fee amount (single fee)
+      Given allowed basket fee "20regen"
       When alice attempts to create a basket with fee "10regen"
-      Then expect the error "minimum fee 20regen, got 10regen: insufficient fee"
+      Then expect the error "fee must be 20regen, got 10regen: insufficient fee"
 
-    Scenario: basket fee is less than minimum basket fee (multiple fees)
-      Given minimum basket fees "20regen,20atom"
+    Scenario: basket fee is less than allowed basket fee amount (multiple fees)
+      Given allowed basket fee "20regen,20atom"
       When alice attempts to create a basket with fee "10regen"
-      Then expect the error "minimum fee one of 20atom,20regen, got 10regen: insufficient fee"
+      Then expect the error "fee must be one of 20atom,20regen, got 10regen: insufficient fee"
 
-  Rule: The user must have a balance greater than or equal to the basket fee amount
+  Rule: The admin must have a balance greater than or equal to an allowed basket fee amount
 
     Background:
       Given a credit type
-      And a minimum basket fee "20regen"
+      And allowed basket fee "20regen"
 
-    Scenario Outline: user has greater than or equal to basket fee amount
+    Scenario Outline: admin balance is greater than or equal to allowed basket fee amount
       Given alice has a token balance "<token-balance>"
       When alice attempts to create a basket with fee "20regen"
       Then expect no error
@@ -131,10 +131,10 @@ Feature: Msg/Create
         | greater than | 30regen       |
         | equal to     | 20regen       |
 
-    Scenario: user has less than basket fee amount
+    Scenario: admin balance is less than allowed basket fee amount
       Given alice has a token balance "10regen"
       When alice attempts to create a basket with fee "20regen"
-      Then expect the error "insufficient balance for bank denom regen: insufficient funds"
+      Then expect the error "insufficient balance 10 for bank denom regen: insufficient funds"
 
   Rule: The basket must include a credit type that exists
 
@@ -182,7 +182,7 @@ Feature: Msg/Create
 
     Background:
       Given a credit type
-      And a minimum basket fee "20regen"
+      And allowed basket fee "20regen"
       And alice has a token balance "40regen"
 
     Scenario Outline: user token balance is updated

x/ecocredit/server/basket/msg_create.go

@@ -18,40 +18,61 @@ import (
 func (k Keeper) Create(ctx context.Context, msg *basket.MsgCreate) (*basket.MsgCreateResponse, error) {
 	sdkCtx := sdk.UnwrapSDKContext(ctx)
 
-	var fee sdk.Coins
-	k.paramsKeeper.Get(sdkCtx, core.KeyBasketFee, &fee)
+	var allowedFees sdk.Coins
+	k.paramsKeeper.Get(sdkCtx, core.KeyBasketFee, &allowedFees)
 
 	curator, err := sdk.AccAddressFromBech32(msg.Curator)
 	if err != nil {
 		return nil, err
 	}
 
-	// In the next version of the basket package, this field will be updated to
-	// a single Coin rather than a list of Coins. In the meantime, the message
-	// will fail basic validation if more than one Coin is provided and only the
-	// minimum fee is checked against the balance of the curator account, sent
-	// to the basket submodule, and then burned by the basket submodule.
-	if len(fee) > 0 {
-
-		// check if single coin in msg.Fee is greater than or equal to any coin in fee
-		if !msg.Fee.IsAnyGTE(fee) {
-			if len(fee) > 1 {
-				return nil, sdkerrors.ErrInsufficientFee.Wrapf("minimum fee one of %s, got %s", fee, msg.Fee)
-			} else {
-				return nil, sdkerrors.ErrInsufficientFee.Wrapf("minimum fee %s, got %s", fee, msg.Fee)
+	// only check and charge fee if allowed fees is not empty
+	if len(allowedFees) > 0 {
+
+		// check if fee is empty
+		if msg.Fee == nil {
+			if len(allowedFees) > 1 {
+				return nil, sdkerrors.ErrInsufficientFee.Wrapf(
+					"fee cannot be empty: must be one of %s", allowedFees,
+				)
+			}
+			return nil, sdkerrors.ErrInsufficientFee.Wrapf(
+				"fee cannot be empty: must be %s", allowedFees,
+			)
+		}
+
+		// In the next version of the basket package, the fee provided will be
+		// updated to a single Coin rather than a list of Coins. In the meantime,
+		// the message will fail basic validation if more than one Coin is provided.
+		fee := msg.Fee[0]
+
+		// check if provided fee is greater than or equal to any coin in allowedFees
+		if !msg.Fee.IsAnyGTE(allowedFees) {
+			if len(allowedFees) > 1 {
+				return nil, sdkerrors.ErrInsufficientFee.Wrapf(
+					"fee must be one of %s, got %s", allowedFees, fee,
+				)
 			}
+			return nil, sdkerrors.ErrInsufficientFee.Wrapf(
+				"fee must be %s, got %s", allowedFees, fee,
+			)
 		}
 
+		// only check and charge the minimum fee amount
 		minimumFee := sdk.Coin{
-			Denom:  msg.Fee[0].Denom,
-			Amount: fee.AmountOf(msg.Fee[0].Denom),
+			Denom:  fee.Denom,
+			Amount: allowedFees.AmountOf(fee.Denom),
 		}
 
+		// check curator balance against minimum fee
 		curatorBalance := k.bankKeeper.GetBalance(sdkCtx, curator, minimumFee.Denom)
 		if curatorBalance.IsNil() || curatorBalance.IsLT(minimumFee) {
-			return nil, sdkerrors.ErrInsufficientFunds.Wrapf("insufficient balance for bank denom %s", minimumFee.Denom)
+			return nil, sdkerrors.ErrInsufficientFunds.Wrapf(
+				"insufficient balance %s for bank denom %s", curatorBalance.Amount, minimumFee.Denom,
+			)
 		}
 
+		// convert minimum fee to multiple coins for processing
 		minimumFees := sdk.Coins{minimumFee}
 
 		err = k.bankKeeper.SendCoinsFromAccountToModule(sdkCtx, curator, basket.BasketSubModuleName, minimumFees)

x/ecocredit/server/basket/msg_create_test.go

@@ -21,7 +21,7 @@ type createSuite struct {
 	*baseSuite
 	alice               sdk.AccAddress
 	aliceBalance        sdk.Coin
-	basketFeeParam      sdk.Coins
+	params              core.Params
 	basketName          string
 	creditTypeAbbrev    string
 	creditTypePrecision uint32
@@ -41,18 +41,11 @@ func (s *createSuite) Before(t gocuke.TestingT) {
 	s.creditTypePrecision = 6
 }
 
-func (s *createSuite) AMinimumBasketFee(a string) {
-	coin, err := sdk.ParseCoinNormalized(a)
-	require.NoError(s.t, err)
-
-	s.basketFeeParam = sdk.NewCoins(coin)
-}
-
-func (s *createSuite) MinimumBasketFees(a string) {
-	coins, err := sdk.ParseCoinsNormalized(a)
+func (s *createSuite) AllowedBasketFee(a string) {
+	basketFee, err := sdk.ParseCoinsNormalized(a)
 	require.NoError(s.t, err)
 
-	s.basketFeeParam = coins
+	s.params.BasketFee = basketFee
 }
 
 func (s *createSuite) ACreditType() {
@@ -223,76 +216,48 @@ func (s *createSuite) ExpectTheResponse(a gocuke.DocString) {
 }
 
 func (s *createSuite) createExpectCalls() {
-	var coins sdk.Coins
+	var basketFee sdk.Coins
 
 	s.paramsKeeper.EXPECT().
-		Get(s.sdkCtx, core.KeyBasketFee, &coins).
-		Do(func(ctx sdk.Context, key []byte, coins *sdk.Coins) {
-			*coins = s.basketFeeParam
+		Get(s.sdkCtx, core.KeyBasketFee, &basketFee).
+		Do(func(ctx sdk.Context, key []byte, basketFee *sdk.Coins) {
+			*basketFee = s.params.BasketFee
 		}).
 		AnyTimes() // not expected on failed attempt
 
-	if len(s.basketFeeParam) == 1 {
-		s.bankKeeper.EXPECT().
-			GetBalance(s.sdkCtx, s.alice, s.basketFeeParam[0].Denom).
-			Return(s.aliceBalance).
-			AnyTimes() // not expected on failed attempt
-	}
-
-	if len(s.basketFeeParam) == 2 {
-		s.bankKeeper.EXPECT().
-			GetBalance(s.sdkCtx, s.alice, s.basketFeeParam[1].Denom).
-			Return(s.aliceBalance).
-			AnyTimes() // not expected on failed attempt
-	}
+	var expectedFee sdk.Coin
+	var expectedFees sdk.Coins
 
-	if len(s.basketFeeParam) == 1 {
-		sendCoins := sdk.NewCoins(s.basketFeeParam[0])
-
-		s.bankKeeper.EXPECT().
-			SendCoinsFromAccountToModule(s.sdkCtx, s.alice, basket.BasketSubModuleName, sendCoins).
-			Do(func(sdk.Context, sdk.AccAddress, string, sdk.Coins) {
-				if s.basketFeeParam != nil {
-					// simulate token balance update unavailable with mocks
-					s.aliceBalance = s.aliceBalance.Sub(s.basketFeeParam[0])
-				}
-			}).
-			Return(nil).
-			AnyTimes() // not expected on failed attempt
+	if len(s.params.BasketFee) == 1 {
+		expectedFee = s.params.BasketFee[0]
+		expectedFees = sdk.Coins{expectedFee}
 	}
 
-	if len(s.basketFeeParam) == 2 {
-		sendCoins := sdk.NewCoins(s.basketFeeParam[1])
-
-		s.bankKeeper.EXPECT().
-			SendCoinsFromAccountToModule(s.sdkCtx, s.alice, basket.BasketSubModuleName, sendCoins).
-			Do(func(sdk.Context, sdk.AccAddress, string, sdk.Coins) {
-				if s.basketFeeParam != nil {
-					// simulate token balance update unavailable with mocks
-					s.aliceBalance = s.aliceBalance.Sub(s.basketFeeParam[1])
-				}
-			}).
-			Return(nil).
-			AnyTimes() // not expected on failed attempt
+	if len(s.params.BasketFee) == 2 {
+		expectedFee = s.params.BasketFee[1]
+		expectedFees = sdk.Coins{expectedFee}
 	}
 
-	if len(s.basketFeeParam) == 1 {
-		burnCoins := sdk.NewCoins(s.basketFeeParam[0])
-
-		s.bankKeeper.EXPECT().
-			BurnCoins(s.sdkCtx, basket.BasketSubModuleName, burnCoins).
-			Return(nil).
-			AnyTimes() // not expected on failed attempt
-	}
+	s.bankKeeper.EXPECT().
+		GetBalance(s.sdkCtx, s.alice, expectedFee.Denom).
+		Return(s.aliceBalance).
+		AnyTimes() // not expected on failed attempt
 
-	if len(s.basketFeeParam) == 2 {
-		burnCoins := sdk.NewCoins(s.basketFeeParam[1])
+	s.bankKeeper.EXPECT().
+		SendCoinsFromAccountToModule(s.sdkCtx, s.alice, basket.BasketSubModuleName, expectedFees).
+		Do(func(sdk.Context, sdk.AccAddress, string, sdk.Coins) {
+			if s.params.BasketFee != nil {
+				// simulate token balance update unavailable with mocks
+				s.aliceBalance = s.aliceBalance.Sub(expectedFee)
+			}
+		}).
+		Return(nil).
+		AnyTimes() // not expected on failed attempt
 
-		s.bankKeeper.EXPECT().
-			BurnCoins(s.sdkCtx, basket.BasketSubModuleName, burnCoins).
-			Return(nil).
-			AnyTimes() // not expected on failed attempt
-	}
+	s.bankKeeper.EXPECT().
+		BurnCoins(s.sdkCtx, basket.BasketSubModuleName, expectedFees).
+		Return(nil).
+		AnyTimes() // not expected on failed attempt
 
 	s.bankKeeper.EXPECT().
 		SetDenomMetaData(s.sdkCtx, s.getDenomMetadata()).