From 487d7c4776a1b02b2e88c5f2f529fc02e69cf155 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ioan=20Biz=C4=83u?= Date: Mon, 11 Nov 2024 16:59:13 +0100 Subject: [PATCH] refactor(core): use warning_hi_prio in ETH flow Commit c300576d6c48662d2d83727cfbc0fb2dd5b83c23 introduced the `default_cancel` parameter to `show_warning` and `confirm_blob` rather using the already existing `flow_warning_hi_prio` which was doing the same thing. This commit reverts all the nonsense. [no changelog] --- core/embed/rust/librust_qstr.h | 1 - .../ui/model_mercury/flow/confirm_action.rs | 108 +++--------------- .../rust/src/ui/model_mercury/flow/mod.rs | 3 +- .../ui/model_mercury/flow/warning_hi_prio.rs | 11 +- .../embed/rust/src/ui/model_mercury/layout.rs | 24 +--- core/embed/rust/src/ui/model_tr/layout.rs | 1 - core/embed/rust/src/ui/model_tt/layout.rs | 1 - core/mocks/generated/trezorui2.pyi | 4 +- core/src/apps/ethereum/layout.py | 13 +-- .../src/trezor/ui/layouts/mercury/__init__.py | 62 +++++----- core/src/trezor/ui/layouts/tr/__init__.py | 8 +- core/src/trezor/ui/layouts/tt/__init__.py | 21 +++- 12 files changed, 90 insertions(+), 167 deletions(-) diff --git a/core/embed/rust/librust_qstr.h b/core/embed/rust/librust_qstr.h index 561813c912..25a9b233c8 100644 --- a/core/embed/rust/librust_qstr.h +++ b/core/embed/rust/librust_qstr.h @@ -224,7 +224,6 @@ static void _librust_qstrs(void) { MP_QSTR_debug__loading_seed; MP_QSTR_debug__loading_seed_not_recommended; MP_QSTR_decode; - MP_QSTR_default_cancel; MP_QSTR_deinit; MP_QSTR_description; MP_QSTR_description_font_green; diff --git a/core/embed/rust/src/ui/model_mercury/flow/confirm_action.rs b/core/embed/rust/src/ui/model_mercury/flow/confirm_action.rs index 00eae93e56..d331ac1cbd 100644 --- a/core/embed/rust/src/ui/model_mercury/flow/confirm_action.rs +++ b/core/embed/rust/src/ui/model_mercury/flow/confirm_action.rs @@ -96,40 +96,6 @@ impl FlowController for ConfirmActionSimple { } } -/// Flow similar to ConfirmActionSimple, but having swipe up cancel the flow -/// rather than confirm. To confirm, the user needs to open the menu. -#[derive(Copy, Clone, PartialEq, Eq)] -pub enum ConfirmActionSimpleDefaultCancel { - Intro, - Menu, -} - -impl FlowController for ConfirmActionSimpleDefaultCancel { - #[inline] - fn index(&'static self) -> usize { - *self as usize - } - - fn handle_swipe(&'static self, direction: Direction) -> Decision { - match (self, direction) { - (Self::Intro, Direction::Left) => Self::Menu.swipe(direction), - (Self::Menu, Direction::Right) => Self::Intro.swipe(direction), - (Self::Intro, Direction::Up) => self.return_msg(FlowMsg::Cancelled), - _ => self.do_nothing(), - } - } - - fn handle_event(&'static self, msg: FlowMsg) -> Decision { - match (self, msg) { - (Self::Intro, FlowMsg::Info) => Self::Menu.goto(), - (Self::Menu, FlowMsg::Cancelled) => Self::Intro.swipe_right(), - (Self::Menu, FlowMsg::Choice(0)) => self.return_msg(FlowMsg::Cancelled), - (Self::Menu, FlowMsg::Choice(1)) => self.return_msg(FlowMsg::Confirmed), - _ => self.do_nothing(), - } - } -} - pub struct ConfirmActionMenu { verb_cancel: Option>, info: bool, @@ -231,28 +197,16 @@ fn new_confirm_action_uni( menu: ConfirmActionMenu, strings: ConfirmActionStrings, hold: bool, - default_cancel: bool, ) -> Result { let (prompt_screen, prompt_pages, flow, page) = - create_flow(strings.title, strings.prompt_screen, hold, default_cancel); + create_flow(strings.title, strings.prompt_screen, hold); let mut content_intro = Frame::left_aligned(strings.title, content) .with_swipe(Direction::Up, SwipeSettings::default()) .with_swipe(Direction::Left, SwipeSettings::default()) - .with_vertical_pages(); - - if default_cancel { - content_intro = content_intro.title_styled(theme::TEXT_WARNING); - content_intro = content_intro.with_danger_menu_button(); - content_intro = content_intro.with_footer( - TR::instructions__swipe_up.into(), - Some(TR::send__cancel_sign.into()), - ); - } else { - content_intro = content_intro.with_menu_button(); - // TODO: conditionally add the verb to the footer as well? - content_intro = content_intro.with_footer(TR::instructions__swipe_up.into(), None); - } + .with_vertical_pages() + .with_menu_button() + .with_footer(TR::instructions__swipe_up.into(), None); if let Some(subtitle) = strings.subtitle { content_intro = content_intro.with_subtitle(subtitle); @@ -267,7 +221,7 @@ fn new_confirm_action_uni( let flow = flow?.with_page(page, content_intro)?; - let flow = create_menu(flow, menu, default_cancel, prompt_screen)?; + let flow = create_menu(flow, menu, prompt_screen)?; let flow = create_confirm(flow, strings.subtitle, hold, prompt_screen)?; @@ -278,7 +232,6 @@ fn create_flow( title: TString<'static>, prompt_screen: Option>, hold: bool, - default_cancel: bool, ) -> ( Option>, usize, @@ -290,11 +243,6 @@ fn create_flow( let (flow, page): (Result, &dyn FlowController) = if prompt_screen.is_some() { (SwipeFlow::new(&ConfirmAction::Intro), &ConfirmAction::Intro) - } else if default_cancel { - ( - SwipeFlow::new(&ConfirmActionSimpleDefaultCancel::Intro), - &ConfirmActionSimpleDefaultCancel::Intro, - ) } else { ( SwipeFlow::new(&ConfirmActionSimple::Intro), @@ -308,29 +256,19 @@ fn create_flow( fn create_menu( flow: SwipeFlow, menu: ConfirmActionMenu, - default_cancel: bool, prompt_screen: Option>, ) -> Result { - let mut menu_choices = VerticalMenu::empty(); - if default_cancel { + let mut menu_choices = VerticalMenu::empty().danger( + theme::ICON_CANCEL, + menu.verb_cancel.unwrap_or(TR::buttons__cancel.into()), + ); + + if menu.info { menu_choices = menu_choices.item( - theme::ICON_CANCEL, - menu.verb_cancel.unwrap_or(TR::buttons__cancel.into()), + theme::ICON_CHEVRON_RIGHT, + menu.verb_info + .unwrap_or(TR::words__title_information.into()), ); - menu_choices = - menu_choices.danger(theme::ICON_CHEVRON_RIGHT, TR::words__continue_anyway.into()); - } else { - menu_choices = menu_choices.danger( - theme::ICON_CANCEL, - menu.verb_cancel.unwrap_or(TR::buttons__cancel.into()), - ); - if menu.info { - menu_choices = menu_choices.item( - theme::ICON_CHEVRON_RIGHT, - menu.verb_info - .unwrap_or(TR::words__title_information.into()), - ); - } } let content_menu = Frame::left_aligned("".into(), menu_choices) @@ -403,23 +341,5 @@ pub fn new_confirm_action_simple menu, strings, hold, - false, - ) -} - -#[inline(never)] -pub fn new_confirm_action_simple_default_cancel( - content: T, - menu: ConfirmActionMenu, - strings: ConfirmActionStrings, - hold: bool, - page_limit: Option, -) -> Result { - new_confirm_action_uni( - SwipeContent::new(SwipePage::vertical(content).with_limit(page_limit)), - menu, - strings, - hold, - true, ) } diff --git a/core/embed/rust/src/ui/model_mercury/flow/mod.rs b/core/embed/rust/src/ui/model_mercury/flow/mod.rs index c00df2ef2b..91c1472367 100644 --- a/core/embed/rust/src/ui/model_mercury/flow/mod.rs +++ b/core/embed/rust/src/ui/model_mercury/flow/mod.rs @@ -20,8 +20,7 @@ pub mod warning_hi_prio; mod util; pub use confirm_action::{ - new_confirm_action, new_confirm_action_simple, new_confirm_action_simple_default_cancel, - ConfirmActionMenu, ConfirmActionStrings, + new_confirm_action, new_confirm_action_simple, ConfirmActionMenu, ConfirmActionStrings, }; #[cfg(feature = "universal_fw")] pub use confirm_fido::new_confirm_fido; diff --git a/core/embed/rust/src/ui/model_mercury/flow/warning_hi_prio.rs b/core/embed/rust/src/ui/model_mercury/flow/warning_hi_prio.rs index 89302b6ace..5eeceae844 100644 --- a/core/embed/rust/src/ui/model_mercury/flow/warning_hi_prio.rs +++ b/core/embed/rust/src/ui/model_mercury/flow/warning_hi_prio.rs @@ -70,10 +70,15 @@ impl WarningHiPrio { let title: TString = kwargs.get_or(Qstr::MP_QSTR_title, TR::words__warning.into())?; let description: TString = kwargs.get(Qstr::MP_QSTR_description)?.try_into()?; let value: TString = kwargs.get_or(Qstr::MP_QSTR_value, "".into())?; - let cancel: TString = TR::words__cancel_and_exit.into(); + let verb_cancel: Option = kwargs + .get(Qstr::MP_QSTR_verb_cancel) + .unwrap_or_else(|_| Obj::const_none()) + .try_into_option()?; let confirm: TString = TR::words__continue_anyway.into(); let done_title: TString = TR::words__operation_cancelled.into(); + let verb_cancel = verb_cancel.unwrap_or(TR::words__cancel_and_exit.into()); + // Message let paragraphs = [ Paragraph::new(&theme::TEXT_MAIN_GREY_LIGHT, description), @@ -83,7 +88,7 @@ impl WarningHiPrio { .into_paragraphs(); let content_message = Frame::left_aligned(title, SwipeContent::new(paragraphs)) .with_menu_button() - .with_footer(TR::instructions__swipe_up.into(), Some(cancel)) + .with_footer(TR::instructions__swipe_up.into(), Some(verb_cancel)) .with_danger() .with_swipe(Direction::Up, SwipeSettings::default()) .with_swipe(Direction::Left, SwipeSettings::default()) @@ -94,7 +99,7 @@ impl WarningHiPrio { let content_menu = Frame::left_aligned( "".into(), VerticalMenu::empty() - .item(theme::ICON_CANCEL, TR::buttons__cancel.into()) + .item(theme::ICON_CANCEL, verb_cancel) .danger(theme::ICON_CHEVRON_RIGHT, confirm), ) .with_cancel_button() diff --git a/core/embed/rust/src/ui/model_mercury/layout.rs b/core/embed/rust/src/ui/model_mercury/layout.rs index 1bd45f2b7f..b5e3b37302 100644 --- a/core/embed/rust/src/ui/model_mercury/layout.rs +++ b/core/embed/rust/src/ui/model_mercury/layout.rs @@ -51,10 +51,7 @@ use crate::{ }, model_mercury::{ component::{check_homescreen_format, SwipeContent}, - flow::{ - new_confirm_action_simple, new_confirm_action_simple_default_cancel, - ConfirmActionMenu, ConfirmActionStrings, - }, + flow::{new_confirm_action_simple, ConfirmActionMenu, ConfirmActionStrings}, theme::ICON_BULLET_CHECKMARK, }, }, @@ -269,7 +266,6 @@ struct ConfirmBlobParams { chunkify: bool, text_mono: bool, page_limit: Option, - default_cancel: bool, } impl ConfirmBlobParams { @@ -298,7 +294,6 @@ impl ConfirmBlobParams { chunkify: false, text_mono: true, page_limit: None, - default_cancel: false, } } @@ -337,11 +332,6 @@ impl ConfirmBlobParams { self } - fn with_default_cancel(mut self, default_cancel: bool) -> Self { - self.default_cancel = default_cancel; - self - } - fn with_description_font(mut self, description_font: &'static TextStyle) -> Self { self.description_font = description_font; self @@ -365,13 +355,7 @@ impl ConfirmBlobParams { } .into_paragraphs(); - let build_flow = if self.default_cancel { - new_confirm_action_simple_default_cancel - } else { - new_confirm_action_simple - }; - - build_flow( + new_confirm_action_simple( paragraphs, ConfirmActionMenu::new(self.verb_cancel, self.info_button, self.verb_info), ConfirmActionStrings::new( @@ -419,7 +403,6 @@ extern "C" fn new_confirm_blob(n_args: usize, args: *const Obj, kwargs: *mut Map let hold: bool = kwargs.get_or(Qstr::MP_QSTR_hold, false)?; let chunkify: bool = kwargs.get_or(Qstr::MP_QSTR_chunkify, false)?; let prompt_screen: bool = kwargs.get_or(Qstr::MP_QSTR_prompt_screen, true)?; - let default_cancel: bool = kwargs.get_or(Qstr::MP_QSTR_default_cancel, false)?; let page_limit: Option = kwargs .get(Qstr::MP_QSTR_page_limit) .unwrap_or_else(|_| Obj::const_none()) @@ -447,7 +430,6 @@ extern "C" fn new_confirm_blob(n_args: usize, args: *const Obj, kwargs: *mut Map .with_extra(extra) .with_info_button(info) .with_chunkify(chunkify) - .with_default_cancel(default_cancel) .with_page_limit(page_limit) .into_flow() }; @@ -1277,7 +1259,6 @@ pub static mp_module_trezorui2: Module = obj_module! { /// hold: bool = False, /// chunkify: bool = False, /// prompt_screen: bool = False, - /// default_cancel: bool = False, /// page_limit: int | None = None, /// ) -> LayoutObj[UiResult]: /// """Confirm byte sequence data.""" @@ -1681,6 +1662,7 @@ pub static mp_module_trezorui2: Module = obj_module! { /// title: str, /// description: str, /// value: str = "", + /// verb_cancel: str | None = None, /// ) -> LayoutObj[UiResult]: /// """Warning modal with multiple steps to confirm.""" Qstr::MP_QSTR_flow_warning_hi_prio => obj_fn_kw!(0, flow::warning_hi_prio::new_warning_hi_prio).as_obj(), diff --git a/core/embed/rust/src/ui/model_tr/layout.rs b/core/embed/rust/src/ui/model_tr/layout.rs index e7cb669367..313b7b195a 100644 --- a/core/embed/rust/src/ui/model_tr/layout.rs +++ b/core/embed/rust/src/ui/model_tr/layout.rs @@ -1752,7 +1752,6 @@ pub static mp_module_trezorui2: Module = obj_module! { /// hold: bool = False, /// chunkify: bool = False, /// prompt_screen: bool = False, - /// default_cancel: bool = False, /// page_limit: int | None = None, /// ) -> LayoutObj[UiResult]: /// """Confirm byte sequence data.""" diff --git a/core/embed/rust/src/ui/model_tt/layout.rs b/core/embed/rust/src/ui/model_tt/layout.rs index 2b5d3aa401..b3b2e403e9 100644 --- a/core/embed/rust/src/ui/model_tt/layout.rs +++ b/core/embed/rust/src/ui/model_tt/layout.rs @@ -1805,7 +1805,6 @@ pub static mp_module_trezorui2: Module = obj_module! { /// hold: bool = False, /// chunkify: bool = False, /// prompt_screen: bool = False, - /// default_cancel: bool = False, /// page_limit: int | None = None, /// ) -> LayoutObj[UiResult]: /// """Confirm byte sequence data.""" diff --git a/core/mocks/generated/trezorui2.pyi b/core/mocks/generated/trezorui2.pyi index 1101c7752d..2f2d5a71a3 100644 --- a/core/mocks/generated/trezorui2.pyi +++ b/core/mocks/generated/trezorui2.pyi @@ -70,7 +70,6 @@ def confirm_blob( hold: bool = False, chunkify: bool = False, prompt_screen: bool = False, - default_cancel: bool = False, page_limit: int | None = None, ) -> LayoutObj[UiResult]: """Confirm byte sequence data.""" @@ -513,6 +512,7 @@ def flow_warning_hi_prio( title: str, description: str, value: str = "", + verb_cancel: str | None = None, ) -> LayoutObj[UiResult]: """Warning modal with multiple steps to confirm.""" @@ -643,7 +643,6 @@ def confirm_blob( hold: bool = False, chunkify: bool = False, prompt_screen: bool = False, - default_cancel: bool = False, page_limit: int | None = None, ) -> LayoutObj[UiResult]: """Confirm byte sequence data.""" @@ -1216,7 +1215,6 @@ def confirm_blob( hold: bool = False, chunkify: bool = False, prompt_screen: bool = False, - default_cancel: bool = False, page_limit: int | None = None, ) -> LayoutObj[UiResult]: """Confirm byte sequence data.""" diff --git a/core/src/apps/ethereum/layout.py b/core/src/apps/ethereum/layout.py index a791eea2a6..2e2eca8932 100644 --- a/core/src/apps/ethereum/layout.py +++ b/core/src/apps/ethereum/layout.py @@ -145,16 +145,13 @@ async def require_confirm_claim( async def require_confirm_unknown_token(address_bytes: bytes) -> None: from ubinascii import hexlify - from trezor.ui.layouts import confirm_address, show_warning - - await show_warning( - "unknown_contract_warning", - TR.ethereum__unknown_contract_address, - default_cancel=True, - verb_cancel=TR.send__cancel_sign, - br_code=ButtonRequestType.Other, + from trezor.ui.layouts import ( + confirm_address, + confirm_ethereum_unknown_contract_warning, ) + await confirm_ethereum_unknown_contract_warning() + contract_address_hex = "0x" + hexlify(address_bytes).decode() await confirm_address( TR.words__address, diff --git a/core/src/trezor/ui/layouts/mercury/__init__.py b/core/src/trezor/ui/layouts/mercury/__init__.py index 97cbd8d72d..fa97b53b0e 100644 --- a/core/src/trezor/ui/layouts/mercury/__init__.py +++ b/core/src/trezor/ui/layouts/mercury/__init__.py @@ -301,36 +301,19 @@ def show_warning( content: str, subheader: str | None = None, button: str | None = None, - default_cancel: bool = False, - verb_cancel: str | None = None, br_code: ButtonRequestType = ButtonRequestType.Warning, ) -> Awaitable[None]: button = button or TR.buttons__continue # def_arg - if default_cancel: - # a kind of warning which makes it easy (swipe up) to cancel - # and makes it harder to continue - return confirm_blob( - br_name, - TR.words__warning, - content, - text_mono=False, - verb_cancel=verb_cancel, - default_cancel=True, - prompt_screen=False, - br_code=br_code, - ) - else: - # traditional warning - return raise_if_not_confirmed( - trezorui2.show_warning( - title=TR.words__important, - value=content, - button=subheader or TR.words__continue_anyway_question, - danger=True, - ), - br_name, - br_code, - ) + return raise_if_not_confirmed( + trezorui2.show_warning( + title=TR.words__important, + value=content, + button=subheader or TR.words__continue_anyway_question, + danger=True, + ), + br_name, + br_code, + ) def show_success( @@ -518,7 +501,6 @@ def confirm_blob( hold: bool = False, br_code: ButtonRequestType = BR_CODE_OTHER, chunkify: bool = False, - default_cancel: bool = False, prompt_screen: bool = True, ) -> Awaitable[None]: layout = trezorui2.confirm_blob( @@ -533,7 +515,6 @@ def confirm_blob( hold=hold, chunkify=chunkify, prompt_screen=prompt_screen, - default_cancel=default_cancel, ) return raise_if_not_confirmed( layout, @@ -739,6 +720,29 @@ def _confirm_summary( if not utils.BITCOIN_ONLY: + def confirm_other_data(data: bytes, data_total: int) -> Awaitable[None]: + return confirm_blob( + "confirm_data", + TR.ethereum__title_input_data, + data, + TR.ethereum__data_size_template.format(data_total), + verb=TR.buttons__confirm, + verb_cancel=TR.send__cancel_sign, + br_code=ButtonRequestType.SignTx, + ask_pagination=True, + ) + + def confirm_ethereum_unknown_contract_warning() -> Awaitable[None]: + return raise_if_not_confirmed( + trezorui2.flow_warning_hi_prio( + title=TR.words__warning, + description=TR.ethereum__unknown_contract_address, + verb_cancel=TR.send__cancel_sign, + ), + "unknown_contract_warning", + ButtonRequestType.Warning, + ) + async def confirm_ethereum_tx( recipient: str | None, total_amount: str, diff --git a/core/src/trezor/ui/layouts/tr/__init__.py b/core/src/trezor/ui/layouts/tr/__init__.py index 3251804417..cb053ff84b 100644 --- a/core/src/trezor/ui/layouts/tr/__init__.py +++ b/core/src/trezor/ui/layouts/tr/__init__.py @@ -385,7 +385,6 @@ def show_warning( content: str, subheader: str | None = None, button: str | None = None, - default_cancel: bool = False, # NB: model R does not implement "default_cancel"-style warnings verb_cancel: str | None = None, br_code: ButtonRequestType = ButtonRequestType.Warning, exc: ExceptionType | None = ActionCancelled, @@ -588,7 +587,6 @@ def confirm_blob( hold: bool = False, br_code: ButtonRequestType = BR_CODE_OTHER, chunkify: bool = False, - default_cancel: bool = False, prompt_screen: bool = True, ask_pagination: bool = False, ) -> Awaitable[None]: @@ -831,6 +829,12 @@ def confirm_total( if not utils.BITCOIN_ONLY: + def confirm_ethereum_unknown_contract_warning() -> Awaitable[ui.UiResult]: + return show_warning( + "unknown_contract_warning", + TR.ethereum__unknown_contract_address, + ) + async def confirm_ethereum_staking_tx( title: str, intro_question: str, diff --git a/core/src/trezor/ui/layouts/tt/__init__.py b/core/src/trezor/ui/layouts/tt/__init__.py index e11e6eabf1..2bfae1feb9 100644 --- a/core/src/trezor/ui/layouts/tt/__init__.py +++ b/core/src/trezor/ui/layouts/tt/__init__.py @@ -349,7 +349,6 @@ def show_warning( content: str, subheader: str | None = None, button: str | None = None, - default_cancel: bool = False, # NB: model T does not implement "default_cancel"-style warnings verb_cancel: str | None = None, br_code: ButtonRequestType = ButtonRequestType.Warning, ) -> Awaitable[None]: @@ -590,7 +589,6 @@ def confirm_blob( hold: bool = False, br_code: ButtonRequestType = BR_CODE_OTHER, chunkify: bool = False, - default_cancel: bool = False, prompt_screen: bool = True, ) -> Awaitable[None]: verb = verb or TR.buttons__confirm # def_arg @@ -793,6 +791,25 @@ def _confirm_summary( if not utils.BITCOIN_ONLY: + def confirm_other_data(data: bytes, data_total: int) -> Awaitable[None]: + return confirm_blob( + "confirm_data", + TR.ethereum__title_input_data, + data, + TR.ethereum__data_size_template.format(data_total), + verb=TR.buttons__confirm, + verb_cancel=None, + br_code=ButtonRequestType.SignTx, + ask_pagination=True, + ) + + def confirm_ethereum_unknown_contract_warning() -> Awaitable[None]: + return show_warning( + "unknown_contract_warning", + TR.ethereum__unknown_contract_address, + TR.words__warning, + ) + async def confirm_ethereum_tx( recipient: str | None, total_amount: str,