From e9ef8300df86ff7434b248fd2c2e9459f0b3e3e5 Mon Sep 17 00:00:00 2001 From: benthecarman Date: Wed, 18 Mar 2026 13:55:14 -0500 Subject: [PATCH] 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. --- bindings/ldk_node.udl | 9 ++-- src/error.rs | 3 ++ src/lib.rs | 73 ++++++++++++++++++--------------- tests/common/mod.rs | 10 ++--- tests/integration_tests_cln.rs | 2 +- tests/integration_tests_lnd.rs | 2 +- tests/integration_tests_rust.rs | 50 +++++++++++++++++++++- 7 files changed, 105 insertions(+), 44 deletions(-) diff --git a/bindings/ldk_node.udl b/bindings/ldk_node.udl index 0addb1fe1..1cc3bf616 100644 --- a/bindings/ldk_node.udl +++ b/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] @@ -169,6 +169,7 @@ enum NodeError { "NotRunning", "OnchainTxCreationFailed", "ConnectionFailed", + "NotConnected", "InvoiceCreationFailed", "InvoiceRequestCreationFailed", "OfferCreationFailed", diff --git a/src/error.rs b/src/error.rs index d07212b00..204d48f66 100644 --- a/src/error.rs +++ b/src/error.rs @@ -25,6 +25,8 @@ pub enum Error { OnchainTxCreationFailed, /// A network connection has been closed. ConnectionFailed, + /// The peer is not connected. + NotConnected, /// Invoice creation failed. InvoiceCreationFailed, /// Invoice request creation failed. @@ -148,6 +150,7 @@ impl fmt::Display for Error { write!(f, "On-chain transaction could not be created.") }, Self::ConnectionFailed => write!(f, "Network connection closed."), + Self::NotConnected => write!(f, "The peer is not connected."), Self::InvoiceCreationFailed => write!(f, "Failed to create invoice."), Self::InvoiceRequestCreationFailed => write!(f, "Failed to create invoice request."), Self::OfferCreationFailed => write!(f, "Failed to create offer."), diff --git a/src/lib.rs b/src/lib.rs index e3beb44db..d8615281e 100644 --- a/src/lib.rs +++ b/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, channel_config: Option, - announce_for_forwarding: bool, + &self, node_id: PublicKey, address: Option, + channel_amount_sats: FundingAmount, push_to_counterparty_msat: Option, + channel_config: Option, announce_for_forwarding: bool, ) -> Result { 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::NotConnected)?; + 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,7 +1205,7 @@ impl Node { ); match self.channel_manager.create_channel( - peer_info.node_id, + node_id, channel_amount_sats, push_msat, user_channel_id, @@ -1205,12 +1213,13 @@ impl Node { 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) => { @@ -1224,7 +1233,7 @@ impl Node { let init_features = self .peer_manager .peer_by_node_id(peer_node_id) - .ok_or(Error::ConnectionFailed)? + .ok_or(Error::NotConnected)? .init_features; let anchor_channel = init_features.requires_anchors_zero_fee_htlc_tx(); Ok(new_channel_anchor_reserve_sats(&self.config, peer_node_id, anchor_channel)) @@ -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, channel_amount_sats: u64, push_to_counterparty_msat: Option, channel_config: Option, ) -> Result { 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, channel_amount_sats: u64, push_to_counterparty_msat: Option, channel_config: Option, ) -> Result { 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, - channel_config: Option, + &self, node_id: PublicKey, address: Option, + push_to_counterparty_msat: Option, channel_config: Option, ) -> Result { 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, - channel_config: Option, + &self, node_id: PublicKey, address: Option, + push_to_counterparty_msat: Option, channel_config: Option, ) -> Result { 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}"); diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 7854a77f2..2312935c8 100644 --- a/tests/common/mod.rs +++ b/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, ) @@ -851,7 +851,7 @@ pub(crate) async fn do_channel_full_cycle( 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, Some(push_msat), None, diff --git a/tests/integration_tests_cln.rs b/tests/integration_tests_cln.rs index 0245f1fdf..97d6b131e 100644 --- a/tests/integration_tests_cln.rs +++ b/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); diff --git a/tests/integration_tests_lnd.rs b/tests/integration_tests_lnd.rs index 8f1d4c868..5f0f0bda9 100755 --- a/tests/integration_tests_lnd.rs +++ b/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); diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 3fde52dc4..330b2d532 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -114,7 +114,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, @@ -877,6 +877,54 @@ async fn do_connection_restart_behavior(persist: bool) { } } +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn open_channel_with_optional_address() { + 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); + + let addr_a = node_a.onchain_payment().new_address().unwrap(); + let addr_b = node_b.onchain_payment().new_address().unwrap(); + + let premine_amount_sat = 2_125_000; + + premine_and_distribute_funds( + &bitcoind.client, + &electrsd.client, + vec![addr_a, addr_b], + Amount::from_sat(premine_amount_sat), + ) + .await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + // Opening a channel with no address and no prior connection should fail. + let res = node_a.open_channel(node_b.node_id(), None, 120000, None, None); + assert_eq!(res, Err(NodeError::NotConnected)); + + // Connect to the peer with persist=false. + let node_addr_b = node_b.listening_addresses().unwrap().first().unwrap().clone(); + node_a.connect(node_b.node_id(), node_addr_b, false).unwrap(); + assert!(!node_a.list_peers().first().unwrap().is_persisted); + + // Opening a channel with no address should now succeed since we're already connected. + node_a.open_channel(node_b.node_id(), None, 120000, None, None).unwrap(); + + // The peer should now be persisted after the channel open. + assert!(node_a.list_peers().first().unwrap().is_persisted); + + let funding_txo = expect_channel_pending_event!(node_a, node_b.node_id()); + expect_channel_pending_event!(node_b, node_a.node_id()); + + wait_for_tx(&electrsd.client, funding_txo.txid).await; + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn concurrent_connections_succeed() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();