Channel splicing support #677

jkczyz · 2025-10-23T21:19:16Z

LDK 0.2 added beta support for splicing. This PR exposes it in LDK Node as two new Node methods: splice_in and splice_out. Funds used for splicing in are taken from the on-chain BDK wallet. When splicing out, any on-chain address can be provided as the destination.

Two new events are provided: SplicePending and SpliceFailed. The former is emitted once the new funding transaction has been broadcast. The latter indicates that any contributed inputs may be re-used, though this is not currently exposed. It should be used internally to unlock UTXOs that were intended to be spent.

ldk-reviews-bot · 2025-10-23T21:19:19Z

👋 Thanks for assigning @wpaulino 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.

ldk-reviews-bot · 2025-10-27T00:00:55Z

🔔 1st Reminder

Hey @tnull @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 · 2025-10-27T00:00:56Z

🔔 1st Reminder

Hey @tnull @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 · 2025-10-27T00:00:58Z

🔔 1st Reminder

Hey @tnull @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.

tnull

Thanks!

Looks mostly good, just a few comments. This will need bindings support, too, and unfortunately also need a rebase by now. Sorry!

src/event.rs

src/wallet/mod.rs

tnull · 2025-10-28T12:27:16Z

src/lib.rs

+		} else {
+			log_error!(
+				self.logger,
+				"Channel not found for user_channel_id: {:?} and counterparty: {}",


Pre-existing, but we should probably implement Display for UserChannelId, now opened #681

src/lib.rs

src/wallet/mod.rs

tnull · 2025-10-28T12:42:57Z

src/wallet/mod.rs

 		Ok(txid)
 	}

+	pub(crate) fn select_confirmed_utxos(


Would be cool to be able to reuse this for impl CoinSelectionSource while dropping impl WalletSource going forward. What do we think the necessary changes would be that would allow for this? Probably just making claim_id optional?

cc: @wpaulino

Hmm... are you saying have LDK Node's Wallet implement CoinSelectionSource directly? Would we implement UTXO locking there? Or are you assuming BDK's wallet will eventually implement this?

To re-use this code, yeah we can extract the Utxo from each FundingTxInput to use in CoinSelection. Though we need to expose that or have a conversion function. Currently, FundingTxInput::utxo is pub(super).

Hmm... are you saying have LDK Node's Wallet implement CoinSelectionSource directly?

Yes, it might be a bit more efficient than the detour over WalletSource, and would allow us to reuse most code, IIUC.

Would we implement UTXO locking there? Or are you assuming BDK's wallet will eventually implement this?

We could, although I still prefer to wait until BDK ships it soon hopefully. Might be debatable if we need to implement it manually in the meantime. However, the UTXO locking discussion seems orthogonal to whether or not to use CoinSelectionSource, no?

We could, although I still prefer to wait until BDK ships it soon hopefully. Might be debatable if we need to implement it manually in the meantime. However, the UTXO locking discussion seems orthogonal to whether or not to use CoinSelectionSource, no?

IIUC, seems we'd replace the lightning::events::bump_transaction::Wallet parameterization in BumpTransactionEventHandler with LDK Node's Wallet. But the former has some sort of locking already. Or at least it prefers not to double spend when possible.

https://github.com/lightningdevkit/rust-lightning/blob/17f7858f01f29ec904c5bddda799365f4f9ff71a/lightning/src/events/bump_transaction/mod.rs#L558-L559

We may be able to maintain a list of OutPoints to pass to TxBuilder::unspendable (filtered by claim id), attempting with and without the list.

We may be able to main a list of OutPoints to pass to TxBuilder::unspendable (filtered by claim id), attempting with and without the list.

Yeah, though note that would require us to track a list of locked OutPoints whenever we generate any Transaction. We'd then need to drop entries whenever they are spent and re-add them in case of a reorg, etc. This might become very complicated real quick, if we want to do it 'properly'.

I think an alternative approach would be to just always act as if we already saw the transaction in the mempool, i.e., call Wallet::apply_unconfirmed_txs, and then the Wallet should do the right thing and eventually mark the transaction as evicted if it wouldn't end up in the mempool afterall.

Hmm, maybe it would even make sense to go with the last described approach in this PR (also for non-splicing funding transactions). If we do, we might however finally need to handle DiscardFunding, too.

I think an alternative approach would be to just always act as if we already saw the transaction in the mempool, i.e., call Wallet::apply_unconfirmed_txs, and then the Wallet should do the right thing and eventually mark the transaction as evicted if it wouldn't end up in the mempool afterall.

Hmmm... would we do this when CoinSelectionSource::sign_psbt is called? We don't have the transaction in CoinSelectionSource::select_confirmed_utxos.

Hmm, maybe it would even make sense to go with the last described approach in this PR (also for non-splicing funding transactions). If we do, we might however finally need to handle DiscardFunding, too.

Which approach?

src/lib.rs

tnull · 2025-10-28T12:51:30Z

src/config.rs

+	/// users should be aware of the backwards compatibility risk prior to using the functionality.
+	///
+	/// [`Node`]: crate::Node
+	pub reject_inbound_splices: bool,


IIUC, contrary to the docs here and in LDK, this flag is necessary to still support downgrades, i.e., forwards, not backwards compatibility (we might want to fix this/make this more clear in the LDK docs).

However, we currently don't support downgrades in LDK Node anyways, so here we can just omit this flag.

I think CI is still failing because reject_inbound_splices is still here even though it hasn't been added to the bindings.

Right, I'm not sure how consistent we've been with using forward and backward compatibility correctly. For instance, IIUC, should this have said backward compatibility?

https://github.com/lightningdevkit/rust-lightning/blob/17f7858f01f29ec904c5bddda799365f4f9ff71a/lightning/src/chain/channelmonitor.rs#L4522-L4523

And these like reject_inbound_splices should have said forward compatibility?

https://github.com/lightningdevkit/rust-lightning/blob/17f7858f01f29ec904c5bddda799365f4f9ff71a/lightning/src/util/config.rs#L915
https://github.com/lightningdevkit/rust-lightning/blob/17f7858f01f29ec904c5bddda799365f4f9ff71a/lightning/src/util/config.rs#L941

Maybe it would be best to avoid "backward" / "forward" and just say "compatibility" when referencing an earlier version. Seems these could be confusing if it is unclear if the term is relative to older or newer version.

I think CI is still failing because reject_inbound_splices is still here even though it hasn't been added to the bindings.

Yup, gonna drop the commit, so that will fix CI.

should this have said backward compatibility

I think so, yes.

And these like reject_inbound_splices should have said forward compatibility?

Yes, I think so, too.

Maybe it would be best to avoid "backward" / "forward" and just say "compatibility" when referencing an earlier version. Seems these could be confusing if it is unclear if the term is relative to older or newer version.

Yeah, that sounds reasonable. Seems there often is confusion around it and 'compatibility with versions prior' is much more precise anyways.

tests/integration_tests_rust.rs

tnull · 2025-10-28T12:53:58Z

tests/integration_tests_rust.rs


+#[test]
+fn splice_channel() {
+	macro_rules! expect_splice_pending_event {


nit: Usually we'd add this to common/mod.rs.

Since it wasn't being used there, having it there would give a build warning.

That's a good point about the build warning.

To resolve that and utilize the macro more broadly, what do you think about adding a test for splice-in/splice-out assertions into do_channel_full_cycle? We could use the new expect_splice_pending_event! macro there, which would justify having it in common/mod.rs and make our full-cycle test more robust for splicing.

Started looking into this. @tnull The channel funding transaction is recognized as an outbound on-chain payment for A -- presumably because we are spending from the wallet. If B were to splice out paying A (or whomever), should the splice funding transaction be considered an outbound on-chain payment for B? It does not currently since it spends from the funding txo.

Started looking into this. @tnull The channel funding transaction is recognized as an outbound on-chain payment for A -- presumably because we are spending from the wallet. If B were to splice out paying A (or whomever), should the splice funding transaction be considered an outbound on-chain payment for B? It does not currently since it spends from the funding txo.

Hmm, yeah, unfortunately that will be the case prior to 0.3 when we get lightningdevkit/rust-lightning#3566. I think having splice-out counting as an outbound onchain transaction makes sense until then. Might be worth noting somewhere that this is the case though, and that PaymentKinds will change in the future. (Tbh. one reason why I think it would make sense to flag splicing support alpha/experimental, simply as the classification API is unstable currently).

Hmm... so currently we don't have the Txid until we see an LdkEvent::FundingTransactionReadyForSigning, but we've lost the context of the transaction at that point. We can assume it is a splice (could be dual-funding later), but it might difficult to determine if it is a splice-out. Maybe simply seeing that none of the inputs belong to our wallet would be sufficient? We should only see the event if we have any contributions / are the initiator, IIRC.

Hmm... so currently we don't have the Txid until we see an LdkEvent::FundingTransactionReadyForSigning, but we've lost the context of the transaction at that point. We can assume it is a splice (could be dual-funding later), but it might difficult to determine if it is a splice-out. Maybe simply seeing that none of the inputs belong to our wallet would be sufficient? We should only see the event if we have any contributions / are the initiator, IIRC.

Okay. That makes me wonder even more if we should wait for LDK#3566 above to be able to classify them properly.

Hmm, maybe let's just stick with adding splice-ins (which will be marked outbound payments) for now, while noting this behavior on the splice_in/splice_out methods under an 'Experimental API' section or similiar?

jkczyz

Need to rebase still.

src/wallet/mod.rs

src/lib.rs

src/wallet/mod.rs

jkczyz · 2025-10-28T23:45:12Z

tests/integration_tests_rust.rs


+#[test]
+fn splice_channel() {
+	macro_rules! expect_splice_pending_event {


Since it wasn't being used there, having it there would give a build warning.

ldk-reviews-bot · 2025-10-29T00:01:48Z

🔔 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 · 2025-10-29T00:01:51Z

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

When a channel is spliced, the existing funding transaction's output is spent and a new funding transaction output is formed. Once the splice is considered locked by both parties, LDK will emit a ChannelReady event which will include the new funding_txo. Additionally, the initial ChannelReady event now includes the original funding_txo. Include this data in LDK Node's ChannelReady event.

LDK introduced similar events with splicing. SplicePending is largely informational like ChannelPending. SpliceFailed indicates the used UTXOs can be reclaimed. This requires UTXO locking, which is not yet implemented.

When the interactive-tx construction protocol completes in LDK during splicing (and in the future dual-funding), LDK Node must provide signatures for any non-shared inputs belonging to its on-chain wallet. This commit implements this when handling the corresponding FundingTransactionReadyForSigning event.

Extract the funds availability checking logic from open_channel_inner into a separate method so that it can be reused for channel splicing.

Instead of closing and re-opening a channel when outbound liquidity is exhausted, splicing allows to adding more funds (splice-in) while keeping the channel operational. This commit implements splice-in using funds from the BDK on-chain wallet.

Instead of closing and re-opening a channel when on-chain funds are needed, splicing allows removing funds (splice-out) while keeping the channel operational. This commit implements splice-out sending funds to a user-provided on-chain address.

chuksys

I'm really excited to see Splicing being added to LDK-Node - this is a fantastic feature! I learned a lot reviewing this PR.

I've left two minor, non-blocking comments regarding the CI failure and a test organization suggestion (to utilize the new macro in do_channel_full_cycle).

Great work!

chuksys · 2025-10-29T15:47:21Z

src/config.rs

+	/// users should be aware of the backwards compatibility risk prior to using the functionality.
+	///
+	/// [`Node`]: crate::Node
+	pub reject_inbound_splices: bool,


I think CI is still failing because reject_inbound_splices is still here even though it hasn't been added to the bindings.

chuksys · 2025-10-29T16:30:56Z

tests/integration_tests_rust.rs


+#[test]
+fn splice_channel() {
+	macro_rules! expect_splice_pending_event {


That's a good point about the build warning.

To resolve that and utilize the macro more broadly, what do you think about adding a test for splice-in/splice-out assertions into do_channel_full_cycle? We could use the new expect_splice_pending_event! macro there, which would justify having it in common/mod.rs and make our full-cycle test more robust for splicing.

Since LDK Node does not support downgrades, there's no need to have a Config parameter for accepting inbound splices. Instead, enable it by default.

jkczyz requested review from TheBlueMatt, tnull and wpaulino October 23, 2025 21:19

tnull reviewed Oct 28, 2025

View reviewed changes

jkczyz force-pushed the 2025-10-splicing branch from 982ce34 to 57ad1c8 Compare October 28, 2025 23:45

jkczyz commented Oct 28, 2025

View reviewed changes

jkczyz added 14 commits October 28, 2025 22:38

f - bindings

4130869

Add SplicePending and SpiceFailed events

85f3b3d

LDK introduced similar events with splicing. SplicePending is largely informational like ChannelPending. SpliceFailed indicates the used UTXOs can be reclaimed. This requires UTXO locking, which is not yet implemented.

f - bindings

e8cf672

f - extract_tx

4b66592

Refactor funds checking logic into reusable method

da0c4d8

Extract the funds availability checking logic from open_channel_inner into a separate method so that it can be reused for channel splicing.

Add Node::splice_in method

8ac745e

Instead of closing and re-opening a channel when outbound liquidity is exhausted, splicing allows to adding more funds (splice-in) while keeping the channel operational. This commit implements splice-in using funds from the BDK on-chain wallet.

f - import

3e654f1

f - fee_estimator clean-up

62164ae

f - bindings

53cc88d

Add Node::splice_out method

235ebee

Instead of closing and re-opening a channel when on-chain funds are needed, splicing allows removing funds (splice-out) while keeping the channel operational. This commit implements splice-out sending funds to a user-provided on-chain address.

f - fee_estimator clean-up

915fe79

f - bindings

549985a

jkczyz force-pushed the 2025-10-splicing branch from 57ad1c8 to 281ebce Compare October 29, 2025 03:49

chuksys reviewed Oct 29, 2025

View reviewed changes

jkczyz added 2 commits October 29, 2025 17:20

Accept inbound splice attempts

791a22f

Since LDK Node does not support downgrades, there's no need to have a Config parameter for accepting inbound splices. Instead, enable it by default.

Add an integration test for splicing

6b963fe

f - WIP test splicing in do_channel_full_cycle

adb59c8

jkczyz force-pushed the 2025-10-splicing branch from 281ebce to adb59c8 Compare October 29, 2025 22:23

jkczyz added this to Weekly Goals Oct 30, 2025

jkczyz self-assigned this Oct 30, 2025

Channel splicing support #677

Are you sure you want to change the base?

Channel splicing support #677

Conversation

jkczyz commented Oct 23, 2025

Uh oh!

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

Uh oh!

Uh oh!

ldk-reviews-bot commented Oct 27, 2025

Uh oh!

ldk-reviews-bot commented Oct 27, 2025

Uh oh!

ldk-reviews-bot commented Oct 27, 2025

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

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

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!

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!

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!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented Oct 29, 2025

Uh oh!

ldk-reviews-bot commented Oct 29, 2025

ldk-reviews-bot commented Oct 23, 2025 •

edited

Loading

jkczyz Oct 29, 2025 •

edited

Loading