eclair icon indicating copy to clipboard operation
eclair copied to clipboard

Dual funding channel confirmation

Open t-bast opened this issue 3 years ago • 1 comments

Finalize the dual-funding flow: exchange signatures and wait for the funding transaction to confirm.

If an error occurs, we need to wait for the funding tx to be double-spent before we can consider the channel closed and safely forget it.

There is some logic duplication between single-funded and dual-funded handlers...I don't know how far we should go to try to remove duplication. If we extract too much, it may make it harder to just read an event handler's code and verify that every step is there and correctly done in the right order. Also, there are small subtleties where single-funded and dual-funded differ, and there will be more as we build new protocols on top of dual funding (e.g. liquidity ads). In the long term, we will eventually remove the code for the single-funded case, as it can be achieved as just a special case of dual-funding (but we'll need all peers on the network to support dual funding before we can really remove that). Let me know what you think and where you think we should refactor.

t-bast avatar May 17 '22 10:05 t-bast

Codecov Report

Merging #2274 (f49116a) into master (33e6fac) will decrease coverage by 0.21%. The diff coverage is 72.34%.

@@            Coverage Diff             @@
##           master    #2274      +/-   ##
==========================================
- Coverage   84.95%   84.74%   -0.22%     
==========================================
  Files         198      199       +1     
  Lines       15255    15455     +200     
  Branches      633      639       +6     
==========================================
+ Hits        12960    13097     +137     
- Misses       2295     2358      +63     
Impacted Files Coverage Δ
...n/scala/fr/acinq/eclair/balance/CheckBalance.scala 64.89% <0.00%> (-1.42%) :arrow_down:
...inq/eclair/channel/fsm/SingleFundingHandlers.scala 80.00% <ø> (-4.10%) :arrow_down:
...main/scala/fr/acinq/eclair/db/DbEventHandler.scala 93.42% <ø> (ø)
...n/scala/fr/acinq/eclair/json/JsonSerializers.scala 95.12% <ø> (ø)
...in/scala/fr/acinq/eclair/channel/fsm/Channel.scala 84.80% <41.26%> (-2.67%) :arrow_down:
...q/eclair/channel/fsm/ChannelOpenSingleFunded.scala 93.80% <66.66%> (+0.12%) :arrow_up:
...acinq/eclair/channel/fsm/DualFundingHandlers.scala 62.16% <66.66%> (+5.01%) :arrow_up:
...inq/eclair/channel/fsm/ChannelOpenDualFunded.scala 91.15% <75.38%> (-5.91%) :arrow_down:
...c/main/scala/fr/acinq/eclair/channel/Helpers.scala 95.03% <88.88%> (+0.03%) :arrow_up:
...la/fr/acinq/eclair/channel/fsm/ErrorHandlers.scala 81.13% <88.88%> (+0.73%) :arrow_up:
... and 27 more

codecov-commenter avatar Jul 01 '22 15:07 codecov-commenter

I changed the watching behavior in https://github.com/ACINQ/eclair/pull/2274/commits/bd479921eae2270b731cd1d03457bdae20acb58c to correctly detect channel force-close while we're offline. The commit looks simple, but it's actually very subtle, please review carefully! The main subtlety is that we when we detect an alternative commitment being used, we don't claim it, we instead wait for the corresponding funding tx to be confirmed before acting. If we claimed it instantly and swapped the main commitments with this alternative one, we would be screwed if it's actually the main funding tx that ends up confirming...

I cannot add yet test cases for those alternative commitments since RBF isn't implemented, but it's on my TODO-list to add to the RBF PR.

t-bast avatar Aug 18 '22 09:08 t-bast