Repository: bitcoin-core/hwi
Issue: HWI won't sign PSBT with OP_RETURNs "Output is not an address"

HWI produces an error when signing OP_RETURN transactions on Trezor with the error `{"error": "Output is not an address", "code": -7}`.  The error does not present on the Trezor device and comes from HWI itself.  The same PSBT can be signed on the Trezor using a paired Electrum wallet.  Similar TXNs without OP_RETURNs are signed without issue.  Details follow:

## Given:
```
  TYPE: trezor
  PATH: webusb:001:1
  FPR:  580aeee5
  TXID: ed5221bb03c5844624e05cc6a8366023080fe516b1c90401ce345ef01b9f10ba
  VOUT: 1
  ADDR: tb1qe8ajanyh0ju5stempl52szzzv427445cpfh0ce
  AMT:  0.00020000
```  
## PSBTs

###   With OP_RETURN: 
```  
    createpsbt '[{"txid":"<TXID>","vout":<VOUT>}]' '[{"data":"deadbeef"},{"<ADDR>":"<AMT>"}]'
    createpsbt_out:
      cHNidP8BAGECAAAAAboQnxvwXjTOAQTJsRblDwgjYDaoxlzgJEaExQO7IVLtAQAA
      AAD/////AgAAAAAAAAAABmoE3q2+7yBOAAAAAAAAFgAUyfsuzJd8uUgvOw/oqAhC
      ZVXq1pgAAAAAAAAAAA==
```
```
    walletprocesspsbt '<createpsbt_out>'
    walletprocesspsbt_out: 
      cHNidP8BAGECAAAAAboQnxvwXjTOAQTJsRblDwgjYDaoxlzgJEaExQO7IVLtAQAA
      AAD/////AgAAAAAAAAAABmoE3q2+7yBOAAAAAAAAFgAUyfsuzJd8uUgvOw/oqAhC
      ZVXq1pgAAAAAAAEAcQIAAAAB/5wSVleWGJvGjrVXwiPOXKyiDiOt3a93Jp2LzQTZ
      7boBAAAAAP7///8CfRWdLQAAAAAWABQIN+uHqhhAL3oOdB9bLUgn2e8qpyBOAAAA
      AAAAFgAUyfsuzJd8uUgvOw/oqAhCZVXq1ph9AxwAAQEfIE4AAAAAAAAWABTJ+y7M
      l3y5SC87D+ioCEJlVerWmCIGAspH30ALxqovTLtF+3ta0WjM1hNIO5gaA6bBfcM8
      Y1j0GFgK7uVUAACAAQAAgAAAAIAAAAAAAQAAAAAAIgICykffQAvGqi9Mu0X7e1rR
      aMzWE0g7mBoDpsF9wzxjWPQYWAru5VQAAIABAACAAAAAgAAAAAABAAAAAA==
```
```
    hwi --testnet -d <PATH> -f <FPR> signtx <walletprocesspsbt_out>
    hwi_signtx_out:
      {"error": "Output is not an address", "code": -7}
```
###  Without OP_RETURN: 
```
    createpsbt '[{"txid":"<TXID>","vout":<VOUT>}]' '[{"<ADDR>":"<AMT>"}]'
    createpsbt_out:
      cHNidP8BAGECAAAAAboQnxvwXjTOAQTJsRblDwgjYDaoxlzgJEaExQO7IVLtAQAA
      AAD/////AgAAAAAAAAAABmoE3q2+7yBOAAAAAAAAFgAUyfsuzJd8uUgvOw/oqAhC
      ZVXq1pgAAAAAAAAAAA==
```
```
    walletprocesspsbt '<createpsbt_out>'
    walletprocesspsbt_out: 
      cHNidP8BAFICAAAAAboQnxvwXjTOAQTJsRblDwgjYDaoxlzgJEaExQO7IVLtAQAA
      AAD/////ASBOAAAAAAAAFgAUyfsuzJd8uUgvOw/oqAhCZVXq1pgAAAAAAAEAcQIA
      AAAB/5wSVleWGJvGjrVXwiPOXKyiDiOt3a93Jp2LzQTZ7boBAAAAAP7///8CfRWd
      LQAAAAAWABQIN+uHqhhAL3oOdB9bLUgn2e8qpyBOAAAAAAAAFgAUyfsuzJd8uUgv
      Ow/oqAhCZVXq1ph9AxwAAQEfIE4AAAAAAAAWABTJ+y7Ml3y5SC87D+ioCEJlVerW
      mCIGAspH30ALxqovTLtF+3ta0WjM1hNIO5gaA6bBfcM8Y1j0GFgK7uVUAACAAQAA
      gAAAAIAAAAAAAQAAAAAiAgLKR99AC8aqL0y7Rft7WtFozNYTSDuYGgOmwX3DPGNY
      9BhYCu7lVAAAgAEAAIAAAACAAAAAAAEAAAAA
```
```
    hwi --testnet -d <PATH> -f <FPR> signtx <walletprocesspsbt_out>
    hwi_signtx_out:
      cHNidP8BAFICAAAAAboQnxvwXjTOAQTJsRblDwgjYDaoxlzgJEaExQO7IVLtAQAA
      AAD/////ASBOAAAAAAAAFgAUyfsuzJd8uUgvOw/oqAhCZVXq1pgAAAAAAAEAcQIA
      AAAB/5wSVleWGJvGjrVXwiPOXKyiDiOt3a93Jp2LzQTZ7boBAAAAAP7///8CfRWd
      LQAAAAAWABQIN+uHqhhAL3oOdB9bLUgn2e8qpyBOAAAAAAAAFgAUyfsuzJd8uUgv
      Ow/oqAhCZVXq1ph9AxwAAQEfIE4AAAAAAAAWABTJ+y7Ml3y5SC87D+ioCEJlVerW
      mCICAspH30ALxqovTLtF+3ta0WjM1hNIO5gaA6bBfcM8Y1j0SDBFAiEAii4omXSi
      lawaw5gvImdVJxCTb49ZGCdrFtjEUJNyQeACIDdQBV26bpqmxNAjxoaIrJ21qsY9
      9aQikOJzDFmw4LBaASIGAspH30ALxqovTLtF+3ta0WjM1hNIO5gaA6bBfcM8Y1j0
      GFgK7uVUAACAAQAAgAAAAIAAAAAAAQAAAAAiAgLKR99AC8aqL0y7Rft7WtFozNYT
      SDuYGgOmwX3DPGNY9BhYCu7lVAAAgAEAAIAAAACAAAAAAAEAAAAA
```

