Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: introduce indexer module #285

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

kant777
Copy link
Contributor

@kant777 kant777 commented Jul 30, 2024

Indexer fetches blocks and transactions from the mev-commit chain and indexes them into Elasticsearch. It does both forwards and backward indexing so that users don't have to wait until the indexing process catches up with the latest block. Instead, it has two go routines. one that starts from latest block and moving forward and other that starts from latest-1 and backfills all the way to 0.

indexer/cmd/main.go Outdated Show resolved Hide resolved
indexer/cmd/main.go Show resolved Hide resolved
indexer/cmd/main.go Outdated Show resolved Hide resolved
indexer/cmd/main.go Outdated Show resolved Hide resolved
indexer/cmd/main.go Show resolved Hide resolved
indexer/pkg/logutil/logutil.go Outdated Show resolved Hide resolved
indexer/pkg/store/elasticsearch/elasticsearch.go Outdated Show resolved Hide resolved
indexer/pkg/store/store.go Show resolved Hide resolved
indexer/pkg/types/types.go Outdated Show resolved Hide resolved
indexer/pkg/types/types.go Outdated Show resolved Hide resolved
@mrekucci mrekucci changed the title feat: indexer indexes blocks and txs of mev-commit chain to any plugg… feat: introduce indexer module Jul 30, 2024
@kant777 kant777 force-pushed the feature/indexer-explorer branch 6 times, most recently from c6844ad to 4e2c562 Compare July 31, 2024 08:32
Copy link
Collaborator

@mrekucci mrekucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indexer module should also be added here so it is checked by ci.

indexer/cmd/indexer.go Show resolved Hide resolved
indexer/cmd/indexer.go Outdated Show resolved Hide resolved
indexer/cmd/indexer.go Outdated Show resolved Hide resolved
indexer/cmd/indexer.go Outdated Show resolved Hide resolved
indexer/cmd/indexer.go Outdated Show resolved Hide resolved
indexer/cmd/indexer.go Show resolved Hide resolved
indexer/cmd/main.go Show resolved Hide resolved
indexer/cmd/main.go Show resolved Hide resolved
indexer/cmd/main.go Outdated Show resolved Hide resolved
indexer/pkg/ethclient/w3client.go Outdated Show resolved Hide resolved
@mrekucci mrekucci requested a review from aloknerurkar July 31, 2024 16:38
@kant777 kant777 force-pushed the feature/indexer-explorer branch 6 times, most recently from e887a8b to b9a0fb9 Compare August 1, 2024 10:58
Copy link
Collaborator

@aloknerurkar aloknerurkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont think this pkg structure is needed. All these pkgs can be part of the main pkg itself in different files.

indexer/pkg/ethclient/w3client.go Outdated Show resolved Hide resolved
indexer/pkg/ethclient/w3client.go Show resolved Hide resolved
indexer/cmd/indexer.go Outdated Show resolved Hide resolved
@kant777 kant777 force-pushed the feature/indexer-explorer branch 2 times, most recently from 2af9de1 to 011d8ec Compare August 1, 2024 21:43
@kant777 kant777 force-pushed the feature/indexer-explorer branch from 011d8ec to faacca6 Compare August 1, 2024 23:26
Copy link
Collaborator

@mrekucci mrekucci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The packages store, ethclient and logutil are not necessary and I would suggest to have only one main package.

It would be nice if there were some tests, the elasticsearch client can use the httptest package from the standard library to mock the responses.

return err
}

if err = bi.initializeBackwardIndex(ctx, latestBlockNumber.Uint64()); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latestBlockNumber should be passed as big.Int, otherwise IsUint64() should be used before converting to Uint64().

return err
}

go bi.fetchForwardBlocks(ctx)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why the fetch and store blocks run asynchronously and are connected by a buffered channel if they are mapped 1:1. If one of them is slower/faster, the buffer channel will only help for a while. I would suggest doing block fetching and storing in one loop under one gorutine.


bi.logger.Info("last indexed block", "blockNumber", lastForwardIndexedBlock, "direction", "forward")

if lastForwardIndexedBlock == nil || lastForwardIndexedBlock.Sign() == 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the lastForwardIndexedBlock == nil check needed here?


bi.logger.Info("last indexed block", "blockNumber", lastBackwardIndexedBlock, "direction", "backward")

if lastBackwardIndexedBlock == nil || lastBackwardIndexedBlock.Sign() == 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the lastBackwardIndexedBlock == nil check needed here?

continue
}

for blockNum := new(big.Int).Add(bi.lastForwardIndexedBlock, big.NewInt(1)); blockNum.Cmp(latestBlockNumber) <= 0; blockNum.Add(blockNum, big.NewInt(5)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we improve readability of this for loop?

}

func (c *ESClient) GetAddresses(ctx context.Context) ([]string, error) {
query := &search.Request{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This request should be made using the typed client.

}

func (c *ESClient) CreateIndices(ctx context.Context) error {
indices := []string{"blocks", "transactions", "accounts"}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Index names are repeated everywhere, I would suggest declaring constants instead.

return fmt.Errorf("failed to check if index %s exists: %w", index, err)
}
if !res {
indexSettings := esapi.IndicesCreateRequest{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This request should be made using the typed client.


func (c *ESClient) Bulk(ctx context.Context, indexName string, docs []interface{}) error {
var (
countSuccessful uint64
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are these counters for?

type Storage interface {
IndexBlock(ctx context.Context, block *IndexBlock) error
IndexTransactions(ctx context.Context, transactions []*IndexTransaction) error
GetLastIndexedBlock(ctx context.Context, direction string) (*big.Int, error)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid Get prefix, the same is below.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants