Include interactive funding candidates on broadcast by jkczyz · Pull Request #4570 · lightningdevkit/rust-lightning

jkczyz · 2026-04-17T21:14:13Z

Include every negotiated funding candidate (original + RBF replacements) at broadcast time so downstream consumers can update their own state tracking from the broadcaster callback. The local contribution data isn't recoverable from the on-chain transaction, and the replaced txids aren't either, so the broadcaster callback is the only place where this information can be observed.

TransactionType::Splice is replaced with TransactionType::InteractiveFunding { candidates: Vec<FundingCandidate> }. Each FundingCandidate carries its txid and the channels participating in it; each ChannelFunding carries the channel id, counterparty node id, purpose (Establishment / Splice), and the local node's contribution for this candidate. The list covers every negotiated candidate in order — original first, then each RBF replacement — letting downstream reconcile any historical txid, not just the immediate predecessor. The alternative (reacting to SplicePending) was worse: that event races with BDK wallet sync and doesn't carry the replaced txid.
The new variant is structured to be forward-compatible with batches and V2 (dual-funded) channel establishment, neither of which is implemented today. For the current single-channel splice path, each candidate's channels list has length 1.
FundingCandidate, ChannelFunding, and FundingPurpose implement Writeable / Readable so downstream can persist them directly without mirroring.
FundingContribution gains public getters for estimated_fee, inputs, and max_feerate; feerate is elevated from pub(super) to pub. Combined with existing value_added, outputs, change_output, this exposes everything a downstream consumer needs without reaching into the raw transaction.

The Hash derive on TransactionType is dropped (unused in the workspace; FundingContribution transitively contains ConfirmedUtxo which lacks Hash).

ldk-reviews-bot · 2026-04-17T21:14:16Z

👋 Thanks for assigning @TheBlueMatt as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

ldk-claude-review-bot · 2026-04-17T21:21:28Z

Good — no remaining references to the old TransactionType::Splice variant.

No new issues found beyond what was already flagged in prior review comments. Here's my summary:

Prior review status:

channel.rs:9395 — filter_map silently dropping candidates: Resolved. Code now uses .map() + .expect().
chaininterface.rs:123 — Grammar nit ("An" → "A"): Resolved.
chaininterface.rs:175 — Dead code TLV serialization impls: Still applicable (minor nit, already flagged).
channel.rs:7256 — 0-conf rebroadcast type mismatch: This was about pre-existing behavior outside the scope of this PR's changes, and the referenced test doesn't exist in this PR. Not applicable to this diff.

New issues: None found.

The contribution offset math (contrib_offset = negotiated_candidates.len() - contributions.len()) correctly handles the invariant that missing contributions are always leading entries. The TransactionType variant rename from Splice to InteractiveFunding is applied consistently across all match sites. The new public getters on FundingContribution are straightforward field accessors. Test assertions properly validate the candidate chain, replaced-txid contract, and contribution presence for both initiator and acceptor.

ldk-reviews-bot · 2026-04-20T00:00:55Z

🔔 1st Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-04-20T00:00:57Z

🔔 1st Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-04-22T00:01:41Z

🔔 2nd Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-04-22T00:01:43Z

🔔 2nd Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-claude-review-bot · 2026-04-22T18:06:12Z

+		if !self.context.is_manual_broadcast
+			&& !waiting_for_batch
+			&& self.funding.get_funding_tx_confirmation_height().is_none()
+		{
+			if let Some(tx) = &self.funding.funding_transaction {
+				return Some((
+					tx.clone(),
+					TransactionType::Funding {
+						channels: vec![(
+							self.context.counterparty_node_id,
+							self.context.channel_id,
+						)],
+					},
+				));
+			}
+		}


Bug: After 0-conf splice promotion, self.pending_splice is None (cleared at maybe_promote_splice_funding line 11680) and self.funding holds the splice's FundingScope with funding_tx_confirmation_height == 0 and funding_transaction = Some(splice_tx). This initial-funding branch will match:

is_manual_broadcast = false (normal channel)

waiting_for_batch = false (channel is ChannelReady)

get_funding_tx_confirmation_height().is_none() = true (height is 0)

funding_transaction = Some(splice_tx)

...and return TransactionType::Funding wrapping the splice transaction. The splice branch below is unreachable because self.pending_splice is None.

The test test_splice_rebroadcast_uses_splice_type_after_0conf_promotion (added in this PR) expects TransactionType::Splice with the original contribution preserved, but this code would produce TransactionType::Funding. That test will fail.

A possible fix: check self.funding.channel_transaction_parameters.splice_parent_funding_txid — it is Some(...) for promoted splices and None for initial funding. You'd also need to persist the contribution and replaced_txid metadata somewhere that survives promotion (e.g., on FundingScope or ChannelContext), since pending_splice is cleared.

