Add FundingNeeded event for splicing
#4290
Conversation
When adding support for mixed splice-in and splice-out, the contribution amount will need to be computed based on the splice-in and splice-out values. Rather than add a third variant to SpliceContribution, which could have an invalid contribution amount, use a more general struct that can represent splice-in, splice-out, and mixed. Constructors are provided for the typical splice-in and splice-out case whereas support for the mixed case will be added in an independent change.
Some splicing use cases require to simultaneously splice in and out in the same splice transaction. Add support for such splices using the funding inputs to pay the appropriate fees just like the splice-in case, opposed to using the channel value like the splice-out case. This requires using the contributed input value when checking if the inputs are sufficient to cover fees, not the net contributed value. The latter may be negative in the net splice-out case.
|
👋 Thanks for assigning @wpaulino as a reviewer! |
Rather than requiring the user to pass FundingTxInputs when initiating a splice, generate a FundingNeeded event once the channel has become quiescent. This simplifies error handling and UTXO / change address clean-up by consolidating it in SpliceFailed event handling. Later, this event will be used for opportunistic contributions (i.e., when the counterparty wins quiescence or initiates), dual-funding, and RBF.
jkczyz
force-pushed
the
2025-12-new-splice-api
branch
from
f5933e5 to
854e9ca
Compare
|
@TheBlueMatt @wpaulino Looking for some high-level feedback on the API introduced in the last commit. In summary:
The same mechanism can be used later for contributing inputs for counterparty-initiated splices or v2 channel opens since Test code still needs to be fixed up, and |
wpaulino
left a comment
wpaulino
left a comment
There was a problem hiding this comment.
The API design LGTM, though there's one issue with WalletSource. One thing users need to keep in mind now is that from the moment they receive FundingNeeded, they need to act quickly to ensure the counterparty doesn't disconnect due to quiescence taking too long.
| fn list_confirmed_utxos(&self) -> Result<Vec<Utxo>, ()>; | ||
|
|
||
| /// | ||
| fn select_confirmed_utxos( |
There was a problem hiding this comment.
Adding this here now requires implementers to satisfy this method when, in the context of anchor channels, WalletSource is only intended to be used such that we perform coin selection on behalf of the user. Ideally, we also give users the option between choosing WalletSource/CoinSelectionSource when funding channels.
There was a problem hiding this comment.
Right, I guess I'm a bit confused why we can't use select_confirmed_utxos as-is? Indeed the claim_id is annoying, but we can make that either an enum across a ClaimId and some unit value describing a splice or just make it an Option. Aside from that it seems to be basically what we want.
There was a problem hiding this comment.
Adding this here now requires implementers to satisfy this method when, in the context of anchor channels,
WalletSourceis only intended to be used such that we perform coin selection on behalf of the user. Ideally, we also give users the option between choosingWalletSource/CoinSelectionSourcewhen funding channels.
Hmm... I see. Would a separate trait be desirable? Also, see my reply to @TheBlueMatt below.
Right, I guess I'm a bit confused why we can't use
select_confirmed_utxosas-is? Indeed theclaim_idis annoying, but we can make that either anenumacross aClaimIdand some unit value describing a splice or just make it anOption. Aside from that it seems to be basically what we want.
The return value also isn't compatible. It contains Utxos but we also need the previous tx and sequence number as part of each FundingTxInput. Though its constructor will give a default sequence number.
We could change CoinSelection to use FundingTxInput instead of Utxo, but that would be odd for use with the anchor context.
There was a problem hiding this comment.
Honestly that seems fine to me? We expect ~all of our users to want to use splicing, which implies they need to support the "return coin selection with full transactions" interface. So what if anchors throw away some of that data?
If we feel strongly about it we can add a new trait method that does return the full transactions and provide a default implementation for the current method so that those that really want to avoid always fetching the transaction data can.
There was a problem hiding this comment.
Right, I don't have a strong opinion, but note that Wallet's implementation of CoinSelectionSource::select_confirmed_utxos delegates to WalletSource::list_confirmed_utxos. So it might be expensive to use that abstraction. @wpaulino WDYT?
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
Aside from the above which-interface question I think the API is good.
4 participants
Rather than requiring the user to pass
FundingTxInputs when initiating a splice, generate aFundingNeededevent once the channel has become quiescent. This simplifies error handling and UTXO / change address clean-up by consolidating it inSpliceFailedevent handling.Later, this event will be used for opportunistic contributions (i.e., when the counterparty wins quiescence or initiates), dual-funding, and RBF.
Based on #4261.
This is still fairly rough. It does not yet include any code for creating aFundingNegotiationContextfrom aFundingContribution. The former may need to a dedicated struct instead so that any data needed fromChannelManagerorChannelContextcan be produced internally. Alternatively, that data could be included inFundingContribution, but it would need to be serializable.