From 821f6cdd1ed77d0c62b0a20120a2644e71a1b464 Mon Sep 17 00:00:00 2001 From: Sergi Delgado Segura Date: Thu, 14 Jan 2021 17:05:38 +0100 Subject: [PATCH] Makes ChannelManager::force_close_channel fail for unknown chan_ids ChannelManager::force_close_channel does not fail if a non-existing channel id is being passed, making it hard to catch from an API point of view. Makes force_close_channel return in the same way close_channel does so the user calling the method with an unknown id can be warned. --- fuzz/src/full_stack.rs | 2 +- lightning-persister/src/lib.rs | 6 +++--- lightning/src/ln/chanmon_update_fail_tests.rs | 2 +- lightning/src/ln/channelmanager.rs | 16 ++++++++++------ lightning/src/ln/functional_tests.rs | 10 +++++----- 5 files changed, 20 insertions(+), 16 deletions(-) diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 915a6cba1..901c9ef1e 100644 --- a/fuzz/src/full_stack.rs +++ b/fuzz/src/full_stack.rs @@ -548,7 +548,7 @@ pub fn do_test(data: &[u8], logger: &Arc) { let channel_id = get_slice!(1)[0] as usize; if channel_id >= channels.len() { return; } channels.sort_by(|a, b| { a.channel_id.cmp(&b.channel_id) }); - channelmanager.force_close_channel(&channels[channel_id].channel_id); + channelmanager.force_close_channel(&channels[channel_id].channel_id).unwrap(); }, // 15 is above _ => return, diff --git a/lightning-persister/src/lib.rs b/lightning-persister/src/lib.rs index bb6eb54c9..c6b0ea641 100644 --- a/lightning-persister/src/lib.rs +++ b/lightning-persister/src/lib.rs @@ -240,7 +240,7 @@ mod tests { // Force close because cooperative close doesn't result in any persisted // updates. - nodes[0].node.force_close_channel(&nodes[0].node.list_channels()[0].channel_id); + nodes[0].node.force_close_channel(&nodes[0].node.list_channels()[0].channel_id).unwrap(); check_closed_broadcast!(nodes[0], false); check_added_monitors!(nodes[0], 1); @@ -354,7 +354,7 @@ mod tests { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); let chan = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); - nodes[1].node.force_close_channel(&chan.2); + nodes[1].node.force_close_channel(&chan.2).unwrap(); let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap(); // Set the persister's directory to read-only, which should result in @@ -390,7 +390,7 @@ mod tests { let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); let chan = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()); - nodes[1].node.force_close_channel(&chan.2); + nodes[1].node.force_close_channel(&chan.2).unwrap(); let mut added_monitors = nodes[1].chain_monitor.added_monitors.lock().unwrap(); // Create the persister with an invalid directory name and test that the diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index b9376029e..98bf905f1 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -239,7 +239,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool, persister_fail } // ...and make sure we can force-close a frozen channel - nodes[0].node.force_close_channel(&channel_id); + nodes[0].node.force_close_channel(&channel_id).unwrap(); check_added_monitors!(nodes[0], 1); check_closed_broadcast!(nodes[0], false); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 49c14a043..e353700be 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -917,8 +917,8 @@ impl } /// Force closes a channel, immediately broadcasting the latest local commitment transaction to - /// the chain and rejecting new HTLCs on the given channel. - pub fn force_close_channel(&self, channel_id: &[u8; 32]) { + /// the chain and rejecting new HTLCs on the given channel. Fails if channel_id is unknown to the manager. + pub fn force_close_channel(&self, channel_id: &[u8; 32]) -> Result<(), APIError>{ let _consistency_lock = self.total_consistency_lock.read().unwrap(); let mut chan = { @@ -930,7 +930,7 @@ impl } chan } else { - return; + return Err(APIError::ChannelUnavailable{err: "No such channel".to_owned()}); } }; log_trace!(self.logger, "Force-closing channel {}", log_bytes!(channel_id[..])); @@ -941,13 +941,15 @@ impl msg: update }); } + + Ok(()) } /// Force close all channels, immediately broadcasting the latest local commitment transaction /// for each to the chain and rejecting new HTLCs on each. pub fn force_close_all_channels(&self) { for chan in self.list_channels() { - self.force_close_channel(&chan.channel_id); + let _ = self.force_close_channel(&chan.channel_id); } } @@ -3471,11 +3473,13 @@ impl