Simplify node reload logic in tests

Store the latest persisted monitor in TestChainMonitor so it no longer
needs to be passed into reload. This change also makes the test more
realistic. If the monitor wasn't actually persisted, it won't reload
properly.

Author: joostjager

lightning/src/ln/async_payments_tests.rs

@@ -2167,8 +2167,7 @@ fn offer_cache_round_trip_ser() {
 	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 	let payee_node_deserialized;
 	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
-	let chan_id =
-		create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0).0.channel_id;
+	create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0);
 	let server = &nodes[0];
 	let recipient = &nodes[1];
 
@@ -2188,12 +2187,10 @@ fn offer_cache_round_trip_ser() {
 	// offers.
 	let cached_offers_pre_ser = recipient.node.flow.test_get_async_receive_offers();
 	let config = test_default_channel_config();
-	let serialized_monitor = get_monitor!(recipient, chan_id).encode();
-	reload_node!(
+	reload_node_and_monitors!(
 		nodes[1],
 		config,
 		recipient.node.encode(),
-		&[&serialized_monitor],
 		persister,
 		chain_monitor,
 		payee_node_deserialized

lightning/src/ln/channelmanager.rs

@@ -19066,10 +19066,9 @@ mod tests {
 		*chanmgr_fwd_htlcs = forward_htlcs.clone();
 		core::mem::drop(chanmgr_fwd_htlcs);
 
-		reload_node!(nodes[0], nodes[0].node.encode(), &[], persister, chain_monitor, deserialized_chanmgr);
+		reload_node_and_monitors!(nodes[0], nodes[0].node.encode(), persister, chain_monitor, deserialized_chanmgr);
 
-		let mut deserialized_fwd_htlcs = nodes[0].node.forward_htlcs.lock().unwrap();
-		for scid in [scid_1, scid_2].iter() {
+		let mut deserialized_fwd_htlcs = nodes[0].node.forward_htlcs.lock().unwrap();		for scid in [scid_1, scid_2].iter() {
 			let deserialized_htlcs = deserialized_fwd_htlcs.remove(scid).unwrap();
 			assert_eq!(forward_htlcs.remove(scid).unwrap(), deserialized_htlcs);
 		}

lightning/src/ln/functional_test_utils.rs

@@ -1382,6 +1382,38 @@ macro_rules! reload_node {
 	};
 }
 
+#[cfg(test)]
+macro_rules! reload_node_and_monitors {
+	($node: expr, $new_config: expr, $chanman_encoded: expr, $persister: ident, $new_chain_monitor: ident, $new_channelmanager: ident) => {
+		let monitors_serialized = {
+			let monitor_map = $node.chain_monitor.persisted_monitors.lock().unwrap();
+			monitor_map.values().cloned().collect::<Vec<_>>()
+		};
+		let monitors_serialized_ref: Vec<&[u8]> =
+			monitors_serialized.iter().map(|v| v.as_slice()).collect();
+
+		reload_node!(
+			$node,
+			$new_config,
+			$chanman_encoded,
+			&monitors_serialized_ref,
+			$persister,
+			$new_chain_monitor,
+			$new_channelmanager
+		);
+	};
+	($node: expr, $chanman_encoded: expr, $persister: ident, $new_chain_monitor: ident, $new_channelmanager: ident) => {
+		reload_node_and_monitors!(
+			$node,
+			$crate::util::config::UserConfig::default(),
+			$chanman_encoded,
+			$persister,
+			$new_chain_monitor,
+			$new_channelmanager
+		);
+	};
+}
+
 pub fn create_funding_transaction<'a, 'b, 'c>(
 	node: &Node<'a, 'b, 'c>, expected_counterparty_node_id: &PublicKey, expected_chan_value: u64,
 	expected_user_chan_id: u128,

lightning/src/ln/monitor_tests.rs

@@ -3244,7 +3244,7 @@ fn test_event_replay_causing_monitor_replay() {
 	let node_deserialized;
 	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 
-	let chan = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 500_000_000);
+	create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 500_000_000);
 
 	let payment_preimage = route_payment(&nodes[0], &[&nodes[1]], 1_000_000).0;
 
@@ -3262,9 +3262,7 @@ fn test_event_replay_causing_monitor_replay() {
 	expect_payment_sent(&nodes[0], payment_preimage, None, true, true /* expected post-event monitor update*/);
 	assert!(nodes[0].node.get_and_clear_needs_persistence());
 
-	let serialized_monitor = get_monitor!(nodes[0], chan.2).encode();
-	reload_node!(nodes[0], &serialized_channel_manager, &[&serialized_monitor], persister, new_chain_monitor, node_deserialized);
-
+	reload_node_and_monitors!(nodes[0], &serialized_channel_manager, persister, new_chain_monitor, node_deserialized);
 	// Expect the `PaymentSent` to get replayed, this time without the duplicate monitor update
 	expect_payment_sent(&nodes[0], payment_preimage, None, false, false /* expected post-event monitor update*/);
 }
@@ -3363,7 +3361,7 @@ fn test_claim_event_never_handled() {
 
 	let init_node_ser = nodes[1].node.encode();
 
-	let chan = create_announced_chan_between_nodes(&nodes, 0, 1);
+	create_announced_chan_between_nodes(&nodes, 0, 1);
 
 	// Send the payment we'll ultimately test the PaymentClaimed event for.
 	let (preimage_a, payment_hash_a, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
@@ -3392,10 +3390,7 @@ fn test_claim_event_never_handled() {
 
 	// Finally, reload node B with an empty `ChannelManager` and check that we get the
 	// `PaymentClaimed` event.
-	let chan_0_monitor_serialized = get_monitor!(nodes[1], chan.2).encode();
-	let mons = &[&chan_0_monitor_serialized[..]];
-	reload_node!(nodes[1], &init_node_ser, mons, persister, new_chain_mon, nodes_1_reload);
-
+	reload_node_and_monitors!(nodes[1], &init_node_ser, persister, new_chain_mon, nodes_1_reload);
 	expect_payment_claimed!(nodes[1], payment_hash_a, 1_000_000);
 	// The reload logic spuriously generates a redundant payment preimage-containing
 	// `ChannelMonitorUpdate`.

lightning/src/ln/payment_tests.rs

@@ -2613,7 +2613,7 @@ fn do_automatic_retries(test: AutoRetry) {
 	let node_b_id = nodes[1].node.get_our_node_id();
 	let node_c_id = nodes[2].node.get_our_node_id();
 
-	let channel_id_1 = create_announced_chan_between_nodes(&nodes, 0, 1).2;
+	create_announced_chan_between_nodes(&nodes, 0, 1);
 	let channel_id_2 = create_announced_chan_between_nodes(&nodes, 2, 1).2;
 
 	// Marshall data to send the payment
@@ -2801,8 +2801,7 @@ fn do_automatic_retries(test: AutoRetry) {
 
 		// Restart the node and ensure that ChannelManager does not use its remaining retry attempt
 		let node_encoded = nodes[0].node.encode();
-		let mon_ser = get_monitor!(nodes[0], channel_id_1).encode();
-		reload_node!(nodes[0], node_encoded, &[&mon_ser], persister, chain_monitor, node_a_reload);
+		reload_node_and_monitors!(nodes[0], node_encoded, persister, chain_monitor, node_a_reload);
 
 		nodes[0].node.process_pending_htlc_forwards();
 		// Make sure we don't retry again.

lightning/src/ln/reload_tests.rs

@@ -93,11 +93,9 @@ fn test_funding_peer_disconnect() {
 	nodes[1].node.handle_channel_reestablish(nodes[0].node.get_our_node_id(), &as_reestablish);
 	let events_4 = nodes[1].node.get_and_clear_pending_msg_events();
 	assert_eq!(events_4.len(), 3);
-	let chan_id;
 	let bs_channel_ready = match events_4[0] {
 		MessageSendEvent::SendChannelReady { ref node_id, ref msg } => {
 			assert_eq!(*node_id, nodes[0].node.get_our_node_id());
-			chan_id = msg.channel_id;
 			msg.clone()
 		},
 		_ => panic!("Unexpected event {:?}", events_4[0]),
@@ -183,9 +181,7 @@ fn test_funding_peer_disconnect() {
 	// channel_announcement from the cached signatures.
 	nodes[1].node.peer_disconnected(nodes[0].node.get_our_node_id());
 
-	let chan_0_monitor_serialized = get_monitor!(nodes[0], chan_id).encode();
-
-	reload_node!(nodes[0], &nodes[0].node.encode(), &[&chan_0_monitor_serialized], persister, new_chain_monitor, nodes_0_deserialized);
+	reload_node_and_monitors!(nodes[0], &nodes[0].node.encode(), persister, new_chain_monitor, nodes_0_deserialized);
 
 	reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1]));
 }
@@ -205,10 +201,7 @@ fn test_no_txn_manager_serialize_deserialize() {
 
 	nodes[1].node.peer_disconnected(nodes[0].node.get_our_node_id());
 
-	let chan_0_monitor_serialized =
-		get_monitor!(nodes[0], ChannelId::v1_from_funding_outpoint(OutPoint { txid: tx.compute_txid(), index: 0 })).encode();
-	reload_node!(nodes[0], nodes[0].node.encode(), &[&chan_0_monitor_serialized], persister, new_chain_monitor, nodes_0_deserialized);
-
+	reload_node_and_monitors!(nodes[0], nodes[0].node.encode(), persister, new_chain_monitor, nodes_0_deserialized);
 	nodes[0].node.peer_connected(nodes[1].node.get_our_node_id(), &msgs::Init {
 		features: nodes[1].node.init_features(), networks: None, remote_network_address: None
 	}, true).unwrap();
@@ -291,11 +284,9 @@ fn test_manager_serialize_deserialize_events() {
 	nodes.push(node_b);
 
 	// Start the de/seriailization process mid-channel creation to check that the channel manager will hold onto events that are serialized
-	let chan_0_monitor_serialized = get_monitor!(nodes[0], bs_funding_signed.channel_id).encode();
-	reload_node!(nodes[0], nodes[0].node.encode(), &[&chan_0_monitor_serialized], persister, new_chain_monitor, nodes_0_deserialized);
+	reload_node_and_monitors!(nodes[0], nodes[0].node.encode(), persister, new_chain_monitor, nodes_0_deserialized);
 
 	nodes[1].node.peer_disconnected(nodes[0].node.get_our_node_id());
-
 	// After deserializing, make sure the funding_transaction is still held by the channel manager
 	let events_4 = nodes[0].node.get_and_clear_pending_events();
 	assert_eq!(events_4.len(), 0);
@@ -341,15 +332,14 @@ fn test_simple_manager_serialize_deserialize() {
 	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
 	let nodes_0_deserialized;
 	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
-	let chan_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;
+	create_announced_chan_between_nodes(&nodes, 0, 1);
 
 	let (our_payment_preimage, ..) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
 	let (_, our_payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
 
 	nodes[1].node.peer_disconnected(nodes[0].node.get_our_node_id());
 
-	let chan_0_monitor_serialized = get_monitor!(nodes[0], chan_id).encode();
-	reload_node!(nodes[0], nodes[0].node.encode(), &[&chan_0_monitor_serialized], persister, new_chain_monitor, nodes_0_deserialized);
+	reload_node_and_monitors!(nodes[0], nodes[0].node.encode(), persister, new_chain_monitor, nodes_0_deserialized);
 
 	reconnect_nodes(ReconnectArgs::new(&nodes[0], &nodes[1]));
 
@@ -1112,7 +1102,7 @@ fn removed_payment_no_manager_persistence() {
 
 	let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
 
-	let chan_id_1 = create_announced_chan_between_nodes(&nodes, 0, 1).2;
+	create_announced_chan_between_nodes(&nodes, 0, 1);
 	let chan_id_2 = create_announced_chan_between_nodes(&nodes, 1, 2).2;
 
 	let (_, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1_000_000);
@@ -1135,9 +1125,7 @@ fn removed_payment_no_manager_persistence() {
 		_ => panic!("Unexpected event"),
 	}
 
-	let chan_0_monitor_serialized = get_monitor!(nodes[1], chan_id_1).encode();
-	let chan_1_monitor_serialized = get_monitor!(nodes[1], chan_id_2).encode();
-	reload_node!(nodes[1], node_encoded, &[&chan_0_monitor_serialized, &chan_1_monitor_serialized], persister, new_chain_monitor, nodes_1_deserialized);
+	reload_node_and_monitors!(nodes[1], node_encoded, persister, new_chain_monitor, nodes_1_deserialized);
 
 	match nodes[1].node.pop_pending_event().unwrap() {
 		Event::ChannelClosed { ref reason, .. } => {
@@ -1206,8 +1194,7 @@ fn test_reload_partial_funding_batch() {
 	// Reload the node while a subset of the channels in the funding batch have persisted monitors.
 	let channel_id_1 = ChannelId::v1_from_funding_outpoint(OutPoint { txid: tx.compute_txid(), index: 0 });
 	let node_encoded = nodes[0].node.encode();
-	let channel_monitor_1_serialized = get_monitor!(nodes[0], channel_id_1).encode();
-	reload_node!(nodes[0], node_encoded, &[&channel_monitor_1_serialized], new_persister, new_chain_monitor, new_channel_manager);
+	reload_node_and_monitors!(nodes[0], node_encoded, new_persister, new_chain_monitor, new_channel_manager);
 
 	// Process monitor events.
 	assert!(nodes[0].node.get_and_clear_pending_events().is_empty());
@@ -1283,8 +1270,7 @@ fn test_htlc_localremoved_persistence() {
 	nodes[0].node.peer_disconnected(nodes[1].node.get_our_node_id());
 	nodes[1].node.peer_disconnected(nodes[0].node.get_our_node_id());
 
-	let monitor_encoded = get_monitor!(nodes[1], _chan.3).encode();
-	reload_node!(nodes[1], nodes[1].node.encode(), &[&monitor_encoded], persister, chain_monitor, deserialized_chanmgr);
+	reload_node_and_monitors!(nodes[1], nodes[1].node.encode(), persister, chain_monitor, deserialized_chanmgr);
 
 	nodes[0].node.peer_connected(nodes[1].node.get_our_node_id(), &msgs::Init {
 		features: nodes[1].node.init_features(), networks: None, remote_network_address: None
@@ -1419,4 +1405,3 @@ fn test_peer_storage() {
 	let res = std::panic::catch_unwind(|| drop(nodes));
 	assert!(res.is_err());
 }
-

lightning/src/util/test_utils.rs

@@ -486,6 +486,8 @@ pub struct TestChainMonitor<'a> {
 	pub expect_monitor_round_trip_fail: Mutex<Option<ChannelId>>,
 	#[cfg(feature = "std")]
 	pub write_blocker: Mutex<Option<std::sync::mpsc::Receiver<()>>>,
+	/// The latest persisted monitor for each channel.
+	pub persisted_monitors: Mutex<HashMap<ChannelId, Vec<u8>>>,
 }
 impl<'a> TestChainMonitor<'a> {
 	pub fn new(
@@ -511,6 +513,7 @@ impl<'a> TestChainMonitor<'a> {
 			expect_monitor_round_trip_fail: Mutex::new(None),
 			#[cfg(feature = "std")]
 			write_blocker: Mutex::new(None),
+			persisted_monitors: Mutex::new(new_hash_map()),
 		}
 	}
 
@@ -564,6 +567,9 @@ impl<'a> chain::Watch<TestChannelSigner> for TestChainMonitor<'a> {
 		// monitor to a serialized copy and get he same one back.
 		let mut w = TestVecWriter(Vec::new());
 		monitor.write(&mut w).unwrap();
+
+		self.persisted_monitors.lock().unwrap().insert(channel_id, w.0.clone());
+
 		let new_monitor = <(BlockHash, ChannelMonitor<TestChannelSigner>)>::read(
 			&mut io::Cursor::new(&w.0),
 			(self.keys_manager, self.keys_manager),
@@ -620,6 +626,9 @@ impl<'a> chain::Watch<TestChannelSigner> for TestChainMonitor<'a> {
 		let monitor = self.chain_monitor.get_monitor(channel_id).unwrap();
 		w.0.clear();
 		monitor.write(&mut w).unwrap();
+
+		self.persisted_monitors.lock().unwrap().insert(channel_id, w.0.clone());
+
 		let new_monitor = <(BlockHash, ChannelMonitor<TestChannelSigner>)>::read(
 			&mut io::Cursor::new(&w.0),
 			(self.keys_manager, self.keys_manager),