beacon panic: close of closed channel #2052
Assignees
Labels
Comments
|
Thank you for submitting the issue @gregbostrom 🙌 This is indeed a problem, same as #1333, that should be solved during the work on #491. |
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
keepnetwork/keep-client:v1.3.0-rc.4
And here is a bit bigger log snippet.
The text was updated successfully, but these errors were encountered: