Skip to content

Fail ETH RPCs when the Tor preference changed after the client was built - #1496

Merged
julian-CStack merged 8 commits into
pr1479/fixfrom
pr1479/eth-client-tor-change
Nov 3, 2026
Merged

julian-CStack merged 8 commits into
pr1479/fixfrom
pr1479/eth-client-tor-change

Conversation

@sneurlax

@sneurlax sneurlax commented Nov 1, 2026

Copy link
Copy Markdown
Member

Summary

Stacked on #1493. Set the base branch to pr1479/fix so the diff shows only this commit; once #1493 merges GitHub retargets this PR to staging. Independent of the ETH transport controls PR, so the two can merge in either order.

createWeb3Client binds the SOCKS proxy to the HttpClient when the client is built, so the Tor decision is fixed for that client's lifetime. getEthClient is called per operation, but updateTransactions (its local getClient closure), the tx loop above it and the token wallet's tx loop each build one client lazily and reuse it for many RPCs. Toggling the Tor preference while one of those runs left the remaining RPCs of that operation on the transport chosen before the toggle: direct after enabling Tor, or still through Tor after disabling it.

The IOClient is now wrapped in a small http.BaseClient that remembers whether it was built proxied and re-reads the preference on every send. If the two disagree it throws a StateError without sending. The operation fails and is logged by the existing catch blocks, and the next operation calls getEthClient again and gets a client built under the current preference.

Deliberately not done here:

  • No rebuild, reroute or retry inside the wrapper. A broadcast that hits the check is not sent rather than replayed on a different transport.
  • No change when the preference is unchanged. A reused client keeps working as before, and there is no per-request cost beyond reading the preference.

Verification

  • dart format --set-exit-if-changed clean on both changed files
  • dart analyze lib test: no new lines versus staging
  • flutter test: all passed
  • ETH over Tor in the running app under strace: RPCs go to the local arti SOCKS port only

Tests added

In test/wallets/ethereum_tor_proxy_test.dart, against a fake SOCKS server and a fake node:

  • client built with Tor off refuses to send once Tor is on
  • client built with Tor on refuses to send once Tor is off
  • unchanged preference keeps a reused client working

kroplabeskidu and others added 7 commits October 27, 2026 03:06
web3dart was handed a plain http Client, so ETH and ERC-20 RPC could not
use Tor and the coin was flagged as not Tor-ready. Give it an IOClient
wrapping a SOCKS5-proxied HttpClient instead, the same way Solana does it,
and do the same for the node connection test.

Also add the usual node tor/clearnet mismatch check and set torSupport.
Nano and Banano already send every request through Tor, and Fact0rn uses
the regular ElectrumX client, so they only needed the torSupport flag.

The Stellar node test proxied its first request but then created a
StellarSDK with the default client, so the follow-up ledger query went
out directly.
Image.network and SvgPicture.network use their own HTTP clients and ignore
the Tor setting, so token logos, exchange and coin logos, CakePay card
images, ShopinBit attachments and theme previews were fetched directly
even in Tor mode.

Add TorAwareNetworkImage and TorAwareSvgNetworkLoader, which go through
the app's HTTP wrapper, and use them at all call sites.
The attachment images were fetched from a query-auth URL carrying the
token and customer key. Now that they go through the HTTP wrapper,
which logs failed requests, and dart:io puts the request URL in its
exceptions, those credentials could end up in the log file.

Fetch the plain URL with the Authorization and External-Customer-Key
headers instead. TorAwareNetworkImage takes the headers but keeps
keying the image cache on the URL alone, so a re-mounted attachment
still comes from the cache.

client.dart wasn't formatted on staging, so its diff includes one
formatting change.
updateDefaults keeps the saved node's tor and clearnet flags, so existing
installs kept the old clearnet-only Fact0rn default even though it now
allows Tor. With Tor turned on that node is rejected and Fact0rn cannot
refresh or pass the node test.

Add a one-time migration that enables Tor on the saved Fact0rn default
node. Only a node that still matches the old shipped default on every
field the user can edit (host, port, name, SSL, failover, login, clearnet
only) is touched, so a node the user customised keeps its settings.
Clearnet stays enabled, so nothing changes with Tor off, and later
choices are left alone.
TorAwareNetworkImage keys its cache on the url alone, so an attachment
path fetched under one customer key would be served from cache to a
later customer key without an authenticated request. Add an optional
cacheScope that is part of the key, and pass the ShopinBit customer key
there. The bearer token stays out of the key so a refreshed token does
not refetch.

The key is captured together with the request response rather than read
from the widget when building the image: FutureBuilder keeps showing the
previous snapshot while a new future is pending, and that snapshot must
not be paired with a later customer's scope.
createWeb3Client binds the SOCKS proxy to the HttpClient when the client
is built, so the Tor decision is fixed for that client's lifetime.
getEthClient is called per operation, but updateTransactions (its local
getClient closure), the tx loop above it and the token wallet's tx loop
each build one client lazily and reuse it for many RPCs. Toggling the
Tor preference while one of those runs left the remaining RPCs of that
operation on the transport chosen before the toggle: direct after
enabling Tor, or still through Tor after disabling it.

Wrap the IOClient in a client that remembers whether it was built
proxied and re-reads the preference on every send. If the two disagree
it throws without sending. The operation fails and is logged by the
existing catch blocks; the next operation calls getEthClient again and
gets a client built under the current preference. The wrapper does not
rebuild or reroute the request and there are no retries, so a broadcast
that hit the check is simply not sent rather than replayed on a
different transport.
@codecov

codecov Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 11.62%. Comparing base (36d1f49) to head (3b04f65).

Files with missing lines Patch % Lines
lib/wallets/wallet/impl/ethereum_wallet.dart 84.61% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           pr1479/fix    #1496   +/-   ##
===========================================
  Coverage       11.61%   11.62%           
===========================================
  Files            1134     1134           
  Lines          114185   114196   +11     
===========================================
+ Hits            13263    13272    +9     
- Misses         100922   100924    +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@julian-CStack
julian-CStack merged commit cf26d6d into pr1479/fix Nov 3, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants