Make node's address optional when opening channel

Previously we always required providing the node's address when opening
a channel, this made the api annoying in some cases as you may already
be connected to the peer but still need to provide the socket address.

Here we make it optional and will return a `ConnectionFailed` error when
the socket address is not provided and we are not connected to the peer.

Author: benthecarman

bindings/ldk_node.udl

@@ -105,13 +105,13 @@ interface Node {
 	[Throws=NodeError]
 	void disconnect(PublicKey node_id);
 	[Throws=NodeError]
-	UserChannelId open_channel(PublicKey node_id, SocketAddress address, u64 channel_amount_sats, u64? push_to_counterparty_msat, ChannelConfig? channel_config);
+	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);
+	UserChannelId open_announced_channel(PublicKey node_id, SocketAddress? address, u64 channel_amount_sats, u64? push_to_counterparty_msat, ChannelConfig? channel_config);
 	[Throws=NodeError]
-	UserChannelId open_channel_with_all(PublicKey node_id, SocketAddress address, u64? push_to_counterparty_msat, ChannelConfig? channel_config);
+	UserChannelId open_channel_with_all(PublicKey node_id, SocketAddress? address, u64? push_to_counterparty_msat, ChannelConfig? channel_config);
 	[Throws=NodeError]
-	UserChannelId open_announced_channel_with_all(PublicKey node_id, SocketAddress address, u64? push_to_counterparty_msat, ChannelConfig? channel_config);
+	UserChannelId open_announced_channel_with_all(PublicKey node_id, SocketAddress? address, u64? push_to_counterparty_msat, ChannelConfig? channel_config);
 	[Throws=NodeError]
 	void splice_in([ByRef]UserChannelId user_channel_id, PublicKey counterparty_node_id, u64 splice_amount_sats);
 	[Throws=NodeError]

src/lib.rs

@@ -54,7 +54,7 @@
 //!
 //! 	let node_id = PublicKey::from_str("NODE_ID").unwrap();
 //! 	let node_addr = SocketAddress::from_str("IP_ADDR:PORT").unwrap();
-//! 	node.open_channel(node_id, node_addr, 10000, None, None).unwrap();
+//! 	node.open_channel(node_id, Some(node_addr), 10000, None, None).unwrap();
 //!
 //! 	let event = node.wait_next_event();
 //! 	println!("EVENT: {:?}", event);
