From b5fec1be289dbee24eec14f44a4b5bca2d6d12fb Mon Sep 17 00:00:00 2001 From: yaruwangway <69694322+yaruwangway@users.noreply.github.com> Date: Wed, 1 Mar 2023 12:28:34 +0100 Subject: [PATCH] merge two global fee refactor branch (#2242) * Update changelog for v9 rc7 (#2219) * Update v8.0.1 changelog (#2221) * Consistently generate build tags on newer Make (#2018) With Make 4.3+, the empty whitespace does not seem to work as originally intended. This causes build tags to be "netgo ledger," on Ubuntu 22.04 and other systems that include the newer Make version. The build tags were intended as "netgo,ledger" which can be observed on Make 4.2 (shipped with Ubuntu 20.04). This change swaps out the `+=` use in favor of an explicit `:=`. Ref: https://www.gnu.org/software/make/manual/html_node/Appending.html Closes cosmos/gaia#2017. Co-authored-by: Marius Poke Co-authored-by: Milan Mulji <98309852+mmulji-ic@users.noreply.github.com> * - Update doc to follow Go conventions - Improve code readability * update params * docs: update docs * fix: lint --------- Co-authored-by: lg <8335464+glnro@users.noreply.github.com> Co-authored-by: Valters Jansons Co-authored-by: Marius Poke Co-authored-by: Milan Mulji <98309852+mmulji-ic@users.noreply.github.com> Co-authored-by: Simon Noetzlin --- CHANGELOG.md | 25 ++++++++++ Makefile | 2 +- docs/modules/globalfee.md | 4 +- x/globalfee/ante/fee.go | 81 ++++++++++++++++++-------------- x/globalfee/ante/fee_utils.go | 51 ++++++++------------ x/globalfee/module.go | 6 ++- x/globalfee/types/params.go | 59 +++++++++-------------- x/globalfee/types/params_test.go | 12 +++++ 8 files changed, 130 insertions(+), 110 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e5dff2deba..6555bab0537 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,11 @@ Ref: https://keepachangelog.com/en/1.0.0/ # Changelog +## [v8.0.0] - 2023-02-17 + +* (gaia) Bump [cosmos-sdk](https://github.com/cosmos/cosmos-sdk) to [v0.45.14](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.45.14). See [CHANGELOG.md](https://github.com/cosmos/cosmos-sdk/blob/release/v0.45.x/CHANGELOG.md) for details. +* (gaia) Bump [tendermint](https://github.com/informalsystems/tendermint) to [0.34.26](https://github.com/informalsystems/tendermint/tree/v0.34.26). See [CHANGELOG.md](https://github.com/informalsystems/tendermint/blob/v0.34.26/CHANGELOG.md) for details. + ## [v8.0.0] - 2023-01-31 * (gaia) Bump [ibc-go](https://github.com/cosmos/ibc-go) to [v3.4.0](https://github.com/cosmos/ibc-go/blob/v3.4.0/CHANGELOG.md) to fix a vulnerability in ICA. See [v3.4.0 CHANGELOG.md](https://github.com/cosmos/cosmos-sdk/blob/v0.45.9/CHANGELOG.md) and [v3.2.1 Release Notes](https://github.com/cosmos/ibc-go/releases/tag/v3.2.1) for details. @@ -53,6 +58,26 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +## [v9.0.0-rc7] - 2023-02-17 + +* (feat) Add [Interchain-Security](https://github.com/cosmos/interchain-security) [v1.0.0-rc7](https://github.com/cosmos/interchain-security/releases/tag/v1.0.0-rc7) provider module. See the [ICS Spec](https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/README.md) for more details. +* (gaia) Bump [cosmos-sdk](https://github.com/cosmos/cosmos-sdk) to [v0.45.13-ics](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.45.13-ics). See [CHANGELOG.md](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.45.12) for details. +* (gaia) Bump [ibc-go](https://github.com/cosmos/ibc-go) to [v4.2.0](https://github.com/cosmos/ibc-go/blob/release/v4.2.x/CHANGELOG.md). See [v4.2 Release Notes](https://github.com/cosmos/ibc-go/releases/tag/v4.2.0) for details. +* (gaia) Bump [tendermint](https://github.com/informalsystems/tendermint) to [0.34.26](https://github.com/informalsystems/tendermint/tree/v0.34.26). See [CHANGELOG.md](https://github.com/informalsystems/tendermint/blob/v0.34.26/CHANGELOG.md#v03426) for details. +* (gaia) Bump [packet-forwarding-middleware](https://github.com/strangelove-ventures/packet-forward-middleware) to [v4.0.4](https://github.com/strangelove-ventures/packet-forward-middleware/releases/tag/v4.0.4). +* (tests) Add [E2E ccv tests](https://github.com/cosmos/gaia/blob/main/tests/e2e/e2e_gov_test.go#L138). Tests covering new functionality introduced by the provider module to add and remove a consumer chain via governance proposal. +* (tests) Add [integration ccv tests](https://github.com/cosmos/gaia/blob/main/tests/ics/interchain_security_test.go). Imports Interchain-Security's `TestCCVTestSuite` and implements Gaia as the provider chain. + +## [v9.0.0-rc6] - 2023-02-13 + +* (feat) Add [Interchain-Security](https://github.com/cosmos/interchain-security) [v1.0.0-rc7](https://github.com/cosmos/interchain-security/releases/tag/v1.0.0-rc7) provider module. See the [ICS Spec](https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/README.md) for more details. +* (gaia) Bump [cosmos-sdk](https://github.com/cosmos/cosmos-sdk) to [v0.45.13-ics](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.45.13-ics). See [CHANGELOG.md](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.45.12) for details. +* (gaia) Bump [ibc-go](https://github.com/cosmos/ibc-go) to [v4.2.0](https://github.com/cosmos/ibc-go/blob/release/v4.2.x/CHANGELOG.md). See [v4.2 Release Notes](https://github.com/cosmos/ibc-go/releases/tag/v4.2.0) for details. +* (gaia) Bump [tendermint](https://github.com/informalsystems/tendermint) to [0.34.26](https://github.com/informalsystems/tendermint/tree/v0.34.26). See [CHANGELOG.md](https://github.com/informalsystems/tendermint/blob/v0.34.26/CHANGELOG.md#v03426) for details. +* (gaia) Bump [packet-forwarding-middleware](https://github.com/strangelove-ventures/packet-forward-middleware) to [v4.0.4](https://github.com/strangelove-ventures/packet-forward-middleware/releases/tag/v4.0.4). +* (tests) Add [E2E ccv tests](https://github.com/cosmos/gaia/blob/main/tests/e2e/e2e_gov_test.go#L138). Tests covering new functionality introduced by the provider module to add and remove a consumer chain via governance proposal. +* (tests) Add [integration ccv tests](https://github.com/cosmos/gaia/blob/main/tests/ics/interchain_security_test.go). Imports Interchain-Security's `TestCCVTestSuite` and implements Gaia as the provider chain. + ## [v9.0.0-rc5] - 2023-02-10 * (feat) Add [Interchain-Security](https://github.com/cosmos/interchain-security) [v1.0.0-rc7](https://github.com/cosmos/interchain-security/releases/tag/v1.0.0-rc7) provider module. See the [ICS Spec](https://github.com/cosmos/ibc/blob/main/spec/app/ics-028-cross-chain-validation/README.md) for more details. diff --git a/Makefile b/Makefile index 46967152919..09748e5908a 100644 --- a/Makefile +++ b/Makefile @@ -58,7 +58,7 @@ build_tags += $(BUILD_TAGS) build_tags := $(strip $(build_tags)) whitespace := -whitespace += $(whitespace) +whitespace := $(whitespace) $(whitespace) comma := , build_tags_comma_sep := $(subst $(whitespace),$(comma),$(build_tags)) diff --git a/docs/modules/globalfee.md b/docs/modules/globalfee.md index b69a1fe917d..e3027769688 100644 --- a/docs/modules/globalfee.md +++ b/docs/modules/globalfee.md @@ -201,9 +201,9 @@ Note that the required amount of `uatom` in globalfee is overwritten by the amou ### Case 7 -**Setting:** globalfee=[0.1uatom], minimum-gas-prices=0.2uatom,1stake, gas=200000, bypass-min-fee-msg-types = ["/cosmos.distribution.v1beta1.MsgWithdrawDelegatorReward"] +**Setting:** globalfee=[0.1uatom], minimum-gas-prices=[0.2uatom, 1stake], gas=200000, bypass-min-fee-msg-types = ["/cosmos.distribution.v1beta1.MsgWithdrawDelegatorReward"] -Note that the required amount of `uatom` in globalfee is overwritten by the amount in minimum-gas-prices. +Note that the required amount of `uatom` in globalfee is overwritten by the amount in minimum-gas-prices. Also, the `1stake` in minimum-gas-prices is ignored. - msg withdraw-all-rewards with paidfee="", `pass` diff --git a/x/globalfee/ante/fee.go b/x/globalfee/ante/fee.go index e818e7d398b..fd74228ec6b 100644 --- a/x/globalfee/ante/fee.go +++ b/x/globalfee/ante/fee.go @@ -12,7 +12,7 @@ import ( "github.com/cosmos/gaia/v9/x/globalfee" ) -// FeeWithBypassDecorator will check if the transaction's fee is at least as large +// FeeWithBypassDecorator checks if the transaction's fee is at least as large // as the local validator's minimum gasFee (defined in validator config) and global fee, and the fee denom should be in the global fees' denoms. // // If fee is too low, decorator returns error and tx is rejected from mempool. @@ -49,17 +49,33 @@ func NewFeeDecorator(bypassMsgTypes []string, globalfeeSubspace, stakingSubspace } } +// AnteHandle implements the AnteDecorator interface func (mfd FeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { // please note: after parsing feeflag, the zero fees are removed already feeTx, ok := tx.(sdk.FeeTx) if !ok { return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") } + + // Only check for minimum fees and global fee if the execution mode is CheckTx + if !ctx.IsCheckTx() || simulate { + return next(ctx, tx, simulate) + } + + // sort fee tx's coins feeCoins := feeTx.GetFee().Sort() gas := feeTx.GetGas() msgs := feeTx.GetMsgs() + // Get required Global Fee and min gas price + requiredGlobalFees, err := mfd.getGlobalFee(ctx, feeTx) + if err != nil { + panic(err) + } + requiredFees := getMinGasPrice(ctx, feeTx) + // Accept zero fee transactions only if both of the following statements are true: + // // - the tx contains only message types that can bypass the minimum fee, // see BypassMinFeeMsgTypes; // - the total gas limit per message does not exceed MaxTotalBypassMinFeeMsgGasUsage, @@ -68,39 +84,7 @@ func (mfd FeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, ne doesNotExceedMaxGasUsage := gas <= mfd.MaxTotalBypassMinFeeMsgGasUsage allowedToBypassMinFee := mfd.containsOnlyBypassMinFeeMsgs(msgs) && doesNotExceedMaxGasUsage - var allFees sdk.Coins - requiredGlobalFees, err := mfd.getGlobalFee(ctx, feeTx) - if err != nil { - panic(err) - } - requiredFees := getMinGasPrice(ctx, feeTx) - - // Only check for minimum fees and global fee if the execution mode is CheckTx - if !ctx.IsCheckTx() || simulate { - return next(ctx, tx, simulate) - } - - if !allowedToBypassMinFee { - // Either the transaction contains at least on message of a type - // that cannot bypass the minimum fee or the total gas limit exceeds - // the imposed threshold. As a result, check that the fees are in - // expected denominations and the amounts are greater or equal than - // the expected amounts. - - allFees = CombinedFeeRequirement(requiredGlobalFees, requiredFees) - - // Check that the fees are in expected denominations. Note that a zero fee - // is accepted if the global fee has an entry with a zero amount, e.g., 0uatoms. - if !DenomsSubsetOfIncludingZero(feeCoins, allFees) { - return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "fee is not a subset of required fees; got %s, required: %s", feeCoins, allFees) - } - // Check that the amounts of the fees are greater or equal than - // the expected amounts, i.e., at least one feeCoin amount must - // be greater or equal to one of the combined required fees. - if !IsAnyGTEIncludingZero(feeCoins, allFees) { - return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, allFees) - } - } else { + if allowedToBypassMinFee { // Transactions with zero fees are accepted if len(feeCoins) == 0 { return next(ctx, tx, simulate) @@ -112,10 +96,32 @@ func (mfd FeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, ne } } + // Either the transaction contains at least on message of a type + // that cannot bypass the minimum fee or the total gas limit exceeds + // the imposed threshold. As a result, check that the fees are in + // expected denominations and the amounts are greater or equal than + // the expected amounts. + + allFees := CombinedFeeRequirement(requiredGlobalFees, requiredFees) + + // Check that the fees are in expected denominations. Note that a zero fee + // is accepted if the global fee has an entry with a zero amount, e.g., 0uatoms. + if !DenomsSubsetOfIncludingZero(feeCoins, allFees) { + return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "fee is not a subset of required fees; got %s, required: %s", feeCoins, allFees) + } + // Check that the amounts of the fees are greater or equal than + // the expected amounts, i.e., at least one feeCoin amount must + // be greater or equal to one of the combined required fees. + if !IsAnyGTEIncludingZero(feeCoins, allFees) { + return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, allFees) + } + return next(ctx, tx, simulate) } -// ParamStoreKeyMinGasPrices type require coins sorted. getGlobalFee will also return sorted coins (might return 0denom if globalMinGasPrice is 0) +// getGlobalFee returns the global fees for a given fee tx's gas (might also returns 0denom if globalMinGasPrice is 0) +// sorted in ascending order. +// Note that ParamStoreKeyMinGasPrices type requires coins sorted. func (mfd FeeDecorator) getGlobalFee(ctx sdk.Context, feeTx sdk.FeeTx) (sdk.Coins, error) { var ( globalMinGasPrices sdk.DecCoins @@ -128,6 +134,9 @@ func (mfd FeeDecorator) getGlobalFee(ctx sdk.Context, feeTx sdk.FeeTx) (sdk.Coin // global fee is empty set, set global fee to 0uatom if len(globalMinGasPrices) == 0 { globalMinGasPrices, err = mfd.DefaultZeroGlobalFee(ctx) + if err != nil { + return sdk.Coins{}, err + } } requiredGlobalFees := make(sdk.Coins, len(globalMinGasPrices)) // Determine the required fees by multiplying each required minimum gas @@ -138,7 +147,7 @@ func (mfd FeeDecorator) getGlobalFee(ctx sdk.Context, feeTx sdk.FeeTx) (sdk.Coin requiredGlobalFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) } - return requiredGlobalFees.Sort(), err + return requiredGlobalFees.Sort(), nil } func (mfd FeeDecorator) DefaultZeroGlobalFee(ctx sdk.Context) ([]sdk.DecCoin, error) { diff --git a/x/globalfee/ante/fee_utils.go b/x/globalfee/ante/fee_utils.go index 9fa48b01e99..374f99925f1 100644 --- a/x/globalfee/ante/fee_utils.go +++ b/x/globalfee/ante/fee_utils.go @@ -1,13 +1,12 @@ package ante import ( - "math" - sdk "github.com/cosmos/cosmos-sdk/types" tmstrings "github.com/tendermint/tendermint/libs/strings" ) -// getMinGasPrice will also return sorted coins +// getMinGasPrice returns the validator's minimum gas prices +// for a given fee tx's gas sorted in ascending order func getMinGasPrice(ctx sdk.Context, feeTx sdk.FeeTx) sdk.Coins { minGasPrices := ctx.MinGasPrices() gas := feeTx.GetGas() @@ -38,11 +37,14 @@ func (mfd FeeDecorator) containsOnlyBypassMinFeeMsgs(msgs []sdk.Msg) bool { return true } -// DenomsSubsetOfIncludingZero and IsAnyGTEIncludingZero are similar to DenomsSubsetOf and IsAnyGTE in sdk. Since we allow zero coins in global fee(zero coins means the chain does not want to set a global fee but still want to define the fee's denom) +// DenomsSubsetOfIncludingZero and IsAnyGTEIncludingZero are similar to DenomsSubsetOf and IsAnyGTE in sdk. +// Since we allow zero coins in global fee(zero coins means the chain does not want to set a global fee but still want to define the fee's denom) // -// overwrite DenomsSubsetOfIncludingZero from sdk, to allow zero amt coins in superset. e.g. 1stake is DenomsSubsetOfIncludingZero 0stake. [] is the DenomsSubsetOfIncludingZero of [0stake] but not [1stake]. +// DenomsSubsetOfIncludingZero overwrites DenomsSubsetOf from sdk, to allow zero amt coins in superset. e.g. +// e.g. [1stake] is the DenomsSubsetOfIncludingZero of [0stake] and +// [] is the DenomsSubsetOfIncludingZero of [0stake] but not [1stake]. // DenomsSubsetOfIncludingZero returns true if coins's denoms is subset of coinsB's denoms. -// if coins is empty set, empty set is any sets' subset +// If coins is empty set, empty set is any sets' subset func DenomsSubsetOfIncludingZero(coins, coinsB sdk.Coins) bool { // more denoms in B than in receiver if len(coins) > len(coinsB) { @@ -67,9 +69,9 @@ func DenomsSubsetOfIncludingZero(coins, coinsB sdk.Coins) bool { return true } -// overwrite the IsAnyGTEIncludingZero from sdk to allow zero coins in coins and coinsB. -// IsAnyGTEIncludingZero returns true if coins contain at least one denom that is present at a greater or equal amount in coinsB; it returns false otherwise. -// if CoinsB is emptyset, no coins sets are IsAnyGTEIncludingZero coinsB unless coins is also empty set. +// IsAnyGTEIncludingZero overwrites the IsAnyGTE from sdk to allow zero coins in coins and coinsB. +// IsAnyGTEIncludingZero returns true if coins contain at least one denom that is present at a greater or equal amount in coinsB; +// it returns false otherwise. If CoinsB is emptyset, no coins sets are IsAnyGTEIncludingZero coinsB unless coins is also empty set. // NOTE: IsAnyGTEIncludingZero operates under the invariant that both coin sets are sorted by denoms. // contract !!!! coins must be DenomsSubsetOfIncludingZero of coinsB func IsAnyGTEIncludingZero(coins, coinsB sdk.Coins) bool { @@ -103,13 +105,13 @@ func IsAnyGTEIncludingZero(coins, coinsB sdk.Coins) bool { return false } -// return true if coinsB is empty or contains zero coins, -// CoinsB must be validate coins !!! -func ContainZeroCoins(coinsB sdk.Coins) bool { - if len(coinsB) == 0 { +// ContainZeroCoins returns true if the given coins is empty or contains zero coins, +// Note that the coins denoms must be validated, see sdk.ValidateDenom +func ContainZeroCoins(coins sdk.Coins) bool { + if len(coins) == 0 { return true } - for _, coin := range coinsB { + for _, coin := range coins { if coin.IsZero() { return true } @@ -118,7 +120,9 @@ func ContainZeroCoins(coinsB sdk.Coins) bool { return false } -// CombinedFeeRequirement will combine the global fee and min_gas_price. Both globalFees and minGasPrices must be valid, but CombinedFeeRequirement does not validate them, so it may return 0denom. +// CombinedFeeRequirement returns the global fee and min_gas_price combined and sorted. +// Both globalFees and minGasPrices must be valid, but CombinedFeeRequirement +// does not validate them, so it may return 0denom. func CombinedFeeRequirement(globalFees, minGasPrices sdk.Coins) sdk.Coins { // empty min_gas_price if len(minGasPrices) == 0 { @@ -144,23 +148,6 @@ func CombinedFeeRequirement(globalFees, minGasPrices sdk.Coins) sdk.Coins { return allFees.Sort() } -// getTxPriority returns a naive tx priority based on the amount of the smallest denomination of the fee -// provided in a transaction. -func GetTxPriority(fee sdk.Coins) int64 { - var priority int64 - for _, c := range fee { - p := int64(math.MaxInt64) - if c.Amount.IsInt64() { - p = c.Amount.Int64() - } - if priority == 0 || p < priority { - priority = p - } - } - - return priority -} - // Find replaces the functionality of Coins.Find from SDK v0.46.x func Find(coins sdk.Coins, denom string) (bool, sdk.Coin) { switch len(coins) { diff --git a/x/globalfee/module.go b/x/globalfee/module.go index 484bb7778ec..2549e8403ce 100644 --- a/x/globalfee/module.go +++ b/x/globalfee/module.go @@ -58,7 +58,11 @@ func (a AppModuleBasic) RegisterRESTRoutes(context client.Context, router *mux.R } func (a AppModuleBasic) RegisterGRPCGatewayRoutes(clientCtx client.Context, mux *runtime.ServeMux) { - _ = types.RegisterQueryHandlerClient(context.Background(), mux, types.NewQueryClient(clientCtx)) + err := types.RegisterQueryHandlerClient(context.Background(), mux, types.NewQueryClient(clientCtx)) + if err != nil { + // same behavior as in cosmos-sdk + panic(err) + } } func (a AppModuleBasic) GetTxCmd() *cobra.Command { diff --git a/x/globalfee/types/params.go b/x/globalfee/types/params.go index 59aa38cc568..c18e403e50d 100644 --- a/x/globalfee/types/params.go +++ b/x/globalfee/types/params.go @@ -45,53 +45,36 @@ func validateMinimumGasPrices(i interface{}) error { return dec.Validate() } -// Validate checks that the DecCoins are sorted, have nonnegtive amount, with a valid and unique -// denomination (i.e no duplicates). Otherwise, it returns an error. type DecCoins sdk.DecCoins +// Validate checks that the DecCoins are sorted, have nonnegtive amount, with a valid and unique +// denomination (i.e no duplicates). Otherwise, it returns an error. func (coins DecCoins) Validate() error { - switch len(coins) { - case 0: + if len(coins) == 0 { return nil + } - case 1: - // match the denom reg expr - if err := sdk.ValidateDenom(coins[0].Denom); err != nil { - return err - } - if coins[0].IsNegative() { - return fmt.Errorf("coin %s amount is negtive", coins[0]) + lowDenom := "" + seenDenoms := make(map[string]bool) + + for _, coin := range coins { + if seenDenoms[coin.Denom] { + return fmt.Errorf("duplicate denomination %s", coin.Denom) } - return nil - default: - // check single coin case - if err := (DecCoins{coins[0]}).Validate(); err != nil { + if err := sdk.ValidateDenom(coin.Denom); err != nil { return err } - - lowDenom := coins[0].Denom - seenDenoms := make(map[string]bool) - seenDenoms[lowDenom] = true - - for _, coin := range coins[1:] { - if seenDenoms[coin.Denom] { - return fmt.Errorf("duplicate denomination %s", coin.Denom) - } - if err := sdk.ValidateDenom(coin.Denom); err != nil { - return err - } - if coin.Denom <= lowDenom { - return fmt.Errorf("denomination %s is not sorted", coin.Denom) - } - if coin.IsNegative() { - return fmt.Errorf("coin %s amount is negtive", coin.Denom) - } - - // we compare each coin against the last denom - lowDenom = coin.Denom - seenDenoms[coin.Denom] = true + if coin.Denom <= lowDenom { + return fmt.Errorf("denomination %s is not sorted", coin.Denom) + } + if coin.IsNegative() { + return fmt.Errorf("coin %s amount is negative", coin.Amount) } - return nil + // we compare each coin against the last denom + lowDenom = coin.Denom + seenDenoms[coin.Denom] = true } + + return nil } diff --git a/x/globalfee/types/params_test.go b/x/globalfee/types/params_test.go index b4e67379f33..ba53a7f1f7f 100644 --- a/x/globalfee/types/params_test.go +++ b/x/globalfee/types/params_test.go @@ -46,6 +46,18 @@ func Test_validateParams(t *testing.T) { }, true, }, + "negative amount, fail": { + sdk.DecCoins{ + sdk.DecCoin{Denom: "photon", Amount: sdk.OneDec().Neg()}, + }, + true, + }, + "invalid denom, fail": { + sdk.DecCoins{ + sdk.DecCoin{Denom: "photon!", Amount: sdk.OneDec().Neg()}, + }, + true, + }, } for name, test := range tests {