Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

global fee refactor #2316

Merged
merged 32 commits into from
Apr 3, 2023
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
7ca8bf4
- Update doc to follow Go conventions
sainoe Feb 21, 2023
52efa9d
update params
sainoe Feb 22, 2023
d47a2af
rebase and fix nits
sainoe Mar 16, 2023
2d82576
add GetMinGasPrice UT
sainoe Mar 17, 2023
90be614
remove unused feetestsuite
sainoe Mar 17, 2023
c29fbfe
rebase
sainoe Mar 20, 2023
f41c3a3
update doc
sainoe Mar 20, 2023
b086f14
nits
sainoe Mar 20, 2023
2ff680a
nits
sainoe Mar 21, 2023
4c26632
refactor fee tests setup
sainoe Mar 21, 2023
5d3a6cd
remove comments
sainoe Mar 21, 2023
0070306
Update x/globalfee/ante/antetest/fee_test.go
sainoe Mar 21, 2023
e9c4bf8
Update x/globalfee/ante/antetest/fee_test.go
sainoe Mar 21, 2023
6c3a5a6
Update x/globalfee/ante/antetest/fee_test.go
sainoe Mar 21, 2023
cbfa419
Update x/globalfee/ante/antetest/fee_test.go
sainoe Mar 21, 2023
547c55a
Update x/globalfee/ante/antetest/fee_test.go
sainoe Mar 21, 2023
76dea00
Update x/globalfee/ante/antetest/fee_test.go
sainoe Mar 21, 2023
935452a
Merge remote-tracking branch 'origin/sainoe/global_fee' into sainoe/g…
sainoe Mar 21, 2023
fdaa76f
fix GetMinGasPrice to not return nil coins
sainoe Mar 22, 2023
ed13007
min gas price zero coins edge cases
sainoe Mar 22, 2023
8f20d53
min gas price zero coins edge cases
sainoe Mar 22, 2023
0922810
remove debug logs
sainoe Mar 22, 2023
679c2e5
udpate comments
sainoe Mar 22, 2023
3bb55cf
udpate comments
sainoe Mar 22, 2023
9ca89a4
add comment
sainoe Mar 22, 2023
dc73b60
Merge branch 'sainoe/global_fee_updated' into sainoe/global_fee
sainoe Mar 22, 2023
b923ed2
Update x/globalfee/types/params.go
sainoe Mar 22, 2023
81d7e49
Update x/globalfee/types/params.go
sainoe Mar 22, 2023
c6bde62
fix: lint
Mar 23, 2023
10dd1d0
Merge branch 'main' into sainoe/global_fee
MSalopek Mar 23, 2023
d046942
update according to PR comments
Mar 24, 2023
ebc3824
Update x/globalfee/ante/fee.go
yaruwangway Mar 24, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions ante/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ import (
gaiafeeante "github.com/cosmos/gaia/v9/x/globalfee/ante"
)

// maxTotalBypassMinFeeMsgGasUsage is the allowed maximum gas usage
// for all the bypass msgs in a transactions.
// A transaction that contains only bypass message types and the gas usage does not
// exceed maxTotalBypassMinFeeMsgGasUsage can be accepted with a zero fee.
// For details, see gaiafeeante.NewFeeDecorator()
var maxTotalBypassMinFeeMsgGasUsage uint64 = 1_000_000

// HandlerOptions extend the SDK's AnteHandler options by requiring the IBC
// channel keeper.
type HandlerOptions struct {
Expand Down Expand Up @@ -53,13 +60,6 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) {
sigGasConsumer = ante.DefaultSigVerificationGasConsumer
}

// maxTotalBypassMinFeeMsgGasUsage is the allowed maximum gas usage
// for all the bypass msgs in a transactions.
// A transaction that contains only bypass message types and the gas usage does not
// exceed maxTotalBypassMinFeeMsgGasUsage can be accepted with a zero fee.
// For details, see gaiafeeante.NewFeeDecorator()
var maxTotalBypassMinFeeMsgGasUsage uint64 = 1_000_000

