Skip to content

Unlock utxos during dual funding failures#2390

Merged
t-bast merged 3 commits into
masterfrom
dual-funding-unlock-utxos
Aug 19, 2022
Merged

Unlock utxos during dual funding failures#2390
t-bast merged 3 commits into
masterfrom
dual-funding-unlock-utxos

Conversation

@t-bast
Copy link
Copy Markdown
Member

@t-bast t-bast commented Aug 19, 2022

When an alternative funding transaction confirms, we need to unlock other candidates: we may not have published them yet if for example we didn't receive remote signatures.

This is a breaking change in codecs since #2274, but shouldn't impact anyone if we merge this before people start using dual funding.

When an alternative funding transaction confirms, we need to unlock other
candidates: we may not have published them yet if for example we didn't
receive remote signatures.
@t-bast t-bast requested a review from pm47 August 19, 2022 13:28
}
final case class DATA_CLOSING(commitments: Commitments,
fundingTx: Option[Transaction], // this will be non-empty if we are the initiator and we got in closing while waiting for our own tx to be published
fundingTx: Option[UnconfirmedFundingTx],
Copy link
Copy Markdown
Member Author

@t-bast t-bast Aug 19, 2022

Choose a reason for hiding this comment

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

The new trait is a bit verbose, but it highlights the fact that this is only used when the funding tx is unconfirmed (which wasn't obvious before). Alternatively, we could just use an Option[Either[Transaction, SignedSharedTransaction]], let me know what you prefer.

What is still a bit confusing is that in the single-funded case, this can be None for two very different reasons:

  1. The funding tx is confirmed
  2. The funding tx is unconfirmed but we're fundee so we don't have it

The dual-funded case doesn't have this ambiguity. I don't know if it's worth fixing for the single-funded case, it would mean wrapping another Option in the SingleFundedUnconfirmedFundingTx case class.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 19, 2022

Codecov Report

Merging #2390 (764ec6d) into master (a97e88f) will decrease coverage by 0.05%.
The diff coverage is 60.00%.

@@            Coverage Diff             @@
##           master    #2390      +/-   ##
==========================================
- Coverage   84.73%   84.68%   -0.06%     
==========================================
  Files         199      199              
  Lines       15458    15472      +14     
  Branches      642      640       -2     
==========================================
+ Hits        13098    13102       +4     
- Misses       2360     2370      +10     
Impacted Files Coverage Δ
...inq/eclair/channel/fsm/ChannelOpenDualFunded.scala 91.55% <0.00%> (-0.41%) ⬇️
...in/scala/fr/acinq/eclair/channel/fsm/Channel.scala 84.51% <14.28%> (-0.41%) ⬇️
...in/scala/fr/acinq/eclair/channel/ChannelData.scala 98.11% <50.00%> (-1.89%) ⬇️
...acinq/eclair/channel/fsm/DualFundingHandlers.scala 60.46% <50.00%> (-1.70%) ⬇️
...ire/internal/channel/version3/ChannelCodecs3.scala 98.54% <75.00%> (+0.02%) ⬆️
...fr/acinq/eclair/channel/InteractiveTxBuilder.scala 86.21% <100.00%> (-0.51%) ⬇️
...la/fr/acinq/eclair/channel/fsm/ErrorHandlers.scala 81.13% <100.00%> (ø)
...ire/internal/channel/version0/ChannelCodecs0.scala 95.65% <100.00%> (ø)
...ire/internal/channel/version1/ChannelCodecs1.scala 96.96% <100.00%> (ø)
...ire/internal/channel/version2/ChannelCodecs2.scala 98.46% <100.00%> (ø)
... and 4 more

Comment thread eclair-core/src/main/scala/fr/acinq/eclair/channel/fsm/Channel.scala Outdated
@t-bast t-bast merged commit ad19a66 into master Aug 19, 2022
@t-bast t-bast deleted the dual-funding-unlock-utxos branch August 19, 2022 16:56
@pm47 pm47 mentioned this pull request Aug 22, 2022
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.

3 participants