From e1560849dd60bd4ba588da8ec3741ec7b57cc648 Mon Sep 17 00:00:00 2001 From: yihuang Date: Fri, 5 Aug 2022 21:00:31 +0800 Subject: [PATCH] feat(ante, evm): set priority for eth transactions (#1214) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Set priority for eth transactions Set the tx priority to the lowest priority in the messages. fix unit tests code cleanup and spec update spec fix go lint add priority integration test add python linter job add access list tx type fix gas limit remove ledger tag, so no need to replace hid dependency fix earlier check ibc-go v5.0.0-beta1 * fix pruned node integration test * Update x/feemarket/spec/09_antehandlers.md Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com> --- CHANGELOG.md | 4 + app/ante/eth.go | 15 ++- app/ante/eth_test.go | 58 +++++++-- app/ante/handler_options.go | 4 +- app/ante/interfaces.go | 2 +- default.nix | 2 +- go.mod | 4 +- go.sum | 4 +- gomod2nix.toml | 5 +- rpc/namespaces/ethereum/eth/api.go | 2 +- .../integration_tests/configs/default.jsonnet | 12 +- tests/integration_tests/network.py | 4 +- tests/integration_tests/test_priority.py | 114 ++++++++++++++++++ tests/integration_tests/utils.py | 34 ++++++ x/evm/handler_test.go | 9 +- x/evm/keeper/integration_test.go | 4 +- x/evm/keeper/keeper.go | 6 +- x/evm/keeper/utils.go | 50 +++++--- x/evm/keeper/utils_test.go | 3 +- x/evm/types/access_list_tx.go | 5 + x/evm/types/dynamic_fee_tx.go | 6 +- x/evm/types/legacy_tx.go | 5 + x/evm/types/tx_data.go | 3 +- x/feemarket/keeper/integration_test.go | 1 - x/feemarket/spec/09_antehandlers.md | 14 ++- 25 files changed, 307 insertions(+), 63 deletions(-) create mode 100644 tests/integration_tests/test_priority.py diff --git a/CHANGELOG.md b/CHANGELOG.md index bf0f42bd71..3c2237c12b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ recompute eth tx hashes in JSON-RPC APIs to fix old blocks. * (deps) [#1168](https://github.com/evmos/ethermint/pull/1168) Upgrade cosmos-sdk to v0.46. +### API Breaking + +* (ante) [#1214](https://github.com/evmos/ethermint/pull/1214) Set mempool priority to evm transactions. + ### Improvements * (feemarket) [\#1165](https://github.com/evmos/ethermint/pull/1165) Add hint in specs about different gas terminology for gas in Cosmos and Ethereum. diff --git a/app/ante/eth.go b/app/ante/eth.go index e14eacc3a7..c997ec65d1 100644 --- a/app/ante/eth.go +++ b/app/ante/eth.go @@ -2,6 +2,7 @@ package ante import ( "errors" + "math" "math/big" "strconv" @@ -170,7 +171,7 @@ func NewEthGasConsumeDecorator( // - user doesn't have enough balance to deduct the transaction fees (gas_limit * gas_price) // - transaction or block gas meter runs out of gas // - sets the gas meter limit -func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { +func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { params := egcd.evmKeeper.GetParams(ctx) ethCfg := params.ChainConfig.EthereumConfig(egcd.evmKeeper.ChainID()) @@ -183,6 +184,9 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula gasWanted := uint64(0) var events sdk.Events + // Use the lowest priority of all the messages as the final one. + minPriority := int64(math.MaxInt64) + for _, msg := range tx.GetMsgs() { msgEthTx, ok := msg.(*evmtypes.MsgEthereumTx) if !ok { @@ -205,7 +209,7 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula gasWanted += txData.GetGas() } - fees, err := egcd.evmKeeper.DeductTxCostsFromUserBalance( + fees, priority, err := egcd.evmKeeper.DeductTxCostsFromUserBalance( ctx, *msgEthTx, txData, @@ -219,6 +223,9 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula } events = append(events, sdk.NewEvent(sdk.EventTypeTx, sdk.NewAttribute(sdk.AttributeKeyFee, fees.String()))) + if priority < minPriority { + minPriority = priority + } } // TODO: change to typed events @@ -240,8 +247,10 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula ctx = ctx.WithGasMeter(ethermint.NewInfiniteGasMeterWithLimit(gasWanted)) ctx.GasMeter().ConsumeGas(gasConsumed, "copy gas consumed") + newCtx := ctx.WithPriority(minPriority) + // we know that we have enough gas on the pool to cover the intrinsic gas - return next(ctx, tx, simulate) + return next(newCtx, tx, simulate) } // CanTransferDecorator checks if the sender is allowed to transfer funds according to the EVM block diff --git a/app/ante/eth_test.go b/app/ante/eth_test.go index e775e8ced0..472312f29c 100644 --- a/app/ante/eth_test.go +++ b/app/ante/eth_test.go @@ -9,6 +9,7 @@ import ( "github.com/evmos/ethermint/app/ante" "github.com/evmos/ethermint/server/config" "github.com/evmos/ethermint/tests" + evmkeeper "github.com/evmos/ethermint/x/evm/keeper" "github.com/evmos/ethermint/x/evm/statedb" evmtypes "github.com/evmos/ethermint/x/evm/types" @@ -221,27 +222,45 @@ func (suite AnteTestSuite) TestEthGasConsumeDecorator() { tx := evmtypes.NewTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), txGasLimit, big.NewInt(1), nil, nil, nil, nil) tx.From = addr.Hex() + ethCfg := suite.app.EvmKeeper.GetParams(suite.ctx). + ChainConfig.EthereumConfig(suite.app.EvmKeeper.ChainID()) + baseFee := suite.app.EvmKeeper.GetBaseFee(suite.ctx, ethCfg) + suite.Require().Equal(int64(1000000000), baseFee.Int64()) + + gasPrice := new(big.Int).Add(baseFee, evmkeeper.DefaultPriorityReduction.BigInt()) + tx2GasLimit := uint64(1000000) - tx2 := evmtypes.NewTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), tx2GasLimit, big.NewInt(1), nil, nil, nil, ðtypes.AccessList{{Address: addr, StorageKeys: nil}}) + tx2 := evmtypes.NewTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), tx2GasLimit, gasPrice, nil, nil, nil, ðtypes.AccessList{{Address: addr, StorageKeys: nil}}) tx2.From = addr.Hex() + tx2Priority := int64(1) + + dynamicFeeTx := evmtypes.NewTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), tx2GasLimit, + nil, // gasPrice + new(big.Int).Add(baseFee, big.NewInt(evmkeeper.DefaultPriorityReduction.Int64()*2)), // gasFeeCap + evmkeeper.DefaultPriorityReduction.BigInt(), // gasTipCap + nil, ðtypes.AccessList{{Address: addr, StorageKeys: nil}}) + dynamicFeeTx.From = addr.Hex() + dynamicFeeTxPriority := int64(1) var vmdb *statedb.StateDB testCases := []struct { - name string - tx sdk.Tx - gasLimit uint64 - malleate func() - expPass bool - expPanic bool + name string + tx sdk.Tx + gasLimit uint64 + malleate func() + expPass bool + expPanic bool + expPriority int64 }{ - {"invalid transaction type", &invalidTx{}, math.MaxUint64, func() {}, false, false}, + {"invalid transaction type", &invalidTx{}, math.MaxUint64, func() {}, false, false, 0}, { "sender not found", evmtypes.NewTxContract(suite.app.EvmKeeper.ChainID(), 1, big.NewInt(10), 1000, big.NewInt(1), nil, nil, nil, nil), math.MaxUint64, func() {}, false, false, + 0, }, { "gas limit too low", @@ -249,6 +268,7 @@ func (suite AnteTestSuite) TestEthGasConsumeDecorator() { math.MaxUint64, func() {}, false, false, + 0, }, { "not enough balance for fees", @@ -256,6 +276,7 @@ func (suite AnteTestSuite) TestEthGasConsumeDecorator() { math.MaxUint64, func() {}, false, false, + 0, }, { "not enough tx gas", @@ -265,6 +286,7 @@ func (suite AnteTestSuite) TestEthGasConsumeDecorator() { vmdb.AddBalance(addr, big.NewInt(1000000)) }, false, true, + 0, }, { "not enough block gas", @@ -272,21 +294,32 @@ func (suite AnteTestSuite) TestEthGasConsumeDecorator() { 0, func() { vmdb.AddBalance(addr, big.NewInt(1000000)) - suite.ctx = suite.ctx.WithBlockGasMeter(sdk.NewGasMeter(1)) }, false, true, + 0, }, { - "success", + "success - legacy tx", tx2, tx2GasLimit, // it's capped func() { - vmdb.AddBalance(addr, big.NewInt(1000000)) - + vmdb.AddBalance(addr, big.NewInt(1001000000000000)) + suite.ctx = suite.ctx.WithBlockGasMeter(sdk.NewGasMeter(10000000000000000000)) + }, + true, false, + tx2Priority, + }, + { + "success - dynamic fee tx", + dynamicFeeTx, + tx2GasLimit, // it's capped + func() { + vmdb.AddBalance(addr, big.NewInt(1001000000000000)) suite.ctx = suite.ctx.WithBlockGasMeter(sdk.NewGasMeter(10000000000000000000)) }, true, false, + dynamicFeeTxPriority, }, } @@ -306,6 +339,7 @@ func (suite AnteTestSuite) TestEthGasConsumeDecorator() { ctx, err := dec.AnteHandle(suite.ctx.WithIsCheckTx(true).WithGasMeter(sdk.NewInfiniteGasMeter()), tc.tx, false, NextFn) if tc.expPass { suite.Require().NoError(err) + suite.Require().Equal(tc.expPriority, ctx.Priority()) } else { suite.Require().Error(err) } diff --git a/app/ante/handler_options.go b/app/ante/handler_options.go index d3568d90e1..cc2446934d 100644 --- a/app/ante/handler_options.go +++ b/app/ante/handler_options.go @@ -82,7 +82,7 @@ func newCosmosAnteHandler(options HandlerOptions) sdk.AnteHandler { ante.NewSigGasConsumeDecorator(options.AccountKeeper, options.SigGasConsumer), ante.NewSigVerificationDecorator(options.AccountKeeper, options.SignModeHandler), ante.NewIncrementSequenceDecorator(options.AccountKeeper), - ibcante.NewRedundancyDecorator(options.IBCKeeper), + ibcante.NewRedundantRelayDecorator(options.IBCKeeper), NewGasWantedDecorator(options.EvmKeeper, options.FeeMarketKeeper), ) } @@ -106,7 +106,7 @@ func newCosmosAnteHandlerEip712(options HandlerOptions) sdk.AnteHandler { // Note: signature verification uses EIP instead of the cosmos signature validator NewEip712SigVerificationDecorator(options.AccountKeeper, options.SignModeHandler), ante.NewIncrementSequenceDecorator(options.AccountKeeper), - ibcante.NewRedundancyDecorator(options.IBCKeeper), + ibcante.NewRedundantRelayDecorator(options.IBCKeeper), NewGasWantedDecorator(options.EvmKeeper, options.FeeMarketKeeper), ) } diff --git a/app/ante/interfaces.go b/app/ante/interfaces.go index 216b0aa4f0..83be8b8855 100644 --- a/app/ante/interfaces.go +++ b/app/ante/interfaces.go @@ -23,7 +23,7 @@ type EVMKeeper interface { NewEVM(ctx sdk.Context, msg core.Message, cfg *evmtypes.EVMConfig, tracer vm.EVMLogger, stateDB vm.StateDB) *vm.EVM DeductTxCostsFromUserBalance( ctx sdk.Context, msgEthTx evmtypes.MsgEthereumTx, txData evmtypes.TxData, denom string, homestead, istanbul, london bool, - ) (sdk.Coins, error) + ) (fees sdk.Coins, priority int64, err error) GetBaseFee(ctx sdk.Context, ethCfg *params.ChainConfig) *big.Int GetBalance(ctx sdk.Context, addr common.Address) *big.Int ResetTransientGasUsed(ctx sdk.Context) diff --git a/default.nix b/default.nix index 066b40939f..4bc36173d6 100644 --- a/default.nix +++ b/default.nix @@ -5,7 +5,7 @@ let version = "v0.17.1"; pname = "ethermintd"; - tags = [ "ledger" "netgo" ]; + tags = [ "netgo" ]; ldflags = lib.concatStringsSep "\n" ([ "-X github.com/cosmos/cosmos-sdk/version.Name=ethermint" "-X github.com/cosmos/cosmos-sdk/version.AppName=${pname}" diff --git a/go.mod b/go.mod index 036486500d..f29cd71873 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/btcsuite/btcutil v1.0.3-0.20201208143702-a53e38424cce github.com/cosmos/cosmos-sdk v0.46.0 github.com/cosmos/go-bip39 v1.0.0 - github.com/cosmos/ibc-go/v5 v5.0.0 + github.com/cosmos/ibc-go/v5 v5.0.0-beta1 github.com/davecgh/go-spew v1.1.1 github.com/ethereum/go-ethereum v1.10.19 github.com/gogo/protobuf v1.3.3 @@ -185,8 +185,6 @@ require ( replace ( github.com/99designs/keyring => github.com/cosmos/keyring v1.1.7-0.20210622111912-ef00f8ac3d76 - github.com/cosmos/ibc-go/v5 => github.com/notional-labs/ibc-go/v5 v5.0.0-20220728121949-040aca93dda5 - // Fix upstream GHSA-h395-qcrw-5vmq vulnerability. // TODO Remove it: https://github.com/cosmos/cosmos-sdk/issues/10409 github.com/gin-gonic/gin => github.com/gin-gonic/gin v1.7.0 diff --git a/go.sum b/go.sum index c57f3abbfb..f03ce59762 100644 --- a/go.sum +++ b/go.sum @@ -324,6 +324,8 @@ github.com/cosmos/gorocksdb v1.2.0 h1:d0l3jJG8M4hBouIZq0mDUHZ+zjOx044J3nGRskwTb4 github.com/cosmos/gorocksdb v1.2.0/go.mod h1:aaKvKItm514hKfNJpUJXnnOWeBnk2GL4+Qw9NHizILw= github.com/cosmos/iavl v0.19.0 h1:sgyrjqOkycXiN7Tuupuo4QAldKFg7Sipyfeg/IL7cps= github.com/cosmos/iavl v0.19.0/go.mod h1:l5h9pAB3m5fihB3pXVgwYqdY8aBsMagqz7T0MUjxZeA= +github.com/cosmos/ibc-go/v5 v5.0.0-beta1 h1:YqC9giQlZId8Wui8xpaUFI+TpVmEupQZSoDlmxAu6yI= +github.com/cosmos/ibc-go/v5 v5.0.0-beta1/go.mod h1:9mmcbzuidgX7nhafIKng/XhXAHDEnRqDjGy/60W1cvg= github.com/cosmos/keyring v1.1.7-0.20210622111912-ef00f8ac3d76 h1:DdzS1m6o/pCqeZ8VOAit/gyATedRgjvkVI+UCrLpyuU= github.com/cosmos/keyring v1.1.7-0.20210622111912-ef00f8ac3d76/go.mod h1:0mkLWIoZuQ7uBoospo5Q9zIpqq6rYCPJDSUdeCJvPM8= github.com/cosmos/ledger-cosmos-go v0.11.1 h1:9JIYsGnXP613pb2vPjFeMMjBI5lEDsEaF6oYorTy6J4= @@ -1074,8 +1076,6 @@ github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLA github.com/nishanths/exhaustive v0.7.11/go.mod h1:gX+MP7DWMKJmNa1HfMozK+u04hQd3na9i0hyqf3/dOI= github.com/nishanths/predeclared v0.0.0-20190419143655-18a43bb90ffc/go.mod h1:62PewwiQTlm/7Rj+cxVYqZvDIUc+JjZq6GHAC1fsObQ= github.com/nishanths/predeclared v0.2.2/go.mod h1:RROzoN6TnGQupbC+lqggsOlcgysk3LMK/HI84Mp280c= -github.com/notional-labs/ibc-go/v5 v5.0.0-20220728121949-040aca93dda5 h1:G/tpnrLpUj8avmWtggo0L3f8u9gS8s4SqCGrXvlEO5w= -github.com/notional-labs/ibc-go/v5 v5.0.0-20220728121949-040aca93dda5/go.mod h1:KxDAWaeoibbXS0remZwoFz4nil+6+Bi4X3XQh0+23Io= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU= diff --git a/gomod2nix.toml b/gomod2nix.toml index 87df9a1739..081d147b24 100644 --- a/gomod2nix.toml +++ b/gomod2nix.toml @@ -102,9 +102,8 @@ schema = 3 version = "v0.19.0" hash = "sha256-NunBVGJqJkpOMcTmFUCnXQiJSjgNEuxlMu+bkj33wIs=" [mod."github.com/cosmos/ibc-go/v5"] - version = "v5.0.0-20220728121949-040aca93dda5" - hash = "sha256-G+hffr22KJZlbshH9HkqMP8m9XLKcSSzwpKCx9cphbo=" - replaced = "github.com/notional-labs/ibc-go/v5" + version = "v5.0.0-beta1" + hash = "sha256-v+dGnhNSNkkQzuCUoWmVGMPQCEb6aL/oB7zxbYw9owE=" [mod."github.com/cosmos/ledger-cosmos-go"] version = "v0.11.1" hash = "sha256-yli+VvVtZmHo2LPvCY6lYVUfcCDn3sBLDL+a8KIlqDA=" diff --git a/rpc/namespaces/ethereum/eth/api.go b/rpc/namespaces/ethereum/eth/api.go index b72687b8c5..9243993266 100644 --- a/rpc/namespaces/ethereum/eth/api.go +++ b/rpc/namespaces/ethereum/eth/api.go @@ -974,7 +974,7 @@ func (e *PublicAPI) GetTransactionReceipt(hash common.Hash) (map[string]interfac // tolerate the error for pruned node. e.logger.Error("fetch basefee failed, node is pruned?", "height", res.Height, "error", err) } else { - receipt["effectiveGasPrice"] = hexutil.Big(*dynamicTx.GetEffectiveGasPrice(baseFee)) + receipt["effectiveGasPrice"] = hexutil.Big(*dynamicTx.EffectiveGasPrice(baseFee)) } } diff --git a/tests/integration_tests/configs/default.jsonnet b/tests/integration_tests/configs/default.jsonnet index b04cc7ce9a..deffe43c88 100644 --- a/tests/integration_tests/configs/default.jsonnet +++ b/tests/integration_tests/configs/default.jsonnet @@ -1,8 +1,18 @@ { dotenv: '../../../scripts/.env', - 'ethermintd_777-1': { + 'ethermint_9000-1': { cmd: 'ethermintd', 'start-flags': '--trace', + config: { + consensus: { + // larger timeout for more stable mempool tests + timeout_commit: '10s', + }, + mempool: { + // use v1 mempool to enable tx prioritization + version: 'v1', + }, + }, 'app-config': { 'minimum-gas-prices': '0aphoton', 'index-events': ['ethereum_tx.ethereumTxHash'], diff --git a/tests/integration_tests/network.py b/tests/integration_tests/network.py index e4f9950bbf..a843a340b7 100644 --- a/tests/integration_tests/network.py +++ b/tests/integration_tests/network.py @@ -4,7 +4,6 @@ import subprocess from pathlib import Path -import tomlkit import web3 from pystarport import ports from web3.middleware import geth_poa_middleware @@ -61,9 +60,10 @@ def __init__(self, w3): def setup_ethermint(path, base_port): - cfg = Path(__file__).parent / "../../scripts/ethermint-devnet.yaml" + cfg = Path(__file__).parent / "configs/default.jsonnet" yield from setup_custom_ethermint(path, base_port, cfg) + def setup_geth(path, base_port): with (path / "geth.log").open("w") as logfile: cmd = [ diff --git a/tests/integration_tests/test_priority.py b/tests/integration_tests/test_priority.py new file mode 100644 index 0000000000..f51eb4ea2f --- /dev/null +++ b/tests/integration_tests/test_priority.py @@ -0,0 +1,114 @@ +from .network import Ethermint +from .utils import KEYS, sign_transaction + +PRIORITY_REDUCTION = 1000000 + + +def effective_gas_price(tx, base_fee): + if "maxFeePerGas" in tx: + # dynamic fee tx + return min(base_fee + tx["maxPriorityFeePerGas"], tx["maxFeePerGas"]) + else: + # legacy tx + return tx["gasPrice"] + + +def tx_priority(tx, base_fee): + if "maxFeePerGas" in tx: + # dynamic fee tx + return ( + min(tx["maxPriorityFeePerGas"], tx["maxFeePerGas"] - base_fee) + // PRIORITY_REDUCTION + ) + else: + # legacy tx + return (tx["gasPrice"] - base_fee) // PRIORITY_REDUCTION + + +def test_priority(ethermint: Ethermint): + """ + test priorities of different tx types + """ + w3 = ethermint.w3 + amount = 10000 + base_fee = w3.eth.get_block("latest").baseFeePerGas + + # [ ( sender, tx ), ... ] + # use different senders to avoid nonce conflicts + test_cases = [ + ( + "validator", + { + "to": "0x0000000000000000000000000000000000000000", + "value": amount, + "gas": 21000, + "maxFeePerGas": base_fee + PRIORITY_REDUCTION * 6, + "maxPriorityFeePerGas": 0, + }, + ), + ( + "community", + { + "to": "0x0000000000000000000000000000000000000000", + "value": amount, + "gas": 21000, + "gasPrice": base_fee + PRIORITY_REDUCTION * 2, + }, + ), + ( + "signer2", + { + "to": "0x0000000000000000000000000000000000000000", + "value": amount, + "gasPrice": base_fee + PRIORITY_REDUCTION * 4, + "accessList": [ + { + "address": "0xde0b295669a9fd93d5f28d9ec85e40f4cb697bae", + "storageKeys": ( + "0x00000000000000000000000000000000000000000000000000000000" + "00000003", + "0x00000000000000000000000000000000000000000000000000000000" + "00000007", + ), + } + ], + }, + ), + ( + "signer1", + { + "to": "0x0000000000000000000000000000000000000000", + "value": amount, + "gas": 21000, + "maxFeePerGas": base_fee + PRIORITY_REDUCTION * 6, + "maxPriorityFeePerGas": PRIORITY_REDUCTION * 6, + }, + ), + ] + + # test cases are ordered by priority + priorities = [tx_priority(tx, base_fee) for _, tx in test_cases] + assert all(a < b for a, b in zip(priorities, priorities[1:])) + + signed = [sign_transaction(w3, tx, key=KEYS[sender]) for sender, tx in test_cases] + # send the txs from low priority to high, + # but the later sent txs should be included earlier. + txhashes = [w3.eth.send_raw_transaction(tx.rawTransaction) for tx in signed] + + receipts = [w3.eth.wait_for_transaction_receipt(txhash) for txhash in txhashes] + print(receipts) + assert all(receipt.status == 1 for receipt in receipts), "expect all txs success" + + # the later txs should be included earlier because of higher priority + # FIXME there's some non-deterministics due to mempool logic + assert all(included_earlier(r2, r1) for r1, r2 in zip(receipts, receipts[1:])) + + +def included_earlier(receipt1, receipt2): + "returns true if receipt1 is earlier than receipt2" + if receipt1.blockNumber < receipt2.blockNumber: + return True + elif receipt1.blockNumber == receipt2.blockNumber: + return receipt1.transactionIndex < receipt2.transactionIndex + else: + return False diff --git a/tests/integration_tests/utils.py b/tests/integration_tests/utils.py index c75de5b625..3074628d67 100644 --- a/tests/integration_tests/utils.py +++ b/tests/integration_tests/utils.py @@ -1,6 +1,22 @@ +import os import socket import time +from eth_account import Account +from web3._utils.transactions import fill_nonce, fill_transaction_defaults + +Account.enable_unaudited_hdwallet_features() + +ACCOUNTS = { + "validator": Account.from_mnemonic(os.getenv("VALIDATOR1_MNEMONIC")), + "community": Account.from_mnemonic(os.getenv("COMMUNITY_MNEMONIC")), + "signer1": Account.from_mnemonic(os.getenv("SIGNER1_MNEMONIC")), + "signer2": Account.from_mnemonic(os.getenv("SIGNER2_MNEMONIC")), +} +KEYS = {name: account.key for name, account in ACCOUNTS.items()} +ADDRS = {name: account.address for name, account in ACCOUNTS.items()} + + def wait_for_port(port, host="127.0.0.1", timeout=40.0): start_time = time.perf_counter() while True: @@ -14,3 +30,21 @@ def wait_for_port(port, host="127.0.0.1", timeout=40.0): "Waited too long for the port {} on host {} to start accepting " "connections.".format(port, host) ) from ex + + +def fill_defaults(w3, tx): + return fill_nonce(w3, fill_transaction_defaults(w3, tx)) + + +def sign_transaction(w3, tx, key=KEYS["validator"]): + "fill default fields and sign" + acct = Account.from_key(key) + tx["from"] = acct.address + tx = fill_defaults(w3, tx) + return acct.sign_transaction(tx) + + +def send_transaction(w3, tx, key=KEYS["validator"]): + signed = sign_transaction(w3, tx, key) + txhash = w3.eth.send_raw_transaction(signed.rawTransaction) + return w3.eth.wait_for_transaction_receipt(txhash) diff --git a/x/evm/handler_test.go b/x/evm/handler_test.go index e2de145d25..117e927ea5 100644 --- a/x/evm/handler_test.go +++ b/x/evm/handler_test.go @@ -569,13 +569,14 @@ func (suite *EvmTestSuite) TestERC20TransferReverted() { k.SetHooks(tc.hooks) // add some fund to pay gas fee - k.SetBalance(suite.ctx, suite.from, big.NewInt(10000000000)) + k.SetBalance(suite.ctx, suite.from, big.NewInt(1000000000000000)) contract := suite.deployERC20Contract() data, err := types.ERC20Contract.ABI.Pack("transfer", suite.from, big.NewInt(10)) suite.Require().NoError(err) + gasPrice := big.NewInt(1000000000) // must be bigger than or equal to baseFee nonce := k.GetNonce(suite.ctx, suite.from) tx := types.NewTx( suite.chainID, @@ -583,7 +584,7 @@ func (suite *EvmTestSuite) TestERC20TransferReverted() { &contract, big.NewInt(0), tc.gasLimit, - big.NewInt(1), + gasPrice, nil, nil, data, @@ -595,7 +596,7 @@ func (suite *EvmTestSuite) TestERC20TransferReverted() { txData, err := types.UnpackTxData(tx.Data) suite.Require().NoError(err) - _, err = k.DeductTxCostsFromUserBalance(suite.ctx, *tx, txData, "aphoton", true, true, true) + _, _, err = k.DeductTxCostsFromUserBalance(suite.ctx, *tx, txData, "aphoton", true, true, true) suite.Require().NoError(err) res, err := k.EthereumTx(sdk.WrapSDKContext(suite.ctx), tx) @@ -614,7 +615,7 @@ func (suite *EvmTestSuite) TestERC20TransferReverted() { } // check gas refund works: only deducted fee for gas used, rather than gas limit. - suite.Require().Equal(big.NewInt(int64(res.GasUsed)), new(big.Int).Sub(before, after)) + suite.Require().Equal(new(big.Int).Mul(gasPrice, big.NewInt(int64(res.GasUsed))), new(big.Int).Sub(before, after)) // nonce should not be increased. nonce2 := k.GetNonce(suite.ctx, suite.from) diff --git a/x/evm/keeper/integration_test.go b/x/evm/keeper/integration_test.go index e342afb118..6c51680449 100644 --- a/x/evm/keeper/integration_test.go +++ b/x/evm/keeper/integration_test.go @@ -29,9 +29,7 @@ import ( ) var _ = Describe("Feemarket", func() { - var ( - privKey *ethsecp256k1.PrivKey - ) + var privKey *ethsecp256k1.PrivKey Describe("Performing EVM transactions", func() { type txParams struct { diff --git a/x/evm/keeper/keeper.go b/x/evm/keeper/keeper.go index 619c21f3c5..1ebb77b26f 100644 --- a/x/evm/keeper/keeper.go +++ b/x/evm/keeper/keeper.go @@ -293,7 +293,11 @@ func (k *Keeper) GetBalance(ctx sdk.Context, addr common.Address) *big.Int { // - `0`: london hardfork enabled but feemarket is not enabled. // - `n`: both london hardfork and feemarket are enabled. func (k Keeper) GetBaseFee(ctx sdk.Context, ethCfg *params.ChainConfig) *big.Int { - if !types.IsLondon(ethCfg, ctx.BlockHeight()) { + return k.getBaseFee(ctx, types.IsLondon(ethCfg, ctx.BlockHeight())) +} + +func (k Keeper) getBaseFee(ctx sdk.Context, london bool) *big.Int { + if !london { return nil } baseFee := k.feeMarketKeeper.GetBaseFee(ctx) diff --git a/x/evm/keeper/utils.go b/x/evm/keeper/utils.go index 93d4e79580..4198a95bba 100644 --- a/x/evm/keeper/utils.go +++ b/x/evm/keeper/utils.go @@ -1,6 +1,7 @@ package keeper import ( + "math" "math/big" sdkmath "cosmossdk.io/math" @@ -14,20 +15,26 @@ import ( ethtypes "github.com/ethereum/go-ethereum/core/types" ) +// DefaultPriorityReduction is the default amount of price values required for 1 unit of priority. +// Because priority is `int64` while price is `big.Int`, it's necessary to scale down the range to keep it more pratical. +// The default value is the same as the `sdk.DefaultPowerReduction`. +var DefaultPriorityReduction = sdk.DefaultPowerReduction + // DeductTxCostsFromUserBalance it calculates the tx costs and deducts the fees +// returns (effectiveFee, priority, error) func (k Keeper) DeductTxCostsFromUserBalance( ctx sdk.Context, msgEthTx evmtypes.MsgEthereumTx, txData evmtypes.TxData, denom string, homestead, istanbul, london bool, -) (sdk.Coins, error) { +) (fees sdk.Coins, priority int64, err error) { isContractCreation := txData.GetTo() == nil // fetch sender account from signature signerAcc, err := authante.GetSignerAcc(ctx, k.accountKeeper, msgEthTx.GetFrom()) if err != nil { - return nil, sdkerrors.Wrapf(err, "account not found for sender %s", msgEthTx.From) + return nil, 0, sdkerrors.Wrapf(err, "account not found for sender %s", msgEthTx.From) } gasLimit := txData.GetGas() @@ -39,7 +46,7 @@ func (k Keeper) DeductTxCostsFromUserBalance( intrinsicGas, err := core.IntrinsicGas(txData.GetData(), accessList, isContractCreation, homestead, istanbul) if err != nil { - return nil, sdkerrors.Wrapf( + return nil, 0, sdkerrors.Wrapf( err, "failed to retrieve intrinsic gas, contract creation = %t; homestead = %t, istanbul = %t", isContractCreation, homestead, istanbul, @@ -48,7 +55,7 @@ func (k Keeper) DeductTxCostsFromUserBalance( // intrinsic gas verification during CheckTx if ctx.IsCheckTx() && gasLimit < intrinsicGas { - return nil, sdkerrors.Wrapf( + return nil, 0, sdkerrors.Wrapf( sdkerrors.ErrOutOfGas, "gas limit too low: %d (gas limit) < %d (intrinsic gas)", gasLimit, intrinsicGas, ) @@ -56,33 +63,42 @@ func (k Keeper) DeductTxCostsFromUserBalance( var feeAmt *big.Int - feeMktParams := k.feeMarketKeeper.GetParams(ctx) - if london && feeMktParams.IsBaseFeeEnabled(ctx.BlockHeight()) && txData.TxType() == ethtypes.DynamicFeeTxType { - baseFee := k.feeMarketKeeper.GetBaseFee(ctx) - if txData.GetGasFeeCap().Cmp(baseFee) < 0 { - return nil, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "the tx gasfeecap is lower than the tx baseFee: %s (gasfeecap), %s (basefee) ", txData.GetGasFeeCap(), baseFee) - } - feeAmt = txData.EffectiveFee(baseFee) - } else { - feeAmt = txData.Fee() + baseFee := k.getBaseFee(ctx, london) + if baseFee != nil && txData.GetGasFeeCap().Cmp(baseFee) < 0 { + return nil, 0, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "the tx gasfeecap is lower than the tx baseFee: %s (gasfeecap), %s (basefee) ", txData.GetGasFeeCap(), baseFee) } + feeAmt = txData.EffectiveFee(baseFee) if feeAmt.Sign() == 0 { // zero fee, no need to deduct - return sdk.Coins{}, nil + return sdk.Coins{}, 0, nil } - fees := sdk.Coins{sdk.NewCoin(denom, sdkmath.NewIntFromBigInt(feeAmt))} + fees = sdk.Coins{sdk.NewCoin(denom, sdkmath.NewIntFromBigInt(feeAmt))} // deduct the full gas cost from the user balance if err := authante.DeductFees(k.bankKeeper, ctx, signerAcc, fees); err != nil { - return nil, sdkerrors.Wrapf( + return nil, 0, sdkerrors.Wrapf( err, "failed to deduct full gas cost %s from the user %s balance", fees, msgEthTx.From, ) } - return fees, nil + + // calculate priority based on effective gas price + tipPrice := txData.EffectiveGasPrice(baseFee) + // if london hardfork is not enabled, tipPrice is the gasPrice + if baseFee != nil { + tipPrice = new(big.Int).Sub(tipPrice, baseFee) + } + priorityBig := new(big.Int).Quo(tipPrice, DefaultPriorityReduction.BigInt()) + if !priorityBig.IsInt64() { + priority = math.MaxInt64 + } else { + priority = priorityBig.Int64() + } + + return fees, priority, nil } // CheckSenderBalance validates that the tx cost value is positive and that the diff --git a/x/evm/keeper/utils_test.go b/x/evm/keeper/utils_test.go index 214329dfec..6f4074c76c 100644 --- a/x/evm/keeper/utils_test.go +++ b/x/evm/keeper/utils_test.go @@ -403,7 +403,7 @@ func (suite *KeeperTestSuite) TestDeductTxCostsFromUserBalance() { txData, _ := evmtypes.UnpackTxData(tx.Data) - fees, err := suite.app.EvmKeeper.DeductTxCostsFromUserBalance( + fees, priority, err := suite.app.EvmKeeper.DeductTxCostsFromUserBalance( suite.ctx, *tx, txData, @@ -424,6 +424,7 @@ func (suite *KeeperTestSuite) TestDeductTxCostsFromUserBalance() { ), "valid test %d failed, fee value is wrong ", i, ) + suite.Require().Equal(int64(0), priority) } else { suite.Require().Equal( fees, diff --git a/x/evm/types/access_list_tx.go b/x/evm/types/access_list_tx.go index 1f04746700..d684f8722c 100644 --- a/x/evm/types/access_list_tx.go +++ b/x/evm/types/access_list_tx.go @@ -232,6 +232,11 @@ func (tx AccessListTx) Cost() *big.Int { return cost(tx.Fee(), tx.GetValue()) } +// EffectiveGasPrice is the same as GasPrice for AccessListTx +func (tx AccessListTx) EffectiveGasPrice(baseFee *big.Int) *big.Int { + return tx.GetGasPrice() +} + // EffectiveFee is the same as Fee for AccessListTx func (tx AccessListTx) EffectiveFee(baseFee *big.Int) *big.Int { return tx.Fee() diff --git a/x/evm/types/dynamic_fee_tx.go b/x/evm/types/dynamic_fee_tx.go index 05568ea5ef..e094e92cd8 100644 --- a/x/evm/types/dynamic_fee_tx.go +++ b/x/evm/types/dynamic_fee_tx.go @@ -265,14 +265,14 @@ func (tx DynamicFeeTx) Cost() *big.Int { return cost(tx.Fee(), tx.GetValue()) } -// GetEffectiveGasPrice returns the effective gas price -func (tx *DynamicFeeTx) GetEffectiveGasPrice(baseFee *big.Int) *big.Int { +// EffectiveGasPrice returns the effective gas price +func (tx *DynamicFeeTx) EffectiveGasPrice(baseFee *big.Int) *big.Int { return math.BigMin(new(big.Int).Add(tx.GasTipCap.BigInt(), baseFee), tx.GasFeeCap.BigInt()) } // EffectiveFee returns effective_gasprice * gaslimit. func (tx DynamicFeeTx) EffectiveFee(baseFee *big.Int) *big.Int { - return fee(tx.GetEffectiveGasPrice(baseFee), tx.GasLimit) + return fee(tx.EffectiveGasPrice(baseFee), tx.GasLimit) } // EffectiveCost returns amount + effective_gasprice * gaslimit. diff --git a/x/evm/types/legacy_tx.go b/x/evm/types/legacy_tx.go index c965d1a6b9..e6ddc0848e 100644 --- a/x/evm/types/legacy_tx.go +++ b/x/evm/types/legacy_tx.go @@ -201,6 +201,11 @@ func (tx LegacyTx) Cost() *big.Int { return cost(tx.Fee(), tx.GetValue()) } +// EffectiveGasPrice is the same as GasPrice for LegacyTx +func (tx LegacyTx) EffectiveGasPrice(baseFee *big.Int) *big.Int { + return tx.GetGasPrice() +} + // EffectiveFee is the same as Fee for LegacyTx func (tx LegacyTx) EffectiveFee(baseFee *big.Int) *big.Int { return tx.Fee() diff --git a/x/evm/types/tx_data.go b/x/evm/types/tx_data.go index 73d8b404fd..aa3b13c9a0 100644 --- a/x/evm/types/tx_data.go +++ b/x/evm/types/tx_data.go @@ -41,7 +41,8 @@ type TxData interface { Fee() *big.Int Cost() *big.Int - // effective fee according to current base fee + // effective gasPrice/fee/cost according to current base fee + EffectiveGasPrice(baseFee *big.Int) *big.Int EffectiveFee(baseFee *big.Int) *big.Int EffectiveCost(baseFee *big.Int) *big.Int } diff --git a/x/feemarket/keeper/integration_test.go b/x/feemarket/keeper/integration_test.go index f9530d5067..41b4799133 100644 --- a/x/feemarket/keeper/integration_test.go +++ b/x/feemarket/keeper/integration_test.go @@ -380,7 +380,6 @@ var _ = Describe("Feemarket", func() { 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) diff --git a/x/feemarket/spec/09_antehandlers.md b/x/feemarket/spec/09_antehandlers.md index f6f544d40c..841f07cb27 100644 --- a/x/feemarket/spec/09_antehandlers.md +++ b/x/feemarket/spec/09_antehandlers.md @@ -6,7 +6,7 @@ order: 5 The `x/feemarket` module provides `AnteDecorator`s that are recursively chained together into a single [`Antehandler`](https://github.com/cosmos/cosmos-sdk/blob/v0.43.0-alpha1/docs/architecture/adr-010-modular-antehandler.md). These decorators perform basic validity checks on an Ethereum or Cosmos SDK transaction, such that it could be thrown out of the transaction Mempool. -Note that the `AnteHandler` is run for every transaction and called on both `CheckTx` and `DeliverTx`. +Note that the `AnteHandler` is run for every transaction and called on both `CheckTx` and `DeliverTx`. ## Decorators @@ -23,3 +23,15 @@ Rejects EVM transactions with transactions fees lower than `MinGasPrice * GasLim ::: tip **Note**: For dynamic transactions, if the `feemarket` formula results in a `BaseFee` that lowers `EffectivePrice < MinGasPrices`, the users must increase the `GasTipCap` (priority fee) until `EffectivePrice > MinGasPrices`. Transactions with `MinGasPrices * GasLimit < transaction fee < EffectiveFee` are rejected by the `feemarket` `AnteHandle`. ::: + +### `EthGasConsumeDecorator` + +Calculates the effective fees to deduct and the tx priority according to EIP-1559 spec, then deducts the fees and sets the tx priority in the response. + +``` +effectivePrice = min(baseFee + tipFeeCap, gasFeeCap) +effectiveTipFee = effectivePrice - baseFee +priority = effectiveTipFee / DefaultPriorityReduction +``` + +When there are multiple messages in the transaction, choose the lowest priority in them.