Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Commit

Permalink
bug(feemarket): set lower bound of base fee to min gas price param (#…
Browse files Browse the repository at this point in the history 
…1135)

* 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>
  • Loading branch information
@danburck @fedekunze
danburck and fedekunze authored Jun 22, 2022
1 parent 8e2c652 commit d333341
Show file tree
Hide file tree
Showing 6 changed files with 394 additions and 335 deletions.
9 changes: 5 additions & 4 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
5 changes: 3 additions & 2 deletions app/ante/fees.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)
}
}
Expand Down
17 changes: 10 additions & 7 deletions x/feemarket/keeper/eip1559.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
131 changes: 66 additions & 65 deletions x/feemarket/keeper/eip1559_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit d333341

Please sign in to comment.