From eed6e0b71c97a9414d1258345f0d73d9ef7d973e Mon Sep 17 00:00:00 2001 From: grdddj Date: Mon, 31 Jul 2023 17:02:36 +0200 Subject: [PATCH] feat(core): create new Ethereum send flow for TR [no changelog] --- core/embed/rust/librust_qstr.h | 3 + core/embed/rust/src/ui/component/text/op.rs | 6 +- .../rust/src/ui/model_tr/component/button.rs | 15 ++- .../src/ui/model_tr/component/flow_pages.rs | 28 +++++- core/embed/rust/src/ui/model_tr/layout.rs | 82 ++++++++++++++++- core/mocks/generated/trezorui2.pyi | 11 +++ core/src/apps/ethereum/layout.py | 92 ++++++++----------- core/src/apps/ethereum/sign_tx.py | 10 +- core/src/apps/ethereum/sign_tx_eip1559.py | 17 ++-- core/src/trezor/ui/layouts/tr/__init__.py | 26 +++++- tools/check-bitcoin-only | 1 + 11 files changed, 213 insertions(+), 78 deletions(-) diff --git a/core/embed/rust/librust_qstr.h b/core/embed/rust/librust_qstr.h index 7fcddbeb56..f29a2c07fd 100644 --- a/core/embed/rust/librust_qstr.h +++ b/core/embed/rust/librust_qstr.h @@ -41,6 +41,7 @@ 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_homescreen; MP_QSTR_confirm_joint_total; @@ -85,6 +86,7 @@ static void _librust_qstrs(void) { MP_QSTR_max_feerate; MP_QSTR_max_len; MP_QSTR_max_rounds; + MP_QSTR_maximum_fee; MP_QSTR_min_count; MP_QSTR_multiple_pages_texts; MP_QSTR_notification; @@ -95,6 +97,7 @@ static void _librust_qstrs(void) { MP_QSTR_path; MP_QSTR_progress_event; MP_QSTR_prompt; + MP_QSTR_recipient; MP_QSTR_request_bip39; MP_QSTR_request_complete_repaint; MP_QSTR_request_number; diff --git a/core/embed/rust/src/ui/component/text/op.rs b/core/embed/rust/src/ui/component/text/op.rs index b7ececf661..4a681ee902 100644 --- a/core/embed/rust/src/ui/component/text/op.rs +++ b/core/embed/rust/src/ui/component/text/op.rs @@ -16,7 +16,7 @@ use heapless::Vec; // So that there is only one implementation, and not multiple generic ones // as would be via `const N: usize` generics. -const MAX_OPS: usize = 15; +const MAX_OPS: usize = 20; /// To account for operations that are not made of characters /// but need to be accounted for somehow. @@ -39,6 +39,10 @@ impl<'a, T: StringType + Clone + 'a> OpTextLayout { } } + pub fn is_empty(&self) -> bool { + self.ops.len() == 0 + } + pub fn place(&mut self, bounds: Rect) -> Rect { self.layout.bounds = bounds; bounds diff --git a/core/embed/rust/src/ui/model_tr/component/button.rs b/core/embed/rust/src/ui/model_tr/component/button.rs index f8c658566b..1435eb1f6d 100644 --- a/core/embed/rust/src/ui/model_tr/component/button.rs +++ b/core/embed/rust/src/ui/model_tr/component/button.rs @@ -383,9 +383,9 @@ where pub fn from_text_possible_icon(text: T) -> Self { if text.as_ref() == "" { Self::cancel_icon() - } else if text.as_ref() == "left_arrow_icon" { + } else if text.as_ref() == "<" { Self::left_arrow_icon() - } else if text.as_ref() == "up_arrow_icon" { + } else if text.as_ref() == "^" { Self::up_arrow_icon() } else { Self::text(text) @@ -662,7 +662,7 @@ where ) } - /// Cancel cross on left and right arrow facing down. + /// Up arrow on left and right arrow facing down. pub fn up_arrow_none_arrow_wide() -> Self { Self::new( Some(ButtonDetails::up_arrow_icon()), @@ -671,6 +671,15 @@ where ) } + /// Up arrow on left, middle text and info on the right. + pub fn up_arrow_armed_info(text: T) -> Self { + Self::new( + Some(ButtonDetails::up_arrow_icon()), + Some(ButtonDetails::armed_text(text)), + Some(ButtonDetails::text("i".into()).with_fixed_width(theme::BUTTON_ICON_WIDTH)), + ) + } + /// Cancel cross on left and right arrow facing down. pub fn cancel_none_arrow_down() -> Self { Self::new( diff --git a/core/embed/rust/src/ui/model_tr/component/flow_pages.rs b/core/embed/rust/src/ui/model_tr/component/flow_pages.rs index c488b93e54..a4cf0b19d6 100644 --- a/core/embed/rust/src/ui/model_tr/component/flow_pages.rs +++ b/core/embed/rust/src/ui/model_tr/component/flow_pages.rs @@ -84,6 +84,7 @@ where current_page: usize, page_count: usize, title: Option, + slim_arrows: bool, } // For `layout.rs` @@ -103,6 +104,7 @@ where current_page: 0, page_count: 1, title: None, + slim_arrows: false, } } } @@ -118,6 +120,12 @@ where self } + /// Using slim arrows instead of wide buttons. + pub fn with_slim_arrows(mut self) -> Self { + self.slim_arrows = true; + self + } + pub fn paint(&mut self) { self.change_page(self.current_page); self.formatted.paint(); @@ -137,17 +145,29 @@ where // On the last page showing only the narrow arrow, so the right // button with possibly long text has enough space. let btn_left = if self.has_prev_page() && !self.has_next_page() { - Some(ButtonDetails::up_arrow_icon()) + if self.slim_arrows { + Some(ButtonDetails::left_arrow_icon()) + } else { + Some(ButtonDetails::up_arrow_icon()) + } } else if self.has_prev_page() { - Some(ButtonDetails::up_arrow_icon_wide()) + if self.slim_arrows { + Some(ButtonDetails::left_arrow_icon()) + } else { + Some(ButtonDetails::up_arrow_icon_wide()) + } } else { current.btn_left }; // Middle button should be shown only on the last page, not to collide - // with the fat right button. + // with the possible fat right button. let (btn_middle, btn_right) = if self.has_next_page() { - (None, Some(ButtonDetails::down_arrow_icon_wide())) + if self.slim_arrows { + (None, Some(ButtonDetails::right_arrow_icon())) + } else { + (None, Some(ButtonDetails::down_arrow_icon_wide())) + } } else { (current.btn_middle, current.btn_right) }; diff --git a/core/embed/rust/src/ui/model_tr/layout.rs b/core/embed/rust/src/ui/model_tr/layout.rs index 09344d4731..3de0cadd75 100644 --- a/core/embed/rust/src/ui/model_tr/layout.rs +++ b/core/embed/rust/src/ui/model_tr/layout.rs @@ -679,6 +679,76 @@ 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 { + 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 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 ops = OpTextLayout::new(theme::TEXT_MONO_DATA).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(); + + let ops = OpTextLayout::new(theme::TEXT_MONO) + .text_mono(total_amount.clone()) + .newline() + .newline_half() + .text_bold("Maximum fee:".into()) + .newline() + .text_mono(maximum_fee.clone()); + + let formatted = FormattedText::new(ops); + Page::new(btn_layout, btn_actions, formatted).with_title("Amount:".into()) + } + 2 => { + // Fee information + let btn_layout = ButtonLayout::arrow_none_none(); + let btn_actions = ButtonActions::prev_none_none(); + + let mut ops = OpTextLayout::new(theme::TEXT_MONO); + + for item in unwrap!(IterBuf::new().try_iterate(items)) { + let [key, value]: [Obj; 2] = unwrap!(iter_into_array(item)); + if !ops.is_empty() { + // Each key-value pair is on its own page + ops = ops.next_page(); + } + ops = ops + .text_bold(unwrap!(key.try_into())) + .newline() + .text_mono(unwrap!(value.try_into())); + } + + let formatted = FormattedText::new(ops).vertically_centered(); + Page::new(btn_layout, btn_actions, formatted) + .with_title("FEE INFORMATION".into()) + .with_slim_arrows() + } + _ => unreachable!(), + } + }; + let pages = FlowPages::new(get_page, 3); + + let obj = LayoutObj::new(Flow::new(pages).with_scrollbar(false))?; + Ok(obj.into()) + }; + unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } +} + extern "C" fn new_confirm_address(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj { let block = move |_args: &[Obj], kwargs: &Map| { let title: StrBuffer = kwargs.get(Qstr::MP_QSTR_title)?.try_into()?; @@ -1099,7 +1169,7 @@ extern "C" fn new_confirm_more(n_args: usize, args: *const Obj, kwargs: *mut Map title, paragraphs.into_paragraphs(), button, - Some("left_arrow_icon".into()), + Some("<".into()), false, ) }; @@ -1599,6 +1669,16 @@ 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( + /// *, + /// recipient: str, + /// total_amount: str, + /// maximum_fee: str, + /// items: Iterable[Tuple[str, str]], + /// ) -> object: + /// """Confirm details about Ethereum transaction.""" + Qstr::MP_QSTR_confirm_ethereum_tx => obj_fn_kw!(0, new_confirm_ethereum_tx).as_obj(), + /// def tutorial() -> object: /// """Show user how to interact with the device.""" Qstr::MP_QSTR_tutorial => obj_fn_kw!(0, tutorial).as_obj(), diff --git a/core/mocks/generated/trezorui2.pyi b/core/mocks/generated/trezorui2.pyi index 3622affb55..cd448fab8a 100644 --- a/core/mocks/generated/trezorui2.pyi +++ b/core/mocks/generated/trezorui2.pyi @@ -149,6 +149,17 @@ def confirm_total( """Confirm summary of a transaction.""" +# rust/src/ui/model_tr/layout.rs +def confirm_ethereum_tx( + *, + recipient: str, + total_amount: str, + maximum_fee: str, + items: Iterable[Tuple[str, str]], +) -> object: + """Confirm details about Ethereum transaction.""" + + # rust/src/ui/model_tr/layout.rs def tutorial() -> object: """Show user how to interact with the device.""" diff --git a/core/src/apps/ethereum/layout.py b/core/src/apps/ethereum/layout.py index cf470eac08..2d3632bd3a 100644 --- a/core/src/apps/ethereum/layout.py +++ b/core/src/apps/ethereum/layout.py @@ -4,14 +4,13 @@ from trezor import ui from trezor.enums import ButtonRequestType from trezor.strings import format_plural from trezor.ui.layouts import ( - confirm_amount, confirm_blob, + confirm_ethereum_tx, confirm_text, - confirm_total, should_show_more, ) -from .helpers import decode_typed_data +from .helpers import address_from_bytes, decode_typed_data if TYPE_CHECKING: from typing import Awaitable, Iterable @@ -24,72 +23,61 @@ if TYPE_CHECKING: ) -def require_confirm_tx( +async def require_confirm_tx( to_bytes: bytes, value: int, - network: EthereumNetworkInfo, - token: EthereumTokenInfo | None, -) -> Awaitable[None]: - from trezor.ui.layouts import confirm_output - - from .helpers import address_from_bytes - - if to_bytes: - to_str = address_from_bytes(to_bytes, network) - else: - to_str = "new contract?" - return confirm_output( - to_str, - format_ethereum_amount(value, token, network), - br_code=ButtonRequestType.SignTx, - ) - - -async def require_confirm_fee( - spending: int, gas_price: int, gas_limit: int, network: EthereumNetworkInfo, token: EthereumTokenInfo | None, ) -> None: - await confirm_amount( - title="Confirm fee", - description="Gas price:", - amount=format_ethereum_amount(gas_price, None, network), - ) - await confirm_total( - total_amount=format_ethereum_amount(spending, token, network), - fee_amount=format_ethereum_amount(gas_price * gas_limit, None, network), - total_label="Amount sent:", - fee_label="Maximum fee:", + if to_bytes: + to_str = address_from_bytes(to_bytes, network) + else: + to_str = "new contract?" + + total_amount = format_ethereum_amount(value, token, network) + maximum_fee = format_ethereum_amount(gas_price * gas_limit, None, network) + gas_limit_str = f"{gas_limit} units" + gas_price_str = format_ethereum_amount(gas_price, None, network) + + items = ( + ("Gas limit:", gas_limit_str), + ("Gas price:", gas_price_str), ) + await confirm_ethereum_tx(to_str, total_amount, maximum_fee, items) -async def require_confirm_eip1559_fee( - spending: int, - max_priority_fee: int, + +async def require_confirm_tx_eip1559( + to_bytes: bytes, + value: int, max_gas_fee: int, + max_priority_fee: int, gas_limit: int, network: EthereumNetworkInfo, token: EthereumTokenInfo | None, ) -> None: - await confirm_amount( - "Confirm fee", - format_ethereum_amount(max_gas_fee, None, network), - "Maximum fee per gas", - ) - await confirm_amount( - "Confirm fee", - format_ethereum_amount(max_priority_fee, None, network), - "Priority fee per gas", - ) - await confirm_total( - format_ethereum_amount(spending, token, network), - format_ethereum_amount(max_gas_fee * gas_limit, None, network), - total_label="Amount sent:", - fee_label="Maximum fee:", + + if to_bytes: + to_str = address_from_bytes(to_bytes, network) + else: + to_str = "new contract?" + + total_amount = format_ethereum_amount(value, token, network) + maximum_fee = format_ethereum_amount(max_gas_fee * gas_limit, None, network) + gas_limit_str = f"{gas_limit} units" + max_gas_fee_str = format_ethereum_amount(max_gas_fee, None, network) + max_priority_fee_str = format_ethereum_amount(max_priority_fee, None, network) + + items = ( + ("Gas limit:", gas_limit_str), + ("Max gas price:", max_gas_fee_str), + ("Priority fee:", max_priority_fee_str), ) + await confirm_ethereum_tx(to_str, total_amount, maximum_fee, items) + def require_confirm_unknown_token(address_bytes: bytes) -> Awaitable[None]: from ubinascii import hexlify diff --git a/core/src/apps/ethereum/sign_tx.py b/core/src/apps/ethereum/sign_tx.py index bd10054b35..7dec659abf 100644 --- a/core/src/apps/ethereum/sign_tx.py +++ b/core/src/apps/ethereum/sign_tx.py @@ -33,7 +33,7 @@ async def sign_tx( from apps.common import paths - from .layout import require_confirm_data, require_confirm_fee, require_confirm_tx + from .layout import require_confirm_data, require_confirm_tx # check if msg.tx_type not in [1, 6, None]: @@ -47,13 +47,13 @@ async def sign_tx( # Handle ERC20s token, address_bytes, recipient, value = await handle_erc20(msg, defs) - data_total = msg.data_length + data_total = msg.data_length # local_cache_attribute - await require_confirm_tx(recipient, value, defs.network, token) - if token is None and msg.data_length > 0: + if token is None and data_total > 0: await require_confirm_data(msg.data_initial_chunk, data_total) - await require_confirm_fee( + await require_confirm_tx( + recipient, value, int.from_bytes(msg.gas_price, "big"), int.from_bytes(msg.gas_limit, "big"), diff --git a/core/src/apps/ethereum/sign_tx_eip1559.py b/core/src/apps/ethereum/sign_tx_eip1559.py index 466b63e93b..13bbff1114 100644 --- a/core/src/apps/ethereum/sign_tx_eip1559.py +++ b/core/src/apps/ethereum/sign_tx_eip1559.py @@ -42,14 +42,11 @@ async def sign_tx_eip1559( from apps.common import paths - from .layout import ( - require_confirm_data, - require_confirm_eip1559_fee, - require_confirm_tx, - ) + from .layout import require_confirm_data, require_confirm_tx_eip1559 from .sign_tx import check_common_fields, handle_erc20, send_request_chunk gas_limit = msg.gas_limit # local_cache_attribute + data_total = msg.data_length # local_cache_attribute # check if len(msg.max_gas_fee) + len(gas_limit) > 30: @@ -63,16 +60,14 @@ async def sign_tx_eip1559( # Handle ERC20s token, address_bytes, recipient, value = await handle_erc20(msg, defs) - data_total = msg.data_length - - await require_confirm_tx(recipient, value, defs.network, token) - if token is None and msg.data_length > 0: + if token is None and data_total > 0: await require_confirm_data(msg.data_initial_chunk, data_total) - await require_confirm_eip1559_fee( + await require_confirm_tx_eip1559( + recipient, value, - int.from_bytes(msg.max_priority_fee, "big"), int.from_bytes(msg.max_gas_fee, "big"), + int.from_bytes(msg.max_priority_fee, "big"), int.from_bytes(gas_limit, "big"), defs.network, token, diff --git a/core/src/trezor/ui/layouts/tr/__init__.py b/core/src/trezor/ui/layouts/tr/__init__.py index 0f48486072..7b2d366f39 100644 --- a/core/src/trezor/ui/layouts/tr/__init__.py +++ b/core/src/trezor/ui/layouts/tr/__init__.py @@ -972,6 +972,30 @@ async def confirm_total( ) +async def confirm_ethereum_tx( + recipient: str, + total_amount: str, + maximum_fee: str, + items: Iterable[tuple[str, str]], + br_type: str = "confirm_ethereum_tx", + br_code: ButtonRequestType = ButtonRequestType.SignTx, +) -> None: + await raise_if_not_confirmed( + interact( + RustLayout( + trezorui2.confirm_ethereum_tx( + recipient=recipient, + total_amount=total_amount, + maximum_fee=maximum_fee, + items=items, + ) + ), + br_type, + br_code, + ) + ) + + async def confirm_joint_total(spending_amount: str, total_amount: str) -> None: await raise_if_not_confirmed( @@ -1114,7 +1138,7 @@ async def confirm_signverify( br_type, "CONFIRM MESSAGE", message, - verb_cancel="up_arrow_icon", + verb_cancel="^", br_code=BR_TYPE_OTHER, ask_pagination=True, ) diff --git a/tools/check-bitcoin-only b/tools/check-bitcoin-only index 8fe245c346..33a82f9eff 100755 --- a/tools/check-bitcoin-only +++ b/tools/check-bitcoin-only @@ -10,6 +10,7 @@ EXCEPTIONS+=( "mnemonic" ) # has NEM in it EXCEPTIONS+=( "workflow" "overflow" ) # has Flo in it EXCEPTIONS+=( "SyntaxError" ) # has Axe in it EXCEPTIONS+=( "DKDNEM" ) # has NEM in it, some sort of weird coincidence +EXCEPTIONS+=( "confirm_ethereum_tx" ) # is model-specific, so is in layout/__init__.py instead of ethereum/layout.py GREP_ARGS=() for exception in "${EXCEPTIONS[@]}"; do