anteDecorators := []sdk.AnteDecorator{
ante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first
ante.NewRejectExtensionOptionsDecorator(),
Expand All @@ -69,7 +69,6 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) {
ante.NewConsumeGasForTxSizeDecorator(opts.AccountKeeper),
NewGovPreventSpamDecorator(opts.Codec, opts.GovKeeper),
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
ante.NewValidateSigCountDecorator(opts.AccountKeeper),
Expand Down
2 changes: 1 addition & 1 deletion docs/modules/globalfee.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ 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"]
sainoe marked this conversation as resolved.
Show resolved Hide resolved

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.
Expand Down
273 changes: 190 additions & 83 deletions x/globalfee/ante/antetest/fee_test.go

Large diffs are not rendered by default.

24 changes: 21 additions & 3 deletions x/globalfee/ante/antetest/fee_test_setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ import (
xauthsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
"github.com/cosmos/cosmos-sdk/x/params/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
gaiahelpers "github.com/cosmos/gaia/v9/app/helpers"
"github.com/stretchr/testify/suite"
tmrand "github.com/tendermint/tendermint/libs/rand"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"

gaiahelpers "github.com/cosmos/gaia/v9/app/helpers"
gaiafeeante "github.com/cosmos/gaia/v9/x/globalfee/ante"

gaiaapp "github.com/cosmos/gaia/v9/app"
"github.com/cosmos/gaia/v9/x/globalfee"
globfeetypes "github.com/cosmos/gaia/v9/x/globalfee/types"
Expand All @@ -31,6 +33,11 @@ type IntegrationTestSuite struct {
txBuilder client.TxBuilder
}

var (
testBondDenom = "uatom"
testMaxTotalBypassMinFeeMsgGasUsage uint64 = 1_000_000
)

func (s *IntegrationTestSuite) SetupTest() {
app := gaiahelpers.Setup(s.T())
ctx := app.BaseApp.NewContext(false, tmproto.Header{
Expand All @@ -47,12 +54,23 @@ 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) (gaiafeeante.FeeDecorator, sdk.AnteHandler) {
subspace := s.app.GetSubspace(globalfee.ModuleName)
subspace.SetParamSet(s.ctx, globalFeeParams)
s.ctx = s.ctx.WithMinGasPrices(minGasPrice).WithIsCheckTx(true)

return subspace
// set staking params
stakingParam := stakingtypes.DefaultParams()
stakingParam.BondDenom = testBondDenom
stakingSubspace := s.SetupTestStakingSubspace(stakingParam)

// build fee decorator
feeDecorator := gaiafeeante.NewFeeDecorator(gaiaapp.GetDefaultBypassFeeMessages(), subspace, stakingSubspace, uint64(1_000_000))

// chain fee decorator to antehandler
antehandler := sdk.ChainAnteDecorators(feeDecorator)

return feeDecorator, antehandler
}

// SetupTestStakingSubspace sets uatom as bond denom for the fee tests.
Expand Down
115 changes: 81 additions & 34 deletions x/globalfee/ante/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@ import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"

"github.com/cosmos/gaia/v9/x/globalfee/types"

tmstrings "github.com/tendermint/tendermint/libs/strings"

"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.
Expand Down Expand Up @@ -49,73 +52,80 @@ 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")
}

// Get required Global Fee and min gas price
requiredGlobalFees, err := mfd.getGlobalFee(ctx, feeTx)
if err != nil {
sainoe marked this conversation as resolved.
Show resolved Hide resolved
return ctx, err
}
requiredFees := GetMinGasPrice(ctx, int64(feeTx.GetGas()))

// Only check for minimum fees and global fee if the execution mode is CheckTx
if !ctx.IsCheckTx() || simulate {
yaruwangway marked this conversation as resolved.
Show resolved Hide resolved
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 MaxTotalBypassMinFeeMsgGasUsage,
// i.e., totalGas <= MaxTotalBypassMinFeeMsgGasUsage
// i.e., totalGas <= MaxBypassMinFeeMsgGasUsage
yaruwangway marked this conversation as resolved.
Show resolved Hide resolved
//
// Otherwise, minimum fees and global fees are checked to prevent spam.
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)
}
allowedToBypassMinFee := mfd.ContainsOnlyBypassMinFeeMsgs(msgs) && doesNotExceedMaxGasUsage

if !allowedToBypassMinFee {
// Either the transaction contains at least on message of a type
if allowedToBypassMinFee {
// Transactions with zero fees are accepted
if len(feeCoins) == 0 {
return next(ctx, tx, simulate)
}
// If the transaction fee is non-zero, then check that the fees are in
// expected denominations.
if !DenomsSubsetOfIncludingZero(feeCoins, requiredGlobalFees) {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "fees denom is wrong; got: %s required: %s", feeCoins, requiredGlobalFees)
}
} else {
// Either the transaction contains at least one 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)
combinedFees := 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)
if !DenomsSubsetOfIncludingZero(feeCoins, combinedFees) {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "fee is not a subset of required fees; got %s, required: %s", feeCoins, combinedFees)
}
// 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 {
// Transactions with zero fees are accepted
if len(feeCoins) == 0 {
return next(ctx, tx, simulate)
}
// If the transaction fee is non-zero, then check that the fees are in
// expected denominations.
if !DenomsSubsetOfIncludingZero(feeCoins, requiredGlobalFees) {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "fees denom is wrong; got: %s required: %s", feeCoins, requiredGlobalFees)
if !IsAnyGTEIncludingZero(feeCoins, combinedFees) {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, combinedFees)
}
}

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
Expand All @@ -128,6 +138,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
Expand All @@ -138,7 +151,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) {
Expand All @@ -158,3 +171,37 @@ func (mfd FeeDecorator) getBondDenom(ctx sdk.Context) string {

return bondDenom
}

// ContainsOnlyBypassMinFeeMsgs returns true if all the given msgs type are listed
// in the BypassMinFeeMsgTypes of the FeeDecorator.
func (mfd FeeDecorator) ContainsOnlyBypassMinFeeMsgs(msgs []sdk.Msg) bool {
for _, msg := range msgs {
sainoe marked this conversation as resolved.
Show resolved Hide resolved
if tmstrings.StringInSlice(sdk.MsgTypeURL(msg), mfd.BypassMinFeeMsgTypes) {
continue
}
return false
}

return true
}

// GetMinGasPrice returns the validator's minimum gas prices
yaruwangway marked this conversation as resolved.
Show resolved Hide resolved
// fees given a gas limit
func GetMinGasPrice(ctx sdk.Context, gasLimit int64) sdk.Coins {
minGasPrices := ctx.MinGasPrices()
// special case: if minGasPrices=[], requiredFees=[]
if minGasPrices.IsZero() {
return sdk.Coins{}
}

requiredFees := make(sdk.Coins, len(minGasPrices))
// Determine the required fees by multiplying each required minimum gas
// price by the gas limit, where fee = ceil(minGasPrice * gasLimit).
glDec := sdk.NewDec(gasLimit)
for i, gp := range minGasPrices {
fee := gp.Amount.Mul(glDec)
requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt())
}

return requiredFees.Sort()
}
Loading