From e3d1c6aefdd2d8948dd8ba17b03badf3560feead Mon Sep 17 00:00:00 2001 From: SomberNight Date: Thu, 14 Aug 2025 16:16:31 +0000 Subject: [PATCH 1/7] wallet: enable/disable_keystore: trivial clean-up --- electrum/gui/qt/wallet_info_dialog.py | 2 +- electrum/wallet.py | 11 ++++++++--- tests/test_wizard.py | 4 ++-- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/electrum/gui/qt/wallet_info_dialog.py b/electrum/gui/qt/wallet_info_dialog.py index 9d6a26c49..9ab87a5fc 100644 --- a/electrum/gui/qt/wallet_info_dialog.py +++ b/electrum/gui/qt/wallet_info_dialog.py @@ -183,7 +183,7 @@ class WalletInfoDialog(WindowModalDialog): self.window.show_message(_('Cannot disable keystore: You have active lightning channels')) return - msg = _('Disable keystore? This will make the keytore watching-only.') + msg = _('Disable keystore? This will make the keystore watching-only.') if self.wallet.storage.is_encrypted_with_hw_device(): msg += '\n\n' + _('Note that this will disable wallet file encryption, because it uses your hardware wallet device.') if not self.window.question(msg): diff --git a/electrum/wallet.py b/electrum/wallet.py index cf68667ae..b41a81bda 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -3261,6 +3261,12 @@ class Abstract_Wallet(ABC, Logger, EventListener): def save_keystore(self): pass + def enable_keystore(self, keystore: KeyStore, is_hardware_keystore: bool, password) -> None: + raise NotImplementedError() + + def disable_keystore(self, keystore: KeyStore) -> None: + raise NotImplementedError() + @abstractmethod def has_seed(self) -> bool: pass @@ -4030,14 +4036,13 @@ class Deterministic_Wallet(Abstract_Wallet): def get_txin_type(self, address=None): return self.txin_type - def enable_keystore(self, keystore, is_hardware_keystore: bool, password): + def enable_keystore(self, keystore: KeyStore, is_hardware_keystore: bool, password) -> None: if not is_hardware_keystore and self.storage.is_encrypted_with_user_pw(): keystore.update_password(None, password) self.db.put('use_encryption', True) self._update_keystore(keystore) - def disable_keystore(self, keystore): - from .keystore import BIP32_KeyStore + def disable_keystore(self, keystore: KeyStore) -> None: assert not self.has_channels() if hasattr(keystore, 'thread') and keystore.thread: keystore.thread.stop() diff --git a/tests/test_wizard.py b/tests/test_wizard.py index 203e37eb9..70dddd5e8 100644 --- a/tests/test_wizard.py +++ b/tests/test_wizard.py @@ -268,8 +268,8 @@ class KeystoreWizardTestCase(WizardTestCase): 'hw_type': 'trezor', 'master_key': 'zpub6rakEaM5ps5UiQ2yhbWiEkd6ceJfmuzegwc62G4itMz8L7rRFRqh6y8bTCScXV6NfTMUhANYQnfqfBd9dYfBRKf4LD1Yyfc8UvwY1MtNKWs', 'root_fingerprint': 'b3569ff0', - 'label': 'test', - 'soft_device_id': '1', + 'label': 'trezor_unittests', + 'soft_device_id': '088C3F260B66F60E15DE0FA5', }) self.assertTrue(w.is_last_view(v.view, d)) v = w.resolve_next(v.view, d) From 6554ec674ea8cd56433d9b17d08c8a8cf1108652 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Thu, 14 Aug 2025 16:44:57 +0000 Subject: [PATCH 2/7] tests: wizard: KeystoreWizard: add test case for old seed --- tests/test_wizard.py | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/tests/test_wizard.py b/tests/test_wizard.py index 70dddd5e8..94d08b0d5 100644 --- a/tests/test_wizard.py +++ b/tests/test_wizard.py @@ -168,8 +168,10 @@ class KeystoreWizardTestCase(WizardTestCase): async def test_haveseed_electrum(self): w, v = self._wizard_for() d = v.wizard_data + myseed = '9dk' + mypassphrase = '' d.update({ - 'seed': '9dk', 'seed_type': 'segwit', 'seed_extend': False, 'seed_variant': 'electrum', + 'seed': myseed, 'seed_type': 'segwit', 'seed_extend': False, 'seed_variant': 'electrum', }) self.assertTrue(w.is_last_view(v.view, d)) w.resolve_next(v.view, d) @@ -181,17 +183,21 @@ class KeystoreWizardTestCase(WizardTestCase): self.assertTrue(wallet.get_keystore().is_watching_only()) wallet.enable_keystore(ks, ishww, None) self.assertFalse(wallet.get_keystore().is_watching_only()) + self.assertEqual(myseed, wallet.get_keystore().get_seed(None)) + self.assertEqual(mypassphrase, wallet.get_keystore().get_passphrase(None)) async def test_haveseed_ext_electrum(self): w, v = self._wizard_for() d = v.wizard_data + myseed = '9dk' + mypassphrase = 'abc' d.update({ - 'seed': '9dk', 'seed_type': 'segwit', 'seed_extend': True, 'seed_variant': 'electrum', + 'seed': myseed, 'seed_type': 'segwit', 'seed_extend': True, 'seed_variant': 'electrum', }) self.assertFalse(w.is_last_view(v.view, d)) v = w.resolve_next(v.view, d) self.assertEqual('enter_ext', v.view) - d.update({'seed_extra_words': 'abc'}) + d.update({'seed_extra_words': mypassphrase}) self.assertTrue(w.is_last_view(v.view, d)) w.resolve_next(v.view, d) ks, ishww = w._result @@ -202,6 +208,30 @@ class KeystoreWizardTestCase(WizardTestCase): self.assertTrue(wallet.get_keystore().is_watching_only()) wallet.enable_keystore(ks, ishww, None) self.assertFalse(wallet.get_keystore().is_watching_only()) + self.assertEqual(myseed, wallet.get_keystore().get_seed(None)) + self.assertEqual(mypassphrase, wallet.get_keystore().get_passphrase(None)) + + async def test_haveseed_electrum_oldseed(self): + w, v = self._wizard_for() + d = v.wizard_data + myseed = 'powerful random nobody notice nothing important anyway look away hidden message over' + mypassphrase = '' + d.update({ + 'seed': myseed, + 'seed_type': 'old', '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.get_master_public_key(), 'e9d4b7866dd1e91c862aebf62a49548c7dbf7bcc6e4b7b8c9da820c7737968df9c09d5a3e271dc814a29981f81b3faaf2737b551ef5dcc6189cf0f8252c442b3') + + wallet = self._create_xpub_keystore_wallet(xpub='e9d4b7866dd1e91c862aebf62a49548c7dbf7bcc6e4b7b8c9da820c7737968df9c09d5a3e271dc814a29981f81b3faaf2737b551ef5dcc6189cf0f8252c442b3') + self.assertTrue(wallet.get_keystore().is_watching_only()) + wallet.enable_keystore(ks, ishww, None) + self.assertFalse(wallet.get_keystore().is_watching_only()) + self.assertEqual(myseed, wallet.get_keystore().get_seed(None)) + self.assertEqual(mypassphrase, wallet.get_keystore().get_passphrase(None)) async def test_haveseed_bip39(self): w, v = self._wizard_for() From 0fea61ac3a3b6543e9c7140de29d9d5019eb47be Mon Sep 17 00:00:00 2001 From: SomberNight Date: Thu, 14 Aug 2025 17:06:52 +0000 Subject: [PATCH 3/7] tests: wizard: KeystoreWizard: also test disable_keystore() --- electrum/gui/common_qt/plugins.py | 7 +++- electrum/wallet.py | 3 ++ tests/test_wizard.py | 62 ++++++++++++++++++++++++------- 3 files changed, 58 insertions(+), 14 deletions(-) diff --git a/electrum/gui/common_qt/plugins.py b/electrum/gui/common_qt/plugins.py index 3d87322bb..a182306d5 100644 --- a/electrum/gui/common_qt/plugins.py +++ b/electrum/gui/common_qt/plugins.py @@ -1,9 +1,14 @@ import sys +from typing import TYPE_CHECKING, Optional from PyQt6.QtCore import pyqtSignal, pyqtProperty, QObject from electrum.logging import get_logger +if TYPE_CHECKING: + from electrum.gui.qml import ElectrumQmlApplication + from electrum.plugin import BasePlugin + class PluginQObject(QObject): logger = get_logger(__name__) @@ -12,7 +17,7 @@ class PluginQObject(QObject): busyChanged = pyqtSignal() pluginEnabledChanged = pyqtSignal() - def __init__(self, plugin, parent): + def __init__(self, plugin: 'BasePlugin', parent: Optional['ElectrumQmlApplication']): super().__init__(parent) self._busy = False diff --git a/electrum/wallet.py b/electrum/wallet.py index b41a81bda..fac15d6e0 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -3267,6 +3267,9 @@ class Abstract_Wallet(ABC, Logger, EventListener): def disable_keystore(self, keystore: KeyStore) -> None: raise NotImplementedError() + def _update_keystore(self, keystore: KeyStore) -> None: + raise NotImplementedError() + @abstractmethod def has_seed(self) -> bool: pass diff --git a/tests/test_wizard.py b/tests/test_wizard.py index 94d08b0d5..96e9d66e7 100644 --- a/tests/test_wizard.py +++ b/tests/test_wizard.py @@ -2,7 +2,7 @@ import os from electrum import SimpleConfig from electrum.interface import ServerAddr -from electrum.keystore import bip44_derivation, Hardware_KeyStore +from electrum.keystore import bip44_derivation, Hardware_KeyStore, KeyStore, BIP32_KeyStore from electrum.network import NetworkParameters, ProxySettings from electrum.plugin import Plugins, DeviceInfo, Device from electrum.wizard import ServerConnectWizard, NewWalletWizard, WizardViewState, KeystoreWizard @@ -124,6 +124,10 @@ class ServerConnectWizardTestCase(WizardTestCase): class KeystoreWizardTestCase(WizardTestCase): + # TODO add test cases for: + # - multisig + # - 2fa + class TKeystoreWizard(KeystoreWizard): def is_single_password(self): """impl abstract reqd""" @@ -165,11 +169,20 @@ class KeystoreWizardTestCase(WizardTestCase): wallet = Daemon._load_wallet(wallet_path, password=None, config=self.config) return wallet + def _sanity_checks_after_disabling_keystore(self, *, ks: 'KeyStore', xpub: str) -> None: + self.assertTrue(ks.is_watching_only()) + self.assertTrue(ks.type in ('bip32', 'old')) + self.assertFalse(ks.has_seed()) + self.assertEqual(ks.get_master_public_key(), xpub) + if isinstance(ks, BIP32_KeyStore): + self.assertEqual(ks.xprv, None) + async def test_haveseed_electrum(self): w, v = self._wizard_for() d = v.wizard_data myseed = '9dk' mypassphrase = '' + myxpub = 'zpub6nAZodjgiMNf9zzX1pTqd6ZVX61ax8azhUDnWRumKVUr1VYATVoqAuqv3qKsb8WJXjxei4wei2p4vnMG9RnpKnen2kmgdhvZUmug2NnHNsr' d.update({ 'seed': myseed, 'seed_type': 'segwit', 'seed_extend': False, 'seed_variant': 'electrum', }) @@ -177,20 +190,24 @@ class KeystoreWizardTestCase(WizardTestCase): w.resolve_next(v.view, d) ks, ishww = w._result self.assertFalse(ishww) - self.assertEqual(ks.xpub, 'zpub6nAZodjgiMNf9zzX1pTqd6ZVX61ax8azhUDnWRumKVUr1VYATVoqAuqv3qKsb8WJXjxei4wei2p4vnMG9RnpKnen2kmgdhvZUmug2NnHNsr') + self.assertEqual(ks.xpub, myxpub) - wallet = self._create_xpub_keystore_wallet(xpub='zpub6nAZodjgiMNf9zzX1pTqd6ZVX61ax8azhUDnWRumKVUr1VYATVoqAuqv3qKsb8WJXjxei4wei2p4vnMG9RnpKnen2kmgdhvZUmug2NnHNsr') + wallet = self._create_xpub_keystore_wallet(xpub=myxpub) self.assertTrue(wallet.get_keystore().is_watching_only()) wallet.enable_keystore(ks, ishww, None) self.assertFalse(wallet.get_keystore().is_watching_only()) self.assertEqual(myseed, wallet.get_keystore().get_seed(None)) self.assertEqual(mypassphrase, wallet.get_keystore().get_passphrase(None)) + wallet.disable_keystore(wallet.get_keystore()) + self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub) + async def test_haveseed_ext_electrum(self): w, v = self._wizard_for() d = v.wizard_data myseed = '9dk' mypassphrase = 'abc' + myxpub = 'zpub6oLFCUpqxT8BUzy8g5miUuRofPZ46ZjjvZfcfH7qJanRM7aRYGpNX4uBGtcJRbgcKbi7dYkiiPw1GB2sc3SufyDcZskuQEWp5jBwbNcj1VL' d.update({ 'seed': myseed, 'seed_type': 'segwit', 'seed_extend': True, 'seed_variant': 'electrum', }) @@ -202,20 +219,24 @@ class KeystoreWizardTestCase(WizardTestCase): w.resolve_next(v.view, d) ks, ishww = w._result self.assertFalse(ishww) - self.assertEqual(ks.xpub, 'zpub6oLFCUpqxT8BUzy8g5miUuRofPZ46ZjjvZfcfH7qJanRM7aRYGpNX4uBGtcJRbgcKbi7dYkiiPw1GB2sc3SufyDcZskuQEWp5jBwbNcj1VL') + self.assertEqual(ks.xpub, myxpub) - wallet = self._create_xpub_keystore_wallet(xpub='zpub6oLFCUpqxT8BUzy8g5miUuRofPZ46ZjjvZfcfH7qJanRM7aRYGpNX4uBGtcJRbgcKbi7dYkiiPw1GB2sc3SufyDcZskuQEWp5jBwbNcj1VL') + wallet = self._create_xpub_keystore_wallet(xpub=myxpub) self.assertTrue(wallet.get_keystore().is_watching_only()) wallet.enable_keystore(ks, ishww, None) self.assertFalse(wallet.get_keystore().is_watching_only()) self.assertEqual(myseed, wallet.get_keystore().get_seed(None)) self.assertEqual(mypassphrase, wallet.get_keystore().get_passphrase(None)) + wallet.disable_keystore(wallet.get_keystore()) + self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub) + async def test_haveseed_electrum_oldseed(self): w, v = self._wizard_for() d = v.wizard_data myseed = 'powerful random nobody notice nothing important anyway look away hidden message over' mypassphrase = '' + myxpub = 'e9d4b7866dd1e91c862aebf62a49548c7dbf7bcc6e4b7b8c9da820c7737968df9c09d5a3e271dc814a29981f81b3faaf2737b551ef5dcc6189cf0f8252c442b3' d.update({ 'seed': myseed, 'seed_type': 'old', 'seed_extend': False, 'seed_variant': 'electrum', @@ -224,18 +245,22 @@ class KeystoreWizardTestCase(WizardTestCase): w.resolve_next(v.view, d) ks, ishww = w._result self.assertFalse(ishww) - self.assertEqual(ks.get_master_public_key(), 'e9d4b7866dd1e91c862aebf62a49548c7dbf7bcc6e4b7b8c9da820c7737968df9c09d5a3e271dc814a29981f81b3faaf2737b551ef5dcc6189cf0f8252c442b3') + self.assertEqual(ks.get_master_public_key(), myxpub) - wallet = self._create_xpub_keystore_wallet(xpub='e9d4b7866dd1e91c862aebf62a49548c7dbf7bcc6e4b7b8c9da820c7737968df9c09d5a3e271dc814a29981f81b3faaf2737b551ef5dcc6189cf0f8252c442b3') + wallet = self._create_xpub_keystore_wallet(xpub=myxpub) self.assertTrue(wallet.get_keystore().is_watching_only()) wallet.enable_keystore(ks, ishww, None) self.assertFalse(wallet.get_keystore().is_watching_only()) self.assertEqual(myseed, wallet.get_keystore().get_seed(None)) self.assertEqual(mypassphrase, wallet.get_keystore().get_passphrase(None)) + wallet.disable_keystore(wallet.get_keystore()) + self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub) + async def test_haveseed_bip39(self): w, v = self._wizard_for() d = v.wizard_data + myxpub = 'zpub6jftahH18ngZwMBBp7epRdBwPMPphfdy9gM6P4n5zFUXdfQJmsYfMNZoBnQMkAoBAiQYRyDQKdpxLYp6QuTrWbgmt6v1cxnFdesyiDSocAs' d.update({ 'seed': '9dk', 'seed_type': 'bip39', 'seed_extend': False, 'seed_variant': 'bip39', }) @@ -246,16 +271,20 @@ class KeystoreWizardTestCase(WizardTestCase): v = w.resolve_next(v.view, d) ks, ishww = w._result self.assertFalse(ishww) - self.assertEqual(ks.xpub, 'zpub6jftahH18ngZwMBBp7epRdBwPMPphfdy9gM6P4n5zFUXdfQJmsYfMNZoBnQMkAoBAiQYRyDQKdpxLYp6QuTrWbgmt6v1cxnFdesyiDSocAs') + self.assertEqual(ks.xpub, myxpub) - wallet = self._create_xpub_keystore_wallet(xpub='zpub6jftahH18ngZwMBBp7epRdBwPMPphfdy9gM6P4n5zFUXdfQJmsYfMNZoBnQMkAoBAiQYRyDQKdpxLYp6QuTrWbgmt6v1cxnFdesyiDSocAs') + wallet = self._create_xpub_keystore_wallet(xpub=myxpub) self.assertTrue(wallet.get_keystore().is_watching_only()) wallet.enable_keystore(ks, ishww, None) self.assertFalse(wallet.get_keystore().is_watching_only()) + wallet.disable_keystore(wallet.get_keystore()) + self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub) + async def test_haveseed_ext_bip39(self): w, v = self._wizard_for() d = v.wizard_data + myxpub = 'zpub6jftahH18ngZwVNQQqNX9vgARaQRs5X89bPzjruSH2hgEBr1LRZN8reopYDALiKngTd8j5jUeGDipb68BXqjP6qMFsReLGwP6naDRvzVHxy' d.update({ 'seed': '9dk', 'seed_type': 'bip39', 'seed_extend': True, 'seed_variant': 'bip39', }) @@ -270,16 +299,20 @@ class KeystoreWizardTestCase(WizardTestCase): v = w.resolve_next(v.view, d) ks, ishww = w._result self.assertFalse(ishww) - self.assertEqual(ks.xpub, 'zpub6jftahH18ngZwVNQQqNX9vgARaQRs5X89bPzjruSH2hgEBr1LRZN8reopYDALiKngTd8j5jUeGDipb68BXqjP6qMFsReLGwP6naDRvzVHxy') + self.assertEqual(ks.xpub, myxpub) - wallet = self._create_xpub_keystore_wallet(xpub='zpub6jftahH18ngZwVNQQqNX9vgARaQRs5X89bPzjruSH2hgEBr1LRZN8reopYDALiKngTd8j5jUeGDipb68BXqjP6qMFsReLGwP6naDRvzVHxy') + wallet = self._create_xpub_keystore_wallet(xpub=myxpub) self.assertTrue(wallet.get_keystore().is_watching_only()) wallet.enable_keystore(ks, ishww, None) self.assertFalse(wallet.get_keystore().is_watching_only()) + wallet.disable_keystore(wallet.get_keystore()) + self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub) + async def test_hww(self): w, v = self._wizard_for(hww=True) d = v.wizard_data + myxpub = 'zpub6rakEaM5ps5UiQ2yhbWiEkd6ceJfmuzegwc62G4itMz8L7rRFRqh6y8bTCScXV6NfTMUhANYQnfqfBd9dYfBRKf4LD1Yyfc8UvwY1MtNKWs' d.update({ 'hardware_device': ( 'trezor', @@ -296,7 +329,7 @@ class KeystoreWizardTestCase(WizardTestCase): self.assertEqual('trezor_xpub', v.view) d.update({ 'hw_type': 'trezor', - 'master_key': 'zpub6rakEaM5ps5UiQ2yhbWiEkd6ceJfmuzegwc62G4itMz8L7rRFRqh6y8bTCScXV6NfTMUhANYQnfqfBd9dYfBRKf4LD1Yyfc8UvwY1MtNKWs', + 'master_key': myxpub, 'root_fingerprint': 'b3569ff0', 'label': 'trezor_unittests', 'soft_device_id': '088C3F260B66F60E15DE0FA5', @@ -307,12 +340,15 @@ class KeystoreWizardTestCase(WizardTestCase): ks, ishww = w._result self.assertTrue(ishww) - wallet = self._create_xpub_keystore_wallet(xpub='zpub6rakEaM5ps5UiQ2yhbWiEkd6ceJfmuzegwc62G4itMz8L7rRFRqh6y8bTCScXV6NfTMUhANYQnfqfBd9dYfBRKf4LD1Yyfc8UvwY1MtNKWs') + wallet = self._create_xpub_keystore_wallet(xpub=myxpub) self.assertTrue(wallet.get_keystore().is_watching_only()) wallet.enable_keystore(ks, ishww, None) self.assertFalse(wallet.get_keystore().is_watching_only()) self.assertTrue(isinstance(wallet.get_keystore(), Hardware_KeyStore)) + wallet.disable_keystore(wallet.get_keystore()) + self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub) + class WalletWizardTestCase(WizardTestCase): From 0ceb54b61f805512ed78f9b0796923c02465ea74 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Fri, 15 Aug 2025 16:18:13 +0000 Subject: [PATCH 4/7] wallet: enable/disable_keystore: rm functionality from 2fa wallets was already not working, but does not even really make sense without larger changes --- electrum/gui/qt/wallet_info_dialog.py | 21 +++++++++++---------- electrum/plugins/trustedcoin/trustedcoin.py | 10 ++++++++++ electrum/wallet.py | 12 ++++++++++++ tests/test_wizard.py | 20 ++++++++++++++++++-- 4 files changed, 51 insertions(+), 12 deletions(-) diff --git a/electrum/gui/qt/wallet_info_dialog.py b/electrum/gui/qt/wallet_info_dialog.py index 9ab87a5fc..b194f0777 100644 --- a/electrum/gui/qt/wallet_info_dialog.py +++ b/electrum/gui/qt/wallet_info_dialog.py @@ -150,16 +150,17 @@ class WalletInfoDialog(WindowModalDialog): bip32fp_hbox.addWidget(bip32fp_text) bip32fp_hbox.addStretch() ks_vbox.addLayout(bip32fp_hbox) - ks_buttons = [] - if not ks.is_watching_only(): - rm_keystore_button = QPushButton('Disable keystore') - rm_keystore_button.clicked.connect(partial(self.disable_keystore, ks)) - ks_buttons.insert(0, rm_keystore_button) - else: - add_keystore_button = QPushButton('Enable Keystore') - add_keystore_button.clicked.connect(self.enable_keystore) - ks_buttons.insert(0, add_keystore_button) - ks_vbox.addLayout(Buttons(*ks_buttons)) + if wallet.can_enable_disable_keystore(ks): + ks_buttons = [] + if not ks.is_watching_only(): + rm_keystore_button = QPushButton('Disable keystore') + rm_keystore_button.clicked.connect(partial(self.disable_keystore, ks)) + ks_buttons.insert(0, rm_keystore_button) + else: + add_keystore_button = QPushButton('Enable Keystore') + add_keystore_button.clicked.connect(self.enable_keystore) + ks_buttons.insert(0, add_keystore_button) + ks_vbox.addLayout(Buttons(*ks_buttons)) tab_label = _("Cosigner") + f' {idx+1}' if len(keystores) > 1 else _("Keystore") index = self.keystore_tabs.addTab(ks_w, tab_label) if not ks.is_watching_only(): diff --git a/electrum/plugins/trustedcoin/trustedcoin.py b/electrum/plugins/trustedcoin/trustedcoin.py index db1605c20..41e90c9a1 100644 --- a/electrum/plugins/trustedcoin/trustedcoin.py +++ b/electrum/plugins/trustedcoin/trustedcoin.py @@ -45,6 +45,7 @@ from electrum.plugin import BasePlugin, hook from electrum.util import NotEnoughFunds, UserFacingException, error_text_str_to_safe_str from electrum.network import Network from electrum.logging import Logger +from electrum.keystore import KeyStore if TYPE_CHECKING: from electrum.wizard import NewWalletWizard @@ -382,6 +383,15 @@ class Wallet_2fa(Multisig_Wallet): def is_billing_address(self, addr: str) -> bool: return addr in self._billing_addresses_set + def can_enable_disable_keystore(self, ks: KeyStore) -> bool: + return False + + def enable_keystore(self, keystore, is_hardware_keystore, password): + raise Exception("2fa wallet cannot enable keystore") + + def disable_keystore(self, keystore): + raise Exception("2fa wallet cannot disable keystore") + # Utility functions diff --git a/electrum/wallet.py b/electrum/wallet.py index fac15d6e0..9a03dc6e6 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -3261,6 +3261,12 @@ class Abstract_Wallet(ABC, Logger, EventListener): def save_keystore(self): pass + def can_enable_disable_keystore(self, ks: KeyStore) -> bool: + """Whether the wallet is capable of disabling/enabling the given keystore. + This is a necessary but not sufficient check: e.g. if wallet has LN channels, we should not allow disabling. + """ + return False + def enable_keystore(self, keystore: KeyStore, is_hardware_keystore: bool, password) -> None: raise NotImplementedError() @@ -4039,14 +4045,20 @@ class Deterministic_Wallet(Abstract_Wallet): def get_txin_type(self, address=None): return self.txin_type + def can_enable_disable_keystore(self, ks: KeyStore) -> bool: + return True + def enable_keystore(self, keystore: KeyStore, is_hardware_keystore: bool, password) -> None: + assert self.can_enable_disable_keystore(keystore) if not is_hardware_keystore and self.storage.is_encrypted_with_user_pw(): keystore.update_password(None, password) self.db.put('use_encryption', True) self._update_keystore(keystore) def disable_keystore(self, keystore: KeyStore) -> None: + assert self.can_enable_disable_keystore(keystore) assert not self.has_channels() + assert keystore in self.get_keystores() if hasattr(keystore, 'thread') and keystore.thread: keystore.thread.stop() if self.storage.is_encrypted_with_hw_device(): diff --git a/tests/test_wizard.py b/tests/test_wizard.py index 96e9d66e7..6ceb1df3f 100644 --- a/tests/test_wizard.py +++ b/tests/test_wizard.py @@ -126,7 +126,6 @@ class ServerConnectWizardTestCase(WizardTestCase): class KeystoreWizardTestCase(WizardTestCase): # TODO add test cases for: # - multisig - # - 2fa class TKeystoreWizard(KeystoreWizard): def is_single_password(self): @@ -194,6 +193,7 @@ class KeystoreWizardTestCase(WizardTestCase): wallet = self._create_xpub_keystore_wallet(xpub=myxpub) self.assertTrue(wallet.get_keystore().is_watching_only()) + self.assertTrue(wallet.can_enable_disable_keystore(ks)) wallet.enable_keystore(ks, ishww, None) self.assertFalse(wallet.get_keystore().is_watching_only()) self.assertEqual(myseed, wallet.get_keystore().get_seed(None)) @@ -223,6 +223,7 @@ class KeystoreWizardTestCase(WizardTestCase): wallet = self._create_xpub_keystore_wallet(xpub=myxpub) self.assertTrue(wallet.get_keystore().is_watching_only()) + self.assertTrue(wallet.can_enable_disable_keystore(ks)) wallet.enable_keystore(ks, ishww, None) self.assertFalse(wallet.get_keystore().is_watching_only()) self.assertEqual(myseed, wallet.get_keystore().get_seed(None)) @@ -249,6 +250,7 @@ class KeystoreWizardTestCase(WizardTestCase): wallet = self._create_xpub_keystore_wallet(xpub=myxpub) self.assertTrue(wallet.get_keystore().is_watching_only()) + self.assertTrue(wallet.can_enable_disable_keystore(ks)) wallet.enable_keystore(ks, ishww, None) self.assertFalse(wallet.get_keystore().is_watching_only()) self.assertEqual(myseed, wallet.get_keystore().get_seed(None)) @@ -275,6 +277,7 @@ class KeystoreWizardTestCase(WizardTestCase): wallet = self._create_xpub_keystore_wallet(xpub=myxpub) self.assertTrue(wallet.get_keystore().is_watching_only()) + self.assertTrue(wallet.can_enable_disable_keystore(ks)) wallet.enable_keystore(ks, ishww, None) self.assertFalse(wallet.get_keystore().is_watching_only()) @@ -303,6 +306,7 @@ class KeystoreWizardTestCase(WizardTestCase): wallet = self._create_xpub_keystore_wallet(xpub=myxpub) self.assertTrue(wallet.get_keystore().is_watching_only()) + self.assertTrue(wallet.can_enable_disable_keystore(ks)) wallet.enable_keystore(ks, ishww, None) self.assertFalse(wallet.get_keystore().is_watching_only()) @@ -342,6 +346,7 @@ class KeystoreWizardTestCase(WizardTestCase): wallet = self._create_xpub_keystore_wallet(xpub=myxpub) self.assertTrue(wallet.get_keystore().is_watching_only()) + self.assertTrue(wallet.can_enable_disable_keystore(ks)) wallet.enable_keystore(ks, ishww, None) self.assertFalse(wallet.get_keystore().is_watching_only()) self.assertTrue(isinstance(wallet.get_keystore(), Hardware_KeyStore)) @@ -701,7 +706,17 @@ class WalletWizardTestCase(WizardTestCase): v = w.resolve_next(v.view, d) self.assertEqual('trustedcoin_show_confirm_otp', v.view) v = w.resolve_next(v.view, d) - self._set_password_and_check_address(v=v, w=w, recv_addr="bc1qnf5qafvpx0afk47433j3tt30pqkxp5wa263m77wt0pvyqq67rmfs522m94") + wallet = self._set_password_and_check_address(v=v, w=w, recv_addr="bc1qnf5qafvpx0afk47433j3tt30pqkxp5wa263m77wt0pvyqq67rmfs522m94") + + with self.subTest(msg="2fa wallet cannot enable/disable keystore"): + for ks in wallet.get_keystores(): + self.assertFalse(wallet.can_enable_disable_keystore(ks)) + with self.assertRaises(Exception) as ctx: + wallet.enable_keystore(ks, False, None) + self.assertTrue("2fa wallet cannot" in ctx.exception.args[0]) + with self.assertRaises(Exception) as ctx: + wallet.enable_keystore(ks, False, None) + self.assertTrue("2fa wallet cannot" in ctx.exception.args[0]) async def test_2fa_haveseed_keep2FAenabled(self): self.assertTrue(self.config.get('enable_plugin_trustedcoin')) @@ -1054,3 +1069,4 @@ class WalletWizardTestCase(WizardTestCase): set(wallet.get_receiving_addresses()), {"bc1qq2tmmcngng78nllq2pvrkchcdukemtj56uyue0", "1LNvv5h6QHoYv1nJcqrp13T2TBkD2sUGn1", "1FJEEB8ihPMbzs2SkLmr37dHyRFzakqUmo"}, ) + self.assertFalse(wallet.can_enable_disable_keystore(wallet.keystore)) From f025a75356cce42e94486c168217253233e53791 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Fri, 15 Aug 2025 17:19:22 +0000 Subject: [PATCH 5/7] tests: wizard: use real bip39 seeds --- tests/test_wizard.py | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/tests/test_wizard.py b/tests/test_wizard.py index 6ceb1df3f..3ba3840e8 100644 --- a/tests/test_wizard.py +++ b/tests/test_wizard.py @@ -126,6 +126,7 @@ class ServerConnectWizardTestCase(WizardTestCase): class KeystoreWizardTestCase(WizardTestCase): # TODO add test cases for: # - multisig + # - mismatching xpub vs seed errors class TKeystoreWizard(KeystoreWizard): def is_single_password(self): @@ -262,14 +263,15 @@ class KeystoreWizardTestCase(WizardTestCase): async def test_haveseed_bip39(self): w, v = self._wizard_for() d = v.wizard_data - myxpub = 'zpub6jftahH18ngZwMBBp7epRdBwPMPphfdy9gM6P4n5zFUXdfQJmsYfMNZoBnQMkAoBAiQYRyDQKdpxLYp6QuTrWbgmt6v1cxnFdesyiDSocAs' + myxpub = 'zpub6rFR7y4Q2AijBEqTUquhVz398htDFrtymD9xYYfG1m4wAcvPhXNfE3EfH1r1ADqtfSdVCToUG868RvUUkgDKf31mGDtKsAYz2oz2AGutZYs' d.update({ - 'seed': '9dk', 'seed_type': 'bip39', 'seed_extend': False, 'seed_variant': 'bip39', + 'seed': 'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about', + 'seed_type': 'bip39', 'seed_extend': False, 'seed_variant': 'bip39', }) self.assertFalse(w.is_last_view(v.view, d)) v = w.resolve_next(v.view, d) self.assertEqual('script_and_derivation', v.view) - d.update({'script_type': 'p2wpkh', 'derivation_path': 'm'}) + d.update({'script_type': 'p2wpkh', 'derivation_path': 'm/84h/0h/0h'}) v = w.resolve_next(v.view, d) ks, ishww = w._result self.assertFalse(ishww) @@ -287,9 +289,10 @@ class KeystoreWizardTestCase(WizardTestCase): async def test_haveseed_ext_bip39(self): w, v = self._wizard_for() d = v.wizard_data - myxpub = 'zpub6jftahH18ngZwVNQQqNX9vgARaQRs5X89bPzjruSH2hgEBr1LRZN8reopYDALiKngTd8j5jUeGDipb68BXqjP6qMFsReLGwP6naDRvzVHxy' + myxpub = 'zpub6qaQ1V7UyjNRXR5u8QzTi1ibaWQkskUsfpi7na4oqwkXrZWzVqqohSKG8g2sL5m8CJju2E8GFRkZBxKKq5iEqS167CLLDK2jNz4vpNAea7X' d.update({ - 'seed': '9dk', 'seed_type': 'bip39', 'seed_extend': True, 'seed_variant': 'bip39', + 'seed': 'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about', + 'seed_type': 'bip39', 'seed_extend': True, 'seed_variant': 'bip39', }) self.assertFalse(w.is_last_view(v.view, d)) v = w.resolve_next(v.view, d) @@ -298,7 +301,7 @@ class KeystoreWizardTestCase(WizardTestCase): v = w.resolve_next(v.view, d) self.assertEqual('script_and_derivation', v.view) - d.update({'script_type': 'p2wpkh', 'derivation_path': 'm'}) + d.update({'script_type': 'p2wpkh', 'derivation_path': 'm/84h/0h/0h'}) v = w.resolve_next(v.view, d) ks, ishww = w._result self.assertFalse(ishww) @@ -592,13 +595,14 @@ class WalletWizardTestCase(WizardTestCase): v = w.resolve_next(v.view, d) self.assertEqual('have_seed', v.view) - d.update({'seed': '9dk', 'seed_type': 'bip39', 'seed_extend': False, 'seed_variant': 'bip39'}) + d.update({'seed': 'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about', + 'seed_type': 'bip39', 'seed_extend': False, 'seed_variant': 'bip39'}) v = w.resolve_next(v.view, d) self.assertEqual('script_and_derivation', v.view) - d.update({'script_type': 'p2wpkh', 'derivation_path': 'm'}) + d.update({'script_type': 'p2wpkh', 'derivation_path': 'm/84h/0h/0h'}) v = w.resolve_next(v.view, d) - self._set_password_and_check_address(v=v, w=w, recv_addr="bc1qrjr8qn4669jgr3s34f2pyj9awhz02eyvk5eh8g") + self._set_password_and_check_address(v=v, w=w, recv_addr="bc1qcr8te4kr609gcawutmrza0j4xv80jy8z306fyu") async def test_create_standard_wallet_haveseed_bip39_passphrase(self): w = self._wizard_for(wallet_type='standard') @@ -610,7 +614,8 @@ class WalletWizardTestCase(WizardTestCase): v = w.resolve_next(v.view, d) self.assertEqual('have_seed', v.view) - d.update({'seed': '9dk', 'seed_type': 'bip39', 'seed_extend': True, 'seed_variant': 'bip39'}) + d.update({'seed': 'abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about', + 'seed_type': 'bip39', 'seed_extend': True, 'seed_variant': 'bip39'}) v = w.resolve_next(v.view, d) self.assertEqual('have_ext', v.view) @@ -618,9 +623,9 @@ class WalletWizardTestCase(WizardTestCase): v = w.resolve_next(v.view, d) self.assertEqual('script_and_derivation', v.view) - d.update({'script_type': 'p2wpkh', 'derivation_path': 'm'}) + d.update({'script_type': 'p2wpkh', 'derivation_path': 'm/84h/0h/0h'}) v = w.resolve_next(v.view, d) - self._set_password_and_check_address(v=v, w=w, recv_addr="bc1qjexrunguxz8rlfuul8h4apafyh3sq5yp9kg98j") + self._set_password_and_check_address(v=v, w=w, recv_addr="bc1qjc3dsy5wxaksae6zqmr3nwjsmuckwqca8flql3") async def test_create_standard_wallet_haveseed_slip39(self): w = self._wizard_for(wallet_type='standard') From cd63be233bc34287a0eacc5f6ebd12e055478a28 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Fri, 15 Aug 2025 17:29:55 +0000 Subject: [PATCH 6/7] wallet: disable_keystore() not to destroy get_key_origin_info() --- electrum/bip32.py | 7 +++++++ electrum/keystore.py | 4 ++-- tests/test_wizard.py | 28 +++++++++++++++++++++------- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/electrum/bip32.py b/electrum/bip32.py index 95720554f..6d36322f4 100644 --- a/electrum/bip32.py +++ b/electrum/bip32.py @@ -525,3 +525,10 @@ class KeyOriginInfo: xfp.extend(self.path) return xfp + def __eq__(self, other) -> bool: + if not isinstance(other, KeyOriginInfo): + return False + return self.serialize() == other.serialize() + + def __repr__(self) -> str: + return f"" diff --git a/electrum/keystore.py b/electrum/keystore.py index 13241f0e1..0408894c6 100644 --- a/electrum/keystore.py +++ b/electrum/keystore.py @@ -643,7 +643,7 @@ class BIP32_KeyStore(Xpub, Deterministic_KeyStore): return BIP32_KeyStore({ 'xpub': self.xpub, 'root_fingerprint': self.get_root_fingerprint(), - 'derivation_prefix': self.get_derivation_prefix(), + 'derivation': self.get_derivation_prefix(), }) def format_seed(self, seed): @@ -903,7 +903,7 @@ class Hardware_KeyStore(Xpub, KeyStore): return BIP32_KeyStore({ 'xpub': self.xpub, 'root_fingerprint': self.get_root_fingerprint(), - 'derivation_prefix': self.get_derivation_prefix(), + 'derivation': self.get_derivation_prefix(), }) def set_label(self, label: Optional[str]) -> None: diff --git a/tests/test_wizard.py b/tests/test_wizard.py index 3ba3840e8..8eb12faa6 100644 --- a/tests/test_wizard.py +++ b/tests/test_wizard.py @@ -10,6 +10,7 @@ from electrum.daemon import Daemon from electrum.wallet import Abstract_Wallet from electrum import util from electrum import slip39 +from electrum.bip32 import KeyOriginInfo from . import ElectrumTestCase from .test_wallet_vertical import UNICODE_HORROR @@ -169,13 +170,20 @@ class KeystoreWizardTestCase(WizardTestCase): wallet = Daemon._load_wallet(wallet_path, password=None, config=self.config) return wallet - def _sanity_checks_after_disabling_keystore(self, *, ks: 'KeyStore', xpub: str) -> None: + def _sanity_checks_after_disabling_keystore( + self, + *, + ks: 'KeyStore', + xpub: str, + key_origin_info: KeyOriginInfo, + ) -> None: self.assertTrue(ks.is_watching_only()) self.assertTrue(ks.type in ('bip32', 'old')) self.assertFalse(ks.has_seed()) self.assertEqual(ks.get_master_public_key(), xpub) if isinstance(ks, BIP32_KeyStore): self.assertEqual(ks.xprv, None) + self.assertEqual(ks.get_key_origin_info(), key_origin_info) async def test_haveseed_electrum(self): w, v = self._wizard_for() @@ -200,8 +208,9 @@ class KeystoreWizardTestCase(WizardTestCase): self.assertEqual(myseed, wallet.get_keystore().get_seed(None)) self.assertEqual(mypassphrase, wallet.get_keystore().get_passphrase(None)) + my_keyorigininfo = wallet.get_keystore().get_key_origin_info() wallet.disable_keystore(wallet.get_keystore()) - self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub) + self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub, key_origin_info=my_keyorigininfo) async def test_haveseed_ext_electrum(self): w, v = self._wizard_for() @@ -230,8 +239,9 @@ class KeystoreWizardTestCase(WizardTestCase): self.assertEqual(myseed, wallet.get_keystore().get_seed(None)) self.assertEqual(mypassphrase, wallet.get_keystore().get_passphrase(None)) + my_keyorigininfo = wallet.get_keystore().get_key_origin_info() wallet.disable_keystore(wallet.get_keystore()) - self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub) + self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub, key_origin_info=my_keyorigininfo) async def test_haveseed_electrum_oldseed(self): w, v = self._wizard_for() @@ -257,8 +267,9 @@ class KeystoreWizardTestCase(WizardTestCase): self.assertEqual(myseed, wallet.get_keystore().get_seed(None)) self.assertEqual(mypassphrase, wallet.get_keystore().get_passphrase(None)) + my_keyorigininfo = wallet.get_keystore().get_key_origin_info() wallet.disable_keystore(wallet.get_keystore()) - self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub) + self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub, key_origin_info=my_keyorigininfo) async def test_haveseed_bip39(self): w, v = self._wizard_for() @@ -283,8 +294,9 @@ class KeystoreWizardTestCase(WizardTestCase): wallet.enable_keystore(ks, ishww, None) self.assertFalse(wallet.get_keystore().is_watching_only()) + my_keyorigininfo = wallet.get_keystore().get_key_origin_info() wallet.disable_keystore(wallet.get_keystore()) - self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub) + self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub, key_origin_info=my_keyorigininfo) async def test_haveseed_ext_bip39(self): w, v = self._wizard_for() @@ -313,8 +325,9 @@ class KeystoreWizardTestCase(WizardTestCase): wallet.enable_keystore(ks, ishww, None) self.assertFalse(wallet.get_keystore().is_watching_only()) + my_keyorigininfo = wallet.get_keystore().get_key_origin_info() wallet.disable_keystore(wallet.get_keystore()) - self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub) + self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub, key_origin_info=my_keyorigininfo) async def test_hww(self): w, v = self._wizard_for(hww=True) @@ -354,8 +367,9 @@ class KeystoreWizardTestCase(WizardTestCase): self.assertFalse(wallet.get_keystore().is_watching_only()) self.assertTrue(isinstance(wallet.get_keystore(), Hardware_KeyStore)) + my_keyorigininfo = wallet.get_keystore().get_key_origin_info() wallet.disable_keystore(wallet.get_keystore()) - self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub) + self._sanity_checks_after_disabling_keystore(ks=wallet.get_keystore(), xpub=myxpub, key_origin_info=my_keyorigininfo) class WalletWizardTestCase(WizardTestCase): From 5997494c460c9c258a7a440b11ed37b16190ad5c Mon Sep 17 00:00:00 2001 From: SomberNight Date: Fri, 15 Aug 2025 17:38:56 +0000 Subject: [PATCH 7/7] tests: wizard: add test "adding unrelated seed to xpub-only ks raises" --- electrum/wallet.py | 3 ++- tests/test_wizard.py | 20 +++++++++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/electrum/wallet.py b/electrum/wallet.py index 9a03dc6e6..b931e4ab6 100644 --- a/electrum/wallet.py +++ b/electrum/wallet.py @@ -4106,7 +4106,8 @@ class Standard_Wallet(Simple_Wallet, Deterministic_Wallet): self.keystore.add_slip_19_ownership_proofs_to_tx(tx=tx, password=None) def _update_keystore(self, keystore): - assert self.keystore.get_master_public_key() == keystore.get_master_public_key() + if self.keystore.get_master_public_key() != keystore.get_master_public_key(): + raise Exception("mismatching xpubs") self.keystore = keystore self.save_keystore() diff --git a/tests/test_wizard.py b/tests/test_wizard.py index 8eb12faa6..dbe54f794 100644 --- a/tests/test_wizard.py +++ b/tests/test_wizard.py @@ -127,7 +127,6 @@ class ServerConnectWizardTestCase(WizardTestCase): class KeystoreWizardTestCase(WizardTestCase): # TODO add test cases for: # - multisig - # - mismatching xpub vs seed errors class TKeystoreWizard(KeystoreWizard): def is_single_password(self): @@ -243,6 +242,25 @@ 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_haveseed_electrum__mismatching_seed(self): + """adding an unrelated seed to an xpub-only keystore should raise""" + w, v = self._wizard_for() + d = v.wizard_data + d.update({ + 'seed': 'abandon bike', '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) + + wallet = self._create_xpub_keystore_wallet(xpub='zpub6nAZodjgiMNf9zzX1pTqd6ZVX61ax8azhUDnWRumKVUr1VYATVoqAuqv3qKsb8WJXjxei4wei2p4vnMG9RnpKnen2kmgdhvZUmug2NnHNsr') + self.assertTrue(wallet.get_keystore().is_watching_only()) + self.assertTrue(wallet.can_enable_disable_keystore(ks)) + with self.assertRaises(Exception) as ctx: + wallet.enable_keystore(ks, ishww, None) + self.assertTrue("mismatching xpubs" in ctx.exception.args[0]) + async def test_haveseed_electrum_oldseed(self): w, v = self._wizard_for() d = v.wizard_data