From 3882b89be96571803a2b7f7f23203b78f9695915 Mon Sep 17 00:00:00 2001 From: Martin Milata Date: Thu, 14 Oct 2021 23:52:58 +0200 Subject: [PATCH] fix(core/ethereum): ask before showing paginated data field --- core/.changelog.d/1819.fixed | 1 + core/src/apps/ethereum/layout.py | 1 + .../trezor/ui/components/common/confirm.py | 9 +- core/src/trezor/ui/components/tt/scroll.py | 61 ++++++++-- core/src/trezor/ui/layouts/tt/__init__.py | 74 +++++++++++- tests/device_tests/ethereum/test_signtx.py | 112 +++++++++++++++++- tests/ui_tests/fixtures.json | 15 ++- 7 files changed, 249 insertions(+), 24 deletions(-) create mode 100644 core/.changelog.d/1819.fixed diff --git a/core/.changelog.d/1819.fixed b/core/.changelog.d/1819.fixed new file mode 100644 index 000000000..7c0c85777 --- /dev/null +++ b/core/.changelog.d/1819.fixed @@ -0,0 +1 @@ +Ethereum: make it optional to view the entire data field when signing transaction. diff --git a/core/src/apps/ethereum/layout.py b/core/src/apps/ethereum/layout.py index 37fa2e4d2..1c30ef265 100644 --- a/core/src/apps/ethereum/layout.py +++ b/core/src/apps/ethereum/layout.py @@ -103,6 +103,7 @@ def require_confirm_data(ctx: Context, data: bytes, data_total: int) -> Awaitabl description=f"Size: {data_total} bytes", data=data, br_code=ButtonRequestType.SignTx, + ask_pagination=True, ) diff --git a/core/src/trezor/ui/components/common/confirm.py b/core/src/trezor/ui/components/common/confirm.py index 5d841554c..2c1a893c5 100644 --- a/core/src/trezor/ui/components/common/confirm.py +++ b/core/src/trezor/ui/components/common/confirm.py @@ -1,21 +1,26 @@ from trezor import loop, ui, wire if False: - from typing import Callable, Any, Awaitable + from typing import Callable, Any, Awaitable, TypeVar + + T = TypeVar("T") CONFIRMED = object() CANCELLED = object() INFO = object() +GO_BACK = object() +SHOW_PAGINATED = object() def is_confirmed(x: Any) -> bool: return x is CONFIRMED -async def raise_if_cancelled(a: Awaitable, exc: Any = wire.ActionCancelled) -> None: +async def raise_if_cancelled(a: Awaitable[T], exc: Any = wire.ActionCancelled) -> T: result = await a if result is CANCELLED: raise exc + return result async def is_confirmed_info( diff --git a/core/src/trezor/ui/components/tt/scroll.py b/core/src/trezor/ui/components/tt/scroll.py index 832e87207..38fb493a2 100644 --- a/core/src/trezor/ui/components/tt/scroll.py +++ b/core/src/trezor/ui/components/tt/scroll.py @@ -4,8 +4,9 @@ from trezor import loop, res, ui, utils, wire, workflow from trezor.enums import ButtonRequestType from trezor.messages import ButtonAck, ButtonRequest +from ..common.confirm import CANCELLED, CONFIRMED, GO_BACK, SHOW_PAGINATED from .button import Button, ButtonCancel, ButtonConfirm, ButtonDefault -from .confirm import CANCELLED, CONFIRMED, Confirm +from .confirm import Confirm from .swipe import SWIPE_DOWN, SWIPE_UP, SWIPE_VERTICAL, Swipe from .text import ( LINE_WIDTH_PAGINATED, @@ -47,7 +48,7 @@ def render_scrollbar(pages: int, page: int) -> None: ui.display.bar_radius(X, Y + i * padding, SIZE, SIZE, fg, ui.BG, 4) -def render_swipe_icon() -> None: +def render_swipe_icon(x_offset: int = 0) -> None: if utils.DISABLE_ANIMATION: c = ui.GREY else: @@ -56,32 +57,46 @@ def render_swipe_icon() -> None: c = ui.blend(ui.GREY, ui.DARK_GREY, t) icon = res.load(ui.ICON_SWIPE) - ui.display.icon(70, 205, icon, c, ui.BG) + ui.display.icon(70 + x_offset, 205, icon, c, ui.BG) -def render_swipe_text() -> None: - ui.display.text_center(130, 220, "Swipe", ui.BOLD, ui.GREY, ui.BG) +def render_swipe_text(x_offset: int = 0) -> None: + ui.display.text_center(130 + x_offset, 220, "Swipe", ui.BOLD, ui.GREY, ui.BG) class Paginated(ui.Layout): - def __init__(self, pages: list[ui.Component], page: int = 0): + def __init__( + self, pages: list[ui.Component], page: int = 0, back_button: bool = False + ): super().__init__() self.pages = pages self.page = page + self.back_button = None + if back_button: + area = ui.grid(16, n_x=4) + icon = res.load(ui.ICON_BACK) + self.back_button = Button(area, icon, ButtonDefault) + self.back_button.on_click = self.on_back_click # type: ignore def dispatch(self, event: int, x: int, y: int) -> None: pages = self.pages page = self.page + length = len(pages) + last_page = page >= length - 1 + x_offset = 0 + pages[page].dispatch(event, x, y) + if self.back_button is not None and not last_page: + self.back_button.dispatch(event, x, y) + x_offset = 30 if event is ui.REPAINT: self.repaint = True elif event is ui.RENDER: - length = len(pages) - if page < length - 1: - render_swipe_icon() + if not last_page: + render_swipe_icon(x_offset=x_offset) if self.repaint: - render_swipe_text() + render_swipe_text(x_offset=x_offset) if self.repaint: render_scrollbar(length, page) self.repaint = False @@ -143,12 +158,35 @@ class Paginated(ui.Layout): def on_change(self) -> None: pass + def on_back_click(self) -> None: + raise ui.Result(GO_BACK) + if __debug__: def read_content(self) -> list[str]: return self.pages[self.page].read_content() +class AskPaginated(ui.Component): + def __init__(self, content: ui.Component) -> None: + super().__init__() + self.content = content + self.button = Button(ui.grid(3, n_x=1), "Show all", ButtonDefault) + self.button.on_click = self.on_show_paginated_click # type: ignore + + def dispatch(self, event: int, x: int, y: int) -> None: + self.content.dispatch(event, x, y) + self.button.dispatch(event, x, y) + + def on_show_paginated_click(self) -> None: + raise ui.Result(SHOW_PAGINATED) + + if __debug__: + + def read_content(self) -> list[str]: + return self.content.read_content() + + class PageWithButtons(ui.Component): def __init__( self, @@ -321,6 +359,7 @@ def paginate_paragraphs( icon_color: int = ui.ORANGE_ICON, break_words: bool = False, confirm: Callable[[ui.Component], ui.Layout] = Confirm, + back_button: bool = False, ) -> ui.Layout: span = Span("", 0, ui.NORMAL, break_words=break_words) lines = 0 @@ -391,4 +430,4 @@ def paginate_paragraphs( content_ctr += 1 pages[-1] = confirm(pages[-1]) - return Paginated(pages) + return Paginated(pages, back_button=back_button) diff --git a/core/src/trezor/ui/layouts/tt/__init__.py b/core/src/trezor/ui/layouts/tt/__init__.py index 55ca58f9e..73de45a37 100644 --- a/core/src/trezor/ui/layouts/tt/__init__.py +++ b/core/src/trezor/ui/layouts/tt/__init__.py @@ -10,12 +10,19 @@ from trezor.ui.qr import Qr from trezor.utils import chunks, chunks_intersperse from ...components.common import break_path_to_lines -from ...components.common.confirm import is_confirmed, raise_if_cancelled +from ...components.common.confirm import ( + CONFIRMED, + GO_BACK, + SHOW_PAGINATED, + is_confirmed, + raise_if_cancelled, +) from ...components.tt import passphrase, pin from ...components.tt.button import ButtonCancel, ButtonDefault from ...components.tt.confirm import Confirm, HoldToConfirm from ...components.tt.scroll import ( PAGEBREAK, + AskPaginated, Paginated, paginate_paragraphs, paginate_text, @@ -32,7 +39,7 @@ from ...constants.tt import ( from ..common import button_request, interact if False: - from typing import Awaitable, Iterator, NoReturn, Sequence + from typing import Awaitable, Iterable, Iterator, NoReturn, Sequence from ..common import PropertyType, ExceptionType @@ -503,6 +510,50 @@ async def confirm_output( await raise_if_cancelled(interact(ctx, content, "confirm_output", br_code)) +async def _confirm_ask_pagination( + ctx: wire.GenericContext, + br_type: str, + title: str, + para: Iterable[tuple[int, str]], + para_truncated: Iterable[tuple[int, str]], + br_code: ButtonRequestType, + icon: str, + icon_color: int, +) -> None: + paginated: ui.Layout | None = None + + truncated = Text( + title, + header_icon=icon, + icon_color=icon_color, + new_lines=False, + max_lines=TEXT_MAX_LINES - 2, + ) + for font, text in para_truncated: + truncated.content.extend((font, text, "\n")) + ask_dialog = Confirm(AskPaginated(truncated)) + + while True: + result = await raise_if_cancelled(interact(ctx, ask_dialog, br_type, br_code)) + if result is CONFIRMED: + return + assert result is SHOW_PAGINATED + + if paginated is None: + paginated = paginate_paragraphs( + para, + header=None, + back_button=True, + confirm=lambda content: Confirm( + content, cancel=None, confirm="Close", confirm_style=ButtonDefault + ), + ) + result = await interact(ctx, paginated, br_type, br_code) + assert result in (CONFIRMED, GO_BACK) + + assert False + + async def confirm_blob( ctx: wire.GenericContext, br_type: str, @@ -512,6 +563,7 @@ async def confirm_blob( br_code: ButtonRequestType = ButtonRequestType.Other, icon: str = ui.ICON_SEND, # TODO cleanup @ redesign icon_color: int = ui.GREEN, # TODO cleanup @ redesign + ask_pagination: bool = False, ) -> None: """Confirm data blob. @@ -556,14 +608,28 @@ async def confirm_blob( per_line = MONO_HEX_PER_LINE text.mono(ui.FG, *chunks_intersperse(data_str, per_line)) content: ui.Layout = Confirm(text) + return await raise_if_cancelled(interact(ctx, content, br_type, br_code)) + + elif ask_pagination: + para = [(ui.MONO, line) for line in chunks(data_str, MONO_HEX_PER_LINE - 2)] + + para_truncated = [] + if description is not None: + para_truncated.append((ui.NORMAL, description)) + para_truncated.extend(para[:TEXT_MAX_LINES]) + + return await _confirm_ask_pagination( + ctx, br_type, title, para, para_truncated, br_code, icon, icon_color + ) else: para = [] if description is not None: para.append((ui.NORMAL, description)) para.extend((ui.MONO, line) for line in chunks(data_str, MONO_HEX_PER_LINE - 2)) - content = paginate_paragraphs(para, title, icon, icon_color) - await raise_if_cancelled(interact(ctx, content, br_type, br_code)) + + paginated = paginate_paragraphs(para, title, icon, icon_color) + return await raise_if_cancelled(interact(ctx, paginated, br_type, br_code)) def confirm_address( diff --git a/tests/device_tests/ethereum/test_signtx.py b/tests/device_tests/ethereum/test_signtx.py index 68973f443..94f121754 100644 --- a/tests/device_tests/ethereum/test_signtx.py +++ b/tests/device_tests/ethereum/test_signtx.py @@ -16,7 +16,7 @@ import pytest -from trezorlib import ethereum, messages +from trezorlib import ethereum, exceptions, messages from trezorlib.debuglink import message_filters from trezorlib.exceptions import TrezorFailure from trezorlib.tools import parse_path @@ -24,6 +24,8 @@ from trezorlib.tools import parse_path from ...common import parametrize_using_common_fixtures TO_ADDR = "0x1d1c328764a41bda0492b66baa30c4a339ff85ef" +SHOW_ALL = (143, 167) +GO_BACK = (16, 220) pytestmark = [pytest.mark.altcoin, pytest.mark.ethereum] @@ -325,3 +327,111 @@ def test_sanity_checks_eip1559(client): max_gas_fee=20, max_priority_fee=1, ) + + +def input_flow_skip(client, cancel=False): + yield # confirm sending + client.debug.press_yes() + + yield # confirm data + if cancel: + client.debug.press_no() + else: + client.debug.press_yes() + yield + client.debug.press_yes() + + +def input_flow_scroll_down(client, cancel=False): + yield # confirm sending + client.debug.wait_layout() + client.debug.press_yes() + + yield # confirm data + client.debug.wait_layout() + client.debug.click(SHOW_ALL) + + br = yield # paginated data + for i in range(br.pages): + client.debug.wait_layout() + if i < br.pages - 1: + client.debug.swipe_up() + + client.debug.press_yes() + yield # confirm data + if cancel: + client.debug.press_no() + else: + client.debug.press_yes() + yield # hold to confirm + client.debug.press_yes() + + +def input_flow_go_back(client, cancel=False): + br = yield # confirm sending + client.debug.wait_layout() + client.debug.press_yes() + + br = yield # confirm data + client.debug.wait_layout() + client.debug.click(SHOW_ALL) + + br = yield # paginated data + for i in range(br.pages): + client.debug.wait_layout() + if i == 2: + client.debug.click(GO_BACK) + yield # confirm data + client.debug.wait_layout() + if cancel: + client.debug.press_no() + else: + client.debug.press_yes() + yield # hold to confirm + client.debug.wait_layout() + client.debug.press_yes() + return + + elif i < br.pages - 1: + client.debug.swipe_up() + + +HEXDATA = "0123456789abcd000023456789abcd010003456789abcd020000456789abcd030000056789abcd040000006789abcd050000000789abcd060000000089abcd070000000009abcd080000000000abcd090000000001abcd0a0000000011abcd0b0000000111abcd0c0000001111abcd0d0000011111abcd0e0000111111abcd0f0000000002abcd100000000022abcd110000000222abcd120000002222abcd130000022222abcd140000222222abcd15" + + +@pytest.mark.parametrize( + "flow", (input_flow_skip, input_flow_scroll_down, input_flow_go_back) +) +@pytest.mark.skip_t1 +def test_signtx_data_pagination(client, flow): + with client: + client.watch_layout() + client.set_input_flow(flow(client)) + ethereum.sign_tx( + client, + n=parse_path("m/44'/60'/0'/0/0"), + nonce=0x0, + gas_price=0x14, + gas_limit=0x14, + to="0x1d1c328764a41bda0492b66baa30c4a339ff85ef", + chain_id=1, + value=0xA, + tx_type=None, + data=bytes.fromhex(HEXDATA), + ) + + with client, pytest.raises(exceptions.Cancelled): + client.watch_layout() + client.set_input_flow(flow(client, cancel=True)) + ethereum.sign_tx( + client, + n=parse_path("m/44'/60'/0'/0/0"), + nonce=0x0, + gas_price=0x14, + gas_limit=0x14, + to="0x1d1c328764a41bda0492b66baa30c4a339ff85ef", + chain_id=1, + value=0xA, + tx_type=None, + data=bytes.fromhex(HEXDATA), + ) diff --git a/tests/ui_tests/fixtures.json b/tests/ui_tests/fixtures.json index d68d37d01..23c66b859 100644 --- a/tests/ui_tests/fixtures.json +++ b/tests/ui_tests/fixtures.json @@ -209,7 +209,7 @@ "ethereum-test_sign_verify_message.py::test_verify[parameters6-result6]": "3a8312fc9f26f2bdf6569d44b4c6f103ea6300da84d4353678ec9a66b42aa05d", "ethereum-test_sign_verify_message.py::test_verify[parameters7-result7]": "8fb2aeb728da4fb973a8cf058d975a78214c3aea7cf09280155fb167077f8951", "ethereum-test_sign_verify_message.py::test_verify_invalid": "7e83f210ce98fee92e34bcc95d311701ec79702f8430239921efa72ff7759af6", -"ethereum-test_signtx.py::test_data_streaming": "50db903970c264bddcc6e45fb2ef95aee0266fc9894613711d26be647b2eb7cd", +"ethereum-test_signtx.py::test_data_streaming": "e0e6179a08c7a96958814d95ddfe09996a96aefeec3f538acfa58844c664d90f", "ethereum-test_signtx.py::test_sanity_checks": "c09de07fbbf1e047442180e2facb5482d06a1a428891b875b7dd93c9e4704ae1", "ethereum-test_signtx.py::test_sanity_checks_eip1559": "c09de07fbbf1e047442180e2facb5482d06a1a428891b875b7dd93c9e4704ae1", "ethereum-test_signtx.py::test_signtx[Auxilium]": "d2ba6424ba04e6db899ec33253ce7fd26bbb1850a5e548f1e5628bbb311cc2fc", @@ -221,19 +221,22 @@ "ethereum-test_signtx.py::test_signtx[Ropsten]": "fe817a91dbf529ef52f64de52ec137d809cb6ef337810e3e1da168bcda6d940b", "ethereum-test_signtx.py::test_signtx[Unknown_chain_id_eth_path]": "e1c268db1580ebbf957eb2912baa747133bf640d886b1339aee7e709494b46a7", "ethereum-test_signtx.py::test_signtx[Unknown_chain_id_testnet_path]": "e1c268db1580ebbf957eb2912baa747133bf640d886b1339aee7e709494b46a7", -"ethereum-test_signtx.py::test_signtx[data_1]": "62e14aaedc636cacffc878a7f6f4045421683ef2be5105e771eaac7ba3ea2b93", -"ethereum-test_signtx.py::test_signtx[data_2_bigdata]": "6feb271ba67066d3c60af76dcc64d9ebdf97f040934e95f7a63d5f1a78a48c8d", +"ethereum-test_signtx.py::test_signtx[data_1]": "277686786f4dee54e641b15850b482480d1de4fd63401b76beb8da085ba27314", +"ethereum-test_signtx.py::test_signtx[data_2_bigdata]": "a728a670736dc4b5a60e1e98ff294f20e0b3c0c97e32e3e032e19140aa040561", "ethereum-test_signtx.py::test_signtx[known_erc20_token]": "313cacddc8234c92ad18f4c94bbd9da366eb38e8f3a9345521cf4b4af491eac8", "ethereum-test_signtx.py::test_signtx[max_chain_id]": "e1c268db1580ebbf957eb2912baa747133bf640d886b1339aee7e709494b46a7", "ethereum-test_signtx.py::test_signtx[max_chain_plus_one]": "e1c268db1580ebbf957eb2912baa747133bf640d886b1339aee7e709494b46a7", "ethereum-test_signtx.py::test_signtx[max_uint64]": "e1c268db1580ebbf957eb2912baa747133bf640d886b1339aee7e709494b46a7", -"ethereum-test_signtx.py::test_signtx[newcontract]": "9304b29803f1e85998127ae897a6d46457c8d6abd41f1daaa03eea90e5c8609f", +"ethereum-test_signtx.py::test_signtx[newcontract]": "251aa9bf7d0febf372a3a9ba9c9a5b988ce02214cead9a768f994ad2a59ecfb3", "ethereum-test_signtx.py::test_signtx[nodata_1]": "0653c875f9e81fbef050769692c650a62f4d10973fbfc02f3dd7e32d80c89176", "ethereum-test_signtx.py::test_signtx[nodata_2_bigvalue]": "a5d66260785d02985106b12e21dd96db82b8579d8b09e4135d796d33e31356c1", "ethereum-test_signtx.py::test_signtx[unknown_erc20_token]": "9663070b464ef7bdd9f41bad540135a46564ee215e5d5d6448e4fa721a137bc8", "ethereum-test_signtx.py::test_signtx[wanchain]": "feb75d11291435a367479d0f55874a6a276aa760b57b804f2339d4fb4143f5c8", -"ethereum-test_signtx.py::test_signtx_eip1559[data_1]": "a5e6df0f1fc2d96604f3b1af38b044b7426d8f0d8ab130e38afe160b317e3ed5", -"ethereum-test_signtx.py::test_signtx_eip1559[data_2_bigdata]": "22524a38623b6c26dab77330e54c04fabe22ec9511dc24fa4044ec58868f2c79", +"ethereum-test_signtx.py::test_signtx_data_pagination[input_flow_go_back]": "e0304501e6dd76d08052edd3ef518e4873f2029e8351bc1fb5ed5ba2a99e740a", +"ethereum-test_signtx.py::test_signtx_data_pagination[input_flow_scroll_down]": "f707da6726a2ebad6888af490eed2a1441147c795a8b72fb805fd0d3a8b071f9", +"ethereum-test_signtx.py::test_signtx_data_pagination[input_flow_skip]": "5d73d7f8366f4abd3eaf3e666f140ee0fcc0aca4f35483a6be27d96e2b64fd27", +"ethereum-test_signtx.py::test_signtx_eip1559[data_1]": "bf7fe1457c4fd99e74b8b9a2090321edfb20bdf68713505d92dd011f44d88184", +"ethereum-test_signtx.py::test_signtx_eip1559[data_2_bigdata]": "05cc77497ca4934f5472f4b8807208f54c734d1252e0eb186937a79e5ecec38b", "ethereum-test_signtx.py::test_signtx_eip1559[known_erc20]": "fd068a40fb0d4729c49316122711d36a92b09c04f60eed6d1fa5ecee57ae6f7a", "ethereum-test_signtx.py::test_signtx_eip1559[large_chainid]": "c8152a3f28ea1985d7c14844b3d8218a4a76e78fe0526e30c1d479f2cf200694", "ethereum-test_signtx.py::test_signtx_eip1559[nodata]": "55faae526aa63bc536114c70f29d5e12a6e973c565ae4247c439b755b589d0df",