diff --git a/electrum/gui/qml/qeaddresslistmodel.py b/electrum/gui/qml/qeaddresslistmodel.py index fa7a546a6299..4ef9595e9aa9 100644 --- a/electrum/gui/qml/qeaddresslistmodel.py +++ b/electrum/gui/qml/qeaddresslistmodel.py @@ -123,9 +123,9 @@ def __init__(self, wallet: 'Abstract_Wallet', parent=None): self._filterModel = None self.register_callbacks() - self.destroyed.connect(lambda: self.on_destroy()) + self.destroyed.connect(self.on_destroy) - QEConfig.instance.freezeReusedAddressUtxosChanged.connect(lambda: self.setDirty()) + QEConfig.instance.freezeReusedAddressUtxosChanged.connect(self.setDirty) self._dirty = True self.initModel() diff --git a/electrum/gui/qml/qechanneldetails.py b/electrum/gui/qml/qechanneldetails.py index be9c2d525604..34ca52998373 100644 --- a/electrum/gui/qml/qechanneldetails.py +++ b/electrum/gui/qml/qechanneldetails.py @@ -50,7 +50,7 @@ def __init__(self, parent=None): self._is_closing = False self.register_callbacks() - self.destroyed.connect(lambda: self.on_destroy()) + self.destroyed.connect(self.on_destroy) @event_listener def on_event_channel(self, wallet: 'Abstract_Wallet', channel: 'AbstractChannel'): diff --git a/electrum/gui/qml/qechannellistmodel.py b/electrum/gui/qml/qechannellistmodel.py index 0079cdcd5b4f..f028bf9169ab 100644 --- a/electrum/gui/qml/qechannellistmodel.py +++ b/electrum/gui/qml/qechannellistmodel.py @@ -46,7 +46,7 @@ def __init__(self, wallet: 'Abstract_Wallet', parent=None): # methods of this class only, and specifically not be # partials, lambdas or methods of subobjects. Hence... self.register_callbacks() - self.destroyed.connect(lambda: self.on_destroy()) + self.destroyed.connect(self.on_destroy) @qt_event_listener def on_event_channel(self, wallet, channel): diff --git a/electrum/gui/qml/qedaemon.py b/electrum/gui/qml/qedaemon.py index 28a6640bd9d6..50a037a109c5 100644 --- a/electrum/gui/qml/qedaemon.py +++ b/electrum/gui/qml/qedaemon.py @@ -152,6 +152,19 @@ class QEDaemon(AuthMixin, QObject): walletDeleteError = pyqtSignal([str, str], arguments=['code', 'message']) walletRenameError = pyqtSignal([str], arguments=['message']) + __qewallet_instances = [] + + # this factory method should be used to instantiate QEWallet + # so we have only one QEWallet for each electrum.wallet + @classmethod + def getQEWalletInstanceFor(cls, wallet): + for i in cls.__qewallet_instances: + if i.wallet == wallet: + return i + i = QEWallet(wallet, QEDaemon.instance) + cls.__qewallet_instances.append(i) + return i + def __init__(self, daemon: 'Daemon', plugins: 'Plugins', parent=None): super().__init__(parent) if QEDaemon.instance: @@ -205,7 +218,7 @@ def loadWallet(self, path=None, password=None): wallet_already_open = self.daemon.get_wallet(self._path) if wallet_already_open is not None: - password = QEWallet.getInstanceFor(wallet_already_open).password + password = QEDaemon.getQEWalletInstanceFor(wallet_already_open).password def load_wallet_task(): success = False @@ -267,7 +280,7 @@ def _on_backend_wallet_loaded(self, password=None): self._logger.debug('_on_backend_wallet_loaded') wallet = self.daemon.get_wallet(self._path) assert wallet is not None - self._current_wallet = QEWallet.getInstanceFor(wallet) + self._current_wallet = QEDaemon.getQEWalletInstanceFor(wallet) self.availableWallets.updateWallet(self._path) wallet.unlock(password or None) # not conditional on wallet.requires_unlock in qml, as # the auth wrapper doesn't pass the entered password, but instead we rely on the password in memory @@ -301,9 +314,7 @@ def checkThenDeleteWallet(self, wallet, confirm_requests=False, confirm_balance= def delete_wallet(self, wallet): path = standardize_path(wallet.wallet.storage.get_path()) self._logger.debug('deleting wallet with path %s' % path) - self._current_wallet = None - # TODO walletLoaded signal is confusing - self.walletLoaded.emit(None, None) + self.unloadWallet(wallet) if not self.daemon.delete_wallet(path): self.walletDeleteError.emit('error', _('Problem deleting wallet')) @@ -349,13 +360,24 @@ def renameWallet(self, new_name: str): new_path = standardize_path(os.path.join(wallet_dir, new_name)) if old_path == new_path: return - self._current_wallet = None - self.daemon.stop_wallet(old_path) + self.unloadWallet(wallet) try: self.daemon.rename_wallet_file(old_path, new_path) except Exception as e: self.walletRenameError.emit(_('Error renaming wallet:\n') + str(e)) - self.walletLoaded.emit(None, None) + + # @pyqtSlot() + # TODO: make slot once the GUI/backend properly handles not ending up in a no wallet loaded scenario + def unloadWallet(self, wallet: QEWallet): + if wallet: + wallet_path = standardize_path(wallet.wallet.storage.get_path()) + self.daemon.stop_wallet(wallet_path) + QEDaemon.__qewallet_instances.remove(wallet) + if wallet == self._current_wallet: + self._current_wallet = None + # TODO walletLoaded signal is confusing + self.walletLoaded.emit(None, None) + wallet.deleteLater() @pyqtProperty(bool, notify=loadingChanged) def loading(self): diff --git a/electrum/gui/qml/qefx.py b/electrum/gui/qml/qefx.py index 8225a3a62fed..1ed71d5941c7 100644 --- a/electrum/gui/qml/qefx.py +++ b/electrum/gui/qml/qefx.py @@ -24,7 +24,7 @@ def __init__(self, fxthread: FxThread, config: SimpleConfig, parent=None): self.fx = fxthread self.config = config self.register_callbacks() - self.destroyed.connect(lambda: self.on_destroy()) + self.destroyed.connect(self.on_destroy) def on_destroy(self): self.unregister_callbacks() diff --git a/electrum/gui/qml/qeinvoice.py b/electrum/gui/qml/qeinvoice.py index 7069663f3441..3cc262b478eb 100644 --- a/electrum/gui/qml/qeinvoice.py +++ b/electrum/gui/qml/qeinvoice.py @@ -79,7 +79,7 @@ def __init__(self, parent=None): self._updating_max = False self.register_callbacks() - self.destroyed.connect(lambda: self.on_destroy()) + self.destroyed.connect(self.on_destroy) def on_destroy(self): self.unregister_callbacks() diff --git a/electrum/gui/qml/qeinvoicelistmodel.py b/electrum/gui/qml/qeinvoicelistmodel.py index 551afb19fa84..0e59c56104b1 100644 --- a/electrum/gui/qml/qeinvoicelistmodel.py +++ b/electrum/gui/qml/qeinvoicelistmodel.py @@ -182,7 +182,7 @@ class QEInvoiceListModel(QEAbstractInvoiceListModel, QtEventListener): def __init__(self, wallet, parent=None): super().__init__(wallet, parent) self.register_callbacks() - self.destroyed.connect(lambda: self.on_destroy()) + self.destroyed.connect(self.on_destroy) _logger = get_logger(__name__) @@ -217,7 +217,7 @@ class QERequestListModel(QEAbstractInvoiceListModel, QtEventListener): def __init__(self, wallet, parent=None): super().__init__(wallet, parent) self.register_callbacks() - self.destroyed.connect(lambda: self.on_destroy()) + self.destroyed.connect(self.on_destroy) _logger = get_logger(__name__) diff --git a/electrum/gui/qml/qenetwork.py b/electrum/gui/qml/qenetwork.py index 5130bf4f9cc2..bb4334634504 100644 --- a/electrum/gui/qml/qenetwork.py +++ b/electrum/gui/qml/qenetwork.py @@ -59,7 +59,7 @@ def __init__(self, network: 'Network', parent=None): self._height = network.get_local_height() # init here, update event can take a while self._server_height = network.get_server_height() # init here, update event can take a while self.register_callbacks() - self.destroyed.connect(lambda: self.on_destroy()) + self.destroyed.connect(self.on_destroy) QEConfig.instance.useGossipChanged.connect(self.on_gossip_setting_changed) diff --git a/electrum/gui/qml/qeqrscanner.py b/electrum/gui/qml/qeqrscanner.py index be93bbf1f288..d582b35f6f3d 100644 --- a/electrum/gui/qml/qeqrscanner.py +++ b/electrum/gui/qml/qeqrscanner.py @@ -1,6 +1,6 @@ import os -from PyQt6.QtCore import pyqtProperty, pyqtSignal, pyqtSlot, QObject, Qt +from PyQt6.QtCore import pyqtProperty, pyqtSignal, pyqtSlot, QObject, Qt, QMetaObject from PyQt6.QtGui import QGuiApplication from electrum.gui.qml.qetypes import QEBytes @@ -25,13 +25,11 @@ class QEQRScanner(QObject): foundText = pyqtSignal(str) foundBinary = pyqtSignal(QEBytes) - finished = pyqtSignal() def __init__(self, parent=None): super().__init__(parent) self._hint = _("Scan a QR code.") - self.finished.connect(self._unbind, Qt.ConnectionType.QueuedConnection) self.destroyed.connect(lambda: self.on_destroy()) @@ -78,6 +76,11 @@ def on_qr_activity_result(self, requestCode, resultCode, intent): send_exception_to_crash_reporter(e) finally: self.finished.emit() + self.unbind() + + def unbind(self): + # submit unbind request queued + QMetaObject.invokeMethod(self, '_unbind', Qt.ConnectionType.QueuedConnection) @pyqtSlot() def _unbind(self): @@ -88,4 +91,3 @@ def _scan_qr_non_android(self): data = QGuiApplication.clipboard().text() self.foundText.emit(data) self.finished.emit() - return diff --git a/electrum/gui/qml/qeserverlistmodel.py b/electrum/gui/qml/qeserverlistmodel.py index bac6578c0b6e..266fda6b40df 100644 --- a/electrum/gui/qml/qeserverlistmodel.py +++ b/electrum/gui/qml/qeserverlistmodel.py @@ -27,7 +27,7 @@ def __init__(self, network, parent=None): self.network = network self.initModel() self.register_callbacks() - self.destroyed.connect(lambda: self.unregister_callbacks()) + self.destroyed.connect(self.unregister_callbacks) @qt_event_listener def on_event_network_updated(self): diff --git a/electrum/gui/qml/qeswaphelper.py b/electrum/gui/qml/qeswaphelper.py index 8bbf9483df0c..8677619ce478 100644 --- a/electrum/gui/qml/qeswaphelper.py +++ b/electrum/gui/qml/qeswaphelper.py @@ -4,7 +4,7 @@ from typing import Union, Optional, TYPE_CHECKING, Sequence from PyQt6.QtCore import (pyqtProperty, pyqtSignal, pyqtSlot, QObject, QTimer, pyqtEnum, QAbstractListModel, Qt, - QModelIndex, QVariant) + QModelIndex, QVariant, QMetaObject) from PyQt6.QtGui import QColor from electrum.i18n import _ @@ -154,7 +154,6 @@ class State(IntEnum): error = pyqtSignal([str], arguments=['message']) undefinedNPub = pyqtSignal() offersUpdated = pyqtSignal() - requestTxUpdate = pyqtSignal() def __init__(self, parent=None): super().__init__(parent) @@ -192,7 +191,6 @@ def __init__(self, parent=None): self._fwd_swap_updatetx_timer = QTimer(self) self._fwd_swap_updatetx_timer.setSingleShot(True) self._fwd_swap_updatetx_timer.timeout.connect(self.fwd_swap_updatetx) - self.requestTxUpdate.connect(self.tx_update_pushback_timer) self.offersUpdated.connect(self.on_offers_updated) self.transport_task: Optional[asyncio.Task] = None @@ -606,10 +604,14 @@ def swap_slider_moved(self): else: # update tx only if slider isn't moved for a while self.valid = False - # trigger tx_update_pushback_timer through signal, as this might be called from other thread - self.requestTxUpdate.emit() + self.requestTxUpdate() - def tx_update_pushback_timer(self): + def requestTxUpdate(self): + # trigger _tx_update_pushback_timer from qt thread, as this might be called from other thread + QMetaObject.invokeMethod(self, '_tx_update_pushback_timer', Qt.ConnectionType.QueuedConnection) + + @pyqtSlot() + def _tx_update_pushback_timer(self): self._fwd_swap_updatetx_timer.start(250) def check_valid(self, send_amount, receive_amount): diff --git a/electrum/gui/qml/qetransactionlistmodel.py b/electrum/gui/qml/qetransactionlistmodel.py index 293e60ee3670..aaf3d9b8e648 100644 --- a/electrum/gui/qml/qetransactionlistmodel.py +++ b/electrum/gui/qml/qetransactionlistmodel.py @@ -1,7 +1,7 @@ from datetime import datetime, timedelta from typing import TYPE_CHECKING, Dict, Any -from PyQt6.QtCore import pyqtProperty, pyqtSignal, pyqtSlot +from PyQt6.QtCore import pyqtProperty, pyqtSignal, pyqtSlot, QMetaObject from PyQt6.QtCore import Qt, QAbstractListModel, QModelIndex from electrum.logging import get_logger @@ -27,8 +27,6 @@ class QETransactionListModel(QAbstractListModel, QtEventListener): _ROLE_MAP = dict(zip(_ROLE_KEYS, [bytearray(x.encode()) for x in _ROLE_NAMES])) _ROLE_RMAP = dict(zip(_ROLE_NAMES, _ROLE_KEYS)) - requestRefresh = pyqtSignal() - def __init__(self, wallet: 'Abstract_Wallet', parent=None, *, onchain_domain=None, include_lightning=True): super().__init__(parent) self.wallet = wallet @@ -38,8 +36,7 @@ def __init__(self, wallet: 'Abstract_Wallet', parent=None, *, onchain_domain=Non self.tx_history = [] self.register_callbacks() - self.destroyed.connect(lambda: self.on_destroy()) - self.requestRefresh.connect(lambda: self.initModel()) + self.destroyed.connect(self.on_destroy) self._dirty = True self.initModel() @@ -87,6 +84,10 @@ def on_event_labels_received(self, wallet, labels): if wallet == self.wallet: self.initModel(True) # TODO: be less dramatic + def requestRefresh(self): + # ensure execute on qt thread + QMetaObject.invokeMethod(self, 'initModel', Qt.ConnectionType.QueuedConnection) + def rowCount(self, index): return len(self.tx_history) diff --git a/electrum/gui/qml/qetxdetails.py b/electrum/gui/qml/qetxdetails.py index a5e4dabe03fd..ac4ceab76be0 100644 --- a/electrum/gui/qml/qetxdetails.py +++ b/electrum/gui/qml/qetxdetails.py @@ -30,8 +30,6 @@ class QETxDetails(QObject, QtEventListener): def __init__(self, parent=None): super().__init__(parent) - self.register_callbacks() - self.destroyed.connect(lambda: self.on_destroy()) self._wallet = None # type: Optional[QEWallet] self._txid = '' @@ -73,6 +71,9 @@ def __init__(self, parent=None): self._header_hash = '' self._short_id = "" + self.register_callbacks() + self.destroyed.connect(self.on_destroy) + def on_destroy(self): self.unregister_callbacks() @@ -407,6 +408,23 @@ def update_mined_status(self, tx_mined_info: TxMinedInfo): self._header_hash = tx_mined_info.header_hash self._short_id = tx_mined_info.short_id() or "" + def on_sign_success(self): + self._logger.debug('on_sign_success') + self.update() + + def on_sign_failed(self, failure): + self._logger.debug('on_sign_failed') + + def on_broadcast_success(self, *args): + self._logger.debug('on_broadcast_success') + self._can_broadcast = False + self.detailsChanged.emit() + + def on_broadcast_failed(self, *args): + self._logger.debug('on_broadcast_failed') + self._can_broadcast = True + self.detailsChanged.emit() + @pyqtSlot() def signAndBroadcast(self): self._sign(broadcast=True) @@ -416,69 +434,25 @@ def sign(self): self._sign(broadcast=False) def _sign(self, broadcast): - # TODO: connecting/disconnecting signal handlers here is hmm - try: - if broadcast: - self._wallet.broadcastSucceeded.disconnect(self.onBroadcastSucceeded) - self._wallet.broadcastFailed.disconnect(self.onBroadcastFailed) - except Exception: - pass - - if broadcast: - self._wallet.broadcastSucceeded.connect(self.onBroadcastSucceeded) - self._wallet.broadcastFailed.connect(self.onBroadcastFailed) - self._wallet.sign_and_broadcast(self._tx, on_success=self.on_signed_tx) - else: - self._wallet.sign(self._tx, on_success=self.on_signed_tx) - - # side-effect: signing updates self._tx - # we rely on this for broadcast + def on_sign_success(): + self._logger.debug('on_sign_success, broadcasting') + self.on_sign_success() # indicate sign success + if self._tx.is_complete(): + self._wallet.broadcast(self._tx, on_success=self.on_broadcast_success, on_failure=self.on_broadcast_failed) + else: + self._logger.warning('tx not complete, not broadcasting') - def on_signed_tx(self, tx: Transaction): - self._logger.debug('on_signed_tx') - self.update() + sign_success_cb = on_sign_success if broadcast else self.on_sign_success + self._wallet.sign(self._tx, on_success=sign_success_cb, on_failure=self.on_sign_failed) @pyqtSlot() def broadcast(self): assert self._tx.is_complete() - try: - self._wallet.broadcastFailed.disconnect(self.onBroadcastFailed) - except Exception: - pass - self._wallet.broadcastFailed.connect(self.onBroadcastFailed) - - self._can_broadcast = False - self.detailsChanged.emit() - - self._wallet.broadcast(self._tx) - - @pyqtSlot(str) - def onBroadcastSucceeded(self, txid): - if txid != self._txid: - return - - self._logger.debug('onBroadcastSucceeded') - try: - self._wallet.broadcastSucceeded.disconnect(self.onBroadcastSucceeded) - except Exception: - pass - self._can_broadcast = False self.detailsChanged.emit() - @pyqtSlot(str, str, str) - def onBroadcastFailed(self, txid, code, reason): - if txid != self._txid: - return - - try: - self._wallet.broadcastFailed.disconnect(self.onBroadcastFailed) - except Exception: - pass - - self._can_broadcast = True - self.detailsChanged.emit() + self._wallet.broadcast(self._tx, on_success=self.on_broadcast_success, on_failure=self.on_broadcast_failed) @pyqtSlot() @pyqtSlot(bool) diff --git a/electrum/gui/qml/qetxfinalizer.py b/electrum/gui/qml/qetxfinalizer.py index d0457c80ecbc..aea626890048 100644 --- a/electrum/gui/qml/qetxfinalizer.py +++ b/electrum/gui/qml/qetxfinalizer.py @@ -572,7 +572,7 @@ def sign(self): self._wallet.sign(self._tx, on_success=partial(self.on_signed_tx, True), on_failure=self.on_sign_failed) - def on_signed_tx(self, save: bool, tx: Transaction): + def on_signed_tx(self, save: bool): self._logger.debug('on_signed_tx') saved = False if save and self._tx.txid(): @@ -580,7 +580,7 @@ def on_signed_tx(self, save: bool, tx: Transaction): saved = True else: self._logger.error('Could not save tx') - self.finished.emit(True, saved, tx.is_complete()) + self.finished.emit(True, saved, self._tx.is_complete()) def on_sign_failed(self, msg: str = None): self._logger.debug('on_sign_failed') @@ -612,7 +612,7 @@ def __init__(self, parent=None): self._txid = '' self.register_callbacks() - self.destroyed.connect(lambda: self.on_destroy()) + self.destroyed.connect(self.on_destroy) def on_destroy(self): self.unregister_callbacks() @@ -1210,7 +1210,7 @@ def update(self): self._valid = True self.validChanged.emit() - self.on_signed_tx(False, tx) + self.on_signed_tx(False) @pyqtSlot() def send(self): diff --git a/electrum/gui/qml/qewallet.py b/electrum/gui/qml/qewallet.py index eeabf5261b43..172e96abd28b 100644 --- a/electrum/gui/qml/qewallet.py +++ b/electrum/gui/qml/qewallet.py @@ -3,7 +3,7 @@ import queue import threading import time -from typing import TYPE_CHECKING, Callable, Optional, Any, Tuple +from typing import TYPE_CHECKING, Callable, Optional, Tuple from functools import partial from PyQt6.QtCore import pyqtProperty, pyqtSignal, pyqtSlot, QObject, QTimer @@ -37,19 +37,6 @@ class QEWallet(AuthMixin, QObject, QtEventListener): - __instances = [] - - # this factory method should be used to instantiate QEWallet - # so we have only one QEWallet for each electrum.wallet - @classmethod - def getInstanceFor(cls, wallet): - for i in cls.__instances: - if i.wallet == wallet: - return i - i = QEWallet(wallet) - cls.__instances.append(i) - return i - _logger = get_logger(__name__) # emitted when wallet wants to display a user notification @@ -125,16 +112,13 @@ def __init__(self, wallet: 'Abstract_Wallet', parent=None): self.sync_progress_timer.setInterval(2000) self.sync_progress_timer.timeout.connect(self.update_sync_progress) - # post-construction init in GUI thread - # QMetaObject.invokeMethod(self, 'qt_init', Qt.QueuedConnection) - # To avoid leaking references to "self" that prevent the # window from being GC-ed when closed, callbacks should be # methods of this class only, and specifically not be # partials, lambdas or methods of subobjects. Hence... - self.register_callbacks() - self.destroyed.connect(lambda: self.on_destroy()) + + self.destroyed.connect(self.on_destroy) self.synchronizing = not wallet.is_up_to_date() synchronizingChanged = pyqtSignal() @@ -256,9 +240,6 @@ def on_event_payment_failed(self, wallet, key, reason): self.paymentFailed.emit(key, reason) def on_destroy(self): - if self not in QEWallet.__instances: - return - QEWallet.__instances.remove(self) self.unregister_callbacks() def add_tx_notification(self, tx: Transaction): @@ -301,35 +282,35 @@ def update_sync_progress(self): @pyqtProperty(QETransactionListModel, notify=historyModelChanged) def historyModel(self): if self._historyModel is None: - self._historyModel = QETransactionListModel(self.wallet) + self._historyModel = QETransactionListModel(self.wallet, parent=self) return self._historyModel addressCoinModelChanged = pyqtSignal() @pyqtProperty(QEAddressCoinListModel, notify=addressCoinModelChanged) def addressCoinModel(self): if self._addressCoinModel is None: - self._addressCoinModel = QEAddressCoinListModel(self.wallet) + self._addressCoinModel = QEAddressCoinListModel(self.wallet, parent=self) return self._addressCoinModel requestModelChanged = pyqtSignal() @pyqtProperty(QERequestListModel, notify=requestModelChanged) def requestModel(self): if self._requestModel is None: - self._requestModel = QERequestListModel(self.wallet) + self._requestModel = QERequestListModel(self.wallet, parent=self) return self._requestModel invoiceModelChanged = pyqtSignal() @pyqtProperty(QEInvoiceListModel, notify=invoiceModelChanged) def invoiceModel(self): if self._invoiceModel is None: - self._invoiceModel = QEInvoiceListModel(self.wallet) + self._invoiceModel = QEInvoiceListModel(self.wallet, parent=self) return self._invoiceModel channelModelChanged = pyqtSignal() @pyqtProperty(QEChannelListModel, notify=channelModelChanged) def channelModel(self): if self._channelModel is None: - self._channelModel = QEChannelListModel(self.wallet) + self._channelModel = QEChannelListModel(self.wallet, parent=self) return self._channelModel nameChanged = pyqtSignal() @@ -527,21 +508,21 @@ def enableLightning(self): @auth_protect(message=_('Sign and send on-chain transaction?')) def sign_and_broadcast(self, tx, *, - on_success: Callable[[Transaction], None] = None, - on_failure: Callable[[Optional[Any]], None] = None) -> None: + on_success: Callable[[], None] = None, + on_failure: Callable[[str], None] = None) -> None: self.do_sign(tx, True, on_success, on_failure) @auth_protect(message=_('Sign on-chain transaction?')) def sign(self, tx, *, - on_success: Callable[[Transaction], None] = None, - on_failure: Callable[[Optional[Any]], None] = None) -> None: + on_success: Callable[[], None] = None, + on_failure: Callable[[str], None] = None) -> None: self.do_sign(tx, False, on_success, on_failure) - def do_sign(self, tx, broadcast, on_success: Callable[[Transaction], None] = None, on_failure: Callable[[Optional[Any]], None] = None): + def do_sign(self, tx, broadcast, on_success: Callable[[], None] = None, on_failure: Callable[[str], None] = None): # tc_sign_wrapper is only used by 2fa. don't pass on_failure handler, it is handled via otpFailed signal sign_hook = run_hook('tc_sign_wrapper', self.wallet, tx, - partial(self.on_sign_complete, broadcast, on_success), - partial(self.on_sign_failed, None)) + partial(self.on_tc_sign_complete, broadcast, on_success), + partial(self.on_tc_sign_failed, None)) try: # ignore_warnings=True, because UI checks and asks user confirmation itself tx = self.wallet.sign_transaction(tx, self.password, ignore_warnings=True) @@ -554,7 +535,7 @@ def do_sign(self, tx, broadcast, on_success: Callable[[Transaction], None] = Non if tx is None: self._logger.info('did not sign') if on_failure: - on_failure() + on_failure(_('Could not sign')) return if sign_hook: @@ -576,18 +557,18 @@ def do_sign(self, tx, broadcast, on_success: Callable[[Transaction], None] = Non self.historyModel.initModel(True) if on_success: - on_success(tx) + on_success() - # this assumes a 2fa wallet, but there are no other tc_sign_wrapper hooks, so that's ok - def on_sign_complete(self, broadcast, cb: Callable[[Transaction], None] = None, tx: Transaction = None): + # trustedcoin tc_sign_wrapper + def on_tc_sign_complete(self, broadcast, cb: Callable[[], None] = None, tx: Transaction = None): self.otpSuccess.emit() if cb: - cb(tx) + cb() if broadcast: self.broadcast(tx) - # this assumes a 2fa wallet, but there are no other tc_sign_wrapper hooks, so that's ok - def on_sign_failed(self, cb: Callable[[], None] = None, error: str = None): + # trustedcoin tc_sign_wrapper + def on_tc_sign_failed(self, cb: Callable[[], None] = None, error: str = None): self.otpFailed.emit('error', error) if cb: cb() @@ -602,7 +583,13 @@ def submit_otp_task(): self._otp_on_submit(otp) threading.Thread(target=submit_otp_task, daemon=True).start() - def broadcast(self, tx): + def broadcast( + self, + tx, + *, + on_success: Callable[[], None] = None, + on_failure: Callable[[str], None] = None + ): assert tx.is_complete() async def broadcast_coro(): @@ -613,15 +600,24 @@ async def broadcast_coro(): except TxBroadcastError as e: self._logger.error(repr(e)) self.broadcastFailed.emit(tx.txid(), '', e.get_message_for_gui()) + if on_failure: + on_failure(e.get_message_for_gui()) except BestEffortRequestFailed as e: self._logger.error(repr(e)) self.broadcastFailed.emit(tx.txid(), '', repr(e)) - except Exception: + if on_failure: + on_failure(repr(e)) + except Exception as e: self._logger.exception("failed to broadcast tx") + self.broadcastFailed.emit(tx.txid(), '', repr(e)) + if on_failure: + on_failure(repr(e)) else: - self._logger.info('broadcast success') self.broadcastSucceeded.emit(tx.txid()) - self.historyModel.requestRefresh.emit() # via qt thread + if self._historyModel: + self._historyModel.requestRefresh() + if on_success: + on_success() finally: self.wallet.set_broadcasting(tx, broadcasting_status=None) diff --git a/electrum/plugins/labels/qml.py b/electrum/plugins/labels/qml.py index 9e43370f4072..84bacba25130 100644 --- a/electrum/plugins/labels/qml.py +++ b/electrum/plugins/labels/qml.py @@ -29,7 +29,7 @@ from electrum.i18n import _ from electrum.plugin import hook -from electrum.gui.qml.qewallet import QEWallet +from electrum.gui.qml.qedaemon import QEDaemon from electrum.gui.common_qt.plugins import PluginQObject from .labels import LabelsPlugin @@ -137,7 +137,7 @@ def pull_thread(_wallet): threading.Thread(target=pull_thread, args=[wallet]).start() def on_pulled(self, wallet): - _wallet = QEWallet.getInstanceFor(wallet) + _wallet = QEDaemon.getQEWalletInstanceFor(wallet) self.logger.debug('wallet ' + ('found' if _wallet else 'not found')) @hook diff --git a/electrum/plugins/trustedcoin/qml.py b/electrum/plugins/trustedcoin/qml.py index f2e66dab1abc..ca61728fec3b 100644 --- a/electrum/plugins/trustedcoin/qml.py +++ b/electrum/plugins/trustedcoin/qml.py @@ -4,7 +4,6 @@ from electrum.plugin import hook from electrum.util import UserFacingException -from electrum.gui.qml.qewallet import QEWallet from electrum.gui.qml.qedaemon import QEDaemon from .common_qt import TrustedcoinPluginQObject @@ -107,7 +106,7 @@ def prompt_user_for_otp(self, wallet, tx, on_success, on_failure): self.on_failure = on_failure if on_failure else lambda x: self.logger.error(x) self.wallet = wallet self.tx = tx - qewallet = QEWallet.getInstanceFor(wallet) + qewallet = QEDaemon.getQEWalletInstanceFor(wallet) qewallet.request_otp(self.on_otp) def on_otp(self, otp): @@ -133,6 +132,6 @@ def on_otp(self, otp): def billing_info_retrieved(self, wallet): self.logger.info('billing_info_retrieved') - qewallet = QEWallet.getInstanceFor(wallet) + qewallet = QEDaemon.getQEWalletInstanceFor(wallet) qewallet.billingInfoChanged.emit() self.so.updateBillingInfo(wallet) diff --git a/tests/qml/qt_util.py b/tests/qml/qt_util.py index cad939ed6f51..f91a1293d837 100644 --- a/tests/qml/qt_util.py +++ b/tests/qml/qt_util.py @@ -1,10 +1,19 @@ +import gc import threading import traceback import unittest from functools import wraps, partial +from typing import List, Sequence from unittest import SkipTest -from PyQt6.QtCore import QCoreApplication, QMetaObject, Qt, pyqtSlot, QObject +from PyQt6.QtCore import QCoreApplication, QMetaObject, Qt, pyqtSlot, QObject, QEventLoop, QTimer, QEvent + +from electrum.util import create_and_start_event_loop + +from electrum.logging import get_logger + + +logger = get_logger(__name__) class TestQCoreApplication(QCoreApplication): @@ -16,62 +25,149 @@ def doInvoke(self): class QEventReceiver(QObject): def __init__(self, *signals): super().__init__() + self._lock = threading.Lock() self.received = [] self.signals = [] for signal in signals: self.signals.append(signal) - signal.connect(partial(self.doReceive, signal)) + signal.connect(partial(self._doReceive, signal)) # intentionally no pyqtSlot decorator, to catch all - def doReceive(self, signal, *args): - self.received.append((signal, args)) - - def receivedForSignal(self, signal): - return list(filter(lambda x: x[0] == signal, self.received)) + def _doReceive(self, signal, *args): + logger.debug(f'received {signal=} {repr(args)}') + with self._lock: + self.received.append((signal, args)) + + def receivedForSignal(self, signal) -> List: + with self._lock: + return list(filter(lambda x: x[0] == signal, self.received)) + + def receivedExactSequence(self, signals: List) -> bool: + """check if the exact signal sequence was received + if the signals parameter is a list of tuples/lists, the received + signal parameters are checked as well + """ + with self._lock: + if len(self.received) != len(signals): + logger.error(f'num of received signals {len(self.received)} != num of required signals {len(signals)}') + return False + + for i in range(0, len(signals)): + signal = signals[i] + rcvd = self.received[i] + if not isinstance(signal, Sequence): + # ignore the received signal args + rcvd = rcvd[0] + if rcvd == signal: + continue + logger.error(f'received signal {rcvd} was unexpected at #{i}\n' + f'received: {repr(rcvd)}\n' + f'expected: {repr(signal)}\n') + return False + + return True def clear(self): - self.received.clear() + with self._lock: + self.received.clear() class QETestCase(unittest.TestCase): - def setUp(self): - super().setUp() - self.app = None - self._e = None - self._testcase_event = threading.Event() - self._app_ready_event = threading.Event() + @classmethod + def setUpClass(cls): + # One QCoreApplication on its own thread and one asyncio loop for the + # whole test case. Per-test object cleanup is handled by qt_teardown() + super().setUpClass() + cls.app = None + cls._app_ready_event = threading.Event() def start_qt_task(): try: - assert self.app is None - self.app = TestQCoreApplication([]) - self._app_ready_event.set() - self.app.exec() - self.app = None + assert cls.app is None + cls.app = TestQCoreApplication([]) + cls._app_ready_event.set() + logger.debug('about to start QApplication') + cls.app.exec() + logger.debug('QApplication stopped') + cls.app = None except Exception as e: - print(f'Problem starting QCoreApplication: {str(e)}') + logger.exception(f'Problem starting QCoreApplication: {str(e)}') + + cls._qt_thread = threading.Thread(target=start_qt_task, name='QtTestThread') + cls._qt_thread.start() + cls._loop, cls._stopping_fut, cls._loop_thread = create_and_start_event_loop() - self._qt_thread = threading.Thread(target=start_qt_task) - self._qt_thread.start() + if not cls._app_ready_event.wait(3): + raise Exception('app not ready in time') + logger.debug(f'started event loop {cls._loop=}, {cls._loop_thread=}') - def tearDown(self): - self.app.exit() - if self._qt_thread.is_alive(): - self._qt_thread.join() + @classmethod + def tearDownClass(cls): + cls.app.exit() + if cls._qt_thread.is_alive(): + cls._qt_thread.join() + + def _resolve_stopping_fut(): + if not cls._stopping_fut.done(): + cls._stopping_fut.set_result(1) + + try: + cls._loop.call_soon_threadsafe(_resolve_stopping_fut) + except RuntimeError: + pass # loop already stopped/closed + if cls._loop_thread.is_alive(): + cls._loop_thread.join() + super().tearDownClass() + + def setUp(self): + super().setUp() + self._e = None + self._testcase_event = threading.Event() + + def qt_teardown(self): + """override to destroy QObjects created during the test""" + pass + + def waitForSignal(self, receiver, signal, *, timeout=5.0): + if receiver.receivedForSignal(signal): + return True + loop = QEventLoop() + signal.connect(loop.quit) + timer = QTimer() + timer.setSingleShot(True) + timer.timeout.connect(loop.quit) + timer.start(int(timeout * 1000)) + try: + # exec() returns when either the awaited signal or the timeout fires; + # re-check the predicate and re-enter in case of a spurious wakeup. + while not receiver.receivedForSignal(signal) and timer.isActive(): + loop.exec() + finally: + timer.stop() + try: + signal.disconnect(loop.quit) + except (TypeError, RuntimeError): + pass + return bool(receiver.receivedForSignal(signal)) def qt_test(func): @wraps(func) def decorator(self, *args): - if threading.current_thread().name == 'MainThread': + logger.debug(f'qt_test decorator, thread={threading.current_thread().name}') + if threading.current_thread().name != 'QtTestThread': res = self._app_ready_event.wait(3) if not res: raise Exception('app not ready in time') self._testcase_event.clear() self.app._instance = self self.app._method = func.__name__ - QMetaObject.invokeMethod(self.app, 'doInvoke', Qt.ConnectionType.QueuedConnection) + try: + QMetaObject.invokeMethod(self.app, 'doInvoke', Qt.ConnectionType.QueuedConnection) + except Exception as e: + logger.exception(f'exception calling invokeMethod on TestQCoreApplication.doInvoke(...): {str(e)}') + res = self._testcase_event.wait(15) if not res: self._e = Exception('testcase timed out') @@ -94,5 +190,33 @@ def decorator(self, *args): except Exception as e: self._e = e finally: + # teardown on the QtTestThread, while the app event loop is still running, + # so subclasses can destroy the QObjects they created on their owning thread. + try: + self.qt_teardown() + except Exception as e: + if self._e is None: + self._e = e + # QObject.deleteLater() merely posts a DeferredDelete event, which Qt only + # delivers once the event loop unwinds to the loop level at which it was + # posted. This test body runs *inside* app.exec() (one level deep, invoked + # via doInvoke), so neither returning here nor a processEvents() call (which + # runs at a deeper level) delivers those events. The C++ objects would then + # linger until some later, racy spin of the QtTestThread's loop, and with + # them everything they reference (e.g. a QEWallet's self.wallet). Flush them + # synchronously now so QObject destruction is deterministic and complete by + # the time the test is considered done. + try: + self.app.sendPostedEvents(None, QEvent.Type.DeferredDelete) + # Force a cyclic GC pass now that the QObjects are gone. The test's + # wallets and helpers sit in reference cycles (e.g. wallet<->txbatcher) + # and would otherwise survive until some later, unrelated gc.collect(). + # Collecting here runs their EventListener.__del__, which unregisters + # their callbacks, so we don't pollute global state (electrum.util. + # callback_mgr) for the tests that run after this one. + gc.collect() + except Exception as e: + if self._e is None: + self._e = e self._testcase_event.set() return decorator diff --git a/tests/qml/test_qewallet.py b/tests/qml/test_qewallet.py new file mode 100644 index 000000000000..62da13469da5 --- /dev/null +++ b/tests/qml/test_qewallet.py @@ -0,0 +1,213 @@ +import gc +import shutil +import tempfile +import weakref + +from PyQt6.QtCore import QEvent + +from electrum import SimpleConfig, keystore, Network +from electrum.address_synchronizer import TX_HEIGHT_UNCONFIRMED +from electrum.fee_policy import FixedFeePolicy +from electrum.gui.qml.qedaemon import QEDaemon +from electrum.gui.qml.qeconfig import QEConfig +from tests.qml.qt_util import QETestCase, QEventReceiver, qt_test +from electrum.transaction import PartialTxOutput, Transaction +from tests.test_wallet_vertical import WalletIntegrityHelper + + +class NetworkMock: + relay_fee = 1000 + + async def get_transaction(self, txid, timeout=None): + if txid == "08557327673db61cc921e1a30826608599b86457836be3021105c13940d9a9a3": + return "02000000000101a5883f3de780d260e6f26cf85144403c7744a65a44cd38f9ff45aecadf010c540100000000fdffffff0220a1070000000000160014db44724ac632ae47ee5765954d64796dd5fec72708de3c000000000016001424b32aadb42a89016c4de8f11741c3b29b15f21c02473044022045cc6c1cc875cbb0c0d8fe323dc1de9716e49ed5659741b0fb3dd9a196894066022077c242640071d12ec5763c5870f482a4823d8713e4bd14353dd621ed29a7f96d012102aea8d439a0f79d8b58e8d7bda83009f587e1f3da350adaa484329bf47cd03465fef61c00" + else: + raise Exception("unexpected txid") + + def has_internet_connection(self): + return True + + run_from_another_thread = Network.run_from_another_thread + + def get_local_height(self): + return 0 + + def blockchain(self): + class BlockchainMock: + def is_tip_stale(self): + return True + + return BlockchainMock() + + def is_connected(self): + return False + + +class TestQEWallet(QETestCase): + + def setUp(self): + super().setUp() + self.electrum_path = tempfile.mkdtemp() + self.config = SimpleConfig({ + 'electrum_path': self.electrum_path, + 'decimal_point': 5 + }) + # QEConfig singleton is assumed always present + QEConfig(self.config) + + def tearDown(self): + super().tearDown() + # drop any QEWallet that survived qt_teardown (e.g. if a test + # failed before it ran) + QEDaemon._QEDaemon__qewallet_instances.clear() + QEConfig.instance = None + shutil.rmtree(self.electrum_path) + + def qt_teardown(self): + # runs on QtTestThread + instances = QEDaemon._QEDaemon__qewallet_instances + for qw in list(instances): + instances.remove(qw) + qw.on_destroy() + qw.deleteLater() + self.app.processEvents() + + def create_standard_wallet_from_seed(self, seed_words, *, with_password=None): + # seed_words = 'bitter grass shiver impose acquire brush forget axis eager alone wine silver' + ks = keystore.from_seed(seed_words, passphrase='', for_multisig=False) + if with_password: + if not ks.may_have_password(): + raise Exception('cannot have password') + ks.update_password(None, with_password) + wallet = WalletIntegrityHelper.create_standard_wallet(ks, config=self.config) + wallet.network = NetworkMock() + + if with_password: + wallet.db.put('use_encryption', True) + return wallet + + @qt_test + def test_single_qobject_per_wallet_instance(self): + wallet = self.create_standard_wallet_from_seed('bitter grass shiver impose acquire brush forget axis eager alone wine silver') + + qw = QEDaemon.getQEWalletInstanceFor(wallet) + self.assertIsNotNone(qw) + + qw2 = QEDaemon.getQEWalletInstanceFor(wallet) + self.assertTrue(qw == qw2) + + @qt_test + def test_standard_wallet_properties(self): + wallet1 = self.create_standard_wallet_from_seed('bitter grass shiver impose acquire brush forget axis eager alone wine silver', + with_password='password') + qw1 = QEDaemon.getQEWalletInstanceFor(wallet1) + + self.assertTrue(qw1.isDeterministic) + self.assertTrue(qw1.hasSeed) + self.assertFalse(qw1.isWatchOnly) + self.assertFalse(qw1.isMultisig) + # self.assertTrue(qw1.isEncrypted) # only checks storage encryption a.t.m + self.assertTrue(qw1.canHaveLightning) + self.assertTrue(qw1.canSignMessage) + self.assertTrue(qw1.canSignWithoutServer) + self.assertTrue(qw1.canSignWithoutCosigner) + self.assertEqual(qw1.derivationPrefix, 'm/0h') + + @qt_test + def test_auth_protected_methods(self): + wallet1 = self.create_standard_wallet_from_seed('bitter grass shiver impose acquire brush forget axis eager alone wine silver', + with_password='password') + wallet2 = self.create_standard_wallet_from_seed('cycle rocket west magnet parrot shuffle foot correct salt library feed song') + + # fund + funding_tx = Transaction( + '01000000014576dacce264c24d81887642b726f5d64aa7825b21b350c7b75a57f337da6845010000006b483045022100a3f8b6155c71a98ad9986edd6161b20d24fad99b6463c23b463856c0ee54826d02200f606017fd987696ebbe5200daedde922eee264325a184d5bbda965ba5160821012102e5c473c051dae31043c335266d0ef89c1daab2f34d885cc7706b267f3269c609ffffffff0240420f00000000001600148a28bddb7f61864bdcf58b2ad13d5aeb3abc3c42a2ddb90e000000001976a914c384950342cb6f8df55175b48586838b03130fad88ac00000000') + funding_txid = funding_tx.txid() + funding_output_value = 1000000 + self.assertEqual('add2535aedcbb5ba79cc2260868bb9e57f328738ca192937f2c92e0e94c19203', funding_txid) + wallet1.adb.receive_tx_callback(funding_tx, tx_height=TX_HEIGHT_UNCONFIRMED) + + qw1 = QEDaemon.getQEWalletInstanceFor(wallet1) + wallet1.unlock('password') + qw2 = QEDaemon.getQEWalletInstanceFor(wallet2) + + # wallet1 -> wallet2 + outputs = [PartialTxOutput.from_address_and_value(wallet2.get_receiving_address(), 250000)] + coins = wallet1.get_spendable_coins() + tx = wallet1.make_unsigned_transaction( + coins=coins, + outputs=outputs, + fee_policy=FixedFeePolicy(100), + ) + + def sign_success(*args): + self._signed = True + + def sign_failed(*args): + self._signed = False + + er1 = QEventReceiver(qw1.authRequired, qw1.paymentFailed) + qw1.sign(tx, on_success=sign_success, on_failure=sign_failed) + + self.assertTrue(er1.receivedForSignal(qw1.authRequired)) + self.assertTrue(er1.receivedExactSequence([(qw1.authRequired, ('payment_auth', 'Sign on-chain transaction?'))])) + + qw1.authProceed() + + self.assertTrue(self._signed) + + er1.clear() + + qw1.requestShowSeed() + self.assertTrue(er1.receivedForSignal(qw1.authRequired)) + qw1.authProceed() + self.assertEqual(qw1.seed, 'bitter grass shiver impose acquire brush forget axis eager alone wine silver') + + er1.clear() + + class InvoiceMock: + def get_id(self): + return '1' + + def get_amount_msat(self): + return '1' + + def is_lightning(self): + return True + + # stub for serialize to db + def to_json(self): + return {} + + qw1.pay_lightning_invoice(InvoiceMock()) + self.assertTrue(er1.receivedForSignal(qw1.authRequired)) + qw1.authProceed() + self.assertTrue(self.waitForSignal(er1, qw1.paymentFailed)) + + self.assertTrue(er1.receivedExactSequence([ + (qw1.authRequired, ('payment_auth', 'Pay lightning invoice?')), + qw1.paymentFailed + ])) + + @qt_test + def test_bound_on_destroy_does_not_leak_wrapper(self): + # QEWallet connects destroyed to the *bound* on_destroy method, not a + # self-capturing lambda, so PyQt holds the receiver weakly and the wrapper + # is collectible once the C++ object is gone. The daemon's registry is the + # only remaining strong ref; unloadWallet() drops it in production, so we + # do the same here. Guards against regressing to a leaking lambda slot or a + # strong registry hold. + wallet = self.create_standard_wallet_from_seed( + 'bitter grass shiver impose acquire brush forget axis eager alone wine silver') + qw = QEDaemon.getQEWalletInstanceFor(wallet) + qw_ref = weakref.ref(qw) + + QEDaemon._QEDaemon__qewallet_instances.remove(qw) + qw.deleteLater() + # deliver DeferredDelete -> destroy C++ object -> emit destroyed -> on_destroy() + self.app.sendPostedEvents(None, QEvent.Type.DeferredDelete) + + del qw, wallet + gc.collect() + gc.collect() + self.assertIsNone(qw_ref(), "QEWallet wrapper leaked after destruction (self-capturing slot or strong registry?)") diff --git a/tests/qml/test_qml_qeconfig.py b/tests/qml/test_qml_qeconfig.py index c6de3156f3d9..10ee8d82dada 100644 --- a/tests/qml/test_qml_qeconfig.py +++ b/tests/qml/test_qml_qeconfig.py @@ -12,12 +12,12 @@ class TestConfig(QETestCase): @classmethod def setUpClass(cls): + super().setUpClass() QEConfig(SimpleConfig()) def setUp(self): super().setUp() self.q: QEConfig = QEConfig.instance - # raise Exception() # NOTE: exceptions in setUp() will block the test @qt_test def test_satstounits(self):