hww: smarter auto-selection of which device to pair with

scenario1:
- 2of2 multisig wallet with trezor1 and trezor2 keystores
- only trezor2 connected
- previously we would pair first keystore with connected device and then display error.
  now we will pair the device with the correct keystore on the first try

scenario2:
- standard wallet with trezor1 keystore
- trezor2 connected (different device)
- previously we would pair trezor2 with the keystore and then display error.
  now we will prompt the user to select which device to pair with (out of one)

related: #5789

electrum/plugin.py

@@ -29,7 +29,7 @@ import time
 import threading
 import sys
 from typing import (NamedTuple, Any, Union, TYPE_CHECKING, Optional, Tuple,
-                    Dict, Iterable, List)
+                    Dict, Iterable, List, Sequence)
 
 from .i18n import _
 from .util import (profiler, DaemonThread, UserCancelled, ThreadJob, UserFacingException)
@@ -289,6 +289,7 @@ class BasePlugin(Logger):
 
 class DeviceUnpairableError(UserFacingException): pass
 class HardwarePluginLibraryUnavailable(Exception): pass
+class CannotAutoSelectDevice(Exception): pass
 
 
 class Device(NamedTuple):
@@ -460,19 +461,27 @@ class DeviceMgr(ThreadJob):
     @with_scan_lock
     def client_for_keystore(self, plugin: 'HW_PluginBase', handler: Optional['HardwareHandlerBase'],
                             keystore: 'Hardware_KeyStore',
-                            force_pair: bool) -> Optional['HardwareClientBase']:
+                            force_pair: bool, *,
+                            devices: Sequence['Device'] = None,
+                            allow_user_interaction: bool = True) -> Optional['HardwareClientBase']:
         self.logger.info("getting client for keystore")
         if handler is None:
             raise Exception(_("Handler not found for") + ' ' + plugin.name + '\n' + _("A library is probably missing."))
         handler.update_status(False)
-        devices = self.scan_devices()
+        if devices is None:
+            devices = self.scan_devices()
         xpub = keystore.xpub
         derivation = keystore.get_derivation_prefix()
         assert derivation is not None
         client = self.client_by_xpub(plugin, xpub, handler, devices)
         if client is None and force_pair:
-            info = self.select_device(plugin, handler, keystore, devices)
-            client = self.force_pair_xpub(plugin, handler, info, xpub, derivation)
+            try:
+                info = self.select_device(plugin, handler, keystore, devices,
+                                          allow_user_interaction=allow_user_interaction)
+            except CannotAutoSelectDevice:
+                pass
+            else:
+                client = self.force_pair_xpub(plugin, handler, info, xpub, derivation)
         if client:
             handler.update_status(True)
         if client:
@@ -481,7 +490,7 @@ class DeviceMgr(ThreadJob):
         return client
 
     def client_by_xpub(self, plugin: 'HW_PluginBase', xpub, handler: 'HardwareHandlerBase',
-                       devices: Iterable['Device']) -> Optional['HardwareClientBase']:
+                       devices: Sequence['Device']) -> Optional['HardwareClientBase']:
         _id = self.xpub_id(xpub)
         client = self.client_lookup(_id)
         if client:
@@ -523,7 +532,7 @@ class DeviceMgr(ThreadJob):
               'receive will be unspendable.').format(plugin.device))
 
     def unpaired_device_infos(self, handler: Optional['HardwareHandlerBase'], plugin: 'HW_PluginBase',
-                              devices: List['Device'] = None,
+                              devices: Sequence['Device'] = None,
                               include_failing_clients=False) -> List['DeviceInfo']:
         '''Returns a list of DeviceInfo objects: one for each connected,
         unpaired device accepted by the plugin.'''
@@ -555,15 +564,17 @@ class DeviceMgr(ThreadJob):
         return infos
 
     def select_device(self, plugin: 'HW_PluginBase', handler: 'HardwareHandlerBase',
-                      keystore: 'Hardware_KeyStore', devices: List['Device'] = None) -> 'DeviceInfo':
-        '''Ask the user to select a device to use if there is more than one,
-        and return the DeviceInfo for the device.'''
+                      keystore: 'Hardware_KeyStore', devices: Sequence['Device'] = None,
+                      *, allow_user_interaction: bool = True) -> 'DeviceInfo':
+        """Select the device to use for keystore."""
         # ideally this should not be called from the GUI thread...
         # assert handler.get_gui_thread() != threading.current_thread(), 'must not be called from GUI thread'
         while True:
             infos = self.unpaired_device_infos(handler, plugin, devices)
             if infos:
                 break
+            if not allow_user_interaction:
+                raise CannotAutoSelectDevice()
             msg = _('Please insert your {}').format(plugin.device)
             if keystore.label:
                 msg += ' ({})'.format(keystore.label)
