From 61ebb19515547f934cae5a9458f990a92eb4c599 Mon Sep 17 00:00:00 2001 From: obrusvit Date: Fri, 29 Nov 2024 15:32:09 +0100 Subject: [PATCH] refactor(core): mercury confirm_summary - old confirm_total removed - flow_confirm_summary refactored to confirm_summary [no changelog] --- core/embed/rust/librust_qstr.h | 8 +- .../model_mercury/component/vertical_menu.rs | 6 +- .../ui/model_mercury/flow/confirm_summary.rs | 73 +++++---- .../embed/rust/src/ui/model_mercury/layout.rs | 147 ++++++++---------- core/mocks/generated/trezorui2.pyi | 32 ++-- .../src/trezor/ui/layouts/mercury/__init__.py | 79 +++++----- 6 files changed, 163 insertions(+), 182 deletions(-) diff --git a/core/embed/rust/librust_qstr.h b/core/embed/rust/librust_qstr.h index beb5513de0..ef3215cd34 100644 --- a/core/embed/rust/librust_qstr.h +++ b/core/embed/rust/librust_qstr.h @@ -37,9 +37,7 @@ static void _librust_qstrs(void) { MP_QSTR___dict__; MP_QSTR___name__; MP_QSTR_account; - MP_QSTR_account_info; MP_QSTR_account_items; - MP_QSTR_account_items_title; MP_QSTR_account_path; MP_QSTR_accounts; MP_QSTR_action; @@ -174,7 +172,6 @@ static void _librust_qstrs(void) { MP_QSTR_buttons__view_all_data; MP_QSTR_can_go_back; MP_QSTR_cancel; - MP_QSTR_cancel_arrow; MP_QSTR_cancel_text; MP_QSTR_case_sensitive; MP_QSTR_check_homescreen_format; @@ -240,9 +237,9 @@ static void _librust_qstrs(void) { MP_QSTR_experimental_mode__only_for_dev; MP_QSTR_experimental_mode__title; MP_QSTR_extra; - MP_QSTR_extra_info; + MP_QSTR_extra_items; + MP_QSTR_extra_title; MP_QSTR_fee; - MP_QSTR_fee_info; MP_QSTR_fee_items; MP_QSTR_fee_label; MP_QSTR_fee_rate_amount; @@ -253,7 +250,6 @@ static void _librust_qstrs(void) { MP_QSTR_flow_confirm_output; MP_QSTR_flow_confirm_reset; MP_QSTR_flow_confirm_set_new_pin; - MP_QSTR_flow_confirm_summary; MP_QSTR_flow_continue_recovery; MP_QSTR_flow_get_address; MP_QSTR_flow_prompt_backup; diff --git a/core/embed/rust/src/ui/model_mercury/component/vertical_menu.rs b/core/embed/rust/src/ui/model_mercury/component/vertical_menu.rs index a4a9db7ebe..5f9b4cce77 100644 --- a/core/embed/rust/src/ui/model_mercury/component/vertical_menu.rs +++ b/core/embed/rust/src/ui/model_mercury/component/vertical_menu.rs @@ -25,7 +25,7 @@ pub enum VerticalMenuChoiceMsg { /// Number of buttons. /// Presently, VerticalMenu holds only fixed number of buttons. -const MAX_ITEMS: usize = 3; +const MENU_MAX_ITEMS: usize = 3; /// Fixed height of each menu button. const MENU_BUTTON_HEIGHT: i16 = 64; @@ -33,7 +33,7 @@ const MENU_BUTTON_HEIGHT: i16 = 64; /// Fixed height of a separator. const MENU_SEP_HEIGHT: i16 = 2; -type VerticalMenuButtons = Vec; +type VerticalMenuButtons = Vec; #[derive(Default, Clone)] struct AttachAnimation { @@ -180,7 +180,7 @@ impl VerticalMenu { fn new(buttons: VerticalMenuButtons) -> Self { Self { buttons, - n_items: MAX_ITEMS, + n_items: MENU_MAX_ITEMS, attach_animation: AttachAnimation::default(), } } diff --git a/core/embed/rust/src/ui/model_mercury/flow/confirm_summary.rs b/core/embed/rust/src/ui/model_mercury/flow/confirm_summary.rs index f7adb903cd..689b9cba74 100644 --- a/core/embed/rust/src/ui/model_mercury/flow/confirm_summary.rs +++ b/core/embed/rust/src/ui/model_mercury/flow/confirm_summary.rs @@ -1,15 +1,15 @@ use heapless::Vec; use crate::{ - error, + error::{self}, + maybe_trace::MaybeTrace, strutil::TString, translations::TR, ui::{ - button_request::ButtonRequest, - component::{swipe_detect::SwipeSettings, ButtonRequestExt, ComponentExt}, + component::{swipe_detect::SwipeSettings, Component, ComponentExt}, flow::{ base::{Decision, DecisionBuilder as _}, - FlowController, FlowMsg, SwipeFlow, + FlowController, FlowMsg, Swipable, SwipeFlow, }, geometry::Direction, }, @@ -27,7 +27,7 @@ use super::{ }; const MENU_ITEM_CANCEL: usize = 0; -const MENU_ITEM_FEE_INFO: usize = 1; +const MENU_ITEM_EXTRA_INFO: usize = 1; const MENU_ITEM_ACCOUNT_INFO: usize = 2; #[derive(Copy, Clone, PartialEq, Eq)] @@ -35,7 +35,7 @@ pub enum ConfirmSummary { Summary, Hold, Menu, - FeeInfo, + ExtraInfo, AccountInfo, CancelTap, } @@ -52,7 +52,7 @@ impl FlowController for ConfirmSummary { (Self::Summary, Direction::Up) => Self::Hold.swipe(direction), (Self::Hold, Direction::Down) => Self::Summary.swipe(direction), (Self::Menu, Direction::Right) => Self::Summary.swipe(direction), - (Self::AccountInfo | Self::FeeInfo | Self::CancelTap, Direction::Right) => { + (Self::ExtraInfo | Self::AccountInfo | Self::CancelTap, Direction::Right) => { Self::Menu.swipe(direction) } _ => self.do_nothing(), @@ -64,7 +64,7 @@ impl FlowController for ConfirmSummary { (_, FlowMsg::Info) => Self::Menu.goto(), (Self::Hold, FlowMsg::Confirmed) => self.return_msg(FlowMsg::Confirmed), (Self::Menu, FlowMsg::Choice(MENU_ITEM_CANCEL)) => Self::CancelTap.swipe_left(), - (Self::Menu, FlowMsg::Choice(MENU_ITEM_FEE_INFO)) => Self::FeeInfo.swipe_left(), + (Self::Menu, FlowMsg::Choice(MENU_ITEM_EXTRA_INFO)) => Self::ExtraInfo.swipe_left(), (Self::Menu, FlowMsg::Choice(MENU_ITEM_ACCOUNT_INFO)) => Self::AccountInfo.swipe_left(), (Self::Menu, FlowMsg::Cancelled) => Self::Summary.swipe_right(), (Self::CancelTap, FlowMsg::Confirmed) => self.return_msg(FlowMsg::Cancelled), @@ -74,18 +74,20 @@ impl FlowController for ConfirmSummary { } } +fn dummy_page() -> impl Component + Swipable + MaybeTrace { + Frame::left_aligned(TString::empty(), VerticalMenu::empty()).map(|_| Some(FlowMsg::Cancelled)) +} + pub fn new_confirm_summary( summary_params: ShowInfoParams, - account_params: ShowInfoParams, - fee_params: ShowInfoParams, - br_name: TString<'static>, - br_code: u16, - cancel_text: Option>, + account_params: Option, + extra_params: Option, + extra_title: Option>, + verb_cancel: Option>, ) -> Result { // Summary let content_summary = summary_params .into_layout()? - .one_button_request(ButtonRequest::from_num(br_code, br_name)) // Summary(1) + Hold(1) .with_pages(|summary_pages| summary_pages + 1); @@ -104,25 +106,26 @@ pub fn new_confirm_summary( _ => None, }); - // FeeInfo - let has_fee_info = !fee_params.is_empty(); - let content_fee = fee_params.into_layout()?; - + // ExtraInfo + let content_extra = extra_params + .map(|params| params.into_layout()) + .transpose()?; // AccountInfo - let has_account_info = !account_params.is_empty(); - let content_account = account_params.into_layout()?; + let content_account = account_params + .map(|params| params.into_layout()) + .transpose()?; - // Menu + // Menu with provided info and cancel let mut menu = VerticalMenu::empty(); let mut menu_items = Vec::::new(); - if has_fee_info { + if content_extra.is_some() { menu = menu.item( theme::ICON_CHEVRON_RIGHT, - TR::confirm_total__title_fee.into(), + extra_title.unwrap_or(TR::buttons__more_info.into()), ); - unwrap!(menu_items.push(MENU_ITEM_FEE_INFO)); + unwrap!(menu_items.push(MENU_ITEM_EXTRA_INFO)); } - if has_account_info { + if content_account.is_some() { menu = menu.item( theme::ICON_CHEVRON_RIGHT, TR::address_details__account_info.into(), @@ -131,7 +134,7 @@ pub fn new_confirm_summary( } menu = menu.danger( theme::ICON_CANCEL, - cancel_text.unwrap_or(TR::send__cancel_sign.into()), + verb_cancel.unwrap_or(TR::send__cancel_sign.into()), ); unwrap!(menu_items.push(MENU_ITEM_CANCEL)); let content_menu = Frame::left_aligned(TString::empty(), menu) @@ -159,13 +162,21 @@ pub fn new_confirm_summary( _ => None, }); - let res = SwipeFlow::new(&ConfirmSummary::Summary)? + let mut res = SwipeFlow::new(&ConfirmSummary::Summary)? .with_page(&ConfirmSummary::Summary, content_summary)? .with_page(&ConfirmSummary::Hold, content_hold)? - .with_page(&ConfirmSummary::Menu, content_menu)? - .with_page(&ConfirmSummary::FeeInfo, content_fee)? - .with_page(&ConfirmSummary::AccountInfo, content_account)? - .with_page(&ConfirmSummary::CancelTap, content_cancel_tap)?; + .with_page(&ConfirmSummary::Menu, content_menu)?; + if let Some(content_extra) = content_extra { + res = res.with_page(&ConfirmSummary::ExtraInfo, content_extra)? + } else { + res = res.with_page(&ConfirmSummary::ExtraInfo, dummy_page())? + }; + if let Some(content_account) = content_account { + res = res.with_page(&ConfirmSummary::AccountInfo, content_account)? + } else { + res = res.with_page(&ConfirmSummary::AccountInfo, dummy_page())? + }; + res = res.with_page(&ConfirmSummary::CancelTap, content_cancel_tap)?; Ok(res) } diff --git a/core/embed/rust/src/ui/model_mercury/layout.rs b/core/embed/rust/src/ui/model_mercury/layout.rs index af8f06e736..931c1c80bd 100644 --- a/core/embed/rust/src/ui/model_mercury/layout.rs +++ b/core/embed/rust/src/ui/model_mercury/layout.rs @@ -624,50 +624,68 @@ extern "C" fn new_confirm_output(n_args: usize, args: *const Obj, kwargs: *mut M extern "C" fn new_confirm_summary(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj { let block = move |_args: &[Obj], kwargs: &Map| { - let title: TString = kwargs.get(Qstr::MP_QSTR_title)?.try_into()?; - let items: Obj = kwargs.get(Qstr::MP_QSTR_items)?; - let account_items: Obj = kwargs.get(Qstr::MP_QSTR_account_items)?; - let account_items_title: Option = kwargs - .get(Qstr::MP_QSTR_account_items_title) - .unwrap_or(Obj::const_none()) + let amount: TString = kwargs.get(Qstr::MP_QSTR_amount)?.try_into()?; + let amount_label: TString = kwargs.get(Qstr::MP_QSTR_amount_label)?.try_into()?; + let fee: TString = kwargs.get(Qstr::MP_QSTR_fee)?.try_into()?; + let fee_label: TString = kwargs.get(Qstr::MP_QSTR_fee_label)?.try_into()?; + let title: Option = kwargs + .get(Qstr::MP_QSTR_title) + .unwrap_or_else(|_| Obj::const_none()) + .try_into_option()?; + let account_items: Option = kwargs + .get(Qstr::MP_QSTR_account_items) + .unwrap_or_else(|_| Obj::const_none()) + .try_into_option()?; + let extra_items: Option = kwargs + .get(Qstr::MP_QSTR_extra_items) + .unwrap_or_else(|_| Obj::const_none()) + .try_into_option()?; + let extra_title: Option = kwargs + .get(Qstr::MP_QSTR_extra_title) + .unwrap_or_else(|_| Obj::const_none()) + .try_into_option()?; + let verb_cancel: Option = kwargs + .get(Qstr::MP_QSTR_verb_cancel) + .unwrap_or_else(|_| Obj::const_none()) .try_into_option()?; - let fee_items: Obj = kwargs.get(Qstr::MP_QSTR_fee_items)?; - let br_name: TString = kwargs.get(Qstr::MP_QSTR_br_name)?.try_into()?; - let br_code: u16 = kwargs.get(Qstr::MP_QSTR_br_code)?.try_into()?; - let cancel_text: Option = - kwargs.get(Qstr::MP_QSTR_cancel_text)?.try_into_option()?; - let mut summary_params = ShowInfoParams::new(title) + let mut summary_params = ShowInfoParams::new(title.unwrap_or(TString::empty())) .with_menu_button() .with_footer(TR::instructions__swipe_up.into(), None) .with_swipe_up(); - for pair in IterBuf::new().try_iterate(items)? { - let [label, value]: [TString; 2] = util::iter_into_array(pair)?; - summary_params = unwrap!(summary_params.add(label, value)); - } + summary_params = unwrap!(summary_params.add(amount_label, amount)); + summary_params = unwrap!(summary_params.add(fee_label, fee)); - let mut account_params = - ShowInfoParams::new(account_items_title.unwrap_or(TR::send__send_from.into())) - .with_cancel_button(); - for pair in IterBuf::new().try_iterate(account_items)? { - let [label, value]: [TString; 2] = util::iter_into_array(pair)?; - account_params = unwrap!(account_params.add(label, value)); - } - - let mut fee_params = - ShowInfoParams::new(TR::confirm_total__title_fee.into()).with_cancel_button(); - for pair in IterBuf::new().try_iterate(fee_items)? { - let [label, value]: [TString; 2] = util::iter_into_array(pair)?; - fee_params = unwrap!(fee_params.add(label, value)); - } + // collect available info + let account_params = if let Some(items) = account_items { + let mut account_params = + ShowInfoParams::new(TR::send__send_from.into()).with_cancel_button(); + for pair in IterBuf::new().try_iterate(items)? { + let [label, value]: [TString; 2] = util::iter_into_array(pair)?; + account_params = unwrap!(account_params.add(label, value)); + } + Some(account_params) + } else { + None + }; + let extra_params = if let Some(items) = extra_items { + let extra_title = extra_title.unwrap_or(TR::buttons__more_info.into()); + let mut extra_params = ShowInfoParams::new(extra_title).with_cancel_button(); + for pair in IterBuf::new().try_iterate(items)? { + let [label, value]: [TString; 2] = util::iter_into_array(pair)?; + extra_params = unwrap!(extra_params.add(label, value)); + } + Some(extra_params) + } else { + None + }; let flow = flow::new_confirm_summary( summary_params, account_params, - fee_params, - br_name, - br_code, - cancel_text, + extra_params, + extra_title, + verb_cancel, )?; Ok(LayoutObj::new_root(flow)?.into()) }; @@ -761,34 +779,6 @@ extern "C" fn new_confirm_value(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_total(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj { - let block = move |_args: &[Obj], kwargs: &Map| { - let title: TString = kwargs.get(Qstr::MP_QSTR_title)?.try_into()?; - let items: Obj = kwargs.get(Qstr::MP_QSTR_items)?; - - let mut paragraphs = ParagraphVecShort::new(); - - for pair in IterBuf::new().try_iterate(items)? { - let [label, value]: [TString; 2] = util::iter_into_array(pair)?; - paragraphs.add(Paragraph::new(&theme::TEXT_NORMAL, label).no_break()); - paragraphs.add(Paragraph::new(&theme::TEXT_MONO, value)); - } - - new_confirm_action_simple( - paragraphs.into_paragraphs(), - ConfirmActionExtra::Menu(ConfirmActionMenuStrings::new()), - ConfirmActionStrings::new(title, None, None, Some(title)), - true, - None, - 0, - false, - ) - .and_then(LayoutObj::new_root) - .map(Into::into) - }; - unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } -} - extern "C" fn new_confirm_modify_output(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj { let block = move |_args: &[Obj], kwargs: &Map| { let sign: i32 = kwargs.get(Qstr::MP_QSTR_sign)?.try_into()?; @@ -1649,16 +1639,6 @@ pub static mp_module_trezorui2: Module = obj_module! { /// """Confirm value. Merge of confirm_total and confirm_output.""" Qstr::MP_QSTR_confirm_value => obj_fn_kw!(0, new_confirm_value).as_obj(), - /// def confirm_total( - /// *, - /// title: str, - /// items: Iterable[tuple[str, str]], - /// info_button: bool = False, - /// cancel_arrow: bool = False, - /// ) -> LayoutObj[UiResult]: - /// """Transaction summary. Always hold to confirm.""" - Qstr::MP_QSTR_confirm_total => obj_fn_kw!(0, new_confirm_total).as_obj(), - /// def confirm_modify_output( /// *, /// sign: int, @@ -2004,19 +1984,20 @@ pub static mp_module_trezorui2: Module = obj_module! { /// """Confirm the recipient, (optionally) confirm the amount and (optionally) confirm the summary and present a Hold to Sign page.""" Qstr::MP_QSTR_flow_confirm_output => obj_fn_kw!(0, new_confirm_output).as_obj(), - /// def flow_confirm_summary( + /// def confirm_summary( /// *, - /// title: str, - /// items: Iterable[tuple[str, str]], - /// account_items: Iterable[tuple[str, str]], - /// account_items_title: str | None, - /// fee_items: Iterable[tuple[str, str]], - /// br_code: ButtonRequestType, - /// br_name: str, - /// cancel_text: str | None = None, + /// amount: str, + /// amount_label: str, + /// fee: str, + /// fee_label: str, + /// title: str | None = None, + /// account_items: Iterable[tuple[str, str]] | None = None, + /// extra_items: Iterable[tuple[str, str]] | None = None, + /// extra_title: str | None = None, + /// verb_cancel: str | None = None, /// ) -> LayoutObj[UiResult]: - /// """Total summary and hold to confirm.""" - Qstr::MP_QSTR_flow_confirm_summary => obj_fn_kw!(0, new_confirm_summary).as_obj(), + /// """Confirm summary of a transaction.""" + Qstr::MP_QSTR_confirm_summary => obj_fn_kw!(0, new_confirm_summary).as_obj(), /// class BacklightLevels: /// """Backlight levels. Values dynamically update based on user settings.""" diff --git a/core/mocks/generated/trezorui2.pyi b/core/mocks/generated/trezorui2.pyi index 1ab7ab68b7..7e4d3eabd3 100644 --- a/core/mocks/generated/trezorui2.pyi +++ b/core/mocks/generated/trezorui2.pyi @@ -144,17 +144,6 @@ def confirm_value( """Confirm value. Merge of confirm_total and confirm_output.""" -# rust/src/ui/model_mercury/layout.rs -def confirm_total( - *, - title: str, - items: Iterable[tuple[str, str]], - info_button: bool = False, - cancel_arrow: bool = False, -) -> LayoutObj[UiResult]: - """Transaction summary. Always hold to confirm.""" - - # rust/src/ui/model_mercury/layout.rs def confirm_modify_output( *, @@ -534,18 +523,19 @@ def flow_confirm_output( # rust/src/ui/model_mercury/layout.rs -def flow_confirm_summary( +def confirm_summary( *, - title: str, - items: Iterable[tuple[str, str]], - account_items: Iterable[tuple[str, str]], - account_items_title: str | None, - fee_items: Iterable[tuple[str, str]], - br_code: ButtonRequestType, - br_name: str, - cancel_text: str | None = None, + amount: str, + amount_label: str, + fee: str, + fee_label: str, + title: str | None = None, + account_items: Iterable[tuple[str, str]] | None = None, + extra_items: Iterable[tuple[str, str]] | None = None, + extra_title: str | None = None, + verb_cancel: str | None = None, ) -> LayoutObj[UiResult]: - """Total summary and hold to confirm.""" + """Confirm summary of a transaction.""" # rust/src/ui/model_mercury/layout.rs diff --git a/core/src/trezor/ui/layouts/mercury/__init__.py b/core/src/trezor/ui/layouts/mercury/__init__.py index e5d63f54a8..3dd1181887 100644 --- a/core/src/trezor/ui/layouts/mercury/__init__.py +++ b/core/src/trezor/ui/layouts/mercury/__init__.py @@ -686,10 +686,6 @@ def confirm_total( total_label = total_label or TR.send__total_amount # def_arg fee_label = fee_label or TR.send__incl_transaction_fee # def_arg - items = [ - (total_label, total_amount), - (fee_label, fee_amount), - ] fee_items = [] account_items = [] if source_account: @@ -700,44 +696,48 @@ def confirm_total( fee_items.append((TR.confirm_total__fee_rate, fee_rate_amount)) return raise_if_not_confirmed( - trezorui2.flow_confirm_summary( + trezorui2.confirm_summary( + amount=total_amount, + amount_label=total_label, + fee=fee_amount, + fee_label=fee_label, title=title, - items=items, - fee_items=fee_items, - account_items=account_items, - account_items_title=None, - br_name=br_name, - br_code=br_code, - cancel_text=TR.send__cancel_sign, + account_items=account_items or None, + extra_items=fee_items or None, + extra_title=TR.confirm_total__title_fee, ), - None, + br_name, + br_code, ) def _confirm_summary( - items: Iterable[tuple[str, str]] | None = None, + amount: str, + amount_label: str, + fee: str, + fee_label: str, title: str | None = None, - info_items: Iterable[tuple[str, str]] | None = None, - info_title: str | None = None, - fee_items: Iterable[tuple[str, str]] | None = None, + account_items: Iterable[tuple[str, str]] | None = None, + extra_items: Iterable[tuple[str, str]] | None = None, + extra_title: str | None = None, br_name: str = "confirm_total", br_code: ButtonRequestType = ButtonRequestType.SignTx, - cancel_text: str | None = None, ) -> Awaitable[None]: title = title or TR.words__title_summary # def_arg return raise_if_not_confirmed( - trezorui2.flow_confirm_summary( + trezorui2.confirm_summary( + amount=amount, + amount_label=amount_label, + fee=fee, + fee_label=fee_label, title=title, - items=items or (), - fee_items=fee_items or (), - account_items=info_items or (), - account_items_title=info_title, - br_name=br_name, - br_code=br_code, - cancel_text=cancel_text, + account_items=account_items or None, + extra_items=extra_items or None, + extra_title=extra_title or None, ), - None, + br_name, + br_code, ) @@ -853,8 +853,11 @@ if not utils.BITCOIN_ONLY: ) # def_arg fee_title = fee_title or TR.words__fee # def_arg return _confirm_summary( - items=((amount_title, amount), (fee_title, fee)), - info_items=items, + amount, + amount_title, + fee, + fee_title, + extra_items=items, br_name=br_name, br_code=br_code, ) @@ -866,13 +869,13 @@ if not utils.BITCOIN_ONLY: ) -> Awaitable[None]: amount_title = TR.send__total_amount fee_title = TR.send__incl_transaction_fee - more_info_title = TR.buttons__more_info return _confirm_summary( - items=((amount_title, amount), (fee_title, fee)), - info_items=items, - info_title=more_info_title, - fee_items=None, + amount, + amount_title, + fee, + fee_title, + extra_items=items, br_name="confirm_cardano_tx", br_code=ButtonRequestType.SignTx, ) @@ -880,10 +883,10 @@ if not utils.BITCOIN_ONLY: def confirm_joint_total(spending_amount: str, total_amount: str) -> Awaitable[None]: return _confirm_summary( - items=( - (TR.send__you_are_contributing, spending_amount), - (TR.send__to_the_total_amount, total_amount), - ), + spending_amount, + TR.send__you_are_contributing, + total_amount, + TR.send__to_the_total_amount, title=TR.send__title_joint_transaction, br_name="confirm_joint_total", br_code=ButtonRequestType.SignTx,