fix(core): don't raise `FirmwareError("button request ack pending")` on THP debug builds
What the developer wrote
Authored by Roman Zeyde
fix(core): don't raise `FirmwareError("button request ack pending")` on THP debug builds
Otherwise, it may cause THP channel to desync due to double write.
[no changelog]
What changed, and why it matters
This commit changes how a Trezor hardware wallet handles an internal error condition during user interactions. In debug builds using the new THP (Trezor Host Protocol) communication channel, the device no longer crashes with a 'FirmwareError' when a button-acknowledgment state is still pending. Instead, it logs the issue and continues. The old behavior could cause the THP channel to become desynchronized because the error path might write to the channel twice. The fix prevents a possible communication desync but deliberately suppresses a previously fatal internal-consistency check.
Review whether suppressing the 'button request ack pending' check on THP could mask a real UI-state bug. Verify that the THP transport correctly handles the case where a button request was pending at layout teardown without a second write. Consider adding a regression test for the desync scenario and ensure the non-THP path still catches inconsistent states.
Security signals we found
THP channel desynchronization risk from double write
Internal consistency check (button_request_ack_pending) relaxed on THP debug builds
FirmwareError no longer raised for a pending button-request ack under THP
Debug-only code path (__debug__) affected
No changelog entry provided
Evidence from the diff
In core/src/trezor/ui/init.py, the Layout.exit cleanup path previously raised wire.FirmwareError(‘button request ack pending’) whenever self.button_request_ack_pending was true at teardown. On THP debug builds (utils.USE_THP), this is now replaced by a log.error call. The commit message states the reason: raising the FirmwareError could lead to a double write and desynchronize the THP channel. The change narrows the exception to non-THP debug builds only, preserving the safety check where the THP-specific double-write risk does not exist.
Changed components
core/src/trezor/ui/__init__.pyLayout class cleanup / __exit__ methodTHP (Trezor Host Protocol) debug buildsButton request acknowledgment flowInspect captured patch +6 / −1
diff --git a/core/src/trezor/ui/__init__.py b/core/src/trezor/ui/__init__.py
index 4faa4337b..5c4ad9039 100644
--- a/core/src/trezor/ui/__init__.py
+++ b/core/src/trezor/ui/__init__.py
@@ -248,7 +248,12 @@ class Layout(Generic[T]):
set_current_layout(None)
if __debug__:
if self.button_request_ack_pending:
- raise wire.FirmwareError("button request ack pending")
+ msg = "button request ack pending"
+ if utils.USE_THP:
+ # Don't raise to avoid THP desync
+ log.error(__name__, msg)
+ else:
+ raise wire.FirmwareError(msg)
self.notify_debuglink(None)
async def get_result(self) -> T:
Why this scored 35/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.