Swap PublicKey for NodeId in UnsignedNodeAnnouncement

Also swaps `PublicKey` for `NodeId` in `get_next_node_announcement`
and `InitSyncTracker` to avoid unnecessary deserialization that came
from changing `UnsignedNodeAnnouncement`.

Author: alecchendev

lightning-net-tokio/src/lib.rs

@@ -584,6 +584,7 @@ mod tests {
 	use lightning::ln::msgs::*;
 	use lightning::ln::peer_handler::{MessageHandler, PeerManager};
 	use lightning::ln::features::NodeFeatures;
+	use lightning::routing::gossip::NodeId;
 	use lightning::util::events::*;
 	use lightning::util::test_utils::TestNodeSigner;
 	use bitcoin::secp256k1::{Secp256k1, SecretKey, PublicKey};
@@ -614,7 +615,7 @@ mod tests {
 		fn handle_channel_announcement(&self, _msg: &ChannelAnnouncement) -> Result<bool, LightningError> { Ok(false) }
 		fn handle_channel_update(&self, _msg: &ChannelUpdate) -> Result<bool, LightningError> { Ok(false) }
 		fn get_next_channel_announcement(&self, _starting_point: u64) -> Option<(ChannelAnnouncement, Option<ChannelUpdate>, Option<ChannelUpdate>)> { None }
-		fn get_next_node_announcement(&self, _starting_point: Option<&PublicKey>) -> Option<NodeAnnouncement> { None }
+		fn get_next_node_announcement(&self, _starting_point: Option<&NodeId>) -> Option<NodeAnnouncement> { None }
 		fn peer_connected(&self, _their_node_id: &PublicKey, _init_msg: &Init) -> Result<(), ()> { Ok(()) }
 		fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: ReplyChannelRange) -> Result<(), LightningError> { Ok(()) }
 		fn handle_reply_short_channel_ids_end(&self, _their_node_id: &PublicKey, _msg: ReplyShortChannelIdsEnd) -> Result<(), LightningError> { Ok(()) }

lightning/src/ln/msgs.rs

@@ -665,7 +665,7 @@ pub struct UnsignedNodeAnnouncement {
 	pub timestamp: u32,
 	/// The `node_id` this announcement originated from (don't rebroadcast the `node_announcement` back
 	/// to this node).
-	pub node_id: PublicKey,
+	pub node_id: NodeId,
 	/// An RGB color for UI purposes
 	pub rgb: [u8; 3],
 	/// An alias, for UI purposes.
@@ -1057,7 +1057,7 @@ pub trait RoutingMessageHandler : MessageSendEventsProvider {
 	/// the node *after* the provided pubkey and including up to one announcement immediately
 	/// higher (as defined by `<PublicKey as Ord>::cmp`) than `starting_point`.
 	/// If `None` is provided for `starting_point`, we start at the first node.
-	fn get_next_node_announcement(&self, starting_point: Option<&PublicKey>) -> Option<NodeAnnouncement>;
+	fn get_next_node_announcement(&self, starting_point: Option<&NodeId>) -> Option<NodeAnnouncement>;
 	/// Called when a connection is established with a peer. This can be used to
 	/// perform routing table synchronization using a strategy defined by the
 	/// implementor.
@@ -1845,7 +1845,7 @@ impl Readable for UnsignedNodeAnnouncement {
 	fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
 		let features: NodeFeatures = Readable::read(r)?;
 		let timestamp: u32 = Readable::read(r)?;
-		let node_id: PublicKey = Readable::read(r)?;
+		let node_id: NodeId = Readable::read(r)?;
 		let mut rgb = [0; 3];
 		r.read_exact(&mut rgb)?;
 		let alias: [u8; 32] = Readable::read(r)?;
@@ -2248,7 +2248,7 @@ mod tests {
 		let unsigned_node_announcement = msgs::UnsignedNodeAnnouncement {
 			features,
 			timestamp: 20190119,
-			node_id: pubkey_1,
+			node_id: NodeId::from_pubkey(&pubkey_1),
 			rgb: [32; 3],
 			alias: [16;32],
 			addresses,

lightning/src/ln/peer_handler.rs

@@ -71,7 +71,7 @@ impl RoutingMessageHandler for IgnoringMessageHandler {
 	fn handle_channel_update(&self, _msg: &msgs::ChannelUpdate) -> Result<bool, LightningError> { Ok(false) }
 	fn get_next_channel_announcement(&self, _starting_point: u64) ->
 		Option<(msgs::ChannelAnnouncement, Option<msgs::ChannelUpdate>, Option<msgs::ChannelUpdate>)> { None }
-	fn get_next_node_announcement(&self, _starting_point: Option<&PublicKey>) -> Option<msgs::NodeAnnouncement> { None }
+	fn get_next_node_announcement(&self, _starting_point: Option<&NodeId>) -> Option<msgs::NodeAnnouncement> { None }
 	fn peer_connected(&self, _their_node_id: &PublicKey, _init: &msgs::Init) -> Result<(), ()> { Ok(()) }
 	fn handle_reply_channel_range(&self, _their_node_id: &PublicKey, _msg: msgs::ReplyChannelRange) -> Result<(), LightningError> { Ok(()) }
 	fn handle_reply_short_channel_ids_end(&self, _their_node_id: &PublicKey, _msg: msgs::ReplyShortChannelIdsEnd) -> Result<(), LightningError> { Ok(()) }
@@ -345,7 +345,7 @@ impl error::Error for PeerHandleError {
 enum InitSyncTracker{
 	NoSyncRequested,
 	ChannelsSyncing(u64),
-	NodesSyncing(PublicKey),
+	NodesSyncing(NodeId),
 }
 
 /// The ratio between buffer sizes at which we stop sending initial sync messages vs when we stop
@@ -434,15 +434,15 @@ impl Peer {
 	}
 
 	/// Similar to the above, but for node announcements indexed by node_id.
-	fn should_forward_node_announcement(&self, node_id: PublicKey) -> bool {
+	fn should_forward_node_announcement(&self, node_id: NodeId) -> bool {
 		if self.their_features.as_ref().unwrap().supports_gossip_queries() &&
 			!self.sent_gossip_timestamp_filter {
 				return false;
 			}
 		match self.sync_status {
 			InitSyncTracker::NoSyncRequested => true,
 			InitSyncTracker::ChannelsSyncing(_) => false,
-			InitSyncTracker::NodesSyncing(pk) => pk < node_id,
+			InitSyncTracker::NodesSyncing(sync_node_id) => sync_node_id.as_slice() < node_id.as_slice(),
 		}
 	}
 
@@ -889,8 +889,8 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 						}
 					},
 					InitSyncTracker::ChannelsSyncing(_) => unreachable!(),
-					InitSyncTracker::NodesSyncing(key) => {
-						if let Some(msg) = self.message_handler.route_handler.get_next_node_announcement(Some(&key)) {
+					InitSyncTracker::NodesSyncing(sync_node_id) => {
+						if let Some(msg) = self.message_handler.route_handler.get_next_node_announcement(Some(&sync_node_id)) {
 							self.enqueue_message(peer, &msg);
 							peer.sync_status = InitSyncTracker::NodesSyncing(msg.contents.node_id);
 						} else {
@@ -1493,8 +1493,10 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 						log_gossip!(self.logger, "Skipping broadcast message to {:?} as its outbound buffer is full", peer.their_node_id);
 						continue;
 					}
-					if peer.their_node_id.as_ref() == Some(&msg.contents.node_id) {
-						continue;
+					if let Some(their_node_id) = peer.their_node_id {
+						if NodeId::from_pubkey(&their_node_id) == msg.contents.node_id {
+							continue;
+						}
 					}
 					if except_node.is_some() && peer.their_node_id.as_ref() == except_node {
 						continue;
@@ -2023,7 +2025,7 @@ impl<Descriptor: SocketDescriptor, CM: Deref, RM: Deref, OM: Deref, L: Deref, CM
 		let announcement = msgs::UnsignedNodeAnnouncement {
 			features,
 			timestamp: self.last_node_announcement_serial.fetch_add(1, Ordering::AcqRel),
-			node_id: self.node_signer.get_node_id(Recipient::Node).unwrap(),
+			node_id: NodeId::from_pubkey(&self.node_signer.get_node_id(Recipient::Node).unwrap()),
 			rgb, alias, addresses,
 			excess_address_data: Vec::new(),
 			excess_data: Vec::new(),

lightning/src/routing/gossip.rs

@@ -385,10 +385,10 @@ where C::Target: chain::Access, L::Target: Logger
 		None
 	}
 
-	fn get_next_node_announcement(&self, starting_point: Option<&PublicKey>) -> Option<NodeAnnouncement> {
+	fn get_next_node_announcement(&self, starting_point: Option<&NodeId>) -> Option<NodeAnnouncement> {
 		let nodes = self.network_graph.nodes.read().unwrap();
-		let iter = if let Some(pubkey) = starting_point {
-				nodes.range((Bound::Excluded(NodeId::from_pubkey(pubkey)), Bound::Unbounded))
+		let iter = if let Some(node_id) = starting_point {
+				nodes.range((Bound::Excluded(node_id), Bound::Unbounded))
 			} else {
 				nodes.range(..)
 			};
@@ -1286,7 +1286,7 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 	/// routing messages from a source using a protocol other than the lightning P2P protocol.
 	pub fn update_node_from_announcement(&self, msg: &msgs::NodeAnnouncement) -> Result<(), LightningError> {
 		let msg_hash = hash_to_message!(&Sha256dHash::hash(&msg.contents.encode()[..])[..]);
-		secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.signature, &msg.contents.node_id, "node_announcement");
+		secp_verify_sig!(self.secp_ctx, &msg_hash, &msg.signature, &get_pubkey_from_node_id!(msg.contents.node_id, "node_announcement"), "node_announcement");
 		self.update_node_from_announcement_intern(&msg.contents, Some(&msg))
 	}
 
@@ -1299,7 +1299,7 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 	}
 
 	fn update_node_from_announcement_intern(&self, msg: &msgs::UnsignedNodeAnnouncement, full_msg: Option<&msgs::NodeAnnouncement>) -> Result<(), LightningError> {
-		match self.nodes.write().unwrap().get_mut(&NodeId::from_pubkey(&msg.node_id)) {
+		match self.nodes.write().unwrap().get_mut(&msg.node_id) {
 			None => Err(LightningError{err: "No existing channels for node_announcement".to_owned(), action: ErrorAction::IgnoreError}),
 			Some(node) => {
 				if let Some(node_info) = node.announcement_info.as_ref() {
@@ -1971,11 +1971,11 @@ mod tests {
 	}
 
 	fn get_signed_node_announcement<F: Fn(&mut UnsignedNodeAnnouncement)>(f: F, node_key: &SecretKey, secp_ctx: &Secp256k1<secp256k1::All>) -> NodeAnnouncement {
-		let node_id = PublicKey::from_secret_key(&secp_ctx, node_key);
+		let node_id = NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, node_key));
 		let mut unsigned_announcement = UnsignedNodeAnnouncement {
 			features: channelmanager::provided_node_features(&UserConfig::default()),
 			timestamp: 100,
-			node_id: node_id,
+			node_id,
 			rgb: [0; 3],
 			alias: [0; 32],
 			addresses: Vec::new(),
@@ -2652,7 +2652,7 @@ mod tests {
 		let (secp_ctx, gossip_sync) = create_gossip_sync(&network_graph);
 		let node_1_privkey = &SecretKey::from_slice(&[42; 32]).unwrap();
 		let node_2_privkey = &SecretKey::from_slice(&[41; 32]).unwrap();
-		let node_id_1 = PublicKey::from_secret_key(&secp_ctx, node_1_privkey);
+		let node_id_1 = NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, node_1_privkey));
 
 		// No nodes yet.
 		let next_announcements = gossip_sync.get_next_node_announcement(None);

lightning/src/routing/test_utils.rs

@@ -66,7 +66,7 @@ pub(super) fn add_or_update_node(
 	gossip_sync: &P2PGossipSync<Arc<NetworkGraph<Arc<test_utils::TestLogger>>>, Arc<test_utils::TestChainSource>, Arc<test_utils::TestLogger>>,
 	secp_ctx: &Secp256k1<All>, node_privkey: &SecretKey, features: NodeFeatures, timestamp: u32
 ) {
-	let node_id = PublicKey::from_secret_key(&secp_ctx, node_privkey);
+	let node_id = NodeId::from_pubkey(&PublicKey::from_secret_key(&secp_ctx, node_privkey));
 	let unsigned_announcement = UnsignedNodeAnnouncement {
 		features,
 		timestamp,

lightning/src/util/test_utils.rs

@@ -511,7 +511,7 @@ impl msgs::RoutingMessageHandler for TestRoutingMessageHandler {
 		Some((chan_ann, Some(chan_upd_1), Some(chan_upd_2)))
 	}
 
-	fn get_next_node_announcement(&self, _starting_point: Option<&PublicKey>) -> Option<msgs::NodeAnnouncement> {
+	fn get_next_node_announcement(&self, _starting_point: Option<&NodeId>) -> Option<msgs::NodeAnnouncement> {
 		None
 	}