Split splice initiation into two phases by jkczyz · Pull Request #4290 · lightningdevkit/rust-lightning

jkczyz · 2025-12-18T16:11:08Z

Previously, splice_channel required callers to manually construct funding inputs and pass them directly, making coin selection the caller's responsibility. This made the API difficult to use and prevented reuse of the existing CoinSelectionSource trait.

Introduce a two-phase API: splice_channel now returns a FundingTemplate that callers use to build a FundingContribution via wallet-backed splice methods (e.g., splice_in_sync, splice_out_sync), which handle coin selection automatically. The completed contribution is then passed to a new funding_contributed method to begin quiescence and negotiation.

This also renames SpliceContribution to FundingContribution and moves fee estimation and input validation into the funding module, co-located with the types they operate on.

ldk-reviews-bot · 2025-12-18T16:11:11Z

👋 Thanks for assigning @TheBlueMatt as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

jkczyz · 2026-01-12T18:03:27Z

@TheBlueMatt @wpaulino Looking for some high-level feedback on the API introduced in the last commit. In summary:

User passes SpliceContribution -- which no longer contains any FundingTxInputs -- to ChannelManager::splice_channel
Upon quiescence LDK generates a FundingNeeded event which contains a FundingTemplate
User calls FundingTemplate::build or FundingTemplate::build_sync with a WalletSource or WalletSourceSync, respectively, to produce a FundingContribution
User passes FundingContribution -- which contains the FundingTxInputs -- to ChannelManager::funding_contributed
LDK validates that the FundingContribution can pay for inputs / outputs, causing LDK to either send splice_init or produce a SpliceFailed event.

The same mechanism can be used later for contributing inputs for counterparty-initiated splices or v2 channel opens since FundingTemplate and FundingContribution contains the context.

Test code still needs to be fixed up, and change_script generation will follow in another commit.

wpaulino

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.

wpaulino · 2026-01-12T19:37:10Z

lightning/src/events/bump_transaction/sync.rs

 	fn list_confirmed_utxos(&self) -> Result<Vec<Utxo>, ()>;
