Merge pull request #10141 from SomberNight/202508_kswizard_multisig

wizard: enable_keystore: fix for multisig

electrum/gui/qt/wizard/wallet.py

@@ -57,7 +57,7 @@ class QEKeystoreWizard(KeystoreWizard, QEAbstractWizard, MessageBoxMixin):
             config: 'SimpleConfig',
             app: 'QElectrumApplication',
             plugins: 'Plugins',
-            start_viewstate: WizardViewState = None
+            start_viewstate: WizardViewState = None,
     ):
         assert 'wallet_type' in start_viewstate.wizard_data, 'wallet_type required'
 
@@ -440,10 +440,6 @@ class WCExtendKeystore(WalletWizardComponent):
 
     def apply(self):
         self.wizard_data['keystore_type'] = self.choice_w.selected_key
-        if multisig_type(self.wizard_data['wallet_type']):
-            self.wizard_data['multisig_participants'] = 2
-            self.wizard_data['multisig_signatures'] = 2
-            self.wizard_data['multisig_cosigner_data'] = {}
 
 
 class WCCreateSeed(WalletWizardComponent):

electrum/gui/qt/wizard/wizard.py

@@ -124,10 +124,7 @@ class QEAbstractWizard(QDialog, MessageBoxMixin):
 
     @pyqtSlot()
     def strt(self):
-        if self.start_viewstate is not None:
+        viewstate = self.start_wizard(start_viewstate=self.start_viewstate)
-            viewstate = self._current = self.start_viewstate
-        else:
-            viewstate = self.start_wizard()
         self.load_next_component(viewstate.view, viewstate.wizard_data, viewstate.params)
         self.set_default_focus()
 
@@ -236,8 +233,8 @@ class QEAbstractWizard(QDialog, MessageBoxMixin):
                 self.prev()  # rollback the submit above
                 raise e
 
-    def start_wizard(self) -> 'WizardViewState':
+    def start_wizard(self, *, start_viewstate: Optional['WizardViewState'] = None) -> 'WizardViewState':
-        self.start()
+        self.start(start_viewstate=start_viewstate)
         return self._current
 
     def view_to_component(self, view) -> QWidget:

electrum/wizard.py

@@ -17,6 +17,7 @@ from electrum.wallet_db import WalletDB
 from electrum.bip32 import normalize_bip32_derivation, xpub_type
 from electrum import keystore, mnemonic, bitcoin
 from electrum.mnemonic import is_any_2fa_seed_type, can_seed_have_passphrase
+from electrum.util import multisig_type
 
 if TYPE_CHECKING:
     from electrum.daemon import Daemon
@@ -257,13 +258,23 @@ class KeystoreWizard(AbstractWizard):
         # one at a time
         return True
 
-    def start(self, initial_data: dict = None) -> WizardViewState:
+    def _convert_wallet_type(self, wizard_data: dict) -> None:
-        if initial_data is None:
+        assert 'wallet_type' in wizard_data
-            initial_data = {}
+        if multisig_type(wizard_data['wallet_type']):
+            wizard_data['wallet_type'] = 'multisig'  # convert from e.g. "2of2" to "multisig"
+            wizard_data['multisig_participants'] = 2
+            wizard_data['multisig_signatures'] = 2
+            wizard_data['multisig_cosigner_data'] = {}
+
+    def start(self, *, start_viewstate: WizardViewState = None) -> WizardViewState:
         self.reset()
-        start_view = 'keystore_type'
+        if start_viewstate is None:
-        params = self.navmap[start_view].get('params', {})
+            start_view = 'keystore_type'
-        self._current = WizardViewState(start_view, initial_data, params)
+            params = self.navmap[start_view].get('params', {})
+            self._current = WizardViewState(start_view, {}, params)
+        else:
+            self._current = start_viewstate
+        self._convert_wallet_type(self._current.wizard_data)  # mutating in-place
         return self._current
 
     # returns (sub)dict of current cosigner (or root if first)
@@ -487,13 +498,14 @@ class NewWalletWizard(KeystoreWizard):
         # todo: load only if needed, like hw plugins
         self.plugins.load_plugin_by_name('trustedcoin')
 
-    def start(self, initial_data: dict = None) -> WizardViewState:
+    def start(self, *, start_viewstate: WizardViewState = None) -> WizardViewState:
-        if initial_data is None:
-            initial_data = {}
         self.reset()
