Guard payment records against splice funding rebroadcasts - #1049
Merged
tnull merged 2 commits intoAug 13, 2026
Conversation
After a 0conf splice is promoted, LDK re-broadcasts the still-unconfirmed funding transaction through its generic funding path on every monitor-update completion, re-typed as a plain funding transaction without contribution metadata. The channel opener (or either side of a V2-established channel) re-broadcasts this way even for splices the interactive-funding classification deliberately declined to record — a splice-out paying an external address, or a counterparty-initiated splice with no local contribution. Recording the re-broadcast minted a spurious zero-amount payment that nothing ever confirms, leaving it stuck pending forever. Skip recording funding broadcasts that move no wallet funds, matching the condition the interactive-funding classification declines on; anything declined there must be skipped here, or its re-broadcast resurrects the record. The fee is deliberately not part of the condition: the wallet derives a splice's fee from the previous funding output whenever it funded the original channel open, so a computable fee is no sign of participation. Channel opens are unaffected: the funder's wallet always contributes the funding input. The re-typed re-broadcasts are upstream behavior rust-lightning should fix by keeping the interactive-funding classification on re-offers (or not re-offering at all), tracked at https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878. The skip leaves no trace in the stores, so a canary test asserts its log line on the initiator of a 0conf splice-out to an external address; when that fails against a newer LDK, re-evaluate whether the skip still sees traffic. Generated with assistance from Claude Code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
LDK re-broadcasts a promoted-but-unconfirmed 0conf splice on every monitor-update completion, typed as a plain funding transaction. Its reclassification carries wallet-view figures rather than the contribution-derived ones the interactive-funding classification recorded, so each rebroadcast overwrote the record's amount and fee and downgraded its transaction type — repeatedly, until the splice confirmed and even across restarts. A funding-typed classification of a record already classified as interactive funding carries nothing the record doesn't have, so skip the merge and leave the record untouched. The re-typed re-broadcasts are upstream behavior rust-lightning should fix by keeping the interactive-funding classification on re-offers (or not re-offering at all), tracked at https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878. Since the guard leaves no trace in the stores, classification logs when a funding-typed broadcast arrives for an interactive-funding record, and a canary test asserts that line on the contributing side of a 0conf splice; when it fails against a newer LDK, re-evaluate whether the guard still sees traffic. Generated with assistance from Claude Code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
👋 Thanks for assigning @tnull as a reviewer! |
This was referenced Aug 12, 2026
tnull
approved these changes
Aug 13, 2026
tnull
left a comment
tnull
left a comment
Collaborator
There was a problem hiding this comment.
One non-blocking nit (not the biggest fan of asserting on logs). Will just go ahead and land this.
| // appears the classification pipeline has demonstrably processed one. | ||
| let skipped = format!("Not recording channel-funding broadcast {}", txo.txid); | ||
| assert!( | ||
| logger_a.wait_for(&skipped).await, |
Collaborator
There was a problem hiding this comment.
nit: Hmm, should we really assert on the log output here? Usually I'm not the biggest fan as that is pretty brittle.
Contributor
Author
There was a problem hiding this comment.
I wasn't a fan either, but the alternative was to allow parameterizing the broadcaster or adding some _test_utils feature to expose a test-only API. Once it is fixed upstream, we can drop the test as it's only here to fail when the workaround is no longer needed.
| // downgrade, leaving no trace that a re-broadcast arrived. Log the arrival so tests can | ||
| // observe the traffic. The read cannot go stale: only the broadcast loop writes | ||
| // interactive-funding classifications, and it runs this classification too. | ||
| if let Some(current) = self.payment_store.get(&payment_id) { |
Collaborator
There was a problem hiding this comment.
Man, there are now so many edge cases to handle in this code. I increasingly think we need to reconsider the chosen approach (classifying through the broadcaster interface rather than keeping state in LDK and providing an API for it). Reopened https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/3566#issuecomment-440916 and added it to the v0.4 milestone.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After a 0conf splice is promoted while its funding transaction is still unconfirmed, LDK re-broadcasts that transaction on every monitor-update completion — re-typed as a plain
Fundingtransaction, without theInteractiveFundingcontribution metadata it was originally broadcast with. This continues until the splice confirms, including across restarts.Two guards in the funding classification path handle these re-offers:
The re-typing is upstream behavior rust-lightning should fix, tracked at https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878. Both commits carry TODOs and canary tests that fail once upstream stops re-typing the re-offers, so the workarounds can be removed when that happens.
Split out of #930, which builds on these commits.
Generated with assistance from Claude Code (Claude Fable 5).