diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 1ca067f43f4..2cd1b044e2a 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1686,27 +1686,20 @@ where let logger = WithChannelContext::from(logger, &self.context(), None); log_info!(logger, "Failed interactive transaction negotiation: {reason}"); - let _interactive_tx_constructor = match &mut self.phase { + match &mut self.phase { ChannelPhase::Undefined => unreachable!(), - ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => None, + ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => {}, ChannelPhase::UnfundedV2(pending_v2_channel) => { - pending_v2_channel.interactive_tx_constructor.take() + pending_v2_channel.interactive_tx_constructor.take(); + }, + ChannelPhase::Funded(funded_channel) => { + if funded_channel.should_reset_pending_splice_funding_negotiation().unwrap_or(true) + { + funded_channel.reset_pending_splice_state(); + } else { + debug_assert!(false, "We should never fail an interactive funding negotiation once we're exchanging tx_signatures"); + } }, - ChannelPhase::Funded(funded_channel) => funded_channel - .pending_splice - .as_mut() - .and_then(|pending_splice| pending_splice.funding_negotiation.take()) - .and_then(|funding_negotiation| { - if let FundingNegotiation::ConstructingTransaction { - interactive_tx_constructor, - .. - } = funding_negotiation - { - Some(interactive_tx_constructor) - } else { - None - } - }), }; reason.into_tx_abort_msg(self.context().channel_id) @@ -1818,11 +1811,11 @@ where where L::Target: Logger, { - // This checks for and resets the interactive negotiation state by `take()`ing it from the channel. - // The existence of the `tx_constructor` indicates that we have not moved into the signing - // phase for this interactively constructed transaction and hence we have not exchanged - // `tx_signatures`. Either way, we never close the channel upon receiving a `tx_abort`: - // https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L574-L576 + // If we have not sent a `tx_abort` message for this negotiation previously, we need to echo + // back a tx_abort message according to the spec: + // https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L560-L561 + // For rationale why we echo back `tx_abort`: + // https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L578-L580 let should_ack = match &mut self.phase { ChannelPhase::Undefined => unreachable!(), ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => { @@ -1832,19 +1825,27 @@ where ChannelPhase::UnfundedV2(pending_v2_channel) => { pending_v2_channel.interactive_tx_constructor.take().is_some() }, - ChannelPhase::Funded(funded_channel) => funded_channel - .pending_splice - .as_mut() - .and_then(|pending_splice| pending_splice.funding_negotiation.take()) - .is_some(), + ChannelPhase::Funded(funded_channel) => { + if let Some(should_reset) = + funded_channel.should_reset_pending_splice_funding_negotiation() + { + if should_reset { + // We may have still tracked the pending funding negotiation state, so we + // should ack with our own `tx_abort`. + funded_channel.reset_pending_splice_state() + } else { + return Err(ChannelError::close( + "Received tx_abort while awaiting tx_signatures exchange".to_owned(), + )); + } + } else { + // We were not tracking the pending funding negotiation state anymore, likely + // due to a disconnection or already having sent our own `tx_abort`. + false + } + }, }; - // NOTE: Since at this point we have not sent a `tx_abort` message for this negotiation - // previously (tx_constructor was `Some`), we need to echo back a tx_abort message according - // to the spec: - // https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L560-L561 - // For rationale why we echo back `tx_abort`: - // https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L578-L580 Ok(should_ack.then(|| { let logger = WithChannelContext::from(logger, &self.context(), None); let reason = @@ -2554,6 +2555,15 @@ impl FundingNegotiation { } impl PendingFunding { + fn can_abandon_funding_negotiation(&self) -> bool { + self.funding_negotiation + .as_ref() + .map(|funding_negotiation| { + !matches!(funding_negotiation, FundingNegotiation::AwaitingSignatures { .. }) + }) + .unwrap_or(true) + } + fn check_get_splice_locked( &mut self, context: &ChannelContext, confirmed_funding_index: usize, height: u32, ) -> Option @@ -6739,6 +6749,45 @@ where ) } + /// Returns `None` if there is no [`FundedChannel::pending_splice`], otherwise a boolean + /// indicating whether we should reset the splice's [`PendingFunding::funding_negotiation`]. + fn should_reset_pending_splice_funding_negotiation(&self) -> Option { + self.pending_splice.as_ref().map(|pending_splice| { + if pending_splice.can_abandon_funding_negotiation() { + true + } else { + self.context + .interactive_tx_signing_session + .as_ref() + .map(|signing_session| !signing_session.has_received_commitment_signed()) + .unwrap_or_else(|| { + debug_assert!(false); + false + }) + } + }) + } + + fn should_reset_pending_splice_state(&self) -> bool { + self.should_reset_pending_splice_funding_negotiation().unwrap_or(true) + && self.pending_funding().is_empty() + } + + fn reset_pending_splice_state(&mut self) -> bool { + debug_assert!(self.should_reset_pending_splice_funding_negotiation().unwrap_or(true)); + self.context.channel_state.clear_quiescent(); + self.context.interactive_tx_signing_session.take(); + let has_funding_negotiation = self + .pending_splice + .as_mut() + .and_then(|pending_splice| pending_splice.funding_negotiation.take()) + .is_some(); + if self.should_reset_pending_splice_state() { + self.pending_splice.take(); + } + has_funding_negotiation + } + #[rustfmt::skip] fn check_remote_fee( channel_type: &ChannelTypeFeatures, fee_estimator: &LowerBoundedFeeEstimator, @@ -8864,7 +8913,14 @@ where } self.context.channel_state.clear_local_stfu_sent(); self.context.channel_state.clear_remote_stfu_sent(); - self.context.channel_state.clear_quiescent(); + if self.should_reset_pending_splice_funding_negotiation().unwrap_or(true) { + // If we were in quiescence but a splice was never negotiated, or the negotiation + // failed due to disconnecting, we shouldn't be quiescent anymore upon reconnecting. + // If there was a pending splice negotiation that has failed due to disconnecting, + // we also take the opportunity to clean up our state. + self.reset_pending_splice_state(); + debug_assert!(!self.context.channel_state.is_quiescent()); + } } self.context.channel_state.set_peer_disconnected(); @@ -13875,7 +13931,12 @@ where } channel_state.clear_local_stfu_sent(); channel_state.clear_remote_stfu_sent(); - channel_state.clear_quiescent(); + if self.should_reset_pending_splice_funding_negotiation().unwrap_or(true) { + // If we were in quiescence but a splice was never negotiated, or the + // negotiation failed due to disconnecting, we shouldn't be quiescent + // anymore upon reconnecting. + channel_state.clear_quiescent(); + } }, ChannelState::FundingNegotiated(_) if self.context.interactive_tx_signing_session.is_some() => {}, @@ -14238,6 +14299,20 @@ where let holder_commitment_point_next = self.holder_commitment_point.next_point(); let holder_commitment_point_pending_next = self.holder_commitment_point.pending_next_point; + let interactive_tx_signing_session = + if self.should_reset_pending_splice_funding_negotiation().unwrap_or(false) { + None + } else { + self.context.interactive_tx_signing_session.as_ref() + }; + let pending_splice = if self.should_reset_pending_splice_state() { + None + } else { + // We don't have to worry about resetting the pending `FundingNegotiation` because we + // can only read `FundingNegotiation::AwaitingSignatures` variants anyway. + self.pending_splice.as_ref() + }; + write_tlv_fields!(writer, { (0, self.context.announcement_sigs, option), // minimum_depth and counterparty_selected_channel_reserve_satoshis used to have a @@ -14281,12 +14356,12 @@ where (53, funding_tx_broadcast_safe_event_emitted, option), // Added in 0.0.124 (55, removed_htlc_attribution_data, optional_vec), // Added in 0.2 (57, holding_cell_attribution_data, optional_vec), // Added in 0.2 - (58, self.context.interactive_tx_signing_session, option), // Added in 0.2 + (58, interactive_tx_signing_session, option), // Added in 0.2 (59, self.funding.minimum_depth_override, option), // Added in 0.2 (60, self.context.historical_scids, optional_vec), // Added in 0.2 (61, fulfill_attribution_data, optional_vec), // Added in 0.2 (63, holder_commitment_point_current, option), // Added in 0.2 - (64, self.pending_splice, option), // Added in 0.2 + (64, pending_splice, option), // Added in 0.2 (65, self.quiescent_action, option), // Added in 0.2 (67, pending_outbound_held_htlc_flags, optional_vec), // Added in 0.2 (69, holding_cell_held_htlc_flags, optional_vec), // Added in 0.2 @@ -14950,6 +15025,15 @@ where } }; + if let Some(funding_negotiation) = pending_splice + .as_ref() + .and_then(|pending_splice| pending_splice.funding_negotiation.as_ref()) + { + if !matches!(funding_negotiation, FundingNegotiation::AwaitingSignatures { .. }) { + return Err(DecodeError::InvalidValue); + } + } + Ok(FundedChannel { funding: FundingScope { value_to_self_msat, diff --git a/lightning/src/ln/quiescence_tests.rs b/lightning/src/ln/quiescence_tests.rs index 983fd8bd2d9..63c50094a67 100644 --- a/lightning/src/ln/quiescence_tests.rs +++ b/lightning/src/ln/quiescence_tests.rs @@ -7,6 +7,7 @@ use crate::ln::functional_test_utils::*; use crate::ln::msgs; use crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler, ErrorAction, MessageSendEvent}; use crate::util::errors::APIError; +use crate::util::ser::Writeable; use crate::util::test_channel_signer::SignerOp; #[test] @@ -699,3 +700,62 @@ fn test_quiescence_during_disconnection() { do_test_quiescence_during_disconnection(false, true); do_test_quiescence_during_disconnection(true, true); } + +#[test] +fn test_quiescence_termination_on_disconnect() { + do_test_quiescence_termination_on_disconnect(false); + do_test_quiescence_termination_on_disconnect(true); +} + +fn do_test_quiescence_termination_on_disconnect(reload: bool) { + // Tests that we terminate quiescence on disconnect if there's no quiescence protocol taking place. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let (persister_0a, persister_1a); + let (chain_monitor_0a, chain_monitor_1a); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let (node_0a, node_1a); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + 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, _) = create_announced_chan_between_nodes(&nodes, 0, 1); + + nodes[0].node.maybe_propose_quiescence(&node_id_1, &channel_id).unwrap(); + + let stfu = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + nodes[1].node.handle_stfu(node_id_0, &stfu); + let stfu = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + nodes[0].node.handle_stfu(node_id_1, &stfu); + + if reload { + let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); + reload_node!( + nodes[0], + &nodes[0].node.encode(), + &[&encoded_monitor_0], + persister_0a, + chain_monitor_0a, + node_0a + ); + let encoded_monitor_1 = get_monitor!(nodes[1], channel_id).encode(); + reload_node!( + nodes[1], + &nodes[1].node.encode(), + &[&encoded_monitor_1], + persister_1a, + chain_monitor_1a, + node_1a + ); + } else { + nodes[0].node.peer_disconnected(node_id_1); + nodes[1].node.peer_disconnected(node_id_0); + } + + let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + reconnect_args.send_channel_ready = (true, true); + reconnect_nodes(reconnect_args); + + send_payment(&nodes[0], &[&nodes[1]], 1_000_000); +} diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 1537a36cc8b..88271d71930 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -19,6 +19,7 @@ use crate::ln::funding::{FundingTxInput, SpliceContribution}; use crate::ln::msgs::{self, BaseMessageHandler, ChannelMessageHandler, MessageSendEvent}; use crate::ln::types::ChannelId; use crate::util::errors::APIError; +use crate::util::ser::Writeable; use bitcoin::{Amount, OutPoint as BitcoinOutPoint, ScriptBuf, Transaction, TxOut}; @@ -315,6 +316,195 @@ fn lock_splice_after_blocks<'a, 'b, 'c, 'd>( node_b.chain_source.remove_watched_txn_and_outputs(prev_funding_outpoint, prev_funding_script); } +#[test] +fn test_splice_state_reset_on_disconnect() { + do_test_splice_state_reset_on_disconnect(false); + do_test_splice_state_reset_on_disconnect(true); +} + +fn do_test_splice_state_reset_on_disconnect(reload: bool) { + // Tests that we're able to forget our pending splice state after a disconnect such that we can + // retry later. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let (persister_0a, persister_0b, persister_0c, persister_1a, persister_1b, persister_1c); + let ( + chain_monitor_0a, + chain_monitor_0b, + chain_monitor_0c, + chain_monitor_1a, + chain_monitor_1b, + chain_monitor_1c, + ); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let (node_0a, node_0b, node_0c, node_1a, node_1b, node_1c); + let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs); + + 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, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 50_000_000); + + let contribution = SpliceContribution::SpliceOut { + outputs: vec![TxOut { + value: Amount::from_sat(1_000), + script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), + }], + }; + nodes[0] + .node + .splice_channel( + &channel_id, + &node_id_1, + contribution.clone(), + FEERATE_FLOOR_SATS_PER_KW, + None, + ) + .unwrap(); + + // Attempt a splice negotiation that only goes up to receiving `splice_init`. Reconnecting + // should implicitly abort the negotiation and reset the splice state such that we're able to + // retry another splice later. + let stfu = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + nodes[1].node.handle_stfu(node_id_0, &stfu); + let stfu = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + nodes[0].node.handle_stfu(node_id_1, &stfu); + + let splice_init = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceInit, node_id_1); + nodes[1].node.handle_splice_init(node_id_0, &splice_init); + let _ = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceAck, node_id_0); + + if reload { + let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); + reload_node!( + nodes[0], + &nodes[0].node.encode(), + &[&encoded_monitor_0], + persister_0a, + chain_monitor_0a, + node_0a + ); + let encoded_monitor_1 = get_monitor!(nodes[1], channel_id).encode(); + reload_node!( + nodes[1], + &nodes[1].node.encode(), + &[&encoded_monitor_1], + persister_1a, + chain_monitor_1a, + node_1a + ); + } else { + nodes[0].node.peer_disconnected(node_id_1); + nodes[1].node.peer_disconnected(node_id_0); + } + + let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + reconnect_args.send_channel_ready = (true, true); + reconnect_nodes(reconnect_args); + + nodes[0] + .node + .splice_channel( + &channel_id, + &node_id_1, + contribution.clone(), + FEERATE_FLOOR_SATS_PER_KW, + None, + ) + .unwrap(); + + // Attempt a splice negotiation that only goes up to exchanging `tx_complete`. Reconnecting + // should implicitly abort the negotiation and reset the splice state such that we're able to + // retry another splice later. + let stfu = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + nodes[1].node.handle_stfu(node_id_0, &stfu); + let stfu = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + nodes[0].node.handle_stfu(node_id_1, &stfu); + + let splice_init = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceInit, node_id_1); + nodes[1].node.handle_splice_init(node_id_0, &splice_init); + let splice_ack = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceAck, node_id_0); + nodes[0].node.handle_splice_ack(node_id_1, &splice_ack); + + let new_funding_script = chan_utils::make_funding_redeemscript( + &splice_init.funding_pubkey, + &splice_ack.funding_pubkey, + ) + .to_p2wsh(); + let _ = complete_interactive_funding_negotiation( + &nodes[0], + &nodes[1], + channel_id, + contribution.clone(), + new_funding_script, + ); + + if reload { + let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); + reload_node!( + nodes[0], + &nodes[0].node.encode(), + &[&encoded_monitor_0], + persister_0b, + chain_monitor_0b, + node_0b + ); + let encoded_monitor_1 = get_monitor!(nodes[1], channel_id).encode(); + reload_node!( + nodes[1], + &nodes[1].node.encode(), + &[&encoded_monitor_1], + persister_1b, + chain_monitor_1b, + node_1b + ); + } else { + nodes[0].node.peer_disconnected(node_id_1); + nodes[1].node.peer_disconnected(node_id_0); + } + + let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + reconnect_args.send_channel_ready = (true, true); + reconnect_nodes(reconnect_args); + + // Attempt a splice negotiation that completes, (i.e. `tx_signatures` are exchanged). Reconnecting + // should not abort the negotiation or reset the splice state. + let splice_tx = splice_channel(&nodes[0], &nodes[1], channel_id, contribution); + + if reload { + let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); + reload_node!( + nodes[0], + &nodes[0].node.encode(), + &[&encoded_monitor_0], + persister_0c, + chain_monitor_0c, + node_0c + ); + let encoded_monitor_1 = get_monitor!(nodes[1], channel_id).encode(); + reload_node!( + nodes[1], + &nodes[1].node.encode(), + &[&encoded_monitor_1], + persister_1c, + chain_monitor_1c, + node_1c + ); + } else { + nodes[0].node.peer_disconnected(node_id_1); + nodes[1].node.peer_disconnected(node_id_0); + } + + let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + reconnect_args.send_channel_ready = (true, true); + reconnect_nodes(reconnect_args); + + mine_transaction(&nodes[0], &splice_tx); + mine_transaction(&nodes[1], &splice_tx); + lock_splice_after_blocks(&nodes[0], &nodes[1], channel_id, ANTI_REORG_DELAY - 1); +} + #[test] fn test_splice_in() { let chanmon_cfgs = create_chanmon_cfgs(2);