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

Fix for issue 4205 (optimize eth_getLogs) #4662

Merged
merged 3 commits into from
Jul 9, 2022
Merged

Conversation

leonardchinonso
Copy link
Contributor

I used rawdb.ReadCanonicalBodyWithTransactions to get the list of Transactions associated with the block hash and number. Then used the txLogIndex for the particular transaction. Apart from the overhead of using the api compared to rawdb, I do not see any other optimization. Did I do it wrong?

@AskAlexSharov
Copy link
Collaborator

@leonardchinonso rawdb read from db only, but when --snapshots=true historical blocks are in snapshots only. So, need use api._blockReader (it's BlockReaderWithSnapshots class) - it will detect by block number: read from snapshots or from db.

@AskAlexSharov
Copy link
Collaborator

you can use another method of api._blockReader or add new method there.

@leonardchinonso
Copy link
Contributor Author

I checked out the _blockReader field and it already had the BodyWithTransactions service that rawdb had. Like you said, it searches the snapshots files too. However, is there any more optimization we can get?
I don't know if we need the whole body of transactions in that block since we will only take the ones that match our criteria

@AskAlexSharov
Copy link
Collaborator

By _blockReader can iterate over transactions (like in db).
Is it possible to know which transactions need to read without reading them?

@leonardchinonso
Copy link
Contributor Author

leonardchinonso commented Jul 7, 2022

Fair enough. In that case, the changes should work for this. Any tests I need to add or rerun?


body, err := api._blockReader.BodyWithTransactions(ctx, tx, blockHash, blockNumber)
if err != nil || body == nil {
return nil, fmt.Errorf("block not found %d", blockNumber)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Or return error message or log it

@AskAlexSharov AskAlexSharov merged commit cfc0518 into devel Jul 9, 2022
@AskAlexSharov AskAlexSharov deleted the fix/issue-4205 branch July 9, 2022 13:42
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.

None yet

2 participants