-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
support new heads polling over http rpc client #14373
base: develop
Are you sure you want to change the base?
support new heads polling over http rpc client #14373
Conversation
…n-of-head-subscriptions
…-flag-for-NewHeadsPollInterval add new flag, NewHeadsPollInterval
…op handling logic. To add unit test
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.
Nice work. Feel free to ignore nit comments, unless there is need for additional changes
Quality Gate passedIssues Measures |
if r.newHeadsPollInterval > 0 { | ||
interval := r.newHeadsPollInterval | ||
timeout := interval | ||
poller, _ := commonclient.NewPoller[*evmtypes.Head](interval, r.latestBlock, timeout, r.rpcLog) |
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.
Why copy r.newHeadsPollInterval
twice? Not a big deal either way I guess, just wondering why not just:
poller, _ := commonclient.NewPoller[*evmtypes.Head](interval, r.latestBlock, interval, r.rpcLog)
or even:
poller, _ := commonclient.NewPoller[*evmtypes.Head](r.newHeadsPollInterval, r.latestBlock, r.newHeadsPollInterval, r.rpcLog)
?
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.
It makes the line 499 more readable, so it's not looking like passing the same parameter twice. And it's more configurable if we decided to update the timeout value later. And it's consistent with the SubscribeToFinalizedHeads
function
Description
EVM RPCClient uses web sockets to provide users with nearly real-time access to new blocks. While WS provides better performance, its reliability is sometimes not sufficient. We should be able to use HTTP instead.
In this PR:
NewHeadsPollInterval
(>0 indicate the polling new head is enabled), it's done with a separate PR for better readability hopefully.In another PR we make the ws url optional if Logbroadcaster is disabled.
Tickets:
BCFR-706