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.

diff --git a/lightning/src/ln/chanmon_update_fail_tests.rs b/lightning/src/ln/chanmon_update_fail_tests.rs
index 236b1e498f45ef26b86b1b5b5d46d870028e70ea..9770638db52d346811aec91270300135cf3ca7b3 100644 (file)
--- 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 3b0c9ab24f3d0fcadd6212b4df48507bb0b6f69b..a923d397be2edc662f801d18f81eb2c100886cee 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -4259,7 +4259,7 @@ impl<Signer: Sign> Channel<Signer> {
        /// 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<Signer: Sign> Channel<Signer> {
                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()[..];