@@ -575,21 +586,30 @@ class DeviceMgr(ThreadJob):
             if not handler.yes_no_question(msg):
                 raise UserCancelled()
             devices = None
-        if len(infos) == 1:
-            return infos[0]
-        # select device by id
+
+        # select device automatically. (but only if we have reasonable expectation it is the correct one)
+        # method 1: select device by id
         if keystore.soft_device_id:
             for info in infos:
                 if info.soft_device_id == keystore.soft_device_id:
                     return info
-        # select device by label automatically;
-        # but only if not a placeholder label and only if there is no collision
+        # method 2: select device by label
+        #           but only if not a placeholder label and only if there is no collision
         device_labels = [info.label for info in infos]
         if (keystore.label not in PLACEHOLDER_HW_CLIENT_LABELS
                 and device_labels.count(keystore.label) == 1):
             for info in infos:
                 if info.label == keystore.label:
                     return info
+        # method 3: if there is only one device connected, and we don't have useful label/soft_device_id
+        #           saved for keystore anyway, select it
+        if (len(infos) == 1
+                and keystore.label in PLACEHOLDER_HW_CLIENT_LABELS
+                and keystore.soft_device_id is None):
+            return infos[0]
+
+        if not allow_user_interaction:
+            raise CannotAutoSelectDevice()
         # ask user to select device manually
         msg = _("Please select which {} device to use:").format(plugin.device)
         descriptions = ["{label} ({init}, {transport})"
@@ -638,7 +658,7 @@ class DeviceMgr(ThreadJob):
         return devices
 
     @with_scan_lock
-    def scan_devices(self) -> List['Device']:
+    def scan_devices(self) -> Sequence['Device']:
         self.logger.info("scanning devices...")
 
         # First see what's connected that we know about

electrum/plugins/coldcard/coldcard.py

@@ -4,7 +4,7 @@
 #
 import os, time, io
 import traceback
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Optional
 import struct
 
 from electrum import bip32
@@ -536,11 +536,12 @@ class ColdcardPlugin(HW_PluginBase):
         xpub = client.get_xpub(derivation, xtype)
         return xpub
 
-    def get_client(self, keystore, force_pair=True) -> 'CKCCClient':
+    def get_client(self, keystore, force_pair=True, *,
+                   devices=None, allow_user_interaction=True) -> Optional['CKCCClient']:
         # Acquire a connection to the hardware device (via USB)
-        devmgr = self.device_manager()
-        handler = keystore.handler
-        client = devmgr.client_for_keystore(self, handler, keystore, force_pair)
+        client = super().get_client(keystore, force_pair,
+                                    devices=devices,
+                                    allow_user_interaction=allow_user_interaction)
 
         if client is not None:
             client.ping_check()

electrum/plugins/digitalbitbox/digitalbitbox.py

@@ -741,10 +741,11 @@ class DigitalBitboxPlugin(HW_PluginBase):
         return xpub
 
 
-    def get_client(self, keystore, force_pair=True):
-        devmgr = self.device_manager()
-        handler = keystore.handler
-        client = devmgr.client_for_keystore(self, handler, keystore, force_pair)
+    def get_client(self, keystore, force_pair=True, *,
+                   devices=None, allow_user_interaction=True):
+        client = super().get_client(keystore, force_pair,
+                                    devices=devices,
+                                    allow_user_interaction=allow_user_interaction)
         if client is not None:
             client.check_device_dialog()
         return client

electrum/plugins/hw_wallet/plugin.py

@@ -83,8 +83,15 @@ class HW_PluginBase(BasePlugin):
         """
         raise NotImplementedError()
 
-    def get_client(self, keystore: 'Hardware_KeyStore', force_pair: bool = True) -> Optional['HardwareClientBase']:
-        raise NotImplementedError()
+    def get_client(self, keystore: 'Hardware_KeyStore', force_pair: bool = True, *,
+                   devices: Sequence['Device'] = None,
+                   allow_user_interaction: bool = True) -> Optional['HardwareClientBase']:
+        devmgr = self.device_manager()
+        handler = keystore.handler
+        client = devmgr.client_for_keystore(self, handler, keystore, force_pair,
+                                            devices=devices,
+                                            allow_user_interaction=allow_user_interaction)
+        return client
 
     def show_address(self, wallet: 'Abstract_Wallet', address, keystore: 'Hardware_KeyStore' = None):
         pass  # implemented in child classes

electrum/plugins/hw_wallet/qt.py

@@ -206,9 +206,11 @@ class QtPluginBase(object):
 
     @hook
     def load_wallet(self: Union['QtPluginBase', HW_PluginBase], wallet: 'Abstract_Wallet', window: ElectrumWindow):
-        for keystore in wallet.get_keystores():
-            if not isinstance(keystore, self.keystore_class):
-                continue
+        relevant_keystores = [keystore for keystore in wallet.get_keystores()
+                              if isinstance(keystore, self.keystore_class)]
+        if not relevant_keystores:
+            return
+        for keystore in relevant_keystores:
             if not self.libraries_available:
                 message = keystore.plugin.get_library_not_available_message()
                 window.show_error(message)
@@ -224,8 +226,31 @@ class QtPluginBase(object):
             keystore.handler = handler
             keystore.thread = TaskThread(window, on_error=partial(self.on_task_thread_error, window, keystore))
             self.add_show_address_on_hw_device_button_for_receive_addr(wallet, keystore, window)
-            # Trigger a pairing
-            keystore.thread.add(partial(self.get_client, keystore))
+        # Trigger pairings
+        def trigger_pairings():
+            devmgr = self.device_manager()
+            devices = devmgr.scan_devices()
+            # first pair with all devices that can be auto-selected
+            for keystore in relevant_keystores:
+                try:
+                    self.get_client(keystore=keystore,
+                                    force_pair=True,
+                                    allow_user_interaction=False,
+                                    devices=devices)
+                except UserCancelled:
+                    pass
+            # now do manual selections
+            for keystore in relevant_keystores:
+                try:
+                    self.get_client(keystore=keystore,
+                                    force_pair=True,
+                                    allow_user_interaction=True,
+                                    devices=devices)
+                except UserCancelled:
+                    pass
+
+        some_keystore = relevant_keystores[0]
+        some_keystore.thread.add(trigger_pairings)
 
     def _on_status_bar_button_click(self, *, window: ElectrumWindow, keystore: 'Hardware_KeyStore'):
         try:

electrum/plugins/keepkey/keepkey.py

@@ -179,10 +179,11 @@ class KeepKeyPlugin(HW_PluginBase):
 
         return client
 
-    def get_client(self, keystore, force_pair=True) -> Optional['KeepKeyClient']:
-        devmgr = self.device_manager()
-        handler = keystore.handler
-        client = devmgr.client_for_keystore(self, handler, keystore, force_pair)
+    def get_client(self, keystore, force_pair=True, *,
+                   devices=None, allow_user_interaction=True) -> Optional['KeepKeyClient']:
+        client = super().get_client(keystore, force_pair,
+                                    devices=devices,
+                                    allow_user_interaction=allow_user_interaction)
         # returns the client for a given keystore. can use xpub
         if client:
             client.used()

electrum/plugins/ledger/ledger.py

@@ -612,11 +612,12 @@ class LedgerPlugin(HW_PluginBase):
         xpub = client.get_xpub(derivation, xtype)
         return xpub
 
-    def get_client(self, keystore, force_pair=True):
+    def get_client(self, keystore, force_pair=True, *,
+                   devices=None, allow_user_interaction=True):
         # All client interaction should not be in the main GUI thread
-        devmgr = self.device_manager()
-        handler = keystore.handler
-        client = devmgr.client_for_keystore(self, handler, keystore, force_pair)
+        client = super().get_client(keystore, force_pair,
+                                    devices=devices,
+                                    allow_user_interaction=allow_user_interaction)
         # returns the client for a given keystore. can use xpub
         #if client:
         #    client.used()

electrum/plugins/safe_t/safe_t.py

@@ -141,10 +141,11 @@ class SafeTPlugin(HW_PluginBase):
 
         return client
 
-    def get_client(self, keystore, force_pair=True) -> Optional['SafeTClient']:
-        devmgr = self.device_manager()
-        handler = keystore.handler
-        client = devmgr.client_for_keystore(self, handler, keystore, force_pair)
+    def get_client(self, keystore, force_pair=True, *,
+                   devices=None, allow_user_interaction=True) -> Optional['SafeTClient']:
+        client = super().get_client(keystore, force_pair,
+                                    devices=devices,
+                                    allow_user_interaction=allow_user_interaction)
         # returns the client for a given keystore. can use xpub
         if client:
             client.used()

electrum/plugins/trezor/trezor.py

@@ -177,10 +177,11 @@ class TrezorPlugin(HW_PluginBase):
         # note that this call can still raise!
         return TrezorClientBase(transport, handler, self)
 
-    def get_client(self, keystore, force_pair=True) -> Optional['TrezorClientBase']:
-        devmgr = self.device_manager()
-        handler = keystore.handler
-        client = devmgr.client_for_keystore(self, handler, keystore, force_pair)
+    def get_client(self, keystore, force_pair=True, *,
+                   devices=None, allow_user_interaction=True) -> Optional['TrezorClientBase']:
+        client = super().get_client(keystore, force_pair,
+                                    devices=devices,
+                                    allow_user_interaction=allow_user_interaction)
         # returns the client for a given keystore. can use xpub
         if client:
             client.used()