Skip to content

ui.Layout ButtonRequest handler is not exception safeΒ #6689

Description

@romanz

ui.Layout.get_result blocks until the last ButtonRequest is acknowledged, to avoid THP desync:

result = await self.result_box
assert CURRENT_LAYOUT is None # the screen is blank now
if is_done is not None:
# Make sure ButtonRequest is ACKed, before the result is returned.
# Otherwise, THP channel may become desynced (due to two consecutive writes).
self.put_button_request(None)
task = loop.spawn(_waiting_screen())
try:
await is_done
finally:
task.close()
return result

However, the existing code doesn't wait in case await self.result_box raises.

Note: if GeneratorExit is raised, the handling code must not yield/await (#6681).

Also, see related comment regarding ButtonRequest decoupling:

since it only happens in debug builds, probably to make sure ButtonAck are correctly sent by the host

this is the answer. in production build we don't want to crash the flow because of it, but there may be some weird consequences.

in an ideal world we'd decouple ButtonRequests from the UI flow by, i dunno, building in a separate ButtonRequest queue in the codec handler, and only sending events to the queue, and the codec itself would resolve whether there are pending BRs, and not e.g. send a next one until the host responds to the previous .... something something. haven't really thought deeply about it.
anyway, the semantic intention is that UI code should signal that a ButtonRequest worthy change has happened, but not require it.
in fact we planned to start ignoring BRs if the reply doesn't come within 500 ms, which on USB would indicate that the host has crashed

Originally posted by @matejcik in #3686 (comment)

Activity

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

Metadata

Metadata

Assignees

Labels

coreTrezor Core firmware. Runs on Trezor Model T and Safe models.

Type

No type

Projects

  • Status
    No status

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions