!fix(evm): Fix eth tx hashes in json-rpc responses (#1176)

* 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

* move some validation to ValidateBasic

* move validation to ValidateBasic

* make ToTransaction returns a valid msg

* restructure the protoTxProvider check

* add comment

* workaround tx hash issue in event parsing

* fix integration test

* fix unit test

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>
This commit is contained in:
yihuang 2022-07-19 23:12:48 +08:00 committed by GitHub
parent 8932a6d743
commit ffe78da36e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
20 changed files with 245 additions and 101 deletions

View File

@ -43,6 +43,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.
* (evm) [\#1174](https://github.com/evmos/ethermint/pull/1174) Don't allow eth txs with 0 in mempool.
* (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

View File

@ -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())
setup := func() {
suite.enableFeemarket = false
suite.SetupTest() // reset
acc = suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr.Bytes())
suite.Require().NoError(acc.SetSequence(1))
suite.app.AccountKeeper.SetAccount(suite.ctx, acc)
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,33 @@ func (suite AnteTestSuite) TestAnteHandler() {
return txBuilder.GetTx()
}, false, 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,
},
}
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

View File

@ -61,8 +61,7 @@ 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(),
)
}
@ -403,7 +402,11 @@ func (vbd EthValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu
// For eth type cosmos tx, some fields should be veified as zero values,
// since we will only verify the signature against the hash of the MsgEthereumTx.Data
if wrapperTx, ok := tx.(protoTxProvider); ok {
wrapperTx, ok := tx.(protoTxProvider)
if !ok {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid tx type %T, didn't implement interface protoTxProvider", tx)
}
protoTx := wrapperTx.GetProtoTx()
body := protoTx.Body
if body.Memo != "" || body.TimeoutHeight != uint64(0) || len(body.NonCriticalExtensionOptions) > 0 {
@ -429,6 +432,11 @@ 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 `From` field
if msgEthTx.From != "" {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid From %s, expect empty string", msgEthTx.From)
}
txGasLimit += msgEthTx.GetGas()
txData, err := evmtypes.UnpackTxData(msgEthTx.Data)
@ -471,7 +479,6 @@ func (vbd EthValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu
if len(sigs) > 0 {
return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "for eth tx Signatures should be empty")
}
}
return next(ctx, tx, simulate)
}

View File

@ -190,6 +190,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)

View File

@ -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 |

View File

@ -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:\"-\"" ];

View File

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

View File

@ -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():

View File

@ -155,8 +155,21 @@ func (p *ParsedTxs) newTx(attrs []abci.EventAttribute) error {
}
// updateTx updates an exiting tx from events, called during parsing.
// In event format 2, we update the tx with the attributes of the second `ethereum_tx` event,
// Due to bug https://github.com/evmos/ethermint/issues/1175, the first `ethereum_tx` event may emit incorrect tx hash,
// so we prefer the second event and override the first one.
func (p *ParsedTxs) updateTx(eventIndex int, attrs []abci.EventAttribute) error {
return fillTxAttributes(&p.Txs[eventIndex], attrs)
tx := NewParsedTx(eventIndex)
if err := fillTxAttributes(&tx, attrs); err != nil {
return err
}
if tx.Hash != p.Txs[eventIndex].Hash {
// if hash is different, index the new one too
p.TxHashes[tx.Hash] = eventIndex
}
// override the tx because the second event is more trustworthy
p.Txs[eventIndex] = tx
return nil
}
// GetTxByHash find ParsedTx by tx hash, returns nil if not exists.

View File

