trezor: restore support for external inputs

Author: matejcik

docs/devices/index.rst

@@ -51,15 +51,15 @@ The table below lists what devices and features are supported for each device.
 +------------------------------------+---------------+---------------+------------+----------------+----------+----------+---------+----------+------------------+
 | Arbitrary witnessScript Inputs     | ✓             | ✓             | ―          | ―              | ✓        | ―        | ―       | ―        | ✓                |
 +------------------------------------+---------------+---------------+------------+----------------+----------+----------+---------+----------+------------------+
-| Non-wallet inputs                  | ✓             | ✓             | ✗\ :sup:`1` | ✗\ :sup:`2`    | ✓        | ―        | ✓       | ✓        | ✓                |
+| Non-wallet inputs                  | ✓             | ✓             | ✓\ :sup:`1` | ✓\ :sup:`2`    | ✓        | ―        | ✓       | ✓        | ✓                |
 +------------------------------------+---------------+---------------+------------+----------------+----------+----------+---------+----------+------------------+
 | Mixed Segwit and Non-Segwit Inputs | ―             | ―             | ✓          | ✓              | ✓        | ✓        | ✓       | ✓        | ✓                |
 +------------------------------------+---------------+---------------+------------+----------------+----------+----------+---------+----------+------------------+
 | Display on device screen           | ✓             | ✓             | ✓          | ✓              | ―        | ✓        | ✓       | ✓        | ✓                |
 +------------------------------------+---------------+---------------+------------+----------------+----------+----------+---------+----------+------------------+
 
-* 1 - Support removed for devices with firmware 1.10.6 and greater.
-* 2 - Support removed for devices with firmware 2.4.4 and greater.
+* 1 - Since firmware 1.10.6, safety checks must be disabled.
+* 2 - Since firmware 2.4.4, safety checks must be disabled.
 
 \* There are some caveats. See the `sign_tx` for these devices.
 

hwilib/devices/trezor.py

@@ -275,6 +275,14 @@ def get_path_transport(
     raise BadArgumentError(f"Could not find device by path: {path}")
 
 
+def _use_external_script_type(client) -> bool:
+    if client.features.model == "1" and client.version > (1, 10, 5):
+        return True
+    if client.features.model == "T" and client.version > (2, 4, 3):
+        return True
+    return False
+
+
 # This class extends the HardwareWalletClient for Trezor specific things
 class TrezorClient(HardwareWalletClient):
 
@@ -327,15 +335,6 @@ def _check_unlocked(self) -> None:
         if self.client.features.pin_protection and not self.client.features.unlocked:
             raise DeviceNotReadyError('{} is locked. Unlock by using \'promptpin\' and then \'sendpin\'.'.format(self.type))
 
-    def _supports_external(self) -> bool:
-        if self.client.features.model == "1" and self.client.version <= (1, 10, 5):
-            return True
-        if self.client.features.model == "T" and self.client.version <= (2, 4, 3):
-            return True
-        if self.client.features.model == "K1-14AM":
-            return True
-        return False
-
     @trezor_exception
     def get_pubkey_at_path(self, path: str) -> ExtendedKey:
         self._check_unlocked()
@@ -431,9 +430,13 @@ def sign_tx(self, tx: PSBT) -> PSBT:
                     p2wsh = True
 
                 def ignore_input() -> None:
-                    txinputtype.address_n = [0x80000000 | 84, 0x80000000 | (0 if self.chain == Chain.MAIN else 1), 0x80000000, 0, 0]
                     txinputtype.multisig = None
-                    txinputtype.script_type = messages.InputScriptType.SPENDWITNESS
+                    if _use_external_script_type(self.client):
+                        txinputtype.script_type = messages.InputScriptType.EXTERNAL
+                        txinputtype.script_pubkey = utxo.scriptPubKey
+                    else:
+                        txinputtype.address_n = [0x80000000 | 84, 0x80000000 | (0 if self.chain == Chain.MAIN else 1), 0x80000000, 0, 0]
+                        txinputtype.script_type = messages.InputScriptType.SPENDWITNESS
                     inputs.append(txinputtype)
                     to_ignore.append(input_num)
 
@@ -446,21 +449,12 @@ def ignore_input() -> None:
                         if utxo.is_p2sh:
                             txinputtype.script_type = messages.InputScriptType.SPENDMULTISIG
                         else:
-                            # Cannot sign bare multisig, ignore it
-                            if not self._supports_external():
-                                raise BadArgumentError("Cannot sign bare multisig")
                             ignore_input()
                             continue
                 elif not is_ms and not is_wit and not is_p2pkh(scriptcode):
-                    # Cannot sign unknown spk, ignore it
-                    if not self._supports_external():
-                        raise BadArgumentError("Cannot sign unknown scripts")
                     ignore_input()
                     continue
                 elif not is_ms and is_wit and p2wsh:
-                    # Cannot sign unknown witness script, ignore it
-                    if not self._supports_external():
-                        raise BadArgumentError("Cannot sign unknown witness versions")
                     ignore_input()
                     continue
 
@@ -497,9 +491,6 @@ def ignore_input() -> None:
                     passes = our_keys
 
                 if not found and not found_in_sigs: # None of our keys were in hd_keypaths or in partial_sigs
-                    # This input is not one of ours
-                    if not self._supports_external():
-                        raise BadArgumentError("Cannot sign external inputs")
                     ignore_input()
                     continue
                 elif not found and found_in_sigs:

test/test_device.py

@@ -573,7 +573,7 @@ def test_signtx(self):
     # Make a huge transaction which might cause some problems with different interfaces
     def test_big_tx(self):
         # make a huge transaction
-        keypool_desc = self.do_command(self.dev_args + ["getkeypool", "--account", "10", "--addr-type", "legacy", "0", "100"])
+        keypool_desc = self.do_command(self.dev_args + ["getkeypool", "--account", "10", "--addr-type", "legacy", "0", "200"])
         import_result = self.wrpc.importdescriptors(keypool_desc)
         self.assertTrue(import_result[0]['success'])
         outputs = []

test/test_trezor.py

@@ -28,7 +28,7 @@
 )
 
 from hwilib._cli import process_commands
