Add splice details to TransactionType::Splice#4570
Add splice details to TransactionType::Splice#4570jkczyz wants to merge 2 commits intolightningdevkit:mainfrom
TransactionType::Splice#4570Conversation
|
👋 Thanks for assigning @TheBlueMatt as a reviewer! |
|
I've thoroughly reviewed the entire diff, examining:
No new issues found beyond what was covered in the prior review pass. The pre-existing concern about the rebroadcast path after 0-conf splice promotion (where |
|
🔔 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
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
Needs rebase, also ci is very sad.
`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>
jkczyz
force-pushed
the
2026-04-splice-transaction-type
branch
from
86dcede to
dacf092
Compare
|
Rebased. Compilation failures were from new tests in main. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4570 +/- ##
==========================================
- Coverage 87.08% 87.08% -0.01%
==========================================
Files 161 161
Lines 109255 109271 +16
Branches 109255 109271 +16
==========================================
+ Hits 95147 95154 +7
- Misses 11627 11632 +5
- Partials 2481 2485 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
🔔 3rd Reminder Hey @TheBlueMatt @wpaulino! This PR has been waiting for your review. |
4 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).