From 0237fd29bac97040d10ad21900dea723fda5c023 Mon Sep 17 00:00:00 2001 From: scgbckbone Date: Thu, 5 Jun 2025 15:26:35 +0200 Subject: [PATCH] txout explorer do not yikes on big QRs --- shared/auth.py | 47 ++++++++++++++++--------- shared/display.py | 23 ++++++++++-- shared/exceptions.py | 4 +++ shared/export.py | 5 +-- shared/lcd_display.py | 82 +++++++++++++++++++++++++++++-------------- shared/qrs.py | 31 +++++++++------- testing/conftest.py | 6 +--- testing/test_sign.py | 51 ++++++++++++++++++++------- 8 files changed, 172 insertions(+), 77 deletions(-) diff --git a/shared/auth.py b/shared/auth.py index c074ca2d..412c5391 100644 --- a/shared/auth.py +++ b/shared/auth.py @@ -17,7 +17,7 @@ from usb import CCBusyError from utils import HexWriter, xfp2str, problem_file_line, cleanup_deriv_path, B2A, show_single_address from psbt import psbtObject, FatalPSBTIssue, FraudulentChangeOutput from files import CardSlot, CardMissingError -from exceptions import HSMDenied +from exceptions import HSMDenied, QRTooBigError from version import MAX_TXN_LEN from charcodes import KEY_QR, KEY_NFC, KEY_ENTER, KEY_CANCEL, KEY_LEFT, KEY_RIGHT from msgsign import sign_message_digest @@ -287,42 +287,52 @@ class ApproveTransaction(UserAuthorizedAction): # Pretty-print a transactions output. # - expects CTxOut object # - gives user-visible string + # returns: tuple(ux_output_rendition, address_or_script_str_for_qr_display) # val = ' '.join(self.chain.render_value(o.nValue)) try: dest = self.chain.render_address(o.scriptPubKey) - + # known script types are short enough that we can display QR on both hw versions return '%s\n - to address -\n%s\n' % (val, show_single_address(dest)), dest except ValueError: pass + # Handle future things better: allow them to happen at least. + # sending to some unknown script, possibly very long + # but full-show required for verification + # OP_RETURN dest contains also OP_RETURN itself (for PSBT qr explorer) + dest = B2A(o.scriptPubKey) + # check for OP_RETURN data = self.chain.op_return(o.scriptPubKey) - if data is not None: + # In UX story only data are shown as OP_RETURN is part of base msg + if data is None: + rv = '%s\n - to script -\n%s\n' % (val, dest) + else: base = '%s\n - OP_RETURN -\n%s' if not data: - return base % (val, "null-data\n"), "" + dest = "" + rv = base % (val, "null-data\n") else: data_ascii = None - if len(data) > 200: + if len(data) > 160: # completely arbitrary limit, prevents huge stories - data_hex = b2a_hex(data[:100]).decode() + "\n ⋯\n" + b2a_hex(data[-100:]).decode() + # anchor data are not relevant for verification - can be hidden + ss = b2a_hex(data[:80]).decode() + "\n ⋯\n" + b2a_hex(data[-80:]).decode() + # but we show empty QR in txn explorer for these big, modified data else: - data_hex = b2a_hex(data).decode() + ss = b2a_hex(data).decode() if (min(data) >= 32) and (max(data) < 127): # printable & not huge try: data_ascii = data.decode("ascii") except: pass - to_ret = base % (val, data_hex) + rv = base % (val, ss) if data_ascii: - to_ret += " (ascii: %s)" % data_ascii - return to_ret + "\n", data_hex + rv += " (ascii: %s)" % data_ascii + rv += "\n" - # Handle future things better: allow them to happen at least. - dest = B2A(o.scriptPubKey) - - return '%s\n - to script -\n%s\n' % (val, dest), dest + return rv, dest async def interact(self): # Prompt user w/ details and get approval @@ -605,7 +615,10 @@ class ApproveTransaction(UserAuthorizedAction): return elif ch in "4"+KEY_QR: from ux import show_qr_codes - await show_qr_codes(addrs, False, start, is_addrs=True, change_idxs=change) + # showing addresses from PSBT, no idea what is in there + # handle QR code failures gracefully + await show_qr_codes(addrs, False, start, is_addrs=True, + change_idxs=change, can_raise=False) continue elif (ch in KEY_LEFT+"7"): if (start - n) < 0: @@ -873,10 +886,10 @@ async def done_signing(psbt, tx_req, input_method=None, filename=None, try: if len(here) > 920: # too big for simple QR - use BBQr instead - raise ValueError + raise QRTooBigError hex_here = b2a_hex(here).upper().decode() await show_qr_code(hex_here, is_alnum=True, msg=msg) - except (ValueError, RuntimeError, TypeError): + except QRTooBigError: from ux_q1 import show_bbqr_codes await show_bbqr_codes('T' if txid else 'P', here, msg) diff --git a/shared/display.py b/shared/display.py index 53a394e1..cd1edcdd 100644 --- a/shared/display.py +++ b/shared/display.py @@ -332,15 +332,25 @@ class Display: # no status bar on Mk4 return + def draw_qr_error(self, idx_hint, msg): + self.clear() + lm = 4 + bw = 54 + y = (self.HEIGHT - bw) // 2 + # empty rectangle + self.dis.fill_rect(lm, y, bw, bw, 1) + self.dis.fill_rect(lm+1, y+1, bw-2, bw-2, 0) + # error in rectangle - handpicked position + self.text(lm+5,y+10, "QR too") + self.text(lm+16,y+24, "big") + self._draw_qr_display(bw, lm, msg, False, None, idx_hint, False) + def draw_qr_display(self, qr_data, msg, is_alnum, sidebar, idx_hint, invert, is_addr=False, force_msg=False, is_change=False): # 'sidebar' is a pre-formated obj to show to right of QR -- oled life # - 'msg' will appear to right if very short, else under in tiny # - ignores "is_addr" because exactly zero space to do anything special - from utils import word_wrap - self.clear() - w = qr_data.width() if w <= 29: # version 1,2,3 => we can double-up the pixels @@ -380,6 +390,13 @@ class Display: gly = framebuf.FrameBuffer(bytearray(packed), w, w, framebuf.MONO_HLSB) self.dis.blit(gly, XO, YO, 1) + self._draw_qr_display(bw, lm, msg, is_alnum, sidebar, idx_hint, invert, is_addr, is_change) + + def _draw_qr_display(self, bw, lm, msg, is_alnum, sidebar, idx_hint, invert, + is_addr=False, is_change=False): + # does not draw actual QR, but all other things in the screen + from utils import word_wrap + if not sidebar and not msg: pass elif not sidebar and ((len(msg) > (5*7)) or is_change): diff --git a/shared/exceptions.py b/shared/exceptions.py index e701d78b..6f84242d 100644 --- a/shared/exceptions.py +++ b/shared/exceptions.py @@ -55,4 +55,8 @@ class UnknownAddressExplained(ValueError): class SpendPolicyViolation(RuntimeError): pass +# data too big for simple QR +class QRTooBigError(ValueError): + pass + # EOF diff --git a/shared/export.py b/shared/export.py index 320b45dc..90c5a4c2 100644 --- a/shared/export.py +++ b/shared/export.py @@ -12,6 +12,7 @@ from msgsign import write_sig_file from public_constants import AF_CLASSIC, AF_P2WPKH, AF_P2WPKH_P2SH, AF_P2WSH, AF_P2WSH_P2SH, AF_P2SH from charcodes import KEY_NFC, KEY_CANCEL, KEY_QR from ownership import OWNERSHIP +from exceptions import QRTooBigError async def export_by_qr(body, label, type_code, force_bbqr=False): # render as QR and show on-screen @@ -19,10 +20,10 @@ async def export_by_qr(body, label, type_code, force_bbqr=False): try: if force_bbqr or len(body) > 2000: - raise ValueError + raise QRTooBigError await show_qr_code(body) - except (ValueError, RuntimeError, TypeError): + except QRTooBigError: if version.has_qwerty: # do BBQr on Q from ux_q1 import show_bbqr_codes diff --git a/shared/lcd_display.py b/shared/lcd_display.py index 8c94d828..918a0431 100644 --- a/shared/lcd_display.py +++ b/shared/lcd_display.py @@ -656,6 +656,59 @@ class Display: return prev_x + @staticmethod + def handle_qr_msg(msg, max_lines=False): + if len(msg) <= CHARS_W: + parts = [msg] + elif ' ' not in msg and (len(msg) <= (CHARS_W * 2)): + # fits in two lines, but has no spaces + hh = len(msg) // 2 + parts = [msg[0:hh], msg[hh:]] + else: + if not max_lines: + # do word wrap + parts = list(word_wrap(msg, CHARS_W)) + else: + # 2 lines max + parts = [msg[:30] + "⋯", "⋯" + msg[-30:]] + + return parts + + def draw_qr_lines(self, lines, is_addr): + y = CHARS_H - len(lines) + prev_x = 0 + for line in lines: + if not is_addr: + self.text(None, y, line) + else: + prev_x = self._draw_addr(y, line, prev_x=prev_x) + y += 1 + + def draw_qr_idx_hint(self, str_idx): + lh = len(str_idx) + assert lh <= 10 + if lh > 5: + # needs 2 lines + self.text(-1, 0, str_idx[:5]) + self.text(-1, 1, str_idx[5:]) + else: + self.text(-1, 0, str_idx) + + def draw_qr_error(self, idx_hint, msg=None): + x = 85 + y = 30 + w = 150 + self.clear() + self.dis.fill_rect(x, y, w, w, COL_TEXT) + self.dis.fill_rect(x + 1, y + 1, w - 2, w - 2) # Black + self.text(12, 3, "QR too big") + if msg: + lines = self.handle_qr_msg(msg, max_lines=True) + self.draw_qr_lines(lines, False) + + self.draw_qr_idx_hint(idx_hint) + self.show() + def draw_qr_display(self, qr_data, msg, is_alnum, sidebar, idx_hint, invert, partial_bar=None, is_addr=False, force_msg=False, is_change=False): # Show a QR code on screen w/ some text under it @@ -677,16 +730,7 @@ class Display: # p2wsh address would need 3 lines to show, so we won't num_lines = 0 elif msg: - if len(msg) <= CHARS_W: - parts = [msg] - elif ' ' not in msg and (len(msg) <= CHARS_W*2): - # fits in two lines, but has no spaces - hh = len(msg) // 2 - parts = [msg[0:hh], msg[hh:]] - else: - # do word wrap - parts = list(word_wrap(msg, CHARS_W)) - + parts = self.handle_qr_msg(msg) num_lines = len(parts) else: num_lines = 0 @@ -755,24 +799,10 @@ class Display: if num_lines: # centered text under that - y = CHARS_H - num_lines - prev_x = 0 - for line in parts: - if not is_addr: - self.text(None, y, line) - else: - prev_x = self._draw_addr(y, line, prev_x=prev_x) - y += 1 + self.draw_qr_lines(parts, is_addr) if idx_hint: - lh = len(idx_hint) - assert lh <= 10 - if lh > 5: - # needs 2 lines - self.text(-1, 0, idx_hint[:5]) - self.text(-1, 1, idx_hint[5:]) - else: - self.text(-1, 0, idx_hint) + self.draw_qr_idx_hint(idx_hint) if is_addr and is_change: for i, c in enumerate("CHANGE", start=4): diff --git a/shared/qrs.py b/shared/qrs.py index ae70f59d..6afd5e2b 100644 --- a/shared/qrs.py +++ b/shared/qrs.py @@ -5,6 +5,7 @@ import framebuf, uqr from ux import UserInteraction, ux_wait_keyup, the_ux from version import has_qwerty +from exceptions import QRTooBigError from charcodes import (KEY_LEFT, KEY_RIGHT, KEY_UP, KEY_DOWN, KEY_HOME, KEY_NFC, KEY_END, KEY_ENTER, KEY_CANCEL) @@ -18,7 +19,7 @@ class QRDisplaySingle(UserInteraction): def __init__(self, addrs, is_alnum, start_n=0, sidebar=None, msg=None, is_addrs=False, force_msg=False, allow_nfc=True, is_secret=False, - change_idxs=None): + change_idxs=None, can_raise=True): self.is_alnum = is_alnum self.idx = 0 # start with first address self.invert = False # looks better, but neither mode is ideal @@ -33,6 +34,7 @@ class QRDisplaySingle(UserInteraction): # only used for NFC sharing secret material - full chip wipe if is_secret=True self.is_secret = is_secret self.change_idxs = change_idxs or [] + self.can_raise = can_raise def calc_qr(self, msg): # Version 2 would be nice, but can't hold what we need, even at min error correction, @@ -75,6 +77,15 @@ class QRDisplaySingle(UserInteraction): # what we are showing inside the QR body = self.addrs[self.idx] + idx_hint = self.idx_hint() + + msg = None + if self.msg: + msg = self.msg + else: + if isinstance(body, str): + # sanity check + msg = body # make the QR, if needed. if not self.qr_data: @@ -83,21 +94,17 @@ class QRDisplaySingle(UserInteraction): self.calc_qr(body) except Exception: dis.busy_bar(False) - raise + if not self.can_raise: + dis.draw_qr_error(idx_hint, msg) + return + + # other code paths require raise to switch to BBQr + raise QRTooBigError # draw display dis.busy_bar(False) - - if self.msg: - msg = self.msg - else: - msg = None - if isinstance(body, str): - # sanity check - msg = body - dis.draw_qr_display(self.qr_data, msg, self.is_alnum, - self.sidebar, self.idx_hint(), self.invert, + self.sidebar, idx_hint, self.invert, is_addr=self.is_addrs, force_msg=self.force_msg, is_change=self.is_change()) diff --git a/testing/conftest.py b/testing/conftest.py index 1916b221..3ea4d489 100644 --- a/testing/conftest.py +++ b/testing/conftest.py @@ -637,11 +637,7 @@ def verify_qr_address(cap_screen_qr, cap_screen, is_q1): for c, line in zip("XXXXXXBACK", full_split): assert not line.endswith(c) - txt = ''.join(l for l in full_split if len(l)>4).replace('~', '') - if txt: - # just index remained - int(txt) - txt = None + txt = None else: if is_change: assert "CHANGE BACK" in full diff --git a/testing/test_sign.py b/testing/test_sign.py index 3f595af0..915fdc11 100644 --- a/testing/test_sign.py +++ b/testing/test_sign.py @@ -3097,12 +3097,14 @@ def test_txout_explorer(chain, data, fake_txn, start_sign, settings_set, txout_e @pytest.mark.parametrize("finalize", [True, False]) @pytest.mark.parametrize("data", [ [(1, b"Coinkite"), (0, b"Mk1 Mk2 Mk3 Mk4 Q"), (100, b"binarywatch.org"), (100, b"a" * 75)], + [(0, b"W"*160), (10000, b"W"*153)], [(0, b"a" * 300), (10, b"x" * 1000), (0, b"anchor output")], [(0, b""), (10, b"")], + [(0, os.urandom(32)), (10, os.urandom(64)), (1000, os.urandom(160)), (0, os.urandom(161))], ]) def test_txout_explorer_op_return(finalize, data, fake_txn, start_sign, cap_story, is_q1, need_keypress, press_cancel, press_select, end_sign, - cap_screen_qr): + cap_screen_qr, cap_screen): psbt = fake_txn(1, 20, segwit_in=False, op_return=data) start_sign(psbt, finalize=finalize) time.sleep(.1) @@ -3134,9 +3136,23 @@ def test_txout_explorer_op_return(finalize, data, fake_txn, start_sign, cap_stor # collect QR codes first need_keypress(KEY_QR if is_q1 else "4") qr_list = [] - for _ in range(len(data)): - qr = cap_screen_qr().decode() - qr_list.append(qr) + for v, d in data: + try: + qr = cap_screen_qr().decode() + qr_list.append(qr) + except RuntimeError: + scr = cap_screen() + if is_q1: + too_big = 650 + assert "QR too big" in scr + else: + too_big = 158 + assert "QR too" in scr + assert "big" in scr + + assert len(d) > too_big + qr_list.append(None) + need_keypress(KEY_RIGHT if is_q1 else "9") time.sleep(.5) @@ -3154,17 +3170,28 @@ def test_txout_explorer_op_return(finalize, data, fake_txn, start_sign, cap_stor if dd == "null-data": assert qr_list[i - 20] == "" elif dd: - hex_str, ascii_str = dd.split(" ", 1) - assert hex_str == qr_list[i-20] - assert f"(ascii: {data.decode()})" == ascii_str - assert data.hex() == hex_str + is_ascii = False + try: + data.decode("ascii") + is_ascii = True + except UnicodeDecodeError: pass + if is_ascii: + hex_str, ascii_str = dd.split(" ", 1) + else: + hex_str = dd + + qr_target = qr_list[i-20] + if qr_target: + assert hex_str in qr_target + assert qr_target.startswith("6a") # OP_RETURN + assert data.hex() == hex_str + if is_ascii: + assert f"(ascii: {data.decode()})" == ascii_str else: - s = data[:100].hex() - e = data[-100:].hex() + s = data[:80].hex() + e = data[-80:].hex() assert s == dd0 assert e == dd1 - qr = qr_list[i - 20] - assert qr == "" press_cancel() # exit txn out explorer end_sign(finalize=finalize)