Skip to content

Add remote_addr field to lightning_net_tokio::setup_outbound #4372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tankyleo wants to merge 1 commit into
base: main
Choose a base branch
from
+21 −2
Open

Add remote_addr field to lightning_net_tokio::setup_outbound #4372

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions lightning-net-tokio/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,16 @@ where
PM::Target: APeerManager<Descriptor = SocketDescriptor>,
{
let remote_addr = get_addr_from_stream(&stream);
setup_outbound_internal(peer_manager, their_node_id, stream, remote_addr)
}

fn setup_outbound_internal<PM: Deref + 'static + Send + Sync + Clone>(
peer_manager: PM, their_node_id: PublicKey, stream: StdTcpStream,
remote_addr: Option<SocketAddress>,
Copy link
Contributor

@tnull tnull Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, rather than adding this on the generic setup_outbound, would it make sense to refactor this to be an setup_outbound_internal and offer two setup_outbound and setup_outbound_tor variants that do the right (tm) thing for the user? Or, put differently, for non-Tor users this remote_addr argument might be confusing?

Copy link
Contributor Author

@tankyleo tankyleo Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know how this sounds below tnull, I avoid adding a setup_outbound_tor, and call setup_outbound_internal directly from connect_outbound_tor. I'd prefer to avoid adding a new function unless we really have to.

) -> impl std::future::Future<Output = ()>
where
PM::Target: APeerManager<Descriptor = SocketDescriptor>,
{
let (reader, mut write_receiver, read_receiver, us) = Connection::new(stream);
#[cfg(test)]
let last_us = Arc::clone(&us);
Expand Down Expand Up @@ -478,8 +488,15 @@ where
/// Routes [`connect_outbound`] through Tor. Implements stream isolation for each connection
/// using a stream isolation parameter sourced from [`EntropySource::get_secure_random_bytes`].
///
/// The `addr` parameter will be set to the [`PeerDetails::socket_address`] for that peer in
Copy link
Contributor

@tnull tnull Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add some test coverage to check this actually holds?

/// [`PeerManager::list_peers`], and if it is not a private IPV4 or IPV6 address, it will also
Copy link
Contributor

@tnull tnull Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, very nitty:

Suggested change
/// [`PeerManager::list_peers`], and if it is not a private IPV4 or IPV6 address, it will also
/// [`PeerManager::list_peers`], and if it is not a private IPv4 or IPv6 address, it will also

/// reported to the peer in our init message.
///
/// Returns a future (as the fn is async) that yields another future, see [`connect_outbound`] for
/// details on this return value.
///
/// [`PeerDetails::socket_address`]: lightning::ln::peer_handler::PeerDetails::socket_address
/// [`PeerManager::list_peers`]: lightning::ln::peer_handler::PeerManager::list_peers
pub async fn tor_connect_outbound<PM: Deref + 'static + Send + Sync + Clone, ES: EntropySource>(
peer_manager: PM, their_node_id: PublicKey, addr: SocketAddress, tor_proxy_addr: SocketAddr,
entropy_source: ES,
Expand All @@ -488,12 +505,14 @@ where
PM::Target: APeerManager<Descriptor = SocketDescriptor>,
{
let connect_fut = async {
tor_connect(addr, tor_proxy_addr, entropy_source).await.map(|s| s.into_std().unwrap())
tor_connect(addr.clone(), tor_proxy_addr, entropy_source)
.await
.map(|s| s.into_std().unwrap())
};
if let Ok(Ok(stream)) =
time::timeout(Duration::from_secs(TOR_CONNECT_OUTBOUND_TIMEOUT), connect_fut).await
{
Some(setup_outbound(peer_manager, their_node_id, stream))
Some(setup_outbound_internal(peer_manager, their_node_id, stream, Some(addr)))
} else {
None
}
Expand Down
Loading