feat!: Store eth tx index separately #1121

yihuang · 2022-06-09T10:01:31Z

Closes: #1075

Description

run a optional indexer service
adapt the json-rpc to the more efficient query
a cli command to index block range manually.

For contributor use:

Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards.
Wrote unit and integration tests
Updated relevant documentation (docs/) or specification (x/<module>/spec/)
Added relevant godoc comments.
Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
Re-reviewed Files changed in the Github PR explorer

For admin use:

Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
Reviewers assigned
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

codecov · 2022-06-09T10:06:51Z

Codecov Report

Merging #1121 (d927def) into main (8f932dd) will decrease coverage by 0.14%.
The diff coverage is 48.07%.

❗ Current head d927def differs from pull request most recent head ecb633e. Consider uploading reports for the commit ecb633e to get more accurate results

@@            Coverage Diff             @@
##             main    #1121      +/-   ##
==========================================
- Coverage   61.74%   61.59%   -0.15%     
==========================================
  Files          89       88       -1     
  Lines        7303     7296       -7     
==========================================
- Hits         4509     4494      -15     
- Misses       2570     2581      +11     
+ Partials      224      221       -3

Impacted Files	Coverage Δ
rpc/types/events.go	`73.01% <48.00%> (-17.59%)`	⬇️
server/config/config.go	`21.68% <50.00%> (+0.34%)`	⬆️
x/evm/types/params.go	`100.00% <0.00%> (ø)`
x/evm/types/tx_data.go	`97.91% <0.00%> (ø)`
x/evm/migrations/v2/migrate.go
x/evm/module.go	`53.73% <0.00%> (+0.87%)`	⬆️
app/ante/eth.go	`82.67% <0.00%> (+2.05%)`	⬆️
x/evm/keeper/migrations.go	`100.00% <0.00%> (+33.33%)`	⬆️

rpc/backend/evm_backend.go

server/config/toml.go

server/flags/flags.go

tomtau

could there be a test for the indexer?

rpc/backend/evm_backend.go

rpc/namespaces/ethereum/debug/api.go

rpc/namespaces/ethereum/eth/api.go

rpc/types/events.go

codecov · 2022-06-29T06:18:41Z

Codecov Report

Merging #1121 (a6fc34a) into main (737c1de) will increase coverage by 0.02%.
The diff coverage is 40.20%.

@@            Coverage Diff             @@
##             main    #1121      +/-   ##
==========================================
+ Coverage   52.27%   52.29%   +0.02%     
==========================================
  Files         104      105       +1     
  Lines        9383     9519     +136     
==========================================
+ Hits         4905     4978      +73     
- Misses       4227     4274      +47     
- Partials      251      267      +16

Impacted Files	Coverage Δ
rpc/backend/tracing.go	`0.00% <0.00%> (ø)`
rpc/backend/tx_info.go	`0.00% <0.00%> (ø)`
rpc/backend/utils.go	`1.29% <ø> (-2.50%)`	⬇️
rpc/types/utils.go	`0.00% <0.00%> (ø)`
rpc/types/events.go	`71.85% <50.00%> (-16.25%)`	⬇️
server/config/config.go	`22.48% <50.00%> (+0.32%)`	⬆️
indexer/kv_indexer.go	`64.96% <64.96%> (ø)`
rpc/backend/backend.go	`51.72% <100.00%> (+1.72%)`	⬆️
rpc/backend/blocks_info.go	`44.41% <100.00%> (ø)`
... and 1 more

lgtm-com · 2022-06-29T07:14:53Z

This pull request introduces 1 alert when merging 0f5cc5b2c4b37a1838985a4678ba675202318c4b into de21773 - view on LGTM.com

new alerts:

1 for Inconsistent direction of for loop

yihuang · 2022-06-30T02:30:16Z

The db size after a test run on a cronos mainnet node (block height: 3104880):

$ du -sh tx_index.db/ evmindexer.db/
290G	tx_index.db/
3.2G	evmindexer.db/

yihuang · 2022-07-01T07:49:39Z

