From 2ff4ae782eccb84e09f86e7d37c426e6153ea961 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 7 Aug 2020 16:27:26 -0400 Subject: [PATCH] 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. --- fuzz/src/chanmon_consistency.rs | 2 +- lightning/src/ln/channelmanager.rs | 24 +++++++++++++++++++++-- lightning/src/ln/functional_test_utils.rs | 2 +- lightning/src/ln/functional_tests.rs | 12 ++++++------ 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 0fc77c58f..609ce996a 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -242,7 +242,7 @@ pub fn do_test(data: &[u8], out: Out) { tx_broadcaster: broadcast.clone(), logger, default_config: config, - channel_monitors: &mut monitor_refs, + channel_monitors: monitor_refs, }; (<(BlockHash, ChannelManager, Arc, Arc, Arc, Arc>)>::read(&mut Cursor::new(&$ser.0), read_args).expect("Failed to read manager").1, monitor) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 83746d01a..dfa12e1c3 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3775,7 +3775,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>, + pub channel_monitors: HashMap>, +} + +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, + T::Target: BroadcasterInterface, + K::Target: KeysInterface, + 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 new(keys_manager: K, fee_estimator: F, monitor: M, tx_broadcaster: T, logger: L, default_config: UserConfig, + mut channel_monitors: Vec<&'a mut ChannelMonitor>) -> 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 @@ -3802,7 +3822,7 @@ impl<'a, ChanSigner: ChannelKeys + Readable, M: Deref, T: Deref, K: Deref, F: De F::Target: FeeEstimator, L::Target: Logger, { - fn read(reader: &mut R, args: ChannelManagerReadArgs<'a, ChanSigner, M, T, K, F, L>) -> Result { + fn read(reader: &mut R, mut args: ChannelManagerReadArgs<'a, ChanSigner, M, T, K, F, L>) -> Result { let _ver: u8 = Readable::read(reader)?; let min_ver: u8 = Readable::read(reader)?; if min_ver > SERIALIZATION_VERSION { diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 904212095..b3d5e8c9e 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/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(); } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index f44a8d59e..9087b17a8 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -4318,7 +4318,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; @@ -4426,7 +4426,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; @@ -4516,7 +4516,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; @@ -4606,7 +4606,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"); }; @@ -4620,7 +4620,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()); @@ -7891,7 +7891,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; -- 2.39.5