From d3333418c5b80176c9d0d0bf1e25b20a0f9a7798 Mon Sep 17 00:00:00 2001 From: Daniel Burckhardt Date: Wed, 22 Jun 2022 12:57:16 +0200 Subject: [PATCH] bug(feemarket): set lower bound of base fee to min gas price param (#1135) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * bug(feemarket): set lower bound of base fee to min gas price param) * fix * bug(feemarket): flag necessary improvement to integration tests, as the baseFee changes for every test * bug(feemarket): add unit tests for CalculateBaseFee * bug(feemarket): move integration test setup out of Describe block * wip fix tests * bug(feemarket): fix integration tests * bug(feemarket): wip improve specs * bug(feemarket): add spec concepts * bug(feemarket): remove todo * bug(feemarket): remove changes used for debugging in params * bug(feemarket): remove todo in integration test * add changelog * address PR comments Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> --- CHANGELOG.md | 9 +- app/ante/fees.go | 5 +- x/feemarket/keeper/eip1559.go | 17 +- x/feemarket/keeper/eip1559_test.go | 131 +++---- x/feemarket/keeper/integration_test.go | 500 +++++++++++++------------ x/feemarket/spec/01_concepts.md | 67 +++- 6 files changed, 394 insertions(+), 335 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c79a2bf024..ceccca5569 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,7 +41,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### State Machine Breaking * (evm) [\#1128](https://github.com/tharsis/ethermint/pull/1128) Clear tx logs if tx failed in post processing hooks -* (evm) [tharsis#1124](https://github.com/tharsis/ethermint/pull/1124) Reject non-replay-protected tx in `AnteHandler` to prevent replay attack +* (evm) [\#1124](https://github.com/tharsis/ethermint/pull/1124) Reject non-replay-protected tx in `AnteHandler` to prevent replay attack ### API Breaking @@ -50,13 +50,14 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* (feemarket) [\#1120](https://github.com/evmos/ethermint/pull/1120) Make `min-gas-multiplier` parameter accept zero value +* (feemarket) [\#1135](https://github.com/evmos/ethermint/pull/1135) Set lower bound of base fee to min gas price param * (evm) [\#1142](https://github.com/evmos/ethermint/pull/1142) Rename `RejectUnprotectedTx` to `AllowUnprotectedTxs` for consistency with go-ethereum. -* (feemarket) [tharsis#1120](https://github.com/evmos/ethermint/pull/1120) Make `min-gas-multiplier` parameter accept zero value ### Bug Fixes -* (evm) [tharsis#1118](https://github.com/evmos/ethermint/pull/1118) Fix `Type()` `Account` method `EmptyCodeHash` comparison -* (rpc) [tharsis#1138](https://github.com/evmos/ethermint/pull/1138) Fix GasPrice calculation with relation to `MinGasPrice` +* (evm) [\#1118](https://github.com/evmos/ethermint/pull/1118) Fix `Type()` `Account` method `EmptyCodeHash` comparison +* (rpc) [\#1138](https://github.com/evmos/ethermint/pull/1138) Fix GasPrice calculation with relation to `MinGasPrice` ## [v0.16.0] - 2022-06-06 diff --git a/app/ante/fees.go b/app/ante/fees.go index d2b130343d..c05470048a 100644 --- a/app/ante/fees.go +++ b/app/ante/fees.go @@ -125,14 +125,15 @@ func (empd EthMinGasPriceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul } gasLimit := sdk.NewDecFromBigInt(new(big.Int).SetUint64(ethMsg.GetGas())) + requiredFee := minGasPrice.Mul(gasLimit) fee := sdk.NewDecFromBigInt(feeAmt) if fee.LT(requiredFee) { return ctx, sdkerrors.Wrapf( sdkerrors.ErrInsufficientFee, - "provided fee < minimum global fee (%s < %s). Please increase the priority tip (for EIP-1559 txs) or the gas prices (for access list or legacy txs)", - fee, requiredFee, + "provided fee < minimum global fee (%d < %d). Please increase the priority tip (for EIP-1559 txs) or the gas prices (for access list or legacy txs)", + fee.TruncateInt().Int64(), requiredFee.TruncateInt().Int64(), ) } } diff --git a/x/feemarket/keeper/eip1559.go b/x/feemarket/keeper/eip1559.go index 161f058d27..8cef65154d 100644 --- a/x/feemarket/keeper/eip1559.go +++ b/x/feemarket/keeper/eip1559.go @@ -58,13 +58,15 @@ func (k Keeper) CalculateBaseFee(ctx sdk.Context) *big.Int { parentGasTarget := parentGasTargetBig.Uint64() baseFeeChangeDenominator := new(big.Int).SetUint64(uint64(params.BaseFeeChangeDenominator)) - // If the parent gasUsed is the same as the target, the baseFee remains unchanged. + // If the parent gasUsed is the same as the target, the baseFee remains + // unchanged. if parentGasUsed == parentGasTarget { return new(big.Int).Set(parentBaseFee) } if parentGasUsed > parentGasTarget { - // If the parent block used more gas than its target, the baseFee should increase. + // If the parent block used more gas than its target, the baseFee should + // increase. gasUsedDelta := new(big.Int).SetUint64(parentGasUsed - parentGasTarget) x := new(big.Int).Mul(parentBaseFee, gasUsedDelta) y := x.Div(x, parentGasTargetBig) @@ -76,14 +78,15 @@ func (k Keeper) CalculateBaseFee(ctx sdk.Context) *big.Int { return x.Add(parentBaseFee, baseFeeDelta) } - // Otherwise if the parent block used less gas than its target, the baseFee should decrease. + // Otherwise if the parent block used less gas than its target, the baseFee + // should decrease. gasUsedDelta := new(big.Int).SetUint64(parentGasTarget - parentGasUsed) x := new(big.Int).Mul(parentBaseFee, gasUsedDelta) y := x.Div(x, parentGasTargetBig) baseFeeDelta := x.Div(y, baseFeeChangeDenominator) - return math.BigMax( - x.Sub(parentBaseFee, baseFeeDelta), - common.Big0, - ) + // Set global min gas price as lower bound of the base fee, transactions below + // the min gas price don't even reach the mempool. + minGasPrice := params.MinGasPrice.TruncateInt().BigInt() + return math.BigMax(x.Sub(parentBaseFee, baseFeeDelta), minGasPrice) } diff --git a/x/feemarket/keeper/eip1559_test.go b/x/feemarket/keeper/eip1559_test.go index f97d69ebd6..d016ab48b6 100644 --- a/x/feemarket/keeper/eip1559_test.go +++ b/x/feemarket/keeper/eip1559_test.go @@ -2,107 +2,108 @@ package keeper_test import ( "fmt" - abci "github.com/tendermint/tendermint/abci/types" "math/big" + + sdk "github.com/cosmos/cosmos-sdk/types" + abci "github.com/tendermint/tendermint/abci/types" ) func (suite *KeeperTestSuite) TestCalculateBaseFee() { testCases := []struct { - name string - NoBaseFee bool - malleate func() - expFee *big.Int + name string + NoBaseFee bool + blockHeight int64 + parentBlockGasWanted uint64 + minGasPrice sdk.Dec + expFee *big.Int }{ { "without BaseFee", true, - func() {}, + 0, + 0, + sdk.ZeroDec(), nil, }, { "with BaseFee - initial EIP-1559 block", false, - func() { - suite.ctx = suite.ctx.WithBlockHeight(0) - }, + 0, + 0, + sdk.ZeroDec(), suite.app.FeeMarketKeeper.GetParams(suite.ctx).BaseFee.BigInt(), }, { - "with BaseFee - parent block wanted the same gas as its target", + "with BaseFee - parent block wanted the same gas as its target (ElasticityMultiplier = 2)", false, - func() { - // non initial block - suite.ctx = suite.ctx.WithBlockHeight(1) - - // Set gas used - suite.app.FeeMarketKeeper.SetBlockGasWanted(suite.ctx, 100) - - // Set target/gasLimit through Consensus Param MaxGas - blockParams := abci.BlockParams{ - MaxGas: 100, - MaxBytes: 10, - } - consParams := abci.ConsensusParams{Block: &blockParams} - suite.ctx = suite.ctx.WithConsensusParams(&consParams) - - // set ElasticityMultiplier - params := suite.app.FeeMarketKeeper.GetParams(suite.ctx) - params.ElasticityMultiplier = 1 - suite.app.FeeMarketKeeper.SetParams(suite.ctx, params) - }, + 1, + 50, + sdk.ZeroDec(), suite.app.FeeMarketKeeper.GetParams(suite.ctx).BaseFee.BigInt(), }, { - "with BaseFee - parent block wanted more gas than its target", + "with BaseFee - parent block wanted the same gas as its target, with higher min gas price (ElasticityMultiplier = 2)", false, - func() { - suite.ctx = suite.ctx.WithBlockHeight(1) - - suite.app.FeeMarketKeeper.SetBlockGasWanted(suite.ctx, 200) - - blockParams := abci.BlockParams{ - MaxGas: 100, - MaxBytes: 10, - } - consParams := abci.ConsensusParams{Block: &blockParams} - suite.ctx = suite.ctx.WithConsensusParams(&consParams) - - params := suite.app.FeeMarketKeeper.GetParams(suite.ctx) - params.ElasticityMultiplier = 1 - suite.app.FeeMarketKeeper.SetParams(suite.ctx, params) - }, + 1, + 50, + sdk.NewDec(1500000000), + suite.app.FeeMarketKeeper.GetParams(suite.ctx).BaseFee.BigInt(), + }, + { + "with BaseFee - parent block wanted more gas than its target (ElasticityMultiplier = 2)", + false, + 1, + 100, + sdk.ZeroDec(), big.NewInt(1125000000), }, { - "with BaseFee - Parent gas wanted smaller than parent gas target", + "with BaseFee - parent block wanted more gas than its target, with higher min gas price (ElasticityMultiplier = 2)", false, - func() { - suite.ctx = suite.ctx.WithBlockHeight(1) - - suite.app.FeeMarketKeeper.SetBlockGasWanted(suite.ctx, 50) - - blockParams := abci.BlockParams{ - MaxGas: 100, - MaxBytes: 10, - } - consParams := abci.ConsensusParams{Block: &blockParams} - suite.ctx = suite.ctx.WithConsensusParams(&consParams) - - params := suite.app.FeeMarketKeeper.GetParams(suite.ctx) - params.ElasticityMultiplier = 1 - suite.app.FeeMarketKeeper.SetParams(suite.ctx, params) - }, + 1, + 100, + sdk.NewDec(1500000000), + big.NewInt(1125000000), + }, + { + "with BaseFee - Parent gas wanted smaller than parent gas target (ElasticityMultiplier = 2)", + false, + 1, + 25, + sdk.ZeroDec(), big.NewInt(937500000), }, + { + "with BaseFee - Parent gas wanted smaller than parent gas target, with higher min gas price (ElasticityMultiplier = 2)", + false, + 1, + 25, + sdk.NewDec(1500000000), + big.NewInt(1500000000), + }, } for _, tc := range testCases { suite.Run(fmt.Sprintf("Case %s", tc.name), func() { suite.SetupTest() // reset + params := suite.app.FeeMarketKeeper.GetParams(suite.ctx) params.NoBaseFee = tc.NoBaseFee + params.MinGasPrice = tc.minGasPrice suite.app.FeeMarketKeeper.SetParams(suite.ctx, params) - tc.malleate() + // Set block height + suite.ctx = suite.ctx.WithBlockHeight(tc.blockHeight) + + // Set parent block gas + suite.app.FeeMarketKeeper.SetBlockGasWanted(suite.ctx, tc.parentBlockGasWanted) + + // Set next block target/gasLimit through Consensus Param MaxGas + blockParams := abci.BlockParams{ + MaxGas: 100, + MaxBytes: 10, + } + consParams := abci.ConsensusParams{Block: &blockParams} + suite.ctx = suite.ctx.WithConsensusParams(&consParams) fee := suite.app.FeeMarketKeeper.CalculateBaseFee(suite.ctx) if tc.NoBaseFee { diff --git a/x/feemarket/keeper/integration_test.go b/x/feemarket/keeper/integration_test.go index 581c7ddb52..608ec3ed76 100644 --- a/x/feemarket/keeper/integration_test.go +++ b/x/feemarket/keeper/integration_test.go @@ -32,87 +32,16 @@ import ( dbm "github.com/tendermint/tm-db" ) -var _ = Describe("Ethermint App min gas prices settings: ", func() { +var _ = Describe("Feemarket", func() { var ( privKey *ethsecp256k1.PrivKey - address sdk.AccAddress msg banktypes.MsgSend ) - setupChain := func(cliMinGasPricesStr string) { - // Initialize the app, so we can use SetMinGasPrices to set the - // validator-specific min-gas-prices setting - db := dbm.NewMemDB() - newapp := app.NewEthermintApp( - log.NewNopLogger(), - db, - nil, - true, - map[int64]bool{}, - app.DefaultNodeHome, - 5, - encoding.MakeConfig(app.ModuleBasics), - simapp.EmptyAppOptions{}, - baseapp.SetMinGasPrices(cliMinGasPricesStr), - ) - - genesisState := app.NewDefaultGenesisState() - genesisState[types.ModuleName] = newapp.AppCodec().MustMarshalJSON(types.DefaultGenesisState()) - - stateBytes, err := json.MarshalIndent(genesisState, "", " ") - s.Require().NoError(err) - - // Initialize the chain - newapp.InitChain( - abci.RequestInitChain{ - ChainId: "ethermint_9000-1", - Validators: []abci.ValidatorUpdate{}, - AppStateBytes: stateBytes, - ConsensusParams: app.DefaultConsensusParams, - }, - ) - - s.app = newapp - s.SetupApp(false) - } - - setupTest := func(cliMinGasPrices string) { - setupChain(cliMinGasPrices) - - privKey, address = generateKey() - amount, ok := sdk.NewIntFromString("10000000000000000000") - s.Require().True(ok) - initBalance := sdk.Coins{sdk.Coin{ - Denom: s.denom, - Amount: amount, - }} - testutil.FundAccount(s.app.BankKeeper, s.ctx, address, initBalance) - - msg = banktypes.MsgSend{ - FromAddress: address.String(), - ToAddress: address.String(), - Amount: sdk.Coins{sdk.Coin{ - Denom: s.denom, - Amount: sdk.NewInt(10000), - }}, - } - s.Commit() - } - - setupContext := func(cliMinGasPrice string, minGasPrice sdk.Dec) { - setupTest(cliMinGasPrice + s.denom) - params := types.DefaultParams() - params.MinGasPrice = minGasPrice - s.app.FeeMarketKeeper.SetParams(s.ctx, params) - s.Commit() - } - - Context("with Cosmos transactions", func() { - Context("min-gas-prices (local) < MinGasPrices (feemarket param)", func() { - cliMinGasPrice := "1" - minGasPrice := sdk.NewDecWithPrec(3, 0) + Describe("Performing Cosmos transactions", func() { + Context("with min-gas-prices (local) < MinGasPrices (feemarket param)", func() { BeforeEach(func() { - setupContext(cliMinGasPrice, minGasPrice) + privKey, msg = setupTestWithContext("1", sdk.NewDec(3), sdk.ZeroInt()) }) Context("during CheckTx", func() { @@ -153,10 +82,8 @@ var _ = Describe("Ethermint App min gas prices settings: ", func() { }) Context("with min-gas-prices (local) == MinGasPrices (feemarket param)", func() { - cliMinGasPrice := "3" - minGasPrice := sdk.NewDecWithPrec(3, 0) BeforeEach(func() { - setupContext(cliMinGasPrice, minGasPrice) + privKey, msg = setupTestWithContext("3", sdk.NewDec(3), sdk.ZeroInt()) }) Context("during CheckTx", func() { @@ -197,10 +124,8 @@ var _ = Describe("Ethermint App min gas prices settings: ", func() { }) Context("with MinGasPrices (feemarket param) < min-gas-prices (local)", func() { - cliMinGasPrice := "5" - minGasPrice := sdk.NewDecWithPrec(3, 0) BeforeEach(func() { - setupContext(cliMinGasPrice, minGasPrice) + privKey, msg = setupTestWithContext("5", sdk.NewDec(3), sdk.ZeroInt()) }) Context("during CheckTx", func() { It("should reject transactions with gasPrice < MinGasPrices", func() { @@ -256,7 +181,7 @@ var _ = Describe("Ethermint App min gas prices settings: ", func() { }) }) - Context("with EVM transactions", func() { + Describe("Performing EVM transactions", func() { type txParams struct { gasPrice *big.Int gasFeeCap *big.Int @@ -265,16 +190,21 @@ var _ = Describe("Ethermint App min gas prices settings: ", func() { } type getprices func() txParams - getBaseFee := func() int64 { - paramsEvm := s.app.EvmKeeper.GetParams(s.ctx) - ethCfg := paramsEvm.ChainConfig.EthereumConfig(s.app.EvmKeeper.ChainID()) - return s.app.EvmKeeper.GetBaseFee(s.ctx, ethCfg).Int64() - } Context("with BaseFee (feemarket) < MinGasPrices (feemarket param)", func() { - var baseFee int64 + var ( + baseFee int64 + minGasPrices int64 + ) + BeforeEach(func() { - baseFee = getBaseFee() - setupContext("1", sdk.NewDecWithPrec(baseFee+30000000000, 0)) + baseFee = 10_000_000_000 + minGasPrices = baseFee + 30_000_000_000 + + // Note that the tests run the same transactions with `gasLimit = + // 100000`. With the fee calculation `Fee = (baseFee + tip) * gasLimit`, + // a `minGasPrices = 40_000_000_000` results in `minGlobalFee = + // 4000000000000000` + privKey, _ = setupTestWithContext("1", sdk.NewDec(minGasPrices), sdk.NewInt(baseFee)) }) Context("during CheckTx", func() { @@ -291,16 +221,18 @@ var _ = Describe("Ethermint App min gas prices settings: ", func() { ).To(BeTrue(), res.GetLog()) }, Entry("legacy tx", func() txParams { - return txParams{big.NewInt(baseFee + 20000000000), nil, nil, nil} + return txParams{big.NewInt(minGasPrices - 10_000_000_000), nil, nil, nil} }), - Entry("dynamic tx", func() txParams { - return txParams{nil, big.NewInt(baseFee + 20000000000), big.NewInt(0), ðtypes.AccessList{}} + Entry("dynamic tx with GasFeeCap < MinGasPrices, no gasTipCap", func() txParams { + return txParams{nil, big.NewInt(minGasPrices - 10_000_000_000), big.NewInt(0), ðtypes.AccessList{}} }), - Entry("dynamic tx with GasFeeCap < MinGasPrices", func() txParams { - return txParams{nil, big.NewInt(baseFee + 29000000000), big.NewInt(29000000000), ðtypes.AccessList{}} + Entry("dynamic tx with GasFeeCap < MinGasPrices, max gasTipCap", func() txParams { + // Note that max priority fee per gas can't be higher than the max fee per gas (gasFeeCap), i.e. 30_000_000_000) + return txParams{nil, big.NewInt(minGasPrices - 10_000_000_000), big.NewInt(30_000_000_000), ðtypes.AccessList{}} }), + Entry("dynamic tx with GasFeeCap > MinGasPrices, EffectivePrice < MinGasPrices", func() txParams { - return txParams{nil, big.NewInt(baseFee + 40000000000), big.NewInt(0), ðtypes.AccessList{}} + return txParams{nil, big.NewInt(minGasPrices + 10_000_000_000), big.NewInt(0), ðtypes.AccessList{}} }), ) @@ -313,10 +245,13 @@ var _ = Describe("Ethermint App min gas prices settings: ", func() { Expect(res.IsOK()).To(Equal(true), "transaction should have succeeded", res.GetLog()) }, Entry("legacy tx", func() txParams { - return txParams{big.NewInt(baseFee + 31000000000), nil, nil, nil} + return txParams{big.NewInt(minGasPrices), nil, nil, nil} }), - Entry("dynamic tx", func() txParams { - return txParams{nil, big.NewInt(baseFee + 31000000000), big.NewInt(31000000000), ðtypes.AccessList{}} + // Note that this tx is not rejected on CheckTx, but not on DeliverTx, + // as the baseFee is set to minGasPrices during DeliverTx when baseFee + // < minGasPrices + Entry("dynamic tx with GasFeeCap > MinGasPrices, EffectivePrice > MinGasPrices", func() txParams { + return txParams{nil, big.NewInt(minGasPrices), big.NewInt(30_000_000_000), ðtypes.AccessList{}} }), ) }) @@ -335,16 +270,14 @@ var _ = Describe("Ethermint App min gas prices settings: ", func() { ).To(BeTrue(), res.GetLog()) }, Entry("legacy tx", func() txParams { - return txParams{big.NewInt(baseFee + 20000000000), nil, nil, nil} + return txParams{big.NewInt(minGasPrices - 10_000_000_000), nil, nil, nil} }), - Entry("dynamic tx", func() txParams { - return txParams{nil, big.NewInt(baseFee + 20000000000), big.NewInt(0), ðtypes.AccessList{}} + Entry("dynamic tx with GasFeeCap < MinGasPrices, no gasTipCap", func() txParams { + return txParams{nil, big.NewInt(minGasPrices - 10_000_000_000), big.NewInt(0), ðtypes.AccessList{}} }), - Entry("dynamic tx with GasFeeCap < MinGasPrices", func() txParams { - return txParams{nil, big.NewInt(baseFee + 29000000000), big.NewInt(29000000000), ðtypes.AccessList{}} - }), - Entry("dynamic tx with GasFeeCap > MinGasPrices, EffectivePrice < MinGasPrices", func() txParams { - return txParams{nil, big.NewInt(baseFee + 40000000000), big.NewInt(0), ðtypes.AccessList{}} + Entry("dynamic tx with GasFeeCap < MinGasPrices, max gasTipCap", func() txParams { + // Note that max priority fee per gas can't be higher than the max fee per gas (gasFeeCap), i.e. 30_000_000_000) + return txParams{nil, big.NewInt(minGasPrices - 10_000_000_000), big.NewInt(30_000_000_000), ðtypes.AccessList{}} }), ) @@ -357,144 +290,229 @@ var _ = Describe("Ethermint App min gas prices settings: ", func() { Expect(res.IsOK()).To(Equal(true), "transaction should have succeeded", res.GetLog()) }, Entry("legacy tx", func() txParams { - return txParams{big.NewInt(baseFee + 30000000001), nil, nil, nil} + return txParams{big.NewInt(minGasPrices + 1), nil, nil, nil} }), - // the base fee decreases in this test, so we use a large gas tip - // to maintain an EffectivePrice > MinGasPrices Entry("dynamic tx, EffectivePrice > MinGasPrices", func() txParams { - return txParams{nil, big.NewInt(baseFee + 30000000001), big.NewInt(30000000001), ðtypes.AccessList{}} + return txParams{nil, big.NewInt(minGasPrices + 10_000_000_000), big.NewInt(30_000_000_000), ðtypes.AccessList{}} }), ) }) - }) - Context("with MinGasPrices (feemarket param) < BaseFee (feemarket)", func() { - var baseFee int64 - BeforeEach(func() { - baseFee = getBaseFee() - s.Require().Greater(baseFee, int64(10)) - setupContext("5", sdk.NewDecWithPrec(10, 0)) - }) - - Context("during CheckTx", func() { - DescribeTable("should reject transactions with gasPrice < MinGasPrices", - func(malleate getprices) { - p := malleate() - to := tests.GenerateAddress() - msgEthereumTx := buildEthTx(privKey, &to, p.gasPrice, p.gasFeeCap, p.gasTipCap, p.accesses) - res := checkEthTx(privKey, msgEthereumTx) - Expect(res.IsOK()).To(Equal(false), "transaction should have failed") - Expect( - strings.Contains(res.GetLog(), - "provided fee < minimum global fee"), - ).To(BeTrue(), res.GetLog()) - }, - Entry("legacy tx", func() txParams { - return txParams{big.NewInt(2), nil, nil, nil} - }), - Entry("dynamic tx", func() txParams { - return txParams{nil, big.NewInt(2), big.NewInt(2), ðtypes.AccessList{}} - }), + Context("with MinGasPrices (feemarket param) < BaseFee (feemarket)", func() { + var ( + baseFee int64 + minGasPrices int64 ) - DescribeTable("should reject transactions with MinGasPrices < tx gasPrice < EffectivePrice", - func(malleate getprices) { - p := malleate() - to := tests.GenerateAddress() - msgEthereumTx := buildEthTx(privKey, &to, p.gasPrice, p.gasFeeCap, p.gasTipCap, p.accesses) - res := checkEthTx(privKey, msgEthereumTx) - Expect(res.IsOK()).To(Equal(false), "transaction should have failed") - Expect( - strings.Contains(res.GetLog(), - "insufficient fee"), - ).To(BeTrue(), res.GetLog()) - }, - Entry("legacy tx", func() txParams { - return txParams{big.NewInt(20), nil, nil, nil} - }), - Entry("dynamic tx", func() txParams { - return txParams{nil, big.NewInt(baseFee - 1), big.NewInt(20), ðtypes.AccessList{}} - }), - ) + BeforeEach(func() { + baseFee = 10_000_000_000 + minGasPrices = baseFee - 5_000_000_000 - DescribeTable("should accept transactions with gasPrice > EffectivePrice", - func(malleate getprices) { - p := malleate() - to := tests.GenerateAddress() - msgEthereumTx := buildEthTx(privKey, &to, p.gasPrice, p.gasFeeCap, p.gasTipCap, p.accesses) - res := checkEthTx(privKey, msgEthereumTx) - Expect(res.IsOK()).To(Equal(true), "transaction should have succeeded", res.GetLog()) - }, - Entry("legacy tx", func() txParams { - return txParams{big.NewInt(baseFee + 1000000000), nil, nil, nil} - }), - Entry("dynamic tx", func() txParams { - return txParams{nil, big.NewInt(baseFee + 1000000000), big.NewInt(10), ðtypes.AccessList{}} - }), - ) - }) - - Context("during DeliverTx", func() { - DescribeTable("should reject transactions with gasPrice < MinGasPrices", - func(malleate getprices) { - p := malleate() - to := tests.GenerateAddress() - msgEthereumTx := buildEthTx(privKey, &to, p.gasPrice, p.gasFeeCap, p.gasTipCap, p.accesses) - res := deliverEthTx(privKey, msgEthereumTx) - Expect(res.IsOK()).To(Equal(false), "transaction should have failed") - Expect( - strings.Contains(res.GetLog(), - "provided fee < minimum global fee"), - ).To(BeTrue(), res.GetLog()) - }, - Entry("legacy tx", func() txParams { - return txParams{big.NewInt(2), nil, nil, nil} - }), - Entry("dynamic tx", func() txParams { - return txParams{nil, big.NewInt(2), big.NewInt(2), ðtypes.AccessList{}} - }), - ) + // Note that the tests run the same transactions with `gasLimit = + // 100_000`. With the fee calculation `Fee = (baseFee + tip) * gasLimit`, + // a `minGasPrices = 5_000_000_000` results in `minGlobalFee = + // 500_000_000_000_000` + privKey, _ = setupTestWithContext("1", sdk.NewDec(minGasPrices), sdk.NewInt(baseFee)) + }) - DescribeTable("should reject transactions with MinGasPrices < gasPrice < EffectivePrice", - func(malleate getprices) { - p := malleate() - to := tests.GenerateAddress() - msgEthereumTx := buildEthTx(privKey, &to, p.gasPrice, p.gasFeeCap, p.gasTipCap, p.accesses) - res := deliverEthTx(privKey, msgEthereumTx) - Expect(res.IsOK()).To(Equal(false), "transaction should have failed") - Expect( - strings.Contains(res.GetLog(), - "insufficient fee"), - ).To(BeTrue(), res.GetLog()) - }, - Entry("legacy tx", func() txParams { - return txParams{big.NewInt(20), nil, nil, nil} - }), - Entry("dynamic tx", func() txParams { - return txParams{nil, big.NewInt(20), big.NewInt(20), ðtypes.AccessList{}} - }), - ) + Context("during CheckTx", func() { + DescribeTable("should reject transactions with gasPrice < MinGasPrices", + func(malleate getprices) { + p := malleate() + to := tests.GenerateAddress() + msgEthereumTx := buildEthTx(privKey, &to, p.gasPrice, p.gasFeeCap, p.gasTipCap, p.accesses) + res := checkEthTx(privKey, msgEthereumTx) + Expect(res.IsOK()).To(Equal(false), "transaction should have failed") + Expect( + strings.Contains(res.GetLog(), + "provided fee < minimum global fee"), + ).To(BeTrue(), res.GetLog()) + }, + Entry("legacy tx", func() txParams { + return txParams{big.NewInt(minGasPrices - 1_000_000_000), nil, nil, nil} + }), + Entry("dynamic tx with GasFeeCap < MinGasPrices, no gasTipCap", func() txParams { + return txParams{nil, big.NewInt(minGasPrices - 1_000_000_000), big.NewInt(0), ðtypes.AccessList{}} + }), + Entry("dynamic tx with GasFeeCap < MinGasPrices, max gasTipCap", func() txParams { + return txParams{nil, big.NewInt(minGasPrices - 1_000_000_000), big.NewInt(minGasPrices - 1_000_000_000), ðtypes.AccessList{}} + }), + ) + + DescribeTable("should reject transactions with MinGasPrices < tx gasPrice < EffectivePrice", + func(malleate getprices) { + p := malleate() + to := tests.GenerateAddress() + msgEthereumTx := buildEthTx(privKey, &to, p.gasPrice, p.gasFeeCap, p.gasTipCap, p.accesses) + res := checkEthTx(privKey, msgEthereumTx) + Expect(res.IsOK()).To(Equal(false), "transaction should have failed") + Expect( + strings.Contains(res.GetLog(), + "insufficient fee"), + ).To(BeTrue(), res.GetLog()) + }, + Entry("legacy tx", func() txParams { + return txParams{big.NewInt(baseFee - 1_000_000_000), nil, nil, nil} + }), + Entry("dynamic tx", func() txParams { + return txParams{nil, big.NewInt(baseFee - 1_000_000_000), big.NewInt(0), ðtypes.AccessList{}} + }), + ) + + DescribeTable("should accept transactions with gasPrice >= EffectivePrice", + func(malleate getprices) { + p := malleate() + to := tests.GenerateAddress() + msgEthereumTx := buildEthTx(privKey, &to, p.gasPrice, p.gasFeeCap, p.gasTipCap, p.accesses) + res := checkEthTx(privKey, msgEthereumTx) + Expect(res.IsOK()).To(Equal(true), "transaction should have succeeded", res.GetLog()) + }, + Entry("legacy tx", func() txParams { + return txParams{big.NewInt(baseFee), nil, nil, nil} + }), + Entry("dynamic tx", func() txParams { + return txParams{nil, big.NewInt(baseFee), big.NewInt(0), ðtypes.AccessList{}} + }), + ) + }) - DescribeTable("should accept transactions with gasPrice > EffectivePrice", - func(malleate getprices) { - p := malleate() - to := tests.GenerateAddress() - msgEthereumTx := buildEthTx(privKey, &to, p.gasPrice, p.gasFeeCap, p.gasTipCap, p.accesses) - res := deliverEthTx(privKey, msgEthereumTx) - Expect(res.IsOK()).To(Equal(true), "transaction should have succeeded", res.GetLog()) - }, - Entry("legacy tx", func() txParams { - return txParams{big.NewInt(baseFee + 10), nil, nil, nil} - }), - Entry("dynamic tx", func() txParams { - return txParams{nil, big.NewInt(baseFee + 10), big.NewInt(10), ðtypes.AccessList{}} - }), - ) + Context("during DeliverTx", func() { + DescribeTable("should reject transactions with gasPrice < MinGasPrices", + func(malleate getprices) { + p := malleate() + to := tests.GenerateAddress() + msgEthereumTx := buildEthTx(privKey, &to, p.gasPrice, p.gasFeeCap, p.gasTipCap, p.accesses) + res := deliverEthTx(privKey, msgEthereumTx) + Expect(res.IsOK()).To(Equal(false), "transaction should have failed") + Expect( + strings.Contains(res.GetLog(), + "provided fee < minimum global fee"), + ).To(BeTrue(), res.GetLog()) + }, + Entry("legacy tx", func() txParams { + return txParams{big.NewInt(minGasPrices - 1_000_000_000), nil, nil, nil} + }), + Entry("dynamic tx", func() txParams { + return txParams{nil, big.NewInt(minGasPrices - 1_000_000_000), nil, ðtypes.AccessList{}} + }), + ) + + DescribeTable("should reject transactions with MinGasPrices < gasPrice < EffectivePrice", + func(malleate getprices) { + p := malleate() + to := tests.GenerateAddress() + msgEthereumTx := buildEthTx(privKey, &to, p.gasPrice, p.gasFeeCap, p.gasTipCap, p.accesses) + res := deliverEthTx(privKey, msgEthereumTx) + Expect(res.IsOK()).To(Equal(false), "transaction should have failed") + Expect( + strings.Contains(res.GetLog(), + "insufficient fee"), + ).To(BeTrue(), res.GetLog()) + }, + // Note that the baseFee is not 10_000_000_000 anymore but updates to 8_750_000_000 because of the s.Commit + Entry("legacy tx", func() txParams { + return txParams{big.NewInt(baseFee - 2_000_000_000), nil, nil, nil} + }), + Entry("dynamic tx", func() txParams { + return txParams{nil, big.NewInt(baseFee - 2_000_000_000), big.NewInt(0), ðtypes.AccessList{}} + }), + ) + + DescribeTable("should accept transactions with gasPrice >= EffectivePrice", + func(malleate getprices) { + p := malleate() + to := tests.GenerateAddress() + msgEthereumTx := buildEthTx(privKey, &to, p.gasPrice, p.gasFeeCap, p.gasTipCap, p.accesses) + res := deliverEthTx(privKey, msgEthereumTx) + Expect(res.IsOK()).To(Equal(true), "transaction should have succeeded", res.GetLog()) + }, + Entry("legacy tx", func() txParams { + return txParams{big.NewInt(baseFee), nil, nil, nil} + }), + Entry("dynamic tx", func() txParams { + return txParams{nil, big.NewInt(baseFee), big.NewInt(0), ðtypes.AccessList{}} + }), + ) + }) }) }) }) }) +// setupTestWithContext sets up a test chain with an example Cosmos send msg, +// given a local (validator config) and a gloabl (feemarket param) minGasPrice +func setupTestWithContext(valMinGasPrice string, minGasPrice sdk.Dec, baseFee sdk.Int) (*ethsecp256k1.PrivKey, banktypes.MsgSend) { + privKey, msg := setupTest(valMinGasPrice + s.denom) + params := types.DefaultParams() + params.MinGasPrice = minGasPrice + s.app.FeeMarketKeeper.SetParams(s.ctx, params) + s.app.FeeMarketKeeper.SetBaseFee(s.ctx, baseFee.BigInt()) + s.Commit() + + return privKey, msg +} + +func setupTest(localMinGasPrices string) (*ethsecp256k1.PrivKey, banktypes.MsgSend) { + setupChain(localMinGasPrices) + + privKey, address := generateKey() + amount, ok := sdk.NewIntFromString("10000000000000000000") + s.Require().True(ok) + initBalance := sdk.Coins{sdk.Coin{ + Denom: s.denom, + Amount: amount, + }} + testutil.FundAccount(s.app.BankKeeper, s.ctx, address, initBalance) + + msg := banktypes.MsgSend{ + FromAddress: address.String(), + ToAddress: address.String(), + Amount: sdk.Coins{sdk.Coin{ + Denom: s.denom, + Amount: sdk.NewInt(10000), + }}, + } + s.Commit() + return privKey, msg +} + +func setupChain(localMinGasPricesStr string) { + // Initialize the app, so we can use SetMinGasPrices to set the + // validator-specific min-gas-prices setting + db := dbm.NewMemDB() + newapp := app.NewEthermintApp( + log.NewNopLogger(), + db, + nil, + true, + map[int64]bool{}, + app.DefaultNodeHome, + 5, + encoding.MakeConfig(app.ModuleBasics), + simapp.EmptyAppOptions{}, + baseapp.SetMinGasPrices(localMinGasPricesStr), + ) + + genesisState := app.NewDefaultGenesisState() + genesisState[types.ModuleName] = newapp.AppCodec().MustMarshalJSON(types.DefaultGenesisState()) + + stateBytes, err := json.MarshalIndent(genesisState, "", " ") + s.Require().NoError(err) + + // Initialize the chain + newapp.InitChain( + abci.RequestInitChain{ + ChainId: "ethermint_9000-1", + Validators: []abci.ValidatorUpdate{}, + AppStateBytes: stateBytes, + ConsensusParams: app.DefaultConsensusParams, + }, + ) + + s.app = newapp + s.SetupApp(false) +} + func generateKey() (*ethsecp256k1.PrivKey, sdk.AccAddress) { address, priv := tests.NewAddrKey() return priv.(*ethsecp256k1.PrivKey), sdk.AccAddress(address.Bytes()) @@ -567,17 +585,17 @@ func prepareEthTx(priv *ethsecp256k1.PrivKey, msgEthereumTx *evmtypes.MsgEthereu return bz } -func deliverEthTx(priv *ethsecp256k1.PrivKey, msgEthereumTx *evmtypes.MsgEthereumTx) abci.ResponseDeliverTx { +func checkEthTx(priv *ethsecp256k1.PrivKey, msgEthereumTx *evmtypes.MsgEthereumTx) abci.ResponseCheckTx { bz := prepareEthTx(priv, msgEthereumTx) - req := abci.RequestDeliverTx{Tx: bz} - res := s.app.BaseApp.DeliverTx(req) + req := abci.RequestCheckTx{Tx: bz} + res := s.app.BaseApp.CheckTx(req) return res } -func checkEthTx(priv *ethsecp256k1.PrivKey, msgEthereumTx *evmtypes.MsgEthereumTx) abci.ResponseCheckTx { +func deliverEthTx(priv *ethsecp256k1.PrivKey, msgEthereumTx *evmtypes.MsgEthereumTx) abci.ResponseDeliverTx { bz := prepareEthTx(priv, msgEthereumTx) - req := abci.RequestCheckTx{Tx: bz} - res := s.app.BaseApp.CheckTx(req) + req := abci.RequestDeliverTx{Tx: bz} + res := s.app.BaseApp.DeliverTx(req) return res } @@ -640,16 +658,16 @@ func prepareCosmosTx(priv *ethsecp256k1.PrivKey, gasPrice *sdk.Int, msgs ...sdk. return bz } -func deliverTx(priv *ethsecp256k1.PrivKey, gasPrice *sdk.Int, msgs ...sdk.Msg) abci.ResponseDeliverTx { +func checkTx(priv *ethsecp256k1.PrivKey, gasPrice *sdk.Int, msgs ...sdk.Msg) abci.ResponseCheckTx { bz := prepareCosmosTx(priv, gasPrice, msgs...) - req := abci.RequestDeliverTx{Tx: bz} - res := s.app.BaseApp.DeliverTx(req) + req := abci.RequestCheckTx{Tx: bz} + res := s.app.BaseApp.CheckTx(req) return res } -func checkTx(priv *ethsecp256k1.PrivKey, gasPrice *sdk.Int, msgs ...sdk.Msg) abci.ResponseCheckTx { +func deliverTx(priv *ethsecp256k1.PrivKey, gasPrice *sdk.Int, msgs ...sdk.Msg) abci.ResponseDeliverTx { bz := prepareCosmosTx(priv, gasPrice, msgs...) - req := abci.RequestCheckTx{Tx: bz} - res := s.app.BaseApp.CheckTx(req) + req := abci.RequestDeliverTx{Tx: bz} + res := s.app.BaseApp.DeliverTx(req) return res } diff --git a/x/feemarket/spec/01_concepts.md b/x/feemarket/spec/01_concepts.md index 405bc93a77..878ccf637a 100644 --- a/x/feemarket/spec/01_concepts.md +++ b/x/feemarket/spec/01_concepts.md @@ -4,33 +4,68 @@ order: 1 # Concepts -## Base fee +## EIP-1559: Fee Market -The base fee is a global base fee defined at the consensus level. It is adjusted for each block based on the total gas used in the previous block and gas target (block gas limit divided by elasticity multiplier) +[EIP-1559](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1559.md) describes a pricing mechanism that was proposed on Ethereum to improve to calculation of transaction fees. It includes a fixed-per-block network fee that is burned and dynamically expands/contracts block sizes to deal with peaks of network congestion. -- it increases when blocks are above the gas target -- it decreases when blocks are below the gas target +Before EIP-1559 the transaction fee is calculated with -Unlike the Cosmos SDK local `minimal-gas-prices`, this value is stored as a module parameter which provides a reliable value for validators to agree upon. +``` +fee = gasPrice * gasLimit +``` -## Tip +, where `gasPrice` is the price per gas and `gasLimit` describes the amount of gas required to perform the transaction. The more complex operations a transaction requires, the higher the gasLimit (See [Executing EVM bytecode](https://docs.evmos.org/modules/evm/01_concepts.html#executing-evm-bytecode)). To submit a transaction, the signer needs to specify the `gasPrice`. -In EIP-1559, the `tip` is a value that can be added to the `baseFee` in order to incentive transaction prioritization. +With EIP-1559 enabled, the transaction fee is calculated with -The transaction fee in Ethereum is calculated using the following the formula : +``` +fee = (baseFee + priorityTip) * gasLimit +``` -`transaction fee = (baseFee + tip) * gas units (limit)` +, where `baseFee` is the fixed-per-block network fee per gas and `priorityTip` is an additional fee per gas that can be set optionally. Note, that both the base fee and the priority tip are a gas prices. To submit a transaction with EIP-1559, the signer needs to specify the `gasFeeCap` a maximum fee per gas they are willing to pay total and optionally the `priorityTip` , which covers both the priority fee and the block's network fee per gas (aka: base fee). -In Cosmos SDK there is no notion of prioritization, thus the tip for an EIP-1559 transaction in Ethermint should be zero (`MaxPriorityFeePerGas` JSON-RPC endpoint returns `0`) +## Base Fee -## EIP-1559 +The base fee per gas (aka base fee) is a global gas price defined at the consensus level. It is stored as a module parameter and is adjusted at the end of each block based on the total gas used in the previous block and gas target (`block gas limit / elasticity multiplier`): -A transaction pricing mechanism introduced in Ethereum that includes fixed-per-block network fee that is burned and dynamically expands/contracts block sizes to deal with transient congestion. +- it increases when blocks are above the gas target, +- it decreases when blocks are below the gas target. -Transactions specify a maximum fee per gas they are willing to pay total (aka: max fee), which covers both the priority fee and the block's network fee per gas (aka: base fee) +Instead of burning the base fee (as implemented on Ethereum), the `feemarket` module allocates the base fee for regular [Cosmos SDK fee distribution](https://docs.evmos.org/modules/distribution/). -Reference: [EIP1559](https://eips.ethereum.org/EIPS/eip-1559) +## Priority Tip -## Global Minimum Gas Price +In EIP-1559, the `max_priority_fee_per_gas`, often referred to as `tip`, is an additional gas price that can be added to the `baseFee` in order to incentive transaction prioritization. The higher the tip, the more likely the transaction is included in the block. -The minimum gas price needed for transactions to be processed. It applies to both Cosmos and EVM transactions. Governance can change this `feemarket` module parameter value. If the effective gas price or the minimum gas price is lower than the global `MinGasPrice` (`min-gas-price (local) < MinGasPrice (global) OR EffectiveGasPrice < MinGasPrice`), then `MinGasPrice` is used as a lower bound. If transactions are rejected due to having a gas price lower than `MinGasPrice`, users need to resend the transactions with a gas price higher than `MinGasPrice`. In the case of EIP-1559 (dynamic fee transactions), users must increase the priority fee for their transactions to be valid. +Until the Cosmos SDK version v0.46, however, there is no notion of transaction prioritization. Thus the tip for an EIP-1559 transaction on Ethermint should be zero (`MaxPriorityFeePerGas` JSON-RPC endpoint returns `0`). Have a look at [ADR 067](https://github.com/tendermint/tendermint/blob/master/docs/architecture/adr-067-mempool-refactor.md) to read about future plans on transaction prioritization in the Cosmos SDK. + +## Effective Gas price + +For EIP-1559 transactions (dynamic fee transactions) the effective gas price descibes the maximum gas price that a transaction is willing to provide. It is derived from the transaction arguments and the base fee parameter. Depending on which one is smaller, the effective gas price is either the `baseFee + tip` or the `gasFeeCap` + +``` +min(baseFee + gasTipCap, gasFeeCap) +``` + +## Local vs. Global Minimum Gas Prices + +Minimum gas prices are used to discard spam transactions in the network, by raising the cost of transactions to the point that it is not economically viable for the spammer. This is achieved by defining a minimum gas price for accepting txs in the mempool for both Cosmos and EVM transactions. A transaction is discarded from the mempool if it doesn't provide at least one of the two types of min gas prices: + +Minimum gas prices are used to discard spam transactions in the network, by raising the cost of transactions to the point that it is not economically viable for the spammer. This is achieved by defining a minimum gas price for accepting txs in the mempool for both Cosmos and EVM transactions. A transaction is discarded from the mempool if it doesn't provide at least one of the two types of min gas prices: + +1. the local min gas prices that validators can set on their node config and +2. the global min gas price, which is set as a parameter in the `feemarket` module, which can be changed through governance. + +The lower bound for a transaction gas price is determined by comparing of gas price bounds according to three cases: + +1. If the effective gas price (`effective gas price = base fee + priority tip`) or the local minimum gas price is lower than the global `MinGasPrice` (`min-gas-price (local) < MinGasPrice (global) OR EffectiveGasPrice < MinGasPrice`), then `MinGasPrice` is used as a lower bound. + +2. If transactions are rejected due to having a gas price lower than `MinGasPrice`, users need to resend the transactions with a gas price higher or equal to `MinGasPrice`. + +3. If the effective gas price or the local `minimum-gas-price` is higher than the global `MinGasPrice`, then the larger value of the two is used as a lower bound. In the case of EIP-1559, users must increase the priority fee for their transactions to be valid. + +The comparison of transaction gas price and the lower bound is implemented through AnteHandler decorators. For EVM transactions, this is done in the `EthMempoolFeeDecorator` and `EthMinGasPriceDecorator` `AnteHandler` and for Cosmos transactions in `NewMempoolFeeDecorator` and `MinGasPriceDecorator` `AnteHandler`. + +::: tip +If the base fee decreases to a value below the global `MinGasPrice`, it is set to the `MinGasPrice`. This is implemented, so that the base fee can't drop to gas prices that wouldn't allow transactions to be accepted in the mempool, because of a higher `MinGasPrice`. +::: \ No newline at end of file