Add splice RBF support by jkczyz · Pull Request #888 · lightningdevkit/ldk-node

jkczyz · 2026-04-24T02:52:02Z

Adds a public Node::rbf_channel API that replaces an in-flight splice transaction with a higher-feerate version.

An RBF leaves multiple candidate splice transactions in flight; only one of them will confirm on chain, and the payment record needs to reflect whichever one did — its amount and this node's share of the fee. The transition from Pending to Succeeded should also match when the Lightning protocol actually considers the new channel state usable — the moment ChannelReady fires — rather than after ANTI_REORG_DELAY confirmations, which doesn't fit the zero-conf extreme on one end or higher-confirmation-depth peer configurations on the other.

To support those properties, this PR:

ties channel-funding and splice payment status to channel lifecycle instead of confirmation depth;
retains each candidate's contribution on the pending record until one of them confirms;
updates the record's amount and fee to match the confirmed candidate's contribution;
discards a funding payment when its channel closes without the funding ever confirming.

Every broadcast from LDK now triggers a store write. For a remote KV store, running that write on LDK's thread would block message handling for hundreds of milliseconds per call, so persistence happens asynchronously while still guaranteeing that the record is committed before the transaction reaches the chain client.

Based on #878.

ldk-reviews-bot · 2026-04-24T02:52:05Z

👋 I see @joostjager was un-assigned.
If you'd like another reviewer assignment, please click here.

jkczyz · 2026-04-24T02:53:37Z

@tnull Any feedback on the last few commits would be appreciated: https://github.com/lightningdevkit/ldk-node/pull/888/changes/492ec9124ae02a4048ecb5270309174b05b366d2..c770999887e1287bef8abe4f4c92ca36e9434067

tnull · 2026-04-29T08:43:42Z

Btw, we just landed #839 - maybe it would be nice to add splicing interop tests with CLN and/or Eclair (see #880 though), in this PR or in a follow-up?

tnull

Excuse the delay. Took a first look, generally makes sense, though it's kind off odd to block the broadcast queue on persistence. Hopefully we'd never get a lot of backpressure from remote persistence, for example.

I have yet to review the (rather verbose) new code for updating/persisting the pending payments in wallet.rs in more detail.

tnull · 2026-05-21T08:10:54Z

@jkczyz Is this ready for review or are we waiting for any upstream changes still?

jkczyz · 2026-05-21T14:39:23Z

@jkczyz Is this ready for review or are we waiting for any upstream changes still?

Yeah, upstream has landed but this is now based on the splicing portion of #882. We can take those here if you prefer.

tnull · 2026-05-21T14:54:14Z

@jkczyz Is this ready for review or are we waiting for any upstream changes still?

Yeah, upstream has landed but this is now based on the splicing portion of #882. We can take those here if you prefer.

Ah, was unaware of that cross dependency, will see to review #882 next to keep things moving then.

ldk-reviews-bot · 2026-05-23T14:40:53Z

🔔 1st Reminder

Hey @tankyleo! 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-05-25T14:40:57Z

🔔 2nd Reminder

Hey @tankyleo! 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-05-27T14:41:41Z

🔔 3rd Reminder

Hey @tankyleo! 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.

Camillarhi

Thanks for this. I took a look at this, and I have a few questions. I have taken a little look at splicing in general.
For this PR, if a user calls bump_fee_rbf on a record with funding_details, the replacement is picked up as a WalletEvent::TxReplaced event on the next sync, and this will currently rewrite the funding_details as NONE. Is bumping a funding/splice payment through bump_fee_rbf meant to be possible at all, or should those records be excluded here in favor of rbf_channel? AND is overwriting an existing funding_details with None intended in the TxReplaced path?

ldk-reviews-bot · 2026-05-30T00:01:35Z

🔔 4th Reminder

Hey @tankyleo! 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-06-01T00:02:16Z

🔔 5th Reminder

Hey @tankyleo! 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-06-03T00:02:47Z

🔔 6th Reminder

Hey @tankyleo! 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-06-05T19:46:08Z

