-
Notifications
You must be signed in to change notification settings - Fork 129
Add Tor support for outbound connections via SOCKS #778
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,7 +45,7 @@ use vss_client::headers::VssHeaderProvider; | |
| use crate::chain::ChainSource; | ||
| use crate::config::{ | ||
| default_user_config, may_announce_channel, AnnounceError, AsyncPaymentsRole, | ||
| BitcoindRestClientConfig, Config, ElectrumSyncConfig, EsploraSyncConfig, | ||
| BitcoindRestClientConfig, Config, ElectrumSyncConfig, EsploraSyncConfig, TorConfig, | ||
| DEFAULT_ESPLORA_SERVER_URL, DEFAULT_LOG_FILENAME, DEFAULT_LOG_LEVEL, | ||
| }; | ||
| use crate::connection::ConnectionManager; | ||
|
|
@@ -165,6 +165,8 @@ pub enum BuildError { | |
| InvalidListeningAddresses, | ||
| /// The given announcement addresses are invalid, e.g. too many were passed. | ||
| InvalidAnnouncementAddresses, | ||
| /// The given tor proxy address is invalid, e.g. an onion address was passed. | ||
| InvalidTorProxyAddress, | ||
| /// The provided alias is invalid. | ||
| InvalidNodeAlias, | ||
| /// An attempt to setup a runtime has failed. | ||
|
|
@@ -206,6 +208,7 @@ impl fmt::Display for BuildError { | |
| Self::InvalidAnnouncementAddresses => { | ||
| write!(f, "Given announcement addresses are invalid.") | ||
| }, | ||
| Self::InvalidTorProxyAddress => write!(f, "Given Tor proxy address is invalid."), | ||
| Self::RuntimeSetupFailed => write!(f, "Failed to setup a runtime."), | ||
| Self::ReadFailed => write!(f, "Failed to read from store."), | ||
| Self::WriteFailed => write!(f, "Failed to write to store."), | ||
|
|
@@ -523,6 +526,23 @@ impl NodeBuilder { | |
| Ok(self) | ||
| } | ||
|
|
||
| /// Configures the [`Node`] instance to use a Tor SOCKS proxy for outbound connections to peers with OnionV3 addresses. | ||
| /// Connections to clearnet addresses are not affected, and are not made over Tor. | ||
| /// The proxy address must not itself be an onion address. | ||
| /// | ||
| /// **Note**: If unset, connecting to peer OnionV3 addresses will fail. | ||
| pub fn set_tor_config(&mut self, tor_config: TorConfig) -> Result<&mut Self, BuildError> { | ||
| match tor_config.proxy_address { | ||
| SocketAddress::OnionV2 { .. } | SocketAddress::OnionV3 { .. } => { | ||
| return Err(BuildError::InvalidTorProxyAddress); | ||
| }, | ||
| _ => {}, | ||
| } | ||
|
|
||
| self.config.tor_config = Some(tor_config); | ||
| Ok(self) | ||
| } | ||
|
|
||
| /// Sets the node alias that will be used when broadcasting announcements to the gossip | ||
| /// network. | ||
| /// | ||
|
|
@@ -957,6 +977,15 @@ impl ArcedNodeBuilder { | |
| self.inner.write().unwrap().set_announcement_addresses(announcement_addresses).map(|_| ()) | ||
| } | ||
|
|
||
| /// Configures the [`Node`] instance to use a Tor SOCKS proxy for outbound connections to peers with OnionV3 addresses. | ||
| /// Connections to clearnet addresses are not affected, and are not made over Tor. | ||
| /// The proxy address must not itself be an onion address. | ||
| /// | ||
| /// **Note**: If unset, connecting to peer OnionV3 addresses will fail. | ||
| pub fn set_tor_config(&self, tor_config: TorConfig) -> Result<(), BuildError> { | ||
| self.inner.write().unwrap().set_tor_config(tor_config).map(|_| ()) | ||
| } | ||
|
|
||
| /// Sets the node alias that will be used when broadcasting announcements to the gossip | ||
| /// network. | ||
| /// | ||
|
|
@@ -1146,6 +1175,15 @@ fn build_with_store_internal( | |
| } | ||
| } | ||
|
|
||
| if let Some(tor_config) = &config.tor_config { | ||
| match tor_config.proxy_address { | ||
| SocketAddress::OnionV2 { .. } | SocketAddress::OnionV3 { .. } => { | ||
| return Err(BuildError::InvalidTorProxyAddress); | ||
| }, | ||
| _ => {}, | ||
| } | ||
| } | ||
|
Comment on lines
+1178
to
+1185
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we put this here in addition to the validation in
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes exactly. |
||
|
|
||
| let tx_broadcaster = Arc::new(TransactionBroadcaster::new(Arc::clone(&logger))); | ||
| let fee_estimator = Arc::new(OnchainFeeEstimator::new()); | ||
|
|
||
|
|
@@ -1779,8 +1817,12 @@ fn build_with_store_internal( | |
|
|
||
| liquidity_source.as_ref().map(|l| l.set_peer_manager(Arc::downgrade(&peer_manager))); | ||
|
|
||
| let connection_manager = | ||
| Arc::new(ConnectionManager::new(Arc::clone(&peer_manager), Arc::clone(&logger))); | ||
| let connection_manager = Arc::new(ConnectionManager::new( | ||
| Arc::clone(&peer_manager), | ||
| config.tor_config.clone(), | ||
| Arc::clone(&keys_manager), | ||
| Arc::clone(&logger), | ||
| )); | ||
|
|
||
| let output_sweeper = match sweeper_bytes_res { | ||
| Ok(output_sweeper) => Arc::new(output_sweeper), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -131,7 +131,8 @@ pub(crate) const LNURL_AUTH_TIMEOUT_SECS: u64 = 15; | |
| /// | `probing_liquidity_limit_multiplier` | 3 | | ||
| /// | `log_level` | Debug | | ||
| /// | `anchor_channels_config` | Some(..) | | ||
| /// | `route_parameters` | None | | ||
| /// | `route_parameters` | None | | ||
| /// | `tor_config` | None | | ||
| /// | ||
| /// See [`AnchorChannelsConfig`] and [`RouteParametersConfig`] for more information regarding their | ||
| /// respective default values. | ||
|
|
@@ -196,6 +197,13 @@ pub struct Config { | |
| /// **Note:** If unset, default parameters will be used, and you will be able to override the | ||
| /// parameters on a per-payment basis in the corresponding method calls. | ||
| pub route_parameters: Option<RouteParametersConfig>, | ||
| /// Configuration options for enabling peer connections via the Tor network. | ||
| /// | ||
| /// Setting [`TorConfig`] enables connecting to peers with OnionV3 addresses. No other connections | ||
| /// are routed via Tor. Please refer to [`TorConfig`] for further information. | ||
| /// | ||
| /// **Note**: If unset, connecting to peer OnionV3 addresses will fail. | ||
| pub tor_config: Option<TorConfig>, | ||
| } | ||
|
|
||
| impl Default for Config { | ||
|
|
@@ -208,6 +216,7 @@ impl Default for Config { | |
| trusted_peers_0conf: Vec::new(), | ||
| probing_liquidity_limit_multiplier: DEFAULT_PROBING_LIQUIDITY_LIMIT_MULTIPLIER, | ||
| anchor_channels_config: Some(AnchorChannelsConfig::default()), | ||
| tor_config: None, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the doc for
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah didn't notice that bit; updated + rebased |
||
| route_parameters: None, | ||
| node_alias: None, | ||
| } | ||
|
|
@@ -487,6 +496,16 @@ pub struct BitcoindRestClientConfig { | |
| pub rest_port: u16, | ||
| } | ||
|
|
||
| /// Configuration for connecting to peers via the Tor Network. | ||
| #[derive(Debug, Clone)] | ||
| #[cfg_attr(feature = "uniffi", derive(uniffi::Record))] | ||
| pub struct TorConfig { | ||
| /// Tor daemon SOCKS proxy address. Only connections to OnionV3 peers will be made | ||
| /// via this proxy; other connections (IPv4 peers, Electrum server) will not be | ||
| /// routed over Tor. | ||
| pub proxy_address: SocketAddress, | ||
| } | ||
|
|
||
| /// Options which apply on a per-channel basis and may change at runtime or based on negotiation | ||
| /// with our counterparty. | ||
| #[derive(Copy, Clone, Debug, PartialEq, Eq)] | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should move this check to the
buildstep so it happens also when set viaBuilder::from_config? Not sure if we even need this setter then?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with also having the check there, though I'll note that there are checks in other setters, like
set_announcement_addresses()andset_listening_addresses(), that aren't mirrored inbuild_with_store_internal()right now.I'd prefer to keep the setter (since I'm using the others as well), but can remove it if needed.