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

Author: enigbe

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 {

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<Vec<SocketAddress>>,
 	/// 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,11 +298,50 @@ 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;
 	}
 
 	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);
+	}
+}

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."),
 		}
 	}
 }

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,15 +115,17 @@ 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;
 #[cfg(not(feature = "uniffi"))]
 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<u64>, channel_config: Option<ChannelConfig>,
-		announce_channel: bool,
 	) -> Result<UserChannelId, Error> {
 		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<u64>, channel_config: Option<Arc<ChannelConfig>>,
+	) -> Result<UserChannelId, Error> {
+		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<u64>, channel_config: Option<Arc<ChannelConfig>>,
+	) -> Result<UserChannelId, Error> {
+		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.
 	///