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 <[email protected]>

* rm chainid check

---------

Signed-off-by: yihuang <[email protected]>
Co-authored-by: yihuang <[email protected]>

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

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)

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
 }

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
 }
 

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(&ethtypes.LegacyTx{
+				Gas:      gasLimit,
+				GasPrice: normal,
+				Value:    normal,
+			}).Raw,
+			false,
+		},
+		{
+			"zero gas limit",
+			types.NewTxWithData(&ethtypes.LegacyTx{
+				Gas:      0,
+				GasPrice: normal,
+				Value:    normal,
+			}).Raw,
+			true,
+		},
+		{
+			"gas price exceeds int256 limit",
+			types.NewTxWithData(&ethtypes.LegacyTx{
+				Value:    normal,
+				Gas:      gasLimit,
+				GasPrice: maxInt256Plus1,
+			}).Raw,
+			true,
+		},
+		{
+			"gas fee cap exceeds int256 limit",
+			types.NewTxWithData(&ethtypes.DynamicFeeTx{
+				Value:     normal,
+				Gas:       gasLimit,
+				GasFeeCap: maxInt256Plus1,
+			}).Raw,
+			true,
+		},
+		{
+			"gas tip cap exceeds int256 limit",
+			types.NewTxWithData(&ethtypes.DynamicFeeTx{
+				Value:     normal,
+				Gas:       gasLimit,
+				GasFeeCap: normal,
+				GasTipCap: maxInt256Plus1,
+			}).Raw,
+			true,
+		},
+		{
+			"LegacyTx cost exceeds int256 limit",
+			types.NewTxWithData(&ethtypes.LegacyTx{
+				Gas:      gasLimit,
+				GasPrice: maxInt256,
+				Value:    normal,
+			}).Raw,
+			true,
+		},
+		{
+			"DynamicFeeTx cost exceeds int256 limit",
+			types.NewTxWithData(&ethtypes.DynamicFeeTx{
+				Gas:   gasLimit,
+				Value: maxInt256Plus1,
+			}).Raw,
+			true,
+		},
+		{
+			"AccessListTx cost exceeds int256 limit",
+			types.NewTxWithData(&ethtypes.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)
+			}
+		})
+	}
+}