Give ChannelManagerReadArgs HashMap-of-monitors ownership

Its somewhat awkward that ChannelManagerReadArgs requires a mutable
reference to a HashMap of ChannelMonitors, forcing the callsite to
define a scope for the HashMap which they almost certainly won't use
after deserializing the ChannelManager. Worse, to map the current
version to C bindings, we'd need to also create a HashMap binding,
which is overkill for just this one use.

Instead, we just give the ReadArgs struct ownership of the HashMap
and add a constructor which fills the HashMap for you.

Author: TheBlueMatt

fuzz/src/chanmon_consistency.rs

@@ -242,7 +242,7 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 				tx_broadcaster: broadcast.clone(),
 				logger,
 				default_config: config,
-				channel_monitors: &mut monitor_refs,
+				channel_monitors: monitor_refs,
 			};
 
 			(<(BlockHash, ChannelManager<EnforcingChannelKeys, Arc<TestChannelMonitor>, Arc<TestBroadcaster>, Arc<KeyProvider>, Arc<FuzzEstimator>, Arc<dyn Logger>>)>::read(&mut Cursor::new(&$ser.0), read_args).expect("Failed to read manager").1, monitor)

lightning/src/ln/channelmanager.rs

@@ -3772,7 +3772,27 @@ pub struct ChannelManagerReadArgs<'a, ChanSigner: 'a + ChannelKeys, M: Deref, T:
 	///
 	/// In such cases the latest local transactions will be sent to the tx_broadcaster included in
 	/// this struct.
-	pub channel_monitors: &'a mut HashMap<OutPoint, &'a mut ChannelMonitor<ChanSigner>>,
+	pub channel_monitors: HashMap<OutPoint, &'a mut ChannelMonitor<ChanSigner>>,
+}
+
+impl<'a, ChanSigner: 'a + ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
+		ChannelManagerReadArgs<'a, ChanSigner, M, T, K, F, L>
+	where M::Target: ManyChannelMonitor<Keys=ChanSigner>,
+		T::Target: BroadcasterInterface,
+		K::Target: KeysInterface<ChanKeySigner = ChanSigner>,
+		F::Target: FeeEstimator,
+		L::Target: Logger,
+	{
+	/// Simple utility function to create a ChannelManagerReadArgs which creates the monitor
+	/// HashMap for you. This is primarily useful for C bindings where it is not practical to
+	/// populate a HashMap directly from C.
+	pub fn construct(keys_manager: K, fee_estimator: F, monitor: M, tx_broadcaster: T, logger: L, default_config: UserConfig,
+			mut channel_monitors: Vec<&'a mut ChannelMonitor<ChanSigner>>) -> Self {
+		Self {
+			keys_manager, fee_estimator, monitor, tx_broadcaster, logger, default_config,
+			channel_monitors: channel_monitors.drain(..).map(|monitor| { (monitor.get_funding_txo().0, monitor) }).collect()
+		}
+	}
 }
 
 // Implement ReadableArgs for an Arc'd ChannelManager to make it a bit easier to work with the
@@ -3799,7 +3819,7 @@ impl<'a, ChanSigner: ChannelKeys + Readable, M: Deref, T: Deref, K: Deref, F: De
         F::Target: FeeEstimator,
         L::Target: Logger,
 {
-	fn read<R: ::std::io::Read>(reader: &mut R, args: ChannelManagerReadArgs<'a, ChanSigner, M, T, K, F, L>) -> Result<Self, DecodeError> {
+	fn read<R: ::std::io::Read>(reader: &mut R, mut args: ChannelManagerReadArgs<'a, ChanSigner, M, T, K, F, L>) -> Result<Self, DecodeError> {
 		let _ver: u8 = Readable::read(reader)?;
 		let min_ver: u8 = Readable::read(reader)?;
 		if min_ver > SERIALIZATION_VERSION {

lightning/src/ln/functional_test_utils.rs

@@ -169,7 +169,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> {
 					monitor: self.chan_monitor,
 					tx_broadcaster: self.tx_broadcaster.clone(),
 					logger: &test_utils::TestLogger::new(),
-					channel_monitors: &mut channel_monitors,
+					channel_monitors,
 				}).unwrap();
 			}
 

lightning/src/ln/functional_tests.rs

@@ -4301,7 +4301,7 @@ fn test_no_txn_manager_serialize_deserialize() {
 			monitor: nodes[0].chan_monitor,
 			tx_broadcaster: nodes[0].tx_broadcaster.clone(),
 			logger: &logger,
-			channel_monitors: &mut channel_monitors,
+			channel_monitors,
 		}).unwrap()
 	};
 	nodes_0_deserialized = nodes_0_deserialized_tmp;
@@ -4409,7 +4409,7 @@ fn test_manager_serialize_deserialize_events() {
 			monitor: nodes[0].chan_monitor,
 			tx_broadcaster: nodes[0].tx_broadcaster.clone(),
 			logger: &logger,
-			channel_monitors: &mut channel_monitors,
+			channel_monitors,
 		}).unwrap()
 	};
 	nodes_0_deserialized = nodes_0_deserialized_tmp;
@@ -4499,7 +4499,7 @@ fn test_simple_manager_serialize_deserialize() {
 			monitor: nodes[0].chan_monitor,
 			tx_broadcaster: nodes[0].tx_broadcaster.clone(),
 			logger: &logger,
-			channel_monitors: &mut channel_monitors,
+			channel_monitors,
 		}).unwrap()
 	};
 	nodes_0_deserialized = nodes_0_deserialized_tmp;
@@ -4589,7 +4589,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
 		monitor: nodes[0].chan_monitor,
 		tx_broadcaster: nodes[0].tx_broadcaster.clone(),
 		logger: &logger,
-		channel_monitors: &mut node_0_stale_monitors.iter_mut().map(|monitor| { (monitor.get_funding_txo().0, monitor) }).collect(),
+		channel_monitors: node_0_stale_monitors.iter_mut().map(|monitor| { (monitor.get_funding_txo().0, monitor) }).collect(),
 	}) { } else {
 		panic!("If the monitor(s) are stale, this indicates a bug and we should get an Err return");
 	};
@@ -4603,7 +4603,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
 		monitor: nodes[0].chan_monitor,
 		tx_broadcaster: nodes[0].tx_broadcaster.clone(),
 		logger: &logger,
-		channel_monitors: &mut node_0_monitors.iter_mut().map(|monitor| { (monitor.get_funding_txo().0, monitor) }).collect(),
+		channel_monitors: node_0_monitors.iter_mut().map(|monitor| { (monitor.get_funding_txo().0, monitor) }).collect(),
 	}).unwrap();
 	nodes_0_deserialized = nodes_0_deserialized_tmp;
 	assert!(nodes_0_read.is_empty());
@@ -7874,7 +7874,7 @@ fn test_data_loss_protect() {
 			logger: &logger,
 			tx_broadcaster: &tx_broadcaster,
 			default_config: UserConfig::default(),
-			channel_monitors: &mut channel_monitors,
+			channel_monitors,
 		}).unwrap().1
 	};
 	nodes[0].node = &node_state_0;