From 922a60971e7edaa3f340c028bbcfc22bca020d18 Mon Sep 17 00:00:00 2001 From: HuangYi Date: Tue, 12 Jul 2022 11:51:28 +0800 Subject: [PATCH] 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 | 2 + app/ante/ante_test.go | 103 +++++++++++++++++---- app/ante/eth.go | 19 +++- app/ante/utils_test.go | 1 + docs/api/proto-docs.md | 2 +- proto/ethermint/evm/v1/tx.proto | 2 +- rpc/backend/evm_backend.go | 1 + rpc/namespaces/ethereum/eth/filters/api.go | 4 +- rpc/types/utils.go | 1 + tests/e2e/integration_test.go | 1 + x/evm/spec/04_transactions.md | 2 +- x/evm/types/msg.go | 4 +- x/evm/types/tx.pb.go | 2 +- x/evm/types/utils.go | 4 +- x/feemarket/keeper/integration_test.go | 1 + 15 files changed, 119 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72ce2c60f5..a172c681c3 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/app/ante/ante_test.go b/app/ante/ante_test.go index 1b09095870..997a38f38d 100644 --- a/app/ante/ante_test.go +++ b/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 diff --git a/app/ante/eth.go b/app/ante/eth.go index 66ac465c5d..f2b62a896a 100644 --- a/app/ante/eth.go +++ b/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) diff --git a/app/ante/utils_test.go b/app/ante/utils_test.go index 5c030eb82d..332b5e1236 100644 --- a/app/ante/utils_test.go +++ b/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) diff --git a/docs/api/proto-docs.md b/docs/api/proto-docs.md index 15471f6bf6..6401c08690 100644 --- a/docs/api/proto-docs.md +++ b/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 | diff --git a/proto/ethermint/evm/v1/tx.proto b/proto/ethermint/evm/v1/tx.proto index 9dc53f2fa0..0898f8e0ac 100644 --- a/proto/ethermint/evm/v1/tx.proto +++ b/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:\"-\"" ]; diff --git a/rpc/backend/evm_backend.go b/rpc/backend/evm_backend.go index 21494cb33a..e8b87801a0 100644 --- a/rpc/backend/evm_backend.go +++ b/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) } } diff --git a/rpc/namespaces/ethereum/eth/filters/api.go b/rpc/namespaces/ethereum/eth/filters/api.go index 10409fd837..9bc5a4700e 100644 --- a/rpc/namespaces/ethereum/eth/filters/api.go +++ b/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(): diff --git a/rpc/types/utils.go b/rpc/types/utils.go index 15da24489a..f8a0c7e11c 100644 --- a/rpc/types/utils.go +++ b/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 diff --git a/tests/e2e/integration_test.go b/tests/e2e/integration_test.go index 6ae14a5f0e..5351eb3b9d 100644 --- a/tests/e2e/integration_test.go +++ b/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) diff --git a/x/evm/spec/04_transactions.md b/x/evm/spec/04_transactions.md index d3609dda23..4a132cf36b 100644 --- a/x/evm/spec/04_transactions.md +++ b/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:"-"` diff --git a/x/evm/types/msg.go b/x/evm/types/msg.go index 8bd40f5f4c..1b5e9fe1f7 100644 --- a/x/evm/types/msg.go +++ b/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 diff --git a/x/evm/types/tx.pb.go b/x/evm/types/tx.pb.go index a2e5fffad2..883fa0013b 100644 --- a/x/evm/types/tx.pb.go +++ b/x/evm/types/tx.pb.go @@ -37,7 +37,7 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package 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:"-"` diff --git a/x/evm/types/utils.go b/x/evm/types/utils.go index c30d2e6550..5f5e02b0ed 100644 --- a/x/evm/types/utils.go +++ b/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 } } diff --git a/x/feemarket/keeper/integration_test.go b/x/feemarket/keeper/integration_test.go index 726c484d97..37f679e4fe 100644 --- a/x/feemarket/keeper/integration_test.go +++ b/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)