Reconnect to all persisted peers after restart

While we previously took care of persisting peer details when a channel
is opened and if the user signals to `connect` to have the connection
persisted, our background task would still only try to reconnect to
channel peers. Here, we fix this omission and add a regression test
ensuring we'll see the proper behavior going forward.

Author: tnull

src/lib.rs

@@ -461,8 +461,7 @@ impl<K: KVStore + Sync + Send + 'static> Node<K> {
 			});
 		}
 
-		// Regularly reconnect to channel peers.
-		let connect_cm = Arc::clone(&self.channel_manager);
+		// Regularly reconnect to persisted peers.
 		let connect_pm = Arc::clone(&self.peer_manager);
 		let connect_logger = Arc::clone(&self.logger);
 		let connect_peer_store = Arc::clone(&self.peer_store);
@@ -481,29 +480,23 @@ impl<K: KVStore + Sync + Send + 'static> Node<K> {
 								.iter()
 								.map(|(peer, _addr)| *peer)
 								.collect::<Vec<_>>();
-							for node_id in connect_cm
-								.list_channels()
-									.iter()
-									.map(|chan| chan.counterparty.node_id)
-									.filter(|id| !pm_peers.contains(id))
-									{
-										if let Some(peer_info) = connect_peer_store.get_peer(&node_id) {
-											let res = do_connect_peer(
-												peer_info.node_id,
-												peer_info.address,
-												Arc::clone(&connect_pm),
-												Arc::clone(&connect_logger),
-												).await;
-											match res {
-												Ok(_) => {
-													log_info!(connect_logger, "Successfully reconnected to peer {}", node_id);
-												},
-												Err(e) => {
-													log_error!(connect_logger, "Failed to reconnect to peer {}: {}", node_id, e);
-												}
-											}
-										}
+
+							for peer_info in connect_peer_store.list_peers().iter().filter(|info| !pm_peers.contains(&info.node_id)) {
+								let res = do_connect_peer(
+									peer_info.node_id,
+									peer_info.address.clone(),
+									Arc::clone(&connect_pm),
+									Arc::clone(&connect_logger),
+									).await;
+								match res {
+									Ok(_) => {
+										log_info!(connect_logger, "Successfully reconnected to peer {}", peer_info.node_id);
+									},
+									Err(e) => {
+										log_error!(connect_logger, "Failed to reconnect to peer {}: {}", peer_info.node_id, e);
 									}
+								}
+							}
 						}
 				}
 			}

tests/common.rs

@@ -355,6 +355,7 @@ pub(crate) fn do_channel_full_cycle<K: KVStore + Sync + Send, E: ElectrumApi>(
 		.unwrap();
 
 	assert_eq!(node_a.list_peers().first().unwrap().node_id, node_b.node_id());
+	assert!(node_a.list_peers().first().unwrap().is_persisted);
 	let funding_txo_a = expect_channel_pending_event!(node_a, node_b.node_id());
 	let funding_txo_b = expect_channel_pending_event!(node_b, node_a.node_id());
 	assert_eq!(funding_txo_a, funding_txo_b);

tests/integration_tests_rust.rs

@@ -272,3 +272,55 @@ fn sign_verify_msg() {
 	let pkey = node.node_id();
 	assert!(node.verify_signature(msg, sig.as_str(), &pkey));
 }
+
+#[test]
+fn connection_restart_behavior() {
+	do_connection_restart_behavior(true);
+	do_connection_restart_behavior(false);
+}
+
+fn do_connection_restart_behavior(persist: bool) {
+	let (_bitcoind, electrsd) = setup_bitcoind_and_electrsd();
+	let (node_a, node_b) = setup_two_nodes(&electrsd, false);
+
+	let node_id_a = node_a.node_id();
+	let node_id_b = node_b.node_id();
+
+	let node_addr_b = node_b.listening_addresses().unwrap().first().unwrap().clone();
+	std::thread::sleep(std::time::Duration::from_secs(1));
+	node_a.connect(node_id_b, node_addr_b, persist).unwrap();
+
+	let peer_details_a = node_a.list_peers().first().unwrap().clone();
+	assert_eq!(peer_details_a.node_id, node_id_b);
+	assert_eq!(peer_details_a.is_persisted, persist);
+	assert!(peer_details_a.is_connected);
+
+	let peer_details_b = node_b.list_peers().first().unwrap().clone();
+	assert_eq!(peer_details_b.node_id, node_id_a);
+	assert_eq!(peer_details_b.is_persisted, false);
+	assert!(peer_details_a.is_connected);
+
+	// Restart nodes.
+	node_a.stop().unwrap();
+	node_b.stop().unwrap();
+	node_b.start().unwrap();
+	node_a.start().unwrap();
+
+	// Sleep a bit to allow for the reconnect to happen.
+	std::thread::sleep(std::time::Duration::from_secs(5));
+
+	if persist {
+		let peer_details_a = node_a.list_peers().first().unwrap().clone();
+		assert_eq!(peer_details_a.node_id, node_id_b);
+		assert_eq!(peer_details_a.is_persisted, persist);
+		assert!(peer_details_a.is_connected);
+
+		let peer_details_b = node_b.list_peers().first().unwrap().clone();
+		assert_eq!(peer_details_b.node_id, node_id_a);
+		assert_eq!(peer_details_b.is_persisted, false);
+		assert!(peer_details_a.is_connected);
+	} else {
+		assert!(node_a.list_peers().is_empty());
+		assert!(node_b.list_peers().is_empty());
+	}
+}