From 4e2129e0a0bb53b581370dcb734e5208b034ef75 Mon Sep 17 00:00:00 2001 From: matejcik Date: Fri, 4 Mar 2022 15:33:33 +0100 Subject: [PATCH] feat(core): automatically choose best QR encoding --- core/.changelog.d/1751.changed | 1 + core/.changelog.d/1751.changed.1 | 1 + core/src/apps/binance/get_address.py | 2 +- core/src/apps/bitcoin/get_address.py | 15 +++++---- core/src/apps/ethereum/get_address.py | 2 +- core/src/apps/nem/get_address.py | 2 +- core/src/apps/ripple/get_address.py | 2 +- core/src/apps/stellar/get_address.py | 4 +-- core/src/apps/tezos/get_address.py | 2 +- core/src/trezor/ui/constants/tt.py | 4 +-- core/src/trezor/ui/layouts/tt/__init__.py | 8 +++-- core/src/trezor/ui/layouts/tt_v2/__init__.py | 2 ++ core/src/trezor/ui/qr.py | 33 ++++++++++++++++++-- tests/ui_tests/fixtures.json | 18 +++++------ 14 files changed, 66 insertions(+), 30 deletions(-) create mode 100644 core/.changelog.d/1751.changed create mode 100644 core/.changelog.d/1751.changed.1 diff --git a/core/.changelog.d/1751.changed b/core/.changelog.d/1751.changed new file mode 100644 index 0000000000..bf07aa4801 --- /dev/null +++ b/core/.changelog.d/1751.changed @@ -0,0 +1 @@ +Automatically choose best size and encoding for QR codes. diff --git a/core/.changelog.d/1751.changed.1 b/core/.changelog.d/1751.changed.1 new file mode 100644 index 0000000000..d0c1c088b3 --- /dev/null +++ b/core/.changelog.d/1751.changed.1 @@ -0,0 +1 @@ +Bitcoin bech32 addresses are encoded in lower-case for QR codes. diff --git a/core/src/apps/binance/get_address.py b/core/src/apps/binance/get_address.py index 910f3fa853..202e64863e 100644 --- a/core/src/apps/binance/get_address.py +++ b/core/src/apps/binance/get_address.py @@ -25,6 +25,6 @@ async def get_address( address = address_from_public_key(pubkey, HRP) if msg.show_display: title = paths.address_n_to_str(msg.address_n) - await show_address(ctx, address=address, address_qr=address, title=title) + await show_address(ctx, address=address, title=title) return BinanceAddress(address=address) diff --git a/core/src/apps/bitcoin/get_address.py b/core/src/apps/bitcoin/get_address.py index e706749f6a..26741c6995 100644 --- a/core/src/apps/bitcoin/get_address.py +++ b/core/src/apps/bitcoin/get_address.py @@ -55,15 +55,15 @@ async def get_address( address = addresses.get_address(msg.script_type, coin, node, msg.multisig) address_short = addresses.address_short(coin, address) + + address_case_sensitive = True if coin.segwit and msg.script_type in ( InputScriptType.SPENDWITNESS, InputScriptType.SPENDTAPROOT, ): - address_qr = address.upper() # bech32 address + address_case_sensitive = False # bech32 address elif coin.cashaddr_prefix is not None: - address_qr = address.upper() # cashaddr address - else: - address_qr = address # base58 address + address_case_sensitive = False # cashaddr address mac: bytes | None = None multisig_xpub_magic = coin.xpub_magic @@ -100,7 +100,7 @@ async def get_address( await show_address( ctx, address=address_short, - address_qr=address_qr, + case_sensitive=address_case_sensitive, title=title, multisig_index=multisig_index, xpubs=_get_xpubs(coin, multisig_xpub_magic, pubnodes), @@ -108,7 +108,10 @@ async def get_address( else: title = address_n_to_str(msg.address_n) await show_address( - ctx, address=address_short, address_qr=address_qr, title=title + ctx, + address=address_short, + case_sensitive=address_case_sensitive, + title=title, ) return Address(address=address, mac=mac) diff --git a/core/src/apps/ethereum/get_address.py b/core/src/apps/ethereum/get_address.py index 7fbb37794e..84147616fc 100644 --- a/core/src/apps/ethereum/get_address.py +++ b/core/src/apps/ethereum/get_address.py @@ -32,6 +32,6 @@ async def get_address( if msg.show_display: title = paths.address_n_to_str(msg.address_n) - await show_address(ctx, address=address, address_qr=address, title=title) + await show_address(ctx, address=address, title=title) return EthereumAddress(address=address) diff --git a/core/src/apps/nem/get_address.py b/core/src/apps/nem/get_address.py index 2b14ac9a80..e00f08c668 100644 --- a/core/src/apps/nem/get_address.py +++ b/core/src/apps/nem/get_address.py @@ -33,7 +33,7 @@ async def get_address( await show_address( ctx, address=address, - address_qr=address.upper(), + case_sensitive=False, title=title, network=get_network_str(msg.network), ) diff --git a/core/src/apps/ripple/get_address.py b/core/src/apps/ripple/get_address.py index 2d5bc6af1c..6fe0c401ec 100644 --- a/core/src/apps/ripple/get_address.py +++ b/core/src/apps/ripple/get_address.py @@ -25,6 +25,6 @@ async def get_address( if msg.show_display: title = paths.address_n_to_str(msg.address_n) - await show_address(ctx, address=address, address_qr=address, title=title) + await show_address(ctx, address=address, title=title) return RippleAddress(address=address) diff --git a/core/src/apps/stellar/get_address.py b/core/src/apps/stellar/get_address.py index ec4d4ce417..0cddaca1b9 100644 --- a/core/src/apps/stellar/get_address.py +++ b/core/src/apps/stellar/get_address.py @@ -25,8 +25,6 @@ async def get_address( if msg.show_display: title = paths.address_n_to_str(msg.address_n) - await show_address( - ctx, address=address, address_qr=address.upper(), title=title - ) + await show_address(ctx, address=address, case_sensitive=False, title=title) return StellarAddress(address=address) diff --git a/core/src/apps/tezos/get_address.py b/core/src/apps/tezos/get_address.py index 040318978b..1c0b22f09e 100644 --- a/core/src/apps/tezos/get_address.py +++ b/core/src/apps/tezos/get_address.py @@ -30,6 +30,6 @@ async def get_address( if msg.show_display: title = paths.address_n_to_str(msg.address_n) - await show_address(ctx, address=address, address_qr=address, title=title) + await show_address(ctx, address=address, title=title) return TezosAddress(address=address) diff --git a/core/src/trezor/ui/constants/tt.py b/core/src/trezor/ui/constants/tt.py index f36c72960a..1cad82ac3e 100644 --- a/core/src/trezor/ui/constants/tt.py +++ b/core/src/trezor/ui/constants/tt.py @@ -12,5 +12,5 @@ MONO_ADDR_PER_LINE = const(17) MONO_HEX_PER_LINE = const(18) QR_X = const(120) -QR_Y = const(115) -QR_SIZE_THRESHOLD = const(63) +QR_Y = const(112) +QR_SIDE_MAX = const(140) diff --git a/core/src/trezor/ui/layouts/tt/__init__.py b/core/src/trezor/ui/layouts/tt/__init__.py index 83dbef147e..12643939ad 100644 --- a/core/src/trezor/ui/layouts/tt/__init__.py +++ b/core/src/trezor/ui/layouts/tt/__init__.py @@ -31,7 +31,6 @@ from ...components.tt.text import LINE_WIDTH_PAGINATED, Span, Text from ...constants.tt import ( MONO_ADDR_PER_LINE, MONO_HEX_PER_LINE, - QR_SIZE_THRESHOLD, QR_X, QR_Y, TEXT_MAX_LINES, @@ -228,11 +227,11 @@ async def confirm_path_warning( def _show_qr( address: str, + case_sensitive: bool, title: str, cancel: str = "Address", ) -> Confirm: - QR_COEF = const(4) if len(address) < QR_SIZE_THRESHOLD else const(3) - qr = Qr(address, QR_X, QR_Y, QR_COEF) + qr = Qr(address, case_sensitive, QR_X, QR_Y) text = Text(title, ui.ICON_RECEIVE, ui.GREEN) return Confirm(Container(qr, text), cancel=cancel, cancel_style=ButtonDefault) @@ -315,7 +314,9 @@ async def show_xpub( async def show_address( ctx: wire.GenericContext, address: str, + *, address_qr: str | None = None, + case_sensitive: bool = True, title: str = "Confirm address", network: str | None = None, multisig_index: int | None = None, @@ -344,6 +345,7 @@ async def show_address( ctx, _show_qr( address if address_qr is None else address_qr, + case_sensitive, title if title_qr is None else title_qr, cancel="XPUBs" if is_multisig else "Address", ), diff --git a/core/src/trezor/ui/layouts/tt_v2/__init__.py b/core/src/trezor/ui/layouts/tt_v2/__init__.py index 6bf1562e6d..db64946c0b 100644 --- a/core/src/trezor/ui/layouts/tt_v2/__init__.py +++ b/core/src/trezor/ui/layouts/tt_v2/__init__.py @@ -134,6 +134,8 @@ async def show_xpub( async def show_address( ctx: wire.GenericContext, address: str, + *, + case_sensitive: bool = True, address_qr: str | None = None, title: str = "Confirm address", network: str | None = None, diff --git a/core/src/trezor/ui/qr.py b/core/src/trezor/ui/qr.py index 85be58d5a4..e81e6f91bd 100644 --- a/core/src/trezor/ui/qr.py +++ b/core/src/trezor/ui/qr.py @@ -1,13 +1,42 @@ +from typing import Sequence + from trezor import ui +from trezor.ui.constants import QR_SIDE_MAX + +_QR_WIDTHS = (21, 25, 29, 33, 37, 41, 45, 49, 53, 57) +_THRESHOLDS_BINARY = (14, 26, 42, 62, 84, 106, 122, 152, 180, 213) +_THRESHOLDS_ALPHANUM = (20, 38, 61, 90, 122, 154, 178, 221, 262, 311) + + +def _qr_version_index(data: str, thresholds: Sequence[int]) -> int: + for i, threshold in enumerate(thresholds): + if len(data) <= threshold: + return i + raise ValueError # data too long + + +def is_alphanum_only(data: str) -> bool: + return all(c in "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ $*+-./:" for c in data) class Qr(ui.Component): - def __init__(self, data: str, x: int, y: int, scale: int): + def __init__(self, data: str, case_sensitive: bool, x: int, y: int) -> None: super().__init__() + if case_sensitive and not is_alphanum_only(data): + # must encode in BINARY mode + version_idx = _qr_version_index(data, _THRESHOLDS_BINARY) + else: + # can make the QR code more readable by using the best version + version_idx = _qr_version_index(data, _THRESHOLDS_ALPHANUM) + if len(data) > _THRESHOLDS_BINARY[version_idx]: + data = data.upper() + + size = _QR_WIDTHS[version_idx] + self.data = data self.x = x self.y = y - self.scale = scale + self.scale = QR_SIDE_MAX // size def on_render(self) -> None: if self.repaint: diff --git a/tests/ui_tests/fixtures.json b/tests/ui_tests/fixtures.json index 91bb794172..5981a3e7aa 100644 --- a/tests/ui_tests/fixtures.json +++ b/tests/ui_tests/fixtures.json @@ -701,17 +701,17 @@ "TT_bitcoin-test_getaddress_segwit_native.py::test_show_segwit[Testnet-m-86h-1h-0h-0-0-InputScr-821a199d": "c09de07fbbf1e047442180e2facb5482d06a1a428891b875b7dd93c9e4704ae1", "TT_bitcoin-test_getaddress_segwit_native.py::test_show_segwit[Testnet-m-86h-1h-0h-1-0-InputScr-9d2fa8bc": "c09de07fbbf1e047442180e2facb5482d06a1a428891b875b7dd93c9e4704ae1", "TT_bitcoin-test_getaddress_segwit_native.py::test_show_segwit[Testnet-m-86h-1h-0h-1-0-InputScr-d5b7f8fc": "4460b43883e6049f719050ee015b01f6dd528c1aa51b96ae97c60230f56bbc7e", -"TT_bitcoin-test_getaddress_show.py::test_show[m-44h-0h-12h-0-0-InputScriptType.SPENDADDRESS-1F-1e4f2f74": "1639a90cd9deccc25478b736c1992bd3f1ef17db95ab7e0ea82451dff0ebb12d", -"TT_bitcoin-test_getaddress_show.py::test_show[m-49h-0h-12h-0-0-InputScriptType.SPENDP2SHWITNES-a986211d": "a2b3af26697700980024d2d9ba1145925d9c3c9dea40f6cc3d4546ea94e6e913", -"TT_bitcoin-test_getaddress_show.py::test_show[m-84h-0h-12h-0-0-InputScriptType.SPENDWITNESS-bc-a5f08dfb": "c3258bfb300e27bb54aec51eaf68008e087191b97794ddb8a1e6ee650fd8a6b2", +"TT_bitcoin-test_getaddress_show.py::test_show[m-44h-0h-12h-0-0-InputScriptType.SPENDADDRESS-1F-1e4f2f74": "f2f31c8493549ddff876e5222b262063a39839c3884ec84d8d1890cc52a0f036", +"TT_bitcoin-test_getaddress_show.py::test_show[m-49h-0h-12h-0-0-InputScriptType.SPENDP2SHWITNES-a986211d": "38ebd99ae3ae7d8f30ce77acb153f6d6637a3a81c86c7e5d038c3e1628ee2509", +"TT_bitcoin-test_getaddress_show.py::test_show[m-84h-0h-12h-0-0-InputScriptType.SPENDWITNESS-bc-a5f08dfb": "48c1738b62a2a04036aa2a521240e192ea025bb98fd6dd2f038bcf2d5af1a214", "TT_bitcoin-test_getaddress_show.py::test_show_multisig_15": "1a656989a6144461d99ea5aae33bb1683c8eafb95eaf5523d21f736df6861043", "TT_bitcoin-test_getaddress_show.py::test_show_multisig_3": "c305f05b829e446d327e69e75ededde3bb76c9cc9f39f5a0f50788874bebd372", -"TT_bitcoin-test_getaddress_show.py::test_show_multisig_xpubs[InputScriptType.SPENDMULTISIG-0-3-4efd9cf3": "67f2450d6655510cc5f4ce1790af851a4565d4ff54c817738b9e33994ff2843d", -"TT_bitcoin-test_getaddress_show.py::test_show_multisig_xpubs[InputScriptType.SPENDMULTISIG-0-3-98a7e339": "67f2450d6655510cc5f4ce1790af851a4565d4ff54c817738b9e33994ff2843d", -"TT_bitcoin-test_getaddress_show.py::test_show_multisig_xpubs[InputScriptType.SPENDP2SHWITNESS--2cf5f03c": "59e21947978d42aa97b9a076e4994b23784a244713441e0d10e18711b1be1eae", -"TT_bitcoin-test_getaddress_show.py::test_show_multisig_xpubs[InputScriptType.SPENDP2SHWITNESS--5ea18367": "9997e053d6f88b966da59eb7c67bdb47015094d7a7ecfadf8d71476a21a3c4c5", -"TT_bitcoin-test_getaddress_show.py::test_show_multisig_xpubs[InputScriptType.SPENDWITNESS-2-bc-e70b56ea": "947c8c6a4ae8b5519dc4e9939496ba579288bbb5334625d44bf4f307ebde4ea9", -"TT_bitcoin-test_getaddress_show.py::test_show_multisig_xpubs[InputScriptType.SPENDWITNESS-2-bc-f3c4650f": "224e62bbc3546a18cc033d7d61d63ae2ed0750a7d8cd9bfd3d0deed18d42de62", +"TT_bitcoin-test_getaddress_show.py::test_show_multisig_xpubs[InputScriptType.SPENDMULTISIG-0-3-4efd9cf3": "bec64e8a3c7079d97ad5b487914a0da9c6fb541714785772dd67ea89f86a040a", +"TT_bitcoin-test_getaddress_show.py::test_show_multisig_xpubs[InputScriptType.SPENDMULTISIG-0-3-98a7e339": "bec64e8a3c7079d97ad5b487914a0da9c6fb541714785772dd67ea89f86a040a", +"TT_bitcoin-test_getaddress_show.py::test_show_multisig_xpubs[InputScriptType.SPENDP2SHWITNESS--2cf5f03c": "cc8a98fb9dd08f15fa27ff0b74b1b147576ffe57e3c56d802e33f0fa0a4c3752", +"TT_bitcoin-test_getaddress_show.py::test_show_multisig_xpubs[InputScriptType.SPENDP2SHWITNESS--5ea18367": "cd7d53102ee2482e4014c602767550cc3f2a9c1c2833e908a857e7d093b7b3d6", +"TT_bitcoin-test_getaddress_show.py::test_show_multisig_xpubs[InputScriptType.SPENDWITNESS-2-bc-e70b56ea": "ed9eafcc09412c67de9f086ac79ec33a9b3335b2b2dec721965c75f0bea87dae", +"TT_bitcoin-test_getaddress_show.py::test_show_multisig_xpubs[InputScriptType.SPENDWITNESS-2-bc-f3c4650f": "9ea4ef4f47d568081aed5f03a43a92e307933ed6d05f274cb2961a565b9d7ec7", "TT_bitcoin-test_getaddress_show.py::test_show_unrecognized_path": "c09de07fbbf1e047442180e2facb5482d06a1a428891b875b7dd93c9e4704ae1", "TT_bitcoin-test_getownershipproof.py::test_attack_ownership_id": "c09de07fbbf1e047442180e2facb5482d06a1a428891b875b7dd93c9e4704ae1", "TT_bitcoin-test_getownershipproof.py::test_confirm_ownership_proof": "a79fd462b99f5ffc755917b6d06d3c5a3061c971649cdfa752cbc9ef068e6acf",