-
Notifications
You must be signed in to change notification settings - Fork 29
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
Implement tx confirmation for channelOpen #1009
Conversation
65d86be
to
825f31b
Compare
Codecov Report
@@ Coverage Diff @@
## master #1009 +/- ##
==========================================
- Coverage 96.04% 96.00% -0.04%
==========================================
Files 95 95
Lines 3588 3633 +45
Branches 759 775 +16
==========================================
+ Hits 3446 3488 +42
- Misses 85 88 +3
Partials 57 57
Continue to review full report at Codecov.
|
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.
looks good, thank you 👍
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.
Tests must have been pain. Good job 👍
Co-Authored-By: Thomas Pischulski <[email protected]>
952025b
to
a0ef008
Compare
Part of #613
Todo in a later PR, is to extend this method to all on-chain transactions.
First time the on-chain transaction is seen, it gets dispatched with
payload.confirmed
property present but set asundefined
, meaning pending. Reducer will persist/serialize it tostate.pendingTx
(thanks to #759).Then, on each block, the possibly confirmed actions (when more than
config.confirmationBlocks
have passed since first/pending action) gets fetched withgetTransactionReceipt
, and if the transaction is still present on-chain, it gets re-emitted withconfirmed=true
, which also removes it from the pendintTx state.If it isn't present, on each block until
2 * confirmationBlocks
, check again. If it still isn't there/confirmed after this timeout, consider the transaction as having been removed by a chain reorg, and re-emit it, now withconfirmed=false
, which also removes it frompendingTx
and rejects any pending promise..
The confirmable actions always being emitted twice (first as pending, later as confirmed or removed) enables each part of the codebase to chose which actions/events to react to, be it first time seeing a tx event, or upon confirmation: e.g. the transfer epics can react early to
channelClose
and avoid accepting new transfers or mediating (in the future), while accepting deposits as increasing the channel's capacity is done only after it has been confirmed.The
confirmationEpic
reacting on state ensures this algorithm is safe upon shutdown and later client restart, even with to-be-confirmed actions being able to be picked up again after much time (as long as receipt can be fetched by eth client, which may not be the case without full or even archiving node with enormous block ranges, but we'd consider this as an unexpected edge case).