From 2c6a13064b97fc01557f46ef3ddb0b931b4e0547 Mon Sep 17 00:00:00 2001 From: obrusvit Date: Fri, 4 Oct 2024 17:17:14 +0200 Subject: [PATCH] feat(cardano): use nicer summary for send tx Also show Recipient {i} for simple tx --- core/.changelog.d/4284.changed | 1 + core/embed/rust/librust_qstr.h | 1 + .../ui/model_mercury/flow/confirm_summary.rs | 10 +++- .../embed/rust/src/ui/model_mercury/layout.rs | 1 + core/mocks/generated/trezorui2.pyi | 1 + core/src/apps/cardano/layout.py | 60 +++++++++++++++---- .../apps/cardano/sign_tx/multisig_signer.py | 4 +- .../apps/cardano/sign_tx/ordinary_signer.py | 3 +- .../src/apps/cardano/sign_tx/plutus_signer.py | 4 +- core/src/apps/cardano/sign_tx/signer.py | 29 ++++----- .../src/trezor/ui/layouts/mercury/__init__.py | 21 ++++++- core/src/trezor/ui/layouts/tr/__init__.py | 31 ++++++++++ core/src/trezor/ui/layouts/tt/__init__.py | 14 +++++ 13 files changed, 147 insertions(+), 33 deletions(-) create mode 100644 core/.changelog.d/4284.changed diff --git a/core/.changelog.d/4284.changed b/core/.changelog.d/4284.changed new file mode 100644 index 0000000000..be07fa3311 --- /dev/null +++ b/core/.changelog.d/4284.changed @@ -0,0 +1 @@ +Simplified UI of Cardano transactions initiated by Trezor Suite. diff --git a/core/embed/rust/librust_qstr.h b/core/embed/rust/librust_qstr.h index 7ac29af8f4..84eb207b34 100644 --- a/core/embed/rust/librust_qstr.h +++ b/core/embed/rust/librust_qstr.h @@ -34,6 +34,7 @@ static void _librust_qstrs(void) { MP_QSTR___name__; MP_QSTR_account; MP_QSTR_account_items; + MP_QSTR_account_items_title; MP_QSTR_account_label; MP_QSTR_account_path; MP_QSTR_accounts; diff --git a/core/embed/rust/src/ui/model_mercury/flow/confirm_summary.rs b/core/embed/rust/src/ui/model_mercury/flow/confirm_summary.rs index 8df9baa749..41f9e7aec3 100644 --- a/core/embed/rust/src/ui/model_mercury/flow/confirm_summary.rs +++ b/core/embed/rust/src/ui/model_mercury/flow/confirm_summary.rs @@ -86,6 +86,10 @@ impl ConfirmSummary { let title: TString = kwargs.get(Qstr::MP_QSTR_title)?.try_into()?; let items: Obj = kwargs.get(Qstr::MP_QSTR_items)?; let account_items: Obj = kwargs.get(Qstr::MP_QSTR_account_items)?; + let account_items_title: Option = kwargs + .get(Qstr::MP_QSTR_account_items_title) + .unwrap_or(Obj::const_none()) + .try_into_option()?; let fee_items: Obj = kwargs.get(Qstr::MP_QSTR_fee_items)?; let br_name: TString = kwargs.get(Qstr::MP_QSTR_br_name)?.try_into()?; let br_code: u16 = kwargs.get(Qstr::MP_QSTR_br_code)?.try_into()?; @@ -134,7 +138,9 @@ impl ConfirmSummary { // AccountInfo let mut has_account_info = false; - let mut account = ShowInfoParams::new(TR::send__send_from.into()).with_cancel_button(); + let mut account = + ShowInfoParams::new(account_items_title.unwrap_or(TR::send__send_from.into())) + .with_cancel_button(); for pair in IterBuf::new().try_iterate(account_items)? { let [label, value]: [TString; 2] = util::iter_into_array(pair)?; account = unwrap!(account.add(label, value)); @@ -155,7 +161,7 @@ impl ConfirmSummary { if has_account_info { menu = menu.item( theme::ICON_CHEVRON_RIGHT, - TR::address_details__account_info.into(), + account_items_title.unwrap_or(TR::address_details__account_info.into()), ); unwrap!(menu_items.push(MENU_ITEM_ACCOUNT_INFO)); } diff --git a/core/embed/rust/src/ui/model_mercury/layout.rs b/core/embed/rust/src/ui/model_mercury/layout.rs index 891f91a32a..652d75c1fe 100644 --- a/core/embed/rust/src/ui/model_mercury/layout.rs +++ b/core/embed/rust/src/ui/model_mercury/layout.rs @@ -1702,6 +1702,7 @@ pub static mp_module_trezorui2: Module = obj_module! { /// title: str, /// items: Iterable[tuple[str, str]], /// account_items: Iterable[tuple[str, str]], + /// account_items_title: str | None, /// fee_items: Iterable[tuple[str, str]], /// br_code: ButtonRequestType, /// br_name: str, diff --git a/core/mocks/generated/trezorui2.pyi b/core/mocks/generated/trezorui2.pyi index 2e7fb95d72..5f5750ba4f 100644 --- a/core/mocks/generated/trezorui2.pyi +++ b/core/mocks/generated/trezorui2.pyi @@ -600,6 +600,7 @@ def flow_confirm_summary( title: str, items: Iterable[tuple[str, str]], account_items: Iterable[tuple[str, str]], + account_items_title: str | None, fee_items: Iterable[tuple[str, str]], br_code: ButtonRequestType, br_name: str, diff --git a/core/src/apps/cardano/layout.py b/core/src/apps/cardano/layout.py index 6605ac73fe..55b1052b6f 100644 --- a/core/src/apps/cardano/layout.py +++ b/core/src/apps/cardano/layout.py @@ -218,11 +218,17 @@ async def confirm_sending( ada_amount: int, to: str, output_type: Literal["address", "change", "collateral-return"], + output_index: int | None, network_id: int, chunkify: bool, ) -> None: + output_index_shown = None if output_type == "address": - title = TR.cardano__sending + if output_index is None: + title = TR.cardano__sending + else: + title = None + output_index_shown = output_index elif output_type == "change": title = TR.cardano__change_output elif output_type == "collateral-return": @@ -236,6 +242,7 @@ async def confirm_sending( title, br_code=ButtonRequestType.Other, chunkify=chunkify, + output_index=output_index_shown, ) @@ -506,20 +513,49 @@ async def confirm_witness_request( async def confirm_tx( + spending: int, fee: int, network_id: int, protocol_magic: int, ttl: int | None, validity_interval_start: int | None, +) -> None: + total_amount = format_coin_amount(spending, network_id) + fee_amount = format_coin_amount(fee, network_id) + items = ( + (TR.cardano__network, f"{protocol_magics.to_ui_string(protocol_magic)}"), + (TR.cardano__valid_since, f"{format_optional_int(validity_interval_start)}"), + (TR.cardano__ttl, f"{format_optional_int(ttl)}"), + ) + + await layouts.confirm_cardano_tx( + total_amount, + fee_amount, + items=items, + ) + + +async def confirm_tx_details( + network_id: int, + protocol_magic: int, + ttl: int | None, + fee: int | None, + validity_interval_start: int | None, total_collateral: int | None, is_network_id_verifiable: bool, tx_hash: bytes | None, ) -> None: - props: list[PropertyType] = [ - (TR.cardano__transaction_fee, format_coin_amount(fee, network_id)), - ] + props: list[PropertyType] = [] append = props.append # local_cache_attribute + if fee is not None: + append( + ( + TR.cardano__transaction_fee, + format_coin_amount(fee, network_id), + ) + ) + if total_collateral is not None: append( ( @@ -547,13 +583,14 @@ async def confirm_tx( if tx_hash: append((TR.cardano__transaction_id, tx_hash)) - await confirm_properties( - "confirm_total", - TR.cardano__confirm_transaction, - props, - hold=True, - br_code=BRT_Other, - ) + if props: + await confirm_properties( + "confirm_total", + TR.cardano__confirm_transaction, + props, + hold=True, + br_code=BRT_Other, + ) async def confirm_certificate( @@ -590,6 +627,7 @@ async def confirm_certificate( "confirm_certificate", TR.cardano__confirm_transaction, props, + hold=False, br_code=BRT_Other, ) diff --git a/core/src/apps/cardano/sign_tx/multisig_signer.py b/core/src/apps/cardano/sign_tx/multisig_signer.py index f54cc64c60..11e2b03057 100644 --- a/core/src/apps/cardano/sign_tx/multisig_signer.py +++ b/core/src/apps/cardano/sign_tx/multisig_signer.py @@ -33,11 +33,11 @@ class MultisigSigner(Signer): # super() omitted intentionally is_network_id_verifiable = self._is_network_id_verifiable() - await layout.confirm_tx( - msg.fee, + await layout.confirm_tx_details( msg.network_id, msg.protocol_magic, msg.ttl, + msg.fee, msg.validity_interval_start, msg.total_collateral, is_network_id_verifiable, diff --git a/core/src/apps/cardano/sign_tx/ordinary_signer.py b/core/src/apps/cardano/sign_tx/ordinary_signer.py index 4cedd4b942..bdd0c26cc6 100644 --- a/core/src/apps/cardano/sign_tx/ordinary_signer.py +++ b/core/src/apps/cardano/sign_tx/ordinary_signer.py @@ -92,8 +92,9 @@ class OrdinarySigner(Signer): # super() omitted intentionally msg = self.msg # local_cache_attribute if self.suite_tx_type is SuiteTxType.SIMPLE_SEND: + spending = self.total_out + msg.fee - self.change_out await layout.confirm_tx( - self.total_amount, + spending, msg.fee, msg.network_id, msg.protocol_magic, diff --git a/core/src/apps/cardano/sign_tx/plutus_signer.py b/core/src/apps/cardano/sign_tx/plutus_signer.py index 7d97163e08..33d870d7ea 100644 --- a/core/src/apps/cardano/sign_tx/plutus_signer.py +++ b/core/src/apps/cardano/sign_tx/plutus_signer.py @@ -38,11 +38,11 @@ class PlutusSigner(Signer): # computed by a trusted device (in case the tx contains many items which are # tedious to check one by one on the Trezor screen). is_network_id_verifiable = self._is_network_id_verifiable() - await layout.confirm_tx( - msg.fee, + await layout.confirm_tx_details( msg.network_id, msg.protocol_magic, msg.ttl, + msg.fee, msg.validity_interval_start, msg.total_collateral, is_network_id_verifiable, diff --git a/core/src/apps/cardano/sign_tx/signer.py b/core/src/apps/cardano/sign_tx/signer.py index 336572a2b2..6c64032be5 100644 --- a/core/src/apps/cardano/sign_tx/signer.py +++ b/core/src/apps/cardano/sign_tx/signer.py @@ -109,6 +109,8 @@ class Signer: self.msg = msg self.keychain = keychain + self.total_out = 0 # sum of output amounts + self.change_out = 0 # sum of change amounts self.account_path_checker = AccountPathChecker() @@ -257,7 +259,6 @@ class Signer: raise ProcessError("Total collateral is out of range!") validate_network_info(msg.network_id, msg.protocol_magic) - async def _show_tx_init(self) -> None: self.should_show_details = await layout.show_tx_init(self.SIGNING_MODE_TITLE) @@ -292,25 +293,25 @@ class Signer: # outputs async def _process_outputs(self, outputs_list: HashBuilderList) -> None: - total_amount = 0 - for _ in range(self.msg.outputs_count): + for output_index in range(self.msg.outputs_count): output: CardanoTxOutput = await ctx_call( CardanoTxItemAck(), CardanoTxOutput ) - await self._process_output(outputs_list, output) + await self._process_output(outputs_list, output, output_index) + self.total_out += output.amount + if self._is_change_output(output): + self.change_out += output.amount - total_amount += output.amount - - if total_amount > LOVELACE_MAX_SUPPLY: + if self.total_out > LOVELACE_MAX_SUPPLY: raise ProcessError("Total transaction amount is out of range!") async def _process_output( - self, outputs_list: HashBuilderList, output: CardanoTxOutput + self, outputs_list: HashBuilderList, output: CardanoTxOutput, output_index: int ) -> None: self._validate_output(output) should_show = self._should_show_output(output) if should_show: - await self._show_output_init(output) + await self._show_output_init(output, output_index) output_items_count = 2 + sum( ( @@ -371,7 +372,9 @@ class Signer: self.account_path_checker.add_output(output) - async def _show_output_init(self, output: CardanoTxOutput) -> None: + async def _show_output_init( + self, output: CardanoTxOutput, output_index: int + ) -> None: address_type = self._get_output_address_type(output) if ( output.datum_hash is None @@ -395,14 +398,11 @@ class Signer: assert output.address is not None # _validate_output address = output.address - if self.suite_tx_type == SuiteTxType.SIMPLE_SEND: - output_type = n - else: - output_type = "change" if self._is_change_output(output) else "address" await layout.confirm_sending( output.amount, address, "change" if self._is_change_output(output) else "address", + output_index if self.suite_tx_type is SuiteTxType.SIMPLE_SEND else None, self.msg.network_id, chunkify=bool(self.msg.chunkify), ) @@ -1078,6 +1078,7 @@ class Signer: output.amount, address, "collateral-return", + None, self.msg.network_id, chunkify=bool(self.msg.chunkify), ) diff --git a/core/src/trezor/ui/layouts/mercury/__init__.py b/core/src/trezor/ui/layouts/mercury/__init__.py index 9e819f44b0..8f24a776e4 100644 --- a/core/src/trezor/ui/layouts/mercury/__init__.py +++ b/core/src/trezor/ui/layouts/mercury/__init__.py @@ -993,6 +993,7 @@ def confirm_total( items=items, fee_items=fee_items, account_items=account_items, + account_items_title=None, br_name=br_name, br_code=br_code, cancel_text=TR.send__cancel_sign, @@ -1011,7 +1012,6 @@ def _confirm_summary( br_code: ButtonRequestType = ButtonRequestType.SignTx, cancel_text: str | None = None, ) -> Awaitable[None]: - # TODO: info_title title = title or TR.words__title_summary # def_arg return raise_if_not_confirmed( @@ -1021,6 +1021,7 @@ def _confirm_summary( items=items or (), fee_items=fee_items or (), account_items=info_items or (), + account_items_title=info_title, br_name=br_name, br_code=br_code, cancel_text=cancel_text, @@ -1137,6 +1138,24 @@ if not utils.BITCOIN_ONLY: br_code=br_code, ) + def confirm_cardano_tx( + amount: str, + fee: str, + items: Iterable[tuple[str, str]], + ) -> Awaitable[None]: + amount_title = TR.send__total_amount + fee_title = TR.send__incl_transaction_fee + more_info_title = TR.buttons__more_info + + return _confirm_summary( + items=((amount_title, amount), (fee_title, fee)), + info_items=items, + info_title=more_info_title, + fee_items=None, + br_name="confirm_cardano_tx", + br_code=ButtonRequestType.SignTx, + ) + def confirm_joint_total(spending_amount: str, total_amount: str) -> Awaitable[None]: return _confirm_summary( diff --git a/core/src/trezor/ui/layouts/tr/__init__.py b/core/src/trezor/ui/layouts/tr/__init__.py index 54105ffeb1..0ba895216b 100644 --- a/core/src/trezor/ui/layouts/tr/__init__.py +++ b/core/src/trezor/ui/layouts/tr/__init__.py @@ -1228,6 +1228,7 @@ if not utils.BITCOIN_ONLY: amount_value=amount_value, fee_title=f"{TR.send__maximum_fee}:", fee_value=maximum_fee, + items_title=TR.confirm_total__title_fee, items=[(f"{k}:", v) for (k, v) in info_items], cancel_cross=True, ) @@ -1258,6 +1259,7 @@ if not utils.BITCOIN_ONLY: amount_value=amount, fee_title=fee_title, fee_value=fee, + items_title=TR.confirm_total__title_fee, items=items, cancel_cross=True, ) @@ -1267,6 +1269,34 @@ if not utils.BITCOIN_ONLY: ) ) + def confirm_cardano_tx( + amount: str, + fee: str, + items: Iterable[tuple[str, str]], + amount_title: str | None = None, + fee_title: str | None = None, + ) -> Awaitable[None]: + amount_title = f"{TR.send__total_amount}:" + fee_title = TR.send__including_fee + + return raise_if_not_confirmed( + interact( + RustLayout( + trezorui2.altcoin_tx_summary( + amount_title=amount_title, + amount_value=amount, + fee_title=fee_title, + fee_value=fee, + items_title=TR.words__title_information, + items=items, + cancel_cross=True, + ) + ), + br_name="confirm_cardano_tx", + br_code=ButtonRequestType.SignTx, + ) + ) + async def confirm_ethereum_tx( recipient: str, total_amount: str, @@ -1284,6 +1314,7 @@ if not utils.BITCOIN_ONLY: amount_value=total_amount, fee_title=f"{TR.send__maximum_fee}:", fee_value=maximum_fee, + items_title=TR.confirm_total__title_fee, items=[(f"{k}:", v) for (k, v) in fee_info_items], ) ) diff --git a/core/src/trezor/ui/layouts/tt/__init__.py b/core/src/trezor/ui/layouts/tt/__init__.py index ba9d86fec1..66b521cee2 100644 --- a/core/src/trezor/ui/layouts/tt/__init__.py +++ b/core/src/trezor/ui/layouts/tt/__init__.py @@ -1208,6 +1208,20 @@ if not utils.BITCOIN_ONLY: br_code=br_code, ) + def confirm_cardano_tx( + amount: str, + fee: str, + items: Iterable[tuple[str, str]], + ) -> Awaitable[None]: + amount_title = f"{TR.send__total_amount}:" + fee_title = TR.send__including_fee + return _confirm_summary( + ((amount_title, amount), (fee_title, fee)), + info_items=items, + br_name="confirm_cardano_tx", + br_code=ButtonRequestType.SignTx, + ) + def confirm_joint_total(spending_amount: str, total_amount: str) -> Awaitable[None]: return raise_if_not_confirmed(