From b747a5bcc90f5d827eb3cb13f7c53cd73d109664 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ioan=20Biz=C4=83u?= Date: Tue, 20 Aug 2024 13:57:04 +0200 Subject: [PATCH] refactor(core): merge confirm_summary flow into confirm_output flow --- core/embed/rust/librust_qstr.h | 4 + core/embed/rust/src/ui/flow/swipe.rs | 2 +- .../ui/model_mercury/flow/confirm_output.rs | 267 +++++++++++++++--- .../ui/model_mercury/flow/confirm_summary.rs | 1 - .../rust/src/ui/model_mercury/flow/util.rs | 11 + .../embed/rust/src/ui/model_mercury/layout.rs | 7 +- core/mocks/generated/trezorui2.pyi | 7 +- .../src/trezor/ui/layouts/mercury/__init__.py | 44 +-- core/translations/signatures.json | 6 +- tests/input_flows_helpers.py | 2 +- 10 files changed, 293 insertions(+), 58 deletions(-) diff --git a/core/embed/rust/librust_qstr.h b/core/embed/rust/librust_qstr.h index a9e6cd1705..9a5ac8936d 100644 --- a/core/embed/rust/librust_qstr.h +++ b/core/embed/rust/librust_qstr.h @@ -666,6 +666,10 @@ static void _librust_qstrs(void) { MP_QSTR_subprompt; MP_QSTR_subtext; MP_QSTR_subtitle; + MP_QSTR_summary_br_code; + MP_QSTR_summary_br_name; + MP_QSTR_summary_items; + MP_QSTR_summary_title; MP_QSTR_text; MP_QSTR_text_confirm; MP_QSTR_text_info; diff --git a/core/embed/rust/src/ui/flow/swipe.rs b/core/embed/rust/src/ui/flow/swipe.rs index d30598562d..3aa50bd76a 100644 --- a/core/embed/rust/src/ui/flow/swipe.rs +++ b/core/embed/rust/src/ui/flow/swipe.rs @@ -98,7 +98,7 @@ pub struct SwipeFlow { /// Current state of the flow. state: &'static dyn FlowState, /// Store of all screens which are part of the flow. - store: Vec, 10>, + store: Vec, 12>, /// Swipe detector. swipe: SwipeDetect, /// Swipe allowed diff --git a/core/embed/rust/src/ui/model_mercury/flow/confirm_output.rs b/core/embed/rust/src/ui/model_mercury/flow/confirm_output.rs index ee4082c60b..6724f63903 100644 --- a/core/embed/rust/src/ui/model_mercury/flow/confirm_output.rs +++ b/core/embed/rust/src/ui/model_mercury/flow/confirm_output.rs @@ -2,12 +2,14 @@ use heapless::Vec; use crate::{ error, - micropython::{map::Map, obj::Obj, qstr::Qstr, util}, + micropython::{iter::IterBuf, map::Map, obj::Obj, qstr::Qstr, util}, strutil::TString, translations::TR, ui::{ button_request::ButtonRequest, - component::{swipe_detect::SwipeSettings, ButtonRequestExt, ComponentExt, SwipeDirection}, + component::{ + swipe_detect::SwipeSettings, ButtonRequestExt, ComponentExt, MsgMap, SwipeDirection, + }, flow::{ base::{DecisionBuilder as _, StateChange}, FlowMsg, FlowState, SwipeFlow, @@ -24,11 +26,12 @@ use super::{ }, theme, }, - util::ConfirmBlobParams, + util::{ConfirmBlobParams, ShowInfoParams}, }; const MENU_ITEM_CANCEL: usize = 0; -const MENU_ITEM_ACCOUNT_INFO: usize = 1; +const MENU_ITEM_FEE_INFO: usize = 1; +const MENU_ITEM_ACCOUNT_INFO: usize = 2; #[derive(Copy, Clone, PartialEq, Eq)] pub enum ConfirmOutput { @@ -102,8 +105,8 @@ impl FlowState for ConfirmOutputWithAmount { fn handle_event(&'static self, msg: FlowMsg) -> StateChange { match (self, msg) { (_, FlowMsg::Info) => Self::Menu.transit(), - (Self::Menu, FlowMsg::Choice(0)) => Self::AccountInfo.transit(), - (Self::Menu, FlowMsg::Choice(1)) => Self::CancelTap.swipe_left(), + (Self::Menu, FlowMsg::Choice(MENU_ITEM_CANCEL)) => Self::CancelTap.swipe_left(), + (Self::Menu, FlowMsg::Choice(MENU_ITEM_ACCOUNT_INFO)) => Self::AccountInfo.transit(), (Self::Menu, FlowMsg::Cancelled) => Self::Address.swipe_right(), (Self::CancelTap, FlowMsg::Confirmed) => self.return_msg(FlowMsg::Cancelled), (_, FlowMsg::Cancelled) => Self::Menu.transit(), @@ -112,6 +115,97 @@ impl FlowState for ConfirmOutputWithAmount { } } +#[derive(Copy, Clone, PartialEq, Eq)] +pub enum ConfirmOutputWithSummary { + Address, + AddressMenu, + AddressMenuCancel, + Summary, + SummaryMenu, + SummaryMenuCancel, + FeeInfo, + Hold, + HoldMenu, + HoldMenuCancel, + AccountInfo, +} + +impl FlowState for ConfirmOutputWithSummary { + #[inline] + fn index(&'static self) -> usize { + *self as usize + } + + fn handle_swipe(&'static self, direction: SwipeDirection) -> StateChange { + match (self, direction) { + (Self::Address, SwipeDirection::Left) => Self::AddressMenu.swipe(direction), + (Self::Address, SwipeDirection::Up) => Self::Summary.swipe(direction), + (Self::AccountInfo, SwipeDirection::Right) => Self::AddressMenu.swipe(direction), + (Self::Summary, SwipeDirection::Left) => Self::SummaryMenu.swipe(direction), + (Self::Summary, SwipeDirection::Up) => Self::Hold.swipe(direction), + (Self::Summary, SwipeDirection::Down) => Self::Address.swipe(direction), + (Self::FeeInfo, SwipeDirection::Right) => Self::SummaryMenu.swipe(direction), + (Self::Hold, SwipeDirection::Left) => Self::HoldMenu.swipe(direction), + (Self::Hold, SwipeDirection::Down) => Self::Summary.swipe(direction), + _ => self.do_nothing(), + } + } + + fn handle_event(&'static self, msg: FlowMsg) -> StateChange { + match (self, msg) { + (Self::Address, FlowMsg::Info) => Self::AddressMenu.transit(), + (Self::AddressMenu, FlowMsg::Choice(MENU_ITEM_CANCEL)) => { + Self::AddressMenuCancel.swipe_left() + } + (Self::AddressMenuCancel, FlowMsg::Cancelled) => Self::AddressMenu.swipe_right(), + (Self::Summary, FlowMsg::Info) => Self::SummaryMenu.transit(), + (Self::SummaryMenu, FlowMsg::Choice(MENU_ITEM_CANCEL)) => { + Self::SummaryMenuCancel.swipe_left() + } + (Self::SummaryMenuCancel, FlowMsg::Cancelled) => Self::SummaryMenu.swipe_right(), + (Self::Hold, FlowMsg::Info) => Self::HoldMenu.transit(), + (Self::HoldMenu, FlowMsg::Choice(MENU_ITEM_CANCEL)) => { + Self::HoldMenuCancel.swipe_left() + } + (Self::HoldMenuCancel, FlowMsg::Cancelled) => Self::HoldMenu.swipe_right(), + (Self::SummaryMenu, FlowMsg::Choice(MENU_ITEM_FEE_INFO)) => Self::FeeInfo.swipe_left(), + (Self::AddressMenu, FlowMsg::Choice(MENU_ITEM_ACCOUNT_INFO)) => { + Self::AccountInfo.swipe_left() + } + (Self::AddressMenu, FlowMsg::Cancelled) => Self::Address.swipe_right(), + (Self::SummaryMenu, FlowMsg::Cancelled) => Self::Summary.swipe_right(), + (Self::FeeInfo, FlowMsg::Cancelled) => Self::SummaryMenu.swipe_right(), + (Self::HoldMenu, FlowMsg::Cancelled) => Self::Hold.swipe_right(), + ( + Self::AddressMenuCancel | Self::SummaryMenuCancel | Self::HoldMenuCancel, + FlowMsg::Confirmed, + ) => self.return_msg(FlowMsg::Cancelled), + (Self::Address, FlowMsg::Cancelled) => Self::AddressMenu.transit(), + (Self::Summary, FlowMsg::Cancelled) => Self::SummaryMenu.transit(), + (Self::Hold, FlowMsg::Cancelled) => Self::HoldMenu.transit(), + (Self::Hold, FlowMsg::Confirmed) => self.return_msg(FlowMsg::Confirmed), + _ => self.do_nothing(), + } + } +} + +fn get_cancel_page( +) -> MsgMap>, impl Fn(FrameMsg) -> Option> { + Frame::left_aligned( + TR::send__cancel_sign.into(), + SwipeContent::new(PromptScreen::new_tap_to_cancel()), + ) + .with_cancel_button() + .with_footer(TR::instructions__tap_to_confirm.into(), None) + .with_swipe(SwipeDirection::Down, SwipeSettings::default()) + .with_swipe(SwipeDirection::Left, SwipeSettings::default()) + .map(|msg| match msg { + FrameMsg::Content(PromptMsg::Confirmed) => Some(FlowMsg::Confirmed), + FrameMsg::Button(_) => Some(FlowMsg::Cancelled), + _ => None, + }) +} + #[allow(clippy::not_unsafe_ptr_arg_deref)] pub extern "C" fn new_confirm_output(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj { unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, new_confirm_output_obj) } @@ -119,9 +213,11 @@ pub extern "C" fn new_confirm_output(n_args: usize, args: *const Obj, kwargs: *m fn new_confirm_output_obj(_args: &[Obj], kwargs: &Map) -> Result { let title: Option = kwargs.get(Qstr::MP_QSTR_title)?.try_into_option()?; + let account: Option = kwargs.get(Qstr::MP_QSTR_account)?.try_into_option()?; let account_path: Option = kwargs.get(Qstr::MP_QSTR_account_path)?.try_into_option()?; + 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()?; @@ -131,6 +227,18 @@ fn new_confirm_output_obj(_args: &[Obj], kwargs: &Map) -> Result = + kwargs.get(Qstr::MP_QSTR_summary_title)?.try_into_option()?; + let summary_br_name: Option = kwargs + .get(Qstr::MP_QSTR_summary_br_name)? + .try_into_option()?; + let summary_br_code: Option = kwargs + .get(Qstr::MP_QSTR_summary_br_code)? + .try_into_option()?; + let cancel_text: Option = kwargs.get(Qstr::MP_QSTR_cancel_text)?.try_into_option()?; // Address @@ -143,27 +251,27 @@ fn new_confirm_output_obj(_args: &[Obj], kwargs: &Map) -> Result::new(); + // AddressMenu + let mut address_menu = VerticalMenu::empty(); + let mut address_menu_items = Vec::::new(); if account.is_some() && account_path.is_some() { - menu = menu.item( + address_menu = address_menu.item( theme::ICON_CHEVRON_RIGHT, TR::address_details__account_info.into(), ); - unwrap!(menu_items.push(MENU_ITEM_ACCOUNT_INFO)); + unwrap!(address_menu_items.push(MENU_ITEM_ACCOUNT_INFO)); } - menu = menu.danger( + address_menu = address_menu.danger( theme::ICON_CANCEL, cancel_text.unwrap_or(TR::send__cancel_sign.into()), ); - unwrap!(menu_items.push(MENU_ITEM_CANCEL)); - let content_menu = Frame::left_aligned(TString::empty(), menu) + unwrap!(address_menu_items.push(MENU_ITEM_CANCEL)); + let content_address_menu = Frame::left_aligned(TString::empty(), address_menu) .with_cancel_button() .with_swipe(SwipeDirection::Right, SwipeSettings::immediate()) .map(move |msg| match msg { FrameMsg::Content(VerticalMenuChoiceMsg::Selected(i)) => { - let selected_item = menu_items[i]; + let selected_item = address_menu_items[i]; Some(FlowMsg::Choice(selected_item)) } FrameMsg::Button(_) => Some(FlowMsg::Cancelled), @@ -173,21 +281,6 @@ fn new_confirm_output_obj(_args: &[Obj], kwargs: &Map) -> Result Some(FlowMsg::Confirmed), - FrameMsg::Button(_) => Some(FlowMsg::Cancelled), - _ => None, - }); - let res = if amount.is_some() { let content_amount = ConfirmBlobParams::new( TR::words__amount.into(), @@ -205,15 +298,121 @@ fn new_confirm_output_obj(_args: &[Obj], kwargs: &Map) -> Result Some(FlowMsg::Confirmed), + FrameMsg::Button(_) => Some(FlowMsg::Info), + _ => None, + }); + + // FeeInfo + let mut has_fee_info = false; + let mut fee = ShowInfoParams::new(TR::confirm_total__title_fee.into()).with_cancel_button(); + if fee_items != Obj::const_none() { + for pair in IterBuf::new().try_iterate(fee_items)? { + let [label, value]: [TString; 2] = util::iter_into_array(pair)?; + fee = unwrap!(fee.add(label, value)); + has_fee_info = true; + } + } + let content_fee = fee.into_layout()?; + + // SummaryMenu + let mut summary_menu = VerticalMenu::empty(); + let mut summary_menu_items = Vec::::new(); + if has_fee_info { + summary_menu = summary_menu.item( + theme::ICON_CHEVRON_RIGHT, + TR::confirm_total__title_fee.into(), + ); + unwrap!(summary_menu_items.push(MENU_ITEM_FEE_INFO)); + } + summary_menu = summary_menu.danger( + theme::ICON_CANCEL, + cancel_text.unwrap_or(TR::send__cancel_sign.into()), + ); + unwrap!(summary_menu_items.push(MENU_ITEM_CANCEL)); + let content_summary_menu = Frame::left_aligned(TString::empty(), summary_menu) + .with_cancel_button() + .with_swipe(SwipeDirection::Right, SwipeSettings::immediate()) + .map(move |msg| match msg { + FrameMsg::Content(VerticalMenuChoiceMsg::Selected(i)) => { + let selected_item = summary_menu_items[i]; + Some(FlowMsg::Choice(selected_item)) + } + FrameMsg::Button(_) => Some(FlowMsg::Cancelled), + }); + + // HoldMenu + let hold_menu = VerticalMenu::empty().danger( + theme::ICON_CANCEL, + cancel_text.unwrap_or(TR::send__cancel_sign.into()), + ); + let content_hold_menu = Frame::left_aligned(TString::empty(), hold_menu) + .with_cancel_button() + .with_swipe(SwipeDirection::Right, SwipeSettings::immediate()) + .map(move |msg| match msg { + FrameMsg::Content(VerticalMenuChoiceMsg::Selected(_)) => { + Some(FlowMsg::Choice(MENU_ITEM_CANCEL)) + } + FrameMsg::Button(_) => Some(FlowMsg::Cancelled), + }); + + SwipeFlow::new(&ConfirmOutputWithSummary::Address)? + .with_page(&ConfirmOutputWithSummary::Address, content_address)? + .with_page(&ConfirmOutputWithSummary::AddressMenu, content_address_menu)? + .with_page( + &ConfirmOutputWithSummary::AddressMenuCancel, + get_cancel_page(), + )? + .with_page(&ConfirmOutputWithSummary::Summary, content_summary)? + .with_page(&ConfirmOutputWithSummary::SummaryMenu, content_summary_menu)? + .with_page( + &ConfirmOutputWithSummary::SummaryMenuCancel, + get_cancel_page(), + )? + .with_page(&ConfirmOutputWithSummary::FeeInfo, content_fee)? + .with_page(&ConfirmOutputWithSummary::Hold, content_hold)? + .with_page(&ConfirmOutputWithSummary::HoldMenu, content_hold_menu)? + .with_page(&ConfirmOutputWithSummary::HoldMenuCancel, get_cancel_page())? + .with_page(&ConfirmOutputWithSummary::AccountInfo, content_account)? } else { SwipeFlow::new(&ConfirmOutput::Address)? .with_page(&ConfirmOutput::Address, content_address)? - .with_page(&ConfirmOutput::Menu, content_menu)? + .with_page(&ConfirmOutput::Menu, content_address_menu)? .with_page(&ConfirmOutput::AccountInfo, content_account)? - .with_page(&ConfirmOutput::CancelTap, content_cancel_tap)? + .with_page(&ConfirmOutput::CancelTap, get_cancel_page())? }; Ok(LayoutObj::new(res)?.into()) 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 b30908cf0c..a5766390f2 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 @@ -53,7 +53,6 @@ impl FlowState for ConfirmSummary { (Self::Summary, SwipeDirection::Up) => Self::Hold.swipe(direction), (Self::Hold, SwipeDirection::Down) => Self::Summary.swipe(direction), (Self::Menu, SwipeDirection::Right) => Self::Summary.swipe(direction), - (Self::Menu, SwipeDirection::Left) => Self::FeeInfo.swipe(direction), (Self::AccountInfo | Self::FeeInfo | Self::CancelTap, SwipeDirection::Right) => { Self::Menu.swipe(direction) } diff --git a/core/embed/rust/src/ui/model_mercury/flow/util.rs b/core/embed/rust/src/ui/model_mercury/flow/util.rs index dbcdbef0d4..0cf9e5d979 100644 --- a/core/embed/rust/src/ui/model_mercury/flow/util.rs +++ b/core/embed/rust/src/ui/model_mercury/flow/util.rs @@ -153,6 +153,7 @@ pub struct ShowInfoParams { footer_description: Option>, chunkify: bool, swipe_up: bool, + swipe_down: bool, items: Vec<(TString<'static>, TString<'static>), 4>, } @@ -167,6 +168,7 @@ impl ShowInfoParams { footer_description: None, chunkify: false, swipe_up: false, + swipe_down: false, items: Vec::new(), } } @@ -218,6 +220,11 @@ impl ShowInfoParams { self } + pub const fn with_swipe_down(mut self) -> Self { + self.swipe_down = true; + self + } + #[inline(never)] pub fn into_layout( self, @@ -268,6 +275,10 @@ impl ShowInfoParams { frame = frame.with_swipe(SwipeDirection::Up, SwipeSettings::default()); } + if self.swipe_down { + frame = frame.with_swipe(SwipeDirection::Down, SwipeSettings::default()); + } + frame = frame.with_vertical_pages(); Ok(frame.map(move |msg| { diff --git a/core/embed/rust/src/ui/model_mercury/layout.rs b/core/embed/rust/src/ui/model_mercury/layout.rs index 8f14345ebf..154ca77bb9 100644 --- a/core/embed/rust/src/ui/model_mercury/layout.rs +++ b/core/embed/rust/src/ui/model_mercury/layout.rs @@ -1764,9 +1764,14 @@ pub static mp_module_trezorui2: Module = obj_module! { /// account_path: str | None, /// br_code: ButtonRequestType, /// br_name: str, + /// summary_items: Iterable[tuple[str, str]] | None = None, + /// fee_items: Iterable[tuple[str, str]] | None = None, + /// summary_title: str | None = None, + /// summary_br_code: ButtonRequestType | None = None, + /// summary_br_name: str | None = None, /// cancel_text: str | None = None, /// ) -> LayoutObj[UiResult]: - /// """Confirm recipient.""" + /// """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, flow::new_confirm_output).as_obj(), /// def flow_confirm_summary( diff --git a/core/mocks/generated/trezorui2.pyi b/core/mocks/generated/trezorui2.pyi index 20bc5f2100..0f251b98ab 100644 --- a/core/mocks/generated/trezorui2.pyi +++ b/core/mocks/generated/trezorui2.pyi @@ -585,9 +585,14 @@ def flow_confirm_output( account_path: str | None, br_code: ButtonRequestType, br_name: str, + summary_items: Iterable[tuple[str, str]] | None = None, + fee_items: Iterable[tuple[str, str]] | None = None, + summary_title: str | None = None, + summary_br_code: ButtonRequestType | None = None, + summary_br_name: str | None = None, cancel_text: str | None = None, ) -> LayoutObj[UiResult]: - """Confirm recipient.""" + """Confirm the recipient, (optionally) confirm the amount and (optionally) confirm the summary and present a Hold to Sign page.""" # 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 e2a522f2e3..60c0d9afbc 100644 --- a/core/src/trezor/ui/layouts/mercury/__init__.py +++ b/core/src/trezor/ui/layouts/mercury/__init__.py @@ -648,14 +648,19 @@ async def confirm_output( await raise_if_not_confirmed( RustLayout( trezorui2.flow_confirm_output( + title=title, address=address, amount=amount, - title=title, chunkify=chunkify, account=source_account, account_path=source_account_path, br_code=br_code, br_name="confirm_output", + summary_items=None, + fee_items=None, + summary_title=None, + summary_br_name=None, + summary_br_code=None, cancel_text=cancel_text, ) ) @@ -1023,21 +1028,28 @@ if not utils.BITCOIN_ONLY: br_code: ButtonRequestType = ButtonRequestType.SignTx, chunkify: bool = False, ) -> None: - await confirm_output( - recipient, - title=TR.words__recipient, - chunkify=chunkify, - cancel_text=TR.buttons__cancel, - br_code=ButtonRequestType.Other, - ) - - await _confirm_summary( - items=( - (TR.words__amount, total_amount), - (TR.send__maximum_fee, maximum_fee), - ), - fee_items=fee_info_items, - cancel_text=TR.buttons__cancel, + await raise_if_not_confirmed( + RustLayout( + trezorui2.flow_confirm_output( + title=TR.words__recipient, + address=recipient, + amount=None, + chunkify=chunkify, + account=None, + account_path=None, + br_code=ButtonRequestType.Other, + br_name="confirm_output", + summary_items=( + (TR.words__amount, total_amount), + (TR.send__maximum_fee, maximum_fee), + ), + fee_items=fee_info_items, + summary_title=TR.words__title_summary, + summary_br_name="confirm_total", + summary_br_code=ButtonRequestType.SignTx, + cancel_text=TR.buttons__cancel, + ) + ) ) async def confirm_ethereum_staking_tx( diff --git a/core/translations/signatures.json b/core/translations/signatures.json index faf8f88322..accc1df2d4 100644 --- a/core/translations/signatures.json +++ b/core/translations/signatures.json @@ -1,8 +1,8 @@ { "current": { - "merkle_root": "14c1d561c00b83c685a89d37be68d94dae0fb8b88835d346c877e6246fe022e3", - "datetime": "2024-08-09T07:44:08.162423", - "commit": "89a3ce360514eff165fe0fcb0c2a218b88d85e07" + "merkle_root": "ec4c2be9904443c49bf659e69670ffb2d979fa9e4335fb3cda666a08ce27c16c", + "datetime": "2024-08-22T07:03:54.917145", + "commit": "bfa6f2c3e44156eea17c08bb09ce12f2307bcc0f" }, "history": [ { diff --git a/tests/input_flows_helpers.py b/tests/input_flows_helpers.py index c0df5e7e9b..7abe27fcfa 100644 --- a/tests/input_flows_helpers.py +++ b/tests/input_flows_helpers.py @@ -454,7 +454,7 @@ class EthereumFlow: if cancel: self.debug.press_no() else: - self.debug.press_yes() + self.debug.swipe_up() yield TR.assert_equals( self.debug.wait_layout().title(), "words__title_summary"