Deterministic reconstruct_manager option in tests

We recently merged (test-only, for now) support for the ChannelManager
reconstructing its set of pending HTLCs from Channel{Monitor} data, rather than
using its own persisted maps. But because we want test coverage of both the new
reconstruction codepaths as well as the old persisted map codepaths,
in tests we would decide between those two sets of codepaths randomly.

We now want to add some tests that require using the new codepaths, so here we
add an option to explicitly set whether to reconstruct or not rather than
choosing randomly.

Author: valentinewallace

lightning/src/ln/channelmanager.rs

@@ -17683,6 +17683,15 @@ pub struct ChannelManagerReadArgs<
 	///
 	/// This is not exported to bindings users because we have no HashMap bindings
 	pub channel_monitors: HashMap<ChannelId, &'a ChannelMonitor<SP::EcdsaSigner>>,
+
+	/// Whether the `ChannelManager` should attempt to reconstruct its set of pending HTLCs from
+	/// `Channel{Monitor}` data rather than its own persisted maps, which is planned to become
+	/// the default behavior in upcoming versions.
+	///
+	/// If `None`, whether we reconstruct or use the legacy maps will be decided randomly during
+	/// `ChannelManager::from_channel_manager_data`.
+	#[cfg(test)]
+	pub reconstruct_manager_from_monitors: Option<bool>,
 }
 
 impl<
@@ -17720,6 +17729,8 @@ impl<
 			channel_monitors: hash_map_from_iter(
 				channel_monitors.drain(..).map(|monitor| (monitor.channel_id(), monitor)),
 			),
+			#[cfg(test)]
+			reconstruct_manager_from_monitors: None,
 		}
 	}
 }
@@ -18409,26 +18420,30 @@ impl<
 		#[cfg(not(test))]
 		let reconstruct_manager_from_monitors = false;
 		#[cfg(test)]
-		let reconstruct_manager_from_monitors = {
-			use core::hash::{BuildHasher, Hasher};
-
-			match std::env::var("LDK_TEST_REBUILD_MGR_FROM_MONITORS") {
-				Ok(val) => match val.as_str() {
-					"1" => true,
-					"0" => false,
-					_ => panic!("LDK_TEST_REBUILD_MGR_FROM_MONITORS must be 0 or 1, got: {}", val),
-				},
-				Err(_) => {
-					let rand_val =
-						std::collections::hash_map::RandomState::new().build_hasher().finish();
-					if rand_val % 2 == 0 {
-						true
-					} else {
-						false
-					}
-				},
-			}
-		};
+		let reconstruct_manager_from_monitors =
+			args.reconstruct_manager_from_monitors.unwrap_or_else(|| {
+				use core::hash::{BuildHasher, Hasher};
+
+				match std::env::var("LDK_TEST_REBUILD_MGR_FROM_MONITORS") {
+					Ok(val) => match val.as_str() {
+						"1" => true,
+						"0" => false,
+						_ => panic!(
+							"LDK_TEST_REBUILD_MGR_FROM_MONITORS must be 0 or 1, got: {}",
+							val
+						),
+					},
+					Err(_) => {
+						let rand_val =
+							std::collections::hash_map::RandomState::new().build_hasher().finish();
+						if rand_val % 2 == 0 {
+							true
+						} else {
+							false
+						}
+					},
+				}
+			});
 
 		// If there's any preimages for forwarded HTLCs hanging around in ChannelMonitors we
 		// should ensure we try them again on the inbound edge. We put them here and do so after we

lightning/src/ln/functional_test_utils.rs

@@ -909,6 +909,8 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
 						tx_broadcaster: &broadcaster,
 						logger: &self.logger,
 						channel_monitors,
+						#[cfg(test)]
+						reconstruct_manager_from_monitors: None,
 					},
 				)
 				.unwrap();
@@ -1307,7 +1309,7 @@ fn check_claimed_htlcs_match_route<'a, 'b, 'c>(
 
 pub fn _reload_node<'a, 'b, 'c>(
 	node: &'a Node<'a, 'b, 'c>, config: UserConfig, chanman_encoded: &[u8],
-	monitors_encoded: &[&[u8]],
+	monitors_encoded: &[&[u8]], _reconstruct_manager_from_monitors: Option<bool>,
 ) -> TestChannelManager<'b, 'c> {
 	let mut monitors_read = Vec::with_capacity(monitors_encoded.len());
 	for encoded in monitors_encoded {
@@ -1341,6 +1343,8 @@ pub fn _reload_node<'a, 'b, 'c>(
 				tx_broadcaster: node.tx_broadcaster,
 				logger: node.logger,
 				channel_monitors,
+				#[cfg(test)]
+				reconstruct_manager_from_monitors: _reconstruct_manager_from_monitors,
 			},
 		)
 		.unwrap()
@@ -1376,7 +1380,7 @@ macro_rules! reload_node {
 		$node.chain_monitor = &$new_chain_monitor;
 
 		$new_channelmanager =
-			_reload_node(&$node, $new_config, &chanman_encoded, $monitors_encoded);
+			_reload_node(&$node, $new_config, &chanman_encoded, $monitors_encoded, None);
 		$node.node = &$new_channelmanager;
 		$node.onion_messenger.set_offers_handler(&$new_channelmanager);
 		$node.onion_messenger.set_async_payments_handler(&$new_channelmanager);

lightning/src/ln/reload_tests.rs

@@ -437,6 +437,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
 		tx_broadcaster: nodes[0].tx_broadcaster,
 		logger: &logger,
 		channel_monitors: node_0_stale_monitors.iter().map(|monitor| { (monitor.channel_id(), monitor) }).collect(),
+		reconstruct_manager_from_monitors: None,
 	}) { } else {
 		panic!("If the monitor(s) are stale, this indicates a bug and we should get an Err return");
 	};
@@ -455,6 +456,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
 		tx_broadcaster: nodes[0].tx_broadcaster,
 		logger: &logger,
 		channel_monitors: node_0_monitors.iter().map(|monitor| { (monitor.channel_id(), monitor) }).collect(),
+		reconstruct_manager_from_monitors: None,
 	}).unwrap();
 	nodes_0_deserialized = nodes_0_deserialized_tmp;
 	assert!(nodes_0_read.is_empty());