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

@@ -150,16 +150,17 @@ class WalletInfoDialog(WindowModalDialog):
                 bip32fp_hbox.addWidget(bip32fp_text)
                 bip32fp_hbox.addStretch()
                 ks_vbox.addLayout(bip32fp_hbox)
-                ks_buttons = []
+                if wallet.can_enable_disable_keystore(ks):
-                if not ks.is_watching_only():
+                    ks_buttons = []
-                    rm_keystore_button = QPushButton('Disable keystore')
+                    if not ks.is_watching_only():
-                    rm_keystore_button.clicked.connect(partial(self.disable_keystore, ks))
+                        rm_keystore_button = QPushButton('Disable keystore')
-                    ks_buttons.insert(0, rm_keystore_button)
+                        rm_keystore_button.clicked.connect(partial(self.disable_keystore, ks))
-                else:
+                        ks_buttons.insert(0, rm_keystore_button)
-                    add_keystore_button = QPushButton('Enable Keystore')
+                    else:
-                    add_keystore_button.clicked.connect(self.enable_keystore)
+                        add_keystore_button = QPushButton('Enable Keystore')
-                    ks_buttons.insert(0, add_keystore_button)
+                        add_keystore_button.clicked.connect(self.enable_keystore)
-                ks_vbox.addLayout(Buttons(*ks_buttons))
+                        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():

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
 

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():

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))