From 7ca9b735d51633d0d7812a20fbd6050ffbf6ff08 Mon Sep 17 00:00:00 2001 From: Sander van Grieken Date: Tue, 10 Oct 2023 16:57:44 +0200 Subject: [PATCH] daemon: refactor load_wallet to not just return None, but raise specific exceptions. The following exceptions should be expected: FileNotFoundError: given wallet path does not exist StorageReadWriteError: given file is not readable/writable or containing folder is not writable InvalidPassword: wallet requires a password but no password or an invalid password was given WalletFileException: any internal wallet data issue. specific subclasses can be caught separately: - WalletRequiresSplit: wallet needs splitting (split_data passed in Exception) - WalletRequiresUpgrade: wallet needs upgrade, and no upgrade=True was passed to load_wallet - WalletUnfinished: wallet file contains an action and needs additional information to finalize. (WalletDB passed in exception) Removed qml/qewalletdb.py This patch also fixes load_wallet calls in electrum/scripts and adds a qml workaround for dialogs opening and closing so fast that the dialog opened==true property change is missed (which we need to manage the dialog/page stack) --- electrum/daemon.py | 21 +-- .../qml/components/LoadingWalletDialog.qml | 16 +- .../gui/qml/components/controls/ElDialog.qml | 5 + electrum/gui/qml/components/main.qml | 18 +- electrum/gui/qml/qeapp.py | 2 - electrum/gui/qml/qedaemon.py | 35 ++-- electrum/gui/qml/qewalletdb.py | 176 ------------------ electrum/gui/qt/__init__.py | 28 +-- electrum/gui/qt/wizard/wallet.py | 13 +- electrum/scripts/ln_features.py | 2 +- electrum/scripts/quick_start.py | 2 +- electrum/wallet_db.py | 14 +- 12 files changed, 85 insertions(+), 247 deletions(-) delete mode 100644 electrum/gui/qml/qewalletdb.py diff --git a/electrum/daemon.py b/electrum/daemon.py index dc6be9baf..4cfd0012d 100644 --- a/electrum/daemon.py +++ b/electrum/daemon.py @@ -24,6 +24,7 @@ # SOFTWARE. import asyncio import ast +import errno import os import time import traceback @@ -41,13 +42,13 @@ from aiorpcx import timeout_after, TaskTimeout, ignore_after from . import util from .network import Network -from .util import (json_decode, to_bytes, to_string, profiler, standardize_path, constant_time_compare) +from .util import (json_decode, to_bytes, to_string, profiler, standardize_path, constant_time_compare, InvalidPassword) from .invoices import PR_PAID, PR_EXPIRED from .util import log_exceptions, ignore_exceptions, randrange, OldTaskGroup from .util import EventListener, event_listener from .wallet import Wallet, Abstract_Wallet from .storage import WalletStorage -from .wallet_db import WalletDB, WalletRequiresSplit, WalletRequiresUpgrade +from .wallet_db import WalletDB, WalletRequiresSplit, WalletRequiresUpgrade, WalletUnfinished from .commands import known_commands, Commands from .simple_config import SimpleConfig from .exchange_rate import FxThread @@ -488,8 +489,6 @@ class Daemon(Logger): if wallet := self._wallets.get(wallet_key): return wallet wallet = self._load_wallet(path, password, upgrade=upgrade, config=self.config) - if wallet is None: - return wallet.start_network(self.network) self.add_wallet(wallet) return wallet @@ -506,20 +505,15 @@ class Daemon(Logger): path = standardize_path(path) storage = WalletStorage(path) if not storage.file_exists(): - return + raise FileNotFoundError(errno.ENOENT, os.strerror(errno.ENOENT), path) if storage.is_encrypted(): if not password: - return + raise InvalidPassword('No password given') storage.decrypt(password) # read data, pass it to db - try: - db = WalletDB(storage.read(), storage=storage, upgrade=upgrade) - except WalletRequiresSplit: - return - except WalletRequiresUpgrade: - return + db = WalletDB(storage.read(), storage=storage, upgrade=upgrade) if db.get_action(): - return + raise WalletUnfinished(db) wallet = Wallet(db, config=config) return wallet @@ -659,7 +653,6 @@ class Daemon(Logger): pass except Exception: self.logger.exception(f'failed to load wallet at {path!r}:') - pass if wallet is None: failed.append(path) continue diff --git a/electrum/gui/qml/components/LoadingWalletDialog.qml b/electrum/gui/qml/components/LoadingWalletDialog.qml index cbe8408bb..ee2ac8c98 100644 --- a/electrum/gui/qml/components/LoadingWalletDialog.qml +++ b/electrum/gui/qml/components/LoadingWalletDialog.qml @@ -19,6 +19,10 @@ ElDialog { y: Math.floor((parent.height - implicitHeight) / 2) // anchors.centerIn: parent // this strangely pixelates the spinner + function open() { + showTimer.start() + } + ColumnLayout { width: parent.width @@ -32,8 +36,18 @@ ElDialog { Connections { target: Daemon function onLoadingChanged() { - if (!Daemon.loading) + console.log('daemon loading ' + Daemon.loading) + if (!Daemon.loading) { + showTimer.stop() dialog.close() + } } } + + Timer { + id: showTimer + interval: 250 + repeat: false + onTriggered: dialog.visible = true + } } diff --git a/electrum/gui/qml/components/controls/ElDialog.qml b/electrum/gui/qml/components/controls/ElDialog.qml index f663811e7..b1c80eb73 100644 --- a/electrum/gui/qml/components/controls/ElDialog.qml +++ b/electrum/gui/qml/components/controls/ElDialog.qml @@ -11,6 +11,8 @@ Dialog { property bool resizeWithKeyboard: true property bool _result: false + // workaround: remember opened state, to inhibit closed -> closed event + property bool _wasOpened: false // called to finally close dialog after checks by onClosing handler in main.qml function doClose() { @@ -46,7 +48,10 @@ Dialog { onOpenedChanged: { if (opened) { app.activeDialogs.push(abstractdialog) + _wasOpened = true } else { + if (!_wasOpened) + return if (app.activeDialogs.indexOf(abstractdialog) < 0) { console.log('dialog should exist in activeDialogs!') app.activeDialogs.pop() diff --git a/electrum/gui/qml/components/main.qml b/electrum/gui/qml/components/main.qml index bf56b9666..160ff35bb 100644 --- a/electrum/gui/qml/components/main.qml +++ b/electrum/gui/qml/components/main.qml @@ -498,17 +498,21 @@ ApplicationWindow property var _opendialog: undefined + function showOpenWalletDialog(name, path) { + if (_opendialog == undefined) { + _opendialog = openWalletDialog.createObject(app, { name: name, path: path }) + _opendialog.closed.connect(function() { + _opendialog = undefined + }) + _opendialog.open() + } + } + Connections { target: Daemon function onWalletRequiresPassword(name, path) { console.log('wallet requires password') - if (_opendialog == undefined) { - _opendialog = openWalletDialog.createObject(app, { path: path, name: name }) - _opendialog.closed.connect(function() { - _opendialog = undefined - }) - _opendialog.open() - } + showOpenWalletDialog(name, path) } function onWalletOpenError(error) { console.log('wallet open error') diff --git a/electrum/gui/qml/qeapp.py b/electrum/gui/qml/qeapp.py index 99ac72123..a28522f3e 100644 --- a/electrum/gui/qml/qeapp.py +++ b/electrum/gui/qml/qeapp.py @@ -25,7 +25,6 @@ from .qedaemon import QEDaemon from .qenetwork import QENetwork from .qewallet import QEWallet from .qeqr import QEQRParser, QEQRImageProvider, QEQRImageProviderHelper -from .qewalletdb import QEWalletDB from .qebitcoin import QEBitcoin from .qefx import QEFX from .qetxfinalizer import QETxFinalizer, QETxRbfFeeBumper, QETxCpfpFeeBumper, QETxCanceller @@ -332,7 +331,6 @@ class ElectrumQmlApplication(QGuiApplication): ElectrumQmlApplication._daemon = daemon qmlRegisterType(QEWallet, 'org.electrum', 1, 0, 'Wallet') - qmlRegisterType(QEWalletDB, 'org.electrum', 1, 0, 'WalletDB') qmlRegisterType(QEBitcoin, 'org.electrum', 1, 0, 'Bitcoin') qmlRegisterType(QEQRParser, 'org.electrum', 1, 0, 'QRParser') qmlRegisterType(QEFX, 'org.electrum', 1, 0, 'FX') diff --git a/electrum/gui/qml/qedaemon.py b/electrum/gui/qml/qedaemon.py index 70bd2684b..cc448171e 100644 --- a/electrum/gui/qml/qedaemon.py +++ b/electrum/gui/qml/qedaemon.py @@ -8,16 +8,16 @@ from PyQt5.QtCore import pyqtProperty, pyqtSignal, pyqtSlot, QObject from electrum.i18n import _ from electrum.logging import get_logger -from electrum.util import WalletFileException, standardize_path +from electrum.util import WalletFileException, standardize_path, InvalidPassword, send_exception_to_crash_reporter from electrum.plugin import run_hook from electrum.lnchannel import ChannelState from electrum.bitcoin import is_address from electrum.ecc import verify_message_with_address +from electrum.storage import StorageReadWriteError from .auth import AuthMixin, auth_protect from .qefx import QEFX from .qewallet import QEWallet -from .qewalletdb import QEWalletDB from .qewizard import QENewWalletWizard, QEServerConnectWizard if TYPE_CHECKING: @@ -152,10 +152,6 @@ class QEDaemon(AuthMixin, QObject): self._backendWalletLoaded.connect(self._on_backend_wallet_loaded) - self._walletdb = QEWalletDB() - self._walletdb.validPasswordChanged.connect(self.passwordValidityCheck) - self._walletdb.walletOpenProblem.connect(self.onWalletOpenProblem) - @pyqtSlot() def passwordValidityCheck(self): if not self._walletdb._validPassword: @@ -192,25 +188,27 @@ class QEDaemon(AuthMixin, QObject): wallet_already_open = self.daemon.get_wallet(self._path) is not None - if not wallet_already_open: - # pre-checks, let walletdb trigger any necessary user interactions - self._walletdb.path = self._path - self._walletdb.password = password - self._walletdb.verify() - if not self._walletdb.ready: - return - def load_wallet_task(): self._loading = True self.loadingChanged.emit() try: local_password = password # need this in local scope - wallet = self.daemon.load_wallet(self._path, local_password, upgrade=True) + wallet = None + try: + wallet = self.daemon.load_wallet(self._path, local_password, upgrade=True) + except InvalidPassword: + self.walletRequiresPassword.emit(self._name, self._path) + except FileNotFoundError: + self.walletOpenError.emit(_('File not found')) + except StorageReadWriteError: + self.walletOpenError.emit(_('Could not read/write file')) + except WalletFileException as e: + self.walletOpenError.emit(_('Could not open wallet: {}').format(str(e))) + if e.should_report_crash: + send_exception_to_crash_reporter(e) if wallet is None: - self._logger.info('could not open wallet') - self.walletOpenError.emit('could not open wallet') return if wallet_already_open: @@ -231,9 +229,6 @@ class QEDaemon(AuthMixin, QObject): run_hook('load_wallet', wallet) self._backendWalletLoaded.emit(local_password) - except WalletFileException as e: - self._logger.error(f"load_wallet_task errored opening wallet: {e!r}") - self.walletOpenError.emit(str(e)) finally: self._loading = False self.loadingChanged.emit() diff --git a/electrum/gui/qml/qewalletdb.py b/electrum/gui/qml/qewalletdb.py deleted file mode 100644 index 5bd705666..000000000 --- a/electrum/gui/qml/qewalletdb.py +++ /dev/null @@ -1,176 +0,0 @@ -from typing import TYPE_CHECKING - -from PyQt5.QtCore import pyqtProperty, pyqtSignal, pyqtSlot, QObject - -from electrum.i18n import _ -from electrum.logging import get_logger -from electrum.storage import WalletStorage -from electrum.wallet_db import WalletDB, WalletRequiresSplit -from electrum.wallet import Wallet -from electrum.util import InvalidPassword, WalletFileException, send_exception_to_crash_reporter - -if TYPE_CHECKING: - from electrum.simple_config import SimpleConfig - - -class QEWalletDB(QObject): - _logger = get_logger(__name__) - - fileNotFound = pyqtSignal() - walletOpenProblem = pyqtSignal([str], arguments=['error']) - pathChanged = pyqtSignal([bool], arguments=['ready']) - needsPasswordChanged = pyqtSignal() - needsHWDeviceChanged = pyqtSignal() - passwordChanged = pyqtSignal() - validPasswordChanged = pyqtSignal() - readyChanged = pyqtSignal() - invalidPassword = pyqtSignal() - - def __init__(self, parent=None): - super().__init__(parent) - - from .qeapp import ElectrumQmlApplication - self.daemon = ElectrumQmlApplication._daemon - self._config = self.daemon.config # type: SimpleConfig - - self.reset() - - def reset(self): - self._path = None - self._needsPassword = False - self._needsHWDevice = False - self._password = '' - self._validPassword = True - - self._storage = None - - self._ready = False - - @pyqtProperty('QString', notify=pathChanged) - def path(self): - return self._path - - @path.setter - def path(self, wallet_path): - self._logger.debug('setting path: ' + wallet_path) - self.reset() - self._path = wallet_path - - self.pathChanged.emit(self._ready) - - @pyqtProperty(bool, notify=needsPasswordChanged) - def needsPassword(self): - return self._needsPassword - - @needsPassword.setter - def needsPassword(self, wallet_needs_password): - if wallet_needs_password == self._needsPassword: - return - - self._needsPassword = wallet_needs_password - self.needsPasswordChanged.emit() - - @pyqtProperty(bool, notify=needsHWDeviceChanged) - def needsHWDevice(self): - return self._needsHWDevice - - @needsHWDevice.setter - def needsHWDevice(self, wallet_needs_hw_device): - if wallet_needs_hw_device == self._needsHWDevice: - return - - self._needsHWDevice = wallet_needs_hw_device - self.needsHWDeviceChanged.emit() - - @pyqtProperty('QString', notify=passwordChanged) - def password(self): - return '' # no read access - - @password.setter - def password(self, wallet_password): - if wallet_password == self._password: - return - - self._password = wallet_password - self.passwordChanged.emit() - - @pyqtProperty(bool, notify=validPasswordChanged) - def validPassword(self): - return self._validPassword - - @validPassword.setter - def validPassword(self, validPassword): - if self._validPassword != validPassword: - self._validPassword = validPassword - self.validPasswordChanged.emit() - - @pyqtProperty(bool, notify=readyChanged) - def ready(self): - return self._ready - - @pyqtSlot() - def verify(self): - try: - self._load_storage() - if self._storage: - self._load_db() - except WalletFileException as e: - self._logger.error(f"verify errored: {repr(e)}") - self._storage = None - self.walletOpenProblem.emit(str(e)) - if e.should_report_crash: - send_exception_to_crash_reporter(e) - - def _load_storage(self): - """can raise WalletFileException""" - self._storage = WalletStorage(self._path) - if not self._storage.file_exists(): - self._logger.warning('file does not exist') - self.fileNotFound.emit() - self._storage = None - return - - if self._storage.is_encrypted(): - self.needsPassword = True - - try: - self._storage.decrypt('' if not self._password else self._password) - self.validPassword = True - except InvalidPassword as e: - self.validPassword = False - self.invalidPassword.emit() - else: # storage not encrypted; but it might still have a keystore pw - # FIXME hack... load both db and full wallet, just to tell if it has keystore pw. - try: - db = WalletDB(self._storage.read(), storage=None, upgrade=True) - except WalletRequiresSplit as e: - raise WalletFileException(_('This wallet requires to be split. This is currently not supported on mobile')) - wallet = Wallet(db, config=self._config) - self.needsPassword = wallet.has_password() - if self.needsPassword: - try: - wallet.check_password('' if not self._password else self._password) - self.validPassword = True - except InvalidPassword as e: - self.validPassword = False - self._storage = None - self.invalidPassword.emit() - - if self._storage: - if not self._storage.is_past_initial_decryption(): - self._storage = None - - def _load_db(self): - """can raise WalletFileException""" - # needs storage accessible - try: - db = WalletDB(self._storage.read(), storage=None, upgrade=True) - except WalletRequiresSplit as e: - self._logger.warning('wallet requires split') - raise WalletFileException(_('This wallet needs splitting. This is not supported on mobile')) - if db.get_action(): - self._logger.warning('action pending. QML version doesn\'t support continuation of wizard') - raise WalletFileException(_('This wallet has an action pending. This is currently not supported on mobile')) - - self._ready = True - self.readyChanged.emit() diff --git a/electrum/gui/qt/__init__.py b/electrum/gui/qt/__init__.py index f3a7e6805..8adb32ffb 100644 --- a/electrum/gui/qt/__init__.py +++ b/electrum/gui/qt/__init__.py @@ -54,9 +54,9 @@ except ImportError as e: from electrum.i18n import _, set_language from electrum.plugin import run_hook from electrum.util import (UserCancelled, profiler, send_exception_to_crash_reporter, - WalletFileException, BitcoinException, get_new_wallet_name) + WalletFileException, BitcoinException, get_new_wallet_name, InvalidPassword) from electrum.wallet import Wallet, Abstract_Wallet -from electrum.wallet_db import WalletDB, WalletRequiresSplit, WalletRequiresUpgrade +from electrum.wallet_db import WalletDB, WalletRequiresSplit, WalletRequiresUpgrade, WalletUnfinished from electrum.logging import Logger from electrum.gui import BaseElectrumGui from electrum.simple_config import SimpleConfig @@ -343,6 +343,12 @@ class ElectrumGui(BaseElectrumGui, Logger): if not force_wizard: try: wallet = self.daemon.load_wallet(path, None) + except InvalidPassword: + pass # open with wizard below + except WalletRequiresSplit: + pass # open with wizard below + except WalletRequiresUpgrade: + pass # open with wizard below except Exception as e: self.logger.exception('') err_text = str(e) if isinstance(e, WalletFileException) else repr(e) @@ -426,20 +432,16 @@ class ElectrumGui(BaseElectrumGui, Logger): else: wallet_file = d['wallet_name'] - storage = WalletStorage(wallet_file) - if storage.is_encrypted_with_user_pw() or storage.is_encrypted_with_hw_device(): - storage.decrypt(d['password']) - try: - db = WalletDB(storage.read(), storage=storage, upgrade=True) + wallet = self.daemon.load_wallet(wallet_file, d['password'], upgrade=True) + return wallet except WalletRequiresSplit as e: - try: - wizard.run_split(storage, e._split_data) - except UserCancelled: - return - - if action := db.get_action(): + wizard.run_split(wallet_file, e._split_data) + return + except WalletUnfinished as e: # wallet creation is not complete, 2fa online phase + db = e._wallet_db + action = db.get_action() assert action[1] == 'accept_terms_of_use', 'only support for resuming trustedcoin split setup' k1 = load_keystore(db, 'x1') if 'password' in d and d['password']: diff --git a/electrum/gui/qt/wizard/wallet.py b/electrum/gui/qt/wizard/wallet.py index d2ecf8800..bc7331377 100644 --- a/electrum/gui/qt/wizard/wallet.py +++ b/electrum/gui/qt/wizard/wallet.py @@ -162,20 +162,17 @@ class QENewWalletWizard(NewWalletWizard, QEAbstractWizard, MessageBoxMixin): self._password = data['password'] self.path = path - def run_split(self, storage, split_data) -> None: - root_path = storage.path + def run_split(self, wallet_path, split_data) -> None: msg = _( "The wallet '{}' contains multiple accounts, which are no longer supported since Electrum 2.7.\n\n" - "Do you want to split your wallet into multiple files?").format(root_path) + "Do you want to split your wallet into multiple files?").format(wallet_path) if self.question(msg): - file_list = WalletDB.split_accounts(root_path, split_data) + file_list = WalletDB.split_accounts(wallet_path, split_data) msg = _('Your accounts have been moved to') + ':\n' + '\n'.join(file_list) + '\n\n' + _( - 'Do you want to delete the old file') + ':\n' + root_path + 'Do you want to delete the old file') + ':\n' + wallet_path if self.question(msg): - os.remove(root_path) + os.remove(wallet_path) self.show_warning(_('The file was removed')) - # raise now, to avoid having the old storage opened - raise UserCancelled() def is_finalized(self, wizard_data: dict) -> bool: # check decryption of existing wallet and keep wizard open if incorrect. diff --git a/electrum/scripts/ln_features.py b/electrum/scripts/ln_features.py index 7b396534d..ba3c8c2c8 100644 --- a/electrum/scripts/ln_features.py +++ b/electrum/scripts/ln_features.py @@ -51,7 +51,7 @@ if not os.path.exists(wallet_path): create_new_wallet(path=wallet_path, config=config) # open wallet -wallet = daemon.load_wallet(wallet_path, password=None, manual_upgrades=False) +wallet = daemon.load_wallet(wallet_path, password=None, upgrade=True) wallet.start_network(network) diff --git a/electrum/scripts/quick_start.py b/electrum/scripts/quick_start.py index 153a74ddd..63b209d88 100755 --- a/electrum/scripts/quick_start.py +++ b/electrum/scripts/quick_start.py @@ -28,7 +28,7 @@ if not os.path.exists(wallet_path): create_new_wallet(path=wallet_path, config=config) # open wallet -wallet = daemon.load_wallet(wallet_path, password=None, manual_upgrades=False) +wallet = daemon.load_wallet(wallet_path, password=None, upgrade=True) wallet.start_network(network) # you can use ~CLI commands by accessing command_runner diff --git a/electrum/wallet_db.py b/electrum/wallet_db.py index f71935732..80f27f34d 100644 --- a/electrum/wallet_db.py +++ b/electrum/wallet_db.py @@ -52,12 +52,19 @@ from .version import ELECTRUM_VERSION class WalletRequiresUpgrade(WalletFileException): pass + + class WalletRequiresSplit(WalletFileException): def __init__(self, split_data): self._split_data = split_data -# seed_version is now used for the version of the wallet file +class WalletUnfinished(WalletFileException): + def __init__(self, wallet_db: 'WalletDB'): + self._wallet_db = wallet_db + + +# seed_version is now used for the version of the wallet file OLD_SEED_VERSION = 4 # electrum versions < 2.0 NEW_SEED_VERSION = 11 # electrum versions >= 2.0 FINAL_SEED_VERSION = 55 # electrum >= 2.7 will set this to prevent @@ -91,6 +98,7 @@ class DBMetadata(StoredObject): # separate tracking issues class WalletFileExceptionVersion51(WalletFileException): pass + # register dicts that require value conversions not handled by constructor json_db.register_dict('transactions', lambda x: tx_from_any(x, deserialize=False), None) json_db.register_dict('prevouts_by_scripthash', lambda x: set(tuple(k) for k in x), None) @@ -106,9 +114,7 @@ for key in ['locked_in', 'fails', 'settles']: json_db.register_parent_key(key, lambda x: HTLCOwner(int(x))) - class WalletDBUpgrader(Logger): - def __init__(self, data): Logger.__init__(self) self.data = data @@ -164,7 +170,7 @@ class WalletDBUpgrader(Logger): new_data['suffix'] = k result.append(new_data) else: - raise WalletFileException("This wallet has multiple accounts and must be split") + raise WalletFileException(f'Unsupported wallet type for split: {wallet_type}') return result def requires_upgrade(self):