Rebased and dropped commits added in #912.

Ah, was unaware of that cross dependency, will see to review #882 next to keep things moving then.

Sorry, that should have said #872. Relevant commits were moved to #912, which has been merged.

Thanks for this. I took a look at this, and I have a few questions. I have taken a little look at splicing in general. For this PR, if a user calls bump_fee_rbf on a record with funding_details, the replacement is picked up as a WalletEvent::TxReplaced event on the next sync, and this will currently rewrite the funding_details as NONE. Is bumping a funding/splice payment through bump_fee_rbf meant to be possible at all, or should those records be excluded here in favor of rbf_channel? AND is overwriting an existing funding_details with None intended in the TxReplaced path?

Good catch @Camillarhi! Updated to disallow using bump_fee_rbf since that won't work for interactively constructed transactions. We need use the interactive-tx construction protocol given some signatures come from the counterparty.

Overwriting the existing funding_details is also unintended. Added a guard to prevent that.

ldk-reviews-bot · 2026-06-06T00:01:15Z

🔔 7th Reminder

Hey @tankyleo! 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-06-08T00:02:06Z

🔔 8th Reminder

Hey @tankyleo! 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-06-20T00:00:42Z

🔔 2nd Reminder

Hey @benthecarman! 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-06-22T00:01:51Z

🔔 3rd Reminder

Hey @benthecarman! 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.

benthecarman · 2026-06-22T21:44:52Z

+				Some((PaymentDirection::Inbound, amt)) => {
+					amount_inbound = amount_inbound.saturating_add(amt);
+				},
+				None => {},


Should we debug_assert here? Seems like a state we shouldn't be hitting. If not at least a comment would be good

Currently, I don't think we can hit this since we don't allow mixed mode. But once LDK supports batched splices, we could run into this. For now, simplified this quite a bit by looking at FundingContribution::net_value and removing this case.

benthecarman · 2026-06-22T21:51:42Z

