diff --git a/lib/wallets/wallet/intermediate/cryptonote_wallet_lifecycle.dart b/lib/wallets/wallet/intermediate/cryptonote_wallet_lifecycle.dart new file mode 100644 index 0000000000..93a787067a --- /dev/null +++ b/lib/wallets/wallet/intermediate/cryptonote_wallet_lifecycle.dart @@ -0,0 +1,130 @@ +import 'dart:async'; + +import 'package:mutex/mutex.dart'; + +Future cancelCryptonoteWalletSubscriptions( + Iterable?> subscriptions, +) async { + Object? firstError; + StackTrace? firstStackTrace; + for (final subscription in subscriptions) { + try { + await subscription?.cancel(); + } catch (error, stackTrace) { + firstError ??= error; + firstStackTrace ??= stackTrace; + } + } + if (firstError != null) { + Error.throwWithStackTrace(firstError, firstStackTrace!); + } +} + +class CryptonoteTorTransitionGate { + Completer? _transition; + + void block() { + _transition ??= Completer(); + } + + void release() { + final transition = _transition; + _transition = null; + if (transition != null && !transition.isCompleted) { + transition.complete(); + } + } + + Future wait({ + required bool Function() isBlocked, + required bool Function() isCurrent, + }) async { + while (isBlocked()) { + final transition = _transition ??= Completer(); + await transition.future; + if (!isCurrent()) { + return false; + } + } + return true; + } +} + +/// Serializes native wallet lifecycle operations and rejects node updates once +/// shutdown begins. +class CryptonoteWalletLifecycle { + final _mutex = Mutex(); + bool _allowsNodeUpdates = true; + + bool get allowsNodeUpdates => _allowsNodeUpdates; + + Future open( + Future Function(bool Function() isCurrent) operation, + ) => _mutex.protect(() async { + _allowsNodeUpdates = true; + try { + await operation(() => _allowsNodeUpdates); + } catch (_) { + _allowsNodeUpdates = false; + rethrow; + } + }); + + Future updateNode( + Future Function(bool Function() isCurrent) operation, + ) => _mutex.protect(() async { + if (_allowsNodeUpdates) { + await operation(() => _allowsNodeUpdates); + } + }); + + Future runIfCurrent(Future Function() operation) => + _mutex.protect(() async { + if (_allowsNodeUpdates) { + await operation(); + } + }); + + Future replaceNative(Future Function() operation) => + _mutex.protect(() { + if (!_allowsNodeUpdates) { + throw StateError("Native wallet lifecycle is closing"); + } + return operation(); + }); + + Future close({ + required Future Function() stopEventSources, + required Future Function() closeNative, + }) async { + _allowsNodeUpdates = false; + + Object? stopError; + StackTrace? stopStackTrace; + Future stopSources() async { + try { + await stopEventSources(); + } catch (error, stackTrace) { + stopError ??= error; + stopStackTrace ??= stackTrace; + } + } + + // Reserve shutdown's place in the queue immediately. Sources are stopped + // before native close, then checked again for a concurrent open. + final firstStopCompleted = Completer(); + final serializedClose = _mutex.protect(() async { + await firstStopCompleted.future; + await stopSources(); + await closeNative(); + }); + + await stopSources(); + firstStopCompleted.complete(); + await serializedClose; + + if (stopError != null) { + Error.throwWithStackTrace(stopError!, stopStackTrace!); + } + } +} diff --git a/lib/wallets/wallet/intermediate/lib_monero_wallet.dart b/lib/wallets/wallet/intermediate/lib_monero_wallet.dart index 6c0c49884e..da442090d3 100644 --- a/lib/wallets/wallet/intermediate/lib_monero_wallet.dart +++ b/lib/wallets/wallet/intermediate/lib_monero_wallet.dart @@ -43,6 +43,7 @@ import '../../models/tx_data.dart'; import '../wallet.dart'; import '../wallet_mixin_interfaces/multi_address_interface.dart'; import '../wallet_mixin_interfaces/view_only_option_interface.dart'; +import 'cryptonote_wallet_lifecycle.dart'; import 'cryptonote_wallet.dart'; abstract class LibMoneroWallet @@ -52,27 +53,27 @@ abstract class LibMoneroWallet @override int get isarTransactionVersion => 2; - LibMoneroWallet(super.currency, this.compatType) { + LibMoneroWallet(super.currency, this.compatType); + + void _attachTorListeners() { + if (_torStatusListener != null || _torPreferenceListener != null) { + return; + } + final bus = GlobalEventBus.instance; // Listen for tor status changes. _torStatusListener = bus.on().listen(( event, - ) async { + ) { switch (event.newStatus) { case TorConnectionStatus.connecting: - if (!_torConnectingLock.isLocked) { - await _torConnectingLock.acquire(); - } - _requireMutex = true; + _torTransitionGate.block(); break; case TorConnectionStatus.connected: case TorConnectionStatus.disconnected: - if (_torConnectingLock.isLocked) { - _torConnectingLock.release(); - } - _requireMutex = false; + _torTransitionGate.release(); break; } }); @@ -81,33 +82,67 @@ abstract class LibMoneroWallet _torPreferenceListener = bus.on().listen(( event, ) async { - await updateNode(); + await _updateNodeFromTor(); }); - // Potentially dangerous hack. See comments in _startInit() - _startInit(); + if (TorService.sharedInstance.status == TorConnectionStatus.connecting) { + _torTransitionGate.block(); + } + } + + Future _updateNodeFromTor() async { + try { + await updateNode(); + } catch (e, s) { + Logging.instance.w( + "Tor-triggered node update failed", + error: e, + stackTrace: s, + ); + } + } + + Future _stopEventSources() async { + try { + await cancelCryptonoteWalletSubscriptions([ + _torStatusListener, + _torPreferenceListener, + _streamSub, + ]); + } finally { + _torStatusListener = null; + _torPreferenceListener = null; + _streamSub = null; + _torTransitionGate.release(); + } } + // cw based wallet listener to handle synchronization of utxo frozen states - late final StreamSubscription> _streamSub; - Future _startInit() async { - // Delay required as `mainDB` is not initialized in constructor. - // This is a hack and could lead to a race condition. - Future.delayed(const Duration(seconds: 2), () { - _streamSub = mainDB.isar.utxos - .where() - .walletIdEqualTo(walletId) - .watch(fireImmediately: true) - .listen((utxos) async { - try { - await onUTXOsChanged(utxos); - await updateBalance(shouldUpdateUtxos: false); - } catch (e, s) { - Logging.instance.e("_startInit", error: e, stackTrace: s); - } - }); - }); + StreamSubscription>? _streamSub; + void _attachUtxoListener() { + _streamSub ??= mainDB.isar.utxos + .where() + .walletIdEqualTo(walletId) + .watch(fireImmediately: true) + .listen((utxos) async { + try { + await _handleUtxosChanged(utxos); + } catch (e, s) { + Logging.instance.e( + "UTXO listener failed", + error: e, + stackTrace: s, + ); + } + }); } + Future _handleUtxosChanged(List utxos) => + _lifecycle.runIfCurrent(() async { + await onUTXOsChanged(utxos); + await updateBalance(shouldUpdateUtxos: false); + }); + final lib_monero_compat.WalletType compatType; lib_monero_compat.SyncStatus? get syncStatus => _syncStatus; @@ -192,12 +227,39 @@ abstract class LibMoneroWallet } @override - Future open() async { - bool wasNull = false; + Future open() => _lifecycle.open(_open); + + Future _open(bool Function() isCurrent) async { + try { + await _openNative(isCurrent); + } catch (error, stackTrace) { + try { + await _stopEventSources(); + } catch (cleanupError, cleanupStackTrace) { + Logging.instance.e( + "Failed to stop wallet event sources after open failure", + error: cleanupError, + stackTrace: cleanupStackTrace, + ); + } + try { + await _exitNative(); + } catch (cleanupError, cleanupStackTrace) { + Logging.instance.e( + "Failed to clean up native wallet after open failure", + error: cleanupError, + stackTrace: cleanupStackTrace, + ); + } + Error.throwWithStackTrace(error, stackTrace); + } + } + + Future _openNative(bool Function() isCurrent) async { + var wasNull = false; if (wallet == null) { wasNull = true; - // libMoneroWalletT?.close(); final path = await pathForWallet(name: walletId, type: compatType); final String password; @@ -210,10 +272,12 @@ abstract class LibMoneroWallet } wallet = await loadWallet(path: path, password: password); - _setListener(); - await updateNode(); + _attachTorListeners(); + await _updateNode(isCurrent); + } else { + _attachTorListeners(); } Address? currentAddress = await getCurrentReceivingAddress(); @@ -234,14 +298,16 @@ abstract class LibMoneroWallet await csMonero.startSyncing(wallet!); } catch (_) { _setSyncStatus(lib_monero_compat.FailedSyncStatus()); - // TODO log } } _setListener(); await csMonero.startListeners(wallet!); csMonero.startAutoSaving(wallet!); - unawaited(refresh()); + if (isCurrent()) { + _attachUtxoListener(); + unawaited(refresh()); + } } @Deprecated("Only used in the case of older wallets") @@ -374,9 +440,7 @@ abstract class LibMoneroWallet ); this.wallet = wallet; - await updateNode(); - await csMonero.close(wallet, save: true); - this.wallet = null; + await _initializeAndClose(wallet); } catch (e, s) { Logging.instance.f("", error: e, stackTrace: s); } @@ -385,8 +449,21 @@ abstract class LibMoneroWallet return super.init(); } + Future _initializeAndClose(WrappedWallet wallet) async { + try { + await updateNode(); + await csMonero.close(wallet, save: true); + } finally { + await _stopEventSources(); + this.wallet = null; + } + } + @override - Future recover({required bool isRescan}) async { + Future recover({required bool isRescan}) => + _lifecycle.replaceNative(() => _recover(isRescan: isRescan)); + + Future _recover({required bool isRescan}) async { if (isRescan) { await refreshMutex.protect(() async { // clear blockchain info @@ -402,7 +479,7 @@ abstract class LibMoneroWallet } if (isViewOnly) { - await recoverViewOnly(); + await _recoverViewOnly(); return; } @@ -441,7 +518,7 @@ abstract class LibMoneroWallet ); if (this.wallet != null) { - await exit(); + await _exitNative(); } this.wallet = wallet; @@ -469,7 +546,7 @@ abstract class LibMoneroWallet Logging.instance.f("", error: e, stackTrace: s); rethrow; } - await updateNode(); + await _updateNode(() => _lifecycle.allowsNodeUpdates); _setListener(); // libMoneroWallet?.setRecoveringFromSeed(isRecovery: true); @@ -503,7 +580,22 @@ abstract class LibMoneroWallet } @override - Future updateNode() async { + Future updateNode() => _lifecycle.updateNode(_updateNode); + + Future _updateNode(bool Function() isCurrent) async { + if (wallet == null) { + return; + } + + _attachTorListeners(); + if (!await _torTransitionGate.wait( + isBlocked: () => + TorService.sharedInstance.status == TorConnectionStatus.connecting, + isCurrent: isCurrent, + )) { + return; + } + final node = getCurrentNode(); if (await _torNodeMismatchGuard(node)) { @@ -513,43 +605,33 @@ abstract class LibMoneroWallet final host = node.host.endsWith(".onion") ? node.host : Uri.parse(node.host).host; - final ({InternetAddress host, int port})? proxy = - AppConfig.hasFeature(AppFeature.tor) && prefs.useTor && !node.forceNoTor - ? TorService.sharedInstance.getProxyInfo() - : null; _setSyncStatus(lib_monero_compat.ConnectingSyncStatus()); try { - if (_requireMutex) { - await _torConnectingLock.protect(() async { - await csMonero.connect( - wallet!, - daemonAddress: "$host:${node.port}", - daemonUsername: node.loginName, - daemonPassword: await node.getPassword(secureStorageInterface), - trusted: node.trusted ?? false, - useSSL: node.useSSL, - socksProxyAddress: node.forceNoTor - ? null - : proxy == null - ? null - : "${proxy.host.address}:${proxy.port}", - ); - }); - } else { - await csMonero.connect( - wallet!, - daemonAddress: "$host:${node.port}", - daemonUsername: node.loginName, - daemonPassword: await node.getPassword(secureStorageInterface), - trusted: node.trusted ?? false, - useSSL: node.useSSL, - socksProxyAddress: node.forceNoTor - ? null - : proxy == null - ? null - : "${proxy.host.address}:${proxy.port}", - ); + if (!isCurrent() || wallet == null) { + return; + } + + final ({InternetAddress host, int port})? proxy = + AppConfig.hasFeature(AppFeature.tor) && + prefs.useTor && + !node.forceNoTor + ? TorService.sharedInstance.getProxyInfo() + : null; + await csMonero.connect( + wallet!, + daemonAddress: "$host:${node.port}", + daemonUsername: node.loginName, + daemonPassword: await node.getPassword(secureStorageInterface), + trusted: node.trusted ?? false, + useSSL: node.useSSL, + socksProxyAddress: proxy == null + ? null + : "${proxy.host.address}:${proxy.port}", + ); + + if (!isCurrent() || wallet == null) { + return; } await csMonero.startSyncing(wallet!); await csMonero.startListeners(wallet!); @@ -564,8 +646,6 @@ abstract class LibMoneroWallet stackTrace: s, ); } - - return; } @override @@ -732,13 +812,20 @@ abstract class LibMoneroWallet @override Future exit() async { Logging.instance.i("exit called on monero $walletId!"); + await _lifecycle.close( + stopEventSources: _stopEventSources, + closeNative: _exitNative, + ); + Logging.instance.i("exit call completed monero $walletId!"); + } + + Future _exitNative() async { if (wallet != null) { csMonero.stopAutoSaving(wallet!); await csMonero.stopListeners(wallet!); await csMonero.stopSyncing(wallet!); await csMonero.save(wallet!); } - Logging.instance.i("exit call completed monero $walletId!"); } Future pathForWalletDir({ @@ -1546,7 +1633,9 @@ abstract class LibMoneroWallet // ============== View only ================================================== @override - Future recoverViewOnly() async { + Future recoverViewOnly() => _lifecycle.replaceNative(_recoverViewOnly); + + Future _recoverViewOnly() async { await refreshMutex.protect(() async { final data = await getViewOnlyWalletData() as CryptonoteViewOnlyWalletData; @@ -1577,8 +1666,8 @@ abstract class LibMoneroWallet height: height, ); - if (this.wallet == null) { - await exit(); + if (this.wallet != null) { + await _exitNative(); } this.wallet = wallet; @@ -1602,7 +1691,7 @@ abstract class LibMoneroWallet isar: mainDB.isar, ); - await updateNode(); + await _updateNode(() => _lifecycle.allowsNodeUpdates); _setListener(); await csMonero.rescanBlockchain(this.wallet!); @@ -1627,6 +1716,7 @@ abstract class LibMoneroWallet StreamSubscription? _torStatusListener; StreamSubscription? _torPreferenceListener; - final Mutex _torConnectingLock = Mutex(); - bool _requireMutex = false; + final _torTransitionGate = CryptonoteTorTransitionGate(); + + final _lifecycle = CryptonoteWalletLifecycle(); } diff --git a/lib/wallets/wallet/intermediate/lib_salvium_wallet.dart b/lib/wallets/wallet/intermediate/lib_salvium_wallet.dart index 03e11f74c0..d033a3478f 100644 --- a/lib/wallets/wallet/intermediate/lib_salvium_wallet.dart +++ b/lib/wallets/wallet/intermediate/lib_salvium_wallet.dart @@ -41,6 +41,7 @@ import '../../models/tx_data.dart'; import '../wallet.dart'; import '../wallet_mixin_interfaces/multi_address_interface.dart'; import '../wallet_mixin_interfaces/view_only_option_interface.dart'; +import 'cryptonote_wallet_lifecycle.dart'; import 'cryptonote_wallet.dart'; abstract class LibSalviumWallet @@ -50,27 +51,27 @@ abstract class LibSalviumWallet @override int get isarTransactionVersion => 2; - LibSalviumWallet(super.currency) { + LibSalviumWallet(super.currency); + + void _attachTorListeners() { + if (_torStatusListener != null || _torPreferenceListener != null) { + return; + } + final bus = GlobalEventBus.instance; // Listen for tor status changes. _torStatusListener = bus.on().listen(( event, - ) async { + ) { switch (event.newStatus) { case TorConnectionStatus.connecting: - if (!_torConnectingLock.isLocked) { - await _torConnectingLock.acquire(); - } - _requireMutex = true; + _torTransitionGate.block(); break; case TorConnectionStatus.connected: case TorConnectionStatus.disconnected: - if (_torConnectingLock.isLocked) { - _torConnectingLock.release(); - } - _requireMutex = false; + _torTransitionGate.release(); break; } }); @@ -79,33 +80,67 @@ abstract class LibSalviumWallet _torPreferenceListener = bus.on().listen(( event, ) async { - await updateNode(); + await _updateNodeFromTor(); }); - // Potentially dangerous hack. See comments in _startInit() - _startInit(); + if (TorService.sharedInstance.status == TorConnectionStatus.connecting) { + _torTransitionGate.block(); + } } + + Future _updateNodeFromTor() async { + try { + await updateNode(); + } catch (e, s) { + Logging.instance.w( + "Tor-triggered node update failed", + error: e, + stackTrace: s, + ); + } + } + + Future _stopEventSources() async { + try { + await cancelCryptonoteWalletSubscriptions([ + _torStatusListener, + _torPreferenceListener, + _streamSub, + ]); + } finally { + _torStatusListener = null; + _torPreferenceListener = null; + _streamSub = null; + _torTransitionGate.release(); + } + } + // cw based wallet listener to handle synchronization of utxo frozen states - late final StreamSubscription> _streamSub; - Future _startInit() async { - // Delay required as `mainDB` is not initialized in constructor. - // This is a hack and could lead to a race condition. - Future.delayed(const Duration(seconds: 2), () { - _streamSub = mainDB.isar.utxos - .where() - .walletIdEqualTo(walletId) - .watch(fireImmediately: true) - .listen((utxos) async { - try { - await onUTXOsChanged(utxos); - await updateBalance(shouldUpdateUtxos: false); - } catch (e, s) { - Logging.instance.e("_startInit", error: e, stackTrace: s); - } - }); - }); + StreamSubscription>? _streamSub; + void _attachUtxoListener() { + _streamSub ??= mainDB.isar.utxos + .where() + .walletIdEqualTo(walletId) + .watch(fireImmediately: true) + .listen((utxos) async { + try { + await _handleUtxosChanged(utxos); + } catch (e, s) { + Logging.instance.e( + "UTXO listener failed", + error: e, + stackTrace: s, + ); + } + }); } + Future _handleUtxosChanged(List utxos) => + _lifecycle.runIfCurrent(() async { + await onUTXOsChanged(utxos); + await updateBalance(shouldUpdateUtxos: false); + }); + SyncStatus? get syncStatus => _syncStatus; SyncStatus? _syncStatus; int _syncedCount = 0; @@ -188,12 +223,39 @@ abstract class LibSalviumWallet } @override - Future open() async { - bool wasNull = false; + Future open() => _lifecycle.open(_open); + + Future _open(bool Function() isCurrent) async { + try { + await _openNative(isCurrent); + } catch (error, stackTrace) { + try { + await _stopEventSources(); + } catch (cleanupError, cleanupStackTrace) { + Logging.instance.e( + "Failed to stop wallet event sources after open failure", + error: cleanupError, + stackTrace: cleanupStackTrace, + ); + } + try { + await _exitNative(); + } catch (cleanupError, cleanupStackTrace) { + Logging.instance.e( + "Failed to clean up native wallet after open failure", + error: cleanupError, + stackTrace: cleanupStackTrace, + ); + } + Error.throwWithStackTrace(error, stackTrace); + } + } + + Future _openNative(bool Function() isCurrent) async { + var wasNull = false; if (wallet == null) { wasNull = true; - // await libSalviumWallet?.close(); final path = await pathForWallet(name: walletId); final String password; @@ -206,10 +268,12 @@ abstract class LibSalviumWallet } wallet = await loadWallet(path: path, password: password); - _setListener(); - await updateNode(); + _attachTorListeners(); + await _updateNode(isCurrent); + } else { + _attachTorListeners(); } Address? currentAddress = await getCurrentReceivingAddress(); @@ -230,14 +294,16 @@ abstract class LibSalviumWallet csSalvium.startSyncing(wallet!); } catch (_) { _setSyncStatus(FailedSyncStatus()); - // TODO log } } _setListener(); csSalvium.startListeners(wallet!); csSalvium.startAutoSaving(wallet!); - unawaited(refresh()); + if (isCurrent()) { + _attachUtxoListener(); + unawaited(refresh()); + } } Future save() async { @@ -352,9 +418,7 @@ abstract class LibSalviumWallet ); this.wallet = wallet; - await updateNode(); - await csSalvium.close(wallet, save: true); - this.wallet = null; + await _initializeAndClose(wallet); } catch (e, s) { Logging.instance.f("", error: e, stackTrace: s); } @@ -363,8 +427,21 @@ abstract class LibSalviumWallet return super.init(); } + Future _initializeAndClose(WrappedWallet wallet) async { + try { + await updateNode(); + await csSalvium.close(wallet, save: true); + } finally { + await _stopEventSources(); + this.wallet = null; + } + } + @override - Future recover({required bool isRescan}) async { + Future recover({required bool isRescan}) => + _lifecycle.replaceNative(() => _recover(isRescan: isRescan)); + + Future _recover({required bool isRescan}) async { if (isRescan) { await refreshMutex.protect(() async { // clear blockchain info @@ -380,7 +457,7 @@ abstract class LibSalviumWallet } if (isViewOnly) { - await recoverViewOnly(); + await _recoverViewOnly(); return; } @@ -419,7 +496,7 @@ abstract class LibSalviumWallet ); if (this.wallet != null) { - await exit(); + await _exitNative(); } this.wallet = wallet; @@ -447,7 +524,7 @@ abstract class LibSalviumWallet Logging.instance.f("", error: e, stackTrace: s); rethrow; } - await updateNode(); + await _updateNode(() => _lifecycle.allowsNodeUpdates); _setListener(); // libSalviumWallet?.setRecoveringFromSeed(isRecovery: true); @@ -480,7 +557,22 @@ abstract class LibSalviumWallet } @override - Future updateNode() async { + Future updateNode() => _lifecycle.updateNode(_updateNode); + + Future _updateNode(bool Function() isCurrent) async { + if (wallet == null) { + return; + } + + _attachTorListeners(); + if (!await _torTransitionGate.wait( + isBlocked: () => + TorService.sharedInstance.status == TorConnectionStatus.connecting, + isCurrent: isCurrent, + )) { + return; + } + final node = getCurrentNode(); if (_torNodeMismatchGuard(node)) { @@ -490,44 +582,34 @@ abstract class LibSalviumWallet final host = node.host.endsWith(".onion") ? node.host : Uri.parse(node.host).host; - final ({InternetAddress host, int port})? proxy = - AppConfig.hasFeature(AppFeature.tor) && prefs.useTor && !node.forceNoTor - ? TorService.sharedInstance.getProxyInfo() - : null; _setSyncStatus(ConnectingSyncStatus()); try { - if (_requireMutex) { - await _torConnectingLock.protect(() async { - await csSalvium.connect( - wallet!, - daemonAddress: "$host:${node.port}", - daemonUsername: node.loginName, - daemonPassword: await node.getPassword(secureStorageInterface), - trusted: node.trusted ?? false, - useSSL: node.useSSL, - socksProxyAddress: node.forceNoTor - ? null - : proxy == null - ? null - : "${proxy.host.address}:${proxy.port}", - ); - }); - } else { - await csSalvium.connect( - wallet!, - daemonAddress: "$host:${node.port}", - daemonUsername: node.loginName, - daemonPassword: await node.getPassword(secureStorageInterface), - trusted: node.trusted ?? false, - useSSL: node.useSSL, - socksProxyAddress: node.forceNoTor - ? null - : proxy == null - ? null - : "${proxy.host.address}:${proxy.port}", - ); + if (!isCurrent() || wallet == null) { + return; } + + final ({InternetAddress host, int port})? proxy = + AppConfig.hasFeature(AppFeature.tor) && + prefs.useTor && + !node.forceNoTor + ? TorService.sharedInstance.getProxyInfo() + : null; + await csSalvium.connect( + wallet!, + daemonAddress: "$host:${node.port}", + daemonUsername: node.loginName, + daemonPassword: await node.getPassword(secureStorageInterface), + trusted: node.trusted ?? false, + useSSL: node.useSSL, + socksProxyAddress: proxy == null + ? null + : "${proxy.host.address}:${proxy.port}", + ); + + if (!isCurrent() || wallet == null) { + return; + } csSalvium.startSyncing(wallet!); csSalvium.startListeners(wallet!); csSalvium.startAutoSaving(wallet!); @@ -541,8 +623,6 @@ abstract class LibSalviumWallet stackTrace: s, ); } - - return; } @override @@ -727,6 +807,13 @@ abstract class LibSalviumWallet @override Future exit() async { Logging.instance.i("exit called on $walletId"); + await _lifecycle.close( + stopEventSources: _stopEventSources, + closeNative: _exitNative, + ); + } + + Future _exitNative() async { if (wallet != null) { csSalvium.stopAutoSaving(wallet!); csSalvium.stopListeners(wallet!); @@ -1512,7 +1599,9 @@ abstract class LibSalviumWallet // ============== View only ================================================== @override - Future recoverViewOnly() async { + Future recoverViewOnly() => _lifecycle.replaceNative(_recoverViewOnly); + + Future _recoverViewOnly() async { await refreshMutex.protect(() async { final data = await getViewOnlyWalletData() as CryptonoteViewOnlyWalletData; @@ -1544,7 +1633,7 @@ abstract class LibSalviumWallet ); if (this.wallet != null) { - await exit(); + await _exitNative(); } this.wallet = wallet; @@ -1569,7 +1658,7 @@ abstract class LibSalviumWallet isar: mainDB.isar, ); - await updateNode(); + await _updateNode(() => _lifecycle.allowsNodeUpdates); _setListener(); unawaited(csSalvium.rescanBlockchain(this.wallet!)); @@ -1594,8 +1683,9 @@ abstract class LibSalviumWallet StreamSubscription? _torStatusListener; StreamSubscription? _torPreferenceListener; - final Mutex _torConnectingLock = Mutex(); - bool _requireMutex = false; + final _torTransitionGate = CryptonoteTorTransitionGate(); + + final _lifecycle = CryptonoteWalletLifecycle(); } String _libSalviumWalletPasswordKey(String walletName) => diff --git a/lib/wallets/wallet/intermediate/lib_wownero_wallet.dart b/lib/wallets/wallet/intermediate/lib_wownero_wallet.dart index 5ebd2191a3..bdf1ed0416 100644 --- a/lib/wallets/wallet/intermediate/lib_wownero_wallet.dart +++ b/lib/wallets/wallet/intermediate/lib_wownero_wallet.dart @@ -45,6 +45,7 @@ import '../../models/tx_data.dart'; import '../wallet.dart'; import '../wallet_mixin_interfaces/multi_address_interface.dart'; import '../wallet_mixin_interfaces/view_only_option_interface.dart'; +import 'cryptonote_wallet_lifecycle.dart'; import 'cryptonote_wallet.dart'; abstract class LibWowneroWallet @@ -54,27 +55,27 @@ abstract class LibWowneroWallet @override int get isarTransactionVersion => 2; - LibWowneroWallet(super.currency, this.compatType) { + LibWowneroWallet(super.currency, this.compatType); + + void _attachTorListeners() { + if (_torStatusListener != null || _torPreferenceListener != null) { + return; + } + final bus = GlobalEventBus.instance; // Listen for tor status changes. _torStatusListener = bus.on().listen(( event, - ) async { + ) { switch (event.newStatus) { case TorConnectionStatus.connecting: - if (!_torConnectingLock.isLocked) { - await _torConnectingLock.acquire(); - } - _requireMutex = true; + _torTransitionGate.block(); break; case TorConnectionStatus.connected: case TorConnectionStatus.disconnected: - if (_torConnectingLock.isLocked) { - _torConnectingLock.release(); - } - _requireMutex = false; + _torTransitionGate.release(); break; } }); @@ -83,33 +84,67 @@ abstract class LibWowneroWallet _torPreferenceListener = bus.on().listen(( event, ) async { - await updateNode(); + await _updateNodeFromTor(); }); - // Potentially dangerous hack. See comments in _startInit() - _startInit(); + if (TorService.sharedInstance.status == TorConnectionStatus.connecting) { + _torTransitionGate.block(); + } } + + Future _updateNodeFromTor() async { + try { + await updateNode(); + } catch (e, s) { + Logging.instance.w( + "Tor-triggered node update failed", + error: e, + stackTrace: s, + ); + } + } + + Future _stopEventSources() async { + try { + await cancelCryptonoteWalletSubscriptions([ + _torStatusListener, + _torPreferenceListener, + _streamSub, + ]); + } finally { + _torStatusListener = null; + _torPreferenceListener = null; + _streamSub = null; + _torTransitionGate.release(); + } + } + // cw based wallet listener to handle synchronization of utxo frozen states - late final StreamSubscription> _streamSub; - Future _startInit() async { - // Delay required as `mainDB` is not initialized in constructor. - // This is a hack and could lead to a race condition. - Future.delayed(const Duration(seconds: 2), () { - _streamSub = mainDB.isar.utxos - .where() - .walletIdEqualTo(walletId) - .watch(fireImmediately: true) - .listen((utxos) async { - try { - await onUTXOsChanged(utxos); - await updateBalance(shouldUpdateUtxos: false); - } catch (e, s) { - Logging.instance.e("_startInit", error: e, stackTrace: s); - } - }); - }); + StreamSubscription>? _streamSub; + void _attachUtxoListener() { + _streamSub ??= mainDB.isar.utxos + .where() + .walletIdEqualTo(walletId) + .watch(fireImmediately: true) + .listen((utxos) async { + try { + await _handleUtxosChanged(utxos); + } catch (e, s) { + Logging.instance.e( + "UTXO listener failed", + error: e, + stackTrace: s, + ); + } + }); } + Future _handleUtxosChanged(List utxos) => + _lifecycle.runIfCurrent(() async { + await onUTXOsChanged(utxos); + await updateBalance(shouldUpdateUtxos: false); + }); + final lib_monero_compat.WalletType compatType; lib_monero_compat.SyncStatus? get syncStatus => _syncStatus; @@ -194,12 +229,39 @@ abstract class LibWowneroWallet } @override - Future open() async { - bool wasNull = false; + Future open() => _lifecycle.open(_open); + + Future _open(bool Function() isCurrent) async { + try { + await _openNative(isCurrent); + } catch (error, stackTrace) { + try { + await _stopEventSources(); + } catch (cleanupError, cleanupStackTrace) { + Logging.instance.e( + "Failed to stop wallet event sources after open failure", + error: cleanupError, + stackTrace: cleanupStackTrace, + ); + } + try { + await _exitNative(); + } catch (cleanupError, cleanupStackTrace) { + Logging.instance.e( + "Failed to clean up native wallet after open failure", + error: cleanupError, + stackTrace: cleanupStackTrace, + ); + } + Error.throwWithStackTrace(error, stackTrace); + } + } + + Future _openNative(bool Function() isCurrent) async { + var wasNull = false; if (wallet == null) { wasNull = true; - // LibWowneroWalletT?.close(); final path = await pathForWallet(name: walletId, type: compatType); final String password; @@ -212,10 +274,12 @@ abstract class LibWowneroWallet } wallet = await loadWallet(path: path, password: password); - _setListener(); - await updateNode(); + _attachTorListeners(); + await _updateNode(isCurrent); + } else { + _attachTorListeners(); } Address? currentAddress = await getCurrentReceivingAddress(); @@ -236,14 +300,16 @@ abstract class LibWowneroWallet csWownero.startSyncing(wallet!); } catch (_) { _setSyncStatus(lib_monero_compat.FailedSyncStatus()); - // TODO log } } _setListener(); csWownero.startListeners(wallet!); csWownero.startAutoSaving(wallet!); - unawaited(refresh()); + if (isCurrent()) { + _attachUtxoListener(); + unawaited(refresh()); + } } @Deprecated("Only used in the case of older wallets") @@ -376,9 +442,7 @@ abstract class LibWowneroWallet ); this.wallet = wallet; - await updateNode(); - await csWownero.close(wallet, save: true); - this.wallet = null; + await _initializeAndClose(wallet); } catch (e, s) { Logging.instance.f("", error: e, stackTrace: s); } @@ -387,8 +451,21 @@ abstract class LibWowneroWallet return super.init(); } + Future _initializeAndClose(WrappedWallet wallet) async { + try { + await updateNode(); + await csWownero.close(wallet, save: true); + } finally { + await _stopEventSources(); + this.wallet = null; + } + } + @override - Future recover({required bool isRescan}) async { + Future recover({required bool isRescan}) => + _lifecycle.replaceNative(() => _recover(isRescan: isRescan)); + + Future _recover({required bool isRescan}) async { if (isRescan) { await refreshMutex.protect(() async { // clear blockchain info @@ -404,7 +481,7 @@ abstract class LibWowneroWallet } if (isViewOnly) { - await recoverViewOnly(); + await _recoverViewOnly(); return; } @@ -443,7 +520,7 @@ abstract class LibWowneroWallet ); if (this.wallet != null) { - await exit(); + await _exitNative(); } this.wallet = wallet; @@ -471,7 +548,7 @@ abstract class LibWowneroWallet Logging.instance.f("", error: e, stackTrace: s); rethrow; } - await updateNode(); + await _updateNode(() => _lifecycle.allowsNodeUpdates); _setListener(); // LibWowneroWallet?.setRecoveringFromSeed(isRecovery: true); @@ -505,7 +582,22 @@ abstract class LibWowneroWallet } @override - Future updateNode() async { + Future updateNode() => _lifecycle.updateNode(_updateNode); + + Future _updateNode(bool Function() isCurrent) async { + if (wallet == null) { + return; + } + + _attachTorListeners(); + if (!await _torTransitionGate.wait( + isBlocked: () => + TorService.sharedInstance.status == TorConnectionStatus.connecting, + isCurrent: isCurrent, + )) { + return; + } + final node = getCurrentNode(); if (_torNodeMismatchGuard(node)) { @@ -515,44 +607,34 @@ abstract class LibWowneroWallet final host = node.host.endsWith(".onion") ? node.host : Uri.parse(node.host).host; - final ({InternetAddress host, int port})? proxy = - AppConfig.hasFeature(AppFeature.tor) && prefs.useTor && !node.forceNoTor - ? TorService.sharedInstance.getProxyInfo() - : null; _setSyncStatus(lib_monero_compat.ConnectingSyncStatus()); try { - if (_requireMutex) { - await _torConnectingLock.protect(() async { - await csWownero.connect( - wallet!, - daemonAddress: "$host:${node.port}", - daemonUsername: node.loginName, - daemonPassword: await node.getPassword(secureStorageInterface), - trusted: node.trusted ?? false, - useSSL: node.useSSL, - socksProxyAddress: node.forceNoTor - ? null - : proxy == null - ? null - : "${proxy.host.address}:${proxy.port}", - ); - }); - } else { - await csWownero.connect( - wallet!, - daemonAddress: "$host:${node.port}", - daemonUsername: node.loginName, - daemonPassword: await node.getPassword(secureStorageInterface), - trusted: node.trusted ?? false, - useSSL: node.useSSL, - socksProxyAddress: node.forceNoTor - ? null - : proxy == null - ? null - : "${proxy.host.address}:${proxy.port}", - ); + if (!isCurrent() || wallet == null) { + return; } + + final ({InternetAddress host, int port})? proxy = + AppConfig.hasFeature(AppFeature.tor) && + prefs.useTor && + !node.forceNoTor + ? TorService.sharedInstance.getProxyInfo() + : null; + await csWownero.connect( + wallet!, + daemonAddress: "$host:${node.port}", + daemonUsername: node.loginName, + daemonPassword: await node.getPassword(secureStorageInterface), + trusted: node.trusted ?? false, + useSSL: node.useSSL, + socksProxyAddress: proxy == null + ? null + : "${proxy.host.address}:${proxy.port}", + ); + + if (!isCurrent() || wallet == null) { + return; + } csWownero.startSyncing(wallet!); csWownero.startListeners(wallet!); csWownero.startAutoSaving(wallet!); @@ -566,8 +648,6 @@ abstract class LibWowneroWallet stackTrace: s, ); } - - return; } @override @@ -750,6 +830,13 @@ abstract class LibWowneroWallet @override Future exit() async { Logging.instance.i("exit called on $wallet!"); + await _lifecycle.close( + stopEventSources: _stopEventSources, + closeNative: _exitNative, + ); + } + + Future _exitNative() async { if (wallet != null) { csWownero.stopAutoSaving(wallet!); csWownero.stopListeners(wallet!); @@ -1524,7 +1611,9 @@ abstract class LibWowneroWallet // ============== View only ================================================== @override - Future recoverViewOnly() async { + Future recoverViewOnly() => _lifecycle.replaceNative(_recoverViewOnly); + + Future _recoverViewOnly() async { await refreshMutex.protect(() async { final data = await getViewOnlyWalletData() as CryptonoteViewOnlyWalletData; @@ -1555,8 +1644,8 @@ abstract class LibWowneroWallet height: height, ); - if (this.wallet == null) { - await exit(); + if (this.wallet != null) { + await _exitNative(); } this.wallet = wallet; @@ -1580,7 +1669,7 @@ abstract class LibWowneroWallet isar: mainDB.isar, ); - await updateNode(); + await _updateNode(() => _lifecycle.allowsNodeUpdates); _setListener(); unawaited(csWownero.rescanBlockchain(this.wallet!)); @@ -1605,6 +1694,7 @@ abstract class LibWowneroWallet StreamSubscription? _torStatusListener; StreamSubscription? _torPreferenceListener; - final Mutex _torConnectingLock = Mutex(); - bool _requireMutex = false; + final _torTransitionGate = CryptonoteTorTransitionGate(); + + final _lifecycle = CryptonoteWalletLifecycle(); } diff --git a/test/wallets/cryptonote_wallet_lifecycle_test.dart b/test/wallets/cryptonote_wallet_lifecycle_test.dart new file mode 100644 index 0000000000..924c52c28e --- /dev/null +++ b/test/wallets/cryptonote_wallet_lifecycle_test.dart @@ -0,0 +1,193 @@ +import 'dart:async'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:stackwallet/wallets/wallet/intermediate/cryptonote_wallet_lifecycle.dart'; + +void main() { + test("Tor transition gate waits for a status change", () async { + final gate = CryptonoteTorTransitionGate()..block(); + var blocked = true; + var completed = false; + + final wait = gate + .wait(isBlocked: () => blocked, isCurrent: () => true) + .then((value) { + completed = true; + return value; + }); + await Future.delayed(Duration.zero); + expect(completed, isFalse); + + blocked = false; + gate.release(); + expect(await wait, isTrue); + }); + + test("Tor transition gate aborts a superseded operation", () async { + final gate = CryptonoteTorTransitionGate()..block(); + var current = true; + + final wait = gate.wait(isBlocked: () => true, isCurrent: () => current); + current = false; + gate.release(); + + expect(await wait, isFalse); + }); + + test("subscription cleanup continues after one cancellation fails", () async { + var secondCancellation = 0; + final first = StreamController( + onCancel: () async => throw Exception("cancel failed"), + ); + final second = StreamController( + onCancel: () async => secondCancellation++, + ); + final subscriptions = [ + first.stream.listen((_) {}), + second.stream.listen((_) {}), + ]; + + await expectLater( + cancelCryptonoteWalletSubscriptions(subscriptions), + throwsA(isA()), + ); + expect(secondCancellation, 1); + await first.close(); + await second.close(); + }); + + test("close stops sources before waiting for an active update", () async { + final lifecycle = CryptonoteWalletLifecycle(); + final updateStarted = Completer(); + final releaseUpdate = Completer(); + final calls = []; + + final update = lifecycle.updateNode((_) async { + calls.add("update start"); + updateStarted.complete(); + await releaseUpdate.future; + calls.add("update end"); + }); + await updateStarted.future; + + final close = lifecycle.close( + stopEventSources: () async => calls.add("stop sources"), + closeNative: () async => calls.add("close native"), + ); + await Future.delayed(Duration.zero); + + expect(calls, ["update start", "stop sources"]); + releaseUpdate.complete(); + await Future.wait([update, close]); + expect(calls, [ + "update start", + "stop sources", + "update end", + "stop sources", + "close native", + ]); + }); + + test("queued node update is rejected after close begins", () async { + final lifecycle = CryptonoteWalletLifecycle(); + final openStarted = Completer(); + final releaseOpen = Completer(); + var updates = 0; + + final open = lifecycle.open((_) async { + openStarted.complete(); + await releaseOpen.future; + }); + await openStarted.future; + final queuedUpdate = lifecycle.updateNode((_) async => updates++); + final close = lifecycle.close( + stopEventSources: () async {}, + closeNative: () async {}, + ); + + releaseOpen.complete(); + await Future.wait([open, queuedUpdate, close]); + expect(updates, 0); + }); + + test("failed open rejects updates until a successful reopen", () async { + final lifecycle = CryptonoteWalletLifecycle(); + + await expectLater( + lifecycle.open((_) async => throw Exception("open failed")), + throwsException, + ); + await lifecycle.updateNode((_) async => fail("update must be rejected")); + + await lifecycle.open((_) async {}); + var updates = 0; + await lifecycle.updateNode((_) async => updates++); + expect(updates, 1); + }); + + test("close catches event sources attached by an active open", () async { + final lifecycle = CryptonoteWalletLifecycle(); + final openStarted = Completer(); + final releaseOpen = Completer(); + var sourceAttached = false; + var stops = 0; + + final open = lifecycle.open((isCurrent) async { + openStarted.complete(); + await releaseOpen.future; + sourceAttached = true; + expect(isCurrent(), isFalse); + }); + await openStarted.future; + final close = lifecycle.close( + stopEventSources: () async { + if (sourceAttached) { + sourceAttached = false; + stops++; + } + }, + closeNative: () async {}, + ); + + releaseOpen.complete(); + await Future.wait([open, close]); + expect(sourceAttached, isFalse); + expect(stops, 1); + }); + + test("queued native work is rejected after close begins", () async { + final lifecycle = CryptonoteWalletLifecycle(); + final stopStarted = Completer(); + final releaseStop = Completer(); + + final close = lifecycle.close( + stopEventSources: () async { + if (!stopStarted.isCompleted) { + stopStarted.complete(); + await releaseStop.future; + } + }, + closeNative: () async {}, + ); + await stopStarted.future; + final nativeWork = lifecycle.replaceNative(() async {}); + + releaseStop.complete(); + await close; + await expectLater(nativeWork, throwsStateError); + }); + + test("source cancellation failure does not skip native close", () async { + final lifecycle = CryptonoteWalletLifecycle(); + var closeCalls = 0; + + await expectLater( + lifecycle.close( + stopEventSources: () async => throw Exception("cancel failed"), + closeNative: () async => closeCalls++, + ), + throwsA(isA()), + ); + expect(closeCalls, 1); + }); +}