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

Implement option_simple_close #2747

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Implement option_simple_close #2747

wants to merge 6 commits into from

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Sep 20, 2023

Implement option_simple_close as defined in lightning/bolts#1096

Hopefully this is the last time we change the mutual close protocol! And at some point that will let us entirely remove all the code supporting the two previous mutual close protocols (this is why I kept the code as separate as possible instead of trying to fit into the existing NEGOTIATING state).

Note that this is a prerequisite for taproot channels: this protocol allows nodes to safely exchange nonces in shutdown and closing_complete to spend a musig2 channel output.

@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2023

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

Codecov Report

Attention: Patch coverage is 87.41497% with 37 lines in your changes missing coverage. Please review.

Project coverage is 86.18%. Comparing base (b8e6800) to head (3708ab9).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
...in/scala/fr/acinq/eclair/channel/fsm/Channel.scala 83.84% 21 Missing ⚠️
...c/main/scala/fr/acinq/eclair/channel/Helpers.scala 88.88% 6 Missing ⚠️
...la/fr/acinq/eclair/channel/fsm/ErrorHandlers.scala 40.00% 6 Missing ⚠️
...la/fr/acinq/eclair/transactions/Transactions.scala 94.87% 2 Missing ⚠️
...n/scala/fr/acinq/eclair/balance/CheckBalance.scala 0.00% 1 Missing ⚠️
...la/fr/acinq/eclair/io/OpenChannelInterceptor.scala 0.00% 1 Missing ⚠️

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2747      +/-   ##
==========================================
- Coverage   86.30%   86.18%   -0.13%     
==========================================
  Files         224      224              
  Lines       19867    20326     +459     
  Branches      795      805      +10     
==========================================
+ Hits        17147    17517     +370     
- Misses       2720     2809      +89     
Files with missing lines Coverage Δ
...core/src/main/scala/fr/acinq/eclair/Features.scala 100.00% <100.00%> (ø)
...in/scala/fr/acinq/eclair/channel/ChannelData.scala 100.00% <100.00%> (ø)
...in/scala/fr/acinq/eclair/channel/Commitments.scala 96.84% <100.00%> (+0.01%) ⬆️
...a/fr/acinq/eclair/channel/fsm/CommonHandlers.scala 89.36% <100.00%> (+4.06%) ⬆️
...inq/eclair/channel/fund/InteractiveTxBuilder.scala 92.82% <100.00%> (+0.21%) ⬆️
...n/scala/fr/acinq/eclair/io/PeerReadyNotifier.scala 71.75% <100.00%> (+0.21%) ⬆️
...n/scala/fr/acinq/eclair/json/JsonSerializers.scala 96.42% <ø> (ø)
...ire/internal/channel/version4/ChannelCodecs4.scala 98.53% <100.00%> (+0.06%) ⬆️
...ala/fr/acinq/eclair/wire/protocol/ChannelTlv.scala 100.00% <100.00%> (ø)
.../eclair/wire/protocol/LightningMessageCodecs.scala 99.47% <100.00%> (+0.01%) ⬆️
... and 7 more

... and 9 files with indirect coverage changes

@t-bast t-bast force-pushed the option-simple-close branch 2 times, most recently from 77023c5 to db7a79d Compare September 22, 2023 15:58
@t-bast t-bast force-pushed the option-simple-close branch 2 times, most recently from a223c78 to e05d16e Compare October 4, 2023 09:45
@t-bast t-bast force-pushed the option-simple-close branch from e05d16e to 57ae172 Compare January 16, 2024 14:24
@t-bast t-bast force-pushed the option-simple-close branch from 3b6879b to 4420858 Compare March 27, 2024 14:55
@t-bast t-bast force-pushed the option-simple-close branch from 4420858 to 7fda599 Compare September 18, 2024 16:58
@t-bast t-bast force-pushed the option-simple-close branch 3 times, most recently from f8b957d to f0571b2 Compare October 11, 2024 07:08
@t-bast t-bast force-pushed the option-simple-close branch from f0571b2 to 623b371 Compare October 18, 2024 06:45
@t-bast t-bast force-pushed the option-simple-close branch from 36ff0c3 to 3708ab9 Compare October 22, 2024 01:37
This feature adds two new messages:

- `closing_complete` sent after exchanging `shutdown`
- `closing_sig` sent in response to `closing_complete`
The spec allows the closer to use an OP_RETURN output if their amount is
too low when using `option_simple_close`.
We introduce a new `NEGOTIATING_SIMPLE` state where we exchange the
`closing_complete` and `closing_sig` messages, and allow RBF-ing previous
transactions and updating our closing script.

We stay in that state until one of the transactions confirms, or a force
close is detected. This is important to ensure we're able to correctly
reconnect and negotiate RBF candidates.

We keep this separate from the previous NEGOTIATING state to make it
easier to remove support for the older mutual close protocols once we're
confident the network has been upgraded.
@t-bast t-bast force-pushed the option-simple-close branch from 3708ab9 to ea979aa Compare December 6, 2024 17:05
Whenever one side sends `shutdown`, we restart a signing round from
scratch. To be compatible with future taproot channels, we require
the receiver to also send `shutdown` before moving on to exchanging
`closing_complete` and `closing_sig`. This will give nodes a message
to exchange fresh musig2 nonces before producing signatures.

On reconnection, we also restart a signing session from scratch and
discard pending partial signatures.
Whenever we exchange `shutdown`, we now require that new closing txs
are signed before allowing another `shutdown` message to be sent to
start a new signing round.

This creates more risk of deadlock when one side fails to send their
sigs, where we'll need to disconnect to start a new signing round.
But that shouldn't happen if nodes are honest and not buggy, so it
probably doesn't matter. If nodes are buggy or malicious, we will
need to force-close anyway.
@t-bast t-bast force-pushed the option-simple-close branch from ea979aa to c45cf33 Compare December 9, 2024 10:57
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.

2 participants