feat(electrum): route onion servers through isolated Tor

[ethicnology]

Stacked on the feat/tor-recoverbull PR. Review that one first.

The second Tor consumer moves onto bull_tor: an Electrum server reached over .onion now gets its own isolated session, separate from RecoverBull's.

ElectrumServerUrl becomes a value object that parses and validates a server address and answers whether it is an onion host, instead of that decision being made from string checks at the call sites. ElectrumSocketConnector performs the connection, over Tor or clear, and ElectrumTorSessionPort is the capability the Electrum layer depends on so it never reaches into bull_tor's data layer.

ElectrumFallbackException makes the fallback path explicit: when an onion server cannot be reached, the caller learns that the fallback happened rather than silently getting a clearnet connection. The connectivity and server-status adapters are updated accordingly, and the Tor proxy error banner now reflects the real state.

Verified: whole-project analyze clean, and the Electrum suites pass, including the new electrum_server_url_test.dart, electrum_socket_connector_test.dart, electrum_tor_session_adapter_test.dart and the connectivity adapter tests.

[claude]

Claude finished @ethicnology's task in 8m 50s —— View job

---

PR Review: route onion Electrum servers through isolated Tor

• Gather context (diff vs feat/tor-recoverbull, no CLAUDE.md in repo)
• Review new value objects & connector (ElectrumServerUrl, ElectrumSocketConnector)
• Review Tor session port/adapter and DI wiring
• Review fallback semantics & connectivity/server-status adapters
• Post findings (inline comments not permitted in this environment — gh api calls to the PR review-comments endpoint require approval, so all findings are anchored below instead)

Summary

The architecture is solid: ElectrumServerUrl centralizes the scattered string checks, the connector's SOCKS5 domain-name trick is well-documented and pinned by a wire-bytes test, ElectrumTorRoute.close() is idempotent, routes are reliably closed in finally blocks, and the DNS-leak refusal for unproxied onions is placed at the right chokepoint. I found one behavioral regression that needs an explicit decision, three correctness bugs, and two minor issues.

---

1. ⚠️ Privacy regression: clearnet Bitcoin servers are no longer routed through Orbot when useTorProxy is enabled

lib/core/electrum/adapters/electrum_servers_adapter.dart:70 (removed block, was lines 40–43)

The old adapter applied socks5 = settings.socks5 ?? '127.0.0.1:<torProxyPort>' to every Bitcoin connection when the Tor proxy toggle was on. Now ElectrumTorSessionAdapter.open() (electrum_tor_session_adapter.dart:24) returns null for any non-onion server, so a user who enabled Orbot to hide their IP from the default clearnet Electrum servers silently loses that protection on update — sync, tx fetch, and connectivity checks all go direct. The new test pins this ("Orbot enabled leaves a clearnet Bitcoin server direct"), so it looks deliberate, but neither the PR description nor user-facing copy mentions that the toggle's meaning changed from "all Bitcoin traffic" to "onion servers only". Please either confirm and document this, or keep routing clearnet Bitcoin through the external proxy when externalProxyEnabled is true. Fix this →

2. 🐛 Onion connectivity checks get a 5-second budget — false "offline" + spurious Tor error banner

lib/core/status/interface_adapters/adapter/electrum_connectivity_adapter.dart:41

checkElectrum(timeout: connection.timeout) overrides ServerStatusAdapter._resolveTimeout's 30s onion default, and the seeded default for ElectrumSettings.timeout is 5 seconds (lib/core/storage/database_seeds.dart:67). Five seconds must cover SOCKS handshake + Tor circuit build + TLS + the JSON-RPC round trip — the exact situation ElectrumRemoteDatasource deliberately special-cases with its 30s _requestTimeout ("the user's clearnet timeout is not a sensible ceiling there"). With default settings, onion connectivity probes will time out, report offline, and trip the Tor error banner even when Tor is healthy. The new test misses this because its fixture uses timeout: 30. Suggest omitting timeout here (or only passing it when no proxy is in play). Fix this →

3. 🐛 An embedded Tor open() failure becomes a hard stop for callers that narrow isTransient

lib/core/electrum/adapters/electrum_servers_adapter.dart:80 and :104-107

_torSessionPort.open() can throw (embedded Tor bootstrap failure — arguably the most likely way an onion server becomes unusable). That exception is neither OnionServerWithoutTorException nor the caller's own transient type, so for ElectrumConnectivityAdapter (isTransient: (e) => e is _ElectrumServerOfflineException) the fallback loop rethrows immediately: remaining servers in the active set — including a healthy clearnet default — are never tried, and connectivity reports offline. This contradicts the comment right there: "An unroutable onion server is skipped, not fatal." Suggest catching open() failures per attempt and rethrowing as OnionServerWithoutTorException so a failed route build skips to the next server exactly like a missing route does. Fix this →

4. 🐛 An empty-string socks5 now fails every fetch; it used to be ignored

lib/core/electrum/frameworks/drift/datasources/electrum_remote_datasource.dart:62-66

TorProxyEndpoint.tryParse('') returns null, so socks5: '' throws "unusable SOCKS5 proxy". Empty strings evidently occur in persisted settings — lib/core/blockchain/data/datasources/bdk_bitcoin_blockchain_datasource.dart:35 explicitly guards with socks5?.isNotEmpty == true, and _isUnroutableOnion also treats empty as "no proxy". Since the same settings.socks5 is stamped onto every connection in the active set, a persisted '' makes all servers fail this path (the old datasource ignored socks5 entirely). The same applies in electrum_connectivity_adapter.dart:29-34, where '' is treated as a malformed proxy and marks every server offline. Suggest normalizing empty/whitespace to null before parsing, keeping the throw for genuinely malformed non-empty values. Fix this →

5. Nit: dead socks5 parameter

lib/core/electrum/frameworks/drift/datasources/electrum_remote_datasource.dart:28

fetch({..., String? socks5}) is never read — the proxy comes from connection.socks5, and the only caller (drift_electrum_transaction_repository.dart:18) doesn't pass it. It invites a future caller to pass a value that's silently ignored; remove it.

6. Nit: hostname proxies fail with ArgumentError

lib/core/electrum/data/electrum_socket_connector.dart:40

InternetAddress(proxy.host) accepts only IP literals. TorProxyEndpoint.tryParse happily produces host: 'localhost' from a user-typed advanced-options value like localhost:9050, so such a proxy now fails on every connector path (the old code handed the raw string to BDK). Contained (callers catch it), but the user only sees "offline". Consider rejecting non-IP hosts in TorProxyEndpoint.tryParse so the settings UI can complain at save time, or resolving the hostname here.

---

Also verified (no issues found): onion detection is safe against case (Dart Uri lowercases reg-name hosts) and substring false-positives (onion.example.com); trailing-dot FQDNs handled; the SOCKS timeout path destroys the late-arriving socket; TLS-upgrade failures destroy the underlying socket before rethrowing; ElectrumTorRoute.close() is memoized; Liquid onion servers are refused rather than leaked to DNS, and the refusal is transient so the rest of the set still runs; DI ordering of the lazy singletons is fine. I did not run the Flutter test suite in this environment (no Flutter setup step available in the workflow); findings are from code analysis.