Add separate min gas price option for txes with high gas wanted (#777)

CHANGELOG.md

@@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Features
 
+- [#724](https://github.com/osmosis-labs/osmosis/pull/724) Make an ante-handler filter for recognizing High gas txs, and having a min gas price for them.
 - [#741](https://github.com/osmosis-labs/osmosis/pull/741) Allow node operators to set a second min gas price for arbitrage txs.
 - [#623](https://github.com/osmosis-labs/osmosis/pull/623) Use gosec for staticly linting for common non-determinism issues in SDK applications.
 

app/ante.go

 package app
 
 import (
-	"fmt"
-
 	servertypes "github.com/cosmos/cosmos-sdk/server/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 
 	"github.com/cosmos/cosmos-sdk/x/auth/signing"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
@@ -28,14 +25,11 @@ func NewAnteHandler(
 	signModeHandler signing.SignModeHandler,
 	channelKeeper channelkeeper.Keeper,
 ) sdk.AnteHandler {
-	mempoolFeeDecorator := txfeeskeeper.NewMempoolFeeDecorator(*txFeesKeeper)
-	// Optional if anyone else is using this repo. Primarily of impact for Osmosis.
-	// TODO: Abstract this better
-	mempoolFeeDecorator.SetArbMinGasFee(parseArbGasFromConfig(appOpts))
+	mempoolFeeOptions := txfeestypes.NewMempoolFeeOptions(appOpts)
+	mempoolFeeDecorator := txfeeskeeper.NewMempoolFeeDecorator(*txFeesKeeper, mempoolFeeOptions)
 	return sdk.ChainAnteDecorators(
 		ante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first
 		ante.NewRejectExtensionOptionsDecorator(),
-		NewMempoolMaxGasPerTxDecorator(),
 		// Use Mempool Fee Decorator from our txfees module instead of default one from auth
 		// https://github.com/cosmos/cosmos-sdk/blob/master/x/auth/middleware/fee.go#L34
 		mempoolFeeDecorator,
@@ -52,57 +46,3 @@ func NewAnteHandler(
 		ibcante.NewAnteDecorator(channelKeeper),
 	)
 }
-
-// TODO: Abstract this function better. We likely need a parse `osmosis-mempool` config section.
-func parseArbGasFromConfig(appOpts servertypes.AppOptions) sdk.Dec {
-	arbMinFeeInterface := appOpts.Get("osmosis-mempool.arbitrage-min-gas-fee")
-	arbMinFee := txfeeskeeper.DefaultArbMinGasFee
-	if arbMinFeeInterface != nil {
-		arbMinFeeStr, ok := arbMinFeeInterface.(string)
-		if !ok {
-			panic("invalidly configured osmosis-mempool.arbitrage-min-gas-fee")
-		}
-		var err error
-		// pre-pend 0 to allow the config to start with a decimal, e.g. ".01"
-		arbMinFee, err = sdk.NewDecFromStr("0" + arbMinFeeStr)
-		if err != nil {
-			panic(fmt.Errorf("invalidly configured osmosis-mempool.arbitrage-min-gas-fee, err= %v", err))
-		}
-	}
-	return arbMinFee
-}
-
-// NewMempoolMaxGasPerTxDecorator will check if the transaction's gas
-// is greater than the local validator's max_gas_wanted_per_tx.
-// TODO: max_gas_per_tx is hardcoded here, should move to being defined in app.toml.
-// If gas_wanted is too high, decorator returns error and tx is rejected from mempool.
-// Note this only applies when ctx.CheckTx = true
-// If gas is sufficiently low or not CheckTx, then call next AnteHandler
-// CONTRACT: Tx must implement FeeTx to use MempoolMaxGasPerTxDecorator
-type MempoolMaxGasPerTxDecorator struct{}
-
-func NewMempoolMaxGasPerTxDecorator() MempoolMaxGasPerTxDecorator {
-	return MempoolMaxGasPerTxDecorator{}
-}
-
-func (mgd MempoolMaxGasPerTxDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) {
-	feeTx, ok := tx.(sdk.FeeTx)
-	if !ok {
-		return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx")
-	}
-
-	gas := feeTx.GetGas()
-	// maximum gas wanted per tx set to 25M
-	max_gas_wanted_per_tx := uint64(25000000)
-
-	// Ensure that the provided gas is less than the maximum gas per tx.
-	// if this is a CheckTx. This is only for local mempool purposes, and thus
-	// is only ran on check tx.
-	if ctx.IsCheckTx() && !simulate {
-		if gas > max_gas_wanted_per_tx {
-			return ctx, sdkerrors.Wrapf(sdkerrors.ErrOutOfGas, "Too much gas wanted: %d, maximum is 25,000,000", gas)
-		}
-	}
-
-	return next(ctx, tx, simulate)
-}

cmd/osmosisd/cmd/root.go

@@ -110,9 +110,17 @@ func initAppConfig() (string, interface{}) {
 ###############################################################################
 
 [osmosis-mempool]
+# This is the max allowed gas any tx. 
+# This is only for local mempool purposes, and thus	is only ran on check tx.
+max-gas-wanted-per-tx = "25000000"
+
 # This is the minimum gas fee any arbitrage tx should have, denominated in uosmo per gas
 # Default value of ".005" then means that a tx with 1 million gas costs (.005 uosmo/gas) * 1_000_000 gas = .005 osmo
 arbitrage-min-gas-fee = ".005"
+
+# This is the minimum gas fee any tx with high gas demand should have, denominated in uosmo per gas
+# Default value of ".0025" then means that a tx with 1 million gas costs (.0025 uosmo/gas) * 1_000_000 gas = .0025 osmo
+min-gas-price-for-high-gas-tx = ".0025"
 `
 
 	return OsmosisAppTemplate, OsmosisAppCfg

x/txfees/README.md

@@ -28,6 +28,8 @@ Currently the only supported metadata & spot price calculator is using a GAMM po
   * Contains both JoinPool and ExitPool messages in one tx.
     * Has some false positives.
   * These false positives seem like they primarily will get hit during batching of many distinct operations, not really in one atomic action.
+* A max wanted gas per any tx can be set to filter out attack txes.
+* If tx wanted gas > than predefined threshold of 1M, then separate 'min-gas-price-for-high-gas-tx' option used to calculate min gas price.
 
 ## New SDK messages
 

x/txfees/keeper/feedecorator.go

@@ -8,11 +8,6 @@ import (
 	"github.com/osmosis-labs/osmosis/x/txfees/types"
 )
 
-// DefaultArbMinGasFee if its not set in a config somewhere.
-// currently 0 uosmo/gas to preserve functionality with old node software
-// TODO: Bump after next minor version. (in 6.2+)
-var DefaultArbMinGasFee sdk.Dec = sdk.ZeroDec()
-
 // MempoolFeeDecorator will check if the transaction's fee is at least as large
 // as the local validator's minimum gasFee (defined in validator config).
 // If fee is too low, decorator returns error and tx is rejected from mempool.
@@ -20,21 +15,17 @@ var DefaultArbMinGasFee sdk.Dec = sdk.ZeroDec()
 // If fee is high enough or not CheckTx, then call next AnteHandler
 // CONTRACT: Tx must implement FeeTx to use MempoolFeeDecorator
 type MempoolFeeDecorator struct {
-	TxFeesKeeper       Keeper
-	ConfigArbMinGasFee sdk.Dec
+	TxFeesKeeper Keeper
+	Opts         types.MempoolFeeOptions
 }
 
-func NewMempoolFeeDecorator(txFeesKeeper Keeper) MempoolFeeDecorator {
+func NewMempoolFeeDecorator(txFeesKeeper Keeper, opts types.MempoolFeeOptions) MempoolFeeDecorator {
 	return MempoolFeeDecorator{
-		TxFeesKeeper:       txFeesKeeper,
-		ConfigArbMinGasFee: DefaultArbMinGasFee.Clone(),
+		TxFeesKeeper: txFeesKeeper,
+		Opts:         opts,
 	}
 }
 
-func (mfd *MempoolFeeDecorator) SetArbMinGasFee(dec sdk.Dec) {
-	mfd.ConfigArbMinGasFee = dec
-}
-
 func (mfd MempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) {
 	// The SDK currently requires all txs to be FeeTx's in CheckTx, within its mempool fee decorator.
 	// See: https://github.com/cosmos/cosmos-sdk/blob/f726a2398a26bdaf71d78dbf56a82621e84fd098/x/auth/middleware/fee.go#L34-L37
@@ -44,6 +35,16 @@ func (mfd MempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b
 		return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx")
 	}
 
+	// Ensure that the provided gas is less than the maximum gas per tx,
+	// if this is a CheckTx. This is only for local mempool purposes, and thus
+	// is only ran on check tx.
+	if ctx.IsCheckTx() && !simulate {
+		if feeTx.GetGas() > mfd.Opts.MaxGasWantedPerTx {
+			msg := "Too much gas wanted: %d, maximum is %d"
+			return ctx, sdkerrors.Wrapf(sdkerrors.ErrOutOfGas, msg, feeTx.GetGas(), mfd.Opts.MaxGasWantedPerTx)
+		}
+	}
+
 	feeCoins := feeTx.GetFee()
 
 	if len(feeCoins) > 1 {
@@ -71,7 +72,7 @@ func (mfd MempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b
 	// So we ensure that the provided fees meet a minimum threshold for the validator,
 	// converting every non-osmo specified asset into an osmo-equivalent amount, to determine sufficiency.
 	if (ctx.IsCheckTx() || ctx.IsReCheckTx()) && !simulate {
-		minBaseGasPrice := mfd.GetMinBaseGasPriceForTx(ctx, baseDenom, tx)
+		minBaseGasPrice := mfd.GetMinBaseGasPriceForTx(ctx, baseDenom, feeTx)
 		if !(minBaseGasPrice.IsZero()) {
 			if len(feeCoins) != 1 {
 				return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "no fee attached")
@@ -108,11 +109,13 @@ func (k Keeper) IsSufficientFee(ctx sdk.Context, minBaseGasPrice sdk.Dec, gasReq
 	return nil
 }
 
-func (mfd MempoolFeeDecorator) GetMinBaseGasPriceForTx(ctx sdk.Context, baseDenom string, tx sdk.Tx) sdk.Dec {
+func (mfd MempoolFeeDecorator) GetMinBaseGasPriceForTx(ctx sdk.Context, baseDenom string, tx sdk.FeeTx) sdk.Dec {
 	cfgMinGasPrice := ctx.MinGasPrices().AmountOf(baseDenom)
-
+	if tx.GetGas() >= mfd.Opts.HighGasTxThreshold {
+		cfgMinGasPrice = sdk.MaxDec(cfgMinGasPrice, mfd.Opts.MinGasPriceForHighGasTx)
+	}
 	if txfee_filters.IsArbTxLoose(tx) {
-		return sdk.MaxDec(cfgMinGasPrice, mfd.ConfigArbMinGasFee)
+		cfgMinGasPrice = sdk.MaxDec(cfgMinGasPrice, mfd.Opts.MinGasPriceForArbitrageTx)
 	}
 	return cfgMinGasPrice
 }

x/txfees/keeper/feedecorator_test.go

@@ -2,6 +2,7 @@ package keeper_test
 
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/osmosis-labs/osmosis/x/txfees/types"
 
 	"github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx"
 
@@ -11,6 +12,8 @@ import (
 func (suite *KeeperTestSuite) TestFeeDecorator() {
 	suite.SetupTest(false)
 
+	mempoolFeeOpts := types.NewDefaultMempoolFeeOptions()
+	mempoolFeeOpts.MinGasPriceForHighGasTx = sdk.MustNewDecFromStr("0.0025")
 	baseDenom, _ := suite.app.TxFeesKeeper.GetBaseDenom(suite.ctx)
 
 	uion := "uion"
@@ -131,6 +134,30 @@ func (suite *KeeperTestSuite) TestFeeDecorator() {
 			isCheckTx:    true,
 			expectPass:   true,
 		},
+		{
+			name:         "tx with gas wanted more than allowed should not pass",
+			txFee:        sdk.NewCoins(sdk.NewInt64Coin(uion, 100000000)),
+			minGasPrices: sdk.NewDecCoins(sdk.NewInt64DecCoin(uion, 1)),
+			gasRequested: mempoolFeeOpts.MaxGasWantedPerTx + 1,
+			isCheckTx:    true,
+			expectPass:   false,
+		},
+		{
+			name:         "tx with high gas and not enough fee should no pass",
+			txFee:        sdk.NewCoins(sdk.NewInt64Coin(uion, 1)),
+			minGasPrices: sdk.NewDecCoins(sdk.NewInt64DecCoin(uion, 1)),
+			gasRequested: mempoolFeeOpts.HighGasTxThreshold,
+			isCheckTx:    true,
+			expectPass:   false,
+		},
+		{
+			name:         "tx with high gas and enough fee should pass",
+			txFee:        sdk.NewCoins(sdk.NewInt64Coin(uion, 10*1000)),
+			minGasPrices: sdk.NewDecCoins(sdk.NewInt64DecCoin(uion, 1)),
+			gasRequested: mempoolFeeOpts.HighGasTxThreshold,
+			isCheckTx:    true,
+			expectPass:   true,
+		},
 	}
 
 	for _, tc := range tests {
@@ -143,7 +170,7 @@ func (suite *KeeperTestSuite) TestFeeDecorator() {
 			tc.txFee,
 		), []legacytx.StdSignature{}, "")
 
-		mfd := keeper.NewMempoolFeeDecorator(*suite.app.TxFeesKeeper)
+		mfd := keeper.NewMempoolFeeDecorator(*suite.app.TxFeesKeeper, mempoolFeeOpts)
 		antehandler := sdk.ChainAnteDecorators(mfd)
 		_, err := antehandler(suite.ctx, tx, false)
 		if tc.expectPass {

x/txfees/types/options.go

+package types
+
+import (
+	"fmt"
+	servertypes "github.com/cosmos/cosmos-sdk/server/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/spf13/cast"
+)
+
+// If Options are not set in a config somewhere,
+// use defaults to preserve functionality with old node software
+
+// TODO: Bump after next minor version. (in 6.2+)
+var DefaultMinGasPriceForArbitrageTx = sdk.ZeroDec()
+var DefaultMinGasPriceForHighGasTx = sdk.ZeroDec()
+var DefaultMaxGasWantedPerTx = uint64(25 * 1000 * 1000)
+var DefaultHighGasTxThreshold = uint64(1 * 1000 * 1000)
+
+type MempoolFeeOptions struct {
+	MaxGasWantedPerTx         uint64
+	MinGasPriceForArbitrageTx sdk.Dec
+	HighGasTxThreshold        uint64
+	MinGasPriceForHighGasTx   sdk.Dec
+}
+
+func NewDefaultMempoolFeeOptions() MempoolFeeOptions {
+	return MempoolFeeOptions{
+		MaxGasWantedPerTx:         DefaultMaxGasWantedPerTx,
+		MinGasPriceForArbitrageTx: DefaultMinGasPriceForArbitrageTx.Clone(),
+		HighGasTxThreshold:        DefaultHighGasTxThreshold,
+		MinGasPriceForHighGasTx:   DefaultMinGasPriceForHighGasTx.Clone(),
+	}
+}
+
+func NewMempoolFeeOptions(opts servertypes.AppOptions) MempoolFeeOptions {
+	return MempoolFeeOptions{
+		MaxGasWantedPerTx:         parseMaxGasWantedPerTx(opts),
+		MinGasPriceForArbitrageTx: parseMinGasPriceForArbitrageTx(opts),
+		HighGasTxThreshold:        DefaultHighGasTxThreshold,
+		MinGasPriceForHighGasTx:   parseMinGasPriceForHighGasTx(opts),
+	}
+}
+
+func parseMaxGasWantedPerTx(opts servertypes.AppOptions) uint64 {
+	valueInterface := opts.Get("osmosis-mempool.max-gas-wanted-per-tx")
+	if valueInterface == nil {
+		return DefaultMaxGasWantedPerTx
+	}
+	value, err := cast.ToUint64E(valueInterface)
+	if err != nil {
+		panic("invalidly configured osmosis-mempool.max-gas-wanted-per-tx")
+	}
+	return value
+}
+
+func parseMinGasPriceForArbitrageTx(opts servertypes.AppOptions) sdk.Dec {
+	return parseDecFromConfig(opts, "arbitrage-min-gas-fee", DefaultMinGasPriceForArbitrageTx.Clone())
+}
+
+func parseMinGasPriceForHighGasTx(opts servertypes.AppOptions) sdk.Dec {
+	return parseDecFromConfig(opts, "min-gas-price-for-high-gas-tx", DefaultMinGasPriceForHighGasTx.Clone())
+}
+
+func parseDecFromConfig(opts servertypes.AppOptions, optName string, defaultValue sdk.Dec) sdk.Dec {
+	valueInterface := opts.Get("osmosis-mempool." + optName)
+	value := defaultValue
+	if valueInterface != nil {
+		valueStr, ok := valueInterface.(string)
+		if !ok {
+			panic("invalidly configured osmosis-mempool." + optName)
+		}
+		var err error
+		// pre-pend 0 to allow the config to start with a decimal, e.g. ".01"
+		value, err = sdk.NewDecFromStr("0" + valueStr)
+		if err != nil {
+			panic(fmt.Errorf("invalidly configured osmosis-mempool.%v, err= %v", optName, err))
+		}
+	}
+	return value
+}