Skip to content

Reclaim addresses of connect failures not attributable to the address. - #942

Open
echennells wants to merge 1 commit into
libbitcoin:masterfrom
echennells:fix/reclaim-unattributable-connect
Open

echennells wants to merge 1 commit into
libbitcoin:masterfrom
echennells:fix/reclaim-unattributable-connect

Conversation

@echennells

@echennells echennells commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

A local network interruption fails connects with ENETUNREACH, ENETDOWN, EADDRNOTAVAIL, ENOLINK or, with the router gone, EHOSTUNREACH. All mapped to connect_failed, and a failed connect has no socket, so reclaim(ec, socket) never restored the address and an interruption drained the pool. Requested in #924.

This splits net_unreachable, host_unreachable and connection_refused out of connect_failed. A connect that fails unreachable (or the socks counterparts) is restored only if every member of its batch failed as unreachable, implying a local outage, and only into free pool capacity; otherwise it is dropped as before. Members that drew no address are not counted. maybe_reclaim adds the unreachable codes and insufficient_buffer, so a connected address that later stops with one is restored, and handle_one restores other socketless maybe_reclaim failures.

Tests cover the mappings and batches that are wholly unreachable, refused, connected, timed out, empty or near a full pool.

Not addressed: a refused or unresolvable socks proxy drops the target, and with the gateway gone a timeout often arrives before EHOSTUNREACH, so such batches still drop.

@evoskuil

evoskuil commented Oct 4, 2026

Copy link
Copy Markdown
Member

Splitting these codes out of connect_failed is right, and the no-socket restore in handle_one closes a real gap. Some concerns:

  1. Reclaiming unreachable codes in all cases (session_outbound.cpp maybe_reclaim). Dropping on ENETUNREACH/EHOSTUNREACH is also how the pool sheds addresses this host can never reach. settings::connectable() accepts any ipv6/cjdns address whether or not this host has a route to it. On an ipv4-only host with gossip_ipv6=true, every ipv6 entry fails immediately with net_unreachable. Before this change those addresses were dropped. Now they're restored forever, and each one keeps taking a connect slot. EHOSTUNREACH is similar: remote routers send it for dead ipv6 hosts, so it was one of the few ways dead entries left the pool. An outage is temporary, but a missing route is permanent. Can reclaim be limited to failures that look local? For example, restore only when every member of the batch failed with an unreachable code, and otherwise drop as before.

  2. address_not_available → net_unreachable (error.cpp). On Linux, EADDRNOTAVAIL from connect is local (no source address, or ephemeral ports exhausted). On Windows, WSAEADDRNOTAVAIL from connect also means the remote address is invalid (e.g. unspecified), which is attributable to the address. That's harmless only as long as such addresses never reach the pool.

On the allow-list question: keep the allow-list. Over-reclaiming is the risk here, and a deny-list would reclaim any future code by default.

@echennells
echennells force-pushed the fix/reclaim-unattributable-connect branch from e551868 to d96a06f Compare October 6, 2026 18:41
@echennells

Copy link
Copy Markdown
Contributor Author

Thanks, reworked.

  1. Unreachable connects are now restored only if every member of the batch failed as unreachable (members that drew no address aren't counted), and only into free pool capacity; otherwise dropped as before. A connected address that later stops unreachable is still restored.

    A timeout counts against restoring, which misses one outage: with the gateway gone, connects wait about 3s for ARP before EHOSTUNREACH, while the connect timer is 2.5-5s, so about 73% of 5-member batches see a timeout first and drop. Should timeouts count neither way? That's one line (only a connection or refusal marks the batch). The cost: an unreachable address whose batch-mates all time out gets another try. Of 4,637 gossiped ipv4 addresses I sampled, 17% connected or refused, so it would take about 1.9 attempts instead of 1.

    With connect_batch_size = 1 every unreachable failure qualifies, so this always restores it, and a pool whose remaining entries are all unroutable never empties, so Seed the address pool when an outbound connection finds no address. #924 never reseeds. Should a single-member batch drop instead?

  2. WSAEADDRNOTAVAIL from connect means an unspecified remote (Winsock docs, and on windows-latest specified but bad remotes returned timeout, unreachable or refused instead). Neither an unspecified address nor port 0 can be pooled: excluded() rejects !is_specified for gossip and seed (settings.cpp:563), and hosts::push rejects it on load.

Kept the allow-list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants