diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 56d38d5545c..e124680e68b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2443,6 +2443,44 @@ impl PendingSplice { } } +pub(crate) struct SpliceInstructions { + our_funding_contribution_satoshis: i64, + our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, + change_script: Option, + funding_feerate_per_kw: u32, + locktime: u32, +} + +impl_writeable_tlv_based!(SpliceInstructions, { + (1, our_funding_contribution_satoshis, required), + (3, our_funding_inputs, required_vec), + (5, change_script, option), + (7, funding_feerate_per_kw, required), + (9, locktime, required), +}); + +pub(crate) enum QuiescentAction { + Splice(SpliceInstructions), + #[cfg(any(test, fuzzing))] + DoNothing, +} + +pub(crate) enum StfuResponse { + Stfu(msgs::Stfu), + #[cfg_attr(not(splicing), allow(unused))] + SpliceInit(msgs::SpliceInit), +} + +#[cfg(any(test, fuzzing))] +impl_writeable_tlv_based_enum_upgradable!(QuiescentAction, + (0, DoNothing) => {}, + {1, Splice} => (), +); +#[cfg(not(any(test, fuzzing)))] +impl_writeable_tlv_based_enum_upgradable!(QuiescentAction,, + {1, Splice} => (), +); + /// Wrapper around a [`Transaction`] useful for caching the result of [`Transaction::compute_txid`]. struct ConfirmedTransaction<'a> { tx: &'a Transaction, @@ -2743,9 +2781,11 @@ where /// store it here and only release it to the `ChannelManager` once it asks for it. blocked_monitor_updates: Vec, - /// Only set when a counterparty `stfu` has been processed to track which node is allowed to - /// propose "something fundamental" upon becoming quiescent. - is_holder_quiescence_initiator: Option, + /// Once we become quiescent, if we're the initiator, there's some action we'll want to take. + /// This keeps track of that action. Note that if we become quiescent and we're not the + /// initiator we may be able to merge this action into what the counterparty wanted to do (e.g. + /// in the case of splicing). + post_quiescence_action: Option, } /// A channel struct implementing this trait can receive an initial counterparty commitment @@ -3321,7 +3361,7 @@ where is_manual_broadcast: false, - is_holder_quiescence_initiator: None, + post_quiescence_action: None, }; Ok((funding, channel_context)) @@ -3559,7 +3599,7 @@ where local_initiated_shutdown: None, is_manual_broadcast: false, - is_holder_quiescence_initiator: None, + post_quiescence_action: None, }; Ok((funding, channel_context)) @@ -5915,7 +5955,7 @@ fn estimate_v2_funding_transaction_fee( fn check_v2_funding_inputs_sufficient( contribution_amount: i64, funding_inputs: &[(TxIn, Transaction, Weight)], is_initiator: bool, is_splice: bool, funding_feerate_sat_per_1000_weight: u32, -) -> Result { +) -> Result { let mut total_input_witness_weight = Weight::from_wu(funding_inputs.iter().map(|(_, _, w)| w.to_wu()).sum()); let mut funding_inputs_len = funding_inputs.len(); if is_initiator && is_splice { @@ -5930,10 +5970,10 @@ fn check_v2_funding_inputs_sufficient( if let Some(output) = input.1.output.get(input.0.previous_output.vout as usize) { total_input_sats = total_input_sats.saturating_add(output.value.to_sat()); } else { - return Err(ChannelError::Warn(format!( + return Err(format!( "Transaction with txid {} does not have an output with vout of {} corresponding to TxIn at funding_inputs[{}]", input.1.compute_txid(), input.0.previous_output.vout, idx - ))); + )); } } @@ -5950,10 +5990,10 @@ fn check_v2_funding_inputs_sufficient( let minimal_input_amount_needed = contribution_amount.saturating_add(estimated_fee as i64); if (total_input_sats as i64) < minimal_input_amount_needed { - Err(ChannelError::Warn(format!( + Err(format!( "Total input amount {} is lower than needed for contribution {}, considering fees of {}. Need more inputs.", total_input_sats, contribution_amount, estimated_fee, - ))) + )) } else { Ok(estimated_fee) } @@ -8215,11 +8255,13 @@ where // Reset any quiescence-related state as it is implicitly terminated once disconnected. if matches!(self.context.channel_state, ChannelState::ChannelReady(_)) { - self.context.channel_state.clear_awaiting_quiescence(); + if self.context.post_quiescence_action.is_some() { + // If we were trying to get quiescent, try again after reconnection. + self.context.channel_state.set_awaiting_quiescence(); + } self.context.channel_state.clear_local_stfu_sent(); self.context.channel_state.clear_remote_stfu_sent(); self.context.channel_state.clear_quiescent(); - self.context.is_holder_quiescence_initiator.take(); } self.context.channel_state.set_peer_disconnected(); @@ -10601,11 +10643,14 @@ where /// - `change_script`: an option change output script. If `None` and needed, one will be /// generated by `SignerProvider::get_destination_script`. #[cfg(splicing)] - pub fn splice_channel( + pub fn splice_channel( &mut self, our_funding_contribution_satoshis: i64, our_funding_inputs: Vec<(TxIn, Transaction, Weight)>, change_script: Option, - funding_feerate_per_kw: u32, locktime: u32, - ) -> Result { + funding_feerate_per_kw: u32, locktime: u32, logger: &L, + ) -> Result, APIError> + where + L::Target: Logger, + { // Check if a splice has been initiated already. // Note: only a single outstanding splice is supported (per spec) if self.pending_splice.is_some() { @@ -10658,11 +10703,44 @@ where err, ), })?; - // Convert inputs - let mut funding_inputs = Vec::new(); - for (tx_in, tx, _w) in our_funding_inputs.into_iter() { - let tx16 = TransactionU16LenLimited::new(tx) - .map_err(|_e| APIError::APIMisuseError { err: format!("Too large transaction") })?; + + // TODO(splicing): Check that transactions aren't too big for the splice_init message here. + + let action = QuiescentAction::Splice(SpliceInstructions { + our_funding_contribution_satoshis, + our_funding_inputs, + change_script, + funding_feerate_per_kw, + locktime, + }); + self.propose_quiescence(logger, action) + .map_err(|e| APIError::APIMisuseError { err: e.to_owned() }) + } + + #[cfg(splicing)] + fn send_splice_init( + &mut self, instructions: SpliceInstructions, + ) -> Result { + let SpliceInstructions { + our_funding_contribution_satoshis, + our_funding_inputs, + change_script, + funding_feerate_per_kw, + locktime, + } = instructions; + + // Check that the channel value hasn't changed out from under us. + let _fee = check_v2_funding_inputs_sufficient( + our_funding_contribution_satoshis, + &our_funding_inputs, + true, + true, + funding_feerate_per_kw, + )?; + + let mut funding_inputs = Vec::with_capacity(our_funding_inputs.len()); + for (tx_in, tx, _weight) in our_funding_inputs { + let tx16 = TransactionU16LenLimited::new(tx).map_err(|_e| "tx too big".to_owned())?; funding_inputs.push((tx_in, tx16)); } @@ -10763,6 +10841,10 @@ where ES::Target: EntropySource, L::Target: Logger, { + if !self.context.channel_state.is_quiescent() { + return Err(ChannelError::WarnAndDisconnect("Quiescence needed to splice".to_owned())); + } + let splice_funding = self.validate_splice_init(msg, our_funding_contribution_satoshis)?; log_info!( @@ -10802,6 +10884,11 @@ where })?; debug_assert!(interactive_tx_constructor.take_initiator_first_message().is_none()); + // TODO(splicing): if post_quiescence_action is set, integrate what the user wants to do + // into the counterparty-initiated splice. For always-on nodes this probably isn't a useful + // optimization, but for often-offline nodes it may be, as we may connect and immediately + // go into splicing from both sides. + let funding_pubkey = splice_funding.get_holder_pubkeys().funding_pubkey; self.pending_splice = Some(PendingSplice { @@ -11554,43 +11641,46 @@ where ); } - #[cfg(any(test, fuzzing))] + #[cfg(any(splicing, test, fuzzing))] #[rustfmt::skip] pub fn propose_quiescence( - &mut self, logger: &L, - ) -> Result, ChannelError> + &mut self, logger: &L, action: QuiescentAction, + ) -> Result, &'static str> where L::Target: Logger, { log_debug!(logger, "Attempting to initiate quiescence"); - if !self.context.is_live() { - return Err(ChannelError::Ignore( - "Channel is not in a live state to propose quiescence".to_owned() - )); + if !self.context.is_usable() { + return Err("Channel is not in a usable state to propose quiescence"); } - if self.context.channel_state.is_quiescent() { - return Err(ChannelError::Ignore("Channel is already quiescent".to_owned())); + if self.context.post_quiescence_action.is_some() { + return Err("Channel is already quiescing"); } - if self.context.channel_state.is_awaiting_quiescence() + self.context.post_quiescence_action = Some(action); + if self.context.channel_state.is_quiescent() + || self.context.channel_state.is_awaiting_quiescence() || self.context.channel_state.is_local_stfu_sent() { return Ok(None); } self.context.channel_state.set_awaiting_quiescence(); - Ok(Some(self.send_stfu(logger)?)) + if self.context.is_live() { + Ok(Some(self.send_stfu(logger)?)) + } else { + Ok(None) + } } // Assumes we are either awaiting quiescence or our counterparty has requested quiescence. #[rustfmt::skip] - pub fn send_stfu(&mut self, logger: &L) -> Result + pub fn send_stfu(&mut self, logger: &L) -> Result where L::Target: Logger, { debug_assert!(!self.context.channel_state.is_local_stfu_sent()); - // Either state being set implies the channel is live. debug_assert!( self.context.channel_state.is_awaiting_quiescence() || self.context.channel_state.is_remote_stfu_sent() @@ -11600,9 +11690,7 @@ where if self.context.is_waiting_on_peer_pending_channel_update() || self.context.is_monitor_or_signer_pending_channel_update() { - return Err(ChannelError::Ignore( - "We cannot send `stfu` while state machine is pending".to_owned() - )); + return Err("We cannot send `stfu` while state machine is pending") } let initiator = if self.context.channel_state.is_remote_stfu_sent() { @@ -11610,18 +11698,10 @@ where self.context.channel_state.clear_awaiting_quiescence(); self.context.channel_state.clear_remote_stfu_sent(); self.context.channel_state.set_quiescent(); - if let Some(initiator) = self.context.is_holder_quiescence_initiator.as_ref() { - log_debug!( - logger, - "Responding to counterparty stfu with our own, channel is now quiescent and we are{} the initiator", - if !initiator { " not" } else { "" } - ); - - *initiator - } else { - debug_assert!(false, "Quiescence initiator must have been set when we received stfu"); - false - } + // We are sending an stfu in response to our couterparty's stfu, but had not yet sent + // our own stfu (even if `awaiting_quiescence` was set). Thus, the counterparty is the + // initiator and they can do "something fundamental". + false } else { log_debug!(logger, "Sending stfu as quiescence initiator"); debug_assert!(self.context.channel_state.is_awaiting_quiescence()); @@ -11636,7 +11716,7 @@ where #[rustfmt::skip] pub fn stfu( &mut self, msg: &msgs::Stfu, logger: &L - ) -> Result, ChannelError> where L::Target: Logger { + ) -> Result, ChannelError> where L::Target: Logger { if self.context.channel_state.is_quiescent() { return Err(ChannelError::Warn("Channel is already quiescent".to_owned())); } @@ -11652,9 +11732,7 @@ where )); } - if self.context.channel_state.is_awaiting_quiescence() - || !self.context.channel_state.is_local_stfu_sent() - { + if !self.context.channel_state.is_local_stfu_sent() { if !msg.initiator { return Err(ChannelError::WarnAndDisconnect( "Peer sent unexpected `stfu` without signaling as initiator".to_owned() @@ -11668,23 +11746,16 @@ where // then. self.context.channel_state.set_remote_stfu_sent(); - let is_holder_initiator = if self.context.channel_state.is_awaiting_quiescence() { - // We were also planning to propose quiescence, let the tie-breaker decide the - // initiator. - self.funding.is_outbound() - } else { - false - }; - self.context.is_holder_quiescence_initiator = Some(is_holder_initiator); - log_debug!(logger, "Received counterparty stfu proposing quiescence"); - return self.send_stfu(logger).map(|stfu| Some(stfu)); + return self + .send_stfu(logger) + .map(|stfu| Some(StfuResponse::Stfu(stfu))) + .map_err(|e| ChannelError::Ignore(e.to_owned())); } // We already sent `stfu` and are now processing theirs. It may be in response to ours, or // we happened to both send `stfu` at the same time and a tie-break is needed. let is_holder_quiescence_initiator = !msg.initiator || self.funding.is_outbound(); - self.context.is_holder_quiescence_initiator = Some(is_holder_quiescence_initiator); // We were expecting to receive `stfu` because we already sent ours. self.mark_response_received(); @@ -11712,6 +11783,28 @@ where if !is_holder_quiescence_initiator { " not" } else { "" } ); + if is_holder_quiescence_initiator { + match self.context.post_quiescence_action.take() { + None => { + debug_assert!(false); + return Err(ChannelError::WarnAndDisconnect( + "Internal Error: Didn't have anything to do after reaching quiescence".to_owned() + )); + }, + Some(QuiescentAction::Splice(_instructions)) => { + #[cfg(splicing)] + return self.send_splice_init(_instructions) + .map(|splice_init| Some(StfuResponse::SpliceInit(splice_init))) + .map_err(|e| ChannelError::Ignore(e.to_owned())); + }, + #[cfg(any(test, fuzzing))] + Some(QuiescentAction::DoNothing) => { + // In quiescence test we want to just hang out here, letting the test manually + // leave quiescence. + }, + } + } + Ok(None) } @@ -11727,13 +11820,20 @@ where && self.context.channel_state.is_remote_stfu_sent()) ); + if !self.context.is_live() { + return Ok(None); + } + // We need to send our `stfu`, either because we're trying to initiate quiescence, or the // counterparty is and we've yet to send ours. if self.context.channel_state.is_awaiting_quiescence() || (self.context.channel_state.is_remote_stfu_sent() && !self.context.channel_state.is_local_stfu_sent()) { - return self.send_stfu(logger).map(|stfu| Some(stfu)); + return self + .send_stfu(logger) + .map(|stfu| Some(stfu)) + .map_err(|e| ChannelError::Ignore(e.to_owned())); } // We're either: @@ -11752,13 +11852,10 @@ where debug_assert!(!self.context.channel_state.is_local_stfu_sent()); debug_assert!(!self.context.channel_state.is_remote_stfu_sent()); - if self.context.channel_state.is_quiescent() { - self.mark_response_received(); - self.context.channel_state.clear_quiescent(); - self.context.is_holder_quiescence_initiator.take().expect("Must always be set while quiescent") - } else { - false - } + self.mark_response_received(); + let was_quiescent = self.context.channel_state.is_quiescent(); + self.context.channel_state.clear_quiescent(); + was_quiescent } pub fn remove_legacy_scids_before_block(&mut self, height: u32) -> alloc::vec::Drain<'_, u64> { @@ -12867,7 +12964,11 @@ where match channel_state { ChannelState::AwaitingChannelReady(_) => {}, ChannelState::ChannelReady(_) => { - channel_state.clear_awaiting_quiescence(); + if self.context.post_quiescence_action.is_some() { + // If we're trying to get quiescent to do something, try again when we + // reconnect to the peer. + channel_state.set_awaiting_quiescence(); + } channel_state.clear_local_stfu_sent(); channel_state.clear_remote_stfu_sent(); channel_state.clear_quiescent(); @@ -13274,6 +13375,7 @@ where (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, self.context.post_quiescence_action, option), // Added in 0.2 }); Ok(()) @@ -13634,6 +13736,8 @@ where let mut minimum_depth_override: Option = None; + let mut post_quiescence_action = None; + read_tlv_fields!(reader, { (0, announcement_sigs, option), (1, minimum_depth, option), @@ -13676,6 +13780,7 @@ where (59, minimum_depth_override, option), // Added in 0.2 (60, historical_scids, optional_vec), // Added in 0.2 (61, fulfill_attribution_data, optional_vec), // Added in 0.2 + (63, post_quiescence_action, upgradable_option), // Added in 0.2 }); let holder_signer = signer_provider.derive_channel_signer(channel_keys_id); @@ -14019,7 +14124,7 @@ where blocked_monitor_updates: blocked_monitor_updates.unwrap(), is_manual_broadcast: is_manual_broadcast.unwrap_or(false), - is_holder_quiescence_initiator: None, + post_quiescence_action, }, interactive_tx_signing_session, holder_commitment_point, @@ -15898,8 +16003,8 @@ mod tests { 2000, ); assert_eq!( - format!("{:?}", res.err().unwrap()), - "Warn: Total input amount 100000 is lower than needed for contribution 220000, considering fees of 1730. Need more inputs.", + res.err().unwrap(), + "Total input amount 100000 is lower than needed for contribution 220000, considering fees of 1730. Need more inputs.", ); } @@ -15934,8 +16039,8 @@ mod tests { 2200, ); assert_eq!( - format!("{:?}", res.err().unwrap()), - "Warn: Total input amount 300000 is lower than needed for contribution 298032, considering fees of 2495. Need more inputs.", + res.err().unwrap(), + "Total input amount 300000 is lower than needed for contribution 298032, considering fees of 2495. Need more inputs.", ); } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 9d1c6292826..e87c28698f6 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -57,12 +57,12 @@ use crate::events::{ }; use crate::events::{FundingInfo, PaidBolt12Invoice}; use crate::ln::chan_utils::selected_commitment_sat_per_1000_weight; -// Since this struct is returned in `list_channels` methods, expose it here in case users want to -// construct one themselves. +#[cfg(any(test, fuzzing))] +use crate::ln::channel::QuiescentAction; use crate::ln::channel::{ self, hold_time_since, Channel, ChannelError, ChannelUpdateStatus, FundedChannel, InboundV1Channel, OutboundV1Channel, PendingV2Channel, ReconnectionMsg, ShutdownResult, - UpdateFulfillCommitFetch, WithChannelContext, + StfuResponse, UpdateFulfillCommitFetch, WithChannelContext, }; use crate::ln::channel_state::ChannelDetails; use crate::ln::inbound_payment; @@ -4503,17 +4503,21 @@ where hash_map::Entry::Occupied(mut chan_phase_entry) => { let locktime = locktime.unwrap_or_else(|| self.current_best_block().height); if let Some(chan) = chan_phase_entry.get_mut().as_funded_mut() { - let msg = chan.splice_channel( + let logger = WithChannelContext::from(&self.logger, &chan.context, None); + let msg_opt = chan.splice_channel( our_funding_contribution_satoshis, our_funding_inputs, change_script, funding_feerate_per_kw, locktime, + &&logger, )?; - peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceInit { - node_id: *counterparty_node_id, - msg, - }); + if let Some(msg) = msg_opt { + peer_state.pending_msg_events.push(MessageSendEvent::SendStfu { + node_id: *counterparty_node_id, + msg, + }); + } Ok(()) } else { Err(APIError::ChannelUnavailable { @@ -10841,7 +10845,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ )); } - let mut sent_stfu = false; match peer_state.channel_by_id.entry(msg.channel_id) { hash_map::Entry::Occupied(mut chan_entry) => { if let Some(chan) = chan_entry.get_mut().as_funded_mut() { @@ -10849,14 +10852,24 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self.logger, Some(*counterparty_node_id), Some(msg.channel_id), None ); - if let Some(stfu) = try_channel_entry!( - self, peer_state, chan.stfu(&msg, &&logger), chan_entry - ) { - sent_stfu = true; - peer_state.pending_msg_events.push(MessageSendEvent::SendStfu { - node_id: *counterparty_node_id, - msg: stfu, - }); + let res = chan.stfu(&msg, &&logger); + let resp = try_channel_entry!(self, peer_state, res, chan_entry); + match resp { + None => Ok(false), + Some(StfuResponse::Stfu(msg)) => { + peer_state.pending_msg_events.push(MessageSendEvent::SendStfu { + node_id: *counterparty_node_id, + msg, + }); + Ok(true) + }, + Some(StfuResponse::SpliceInit(msg)) => { + peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceInit { + node_id: *counterparty_node_id, + msg, + }); + Ok(true) + }, } } else { let msg = "Peer sent `stfu` for an unfunded channel"; @@ -10871,8 +10884,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ msg.channel_id )) } - - Ok(sent_stfu) } #[rustfmt::skip] @@ -11660,7 +11671,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self.logger, Some(*counterparty_node_id), Some(*channel_id), None ); - match chan.propose_quiescence(&&logger) { + match chan.propose_quiescence(&&logger, QuiescentAction::DoNothing) { Ok(None) => {}, Ok(Some(stfu)) => { peer_state.pending_msg_events.push(MessageSendEvent::SendStfu { @@ -13884,8 +13895,8 @@ where let persist = match &res { Err(e) if e.closes_channel() => NotifyOption::DoPersist, Err(_) => NotifyOption::SkipPersistHandleEvents, - Ok(sent_stfu) => { - if *sent_stfu { + Ok(responded) => { + if *responded { NotifyOption::SkipPersistHandleEvents } else { NotifyOption::SkipPersistNoEvents diff --git a/lightning/src/ln/quiescence_tests.rs b/lightning/src/ln/quiescence_tests.rs index 211e79adb6d..737a2e02569 100644 --- a/lightning/src/ln/quiescence_tests.rs +++ b/lightning/src/ln/quiescence_tests.rs @@ -33,7 +33,7 @@ fn test_quiescence_tie() { assert!(stfu_node_0.initiator && stfu_node_1.initiator); assert!(nodes[0].node.exit_quiescence(&nodes[1].node.get_our_node_id(), &chan_id).unwrap()); - assert!(!nodes[1].node.exit_quiescence(&nodes[0].node.get_our_node_id(), &chan_id).unwrap()); + assert!(nodes[1].node.exit_quiescence(&nodes[0].node.get_our_node_id(), &chan_id).unwrap()); } #[test] @@ -173,7 +173,8 @@ fn allow_shutdown_while_awaiting_quiescence(local_shutdown: bool) { // Now that the state machine is no longer pending, and `closing_signed` is ready to be sent, // make sure we're still not waiting for the quiescence handshake to complete. - local_node.node.exit_quiescence(&remote_node_id, &chan_id).unwrap(); + // Note that we never actually reached full quiescence here. + assert!(!local_node.node.exit_quiescence(&remote_node_id, &chan_id).unwrap()); let _ = get_event_msg!(local_node, MessageSendEvent::SendClosingSigned, remote_node_id); check_added_monitors(local_node, 2); // One for the last revoke_and_ack, another for closing_signed @@ -279,8 +280,8 @@ fn test_quiescence_waits_for_async_signer_and_monitor_update() { let stfu = get_event_msg!(&nodes[0], MessageSendEvent::SendStfu, node_id_1); nodes[1].node.handle_stfu(node_id_0, &stfu); - nodes[0].node.exit_quiescence(&node_id_1, &chan_id).unwrap(); - nodes[1].node.exit_quiescence(&node_id_0, &chan_id).unwrap(); + assert!(nodes[0].node.exit_quiescence(&node_id_1, &chan_id).unwrap()); + assert!(nodes[1].node.exit_quiescence(&node_id_0, &chan_id).unwrap()); // After exiting quiescence, we should be able to resume payments from nodes[0]. send_payment(&nodes[0], &[&nodes[1]], payment_amount); @@ -336,8 +337,8 @@ fn test_quiescence_on_final_revoke_and_ack_pending_monitor_update() { panic!(); } - nodes[0].node.exit_quiescence(&node_id_1, &chan_id).unwrap(); - nodes[1].node.exit_quiescence(&node_id_0, &chan_id).unwrap(); + assert!(nodes[0].node.exit_quiescence(&node_id_1, &chan_id).unwrap()); + assert!(nodes[1].node.exit_quiescence(&node_id_0, &chan_id).unwrap()); } #[test] @@ -406,8 +407,8 @@ fn quiescence_updates_go_to_holding_cell(fail_htlc: bool) { let stfu = get_event_msg!(&nodes[0], MessageSendEvent::SendStfu, node_id_1); nodes[1].node.handle_stfu(node_id_0, &stfu); - nodes[0].node.exit_quiescence(&node_id_1, &chan_id).unwrap(); - nodes[1].node.exit_quiescence(&node_id_0, &chan_id).unwrap(); + assert!(nodes[0].node.exit_quiescence(&node_id_1, &chan_id).unwrap()); + assert!(nodes[1].node.exit_quiescence(&node_id_0, &chan_id).unwrap()); // Now that quiescence is over, nodes are allowed to make updates again. nodes[1] will have its // outbound HTLC finally go out, along with the fail/claim of nodes[0]'s payment. @@ -547,3 +548,154 @@ fn test_quiescence_timeout_while_waiting_for_counterparty_stfu() { }; assert!(nodes[1].node.get_and_clear_pending_msg_events().iter().find_map(f).is_some()); } + +#[test] +fn test_quiescence_timeout_while_waiting_for_counterparty_something_fundamental() { + // Test that we'll disconnect if the counterparty does not send their "something fundamental" + // within a reasonable time if we've reached quiescence. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let chan_id = create_announced_chan_between_nodes(&nodes, 0, 1).2; + + let node_id_0 = nodes[0].node.get_our_node_id(); + let node_id_1 = nodes[1].node.get_our_node_id(); + + nodes[1].node.maybe_propose_quiescence(&node_id_0, &chan_id).unwrap(); + let stfu = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + + nodes[0].node.handle_stfu(node_id_1, &stfu); + let _stfu = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + + for _ in 0..DISCONNECT_PEER_AWAITING_RESPONSE_TICKS { + nodes[0].node.timer_tick_occurred(); + nodes[1].node.timer_tick_occurred(); + } + + // nodes[1] didn't receive nodes[0]'s stfu within the timeout so it'll disconnect. + let f = |event| { + if let MessageSendEvent::HandleError { action, .. } = event { + if let msgs::ErrorAction::DisconnectPeerWithWarning { .. } = action { + Some(()) + } else { + None + } + } else { + None + } + }; + // At this point, node A is waiting on B to do something fundamental, and node B is waiting on + // A's stfu that we never delivered. Thus both should disconnect each other. + assert!(nodes[0].node.get_and_clear_pending_msg_events().into_iter().find_map(&f).is_some()); + assert!(nodes[1].node.get_and_clear_pending_msg_events().into_iter().find_map(&f).is_some()); +} + +fn do_test_quiescence_during_disconnection(with_pending_claim: bool, propose_disconnected: bool) { + // Test that we'll start trying for quiescence immediately after reconnection if we're waiting + // to do some quiescence-required action. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let chan_id = create_announced_chan_between_nodes(&nodes, 0, 1).2; + + let node_a_id = nodes[0].node.get_our_node_id(); + let node_b_id = nodes[1].node.get_our_node_id(); + + // First get both nodes off the starting state so we don't have to deal with channel_ready + // retransmissions on reconect. + send_payment(&nodes[0], &[&nodes[1]], 100_000); + + let (preimage, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 100_000); + if with_pending_claim { + // Optionally reconnect with pending quiescence while there's some pending messages to + // deliver. + nodes[1].node.claim_funds(preimage); + check_added_monitors(&nodes[1], 1); + expect_payment_claimed!(nodes[1], payment_hash, 100_000); + let _ = get_htlc_update_msgs(&nodes[1], &node_a_id); + } + + if !propose_disconnected { + nodes[1].node.maybe_propose_quiescence(&node_a_id, &chan_id).unwrap(); + } + + nodes[0].node.peer_disconnected(node_b_id); + nodes[1].node.peer_disconnected(node_a_id); + + if propose_disconnected { + nodes[1].node.maybe_propose_quiescence(&node_a_id, &chan_id).unwrap(); + } + + let init_msg = msgs::Init { + features: nodes[1].node.init_features(), + networks: None, + remote_network_address: None, + }; + nodes[0].node.peer_connected(node_b_id, &init_msg, true).unwrap(); + nodes[1].node.peer_connected(node_a_id, &init_msg, true).unwrap(); + + let reestab_a = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, node_b_id); + let reestab_b = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, node_a_id); + + nodes[0].node.handle_channel_reestablish(node_b_id, &reestab_b); + get_event_msg!(nodes[0], MessageSendEvent::SendChannelUpdate, node_b_id); + + nodes[1].node.handle_channel_reestablish(node_a_id, &reestab_a); + let mut bs_msgs = nodes[1].node.get_and_clear_pending_msg_events(); + bs_msgs.retain(|msg| !matches!(msg, MessageSendEvent::SendChannelUpdate { .. })); + assert_eq!(bs_msgs.len(), 1, "{bs_msgs:?}"); + let stfu = if with_pending_claim { + // Node B should first re-send its channel update, then try to enter quiescence once that + // completes... + let msg = bs_msgs.pop().unwrap(); + if let MessageSendEvent::UpdateHTLCs { mut updates, .. } = msg { + let fulfill = updates.update_fulfill_htlcs.pop().unwrap(); + nodes[0].node.handle_update_fulfill_htlc(node_b_id, fulfill); + let cs = updates.commitment_signed; + nodes[0].node.handle_commitment_signed_batch_test(node_b_id, &cs); + check_added_monitors(&nodes[0], 1); + + let (raa, cs) = get_revoke_commit_msgs(&nodes[0], &node_b_id); + nodes[1].node.handle_revoke_and_ack(node_a_id, &raa); + check_added_monitors(&nodes[1], 1); + nodes[1].node.handle_commitment_signed_batch_test(node_a_id, &cs); + check_added_monitors(&nodes[1], 1); + + let mut bs_raa_stfu = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(bs_raa_stfu.len(), 2); + if let MessageSendEvent::SendRevokeAndACK { msg, .. } = &bs_raa_stfu[0] { + nodes[0].node.handle_revoke_and_ack(node_b_id, &msg); + expect_payment_sent!(&nodes[0], preimage); + } else { + panic!("Unexpected first message {bs_raa_stfu:?}"); + } + + bs_raa_stfu.pop().unwrap() + } else { + panic!("Unexpected message {msg:?}"); + } + } else { + bs_msgs.pop().unwrap() + }; + if let MessageSendEvent::SendStfu { msg, .. } = stfu { + nodes[0].node.handle_stfu(node_b_id, &msg); + } else { + panic!("Unexpected message {stfu:?}"); + } + + let stfu_resp = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_b_id); + nodes[1].node.handle_stfu(node_a_id, &stfu_resp); + + assert!(nodes[0].node.exit_quiescence(&node_b_id, &chan_id).unwrap()); + assert!(nodes[1].node.exit_quiescence(&node_a_id, &chan_id).unwrap()); +} + +#[test] +fn test_quiescence_during_disconnection() { + do_test_quiescence_during_disconnection(false, false); + do_test_quiescence_during_disconnection(true, false); + do_test_quiescence_during_disconnection(false, true); + do_test_quiescence_during_disconnection(true, true); +} diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 51f9f2e387e..a4f18b30f81 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -25,6 +25,8 @@ fn test_v1_splice_in() { let acceptor_node_index = 1; let initiator_node = &nodes[initiator_node_index]; let acceptor_node = &nodes[acceptor_node_index]; + let initiator_node_id = initiator_node.node.get_our_node_id(); + let acceptor_node_id = acceptor_node.node.get_our_node_id(); let channel_value_sat = 100_000; let channel_reserve_amnt_sat = 1_000; @@ -79,12 +81,16 @@ fn test_v1_splice_in() { None, // locktime ) .unwrap(); + + let init_stfu = get_event_msg!(initiator_node, MessageSendEvent::SendStfu, acceptor_node_id); + acceptor_node.node.handle_stfu(initiator_node_id, &init_stfu); + + let ack_stfu = get_event_msg!(acceptor_node, MessageSendEvent::SendStfu, initiator_node_id); + initiator_node.node.handle_stfu(acceptor_node_id, &ack_stfu); + // Extract the splice_init message - let splice_init_msg = get_event_msg!( - initiator_node, - MessageSendEvent::SendSpliceInit, - acceptor_node.node.get_our_node_id() - ); + let splice_init_msg = + get_event_msg!(initiator_node, MessageSendEvent::SendSpliceInit, acceptor_node_id); assert_eq!(splice_init_msg.funding_contribution_satoshis, splice_in_sats as i64); assert_eq!(splice_init_msg.funding_feerate_per_kw, funding_feerate_per_kw); assert_eq!(splice_init_msg.funding_pubkey.to_string(), expected_initiator_funding_key);