p2p: add optional SOCKS5 proxy support for outbound peer dials #33346
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds optional SOCKS5 proxy support for outbound peer connections in geth's p2p networking layer. When enabled via the --use-proxy CLI flag, peer dials are routed through a SOCKS5 proxy specified by the ALL_PROXY or all_proxy environment variables. This feature is useful for running nodes behind network restrictions or for privacy-enhanced operations.
Key changes:
- Adds
UseProxyconfiguration field to control proxy usage - Implements proxy-aware dialing in
tcpDialerwith context support - Provides CLI flag
--use-proxyfor user control
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| p2p/config.go | Adds UseProxy boolean field to enable proxy support in p2p configuration |
| p2p/dial.go | Implements proxy dialing logic using golang.org/x/net/proxy with context-aware fallback |
| p2p/server.go | Passes UseProxy config value to tcpDialer during dial scheduler setup |
| cmd/utils/flags.go | Defines --use-proxy CLI flag and wires it to p2p configuration |
| cmd/geth/main.go | Registers the UseProxyFlag with the geth command |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else { | ||
| log.Warn("Proxy dialer does not support context, falling back to direct", "addr", addr.String()) | ||
| } |
There was a problem hiding this comment.
When the proxy dialer doesn't support DialContext, the code logs a warning and falls back to direct connection without actually using the non-context-aware proxy dialer. This fallback behavior is inconsistent with the user's intention to use a proxy. Consider either returning an error when UseProxy is true but the proxy doesn't support context, or actually calling the non-context-aware Dial method (though this would lose context cancellation support). The current implementation silently ignores the proxy requirement.
| if t.useProxy { | ||
| log.Debug("Dialing peer via proxy", "direct", proxyDialer == proxy.Direct, "addr", addr.String()) | ||
| if v, ok := proxyDialer.(dialerWithContext); ok { | ||
| return v.DialContext(ctx, "tcp", addr.String()) | ||
| } else { | ||
| log.Warn("Proxy dialer does not support context, falling back to direct", "addr", addr.String()) | ||
| } | ||
| } |
There was a problem hiding this comment.
The new proxy dialing functionality lacks test coverage. The codebase has comprehensive tests for the dialing logic in dial_test.go, but there are no tests verifying the proxy behavior. Consider adding tests to cover: (1) successful proxy connections when useProxy is true, (2) fallback to direct connection when proxy dialer doesn't support context, (3) behavior when proxy environment variables are not set, and (4) interaction with the existing dial scheduler.
| DialContext(ctx context.Context, network, address string) (net.Conn, error) | ||
| } | ||
|
|
||
| var proxyDialer = proxy.FromEnvironment() |
There was a problem hiding this comment.
The global variable proxyDialer is initialized once at package level and accessed by all tcpDialer instances. This could lead to a race condition if the environment variable changes at runtime or if different instances need different proxy configurations. Consider making proxyDialer a field of tcpDialer that is initialized when the dialer is created, or use sync.Once to ensure thread-safe initialization if shared state is intended.
- Add `UseProxy` field to `p2p.Config` and corresponding `--use-proxy` CLI flag. - Implement proxy-aware `tcpDialer` (uses `golang.org/x/net/proxy` and attempts context-aware DialContext when supported). - Wire `UseProxy` through `Server` to the dialer so peer dials can go via SOCKS5 proxy defined by `ALL_PROXY`/`all_proxy`. - Minor debug `start.sh` for running geth with `--use-proxy`. Signed-off-by: cloorc <[email protected]>
1 participant
UseProxyfield top2p.Configand corresponding--use-proxyCLI flag.tcpDialer(usesgolang.org/x/net/proxyand attempts context-aware DialContext when supported).UseProxythroughServerto the dialer so peer dials can go via SOCKS5 proxy defined byALL_PROXY/all_proxy.start.shfor running geth with--use-proxy.This will fix #33345