From b8cad024ce8d693618d7e2af18f645b0588147f1 Mon Sep 17 00:00:00 2001 From: HuangYi Date: Tue, 22 Mar 2022 17:50:42 +0800 Subject: [PATCH 1/8] Problem: eth_newPendingTransactionFilter don't return correct tx hash Closes: #1011 Solution: - use eth tx hash rather than tendermint one --- rpc/apis.go | 2 +- rpc/ethereum/namespaces/eth/filters/api.go | 47 ++++++++++++++++------ 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/rpc/apis.go b/rpc/apis.go index 7519e5c834..29c431a9fe 100644 --- a/rpc/apis.go +++ b/rpc/apis.go @@ -57,7 +57,7 @@ func GetRPCAPIs(ctx *server.Context, clientCtx client.Context, tmWSClient *rpccl rpc.API{ Namespace: EthNamespace, Version: apiVersion, - Service: filters.NewPublicAPI(ctx.Logger, tmWSClient, evmBackend), + Service: filters.NewPublicAPI(ctx.Logger, clientCtx, tmWSClient, evmBackend), Public: true, }, ) diff --git a/rpc/ethereum/namespaces/eth/filters/api.go b/rpc/ethereum/namespaces/eth/filters/api.go index 5d6a788a1f..5e1cc473e4 100644 --- a/rpc/ethereum/namespaces/eth/filters/api.go +++ b/rpc/ethereum/namespaces/eth/filters/api.go @@ -6,6 +6,7 @@ import ( "sync" "time" + "github.com/cosmos/cosmos-sdk/client" "github.com/tharsis/ethermint/rpc/ethereum/types" "github.com/tendermint/tendermint/libs/log" @@ -56,6 +57,7 @@ type filter struct { // information related to the Ethereum protocol such as blocks, transactions and logs. type PublicFilterAPI struct { logger log.Logger + clientCtx client.Context backend Backend events *EventSystem filtersMu sync.Mutex @@ -63,13 +65,14 @@ type PublicFilterAPI struct { } // NewPublicAPI returns a new PublicFilterAPI instance. -func NewPublicAPI(logger log.Logger, tmWSClient *rpcclient.WSClient, backend Backend) *PublicFilterAPI { +func NewPublicAPI(logger log.Logger, clientCtx client.Context, tmWSClient *rpcclient.WSClient, backend Backend) *PublicFilterAPI { logger = logger.With("api", "filter") api := &PublicFilterAPI{ - logger: logger, - backend: backend, - filters: make(map[rpc.ID]*filter), - events: NewEventSystem(logger, tmWSClient), + logger: logger, + clientCtx: clientCtx, + backend: backend, + filters: make(map[rpc.ID]*filter), + events: NewEventSystem(logger, tmWSClient), } go api.timeoutLoop() @@ -141,11 +144,20 @@ func (api *PublicFilterAPI) NewPendingTransactionFilter() rpc.ID { continue } - txHash := common.BytesToHash(tmtypes.Tx(data.Tx).Hash()) + tx, err := api.clientCtx.TxConfig.TxDecoder()(data.Tx) + if err != nil { + api.logger.Debug("fail to decode tx", "error", err.Error()) + continue + } api.filtersMu.Lock() if f, found := api.filters[pendingTxSub.ID()]; found { - f.hashes = append(f.hashes, txHash) + for _, msg := range tx.GetMsgs() { + ethTx, ok := msg.(*evmtypes.MsgEthereumTx) + if ok { + f.hashes = append(f.hashes, common.HexToHash(ethTx.Hash)) + } + } } api.filtersMu.Unlock() case <-errCh: @@ -198,13 +210,22 @@ func (api *PublicFilterAPI) NewPendingTransactions(ctx context.Context) (*rpc.Su continue } - txHash := common.BytesToHash(tmtypes.Tx(data.Tx).Hash()) - - // To keep the original behavior, send a single tx hash in one notification. - // TODO(rjl493456442) Send a batch of tx hashes in one notification - err = notifier.Notify(rpcSub.ID, txHash) + tx, err := api.clientCtx.TxConfig.TxDecoder()(data.Tx) if err != nil { - return + api.logger.Debug("fail to decode tx", "error", err.Error()) + continue + } + + for _, msg := range tx.GetMsgs() { + ethTx, ok := msg.(*evmtypes.MsgEthereumTx) + if ok { + // To keep the original behavior, send a single tx hash in one notification. + // TODO(rjl493456442) Send a batch of tx hashes in one notification + err = notifier.Notify(rpcSub.ID, common.HexToHash(ethTx.Hash)) + if err != nil { + return + } + } } case <-rpcSub.Err(): pendingTxSub.Unsubscribe(api.events) From c776df69e501018a476564af4447f48c9c4d1dde Mon Sep 17 00:00:00 2001 From: HuangYi Date: Tue, 22 Mar 2022 18:36:34 +0800 Subject: [PATCH 2/8] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44e328d971..45829ccf31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (ante) [tharsis#991](https://github.com/tharsis/ethermint/pull/991) Set an upper bound to gasWanted to prevent DoS attack. * (rpc) [tharsis#1006](https://github.com/tharsis/ethermint/pull/1006) Use `string` as the parameters type to correct ambiguous results. * (ante) [tharsis#1004](https://github.com/tharsis/ethermint/pull/1004) make MaxTxGasWanted configurable. +* (rpc) [tharsis#1012](https://github.com/tharsis/ethermint/pull/1012) fix the tx hash in filter entries created by `eth_newPendingTransactionFilter`. ## [v0.11.0] - 2022-03-06 From caac781225db28e630339ed6356f1a49f4ccfddf Mon Sep 17 00:00:00 2001 From: HuangYi Date: Wed, 23 Mar 2022 09:22:08 +0800 Subject: [PATCH 3/8] remove copied TODO comment and ignore err result of Notify --- rpc/ethereum/namespaces/eth/filters/api.go | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/rpc/ethereum/namespaces/eth/filters/api.go b/rpc/ethereum/namespaces/eth/filters/api.go index 5e1cc473e4..54b44944a6 100644 --- a/rpc/ethereum/namespaces/eth/filters/api.go +++ b/rpc/ethereum/namespaces/eth/filters/api.go @@ -219,12 +219,7 @@ func (api *PublicFilterAPI) NewPendingTransactions(ctx context.Context) (*rpc.Su for _, msg := range tx.GetMsgs() { ethTx, ok := msg.(*evmtypes.MsgEthereumTx) if ok { - // To keep the original behavior, send a single tx hash in one notification. - // TODO(rjl493456442) Send a batch of tx hashes in one notification - err = notifier.Notify(rpcSub.ID, common.HexToHash(ethTx.Hash)) - if err != nil { - return - } + notifier.Notify(rpcSub.ID, common.HexToHash(ethTx.Hash)) // nolint: errcheck } } case <-rpcSub.Err(): @@ -335,11 +330,7 @@ func (api *PublicFilterAPI) NewHeads(ctx context.Context) (*rpc.Subscription, er // TODO: fetch bloom from events header := types.EthHeaderFromTendermint(data.Header, ethtypes.Bloom{}, baseFee) - err = notifier.Notify(rpcSub.ID, header) - if err != nil { - headersSub.err <- err - return - } + notifier.Notify(rpcSub.ID, header) // nolint: errcheck case <-rpcSub.Err(): headersSub.Unsubscribe(api.events) return @@ -402,10 +393,7 @@ func (api *PublicFilterAPI) Logs(ctx context.Context, crit filters.FilterCriteri logs := FilterLogs(evmtypes.LogsToEthereum(txResponse.Logs), crit.FromBlock, crit.ToBlock, crit.Addresses, crit.Topics) for _, log := range logs { - err = notifier.Notify(rpcSub.ID, log) - if err != nil { - return - } + notifier.Notify(rpcSub.ID, log) // nolint: errcheck } case <-rpcSub.Err(): // client send an unsubscribe request logsSub.Unsubscribe(api.events) From 66f731f034fe66c98c5648ab204cdfc39db604da Mon Sep 17 00:00:00 2001 From: HuangYi Date: Wed, 23 Mar 2022 10:03:51 +0800 Subject: [PATCH 4/8] add e2e test --- tests/e2e/integration_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/e2e/integration_test.go b/tests/e2e/integration_test.go index ace2ad248c..87d94d03c3 100644 --- a/tests/e2e/integration_test.go +++ b/tests/e2e/integration_test.go @@ -721,3 +721,27 @@ func (s *IntegrationTestSuite) TestWeb3Sha3() { }) } } + +func (s *IntegrationTestSuite) TestPendingTransactionFilter() { + var ( + filterID string + filterResult []common.Hash + ) + // create filter + err := s.rpcClient.Call(&filterID, "eth_newPendingTransactionFilter") + s.Require().NoError(err) + // check filter result is empty + err = s.rpcClient.Call(&filterResult, "eth_getFilterChanges", filterID) + s.Require().NoError(err) + s.Require().Empty(filterResult) + // send transaction + signedTx := s.signValidTx(common.HexToAddress("0x378c50D9264C63F3F92B806d4ee56E9D86FfB3Ec"), big.NewInt(10)).AsTransaction() + err = s.network.Validators[0].JSONRPCClient.SendTransaction(s.ctx, signedTx) + s.Require().NoError(err) + s.expectSuccessReceipt(signedTx.Hash()) + + // check filter changes match the tx hash + err = s.rpcClient.Call(&filterResult, "eth_getFilterChanges", filterID) + s.Require().NoError(err) + s.Require().Equal([]common.Hash{signedTx.Hash()}, filterResult) +} From 8a6e0b1a197282bcc24dffecca99365b80dc3e1b Mon Sep 17 00:00:00 2001 From: HuangYi Date: Thu, 24 Mar 2022 10:38:19 +0800 Subject: [PATCH 5/8] fix ws client in e2e test --- testutil/network/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testutil/network/util.go b/testutil/network/util.go index 1c4c8f50f5..cf41f2bebe 100644 --- a/testutil/network/util.go +++ b/testutil/network/util.go @@ -128,7 +128,7 @@ func startInProcess(cfg Config, val *Validator) error { } tmEndpoint := "/websocket" - tmRPCAddr := fmt.Sprintf("tcp://%s", val.AppConfig.GRPC.Address) + tmRPCAddr := val.RPCAddress val.jsonrpc, val.jsonrpcDone, err = server.StartJSONRPC(val.Ctx, val.ClientCtx, tmRPCAddr, tmEndpoint, *val.AppConfig) if err != nil { From 4c6f9012994153cb1b039e1193916a095eec8195 Mon Sep 17 00:00:00 2001 From: HuangYi Date: Thu, 24 Mar 2022 10:56:59 +0800 Subject: [PATCH 6/8] fix test --- tests/e2e/integration_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/e2e/integration_test.go b/tests/e2e/integration_test.go index 87d94d03c3..0052ee2ca0 100644 --- a/tests/e2e/integration_test.go +++ b/tests/e2e/integration_test.go @@ -738,6 +738,8 @@ func (s *IntegrationTestSuite) TestPendingTransactionFilter() { signedTx := s.signValidTx(common.HexToAddress("0x378c50D9264C63F3F92B806d4ee56E9D86FfB3Ec"), big.NewInt(10)).AsTransaction() err = s.network.Validators[0].JSONRPCClient.SendTransaction(s.ctx, signedTx) s.Require().NoError(err) + + s.waitForTransaction() s.expectSuccessReceipt(signedTx.Hash()) // check filter changes match the tx hash From 6eddbf44dcd37efffb098362f96888d53258751a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Federico=20Kunze=20K=C3=BCllmer?= <31522760+fedekunze@users.noreply.github.com> Date: Thu, 24 Mar 2022 08:43:30 +0100 Subject: [PATCH 7/8] Apply suggestions from code review --- rpc/ethereum/namespaces/eth/filters/api.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rpc/ethereum/namespaces/eth/filters/api.go b/rpc/ethereum/namespaces/eth/filters/api.go index 54b44944a6..419130c31a 100644 --- a/rpc/ethereum/namespaces/eth/filters/api.go +++ b/rpc/ethereum/namespaces/eth/filters/api.go @@ -219,7 +219,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)) // nolint: errcheck + _ = notifier.Notify(rpcSub.ID, common.HexToHash(ethTx.Hash)) // nolint: errcheck } } case <-rpcSub.Err(): @@ -330,7 +330,7 @@ func (api *PublicFilterAPI) NewHeads(ctx context.Context) (*rpc.Subscription, er // TODO: fetch bloom from events header := types.EthHeaderFromTendermint(data.Header, ethtypes.Bloom{}, baseFee) - notifier.Notify(rpcSub.ID, header) // nolint: errcheck + _ = notifier.Notify(rpcSub.ID, header) // nolint: errcheck case <-rpcSub.Err(): headersSub.Unsubscribe(api.events) return @@ -393,7 +393,7 @@ func (api *PublicFilterAPI) Logs(ctx context.Context, crit filters.FilterCriteri logs := FilterLogs(evmtypes.LogsToEthereum(txResponse.Logs), crit.FromBlock, crit.ToBlock, crit.Addresses, crit.Topics) for _, log := range logs { - notifier.Notify(rpcSub.ID, log) // nolint: errcheck + _ = notifier.Notify(rpcSub.ID, log) // nolint: errcheck } case <-rpcSub.Err(): // client send an unsubscribe request logsSub.Unsubscribe(api.events) From bba7330cefafd556fee47873e53412895895ce8c Mon Sep 17 00:00:00 2001 From: yihuang Date: Thu, 24 Mar 2022 15:50:31 +0800 Subject: [PATCH 8/8] Apply suggestions from code review --- rpc/ethereum/namespaces/eth/filters/api.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rpc/ethereum/namespaces/eth/filters/api.go b/rpc/ethereum/namespaces/eth/filters/api.go index 419130c31a..4c286312d0 100644 --- a/rpc/ethereum/namespaces/eth/filters/api.go +++ b/rpc/ethereum/namespaces/eth/filters/api.go @@ -219,7 +219,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)) // nolint: errcheck + _ = notifier.Notify(rpcSub.ID, common.HexToHash(ethTx.Hash)) } } case <-rpcSub.Err(): @@ -330,7 +330,7 @@ func (api *PublicFilterAPI) NewHeads(ctx context.Context) (*rpc.Subscription, er // TODO: fetch bloom from events header := types.EthHeaderFromTendermint(data.Header, ethtypes.Bloom{}, baseFee) - _ = notifier.Notify(rpcSub.ID, header) // nolint: errcheck + _ = notifier.Notify(rpcSub.ID, header) case <-rpcSub.Err(): headersSub.Unsubscribe(api.events) return @@ -393,7 +393,7 @@ func (api *PublicFilterAPI) Logs(ctx context.Context, crit filters.FilterCriteri logs := FilterLogs(evmtypes.LogsToEthereum(txResponse.Logs), crit.FromBlock, crit.ToBlock, crit.Addresses, crit.Topics) for _, log := range logs { - _ = notifier.Notify(rpcSub.ID, log) // nolint: errcheck + _ = notifier.Notify(rpcSub.ID, log) } case <-rpcSub.Err(): // client send an unsubscribe request logsSub.Unsubscribe(api.events)