wizard: fix wizard_data instance not isolated between pages,

combine is_bip39_seed and is_slip39_seed into cosigner aware needs_derivation_path
add instance id logging to wizard.log_stack()
qt: add updated signal to SeedLayout

electrum/gui/qt/seed_dialog.py

@@ -25,7 +25,7 @@
 
 from typing import TYPE_CHECKING
 
-from PyQt5.QtCore import Qt
+from PyQt5.QtCore import Qt, pyqtSignal
 from PyQt5.QtGui import QPixmap
 from PyQt5.QtWidgets import (QVBoxLayout, QCheckBox, QHBoxLayout, QLineEdit,
                              QLabel, QCompleter, QDialog, QStyledItemDelegate,
@@ -72,6 +72,8 @@ def seed_warning_msg(seed):
 
 class SeedLayout(QVBoxLayout):
 
+    updated = pyqtSignal()
+
     def seed_options(self):
         dialog = QDialog()
         dialog.setWindowTitle(_("Seed Options"))
@@ -128,6 +130,7 @@ class SeedLayout(QVBoxLayout):
             return None
         self.is_ext = cb_ext.isChecked() if 'ext' in self.options else False
         self.seed_type = seed_type_values[clayout.selected_index()] if len(seed_types) >= 2 else 'electrum'
+        self.updated.emit()
 
     def __init__(
             self,

electrum/gui/qt/wizard/wallet.py

@@ -43,11 +43,8 @@ class QENewWalletWizard(NewWalletWizard, QEAbstractWizard):
             'wallet_type': { 'gui': WCWalletType },
             'keystore_type': { 'gui': WCKeystoreType },
             'create_seed': { 'gui': WCCreateSeed },
-            'create_ext': { 'gui': WCEnterExt },
             'confirm_seed': { 'gui': WCConfirmSeed },
-            'confirm_ext': { 'gui': WCConfirmExt },
             'have_seed': { 'gui': WCHaveSeed },
-            'have_ext': { 'gui': WCEnterExt },
             'script_and_derivation': { 'gui': WCScriptAndDerivation},
             'have_master_key': { 'gui': WCHaveMasterKey },
             'multisig': { 'gui': WCMultisig },
@@ -66,22 +63,37 @@ class QENewWalletWizard(NewWalletWizard, QEAbstractWizard):
             },
             'create_ext': {
                 'next': 'confirm_seed',
+                'gui': WCEnterExt
             },
             'confirm_seed': {
                 'next': lambda d: 'confirm_ext' if d['seed_extend'] else self.on_have_or_confirm_seed(d),
-                'accept': lambda d: None if d['seed_extend'] else self.maybe_master_pubkey(d),
+                'accept': lambda d: None if d['seed_extend'] else self.maybe_master_pubkey(d)
             },
             'confirm_ext': {
                 'next': self.on_have_or_confirm_seed,
                 'accept': self.maybe_master_pubkey,
+                'gui': WCConfirmExt
             },
             'have_seed': {
-                'next': lambda d: 'have_ext' if d['seed_extend'] else self.on_have_or_confirm_seed(d),
+                'next': lambda d: 'have_ext' if self.wants_ext(d) else self.on_have_or_confirm_seed(d),
+                'last': lambda d: self.is_single_password() and not
+                                  (self.needs_derivation_path(d) or self.is_multisig(d) or self.wants_ext(d))
             },
             'have_ext': {
                 'next': self.on_have_or_confirm_seed,
                 'accept': self.maybe_master_pubkey,
+                'gui': WCEnterExt
             },
+            'multisig_cosigner_seed': {
+                'next': lambda d: 'multisig_cosigner_have_ext' if self.wants_ext(d) else self.on_have_cosigner_seed(d),
+                'last': lambda d: self.is_single_password() and self.last_cosigner(d) and not
+                                  (self.needs_derivation_path(d) or self.wants_ext(d))
+            },
+            'multisig_cosigner_have_ext': {
+                'next': self.on_have_cosigner_seed,
+                'last': lambda d: self.is_single_password() and self.last_cosigner(d) and not self.needs_derivation_path(d),
+                'gui': WCEnterExt
+            }
         })
 
     @property
