From 617eac98d1fbe74040add1508992f9c7eeda898b Mon Sep 17 00:00:00 2001 From: mmsqe Date: Fri, 4 Oct 2024 15:19:15 +0800 Subject: [PATCH] Problem: validation broke after transaction conversion with raw field (#536) * Problem: validation broke after transaction conversion with raw field * cleanup * less validate * borrow * Apply suggestions from code review Signed-off-by: yihuang * rm chainid check --------- Signed-off-by: yihuang Co-authored-by: yihuang --- CHANGELOG.md | 1 + tests/integration_tests/test_priority.py | 24 +++++- x/evm/types/eth.go | 15 +++- x/evm/types/msg.go | 2 - x/evm/types/msg_test.go | 96 ++++++++++++++++++++++++ 5 files changed, 131 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 372dbc4ac4..8eba179458 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (rpc) [#516](https://github.com/crypto-org-chain/ethermint/pull/516) Avoid method eth_chainId crashed due to nil pointer on IsEIP155 check. * (cli) [#524](https://github.com/crypto-org-chain/ethermint/pull/524) Allow tx evm raw run for generate only when offline with evm-denom flag. * (rpc) [#527](https://github.com/crypto-org-chain/ethermint/pull/527) Fix balance consistency between trace-block and state machine. +* (rpc) [#536](https://github.com/crypto-org-chain/ethermint/pull/536) Fix validate basic after transaction conversion with raw field. * (cli) [#537](https://github.com/crypto-org-chain/ethermint/pull/537) Fix unsuppored sign mode SIGN_MODE_TEXTUAL for bank transfer. ### Improvements diff --git a/tests/integration_tests/test_priority.py b/tests/integration_tests/test_priority.py index b899fcf22d..e1191e138b 100644 --- a/tests/integration_tests/test_priority.py +++ b/tests/integration_tests/test_priority.py @@ -3,7 +3,14 @@ import pytest from .network import setup_ethermint -from .utils import ADDRS, KEYS, eth_to_bech32, sign_transaction, wait_for_new_blocks +from .utils import ( + ADDRS, + KEYS, + eth_to_bech32, + send_transaction, + sign_transaction, + wait_for_new_blocks, +) PRIORITY_REDUCTION = 1000000 @@ -202,3 +209,18 @@ def test_native_tx_priority(ethermint): def get_max_priority_price(max_priority_price): "default to max int64 if None" return max_priority_price if max_priority_price is not None else sys.maxsize + + +def test_validate(ethermint): + w3 = ethermint.w3 + gas = int(1.2 * w3.eth.gas_price) + tx = { + "to": "0x0000000000000000000000000000000000000000", + "value": 1, + "gas": 21000, + "maxFeePerGas": gas, + "maxPriorityFeePerGas": gas + 1, + } + with pytest.raises(ValueError) as exc: + send_transaction(w3, tx) + assert "max priority fee per gas higher than max fee per gas" in str(exc) diff --git a/x/evm/types/eth.go b/x/evm/types/eth.go index 81290c20f8..4ac6e9fbaa 100644 --- a/x/evm/types/eth.go +++ b/x/evm/types/eth.go @@ -72,17 +72,24 @@ func (tx EthereumTx) Validate() error { if tx.Gas() == 0 { return errorsmod.Wrap(ErrInvalidGasLimit, "gas limit must not be zero") } - if !types.IsValidInt256(tx.GasPrice()) { + if tx.GasPrice().BitLen() > 256 { return errorsmod.Wrap(ErrInvalidGasPrice, "out of bound") } - if !types.IsValidInt256(tx.GasFeeCap()) { + if tx.GasFeeCap().BitLen() > 256 { return errorsmod.Wrap(ErrInvalidGasPrice, "out of bound") } - if !types.IsValidInt256(tx.GasTipCap()) { + if tx.GasTipCap().BitLen() > 256 { return errorsmod.Wrap(ErrInvalidGasPrice, "out of bound") } - if !types.IsValidInt256(tx.Cost()) { + if tx.Cost().BitLen() > 256 { return errorsmod.Wrap(ErrInvalidGasFee, "out of bound") } + if tx.GasFeeCapIntCmp(tx.GasTipCap()) < 0 { + return errorsmod.Wrapf( + ErrInvalidGasCap, + "max priority fee per gas higher than max fee per gas (%s > %s)", + tx.GasTipCap(), tx.GasFeeCap(), + ) + } return nil } diff --git a/x/evm/types/msg.go b/x/evm/types/msg.go index c2089b18f9..9b395a46ba 100644 --- a/x/evm/types/msg.go +++ b/x/evm/types/msg.go @@ -178,11 +178,9 @@ func (msg MsgEthereumTx) ValidateBasic() error { if msg.Data != nil { return errorsmod.Wrapf(errortypes.ErrInvalidRequest, "tx data is deprecated in favor of Raw") } - if err := msg.Raw.Validate(); err != nil { return err } - return nil } diff --git a/x/evm/types/msg_test.go b/x/evm/types/msg_test.go index fbaaa66d49..f2ff8992f2 100644 --- a/x/evm/types/msg_test.go +++ b/x/evm/types/msg_test.go @@ -19,6 +19,7 @@ import ( "github.com/evmos/ethermint/crypto/ethsecp256k1" "github.com/evmos/ethermint/encoding" "github.com/evmos/ethermint/tests" + ethermint "github.com/evmos/ethermint/types" "github.com/evmos/ethermint/x/evm/types" ) @@ -782,3 +783,98 @@ func assertEqual(orig *ethtypes.Transaction, cpy *ethtypes.Transaction) error { } return nil } + +func (suite *MsgsTestSuite) TestValidateEthereumTx() { + maxInt256 := ethermint.MaxInt256 + maxInt256Plus1 := new(big.Int).Add(ethermint.MaxInt256, big.NewInt(1)) + normal := big.NewInt(100) + gasLimit := uint64(21000) + testCases := []struct { + name string + tx types.EthereumTx + expError bool + }{ + { + "valid transaction", + types.NewTxWithData(ðtypes.LegacyTx{ + Gas: gasLimit, + GasPrice: normal, + Value: normal, + }).Raw, + false, + }, + { + "zero gas limit", + types.NewTxWithData(ðtypes.LegacyTx{ + Gas: 0, + GasPrice: normal, + Value: normal, + }).Raw, + true, + }, + { + "gas price exceeds int256 limit", + types.NewTxWithData(ðtypes.LegacyTx{ + Value: normal, + Gas: gasLimit, + GasPrice: maxInt256Plus1, + }).Raw, + true, + }, + { + "gas fee cap exceeds int256 limit", + types.NewTxWithData(ðtypes.DynamicFeeTx{ + Value: normal, + Gas: gasLimit, + GasFeeCap: maxInt256Plus1, + }).Raw, + true, + }, + { + "gas tip cap exceeds int256 limit", + types.NewTxWithData(ðtypes.DynamicFeeTx{ + Value: normal, + Gas: gasLimit, + GasFeeCap: normal, + GasTipCap: maxInt256Plus1, + }).Raw, + true, + }, + { + "LegacyTx cost exceeds int256 limit", + types.NewTxWithData(ðtypes.LegacyTx{ + Gas: gasLimit, + GasPrice: maxInt256, + Value: normal, + }).Raw, + true, + }, + { + "DynamicFeeTx cost exceeds int256 limit", + types.NewTxWithData(ðtypes.DynamicFeeTx{ + Gas: gasLimit, + Value: maxInt256Plus1, + }).Raw, + true, + }, + { + "AccessListTx cost exceeds int256 limit", + types.NewTxWithData(ðtypes.AccessListTx{ + Gas: gasLimit, + GasPrice: maxInt256, + Value: normal, + }).Raw, + true, + }, + } + for _, tc := range testCases { + suite.Run(tc.name, func() { + err := tc.tx.Validate() + if tc.expError { + suite.Require().Error(err, tc.name) + } else { + suite.Require().NoError(err, tc.name) + } + }) + } +}