-
Notifications
You must be signed in to change notification settings - Fork 493
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
Channel Splicing (feature 62/63) #1160
base: master
Are you sure you want to change the base?
Conversation
Can I suggest we do this as an extension BOLT rather than layering it in with the existing BOLT2 text? It makes it easier to implement when all of the requirements deltas are in a single document than when it is inlined into the original spec. Otherwise, the PR/branch-diff itself is the only way to see the diff and that can get very messy during the review process as people's commentary comes in. While there are other ways to get at this diff without the commentary, it would make the UX of getting at this diff rather straightforward. Given that the change is gated behind a feature bit anyway it also makes it easier for a new implementation to bootstrap itself without the splice feature by just reading the main BOLTs as is. At some point in the future when splicing support becomes standard across the network we can consolidate the extension BOLT into the main BOLTs if people still prefer. |
Why not, if others also feel that it would be better as an extension bolt. I prefer it directly in Bolt 2, because of the following reasons:
But if I'm the only one thinking this is better, I'll move it to a separate document! One thing to note is that we already have two implementations ( |
One thing I've been thinking about is with large splices across many nodes, if some node fails to send signatures (likely because two nodes in the cluster demand to sign last) than splice will hang one I believe we need two things to address this:
Currently CLN fails the channel in this case as taking signatures and not responding is rather rude but this is bad because it could lead to clusters of splice channels being closed. The unfortunate side effect of this is we have to be comfortable sending out signatures with no recourse for not getting any back. I believe long term the solution is to maintain a signature-sending reputation for each peer and eventually blacklist peers from doing splices and / or fail your channels with that peer. A reputation system may be beyond the needs of the spec but what to do with hanging |
This is already covered at the quiescence level: quiescence will timeout if the splice doesn't complete (e.g. because we haven't received
I don't think this is necessary, and I think we should really require people to send
It seems like we've discussed this many times already: this simply cannot happen because ordering based on contributed amount fixes this? Can you detail a concrete scenario where |
- Either side has added an output other than the channel funding output | ||
and the balance for that side is less than the channel reserve that | ||
matches the new channel capacity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does it mean to have a channel reserve to "match the new channel capacity". AFAICT the channel_reserve is specified in satoshis and reading the negotiation process of this proposal doesn't seem to indicate that there is any change happening to that parameter during negotiation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAICT the channel_reserve is specified in satoshis
Not with dual-funding, where the channel reserve is 1% of the channel capacity. That's why this is potentially changing "automatically" when splicing on top of a dual-funded channel if we want to keep using 1%.
But you're right to highlight this: the channel reserve behavior is very loosely specified for now, and there were a lot of previous discussions with @morehouse regarding what we should do when splicing. Another edge case that we must better specify is what happens when splicing on top of a non-dual-funded channel, where the channel reserve was indeed a static value instead of a proportional one!
The channel reserve behavior is IMO the only missing piece of this specification, that we should discuss, thanks for bringing it up!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be a good thing to discuss in Tokyo!
Also worth stepping back and double checking the reserve requirement makes sense in its current form generally 👀.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of the following behavior for handling channel reserves:
- Whenever a splice happens, the channel is automatically enrolled into the 1% reserve policy, even if it wasn't initially a dual-funded channel (unless 0-reserve is used of course, see Add
option_zero_reserve
(FEATURE 64/65) #1140) - Splice-out is not allowed if you end up below your pre-splice reserve (your peer will reject that splice with
tx_abort
) - Otherwise, it's ok if one side ends up below the channel reserve after a splice: this is the same behavior as when a new channel is created. If we get into that state, the peer that is below the channel reserve:
- is not allowed to send outgoing HTLCs
- is allowed to receive incoming HTLCs
- if it is paying the commit fees, it is allowed to dip further into its channel reserve to receive HTLCs (because of the added weight of the HTLC output), because we must be able to move liquidity to their side to get them above their reserve
- When there are multiple unconfirmed splices, we use the highest channel reserve of all pending splices (ie requirements must be satisfied for all pending splice transactions)
As discussed during yesterday's meeting, there are subtle edge cases due to concurrent updates: this is inherent to the current commitment protocol, but will eventually become much simpler with #867
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
related: ACINQ/eclair#2899 (comment), tries to specify the concurrent edge cases and also the requirement when we would already (without splicing) allow the peer paying the fees being dipped below its reserve.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That all seems reasonable to me. The one part where we could get into trouble is:
if it is paying the commit fees, it is allowed to dip further into its channel reserve to receive HTLCs (because of the added weight of the HTLC output), because we must be able to move liquidity to their side to get them above their reserve
This allows the reserve to be violated, potentially all the way down to 0. In that situation, there is ~zero incentive to broadcast the latest commitment on force close.
That said, I know the implementation details are hairy to do things completely safely. And we can also look forward to zero-fee commitments with TRUC and ephemeral anchors, which would obsolete the "dip-into-reserve to pay fees" exception entirely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This allows the reserve to be violated, potentially all the way down to 0. In that situation, there is ~zero incentive to broadcast the latest commitment on force close.
Since we only allow this to happen when the node paying the fee receives HTLCs, the other node sending that HTLC can limit the exposure by controlling how many HTLCs they send in a batch (or keep pending the commit tx) when we're in this state.
There are unfortunately cases where even a single HTLC would make the node paying the fee have no output (small channels with high feerate), but when that happens you really don't have any other option, the channel is otherwise unusable, so your only other option is to force-close anyway which isn't great...
And we can also look forward to zero-fee commitments with TRUC and ephemeral anchors, which would obsolete the "dip-into-reserve to pay fees" exception entirely.
Exactly, this is coming together (look at this beautiful 0-fee commitment transaction: https://mempool.space/testnet4/tx/85f2256c8d6d61498c074d53912d1f0ef907ee508bb06f5701f3826432ba53b8) which will finally get rid of this kind of mess: I'm fine with using an imperfect but simple work-around in the meantime!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this requirement would solely be used for the splicing case, allowing HTLC which dip the opener into its reserve or should we make this an overall requirement. If so there is the problem with backwards compatibility, because older nodes (speaking for LND nodes) will force close if the opener dips below its reserve. So maybe it makes sense to only activate it for splicing use cases so that we don't run into the backwards compatibility issues ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea!
I added a PR to fix the spec for |
@t-bast asked me to put together a summary for Richard on how to implement the
|
@ddustin I've added more details for the announcement/gossip part in adf968c and finished implementing it in eclair. You can grab the last version of ACINQ/eclair#2887 for your cross-compatibility tests which should contain everything! |
Rebased to fix conflicts and squashed commits. Please carefully read the reconnection requirements, especially around handling of |
@ddustin the last commits add more information to |
Will do! Excited to get this all "locked" in |
02-peer-protocol.md
Outdated
`splice_locked` it has sent: | ||
- MUST retransmit `splice_locked`. | ||
- otherwise: | ||
- MUST NOT retransmit `splice_locked`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the routing-gossip spec it says on reconnection we should wait for splice_locked
before retransmitting announcement_signatures
.
If nodes are disconnected after splice_locked
messages are exchanged, but before announcement_signatures
are sent, should announcement_signatures
be sent after the channel_reestablish
message, or should this situation also trigger the splice_locked
message to be resent ?
Both nodes will know when they receive channel_reestablish
that splice_locked
has been exchanged but not announcement_signatures
so could proceed directly to exchanging signatures rather then wait for splice_locked
.
As currently written, both nodes will not send splice_locked
because your_last_funding_locked_txid
will match the most recent splice_locked
each node has sent and so the announcement_signatures
for a splice will not be exchanged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good point! In order to be compatible with taproot, we need to re-exchange splice_locked
in that case, because that's where we'll have the opportunity to provide the partial nonces necessary to sign the announcement.
We should do the same thing as what we're doing for the initial announcement_signatures
: on reconnection, if one side has not received the remote announcement_signatures
, it should re-send splice_locked
. When receiving that splice_locked
, the remote node should re-send splice_locked
once (if not already sent) to provide its partial nonces. Then both nodes can exchange announcement_signatures
again.
Thanks for pointing this, I'll fix it. I'll rebase the PR to fix the conflicts and will squash the reestablish commits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done a clean rebase to fix conflicts with option_simple_close
and took this opportunity to clean-up the commits:
- 43b5785 contains the bulk of the splicing protocol
- 5912705 contains the new TLV in
channel_reestablish
to clean-upsplice_locked
retransmission - c79dca9 describes the announcement logic and adds the retransmission requirements that make splicing future-proof with taproot
@ddustin I believe cln
will currently be missing implementation of the last two commits, you can focus your review on those two commits and ignore the first one which is (I think) already fully implemented in cln
.
Splicing allows spending the current funding transaction to replace it with a new one that changes the capacity of the channel, allowing both peers to add or remove funds to/from their channel balance. Splicing takes place while a channel is quiescent, to ensure that both peers have the same view of the current commitments. We don't want channels to be unusable while waiting for transactions to confirm, so channel operation returns to normal once the splice tx has been signed and we're waiting for it to confirm. The channel can then be used for payments, as long as those payments are valid for every pending splice transactions. Splice transactions can be RBF-ed to speed up confirmation. Once one of the pending splice transactions confirms and reaches acceptable depth, peers exchange `splice_locked` to discard the other pending splice transactions and the previous funding transaction. The confirmed splice transaction becomes the channel funding transaction. Nodes then advertize this spliced channel to the network, so that nodes keep routing payments through it without any downtime.
If one side sent `splice_locked` and the other side is ready to send its own `splice_locked` while they are disconnected, this creates a race condition on reestablish because `splice_locked` is retransmitted after `channel_reestablish`, and other channel updates can be inserted by the other node before receiving `splice_locked`. This will be an issue for taproot channels, because nonces will be missing. This race condition is described in more details in lightning#1223. We fix this race condition by adding TLVs to `channel_reestablish` that provide information about the latest locked transaction. This additional information also makes it easier to detect when we need to retransmit our previous `splice_locked`.
We make the requirements for `announcement_signatures` more clear. It is important that both nodes are able to generate the corresponding `channel_announcement` to allow them to create a new `channel_update` using the `short_channel_id` of the confirmed splice. We insist on exchanging `splice_locked` before generating signatures to ensure compatibility with taproot channels, where nonces will be exchanged in `splice_locked` messages. This means that we need to retransmit `splice_locked` on reconnection if `announcement_signatures` hasn't been fully exchanged. Importantly, after announcing a splice, nodes must still allow payments that use the previous `short_channel_id`, because remote nodes may not have processed the `channel_announcement` and `channel_update`s yet.
Splicing allows spending the current funding transaction to replace it with a new one that changes the capacity of the channel, allowing both peers to add or remove funds to/from their channel balance.
Splicing takes place while a channel is quiescent, to ensure that both peers have the same view of the current commitments.
We don't want channels to be unusable while waiting for transactions to confirm, so channel operation returns to normal once the splice transaction has been signed and we're waiting for it to confirm. The channel can then be used for payments, as long as those payments are valid for every pending splice transactions. Splice transactions can be RBF-ed to speed up confirmation.
Once one of the pending splice transactions confirms and reaches acceptable depth, peers exchange
splice_locked
to discard the other pending splice transactions and the previous funding transaction. The confirmed splice transaction becomes the channel funding transaction.Nodes then advertise this spliced channel to the network, so that nodes keep routing payments through it without any downtime.
This PR replaces #863 which contains a lot of legacy mechanisms for early versions of splicing, which didn't work in some edge cases (detailed in the test vectors provided in this PR). It can be very helpful to read the protocol flows described in the test vector: they give a better intuition of how splicing works, and how it deals with message concurrency and disconnections.
This PR requires the quiescence feature (#869) to start negotiating a splice.
Credits to @rustyrussell and @ddustin will be added in the commit messages once we're ready to merge this PR.