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.

diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs
index 0fc77c58f56c4bd1c8d6d2584639da30bfeb9d42..609ce996a18528e9990ced697b6992f6a2f5263a 100644 (file)
--- a/fuzz/src/chanmon_consistency.rs
+++ b/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)

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 83746d01a94c86d1d062c61f9f36b6e5224f5f9f..dfa12e1c3365bbcc319eb3758c4edb2d3f687703 100644 (file)
--- 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<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 new(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
@@ -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<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 {

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index 9042120955925bb338115aab1d8210ad4165256a..b3d5e8c9e9da934a72989a88a287816cae79446a 100644 (file)
--- 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 f44a8d59e0981c75d2207163e0a7f8aaf317a89f..9087b17a81950028857e8fb776ea21989fb5d531 100644 (file)
--- 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;