@@ -1126,39 +1126,47 @@ impl Node {
 	}
 
 	fn open_channel_inner(
-		&self, node_id: PublicKey, address: SocketAddress, channel_amount_sats: FundingAmount,
-		push_to_counterparty_msat: Option<u64>, channel_config: Option<ChannelConfig>,
-		announce_for_forwarding: bool,
+		&self, node_id: PublicKey, address: Option<SocketAddress>,
+		channel_amount_sats: FundingAmount, push_to_counterparty_msat: Option<u64>,
+		channel_config: Option<ChannelConfig>, announce_for_forwarding: bool,
 	) -> Result<UserChannelId, Error> {
 		if !*self.is_running.read().unwrap() {
 			return Err(Error::NotRunning);
 		}
 
-		let peer_info = PeerInfo { node_id, address };
-
-		let con_node_id = peer_info.node_id;
-		let con_addr = peer_info.address.clone();
-		let con_cm = Arc::clone(&self.connection_manager);
-
-		// We need to use our main runtime here as a local runtime might not be around to poll
-		// connection futures going forward.
-		self.runtime.block_on(async move {
-			con_cm.connect_peer_if_necessary(con_node_id, con_addr).await
-		})?;
+		// if we don't have the socket address, check if we are already connected
+		let address = match address {
+			Some(address) => {
+				// We need to use our main runtime here as a local runtime might not be around to poll
+				// connection futures going forward.
+				let con_cm = Arc::clone(&self.connection_manager);
+				let con_addr = address.clone();
+				self.runtime.block_on(async move {
+					con_cm.connect_peer_if_necessary(node_id, con_addr).await
+				})?;
+				Some(address)
+			},
+			None => {
+				// If we are connected, grab the socket address as we need to make sure we have it persisted
+				// in our peer storage for future reconnections.
+				let peer =
+					self.peer_manager.peer_by_node_id(&node_id).ok_or(Error::ConnectionFailed)?;
+				peer.socket_address
+			},
+		};
 
 		let channel_amount_sats = match channel_amount_sats {
 			FundingAmount::Exact { amount_sats } => {
 				// Check funds availability after connection (includes anchor reserve
 				// calculation).
-				self.check_sufficient_funds_for_channel(amount_sats, &peer_info.node_id)?;
+				self.check_sufficient_funds_for_channel(amount_sats, &node_id)?;
 				amount_sats
 			},
 			FundingAmount::Max => {
 				// Determine max funding amount from all available on-chain funds.
 				let cur_anchor_reserve_sats =
 					total_anchor_channels_reserve_sats(&self.channel_manager, &self.config);
-				let new_channel_reserve =
-					self.new_channel_anchor_reserve_sats(&peer_info.node_id)?;
+				let new_channel_reserve = self.new_channel_anchor_reserve_sats(&node_id)?;
 				let total_anchor_reserve_sats = cur_anchor_reserve_sats + new_channel_reserve;
 
 				let fee_rate =
@@ -1197,20 +1205,21 @@ impl Node {
 		);
 
 		match self.channel_manager.create_channel(
-			peer_info.node_id,
+			node_id,
 			channel_amount_sats,
 			push_msat,
 			user_channel_id,
 			None,
 			Some(user_config),
 		) {
 			Ok(_) => {
-				log_info!(
-					self.logger,
-					"Initiated channel creation with peer {}. ",
-					peer_info.node_id
-				);
-				self.peer_store.add_peer(peer_info)?;
+				log_info!(self.logger, "Initiated channel creation with peer {}. ", node_id);
+
+				if let Some(address) = address {
+					let peer_info = PeerInfo { node_id, address };
+					self.peer_store.add_peer(peer_info)?;
+				}
+
 				Ok(UserChannelId(user_channel_id))
 			},
 			Err(e) => {
@@ -1280,7 +1289,7 @@ impl Node {
 	///
 	/// [`AnchorChannelsConfig::per_channel_reserve_sats`]: crate::config::AnchorChannelsConfig::per_channel_reserve_sats
 	pub fn open_channel(
-		&self, node_id: PublicKey, address: SocketAddress, channel_amount_sats: u64,
+		&self, node_id: PublicKey, address: Option<SocketAddress>, channel_amount_sats: u64,
 		push_to_counterparty_msat: Option<u64>, channel_config: Option<ChannelConfig>,
 	) -> Result<UserChannelId, Error> {
 		self.open_channel_inner(
@@ -1315,7 +1324,7 @@ impl Node {
 	///
 	/// [`AnchorChannelsConfig::per_channel_reserve_sats`]: crate::config::AnchorChannelsConfig::per_channel_reserve_sats
 	pub fn open_announced_channel(
-		&self, node_id: PublicKey, address: SocketAddress, channel_amount_sats: u64,
+		&self, node_id: PublicKey, address: Option<SocketAddress>, channel_amount_sats: u64,
 		push_to_counterparty_msat: Option<u64>, channel_config: Option<ChannelConfig>,
 	) -> Result<UserChannelId, Error> {
 		if let Err(err) = may_announce_channel(&self.config) {
@@ -1348,8 +1357,8 @@ impl Node {
 	///
 	/// [`AnchorChannelsConfig::per_channel_reserve_sats`]: crate::config::AnchorChannelsConfig::per_channel_reserve_sats
 	pub fn open_channel_with_all(
-		&self, node_id: PublicKey, address: SocketAddress, push_to_counterparty_msat: Option<u64>,
-		channel_config: Option<ChannelConfig>,
+		&self, node_id: PublicKey, address: Option<SocketAddress>,
+		push_to_counterparty_msat: Option<u64>, channel_config: Option<ChannelConfig>,
 	) -> Result<UserChannelId, Error> {
 		self.open_channel_inner(
 			node_id,
@@ -1380,8 +1389,8 @@ impl Node {
 	///
 	/// [`AnchorChannelsConfig::per_channel_reserve_sats`]: crate::config::AnchorChannelsConfig::per_channel_reserve_sats
 	pub fn open_announced_channel_with_all(
-		&self, node_id: PublicKey, address: SocketAddress, push_to_counterparty_msat: Option<u64>,
-		channel_config: Option<ChannelConfig>,
+		&self, node_id: PublicKey, address: Option<SocketAddress>,
+		push_to_counterparty_msat: Option<u64>, channel_config: Option<ChannelConfig>,
 	) -> Result<UserChannelId, Error> {
 		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}");

tests/common/mod.rs

@@ -721,7 +721,7 @@ pub async fn open_channel_push_amt(
 		node_a
 			.open_announced_channel(
 				node_b.node_id(),
-				node_b.listening_addresses().unwrap().first().unwrap().clone(),
+				node_b.listening_addresses().unwrap().first().cloned(),
 				funding_amount_sat,
 				push_amount_msat,
 				None,
@@ -731,7 +731,7 @@ pub async fn open_channel_push_amt(
 		node_a
 			.open_channel(
 				node_b.node_id(),
-				node_b.listening_addresses().unwrap().first().unwrap().clone(),
+				node_b.listening_addresses().unwrap().first().cloned(),
 				funding_amount_sat,
 				push_amount_msat,
 				None,
@@ -755,7 +755,7 @@ pub async fn open_channel_with_all(
 		node_a
 			.open_announced_channel_with_all(
 				node_b.node_id(),
-				node_b.listening_addresses().unwrap().first().unwrap().clone(),
+				node_b.listening_addresses().unwrap().first().cloned(),
 				None,
 				None,
 			)
@@ -764,7 +764,7 @@ pub async fn open_channel_with_all(
 		node_a
 			.open_channel_with_all(
 				node_b.node_id(),
-				node_b.listening_addresses().unwrap().first().unwrap().clone(),
+				node_b.listening_addresses().unwrap().first().cloned(),
 				None,
 				None,
 			)
@@ -792,7 +792,7 @@ pub async fn splice_in_with_all(
 
 pub(crate) async fn do_channel_full_cycle<E: ElectrumApi>(
 	node_a: TestNode, node_b: TestNode, bitcoind: &BitcoindClient, electrsd: &E, allow_0conf: bool,
-	expect_anchor_channel: bool, force_close: bool,
+	expect_anchor_channel: bool, force_close: bool, connect_first: bool,
 ) {
 	let addr_a = node_a.onchain_payment().new_address().unwrap();
 	let addr_b = node_b.onchain_payment().new_address().unwrap();
@@ -845,13 +845,22 @@ pub(crate) async fn do_channel_full_cycle<E: ElectrumApi>(
 	assert_eq!(node_a.next_event(), None);
 	assert_eq!(node_b.next_event(), None);
 
+	let addr = node_b.listening_addresses().unwrap().first().unwrap().clone();
+	let addr_opt = if connect_first {
+		// set persist=false so we make sure we test that it is persisted with the open
+		node_a.connect(node_b.node_id(), addr, false).unwrap();
+		None
+	} else {
+		Some(addr)
+	};
+
 	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_announced_channel(
 			node_b.node_id(),
-			node_b.listening_addresses().unwrap().first().unwrap().clone(),
+			addr_opt,
 			funding_amount_sat,
 			Some(push_msat),
 			None,

tests/integration_tests_cln.rs

@@ -89,7 +89,7 @@ async 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)
+	node.open_channel(cln_node_id, Some(cln_address), funding_amount_sat, Some(500_000_000), None)
 		.unwrap();
 
 	let funding_txo = common::expect_channel_pending_event!(node, cln_node_id);

tests/integration_tests_lnd.rs

@@ -70,7 +70,7 @@ async fn test_lnd() {
 	// Open the channel
 	let funding_amount_sat = 1_000_000;
 
-	node.open_channel(lnd_node_id, lnd_address, funding_amount_sat, Some(500_000_000), None)
+	node.open_channel(lnd_node_id, Some(lnd_address), funding_amount_sat, Some(500_000_000), None)
 		.unwrap();
 
 	let funding_txo = common::expect_channel_pending_event!(node, lnd_node_id);

tests/integration_tests_rust.rs

@@ -46,44 +46,107 @@ async fn channel_full_cycle() {
 	let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();
 	let chain_source = random_chain_source(&bitcoind, &electrsd);
 	let (node_a, node_b) = setup_two_nodes(&chain_source, false, true, false);
-	do_channel_full_cycle(node_a, node_b, &bitcoind.client, &electrsd.client, false, true, false)
-		.await;
+	do_channel_full_cycle(
+		node_a,
+		node_b,
+		&bitcoind.client,
+		&electrsd.client,
+		false,
+		true,
+		false,
+		false,
+	)
+	.await;
 }
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
 async fn channel_full_cycle_force_close() {
 	let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();
 	let chain_source = random_chain_source(&bitcoind, &electrsd);
 	let (node_a, node_b) = setup_two_nodes(&chain_source, false, true, false);
-	do_channel_full_cycle(node_a, node_b, &bitcoind.client, &electrsd.client, false, true, true)
-		.await;
+	do_channel_full_cycle(
+		node_a,
+		node_b,
+		&bitcoind.client,
+		&electrsd.client,
+		false,
+		true,
+		true,
+		false,
+	)
+	.await;
 }
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
 async fn channel_full_cycle_force_close_trusted_no_reserve() {
 	let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();
 	let chain_source = random_chain_source(&bitcoind, &electrsd);
 	let (node_a, node_b) = setup_two_nodes(&chain_source, false, true, true);
-	do_channel_full_cycle(node_a, node_b, &bitcoind.client, &electrsd.client, false, true, true)
-		.await;
+	do_channel_full_cycle(
+		node_a,
+		node_b,
+		&bitcoind.client,
+		&electrsd.client,
+		false,
+		true,
+		true,
+		false,
+	)
+	.await;
 }
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
 async fn channel_full_cycle_0conf() {
 	let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();
 	let chain_source = random_chain_source(&bitcoind, &electrsd);
 	let (node_a, node_b) = setup_two_nodes(&chain_source, true, true, false);
-	do_channel_full_cycle(node_a, node_b, &bitcoind.client, &electrsd.client, true, true, false)
-		.await;
+	do_channel_full_cycle(
+		node_a,
+		node_b,
+		&bitcoind.client,
+		&electrsd.client,
+		true,
+		true,
+		false,
+		false,
+	)
+	.await;
 }
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
 async fn channel_full_cycle_legacy_staticremotekey() {
 	let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();
 	let chain_source = random_chain_source(&bitcoind, &electrsd);
 	let (node_a, node_b) = setup_two_nodes(&chain_source, false, false, false);
-	do_channel_full_cycle(node_a, node_b, &bitcoind.client, &electrsd.client, false, false, false)
-		.await;
+	do_channel_full_cycle(
+		node_a,
+		node_b,
+		&bitcoind.client,
+		&electrsd.client,
+		false,
+		false,
+		false,
+		false,
+	)
+	.await;
+}
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
+async fn channel_full_cycle_connect_first() {
+	let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();
+	let chain_source = random_chain_source(&bitcoind, &electrsd);
+	let (node_a, node_b) = setup_two_nodes(&chain_source, false, true, false);
+	do_channel_full_cycle(
+		node_a,
+		node_b,
+		&bitcoind.client,
+		&electrsd.client,
+		false,
+		true,
+		false,
+		true,
+	)
+	.await;
 }
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
@@ -114,7 +177,7 @@ async fn channel_open_fails_when_funds_insufficient() {
 		Err(NodeError::InsufficientFunds),
 		node_a.open_channel(
 			node_b.node_id(),
-			node_b.listening_addresses().unwrap().first().unwrap().clone(),
+			node_b.listening_addresses().unwrap().first().cloned(),
 			120000,
 			None,
 			None,