Skip to content

Commit

Permalink
consistent baseFee check logic
Browse files Browse the repository at this point in the history 
Closes: evmos#755

```
if not london_hardfork {
    # reject DynamicFeeTx
    # no `baseFeePerGas` field in block response
    # baseFee = nil
} else {
    # allow DynamicFeeTx
    # add `baseFeePerGas` field in block response
    if feemarketParams.NoBaseFee or height < feemarketParams.EnableHeight {
        # baseFee = 0
    } else {
        # init baseFee to initBaseFee and adjust in later blocks
    }
}
```

Update x/evm/keeper/keeper.go

Co-authored-by: Federico Kunze Küllmer <[email protected]>

add unit tests

Update app/ante/utils_test.go

Co-authored-by: Federico Kunze Küllmer <[email protected]>

changelog
  • Loading branch information
@yihuang
yihuang committed Dec 27, 2021
1 parent 8bc3cc4 commit a6c9658
Show file tree
Hide file tree
Showing 17 changed files with 435 additions and 334 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
- (evm) [tharsis#808](https://github.com/tharsis/ethermint/issues/808) increase nonce in ante handler for contract creation transaction.
- (evm) [tharsis#N/A]() reject invalid `MsgEthereumTx` wrapping tx
- (evm) [tharsis#N/A]() Fix SelfDestruct opcode by deleting account code and state
- (feemarket) [tharsis#855](https://github.com/tharsis/ethermint/pull/855) consistent baseFee check logic

### Improvements

Expand Down
91 changes: 65 additions & 26 deletions app/ante/ante_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
)

func (suite AnteTestSuite) TestAnteHandler() {
suite.dynamicTxFee = false
suite.enableFeemarket = false
suite.SetupTest() // reset

addr, privKey := tests.NewAddrKey()
Expand Down Expand Up @@ -314,22 +314,16 @@ func (suite AnteTestSuite) TestAnteHandler() {
}

func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
suite.dynamicTxFee = true
suite.SetupTest() // reset

addr, privKey := tests.NewAddrKey()
to := tests.GenerateAddress()

acc := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr.Bytes())
suite.Require().NoError(acc.SetSequence(1))
suite.app.AccountKeeper.SetAccount(suite.ctx, acc)

testCases := []struct {
name string
txFn func() sdk.Tx
checkTx bool
reCheckTx bool
expPass bool
name string
txFn func() sdk.Tx
enableLondonHF bool
checkTx bool
reCheckTx bool
expPass bool
}{
{
"success - DeliverTx (contract)",
Expand All @@ -351,6 +345,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
return tx
},
true,
false, false, true,
},
{
Expand All @@ -359,7 +354,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
signedContractTx :=
evmtypes.NewTxContract(
suite.app.EvmKeeper.ChainID(),
2,
1,
big.NewInt(10),
100000,
nil,
Expand All @@ -373,6 +368,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
return tx
},
true,
true, false, true,
},
{
Expand All @@ -381,7 +377,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
signedContractTx :=
evmtypes.NewTxContract(
suite.app.EvmKeeper.ChainID(),
3,
1,
big.NewInt(10),
100000,
nil,
Expand All @@ -395,6 +391,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
return tx
},
true,
false, true, true,
},
{
Expand All @@ -403,7 +400,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
signedTx :=
evmtypes.NewTx(
suite.app.EvmKeeper.ChainID(),
4,
1,
&to,
big.NewInt(10),
100000,
Expand All @@ -418,6 +415,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
tx := suite.CreateTestTx(signedTx, privKey, 1, false)
return tx
},
true,
false, false, true,
},
{
Expand All @@ -426,7 +424,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
signedTx :=
evmtypes.NewTx(
suite.app.EvmKeeper.ChainID(),
5,
1,
&to,
big.NewInt(10),
100000,
Expand All @@ -441,6 +439,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
tx := suite.CreateTestTx(signedTx, privKey, 1, false)
return tx
},
true,
true, false, true,
},
{
Expand All @@ -449,7 +448,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
signedTx :=
evmtypes.NewTx(
suite.app.EvmKeeper.ChainID(),
3,
1,
&to,
big.NewInt(10),
100000,
Expand All @@ -463,15 +462,17 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {

tx := suite.CreateTestTx(signedTx, privKey, 1, false)
return tx
}, false, true, true,
},
true,
false, true, true,
},
{
"success - CheckTx (cosmos tx not signed)",
func() sdk.Tx {
signedTx :=
evmtypes.NewTx(
suite.app.EvmKeeper.ChainID(),
4,
1,
&to,
big.NewInt(10),
100000,
Expand All @@ -485,15 +486,17 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {

tx := suite.CreateTestTx(signedTx, privKey, 1, false)
return tx
}, false, true, true,
},
true,
false, true, true,
},
{
"fail - CheckTx (cosmos tx is not valid)",
func() sdk.Tx {
signedTx :=
evmtypes.NewTx(
suite.app.EvmKeeper.ChainID(),
4,
1,
&to,
big.NewInt(10),
100000,
Expand All @@ -509,15 +512,17 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
// bigger than MaxGasWanted
txBuilder.SetGasLimit(uint64(1 << 63))
return txBuilder.GetTx()
}, true, false, false,
},
true,
true, false, false,
},
{
"fail - CheckTx (memo too long)",
func() sdk.Tx {
signedTx :=
evmtypes.NewTx(
suite.app.EvmKeeper.ChainID(),
5,
1,
&to,
big.NewInt(10),
100000,
Expand All @@ -532,12 +537,45 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
txBuilder.SetMemo(strings.Repeat("*", 257))
return txBuilder.GetTx()
}, true, false, false,
},
true,
true, false, false,
},
{
"fail - DynamicFeeTx without london hark fork",
func() sdk.Tx {
signedContractTx :=
evmtypes.NewTxContract(
suite.app.EvmKeeper.ChainID(),
1,
big.NewInt(10),
100000,
nil,
big.NewInt(ethparams.InitialBaseFee+1),
big.NewInt(1),
nil,
&types.AccessList{},
)
signedContractTx.From = addr.Hex()

tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
return tx
},
false,
false, false, false,
},
}

for _, tc := range testCases {
suite.Run(tc.name, func() {
suite.enableFeemarket = true
suite.enableLondonHF = tc.enableLondonHF
suite.SetupTest() // reset

acc := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr.Bytes())
suite.Require().NoError(acc.SetSequence(1))
suite.app.AccountKeeper.SetAccount(suite.ctx, acc)

suite.ctx = suite.ctx.WithIsCheckTx(tc.checkTx).WithIsReCheckTx(tc.reCheckTx)
suite.app.EvmKeeper.AddBalance(addr, big.NewInt((ethparams.InitialBaseFee+10)*100000))
_, err := suite.anteHandler(suite.ctx, tc.txFn(), false)
Expand All @@ -548,5 +586,6 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
}
})
}
suite.dynamicTxFee = false
suite.enableFeemarket = false
suite.enableLondonHF = true
}
46 changes: 29 additions & 17 deletions app/ante/eth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/ethereum/go-ethereum/core"
ethtypes "github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/core/vm"
"github.com/ethereum/go-ethereum/params"
)

