From aaa2ece3ba8439fd416044b7a7988ce0c3c7bc42 Mon Sep 17 00:00:00 2001 From: obrusvit Date: Tue, 11 Jun 2024 16:31:50 +0200 Subject: [PATCH] feat(core/ui): highlight repeated words on T3T1 ShowShareWords flow now informs the user if the word is repeated. The most typical usecase in 1-of-1 shamir (SingleShare) where 3rd and 4th word is "academic". --- core/embed/rust/librust_qstr.h | 2 + .../generated/translated_string.rs | 3 + core/embed/rust/src/ui/component/label.rs | 4 + .../src/ui/model_mercury/component/frame.rs | 36 +++- .../ui/model_mercury/component/share_words.rs | 177 ++++++++++++++++-- .../ui/model_mercury/flow/show_share_words.rs | 9 +- .../embed/rust/src/ui/model_mercury/layout.rs | 1 + .../rust/src/ui/model_mercury/theme/mod.rs | 2 + core/mocks/generated/trezorui2.pyi | 1 + core/mocks/trezortranslate_keys.pyi | 1 + core/src/trezor/ui/layouts/mercury/reset.py | 2 + core/translations/en.json | 1 + core/translations/order.json | 3 +- core/translations/signatures.json | 6 +- 14 files changed, 218 insertions(+), 30 deletions(-) diff --git a/core/embed/rust/librust_qstr.h b/core/embed/rust/librust_qstr.h index 270c7f9df8..f2168dc75e 100644 --- a/core/embed/rust/librust_qstr.h +++ b/core/embed/rust/librust_qstr.h @@ -246,6 +246,7 @@ static void _librust_qstrs(void) { MP_QSTR_haptic_feedback__enable; MP_QSTR_haptic_feedback__subtitle; MP_QSTR_haptic_feedback__title; + MP_QSTR_highlight_repeated; MP_QSTR_hold; MP_QSTR_hold_danger; MP_QSTR_homescreen__click_to_connect; @@ -528,6 +529,7 @@ static void _librust_qstrs(void) { MP_QSTR_reset__slip39_checklist_write_down; MP_QSTR_reset__slip39_checklist_write_down_recovery; MP_QSTR_reset__the_threshold_sets_the_number_of_shares; + MP_QSTR_reset__the_word_is_repeated; MP_QSTR_reset__threshold_info; MP_QSTR_reset__title_backup_is_done; MP_QSTR_reset__title_create_wallet; diff --git a/core/embed/rust/src/translations/generated/translated_string.rs b/core/embed/rust/src/translations/generated/translated_string.rs index f6f1024add..0e3caf447f 100644 --- a/core/embed/rust/src/translations/generated/translated_string.rs +++ b/core/embed/rust/src/translations/generated/translated_string.rs @@ -1339,6 +1339,7 @@ pub enum TranslatedString { reset__repeat_for_all_shares = 938, // "Repeat for all shares." homescreen__settings_subtitle = 939, // "Settings" homescreen__settings_title = 940, // "Homescreen" + reset__the_word_is_repeated = 941, // "The word is repeated" } impl TranslatedString { @@ -2672,6 +2673,7 @@ impl TranslatedString { Self::reset__repeat_for_all_shares => "Repeat for all shares.", Self::homescreen__settings_subtitle => "Settings", Self::homescreen__settings_title => "Homescreen", + Self::reset__the_word_is_repeated => "The word is repeated", } } @@ -4006,6 +4008,7 @@ impl TranslatedString { Qstr::MP_QSTR_reset__repeat_for_all_shares => Some(Self::reset__repeat_for_all_shares), Qstr::MP_QSTR_homescreen__settings_subtitle => Some(Self::homescreen__settings_subtitle), Qstr::MP_QSTR_homescreen__settings_title => Some(Self::homescreen__settings_title), + Qstr::MP_QSTR_reset__the_word_is_repeated => Some(Self::reset__the_word_is_repeated), _ => None, } } diff --git a/core/embed/rust/src/ui/component/label.rs b/core/embed/rust/src/ui/component/label.rs index 087583d1f0..d9fe59ffc2 100644 --- a/core/embed/rust/src/ui/component/label.rs +++ b/core/embed/rust/src/ui/component/label.rs @@ -66,6 +66,10 @@ impl<'a> Label<'a> { self.text = text; } + pub fn set_style(&mut self, style: TextStyle) { + self.layout.style = style; + } + pub fn font(&self) -> Font { self.layout.style.text_font } diff --git a/core/embed/rust/src/ui/model_mercury/component/frame.rs b/core/embed/rust/src/ui/model_mercury/component/frame.rs index a2f80b6884..a7d284a8f4 100644 --- a/core/embed/rust/src/ui/model_mercury/component/frame.rs +++ b/core/embed/rust/src/ui/model_mercury/component/frame.rs @@ -86,11 +86,6 @@ where self } - pub fn title_styled(mut self, style: TextStyle) -> Self { - self.title = self.title.styled(style); - self - } - #[inline(never)] pub fn with_subtitle(mut self, subtitle: TString<'static>) -> Self { let style = theme::TEXT_SUB_GREY; @@ -125,6 +120,18 @@ where .button_styled(theme::button_danger()) } + pub fn title_styled(mut self, style: TextStyle) -> Self { + self.title = self.title.styled(style); + self + } + + pub fn subtitle_styled(mut self, style: TextStyle) -> Self { + if let Some(subtitle) = self.subtitle.take() { + self.subtitle = Some(subtitle.styled(style)) + } + self + } + pub fn button_styled(mut self, style: ButtonStyleSheet) -> Self { if self.button.is_some() { self.button = Some(self.button.unwrap().styled(style)); @@ -160,6 +167,25 @@ where ctx.request_paint(); } + pub fn update_subtitle( + &mut self, + ctx: &mut EventCtx, + new_subtitle: TString<'static>, + new_style: Option, + ) { + let style = new_style.unwrap_or(theme::TEXT_SUB_GREY); + match &mut self.subtitle { + Some(subtitle) => { + subtitle.set_style(style); + subtitle.set_text(new_subtitle); + } + None => { + self.subtitle = Some(Label::new(new_subtitle, self.title.alignment(), style)); + } + } + ctx.request_paint(); + } + pub fn update_content(&mut self, ctx: &mut EventCtx, update_fn: F) -> R where F: Fn(&mut EventCtx, &mut T) -> R, diff --git a/core/embed/rust/src/ui/model_mercury/component/share_words.rs b/core/embed/rust/src/ui/model_mercury/component/share_words.rs index 1d3173d38f..4eac9d94f3 100644 --- a/core/embed/rust/src/ui/model_mercury/component/share_words.rs +++ b/core/embed/rust/src/ui/model_mercury/component/share_words.rs @@ -5,10 +5,13 @@ use crate::{ translations::TR, ui::{ animation::Animation, - component::{Component, Event, EventCtx, Never, SwipeDirection}, + component::{ + swipe_detect::{SwipeConfig, SwipeSettings}, + Component, Event, EventCtx, Never, SwipeDirection, + }, event::SwipeEvent, geometry::{Alignment, Alignment2D, Insets, Offset, Rect}, - model_mercury::component::Footer, + model_mercury::component::{Footer, Frame, FrameMsg}, shape::{self, Renderer}, util, }, @@ -17,12 +20,112 @@ use heapless::Vec; const MAX_WORDS: usize = 33; // super-shamir has 33 words, all other have less const ANIMATION_DURATION_MS: Duration = Duration::from_millis(166); +type IndexVec = Vec; /// Component showing mnemonic/share words during backup procedure. Model T3T1 /// contains one word per screen. A user is instructed to swipe up/down to see /// next/previous word. +/// This is a wrapper around a Frame so that the subtitle and Footer of the +/// Frame can be updated based on the index of the word shown. Actual share +/// words are rendered within `ShareWordsInner` component, pub struct ShareWords<'a> { - area: Rect, + subtitle: TString<'static>, + frame: Frame>, + repeated_indices: Option, +} + +impl<'a> ShareWords<'a> { + pub fn new( + title: TString<'static>, + subtitle: TString<'static>, + share_words: Vec, MAX_WORDS>, + highlight_repeated: bool, + ) -> Self { + let repeated_indices = if highlight_repeated { + Some(Self::find_repeated(share_words.as_slice())) + } else { + None + }; + Self { + subtitle, + frame: Frame::left_aligned(title, ShareWordsInner::new(share_words)) + .with_swipe(SwipeDirection::Up, SwipeSettings::default()) + .with_swipe(SwipeDirection::Down, SwipeSettings::default()) + .with_vertical_pages() + .with_subtitle(subtitle), + repeated_indices, + } + } + + fn find_repeated(share_words: &[TString]) -> IndexVec { + let mut repeated_indices = IndexVec::new(); + for i in (0..share_words.len()).rev() { + let word = share_words[i]; + if share_words[..i].contains(&word) { + unwrap!(repeated_indices.push(i as u8)); + } + } + repeated_indices.reverse(); + repeated_indices + } +} + +impl<'a> Component for ShareWords<'a> { + type Msg = FrameMsg; + + fn place(&mut self, bounds: Rect) -> Rect { + self.frame.place(bounds); + bounds + } + + fn event(&mut self, ctx: &mut EventCtx, event: Event) -> Option { + let page_index = self.frame.inner().page_index as u8; + if let Some(repeated_indices) = &self.repeated_indices { + if repeated_indices.contains(&(page_index as usize)) { + let updated_subtitle = TString::from_translation(TR::reset__the_word_is_repeated); + self.frame + .update_subtitle(ctx, updated_subtitle, Some(theme::TEXT_SUB_GREEN_LIME)); + } else { + self.frame + .update_subtitle(ctx, self.subtitle, Some(theme::TEXT_SUB_GREY)); + } + } + self.frame.update_footer_counter(ctx, page_index + 1); + self.frame.event(ctx, event) + } + + fn paint(&mut self) { + // TODO: remove when ui-t3t1 done + } + + fn render<'s>(&'s self, target: &mut impl Renderer<'s>) { + self.frame.render(target); + } + + #[cfg(feature = "ui_bounds")] + fn bounds(&self, _sink: &mut dyn FnMut(Rect)) {} +} + +#[cfg(feature = "micropython")] +impl<'a> crate::ui::flow::Swipable for ShareWords<'a> { + fn get_swipe_config(&self) -> SwipeConfig { + self.frame.get_swipe_config() + } + + fn get_internal_page_count(&self) -> usize { + self.frame.get_internal_page_count() + } +} + +#[cfg(feature = "ui_debug")] +impl<'a> crate::trace::Trace for ShareWords<'a> { + fn trace(&self, t: &mut dyn crate::trace::Tracer) { + t.component("ShareWords"); + t.child("inner", &self.frame); + } +} + +struct ShareWordsInner<'a> { share_words: Vec, MAX_WORDS>, page_index: i16, next_index: i16, @@ -35,12 +138,11 @@ pub struct ShareWords<'a> { progress: i16, } -impl<'a> ShareWords<'a> { +impl<'a> ShareWordsInner<'a> { const AREA_WORD_HEIGHT: i16 = 91; - pub fn new(share_words: Vec, MAX_WORDS>) -> Self { + fn new(share_words: Vec, MAX_WORDS>) -> Self { Self { - area: Rect::zero(), share_words, page_index: 0, next_index: 0, @@ -76,25 +178,21 @@ impl<'a> ShareWords<'a> { } } -impl<'a> Component for ShareWords<'a> { +impl<'a> Component for ShareWordsInner<'a> { type Msg = Never; fn place(&mut self, bounds: Rect) -> Rect { - self.area = bounds; let used_area = bounds .inset(Insets::sides(theme::SPACING)) .inset(Insets::bottom(theme::SPACING)); self.area_word = Rect::snap( used_area.center(), - Offset::new(used_area.width(), ShareWords::AREA_WORD_HEIGHT), + Offset::new(used_area.width(), ShareWordsInner::AREA_WORD_HEIGHT), Alignment2D::CENTER, ); - self.footer - .place(used_area.split_bottom(Footer::HEIGHT_SIMPLE).1); - - self.area + bounds } fn event(&mut self, ctx: &mut EventCtx, event: Event) -> Option { @@ -199,12 +297,61 @@ impl<'a> Component for ShareWords<'a> { } #[cfg(feature = "ui_debug")] -impl<'a> crate::trace::Trace for ShareWords<'a> { +impl<'a> crate::trace::Trace for ShareWordsInner<'a> { fn trace(&self, t: &mut dyn crate::trace::Tracer) { - t.component("ShareWords"); + t.component("ShareWordsInner"); let word = &self.share_words[self.page_index as usize]; let content = word.map(|w| uformat!("{}. {}\n", self.page_index + 1, w)); t.string("screen_content", content.as_str().into()); t.int("page_count", self.share_words.len() as i64) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_find_repeated_indices() { + let words0 = []; + let words1 = [ + TString::from_str("aaa"), + TString::from_str("bbb"), + TString::from_str("ccc"), + ]; + let words2 = [ + TString::from_str("aaa"), + TString::from_str("aaa"), + TString::from_str("bbb"), + ]; + let words3 = [ + TString::from_str("aaa"), + TString::from_str("aaa"), + TString::from_str("bbb"), + TString::from_str("bbb"), + TString::from_str("aaa"), + ]; + let words4 = [ + TString::from_str("aaa"), + TString::from_str("aaa"), + TString::from_str("aaa"), + TString::from_str("aaa"), + TString::from_str("aaa"), + ]; + + assert_eq!(ShareWords::find_repeated(&words0), IndexVec::new()); + assert_eq!(ShareWords::find_repeated(&words1), IndexVec::new()); + assert_eq!( + ShareWords::find_repeated(&words2), + IndexVec::from_slice(&[1]).unwrap() + ); + assert_eq!( + ShareWords::find_repeated(&words3), + IndexVec::from_slice(&[1, 3, 4]).unwrap() + ); + assert_eq!( + ShareWords::find_repeated(&words4), + IndexVec::from_slice(&[1, 2, 3, 4]).unwrap() + ); + } +} diff --git a/core/embed/rust/src/ui/model_mercury/flow/show_share_words.rs b/core/embed/rust/src/ui/model_mercury/flow/show_share_words.rs index 37337020f2..a044be6b79 100644 --- a/core/embed/rust/src/ui/model_mercury/flow/show_share_words.rs +++ b/core/embed/rust/src/ui/model_mercury/flow/show_share_words.rs @@ -85,6 +85,7 @@ impl ShowShareWords { .and_then(|desc: TString| if desc.is_empty() { None } else { Some(desc) }); let text_info: Obj = kwargs.get(Qstr::MP_QSTR_text_info)?; let text_confirm: TString = kwargs.get(Qstr::MP_QSTR_text_confirm)?.try_into()?; + let highlight_repeated: bool = kwargs.get(Qstr::MP_QSTR_highlight_repeated)?.try_into()?; let nwords = share_words_vec.len(); let mut instructions_paragraphs = ParagraphVecShort::new(); @@ -108,12 +109,8 @@ impl ShowShareWords { .one_button_request(ButtonRequestCode::ResetDevice.with_type("share_words")) .with_pages(move |_| nwords + 2); - let content_words = Frame::left_aligned(title, ShareWords::new(share_words_vec)) - .with_subtitle(subtitle) - .with_swipe(SwipeDirection::Up, SwipeSettings::default()) - .with_swipe(SwipeDirection::Down, SwipeSettings::default()) - .with_vertical_pages() - .map(|_| None); + let content_words = + ShareWords::new(title, subtitle, share_words_vec, highlight_repeated).map(|_| None); let content_confirm = Frame::left_aligned(text_confirm, PromptScreen::new_hold_to_confirm()) diff --git a/core/embed/rust/src/ui/model_mercury/layout.rs b/core/embed/rust/src/ui/model_mercury/layout.rs index 3615cd2c77..1810aa959a 100644 --- a/core/embed/rust/src/ui/model_mercury/layout.rs +++ b/core/embed/rust/src/ui/model_mercury/layout.rs @@ -1710,6 +1710,7 @@ pub static mp_module_trezorui2: Module = obj_module! { /// description: str, /// text_info: Iterable[str], /// text_confirm: str, + /// highlight_repeated: bool, /// ) -> LayoutObj[UiResult]: /// """Show wallet backup words preceded by an instruction screen and followed by /// confirmation.""" diff --git a/core/embed/rust/src/ui/model_mercury/theme/mod.rs b/core/embed/rust/src/ui/model_mercury/theme/mod.rs index 1e5909e2ab..21e65597ff 100644 --- a/core/embed/rust/src/ui/model_mercury/theme/mod.rs +++ b/core/embed/rust/src/ui/model_mercury/theme/mod.rs @@ -717,6 +717,8 @@ pub const TEXT_MAIN_GREY_LIGHT: TextStyle = TextStyle::new(Font::NORMAL, GREY_LIGHT, BG, GREY, GREY); pub const TEXT_SUB_GREY_LIGHT: TextStyle = TextStyle::new(Font::SUB, GREY_LIGHT, BG, GREY, GREY); pub const TEXT_SUB_GREY: TextStyle = TextStyle::new(Font::SUB, GREY, BG, GREY, GREY); +pub const TEXT_SUB_GREEN_LIME: TextStyle = + TextStyle::new(Font::SUB, GREEN_LIME, BG, GREEN_LIME, GREEN_LIME); pub const TEXT_WARNING: TextStyle = TextStyle::new(Font::NORMAL, ORANGE_LIGHT, BG, GREY, GREY); pub const TEXT_MONO: TextStyle = TextStyle::new(Font::MONO, GREY_EXTRA_LIGHT, BG, GREY, GREY) .with_line_breaking(LineBreaking::BreakWordsNoHyphen) diff --git a/core/mocks/generated/trezorui2.pyi b/core/mocks/generated/trezorui2.pyi index 3fd35708d5..b5d3856fc7 100644 --- a/core/mocks/generated/trezorui2.pyi +++ b/core/mocks/generated/trezorui2.pyi @@ -405,6 +405,7 @@ def flow_show_share_words( description: str, text_info: Iterable[str], text_confirm: str, + highlight_repeated: bool, ) -> LayoutObj[UiResult]: """Show wallet backup words preceded by an instruction screen and followed by confirmation.""" diff --git a/core/mocks/trezortranslate_keys.pyi b/core/mocks/trezortranslate_keys.pyi index ecc3341de0..472a8870a5 100644 --- a/core/mocks/trezortranslate_keys.pyi +++ b/core/mocks/trezortranslate_keys.pyi @@ -664,6 +664,7 @@ class TR: reset__slip39_checklist_write_down: str = "Write down and check all shares" reset__slip39_checklist_write_down_recovery: str = "Write down & check all wallet backup shares" reset__the_threshold_sets_the_number_of_shares: str = "The threshold sets the number of shares " + reset__the_word_is_repeated: str = "The word is repeated" reset__threshold_info: str = "= minimum number of unique word lists used for recovery." reset__title_backup_is_done: str = "Backup is done" reset__title_create_wallet: str = "Create wallet" diff --git a/core/src/trezor/ui/layouts/mercury/reset.py b/core/src/trezor/ui/layouts/mercury/reset.py index e3cc9b0ae2..a7ff154c55 100644 --- a/core/src/trezor/ui/layouts/mercury/reset.py +++ b/core/src/trezor/ui/layouts/mercury/reset.py @@ -23,6 +23,7 @@ async def show_share_words( ) -> None: title = TR.reset__recovery_wallet_backup_title + highlight_repeated = True if share_index is None: subtitle = "" elif group_index is None: @@ -51,6 +52,7 @@ async def show_share_words( description=description, text_info=text_info, text_confirm=text_confirm, + highlight_repeated=highlight_repeated, ) ) diff --git a/core/translations/en.json b/core/translations/en.json index 9d785d50ec..f4148d799f 100644 --- a/core/translations/en.json +++ b/core/translations/en.json @@ -666,6 +666,7 @@ "reset__slip39_checklist_write_down": "Write down and check all shares", "reset__slip39_checklist_write_down_recovery": "Write down & check all wallet backup shares", "reset__the_threshold_sets_the_number_of_shares": "The threshold sets the number of shares ", + "reset__the_word_is_repeated": "The word is repeated", "reset__threshold_info": "= minimum number of unique word lists used for recovery.", "reset__title_backup_is_done": "Backup is done", "reset__title_create_wallet": "Create wallet", diff --git a/core/translations/order.json b/core/translations/order.json index 23d3a0db17..076ca0eda1 100644 --- a/core/translations/order.json +++ b/core/translations/order.json @@ -939,5 +939,6 @@ "937": "reset__words_may_repeat", "938": "reset__repeat_for_all_shares", "939": "homescreen__settings_subtitle", - "940": "homescreen__settings_title" + "940": "homescreen__settings_title", + "941": "reset__the_word_is_repeated" } diff --git a/core/translations/signatures.json b/core/translations/signatures.json index b6324b7af4..5e71313709 100644 --- a/core/translations/signatures.json +++ b/core/translations/signatures.json @@ -1,8 +1,8 @@ { "current": { - "merkle_root": "5b5781c3374ff27125228c121ab97436a2ba1f0581657ee56b9fa601fe3bde97", - "datetime": "2024-06-24T13:47:55.544267", - "commit": "d79768cec726c26ac1f82e62fc71b6d4568786a2" + "merkle_root": "e283dd87dd502e2b2c3e2cbe1f52efcbe99c2bf6cd8ed883ed26800a6885e4e7", + "datetime": "2024-06-24T12:47:16.181365", + "commit": "c66a73b895b37f2a9de3ac6427f372c649ecea8d" }, "history": [ {