Fix eth tx hashes in json-rpc responses

Closes: #1175

- Remove Size_ field
- Validate From/Hash fields in ante handler
- Recompute tx hashes in json-rpc apis to cope with old blocks

Update CHANGELOG.md

remove Size_, validate Hash/From, add unit tests

update spec

Update CHANGELOG.md

Update app/ante/eth.go

populate From in SendRawTransaction

Apply suggestions from code review

keep Size_ field to avoid breaking tx format

CHANGELOG.md

@@ -42,6 +42,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 * (deps) [\#1159](https://github.com/evmos/ethermint/pull/1159) Bump Geth version to `v1.10.19`.
 * (deps) [#1167](https://github.com/evmos/ethermint/pull/1167) Upgrade ibc-go to v4.
+* (ante) [#1176](https://github.com/evmos/ethermint/pull/1176) Fix invalid tx hashes; Remove `Size_` field and validate `Hash`/`From` fields in ante handler,
+  recompute eth tx hashes in JSON-RPC APIs to fix old blocks.
 
 ### Improvements
 

app/ante/ante_test.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	"github.com/cosmos/cosmos-sdk/types/tx/signing"
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
@@ -16,19 +17,22 @@ import (
 )
 
 func (suite AnteTestSuite) TestAnteHandler() {
-	suite.enableFeemarket = false
-	suite.SetupTest() // reset
-
+	var acc authtypes.AccountI
 	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)
+	setup := func() {
+		suite.enableFeemarket = false
+		suite.SetupTest() // reset
 
-	suite.app.EvmKeeper.SetBalance(suite.ctx, addr, big.NewInt(10000000000))
+		acc = suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr.Bytes())
+		suite.Require().NoError(acc.SetSequence(1))
+		suite.app.AccountKeeper.SetAccount(suite.ctx, acc)
 
-	suite.app.FeeMarketKeeper.SetBaseFee(suite.ctx, big.NewInt(100))
+		suite.app.EvmKeeper.SetBalance(suite.ctx, addr, big.NewInt(10000000000))
+
+		suite.app.FeeMarketKeeper.SetBaseFee(suite.ctx, big.NewInt(100))
+	}
 
 	testCases := []struct {
 		name      string
@@ -63,7 +67,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
 			func() sdk.Tx {
 				signedContractTx := evmtypes.NewTxContract(
 					suite.app.EvmKeeper.ChainID(),
-					2,
+					1,
 					big.NewInt(10),
 					100000,
 					big.NewInt(150),
@@ -84,7 +88,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
 			func() sdk.Tx {
 				signedContractTx := evmtypes.NewTxContract(
 					suite.app.EvmKeeper.ChainID(),
-					3,
+					1,
 					big.NewInt(10),
 					100000,
 					big.NewInt(150),
@@ -105,7 +109,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
 			func() sdk.Tx {
 				signedTx := evmtypes.NewTx(
 					suite.app.EvmKeeper.ChainID(),
-					4,
+					1,
 					&to,
 					big.NewInt(10),
 					100000,
@@ -127,7 +131,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
 			func() sdk.Tx {
 				signedTx := evmtypes.NewTx(
 					suite.app.EvmKeeper.ChainID(),
-					5,
+					1,
 					&to,
 					big.NewInt(10),
 					100000,
@@ -149,7 +153,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
 			func() sdk.Tx {
 				signedTx := evmtypes.NewTx(
 					suite.app.EvmKeeper.ChainID(),
-					6,
+					1,
 					&to,
 					big.NewInt(10),
 					100000,
@@ -170,7 +174,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
 			func() sdk.Tx {
 				signedTx := evmtypes.NewTx(
 					suite.app.EvmKeeper.ChainID(),
-					7,
+					1,
 					&to,
 					big.NewInt(10),
 					100000,
@@ -189,7 +193,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
 		{
 			"fail - CheckTx (cosmos tx is not valid)",
 			func() sdk.Tx {
-				signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 8, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
+				signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 1, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
 				signedTx.From = addr.Hex()
 
 				txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
@@ -201,7 +205,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
 		{
 			"fail - CheckTx (memo too long)",
 			func() sdk.Tx {
-				signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 5, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
+				signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 1, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
 				signedTx.From = addr.Hex()
 
 				txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
@@ -212,7 +216,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
 		{
 			"fail - CheckTx (ExtensionOptionsEthereumTx not set)",
 			func() sdk.Tx {
-				signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 5, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
+				signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 1, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
 				signedTx.From = addr.Hex()
 
 				txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false, true)
@@ -390,10 +394,75 @@ func (suite AnteTestSuite) TestAnteHandler() {
 				return txBuilder.GetTx()
 			}, false, false, false,
 		},
+		{
+			"fails - invalid tx hash",
+			func() sdk.Tx {
+				msg := evmtypes.NewTxContract(
+					suite.app.EvmKeeper.ChainID(),
+					1,
+					big.NewInt(10),
+					100000,
+					big.NewInt(150),
+					big.NewInt(200),
+					nil,
+					nil,
+					nil,
+				)
+				msg.From = addr.Hex()
+				tx := suite.CreateTestTx(msg, privKey, 1, false)
+				msg = tx.GetMsgs()[0].(*evmtypes.MsgEthereumTx)
+				msg.Hash = "0x00"
+				return tx
+			}, true, false, false,
+		},
+		{
+			"fails - invalid from",
+			func() sdk.Tx {
+				msg := evmtypes.NewTxContract(
+					suite.app.EvmKeeper.ChainID(),
+					1,
+					big.NewInt(10),
+					100000,
+					big.NewInt(150),
+					big.NewInt(200),
+					nil,
+					nil,
+					nil,
+				)
+				msg.From = addr.Hex()
+				tx := suite.CreateTestTx(msg, privKey, 1, false)
+				msg = tx.GetMsgs()[0].(*evmtypes.MsgEthereumTx)
+				msg.From = addr.Hex()
+				return tx
+			}, true, false, false,
+		},
+		{
+			"fails - invalid size",
+			func() sdk.Tx {
+				msg := evmtypes.NewTxContract(
+					suite.app.EvmKeeper.ChainID(),
+					1,
+					big.NewInt(10),
+					100000,
+					big.NewInt(150),
+					big.NewInt(200),
+					nil,
+					nil,
+					nil,
+				)
+				msg.From = addr.Hex()
+				tx := suite.CreateTestTx(msg, privKey, 1, false)
+				msg = tx.GetMsgs()[0].(*evmtypes.MsgEthereumTx)
+				msg.Size_ = 1
+				return tx
+			}, true, false, false,
+		},
 	}
 
 	for _, tc := range testCases {
 		suite.Run(tc.name, func() {
+			setup()
+
 			suite.ctx = suite.ctx.WithIsCheckTx(tc.checkTx).WithIsReCheckTx(tc.reCheckTx)
 
 			// expConsumed := params.TxGasContractCreation + params.TxGas

app/ante/eth.go

@@ -61,13 +61,10 @@ func (esvd EthSigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, s
 		if err != nil {
 			return ctx, sdkerrors.Wrapf(
 				sdkerrors.ErrorInvalidSigner,
-				"couldn't retrieve sender address ('%s') from the ethereum transaction: %s",
-				msgEthTx.From,
+				"couldn't retrieve sender address from the ethereum transaction: %s",
 				err.Error(),
 			)
 		}
-
-		// set up the sender to the transaction field if not already
 		msgEthTx.From = sender.Hex()
 	}
 
@@ -429,6 +426,20 @@ func (vbd EthValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu
 				return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evmtypes.MsgEthereumTx)(nil))
 			}
 
+			// Validate Size_ field, should be kept empty
+			if msgEthTx.Size_ != 0 {
+				return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "dirty tx size field %f, expected: 0", msgEthTx.Size_)
+			}
+			// Validate Hash field
+			expHash := msgEthTx.AsTransaction().Hash().Hex()
+			if msgEthTx.Hash != expHash {
+				return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid tx hash %s, expected: %s", msgEthTx.Hash, expHash)
+			}
+			// Validate `From` field
+			if msgEthTx.From != "" {
+				return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid From %s, expect empty string", msgEthTx.From)
+			}
+
 			txGasLimit += msgEthTx.GetGas()
 
 			txData, err := evmtypes.UnpackTxData(msgEthTx.Data)

app/ante/utils_test.go

@@ -191,6 +191,7 @@ func (suite *AnteTestSuite) CreateTestTxBuilder(
 	err = msg.Sign(suite.ethSigner, tests.NewSigner(priv))
 	suite.Require().NoError(err)
 
+	msg.From = ""
 	err = builder.SetMsgs(msg)
 	suite.Require().NoError(err)
 

docs/api/proto-docs.md

@@ -477,7 +477,7 @@ MsgEthereumTx encapsulates an Ethereum transaction as an SDK message.
 | `data` | [google.protobuf.Any](#google.protobuf.Any) |  | inner transaction data
 
 caches |
-| `size` | [double](#double) |  | encoded storage size of the transaction |
+| `size` | [double](#double) |  | DEPRECATED: encoded storage size of the transaction |
 | `hash` | [string](#string) |  | transaction hash in hex format |
 | `from` | [string](#string) |  | ethereum signer address in hex format. This address value is checked against the address derived from the signature (V, R, S) using the secp256k1 elliptic curve |
 

proto/ethermint/evm/v1/tx.proto

@@ -25,7 +25,7 @@ message MsgEthereumTx {
   google.protobuf.Any data = 1;
   // caches
 
-  // encoded storage size of the transaction
+  // DEPRECATED: encoded storage size of the transaction
   double size = 2 [ (gogoproto.jsontag) = "-" ];
   // transaction hash in hex format
   string hash = 3 [ (gogoproto.moretags) = "rlp:\"-\"" ];

rpc/backend/evm_backend.go

@@ -1009,6 +1009,7 @@ func (b *Backend) GetEthereumMsgsFromTendermintBlock(resBlock *tmrpctypes.Result
 				continue
 			}
 
+			ethMsg.Hash = ethMsg.AsTransaction().Hash().Hex()
 			result = append(result, ethMsg)
 		}
 	}

rpc/namespaces/ethereum/eth/filters/api.go

@@ -157,7 +157,7 @@ func (api *PublicFilterAPI) NewPendingTransactionFilter() rpc.ID {
 					for _, msg := range tx.GetMsgs() {
 						ethTx, ok := msg.(*evmtypes.MsgEthereumTx)
 						if ok {
-							f.hashes = append(f.hashes, common.HexToHash(ethTx.Hash))
+							f.hashes = append(f.hashes, ethTx.AsTransaction().Hash())
 						}
 					}
 				}
@@ -221,7 +221,7 @@ func (api *PublicFilterAPI) NewPendingTransactions(ctx context.Context) (*rpc.Su
 				for _, msg := range tx.GetMsgs() {
 					ethTx, ok := msg.(*evmtypes.MsgEthereumTx)
 					if ok {
-						_ = notifier.Notify(rpcSub.ID, common.HexToHash(ethTx.Hash))
+						_ = notifier.Notify(rpcSub.ID, ethTx.AsTransaction().Hash())
 					}
 				}
 			case <-rpcSub.Err():

rpc/types/utils.go

@@ -34,6 +34,7 @@ func RawTxToEthTx(clientCtx client.Context, txBz tmtypes.Tx) ([]*evmtypes.MsgEth
 		if !ok {
 			return nil, fmt.Errorf("invalid message type %T, expected %T", msg, &evmtypes.MsgEthereumTx{})
 		}
+		ethTx.Hash = ethTx.AsTransaction().Hash().Hex()
 		ethTxs[i] = ethTx
 	}
 	return ethTxs, nil

tests/e2e/integration_test.go

@@ -794,6 +794,7 @@ func (s *IntegrationTestSuite) TestBatchETHTransactions() {
 		err = msgTx.Sign(s.ethSigner, s.network.Validators[0].ClientCtx.Keyring)
 		s.Require().NoError(err)
 
+		msgTx.From = ""
 		msgs = append(msgs, msgTx.GetMsgs()...)
 		txData, err := evmtypes.UnpackTxData(msgTx.Data)
 		s.Require().NoError(err)

x/evm/spec/04_transactions.md

@@ -14,7 +14,7 @@ An EVM state transition can be achieved by using the `MsgEthereumTx`. This messa
 type MsgEthereumTx struct {
  // inner transaction data
  Data *types.Any `protobuf:"bytes,1,opt,name=data,proto3" json:"data,omitempty"`
- // encoded storage size of the transaction
+ // DEPRECATED: encoded storage size of the transaction
  Size_ float64 `protobuf:"fixed64,2,opt,name=size,proto3" json:"-"`
  // transaction hash in hex format
  Hash string `protobuf:"bytes,3,opt,name=hash,proto3" json:"hash,omitempty" rlp:"-"`

x/evm/types/msg.go

@@ -133,7 +133,7 @@ func newMsgEthereumTx(
 	return &MsgEthereumTx{Data: dataAny}
 }
 
-// fromEthereumTx populates the message fields from the given ethereum transaction
+// FromEthereumTx populates the message fields from the given ethereum transaction
 func (msg *MsgEthereumTx) FromEthereumTx(tx *ethtypes.Transaction) error {
 	txData, err := NewTxDataFromTx(tx)
 	if err != nil {
@@ -146,7 +146,6 @@ func (msg *MsgEthereumTx) FromEthereumTx(tx *ethtypes.Transaction) error {
 	}
 
 	msg.Data = anyTxData
-	msg.Size_ = float64(tx.Size())
 	msg.Hash = tx.Hash().Hex()
 	return nil
 }
@@ -337,6 +336,7 @@ func (msg *MsgEthereumTx) BuildTx(b client.TxBuilder, evmDenom string) (signing.
 	}
 
 	builder.SetExtensionOptions(option)
+	msg.From = ""
 	err = builder.SetMsgs(msg)
 	if err != nil {
 		return nil, err

x/evm/types/utils.go

@@ -62,7 +62,9 @@ func UnwrapEthereumMsg(tx *sdk.Tx, ethHash common.Hash) (*MsgEthereumTx, error)
 		if !ok {
 			return nil, fmt.Errorf("invalid tx type: %T", tx)
 		}
-		if ethMsg.AsTransaction().Hash() == ethHash {
+		txHash := ethMsg.AsTransaction().Hash()
+		ethMsg.Hash = txHash.Hex()
+		if txHash == ethHash {
 			return ethMsg, nil
 		}
 	}

x/feemarket/keeper/integration_test.go

@@ -567,6 +567,7 @@ func prepareEthTx(priv *ethsecp256k1.PrivKey, msgEthereumTx *evmtypes.MsgEthereu
 	err = msgEthereumTx.Sign(s.ethSigner, tests.NewSigner(priv))
 	s.Require().NoError(err)
 
+	msgEthereumTx.From = ""
 	err = txBuilder.SetMsgs(msgEthereumTx)
 	s.Require().NoError(err)