Fix: use bondDenom as default globalfee denom (cosmos#1824)

* fix: use bond_denom as default globalfee denom * fix: fee test by setup uatom as staking bond denom * chore: update error logs * fix: subspace notfound err type * test: add test for defaultGlobalFee * chore: return error for DefaultZeroGlobalFee * refactor: move FeeDecorator methods from fee_utils.go to fee.go * fix: use SetParamSet rather than set when setting bondDenom * chore: code improvement * fix: move panic to antehandler Co-authored-by: Danilo Pantani <[email protected]>
chillyvee · Oct 28, 2022 · a7e70bb · a7e70bb
1 parent 4e9f043
commit a7e70bb
Show file tree

Hide file tree

Showing 6 changed files with 110 additions and 41 deletions.
diff --git a/ante/ante.go b/ante/ante.go
@@ -18,6 +18,7 @@ type HandlerOptions struct {
 	IBCkeeper            *ibckeeper.Keeper
 	BypassMinFeeMsgTypes []string
 	GlobalFeeSubspace    paramtypes.Subspace
+	StakingSubspace      paramtypes.Subspace
 }
 
 func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) {
@@ -28,13 +29,16 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) {
 		return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "bank keeper is required for AnteHandler")
 	}
 	if opts.SignModeHandler == nil {
-		return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "sign mode handler is required for ante builder")
+		return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "sign mode handler is required for AnteHandler")
 	}
 	if opts.IBCkeeper == nil {
-		return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "IBC keeper is required for middlewares")
+		return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "IBC keeper is required for AnteHandler")
 	}
 	if opts.GlobalFeeSubspace.Name() == "" {
-		return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "param store is required for ante builder")
+		return nil, sdkerrors.Wrap(sdkerrors.ErrNotFound, "globalfee param store is required for AnteHandler")
+	}
+	if opts.StakingSubspace.Name() == "" {
+		return nil, sdkerrors.Wrap(sdkerrors.ErrNotFound, "staking param store is required for AnteHandler")
 	}
 
 	sigGasConsumer := opts.SigGasConsumer
