-
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
Splice draft (feature 62/63) #863
base: master
Are you sure you want to change the base?
Conversation
55b3992
to
6b3dc26
Compare
OK, many tweaks to wording, but in particular:
|
0750f9e
to
d2bf69f
Compare
OK, I reworked this on top of quiescence, and dropped the deterministic points which I am no longer convinced by. |
d2bf69f
to
596fe0b
Compare
I don't see a way to merge two existing channels into one. Is it being considered? |
AFAIK there's no "magic" trick but you can easily merge multiple channels. If you have N channels, just do a mutual close on N-1 of these channels and use the resulting utxos to splice into your last remaining channel. It does have an on-chain cost but there's no way around it (and once it's done, you have a single channel and you're good to go forever). |
Would be really nice being able to do it in a single transaction. It also wouldn't need confirmation because cheating is impossible in that case. |
It won't be possible in a single transaction, every existing channel needs one transaction to close and they're completely independent of each other. |
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 believe there is quite a bit of clean-up to do on this PR to remove references to old, obsolete parts of the proposal and tidying everything up. We should also:
- rebase the quiescence PR on top of
master
and squash it to a single commit - rebase splice on top of that quiescence PR and squash it to a single commit after cleaning it up to match the latest state of splice
- then we can iterate to converge
It will make it much easier for newcomers to jump in and waste less time explaining obsolete details of earlier prototypes.
@@ -32,6 +32,8 @@ operation, and closing. | |||
* [The `commitment_signed` Message](#the-commitment_signed-message) | |||
* [Sharing funding signatures: `tx_signatures`](#sharing-funding-signatures-tx_signatures) | |||
* [Fee bumping: `tx_init_rbf` and `tx_ack_rbf`](#fee-bumping-tx_init_rbf-and-tx_ack_rbf) | |||
* [The `funding_locked` Message](#the-funding_locked-message) |
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.
nit: rebase leftover?
@@ -203,6 +205,7 @@ This message contains a transaction input. | |||
|
|||
The sending node: | |||
- MUST add all sent inputs to the transaction | |||
- MUST only send confirmed inputs |
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.
Why? This should be gated by the require_confirmed_inputs
TLV on splice_init
/ splice_ack
.
@@ -32,6 +32,8 @@ operation, and closing. | |||
* [The `commitment_signed` Message](#the-commitment_signed-message) | |||
* [Sharing funding signatures: `tx_signatures`](#sharing-funding-signatures-tx_signatures) | |||
* [Fee bumping: `tx_init_rbf` and `tx_ack_rbf`](#fee-bumping-tx_init_rbf-and-tx_ack_rbf) | |||
* [The `funding_locked` Message](#the-funding_locked-message) | |||
* [Channel Quiescence](#channel-quiescence) |
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.
Can you add a link to the splicing
section?
1. type: 80 (`splice`) | ||
2. data: | ||
* [`channel_id`:`channel_id`] | ||
* [`chain_hash`:`chain_hash`] |
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.
Why a chain_hash
parameter? This applies to an existing channel, so it applies to its chain?
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.
Makes sense to me, would love to get @rustyrussell's opinion on perhaps the next spec call.
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.
@rustyrussell "Yes that is redundant now"
|
||
### The `splice` Message | ||
|
||
1. type: 80 (`splice`) |
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.
Can you add the require_confirmed_inputs
TLV to splice
and splice_ack
?
Upon receipt of consecutive `tx_complete`s, each node: | ||
- MUST fail negotiation if there is not exactly one input spending the current funding transaction. | ||
- MUST fail negotiation if there is not exactly one output with zero value paying to the two funding keys (a.k.a. the new channel funding output) | ||
- MUST calculate the channel capacity for each side: |
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's not how it works anymore since we started using a signed funding contribution (that is a delta of the what each peer is adding or removing), that section should be re-worked.
- MUST calculate the channel capacity for each side: | ||
- Start with the previous balance | ||
- Add that side's new inputs (excluding the one spending the current funding transaction) | ||
- Subtracting each sides new outputs (except the zero-value one paying to the funding keys) | ||
- Subtract the total fee that side is paying for the splice transaction. | ||
- MUST replace the zero-value funding output amount with the total 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.
This should be removed as well.
- If either side has added an output other than the new channel funding output: | ||
- MUST fail the negotiation if the balance for that side is less than 1% of the total 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.
I don't think this is accurate either: what we want to capture here is that nodes cannot splice-out below their reserve, right? This should be based on the relative_amount
in splice
/ splice_ack
.
2. types: | ||
1. type: 0 (`splice_info`) | ||
2. data: | ||
* [`channel_id`:`splice_channel_id`] |
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.
As previously discussed, I think we should rather use a batch_size
here, because otherwise you cannot properly deal with concurrency issues between splice_locked
and commit_sig
and may deadlock.
We need to specify in which order commit_sig
s are sent in that "batch": eclair currently sends them ordered by sending the commit_sig
matching the latest splice first, and the older ones afterwards.
@@ -1032,7 +1270,15 @@ A receiving node: | |||
- MUST fail the channel. | |||
- if any `htlc_signature` is not valid for the corresponding HTLC transaction OR non-compliant with LOW-S-standard rule <sup>[LOWS](https://github.com/bitcoin/bitcoin/pull/6769)</sup>: | |||
- MUST fail the channel. | |||
- MUST respond with a `revoke_and_ack` message. | |||
- if there is not exactly one `commitsigs` for each splice in progress: |
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 should just be:
- if there is not exactly one `commitsigs` for each splice in progress: | |
- if there is not exactly one `commit_sig` message for each splice in progress: |
This is much clearer?
Ah, yes, you're absolutely right. Adding
Sounds like a good thing to bring up on the spec call 📞
|
I wonder if It's not essential but it feels messy that say the last |
You mean that we would instead use something like
Agreed, I think the reserve requirements and edge cases it may create around splice-out (and combined splice-in and splice-out) is the main question that is still open for feedback, as we haven't really explored all of those edge cases yet! I won't be able to attend the next spec call, but I'll read the meeting notes to see the outcome of that brainstorm! |
Came up in the spec meeting: Should we be adding a |
This is somewhat orthogonal to splicing: But on the other hand, it makes |
There are some key design considerations when it comes to changing the The Dynamic Commitments proposal's main goal is to upgrade the The trouble is that if we were to just negotiate a taproot upgrade and then immediately broadcast the transaction that converts the funding output to the new segwit v1 output, we run into the problem wherein the new funding output is incompatible with our current gossip system. We are choosing to sidestep this by making it such that the output conversion transaction (referred to in the proposal as the "kickoff transaction") is not immediately broadcast. Like with splicing we can continue to use the channel when the kickoff hasn't confirmed yet, which in the extreme case doesn't have to confirm until the channel is closed! Splices can't do this because of the case where a splice-in's have other transaction inputs which may be double-spent to invalidate the splice transaction. (Note that if you are only splicing out, this is not the case!) Dynamic commitment If you want to upgrade During my research into the different mechanisms that had to be balanced I did notice that the "ideal" protocol would be capable of renegotiating all channel parameters that were originally negotiated in the I do think that it is unfortunate that we can't really unify the renegotiation of all of In summary, if you want to add |
Indeed, we would only use this after adding gossip support. FWIW I would have only enabled upgrading to taproot channels after adding support for taproot gossip, which makes this point moot. I think we should go ahead with taproot gossip as soon as possible to provide a clean path for channel upgrades.
Note that this "kick-off" transaction would thus need anchor outputs, or some other way of mitigating pinning.
Won't we run into issues where dynamic commitments and splicing create incompatibilities between each other? If you have an unconfirmed / unlocked kick-off transaction, how do you subsequently splice on top of this? The way I see it, splicing requires broadcasting a transaction that spends the current funding output. It makes it suitable for adding and removing funds into the channel, and upgrading the To be honest, it seems to me that dynamic commitment upgrades as it is proposed today is rather a hack to enable updating channels to taproot before taproot gossip, instead of being a generally useful complement to splicing. That may be fine if it doesn't interfere negatively with splicing, but if it makes those upgraded channels incompatible with splicing, that is IMO an issue. |
Yes. That is its most important goal. I think it's a good one since gossip upgrades require the whole network to adopt the new code, rather than dynamic commitments which only requires the two nodes on either side of the taproot edge to adopt it to keep it in sync with the rest of the network.
It was never supposed to be a useful complement to splicing to my knowledge. It was always a separate goal of making upgrading to taproot channels possible in a broadly usable way prior to a widely deployed new gossip system.
You have a few choices but they all have various sacrifices. One way or another you have to build the splice tx on top of the dyncomm kickoff, which would require a kickoff broadcast since the splice requires broadcast, or you rebuild the kickoff on top of the splice transaction which makes things a bit more complicated as you have to remember some extra context when doing chain monitoring and splicing.
It isn't fundamentally incompatible by my estimation, but mixing them is a thorny engineering issue with some discussions to be had and it would be incompatible until those engineering issues are solved. |
But then if that's the case, dynamic commitment upgrades doesn't seem like a good candidate for inclusion in the BOLTs, because once taproot gossip is deployed, upgrading to taproot channels will be more cleanly done using splicing? I don't think the BOLTs should contain complex protocols whose only goal is enabling something in the short term to work around a temporary limitation (lack of taproot gossip), because that creates technical debt. A cleaner path towards upgrading to taproot channels is to work more aggressively on taproot gossip and upgrade after that. In my opinion, the BOLTs should contain:
Those protocols would complement each other nicely without creating technical debt. That being said, I understand that |
It depends on your goal. If you don't care about forcing all users to do an on chain transaction to migrate to the new output type, then ofc you can just have them all close their transactions and re-open them. Today we have ~50k public channels, our goal is to be able to migrate the vast majority of them without having some flag day resulting in 100k transactions on chain. Commitment upgrades that require an off-chain kick off is just one of the upgrade types that dynamic commitments will enable. Even if everyone had already implemented taproot gossip, I still think we'd pursue this path to avoid forcing pretty much every publicly routable mainnet channel to close. If we can allow the channels that have been open for years to pretty much never close, then why wouldn't we pursue that path? The interaction of splicing and the greater gossip network is still incomplete: the only way to handle the change over in lock step is by using an on-chain hint, but not everyone seems to be a fan of that approach. The kick off upgrade mechanism is also the most direct path to enabling PTLCs across the network as well. Once upgraded a new channel update bit can be used to signal that the underlying channel actually supports the new payment type.
As I mentioned above, I don't think those two events are intertwined. Even if we already had taproot gossip widely deployed, we wouldn't want to force the entire public network to close and re-open channels.
Nothing inherently makes the proposal I also don't think it's accurate to convey splicing as designed today, as a more generic mechanism than it actually is. If you only want to change the funding output on a channel, then you really don't need: dynamic ability to add multiple inputs, have multiple in flight splices, the RBF mechanism, ambiguity of settled balances, etc, etc. Not to mention the pinning concerns that the current approach deems to be out of scope. |
There's no requirement to spend the funding transaction to update those channel parameters. I think you might be understanding some key concepts w.r.t the proposal. Negotiation and execution are distinct. If I negotiate a channel type that needs to change the funding output, then during execution we'd need to handle creation + signing of that kick off. If we negotiate a change to the dust param, then we'd use the existing HTLC update log semantics, and then next signature would cover a state that applies that new update. Pure param updates would be similar to the way |
This will never happen as a flag day anyway, since people won't upgrade their lightning implementation and decide to migrate all of their channels at the same time. Implementations should provide tools to upgrade channels when the feerate is low enough, to smooth out this transition. Using an unconfirmed kick-off transaction just hides the problem in the short term, but doesn't fix it at all?
I don't see what you are referring to exactly in this vague comment: dynamic commitments without publishing the kick-off transaction create a much greater security risk regarding pinning that splicing...
Not really, we can do something much simpler than that, there's no reason for a kick-off transaction here. We can modify commitment transactions without spending the current funding output.
I agree, and that's exactly what I'm saying in my comment. That kind of upgrades are useful, and cannot be done cleanly with splicing. But what I'm arguing is that the protocol that enables such upgrades should never require spending the funding output, as anything that spends the funding output should instead be done on top of splicing to avoid the technical debt of two competing protocols. |
Sure, that isn't how dynamic commitments is specified today. Negotiation and execution are distinct. You can update your dust limit without needing to spend the funding output. |
Based on
#862#869:We use the interactive tx construction protocol to make a splice: