Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

Commit

Permalink
revert PR #1176
Browse files Browse the repository at this point in the history
  • Loading branch information
fedekunze committed Aug 3, 2022
1 parent 6c39147 commit b0aebf5
Show file tree
Hide file tree
Showing 19 changed files with 98 additions and 245 deletions.
2 changes: 0 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ Ref: https://keepachangelog.com/en/1.0.0/
### State Machine Breaking

* (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.

## [v0.17.2] - 2022-07-26

Expand Down
61 changes: 17 additions & 44 deletions app/ante/ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ 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"

Expand All @@ -17,22 +16,19 @@ import (
)

func (suite AnteTestSuite) TestAnteHandler() {
var acc authtypes.AccountI
suite.enableFeemarket = false
suite.SetupTest() // reset

addr, privKey := tests.NewAddrKey()
to := tests.GenerateAddress()

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)

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.EvmKeeper.SetBalance(suite.ctx, addr, big.NewInt(10000000000))

suite.app.FeeMarketKeeper.SetBaseFee(suite.ctx, big.NewInt(100))
}
suite.app.FeeMarketKeeper.SetBaseFee(suite.ctx, big.NewInt(100))

testCases := []struct {
name string
Expand Down Expand Up @@ -67,7 +63,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
func() sdk.Tx {
signedContractTx := evmtypes.NewTxContract(
suite.app.EvmKeeper.ChainID(),
1,
2,
big.NewInt(10),
100000,
big.NewInt(150),
Expand All @@ -88,7 +84,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
func() sdk.Tx {
signedContractTx := evmtypes.NewTxContract(
suite.app.EvmKeeper.ChainID(),
1,
3,
big.NewInt(10),
100000,
big.NewInt(150),
Expand All @@ -109,7 +105,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
func() sdk.Tx {
signedTx := evmtypes.NewTx(
suite.app.EvmKeeper.ChainID(),
1,
4,
&to,
big.NewInt(10),
100000,
Expand All @@ -131,7 +127,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
func() sdk.Tx {
signedTx := evmtypes.NewTx(
suite.app.EvmKeeper.ChainID(),
1,
5,
&to,
big.NewInt(10),
100000,
Expand All @@ -153,7 +149,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
func() sdk.Tx {
signedTx := evmtypes.NewTx(
suite.app.EvmKeeper.ChainID(),
1,
6,
&to,
big.NewInt(10),
100000,
Expand All @@ -174,7 +170,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
func() sdk.Tx {
signedTx := evmtypes.NewTx(
suite.app.EvmKeeper.ChainID(),
1,
7,
&to,
big.NewInt(10),
100000,
Expand All @@ -193,7 +189,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
{
"fail - CheckTx (cosmos tx is not valid)",
func() sdk.Tx {
signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 1, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 8, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
signedTx.From = addr.Hex()

txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
Expand All @@ -205,7 +201,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
{
"fail - CheckTx (memo too long)",
func() sdk.Tx {
signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 1, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 5, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
signedTx.From = addr.Hex()

txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
Expand All @@ -216,7 +212,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
{
"fail - CheckTx (ExtensionOptionsEthereumTx not set)",
func() sdk.Tx {
signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 1, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 5, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
signedTx.From = addr.Hex()

txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false, true)
Expand Down Expand Up @@ -394,33 +390,10 @@ 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
Expand Down
115 changes: 54 additions & 61 deletions app/ante/eth.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ 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 from the ethereum transaction: %s",
"couldn't retrieve sender address ('%s') from the ethereum transaction: %s",
msgEthTx.From,
err.Error(),
)
}
Expand Down Expand Up @@ -402,82 +403,74 @@ 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
wrapperTx, ok := tx.(protoTxProvider)
if !ok {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid tx type %T, didn't implement interface protoTxProvider", tx)
}
if wrapperTx, ok := tx.(protoTxProvider); ok {
protoTx := wrapperTx.GetProtoTx()
body := protoTx.Body
if body.Memo != "" || body.TimeoutHeight != uint64(0) || len(body.NonCriticalExtensionOptions) > 0 {
return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest,
"for eth tx body Memo TimeoutHeight NonCriticalExtensionOptions should be empty")
}

protoTx := wrapperTx.GetProtoTx()
body := protoTx.Body
if body.Memo != "" || body.TimeoutHeight != uint64(0) || len(body.NonCriticalExtensionOptions) > 0 {
return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest,
"for eth tx body Memo TimeoutHeight NonCriticalExtensionOptions should be empty")
}
if len(body.ExtensionOptions) != 1 {
return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "for eth tx length of ExtensionOptions should be 1")
}

if len(body.ExtensionOptions) != 1 {
return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "for eth tx length of ExtensionOptions should be 1")
}
txFee := sdk.Coins{}
txGasLimit := uint64(0)

txFee := sdk.Coins{}
txGasLimit := uint64(0)
params := vbd.evmKeeper.GetParams(ctx)
chainID := vbd.evmKeeper.ChainID()
ethCfg := params.ChainConfig.EthereumConfig(chainID)
baseFee := vbd.evmKeeper.GetBaseFee(ctx, ethCfg)

params := vbd.evmKeeper.GetParams(ctx)
chainID := vbd.evmKeeper.ChainID()
ethCfg := params.ChainConfig.EthereumConfig(chainID)
baseFee := vbd.evmKeeper.GetBaseFee(ctx, ethCfg)
for _, msg := range protoTx.GetMsgs() {
msgEthTx, ok := msg.(*evmtypes.MsgEthereumTx)
if !ok {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evmtypes.MsgEthereumTx)(nil))
}

for _, msg := range protoTx.GetMsgs() {
msgEthTx, ok := msg.(*evmtypes.MsgEthereumTx)
if !ok {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evmtypes.MsgEthereumTx)(nil))
}
txGasLimit += msgEthTx.GetGas()

// Validate `From` field
if msgEthTx.From != "" {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid From %s, expect empty string", msgEthTx.From)
}
txData, err := evmtypes.UnpackTxData(msgEthTx.Data)
if err != nil {
return ctx, sdkerrors.Wrap(err, "failed to unpack MsgEthereumTx Data")
}

// return error if contract creation or call are disabled through governance
if !params.EnableCreate && txData.GetTo() == nil {
return ctx, sdkerrors.Wrap(evmtypes.ErrCreateDisabled, "failed to create new contract")
} else if !params.EnableCall && txData.GetTo() != nil {
return ctx, sdkerrors.Wrap(evmtypes.ErrCallDisabled, "failed to call contract")
}

txGasLimit += msgEthTx.GetGas()
if baseFee == nil && txData.TxType() == ethtypes.DynamicFeeTxType {
return ctx, sdkerrors.Wrap(ethtypes.ErrTxTypeNotSupported, "dynamic fee tx not supported")
}

txData, err := evmtypes.UnpackTxData(msgEthTx.Data)
if err != nil {
return ctx, sdkerrors.Wrap(err, "failed to unpack MsgEthereumTx Data")
txFee = txFee.Add(sdk.NewCoin(params.EvmDenom, sdk.NewIntFromBigInt(txData.Fee())))
}

// return error if contract creation or call are disabled through governance
if !params.EnableCreate && txData.GetTo() == nil {
return ctx, sdkerrors.Wrap(evmtypes.ErrCreateDisabled, "failed to create new contract")
} else if !params.EnableCall && txData.GetTo() != nil {
return ctx, sdkerrors.Wrap(evmtypes.ErrCallDisabled, "failed to call contract")
authInfo := protoTx.AuthInfo
if len(authInfo.SignerInfos) > 0 {
return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "for eth tx AuthInfo SignerInfos should be empty")
}

if baseFee == nil && txData.TxType() == ethtypes.DynamicFeeTxType {
return ctx, sdkerrors.Wrap(ethtypes.ErrTxTypeNotSupported, "dynamic fee tx not supported")
if authInfo.Fee.Payer != "" || authInfo.Fee.Granter != "" {
return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "for eth tx AuthInfo Fee payer and granter should be empty")
}

txFee = txFee.Add(sdk.NewCoin(params.EvmDenom, sdk.NewIntFromBigInt(txData.Fee())))
}

authInfo := protoTx.AuthInfo
if len(authInfo.SignerInfos) > 0 {
return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "for eth tx AuthInfo SignerInfos should be empty")
}

if authInfo.Fee.Payer != "" || authInfo.Fee.Granter != "" {
return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "for eth tx AuthInfo Fee payer and granter should be empty")
}

if !authInfo.Fee.Amount.IsEqual(txFee) {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid AuthInfo Fee Amount (%s != %s)", authInfo.Fee.Amount, txFee)
}
if !authInfo.Fee.Amount.IsEqual(txFee) {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid AuthInfo Fee Amount (%s != %s)", authInfo.Fee.Amount, txFee)
}

if authInfo.Fee.GasLimit != txGasLimit {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid AuthInfo Fee GasLimit (%d != %d)", authInfo.Fee.GasLimit, txGasLimit)
}
if authInfo.Fee.GasLimit != txGasLimit {
return ctx, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "invalid AuthInfo Fee GasLimit (%d != %d)", authInfo.Fee.GasLimit, txGasLimit)
}

sigs := protoTx.Signatures
if len(sigs) > 0 {
return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "for eth tx Signatures should be empty")
sigs := protoTx.Signatures
if len(sigs) > 0 {
return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "for eth tx Signatures should be empty")
}
}

return next(ctx, tx, simulate)
Expand Down
1 change: 0 additions & 1 deletion app/ante/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ 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)

Expand Down
2 changes: 1 addition & 1 deletion docs/api/proto-docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ MsgEthereumTx encapsulates an Ethereum transaction as an SDK message.
| `data` | [google.protobuf.Any](#google.protobuf.Any) | | inner transaction data

caches |
| `size` | [double](#double) | | DEPRECATED: encoded storage size of the transaction |
| `size` | [double](#double) | | 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 |

Expand Down
2 changes: 1 addition & 1 deletion proto/ethermint/evm/v1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ message MsgEthereumTx {
google.protobuf.Any data = 1;
// caches

// DEPRECATED: encoded storage size of the transaction
// encoded storage size of the transaction
double size = 2 [ (gogoproto.jsontag) = "-" ];
// transaction hash in hex format
string hash = 3 [ (gogoproto.moretags) = "rlp:\"-\"" ];
Expand Down
1 change: 0 additions & 1 deletion rpc/backend/evm_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -1009,7 +1009,6 @@ func (b *Backend) GetEthereumMsgsFromTendermintBlock(resBlock *tmrpctypes.Result
continue
}

ethMsg.Hash = ethMsg.AsTransaction().Hash().Hex()
result = append(result, ethMsg)
}
}
Expand Down
4 changes: 2 additions & 2 deletions rpc/namespaces/ethereum/eth/filters/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, ethTx.AsTransaction().Hash())
f.hashes = append(f.hashes, common.HexToHash(ethTx.Hash))
}
}
}
Expand Down Expand Up @@ -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, ethTx.AsTransaction().Hash())
_ = notifier.Notify(rpcSub.ID, common.HexToHash(ethTx.Hash))
}
}
case <-rpcSub.Err():
Expand Down
15 changes: 1 addition & 14 deletions rpc/types/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,21 +155,8 @@ 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 {
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
return fillTxAttributes(&p.Txs[eventIndex], attrs)
}

// GetTxByHash find ParsedTx by tx hash, returns nil if not exists.
Expand Down
2 changes: 0 additions & 2 deletions rpc/types/events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,6 @@ 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")},
Expand Down
1 change: 0 additions & 1 deletion rpc/types/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ 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
Expand Down
Loading

0 comments on commit b0aebf5

Please sign in to comment.