-from hwilib.devices.trezor import TrezorClient
+from hwilib.devices.trezor import TrezorClient, _use_external_script_type
 
 from types import MethodType
 
@@ -63,6 +63,7 @@ def __init__(self, path, model):
         self.strict_bip48 = True
         self.include_xpubs = False
         self.supports_device_multiple_multisig = True
+        self.client = None
 
     def start(self):
         super().start()
@@ -85,12 +86,12 @@ def start(self):
 
         # Setup the emulator
         wirelink = UdpTransport.enumerate()[0]
-        client = TrezorClientDebugLink(wirelink)
-        client.init_device()
-        device.wipe(client)
-        load_device_by_mnemonic(client=client, mnemonic='alcohol woman abuse must during monitor noble actual mixed trade anger aisle', pin='', passphrase_protection=False, label='test') # From Trezor device tests
+        self.client = TrezorClientDebugLink(wirelink)
+        self.client.init_device()
+        device.wipe(self.client)
+        load_device_by_mnemonic(client=self.client, mnemonic='alcohol woman abuse must during monitor noble actual mixed trade anger aisle', pin='', passphrase_protection=False, label='test') # From Trezor device tests
         atexit.register(self.stop)
-        return client
+        return self.client
 
     def stop(self):
         super().stop()
@@ -407,6 +408,16 @@ def test_passphrase(self):
         else:
             self.fail("Did not enumerate device")
 
+class TestSignTxTrezorExternal(TestSignTx):
+    def setUp(self):
+        super().setUp()
+        if _use_external_script_type(self.emulator.client):
+            device.apply_settings(self.emulator.client, safety_checks=messages.SafetyCheckLevel.PromptTemporarily)
+
+    def tearDown(self):
+        device.apply_settings(self.emulator.client, safety_checks=messages.SafetyCheckLevel.Strict)
+        return super().tearDown()
+
 def trezor_test_suite(emulator, bitcoind, interface, model):
     assert model in TREZOR_MODELS
     # Redirect stderr to /dev/null as it's super spammy
@@ -421,13 +432,15 @@ def trezor_test_suite(emulator, bitcoind, interface, model):
         (["legacy", "segwit"], ["legacy", "segwit"], False, True),
         (["legacy", "segwit", "tap"], ["legacy", "segwit"], False, True),
     ]
+    signtx_cases_external = [(_addr, _multi, True, _opret) for _addr, _multi, _ext, _opret in signtx_cases]
 
     # Generic Device tests
     suite = unittest.TestSuite()
     suite.addTest(DeviceTestCase.parameterize(TestDeviceConnect, bitcoind, emulator=dev_emulator, interface=interface, detect_type="trezor"))
     suite.addTest(DeviceTestCase.parameterize(TestGetDescriptors, bitcoind, emulator=dev_emulator, interface=interface))
     suite.addTest(DeviceTestCase.parameterize(TestGetKeypool, bitcoind, emulator=dev_emulator, interface=interface))
     suite.addTest(DeviceTestCase.parameterize(TestSignTx, bitcoind, emulator=dev_emulator, interface=interface, signtx_cases=signtx_cases))
+    suite.addTest(DeviceTestCase.parameterize(TestSignTxTrezorExternal, bitcoind, emulator=dev_emulator, interface=interface, signtx_cases=signtx_cases_external))
     suite.addTest(DeviceTestCase.parameterize(TestDisplayAddress, bitcoind, emulator=dev_emulator, interface=interface))
     suite.addTest(DeviceTestCase.parameterize(TestSignMessage, bitcoind, emulator=dev_emulator, interface=interface))
     if model != 't':