-        start_view = 'wallet_name'
+        if start_viewstate is None:
-        params = self.navmap[start_view].get('params', {})
+            start_view = 'wallet_name'
-        self._current = WizardViewState(start_view, initial_data, params)
+            params = self.navmap[start_view].get('params', {})
+            self._current = WizardViewState(start_view, {}, params)
+        else:
+            self._current = start_viewstate
         return self._current
 
     def is_single_password(self) -> bool:
@@ -861,13 +873,14 @@ class ServerConnectWizard(AbstractWizard):
             if wizard_data.get('autoconnect') is not None:
                 self._daemon.config.NETWORK_AUTO_CONNECT = wizard_data.get('autoconnect')
 
-    def start(self, initial_data: dict = None) -> WizardViewState:
+    def start(self, *, start_viewstate: WizardViewState = None) -> WizardViewState:
-        if initial_data is None:
-            initial_data = {}
         self.reset()
-        start_view = 'welcome'
+        if start_viewstate is None:
-        params = self.navmap[start_view].get('params', {})
+            start_view = 'welcome'
-        self._current = WizardViewState(start_view, initial_data, params)
+            params = self.navmap[start_view].get('params', {})
+            self._current = WizardViewState(start_view, {}, params)
+        else:
+            self._current = start_viewstate
         return self._current
 
 
@@ -888,11 +901,12 @@ class TermsOfUseWizard(AbstractWizard):
     def accept_terms_of_use(self, _):
         self._config.TERMS_OF_USE_ACCEPTED = TERMS_OF_USE_LATEST_VERSION
 
-    def start(self, initial_data: dict = None) -> WizardViewState:
+    def start(self, *, start_viewstate: WizardViewState = None) -> WizardViewState:
-        if initial_data is None:
-            initial_data = {}
         self.reset()
-        start_view = 'terms_of_use'
+        if start_viewstate is None:
-        params = self.navmap[start_view].get('params', {})
+            start_view = 'terms_of_use'
-        self._current = WizardViewState(start_view, initial_data, params)
+            params = self.navmap[start_view].get('params', {})
+            self._current = WizardViewState(start_view, {}, params)
+        else:
+            self._current = start_viewstate
         return self._current

tests/test_wallet_vertical.py

@@ -6,7 +6,9 @@ from typing import Sequence
 import asyncio
 import copy
 
-from electrum import storage, bitcoin, keystore, bip32, slip39, wallet
+from electrum import bitcoin, keystore, bip32, slip39, wallet
+from electrum.wallet_db import WalletDB
+from electrum.storage import WalletStorage
 from electrum import SimpleConfig
 from electrum import util
 from electrum.address_synchronizer import TX_HEIGHT_UNCONFIRMED, TX_HEIGHT_UNCONF_PARENT, TX_HEIGHT_LOCAL, TX_HEIGHT_FUTURE
@@ -35,7 +37,6 @@ class WalletIntegrityHelper:
 
     gap_limit = 1  # make tests run faster
     gap_limit_for_change = 1  # make tests run faster
-    # TODO also use short gap limit for change addrs, for performance
 
     @classmethod
     def check_seeded_keystore_sanity(cls, test_obj, ks):
@@ -53,7 +54,7 @@ class WalletIntegrityHelper:
 
     @classmethod
     def create_standard_wallet(cls, ks, *, config: SimpleConfig, gap_limit=None, gap_limit_for_change=None):
-        db = storage.WalletDB('', storage=None, upgrade=True)
+        db = WalletDB('', storage=None, upgrade=True)
         db.put('keystore', ks.dump())
         db.put('gap_limit', gap_limit or cls.gap_limit)
         db.put('gap_limit_for_change', gap_limit_for_change or cls.gap_limit_for_change)
@@ -63,7 +64,7 @@ class WalletIntegrityHelper:
 
     @classmethod
     def create_imported_wallet(cls, *, config: SimpleConfig, privkeys: bool):
-        db = storage.WalletDB('', storage=None, upgrade=True)
+        db = WalletDB('', storage=None, upgrade=True)
         if privkeys:
             k = keystore.Imported_KeyStore({})
             db.put('keystore', k.dump())
@@ -71,10 +72,18 @@ class WalletIntegrityHelper:
         return w
 
     @classmethod
-    def create_multisig_wallet(cls, keystores: Sequence, multisig_type: str, *,
+    def create_multisig_wallet(
-                               config: SimpleConfig, gap_limit=None, gap_limit_for_change=None):
+        cls,
+        keystores: Sequence,
+        multisig_type: str,
+        *,
+        config: SimpleConfig,
+        storage: WalletStorage | None = None,
+        gap_limit=None,
+        gap_limit_for_change=None,
+    ):
         """Creates a multisig wallet."""
