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.

Author: TheBlueMatt

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,20 +1853,32 @@ 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();
 	nodes[1].chain_monitor.chain_monitor.force_channel_monitor_updated(outpoint, latest_update);
 	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]

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()[..];