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.

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

I've thoroughly reviewed every file and hunk in this diff, cross-referencing my prior review context and verifying the key invariants.

Verification performed:

Contribution offset math (channel.rs:9386-9401): Correct for all cases. saturating_sub handles the (impossible) case where contributions.len() > negotiated_candidates.len() gracefully. The checked_sub(contrib_offset) + .get(j) properly maps leading no-contribution rounds to None and trailing contribution rounds to the correct index.
Hash derive chain: Complete. FundingContribution (newly Hash) → FundingTxInput (ConfirmedUtxo, newly Hash) → Utxo (already Hash) → bitcoin::Transaction (implements Hash in bitcoin 0.32.x). All intermediate types in the chain implement Hash.
All TransactionType::Splice match sites: Properly updated to TransactionType::InteractiveFunding. No orphaned references remain.
Test assertions in sign_interactive_funding_tx_with_acceptor_contribution: The assert_broadcast closure correctly validates candidate chain structure, replaced-txid contract (via checked_sub(2)), contribution presence, and purpose for both initiator and acceptor across all callers.
New public getters (estimated_fee, inputs, feerate, max_feerate): Straightforward field accessors with no logic errors. feerate elevation from pub(super) to pub is a strict visibility widening with no impact on existing callers.
All test callers pass correct expected_replaced_txid values: None for first-splice, Some(prior_tx.compute_txid()) for RBF rounds.

Prior review comment status:

channel.rs:7256 — 0-conf rebroadcast type mismatch: Pre-existing issue, still applicable (outside this PR's scope).
chaininterface.rs:177 — Dead code TLV serialization impls: Still applicable (minor).
channel.rs:9395 — filter_map silently dropping candidates: Resolved (now uses .map() + .expect()).
chaininterface.rs:123 — Grammar nit ("An" → "A"): Resolved.

No new issues found.

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 74.28571% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.16%. Comparing base (42e198c) to head (1d36f7b).
⚠️ Report is 28 commits behind head on main.

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% 86.16% -0.99% 
==========================================
Files 161 156 -5 Lines 109251 108669 -582 Branches 109251 108669 -582 ==========================================
- Hits 95215 93638 -1577 - Misses 11560 12420 +860 - Partials 2476 2611 +135

Flag	Coverage Δ
fuzzing-fake-hashes	`?`
fuzzing-real-hashes	`?`
tests	`86.16% <74.28%> (-0.06%)`	⬇️

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.

ldk-reviews-bot · 2026-05-02T00:00:47Z

🔔 6th Reminder

Hey @tnull @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-05-02T00:00:51Z

🔔 1st Reminder

Hey @tnull @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-05-04T00:01:44Z

🔔 7th Reminder

Hey @tnull @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-05-04T00:01:48Z

🔔 2nd Reminder

Hey @tnull @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

LGTM, one non-blocking question.

wpaulino · 2026-05-05T22:43:30Z

+ .get_funding_txid()
+ .expect("negotiated candidates should have a funding txid");
+ let contribution = i
+ .checked_sub(contrib_offset)


Isn't the offset always 0 here? We're currently exchanging tx_signatures, which means we can't have a different negotiation in progress and the current one we're exchanging signatures for has already been tracked as a candidate above.

The offset is for indexing into contributions. We may not have contributed to earlier rounds, so contributions may have fewer elements than negotiated_candidates.

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 · 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.

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

I've thoroughly reviewed every file and hunk in this diff, cross-referencing my prior review context and verifying the key invariants.

Verification performed:

Contribution offset math (channel.rs:9386-9401): Correct for all cases. saturating_sub handles the (impossible) case where contributions.len() > negotiated_candidates.len() gracefully. The checked_sub(contrib_offset) + .get(j) properly maps leading no-contribution rounds to None and trailing contribution rounds to the correct index.
Hash derive chain: Complete. FundingContribution (newly Hash) → FundingTxInput (ConfirmedUtxo, newly Hash) → Utxo (already Hash) → bitcoin::Transaction (implements Hash in bitcoin 0.32.x). All intermediate types in the chain implement Hash.
All TransactionType::Splice match sites: Properly updated to TransactionType::InteractiveFunding. No orphaned references remain.
Test assertions in sign_interactive_funding_tx_with_acceptor_contribution: The assert_broadcast closure correctly validates candidate chain structure, replaced-txid contract (via checked_sub(2)), contribution presence, and purpose for both initiator and acceptor across all callers.
New public getters (estimated_fee, inputs, feerate, max_feerate): Straightforward field accessors with no logic errors. feerate elevation from pub(super) to pub is a strict visibility widening with no impact on existing callers.
All test callers pass correct expected_replaced_txid values: None for first-splice, Some(prior_tx.compute_txid()) for RBF rounds.

Prior review comment status:

channel.rs:7256 — 0-conf rebroadcast type mismatch: Pre-existing issue, still applicable (outside this PR's scope).
chaininterface.rs:177 — Dead code TLV serialization impls: Still applicable (minor).
channel.rs:9395 — filter_map silently dropping candidates: Resolved (now uses .map() + .expect()).
chaininterface.rs:123 — Grammar nit ("An" → "A"): Resolved.

No new issues found.

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 74.28571% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.16%. Comparing base (42e198c) to head (1d36f7b).
⚠️ Report is 28 commits behind head on main.

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% 86.16% -0.99% 
==========================================
Files 161 156 -5 Lines 109251 108669 -582 Branches 109251 108669 -582 ==========================================
- Hits 95215 93638 -1577 - Misses 11560 12420 +860 - Partials 2476 2611 +135

Flag	Coverage Δ
fuzzing-fake-hashes	`?`
fuzzing-real-hashes	`?`
tests	`86.16% <74.28%> (-0.06%)`	⬇️

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.

ldk-reviews-bot · 2026-05-02T00:00:47Z

🔔 6th Reminder

Hey @tnull @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-05-02T00:00:51Z

🔔 1st Reminder

Hey @tnull @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-05-04T00:01:44Z

🔔 7th Reminder

Hey @tnull @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-05-04T00:01:48Z

🔔 2nd Reminder

Hey @tnull @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

LGTM, one non-blocking question.

wpaulino · 2026-05-05T22:43:30Z

+ .get_funding_txid()
+ .expect("negotiated candidates should have a funding txid");
+ let contribution = i
+ .checked_sub(contrib_offset)


Isn't the offset always 0 here? We're currently exchanging tx_signatures, which means we can't have a different negotiation in progress and the current one we're exchanging signatures for has already been tracked as a candidate above.

The offset is for indexing into contributions. We may not have contributed to earlier rounds, so contributions may have fewer elements than negotiated_candidates.

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 · 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.

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

I've thoroughly reviewed every file and hunk in this diff, cross-referencing my prior review context and verifying the key invariants.

Verification performed:

Contribution offset math (channel.rs:9386-9401): Correct for all cases. saturating_sub handles the (impossible) case where contributions.len() > negotiated_candidates.len() gracefully. The checked_sub(contrib_offset) + .get(j) properly maps leading no-contribution rounds to None and trailing contribution rounds to the correct index.
Hash derive chain: Complete. FundingContribution (newly Hash) → FundingTxInput (ConfirmedUtxo, newly Hash) → Utxo (already Hash) → bitcoin::Transaction (implements Hash in bitcoin 0.32.x). All intermediate types in the chain implement Hash.
All TransactionType::Splice match sites: Properly updated to TransactionType::InteractiveFunding. No orphaned references remain.
Test assertions in sign_interactive_funding_tx_with_acceptor_contribution: The assert_broadcast closure correctly validates candidate chain structure, replaced-txid contract (via checked_sub(2)), contribution presence, and purpose for both initiator and acceptor across all callers.
New public getters (estimated_fee, inputs, feerate, max_feerate): Straightforward field accessors with no logic errors. feerate elevation from pub(super) to pub is a strict visibility widening with no impact on existing callers.
All test callers pass correct expected_replaced_txid values: None for first-splice, Some(prior_tx.compute_txid()) for RBF rounds.

Prior review comment status:

channel.rs:7256 — 0-conf rebroadcast type mismatch: Pre-existing issue, still applicable (outside this PR's scope).
chaininterface.rs:177 — Dead code TLV serialization impls: Still applicable (minor).
channel.rs:9395 — filter_map silently dropping candidates: Resolved (now uses .map() + .expect()).
chaininterface.rs:123 — Grammar nit ("An" → "A"): Resolved.

No new issues found.

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 74.28571% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.16%. Comparing base (42e198c) to head (1d36f7b).
⚠️ Report is 28 commits behind head on main.

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% 86.16% -0.99% 
==========================================
Files 161 156 -5 Lines 109251 108669 -582 Branches 109251 108669 -582 ==========================================
- Hits 95215 93638 -1577 - Misses 11560 12420 +860 - Partials 2476 2611 +135

Flag	Coverage Δ
fuzzing-fake-hashes	`?`
fuzzing-real-hashes	`?`
tests	`86.16% <74.28%> (-0.06%)`	⬇️

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.

ldk-reviews-bot · 2026-05-02T00:00:47Z

🔔 6th Reminder

Hey @tnull @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-05-02T00:00:51Z

🔔 1st Reminder

Hey @tnull @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-05-04T00:01:44Z

🔔 7th Reminder

Hey @tnull @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-05-04T00:01:48Z

🔔 2nd Reminder

Hey @tnull @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

LGTM, one non-blocking question.

wpaulino · 2026-05-05T22:43:30Z

+ .get_funding_txid()
+ .expect("negotiated candidates should have a funding txid");
+ let contribution = i
+ .checked_sub(contrib_offset)


Isn't the offset always 0 here? We're currently exchanging tx_signatures, which means we can't have a different negotiation in progress and the current one we're exchanging signatures for has already been tracked as a candidate above.

The offset is for indexing into contributions. We may not have contributed to earlier rounds, so contributions may have fewer elements than negotiated_candidates.

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 · 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.

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

I've thoroughly reviewed every file and hunk in this diff, cross-referencing my prior review context and verifying the key invariants.

Verification performed:

Contribution offset math (channel.rs:9386-9401): Correct for all cases. saturating_sub handles the (impossible) case where contributions.len() > negotiated_candidates.len() gracefully. The checked_sub(contrib_offset) + .get(j) properly maps leading no-contribution rounds to None and trailing contribution rounds to the correct index.
Hash derive chain: Complete. FundingContribution (newly Hash) → FundingTxInput (ConfirmedUtxo, newly Hash) → Utxo (already Hash) → bitcoin::Transaction (implements Hash in bitcoin 0.32.x). All intermediate types in the chain implement Hash.
All TransactionType::Splice match sites: Properly updated to TransactionType::InteractiveFunding. No orphaned references remain.
Test assertions in sign_interactive_funding_tx_with_acceptor_contribution: The assert_broadcast closure correctly validates candidate chain structure, replaced-txid contract (via checked_sub(2)), contribution presence, and purpose for both initiator and acceptor across all callers.
New public getters (estimated_fee, inputs, feerate, max_feerate): Straightforward field accessors with no logic errors. feerate elevation from pub(super) to pub is a strict visibility widening with no impact on existing callers.
All test callers pass correct expected_replaced_txid values: None for first-splice, Some(prior_tx.compute_txid()) for RBF rounds.

Prior review comment status:

channel.rs:7256 — 0-conf rebroadcast type mismatch: Pre-existing issue, still applicable (outside this PR's scope).
chaininterface.rs:177 — Dead code TLV serialization impls: Still applicable (minor).
channel.rs:9395 — filter_map silently dropping candidates: Resolved (now uses .map() + .expect()).
chaininterface.rs:123 — Grammar nit ("An" → "A"): Resolved.

No new issues found.

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 74.28571% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.16%. Comparing base (42e198c) to head (1d36f7b).
⚠️ Report is 28 commits behind head on main.

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% 86.16% -0.99% 
==========================================
Files 161 156 -5 Lines 109251 108669 -582 Branches 109251 108669 -582 ==========================================
- Hits 95215 93638 -1577 - Misses 11560 12420 +860 - Partials 2476 2611 +135

Flag	Coverage Δ
fuzzing-fake-hashes	`?`
fuzzing-real-hashes	`?`
tests	`86.16% <74.28%> (-0.06%)`	⬇️

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.

ldk-reviews-bot · 2026-05-02T00:00:47Z

🔔 6th Reminder

Hey @tnull @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-05-02T00:00:51Z

🔔 1st Reminder

Hey @tnull @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-05-04T00:01:44Z

🔔 7th Reminder

Hey @tnull @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-05-04T00:01:48Z

🔔 2nd Reminder

Hey @tnull @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

LGTM, one non-blocking question.

wpaulino · 2026-05-05T22:43:30Z

+ .get_funding_txid()
+ .expect("negotiated candidates should have a funding txid");
+ let contribution = i
+ .checked_sub(contrib_offset)


Isn't the offset always 0 here? We're currently exchanging tx_signatures, which means we can't have a different negotiation in progress and the current one we're exchanging signatures for has already been tracked as a candidate above.

The offset is for indexing into contributions. We may not have contributed to earlier rounds, so contributions may have fewer elements than negotiated_candidates.

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 · 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.

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

I've thoroughly reviewed every file and hunk in this diff, cross-referencing my prior review context and verifying the key invariants.

Verification performed:

Contribution offset math (channel.rs:9386-9401): Correct for all cases. saturating_sub handles the (impossible) case where contributions.len() > negotiated_candidates.len() gracefully. The checked_sub(contrib_offset) + .get(j) properly maps leading no-contribution rounds to None and trailing contribution rounds to the correct index.
Hash derive chain: Complete. FundingContribution (newly Hash) → FundingTxInput (ConfirmedUtxo, newly Hash) → Utxo (already Hash) → bitcoin::Transaction (implements Hash in bitcoin 0.32.x). All intermediate types in the chain implement Hash.
All TransactionType::Splice match sites: Properly updated to TransactionType::InteractiveFunding. No orphaned references remain.
Test assertions in sign_interactive_funding_tx_with_acceptor_contribution: The assert_broadcast closure correctly validates candidate chain structure, replaced-txid contract (via checked_sub(2)), contribution presence, and purpose for both initiator and acceptor across all callers.
New public getters (estimated_fee, inputs, feerate, max_feerate): Straightforward field accessors with no logic errors. feerate elevation from pub(super) to pub is a strict visibility widening with no impact on existing callers.
All test callers pass correct expected_replaced_txid values: None for first-splice, Some(prior_tx.compute_txid()) for RBF rounds.

Prior review comment status:

channel.rs:7256 — 0-conf rebroadcast type mismatch: Pre-existing issue, still applicable (outside this PR's scope).
chaininterface.rs:177 — Dead code TLV serialization impls: Still applicable (minor).
channel.rs:9395 — filter_map silently dropping candidates: Resolved (now uses .map() + .expect()).
chaininterface.rs:123 — Grammar nit ("An" → "A"): Resolved.

No new issues found.

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 74.28571% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.16%. Comparing base (42e198c) to head (1d36f7b).
⚠️ Report is 28 commits behind head on main.

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% 86.16% -0.99% 
==========================================
Files 161 156 -5 Lines 109251 108669 -582 Branches 109251 108669 -582 ==========================================
- Hits 95215 93638 -1577 - Misses 11560 12420 +860 - Partials 2476 2611 +135

Flag	Coverage Δ
fuzzing-fake-hashes	`?`
fuzzing-real-hashes	`?`
tests	`86.16% <74.28%> (-0.06%)`	⬇️

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.

ldk-reviews-bot · 2026-05-02T00:00:47Z

🔔 6th Reminder

Hey @tnull @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-05-02T00:00:51Z

🔔 1st Reminder

Hey @tnull @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-05-04T00:01:44Z

🔔 7th Reminder

Hey @tnull @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-05-04T00:01:48Z

🔔 2nd Reminder

Hey @tnull @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

LGTM, one non-blocking question.

wpaulino · 2026-05-05T22:43:30Z

+ .get_funding_txid()
+ .expect("negotiated candidates should have a funding txid");
+ let contribution = i
+ .checked_sub(contrib_offset)


Isn't the offset always 0 here? We're currently exchanging tx_signatures, which means we can't have a different negotiation in progress and the current one we're exchanging signatures for has already been tracked as a candidate above.

The offset is for indexing into contributions. We may not have contributed to earlier rounds, so contributions may have fewer elements than negotiated_candidates.

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 · 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.

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

I've thoroughly reviewed every file and hunk in this diff, cross-referencing my prior review context and verifying the key invariants.

Verification performed:

Contribution offset math (channel.rs:9386-9401): Correct for all cases. saturating_sub handles the (impossible) case where contributions.len() > negotiated_candidates.len() gracefully. The checked_sub(contrib_offset) + .get(j) properly maps leading no-contribution rounds to None and trailing contribution rounds to the correct index.
Hash derive chain: Complete. FundingContribution (newly Hash) → FundingTxInput (ConfirmedUtxo, newly Hash) → Utxo (already Hash) → bitcoin::Transaction (implements Hash in bitcoin 0.32.x). All intermediate types in the chain implement Hash.
All TransactionType::Splice match sites: Properly updated to TransactionType::InteractiveFunding. No orphaned references remain.
Test assertions in sign_interactive_funding_tx_with_acceptor_contribution: The assert_broadcast closure correctly validates candidate chain structure, replaced-txid contract (via checked_sub(2)), contribution presence, and purpose for both initiator and acceptor across all callers.
New public getters (estimated_fee, inputs, feerate, max_feerate): Straightforward field accessors with no logic errors. feerate elevation from pub(super) to pub is a strict visibility widening with no impact on existing callers.
All test callers pass correct expected_replaced_txid values: None for first-splice, Some(prior_tx.compute_txid()) for RBF rounds.

Prior review comment status:

channel.rs:7256 — 0-conf rebroadcast type mismatch: Pre-existing issue, still applicable (outside this PR's scope).
chaininterface.rs:177 — Dead code TLV serialization impls: Still applicable (minor).
channel.rs:9395 — filter_map silently dropping candidates: Resolved (now uses .map() + .expect()).
chaininterface.rs:123 — Grammar nit ("An" → "A"): Resolved.

No new issues found.

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 74.28571% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.16%. Comparing base (42e198c) to head (1d36f7b).
⚠️ Report is 28 commits behind head on main.

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% 86.16% -0.99% 
==========================================
Files 161 156 -5 Lines 109251 108669 -582 Branches 109251 108669 -582 ==========================================
- Hits 95215 93638 -1577 - Misses 11560 12420 +860 - Partials 2476 2611 +135

Flag	Coverage Δ
fuzzing-fake-hashes	`?`
fuzzing-real-hashes	`?`
tests	`86.16% <74.28%> (-0.06%)`	⬇️

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.

ldk-reviews-bot · 2026-05-02T00:00:47Z

🔔 6th Reminder

Hey @tnull @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-05-02T00:00:51Z

🔔 1st Reminder

Hey @tnull @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-05-04T00:01:44Z

🔔 7th Reminder

Hey @tnull @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-05-04T00:01:48Z

🔔 2nd Reminder

Hey @tnull @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

LGTM, one non-blocking question.

wpaulino · 2026-05-05T22:43:30Z

+ .get_funding_txid()
+ .expect("negotiated candidates should have a funding txid");
+ let contribution = i
+ .checked_sub(contrib_offset)


Isn't the offset always 0 here? We're currently exchanging tx_signatures, which means we can't have a different negotiation in progress and the current one we're exchanging signatures for has already been tracked as a candidate above.

The offset is for indexing into contributions. We may not have contributed to earlier rounds, so contributions may have fewer elements than negotiated_candidates.

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 · 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.

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

I've thoroughly reviewed every file and hunk in this diff, cross-referencing my prior review context and verifying the key invariants.

Verification performed:

Contribution offset math (channel.rs:9386-9401): Correct for all cases. saturating_sub handles the (impossible) case where contributions.len() > negotiated_candidates.len() gracefully. The checked_sub(contrib_offset) + .get(j) properly maps leading no-contribution rounds to None and trailing contribution rounds to the correct index.
Hash derive chain: Complete. FundingContribution (newly Hash) → FundingTxInput (ConfirmedUtxo, newly Hash) → Utxo (already Hash) → bitcoin::Transaction (implements Hash in bitcoin 0.32.x). All intermediate types in the chain implement Hash.
All TransactionType::Splice match sites: Properly updated to TransactionType::InteractiveFunding. No orphaned references remain.
Test assertions in sign_interactive_funding_tx_with_acceptor_contribution: The assert_broadcast closure correctly validates candidate chain structure, replaced-txid contract (via checked_sub(2)), contribution presence, and purpose for both initiator and acceptor across all callers.
New public getters (estimated_fee, inputs, feerate, max_feerate): Straightforward field accessors with no logic errors. feerate elevation from pub(super) to pub is a strict visibility widening with no impact on existing callers.
All test callers pass correct expected_replaced_txid values: None for first-splice, Some(prior_tx.compute_txid()) for RBF rounds.

Prior review comment status:

channel.rs:7256 — 0-conf rebroadcast type mismatch: Pre-existing issue, still applicable (outside this PR's scope).
chaininterface.rs:177 — Dead code TLV serialization impls: Still applicable (minor).
channel.rs:9395 — filter_map silently dropping candidates: Resolved (now uses .map() + .expect()).
chaininterface.rs:123 — Grammar nit ("An" → "A"): Resolved.

No new issues found.

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 74.28571% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.16%. Comparing base (42e198c) to head (1d36f7b).
⚠️ Report is 28 commits behind head on main.

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% 86.16% -0.99% 
==========================================
Files 161 156 -5 Lines 109251 108669 -582 Branches 109251 108669 -582 ==========================================
- Hits 95215 93638 -1577 - Misses 11560 12420 +860 - Partials 2476 2611 +135

Flag	Coverage Δ
fuzzing-fake-hashes	`?`
fuzzing-real-hashes	`?`
tests	`86.16% <74.28%> (-0.06%)`	⬇️

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.

ldk-reviews-bot · 2026-05-02T00:00:47Z

🔔 6th Reminder

Hey @tnull @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-05-02T00:00:51Z

🔔 1st Reminder

Hey @tnull @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-05-04T00:01:44Z

🔔 7th Reminder

Hey @tnull @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-05-04T00:01:48Z

🔔 2nd Reminder

Hey @tnull @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

LGTM, one non-blocking question.

wpaulino · 2026-05-05T22:43:30Z

+ .get_funding_txid()
+ .expect("negotiated candidates should have a funding txid");
+ let contribution = i
+ .checked_sub(contrib_offset)


Isn't the offset always 0 here? We're currently exchanging tx_signatures, which means we can't have a different negotiation in progress and the current one we're exchanging signatures for has already been tracked as a candidate above.

The offset is for indexing into contributions. We may not have contributed to earlier rounds, so contributions may have fewer elements than negotiated_candidates.

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 · 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.

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

I've thoroughly reviewed every file and hunk in this diff, cross-referencing my prior review context and verifying the key invariants.

Verification performed:

Contribution offset math (channel.rs:9386-9401): Correct for all cases. saturating_sub handles the (impossible) case where contributions.len() > negotiated_candidates.len() gracefully. The checked_sub(contrib_offset) + .get(j) properly maps leading no-contribution rounds to None and trailing contribution rounds to the correct index.
Hash derive chain: Complete. FundingContribution (newly Hash) → FundingTxInput (ConfirmedUtxo, newly Hash) → Utxo (already Hash) → bitcoin::Transaction (implements Hash in bitcoin 0.32.x). All intermediate types in the chain implement Hash.
All TransactionType::Splice match sites: Properly updated to TransactionType::InteractiveFunding. No orphaned references remain.
Test assertions in sign_interactive_funding_tx_with_acceptor_contribution: The assert_broadcast closure correctly validates candidate chain structure, replaced-txid contract (via checked_sub(2)), contribution presence, and purpose for both initiator and acceptor across all callers.
New public getters (estimated_fee, inputs, feerate, max_feerate): Straightforward field accessors with no logic errors. feerate elevation from pub(super) to pub is a strict visibility widening with no impact on existing callers.
All test callers pass correct expected_replaced_txid values: None for first-splice, Some(prior_tx.compute_txid()) for RBF rounds.

Prior review comment status:

channel.rs:7256 — 0-conf rebroadcast type mismatch: Pre-existing issue, still applicable (outside this PR's scope).
chaininterface.rs:177 — Dead code TLV serialization impls: Still applicable (minor).
channel.rs:9395 — filter_map silently dropping candidates: Resolved (now uses .map() + .expect()).
chaininterface.rs:123 — Grammar nit ("An" → "A"): Resolved.

No new issues found.

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 74.28571% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.16%. Comparing base (42e198c) to head (1d36f7b).
⚠️ Report is 28 commits behind head on main.

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% 86.16% -0.99% 
==========================================
Files 161 156 -5 Lines 109251 108669 -582 Branches 109251 108669 -582 ==========================================
- Hits 95215 93638 -1577 - Misses 11560 12420 +860 - Partials 2476 2611 +135

Flag	Coverage Δ
fuzzing-fake-hashes	`?`
fuzzing-real-hashes	`?`
tests	`86.16% <74.28%> (-0.06%)`	⬇️

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.

ldk-reviews-bot · 2026-05-02T00:00:47Z

🔔 6th Reminder

Hey @tnull @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-05-02T00:00:51Z

🔔 1st Reminder

Hey @tnull @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-05-04T00:01:44Z

🔔 7th Reminder

Hey @tnull @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-05-04T00:01:48Z

🔔 2nd Reminder

Hey @tnull @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

LGTM, one non-blocking question.

wpaulino · 2026-05-05T22:43:30Z

+ .get_funding_txid()
+ .expect("negotiated candidates should have a funding txid");
+ let contribution = i
+ .checked_sub(contrib_offset)


Isn't the offset always 0 here? We're currently exchanging tx_signatures, which means we can't have a different negotiation in progress and the current one we're exchanging signatures for has already been tracked as a candidate above.

The offset is for indexing into contributions. We may not have contributed to earlier rounds, so contributions may have fewer elements than negotiated_candidates.

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 86dcedeCompare 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 dacf092Compare 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 925b58dApril 29, 2026 20:42

jkczyz force-pushed the 2026-04-splice-transaction-type branch 2 times, most recently from 925b58d to 4b82770Compare 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

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

Comment threadlightning/src/ln/channel.rs Outdated

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

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

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

Comment threadlightning/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 2 times, most recently from f245f31 to d94e8fbCompare April 30, 2026 19:26

TheBlueMatt previously approved these changes Apr 30, 2026
View reviewed changes

tnull previously approved these changes May 5, 2026
View reviewed changes

Comment threadlightning/src/chain/chaininterface.rs
Comment threadlightning/src/chain/chaininterface.rs

jkczyz dismissed stale reviews from tnull and TheBlueMatt via 6e1894bMay 5, 2026 17:46

jkczyz force-pushed the 2026-04-splice-transaction-type branch from d94e8fb to 6e1894bCompare May 5, 2026 17:46

tnull previously approved these changes May 5, 2026
View reviewed changes

wpaulino reviewed May 5, 2026
View reviewed changes

jkczyzand others added 2 commits May 6, 2026 11:01

jkczyz dismissed tnull’s stale review via 1d36f7bMay 6, 2026 16:02

jkczyz force-pushed the 2026-04-splice-transaction-type branch from 6e1894b to 1d36f7bCompare May 6, 2026 16:02

jkczyz requested review from TheBlueMatt and wpaulino May 6, 2026 16:02

wpaulino approved these changes May 6, 2026
View reviewed changes

TheBlueMatt approved these changes May 7, 2026
View reviewed changes

TheBlueMatt merged commit 38f5651 into lightningdevkit:mainMay 7, 2026
23 of 25 checks passed

github-project-automationBot moved this to Done in Weekly Goals May 7, 2026

jkczyz mentioned this pull request May 7, 2026
Support async signing of splice shared input #4579
Merged

jkczyz mentioned this pull request Jun 15, 2026
Splicing / Dual Funding #4284
Open
50 tasks