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

@@ -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)
-            db = WalletDB(storage.read(), storage=storage, upgrade=upgrade)
-        except WalletRequiresSplit:
-            return
-        except WalletRequiresUpgrade:
-            return
         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

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
+    }
 }

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()

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) {
+            showOpenWalletDialog(name, path)
-                _opendialog = openWalletDialog.createObject(app, { path: path, name: name })
-                _opendialog.closed.connect(function() {
-                    _opendialog = undefined
-                })
-                _opendialog.open()
-            }
         }
         function onWalletOpenError(error) {
             console.log('wallet open error')

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')

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()

electrum/gui/qml/qewalletdb.py

@@ -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()

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(wallet_file, e._split_data)
-                wizard.run_split(storage, e._split_data)
+            return
-            except UserCancelled:
+        except WalletUnfinished as e:
-                return
-
-        if action := db.get_action():
             # 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']:

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:
+    def run_split(self, wallet_path, split_data) -> None:
-        root_path = storage.path
         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.

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)
 
 

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

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):