net: prevent redundant connections to the same peer #36172

Summary

Fixes #22559.

Two gaps in connection deduplication let a node end up with more than one simultaneous connection to the same peer:

• ConnectNode() checked "are we already connected to this address?" before performing the (potentially slow, e.g. an I2P SAM handshake) connect itself, and only added the new node to m_nodes once the connect succeeded. Two concurrent callers (e.g. ThreadOpenConnections racing an addnode/addconnection RPC) could both pass the check and both spend time connecting to the same address, ending up with duplicate connections to it. This matches the race analyzed in the issue by @vasild and @tryphe, reproducible today via e.g. two near-simultaneous addnode ... onetry calls to the same slow-to-connect (I2P) address.
• CreateNodeFromAcceptedSocket() performed no address-based dedup at all on the accept path, so a peer could open more than one simultaneous inbound connection — the "2 inbound" case reported in the issue.

Fix

• Close the outbound race by having ConnectNode() atomically reserve the target address (in a new m_connecting set, guarded by the existing m_nodes_mutex) before starting the connect, releasing the reservation on failure or once the caller has added the resulting node to m_nodes. AlreadyConnectedToAddressPort()/AlreadyConnectedToAddress() now also consult this set, so other callers (e.g. the RPC dedup checks) see in-flight attempts too.
• For the inbound gap, reject a second connection when the accepted address is already connected/connecting. This is deliberately scoped to I2P only: its inbound source address is the peer's real, non-shareable destination and always uses port 0, so an exact match reliably means the same peer. The same is not true for a Tor hidden-service inbound connection (source masked by the local Tor proxy) or a clearnet IPv4/IPv6 one (source IP can be legitimately shared by distinct peers behind NAT), so both are deliberately left alone — this also avoids affecting the functional test framework, which connects many mock peers from 127.0.0.1.

Test plan

• Added connecting_addr_claim (unit test for the new reservation primitive and its interaction with the existing dedup checks)
• Added create_node_from_accepted_socket_i2p_dedup (confirms a duplicate I2P inbound connection is dropped, a different I2P peer is unaffected, and clearnet peers sharing a source IP are not deduplicated)
• Added connect_node_rejects_in_flight_address (exercises the real ConnectNode() path via the existing ConnectNodePublic test helper, confirming the fix is wired into production code, not just the helper in isolation)
• Full test_bitcoin suite passes locally (763 test cases, no regressions)

🤖 Generated with Claude Code

♻️ Automatically closing for now based on heuristics. Please leave a comment, if this was erroneous. Generally, please focus on creating high-quality, original content that demonstrates a clear understanding of the project's requirements and goals.

📝 Moderators: If this is spam, please replace the title with ., so that the thread does not appear in search results.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36172.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline and AI policy for information on the review process. A summary of reviews will appear here.

<!--5faf32d7da4f0f540f40219e4f7537a3-->