From e5f1945d7414fff5f1633a069cd4603135a1b4ef Mon Sep 17 00:00:00 2001 From: yaruwangway <69694322+yaruwangway@users.noreply.github.com> Date: Wed, 5 Oct 2022 14:28:48 +0200 Subject: [PATCH] refactor: globalfee antehandler (#1781) * refactor: globalfee antehandler * fix: remove unused field * fix: linter, unused func * fix: deadlink * fix: test --- ante/ante.go | 4 +++- app/app.go | 3 ++- docs/modules/globalfee/README.md | 2 +- .../globalfee/ante/antetest}/fee_test.go | 15 ++++++++++---- .../globalfee/ante/antetest/fee_test_setup.go | 20 +++++++------------ .../ante/antetest}/fee_utils_test.go | 12 ++++++----- {ante => x/globalfee/ante}/fee.go | 12 +++++------ {ante => x/globalfee/ante}/fee_utils.go | 12 +++++------ 8 files changed, 43 insertions(+), 37 deletions(-) rename {ante => x/globalfee/ante/antetest}/fee_test.go (98%) rename ante/ante_test.go => x/globalfee/ante/antetest/fee_test_setup.go (89%) rename {ante => x/globalfee/ante/antetest}/fee_utils_test.go (97%) rename {ante => x/globalfee/ante}/fee.go (89%) rename {ante => x/globalfee/ante}/fee_utils.go (94%) diff --git a/ante/ante.go b/ante/ante.go index b3b49984..f1cfbb9b 100644 --- a/ante/ante.go +++ b/ante/ante.go @@ -7,6 +7,8 @@ import ( paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" ibcante "github.com/cosmos/ibc-go/v5/modules/core/ante" ibckeeper "github.com/cosmos/ibc-go/v5/modules/core/keeper" + + gaiafeeante "github.com/cosmos/gaia/v8/x/globalfee/ante" ) // HandlerOptions extend the SDK's AnteHandler options by requiring the IBC @@ -47,7 +49,7 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) { ante.NewTxTimeoutHeightDecorator(), ante.NewValidateMemoDecorator(opts.AccountKeeper), ante.NewConsumeGasForTxSizeDecorator(opts.AccountKeeper), - NewBypassMinFeeDecorator(opts.BypassMinFeeMsgTypes, opts.GlobalFeeSubspace), + gaiafeeante.NewFeeDecorator(opts.BypassMinFeeMsgTypes, opts.GlobalFeeSubspace), // if opts.TxFeeCheck is nil, it is the default fee check ante.NewDeductFeeDecorator(opts.AccountKeeper, opts.BankKeeper, opts.FeegrantKeeper, opts.TxFeeChecker), ante.NewSetPubKeyDecorator(opts.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators diff --git a/app/app.go b/app/app.go index f37955fb..492eede7 100644 --- a/app/app.go +++ b/app/app.go @@ -122,6 +122,7 @@ import ( gaiaante "github.com/cosmos/gaia/v8/ante" gaiaappparams "github.com/cosmos/gaia/v8/app/params" "github.com/cosmos/gaia/v8/x/globalfee" + gaiafeeante "github.com/cosmos/gaia/v8/x/globalfee/ante" "github.com/cosmos/gaia/v8/x/icamauth" icamauthkeeper "github.com/cosmos/gaia/v8/x/icamauth/keeper" icamauthtypes "github.com/cosmos/gaia/v8/x/icamauth/types" @@ -739,7 +740,7 @@ func NewGaiaApp( } feeCoins := feeTx.GetFee() - priority := gaiaante.GetTxPriority(feeCoins) + priority := gaiafeeante.GetTxPriority(feeCoins) return feeCoins, priority, nil }, diff --git a/docs/modules/globalfee/README.md b/docs/modules/globalfee/README.md index 25070154..8a3a0460 100644 --- a/docs/modules/globalfee/README.md +++ b/docs/modules/globalfee/README.md @@ -64,7 +64,7 @@ Global fees, min_gas_prices and the paid fees all allow zero coins setup. After Only global fees might contain zero coins, which is used to define the allowed denoms of paid fees. -The [Fee AnteHandle](../../../ante/fee.go) will take global fees and min_gas_prices and merge them into one [combined `sdk.Deccoins`](https://github.com/cosmos/gaia/blob/f2be720353a969b6362feff369218eb9056a60b9/ante/fee.go#L79) according to the denoms and amounts of global fees and min_gas_prices. +The [Fee AnteHandle](../../../x/globalfee/ante/fee.go) will take global fees and min_gas_prices and merge them into one [combined `sdk.Deccoins`](https://github.com/cosmos/gaia/blob/f2be720353a969b6362feff369218eb9056a60b9/ante/fee.go#L79) according to the denoms and amounts of global fees and min_gas_prices. If the paid fee is a subset of the combined fees set and the paid fee amount is greater than or equal to the required fees amount, the transaction can pass the fee check, otherwise an error will occur. diff --git a/ante/fee_test.go b/x/globalfee/ante/antetest/fee_test.go similarity index 98% rename from ante/fee_test.go rename to x/globalfee/ante/antetest/fee_test.go index 728086a2..02179c0b 100644 --- a/ante/fee_test.go +++ b/x/globalfee/ante/antetest/fee_test.go @@ -1,17 +1,24 @@ -package ante_test +package antetest import ( + "testing" + cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types" "github.com/cosmos/cosmos-sdk/testutil/testdata" sdk "github.com/cosmos/cosmos-sdk/types" ibcclienttypes "github.com/cosmos/ibc-go/v5/modules/core/02-client/types" ibcchanneltypes "github.com/cosmos/ibc-go/v5/modules/core/04-channel/types" + "github.com/stretchr/testify/suite" - "github.com/cosmos/gaia/v8/ante" gaiaapp "github.com/cosmos/gaia/v8/app" + gaiafeeante "github.com/cosmos/gaia/v8/x/globalfee/ante" globfeetypes "github.com/cosmos/gaia/v8/x/globalfee/types" ) +func TestIntegrationTestSuite(t *testing.T) { + suite.Run(t, new(IntegrationTestSuite)) +} + // test global fees and min_gas_price with bypass msg types. // please note even globalfee=0, min_gas_price=0, we do not let fee=0random_denom pass // paid fees are already sanitized by removing zero coins(through feeFlag parsing), so use sdk.NewCoins() to create it. @@ -529,9 +536,9 @@ func (s *IntegrationTestSuite) TestGlobalFeeMinimumGasFeeAnteHandler() { for name, testCase := range testCases { s.Run(name, func() { // set globalfees and min gas price - subspace := s.setupTestGlobalFeeStoreAndMinGasPrice(testCase.minGasPrice, testCase.globalFeeParams) + subspace := s.SetupTestGlobalFeeStoreAndMinGasPrice(testCase.minGasPrice, testCase.globalFeeParams) // setup antehandler - mfd := ante.NewBypassMinFeeDecorator(gaiaapp.GetDefaultBypassFeeMessages(), subspace) + mfd := gaiafeeante.NewFeeDecorator(gaiaapp.GetDefaultBypassFeeMessages(), subspace) antehandler := sdk.ChainAnteDecorators(mfd) s.Require().NoError(s.txBuilder.SetMsgs(testCase.txMsg)) diff --git a/ante/ante_test.go b/x/globalfee/ante/antetest/fee_test_setup.go similarity index 89% rename from ante/ante_test.go rename to x/globalfee/ante/antetest/fee_test_setup.go index 13915fc0..c156a3dc 100644 --- a/ante/ante_test.go +++ b/x/globalfee/ante/antetest/fee_test_setup.go @@ -1,8 +1,7 @@ -package ante_test +package antetest import ( "fmt" - "testing" "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/client/tx" @@ -12,12 +11,12 @@ import ( "github.com/cosmos/cosmos-sdk/types/tx/signing" xauthsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" "github.com/cosmos/cosmos-sdk/x/params/types" + gaiahelpers "github.com/cosmos/gaia/v8/app/helpers" "github.com/stretchr/testify/suite" tmrand "github.com/tendermint/tendermint/libs/rand" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" gaiaapp "github.com/cosmos/gaia/v8/app" - gaiahelpers "github.com/cosmos/gaia/v8/app/helpers" "github.com/cosmos/gaia/v8/x/globalfee" globfeetypes "github.com/cosmos/gaia/v8/x/globalfee/types" ) @@ -25,15 +24,10 @@ import ( type IntegrationTestSuite struct { suite.Suite - app *gaiaapp.GaiaApp - anteHandler sdk.AnteHandler - ctx sdk.Context - clientCtx client.Context - txBuilder client.TxBuilder -} - -func TestIntegrationTestSuite(t *testing.T) { - suite.Run(t, new(IntegrationTestSuite)) + app *gaiaapp.GaiaApp + ctx sdk.Context + clientCtx client.Context + txBuilder client.TxBuilder } func (s *IntegrationTestSuite) SetupTest() { @@ -52,7 +46,7 @@ func (s *IntegrationTestSuite) SetupTest() { s.clientCtx = client.Context{}.WithTxConfig(encodingConfig.TxConfig) } -func (s *IntegrationTestSuite) setupTestGlobalFeeStoreAndMinGasPrice(minGasPrice []sdk.DecCoin, globalFeeParams *globfeetypes.Params) types.Subspace { +func (s *IntegrationTestSuite) SetupTestGlobalFeeStoreAndMinGasPrice(minGasPrice []sdk.DecCoin, globalFeeParams *globfeetypes.Params) types.Subspace { subspace := s.app.GetSubspace(globalfee.ModuleName) subspace.SetParamSet(s.ctx, globalFeeParams) s.ctx = s.ctx.WithMinGasPrices(minGasPrice).WithIsCheckTx(true) diff --git a/ante/fee_utils_test.go b/x/globalfee/ante/antetest/fee_utils_test.go similarity index 97% rename from ante/fee_utils_test.go rename to x/globalfee/ante/antetest/fee_utils_test.go index b43fc13a..7dbf8702 100644 --- a/ante/fee_utils_test.go +++ b/x/globalfee/ante/antetest/fee_utils_test.go @@ -1,10 +1,12 @@ -package ante +package antetest import ( "testing" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/suite" + + "github.com/cosmos/gaia/v8/x/globalfee/ante" ) type feeUtilsTestSuite struct { @@ -55,7 +57,7 @@ func (s *feeUtilsTestSuite) TestContainZeroCoins() { } for _, test := range tests { - ok := containZeroCoins(test.c) + ok := ante.ContainZeroCoins(test.c) s.Require().Equal(test.ok, ok) } } @@ -160,7 +162,7 @@ func (s *feeUtilsTestSuite) TestCombinedFeeRequirement() { for name, test := range tests { s.Run(name, func() { - allFees := CombinedFeeRequirement(test.cGlobal, test.c) + allFees := ante.CombinedFeeRequirement(test.cGlobal, test.c) s.Require().Equal(test.combined, allFees) }) } @@ -265,7 +267,7 @@ func (s *feeUtilsTestSuite) TestDenomsSubsetOfIncludingZero() { for name, test := range tests { s.Run(name, func() { - subset := DenomsSubsetOfIncludingZero(test.set, test.superset) + subset := ante.DenomsSubsetOfIncludingZero(test.set, test.superset) s.Require().Equal(test.subset, subset) }) } @@ -406,7 +408,7 @@ func (s *feeUtilsTestSuite) TestIsAnyGTEIncludingZero() { for name, test := range tests { s.Run(name, func() { - gte := IsAnyGTEIncludingZero(test.c2, test.c1) + gte := ante.IsAnyGTEIncludingZero(test.c2, test.c1) s.Require().Equal(test.gte, gte) }) } diff --git a/ante/fee.go b/x/globalfee/ante/fee.go similarity index 89% rename from ante/fee.go rename to x/globalfee/ante/fee.go index dc5a3015..11d3a5a4 100644 --- a/ante/fee.go +++ b/x/globalfee/ante/fee.go @@ -17,13 +17,13 @@ const maxBypassMinFeeMsgGasUsage = uint64(200_000) // Note this only applies when ctx.CheckTx = true. If fee is high enough or not // CheckTx, then call next AnteHandler. // -// CONTRACT: Tx must implement FeeTx to use BypassMinFeeDecorator +// CONTRACT: Tx must implement FeeTx to use FeeDecorator // If the tx msg type is one of the bypass msg types, the tx is valid even if the min fee is lower than normally required. // If the bypass tx still carries fees, the fee denom should be the same as global fee required. -var _ sdk.AnteDecorator = BypassMinFeeDecorator{} +var _ sdk.AnteDecorator = FeeDecorator{} -type BypassMinFeeDecorator struct { +type FeeDecorator struct { BypassMinFeeMsgTypes []string GlobalMinFee globalfee.ParamSource } @@ -34,18 +34,18 @@ func DefaultZeroGlobalFee() []sdk.DecCoin { return []sdk.DecCoin{sdk.NewDecCoinFromDec(defaultZeroGlobalFeeDenom, sdk.NewDec(0))} } -func NewBypassMinFeeDecorator(bypassMsgTypes []string, paramSpace paramtypes.Subspace) BypassMinFeeDecorator { +func NewFeeDecorator(bypassMsgTypes []string, paramSpace paramtypes.Subspace) FeeDecorator { if !paramSpace.HasKeyTable() { panic("global fee paramspace was not set up via module") } - return BypassMinFeeDecorator{ + return FeeDecorator{ BypassMinFeeMsgTypes: bypassMsgTypes, GlobalMinFee: paramSpace, } } -func (mfd BypassMinFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { +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 { diff --git a/ante/fee_utils.go b/x/globalfee/ante/fee_utils.go similarity index 94% rename from ante/fee_utils.go rename to x/globalfee/ante/fee_utils.go index ccdbf01d..5abe6595 100644 --- a/ante/fee_utils.go +++ b/x/globalfee/ante/fee_utils.go @@ -10,7 +10,7 @@ import ( ) // ParamStoreKeyMinGasPrices type require coins sorted. getGlobalFee will also return sorted coins (might return 0denom if globalMinGasPrice is 0) -func (mfd BypassMinFeeDecorator) getGlobalFee(ctx sdk.Context, feeTx sdk.FeeTx) sdk.Coins { +func (mfd FeeDecorator) getGlobalFee(ctx sdk.Context, feeTx sdk.FeeTx) sdk.Coins { var globalMinGasPrices sdk.DecCoins if mfd.GlobalMinFee.Has(ctx, types.ParamStoreKeyMinGasPrices) { mfd.GlobalMinFee.Get(ctx, types.ParamStoreKeyMinGasPrices, &globalMinGasPrices) @@ -51,7 +51,7 @@ func getMinGasPrice(ctx sdk.Context, feeTx sdk.FeeTx) sdk.Coins { return requiredFees.Sort() } -func (mfd BypassMinFeeDecorator) bypassMinFeeMsgs(msgs []sdk.Msg) bool { +func (mfd FeeDecorator) bypassMinFeeMsgs(msgs []sdk.Msg) bool { for _, msg := range msgs { if tmstrings.StringInSlice(sdk.MsgTypeURL(msg), mfd.BypassMinFeeMsgTypes) { continue @@ -74,7 +74,7 @@ func DenomsSubsetOfIncludingZero(coins, coinsB sdk.Coins) bool { } // coins=[], coinsB=[0stake] // let all len(coins) == 0 pass and reject later at IsAnyGTEIncludingZero - if len(coins) == 0 && containZeroCoins(coinsB) { + if len(coins) == 0 && ContainZeroCoins(coinsB) { return true } // coins=1stake, coinsB=[0stake,1uatom] @@ -109,7 +109,7 @@ func IsAnyGTEIncludingZero(coins, coinsB sdk.Coins) bool { } // if feecoins empty (len(coins)==0 && len(coinsB) != 0 ), and globalfee has one denom of amt zero, return true if len(coins) == 0 { - return containZeroCoins(coinsB) + return ContainZeroCoins(coinsB) } // len(coinsB) != 0 && len(coins) != 0 @@ -130,7 +130,7 @@ func IsAnyGTEIncludingZero(coins, coinsB sdk.Coins) bool { // return true if coinsB is empty or contains zero coins, // CoinsB must be validate coins !!! -func containZeroCoins(coinsB sdk.Coins) bool { +func ContainZeroCoins(coinsB sdk.Coins) bool { if len(coinsB) == 0 { return true } @@ -143,7 +143,7 @@ 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 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. func CombinedFeeRequirement(globalFees, minGasPrices sdk.Coins) sdk.Coins { // empty min_gas_price if len(minGasPrices) == 0 {