What changed, and why it matters
This commit fixes a bug in Electrum's wallet setup wizard that affected multisig wallets. When a user tried to enable (reactivate) a cosigner's key in a multisig wallet, the wizard did not correctly recognize the wallet as multisig, which could cause the operation to fail or behave incorrectly. The fix moves the multisig setup logic into the wizard's start routine so it always runs, and adds a regression test. There is no direct evidence this bug could be exploited by an attacker; it appears to be a functional regression that could confuse users or prevent normal wallet management.
Treat as a normal bug-fix commit. Reviewers should verify that the new _convert_wallet_type() is invoked consistently for all wizard entry points and that the added test passes. No urgent security response is indicated by the diff alone.
Security signals we found
Regression fix in wallet wizard state handling for multisig
Move multisig metadata initialization from GUI component to core wizard start
Added unit test covering multisig keystore enable/disable flow
Evidence from the diff
The patch addresses a regression introduced by commit 66c0fec1eaa4ebf4b75ec70b5395320150a2801c. Previously, WCExtendKeystore.apply() set multisig_participants, multisig_signatures, and multisig_cosigner_data only when extending a seed in the GUI wizard. After the regression, multisig wallet flows that did not go through WCExtendKeystore lacked these fields, causing enable_keystore to mis-handle multisig wallets. The fix centralizes conversion of wallet_type strings like ‘2of2’ into ‘multisig’ plus the required metadata in KeystoreWizard.start() via a new _convert_wallet_type() helper. Tests are updated to exercise the multisig enable/disable path.
Changed components
electrum/wizard.pyelectrum/gui/qt/wizard/wallet.pytests/test_wizard.pytests/test_wallet_vertical.pyInspect captured patch +69 / −14
diff --git a/electrum/gui/qt/wizard/wallet.py b/electrum/gui/qt/wizard/wallet.py
index eecc68f..67cc26f 100644
--- a/electrum/gui/qt/wizard/wallet.py
+++ b/electrum/gui/qt/wizard/wallet.py
@@ -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):
diff --git a/electrum/wizard.py b/electrum/wizard.py
index 0a475b5..f15ea6d 100644
--- a/electrum/wizard.py
+++ b/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,6 +258,14 @@ class KeystoreWizard(AbstractWizard):
# one at a time
return True
+ def _convert_wallet_type(self, wizard_data: dict) -> None:
+ assert 'wallet_type' in wizard_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()
if start_viewstate is None:
@@ -265,6 +274,7 @@ class KeystoreWizard(AbstractWizard):
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)
diff --git a/tests/test_wallet_vertical.py b/tests/test_wallet_vertical.py
index 700e9e4..1a41562 100644
--- a/tests/test_wallet_vertical.py
+++ b/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, *,
- config: SimpleConfig, gap_limit=None, gap_limit_for_change=None):
+ def create_multisig_wallet(
+ 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())
diff --git a/tests/test_wizard.py b/tests/test_wizard.py
index e1872f6..a758377 100644
--- a/tests/test_wizard.py
+++ b/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):
@@ -390,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):
Why this scored 22/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.