From e7facb1b66a7b3bfdd3dedd41739dcedd8de4b11 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 18 Nov 2021 21:54:10 +0000 Subject: [PATCH] Unset `Channel::is_usable` if mon update is blocking funding_locked If we have not yet sent `funding_locked` only because of a pending channel monitor update, we shouldn't consider a channel `is_usable`. This has a number of downstream effects, including not attempting to route payments through the channel, not sending private `channel_update` messages to our counterparty, or sending channel_announcement messages if our couterparty has already signed for it. We further gate generation of `node_announcement`s on `is_usable`, preventing generation of those or `announcement_signatures` until we've sent our `funding_locked`. Finally, `during_funding_monitor_fail` is updated to test a case where we see the funding transaction lock in but have a pending monitor update failure, then receive `funding_locked` from our counterparty and ensure we don't generate the above messages until after the monitor update completes. --- lightning/src/ln/chanmon_update_fail_tests.rs | 35 +++++++++++++------ lightning/src/ln/channel.rs | 9 ++--- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs index 236b1e498..9770638db 100644 --- a/lightning/src/ln/chanmon_update_fail_tests.rs +++ b/lightning/src/ln/chanmon_update_fail_tests.rs @@ -1789,9 +1789,9 @@ fn monitor_update_claim_fail_no_response() { claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2); } -// confirm_a_first and restore_b_before_conf are wholly unrelated to earlier bools and // restore_b_before_conf has no meaning if !confirm_a_first -fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf: bool) { +// restore_b_before_lock has no meaning if confirm_a_first +fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf: bool, restore_b_before_lock: bool) { // Test that if the monitor update generated by funding_transaction_generated fails we continue // the channel setup happily after the update is restored. let chanmon_cfgs = create_chanmon_cfgs(2); @@ -1833,6 +1833,8 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf: if confirm_a_first { confirm_transaction(&nodes[0], &funding_tx); nodes[1].node.handle_funding_locked(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendFundingLocked, nodes[1].node.get_our_node_id())); + assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); } else { assert!(!restore_b_before_conf); confirm_transaction(&nodes[1], &funding_tx); @@ -1851,6 +1853,12 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf: assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); } + if !confirm_a_first && !restore_b_before_lock { + confirm_transaction(&nodes[0], &funding_tx); + nodes[1].node.handle_funding_locked(&nodes[0].node.get_our_node_id(), &get_event_msg!(nodes[0], MessageSendEvent::SendFundingLocked, nodes[1].node.get_our_node_id())); + assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); + } chanmon_cfgs[1].persister.set_update_ret(Ok(())); let (outpoint, latest_update, _) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone(); @@ -1858,13 +1866,19 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf: check_added_monitors!(nodes[1], 0); let (channel_id, (announcement, as_update, bs_update)) = if !confirm_a_first { - nodes[0].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendFundingLocked, nodes[0].node.get_our_node_id())); - - confirm_transaction(&nodes[0], &funding_tx); - let (funding_locked, channel_id) = create_chan_between_nodes_with_value_confirm_second(&nodes[1], &nodes[0]); - (channel_id, create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &funding_locked)) + if !restore_b_before_lock { + let (funding_locked, channel_id) = create_chan_between_nodes_with_value_confirm_second(&nodes[0], &nodes[1]); + (channel_id, create_chan_between_nodes_with_value_b(&nodes[1], &nodes[0], &funding_locked)) + } else { + nodes[0].node.handle_funding_locked(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendFundingLocked, nodes[0].node.get_our_node_id())); + confirm_transaction(&nodes[0], &funding_tx); + let (funding_locked, channel_id) = create_chan_between_nodes_with_value_confirm_second(&nodes[1], &nodes[0]); + (channel_id, create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &funding_locked)) + } } else { if restore_b_before_conf { + assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); confirm_transaction(&nodes[1], &funding_tx); } let (funding_locked, channel_id) = create_chan_between_nodes_with_value_confirm_second(&nodes[0], &nodes[1]); @@ -1884,9 +1898,10 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf: #[test] fn during_funding_monitor_fail() { - do_during_funding_monitor_fail(true, true); - do_during_funding_monitor_fail(true, false); - do_during_funding_monitor_fail(false, false); + do_during_funding_monitor_fail(true, true, false); + do_during_funding_monitor_fail(true, false, false); + do_during_funding_monitor_fail(false, false, false); + do_during_funding_monitor_fail(false, false, true); } #[test] diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 3b0c9ab24..a923d397b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4259,7 +4259,7 @@ impl Channel { /// Allowed in any state (including after shutdown) pub fn is_usable(&self) -> bool { let mask = ChannelState::ChannelFunded as u32 | BOTH_SIDES_SHUTDOWN_MASK; - (self.channel_state & mask) == (ChannelState::ChannelFunded as u32) + (self.channel_state & mask) == (ChannelState::ChannelFunded as u32) && !self.monitor_pending_funding_locked } /// Returns true if this channel is currently available for use. This is a superset of @@ -4670,11 +4670,8 @@ impl Channel { if !self.config.announced_channel { return Err(ChannelError::Ignore("Channel is not available for public announcements".to_owned())); } - if self.channel_state & (ChannelState::ChannelFunded as u32) == 0 { - return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement until the channel funding has been locked".to_owned())); - } - if (self.channel_state & (ChannelState::LocalShutdownSent as u32 | ChannelState::ShutdownComplete as u32)) != 0 { - return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement once the channel is closing".to_owned())); + if !self.is_usable() { + return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement if the channel is not currently usable".to_owned())); } let were_node_one = node_id.serialize()[..] < self.counterparty_node_id.serialize()[..]; -- 2.39.5