// EVMKeeper defines the expected keeper interface used on the Eth AnteHandler
Expand All @@ -32,6 +33,7 @@ type EVMKeeper interface {
DeductTxCostsFromUserBalance(
ctx sdk.Context, msgEthTx evmtypes.MsgEthereumTx, txData evmtypes.TxData, denom string, homestead, istanbul, london bool,
) (sdk.Coins, error)
BaseFee(ctx sdk.Context, ethCfg *params.ChainConfig) *big.Int
}

type protoTxProvider interface {
Expand Down Expand Up @@ -326,8 +328,6 @@ func (ctd CanTransferDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate
ctd.evmKeeper.WithContext(ctx)

params := ctd.evmKeeper.GetParams(ctx)
feeMktParams := ctd.feemarketKeeper.GetParams(ctx)

ethCfg := params.ChainConfig.EthereumConfig(ctd.evmKeeper.ChainID())
signer := ethtypes.MakeSigner(ethCfg, big.NewInt(ctx.BlockHeight()))

Expand All @@ -337,10 +337,7 @@ func (ctd CanTransferDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate
return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid transaction type %T, expected %T", tx, (*evmtypes.MsgEthereumTx)(nil))
}

var baseFee *big.Int
if evmtypes.IsLondon(ethCfg, ctx.BlockHeight()) && !feeMktParams.NoBaseFee {
baseFee = ctd.feemarketKeeper.GetBaseFee(ctx)
}
baseFee := ctd.evmKeeper.BaseFee(ctx, ethCfg)

coreMsg, err := msgEthTx.AsMessage(signer, baseFee)
if err != nil {
Expand Down Expand Up @@ -370,17 +367,23 @@ func (ctd CanTransferDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate
)
}

if evmtypes.IsLondon(ethCfg, ctx.BlockHeight()) && !feeMktParams.NoBaseFee && baseFee == nil {
return ctx, sdkerrors.Wrap(evmtypes.ErrInvalidBaseFee, "base fee is supported but evm block context value is nil")
}

if evmtypes.IsLondon(ethCfg, ctx.BlockHeight()) && !feeMktParams.NoBaseFee && baseFee != nil && coreMsg.GasFeeCap().Cmp(baseFee) < 0 {
return ctx, sdkerrors.Wrapf(evmtypes.ErrInvalidBaseFee, "max fee per gas less than block base fee (%s < %s)", coreMsg.GasFeeCap(), baseFee)
if evmtypes.IsLondon(ethCfg, ctx.BlockHeight()) {
if baseFee == nil {
return ctx, sdkerrors.Wrap(
evmtypes.ErrInvalidBaseFee,
"base fee is supported but evm block context value is nil",
)
}
if coreMsg.GasFeeCap().Cmp(baseFee) < 0 {
return ctx, sdkerrors.Wrapf(
evmtypes.ErrInvalidBaseFee,
"max fee per gas less than block base fee (%s < %s)",
coreMsg.GasFeeCap(), baseFee,
)
}
}
}

ctd.evmKeeper.WithContext(ctx)

// set the original gas meter
return next(ctx, tx, simulate)
}
Expand Down Expand Up @@ -443,6 +446,8 @@ func (vbd EthValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu
return next(ctx, tx, simulate)
}

vbd.evmKeeper.WithContext(ctx)

err := tx.ValidateBasic()
// ErrNoSignatures is fine with eth tx
if err != nil && !errors.Is(err, sdkerrors.ErrNoSignatures) {
Expand Down Expand Up @@ -477,7 +482,15 @@ func (vbd EthValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu
if err != nil {
return ctx, sdkerrors.Wrap(err, "failed to unpack MsgEthereumTx Data")
}

params := vbd.evmKeeper.GetParams(ctx)
chainID := vbd.evmKeeper.ChainID()
ethCfg := params.ChainConfig.EthereumConfig(chainID)
baseFee := vbd.evmKeeper.BaseFee(ctx, ethCfg)
if baseFee == nil && txData.TxType() == ethtypes.DynamicFeeTxType {
return ctx, sdkerrors.Wrap(ethtypes.ErrTxTypeNotSupported, "dynamic fee tx not supported")
}

ethFeeAmount := sdk.Coins{sdk.NewCoin(params.EvmDenom, sdk.NewIntFromBigInt(txData.Fee()))}

authInfo := protoTx.AuthInfo
Expand Down Expand Up @@ -558,13 +571,12 @@ func (mfd EthMempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulat

var feeAmt *big.Int

feeMktParams := mfd.feemarketKeeper.GetParams(ctx)
params := mfd.evmKeeper.GetParams(ctx)
chainID := mfd.evmKeeper.ChainID()
ethCfg := params.ChainConfig.EthereumConfig(chainID)
evmDenom := params.EvmDenom
if evmtypes.IsLondon(ethCfg, ctx.BlockHeight()) && !feeMktParams.NoBaseFee {
baseFee := mfd.feemarketKeeper.GetBaseFee(ctx)
baseFee := mfd.evmKeeper.BaseFee(ctx, ethCfg)
if baseFee != nil {
feeAmt = msg.GetEffectiveFee(baseFee)
} else {
feeAmt = msg.GetFee()
Expand Down
2 changes: 1 addition & 1 deletion app/ante/sigs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
)

func (suite AnteTestSuite) TestSignatures() {
suite.dynamicTxFee = false
suite.enableFeemarket = false
suite.SetupTest() // reset

addr, privKey := tests.NewAddrKey()
Expand Down
Loading

0 comments on commit a6c9658

Please sign in to comment.