@@ -338,6 +350,8 @@ class WCConfirmSeed(WizardComponent):
 
 
 class WCEnterExt(WizardComponent):
+    _logger = get_logger(__name__)
+
     def __init__(self, parent, wizard):
         WizardComponent.__init__(self, parent, wizard, title=_('Seed Extension'))
 
@@ -356,12 +370,41 @@ class WCEnterExt(WizardComponent):
         self.layout().addStretch(1)
 
     def on_text_edited(self, text):
+        # TODO also for cosigners?
         self.ext_edit.warn_issue4566 = self.wizard_data['keystore_type'] == 'haveseed' and \
                                        self.wizard_data['seed_type'] == 'bip39'
-        self.valid = len(text) > 0
+        self.validate()
+
+    def validate(self):
+        self.apply()
+        text = self.ext_edit.text()
+        if len(text) == 0:
+            self.valid = False
+            return
+
+        cosigner_data = self._current_cosigner(self.wizard_data)
+
+        if self.wizard_data['wallet_type'] == 'multisig':
+            if 'seed_variant' in cosigner_data and cosigner_data['seed_variant'] in ['bip39', 'slip39']:
+                # defer validation to when derivation path is known
+                self.valid = True
+            else:
+                if self.wizard.has_duplicate_masterkeys(self.wizard_data):
+                    self._logger.debug('Duplicate master keys!')
+                    # TODO: user feedback
+                    self.valid = False
+                elif self.wizard.has_heterogeneous_masterkeys(self.wizard_data):
+                    self._logger.debug('Heterogenous master keys!')
+                    # TODO: user feedback
+                    self.valid = False
+                else:
+                    self.valid = True
+        else:
+            self.valid = True
 
     def apply(self):
-        self.wizard_data['seed_extra_words'] = self.ext_edit.text()
+        cosigner_data = self._current_cosigner(self.wizard_data)
+        cosigner_data['seed_extra_words'] = self.ext_edit.text()
 
 
 class WCConfirmExt(WizardComponent):
@@ -409,6 +452,8 @@ class WCHaveSeed(WizardComponent):
             parent=self,
             config=self.wizard.config,
         )
+        self.slayout.updated.connect(self.validate)
+
         self.layout().addLayout(self.slayout)
 
     def is_seed(self, x):
@@ -428,8 +473,8 @@ class WCHaveSeed(WizardComponent):
             self.valid = False
             return
 
-        if seed_type in ['bip39', 'slip39']:
-            # defer validation to when derivation path is known
+        if seed_type in ['bip39', 'slip39'] or self.slayout.is_ext:
+            # defer validation to when derivation path and/or passphrase/ext is known
             self.valid = True
         else:
             self.apply()
@@ -445,20 +490,16 @@ class WCHaveSeed(WizardComponent):
             self.valid = seed_valid
 
     def apply(self):
-        wizard_data = self.wizard_data
-        if self.wizard_data['wallet_type'] == 'multisig' and 'multisig_current_cosigner' in self.wizard_data:
-            cosigner = self.wizard_data['multisig_current_cosigner']
-            if cosigner != 0:
-                wizard_data = self.wizard_data['multisig_cosigner_data'][str(cosigner)]
+        cosigner_data = self._current_cosigner(self.wizard_data)
 
-        wizard_data['seed'] = self.slayout.get_seed()
-        wizard_data['seed_variant'] = self.slayout.seed_type
+        cosigner_data['seed'] = self.slayout.get_seed()
+        cosigner_data['seed_variant'] = self.slayout.seed_type
         if self.slayout.seed_type == 'electrum':
-            wizard_data['seed_type'] = mnemonic.seed_type(self.slayout.get_seed())
+            cosigner_data['seed_type'] = mnemonic.seed_type(self.slayout.get_seed())
         else:
-            wizard_data['seed_type'] = self.slayout.seed_type
-        wizard_data['seed_extend'] = self.slayout.is_ext
-        wizard_data['seed_extra_words'] = ''  # empty default
+            cosigner_data['seed_type'] = self.slayout.seed_type
+        cosigner_data['seed_extend'] = self.slayout.is_ext
+        cosigner_data['seed_extra_words'] = ''  # empty default
 
 
 class WCScriptAndDerivation(WizardComponent):
@@ -553,15 +594,10 @@ class WCScriptAndDerivation(WizardComponent):
     def validate(self):
         self.apply()
 
-        wizard_data = self.wizard_data
-        if self.wizard_data['wallet_type'] == 'multisig' and 'multisig_current_cosigner' in self.wizard_data:
-            cosigner = self.wizard_data['multisig_current_cosigner']
-            if cosigner != 0:
-                wizard_data = self.wizard_data['multisig_cosigner_data'][str(cosigner)]
+        cosigner_data = self._current_cosigner(self.wizard_data)
+        derivation_valid = is_bip32_derivation(cosigner_data['derivation_path'])
 
-        derivation_valid = is_bip32_derivation(wizard_data['derivation_path'])
-
-        if self.wizard_data['wallet_type'] == 'multisig':
+        if derivation_valid and self.wizard_data['wallet_type'] == 'multisig':
             if self.wizard.has_duplicate_masterkeys(self.wizard_data):
                 self._logger.debug('Duplicate master keys!')
                 # TODO: user feedback
@@ -574,14 +610,9 @@ class WCScriptAndDerivation(WizardComponent):
         self.valid = derivation_valid
 
     def apply(self):
-        wizard_data = self.wizard_data
-        if self.wizard_data['wallet_type'] == 'multisig' and 'multisig_current_cosigner' in self.wizard_data:
-            cosigner = self.wizard_data['multisig_current_cosigner']
-            if cosigner != 0:
-                wizard_data = self.wizard_data['multisig_cosigner_data'][str(cosigner)]
-
-        wizard_data['script_type'] = self.c_values[self.clayout.selected_index()]
-        wizard_data['derivation_path'] = str(self.derivation_path_edit.text())
+        cosigner_data = self._current_cosigner(self.wizard_data)
+        cosigner_data['script_type'] = self.c_values[self.clayout.selected_index()]
+        cosigner_data['derivation_path'] = str(self.derivation_path_edit.text())
 
 
 class WCCosignerKeystore(WizardComponent):
@@ -686,14 +717,8 @@ class WCHaveMasterKey(WizardComponent):
 
     def apply(self):
         text = self.slayout.get_text()
-        if self.wizard_data['wallet_type'] == 'standard':
-            self.wizard_data['master_key'] = text
-        elif self.wizard_data['wallet_type'] == 'multisig':
-            cosigner = self.wizard_data['multisig_current_cosigner']
-            if cosigner == 0:
-                self.wizard_data['master_key'] = text
-            else:
-                self.wizard_data['multisig_cosigner_data'][str(cosigner)]['master_key'] = text
+        cosigner_data = self._current_cosigner(self.wizard_data)
+        cosigner_data['master_key'] = text
 
 
 class WCMultisig(WizardComponent):

electrum/gui/qt/wizard/wizard.py

@@ -122,7 +122,6 @@ class QEAbstractWizard(QDialog):
 
     def update(self):
         page = self.main_widget.currentWidget()
-        self.title.setText(page.title)
         self.title.setText(f'<b>{page.title}</b>' if page.title else '')
         self.back_button.setText(_('Back') if self.can_go_back() else _('Cancel'))
         self.next_button.setText(_('Next') if not self.is_last(page.wizard_data) else _('Finish'))
@@ -219,3 +218,10 @@ class WizardComponent(QWidget):
     def on_updated(self, *args):
         self.updated.emit(self)
 
+    # returns (sub)dict of current cosigner (or root if first)
+    def _current_cosigner(self, wizard_data):
+        wdata = wizard_data
+        if wizard_data['wallet_type'] == 'multisig' and 'multisig_current_cosigner' in wizard_data:
+            cosigner = wizard_data['multisig_current_cosigner']
+            wdata = wizard_data['multisig_cosigner_data'][str(cosigner)]
+        return wdata

electrum/wizard.py

@@ -67,6 +67,9 @@ class AbstractWizard:
             else:
                 raise Exception(f'accept handler for view {view} is not callable')
 
+        # make a clone for next view
+        wizard_data = copy.deepcopy(wizard_data)
+
         is_finished = False
         if 'next' not in nav:
             # finished
