f nodeannouncementinfo refactor

Author: arik-so

lightning/src/onion_message/messenger.rs

@@ -386,8 +386,8 @@ where
 			let node_announcement = network_graph
 				.node(&NodeId::from_pubkey(&first_node))
 				.and_then(|node_info| node_info.announcement_info.as_ref())
-				.and_then(|announcement_info| announcement_info.announcement_message.as_ref())
-				.map(|node_announcement| &node_announcement.contents);
+				.and_then(|announcement_info| announcement_info.announcement_message().cloned())
+				.map(|node_announcement| node_announcement.contents);
 
 			match node_announcement {
 				Some(node_announcement) if node_announcement.features.supports_onion_messages() => {

lightning/src/routing/gossip.rs

@@ -504,8 +504,8 @@ where U::Target: UtxoLookup, L::Target: Logger
 			};
 		for (_, ref node) in iter {
 			if let Some(node_info) = node.announcement_info.as_ref() {
-				if let Some(msg) = node_info.announcement_message.clone() {
-					return Some(msg);
+				if let NodeAnnouncementInfo::Relayed(announcement) = node_info {
+					return Some(announcement.clone());
 				}
 			}
 		}
@@ -1130,71 +1130,162 @@ impl_writeable_tlv_based!(RoutingFees, {
 });
 
 #[derive(Clone, Debug, PartialEq, Eq)]
-/// Information received in the latest node_announcement from this node.
-pub struct NodeAnnouncementInfo {
+/// Non-relayable information received in the latest node_announcement from this node.
+pub struct NodeAnnouncementDetails {
 	/// Protocol features the node announced support for
-	pub features: NodeFeatures,
+	features: NodeFeatures,
+
 	/// When the last known update to the node state was issued.
 	/// Value is opaque, as set in the announcement.
-	pub last_update: u32,
+	last_update: u32,
+
 	/// Color assigned to the node
-	pub rgb: [u8; 3],
+	rgb: [u8; 3],
+
 	/// Moniker assigned to the node.
 	/// May be invalid or malicious (eg control chars),
 	/// should not be exposed to the user.
-	pub alias: NodeAlias,
+	alias: NodeAlias,
 
 	/// Internet-level addresses via which one can connect to the node
-	pub addresses: Vec<SocketAddress>,
+	addresses: Vec<SocketAddress>,
+}
 
+#[derive(Clone, Debug, PartialEq, Eq)]
+/// Information received in the latest node_announcement from this node.
+pub enum NodeAnnouncementInfo {
 	/// An initial announcement of the node
-	/// Mostly redundant with the data we store in fields explicitly.
 	/// Everything else is useful only for sending out for initial routing sync.
 	/// Not stored if contains excess data to prevent DoS.
-	pub announcement_message: Option<NodeAnnouncement>,
+	Relayed(NodeAnnouncement),
+
+	/// Non-relayable information received in the latest node_announcement from this node.
+	Local(NodeAnnouncementDetails),
 }
 
 impl NodeAnnouncementInfo {
+
+	/// Protocol features the node announced support for
+	pub fn features(&self) -> &NodeFeatures {
+		match self {
+			NodeAnnouncementInfo::Relayed(relayed) => {
+				&relayed.contents.features
+			}
+			NodeAnnouncementInfo::Local(local) => {
+				&local.features
+			}
+		}
+	}
+
+	/// When the last known update to the node state was issued.
+	/// Value is opaque, as set in the announcement.
+	pub fn last_update(&self) -> u32 {
+		match self {
+			NodeAnnouncementInfo::Relayed(relayed) => {
+				relayed.contents.timestamp
+			}
+			NodeAnnouncementInfo::Local(local) => {
+				local.last_update
+			}
+		}
+	}
+
+	/// Color assigned to the node
+	pub fn rgb(&self) -> [u8; 3] {
+		match self {
+			NodeAnnouncementInfo::Relayed(relayed) => {
+				relayed.contents.rgb
+			}
+			NodeAnnouncementInfo::Local(local) => {
+				local.rgb
+			}
+		}
+	}
+
+	/// Moniker assigned to the node.
+	/// May be invalid or malicious (eg control chars),
+	/// should not be exposed to the user.
+	pub fn alias(&self) -> &NodeAlias {
+		match self {
+			NodeAnnouncementInfo::Relayed(relayed) => {
+				&relayed.contents.alias
+			}
+			NodeAnnouncementInfo::Local(local) => {
+				&local.alias
+			}
+		}
+	}
+
 	/// Internet-level addresses via which one can connect to the node
 	pub fn addresses(&self) -> &[SocketAddress] {
-		self.announcement_message.as_ref()
-			.map(|msg| msg.contents.addresses.as_slice())
-			.unwrap_or(self.addresses.as_slice())
+		match self {
+			NodeAnnouncementInfo::Relayed(relayed) => {
+				relayed.contents.addresses.as_slice()
+			}
+			NodeAnnouncementInfo::Local(local) => {
+				local.addresses.as_slice()
+			}
+		}
+	}
+
+	/// An initial announcement of the node
+	/// Not stored if contains excess data to prevent DoS.
+	pub fn announcement_message(&self) -> Option<&NodeAnnouncement> {
+		match self {
+			NodeAnnouncementInfo::Relayed(announcement) => {
+				Some(announcement)
+			}
+			NodeAnnouncementInfo::Local(_) => {
+				None
+			}
+		}
 	}
 }
 
 impl Writeable for NodeAnnouncementInfo {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
-		write_tlv_fields!(writer, {
-			(0, self.features, required),
-			(2, self.last_update, required),
-			(4, self.rgb, required),
-			(6, self.alias, required),
-			(8, self.announcement_message, option),
-			(10, self.addresses, required_vec), // Versions prior to 0.0.115 require this field
-		});
+		match self {
+			NodeAnnouncementInfo::Relayed(announcement) => {
+				write_tlv_fields!(writer, {
+					(8, announcement, required),
+				});
+			}
+			NodeAnnouncementInfo::Local(local) => {
+				write_tlv_fields!(writer, {
+					(0, local.features, required),
+					(2, local.last_update, required),
+					(4, local.rgb, required),
+					(6, local.alias, required),
+					(10, local.addresses, required_vec), // Versions prior to 0.0.115 require this field
+				});
+			}
+		}
+
 		Ok(())
 	}
 }
 
 impl Readable for NodeAnnouncementInfo {
 	fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
 		_init_and_read_len_prefixed_tlv_fields!(reader, {
-			(0, features, required),
-			(2, last_update, required),
-			(4, rgb, required),
-			(6, alias, required),
+			(0, features, option),
+			(2, last_update, option),
+			(4, rgb, option),
+			(6, alias, option),
 			(8, announcement_message, option),
 			(10, addresses, optional_vec),
 		});
-		Ok(Self {
-			features: features.0.unwrap(),
-			last_update: last_update.0.unwrap(),
-			rgb: rgb.0.unwrap(),
-			alias: alias.0.unwrap(),
-			addresses: addresses.unwrap_or(Vec::new()),
-			announcement_message,
-		})
+		if let Some(announcement) = announcement_message {
+			Ok(Self::Relayed(announcement))
+		} else {
+			Ok(Self::Local(NodeAnnouncementDetails {
+				features: features.unwrap(),
+				last_update: last_update.unwrap(),
+				rgb: rgb.unwrap(),
+				alias: alias.unwrap(),
+				addresses: addresses.unwrap_or(Vec::new()),
+			}))
+		}
 	}
 }
 
@@ -1496,9 +1587,9 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 					// The timestamp field is somewhat of a misnomer - the BOLTs use it to order
 					// updates to ensure you always have the latest one, only vaguely suggesting
 					// that it be at least the current time.
-					if node_info.last_update  > msg.timestamp {
+					if node_info.last_update()  > msg.timestamp {
 						return Err(LightningError{err: "Update older than last processed update".to_owned(), action: ErrorAction::IgnoreDuplicateGossip});
-					} else if node_info.last_update  == msg.timestamp {
+					} else if node_info.last_update()  == msg.timestamp {
 						return Err(LightningError{err: "Update had the same timestamp as last processed update".to_owned(), action: ErrorAction::IgnoreDuplicateGossip});
 					}
 				}
@@ -1507,14 +1598,18 @@ impl<L: Deref> NetworkGraph<L> where L::Target: Logger {
 					msg.excess_data.len() <= MAX_EXCESS_BYTES_FOR_RELAY &&
 					msg.excess_address_data.len() <= MAX_EXCESS_BYTES_FOR_RELAY &&
 					msg.excess_data.len() + msg.excess_address_data.len() <= MAX_EXCESS_BYTES_FOR_RELAY;
-				node.announcement_info = Some(NodeAnnouncementInfo {
-					features: msg.features.clone(),
-					last_update: msg.timestamp,
-					rgb: msg.rgb,
-					alias: msg.alias,
-					addresses: msg.addresses.clone(),
-					announcement_message: if should_relay { full_msg.cloned() } else { None },
-				});
+
+				node.announcement_info = if should_relay {
+					Some(NodeAnnouncementInfo::Relayed(full_msg.unwrap().clone()))
+				} else {
+					Some(NodeAnnouncementInfo::Local(NodeAnnouncementDetails {
+						features: msg.features.clone(),
+						last_update: msg.timestamp,
+						rgb: msg.rgb,
+						alias: msg.alias,
+						addresses: msg.addresses.clone(),
+					}))
+				};
 
 				Ok(())
 			}
@@ -3457,14 +3552,7 @@ pub(crate) mod tests {
 		// 1. Check we can read a valid NodeAnnouncementInfo and fail on an invalid one
 		let announcement_message = <Vec<u8>>::from_hex("d977cb9b53d93a6ff64bb5f1e158b4094b66e798fb12911168a3ccdf80a83096340a6a95da0ae8d9f776528eecdbb747eb6b545495a4319ed5378e35b21e073a000122013413a7031b84c5567b126440995d3ed5aaba0565d71e1834604819ff9c17f5e9d5dd078f2020201010101010101010101010101010101010101010101010101010101010101010000701fffefdfc2607").unwrap();
 		let announcement_message = NodeAnnouncement::read(&mut announcement_message.as_slice()).unwrap();
-		let valid_node_ann_info = NodeAnnouncementInfo {
-			features: channelmanager::provided_node_features(&UserConfig::default()),
-			last_update: 0,
-			rgb: [0u8; 3],
-			alias: NodeAlias([0u8; 32]),
-			addresses: announcement_message.contents.addresses.clone(),
-			announcement_message: Some(announcement_message)
-		};
+		let valid_node_ann_info = NodeAnnouncementInfo::Relayed(announcement_message);
 
 		let mut encoded_valid_node_ann_info = Vec::new();
 		assert!(valid_node_ann_info.write(&mut encoded_valid_node_ann_info).is_ok());

lightning/src/routing/router.rs

@@ -1984,7 +1984,7 @@ where L::Target: Logger {
 		true
 	} else if let Some(payee) = payee_node_id_opt {
 		network_nodes.get(&payee).map_or(false, |node| node.announcement_info.as_ref().map_or(false,
-			|info| info.features.supports_basic_mpp()))
+			|info| info.features().supports_basic_mpp()))
 	} else { false };
 
 	let max_total_routing_fee_msat = route_params.max_total_routing_fee_msat.unwrap_or(u64::max_value());
@@ -2469,7 +2469,7 @@ where L::Target: Logger {
 				}
 
 				let features = if let Some(node_info) = $node.announcement_info.as_ref() {
-					&node_info.features
+					&node_info.features()
 				} else {
 					&default_node_features
 				};
@@ -2799,7 +2799,7 @@ where L::Target: Logger {
 					if !features_set {
 						if let Some(node) = network_nodes.get(&target) {
 							if let Some(node_info) = node.announcement_info.as_ref() {
-								ordered_hops.last_mut().unwrap().1 = node_info.features.clone();
+								ordered_hops.last_mut().unwrap().1 = node_info.features().clone();
 							} else {
 								ordered_hops.last_mut().unwrap().1 = default_node_features.clone();
 							}