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

lib/wallets/wallet/impl/ethereum_wallet.dart

@@ -5,6 +5,7 @@ import 'dart:io';
 import 'package:decimal/decimal.dart';
 import 'package:ethereum_addresses/ethereum_addresses.dart';
 import 'package:flutter/foundation.dart';
+import 'package:http/http.dart' as http;
 import 'package:http/io_client.dart';
 import 'package:isar_community/isar.dart';
 import 'package:socks5_proxy/socks.dart';
@@ -172,24 +173,31 @@ class EthereumWallet extends Bip39Wallet with PrivateKeyInterface {
   }
 
   // static helper function for building a web3 client, proxied over tor if
-  // enabled
+  // enabled. The returned client refuses to send once the tor preference no
+  // longer matches the transport it was built with; see _TorPreferenceClient.
   static web3.Web3Client createWeb3Client(
     String url,
     Prefs prefs,
     TorService torService,
   ) {
     final httpClient = HttpClient();
 
-    if (AppConfig.hasFeature(AppFeature.tor) && prefs.useTor) {
+    if (_shouldProxy(prefs)) {
       final proxyInfo = torService.getProxyInfo();
       SocksTCPClient.assignToHttpClient(httpClient, [
         ProxySettings(proxyInfo.host, proxyInfo.port),
       ]);
     }
 
-    return web3.Web3Client(url, IOClient(httpClient));
+    return web3.Web3Client(
+      url,
+      _TorPreferenceClient(IOClient(httpClient), prefs),
+    );
   }
 
+  static bool _shouldProxy(Prefs prefs) =>
+      AppConfig.hasFeature(AppFeature.tor) && prefs.useTor;
+
   Amount estimateEthFee(BigInt feeRate, int gasLimit, int decimals) {
     final gweiAmount = feeRate.toDecimal() / (Decimal.ten.pow(9).toDecimal());
     final fee =
@@ -772,3 +780,32 @@ class EthereumWallet extends Bip39Wallet with PrivateKeyInterface {
     await super.exit();
   }
 }
+
+// The proxy is bound to the HttpClient when it is built, but some operations
+// (updateTransactions, the token wallet's tx loop) build one client and reuse
+// it for many RPCs. If the tor preference flips meanwhile, the remaining RPCs
+// would still use the old transport. Fail those instead of sending: the
+// operation errors out and the next one builds a client under the current
+// preference. Nothing is retried here, so a broadcast is never replayed.
+class _TorPreferenceClient extends http.BaseClient {
+  _TorPreferenceClient(this._inner, this._prefs)
+    : _proxied = EthereumWallet._shouldProxy(_prefs);
+
+  final http.Client _inner;
+  final Prefs _prefs;
+  final bool _proxied;
+
+  @override
+  Future<http.StreamedResponse> send(http.BaseRequest request) async {
+    if (EthereumWallet._shouldProxy(_prefs) != _proxied) {
+      throw StateError(
+        'Tor preference changed since this client was created; '
+        'the request was not sent',
+      );
+    }
+    return _inner.send(request);
+  }
+
+  @override
+  void close() => _inner.close();
+}

test/wallets/ethereum_tor_proxy_test.dart

@@ -9,12 +9,10 @@ import 'package:stackwallet/utilities/prefs.dart';
 import 'package:stackwallet/wallets/wallet/impl/ethereum_wallet.dart';
 
 class _MockPrefs extends Mock implements Prefs {
-  _MockPrefs(this._useTor);
-
-  final bool _useTor;
+  _MockPrefs(this.useTor);
 
   @override
-  bool get useTor => _useTor;
+  bool useTor;
 }
 
 class _FakeTorService extends Fake implements TorService {
@@ -43,6 +41,7 @@ String _rpcHttpResponse() =>
 // tunnelled HTTP request with a canned JSON-RPC eth_blockNumber result.
 class _FakeSocksServer {
   late final ServerSocket _server;
+  int connections = 0;
   int? addressType;
   String? target;
   int? port;
@@ -53,6 +52,7 @@ class _FakeSocksServer {
   Future<void> start() async {
     _server = await ServerSocket.bind(InternetAddress.loopbackIPv4, 0);
     _server.listen((socket) {
+      connections++;
       var stage = 0;
       socket.listen((Uint8List bytes) {
         switch (stage) {
@@ -84,6 +84,36 @@ class _FakeSocksServer {
   Future<void> stop() => _server.close();
 }
 
+// Local node that answers every RPC with the canned result and counts them.
+class _FakeNode {
+  late final HttpServer _server;
+  int requests = 0;
+
+  String get url => 'http://127.0.0.1:${_server.port}';
+
+  Future<void> start() async {
+    _server = await HttpServer.bind(InternetAddress.loopbackIPv4, 0);
+    _server.listen((req) async {
+      requests++;
+      await utf8.decoder.bind(req).join();
+      req.response
+        ..headers.contentType = ContentType.json
+        ..write(_rpcBody);
+      await req.response.close();
+    });
+  }
+
+  Future<void> stop() => _server.close(force: true);
+}
+
+final _torPreferenceChanged = throwsA(
+  isA<StateError>().having(
+    (e) => e.message,
+    'message',
+    contains('Tor preference changed'),
+  ),
+);
+
 void main() {
   late _FakeSocksServer socks;
 
@@ -118,31 +148,111 @@ void main() {
   });
 
   test('web3 client connects directly when Tor is off', () async {
-    final server = await HttpServer.bind(InternetAddress.loopbackIPv4, 0);
-    server.listen((req) async {
-      await utf8.decoder.bind(req).join();
-      req.response
-        ..headers.contentType = ContentType.json
-        ..write(_rpcBody);
-      await req.response.close();
-    });
+    final node = _FakeNode();
+    await node.start();
 
     final torService = _FakeTorService((
       host: InternetAddress.loopbackIPv4,
       port: socks.listeningPort,
     ));
     final client = EthereumWallet.createWeb3Client(
-      'http://127.0.0.1:${server.port}',
+      node.url,
       _MockPrefs(false),
       torService,
     );
 
     final height = await client.getBlockNumber();
     await client.dispose();
-    await server.close(force: true);
+    await node.stop();
 
     expect(height, 42);
+    expect(node.requests, 1);
     expect(torService.proxyInfoReads, 0);
     expect(socks.target, isNull);
   });
+
+  group('tor preference changed after the client was built', () {
+    late _FakeNode node;
+    late _FakeTorService torService;
+
+    setUp(() async {
+      node = _FakeNode();
+      await node.start();
+      torService = _FakeTorService((
+        host: InternetAddress.loopbackIPv4,
+        port: socks.listeningPort,
+      ));
+    });
+
+    tearDown(() => node.stop());
+
+    test('client built with Tor off refuses to send once Tor is on', () async {
+      final prefs = _MockPrefs(false);
+      final client = EthereumWallet.createWeb3Client(
+        node.url,
+        prefs,
+        torService,
+      );
+
+      expect(await client.getBlockNumber(), 42);
+      expect(node.requests, 1);
+
+      prefs.useTor = true;
+
+      await expectLater(client.getBlockNumber(), _torPreferenceChanged);
+      await client.dispose();
+
+      // Nothing went out directly and nothing was rerouted via the proxy.
+      expect(node.requests, 1);
+      expect(socks.connections, 0);
+      expect(torService.proxyInfoReads, 0);
+    });
+
+    test('client built with Tor on refuses to send once Tor is off', () async {
+      final prefs = _MockPrefs(true);
+      final client = EthereumWallet.createWeb3Client(
+        node.url,
+        prefs,
+        torService,
+      );
+
+      expect(await client.getBlockNumber(), 42);
+      expect(socks.connections, 1);
+      expect(socks.request.toString(), contains('eth_blockNumber'));
+
+      prefs.useTor = false;
+
+      await expectLater(client.getBlockNumber(), _torPreferenceChanged);
+      await client.dispose();
+
+      // No new proxy connection and nothing went to the node directly.
+      expect(socks.connections, 1);
+      expect(node.requests, 0);
+      expect(torService.proxyInfoReads, 1);
+    });
+
+    test('unchanged preference keeps a reused client working', () async {
+      final direct = EthereumWallet.createWeb3Client(
+        node.url,
+        _MockPrefs(false),
+        torService,
+      );
+      expect(await direct.getBlockNumber(), 42);
+      expect(await direct.getBlockNumber(), 42);
+      await direct.dispose();
+      expect(node.requests, 2);
+      expect(socks.connections, 0);
+
+      final proxied = EthereumWallet.createWeb3Client(
+        node.url,
+        _MockPrefs(true),
+        torService,
+      );
+      expect(await proxied.getBlockNumber(), 42);
+      expect(await proxied.getBlockNumber(), 42);
+      await proxied.dispose();
+      expect(node.requests, 2);
+      expect(socks.connections, 2);
+    });
+  });
 }