+
+	///
+	fn select_confirmed_utxos(


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.

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.

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.

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_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.

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.

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.

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?

I was thinking of keeping CoinSelectionSource the same (with the full transaction data in the response, or with the default-impl-method described above) but changing WalletSource so that we don't have to fetch all previous-transactions at the start.

Yeah, Wallet wraps a WalletSource and implements CoinSelectionSource by listing all the UTXOs and selecting from them. So WalletSource's interface would remain unchanged while CoinSelection would use FundingTxInput instead of Utxo. Which I guess means FundingTemplate::build should actually take a CoinSelectionSource.

Seems reasonable to require a sequence number in the response for that as well, even for anchors?

Hmm... in WalletSource::list_confirmed_utxos by adding a field to Utxto (and removing it from FundingTxInput)? Or by having the CoinSelectionSource implementation fill it in on FundingTxInput?

So WalletSource's interface would remain unchanged

Wouldn't we need a WalletSource::get_previous_transaction_for_utxo method to fetch the full tx data for the UTXOs we selected?

Hmm... in WalletSource::list_confirmed_utxos by adding a field to Utxto (and removing it from FundingTxInput)? Or by having the CoinSelectionSource implementation fill it in on FundingTxInput?

ISTM we should replace Utxo with FundingTxInput since FundingTxInput has strictly more fields (it contains a Utxo!) and we'd move to returning FundingTxInput from the trait.

Wouldn't we need a WalletSource::get_previous_transaction_for_utxo method to fetch the full tx data for the UTXOs we selected?

Right, we need another method for that.

ISTM we should replace Utxo with FundingTxInput since FundingTxInput has strictly more fields (it contains a Utxo!) and we'd move to returning FundingTxInput from the trait.

The question is more what should be setting Sequence? Either:

(1) Move it to Utxo and have WalletSource::list_confirmed_utxos set it since it returns Vec<Utxo>.
(2) Have CoinSelectionSource::select_confirmed_utxos set it since CoinSelection would now contain Vec<FundingTxInput>

We just can't replace Utxo with FundingTxInput in WalletSource::list_confirmed_utxos since we don't want to return the previous tx there.

(1) Move it to Utxo and have WalletSource::list_confirmed_utxos set it since it returns Vec.

Presumably this. No reason to want it to not be possible in WalletSource.

Done as discussed here and offline. I'm in the middle of updating the tests, but I've pushed an update for now.

TheBlueMatt

Aside from the above which-interface question I think the API is good.

ldk-reviews-bot · 2026-01-15T00:28:47Z

🔔 1st Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-15T00:28:48Z

🔔 1st Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-17T00:29:31Z

🔔 2nd Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-17T00:29:32Z

🔔 2nd Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-19T00:29:48Z

🔔 3rd Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-19T00:29:50Z

🔔 3rd Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

jkczyz · 2026-01-20T19:20:35Z

lightning/src/ln/funding.rs

+			// FIXME: Should claim_id be an Option?
+			let claim_id = ClaimId([0; 32]);


Regarding the CoinSelectionSource API, do we want to make claim_id an Option?

jkczyz · 2026-01-20T19:24:28Z

lightning/src/ln/splicing_tests.rs

-		Amount::from_sat(383)
+		Amount::from_sat(385)
 	} else {
-		Amount::from_sat(384)
+		Amount::from_sat(386)


Seems select_confirmed_utxos_internal might be off on the change calculation because it's using the weight of the change output to compute additional fees instead of re-computing the total fees using the total weight when including a change output.

ldk-reviews-bot · 2026-01-21T00:30:25Z

🔔 4th Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-01-21T00:30:27Z

🔔 4th Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

codecov · 2026-01-21T19:58:51Z

Codecov Report

❌ Patch coverage is 73.59477% with 202 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.89%. Comparing base (3497b59) to head (20916b7).
⚠️ Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channel.rs	56.18%	83 Missing and 2 partials ⚠️
lightning/src/ln/funding.rs	82.50%	47 Missing and 20 partials ⚠️
lightning/src/ln/channelmanager.rs	52.00%	36 Missing ⚠️
lightning/src/events/bump_transaction/mod.rs	85.50%	6 Missing and 4 partials ⚠️
lightning/src/ln/functional_test_utils.rs	50.00%	3 Missing ⚠️
lightning/src/util/ser.rs	85.71%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4290      +/-   ##
==========================================
- Coverage   86.09%   85.89%   -0.20%     
==========================================
  Files         156      156              
  Lines      103623   103963     +340     
  Branches   103623   103963     +340     
==========================================
+ Hits        89211    89304      +93     
- Misses      11895    12132     +237     
- Partials     2517     2527      +10

Flag	Coverage Δ
tests	`85.89% <73.59%> (-0.20%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

TheBlueMatt

API looks good. some misc comments i noted while skimming it

lightning/src/events/bump_transaction/mod.rs

TheBlueMatt · 2026-01-21T22:30:23Z

lightning/src/events/mod.rs

+	/// Indicates that funding is needed for a channel splice or a dual-funded channel open.
+	///
+	/// The client should build a [`FundingContribution`] from the provided [`FundingTemplate`] and
+	/// pass it to [`ChannelManager::funding_contributed`].


Needs a call-out to what to do if you actually don't want to splice anymore (ie on failure)

Also maybe a note that the channel is hung waiting on our response, so we need to respond quickly.

Should they simply not handle the event? We'll be in quiescence and timeout after DISCONNECT_PEER_AWAITING_RESPONSE_TICKS. Though it seems this now inadvertently (but maybe expectedly) now applies to us sending splice_init. So maybe some renaming is in order?

Or should we expose ChannelManager::exit_quiescence?

Now that the presence of this event in the pending events queue determines whether or not a splice can be initiated, no action is needed from the user if they no longer want to splice.

TheBlueMatt · 2026-01-21T22:39:22Z

lightning/src/ln/funding.rs

+			// FIXME: Should claim_id be an Option?
+			let claim_id = ClaimId([0; 32]);


TheBlueMatt · 2026-01-21T22:40:28Z

lightning/src/ln/funding.rs

+
+	/// Creates a `FundingContribution` from the template by using `wallet` to perform coin
+	/// selection with the given fee rate.
+	pub fn build_sync<W: Deref>(


Can we also have a method for building with a provided set of inputs rather than going through the trait?

We could... or probably a CoinSelection so that a change output can be given.

test_cases/chanmon_consistency/any_filename_works

TheBlueMatt · 2026-01-21T22:47:47Z

lightning/src/ln/channel.rs

+		if self.context.channel_state.is_quiescent() {
+			return Err(APIError::APIMisuseError {
+				err: format!(
+					"Channel {} cannot be spliced as it is already quiescent",


Don't we want to support queuing up the splice to do afterwards?

Hmmm... this was to handle the time after we've generated FundingNeeded when we no longer have a quiescent_action but are still quiescent waiting on the user to call funding_contributed. But then we can't differentiate this from counterparty-initiated quiescence. Maybe we need to make a placeholder QuiescentAction for when we are waiting on the user to respond? Something like AwaitingFundingContribution, which could also be checked when funding_contributed is called.

Regarding error handling, since the FundingContribution is taken by value we need to generate a SpliceFailed event to allow the user to unlock the UTXOs. However, some failure may be result of misuse (e.g., wrong channel / counterparty, unexpected funding) or bad timing (e.g., peer already disconnected timing out quiescence).

In those cases, it doesn't make sense to generate SpliceFailed. Maybe we need to use DiscardFunding for some of these cases? We'd need another FundingInfo variant, though.

Otherwise, we'd need to return the UTXOs back to the caller, which we wanted to avoid.

Thoughts?

I'll leave this for a follow-up PR.

Seems like the motivation for this is no longer relevant now that FundingNeeded comes before quiescence. We should definitely allow queueing the splice when quiescent, but only if we're not the initiator and/or we're attempting a different quiescent protocol (not possible yet). If we're already splicing, we should suggest doing an RBF once that's supported.

Seems like the motivation for this is no longer relevant now that FundingNeeded comes before quiescence.

The misuse cases still hold when calling funding_contributed. Maybe they are less likely though given it would essentially be a programmer error? Although, is it still possible when a channel is closed before calling funding_contributed?

We should definitely allow queueing the splice when quiescent, but only if we're not the initiator and/or we're attempting a different quiescent protocol (not possible yet). If we're already splicing, we should suggest doing an RBF once that's supported.

Ok, that should work, but note that once the splice is pending and we are no longer quiescent, we've lost whether we had been the initiator. So we'd need to allow queueing another splice then, which I think should be fine.

When implementing RBF, either side could include the contributions from the enqueued action.

TheBlueMatt · 2026-01-21T22:50:38Z

lightning/src/ln/channel.rs

+	) -> Result<msgs::SpliceInit, SpliceFundingFailed>
+	where
+		L::Target: Logger,
+	{


presumably we need to check that we're quiescent and its our turn to talk?

Based on offline discussion, we are no longer waiting to enter quiescence to generate FundingNeeded. It will instead be initiated when funding_contributed is called.

lightning/src/ln/channel.rs

jkczyz · 2026-01-22T01:29:24Z

Given that the new API is to be reused for RBF, I wrote up a proposal to consider with this PR. @TheBlueMatt @wpaulino Looking for feedback / alternative ideas before continuing that work.

Observations

DiscardFunding contains the funding outpoint. So I assume users will need to look up the transaction to unlock any UTXOs and reclaim script pubkeys?
For RBF, I see we have FundingScope::funding_transaction, which is an Option. I thought we wanted to clear this, but I don't see where we are doing that.
Either of those require the user to know which outputs are their own and isn't change in order to recreate the SpliceContribution.
We produce a FundingTemplate from a SpliceContribution. The user may add more value / outputs before constructing a FundingContribution.
When implementing the acceptor case, there will not be a SpliceContribution -- just an empty FundingTemplate to be modified if needed before producing a FundingContribution. Similarly for dual funding.
FundingContribution can't differentiate user selected UTXOs from change.

Proposal

Store change output separately in FundingContribution instead of losing the information to outputs.
Store the shared input in FundingContribution to allow recreating the FundingTemplate
Recreate the FundingTemplate internally from the FundingContribution (before it is consumed into a FundingNegotiationContext) and keep track of it until it can be included alongside the negotiated FundingScopes
Alternatively, re-create FundingTemplate somehow from the InteractiveTxConstructor / InteractiveTxSigningSession data. Note, however, that which output, if any, is change will have been already lost.
Expose these FundingTemplates in ChannelDetails for informational purposes.
User initiated RBF doesn't require passing a SpliceContribution.
Instead another FundingNeeded event is generated -- once the channel is quiescent -- containing the last used FundingTemplate
User uses any of the previously used FundingTemplates or the one given in the event (modifying as they see fit) to call ChannelManager::funding_contributed.

Questions

How should we expose the FeeRate used in each negotiation? We need to make sure the RBF uses a greater fee rate, so at very least we need to keep track of it. User also needs to know to pass a higher fee rate when creating the new FundingContribution.
Do we need to also expose the inputs used in each negotiation? They aren't needed for RBF, so would only be informational.

jkczyz · 2026-02-13T20:57:12Z

After some offline discussion, we decided to drop the FundingNeeded event entirely and simply return FundingTemplate from splice_channel. This will allow us to handle more mixed mode use cases in the future.

A forthcoming commit will change CoinSelection to include FundingTxInput instead of Utxo, though the former will probably be renamed. This is so CoinSelectionSource can be used when funding a splice. Further updating WalletSource to use FundingTxInput is not desirable, however, as it would result in looking up each confirmed UTXOs previous transaction even if it is not selected. See Wallet's implementation of CoinSelectionSource, which delegates to WalletSource for listing all confirmed UTXOs. This commit moves FundingTxInput::sequence to Utxo, and thus the responsibility for setting it to WalletSource implementations. Doing so will allow Wallet's CoinSelectionSource implementation to delegate looking up previous transactions to WalletSource without having to explicitly set the sequence on any FundingTxInput.

jkczyz · 2026-02-13T21:26:02Z

Rebased to fix build error from removal of SpliceContribution.

ldk-reviews-bot · 2026-02-14T00:00:57Z

🔔 4th Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:00:55Z

🔔 2nd Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-02-16T00:02:11Z

🔔 5th Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

TheBlueMatt

A handful of comments that are worth following up on, but I'd much rather land this as-is so that various conflicts (eg chanmon_consistency updates) can be avoided and we can get this train moving.

lightning/src/util/test_utils.rs

lightning/src/events/bump_transaction/mod.rs

TheBlueMatt · 2026-02-15T23:44:45Z

lightning/src/ln/channel.rs

+					}
+
+					let prev_funding_input = self.funding.to_splice_funding_input();
+					let is_initiator = contribution.is_initiator();


We should always be the initiator at this point - we're in if is_holder_quiescence_initiator {. Probably we should debug-assert that here because otherwise we probably have the wrong funding.

I don't think we'll ever need is_initiator in a QuiescentAction now that we won't use an event for acceptors to contribute. Instead, we'll set it later based on how the QuiescentAction ends up being used.

FWIW, I don't think we have a good way to let the acceptor to not over pay fees generally. By the time we know who won quiescence, the coins have already been selected. We could try adjusting the change output once we know we've lost, but that assumes there is a change output to adjust.

TheBlueMatt · 2026-02-15T23:53:06Z

lightning/src/ln/funding.rs

-		let value_added = self.value_added.to_signed().unwrap_or(SignedAmount::MAX);
+	/// spliced out than spliced in. Fees will be deducted from the expected splice-out amount
+	/// if no inputs were included.
+	pub fn net_value(&self) -> Result<SignedAmount, String> {


Certainly can be in a followup but imo this should be infallible and checked when constructing the contribution.

The problem is that we can't perform some checks until after coin selection. But if we fail at that point, the user isn't able to reclaim their UTXOs.

Ended up splitting the checks into two, some performed when building the FundingContribution while the others are in a separate validate method called in funding_contributed. This let's us make net_value infallible when handling stfu.

TheBlueMatt · 2026-02-15T23:55:40Z

lightning/src/ln/channelmanager.rs

+		&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, feerate: FeeRate,
+	) -> Result<FundingTemplate, APIError> {
+		let mut res = Err(APIError::APIMisuseError { err: String::new() });
 		PersistenceNotifierGuard::optionally_notify(self, || {


nit: we dont have to block persistence just reading channel fields now (and can presumably drop internal_splice_channel)

TheBlueMatt · 2026-02-16T00:14:13Z

lightning/src/ln/funding.rs

+	shared_input: Option<Input>,
+
+	/// The fee rate to use for coin selection.
+	feerate: FeeRate,


Do we need this here or can we pass it when building the actual funding?

When initiating an RBF later, we'd like the returned FundingTemplate to use a FeeRate that meets the spec requirements instead of asking the user to calculate it correctly.

Wouldn't they still provide a feerate upfront in splice_channel? I guess if they provide something higher, we'd keep it the same, but bump it to the minimum required if it's lower.

Right, though there are two scenarios. They call splice_channel as you said and we try to RBF to add their contribution. Or they call something like initiate_rbf because the splice isn't confirming and they need to run coin selection again with a higher fee rate.

ISTM we could have a minimum-feerate in the template, but presumably even in the rbf case they might want to rbf to a higher feerate than some minimum required calculation so we'll want to enable that anyway?

If we don't have them specify a fee rate in splice_channel, what should the resulting template use for the minimum fee rate? Something from the fee estimator? Presuming there is no outstanding counterparty-initiated splice.

TheBlueMatt · 2026-02-16T01:04:27Z

lightning/src/ln/funding.rs

+			wallet,
+		)
+	}
+}


I noted it previously but in a followup we do need a "manual builder" here that doesn't rely on a coin selection.

TheBlueMatt · 2026-02-16T01:04:44Z

lightning/src/ln/funding.rs

+
+	/// Creates a [`FundingContribution`] for both adding and removing funds from a channel using
+	/// `wallet` to perform coin selection.
+	pub fn splice_in_and_out_sync<W: Deref>(


We dropped the Deref pattern and should avoid adding it in new code.

Having trouble making this work because of the 'a lifetime on the trait.

error[E0309]: the parameter type `T` may not live long enough --> lightning/src/util/wallet_utils.rs:408:3 | 404 | fn select_confirmed_utxos<'a>( | -- the parameter type `T` must be valid for the lifetime `'a` as defined here... ... 408 | self.deref().select_confirmed_utxos(claim_id, must_spend, must_pay_to, target_feerate_sat_per_1000_weight, max_tx_weight) | ^^^^^^^^^^^^ ...so that the type `T` will meet its required lifetime bounds | help: consider adding an explicit lifetime bound | 407 | ) -> impl Future<Output = Result<CoinSelection, ()>> + MaybeSend + 'a where T: 'a { | +++++++++++

But the suggestion won't work because it causes the trait signature to no longer match. And there's no T in the trait to allow to match.

error[E0195]: lifetime parameters or bounds on method `select_confirmed_utxos` do not match the trait declaration --> lightning/src/util/wallet_utils.rs:404:27 | 389 | fn select_confirmed_utxos<'a>( | ---- lifetimes in impl do not match this method in trait ... 404 | fn select_confirmed_utxos<'a>( | ^^^^ lifetimes do not match method in trait ... 407 | ) -> impl Future<Output = Result<CoinSelection, ()>> + MaybeSend + 'a where T: 'a { | ----------- this `where` clause might not match the one in the trait

Hmm, maybe an issue in some of your later PRs, but this builds fine on this PR:

diff --git a/lightning/src/events/bump_transaction/mod.rs b/lightning/src/events/bump_transaction/mod.rs index c783e96381..909bf15fbe 100644 --- a/lightning/src/events/bump_transaction/mod.rs +++ b/lightning/src/events/bump_transaction/mod.rs @@ -443,6 +443,26 @@ pub trait CoinSelectionSource { ) -> impl Future<Output = Result<Transaction, ()>> + MaybeSend + 'a; } +impl<C: Deref> CoinSelectionSource for C +where + C::Target: CoinSelectionSource, +{ + fn select_confirmed_utxos<'a>( + &'a self, claim_id: Option<ClaimId>, must_spend: Vec<Input>, must_pay_to: &'a [TxOut], + target_feerate: u32, max_weight: u64, + ) -> impl Future<Output = Result<CoinSelection, ()>> + MaybeSend + 'a { + self + .deref() + .select_confirmed_utxos(claim_id, must_spend, must_pay_to, target_feerate, max_weight) + } + + fn sign_psbt<'a>( + &'a self, psbt: Psbt, + ) -> impl Future<Output = Result<Transaction, ()>> + MaybeSend + 'a { + self.deref().sign_psbt(psbt) + } +} + /// An alternative to [`CoinSelectionSource`] that can be implemented and used along [`Wallet`] to /// provide a default implementation to [`CoinSelectionSource`]. /// @@ -763,12 +783,10 @@ where // Note that updates to documentation on this struct should be copied to the synchronous version. pub struct BumpTransactionEventHandler< B: BroadcasterInterface, - C: Deref, + C: CoinSelectionSource, SP: SignerProvider, L: Logger, -> where - C::Target: CoinSelectionSource, -{ +> { broadcaster: B, utxo_source: C, signer_provider: SP, @@ -776,10 +794,8 @@ pub struct BumpTransactionEventHandler< secp: Secp256k1<secp256k1::All>, } -impl<B: BroadcasterInterface, C: Deref, SP: SignerProvider, L: Logger> +impl<B: BroadcasterInterface, C: CoinSelectionSource, SP: SignerProvider, L: Logger> BumpTransactionEventHandler<B, C, SP, L> -where - C::Target: CoinSelectionSource, { /// Returns a new instance capable of handling [`Event::BumpTransaction`] events. /// diff --git a/lightning/src/events/bump_transaction/sync.rs b/lightning/src/events/bump_transaction/sync.rs index 39088bb0e9..f800fc98e6 100644 --- a/lightning/src/events/bump_transaction/sync.rs +++ b/lightning/src/events/bump_transaction/sync.rs @@ -226,26 +226,27 @@ pub trait CoinSelectionSourceSync { fn sign_psbt(&self, psbt: Psbt) -> Result<Transaction, ()>; } -struct CoinSelectionSourceSyncWrapper<T: Deref>(T) +impl<C: Deref> CoinSelectionSourceSync for C where - T::Target: CoinSelectionSourceSync; - -// Implement `Deref` directly on CoinSelectionSourceSyncWrapper so that it can be used directly -// below, rather than via a wrapper. -impl<T: Deref> Deref for CoinSelectionSourceSyncWrapper<T> -where - T::Target: CoinSelectionSourceSync, + C::Target: CoinSelectionSourceSync, { - type Target = Self; - fn deref(&self) -> &Self { + fn select_confirmed_utxos( + &self, claim_id: Option<ClaimId>, must_spend: Vec<Input>, must_pay_to: &[TxOut], + target_feerate: u32, max_weight: u64, + ) -> Result<CoinSelection, ()> { self + .deref() + .select_confirmed_utxos(claim_id, must_spend, must_pay_to, target_feerate, max_weight) + } + + fn sign_psbt(&self, psbt: Psbt) -> Result<Transaction, ()> { + self.deref().sign_psbt(psbt) } } -impl<T: Deref> CoinSelectionSource for CoinSelectionSourceSyncWrapper<T> -where - T::Target: CoinSelectionSourceSync, -{ +struct CoinSelectionSourceSyncWrapper<T: CoinSelectionSourceSync>(T); + +impl<T: CoinSelectionSourceSync> CoinSelectionSource for CoinSelectionSourceSyncWrapper<T> { fn select_confirmed_utxos<'a>( &'a self, claim_id: Option<ClaimId>, must_spend: Vec<Input>, must_pay_to: &'a [TxOut], target_feerate_sat_per_1000_weight: u32, max_tx_weight: u64, @@ -278,20 +279,16 @@ where // Note that updates to documentation on this struct should be copied to the synchronous version. pub struct BumpTransactionEventHandlerSync< B: BroadcasterInterface, - C: Deref, + C: CoinSelectionSourceSync, SP: SignerProvider, L: Logger, -> where - C::Target: CoinSelectionSourceSync, -{ +> { bump_transaction_event_handler: BumpTransactionEventHandler<B, CoinSelectionSourceSyncWrapper<C>, SP, L>, } -impl<B: BroadcasterInterface, C: Deref, SP: SignerProvider, L: Logger> +impl<B: BroadcasterInterface, C: CoinSelectionSourceSync, SP: SignerProvider, L: Logger> BumpTransactionEventHandlerSync<B, C, SP, L> -where - C::Target: CoinSelectionSourceSync, { /// Constructs a new instance of [`BumpTransactionEventHandlerSync`]. pub fn new(broadcaster: B, utxo_source: C, signer_provider: SP, logger: L) -> Self { diff --git a/lightning/src/ln/funding.rs b/lightning/src/ln/funding.rs index 06b972d712..c3bbf661ca 100644 --- a/lightning/src/ln/funding.rs +++ b/lightning/src/ln/funding.rs @@ -202,12 +202,9 @@ impl FundingTemplate { /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using /// `wallet` to perform coin selection. - pub async fn splice_in_and_out<W: Deref + MaybeSend>( + pub async fn splice_in_and_out<W: CoinSelectionSource + MaybeSend>( self, value_added: Amount, outputs: Vec<TxOut>, wallet: W, - ) -> Result<FundingContribution, ()> - where - W::Target: CoinSelectionSource + MaybeSend, - { + ) -> Result<FundingContribution, ()> { if value_added == Amount::ZERO && outputs.is_empty() { return Err(()); } @@ -217,12 +214,9 @@ impl FundingTemplate { /// Creates a [`FundingContribution`] for both adding and removing funds from a channel using /// `wallet` to perform coin selection. - pub fn splice_in_and_out_sync<W: Deref>( + pub fn splice_in_and_out_sync<W: CoinSelectionSourceSync>( self, value_added: Amount, outputs: Vec<TxOut>, wallet: W, - ) -> Result<FundingContribution, ()> - where - W::Target: CoinSelectionSourceSync, - { + ) -> Result<FundingContribution, ()> { if value_added == Amount::ZERO && outputs.is_empty() { return Err(()); }

This works but it doesn't follow that pattern of NodeSigner:

rust-lightning/lightning/src/sign/mod.rs

Line 1008 in 0a20fa5

impl<T: NodeSigner + ?Sized, N: Deref<Target = T>> NodeSigner for N {

That pattern works for CoinSelectionSourceSync but not CoinSelectionSource. Though using a where clause for Target and adding ?Sized to the parameter requiring Deref instead of Target works:

impl<C: Deref + ?Sized> CoinSelectionSource for C where C::Target: CoinSelectionSource, {

Gemini tells me this approach is more likely correct. Do you agree?

Ok, nevermind about that. I just had Claude do it and ask why it dropped the ?Sized constraint. This is what it said:

❯ Why does the ?Sized constraint need to be removed?

⏺ Looking at the sync blanket impl that already exists:

impl<T: CoinSelectionSourceSync + ?Sized, C: Deref<Target = T>> CoinSelectionSourceSync for C {

It uses ?Sized with a separate T parameter. The async version I wrote uses the C::Target associated type form instead:

impl<C: Deref> CoinSelectionSource for C
where
C::Target: CoinSelectionSource,
{

I switched to this form to work around the T may not live long enough error — with a separate T parameter, the compiler couldn't prove
T: 'a for the RPITIT return types. Using C::Target instead lets the compiler deduce the lifetime bound from &'a C (since C: 'a implies
C::Target: 'a through well-formedness of associated types).

But ?Sized doesn't actually need to be removed — Deref::Target is already ?Sized by default. The C::Target: CoinSelectionSource form
still allows unsized targets. So the two forms are equivalent in that regard; I just changed the spelling to work around the lifetime
inference issue.

TheBlueMatt · 2026-02-16T01:06:47Z

Argh, check_commits is showing fuzz build failures in intermediate commits. Can you make sure tests pass+fuzz builds after each commit?

In order to reuse CoinSelectionSource for splicing, the previous transaction of each UTXO is needed. Update CoinSelection to use FundingTxInput (renamed to ConfirmedUtxo) so that it is available. This requires adding a method to WalletSource to look up a previous transaction for a UTXO. Otherwise, Wallet's implementation of CoinSelectionSource would need WalletSource to include the previous transactions when listing confirmed UTXOs to select from. But this would be inefficient since only some UTXOs are selected.

CoinSelectionSource is used for anchor bumping where a ClaimId is passed in to avoid double spending other claims. To re-use this trait for funding a splice, the ClaimId must be optional. And, if None, then any locked UTXOs may be considered ineligible by an implementation.

Previously, splice_channel required callers to manually construct funding inputs and pass them directly, making coin selection the caller's responsibility. This made the API difficult to use and prevented reuse of the existing CoinSelectionSource trait. Introduce a two-phase API: splice_channel now returns a FundingTemplate that callers use to build a FundingContribution via wallet-backed splice methods (e.g., splice_in_sync, splice_out_sync), which handle coin selection automatically. The completed contribution is then passed to a new funding_contributed method to begin quiescence and negotiation. This also renames SpliceContribution to FundingContribution and moves fee estimation and input validation into the funding module, co-located with the types they operate on. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Now that CoinSelection is used to fund a splice funding transaction, use that for determining of a change output should be used. Previously, the initiator could either provide a change script upfront or let LDK generate one using SignerProvider::get_destination_script. Since older versions may have serialized a SpliceInstruction without a change script while waiting on quiescence, LDK must still generate a change output in this case.

Instead of logging both inside propose_quiescence and at the call site, only log inside it. This simplifies the return type.

jkczyz · 2026-02-17T17:45:01Z

Argh, check_commits is showing fuzz build failures in intermediate commits. Can you make sure tests pass+fuzz builds after each commit?

Sorry, I was limiting what claude ran and missed this. Fixed now along with the claim_id check. Will include the other fixes in the follow-up PR.

TheBlueMatt

obv needs ldk node update now.

jkczyz · 2026-02-18T00:01:12Z

All follow-ups are in #4382

jkczyz added this to Weekly Goals Dec 18, 2025

jkczyz self-assigned this Dec 18, 2025

jkczyz force-pushed the 2025-12-new-splice-api branch from f5933e5 to 854e9ca Compare January 12, 2026 17:47

wpaulino reviewed Jan 12, 2026

View reviewed changes

TheBlueMatt reviewed Jan 12, 2026

View reviewed changes

jkczyz requested review from TheBlueMatt and wpaulino January 13, 2026 00:28

jkczyz force-pushed the 2025-12-new-splice-api branch from 854e9ca to 6d78c3f Compare January 14, 2026 17:03

jkczyz force-pushed the 2025-12-new-splice-api branch from 6d78c3f to 94b1aa9 Compare January 15, 2026 17:02

jkczyz mentioned this pull request Jan 16, 2026

Mixed mode splicing #4261

Merged

jkczyz force-pushed the 2025-12-new-splice-api branch from 94b1aa9 to c3f3453 Compare January 20, 2026 19:16

jkczyz commented Jan 20, 2026

View reviewed changes

jkczyz marked this pull request as ready for review January 20, 2026 19:25

jkczyz force-pushed the 2025-12-new-splice-api branch from c3f3453 to 3253a99 Compare January 20, 2026 23:57

jkczyz force-pushed the 2025-12-new-splice-api branch 3 times, most recently from f2d9fa5 to fcccbac Compare January 21, 2026 18:22

TheBlueMatt reviewed Jan 21, 2026

View reviewed changes

jkczyz force-pushed the 2025-12-new-splice-api branch 2 times, most recently from 88ab4ca to 3c86404 Compare February 13, 2026 20:51

jkczyz changed the title ~~Add FundingNeeded event for splicing~~ Split splice initiation into two phases Feb 13, 2026

jkczyz force-pushed the 2025-12-new-splice-api branch from 3c86404 to 7e636ad Compare February 13, 2026 21:25

jkczyz force-pushed the 2025-12-new-splice-api branch from 7e636ad to 7abb73a Compare February 13, 2026 23:33

jkczyz force-pushed the 2025-12-new-splice-api branch from 7abb73a to 05067c5 Compare February 14, 2026 00:23

TheBlueMatt previously approved these changes Feb 16, 2026

View reviewed changes

jkczyz and others added 6 commits February 17, 2026 11:23

Run rustfmt on fuzz

df777e8

Consistently log in propose_quiescence

20916b7

Instead of logging both inside propose_quiescence and at the call site, only log inside it. This simplifies the return type.

jkczyz dismissed TheBlueMatt’s stale review via 20916b7 February 17, 2026 17:43

jkczyz force-pushed the 2025-12-new-splice-api branch from 05067c5 to 20916b7 Compare February 17, 2026 17:43

jkczyz requested a review from TheBlueMatt February 17, 2026 18:28

wpaulino approved these changes Feb 17, 2026

View reviewed changes

TheBlueMatt approved these changes Feb 17, 2026

View reviewed changes

TheBlueMatt merged commit 0a20fa5 into lightningdevkit:main Feb 17, 2026
17 of 31 checks passed

github-project-automation bot moved this to Done in Weekly Goals Feb 17, 2026

wpaulino mentioned this pull request Feb 17, 2026

Free holding cell in remaining quiescence-exit code paths #4415

Open

		// FIXME: Should claim_id be an Option?
		let claim_id = ClaimId([0; 32]);

Conversation

jkczyz commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-reviews-bot commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkczyz commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpaulino left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented Jan 15, 2026

Uh oh!

ldk-reviews-bot commented Jan 15, 2026

Uh oh!

ldk-reviews-bot commented Jan 17, 2026

Uh oh!

ldk-reviews-bot commented Jan 17, 2026

Uh oh!

ldk-reviews-bot commented Jan 19, 2026

Uh oh!

ldk-reviews-bot commented Jan 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented Jan 21, 2026

Uh oh!

ldk-reviews-bot commented Jan 21, 2026

Uh oh!

codecov bot commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkczyz commented Dec 18, 2025 •

edited

Loading

ldk-reviews-bot commented Dec 18, 2025 •

edited

Loading

jkczyz commented Jan 12, 2026 •

edited

Loading

codecov bot commented Jan 21, 2026 •

edited

Loading