Keep funding payment records accurate by jkczyz · Pull Request #1057 · lightningdevkit/ldk-node

jkczyz · 2026-08-19T02:26:11Z

Five bugfixes for funding payment records (channel opens and splices). Found while building the splice-retry work stacked on top (#930's replacement) but independent of it.

Only adopt a funding payment's own transactions from wallet sync. Sync adopted the txid and confirmation of any transaction linked to a funding record through its conflicting txids. A cooperative close conflicts with a pending splice in exactly that way, so the splice record could adopt the close's confirmation and graduate as if the splice had confirmed.
Fail funding payments lost to a confirmed conflict. With the close's confirmation no longer adopted, a funding payment whose transaction was double-spent stayed Pending forever. It is now marked Failed once a conflict outside its candidate history has confirmed through ANTI_REORG_DELAY while neither its own transaction nor any RBF candidate can still confirm.
Retry funding-broadcast classification instead of dropping it. A broadcast whose payment-record classification failed was dropped. For interactive funding the counterparty broadcasts the same transaction anyway, so the drop keeps nothing off-chain — it just leaves the round unrecorded, permanently stranding its confirmation on a duplicate record. Classification is now retried, with the broadcast held back, until it succeeds or the node shuts down. Since splice rounds are now recorded at signing (below), their broadcast writes nothing and is never queued; the retry serves v1 channel opens, which are still recorded at broadcast, and the broadcaster's other record writes.
Record splice funding payments when signing. Recording a splice round only when it is broadcast races wallet sync: once tx_signatures are exchanged the counterparty may broadcast first, and sync then files the round under a duplicate record that shadows the funding record from then on. Writing the record while handling FundingTransactionReadyForSigning, before our signatures leave the node, avoids the race. A round recorded that early can still be abandoned before broadcast, so such rounds are dropped once LDK no longer holds them.
Resolve funding payments when LDK discards a splice round. A round of ours that LDK gives up on — kept through a close the monitor watched until it matured, or replaced by a sibling round we did not contribute to — stayed Pending forever, because the DiscardFunding handler only reclaimed the contribution's addresses. The event names this node's contribution rather than the round, so the payments are resolved from what LDK holds instead. As the promoted round's ChannelReady is handled, every funding payment of the channel left with no round of ours among the rounds the channel manager still holds, and none promoted before, is failed. ChannelClosed fails the payments a close leaves with no round of ours the channel's monitor still watches and none promoted before, and a DiscardFunding for a channel the manager no longer lists resolves them the same way from the monitor's funding and watched rounds. Each payment records the rounds LDK promoted to the funding, so a zero-conf round promoted once still counts at later discards and at the close. A DiscardFunding for a listed channel only drops a round nothing broadcast and reclaims the contribution's addresses.

Each fix has a test that fails without it; the commit messages have the details.

First of three stacked PRs replacing #930's restart persistence for this release, per the discussion there; #1079 (payment-model groundwork) and #1080 (in-flight splice tracking) follow.

Developed with assistance from Claude Code.

ldk-reviews-bot · 2026-08-19T02:26:13Z

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

Jolah1

Third commit: the funding-kind check only matches tx_type: Some(Funding | InteractiveFunding), so the stale untyped record the commit message calls out passes it. An on-chain RBF replacing channel funding stays reachable after this PR, narrower than main, but still a funding double-spend, and it now rides on the rest of the stack landing. Worth its own issue.

Jolah1 · 2026-08-19T09:36:07Z

+	/// elapses, the node is shutting down and the package is dropped with it.
+	pub(crate) fn requeue_failed_classify(&self, package: BroadcastPackage) {
+		let sender = self.queue_sender.clone();
+		tokio::spawn(async move {


Only detached tokio::spawn left in non-test production code outside postgres_store. It's also what reorders the queue — the requeued package lands behind anything queued after it.
Holding the failed package in the loop and adding a sleep branch to the existing select! avoids both, and needs no runtime handle.

I don't think the ordering part is fixed at the current head. Suppose candidate A's classification fails and is parked. While A waits, newer candidate B arrives carrying history [A, B] and classifies successfully. When A retries, funding_reclassification_update can rotate the unconfirmed record back to A, while the pending update replaces [A, B] with [A]; A is then broadcast after B.

If B is subsequently observed, it can be treated as foreign and recorded as a duplicate. Could we preserve monotonic candidate history and freshness, with a regression test asserting that the record remains on B with history [A, B] after A retries?

Re-reviewed the delta since my last pass. Commit 1 is unchanged apart from the async store conversion; the responses landed as the two f - fixups on top.

@joostjager is right that the ordering isn't fixed, and it's the second half of my own earlier comment: holding the package in the loop removed the detached task but not the reorder a parked package still classifies and
broadcasts after everything queued behind it, so "avoids both" was wrong of me.

I reproduced his A/B case at fb85dd0. The rotation isn't merely possible: both guards that could stop it are Confirmed-only (wallet/mod.rs:2718, payment/store.rs:290), so for an unconfirmed record it always applies, and classify_interactive_funding has no freshness check before persist_funding_payment. Candidate histories only grow, so persist_funding_payment, which already holds the cross-store lock, can read the pending entry and skip when the incoming list is a strict prefix of the stored one. Happy to hand over the regression test.

Could we preserve monotonic candidate history and freshness, with a regression test asserting that the record remains on B with history [A, B] after A retries?

🤖 Done — essentially with @Jolah1's proposal generalized:

Candidate histories only grow, so persist_funding_payment, which already holds the cross-store lock, can read the pending entry and skip when the incoming list is a strict prefix of the stored one.

🤖 Rather than skip strict prefixes in one place, both writes now ignore stale candidate lists: the pending entry's stored list is only ever replaced by a list containing everything already in it, and the record is only updated by a classification whose list contains the record's current txid. A stale retry of A carries [A] — no B — so it changes nothing at either site.

tnull

This needs a rebase unfortunately.

tnull · 2026-08-19T13:14:02Z

+	/// the counterparty broadcasts it regardless — it would only leave the transaction
+	/// confirming without a recorded candidate. If the queue has closed by the time the delay
+	/// elapses, the node is shutting down and the package is dropped with it.
+	pub(crate) fn requeue_failed_classify(&self, package: BroadcastPackage) {


Codex:

[P1] Delayed requeue leaves the duplicate-record race open. /home/tnull/worktrees/ldk-node/pr-1057-review-20260819/src/tx_broadcaster.rs:164 removes the failed package and waits two seconds before requeueing it. If persistence recovers and wallet sync observes an interactive-RBF candidate
during that interval, sync creates a generic record keyed by the active txid. Classification later creates the funding record keyed by the first candidate, while direct lookup continues to prefer the generic record. The funding record can therefore remain pending—the outcome this commit
intends to prevent. The test only exercises a single Funding transaction whose payment ID equals its txid, without concurrent wallet sync.

🤖 Yeah, the retry only narrows the window — sync can still record the tx under its own txid while classification is failing. The follow-up PR handles that by merging the duplicate into the funding record once classification eventually succeeds. What this PR fixes is the drop: on main, one failure means classification never runs again, so the duplicate is permanent.

tnull · 2026-08-19T13:16:31Z

+	/// elapses, the node is shutting down and the package is dropped with it.
+	pub(crate) fn requeue_failed_classify(&self, package: BroadcastPackage) {
+		let sender = self.queue_sender.clone();
+		tokio::spawn(async move {


As noted above, this likely should be spawn_cancellable_background_task. Though given the codex comment above, not even sure if doing it in the background is the right approach?

No longer applicable.

🤖 I ended up removing the spawn entirely rather than tracking it: the retry is a timer branch in the broadcast loop's select!, so it's cancelled with the loop on stop(). The detached task was also buggier than it looked — its comment claimed a re-send after shutdown would fail because the queue had closed, but the receiver isn't dropped until the Node is, so the send succeeded and a stale package could be broadcast after stop()/start(). Added failed_classification_retry_dies_at_stop for that.

tnull · 2026-08-19T13:16:57Z

+				// funding history: its current txid or a classified candidate. A conflicting
+				// transaction that is neither — a close also spends the funding outpoint — must
+				// not overwrite the record.
+				let pending = self.pending_payment_store.get(&payment_id);


Codex:

[P2] Legitimate older candidates are classified as foreign. The gate at /home/tnull/worktrees/ldk-node/pr-1057-review-20260819/src/wallet/mod.rs:1986 accepts only the current txid or a recorded candidate. However, the persisted format explicitly permits an empty candidate list for older
records at /home/tnull/worktrees/ldk-node/pr-1057-review-20260819/src/payment/pending_payment_store.rs:46. If an earlier RBF candidate exists only in conflicting_txids and confirms, it is treated as foreign, producing a duplicate and leaving the funding record pending.

Mostly not a concern, but the follow-up will fix a gap when we crash.

🤖 To hit this you'd need a funding record with no candidates recorded at all, and I don't think a node can get into that state in practice: the pending store hasn't shipped in a release yet, so only a node that ran a few commits of main at the wrong time could have such a record. I'm also hesitant to loosen the check. A txid that only shows up in conflicting_txids could just as easily be a coop close or a third-party double-spend, and adopting one of those would corrupt the record. What can still go wrong is a crash before a round's classification finishes — nothing retries it after restart. The fix we have in mind is a startup pass that backfills the record's candidates from LDK's splice state; signed rounds survive restart with their txids, so it doesn't need any new persistence.

tnull · 2026-08-19T13:17:33Z

+			},
+		)]);
+
+		// Let the loop fail at least one classification round; a failed classification must not


Codex:

[P2] The retry regression test lacks a failure barrier. /home/tnull/worktrees/ldk-node/pr-1057-review-20260819/src/wallet/mod.rs:4265 sleeps for three seconds but never proves the queue attempted—and failed—classification. If the loop is delayed until writes are re-enabled, the test can
pass on the pre-fix implementation. The store should signal/count an observed failed write before recovery is enabled.

tnull · 2026-08-19T13:28:22Z

+		// classification re-types records concurrently, and a classification landing after the
+		// funding-kind check below would let the RBF replace a funding transaction. Acquired
+		// after the persister, matching the lock order of the wallet sync paths.
+		let funding_guard = self.funding_payment_update_lock.lock().await;


Ngl, it's kind of odd that we now also mix in the funding lock here with the regular RBF flow.

Do we really need to fix this? IIUC, not only does it require the wallet sync racing the LDK classification, it also requires that the user calls bump_fee_rbf on the wrong (i.e., funding transaction) record at exactly the right time, no?

Dropped. An RBF would need to spend the channel funding output, which isn't part of the wallet. But this still could be a problem for dual-funded channels, once supported. Opened #1072.

tnull

Btw, if we now retry classification/broadcast anyways as the counterparty might also broadcast, couldn't we unblock the broadcast queue again, i.e., don't have it block on the persistence succeeding?

jkczyz · 2026-08-28T22:33:42Z

Third commit: the funding-kind check only matches tx_type: Some(Funding | InteractiveFunding), so the stale untyped record the commit message calls out passes it. An on-chain RBF replacing channel funding stays reachable after this PR, narrower than main, but still a funding double-spend, and it now rides on the rest of the stack landing. Worth its own issue.

@Jolah1 The bump will fail for splices, but will be a problem for dual-funded channels, once supported. Opened #1072.

Btw, if we now retry classification/broadcast anyways as the counterparty might also broadcast, couldn't we unblock the broadcast queue again, i.e., don't have it block on the persistence succeeding?

@tnull 🤖 Only the failing package waits — the queue keeps flowing. True, the counterparty can broadcast regardless; the retry narrows that window and the follow-up merges the duplicate. Broadcasting before recording would just make that race the norm.

jkczyz · 2026-08-28T22:44:32Z

Rebased

Wallet sync resolves a funding payment's id for any transaction linked to the record through its conflicting txids, and then adopted that transaction's txid and confirmation outright. A cooperative close conflicts with a pending splice in exactly that way: the splice record would report the close's txid and confirmation under its InteractiveFunding type and contribution figures and graduate as if the splice had confirmed, while the close's own record never received its confirmation. Adopt a transaction only when it is part of the payment's funding history — the record's current txid or a classified candidate. Anything else is recorded under its own txid-keyed id, which also delivers the close's confirmation to the close's own record. Generated with assistance from Claude Code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

joostjager

The fixes LGTM aside from the small remarks below.

I do think that this PR and the gaps it leaves open reinforce the value of one consistent commit boundary for state, funding, payment records, and durable broadcast intent.

joostjager · 2026-09-02T09:39:54Z

+		// Packages whose classification failed, each waiting out FAILED_CLASSIFY_RETRY_DELAY
+		// before its next attempt. New packages keep flowing while these wait, and pending
+		// retries die with the loop on shutdown rather than resurfacing after a later start.
+		let mut parked: Vec<(tokio::time::Instant, BroadcastPackage)> = Vec::new();


[P2] Keep classification retries bounded and deduplicated

receiver.recv() continues draining the 256-entry channel while every failed package is appended to this unbounded Vec. For a transaction whose first classification cannot persist, LDK's periodic claim or sweep rebroadcasts can enqueue additional copies while the store remains unavailable. Every copy is then retried and logged, while remove(0) shifts the remaining entries.

A store outage coinciding with a force-close wave can therefore grow memory, CPU, and store load without bound, then produce a duplicate broadcast burst on recovery. Could we keep this bounded and coalesce packages by transaction or package identity, using a VecDeque or equivalent?

🤖 Done — failed packages now wait in a retry queue capped at the broadcast queue's own size. A package that's already queued isn't added twice; the cap exists for fee bumps — during a store outage LDK keeps re-sending its claims, and each send at a bumped fee is a new txid taking a new slot, so a single claim could grow the queue for as long as the outage lasts. Dropping the oldest entry once the cap is hit is safe because everything non-funding is regenerated on its own schedule (LDK's rebroadcast timer, the sweeper's per-block pass), so only the newest copy matters once the store recovers. Funding packages are exempt and never dropped: nothing re-sends them for us, and the payment record needs every negotiated version in its candidate history. The exemption can't grow the queue on its own — a new funding version only exists when another negotiation with the peer completes, never on a timer.

I did consider having a new package replace whatever queued entry it double-spends — that would size the queue naturally — but Claim and Sweep transactions combine many spends into one, so telling whether two entries are versions of the same transaction means comparing their inputs, with its own edge cases; the cap gets the same behavior with less machinery.

The cap and the dedup cover the memory growth and the recovery burst. One thing that's now constant rather than bounded with a fixed 2s delay per package, the queue is re-attempted at cap/delay, so a full queue is roughly 128 classification attempts per second for as long as the store is unavailable, each one a store write and a log_error! from classify_and_broadcast. Against SQLite that's mostly log volume, but with VssStore every attempt is a round trip to the store that's already struggling. Is a backoff worth adding here, or is a constant rate the deliberate choice so recovery gets picked up promptly?

If the store is struggling (i.e., slow) rather than just being unavailable, we wouldn't be hitting that rate since the queue is processed sequentially. And yes, the constant rate is deliberate: the queue holds time-sensitive claims, so once the store recovers everything retries within ~2s.

joostjager · 2026-09-02T09:40:33Z

+	/// elapses, the node is shutting down and the package is dropped with it.
+	pub(crate) fn requeue_failed_classify(&self, package: BroadcastPackage) {
+		let sender = self.queue_sender.clone();
+		tokio::spawn(async move {


I don't think the ordering part is fixed at the current head. Suppose candidate A's classification fails and is parked. While A waits, newer candidate B arrives carrying history [A, B] and classifies successfully. When A retries, funding_reclassification_update can rotate the unconfirmed record back to A, while the pending update replaces [A, B] with [A]; A is then broadcast after B.

If B is subsequently observed, it can be treated as foreign and recorded as a duplicate. Could we preserve monotonic candidate history and freshness, with a regression test asserting that the record remains on B with history [A, B] after A retries?

joostjager · 2026-09-02T10:13:22Z

+	async fn classify_and_broadcast(
+		&self, package: BroadcastPackage,
+	) -> Result<(), BroadcastPackage> {
+		if let Err(e) = self.tx_broadcaster.classify_package(&package).await {


Why maintain separate immediate and retry paths instead of treating every broadcast as scheduled retryable work?

Refactor the duplicated code, but kept the paths separate. Now that we have a bounded queue and deduplication, using the same path would mean we'd drop newer packages.

Jolah1 · 2026-09-02T16:44:35Z

Also worth folding in before merge: ebc0086 doesn't compile its tests standalone (list_filter on the bounded payment store), so the series isn't bisectable until the fixups are squashed.

Minor, likely follow-up: after commit 1 declines the close, nothing ever ends the splice record's life — it stays Pending indefinitely. Intended for the payment-model PR i guess

jkczyz · 2026-09-03T00:00:01Z

Also worth folding in before merge: ebc0086 doesn't compile its tests standalone (list_filter on the bounded payment store), so the series isn't bisectable until the fixups are squashed.

The compilation will be fixed once the fixups are squashed.

Minor, likely follow-up: after commit 1 declines the close, nothing ever ends the splice record's life — it stays Pending indefinitely. Intended for the payment-model PR i guess

Added a commit marking the record Failed once a conflict from outside its candidate history confirms past ANTI_REORG_DELAY while no candidate can still confirm. Opened issue #1078 covering the remaining problems.

joostjager

I know we agreed in the team meeting to press on with ldk-node under the current persistence model, but this PR and the follow-up work are changing my view.

Most of this PR is compensation for not having a consistent commit boundary. Especially now that AI highlights all the edge cases, it becomes increasingly difficult to reason about for a human. And it also becomes clear what we got ourselves into.

I think we should stop trying to force a release on top of this architecture and go back to the drawing board before adding more compensating logic.

joostjager · 2026-09-03T11:58:59Z

+			// periodically, while the incoming package may carry a fresher fee-bumped variant.
+			// A funding package is never dropped — nothing would re-broadcast it, and losing it
+			// leaves its transaction confirming without a recorded candidate.
+			match self.0.iter().position(|(_, _, waiting)| !waiting.contains_funding()) {


🤖 The deduplication fixes the periodic-growth problem, but the eviction assumption does not hold for every non-funding package. A CooperativeClose goes through classify_regular_broadcast, so a payment-store failure can park it here. rust-lightning emits the fully signed close from a one-shot close path and then removes the channel; the claim and sweeper timers do not recreate it. Once it becomes the oldest non-funding entry, this code can evict it, or refuse it when only protected funding entries are waiting.

That can discard our only local broadcast attempt and leave us dependent on the peer to publish the close. Could eviction be limited to transaction types known to be periodically regenerated, while treating cooperative closes and other one-shot broadcasts as non-droppable?

Right, nothing re-broadcasts a cooperative close. Would it be simpler to just panic if the queue is full? We already panic when ChannelMonitors and ChannelManager persistence fails.

But would a panic be recoverable then because anything still has the tx on disk?

But would a panic be recoverable then because anything still has the tx on disk?

🤖 Depends on the type. Claims and sweeps are on disk — the monitor and sweeper persist and re-broadcast them on their own, which is what made eviction safe for them. The closing tx is on disk nowhere: the channel is removed from the ChannelManager before the broadcaster is even called. What usually saves it is a rewind: if the store is down, the manager persist recording the removal also fails, we already panic on that, and the reloaded manager still has the channel — negotiation restarts on reconnect and broadcasts a fresh closing tx. So a queue-full panic mostly duplicates the persist panic that fired first. Where neither panic helps is a partial failure — manager persists, payment-store writes keep failing: there, only keeping the close in the queue recovers it once the store returns. The latest fixup does that: only claims, sweeps, and anchor bumps can be dropped at the bound now; cooperative closes wait alongside fundings.

Could eviction be limited to transaction types known to be periodically regenerated, while treating cooperative closes and other one-shot broadcasts as non-droppable?

Ended up adding a fixup doing this instead as noted above.

TheBlueMatt

Most of this PR is compensation for not having a consistent commit boundary.

Huh? AFAICT almost none of the code here would be fixed by some god-persistence write. It seems to ~all be due to BDK detecting a transaction on its own.

TheBlueMatt · 2026-09-03T16:46:40Z

+	Refused(BroadcastPackage),
+}
+
+/// Packages whose classification failed, each waiting out a retry delay before its next attempt.


Why do we need a queue? Can't we just spawn a tokio task and rebroadcast in a loop?

Note that the queue isn't for rebroadcasting. It's for retrying failed persistence, which needs to succeed before broadcasting. Since LDK periodically re-broadcasts claims, if persistence is failing we need to dedup them rather than spawning more tasks.

Do you have any opinion on #1057 (comment)?

jkczyz · 2026-09-04T04:30:30Z

I know we agreed in the team meeting to press on with ldk-node under the current persistence model, but this PR and the follow-up work are changing my view.

Most of this PR is compensation for not having a consistent commit boundary. Especially now that AI highlights all the edge cases, it becomes increasingly difficult to reason about for a human. And it also becomes clear what we got ourselves into.

I think we should stop trying to force a release on top of this architecture and go back to the drawing board before adding more compensating logic.

Discussed offline. The last PR in the stack (#1080) now creates the a payment record before signing when processing the FundingTransactionReadyForSigning event. This eliminates most of the edge cases resulting from having the wallet sync pick-up the counterparty's broadcast before we process our own.

joostjager · 2026-09-04T08:24:49Z

Huh? AFAICT almost none of the code here would be fixed by some god-persistence write. It seems to ~all be due to BDK detecting a transaction on its own.

I did not mean one god commit spanning every store. My thinking was that if the creator of the operation performs the classification and commits it along with the rest of its state, the broadcaster would not need the retry queue or ordering logic.

joostjager

Creating the record before signing indeed seems like a good solution to avoid the race and retry loop.

I assume this PR cannot simply be reduced because other transactions still need to be (retryably) classified in the broadcaster? Wondering if for those, the payment records can also be created closer to where the txes originate as well, at a point where failure isn't unsafe, and making the second bug fix unnecessary?

jkczyz · 2026-09-04T17:05:23Z

Creating the record before signing indeed seems like a good solution to avoid the race and retry loop.

I assume this PR cannot simply be reduced because other transactions still need to be (retryably) classified in the broadcaster? Wondering if for those, the payment records can also be created closer to where the txes originate as well, at a point where failure isn't unsafe, and making the second bug fix unnecessary?

The race only existed for splices. The other types use the txid for the PaymentId, and only cooperative closes are broadcast by both parties. Other than v1 opens, we only see the transaction when LDK gives it to us for broadcasting.

I did not mean one god commit spanning every store. My thinking was that if the creator of the operation performs the classification and commits it along with the rest of its state, the broadcaster would not need the retry queue or ordering logic.

Note that we still need to update the splice payment record when broadcasting since it contains InteractiveFunding data, which is not available when signing.

jkczyz · 2026-09-08T17:34:07Z

After some offline discussion, we decided to use Event::FundingTransactionReadyForSigning to create the payment record in order to avoid the race. However, it looks like more work is needed to no longer need to use InteractiveFunding at tx broadcast time. Looking into that now.

Separately, I filled two issues upstream to allow simplifying the current approach further:

No need to re-review yet.

A queued broadcast whose payment-record classification failed was dropped outright, on the theory that broadcasting a transaction we failed to record would leave it on-chain without a payment. For interactive funding that theory doesn't hold: the counterparty broadcasts the same transaction once the signature exchange completes, so dropping the package keeps nothing off-chain -- it only guarantees the round is never recorded as a candidate on our side. The funding-status ownership gate then treats the round's confirmation as foreign to the funding record and re-keys it to a stray duplicate record, which shadows the funding record's txid lookups permanently: the splice payment stays Pending forever while an untyped duplicate holds the confirmation. Keep the package alive instead: retry classification after a short delay, holding the broadcast back until it succeeds. Other packages keep flowing while a retry waits, and pending retries are dropped when the node stops -- a retry that outlived a stop would classify and broadcast a stale package after a later start. Classification failures are persistence failures, so there is no limit on attempts -- a store that never recovers keeps the node from functioning anyway -- and every failed round is logged. The waiting packages are deduplicated and bounded. LDK re-broadcasts pending claims every 30 seconds and regenerates sweeps once per block until they confirm, so over a long store outage a copy per rebroadcast would otherwise pile up and replay as a burst on recovery. A package whose transactions already await a retry is not queued again. At the bound, an incoming package that LDK would re-broadcast anyway makes room by dropping the oldest such waiting package, whose transactions return with the next rebroadcast; if every waiting package is one nothing re-broadcasts, the incoming package is dropped instead. Fundings and cooperative closes are never dropped to make room and never refused at the bound, since nothing re-broadcasts them: a dropped funding would leave its transaction confirming without a recorded candidate, and a dropped cooperative close might lose the only copy of the signed closing transaction. Fee-bumped rebroadcasts carry new txids, so the bound, not the deduplication, is what limits their accumulation. Generated with assistance from Claude Code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Since declining to adopt a conflicting close's confirmation, a funding payment whose transaction was double-spent stayed Pending forever -- nothing wrote a terminal status for an on-chain record -- and the sync loop kept re-queueing the dead transaction for rebroadcast on every tip change. Mark such a record Failed once a conflict from outside its candidate history has confirmed through ANTI_REORG_DELAY while neither its own transaction nor any RBF candidate can still confirm, mirroring the anti-reorg finality the Succeeded transition already assumes. Removing the payment's pending entry then stops the re-queueing. Settling also removes the entry that maps candidate txids to the record, so a later wallet event for a dead candidate falls back to keying by that candidate's txid -- which, for the first candidate, is the record's own id. Skip such events rather than let the generic handling resurrect the settled record, and let a replayed replacement event finish an entry removal a crash interrupted instead of stamping the terminal status into the leftover entry. Implemented with Claude Code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Wallet sync can learn of a splice transaction before broadcast-time classification records it: once tx_signatures are exchanged, the counterparty may broadcast first, and sync then files the round under a duplicate record keyed by its txid, which shadows the funding record's txid lookups from then on. Retrying a failed classification only narrows that window: a round the counterparty broadcasts is still observed before our record exists. Record the funding payment while handling FundingTransactionReadyForSigning, before funding_transaction_signed hands our signatures to LDK. The counterparty cannot broadcast without them, so the record precedes anything wallet sync can observe, and every later observer resolves to it. The record is written in full from the channel's pending splice history, so the round's broadcast has nothing left to record and records nothing. If the record cannot be written, the event is replayed rather than proceeding unrecorded: LDK re-offers it in-session and regenerates it across restarts while the transaction remains unsigned. A failed write leaves no half-written record behind for the replayed event to build on. Should undoing it fail as well, the replayed event removes what was left of a first round once the round is gone from the channel's history; the leftovers of a bump live under an earlier round's record, which wallet sync moves on as that round confirms or fails. Recording before the round is negotiated means a recorded round can still be abandoned: the counterparty may abort after we sign but before its commitment_signed, or the channel may close, and until LDK has released our signatures nothing can ever broadcast the transaction. Left in place, the record would wait forever on a payment nothing can confirm. The signed round is therefore marked as awaiting broadcast until LDK reports the splice negotiated, which it does as it hands the fully signed round to the broadcaster: from then on the counterparty holds our signatures and can broadcast on its own. If the mark cannot be cleared, that event is replayed as well. A marked round is dropped once LDK no longer holds it, unless the wallet has seen its transaction: the counterparty may broadcast a round it received our signatures for while LDK still waits on its own. A round whose negotiation LDK has reported keeps its place whether or not wallet sync has seen it yet, and so does the channel's current funding: a zero-conf splice becomes the funding as soon as splice_locked is exchanged, before its transaction confirms or LDK's report of its negotiation has necessarily been handled. Dropping a round leaves the record on the last remaining round this node contributed to, moving it there if it still names the dropped round, or removes the record when none remains. LDK's view is consulted when it reports the failed negotiation of a channel it still lists, when the channel closes -- a round awaiting the counterparty's signatures gets no failure report then, and a failure reported once the channel is gone is resolved by what this report carries, the channel's last funding, and by the rounds its monitor still watches -- and at startup, before any background task runs: LDK reports the loss of a negotiation its last channel manager write carried mid-way, but a round committed, negotiated and signed since that write gets no report if the node stops before the next one. The channel manager forgets a closed channel's pending rounds, but its monitor keeps watching every round the counterparty's commitment_signed reached, and our signatures cannot have left the node before that message: the counterparty may hold the fully signed transaction and broadcast it, as when this node's contributed input value is the smaller and its tx_signatures therefore go first, so such a round is kept for wallet sync to resolve should it confirm, while a marked round the monitor never watched is dropped, as nothing can broadcast it. A round already missing from the channel's history when the signing event is handled is not recorded at all. Rounds without a local contribution emit no signing event and are not recorded at broadcast either, as before; they are left to wallet sync. Developed with assistance from Claude Code. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

A splice round this node signed is kept at `ChannelClosed` when the channel's monitor watches it: the counterparty committed to it, so our signatures may have left the node, and the counterparty may broadcast the round and see it confirm. A close the wallet sees as a conflict -- a cooperative close spending an input the round shares -- fails the payment once it confirms beyond the reorg depth, but nothing resolved such a record when a commitment transaction, which pays no wallet script, won instead. Once the close matures -- after the reorg delay for a counterparty's commitment transaction, and once the to_self_delay on our balance has passed for one of our own -- the monitor stops watching the rounds it kept and queues a `DiscardFunding` event for each, and the handler only reclaimed the contribution's addresses: the funding payment stayed `Pending` forever. Likewise for a round of ours that a sibling round this node did not contribute to replaced on an open channel: LDK discards our round as the sibling locks, and the payment stayed `Pending` for a transaction that can no longer confirm. Resolve the channel's funding payments by the rounds LDK holds. A round nothing ever broadcast is dropped first, as `ChannelClosed` already did, and with it a record no broadcast round of ours remains under. A payment is then left alone if a round of ours that LDK still holds remains in its record -- the round that locked, or one still pending -- or one LDK promoted to the funding before, and failed otherwise: no round of ours can confirm anymore, whether the channel closed on a commitment transaction or a round we did not contribute to locked. The rounds LDK holds are the channel's pending rounds and funding while the manager lists the channel, and once it does not, the funding its monitor settled on plus whatever the monitor still watches. The monitor is left out for a listed channel: its updates land after the manager's, deferred to the background processor's flush, so it may still watch a round the manager let go. The event names this node's contribution, not the round: the inputs and output scripts LDK returns of it. Matching that to a recorded round would take the parts of every contribution on record. LDK discards the round's siblings as it promotes the round and reports the promotion through `ChannelReady`, so that event resolves the payments of a listed channel instead: it records the promotion and resolves the channel's other payments by the rounds the manager holds once updated -- the promoted round, and whatever was negotiated behind it. For a channel the manager no longer lists it records the promotion alone and leaves the payments to the close. A `DiscardFunding` for a listed channel then only drops a round nothing broadcast that the manager no longer holds and reclaims the contribution's addresses. A zero-conf splice is promoted to the funding as `splice_locked` is exchanged, before its transaction confirms, and a later splice moves the funding on again: at the close neither the manager nor the monitor holds the earlier round, although it can still confirm, the later round descending from it. So the funding payment records each promotion LDK reports through `ChannelReady`, and a round promoted once counts as one that can confirm wherever the rounds LDK holds decide: as a sibling round is promoted, and when the channel closes. The monitor's events can reach the handler ahead of the channel's `ChannelClosed` when one sync delivers the close and its maturity: the channel manager polls the monitor's report of the close at the start of each event pass and on peer traffic, and the monitor's own events are handled right after the manager's. Each event then finds the channel still listed and leaves the payments, there being no promotion to resolve them. So `ChannelClosed` fails every payment of the channel left with no round of ours the monitor watches and none promoted before, and a `DiscardFunding` event for a channel the manager no longer lists resolves each record the same way, by the funding its monitor settled on and whatever it still watches. Developed with assistance from Claude Code. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

ldk-reviews-bot requested a review from tnull August 19, 2026 02:36

jkczyz mentioned this pull request Aug 19, 2026

Retry user-initiated splices across restarts and disconnects #930

Closed

Jolah1 reviewed Aug 19, 2026

View reviewed changes

tnull reviewed Aug 19, 2026

View reviewed changes

tnull added this to the 0.8 milestone Aug 19, 2026

tnull reviewed Aug 19, 2026

View reviewed changes

jkczyz force-pushed the 2026-08-funding-payment-bugfixes branch from 6093418 to 9e29da5 Compare August 28, 2026 22:33

jkczyz force-pushed the 2026-08-funding-payment-bugfixes branch from 9e29da5 to fb85dd0 Compare August 28, 2026 22:44

jkczyz changed the title ~~Fix three funding payment record bugs~~ Fix two funding payment record bugs Aug 31, 2026

jkczyz requested review from Jolah1, joostjager and tnull September 1, 2026 15:39

joostjager reviewed Sep 2, 2026

View reviewed changes

jkczyz changed the title ~~Fix two funding payment record bugs~~ Keep funding payment records accurate Sep 2, 2026

jkczyz mentioned this pull request Sep 2, 2026

On-chain payment records can stay Pending forever when their transaction is double-spent #1078

Open

jkczyz force-pushed the 2026-08-funding-payment-bugfixes branch from fb85dd0 to b15d50d Compare September 2, 2026 23:59

jkczyz requested a review from joostjager September 3, 2026 00:02

This was referenced Sep 3, 2026

Track each channel funding in a single payment record #1079

Draft

Track in-flight splices for failure reporting and crash recovery #1080

Draft

joostjager reviewed Sep 3, 2026

View reviewed changes

TheBlueMatt reviewed Sep 3, 2026

View reviewed changes

jkczyz requested review from TheBlueMatt and joostjager September 4, 2026 04:31

joostjager reviewed Sep 4, 2026

View reviewed changes

jkczyz force-pushed the 2026-08-funding-payment-bugfixes branch from b15d50d to a55bf73 Compare September 4, 2026 17:08

jkczyz requested a review from joostjager September 4, 2026 17:12

jkczyz force-pushed the 2026-08-funding-payment-bugfixes branch from b24cb5a to 1b0a194 Compare September 10, 2026 15:17

jkczyz and others added 4 commits September 10, 2026 12:37

jkczyz force-pushed the 2026-08-funding-payment-bugfixes branch from 1b0a194 to 4fd10b8 Compare September 10, 2026 19:59

Uh oh!

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

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

tnull Aug 19, 2026 • 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!

tnull left a comment

Choose a reason for hiding this comment

Uh oh!

jkczyz commented Aug 28, 2026

Uh oh!

jkczyz commented Aug 28, 2026

Uh oh!

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

Jolah1 commented Sep 2, 2026

Uh oh!

jkczyz commented Sep 3, 2026

Uh oh!

joostjager 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

jkczyz commented Aug 19, 2026 •

edited

Loading

ldk-reviews-bot commented Aug 19, 2026 •

edited

Loading

tnull Aug 19, 2026 •

edited

Loading