Changed files:
test/test_device.py, hwilib/devices/trezor.py, hwilib/serializations.py

diff --git a/hwilib/devices/trezor.py b/hwilib/devices/trezor.py
index 93df0b7..25898b3 100644
--- a/hwilib/devices/trezor.py
+++ b/hwilib/devices/trezor.py
@@ -328,6 +328,9 @@ class TrezorClient(HardwareWalletClient):
                     txoutput.address = to_address(out.scriptPubKey[3:23], p2pkh_version)
                 elif out.is_p2sh():
                     txoutput.address = to_address(out.scriptPubKey[2:22], p2sh_version)
+                elif out.is_opreturn():
+                    txoutput.script_type = proto.OutputScriptType.PAYTOOPRETURN
+                    txoutput.op_return_data = out.scriptPubKey[2:]
                 else:
                     wit, ver, prog = out.is_witness()
                     if wit:
diff --git a/hwilib/serializations.py b/hwilib/serializations.py
index e2a5f72..990f366 100644
--- a/hwilib/serializations.py
+++ b/hwilib/serializations.py
@@ -283,6 +283,9 @@ class CTxIn(object):
                self.nSequence)
 
 
+def is_opreturn(script: bytes) -> bool:
+    return script[0] == 0x6a
+
 def is_p2sh(script: bytes) -> bool:
     return len(script) == 23 and script[0] == 0xa9 and script[1] == 0x14 and script[22] == 0x87
 
@@ -336,6 +339,9 @@ class CTxOut(object):
         r += ser_string(self.scriptPubKey)
         return r
 
+    def is_opreturn(self) -> bool:
+        return is_opreturn(self.scriptPubKey)
+
     def is_p2sh(self) -> bool:
         return is_p2sh(self.scriptPubKey)
 
diff --git a/test/test_device.py b/test/test_device.py
index 5c5aff1..1276852 100644
--- a/test/test_device.py
+++ b/test/test_device.py
@@ -19,6 +19,10 @@ from hwilib.serializations import PSBT
 
 SUPPORTS_MS_DISPLAY = {'trezor_1', 'keepkey', 'coldcard', 'trezor_t'}
 SUPPORTS_XPUB_MS_DISPLAY = {'trezor_1', 'trezor_t'}
