refactor(core/mercury): separate upy args parsing #4182

obrusvit · 2024-09-15T20:39:42Z

This PR separates the Rust flows from micropython layer.

The idea is to do the uPy argument parsing in ui/model_mercury/layout.rs and then pass them as regular arguments to code in ui/model_mercury/flow/**.rs, i.e. they won't be passed as args: &[Obj], kwargs: &Map. The goal is to keep the rust flows independent from micropython sub-crate. This separation is done for two reasons:

it simplifies the code in flows creation
it paves the way for apps done completely in Rust and for UiFeatures

This PR has no impact on UI or features.

Note:

there is a few places with the usage of Obj and IterBuf - last place is get_address.rs, postponing this for now

obrusvit · 2024-09-22T17:27:07Z

Similar thing was done here for bootloader and "common". Need to take a look how to take the same/similar approach for firmware. #3658

obrusvit · 2024-09-30T10:34:29Z

The only conflict with #3686 should be: Layout::new moved to layout.rs and renamed to Layout::new_root

mmilata

Wrote some thoughts inline but overall this should be a harmless refactor that can be merged when CI passes.

core/embed/rust/src/ui/model_mercury/flow/confirm_output.rs

mmilata · 2024-10-03T18:16:35Z

core/embed/rust/src/ui/model_mercury/flow/get_address.rs

+    case_sensitive: bool,
+    account: Option<TString<'static>>,
+    path: Option<TString<'static>>,
+    xpubs: Obj, // TODO: get rid of Obj


Perhaps this can use some kind of closure?

mmilata · 2024-10-03T18:17:19Z

core/embed/rust/src/ui/model_mercury/layout.rs

@@ -1540,16 +1866,16 @@ pub static mp_module_trezorui2: Module = obj_module! {
    ///     br_code: ButtonRequestType,
    ///     br_name: str,
    /// ) -> LayoutObj[tuple[UiResult, int]]:
-    ///     """Numer input with + and - buttons, description, and context menu with cancel and
+    ///     """Numebr input with + and - buttons, description, and context menu with cancel and


Suggested change

/// """Numebr input with + and - buttons, description, and context menu with cancel and

/// """Number input with + and - buttons, description, and context menu with cancel and

Making a typo while fixing a typo.
Fixed on rebase.

core/embed/rust/src/ui/model_mercury/flow/confirm_output.rs

github-actions · 2024-10-26T12:23:47Z

core UI changes	device test	click test	persistence test
T2T1 Model T	test(screens) main(screens)	test(screens) main(screens)	test(screens) main(screens)
T3B1 Safe 3	test(screens) main(screens)	test(screens) main(screens)	test(screens) main(screens)
T3T1 Safe 5	test(screens) main(screens)	test(screens) main(screens)	test(screens) main(screens)
All	main(screens)

github-actions · 2024-10-26T12:23:48Z

legacy UI changes	device test(screens) main(screens)

obrusvit · 2024-10-26T12:25:00Z

Re-targeting to GFL branch #3686 and it can go right after.

core/embed/rust/src/ui/model_mercury/layout.rs

ibz · 2024-11-18T16:04:03Z

core/embed/rust/src/ui/model_mercury/layout.rs

+
+        let mut account_params =
+            ShowInfoParams::new(TR::send__send_from.into()).with_cancel_button();
+        for pair in IterBuf::new().try_iterate(account_items)? {


Can't this be extracted into a ShowInfoParams::add_items?

Not without making ShowInfoParams dependent on micropython crate.

- removes 2nd definition of ConfirmBlobParams, the choice of fields and what supplied in ctor and what in `with_` methods should be thought through again. [no changelog]

obrusvit · 2024-11-19T13:41:20Z

I had to use #[allow(clippy::too_many_arguments)] for a few SwipeFlow constructors.

This could be resolved as a part of #4322

matejcik

utACK

matejcik · 2024-11-19T13:49:13Z

i'm thinking though, when do we bite the bullet on proc macros? :)
jpochyla was strongly cautioning against it, but making a #[upy_function_call] fn confirm_action(title: TString, description: Option<TString>, ...) is the perfect usecase

[no changelog]

Supply the rust layout with dedicated paremeter type instead of plain micropython::Obj. The types used are ConfirmBlobParams and ShowInfoParams. [no changelog]

obrusvit added code Code improvements T3T1 Trezor Safe 5 labels Sep 15, 2024

obrusvit self-assigned this Sep 15, 2024

obrusvit force-pushed the obrusvit/mercury/refactor-rust-layout-upy-parsing branch 4 times, most recently from b616b27 to 58347bc Compare September 30, 2024 10:28

obrusvit marked this pull request as ready for review September 30, 2024 10:30

obrusvit requested a review from prusnak as a code owner September 30, 2024 10:30

obrusvit requested review from mmilata and removed request for prusnak September 30, 2024 10:30

mmilata approved these changes Oct 3, 2024

View reviewed changes

obrusvit force-pushed the obrusvit/mercury/refactor-rust-layout-upy-parsing branch from 58347bc to 57d2b15 Compare October 26, 2024 12:23

obrusvit requested review from matejcik and andrewkozlik as code owners October 26, 2024 12:23

obrusvit changed the base branch from main to matejcik/global-layout-only3 October 26, 2024 12:24

obrusvit removed the request for review from andrewkozlik October 26, 2024 12:25

obrusvit force-pushed the obrusvit/mercury/refactor-rust-layout-upy-parsing branch from 57d2b15 to 5086bd3 Compare October 27, 2024 11:55

obrusvit added the blocked Blocked by external force. Third party inputs required. label Nov 1, 2024

matejcik force-pushed the matejcik/global-layout-only3 branch 4 times, most recently from 9297722 to 6b8585b Compare November 12, 2024 15:21

Base automatically changed from matejcik/global-layout-only3 to main November 12, 2024 15:55

obrusvit force-pushed the obrusvit/mercury/refactor-rust-layout-upy-parsing branch from 5086bd3 to 0d51a8e Compare November 18, 2024 12:37

obrusvit changed the base branch from main to ibz/20241112-cleanup November 18, 2024 12:38

obrusvit force-pushed the obrusvit/mercury/refactor-rust-layout-upy-parsing branch from 0d51a8e to 752c8b3 Compare November 18, 2024 12:40

obrusvit removed the blocked Blocked by external force. Third party inputs required. label Nov 18, 2024

obrusvit commented Nov 18, 2024

View reviewed changes

core/embed/rust/src/ui/model_mercury/layout.rs Outdated Show resolved Hide resolved

obrusvit force-pushed the obrusvit/mercury/refactor-rust-layout-upy-parsing branch from 752c8b3 to 172af6d Compare November 18, 2024 12:55

obrusvit requested a review from ibz November 18, 2024 12:57

ibz approved these changes Nov 18, 2024

View reviewed changes

ibz force-pushed the ibz/20241112-cleanup branch from 313f806 to 69d448f Compare November 19, 2024 10:29

Base automatically changed from ibz/20241112-cleanup to main November 19, 2024 11:36

refactor(core/mercury): remove duplicated struct

a8bf2cd

- removes 2nd definition of ConfirmBlobParams, the choice of fields and what supplied in ctor and what in `with_` methods should be thought through again. [no changelog]

obrusvit force-pushed the obrusvit/mercury/refactor-rust-layout-upy-parsing branch 2 times, most recently from bbb65ed to 320eb1f Compare November 19, 2024 13:38

matejcik approved these changes Nov 19, 2024

View reviewed changes

obrusvit added 2 commits November 19, 2024 15:36

refactor(core/mercury): separate upy args parsing

552b83b

[no changelog]

refactor(core/mercury): use params structs

abc7203

Supply the rust layout with dedicated paremeter type instead of plain micropython::Obj. The types used are ConfirmBlobParams and ShowInfoParams. [no changelog]

obrusvit force-pushed the obrusvit/mercury/refactor-rust-layout-upy-parsing branch from 320eb1f to abc7203 Compare November 19, 2024 14:39

obrusvit merged commit af55445 into main Nov 19, 2024
94 checks passed

obrusvit deleted the obrusvit/mercury/refactor-rust-layout-upy-parsing branch November 19, 2024 15:19

obrusvit mentioned this pull request Dec 2, 2024

Receive address confirmed after xpub check #3947

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(core/mercury): separate upy args parsing #4182

refactor(core/mercury): separate upy args parsing #4182

obrusvit commented Sep 15, 2024 •

edited

Loading

obrusvit commented Sep 22, 2024

obrusvit commented Sep 30, 2024 •

edited

Loading

mmilata left a comment

mmilata Oct 3, 2024

mmilata Oct 3, 2024

obrusvit Oct 27, 2024

github-actions bot commented Oct 26, 2024 •

edited

Loading

github-actions bot commented Oct 26, 2024 •

edited

Loading

obrusvit commented Oct 26, 2024

ibz Nov 18, 2024

obrusvit Nov 19, 2024

obrusvit commented Nov 19, 2024

matejcik left a comment

matejcik commented Nov 19, 2024

	/// """Numebr input with + and - buttons, description, and context menu with cancel and
	/// """Number input with + and - buttons, description, and context menu with cancel and

refactor(core/mercury): separate upy args parsing #4182

refactor(core/mercury): separate upy args parsing #4182

Conversation

obrusvit commented Sep 15, 2024 • edited Loading

obrusvit commented Sep 22, 2024

obrusvit commented Sep 30, 2024 • edited Loading

mmilata left a comment

Choose a reason for hiding this comment

mmilata Oct 3, 2024

Choose a reason for hiding this comment

mmilata Oct 3, 2024

Choose a reason for hiding this comment

obrusvit Oct 27, 2024

Choose a reason for hiding this comment

github-actions bot commented Oct 26, 2024 • edited Loading

github-actions bot commented Oct 26, 2024 • edited Loading

obrusvit commented Oct 26, 2024

ibz Nov 18, 2024

Choose a reason for hiding this comment

obrusvit Nov 19, 2024

Choose a reason for hiding this comment

obrusvit commented Nov 19, 2024

matejcik left a comment

Choose a reason for hiding this comment

matejcik commented Nov 19, 2024

obrusvit commented Sep 15, 2024 •

edited

Loading

obrusvit commented Sep 30, 2024 •

edited

Loading

github-actions bot commented Oct 26, 2024 •

edited

Loading

github-actions bot commented Oct 26, 2024 •

edited

Loading