feat: Tor hidden service management and outbound .onion peer connections#2036
feat: Tor hidden service management and outbound .onion peer connections#2036sat-engineer wants to merge 1 commit intogetAlby:masterfrom
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds Tor hidden-service support: new tor package, AppConfig Tor fields, LDK integration to configure SOCKS and start/stop hidden service, API and model extensions for Tor status/address, frontend routes/UI for Tor settings and channel Tor URIs. Changes
Sequence DiagramsequenceDiagram
participant App as Application Init
participant LDK as LDKService
participant TorSvc as Tor Service
participant TorCtrl as Tor Control Port
participant Onion as Onion Service
App->>LDK: Start with AppConfig
alt LDKTorEnabled == true
LDK->>LDK: Configure outbound SOCKS proxy
LDK->>TorSvc: NewService()
LDK->>LDK: Start LDK node / await sync
LDK->>TorSvc: Start(Config)
TorSvc->>TorCtrl: Connect + Authenticate (retries)
TorSvc->>TorSvc: loadOrCreateKey -> persist key
TorSvc->>TorCtrl: ADD_ONION (map onion port -> local port)
TorCtrl-->>TorSvc: Onion address
TorSvc->>LDK: Report onion address/port
LDK->>LDK: Attach torAddress/torPort to NodeConnectionInfo
else
LDK->>LDK: Skip Tor initialization
end
LDK->>TorSvc: Stop() on shutdown (if running)
TorSvc->>TorCtrl: DEL_ONION + Close
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@go.mod`:
- Around line 272-274: The go.mod currently contains a replace directive
pointing to the personal fork github.com/sat-engineer/ldk-node-go which must be
reverted once the upstream PR is merged; remove or change the replace directive
that maps github.com/getAlby/ldk-node-go => github.com/sat-engineer/ldk-node-go
v0.0.0-... and instead reference the official github.com/getAlby/ldk-node-go
module (or remove the replace entirely) and set the dependency to the
appropriate upstream semantic version or commit from the merged
getAlby/ldk-node-go#32 to eliminate the forked supply-chain reference.
In `@lnclient/ldk/ldk.go`:
- Around line 194-209: When LDK_TOR is explicitly enabled
(cfg.GetEnv().LDKTorEnabled) and a SOCKS host is configured
(cfg.GetEnv().LDKTorSocksHost), do not silently continue if
builder.SetTorProxyAddress(torSocksAddr) fails; instead surface the failure to
stop startup so the node won't use clearnet. Update the block around
builder.SetTorProxyAddress to return or propagate an error (or call a fatal
exit) when SetTorProxyAddress returns an error, including the error and
socks_addr in the returned error/context so callers (or UI) can surface it; keep
the resolution logic that tries net.LookupHost but ensure any failure from
SetTorProxyAddress causes startup to fail when cfg.GetEnv().LDKTorEnabled is
true.
🧹 Nitpick comments (4)
tor/tor.go (3)
67-116:Start()holds the mutex for the entire retry loop (potentially ~60 seconds).The mutex is acquired at Line 68 and held through the retry loop (30 retries × 2s sleep = ~60s worst case). During this time, any call to
GetOnionAddress(),IsRunning(), orStop()from another goroutine (e.g., health check, API handler) will block. Consider either: (a) acquiring the lock only to check/setrunningstate and releasing it before the I/O, or (b) switching tosync.RWMutexwith a separate "starting" state.In practice, the current LDK integration calls these sequentially, so this is unlikely to cause issues today.
152-156: Dead code:key == nilis unreachable after successfulloadOrCreateKey.
loadOrCreateKey(Line 129) either returns a non-nil key or an error. After the error check on Line 130,keyis guaranteed non-nil, making the conditionresp.Key != nil && key == nilalways false.
265-273:encodeOnionV3is misleading — it does not produce a v3 onion address.The function base64-encodes only the first 8 bytes of the public key, which is not a valid onion v3 address (those are base32-encoded with checksum + version). The inline comment acknowledges this, but the function name suggests otherwise. Consider renaming to something like
pubkeyPrefixortruncatedPubkeyForLogto avoid confusion during future maintenance.Suggested rename
-// encodeOnionV3 derives a v3 .onion address (without .onion suffix) from a public key -func encodeOnionV3(pubKey tored25519.PublicKey) string { +// pubkeyLogLabel returns a short identifier from a public key for logging purposes. +// The actual onion address is provided by Tor in the AddOnion response. +func pubkeyLogLabel(pubKey tored25519.PublicKey) string {lnclient/ldk/ldk.go (1)
353-383: Tor hidden service startup — non-fatal failure is reasonable for inbound, but consider logging more prominently.The "continuing without Tor" fallback on Line 378 is acceptable for the inbound hidden service (it's about reachability, not privacy). The implementation correctly parses the listening port from the config and handles IPv6 addresses via
LastIndex.One minor note:
OnionServicePortis hardcoded to9735(Line 374) whileTorPortis also hardcoded to9735at Line 1070. Consider extracting a constant to keep these in sync.Extract constant for the onion service port
You could reuse the existing
albytor.DefaultOnionServicePortconstant fromtor/tor.go:torCfg := &albytor.Config{ ... - OnionServicePort: 9735, + OnionServicePort: albytor.DefaultOnionServicePort, ... }And in
GetNodeConnectionInfo:- nodeConnectionInfo.TorPort = 9735 + nodeConnectionInfo.TorPort = albytor.DefaultOnionServicePort
|
|
||
| // Tor SOCKS5 outbound support for .onion peer connections | ||
| replace github.com/getAlby/ldk-node-go => github.com/sat-engineer/ldk-node-go v0.0.0-20260207201420-ed1bf13381b8 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the go.mod file to verify the details mentioned
cat -n go.mod | head -20
echo "---"
cat -n go.mod | tail -20
echo "---"
# Check specific lines around the replace directive
sed -n '270,280p' go.modRepository: getAlby/hub
Length of output: 2308
🌐 Web query:
cretz/bine v0.2.0 Go Tor SOCKS5
💡 Result:
github.com/cretz/bine v0.2.0 is a Go library to start/control Tor and then dial outbound connections through Tor’s SOCKS5 proxy. The tor package exposes a Tor.Dialer(...) helper that uses Tor’s SOCKS5 proxy (it can auto-discover the proxy address if you don’t set one). [1] The default StartConf also adds --SocksPort auto unless you disable it via NoAutoSocksPort. [2]
Minimal client example (SOCKS5 via Tor)
package main
import (
"context"
"io"
"log"
"net/http"
"time"
"github.com/cretz/bine/tor"
)
func main() {
// Start Tor (spawns a tor process unless you use an embedded ProcessCreator)
t, err := tor.Start(nil, nil)
if err != nil { log.Fatal(err) }
defer t.Close()
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
defer cancel()
// Create a dialer that goes through Tor's SOCKS5 proxy
d, err := t.Dialer(ctx, nil) // DialConf nil => proxy address is looked up
if err != nil { log.Fatal(err) }
// Use it with net/http
tr := &http.Transport{DialContext: d.DialContext}
c := &http.Client{Transport: tr}
resp, err := c.Get("https://check.torproject.org/")
if err != nil { log.Fatal(err) }
defer resp.Body.Close()
b, _ := io.ReadAll(resp.Body)
log.Printf("status=%s bytes=%d", resp.Status, len(b))
}Notes:
- Tor’s SOCKS implementation supports Tor-specific SOCKS extensions like
RESOLVE/RESOLVE_PTR(useful for remote DNS resolution through Tor). [3] - v0.2.0 is the upstream tagged release (May 31, 2021). [4]
Sources: [1] [2] [3] [4]
Merge personal fork upstream before shipping.
The replace directive targets github.com/sat-engineer/ldk-node-go, a personal fork. This introduces a supply-chain risk: if the fork is deleted, force-pushed, or the account is compromised, builds will break or be silently altered. Once getAlby/ldk-node-go#32 is merged upstream, update this to point to the official getAlby module rather than the fork.
🤖 Prompt for AI Agents
In `@go.mod` around lines 272 - 274, The go.mod currently contains a replace
directive pointing to the personal fork github.com/sat-engineer/ldk-node-go
which must be reverted once the upstream PR is merged; remove or change the
replace directive that maps github.com/getAlby/ldk-node-go =>
github.com/sat-engineer/ldk-node-go v0.0.0-... and instead reference the
official github.com/getAlby/ldk-node-go module (or remove the replace entirely)
and set the dependency to the appropriate upstream semantic version or commit
from the merged getAlby/ldk-node-go#32 to eliminate the forked supply-chain
reference.
| // Configure Tor SOCKS proxy for outbound .onion connections | ||
| if cfg.GetEnv().LDKTorEnabled && cfg.GetEnv().LDKTorSocksHost != "" { | ||
| // Resolve hostname to IP — SetTorProxyAddress requires a numeric IP:port | ||
| socksHost := cfg.GetEnv().LDKTorSocksHost | ||
| if addrs, err := net.LookupHost(socksHost); err == nil && len(addrs) > 0 { | ||
| socksHost = addrs[0] | ||
| } else if err != nil { | ||
| logger.Logger.WithError(err).WithField("host", socksHost).Warn("Failed to resolve Tor SOCKS host, using raw hostname") | ||
| } | ||
| torSocksAddr := fmt.Sprintf("%s:%d", socksHost, cfg.GetEnv().LDKTorSocksPort) | ||
| if err := builder.SetTorProxyAddress(torSocksAddr); err != nil { | ||
| logger.Logger.WithError(err).WithField("socks_addr", torSocksAddr).Error("Failed to set Tor SOCKS proxy address") | ||
| } else { | ||
| logger.Logger.WithField("socks_addr", torSocksAddr).Info("Configured Tor SOCKS proxy for outbound .onion connections") | ||
| } | ||
| } |
There was a problem hiding this comment.
Silent fallback when SOCKS proxy setup fails could compromise user privacy.
When LDK_TOR_ENABLED=true and LDK_TOR_SOCKS_HOST is set, the user expects outbound connections to route through Tor. If SetTorProxyAddress fails (Line 205), the error is only logged and the node starts without Tor proxy — outbound connections will go over clearnet. A user relying on Tor for anonymity wouldn't know their traffic is unprotected.
Consider making this failure fatal (return an error) when Tor is explicitly enabled, or at minimum emitting a prominent warning/event that the UI can surface.
Proposed fix: fail startup when SOCKS proxy cannot be set
torSocksAddr := fmt.Sprintf("%s:%d", socksHost, cfg.GetEnv().LDKTorSocksPort)
if err := builder.SetTorProxyAddress(torSocksAddr); err != nil {
- logger.Logger.WithError(err).WithField("socks_addr", torSocksAddr).Error("Failed to set Tor SOCKS proxy address")
- } else {
+ logger.Logger.WithError(err).WithField("socks_addr", torSocksAddr).Error("Failed to set Tor SOCKS proxy address")
+ return nil, fmt.Errorf("tor is enabled but SOCKS proxy setup failed: %w", err)
+ }
- logger.Logger.WithField("socks_addr", torSocksAddr).Info("Configured Tor SOCKS proxy for outbound .onion connections")
- }
+ logger.Logger.WithField("socks_addr", torSocksAddr).Info("Configured Tor SOCKS proxy for outbound .onion connections")🤖 Prompt for AI Agents
In `@lnclient/ldk/ldk.go` around lines 194 - 209, When LDK_TOR is explicitly
enabled (cfg.GetEnv().LDKTorEnabled) and a SOCKS host is configured
(cfg.GetEnv().LDKTorSocksHost), do not silently continue if
builder.SetTorProxyAddress(torSocksAddr) fails; instead surface the failure to
stop startup so the node won't use clearnet. Update the block around
builder.SetTorProxyAddress to return or propagate an error (or call a fatal
exit) when SetTorProxyAddress returns an error, including the error and
socks_addr in the returned error/context so callers (or UI) can surface it; keep
the resolution logic that tries net.LookupHost but ensure any failure from
SetTorProxyAddress causes startup to fail when cfg.GetEnv().LDKTorEnabled is
true.
…ections Add Go-native Tor hidden service management and wire up SOCKS5 proxy support for outbound .onion peer connections via LDK-node. Backend: - New tor/ package: manages Tor hidden service via control port using cretz/bine library. Handles key persistence, retry logic, and lifecycle management. - Wire up builder.SetTorProxyAddress() in LDK service init when LDK_TOR_SOCKS_HOST is set, with DNS resolution for Docker hostnames. - Expose onion address in node connection info API. - New env vars: LDK_TOR_ENABLED, LDK_TOR_CONTROL_HOST/PORT/PASSWORD, LDK_TOR_TARGET_HOST, LDK_TOR_SOCKS_HOST/PORT. Frontend: - Tor settings page showing hidden service status and onion address. - Tor URI copy button on Channels page. - Settings sidebar entry for Tor (LDK backend only). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
sat-engineer
force-pushed
the
feat/tor-socks-outbound
branch
from
6df3742 to
7869019
Compare
|
Reopening with clean history |
1 participant
Summary
tor/package).onionpeer connections via LDK-nodeDetails
Depends on: getAlby/ldk-node-go#32
Backend
New
tor/package (274 lines + 86 lines tests):cretz/binelibraryADD_ONION— forwards.onion:9735to LDK's listenerLDK integration (
lnclient/ldk/ldk.go):builder.SetTorProxyAddress()whenLDK_TOR_SOCKS_HOSTis setnet.LookupHost()(Rust FFI needs numericIP:port)tor.Servicefor inbound hidden service lifecycle.onionaddress inGetNodeConnectionInfo()New env vars (
config/models.go):LDK_TOR_ENABLED— enable Tor integration (default: false)LDK_TOR_CONTROL_HOST/PORT— Tor control port addressLDK_TOR_CONTROL_PASSWORD— Tor control port authLDK_TOR_TARGET_HOST— container hostname for hidden service forwardingLDK_TOR_SOCKS_HOST/PORT— SOCKS5 proxy for outbound.onionconnectionsFrontend
TorSettings.tsx) — shows hidden service status and onion address with copy buttontorAddress,torPort,torEnabledfieldsArchitecture
The
tor/Go package handles inbound (hidden service registration via control port), while the Rust-side SOCKS5 proxy handles outbound (connecting to.onionpeers). This separation gives Go-side access to the onion address for the API/UI while keeping the performance-critical peer connection logic in Rust.Test plan
go build ./...passesgo test ./tor/passes (4 tests).onionto.onion)🤖 Generated with Claude Code
Summary by CodeRabbit