feat: refactor global fee module for SDK v47 (#2660)

* save * upgrade GlobalFee module w UTs * make pass BypassFee e2e with sdk FeeChecker Tx priority * full e2e pass wo bypass msg hermes * pass bypass fee e2e tests * update Hermes Dockerfile * build Hermes image in CI * fix Hermes Builder * fix Hermes Builder * test local min gas prices REST query * fix linter and typos * add FeeCheck ante handler comment --------- Co-authored-by: yaruwangway <[email protected]>
cosmos · Jul 24, 2023 · 0510ee7 · 0510ee7
1 parent f5fc4ba
commit 0510ee7
Show file tree

Hide file tree

Showing 41 changed files with 4,947 additions and 3,489 deletions.
diff --git a/.changelog/unreleased/features/2629-add-query-local-min-gas-prices.md b/.changelog/unreleased/features/2629-add-query-local-min-gas-prices.md
@@ -0,0 +1,3 @@
+- register NodeService to enable query /cosmos/base/node/v1beta1/config
+  gRPC query to disclose node operator's configured minimum-gas-price.
+  ([\#2629](https://github.com/cosmos/gaia/issues/2629))
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -73,8 +73,10 @@ jobs:
             **/**.go
             go.mod
             go.sum
-      - name: Build Docker Image
+      - name: Build Gaia Docker Image
         run: make docker-build-debug
+      - name: Build Hermes Docker Image
+        run: make docker-build-hermes
       - name: Test E2E
         run: make test-e2e
 

diff --git a/Makefile b/Makefile
@@ -234,7 +234,7 @@ docker-build-debug:
 # TODO: Push this to the Cosmos Dockerhub so we don't have to keep building it
 # in CI.
 docker-build-hermes:
-	@cd tests/e2e/docker; docker build -t cosmos/hermes-e2e:latest -f hermes.Dockerfile .
+	@cd tests/e2e/docker; docker build -t ghcr.io/cosmos/hermes-e2e:1.4.1 -f hermes.Dockerfile .
 
 docker-build-all: docker-build-debug docker-build-hermes
 

diff --git a/ante/ante.go b/ante/ante.go
@@ -7,11 +7,12 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/auth/ante"
 	govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper"
 	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
+	stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
 	ibcante "github.com/cosmos/ibc-go/v7/modules/core/ante"
 	ibckeeper "github.com/cosmos/ibc-go/v7/modules/core/keeper"
 
 	gaiaerrors "github.com/cosmos/gaia/v11/types/errors"
-	// gaiafeeante "github.com/cosmos/gaia/v11/x/globalfee/ante"
+	gaiafeeante "github.com/cosmos/gaia/v11/x/globalfee/ante"
 )
 
 // HandlerOptions extend the SDK's AnteHandler options by requiring the IBC
@@ -22,7 +23,8 @@ type HandlerOptions struct {
 	GovKeeper         *govkeeper.Keeper
 	IBCkeeper         *ibckeeper.Keeper
 	GlobalFeeSubspace paramtypes.Subspace
-	StakingSubspace   paramtypes.Subspace
+	StakingKeeper     *stakingkeeper.Keeper
+	TxFeeChecker      ante.TxFeeChecker
 }
 
 func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) {
@@ -39,10 +41,11 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) {
 		return nil, errorsmod.Wrap(gaiaerrors.ErrLogic, "IBC keeper is required for AnteHandler")
 	}
 	// TODO: Enable with Globalfee
-	// if opts.GlobalFeeSubspace.Name() == "" {
-	// 	return nil, errorsmod.Wrap(gaiaerrors.ErrNotFound, "globalfee param store is required for AnteHandler")
-	// }
-	if opts.StakingSubspace.Name() == "" {
+	if opts.GlobalFeeSubspace.Name() == "" {
+		return nil, errorsmod.Wrap(gaiaerrors.ErrNotFound, "globalfee param store is required for AnteHandler")
+	}
+
+	if opts.StakingKeeper == nil {
 		return nil, errorsmod.Wrap(gaiaerrors.ErrNotFound, "staking param store is required for AnteHandler")
 	}
 	if opts.GovKeeper == nil {
@@ -62,8 +65,7 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) {
 		ante.NewValidateMemoDecorator(opts.AccountKeeper),
 		ante.NewConsumeGasForTxSizeDecorator(opts.AccountKeeper),
 		NewGovPreventSpamDecorator(opts.Codec, opts.GovKeeper),
-		// TODO: Enable with GlobalFee
-		// gaiafeeante.NewFeeDecorator(opts.GlobalFeeSubspace, opts.StakingSubspace),
+		gaiafeeante.NewFeeDecorator(opts.GlobalFeeSubspace, opts.StakingKeeper),
 		ante.NewDeductFeeDecorator(opts.AccountKeeper, opts.BankKeeper, opts.FeegrantKeeper, opts.TxFeeChecker),
 		ante.NewSetPubKeyDecorator(opts.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators
 		ante.NewValidateSigCountDecorator(opts.AccountKeeper),

diff --git a/ante/gov_ante_test.go b/ante/gov_ante_test.go
@@ -12,18 +12,20 @@ import (
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/testutil/testdata"
 
-	// "github.com/cosmos/gaia/v11/ante"
+	govv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
+
+	"github.com/cosmos/gaia/v11/ante"
 	gaiahelpers "github.com/cosmos/gaia/v11/app/helpers"
 
 	gaiaapp "github.com/cosmos/gaia/v11/app"
 )
 
-// var (
-// 	insufficientCoins = sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, 100))
-// 	minCoins          = sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, 1000000))
-// 	moreThanMinCoins  = sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, 2500000))
-// 	testAddr          = sdk.AccAddress("test1")
-// )
+var (
+	insufficientCoins = sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, 100))
+	minCoins          = sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, 1000000))
+	moreThanMinCoins  = sdk.NewCoins(sdk.NewInt64Coin(sdk.DefaultBondDenom, 2500000))
+	testAddr          = sdk.AccAddress("test1")
+)
 
 type GovAnteHandlerTestSuite struct {
 	suite.Suite
@@ -53,41 +55,40 @@ func TestGovSpamPreventionSuite(t *testing.T) {
 	suite.Run(t, new(GovAnteHandlerTestSuite))
 }
 
-// TODO: Enable with Global Fee
-// func (s *GovAnteHandlerTestSuite) TestGlobalFeeMinimumGasFeeAnteHandler() {
-//	// setup test
-//	s.SetupTest()
-//	tests := []struct {
-//		title, description string
-//		proposalType       string
-//		proposerAddr       sdk.AccAddress
-//		initialDeposit     sdk.Coins
-//		expectPass         bool
-//	}{
-//		{"Passing proposal 1", "the purpose of this proposal is to pass", govv1beta1.ProposalTypeText, testAddr, minCoins, true},
-//		{"Passing proposal 2", "the purpose of this proposal is to pass with more coins than minimum", govv1beta1.ProposalTypeText, testAddr, moreThanMinCoins, true},
-//		{"Failing proposal", "the purpose of this proposal is to fail", govv1beta1.ProposalTypeText, testAddr, insufficientCoins, false},
-//	}
-//
-//	decorator := ante.NewGovPreventSpamDecorator(s.app.AppCodec(), &s.app.GovKeeper)
-//
-//	for _, tc := range tests {
-//		content, _ := govv1beta1.ContentFromProposalType(tc.title, tc.description, tc.proposalType)
-//		s.Require().NotNil(content)
-//
-//		msg, err := govv1beta1.NewMsgSubmitProposal(
-//			content,
-//			tc.initialDeposit,
-//			tc.proposerAddr,
-//		)
-//
-//		s.Require().NoError(err)
-//
-//		err = decorator.ValidateGovMsgs(s.ctx, []sdk.Msg{msg})
-//		if tc.expectPass {
-//			s.Require().NoError(err, "expected %v to pass", tc.title)
-//		} else {
-//			s.Require().Error(err, "expected %v to fail", tc.title)
-//		}
-//	}
-//}
+func (s *GovAnteHandlerTestSuite) TestGlobalFeeMinimumGasFeeAnteHandler() {
+	// setup test
+	s.SetupTest()
+	tests := []struct {
+		title, description string
+		proposalType       string
+		proposerAddr       sdk.AccAddress
+		initialDeposit     sdk.Coins
+		expectPass         bool
+	}{
+		{"Passing proposal 1", "the purpose of this proposal is to pass", govv1beta1.ProposalTypeText, testAddr, minCoins, true},
+		{"Passing proposal 2", "the purpose of this proposal is to pass with more coins than minimum", govv1beta1.ProposalTypeText, testAddr, moreThanMinCoins, true},
+		{"Failing proposal", "the purpose of this proposal is to fail", govv1beta1.ProposalTypeText, testAddr, insufficientCoins, false},
+	}
+
+	decorator := ante.NewGovPreventSpamDecorator(s.app.AppCodec(), &s.app.GovKeeper)
+
+	for _, tc := range tests {
+		content, _ := govv1beta1.ContentFromProposalType(tc.title, tc.description, tc.proposalType)
+		s.Require().NotNil(content)
+
+		msg, err := govv1beta1.NewMsgSubmitProposal(
+			content,
+			tc.initialDeposit,
+			tc.proposerAddr,
+		)
+
+		s.Require().NoError(err)
+
+		err = decorator.ValidateGovMsgs(s.ctx, []sdk.Msg{msg})
+		if tc.expectPass {
+			s.Require().NoError(err, "expected %v to pass", tc.title)
+		} else {
+			s.Require().Error(err, "expected %v to fail", tc.title)
+		}
+	}
+}
diff --git a/app/app.go b/app/app.go
@@ -16,6 +16,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/server"
 	"github.com/cosmos/cosmos-sdk/testutil/testdata"
 
+	errorsmod "cosmossdk.io/errors"
 	dbm "github.com/cometbft/cometbft-db"
 	abci "github.com/cometbft/cometbft/abci/types"
 	tmjson "github.com/cometbft/cometbft/libs/json"
@@ -32,14 +33,14 @@ import (
 	"github.com/cosmos/cosmos-sdk/server/config"
 	servertypes "github.com/cosmos/cosmos-sdk/server/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/types/module"
 	"github.com/cosmos/cosmos-sdk/version"
 	"github.com/cosmos/cosmos-sdk/x/auth/ante"
 	authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
 	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"
 	ibctesting "github.com/cosmos/interchain-security/v3/legacy_ibc_testing/testing"
 	providertypes "github.com/cosmos/interchain-security/v3/x/ccv/provider/types"
@@ -52,7 +53,7 @@ import (
 	"github.com/cosmos/gaia/v11/app/upgrades"
 	v11 "github.com/cosmos/gaia/v11/app/upgrades/v11"
 
-	// "github.com/cosmos/gaia/v11/x/globalfee"
+	"github.com/cosmos/gaia/v11/x/globalfee"
 
 	// unnamed import of statik for swagger UI support
 	_ "github.com/cosmos/cosmos-sdk/client/docs/statik"
@@ -222,11 +223,14 @@ func NewGaiaApp(
 				SignModeHandler: encodingConfig.TxConfig.SignModeHandler(),
 				SigGasConsumer:  ante.DefaultSigVerificationGasConsumer,
 			},
-			Codec:     appCodec,
-			IBCkeeper: app.IBCKeeper,
-			GovKeeper: &app.GovKeeper,
-			// GlobalFeeSubspace: app.GetSubspace(globalfee.ModuleName),
-			StakingSubspace: app.GetSubspace(stakingtypes.ModuleName),
+			Codec:             appCodec,
+			IBCkeeper:         app.IBCKeeper,
+			GovKeeper:         &app.GovKeeper,
+			GlobalFeeSubspace: app.GetSubspace(globalfee.ModuleName),
+			StakingKeeper:     app.StakingKeeper,
+			// If TxFeeChecker is nil the default ante TxFeeChecker is used
+			// so we use this no-op to keep the global fee module behaviour unchanged
+			TxFeeChecker: noOpTxFeeChecker,
 		},
 	)
 	if err != nil {
@@ -364,6 +368,7 @@ func (app *GaiaApp) RegisterTendermintService(clientCtx client.Context) {
 	)
 }
 
+// RegisterTxService allows query minimum-gas-prices in app.toml
 func (app *GaiaApp) RegisterNodeService(clientCtx client.Context) {
 	nodeservice.RegisterNodeService(clientCtx, app.GRPCQueryRouter())
 }
@@ -435,3 +440,14 @@ type EmptyAppOptions struct{}
 func (ao EmptyAppOptions) Get(_ string) interface{} {
 	return nil
 }
+
+// noOpTxFeeChecker is an ante TxFeeChecker for the DeductFeeDecorator, see x/auth/ante/fee.go,
+// it performs a no-op by not checking tx fees and always returns a zero tx priority
+func noOpTxFeeChecker(_ sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) {
+	feeTx, ok := tx.(sdk.FeeTx)
+	if !ok {
+		return nil, 0, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx")
+	}
+
+	return feeTx.GetFee(), 0, nil
+}
diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go
@@ -31,6 +31,8 @@ import (
 	govv1beta1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1"
 	ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported"
 
+	"github.com/cosmos/gaia/v11/x/globalfee"
+
 	providertypes "github.com/cosmos/interchain-security/v3/x/ccv/provider/types"
 
 	govkeeper "github.com/cosmos/cosmos-sdk/x/gov/keeper"
@@ -435,7 +437,10 @@ func NewAppKeeper(
 
 // GetSubspace returns a param subspace for a given module name.
 func (appKeepers *AppKeepers) GetSubspace(moduleName string) paramstypes.Subspace {
-	subspace, _ := appKeepers.ParamsKeeper.GetSubspace(moduleName)
+	subspace, ok := appKeepers.ParamsKeeper.GetSubspace(moduleName)
+	if !ok {
+		panic("couldn't load subspace for module: " + moduleName)
+	}
 	return subspace
 }
 
@@ -457,8 +462,7 @@ func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino
 	paramsKeeper.Subspace(icahosttypes.SubModuleName)
 
 	paramsKeeper.Subspace(pfmroutertypes.ModuleName).WithKeyTable(pfmroutertypes.ParamKeyTable())
-	// TODO: Enable with Globalfee
-	// paramsKeeper.Subspace(globalfee.ModuleName)
+	paramsKeeper.Subspace(globalfee.ModuleName)
 	paramsKeeper.Subspace(providertypes.ModuleName)
 
 	return paramsKeeper

diff --git a/app/modules.go b/app/modules.go
@@ -56,7 +56,7 @@ import (
 	routertypes "github.com/strangelove-ventures/packet-forward-middleware/v7/router/types"
 
 	gaiaparams "github.com/cosmos/gaia/v11/app/params"
-	// "github.com/cosmos/gaia/v11/x/globalfee"
+	"github.com/cosmos/gaia/v11/x/globalfee"
 )
 
 var maccPerms = map[string][]string{
@@ -108,8 +108,7 @@ var ModuleBasics = module.NewBasicManager(
 	vesting.AppModuleBasic{},
 	router.AppModuleBasic{},
 	ica.AppModuleBasic{},
-	// TODO: Enable with Global Fee
-	// globalfee.AppModule{},
+	globalfee.AppModule{},
 	ibcprovider.AppModuleBasic{},
 	consensus.AppModuleBasic{},
 )
@@ -144,7 +143,7 @@ func appModules(
 		authzmodule.NewAppModule(appCodec, app.AuthzKeeper, app.AccountKeeper, app.BankKeeper, app.interfaceRegistry),
 		ibc.NewAppModule(app.IBCKeeper),
 		sdkparams.NewAppModule(app.ParamsKeeper),
-		// globalfee.NewAppModule(app.GetSubspace(globalfee.ModuleName)),
+		globalfee.NewAppModule(app.GetSubspace(globalfee.ModuleName)),
 		app.TransferModule,
 		app.ICAModule,
 		app.PFMRouterModule,
@@ -216,7 +215,7 @@ func orderBeginBlockers() []string {
 		feegrant.ModuleName,
 		paramstypes.ModuleName,
 		vestingtypes.ModuleName,
-		// globalfee.ModuleName,
+		globalfee.ModuleName,
 		providertypes.ModuleName,
 		consensusparamtypes.ModuleName,
 	}
@@ -252,7 +251,7 @@ func orderEndBlockers() []string {
 		paramstypes.ModuleName,
 		upgradetypes.ModuleName,
 		vestingtypes.ModuleName,
-		// globalfee.ModuleName,
+		globalfee.ModuleName,
 		providertypes.ModuleName,
 		consensusparamtypes.ModuleName,
 	}
@@ -296,7 +295,7 @@ func orderInitBlockers() []string {
 		// To resolve this issue, we should initialize the globalfee module after genutil, ensuring that the global
 		// min fee is empty when gentx is called.
 		// For more details, please refer to the following link: https://github.com/cosmos/gaia/issues/2489
-		// globalfee.ModuleName,
+		globalfee.ModuleName,
 		providertypes.ModuleName,
 		consensusparamtypes.ModuleName,
 	}

diff --git a/go.mod b/go.mod
@@ -35,11 +35,11 @@ require (
 	cloud.google.com/go/compute/metadata v0.2.3 // indirect
 	cloud.google.com/go/iam v0.13.0 // indirect
 	cloud.google.com/go/storage v1.29.0 // indirect
-	github.com/golang/protobuf v1.5.3 // indirect
+	github.com/golang/protobuf v1.5.3
 	// github.com/gravity-devs/liquidity v1.6.0
-	github.com/grpc-ecosystem/grpc-gateway v1.16.0 // indirect
-	google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1 // indirect
-	google.golang.org/grpc v1.55.0 // indirect
+	github.com/grpc-ecosystem/grpc-gateway v1.16.0
+	google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1
+	google.golang.org/grpc v1.55.0
 )
 
 require (