From 775a5adfb917f33b6d781f16660cd14134bb0250 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 9 Jun 2020 23:00:30 -0400 Subject: [PATCH] ChannelMonitor::get_funding_txo returns both the txid and scriptPK ... instead of only the txid. This is another instance of it not being possible to fully re-implement SimpleManyChannelMonitor using only public methods. In this case you couldn't properly register outpoints for monitoring so that the funding transaction would be matched. --- lightning/src/ln/channelmanager.rs | 2 +- lightning/src/ln/channelmonitor.rs | 19 +++++++++++-------- lightning/src/ln/functional_test_utils.rs | 4 ++-- lightning/src/ln/functional_tests.rs | 18 +++++++++--------- 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index c0bea87b0..f453ffc06 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -2274,7 +2274,7 @@ impl }; // Because we have exclusive ownership of the channel here we can release the channel_state // lock before add_monitor - if let Err(e) = self.monitor.add_monitor(monitor_update.get_funding_txo(), monitor_update) { + if let Err(e) = self.monitor.add_monitor(monitor_update.get_funding_txo().0, monitor_update) { match e { ChannelMonitorUpdateErr::PermanentFailure => { // Note that we reply with the new channel_id in error messages if we gave up on the diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index d8689faeb..330427118 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -235,12 +235,15 @@ impl return Err(MonitorUpdateError("Channel monitor for given key is already present")), hash_map::Entry::Vacant(e) => e, }; - log_trace!(self.logger, "Got new Channel Monitor for channel {}", log_bytes!(monitor.funding_info.0.to_channel_id()[..])); - self.chain_monitor.install_watch_tx(&monitor.funding_info.0.txid, &monitor.funding_info.1); - self.chain_monitor.install_watch_outpoint((monitor.funding_info.0.txid, monitor.funding_info.0.index as u32), &monitor.funding_info.1); - for (txid, outputs) in monitor.get_outputs_to_watch().iter() { - for (idx, script) in outputs.iter().enumerate() { - self.chain_monitor.install_watch_outpoint((*txid, idx as u32), script); + { + let funding_txo = monitor.get_funding_txo(); + log_trace!(self.logger, "Got new Channel Monitor for channel {}", log_bytes!(funding_txo.0.to_channel_id()[..])); + self.chain_monitor.install_watch_tx(&funding_txo.0.txid, &funding_txo.1); + self.chain_monitor.install_watch_outpoint((funding_txo.0.txid, funding_txo.0.index as u32), &funding_txo.1); + for (txid, outputs) in monitor.get_outputs_to_watch().iter() { + for (idx, script) in outputs.iter().enumerate() { + self.chain_monitor.install_watch_outpoint((*txid, idx as u32), script); + } } } entry.insert(monitor); @@ -1408,8 +1411,8 @@ impl ChannelMonitor { } /// Gets the funding transaction outpoint of the channel this ChannelMonitor is monitoring for. - pub fn get_funding_txo(&self) -> OutPoint { - self.funding_info.0 + pub fn get_funding_txo(&self) -> &(OutPoint, Script) { + &self.funding_info } /// Gets a list of txids, with their output scripts (in the order they appear in the diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 547e58bc2..41cd1610e 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -148,7 +148,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> { { let mut channel_monitors = HashMap::new(); for monitor in deserialized_monitors.iter_mut() { - channel_monitors.insert(monitor.get_funding_txo(), monitor); + channel_monitors.insert(monitor.get_funding_txo().0, monitor); } let mut w = test_utils::TestVecWriter(Vec::new()); @@ -167,7 +167,7 @@ impl<'a, 'b, 'c> Drop for Node<'a, 'b, 'c> { let chain_watch = chaininterface::ChainWatchInterfaceUtil::new(Network::Testnet); let channel_monitor = test_utils::TestChannelMonitor::new(&chain_watch, self.tx_broadcaster.clone(), &self.logger, &feeest); for deserialized_monitor in deserialized_monitors.drain(..) { - if let Err(_) = channel_monitor.add_monitor(deserialized_monitor.get_funding_txo(), deserialized_monitor) { + if let Err(_) = channel_monitor.add_monitor(deserialized_monitor.get_funding_txo().0, deserialized_monitor) { panic!(); } } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index e7d793b7d..7f79e9387 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -4083,7 +4083,7 @@ fn test_no_txn_manager_serialize_deserialize() { keys_manager = test_utils::TestKeysInterface::new(&nodes[0].node_seed, Network::Testnet); let (_, nodes_0_deserialized_tmp) = { let mut channel_monitors = HashMap::new(); - channel_monitors.insert(chan_0_monitor.get_funding_txo(), &mut chan_0_monitor); + channel_monitors.insert(chan_0_monitor.get_funding_txo().0, &mut chan_0_monitor); <(BlockHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { default_config: config, keys_manager: &keys_manager, @@ -4097,7 +4097,7 @@ fn test_no_txn_manager_serialize_deserialize() { nodes_0_deserialized = nodes_0_deserialized_tmp; assert!(nodes_0_read.is_empty()); - assert!(nodes[0].chan_monitor.add_monitor(chan_0_monitor.get_funding_txo(), chan_0_monitor).is_ok()); + assert!(nodes[0].chan_monitor.add_monitor(chan_0_monitor.get_funding_txo().0, chan_0_monitor).is_ok()); nodes[0].node = &nodes_0_deserialized; nodes[0].block_notifier.register_listener(nodes[0].node); assert_eq!(nodes[0].node.list_channels().len(), 1); @@ -4191,7 +4191,7 @@ fn test_manager_serialize_deserialize_events() { keys_manager = test_utils::TestKeysInterface::new(&nodes[0].node_seed, Network::Testnet); let (_, nodes_0_deserialized_tmp) = { let mut channel_monitors = HashMap::new(); - channel_monitors.insert(chan_0_monitor.get_funding_txo(), &mut chan_0_monitor); + channel_monitors.insert(chan_0_monitor.get_funding_txo().0, &mut chan_0_monitor); <(BlockHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { default_config: config, keys_manager: &keys_manager, @@ -4207,7 +4207,7 @@ fn test_manager_serialize_deserialize_events() { nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false); - assert!(nodes[0].chan_monitor.add_monitor(chan_0_monitor.get_funding_txo(), chan_0_monitor).is_ok()); + assert!(nodes[0].chan_monitor.add_monitor(chan_0_monitor.get_funding_txo().0, chan_0_monitor).is_ok()); nodes[0].node = &nodes_0_deserialized; // After deserializing, make sure the FundingBroadcastSafe event is still held by the channel manager @@ -4281,7 +4281,7 @@ fn test_simple_manager_serialize_deserialize() { keys_manager = test_utils::TestKeysInterface::new(&nodes[0].node_seed, Network::Testnet); let (_, nodes_0_deserialized_tmp) = { let mut channel_monitors = HashMap::new(); - channel_monitors.insert(chan_0_monitor.get_funding_txo(), &mut chan_0_monitor); + channel_monitors.insert(chan_0_monitor.get_funding_txo().0, &mut chan_0_monitor); <(BlockHash, ChannelManager)>::read(&mut nodes_0_read, ChannelManagerReadArgs { default_config: UserConfig::default(), keys_manager: &keys_manager, @@ -4295,7 +4295,7 @@ fn test_simple_manager_serialize_deserialize() { nodes_0_deserialized = nodes_0_deserialized_tmp; assert!(nodes_0_read.is_empty()); - assert!(nodes[0].chan_monitor.add_monitor(chan_0_monitor.get_funding_txo(), chan_0_monitor).is_ok()); + assert!(nodes[0].chan_monitor.add_monitor(chan_0_monitor.get_funding_txo().0, chan_0_monitor).is_ok()); nodes[0].node = &nodes_0_deserialized; check_added_monitors!(nodes[0], 1); @@ -4379,7 +4379,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(), monitor) }).collect(), + channel_monitors: &mut 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"); }; @@ -4393,7 +4393,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(), monitor) }).collect(), + channel_monitors: &mut 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()); @@ -4406,7 +4406,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() { } for monitor in node_0_monitors.drain(..) { - assert!(nodes[0].chan_monitor.add_monitor(monitor.get_funding_txo(), monitor).is_ok()); + assert!(nodes[0].chan_monitor.add_monitor(monitor.get_funding_txo().0, monitor).is_ok()); check_added_monitors!(nodes[0], 1); } nodes[0].node = &nodes_0_deserialized; -- 2.39.5