@@ -49,7 +53,7 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) {
 		ante.NewTxTimeoutHeightDecorator(),
 		ante.NewValidateMemoDecorator(opts.AccountKeeper),
 		ante.NewConsumeGasForTxSizeDecorator(opts.AccountKeeper),
-		gaiafeeante.NewFeeDecorator(opts.BypassMinFeeMsgTypes, opts.GlobalFeeSubspace),
+		gaiafeeante.NewFeeDecorator(opts.BypassMinFeeMsgTypes, opts.GlobalFeeSubspace, opts.StakingSubspace),
 		// 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
@@ -26,6 +26,7 @@ import (
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	"github.com/cosmos/cosmos-sdk/x/crisis"
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
+	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 	upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/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"
@@ -211,6 +212,7 @@ func NewGaiaApp(
 			IBCkeeper:            app.IBCKeeper,
 			BypassMinFeeMsgTypes: bypassMinFeeMsgTypes,
 			GlobalFeeSubspace:    app.GetSubspace(globalfee.ModuleName),
+			StakingSubspace:      app.GetSubspace(stakingtypes.ModuleName),
 		},
 	)
 	if err != nil {

diff --git a/x/globalfee/ante/antetest/fee_test.go b/x/globalfee/ante/antetest/fee_test.go
@@ -6,6 +6,7 @@ import (
 	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
 	"github.com/cosmos/cosmos-sdk/testutil/testdata"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/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"
@@ -19,6 +20,28 @@ func TestIntegrationTestSuite(t *testing.T) {
 	suite.Run(t, new(IntegrationTestSuite))
 }
 
+func (s *IntegrationTestSuite) TestGetDefaultGlobalFees() {
+	// set globalfees and min gas price
+	globalfeeSubspace := s.SetupTestGlobalFeeStoreAndMinGasPrice([]sdk.DecCoin{}, &globfeetypes.Params{})
+
+	// set staking params
+	stakingParam := stakingtypes.DefaultParams()
+	bondDenom := "uatom"
+	stakingParam.BondDenom = bondDenom
+	stakingSubspace := s.SetupTestStakingSubspace(stakingParam)
+
+	// setup antehandler
+	mfd := gaiafeeante.NewFeeDecorator(gaiaapp.GetDefaultBypassFeeMessages(), globalfeeSubspace, stakingSubspace)
+
+	defaultGlobalFees, err := mfd.DefaultZeroGlobalFee(s.ctx)
+	s.Require().NoError(err)
+	s.Require().Greater(len(defaultGlobalFees), 0)
+
+	if defaultGlobalFees[0].Denom != bondDenom {
+		s.T().Fatalf("bond denom: %s, default global fee denom: %s", bondDenom, defaultGlobalFees[0].Denom)
+	}
+}
+
 // 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.
@@ -536,9 +559,12 @@ 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)
+			globalfeeSubspace := s.SetupTestGlobalFeeStoreAndMinGasPrice(testCase.minGasPrice, testCase.globalFeeParams)
+			stakingParam := stakingtypes.DefaultParams()
+			stakingParam.BondDenom = "uatom"
+			stakingSubspace := s.SetupTestStakingSubspace(stakingParam)
 			// setup antehandler
-			mfd := gaiafeeante.NewFeeDecorator(gaiaapp.GetDefaultBypassFeeMessages(), subspace)
+			mfd := gaiafeeante.NewFeeDecorator(gaiaapp.GetDefaultBypassFeeMessages(), globalfeeSubspace, stakingSubspace)
 			antehandler := sdk.ChainAnteDecorators(mfd)
 
 			s.Require().NoError(s.txBuilder.SetMsgs(testCase.txMsg))

diff --git a/x/globalfee/ante/antetest/fee_test_setup.go b/x/globalfee/ante/antetest/fee_test_setup.go
@@ -11,6 +11,7 @@ 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"
+	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 	gaiahelpers "github.com/cosmos/gaia/v8/app/helpers"
 	"github.com/stretchr/testify/suite"
 	tmrand "github.com/tendermint/tendermint/libs/rand"
@@ -54,6 +55,12 @@ func (s *IntegrationTestSuite) SetupTestGlobalFeeStoreAndMinGasPrice(minGasPrice
 	return subspace
 }
 
+// SetupTestStakingSubspace sets uatom as bond denom for the fee tests.
+func (s *IntegrationTestSuite) SetupTestStakingSubspace(params stakingtypes.Params) types.Subspace {
+	s.app.GetSubspace(stakingtypes.ModuleName).SetParamSet(s.ctx, &params)
+	return s.app.GetSubspace(stakingtypes.ModuleName)
+}
+
 func (s *IntegrationTestSuite) CreateTestTx(privs []cryptotypes.PrivKey, accNums []uint64, accSeqs []uint64, chainID string) (xauthsigning.Tx, error) {
 	var sigsV2 []signing.SignatureV2
 	for i, priv := range privs {

diff --git a/x/globalfee/ante/fee.go b/x/globalfee/ante/fee.go
@@ -1,9 +1,13 @@
 package ante
 
 import (
+	"errors"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	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/v8/x/globalfee/types"
 
 	"github.com/cosmos/gaia/v8/x/globalfee"
 )
@@ -26,22 +30,22 @@ var _ sdk.AnteDecorator = FeeDecorator{}
 type FeeDecorator struct {
 	BypassMinFeeMsgTypes []string
 	GlobalMinFee         globalfee.ParamSource
+	StakingSubspace      paramtypes.Subspace
 }
 
-const defaultZeroGlobalFeeDenom = "uatom"
-
-func DefaultZeroGlobalFee() []sdk.DecCoin {
-	return []sdk.DecCoin{sdk.NewDecCoinFromDec(defaultZeroGlobalFeeDenom, sdk.NewDec(0))}
-}
-
-func NewFeeDecorator(bypassMsgTypes []string, paramSpace paramtypes.Subspace) FeeDecorator {
-	if !paramSpace.HasKeyTable() {
+func NewFeeDecorator(bypassMsgTypes []string, globalfeeSubspace, stakingSubspace paramtypes.Subspace) FeeDecorator {
+	if !globalfeeSubspace.HasKeyTable() {
 		panic("global fee paramspace was not set up via module")
 	}
 
+	if !stakingSubspace.HasKeyTable() {
+		panic("staking paramspace was not set up via module")
+	}
+
 	return FeeDecorator{
 		BypassMinFeeMsgTypes: bypassMsgTypes,
-		GlobalMinFee:         paramSpace,
+		GlobalMinFee:         globalfeeSubspace,
+		StakingSubspace:      stakingSubspace,
 	}
 }
 
@@ -68,7 +72,10 @@ func (mfd FeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, ne
 
 	if ctx.IsCheckTx() && !simulate && !allowedToBypassMinFee {
 
-		requiredGlobalFees := mfd.getGlobalFee(ctx, feeTx)
+		requiredGlobalFees, err := mfd.getGlobalFee(ctx, feeTx)
+		if err != nil {
+			panic(err)
+		}
 		allFees = CombinedFeeRequirement(requiredGlobalFees, requiredFees)
 
 		// this is to ban 1stake passing if the globalfee is 1photon or 0photon
@@ -84,7 +91,10 @@ func (mfd FeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, ne
 
 	// when the tx is bypass msg type, still need to check the denom is not some unknown denom
 	if ctx.IsCheckTx() && !simulate && allowedToBypassMinFee {
-		requiredGlobalFees := mfd.getGlobalFee(ctx, feeTx)
+		requiredGlobalFees, err := mfd.getGlobalFee(ctx, feeTx)
+		if err != nil {
+			panic(err)
+		}
 		// bypass tx without pay fee
 		if len(feeCoins) == 0 {
 			return next(ctx, tx, simulate)
@@ -97,3 +107,47 @@ func (mfd FeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, ne
 
 	return next(ctx, tx, simulate)
 }
+
+// ParamStoreKeyMinGasPrices type require coins sorted. getGlobalFee will also return sorted coins (might return 0denom if globalMinGasPrice is 0)
+func (mfd FeeDecorator) getGlobalFee(ctx sdk.Context, feeTx sdk.FeeTx) (sdk.Coins, error) {
+	var (
+		globalMinGasPrices sdk.DecCoins
+		err                error
+	)
+
+	if mfd.GlobalMinFee.Has(ctx, types.ParamStoreKeyMinGasPrices) {
+		mfd.GlobalMinFee.Get(ctx, types.ParamStoreKeyMinGasPrices, &globalMinGasPrices)
+	}
+	// global fee is empty set, set global fee to 0uatom
+	if len(globalMinGasPrices) == 0 {
+		globalMinGasPrices, err = mfd.DefaultZeroGlobalFee(ctx)
+	}
+	requiredGlobalFees := make(sdk.Coins, len(globalMinGasPrices))
+	// Determine the required fees by multiplying each required minimum gas
+	// price by the gas limit, where fee = ceil(minGasPrice * gasLimit).
+	glDec := sdk.NewDec(int64(feeTx.GetGas()))
+	for i, gp := range globalMinGasPrices {
+		fee := gp.Amount.Mul(glDec)
+		requiredGlobalFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt())
+	}
+
+	return requiredGlobalFees.Sort(), err
+}
+
+func (mfd FeeDecorator) DefaultZeroGlobalFee(ctx sdk.Context) ([]sdk.DecCoin, error) {
+	bondDenom := mfd.getBondDenom(ctx)
+	if bondDenom == "" {
+		return nil, errors.New("empty staking bond denomination")
+	}
+
+	return []sdk.DecCoin{sdk.NewDecCoinFromDec(bondDenom, sdk.NewDec(0))}, nil
+}
+
+func (mfd FeeDecorator) getBondDenom(ctx sdk.Context) string {
+	var bondDenom string
+	if mfd.StakingSubspace.Has(ctx, stakingtypes.KeyBondDenom) {
+		mfd.StakingSubspace.Get(ctx, stakingtypes.KeyBondDenom, &bondDenom)
+	}
+
+	return bondDenom
+}
diff --git a/x/globalfee/ante/fee_utils.go b/x/globalfee/ante/fee_utils.go
@@ -5,32 +5,8 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	tmstrings "github.com/tendermint/tendermint/libs/strings"
-
-	"github.com/cosmos/gaia/v8/x/globalfee/types"
 )
 
-// ParamStoreKeyMinGasPrices type require coins sorted. getGlobalFee will also return sorted coins (might return 0denom if globalMinGasPrice is 0)
-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)
-	}
-	// global fee is empty set, set global fee to 0uatom
-	if len(globalMinGasPrices) == 0 {
-		globalMinGasPrices = DefaultZeroGlobalFee()
-	}
-	requiredGlobalFees := make(sdk.Coins, len(globalMinGasPrices))
-	// Determine the required fees by multiplying each required minimum gas
-	// price by the gas limit, where fee = ceil(minGasPrice * gasLimit).
-	glDec := sdk.NewDec(int64(feeTx.GetGas()))
-	for i, gp := range globalMinGasPrices {
-		fee := gp.Amount.Mul(glDec)
-		requiredGlobalFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt())
-	}
-
-	return requiredGlobalFees.Sort()
-}
-
 // getMinGasPrice will also return sorted coins
 func getMinGasPrice(ctx sdk.Context, feeTx sdk.FeeTx) sdk.Coins {
 	minGasPrices := ctx.MinGasPrices()