net: give SOCKS dials a 30 s connect bound

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.

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(

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;

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 {

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;

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};

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,

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<NetAddr, NetError> {
     let Some((host, port_s)) = s.rsplit_once(':') else {

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

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<SocketAddr>,
         randomize: bool,
         shared_creds: Option<Arc<(Vec<u8>, Vec<u8>)>>,
+        /// 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,
         }
     }
 

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();

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 {

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() => {

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]