From 672d6b7d13cd160e89f97aa032ab4124eab128ac Mon Sep 17 00:00:00 2001 From: grdddj Date: Tue, 20 Jun 2023 09:25:02 +0200 Subject: [PATCH] feat(core): new design of PIN dialogs [no changelog] --- core/embed/rust/librust_qstr.h | 1 + .../rust/src/ui/model_tr/component/button.rs | 19 +++ .../ui/model_tr/component/changing_text.rs | 11 +- .../model_tr/component/input_methods/pin.rs | 78 ++++++------- core/embed/rust/src/ui/model_tr/layout.rs | 109 +++++++++++++++++- core/mocks/generated/trezorui2.pyi | 20 ++++ core/src/apps/management/change_pin.py | 24 ++-- core/src/apps/management/change_wipe_code.py | 12 +- core/src/trezor/ui/layouts/tr/__init__.py | 107 +++++++++++------ core/src/trezor/ui/layouts/tt_v2/__init__.py | 34 ++++-- python/src/trezorlib/debuglink.py | 2 +- tests/click_tests/test_autolock.py | 6 +- tests/click_tests/test_lock.py | 4 +- tests/click_tests/test_pin.py | 17 +-- tests/device_tests/bitcoin/test_signtx.py | 12 +- .../test_msg_change_wipe_code_t2.py | 6 +- tests/device_tests/test_sdcard.py | 12 +- tests/input_flows.py | 6 +- 18 files changed, 343 insertions(+), 137 deletions(-) diff --git a/core/embed/rust/librust_qstr.h b/core/embed/rust/librust_qstr.h index cbe5dc7e40..fd181f1183 100644 --- a/core/embed/rust/librust_qstr.h +++ b/core/embed/rust/librust_qstr.h @@ -85,6 +85,7 @@ static void _librust_qstrs(void) { MP_QSTR_max_len; MP_QSTR_max_rounds; MP_QSTR_min_count; + MP_QSTR_multiple_pages_texts; MP_QSTR_notification; MP_QSTR_notification_level; MP_QSTR_page_count; 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 d6228a1209..876c8d7c11 100644 --- a/core/embed/rust/src/ui/model_tr/component/button.rs +++ b/core/embed/rust/src/ui/model_tr/component/button.rs @@ -617,6 +617,15 @@ where ) } + /// Cancel cross on left and right arrow facing down. + pub fn up_arrow_none_arrow_wide() -> Self { + Self::new( + Some(ButtonDetails::up_arrow_icon()), + None, + Some(ButtonDetails::down_arrow_icon_wide()), + ) + } + /// Cancel cross on left and right arrow facing down. pub fn cancel_none_arrow_down() -> Self { Self::new( @@ -756,6 +765,11 @@ impl ButtonActions { ) } + /// Only confirming with middle + pub fn none_confirm_none() -> Self { + Self::new(None, Some(ButtonAction::Confirm), None) + } + /// Going to last page with left, to the next page with right pub fn last_none_next() -> Self { Self::new( @@ -799,6 +813,11 @@ impl ButtonActions { Self::new(None, None, Some(ButtonAction::NextPage)) } + /// Only going to the next page with middle + pub fn none_next_none() -> Self { + Self::new(None, Some(ButtonAction::NextPage), None) + } + /// Only going to the prev page with left pub fn prev_none_none() -> Self { Self::new(Some(ButtonAction::PrevPage), None, None) diff --git a/core/embed/rust/src/ui/model_tr/component/changing_text.rs b/core/embed/rust/src/ui/model_tr/component/changing_text.rs index f1f3dd69d5..7a78932323 100644 --- a/core/embed/rust/src/ui/model_tr/component/changing_text.rs +++ b/core/embed/rust/src/ui/model_tr/component/changing_text.rs @@ -16,6 +16,8 @@ pub struct ChangingTextLine { font: Font, /// Whether to show the text. Can be disabled. show_content: bool, + /// What to show in front of the text if it doesn't fit. + ellipsis: &'static str, alignment: Alignment, } @@ -29,6 +31,7 @@ where text, font, show_content: true, + ellipsis: "...", alignment, } } @@ -41,6 +44,12 @@ where Self::new(text, Font::BOLD, Alignment::Center) } + /// Not showing ellipsis at the beginning of longer texts. + pub fn without_ellipsis(mut self) -> Self { + self.ellipsis = ""; + self + } + /// Update the text to be displayed in the line. pub fn update_text(&mut self, text: T) { self.text = text; @@ -93,7 +102,7 @@ where fn paint_long_content_with_ellipsis(&self) { let text_to_display = long_line_content_with_ellipsis( self.text.as_ref(), - "...", + self.ellipsis, self.font, self.pad.area.width(), ); diff --git a/core/embed/rust/src/ui/model_tr/component/input_methods/pin.rs b/core/embed/rust/src/ui/model_tr/component/input_methods/pin.rs index 32df162259..5405b62e56 100644 --- a/core/embed/rust/src/ui/model_tr/component/input_methods/pin.rs +++ b/core/embed/rust/src/ui/model_tr/component/input_methods/pin.rs @@ -23,6 +23,7 @@ enum PinAction { } const MAX_PIN_LENGTH: usize = 50; +const EMPTY_PIN_STR: &str = "_"; const CHOICE_LENGTH: usize = 13; const NUMBER_START_INDEX: usize = 3; @@ -80,9 +81,9 @@ impl ChoiceFactory for ChoiceFactoryPIN { pub struct PinEntry { choice_page: ChoicePage, pin_line: Child>>, - subprompt_line: Child>, - prompt: T, + subprompt: T, show_real_pin: bool, + show_last_digit: bool, textbox: TextBox, } @@ -90,61 +91,62 @@ impl PinEntry where T: StringType + Clone, { - pub fn new(prompt: T, subprompt: T) -> Self { - let choices = ChoiceFactoryPIN; + pub fn new(subprompt: T) -> Self { + let pin_line_content = if !subprompt.as_ref().is_empty() { + String::from(subprompt.as_ref()) + } else { + String::from(EMPTY_PIN_STR) + }; Self { // Starting at a random digit. - choice_page: ChoicePage::new(choices) + choice_page: ChoicePage::new(ChoiceFactoryPIN) .with_initial_page_counter(get_random_digit_position()) .with_carousel(true), - pin_line: Child::new(ChangingTextLine::center_bold(String::from(prompt.as_ref()))), - subprompt_line: Child::new(ChangingTextLine::center_mono(subprompt)), - prompt, + pin_line: Child::new( + ChangingTextLine::center_bold(pin_line_content).without_ellipsis(), + ), + subprompt, show_real_pin: false, + show_last_digit: false, textbox: TextBox::empty(), } } /// Performs overall update of the screen. fn update(&mut self, ctx: &mut EventCtx) { - self.update_header_info(ctx); + self.update_pin_line(ctx); ctx.request_paint(); } - /// Update the header information - (sub)prompt and visible PIN. - /// If PIN is empty, showing prompt in `pin_line` and sub-prompt in the - /// `subprompt_line`. Otherwise disabling the `subprompt_line` and showing - /// the PIN - either in real numbers or masked in asterisks. - fn update_header_info(&mut self, ctx: &mut EventCtx) { - let show_prompts = self.is_empty(); - - let text = if show_prompts { - String::from(self.prompt.as_ref()) + /// Show updated content in the changing line. + /// Many possibilities, according to the PIN state. + fn update_pin_line(&mut self, ctx: &mut EventCtx) { + let pin_line_text = if self.is_empty() && !self.subprompt.as_ref().is_empty() { + String::from(self.subprompt.as_ref()) + } else if self.is_empty() { + String::from(EMPTY_PIN_STR) } else if self.show_real_pin { String::from(self.pin()) } else { - // Showing asterisks and the last digit. + // Showing asterisks and possibly the last digit. let mut dots: String = String::new(); for _ in 0..self.textbox.len() - 1 { unwrap!(dots.push('*')); } - let last_char = unwrap!(self.textbox.content().chars().last()); + let last_char = if self.show_last_digit { + unwrap!(self.textbox.content().chars().last()) + } else { + '*' + }; unwrap!(dots.push(last_char)); dots }; - // Force repaint of the whole header. - // Putting the current text into the PIN line. self.pin_line.mutate(ctx, |ctx, pin_line| { - pin_line.update_text(text); + pin_line.update_text(pin_line_text); pin_line.request_complete_repaint(ctx); }); - // Showing subprompt only conditionally. - self.subprompt_line.mutate(ctx, |ctx, subprompt_line| { - subprompt_line.show_or_not(show_prompts); - subprompt_line.request_complete_repaint(ctx); - }); } pub fn pin(&self) -> &str { @@ -168,21 +170,23 @@ where fn place(&mut self, bounds: Rect) -> Rect { let pin_height = self.pin_line.inner().needed_height(); - let subtitle_height = self.subprompt_line.inner().needed_height(); - let (title_area, subtitle_and_choice_area) = bounds.split_top(pin_height); - let (subtitle_area, choice_area) = subtitle_and_choice_area.split_top(subtitle_height); - self.pin_line.place(title_area); - self.subprompt_line.place(subtitle_area); + let (pin_area, choice_area) = bounds.split_top(pin_height); + self.pin_line.place(pin_area); self.choice_page.place(choice_area); bounds } fn event(&mut self, ctx: &mut EventCtx, event: Event) -> Option { // Any event when showing real PIN should hide it + // Same with showing last digit if self.show_real_pin { self.show_real_pin = false; self.update(ctx) } + if self.show_last_digit { + self.show_last_digit = false; + self.update(ctx) + } match self.choice_page.event(ctx, event) { Some(PinAction::Delete) => { @@ -201,6 +205,7 @@ where // Choosing random digit to be shown next self.choice_page .set_page_counter(ctx, get_random_digit_position()); + self.show_last_digit = true; self.update(ctx); None } @@ -210,7 +215,6 @@ where fn paint(&mut self) { self.pin_line.paint(); - self.subprompt_line.paint(); self.choice_page.paint(); } } @@ -224,11 +228,7 @@ where { fn trace(&self, t: &mut dyn crate::trace::Tracer) { t.component("PinKeyboard"); - t.string("prompt", self.prompt.as_ref()); - let subprompt = self.subprompt_line.inner().get_text(); - if !subprompt.as_ref().is_empty() { - t.string("subprompt", subprompt.as_ref()); - } + t.string("subprompt", self.subprompt.as_ref()); t.string("pin", self.textbox.content()); t.child("choice_page", &self.choice_page); } diff --git a/core/embed/rust/src/ui/model_tr/layout.rs b/core/embed/rust/src/ui/model_tr/layout.rs index 7043e9c806..0e8ac87539 100644 --- a/core/embed/rust/src/ui/model_tr/layout.rs +++ b/core/embed/rust/src/ui/model_tr/layout.rs @@ -24,7 +24,8 @@ use crate::{ }, ComponentExt, FormattedText, LineBreaking, Timeout, }, - display, geometry, + display, + geometry::{self, Alignment}, layout::{ obj::{ComponentMsgObj, LayoutObj}, result::{CANCELLED, CONFIRMED, INFO}, @@ -740,7 +741,7 @@ extern "C" fn tutorial(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj "CONFIRM", "Press both left and right at the same\ntime to confirm.", ButtonLayout::none_armed_none("CONFIRM".into()), - ButtonActions::prev_next_none(), + ButtonActions::none_next_none(), ) }, 5 => { @@ -775,6 +776,33 @@ extern "C" fn tutorial(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } } +extern "C" fn new_show_error(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj { + let block = move |_args: &[Obj], kwargs: &Map| { + let button: StrBuffer = kwargs.get(Qstr::MP_QSTR_button)?.try_into()?; + let title: StrBuffer = kwargs.get(Qstr::MP_QSTR_title)?.try_into()?; + let description: StrBuffer = kwargs.get(Qstr::MP_QSTR_description)?.try_into()?; + + let get_page = move |page_index| { + assert!(page_index == 0); + + let btn_layout = ButtonLayout::none_armed_none(button.clone()); + let btn_actions = ButtonActions::none_confirm_none(); + let ops = OpTextLayout::::new(theme::TEXT_NORMAL) + .alignment(Alignment::Center) + .text_bold(title.clone()) + .newline() + .text_normal(description.clone()); + let formatted = FormattedText::new(ops).vertically_aligned(Alignment::Center); + Page::new(btn_layout, btn_actions, formatted) + }; + let pages = FlowPages::new(get_page, 1); + + let obj = LayoutObj::new(Flow::new(pages))?; + Ok(obj.into()) + }; + unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } +} + extern "C" fn new_confirm_modify_fee(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()?; @@ -814,6 +842,61 @@ extern "C" fn new_confirm_modify_fee(n_args: usize, args: *const Obj, kwargs: *m unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } } +extern "C" fn new_multiple_pages_texts(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()?; + let verb: StrBuffer = kwargs.get(Qstr::MP_QSTR_verb)?.try_into()?; + let items: Gc = kwargs.get(Qstr::MP_QSTR_items)?.try_into()?; + + // Cache the page count so that we can move `items` into the closure. + let page_count = items.len(); + + // Closure to lazy-load the information on given page index. + // Done like this to allow arbitrarily many pages without + // the need of any allocation here in Rust. + let get_page = move |page_index| { + let item_obj = unwrap!(items.get(page_index)); + let text = unwrap!(item_obj.try_into()); + + let (btn_layout, btn_actions) = if page_count == 1 { + // There is only one page + ( + ButtonLayout::cancel_none_text(verb.clone()), + ButtonActions::cancel_none_confirm(), + ) + } else if page_index == 0 { + // First page + ( + ButtonLayout::cancel_none_arrow_wide(), + ButtonActions::cancel_none_next(), + ) + } else if page_index == page_count - 1 { + // Last page + ( + ButtonLayout::up_arrow_none_text(verb.clone()), + ButtonActions::prev_none_confirm(), + ) + } else { + // Page in the middle + ( + ButtonLayout::up_arrow_none_arrow_wide(), + ButtonActions::prev_none_next(), + ) + }; + + let ops = OpTextLayout::new(theme::TEXT_NORMAL).text_normal(text); + let formatted = FormattedText::new(ops).vertically_aligned(Alignment::Center); + + Page::new(btn_layout, btn_actions, formatted) + }; + + let pages = FlowPages::new(get_page, page_count); + let obj = LayoutObj::new(Flow::new(pages).with_common_title(title))?; + Ok(obj.into()) + }; + unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } +} + extern "C" fn new_confirm_fido(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()?; @@ -999,7 +1082,9 @@ extern "C" fn new_request_pin(n_args: usize, args: *const Obj, kwargs: *mut Map) let prompt: StrBuffer = kwargs.get(Qstr::MP_QSTR_prompt)?.try_into()?; let subprompt: StrBuffer = kwargs.get(Qstr::MP_QSTR_subprompt)?.try_into()?; - let obj = LayoutObj::new(PinEntry::new(prompt, subprompt))?; + let obj = + LayoutObj::new(Frame::new(prompt, PinEntry::new(subprompt)).with_title_centered())?; + Ok(obj.into()) }; unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } @@ -1442,6 +1527,15 @@ pub static mp_module_trezorui2: Module = obj_module! { /// """Show user how to interact with the device.""" Qstr::MP_QSTR_tutorial => obj_fn_kw!(0, tutorial).as_obj(), + /// def show_error( + /// *, + /// title: str, + /// description: str, + /// button: str, + /// ) -> object: + /// """Show a popup with text centered both vertically and horizontally. With just a middle button.""" + Qstr::MP_QSTR_show_error => obj_fn_kw!(0, new_show_error).as_obj(), + /// def confirm_modify_fee( /// *, /// title: str, # ignored @@ -1466,6 +1560,15 @@ pub static mp_module_trezorui2: Module = obj_module! { /// """ Qstr::MP_QSTR_confirm_fido => obj_fn_kw!(0, new_confirm_fido).as_obj(), + /// def multiple_pages_texts( + /// *, + /// title: str, + /// verb: str, + /// items: list[str], + /// ) -> object: + /// """Show multiple texts, each on its own page.""" + Qstr::MP_QSTR_multiple_pages_texts => obj_fn_kw!(0, new_multiple_pages_texts).as_obj(), + /// def show_info( /// *, /// title: str, diff --git a/core/mocks/generated/trezorui2.pyi b/core/mocks/generated/trezorui2.pyi index cffe55f0f5..fc29a4530f 100644 --- a/core/mocks/generated/trezorui2.pyi +++ b/core/mocks/generated/trezorui2.pyi @@ -149,6 +149,16 @@ def tutorial() -> object: """Show user how to interact with the device.""" +# rust/src/ui/model_tr/layout.rs +def show_error( + *, + title: str, + description: str, + button: str, +) -> object: + """Show a popup with text centered both vertically and horizontally. With just a middle button.""" + + # rust/src/ui/model_tr/layout.rs def confirm_modify_fee( *, @@ -174,6 +184,16 @@ def confirm_fido( """ +# rust/src/ui/model_tr/layout.rs +def multiple_pages_texts( + *, + title: str, + verb: str, + items: list[str], +) -> object: + """Show multiple texts, each on its own page.""" + + # rust/src/ui/model_tr/layout.rs def show_info( *, diff --git a/core/src/apps/management/change_pin.py b/core/src/apps/management/change_pin.py index 8f80561690..e169943533 100644 --- a/core/src/apps/management/change_pin.py +++ b/core/src/apps/management/change_pin.py @@ -52,10 +52,10 @@ async def change_pin(msg: ChangePin) -> Success: msg_screen = "PIN changed." msg_wire = "PIN changed" else: - msg_screen = "PIN protection enabled." + msg_screen = "PIN protection\nturned on." msg_wire = "PIN enabled" else: - msg_screen = "PIN protection disabled." + msg_screen = "PIN protection\nturned off." msg_wire = "PIN removed" await show_success("success_pin", msg_screen) @@ -67,34 +67,30 @@ def _require_confirm_change_pin(msg: ChangePin) -> Awaitable[None]: has_pin = config.has_pin() - br_type = "set_pin" title = "PIN settings" if msg.remove and has_pin: # removing pin return confirm_action( - br_type, + "disable_pin", title, - description="Do you want to disable PIN protection?", - verb="Disable", + description="Are you sure you want to turn off PIN protection?", + verb="Turn off", ) if not msg.remove and has_pin: # changing pin return confirm_action( - br_type, + "change_pin", title, - description="Do you want to change your PIN?", + description="Change PIN?", verb="Change", ) if not msg.remove and not has_pin: # setting new pin return confirm_set_new_pin( - br_type, + "set_pin", title, - "Do you want to enable PIN protection?", - [ - "PIN will be used to access this device.", - "PIN should be 4-50 digits long.", - ], + "PIN", + "PIN will be required to access this device.", ) # removing non-existing PIN diff --git a/core/src/apps/management/change_wipe_code.py b/core/src/apps/management/change_wipe_code.py index 2a4ac8cd9b..a79c2b5aeb 100644 --- a/core/src/apps/management/change_wipe_code.py +++ b/core/src/apps/management/change_wipe_code.py @@ -68,15 +68,15 @@ def _require_confirm_action( return confirm_action( "disable_wipe_code", title, - description="Do you want to disable wipe code protection?", - verb="Disable", + description="Turn off wipe code protection?", + verb="Turn off", ) if not msg.remove and has_wipe_code: return confirm_action( "change_wipe_code", title, - description="Do you want to change the wipe code?", + description="Change wipe code?", verb="Change", ) @@ -84,10 +84,8 @@ def _require_confirm_action( return confirm_set_new_pin( "set_wipe_code", title, - "Do you want to enable wipe code?", - [ - "Wipe code can be used to erase all data from this device.", - ], + "wipe code", + "Wipe code can be used to erase all data from this device.", ) # Removing non-existing wipe code. diff --git a/core/src/trezor/ui/layouts/tr/__init__.py b/core/src/trezor/ui/layouts/tr/__init__.py index ec2ec96c28..dc7e2306ca 100644 --- a/core/src/trezor/ui/layouts/tr/__init__.py +++ b/core/src/trezor/ui/layouts/tr/__init__.py @@ -1130,7 +1130,7 @@ async def request_pin_on_device( result = await interact( RustLayout( trezorui2.request_pin( - prompt=prompt, + prompt=prompt.upper(), subprompt=subprompt, allow_cancel=allow_cancel, wrong_pin=wrong_pin, @@ -1151,33 +1151,73 @@ async def confirm_reenter_pin( ) -> None: br_type = "reenter_wipe_code" if is_wipe_code else "reenter_pin" title = "CHECK WIPE CODE" if is_wipe_code else "CHECK PIN" + description = "wipe code" if is_wipe_code else "PIN" return await confirm_action( br_type, title, - action="Please re-enter to confirm.", - verb="BEGIN", + description=f"Please re-enter {description} to confirm.", + verb="CONTINUE", br_code=BR_TYPE_OTHER, ) +async def show_error( + br_type: str, + title: str, + description: str, + button: str, + br_code: ButtonRequestType = BR_TYPE_OTHER, +) -> None: + await interact( + RustLayout( + trezorui2.show_error( + title=title, + description=description, + button=button, + ) + ), + br_type, + br_code, + ) + + +async def confirm_multiple_pages_texts( + br_type: str, + title: str, + items: list[str], + verb: str, + br_code: ButtonRequestType = BR_TYPE_OTHER, +) -> None: + await raise_if_not_confirmed( + interact( + RustLayout( + trezorui2.multiple_pages_texts( + title=title, + verb=verb, + items=items, + ) + ), + br_type, + br_code, + ) + ) + + async def pin_mismatch_popup( is_wipe_code: bool = False, ) -> None: - title = "WIPE CODE MISMATCH" if is_wipe_code else "PIN MISMATCH" description = "wipe codes" if is_wipe_code else "PINs" - return await confirm_action( - "pin_mismatch", - title, - description=f"The {description} you entered do not match.\nPlease try again.", - verb="TRY AGAIN", - verb_cancel=None, - br_code=BR_TYPE_OTHER, + br_code = "wipe_code_mismatch" if is_wipe_code else "pin_mismatch" + return await show_error( + br_code, + f"Entered {description} do not match!", + "Please check again.", + "CHECK AGAIN", + BR_TYPE_OTHER, ) -async def wipe_code_same_as_pin_popup( - is_wipe_code: bool = False, -) -> None: +async def wipe_code_same_as_pin_popup() -> None: return await confirm_action( "wipe_code_same_as_pin", "INVALID WIPE CODE", @@ -1192,34 +1232,33 @@ async def confirm_set_new_pin( br_type: str, title: str, description: str, - information: list[str], + information: str, br_code: ButtonRequestType = BR_TYPE_OTHER, ) -> None: - await confirm_action( + question = f"Turn on {description} protection?" + await confirm_multiple_pages_texts( br_type, - title, - description=description, - verb="ENABLE", - br_code=br_code, + title.upper(), + [question, information], + "TURN ON", + br_code, ) - # Additional information for the user to know about PIN/WIPE CODE - + # Not showing extra info for wipe code if "wipe_code" in br_type: - verb = "HODL TO BEGIN" # Easter egg from @Hannsek - else: - information.append( - "Position of individual numbers will change between entries for enhanced security." - ) - verb = "HOLD TO BEGIN" + return - return await confirm_action( + # Additional information for the user to know about PIN + next_info = [ + "PIN should be 4-50 digits long.", + "Position of the cursor will change between entries for enhanced security.", + ] + await confirm_multiple_pages_texts( br_type, - "", - description="\n\r".join(information), - verb=verb, - hold=True, - br_code=br_code, + title.upper(), + next_info, + "CONTINUE", + br_code, ) diff --git a/core/src/trezor/ui/layouts/tt_v2/__init__.py b/core/src/trezor/ui/layouts/tt_v2/__init__.py index 3f82c1a92e..1454f26523 100644 --- a/core/src/trezor/ui/layouts/tt_v2/__init__.py +++ b/core/src/trezor/ui/layouts/tt_v2/__init__.py @@ -1217,17 +1217,37 @@ async def confirm_set_new_pin( br_type: str, title: str, description: str, - information: list[str], # unused on TT + information: str, br_code: ButtonRequestType = BR_TYPE_OTHER, ) -> None: - await confirm_action( - br_type, - title, - description=description, - verb="ENABLE", - br_code=br_code, + await raise_if_not_confirmed( + interact( + RustLayout( + trezorui2.confirm_emphasized( + title=title.upper(), + items=( + "Turn on ", + (True, description), + " protection?\n\n", + information, + ), + verb="TURN ON", + ) + ), + br_type, + br_code, + ) ) + # await confirm_action( + # ctx, + # br_type, + # title, + # description=description, + # verb="TURN ON", + # br_code=br_code, + # ) + async def mnemonic_word_entering() -> None: """Not supported for TT.""" diff --git a/python/src/trezorlib/debuglink.py b/python/src/trezorlib/debuglink.py index 831644c863..da69ba643d 100644 --- a/python/src/trezorlib/debuglink.py +++ b/python/src/trezorlib/debuglink.py @@ -308,7 +308,7 @@ class LayoutContent(UnstructuredJSONReader): def pin(self) -> str: """Get PIN from the layout.""" - assert self.main_component() == "PinKeyboard" + assert "PinKeyboard" in self.all_components() return self.find_unique_value_by_key("pin", default="", only_type=str) def passphrase(self) -> str: diff --git a/tests/click_tests/test_autolock.py b/tests/click_tests/test_autolock.py index 42056f3f18..6e1fa07257 100644 --- a/tests/click_tests/test_autolock.py +++ b/tests/click_tests/test_autolock.py @@ -58,7 +58,7 @@ def set_autolock_delay(device_handler: "BackgroundDeviceHandler", delay_ms: int) device_handler.run(device.apply_settings, auto_lock_delay_ms=delay_ms) # type: ignore - assert debug.wait_layout().main_component() == "PinKeyboard" + assert "PinKeyboard" in debug.wait_layout().all_components() debug.input("1234") @@ -246,7 +246,7 @@ def unlock_dry_run(debug: "DebugLink") -> "LayoutContent": in debug.wait_layout().text_content() ) layout = go_next(debug, wait=True) - assert layout.main_component() == "PinKeyboard" + assert "PinKeyboard" in layout.all_components() layout = debug.input(PIN4, wait=True) assert layout is not None @@ -276,7 +276,7 @@ def test_dryrun_locks_at_number_of_words(device_handler: "BackgroundDeviceHandle # lockscreen triggered automatically debug.wait_layout(wait_for_external_change=True) layout = go_next(debug, wait=True) - assert layout.main_component() == "PinKeyboard" + assert "PinKeyboard" in layout.all_components() layout = debug.input(PIN4, wait=True) assert layout is not None diff --git a/tests/click_tests/test_lock.py b/tests/click_tests/test_lock.py index 0b6cd2f280..d0bc431a89 100644 --- a/tests/click_tests/test_lock.py +++ b/tests/click_tests/test_lock.py @@ -46,7 +46,7 @@ def test_hold_to_lock(device_handler: "BackgroundDeviceHandler"): # unlock with message device_handler.run(common.get_test_address) - assert debug.wait_layout().main_component() == "PinKeyboard" + assert "PinKeyboard" in debug.wait_layout().all_components() debug.input("1234", wait=True) assert device_handler.result() @@ -69,7 +69,7 @@ def test_hold_to_lock(device_handler: "BackgroundDeviceHandler"): layout = debug.wait_layout() else: layout = debug.click(buttons.INFO, wait=True) - assert layout.main_component() == "PinKeyboard" + assert "PinKeyboard" in layout.all_components() debug.input("1234", wait=True) assert device_handler.features().unlocked is True diff --git a/tests/click_tests/test_pin.py b/tests/click_tests/test_pin.py index b74dfe96f6..f39781113c 100644 --- a/tests/click_tests/test_pin.py +++ b/tests/click_tests/test_pin.py @@ -83,19 +83,19 @@ def prepare( elif situation == Situation.PIN_SETUP: # Set new PIN device_handler.run(device.change_pin) # type: ignore - assert "enable PIN protection" in debug.wait_layout().text_content() + assert "Turn on" in debug.wait_layout().text_content() if debug.model == "T": go_next(debug) elif debug.model == "R": go_next(debug, wait=True) go_next(debug, wait=True) go_next(debug, wait=True) - debug.press_right_htc(1000) + go_next(debug, wait=True) elif situation == Situation.PIN_CHANGE: # Change PIN device_handler.run(device.change_pin) # type: ignore _input_see_confirm(debug, old_pin) - assert "change your PIN" in debug.read_layout().text_content() + assert "Change PIN" in debug.read_layout().text_content() go_next(debug, wait=True) _input_see_confirm(debug, old_pin) elif situation == Situation.WIPE_CODE_SETUP: @@ -103,10 +103,12 @@ def prepare( device_handler.run(device.change_wipe_code) # type: ignore if old_pin: _input_see_confirm(debug, old_pin) - assert "enable wipe code" in debug.wait_layout().text_content() + assert "Turn on" in debug.wait_layout().text_content() go_next(debug, wait=True) if debug.model == "R": - debug.press_right_htc(1000) + go_next(debug, wait=True) + go_next(debug, wait=True) + go_next(debug, wait=True) if old_pin: debug.wait_layout() _input_see_confirm(debug, old_pin) @@ -119,7 +121,7 @@ def prepare( def _assert_pin_entry(debug: "DebugLink") -> None: - assert debug.read_layout().main_component() == "PinKeyboard" + assert "PinKeyboard" in debug.read_layout().all_components() def _input_pin(debug: "DebugLink", pin: str, check: bool = False) -> None: @@ -262,10 +264,11 @@ def test_pin_setup(device_handler: "BackgroundDeviceHandler"): def test_pin_setup_mismatch(device_handler: "BackgroundDeviceHandler"): with PIN_CANCELLED, prepare(device_handler, Situation.PIN_SETUP) as debug: _enter_two_times(debug, "1", "2") - go_next(debug) if debug.model == "T": + go_next(debug) _cancel_pin(debug) elif debug.model == "R": + debug.press_middle() debug.press_no() diff --git a/tests/device_tests/bitcoin/test_signtx.py b/tests/device_tests/bitcoin/test_signtx.py index e530d09f9c..7a6dc6d789 100644 --- a/tests/device_tests/bitcoin/test_signtx.py +++ b/tests/device_tests/bitcoin/test_signtx.py @@ -1278,7 +1278,7 @@ def test_prevtx_forbidden_fields(client: Client, field, value): "field, value", (("expiry", 9), ("timestamp", 42), ("version_group_id", 69), ("branch_id", 13)), ) -def test_signtx_forbidden_fields(client: Client, field, value): +def test_signtx_forbidden_fields(client: Client, field: str, value: int): inp0 = messages.TxInputType( address_n=parse_path("m/44h/0h/0h/0/0"), # 1JAd7XCBzGudGpJQSDSfpmJhiygtLQWaGL prev_hash=TXHASH_157041, @@ -1357,7 +1357,9 @@ def test_incorrect_input_script_type(client: Client, script_type): messages.OutputScriptType.PAYTOSCRIPTHASH, ), ) -def test_incorrect_output_script_type(client: Client, script_type): +def test_incorrect_output_script_type( + client: Client, script_type: messages.OutputScriptType +): address_n = parse_path("m/44h/1h/0h/0/0") # mvbu1Gdy8SUjTenqerxUaZyYjmveZvt33q attacker_multisig_public_key = bytes.fromhex( "030e669acac1f280d1ddf441cd2ba5e97417bf2689e4bbec86df4f831bf9f7ffd0" @@ -1407,7 +1409,7 @@ def test_incorrect_output_script_type(client: Client, script_type): "lock_time, sequence", ((499_999_999, 0xFFFFFFFE), (500_000_000, 0xFFFFFFFE), (1, 0xFFFFFFFF)), ) -def test_lock_time(client: Client, lock_time, sequence): +def test_lock_time(client: Client, lock_time: int, sequence: int): # input tx: 0dac366fd8a67b2a89fbb0d31086e7acded7a5bbf9ef9daa935bc873229ef5b5 inp1 = messages.TxInputType( @@ -1548,7 +1550,6 @@ def test_information(client: Client): with client: IF = InputFlowSignTxInformation(client) client.set_input_flow(IF.get()) - client.watch_layout(True) btc.sign_tx( client, @@ -1584,7 +1585,6 @@ def test_information_mixed(client: Client): with client: IF = InputFlowSignTxInformationMixed(client) client.set_input_flow(IF.get()) - client.watch_layout(True) btc.sign_tx( client, @@ -1616,7 +1616,6 @@ def test_information_cancel(client: Client): with client, pytest.raises(Cancelled): IF = InputFlowSignTxInformationCancel(client) client.set_input_flow(IF.get()) - client.watch_layout(True) btc.sign_tx( client, @@ -1663,7 +1662,6 @@ def test_information_replacement(client: Client): with client: IF = InputFlowSignTxInformationReplacement(client) client.set_input_flow(IF.get()) - client.watch_layout(True) btc.sign_tx( client, diff --git a/tests/device_tests/test_msg_change_wipe_code_t2.py b/tests/device_tests/test_msg_change_wipe_code_t2.py index 113659bf46..80d1703b65 100644 --- a/tests/device_tests/test_msg_change_wipe_code_t2.py +++ b/tests/device_tests/test_msg_change_wipe_code_t2.py @@ -63,7 +63,7 @@ def test_set_remove_wipe_code(client: Client): assert client.features.wipe_code_protection is False with client: - br_amount = 5 if client.debug.model == "T" else 7 + br_amount = 5 if client.debug.model == "T" else 6 client.set_expected_responses( [messages.ButtonRequest()] * br_amount + [messages.Success, messages.Features] @@ -118,7 +118,7 @@ def test_set_wipe_code_to_pin(client: Client): _ensure_unlocked(client, PIN4) with client: - br_amount = 7 if client.debug.model == "T" else 9 + br_amount = 7 if client.debug.model == "T" else 8 client.set_expected_responses( [messages.ButtonRequest()] * br_amount + [messages.Success, messages.Features] @@ -134,7 +134,7 @@ def test_set_wipe_code_to_pin(client: Client): def test_set_pin_to_wipe_code(client: Client): # Set wipe code. with client: - br_amount = 4 if client.debug.model == "T" else 6 + br_amount = 4 if client.debug.model == "T" else 5 client.set_expected_responses( [messages.ButtonRequest()] * br_amount + [messages.Success, messages.Features] diff --git a/tests/device_tests/test_sdcard.py b/tests/device_tests/test_sdcard.py index 7d47164145..983d1d914e 100644 --- a/tests/device_tests/test_sdcard.py +++ b/tests/device_tests/test_sdcard.py @@ -53,7 +53,7 @@ def test_sd_protect_unlock(client: Client): def input_flow_enable_sd_protect(): yield # Enter PIN to unlock device - assert layout().main_component() == "PinKeyboard" + assert "PinKeyboard" in layout().all_components() client.debug.input("1234") yield # do you really want to enable SD protection @@ -61,7 +61,7 @@ def test_sd_protect_unlock(client: Client): client.debug.press_yes() yield # enter current PIN - assert layout().main_component() == "PinKeyboard" + assert "PinKeyboard" in layout().all_components() client.debug.input("1234") yield # you have successfully enabled SD protection @@ -79,15 +79,15 @@ def test_sd_protect_unlock(client: Client): client.debug.press_yes() yield # enter current PIN - assert layout().main_component() == "PinKeyboard" + assert "PinKeyboard" in layout().all_components() client.debug.input("1234") yield # enter new PIN - assert layout().main_component() == "PinKeyboard" + assert "PinKeyboard" in layout().all_components() client.debug.input("1234") yield # enter new PIN again - assert layout().main_component() == "PinKeyboard" + assert "PinKeyboard" in layout().all_components() client.debug.input("1234") yield # Pin change successful @@ -107,7 +107,7 @@ def test_sd_protect_unlock(client: Client): client.debug.press_yes() yield # enter current PIN - assert layout().main_component() == "PinKeyboard" + assert "PinKeyboard" in layout().all_components() client.debug.input("1234") yield # SD card problem diff --git a/tests/input_flows.py b/tests/input_flows.py index 10e7288b4d..994c2ad281 100644 --- a/tests/input_flows.py +++ b/tests/input_flows.py @@ -1413,11 +1413,11 @@ def bip39_recovery_possible_pin( # PIN when requested if pin is not None: yield - assert debug.wait_layout().main_component() == "PinKeyboard" + assert "PinKeyboard" in debug.wait_layout().all_components() debug.input(pin) yield - assert debug.wait_layout().main_component() == "PinKeyboard" + assert "PinKeyboard" in debug.wait_layout().all_components() debug.input(pin) yield @@ -1464,7 +1464,7 @@ class InputFlowBip39RecoveryPIN(InputFlowBase): self.debug.input("654") yield - assert "re-enter to confirm" in self.layout().text_content() + assert "re-enter PIN" in self.layout().text_content() self.debug.press_right() yield