[portlandhodl]

Problem

Each isolated SOCKS dial (--proxy-randomize, the default) builds its own Tor circuit. On a healthy Tor client:

parallel isolated SOCKS connects to one good peer	min	median	max	over 8 s
1	0.2 s	0.2 s	0.2 s	0
32	4.0 s	7.0 s	9.4 s	8

An IBD wave is 32 dials, and its bound covers connect plus handshake, so 8 s timed out whole waves. Onion dials used the same 8 s.

Fix

Dialer::connect_timeout(addr, base) replaces the free connect_timeout_for at every caller (IBD dial_batch, prepare_outbound_session, tip-follow):

• clearnet through --proxy, and onion: base.max(connect_floor). connect_floor is a Dialer::Socks field, 30 s from with_proxies.
• I2P: base.max(90 s) (unchanged)
• direct and CJDNS: base (unchanged)

Scope, now in the rustdoc: IBD dial_batch and tip-follow wrap connect and handshake in this one bound, as they did with 8 s, so a slow peer is skipped rather than held through a wave. prepare_outbound_session bounds only the connect with it and keeps the 60 s HANDSHAKE_TIMEOUT for the handshake.

Tests

overlay_and_socks_connects_outlast_a_direct_one (crates/rbitcoin-ibd/tests/ibd/dial.rs) pins the production values: 30 s for proxied clearnet and onion; 90 s for I2P, direct or proxied; base for direct, CJDNS and an onion-only dialer with a clearnet address; a base above a floor (120 s) kept; and with_connect_floor lowering it. redial_skips_a_connect_target_still_dialling (crates/rbitcoin-net/tests/service.rs) waits out a stalled proxy with a lowered floor (#[doc(hidden)] with_connect_floor), not a cfg(test) constant.

Rebased onto the crate-layout move (#1097) as one commit: the earlier two-commit history used a cfg(test) floor that can't reach the tests now under tests/.

🤖 Generated with Claude Code

[rearden-grok]

Review

The three production callers of connect_timeout_for now go through Dialer::connect_timeout, and the free function is gone. Proxied clearnet (proxy: Some) and onion addresses use base.max(SOCKS_CONNECT_TIMEOUT). I2P stays base.max(90s). Direct IP and CJDNS stay on base. Duration::max does not shrink a larger base. An onion-only dialer still connects clearnet directly, and that address stays on base. I2P is matched before the SOCKS arms, so a proxied dialer still gives I2P 90s.

Severity	Location	Finding
Medium	crates/rbitcoin-net/src/socks.rs:89	The 30s constant is cfg(not(test)). Tests compile 3s. overlay_and_socks_connects_outlast_a_direct_one asserts a SOCKS result > 1s, so a production floor of 8s or 300s still passes. The test never calls dial_batch, and it does not cover CJDNS, a proxied I2P address, or a base already above the floor.
Medium	crates/rbitcoin-net/src/ibd/dial.rs:306	IBD wraps spawn_peer (SOCKS connect, then connect_and_handshake_timed with HANDSHAKE_TIMEOUT of 60s) in this single timer. Handshake time comes out of the same 30s. prepare_outbound_session applies the helper to the connect future only and then allows another 60s of handshake. Tip-follow uses the helper as the outer bound around follow_from_net_as (run.rs), so a connect that uses most of the 30s leaves the handshake inside that same window.
Low	crates/rbitcoin-net/src/ibd/mod.rs:361	The initial dial awaits the whole batch. Default max_outbound is 16, so the wave is 32 tasks, and slots are admitted after every dial finishes. One unanswered SOCKS peer holds the wave for the full floor and holds one fd per dial. These are tokio tasks. Later redials are already off the IBD loop.

Call sites on the parent:

Parent	Head
dial_batch	dialer.connect_timeout(addr, connect_timeout)
prepare_outbound_session	dialer.connect_timeout(peer_net, OUTBOUND_CONNECT_TIMEOUT)
follow_connect_timeout	dialer.connect_timeout(peer, 8s)

fuzz uses TcpStream::connect_timeout, which is a different function. The 3s test constant is what lets redial_skips_a_connect_target_still_dialling finish: its deadline is OUTBOUND_CONNECT_TIMEOUT * 3 (6s under cfg(test)), and a proxied dial now waits max(2s, 3s).

Pin the production floor with an explicit 30s expectation outside cfg(test) (or a const the test can see), and decide whether the IBD and tip-follow timers should cover connect only, with the handshake on its own 60s budget the way prepare_outbound_session already does.

[portlandhodl]

The coverage failures here (isolated_mutated_head_punishes_its_sender_when_lookup_runs_ahead, then batched_pin_engine_fault_retries_one_block_at_a_time) are not from this change: both show an IBD stall on a BadPrev that races its parent commit. The fix is stacked on this PR in #1105.

[portlandhodl]

Thanks. Addressed in 61fbe47d:

Finding	Change
Medium, socks.rs:89: floor not pinned	The cfg(test) constant is gone. The floor is a Dialer::Socks field, connect_floor, always 30 s from with_proxies. The test asserts 30 s exactly for proxied clearnet and onion, 90 s for I2P through a proxy, base for direct and CJDNS, and that a base above the floor (120 s) is kept. redial_skips_a_connect_target_still_dialling lowers the floor with the test-only with_connect_floor.
Medium, dial.rs:306: connect vs handshake scope	Decision: keep IBD dial_batch and tip-follow bounding connect and handshake together, as they did at 8 s. An IBD wave admits its slots only after every dial returns, so a connect-only bound plus a separate 60 s handshake would let one slow peer hold a whole wave for up to 90 s. A peer that needs more than 30 s for both through Tor is a poor download peer. This is now documented on Dialer::connect_timeout, including that prepare_outbound_session bounds only the connect.
Low, ibd/mod.rs:361: the wave awaits every dial	Agreed, and it predates this PR. I left it alone here.

The coverage failures on this PR come from a separate IBD race, fixed in #1105 (stacked on this).

[portlandhodl]

Rebased onto master (a953d8ca, after #1097). The two commits are now one (1094e987): the first commit relied on a cfg(test) floor, which can no longer reach the tests now under tests/, so it would not pass on its own. The content is the reviewed follow-up: a Dialer::Socks connect_floor field, with with_connect_floor now #[doc(hidden)] pub for the integration tests. #1105 is restacked on it.

[rearden-grok]

Review

1094e987 replaces the cfg(test) 3s constant. connect_floor is a Dialer::Socks field with no default, set to 30s in with_proxies, which is what production dialers use. A Socks { ... } literal that omits it does not compile. with_connect_floor is doc(hidden) and only the stalled-proxy redial test and the floor unit check call it.

overlay_and_socks_connects_outlast_a_direct_one asserts equality with 30s for proxied clearnet and onion, 90s for I2P on a direct or proxied dialer, base for direct, CJDNS, and onion-only clearnet, and keeps a 120s base. The three former connect_timeout_for callers go through Dialer::connect_timeout. IBD dial_batch and tip-follow still wrap connect and handshake in that one bound; the rustdoc says so, and prepare_outbound_session still bounds only the connect before the separate 60s handshake.