Rebroadcast channel_announcements when we broadcast a node_announce

Author: TheBlueMatt

lightning/src/ln/channel.rs

@@ -3831,6 +3831,8 @@ impl<Signer: Sign> Channel<Signer> {
 	/// closing).
 	/// Note that the "channel must be funded" requirement is stricter than BOLT 7 requires - see
 	/// https://github.com/lightningnetwork/lightning-rfc/issues/468
+	///
+	/// This will only return ChannelError::Ignore upon failure.
 	pub fn get_channel_announcement(&self, node_id: PublicKey, chain_hash: BlockHash) -> Result<(msgs::UnsignedChannelAnnouncement, Signature), ChannelError> {
 		if !self.config.announced_channel {
 			return Err(ChannelError::Ignore("Channel is not available for public announcements".to_owned()));
@@ -3861,6 +3863,8 @@ impl<Signer: Sign> Channel<Signer> {
 		Ok((msg, sig))
 	}
 
+	/// Signs the given channel announcement, returning a ChannelError::Ignore if no keys are
+	/// available.
 	fn sign_channel_announcement(&self, our_node_secret: &SecretKey, our_node_id: PublicKey, msghash: secp256k1::Message, announcement: msgs::UnsignedChannelAnnouncement, our_bitcoin_sig: Signature) -> Result<msgs::ChannelAnnouncement, ChannelError> {
 		if let Some((their_node_sig, their_bitcoin_sig)) = self.announcement_sigs {
 			let were_node_one = announcement.node_id_1 == our_node_id;
@@ -3902,6 +3906,20 @@ impl<Signer: Sign> Channel<Signer> {
 		self.sign_channel_announcement(our_node_secret, our_node_id, msghash, announcement, our_bitcoin_sig)
 	}
 
+	/// Gets a signed channel_announcement for this channel, if we previously received an
+	/// announcement_signatures from our counterparty.
+	pub fn get_signed_channel_announcement(&self, our_node_secret: &SecretKey, our_node_id: PublicKey, chain_hash: BlockHash) -> Option<msgs::ChannelAnnouncement> {
+		let (announcement, our_bitcoin_sig) = match self.get_channel_announcement(our_node_id.clone(), chain_hash) {
+			Ok(res) => res,
+			Err(_) => return None,
+		};
+		let msghash = hash_to_message!(&Sha256d::hash(&announcement.encode()[..])[..]);
+		match self.sign_channel_announcement(our_node_secret, our_node_id, msghash, announcement, our_bitcoin_sig) {
+			Ok(res) => Some(res),
+			Err(_) => None,
+		}
+	}
+
 	/// May panic if called on a channel that wasn't immediately-previously
 	/// self.remove_uncommitted_htlcs_and_mark_paused()'d
 	pub fn get_channel_reestablish<L: Deref>(&self, logger: &L) -> msgs::ChannelReestablish where L::Target: Logger {

lightning/src/ln/channelmanager.rs

@@ -1933,19 +1933,24 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 	// smaller than 500:
 	const STATIC_ASSERT: u32 = Self::HALF_MESSAGE_IS_ADDRS - 500;
 
-	/// Generates a signed node_announcement from the given arguments and creates a
-	/// BroadcastNodeAnnouncement event. Note that such messages will be ignored unless peers have
-	/// seen a channel_announcement from us (ie unless we have public channels open).
+	/// Regenerates channel_announcements and generates a signed node_announcement from the given
+	/// arguments, providing them in corresponding events via
+	/// [`get_and_clear_pending_msg_events`], if at least one public channel has been confirmed
+	/// on-chain. This effectively re-broadcasts all channel announcements and sends our node
+	/// announcement to ensure that the lightning P2P network is aware of the channels we have and
+	/// our network addresses.
 	///
-	/// RGB is a node "color" and alias is a printable human-readable string to describe this node
-	/// to humans. They carry no in-protocol meaning.
+	/// `rgb` is a node "color" and `alias` is a printable human-readable string to describe this
+	/// node to humans. They carry no in-protocol meaning.
 	///
-	/// addresses represent the set (possibly empty) of socket addresses on which this node accepts
-	/// incoming connections. These will be broadcast to the network, publicly tying these
-	/// addresses together. If you wish to preserve user privacy, addresses should likely contain
-	/// only Tor Onion addresses.
+	/// `addresses` represent the set (possibly empty) of socket addresses on which this node
+	/// accepts incoming connections. These will be included in the node_announcement, publicly
+	/// tying these addresses together and to this node. If you wish to preserve user privacy,
+	/// addresses should likely contain only Tor Onion addresses.
 	///
-	/// Panics if addresses is absurdly large (more than 500).
+	/// Panics if `addresses` is absurdly large (more than 500).
+	///
+	/// [`get_and_clear_pending_msg_events`]: MessageSendEventsProvider::get_and_clear_pending_msg_events
 	pub fn broadcast_node_announcement(&self, rgb: [u8; 3], alias: [u8; 32], mut addresses: Vec<NetAddress>) {
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
 
@@ -1966,14 +1971,37 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 			excess_data: Vec::new(),
 		};
 		let msghash = hash_to_message!(&Sha256dHash::hash(&announcement.encode()[..])[..]);
+		let node_announce_sig = self.secp_ctx.sign(&msghash, &self.our_network_key);
 
-		let mut channel_state = self.channel_state.lock().unwrap();
-		channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastNodeAnnouncement {
-			msg: msgs::NodeAnnouncement {
-				signature: self.secp_ctx.sign(&msghash, &self.our_network_key),
-				contents: announcement
-			},
-		});
+		let mut channel_state_lock = self.channel_state.lock().unwrap();
+		let channel_state = &mut *channel_state_lock;
+
+		let mut announced_chans = false;
+		for (_, chan) in channel_state.by_id.iter() {
+			if let Some(msg) = chan.get_signed_channel_announcement(&self.our_network_key, self.get_our_node_id(), self.genesis_hash.clone()) {
+				channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastChannelAnnouncement {
+					msg,
+					update_msg: match self.get_channel_update(chan) {
+						Ok(msg) => msg,
+						Err(_) => continue,
+					},
+				});
+				announced_chans = true;
+			} else {
+				// If the channel is not public or has not yet reached funding_locked, check the
+				// next channel. If we don't yet have any public channels, we'll skip the broadcast
+				// below as peers may not accept it without channels on chain first.
+			}
+		}
+
+		if announced_chans {
+			channel_state.pending_msg_events.push(events::MessageSendEvent::BroadcastNodeAnnouncement {
+				msg: msgs::NodeAnnouncement {
+					signature: node_announce_sig,
+					contents: announcement
+				},
+			});
+		}
 	}
 
 	/// Processes HTLCs which are pending waiting on random forward delay.

lightning/src/ln/functional_test_utils.rs

@@ -615,8 +615,22 @@ pub fn create_announced_chan_between_nodes_with_value<'a, 'b, 'c, 'd>(nodes: &'a
 pub fn update_nodes_with_chan_announce<'a, 'b, 'c, 'd>(nodes: &'a Vec<Node<'b, 'c, 'd>>, a: usize, b: usize, ann: &msgs::ChannelAnnouncement, upd_1: &msgs::ChannelUpdate, upd_2: &msgs::ChannelUpdate) {
 	nodes[a].node.broadcast_node_announcement([0, 0, 0], [0; 32], Vec::new());
 	let a_events = nodes[a].node.get_and_clear_pending_msg_events();
-	assert_eq!(a_events.len(), 1);
-	let a_node_announcement = match a_events[0] {
+	assert!(a_events.len() >= 2);
+
+	// ann should be re-generated by broadcast_node_announcement - check that we have it.
+	let mut found_ann_1 = false;
+	for event in a_events.iter() {
+		match event {
+			MessageSendEvent::BroadcastChannelAnnouncement { ref msg, .. } => {
+				if msg == ann { found_ann_1 = true; }
+			},
+			MessageSendEvent::BroadcastNodeAnnouncement { .. } => {},
+			_ => panic!("Unexpected event {:?}", event),
+		}
+	}
+	assert!(found_ann_1);
+
+	let a_node_announcement = match a_events.last().unwrap() {
 		MessageSendEvent::BroadcastNodeAnnouncement { ref msg } => {
 			(*msg).clone()
 		},
@@ -625,8 +639,22 @@ pub fn update_nodes_with_chan_announce<'a, 'b, 'c, 'd>(nodes: &'a Vec<Node<'b, '
 
 	nodes[b].node.broadcast_node_announcement([1, 1, 1], [1; 32], Vec::new());
 	let b_events = nodes[b].node.get_and_clear_pending_msg_events();
-	assert_eq!(b_events.len(), 1);
-	let b_node_announcement = match b_events[0] {
+	assert!(b_events.len() >= 2);
+
+	// ann should be re-generated by broadcast_node_announcement - check that we have it.
+	let mut found_ann_2 = false;
+	for event in b_events.iter() {
+		match event {
+			MessageSendEvent::BroadcastChannelAnnouncement { ref msg, .. } => {
+				if msg == ann { found_ann_2 = true; }
+			},
+			MessageSendEvent::BroadcastNodeAnnouncement { .. } => {},
+			_ => panic!("Unexpected event"),
+		}
+	}
+	assert!(found_ann_2);
+
+	let b_node_announcement = match b_events.last().unwrap() {
 		MessageSendEvent::BroadcastNodeAnnouncement { ref msg } => {
 			(*msg).clone()
 		},

lightning/src/ln/functional_tests.rs

@@ -3806,7 +3806,10 @@ fn test_funding_peer_disconnect() {
 	let chanmon_cfgs = create_chanmon_cfgs(2);
 	let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
 	let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
-	let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+	let persister: test_utils::TestPersister;
+	let new_chain_monitor: test_utils::TestChainMonitor;
+	let nodes_0_deserialized: ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>;
+	let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
 	let tx = create_chan_between_nodes_with_value_init(&nodes[0], &nodes[1], 100000, 10001, InitFeatures::known(), InitFeatures::known());
 
 	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
@@ -3884,6 +3887,61 @@ fn test_funding_peer_disconnect() {
 	let route = get_route(&nodes[0].node.get_our_node_id(), &net_graph_msg_handler.network_graph.read().unwrap(), &nodes[1].node.get_our_node_id(), Some(InvoiceFeatures::known()), None, &Vec::new(), 1000000, TEST_FINAL_CLTV, &logger).unwrap();
 	let (payment_preimage, _, _) = send_along_route(&nodes[0], route, &[&nodes[1]], 1000000);
 	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage);
+
+	// Check that after deserialization and reconnection we can still generate an identical
+	// channel_announcement from the cached signatures.
+	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
+
+	let nodes_0_serialized = nodes[0].node.encode();
+	let mut chan_0_monitor_serialized = test_utils::TestVecWriter(Vec::new());
+	nodes[0].chain_monitor.chain_monitor.monitors.read().unwrap().iter().next().unwrap().1.write(&mut chan_0_monitor_serialized).unwrap();
+
+	persister = test_utils::TestPersister::new();
+	let keys_manager = &chanmon_cfgs[0].keys_manager;
+	new_chain_monitor = test_utils::TestChainMonitor::new(Some(nodes[0].chain_source), nodes[0].tx_broadcaster.clone(), nodes[0].logger, node_cfgs[0].fee_estimator, &persister, keys_manager);
+	nodes[0].chain_monitor = &new_chain_monitor;
+	let mut chan_0_monitor_read = &chan_0_monitor_serialized.0[..];
+	let (_, mut chan_0_monitor) = <(BlockHash, ChannelMonitor<EnforcingSigner>)>::read(
+		&mut chan_0_monitor_read, keys_manager).unwrap();
+	assert!(chan_0_monitor_read.is_empty());
+
+	let mut nodes_0_read = &nodes_0_serialized[..];
+	let (_, nodes_0_deserialized_tmp) = {
+		let mut channel_monitors = HashMap::new();
+		channel_monitors.insert(chan_0_monitor.get_funding_txo().0, &mut chan_0_monitor);
+		<(BlockHash, ChannelManager<EnforcingSigner, &test_utils::TestChainMonitor, &test_utils::TestBroadcaster, &test_utils::TestKeysInterface, &test_utils::TestFeeEstimator, &test_utils::TestLogger>)>::read(&mut nodes_0_read, ChannelManagerReadArgs {
+			default_config: UserConfig::default(),
+			keys_manager,
+			fee_estimator: node_cfgs[0].fee_estimator,
+			chain_monitor: nodes[0].chain_monitor,
+			tx_broadcaster: nodes[0].tx_broadcaster.clone(),
+			logger: nodes[0].logger,
+			channel_monitors,
+		}).unwrap()
+	};
+	nodes_0_deserialized = nodes_0_deserialized_tmp;
+	assert!(nodes_0_read.is_empty());
+
+	assert!(nodes[0].chain_monitor.watch_channel(chan_0_monitor.get_funding_txo().0, chan_0_monitor).is_ok());
+	nodes[0].node = &nodes_0_deserialized;
+	check_added_monitors!(nodes[0], 1);
+
+	reconnect_nodes(&nodes[0], &nodes[1], (false, false), (0, 0), (0, 0), (0, 0), (0, 0), (false, false));
+
+	// as_announcement should be re-generated exactly by broadcast_node_announcement.
+	nodes[0].node.broadcast_node_announcement([0, 0, 0], [0; 32], Vec::new());
+	let msgs = nodes[0].node.get_and_clear_pending_msg_events();
+	let mut found_announcement = false;
+	for event in msgs.iter() {
+		match event {
+			MessageSendEvent::BroadcastChannelAnnouncement { ref msg, .. } => {
+				if *msg == as_announcement { found_announcement = true; }
+			},
+			MessageSendEvent::BroadcastNodeAnnouncement { .. } => {},
+			_ => panic!("Unexpected event"),
+		}
+	}
+	assert!(found_announcement);
 }
 
 #[test]