diff --git a/ante/ante.go b/ante/ante.go index f1e1ce391da..15a097ccd43 100644 --- a/ante/ante.go +++ b/ante/ante.go @@ -50,7 +50,7 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) { // so that a transaction that contains only message types that can // bypass the minimum fee can be accepted with a zero fee. // For details, see gaiafeeante.NewFeeDecorator() - var maxBypassMinFeeMsgGasUsage uint64 = 200_000 + var maxTotalBypassMinFeeMsgGasUsage uint64 = 1_000_000 anteDecorators := []sdk.AnteDecorator{ ante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first @@ -59,7 +59,7 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) { ante.NewTxTimeoutHeightDecorator(), ante.NewValidateMemoDecorator(opts.AccountKeeper), ante.NewConsumeGasForTxSizeDecorator(opts.AccountKeeper), - gaiafeeante.NewFeeDecorator(opts.BypassMinFeeMsgTypes, opts.GlobalFeeSubspace, opts.StakingSubspace, maxBypassMinFeeMsgGasUsage), + gaiafeeante.NewFeeDecorator(opts.BypassMinFeeMsgTypes, opts.GlobalFeeSubspace, opts.StakingSubspace, maxTotalBypassMinFeeMsgGasUsage), ante.NewDeductFeeDecorator(opts.AccountKeeper, opts.BankKeeper, opts.FeegrantKeeper), ante.NewSetPubKeyDecorator(opts.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators diff --git a/app/app.go b/app/app.go index 78455444069..dbbbbb4c491 100644 --- a/app/app.go +++ b/app/app.go @@ -234,6 +234,8 @@ func GetDefaultBypassFeeMessages() []string { sdk.MsgTypeURL(&ibcchanneltypes.MsgRecvPacket{}), sdk.MsgTypeURL(&ibcchanneltypes.MsgAcknowledgement{}), sdk.MsgTypeURL(&ibcclienttypes.MsgUpdateClient{}), + sdk.MsgTypeURL(&ibcchanneltypes.MsgTimeout{}), + sdk.MsgTypeURL(&ibcchanneltypes.MsgTimeoutOnClose{}), } } diff --git a/docs/modules/globalfee.md b/docs/modules/globalfee.md index b8853c0b96b..e3027769688 100644 --- a/docs/modules/globalfee.md +++ b/docs/modules/globalfee.md @@ -51,14 +51,15 @@ The `minimum-gas-prices` config parameter allows node operators to impose additi Bypass messages are messages that are exempt from paying fees. The above global fees and `minimum-gas-prices` checks do not apply for transactions that satisfy the following conditions: - Contains only bypass message types, i.e., bypass transactions. -- The total gas used is less than or equal to `len(messages) * MaxBypassMinFeeMsgGasUsage`. Note: the current `MaxBypassMinFeeMsgGasUsage` is set to `200,000`. +- The total gas used is less than or equal to `MaxTotalBypassMinFeeMsgGasUsage`. Note: the current `MaxTotalBypassMinFeeMsgGasUsage` is set to `1,000,000`. - In case of non-zero transaction fees, the denom has to be a subset of denoms defined in the global fees list. Node operators can configure `bypass-min-fee-msg-types` in `config/app.toml`. -Nodes created using Gaiad `v7.0.2` or later use `["/ibc.core.channel.v1.MsgRecvPacket", "/ibc.core.channel.v1.MsgAcknowledgement","/ibc.applications.transfer.v1.MsgTransfer"]` as defaults. Nodes with `bypass-min-fee-msg-types = []` or missing this field in `app.toml` also use default bypass message types. - -Nodes created using Gaiad `v7.0.1` or earlier do not have `bypass-min-fee-msg-types` configured in `config/app.toml` - they are also using default values. The `bypass-min-fee-msg-types` config option can be added to `config/app.toml` before the `[telemetry]` field. +- Nodes created using Gaiad `v7.0.2` or later use `["/ibc.core.channel.v1.MsgRecvPacket", "/ibc.core.channel.v1.MsgAcknowledgement","/ibc.applications.transfer.v1.MsgTransfer"]` as defaults. +- Nodes created using Gaiad `v9.0.1` or later use `["/ibc.core.channel.v1.MsgRecvPacket", "/ibc.core.channel.v1.MsgAcknowledgement","/ibc.applications.transfer.v1.MsgTransfer", "/ibc.core.channel.v1.MsgTimeout", "/ibc.core.channel.v1.MsgTimeoutOnClose"]` as defaults. +- Node Nodes with `bypass-min-fee-msg-types = []` or missing this field in `app.toml` also use default bypass message types. +- Nodes created using Gaiad `v7.0.1` and `v7.0.0` do not have `bypass-min-fee-msg-types` configured in `config/app.toml` - they are also using same default values as in `v7.0.2`. The `bypass-min-fee-msg-types` config option can be added to `config/app.toml` before the `[telemetry]` field. An example of `bypass-min-fee-msg-types` in `app.toml`: @@ -72,7 +73,7 @@ An example of `bypass-min-fee-msg-types` in `app.toml`: # # Example: # ["/ibc.core.channel.v1.MsgRecvPacket", "/ibc.core.channel.v1.MsgAcknowledgement", ...] -bypass-min-fee-msg-types = ["/ibc.core.channel.v1.MsgRecvPacket", "/ibc.core.channel.v1.MsgAcknowledgement","/ibc.applications.transfer.v1.MsgTransfer"] +bypass-min-fee-msg-types = ["/ibc.core.channel.v1.MsgRecvPacket", "/ibc.core.channel.v1.MsgAcknowledgement","/ibc.applications.transfer.v1.MsgTransfer", "/ibc.core.channel.v1.MsgTimeout", "/ibc.core.channel.v1.MsgTimeoutOnClose"] ``` @@ -200,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/antetest/fee_test.go b/x/globalfee/ante/antetest/fee_test.go index e951ac0a288..2cb1aeffdca 100644 --- a/x/globalfee/ante/antetest/fee_test.go +++ b/x/globalfee/ante/antetest/fee_test.go @@ -468,8 +468,7 @@ func (s *IntegrationTestSuite) TestGlobalFeeMinimumGasFeeAnteHandler() { txCheck: true, expErr: false, }, - // test bypass msg - "msg type ibc, zero fee in globalfee denom": { + "bypass msg type: ibc.core.channel.v1.MsgRecvPacket": { minGasPrice: minGasPrice, globalFeeParams: globalfeeParamsLow, gasPrice: sdk.NewCoins(sdk.NewCoin("uatom", sdk.ZeroInt())), @@ -479,6 +478,46 @@ func (s *IntegrationTestSuite) TestGlobalFeeMinimumGasFeeAnteHandler() { txCheck: true, expErr: false, }, + "bypass msg type: ibc.core.channel.v1.MsgTimeout": { + minGasPrice: minGasPrice, + globalFeeParams: globalfeeParamsLow, + gasPrice: sdk.NewCoins(sdk.NewCoin("uatom", sdk.ZeroInt())), + gasLimit: newTestGasLimit(), + txMsg: ibcchanneltypes.NewMsgTimeout( + ibcchanneltypes.Packet{}, 1, nil, ibcclienttypes.Height{}, ""), + txCheck: true, + expErr: false, + }, + "bypass msg type: ibc.core.channel.v1.MsgTimeoutOnClose": { + minGasPrice: minGasPrice, + globalFeeParams: globalfeeParamsLow, + gasPrice: sdk.NewCoins(sdk.NewCoin("uatom", sdk.ZeroInt())), + gasLimit: newTestGasLimit(), + txMsg: ibcchanneltypes.NewMsgTimeout( + ibcchanneltypes.Packet{}, 2, nil, ibcclienttypes.Height{}, ""), + txCheck: true, + expErr: false, + }, + "bypass msg gas usage exceeds maxTotalBypassMinFeeMsgGasUsage": { + minGasPrice: minGasPrice, + globalFeeParams: globalfeeParamsLow, + gasPrice: sdk.NewCoins(sdk.NewCoin("uatom", sdk.ZeroInt())), + gasLimit: 2 * newTestMaxTotalBypassMinFeeMsgGasUsage(), + txMsg: ibcchanneltypes.NewMsgTimeout( + ibcchanneltypes.Packet{}, 2, nil, ibcclienttypes.Height{}, ""), + txCheck: true, + expErr: true, + }, + "bypass msg gas usage equals to maxTotalBypassMinFeeMsgGasUsage": { + minGasPrice: minGasPrice, + globalFeeParams: globalfeeParamsLow, + gasPrice: sdk.NewCoins(sdk.NewCoin("uatom", sdk.ZeroInt())), + gasLimit: newTestMaxTotalBypassMinFeeMsgGasUsage(), + txMsg: ibcchanneltypes.NewMsgTimeout( + ibcchanneltypes.Packet{}, 3, nil, ibcclienttypes.Height{}, ""), + txCheck: true, + expErr: false, + }, "msg type ibc, zero fee not in globalfee denom": { minGasPrice: minGasPrice, globalFeeParams: globalfeeParamsLow, @@ -573,9 +612,8 @@ func (s *IntegrationTestSuite) TestGlobalFeeMinimumGasFeeAnteHandler() { stakingParam.BondDenom = "uatom" stakingSubspace := s.SetupTestStakingSubspace(stakingParam) // setup antehandler - mfd := gaiafeeante.NewFeeDecorator(gaiaapp.GetDefaultBypassFeeMessages(), globalfeeSubspace, stakingSubspace, newTestGasLimit()) + mfd := gaiafeeante.NewFeeDecorator(gaiaapp.GetDefaultBypassFeeMessages(), globalfeeSubspace, stakingSubspace, newTestMaxTotalBypassMinFeeMsgGasUsage()) antehandler := sdk.ChainAnteDecorators(mfd) - s.Require().NoError(s.txBuilder.SetMsgs(testCase.txMsg)) s.txBuilder.SetFeeAmount(testCase.gasPrice) s.txBuilder.SetGasLimit(testCase.gasLimit) @@ -597,3 +635,7 @@ func (s *IntegrationTestSuite) TestGlobalFeeMinimumGasFeeAnteHandler() { func newTestGasLimit() uint64 { return 200000 } + +func newTestMaxTotalBypassMinFeeMsgGasUsage() uint64 { + return 1000000 +} diff --git a/x/globalfee/ante/fee.go b/x/globalfee/ante/fee.go index 650acb2f779..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. @@ -26,13 +26,13 @@ import ( var _ sdk.AnteDecorator = FeeDecorator{} type FeeDecorator struct { - BypassMinFeeMsgTypes []string - GlobalMinFee globalfee.ParamSource - StakingSubspace paramtypes.Subspace - MaxBypassMinFeeMsgGasUsage uint64 + BypassMinFeeMsgTypes []string + GlobalMinFee globalfee.ParamSource + StakingSubspace paramtypes.Subspace + MaxTotalBypassMinFeeMsgGasUsage uint64 } -func NewFeeDecorator(bypassMsgTypes []string, globalfeeSubspace, stakingSubspace paramtypes.Subspace, maxBypassMinFeeMsgGasUsage uint64) FeeDecorator { +func NewFeeDecorator(bypassMsgTypes []string, globalfeeSubspace, stakingSubspace paramtypes.Subspace, maxTotalBypassMinFeeMsgGasUsage uint64) FeeDecorator { if !globalfeeSubspace.HasKeyTable() { panic("global fee paramspace was not set up via module") } @@ -42,66 +42,49 @@ func NewFeeDecorator(bypassMsgTypes []string, globalfeeSubspace, stakingSubspace } return FeeDecorator{ - BypassMinFeeMsgTypes: bypassMsgTypes, - GlobalMinFee: globalfeeSubspace, - StakingSubspace: stakingSubspace, - MaxBypassMinFeeMsgGasUsage: maxBypassMinFeeMsgGasUsage, + BypassMinFeeMsgTypes: bypassMsgTypes, + GlobalMinFee: globalfeeSubspace, + StakingSubspace: stakingSubspace, + MaxTotalBypassMinFeeMsgGasUsage: maxTotalBypassMinFeeMsgGasUsage, } } +// 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() - // 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 MaxBypassMinFeeMsgGasUsage, - // i.e., totalGas <= len(msgs) * MaxBypassMinFeeMsgGasUsage - // Otherwise, minimum fees and global fees are checked to prevent spam. - containsOnlyBypassMinFeeMsgs := mfd.bypassMinFeeMsgs(msgs) - doesNotExceedMaxGasUsage := gas <= uint64(len(msgs))*mfd.MaxBypassMinFeeMsgGasUsage - allowedToBypassMinFee := containsOnlyBypassMinFeeMsgs && doesNotExceedMaxGasUsage - - var allFees sdk.Coins + // Get required Global Fee and min gas price 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) + // 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, + // i.e., totalGas <= MaxTotalBypassMinFeeMsgGasUsage + // Otherwise, minimum fees and global fees are checked to prevent spam. + doesNotExceedMaxGasUsage := gas <= mfd.MaxTotalBypassMinFeeMsgGasUsage + allowedToBypassMinFee := mfd.containsOnlyBypassMinFeeMsgs(msgs) && doesNotExceedMaxGasUsage - // 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) @@ -113,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 @@ -129,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 @@ -139,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 c5b353a888b..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() @@ -27,7 +26,7 @@ func getMinGasPrice(ctx sdk.Context, feeTx sdk.FeeTx) sdk.Coins { return requiredFees.Sort() } -func (mfd FeeDecorator) bypassMinFeeMsgs(msgs []sdk.Msg) bool { +func (mfd FeeDecorator) containsOnlyBypassMinFeeMsgs(msgs []sdk.Msg) bool { for _, msg := range msgs { if tmstrings.StringInSlice(sdk.MsgTypeURL(msg), mfd.BypassMinFeeMsgTypes) { continue @@ -38,11 +37,14 @@ func (mfd FeeDecorator) bypassMinFeeMsgs(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..305849ea836 100644 --- a/x/globalfee/types/params.go +++ b/x/globalfee/types/params.go @@ -45,53 +45,37 @@ 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 {