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

Splice Interop #7719

Merged
merged 22 commits into from
Nov 21, 2024
Merged

Conversation

ddustin
Copy link
Collaborator

@ddustin ddustin commented Oct 4, 2024

Interop PR

This PR brings Eclair and CLN into interop status allowing splices to be executed between the two implementations.

Many things needed to be done to accomplish this:

  • Eclair rotates their funding pubkey each time.
    • The heavy lift of this PR is supporting rotating remote funding pubkeys.
      • Storing remote_funding.
      • Managing remote_funding on the splice.
      • Re-working commitment signed & related functions to support dynamic funding pubkeys.
  • Eclair had numerous little differences.
    • Needed batch_size for commitment signed messages.
    • Not transmitting the prevtx for the funding tx due to packet size limitation.
    • Removal of blockhash from messages.
    • New funding output balance must be set upfront (instead of being 0).
    • splice and splice_ack message numbers moved to 80 & 81.
  • Some general bug fixes were made that were noticed while digging around.
    • tx_abort message encoding was corrected.
    • Multiple splice failure modes would leave the user hanging without indication of what happened.
      • Added explicit error message for more cases.
      • Caught channel warning & error messages, canceled the active splice command, and showed the message to the user.

Things left to be done:

  • HTLC sigs appear to be using the old funding pubkey value and need to be updated.

@ddustin ddustin force-pushed the ddustin/splice_interop branch 2 times, most recently from 2d1a68b to 201e95b Compare October 4, 2024 23:29
@ddustin ddustin marked this pull request as ready for review November 10, 2024 20:51
@ddustin ddustin force-pushed the ddustin/splice_interop branch 4 times, most recently from 17aa694 to cccb1d2 Compare November 15, 2024 21:03
@ddustin
Copy link
Collaborator Author

ddustin commented Nov 15, 2024

Rebasing off master broke a bunch of things -- working on them now

@ddustin ddustin force-pushed the ddustin/splice_interop branch 3 times, most recently from c25f3c8 to 761bc05 Compare November 16, 2024 14:00
@rustyrussell rustyrussell added this to the v24.11 milestone Nov 18, 2024
@ddustin ddustin force-pushed the ddustin/splice_interop branch 5 times, most recently from 5485e9f to 0b56f55 Compare November 20, 2024 07:08
@ddustin
Copy link
Collaborator Author

ddustin commented Nov 20, 2024

This last failure appears to be a logic issue that was exposed by switching to eclair's behavior around tx_abort.

Working through it here: https://gist.github.com/ddustin/10f4cfd1e5d84395c9678ea90f1a7f7f

Edit: After reviewing further this logic is already handled correctly in channel_control -- the test just needs updating.

@ddustin ddustin force-pushed the ddustin/splice_interop branch 3 times, most recently from 7996a36 to 75efc61 Compare November 20, 2024 21:26
Added and updated error messages when splicing to make it more clear to the user why a splice is failing.

Changelog-Changed: Improved error messaging for splice commands.
Switch to using same message format for `tx_abort` that wire_error and wire_warning use.

Changelog-None
Enable storing the remote funding pubkey in DB if the channel peer decides to change it during splicing. It needs to be in DB incase of restarts mid-splice.

Changelog-None
It is possible for prevtx to be larger than max packet size, so for shared outputs (currently only the funding tx) we add support for sending the `txid` only across the wire and filling in the prevtx locally.

Changelog-None
Instead of assuming the remote funding pubkey does not change during splice, we store the new pubkey in the splice object.

Changelog-None
Channeld stores its own cache of `inflight` and that needs to have a copy of `remote_funding` as well.

Since copying a secp256k1 pubkey isn’t documented and `copy_inflight` isn’t used anyway — we’re dropping `copy_inflight`.

Changelog-None
Update lightningd and channeld interface to pass the remote funding pubkey back and forth to both daemons.

Changelog-None
In anticipation of adding support for rotating funding pubkeys during a splice, `channel_txs` is updated to support specifying these manually instead of using the channel’s funding pubkeys.

Changelog-None
Since funding keys can be rotated during splice, commit sig routines must be able to handle a dynamic value for the funding keys.

Changelog-None
This is no longer used.

Changelog-None
Update to use spec signature type.

Changelog-None
The prior spec left this value at 0 to be calculted later but the current spec requires we fill it in in advance.

Changelog-None
As per eclair spec proposal.
1) A renaming to `funding_txid`
2) Adding of `batch_size` to indicate how many commitment_signed msgs are expected.

Changelog-None
Update the sending and receiving of commit sigs to use dyniamic funding pubkeys incase our remote peer rotates theirs during a splice.

Changelog-None
We need to differentiate the funding pubkey since we allow the peer to rotate it now.

Changelog-None
This function needs to be used earlier in the file so it is moved vertically up.

Chanelog-None
By placing the funding tx into `interactivetx`, the message will be compressed by only sending the txid via tlvs.

Changelog-None
`tx_abort` should not send reestablish message and instead go into ‘reconnect only mode’

Changelog-None
Set the remote funding pubkey on both lightningd and channeld when mutual splice lock is achieved.

This will be needed once rotating funding keys is enabled during splicing

Changelog-None.
Allows our peer to change their funding pub key during a splice.

Changelog-Changed: Support added for peers that wish to rotate their funding pubkey during a splice.
Changelog-Changed: Splicing moved from test numbers to spec numbers.
As per eclair implementation we skip `channel_reestablish` and go straight into the channel for `tx_abort` events.

Changelog-None
@ddustin ddustin force-pushed the ddustin/splice_interop branch from 75efc61 to f1ed25e Compare November 20, 2024 21:26
@rustyrussell rustyrussell merged commit d04e646 into ElementsProject:master Nov 21, 2024
39 checks passed
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.

2 participants