password unification refactor: move methods from wallet to daemon

Note in particular that check_password_for_directory was not safe to use while the daemon had wallets loaded,
as the same file would have two corresponding Wallet() instances in memory. This was specifically handled in
the kivy GUI, on the caller side, by stopping-before and reloading-after the wallets; but it was dirty to
have the caller handle this.

electrum/daemon.py

@@ -478,6 +478,7 @@ class Daemon(Logger):
         self.gui_object = None
         # path -> wallet;   make sure path is standardized.
         self._wallets = {}  # type: Dict[str, Abstract_Wallet]
+        self._wallet_lock = threading.RLock()
         daemon_jobs = []
         # Setup commands server
         self.commands_server = None
@@ -526,12 +527,35 @@ class Daemon(Logger):
             #       not see the exception (especially if the GUI did not start yet).
             self._stopping_soon_or_errored.set()
 
+    def with_wallet_lock(func):
+        def func_wrapper(self: 'Daemon', *args, **kwargs):
+            with self._wallet_lock:
+                return func(self, *args, **kwargs)
+        return func_wrapper
+
+    @with_wallet_lock
     def load_wallet(self, path, password, *, manual_upgrades=True) -> Optional[Abstract_Wallet]:
         path = standardize_path(path)
         # wizard will be launched if we return
         if path in self._wallets:
             wallet = self._wallets[path]
             return wallet
+        wallet = self._load_wallet(path, password, manual_upgrades=manual_upgrades, config=self.config)
+        if wallet is None:
+            return
+        wallet.start_network(self.network)
+        self._wallets[path] = wallet
+        return wallet
+
+    @staticmethod
+    def _load_wallet(
+            path,
+            password,
+            *,
+            manual_upgrades: bool = True,
+            config: SimpleConfig,
+    ) -> Optional[Abstract_Wallet]:
+        path = standardize_path(path)
         storage = WalletStorage(path)
         if not storage.file_exists():
             return
@@ -547,11 +571,10 @@ class Daemon(Logger):
             return
         if db.get_action():
             return
-        wallet = Wallet(db, storage, config=self.config)
+        wallet = Wallet(db, storage, config=config)
-        wallet.start_network(self.network)
-        self._wallets[path] = wallet
         return wallet
 
+    @with_wallet_lock
     def add_wallet(self, wallet: Abstract_Wallet) -> None:
         path = wallet.storage.path
         path = standardize_path(path)
@@ -561,6 +584,7 @@ class Daemon(Logger):
         path = standardize_path(path)
         return self._wallets.get(path)
 
+    @with_wallet_lock
     def get_wallets(self) -> Dict[str, Abstract_Wallet]:
         return dict(self._wallets)  # copy
 
@@ -577,6 +601,7 @@ class Daemon(Logger):
         fut = asyncio.run_coroutine_threadsafe(self._stop_wallet(path), self.asyncio_loop)
         return fut.result()
 
+    @with_wallet_lock
     async def _stop_wallet(self, path: str) -> bool:
         """Returns True iff a wallet was found."""
         path = standardize_path(path)
@@ -642,3 +667,66 @@ class Daemon(Logger):
         finally:
             # app will exit now
             asyncio.run_coroutine_threadsafe(self.stop(), self.asyncio_loop).result()
+
+    @with_wallet_lock
+    def _check_password_for_directory(self, *, old_password, new_password=None, wallet_dir: str) -> Tuple[bool, bool]:
+        """Checks password against all wallets (in dir), returns whether they can be unified and whether they are already.
+        If new_password is not None, update all wallet passwords to new_password.
+        """
+        assert os.path.exists(wallet_dir), f"path {wallet_dir!r} does not exist"
+        failed = []
+        is_unified = True
+        for filename in os.listdir(wallet_dir):
+            path = os.path.join(wallet_dir, filename)
+            path = standardize_path(path)
+            if not os.path.isfile(path):
+                continue
+            wallet = self.get_wallet(path)
+            if wallet is None:
+                try:
+                    wallet = self._load_wallet(path, old_password, manual_upgrades=False, config=self.config)
+                except util.InvalidPassword:
+                    pass
+                except Exception:
+                    self.logger.exception(f'failed to load wallet at {path!r}:')
+                    pass
+            if wallet is None:
+                failed.append(path)
+                continue
+            if not wallet.storage.is_encrypted():
+                is_unified = False
+            try:
+                wallet.check_password(old_password)
+            except Exception:
+                failed.append(path)
+                continue
+            if new_password:
+                self.logger.info(f'updating password for wallet: {path!r}')
+                wallet.update_password(old_password, new_password, encrypt_storage=True)
+        can_be_unified = failed == []
+        is_unified = can_be_unified and is_unified
+        return can_be_unified, is_unified
+
+    @with_wallet_lock
+    def update_password_for_directory(
+            self,
+            *,
+            old_password,
+            new_password,
+            wallet_dir: Optional[str] = None,
+    ) -> bool:
+        """returns whether password is unified"""
+        if new_password is None:
+            # we opened a non-encrypted wallet
+            return False
+        if wallet_dir is None:
+            wallet_dir = os.path.dirname(self.config.get_wallet_path())
+        can_be_unified, is_unified = self._check_password_for_directory(
+            old_password=old_password, new_password=None, wallet_dir=wallet_dir)
+        if not can_be_unified:
+            return False
+        if is_unified and old_password == new_password:
+            return True
+        self._check_password_for_directory(
+            old_password=old_password, new_password=new_password, wallet_dir=wallet_dir)
+        return True

