Merge pull request #8417 from SomberNight/202305_db_version_52

wallet_db version 52: break non-homogeneous multisig wallets

electrum/gui/qml/components/main.qml

@@ -495,7 +495,7 @@ ApplicationWindow
         }
         function onWalletOpenError(error) {
             console.log('wallet open error')
-            var dialog = app.messageDialog.createObject(app, {'text': error})
+            var dialog = app.messageDialog.createObject(app, { title: qsTr('Error'), 'text': error })
             dialog.open()
         }
         function onAuthRequired(method, authMessage) {

electrum/gui/qml/qedaemon.py

@@ -223,7 +223,7 @@ class QEDaemon(AuthMixin, QObject):
 
                 self._backendWalletLoaded.emit(local_password)
             except WalletFileException as e:
-                self._logger.error(str(e))
+                self._logger.error(f"load_wallet_task errored opening wallet: {e!r}")
                 self.walletOpenError.emit(str(e))
             finally:
                 self._loading = False

electrum/gui/qml/qewalletdb.py

@@ -8,7 +8,7 @@ from electrum.storage import WalletStorage, StorageEncryptionVersion
 from electrum.wallet_db import WalletDB
 from electrum.wallet import Wallet
 from electrum.bip32 import normalize_bip32_derivation, xpub_type
-from electrum.util import InvalidPassword, WalletFileException
+from electrum.util import InvalidPassword, WalletFileException, send_exception_to_crash_reporter
 from electrum import keystore
 
 if TYPE_CHECKING:
@@ -120,9 +120,16 @@ class QEWalletDB(QObject):
 
     @pyqtSlot()
     def verify(self):
-        self.load_storage()
-        if self._storage:
-            self.load_db()
+        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)
 
     @pyqtSlot()
     def doSplit(self):
@@ -134,7 +141,8 @@ class QEWalletDB(QObject):
 
         self.splitFinished.emit()
 
-    def load_storage(self):
+    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')
@@ -170,27 +178,23 @@ class QEWalletDB(QObject):
             if not self._storage.is_past_initial_decryption():
                 self._storage = None
 
-    def load_db(self):
+    def _load_db(self):
+        """can raise WalletFileException"""
         # needs storage accessible
-        try:
-            self._db = WalletDB(self._storage.read(), manual_upgrades=True)
-            if self._db.requires_split():
-                self._logger.warning('wallet requires split')
-                self._requiresSplit = True
-                self.requiresSplitChanged.emit()
-                return
-            if self._db.get_action():
-                self._logger.warning('action pending. QML version doesn\'t support continuation of wizard')
-                return
-
-            if self._db.requires_upgrade():
-                self._logger.warning('wallet requires upgrade, upgrading')
-                self._db.upgrade()
-                self._db.write(self._storage)
-
-            self._ready = True
-            self.readyChanged.emit()
-        except WalletFileException as e:
-            self._logger.error(f'{repr(e)}')
-            self._storage = None
-            self.walletOpenProblem.emit(str(e))
+        self._db = WalletDB(self._storage.read(), manual_upgrades=True)
+        if self._db.requires_split():
+            self._logger.warning('wallet requires split')
+            self._requiresSplit = True
+            self.requiresSplitChanged.emit()
+            return
+        if self._db.get_action():
+            self._logger.warning('action pending. QML version doesn\'t support continuation of wizard')
+            return
+
+        if self._db.requires_upgrade():
+            self._logger.warning('wallet requires upgrade, upgrading')
+            self._db.upgrade()
+            self._db.write(self._storage)
+
+        self._ready = True
+        self.readyChanged.emit()

electrum/gui/qt/__init__.py

@@ -342,10 +342,13 @@ class ElectrumGui(BaseElectrumGui, Logger):
                 wallet = self.daemon.load_wallet(path, None)
             except Exception as e:
                 self.logger.exception('')
+                err_text = str(e) if isinstance(e, WalletFileException) else repr(e)
                 custom_message_box(icon=QMessageBox.Warning,
                                    parent=None,
                                    title=_('Error'),
-                                   text=_('Cannot load wallet') + ' (1):\n' + repr(e))
+                                   text=_('Cannot load wallet') + ' (1):\n' + err_text)
+                if isinstance(e, WalletFileException) and e.should_report_crash:
+                    send_exception_to_crash_reporter(e)
                 # if app is starting, still let wizard appear
                 if not app_is_starting:
                     return
@@ -364,10 +367,13 @@ class ElectrumGui(BaseElectrumGui, Logger):
                 window = self._create_window_for_wallet(wallet)
         except Exception as e:
             self.logger.exception('')
+            err_text = str(e) if isinstance(e, WalletFileException) else repr(e)
             custom_message_box(icon=QMessageBox.Warning,
                                parent=None,
                                title=_('Error'),
-                               text=_('Cannot load wallet') + '(2) :\n' + repr(e))
+                               text=_('Cannot load wallet') + '(2) :\n' + err_text)
+            if isinstance(e, WalletFileException) and e.should_report_crash:
+                send_exception_to_crash_reporter(e)
             if app_is_starting:
                 # If we raise in this context, there are no more fallbacks, we will shut down.
                 # Worst case scenario, we might have gotten here without user interaction,

electrum/util.py

@@ -187,7 +187,10 @@ class FileExportFailed(Exception):
         return _("Failed to export to file.") + "\n" + self.message
 
 
-class WalletFileException(Exception): pass
+class WalletFileException(Exception):
+    def __init__(self, message='', *, should_report_crash: bool = False):
+        Exception.__init__(self, message)
+        self.should_report_crash = should_report_crash
 
 
 class BitcoinException(Exception): pass

