Add missing pending channel maps checks in ChannelManager

One of a series of follow-up commits to address some issues found
in PR 2077, where we split channels up into different maps and structs
depending on phase in their life.

Author: dunxen

lightning/src/ln/channelmanager.rs

@@ -691,6 +691,18 @@ impl <Signer: ChannelSigner> PeerState<Signer> {
 			self.outbound_v1_channel_by_id.contains_key(channel_id) ||
 			self.inbound_v1_channel_by_id.contains_key(channel_id)
 	}
+
+	/// Returns a bool indicating if the given `channel_id` matches a channel we have with this peer
+	/// that is in one of our pending (unfunded) channel maps.
+	///
+	/// NOTE: Although V1 established channels will always have a `temporary_channel_id` if they're
+	///       in `(outbound/inbound)_v1_channel_by_id`, we use the more general `channel_id` as V2
+	///       established channels will have a fixed `channel_id` already after the `accept_channel2`
+	///       message is sent/received.
+	fn has_pending_channel(&self, channel_id: &[u8; 32]) -> bool {
+		self.outbound_v1_channel_by_id.contains_key(channel_id) ||
+			self.inbound_v1_channel_by_id.contains_key(channel_id)
+	}
 }
 
 /// Stores a PaymentSecret and any other data we may need to validate an inbound payment is
@@ -2241,6 +2253,7 @@ where
 			for (_cp_id, peer_state_mutex) in per_peer_state.iter() {
 				let mut peer_state_lock = peer_state_mutex.lock().unwrap();
 				let peer_state = &mut *peer_state_lock;
+				// Only `Channels` in the channel_by_id map can be considered funded.
 				for (_channel_id, channel) in peer_state.channel_by_id.iter().filter(f) {
 					let details = ChannelDetails::from_channel_context(&channel.context, best_block_height,
 						peer_state.latest_features.clone(), &self.fee_estimator);
@@ -2309,11 +2322,15 @@ where
 			let mut peer_state_lock = peer_state_mutex.lock().unwrap();
 			let peer_state = &mut *peer_state_lock;
 			let features = &peer_state.latest_features;
+			let chan_context_to_details = |context| {
+				ChannelDetails::from_channel_context(context, best_block_height, features.clone(), &self.fee_estimator)
+			};
 			return peer_state.channel_by_id
 				.iter()
-				.map(|(_, channel)|
-					ChannelDetails::from_channel_context(&channel.context, best_block_height,
-					features.clone(), &self.fee_estimator))
+				.map(|(_, channel)| &channel.context)
+				.chain(peer_state.outbound_v1_channel_by_id.iter().map(|(_, channel)| &channel.context))
+				.chain(peer_state.inbound_v1_channel_by_id.iter().map(|(_, channel)| &channel.context))
+				.map(chan_context_to_details)
 				.collect();
 		}
 		vec![]
@@ -7146,37 +7163,26 @@ where
 		log_debug!(self.logger, "Generating channel_reestablish events for {}", log_pubkey!(counterparty_node_id));
 
 		let per_peer_state = self.per_peer_state.read().unwrap();
-		for (_cp_id, peer_state_mutex) in per_peer_state.iter() {
+		if let Some(peer_state_mutex) = per_peer_state.get(counterparty_node_id) {
 			let mut peer_state_lock = peer_state_mutex.lock().unwrap();
 			let peer_state = &mut *peer_state_lock;
 			let pending_msg_events = &mut peer_state.pending_msg_events;
-			peer_state.channel_by_id.retain(|_, chan| {
-				let retain = if chan.context.get_counterparty_node_id() == *counterparty_node_id {
-					if !chan.context.have_received_message() {
-						// If we created this (outbound) channel while we were disconnected from the
-						// peer we probably failed to send the open_channel message, which is now
-						// lost. We can't have had anything pending related to this channel, so we just
-						// drop it.
-						false
-					} else {
-						pending_msg_events.push(events::MessageSendEvent::SendChannelReestablish {
-							node_id: chan.context.get_counterparty_node_id(),
-							msg: chan.get_channel_reestablish(&self.logger),
-						});
-						true
-					}
-				} else { true };
-				if retain && chan.context.get_counterparty_node_id() != *counterparty_node_id {
-					if let Some(msg) = chan.get_signed_channel_announcement(&self.node_signer, self.genesis_hash.clone(), self.best_block.read().unwrap().height(), &self.default_configuration) {
-						if let Ok(update_msg) = self.get_channel_update_for_broadcast(chan) {
-							pending_msg_events.push(events::MessageSendEvent::SendChannelAnnouncement {
-								node_id: *counterparty_node_id,
-								msg, update_msg,
-							});
-						}
-					}
-				}
-				retain
+			for (_, chan) in peer_state.outbound_v1_channel_by_id.iter_mut() {
+				// We shouldn't have any pending outbound channels upon reconnect. If one exists,
+				// then we probably tried to open a channel while the peer was disconnected and they
+				// wouldn't have received the corresponding channel open message. We can just force-close
+				// these channels.
+				log_error!(self.logger, "Force-closing channel {}", log_bytes!(chan.context.channel_id()));
+				self.issue_channel_close_events(&chan.context, ClosureReason::HolderForceClosed);
+				self.finish_force_close_channel(chan.context.force_shutdown(false));
+			}
+			// Drop all outbound channels which we have force-closed above.
+			peer_state.outbound_v1_channel_by_id.clear();
+			peer_state.channel_by_id.iter_mut().for_each(|(_, chan)| {
+				pending_msg_events.push(events::MessageSendEvent::SendChannelReestablish {
+					node_id: chan.context.get_counterparty_node_id(),
+					msg: chan.get_channel_reestablish(&self.logger),
+				});
 			});
 		}
 		//TODO: Also re-broadcast announcement_signatures

lightning/src/ln/functional_test_utils.rs

@@ -2828,13 +2828,6 @@ macro_rules! get_chan_reestablish_msgs {
 					panic!("Unexpected event")
 				}
 			}
-			for chan in $src_node.node.list_channels() {
-				if chan.is_public && chan.counterparty.node_id != $dst_node.node.get_our_node_id() {
-					if let Some(scid) = chan.short_channel_id {
-						assert!(announcements.remove(&scid));
-					}
-				}
-			}
 			assert!(announcements.is_empty());
 			res
 		}