f: review comments

Author: joostjager

bindings/ldk_node.udl

@@ -94,6 +94,7 @@ interface Builder {
 	void set_announcement_addresses(sequence<SocketAddress> announcement_addresses);
 	[Throws=BuildError]
 	void set_node_alias(string node_alias);
+	[Throws=BuildError]
 	void set_async_payments_role(AsyncPaymentsRole? role);
 	[Throws=BuildError]
 	Node build();
@@ -356,6 +357,7 @@ enum BuildError {
 	"WalletSetupFailed",
 	"LoggerSetupFailed",
 	"NetworkMismatch",
+	"AsyncPaymentsConfigMismatch",
 };
 
 [Trait]

src/builder.rs

@@ -192,6 +192,8 @@ pub enum BuildError {
 	LoggerSetupFailed,
 	/// The given network does not match the node's previously configured network.
 	NetworkMismatch,
+	/// The role of the node in an asynchronous payments context is not compatible with the current configuration.
+	AsyncPaymentsConfigMismatch,
 }
 
 impl fmt::Display for BuildError {
@@ -220,6 +222,12 @@ impl fmt::Display for BuildError {
 			Self::NetworkMismatch => {
 				write!(f, "Given network does not match the node's previously configured network.")
 			},
+			Self::AsyncPaymentsConfigMismatch => {
+				write!(
+					f,
+					"The async payments role is not compatible with the current configuration."
+				)
+			},
 		}
 	}
 }
@@ -241,8 +249,8 @@ pub struct NodeBuilder {
 	gossip_source_config: Option<GossipSourceConfig>,
 	liquidity_source_config: Option<LiquiditySourceConfig>,
 	log_writer_config: Option<LogWriterConfig>,
-	runtime_handle: Option<tokio::runtime::Handle>,
 	async_payments_role: Option<AsyncPaymentsRole>,
+	runtime_handle: Option<tokio::runtime::Handle>,
 }
 
 impl NodeBuilder {
@@ -547,10 +555,18 @@ impl NodeBuilder {
 		Ok(self)
 	}
 
-	/// Sets the role of the node in an asynchronous payments context.
-	pub fn set_async_payments_role(&mut self, role: Option<AsyncPaymentsRole>) -> &mut Self {
+	/// Sets the role of the node in an asynchronous payments context. See https://github.com/lightning/bolts/pull/1149
+	/// for more information about the async payments protocol.
+	pub fn set_async_payments_role(
+		&mut self, role: Option<AsyncPaymentsRole>,
+	) -> Result<&mut Self, BuildError> {
+		if let Some(AsyncPaymentsRole::Server) = role {
+			may_announce_channel(&self.config)
+				.map_err(|_| BuildError::AsyncPaymentsConfigMismatch)?;
+		}
+
 		self.async_payments_role = role;
-		self
+		Ok(self)
 	}
 
 	/// Builds a [`Node`] instance with a [`SqliteStore`] backend and according to the options
@@ -709,11 +725,11 @@ impl NodeBuilder {
 			self.chain_data_source_config.as_ref(),
 			self.gossip_source_config.as_ref(),
 			self.liquidity_source_config.as_ref(),
+			self.async_payments_role,
 			seed_bytes,
 			runtime,
 			logger,
 			Arc::new(vss_store),
-			self.async_payments_role,
 		)
 	}
 
@@ -742,11 +758,11 @@ impl NodeBuilder {
 			self.chain_data_source_config.as_ref(),
 			self.gossip_source_config.as_ref(),
 			self.liquidity_source_config.as_ref(),
+			self.async_payments_role,
 			seed_bytes,
 			runtime,
 			logger,
 			kv_store,
-			self.async_payments_role,
 		)
 	}
 }
@@ -1001,8 +1017,10 @@ impl ArcedNodeBuilder {
 	}
 
 	/// Sets the role of the node in an asynchronous payments context.
