From ce2feb5f1de7758dacd1d91dbc42b275d51d6636 Mon Sep 17 00:00:00 2001 From: scgbckbone Date: Fri, 17 Jun 2022 14:35:00 +0200 Subject: [PATCH] allow unknown scripts in HSM; add support for OP_RETURN --- releases/ChangeLog-mk4.md | 2 ++ shared/auth.py | 8 ++++++++ shared/chains.py | 22 ++++++++++++++++++++-- shared/hsm.py | 5 ++++- shared/opcodes.py | 2 +- testing/test_hsm.py | 23 ++++++++++++++++++++--- testing/test_sign.py | 37 +++++++++++++++++++++++++++++++++++-- 7 files changed, 90 insertions(+), 9 deletions(-) diff --git a/releases/ChangeLog-mk4.md b/releases/ChangeLog-mk4.md index 62595576..b4ed8a08 100644 --- a/releases/ChangeLog-mk4.md +++ b/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 diff --git a/shared/auth.py b/shared/auth.py index 443916b5..bf73abbe 100644 --- a/shared/auth.py +++ b/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( diff --git a/shared/chains.py b/shared/chains.py index 15267477..22ee87f3 100644 --- a/shared/chains.py +++ b/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 # 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 ctype = 'BTC' diff --git a/shared/hsm.py b/shared/hsm.py index 3ddbe45b..50c3fb00 100644 --- a/shared/hsm.py +++ b/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 = [] diff --git a/shared/opcodes.py b/shared/opcodes.py index 8e3600e1..d015d173 100644 --- a/shared/opcodes.py +++ b/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) diff --git a/testing/test_hsm.py b/testing/test_hsm.py index ddfb12ea..bcebf612 100644 --- a/testing/test_hsm.py +++ b/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 diff --git a/testing/test_sign.py b/testing/test_sign.py index 681d52a2..9e3d4035 100644 --- a/testing/test_sign.py +++ b/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