Allow cancellation of pending splice funding negotiations#4490
Allow cancellation of pending splice funding negotiations#4490wpaulino wants to merge 1 commit intolightningdevkit:mainfrom
Conversation
A user may wish to cancel an in-flight funding negotiation for whatever reason (e.g., mempool feerates have gone down, inability to sign, etc.), so we should make it possible for them to do so. Note that this can only be done for splice funding negotiations for which the user has made a contribution to.
|
👋 Thanks for assigning @jkczyz as a reviewer! |
| let splice_funding_failed = splice_funding_failed | ||
| .expect("Only splices with local contributions can be canceled"); |
There was a problem hiding this comment.
This .expect() can panic in release builds. While cancel_splice in channel.rs verifies made_contribution is true, the maybe_create_splice_funding_failed! macro additionally subtracts contributions that overlap with prior RBF rounds (via prior_contributed_inputs/outputs). For a non-initiator who reuses the same UTXOs across RBF attempts with no explicit output contributions, the subtraction can empty the lists, causing the macro to return None (line 6740-6742 of channel.rs: if !is_initiator && contributed_inputs.is_empty() && contributed_outputs.is_empty() { return None; }).
The debug_assert!(splice_funding_failed.is_some()) at channel.rs:12323 catches this in debug, but this expect will crash in release for that edge case. Consider handling None gracefully, e.g. by returning an APIError or skipping the events.
| FundingNegotiation::AwaitingSignatures { .. } => self | ||
| .context | ||
| .interactive_tx_signing_session | ||
| .as_ref() | ||
| .expect("We have a pending splice awaiting signatures") | ||
| .has_local_contribution(), |
There was a problem hiding this comment.
Nit: the contribution check here (has_local_contribution) differs in how it computes "has contributions" compared to the AwaitingAck/ConstructingTransaction branches. has_local_contribution() uses the signing session's local_inputs_count/local_outputs_count (which filter by is_local on the constructed transaction metadata), while the other branches use contributed_inputs()/contributed_outputs() from the context/constructor (which use our_funding_inputs/our_funding_outputs).
These should be semantically equivalent since both exclude the shared input and reflect local contributions, but the different computation paths mean a divergence in one path wouldn't be caught by the other. Consider adding a comment noting the equivalence, or unifying the approach.
Review SummaryThe approach is sound—making
🤖 Generated with Claude Code |
3 participants
A user may wish to cancel an in-flight funding negotiation for whatever reason (e.g., mempool feerates have gone down, inability to sign, etc.), so we should make it possible for them to do so. Note that this can only be done for splice funding negotiations for which the user has made a contribution to.