Allow quiescence-init while disconnected from peers

There are a number of things in LDK where we've been lazy and not
allowed the user to initiate an action while a peer is
disconnected. While it may be accurate in the sense that the action
cannot be started while the peer is disconnected, it is terrible
dev UX - these actions can fail without the developer being at
fault and the only way for them to address it is just try again.

Here we fix this dev UX shortcoming for splicing, keeping any
queued post-quiescent actions around when a peer disconnects and
retrying the action (and quiescence generally) when the peer
reconnects.

Author: TheBlueMatt

lightning/src/ln/channel.rs

@@ -8223,7 +8223,10 @@ 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.quiescent_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();
@@ -11571,9 +11574,9 @@ where
 	{
 		log_debug!(logger, "Attempting to initiate quiescence");
 
-		if !self.context.is_live() {
+		if !self.context.is_usable() {
 			return Err(ChannelError::Ignore(
-				"Channel is not in a live state to propose quiescence".to_owned()
+				"Channel is not in a usable state to propose quiescence".to_owned()
 			));
 		}
 		if self.quiescent_action.is_some() {
@@ -11589,7 +11592,11 @@ where
 		}
 
 		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.
@@ -11599,7 +11606,6 @@ 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()
@@ -11731,6 +11737,10 @@ 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()
@@ -12870,7 +12880,11 @@ where
 			match channel_state {
 				ChannelState::AwaitingChannelReady(_) => {},
 				ChannelState::ChannelReady(_) => {
-					channel_state.clear_awaiting_quiescence();
+					if self.quiescent_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();
@@ -13277,6 +13291,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.quiescent_action, option), // Added in 0.2
 		});
 
 		Ok(())
@@ -13637,6 +13652,8 @@ where
 
 		let mut minimum_depth_override: Option<u32> = None;
 
+		let mut quiescent_action = None;
+
 		read_tlv_fields!(reader, {
 			(0, announcement_sigs, option),
 			(1, minimum_depth, option),
@@ -13679,6 +13696,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, quiescent_action, upgradable_option), // Added in 0.2
 		});
 
 		let holder_signer = signer_provider.derive_channel_signer(channel_keys_id);
@@ -14026,7 +14044,7 @@ where
 			holder_commitment_point,
 			#[cfg(splicing)]
 			pending_splice: None,
-			quiescent_action: None,
+			quiescent_action,
 		})
 	}
 }

lightning/src/ln/quiescence_tests.rs

@@ -548,3 +548,112 @@ 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());
 }
+
+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);
+}