net: support overriding the proxy selection in ConnectNode() #33454

1. 
   vasild commented at 1:57 pm on September 22, 2025: contributor

   Normally ConnectNode() could choose whether to use a proxy and which one. Make it possible to override this from the callers and same for OpenNetworkConnection() - pass down the proxy to ConnectNode().

   Document both functions.

   This is useful if we want to open connections to IPv4 or IPv6 peers through the Tor SOCKS5 proxy.

   ---

   This is part of #29415 Broadcast own transactions only via short-lived Tor or I2P connections. Putting it in its own PR to reduce the size of #29415 and because it does not depend on the other commits from there.

2. DrahtBot added the label P2P on Sep 22, 2025
3. 
   DrahtBot commented at 1:57 pm on September 22, 2025: contributor

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

   Code Coverage & Benchmarks

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

   Reviews

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #32747 (Introduce SockMan (“lite”): low-level socket handling for HTTP by pinheadmz)
   • #32326 (net: improve the interface around FindNode() and avoid a recursive mutex lock by vasild)
   • #29415 (Broadcast own transactions only via short-lived Tor or I2P connections by vasild)
   • #28584 (Fuzz: extend CConnman tests by vasild)
   • #28463 (p2p: Increase inbound capacity for block-relay only connections by mzumsande)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

4. vasild force-pushed on Sep 22, 2025
5. 
   andrewtoth commented at 3:45 pm on September 22, 2025: contributor

   Need to update https://github.com/bitcoin/bitcoin/pull/33454/files#diff-00021eed586a482abdb09d6cdada1d90115abe988a91421851960e26658bed02L2882 strDest -> pszDest to placate tidy. If you’re updating the name anyways, maybe just get rid of the hungarian notation and call it destination?
6. vasild force-pushed on Sep 23, 2025
7. 
   vasild commented at 9:34 am on September 23, 2025: contributor

   504803f909...48c5939cf4: fix tidy, note that also in master there is inconsistency: the prototype of OpenNetworkConnection() in net.h uses strDest whereas the implementation in net.cpp uses pszDest. Resolved by this PR.

   If you’re updating the name anyways, maybe just get rid of the hungarian notation and call it destination?

   Yes, I think this would be very nice to do. I avoided it because it would bloat the diff as a lot of lines inside the body of the functions will be touched. Then there are also the other parameters addrConnect and fCountFailure that would warrant a rename. Further, addrConnect and pszDest both mean the same thing and should both be called destination :-X maybe combine them into one variable std::variant<CAddress, const char*> destination? Either way I would like to keep this PR minimal, only containing what is needed for #29415.

8. in src/net.cpp:401 in 48c5939cf4

   397+CNode* CConnman::ConnectNode(CAddress addrConnect,
   398+                             const char* pszDest,
   399+                             bool fCountFailure,
   400+                             ConnectionType conn_type,
   401+                             bool use_v2transport,
   402+                             std::optional<Proxy> proxy_arg)
   

   ---

   ---

   

   sipa commented at 12:17 pm on September 23, 2025:

   Pass by reference? Proxy is pretty large (80 bytes).

   ---

   

   vasild commented at 1:29 pm on September 23, 2025:

   Right, done!
9. net: support overriding the proxy selection in ConnectNode()

   Normally `ConnectNode()` could choose whether to use a proxy and which
   one. Make it possible to override this from the callers and same for
   `OpenNetworkConnection()` - pass down the proxy to `ConnectNode()`.
   
   Document both functions.
   
   This is useful if we want to open connections to IPv4 or IPv6 peers
   through the Tor SOCKS5 proxy.

   ec7c215345
10. vasild force-pushed on Sep 23, 2025
11. 
    vasild commented at 1:28 pm on September 23, 2025: contributor

    48c5939cf4...ec7c215345: do #33454 (review)

Contributors
vasild DrahtBot andrewtoth sipa

Labels
P2P