From 8791d7c786613b968820b11eeae7ffcadbaf8362 Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Tue, 6 Aug 2024 13:10:44 +0100 Subject: [PATCH] fix: correct node announcement, simplify setting alias, clean alias sanitization - Skips broadcasting node announcement in the event that either the node alias or the listening addresses are not set. - Aligns the InvalidNodeAlias error variant with the others to make it work with language bindings. - Simplifies the method to set the node alias. - Cleans up the alias sanitizing function to ensure that protocol- compliant aliases (in this case, empty strings) are not flagged. Additionally, removes the check for sandwiched null byte. - Finally, adds the relevant update to struct and interface to reflect changes in Rust types. --- bindings/ldk_node.udl | 4 ++++ src/builder.rs | 52 ++++++++++++++++--------------------------- src/lib.rs | 20 +++++++---------- 3 files changed, 31 insertions(+), 45 deletions(-) diff --git a/bindings/ldk_node.udl b/bindings/ldk_node.udl index aedf9f6ab..779e13bed 100644 --- a/bindings/ldk_node.udl +++ b/bindings/ldk_node.udl @@ -16,6 +16,7 @@ dictionary Config { u64 probing_liquidity_limit_multiplier; LogLevel log_level; AnchorChannelsConfig? anchor_channels_config; + string? node_alias; }; dictionary AnchorChannelsConfig { @@ -40,6 +41,8 @@ interface Builder { [Throws=BuildError] void set_listening_addresses(sequence listening_addresses); [Throws=BuildError] + void set_node_alias(string node_alias); + [Throws=BuildError] Node build(); [Throws=BuildError] Node build_with_fs_store(); @@ -237,6 +240,7 @@ enum BuildError { "KVStoreSetupFailed", "WalletSetupFailed", "LoggerSetupFailed", + "InvalidNodeAlias" }; [Enum] diff --git a/src/builder.rs b/src/builder.rs index 834bf7b1d..18afc1fed 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -114,6 +114,8 @@ pub enum BuildError { InvalidChannelMonitor, /// The given listening addresses are invalid, e.g. too many were passed. InvalidListeningAddresses, + /// The provided alias is invalid + InvalidNodeAlias, /// We failed to read data from the [`KVStore`]. /// /// [`KVStore`]: lightning::util::persist::KVStore @@ -132,8 +134,6 @@ pub enum BuildError { WalletSetupFailed, /// We failed to setup the logger. LoggerSetupFailed, - /// The provided alias is invalid - InvalidNodeAlias(String), } impl fmt::Display for BuildError { @@ -154,9 +154,7 @@ impl fmt::Display for BuildError { Self::KVStoreSetupFailed => write!(f, "Failed to setup KVStore."), Self::WalletSetupFailed => write!(f, "Failed to setup onchain wallet."), Self::LoggerSetupFailed => write!(f, "Failed to setup the logger."), - Self::InvalidNodeAlias(ref reason) => { - write!(f, "Given node alias is invalid: {}", reason) - }, + Self::InvalidNodeAlias => write!(f, "Given node alias is invalid."), } } } @@ -309,9 +307,7 @@ impl NodeBuilder { /// Sets the alias the [`Node`] will use in its announcement. The provided /// alias must be a valid UTF-8 string. - pub fn set_node_alias>( - &mut self, node_alias: T, - ) -> Result<&mut Self, BuildError> { + pub fn set_node_alias(&mut self, node_alias: String) -> Result<&mut Self, BuildError> { let node_alias = sanitize_alias(node_alias).map_err(|e| e)?; self.config.node_alias = Some(node_alias); @@ -515,6 +511,11 @@ impl ArcedNodeBuilder { self.inner.write().unwrap().set_log_level(level); } + /// Sets the node alias. + pub fn set_node_alias(&self, node_alias: String) -> Result<(), BuildError> { + self.inner.write().unwrap().set_node_alias(node_alias).map(|_| ()) + } + /// Builds a [`Node`] instance with a [`SqliteStore`] backend and according to the options /// previously configured. pub fn build(&self) -> Result, BuildError> { @@ -1066,21 +1067,12 @@ fn sanitize_alias>(node_alias: T) -> Result let node_alias: String = node_alias.into(); let alias = node_alias.trim(); - // Alias is non-empty - if alias.is_empty() { - return Err(BuildError::InvalidNodeAlias("Node alias cannot be empty.".to_string())); - } - - // Alias valid up to first null byte - let first_null = alias.as_bytes().iter().position(|b| *b == 0).unwrap_or(alias.len()); - let actual_alias = alias.split_at(first_null).0; - // Alias must be 32-bytes long or less - if actual_alias.as_bytes().len() > 32 { - return Err(BuildError::InvalidNodeAlias("Node alias cannot exceed 32 bytes.".to_string())); + if alias.as_bytes().len() > 32 { + return Err(BuildError::InvalidNodeAlias); } - Ok(actual_alias.to_string()) + Ok(alias.to_string()) } #[cfg(test)] @@ -1089,19 +1081,16 @@ mod tests { use super::NodeBuilder; - fn create_node_with_alias>(alias: T) -> Result { - NodeBuilder::new().set_node_alias(&alias.into())?.build() + fn create_node_with_alias(alias: String) -> Result { + NodeBuilder::new().set_node_alias(alias)?.build() } #[test] fn empty_node_alias() { // Empty node alias let alias = ""; - let node = create_node_with_alias(alias); - assert_eq!( - node.err().unwrap(), - BuildError::InvalidNodeAlias("Node alias cannot be empty.".to_string()) - ); + let node = create_node_with_alias(alias.to_string()); + assert_eq!(node.err().unwrap(), BuildError::InvalidNodeAlias); } #[test] @@ -1109,7 +1098,7 @@ mod tests { // Alias with emojis let expected_alias = "I\u{1F496}LDK-Node!"; let user_provided_alias = "I\u{1F496}LDK-Node!\0\u{26A1}"; - let node = create_node_with_alias(user_provided_alias).unwrap(); + let node = create_node_with_alias(user_provided_alias.to_string()).unwrap(); assert_eq!(expected_alias, node.config().node_alias.unwrap()); } @@ -1117,10 +1106,7 @@ mod tests { #[test] fn node_alias_longer_than_32_bytes() { let alias = "This is a string longer than thirty-two bytes!"; // 46 bytes - let node = create_node_with_alias(alias); - assert_eq!( - node.err().unwrap(), - BuildError::InvalidNodeAlias("Node alias cannot exceed 32 bytes.".to_string()) - ); + let node = create_node_with_alias(alias.to_string()); + assert_eq!(node.err().unwrap(), BuildError::InvalidNodeAlias); } } diff --git a/src/lib.rs b/src/lib.rs index 1e4945a49..30280d39b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -647,22 +647,18 @@ impl Node { } let addresses = bcast_config.listening_addresses.clone().unwrap_or(Vec::new()); - - if addresses.is_empty() { - // Skip if we are not listening on any addresses. - continue; - } - - // Extract alias if set, else select the default - let alias = if let Some(ref alias) = node_alias { + let alias = node_alias.clone().map(|alias| { let mut buf = [0_u8; 32]; buf[..alias.as_bytes().len()].copy_from_slice(alias.as_bytes()); buf - } else { - [0; 32] - }; + }); + + if addresses.is_empty() || alias.is_none() { + // Skip if we are not listening on any addresses or if the node alias is not set. + continue; + } - bcast_pm.broadcast_node_announcement([0; 3], alias, addresses); + bcast_pm.broadcast_node_announcement([0; 3], alias.unwrap(), addresses); let unix_time_secs_opt = SystemTime::now().duration_since(UNIX_EPOCH).ok().map(|d| d.as_secs());