-	pub fn set_async_payments_role(&self, role: Option<AsyncPaymentsRole>) {
-		_ = self.inner.write().unwrap().set_async_payments_role(role)
+	pub fn set_async_payments_role(
+		&self, role: Option<AsyncPaymentsRole>,
+	) -> Result<(), BuildError> {
+		self.inner.write().unwrap().set_async_payments_role(role).map(|_| ())
 	}
 
 	/// Builds a [`Node`] instance with a [`SqliteStore`] backend and according to the options
@@ -1098,9 +1116,9 @@ impl ArcedNodeBuilder {
 fn build_with_store_internal(
 	config: Arc<Config>, chain_data_source_config: Option<&ChainDataSourceConfig>,
 	gossip_source_config: Option<&GossipSourceConfig>,
-	liquidity_source_config: Option<&LiquiditySourceConfig>, seed_bytes: [u8; 64],
-	runtime: Arc<Runtime>, logger: Arc<Logger>, kv_store: Arc<DynStore>,
-	async_payments_role: Option<AsyncPaymentsRole>,
+	liquidity_source_config: Option<&LiquiditySourceConfig>,
+	async_payments_role: Option<AsyncPaymentsRole>, seed_bytes: [u8; 64], runtime: Arc<Runtime>,
+	logger: Arc<Logger>, kv_store: Arc<DynStore>,
 ) -> Result<Node, BuildError> {
 	optionally_install_rustls_cryptoprovider();
 

src/config.rs

@@ -535,12 +535,13 @@ impl From<MaxDustHTLCExposure> for LdkMaxDustHTLCExposure {
 }
 
 #[derive(Debug, Clone, Copy)]
-/// The role of the node in an asynchronous payments context.
+/// The role of the node in an asynchronous payments context. See https://github.com/lightning/bolts/pull/1149 for more
+/// information about the async payments protocol.
 pub enum AsyncPaymentsRole {
 	/// Node acts a client in an async payments context. This means that if possible, it will instruct its peers to hold
-	/// htlcs for it, so that it can go offline.
+	/// HTLCs for it, so that it can go offline.
 	Client,
-	/// Node acts as a server in an async payments context. This means that it will hold async payments htlcs and onion
+	/// Node acts as a server in an async payments context. This means that it will hold async payments HTLCs and onion
 	/// messages for its peers.
 	Server,
 }

src/lib.rs

@@ -126,10 +126,9 @@ pub use builder::BuildError;
 pub use builder::NodeBuilder as Builder;
 
 use chain::ChainSource;
-use config::AsyncPaymentsRole;
 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, AsyncPaymentsRole, ChannelConfig, Config,
+	NODE_ANN_BCAST_INTERVAL, PEER_RECONNECTION_INTERVAL, RGS_SYNC_INTERVAL,
 };
 use connection::ConnectionManager;
 use event::{EventHandler, EventQueue};

src/payment/asynchronous/om_mailbox.rs

@@ -5,7 +5,7 @@ use bitcoin::secp256k1::PublicKey;
 use lightning::ln::msgs::OnionMessage;
 
 pub(crate) struct OnionMessageMailbox {
-	map: Mutex<HashMap<bitcoin::secp256k1::PublicKey, VecDeque<lightning::ln::msgs::OnionMessage>>>,
+	map: Mutex<HashMap<PublicKey, VecDeque<OnionMessage>>>,
 }
 
 impl OnionMessageMailbox {
@@ -35,7 +35,7 @@ impl OnionMessageMailbox {
 	}
 
 	pub(crate) fn onion_message_peer_connected(
-		&self, peer_node_id: bitcoin::secp256k1::PublicKey,
+		&self, peer_node_id: PublicKey,
 	) -> Vec<OnionMessage> {
 		let mut map = self.map.lock().unwrap();
 

tests/common/mod.rs

@@ -382,7 +382,7 @@ pub(crate) fn setup_node_for_async_payments(
 		}
 	}
 
-	builder.set_async_payments_role(async_payments_role);
+	builder.set_async_payments_role(async_payments_role).unwrap();
 
 	let test_sync_store = Arc::new(TestSyncStore::new(config.node_config.storage_dir_path.into()));
 	let node = builder.build_with_store(test_sync_store).unwrap();