Enforce disconnect timeout during quiescence

Since new updates are not allowed during quiescence (local updates enter
the holding cell), we want to ensure quiescence eventually terminates if
the handshake takes too long or our counterparty is uncooperative.
Disconnecting implicitly terminates quiescence, so the holding cell can
be freed upon re-establishing the channel (assuming quiescence is not
requested again).

Author: wpaulino

lightning/src/ln/channel.rs

@@ -7093,6 +7093,10 @@ impl<SP: Deref> FundedChannel<SP> where
 		} else if
 			// Cleared upon receiving `channel_reestablish`.
 			self.context.channel_state.is_peer_disconnected()
+			// Cleared upon receiving `stfu`.
+			|| self.context.channel_state.is_local_stfu_sent()
+			// Cleared upon receiving a message that triggers the end of quiescence.
+			|| self.context.channel_state.is_quiescent()
 			// Cleared upon receiving `revoke_and_ack`.
 			|| self.context.has_pending_channel_update()
 		{
@@ -8871,6 +8875,9 @@ impl<SP: Deref> FundedChannel<SP> where
 		let is_holder_quiescence_initiator = !msg.initiator || self.context.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();
+
 		if self.context.has_pending_channel_update() {
 			// Since we've already sent `stfu`, it should not be possible for one of our updates to
 			// be pending, so anything pending currently must be from a counterparty update.
@@ -8928,6 +8935,7 @@ impl<SP: Deref> FundedChannel<SP> where
 		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 {

lightning/src/ln/quiescence_tests.rs

@@ -3,9 +3,11 @@ use crate::events::Event;
 use crate::events::HTLCDestination;
 use crate::events::MessageSendEvent;
 use crate::events::MessageSendEventsProvider;
+use crate::ln::channel::DISCONNECT_PEER_AWAITING_RESPONSE_TICKS;
 use crate::ln::channelmanager::PaymentId;
 use crate::ln::channelmanager::RecipientOnionFields;
 use crate::ln::functional_test_utils::*;
+use crate::ln::msgs;
 use crate::ln::msgs::{ChannelMessageHandler, ErrorAction};
 use crate::util::errors::APIError;
 use crate::util::test_channel_signer::SignerOp;
@@ -413,3 +415,87 @@ fn quiescence_updates_go_to_holding_cell(fail_htlc: bool) {
 		expect_payment_sent(&nodes[1], payment_preimage1, None, true, true);
 	}
 }
+
+#[test]
+fn test_quiescence_timeout() {
+	// Test that we'll disconnect if we remain quiescent for `DISCONNECT_PEER_AWAITING_RESPONSE_TICKS`.
+	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[0].node.maybe_propose_quiescence(&nodes[1].node.get_our_node_id(), &chan_id).unwrap();
+
+	let stfu_initiator = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1);
+	nodes[1].node.handle_stfu(node_id_0, &stfu_initiator);
+
+	let stfu_responder = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0);
+	nodes[0].node.handle_stfu(node_id_1, &stfu_responder);
+
+	assert!(stfu_initiator.initiator && !stfu_responder.initiator);
+
+	for _ in 0..DISCONNECT_PEER_AWAITING_RESPONSE_TICKS {
+		nodes[0].node.timer_tick_occurred();
+		nodes[1].node.timer_tick_occurred();
+	}
+
+	let f = |event| {
+		if let MessageSendEvent::HandleError { action, .. } = event {
+			if let msgs::ErrorAction::DisconnectPeerWithWarning { .. } = action {
+				Some(())
+			} else {
+				None
+			}
+		} else {
+			None
+		}
+	};
+	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());
+}
+
+#[test]
+fn test_quiescence_timeout_while_waiting_for_counterparty_stfu() {
+	// Test that we'll disconnect if the counterparty does not send their stfu within a reasonable
+	// time if we've already sent ours.
+	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();
+
+	nodes[1].node.maybe_propose_quiescence(&node_id_0, &chan_id).unwrap();
+	let _ = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0);
+
+	// Route a payment in between to ensure expecting to receive `revoke_and_ack` doesn't override
+	// the expectation of receiving `stfu` as well.
+	let _ = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
+
+	for _ in 0..DISCONNECT_PEER_AWAITING_RESPONSE_TICKS {
+		nodes[0].node.timer_tick_occurred();
+		nodes[1].node.timer_tick_occurred();
+	}
+
+	// nodes[0] hasn't received stfu from nodes[1], so it's not enforcing any timeouts.
+	assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+
+	// nodes[1] didn't receive nodes[0]'s stfu within the timeout so it'll disconnect.
+	let f = |&ref event| {
+		if let MessageSendEvent::HandleError { action, .. } = event {
+			if let msgs::ErrorAction::DisconnectPeerWithWarning { .. } = action {
+				Some(())
+			} else {
+				None
+			}
+		} else {
+			None
+		}
+	};
+	assert!(nodes[1].node.get_and_clear_pending_msg_events().iter().find_map(f).is_some());
+}