Skip to content
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

Allow splicing on non dual-funded channels #2727

Merged
merged 2 commits into from Sep 26, 2023
Merged

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Aug 22, 2023

We currently allow splicing on top of a channel that was created without using dual funding, but it results in an incorrect channel reserve being used post-splice.

Another approach would be to move the channel reserve field to each commitment instead of having the initial value in localParams and remoteParams. This way we would always explicitly store the value for local/remote reserves, which may make custom channel reserves (0-reserve) more explicitly dealt with as well. But the issue is that we still need to keep track of the requested values from open_channel / accept_channel and propagate them until a Commitment is created, which requires a lot of codec changes without bringing much value. I think we should go with this simple change for now and we can always introduce per-commitment reserves later if it becomes really necessary.

@t-bast t-bast requested a review from pm47 August 22, 2023 15:34
@codecov-commenter
Copy link

codecov-commenter commented Sep 11, 2023

Codecov Report

Merging #2727 (a061f2f) into master (96ebbfe) will increase coverage by 0.05%.
Report is 3 commits behind head on master.
The diff coverage is 92.30%.

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

@@            Coverage Diff             @@
##           master    #2727      +/-   ##
==========================================
+ Coverage   85.74%   85.79%   +0.05%     
==========================================
  Files         216      216              
  Lines       18017    18053      +36     
  Branches      759      794      +35     
==========================================
+ Hits        15449    15489      +40     
+ Misses       2568     2564       -4     
Files Coverage Δ
...r/acinq/eclair/blockchain/fee/OnChainFeeConf.scala 95.23% <100.00%> (+18.76%) ⬆️
...in/scala/fr/acinq/eclair/channel/ChannelData.scala 100.00% <100.00%> (ø)
...in/scala/fr/acinq/eclair/channel/Commitments.scala 96.81% <100.00%> (+0.01%) ⬆️
...inq/eclair/channel/fsm/ChannelOpenDualFunded.scala 87.44% <100.00%> (ø)
...q/eclair/channel/fsm/ChannelOpenSingleFunded.scala 93.46% <100.00%> (ø)
...inq/eclair/channel/fsm/CommonFundingHandlers.scala 94.87% <100.00%> (ø)
...la/fr/acinq/eclair/channel/fsm/ErrorHandlers.scala 83.44% <100.00%> (+0.11%) ⬆️
...inq/eclair/channel/fund/InteractiveTxBuilder.scala 91.37% <100.00%> (ø)
...ir/channel/publish/ReplaceableTxPrePublisher.scala 66.66% <100.00%> (+1.24%) ⬆️
.../fr/acinq/eclair/channel/publish/TxPublisher.scala 98.78% <100.00%> (+1.28%) ⬆️
... and 5 more

... and 3 files with indirect coverage changes

We currently allow splicing on top of a channel that was created without
using dual funding, but it results in an incorrect channel reserve being
used post-splice.
This field only applies to the first funding index, so we rename it to
make that explicit. Once a splice has happened, it will be ignored.
@t-bast t-bast merged commit d274fc1 into master Sep 26, 2023
1 check passed
@t-bast t-bast deleted the splice-non-dual-fund-channel branch September 26, 2023 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants