feat: setting protocol controlled min gas price (#1326)

## Description Closes: #1214 This is a "v1" solution for requiring min-gas price. More flexible mechanism ([#1017](#1017)) we will develop for v4 (or v5). Notes: + added `fee-notes.md` files with stats for the fees. --- ### Author Checklist _All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues._ I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] added appropriate labels to the PR - [ ] targeted the correct branch (see [PR Targeting](https://github.com/umee-network/umee/blob/main/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist _All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items._ I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)
umee-network · Sep 8, 2022 · 49a5dbd · 49a5dbd
1 parent 9675193
commit 49a5dbd
Show file tree

Hide file tree

Showing 56 changed files with 687 additions and 388 deletions.
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -137,7 +137,7 @@ jobs:
 
   mainnet-upgrade-v1-v3:
     runs-on: ubuntu-latest
-    timeout-minutes: 10
+    timeout-minutes: 20
     steps:
       - uses: actions/checkout@v3
       - uses: technote-space/get-diff-action@v6.1.0

diff --git a/.gitignore b/.gitignore
@@ -20,6 +20,8 @@ contrib/scripts/cosmos-genesis-tinkerer
 contrib/scripts/tinkered_genesis.json
 contrib/scripts/preprocessing.json
 contrib/scripts/umeed-releases
+# preprocessing can appear from the shell curdir that is cosmos-genesis-tinkerer is running
+preprocessing.json
 
 # Dependency directories (remove the comment below to include it)
 # vendor/

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -46,6 +46,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
+### State Machine Breaking
+
+- [#1326](https://github.com/umee-network/umee/pull/1326) Setting protocol controlled min gas price.
+
 ### API Breaking
 
 - [1029](https://github.com/umee-network/umee/pull/1029) Removed MsgSetCollateral(addr,denom,bool), and replaced with MsgAddCollateral(addr,coin) and MsgRemoveCollateral(addr,coin)

diff --git a/ante/fee-notes.md b/ante/fee-notes.md
@@ -0,0 +1,56 @@
+# Notes about Gas prices and gas used
+
+NOTES:
+
+- the gas used depends on the current state of the chain
+- the gas records below are based on the empty state
+- user is charged the gas-limit, even if tx consumed less gas
+
+| operation                                      | gas used |
+| :--------------------------------------------- | -------: |
+| x/bank send                                    |   31'029 |
+| x/group create                                 |   68'908 |
+| x/oracle MsgAggregateExchangeRateVote (3 curr) |   66'251 |
+| x/oracle MsgAggregateExchangeRateVote (6 curr) |   69'726 |
+| default gas limit                              |  200'000 |
+
+## Target price (in USD cent) for x/bank send
+
+| umee price (usd cent) | gas price in uumee | fee (usd cent) |
+| --------------------: | -----------------: | -------------: |
+|                     2 |                0.2 |         0.0124 |
+|                     2 |                0.1 |         0.0062 |
+|                     2 |               0.02 |        0.00124 |
+|                     5 |                0.2 |          0.031 |
+|                     5 |                0.1 |         0.0155 |
+|                     5 |               0.02 |         0.0031 |
+
+## Target price (in USD cent) for validator oracle txs (with 6 currencies) per day
+
+There are roughly 10tx / minute and 14400 per day.
+Validator has to do 2 tx (prevote and vote) every 5 blocks.
+Validator will need to do `5760 = 2*14400/5` tx.
+In table below we assume the same price scheme as above, but in the code most likely we will apply a fixed discount (eg 10x).
+
+The prices are indicative. For some transactions (especially oracle) fees can be disabled.
+See fee.go file for details.
+
+| umee price (usd cent) | gas price in uumee | fee (usd cent) |
+| --------------------: | -----------------: | -------------: |
+|                     2 |                0.2 |         161.28 |
+|                     2 |                0.1 |          80.64 |
+|                     2 |               0.02 |         16.128 |
+|                     5 |                0.2 |          403.2 |
+|                     5 |                0.1 |          201.6 |
+|                     5 |               0.02 |          40.32 |
+
+## Target price (in USD) for default gas limit
+
+| umee price (usd cent) | gas price in uumee | fee (usd cent) |
+| --------------------: | -----------------: | -------------: |
+|                     2 |                0.2 |           0.08 |
+|                     2 |                0.1 |           0.04 |
+|                     2 |               0.02 |          0.008 |
+|                     5 |                0.2 |            0.2 |
+|                     5 |                0.1 |            0.1 |
+|                     5 |               0.02 |           0.02 |
diff --git a/ante/fee.go b/ante/fee.go
@@ -6,12 +6,13 @@ import (
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types"
 
+	appparams "github.com/umee-network/umee/v3/app/params"
 	leveragetypes "github.com/umee-network/umee/v3/x/leverage/types"
 	oracletypes "github.com/umee-network/umee/v3/x/oracle/types"
 )
 
 // MaxMsgGasUsage defines the maximum gas allowed for an oracle transaction.
-const MaxMsgGasUsage = uint64(100000)
+const MaxMsgGasUsage = uint64(100_000)
 
 // FeeAndPriority ensures tx has enough fee coins to pay for the gas at the CheckTx time
 // to early remove transactions from the mempool without enough attached fee.
@@ -24,37 +25,55 @@ func FeeAndPriority(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) {
 		return nil, 0, sdkerrors.ErrTxDecode.Wrap("Tx must be a FeeTx")
 	}
 
-	feeCoins := feeTx.GetFee()
-	gas := feeTx.GetGas()
+	providedFees := feeTx.GetFee()
+	gasLimit := feeTx.GetGas()
 	msgs := feeTx.GetMsgs()
 	isOracleOrGravity := IsOracleOrGravityTx(msgs)
-	chargeFees := !isOracleOrGravity || gas > uint64(len(msgs))*MaxMsgGasUsage
-
-	if ctx.IsCheckTx() && chargeFees {
-		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 := sdk.NewDec(int64(gas))
-			for i, gp := range minGasPrices {
-				fee := gp.Amount.Mul(glDec)
-				requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt())
-			}
+	priority := getTxPriority(isOracleOrGravity, msgs)
+	chargeFees := !isOracleOrGravity || gasLimit > uint64(len(msgs))*MaxMsgGasUsage
+	// we also don't charge transaction fees for the first block, for the genesis transactions.
+	if !chargeFees || ctx.BlockHeight() == 0 {
+		return sdk.Coins{}, priority, nil
+	}
 
-			if !feeCoins.IsAnyGTE(requiredFees) {
-				return nil, 0, sdkerrors.ErrInsufficientFee.Wrapf(
-					"insufficient fees; got: %s required: %s", feeCoins, requiredFees)
-			}
+	var err error
+	if ctx.IsCheckTx() {
+		err = checkFees(ctx.MinGasPrices(), providedFees, gasLimit)
+	} else {
+		err = checkFees(nil, providedFees, gasLimit)
+	}
+	if err != nil {
+		err = sdkerrors.Wrap(err, msgs[0].String())
+	}
+	return providedFees, priority, err
+}
+
+func checkFees(minGasPrices sdk.DecCoins, fees sdk.Coins, gasLimit uint64) error {
+	if minGasPrices != nil {
+		// check minGasPrices set by validator
+		if err := AssertMinProtocolGasPrice(minGasPrices); err != nil {
+			return err
 		}
+	} else {
+		// in deliverTx = use protocol min gas price
+		minGasPrices = sdk.DecCoins{appparams.MinMinGasPrice}
 	}
 
-	priority := getTxPriority(isOracleOrGravity, msgs)
-	if !chargeFees {
-		return sdk.Coins{}, priority, 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 := sdk.NewDec(int64(gasLimit))
+	for i, gp := range minGasPrices {
+		fee := gp.Amount.Mul(glDec)
+		requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt())
 	}
-	return feeCoins, priority, nil
+
+	if !fees.IsAnyGTE(requiredFees) {
+		return sdkerrors.ErrInsufficientFee.Wrapf(
+			"insufficient fees; got: %s required: %s", fees, requiredFees)
+	}
+	return nil
 }
 
 // IsOracleOrGravityTx checks if all messages are oracle messages
@@ -68,9 +87,8 @@ func IsOracleOrGravityTx(msgs []sdk.Msg) bool {
 			*oracletypes.MsgAggregateExchangeRateVote:
 			continue
 
-		// TODO: remove messages which should not be "free":
+		// TODO: revisit free gravity msg set
 		case *gbtypes.MsgValsetConfirm,
-			*gbtypes.MsgRequestBatch,
 			*gbtypes.MsgConfirmBatch,
 			*gbtypes.MsgERC20DeployedClaim,
 			*gbtypes.MsgConfirmLogicCall,
@@ -90,6 +108,21 @@ func IsOracleOrGravityTx(msgs []sdk.Msg) bool {
 	return true
 }
 
+// AssertMinProtocolGasPrice returns an error if the provided gasPrices are lower then
+// the required by protocol.
+func AssertMinProtocolGasPrice(gasPrices sdk.DecCoins) error {
+	for _, c := range gasPrices {
+		if c.Denom == appparams.MinMinGasPrice.Denom {
+			if c.Amount.LT(appparams.MinMinGasPrice.Amount) {
+				break // go to error below
+			}
+			return nil
+		}
+	}
+	return sdkerrors.ErrInsufficientFee.Wrapf(
+		"gas price too small; got: %v required min: %v", gasPrices, appparams.MinMinGasPrice)
+}
+
 // getTxPriority returns naive tx priority based on the lowest fee amount (regardless of the
 // denom) and oracle tx check.
 // Dirty optimization: since we already check if msgs are oracle or gravity messages, then we

diff --git a/ante/fee_test.go b/ante/fee_test.go
@@ -5,8 +5,11 @@ import (
 	"github.com/cosmos/cosmos-sdk/testutil/testdata"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	signing "github.com/cosmos/cosmos-sdk/x/auth/signing"
 
 	"github.com/umee-network/umee/v3/ante"
+	appparams "github.com/umee-network/umee/v3/app/params"
+	"github.com/umee-network/umee/v3/util/coin"
 	oracletypes "github.com/umee-network/umee/v3/x/oracle/types"
 )
 
@@ -18,32 +21,73 @@ func (suite *IntegrationTestSuite) TestFeeAndPriority() {
 
 	msgs := testdata.NewTestMsg(addr1)
 	require.NoError(suite.txBuilder.SetMsgs(msgs))
-	fee := sdk.NewCoins(sdk.NewInt64Coin("atom", 150))
-	gasLimit := 200000
-	suite.txBuilder.SetFeeAmount(fee)
-	suite.txBuilder.SetGasLimit(uint64(gasLimit))
-
-	// Test1: validator min gas price check
-	// Ante should fail when validator min gas price is above the transaction gas limit
-	minGasPrice := sdk.NewDecCoinFromDec("atom", sdk.NewDecFromInt(fee[0].Amount).QuoInt64(int64(gasLimit/2)))
+	minGas := appparams.MinMinGasPrice
+	mkFee := func(factor string) sdk.Coins {
+		return coin.NewDecBld(minGas).Scale(int64(appparams.DefaultGasLimit)).ScaleStr(factor).ToCoins()
+	}
+	mkGas := func(denom, factor string) sdk.DecCoins {
+		if denom == "" {
+			denom = minGas.Denom
+		}
+		f := sdk.MustNewDecFromStr(factor)
+		return sdk.DecCoins{sdk.NewDecCoinFromDec(denom, minGas.Amount.Mul(f))}
+	}
+	mkTx := func(fee sdk.Coins) signing.Tx {
+		suite.txBuilder.SetFeeAmount(fee)
+		tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID())
+		require.NoError(err)
+		return tx
+	}
+
+	suite.txBuilder.SetGasLimit(appparams.DefaultGasLimit)
+	// we set fee to 2*gasLimit*minGasPrice
+	fee := mkFee("2")
+	tx := mkTx(fee)
+
+	//
+	// Test CheckTX
+	//
 	ctx := suite.ctx.
-		WithMinGasPrices([]sdk.DecCoin{minGasPrice}).
+		WithMinGasPrices(sdk.DecCoins{minGas}).
 		WithIsCheckTx(true)
-	tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID())
-	require.NoError(err)
-	_, _, err = ante.FeeAndPriority(ctx, tx)
-	require.ErrorIs(sdkerrors.ErrInsufficientFee, err)
 
-	// Test2: min gas price not checked in DeliverTx
-	ctx = suite.ctx.WithIsCheckTx(false)
+	// min-gas-settings should work
 	suite.checkFeeAnte(tx, fee, ctx)
 
-	// Test3: should not error when min gas price is same or lower than the fee
-	ctx = ctx.WithMinGasPrices(sdk.NewDecCoinsFromCoins(fee...))
-	suite.checkFeeAnte(tx, fee, ctx)
+	// should work when exact fee is provided
+	suite.checkFeeAnte(tx, fee, ctx.WithMinGasPrices(mkGas("", "2")))
+
+	// should fail when not enough fee is provided
+	suite.checkFeeFailed(tx, ctx.WithMinGasPrices(mkGas("", "3")))
 
-	ctx = ctx.WithMinGasPrices([]sdk.DecCoin{sdk.NewDecCoin(fee[0].Denom, fee[0].Amount.QuoRaw(2))})
+	// should fail when other denom is required
+	suite.checkFeeFailed(tx, ctx.WithMinGasPrices(mkGas("other", "1")))
+
+	// should fail when some fee doesn't include all gas denoms
+	ctx = ctx.WithMinGasPrices(sdk.DecCoins{minGas,
+		sdk.NewDecCoinFromDec("other", sdk.NewDec(10))})
+	suite.checkFeeFailed(tx, ctx)
+
+	//
+	// Test DeliverTx
+	//
+	ctx = suite.ctx.
+		WithMinGasPrices(sdk.DecCoins{minGas}).
+		WithIsCheckTx(false)
+
+	// ctx.MinGasPrice shouldn't matter
 	suite.checkFeeAnte(tx, fee, ctx)
+	suite.checkFeeAnte(tx, fee, ctx.WithMinGasPrices(mkGas("", "3")))
+	suite.checkFeeAnte(tx, fee, ctx.WithMinGasPrices(mkGas("other", "1")))
+	suite.checkFeeAnte(tx, fee, ctx.WithMinGasPrices(sdk.DecCoins{}))
+
+	// should fail when not enough fee is provided
+	suite.checkFeeFailed(mkTx(mkFee("0.5")), ctx)
+	suite.checkFeeFailed(mkTx(sdk.Coins{}), ctx)
+
+	// should  work when more fees are applied
+	fee = append(fee, sdk.NewInt64Coin("other", 10))
+	suite.checkFeeAnte(mkTx(fee), fee, ctx)
 
 	// Test4: ensure no fees for oracle msgs
 	require.NoError(suite.txBuilder.SetMsgs(
@@ -62,6 +106,11 @@ func (suite *IntegrationTestSuite) TestFeeAndPriority() {
 	suite.checkFeeAnte(oracleTx, sdk.Coins{}, suite.ctx.WithIsCheckTx(false))
 }
 
+func (suite *IntegrationTestSuite) checkFeeFailed(tx sdk.Tx, ctx sdk.Context) {
+	_, _, err := ante.FeeAndPriority(ctx, tx)
+	suite.Require().ErrorIs(sdkerrors.ErrInsufficientFee, err)
+}
+
 func (suite *IntegrationTestSuite) checkFeeAnte(tx sdk.Tx, feeExpected sdk.Coins, ctx sdk.Context) {
 	require := suite.Require()
 	fee, _, err := ante.FeeAndPriority(ctx, tx)

diff --git a/ante/spam_prevention.go b/ante/spam_prevention.go
@@ -64,6 +64,7 @@ func (spd *SpamPreventionDecorator) CheckOracleSpam(ctx sdk.Context, msgs []sdk.
 			err = spd.validate(ctx, msg.Feeder, msg.Validator, spd.oracleVoteMap, curHeight, "vote")
 		default:
 			// non oracle msg: stop validation!
+			// NOTE: only tx which contains only oracle Msgs are considered oracle-prioritized
 			return nil
 		}
 		if err != nil {