bugfix: p2pk

This commit is contained in:
scgbckbone 2026-05-09 15:08:06 +02:00 committed by doc-hex
parent 59eb529a20
commit 0d04e5e1f8
8 changed files with 268 additions and 32 deletions

View File

@ -13,6 +13,7 @@ This lists the new changes that have not yet been published in a normal release.
When both UTXO fields are present the full non_witness_utxo is now preferred for amount/script lookup. Thanks, @Damir
- Bugfix: Emit warning and do not calculate fee for legacy UTXOs with only witness utxo
- Bugfix: Disable Virtual Disk and NFC before activating HSM
- Bugfix: P2PK signing was broken. Now supports both compressed and uncompressed P2PK spend
- Bugfix: Custom address default menu position wrong
- Bugfix: Delta Mode Trick PIN was never restored from backup
- Bugfix: Proper error message for incorrect 7z headers

View File

@ -651,7 +651,8 @@ class ApproveTransaction(UserAuthorizedAction):
has_change = True
total_change += tx_out.nValue
if len(largest_change) < MAX_VISIBLE_CHANGE:
largest_change.append((tx_out.nValue, self.chain.render_address(tx_out.scriptPubKey)))
_, addr = self.render_output(tx_out)
largest_change.append((tx_out.nValue, addr))
if len(largest_change) == MAX_VISIBLE_CHANGE:
largest_change = sorted(largest_change, key=lambda x: x[0], reverse=True)
continue
@ -676,12 +677,9 @@ class ApproveTransaction(UserAuthorizedAction):
continue # too small
largest.pop(-1)
if outp.is_change:
ret = (here, self.chain.render_address(tx_out.scriptPubKey))
else:
rendered, _ = self.render_output(tx_out)
ret = (here, rendered)
largest.insert(keep, ret)
rendered, dest = self.render_output(tx_out)
largest.insert(keep, (here, dest if outp.is_change else rendered))
# foreign outputs (soon to be other people's coins)
visible_out_sum = 0
@ -1701,9 +1699,11 @@ class TXInpExplorer(TXExplorer):
item += "=== UTXO ===\n\n%s %s\n\n%s\n\n" % (val, unit, spk)
if addr:
item += show_single_address(addr) + "\n\n"
item += "Address Format: %s\n\n" % chains.addr_fmt_str(inp.addr_fmt)
qr_items.append(addr)
if inp.addr_fmt is not None:
item += "Address Format: %s\n\n" % chains.addr_fmt_str(inp.addr_fmt)
if self.user_auth_action.psbt.txn_version >= 2:
has_rtl = inp.has_relative_timelock(txin)
if has_rtl:

View File

