From 12dfc1a250d6ed19d98cefdbac57624bea2c9d7f Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Mon, 22 Jul 2024 21:20:36 +0100 Subject: [PATCH 01/14] feat: sanitize and set node alias What this commit does: Implements a method `set_node_alias` on NodeBuilder to allow callers customize/set the value of the node alias. This method sanitizes the user-provided alias by ensuring the following: + Node alias is UTF-8-encoded String + Node alias is non-empty + Node alias cannot exceed 32 bytes + Node alias is only valid up to the first null byte. Every character after the null byte is discraded Additionally, a test case is provided to cover sanitizing empty node alias, as well as an alias with emojis (copied and modified from rust-lightning) and a sandwiched null byte. --- src/builder.rs | 83 +++++++++++++++++++++++++++++++++++++++++++++++++- src/config.rs | 6 ++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/src/builder.rs b/src/builder.rs index fc9f839b0..f6eeb7cd3 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -109,7 +109,7 @@ impl Default for LiquiditySourceConfig { /// An error encountered during building a [`Node`]. /// /// [`Node`]: crate::Node -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub enum BuildError { /// The given seed bytes are invalid, e.g., have invalid length. InvalidSeedBytes, @@ -139,6 +139,8 @@ pub enum BuildError { WalletSetupFailed, /// We failed to setup the logger. LoggerSetupFailed, + /// The provided alias is invalid + InvalidNodeAlias(String), } impl fmt::Display for BuildError { @@ -159,6 +161,9 @@ 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) + }, } } } @@ -309,6 +314,17 @@ impl NodeBuilder { self } + /// 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> { + let node_alias = sanitize_alias(node_alias).map_err(|e| e)?; + + self.config.node_alias = Some(node_alias); + Ok(self) + } + /// Builds a [`Node`] instance with a [`SqliteStore`] backend and according to the options /// previously configured. pub fn build(&self) -> Result { @@ -1050,3 +1066,68 @@ fn seed_bytes_from_config( }, } } + +/// Sanitize the user-provided node alias to ensure that it is a valid protocol-specified UTF-8 string. +fn sanitize_alias>(node_alias: T) -> Result { + // Alias is convertible into UTF-8 encoded string + 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())); + } + + Ok(actual_alias.to_string()) +} + +#[cfg(test)] +mod tests { + use crate::{BuildError, Node}; + + use super::NodeBuilder; + + fn create_node_with_alias>(alias: T) -> Result { + NodeBuilder::new().set_node_alias(&alias.into())?.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()) + ); + } + + #[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(); + + 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()) + ); + } +} diff --git a/src/config.rs b/src/config.rs index fac25b562..69ff49a38 100644 --- a/src/config.rs +++ b/src/config.rs @@ -163,6 +163,11 @@ pub struct Config { /// **Note:** If unset, default parameters will be used, and you will be able to override the /// parameters on a per-payment basis in the corresponding method calls. pub sending_parameters: Option, + /// The node alias to be used in announcements. + /// + /// **Note**: This is required if, alongside a valid public socket address, node announcements + /// are to be broadcast. + pub node_alias: Option, } impl Default for Config { @@ -180,6 +185,7 @@ impl Default for Config { log_level: DEFAULT_LOG_LEVEL, anchor_channels_config: Some(AnchorChannelsConfig::default()), sending_parameters: None, + node_alias: None, } } } From e54bfe05c543405d094651f3054d6f81be7a4726 Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Thu, 25 Jul 2024 18:30:30 +0100 Subject: [PATCH 02/14] feat: broadcast node announcement with set alias What this commit does: Broadcasts node announcement with the user-provided alias, if set, else, uses the default [0u8;32]. Additionally, adds a random node alias generating function for use in the generation of random configuration. --- src/lib.rs | 12 +++++++++++- tests/common/mod.rs | 13 +++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 0148cf8d4..f432fb7dc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -601,6 +601,7 @@ impl Node { let bcast_logger = Arc::clone(&self.logger); let bcast_ann_timestamp = Arc::clone(&self.latest_node_announcement_broadcast_timestamp); let mut stop_bcast = self.stop_sender.subscribe(); + let node_alias = self.config().node_alias; runtime.spawn(async move { // We check every 30 secs whether our last broadcast is NODE_ANN_BCAST_INTERVAL away. #[cfg(not(test))] @@ -650,7 +651,16 @@ impl Node { continue; } - bcast_pm.broadcast_node_announcement([0; 3], [0; 32], addresses); + // Extract alias if set, else select the default + let alias = if let Some(ref alias) = node_alias { + let mut buf = [0_u8; 32]; + buf[..alias.as_bytes().len()].copy_from_slice(alias.as_bytes()); + buf + } else { + [0; 32] + }; + + bcast_pm.broadcast_node_announcement([0; 3], alias, addresses); let unix_time_secs_opt = SystemTime::now().duration_since(UNIX_EPOCH).ok().map(|d| d.as_secs()); diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 09c17dcf2..9b5d01ae3 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -200,6 +200,15 @@ pub(crate) fn random_listening_addresses() -> Vec { listening_addresses } +pub(crate) fn random_node_alias() -> Option { + let mut rng = thread_rng(); + let ranged_val = rng.gen_range(0..10); + match ranged_val { + 0 => None, + val => Some(format!("ldk-node-{}", val)), + } +} + pub(crate) fn random_config(anchor_channels: bool) -> Config { let mut config = Config::default(); @@ -220,6 +229,10 @@ pub(crate) fn random_config(anchor_channels: bool) -> Config { println!("Setting random LDK listening addresses: {:?}", rand_listening_addresses); config.listening_addresses = Some(rand_listening_addresses); + let alias = random_node_alias(); + println!("Setting random LDK node alias: {:?}", alias); + config.node_alias = alias; + config.log_level = LogLevel::Gossip; config From 5bd4a88b9c6c9135a7faab801e639298e17a3ac4 Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Tue, 6 Aug 2024 13:10:44 +0100 Subject: [PATCH 03/14] 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 1c9497264..55725c6a2 100644 --- a/bindings/ldk_node.udl +++ b/bindings/ldk_node.udl @@ -16,6 +16,7 @@ dictionary Config { LogLevel log_level; AnchorChannelsConfig? anchor_channels_config; SendingParameters? sending_parameters; + 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(); @@ -238,6 +241,7 @@ enum BuildError { "KVStoreSetupFailed", "WalletSetupFailed", "LoggerSetupFailed", + "InvalidNodeAlias" }; [Enum] diff --git a/src/builder.rs b/src/builder.rs index f6eeb7cd3..7895900c9 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -121,6 +121,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 @@ -139,8 +141,6 @@ pub enum BuildError { WalletSetupFailed, /// We failed to setup the logger. LoggerSetupFailed, - /// The provided alias is invalid - InvalidNodeAlias(String), } impl fmt::Display for BuildError { @@ -161,9 +161,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."), } } } @@ -316,9 +314,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); @@ -522,6 +518,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> { @@ -1073,21 +1074,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)] @@ -1096,19 +1088,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] @@ -1116,7 +1105,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()); } @@ -1124,10 +1113,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 f432fb7dc..5d306dc5d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -645,22 +645,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()); From 8e73296d7331b85934388acede59c3a6291d335b Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Tue, 6 Aug 2024 15:41:29 +0100 Subject: [PATCH 04/14] docs: clarify node announcement broadcast logic --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 4078ce67b..feabca0f8 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,9 @@ LDK Node currently comes with a decidedly opinionated set of design choices: - Gossip data may be sourced via Lightning's peer-to-peer network or the [Rapid Gossip Sync](https://docs.rs/lightning-rapid-gossip-sync/*/lightning_rapid_gossip_sync/) protocol. - Entropy for the Lightning and on-chain wallets may be sourced from raw bytes or a [BIP39](https://github.com/bitcoin/bips/blob/master/bip-0039.mediawiki) mnemonic. In addition, LDK Node offers the means to generate and persist the entropy bytes to disk. +**Note**: +Regarding node announcements, we have decided not to broadcast these announcements if either the node's listening addresses or its node alias are not set. + ## Language Support LDK Node itself is written in [Rust][rust] and may therefore be natively added as a library dependency to any `std` Rust program. However, beyond its Rust API it also offers language bindings for [Swift][swift], [Kotlin][kotlin], and [Python][python] based on the [UniFFI](https://github.com/mozilla/uniffi-rs/). Moreover, [Flutter bindings][flutter_bindings] are also available. From 7ad7029415022fc29da245454a3dc8b3aed1f92c Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Thu, 8 Aug 2024 14:14:25 +0100 Subject: [PATCH 05/14] refactor: update UserConfig if node alias/listening addresses are unconfigured --- src/config.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/config.rs b/src/config.rs index 69ff49a38..54d7158fd 100644 --- a/src/config.rs +++ b/src/config.rs @@ -283,5 +283,11 @@ pub(crate) fn default_user_config(config: &Config) -> UserConfig { user_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = config.anchor_channels_config.is_some(); + if config.listening_addresses.is_none() || config.node_alias.is_none() { + user_config.accept_forwards_to_priv_channels = false; + user_config.channel_handshake_config.announced_channel = false; + user_config.channel_handshake_limits.force_announced_channel_preference = true; + } + user_config } From 81a0f4d7fb4e04dd7f1b9fd461d8d075b8691263 Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Thu, 29 Aug 2024 07:57:25 +0100 Subject: [PATCH 06/14] refactor: update node alias sanitization What this commit does: + Updates the sanitization function for node alias to return NodeAlias + Updates the node alias type in the configuration to NodeAlias and implements a conversion to/from String for bindings + With this update, regardless of where the alias is set, i.e. in the set_node_alias or directly, sanitization occurs. --- bindings/ldk_node.udl | 7 +++++- src/builder.rs | 54 +++++++++++++++++++++++++------------------ src/config.rs | 7 +++--- src/uniffi_types.rs | 15 +++++++++++- 4 files changed, 55 insertions(+), 28 deletions(-) diff --git a/bindings/ldk_node.udl b/bindings/ldk_node.udl index 55725c6a2..adcfe9ea4 100644 --- a/bindings/ldk_node.udl +++ b/bindings/ldk_node.udl @@ -16,7 +16,7 @@ dictionary Config { LogLevel log_level; AnchorChannelsConfig? anchor_channels_config; SendingParameters? sending_parameters; - string? node_alias; + NodeAlias? node_alias; }; dictionary AnchorChannelsConfig { @@ -205,6 +205,7 @@ enum NodeError { "InvalidNetwork", "InvalidUri", "InvalidQuantity", + "InvalidNodeAlias", "DuplicatePayment", "UnsupportedCurrency", "InsufficientFunds", @@ -235,6 +236,7 @@ enum BuildError { "InvalidSystemTime", "InvalidChannelMonitor", "InvalidListeningAddresses", + "InvalidNodeAlias", "ReadFailed", "WriteFailed", "StoragePathAccessFailed", @@ -533,3 +535,6 @@ typedef string Mnemonic; [Custom] typedef string UntrustedString; + +[Custom] +typedef string NodeAlias; diff --git a/src/builder.rs b/src/builder.rs index 7895900c9..9481d5343 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -32,6 +32,7 @@ use lightning::chain::{chainmonitor, BestBlock, Watch}; use lightning::ln::channelmanager::{self, ChainParameters, ChannelManagerReadArgs}; use lightning::ln::msgs::{RoutingMessageHandler, SocketAddress}; use lightning::ln::peer_handler::{IgnoringMessageHandler, MessageHandler}; +use lightning::routing::gossip::NodeAlias; use lightning::routing::router::DefaultRouter; use lightning::routing::scoring::{ ProbabilisticScorer, ProbabilisticScoringDecayParameters, ProbabilisticScoringFeeParameters, @@ -121,7 +122,7 @@ pub enum BuildError { InvalidChannelMonitor, /// The given listening addresses are invalid, e.g. too many were passed. InvalidListeningAddresses, - /// The provided alias is invalid + /// The provided alias is invalid. InvalidNodeAlias, /// We failed to read data from the [`KVStore`]. /// @@ -315,7 +316,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: String) -> Result<&mut Self, BuildError> { - let node_alias = sanitize_alias(node_alias).map_err(|e| e)?; + let node_alias = sanitize_alias(&node_alias)?; self.config.node_alias = Some(node_alias); Ok(self) @@ -1069,51 +1070,58 @@ fn seed_bytes_from_config( } /// Sanitize the user-provided node alias to ensure that it is a valid protocol-specified UTF-8 string. -fn sanitize_alias>(node_alias: T) -> Result { - // Alias is convertible into UTF-8 encoded string - let node_alias: String = node_alias.into(); - let alias = node_alias.trim(); +pub fn sanitize_alias(alias_str: &str) -> Result { + let alias = alias_str.trim(); - // Alias must be 32-bytes long or less + // Alias must be 32-bytes long or less. if alias.as_bytes().len() > 32 { return Err(BuildError::InvalidNodeAlias); } - Ok(alias.to_string()) + let mut bytes = [0u8; 32]; + bytes[..alias.as_bytes().len()].copy_from_slice(alias.as_bytes()); + Ok(NodeAlias(bytes)) } #[cfg(test)] mod tests { - use crate::{BuildError, Node}; + use lightning::routing::gossip::NodeAlias; - use super::NodeBuilder; - - fn create_node_with_alias(alias: String) -> Result { - NodeBuilder::new().set_node_alias(alias)?.build() - } + use crate::{builder::sanitize_alias, BuildError}; #[test] - fn empty_node_alias() { + fn sanitize_empty_node_alias() { // Empty node alias let alias = ""; - let node = create_node_with_alias(alias.to_string()); - assert_eq!(node.err().unwrap(), BuildError::InvalidNodeAlias); + let mut buf = [0u8; 32]; + buf[..alias.as_bytes().len()].copy_from_slice(alias.as_bytes()); + + let expected_node_alias = NodeAlias([0; 32]); + let node_alias = sanitize_alias(alias).unwrap(); + assert_eq!(node_alias, expected_node_alias); } #[test] - fn node_alias_with_sandwiched_null() { + fn sanitize_alias_with_sandwiched_null() { // Alias with emojis - let expected_alias = "I\u{1F496}LDK-Node!"; + let alias = "I\u{1F496}LDK-Node!"; + let mut buf = [0u8; 32]; + buf[..alias.as_bytes().len()].copy_from_slice(alias.as_bytes()); + let expected_alias = NodeAlias(buf); + let user_provided_alias = "I\u{1F496}LDK-Node!\0\u{26A1}"; - let node = create_node_with_alias(user_provided_alias.to_string()).unwrap(); + let node_alias = sanitize_alias(user_provided_alias).unwrap(); + + let node_alias_display = format!("{}", node_alias); - assert_eq!(expected_alias, node.config().node_alias.unwrap()); + assert_eq!(alias, &node_alias_display); + assert_ne!(expected_alias, node_alias); } #[test] - fn node_alias_longer_than_32_bytes() { + fn sanitize_alias_gt_32_bytes() { let alias = "This is a string longer than thirty-two bytes!"; // 46 bytes - let node = create_node_with_alias(alias.to_string()); + let node = sanitize_alias(alias); assert_eq!(node.err().unwrap(), BuildError::InvalidNodeAlias); } } diff --git a/src/config.rs b/src/config.rs index 54d7158fd..275a5d5f1 100644 --- a/src/config.rs +++ b/src/config.rs @@ -8,6 +8,7 @@ use crate::payment::SendingParameters; use lightning::ln::msgs::SocketAddress; +use lightning::routing::gossip::NodeAlias; use lightning::util::config::UserConfig; use lightning::util::logger::Level as LogLevel; @@ -165,9 +166,9 @@ pub struct Config { pub sending_parameters: Option, /// The node alias to be used in announcements. /// - /// **Note**: This is required if, alongside a valid public socket address, node announcements - /// are to be broadcast. - pub node_alias: Option, + /// **Note**: Node announcements will only be broadcast if the node_alias and the + /// listening_addresses are set. + pub node_alias: Option, } impl Default for Config { diff --git a/src/uniffi_types.rs b/src/uniffi_types.rs index 17e5713d9..2a6ac8da3 100644 --- a/src/uniffi_types.rs +++ b/src/uniffi_types.rs @@ -19,7 +19,7 @@ pub use lightning::ln::{ChannelId, PaymentHash, PaymentPreimage, PaymentSecret}; pub use lightning::offers::invoice::Bolt12Invoice; pub use lightning::offers::offer::{Offer, OfferId}; pub use lightning::offers::refund::Refund; -pub use lightning::routing::gossip::{NodeId, RoutingFees}; +pub use lightning::routing::gossip::{NodeAlias, NodeId, RoutingFees}; pub use lightning::util::string::UntrustedString; pub use lightning_invoice::Bolt11Invoice; @@ -30,6 +30,7 @@ pub use bip39::Mnemonic; use crate::UniffiCustomTypeConverter; +use crate::builder::sanitize_alias; use crate::error::Error; use crate::hex_utils; use crate::{SocketAddress, UserChannelId}; @@ -324,3 +325,15 @@ impl UniffiCustomTypeConverter for UntrustedString { obj.to_string() } } + +impl UniffiCustomTypeConverter for NodeAlias { + type Builtin = String; + + fn into_custom(val: Self::Builtin) -> uniffi::Result { + Ok(sanitize_alias(&val).map_err(|_| Error::InvalidNodeAlias)?) + } + + fn from_custom(obj: Self) -> Self::Builtin { + obj.to_string() + } +} From 32e70964a364b7cdb7f1878711878dcc1c823acd Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Thu, 29 Aug 2024 08:12:47 +0100 Subject: [PATCH 07/14] refactor: decompose connecting w/ peer & opening a channel What this commit does: + Decomposes connect_open_channel into two different functions: open_channel and open_announced_channel. This allows opening announced channels based on configured node alias and listening addresses values. + This enforces channel announcement only on the condition that both configuration values are set. + Additionally, a new error variant `OpenAnnouncedChannelFailed` is introduced to capture failure. Note: I thought I added the `InvalidNodeAlias` variant in the previous commit --- bindings/ldk_node.udl | 5 +++- src/config.rs | 55 +++++++++++++++++++++++++++++++++++- src/error.rs | 8 ++++++ src/lib.rs | 66 ++++++++++++++++++++++++++++++++++--------- 4 files changed, 118 insertions(+), 16 deletions(-) diff --git a/bindings/ldk_node.udl b/bindings/ldk_node.udl index adcfe9ea4..db0d609ae 100644 --- a/bindings/ldk_node.udl +++ b/bindings/ldk_node.udl @@ -72,7 +72,9 @@ interface Node { [Throws=NodeError] void disconnect(PublicKey node_id); [Throws=NodeError] - UserChannelId connect_open_channel(PublicKey node_id, SocketAddress address, u64 channel_amount_sats, u64? push_to_counterparty_msat, ChannelConfig? channel_config, boolean announce_channel); + UserChannelId open_channel(PublicKey node_id, SocketAddress address, u64 channel_amount_sats, u64? push_to_counterparty_msat, ChannelConfig? channel_config); + [Throws=NodeError] + UserChannelId open_announced_channel(PublicKey node_id, SocketAddress address, u64 channel_amount_sats, u64? push_to_counterparty_msat, ChannelConfig? channel_config); [Throws=NodeError] void close_channel([ByRef]UserChannelId user_channel_id, PublicKey counterparty_node_id); [Throws=NodeError] @@ -211,6 +213,7 @@ enum NodeError { "InsufficientFunds", "LiquiditySourceUnavailable", "LiquidityFeeTooHigh", + "OpenAnnouncedChannelFailed" }; dictionary NodeStatus { diff --git a/src/config.rs b/src/config.rs index 275a5d5f1..81246c51b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -111,6 +111,9 @@ pub struct Config { /// The used Bitcoin network. pub network: Network, /// The addresses on which the node will listen for incoming connections. + /// + /// **Note**: Node announcements will only be broadcast if the node_alias and the + /// listening_addresses are set. pub listening_addresses: Option>, /// The time in-between background sync attempts of the onchain wallet, in seconds. /// @@ -272,6 +275,17 @@ pub fn default_config() -> Config { Config::default() } +/// Checks if a node is can announce a channel based on the configured values of both the node's +/// alias and its listening addresses. If either of them is unset, the node cannot announce the +/// channel. +pub fn can_announce_channel(config: &Config) -> bool { + let are_addresses_set = + config.listening_addresses.clone().is_some_and(|addr_vec| !addr_vec.is_empty()); + let is_alias_set = config.node_alias.is_some(); + + is_alias_set && are_addresses_set +} + pub(crate) fn default_user_config(config: &Config) -> UserConfig { // Initialize the default config values. // @@ -284,7 +298,7 @@ pub(crate) fn default_user_config(config: &Config) -> UserConfig { user_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = config.anchor_channels_config.is_some(); - if config.listening_addresses.is_none() || config.node_alias.is_none() { + if !can_announce_channel(config) { user_config.accept_forwards_to_priv_channels = false; user_config.channel_handshake_config.announced_channel = false; user_config.channel_handshake_limits.force_announced_channel_preference = true; @@ -292,3 +306,42 @@ pub(crate) fn default_user_config(config: &Config) -> UserConfig { user_config } + +#[cfg(test)] +mod tests { + use std::str::FromStr; + + use lightning::{ln::msgs::SocketAddress, routing::gossip::NodeAlias}; + + use crate::config::can_announce_channel; + + use super::Config; + + #[test] + fn node_can_announce_channel() { + // Default configuration with node alias and listening addresses unset + let mut node_config = Config::default(); + assert_eq!(can_announce_channel(&node_config), false); + + // Set node alias with listening addresses unset + let alias_frm_str = |alias: &str| { + let mut bytes = [0u8; 32]; + bytes[..alias.as_bytes().len()].copy_from_slice(alias.as_bytes()); + NodeAlias(bytes) + }; + node_config.node_alias = Some(alias_frm_str("LDK_Node")); + assert_eq!(can_announce_channel(&node_config), false); + + // Set node alias with an empty list of listening addresses + node_config.listening_addresses = Some(vec![]); + assert_eq!(can_announce_channel(&node_config), false); + + // Set node alias with a non-empty list of listening addresses + let socket_address = + SocketAddress::from_str("localhost:8000").expect("Socket address conversion failed."); + if let Some(ref mut addresses) = node_config.listening_addresses { + addresses.push(socket_address); + } + assert_eq!(can_announce_channel(&node_config), true); + } +} diff --git a/src/error.rs b/src/error.rs index 660c2036e..5e7dbeacd 100644 --- a/src/error.rs +++ b/src/error.rs @@ -98,6 +98,8 @@ pub enum Error { InvalidUri, /// The given quantity is invalid. InvalidQuantity, + /// The given node alias is invalid. + InvalidNodeAlias, /// A payment with the given hash has already been initiated. DuplicatePayment, /// The provided offer was denonminated in an unsupported currency. @@ -108,6 +110,10 @@ pub enum Error { LiquiditySourceUnavailable, /// The given operation failed due to the LSP's required opening fee being too high. LiquidityFeeTooHigh, + /// Returned when trying to open an announced channel with a peer. This + /// error occurs when a [`crate::Node`'s] alias or listening addresses + /// are unconfigured. + OpenAnnouncedChannelFailed, } impl fmt::Display for Error { @@ -163,6 +169,7 @@ impl fmt::Display for Error { Self::InvalidNetwork => write!(f, "The given network is invalid."), Self::InvalidUri => write!(f, "The given URI is invalid."), Self::InvalidQuantity => write!(f, "The given quantity is invalid."), + Self::InvalidNodeAlias => write!(f, "The given node alias is invalid."), Self::DuplicatePayment => { write!(f, "A payment with the given hash has already been initiated.") }, @@ -178,6 +185,7 @@ impl fmt::Display for Error { Self::LiquidityFeeTooHigh => { write!(f, "The given operation failed due to the LSP's required opening fee being too high.") }, + Self::OpenAnnouncedChannelFailed => write!(f, "Failed to open an announced channel."), } } } diff --git a/src/lib.rs b/src/lib.rs index 5d306dc5d..ce5956cbc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -47,7 +47,7 @@ //! //! let node_id = PublicKey::from_str("NODE_ID").unwrap(); //! let node_addr = SocketAddress::from_str("IP_ADDR:PORT").unwrap(); -//! node.connect_open_channel(node_id, node_addr, 10000, None, None, false).unwrap(); +//! node.open_channel(node_id, node_addr, 10000, None, None).unwrap(); //! //! let event = node.wait_next_event(); //! println!("EVENT: {:?}", event); @@ -63,7 +63,8 @@ //! [`build`]: Builder::build //! [`start`]: Node::start //! [`stop`]: Node::stop -//! [`connect_open_channel`]: Node::connect_open_channel +//! [`open_channel`]: Node::open_channel +//! [`open_announced_channel`]: Node::open_announced_channel //! [`send`]: Bolt11Payment::send //! #![cfg_attr(not(feature = "uniffi"), deny(missing_docs))] @@ -114,6 +115,7 @@ pub use io::utils::generate_entropy_mnemonic; #[cfg(feature = "uniffi")] use uniffi_types::*; +pub use builder::sanitize_alias; #[cfg(feature = "uniffi")] pub use builder::ArcedNodeBuilder as Builder; pub use builder::BuildError; @@ -121,8 +123,9 @@ pub use builder::BuildError; pub use builder::NodeBuilder as Builder; use config::{ - default_user_config, LDK_WALLET_SYNC_TIMEOUT_SECS, NODE_ANN_BCAST_INTERVAL, - PEER_RECONNECTION_INTERVAL, RESOLVED_CHANNEL_MONITOR_ARCHIVAL_INTERVAL, RGS_SYNC_INTERVAL, + can_announce_channel, default_user_config, LDK_WALLET_SYNC_TIMEOUT_SECS, + NODE_ANN_BCAST_INTERVAL, PEER_RECONNECTION_INTERVAL, + RESOLVED_CHANNEL_MONITOR_ARCHIVAL_INTERVAL, RGS_SYNC_INTERVAL, WALLET_SYNC_INTERVAL_MINIMUM_SECS, }; use connection::ConnectionManager; @@ -601,7 +604,8 @@ impl Node { let bcast_logger = Arc::clone(&self.logger); let bcast_ann_timestamp = Arc::clone(&self.latest_node_announcement_broadcast_timestamp); let mut stop_bcast = self.stop_sender.subscribe(); - let node_alias = self.config().node_alias; + let node_alias = self.config.node_alias.clone(); + let can_announce_channel = can_announce_channel(&self.config); runtime.spawn(async move { // We check every 30 secs whether our last broadcast is NODE_ANN_BCAST_INTERVAL away. #[cfg(not(test))] @@ -646,17 +650,19 @@ impl Node { let addresses = bcast_config.listening_addresses.clone().unwrap_or(Vec::new()); 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 + alias.0 }); - if addresses.is_empty() || alias.is_none() { + if !can_announce_channel { // 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.unwrap(), addresses); + if let Some(node_alias) = alias { + bcast_pm.broadcast_node_announcement([0; 3], node_alias, addresses); + } else { + continue + } let unix_time_secs_opt = SystemTime::now().duration_since(UNIX_EPOCH).ok().map(|d| d.as_secs()); @@ -1189,10 +1195,9 @@ impl Node { /// opening the channel. /// /// Returns a [`UserChannelId`] allowing to locally keep track of the channel. - pub fn connect_open_channel( + fn connect_open_channel( &self, node_id: PublicKey, address: SocketAddress, channel_amount_sats: u64, push_to_counterparty_msat: Option, channel_config: Option, - announce_channel: bool, ) -> Result { let rt_lock = self.runtime.read().unwrap(); if rt_lock.is_none() { @@ -1254,12 +1259,13 @@ impl Node { } let mut user_config = default_user_config(&self.config); - user_config.channel_handshake_config.announced_channel = announce_channel; + let can_announce_channel = can_announce_channel(&self.config); + user_config.channel_handshake_config.announced_channel = can_announce_channel; user_config.channel_config = (channel_config.unwrap_or_default()).clone().into(); // We set the max inflight to 100% for private channels. // FIXME: LDK will default to this behavior soon, too, at which point we should drop this // manual override. - if !announce_channel { + if !can_announce_channel { user_config .channel_handshake_config .max_inbound_htlc_value_in_flight_percent_of_channel = 100; @@ -1292,6 +1298,38 @@ impl Node { } } + /// Opens a channel with a peer. + pub fn open_channel( + &self, node_id: PublicKey, address: SocketAddress, channel_amount_sats: u64, + push_to_counterparty_msat: Option, channel_config: Option>, + ) -> Result { + self.connect_open_channel( + node_id, + address, + channel_amount_sats, + push_to_counterparty_msat, + channel_config, + ) + } + + /// Opens an announced channel with a peer. + pub fn open_announced_channel( + &self, node_id: PublicKey, address: SocketAddress, channel_amount_sats: u64, + push_to_counterparty_msat: Option, channel_config: Option>, + ) -> Result { + if !can_announce_channel(&self.config) { + return Err(Error::OpenAnnouncedChannelFailed); + } + + self.open_channel( + node_id, + address, + channel_amount_sats, + push_to_counterparty_msat, + channel_config, + ) + } + /// Manually sync the LDK and BDK wallets with the current chain state and update the fee rate /// cache. /// From 059a30e98f2bfbd8614ef7cb0fe417073e499351 Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Thu, 29 Aug 2024 08:13:56 +0100 Subject: [PATCH 08/14] docs: cleanup documentation --- README.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index feabca0f8..7bcfb6f78 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ A ready-to-go Lightning node library built using [LDK][ldk] and [BDK][bdk]. LDK Node is a self-custodial Lightning node in library form. Its central goal is to provide a small, simple, and straightforward interface that enables users to easily set up and run a Lightning node with an integrated on-chain wallet. While minimalism is at its core, LDK Node aims to be sufficiently modular and configurable to be useful for a variety of use cases. ## Getting Started -The primary abstraction of the library is the [`Node`][api_docs_node], which can be retrieved by setting up and configuring a [`Builder`][api_docs_builder] to your liking and calling one of the `build` methods. `Node` can then be controlled via commands such as `start`, `stop`, `connect_open_channel`, `send`, etc. +The primary abstraction of the library is the [`Node`][api_docs_node], which can be retrieved by setting up and configuring a [`Builder`][api_docs_builder] to your liking and calling one of the `build` methods. `Node` can then be controlled via commands such as `start`, `stop`, `open_channel`, `open_announced_channel`, `send`, etc. ```rust use ldk_node::Builder; @@ -37,7 +37,7 @@ fn main() { let node_id = PublicKey::from_str("NODE_ID").unwrap(); let node_addr = SocketAddress::from_str("IP_ADDR:PORT").unwrap(); - node.connect_open_channel(node_id, node_addr, 10000, None, None, false).unwrap(); + node.open_channel(node_id, node_addr, 10000, None, None).unwrap(); let event = node.wait_next_event(); println!("EVENT: {:?}", event); @@ -60,9 +60,6 @@ LDK Node currently comes with a decidedly opinionated set of design choices: - Gossip data may be sourced via Lightning's peer-to-peer network or the [Rapid Gossip Sync](https://docs.rs/lightning-rapid-gossip-sync/*/lightning_rapid_gossip_sync/) protocol. - Entropy for the Lightning and on-chain wallets may be sourced from raw bytes or a [BIP39](https://github.com/bitcoin/bips/blob/master/bip-0039.mediawiki) mnemonic. In addition, LDK Node offers the means to generate and persist the entropy bytes to disk. -**Note**: -Regarding node announcements, we have decided not to broadcast these announcements if either the node's listening addresses or its node alias are not set. - ## Language Support LDK Node itself is written in [Rust][rust] and may therefore be natively added as a library dependency to any `std` Rust program. However, beyond its Rust API it also offers language bindings for [Swift][swift], [Kotlin][kotlin], and [Python][python] based on the [UniFFI](https://github.com/mozilla/uniffi-rs/). Moreover, [Flutter bindings][flutter_bindings] are also available. From 09fca09e53d844c459a86ea47312a063491c917e Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Thu, 29 Aug 2024 08:19:07 +0100 Subject: [PATCH 09/14] test: update tests due to `connect_open_channel` decomposition What this commit does: + Replaces calls to `connect_open_channel` with `open_channel` and `open_announced_channel` where appropriate. Status: Work In Progress (WIP) Observation: + The integration tests are now flaky and need further investigation to ascertain the reason(s) why and then to fix. --- tests/common/mod.rs | 17 ++++++++--------- tests/integration_tests_cln.rs | 11 ++--------- tests/integration_tests_rust.rs | 19 +++++++++---------- 3 files changed, 19 insertions(+), 28 deletions(-) diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 9b5d01ae3..b616f7eb5 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -11,11 +11,13 @@ use ldk_node::io::sqlite_store::SqliteStore; use ldk_node::payment::{PaymentDirection, PaymentKind, PaymentStatus}; use ldk_node::{ - Builder, Config, Event, LightningBalance, LogLevel, Node, NodeError, PendingSweepBalance, + sanitize_alias, Builder, Config, Event, LightningBalance, LogLevel, Node, NodeError, + PendingSweepBalance, }; use lightning::ln::msgs::SocketAddress; use lightning::ln::{PaymentHash, PaymentPreimage}; +use lightning::routing::gossip::NodeAlias; use lightning::util::persist::KVStore; use lightning::util::test_utils::TestStore; use lightning_persister::fs_store::FilesystemStore; @@ -200,12 +202,12 @@ pub(crate) fn random_listening_addresses() -> Vec { listening_addresses } -pub(crate) fn random_node_alias() -> Option { +pub(crate) fn random_node_alias() -> Option { let mut rng = thread_rng(); let ranged_val = rng.gen_range(0..10); match ranged_val { 0 => None, - val => Some(format!("ldk-node-{}", val)), + val => Some(sanitize_alias(&format!("ldk-node-{}", val)).unwrap()), } } @@ -398,17 +400,15 @@ pub(crate) fn premine_and_distribute_funds( } pub fn open_channel( - node_a: &TestNode, node_b: &TestNode, funding_amount_sat: u64, announce: bool, - electrsd: &ElectrsD, + node_a: &TestNode, node_b: &TestNode, funding_amount_sat: u64, electrsd: &ElectrsD, ) { node_a - .connect_open_channel( + .open_announced_channel( node_b.node_id(), node_b.listening_addresses().unwrap().first().unwrap().clone(), funding_amount_sat, None, None, - announce, ) .unwrap(); assert!(node_a.list_peers().iter().find(|c| { c.node_id == node_b.node_id() }).is_some()); @@ -447,13 +447,12 @@ pub(crate) fn do_channel_full_cycle( let funding_amount_sat = 2_080_000; let push_msat = (funding_amount_sat / 2) * 1000; // balance the channel node_a - .connect_open_channel( + .open_channel( node_b.node_id(), node_b.listening_addresses().unwrap().first().unwrap().clone(), funding_amount_sat, Some(push_msat), None, - true, ) .unwrap(); diff --git a/tests/integration_tests_cln.rs b/tests/integration_tests_cln.rs index bcb84833f..13b5c44c6 100644 --- a/tests/integration_tests_cln.rs +++ b/tests/integration_tests_cln.rs @@ -82,15 +82,8 @@ fn test_cln() { // Open the channel let funding_amount_sat = 1_000_000; - node.connect_open_channel( - cln_node_id, - cln_address, - funding_amount_sat, - Some(500_000_000), - None, - false, - ) - .unwrap(); + node.open_channel(cln_node_id, cln_address, funding_amount_sat, Some(500_000_000), None) + .unwrap(); let funding_txo = common::expect_channel_pending_event!(node, cln_node_id); common::wait_for_tx(&electrs_client, funding_txo.txid); diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 6b5b405dd..89786f826 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -83,13 +83,12 @@ fn channel_open_fails_when_funds_insufficient() { println!("\nA -- connect_open_channel -> B"); assert_eq!( Err(NodeError::InsufficientFunds), - node_a.connect_open_channel( + node_a.open_channel( node_b.node_id(), node_b.listening_addresses().unwrap().first().unwrap().clone(), 120000, None, None, - true ) ); } @@ -132,16 +131,16 @@ fn multi_hop_sending() { // \ / // (1M:0)- N3 -(1M:0) - open_channel(&nodes[0], &nodes[1], 100_000, true, &electrsd); - open_channel(&nodes[1], &nodes[2], 1_000_000, true, &electrsd); + open_channel(&nodes[0], &nodes[1], 100_000, &electrsd); + open_channel(&nodes[1], &nodes[2], 1_000_000, &electrsd); // We need to sync wallets in-between back-to-back channel opens from the same node so BDK // wallet picks up on the broadcast funding tx and doesn't double-spend itself. // // TODO: Remove once fixed in BDK. nodes[1].sync_wallets().unwrap(); - open_channel(&nodes[1], &nodes[3], 1_000_000, true, &electrsd); - open_channel(&nodes[2], &nodes[4], 1_000_000, true, &electrsd); - open_channel(&nodes[3], &nodes[4], 1_000_000, true, &electrsd); + open_channel(&nodes[1], &nodes[3], 1_000_000, &electrsd); + open_channel(&nodes[2], &nodes[4], 1_000_000, &electrsd); + open_channel(&nodes[3], &nodes[4], 1_000_000, &electrsd); generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6); @@ -419,7 +418,7 @@ fn simple_bolt12_send_receive() { ); node_a.sync_wallets().unwrap(); - open_channel(&node_a, &node_b, 4_000_000, true, &electrsd); + open_channel(&node_a, &node_b, 4_000_000, &electrsd); generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6); @@ -626,7 +625,7 @@ fn generate_bip21_uri() { ); node_a.sync_wallets().unwrap(); - open_channel(&node_a, &node_b, 4_000_000, true, &electrsd); + open_channel(&node_a, &node_b, 4_000_000, &electrsd); generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6); node_a.sync_wallets().unwrap(); @@ -667,7 +666,7 @@ fn unified_qr_send_receive() { ); node_a.sync_wallets().unwrap(); - open_channel(&node_a, &node_b, 4_000_000, true, &electrsd); + open_channel(&node_a, &node_b, 4_000_000, &electrsd); generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6); node_a.sync_wallets().unwrap(); From 116003435503c422ba24ab108853db4972829be5 Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Thu, 29 Aug 2024 08:41:20 +0100 Subject: [PATCH 10/14] fix: change channel config type in open_(announced)_channel What this commit does: + Removes the wrapping Arc from the channel config. This is a missed update after rebasing. --- src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ce5956cbc..12494d00d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1301,7 +1301,7 @@ impl Node { /// Opens a channel with a peer. pub fn open_channel( &self, node_id: PublicKey, address: SocketAddress, channel_amount_sats: u64, - push_to_counterparty_msat: Option, channel_config: Option>, + push_to_counterparty_msat: Option, channel_config: Option, ) -> Result { self.connect_open_channel( node_id, @@ -1315,7 +1315,7 @@ impl Node { /// Opens an announced channel with a peer. pub fn open_announced_channel( &self, node_id: PublicKey, address: SocketAddress, channel_amount_sats: u64, - push_to_counterparty_msat: Option, channel_config: Option>, + push_to_counterparty_msat: Option, channel_config: Option, ) -> Result { if !can_announce_channel(&self.config) { return Err(Error::OpenAnnouncedChannelFailed); From 6c3deafa23f41f69a1d518220487ead0554e8427 Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Thu, 29 Aug 2024 09:17:58 +0100 Subject: [PATCH 11/14] fix: remove broken intra doc link --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 12494d00d..5405dedec 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,7 +20,7 @@ //! //! The primary abstraction of the library is the [`Node`], which can be retrieved by setting up //! and configuring a [`Builder`] to your liking and calling [`build`]. `Node` can then be -//! controlled via commands such as [`start`], [`stop`], [`connect_open_channel`], +//! controlled via commands such as [`start`], [`stop`], [`open_channel`], [`open_announced_channel`] //! [`send`], etc.: //! //! ```no_run From 6aff282be46876800a6b06466a2ef637f4c1a5e3 Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Thu, 29 Aug 2024 09:40:58 +0100 Subject: [PATCH 12/14] fix: remove unstable feature `is_some_and` --- src/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config.rs b/src/config.rs index 81246c51b..5a76343c9 100644 --- a/src/config.rs +++ b/src/config.rs @@ -280,7 +280,7 @@ pub fn default_config() -> Config { /// channel. pub fn can_announce_channel(config: &Config) -> bool { let are_addresses_set = - config.listening_addresses.clone().is_some_and(|addr_vec| !addr_vec.is_empty()); + config.listening_addresses.clone().map_or(false, |addr_vec| !addr_vec.is_empty()); let is_alias_set = config.node_alias.is_some(); is_alias_set && are_addresses_set From 0af5df624d6b6cb0d6ed0d17d59b5468e559daea Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Mon, 23 Sep 2024 21:26:31 +0100 Subject: [PATCH 13/14] refactor: improve channel announcement logic and fix binding tests This commit addresses changes necessary to: - fix failing tests for generated bindings - remove unnecessary error variant previously introduced to capture failure associated with opening announced channels, and re-use existing variants that better capture the reasons, i.e. `InvalidNodeAlias` and `InvalidSocketAddress`, why opening an announced channel failed. - correct visibility specifiers for objects, and - cleanup nitpicks Specific modifications across several files include: - updating the UDL file, as well as tests related to python and kotlin that call `open_channel` and/or open_announced_channel - repositioning/rearranging methods and struct fields - introducing enums (`ChannelAnnouncementStatus` & `ChannelAnnouncementBlocker`) to capture and codify channel announceable eligibility, providing reasons for unannounceable channels - modifying `can_announce_channel` to utilize the aforementioned enums, as opposed to simply returning a boolean value. - cleaning up and renaming `connect_open_channel` to `open_channel_inner`, and maintaining a boolean flag for channel announcement - updating documentation, unit, and integration tests that factor all these changes --- .../lightningdevkit/ldknode/LibraryTest.kt | 2 +- bindings/ldk_node.udl | 5 +- bindings/python/src/ldk_node/test_ldk_node.py | 2 +- src/builder.rs | 33 +++-- src/config.rs | 110 +++++++++++---- src/error.rs | 5 - src/lib.rs | 130 +++++++++++------- tests/common/mod.rs | 12 +- tests/integration_tests_rust.rs | 2 +- 9 files changed, 194 insertions(+), 107 deletions(-) diff --git a/bindings/kotlin/ldk-node-jvm/lib/src/test/kotlin/org/lightningdevkit/ldknode/LibraryTest.kt b/bindings/kotlin/ldk-node-jvm/lib/src/test/kotlin/org/lightningdevkit/ldknode/LibraryTest.kt index b629793cd..e2bcd4c89 100644 --- a/bindings/kotlin/ldk-node-jvm/lib/src/test/kotlin/org/lightningdevkit/ldknode/LibraryTest.kt +++ b/bindings/kotlin/ldk-node-jvm/lib/src/test/kotlin/org/lightningdevkit/ldknode/LibraryTest.kt @@ -175,7 +175,7 @@ class LibraryTest { assertEquals(100000uL, totalBalance1) assertEquals(100000uL, totalBalance2) - node1.connectOpenChannel(nodeId2, listenAddress2, 50000u, null, null, true) + node1.openChannel(nodeId2, listenAddress2, 50000u, null, null) val channelPendingEvent1 = node1.waitNextEvent() println("Got event: $channelPendingEvent1") diff --git a/bindings/ldk_node.udl b/bindings/ldk_node.udl index db0d609ae..6663604a2 100644 --- a/bindings/ldk_node.udl +++ b/bindings/ldk_node.udl @@ -8,6 +8,7 @@ dictionary Config { string? log_dir_path; Network network; sequence? listening_addresses; + NodeAlias? node_alias; u64 onchain_wallet_sync_interval_secs; u64 wallet_sync_interval_secs; u64 fee_rate_cache_update_interval_secs; @@ -16,7 +17,6 @@ dictionary Config { LogLevel log_level; AnchorChannelsConfig? anchor_channels_config; SendingParameters? sending_parameters; - NodeAlias? node_alias; }; dictionary AnchorChannelsConfig { @@ -62,6 +62,7 @@ interface Node { void event_handled(); PublicKey node_id(); sequence? listening_addresses(); + NodeAlias? node_alias(); Bolt11Payment bolt11_payment(); Bolt12Payment bolt12_payment(); SpontaneousPayment spontaneous_payment(); @@ -213,7 +214,6 @@ enum NodeError { "InsufficientFunds", "LiquiditySourceUnavailable", "LiquidityFeeTooHigh", - "OpenAnnouncedChannelFailed" }; dictionary NodeStatus { @@ -246,7 +246,6 @@ enum BuildError { "KVStoreSetupFailed", "WalletSetupFailed", "LoggerSetupFailed", - "InvalidNodeAlias" }; [Enum] diff --git a/bindings/python/src/ldk_node/test_ldk_node.py b/bindings/python/src/ldk_node/test_ldk_node.py index 92c4bf2d1..4f2931440 100644 --- a/bindings/python/src/ldk_node/test_ldk_node.py +++ b/bindings/python/src/ldk_node/test_ldk_node.py @@ -155,7 +155,7 @@ def test_channel_full_cycle(self): print("TOTAL 2:", total_balance_2) self.assertEqual(total_balance_2, 100000) - node_1.connect_open_channel(node_id_2, listening_addresses_2[0], 50000, None, None, True) + node_1.open_channel(node_id_2, listening_addresses_2[0], 50000, None, None) channel_pending_event_1 = node_1.wait_next_event() assert isinstance(channel_pending_event_1, Event.CHANNEL_PENDING) diff --git a/src/builder.rs b/src/builder.rs index 9481d5343..d2ceb5f22 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -307,14 +307,9 @@ impl NodeBuilder { Ok(self) } - /// Sets the level at which [`Node`] will log messages. - pub fn set_log_level(&mut self, level: LogLevel) -> &mut Self { - self.config.log_level = level; - self - } - - /// Sets the alias the [`Node`] will use in its announcement. The provided - /// alias must be a valid UTF-8 string. + /// 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: String) -> Result<&mut Self, BuildError> { let node_alias = sanitize_alias(&node_alias)?; @@ -322,6 +317,12 @@ impl NodeBuilder { Ok(self) } + /// Sets the level at which [`Node`] will log messages. + pub fn set_log_level(&mut self, level: LogLevel) -> &mut Self { + self.config.log_level = level; + self + } + /// Builds a [`Node`] instance with a [`SqliteStore`] backend and according to the options /// previously configured. pub fn build(&self) -> Result { @@ -514,16 +515,16 @@ impl ArcedNodeBuilder { self.inner.write().unwrap().set_listening_addresses(listening_addresses).map(|_| ()) } - /// Sets the level at which [`Node`] will log messages. - pub fn set_log_level(&self, level: LogLevel) { - 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(|_| ()) } + /// Sets the level at which [`Node`] will log messages. + pub fn set_log_level(&self, level: LogLevel) { + self.inner.write().unwrap().set_log_level(level); + } + /// Builds a [`Node`] instance with a [`SqliteStore`] backend and according to the options /// previously configured. pub fn build(&self) -> Result, BuildError> { @@ -1070,7 +1071,7 @@ fn seed_bytes_from_config( } /// Sanitize the user-provided node alias to ensure that it is a valid protocol-specified UTF-8 string. -pub fn sanitize_alias(alias_str: &str) -> Result { +pub(crate) fn sanitize_alias(alias_str: &str) -> Result { let alias = alias_str.trim(); // Alias must be 32-bytes long or less. @@ -1085,9 +1086,7 @@ pub fn sanitize_alias(alias_str: &str) -> Result { #[cfg(test)] mod tests { - use lightning::routing::gossip::NodeAlias; - - use crate::{builder::sanitize_alias, BuildError}; + use super::{sanitize_alias, BuildError, NodeAlias}; #[test] fn sanitize_empty_node_alias() { diff --git a/src/config.rs b/src/config.rs index 5a76343c9..574789ac6 100644 --- a/src/config.rs +++ b/src/config.rs @@ -87,6 +87,7 @@ pub(crate) const WALLET_KEYS_SEED_LEN: usize = 64; /// | `log_dir_path` | None | /// | `network` | Bitcoin | /// | `listening_addresses` | None | +/// | `node_alias` | None | /// | `default_cltv_expiry_delta` | 144 | /// | `onchain_wallet_sync_interval_secs` | 80 | /// | `wallet_sync_interval_secs` | 30 | @@ -112,9 +113,14 @@ pub struct Config { pub network: Network, /// The addresses on which the node will listen for incoming connections. /// - /// **Note**: Node announcements will only be broadcast if the node_alias and the - /// listening_addresses are set. + /// **Note**: Node announcements will only be broadcast if the `node_alias` and the + /// `listening_addresses` are set. pub listening_addresses: Option>, + /// The node alias to be used in announcements. + /// + /// **Note**: Node announcements will only be broadcast if the `node_alias` and the + /// `listening_addresses` are set. + pub node_alias: Option, /// The time in-between background sync attempts of the onchain wallet, in seconds. /// /// **Note:** A minimum of 10 seconds is always enforced. @@ -167,11 +173,6 @@ pub struct Config { /// **Note:** If unset, default parameters will be used, and you will be able to override the /// parameters on a per-payment basis in the corresponding method calls. pub sending_parameters: Option, - /// The node alias to be used in announcements. - /// - /// **Note**: Node announcements will only be broadcast if the node_alias and the - /// listening_addresses are set. - pub node_alias: Option, } impl Default for Config { @@ -275,33 +276,68 @@ pub fn default_config() -> Config { Config::default() } +/// Specifies reasons why a channel cannot be announced. +#[derive(Debug, PartialEq)] +pub(crate) enum ChannelAnnouncementBlocker { + /// The node alias is not set. + MissingNodeAlias, + /// The listening addresses are not set. + MissingListeningAddresses, + // This listening addresses is set but the vector is empty. + EmptyListeningAddresses, +} + +/// Enumeration defining the announcement status of a channel. +#[derive(Debug, PartialEq)] +pub(crate) enum ChannelAnnouncementStatus { + /// The channel is announceable. + Announceable, + /// The channel is not announceable. + Unannounceable(ChannelAnnouncementBlocker), +} + /// Checks if a node is can announce a channel based on the configured values of both the node's -/// alias and its listening addresses. If either of them is unset, the node cannot announce the -/// channel. -pub fn can_announce_channel(config: &Config) -> bool { - let are_addresses_set = - config.listening_addresses.clone().map_or(false, |addr_vec| !addr_vec.is_empty()); - let is_alias_set = config.node_alias.is_some(); - - is_alias_set && are_addresses_set +/// alias and its listening addresses. +/// +/// If either of them is unset, the node cannot announce the channel. This ability to announce/ +/// unannounce a channel is codified with `ChannelAnnouncementStatus` +pub(crate) fn can_announce_channel(config: &Config) -> ChannelAnnouncementStatus { + if config.node_alias.is_none() { + return ChannelAnnouncementStatus::Unannounceable( + ChannelAnnouncementBlocker::MissingNodeAlias, + ); + } + + match &config.listening_addresses { + None => ChannelAnnouncementStatus::Unannounceable( + ChannelAnnouncementBlocker::MissingListeningAddresses, + ), + Some(addresses) if addresses.is_empty() => ChannelAnnouncementStatus::Unannounceable( + ChannelAnnouncementBlocker::EmptyListeningAddresses, + ), + Some(_) => ChannelAnnouncementStatus::Announceable, + } } pub(crate) fn default_user_config(config: &Config) -> UserConfig { // Initialize the default config values. // - // Note that methods such as Node::connect_open_channel might override some of the values set - // here, e.g. the ChannelHandshakeConfig, meaning these default values will mostly be relevant - // for inbound channels. + // Note that methods such as Node::open_channel and Node::open_announced_channel might override + // some of the values set here, e.g. the ChannelHandshakeConfig, meaning these default values + // will mostly be relevant for inbound channels. let mut user_config = UserConfig::default(); user_config.channel_handshake_limits.force_announced_channel_preference = false; user_config.manually_accept_inbound_channels = true; user_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = config.anchor_channels_config.is_some(); - if !can_announce_channel(config) { - user_config.accept_forwards_to_priv_channels = false; - user_config.channel_handshake_config.announced_channel = false; - user_config.channel_handshake_limits.force_announced_channel_preference = true; + match can_announce_channel(config) { + ChannelAnnouncementStatus::Announceable => (), + ChannelAnnouncementStatus::Unannounceable(_) => { + user_config.accept_forwards_to_priv_channels = false; + user_config.channel_handshake_config.announced_channel = false; + user_config.channel_handshake_limits.force_announced_channel_preference = true; + }, } user_config @@ -311,17 +347,23 @@ pub(crate) fn default_user_config(config: &Config) -> UserConfig { mod tests { use std::str::FromStr; - use lightning::{ln::msgs::SocketAddress, routing::gossip::NodeAlias}; - - use crate::config::can_announce_channel; + use crate::config::ChannelAnnouncementStatus; + use super::can_announce_channel; use super::Config; + use super::NodeAlias; + use super::SocketAddress; #[test] fn node_can_announce_channel() { // Default configuration with node alias and listening addresses unset let mut node_config = Config::default(); - assert_eq!(can_announce_channel(&node_config), false); + assert_eq!( + can_announce_channel(&node_config), + ChannelAnnouncementStatus::Unannounceable( + crate::config::ChannelAnnouncementBlocker::MissingNodeAlias + ) + ); // Set node alias with listening addresses unset let alias_frm_str = |alias: &str| { @@ -330,11 +372,21 @@ mod tests { NodeAlias(bytes) }; node_config.node_alias = Some(alias_frm_str("LDK_Node")); - assert_eq!(can_announce_channel(&node_config), false); + assert_eq!( + can_announce_channel(&node_config), + ChannelAnnouncementStatus::Unannounceable( + crate::config::ChannelAnnouncementBlocker::MissingListeningAddresses + ) + ); // Set node alias with an empty list of listening addresses node_config.listening_addresses = Some(vec![]); - assert_eq!(can_announce_channel(&node_config), false); + assert_eq!( + can_announce_channel(&node_config), + ChannelAnnouncementStatus::Unannounceable( + crate::config::ChannelAnnouncementBlocker::EmptyListeningAddresses + ) + ); // Set node alias with a non-empty list of listening addresses let socket_address = @@ -342,6 +394,6 @@ mod tests { if let Some(ref mut addresses) = node_config.listening_addresses { addresses.push(socket_address); } - assert_eq!(can_announce_channel(&node_config), true); + assert_eq!(can_announce_channel(&node_config), ChannelAnnouncementStatus::Announceable); } } diff --git a/src/error.rs b/src/error.rs index 5e7dbeacd..807e1ca54 100644 --- a/src/error.rs +++ b/src/error.rs @@ -110,10 +110,6 @@ pub enum Error { LiquiditySourceUnavailable, /// The given operation failed due to the LSP's required opening fee being too high. LiquidityFeeTooHigh, - /// Returned when trying to open an announced channel with a peer. This - /// error occurs when a [`crate::Node`'s] alias or listening addresses - /// are unconfigured. - OpenAnnouncedChannelFailed, } impl fmt::Display for Error { @@ -185,7 +181,6 @@ impl fmt::Display for Error { Self::LiquidityFeeTooHigh => { write!(f, "The given operation failed due to the LSP's required opening fee being too high.") }, - Self::OpenAnnouncedChannelFailed => write!(f, "Failed to open an announced channel."), } } } diff --git a/src/lib.rs b/src/lib.rs index 5405dedec..ad377f959 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -100,6 +100,7 @@ mod wallet; pub use bip39; pub use bitcoin; pub use lightning; +use lightning::routing::gossip::NodeAlias; pub use lightning_invoice; pub use balance::{BalanceDetails, LightningBalance, PendingSweepBalance}; @@ -115,7 +116,6 @@ pub use io::utils::generate_entropy_mnemonic; #[cfg(feature = "uniffi")] use uniffi_types::*; -pub use builder::sanitize_alias; #[cfg(feature = "uniffi")] pub use builder::ArcedNodeBuilder as Builder; pub use builder::BuildError; @@ -123,8 +123,8 @@ pub use builder::BuildError; pub use builder::NodeBuilder as Builder; use config::{ - can_announce_channel, default_user_config, LDK_WALLET_SYNC_TIMEOUT_SECS, - NODE_ANN_BCAST_INTERVAL, PEER_RECONNECTION_INTERVAL, + can_announce_channel, default_user_config, ChannelAnnouncementStatus, + LDK_WALLET_SYNC_TIMEOUT_SECS, NODE_ANN_BCAST_INTERVAL, PEER_RECONNECTION_INTERVAL, RESOLVED_CHANNEL_MONITOR_ARCHIVAL_INTERVAL, RGS_SYNC_INTERVAL, WALLET_SYNC_INTERVAL_MINIMUM_SECS, }; @@ -648,20 +648,19 @@ impl Node { continue; } - let addresses = bcast_config.listening_addresses.clone().unwrap_or(Vec::new()); - let alias = node_alias.clone().map(|alias| { - alias.0 - }); - - if !can_announce_channel { - // Skip if we are not listening on any addresses or if the node alias is not set. - continue; - } - - if let Some(node_alias) = alias { - bcast_pm.broadcast_node_announcement([0; 3], node_alias, addresses); - } else { - continue + match can_announce_channel { + ChannelAnnouncementStatus::Unannounceable(_) => { + // Skip if we are not listening on any addresses or if the node alias is not set. + continue; + } + ChannelAnnouncementStatus::Announceable => { + let addresses = bcast_config.listening_addresses.clone().unwrap_or(Vec::new()); + if let Some(node_alias) = node_alias.as_ref() { + bcast_pm.broadcast_node_announcement([0; 3], node_alias.0, addresses); + } else { + continue + } + } } let unix_time_secs_opt = @@ -973,6 +972,11 @@ impl Node { self.config.listening_addresses.clone() } + /// Returns our node alias. + pub fn node_alias(&self) -> Option { + self.config.node_alias + } + /// Returns a payment handler allowing to create and pay [BOLT 11] invoices. /// /// [BOLT 11]: https://github.com/lightning/bolts/blob/master/11-payment-encoding.md @@ -1182,22 +1186,14 @@ impl Node { Ok(()) } - /// Connect to a node and open a new channel. Disconnects and re-connects are handled automatically - /// - /// Disconnects and reconnects are handled automatically. - /// - /// If `push_to_counterparty_msat` is set, the given value will be pushed (read: sent) to the - /// channel counterparty on channel open. This can be useful to start out with the balance not - /// entirely shifted to one side, therefore allowing to receive payments from the getgo. - /// - /// If Anchor channels are enabled, this will ensure the configured - /// [`AnchorChannelsConfig::per_channel_reserve_sats`] is available and will be retained before - /// opening the channel. + /// Connect to a node and open a new channel. /// - /// Returns a [`UserChannelId`] allowing to locally keep track of the channel. - fn connect_open_channel( + /// See [`Node::open_channel`] or [`Node::open_announced_channel`] for more information about + /// parameters. + fn open_channel_inner( &self, node_id: PublicKey, address: SocketAddress, channel_amount_sats: u64, push_to_counterparty_msat: Option, channel_config: Option, + announce_channel: bool, ) -> Result { let rt_lock = self.runtime.read().unwrap(); if rt_lock.is_none() { @@ -1259,13 +1255,12 @@ impl Node { } let mut user_config = default_user_config(&self.config); - let can_announce_channel = can_announce_channel(&self.config); - user_config.channel_handshake_config.announced_channel = can_announce_channel; + user_config.channel_handshake_config.announced_channel = announce_channel; user_config.channel_config = (channel_config.unwrap_or_default()).clone().into(); // We set the max inflight to 100% for private channels. // FIXME: LDK will default to this behavior soon, too, at which point we should drop this // manual override. - if !can_announce_channel { + if !announce_channel { user_config .channel_handshake_config .max_inbound_htlc_value_in_flight_percent_of_channel = 100; @@ -1298,36 +1293,79 @@ impl Node { } } - /// Opens a channel with a peer. + /// Connect to a node and open a new channel. + /// + /// Disconnects and reconnects are handled automatically. + /// + /// If `push_to_counterparty_msat` is set, the given value will be pushed (read: sent) to the + /// channel counterparty on channel open. This can be useful to start out with the balance not + /// entirely shifted to one side, therefore allowing to receive payments from the getgo. + /// + /// If Anchor channels are enabled, this will ensure the configured + /// [`AnchorChannelsConfig::per_channel_reserve_sats`] is available and will be retained before + /// opening the channel. + /// + /// Calls `Node::open_channel_inner` with `announce_channel` set to `false`. + /// + /// Returns a [`UserChannelId`] allowing to locally keep track of the channel. pub fn open_channel( &self, node_id: PublicKey, address: SocketAddress, channel_amount_sats: u64, push_to_counterparty_msat: Option, channel_config: Option, ) -> Result { - self.connect_open_channel( + self.open_channel_inner( node_id, address, channel_amount_sats, push_to_counterparty_msat, channel_config, + false, ) } - /// Opens an announced channel with a peer. + /// Connect to a node and open a new announced channel. + /// + /// Disconnects and reconnects are handled automatically. + /// + /// If `push_to_counterparty_msat` is set, the given value will be pushed (read: sent) to the + /// channel counterparty on channel open. This can be useful to start out with the balance not + /// entirely shifted to one side, therefore allowing to receive payments from the getgo. + /// + /// If Anchor channels are enabled, this will ensure the configured + /// [`AnchorChannelsConfig::per_channel_reserve_sats`] is available and will be retained before + /// opening the channel. + /// + /// Note that, regardless of the value of `announce_channel` passed, this function + /// checks that a node is configured to announce the channel to be openned and returns + /// an error if the configuration is wrong. Otherwise, calls `Node::open_channel_inner` + /// with `announced_channel` equals to `true`. + /// See `config::can_announce_channel` for more details. + /// + /// Returns a [`UserChannelId`] allowing to locally keep track of the channel. pub fn open_announced_channel( &self, node_id: PublicKey, address: SocketAddress, channel_amount_sats: u64, push_to_counterparty_msat: Option, channel_config: Option, ) -> Result { - if !can_announce_channel(&self.config) { - return Err(Error::OpenAnnouncedChannelFailed); + match can_announce_channel(&self.config) { + config::ChannelAnnouncementStatus::Announceable => self.open_channel_inner( + node_id, + address, + channel_amount_sats, + push_to_counterparty_msat, + channel_config, + true, + ), + config::ChannelAnnouncementStatus::Unannounceable(reason) => match reason { + config::ChannelAnnouncementBlocker::MissingNodeAlias => { + return Err(Error::InvalidNodeAlias) + }, + config::ChannelAnnouncementBlocker::MissingListeningAddresses => { + return Err(Error::InvalidSocketAddress) + }, + config::ChannelAnnouncementBlocker::EmptyListeningAddresses => { + return Err(Error::InvalidSocketAddress) + }, + }, } - - self.open_channel( - node_id, - address, - channel_amount_sats, - push_to_counterparty_msat, - channel_config, - ) } /// Manually sync the LDK and BDK wallets with the current chain state and update the fee rate diff --git a/tests/common/mod.rs b/tests/common/mod.rs index b616f7eb5..c0059b8f4 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -11,8 +11,7 @@ use ldk_node::io::sqlite_store::SqliteStore; use ldk_node::payment::{PaymentDirection, PaymentKind, PaymentStatus}; use ldk_node::{ - sanitize_alias, Builder, Config, Event, LightningBalance, LogLevel, Node, NodeError, - PendingSweepBalance, + Builder, Config, Event, LightningBalance, LogLevel, Node, NodeError, PendingSweepBalance, }; use lightning::ln::msgs::SocketAddress; @@ -207,7 +206,12 @@ pub(crate) fn random_node_alias() -> Option { let ranged_val = rng.gen_range(0..10); match ranged_val { 0 => None, - val => Some(sanitize_alias(&format!("ldk-node-{}", val)).unwrap()), + val => { + let alias = format!("ldk-node-{}", val); + let mut bytes = [0u8; 32]; + bytes[..alias.as_bytes().len()].copy_from_slice(alias.as_bytes()); + Some(NodeAlias(bytes)) + }, } } @@ -443,7 +447,7 @@ pub(crate) fn do_channel_full_cycle( assert_eq!(node_a.next_event(), None); assert_eq!(node_b.next_event(), None); - println!("\nA -- connect_open_channel -> B"); + println!("\nA -- open_channel -> B"); let funding_amount_sat = 2_080_000; let push_msat = (funding_amount_sat / 2) * 1000; // balance the channel node_a diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 89786f826..68d1effbb 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -80,7 +80,7 @@ fn channel_open_fails_when_funds_insufficient() { assert_eq!(node_a.list_balances().spendable_onchain_balance_sats, premine_amount_sat); assert_eq!(node_b.list_balances().spendable_onchain_balance_sats, premine_amount_sat); - println!("\nA -- connect_open_channel -> B"); + println!("\nA -- open_channel -> B"); assert_eq!( Err(NodeError::InsufficientFunds), node_a.open_channel( From 4fd1cb8ed98c5875627e31d9c7cbf3c5e0fe8d6b Mon Sep 17 00:00:00 2001 From: Enigbe Ochekliye Date: Thu, 26 Sep 2024 21:26:18 +0100 Subject: [PATCH 14/14] fix(test): Implement conditional channel opening based on aliases and addresses This commit addresses flaky test issues related to conditional channel opening between nodes, considering node aliases and listening addresses. Changes in test modules: - Add/modify helper functions to randomize channel announcement flags - Generate random node aliases based on announcement flags: * Set custom alias if announce_channel is true * Use default alias otherwise - Update channel opening logic to account for node and channel announcements --- src/lib.rs | 1 + tests/common/mod.rs | 101 ++++-- tests/integration_tests_cln.rs | 16 +- tests/integration_tests_rust.rs | 611 +++++++++++++++++--------------- tests/integration_tests_vss.rs | 6 +- 5 files changed, 419 insertions(+), 316 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ad377f959..48750f74e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -654,6 +654,7 @@ impl Node { continue; } ChannelAnnouncementStatus::Announceable => { + // Broadcast node announcement. let addresses = bcast_config.listening_addresses.clone().unwrap_or(Vec::new()); if let Some(node_alias) = node_alias.as_ref() { bcast_pm.broadcast_node_announcement([0; 3], node_alias.0, addresses); diff --git a/tests/common/mod.rs b/tests/common/mod.rs index c0059b8f4..4dcbfd999 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -204,24 +204,36 @@ pub(crate) fn random_listening_addresses() -> Vec { pub(crate) fn random_node_alias() -> Option { let mut rng = thread_rng(); let ranged_val = rng.gen_range(0..10); + + let alias = format!("ldk-node-{}", ranged_val); + let mut bytes = [0u8; 32]; + bytes[..alias.as_bytes().len()].copy_from_slice(alias.as_bytes()); + + Some(NodeAlias(bytes)) +} + +pub(crate) fn random_announce_channel() -> bool { + let mut rng = thread_rng(); + let ranged_val = rng.gen_range(0..=1); match ranged_val { - 0 => None, - val => { - let alias = format!("ldk-node-{}", val); - let mut bytes = [0u8; 32]; - bytes[..alias.as_bytes().len()].copy_from_slice(alias.as_bytes()); - Some(NodeAlias(bytes)) - }, + 0 => false, + _ => true, } } -pub(crate) fn random_config(anchor_channels: bool) -> Config { +pub(crate) fn random_config(anchor_channels: bool, announce_channel: bool) -> Config { let mut config = Config::default(); if !anchor_channels { config.anchor_channels_config = None; } + if announce_channel { + let alias = random_node_alias(); + println!("Setting random LDK node alias: {:?}", alias); + config.node_alias = alias; + } + config.network = Network::Regtest; config.onchain_wallet_sync_interval_secs = 100000; config.wallet_sync_interval_secs = 100000; @@ -235,10 +247,6 @@ pub(crate) fn random_config(anchor_channels: bool) -> Config { println!("Setting random LDK listening addresses: {:?}", rand_listening_addresses); config.listening_addresses = Some(rand_listening_addresses); - let alias = random_node_alias(); - println!("Setting random LDK node alias: {:?}", alias); - config.node_alias = alias; - config.log_level = LogLevel::Gossip; config @@ -261,14 +269,15 @@ macro_rules! setup_builder { pub(crate) use setup_builder; pub(crate) fn setup_two_nodes( - electrsd: &ElectrsD, allow_0conf: bool, anchor_channels: bool, anchors_trusted_no_reserve: bool, + electrsd: &ElectrsD, allow_0conf: bool, anchor_channels: bool, + anchors_trusted_no_reserve: bool, announce_channel: bool, ) -> (TestNode, TestNode) { println!("== Node A =="); - let config_a = random_config(anchor_channels); + let config_a = random_config(anchor_channels, announce_channel); let node_a = setup_node(electrsd, config_a); println!("\n== Node B =="); - let mut config_b = random_config(anchor_channels); + let mut config_b = random_config(anchor_channels, announce_channel); if allow_0conf { config_b.trusted_peers_0conf.push(node_a.node_id()); } @@ -406,15 +415,28 @@ pub(crate) fn premine_and_distribute_funds( pub fn open_channel( node_a: &TestNode, node_b: &TestNode, funding_amount_sat: u64, electrsd: &ElectrsD, ) { - node_a - .open_announced_channel( - node_b.node_id(), - node_b.listening_addresses().unwrap().first().unwrap().clone(), - funding_amount_sat, - None, - None, - ) - .unwrap(); + if node_a.config().node_alias.is_some() { + node_a + .open_announced_channel( + node_b.node_id(), + node_b.listening_addresses().unwrap().first().unwrap().clone(), + funding_amount_sat, + None, + None, + ) + .unwrap(); + } else { + node_a + .open_channel( + node_b.node_id(), + node_b.listening_addresses().unwrap().first().unwrap().clone(), + funding_amount_sat, + None, + None, + ) + .unwrap(); + } + assert!(node_a.list_peers().iter().find(|c| { c.node_id == node_b.node_id() }).is_some()); let funding_txo_a = expect_channel_pending_event!(node_a, node_b.node_id()); @@ -450,15 +472,28 @@ pub(crate) fn do_channel_full_cycle( println!("\nA -- open_channel -> B"); let funding_amount_sat = 2_080_000; let push_msat = (funding_amount_sat / 2) * 1000; // balance the channel - node_a - .open_channel( - node_b.node_id(), - node_b.listening_addresses().unwrap().first().unwrap().clone(), - funding_amount_sat, - Some(push_msat), - None, - ) - .unwrap(); + + if node_a.config().node_alias.is_some() { + node_a + .open_announced_channel( + node_b.node_id(), + node_b.listening_addresses().unwrap().first().unwrap().clone(), + funding_amount_sat, + Some(push_msat), + None, + ) + .unwrap(); + } else { + node_a + .open_channel( + node_b.node_id(), + node_b.listening_addresses().unwrap().first().unwrap().clone(), + funding_amount_sat, + Some(push_msat), + None, + ) + .unwrap(); + } assert_eq!(node_a.list_peers().first().unwrap().node_id, node_b.node_id()); assert!(node_a.list_peers().first().unwrap().is_persisted); diff --git a/tests/integration_tests_cln.rs b/tests/integration_tests_cln.rs index 13b5c44c6..11065bfe6 100644 --- a/tests/integration_tests_cln.rs +++ b/tests/integration_tests_cln.rs @@ -9,6 +9,7 @@ mod common; +use common::random_announce_channel; use ldk_node::bitcoin::secp256k1::PublicKey; use ldk_node::bitcoin::Amount; use ldk_node::lightning::ln::msgs::SocketAddress; @@ -43,7 +44,7 @@ fn test_cln() { common::generate_blocks_and_wait(&bitcoind_client, &electrs_client, 1); // Setup LDK Node - let config = common::random_config(true); + let config = common::random_config(true, random_announce_channel()); let mut builder = Builder::from_config(config); builder.set_esplora_server("http://127.0.0.1:3002".to_string()); @@ -82,8 +83,19 @@ fn test_cln() { // Open the channel let funding_amount_sat = 1_000_000; - node.open_channel(cln_node_id, cln_address, funding_amount_sat, Some(500_000_000), None) + if node.config().node_alias.is_none() { + node.open_channel(cln_node_id, cln_address, funding_amount_sat, Some(500_000_000), None) + .unwrap(); + } else { + node.open_announced_channel( + cln_node_id, + cln_address, + funding_amount_sat, + Some(500_000_000), + None, + ) .unwrap(); + } let funding_txo = common::expect_channel_pending_event!(node, cln_node_id); common::wait_for_tx(&electrs_client, funding_txo.txid); diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 68d1effbb..9c244e90a 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -10,8 +10,9 @@ mod common; use common::{ do_channel_full_cycle, expect_channel_ready_event, expect_event, expect_payment_received_event, expect_payment_successful_event, generate_blocks_and_wait, open_channel, - premine_and_distribute_funds, random_config, setup_bitcoind_and_electrsd, setup_builder, - setup_node, setup_two_nodes, wait_for_tx, TestSyncStore, + premine_and_distribute_funds, random_announce_channel, random_config, + setup_bitcoind_and_electrsd, setup_builder, setup_node, setup_two_nodes, wait_for_tx, + TestSyncStore, }; use ldk_node::payment::{PaymentKind, QrPaymentResult, SendingParameters}; @@ -27,42 +28,46 @@ use std::sync::Arc; #[test] fn channel_full_cycle() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); - let (node_a, node_b) = setup_two_nodes(&electrsd, false, true, false); + let (node_a, node_b) = + setup_two_nodes(&electrsd, false, true, false, random_announce_channel()); do_channel_full_cycle(node_a, node_b, &bitcoind.client, &electrsd.client, false, true, false); } #[test] fn channel_full_cycle_force_close() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); - let (node_a, node_b) = setup_two_nodes(&electrsd, false, true, false); + let (node_a, node_b) = + setup_two_nodes(&electrsd, false, true, false, random_announce_channel()); do_channel_full_cycle(node_a, node_b, &bitcoind.client, &electrsd.client, false, true, true); } #[test] fn channel_full_cycle_force_close_trusted_no_reserve() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); - let (node_a, node_b) = setup_two_nodes(&electrsd, false, true, true); + let (node_a, node_b) = setup_two_nodes(&electrsd, false, true, true, random_announce_channel()); do_channel_full_cycle(node_a, node_b, &bitcoind.client, &electrsd.client, false, true, true); } #[test] fn channel_full_cycle_0conf() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); - let (node_a, node_b) = setup_two_nodes(&electrsd, true, true, false); + let (node_a, node_b) = setup_two_nodes(&electrsd, true, true, false, random_announce_channel()); do_channel_full_cycle(node_a, node_b, &bitcoind.client, &electrsd.client, true, true, false) } #[test] fn channel_full_cycle_legacy_staticremotekey() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); - let (node_a, node_b) = setup_two_nodes(&electrsd, false, false, false); + let (node_a, node_b) = + setup_two_nodes(&electrsd, false, false, false, random_announce_channel()); do_channel_full_cycle(node_a, node_b, &bitcoind.client, &electrsd.client, false, false, false); } #[test] fn channel_open_fails_when_funds_insufficient() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); - let (node_a, node_b) = setup_two_nodes(&electrsd, false, true, false); + let (node_a, node_b) = + setup_two_nodes(&electrsd, false, true, false, random_announce_channel()); let addr_a = node_a.onchain_payment().new_address().unwrap(); let addr_b = node_b.onchain_payment().new_address().unwrap(); @@ -83,13 +88,23 @@ fn channel_open_fails_when_funds_insufficient() { println!("\nA -- open_channel -> B"); assert_eq!( Err(NodeError::InsufficientFunds), - node_a.open_channel( - node_b.node_id(), - node_b.listening_addresses().unwrap().first().unwrap().clone(), - 120000, - None, - None, - ) + if node_a.config().node_alias.is_some() { + node_a.open_announced_channel( + node_b.node_id(), + node_b.listening_addresses().unwrap().first().unwrap().clone(), + 120000, + None, + None, + ) + } else { + node_a.open_channel( + node_b.node_id(), + node_b.listening_addresses().unwrap().first().unwrap().clone(), + 120000, + None, + None, + ) + } ); } @@ -100,8 +115,9 @@ fn multi_hop_sending() { // Setup and fund 5 nodes let mut nodes = Vec::new(); + let announce_channel = random_announce_channel(); for _ in 0..5 { - let config = random_config(true); + let config = random_config(true, announce_channel); setup_builder!(builder, config); builder.set_esplora_server(esplora_url.clone()); let node = builder.build().unwrap(); @@ -170,16 +186,22 @@ fn multi_hop_sending() { }; let invoice = nodes[4].bolt11_payment().receive(2_500_000, &"asdf", 9217).unwrap(); - nodes[0].bolt11_payment().send(&invoice, Some(sending_params)).unwrap(); + let send_res = nodes[0].bolt11_payment().send(&invoice, Some(sending_params)); - let payment_id = expect_payment_received_event!(&nodes[4], 2_500_000); - let fee_paid_msat = Some(2000); - expect_payment_successful_event!(nodes[0], payment_id, Some(fee_paid_msat)); + // N0 cannot find a route to N4 if node and channel is unannounced. + if nodes[0].config().node_alias.is_none() { + assert_eq!(send_res, Err(NodeError::PaymentSendingFailed)) + } else { + let payment_id = expect_payment_received_event!(&nodes[4], 2_500_000); + assert_eq!(send_res.unwrap(), payment_id.unwrap()); + let fee_paid_msat = Some(2000); + expect_payment_successful_event!(nodes[0], payment_id, Some(fee_paid_msat)); + } } #[test] fn connect_to_public_testnet_esplora() { - let mut config = random_config(true); + let mut config = random_config(true, random_announce_channel()); config.network = Network::Testnet; setup_builder!(builder, config); builder.set_esplora_server("https://blockstream.info/testnet/api".to_string()); @@ -191,7 +213,7 @@ fn connect_to_public_testnet_esplora() { #[test] fn start_stop_reinit() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); - let config = random_config(true); + let config = random_config(true, random_announce_channel()); let esplora_url = format!("http://{}", electrsd.esplora_url.as_ref().unwrap()); @@ -259,7 +281,8 @@ fn start_stop_reinit() { #[test] fn onchain_spend_receive() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); - let (node_a, node_b) = setup_two_nodes(&electrsd, false, true, false); + let (node_a, node_b) = + setup_two_nodes(&electrsd, false, true, false, random_announce_channel()); let addr_a = node_a.onchain_payment().new_address().unwrap(); let addr_b = node_b.onchain_payment().new_address().unwrap(); @@ -307,7 +330,7 @@ fn onchain_spend_receive() { #[test] fn sign_verify_msg() { let (_bitcoind, electrsd) = setup_bitcoind_and_electrsd(); - let config = random_config(true); + let config = random_config(true, random_announce_channel()); let node = setup_node(&electrsd, config); // Tests arbitrary message signing and later verification @@ -325,7 +348,8 @@ fn connection_restart_behavior() { fn do_connection_restart_behavior(persist: bool) { let (_bitcoind, electrsd) = setup_bitcoind_and_electrsd(); - let (node_a, node_b) = setup_two_nodes(&electrsd, false, false, false); + let (node_a, node_b) = + setup_two_nodes(&electrsd, false, false, false, random_announce_channel()); let node_id_a = node_a.node_id(); let node_id_b = node_b.node_id(); @@ -376,7 +400,8 @@ fn do_connection_restart_behavior(persist: bool) { #[test] fn concurrent_connections_succeed() { let (_bitcoind, electrsd) = setup_bitcoind_and_electrsd(); - let (node_a, node_b) = setup_two_nodes(&electrsd, false, true, false); + let (node_a, node_b) = + setup_two_nodes(&electrsd, false, true, false, random_announce_channel()); let node_a = Arc::new(node_a); let node_b = Arc::new(node_b); @@ -406,7 +431,8 @@ fn concurrent_connections_succeed() { #[test] fn simple_bolt12_send_receive() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); - let (node_a, node_b) = setup_two_nodes(&electrsd, false, true, false); + let (node_a, node_b) = + setup_two_nodes(&electrsd, false, true, false, random_announce_channel()); let address_a = node_a.onchain_payment().new_address().unwrap(); let premine_amount_sat = 5_000_000; @@ -428,191 +454,203 @@ fn simple_bolt12_send_receive() { expect_channel_ready_event!(node_a, node_b.node_id()); expect_channel_ready_event!(node_b, node_a.node_id()); - // Sleep until we broadcasted a node announcement. - while node_b.status().latest_node_announcement_broadcast_timestamp.is_none() { - std::thread::sleep(std::time::Duration::from_millis(10)); + // For announced nodes, we make a trivial check that node_alias is set knowing that + // `random_listening_addresses()` always sets the listening_addresses. This check + // is important to prevent the test from looping endlessly. + if node_a.config().node_alias.is_some() && node_b.config().node_alias.is_some() { + // Sleep until we broadcasted a node announcement. + while node_b.status().latest_node_announcement_broadcast_timestamp.is_none() { + std::thread::sleep(std::time::Duration::from_millis(10)); + } + + // Sleep one more sec to make sure the node announcement propagates. + std::thread::sleep(std::time::Duration::from_secs(1)); } - // Sleep one more sec to make sure the node announcement propagates. - std::thread::sleep(std::time::Duration::from_secs(1)); - let expected_amount_msat = 100_000_000; - let offer = node_b.bolt12_payment().receive(expected_amount_msat, "asdf", Some(1)).unwrap(); - let expected_quantity = Some(1); - let expected_payer_note = Some("Test".to_string()); - let payment_id = node_a - .bolt12_payment() - .send(&offer, expected_quantity, expected_payer_note.clone()) - .unwrap(); - - expect_payment_successful_event!(node_a, Some(payment_id), None); - let node_a_payments = node_a.list_payments(); - assert_eq!(node_a_payments.len(), 1); - match node_a_payments.first().unwrap().kind { - PaymentKind::Bolt12Offer { - hash, - preimage, - secret: _, - offer_id, - quantity: ref qty, - payer_note: ref note, - } => { - assert!(hash.is_some()); - assert!(preimage.is_some()); - assert_eq!(offer_id, offer.id()); - assert_eq!(&expected_quantity, qty); - assert_eq!(expected_payer_note.unwrap(), note.clone().unwrap().0); - //TODO: We should eventually set and assert the secret sender-side, too, but the BOLT12 - //API currently doesn't allow to do that. - }, - _ => { - panic!("Unexpected payment kind"); - }, - } - assert_eq!(node_a_payments.first().unwrap().amount_msat, Some(expected_amount_msat)); - - expect_payment_received_event!(node_b, expected_amount_msat); - let node_b_payments = node_b.list_payments(); - assert_eq!(node_b_payments.len(), 1); - match node_b_payments.first().unwrap().kind { - PaymentKind::Bolt12Offer { hash, preimage, secret, offer_id, .. } => { - assert!(hash.is_some()); - assert!(preimage.is_some()); - assert!(secret.is_some()); - assert_eq!(offer_id, offer.id()); - }, - _ => { - panic!("Unexpected payment kind"); - }, - } - assert_eq!(node_b_payments.first().unwrap().amount_msat, Some(expected_amount_msat)); - - // Test send_using_amount - let offer_amount_msat = 100_000_000; - let less_than_offer_amount = offer_amount_msat - 10_000; - let expected_amount_msat = offer_amount_msat + 10_000; - let offer = node_b.bolt12_payment().receive(offer_amount_msat, "asdf", Some(1)).unwrap(); - let expected_quantity = Some(1); - let expected_payer_note = Some("Test".to_string()); - assert!(node_a - .bolt12_payment() - .send_using_amount(&offer, less_than_offer_amount, None, None) - .is_err()); - let payment_id = node_a - .bolt12_payment() - .send_using_amount( - &offer, - expected_amount_msat, - expected_quantity, - expected_payer_note.clone(), - ) - .unwrap(); - - expect_payment_successful_event!(node_a, Some(payment_id), None); - let node_a_payments = node_a.list_payments_with_filter(|p| p.id == payment_id); - assert_eq!(node_a_payments.len(), 1); - let payment_hash = match node_a_payments.first().unwrap().kind { - PaymentKind::Bolt12Offer { - hash, - preimage, - secret: _, - offer_id, - quantity: ref qty, - payer_note: ref note, - } => { - assert!(hash.is_some()); - assert!(preimage.is_some()); - assert_eq!(offer_id, offer.id()); - assert_eq!(&expected_quantity, qty); - assert_eq!(expected_payer_note.unwrap(), note.clone().unwrap().0); - //TODO: We should eventually set and assert the secret sender-side, too, but the BOLT12 - //API currently doesn't allow to do that. - hash.unwrap() - }, - _ => { - panic!("Unexpected payment kind"); - }, - }; - assert_eq!(node_a_payments.first().unwrap().amount_msat, Some(expected_amount_msat)); - - expect_payment_received_event!(node_b, expected_amount_msat); - let node_b_payment_id = PaymentId(payment_hash.0); - let node_b_payments = node_b.list_payments_with_filter(|p| p.id == node_b_payment_id); - assert_eq!(node_b_payments.len(), 1); - match node_b_payments.first().unwrap().kind { - PaymentKind::Bolt12Offer { hash, preimage, secret, offer_id, .. } => { - assert!(hash.is_some()); - assert!(preimage.is_some()); - assert!(secret.is_some()); - assert_eq!(offer_id, offer.id()); - }, - _ => { - panic!("Unexpected payment kind"); - }, - } - assert_eq!(node_b_payments.first().unwrap().amount_msat, Some(expected_amount_msat)); - - // Now node_b refunds the amount node_a just overpaid. - let overpaid_amount = expected_amount_msat - offer_amount_msat; - let expected_quantity = Some(1); - let expected_payer_note = Some("Test".to_string()); - let refund = node_b - .bolt12_payment() - .initiate_refund(overpaid_amount, 3600, expected_quantity, expected_payer_note.clone()) - .unwrap(); - let invoice = node_a.bolt12_payment().request_refund_payment(&refund).unwrap(); - expect_payment_received_event!(node_a, overpaid_amount); - - let node_b_payment_id = node_b - .list_payments_with_filter(|p| p.amount_msat == Some(overpaid_amount)) - .first() - .unwrap() - .id; - expect_payment_successful_event!(node_b, Some(node_b_payment_id), None); - - let node_b_payments = node_b.list_payments_with_filter(|p| p.id == node_b_payment_id); - assert_eq!(node_b_payments.len(), 1); - match node_b_payments.first().unwrap().kind { - PaymentKind::Bolt12Refund { - hash, - preimage, - secret: _, - quantity: ref qty, - payer_note: ref note, - } => { - assert!(hash.is_some()); - assert!(preimage.is_some()); - assert_eq!(&expected_quantity, qty); - assert_eq!(expected_payer_note.unwrap(), note.clone().unwrap().0) - //TODO: We should eventually set and assert the secret sender-side, too, but the BOLT12 - //API currently doesn't allow to do that. - }, - _ => { - panic!("Unexpected payment kind"); - }, - } - assert_eq!(node_b_payments.first().unwrap().amount_msat, Some(overpaid_amount)); - - let node_a_payment_id = PaymentId(invoice.payment_hash().0); - let node_a_payments = node_a.list_payments_with_filter(|p| p.id == node_a_payment_id); - assert_eq!(node_a_payments.len(), 1); - match node_a_payments.first().unwrap().kind { - PaymentKind::Bolt12Refund { hash, preimage, secret, .. } => { - assert!(hash.is_some()); - assert!(preimage.is_some()); - assert!(secret.is_some()); - }, - _ => { - panic!("Unexpected payment kind"); - }, + let offer_res = node_b.bolt12_payment().receive(expected_amount_msat, "asdf", Some(1)); + if node_a.config().node_alias.is_none() && node_b.config().node_alias.is_none() { + // Node must be announced if alternative one-hop `BlindedPath` is to be used. + assert_eq!(offer_res, Err(NodeError::OfferCreationFailed)) + } else { + let offer = offer_res.unwrap(); + let expected_quantity = Some(1); + let expected_payer_note = Some("Test".to_string()); + let payment_id = node_a + .bolt12_payment() + .send(&offer, expected_quantity, expected_payer_note.clone()) + .unwrap(); + + expect_payment_successful_event!(node_a, Some(payment_id), None); + let node_a_payments = node_a.list_payments(); + assert_eq!(node_a_payments.len(), 1); + match node_a_payments.first().unwrap().kind { + PaymentKind::Bolt12Offer { + hash, + preimage, + secret: _, + offer_id, + quantity: ref qty, + payer_note: ref note, + } => { + assert!(hash.is_some()); + assert!(preimage.is_some()); + assert_eq!(offer_id, offer.id()); + assert_eq!(&expected_quantity, qty); + assert_eq!(expected_payer_note.unwrap(), note.clone().unwrap().0); + //TODO: We should eventually set and assert the secret sender-side, too, but the BOLT12 + //API currently doesn't allow to do that. + }, + _ => { + panic!("Unexpected payment kind"); + }, + } + assert_eq!(node_a_payments.first().unwrap().amount_msat, Some(expected_amount_msat)); + + expect_payment_received_event!(node_b, expected_amount_msat); + let node_b_payments = node_b.list_payments(); + assert_eq!(node_b_payments.len(), 1); + match node_b_payments.first().unwrap().kind { + PaymentKind::Bolt12Offer { hash, preimage, secret, offer_id, .. } => { + assert!(hash.is_some()); + assert!(preimage.is_some()); + assert!(secret.is_some()); + assert_eq!(offer_id, offer.id()); + }, + _ => { + panic!("Unexpected payment kind"); + }, + } + assert_eq!(node_b_payments.first().unwrap().amount_msat, Some(expected_amount_msat)); + + // Test send_using_amount + let offer_amount_msat = 100_000_000; + let less_than_offer_amount = offer_amount_msat - 10_000; + let expected_amount_msat = offer_amount_msat + 10_000; + let offer = node_b.bolt12_payment().receive(offer_amount_msat, "asdf", Some(1)).unwrap(); + let expected_quantity = Some(1); + let expected_payer_note = Some("Test".to_string()); + assert!(node_a + .bolt12_payment() + .send_using_amount(&offer, less_than_offer_amount, None, None) + .is_err()); + let payment_id = node_a + .bolt12_payment() + .send_using_amount( + &offer, + expected_amount_msat, + expected_quantity, + expected_payer_note.clone(), + ) + .unwrap(); + + expect_payment_successful_event!(node_a, Some(payment_id), None); + let node_a_payments = node_a.list_payments_with_filter(|p| p.id == payment_id); + assert_eq!(node_a_payments.len(), 1); + let payment_hash = match node_a_payments.first().unwrap().kind { + PaymentKind::Bolt12Offer { + hash, + preimage, + secret: _, + offer_id, + quantity: ref qty, + payer_note: ref note, + } => { + assert!(hash.is_some()); + assert!(preimage.is_some()); + assert_eq!(offer_id, offer.id()); + assert_eq!(&expected_quantity, qty); + assert_eq!(expected_payer_note.unwrap(), note.clone().unwrap().0); + //TODO: We should eventually set and assert the secret sender-side, too, but the BOLT12 + //API currently doesn't allow to do that. + hash.unwrap() + }, + _ => { + panic!("Unexpected payment kind"); + }, + }; + assert_eq!(node_a_payments.first().unwrap().amount_msat, Some(expected_amount_msat)); + + expect_payment_received_event!(node_b, expected_amount_msat); + let node_b_payment_id = PaymentId(payment_hash.0); + let node_b_payments = node_b.list_payments_with_filter(|p| p.id == node_b_payment_id); + assert_eq!(node_b_payments.len(), 1); + match node_b_payments.first().unwrap().kind { + PaymentKind::Bolt12Offer { hash, preimage, secret, offer_id, .. } => { + assert!(hash.is_some()); + assert!(preimage.is_some()); + assert!(secret.is_some()); + assert_eq!(offer_id, offer.id()); + }, + _ => { + panic!("Unexpected payment kind"); + }, + } + assert_eq!(node_b_payments.first().unwrap().amount_msat, Some(expected_amount_msat)); + + // Now node_b refunds the amount node_a just overpaid. + let overpaid_amount = expected_amount_msat - offer_amount_msat; + let expected_quantity = Some(1); + let expected_payer_note = Some("Test".to_string()); + let refund = node_b + .bolt12_payment() + .initiate_refund(overpaid_amount, 3600, expected_quantity, expected_payer_note.clone()) + .unwrap(); + let invoice = node_a.bolt12_payment().request_refund_payment(&refund).unwrap(); + expect_payment_received_event!(node_a, overpaid_amount); + + let node_b_payment_id = node_b + .list_payments_with_filter(|p| p.amount_msat == Some(overpaid_amount)) + .first() + .unwrap() + .id; + expect_payment_successful_event!(node_b, Some(node_b_payment_id), None); + + let node_b_payments = node_b.list_payments_with_filter(|p| p.id == node_b_payment_id); + assert_eq!(node_b_payments.len(), 1); + match node_b_payments.first().unwrap().kind { + PaymentKind::Bolt12Refund { + hash, + preimage, + secret: _, + quantity: ref qty, + payer_note: ref note, + } => { + assert!(hash.is_some()); + assert!(preimage.is_some()); + assert_eq!(&expected_quantity, qty); + assert_eq!(expected_payer_note.unwrap(), note.clone().unwrap().0) + //TODO: We should eventually set and assert the secret sender-side, too, but the BOLT12 + //API currently doesn't allow to do that. + }, + _ => { + panic!("Unexpected payment kind"); + }, + } + assert_eq!(node_b_payments.first().unwrap().amount_msat, Some(overpaid_amount)); + + let node_a_payment_id = PaymentId(invoice.payment_hash().0); + let node_a_payments = node_a.list_payments_with_filter(|p| p.id == node_a_payment_id); + assert_eq!(node_a_payments.len(), 1); + match node_a_payments.first().unwrap().kind { + PaymentKind::Bolt12Refund { hash, preimage, secret, .. } => { + assert!(hash.is_some()); + assert!(preimage.is_some()); + assert!(secret.is_some()); + }, + _ => { + panic!("Unexpected payment kind"); + }, + } + assert_eq!(node_a_payments.first().unwrap().amount_msat, Some(overpaid_amount)); } - assert_eq!(node_a_payments.first().unwrap().amount_msat, Some(overpaid_amount)); } #[test] fn generate_bip21_uri() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); - let (node_a, node_b) = setup_two_nodes(&electrsd, false, true, false); + let (node_a, node_b) = + setup_two_nodes(&electrsd, false, true, false, random_announce_channel()); let address_a = node_a.onchain_payment().new_address().unwrap(); let premined_sats = 5_000_000; @@ -639,21 +677,26 @@ fn generate_bip21_uri() { let uqr_payment = node_b.unified_qr_payment().receive(expected_amount_sats, "asdf", expiry_sec); - match uqr_payment.clone() { - Ok(ref uri) => { - println!("Generated URI: {}", uri); - assert!(uri.contains("BITCOIN:")); - assert!(uri.contains("lightning=")); - assert!(uri.contains("lno=")); - }, - Err(e) => panic!("Failed to generate URI: {:?}", e), + if node_a.config().node_alias.is_none() && node_b.config().node_alias.is_none() { + assert_eq!(uqr_payment, Err(NodeError::OfferCreationFailed)); + } else { + match uqr_payment.clone() { + Ok(ref uri) => { + println!("Generated URI: {}", uri); + assert!(uri.contains("BITCOIN:")); + assert!(uri.contains("lightning=")); + assert!(uri.contains("lno=")); + }, + Err(e) => panic!("Failed to generate URI: {:?}", e), + } } } #[test] fn unified_qr_send_receive() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); - let (node_a, node_b) = setup_two_nodes(&electrsd, false, true, false); + let (node_a, node_b) = + setup_two_nodes(&electrsd, false, true, false, random_announce_channel()); let address_a = node_a.onchain_payment().new_address().unwrap(); let premined_sats = 5_000_000; @@ -675,88 +718,98 @@ fn unified_qr_send_receive() { expect_channel_ready_event!(node_a, node_b.node_id()); expect_channel_ready_event!(node_b, node_a.node_id()); - // Sleep until we broadcast a node announcement. - while node_b.status().latest_node_announcement_broadcast_timestamp.is_none() { - std::thread::sleep(std::time::Duration::from_millis(10)); + // For announced nodes, we make a trivial check that node_alias is set knowing that + // `random_listening_addresses()` always sets the listening_addresses. This check + // is important to prevent the test from looping endlessly. + if node_a.config().node_alias.is_some() && node_b.config().node_alias.is_some() { + // Sleep until we broadcasted a node announcement. + while node_b.status().latest_node_announcement_broadcast_timestamp.is_none() { + std::thread::sleep(std::time::Duration::from_millis(10)); + } + + // Sleep one more sec to make sure the node announcement propagates. + std::thread::sleep(std::time::Duration::from_secs(1)); } - // Sleep one more sec to make sure the node announcement propagates. - std::thread::sleep(std::time::Duration::from_secs(1)); - let expected_amount_sats = 100_000; let expiry_sec = 4_000; let uqr_payment = node_b.unified_qr_payment().receive(expected_amount_sats, "asdf", expiry_sec); - let uri_str = uqr_payment.clone().unwrap(); - let offer_payment_id: PaymentId = match node_a.unified_qr_payment().send(&uri_str) { - Ok(QrPaymentResult::Bolt12 { payment_id }) => { - println!("\nBolt12 payment sent successfully with PaymentID: {:?}", payment_id); - payment_id - }, - Ok(QrPaymentResult::Bolt11 { payment_id: _ }) => { - panic!("Expected Bolt12 payment but got Bolt11"); - }, - Ok(QrPaymentResult::Onchain { txid: _ }) => { - panic!("Expected Bolt12 payment but get On-chain transaction"); - }, - Err(e) => { - panic!("Expected Bolt12 payment but got error: {:?}", e); - }, - }; - - expect_payment_successful_event!(node_a, Some(offer_payment_id), None); + if node_a.config().node_alias.is_none() && node_b.config().node_alias.is_none() { + // Node must be announced if alternative one-hop `BlindedPath` is to be used used. + assert_eq!(uqr_payment, Err(NodeError::OfferCreationFailed)) + } else { + let uri_str = uqr_payment.clone().unwrap(); + let offer_payment_id: PaymentId = match node_a.unified_qr_payment().send(&uri_str) { + Ok(QrPaymentResult::Bolt12 { payment_id }) => { + println!("\nBolt12 payment sent successfully with PaymentID: {:?}", payment_id); + payment_id + }, + Ok(QrPaymentResult::Bolt11 { payment_id: _ }) => { + panic!("Expected Bolt12 payment but got Bolt11"); + }, + Ok(QrPaymentResult::Onchain { txid: _ }) => { + panic!("Expected Bolt12 payment but get On-chain transaction"); + }, + Err(e) => { + panic!("Expected Bolt12 payment but got error: {:?}", e); + }, + }; - // Removed one character from the offer to fall back on to invoice. - // Still needs work - let uri_str_with_invalid_offer = &uri_str[..uri_str.len() - 1]; - let invoice_payment_id: PaymentId = - match node_a.unified_qr_payment().send(uri_str_with_invalid_offer) { + expect_payment_successful_event!(node_a, Some(offer_payment_id), None); + + // Removed one character from the offer to fall back on to invoice. + // Still needs work + let uri_str_with_invalid_offer = &uri_str[..uri_str.len() - 1]; + let invoice_payment_id: PaymentId = + match node_a.unified_qr_payment().send(uri_str_with_invalid_offer) { + Ok(QrPaymentResult::Bolt12 { payment_id: _ }) => { + panic!("Expected Bolt11 payment but got Bolt12"); + }, + Ok(QrPaymentResult::Bolt11 { payment_id }) => { + println!("\nBolt11 payment sent successfully with PaymentID: {:?}", payment_id); + payment_id + }, + Ok(QrPaymentResult::Onchain { txid: _ }) => { + panic!("Expected Bolt11 payment but got on-chain transaction"); + }, + Err(e) => { + panic!("Expected Bolt11 payment but got error: {:?}", e); + }, + }; + expect_payment_successful_event!(node_a, Some(invoice_payment_id), None); + + let expect_onchain_amount_sats = 800_000; + let onchain_uqr_payment = + node_b.unified_qr_payment().receive(expect_onchain_amount_sats, "asdf", 4_000).unwrap(); + + // Removed a character from the offer, so it would move on to the other parameters. + let txid = match node_a + .unified_qr_payment() + .send(&onchain_uqr_payment.as_str()[..onchain_uqr_payment.len() - 1]) + { Ok(QrPaymentResult::Bolt12 { payment_id: _ }) => { - panic!("Expected Bolt11 payment but got Bolt12"); + panic!("Expected on-chain payment but got Bolt12") }, - Ok(QrPaymentResult::Bolt11 { payment_id }) => { - println!("\nBolt11 payment sent successfully with PaymentID: {:?}", payment_id); - payment_id + Ok(QrPaymentResult::Bolt11 { payment_id: _ }) => { + panic!("Expected on-chain payment but got Bolt11"); }, - Ok(QrPaymentResult::Onchain { txid: _ }) => { - panic!("Expected Bolt11 payment but got on-chain transaction"); + Ok(QrPaymentResult::Onchain { txid }) => { + println!("\nOn-chain transaction successful with Txid: {}", txid); + txid }, Err(e) => { - panic!("Expected Bolt11 payment but got error: {:?}", e); + panic!("Expected on-chain payment but got error: {:?}", e); }, }; - expect_payment_successful_event!(node_a, Some(invoice_payment_id), None); - - let expect_onchain_amount_sats = 800_000; - let onchain_uqr_payment = - node_b.unified_qr_payment().receive(expect_onchain_amount_sats, "asdf", 4_000).unwrap(); - - // Removed a character from the offer, so it would move on to the other parameters. - let txid = match node_a - .unified_qr_payment() - .send(&onchain_uqr_payment.as_str()[..onchain_uqr_payment.len() - 1]) - { - Ok(QrPaymentResult::Bolt12 { payment_id: _ }) => { - panic!("Expected on-chain payment but got Bolt12") - }, - Ok(QrPaymentResult::Bolt11 { payment_id: _ }) => { - panic!("Expected on-chain payment but got Bolt11"); - }, - Ok(QrPaymentResult::Onchain { txid }) => { - println!("\nOn-chain transaction successful with Txid: {}", txid); - txid - }, - Err(e) => { - panic!("Expected on-chain payment but got error: {:?}", e); - }, - }; - generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6); - wait_for_tx(&electrsd.client, txid); + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6); + wait_for_tx(&electrsd.client, txid); - node_a.sync_wallets().unwrap(); - node_b.sync_wallets().unwrap(); + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); - assert_eq!(node_b.list_balances().total_onchain_balance_sats, 800_000); - assert_eq!(node_b.list_balances().total_lightning_balance_sats, 200_000); + assert_eq!(node_b.list_balances().total_onchain_balance_sats, 800_000); + assert_eq!(node_b.list_balances().total_lightning_balance_sats, 200_000); + } } diff --git a/tests/integration_tests_vss.rs b/tests/integration_tests_vss.rs index c572fbcd8..b7fc8ba42 100644 --- a/tests/integration_tests_vss.rs +++ b/tests/integration_tests_vss.rs @@ -9,6 +9,7 @@ mod common; +use common::random_announce_channel; use ldk_node::Builder; #[test] @@ -16,7 +17,8 @@ fn channel_full_cycle_with_vss_store() { let (bitcoind, electrsd) = common::setup_bitcoind_and_electrsd(); println!("== Node A =="); let esplora_url = format!("http://{}", electrsd.esplora_url.as_ref().unwrap()); - let config_a = common::random_config(true); + let announce_channel = random_announce_channel(); + let config_a = common::random_config(true, announce_channel); let mut builder_a = Builder::from_config(config_a); builder_a.set_esplora_server(esplora_url.clone()); let vss_base_url = std::env::var("TEST_VSS_BASE_URL").unwrap(); @@ -25,7 +27,7 @@ fn channel_full_cycle_with_vss_store() { node_a.start().unwrap(); println!("\n== Node B =="); - let config_b = common::random_config(true); + let config_b = common::random_config(true, announce_channel); let mut builder_b = Builder::from_config(config_b); builder_b.set_esplora_server(esplora_url); let node_b = builder_b.build_with_vss_store(vss_base_url, "node_2_store".to_string()).unwrap();