@ -107,6 +107,8 @@ func TestParseTxResult(t *testing.T) {
}},
{Type: evmtypes.EventTypeEthereumTx, Attributes: []abci.EventAttribute{
{Key: []byte("amount"), Value: []byte("1000")},
{Key: []byte("ethereumTxHash"), Value: []byte(txHash.Hex())},
{Key: []byte("txIndex"), Value: []byte("0")},
{Key: []byte("txGasUsed"), Value: []byte("21000")},
{Key: []byte("txHash"), Value: []byte("14A84ED06282645EFBF080E0B7ED80D8D8D6A36337668A12B5F229F81CDD3F57")},
{Key: []byte("recipient"), Value: []byte("0x775b87ef5D82ca211811C1a02CE0fE0CA3a455d7")},

View File

@ -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

View File

@ -794,6 +794,8 @@ func (s *IntegrationTestSuite) TestBatchETHTransactions() {
err = msgTx.Sign(s.ethSigner, s.network.Validators[0].ClientCtx.Keyring)
s.Require().NoError(err)
// A valid msg should have empty `From`
msgTx.From = ""
msgs = append(msgs, msgTx.GetMsgs()...)
txData, err := evmtypes.UnpackTxData(msgTx.Data)
s.Require().NoError(err)

View File

@ -257,6 +257,8 @@ func prepareEthTx(priv *ethsecp256k1.PrivKey, msgEthereumTx *evmtypes.MsgEthereu
err = msgEthereumTx.Sign(s.ethSigner, tests.NewSigner(priv))
s.Require().NoError(err)
// A valid msg should have empty `From`
msgEthereumTx.From = ""
err = txBuilder.SetMsgs(msgEthereumTx)
s.Require().NoError(err)

View File

@ -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:"-"`

View File

@ -130,10 +130,12 @@ func newMsgEthereumTx(
panic(err)
}
return &MsgEthereumTx{Data: dataAny}
msg := MsgEthereumTx{Data: dataAny}
msg.Hash = msg.AsTransaction().Hash().Hex()
return &msg
}
// 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 +148,6 @@ func (msg *MsgEthereumTx) FromEthereumTx(tx *ethtypes.Transaction) error {
}
msg.Data = anyTxData
msg.Size_ = float64(tx.Size())
msg.Hash = tx.Hash().Hex()
return nil
}
@ -166,6 +167,11 @@ func (msg MsgEthereumTx) ValidateBasic() error {
}
}
// Validate Size_ field, should be kept empty
if msg.Size_ != 0 {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "tx size is deprecated")
}
txData, err := UnpackTxData(msg.Data)
if err != nil {
return sdkerrors.Wrap(err, "failed to unpack tx data")
@ -176,7 +182,17 @@ func (msg MsgEthereumTx) ValidateBasic() error {
return sdkerrors.Wrap(ErrInvalidGasLimit, "gas limit must not be zero")
}
return txData.Validate()
if err := txData.Validate(); err != nil {
return err
}
// Validate Hash field after validated txData to avoid panic
txHash := msg.AsTransaction().Hash().Hex()
if msg.Hash != txHash {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid tx hash %s, expected: %s", msg.Hash, txHash)
}
return nil
}
// GetMsgs returns a single MsgEthereumTx as an sdk.Msg.
@ -342,6 +358,10 @@ func (msg *MsgEthereumTx) BuildTx(b client.TxBuilder, evmDenom string) (signing.
}
builder.SetExtensionOptions(option)
// A valid msg should have empty `From`
msg.From = ""
err = builder.SetMsgs(msg)
if err != nil {
return nil, err

View File

@ -372,7 +372,8 @@ func (suite *MsgsTestSuite) TestMsgEthereumTx_ValidateBasic() {
},
}
for i, tc := range testCases {
for _, tc := range testCases {
suite.Run(tc.msg, func() {
to := common.HexToAddress(tc.from)
tx := types.NewTx(tc.chainID, 1, &to, tc.amount, tc.gasLimit, tc.gasPrice, tc.gasFeeCap, tc.gasTipCap, nil, tc.accessList)
@ -386,10 +387,70 @@ func (suite *MsgsTestSuite) TestMsgEthereumTx_ValidateBasic() {
err := tx.ValidateBasic()
if tc.expectPass {
suite.Require().NoError(err, "valid test %d failed: %s, %v", i, tc.msg)
suite.Require().NoError(err)
} else {
suite.Require().Error(err, "invalid test %d passed: %s, %v", i, tc.msg)
suite.Require().Error(err)
}
})
}
}
func (suite *MsgsTestSuite) TestMsgEthereumTx_ValidateBasicAdvanced() {
hundredInt := big.NewInt(100)
testCases := []struct {
msg string
msgBuilder func() *types.MsgEthereumTx
expectPass bool
}{
{
"fails - invalid tx hash",
func() *types.MsgEthereumTx {
msg := types.NewTxContract(
hundredInt,
1,
big.NewInt(10),
100000,
big.NewInt(150),
big.NewInt(200),
nil,
nil,
nil,
)
msg.Hash = "0x00"
return msg
},
false,
},
{
"fails - invalid size",
func() *types.MsgEthereumTx {
msg := types.NewTxContract(
hundredInt,
1,
big.NewInt(10),
100000,
big.NewInt(150),
big.NewInt(200),
nil,
nil,
nil,
)
msg.Size_ = 1
return msg
},
false,
},
}
for _, tc := range testCases {
suite.Run(tc.msg, func() {
err := tc.msgBuilder().ValidateBasic()
if tc.expectPass {
suite.Require().NoError(err)
} else {
suite.Require().Error(err)
}
})
}
}

2
x/evm/types/tx.pb.go generated
View File

@ -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:"-"`

View File

@ -143,10 +143,12 @@ func (args *TransactionArgs) ToTransaction() *MsgEthereumTx {
from = args.From.Hex()
}
return &MsgEthereumTx{
msg := MsgEthereumTx{
Data: any,
From: from,
}
msg.Hash = msg.AsTransaction().Hash().Hex()
return &msg
}
// ToMessage converts the arguments to the Message type used by the core evm.

View File

@ -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
}
}

View File

@ -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)