New interface for EventSubscriptions #491
Labels
Comments
|
During work on this card, we need to ensure our subscriptions are handled properly. We have some technical debt to pay in Ethereum integration layer - a lot of code duplication, some hairy mutex synchronization, and some complex conditions preventing us from writing to closed channels. It all needs to be sorted out during the work on this card. It covers not only designing and implementing the mechanism but also porting all of the existing code to the new solution. Some potential inspirations: https://blog.golang.org/pipelines |
~ 13 days |
4 tasks
This was referenced May 20, 2019
Closed
|
Picked this guy up. Also, possible related issue. |
|
After 12+ hours of executing relay requests locally, I observed a fatal failure of the client: Peers connected to that one lost him: |
Merged
nkuba
added a commit
to keep-network/keep-common
that referenced
this issue
Jan 28, 2021
New Ethereum event subscription API, background event pull loop # Overview There are two major changes to Ethereum subscription API proposed here: - new subscription API with `OnEvent` and `Pipe` functions, - background monitoring loop pulling past events from the chain. The first change should allow implementing some handler logic easier and to avoid complex logic leading to bugs such as keep-network/keep-core#1333 or keep-network/keep-core#2052. The second change should improve client responsiveness for operators running their nodes against Ethereum deployments that are not very reliable on the event delivery front. This code has been integrated with ECDSA keep client in `keep-ecdsa` repository and can be tested there on the branch `pipe-it` (keep-network/keep-ecdsa#671). # New API Event subscription API has been refactored to resemble the proposition from keep-network/keep-core#491. The new event subscription mechanism allows installing event callback handler function with `OnEvent` function as well as piping events from a subscription to a channel with `Pipe` function. Example usage of `OnEvent`: ``` handlerFn := func( submittingMember common.Address, conflictingPublicKey []byte, blockNumber uint64, ) { // (...) } subscription := keepContract.ConflictingPublicKeySubmitted( nil, // default SubscribeOpts nil, // no filtering on submitting member ).OnEvent(handlerFn) ``` The same subscription but with a `Pipe`: ``` sink := make(chan *abi.BondedECDSAKeepConflictingPublicKeySubmitted) subscription := keepContract.ConflictingPublicKeySubmitted( nil, // default SubscribeOpts nil, // no filtering on submitting member ).Pipe(sink) ``` Currently, all our event subscriptions use function handlers. While it is convenient in some cases, in some other cases it is the opposite. For example, `OnBondedECDSAKeepCreated` handler in ECDSA client works perfectly fine as a function. It triggers the protocol and does not have to constantly monitor the state of the chain. On the other hand, `OnDKGResultSubmitted` handler from the beacon client needs to monitor the chain and exit the process of event publication in case another node has published the result. In this case, the code could be better structured with a channel-based subscription that would allow listening for block counter events and events from DKG result submitted subscription in one for-select loop. # Background monitoring loop Some nodes in the network are running against Ethereum setups that are not particularly reliable in delivering events. Events are not delivered, nodes are not starting key-generation, or are not participating in redemption signing. Another problem is the stability of the event subscription mechanism (see #62). If the web socket connection is dropped too often, the resubscription mechanism is not enough to receive events emitted when the connection was in a weird, stale state. To address this problem, we introduce a background loop periodically pulling past events from the chain next to a regular `watchLogs` subscription. How often events are pulled and how many blocks are taken into account can be configured with `SubscribeOpts` parameters. This way, even if the event was lost by `watchLogs` subscription for whatever reason, it should be pulled by a background monitoring loop later. An extremely important implication of this change is that handlers should have a logic in place allowing them to de-duplicate received events even if a lot of time passed between receiving the original event and the duplicate. I have been experimenting with various options here, including de-duplication events in the chain implementation layer, but none of them proved to be successful as the correct de-duplication algorithm requires domain knowledge about a certain type of an event and in what circumstances identical event emitted later should or should not be identified as a duplicate. De-duplicator implementations should be added to `keep-core` and `keep-ecdsa` clients and are out of the scope of `keep-common` and this PR.
nkuba
added a commit
to keep-network/keep-ecdsa
that referenced
this issue
Jan 29, 2021
Incorporating the new Ethereum event subscription API and background event pull loop Refs #680 Depends on #663 Depends on keep-network/keep-common#63 # Overview There are two major changes to Ethereum subscriptions proposed here, as a result of the changes implemented in keep-network/keep-common#63: - new subscription API with `OnEvent` and `Pipe` functions, - background monitoring loop pulling past events from the chain. The first change should allow implementing some handler logic easier and to avoid complex logic leading to bugs such as keep-network/keep-core#1333 or keep-network/keep-core#2052. ECDSA keep client is currently not that much affected by this chain but this may change when proper event deduplication will be implemented. The second change should improve client responsiveness for operators running their nodes against Ethereum deployments that are not very reliable on the event delivery front. This should hopefully improve SLA of some mainnet operators of ECDSA client. # New API Event subscription API has been refactored in keep-network/keep-common#63 to resemble the proposition from keep-network/keep-core#491. The new event subscription mechanism allows installing event callback handler function with `OnEvent` function as well as piping events from a subscription to a channel with `Pipe` function. Example usage of `OnEvent`: ``` handlerFn := func( submittingMember common.Address, conflictingPublicKey []byte, blockNumber uint64, ) { // (...) } subscription := keepContract.ConflictingPublicKeySubmitted( nil, // default SubscribeOpts nil, // no filtering on submitting member ).OnEvent(handlerFn) ``` The same subscription but with a `Pipe`: ``` sink := make(chan *abi.BondedECDSAKeepConflictingPublicKeySubmitted) subscription := keepContract.ConflictingPublicKeySubmitted( nil, // default SubscribeOpts nil, // no filtering on submitting member ).Pipe(sink) ``` Currently, all our event subscriptions in ECDSA client use function handlers and the code has been adjusted to the new API but is still using function handlers. This may or may not change in the future depending on an individual use case. # Background monitoring loop Some nodes in the network are running against Ethereum setups that are not particularly reliable in delivering events. Events are not delivered, nodes are not starting key-generation, or are not participating in redemption signing. Another problem is the stability of the event subscription mechanism (see #663). If the web socket connection is dropped too often, the resubscription mechanism is not enough to receive events emitted when the connection was in a weird, stale state. To address this problem, keep-network/keep-common#63 introduces a background loop periodically pulling past events from the chain next to a regular `watchLogs` subscription. How often events are pulled and how many blocks are taken into account can be configured with `SubscribeOpts` parameters. This way, even if the event was lost by `watchLogs` subscription for whatever reason, it should be pulled by a background monitoring loop later. An extremely important implication of this change is that handlers should have logic in place allowing them to de-duplicate received events even if a lot of time passed between receiving the original event and the duplicate. This part has been implemented in `event_deduplicator.go` for four events: - opened keep (key generation requested), - redemption signature requested, - keep closed, - keep archived. tBTC-specific events are covered separately in #679. The only event that is not covered with deduplication is conflicting public key submitted event but it does not look to me as something that needs to be deduplicated as the subscription is canceled immediately after this event is received.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
PR #474 introduces a way for us to unsubscribe from callbacks. Typically our callbacks are attached to long running processes (ie. Ethereum Watchers: goroutines which parse log files and signal back on a channel). Unfortunately, in the implementation, there's quite a bit of code duplication (generics!) and the unsubscribe isn't deferred.
@Shadowfiend proposed the following:
This presents a more ideal way forward (less duplication, deferrable unsubscribes, etc). I attempted this with a bit of code generation, but gave up half way through (time). Let's pick this up again.
The text was updated successfully, but these errors were encountered: