-
Notifications
You must be signed in to change notification settings - Fork 177
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
Reduce number of ETH RPC calls during block polling #2775
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, thanks for the detailed explanation of why we've done each piece. If this works then it's also a big win in terms of reducing our code complexity!
f4da0f0
to
109a6f3
Compare
Codecov Report
@@ Coverage Diff @@
## master #2775 +/- ##
===================================================
+ Coverage 56.28521% 56.60971% +0.32450%
===================================================
Files 88 88
Lines 19172 19078 -94
===================================================
+ Hits 10791 10800 +9
+ Misses 7789 7684 -105
- Partials 592 594 +2
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
What does this pull request do? Explain your changes. (required)
Reduce the number of ETH RPC calls during the block polling operations.
From my measurements, this change (together with changing
blockPollingInterval
from10
to20
) should bring the RPC req reduction by the factor of 7-8x.The main improvement is that I removed the
pollNextBlock()
andbuildCanonicalChain()
parts, which I believe are not important after the migration to Arbitrum. The rationale is that Ethereum had a different approach to the block finality than Arbitrum:123
,234
,345
, but after some time they could be reorganized to123
,234
,456
.123
,234
,345
, they will always stay this wayI believe this whole code part with
pollNextBlock()
was so complex because the block reorg needed to be supported.Specific updates (required)
pollNextBlock()
andbuidCanonicalChain()
maxBlocksInGetLogsQuery
from60
to1000
(this limit was too low which resulted in multipleeth_getLogs
calls instead of one callHow did you test each of these updates (required)
Tested with local runs (running for a long time).
Further improvements
The code could be refactored even further, but I don't want to do it in this PR, since I'd like to limit the number of changes done here to the minimum that brings good results. Additional possible improvements:
stack
and its operations (stack.go)fix #2298
Checklist:
make
runs successfully./test.sh
passREADME and other documentation updated