Discussed offline. Dropped the commit to retry for now. We may do this later in ChannelMonitor where we'll store the FundingContribution.

TheBlueMatt

Needs rebase, also ci is very sad.

jkczyz · 2026-04-23T22:17:32Z

Rebased. Compilation failures were from new tests in main.

codecov · 2026-04-23T23:10:49Z

Codecov Report

❌ Patch coverage is 78.04878% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.16%. Comparing base (42e198c) to head (d94e8fb).

Files with missing lines	Patch %	Lines
lightning/src/ln/funding.rs	25.00%	9 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4570      +/-   ##
==========================================
+ Coverage   87.15%   87.16%   +0.01%     
==========================================
  Files         161      161              
  Lines      109251   109284      +33     
  Branches   109251   109284      +33     
==========================================
+ Hits        95215    95260      +45     
+ Misses      11560    11548      -12     
  Partials     2476     2476

Flag	Coverage Δ
fuzzing-fake-hashes	`31.17% <75.60%> (+0.01%)`	⬆️
fuzzing-real-hashes	`22.91% <0.00%> (-0.02%)`	⬇️
tests	`86.23% <78.04%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ldk-reviews-bot · 2026-04-25T00:00:43Z

🔔 3rd Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-04-27T00:00:15Z

🔔 1st Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

ldk-reviews-bot · 2026-04-27T00:01:48Z

🔔 4th Reminder

Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

tnull · 2026-04-27T07:38:56Z

+		contribution: Option<FundingContribution>,
+		/// For an RBF replacement, the txid of the prior negotiated splice candidate being
+		/// replaced. `None` for the first splice attempt.
+		replaced_txid: Option<Txid>,


Hmm, what happens when we make multiple subsequent RBFs? Could/should we make this a Vec and track all prior conflicting Txids to make sure we're not losing any intermediary step when tracking these events?

Also, when integrating BDK's RBF events in LDK Node we found that it would be nice if the API clearly defined an ordering for the replaced_txids, i.e., so we can always identify the 'original' txid (under which we started tracking a certain payment). That is, if we make this a Vec it would be great if the API could guarantee that replaced_txids[0] is always the 'original'.

That seems reasonable, though things get hairy if we have multiple splice-RBFs that conflict across channels. ISTM LDK Node might need to track that somehow, but maybe we can just avoid ever building such things?

Ok, discussed a bit offline. Ideally, we'll want to use a format in LDK Node that is compatible with batching. I've pushed a change that refactors TransactionType::Splice to TransactionType::InteractiveFunding, which supports batches of both v2 channel establishment and splices in the future. It also contains all previous attempts (including contributions) along with the corresponding txid instead of replaced_txid. Including simplifies the downstream handling. Then in 0.4 we can add re-broadcasts.

See how it's used in LDK Node here: lightningdevkit/ldk-node#888. LMK what you think.

TheBlueMatt

Changes LGTM, no strong opinion on the API, if it works for LDK Node alright, though I agree exposing all the previous RBFs might be nice.

TheBlueMatt · 2026-04-28T14:23:00Z

+		contribution: Option<FundingContribution>,
+		/// For an RBF replacement, the txid of the prior negotiated splice candidate being
+		/// replaced. `None` for the first splice attempt.
+		replaced_txid: Option<Txid>,


That seems reasonable, though things get hairy if we have multiple splice-RBFs that conflict across channels. ISTM LDK Node might need to track that somehow, but maybe we can just avoid ever building such things?

ldk-reviews-bot · 2026-04-29T00:02:18Z

🔔 5th Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

TheBlueMatt · 2026-04-30T15:46:30Z

CI is very sad but feel free to squash.

jkczyz · 2026-04-30T18:51:18Z

Squashed and rebased to fix CI.

ldk-claude-review-bot · 2026-04-30T18:55:35Z

+impl_writeable_tlv_based!(FundingCandidate, {
+	(1, txid, required),
+	(3, channels, required_vec),
+});
+
+impl_writeable_tlv_based!(ChannelFunding, {
+	(1, counterparty_node_id, required),
+	(3, channel_id, required),
+	(5, purpose, required),
+	(7, contribution, option),
+});
+
+impl_writeable_tlv_based_enum!(FundingPurpose,
+	(0, Establishment) => {},
+	(2, Splice) => {},
+);


Nit: these three serialization impls (FundingCandidate, ChannelFunding, FundingPurpose) appear to be dead code — TransactionType is never serialized (no Writeable/Readable impl), and these types are only used inside TransactionType::InteractiveFunding, which is purely in-memory and passed through BroadcasterInterface::broadcast_transactions as a reference.

If these are forward-looking (e.g., for future persistence or cross-process messaging), a brief comment explaining the intent would help. Otherwise they could be removed to avoid confusion.

We need these for downstream

Replace TransactionType::Splice with TransactionType::InteractiveFunding so downstream consumers can update their own state tracking from the broadcast callback. The local contribution data isn't recoverable from the on-chain transaction, so the broadcast must surface it directly. Each candidate carries the participating channels and their local contributions; the broadcast lists every negotiated candidate — original first, then each RBF replacement — letting downstream reconcile any historical txid, not just the immediate predecessor. The new variant is structured to be forward-compatible with batches and V2 (dual-funded) channel establishment, neither of which is implemented today. The new types are Writeable/Readable so downstream can persist them directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Add public getters for `estimated_fee`, `inputs`, and `max_feerate`, and elevate `feerate` from `pub(super)` to `pub`. Together with the existing `value_added`, `outputs`, and `change_output`, this gives downstream consumers of `TransactionType::Splice` (notably LDK Node, which updates `PaymentDetails` from the broadcast callback) the data they need without reaching into the raw transaction. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

jkczyz requested review from TheBlueMatt and wpaulino April 17, 2026 21:14

ldk-claude-review-bot reviewed Apr 22, 2026

View reviewed changes

jkczyz force-pushed the 2026-04-splice-transaction-type branch from dbaf6d9 to 86dcede Compare April 22, 2026 18:36

TheBlueMatt reviewed Apr 23, 2026

View reviewed changes

jkczyz self-assigned this Apr 23, 2026

jkczyz added this to Weekly Goals Apr 23, 2026

jkczyz force-pushed the 2026-04-splice-transaction-type branch from 86dcede to dacf092 Compare April 23, 2026 21:42

jkczyz requested a review from TheBlueMatt April 23, 2026 22:17

tnull reviewed Apr 27, 2026

View reviewed changes

TheBlueMatt previously approved these changes Apr 28, 2026

View reviewed changes

jkczyz dismissed TheBlueMatt’s stale review via 925b58d April 29, 2026 20:42

jkczyz force-pushed the 2026-04-splice-transaction-type branch 2 times, most recently from 925b58d to 4b82770 Compare April 29, 2026 21:13

jkczyz requested review from TheBlueMatt and tnull April 29, 2026 22:35

TheBlueMatt previously approved these changes Apr 30, 2026

View reviewed changes

jkczyz dismissed TheBlueMatt’s stale review via 741430b April 30, 2026 18:49

jkczyz force-pushed the 2026-04-splice-transaction-type branch from 4b82770 to 741430b Compare April 30, 2026 18:49

ldk-claude-review-bot reviewed Apr 30, 2026

View reviewed changes

Comment thread lightning/src/ln/channel.rs Outdated

jkczyz changed the title ~~Add splice details to TransactionType::Splice~~ Include interactive funding candidates on broadcast Apr 30, 2026

jkczyz force-pushed the 2026-04-splice-transaction-type branch from 741430b to 8c3d58f Compare April 30, 2026 19:01

ldk-claude-review-bot reviewed Apr 30, 2026

View reviewed changes

Comment thread lightning/src/chain/chaininterface.rs Outdated

jkczyz requested a review from TheBlueMatt April 30, 2026 19:16

jkczyz force-pushed the 2026-04-splice-transaction-type branch from 8c3d58f to f245f31 Compare April 30, 2026 19:17

jkczyz and others added 2 commits April 30, 2026 14:19

jkczyz force-pushed the 2026-04-splice-transaction-type branch from f245f31 to d94e8fb Compare April 30, 2026 19:26

TheBlueMatt approved these changes Apr 30, 2026

View reviewed changes

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

ldk-reviews-bot commented Apr 20, 2026

Uh oh!

ldk-reviews-bot commented Apr 20, 2026

Uh oh!

ldk-reviews-bot commented Apr 22, 2026

Uh oh!

ldk-reviews-bot commented Apr 22, 2026

Uh oh!

ldk-claude-review-bot Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

jkczyz Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

jkczyz commented Apr 23, 2026

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

ldk-reviews-bot commented Apr 25, 2026

Uh oh!

ldk-reviews-bot commented Apr 27, 2026

Uh oh!

ldk-reviews-bot commented Apr 27, 2026

Uh oh!

tnull Apr 27, 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!

TheBlueMatt Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

jkczyz Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented Apr 29, 2026

Uh oh!

TheBlueMatt commented Apr 30, 2026

Uh oh!

jkczyz commented Apr 30, 2026

Uh oh!

ldk-claude-review-bot Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

jkczyz Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jkczyz commented Apr 17, 2026 •

edited

Loading

ldk-reviews-bot commented Apr 17, 2026 •

edited

Loading

ldk-claude-review-bot commented Apr 17, 2026 •

edited

Loading

codecov Bot commented Apr 23, 2026 •

edited

Loading

tnull Apr 27, 2026 •

edited

Loading