Skip to content

Dual funding RBF support#2275

Merged
t-bast merged 6 commits into
masterfrom
dual-funding-rbf
Aug 22, 2022
Merged

Dual funding RBF support#2275
t-bast merged 6 commits into
masterfrom
dual-funding-rbf

Conversation

@t-bast
Copy link
Copy Markdown
Member

@t-bast t-bast commented May 17, 2022

Add support for bumping the fees of a dual funding transaction. We spawn a transient dedicated actor: if the RBF attempt fails, or if we are disconnected before completing the protocol, we should forget it.

With that PR, dual-funding is fully supported by eclair! There is no way for the non-initiator to contribute though, as we'd need a mechanism to let node operators preset some "strategy". The first use-case for contributing as the non-initiator will be liquidity ads, which we will support soon.

Note that I haven't been able to do cross-compatibility tests with cln yet, as their version of dual funding doesn't match the latest state of the spec PR. @niftynei is working on this and I'll do cross-compat tests as soon as they're ready, but I don't think this prevents us from merging this PR to master since dual funding is disabled by default and we've documented that we still may break backwards-compatibility.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 17, 2022

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 75.31646% with 39 lines in your changes missing coverage. Please review.

Project coverage is 84.85%. Comparing base (ad19a66) to head (135870b).
Report is 476 commits behind head on master.

Files with missing lines Patch % Lines
...inq/eclair/channel/fsm/ChannelOpenDualFunded.scala 76.06% 28 Missing ⚠️
...in/scala/fr/acinq/eclair/channel/fsm/Channel.scala 60.00% 6 Missing ⚠️
...r-core/src/main/scala/fr/acinq/eclair/Eclair.scala 0.00% 2 Missing ⚠️
...q/eclair/channel/fsm/ChannelOpenSingleFunded.scala 80.00% 1 Missing ⚠️
...acinq/eclair/channel/fsm/DualFundingHandlers.scala 88.88% 1 Missing ⚠️
...la/fr/acinq/eclair/channel/fsm/ErrorHandlers.scala 66.66% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2275      +/-   ##
==========================================
+ Coverage   84.67%   84.85%   +0.18%     
==========================================
  Files         199      199              
  Lines       15472    15584     +112     
  Branches      640      662      +22     
==========================================
+ Hits        13101    13224     +123     
+ Misses       2371     2360      -11     
Files with missing lines Coverage Δ
...in/scala/fr/acinq/eclair/channel/ChannelData.scala 98.11% <ø> (ø)
...in/scala/fr/acinq/eclair/channel/Commitments.scala 94.84% <100.00%> (+0.01%) ⬆️
...fr/acinq/eclair/channel/InteractiveTxBuilder.scala 85.67% <100.00%> (+0.04%) ⬆️
...inq/eclair/channel/fsm/CommonFundingHandlers.scala 91.66% <100.00%> (ø)
...main/scala/fr/acinq/eclair/router/Validation.scala 95.42% <100.00%> (+1.30%) ⬆️
...ire/internal/channel/version3/ChannelCodecs3.scala 98.54% <ø> (ø)
...q/eclair/wire/protocol/LightningMessageTypes.scala 98.21% <100.00%> (+3.57%) ⬆️
...q/eclair/channel/fsm/ChannelOpenSingleFunded.scala 94.19% <80.00%> (ø)
...acinq/eclair/channel/fsm/DualFundingHandlers.scala 80.43% <88.88%> (+19.96%) ⬆️
...la/fr/acinq/eclair/channel/fsm/ErrorHandlers.scala 81.13% <66.66%> (ø)
... and 3 more

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@t-bast t-bast force-pushed the dual-funding-rbf branch from 6937ed4 to 0f6d9ad Compare May 18, 2022 07:29
@t-bast t-bast marked this pull request as ready for review May 18, 2022 07:53
@t-bast t-bast requested a review from pm47 May 18, 2022 07:54
@t-bast t-bast force-pushed the dual-funding-rbf branch 2 times, most recently from 9b63f8e to 9ea7934 Compare July 1, 2022 15:40
t-bast added 2 commits August 22, 2022 08:56
Add support for bumping the fees of a dual funding transaction.
We spawn a transient dedicated actor: if the RBF attempt fails, or if we
are disconnected before completing the protocol, we should forget it.
Add more tests for scenarios where an unconfirmed channel is force-closed,
where the funding transaction that confirms may not be the last one.
Comment thread eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/DualFundingHandlers.scala Outdated
Comment thread eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelData.scala Outdated
Copy link
Copy Markdown
Member

@pm47 pm47 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That one was easy 😎

@t-bast t-bast merged commit a735ba8 into master Aug 22, 2022
@t-bast t-bast deleted the dual-funding-rbf branch August 22, 2022 12:37
t-bast added a commit that referenced this pull request Dec 1, 2022
The release introduces a few API changes:

- channelbalances retrieves information about the balances of all local
  channels (#2196)
- channelbalances and usablebalances return a shortIds object instead
  of a single shortChannelId (#2323)
- stop stops eclair: please note that the recommended way of stopping
  eclair is simply to kill its process (#2233)
- rbfopen lets the initiator of a dual-funded channel RBF the funding
  transaction (#2275)
- listinvoices and listpendinginvoices now accept --count and --skip
  parameters to limit the number of retrieved items (#2474)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants