diff --git a/changelog.d/socks-connect-timeout.md b/changelog.d/socks-connect-timeout.md new file mode 100644 index 0000000000..8a86f95121 --- /dev/null +++ b/changelog.d/socks-connect-timeout.md @@ -0,0 +1,8 @@ +Changed + +- **SOCKS dials get 30 s to connect.** Each isolated SOCKS dial builds its + own Tor circuit, and a 32-dial IBD wave queues in Tor for longer than the + 8 s clearnet bound, so whole waves timed out. Clearnet dials through + `--proxy` and onion dials now get at least 30 s. Direct and CJDNS dials + keep 8 s and I2P keeps 90 s. The rule lives on `Dialer::connect_timeout`, + which replaces `connect_timeout_for`; the floor is a `Dialer::Socks` field. diff --git a/crates/rbitcoin-ibd/src/ibd/dial.rs b/crates/rbitcoin-ibd/src/ibd/dial.rs index eae30dd8ad..98a17bf8de 100644 --- a/crates/rbitcoin-ibd/src/ibd/dial.rs +++ b/crates/rbitcoin-ibd/src/ibd/dial.rs @@ -299,7 +299,7 @@ pub async fn dial_batch( trying_connection_log(PeerConnType::OutboundFullRelay, addr) ); let dialer = dialer.clone(); - let to = rbitcoin_net::connect_timeout_for(addr, connect_timeout); + let to = dialer.connect_timeout(addr, connect_timeout); let dial_perms = perms.clone(); handles.push(tokio::spawn(async move { let fut = spawn_peer( diff --git a/crates/rbitcoin-ibd/tests/ibd/archive_sat_tests.rs b/crates/rbitcoin-ibd/tests/ibd/archive_sat_tests.rs index deb2f8a53e..d16c82b4fa 100644 --- a/crates/rbitcoin-ibd/tests/ibd/archive_sat_tests.rs +++ b/crates/rbitcoin-ibd/tests/ibd/archive_sat_tests.rs @@ -47,7 +47,6 @@ use rbitcoin_ibd::ibd::status::LoopStats; use rbitcoin_log::{info, warn}; use rbitcoin_net::chain::ChainHub; use rbitcoin_net::codec::MAX_HEADERS_RESULTS; -use rbitcoin_net::connect_timeout_for; use rbitcoin_net::error::NetError; use std::collections::{HashMap, HashSet}; use std::net::SocketAddr; diff --git a/crates/rbitcoin-ibd/tests/ibd/dial.rs b/crates/rbitcoin-ibd/tests/ibd/dial.rs index 53cecbda14..b0b0298d1b 100644 --- a/crates/rbitcoin-ibd/tests/ibd/dial.rs +++ b/crates/rbitcoin-ibd/tests/ibd/dial.rs @@ -7,7 +7,6 @@ use rbitcoin_ibd::ibd::peer_io::{ibd_mono_ms, spawn_peer, PeerCmd, PeerEventSink use rbitcoin_ibd::ibd::rate::{PROGRESS_STEP, RELSLOW_ACTIVE_MS}; use rbitcoin_log::{debug, error, warn}; use rbitcoin_net::chain::ChainHub; -use rbitcoin_net::connect_timeout_for; use rbitcoin_net::error::NetError; use rbitcoin_net::peers::{trying_connection_log, PeerConnType}; use rbitcoin_net::seeds::AddrMan; @@ -24,16 +23,47 @@ fn addr(o: u8) -> SocketAddr { SocketAddr::new(IpAddr::V4(Ipv4Addr::new(10, 0, 0, o)), 8333) } +/// A wave of isolated SOCKS dials queues on Tor circuit builds for longer +/// than a clearnet RTT; I2P waits on tunnels and leasesets. A base above +/// a floor is kept. #[test] -fn i2p_connect_timeout_is_longer_than_clearnet() { +fn overlay_and_socks_connects_outlast_a_direct_one() { let i2p = rbitcoin_net::NetAddr::I2p { dest: [0u8; 32], port: 1, }; + let onion: rbitcoin_net::NetAddr = + "pg6mmjiyjmcrsslvykfwnntlaru7p5svn6y2ymmju6nubxndf4pscryd.onion:8333" + .parse() + .unwrap(); let ip: rbitcoin_net::NetAddr = "127.0.0.1:1".parse().unwrap(); + let cjdns = rbitcoin_net::NetAddr::Cjdns { + ip: std::net::Ipv6Addr::new(0xfc00, 1, 2, 3, 4, 5, 6, 7), + port: 8333, + }; + let tor: SocketAddr = "127.0.0.1:9050".parse().unwrap(); let base = Duration::from_secs(8); - assert_eq!(connect_timeout_for(ip, base), base); - assert_eq!(connect_timeout_for(i2p, base), Duration::from_secs(90)); + let socks = Duration::from_secs(30); + let direct = rbitcoin_net::Dialer::Direct; + let proxied = rbitcoin_net::Dialer::socks(tor, true); + let onion_only = rbitcoin_net::Dialer::with_proxies(None, Some(tor), true); + assert_eq!(direct.connect_timeout(ip, base), base); + assert_eq!(direct.connect_timeout(cjdns, base), base); + assert_eq!(onion_only.connect_timeout(ip, base), base); + assert_eq!(proxied.connect_timeout(cjdns, base), base); + assert_eq!(proxied.connect_timeout(ip, base), socks); + assert_eq!(onion_only.connect_timeout(onion, base), socks); + assert_eq!(proxied.connect_timeout(onion, base), socks); + assert_eq!(direct.connect_timeout(i2p, base), Duration::from_secs(90)); + assert_eq!(proxied.connect_timeout(i2p, base), Duration::from_secs(90)); + let long = Duration::from_secs(120); + assert_eq!(proxied.connect_timeout(ip, long), long); + assert_eq!(proxied.connect_timeout(i2p, long), long); + let lowered = proxied.with_connect_floor(Duration::from_secs(2)); + assert_eq!( + lowered.connect_timeout(ip, Duration::from_secs(1)), + Duration::from_secs(2) + ); } fn dummy_slot(id: usize, a: SocketAddr, alive: bool) -> PeerSlot { diff --git a/crates/rbitcoin-ibd/tests/ibd/mod.rs b/crates/rbitcoin-ibd/tests/ibd/mod.rs index 7b321deb72..423a1600b0 100644 --- a/crates/rbitcoin-ibd/tests/ibd/mod.rs +++ b/crates/rbitcoin-ibd/tests/ibd/mod.rs @@ -47,7 +47,6 @@ use rbitcoin_ibd::ibd::status::LoopStats; use rbitcoin_log::{info, warn}; use rbitcoin_net::chain::ChainHub; use rbitcoin_net::codec::MAX_HEADERS_RESULTS; -use rbitcoin_net::connect_timeout_for; use rbitcoin_net::error::NetError; use std::collections::{HashMap, HashSet}; use std::net::SocketAddr; diff --git a/crates/rbitcoin-ibd/tests/ibd/peer_book_and_config_tests.rs b/crates/rbitcoin-ibd/tests/ibd/peer_book_and_config_tests.rs index e57dae5598..841a74a16d 100644 --- a/crates/rbitcoin-ibd/tests/ibd/peer_book_and_config_tests.rs +++ b/crates/rbitcoin-ibd/tests/ibd/peer_book_and_config_tests.rs @@ -48,7 +48,6 @@ use rbitcoin_ibd::ibd::PeerBookSession; use rbitcoin_log::{info, warn}; use rbitcoin_net::chain::ChainHub; use rbitcoin_net::codec::MAX_HEADERS_RESULTS; -use rbitcoin_net::connect_timeout_for; use rbitcoin_net::error::NetError; use rbitcoin_net::seeds::AddrMan; use std::collections::{HashMap, HashSet}; diff --git a/crates/rbitcoin-net/src/lib.rs b/crates/rbitcoin-net/src/lib.rs index 13dfa880fe..1bdf36302e 100644 --- a/crates/rbitcoin-net/src/lib.rs +++ b/crates/rbitcoin-net/src/lib.rs @@ -79,7 +79,7 @@ pub use net_permissions::{ apply_implicit, parse_whitebind, parse_whitelist, NetPermTable, NetPermissionFlags, WhitebindGrant, WhitelistGrant, DEFAULT_WHITELISTFORCERELAY, DEFAULT_WHITELISTRELAY, }; -pub use netaddr::{connect_timeout_for, is_cjdns_ip, NetAddr, OnlyNet}; +pub use netaddr::{is_cjdns_ip, NetAddr, OnlyNet}; pub use netgroup::netgroup; pub use peer::{ drain_pending_now, flush_tx_invs, force_announce_txid, local_service_flags, run_feeler_timed, diff --git a/crates/rbitcoin-net/src/netaddr.rs b/crates/rbitcoin-net/src/netaddr.rs index 365eb7bad0..063016c793 100644 --- a/crates/rbitcoin-net/src/netaddr.rs +++ b/crates/rbitcoin-net/src/netaddr.rs @@ -6,7 +6,6 @@ use sha3::{Digest, Sha3_256}; use std::fmt; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; use std::str::FromStr; -use std::time::Duration; #[doc(hidden)] pub const B32: &[u8; 32] = b"abcdefghijklmnopqrstuvwxyz234567"; @@ -346,14 +345,6 @@ impl NetAddr { } } -/// I2P STREAM CONNECT waits on tunnel + leaseset lookup; 8s is a clearnet RTT. -pub fn connect_timeout_for(addr: NetAddr, base: Duration) -> Duration { - match addr { - NetAddr::I2p { .. } => base.max(Duration::from_secs(90)), - _ => base, - } -} - #[doc(hidden)] pub fn parse_net_addr(s: &str) -> Result { let Some((host, port_s)) = s.rsplit_once(':') else { diff --git a/crates/rbitcoin-net/src/service.rs b/crates/rbitcoin-net/src/service.rs index 978c9b606a..2bfff054ed 100644 --- a/crates/rbitcoin-net/src/service.rs +++ b/crates/rbitcoin-net/src/service.rs @@ -640,8 +640,8 @@ const REDIAL_INTERVAL: Duration = Duration::from_secs(2); const REDIAL_INTERVAL: Duration = Duration::from_millis(250); /// Bound on the TCP / SOCKS / SAM connect of an outbound dial, the same 8 s -/// as the IBD and tip-follow dials ([`crate::connect_timeout_for`] gives I2P -/// longer). Without it a proxy that accepts and never answers holds the +/// as the IBD and tip-follow dials ([`crate::socks::Dialer::connect_timeout`] +/// gives SOCKS and I2P longer). Without it a proxy that accepts and never answers holds the /// dial, and its [`crate::peers::DialInFlight`] mark, for good, so the /// redial pass would never try that target again. Debug builds keep 8 s. /// The short redial interval is separate. @@ -677,7 +677,7 @@ pub async fn prepare_outbound_session( } }; let stream = tokio::time::timeout( - crate::connect_timeout_for(peer_net, OUTBOUND_CONNECT_TIMEOUT), + dialer.connect_timeout(peer_net, OUTBOUND_CONNECT_TIMEOUT), connect, ) .await diff --git a/crates/rbitcoin-net/src/socks.rs b/crates/rbitcoin-net/src/socks.rs index 5e9a00d722..d297c318dd 100644 --- a/crates/rbitcoin-net/src/socks.rs +++ b/crates/rbitcoin-net/src/socks.rs @@ -3,6 +3,7 @@ use crate::error::NetError; use std::net::SocketAddr; use std::sync::Arc; +use std::time::Duration; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::TcpStream; @@ -78,10 +79,52 @@ pub enum Dialer { onion: Option, randomize: bool, shared_creds: Option, Vec)>>, + /// Least [`Dialer::connect_timeout`] for a dial through this proxy. + connect_floor: Duration, }, } +/// Each isolated SOCKS dial builds its own Tor circuit, and a wave of them +/// queues inside Tor for longer than a clearnet RTT. +const SOCKS_CONNECT_TIMEOUT: Duration = Duration::from_secs(30); + +/// I2P STREAM CONNECT waits on tunnel + leaseset lookup. +const I2P_CONNECT_TIMEOUT: Duration = Duration::from_secs(90); + impl Dialer { + /// Dial bound for `addr` over this dialer: `base` for a direct socket, + /// at least the SOCKS floor through a proxy, at least 90 s for I2P. + /// + /// IBD `dial_batch` and tip-follow wrap connect and handshake in this one + /// bound, so a slow peer is skipped rather than held through a wave. + /// `prepare_outbound_session` bounds only the connect with it, then gives + /// the handshake its own `HANDSHAKE_TIMEOUT`. + pub fn connect_timeout(&self, addr: crate::NetAddr, base: Duration) -> Duration { + match (self, addr) { + (_, crate::NetAddr::I2p { .. }) => base.max(I2P_CONNECT_TIMEOUT), + (Dialer::Socks { connect_floor, .. }, crate::NetAddr::Onion { .. }) + | ( + Dialer::Socks { + proxy: Some(_), + connect_floor, + .. + }, + crate::NetAddr::Ip(_), + ) => base.max(*connect_floor), + _ => base, + } + } + + /// The same dialer with a lower SOCKS floor, for a test that waits out a + /// stalled proxy. + #[doc(hidden)] + pub fn with_connect_floor(mut self, floor: Duration) -> Self { + if let Dialer::Socks { connect_floor, .. } = &mut self { + *connect_floor = floor; + } + self + } + pub fn socks(proxy: SocketAddr, randomize: bool) -> Self { Self::with_proxies(Some(proxy), None, randomize) } @@ -103,6 +146,7 @@ impl Dialer { onion, randomize, shared_creds, + connect_floor: SOCKS_CONNECT_TIMEOUT, } } diff --git a/crates/rbitcoin-net/tests/service.rs b/crates/rbitcoin-net/tests/service.rs index 85ab8c7739..e9acb0d1f5 100644 --- a/crates/rbitcoin-net/tests/service.rs +++ b/crates/rbitcoin-net/tests/service.rs @@ -382,7 +382,8 @@ async fn redial_skips_a_connect_target_still_dialling() { Milestone::NONE, "/rbitcoin:0.1.0(redial)/".into(), 0, - rbitcoin_net::socks::Dialer::socks(proxy_addr, false), + rbitcoin_net::socks::Dialer::socks(proxy_addr, false) + .with_connect_floor(OUTBOUND_CONNECT_TIMEOUT), ) .await .unwrap(); diff --git a/crates/rbitcoin-net/tests/socks.rs b/crates/rbitcoin-net/tests/socks.rs index b54b911124..229a22da42 100644 --- a/crates/rbitcoin-net/tests/socks.rs +++ b/crates/rbitcoin-net/tests/socks.rs @@ -540,6 +540,7 @@ async fn connect_isolated_net_dials_ip_and_onion_and_rejects_the_rest() { onion: None, randomize: true, shared_creds: None, + connect_floor: std::time::Duration::from_secs(30), }; let err = bare .connect_isolated_net(NetAddr::Onion { diff --git a/crates/rbitcoin-node/src/run.rs b/crates/rbitcoin-node/src/run.rs index 7a923672be..051aa7050e 100644 --- a/crates/rbitcoin-node/src/run.rs +++ b/crates/rbitcoin-node/src/run.rs @@ -108,8 +108,8 @@ impl Shutdown { pub const FOLLOW_CONNECT_SECS: u64 = 8; #[doc(hidden)] -pub fn follow_connect_timeout(peer: rbitcoin_net::NetAddr) -> Duration { - rbitcoin_net::connect_timeout_for(peer, Duration::from_secs(FOLLOW_CONNECT_SECS)) +pub fn follow_connect_timeout(dialer: &Dialer, peer: rbitcoin_net::NetAddr) -> Duration { + dialer.connect_timeout(peer, Duration::from_secs(FOLLOW_CONNECT_SECS)) } /// Install SIGTERM / SIGINT (and Ctrl+C) handlers that trip `shutdown`. @@ -827,7 +827,7 @@ pub async fn run_p2p(config: NodeConfig) -> Result<(), NodeError> { if shutdown.requested() { break; } - let to = follow_connect_timeout(*peer); + let to = follow_connect_timeout(&node.dialer(), *peer); tokio::select! { biased; _ = shutdown.cancelled() => { diff --git a/crates/rbitcoin-node/tests/run.rs b/crates/rbitcoin-node/tests/run.rs index 93fd978245..023390e541 100644 --- a/crates/rbitcoin-node/tests/run.rs +++ b/crates/rbitcoin-node/tests/run.rs @@ -358,10 +358,13 @@ fn follow_connect_timeout_i2p_is_longer_than_clearnet() { }; let ip: rbitcoin_net::NetAddr = "127.0.0.1:1".parse().unwrap(); assert_eq!( - follow_connect_timeout(ip), + follow_connect_timeout(&Dialer::Direct, ip), Duration::from_secs(FOLLOW_CONNECT_SECS) ); - assert_eq!(follow_connect_timeout(i2p), Duration::from_secs(90)); + assert_eq!( + follow_connect_timeout(&Dialer::Direct, i2p), + Duration::from_secs(90) + ); } #[test]