-        db = storage.WalletDB('', storage=None, upgrade=False)
+        db = WalletDB('', storage=storage, upgrade=False)
         for i, ks in enumerate(keystores):
             cosigner_index = i + 1
             db.put('x%d' % cosigner_index, ks.dump())

tests/test_wizard.py

@@ -11,9 +11,11 @@ from electrum.wallet import Abstract_Wallet
 from electrum import util
 from electrum import slip39
 from electrum.bip32 import KeyOriginInfo
+from electrum import keystore
+from electrum.storage import WalletStorage
 
 from . import ElectrumTestCase
-from .test_wallet_vertical import UNICODE_HORROR
+from .test_wallet_vertical import UNICODE_HORROR, WalletIntegrityHelper
 
 
 class NetworkMock:
@@ -125,8 +127,6 @@ class ServerConnectWizardTestCase(WizardTestCase):
 
 
 class KeystoreWizardTestCase(WizardTestCase):
-    # TODO add test cases for:
-    #  - multisig
 
     class TKeystoreWizard(KeystoreWizard):
         def is_single_password(self):
@@ -140,7 +140,8 @@ class KeystoreWizardTestCase(WizardTestCase):
 
     def _wizard_for(self, *, wallet_type: str = 'standard', hww: bool = False) -> tuple[KeystoreWizard, WizardViewState]:
         w = KeystoreWizardTestCase.TKeystoreWizard(self.plugins)
-        v = w.start({'wallet_type': wallet_type})
+        start_viewstate = WizardViewState('keystore_type', {'wallet_type': wallet_type}, {})
+        v = w.start(start_viewstate=start_viewstate)
         self.assertEqual('keystore_type', v.view)
         d = v.wizard_data
         if hww:
@@ -389,6 +390,46 @@ class KeystoreWizardTestCase(WizardTestCase):
         wallet.disable_keystore(wallet.get_keystore())
         self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub, key_origin_info=my_keyorigininfo)
 
+    async def test_multisig(self):
+        seed1 = "bitter grass shiver impose acquire brush forget axis eager alone wine silver"
+        xpub1 = "Zpub6ymNkfdyhypEoqQNNGAUz9gXeiWJsW8AWx8Aa6PnDdeL76UC9b1UPGmEvwWzzkVVghVQuDBry7CK7wCBBdysRQgFFmdDSqi5kWoZ3A4cBuA"
+        seed2 = "snow nest raise royal more walk demise rotate smooth spirit canyon gun"
+        xpub2 = "Zpub6xwgqLvc42wXB1wEELTdALD9iXwStMUkGqBgxkJFYumaL2dWgNvUkjEDWyDFZD3fZuDWDzd1KQJ4NwVHS7hs6H6QkpNYSShfNiUZsgMdtNg"
+
+        wallet = WalletIntegrityHelper.create_multisig_wallet(
+            [
+                keystore.from_seed(seed1, passphrase='', for_multisig=True),
+                keystore.from_xpub(xpub2),
+            ],
+            '2of2',
+            config=self.config,
+            storage=WalletStorage(self.wallet_path),
+        )
+
+        w, v = self._wizard_for(wallet_type=wallet.wallet_type)
+        d = v.wizard_data
+        d.update({
+            'seed': seed2, 'seed_type': 'segwit', 'seed_extend': False, 'seed_variant': 'electrum',
+        })
+        self.assertTrue(w.is_last_view(v.view, d))
+        w.resolve_next(v.view, d)
+        ks, ishww = w._result
+        self.assertFalse(ishww)
+        self.assertEqual(ks.xpub, xpub2)
+
+        self.assertFalse(wallet.get_keystores()[0].is_watching_only())
+        self.assertTrue(wallet.get_keystores()[1].is_watching_only())
+        self.assertTrue(wallet.can_enable_disable_keystore(ks))
+        wallet.enable_keystore(ks, ishww, None)
+        self.assertFalse(wallet.get_keystores()[0].is_watching_only())
+        self.assertFalse(wallet.get_keystores()[1].is_watching_only())
+        self.assertEqual(seed1, wallet.get_keystores()[0].get_seed(None))
+        self.assertEqual(seed2, wallet.get_keystores()[1].get_seed(None))
+
+        keyorigininfo1 = wallet.get_keystores()[0].get_key_origin_info()
+        wallet.disable_keystore(wallet.get_keystores()[0])
+        self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystores()[0], xpub=xpub1, key_origin_info=keyorigininfo1)
+
 
 class WalletWizardTestCase(WizardTestCase):