From 39d9a6921c7b3074290182434a966f80319517ea Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Mon, 5 Aug 2024 16:15:42 +0530 Subject: [PATCH 01/28] feat(server/v2): add min gas price and check with tx fee --- server/v2/cometbft/abci.go | 42 +++++++++++++++++++++++ server/v2/cometbft/config.go | 29 ++++++++++++++++ server/v2/cometbft/flags.go | 13 +++---- server/v2/cometbft/go.mod | 2 +- server/v2/cometbft/server.go | 6 ++++ server/v2/cometbft/types/errors/errors.go | 9 +++++ simapp/v2/simdv2/cmd/commands.go | 23 ++++++++++++- simapp/v2/simdv2/cmd/testnet.go | 4 +++ tools/confix/data/v2-app.toml | 2 ++ tools/confix/migrations.go | 11 +++--- 10 files changed, 128 insertions(+), 13 deletions(-) diff --git a/server/v2/cometbft/abci.go b/server/v2/cometbft/abci.go index 7519b3d8c732..d60e97b041fd 100644 --- a/server/v2/cometbft/abci.go +++ b/server/v2/cometbft/abci.go @@ -18,6 +18,7 @@ import ( "cosmossdk.io/core/transaction" errorsmod "cosmossdk.io/errors" "cosmossdk.io/log" + sdkmath "cosmossdk.io/math" "cosmossdk.io/server/v2/appmanager" "cosmossdk.io/server/v2/cometbft/client/grpc/cmtservice" "cosmossdk.io/server/v2/cometbft/handlers" @@ -27,6 +28,7 @@ import ( "cosmossdk.io/server/v2/streaming" "cosmossdk.io/store/v2/snapshots" consensustypes "cosmossdk.io/x/consensus/types" + sdk "github.com/cosmos/cosmos-sdk/types" ) var _ abci.Application = (*Consensus[transaction.Tx])(nil) @@ -123,6 +125,14 @@ func (c *Consensus[T]) CheckTx(ctx context.Context, req *abciproto.CheckTxReques return nil, err } + // check tx fee with validator's minimum-gas-price config + if err := c.checkTxFeeWithMinGasPrices(decodedTx); err != nil { + return &abciproto.CheckTxResponse{ + Code: 1, + Log: err.Error(), + }, nil + } + resp, err := c.app.ValidateTx(ctx, decodedTx) if err != nil { return nil, err @@ -145,6 +155,38 @@ func (c *Consensus[T]) CheckTx(ctx context.Context, req *abciproto.CheckTxReques return cometResp, nil } +// checkTxFeeWithMinGasPrices ensure that the provided fees meet a minimum threshold for the validator, +// if this is a CheckTx. This is only for local mempool purposes, and thus +// is only ran on check tx. +func (c *Consensus[T]) checkTxFeeWithMinGasPrices(tx transaction.Tx) error { + feeTx, ok := tx.(sdk.FeeTx) + if !ok { + return errorsmod.Wrap(cometerrors.ErrTxDecode, "tx must be a feeTx") + } + + feeCoins := feeTx.GetFee() + gas := feeTx.GetGas() + + minGasPrices := c.cfg.GetMinGasPrices() + if !minGasPrices.IsZero() { + 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 := sdkmath.LegacyNewDec(int64(gas)) + for i, gp := range minGasPrices { + fee := gp.Amount.Mul(glDec) + requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) + } + + if !feeCoins.IsAnyGTE(requiredFees) { + return errorsmod.Wrapf(cometerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) + } + } + + return nil +} + // Info implements types.Application. func (c *Consensus[T]) Info(ctx context.Context, _ *abciproto.InfoRequest) (*abciproto.InfoResponse, error) { version, _, err := c.store.StateLatest() diff --git a/server/v2/cometbft/config.go b/server/v2/cometbft/config.go index 3f03e383c601..d4f55c6a2a3a 100644 --- a/server/v2/cometbft/config.go +++ b/server/v2/cometbft/config.go @@ -1,10 +1,14 @@ package cometbft import ( + "fmt" + cmtcfg "github.com/cometbft/cometbft/config" "github.com/spf13/viper" serverv2 "cosmossdk.io/server/v2" + cometerrors "cosmossdk.io/server/v2/cometbft/types/errors" + sdk "github.com/cosmos/cosmos-sdk/types" ) // Config is the configuration for the CometBFT application @@ -27,6 +31,7 @@ func DefaultAppTomlConfig() *AppTomlConfig { } type AppTomlConfig struct { + MinGasPrices string `mapstructure:"minimum-gas-prices" toml:"minimum-gas-prices" comment:"minimum-gas-prices defines the price which a validator is willing to accept for processing a transaction. A transaction's fees must meet the minimum of any denomination specified in this config (e.g. 0.25token1;0.0001token2)."` MinRetainBlocks uint64 `mapstructure:"min-retain-blocks" toml:"min-retain-blocks" comment:"min-retain-blocks defines the minimum block height offset from the current block being committed, such that all blocks past this offset are pruned from CometBFT. A value of 0 indicates that no blocks should be pruned."` IndexEvents []string `mapstructure:"index-events" toml:"index-events" comment:"index-events defines the set of events in the form {eventType}.{attributeKey}, which informs CometBFT what to index. If empty, all events will be indexed."` HaltHeight uint64 `mapstructure:"halt-height" toml:"halt-height" comment:"halt-height contains a non-zero block height at which a node will gracefully halt and shutdown that can be used to assist upgrades and testing."` @@ -64,3 +69,27 @@ func getConfigTomlFromViper(v *viper.Viper) *cmtcfg.Config { return conf.SetRoot(rootDir) } + +// GetMinGasPrices returns the validator's minimum gas prices based on the set configuration. +func (c Config) GetMinGasPrices() sdk.DecCoins { + minGasPricesStr := c.AppTomlConfig.MinGasPrices + if minGasPricesStr == "" { + return sdk.DecCoins{} + } + + gasPrices, err := sdk.ParseDecCoins(minGasPricesStr) + if err != nil { + panic(fmt.Sprintf("invalid minimum gas prices: %v", err)) + } + + return gasPrices +} + +// ValidateBasic returns an error if min-gas-prices field is empty in Config. Otherwise, it returns nil. +func (c Config) ValidateBasic() error { + if c.AppTomlConfig.MinGasPrices == "" { + return cometerrors.ErrAppConfig.Wrap("set minimum-gas-prices in app.toml or flag or env variable") + } + + return nil +} diff --git a/server/v2/cometbft/flags.go b/server/v2/cometbft/flags.go index 55fa0a14e771..3380d6f39c4e 100644 --- a/server/v2/cometbft/flags.go +++ b/server/v2/cometbft/flags.go @@ -51,10 +51,11 @@ func prefix(f string) string { // Server flags var ( - Standalone = prefix("standalone") - FlagAddress = prefix("address") - FlagTransport = prefix("transport") - FlagHaltHeight = prefix("halt-height") - FlagHaltTime = prefix("halt-time") - FlagTrace = prefix("trace") + Standalone = prefix("standalone") + FlagAddress = prefix("address") + FlagTransport = prefix("transport") + FlagHaltHeight = prefix("halt-height") + FlagHaltTime = prefix("halt-time") + FlagTrace = prefix("trace") + FlagMinGasPrices = prefix("minimum-gas-prices") ) diff --git a/server/v2/cometbft/go.mod b/server/v2/cometbft/go.mod index ef9c06a6c679..26136831ea0b 100644 --- a/server/v2/cometbft/go.mod +++ b/server/v2/cometbft/go.mod @@ -25,6 +25,7 @@ require ( cosmossdk.io/core v0.12.1-0.20231114100755-569e3ff6a0d7 cosmossdk.io/errors v1.0.1 cosmossdk.io/log v1.3.1 + cosmossdk.io/math v1.3.0 cosmossdk.io/server/v2 v2.0.0-00010101000000-000000000000 cosmossdk.io/server/v2/appmanager v0.0.0-00010101000000-000000000000 cosmossdk.io/store/v2 v2.0.0-00010101000000-000000000000 @@ -50,7 +51,6 @@ require ( cosmossdk.io/collections v0.4.0 // indirect cosmossdk.io/depinject v1.0.0 // indirect cosmossdk.io/errors/v2 v2.0.0-20240731132947-df72853b3ca5 // indirect - cosmossdk.io/math v1.3.0 // indirect cosmossdk.io/store v1.1.1-0.20240418092142-896cdf1971bc // indirect cosmossdk.io/x/auth v0.0.0-00010101000000-000000000000 // indirect cosmossdk.io/x/bank v0.0.0-20240226161501-23359a0b6d91 // indirect diff --git a/server/v2/cometbft/server.go b/server/v2/cometbft/server.go index c06c56c9be9e..c1ec2c6356da 100644 --- a/server/v2/cometbft/server.go +++ b/server/v2/cometbft/server.go @@ -130,6 +130,11 @@ func (s *CometBFTServer[T]) Name() string { } func (s *CometBFTServer[T]) Start(ctx context.Context) error { + // validate cometbft config + if err := s.config.ValidateBasic(); err != nil { + return err + } + wrappedLogger := cometlog.CometLoggerWrapper{Logger: s.logger} if s.config.AppTomlConfig.Standalone { svr, err := abciserver.NewServer(s.config.AppTomlConfig.Address, s.config.AppTomlConfig.Transport, s.Consensus) @@ -218,6 +223,7 @@ func (s *CometBFTServer[T]) StartCmdFlags() *pflag.FlagSet { flags.String(FlagTransport, "socket", "Transport protocol: socket, grpc") flags.Uint64(FlagHaltHeight, 0, "Block height at which to gracefully halt the chain and shutdown the node") flags.Uint64(FlagHaltTime, 0, "Minimum block time (in Unix seconds) at which to gracefully halt the chain and shutdown the node") + flags.String(FlagMinGasPrices, "", "Minimum gas prices to accept for transactions; Any fee in a tx must meet this minimum (e.g. 0.01photino;0.0001stake)") flags.Bool(FlagTrace, false, "Provide full stack traces for errors in ABCI Log") flags.Bool(Standalone, false, "Run app without CometBFT") diff --git a/server/v2/cometbft/types/errors/errors.go b/server/v2/cometbft/types/errors/errors.go index 380c176ff306..93406a332cf4 100644 --- a/server/v2/cometbft/types/errors/errors.go +++ b/server/v2/cometbft/types/errors/errors.go @@ -14,4 +14,13 @@ var ( // ErrInvalidRequest defines an ABCI typed error where the request contains // invalid data. ErrInvalidRequest = errorsmod.Register(RootCodespace, 2, "invalid request") + + // ErrTxDecode is returned if we cannot parse a transaction + ErrTxDecode = errorsmod.Register(RootCodespace, 3, "tx parse error") + + // ErrInsufficientFee is returned if provided fee in tx is less than required fee + ErrInsufficientFee = errorsmod.Register(RootCodespace, 4, "insufficient fee") + + // ErrAppConfig defines an error occurred if application configuration is misconfigured + ErrAppConfig = errorsmod.Register(RootCodespace, 5, "error in app.toml") ) diff --git a/simapp/v2/simdv2/cmd/commands.go b/simapp/v2/simdv2/cmd/commands.go index 8d1e8bd40a63..1915ba96d7e8 100644 --- a/simapp/v2/simdv2/cmd/commands.go +++ b/simapp/v2/simdv2/cmd/commands.go @@ -70,12 +70,33 @@ func initRootCmd[T transaction.Tx]( offchain.OffChain(), ) + // Optionally allow the chain developer to overwrite the cometbft default + // app toml config. + cometAppTomlCfg := cometbft.DefaultAppTomlConfig() + // The cometbft server's default minimum gas price is set to "" (empty value) inside + // app.toml. If left empty by validators, the node will halt on startup. + // However, the chain developer can set a default app.toml value for their + // validators here. + // + // In summary: + // - if you leave cometAppTomlCfg.MinGasPrices = "", all validators MUST tweak their + // own app.toml config, + // - if you set cometAppTomlCfg.MinGasPrices non-empty, validators CAN tweak their + // own app.toml to override, or use this default value. + // + // In simapp, we set the min gas prices to 0. + cometAppTomlCfg.MinGasPrices = "0stake" + // wire server commands if err = serverv2.AddCommands( rootCmd, newApp, logger, - cometbft.New(&genericTxDecoder[T]{txConfig}, cometbft.DefaultServerOptions[T]()), + cometbft.New( + &genericTxDecoder[T]{txConfig}, + cometbft.DefaultServerOptions[T](), + cometbft.OverwriteDefaultAppTomlConfig(cometAppTomlCfg), + ), grpc.New[T](), store.New[T](), ); err != nil { diff --git a/simapp/v2/simdv2/cmd/testnet.go b/simapp/v2/simdv2/cmd/testnet.go index 2d4c3b348db5..2a35b3c4b31b 100644 --- a/simapp/v2/simdv2/cmd/testnet.go +++ b/simapp/v2/simdv2/cmd/testnet.go @@ -337,11 +337,15 @@ func initTestnetFiles[T transaction.Tx]( return err } + cometAppTomlCfg := cometbft.DefaultAppTomlConfig() + cometAppTomlCfg.MinGasPrices = args.minGasPrices + // Write server config cometServer := cometbft.New[T]( &genericTxDecoder[T]{clientCtx.TxConfig}, cometbft.ServerOptions[T]{}, cometbft.OverwriteDefaultConfigTomlConfig(nodeConfig), + cometbft.OverwriteDefaultAppTomlConfig(cometAppTomlCfg), ) grpcServer := grpc.New[T](grpc.OverwriteDefaultConfig(grpcConfig)) storeServer := store.New[T]() diff --git a/tools/confix/data/v2-app.toml b/tools/confix/data/v2-app.toml index 6a93006eccfb..1f7700aef33b 100644 --- a/tools/confix/data/v2-app.toml +++ b/tools/confix/data/v2-app.toml @@ -1,4 +1,6 @@ [comet] +# minimum-gas-prices defines the price which a validator is willing to accept for processing a transaction. A transaction's fees must meet the minimum of any denomination specified in this config (e.g. 0.25token1;0.0001token2). +minimum-gas-prices = '0stake' # min-retain-blocks defines the minimum block height offset from the current block being committed, such that all blocks past this offset are pruned from CometBFT. A value of 0 indicates that no blocks should be pruned. min-retain-blocks = 0 # index-events defines the set of events in the form {eventType}.{attributeKey}, which informs CometBFT what to index. If empty, all events will be indexed. diff --git a/tools/confix/migrations.go b/tools/confix/migrations.go index 737ed6b822cb..b48e9096a284 100644 --- a/tools/confix/migrations.go +++ b/tools/confix/migrations.go @@ -35,11 +35,12 @@ type v2KeyChangesMap map[string][]string // list all the keys which are need to be modified in v2 var v2KeyChanges = v2KeyChangesMap{ - "min-retain-blocks": []string{"comet.min-retain-blocks"}, - "index-events": []string{"comet.index-events"}, - "halt-height": []string{"comet.halt-height"}, - "halt-time": []string{"comet.halt-time"}, - "app-db-backend": []string{"store.app-db-backend"}, + "minimum-gas-prices": []string{"comet.minimum-gas-prices"}, + "min-retain-blocks": []string{"comet.min-retain-blocks"}, + "index-events": []string{"comet.index-events"}, + "halt-height": []string{"comet.halt-height"}, + "halt-time": []string{"comet.halt-time"}, + "app-db-backend": []string{"store.app-db-backend"}, "pruning-keep-recent": []string{ "store.options.ss-pruning-option.keep-recent", "store.options.sc-pruning-option.keep-recent", From a00a86618d17740ab9b8fc857daa4380a1e47529 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Tue, 6 Aug 2024 10:12:43 +0530 Subject: [PATCH 02/28] address comment --- server/v2/cometbft/abci.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/v2/cometbft/abci.go b/server/v2/cometbft/abci.go index d60e97b041fd..e4a40584848d 100644 --- a/server/v2/cometbft/abci.go +++ b/server/v2/cometbft/abci.go @@ -161,7 +161,7 @@ func (c *Consensus[T]) CheckTx(ctx context.Context, req *abciproto.CheckTxReques func (c *Consensus[T]) checkTxFeeWithMinGasPrices(tx transaction.Tx) error { feeTx, ok := tx.(sdk.FeeTx) if !ok { - return errorsmod.Wrap(cometerrors.ErrTxDecode, "tx must be a feeTx") + return nil // don't force users to implement fee tx } feeCoins := feeTx.GetFee() From 36e4aed015bdcd8090af51fd5c1c96d8dc1b17b4 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Tue, 6 Aug 2024 11:00:09 +0530 Subject: [PATCH 03/28] address comment --- server/v2/cometbft/abci.go | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/server/v2/cometbft/abci.go b/server/v2/cometbft/abci.go index e4a40584848d..b88af60f61d9 100644 --- a/server/v2/cometbft/abci.go +++ b/server/v2/cometbft/abci.go @@ -168,20 +168,22 @@ func (c *Consensus[T]) checkTxFeeWithMinGasPrices(tx transaction.Tx) error { gas := feeTx.GetGas() minGasPrices := c.cfg.GetMinGasPrices() - if !minGasPrices.IsZero() { - 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 := sdkmath.LegacyNewDec(int64(gas)) - for i, gp := range minGasPrices { - fee := gp.Amount.Mul(glDec) - requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) - } + if minGasPrices.IsZero() { + return nil + } - if !feeCoins.IsAnyGTE(requiredFees) { - return errorsmod.Wrapf(cometerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) - } + 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 := sdkmath.LegacyNewDec(int64(gas)) + for i, gp := range minGasPrices { + fee := gp.Amount.Mul(glDec) + requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) + } + + if !feeCoins.IsAnyGTE(requiredFees) { + return errorsmod.Wrapf(cometerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) } return nil From bc8ad354e57edfbe7c9a1ca1a0c929bdc9b1b800 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Wed, 7 Aug 2024 15:51:49 +0530 Subject: [PATCH 04/28] WIP: add min-gas-prices in main server component --- server/v2/config.go | 24 +++++++++++++++++++ server/v2/flags.go | 11 +++++++++ server/v2/server.go | 41 ++++++++++++++++++++++++++++++++- simapp/v2/simdv2/cmd/testnet.go | 6 ++--- 4 files changed, 78 insertions(+), 4 deletions(-) diff --git a/server/v2/config.go b/server/v2/config.go index 7ce73fe88a9a..69aa25c3bdad 100644 --- a/server/v2/config.go +++ b/server/v2/config.go @@ -7,6 +7,30 @@ import ( "github.com/spf13/viper" ) +// ServerConfig defines configuration for the main server. +type ServerConfig struct { + MinGasPrices string `mapstructure:"minimum-gas-prices" toml:"minimum-gas-prices" comment:"minimum-gas-prices defines the price which a validator is willing to accept for processing a transaction. A transaction's fees must meet the minimum of any denomination specified in this config (e.g. 0.25token1;0.0001token2)."` +} + +// ValidateBasic returns an error if min-gas-prices field is empty in Config. Otherwise, it returns nil. +func (c ServerConfig) ValidateBasic() error { + if c.MinGasPrices == "" { + return fmt.Errorf("error in app.toml: set minimum-gas-prices in app.toml or flag or env variable") + } + + return nil +} + +// DefaultMainServerConfig returns the default config of main server component +func DefaultMainServerConfig() ServerConfig { + return ServerConfig{} +} + +// OverwriteDefaultConfig overwrites main server config with given config +func (s *Server[T]) OverwriteDefaultConfig(cfg ServerConfig) { + s.config = cfg +} + // ReadConfig returns a viper instance of the config file func ReadConfig(configPath string) (*viper.Viper, error) { v := viper.New() diff --git a/server/v2/flags.go b/server/v2/flags.go index a86154eb769c..6c253d419ab0 100644 --- a/server/v2/flags.go +++ b/server/v2/flags.go @@ -1,6 +1,17 @@ // Package serverv2 defines constants for server configuration flags and output formats. package serverv2 +import "fmt" + +// start flags are prefixed with the server name +// as the config in prefixed with the server name +// this allows viper to properly bind the flags +func prefix(f string) string { + return fmt.Sprintf("%s.%s", serverName, f) +} + +var FlagMinGasPrices = prefix("minimum-gas-prices") + const ( // FlagHome specifies the home directory flag. FlagHome = "home" diff --git a/server/v2/server.go b/server/v2/server.go index 6e1a4e7114db..2221358f5bae 100644 --- a/server/v2/server.go +++ b/server/v2/server.go @@ -56,11 +56,16 @@ type CLIConfig struct { Txs []*cobra.Command } +const ( + serverName = "server" +) + var _ ServerComponent[transaction.Tx] = (*Server[transaction.Tx])(nil) type Server[T transaction.Tx] struct { logger log.Logger components []ServerComponent[T] + config ServerConfig } func NewServer[T transaction.Tx]( @@ -74,11 +79,16 @@ func NewServer[T transaction.Tx]( } func (s *Server[T]) Name() string { - return "server" + return serverName } // Start starts all components concurrently. func (s *Server[T]) Start(ctx context.Context) error { + // validate main server config + if err := s.config.ValidateBasic(); err != nil { + return err + } + s.logger.Info("starting servers...") g, ctx := errgroup.WithContext(ctx) @@ -151,9 +161,19 @@ func (s *Server[T]) CLICommands() CLIConfig { return commands } +// Config returns config of the main server component +func (s *Server[T]) Config() ServerConfig { + return s.config +} + // Configs returns all configs of all server components. func (s *Server[T]) Configs() map[string]any { cfgs := make(map[string]any) + + // add main server component config + cfgs[s.Name()] = s.config + + // add other components' config for _, mod := range s.components { if configmod, ok := mod.(HasConfig); ok { cfg := configmod.Config() @@ -164,9 +184,22 @@ func (s *Server[T]) Configs() map[string]any { return cfgs } +func (s *Server[T]) StartCmdFlags() *pflag.FlagSet { + flags := pflag.NewFlagSet(s.Name(), pflag.ExitOnError) + flags.String(FlagMinGasPrices, "", "Minimum gas prices to accept for transactions; Any fee in a tx must meet this minimum (e.g. 0.01photino;0.0001stake)") + return flags +} + // Init initializes all server components with the provided application, configuration, and logger. // It returns an error if any component fails to initialize. func (s *Server[T]) Init(appI AppI[T], v *viper.Viper, logger log.Logger) error { + cfg := s.config + if v != nil { + if err := UnmarshalSubConfig(v, s.Name(), &cfg); err != nil { + return fmt.Errorf("failed to unmarshal config: %w", err) + } + } + var components []ServerComponent[T] for _, mod := range s.components { mod := mod @@ -177,6 +210,7 @@ func (s *Server[T]) Init(appI AppI[T], v *viper.Viper, logger log.Logger) error components = append(components, mod) } + s.config = cfg s.components = components return nil } @@ -217,6 +251,11 @@ func (s *Server[T]) WriteConfig(configPath string) error { // StartFlags returns all flags of all server components. func (s *Server[T]) StartFlags() []*pflag.FlagSet { flags := []*pflag.FlagSet{} + + // add main server component flags + flags = append(flags, s.StartCmdFlags()) + + // add other components' start cmd flags for _, mod := range s.components { if startmod, ok := mod.(HasStartFlags); ok { flags = append(flags, startmod.StartCmdFlags()) diff --git a/simapp/v2/simdv2/cmd/testnet.go b/simapp/v2/simdv2/cmd/testnet.go index 2a35b3c4b31b..823e4e1df7df 100644 --- a/simapp/v2/simdv2/cmd/testnet.go +++ b/simapp/v2/simdv2/cmd/testnet.go @@ -337,19 +337,19 @@ func initTestnetFiles[T transaction.Tx]( return err } - cometAppTomlCfg := cometbft.DefaultAppTomlConfig() - cometAppTomlCfg.MinGasPrices = args.minGasPrices + serverCfg := serverv2.DefaultMainServerConfig() + serverCfg.MinGasPrices = args.minGasPrices // Write server config cometServer := cometbft.New[T]( &genericTxDecoder[T]{clientCtx.TxConfig}, cometbft.ServerOptions[T]{}, cometbft.OverwriteDefaultConfigTomlConfig(nodeConfig), - cometbft.OverwriteDefaultAppTomlConfig(cometAppTomlCfg), ) grpcServer := grpc.New[T](grpc.OverwriteDefaultConfig(grpcConfig)) storeServer := store.New[T]() server := serverv2.NewServer(log.NewNopLogger(), cometServer, grpcServer, storeServer) + server.OverwriteDefaultConfig(serverCfg) err = server.WriteConfig(filepath.Join(nodeDir, "config")) if err != nil { return err From d943ad8a78889769e453fa7a346ca51d81dcc5c6 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Thu, 8 Aug 2024 12:19:58 +0530 Subject: [PATCH 05/28] clean and integrate min gas prices in auth --- server/v2/cometbft/abci.go | 44 ---------------- server/v2/cometbft/config.go | 44 +++------------- server/v2/cometbft/flags.go | 13 +++-- server/v2/cometbft/go.mod | 2 +- server/v2/cometbft/server.go | 6 --- server/v2/cometbft/types/errors/errors.go | 9 ---- server/v2/commands.go | 3 +- server/v2/config.go | 4 +- server/v2/server.go | 2 + server/v2/server_test.go | 1 + server/v2/testdata/app.toml | 4 ++ simapp/v2/simdv2/cmd/commands.go | 15 +++--- simapp/v2/simdv2/cmd/testnet.go | 3 +- tools/confix/data/v2-app.toml | 6 ++- tools/confix/migrations.go | 2 +- x/auth/ante/validator_tx_fee.go | 62 ++++++++++++++++++----- x/auth/depinject.go | 17 +++++++ x/auth/module.go | 22 ++++++-- 18 files changed, 119 insertions(+), 140 deletions(-) diff --git a/server/v2/cometbft/abci.go b/server/v2/cometbft/abci.go index b88af60f61d9..7519b3d8c732 100644 --- a/server/v2/cometbft/abci.go +++ b/server/v2/cometbft/abci.go @@ -18,7 +18,6 @@ import ( "cosmossdk.io/core/transaction" errorsmod "cosmossdk.io/errors" "cosmossdk.io/log" - sdkmath "cosmossdk.io/math" "cosmossdk.io/server/v2/appmanager" "cosmossdk.io/server/v2/cometbft/client/grpc/cmtservice" "cosmossdk.io/server/v2/cometbft/handlers" @@ -28,7 +27,6 @@ import ( "cosmossdk.io/server/v2/streaming" "cosmossdk.io/store/v2/snapshots" consensustypes "cosmossdk.io/x/consensus/types" - sdk "github.com/cosmos/cosmos-sdk/types" ) var _ abci.Application = (*Consensus[transaction.Tx])(nil) @@ -125,14 +123,6 @@ func (c *Consensus[T]) CheckTx(ctx context.Context, req *abciproto.CheckTxReques return nil, err } - // check tx fee with validator's minimum-gas-price config - if err := c.checkTxFeeWithMinGasPrices(decodedTx); err != nil { - return &abciproto.CheckTxResponse{ - Code: 1, - Log: err.Error(), - }, nil - } - resp, err := c.app.ValidateTx(ctx, decodedTx) if err != nil { return nil, err @@ -155,40 +145,6 @@ func (c *Consensus[T]) CheckTx(ctx context.Context, req *abciproto.CheckTxReques return cometResp, nil } -// checkTxFeeWithMinGasPrices ensure that the provided fees meet a minimum threshold for the validator, -// if this is a CheckTx. This is only for local mempool purposes, and thus -// is only ran on check tx. -func (c *Consensus[T]) checkTxFeeWithMinGasPrices(tx transaction.Tx) error { - feeTx, ok := tx.(sdk.FeeTx) - if !ok { - return nil // don't force users to implement fee tx - } - - feeCoins := feeTx.GetFee() - gas := feeTx.GetGas() - - minGasPrices := c.cfg.GetMinGasPrices() - if minGasPrices.IsZero() { - return nil - } - - 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 := sdkmath.LegacyNewDec(int64(gas)) - for i, gp := range minGasPrices { - fee := gp.Amount.Mul(glDec) - requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) - } - - if !feeCoins.IsAnyGTE(requiredFees) { - return errorsmod.Wrapf(cometerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) - } - - return nil -} - // Info implements types.Application. func (c *Consensus[T]) Info(ctx context.Context, _ *abciproto.InfoRequest) (*abciproto.InfoResponse, error) { version, _, err := c.store.StateLatest() diff --git a/server/v2/cometbft/config.go b/server/v2/cometbft/config.go index d4f55c6a2a3a..8f2d3a929e5f 100644 --- a/server/v2/cometbft/config.go +++ b/server/v2/cometbft/config.go @@ -1,14 +1,10 @@ package cometbft import ( - "fmt" - cmtcfg "github.com/cometbft/cometbft/config" "github.com/spf13/viper" serverv2 "cosmossdk.io/server/v2" - cometerrors "cosmossdk.io/server/v2/cometbft/types/errors" - sdk "github.com/cosmos/cosmos-sdk/types" ) // Config is the configuration for the CometBFT application @@ -19,19 +15,17 @@ type Config struct { func DefaultAppTomlConfig() *AppTomlConfig { return &AppTomlConfig{ - MinRetainBlocks: 0, - IndexEvents: make([]string, 0), - HaltHeight: 0, - HaltTime: 0, - Address: "tcp://127.0.0.1:26658", - Transport: "socket", - Trace: false, - Standalone: false, + IndexEvents: make([]string, 0), + HaltHeight: 0, + HaltTime: 0, + Address: "tcp://127.0.0.1:26658", + Transport: "socket", + Trace: false, + Standalone: false, } } type AppTomlConfig struct { - MinGasPrices string `mapstructure:"minimum-gas-prices" toml:"minimum-gas-prices" comment:"minimum-gas-prices defines the price which a validator is willing to accept for processing a transaction. A transaction's fees must meet the minimum of any denomination specified in this config (e.g. 0.25token1;0.0001token2)."` MinRetainBlocks uint64 `mapstructure:"min-retain-blocks" toml:"min-retain-blocks" comment:"min-retain-blocks defines the minimum block height offset from the current block being committed, such that all blocks past this offset are pruned from CometBFT. A value of 0 indicates that no blocks should be pruned."` IndexEvents []string `mapstructure:"index-events" toml:"index-events" comment:"index-events defines the set of events in the form {eventType}.{attributeKey}, which informs CometBFT what to index. If empty, all events will be indexed."` HaltHeight uint64 `mapstructure:"halt-height" toml:"halt-height" comment:"halt-height contains a non-zero block height at which a node will gracefully halt and shutdown that can be used to assist upgrades and testing."` @@ -69,27 +63,3 @@ func getConfigTomlFromViper(v *viper.Viper) *cmtcfg.Config { return conf.SetRoot(rootDir) } - -// GetMinGasPrices returns the validator's minimum gas prices based on the set configuration. -func (c Config) GetMinGasPrices() sdk.DecCoins { - minGasPricesStr := c.AppTomlConfig.MinGasPrices - if minGasPricesStr == "" { - return sdk.DecCoins{} - } - - gasPrices, err := sdk.ParseDecCoins(minGasPricesStr) - if err != nil { - panic(fmt.Sprintf("invalid minimum gas prices: %v", err)) - } - - return gasPrices -} - -// ValidateBasic returns an error if min-gas-prices field is empty in Config. Otherwise, it returns nil. -func (c Config) ValidateBasic() error { - if c.AppTomlConfig.MinGasPrices == "" { - return cometerrors.ErrAppConfig.Wrap("set minimum-gas-prices in app.toml or flag or env variable") - } - - return nil -} diff --git a/server/v2/cometbft/flags.go b/server/v2/cometbft/flags.go index 3380d6f39c4e..55fa0a14e771 100644 --- a/server/v2/cometbft/flags.go +++ b/server/v2/cometbft/flags.go @@ -51,11 +51,10 @@ func prefix(f string) string { // Server flags var ( - Standalone = prefix("standalone") - FlagAddress = prefix("address") - FlagTransport = prefix("transport") - FlagHaltHeight = prefix("halt-height") - FlagHaltTime = prefix("halt-time") - FlagTrace = prefix("trace") - FlagMinGasPrices = prefix("minimum-gas-prices") + Standalone = prefix("standalone") + FlagAddress = prefix("address") + FlagTransport = prefix("transport") + FlagHaltHeight = prefix("halt-height") + FlagHaltTime = prefix("halt-time") + FlagTrace = prefix("trace") ) diff --git a/server/v2/cometbft/go.mod b/server/v2/cometbft/go.mod index 28ae4403dc47..7968bfd4175b 100644 --- a/server/v2/cometbft/go.mod +++ b/server/v2/cometbft/go.mod @@ -25,7 +25,6 @@ require ( cosmossdk.io/core v0.12.1-0.20240725072823-6a2d039e1212 cosmossdk.io/errors v1.0.1 cosmossdk.io/log v1.3.1 - cosmossdk.io/math v1.3.0 cosmossdk.io/server/v2 v2.0.0-00010101000000-000000000000 cosmossdk.io/server/v2/appmanager v0.0.0-00010101000000-000000000000 cosmossdk.io/store/v2 v2.0.0-00010101000000-000000000000 @@ -51,6 +50,7 @@ require ( cosmossdk.io/collections v0.4.0 // indirect cosmossdk.io/depinject v1.0.0 // indirect cosmossdk.io/errors/v2 v2.0.0-20240731132947-df72853b3ca5 // indirect + cosmossdk.io/math v1.3.0 // indirect cosmossdk.io/store v1.1.1-0.20240418092142-896cdf1971bc // indirect cosmossdk.io/x/auth v0.0.0-00010101000000-000000000000 // indirect cosmossdk.io/x/bank v0.0.0-20240226161501-23359a0b6d91 // indirect diff --git a/server/v2/cometbft/server.go b/server/v2/cometbft/server.go index c1ec2c6356da..c06c56c9be9e 100644 --- a/server/v2/cometbft/server.go +++ b/server/v2/cometbft/server.go @@ -130,11 +130,6 @@ func (s *CometBFTServer[T]) Name() string { } func (s *CometBFTServer[T]) Start(ctx context.Context) error { - // validate cometbft config - if err := s.config.ValidateBasic(); err != nil { - return err - } - wrappedLogger := cometlog.CometLoggerWrapper{Logger: s.logger} if s.config.AppTomlConfig.Standalone { svr, err := abciserver.NewServer(s.config.AppTomlConfig.Address, s.config.AppTomlConfig.Transport, s.Consensus) @@ -223,7 +218,6 @@ func (s *CometBFTServer[T]) StartCmdFlags() *pflag.FlagSet { flags.String(FlagTransport, "socket", "Transport protocol: socket, grpc") flags.Uint64(FlagHaltHeight, 0, "Block height at which to gracefully halt the chain and shutdown the node") flags.Uint64(FlagHaltTime, 0, "Minimum block time (in Unix seconds) at which to gracefully halt the chain and shutdown the node") - flags.String(FlagMinGasPrices, "", "Minimum gas prices to accept for transactions; Any fee in a tx must meet this minimum (e.g. 0.01photino;0.0001stake)") flags.Bool(FlagTrace, false, "Provide full stack traces for errors in ABCI Log") flags.Bool(Standalone, false, "Run app without CometBFT") diff --git a/server/v2/cometbft/types/errors/errors.go b/server/v2/cometbft/types/errors/errors.go index 93406a332cf4..380c176ff306 100644 --- a/server/v2/cometbft/types/errors/errors.go +++ b/server/v2/cometbft/types/errors/errors.go @@ -14,13 +14,4 @@ var ( // ErrInvalidRequest defines an ABCI typed error where the request contains // invalid data. ErrInvalidRequest = errorsmod.Register(RootCodespace, 2, "invalid request") - - // ErrTxDecode is returned if we cannot parse a transaction - ErrTxDecode = errorsmod.Register(RootCodespace, 3, "tx parse error") - - // ErrInsufficientFee is returned if provided fee in tx is less than required fee - ErrInsufficientFee = errorsmod.Register(RootCodespace, 4, "insufficient fee") - - // ErrAppConfig defines an error occurred if application configuration is misconfigured - ErrAppConfig = errorsmod.Register(RootCodespace, 5, "error in app.toml") ) diff --git a/server/v2/commands.go b/server/v2/commands.go index a0b2b05ae902..101a23d3983e 100644 --- a/server/v2/commands.go +++ b/server/v2/commands.go @@ -38,13 +38,14 @@ func AddCommands[T transaction.Tx]( rootCmd *cobra.Command, newApp AppCreator[T], logger log.Logger, + serverCfg ServerConfig, components ...ServerComponent[T], ) error { if len(components) == 0 { return errors.New("no components provided") } - server := NewServer(logger, components...) + server := NewServer(logger, serverCfg, components...) originalPersistentPreRunE := rootCmd.PersistentPreRunE rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { // set the default command outputs diff --git a/server/v2/config.go b/server/v2/config.go index 69aa25c3bdad..e15b315cc4b9 100644 --- a/server/v2/config.go +++ b/server/v2/config.go @@ -26,8 +26,8 @@ func DefaultMainServerConfig() ServerConfig { return ServerConfig{} } -// OverwriteDefaultConfig overwrites main server config with given config -func (s *Server[T]) OverwriteDefaultConfig(cfg ServerConfig) { +// SetMainServerConfig sets main server config with given config +func (s *Server[T]) SetMainServerConfig(cfg ServerConfig) { s.config = cfg } diff --git a/server/v2/server.go b/server/v2/server.go index 2221358f5bae..d08c57fa7553 100644 --- a/server/v2/server.go +++ b/server/v2/server.go @@ -70,10 +70,12 @@ type Server[T transaction.Tx] struct { func NewServer[T transaction.Tx]( logger log.Logger, + config ServerConfig, components ...ServerComponent[T], ) *Server[T] { return &Server[T]{ logger: logger, + config: config, components: components, } } diff --git a/server/v2/server_test.go b/server/v2/server_test.go index e757e7ecd5ca..f7de8a3f82e5 100644 --- a/server/v2/server_test.go +++ b/server/v2/server_test.go @@ -65,6 +65,7 @@ func TestServer(t *testing.T) { server := serverv2.NewServer( logger, + serverv2.ServerConfig{MinGasPrices: "0stake"}, grpcServer, mockServer, ) diff --git a/server/v2/testdata/app.toml b/server/v2/testdata/app.toml index ec7d77995d41..af2e3e7059d8 100644 --- a/server/v2/testdata/app.toml +++ b/server/v2/testdata/app.toml @@ -10,6 +10,10 @@ max-recv-msg-size = 10485760 # The default value is math.MaxInt32. max-send-msg-size = 2147483647 +[server] +# minimum-gas-prices defines the price which a validator is willing to accept for processing a transaction. A transaction's fees must meet the minimum of any denomination specified in this config (e.g. 0.25token1;0.0001token2). +minimum-gas-prices = '0stake' + [store] # The type of database for application and snapshots databases. app-db-backend = 'goleveldb' diff --git a/simapp/v2/simdv2/cmd/commands.go b/simapp/v2/simdv2/cmd/commands.go index 1915ba96d7e8..0d46823c79ec 100644 --- a/simapp/v2/simdv2/cmd/commands.go +++ b/simapp/v2/simdv2/cmd/commands.go @@ -72,31 +72,28 @@ func initRootCmd[T transaction.Tx]( // Optionally allow the chain developer to overwrite the cometbft default // app toml config. - cometAppTomlCfg := cometbft.DefaultAppTomlConfig() + serverCfg := serverv2.DefaultMainServerConfig() // The cometbft server's default minimum gas price is set to "" (empty value) inside // app.toml. If left empty by validators, the node will halt on startup. // However, the chain developer can set a default app.toml value for their // validators here. // // In summary: - // - if you leave cometAppTomlCfg.MinGasPrices = "", all validators MUST tweak their + // - if you leave serverCfg.MinGasPrices = "", all validators MUST tweak their // own app.toml config, - // - if you set cometAppTomlCfg.MinGasPrices non-empty, validators CAN tweak their + // - if you set serverCfg.MinGasPrices non-empty, validators CAN tweak their // own app.toml to override, or use this default value. // // In simapp, we set the min gas prices to 0. - cometAppTomlCfg.MinGasPrices = "0stake" + serverCfg.MinGasPrices = "0stake" // wire server commands if err = serverv2.AddCommands( rootCmd, newApp, logger, - cometbft.New( - &genericTxDecoder[T]{txConfig}, - cometbft.DefaultServerOptions[T](), - cometbft.OverwriteDefaultAppTomlConfig(cometAppTomlCfg), - ), + serverCfg, + cometbft.New(&genericTxDecoder[T]{txConfig}, cometbft.DefaultServerOptions[T]()), grpc.New[T](), store.New[T](), ); err != nil { diff --git a/simapp/v2/simdv2/cmd/testnet.go b/simapp/v2/simdv2/cmd/testnet.go index 823e4e1df7df..cefc65ce1ced 100644 --- a/simapp/v2/simdv2/cmd/testnet.go +++ b/simapp/v2/simdv2/cmd/testnet.go @@ -348,8 +348,7 @@ func initTestnetFiles[T transaction.Tx]( ) grpcServer := grpc.New[T](grpc.OverwriteDefaultConfig(grpcConfig)) storeServer := store.New[T]() - server := serverv2.NewServer(log.NewNopLogger(), cometServer, grpcServer, storeServer) - server.OverwriteDefaultConfig(serverCfg) + server := serverv2.NewServer(log.NewNopLogger(), serverCfg, cometServer, grpcServer, storeServer) err = server.WriteConfig(filepath.Join(nodeDir, "config")) if err != nil { return err diff --git a/tools/confix/data/v2-app.toml b/tools/confix/data/v2-app.toml index 1f7700aef33b..8f7f7f9940d2 100644 --- a/tools/confix/data/v2-app.toml +++ b/tools/confix/data/v2-app.toml @@ -1,6 +1,4 @@ [comet] -# minimum-gas-prices defines the price which a validator is willing to accept for processing a transaction. A transaction's fees must meet the minimum of any denomination specified in this config (e.g. 0.25token1;0.0001token2). -minimum-gas-prices = '0stake' # min-retain-blocks defines the minimum block height offset from the current block being committed, such that all blocks past this offset are pruned from CometBFT. A value of 0 indicates that no blocks should be pruned. min-retain-blocks = 0 # index-events defines the set of events in the form {eventType}.{attributeKey}, which informs CometBFT what to index. If empty, all events will be indexed. @@ -30,6 +28,10 @@ max-recv-msg-size = 10485760 # The default value is math.MaxInt32. max-send-msg-size = 2147483647 +[server] +# minimum-gas-prices defines the price which a validator is willing to accept for processing a transaction. A transaction's fees must meet the minimum of any denomination specified in this config (e.g. 0.25token1;0.0001token2). +minimum-gas-prices = '0stake' + [store] # The type of database for application and snapshots databases. app-db-backend = 'goleveldb' diff --git a/tools/confix/migrations.go b/tools/confix/migrations.go index b48e9096a284..250656a2f946 100644 --- a/tools/confix/migrations.go +++ b/tools/confix/migrations.go @@ -35,7 +35,7 @@ type v2KeyChangesMap map[string][]string // list all the keys which are need to be modified in v2 var v2KeyChanges = v2KeyChangesMap{ - "minimum-gas-prices": []string{"comet.minimum-gas-prices"}, + "minimum-gas-prices": []string{"server.minimum-gas-prices"}, "min-retain-blocks": []string{"comet.min-retain-blocks"}, "index-events": []string{"comet.index-events"}, "halt-height": []string{"comet.halt-height"}, diff --git a/x/auth/ante/validator_tx_fee.go b/x/auth/ante/validator_tx_fee.go index af4f02fed05f..5827edb585e1 100644 --- a/x/auth/ante/validator_tx_fee.go +++ b/x/auth/ante/validator_tx_fee.go @@ -1,8 +1,11 @@ package ante import ( + "context" "math" + "cosmossdk.io/core/appmodule" + "cosmossdk.io/core/transaction" errorsmod "cosmossdk.io/errors" sdkmath "cosmossdk.io/math" @@ -26,20 +29,8 @@ func checkTxFeeWithValidatorMinGasPrices(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, // is only ran on check tx. if ctx.ExecMode() == sdk.ExecModeCheck { // NOTE: using environment here breaks the API of fee logic, an alternative must be found for server/v2. ref: https://github.com/cosmos/cosmos-sdk/issues/19640 minGasPrices := ctx.MinGasPrices() - if !minGasPrices.IsZero() { - 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 := sdkmath.LegacyNewDec(int64(gas)) - for i, gp := range minGasPrices { - fee := gp.Amount.Mul(glDec) - requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) - } - - if !feeCoins.IsAnyGTE(requiredFees) { - return nil, 0, errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) - } + if err := validateMinGasPricesWithFee(feeCoins, gas, minGasPrices); err != nil { + return nil, 0, err } } @@ -66,3 +57,46 @@ func getTxPriority(fee sdk.Coins, gas int64) int64 { return priority } + +// validateMinGasPricesWithFee ensure that the provided fees meet a minimum threshold for the validator, +// if this is a CheckTx. This is only for local mempool purposes, and thus +// is only ran on check tx. +func validateMinGasPricesWithFee(feeCoins sdk.Coins, gas uint64, minGasPrices sdk.DecCoins) error { + if !minGasPrices.IsZero() { + 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 := sdkmath.LegacyNewDec(int64(gas)) + for i, gp := range minGasPrices { + fee := gp.Amount.Mul(glDec) + requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) + } + + if !feeCoins.IsAnyGTE(requiredFees) { + return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) + } + } + + return nil +} + +// CheckTxFeeWithMinGasPricesV2 calculates the default fee logic, where the minimum price per +// unit of gas is fixed and set by each validator and validates it with tx fees +func CheckTxFeeWithMinGasPricesV2(ctx context.Context, env appmodule.Environment, tx sdk.Tx, minGasPrices sdk.DecCoins) error { + // validate only on check tx + txService := env.TransactionService + if txService.ExecMode(ctx) != transaction.ExecModeCheck { + return nil + } + + feeTx, ok := tx.(sdk.FeeTx) + if !ok { + return nil // don't force users to implement fee tx + } + + feeCoins := feeTx.GetFee() + gas := feeTx.GetGas() + + return validateMinGasPricesWithFee(feeCoins, gas, minGasPrices) +} diff --git a/x/auth/depinject.go b/x/auth/depinject.go index 834912a4de30..6afd7e1f0500 100644 --- a/x/auth/depinject.go +++ b/x/auth/depinject.go @@ -1,6 +1,8 @@ package auth import ( + "fmt" + modulev1 "cosmossdk.io/api/cosmos/auth/module/v1" "cosmossdk.io/core/address" "cosmossdk.io/core/appmodule" @@ -12,6 +14,11 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/spf13/viper" +) + +const ( + FlagMinGasPricesV2 = "server.minimum-gas-prices" ) var _ depinject.OnePerModuleType = AppModule{} @@ -36,6 +43,7 @@ type ModuleInputs struct { AddressCodec address.Codec RandomGenesisAccountsFn types.RandomGenesisAccountsFn `optional:"true"` AccountI func() sdk.AccountI `optional:"true"` + Viper *viper.Viper `optional:"true"` // server v2 } type ModuleOutputs struct { @@ -73,5 +81,14 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { k := keeper.NewAccountKeeper(in.Environment, in.Cdc, in.AccountI, in.AccountsModKeeper, maccPerms, in.AddressCodec, in.Config.Bech32Prefix, auth) m := NewAppModule(in.Cdc, k, in.AccountsModKeeper, in.RandomGenesisAccountsFn) + if in.Viper != nil { + minGasPricesStr := in.Viper.GetString(FlagMinGasPricesV2) + minGasPrices, err := sdk.ParseDecCoins(minGasPricesStr) + if err != nil { + panic(fmt.Sprintf("invalid minimum gas prices: %v", err)) + } + m.SetMinGasPrices(minGasPrices) + } + return ModuleOutputs{AccountKeeper: k, Module: m} } diff --git a/x/auth/module.go b/x/auth/module.go index d440c8756990..e7b523a8f607 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -46,6 +46,7 @@ type AppModule struct { randGenAccountsFn types.RandomGenesisAccountsFn accountsModKeeper types.AccountsModKeeper cdc codec.Codec + minGasPrices sdk.DecCoins } // IsAppModule implements the appmodule.AppModule interface. @@ -151,9 +152,25 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) return am.cdc.MarshalJSON(gs) } +// SetMinGasPrices sets minimum gas prices in AppModule +func (am *AppModule) SetMinGasPrices(minGasPrices sdk.DecCoins) { + am.minGasPrices = minGasPrices +} + // TxValidator implements appmodulev2.HasTxValidator. // It replaces auth ante handlers for server/v2 func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { + sdkTx, ok := tx.(sdk.Tx) + if !ok { + return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx) + } + + // check tx fee with validator's minimum-gas-price config + if err := ante.CheckTxFeeWithMinGasPricesV2(ctx, am.accountKeeper.GetEnvironment(), + sdkTx, am.minGasPrices); err != nil { + return err + } + validators := []appmodulev2.TxValidator[sdk.Tx]{ ante.NewValidateBasicDecorator(am.accountKeeper.GetEnvironment()), ante.NewTxTimeoutHeightDecorator(am.accountKeeper.GetEnvironment()), @@ -162,11 +179,6 @@ func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { ante.NewValidateSigCountDecorator(am.accountKeeper), } - sdkTx, ok := tx.(sdk.Tx) - if !ok { - return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx) - } - for _, validator := range validators { if err := validator.ValidateTx(ctx, sdkTx); err != nil { return err From b1593b59238634b3a19c88d4708f72ddfb38aa3b Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Thu, 8 Aug 2024 12:26:15 +0530 Subject: [PATCH 06/28] revert typo --- server/v2/cometbft/config.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/server/v2/cometbft/config.go b/server/v2/cometbft/config.go index 8f2d3a929e5f..3f03e383c601 100644 --- a/server/v2/cometbft/config.go +++ b/server/v2/cometbft/config.go @@ -15,13 +15,14 @@ type Config struct { func DefaultAppTomlConfig() *AppTomlConfig { return &AppTomlConfig{ - IndexEvents: make([]string, 0), - HaltHeight: 0, - HaltTime: 0, - Address: "tcp://127.0.0.1:26658", - Transport: "socket", - Trace: false, - Standalone: false, + MinRetainBlocks: 0, + IndexEvents: make([]string, 0), + HaltHeight: 0, + HaltTime: 0, + Address: "tcp://127.0.0.1:26658", + Transport: "socket", + Trace: false, + Standalone: false, } } From 650bfef72664d72c9962d57a4fe30d72d9f7ec87 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Thu, 8 Aug 2024 16:30:23 +0530 Subject: [PATCH 07/28] fix lint --- x/auth/depinject.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/auth/depinject.go b/x/auth/depinject.go index 6afd7e1f0500..58adbf23caa3 100644 --- a/x/auth/depinject.go +++ b/x/auth/depinject.go @@ -3,6 +3,8 @@ package auth import ( "fmt" + "github.com/spf13/viper" + modulev1 "cosmossdk.io/api/cosmos/auth/module/v1" "cosmossdk.io/core/address" "cosmossdk.io/core/appmodule" @@ -14,7 +16,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/spf13/viper" ) const ( From f5c12723f51f9d1e720b37e8960595df3d9bea4d Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Fri, 9 Aug 2024 16:32:24 +0530 Subject: [PATCH 08/28] WIP: update DeductFeeDecorator --- x/auth/ante/fee.go | 137 ++++++++++++++++++++++---------- x/auth/ante/validator_tx_fee.go | 76 +++++------------- x/auth/depinject.go | 19 +++++ x/auth/module.go | 38 ++++++--- x/auth/txvalidator.go | 27 +++++++ 5 files changed, 185 insertions(+), 112 deletions(-) create mode 100644 x/auth/txvalidator.go diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index 298fae30a4cc..d8b5ee1fa8e2 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -2,6 +2,7 @@ package ante import ( "bytes" + "context" "fmt" "cosmossdk.io/core/transaction" @@ -14,7 +15,7 @@ import ( // TxFeeChecker checks if the provided fee is enough and returns the effective fee and tx priority. // The effective fee should be deducted later, and the priority should be returned in the ABCI response. -type TxFeeChecker func(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) +type TxFeeChecker func(ctx context.Context, tx sdk.Tx) (sdk.Coins, int64, error) // DeductFeeDecorator deducts fees from the fee payer. The fee payer is the fee granter (if specified) or first signer of the tx. // If the fee payer does not have the funds to pay for the fees, return an InsufficientFunds error. @@ -25,68 +26,122 @@ type DeductFeeDecorator struct { bankKeeper types.BankKeeper feegrantKeeper FeegrantKeeper txFeeChecker TxFeeChecker + + // store below fields to use in few methods + globalFields dfdGlobalFields } -func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper, tfc TxFeeChecker) DeductFeeDecorator { - if tfc == nil { - tfc = checkTxFeeWithValidatorMinGasPrices - } +const globalFieldsKey = "dfdGlobalFields" - return DeductFeeDecorator{ +type dfdGlobalFields struct { + minGasPrices sdk.DecCoins + feeTx sdk.FeeTx + txPriority int64 + deductFeesFrom []byte + txFee sdk.Coins + execMode transaction.ExecMode +} + +func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper, tfc TxFeeChecker) DeductFeeDecorator { + dfd := DeductFeeDecorator{ accountKeeper: ak, bankKeeper: bk, feegrantKeeper: fk, txFeeChecker: tfc, } + + if tfc == nil { + dfd.txFeeChecker = func(ctx context.Context, tx sdk.Tx) (sdk.Coins, int64, error) { + return dfd.checkTxFeeWithValidatorMinGasPrices(ctx, tx) + } + } + + return dfd +} + +// UpdateGlobalFields updates the global fields required for the DeductFeeDecorator +func (dfd *DeductFeeDecorator) UpdateGlobalFields(updated dfdGlobalFields) { + dfd.globalFields = updated +} + +// SetMinGasPrices sets the minimum-gas-prices value in global fields of DeductFeeDecorator +func (dfd *DeductFeeDecorator) SetMinGasPrices(minGasPrices sdk.DecCoins) { + dfd.globalFields.minGasPrices = minGasPrices } +// AnteHandle implements an AnteHandler decorator for the DeductFeeDecorator func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (sdk.Context, error) { + // update min-gas-price + dfd.SetMinGasPrices(ctx.MinGasPrices()) + + if err := dfd.ValidateTx(ctx, tx); err != nil { + return ctx, err + } + + events := sdk.Events{ + sdk.NewEvent( + sdk.EventTypeTx, + sdk.NewAttribute(sdk.AttributeKeyFee, dfd.globalFields.txFee.String()), + sdk.NewAttribute(sdk.AttributeKeyFeePayer, sdk.AccAddress(dfd.globalFields.deductFeesFrom).String()), + ), + } + ctx.EventManager().EmitEvents(events) + + newCtx := ctx.WithPriority(dfd.globalFields.txPriority) + return next(newCtx, tx, false) +} + +// ValidateTx implements an TxValidator for DeductFeeDecorator +func (dfd DeductFeeDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error { + globalFields := dfd.globalFields + feeTx, ok := tx.(sdk.FeeTx) if !ok { - return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must implement the FeeTx interface") + return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must implement the FeeTx interface") } + globalFields.feeTx = feeTx + + globalFields.execMode = dfd.accountKeeper.GetEnvironment().TransactionService.ExecMode(ctx) + headerInfo := dfd.accountKeeper.GetEnvironment().HeaderService.HeaderInfo(ctx) - txService := dfd.accountKeeper.GetEnvironment().TransactionService - execMode := txService.ExecMode(ctx) - if execMode != transaction.ExecModeSimulate && ctx.BlockHeight() > 0 && feeTx.GetGas() == 0 { - return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") + if globalFields.execMode != transaction.ExecModeSimulate && headerInfo.Height > 0 && globalFields.feeTx.GetGas() == 0 { + return errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") } - var ( - priority int64 - err error - ) + // update global fields values + globalFields.txFee = globalFields.feeTx.GetFee() + dfd.UpdateGlobalFields(globalFields) - fee := feeTx.GetFee() - if execMode != transaction.ExecModeSimulate { - fee, priority, err = dfd.txFeeChecker(ctx, tx) + ctx = context.WithValue(ctx, globalFieldsKey, dfd.globalFields) + + var err error + + if globalFields.execMode != transaction.ExecModeSimulate { + dfd.globalFields.txFee, dfd.globalFields.txPriority, err = dfd.txFeeChecker(ctx, tx) if err != nil { - return ctx, err + return err } } - if err := dfd.checkDeductFee(ctx, tx, fee); err != nil { - return ctx, err - } - newCtx := ctx.WithPriority(priority) + dfd.UpdateGlobalFields(globalFields) - return next(newCtx, tx, false) + if err := dfd.checkDeductFee(ctx, tx, dfd.globalFields.txFee); err != nil { + return err + } + return nil } -func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee sdk.Coins) error { - feeTx, ok := sdkTx.(sdk.FeeTx) - if !ok { - return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must implement the FeeTx interface") - } +func (dfd DeductFeeDecorator) checkDeductFee(ctx context.Context, sdkTx sdk.Tx, fee sdk.Coins) error { + globalFields := dfd.globalFields addr := dfd.accountKeeper.GetModuleAddress(types.FeeCollectorName) if len(addr) == 0 { return fmt.Errorf("fee collector module account (%s) has not been set", types.FeeCollectorName) } - feePayer := feeTx.FeePayer() - feeGranter := feeTx.FeeGranter() - deductFeesFrom := feePayer + feePayer := globalFields.feeTx.FeePayer() + feeGranter := globalFields.feeTx.FeeGranter() + globalFields.deductFeesFrom = feePayer // if feegranter set, deduct fee from feegranter account. // this works only when feegrant is enabled. @@ -110,31 +165,25 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx sdk.Context, sdkTx sdk.Tx, fee } } - deductFeesFrom = feeGranterAddr + globalFields.deductFeesFrom = feeGranterAddr } + // update global fields + dfd.UpdateGlobalFields(globalFields) + // deduct the fees if !fee.IsZero() { - err := DeductFees(dfd.bankKeeper, ctx, deductFeesFrom, fee) + err := DeductFees(dfd.bankKeeper, ctx, globalFields.deductFeesFrom, fee) if err != nil { return err } } - events := sdk.Events{ - sdk.NewEvent( - sdk.EventTypeTx, - sdk.NewAttribute(sdk.AttributeKeyFee, fee.String()), - sdk.NewAttribute(sdk.AttributeKeyFeePayer, sdk.AccAddress(deductFeesFrom).String()), - ), - } - ctx.EventManager().EmitEvents(events) - return nil } // DeductFees deducts fees from the given account. -func DeductFees(bankKeeper types.BankKeeper, ctx sdk.Context, acc []byte, fees sdk.Coins) error { +func DeductFees(bankKeeper types.BankKeeper, ctx context.Context, acc []byte, fees sdk.Coins) error { if !fees.IsValid() { return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) } diff --git a/x/auth/ante/validator_tx_fee.go b/x/auth/ante/validator_tx_fee.go index 5827edb585e1..72e1993c0cf8 100644 --- a/x/auth/ante/validator_tx_fee.go +++ b/x/auth/ante/validator_tx_fee.go @@ -4,7 +4,6 @@ import ( "context" "math" - "cosmossdk.io/core/appmodule" "cosmossdk.io/core/transaction" errorsmod "cosmossdk.io/errors" sdkmath "cosmossdk.io/math" @@ -15,22 +14,30 @@ import ( // checkTxFeeWithValidatorMinGasPrices implements the default fee logic, where the minimum price per // unit of gas is fixed and set by each validator, can the tx priority is computed from the gas price. -func checkTxFeeWithValidatorMinGasPrices(ctx 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") - } - - feeCoins := feeTx.GetFee() - gas := feeTx.GetGas() +func (dfd DeductFeeDecorator) checkTxFeeWithValidatorMinGasPrices(ctx context.Context, _ sdk.Tx) (sdk.Coins, int64, error) { + globalFields := ctx.Value(globalFieldsKey).(dfdGlobalFields) + feeCoins := globalFields.feeTx.GetFee() + gas := globalFields.feeTx.GetGas() // Ensure that the provided fees meet a minimum threshold for the validator, // if this is a CheckTx. This is only for local mempool purposes, and thus // is only ran on check tx. - if ctx.ExecMode() == sdk.ExecModeCheck { // NOTE: using environment here breaks the API of fee logic, an alternative must be found for server/v2. ref: https://github.com/cosmos/cosmos-sdk/issues/19640 - minGasPrices := ctx.MinGasPrices() - if err := validateMinGasPricesWithFee(feeCoins, gas, minGasPrices); err != nil { - return nil, 0, err + if globalFields.execMode == transaction.ExecModeCheck { + minGasPrices := globalFields.minGasPrices + if !minGasPrices.IsZero() { + 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 := sdkmath.LegacyNewDec(int64(gas)) + for i, gp := range minGasPrices { + fee := gp.Amount.Mul(glDec) + requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) + } + + if !feeCoins.IsAnyGTE(requiredFees) { + return nil, 0, errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) + } } } @@ -57,46 +64,3 @@ func getTxPriority(fee sdk.Coins, gas int64) int64 { return priority } - -// validateMinGasPricesWithFee ensure that the provided fees meet a minimum threshold for the validator, -// if this is a CheckTx. This is only for local mempool purposes, and thus -// is only ran on check tx. -func validateMinGasPricesWithFee(feeCoins sdk.Coins, gas uint64, minGasPrices sdk.DecCoins) error { - if !minGasPrices.IsZero() { - 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 := sdkmath.LegacyNewDec(int64(gas)) - for i, gp := range minGasPrices { - fee := gp.Amount.Mul(glDec) - requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) - } - - if !feeCoins.IsAnyGTE(requiredFees) { - return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) - } - } - - return nil -} - -// CheckTxFeeWithMinGasPricesV2 calculates the default fee logic, where the minimum price per -// unit of gas is fixed and set by each validator and validates it with tx fees -func CheckTxFeeWithMinGasPricesV2(ctx context.Context, env appmodule.Environment, tx sdk.Tx, minGasPrices sdk.DecCoins) error { - // validate only on check tx - txService := env.TransactionService - if txService.ExecMode(ctx) != transaction.ExecModeCheck { - return nil - } - - feeTx, ok := tx.(sdk.FeeTx) - if !ok { - return nil // don't force users to implement fee tx - } - - feeCoins := feeTx.GetFee() - gas := feeTx.GetGas() - - return validateMinGasPricesWithFee(feeCoins, gas, minGasPrices) -} diff --git a/x/auth/depinject.go b/x/auth/depinject.go index 58adbf23caa3..9b92275c0d61 100644 --- a/x/auth/depinject.go +++ b/x/auth/depinject.go @@ -10,6 +10,7 @@ import ( "cosmossdk.io/core/appmodule" "cosmossdk.io/depinject" "cosmossdk.io/depinject/appconfig" + "cosmossdk.io/x/auth/ante" "cosmossdk.io/x/auth/keeper" "cosmossdk.io/x/auth/simulation" "cosmossdk.io/x/auth/types" @@ -45,6 +46,10 @@ type ModuleInputs struct { RandomGenesisAccountsFn types.RandomGenesisAccountsFn `optional:"true"` AccountI func() sdk.AccountI `optional:"true"` Viper *viper.Viper `optional:"true"` // server v2 + // BankKeeper is the expected bank keeper to be passed to TxValidator + BankKeeper types.BankKeeper `optional:"true"` // server v2 + // FeegrantKeeper is the expected feegrant keeper to be passed to TxValidator + FeegrantKeeper ante.FeegrantKeeper `optional:"true"` // server v2 } type ModuleOutputs struct { @@ -91,5 +96,19 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { m.SetMinGasPrices(minGasPrices) } + // check bank keeper and add it to tx validator options if found + if in.BankKeeper != nil { + options := m.TxValidatorOptions() + options.BankKeeper = in.BankKeeper + m.SetTxValidatorOptions(options) + } + + // check feegrant keeper and add it to tx validator options if found + if in.FeegrantKeeper != nil { + options := m.TxValidatorOptions() + options.FeegrantKeeper = in.FeegrantKeeper + m.SetTxValidatorOptions(options) + } + return ModuleOutputs{AccountKeeper: k, Module: m} } diff --git a/x/auth/module.go b/x/auth/module.go index 79b7d6a73f8f..88a7be0c97a9 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -45,7 +45,10 @@ type AppModule struct { randGenAccountsFn types.RandomGenesisAccountsFn accountsModKeeper types.AccountsModKeeper cdc codec.Codec - minGasPrices sdk.DecCoins + + // v2 tx validator + txValidatorOptions TxValidatorOptions + minGasPrices sdk.DecCoins } // IsAppModule implements the appmodule.AppModule interface. @@ -151,6 +154,16 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) return am.cdc.MarshalJSON(gs) } +// SetTxValidatorOptions sets txValidationOptions of AppModule +func (am *AppModule) SetTxValidatorOptions(options TxValidatorOptions) { + am.txValidatorOptions = options +} + +// TxValidatorOptions sets txValidationOptions of AppModule +func (am AppModule) TxValidatorOptions() TxValidatorOptions { + return am.txValidatorOptions +} + // SetMinGasPrices sets minimum gas prices in AppModule func (am *AppModule) SetMinGasPrices(minGasPrices sdk.DecCoins) { am.minGasPrices = minGasPrices @@ -159,17 +172,6 @@ func (am *AppModule) SetMinGasPrices(minGasPrices sdk.DecCoins) { // TxValidator implements appmodulev2.HasTxValidator. // It replaces auth ante handlers for server/v2 func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { - sdkTx, ok := tx.(sdk.Tx) - if !ok { - return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx) - } - - // check tx fee with validator's minimum-gas-price config - if err := ante.CheckTxFeeWithMinGasPricesV2(ctx, am.accountKeeper.GetEnvironment(), - sdkTx, am.minGasPrices); err != nil { - return err - } - validators := []appmodulev2.TxValidator[sdk.Tx]{ ante.NewValidateBasicDecorator(am.accountKeeper.GetEnvironment()), ante.NewTxTimeoutHeightDecorator(am.accountKeeper.GetEnvironment()), @@ -178,6 +180,18 @@ func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { ante.NewValidateSigCountDecorator(am.accountKeeper), } + if am.txValidatorOptions.Validate() == nil { + dfd := ante.NewDeductFeeDecorator(am.accountKeeper, am.txValidatorOptions.BankKeeper, + am.txValidatorOptions.FeegrantKeeper, nil) + dfd.SetMinGasPrices(am.minGasPrices) + validators = append(validators, dfd) + } + + sdkTx, ok := tx.(sdk.Tx) + if !ok { + return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx) + } + for _, validator := range validators { if err := validator.ValidateTx(ctx, sdkTx); err != nil { return err diff --git a/x/auth/txvalidator.go b/x/auth/txvalidator.go new file mode 100644 index 000000000000..c5470d84062b --- /dev/null +++ b/x/auth/txvalidator.go @@ -0,0 +1,27 @@ +package auth + +import ( + "fmt" + + "cosmossdk.io/x/auth/ante" + "cosmossdk.io/x/auth/types" +) + +// TxValidatorOptions defines the keepers needed to run checks in TxValidator +type TxValidatorOptions struct { + BankKeeper types.BankKeeper + FeegrantKeeper ante.FeegrantKeeper +} + +// Validate validates the options of tx validator +func (options TxValidatorOptions) Validate() error { + if options.BankKeeper == nil { + return fmt.Errorf("no bank keeper found for tx validator") + } + + if options.FeegrantKeeper == nil { + return fmt.Errorf("no feegrant keeper found for tx validator") + } + + return nil +} From 7cad0b8222840da110ac569ba2de5be0b5a67006 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Mon, 12 Aug 2024 12:03:20 +0530 Subject: [PATCH 09/28] address comments, fix tests --- server/v2/config.go | 9 ++---- simapp/v2/simdv2/cmd/commands.go | 2 +- simapp/v2/simdv2/cmd/testnet.go | 2 +- x/auth/ante/fee.go | 47 +++++++++----------------------- x/auth/ante/fee_test.go | 5 ++++ x/auth/ante/validator_tx_fee.go | 1 - x/auth/module.go | 3 +- 7 files changed, 24 insertions(+), 45 deletions(-) diff --git a/server/v2/config.go b/server/v2/config.go index e15b315cc4b9..7b2a4fdb9492 100644 --- a/server/v2/config.go +++ b/server/v2/config.go @@ -21,16 +21,11 @@ func (c ServerConfig) ValidateBasic() error { return nil } -// DefaultMainServerConfig returns the default config of main server component -func DefaultMainServerConfig() ServerConfig { +// DefaultServerConfig returns the default config of server component +func DefaultServerConfig() ServerConfig { return ServerConfig{} } -// SetMainServerConfig sets main server config with given config -func (s *Server[T]) SetMainServerConfig(cfg ServerConfig) { - s.config = cfg -} - // ReadConfig returns a viper instance of the config file func ReadConfig(configPath string) (*viper.Viper, error) { v := viper.New() diff --git a/simapp/v2/simdv2/cmd/commands.go b/simapp/v2/simdv2/cmd/commands.go index 5b056265a021..3466963d226b 100644 --- a/simapp/v2/simdv2/cmd/commands.go +++ b/simapp/v2/simdv2/cmd/commands.go @@ -70,7 +70,7 @@ func initRootCmd[T transaction.Tx]( // Optionally allow the chain developer to overwrite the cometbft default // app toml config. - serverCfg := serverv2.DefaultMainServerConfig() + serverCfg := serverv2.DefaultServerConfig() // The cometbft server's default minimum gas price is set to "" (empty value) inside // app.toml. If left empty by validators, the node will halt on startup. // However, the chain developer can set a default app.toml value for their diff --git a/simapp/v2/simdv2/cmd/testnet.go b/simapp/v2/simdv2/cmd/testnet.go index c59829e35e39..0e2c90728387 100644 --- a/simapp/v2/simdv2/cmd/testnet.go +++ b/simapp/v2/simdv2/cmd/testnet.go @@ -337,7 +337,7 @@ func initTestnetFiles[T transaction.Tx]( return err } - serverCfg := serverv2.DefaultMainServerConfig() + serverCfg := serverv2.DefaultServerConfig() serverCfg.MinGasPrices = args.minGasPrices // Write server config diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index d8b5ee1fa8e2..16fd24189577 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -26,13 +26,9 @@ type DeductFeeDecorator struct { bankKeeper types.BankKeeper feegrantKeeper FeegrantKeeper txFeeChecker TxFeeChecker - - // store below fields to use in few methods - globalFields dfdGlobalFields } -const globalFieldsKey = "dfdGlobalFields" - +// store below fields globally to use in different methods type dfdGlobalFields struct { minGasPrices sdk.DecCoins feeTx sdk.FeeTx @@ -42,6 +38,8 @@ type dfdGlobalFields struct { execMode transaction.ExecMode } +var globalFields = dfdGlobalFields{} + func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper, tfc TxFeeChecker) DeductFeeDecorator { dfd := DeductFeeDecorator{ accountKeeper: ak, @@ -51,28 +49,20 @@ func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKee } if tfc == nil { - dfd.txFeeChecker = func(ctx context.Context, tx sdk.Tx) (sdk.Coins, int64, error) { - return dfd.checkTxFeeWithValidatorMinGasPrices(ctx, tx) - } + dfd.txFeeChecker = dfd.checkTxFeeWithValidatorMinGasPrices } return dfd } -// UpdateGlobalFields updates the global fields required for the DeductFeeDecorator -func (dfd *DeductFeeDecorator) UpdateGlobalFields(updated dfdGlobalFields) { - dfd.globalFields = updated -} - // SetMinGasPrices sets the minimum-gas-prices value in global fields of DeductFeeDecorator -func (dfd *DeductFeeDecorator) SetMinGasPrices(minGasPrices sdk.DecCoins) { - dfd.globalFields.minGasPrices = minGasPrices +func SetMinGasPrices(minGasPrices sdk.DecCoins) { + globalFields.minGasPrices = minGasPrices } // AnteHandle implements an AnteHandler decorator for the DeductFeeDecorator func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (sdk.Context, error) { - // update min-gas-price - dfd.SetMinGasPrices(ctx.MinGasPrices()) + globalFields.minGasPrices = ctx.MinGasPrices() if err := dfd.ValidateTx(ctx, tx); err != nil { return ctx, err @@ -81,20 +71,18 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, nex events := sdk.Events{ sdk.NewEvent( sdk.EventTypeTx, - sdk.NewAttribute(sdk.AttributeKeyFee, dfd.globalFields.txFee.String()), - sdk.NewAttribute(sdk.AttributeKeyFeePayer, sdk.AccAddress(dfd.globalFields.deductFeesFrom).String()), + sdk.NewAttribute(sdk.AttributeKeyFee, globalFields.txFee.String()), + sdk.NewAttribute(sdk.AttributeKeyFeePayer, sdk.AccAddress(globalFields.deductFeesFrom).String()), ), } ctx.EventManager().EmitEvents(events) - newCtx := ctx.WithPriority(dfd.globalFields.txPriority) + newCtx := ctx.WithPriority(globalFields.txPriority) return next(newCtx, tx, false) } // ValidateTx implements an TxValidator for DeductFeeDecorator func (dfd DeductFeeDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error { - globalFields := dfd.globalFields - feeTx, ok := tx.(sdk.FeeTx) if !ok { return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must implement the FeeTx interface") @@ -110,30 +98,24 @@ func (dfd DeductFeeDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error { // update global fields values globalFields.txFee = globalFields.feeTx.GetFee() - dfd.UpdateGlobalFields(globalFields) - - ctx = context.WithValue(ctx, globalFieldsKey, dfd.globalFields) var err error if globalFields.execMode != transaction.ExecModeSimulate { - dfd.globalFields.txFee, dfd.globalFields.txPriority, err = dfd.txFeeChecker(ctx, tx) + globalFields.txFee, globalFields.txPriority, err = dfd.txFeeChecker(ctx, tx) if err != nil { return err } } - dfd.UpdateGlobalFields(globalFields) - - if err := dfd.checkDeductFee(ctx, tx, dfd.globalFields.txFee); err != nil { + if err := dfd.checkDeductFee(ctx, tx, globalFields.txFee); err != nil { return err } + return nil } func (dfd DeductFeeDecorator) checkDeductFee(ctx context.Context, sdkTx sdk.Tx, fee sdk.Coins) error { - globalFields := dfd.globalFields - addr := dfd.accountKeeper.GetModuleAddress(types.FeeCollectorName) if len(addr) == 0 { return fmt.Errorf("fee collector module account (%s) has not been set", types.FeeCollectorName) @@ -168,9 +150,6 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx context.Context, sdkTx sdk.Tx, globalFields.deductFeesFrom = feeGranterAddr } - // update global fields - dfd.UpdateGlobalFields(globalFields) - // deduct the fees if !fee.IsZero() { err := DeductFees(dfd.bankKeeper, ctx, globalFields.deductFeesFrom, fee) diff --git a/x/auth/ante/fee_test.go b/x/auth/ante/fee_test.go index 0c9fa299a5e4..6736f17aed99 100644 --- a/x/auth/ante/fee_test.go +++ b/x/auth/ante/fee_test.go @@ -41,6 +41,11 @@ func TestDeductFeeDecorator_ZeroGas(t *testing.T) { // Set IsCheckTx to true s.ctx = s.ctx.WithIsCheckTx(true) + // Set current block height in headerInfo + headerInfo := s.ctx.HeaderInfo() + headerInfo.Height = s.ctx.BlockHeight() + s.ctx = s.ctx.WithHeaderInfo(headerInfo) + _, err = antehandler(s.ctx, tx, false) require.Error(t, err) diff --git a/x/auth/ante/validator_tx_fee.go b/x/auth/ante/validator_tx_fee.go index 72e1993c0cf8..886feca0c691 100644 --- a/x/auth/ante/validator_tx_fee.go +++ b/x/auth/ante/validator_tx_fee.go @@ -15,7 +15,6 @@ import ( // checkTxFeeWithValidatorMinGasPrices implements the default fee logic, where the minimum price per // unit of gas is fixed and set by each validator, can the tx priority is computed from the gas price. func (dfd DeductFeeDecorator) checkTxFeeWithValidatorMinGasPrices(ctx context.Context, _ sdk.Tx) (sdk.Coins, int64, error) { - globalFields := ctx.Value(globalFieldsKey).(dfdGlobalFields) feeCoins := globalFields.feeTx.GetFee() gas := globalFields.feeTx.GetGas() diff --git a/x/auth/module.go b/x/auth/module.go index 88a7be0c97a9..7ec5bf7c1be2 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -183,7 +183,8 @@ func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { if am.txValidatorOptions.Validate() == nil { dfd := ante.NewDeductFeeDecorator(am.accountKeeper, am.txValidatorOptions.BankKeeper, am.txValidatorOptions.FeegrantKeeper, nil) - dfd.SetMinGasPrices(am.minGasPrices) + // set minimum-gas-prices to use in DeductFeeDecorator + ante.SetMinGasPrices(am.minGasPrices) validators = append(validators, dfd) } From 25528b0c911b6fcc67ecc3e7aac4f3aed01f805f Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Mon, 12 Aug 2024 14:41:10 +0530 Subject: [PATCH 10/28] move min-gas-price check to feegrant TxValidator --- x/auth/ante/fee.go | 1 + x/auth/depinject.go | 38 -------- x/auth/module.go | 27 ------ x/auth/txvalidator.go | 27 ------ x/feegrant/expected_keepers.go | 6 ++ x/feegrant/module/depinject.go | 19 ++++ x/feegrant/module/module.go | 36 ++++++++ x/feegrant/testutil/expected_keepers_mocks.go | 89 ++++++++++++++++--- 8 files changed, 138 insertions(+), 105 deletions(-) delete mode 100644 x/auth/txvalidator.go diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index 16fd24189577..7e7de75bebca 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -68,6 +68,7 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, nex return ctx, err } + // TODO: emit this event in v2 after executing ValidateTx events := sdk.Events{ sdk.NewEvent( sdk.EventTypeTx, diff --git a/x/auth/depinject.go b/x/auth/depinject.go index 9b92275c0d61..f11a3bb57538 100644 --- a/x/auth/depinject.go +++ b/x/auth/depinject.go @@ -1,16 +1,11 @@ package auth import ( - "fmt" - - "github.com/spf13/viper" - modulev1 "cosmossdk.io/api/cosmos/auth/module/v1" "cosmossdk.io/core/address" "cosmossdk.io/core/appmodule" "cosmossdk.io/depinject" "cosmossdk.io/depinject/appconfig" - "cosmossdk.io/x/auth/ante" "cosmossdk.io/x/auth/keeper" "cosmossdk.io/x/auth/simulation" "cosmossdk.io/x/auth/types" @@ -19,10 +14,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -const ( - FlagMinGasPricesV2 = "server.minimum-gas-prices" -) - var _ depinject.OnePerModuleType = AppModule{} // IsOnePerModuleType implements the depinject.OnePerModuleType interface. @@ -45,11 +36,6 @@ type ModuleInputs struct { AddressCodec address.Codec RandomGenesisAccountsFn types.RandomGenesisAccountsFn `optional:"true"` AccountI func() sdk.AccountI `optional:"true"` - Viper *viper.Viper `optional:"true"` // server v2 - // BankKeeper is the expected bank keeper to be passed to TxValidator - BankKeeper types.BankKeeper `optional:"true"` // server v2 - // FeegrantKeeper is the expected feegrant keeper to be passed to TxValidator - FeegrantKeeper ante.FeegrantKeeper `optional:"true"` // server v2 } type ModuleOutputs struct { @@ -86,29 +72,5 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { k := keeper.NewAccountKeeper(in.Environment, in.Cdc, in.AccountI, in.AccountsModKeeper, maccPerms, in.AddressCodec, in.Config.Bech32Prefix, auth) m := NewAppModule(in.Cdc, k, in.AccountsModKeeper, in.RandomGenesisAccountsFn) - - if in.Viper != nil { - minGasPricesStr := in.Viper.GetString(FlagMinGasPricesV2) - minGasPrices, err := sdk.ParseDecCoins(minGasPricesStr) - if err != nil { - panic(fmt.Sprintf("invalid minimum gas prices: %v", err)) - } - m.SetMinGasPrices(minGasPrices) - } - - // check bank keeper and add it to tx validator options if found - if in.BankKeeper != nil { - options := m.TxValidatorOptions() - options.BankKeeper = in.BankKeeper - m.SetTxValidatorOptions(options) - } - - // check feegrant keeper and add it to tx validator options if found - if in.FeegrantKeeper != nil { - options := m.TxValidatorOptions() - options.FeegrantKeeper = in.FeegrantKeeper - m.SetTxValidatorOptions(options) - } - return ModuleOutputs{AccountKeeper: k, Module: m} } diff --git a/x/auth/module.go b/x/auth/module.go index 7ec5bf7c1be2..ae028ab5fcda 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -45,10 +45,6 @@ type AppModule struct { randGenAccountsFn types.RandomGenesisAccountsFn accountsModKeeper types.AccountsModKeeper cdc codec.Codec - - // v2 tx validator - txValidatorOptions TxValidatorOptions - minGasPrices sdk.DecCoins } // IsAppModule implements the appmodule.AppModule interface. @@ -154,21 +150,6 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) return am.cdc.MarshalJSON(gs) } -// SetTxValidatorOptions sets txValidationOptions of AppModule -func (am *AppModule) SetTxValidatorOptions(options TxValidatorOptions) { - am.txValidatorOptions = options -} - -// TxValidatorOptions sets txValidationOptions of AppModule -func (am AppModule) TxValidatorOptions() TxValidatorOptions { - return am.txValidatorOptions -} - -// SetMinGasPrices sets minimum gas prices in AppModule -func (am *AppModule) SetMinGasPrices(minGasPrices sdk.DecCoins) { - am.minGasPrices = minGasPrices -} - // TxValidator implements appmodulev2.HasTxValidator. // It replaces auth ante handlers for server/v2 func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { @@ -180,14 +161,6 @@ func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { ante.NewValidateSigCountDecorator(am.accountKeeper), } - if am.txValidatorOptions.Validate() == nil { - dfd := ante.NewDeductFeeDecorator(am.accountKeeper, am.txValidatorOptions.BankKeeper, - am.txValidatorOptions.FeegrantKeeper, nil) - // set minimum-gas-prices to use in DeductFeeDecorator - ante.SetMinGasPrices(am.minGasPrices) - validators = append(validators, dfd) - } - sdkTx, ok := tx.(sdk.Tx) if !ok { return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx) diff --git a/x/auth/txvalidator.go b/x/auth/txvalidator.go deleted file mode 100644 index c5470d84062b..000000000000 --- a/x/auth/txvalidator.go +++ /dev/null @@ -1,27 +0,0 @@ -package auth - -import ( - "fmt" - - "cosmossdk.io/x/auth/ante" - "cosmossdk.io/x/auth/types" -) - -// TxValidatorOptions defines the keepers needed to run checks in TxValidator -type TxValidatorOptions struct { - BankKeeper types.BankKeeper - FeegrantKeeper ante.FeegrantKeeper -} - -// Validate validates the options of tx validator -func (options TxValidatorOptions) Validate() error { - if options.BankKeeper == nil { - return fmt.Errorf("no bank keeper found for tx validator") - } - - if options.FeegrantKeeper == nil { - return fmt.Errorf("no feegrant keeper found for tx validator") - } - - return nil -} diff --git a/x/feegrant/expected_keepers.go b/x/feegrant/expected_keepers.go index ce6a2d0a5698..addc4bdf11d7 100644 --- a/x/feegrant/expected_keepers.go +++ b/x/feegrant/expected_keepers.go @@ -4,6 +4,8 @@ import ( "context" "cosmossdk.io/core/address" + "cosmossdk.io/core/appmodule" + authtypes "cosmossdk.io/x/auth/types" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -18,10 +20,14 @@ type AccountKeeper interface { NewAccountWithAddress(ctx context.Context, addr sdk.AccAddress) sdk.AccountI GetAccount(ctx context.Context, addr sdk.AccAddress) sdk.AccountI SetAccount(ctx context.Context, acc sdk.AccountI) + GetParams(ctx context.Context) (params authtypes.Params) + GetEnvironment() appmodule.Environment } // BankKeeper defines the expected supply Keeper (noalias) type BankKeeper interface { SpendableCoins(ctx context.Context, addr sdk.AccAddress) sdk.Coins SendCoinsFromAccountToModule(ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error + IsSendEnabledCoins(ctx context.Context, coins ...sdk.Coin) error + SendCoins(ctx context.Context, from, to sdk.AccAddress, amt sdk.Coins) error } diff --git a/x/feegrant/module/depinject.go b/x/feegrant/module/depinject.go index 3fb6afa68984..18c27254617b 100644 --- a/x/feegrant/module/depinject.go +++ b/x/feegrant/module/depinject.go @@ -1,6 +1,10 @@ package module import ( + "fmt" + + "github.com/spf13/viper" + modulev1 "cosmossdk.io/api/cosmos/feegrant/module/v1" "cosmossdk.io/core/appmodule" "cosmossdk.io/depinject" @@ -11,10 +15,13 @@ import ( "github.com/cosmos/cosmos-sdk/codec" cdctypes "github.com/cosmos/cosmos-sdk/codec/types" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" simtypes "github.com/cosmos/cosmos-sdk/types/simulation" ) +const FlagMinGasPricesV2 = "server.minimum-gas-prices" + var _ depinject.OnePerModuleType = AppModule{} // IsOnePerModuleType implements the depinject.OnePerModuleType interface. @@ -34,11 +41,23 @@ type FeegrantInputs struct { AccountKeeper feegrant.AccountKeeper BankKeeper feegrant.BankKeeper Registry cdctypes.InterfaceRegistry + + Viper *viper.Viper `optional:"true"` // server v2 } func ProvideModule(in FeegrantInputs) (keeper.Keeper, appmodule.AppModule) { k := keeper.NewKeeper(in.Environment, in.Cdc, in.AccountKeeper) m := NewAppModule(in.Cdc, in.AccountKeeper, in.BankKeeper, k, in.Registry) + + if in.Viper != nil { + minGasPricesStr := in.Viper.GetString(FlagMinGasPricesV2) + minGasPrices, err := sdk.ParseDecCoins(minGasPricesStr) + if err != nil { + panic(fmt.Sprintf("invalid minimum gas prices: %v", err)) + } + m.SetMinGasPrices(minGasPrices) + } + return k, m } diff --git a/x/feegrant/module/module.go b/x/feegrant/module/module.go index 134160422723..297f28d95c3e 100644 --- a/x/feegrant/module/module.go +++ b/x/feegrant/module/module.go @@ -10,9 +10,12 @@ import ( "google.golang.org/grpc" "cosmossdk.io/core/appmodule" + appmodulev2 "cosmossdk.io/core/appmodule/v2" "cosmossdk.io/core/legacy" "cosmossdk.io/core/registry" + "cosmossdk.io/core/transaction" "cosmossdk.io/errors" + "cosmossdk.io/x/auth/ante" "cosmossdk.io/x/feegrant" "cosmossdk.io/x/feegrant/client/cli" "cosmossdk.io/x/feegrant/keeper" @@ -20,6 +23,7 @@ import ( sdkclient "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" cdctypes "github.com/cosmos/cosmos-sdk/codec/types" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" ) @@ -43,6 +47,9 @@ type AppModule struct { keeper keeper.Keeper accountKeeper feegrant.AccountKeeper bankKeeper feegrant.BankKeeper + + // need for txValidator + minGasPrices sdk.DecCoins } // NewAppModule creates a new AppModule object @@ -145,6 +152,35 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) return am.cdc.MarshalJSON(gs) } +// SetMinGasPrices sets minimum gas prices in AppModule +func (am *AppModule) SetMinGasPrices(minGasPrices sdk.DecCoins) { + am.minGasPrices = minGasPrices +} + +// TxValidator implements appmodulev2.HasTxValidator. +// It replaces auth ante handlers for server/v2 +func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { + dfd := ante.NewDeductFeeDecorator(am.accountKeeper, am.bankKeeper, + am.keeper, nil) + // set minimum-gas-prices to use in DeductFeeDecorator + ante.SetMinGasPrices(am.minGasPrices) + + validators := []appmodulev2.TxValidator[sdk.Tx]{dfd} + + sdkTx, ok := tx.(sdk.Tx) + if !ok { + return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx) + } + + for _, validator := range validators { + if err := validator.ValidateTx(ctx, sdkTx); err != nil { + return err + } + } + + return nil +} + // ConsensusVersion implements HasConsensusVersion func (AppModule) ConsensusVersion() uint64 { return 2 } diff --git a/x/feegrant/testutil/expected_keepers_mocks.go b/x/feegrant/testutil/expected_keepers_mocks.go index d7567b8266d3..efbe10cf992b 100644 --- a/x/feegrant/testutil/expected_keepers_mocks.go +++ b/x/feegrant/testutil/expected_keepers_mocks.go @@ -9,7 +9,9 @@ import ( reflect "reflect" address "cosmossdk.io/core/address" - types "github.com/cosmos/cosmos-sdk/types" + appmodule "cosmossdk.io/core/appmodule" + types "cosmossdk.io/x/auth/types" + types0 "github.com/cosmos/cosmos-sdk/types" gomock "github.com/golang/mock/gomock" ) @@ -51,10 +53,10 @@ func (mr *MockAccountKeeperMockRecorder) AddressCodec() *gomock.Call { } // GetAccount mocks base method. -func (m *MockAccountKeeper) GetAccount(ctx context.Context, addr types.AccAddress) types.AccountI { +func (m *MockAccountKeeper) GetAccount(ctx context.Context, addr types0.AccAddress) types0.AccountI { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetAccount", ctx, addr) - ret0, _ := ret[0].(types.AccountI) + ret0, _ := ret[0].(types0.AccountI) return ret0 } @@ -64,11 +66,25 @@ func (mr *MockAccountKeeperMockRecorder) GetAccount(ctx, addr interface{}) *gomo return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAccount", reflect.TypeOf((*MockAccountKeeper)(nil).GetAccount), ctx, addr) } +// GetEnvironment mocks base method. +func (m *MockAccountKeeper) GetEnvironment() appmodule.Environment { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetEnvironment") + ret0, _ := ret[0].(appmodule.Environment) + return ret0 +} + +// GetEnvironment indicates an expected call of GetEnvironment. +func (mr *MockAccountKeeperMockRecorder) GetEnvironment() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEnvironment", reflect.TypeOf((*MockAccountKeeper)(nil).GetEnvironment)) +} + // GetModuleAccount mocks base method. -func (m *MockAccountKeeper) GetModuleAccount(ctx context.Context, moduleName string) types.ModuleAccountI { +func (m *MockAccountKeeper) GetModuleAccount(ctx context.Context, moduleName string) types0.ModuleAccountI { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetModuleAccount", ctx, moduleName) - ret0, _ := ret[0].(types.ModuleAccountI) + ret0, _ := ret[0].(types0.ModuleAccountI) return ret0 } @@ -79,10 +95,10 @@ func (mr *MockAccountKeeperMockRecorder) GetModuleAccount(ctx, moduleName interf } // GetModuleAddress mocks base method. -func (m *MockAccountKeeper) GetModuleAddress(moduleName string) types.AccAddress { +func (m *MockAccountKeeper) GetModuleAddress(moduleName string) types0.AccAddress { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetModuleAddress", moduleName) - ret0, _ := ret[0].(types.AccAddress) + ret0, _ := ret[0].(types0.AccAddress) return ret0 } @@ -92,11 +108,25 @@ func (mr *MockAccountKeeperMockRecorder) GetModuleAddress(moduleName interface{} return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetModuleAddress", reflect.TypeOf((*MockAccountKeeper)(nil).GetModuleAddress), moduleName) } +// GetParams mocks base method. +func (m *MockAccountKeeper) GetParams(ctx context.Context) types.Params { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetParams", ctx) + ret0, _ := ret[0].(types.Params) + return ret0 +} + +// GetParams indicates an expected call of GetParams. +func (mr *MockAccountKeeperMockRecorder) GetParams(ctx interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetParams", reflect.TypeOf((*MockAccountKeeper)(nil).GetParams), ctx) +} + // NewAccountWithAddress mocks base method. -func (m *MockAccountKeeper) NewAccountWithAddress(ctx context.Context, addr types.AccAddress) types.AccountI { +func (m *MockAccountKeeper) NewAccountWithAddress(ctx context.Context, addr types0.AccAddress) types0.AccountI { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "NewAccountWithAddress", ctx, addr) - ret0, _ := ret[0].(types.AccountI) + ret0, _ := ret[0].(types0.AccountI) return ret0 } @@ -107,7 +137,7 @@ func (mr *MockAccountKeeperMockRecorder) NewAccountWithAddress(ctx, addr interfa } // SetAccount mocks base method. -func (m *MockAccountKeeper) SetAccount(ctx context.Context, acc types.AccountI) { +func (m *MockAccountKeeper) SetAccount(ctx context.Context, acc types0.AccountI) { m.ctrl.T.Helper() m.ctrl.Call(m, "SetAccount", ctx, acc) } @@ -141,8 +171,41 @@ func (m *MockBankKeeper) EXPECT() *MockBankKeeperMockRecorder { return m.recorder } +// IsSendEnabledCoins mocks base method. +func (m *MockBankKeeper) IsSendEnabledCoins(ctx context.Context, coins ...types0.Coin) error { + m.ctrl.T.Helper() + varargs := []interface{}{ctx} + for _, a := range coins { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "IsSendEnabledCoins", varargs...) + ret0, _ := ret[0].(error) + return ret0 +} + +// IsSendEnabledCoins indicates an expected call of IsSendEnabledCoins. +func (mr *MockBankKeeperMockRecorder) IsSendEnabledCoins(ctx interface{}, coins ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{ctx}, coins...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsSendEnabledCoins", reflect.TypeOf((*MockBankKeeper)(nil).IsSendEnabledCoins), varargs...) +} + +// SendCoins mocks base method. +func (m *MockBankKeeper) SendCoins(ctx context.Context, from, to types0.AccAddress, amt types0.Coins) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SendCoins", ctx, from, to, amt) + ret0, _ := ret[0].(error) + return ret0 +} + +// SendCoins indicates an expected call of SendCoins. +func (mr *MockBankKeeperMockRecorder) SendCoins(ctx, from, to, amt interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendCoins", reflect.TypeOf((*MockBankKeeper)(nil).SendCoins), ctx, from, to, amt) +} + // SendCoinsFromAccountToModule mocks base method. -func (m *MockBankKeeper) SendCoinsFromAccountToModule(ctx context.Context, senderAddr types.AccAddress, recipientModule string, amt types.Coins) error { +func (m *MockBankKeeper) SendCoinsFromAccountToModule(ctx context.Context, senderAddr types0.AccAddress, recipientModule string, amt types0.Coins) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SendCoinsFromAccountToModule", ctx, senderAddr, recipientModule, amt) ret0, _ := ret[0].(error) @@ -156,10 +219,10 @@ func (mr *MockBankKeeperMockRecorder) SendCoinsFromAccountToModule(ctx, senderAd } // SpendableCoins mocks base method. -func (m *MockBankKeeper) SpendableCoins(ctx context.Context, addr types.AccAddress) types.Coins { +func (m *MockBankKeeper) SpendableCoins(ctx context.Context, addr types0.AccAddress) types0.Coins { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SpendableCoins", ctx, addr) - ret0, _ := ret[0].(types.Coins) + ret0, _ := ret[0].(types0.Coins) return ret0 } From 2bdda6215bb20417abef38839ff43e10bdb30398 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Mon, 12 Aug 2024 14:45:45 +0530 Subject: [PATCH 11/28] revert change and comment --- x/auth/ante/fee.go | 1 - x/auth/depinject.go | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index 7e7de75bebca..237654698d20 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -97,7 +97,6 @@ func (dfd DeductFeeDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error { return errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") } - // update global fields values globalFields.txFee = globalFields.feeTx.GetFee() var err error diff --git a/x/auth/depinject.go b/x/auth/depinject.go index f11a3bb57538..834912a4de30 100644 --- a/x/auth/depinject.go +++ b/x/auth/depinject.go @@ -72,5 +72,6 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { k := keeper.NewAccountKeeper(in.Environment, in.Cdc, in.AccountI, in.AccountsModKeeper, maccPerms, in.AddressCodec, in.Config.Bech32Prefix, auth) m := NewAppModule(in.Cdc, k, in.AccountsModKeeper, in.RandomGenesisAccountsFn) + return ModuleOutputs{AccountKeeper: k, Module: m} } From 84fed9ebc04d135ae9dc9c04592ac1a36c785e28 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Mon, 12 Aug 2024 16:29:00 +0530 Subject: [PATCH 12/28] WIP: return feeTxValidator from auth/tx and revert feegrant changes --- x/auth/ante/fee.go | 61 ++++++++----- x/auth/ante/validator_tx_fee.go | 8 +- x/auth/depinject.go | 22 +++++ x/auth/module.go | 20 +++++ x/auth/tx/config/depinject.go | 14 ++- x/feegrant/expected_keepers.go | 6 -- x/feegrant/module/depinject.go | 19 ---- x/feegrant/module/module.go | 36 -------- x/feegrant/testutil/expected_keepers_mocks.go | 89 +++---------------- 9 files changed, 109 insertions(+), 166 deletions(-) diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index 237654698d20..28a4200d75a6 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -9,6 +9,7 @@ import ( errorsmod "cosmossdk.io/errors" "cosmossdk.io/x/auth/types" + appmodulev2 "cosmossdk.io/core/appmodule/v2" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) @@ -17,6 +18,14 @@ import ( // The effective fee should be deducted later, and the priority should be returned in the ABCI response. type TxFeeChecker func(ctx context.Context, tx sdk.Tx) (sdk.Coins, int64, error) +// FeeTxValidator defines custom type used to represent deduct fee decorator +// which will be passed from x/auth/tx to x/auth module. +type FeeTxValidator interface { + appmodulev2.TxValidator[sdk.Tx] + + SetMinGasPrices(sdk.DecCoins) +} + // DeductFeeDecorator deducts fees from the fee payer. The fee payer is the fee granter (if specified) or first signer of the tx. // If the fee payer does not have the funds to pay for the fees, return an InsufficientFunds error. // Call next AnteHandler if fees are successfully deducted. @@ -26,10 +35,13 @@ type DeductFeeDecorator struct { bankKeeper types.BankKeeper feegrantKeeper FeegrantKeeper txFeeChecker TxFeeChecker + + // pointer to a separate state struct + state *deductFeeState } -// store below fields globally to use in different methods -type dfdGlobalFields struct { +// deductFeeState holds the mutable state needed across method calls +type deductFeeState struct { minGasPrices sdk.DecCoins feeTx sdk.FeeTx txPriority int64 @@ -38,14 +50,13 @@ type dfdGlobalFields struct { execMode transaction.ExecMode } -var globalFields = dfdGlobalFields{} - func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper, tfc TxFeeChecker) DeductFeeDecorator { dfd := DeductFeeDecorator{ accountKeeper: ak, bankKeeper: bk, feegrantKeeper: fk, txFeeChecker: tfc, + state: &deductFeeState{}, // Initialize the state } if tfc == nil { @@ -55,30 +66,30 @@ func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKee return dfd } -// SetMinGasPrices sets the minimum-gas-prices value in global fields of DeductFeeDecorator -func SetMinGasPrices(minGasPrices sdk.DecCoins) { - globalFields.minGasPrices = minGasPrices +// SetMinGasPrices sets the minimum-gas-prices value in the state of DeductFeeDecorator +func (dfd DeductFeeDecorator) SetMinGasPrices(minGasPrices sdk.DecCoins) { + dfd.state.minGasPrices = minGasPrices } // AnteHandle implements an AnteHandler decorator for the DeductFeeDecorator func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (sdk.Context, error) { - globalFields.minGasPrices = ctx.MinGasPrices() + dfd.state.minGasPrices = ctx.MinGasPrices() if err := dfd.ValidateTx(ctx, tx); err != nil { return ctx, err } - // TODO: emit this event in v2 after executing ValidateTx + // TODO: emit this event in v2 after executing ValidateTx method events := sdk.Events{ sdk.NewEvent( sdk.EventTypeTx, - sdk.NewAttribute(sdk.AttributeKeyFee, globalFields.txFee.String()), - sdk.NewAttribute(sdk.AttributeKeyFeePayer, sdk.AccAddress(globalFields.deductFeesFrom).String()), + sdk.NewAttribute(sdk.AttributeKeyFee, dfd.state.txFee.String()), + sdk.NewAttribute(sdk.AttributeKeyFeePayer, sdk.AccAddress(dfd.state.deductFeesFrom).String()), ), } ctx.EventManager().EmitEvents(events) - newCtx := ctx.WithPriority(globalFields.txPriority) + newCtx := ctx.WithPriority(dfd.state.txPriority) return next(newCtx, tx, false) } @@ -88,27 +99,29 @@ func (dfd DeductFeeDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error { if !ok { return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must implement the FeeTx interface") } - globalFields.feeTx = feeTx - globalFields.execMode = dfd.accountKeeper.GetEnvironment().TransactionService.ExecMode(ctx) + // update the state with the current transaction + dfd.state.feeTx = feeTx + + dfd.state.execMode = dfd.accountKeeper.GetEnvironment().TransactionService.ExecMode(ctx) headerInfo := dfd.accountKeeper.GetEnvironment().HeaderService.HeaderInfo(ctx) - if globalFields.execMode != transaction.ExecModeSimulate && headerInfo.Height > 0 && globalFields.feeTx.GetGas() == 0 { + if dfd.state.execMode != transaction.ExecModeSimulate && headerInfo.Height > 0 && dfd.state.feeTx.GetGas() == 0 { return errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") } - globalFields.txFee = globalFields.feeTx.GetFee() + dfd.state.txFee = dfd.state.feeTx.GetFee() var err error - if globalFields.execMode != transaction.ExecModeSimulate { - globalFields.txFee, globalFields.txPriority, err = dfd.txFeeChecker(ctx, tx) + if dfd.state.execMode != transaction.ExecModeSimulate { + dfd.state.txFee, dfd.state.txPriority, err = dfd.txFeeChecker(ctx, tx) if err != nil { return err } } - if err := dfd.checkDeductFee(ctx, tx, globalFields.txFee); err != nil { + if err := dfd.checkDeductFee(ctx, tx, dfd.state.txFee); err != nil { return err } @@ -121,9 +134,9 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx context.Context, sdkTx sdk.Tx, return fmt.Errorf("fee collector module account (%s) has not been set", types.FeeCollectorName) } - feePayer := globalFields.feeTx.FeePayer() - feeGranter := globalFields.feeTx.FeeGranter() - globalFields.deductFeesFrom = feePayer + feePayer := dfd.state.feeTx.FeePayer() + feeGranter := dfd.state.feeTx.FeeGranter() + dfd.state.deductFeesFrom = feePayer // if feegranter set, deduct fee from feegranter account. // this works only when feegrant is enabled. @@ -147,12 +160,12 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx context.Context, sdkTx sdk.Tx, } } - globalFields.deductFeesFrom = feeGranterAddr + dfd.state.deductFeesFrom = feeGranterAddr } // deduct the fees if !fee.IsZero() { - err := DeductFees(dfd.bankKeeper, ctx, globalFields.deductFeesFrom, fee) + err := DeductFees(dfd.bankKeeper, ctx, dfd.state.deductFeesFrom, fee) if err != nil { return err } diff --git a/x/auth/ante/validator_tx_fee.go b/x/auth/ante/validator_tx_fee.go index 886feca0c691..7733c12d3f5c 100644 --- a/x/auth/ante/validator_tx_fee.go +++ b/x/auth/ante/validator_tx_fee.go @@ -15,14 +15,14 @@ import ( // checkTxFeeWithValidatorMinGasPrices implements the default fee logic, where the minimum price per // unit of gas is fixed and set by each validator, can the tx priority is computed from the gas price. func (dfd DeductFeeDecorator) checkTxFeeWithValidatorMinGasPrices(ctx context.Context, _ sdk.Tx) (sdk.Coins, int64, error) { - feeCoins := globalFields.feeTx.GetFee() - gas := globalFields.feeTx.GetGas() + feeCoins := dfd.state.feeTx.GetFee() + gas := dfd.state.feeTx.GetGas() // Ensure that the provided fees meet a minimum threshold for the validator, // if this is a CheckTx. This is only for local mempool purposes, and thus // is only ran on check tx. - if globalFields.execMode == transaction.ExecModeCheck { - minGasPrices := globalFields.minGasPrices + if dfd.state.execMode == transaction.ExecModeCheck { + minGasPrices := dfd.state.minGasPrices if !minGasPrices.IsZero() { requiredFees := make(sdk.Coins, len(minGasPrices)) diff --git a/x/auth/depinject.go b/x/auth/depinject.go index 834912a4de30..c24a6854f10f 100644 --- a/x/auth/depinject.go +++ b/x/auth/depinject.go @@ -1,19 +1,25 @@ package auth import ( + "fmt" + modulev1 "cosmossdk.io/api/cosmos/auth/module/v1" "cosmossdk.io/core/address" "cosmossdk.io/core/appmodule" "cosmossdk.io/depinject" "cosmossdk.io/depinject/appconfig" + "cosmossdk.io/x/auth/ante" "cosmossdk.io/x/auth/keeper" "cosmossdk.io/x/auth/simulation" "cosmossdk.io/x/auth/types" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/spf13/viper" ) +const FlagMinGasPricesV2 = "server.minimum-gas-prices" + var _ depinject.OnePerModuleType = AppModule{} // IsOnePerModuleType implements the depinject.OnePerModuleType interface. @@ -36,6 +42,8 @@ type ModuleInputs struct { AddressCodec address.Codec RandomGenesisAccountsFn types.RandomGenesisAccountsFn `optional:"true"` AccountI func() sdk.AccountI `optional:"true"` + Viper *viper.Viper `optional:"true"` // server v2 + FeeTxValidator ante.FeeTxValidator `optional:"true"` // server v2 } type ModuleOutputs struct { @@ -73,5 +81,19 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { k := keeper.NewAccountKeeper(in.Environment, in.Cdc, in.AccountI, in.AccountsModKeeper, maccPerms, in.AddressCodec, in.Config.Bech32Prefix, auth) m := NewAppModule(in.Cdc, k, in.AccountsModKeeper, in.RandomGenesisAccountsFn) + if in.Viper != nil { + minGasPricesStr := in.Viper.GetString(FlagMinGasPricesV2) + minGasPrices, err := sdk.ParseDecCoins(minGasPricesStr) + if err != nil { + panic(fmt.Sprintf("invalid minimum gas prices: %v", err)) + } + m.SetMinGasPrices(minGasPrices) + } + + // set feeTxValidator if found + if in.FeeTxValidator != nil { + m.SetFeeTxValidator(in.FeeTxValidator) + } + return ModuleOutputs{AccountKeeper: k, Module: m} } diff --git a/x/auth/module.go b/x/auth/module.go index ae028ab5fcda..9abc206e3f8d 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -45,6 +45,10 @@ type AppModule struct { randGenAccountsFn types.RandomGenesisAccountsFn accountsModKeeper types.AccountsModKeeper cdc codec.Codec + + // deduct fee v2 tx validator + feeTxValidator ante.FeeTxValidator + minGasPrices sdk.DecCoins } // IsAppModule implements the appmodule.AppModule interface. @@ -150,6 +154,16 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) return am.cdc.MarshalJSON(gs) } +// SetFeeTxValidator sets feeTxValidator in AppModule which is used for deducting fee +func (am *AppModule) SetFeeTxValidator(validator ante.FeeTxValidator) { + am.feeTxValidator = validator +} + +// SetMinGasPrices sets minimum gas prices in AppModule +func (am *AppModule) SetMinGasPrices(minGasPrices sdk.DecCoins) { + am.minGasPrices = minGasPrices +} + // TxValidator implements appmodulev2.HasTxValidator. // It replaces auth ante handlers for server/v2 func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { @@ -161,6 +175,12 @@ func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { ante.NewValidateSigCountDecorator(am.accountKeeper), } + if am.feeTxValidator != nil { + // set minimum-gas-prices to use in DeductFeeDecorator + am.feeTxValidator.SetMinGasPrices(am.minGasPrices) + validators = append(validators, am.feeTxValidator) + } + sdkTx, ok := tx.(sdk.Tx) if !ok { return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx) diff --git a/x/auth/tx/config/depinject.go b/x/auth/tx/config/depinject.go index d85c2921fc8c..d3942d751006 100644 --- a/x/auth/tx/config/depinject.go +++ b/x/auth/tx/config/depinject.go @@ -63,6 +63,7 @@ type ModuleOutputs struct { TxConfig client.TxConfig TxConfigOptions tx.ConfigOptions BaseAppOption runtime.BaseAppOption // This is only useful for chains using baseapp. Server/v2 chains use TxValidator. + FeeTxValidator ante.FeeTxValidator // pass deduct fee decorator to auth TxValidator } func ProvideProtoRegistry() txsigning.ProtoFileResolver { @@ -140,7 +141,18 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { app.SetTxEncoder(txConfig.TxEncoder()) } - return ModuleOutputs{TxConfig: txConfig, TxConfigOptions: txConfigOptions, BaseAppOption: baseAppOption} + var feeTxValidator ante.FeeTxValidator + // check required keepers' exists and return deduct fee decorator + if in.AccountKeeper != nil && in.BankKeeper != nil && in.FeeGrantKeeper != nil { + feeTxValidator = ante.NewDeductFeeDecorator(in.AccountKeeper, in.BankKeeper, in.FeeGrantKeeper, nil) + } + + return ModuleOutputs{ + TxConfig: txConfig, + TxConfigOptions: txConfigOptions, + BaseAppOption: baseAppOption, + FeeTxValidator: feeTxValidator, + } } func newAnteHandler(txConfig client.TxConfig, in ModuleInputs) (sdk.AnteHandler, error) { diff --git a/x/feegrant/expected_keepers.go b/x/feegrant/expected_keepers.go index addc4bdf11d7..ce6a2d0a5698 100644 --- a/x/feegrant/expected_keepers.go +++ b/x/feegrant/expected_keepers.go @@ -4,8 +4,6 @@ import ( "context" "cosmossdk.io/core/address" - "cosmossdk.io/core/appmodule" - authtypes "cosmossdk.io/x/auth/types" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -20,14 +18,10 @@ type AccountKeeper interface { NewAccountWithAddress(ctx context.Context, addr sdk.AccAddress) sdk.AccountI GetAccount(ctx context.Context, addr sdk.AccAddress) sdk.AccountI SetAccount(ctx context.Context, acc sdk.AccountI) - GetParams(ctx context.Context) (params authtypes.Params) - GetEnvironment() appmodule.Environment } // BankKeeper defines the expected supply Keeper (noalias) type BankKeeper interface { SpendableCoins(ctx context.Context, addr sdk.AccAddress) sdk.Coins SendCoinsFromAccountToModule(ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error - IsSendEnabledCoins(ctx context.Context, coins ...sdk.Coin) error - SendCoins(ctx context.Context, from, to sdk.AccAddress, amt sdk.Coins) error } diff --git a/x/feegrant/module/depinject.go b/x/feegrant/module/depinject.go index 18c27254617b..3fb6afa68984 100644 --- a/x/feegrant/module/depinject.go +++ b/x/feegrant/module/depinject.go @@ -1,10 +1,6 @@ package module import ( - "fmt" - - "github.com/spf13/viper" - modulev1 "cosmossdk.io/api/cosmos/feegrant/module/v1" "cosmossdk.io/core/appmodule" "cosmossdk.io/depinject" @@ -15,13 +11,10 @@ import ( "github.com/cosmos/cosmos-sdk/codec" cdctypes "github.com/cosmos/cosmos-sdk/codec/types" - sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" simtypes "github.com/cosmos/cosmos-sdk/types/simulation" ) -const FlagMinGasPricesV2 = "server.minimum-gas-prices" - var _ depinject.OnePerModuleType = AppModule{} // IsOnePerModuleType implements the depinject.OnePerModuleType interface. @@ -41,23 +34,11 @@ type FeegrantInputs struct { AccountKeeper feegrant.AccountKeeper BankKeeper feegrant.BankKeeper Registry cdctypes.InterfaceRegistry - - Viper *viper.Viper `optional:"true"` // server v2 } func ProvideModule(in FeegrantInputs) (keeper.Keeper, appmodule.AppModule) { k := keeper.NewKeeper(in.Environment, in.Cdc, in.AccountKeeper) m := NewAppModule(in.Cdc, in.AccountKeeper, in.BankKeeper, k, in.Registry) - - if in.Viper != nil { - minGasPricesStr := in.Viper.GetString(FlagMinGasPricesV2) - minGasPrices, err := sdk.ParseDecCoins(minGasPricesStr) - if err != nil { - panic(fmt.Sprintf("invalid minimum gas prices: %v", err)) - } - m.SetMinGasPrices(minGasPrices) - } - return k, m } diff --git a/x/feegrant/module/module.go b/x/feegrant/module/module.go index 297f28d95c3e..134160422723 100644 --- a/x/feegrant/module/module.go +++ b/x/feegrant/module/module.go @@ -10,12 +10,9 @@ import ( "google.golang.org/grpc" "cosmossdk.io/core/appmodule" - appmodulev2 "cosmossdk.io/core/appmodule/v2" "cosmossdk.io/core/legacy" "cosmossdk.io/core/registry" - "cosmossdk.io/core/transaction" "cosmossdk.io/errors" - "cosmossdk.io/x/auth/ante" "cosmossdk.io/x/feegrant" "cosmossdk.io/x/feegrant/client/cli" "cosmossdk.io/x/feegrant/keeper" @@ -23,7 +20,6 @@ import ( sdkclient "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" cdctypes "github.com/cosmos/cosmos-sdk/codec/types" - sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" ) @@ -47,9 +43,6 @@ type AppModule struct { keeper keeper.Keeper accountKeeper feegrant.AccountKeeper bankKeeper feegrant.BankKeeper - - // need for txValidator - minGasPrices sdk.DecCoins } // NewAppModule creates a new AppModule object @@ -152,35 +145,6 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) return am.cdc.MarshalJSON(gs) } -// SetMinGasPrices sets minimum gas prices in AppModule -func (am *AppModule) SetMinGasPrices(minGasPrices sdk.DecCoins) { - am.minGasPrices = minGasPrices -} - -// TxValidator implements appmodulev2.HasTxValidator. -// It replaces auth ante handlers for server/v2 -func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { - dfd := ante.NewDeductFeeDecorator(am.accountKeeper, am.bankKeeper, - am.keeper, nil) - // set minimum-gas-prices to use in DeductFeeDecorator - ante.SetMinGasPrices(am.minGasPrices) - - validators := []appmodulev2.TxValidator[sdk.Tx]{dfd} - - sdkTx, ok := tx.(sdk.Tx) - if !ok { - return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx) - } - - for _, validator := range validators { - if err := validator.ValidateTx(ctx, sdkTx); err != nil { - return err - } - } - - return nil -} - // ConsensusVersion implements HasConsensusVersion func (AppModule) ConsensusVersion() uint64 { return 2 } diff --git a/x/feegrant/testutil/expected_keepers_mocks.go b/x/feegrant/testutil/expected_keepers_mocks.go index efbe10cf992b..d7567b8266d3 100644 --- a/x/feegrant/testutil/expected_keepers_mocks.go +++ b/x/feegrant/testutil/expected_keepers_mocks.go @@ -9,9 +9,7 @@ import ( reflect "reflect" address "cosmossdk.io/core/address" - appmodule "cosmossdk.io/core/appmodule" - types "cosmossdk.io/x/auth/types" - types0 "github.com/cosmos/cosmos-sdk/types" + types "github.com/cosmos/cosmos-sdk/types" gomock "github.com/golang/mock/gomock" ) @@ -53,10 +51,10 @@ func (mr *MockAccountKeeperMockRecorder) AddressCodec() *gomock.Call { } // GetAccount mocks base method. -func (m *MockAccountKeeper) GetAccount(ctx context.Context, addr types0.AccAddress) types0.AccountI { +func (m *MockAccountKeeper) GetAccount(ctx context.Context, addr types.AccAddress) types.AccountI { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetAccount", ctx, addr) - ret0, _ := ret[0].(types0.AccountI) + ret0, _ := ret[0].(types.AccountI) return ret0 } @@ -66,25 +64,11 @@ func (mr *MockAccountKeeperMockRecorder) GetAccount(ctx, addr interface{}) *gomo return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAccount", reflect.TypeOf((*MockAccountKeeper)(nil).GetAccount), ctx, addr) } -// GetEnvironment mocks base method. -func (m *MockAccountKeeper) GetEnvironment() appmodule.Environment { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetEnvironment") - ret0, _ := ret[0].(appmodule.Environment) - return ret0 -} - -// GetEnvironment indicates an expected call of GetEnvironment. -func (mr *MockAccountKeeperMockRecorder) GetEnvironment() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEnvironment", reflect.TypeOf((*MockAccountKeeper)(nil).GetEnvironment)) -} - // GetModuleAccount mocks base method. -func (m *MockAccountKeeper) GetModuleAccount(ctx context.Context, moduleName string) types0.ModuleAccountI { +func (m *MockAccountKeeper) GetModuleAccount(ctx context.Context, moduleName string) types.ModuleAccountI { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetModuleAccount", ctx, moduleName) - ret0, _ := ret[0].(types0.ModuleAccountI) + ret0, _ := ret[0].(types.ModuleAccountI) return ret0 } @@ -95,10 +79,10 @@ func (mr *MockAccountKeeperMockRecorder) GetModuleAccount(ctx, moduleName interf } // GetModuleAddress mocks base method. -func (m *MockAccountKeeper) GetModuleAddress(moduleName string) types0.AccAddress { +func (m *MockAccountKeeper) GetModuleAddress(moduleName string) types.AccAddress { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetModuleAddress", moduleName) - ret0, _ := ret[0].(types0.AccAddress) + ret0, _ := ret[0].(types.AccAddress) return ret0 } @@ -108,25 +92,11 @@ func (mr *MockAccountKeeperMockRecorder) GetModuleAddress(moduleName interface{} return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetModuleAddress", reflect.TypeOf((*MockAccountKeeper)(nil).GetModuleAddress), moduleName) } -// GetParams mocks base method. -func (m *MockAccountKeeper) GetParams(ctx context.Context) types.Params { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetParams", ctx) - ret0, _ := ret[0].(types.Params) - return ret0 -} - -// GetParams indicates an expected call of GetParams. -func (mr *MockAccountKeeperMockRecorder) GetParams(ctx interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetParams", reflect.TypeOf((*MockAccountKeeper)(nil).GetParams), ctx) -} - // NewAccountWithAddress mocks base method. -func (m *MockAccountKeeper) NewAccountWithAddress(ctx context.Context, addr types0.AccAddress) types0.AccountI { +func (m *MockAccountKeeper) NewAccountWithAddress(ctx context.Context, addr types.AccAddress) types.AccountI { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "NewAccountWithAddress", ctx, addr) - ret0, _ := ret[0].(types0.AccountI) + ret0, _ := ret[0].(types.AccountI) return ret0 } @@ -137,7 +107,7 @@ func (mr *MockAccountKeeperMockRecorder) NewAccountWithAddress(ctx, addr interfa } // SetAccount mocks base method. -func (m *MockAccountKeeper) SetAccount(ctx context.Context, acc types0.AccountI) { +func (m *MockAccountKeeper) SetAccount(ctx context.Context, acc types.AccountI) { m.ctrl.T.Helper() m.ctrl.Call(m, "SetAccount", ctx, acc) } @@ -171,41 +141,8 @@ func (m *MockBankKeeper) EXPECT() *MockBankKeeperMockRecorder { return m.recorder } -// IsSendEnabledCoins mocks base method. -func (m *MockBankKeeper) IsSendEnabledCoins(ctx context.Context, coins ...types0.Coin) error { - m.ctrl.T.Helper() - varargs := []interface{}{ctx} - for _, a := range coins { - varargs = append(varargs, a) - } - ret := m.ctrl.Call(m, "IsSendEnabledCoins", varargs...) - ret0, _ := ret[0].(error) - return ret0 -} - -// IsSendEnabledCoins indicates an expected call of IsSendEnabledCoins. -func (mr *MockBankKeeperMockRecorder) IsSendEnabledCoins(ctx interface{}, coins ...interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - varargs := append([]interface{}{ctx}, coins...) - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsSendEnabledCoins", reflect.TypeOf((*MockBankKeeper)(nil).IsSendEnabledCoins), varargs...) -} - -// SendCoins mocks base method. -func (m *MockBankKeeper) SendCoins(ctx context.Context, from, to types0.AccAddress, amt types0.Coins) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "SendCoins", ctx, from, to, amt) - ret0, _ := ret[0].(error) - return ret0 -} - -// SendCoins indicates an expected call of SendCoins. -func (mr *MockBankKeeperMockRecorder) SendCoins(ctx, from, to, amt interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SendCoins", reflect.TypeOf((*MockBankKeeper)(nil).SendCoins), ctx, from, to, amt) -} - // SendCoinsFromAccountToModule mocks base method. -func (m *MockBankKeeper) SendCoinsFromAccountToModule(ctx context.Context, senderAddr types0.AccAddress, recipientModule string, amt types0.Coins) error { +func (m *MockBankKeeper) SendCoinsFromAccountToModule(ctx context.Context, senderAddr types.AccAddress, recipientModule string, amt types.Coins) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SendCoinsFromAccountToModule", ctx, senderAddr, recipientModule, amt) ret0, _ := ret[0].(error) @@ -219,10 +156,10 @@ func (mr *MockBankKeeperMockRecorder) SendCoinsFromAccountToModule(ctx, senderAd } // SpendableCoins mocks base method. -func (m *MockBankKeeper) SpendableCoins(ctx context.Context, addr types0.AccAddress) types0.Coins { +func (m *MockBankKeeper) SpendableCoins(ctx context.Context, addr types.AccAddress) types.Coins { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SpendableCoins", ctx, addr) - ret0, _ := ret[0].(types0.Coins) + ret0, _ := ret[0].(types.Coins) return ret0 } From dc59cae0256014edebdd015ad062cb170bb4208b Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Tue, 20 Aug 2024 15:45:33 +0530 Subject: [PATCH 13/28] update approach --- x/auth/ante/fee.go | 7 +++ x/auth/depinject.go | 22 ---------- x/auth/module.go | 20 --------- x/auth/tx/config/depinject.go | 14 +----- x/bank/depinject.go | 52 +++++++++++++++++++++-- x/bank/module.go | 39 +++++++++++++++++ x/bank/testutil/expected_keepers_mocks.go | 29 +++++++++++++ x/bank/types/expected_keepers.go | 3 ++ x/feegrant/module/depinject.go | 9 ++++ x/feegrant/module/module.go | 33 ++++++++++++++ 10 files changed, 169 insertions(+), 59 deletions(-) diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index 28a4200d75a6..05549b751f32 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -24,6 +24,7 @@ type FeeTxValidator interface { appmodulev2.TxValidator[sdk.Tx] SetMinGasPrices(sdk.DecCoins) + SetFeegrantKeeper(FeegrantKeeper) FeeTxValidator } // DeductFeeDecorator deducts fees from the fee payer. The fee payer is the fee granter (if specified) or first signer of the tx. @@ -71,6 +72,12 @@ func (dfd DeductFeeDecorator) SetMinGasPrices(minGasPrices sdk.DecCoins) { dfd.state.minGasPrices = minGasPrices } +// SetFeegrantKeeper sets the feegrant keeper in DeductFeeDecorator +func (dfd DeductFeeDecorator) SetFeegrantKeeper(feegrantKeeper FeegrantKeeper) FeeTxValidator { + dfd.feegrantKeeper = feegrantKeeper + return dfd +} + // AnteHandle implements an AnteHandler decorator for the DeductFeeDecorator func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (sdk.Context, error) { dfd.state.minGasPrices = ctx.MinGasPrices() diff --git a/x/auth/depinject.go b/x/auth/depinject.go index c24a6854f10f..834912a4de30 100644 --- a/x/auth/depinject.go +++ b/x/auth/depinject.go @@ -1,25 +1,19 @@ package auth import ( - "fmt" - modulev1 "cosmossdk.io/api/cosmos/auth/module/v1" "cosmossdk.io/core/address" "cosmossdk.io/core/appmodule" "cosmossdk.io/depinject" "cosmossdk.io/depinject/appconfig" - "cosmossdk.io/x/auth/ante" "cosmossdk.io/x/auth/keeper" "cosmossdk.io/x/auth/simulation" "cosmossdk.io/x/auth/types" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/spf13/viper" ) -const FlagMinGasPricesV2 = "server.minimum-gas-prices" - var _ depinject.OnePerModuleType = AppModule{} // IsOnePerModuleType implements the depinject.OnePerModuleType interface. @@ -42,8 +36,6 @@ type ModuleInputs struct { AddressCodec address.Codec RandomGenesisAccountsFn types.RandomGenesisAccountsFn `optional:"true"` AccountI func() sdk.AccountI `optional:"true"` - Viper *viper.Viper `optional:"true"` // server v2 - FeeTxValidator ante.FeeTxValidator `optional:"true"` // server v2 } type ModuleOutputs struct { @@ -81,19 +73,5 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { k := keeper.NewAccountKeeper(in.Environment, in.Cdc, in.AccountI, in.AccountsModKeeper, maccPerms, in.AddressCodec, in.Config.Bech32Prefix, auth) m := NewAppModule(in.Cdc, k, in.AccountsModKeeper, in.RandomGenesisAccountsFn) - if in.Viper != nil { - minGasPricesStr := in.Viper.GetString(FlagMinGasPricesV2) - minGasPrices, err := sdk.ParseDecCoins(minGasPricesStr) - if err != nil { - panic(fmt.Sprintf("invalid minimum gas prices: %v", err)) - } - m.SetMinGasPrices(minGasPrices) - } - - // set feeTxValidator if found - if in.FeeTxValidator != nil { - m.SetFeeTxValidator(in.FeeTxValidator) - } - return ModuleOutputs{AccountKeeper: k, Module: m} } diff --git a/x/auth/module.go b/x/auth/module.go index 9abc206e3f8d..ae028ab5fcda 100644 --- a/x/auth/module.go +++ b/x/auth/module.go @@ -45,10 +45,6 @@ type AppModule struct { randGenAccountsFn types.RandomGenesisAccountsFn accountsModKeeper types.AccountsModKeeper cdc codec.Codec - - // deduct fee v2 tx validator - feeTxValidator ante.FeeTxValidator - minGasPrices sdk.DecCoins } // IsAppModule implements the appmodule.AppModule interface. @@ -154,16 +150,6 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) return am.cdc.MarshalJSON(gs) } -// SetFeeTxValidator sets feeTxValidator in AppModule which is used for deducting fee -func (am *AppModule) SetFeeTxValidator(validator ante.FeeTxValidator) { - am.feeTxValidator = validator -} - -// SetMinGasPrices sets minimum gas prices in AppModule -func (am *AppModule) SetMinGasPrices(minGasPrices sdk.DecCoins) { - am.minGasPrices = minGasPrices -} - // TxValidator implements appmodulev2.HasTxValidator. // It replaces auth ante handlers for server/v2 func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { @@ -175,12 +161,6 @@ func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { ante.NewValidateSigCountDecorator(am.accountKeeper), } - if am.feeTxValidator != nil { - // set minimum-gas-prices to use in DeductFeeDecorator - am.feeTxValidator.SetMinGasPrices(am.minGasPrices) - validators = append(validators, am.feeTxValidator) - } - sdkTx, ok := tx.(sdk.Tx) if !ok { return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx) diff --git a/x/auth/tx/config/depinject.go b/x/auth/tx/config/depinject.go index d3942d751006..d85c2921fc8c 100644 --- a/x/auth/tx/config/depinject.go +++ b/x/auth/tx/config/depinject.go @@ -63,7 +63,6 @@ type ModuleOutputs struct { TxConfig client.TxConfig TxConfigOptions tx.ConfigOptions BaseAppOption runtime.BaseAppOption // This is only useful for chains using baseapp. Server/v2 chains use TxValidator. - FeeTxValidator ante.FeeTxValidator // pass deduct fee decorator to auth TxValidator } func ProvideProtoRegistry() txsigning.ProtoFileResolver { @@ -141,18 +140,7 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { app.SetTxEncoder(txConfig.TxEncoder()) } - var feeTxValidator ante.FeeTxValidator - // check required keepers' exists and return deduct fee decorator - if in.AccountKeeper != nil && in.BankKeeper != nil && in.FeeGrantKeeper != nil { - feeTxValidator = ante.NewDeductFeeDecorator(in.AccountKeeper, in.BankKeeper, in.FeeGrantKeeper, nil) - } - - return ModuleOutputs{ - TxConfig: txConfig, - TxConfigOptions: txConfigOptions, - BaseAppOption: baseAppOption, - FeeTxValidator: feeTxValidator, - } + return ModuleOutputs{TxConfig: txConfig, TxConfigOptions: txConfigOptions, BaseAppOption: baseAppOption} } func newAnteHandler(txConfig client.TxConfig, in ModuleInputs) (sdk.AnteHandler, error) { diff --git a/x/bank/depinject.go b/x/bank/depinject.go index 1d6f7dd3e533..3fac5aa95c83 100644 --- a/x/bank/depinject.go +++ b/x/bank/depinject.go @@ -10,11 +10,19 @@ import ( "cosmossdk.io/core/appmodule" "cosmossdk.io/depinject" "cosmossdk.io/depinject/appconfig" + "cosmossdk.io/x/auth/ante" authtypes "cosmossdk.io/x/auth/types" "cosmossdk.io/x/bank/keeper" "cosmossdk.io/x/bank/types" "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/spf13/viper" +) + +const ( + FlagMinGasPricesV2 = "server.minimum-gas-prices" + feegrantModuleName = "feegrant" ) var _ depinject.OnePerModuleType = AppModule{} @@ -26,7 +34,7 @@ func init() { appconfig.RegisterModule( &modulev1.Module{}, appconfig.Provide(ProvideModule), - appconfig.Invoke(InvokeSetSendRestrictions), + appconfig.Invoke(InvokeSetSendRestrictions, InvokeCheckFeeGrantPresent), ) } @@ -38,13 +46,15 @@ type ModuleInputs struct { Environment appmodule.Environment AccountKeeper types.AccountKeeper + Viper *viper.Viper `optional:"true"` // server v2 } type ModuleOutputs struct { depinject.Out - BankKeeper keeper.BaseKeeper - Module appmodule.AppModule + BankKeeper keeper.BaseKeeper + Module appmodule.AppModule + FeeTxValidator ante.FeeTxValidator // pass deduct fee decorator to feegrant TxValidator } func ProvideModule(in ModuleInputs) ModuleOutputs { @@ -91,7 +101,25 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { ) m := NewAppModule(in.Cdc, bankKeeper, in.AccountKeeper) - return ModuleOutputs{BankKeeper: bankKeeper, Module: m} + var minGasPrices sdk.DecCoins + if in.Viper != nil { + minGasPricesStr := in.Viper.GetString(FlagMinGasPricesV2) + minGasPrices, err = sdk.ParseDecCoins(minGasPricesStr) + if err != nil { + panic(fmt.Sprintf("invalid minimum gas prices: %v", err)) + } + } + + var feeTxValidator ante.FeeTxValidator + if in.AccountKeeper != nil { + feeTxValidator = ante.NewDeductFeeDecorator(in.AccountKeeper, bankKeeper, nil, nil) + // set min gas price in deduct fee decorator + feeTxValidator.SetMinGasPrices(minGasPrices) + // pass deduct fee decorator to app module + m.SetFeeTxValidator(feeTxValidator) + } + + return ModuleOutputs{BankKeeper: bankKeeper, Module: m, FeeTxValidator: feeTxValidator} } func InvokeSetSendRestrictions( @@ -129,3 +157,19 @@ func InvokeSetSendRestrictions( return nil } + +func InvokeCheckFeeGrantPresent(modules map[string]appmodule.AppModule) error { + _, ok := modules[feegrantModuleName] + if ok { + // get bank module + bankMod, ok := modules[types.ModuleName] + if !ok { + return nil + } + + // set isFeegrant + m := bankMod.(AppModule) + m.SetFeeTxValidator(nil) + } + return nil +} diff --git a/x/bank/module.go b/x/bank/module.go index 763bc69056f7..7e320d61f799 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -10,8 +10,11 @@ import ( "google.golang.org/grpc" "cosmossdk.io/core/appmodule" + appmodulev2 "cosmossdk.io/core/appmodule/v2" "cosmossdk.io/core/legacy" "cosmossdk.io/core/registry" + "cosmossdk.io/core/transaction" + "cosmossdk.io/x/auth/ante" "cosmossdk.io/x/bank/client/cli" "cosmossdk.io/x/bank/keeper" "cosmossdk.io/x/bank/simulation" @@ -44,6 +47,13 @@ type AppModule struct { cdc codec.Codec keeper keeper.Keeper accountKeeper types.AccountKeeper + + state *feeTxState +} + +type feeTxState struct { + // deduct fee v2 tx validator + feeTxValidator ante.FeeTxValidator } // NewAppModule creates a new AppModule object @@ -52,6 +62,7 @@ func NewAppModule(cdc codec.Codec, keeper keeper.Keeper, accountKeeper types.Acc cdc: cdc, keeper: keeper, accountKeeper: accountKeeper, + state: &feeTxState{}, } } @@ -151,6 +162,34 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) return am.cdc.MarshalJSON(gs) } +// SetFeeTxValidator sets feeTxValidator in AppModule which is used for deducting fee +func (am AppModule) SetFeeTxValidator(validator ante.FeeTxValidator) { + am.state.feeTxValidator = validator +} + +// TxValidator implements appmodulev2.HasTxValidator. +// It replaces auth ante handlers for server/v2 +func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { + validators := []appmodulev2.TxValidator[sdk.Tx]{} + + if am.state != nil && am.state.feeTxValidator != nil { + validators = append(validators, am.state.feeTxValidator) + } + + sdkTx, ok := tx.(sdk.Tx) + if !ok { + return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx) + } + + for _, validator := range validators { + if err := validator.ValidateTx(ctx, sdkTx); err != nil { + return err + } + } + + return nil +} + // ConsensusVersion implements HasConsensusVersion func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion } diff --git a/x/bank/testutil/expected_keepers_mocks.go b/x/bank/testutil/expected_keepers_mocks.go index 058c4e2d7e36..e2a53193f7d6 100644 --- a/x/bank/testutil/expected_keepers_mocks.go +++ b/x/bank/testutil/expected_keepers_mocks.go @@ -9,6 +9,7 @@ import ( reflect "reflect" address "cosmossdk.io/core/address" + appmodule "cosmossdk.io/core/appmodule/v2" types "cosmossdk.io/x/auth/types" types0 "github.com/cosmos/cosmos-sdk/types" gomock "github.com/golang/mock/gomock" @@ -65,6 +66,20 @@ func (mr *MockAccountKeeperMockRecorder) GetAccount(ctx, addr interface{}) *gomo return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAccount", reflect.TypeOf((*MockAccountKeeper)(nil).GetAccount), ctx, addr) } +// GetEnvironment mocks base method. +func (m *MockAccountKeeper) GetEnvironment() appmodule.Environment { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetEnvironment") + ret0, _ := ret[0].(appmodule.Environment) + return ret0 +} + +// GetEnvironment indicates an expected call of GetEnvironment. +func (mr *MockAccountKeeperMockRecorder) GetEnvironment() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEnvironment", reflect.TypeOf((*MockAccountKeeper)(nil).GetEnvironment)) +} + // GetModuleAccount mocks base method. func (m *MockAccountKeeper) GetModuleAccount(ctx context.Context, moduleName string) types0.ModuleAccountI { m.ctrl.T.Helper() @@ -137,6 +152,20 @@ func (mr *MockAccountKeeperMockRecorder) GetModulePermissions() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetModulePermissions", reflect.TypeOf((*MockAccountKeeper)(nil).GetModulePermissions)) } +// GetParams mocks base method. +func (m *MockAccountKeeper) GetParams(ctx context.Context) types.Params { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetParams", ctx) + ret0, _ := ret[0].(types.Params) + return ret0 +} + +// GetParams indicates an expected call of GetParams. +func (mr *MockAccountKeeperMockRecorder) GetParams(ctx interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetParams", reflect.TypeOf((*MockAccountKeeper)(nil).GetParams), ctx) +} + // HasAccount mocks base method. func (m *MockAccountKeeper) HasAccount(ctx context.Context, addr types0.AccAddress) bool { m.ctrl.T.Helper() diff --git a/x/bank/types/expected_keepers.go b/x/bank/types/expected_keepers.go index d49bf4cc4b98..19389558d831 100644 --- a/x/bank/types/expected_keepers.go +++ b/x/bank/types/expected_keepers.go @@ -4,6 +4,7 @@ import ( "context" "cosmossdk.io/core/address" + "cosmossdk.io/core/appmodule/v2" "cosmossdk.io/x/auth/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -17,6 +18,7 @@ type AccountKeeper interface { NewAccount(context.Context, sdk.AccountI) sdk.AccountI NewAccountWithAddress(ctx context.Context, addr sdk.AccAddress) sdk.AccountI + GetParams(ctx context.Context) (params types.Params) GetAccount(ctx context.Context, addr sdk.AccAddress) sdk.AccountI HasAccount(ctx context.Context, addr sdk.AccAddress) bool SetAccount(ctx context.Context, acc sdk.AccountI) @@ -28,4 +30,5 @@ type AccountKeeper interface { GetModuleAccount(ctx context.Context, moduleName string) sdk.ModuleAccountI SetModuleAccount(ctx context.Context, macc sdk.ModuleAccountI) GetModulePermissions() map[string]types.PermissionsForAddress + GetEnvironment() appmodule.Environment } diff --git a/x/feegrant/module/depinject.go b/x/feegrant/module/depinject.go index 3fb6afa68984..711fa2f606ab 100644 --- a/x/feegrant/module/depinject.go +++ b/x/feegrant/module/depinject.go @@ -5,6 +5,7 @@ import ( "cosmossdk.io/core/appmodule" "cosmossdk.io/depinject" "cosmossdk.io/depinject/appconfig" + "cosmossdk.io/x/auth/ante" "cosmossdk.io/x/feegrant" "cosmossdk.io/x/feegrant/keeper" "cosmossdk.io/x/feegrant/simulation" @@ -34,11 +35,19 @@ type FeegrantInputs struct { AccountKeeper feegrant.AccountKeeper BankKeeper feegrant.BankKeeper Registry cdctypes.InterfaceRegistry + + FeeTxValidator ante.FeeTxValidator `optional:"true"` // server v2 } func ProvideModule(in FeegrantInputs) (keeper.Keeper, appmodule.AppModule) { k := keeper.NewKeeper(in.Environment, in.Cdc, in.AccountKeeper) m := NewAppModule(in.Cdc, in.AccountKeeper, in.BankKeeper, k, in.Registry) + + if in.FeeTxValidator != nil { + feeTxValidator := in.FeeTxValidator.SetFeegrantKeeper(k) + m.SetFeeTxValidator(feeTxValidator) + } + return k, m } diff --git a/x/feegrant/module/module.go b/x/feegrant/module/module.go index 134160422723..67d33f6ffab7 100644 --- a/x/feegrant/module/module.go +++ b/x/feegrant/module/module.go @@ -10,9 +10,12 @@ import ( "google.golang.org/grpc" "cosmossdk.io/core/appmodule" + appmodulev2 "cosmossdk.io/core/appmodule/v2" "cosmossdk.io/core/legacy" "cosmossdk.io/core/registry" + "cosmossdk.io/core/transaction" "cosmossdk.io/errors" + "cosmossdk.io/x/auth/ante" "cosmossdk.io/x/feegrant" "cosmossdk.io/x/feegrant/client/cli" "cosmossdk.io/x/feegrant/keeper" @@ -20,6 +23,7 @@ import ( sdkclient "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" cdctypes "github.com/cosmos/cosmos-sdk/codec/types" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" ) @@ -43,6 +47,9 @@ type AppModule struct { keeper keeper.Keeper accountKeeper feegrant.AccountKeeper bankKeeper feegrant.BankKeeper + + // deduct fee v2 tx validator + feeTxValidator ante.FeeTxValidator } // NewAppModule creates a new AppModule object @@ -145,6 +152,32 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) return am.cdc.MarshalJSON(gs) } +// SetFeeTxValidator sets feeTxValidator in AppModule which is used for deducting fee +func (am *AppModule) SetFeeTxValidator(validator ante.FeeTxValidator) { + am.feeTxValidator = validator +} + +// TxValidator implements appmodulev2.HasTxValidator. +// It replaces auth ante handlers for server/v2 +func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { + validators := []appmodulev2.TxValidator[sdk.Tx]{} + + if am.feeTxValidator != nil { + validators = append(validators, am.feeTxValidator) + } + + sdkTx, ok := tx.(sdk.Tx) + if !ok { + return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx) + } + for _, validator := range validators { + if err := validator.ValidateTx(ctx, sdkTx); err != nil { + return err + } + } + return nil +} + // ConsensusVersion implements HasConsensusVersion func (AppModule) ConsensusVersion() uint64 { return 2 } From ca0a801cfe4199146f3044bec491477ad8df9c55 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Wed, 21 Aug 2024 13:07:07 +0530 Subject: [PATCH 14/28] fix lint and small changes --- server/v2/config.go | 2 +- server/v2/flags.go | 1 - server/v2/server.go | 8 ++++---- x/auth/ante/fee.go | 2 +- x/bank/depinject.go | 2 +- x/bank/go.mod | 2 +- x/bank/module.go | 6 +++--- 7 files changed, 11 insertions(+), 12 deletions(-) diff --git a/server/v2/config.go b/server/v2/config.go index 7b2a4fdb9492..48386a716fbd 100644 --- a/server/v2/config.go +++ b/server/v2/config.go @@ -7,7 +7,7 @@ import ( "github.com/spf13/viper" ) -// ServerConfig defines configuration for the main server. +// ServerConfig defines configuration for the server component. type ServerConfig struct { MinGasPrices string `mapstructure:"minimum-gas-prices" toml:"minimum-gas-prices" comment:"minimum-gas-prices defines the price which a validator is willing to accept for processing a transaction. A transaction's fees must meet the minimum of any denomination specified in this config (e.g. 0.25token1;0.0001token2)."` } diff --git a/server/v2/flags.go b/server/v2/flags.go index 6c253d419ab0..fc36cdcf2b4c 100644 --- a/server/v2/flags.go +++ b/server/v2/flags.go @@ -4,7 +4,6 @@ package serverv2 import "fmt" // start flags are prefixed with the server name -// as the config in prefixed with the server name // this allows viper to properly bind the flags func prefix(f string) string { return fmt.Sprintf("%s.%s", serverName, f) diff --git a/server/v2/server.go b/server/v2/server.go index d08c57fa7553..22d0ea27f399 100644 --- a/server/v2/server.go +++ b/server/v2/server.go @@ -86,7 +86,7 @@ func (s *Server[T]) Name() string { // Start starts all components concurrently. func (s *Server[T]) Start(ctx context.Context) error { - // validate main server config + // validate server component config if err := s.config.ValidateBasic(); err != nil { return err } @@ -163,7 +163,7 @@ func (s *Server[T]) CLICommands() CLIConfig { return commands } -// Config returns config of the main server component +// Config returns config of the server component func (s *Server[T]) Config() ServerConfig { return s.config } @@ -172,7 +172,7 @@ func (s *Server[T]) Config() ServerConfig { func (s *Server[T]) Configs() map[string]any { cfgs := make(map[string]any) - // add main server component config + // add server component config cfgs[s.Name()] = s.config // add other components' config @@ -254,7 +254,7 @@ func (s *Server[T]) WriteConfig(configPath string) error { func (s *Server[T]) StartFlags() []*pflag.FlagSet { flags := []*pflag.FlagSet{} - // add main server component flags + // add server component flags flags = append(flags, s.StartCmdFlags()) // add other components' start cmd flags diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index 05549b751f32..2c25602bb3fe 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -5,11 +5,11 @@ import ( "context" "fmt" + appmodulev2 "cosmossdk.io/core/appmodule/v2" "cosmossdk.io/core/transaction" errorsmod "cosmossdk.io/errors" "cosmossdk.io/x/auth/types" - appmodulev2 "cosmossdk.io/core/appmodule/v2" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) diff --git a/x/bank/depinject.go b/x/bank/depinject.go index 3fac5aa95c83..8f8d7ed698ef 100644 --- a/x/bank/depinject.go +++ b/x/bank/depinject.go @@ -4,6 +4,7 @@ import ( "fmt" "sort" + "github.com/spf13/viper" "golang.org/x/exp/maps" modulev1 "cosmossdk.io/api/cosmos/bank/module/v1" @@ -17,7 +18,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/spf13/viper" ) const ( diff --git a/x/bank/go.mod b/x/bank/go.mod index 1c611ee920f7..255caed74df3 100644 --- a/x/bank/go.mod +++ b/x/bank/go.mod @@ -135,7 +135,7 @@ require ( github.com/spf13/afero v1.11.0 // indirect github.com/spf13/cast v1.7.0 // indirect github.com/spf13/pflag v1.0.5 // indirect - github.com/spf13/viper v1.19.0 // indirect + github.com/spf13/viper v1.19.0 github.com/subosito/gotenv v1.6.0 // indirect github.com/supranational/blst v0.3.12 // indirect github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d // indirect diff --git a/x/bank/module.go b/x/bank/module.go index 7e320d61f799..411e136986e6 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -48,10 +48,10 @@ type AppModule struct { keeper keeper.Keeper accountKeeper types.AccountKeeper - state *feeTxState + state *deductFeeTxState } -type feeTxState struct { +type deductFeeTxState struct { // deduct fee v2 tx validator feeTxValidator ante.FeeTxValidator } @@ -62,7 +62,7 @@ func NewAppModule(cdc codec.Codec, keeper keeper.Keeper, accountKeeper types.Acc cdc: cdc, keeper: keeper, accountKeeper: accountKeeper, - state: &feeTxState{}, + state: &deductFeeTxState{}, // initialize state } } From 31575656c16c5788d21c2be1a2ab8e2ad86232f4 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Wed, 21 Aug 2024 14:47:56 +0530 Subject: [PATCH 15/28] add todo --- x/bank/depinject.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x/bank/depinject.go b/x/bank/depinject.go index 5a572d0381c7..90e06b3dbff2 100644 --- a/x/bank/depinject.go +++ b/x/bank/depinject.go @@ -159,6 +159,8 @@ func InvokeSetSendRestrictions( return nil } +// TODO: Remove below check and move deduct-fee-decorator check completely to x/bank txValidator once sims v2 is done. +// Sims v2 PR will remove dependency between x/bank and x/feegrant.https://github.com/cosmos/cosmos-sdk/pull/20940 func InvokeCheckFeeGrantPresent(modules map[string]appmodule.AppModule) error { _, ok := modules[feegrantModuleName] if ok { From 44a7c0fd18f6e597bc2c6bce56a25c3e63edd02c Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Thu, 22 Aug 2024 10:38:18 +0530 Subject: [PATCH 16/28] fix lint --- x/auth/client/cli/tx_multisign.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/client/cli/tx_multisign.go b/x/auth/client/cli/tx_multisign.go index d98cae0ca1e8..497cc50d8850 100644 --- a/x/auth/client/cli/tx_multisign.go +++ b/x/auth/client/cli/tx_multisign.go @@ -29,7 +29,7 @@ import ( // GetMultiSignCommand returns the multi-sign command func GetMultiSignCommand() *cobra.Command { cmd := &cobra.Command{ - Use: "multi-sign [...]", + Use: "multi-sign [...]", Aliases: []string{"multisign"}, Short: "Generate multisig signatures for transactions generated offline", Long: strings.TrimSpace( From 76cbd83beb668660050933456fcdbf67e4a729a3 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Fri, 23 Aug 2024 14:45:47 +0530 Subject: [PATCH 17/28] address comments --- server/v2/config.go | 13 +++---------- server/v2/server.go | 5 ----- server/v2/server_test.go | 2 +- simapp/v2/simdv2/cmd/commands.go | 13 +++++-------- 4 files changed, 9 insertions(+), 24 deletions(-) diff --git a/server/v2/config.go b/server/v2/config.go index 48386a716fbd..b5c525fdf682 100644 --- a/server/v2/config.go +++ b/server/v2/config.go @@ -12,18 +12,11 @@ type ServerConfig struct { MinGasPrices string `mapstructure:"minimum-gas-prices" toml:"minimum-gas-prices" comment:"minimum-gas-prices defines the price which a validator is willing to accept for processing a transaction. A transaction's fees must meet the minimum of any denomination specified in this config (e.g. 0.25token1;0.0001token2)."` } -// ValidateBasic returns an error if min-gas-prices field is empty in Config. Otherwise, it returns nil. -func (c ServerConfig) ValidateBasic() error { - if c.MinGasPrices == "" { - return fmt.Errorf("error in app.toml: set minimum-gas-prices in app.toml or flag or env variable") - } - - return nil -} - // DefaultServerConfig returns the default config of server component func DefaultServerConfig() ServerConfig { - return ServerConfig{} + return ServerConfig{ + MinGasPrices: "0stake", + } } // ReadConfig returns a viper instance of the config file diff --git a/server/v2/server.go b/server/v2/server.go index 22d0ea27f399..08ae9a6a06fa 100644 --- a/server/v2/server.go +++ b/server/v2/server.go @@ -86,11 +86,6 @@ func (s *Server[T]) Name() string { // Start starts all components concurrently. func (s *Server[T]) Start(ctx context.Context) error { - // validate server component config - if err := s.config.ValidateBasic(); err != nil { - return err - } - s.logger.Info("starting servers...") g, ctx := errgroup.WithContext(ctx) diff --git a/server/v2/server_test.go b/server/v2/server_test.go index f7de8a3f82e5..9a684d8a074f 100644 --- a/server/v2/server_test.go +++ b/server/v2/server_test.go @@ -65,7 +65,7 @@ func TestServer(t *testing.T) { server := serverv2.NewServer( logger, - serverv2.ServerConfig{MinGasPrices: "0stake"}, + serverv2.DefaultServerConfig(), grpcServer, mockServer, ) diff --git a/simapp/v2/simdv2/cmd/commands.go b/simapp/v2/simdv2/cmd/commands.go index 05ed1a176579..2b3f2dd014b1 100644 --- a/simapp/v2/simdv2/cmd/commands.go +++ b/simapp/v2/simdv2/cmd/commands.go @@ -70,18 +70,15 @@ func initRootCmd[T transaction.Tx]( offchain.OffChain(), ) - // Optionally allow the chain developer to overwrite the cometbft default + // Allow the chain developer to overwrite the cometbft default // app toml config. serverCfg := serverv2.DefaultServerConfig() - // The cometbft server's default minimum gas price is set to "" (empty value) inside - // app.toml. If left empty by validators, the node will halt on startup. - // However, the chain developer can set a default app.toml value for their - // validators here. + // The cometbft server's default minimum gas price is set to "0stake" inside + // app.toml. However, the chain developer can set a default app.toml value for their + // validators here. Please update value based on chain denom. // // In summary: - // - if you leave serverCfg.MinGasPrices = "", all validators MUST tweak their - // own app.toml config, - // - if you set serverCfg.MinGasPrices non-empty, validators CAN tweak their + // - if you set serverCfg.MinGasPrices value, validators CAN tweak their // own app.toml to override, or use this default value. // // In simapp, we set the min gas prices to 0. From 98d5914aa564d8a5cac6fd3f60bb3b23a6fd05db Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Mon, 26 Aug 2024 17:03:31 +0530 Subject: [PATCH 18/28] update approach --- x/auth/ante/fee.go | 16 ------- x/auth/tx/config/depinject.go | 41 ++++++++++++++++- x/bank/depinject.go | 55 ++--------------------- x/bank/go.mod | 2 +- x/bank/module.go | 39 ---------------- x/bank/testutil/expected_keepers_mocks.go | 29 ------------ x/bank/types/expected_keepers.go | 3 -- x/feegrant/module/depinject.go | 9 ---- x/feegrant/module/module.go | 33 -------------- 9 files changed, 44 insertions(+), 183 deletions(-) diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index 2c25602bb3fe..bb774d01dd5c 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -5,7 +5,6 @@ import ( "context" "fmt" - appmodulev2 "cosmossdk.io/core/appmodule/v2" "cosmossdk.io/core/transaction" errorsmod "cosmossdk.io/errors" "cosmossdk.io/x/auth/types" @@ -18,15 +17,6 @@ import ( // The effective fee should be deducted later, and the priority should be returned in the ABCI response. type TxFeeChecker func(ctx context.Context, tx sdk.Tx) (sdk.Coins, int64, error) -// FeeTxValidator defines custom type used to represent deduct fee decorator -// which will be passed from x/auth/tx to x/auth module. -type FeeTxValidator interface { - appmodulev2.TxValidator[sdk.Tx] - - SetMinGasPrices(sdk.DecCoins) - SetFeegrantKeeper(FeegrantKeeper) FeeTxValidator -} - // DeductFeeDecorator deducts fees from the fee payer. The fee payer is the fee granter (if specified) or first signer of the tx. // If the fee payer does not have the funds to pay for the fees, return an InsufficientFunds error. // Call next AnteHandler if fees are successfully deducted. @@ -72,12 +62,6 @@ func (dfd DeductFeeDecorator) SetMinGasPrices(minGasPrices sdk.DecCoins) { dfd.state.minGasPrices = minGasPrices } -// SetFeegrantKeeper sets the feegrant keeper in DeductFeeDecorator -func (dfd DeductFeeDecorator) SetFeegrantKeeper(feegrantKeeper FeegrantKeeper) FeeTxValidator { - dfd.feegrantKeeper = feegrantKeeper - return dfd -} - // AnteHandle implements an AnteHandler decorator for the DeductFeeDecorator func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (sdk.Context, error) { dfd.state.minGasPrices = ctx.MinGasPrices() diff --git a/x/auth/tx/config/depinject.go b/x/auth/tx/config/depinject.go index bdf3879efbc0..613919840c6e 100644 --- a/x/auth/tx/config/depinject.go +++ b/x/auth/tx/config/depinject.go @@ -6,6 +6,7 @@ import ( "fmt" gogoproto "github.com/cosmos/gogoproto/proto" + "github.com/spf13/viper" "google.golang.org/grpc" "google.golang.org/grpc/codes" grpcstatus "google.golang.org/grpc/status" @@ -34,6 +35,8 @@ import ( signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" ) +const FlagMinGasPricesV2 = "server.minimum-gas-prices" + func init() { appconfig.RegisterModule(&txconfigv1.Config{}, appconfig.Provide(ProvideModule), @@ -58,6 +61,7 @@ type ModuleInputs struct { AccountAbstractionKeeper ante.AccountAbstractionKeeper `optional:"true"` CustomSignModeHandlers func() []txsigning.SignModeHandler `optional:"true"` CustomGetSigners []txsigning.CustomGetSigner `optional:"true"` + Viper *viper.Viper `optional:"true"` // server v2 } type ModuleOutputs struct { @@ -150,7 +154,24 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { ante.DefaultSigVerificationGasConsumer, in.AccountAbstractionKeeper, ) - appModule := AppModule{svd} + appModule := AppModule{sigVerification: svd} + + var minGasPrices sdk.DecCoins + if in.Viper != nil { + minGasPricesStr := in.Viper.GetString(FlagMinGasPricesV2) + minGasPrices, err = sdk.ParseDecCoins(minGasPricesStr) + if err != nil { + panic(fmt.Sprintf("invalid minimum gas prices: %v", err)) + } + } + + if in.AccountKeeper != nil && in.BankKeeper != nil { + feeTxValidator := ante.NewDeductFeeDecorator(in.AccountKeeper, in.BankKeeper, nil, nil) + // set min gas price in deduct fee decorator + feeTxValidator.SetMinGasPrices(minGasPrices) + // set deduct fee decorator to app module + appModule.feeTxValidator = feeTxValidator + } return ModuleOutputs{TxConfig: txConfig, TxConfigOptions: txConfigOptions, BaseAppOption: baseAppOption, Module: appModule} } @@ -240,11 +261,27 @@ var ( type AppModule struct { sigVerification ante.SigVerificationDecorator + + // deduct fee v2 tx validator + feeTxValidator ante.DeductFeeDecorator } // TxValidator implements appmodule.HasTxValidator. func (a AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { - return a.sigVerification.ValidateTx(ctx, tx) + if err := a.sigVerification.ValidateTx(ctx, tx); err != nil { + return err + } + + sdkTx, ok := tx.(sdk.Tx) + if !ok { + return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx) + } + + if err := a.feeTxValidator.ValidateTx(ctx, sdkTx); err != nil { + return err + } + + return nil } // IsAppModule implements appmodule.AppModule. diff --git a/x/bank/depinject.go b/x/bank/depinject.go index 90e06b3dbff2..ecc05abf4732 100644 --- a/x/bank/depinject.go +++ b/x/bank/depinject.go @@ -6,24 +6,15 @@ import ( "slices" "sort" - "github.com/spf13/viper" - modulev1 "cosmossdk.io/api/cosmos/bank/module/v1" "cosmossdk.io/core/appmodule" "cosmossdk.io/depinject" "cosmossdk.io/depinject/appconfig" - "cosmossdk.io/x/auth/ante" authtypes "cosmossdk.io/x/auth/types" "cosmossdk.io/x/bank/keeper" "cosmossdk.io/x/bank/types" "github.com/cosmos/cosmos-sdk/codec" - sdk "github.com/cosmos/cosmos-sdk/types" -) - -const ( - FlagMinGasPricesV2 = "server.minimum-gas-prices" - feegrantModuleName = "feegrant" ) var _ depinject.OnePerModuleType = AppModule{} @@ -35,7 +26,7 @@ func init() { appconfig.RegisterModule( &modulev1.Module{}, appconfig.Provide(ProvideModule), - appconfig.Invoke(InvokeSetSendRestrictions, InvokeCheckFeeGrantPresent), + appconfig.Invoke(InvokeSetSendRestrictions), ) } @@ -47,15 +38,13 @@ type ModuleInputs struct { Environment appmodule.Environment AccountKeeper types.AccountKeeper - Viper *viper.Viper `optional:"true"` // server v2 } type ModuleOutputs struct { depinject.Out - BankKeeper keeper.BaseKeeper - Module appmodule.AppModule - FeeTxValidator ante.FeeTxValidator // pass deduct fee decorator to feegrant TxValidator + BankKeeper keeper.BaseKeeper + Module appmodule.AppModule } func ProvideModule(in ModuleInputs) ModuleOutputs { @@ -102,25 +91,7 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { ) m := NewAppModule(in.Cdc, bankKeeper, in.AccountKeeper) - var minGasPrices sdk.DecCoins - if in.Viper != nil { - minGasPricesStr := in.Viper.GetString(FlagMinGasPricesV2) - minGasPrices, err = sdk.ParseDecCoins(minGasPricesStr) - if err != nil { - panic(fmt.Sprintf("invalid minimum gas prices: %v", err)) - } - } - - var feeTxValidator ante.FeeTxValidator - if in.AccountKeeper != nil { - feeTxValidator = ante.NewDeductFeeDecorator(in.AccountKeeper, bankKeeper, nil, nil) - // set min gas price in deduct fee decorator - feeTxValidator.SetMinGasPrices(minGasPrices) - // pass deduct fee decorator to app module - m.SetFeeTxValidator(feeTxValidator) - } - - return ModuleOutputs{BankKeeper: bankKeeper, Module: m, FeeTxValidator: feeTxValidator} + return ModuleOutputs{BankKeeper: bankKeeper, Module: m} } func InvokeSetSendRestrictions( @@ -158,21 +129,3 @@ func InvokeSetSendRestrictions( return nil } - -// TODO: Remove below check and move deduct-fee-decorator check completely to x/bank txValidator once sims v2 is done. -// Sims v2 PR will remove dependency between x/bank and x/feegrant.https://github.com/cosmos/cosmos-sdk/pull/20940 -func InvokeCheckFeeGrantPresent(modules map[string]appmodule.AppModule) error { - _, ok := modules[feegrantModuleName] - if ok { - // get bank module - bankMod, ok := modules[types.ModuleName] - if !ok { - return nil - } - - // set isFeegrant - m := bankMod.(AppModule) - m.SetFeeTxValidator(nil) - } - return nil -} diff --git a/x/bank/go.mod b/x/bank/go.mod index 53d532d432f3..db1275f7a032 100644 --- a/x/bank/go.mod +++ b/x/bank/go.mod @@ -135,7 +135,7 @@ require ( github.com/spf13/afero v1.11.0 // indirect github.com/spf13/cast v1.7.0 // indirect github.com/spf13/pflag v1.0.5 // indirect - github.com/spf13/viper v1.19.0 + github.com/spf13/viper v1.19.0 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/supranational/blst v0.3.12 // indirect github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d // indirect diff --git a/x/bank/module.go b/x/bank/module.go index 411e136986e6..763bc69056f7 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -10,11 +10,8 @@ import ( "google.golang.org/grpc" "cosmossdk.io/core/appmodule" - appmodulev2 "cosmossdk.io/core/appmodule/v2" "cosmossdk.io/core/legacy" "cosmossdk.io/core/registry" - "cosmossdk.io/core/transaction" - "cosmossdk.io/x/auth/ante" "cosmossdk.io/x/bank/client/cli" "cosmossdk.io/x/bank/keeper" "cosmossdk.io/x/bank/simulation" @@ -47,13 +44,6 @@ type AppModule struct { cdc codec.Codec keeper keeper.Keeper accountKeeper types.AccountKeeper - - state *deductFeeTxState -} - -type deductFeeTxState struct { - // deduct fee v2 tx validator - feeTxValidator ante.FeeTxValidator } // NewAppModule creates a new AppModule object @@ -62,7 +52,6 @@ func NewAppModule(cdc codec.Codec, keeper keeper.Keeper, accountKeeper types.Acc cdc: cdc, keeper: keeper, accountKeeper: accountKeeper, - state: &deductFeeTxState{}, // initialize state } } @@ -162,34 +151,6 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) return am.cdc.MarshalJSON(gs) } -// SetFeeTxValidator sets feeTxValidator in AppModule which is used for deducting fee -func (am AppModule) SetFeeTxValidator(validator ante.FeeTxValidator) { - am.state.feeTxValidator = validator -} - -// TxValidator implements appmodulev2.HasTxValidator. -// It replaces auth ante handlers for server/v2 -func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { - validators := []appmodulev2.TxValidator[sdk.Tx]{} - - if am.state != nil && am.state.feeTxValidator != nil { - validators = append(validators, am.state.feeTxValidator) - } - - sdkTx, ok := tx.(sdk.Tx) - if !ok { - return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx) - } - - for _, validator := range validators { - if err := validator.ValidateTx(ctx, sdkTx); err != nil { - return err - } - } - - return nil -} - // ConsensusVersion implements HasConsensusVersion func (AppModule) ConsensusVersion() uint64 { return ConsensusVersion } diff --git a/x/bank/testutil/expected_keepers_mocks.go b/x/bank/testutil/expected_keepers_mocks.go index e2a53193f7d6..058c4e2d7e36 100644 --- a/x/bank/testutil/expected_keepers_mocks.go +++ b/x/bank/testutil/expected_keepers_mocks.go @@ -9,7 +9,6 @@ import ( reflect "reflect" address "cosmossdk.io/core/address" - appmodule "cosmossdk.io/core/appmodule/v2" types "cosmossdk.io/x/auth/types" types0 "github.com/cosmos/cosmos-sdk/types" gomock "github.com/golang/mock/gomock" @@ -66,20 +65,6 @@ func (mr *MockAccountKeeperMockRecorder) GetAccount(ctx, addr interface{}) *gomo return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAccount", reflect.TypeOf((*MockAccountKeeper)(nil).GetAccount), ctx, addr) } -// GetEnvironment mocks base method. -func (m *MockAccountKeeper) GetEnvironment() appmodule.Environment { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetEnvironment") - ret0, _ := ret[0].(appmodule.Environment) - return ret0 -} - -// GetEnvironment indicates an expected call of GetEnvironment. -func (mr *MockAccountKeeperMockRecorder) GetEnvironment() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEnvironment", reflect.TypeOf((*MockAccountKeeper)(nil).GetEnvironment)) -} - // GetModuleAccount mocks base method. func (m *MockAccountKeeper) GetModuleAccount(ctx context.Context, moduleName string) types0.ModuleAccountI { m.ctrl.T.Helper() @@ -152,20 +137,6 @@ func (mr *MockAccountKeeperMockRecorder) GetModulePermissions() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetModulePermissions", reflect.TypeOf((*MockAccountKeeper)(nil).GetModulePermissions)) } -// GetParams mocks base method. -func (m *MockAccountKeeper) GetParams(ctx context.Context) types.Params { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetParams", ctx) - ret0, _ := ret[0].(types.Params) - return ret0 -} - -// GetParams indicates an expected call of GetParams. -func (mr *MockAccountKeeperMockRecorder) GetParams(ctx interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetParams", reflect.TypeOf((*MockAccountKeeper)(nil).GetParams), ctx) -} - // HasAccount mocks base method. func (m *MockAccountKeeper) HasAccount(ctx context.Context, addr types0.AccAddress) bool { m.ctrl.T.Helper() diff --git a/x/bank/types/expected_keepers.go b/x/bank/types/expected_keepers.go index 19389558d831..d49bf4cc4b98 100644 --- a/x/bank/types/expected_keepers.go +++ b/x/bank/types/expected_keepers.go @@ -4,7 +4,6 @@ import ( "context" "cosmossdk.io/core/address" - "cosmossdk.io/core/appmodule/v2" "cosmossdk.io/x/auth/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -18,7 +17,6 @@ type AccountKeeper interface { NewAccount(context.Context, sdk.AccountI) sdk.AccountI NewAccountWithAddress(ctx context.Context, addr sdk.AccAddress) sdk.AccountI - GetParams(ctx context.Context) (params types.Params) GetAccount(ctx context.Context, addr sdk.AccAddress) sdk.AccountI HasAccount(ctx context.Context, addr sdk.AccAddress) bool SetAccount(ctx context.Context, acc sdk.AccountI) @@ -30,5 +28,4 @@ type AccountKeeper interface { GetModuleAccount(ctx context.Context, moduleName string) sdk.ModuleAccountI SetModuleAccount(ctx context.Context, macc sdk.ModuleAccountI) GetModulePermissions() map[string]types.PermissionsForAddress - GetEnvironment() appmodule.Environment } diff --git a/x/feegrant/module/depinject.go b/x/feegrant/module/depinject.go index 85455e9df410..018b09d3d593 100644 --- a/x/feegrant/module/depinject.go +++ b/x/feegrant/module/depinject.go @@ -5,7 +5,6 @@ import ( "cosmossdk.io/core/appmodule" "cosmossdk.io/depinject" "cosmossdk.io/depinject/appconfig" - "cosmossdk.io/x/auth/ante" "cosmossdk.io/x/feegrant" "cosmossdk.io/x/feegrant/keeper" @@ -32,18 +31,10 @@ type FeegrantInputs struct { AccountKeeper feegrant.AccountKeeper BankKeeper feegrant.BankKeeper Registry cdctypes.InterfaceRegistry - - FeeTxValidator ante.FeeTxValidator `optional:"true"` // server v2 } func ProvideModule(in FeegrantInputs) (keeper.Keeper, appmodule.AppModule) { k := keeper.NewKeeper(in.Environment, in.Cdc, in.AccountKeeper) m := NewAppModule(in.Cdc, in.AccountKeeper, in.BankKeeper, k, in.Registry) - - if in.FeeTxValidator != nil { - feeTxValidator := in.FeeTxValidator.SetFeegrantKeeper(k) - m.SetFeeTxValidator(feeTxValidator) - } - return k, m } diff --git a/x/feegrant/module/module.go b/x/feegrant/module/module.go index 2889cb8aaeec..5fde472bdbf9 100644 --- a/x/feegrant/module/module.go +++ b/x/feegrant/module/module.go @@ -10,12 +10,9 @@ import ( "google.golang.org/grpc" "cosmossdk.io/core/appmodule" - appmodulev2 "cosmossdk.io/core/appmodule/v2" "cosmossdk.io/core/legacy" "cosmossdk.io/core/registry" - "cosmossdk.io/core/transaction" "cosmossdk.io/errors" - "cosmossdk.io/x/auth/ante" "cosmossdk.io/x/feegrant" "cosmossdk.io/x/feegrant/client/cli" "cosmossdk.io/x/feegrant/keeper" @@ -24,7 +21,6 @@ import ( sdkclient "github.com/cosmos/cosmos-sdk/client" "github.com/cosmos/cosmos-sdk/codec" cdctypes "github.com/cosmos/cosmos-sdk/codec/types" - sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/module" simtypes "github.com/cosmos/cosmos-sdk/types/simulation" ) @@ -49,9 +45,6 @@ type AppModule struct { keeper keeper.Keeper accountKeeper feegrant.AccountKeeper bankKeeper feegrant.BankKeeper - - // deduct fee v2 tx validator - feeTxValidator ante.FeeTxValidator } // NewAppModule creates a new AppModule object @@ -154,32 +147,6 @@ func (am AppModule) ExportGenesis(ctx context.Context) (json.RawMessage, error) return am.cdc.MarshalJSON(gs) } -// SetFeeTxValidator sets feeTxValidator in AppModule which is used for deducting fee -func (am *AppModule) SetFeeTxValidator(validator ante.FeeTxValidator) { - am.feeTxValidator = validator -} - -// TxValidator implements appmodulev2.HasTxValidator. -// It replaces auth ante handlers for server/v2 -func (am AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { - validators := []appmodulev2.TxValidator[sdk.Tx]{} - - if am.feeTxValidator != nil { - validators = append(validators, am.feeTxValidator) - } - - sdkTx, ok := tx.(sdk.Tx) - if !ok { - return fmt.Errorf("invalid tx type %T, expected sdk.Tx", tx) - } - for _, validator := range validators { - if err := validator.ValidateTx(ctx, sdkTx); err != nil { - return err - } - } - return nil -} - // ConsensusVersion implements HasConsensusVersion func (AppModule) ConsensusVersion() uint64 { return 2 } From 93bebd41ee38c95e7b988b7040c6d21a19588745 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Tue, 27 Aug 2024 11:47:47 +0530 Subject: [PATCH 19/28] emit v2 event and pass feegrant keeper --- x/auth/ante/fee.go | 19 +++++++++---------- x/auth/tx/config/depinject.go | 2 +- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index bb774d01dd5c..3082fec2baa3 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -5,6 +5,7 @@ import ( "context" "fmt" + "cosmossdk.io/core/event" "cosmossdk.io/core/transaction" errorsmod "cosmossdk.io/errors" "cosmossdk.io/x/auth/types" @@ -70,16 +71,6 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, nex return ctx, err } - // TODO: emit this event in v2 after executing ValidateTx method - events := sdk.Events{ - sdk.NewEvent( - sdk.EventTypeTx, - sdk.NewAttribute(sdk.AttributeKeyFee, dfd.state.txFee.String()), - sdk.NewAttribute(sdk.AttributeKeyFeePayer, sdk.AccAddress(dfd.state.deductFeesFrom).String()), - ), - } - ctx.EventManager().EmitEvents(events) - newCtx := ctx.WithPriority(dfd.state.txPriority) return next(newCtx, tx, false) } @@ -116,6 +107,14 @@ func (dfd DeductFeeDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error { return err } + if err := dfd.accountKeeper.GetEnvironment().EventService.EventManager(ctx).EmitKV( + sdk.EventTypeTx, + event.NewAttribute(sdk.AttributeKeyFee, dfd.state.txFee.String()), + event.NewAttribute(sdk.AttributeKeyFeePayer, sdk.AccAddress(dfd.state.deductFeesFrom).String()), + ); err != nil { + return err + } + return nil } diff --git a/x/auth/tx/config/depinject.go b/x/auth/tx/config/depinject.go index 613919840c6e..995215f6fab0 100644 --- a/x/auth/tx/config/depinject.go +++ b/x/auth/tx/config/depinject.go @@ -166,7 +166,7 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { } if in.AccountKeeper != nil && in.BankKeeper != nil { - feeTxValidator := ante.NewDeductFeeDecorator(in.AccountKeeper, in.BankKeeper, nil, nil) + feeTxValidator := ante.NewDeductFeeDecorator(in.AccountKeeper, in.BankKeeper, in.FeeGrantKeeper, nil) // set min gas price in deduct fee decorator feeTxValidator.SetMinGasPrices(minGasPrices) // set deduct fee decorator to app module From 50e95fe8b752cf8ca7b7af94d5054f2007bc7cd3 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Tue, 27 Aug 2024 13:06:41 +0530 Subject: [PATCH 20/28] address comments --- simapp/v2/simdv2/cmd/commands.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/simapp/v2/simdv2/cmd/commands.go b/simapp/v2/simdv2/cmd/commands.go index 2b3f2dd014b1..78aae574c2b4 100644 --- a/simapp/v2/simdv2/cmd/commands.go +++ b/simapp/v2/simdv2/cmd/commands.go @@ -70,10 +70,10 @@ func initRootCmd[T transaction.Tx]( offchain.OffChain(), ) - // Allow the chain developer to overwrite the cometbft default + // Allow the chain developer to overwrite the server default // app toml config. serverCfg := serverv2.DefaultServerConfig() - // The cometbft server's default minimum gas price is set to "0stake" inside + // The server's default minimum gas price is set to "0stake" inside // app.toml. However, the chain developer can set a default app.toml value for their // validators here. Please update value based on chain denom. // From e8365f5f434237389849a1a7b0cd4b24d9c997a7 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Tue, 27 Aug 2024 14:53:55 +0530 Subject: [PATCH 21/28] address comments --- tests/systemtests/staking_test.go | 1 - x/auth/ante/fee.go | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/systemtests/staking_test.go b/tests/systemtests/staking_test.go index 0b18408410df..f197bff1a0ac 100644 --- a/tests/systemtests/staking_test.go +++ b/tests/systemtests/staking_test.go @@ -10,7 +10,6 @@ import ( ) func TestStakeUnstake(t *testing.T) { - t.Skip("The fee deduction is not yet implemented in v2") // Scenario: // delegate tokens to validator // undelegate some tokens diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index 3082fec2baa3..e2a318c2dc61 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -76,6 +76,7 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, nex } // ValidateTx implements an TxValidator for DeductFeeDecorator +// Note: This method is applicable only for transactions that implement the sdk.FeeTx interface. func (dfd DeductFeeDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error { feeTx, ok := tx.(sdk.FeeTx) if !ok { From 5071eac5c83c8a5e5c6b6fa76d78968c4e2ab844 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 28 Aug 2024 13:31:12 +0200 Subject: [PATCH 22/28] remove state struct --- x/auth/ante/fee.go | 100 ++++++++++++++------------------ x/auth/ante/validator_tx_fee.go | 15 +++-- 2 files changed, 53 insertions(+), 62 deletions(-) diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index e2a318c2dc61..60da2e89b3cd 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -27,28 +27,16 @@ type DeductFeeDecorator struct { bankKeeper types.BankKeeper feegrantKeeper FeegrantKeeper txFeeChecker TxFeeChecker - - // pointer to a separate state struct - state *deductFeeState -} - -// deductFeeState holds the mutable state needed across method calls -type deductFeeState struct { minGasPrices sdk.DecCoins - feeTx sdk.FeeTx - txPriority int64 - deductFeesFrom []byte - txFee sdk.Coins - execMode transaction.ExecMode } -func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper, tfc TxFeeChecker) DeductFeeDecorator { - dfd := DeductFeeDecorator{ +func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper, tfc TxFeeChecker) *DeductFeeDecorator { + dfd := &DeductFeeDecorator{ accountKeeper: ak, bankKeeper: bk, feegrantKeeper: fk, txFeeChecker: tfc, - state: &deductFeeState{}, // Initialize the state + minGasPrices: sdk.NewDecCoins(), } if tfc == nil { @@ -59,75 +47,66 @@ func NewDeductFeeDecorator(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKee } // SetMinGasPrices sets the minimum-gas-prices value in the state of DeductFeeDecorator -func (dfd DeductFeeDecorator) SetMinGasPrices(minGasPrices sdk.DecCoins) { - dfd.state.minGasPrices = minGasPrices +func (dfd *DeductFeeDecorator) SetMinGasPrices(minGasPrices sdk.DecCoins) { + dfd.minGasPrices = minGasPrices } // AnteHandle implements an AnteHandler decorator for the DeductFeeDecorator -func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (sdk.Context, error) { - dfd.state.minGasPrices = ctx.MinGasPrices() - - if err := dfd.ValidateTx(ctx, tx); err != nil { +func (dfd *DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, next sdk.AnteHandler) (sdk.Context, error) { + dfd.minGasPrices = ctx.MinGasPrices() + txPriority, err := dfd.innerValidateTx(ctx, tx) + if err != nil { return ctx, err } - newCtx := ctx.WithPriority(dfd.state.txPriority) + newCtx := ctx.WithPriority(txPriority) return next(newCtx, tx, false) } -// ValidateTx implements an TxValidator for DeductFeeDecorator -// Note: This method is applicable only for transactions that implement the sdk.FeeTx interface. -func (dfd DeductFeeDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error { +func (dfd *DeductFeeDecorator) innerValidateTx(ctx context.Context, tx sdk.Tx) (txPriority int64, err error) { feeTx, ok := tx.(sdk.FeeTx) if !ok { - return errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must implement the FeeTx interface") + return 0, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must implement the FeeTx interface") } - // update the state with the current transaction - dfd.state.feeTx = feeTx - - dfd.state.execMode = dfd.accountKeeper.GetEnvironment().TransactionService.ExecMode(ctx) + execMode := dfd.accountKeeper.GetEnvironment().TransactionService.ExecMode(ctx) headerInfo := dfd.accountKeeper.GetEnvironment().HeaderService.HeaderInfo(ctx) - if dfd.state.execMode != transaction.ExecModeSimulate && headerInfo.Height > 0 && dfd.state.feeTx.GetGas() == 0 { - return errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") + if execMode != transaction.ExecModeSimulate && headerInfo.Height > 0 && feeTx.GetGas() == 0 { + return 0, errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") } - dfd.state.txFee = dfd.state.feeTx.GetFee() - - var err error - - if dfd.state.execMode != transaction.ExecModeSimulate { - dfd.state.txFee, dfd.state.txPriority, err = dfd.txFeeChecker(ctx, tx) + txFee := feeTx.GetFee() + if execMode != transaction.ExecModeSimulate { + txFee, txPriority, err = dfd.txFeeChecker(ctx, tx) if err != nil { - return err + return 0, err } } - if err := dfd.checkDeductFee(ctx, tx, dfd.state.txFee); err != nil { - return err + if err := dfd.checkDeductFee(ctx, feeTx, txFee); err != nil { + return 0, err } - if err := dfd.accountKeeper.GetEnvironment().EventService.EventManager(ctx).EmitKV( - sdk.EventTypeTx, - event.NewAttribute(sdk.AttributeKeyFee, dfd.state.txFee.String()), - event.NewAttribute(sdk.AttributeKeyFeePayer, sdk.AccAddress(dfd.state.deductFeesFrom).String()), - ); err != nil { - return err - } + return txPriority, nil +} - return nil +// ValidateTx implements an TxValidator for DeductFeeDecorator +// Note: This method is applicable only for transactions that implement the sdk.FeeTx interface. +func (dfd *DeductFeeDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error { + _, err := dfd.innerValidateTx(ctx, tx) + return err } -func (dfd DeductFeeDecorator) checkDeductFee(ctx context.Context, sdkTx sdk.Tx, fee sdk.Coins) error { +func (dfd *DeductFeeDecorator) checkDeductFee(ctx context.Context, feeTx sdk.FeeTx, fee sdk.Coins) error { addr := dfd.accountKeeper.GetModuleAddress(types.FeeCollectorName) if len(addr) == 0 { return fmt.Errorf("fee collector module account (%s) has not been set", types.FeeCollectorName) } - feePayer := dfd.state.feeTx.FeePayer() - feeGranter := dfd.state.feeTx.FeeGranter() - dfd.state.deductFeesFrom = feePayer + feePayer := feeTx.FeePayer() + feeGranter := feeTx.FeeGranter() + deductFeesFrom := feePayer // if feegranter set, deduct fee from feegranter account. // this works only when feegrant is enabled. @@ -137,7 +116,7 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx context.Context, sdkTx sdk.Tx, if dfd.feegrantKeeper == nil { return sdkerrors.ErrInvalidRequest.Wrap("fee grants are not enabled") } else if !bytes.Equal(feeGranterAddr, feePayer) { - err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranterAddr, feePayer, fee, sdkTx.GetMsgs()) + err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranterAddr, feePayer, fee, feeTx.GetMsgs()) if err != nil { granterAddr, acErr := dfd.accountKeeper.AddressCodec().BytesToString(feeGranter) if acErr != nil { @@ -150,18 +129,25 @@ func (dfd DeductFeeDecorator) checkDeductFee(ctx context.Context, sdkTx sdk.Tx, return errorsmod.Wrapf(err, "%s does not allow to pay fees for %s", granterAddr, payerAddr) } } - - dfd.state.deductFeesFrom = feeGranterAddr + deductFeesFrom = feeGranterAddr } // deduct the fees if !fee.IsZero() { - err := DeductFees(dfd.bankKeeper, ctx, dfd.state.deductFeesFrom, fee) + err := DeductFees(dfd.bankKeeper, ctx, deductFeesFrom, fee) if err != nil { return err } } + if err := dfd.accountKeeper.GetEnvironment().EventService.EventManager(ctx).EmitKV( + sdk.EventTypeTx, + event.NewAttribute(sdk.AttributeKeyFee, fee.String()), + event.NewAttribute(sdk.AttributeKeyFeePayer, sdk.AccAddress(deductFeesFrom).String()), + ); err != nil { + return err + } + return nil } diff --git a/x/auth/ante/validator_tx_fee.go b/x/auth/ante/validator_tx_fee.go index 7733c12d3f5c..50daa5a8cee3 100644 --- a/x/auth/ante/validator_tx_fee.go +++ b/x/auth/ante/validator_tx_fee.go @@ -14,15 +14,20 @@ import ( // checkTxFeeWithValidatorMinGasPrices implements the default fee logic, where the minimum price per // unit of gas is fixed and set by each validator, can the tx priority is computed from the gas price. -func (dfd DeductFeeDecorator) checkTxFeeWithValidatorMinGasPrices(ctx context.Context, _ sdk.Tx) (sdk.Coins, int64, error) { - feeCoins := dfd.state.feeTx.GetFee() - gas := dfd.state.feeTx.GetGas() +func (dfd DeductFeeDecorator) checkTxFeeWithValidatorMinGasPrices(ctx context.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") + } + + feeCoins := feeTx.GetFee() + gas := feeTx.GetGas() // Ensure that the provided fees meet a minimum threshold for the validator, // if this is a CheckTx. This is only for local mempool purposes, and thus // is only ran on check tx. - if dfd.state.execMode == transaction.ExecModeCheck { - minGasPrices := dfd.state.minGasPrices + if dfd.accountKeeper.GetEnvironment().TransactionService.ExecMode(ctx) == transaction.ExecModeCheck { + minGasPrices := dfd.minGasPrices if !minGasPrices.IsZero() { requiredFees := make(sdk.Coins, len(minGasPrices)) From ba0d19939a46dd0900d2f24f100a451a8a3bfd2c Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 28 Aug 2024 13:40:45 +0200 Subject: [PATCH 23/28] updates --- simapp/v2/simdv2/cmd/commands.go | 16 +--------------- x/auth/ante/fee.go | 16 +++++++--------- x/auth/tx/config/depinject.go | 23 +++++++++++------------ 3 files changed, 19 insertions(+), 36 deletions(-) diff --git a/simapp/v2/simdv2/cmd/commands.go b/simapp/v2/simdv2/cmd/commands.go index 78aae574c2b4..fd8e918a9efa 100644 --- a/simapp/v2/simdv2/cmd/commands.go +++ b/simapp/v2/simdv2/cmd/commands.go @@ -70,26 +70,12 @@ func initRootCmd[T transaction.Tx]( offchain.OffChain(), ) - // Allow the chain developer to overwrite the server default - // app toml config. - serverCfg := serverv2.DefaultServerConfig() - // The server's default minimum gas price is set to "0stake" inside - // app.toml. However, the chain developer can set a default app.toml value for their - // validators here. Please update value based on chain denom. - // - // In summary: - // - if you set serverCfg.MinGasPrices value, validators CAN tweak their - // own app.toml to override, or use this default value. - // - // In simapp, we set the min gas prices to 0. - serverCfg.MinGasPrices = "0stake" - // wire server commands if err = serverv2.AddCommands( rootCmd, newApp, logger, - serverCfg, + serverv2.DefaultServerConfig(), cometbft.New(&genericTxDecoder[T]{txConfig}, cometbft.DefaultServerOptions[T]()), grpc.New[T](), store.New[T](newApp), diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index 60da2e89b3cd..2254af5acc5c 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -63,7 +63,7 @@ func (dfd *DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, ne return next(newCtx, tx, false) } -func (dfd *DeductFeeDecorator) innerValidateTx(ctx context.Context, tx sdk.Tx) (txPriority int64, err error) { +func (dfd *DeductFeeDecorator) innerValidateTx(ctx context.Context, tx sdk.Tx) (priority int64, err error) { feeTx, ok := tx.(sdk.FeeTx) if !ok { return 0, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must implement the FeeTx interface") @@ -76,19 +76,19 @@ func (dfd *DeductFeeDecorator) innerValidateTx(ctx context.Context, tx sdk.Tx) ( return 0, errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "must provide positive gas") } - txFee := feeTx.GetFee() + fee := feeTx.GetFee() if execMode != transaction.ExecModeSimulate { - txFee, txPriority, err = dfd.txFeeChecker(ctx, tx) + fee, priority, err = dfd.txFeeChecker(ctx, tx) if err != nil { return 0, err } } - if err := dfd.checkDeductFee(ctx, feeTx, txFee); err != nil { + if err := dfd.checkDeductFee(ctx, feeTx, fee); err != nil { return 0, err } - return txPriority, nil + return priority, nil } // ValidateTx implements an TxValidator for DeductFeeDecorator @@ -134,8 +134,7 @@ func (dfd *DeductFeeDecorator) checkDeductFee(ctx context.Context, feeTx sdk.Fee // deduct the fees if !fee.IsZero() { - err := DeductFees(dfd.bankKeeper, ctx, deductFeesFrom, fee) - if err != nil { + if err := DeductFees(dfd.bankKeeper, ctx, deductFeesFrom, fee); err != nil { return err } } @@ -157,8 +156,7 @@ func DeductFees(bankKeeper types.BankKeeper, ctx context.Context, acc []byte, fe return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) } - err := bankKeeper.SendCoinsFromAccountToModule(ctx, sdk.AccAddress(acc), types.FeeCollectorName, fees) - if err != nil { + if err := bankKeeper.SendCoinsFromAccountToModule(ctx, sdk.AccAddress(acc), types.FeeCollectorName, fees); err != nil { return fmt.Errorf("failed to deduct fees: %w", err) } diff --git a/x/auth/tx/config/depinject.go b/x/auth/tx/config/depinject.go index a7b147792dd5..e4e43ac3d80f 100644 --- a/x/auth/tx/config/depinject.go +++ b/x/auth/tx/config/depinject.go @@ -55,18 +55,17 @@ type ModuleInputs struct { ProtoFileResolver txsigning.ProtoFileResolver Environment appmodule.Environment // BankKeeper is the expected bank keeper to be passed to AnteHandlers / Tx Validators - BankKeeper authtypes.BankKeeper `optional:"true"` - MetadataBankKeeper BankKeeper `optional:"true"` - AccountKeeper ante.AccountKeeper `optional:"true"` - FeeGrantKeeper ante.FeegrantKeeper `optional:"true"` - AccountAbstractionKeeper ante.AccountAbstractionKeeper `optional:"true"` - CustomSignModeHandlers func() []txsigning.SignModeHandler `optional:"true"` - CustomGetSigners []txsigning.CustomGetSigner `optional:"true"` - UnorderedTxManager *unorderedtx.Manager `optional:"true"` - TxFeeChecker ante.TxFeeChecker `optional:"true"` - Viper *viper.Viper `optional:"true"` // server v2 - - ExtraTxValidators []appmodule.TxValidator[transaction.Tx] `optional:"true"` + BankKeeper authtypes.BankKeeper `optional:"true"` + MetadataBankKeeper BankKeeper `optional:"true"` + AccountKeeper ante.AccountKeeper `optional:"true"` + FeeGrantKeeper ante.FeegrantKeeper `optional:"true"` + AccountAbstractionKeeper ante.AccountAbstractionKeeper `optional:"true"` + CustomSignModeHandlers func() []txsigning.SignModeHandler `optional:"true"` + CustomGetSigners []txsigning.CustomGetSigner `optional:"true"` + ExtraTxValidators []appmodule.TxValidator[transaction.Tx] `optional:"true"` + UnorderedTxManager *unorderedtx.Manager `optional:"true"` + TxFeeChecker ante.TxFeeChecker `optional:"true"` + Viper *viper.Viper `optional:"true"` // server v2 } type ModuleOutputs struct { From 8d9dc75a203bf647db8ba6cfbacd0522978841d6 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 28 Aug 2024 13:43:05 +0200 Subject: [PATCH 24/28] correct testnet flag --- simapp/v2/simdv2/cmd/testnet.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/simapp/v2/simdv2/cmd/testnet.go b/simapp/v2/simdv2/cmd/testnet.go index 25bbef8e7440..5ec782389036 100644 --- a/simapp/v2/simdv2/cmd/testnet.go +++ b/simapp/v2/simdv2/cmd/testnet.go @@ -41,7 +41,6 @@ import ( ) var ( - flagMinGasPrices = "minimum-gas-prices" flagNodeDirPrefix = "node-dir-prefix" flagNumValidators = "validator-count" flagOutputDir = "output-dir" @@ -72,7 +71,7 @@ func addTestnetFlagsToCmd(cmd *cobra.Command) { cmd.Flags().IntP(flagNumValidators, "n", 4, "Number of validators to initialize the testnet with") cmd.Flags().StringP(flagOutputDir, "o", "./.testnets", "Directory to store initialization data for the testnet") cmd.Flags().String(flags.FlagChainID, "", "genesis file chain-id, if left blank will be randomly created") - cmd.Flags().String(flagMinGasPrices, fmt.Sprintf("0.000006%s", sdk.DefaultBondDenom), "Minimum gas prices to accept for transactions; All fees in a tx must meet this minimum (e.g. 0.01photino,0.001stake)") + cmd.Flags().String(serverv2.FlagMinGasPrices, fmt.Sprintf("0.000006%s", sdk.DefaultBondDenom), "Minimum gas prices to accept for transactions; All fees in a tx must meet this minimum (e.g. 0.01photino,0.001stake)") cmd.Flags().String(flags.FlagKeyType, string(hd.Secp256k1Type), "Key signing algorithm to generate keys for") // support old flags name for backwards compatibility @@ -129,7 +128,7 @@ Example: args.outputDir, _ = cmd.Flags().GetString(flagOutputDir) args.keyringBackend, _ = cmd.Flags().GetString(flags.FlagKeyringBackend) args.chainID, _ = cmd.Flags().GetString(flags.FlagChainID) - args.minGasPrices, _ = cmd.Flags().GetString(flagMinGasPrices) + args.minGasPrices, _ = cmd.Flags().GetString(serverv2.FlagMinGasPrices) args.nodeDirPrefix, _ = cmd.Flags().GetString(flagNodeDirPrefix) args.nodeDaemonHome, _ = cmd.Flags().GetString(flagNodeDaemonHome) args.startingIPAddress, _ = cmd.Flags().GetString(flagStartingIPAddress) From 8e966e66a5aea92a3e711aca98acb1e5f8216d8d Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 28 Aug 2024 13:51:46 +0200 Subject: [PATCH 25/28] consistent config placement --- simapp/v2/simdv2/cmd/commands.go | 2 +- simapp/v2/simdv2/cmd/config.go | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/simapp/v2/simdv2/cmd/commands.go b/simapp/v2/simdv2/cmd/commands.go index fd8e918a9efa..d9fa1fa54983 100644 --- a/simapp/v2/simdv2/cmd/commands.go +++ b/simapp/v2/simdv2/cmd/commands.go @@ -75,7 +75,7 @@ func initRootCmd[T transaction.Tx]( rootCmd, newApp, logger, - serverv2.DefaultServerConfig(), + initServerConfig(), cometbft.New(&genericTxDecoder[T]{txConfig}, cometbft.DefaultServerOptions[T]()), grpc.New[T](), store.New[T](newApp), diff --git a/simapp/v2/simdv2/cmd/config.go b/simapp/v2/simdv2/cmd/config.go index 0794a8a42a79..c7f6b707c89c 100644 --- a/simapp/v2/simdv2/cmd/config.go +++ b/simapp/v2/simdv2/cmd/config.go @@ -3,6 +3,8 @@ package cmd import ( "strings" + serverv2 "cosmossdk.io/server/v2" + clientconfig "github.com/cosmos/cosmos-sdk/client/config" "github.com/cosmos/cosmos-sdk/crypto/keyring" ) @@ -49,3 +51,20 @@ gas-adjustment = {{ .GasConfig.GasAdjustment }} return customClientConfigTemplate, customClientConfig } + +// Allow the chain developer to overwrite the server default app toml config. +func initServerConfig() serverv2.ServerConfig { + serverCfg := serverv2.DefaultServerConfig() + // The server's default minimum gas price is set to "0stake" inside + // app.toml. However, the chain developer can set a default app.toml value for their + // validators here. Please update value based on chain denom. + // + // In summary: + // - if you set serverCfg.MinGasPrices value, validators CAN tweak their + // own app.toml to override, or use this default value. + // + // In simapp, we set the min gas prices to 0. + serverCfg.MinGasPrices = "0stake" + + return serverCfg +} From 2c3b9d8a8e19087cdf7c38d9f511fa4e8692baa7 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 28 Aug 2024 17:37:12 +0200 Subject: [PATCH 26/28] fix --- x/auth/tx/config/module.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/auth/tx/config/module.go b/x/auth/tx/config/module.go index ecd0235d760a..21c3131a4e00 100644 --- a/x/auth/tx/config/module.go +++ b/x/auth/tx/config/module.go @@ -34,6 +34,7 @@ func NewAppModule( ) AppModule { return AppModule{ sigVerification: sigVerification, + feeTxValidator: feeTxValidator, txValidators: txValidators, } } From 46cef74097fff89d42299195f715b7cd5d748bef Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 28 Aug 2024 22:28:53 +0200 Subject: [PATCH 27/28] ok --- x/auth/ante/fee.go | 10 ++++------ x/auth/ante/validator_tx_fee.go | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index 2254af5acc5c..4d87c6cbe950 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -111,12 +111,10 @@ func (dfd *DeductFeeDecorator) checkDeductFee(ctx context.Context, feeTx sdk.Fee // if feegranter set, deduct fee from feegranter account. // this works only when feegrant is enabled. if feeGranter != nil { - feeGranterAddr := sdk.AccAddress(feeGranter) - if dfd.feegrantKeeper == nil { return sdkerrors.ErrInvalidRequest.Wrap("fee grants are not enabled") - } else if !bytes.Equal(feeGranterAddr, feePayer) { - err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranterAddr, feePayer, fee, feeTx.GetMsgs()) + } else if !bytes.Equal(feeGranter, feePayer) { + err := dfd.feegrantKeeper.UseGrantedFees(ctx, feeGranter, feePayer, fee, feeTx.GetMsgs()) if err != nil { granterAddr, acErr := dfd.accountKeeper.AddressCodec().BytesToString(feeGranter) if acErr != nil { @@ -129,7 +127,7 @@ func (dfd *DeductFeeDecorator) checkDeductFee(ctx context.Context, feeTx sdk.Fee return errorsmod.Wrapf(err, "%s does not allow to pay fees for %s", granterAddr, payerAddr) } } - deductFeesFrom = feeGranterAddr + deductFeesFrom = feeGranter } // deduct the fees @@ -156,7 +154,7 @@ func DeductFees(bankKeeper types.BankKeeper, ctx context.Context, acc []byte, fe return errorsmod.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) } - if err := bankKeeper.SendCoinsFromAccountToModule(ctx, sdk.AccAddress(acc), types.FeeCollectorName, fees); err != nil { + if err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc, types.FeeCollectorName, fees); err != nil { return fmt.Errorf("failed to deduct fees: %w", err) } diff --git a/x/auth/ante/validator_tx_fee.go b/x/auth/ante/validator_tx_fee.go index 50daa5a8cee3..3b00c1ba6c29 100644 --- a/x/auth/ante/validator_tx_fee.go +++ b/x/auth/ante/validator_tx_fee.go @@ -14,7 +14,7 @@ import ( // checkTxFeeWithValidatorMinGasPrices implements the default fee logic, where the minimum price per // unit of gas is fixed and set by each validator, can the tx priority is computed from the gas price. -func (dfd DeductFeeDecorator) checkTxFeeWithValidatorMinGasPrices(ctx context.Context, tx sdk.Tx) (sdk.Coins, int64, error) { +func (dfd *DeductFeeDecorator) checkTxFeeWithValidatorMinGasPrices(ctx context.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") From 0c5f7747f04039f70e5033bc5c79dde042427bb9 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 28 Aug 2024 22:37:04 +0200 Subject: [PATCH 28/28] done --- x/auth/ante/fee.go | 6 +++--- x/auth/ante/validator_tx_fee.go | 2 +- x/auth/tx/config/module.go | 4 ++++ 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index 4d87c6cbe950..248d7163471b 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -16,7 +16,7 @@ import ( // TxFeeChecker checks if the provided fee is enough and returns the effective fee and tx priority. // The effective fee should be deducted later, and the priority should be returned in the ABCI response. -type TxFeeChecker func(ctx context.Context, tx sdk.Tx) (sdk.Coins, int64, error) +type TxFeeChecker func(ctx context.Context, tx transaction.Tx) (sdk.Coins, int64, error) // DeductFeeDecorator deducts fees from the fee payer. The fee payer is the fee granter (if specified) or first signer of the tx. // If the fee payer does not have the funds to pay for the fees, return an InsufficientFunds error. @@ -63,7 +63,7 @@ func (dfd *DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, _ bool, ne return next(newCtx, tx, false) } -func (dfd *DeductFeeDecorator) innerValidateTx(ctx context.Context, tx sdk.Tx) (priority int64, err error) { +func (dfd *DeductFeeDecorator) innerValidateTx(ctx context.Context, tx transaction.Tx) (priority int64, err error) { feeTx, ok := tx.(sdk.FeeTx) if !ok { return 0, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must implement the FeeTx interface") @@ -93,7 +93,7 @@ func (dfd *DeductFeeDecorator) innerValidateTx(ctx context.Context, tx sdk.Tx) ( // ValidateTx implements an TxValidator for DeductFeeDecorator // Note: This method is applicable only for transactions that implement the sdk.FeeTx interface. -func (dfd *DeductFeeDecorator) ValidateTx(ctx context.Context, tx sdk.Tx) error { +func (dfd *DeductFeeDecorator) ValidateTx(ctx context.Context, tx transaction.Tx) error { _, err := dfd.innerValidateTx(ctx, tx) return err } diff --git a/x/auth/ante/validator_tx_fee.go b/x/auth/ante/validator_tx_fee.go index 3b00c1ba6c29..421068175bea 100644 --- a/x/auth/ante/validator_tx_fee.go +++ b/x/auth/ante/validator_tx_fee.go @@ -14,7 +14,7 @@ import ( // checkTxFeeWithValidatorMinGasPrices implements the default fee logic, where the minimum price per // unit of gas is fixed and set by each validator, can the tx priority is computed from the gas price. -func (dfd *DeductFeeDecorator) checkTxFeeWithValidatorMinGasPrices(ctx context.Context, tx sdk.Tx) (sdk.Coins, int64, error) { +func (dfd *DeductFeeDecorator) checkTxFeeWithValidatorMinGasPrices(ctx context.Context, tx transaction.Tx) (sdk.Coins, int64, error) { feeTx, ok := tx.(sdk.FeeTx) if !ok { return nil, 0, errorsmod.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") diff --git a/x/auth/tx/config/module.go b/x/auth/tx/config/module.go index 21c3131a4e00..f4a1207fa7b1 100644 --- a/x/auth/tx/config/module.go +++ b/x/auth/tx/config/module.go @@ -53,5 +53,9 @@ func (a AppModule) TxValidator(ctx context.Context, tx transaction.Tx) error { } } + if err := a.feeTxValidator.ValidateTx(ctx, tx); err != nil { + return err + } + return a.sigVerification.ValidateTx(ctx, tx) }