electrum/wallet.py

@@ -3532,6 +3532,12 @@ class Multisig_Wallet(Deterministic_Wallet):
         self.wallet_type = db.get('wallet_type')
         self.m, self.n = multisig_type(self.wallet_type)
         Deterministic_Wallet.__init__(self, db, storage, config=config)
+        # sanity checks
+        for ks in self.get_keystores():
+            if not isinstance(ks, keystore.Xpub):
+                raise Exception(f"unexpected keystore type={type(ks)} in multisig")
+            if bip32.xpub_type(self.keystore.xpub) != bip32.xpub_type(ks.xpub):
+                raise Exception(f"multisig wallet needs to have homogeneous xpub types")
 
     def get_public_keys(self, address):
         return [pk.hex() for pk in self.get_public_keys_with_deriv_info(address)]

electrum/wallet_db.py

@@ -57,7 +57,7 @@ if TYPE_CHECKING:
 
 OLD_SEED_VERSION = 4        # electrum versions < 2.0
 NEW_SEED_VERSION = 11       # electrum versions >= 2.0
-FINAL_SEED_VERSION = 51     # electrum >= 2.7 will set this to prevent
+FINAL_SEED_VERSION = 52     # electrum >= 2.7 will set this to prevent
                             # old versions from overwriting new format
 
 
@@ -81,6 +81,12 @@ class DBMetadata(StoredObject):
         return f"using {ver}, on {date_str}"
 
 
+# note: subclassing WalletFileException for some specific cases
+#       allows the crash reporter to distinguish them and open
+#       separate tracking issues
+class WalletFileExceptionVersion51(WalletFileException): pass
+
+
 class WalletDB(JsonDB):
 
     def __init__(self, raw, *, manual_upgrades: bool):
@@ -220,6 +226,7 @@ class WalletDB(JsonDB):
         self._convert_version_49()
         self._convert_version_50()
         self._convert_version_51()
+        self._convert_version_52()
         self.put('seed_version', FINAL_SEED_VERSION)  # just to be sure
 
         self._after_upgrade_tasks()
@@ -1016,6 +1023,38 @@ class WalletDB(JsonDB):
             item['payment_hash'] = payment_hash
         self.data['seed_version'] = 51
 
+    def _detect_insane_version_51(self) -> int:
+        """Returns 0 if file okay,
+        error code 1: multisig wallet has old_mpk
+        error code 2: multisig wallet has mixed Ypub/Zpub
+        """
+        assert self.get('seed_version') == 51
+        xpub_type = None
+        for ks_name in ['x{}/'.format(i) for i in range(1, 16)]:  # having any such field <=> multisig wallet
+            ks = self.data.get(ks_name, None)
+            if ks is None: continue
+            ks_type = ks.get('type')
+            if ks_type == "old":
+                return 1  # error
+            assert ks_type in ("bip32", "hardware"), f"unexpected {ks_type=}"
+            xpub = ks.get('xpub') or None
+            assert xpub is not None
+            assert isinstance(xpub, str)
+            if xpub_type is None:  # first iter
+                xpub_type = xpub[0:4]
+            if xpub[0:4] != xpub_type:
+                return 2  # error
+        # looks okay
+        return 0
+
+    def _convert_version_52(self):
+        if not self._is_upgrade_method_needed(51, 51):
+            return
+        if (error_code := self._detect_insane_version_51()) != 0:
+            # should not get here; get_seed_version should have caught this
+            raise Exception(f'unsupported wallet file: version_51 with error {error_code}')
+        self.data['seed_version'] = 52
+
     def _convert_imported(self):
         if not self._is_upgrade_method_needed(0, 13):
             return
@@ -1071,9 +1110,11 @@ class WalletDB(JsonDB):
             raise WalletFileException('This version of Electrum is too old to open this wallet.\n'
                                       '(highest supported storage version: {}, version of this file: {})'
                                       .format(FINAL_SEED_VERSION, seed_version))
-        if seed_version==14 and self.get('seed_type') == 'segwit':
+        if seed_version == 14 and self.get('seed_type') == 'segwit':
             self._raise_unsupported_version(seed_version)
-        if seed_version >=12:
+        if seed_version == 51 and self._detect_insane_version_51():
+            self._raise_unsupported_version(seed_version)
+        if seed_version >= 12:
             return seed_version
         if seed_version not in [OLD_SEED_VERSION, NEW_SEED_VERSION]:
             self._raise_unsupported_version(seed_version)
@@ -1092,6 +1133,23 @@ class WalletDB(JsonDB):
             else:
                 # creation was complete if electrum was run from source
                 msg += "\nPlease open this file with Electrum 1.9.8, and move your coins to a new wallet."
+        if seed_version == 51:
+            error_code = self._detect_insane_version_51()
+            assert error_code != 0
+            msg += f" ({error_code=})"
+            if error_code == 1:
+                msg += "\nThis is a multisig wallet containing an old_mpk (pre-bip32 master public key)."
+                msg += "\nPlease contact us to help recover it by opening an issue on GitHub."
+            elif error_code == 2:
+                msg += ("\nThis is a multisig wallet containing mixed xpub/Ypub/Zpub."
+                        "\nThe script type is determined by the type of the first keystore."
+                        "\nTo recover, you should re-create the wallet with matching type "
+                        "(converted if needed) master keys."
+                        "\nOr you can contact us to help recover it by opening an issue on GitHub.")
+            else:
+                raise Exception(f"unexpected {error_code=}")
+            raise WalletFileExceptionVersion51(msg, should_report_crash=True)
+        # generic exception
         raise WalletFileException(msg)
 
     def _add_db_creation_metadata(self):