@@ -109,13 +112,12 @@ class AbstractWizard:
         return new_view
 
     def resolve_prev(self):
-        prev_view = self._stack.pop()
+        self._current = self._stack.pop()
 
-        self._logger.debug(f'resolve_prev view is {prev_view}')
+        self._logger.debug(f'resolve_prev view is "{self._current.view}"')
         self.log_stack()
 
-        self._current = prev_view
-        return prev_view
+        return self._current
 
     # check if this view is the final view
     def is_last_view(self, view, wizard_data):
@@ -149,27 +151,29 @@ class AbstractWizard:
 
     def log_stack(self):
         logstr = 'wizard stack:'
-        stack = copy.deepcopy(self._stack)
         i = 0
-        for item in stack:
-            self.sanitize_stack_item(item.wizard_data)
-            logstr += f'\n{i}: {repr(item.wizard_data)}'
+        for item in self._stack:
+            ssi = self.sanitize_stack_item(item.wizard_data)
+            logstr += f'\n{i}: {hex(id(item.wizard_data))} - {repr(ssi)}'
             i += 1
-        current = copy.deepcopy(self._current)
-        self.sanitize_stack_item(current.wizard_data)
-        logstr += f'\nc: {repr(current.wizard_data)}'
+        sci = self.sanitize_stack_item(self._current.wizard_data)
+        logstr += f'\nc: {hex(id(self._current.wizard_data))} - {repr(sci)}'
         self._logger.debug(logstr)
 
-    def sanitize_stack_item(self, _stack_item):
+    def sanitize_stack_item(self, _stack_item) -> dict:
         sensitive_keys = ['seed', 'seed_extra_words', 'master_key', 'private_key_list', 'password']
         def sanitize(_dict):
+            result = {}
             for item in _dict:
                 if isinstance(_dict[item], dict):
-                    sanitize(_dict[item])
+                    result[item] = sanitize(_dict[item])
                 else:
                     if item in sensitive_keys:
-                        _dict[item] = '<sensitive value removed>'
-        sanitize(_stack_item)
+                        result[item] = '<sensitive value removed>'
+                    else:
+                        result[item] = _dict[item]
+            return result
+        return sanitize(_stack_item)
 
 
 class NewWalletWizard(AbstractWizard):
@@ -199,7 +203,8 @@ class NewWalletWizard(AbstractWizard):
             'have_seed': {
                 'next': self.on_have_or_confirm_seed,
                 'accept': self.maybe_master_pubkey,
-                'last': lambda d: self.is_single_password() and not self.is_bip39_seed(d) and not self.is_multisig(d)
+                'last': lambda d: self.is_single_password() and not
+                                  (self.needs_derivation_path(d) or self.is_multisig(d))
             },
             'script_and_derivation': {
                 'next': lambda d: 'wallet_password' if not self.is_multisig(d) else 'multisig_cosigner_keystore',
@@ -218,16 +223,16 @@ class NewWalletWizard(AbstractWizard):
                 'next': self.on_cosigner_keystore_type
             },
             'multisig_cosigner_key': {
-                'next': lambda d: 'wallet_password' if self.has_all_cosigner_data(d) else 'multisig_cosigner_keystore',
-                'last': lambda d: self.is_single_password() and self.has_all_cosigner_data(d)
+                'next': lambda d: 'wallet_password' if self.last_cosigner(d) else 'multisig_cosigner_keystore',
+                'last': lambda d: self.is_single_password() and self.last_cosigner(d)
             },
             'multisig_cosigner_seed': {
                 'next': self.on_have_cosigner_seed,
-                'last': lambda d: self.is_single_password() and self.has_all_cosigner_data(d)
+                'last': lambda d: self.is_single_password() and self.last_cosigner(d) and not self.needs_derivation_path(d)
             },
             'multisig_cosigner_script_and_derivation': {
-                'next': lambda d: 'wallet_password' if self.has_all_cosigner_data(d) else 'multisig_cosigner_keystore',
-                'last': lambda d: self.is_single_password() and self.has_all_cosigner_data(d)
+                'next': lambda d: 'wallet_password' if self.last_cosigner(d) else 'multisig_cosigner_keystore',
+                'last': lambda d: self.is_single_password() and self.last_cosigner(d)
             },
             'imported': {
                 'next': 'wallet_password',
@@ -249,11 +254,21 @@ class NewWalletWizard(AbstractWizard):
     def is_single_password(self):
         raise NotImplementedError()
 
-    def is_bip39_seed(self, wizard_data):
-        return wizard_data.get('seed_variant') == 'bip39'
+    # returns (sub)dict of current cosigner (or root if first)
+    def _current_cosigner(self, wizard_data):
+        wdata = wizard_data
+        if wizard_data['wallet_type'] == 'multisig' and 'multisig_current_cosigner' in wizard_data:
+            cosigner = wizard_data['multisig_current_cosigner']
+            wdata = wizard_data['multisig_cosigner_data'][str(cosigner)]
+        return wdata
 
-    def is_slip39_seed(self, wizard_data):
-        return wizard_data.get('seed_variant') == 'slip39'
+    def needs_derivation_path(self, wizard_data):
+        wdata = self._current_cosigner(wizard_data)
+        return 'seed_variant' in wdata and wdata['seed_variant'] in ['bip39', 'slip39']
+
+    def wants_ext(self, wizard_data):
+        wdata = self._current_cosigner(wizard_data)
+        return 'seed_variant' in wdata and wdata['seed_extend']
 
     def is_multisig(self, wizard_data):
         return wizard_data['wallet_type'] == 'multisig'
@@ -276,9 +291,7 @@ class NewWalletWizard(AbstractWizard):
         }.get(t)
 
     def on_have_or_confirm_seed(self, wizard_data):
