Skip to content

Commit a000c84

Browse files
committed
fix(core): avoid failing ButtonRequest handling at ContinueOnErrors
No more button requests / errors will be sent later during the backup workflow after an I/O-related error/timeout. Host will be ignored until the backup workflow is over - sending the final "Success" response may fail. Other workflows can be aborted after user cancellation via `_waiting_screen()`.
1 parent 45c29c9 commit a000c84

3 files changed

Lines changed: 96 additions & 35 deletions

File tree

core/.changelog.d/6348.fixed

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Avoid failing backup flow on I/O errors.

core/src/trezor/ui/__init__.py

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,6 @@
5252
See `trezor::ui::layout::base::EventCtx::ANIM_FRAME_TIMER`.
5353
"""
5454

55-
_UNRESPONSIVE_WARNING_TIMEOUT_MS = const(2000)
56-
57-
5855
# allow only one alert at a time to avoid alerts overlapping
5956
_alert_in_progress = False
6057

@@ -84,14 +81,6 @@ def alert(count: int = 3) -> None:
8481
loop.schedule(_alert(count))
8582

8683

87-
async def _waiting_screen() -> None:
88-
from trezor import TR
89-
from trezor.ui.layouts import show_wait_text
90-
91-
await loop.sleep(_UNRESPONSIVE_WARNING_TIMEOUT_MS)
92-
show_wait_text(TR.words__comm_trouble)
93-
94-
9584
class Shutdown(Exception):
9685
pass
9786

@@ -282,7 +271,7 @@ async def get_result(self) -> T:
282271
if br_handler is not None:
283272
# Make sure ButtonRequest is ACKed, before the result is returned.
284273
# Otherwise, THP channel may become desynced (due to two consecutive writes).
285-
await br_handler.join(_waiting_screen())
274+
await br_handler.join()
286275

287276
return result
288277
finally:

core/src/trezor/wire/protocol_common.py

Lines changed: 94 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from micropython import const
12
from typing import TYPE_CHECKING
23

34
from trezor import loop, protobuf
@@ -131,12 +132,44 @@ def cache(self) -> DataCache:
131132
...
132133

133134

135+
# Show "trouble communicating" warning after 2s of "blank" screen (if ButtonRequest is not ACKed)
136+
_UNRESPONSIVE_WARNING_TIMEOUT_MS = const(2000)
137+
138+
139+
async def _waiting_screen(raise_on_cancel: type[Exception] | None) -> None:
140+
import trezorui_api
141+
from trezor import TR
142+
from trezor.ui import Layout
143+
144+
verb = TR.buttons__abort if raise_on_cancel is not None else TR.buttons__continue
145+
146+
await loop.sleep(_UNRESPONSIVE_WARNING_TIMEOUT_MS)
147+
with trezorui_api.show_warning(
148+
title="",
149+
description=TR.words__comm_trouble,
150+
button=verb,
151+
danger=True,
152+
allow_cancel=False,
153+
) as obj:
154+
# Block until the user confirmation.
155+
# Don't use `interact` to avoid cancelling current workflow.
156+
await Layout(obj).get_result()
157+
158+
if raise_on_cancel:
159+
raise raise_on_cancel()
160+
161+
134162
class ButtonRequestHandler:
135163
"""Handle button requests and unexpected messages from host."""
136164

137165
def __init__(self, ctx: Context) -> None:
166+
from trezor.wire.errors import ActionCancelled
167+
138168
self.ctx = ctx # used for communication with the host.
139169

170+
# will be raised from `self.join()` on user cancellation.
171+
self.raise_on_cancel: type[ActionCancelled] | None = ActionCancelled
172+
140173
# Receives ButtonRequest notifications from the active layout,
141174
# or `None` when the layout is closed.
142175
self.box: loop.mailbox[ButtonRequest | None] = loop.mailbox()
@@ -172,19 +205,15 @@ def br_task(self, ack_callback: AckCallback) -> Generator[Any, Any, None]:
172205
# no pending I/O - mark as done, to unblock `join()`.
173206
self.is_done.put(None)
174207

175-
async def join(self, wait_task: loop.Task[None]) -> None:
208+
async def join(self) -> None:
176209
# `br_task()` must be scheduled before joining.
177210

178211
# notify the handler that no more button requests are expected
179212
# in production, we don't want this to fail, hence replace=True
180213
self.box.put(None, replace=True)
181214

182-
task = loop.spawn(wait_task)
183-
try:
184-
await self.is_done
185-
finally:
186-
assert self.is_done.is_empty()
187-
task.close()
215+
# Wait for the ButtonRequest handler to finish (or user cancellation)
216+
await loop.race(self.is_done, _waiting_screen(self.raise_on_cancel))
188217

189218
async def _handle(self, ack_callback: AckCallback) -> None:
190219
from trezor.messages import ButtonAck
@@ -214,29 +243,71 @@ class ContinueOnErrors(ButtonRequestHandler):
214243

215244
def __init__(self, ctx: Context, msg: str) -> None:
216245
super().__init__(ctx)
246+
self.raise_on_cancel = None # continue on user cancellation
217247
self._prev_handler: ButtonRequestHandler | None = None
218248
self.msg = msg
249+
self.ignore = False
250+
251+
def put(self, br: ButtonRequest) -> None:
252+
if self.ignore:
253+
# Stop handling ButtonRequests in case of unexpected error.
254+
if __debug__:
255+
log.debug(__name__, "ButtonRequest: skipped %s (%s)", br.code, br.name)
256+
return
257+
258+
super().put(br)
259+
260+
async def join(self) -> None:
261+
if self.ignore:
262+
# Stop handling ButtonRequests in case of unexpected error.
263+
return
264+
265+
await super().join()
266+
267+
def br_task(self, ack_callback: AckCallback) -> Generator[Any, Any, None]:
268+
if self.ignore:
269+
# Stop handling ButtonRequests in case of unexpected error.
270+
return None
271+
272+
return (yield from super().br_task(ack_callback))
219273

220274
async def _handle(self, ack_callback: AckCallback) -> None:
221275
"""Unexpected messages will not cause the handler to fail."""
276+
222277
from .context import UnexpectedMessageException
223278

224-
while True:
225-
try:
226-
# Exit the loop when the layout is done.
227-
return await super()._handle(ack_callback)
228-
except UnexpectedMessageException as exc:
229-
# in case of THP channel preemption, `msg` is not set.
230-
# TRANSPORT_BUSY error has been already sent by `InterfaceContext.handle_packet()`.
231-
if exc.msg:
232-
from trezor.enums import FailureType
233-
from trezor.messages import Failure
234-
235-
# notify the host that the device cannot be preempted
236-
await self.ctx.write(
237-
Failure(code=FailureType.InProgress, message=self.msg)
238-
)
239-
# continue receiving messages
279+
# In case of an unexpected error, stop handling ButtonRequests till the end of this workflow.
280+
# The host will be ignored, disabling host-side cancellation of this workflow.
281+
success = False
282+
try:
283+
while True:
284+
try:
285+
# Exit the loop when the layout is done.
286+
await super(ContinueOnErrors, self)._handle(ack_callback)
287+
# All is well, continue handling ButtonRequests.
288+
success = True
289+
return
290+
except UnexpectedMessageException as exc:
291+
# in case of THP channel preemption, `msg` is not set.
292+
# TRANSPORT_BUSY error has been already sent by `InterfaceContext.handle_packet()`.
293+
if exc.msg:
294+
from trezor.enums import FailureType
295+
from trezor.messages import Failure
296+
297+
# notify the host that the device cannot be preempted
298+
await self.ctx.write(
299+
Failure(code=FailureType.InProgress, message=self.msg)
300+
)
301+
# continue receiving messages
302+
except Exception as exc:
303+
if __debug__:
304+
log.error(__name__, "ButtonRequest: ignored %s", exc)
305+
log.exception(__name__, exc)
306+
# Stop handling ButtonRequests in case of unexpected error (without failing the flow)
307+
return
308+
finally:
309+
# Handles GeneratorExit as well (in case of task cancellation).
310+
self.ignore = not success
240311

241312
def __enter__(self) -> None:
242313
assert self._prev_handler is None

0 commit comments

Comments
 (0)