could there be a test for the indexer?

added some unit tests for the kv indexer.

rpc/types/events.go

server/indexer.go

facs95

Gave it another pass after conflicts were solved! LGTM 🚀

fedekunze

Final pass. @danburck can you also review?

proto/ethermint/types/v1/indexer.proto

rpc/backend/backend.go

rpc/backend/backend_suite_test.go

rpc/backend/tx_info.go

server/indexer_cmd.go

server/start.go

server/indexer.go

- test indexer in backend unit test - add comments

fedekunze

ACK. I think this PR could be refactored further tho. Let's work on a document for how to enable this

server/indexer_service.go

server/indexer_cmd.go

danburck

LGTM, aggree that we need documentation for this. @yihuang can you add some after the merge?

yihuang · 2022-08-12T01:40:50Z

LGTM, aggree that we need documentation for this. @yihuang can you add some after the merge?

sure #1235

* Store eth tx index separately Closes: #1075 Solution: - run a optional indexer service - adapt the json-rpc to the more efficient query changelog changelog fix lint fix backward compatibility fix lint timeout better strconv fix linter fix package name add cli command to index old tx fix for loop indexer cmd don't have access to local rpc workaround exceed block gas limit situation add unit tests for indexer refactor polish the indexer module Update server/config/toml.go Co-authored-by: Federico Kunze Küllmer <[email protected]> improve comments share code between GetTxByEthHash and GetTxByIndex fix unit test Update server/indexer.go Co-authored-by: Freddy Caceres <[email protected]> * Apply suggestions from code review * test enable-indexer in integration test * fix go lint * address review suggestions * fix linter * address review suggestions - test indexer in backend unit test - add comments * fix build * fix test * service name Co-authored-by: Freddy Caceres <[email protected]> Co-authored-by: Federico Kunze Küllmer <[email protected]>

* Store eth tx index separately Closes: evmos#1075 Solution: - run a optional indexer service - adapt the json-rpc to the more efficient query changelog changelog fix lint fix backward compatibility fix lint timeout better strconv fix linter fix package name add cli command to index old tx fix for loop indexer cmd don't have access to local rpc workaround exceed block gas limit situation add unit tests for indexer refactor polish the indexer module Update server/config/toml.go Co-authored-by: Federico Kunze Küllmer <[email protected]> improve comments share code between GetTxByEthHash and GetTxByIndex fix unit test Update server/indexer.go Co-authored-by: Freddy Caceres <[email protected]> * Apply suggestions from code review * test enable-indexer in integration test * fix go lint * address review suggestions * fix linter * address review suggestions - test indexer in backend unit test - add comments * fix build * fix test * service name Co-authored-by: Freddy Caceres <[email protected]> Co-authored-by: Federico Kunze Küllmer <[email protected]>

onewhiskeypls

utACK: quick pass with a nit and thought

onewhiskeypls · 2022-08-29T19:29:28Z

indexer/kv_indexer.go