@ -5,7 +5,7 @@
import ngu
from uhashlib import sha256
from ubinascii import hexlify as b2a_hex
from public_constants import AF_CLASSIC, AF_P2WPKH, AF_P2TR
from public_constants import AF_BARE_PK, AF_CLASSIC, AF_P2WPKH, AF_P2TR
from public_constants import AF_P2SH, AF_P2WSH, AF_P2WPKH_P2SH, AF_P2WSH_P2SH
from public_constants import AFC_PUBKEY, AFC_SEGWIT, AFC_BECH32, AFC_SCRIPT
from block_height import BLOCK_HEIGHT
@ -450,6 +450,7 @@ def addr_fmt_label(addr_fmt):
def addr_fmt_str(addr_fmt):
# Short string codes used for address format (industry standard)
return {AF_CLASSIC: "p2pkh",
AF_BARE_PK: "p2pk",
AF_P2SH: "p2sh",
AF_P2TR: "p2tr",
AF_P2WPKH: "p2wpkh",

View File

@ -440,7 +440,7 @@ class psbtOutputProxy(psbtProxy):
if af == AF_BARE_PK:
# output is public key (not a hash, much less common)
assert len(addr_or_pubkey) == 33
assert len(addr_or_pubkey) in (33, 65) # compressed or uncompressed
if addr_or_pubkey != expect_pubkey:
raise FraudulentChangeOutput(out_idx, "P2PK change output is fraudulent")
@ -885,7 +885,7 @@ class psbtInputProxy(psbtProxy):
elif self.addr_fmt == AF_BARE_PK:
# input is single public key (less common)
self.scriptSig = utxo.scriptPubKey
assert len(addr_or_pubkey) == 33
assert len(addr_or_pubkey) in (33, 65) # compressed or uncompressed
if addr_or_pubkey in subpaths:
which_key = addr_or_pubkey
@ -2084,7 +2084,13 @@ class psbtObject(psbtProxy):
node = sv.derive_path(skp)
# check the pubkey of this BIP-32 node
if target_pk == node.pubkey():
pu = node.pubkey() # always 33-byte compressed
if len(target_pk) == 65:
# P2PK with uncompressed pubkey: re-serialize node's pubkey in
# uncompressed form for direct comparison.
pu = ngu.secp256k1.pubkey(pu).to_bytes(True)
if target_pk == pu:
return node
return None
@ -2218,14 +2224,9 @@ class psbtObject(psbtProxy):
else:
assert which_key in inp.subpaths, 'unk key'
# get node required
skp = keypath_to_str(inp.subpaths[which_key])
node = sv.derive_path(skp, register=False)
# expensive test, but works... and important
pu = node.pubkey()
assert pu == which_key, \
"Path (%s) led to wrong pubkey for input#%d"%(skp, in_idx)
node = self.check_pubkey_at_path(sv, inp.subpaths[which_key], which_key)
assert node, "Path (%s) led to wrong pubkey for input#%d" % (
keypath_to_str(inp.subpaths[which_key]), in_idx)
# track wallet usage
@ -2588,7 +2589,12 @@ class psbtObject(psbtProxy):
else:
pubkey, der_sig = ssig
txi.scriptSig = ser_push_data(der_sig) + ser_push_data(pubkey)
if inp.addr_fmt == AF_BARE_PK:
# P2PK: pubkey is already in scriptPubKey, scriptSig is just <sig>
txi.scriptSig = ser_push_data(der_sig)
else:
# P2PKH: scriptSig is <sig> <pubkey>
txi.scriptSig = ser_push_data(der_sig) + ser_push_data(pubkey)
fd.write(txi.serialize())

View File

@ -376,8 +376,10 @@ class CTxOut(object):
return AF_P2SH, self.scriptPubKey[2:2+20], False
if self.is_p2pk():
# rare, pay to full pubkey
return AF_BARE_PK, self.scriptPubKey[2:2+33], False
# rare, pay to full pubkey: <push_op> <pubkey> OP_CHECKSIG
# push_op is 0x21 (33) for compressed, 0x41 (65) for uncompressed
pk_len = self.scriptPubKey[0]
return AF_BARE_PK, self.scriptPubKey[1:1+pk_len], False
if self.is_op_return():
return OP_RETURN, self.scriptPubKey, False
@ -407,8 +409,8 @@ class CTxOut(object):
def is_p2pk(self):
return (len(self.scriptPubKey) == 35 or len(self.scriptPubKey) == 67) \
and (self.scriptPubKey[0] == 0x21 or self.scriptPubKey[0] == 0x41) \
and self.scriptPubKey[-1] == 0xac
and self.scriptPubKey[0] == len(self.scriptPubKey) - 2 \
and self.scriptPubKey[-1] == OP_CHECKSIG
def is_op_return(self):
return self.scriptPubKey and (self.scriptPubKey[0] == OP_RETURN)

View File

@ -50,6 +50,14 @@ def fake_dest_addr(style='p2pkh'):
if style == 'p2pkh':
return bytes([0x76, 0xa9, 0x14]) + prandom(20) + bytes([0x88, 0xac])
if style in ('p2pk', 'p2pk-compressed'):
pubkey = bytes([random.choice((2, 3))]) + prandom(32)
return bytes([len(pubkey)]) + pubkey + bytes([0xac])
if style == 'p2pk-uncompressed':
pubkey = bytes([4]) + prandom(64)
return bytes([len(pubkey)]) + pubkey + bytes([0xac])
if style == "p2tr":
return bytes([81, 32]) + prandom(32)
@ -58,8 +66,6 @@ def fake_dest_addr(style='p2pkh'):
hex_str = "049f7b2a5cb17576a914371c20fb2e9899338ce5e99908e64fd30b78931388ac"
return bytes.fromhex(hex_str)
# missing: if style == 'p2pk' => pay to pubkey, considered obsolete
raise ValueError('not supported: ' + style)
def make_change_addr(wallet, style):
@ -77,9 +83,13 @@ def make_change_addr(wallet, style):
assert len(target) == 20
is_segwit = True
pubkey = dest.sec(compressed=(style != 'p2pk-uncompressed'))
if style == 'p2pkh':
redeem_scr = bytes([0x76, 0xa9, 0x14]) + target + bytes([0x88, 0xac])
is_segwit = False
elif style in ('p2pk', 'p2pk-compressed', 'p2pk-uncompressed'):
redeem_scr = bytes([len(pubkey)]) + pubkey + bytes([0xac])
is_segwit = False
elif style == 'p2wpkh':
redeem_scr = bytes([0, 20]) + target
elif style == 'p2wpkh-p2sh':
@ -92,7 +102,7 @@ def make_change_addr(wallet, style):
else:
raise ValueError('cant make fake change output of type: ' + style)
return redeem_scr, actual_scr, is_segwit, dest.sec(), struct.pack('4I', xfp, *deriv)
return redeem_scr, actual_scr, is_segwit, pubkey, struct.pack('4I', xfp, *deriv)
def swab32(n):
# endian swap: 32 bits

View File

@ -3932,4 +3932,207 @@ def test_empty_input_scriptPubKey(segwit_in, dev, fake_txn, start_sign, cap_stor
title, _ = cap_story()
assert title == "Failure"
@pytest.mark.bitcoind
@pytest.mark.parametrize('finalize', [True, False])
@pytest.mark.parametrize('mode', ['compressed', 'uncompressed'])
def test_spend_p2pk(mode, finalize, bitcoind, bitcoind_d_wallet, dev,
start_sign, end_sign, cap_story, use_regtest, goto_home):
use_regtest()
goto_home()
xfp_str = xfp2str(dev.master_fingerprint).lower()
path = "44h/1h/0h/0/0"
xpub = dev.send_recv(CCProtocolPacker.get_xpub("m/" + path), timeout=5000)
node = BIP32Node.from_wallet_key(xpub)
if mode == 'compressed':
pubkey_hex = node.node.public_key.sec(compressed=True).hex()
else:
pubkey_hex = node.node.public_key.sec(compressed=False).hex()
# pk() descriptor with origin info — Core writes the (xfp, path) into the
# PSBT's BIP32_DERIVATION so Coldcard recognizes it as its own key.
desc = "pk([%s/%s]%s)" % (xfp_str, path, pubkey_hex)
desc = bitcoind.rpc.getdescriptorinfo(desc)["descriptor"]
res = bitcoind_d_wallet.importdescriptors([{
"desc": desc, "timestamp": "now", "watchonly": True,
}])
assert res[0]["success"], res
# Fund the P2PK output. No Core RPC accepts a raw scriptPubKey as an
# output, so we hand-build a skeleton tx with the P2PK output and let
# fundrawtransaction fill in inputs, change, and fee from supply_wallet.
push_op = b'\x21' if mode == 'compressed' else b'\x41'
spk = push_op + bytes.fromhex(pubkey_hex) + b'\xac'
txn = CTransaction()
txn.vout = [CTxOut(5_00_000_000, spk)] # 5 BTC
funded = bitcoind.supply_wallet.fundrawtransaction(txn.serialize().hex())
signed = bitcoind.supply_wallet.signrawtransactionwithwallet(funded["hex"])
bitcoind.rpc.sendrawtransaction(signed["hex"])
bitcoind.supply_wallet.generatetoaddress(1, bitcoind.supply_wallet.getnewaddress())
assert bitcoind_d_wallet.listunspent()
dest = bitcoind.supply_wallet.getnewaddress()
resp = bitcoind_d_wallet.walletcreatefundedpsbt(
[], [{dest: bitcoind_d_wallet.getbalance()}], 0,
{"fee_rate": 3, "subtractFeeFromOutputs": [0]})
psbt_bytes = base64.b64decode(resp["psbt"])
# Sanity: confirm Core encoded the pubkey form we asked for in the PSBT's
# BIP32_DERIVATION (key field). 33 bytes for compressed, 65 for uncompressed.
# Single-sig P2PK has exactly one entry; `all` rejects any extra entries
# in unexpected form.
expect_pk_len = 33 if mode == 'compressed' else 65
pre_po = BasicPSBT().parse(psbt_bytes)
pre_keys = list(pre_po.inputs[0].bip32_paths.keys())
assert pre_keys
assert all(len(k) == expect_pk_len for k in pre_keys)
start_sign(psbt_bytes, finalize=finalize)
time.sleep(.1)
title, story = cap_story()
assert title == "OK TO SEND?"
out = end_sign(accept=True, finalize=finalize)
if finalize:
# Coldcard returned the network tx; broadcast directly.
tx_hex = out.hex()
else:
# Coldcard returned a partially-signed PSBT. Verify the partial sig
# carries the same-form pubkey as keydata, then have bitcoind finalize.
signed_po = BasicPSBT().parse(out)
sig_keys = list(signed_po.inputs[0].part_sigs.keys())
assert sig_keys
assert all(len(k) == expect_pk_len for k in sig_keys)
finalized = bitcoind.rpc.finalizepsbt(base64.b64encode(out).decode())
assert finalized["complete"], finalized
tx_hex = finalized["hex"]
accept = bitcoind.rpc.testmempoolaccept([tx_hex])
assert accept[0]["allowed"], accept
txid = bitcoind.rpc.sendrawtransaction(tx_hex)
assert len(txid) == 64
goto_home()
@pytest.mark.parametrize('finalize', [True, False])
@pytest.mark.parametrize('mode', ['compressed', 'uncompressed'])
def test_fake_txn_spend_p2pk(mode, finalize, fake_txn, start_sign, end_sign, cap_story):
expect_pk_len = 33 if mode == 'compressed' else 65
style = 'p2pk' if mode == 'compressed' else 'p2pk-uncompressed'
psbt = fake_txn(1, 2, p2pk_in=mode, outstyles=[style, 'p2pkh'], change_outputs=[0])
pre = BasicPSBT().parse(psbt)
change_keys = list(pre.outputs[0].bip32_paths.keys())
assert change_keys and all(len(k) == expect_pk_len for k in change_keys)
start_sign(psbt, finalize=finalize)
time.sleep(.1)
title, story = cap_story()
assert title == "OK TO SEND?"
assert "Change back:" in story
out = end_sign(accept=True, finalize=finalize)
if not finalize:
signed = BasicPSBT().parse(out)
sig_keys = list(signed.inputs[0].part_sigs.keys())
assert sig_keys and all(len(k) == expect_pk_len for k in sig_keys)
@pytest.mark.parametrize('mode', ['compressed', 'uncompressed'])
@pytest.mark.parametrize('change', [True, False])
def test_p2pk_change_output_renders(mode, change, fake_txn, start_sign, cap_story, dev,
goto_home, need_keypress, pick_menu_item, press_cancel):
master_xpub = dev.master_xpub or simulator_fixed_tprv
mk = BIP32Node.from_wallet_key(master_xpub)
xfp = mk.fingerprint()
input_leaf = mk.subkey_for_path("0/0")
input_pk = input_leaf.node.public_key.sec(compressed=(mode == 'compressed'))
input_spk = bytes([len(input_pk)]) + input_pk + b'\xac'
leaf = mk.subkey_for_path("0/77")
if mode == 'compressed':
pk = leaf.node.public_key.sec(compressed=True)
push_op = b'\x21'
else:
pk = leaf.node.public_key.sec(compressed=False)
push_op = b'\x41'
assert len(pk) == (33 if mode == 'compressed' else 65)
p2pk_spk = push_op + pk + b'\xac' # <push> <pubkey> OP_CHECKSIG
def hack(psbt):
t = CTransaction()
t.deserialize(BytesIO(psbt.txn))
t.vout[0].scriptPubKey = p2pk_spk
psbt.txn = t.serialize_with_witness()
if change:
psbt.outputs[0].bip32_paths = {pk: xfp + struct.pack('<II', 0, 77)}
outvals = [1_000_000, 98_990_000]
psbt = fake_txn(1, 2, p2pk_in=mode, psbt_v2=False, psbt_hacker=hack,
outvals=outvals)
goto_home()
start_sign(psbt)
time.sleep(.1)
title, story = cap_story()
assert title == "OK TO SEND?" # approval screen built, no crash
assert ("Change back:" in story) == change
if change:
assert p2pk_spk.hex() in story.replace(" ", "").replace("\n", "")
need_keypress("2")
pick_menu_item("Outputs")
time.sleep(.1)
_, story = cap_story()
assert (f"Output 0 (change):" if change else "Output 0:") in story
assert p2pk_spk.hex() in story.replace(" ", "").replace("\n", "")
press_cancel()
time.sleep(.1)
pick_menu_item("Inputs")
time.sleep(.1)
title, story = cap_story()
assert title == "Input 0"
stripped = story.replace(" ", "").replace("\n", "")
assert input_spk.hex() in stripped
assert input_pk.hex() in stripped
for _ in range(3):
press_cancel()
def test_malformed_p2pk_change_output(fake_txn, start_sign, cap_story, end_sign, dev):
master_xpub = dev.master_xpub or simulator_fixed_tprv
mk = BIP32Node.from_wallet_key(master_xpub)
xfp = mk.fingerprint()
leaf = mk.subkey_for_path("0/77")
pk = leaf.node.public_key.sec(compressed=True)
malformed_p2pk = b'\x21' + pk + os.urandom(32) + b'\xac'
assert len(malformed_p2pk) == 67
def hack(psbt):
t = CTransaction()
t.deserialize(BytesIO(psbt.txn))
t.vout[0].scriptPubKey = malformed_p2pk
psbt.txn = t.serialize_with_witness()
psbt.outputs[0].bip32_paths = {pk: xfp + struct.pack('<II', 0, 77)}
psbt = fake_txn(1, 2, segwit_in=True, psbt_v2=False, psbt_hacker=hack)
start_sign(psbt)
time.sleep(.1)
title, story = cap_story()
assert title == "OK TO SEND?"
assert "to script" in story
assert "Sending to 1 not well understood script(s)" in story
assert "Change back:" not in story
end_sign(accept=True)
# EOF

View File

@ -25,7 +25,7 @@ def fake_txn(dev, pytestconfig):
outstyles=['p2pkh'], psbt_hacker=None, change_outputs=[],
capture_scripts=None, add_xpub=None, op_return=None, taproot_in=False,
psbt_v2=None, input_amount=1E8, unknown_out_script=None, lock_time=0,
sequences=None, sighashes=None, dupe_ins=[]):
sequences=None, sighashes=None, dupe_ins=[], p2pk_in=False):
psbt = BasicPSBT()
@ -66,9 +66,15 @@ def fake_txn(dev, pytestconfig):
bytes_path = b""
else:
subkey = mk.subkey_for_path(subpath % i)
sec = subkey.sec()
assert len(sec) == 33, "expect compressed"
if p2pk_in:
assert not segwit_in and not taproot_in and not wrapped
assert p2pk_in in (True, 'compressed', 'uncompressed')
sec = subkey.sec(compressed=(p2pk_in != 'uncompressed'))
else:
sec = subkey.sec()
assert len(sec) == 33, "expect compressed"
assert subpath[0:2] == '0/'
# TODO does not respect subpath parameter
bytes_path = struct.pack('<II', 0, i)
@ -83,7 +89,10 @@ def fake_txn(dev, pytestconfig):
)
supply.vin = [CTxIn(out_point, nSequence=0xffffffff)]
if segwit_in:
if p2pk_in:
# p2pk: <push pubkey> OP_CHECKSIG
scr = bytes([len(sec)]) + sec + b'\xac'
elif segwit_in:
# p2wpkh
scr = bytes([0x00, 0x14]) + subkey.hash160()
if wrapped:
@ -259,6 +268,10 @@ def render_address(script, testnet=True):
b58_script = bytes([196])
b58_privkey = bytes([239])
if ll in (35, 67) and script[0] == (ll - 2) and script[-1] == 0xac:
# does not have address format - just show raw scriptPubKey
return b2a_hex(script).decode()
# P2PKH
if ll == 25 and script[0:3] == b'\x76\xA9\x14' and script[23:26] == b'\x88\xAC':
return encode_base58_checksum(b58_addr + script[3:3+20])