allow unknown scripts in HSM; add support for OP_RETURN

releases/ChangeLog-mk4.md

@@ -4,6 +4,8 @@
   Only applies to cases where partial signatures are being added.
   Thanks to [@straylight-orbit](https://github.com/straylight-orbit)
 - Bugfix: order of multisig wallet registration does NOT matter.
+- Bugfix: allow unknown scripts in HSM mode
+- Enhancement: OP_RETURN is now a known script and is displayed in ascii if possible
 
 ## 5.0.4 - 2022-05-27
 

shared/auth.py

@@ -384,6 +384,14 @@ class ApproveTransaction(UserAuthorizedAction):
             return '%s\n - to address -\n%s\n' % (val, dest)
         except ValueError:
             pass
+        # check for OP_RETURN
+        data = self.chain.op_return(o.scriptPubKey)
+        if data:
+            data_hex, data_ascii = data
+            to_ret = '%s\n - OP_RETURN -\n%s' % (val, data_hex)
+            if data_ascii:
+                return to_ret + " (ascii: %s)\n" % data_ascii
+            return to_ret + "\n"
 
         # Handle future things better: allow them to happen at least.
         self.psbt.warnings.append(

shared/chains.py

@@ -4,11 +4,12 @@
 #
 import ngu
 from uhashlib import sha256
+from ubinascii import hexlify as b2a_hex
 from public_constants import AF_CLASSIC, AF_P2SH, AF_P2WPKH, AF_P2WSH, AF_P2WPKH_P2SH, AF_P2WSH_P2SH
 from public_constants import AFC_PUBKEY, AFC_SEGWIT, AFC_BECH32, AFC_SCRIPT, AFC_WRAPPED
-from serializations import hash160, ser_compact_size
+from serializations import hash160, ser_compact_size, disassemble
 from ucollections import namedtuple
-from opcodes import OP_CHECKMULTISIG
+from opcodes import OP_CHECKMULTISIG, OP_RETURN
 
 # See SLIP 132 <https://github.com/satoshilabs/slips/blob/master/slip-0132.md>
 # for background on these version bytes. Not to be confused with SLIP-32 which involves Bech32.
@@ -207,6 +208,23 @@ class ChainsBase:
 
         raise ValueError('Unknown payment script', repr(script))
 
+    @classmethod
+    def op_return(cls, script):
+        """Returns decoded string op return data if script is op return otherwise None"""
+        gen = disassemble(script)
+        script_type = next(gen)
+        if OP_RETURN in script_type:
+            data = next(gen)[0]
+            data_hex = b2a_hex(data).decode()
+            data_ascii = None
+            if min(data) >= 32 and max(data) < 127:  # printable
+                try:
+                    data_ascii = data.decode("ascii")
+                except:
+                    pass
+            return data_hex, data_ascii
+        return None
+
 class BitcoinMain(ChainsBase):
     # see <https://github.com/bitcoin/bitcoin/blob/master/src/chainparams.cpp#L140>
     ctype = 'BTC'

shared/hsm.py

@@ -759,7 +759,10 @@ class HSMPolicy:
                 for idx, tx_out in psbt.output_iter():
                     if not psbt.outputs[idx].is_change:
                         total_out += tx_out.nValue
-                        dests.append(chain.render_address(tx_out.scriptPubKey))
+                        try:
+                            dests.append(chain.render_address(tx_out.scriptPubKey))
+                        except ValueError:
+                            dests.append(str(b2a_hex(tx_out.scriptPubKey), 'ascii'))
 
                 # Pick a rule to apply to this specific txn
                 reasons = []

shared/opcodes.py

@@ -32,7 +32,7 @@ OP_16 = const(96)
 #OP_ELSE = const(103)
 #OP_ENDIF = const(104)
 #OP_VERIFY = const(105)
-#OP_RETURN = const(106)
+OP_RETURN = const(106)
 #OP_TOALTSTACK = const(107)
 #OP_FROMALTSTACK = const(108)
 #OP_2DROP = const(109)

testing/test_hsm.py

@@ -9,7 +9,8 @@
 # - command line: py.test test_hsm.py --dev -s --ff
 # - no microSD card installed
 #
-import pytest, time, struct, os, itertools
+
+import pytest, time, struct, os, itertools, base64
 from binascii import b2a_hex, a2b_hex
 from hashlib import sha256
 from ckcc_protocol.protocol import MAX_MSG_LEN, CCProtocolPacker, CCProtoError
@@ -1127,7 +1128,7 @@ def test_boot_to_hsm_unlock(start_hsm, hsm_status, enter_local_code):
     enter_local_code('123123')
     time.sleep(.5)
     assert hsm_status().active == False
-    assert hsm_status().policy_available == True  # we haven't removed anythong why shoudl it be not available?
+    assert hsm_status().policy_available == True  # we haven't removed anything why should it be not available?
 
 def test_boot_to_hsm_too_late(dev, start_hsm, hsm_status, enter_local_code):
     if dev.is_simulator:
@@ -1166,7 +1167,23 @@ def test_priv_over_ux(quick_start_hsm, hsm_status, load_hsm_users):
     s = quick_start_hsm(dict(priv_over_ux=False))
     assert all((f in s) for f in flds)
 
-
+@pytest.mark.bitcoind
+@pytest.mark.parametrize("op_return_data", [
+    b"Coldcard is the best signing device",  # to test with both pushdata opcodes
+    b"Coldcard, the best signing deviceaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",  # len 80 max
+])
+def test_op_return_output(op_return_data, start_hsm, attempt_psbt, bitcoind_d_sim_watch, bitcoind, hsm_reset):
+    cc = bitcoind_d_sim_watch
+    dest_address = cc.getnewaddress()
+    bitcoind.supply_wallet.generatetoaddress(101, dest_address)
+    psbt = cc.walletcreatefundedpsbt([], [{dest_address: 1.0}, {"data": op_return_data.hex()}], 0, {"fee_rate": 20})["psbt"]
+    policy = DICT(rules=[dict(max_amount=10)])
+    start_hsm(policy)
+    attempt_psbt(base64.b64decode(psbt))
+    policy = DICT(rules=[dict(whitelist=['131CnJGaDyPaJsb5P4NHFxcRi29zo3ZXw'])])
+    hsm_reset()
+    start_hsm(policy)
+    attempt_psbt(base64.b64decode(psbt), refuse="non-whitelisted address: 6a")  # 6a --> OP_RETURN
 
 # KEEP LAST -- can only be run once, will crash device
 @pytest.mark.onetime

testing/test_sign.py

@@ -2,8 +2,8 @@
 #
 # Transaction Signing. Important.
 #
-import base64
-import time, pytest, os, random, pdb, struct
+
+import time, pytest, os, random, pdb, struct, base64, binascii
 from ckcc_protocol.protocol import CCProtocolPacker, CCProtoError, MAX_TXN_LEN, CCUserRefused
 from binascii import b2a_hex, a2b_hex
 from psbt import BasicPSBT, BasicPSBTInput, BasicPSBTOutput, PSBT_IN_REDEEM_SCRIPT
@@ -1680,4 +1680,37 @@ def test_bitcoind_missing_foreign_utxo(bitcoind, bitcoind_d_sim_watch, microsd_p
     tx_id = alice.sendrawtransaction(tx)
     assert isinstance(tx_id, str) and len(tx_id) == 64
 
+@pytest.mark.bitcoind
+@pytest.mark.parametrize("op_return_data", [
+    b"Coldcard is the best signing device",  # to test with both pushdata opcodes
+    b"Coldcard, the best signing deviceaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",  # len 80 max
+    b"\x80" * 80,
+    "££££££££££".encode("utf-8"),
+    bytes.fromhex("aa21a9ed68512d3c6b0514b18fbc9f0c540d5bec8f4ae62da4bf6c9b16f90b655f9f4210"),
+    b"$$$$$$$$$$$$$$ Bitcoin",
+    b"\xeb\x97\xf7\xb7\xf78\x9a';\x90F_\xfc\xe2b\xa4\x93)\xea\xac\xacR\xff\x9c\xbe\x1c\xf1\xad\xe9!\xee\xd9t1\x1f\x92\x83\x97\xb3\x98/\xff\xc8\xff\xc1\xc0\xdd\x1et\x00L\x13\xe0\xe3\x90\xe4\xd4\xf2x:\xf7Ab\x04\x91\x1e\xa8R\x92\xd3\x96OK\xc6I\x06\x9e\xce=\xb3",
+])
+def test_op_return_signing(op_return_data, dev, fake_txn, bitcoind_d_sim_watch, bitcoind, start_sign, end_sign, cap_story):
+    cc = bitcoind_d_sim_watch
+    dest_address = cc.getnewaddress()
+    bitcoind.supply_wallet.generatetoaddress(101, dest_address)
+    psbt = cc.walletcreatefundedpsbt([], [{dest_address: 1.0}, {"data": op_return_data.hex()}], 0, {"fee_rate": 20})["psbt"]
+    start_sign(base64.b64decode(psbt))
+    time.sleep(.1)
+    title, story = cap_story()
+    assert title == "OK TO SEND?"
+    # in older implementations, one would seen a warning for OP_RETURN --> not now
+    assert "warning" not in story
+    assert "OP_RETURN" in story
+    try:
+        expect = op_return_data.decode("ascii")
+    except:
+        expect = binascii.hexlify(op_return_data).decode()
+    assert expect in story
+    signed = end_sign(accept=True)
+    tx = cc.finalizepsbt(base64.b64encode(signed).decode())["hex"]
+    assert cc.testmempoolaccept([tx])[0]["allowed"] is True
+    tx_id = cc.sendrawtransaction(tx)
+    assert isinstance(tx_id, str) and len(tx_id) == 64
+
 # EOF