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

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

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 · 2026-06-26T05:19:59Z

No longer based on #937 now that it dropped some commits affecting the pending payment store.

tnull

Some minor comments, please squash!

tnull · 2026-06-26T07:24:41Z

+
+		payment.kind =
+			PaymentKind::Onchain { txid: event_txid, status: confirmation_status, tx_type };
+		self.runtime.block_on(self.payment_store.insert_or_update(payment.clone()))?;


Fine for now, but I think I might open a follow-up to attempt to reduce the number of block_on calls.

tnull · 2026-06-26T07:34:43Z

+	/// For interactive funding (splices), this node's per-candidate funding figures across the
+	/// RBF history, keyed by each candidate's txid. Empty for non-funding payments and for
+	/// records written before per-candidate tracking existed.
+	pub(crate) candidates: Vec<FundingTxCandidate>,


Also fine for now, but given that PaymentDetails might also be non-onchain payment kinds, maybe we'd should only add these fields (also conflicting_txids) only to PendingPaymentDetails variants that they are actually relevant for. But as mentioned elsewhere (and on #950), reconsidering the type structure here can happen after this landed.

jkczyz

Addressed all requested fixes and squashed fixups. Will rebase and resolve merge conflicts next.

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>

`create_payment_from_tx` duplicated the amount/fee/direction derivation that `onchain_payment_fields` already performs. Share it via a helper that operates on the already-locked wallet, so both paths agree by construction. 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 a max_funding_feerate helper and keep it, alongside rbf_splice_feerates, in fee_estimator.rs so the splice funding-feerate policy lives in one place. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

In a splice, both channel parties broadcast the funding transaction, and the tests drive a single shared bitcoind, so the counterparty's broadcast can surface it to this node's wallet sync before this node's own funding classification has run. Under parallel test execution that classification can lag far enough behind for the sync to record the transaction as a plain on-chain payment, failing the funding-payment assertions. Wait for the funding broadcast to be classified before each affected splice test syncs its wallets. This is test-only: on a real node the classification runs locally, well ahead of a counterparty's broadcast arriving over the network, so the race does not occur. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

jkczyz · 2026-06-26T20:49:36Z

Rebased. Also added a commit to fix some test non-determinism. Since splices are interactively negotiated, both nodes broadcast the new funding transaction. Since the test nodes share a bitcoind instance, we can end up seeing the wallet event before persisting the pending data.

ldk-reviews-bot · 2026-06-27T01:40:32Z

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

tnull

Thanks! Going to land this, if we deem the codex comments worth we could still address them in a follow-up.

tnull · 2026-06-29T11:48:21Z

+	async fn persist_funding_payment(
+		&self, details: PaymentDetails, candidates: Vec<FundingTxCandidate>,
+	) -> Result<(), Error> {
+		self.payment_store.insert_or_update(details.clone()).await?;


Codex:

[P1] Late funding classification can downgrade confirmed payments: /home/tnull/workspace/ldk-node-pr-888-review/src/wallet/mod.rs:1346. persist_funding_payment always writes a fresh Pending/Unconfirmed record through insert_or_update. If wallet sync observes the tx before the broadcaster
classifies it, which the new test helper explicitly guards against, sync may already have recorded it as Confirmed or Succeeded; the later classification update then overwrites status/confirmation via PaymentDetails::update. Fix by merging only tx_type, candidate history, and funding
economics into an existing record while preserving any newer confirmation/top-level status.

tnull · 2026-06-29T11:51:30Z

+		PendingPaymentDetails::new(payment, conflicting_txids, Vec::new())
 	}

 	fn find_payment_by_txid(&self, target_txid: Txid) -> Option<PaymentId> {


Codex:

[P1] Graduated RBF splice payments cannot be mapped on later reorg: /home/tnull/workspace/ldk-node-pr-888-review/src/wallet/mod.rs:1420. After ChainTipChanged removes the pending entry, find_payment_by_txid only checks the pending store, so an event for a confirmed RBF candidate txid no
longer maps back to the original payment id. The fallback creates/updates PaymentId(event_txid), producing a duplicate generic on-chain payment instead of reverting the classified splice payment. Fix by looking up existing payment-store entries by on-chain txid, or keeping a durable txid-to-
payment-id index for classified funding/RBF records.

FWIW, this is somewhat bogus, but maybe we should ensure we fail early if the user tried to bump an already-confirmed (>6conf) splice? That might already be the case already, though?

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

jkczyz requested review from benthecarman and joostjager 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

jkczyz force-pushed the 2026-04-splicing-payment-using-tx-type branch from f2990d5 to b90b294 Compare June 26, 2026 05:18

jkczyz requested a review from tnull June 26, 2026 05:20

tnull reviewed Jun 26, 2026

View reviewed changes

jkczyz force-pushed the 2026-04-splicing-payment-using-tx-type branch from b90b294 to 7884706 Compare June 26, 2026 16:29

jkczyz commented Jun 26, 2026

View reviewed changes

jkczyz and others added 10 commits June 26, 2026 11:58

jkczyz force-pushed the 2026-04-splicing-payment-using-tx-type branch from 7884706 to 5d21bd7 Compare June 26, 2026 20:46

tnull reviewed Jun 29, 2026

View reviewed changes

tnull approved these changes Jun 29, 2026

View reviewed changes

tnull merged commit 3b296f2 into lightningdevkit:main Jun 29, 2026
23 of 25 checks passed

github-project-automation Bot moved this to Done in Weekly Goals Jun 29, 2026

tnull mentioned this pull request Jun 29, 2026

Classify onchain transactions #791

Open

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!

tnull commented Jun 25, 2026

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

jkczyz commented Jun 26, 2026

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tnull Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tnull Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

jkczyz commented Jun 26, 2026

Uh oh!

ldk-reviews-bot commented Jun 27, 2026

Uh oh!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

tnull Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

tnull Jun 29, 2026

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