Add splice details to TransactionType::Splice#4570
Add splice details to TransactionType::Splice#4570jkczyz wants to merge 2 commits intolightningdevkit:mainfrom
TransactionType::Splice#4570Conversation
`TransactionType::Splice` now carries the local contribution to the splice and, for RBF, the txid of the prior negotiated candidate being replaced. This lets LDK Node update its `PaymentDetails` and `PendingPaymentDetails` from the broadcast callback without re-deriving the contribution from the raw transaction or tracking RBF chains itself. Driving these updates from the `SplicePending` event instead is more complicated because that event races with BDK wallet syncing and doesn't carry the replaced txid; the broadcast callback is a cleaner integration point. The `Hash` derive on `TransactionType` is dropped since it isn't used anywhere in the workspace and `FundingContribution` (now embedded) transitively contains `ConfirmedUtxo`, which doesn't derive `Hash`. 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>
|
👋 Thanks for assigning @wpaulino as a reviewer! |
Review SummaryNo new issues found beyond what was covered in the prior review pass. Analysis of the changes:
The prior review's concern about post-0-conf-promotion rebroadcast using |
|
🔔 1st Reminder Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review. |
1 similar comment
|
🔔 1st Reminder Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review. |
|
🔔 2nd Reminder Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review. |
1 similar comment
|
🔔 2nd Reminder Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review. |
| 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, | ||
| )], | ||
| }, | ||
| )); | ||
| } | ||
| } |
There was a problem hiding this comment.
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 isChannelReady)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.
There was a problem hiding this comment.
Discussed offline. Dropped the commit to retry for now. We may do this later in ChannelMonitor where we'll store the FundingContribution.
jkczyz
force-pushed
the
2026-04-splice-transaction-type
branch
from
dbaf6d9 to
86dcede
Compare
3 participants
Surface splice broadcast metadata downstream, so consumers (notably LDK Node) can reconcile
PaymentDetails/PendingPaymentDetailsdirectly from the broadcaster callback.TransactionType::Splicenow carriescontribution: Option<FundingContribution>— our local contribution for this round, andreplaced_txid: Option<Txid>— the prior negotiated candidate being replaced on RBF. The alternative (reacting toSplicePending) is worse: that event races with BDK wallet sync and doesn't carry the replaced txid.FundingContributiongains public getters forestimated_fee,inputs, andmax_feerate;feerateis elevated frompub(super)topub. Combined with existingvalue_added,outputs,change_output, this exposes everything a downstream consumer needs without reaching into the raw transaction.The
Hashderive onTransactionTypeis dropped (unused in the workspace;FundingContributiontransitively containsConfirmedUtxowhich lacksHash).