-        if self.is_bip39_seed(wizard_data):
-            return 'script_and_derivation'
-        elif self.is_slip39_seed(wizard_data):
+        if self.needs_derivation_path(wizard_data):
             return 'script_and_derivation'
         elif self.is_multisig(wizard_data):
             return 'multisig_cosigner_keystore'
@@ -287,7 +300,7 @@ class NewWalletWizard(AbstractWizard):
 
     def maybe_master_pubkey(self, wizard_data):
         self._logger.debug('maybe_master_pubkey')
-        if (self.is_bip39_seed(wizard_data) or self.is_slip39_seed(wizard_data)) and 'derivation_path' not in wizard_data:
+        if self.needs_derivation_path(wizard_data) and 'derivation_path' not in wizard_data:
             self._logger.debug('deferred, missing derivation_path')
             return
 
@@ -302,23 +315,19 @@ class NewWalletWizard(AbstractWizard):
 
     def on_have_cosigner_seed(self, wizard_data):
         current_cosigner_data = wizard_data['multisig_cosigner_data'][str(wizard_data['multisig_current_cosigner'])]
-        if self.has_all_cosigner_data(wizard_data):
-            return 'wallet_password'
-        elif current_cosigner_data['seed_type'] == 'bip39' and 'derivation_path' not in current_cosigner_data:
+        if self.needs_derivation_path(wizard_data) and 'derivation_path' not in current_cosigner_data:
             return 'multisig_cosigner_script_and_derivation'
+        elif self.last_cosigner(wizard_data):
+            return 'wallet_password'
         else:
             return 'multisig_cosigner_keystore'
 
-    def has_all_cosigner_data(self, wizard_data):
-        # number of items in multisig_cosigner_data is less than participants?
+    def last_cosigner(self, wizard_data):
+        # check if we have the final number of cosigners. Doesn't check if cosigner data itself is complete
+        # (should be validated by wizardcomponents)
         if len(wizard_data['multisig_cosigner_data']) < (wizard_data['multisig_participants'] - 1):
             return False
 
-        # if last cosigner uses bip39 seed, we still need derivation path
-        current_cosigner_data = wizard_data['multisig_cosigner_data'][str(wizard_data['multisig_current_cosigner'])]
-        if 'seed_type' in current_cosigner_data and current_cosigner_data['seed_type'] == 'bip39' and 'derivation_path' not in current_cosigner_data:
-            return False
-
         return True
 
     def has_duplicate_masterkeys(self, wizard_data) -> bool: