diff --git a/core/embed/rust/librust_qstr.h b/core/embed/rust/librust_qstr.h index ad9a53301e..4d4e42bf61 100644 --- a/core/embed/rust/librust_qstr.h +++ b/core/embed/rust/librust_qstr.h @@ -130,5 +130,6 @@ static void _librust_qstrs(void) { MP_QSTR_fee_rate_amount; MP_QSTR_total_label; MP_QSTR_fee_label; - MP_QSTR_truncated_address; + MP_QSTR_address_title; + MP_QSTR_amount_title; } diff --git a/core/embed/rust/src/micropython/buffer.rs b/core/embed/rust/src/micropython/buffer.rs index 2ddd00e8d1..697700f6f9 100644 --- a/core/embed/rust/src/micropython/buffer.rs +++ b/core/embed/rust/src/micropython/buffer.rs @@ -20,7 +20,7 @@ use super::ffi; /// The `off` field represents offset from the `ptr` and allows us to do /// substring slices while keeping the head pointer as required by GC. #[repr(C)] -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Copy)] pub struct StrBuffer { ptr: *const u8, len: u16, diff --git a/core/embed/rust/src/ui/debug.rs b/core/embed/rust/src/ui/debug.rs index c448636b73..f7d7c756bb 100644 --- a/core/embed/rust/src/ui/debug.rs +++ b/core/embed/rust/src/ui/debug.rs @@ -106,7 +106,7 @@ impl Font { #[cfg(feature = "model_tr")] impl ButtonDetails { pub fn print(&self) { - let text: String<20> = if let Some(text) = self.text.clone() { + let text: String<20> = if let Some(text) = self.text { text.as_ref().into() } else { "None".into() 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 f2ebef18f3..e9e9cc40c4 100644 --- a/core/embed/rust/src/ui/model_tr/component/button.rs +++ b/core/embed/rust/src/ui/model_tr/component/button.rs @@ -644,7 +644,7 @@ impl crate::trace::Trace for ButtonDetails { /// What happens when a button is triggered. /// Theoretically any action can be connected /// with any button. -#[derive(Clone, PartialEq, Eq)] +#[derive(Clone, PartialEq, Eq, Copy)] pub enum ButtonAction { /// Go to the next page of this flow NextPage, @@ -699,7 +699,7 @@ impl ButtonAction { } /// Storing actions for all three possible buttons. -#[derive(Clone)] +#[derive(Clone, Copy)] pub struct ButtonActions { pub left: Option, pub middle: Option, @@ -799,9 +799,9 @@ impl ButtonActions { /// Having access to appropriate action based on the `ButtonPos` pub fn get_action(&self, pos: ButtonPos) -> Option { match pos { - ButtonPos::Left => self.left.clone(), - ButtonPos::Middle => self.middle.clone(), - ButtonPos::Right => self.right.clone(), + ButtonPos::Left => self.left, + ButtonPos::Middle => self.middle, + ButtonPos::Right => self.right, } } } diff --git a/core/embed/rust/src/ui/model_tr/component/button_controller.rs b/core/embed/rust/src/ui/model_tr/component/button_controller.rs index 967c96b7bb..e2f2507a29 100644 --- a/core/embed/rust/src/ui/model_tr/component/button_controller.rs +++ b/core/embed/rust/src/ui/model_tr/component/button_controller.rs @@ -68,7 +68,7 @@ impl ButtonType { /// Create `Button` component from `btn_details`. fn get_button(pos: ButtonPos, btn_details: ButtonDetails) -> Button { // Deciding between text and icon - if let Some(text) = btn_details.clone().text { + if let Some(text) = btn_details.text { Button::with_text(pos, text, btn_details.style()) } else if let Some(icon) = btn_details.icon { Button::with_icon(pos, icon, btn_details.style()) diff --git a/core/embed/rust/src/ui/model_tr/component/flow.rs b/core/embed/rust/src/ui/model_tr/component/flow.rs index ca539d3e5e..51d583f522 100644 --- a/core/embed/rust/src/ui/model_tr/component/flow.rs +++ b/core/embed/rust/src/ui/model_tr/component/flow.rs @@ -1,14 +1,14 @@ use crate::{ micropython::buffer::StrBuffer, ui::{ - component::{Child, Component, Event, EventCtx, Pad}, + component::{Child, Component, ComponentExt, Event, EventCtx, Pad}, geometry::Rect, }, }; use super::{ common, theme, ButtonAction, ButtonController, ButtonControllerMsg, ButtonLayout, ButtonPos, - FlowPages, Page, + FlowPages, Page, ScrollBar, }; /// To be returned directly from Flow. @@ -18,9 +18,13 @@ pub enum FlowMsg { } pub struct Flow { + /// Function to get pages from pages: FlowPages, + /// Instance of the current Page current_page: Page, + /// Title being shown at the top in bold common_title: Option, + scrollbar: Child, content_area: Rect, title_area: Rect, pad: Pad, @@ -40,6 +44,7 @@ where common_title: None, content_area: Rect::zero(), title_area: Rect::zero(), + scrollbar: Child::new(ScrollBar::to_be_filled_later()), pad: Pad::with_background(theme::BG), // Setting empty layout for now, we do not yet know how many sub-pages the first page // has. Initial button layout will be set in `place()` after we can call @@ -56,13 +61,30 @@ where self } + /// Getting new current page according to page counter. + /// Also updating the possible title and moving the scrollbar to correct + /// position. + fn change_current_page(&mut self) { + self.current_page = self.pages.get(self.page_counter); + if self.common_title.is_some() && let Some(title) = self.current_page.title() { + self.common_title = Some(title); + } + let scrollbar_active_index = self + .pages + .scrollbar_page_index(self.content_area, self.page_counter); + self.scrollbar + .inner_mut() + .set_active_page(scrollbar_active_index); + } + /// Placing current page, setting current buttons and clearing. fn update(&mut self, ctx: &mut EventCtx, get_new_page: bool) { if get_new_page { - self.current_page = self.pages.get(self.page_counter); + self.change_current_page(); } self.current_page.place(self.content_area); self.set_buttons(ctx); + self.scrollbar.request_complete_repaint(ctx); self.clear_and_repaint(ctx); } @@ -117,10 +139,12 @@ where fn event_consumed_by_current_choice(&mut self, ctx: &mut EventCtx, pos: ButtonPos) -> bool { if matches!(pos, ButtonPos::Left) && self.current_page.has_prev_page() { self.current_page.go_to_prev_page(); + self.scrollbar.inner_mut().go_to_previous_page(); self.update(ctx, false); true } else if matches!(pos, ButtonPos::Right) && self.current_page.has_next_page() { self.current_page.go_to_next_page(); + self.scrollbar.inner_mut().go_to_next_page(); self.update(ctx, false); true } else { @@ -146,6 +170,16 @@ where self.title_area = title_area; self.content_area = content_area; + // Placing a scrollbar in case the title is there + if self.common_title.is_some() { + // Finding out the total amount of pages in this flow + let complete_page_count = self.pages.scrollbar_page_count(content_area); + self.scrollbar + .inner_mut() + .set_page_count(complete_page_count); + self.scrollbar.place(title_area); + } + // We finally found how long is the first page, and can set its button layout. self.current_page.place(content_area); self.buttons = Child::new(ButtonController::new(self.current_page.btn_layout())); @@ -198,10 +232,10 @@ where } fn paint(&mut self) { - // TODO: might put horizontal scrollbar at the top right - // (not compatible with longer/centered titles) self.pad.paint(); + // Scrollbars are painted only with a title if let Some(title) = &self.common_title { + self.scrollbar.paint(); common::paint_header_left(title, self.title_area); } self.current_page.paint(); diff --git a/core/embed/rust/src/ui/model_tr/component/flow_pages.rs b/core/embed/rust/src/ui/model_tr/component/flow_pages.rs index d79533edf7..8ab0dfd38c 100644 --- a/core/embed/rust/src/ui/model_tr/component/flow_pages.rs +++ b/core/embed/rust/src/ui/model_tr/component/flow_pages.rs @@ -45,13 +45,33 @@ where } } + /// Returns a page on demand on a specified index. pub fn get(&self, page_index: u8) -> Page { (self.get_page)(page_index) } + /// Total amount of pages. pub fn count(&self) -> u8 { self.page_count } + + /// How many scrollable pages are there in the flow + /// (each page can have arbitrary number of "sub-pages"). + pub fn scrollbar_page_count(&self, bounds: Rect) -> usize { + self.scrollbar_page_index(bounds, self.page_count) + } + + /// Active scrollbar position connected with the beginning of a specific + /// page index + pub fn scrollbar_page_index(&self, bounds: Rect, page_index: u8) -> usize { + let mut page_count = 0; + for i in 0..page_index { + let mut current_page = self.get(i); + current_page.place(bounds); + page_count += current_page.page_count; + } + page_count + } } #[derive(Clone)] @@ -63,6 +83,7 @@ pub struct Page { current_page: usize, page_count: usize, char_offset: usize, + title: Option, } // For `layout.rs` @@ -88,17 +109,30 @@ impl Page { current_page: 0, page_count: 1, char_offset: 0, + title: None, } } } // For `flow.rs` impl Page { + /// Adding title. + pub fn with_title(mut self, title: StrBuffer) -> Self { + self.title = Some(title); + self + } + pub fn paint(&mut self) { self.change_page(self.current_page); self.layout_content(&mut TextRenderer); } + pub fn place(&mut self, bounds: Rect) -> Rect { + self.text_layout.bounds = bounds; + self.page_count = self.page_count(); + bounds + } + pub fn btn_layout(&self) -> ButtonLayout { // When we are in pagination inside this flow, // show the up and down arrows on appropriate sides. @@ -123,14 +157,12 @@ impl Page { ButtonLayout::new(btn_left, current.btn_middle, btn_right) } - pub fn place(&mut self, bounds: Rect) -> Rect { - self.text_layout.bounds = bounds; - self.page_count = self.page_count(); - bounds + pub fn btn_actions(&self) -> ButtonActions { + self.btn_actions } - pub fn btn_actions(&self) -> ButtonActions { - self.btn_actions.clone() + pub fn title(&self) -> Option { + self.title } pub fn has_prev_page(&self) -> bool { @@ -194,17 +226,6 @@ impl Page { // For `layout.rs` - aggregating operations impl Page { - pub fn icon_label_text(self, icon: IconAndName, label: StrBuffer, text: StrBuffer) -> Self { - self.icon_with_offset(icon, 3) - .text_mono(label) - .newline() - .text_bold(text) - } - - pub fn icon_with_offset(self, icon: IconAndName, x_offset: i16) -> Self { - self.icon(icon).offset(Offset::x(x_offset)) - } - pub fn text_normal(self, text: StrBuffer) -> Self { self.font(Font::NORMAL).text(text) } diff --git a/core/embed/rust/src/ui/model_tr/component/flow_pages_poc_helpers.rs b/core/embed/rust/src/ui/model_tr/component/flow_pages_poc_helpers.rs index 36382a3d97..2cee012b9c 100644 --- a/core/embed/rust/src/ui/model_tr/component/flow_pages_poc_helpers.rs +++ b/core/embed/rust/src/ui/model_tr/component/flow_pages_poc_helpers.rs @@ -22,7 +22,7 @@ pub struct ToDisplay { impl ToDisplay { pub fn new(text: StrBuffer) -> Self { Self { - text: text.clone(), + text, length_from_end: text.len(), } } diff --git a/core/embed/rust/src/ui/model_tr/component/hold_to_confirm.rs b/core/embed/rust/src/ui/model_tr/component/hold_to_confirm.rs index 2dbdf8a17f..06ac22199b 100644 --- a/core/embed/rust/src/ui/model_tr/component/hold_to_confirm.rs +++ b/core/embed/rust/src/ui/model_tr/component/hold_to_confirm.rs @@ -39,7 +39,7 @@ impl HoldToConfirm { /// Updating the text of the component and re-placing it. pub fn set_text(&mut self, text: StrBuffer, button_area: Rect) { - self.text_width = self.loader.get_text_width(text.clone()) as i16; + self.text_width = self.loader.get_text_width(text) as i16; self.loader.set_text(text); self.place(button_area); } diff --git a/core/embed/rust/src/ui/model_tr/component/page.rs b/core/embed/rust/src/ui/model_tr/component/page.rs index 25e1ef705c..a3889c508b 100644 --- a/core/embed/rust/src/ui/model_tr/component/page.rs +++ b/core/embed/rust/src/ui/model_tr/component/page.rs @@ -1,7 +1,7 @@ use crate::ui::{ component::{Child, Component, ComponentExt, Event, EventCtx, Pad, PageMsg, Paginate}, display::Color, - geometry::{Insets, Offset, Rect}, + geometry::{Insets, Rect}, }; use super::{ @@ -159,21 +159,8 @@ where // Put it into its dedicated area when parent component already chose it, // otherwise place it into the right top of the content. if self.show_scrollbar { - let max_scrollbar_area = if let Some(scrollbar_area) = self.parent_scrollbar_area { - scrollbar_area - } else { - content_area - }; - // Occupying as little space as possible (according to the number of pages), - // aligning to the right. - let min_scrollbar_area = Rect::from_top_right_and_size( - max_scrollbar_area.top_right(), - Offset::new( - self.scrollbar.inner().overall_width(), - ScrollBar::MAX_DOT_SIZE, - ), - ); - self.scrollbar.place(min_scrollbar_area); + let scrollbar_area = self.parent_scrollbar_area.unwrap_or(content_area); + self.scrollbar.place(scrollbar_area); } self.buttons.place(button_area); diff --git a/core/embed/rust/src/ui/model_tr/component/scrollbar.rs b/core/embed/rust/src/ui/model_tr/component/scrollbar.rs index 91048bd70c..6c744b3cad 100644 --- a/core/embed/rust/src/ui/model_tr/component/scrollbar.rs +++ b/core/embed/rust/src/ui/model_tr/component/scrollbar.rs @@ -17,10 +17,10 @@ pub struct ScrollBar { /// Carrying the appearance of the scrollbar dot. #[derive(Debug)] enum DotType { - BigFull, - Big, - Middle, - Small, + BigFull, // * + Big, // O + Middle, // o + Small, // . } /// How many dots at most will there be @@ -132,6 +132,9 @@ impl ScrollBar { match self.page_count { 0..=3 => { + // *OO + // O*O + // OO* for i in 0..self.page_count { if i == self.active_page { unwrap!(dots.push(DotType::BigFull)); @@ -141,6 +144,10 @@ impl ScrollBar { } } 4 => { + // *OOo + // O*Oo + // oO*O + // oOO* match self.active_page { 0 => unwrap!(dots.push(DotType::BigFull)), 1 => unwrap!(dots.push(DotType::Big)), @@ -161,6 +168,13 @@ impl ScrollBar { }; } _ => { + // *OOo. + // O*Oo. + // oO*Oo + // ... + // oO*Oo + // .oO*O + // .oOO* let full_dot_index = match self.active_page { 0 => 0, 1 => 1, @@ -214,8 +228,14 @@ impl Component for ScrollBar { type Msg = Never; fn place(&mut self, bounds: Rect) -> Rect { - self.pad.place(bounds); - bounds + // Occupying as little space as possible (according to the number of pages), + // aligning to the right. + let scrollbar_area = Rect::from_top_right_and_size( + bounds.top_right(), + Offset::new(self.overall_width(), Self::MAX_DOT_SIZE), + ); + self.pad.place(scrollbar_area); + scrollbar_area } fn event(&mut self, _ctx: &mut EventCtx, _event: Event) -> Option { diff --git a/core/embed/rust/src/ui/model_tr/component/share_words.rs b/core/embed/rust/src/ui/model_tr/component/share_words.rs index 4fb9f1e97d..8a1675ee10 100644 --- a/core/embed/rust/src/ui/model_tr/component/share_words.rs +++ b/core/embed/rust/src/ui/model_tr/component/share_words.rs @@ -100,7 +100,7 @@ impl ShareWords { for i in 0..WORDS_PER_PAGE { y_offset += NUMBER_FONT.line_height() + EXTRA_LINE_HEIGHT; let index = self.word_index() + i; - let word = self.share_words[index].clone(); + let word = self.share_words[index]; let baseline = self.area.top_left() + Offset::new(NUMBER_X_OFFSET, y_offset); display(baseline, &inttostr!(index as u8 + 1), NUMBER_FONT); display(baseline + Offset::x(NUMBER_WORD_OFFSET), &word, WORD_FONT); @@ -154,7 +154,7 @@ impl crate::trace::Trace for ShareWords { } else { for i in 0..WORDS_PER_PAGE { let index = self.word_index() + i; - let word = self.share_words[index].clone(); + let word = self.share_words[index]; let content = build_string!(20, inttostr!(index as u8 + 1), " ", &word, "\n"); t.string(&content); } diff --git a/core/embed/rust/src/ui/model_tr/layout.rs b/core/embed/rust/src/ui/model_tr/layout.rs index fe8a03f732..3a310923bc 100644 --- a/core/embed/rust/src/ui/model_tr/layout.rs +++ b/core/embed/rust/src/ui/model_tr/layout.rs @@ -285,51 +285,44 @@ extern "C" fn new_confirm_properties(n_args: usize, args: *const Obj, kwargs: *m extern "C" fn new_confirm_output(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj { let block = |_args: &[Obj], kwargs: &Map| { let address: StrBuffer = kwargs.get(Qstr::MP_QSTR_address)?.try_into()?; - let truncated_address: StrBuffer = - kwargs.get(Qstr::MP_QSTR_truncated_address)?.try_into()?; let amount: StrBuffer = kwargs.get(Qstr::MP_QSTR_amount)?.try_into()?; - let title: StrBuffer = "SEND".into(); + let address_title: StrBuffer = kwargs.get(Qstr::MP_QSTR_address_title)?.try_into()?; + let amount_title: StrBuffer = kwargs.get(Qstr::MP_QSTR_amount_title)?.try_into()?; let get_page = move |page_index| { // Showing two screens - the recipient address and summary confirmation match page_index { 0 => { - // `icon + label + address` + // RECIPIENT + address let btn_layout = ButtonLayout::new( Some(ButtonDetails::cancel_icon()), None, - Some(ButtonDetails::text("CONTINUE".into())), + Some(ButtonDetails::text("CONFIRM".into())), ); let btn_actions = ButtonActions::cancel_next(); - Page::<20>::new(btn_layout, btn_actions, Font::MONO).icon_label_text( - theme::ICON_USER, - "Recipient".into(), - address.clone(), - ) + Page::<10>::new(btn_layout, btn_actions, Font::MONO) + .with_title(address_title) + .text_mono(address) } 1 => { - // 2 pairs `icon + label + text` + // AMOUNT + amount let btn_layout = ButtonLayout::new( - Some(ButtonDetails::cancel_icon()), + Some(ButtonDetails::up_arrow_icon()), None, - Some(ButtonDetails::text("HOLD TO CONFIRM".into()).with_default_duration()), + Some(ButtonDetails::text("CONFIRM".into())), ); let btn_actions = ButtonActions::cancel_confirm(); - Page::<20>::new(btn_layout, btn_actions, Font::MONO) - .icon_label_text( - theme::ICON_USER, - "Recipient".into(), - truncated_address.clone(), - ) + Page::<10>::new(btn_layout, btn_actions, Font::MONO) + .with_title(amount_title) .newline() - .icon_label_text(theme::ICON_AMOUNT, "Amount".into(), amount.clone()) + .text_mono(amount) } _ => unreachable!(), } }; let pages = FlowPages::new(get_page, 2); - let obj = LayoutObj::new(Flow::new(pages).with_common_title(title))?; + let obj = LayoutObj::new(Flow::new(pages).with_common_title(address_title))?; Ok(obj.into()) }; unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } @@ -337,7 +330,6 @@ extern "C" fn new_confirm_output(n_args: usize, args: *const Obj, kwargs: *mut M extern "C" fn new_confirm_total(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj { let block = |_args: &[Obj], kwargs: &Map| { - let title: StrBuffer = kwargs.get(Qstr::MP_QSTR_title)?.try_into()?; let total_amount: StrBuffer = kwargs.get(Qstr::MP_QSTR_total_amount)?.try_into()?; let fee_amount: StrBuffer = kwargs.get(Qstr::MP_QSTR_fee_amount)?.try_into()?; let fee_rate_amount: Option = kwargs @@ -347,33 +339,35 @@ extern "C" fn new_confirm_total(n_args: usize, args: *const Obj, kwargs: *mut Ma let fee_label: StrBuffer = kwargs.get(Qstr::MP_QSTR_fee_label)?.try_into()?; let get_page = move |page_index| { - // One page with 2 or 3 pairs `icon + label + text` + // Total amount + fee assert!(page_index == 0); let btn_layout = ButtonLayout::new( Some(ButtonDetails::cancel_icon()), None, - Some(ButtonDetails::text("HOLD TO SEND".into()).with_default_duration()), + Some(ButtonDetails::text("HOLD TO CONFIRM".into()).with_default_duration()), ); let btn_actions = ButtonActions::cancel_confirm(); - let mut flow_page = Page::<25>::new(btn_layout, btn_actions, Font::MONO) - .icon_label_text(theme::ICON_PARAM, total_label.clone(), total_amount.clone()) + let mut flow_page = Page::<15>::new(btn_layout, btn_actions, Font::MONO) + .text_bold(total_label) .newline() - .icon_label_text(theme::ICON_PARAM, fee_label.clone(), fee_amount.clone()); + .text_mono(total_amount) + .newline() + .text_bold(fee_label) + .newline() + .text_mono(fee_amount); - if let Some(fee_rate_amount) = &fee_rate_amount { - flow_page = flow_page.newline().icon_label_text( - theme::ICON_PARAM, - "Fee rate".into(), - fee_rate_amount.clone(), - ) + // Fee rate amount might not be there + if let Some(fee_rate_amount) = fee_rate_amount { + flow_page = flow_page.newline().text_mono(fee_rate_amount) } + flow_page }; let pages = FlowPages::new(get_page, 1); - let obj = LayoutObj::new(Flow::new(pages).with_common_title(title))?; + let obj = LayoutObj::new(Flow::new(pages))?; Ok(obj.into()) }; unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } @@ -748,15 +742,15 @@ pub static mp_module_trezorui2: Module = obj_module! { /// def confirm_output_r( /// *, /// address: str, - /// truncated_address: str, /// amount: str, + /// address_title: str, + /// amount_title: str, /// ) -> object: /// """Confirm output. Specific for model R.""" Qstr::MP_QSTR_confirm_output_r => obj_fn_kw!(0, new_confirm_output).as_obj(), /// def confirm_total_r( /// *, - /// title: str, /// total_amount: str, /// fee_amount: str, /// fee_rate_amount: str | None = None, diff --git a/core/mocks/generated/trezorui2.pyi b/core/mocks/generated/trezorui2.pyi index 6eaf896ea5..847973c140 100644 --- a/core/mocks/generated/trezorui2.pyi +++ b/core/mocks/generated/trezorui2.pyi @@ -40,8 +40,9 @@ def confirm_properties( def confirm_output_r( *, address: str, - truncated_address: str, amount: str, + address_title: str, + amount_title: str, ) -> object: """Confirm output. Specific for model R.""" @@ -49,7 +50,6 @@ def confirm_output_r( # rust/src/ui/model_tr/layout.rs def confirm_total_r( *, - title: str, total_amount: str, fee_amount: str, fee_rate_amount: str | None = None, diff --git a/core/src/apps/bitcoin/sign_tx/approvers.py b/core/src/apps/bitcoin/sign_tx/approvers.py index 14a4f9c078..c659425a0b 100644 --- a/core/src/apps/bitcoin/sign_tx/approvers.py +++ b/core/src/apps/bitcoin/sign_tx/approvers.py @@ -115,6 +115,7 @@ class Approver: self, txo: TxOutput, script_pubkey: bytes, + index: int | None, orig_txo: TxOutput | None = None, ) -> None: self._add_output(txo, script_pubkey) @@ -168,11 +169,12 @@ class BasicApprover(Approver): self, txo: TxOutput, script_pubkey: bytes, + index: int | None, orig_txo: TxOutput | None = None, ) -> None: from trezor.enums import OutputScriptType - await super().add_external_output(txo, script_pubkey, orig_txo) + await super().add_external_output(txo, script_pubkey, index, orig_txo) if orig_txo: if txo.amount < orig_txo.amount: @@ -207,7 +209,7 @@ class BasicApprover(Approver): elif txo.payment_req_index is None or self.show_payment_req_details: # Ask user to confirm output, unless it is part of a payment # request, which gets confirmed separately. - await helpers.confirm_output(txo, self.coin, self.amount_unit) + await helpers.confirm_output(txo, self.coin, self.amount_unit, index) async def add_payment_request( self, msg: TxAckPaymentRequest, keychain: Keychain diff --git a/core/src/apps/bitcoin/sign_tx/bitcoin.py b/core/src/apps/bitcoin/sign_tx/bitcoin.py index 36b4e862de..68115caf8b 100644 --- a/core/src/apps/bitcoin/sign_tx/bitcoin.py +++ b/core/src/apps/bitcoin/sign_tx/bitcoin.py @@ -215,7 +215,7 @@ class Bitcoin: orig_txo: TxOutput | None = None if txo.orig_hash: orig_txo = await self.get_original_output(txo, script_pubkey) - await self.approve_output(txo, script_pubkey, orig_txo) + await self.approve_output(txo, script_pubkey, orig_txo, i) # Finalize original outputs. for orig in self.orig_txs: @@ -499,6 +499,7 @@ class Bitcoin: txo: TxOutput, script_pubkey: bytes, orig_txo: TxOutput | None, + index: int | None, ) -> None: payment_req_index = txo.payment_req_index # local_cache_attribute approver = self.approver # local_cache_attribute @@ -517,7 +518,7 @@ class Bitcoin: # Output is change and does not need approval. approver.add_change_output(txo, script_pubkey) else: - await approver.add_external_output(txo, script_pubkey, orig_txo) + await approver.add_external_output(txo, script_pubkey, index, orig_txo) self.tx_info.add_output(txo, script_pubkey) diff --git a/core/src/apps/bitcoin/sign_tx/decred.py b/core/src/apps/bitcoin/sign_tx/decred.py index 8054d6ce83..a40ef129d9 100644 --- a/core/src/apps/bitcoin/sign_tx/decred.py +++ b/core/src/apps/bitcoin/sign_tx/decred.py @@ -99,7 +99,9 @@ class DecredApprover(BasicApprover): ) -> None: # NOTE: The following calls Approver.add_external_output(), not BasicApprover.add_external_output(). # This is needed to skip calling helpers.confirm_output(), which is what BasicApprover would do. - await super(BasicApprover, self).add_external_output(txo, script_pubkey, None) + await super(BasicApprover, self).add_external_output( + txo, script_pubkey, None, None + ) await helpers.confirm_decred_sstx_submission(txo, self.coin, self.amount_unit) @@ -206,8 +208,9 @@ class Decred(Bitcoin): txo: TxOutput, script_pubkey: bytes, orig_txo: TxOutput | None, + index: int | None, ) -> None: - await super().approve_output(txo, script_pubkey, orig_txo) + await super().approve_output(txo, script_pubkey, orig_txo, index) if self.serialize: self.write_tx_output(self.serialized_tx, txo, script_pubkey) diff --git a/core/src/apps/bitcoin/sign_tx/helpers.py b/core/src/apps/bitcoin/sign_tx/helpers.py index 4601e22af0..857dff6db3 100644 --- a/core/src/apps/bitcoin/sign_tx/helpers.py +++ b/core/src/apps/bitcoin/sign_tx/helpers.py @@ -37,13 +37,22 @@ class UiConfirm: class UiConfirmOutput(UiConfirm): - def __init__(self, output: TxOutput, coin: CoinInfo, amount_unit: AmountUnit): + def __init__( + self, + output: TxOutput, + coin: CoinInfo, + amount_unit: AmountUnit, + index: int | None, + ): self.output = output self.coin = coin self.amount_unit = amount_unit + self.index = index def confirm_dialog(self, ctx: Context) -> Awaitable[Any]: - return layout.confirm_output(ctx, self.output, self.coin, self.amount_unit) + return layout.confirm_output( + ctx, self.output, self.coin, self.amount_unit, self.index + ) class UiConfirmDecredSSTXSubmission(UiConfirm): @@ -213,8 +222,8 @@ class UiConfirmNonDefaultLocktime(UiConfirm): ) -def confirm_output(output: TxOutput, coin: CoinInfo, amount_unit: AmountUnit) -> Awaitable[None]: # type: ignore [awaitable-is-generator] - return (yield UiConfirmOutput(output, coin, amount_unit)) +def confirm_output(output: TxOutput, coin: CoinInfo, amount_unit: AmountUnit, index: int | None) -> Awaitable[None]: # type: ignore [awaitable-is-generator] + return (yield UiConfirmOutput(output, coin, amount_unit, index)) def confirm_decred_sstx_submission(output: TxOutput, coin: CoinInfo, amount_unit: AmountUnit) -> Awaitable[None]: # type: ignore [awaitable-is-generator] diff --git a/core/src/apps/bitcoin/sign_tx/layout.py b/core/src/apps/bitcoin/sign_tx/layout.py index aad81fa0c7..cbb7998003 100644 --- a/core/src/apps/bitcoin/sign_tx/layout.py +++ b/core/src/apps/bitcoin/sign_tx/layout.py @@ -44,7 +44,11 @@ def format_coin_amount(amount: int, coin: CoinInfo, amount_unit: AmountUnit) -> async def confirm_output( - ctx: Context, output: TxOutput, coin: CoinInfo, amount_unit: AmountUnit + ctx: Context, + output: TxOutput, + coin: CoinInfo, + amount_unit: AmountUnit, + index: int | None, ) -> None: from . import omni from trezor.enums import OutputScriptType @@ -83,6 +87,7 @@ async def confirm_output( address_short, format_coin_amount(output.amount, coin, amount_unit), title=title, + index=index, ) await layout diff --git a/core/src/apps/cardano/layout.py b/core/src/apps/cardano/layout.py index 8fe131679a..754b436745 100644 --- a/core/src/apps/cardano/layout.py +++ b/core/src/apps/cardano/layout.py @@ -232,7 +232,7 @@ async def confirm_sending( to, format_coin_amount(ada_amount, network_id), title, - ButtonRequestType.Other, + br_code=ButtonRequestType.Other, ) diff --git a/core/src/trezor/ui/layouts/tr/__init__.py b/core/src/trezor/ui/layouts/tr/__init__.py index 7d347266c1..aa19016eee 100644 --- a/core/src/trezor/ui/layouts/tr/__init__.py +++ b/core/src/trezor/ui/layouts/tr/__init__.py @@ -827,30 +827,20 @@ async def confirm_output( ctx: GenericContext, address: str, amount: str, - subtitle: str = "", title: str = "Confirm sending", + index: int | None = None, br_code: ButtonRequestType = ButtonRequestType.ConfirmOutput, ) -> None: - # Creating the truncated address here, not having to do it in Rust - chars_to_take = 4 - ellipsis = " ... " - truncated_address = address[:chars_to_take] + ellipsis + address[-chars_to_take:] - - # Also splitting the address into chunks delimited by whitespace - chunk_length = 4 - delimiter = " " - address_chunks: list[str] = [] - for i in range(0, len(address), chunk_length): - address_chunks.append(address[i : i + chunk_length]) - address_str = delimiter.join(address_chunks) - + address_title = "RECIPIENT" if index is None else f"RECIPIENT #{index + 1}" + amount_title = "AMOUNT" if index is None else f"AMOUNT #{index + 1}" await raise_if_cancelled( interact( ctx, RustLayout( trezorui2.confirm_output_r( - address=address_str, - truncated_address=truncated_address, + address=address, + address_title=address_title, + amount_title=amount_title, amount=amount, ) ), @@ -1045,9 +1035,8 @@ async def confirm_total( total_amount: str, fee_amount: str, fee_rate_amount: str | None = None, - title: str = "Send transaction?", - total_label: str = "Total amount:", - fee_label: str = "Including fee:", + total_label: str = "Total amount", + fee_label: str = "Including fee", br_type: str = "confirm_total", br_code: ButtonRequestType = ButtonRequestType.SignTx, ) -> None: @@ -1056,12 +1045,11 @@ async def confirm_total( ctx, RustLayout( trezorui2.confirm_total_r( - title=title.upper(), total_amount=total_amount, fee_amount=fee_amount, fee_rate_amount=fee_rate_amount, - total_label=total_label, - fee_label=fee_label, + total_label=total_label.upper(), + fee_label=fee_label.upper(), ) ), br_type, diff --git a/core/src/trezor/ui/layouts/tt_v2/__init__.py b/core/src/trezor/ui/layouts/tt_v2/__init__.py index 8f2c9701e2..65c06b90ec 100644 --- a/core/src/trezor/ui/layouts/tt_v2/__init__.py +++ b/core/src/trezor/ui/layouts/tt_v2/__init__.py @@ -528,12 +528,15 @@ async def confirm_output( address: str, amount: str, title: str = "SENDING", + index: int | None = None, br_code: ButtonRequestType = ButtonRequestType.ConfirmOutput, ) -> None: title = title.upper() if title.startswith("CONFIRM "): title = title[len("CONFIRM ") :] + # TODO: include index to be consistent with TR? + await confirm_value( ctx, title,