electrum/gui/kivy/main_window.py

@@ -12,7 +12,6 @@ from typing import TYPE_CHECKING, Optional, Union, Callable, Sequence
 from electrum.storage import WalletStorage, StorageReadWriteError
 from electrum.wallet_db import WalletDB
 from electrum.wallet import Wallet, InternalAddressCorruption, Abstract_Wallet
-from electrum.wallet import update_password_for_directory
 
 from electrum.plugin import run_hook
 from electrum import util
@@ -679,7 +678,8 @@ class ElectrumWindow(App, Logger, EventListener):
     def on_wizard_success(self, storage, db, password):
         self.password = password
         if self.electrum_config.get('single_password'):
-            self._use_single_password = update_password_for_directory(self.electrum_config, password, password)
+            self._use_single_password = self.daemon.update_password_for_directory(
+                old_password=password, new_password=password)
         self.logger.info(f'use single password: {self._use_single_password}')
         wallet = Wallet(db, storage, config=self.electrum_config)
         wallet.start_network(self.daemon.network)
@@ -1346,10 +1346,7 @@ class ElectrumWindow(App, Logger, EventListener):
             # called if old_password works on self.wallet
             self.password = new_password
             if self._use_single_password:
-                path = self.wallet.storage.path
+                self.daemon.update_password_for_directory(old_password=old_password, new_password=new_password)
-                self.stop_wallet()
-                update_password_for_directory(self.electrum_config, old_password, new_password)
-                self.load_wallet_by_name(path)
                 msg = _("Password updated successfully")
             else:
                 self.wallet.update_password(old_password, new_password)

electrum/storage.py

@@ -146,6 +146,8 @@ class WalletStorage(Logger):
 
     @staticmethod
     def get_eckey_from_password(password):
+        if password is None:
+            password = ""
         secret = hashlib.pbkdf2_hmac('sha512', password.encode('utf-8'), b'', iterations=1024)
         ec_key = ecc.ECPrivkey.from_arbitrary_size_secret(secret)
         return ec_key
@@ -160,6 +162,7 @@ class WalletStorage(Logger):
             raise WalletFileException('no encryption magic for version: %s' % v)
 
     def decrypt(self, password) -> None:
+        """Raises an InvalidPassword exception on invalid password"""
         if self.is_past_initial_decryption():
             return
         ec_key = self.get_eckey_from_password(password)

electrum/wallet.py

@@ -3525,80 +3525,3 @@ def restore_wallet_from_text(text, *, path, config: SimpleConfig,
            "Start a daemon and use load_wallet to sync its history.")
     wallet.save_db()
     return {'wallet': wallet, 'msg': msg}
-
-
-def check_password_for_directory(config: SimpleConfig, old_password, new_password=None) -> Tuple[bool, bool]:
-    """Checks password against all wallets, returns whether they can be unified and whether they are already.
-    If new_password is not None, update all wallet passwords to new_password.
-    """
-    dirname = os.path.dirname(config.get_wallet_path())
-    failed = []
-    is_unified = True
-    for filename in os.listdir(dirname):
-        path = os.path.join(dirname, filename)
-        if not os.path.isfile(path):
-            continue
-        basename = os.path.basename(path)
-        storage = WalletStorage(path)
-        if not storage.is_encrypted():
-            is_unified = False
-            # it is a bit wasteful load the wallet here, but that is fine
-            # because we are progressively enforcing storage encryption.
-            try:
-                db = WalletDB(storage.read(), manual_upgrades=False)
-                wallet = Wallet(db, storage, config=config)
-            except:
-                _logger.exception(f'failed to load {basename}:')
-                failed.append(basename)
-                continue
-            if wallet.has_keystore_encryption():
-                try:
-                    wallet.check_password(old_password)
-                except:
-                    failed.append(basename)
-                    continue
-                if new_password:
-                    wallet.update_password(old_password, new_password)
-            else:
-                if new_password:
-                    wallet.update_password(None, new_password)
-            continue
-        if not storage.is_encrypted_with_user_pw():
-            failed.append(basename)
-            continue
-        try:
-            storage.check_password(old_password)
-        except:
-            failed.append(basename)
-            continue
-        try:
-            db = WalletDB(storage.read(), manual_upgrades=False)
-            wallet = Wallet(db, storage, config=config)
-        except:
-            _logger.exception(f'failed to load {basename}:')
-            failed.append(basename)
-            continue
-        try:
-            wallet.check_password(old_password)
-        except:
-            failed.append(basename)
-            continue
-        if new_password:
-            wallet.update_password(old_password, new_password)
-    can_be_unified = failed == []
-    is_unified = can_be_unified and is_unified
-    return can_be_unified, is_unified
-
-
-def update_password_for_directory(config: SimpleConfig, old_password, new_password) -> bool:
-    " returns whether password is unified "
-    if new_password is None:
-        # we opened a non-encrypted wallet
-        return False
-    can_be_unified, is_unified = check_password_for_directory(config, old_password, None)
-    if not can_be_unified:
-        return False
-    if is_unified and old_password == new_password:
-        return True
-    check_password_for_directory(config, old_password, new_password)
-    return True