introduce AnnounceError to know why we can't announce

Author: aylagreystone

src/builder.rs

@@ -7,8 +7,8 @@
 
 use crate::chain::{ChainSource, DEFAULT_ESPLORA_SERVER_URL};
 use crate::config::{
-	default_user_config, Config, EsploraSyncConfig, DEFAULT_LOG_FILENAME, DEFAULT_LOG_LEVEL,
-	WALLET_KEYS_SEED_LEN,
+	default_user_config, AnnounceError, Config, EsploraSyncConfig, DEFAULT_LOG_FILENAME,
+	DEFAULT_LOG_LEVEL, WALLET_KEYS_SEED_LEN,
 };
 
 use crate::connection::ConnectionManager;
@@ -896,14 +896,29 @@ fn build_with_store_internal(
 	liquidity_source_config: Option<&LiquiditySourceConfig>, seed_bytes: [u8; 64],
 	logger: Arc<Logger>, kv_store: Arc<DynStore>,
 ) -> Result<Node, BuildError> {
-	if config.announcement_addresses.is_some() && !may_announce_channel(&config) {
-		log_error!(logger, "Announcement addresses are set but the node will not announce itself because both node alias and listening addresses are not set.");
-		return Err(BuildError::InvalidAnnouncementAddresses);
-	}
+	if let Err(err) = may_announce_channel(&config) {
+		let error_msg_for = |prefix: &str| -> String {
+			match err {
+				AnnounceError::MissingNodeAlias =>
+					format!("{} set but the node will not announce itself as node alias is not configured. Please set a node alias.", prefix),
+				AnnounceError::MissingListeningAddresses =>
+					format!("{} set but the node will not announce itself as listening addresses are not configured. Please set listening addresses.", prefix),
+				AnnounceError::MissingBoth =>
+					format!("{} set but the node will not announce itself as both node alias and listening addresses are not configured. Please configure both.", prefix),
+			}
+		};
+		if config.announcement_addresses.is_some() {
+			log_error!(logger, "{}", error_msg_for("Announcement addresses are"));
+			return Err(match err {
+				AnnounceError::MissingNodeAlias => BuildError::InvalidNodeAlias,
+				_ => BuildError::InvalidListeningAddresses,
+			});
+		}
 
-	if config.node_alias.is_some() && !may_announce_channel(&config) {
-		log_error!(logger, "Node alias is set but the node will not announce itself because no listening addresses were provided.");
-		return Err(BuildError::InvalidListeningAddresses);
+		if config.node_alias.is_some() {
+			log_error!(logger, "{}", error_msg_for("Node alias is"));
+			return Err(BuildError::InvalidListeningAddresses);
+		}
 	}
 
 	// Initialize the status fields.

src/config.rs

@@ -261,9 +261,23 @@ pub fn default_config() -> Config {
 	Config::default()
 }
 
-pub(crate) fn may_announce_channel(config: &Config) -> bool {
-	config.node_alias.is_some()
-		&& config.listening_addresses.as_ref().map_or(false, |addrs| !addrs.is_empty())
+#[derive(Debug, PartialEq)]
+pub(crate) enum AnnounceError {
+	MissingNodeAlias,
+	MissingListeningAddresses,
+	MissingBoth,
+}
+
+pub(crate) fn may_announce_channel(config: &Config) -> Result<(), AnnounceError> {
+	let has_listening_addresses =
+		config.listening_addresses.as_ref().map_or(false, |addrs| !addrs.is_empty());
+
+	match (config.node_alias.is_some(), has_listening_addresses) {
+		(true, true) => Ok(()),
+		(true, false) => Err(AnnounceError::MissingListeningAddresses),
+		(false, true) => Err(AnnounceError::MissingNodeAlias),
+		(false, false) => Err(AnnounceError::MissingBoth),
+	}
 }
 
 pub(crate) fn default_user_config(config: &Config) -> UserConfig {
@@ -278,7 +292,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 !may_announce_channel(config) {
+	if may_announce_channel(config).is_err() {
 		user_config.accept_forwards_to_priv_channels = false;
 		user_config.channel_handshake_config.announce_for_forwarding = false;
 		user_config.channel_handshake_limits.force_announced_channel_preference = true;
@@ -443,6 +457,7 @@ mod tests {
 	use std::str::FromStr;
 
 	use super::may_announce_channel;
+	use super::AnnounceError;
 	use super::Config;
 	use super::NodeAlias;
 	use super::SocketAddress;
@@ -451,7 +466,7 @@ mod tests {
 	fn node_announce_channel() {
 		// Default configuration with node alias and listening addresses unset
 		let mut node_config = Config::default();
-		assert!(!may_announce_channel(&node_config));
+		assert_eq!(may_announce_channel(&node_config), Err(AnnounceError::MissingBoth));
 
 		// Set node alias with listening addresses unset
 		let alias_frm_str = |alias: &str| {
@@ -460,24 +475,33 @@ mod tests {
 			NodeAlias(bytes)
 		};
 		node_config.node_alias = Some(alias_frm_str("LDK_Node"));
-		assert!(!may_announce_channel(&node_config));
+		assert_eq!(
+			may_announce_channel(&node_config),
+			Err(AnnounceError::MissingListeningAddresses)
+		);
 
 		// Set announcement addresses with listening addresses unset
 		let announcement_address = SocketAddress::from_str("123.45.67.89:9735")
 			.expect("Socket address conversion failed.");
 		node_config.announcement_addresses = Some(vec![announcement_address]);
-		assert!(!may_announce_channel(&node_config));
+		assert_eq!(
+			may_announce_channel(&node_config),
+			Err(AnnounceError::MissingListeningAddresses)
+		);
 
 		// Set node alias with an empty list of listening addresses
 		node_config.listening_addresses = Some(vec![]);
-		assert!(!may_announce_channel(&node_config));
+		assert_eq!(
+			may_announce_channel(&node_config),
+			Err(AnnounceError::MissingListeningAddresses)
+		);
 
 		// 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!(may_announce_channel(&node_config));
+		assert!(may_announce_channel(&node_config).is_ok());
 	}
 }

src/event.rs

@@ -12,7 +12,7 @@ use crate::{
 	UserChannelId,
 };
 
-use crate::config::{may_announce_channel, Config};
+use crate::config::{may_announce_channel, AnnounceError, Config};
 use crate::connection::ConnectionManager;
 use crate::fee_estimator::ConfirmationTarget;
 use crate::liquidity::LiquiditySource;
@@ -1057,23 +1057,30 @@ where
 				is_announced,
 				params: _,
 			} => {
-				if is_announced && !may_announce_channel(&*self.config) {
-					log_error!(
-						self.logger,
-						"Rejecting inbound announced channel from peer {} as not all required details are set. Please ensure node alias and listening addresses have been configured.",
-						counterparty_node_id,
-					);
+				if is_announced {
+					if let Err(err) = may_announce_channel(&*self.config) {
+						let error_msg = match err {
+							AnnounceError::MissingNodeAlias =>
+									format!("Rejecting inbound announced channel from peer {} as node alias is not set. Please ensure node alias has been configured.", counterparty_node_id),
+							AnnounceError::MissingListeningAddresses =>
+									format!("Rejecting inbound announced channel from peer {} as listening addresses are not set. Please ensure listening addresses have been configured.", counterparty_node_id),
+							AnnounceError::MissingBoth =>
+									format!("Rejecting inbound announced channel from peer {} as node alias and listening addresses are not set. Please ensure both have been configured.", counterparty_node_id),
+						};
 
-					self.channel_manager
-						.force_close_without_broadcasting_txn(
-							&temporary_channel_id,
-							&counterparty_node_id,
-							"Channel request rejected".to_string(),
-						)
-						.unwrap_or_else(|e| {
-							log_error!(self.logger, "Failed to reject channel: {:?}", e)
-						});
-					return Ok(());
+						log_error!(self.logger, "{}", error_msg,);
+
+						self.channel_manager
+							.force_close_without_broadcasting_txn(
+								&temporary_channel_id,
+								&counterparty_node_id,
+								"Channel request rejected".to_string(),
+							)
+							.unwrap_or_else(|e| {
+								log_error!(self.logger, "Failed to reject channel: {:?}", e)
+							});
+						return Ok(());
+					}
 				}
 
 				let anchor_channel = channel_type.requires_anchors_zero_fee_htlc_tx();

src/lib.rs

@@ -123,8 +123,8 @@ pub use builder::NodeBuilder as Builder;
 
 use chain::ChainSource;
 use config::{
-	default_user_config, may_announce_channel, ChannelConfig, Config, NODE_ANN_BCAST_INTERVAL,
-	PEER_RECONNECTION_INTERVAL, RGS_SYNC_INTERVAL,
+	default_user_config, may_announce_channel, AnnounceError, ChannelConfig, Config,
+	NODE_ANN_BCAST_INTERVAL, PEER_RECONNECTION_INTERVAL, RGS_SYNC_INTERVAL,
 };
 use connection::ConnectionManager;
 use event::{EventHandler, EventQueue};
@@ -414,7 +414,7 @@ impl Node {
 		let bcast_node_metrics = Arc::clone(&self.node_metrics);
 		let mut stop_bcast = self.stop_sender.subscribe();
 		let node_alias = self.config.node_alias.clone();
-		if may_announce_channel(&self.config) {
+		if may_announce_channel(&self.config).is_ok() {
 			runtime.spawn(async move {
 				// We check every 30 secs whether our last broadcast is NODE_ANN_BCAST_INTERVAL away.
 				#[cfg(not(test))]
@@ -1211,19 +1211,24 @@ impl Node {
 		&self, node_id: PublicKey, address: SocketAddress, channel_amount_sats: u64,
 		push_to_counterparty_msat: Option<u64>, channel_config: Option<ChannelConfig>,
 	) -> Result<UserChannelId, Error> {
-		if may_announce_channel(&self.config) {
-			self.open_channel_inner(
-				node_id,
-				address,
-				channel_amount_sats,
-				push_to_counterparty_msat,
-				channel_config,
-				true,
-			)
-		} else {
-			log_error!(self.logger, "Failed to open announced channel as the node hasn't been sufficiently configured to act as a forwarding node. Please make sure to configure listening addreesses and node alias");
+		if let Err(err) = may_announce_channel(&self.config) {
+			let error_msg = match err {
+					AnnounceError::MissingNodeAlias => "Failed to open announced channel as the node hasn't been sufficiently configured to act as a forwarding node. Please make sure to configure a node alias",
+					AnnounceError::MissingListeningAddresses => "Failed to open announced channel as the node hasn't been sufficiently configured to act as a forwarding node. Please make sure to configure listening addreesses",
+					AnnounceError::MissingBoth => "Failed to open announced channel as the node hasn't been sufficiently configured to act as a forwarding node. Please make sure to configure listening addreesses and node alias",
+			};
+			log_error!(self.logger, "{}", error_msg);
 			return Err(Error::ChannelCreationFailed);
 		}
+
+		self.open_channel_inner(
+			node_id,
+			address,
+			channel_amount_sats,
+			push_to_counterparty_msat,
+			channel_config,
+			true,
+		)
 	}
 
 	/// Manually sync the LDK and BDK wallets with the current chain state and update the fee rate