Replace PublicKey with [u8; 33] in NetworkGraph

Author: dunxen

lightning/src/routing/network_graph.rs

@@ -50,12 +50,15 @@ const MAX_EXCESS_BYTES_FOR_RELAY: usize = 1024;
 /// This value ensures a reply fits within the 65k payload limit and is consistent with other implementations.
 const MAX_SCIDS_PER_REPLY: usize = 8000;
 
+/// Represents the compressed public key of a node
+pub type NodeId = [u8; 33];
+
 /// Represents the network as nodes and channels between them
 pub struct NetworkGraph {
 	genesis_hash: BlockHash,
 	// Lock order: channels -> nodes
 	channels: RwLock<BTreeMap<u64, ChannelInfo>>,
-	nodes: RwLock<BTreeMap<PublicKey, NodeInfo>>,
+	nodes: RwLock<BTreeMap<NodeId, NodeInfo>>,
 }
 
 impl Clone for NetworkGraph {
@@ -73,7 +76,7 @@ impl Clone for NetworkGraph {
 /// A read-only view of [`NetworkGraph`].
 pub struct ReadOnlyNetworkGraph<'a> {
 	channels: RwLockReadGuard<'a, BTreeMap<u64, ChannelInfo>>,
-	nodes: RwLockReadGuard<'a, BTreeMap<PublicKey, NodeInfo>>,
+	nodes: RwLockReadGuard<'a, BTreeMap<NodeId, NodeInfo>>,
 }
 
 /// Update to the [`NetworkGraph`] based on payment failure information conveyed via the Onion
@@ -209,7 +212,7 @@ where C::Target: chain::Access, L::Target: Logger
 			NetworkUpdate::NodeFailure { ref node_id, is_permanent } => {
 				let action = if is_permanent { "Removing" } else { "Disabling" };
 				log_debug!(self.logger, "{} node graph entry for {} due to a payment failure.", action, node_id);
-				self.network_graph.fail_node(node_id, is_permanent);
+				self.network_graph.fail_node(&node_id.serialize(), is_permanent);
 			},
 		}
 	}
@@ -277,11 +280,11 @@ where C::Target: chain::Access, L::Target: Logger
 		let mut result = Vec::with_capacity(batch_amount as usize);
 		let nodes = self.network_graph.nodes.read().unwrap();
 		let mut iter = if let Some(pubkey) = starting_point {
-				let mut iter = nodes.range((*pubkey)..);
+				let mut iter = nodes.range((pubkey.serialize())..);
 				iter.next();
 				iter
 			} else {
-				nodes.range(..)
+				nodes.range::<NodeId, _>(..)
 			};
 		while result.len() < batch_amount as usize {
 			if let Some((_, ref node)) = iter.next() {
@@ -314,7 +317,7 @@ where C::Target: chain::Access, L::Target: Logger
 		}
 
 		// Check if we need to perform a full synchronization with this peer
-		if !self.should_request_full_sync(their_node_id) {
+		if !self.should_request_full_sync(&their_node_id) {
 			return ();
 		}
 
@@ -551,11 +554,11 @@ pub struct ChannelInfo {
 	/// Protocol features of a channel communicated during its announcement
 	pub features: ChannelFeatures,
 	/// Source node of the first direction of a channel
-	pub node_one: PublicKey,
+	pub node_one: NodeId,
 	/// Details about the first direction of a channel
 	pub one_to_two: Option<DirectionalChannelInfo>,
 	/// Source node of the second direction of a channel
-	pub node_two: PublicKey,
+	pub node_two: NodeId,
 	/// Details about the second direction of a channel
 	pub two_to_one: Option<DirectionalChannelInfo>,
 	/// The channel capacity as seen on-chain, if chain lookup is available.
@@ -570,7 +573,7 @@ pub struct ChannelInfo {
 impl fmt::Display for ChannelInfo {
 	fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
 		write!(f, "features: {}, node_one: {}, one_to_two: {:?}, node_two: {}, two_to_one: {:?}",
-		   log_bytes!(self.features.encode()), log_pubkey!(self.node_one), self.one_to_two, log_pubkey!(self.node_two), self.two_to_one)?;
+		   log_bytes!(self.features.encode()), log_bytes!(self.node_one), self.one_to_two, log_bytes!(self.node_two), self.two_to_one)?;
 		Ok(())
 	}
 }
@@ -725,7 +728,7 @@ impl fmt::Display for NetworkGraph {
 		}
 		writeln!(f, "[Nodes]")?;
 		for (key, val) in self.nodes.read().unwrap().iter() {
-			writeln!(f, " {}: {}", log_pubkey!(key), val)?;
+			writeln!(f, " {}: {}", log_bytes!(*key), val)?;
 		}
 		Ok(())
 	}
@@ -780,7 +783,7 @@ impl NetworkGraph {
 	}
 
 	fn update_node_from_announcement_intern(&self, msg: &msgs::UnsignedNodeAnnouncement, full_msg: Option<&msgs::NodeAnnouncement>) -> Result<(), LightningError> {
-		match self.nodes.write().unwrap().get_mut(&msg.node_id) {
+		match self.nodes.write().unwrap().get_mut(&msg.node_id.serialize()) {
 			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() {
@@ -886,9 +889,9 @@ impl NetworkGraph {
 
 		let chan_info = ChannelInfo {
 				features: msg.features.clone(),
-				node_one: msg.node_id_1.clone(),
+				node_one: msg.node_id_1.serialize(),
 				one_to_two: None,
-				node_two: msg.node_id_2.clone(),
+				node_two: msg.node_id_2.serialize(),
 				two_to_one: None,
 				capacity_sats: utxo_value,
 				announcement_message: if msg.excess_data.len() <= MAX_EXCESS_BYTES_FOR_RELAY
@@ -939,8 +942,8 @@ impl NetworkGraph {
 			};
 		}
 
-		add_channel_to_node!(msg.node_id_1);
-		add_channel_to_node!(msg.node_id_2);
+		add_channel_to_node!(msg.node_id_1.serialize());
+		add_channel_to_node!(msg.node_id_2.serialize());
 
 		Ok(())
 	}
@@ -969,7 +972,7 @@ impl NetworkGraph {
 	}
 
 	/// Marks a node in the graph as failed.
-	pub fn fail_node(&self, _node_id: &PublicKey, is_permanent: bool) {
+	pub fn fail_node(&self, _node_id: &NodeId, is_permanent: bool) {
 		if is_permanent {
 			// TODO: Wholly remove the node
 		} else {
@@ -1050,13 +1053,19 @@ impl NetworkGraph {
 				if msg.flags & 1 == 1 {
 					dest_node_id = channel.node_one.clone();
 					if let Some((sig, ctx)) = sig_info {
-						secp_verify_sig!(ctx, &msg_hash, &sig, &channel.node_two);
+						secp_verify_sig!(ctx, &msg_hash, &sig, &PublicKey::from_slice(&channel.node_two).map_err(|_| LightningError{
+							err: "Couldn't parse source node pubkey".to_owned(),
+							action: ErrorAction::IgnoreAndLog(Level::Debug)
+						})?);
 					}
 					maybe_update_channel_info!(channel.two_to_one, channel.node_two);
 				} else {
 					dest_node_id = channel.node_two.clone();
 					if let Some((sig, ctx)) = sig_info {
-						secp_verify_sig!(ctx, &msg_hash, &sig, &channel.node_one);
+						secp_verify_sig!(ctx, &msg_hash, &sig, &PublicKey::from_slice(&channel.node_one).map_err(|_| LightningError{
+							err: "Couldn't parse destination node pubkey".to_owned(),
+							action: ErrorAction::IgnoreAndLog(Level::Debug)
+						})?);
 					}
 					maybe_update_channel_info!(channel.one_to_two, channel.node_one);
 				}
@@ -1104,7 +1113,7 @@ impl NetworkGraph {
 		Ok(())
 	}
 
-	fn remove_channel_in_nodes(nodes: &mut BTreeMap<PublicKey, NodeInfo>, chan: &ChannelInfo, short_channel_id: u64) {
+	fn remove_channel_in_nodes(nodes: &mut BTreeMap<NodeId, NodeInfo>, chan: &ChannelInfo, short_channel_id: u64) {
 		macro_rules! remove_from_node {
 			($node_id: expr) => {
 				if let BtreeEntry::Occupied(mut entry) = nodes.entry($node_id) {
@@ -1136,7 +1145,7 @@ impl ReadOnlyNetworkGraph<'_> {
 	/// Returns all known nodes' public keys along with announced node info.
 	///
 	/// (C-not exported) because we have no mapping for `BTreeMap`s
-	pub fn nodes(&self) -> &BTreeMap<PublicKey, NodeInfo> {
+	pub fn nodes(&self) -> &BTreeMap<NodeId, NodeInfo> {
 		&*self.nodes
 	}
 
@@ -1146,7 +1155,7 @@ impl ReadOnlyNetworkGraph<'_> {
 	///
 	/// (C-not exported) as there is no practical way to track lifetimes of returned values.
 	pub fn get_addresses(&self, pubkey: &PublicKey) -> Option<&Vec<NetAddress>> {
-		if let Some(node) = self.nodes.get(pubkey) {
+		if let Some(node) = self.nodes.get(&pubkey.serialize()) {
 			if let Some(node_info) = node.announcement_info.as_ref() {
 				return Some(&node_info.addresses)
 			}