From 3af10b332ffa018fd4136dd9db0805ca83cdc0c4 Mon Sep 17 00:00:00 2001 From: Ryan Christoffersen <12519942+ryanchristo@users.noreply.github.com> Date: Fri, 8 Jul 2022 10:04:41 -0700 Subject: [PATCH] fix(x/ecocredit): fix create class fee if not required and update tests (#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 --- .../server/basket/features/msg_create.feature | 84 +++--- x/ecocredit/server/basket/msg_create.go | 57 ++-- x/ecocredit/server/basket/msg_create_test.go | 103 +++---- x/ecocredit/server/core/create_class_test.go | 141 --------- .../core/features/msg_create_class.feature | 242 +++++++++++++++ .../{create_class.go => msg_create_class.go} | 53 +++- .../server/core/msg_create_class_test.go | 278 ++++++++++++++++++ 7 files changed, 677 insertions(+), 281 deletions(-) delete mode 100644 x/ecocredit/server/core/create_class_test.go create mode 100644 x/ecocredit/server/core/features/msg_create_class.feature rename x/ecocredit/server/core/{create_class.go => msg_create_class.go} (72%) create mode 100644 x/ecocredit/server/core/msg_create_class_test.go diff --git a/x/ecocredit/server/basket/features/msg_create.feature b/x/ecocredit/server/basket/features/msg_create.feature index 442117f0b0..cd2fb3c14a 100644 --- a/x/ecocredit/server/basket/features/msg_create.feature +++ b/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 "" 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 "" 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 "" 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 diff --git a/x/ecocredit/server/basket/msg_create.go b/x/ecocredit/server/basket/msg_create.go index 7deb86d0c8..189200f60a 100644 --- a/x/ecocredit/server/basket/msg_create.go +++ b/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) diff --git a/x/ecocredit/server/basket/msg_create_test.go b/x/ecocredit/server/basket/msg_create_test.go index 35eb4650ad..1c38cc4ea7 100644 --- a/x/ecocredit/server/basket/msg_create_test.go +++ b/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()). diff --git a/x/ecocredit/server/core/create_class_test.go b/x/ecocredit/server/core/create_class_test.go deleted file mode 100644 index c685ce99a9..0000000000 --- a/x/ecocredit/server/core/create_class_test.go +++ /dev/null @@ -1,141 +0,0 @@ -package core - -import ( - "testing" - - "github.com/golang/mock/gomock" - "gotest.tools/v3/assert" - - sdk "github.com/cosmos/cosmos-sdk/types" - - "github.com/regen-network/regen-ledger/x/ecocredit/core" - "github.com/regen-network/regen-ledger/x/ecocredit/server/utils" -) - -func TestCreateClass_Valid(t *testing.T) { - t.Parallel() - s := setupBase(t) - gmAny := gomock.Any() - ccFee := core.DefaultParams().CreditClassFee[0] - - allowListEnabled := true - creditClassFees := core.DefaultParams().CreditClassFee - allowList := []string{s.addr.String()} - utils.ExpectParamGet(&allowListEnabled, s.paramsKeeper, core.KeyAllowlistEnabled, 1) - utils.ExpectParamGet(&allowList, s.paramsKeeper, core.KeyAllowedClassCreators, 1) - utils.ExpectParamGet(&creditClassFees, s.paramsKeeper, core.KeyCreditClassFee, 1) - s.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gmAny, gmAny, gmAny, gmAny).Return(nil).Times(1) - s.bankKeeper.EXPECT().BurnCoins(gmAny, gmAny, gmAny).Return(nil).Times(1) - - res, err := s.k.CreateClass(s.ctx, &core.MsgCreateClass{ - Admin: s.addr.String(), - Issuers: []string{s.addr.String()}, - Metadata: "", - CreditTypeAbbrev: "C", - Fee: &ccFee, - }) - assert.NilError(t, err, "error creating class: %+w", err) - assert.Equal(t, res.ClassId, "C01") - - // check class info - ci, err := s.stateStore.ClassTable().GetById(s.ctx, res.ClassId) - assert.NilError(t, err) - assert.Equal(t, res.ClassId, ci.Id) - - // check class issuer - _, err = s.stateStore.ClassIssuerTable().Get(s.ctx, ci.Key, s.addr) - assert.NilError(t, err) - - // check sequence number - seq, err := s.stateStore.ClassSequenceTable().Get(s.ctx, "C") - assert.NilError(t, err) - assert.Equal(t, uint64(2), seq.NextSequence) -} - -func TestCreateClass_Unauthorized(t *testing.T) { - t.Parallel() - s := setupBase(t) - - // allowlist = true and sender is not in allowlist - allowListEnabled := true - allowList := []string{sdk.AccAddress("foobar").String()} - utils.ExpectParamGet(&allowListEnabled, s.paramsKeeper, core.KeyAllowlistEnabled, 1) - utils.ExpectParamGet(&allowList, s.paramsKeeper, core.KeyAllowedClassCreators, 1) - _, err := s.k.CreateClass(s.ctx, &core.MsgCreateClass{ - Admin: s.addr.String(), - Issuers: []string{s.addr.String()}, - Metadata: "", - CreditTypeAbbrev: "C", - }) - assert.ErrorContains(t, err, "is not allowed to create credit classes") -} - -func TestCreateClass_Sequence(t *testing.T) { - t.Parallel() - s := setupBase(t) - ccFee := core.DefaultParams().CreditClassFee[0] - gmAny := gomock.Any() - - allowListEnabled := false - classFees := core.DefaultParams().CreditClassFee - utils.ExpectParamGet(&allowListEnabled, s.paramsKeeper, core.KeyAllowlistEnabled, 1) - utils.ExpectParamGet(&classFees, s.paramsKeeper, core.KeyCreditClassFee, 1) - - s.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gmAny, gmAny, gmAny, gmAny).Return(nil).Times(2) - s.bankKeeper.EXPECT().BurnCoins(gmAny, gmAny, gmAny).Return(nil).Times(2) - - res, err := s.k.CreateClass(s.ctx, &core.MsgCreateClass{ - Admin: s.addr.String(), - Issuers: []string{s.addr.String()}, - Metadata: "", - CreditTypeAbbrev: "C", - Fee: &ccFee, - }) - assert.NilError(t, err, "error creating class: %+w", err) - - utils.ExpectParamGet(&allowListEnabled, s.paramsKeeper, core.KeyAllowlistEnabled, 1) - utils.ExpectParamGet(&classFees, s.paramsKeeper, core.KeyCreditClassFee, 1) - res2, err := s.k.CreateClass(s.ctx, &core.MsgCreateClass{ - Admin: s.addr.String(), - Issuers: []string{s.addr.String()}, - Metadata: "", - CreditTypeAbbrev: "C", - Fee: &ccFee, - }) - assert.NilError(t, err, "error creating class: %+w", err) - - assert.Equal(t, res.ClassId, "C01") - assert.Equal(t, res2.ClassId, "C02") -} - -func TestCreateClass_Fees(t *testing.T) { - t.Parallel() - s := setupBase(t) - - allowListEnabled := false - classFees := core.DefaultParams().CreditClassFee - utils.ExpectParamGet(&allowListEnabled, s.paramsKeeper, core.KeyAllowlistEnabled, 1) - utils.ExpectParamGet(&classFees, s.paramsKeeper, core.KeyCreditClassFee, 1) - - // wrong denom - _, err := s.k.CreateClass(s.ctx, &core.MsgCreateClass{ - Admin: s.addr.String(), - Issuers: []string{s.addr.String()}, - Metadata: "", - CreditTypeAbbrev: "C", - Fee: &sdk.Coin{Denom: "bar", Amount: sdk.NewInt(10)}, - }) - assert.ErrorContains(t, err, "bar is not allowed to be used in credit class fees") - - // fee too low - utils.ExpectParamGet(&allowListEnabled, s.paramsKeeper, core.KeyAllowlistEnabled, 1) - utils.ExpectParamGet(&classFees, s.paramsKeeper, core.KeyCreditClassFee, 1) - _, err = s.k.CreateClass(s.ctx, &core.MsgCreateClass{ - Admin: s.addr.String(), - Issuers: []string{s.addr.String()}, - Metadata: "", - CreditTypeAbbrev: "C", - Fee: &sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(1)}, - }) - assert.ErrorContains(t, err, "expected 20000000stake for fee, got 1stake") -} diff --git a/x/ecocredit/server/core/features/msg_create_class.feature b/x/ecocredit/server/core/features/msg_create_class.feature new file mode 100644 index 0000000000..31e44fdd86 --- /dev/null +++ b/x/ecocredit/server/core/features/msg_create_class.feature @@ -0,0 +1,242 @@ +Feature: Msg/CreateClass + + A credit class can be created: + - when the credit type exists + - when the allowlist is enabled and the admin is an approved credit class creator + - when an allowed credit class fee is not set and no fee is provided + - when the credit class fee denom matches an allowed credit class fee denom + - when the credit class fee amount is greater than or equal to an allowed credit class fee amount + - when the admin balance is greater than or equal to an allowed basket fee amount + - the admin balance is updated and only the minimum fee is taken + - the class sequence is updated + - the class issuers are added + - the class properties are added + - the response includes the class id + + Rule: The credit type must exist + + Scenario: credit type exists + Given a credit type with abbreviation "A" + When alice attempts to create a credit class with credit type "A" + Then expect no error + + Scenario: credit type exists + Given a credit type with abbreviation "A" + When alice attempts to create a credit class with credit type "B" + Then expect the error "could not get credit type with abbreviation B: not found: invalid request" + + Rule: The admin must be an approved credit class creator if the allowlist is enabled + + Background: + Given a credit type + + Scenario: allowlist disabled and admin is an approved creator + Given allowlist enabled "false" + And alice is an approved credit class creator + When alice attempts to create a credit class + Then expect no error + + Scenario: allowlist disabled and admin is not an approved creator + Given allowlist enabled "false" + When alice attempts to create a credit class + Then expect no error + + Scenario: allowlist enabled and admin is an approved creator + Given allowlist enabled "true" + And alice is an approved credit class creator + When alice attempts to create a credit class + Then expect no error + + Scenario: allowlist enabled and admin is not an approved creator + Given allowlist enabled "true" + When alice attempts to create a credit class + Then expect error contains "is not allowed to create credit classes: unauthorized" + + Rule: The credit class fee is not required if an allowed credit class fee is not set + + Background: + Given a credit type + + Scenario: credit class fee provided and allowed credit class fee not set + Given alice has a token balance "20regen" + When alice attempts to create a credit class with fee "20regen" + Then expect no error + + Scenario: credit class fee not provided and allowed credit class fee not set + When alice attempts to create a credit class + Then expect no error + + # no failing scenario - credit class fee is not required if minimum credit class fee is not set + + Rule: The credit class fee must match a credit class fee denom + + Background: + Given a credit type + And alice has a token balance "20regen" + + Scenario: credit class fee matches allowed credit class fee denom (single fee) + Given allowed credit class fee "20regen" + When alice attempts to create a credit class with fee "20regen" + Then expect no error + + Scenario: credit class fee matches allowed credit class fee denom (multiple fees) + Given allowed credit class fee "20regen,20atom" + When alice attempts to create a credit class with fee "20regen" + Then expect no error + + Scenario: credit class fee does not match allowed credit class fee denom (single fee) + Given allowed credit class fee "20regen" + When alice attempts to create a credit class with fee "20atom" + Then expect the error "fee must be 20regen, got 20atom: insufficient fee" + + Scenario: credit class fee does not match allowed credit class fee denom (multiple fees) + Given allowed credit class fee "20regen,20atom" + When alice attempts to create a credit class with fee "20stake" + Then expect the error "fee must be one of 20atom,20regen, got 20stake: insufficient fee" + + Scenario: credit class fee not provided and allowed credit class fee set (single fee) + Given allowed credit class fee "20regen" + When alice attempts to create a credit class + Then expect the error "fee cannot be empty: must be 20regen: insufficient fee" + + Scenario: credit class fee not provided and allowed credit class fee set (multiple fees) + Given allowed credit class fee "20regen,20atom" + When alice attempts to create a credit class + Then expect the error "fee cannot be empty: must be one of 20atom,20regen: insufficient fee" + + Rule: The credit class fee must be greater than or equal to a credit class fee + + Background: + Given a credit type + And alice has a token balance "20regen" + + Scenario Outline: credit class fee is greater than or equal to minimum credit class fee (single fee) + Given allowed credit class fee "20regen" + When alice attempts to create a credit class with fee "" + Then expect no error + + Examples: + | description | credit-class-fee | + | greater than | 30regen | + | equal to | 20regen | + + Scenario Outline: credit class fee is greater than or equal to minimum credit class fee (multiple fees) + Given allowed credit class fee "20regen,20atom" + When alice attempts to create a credit class with fee "" + Then expect no error + + Examples: + | description | credit-class-fee | + | greater than | 30regen | + | equal to | 20regen | + + Scenario: credit class fee is less than minimum credit class fee (single fee) + Given allowed credit class fee "20regen" + When alice attempts to create a credit class with fee "10regen" + Then expect the error "fee must be 20regen, got 10regen: insufficient fee" + + Scenario: credit class fee is less than minimum credit class fee (multiple fees) + Given allowed credit class fee "20regen,20atom" + When alice attempts to create a credit class with fee "10regen" + 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 credit class fee amount + + Background: + Given a credit type + And allowed credit class fee "20regen" + + Scenario Outline: admin balance is greater than or equal to credit class fee amount + Given alice has a token balance "" + When alice attempts to create a credit class with fee "20regen" + Then expect no error + + Examples: + | description | token-balance | + | greater than | 30regen | + | equal to | 20regen | + + Scenario: admin balance is less than credit class fee amount + Given alice has a token balance "10regen" + When alice attempts to create a credit class with fee "20regen" + Then expect the error "insufficient balance 10 for bank denom regen: insufficient funds" + + Rule: The class sequence is updated + + Background: + Given a credit type with abbreviation "A" + + Scenario: the class sequence is updated + Given a class sequence with credit type "A" and next sequence "1" + When alice attempts to create a credit class with credit type "A" + Then expect class sequence with credit type "A" and next sequence "2" + + Scenario: the class sequence is not updated + Given a class sequence with credit type "A" and next sequence "1" + When alice attempts to create a credit class with credit type "B" + Then expect class sequence with credit type "A" and next sequence "1" + + # no failing scenario - state transitions only occur upon successful message execution + + Rule: The class issuers are added + + Background: + Given a credit type + + Scenario: the class issuers are added + When alice attempts to create a credit class with issuers + """ + [ + "regen1depk54cuajgkzea6zpgkq36tnjwdzv4ak663u6", + "regen1tnh2q55v8wyygtt9srz5safamzdengsnlm0yy4" + ] + """ + Then expect class issuers + """ + [ + "regen1depk54cuajgkzea6zpgkq36tnjwdzv4ak663u6", + "regen1tnh2q55v8wyygtt9srz5safamzdengsnlm0yy4" + ] + """ + + # no failing scenario - state transitions only occur upon successful message execution + + Rule: The class properties are added + + Background: + Given a credit type with abbreviation "A" + + Scenario: the class properties are added + When alice attempts to create a credit class with properties + """ + { + "credit_type_abbrev": "A", + "metadata": "regen:13toVfvC2YxrrfSXWB5h2BGHiXZURsKxWUz72uDRDSPMCrYPguGUXSC.rdf" + } + """ + Then expect class properties + """ + { + "id": "A01", + "credit_type_abbrev": "A", + "metadata": "regen:13toVfvC2YxrrfSXWB5h2BGHiXZURsKxWUz72uDRDSPMCrYPguGUXSC.rdf" + } + """ + + # no failing scenario - state transitions only occur upon successful message execution + + Rule: The response includes the class id + + Background: + Given a credit type with abbreviation "A" + + Scenario: the response includes the class id + When alice attempts to create a credit class with credit type "A" + Then expect the response + """ + { + "class_id": "A01" + } + """ + + # no failing scenario - response should always be empty when message execution fails diff --git a/x/ecocredit/server/core/create_class.go b/x/ecocredit/server/core/msg_create_class.go similarity index 72% rename from x/ecocredit/server/core/create_class.go rename to x/ecocredit/server/core/msg_create_class.go index 54299f08f2..ca8d0ace5a 100644 --- a/x/ecocredit/server/core/create_class.go +++ b/x/ecocredit/server/core/msg_create_class.go @@ -29,24 +29,55 @@ func (k Keeper) CreateClass(goCtx context.Context, req *core.MsgCreateClass) (*c } // TODO: remove params https://github.com/regen-network/regen-ledger/issues/729 - var fee sdk.Coins - k.paramsKeeper.Get(sdkCtx, core.KeyCreditClassFee, &fee) + var allowedFees sdk.Coins + k.paramsKeeper.Get(sdkCtx, core.KeyCreditClassFee, &allowedFees) - if fee.Len() > 0 { + // only check and charge fee if allowed fees is not empty + if allowedFees.Len() > 0 { + + // check if fee is empty if req.Fee == nil { - return nil, sdkerrors.ErrInvalidRequest.Wrapf("fee must be one of %s", fee) + 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, + ) + } + + // convert fee to multiple coins for verification + coins := sdk.Coins{*req.Fee} + + // check if fee is greater than or equal to any coin in allowedFees + if !coins.IsAnyGTE(allowedFees) { + if len(allowedFees) > 1 { + return nil, sdkerrors.ErrInsufficientFee.Wrapf( + "fee must be one of %s, got %s", allowedFees, req.Fee, + ) + } + return nil, sdkerrors.ErrInsufficientFee.Wrapf( + "fee must be %s, got %s", allowedFees, req.Fee, + ) } - feeAmt := fee.AmountOf(req.Fee.Denom) - if feeAmt.IsZero() { - return nil, sdkerrors.ErrInvalidRequest.Wrapf("%s is not allowed to be used in credit class fees", req.Fee.Denom) + // only check and charge the minimum fee amount + minimumFee := sdk.Coin{ + Denom: req.Fee.Denom, + Amount: allowedFees.AmountOf(req.Fee.Denom), } - if req.Fee.Amount.LT(feeAmt) { - return nil, sdkerrors.ErrInsufficientFee.Wrapf("expected %v%s for fee, got %v", feeAmt, req.Fee.Denom, req.Fee) + + // check admin balance against minimum fee + adminBalance := k.bankKeeper.GetBalance(sdkCtx, adminAddress, minimumFee.Denom) + if adminBalance.IsNil() || adminBalance.IsLT(minimumFee) { + return nil, sdkerrors.ErrInsufficientFunds.Wrapf( + "insufficient balance %s for bank denom %s", adminBalance.Amount, minimumFee.Denom, + ) } - // Charge the admin a fee to create the credit class - err = k.chargeCreditClassFee(sdkCtx, adminAddress, sdk.Coins{sdk.Coin{Denom: req.Fee.Denom, Amount: feeAmt}}) + // send coins from account to module and then burn the coins + err = k.chargeCreditClassFee(sdkCtx, adminAddress, sdk.Coins{minimumFee}) if err != nil { return nil, err } diff --git a/x/ecocredit/server/core/msg_create_class_test.go b/x/ecocredit/server/core/msg_create_class_test.go new file mode 100644 index 0000000000..e387848cfc --- /dev/null +++ b/x/ecocredit/server/core/msg_create_class_test.go @@ -0,0 +1,278 @@ +package core + +import ( + "encoding/json" + "strconv" + "testing" + + "github.com/regen-network/gocuke" + "github.com/stretchr/testify/require" + + sdk "github.com/cosmos/cosmos-sdk/types" + + api "github.com/regen-network/regen-ledger/api/regen/ecocredit/v1" + "github.com/regen-network/regen-ledger/x/ecocredit" + "github.com/regen-network/regen-ledger/x/ecocredit/core" +) + +type createClassSuite struct { + *baseSuite + alice sdk.AccAddress + aliceBalance sdk.Coin + params core.Params + creditTypeAbbrev string + classId string + res *core.MsgCreateClassResponse + err error +} + +func TestCreateClass(t *testing.T) { + gocuke.NewRunner(t, &createClassSuite{}).Path("./features/msg_create_class.feature").Run() +} + +func (s *createClassSuite) Before(t gocuke.TestingT) { + // TODO: move to init function in the root directory of the module #1243 + cfg := sdk.GetConfig() + cfg.SetBech32PrefixForAccount("regen", "regenpub") + + s.baseSuite = setupBase(t) + s.alice = s.addr + s.creditTypeAbbrev = "C" + s.classId = "C01" +} + +func (s *createClassSuite) ACreditType() { + // TODO: Save for now but credit type should not exist prior to unit test #893 + err := s.k.stateStore.CreditTypeTable().Save(s.ctx, &api.CreditType{ + Abbreviation: s.creditTypeAbbrev, + }) + require.NoError(s.t, err) +} + +func (s *createClassSuite) ACreditTypeWithAbbreviation(a string) { + err := s.k.stateStore.CreditTypeTable().Insert(s.ctx, &api.CreditType{ + Abbreviation: a, + }) + require.NoError(s.t, err) +} + +func (s *createClassSuite) AllowlistEnabled(a string) { + allowlistEnabled, err := strconv.ParseBool(a) + require.NoError(s.t, err) + + s.params.AllowlistEnabled = allowlistEnabled +} + +func (s *createClassSuite) AliceIsAnApprovedCreditClassCreator() { + s.params.AllowedClassCreators = append(s.params.AllowedClassCreators, s.alice.String()) +} + +func (s *createClassSuite) AllowedCreditClassFee(a string) { + creditClassFee, err := sdk.ParseCoinsNormalized(a) + require.NoError(s.t, err) + + s.params.CreditClassFee = creditClassFee +} + +func (s *createClassSuite) AliceHasATokenBalance(a string) { + balance, err := sdk.ParseCoinNormalized(a) + require.NoError(s.t, err) + + s.aliceBalance = balance +} + +func (s *createClassSuite) AClassSequenceWithCreditTypeAndNextSequence(a string, b string) { + nextSequence, err := strconv.ParseUint(b, 10, 64) + require.NoError(s.t, err) + + err = s.stateStore.ClassSequenceTable().Insert(s.ctx, &api.ClassSequence{ + CreditTypeAbbrev: a, + NextSequence: nextSequence, + }) + require.NoError(s.t, err) +} + +func (s *createClassSuite) AliceAttemptsToCreateACreditClass() { + s.createClassExpectCalls() + + s.res, s.err = s.k.CreateClass(s.ctx, &core.MsgCreateClass{ + Admin: s.alice.String(), + CreditTypeAbbrev: s.creditTypeAbbrev, + }) +} + +func (s *createClassSuite) AliceAttemptsToCreateACreditClassWithCreditType(a string) { + s.createClassExpectCalls() + + s.res, s.err = s.k.CreateClass(s.ctx, &core.MsgCreateClass{ + Admin: s.alice.String(), + CreditTypeAbbrev: a, + }) +} + +func (s *createClassSuite) AliceAttemptsToCreateACreditClassWithIssuers(a gocuke.DocString) { + var issuers []string + + err := json.Unmarshal([]byte(a.Content), &issuers) + require.NoError(s.t, err) + + s.createClassExpectCalls() + + s.res, s.err = s.k.CreateClass(s.ctx, &core.MsgCreateClass{ + Admin: s.alice.String(), + CreditTypeAbbrev: s.creditTypeAbbrev, + Issuers: issuers, + }) +} + +func (s *createClassSuite) AliceAttemptsToCreateACreditClassWithProperties(a gocuke.DocString) { + var msg *core.MsgCreateClass + + err := json.Unmarshal([]byte(a.Content), &msg) + require.NoError(s.t, err) + + s.createClassExpectCalls() + + s.res, s.err = s.k.CreateClass(s.ctx, &core.MsgCreateClass{ + Admin: s.alice.String(), + CreditTypeAbbrev: msg.CreditTypeAbbrev, + Metadata: msg.Metadata, + }) +} + +func (s *createClassSuite) AliceAttemptsToCreateACreditClassWithFee(a string) { + fee, err := sdk.ParseCoinNormalized(a) + require.NoError(s.t, err) + + s.createClassExpectCalls() + + s.res, s.err = s.k.CreateClass(s.ctx, &core.MsgCreateClass{ + Admin: s.alice.String(), + CreditTypeAbbrev: s.creditTypeAbbrev, + Fee: &fee, + }) +} + +func (s *createClassSuite) ExpectNoError() { + require.NoError(s.t, s.err) +} + +func (s *createClassSuite) ExpectTheError(a string) { + require.EqualError(s.t, s.err, a) +} + +func (s *createClassSuite) ExpectErrorContains(a string) { + require.ErrorContains(s.t, s.err, a) +} + +func (s *createClassSuite) ExpectClassSequenceWithCreditTypeAndNextSequence(a string, b string) { + nextSequence, err := strconv.ParseUint(b, 10, 64) + require.NoError(s.t, err) + + classSequence, err := s.stateStore.ClassSequenceTable().Get(s.ctx, a) + require.NoError(s.t, err) + + require.Equal(s.t, nextSequence, classSequence.NextSequence) +} + +func (s *createClassSuite) ExpectClassIssuers(a gocuke.DocString) { + var issuers []string + + err := json.Unmarshal([]byte(a.Content), &issuers) + require.NoError(s.t, err) + + class, err := s.stateStore.ClassTable().GetById(s.ctx, s.classId) + require.NoError(s.t, err) + + for _, issuer := range issuers { + account, err := sdk.AccAddressFromBech32(issuer) + require.NoError(s.t, err) + + exists, err := s.stateStore.ClassIssuerTable().Has(s.ctx, class.Key, account) + require.NoError(s.t, err) + require.True(s.t, exists) + } +} + +func (s *createClassSuite) ExpectClassProperties(a gocuke.DocString) { + var expected *api.Class + + err := json.Unmarshal([]byte(a.Content), &expected) + require.NoError(s.t, err) + + class, err := s.stateStore.ClassTable().GetById(s.ctx, expected.Id) + require.NoError(s.t, err) + + require.Equal(s.t, expected.CreditTypeAbbrev, class.CreditTypeAbbrev) + require.Equal(s.t, expected.Metadata, class.Metadata) +} + +func (s *createClassSuite) ExpectTheResponse(a gocuke.DocString) { + var res *core.MsgCreateClassResponse + + err := json.Unmarshal([]byte(a.Content), &res) + require.NoError(s.t, err) + + require.Equal(s.t, res, s.res) +} + +func (s *createClassSuite) createClassExpectCalls() { + var allowlistEnabled bool + var allowedClassCreators []string + var creditClassFee sdk.Coins + + s.paramsKeeper.EXPECT(). + Get(s.sdkCtx, core.KeyAllowlistEnabled, &allowlistEnabled). + Do(func(ctx sdk.Context, key []byte, allowlistEnabled *bool) { + *allowlistEnabled = s.params.AllowlistEnabled + }). + AnyTimes() // not expected on failed attempt + + s.paramsKeeper.EXPECT(). + Get(s.sdkCtx, core.KeyAllowedClassCreators, &allowedClassCreators). + Do(func(ctx sdk.Context, key []byte, allowedClassCreators *[]string) { + *allowedClassCreators = s.params.AllowedClassCreators + }). + AnyTimes() // not expected on failed attempt + + s.paramsKeeper.EXPECT(). + Get(s.sdkCtx, core.KeyCreditClassFee, &creditClassFee). + Do(func(ctx sdk.Context, key []byte, creditClassFee *sdk.Coins) { + *creditClassFee = s.params.CreditClassFee + }). + AnyTimes() // not expected on failed attempt + + var expectedFee sdk.Coin + var expectedFees sdk.Coins + + if len(s.params.CreditClassFee) == 1 { + expectedFee = s.params.CreditClassFee[0] + expectedFees = sdk.Coins{expectedFee} + } + + if len(s.params.CreditClassFee) == 2 { + expectedFee = s.params.CreditClassFee[1] + expectedFees = sdk.Coins{expectedFee} + } + + s.bankKeeper.EXPECT(). + GetBalance(s.sdkCtx, s.alice, expectedFee.Denom). + Return(s.aliceBalance). + AnyTimes() // not expected on failed attempt + + s.bankKeeper.EXPECT(). + SendCoinsFromAccountToModule(s.sdkCtx, s.alice, ecocredit.ModuleName, expectedFees). + Do(func(sdk.Context, sdk.AccAddress, string, sdk.Coins) { + if s.params.CreditClassFee != 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, ecocredit.ModuleName, expectedFees). + Return(nil). + AnyTimes() // not expected on failed attempt +}