From 9db8ff1faed800eac57c6524905f084aceb90805 Mon Sep 17 00:00:00 2001 From: grdddj Date: Tue, 7 Nov 2023 10:00:07 +0100 Subject: [PATCH] fix(core): unify sent button requests in ETH send flow Makes sure T2B1 will send the same ButtonRequests as T2T1. Does it by splitting the Rust layout into two separate dialogs. [no changelog] --- core/embed/rust/librust_qstr.h | 3 +- core/embed/rust/src/ui/model_tr/layout.rs | 31 +++------------- core/mocks/generated/trezorui2.pyi | 4 +-- core/src/trezor/ui/layouts/tr/__init__.py | 41 +++++++++++++++------- tests/device_tests/ethereum/test_signtx.py | 6 ++-- tests/input_flows_helpers.py | 1 + 6 files changed, 40 insertions(+), 46 deletions(-) diff --git a/core/embed/rust/librust_qstr.h b/core/embed/rust/librust_qstr.h index fb87b2e1fb..4b3dbd1e7a 100644 --- a/core/embed/rust/librust_qstr.h +++ b/core/embed/rust/librust_qstr.h @@ -45,7 +45,6 @@ static void _librust_qstrs(void) { MP_QSTR_confirm_blob; MP_QSTR_confirm_coinjoin; MP_QSTR_confirm_emphasized; - MP_QSTR_confirm_ethereum_tx; MP_QSTR_confirm_fido; MP_QSTR_confirm_firmware_update; MP_QSTR_confirm_homescreen; @@ -70,6 +69,7 @@ static void _librust_qstrs(void) { MP_QSTR_dry_run; MP_QSTR_encode; MP_QSTR_encoded_length; + MP_QSTR_ethereum_tx_summary; MP_QSTR_extra; MP_QSTR_fee_amount; MP_QSTR_fee_label; @@ -102,7 +102,6 @@ static void _librust_qstrs(void) { MP_QSTR_progress_event; MP_QSTR_prompt; MP_QSTR_qr_title; - MP_QSTR_recipient; MP_QSTR_request_bip39; MP_QSTR_request_complete_repaint; MP_QSTR_request_number; diff --git a/core/embed/rust/src/ui/model_tr/layout.rs b/core/embed/rust/src/ui/model_tr/layout.rs index d1a03e93b4..2ecb02b74e 100644 --- a/core/embed/rust/src/ui/model_tr/layout.rs +++ b/core/embed/rust/src/ui/model_tr/layout.rs @@ -754,34 +754,15 @@ extern "C" fn new_confirm_total(n_args: usize, args: *const Obj, kwargs: *mut Ma unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } } -extern "C" fn new_confirm_ethereum_tx(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj { +extern "C" fn new_ethereum_tx_summary(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj { let block = |_args: &[Obj], kwargs: &Map| { - let recipient: StrBuffer = kwargs.get(Qstr::MP_QSTR_recipient)?.try_into()?; let total_amount: StrBuffer = kwargs.get(Qstr::MP_QSTR_total_amount)?.try_into()?; let maximum_fee: StrBuffer = kwargs.get(Qstr::MP_QSTR_maximum_fee)?.try_into()?; let items: Obj = kwargs.get(Qstr::MP_QSTR_items)?; - let chunkify: bool = kwargs.get_or(Qstr::MP_QSTR_chunkify, false)?; let get_page = move |page_index| { match page_index { 0 => { - // RECIPIENT - let btn_layout = ButtonLayout::cancel_none_text("CONTINUE".into()); - let btn_actions = ButtonActions::cancel_none_next(); - - let style = if chunkify { - // Chunkifying the address into smaller pieces when requested - theme::TEXT_MONO_ADDRESS_CHUNKS - } else { - theme::TEXT_MONO_DATA - }; - - let ops = OpTextLayout::new(style).text_mono(recipient.clone()); - - let formatted = FormattedText::new(ops).vertically_centered(); - Page::new(btn_layout, btn_actions, formatted).with_title("RECIPIENT".into()) - } - 1 => { // Total amount + fee let btn_layout = ButtonLayout::up_arrow_armed_info("CONFIRM".into()); let btn_actions = ButtonActions::prev_confirm_next(); @@ -797,7 +778,7 @@ extern "C" fn new_confirm_ethereum_tx(n_args: usize, args: *const Obj, kwargs: * let formatted = FormattedText::new(ops); Page::new(btn_layout, btn_actions, formatted).with_title("Amount:".into()) } - 2 => { + 1 => { // Fee information let btn_layout = ButtonLayout::arrow_none_none(); let btn_actions = ButtonActions::prev_none_none(); @@ -824,7 +805,7 @@ extern "C" fn new_confirm_ethereum_tx(n_args: usize, args: *const Obj, kwargs: * _ => unreachable!(), } }; - let pages = FlowPages::new(get_page, 3); + let pages = FlowPages::new(get_page, 2); let obj = LayoutObj::new(Flow::new(pages).with_scrollbar(false))?; Ok(obj.into()) @@ -1816,16 +1797,14 @@ pub static mp_module_trezorui2: Module = obj_module! { /// """Confirm summary of a transaction.""" Qstr::MP_QSTR_confirm_total => obj_fn_kw!(0, new_confirm_total).as_obj(), - /// def confirm_ethereum_tx( + /// def ethereum_tx_summary( /// *, - /// recipient: str, /// total_amount: str, /// maximum_fee: str, /// items: Iterable[Tuple[str, str]], - /// chunkify: bool = False, /// ) -> object: /// """Confirm details about Ethereum transaction.""" - Qstr::MP_QSTR_confirm_ethereum_tx => obj_fn_kw!(0, new_confirm_ethereum_tx).as_obj(), + Qstr::MP_QSTR_ethereum_tx_summary => obj_fn_kw!(0, new_ethereum_tx_summary).as_obj(), /// def tutorial() -> object: /// """Show user how to interact with the device.""" diff --git a/core/mocks/generated/trezorui2.pyi b/core/mocks/generated/trezorui2.pyi index e32a660a0f..0569020e3c 100644 --- a/core/mocks/generated/trezorui2.pyi +++ b/core/mocks/generated/trezorui2.pyi @@ -170,13 +170,11 @@ def confirm_total( # rust/src/ui/model_tr/layout.rs -def confirm_ethereum_tx( +def ethereum_tx_summary( *, - recipient: str, total_amount: str, maximum_fee: str, items: Iterable[Tuple[str, str]], - chunkify: bool = False, ) -> object: """Confirm details about Ethereum transaction.""" diff --git a/core/src/trezor/ui/layouts/tr/__init__.py b/core/src/trezor/ui/layouts/tr/__init__.py index 17f505d2b6..e10086862c 100644 --- a/core/src/trezor/ui/layouts/tr/__init__.py +++ b/core/src/trezor/ui/layouts/tr/__init__.py @@ -1000,22 +1000,37 @@ async def confirm_ethereum_tx( br_code: ButtonRequestType = ButtonRequestType.SignTx, chunkify: bool = False, ) -> None: - await raise_if_not_confirmed( - interact( - RustLayout( - trezorui2.confirm_ethereum_tx( - recipient=recipient, - total_amount=total_amount, - maximum_fee=maximum_fee, - items=items, - chunkify=chunkify, - ) - ), - br_type, - br_code, + summary_layout = RustLayout( + trezorui2.ethereum_tx_summary( + total_amount=total_amount, + maximum_fee=maximum_fee, + items=items, ) ) + while True: + # Allowing going back and forth between recipient and summary/details + await confirm_blob( + br_type, + "RECIPIENT", + recipient, + verb="CONTINUE", + chunkify=chunkify, + ) + + try: + summary_layout.request_complete_repaint() + await raise_if_not_confirmed( + interact( + summary_layout, + br_type, + br_code, + ) + ) + break + except ActionCancelled: + continue + async def confirm_joint_total(spending_amount: str, total_amount: str) -> None: await raise_if_not_confirmed( diff --git a/tests/device_tests/ethereum/test_signtx.py b/tests/device_tests/ethereum/test_signtx.py index 28df5d0753..209220c46e 100644 --- a/tests/device_tests/ethereum/test_signtx.py +++ b/tests/device_tests/ethereum/test_signtx.py @@ -187,12 +187,14 @@ def test_data_streaming(client: Client): """ with client: is_t1 = client.features.model == "1" - is_tt = client.features.model == "T" client.set_expected_responses( [ messages.ButtonRequest(code=messages.ButtonRequestType.SignTx), (is_t1, messages.ButtonRequest(code=messages.ButtonRequestType.SignTx)), - (is_tt, messages.ButtonRequest(code=messages.ButtonRequestType.Other)), + ( + not is_t1, + messages.ButtonRequest(code=messages.ButtonRequestType.Other), + ), messages.ButtonRequest(code=messages.ButtonRequestType.SignTx), message_filters.EthereumTxRequest( data_length=1_024, diff --git a/tests/input_flows_helpers.py b/tests/input_flows_helpers.py index 64aefaa739..f318a35c75 100644 --- a/tests/input_flows_helpers.py +++ b/tests/input_flows_helpers.py @@ -316,6 +316,7 @@ class EthereumFlow: self.debug.press_left() else: self.debug.press_right() + yield assert "Maximum fee:" in self.debug.wait_layout().text_content() if info: self.debug.press_right(wait=True)