Refactor may_announce_channel to return AnnounceError

This error type can provide more context to callers about why
the node configuration does not support channel announcements.

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, may_announce_channel, AnnounceError, Config, EsploraSyncConfig,
+	DEFAULT_LOG_FILENAME, DEFAULT_LOG_LEVEL, WALLET_KEYS_SEED_LEN,
 };
 
 use crate::connection::ConnectionManager;
@@ -32,8 +32,7 @@ use crate::types::{
 };
 use crate::wallet::persist::KVStoreWalletPersister;
 use crate::wallet::Wallet;
-use crate::Node;
-use crate::{io, NodeMetrics};
+use crate::{io, Node, NodeMetrics};
 
 use lightning::chain::{chainmonitor, BestBlock, Watch};
 use lightning::io::Cursor;
@@ -912,6 +911,21 @@ 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 let Err(err) = may_announce_channel(&config) {
+		if config.announcement_addresses.is_some() {
+			log_error!(logger, "Announcement addresses were set but some required configuration options for node announcement are missing: {}", err);
+			return Err(match err {
+				AnnounceError::MissingNodeAlias => BuildError::InvalidNodeAlias,
+				_ => BuildError::InvalidListeningAddresses,
+			});
+		}
+
+		if config.node_alias.is_some() {
+			log_error!(logger, "Node alias was set but some required configuration options for node announcement are missing: {}", err);
+			return Err(BuildError::InvalidListeningAddresses);
+		}
+	}
+
 	// Initialize the status fields.
 	let is_listening = Arc::new(AtomicBool::new(false));
 	let node_metrics = match read_node_metrics(Arc::clone(&kv_store), Arc::clone(&logger)) {

src/config.rs

@@ -19,6 +19,7 @@ use lightning::util::config::UserConfig;
 use bitcoin::secp256k1::PublicKey;
 use bitcoin::Network;
 
+use std::fmt;
 use std::time::Duration;
 
 // Config defaults
@@ -261,9 +262,37 @@ 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,
+}
+
+impl fmt::Display for AnnounceError {
+	fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+		match self {
+			AnnounceError::MissingNodeAlias => write!(f, "Node alias is not configured"),
+			AnnounceError::MissingListeningAddresses => {
+				write!(f, "Listening addresses are not configured")
+			},
+			AnnounceError::MissingBoth => {
+				write!(f, "Node alias and listening addresses are not configured")
+			},
+		}
+	}
+}
+
+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 +307,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 +472,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 +481,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 +490,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,21 @@ 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) {
+						log_error!(self.logger, "Rejecting inbound announced channel from peer {} due to missing configuration: {}", counterparty_node_id, err);
 
-					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(());
+						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,19 @@ 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) {
+			log_error!(self.logger, "Failed to open announced channel as the node hasn't been sufficiently configured to act as a forwarding node: {}", err);
 			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