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: ETH getLogs: fix slowness at head and ignore null blocks #12207

Merged
merged 2 commits into from
Jul 11, 2024

Conversation

aarshkshah1992
Copy link
Contributor

Temporary patch for the v1.28 release to fix some of the issues mentioned in #12205.

  • Event Index is not aware of null blocks because it is never invoked for null blocks.
  • If user does not explicitly specify "latest", eth_getlogs should inly block will "latest"-1 as the events for the "latest" block will only be available after the block is executed (deferred execution).

The right thing to do here is revisit how all the ETH RPC APIs deal with the "latest" flag and ensure they are "deferred execution-safe". We also need to make the Event Index aware of null blocks(or atleast have a way to ensure that a given height resulted in a null block and so it has already "processed" the events for that height). Let's prioritise this as part of our ETH RPC work.

@aarshkshah1992 aarshkshah1992 added the skip/changelog This change does not require CHANGELOG.md update label Jul 10, 2024
@aarshkshah1992 aarshkshah1992 self-assigned this Jul 10, 2024
node/impl/full/eth.go Outdated Show resolved Hide resolved
node/impl/full/eth.go Outdated Show resolved Hide resolved
Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

left a couple of small suggestions, I'm mainly interested in leaving good comments in here so we don't forget what needs to be done

@aarshkshah1992
Copy link
Contributor Author

@rvagg Addressed your review here.

@aarshkshah1992
Copy link
Contributor Author

Test is a flaky. Added to #12001.

@aarshkshah1992 aarshkshah1992 merged commit 9741571 into master Jul 11, 2024
78 of 79 checks passed
@aarshkshah1992 aarshkshah1992 deleted the fix/get-logs-head branch July 11, 2024 11:16
aarshkshah1992 added a commit that referenced this pull request Jul 11, 2024
* fix get logs slowness and handling of null blocks
aarshkshah1992 added a commit that referenced this pull request Jul 11, 2024
* fix get logs slowness and handling of null blocks
aarshkshah1992 added a commit that referenced this pull request Jul 17, 2024
* fix get logs slowness and handling of null blocks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip/changelog This change does not require CHANGELOG.md update
Projects
Status: ☑️Done (Archive)
Development

Successfully merging this pull request may close these issues.

None yet

2 participants