+					self.persist_pending_payment(pending_payment)?;
 				},
 				WalletEvent::TxReplaced { txid, conflicts, .. } => {
 					let Some(payment_id) = self.find_payment_by_txid(txid) else {


claude:

a graduated funding payment can be resurrected into the pending store, tripping the
ChainTipChanged debug_assert (PLAUSIBLE)
find_payment_by_txid now also matches the main store by Onchain txid. After
graduation, the record is gone from the pending store, so the TxReplaced guard
pending_payment_store.get(id).map_or(false, |p| p.funding_details.is_some()) is
false. A TxReplaced for the confirmed funding txid (a reorg evicting it while a
conflict exists) therefore falls through to create_pending_payment_from_tx, which
copies status = Succeeded, funding_details = None, and re-inserts it into the pending
store. The next ChainTipChanged sweep then hits
debug_assert!(status == Pending, "Non-pending payment ... found in pending store") and,
in release, the Succeeded record lingers forever. The TxReplaced arm is the only
wallet-event arm that doesn't call apply_funding_details_status_update first — mirroring
the other three arms would close the gap.

benthecarman · 2026-06-22T21:52:32Z

+		self.runtime.block_on(self.payment_store.insert_or_update(pending.details.clone()))?;
+		self.runtime.block_on(self.pending_payment_store.insert_or_update(pending))?;


would it be better to do a join on these futures and then block on?

Hmmm... if the payment_store update fails, are we ok with the pending_payment_store completing? Given this called in places where we simply log an error and continue, I wonder what the downstream consequences of that are?

Also, IIUC, if persistence fails we'll have an in-memory representation that is out-of-sync with the persisted representation, whether or not these are done sequentially.

Further, a failure here would prevent us from broadcasting the transaction, but our counterparty may still do so.

On-chain payment records don't capture what a transaction was for -- a channel open, splice, close, sweep, or a plain send. Record that classification on each on-chain payment, derived from the type LDK reports when broadcasting the transaction, so it survives restarts alongside the payment. The tag keeps only which channels a transaction relates to; amounts and fees stay on the payment. Existing records keep decoding unchanged. Compatible with the on-chain transaction classification proposed in lightningdevkit#791. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Record channel-open and splice funding transactions as on-chain payments at broadcast, and carry them to Succeeded through ANTI_REORG_DELAY confirmations like any other on-chain payment, instead of tying their status to the Lightning channel lifecycle. A splice's recorded amount and fee are this node's share of the funding contribution, which wallet sync preserves rather than overwriting with its own view of the (possibly multi-party) transaction. On-chain RBF of these payments is rejected: LDK drives funding and splice transactions, so replacing one would broadcast a transaction it isn't tracking and, for a splice, can't re-sign. Addresses review feedback to keep on-chain payment status confirmation- driven rather than gated on ChannelReady. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

A splice's funding transaction can be stuck at too low a fee rate with no way to raise it: on-chain RBF is rejected for funding transactions, and re-issuing splice_in / splice_out errors while a splice is already pending. Add bump_channel_funding_fee, which replaces the pending splice's funding transaction at a higher fee rate while preserving its amount and destination, and point the "a prior splice contribution is pending" errors at it. Replacing the transaction also requires signing a funding input the wallet already treats as spent by the splice being replaced, which it would otherwise skip after syncing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Cover the wallet-event-driven funding payment lifecycle end to end: a channel-open funding payment reaches Succeeded from wallet sync alone, asserted before any ChannelReady event is drained to show payment status no longer depends on the channel-ready signal; and a splice fee-bumped via RBF stays a single on-chain payment that follows the winning candidate while keeping its interactive-funding classification across the replacement. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

A splice funding payment can be fee-bumped via RBF, producing several candidate transactions with increasing fees. The payment recorded the last-broadcast candidate's amount and fee and kept them on confirmation, but the candidate that actually confirms need not be the last one broadcast — so an earlier, lower-fee candidate confirming left the payment over-reporting its fee. Record each candidate's amount and fee, keyed by txid, so that on confirmation the payment reflects the candidate that actually confirmed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

splice_channel only checked the splice-out fee; also assert it is recorded as a confirmed interactive-funding payment. Add a test that a confirmed splice payment returns to unconfirmed when its block is reorged out, exercising the unconfirm path for funding payments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Contributing to an already-pending splice — e.g. adding our funds to a counterparty-initiated splice via splice_in or splice_out — replaces the in-flight funding transaction, so the funding template requires at least the RBF minimum feerate. We passed our plain ChannelFunding feerate estimate, which can sit below that minimum (it does at the regtest floor), so the contribution was rejected with FeeRateBelowRbfMinimum. Raise the contribution feerate to the template's RBF minimum when one applies, capped by our max, so it can replace the pending splice. A node can therefore now contribute to a counterparty's pending splice; the rbf_splice_channel check that expected splice_out to fail while a splice was pending relied on this very bug and is dropped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

The 1.5x-of-estimate funding feerate ceiling was open-coded identically in splice_in and splice_out. Route both through the max_funding_feerate helper so the ceiling lives in one place. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

jkczyz

Now based on #937.

jkczyz · 2026-06-24T20:02:49Z

+		if let Some(channel_details) =
+			open_channels.iter().find(|c| c.user_channel_id == user_channel_id.0)
+		{
+			let min_feerate =


Yeah, good catch. Done.

jkczyz · 2026-06-24T20:29:41Z

+	/// # Experimental API
+	///
+	/// This API is experimental and may change in the future.
+	pub fn bump_channel_funding_fee(


Note that we currently limit how much we'll pay as an acceptor (i.e., if we lose quiescence) to 1.5x the channel funding fee rate. This is max_feerate below. The minimum rate we use here is the least the spec requires us to bump. And we need to make sure to use our min_feereate if it is above the spec-compliant fee rate. But should we fail if this is above max_feerate?

I'd imagine yes as anything above that means we'd be overpaying based on our fee estimator. Unless of course we want to allow users to target something quicker than 12 blocks (ChannelFunding) like OnchainPayment's 6 blocks, if they are using splice-out to make a payment, for instance.

jkczyz · 2026-06-24T20:50:31Z

+	///
+	/// # Experimental API
+	///
+	/// This API is experimental and may change in the future.


Yeah, dropped. There's similar notes on the other splicing methods, but they mention payment direction.

jkczyz · 2026-06-24T21:13:01Z

+				Some((PaymentDirection::Inbound, amt)) => {
+					amount_inbound = amount_inbound.saturating_add(amt);
+				},
+				None => {},


Currently, I don't think we can hit this since we don't allow mixed mode. But once LDK supports batched splices, we could run into this. For now, simplified this quite a bit by looking at FundingContribution::net_value and removing this case.

jkczyz · 2026-06-24T21:48:31Z

+		// Skip broadcasts that don't move funds in or out of our on-chain wallet — e.g.
+		// a splice-out we initiated toward an external address.


@tnull Do we skip because this wouldn't be considered "on-chain"? That is, we could be paying someone from our lightning balance?

It seems that code you're referring to is no longer there? Not sure I'm understanding the context - could you reiterate the question?

Ah, it should be this: https://github.com/jkczyz/ldk-node/blob/f2990d57329e23016708af857b985f7fcba82b03/src/wallet/mod.rs#L1314-L1326

Basically, a splice-out to an address that is not ours.

jkczyz · 2026-06-24T23:32:34Z

+	) -> Result<BroadcastPackage, Error> {
+		let wallet_opt = self.wallet.lock().expect("lock").as_ref().and_then(Weak::upgrade);
+		if let Some(wallet) = wallet_opt {
+			tokio::task::spawn_blocking(move || {


Yeah, just had to make all the other methods it calls async.

jkczyz · 2026-06-25T01:09:27Z

 	pub conflicting_txids: Vec<Txid>,
+	/// Set when the payment's transaction is an interactive-funding broadcast (channel
+	/// open or splice). The record transitions to [`PaymentStatus::Succeeded`] on
+	/// `ChannelReady` instead of after [`ANTI_REORG_DELAY`] confirmations.


Discussed offline. Abandoned this approach to avoid tying payment status with channel lifecycle.

jkczyz · 2026-06-25T01:20:56Z

+		self.runtime.block_on(self.payment_store.insert_or_update(pending.details.clone()))?;
+		self.runtime.block_on(self.pending_payment_store.insert_or_update(pending))?;


Hmmm... if the payment_store update fails, are we ok with the pending_payment_store completing? Given this called in places where we simply log an error and continue, I wonder what the downstream consequences of that are?

Also, IIUC, if persistence fails we'll have an in-memory representation that is out-of-sync with the persisted representation, whether or not these are done sequentially.

Further, a failure here would prevent us from broadcasting the transaction, but our counterparty may still do so.

jkczyz · 2026-06-25T01:32:09Z

+			pending.details.fee_paid_msat = aggregate.fee_paid_msat;
+		}
+
+		// Preserve the confirmation status already on the record (set by wallet sync if


I think this may be obsolete now.

tnull · 2026-06-25T08:10:56Z

Now based on #937.

Note I now dropped two commits from #937 to keep PaymentDetails around for the time being (tbd whether we can still refactor it out post-#948), but shouldn't make too much of a difference approach-wise I hope?

tnull

Kotlin tests are failing right now.

jkczyz force-pushed the 2026-04-splicing-payment-using-tx-type branch 2 times, most recently from 198bfbf to d5b6bda Compare April 28, 2026 16:14

jkczyz force-pushed the 2026-04-splicing-payment-using-tx-type branch from d5b6bda to 91166b9 Compare April 29, 2026 22:28

jkczyz mentioned this pull request Apr 29, 2026

Include interactive funding candidates on broadcast lightningdevkit/rust-lightning#4570

Merged

jkczyz self-assigned this Apr 30, 2026

jkczyz added this to Weekly Goals Apr 30, 2026

tnull reviewed May 5, 2026

View reviewed changes

Comment thread src/lib.rs Outdated

ca-ruz mentioned this pull request May 16, 2026

Topic Suggestions for Socratic Seminar #21 guadalajara-bitdevs/BitDevsGDL.github.io#66

Closed

jkczyz mentioned this pull request May 18, 2026

Safely migrate to FileSystemStoreV2 #872

Merged

jkczyz force-pushed the 2026-04-splicing-payment-using-tx-type branch from 91166b9 to c4d3282 Compare May 20, 2026 21:00

jkczyz marked this pull request as ready for review May 21, 2026 14:39

ldk-reviews-bot requested a review from tankyleo May 21, 2026 14:40

Camillarhi reviewed May 28, 2026

View reviewed changes

Comment thread src/wallet/mod.rs Outdated

jkczyz force-pushed the 2026-04-splicing-payment-using-tx-type branch from c4d3282 to bec7723 Compare June 5, 2026 19:46

jkczyz requested a review from tnull June 8, 2026 14:49

benthecarman reviewed Jun 22, 2026

View reviewed changes

jkczyz mentioned this pull request Jun 23, 2026

Bump BDK, address pending payment store follow ups #937

Open

tnull reviewed Jun 23, 2026

View reviewed changes

Comment thread src/tx_broadcaster.rs Outdated

tnull mentioned this pull request Jun 23, 2026

Zero-fee commitments support #660

Open

jkczyz mentioned this pull request Jun 23, 2026

Add persistent closed channel history and list_closed_channels() #882

Open

jkczyz and others added 12 commits June 24, 2026 18:10

f - sum contribution net values to classify the funding direction

dc6b89f

f - clarify the wallet-involvement skip comment

4da7335

f - log that we skip the payment record, not the broadcast

737b4b2

f - make BroadcastPackage a struct built from a single broadcast call

02575ed

f - select the bump feerate from the current estimate and RBF minimum

b6f32aa

jkczyz mentioned this pull request Jun 25, 2026

Add a per-channel record store #946

Open

jkczyz force-pushed the 2026-04-splicing-payment-using-tx-type branch from 328bdc3 to f2990d5 Compare June 25, 2026 01:38

jkczyz commented Jun 25, 2026

View reviewed changes

jkczyz requested review from benthecarman, joostjager and tnull June 25, 2026 01:39

tnull reviewed Jun 25, 2026

View reviewed changes

tnull mentioned this pull request Jun 25, 2026

Consider simplifying/refactoring PendingPaymentStore #950

Open

joostjager removed their request for review June 25, 2026 09:06

		self.runtime.block_on(self.payment_store.insert_or_update(pending.details.clone()))?;
		self.runtime.block_on(self.pending_payment_store.insert_or_update(pending))?;

		// Skip broadcasts that don't move funds in or out of our on-chain wallet — e.g.
		// a splice-out we initiated toward an external address.

Uh oh!

Conversation

jkczyz commented Apr 24, 2026

Uh oh!

ldk-reviews-bot commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkczyz commented Apr 24, 2026

Uh oh!

tnull commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tnull commented May 21, 2026

Uh oh!

jkczyz commented May 21, 2026

Uh oh!

tnull commented May 21, 2026

Uh oh!

ldk-reviews-bot commented May 23, 2026

Uh oh!

ldk-reviews-bot commented May 25, 2026

Uh oh!

ldk-reviews-bot commented May 27, 2026

Uh oh!

Camillarhi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ldk-reviews-bot commented May 30, 2026

Uh oh!

ldk-reviews-bot commented Jun 1, 2026

Uh oh!

ldk-reviews-bot commented Jun 3, 2026

Uh oh!

jkczyz commented Jun 5, 2026

Uh oh!

ldk-reviews-bot commented Jun 6, 2026

Uh oh!

ldk-reviews-bot commented Jun 8, 2026

Uh oh!

ldk-reviews-bot commented Jun 20, 2026

Uh oh!

ldk-reviews-bot commented Jun 22, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jkczyz 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!

ldk-reviews-bot commented Apr 24, 2026 •

edited

Loading

tnull commented Apr 29, 2026 •

edited

Loading