From 9a53ba9f44e047f7a8d5039dd3e37d19b0b6be5d Mon Sep 17 00:00:00 2001 From: grdddj Date: Tue, 7 Nov 2023 11:06:08 +0100 Subject: [PATCH] fix(core): unify button requests in modify amount flow [no changelog] --- core/embed/rust/src/ui/model_tr/layout.rs | 6 +-- core/embed/rust/src/ui/model_tt/layout.rs | 3 +- core/mocks/generated/trezorui2.pyi | 6 +-- core/src/trezor/ui/layouts/tr/__init__.py | 49 ++++++++++++++----- core/src/trezor/ui/layouts/tt/__init__.py | 46 +++++++++-------- .../bitcoin/test_signtx_replacement.py | 3 +- tests/input_flows.py | 1 + 7 files changed, 65 insertions(+), 49 deletions(-) diff --git a/core/embed/rust/src/ui/model_tr/layout.rs b/core/embed/rust/src/ui/model_tr/layout.rs index 2ecb02b74e..858a27a849 100644 --- a/core/embed/rust/src/ui/model_tr/layout.rs +++ b/core/embed/rust/src/ui/model_tr/layout.rs @@ -578,7 +578,6 @@ extern "C" fn new_confirm_joint_total(n_args: usize, args: *const Obj, kwargs: * extern "C" fn new_confirm_modify_output(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj { let block = move |_args: &[Obj], kwargs: &Map| { - let address: StrBuffer = kwargs.get(Qstr::MP_QSTR_address)?.try_into()?; let sign: i32 = kwargs.get(Qstr::MP_QSTR_sign)?.try_into()?; let amount_change: StrBuffer = kwargs.get(Qstr::MP_QSTR_amount_change)?.try_into()?; let amount_new: StrBuffer = kwargs.get(Qstr::MP_QSTR_amount_new)?.try_into()?; @@ -590,8 +589,6 @@ extern "C" fn new_confirm_modify_output(n_args: usize, args: *const Obj, kwargs: }; let paragraphs = Paragraphs::new([ - Paragraph::new(&theme::TEXT_BOLD, "Address:".into()), - Paragraph::new(&theme::TEXT_MONO, address).break_after(), Paragraph::new(&theme::TEXT_NORMAL, description.into()), Paragraph::new(&theme::TEXT_MONO, amount_change).break_after(), Paragraph::new(&theme::TEXT_BOLD, "New amount:".into()), @@ -1759,12 +1756,11 @@ pub static mp_module_trezorui2: Module = obj_module! { /// def confirm_modify_output( /// *, - /// address: str, /// sign: int, /// amount_change: str, /// amount_new: str, /// ) -> object: - /// """Decrease or increase amount for given address.""" + /// """Decrease or increase output amount.""" Qstr::MP_QSTR_confirm_modify_output => obj_fn_kw!(0, new_confirm_modify_output).as_obj(), /// def confirm_output_address( diff --git a/core/embed/rust/src/ui/model_tt/layout.rs b/core/embed/rust/src/ui/model_tt/layout.rs index b3ecdcf52d..96d244d59e 100644 --- a/core/embed/rust/src/ui/model_tt/layout.rs +++ b/core/embed/rust/src/ui/model_tt/layout.rs @@ -1766,12 +1766,11 @@ pub static mp_module_trezorui2: Module = obj_module! { /// def confirm_modify_output( /// *, - /// address: str, # ignored /// sign: int, /// amount_change: str, /// amount_new: str, /// ) -> object: - /// """Decrease or increase amount for given address.""" + /// """Decrease or increase output amount.""" Qstr::MP_QSTR_confirm_modify_output => obj_fn_kw!(0, new_confirm_modify_output).as_obj(), /// def confirm_modify_fee( diff --git a/core/mocks/generated/trezorui2.pyi b/core/mocks/generated/trezorui2.pyi index 0569020e3c..474b487443 100644 --- a/core/mocks/generated/trezorui2.pyi +++ b/core/mocks/generated/trezorui2.pyi @@ -128,12 +128,11 @@ def confirm_joint_total( # rust/src/ui/model_tr/layout.rs def confirm_modify_output( *, - address: str, sign: int, amount_change: str, amount_new: str, ) -> object: - """Decrease or increase amount for given address.""" + """Decrease or increase output amount.""" # rust/src/ui/model_tr/layout.rs @@ -597,12 +596,11 @@ def confirm_total( # rust/src/ui/model_tt/layout.rs def confirm_modify_output( *, - address: str, # ignored sign: int, amount_change: str, amount_new: str, ) -> object: - """Decrease or increase amount for given address.""" + """Decrease or increase output amount.""" # rust/src/ui/model_tt/layout.rs diff --git a/core/src/trezor/ui/layouts/tr/__init__.py b/core/src/trezor/ui/layouts/tr/__init__.py index 5c00508148..e0bfcf6ab6 100644 --- a/core/src/trezor/ui/layouts/tr/__init__.py +++ b/core/src/trezor/ui/layouts/tr/__init__.py @@ -1080,20 +1080,45 @@ async def confirm_modify_output( amount_change: str, amount_new: str, ) -> None: - await raise_if_not_confirmed( - interact( - RustLayout( - trezorui2.confirm_modify_output( - address=address, - sign=sign, - amount_change=amount_change, - amount_new=amount_new, - ) - ), - "modify_output", - ButtonRequestType.ConfirmOutput, + address_layout = RustLayout( + trezorui2.confirm_blob( + title="MODIFY AMOUNT", + data=address, + verb="CONTINUE", + verb_cancel=None, + description="Address:", + extra=None, ) ) + modify_layout = RustLayout( + trezorui2.confirm_modify_output( + sign=sign, + amount_change=amount_change, + amount_new=amount_new, + ) + ) + + send_button_request = True + while True: + if send_button_request: + await button_request( + "modify_output", + ButtonRequestType.ConfirmOutput, + address_layout.page_count(), + ) + await raise_if_not_confirmed(ctx_wait(address_layout)) + + if send_button_request: + send_button_request = False + await button_request( + "modify_output", + ButtonRequestType.ConfirmOutput, + modify_layout.page_count(), + ) + result = await ctx_wait(modify_layout) + + if result is CONFIRMED: + break async def confirm_modify_fee( diff --git a/core/src/trezor/ui/layouts/tt/__init__.py b/core/src/trezor/ui/layouts/tt/__init__.py index a53f23c984..0e63760df4 100644 --- a/core/src/trezor/ui/layouts/tt/__init__.py +++ b/core/src/trezor/ui/layouts/tt/__init__.py @@ -1033,44 +1033,42 @@ async def confirm_modify_output( amount_change: str, amount_new: str, ) -> None: + address_layout = RustLayout( + trezorui2.confirm_blob( + title="MODIFY AMOUNT", + data=address, + verb="CONTINUE", + verb_cancel=None, + description="Address:", + extra=None, + ) + ) + modify_layout = RustLayout( + trezorui2.confirm_modify_output( + sign=sign, + amount_change=amount_change, + amount_new=amount_new, + ) + ) + send_button_request = True while True: if send_button_request: await button_request( "modify_output", ButtonRequestType.ConfirmOutput, + address_layout.page_count(), ) - await raise_if_not_confirmed( - ctx_wait( - RustLayout( - trezorui2.confirm_blob( - title="MODIFY AMOUNT", - data=address, - verb="CONTINUE", - verb_cancel=None, - description="Address:", - extra=None, - ) - ) - ) - ) + await raise_if_not_confirmed(ctx_wait(address_layout)) if send_button_request: send_button_request = False await button_request( "modify_output", ButtonRequestType.ConfirmOutput, + modify_layout.page_count(), ) - result = await ctx_wait( - RustLayout( - trezorui2.confirm_modify_output( - address=address, - sign=sign, - amount_change=amount_change, - amount_new=amount_new, - ) - ), - ) + result = await ctx_wait(modify_layout) if result is CONFIRMED: break diff --git a/tests/device_tests/bitcoin/test_signtx_replacement.py b/tests/device_tests/bitcoin/test_signtx_replacement.py index eb912535f1..08ce57630f 100644 --- a/tests/device_tests/bitcoin/test_signtx_replacement.py +++ b/tests/device_tests/bitcoin/test_signtx_replacement.py @@ -600,7 +600,6 @@ def test_p2wpkh_in_p2sh_fee_bump_from_external(client: Client): orig_index=0, ) - is_tr = client.features.model == "Safe 3" with client: client.set_expected_responses( [ @@ -613,7 +612,7 @@ def test_p2wpkh_in_p2sh_fee_bump_from_external(client: Client): request_output(0), request_orig_output(0, TXHASH_334cd7), messages.ButtonRequest(code=B.ConfirmOutput), - (not is_tr, messages.ButtonRequest(code=B.ConfirmOutput)), + messages.ButtonRequest(code=B.ConfirmOutput), request_orig_output(1, TXHASH_334cd7), messages.ButtonRequest(code=B.SignTx), request_input(0), diff --git a/tests/input_flows.py b/tests/input_flows.py index c416e460fa..33f280a66c 100644 --- a/tests/input_flows.py +++ b/tests/input_flows.py @@ -683,6 +683,7 @@ class InputFlowSignTxInformationReplacement(InputFlowBase): self.debug.press_right() self.debug.press_right() self.debug.press_right() + yield def lock_time_input_flow_tt(