-
Notifications
You must be signed in to change notification settings - Fork 32
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
Add synchronized flag to subscriptions #235
Conversation
Signed-off-by: Nicko Guyer <nicko.guyer@kaleido.io>
Codecov Report
@@ Coverage Diff @@
## main #235 +/- ##
=======================================
Coverage 97.26% 97.26%
=======================================
Files 59 59
Lines 7521 7524 +3
=======================================
+ Hits 7315 7318 +3
- Misses 163 164 +1
+ Partials 43 42 -1 |
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.
Really small (hopefully) request here @nguyer so that we don't return true
until we've processed the logs between the end of the catchup window, and the head of the chain.
internal/events/subscription.go
Outdated
@@ -214,6 +215,7 @@ func (s *subscription) createFilter(ctx context.Context, since *big.Int) error { | |||
return errors.Errorf(errors.RPCCallReturnedError, "eth_newFilter", err) | |||
} | |||
s.catchupBlock = nil // we are not in catchup mode now | |||
s.info.Synchronized = true |
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.
I think we should wait to flick this to true until we've done our first eth_getFilterLogs
and switch to eth_getFilterChanges
:
firefly-ethconnect/internal/events/subscription.go
Lines 363 to 365 in e4bd748
if s.filteredOnce { | |
rpcMethod = "eth_getFilterChanges" | |
} |
Signed-off-by: Nicko Guyer <nicko.guyer@kaleido.io>
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.
👍 - thanks for making that tweak. Looks great
This PR adds a new flag to a subscription to indicate whether ETHConnect is still catching up or has caught up to the configured block height gap, and is keeping up with the chain head. When you do a
GET
on a subscription you will now see asynchronized
field like this: