Skip to content

feat(core): show success screen after sending a response to the host #5115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Jun 20, 2025
Merged

feat(core): show success screen after sending a response to the host #5115

merged 4 commits into from
Jun 20, 2025
+154 −329

Conversation

romanz
Copy link
Contributor

@romanz romanz commented May 28, 2025

It should implement similar workflow handling to 151eb62150 (keeping the current session until all workflows are over).

@romanz romanz self-assigned this May 28, 2025
@trezor-bot trezor-bot bot added this to Firmware May 28, 2025
@github-project-automation github-project-automation bot moved this to 🔎 Needs review in Firmware May 28, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented May 28, 2025 •
edited
Loading

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)
T3W1 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
All main(screens)

@romanz romanz moved this from 🔎 Needs review to 🏃‍♀️ In progress in Firmware May 28, 2025
@romanz romanz added the core Trezor Core firmware. Runs on Trezor Model T and T2B1. label May 28, 2025
@romanz
Copy link
Contributor Author

romanz commented May 28, 2025 •
edited
Loading

GetAddress flow:

Screencast.from.2025-05-28.20-06-44.webm

@romanz romanz force-pushed the romanz/async-success branch 4 times, most recently from e337ceb to bb8c5cb Compare May 29, 2025 06:31
@romanz romanz marked this pull request as ready for review May 30, 2025 04:46
@romanz romanz requested review from matejcik and onvej-sl as code owners May 30, 2025 04:46
@romanz romanz removed the request for review from onvej-sl May 30, 2025 04:46
@romanz romanz changed the title [WIP] feat(core): join active workflows before restart feat(core): join active workflows before restart May 30, 2025
@romanz romanz moved this from 🏃‍♀️ In progress to 🔎 Needs review in Firmware May 30, 2025
@romanz romanz linked an issue May 30, 2025 that may be closed by this pull request
T3T1 UI: flows that run after final message is sent to host #3666
Closed
matejcik
matejcik reviewed May 30, 2025
View reviewed changes
Copy link
Contributor

@matejcik matejcik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally fine with this approach.
we still may end up needing units_of_work in order to make THP behave nicely -- THP runs some amount of background loops for retransmissions and some miscellaneous stuff, those must not be interrupted by the session restarts but at the same time aren't workflows

@romanz romanz marked this pull request as draft May 30, 2025 15:52
@romanz romanz mentioned this pull request May 31, 2025
Closed
@romanz
Copy link
Contributor Author

romanz commented May 31, 2025

Rebasing over #4775 to fix failing tests.

@romanz romanz mentioned this pull request Jun 1, 2025
Merged
@romanz romanz force-pushed the romanz/async-success branch from 56bd8e4 to 3e04b2d Compare June 1, 2025 10:41
@romanz romanz changed the base branch from main to romanz/eckhart-pages June 1, 2025 10:41
@romanz romanz marked this pull request as ready for review June 2, 2025 12:58
@romanz romanz requested a review from prusnak as a code owner June 2, 2025 12:58
@romanz romanz requested review from matejcik and removed request for prusnak June 2, 2025 12:58
@obrusvit obrusvit mentioned this pull request Jun 3, 2025
Closed
@romanz romanz force-pushed the romanz/eckhart-pages branch from 497a2d2 to 32103fe Compare June 3, 2025 14:23
@romanz romanz added T3W1 T3T1 Trezor Safe 5 labels Jun 3, 2025
Base automatically changed from romanz/eckhart-pages to main June 3, 2025 15:10
@romanz
Copy link
Contributor Author

romanz commented Jun 3, 2025

Rebasing to resolve a conflict.

@romanz romanz force-pushed the romanz/async-success branch from 3e04b2d to 3c209ad Compare June 3, 2025 15:16
@romanz
Copy link
Contributor Author

romanz commented Jun 10, 2025

THP runs some amount of background loops for retransmissions and some miscellaneous stuff, those must not be interrupted by the session restarts but at the same time aren't workflows

Opened #5166, which can be useful for background loops' cancellation.

@romanz romanz mentioned this pull request Jun 10, 2025
Closed
@romanz
Copy link
Contributor Author

romanz commented Jun 18, 2025

Rebased to resolve a merge conflict.

@romanz romanz force-pushed the romanz/async-success branch from 3c209ad to 74efa66 Compare June 18, 2025 06:15
@romanz romanz changed the title feat(core): join active workflows before restart feat(core): show success screen after sending a response to the host Jun 18, 2025
@romanz romanz added this to the Release 25.07 milestone Jun 18, 2025
@romanz
Copy link
Contributor Author

romanz commented Jun 18, 2025

Pushed a small test-related fix (to handle a race in https://data.trezor.io/dev/firmware/ui_report/15725349158/T3W1-en-core_device_test-index.html).

There is no need in the last self.debug.click(self.debug.screen_buttons.ok()) since we use "zero timeout" when animations are disabled.

@romanz
Copy link
Contributor Author

romanz commented Jun 20, 2025 •
edited
Loading

Ran TS5 device tests on 176f0a6:

+ pytest tests/device_tests/ -m 'not sd_card' --random-order-seed=1 --random-order-bucket=module
...
=== 1405 passed, 188 skipped, 6 deselected, 12 xfailed in 8898.82s (2:28:18) ===

romanz added 4 commits June 20, 2025 14:07
@romanz
Revert "feat(core): add success screen to transactions' signature flow"
1571da5 
This reverts commit 1bdc48a.
@romanz
Revert "feat(core): send address & public key response before showing…
23cde4e 
… `StatusScreen`"

This reverts commit 946fc89.
@romanz
feat(core): join active workflows before restart
f331b13 
It should support asynchronous "success" confirmation layouts
for address, public key and signature confirmation flows.

[no changelog]
@romanz
test(core): don't wait on StatusScreen if animation is disabled
4633611 
[no changelog]
@romanz romanz force-pushed the romanz/async-success branch from 176f0a6 to 4633611 Compare June 20, 2025 11:10
@romanz
Copy link
Contributor Author

romanz commented Jun 20, 2025 •
edited
Loading

Tested address, xpub and transaction signature confirmation on Suite with BTC Testnet4.

@romanz romanz merged commit 4633611 into main Jun 20, 2025
104 checks passed
@romanz romanz deleted the romanz/async-success branch June 20, 2025 11:39
@github-project-automation github-project-automation bot moved this from 🔎 Needs review to 🤝 Needs QA in Firmware Jun 20, 2025
@Hannsek
Copy link
Contributor

Hannsek commented Jun 20, 2025

@bosomt Please test it all various cases including altcoins, staking, swaps, … with various wallets (not only Trezor Suite)

@Hannsek
Copy link
Contributor

Hannsek commented Jun 20, 2025

Related Suite PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matejcik matejcik matejcik approved these changes

Assignees

@romanz romanz

Labels
core Trezor Core firmware. Runs on Trezor Model T and T2B1. T3T1 Trezor Safe 5 T3W1
Projects
Firmware
Status: 🤝 Needs QA
Milestone
Release 25.07
Development

Successfully merging this pull request may close these issues.

T3T1 UI: flows that run after final message is sent to host
3 participants
@romanz @Hannsek @matejcik