Skip to content

Commit

Permalink
refactor: globalfee antehandler (#1781)
Browse files Browse the repository at this point in the history
* refactor: globalfee antehandler

* fix: remove unused field

* fix: linter, unused func

* fix: deadlink

* fix: test
  • Loading branch information
yaruwangway authored Oct 5, 2022
1 parent 850aeb7 commit e5f1945
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 37 deletions.
4 changes: 3 additions & 1 deletion ante/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -739,7 +740,7 @@ func NewGaiaApp(
}

feeCoins := feeTx.GetFee()
priority := gaiaante.GetTxPriority(feeCoins)
priority := gaiafeeante.GetTxPriority(feeCoins)

return feeCoins, priority, nil
},
Expand Down
2 changes: 1 addition & 1 deletion docs/modules/globalfee/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
15 changes: 11 additions & 4 deletions ante/fee_test.go → x/globalfee/ante/antetest/fee_test.go
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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))
Expand Down
20 changes: 7 additions & 13 deletions ante/ante_test.go → x/globalfee/ante/antetest/fee_test_setup.go
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -12,28 +11,23 @@ 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"
)

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() {
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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)
}
}
Expand Down Expand Up @@ -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)
})
}
Expand Down Expand Up @@ -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)
})
}
Expand Down Expand Up @@ -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)
})
}
Expand Down
12 changes: 6 additions & 6 deletions ante/fee.go → x/globalfee/ante/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down
12 changes: 6 additions & 6 deletions ante/fee_utils.go → x/globalfee/ante/fee_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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]
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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 {
Expand Down

0 comments on commit e5f1945

Please sign in to comment.