From baa25d3a7d7c1816a68fc1dad53e9cceeec141ab Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Jun 2026 11:32:01 -0700 Subject: [PATCH 1/6] Only emit Event::SpliceNegotiated when contributing There's no need to inform users of negotiated splices when they're not contributing as it just produces noise. Once they do start contributing, they cannot stop, so we always emit the event going forward. Note that we still emit `Event::ChannelReady` with the new locked funding outpoint for each locked splice, so users can still learn that a splice occurred that way. --- lightning/src/events/mod.rs | 14 ++-- lightning/src/ln/async_signer_tests.rs | 6 +- lightning/src/ln/channel.rs | 10 +++ lightning/src/ln/channelmanager.rs | 106 ++++++++++++++----------- lightning/src/ln/splicing_tests.rs | 67 ++++++++-------- 5 files changed, 113 insertions(+), 90 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index ec0ad6ccd9b..2e56d35c887 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -1647,8 +1647,12 @@ pub enum Event { /// [`ChainMonitor::get_claimable_balances`]: crate::chain::chainmonitor::ChainMonitor::get_claimable_balances last_local_balance_msat: Option, }, - /// Used to indicate that a splice for the given `channel_id` has been negotiated and its - /// funding transaction has been broadcast. + /// Used to indicate that a splice for the given `channel_id` has been negotiated, its + /// funding transaction has been broadcast, and local inputs or outputs were contributed to + /// it. + /// + /// This event is not emitted if the counterparty negotiated a splice without using a local + /// contribution. /// /// The splice is then considered pending until both parties have seen enough confirmations to /// consider the funding locked. Once this occurs, an [`Event::ChannelReady`] will be emitted. @@ -1679,9 +1683,9 @@ pub enum Event { }, /// Used to indicate that a splice negotiation round for the given `channel_id` has failed. /// - /// Each splice attempt (initial or RBF) resolves to either [`Event::SpliceNegotiated`] on - /// success or this event on failure. Prior successfully negotiated splice transactions are - /// unaffected. + /// Each splice attempt (initial or RBF) resolves to this event on failure. On success, + /// [`Event::SpliceNegotiated`] is emitted if the negotiated transaction includes local + /// inputs or outputs. Prior successfully negotiated splice transactions are unaffected. /// /// Any UTXOs contributed to the failed round that are not committed to a prior negotiated /// splice transaction will be returned via a preceding [`Event::DiscardFunding`]. diff --git a/lightning/src/ln/async_signer_tests.rs b/lightning/src/ln/async_signer_tests.rs index f36c19748f0..f60e63a87e9 100644 --- a/lightning/src/ln/async_signer_tests.rs +++ b/lightning/src/ln/async_signer_tests.rs @@ -1853,7 +1853,7 @@ fn test_async_splice_initial_commit_sig() { acceptor.node.handle_tx_signatures(initiator_node_id, &tx_signatures); let _ = get_event!(initiator, Event::SpliceNegotiated); - let _ = get_event!(acceptor, Event::SpliceNegotiated); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); } #[test] @@ -1945,7 +1945,7 @@ fn test_async_splice_initial_commit_sig_waits_for_monitor_before_tx_signatures() acceptor.node.handle_tx_signatures(initiator_node_id, &tx_signatures); let _ = get_event!(initiator, Event::SpliceNegotiated); - let _ = get_event!(acceptor, Event::SpliceNegotiated); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); } #[test] @@ -2022,5 +2022,5 @@ fn test_async_splice_shared_input_signature_released_on_unblock() { acceptor.node.handle_tx_signatures(initiator_node_id, &tx_signatures); let _ = get_event!(initiator, Event::SpliceNegotiated); - let _ = get_event!(acceptor, Event::SpliceNegotiated); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index d0fc940eb62..7b1ebc81577 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7182,6 +7182,9 @@ pub struct SpliceFundingNegotiated { /// The outpoint of the channel's splice funding transaction. pub funding_txo: bitcoin::OutPoint, + /// Whether the holder contributed local inputs or outputs to the negotiated splice. + pub has_local_contribution: bool, + /// The features that this channel will operate with. pub channel_type: ChannelTypeFeatures, @@ -9545,11 +9548,18 @@ where funding.get_funding_txo().expect("funding outpoint should be set"); let channel_type = funding.get_channel_type().clone(); let funding_redeem_script = funding.get_funding_redeemscript(); + let has_local_contribution = self + .context + .interactive_tx_signing_session + .as_ref() + .map(|signing_session| signing_session.has_local_contribution()) + .unwrap_or(false); pending_splice.negotiated_candidates.push(funding); let splice_negotiated = SpliceFundingNegotiated { funding_txo: funding_txo.into_bitcoin_outpoint(), + has_local_contribution, channel_type, funding_redeem_script, }; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 2d00b1d1098..920021c40da 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -6781,8 +6781,9 @@ impl< /// /// Calling this method will commence the process of creating a new funding transaction for the /// channel. Once the funding transaction has been constructed, an [`Event::SpliceNegotiated`] - /// will be emitted. At this point, any inputs contributed to the splice can only be re-spent - /// if an [`Event::DiscardFunding`] is seen. + /// will be emitted if the negotiated transaction includes local inputs or outputs. At this + /// point, any inputs contributed to the splice can only be re-spent if an + /// [`Event::DiscardFunding`] is seen. /// /// If any failures occur while negotiating the funding transaction, an /// [`Event::SpliceNegotiationFailed`] will be emitted. Any contributed inputs no longer used @@ -7005,18 +7006,20 @@ impl< ); } if let Some(splice_negotiated) = splice_negotiated { - self.pending_events.lock().unwrap().push_back(( - events::Event::SpliceNegotiated { - channel_id: *channel_id, - counterparty_node_id: *counterparty_node_id, - user_channel_id: chan.context().get_user_id(), - new_funding_txo: splice_negotiated.funding_txo, - channel_type: splice_negotiated.channel_type, - new_funding_redeem_script: splice_negotiated - .funding_redeem_script, - }, - None, - )); + if splice_negotiated.has_local_contribution { + self.pending_events.lock().unwrap().push_back(( + events::Event::SpliceNegotiated { + channel_id: *channel_id, + counterparty_node_id: *counterparty_node_id, + user_channel_id: chan.context().get_user_id(), + new_funding_txo: splice_negotiated.funding_txo, + channel_type: splice_negotiated.channel_type, + new_funding_redeem_script: splice_negotiated + .funding_redeem_script, + }, + None, + )); + } } if chan.context().is_connected() { @@ -11199,17 +11202,19 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ .as_mut() .and_then(|v| v.splice_negotiated.take()) { - pending_events.push_back(( - events::Event::SpliceNegotiated { - channel_id: channel.context.channel_id(), - counterparty_node_id, - user_channel_id: channel.context.get_user_id(), - new_funding_txo: splice_negotiated.funding_txo, - channel_type: splice_negotiated.channel_type, - new_funding_redeem_script: splice_negotiated.funding_redeem_script, - }, - None, - )); + if splice_negotiated.has_local_contribution { + pending_events.push_back(( + events::Event::SpliceNegotiated { + channel_id: channel.context.channel_id(), + counterparty_node_id, + user_channel_id: channel.context.get_user_id(), + new_funding_txo: splice_negotiated.funding_txo, + channel_type: splice_negotiated.channel_type, + new_funding_redeem_script: splice_negotiated.funding_redeem_script, + }, + None, + )); + } } } @@ -12299,18 +12304,20 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ // which also terminates quiescence. let needs_holding_cell_release = splice_negotiated.is_some(); if let Some(splice_negotiated) = splice_negotiated { - self.pending_events.lock().unwrap().push_back(( - events::Event::SpliceNegotiated { - channel_id: msg.channel_id, - counterparty_node_id: *counterparty_node_id, - user_channel_id: chan.context.get_user_id(), - new_funding_txo: splice_negotiated.funding_txo, - channel_type: splice_negotiated.channel_type, - new_funding_redeem_script: splice_negotiated - .funding_redeem_script, - }, - None, - )); + if splice_negotiated.has_local_contribution { + self.pending_events.lock().unwrap().push_back(( + events::Event::SpliceNegotiated { + channel_id: msg.channel_id, + counterparty_node_id: *counterparty_node_id, + user_channel_id: chan.context.get_user_id(), + new_funding_txo: splice_negotiated.funding_txo, + channel_type: splice_negotiated.channel_type, + new_funding_redeem_script: splice_negotiated + .funding_redeem_script, + }, + None, + )); + } } let holding_cell_res = if needs_holding_cell_release { self.check_free_peer_holding_cells(peer_state) @@ -14161,17 +14168,20 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ .and_then(|funding_tx_signed| funding_tx_signed.splice_negotiated.take()) { *needs_holding_cell_release = true; - self.pending_events.lock().unwrap().push_back(( - events::Event::SpliceNegotiated { - channel_id, - counterparty_node_id: node_id, - user_channel_id: funded_chan.context.get_user_id(), - new_funding_txo: splice_negotiated.funding_txo, - channel_type: splice_negotiated.channel_type, - new_funding_redeem_script: splice_negotiated.funding_redeem_script, - }, - None, - )); + if splice_negotiated.has_local_contribution { + self.pending_events.lock().unwrap().push_back(( + events::Event::SpliceNegotiated { + channel_id, + counterparty_node_id: node_id, + user_channel_id: funded_chan.context.get_user_id(), + new_funding_txo: splice_negotiated.funding_txo, + channel_type: splice_negotiated.channel_type, + new_funding_redeem_script: splice_negotiated + .funding_redeem_script, + }, + None, + )); + } } if let Some(broadcast_tx) = msgs.signed_closing_tx { log_info!(logger, "Broadcasting closing tx {}", log_tx!(broadcast_tx)); diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index f480c4e9bc0..c201e25dff0 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -702,7 +702,6 @@ pub fn splice_channel<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, funding_contribution: FundingContribution, ) -> (Transaction, ScriptBuf) { - let node_id_initiator = initiator.node.get_our_node_id(); let node_id_acceptor = acceptor.node.get_our_node_id(); let new_funding_script = complete_splice_handshake(initiator, acceptor); @@ -718,7 +717,7 @@ pub fn splice_channel<'a, 'b, 'c, 'd>( assert!(splice_locked.is_none()); expect_splice_pending_event(initiator, &node_id_acceptor); - expect_splice_pending_event(acceptor, &node_id_initiator); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); (splice_tx, new_funding_script) } @@ -1749,7 +1748,7 @@ fn fails_initiating_concurrent_splices(reconnect: bool) { assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_1_id); - expect_splice_pending_event(&nodes[1], &node_0_id); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // Now that the splice is pending, another splice may be initiated. assert!(nodes[0].node.splice_channel(&channel_id, &node_1_id).is_ok()); @@ -2023,7 +2022,7 @@ fn do_test_splice_tiebreak( assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); mine_transaction(&nodes[0], &tx); mine_transaction(&nodes[1], &tx); @@ -2070,7 +2069,7 @@ fn do_test_splice_tiebreak( assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[1], &node_id_0); - expect_splice_pending_event(&nodes[0], &node_id_1); + assert!(nodes[0].node.get_and_clear_pending_events().is_empty()); mine_transaction(&nodes[1], &new_splice_tx); mine_transaction(&nodes[0], &new_splice_tx); @@ -2536,7 +2535,7 @@ fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) { reconnect_nodes!(|reconnect_args: &mut ReconnectArgs| { reconnect_args.send_interactive_tx_sigs = (false, true); }); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // Reestablish the channel again to make sure node 0 doesn't retransmit `tx_signatures` // unnecessarily as it was delivered in the previous reestablishment. @@ -2930,7 +2929,7 @@ fn test_splice_reestablish_waits_for_holder_tx_signatures_before_commitment_sign nodes[1].node.handle_tx_signatures(node_id_0, &initiator_tx_signatures); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); } #[test] @@ -3034,7 +3033,7 @@ fn test_splice_reestablish_sends_commitment_signed_before_tx_signatures() { nodes[1].node.handle_tx_signatures(node_id_0, &initiator_tx_signatures); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); } #[test] @@ -4023,7 +4022,7 @@ fn acceptor_can_cancel_queued_funding_contributed_during_counterparty_splice() { let (splice_tx, splice_locked) = sign_interactive_funding_tx(initiator, acceptor, false, None); assert!(splice_locked.is_none()); expect_splice_pending_event(initiator, &node_id_acceptor); - expect_splice_pending_event(acceptor, &node_id_initiator); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); mine_transaction(initiator, &splice_tx); mine_transaction(acceptor, &splice_tx); @@ -4422,7 +4421,7 @@ fn free_holding_cell_on_tx_signatures_quiescence_exit() { } expect_splice_pending_event(initiator, &node_id_acceptor); - expect_splice_pending_event(acceptor, &node_id_initiator); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); } #[test] @@ -4902,7 +4901,7 @@ fn test_splice_buffer_commitment_signed_until_funding_tx_signed() { } expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // Both nodes should broadcast the splice transaction. let splice_tx = { @@ -5144,7 +5143,7 @@ fn do_splice_waits_for_initial_commitment_monitor_update_before_releasing_tx_sig expect_splice_pending_event(&nodes[0], &node_id_1); if !complete_update_while_disconnected { - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); } } @@ -6067,7 +6066,7 @@ fn test_splice_rbf_acceptor_basic() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); let initial_channel_value_sat = 100_000; @@ -6116,7 +6115,7 @@ fn test_splice_rbf_acceptor_basic() { assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // Step 11: Mine, lock, and verify DiscardFunding for the replaced splice candidate. let result = lock_rbf_splice_after_blocks( @@ -6148,7 +6147,7 @@ fn test_splice_rbf_discard_unique_contribution() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); let initial_channel_value_sat = 100_000; @@ -6217,7 +6216,7 @@ fn test_splice_rbf_discard_unique_contribution() { assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); let result = lock_rbf_splice_after_blocks( &nodes[0], @@ -6249,7 +6248,7 @@ fn test_splice_rbf_at_high_feerate() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); let initial_channel_value_sat = 100_000; @@ -6284,7 +6283,7 @@ fn test_splice_rbf_at_high_feerate() { ); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // Step 3: RBF again using the template's min_rbf_feerate. The counterparty must accept it. provide_utxo_reserves(&nodes, 2, added_value * 2); @@ -6305,7 +6304,7 @@ fn test_splice_rbf_at_high_feerate() { sign_interactive_funding_tx(&nodes[0], &nodes[1], false, Some(rbf_tx_1.compute_txid())); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); } #[test] @@ -6506,7 +6505,7 @@ fn test_splice_rbf_insufficient_feerate_high() { sign_interactive_funding_tx(&nodes[0], &nodes[1], false, Some(splice_tx.compute_txid())); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // prev=1000: flat increment gives 1000+25=1025, 25/24 rule gives 1000*25/24=1041. // Feerate 1025 satisfies the flat increment but not 25/24 — rejected. @@ -7218,7 +7217,7 @@ pub fn do_test_splice_rbf_tiebreak( assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // Mine, lock, and verify DiscardFunding for the replaced splice candidate. // Node 1's QuiescentAction was preserved, so after splice_locked it re-initiates @@ -7281,7 +7280,7 @@ pub fn do_test_splice_rbf_tiebreak( assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[1], &node_id_0); - expect_splice_pending_event(&nodes[0], &node_id_1); + assert!(nodes[0].node.get_and_clear_pending_events().is_empty()); // Mine and lock. mine_transaction(&nodes[1], &new_splice_tx); @@ -7780,7 +7779,7 @@ fn test_splice_rbf_sequential() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); let initial_channel_value_sat = 100_000; @@ -7818,7 +7817,7 @@ fn test_splice_rbf_sequential() { sign_interactive_funding_tx(&nodes[0], &nodes[1], false, Some(splice_tx_0.compute_txid())); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // --- Round 2: RBF #2 at feerate 303. --- provide_utxo_reserves(&nodes, 2, added_value * 2); @@ -7839,7 +7838,7 @@ fn test_splice_rbf_sequential() { sign_interactive_funding_tx(&nodes[0], &nodes[1], false, Some(splice_tx_1.compute_txid())); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // --- Mine and lock the final RBF, verifying DiscardFunding for both replaced candidates. --- let splice_tx_0_txid = splice_tx_0.compute_txid(); @@ -7865,7 +7864,7 @@ fn test_splice_rbf_amends_prior_net_positive_contribution_request() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); let (_, _, channel_id, _) = @@ -7908,7 +7907,7 @@ fn test_splice_rbf_amends_prior_net_positive_contribution_request() { sign_interactive_funding_tx(&nodes[0], &nodes[1], false, Some(replaced_txid)); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); tx }; @@ -7997,7 +7996,7 @@ fn test_splice_rbf_amends_prior_net_negative_contribution_request() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); let (_, _, channel_id, _) = @@ -8042,7 +8041,7 @@ fn test_splice_rbf_amends_prior_net_negative_contribution_request() { sign_interactive_funding_tx(&nodes[0], &nodes[1], false, Some(replaced_txid)); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); tx }; @@ -9088,7 +9087,7 @@ fn test_splice_rbf_rejects_low_feerate_after_several_attempts() { ); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); prev_feerate = feerate; prev_splice_tx = rbf_tx; } @@ -9120,7 +9119,7 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); let initial_channel_value_sat = 100_000; @@ -9163,7 +9162,7 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() { ); assert!(splice_locked.is_none()); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); prev_feerate = feerate; prev_splice_tx = rbf_tx; } @@ -9233,10 +9232,10 @@ fn test_no_disconnect_after_splice_completes() { let (_, splice_locked) = sign_interactive_funding_tx(&nodes[0], &nodes[1], false, None); assert!(splice_locked.is_none()); - let node_id_0 = nodes[0].node.get_our_node_id(); + let _node_id_0 = nodes[0].node.get_our_node_id(); let node_id_1 = nodes[1].node.get_our_node_id(); expect_splice_pending_event(&nodes[0], &node_id_1); - expect_splice_pending_event(&nodes[1], &node_id_0); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); // Fire enough ticks to trigger a disconnect if the timer wasn't properly cleared. for _ in 0..DISCONNECT_PEER_AWAITING_RESPONSE_TICKS { From c671f20472bc2c98215de4fc066d828e52d2a6d0 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Tue, 16 Jun 2026 12:00:35 -0700 Subject: [PATCH 2/6] Always emit SpliceNegotiationFailed when contributing Previously, this could result in an acceptor not receiving a `Event::SpliceNegotiationFailed` for a splice in which they reused the same contribution (except for the feerate change). Our API should guarantee that users should always see `SpliceNegotiated` and `SpliceNegotiationFailed` events for splices that they contribute to. --- lightning/src/ln/channel.rs | 47 +++++++----------------------- lightning/src/ln/splicing_tests.rs | 18 +++--------- 2 files changed, 15 insertions(+), 50 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 7b1ebc81577..27ed66e46a5 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -7224,8 +7224,7 @@ impl SpliceFundingFailed { } macro_rules! splice_funding_failed_for { - ($self: expr, $is_initiator: expr, $contribution: expr, - $contributed_inputs: ident, $contributed_outputs: ident) => {{ + ($self: expr, $contribution: expr, $contributed_inputs: ident, $contributed_outputs: ident) => {{ let contribution = $contribution; let existing_inputs = $self.pending_splice.as_ref().into_iter().flat_map(|ps| ps.$contributed_inputs()); @@ -7234,17 +7233,16 @@ macro_rules! splice_funding_failed_for { let filtered = contribution.clone().into_unique_contributions(existing_inputs, existing_outputs); match filtered { - None if !$is_initiator => None, - None => Some(SpliceFundingFailed { + None => SpliceFundingFailed { contributed_inputs: vec![], contributed_outputs: vec![], contribution: Some(contribution), - }), - Some((contributed_inputs, contributed_outputs)) => Some(SpliceFundingFailed { + }, + Some((contributed_inputs, contributed_outputs)) => SpliceFundingFailed { contributed_inputs, contributed_outputs, contribution: Some(contribution), - }), + }, } }}; } @@ -7277,14 +7275,7 @@ where fn splice_funding_failed_for(&self, contribution: FundingContribution) -> SpliceFundingFailed { // The contribution was never pushed to `contributions`, so `contributed_inputs()` and // `contributed_outputs()` return only prior rounds' entries for filtering. - splice_funding_failed_for!( - self, - true, - contribution, - contributed_inputs, - contributed_outputs - ) - .expect("is_initiator is true so this always returns Some") + splice_funding_failed_for!(self, contribution, contributed_inputs, contributed_outputs) } fn abandon_quiescent_action(&mut self) -> Option { @@ -7426,11 +7417,7 @@ where pending_splice.funding_negotiation.is_some(), "reset_pending_splice_state requires an active funding negotiation" ); - let is_initiator = pending_splice - .funding_negotiation - .take() - .map(|negotiation| negotiation.is_initiator()) - .unwrap_or(false); + pending_splice.funding_negotiation.take(); let contribution = pending_splice.contributions.pop(); if let Some(ref contribution) = contribution { debug_assert!( @@ -7444,14 +7431,8 @@ where // After pop, `contributed_inputs()` / `contributed_outputs()` return only prior // rounds for filtering. - let splice_funding_failed = contribution.and_then(|contribution| { - splice_funding_failed_for!( - self, - is_initiator, - contribution, - contributed_inputs, - contributed_outputs - ) + let splice_funding_failed = contribution.map(|contribution| { + splice_funding_failed_for!(self, contribution, contributed_inputs, contributed_outputs) }); if self.pending_funding().is_empty() { @@ -7476,19 +7457,13 @@ where pending_splice.funding_negotiation.is_some(), "maybe_splice_funding_failed requires an active funding negotiation" ); - let is_initiator = pending_splice - .funding_negotiation - .as_ref() - .map(|negotiation| negotiation.is_initiator()) - .unwrap_or(false); let contribution = pending_splice.contributions.last().cloned()?; - splice_funding_failed_for!( + Some(splice_funding_failed_for!( self, - is_initiator, contribution, prior_contributed_inputs, prior_contributed_outputs - ) + )) } #[rustfmt::skip] diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index c201e25dff0..cd5464e9fb9 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -8227,23 +8227,13 @@ fn test_splice_rbf_acceptor_contributes_then_disconnects() { // The initiator re-used the same UTXOs as round 0. Since those UTXOs are still committed // to round 0's splice, they are filtered and no DiscardFunding is emitted. - let events = nodes[0].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 1, "{events:?}"); - match &events[0] { - Event::SpliceNegotiationFailed { channel_id: cid, reason, contribution, .. } => { - assert_eq!(*cid, channel_id); - assert_eq!(*reason, NegotiationFailureReason::PeerDisconnected); - assert!(contribution.is_some()); - }, - other => panic!("Expected SpliceNegotiationFailed, got {:?}", other), - } + let _ = get_event!(&nodes[0], Event::SpliceNegotiationFailed); // The acceptor re-contributed the same UTXOs as round 0 (via prior contribution // adjustment). Since those UTXOs are still committed to round 0's splice, they are - // filtered and no DiscardFunding is emitted. With all inputs/outputs filtered, no events - // are emitted for the acceptor. - let events = nodes[1].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 0, "{events:?}"); + // filtered and no DiscardFunding is emitted. The contribution still fails and needs a + // SpliceNegotiationFailed event so the wallet can resume funding. + let _ = get_event!(&nodes[1], Event::SpliceNegotiationFailed); // Reconnect. let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); From 3856a6478482b4993dbb18828164e1641c0de538 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Jun 2026 09:34:15 -0700 Subject: [PATCH 3/6] Allow invalid contribution error upon quiescence While a contribution may be valid at the time the splice is requested, quiescence still needs to happen, which can affect the balances of the channel as it fully settles all pending state. After doing so, it's possible that the contribution is no longer valid. Since quiescence itself doesn't have a terminal message, we see a `WarnAndDisconnect` event happen. --- fuzz/src/chanmon_consistency.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index d8291571884..7c956263127 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -936,7 +936,8 @@ fn assert_disconnect_action(action: &msgs::ErrorAction) -> (&msgs::WarningMessag // Since sending/receiving messages may be delayed, `timer_tick_occurred` may cause a node to // disconnect their counterparty if they're expecting a timely response. if let msgs::ErrorAction::DisconnectPeerWithWarning { ref msg } = action { - let is_quiescent_msg = msg.data.contains("already sent splice_locked, cannot RBF"); + let is_quiescent_msg = msg.data.contains("already sent splice_locked, cannot RBF") + || msg.data.contains("contribution no longer valid at quiescence"); if !msg.data.contains("Disconnecting due to timeout awaiting response") && !is_quiescent_msg { panic!("Unexpected disconnect case: {}", msg.data); From 1c9c3b516b4f17729716762956a8cf30a8abdb3e Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Tue, 16 Jun 2026 14:40:10 -0700 Subject: [PATCH 4/6] Prefer tx_abort over disconnection for splice negotiation errors We keep some `WarnAndDisconnect` cases as mandated by the spec, but otherwise prefer sending `tx_abort` to terminate quiescence and avoid reconnection loops. --- lightning/src/ln/channel.rs | 95 ++++++++++++++--------------- lightning/src/ln/channelmanager.rs | 2 +- lightning/src/ln/interactivetxs.rs | 7 ++- lightning/src/ln/splicing_tests.rs | 98 ++++++++++++++---------------- 4 files changed, 99 insertions(+), 103 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 27ed66e46a5..21d7d3830a2 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -13133,11 +13133,13 @@ where /// Checks during handling splice_init pub fn validate_splice_init(&self, msg: &msgs::SpliceInit) -> Result<(), ChannelError> { - if self.holder_commitment_point.current_point().is_none() { - return Err(ChannelError::WarnAndDisconnect(format!( - "Channel {} commitment point needs to be advanced once before spliced", - self.context.channel_id(), - ))); + // - If it has received shutdown: + // MUST send a warning and close the connection or send an error + // and fail the channel. + if !self.context.is_live() { + return Err(ChannelError::WarnAndDisconnect( + "Splicing requested on a channel that is not live".to_owned(), + )); } if !self.context.channel_state.is_quiescent() { @@ -13152,15 +13154,6 @@ where ))); } - // - If it has received shutdown: - // MUST send a warning and close the connection or send an error - // and fail the channel. - if !self.context.is_live() { - return Err(ChannelError::WarnAndDisconnect( - "Splicing requested on a channel that is not live".to_owned(), - )); - } - let their_funding_contribution = SignedAmount::from_sat(msg.funding_contribution_satoshis); if their_funding_contribution == SignedAmount::ZERO { return Err(ChannelError::WarnAndDisconnect(format!( @@ -13169,6 +13162,12 @@ where ))); } + if self.holder_commitment_point.current_point().is_none() { + return Err(ChannelError::Abort(AbortReason::InternalError( + "Commitment point needs to be advanced once before spliced".into(), + ))); + } + Ok(()) } @@ -13185,13 +13184,10 @@ where counterparty_funding_pubkey, our_new_holder_keys, min_funding_satoshis, - ) - .map_err(|e| format!("Channel {} cannot be spliced; {}", self.context.channel_id(), e))?; + )?; let (post_splice_holder_balance, post_splice_counterparty_balance) = - self.get_holder_counterparty_balances_floor_incl_fee(&candidate_scope).map_err( - |e| format!("Channel {} cannot be spliced; {}", self.context.channel_id(), e), - )?; + self.get_holder_counterparty_balances_floor_incl_fee(&candidate_scope)?; let holder_selected_channel_reserve = Amount::from_sat(candidate_scope.holder_selected_channel_reserve_satoshis); @@ -13201,25 +13197,23 @@ where // We allow parties to draw from their previous reserve, as long as they satisfy their v2 reserve if our_funding_contribution != SignedAmount::ZERO { - post_splice_holder_balance.checked_sub(counterparty_selected_channel_reserve) - .ok_or(format!( - "Channel {} cannot be {}; our post-splice channel balance {} is smaller than their selected v2 reserve {}", - self.context.channel_id(), - if our_funding_contribution.is_positive() { "spliced in" } else { "spliced out" }, - post_splice_holder_balance, - counterparty_selected_channel_reserve, - ))?; + post_splice_holder_balance.checked_sub(counterparty_selected_channel_reserve).ok_or( + format!( + "Our post-splice channel balance {} is smaller than their selected v2 reserve {}", + post_splice_holder_balance, + counterparty_selected_channel_reserve, + ), + )?; } if their_funding_contribution != SignedAmount::ZERO { - post_splice_counterparty_balance.checked_sub(holder_selected_channel_reserve) - .ok_or(format!( - "Channel {} cannot be {}; their post-splice channel balance {} is smaller than our selected v2 reserve {}", - self.context.channel_id(), - if their_funding_contribution.is_positive() { "spliced in" } else { "spliced out" }, - post_splice_counterparty_balance, - holder_selected_channel_reserve, - ))?; + post_splice_counterparty_balance.checked_sub(holder_selected_channel_reserve).ok_or( + format!( + "Their post-splice channel balance {} is smaller than our selected v2 reserve {}", + post_splice_counterparty_balance, + holder_selected_channel_reserve, + ), + )?; } #[cfg(debug_assertions)] @@ -13330,7 +13324,11 @@ where holder_pubkeys, min_funding_satoshis, ) - .map_err(|e| self.quiescent_negotiation_err(ChannelError::WarnAndDisconnect(e)))?; + .map_err(|e| { + self.quiescent_negotiation_err(ChannelError::Abort( + AbortReason::InvalidContribution(e), + )) + })?; // Adjust for the feerate and clone so we can store it for future RBF re-use. let (adjusted_contribution, our_funding_inputs, our_funding_outputs) = @@ -13389,17 +13387,16 @@ where fn validate_tx_init_rbf( &self, msg: &msgs::TxInitRbf, fee_estimator: &LowerBoundedFeeEstimator, ) -> Result<(ChannelPublicKeys, PublicKey), ChannelError> { - if self.holder_commitment_point.current_point().is_none() { - return Err(ChannelError::WarnAndDisconnect(format!( - "Channel {} commitment point needs to be advanced once before RBF", - self.context.channel_id(), - ))); - } - if !self.context.channel_state.is_quiescent() { return Err(ChannelError::WarnAndDisconnect("Quiescence needed for RBF".to_owned())); } + if self.holder_commitment_point.current_point().is_none() { + return Err(ChannelError::Abort(AbortReason::InternalError( + "Commitment point needs to be advanced once before RBF".into(), + ))); + } + self.is_rbf_compatible().map_err(|msg| ChannelError::WarnAndDisconnect(msg))?; let pending_splice = match &self.pending_splice { @@ -13513,7 +13510,11 @@ where holder_pubkeys, min_funding_satoshis, ) - .map_err(|e| self.quiescent_negotiation_err(ChannelError::WarnAndDisconnect(e)))?; + .map_err(|e| { + self.quiescent_negotiation_err(ChannelError::Abort( + AbortReason::InvalidContribution(e), + )) + })?; // Consume the appropriate contribution source. let (our_funding_inputs, our_funding_outputs) = if queued_net_value.is_some() { @@ -13613,7 +13614,7 @@ where holder_pubkeys, min_funding_satoshis, ) - .map_err(|e| ChannelError::WarnAndDisconnect(e))?; + .map_err(|e| ChannelError::Abort(AbortReason::InvalidContribution(e)))?; Ok(new_funding) } @@ -13690,8 +13691,6 @@ where fn validate_splice_ack( &self, msg: &msgs::SpliceAck, min_funding_satoshis: u64, ) -> Result { - // TODO(splicing): Add check that we are the splice (quiescence) initiator - let pending_splice = self .pending_splice .as_ref() @@ -13714,7 +13713,7 @@ where new_keys, min_funding_satoshis, ) - .map_err(|e| ChannelError::WarnAndDisconnect(e))?; + .map_err(|e| ChannelError::Abort(AbortReason::InvalidContribution(e)))?; Ok(new_funding) } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 920021c40da..c4d6c48e99a 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1147,7 +1147,7 @@ impl MsgHandleErrInternal { fn from_chan_no_close(err: ChannelError, channel_id: ChannelId) -> Self { let tx_abort = match &err { - &ChannelError::Abort(reason) => Some(reason.into_tx_abort_msg(channel_id)), + ChannelError::Abort(reason) => Some(reason.clone().into_tx_abort_msg(channel_id)), _ => None, }; let err = match err { diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index dfb702a2657..a0e325abf35 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -91,7 +91,7 @@ impl SerialIdExt for SerialId { } } -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Debug, Clone, PartialEq)] pub(crate) enum AbortReason { InvalidStateTransition, UnexpectedCounterpartyMessage, @@ -142,6 +142,8 @@ pub(crate) enum AbortReason { /// /// [`ChannelManager::cancel_funding_contributed`]: crate::ln::channelmanager::ChannelManager::cancel_funding_contributed ManualIntervention, + /// The contribution is not valid given the current balances of the channel. + InvalidContribution(String), /// Internal error InternalError(&'static str), } @@ -209,6 +211,9 @@ impl Display for AbortReason { f.write_str("The initiator's feerate exceeds our maximum") }, AbortReason::ManualIntervention => f.write_str("Manually aborted funding negotiation"), + AbortReason::InvalidContribution(text) => { + f.write_fmt(format_args!("Invalid contribution: {}", text)) + }, AbortReason::InternalError(text) => { f.write_fmt(format_args!("Internal error: {}", text)) }, diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index cd5464e9fb9..5623097dd6f 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -174,23 +174,21 @@ fn config_with_min_funding_satoshis(min_funding_satoshis: u64) -> UserConfig { } #[cfg(test)] -fn assert_min_funding_error<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, min_funding_satoshis: u64) { - let msg_events = node.node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1, "{msg_events:?}"); - match &msg_events[0] { - MessageSendEvent::HandleError { - action: msgs::ErrorAction::DisconnectPeerWithWarning { msg }, - .. - } => { - assert!( - msg.data - .contains(&format!("configured min_funding_satoshis {min_funding_satoshis}")), - "unexpected warning: {}", - msg.data - ); - }, - _ => panic!("Expected HandleError with warning, got {:?}", msg_events[0]), - } +fn assert_min_funding_error<'a, 'b, 'c>( + node: &Node<'a, 'b, 'c>, recipient: PublicKey, min_funding_satoshis: u64, +) { + let msg = get_event_msg!(node, MessageSendEvent::SendTxAbort, recipient); + let data = tx_abort_data(&msg); + assert!( + data.contains(&format!("configured min_funding_satoshis {min_funding_satoshis}")), + "unexpected tx_abort: {}", + data + ); +} + +#[cfg(test)] +fn tx_abort_data(msg: &msgs::TxAbort) -> String { + String::from_utf8(msg.data.clone()).expect("tx_abort data should be valid UTF-8") } pub fn negotiate_splice_tx<'a, 'b, 'c, 'd>( @@ -1373,7 +1371,7 @@ fn test_min_funding_satoshis_rejects_splice_init_with_negative_counterparty_cont let splice_init = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceInit, node_id_1); assert!(splice_init.funding_contribution_satoshis < 0); nodes[1].node.handle_splice_init(node_id_0, &splice_init); - assert_min_funding_error(&nodes[1], min_funding_satoshis); + assert_min_funding_error(&nodes[1], node_id_0, min_funding_satoshis); } #[test] @@ -1471,7 +1469,7 @@ fn test_min_funding_satoshis_rejects_splice_ack_with_negative_counterparty_contr let splice_ack = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceAck, node_id_0); assert!(splice_ack.funding_contribution_satoshis < 0); nodes[0].node.handle_splice_ack(node_id_1, &splice_ack); - assert_min_funding_error(&nodes[0], min_funding_satoshis); + assert_min_funding_error(&nodes[0], node_id_1, min_funding_satoshis); } #[test] @@ -1513,7 +1511,7 @@ fn test_min_funding_satoshis_rejects_tx_init_rbf_with_negative_counterparty_cont let tx_init_rbf = get_event_msg!(nodes[0], MessageSendEvent::SendTxInitRbf, node_id_1); assert!(tx_init_rbf.funding_output_contribution.unwrap() < 0); nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); - assert_min_funding_error(&nodes[1], min_funding_satoshis); + assert_min_funding_error(&nodes[1], node_id_0, min_funding_satoshis); } #[test] @@ -1570,7 +1568,7 @@ fn test_min_funding_satoshis_rejects_tx_ack_rbf_with_negative_counterparty_contr let tx_ack_rbf = get_event_msg!(nodes[1], MessageSendEvent::SendTxAckRbf, node_id_0); assert!(tx_ack_rbf.funding_output_contribution.unwrap() < 0); nodes[0].node.handle_tx_ack_rbf(node_id_1, &tx_ack_rbf); - assert_min_funding_error(&nodes[0], min_funding_satoshis); + assert_min_funding_error(&nodes[0], node_id_1, min_funding_satoshis); } #[test] @@ -5807,13 +5805,14 @@ fn do_test_splice_pending_htlcs(config: UserConfig) { splice_init.funding_contribution_satoshis -= 1; acceptor.node.handle_splice_init(node_id_initiator, &splice_init); - let msg = get_warning_msg(acceptor, &node_id_initiator); + let msg = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); assert_eq!(msg.channel_id, channel_id); let cannot_be_spliced_out = format!( - "Channel {} cannot be spliced out; their post-splice channel balance {} is smaller than our selected v2 reserve {}", - channel_id, post_splice_reserve - Amount::ONE_SAT, post_splice_reserve + "Their post-splice channel balance {} is smaller than our selected v2 reserve {}", + post_splice_reserve - Amount::ONE_SAT, + post_splice_reserve ); - assert_eq!(msg.data, cannot_be_spliced_out); + assert_eq!(tx_abort_data(&msg), format!("Invalid contribution: {cannot_be_spliced_out}")); acceptor.node.peer_disconnected(node_id_initiator); initiator.node.peer_disconnected(node_id_acceptor); @@ -9717,40 +9716,35 @@ fn do_test_0reserve_splice_counterparty_validation( get_event_msg!(acceptor, MessageSendEvent::SendSpliceAck, node_id_initiator); } else { acceptor.node.handle_splice_init(node_id_initiator, &splice_init); - let msg_events = acceptor.node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1); - if let MessageSendEvent::HandleError { action, .. } = &msg_events[0] { - assert!(matches!(action, msgs::ErrorAction::DisconnectPeerWithWarning { .. })); - } else { - panic!("Expected MessageSendEvent::HandleError"); - } + let msg = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); + assert_eq!(msg.channel_id, channel_id); let cannot_splice_out = if u64::try_from(funding_contribution_sat.abs()).unwrap() > initiator_value_to_self_sat { // They obviously can't afford their contribution, so we fail before even // querying `TxBuilder` format!( - "Got non-closing error: Channel {channel_id} cannot be spliced; \ - Their contribution candidate {funding_contribution_sat}sat \ + "Their contribution candidate {funding_contribution_sat}sat \ is greater than their total balance in the channel {initiator_value_to_self_sat}sat" ) } else if post_channel_value_sat < MIN_CHANNEL_VALUE_SATOSHIS { // We require all spliced channels to have a value of at least 1000 satoshis after the splice format!( - "Got non-closing error: Channel {channel_id} cannot be spliced; \ - Spliced channel value must be at least {MIN_CHANNEL_VALUE_SATOSHIS} satoshis. \ + "Spliced channel value must be at least {MIN_CHANNEL_VALUE_SATOSHIS} satoshis. \ It would be {post_channel_value_sat}" ) } else { // Last but not least, `TxBuilder` decides whether all parties can afford // HTLCs, anchors, and transaction fees while retaining at least one // output on the commitments - format!( - "Got non-closing error: Channel {channel_id} cannot \ - be spliced; Balance exhausted on local commitment" - ) + "Balance exhausted on local commitment".to_string() }; - acceptor.logger.assert_log("lightning::ln::channelmanager", cannot_splice_out, 1); + assert_eq!(tx_abort_data(&msg), format!("Invalid contribution: {cannot_splice_out}")); + acceptor.logger.assert_log( + "lightning::ln::channelmanager", + format!("Got non-closing error: Invalid contribution: {cannot_splice_out}"), + 1, + ); } channel_type @@ -9991,18 +9985,12 @@ fn do_test_splice_out_initiator_reserve_breach_zero_fee_commitments( // balance, we previously would not complain. splice_init.funding_contribution_satoshis = funding_contribution_sat; acceptor.node.handle_splice_init(node_id_initiator, &splice_init); - let msg_events = acceptor.node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1); - if let MessageSendEvent::HandleError { action, .. } = &msg_events[0] { - assert!(matches!(action, msgs::ErrorAction::DisconnectPeerWithWarning { .. })); - } else { - panic!("Expected MessageSendEvent::HandleError"); - } + let msg = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); + assert_eq!(msg.channel_id, channel_id); let post_splice_channel_value_sat = node_0_balance_leftover_amount.to_sat(); let cannot_splice_out = if matches!(acceptor_balance, AcceptorBalance::NoBalance) { format!( - "Got non-closing error: Channel {channel_id} cannot \ - be spliced; The post-splice channel value {post_splice_channel_value_sat} \ + "The post-splice channel value {post_splice_channel_value_sat} \ is smaller than their dust limit {high_dust_limit_satoshis}" ) } else { @@ -10015,13 +10003,17 @@ fn do_test_splice_out_initiator_reserve_breach_zero_fee_commitments( high_dust_limit_satoshis ); format!( - "Got non-closing error: Channel {channel_id} cannot \ - be spliced out; their post-splice channel balance \ + "Their post-splice channel balance \ {node_0_balance_leftover_amount} is smaller than our selected v2 reserve \ {v2_channel_reserve}" ) }; - acceptor.logger.assert_log("lightning::ln::channelmanager", cannot_splice_out, 1); + assert_eq!(tx_abort_data(&msg), format!("Invalid contribution: {cannot_splice_out}")); + acceptor.logger.assert_log( + "lightning::ln::channelmanager", + format!("Got non-closing error: Invalid contribution: {cannot_splice_out}"), + 1, + ); } } From 397f2c42611286533558223b2a9fbbb613fb9abe Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Jun 2026 09:35:09 -0700 Subject: [PATCH 5/6] Prefer tx_abort over disconnection for inability to RBF Send `tx_abort` to terminate quiescence and avoid reconnection loops. --- lightning/src/ln/channel.rs | 51 ++++++++++++------------ lightning/src/ln/interactivetxs.rs | 5 +++ lightning/src/ln/splicing_tests.rs | 63 ++++++------------------------ 3 files changed, 41 insertions(+), 78 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 21d7d3830a2..e6429769b62 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -13397,46 +13397,41 @@ where ))); } - self.is_rbf_compatible().map_err(|msg| ChannelError::WarnAndDisconnect(msg))?; + self.is_rbf_compatible() + .map_err(|msg| ChannelError::Abort(AbortReason::RbfUnavailable(msg)))?; - let pending_splice = match &self.pending_splice { - Some(pending_splice) => pending_splice, - None => { - return Err(ChannelError::WarnAndDisconnect(format!( - "Channel {} has no pending splice to RBF", - self.context.channel_id(), - ))); - }, - }; + let (pending_splice, last_candidate) = self + .pending_splice + .as_ref() + .filter(|pending_splice| !pending_splice.negotiated_candidates.is_empty()) + .map(|pending_splice| { + ( + pending_splice, + pending_splice.negotiated_candidates.last().expect("checked above"), + ) + }) + .ok_or_else(|| { + ChannelError::Abort(AbortReason::RbfUnavailable( + "No pending splice available to RBF".into(), + )) + })?; if pending_splice.funding_negotiation.is_some() { return Err(ChannelError::Abort(AbortReason::NegotiationInProgress)); } if pending_splice.received_funding_txid.is_some() { - return Err(ChannelError::WarnAndDisconnect(format!( - "Channel {} counterparty already sent splice_locked, cannot RBF", - self.context.channel_id(), + return Err(ChannelError::Abort(AbortReason::RbfUnavailable( + "Already received splice_locked".into(), ))); } if pending_splice.sent_funding_txid.is_some() { - return Err(ChannelError::WarnAndDisconnect(format!( - "Channel {} already sent splice_locked, cannot RBF", - self.context.channel_id(), + return Err(ChannelError::Abort(AbortReason::RbfUnavailable( + "Already sent splice_locked".into(), ))); } - let last_candidate = match pending_splice.negotiated_candidates.last() { - Some(candidate) => candidate, - None => { - return Err(ChannelError::WarnAndDisconnect(format!( - "Channel {} has no negotiated splice candidates to RBF", - self.context.channel_id(), - ))); - }, - }; - let prev_feerate = pending_splice.last_funding_feerate_sat_per_1000_weight.unwrap_or_else(|| { fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::UrgentOnChainSweep) @@ -13601,7 +13596,9 @@ where }; let last_candidate = pending_splice.negotiated_candidates.last().ok_or_else(|| { - ChannelError::WarnAndDisconnect("No negotiated splice candidates for RBF".to_owned()) + ChannelError::Abort(AbortReason::RbfUnavailable( + "No pending splice available to RBF".into(), + )) })?; let holder_pubkeys = last_candidate.get_holder_pubkeys().clone(); let counterparty_funding_pubkey = *last_candidate.counterparty_funding_pubkey(); diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index a0e325abf35..6769e2de3e5 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -144,6 +144,8 @@ pub(crate) enum AbortReason { ManualIntervention, /// The contribution is not valid given the current balances of the channel. InvalidContribution(String), + /// A RBF is not available at this time. + RbfUnavailable(String), /// Internal error InternalError(&'static str), } @@ -214,6 +216,9 @@ impl Display for AbortReason { AbortReason::InvalidContribution(text) => { f.write_fmt(format_args!("Invalid contribution: {}", text)) }, + AbortReason::RbfUnavailable(text) => { + f.write_fmt(format_args!("Rejecting RBF attempt: {}", text)) + }, AbortReason::InternalError(text) => { f.write_fmt(format_args!("Internal error: {}", text)) }, diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 5623097dd6f..8beae18a662 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -6581,22 +6581,11 @@ fn test_splice_rbf_no_pending_splice() { nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); - let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1); - match &msg_events[0] { - MessageSendEvent::HandleError { action, .. } => { - assert_eq!( - *action, - msgs::ErrorAction::DisconnectPeerWithWarning { - msg: msgs::WarningMessage { - channel_id, - data: format!("Channel {} has no pending splice to RBF", channel_id), - }, - } - ); - }, - _ => panic!("Expected HandleError, got {:?}", msg_events[0]), - } + let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); + assert_eq!( + tx_abort_data(&tx_abort), + "Rejecting RBF attempt: No pending splice available to RBF" + ); } #[test] @@ -6694,25 +6683,8 @@ fn test_splice_rbf_after_splice_locked() { nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); - let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1); - match &msg_events[0] { - MessageSendEvent::HandleError { action, .. } => { - assert_eq!( - *action, - msgs::ErrorAction::DisconnectPeerWithWarning { - msg: msgs::WarningMessage { - channel_id, - data: format!( - "Channel {} counterparty already sent splice_locked, cannot RBF", - channel_id, - ), - }, - } - ); - }, - _ => panic!("Expected HandleError, got {:?}", msg_events[0]), - } + let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); + assert_eq!(tx_abort_data(&tx_abort), "Rejecting RBF attempt: Already received splice_locked"); } #[test] @@ -6895,22 +6867,11 @@ fn test_splice_rbf_zeroconf_rejected() { nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); - let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1); - match &msg_events[0] { - MessageSendEvent::HandleError { action, .. } => { - assert_eq!( - *action, - msgs::ErrorAction::DisconnectPeerWithWarning { - msg: msgs::WarningMessage { - channel_id, - data: format!("Channel {} has option_zeroconf, cannot RBF", channel_id,), - }, - } - ); - }, - _ => panic!("Expected HandleError, got {:?}", msg_events[0]), - } + let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); + assert_eq!( + tx_abort_data(&tx_abort), + format!("Rejecting RBF attempt: Channel {} has option_zeroconf, cannot RBF", channel_id) + ); } #[test] From a120ac16c4ba67f16742b75eee4ae12943821d1c Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 17 Jun 2026 10:36:34 -0700 Subject: [PATCH 6/6] Check channel is live while handling counterparty tx_init_rbf This mirrors what we do for counterparty `splice_init` messages, making sure we don't accept RBFs once a channel has requested shutdown. --- lightning/src/ln/channel.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e6429769b62..b906e6f8545 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -13387,6 +13387,11 @@ where fn validate_tx_init_rbf( &self, msg: &msgs::TxInitRbf, fee_estimator: &LowerBoundedFeeEstimator, ) -> Result<(ChannelPublicKeys, PublicKey), ChannelError> { + if !self.context.is_live() { + return Err(ChannelError::WarnAndDisconnect( + "RBF requested on a channel that is not live".to_owned(), + )); + } if !self.context.channel_state.is_quiescent() { return Err(ChannelError::WarnAndDisconnect("Quiescence needed for RBF".to_owned())); }