+			ethMsg := msg.(*evmtypes.MsgEthereumTx)
+			txHash := common.HexToHash(ethMsg.Hash)
+
+			txResult := ethermint.TxResult{


thought: For the following if-else block, could we set Failed here as true or would that be poor practice?

It can save a branch block if we init both GasUsed and Failed to the failed cases here.

onewhiskeypls · 2022-08-30T00:16:46Z

indexer/kv_indexer.go

+	"github.com/cosmos/cosmos-sdk/client"
+	"github.com/cosmos/cosmos-sdk/codec"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"


nitpick: This package was deprecated and moved to its own module here "cosmossdk.io/errors"

We can migrate that globally sometime.

gopherine · 2022-09-23T10:05:16Z

server/indexer_cmd.go

+				if err := idxer.IndexBlock(blk, resBlk.DeliverTxs); err != nil {
+					return err
+				}
+				fmt.Println(height)


Consider: A progress bar instead or remove it, this feels like a very vague way to keep track of the progress?

gopherine · 2022-09-23T10:10:14Z

rpc/types/events.go

@@ -97,40 +83,38 @@ func ParseTxResult(result *abci.ResponseDeliverTx) (*ParsedTxs, error) {
 		TxHashes: make(map[common.Hash]int),
 	}
 	for _, event := range result.Events {


Consider: extracting the format check, then seperate each formats into its own functional block, this will also make it easier to purge deprecated format eventually

* Store eth tx index separately Closes: evmos#1075 Solution: - run a optional indexer service - adapt the json-rpc to the more efficient query changelog changelog fix lint fix backward compatibility fix lint timeout better strconv fix linter fix package name add cli command to index old tx fix for loop indexer cmd don't have access to local rpc workaround exceed block gas limit situation add unit tests for indexer refactor polish the indexer module Update server/config/toml.go Co-authored-by: Federico Kunze Küllmer <[email protected]> improve comments share code between GetTxByEthHash and GetTxByIndex fix unit test Update server/indexer.go Co-authored-by: Freddy Caceres <[email protected]> * Apply suggestions from code review * test enable-indexer in integration test * fix go lint * address review suggestions * fix linter * address review suggestions - test indexer in backend unit test - add comments * fix build * fix test * service name Co-authored-by: Freddy Caceres <[email protected]> Co-authored-by: Federico Kunze Küllmer <[email protected]> (cherry picked from commit a8723c5)

yihuang requested review from fedekunze, khoslaventures and jolube as code owners June 9, 2022 10:01

yihuang marked this pull request as draft June 9, 2022 10:07

yihuang force-pushed the indexer branch 3 times, most recently from 4a0217d to 81ff827 Compare June 10, 2022 09:17

yihuang changed the title ~~Store eth tx index separately~~ feat!: Store eth tx index separately Jun 10, 2022

yihuang mentioned this pull request Jun 13, 2022

Problem: customer tx indexer feature is not enabled crypto-org-chain/cronos#549

Merged

13 tasks

yihuang marked this pull request as ready for review June 13, 2022 08:26

yihuang commented Jun 13, 2022

View reviewed changes

rpc/backend/evm_backend.go Outdated Show resolved Hide resolved

yihuang commented Jun 13, 2022

View reviewed changes

server/config/toml.go Outdated Show resolved Hide resolved

yihuang commented Jun 13, 2022

View reviewed changes

server/flags/flags.go Outdated Show resolved Hide resolved

yihuang force-pushed the indexer branch 3 times, most recently from 1bf47b8 to ecb633e Compare June 13, 2022 09:46

tomtau reviewed Jun 27, 2022

View reviewed changes

yihuang force-pushed the indexer branch from ecb633e to deebb61 Compare June 29, 2022 04:01

yihuang force-pushed the indexer branch from 096d786 to 55348e0 Compare June 29, 2022 08:27

yihuang marked this pull request as draft June 29, 2022 14:23

yihuang force-pushed the indexer branch from fb2e765 to faf9b2e Compare June 30, 2022 01:44

yihuang mentioned this pull request Jun 30, 2022

Problem: db size increase too fast crypto-org-chain/cronos#451

Closed

yihuang marked this pull request as ready for review July 1, 2022 07:49

yihuang force-pushed the indexer branch from 7d5774d to 8a5e97e Compare July 4, 2022 06:32

tomtau approved these changes Jul 4, 2022

View reviewed changes

rpc/types/events.go Show resolved Hide resolved

server/indexer.go Outdated Show resolved Hide resolved

yihuang added 3 commits August 10, 2022 19:33

Merge remote-tracking branch 'fork/indexer' into indexer

feeb7b8

fix go lint

2a1c17e

Merge branch 'main' into indexer

c2fbd4e

facs95 approved these changes Aug 10, 2022

View reviewed changes

address review suggestions

8ade689

yihuang requested a review from fedekunze August 10, 2022 13:17

facs95 and others added 3 commits August 10, 2022 09:21

fix linter

b0b4847

Merge remote-tracking branch 'origin/main' into indexer

97f62a2

Merge branch 'main' into indexer

02e9849

fedekunze reviewed Aug 11, 2022

View reviewed changes

address review suggestions

724bc76

- test indexer in backend unit test - add comments

yihuang force-pushed the indexer branch from d3146b9 to 724bc76 Compare August 11, 2022 11:12

yihuang added 2 commits August 11, 2022 19:16

fix build

436188d

fix test

c1e299e

yihuang requested a review from fedekunze August 11, 2022 12:19

fedekunze approved these changes Aug 11, 2022

View reviewed changes

server/indexer_service.go Outdated Show resolved Hide resolved

server/indexer_cmd.go Show resolved Hide resolved

service name

a6fc34a

danburck approved these changes Aug 11, 2022

View reviewed changes

danburck enabled auto-merge (squash) August 11, 2022 20:45

danburck merged commit 77ed4aa into evmos:main Aug 11, 2022

yihuang deleted the indexer branch August 12, 2022 00:01

onewhiskeypls reviewed Aug 30, 2022

View reviewed changes

gopherine reviewed Sep 23, 2022

View reviewed changes

danburck mentioned this pull request Nov 30, 2022

1512 set RANDOM opcode #1519

Closed

lumtis mentioned this pull request Aug 21, 2023

Emitting Ethereum related events in PostTxProcessing EVM hooks make Ethereum events inconsistent zeta-chain/node#1010

Open

fedekunze mentioned this pull request Sep 14, 2024

rpc: remove unused tx indexer evmos/evmos#2846

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: Store eth tx index separately #1121

feat!: Store eth tx index separately #1121

yihuang commented Jun 9, 2022 •

edited

Loading

codecov bot commented Jun 9, 2022 •

edited

Loading

tomtau left a comment

codecov bot commented Jun 29, 2022 •

edited

Loading

lgtm-com bot commented Jun 29, 2022

yihuang commented Jun 30, 2022 •

edited

Loading

yihuang commented Jul 1, 2022

facs95 left a comment •

edited

Loading

fedekunze left a comment

fedekunze left a comment

danburck left a comment •

edited

Loading

yihuang commented Aug 12, 2022

onewhiskeypls left a comment

onewhiskeypls Aug 29, 2022

yihuang Aug 30, 2022

onewhiskeypls Aug 30, 2022

yihuang Aug 30, 2022

gopherine Sep 23, 2022

gopherine Sep 23, 2022

feat!: Store eth tx index separately #1121

feat!: Store eth tx index separately #1121

Conversation

yihuang commented Jun 9, 2022 • edited Loading

Description

codecov bot commented Jun 9, 2022 • edited Loading

Codecov Report

tomtau left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 29, 2022 • edited Loading

Codecov Report

lgtm-com bot commented Jun 29, 2022

yihuang commented Jun 30, 2022 • edited Loading

yihuang commented Jul 1, 2022

facs95 left a comment • edited Loading

Choose a reason for hiding this comment

fedekunze left a comment

Choose a reason for hiding this comment

fedekunze left a comment

Choose a reason for hiding this comment

danburck left a comment • edited Loading

Choose a reason for hiding this comment

yihuang commented Aug 12, 2022

onewhiskeypls left a comment

Choose a reason for hiding this comment

onewhiskeypls Aug 29, 2022

Choose a reason for hiding this comment

yihuang Aug 30, 2022

Choose a reason for hiding this comment

onewhiskeypls Aug 30, 2022

Choose a reason for hiding this comment

yihuang Aug 30, 2022

Choose a reason for hiding this comment

gopherine Sep 23, 2022

Choose a reason for hiding this comment

gopherine Sep 23, 2022

Choose a reason for hiding this comment

yihuang commented Jun 9, 2022 •

edited

Loading

codecov bot commented Jun 9, 2022 •

edited

Loading

codecov bot commented Jun 29, 2022 •

edited

Loading

yihuang commented Jun 30, 2022 •

edited

Loading

facs95 left a comment •

edited

Loading

danburck left a comment •

edited

Loading