Skip to content

Commit

Permalink
fix: correct node announcement, simplify setting alias, clean
Browse files Browse the repository at this point in the history
     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.
  • Loading branch information
enigbe committed Aug 6, 2024
1 parent 202154d commit 8791d7c
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 45 deletions.
4 changes: 4 additions & 0 deletions bindings/ldk_node.udl
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ dictionary Config {
u64 probing_liquidity_limit_multiplier;
LogLevel log_level;
AnchorChannelsConfig? anchor_channels_config;
string? node_alias;
};

dictionary AnchorChannelsConfig {
Expand All @@ -40,6 +41,8 @@ interface Builder {
[Throws=BuildError]
void set_listening_addresses(sequence<SocketAddress> listening_addresses);
[Throws=BuildError]
void set_node_alias(string node_alias);
[Throws=BuildError]
Node build();
[Throws=BuildError]
Node build_with_fs_store();
Expand Down Expand Up @@ -237,6 +240,7 @@ enum BuildError {
"KVStoreSetupFailed",
"WalletSetupFailed",
"LoggerSetupFailed",
"InvalidNodeAlias"
};

[Enum]
Expand Down
52 changes: 19 additions & 33 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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."),
}
}
}
Expand Down Expand Up @@ -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<T: Into<String>>(
&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);
Expand Down Expand Up @@ -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<Arc<Node>, BuildError> {
Expand Down Expand Up @@ -1066,21 +1067,12 @@ fn sanitize_alias<T: Into<String>>(node_alias: T) -> Result<String, BuildError>
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)]
Expand All @@ -1089,38 +1081,32 @@ mod tests {

use super::NodeBuilder;

fn create_node_with_alias<T: Into<String>>(alias: T) -> Result<Node, BuildError> {
NodeBuilder::new().set_node_alias(&alias.into())?.build()
fn create_node_with_alias(alias: String) -> Result<Node, BuildError> {
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]
fn node_alias_with_sandwiched_null() {
// 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());
}

#[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);
}
}
20 changes: 8 additions & 12 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down

0 comments on commit 8791d7c

Please sign in to comment.