lightningdevkit / rust-lightning

A highly modular Bitcoin Lightning library written in Rust. It's rust-lightning, not Rusty's Lightning!
Other
1.16k stars 364 forks source link

Support RBF for interactively constructed transactions #3281

Open dunxen opened 2 months ago

dunxen commented 2 months ago

Overview

Support fee-bumping for interactively constructed transactions as described in the spec.

Each RBF attempt would need to double-spend any previous interactive transaction for that channel.

Also importantly from the spec,

Peers can use different values in tx_init_rbf.funding_output_contribution and tx_ack_rbf.funding_output_contribution from the amounts transmitted in open_channel2 and accept_channel2: they are allowed to change how much they wish to commit to the funding output.

so we'd need to ensure we update channel capacity and other relevant details to reflect a possible new funding output depending on which transaction confirms.

In dual-funding, since the channel ID is independent of the funding txid, all attempts can be mapped to that key and persisted in some "funding fee bumps" attempts map.

In splicing the same idea should apply as we already have an existing channel with a fixed channel ID.

This work at least depends on #3137.

Requirements

optout21 commented 2 months ago

Some ideas for RBF:

In more details:

Additional important considerations:

dunxen commented 1 month ago

Some ideas for RBF:

* Split the current `Funded` channel phase into `FundingPending` and `Funded`

* In `FundingPending` have the option to have not one channel, but several -- the RBF variants

In more details:

* Current `Funded(channel)` phase is used for channels where the funding tx is known, regardless if it is confirmed or not

* Separate this phase into two: when funding tx is negotiated, first if goes to `FundingPending`, and when the tx is confirmed, it is changed into `Funded` (note: it can also go back in case of unconfirmation)

* This separation may be beneficial for other reasons as well (the two cases can be discriminated w/o looking inside the channel)

* When new RBF variant is initiated, a new channel object will be created, and added to the collection of channels in `FundingPending`

* When the funding tx of one of the channels (variants) confirms, the `FundingPending(channels)` is transitioned to `Funded(channel)`, with the locked channel. The other variants are discarded.

* There are many code branches that currently work only for `Funded` phase currently. The neutral mechanical change is to extend the conditions for both phases everywhere, though in many code paths only one phase makes sense. In many cases it has to be decided one-by-one if the logic makes no sense for `FundingPending`, or it makes sense and have to be applied to _all_ variants.

Additional important considerations:

* Persistence, b/w compatibility

Thanks for thinking a bit about the design!

This implementation sounds doable but (at least from working on the dual-funding spec) I'm not entirely convinced these RBF transactions should be stored at the channel/channel phase level. Since details of these transactions will need to be persisted that might make for an awkward Writeable implementation for channels.

The distinction made by the Funded vs FundingPending variants seems helpful, but I do believe (partly my fault" that "funding pending" has become a bit overloaded as a term now :(. At least that's "just" a naming problem that can be solved.

I still believe it might be better to have a new map at the PeerState level that maps ChannelId to RBF attempts (rbf tx details).

optout21 commented 1 month ago

RBF attempts differ only in transaction details, but I am also considering splicing, where pre-splice channel and post-splice channel -- with possibly multiple variants -- have to considered together. Pre-splice channel differs in other parameters as well, hence I was thinking of a more general approach. But this may not be the right one.

In the meantime I also found a difficulty with my earlier proposal: the RBF variants can contain a first negotiated variant, and a second under-negotiation variant, which require different contexts (Channel<SP> vs. OutboundV2Channel<SP> or inbound).

Another idea is to carve-out transaction-related info from ChannelContext, and extending ChannelContext to be able to hold additional funding variants.

optout21 commented 1 month ago

A current proposal for channel states/phases is described here: https://gist.github.com/optout21/2ead7a0f4e2c3d34aed1c9703c8cfb39