+SUPPORTS_MIXED = {'coldcard', 'trezor_1', 'digitalbitbox', 'keepkey', 'trezor_t'}
+SUPPORTS_MULTISIG = {'ledger', 'trezor_1', 'digitalbitbox', 'keepkey', 'coldcard', 'trezor_t'}
+SUPPORTS_EXTERNAL = {'ledger', 'trezor_1', 'digitalbitbox', 'keepkey', 'coldcard', 'trezor_t'}
+SUPPORTS_OP_RETURN = {'ledger', 'digitalbitbox', 'trezor_1', 'trezor_t', 'keepkey'}
 
 # Class for emulator control
 class DeviceEmulator():
@@ -354,7 +358,7 @@ class TestSignTx(DeviceTestCase):
 
         return sh_desc, sh_ms_info["address"], sh_wsh_desc, sh_wsh_ms_info["address"], wsh_desc, wsh_ms_info["address"]
 
-    def _test_signtx(self, input_type, multisig, external):
+    def _test_signtx(self, input_type, multisig, external, op_return: bool):
         # Import some keys to the watch only wallet and send coins to them
         keypool_desc = self.do_command(self.dev_args + ['getkeypool', '--all', '30', '50'])
         import_result = self.wrpc.importdescriptors(keypool_desc)
@@ -404,7 +408,14 @@ class TestSignTx(DeviceTestCase):
                 self.assertEqual((i + 1) * in_amt, self.wrpc.getbalance("*", 0, True))
                 change_addr = self.wpk_rpc.getrawchangeaddress()
             out_val = (i + 1) * out_amt
-            psbt = self.wrpc.walletcreatefundedpsbt([], [{self.wpk_rpc.getnewaddress('', 'legacy'): out_val}, {self.wpk_rpc.getnewaddress('', 'p2sh-segwit'): out_val}, {self.wpk_rpc.getnewaddress('', 'bech32'): out_val}], 0, {'includeWatching': True, "changePosition": 3, "changeAddress": change_addr}, True)
+            outputs = [
+                {self.wpk_rpc.getnewaddress('', 'legacy'): out_val},
+                {self.wpk_rpc.getnewaddress('', 'p2sh-segwit'): out_val},
+                {self.wpk_rpc.getnewaddress('', 'bech32'): out_val}
+            ]
+            if op_return:
+                outputs.append({"data": "000102030405060708090a0b0c0d0e0f10111213141516171819101a1b1c1d1e1f"})
+            psbt = self.wrpc.walletcreatefundedpsbt([], outputs, 0, {'includeWatching': True, "changePosition": 3, "changeAddress": change_addr}, True)
 
             if external:
                 # Sign with unknown inputs in two steps
@@ -417,18 +428,16 @@ class TestSignTx(DeviceTestCase):
 
     # Test wrapper to avoid mixed-inputs signing for Ledger
     def test_signtx(self):
-        supports_mixed = {'coldcard', 'trezor_1', 'digitalbitbox', 'keepkey', 'trezor_t'}
-        supports_multisig = {'ledger', 'trezor_1', 'digitalbitbox', 'keepkey', 'coldcard', 'trezor_t'}
-        supports_external = {'ledger', 'trezor_1', 'digitalbitbox', 'keepkey', 'coldcard', 'trezor_t'}
-        multisig = self.full_type in supports_multisig
-        external = self.full_type in supports_external
+        multisig = self.full_type in SUPPORTS_MULTISIG
+        external = self.full_type in SUPPORTS_EXTERNAL
+        op_return = self.full_type in SUPPORTS_OP_RETURN
         with self.subTest(addrtype="legacy", multisig=multisig, external=external):
-            self._test_signtx("legacy", multisig, external)
+            self._test_signtx("legacy", multisig, external, op_return)
         with self.subTest(addrtype="segwit", multisig=multisig, external=external):
-            self._test_signtx("segwit", multisig, external)
-        if self.full_type in supports_mixed:
+            self._test_signtx("segwit", multisig, external, op_return)
+        if self.full_type in SUPPORTS_MIXED:
             with self.subTest(addrtype="all", multisig=multisig, external=external):
-                self._test_signtx("all", multisig, external)
+                self._test_signtx("all", multisig, external, op_return)
 
     # Make a huge transaction which might cause some problems with different interfaces
     def test_big_tx(self):