From abb1d59843e8d5ad061734835f066ffb6d52ba83 Mon Sep 17 00:00:00 2001 From: Roman Zeyde Date: Wed, 5 Feb 2025 17:42:44 +0200 Subject: [PATCH] fixup! test(core): don't fetch `DebugLinkState` by default --- common/protob/messages-debug.proto | 2 + core/src/apps/debug/__init__.py | 5 + core/src/trezor/messages.py | 2 + python/src/trezorlib/debuglink.py | 3 +- python/src/trezorlib/messages.py | 3 + .../src/protos/generated/messages_debug.rs | 98 +++++++++++++------ tests/click_tests/common.py | 1 - tests/click_tests/reset.py | 2 - tests/click_tests/test_autolock.py | 7 -- tests/click_tests/test_lock.py | 2 - tests/click_tests/test_passphrase_bolt.py | 1 - tests/click_tests/test_passphrase_delizia.py | 1 - tests/click_tests/test_pin.py | 1 - 13 files changed, 82 insertions(+), 46 deletions(-) diff --git a/common/protob/messages-debug.proto b/common/protob/messages-debug.proto index 88d2820259..3727b6243f 100644 --- a/common/protob/messages-debug.proto +++ b/common/protob/messages-debug.proto @@ -110,6 +110,8 @@ message DebugLinkGetState { // trezor-core only - wait until current layout changes // changed in 2.6.4: multiple wait types instead of true/false. optional DebugWaitType wait_layout = 3 [default=IMMEDIATE]; + // Responds immediately with an empty `DebugLinkState` (used for client-side synchronization). + optional bool return_empty_state = 4 [default=false]; } /** diff --git a/core/src/apps/debug/__init__.py b/core/src/apps/debug/__init__.py index 6486a27e1c..21ffc9f1bd 100644 --- a/core/src/apps/debug/__init__.py +++ b/core/src/apps/debug/__init__.py @@ -268,6 +268,11 @@ if __debug__: async def dispatch_DebugLinkGetState( msg: DebugLinkGetState, ) -> DebugLinkState | None: + if msg.return_empty_state: + from trezor.messages import DebugLinkState + + return DebugLinkState() + if msg.wait_layout == DebugWaitType.IMMEDIATE: return _state() diff --git a/core/src/trezor/messages.py b/core/src/trezor/messages.py index e529707f4b..1dbfcbc407 100644 --- a/core/src/trezor/messages.py +++ b/core/src/trezor/messages.py @@ -2898,11 +2898,13 @@ if TYPE_CHECKING: class DebugLinkGetState(protobuf.MessageType): wait_layout: "DebugWaitType" + return_empty_state: "bool" def __init__( self, *, wait_layout: "DebugWaitType | None" = None, + return_empty_state: "bool | None" = None, ) -> None: pass diff --git a/python/src/trezorlib/debuglink.py b/python/src/trezorlib/debuglink.py index 52951c9fe5..f2e0c4449c 100644 --- a/python/src/trezorlib/debuglink.py +++ b/python/src/trezorlib/debuglink.py @@ -637,6 +637,8 @@ class DebugLink: decision.hold_ms += 200 self._write(decision) + # "Flush" the DebugLink queue, to be sure that `decision` has been processed. + self._call(messages.DebugLinkGetState(return_empty_state=True)) press_yes = _make_input_func(button=messages.DebugButton.YES) """Confirm current layout. See `_decision` for more details.""" @@ -804,7 +806,6 @@ class DebugUI: if br.pages is not None: for _ in range(br.pages - 1): self.debuglink.swipe_up() - self.debuglink.state(DebugWaitType.CURRENT_LAYOUT) if self.debuglink.model is models.T3T1: layout = self.debuglink.read_layout() diff --git a/python/src/trezorlib/messages.py b/python/src/trezorlib/messages.py index 6d28305f69..024c3ae696 100644 --- a/python/src/trezorlib/messages.py +++ b/python/src/trezorlib/messages.py @@ -4136,6 +4136,7 @@ class DebugLinkGetState(protobuf.MessageType): 1: protobuf.Field("wait_word_list", "bool", repeated=False, required=False, default=None), 2: protobuf.Field("wait_word_pos", "bool", repeated=False, required=False, default=None), 3: protobuf.Field("wait_layout", "DebugWaitType", repeated=False, required=False, default=DebugWaitType.IMMEDIATE), + 4: protobuf.Field("return_empty_state", "bool", repeated=False, required=False, default=False), } def __init__( @@ -4144,10 +4145,12 @@ class DebugLinkGetState(protobuf.MessageType): wait_word_list: Optional["bool"] = None, wait_word_pos: Optional["bool"] = None, wait_layout: Optional["DebugWaitType"] = DebugWaitType.IMMEDIATE, + return_empty_state: Optional["bool"] = False, ) -> None: self.wait_word_list = wait_word_list self.wait_word_pos = wait_word_pos self.wait_layout = wait_layout + self.return_empty_state = return_empty_state class DebugLinkState(protobuf.MessageType): diff --git a/rust/trezor-client/src/protos/generated/messages_debug.rs b/rust/trezor-client/src/protos/generated/messages_debug.rs index d384b11545..58328f5250 100644 --- a/rust/trezor-client/src/protos/generated/messages_debug.rs +++ b/rust/trezor-client/src/protos/generated/messages_debug.rs @@ -1128,6 +1128,8 @@ pub struct DebugLinkGetState { pub wait_word_pos: ::std::option::Option, // @@protoc_insertion_point(field:hw.trezor.messages.debug.DebugLinkGetState.wait_layout) pub wait_layout: ::std::option::Option<::protobuf::EnumOrUnknown>, + // @@protoc_insertion_point(field:hw.trezor.messages.debug.DebugLinkGetState.return_empty_state) + pub return_empty_state: ::std::option::Option, // special fields // @@protoc_insertion_point(special_field:hw.trezor.messages.debug.DebugLinkGetState.special_fields) pub special_fields: ::protobuf::SpecialFields, @@ -1204,8 +1206,27 @@ impl DebugLinkGetState { self.wait_layout = ::std::option::Option::Some(::protobuf::EnumOrUnknown::new(v)); } + // optional bool return_empty_state = 4; + + pub fn return_empty_state(&self) -> bool { + self.return_empty_state.unwrap_or(false) + } + + pub fn clear_return_empty_state(&mut self) { + self.return_empty_state = ::std::option::Option::None; + } + + pub fn has_return_empty_state(&self) -> bool { + self.return_empty_state.is_some() + } + + // Param is passed by value, moved + pub fn set_return_empty_state(&mut self, v: bool) { + self.return_empty_state = ::std::option::Option::Some(v); + } + fn generated_message_descriptor_data() -> ::protobuf::reflect::GeneratedMessageDescriptorData { - let mut fields = ::std::vec::Vec::with_capacity(3); + let mut fields = ::std::vec::Vec::with_capacity(4); let mut oneofs = ::std::vec::Vec::with_capacity(0); fields.push(::protobuf::reflect::rt::v2::make_option_accessor::<_, _>( "wait_word_list", @@ -1222,6 +1243,11 @@ impl DebugLinkGetState { |m: &DebugLinkGetState| { &m.wait_layout }, |m: &mut DebugLinkGetState| { &mut m.wait_layout }, )); + fields.push(::protobuf::reflect::rt::v2::make_option_accessor::<_, _>( + "return_empty_state", + |m: &DebugLinkGetState| { &m.return_empty_state }, + |m: &mut DebugLinkGetState| { &mut m.return_empty_state }, + )); ::protobuf::reflect::GeneratedMessageDescriptorData::new_2::( "DebugLinkGetState", fields, @@ -1249,6 +1275,9 @@ impl ::protobuf::Message for DebugLinkGetState { 24 => { self.wait_layout = ::std::option::Option::Some(is.read_enum_or_unknown()?); }, + 32 => { + self.return_empty_state = ::std::option::Option::Some(is.read_bool()?); + }, tag => { ::protobuf::rt::read_unknown_or_skip_group(tag, is, self.special_fields.mut_unknown_fields())?; }, @@ -1270,6 +1299,9 @@ impl ::protobuf::Message for DebugLinkGetState { if let Some(v) = self.wait_layout { my_size += ::protobuf::rt::int32_size(3, v.value()); } + if let Some(v) = self.return_empty_state { + my_size += 1 + 1; + } my_size += ::protobuf::rt::unknown_fields_size(self.special_fields.unknown_fields()); self.special_fields.cached_size().set(my_size as u32); my_size @@ -1285,6 +1317,9 @@ impl ::protobuf::Message for DebugLinkGetState { if let Some(v) = self.wait_layout { os.write_enum(3, ::protobuf::EnumOrUnknown::value(&v))?; } + if let Some(v) = self.return_empty_state { + os.write_bool(4, v)?; + } os.write_unknown_fields(self.special_fields.unknown_fields())?; ::std::result::Result::Ok(()) } @@ -1305,6 +1340,7 @@ impl ::protobuf::Message for DebugLinkGetState { self.wait_word_list = ::std::option::Option::None; self.wait_word_pos = ::std::option::Option::None; self.wait_layout = ::std::option::Option::None; + self.return_empty_state = ::std::option::Option::None; self.special_fields.clear(); } @@ -1313,6 +1349,7 @@ impl ::protobuf::Message for DebugLinkGetState { wait_word_list: ::std::option::Option::None, wait_word_pos: ::std::option::Option::None, wait_layout: ::std::option::Option::None, + return_empty_state: ::std::option::Option::None, special_fields: ::protobuf::SpecialFields::new(), }; &instance @@ -3650,39 +3687,40 @@ static file_descriptor_proto_data: &'static [u8] = b"\ \x01\x20\x03(\tR\x06tokens:\x02\x18\x01\"-\n\x15DebugLinkReseedRandom\ \x12\x14\n\x05value\x18\x01\x20\x01(\rR\x05value\"j\n\x15DebugLinkRecord\ Screen\x12)\n\x10target_directory\x18\x01\x20\x01(\tR\x0ftargetDirectory\ - \x12&\n\rrefresh_index\x18\x02\x20\x01(\r:\x010R\x0crefreshIndex\"\x91\ + \x12&\n\rrefresh_index\x18\x02\x20\x01(\r:\x010R\x0crefreshIndex\"\xc6\ \x02\n\x11DebugLinkGetState\x12(\n\x0ewait_word_list\x18\x01\x20\x01(\ \x08R\x0cwaitWordListB\x02\x18\x01\x12&\n\rwait_word_pos\x18\x02\x20\x01\ (\x08R\x0bwaitWordPosB\x02\x18\x01\x12e\n\x0bwait_layout\x18\x03\x20\x01\ (\x0e29.hw.trezor.messages.debug.DebugLinkGetState.DebugWaitType:\tIMMED\ - IATER\nwaitLayout\"C\n\rDebugWaitType\x12\r\n\tIMMEDIATE\x10\0\x12\x0f\n\ - \x0bNEXT_LAYOUT\x10\x01\x12\x12\n\x0eCURRENT_LAYOUT\x10\x02\"\x97\x04\n\ - \x0eDebugLinkState\x12\x16\n\x06layout\x18\x01\x20\x01(\x0cR\x06layout\ - \x12\x10\n\x03pin\x18\x02\x20\x01(\tR\x03pin\x12\x16\n\x06matrix\x18\x03\ - \x20\x01(\tR\x06matrix\x12'\n\x0fmnemonic_secret\x18\x04\x20\x01(\x0cR\ - \x0emnemonicSecret\x129\n\x04node\x18\x05\x20\x01(\x0b2%.hw.trezor.messa\ - ges.common.HDNodeTypeR\x04node\x123\n\x15passphrase_protection\x18\x06\ - \x20\x01(\x08R\x14passphraseProtection\x12\x1d\n\nreset_word\x18\x07\x20\ - \x01(\tR\tresetWord\x12#\n\rreset_entropy\x18\x08\x20\x01(\x0cR\x0creset\ - Entropy\x12,\n\x12recovery_fake_word\x18\t\x20\x01(\tR\x10recoveryFakeWo\ - rd\x12*\n\x11recovery_word_pos\x18\n\x20\x01(\rR\x0frecoveryWordPos\x12$\ - \n\x0ereset_word_pos\x18\x0b\x20\x01(\rR\x0cresetWordPos\x12N\n\rmnemoni\ - c_type\x18\x0c\x20\x01(\x0e2).hw.trezor.messages.management.BackupTypeR\ - \x0cmnemonicType\x12\x16\n\x06tokens\x18\r\x20\x03(\tR\x06tokens\"\x0f\n\ - \rDebugLinkStop\"P\n\x0cDebugLinkLog\x12\x14\n\x05level\x18\x01\x20\x01(\ - \rR\x05level\x12\x16\n\x06bucket\x18\x02\x20\x01(\tR\x06bucket\x12\x12\n\ - \x04text\x18\x03\x20\x01(\tR\x04text\"G\n\x13DebugLinkMemoryRead\x12\x18\ - \n\x07address\x18\x01\x20\x01(\rR\x07address\x12\x16\n\x06length\x18\x02\ - \x20\x01(\rR\x06length\")\n\x0fDebugLinkMemory\x12\x16\n\x06memory\x18\ - \x01\x20\x01(\x0cR\x06memory\"^\n\x14DebugLinkMemoryWrite\x12\x18\n\x07a\ - ddress\x18\x01\x20\x01(\rR\x07address\x12\x16\n\x06memory\x18\x02\x20\ - \x01(\x0cR\x06memory\x12\x14\n\x05flash\x18\x03\x20\x01(\x08R\x05flash\"\ - -\n\x13DebugLinkFlashErase\x12\x16\n\x06sector\x18\x01\x20\x01(\rR\x06se\ - ctor\".\n\x14DebugLinkEraseSdCard\x12\x16\n\x06format\x18\x01\x20\x01(\ - \x08R\x06format\"0\n\x14DebugLinkWatchLayout\x12\x14\n\x05watch\x18\x01\ - \x20\x01(\x08R\x05watch:\x02\x18\x01\"\x1f\n\x19DebugLinkResetDebugEvent\ - s:\x02\x18\x01\"\x1a\n\x18DebugLinkOptigaSetSecMaxB=\n#com.satoshilabs.t\ - rezor.lib.protobufB\x12TrezorMessageDebug\x80\xa6\x1d\x01\ + IATER\nwaitLayout\x123\n\x12return_empty_state\x18\x04\x20\x01(\x08:\x05\ + falseR\x10returnEmptyState\"C\n\rDebugWaitType\x12\r\n\tIMMEDIATE\x10\0\ + \x12\x0f\n\x0bNEXT_LAYOUT\x10\x01\x12\x12\n\x0eCURRENT_LAYOUT\x10\x02\"\ + \x97\x04\n\x0eDebugLinkState\x12\x16\n\x06layout\x18\x01\x20\x01(\x0cR\ + \x06layout\x12\x10\n\x03pin\x18\x02\x20\x01(\tR\x03pin\x12\x16\n\x06matr\ + ix\x18\x03\x20\x01(\tR\x06matrix\x12'\n\x0fmnemonic_secret\x18\x04\x20\ + \x01(\x0cR\x0emnemonicSecret\x129\n\x04node\x18\x05\x20\x01(\x0b2%.hw.tr\ + ezor.messages.common.HDNodeTypeR\x04node\x123\n\x15passphrase_protection\ + \x18\x06\x20\x01(\x08R\x14passphraseProtection\x12\x1d\n\nreset_word\x18\ + \x07\x20\x01(\tR\tresetWord\x12#\n\rreset_entropy\x18\x08\x20\x01(\x0cR\ + \x0cresetEntropy\x12,\n\x12recovery_fake_word\x18\t\x20\x01(\tR\x10recov\ + eryFakeWord\x12*\n\x11recovery_word_pos\x18\n\x20\x01(\rR\x0frecoveryWor\ + dPos\x12$\n\x0ereset_word_pos\x18\x0b\x20\x01(\rR\x0cresetWordPos\x12N\n\ + \rmnemonic_type\x18\x0c\x20\x01(\x0e2).hw.trezor.messages.management.Bac\ + kupTypeR\x0cmnemonicType\x12\x16\n\x06tokens\x18\r\x20\x03(\tR\x06tokens\ + \"\x0f\n\rDebugLinkStop\"P\n\x0cDebugLinkLog\x12\x14\n\x05level\x18\x01\ + \x20\x01(\rR\x05level\x12\x16\n\x06bucket\x18\x02\x20\x01(\tR\x06bucket\ + \x12\x12\n\x04text\x18\x03\x20\x01(\tR\x04text\"G\n\x13DebugLinkMemoryRe\ + ad\x12\x18\n\x07address\x18\x01\x20\x01(\rR\x07address\x12\x16\n\x06leng\ + th\x18\x02\x20\x01(\rR\x06length\")\n\x0fDebugLinkMemory\x12\x16\n\x06me\ + mory\x18\x01\x20\x01(\x0cR\x06memory\"^\n\x14DebugLinkMemoryWrite\x12\ + \x18\n\x07address\x18\x01\x20\x01(\rR\x07address\x12\x16\n\x06memory\x18\ + \x02\x20\x01(\x0cR\x06memory\x12\x14\n\x05flash\x18\x03\x20\x01(\x08R\ + \x05flash\"-\n\x13DebugLinkFlashErase\x12\x16\n\x06sector\x18\x01\x20\ + \x01(\rR\x06sector\".\n\x14DebugLinkEraseSdCard\x12\x16\n\x06format\x18\ + \x01\x20\x01(\x08R\x06format\"0\n\x14DebugLinkWatchLayout\x12\x14\n\x05w\ + atch\x18\x01\x20\x01(\x08R\x05watch:\x02\x18\x01\"\x1f\n\x19DebugLinkRes\ + etDebugEvents:\x02\x18\x01\"\x1a\n\x18DebugLinkOptigaSetSecMaxB=\n#com.s\ + atoshilabs.trezor.lib.protobufB\x12TrezorMessageDebug\x80\xa6\x1d\x01\ "; /// `FileDescriptorProto` object which was a source for this generated file diff --git a/tests/click_tests/common.py b/tests/click_tests/common.py index 2060c9726b..a5e330e2ff 100644 --- a/tests/click_tests/common.py +++ b/tests/click_tests/common.py @@ -118,7 +118,6 @@ def navigate_to_action_and_press( # Press or hold debug.press_middle(hold_ms=hold_ms) - debug.read_layout() # TODO: make sure the press above takes action def _carousel_steps(current_index: int, wanted_index: int, length: int) -> int: diff --git a/tests/click_tests/reset.py b/tests/click_tests/reset.py index 207162f1e9..d0a34a8a04 100644 --- a/tests/click_tests/reset.py +++ b/tests/click_tests/reset.py @@ -44,7 +44,6 @@ def confirm_read(debug: "DebugLink", middle_r: bool = False) -> None: debug.press_middle() else: debug.press_right() - debug.read_layout() # TODO: what is being confirmed here? def cancel_backup( @@ -62,7 +61,6 @@ def cancel_backup( elif debug.layout_type is LayoutType.Caesar: debug.press_left() debug.press_left() - debug.read_layout() # TODO: make sure cancellation took place def set_selection(debug: "DebugLink", button: tuple[int, int], diff: int) -> None: diff --git a/tests/click_tests/test_autolock.py b/tests/click_tests/test_autolock.py index e6b1d6916b..17f2d6a9b9 100644 --- a/tests/click_tests/test_autolock.py +++ b/tests/click_tests/test_autolock.py @@ -194,13 +194,6 @@ def test_autolock_does_not_interrupt_signing(device_handler: "BackgroundDeviceHa elif debug.layout_type is LayoutType.Caesar: debug.press_middle() - # In all cases we set wait=False to avoid waiting for the screen and triggering - # the layout deadlock detection. In reality there is no deadlock but the - # `sleepy_filter` delays the response by 10 secs while the layout deadlock - # timeout is 3. In this test we don't need the result of the input event so - # waiting for it is not necessary. - debug.read_layout(wait=False) - signatures, tx = device_handler.result() assert len(signatures) == 1 assert tx diff --git a/tests/click_tests/test_lock.py b/tests/click_tests/test_lock.py index 85ac51a92e..9a0340910f 100644 --- a/tests/click_tests/test_lock.py +++ b/tests/click_tests/test_lock.py @@ -55,7 +55,6 @@ def test_hold_to_lock(device_handler: "BackgroundDeviceHandler"): debug.press_right(hold_ms=duration) else: debug.click((13, 37), hold_ms=duration) - debug.read_layout() # TODO: is it needed? assert device_handler.features().unlocked is False @@ -86,7 +85,6 @@ def test_hold_to_lock(device_handler: "BackgroundDeviceHandler"): layout = debug.read_layout() assert "PinKeyboard" in layout.all_components() debug.input("1234") - debug.read_layout() # TODO: is it needed? assert device_handler.features().unlocked is True diff --git a/tests/click_tests/test_passphrase_bolt.py b/tests/click_tests/test_passphrase_bolt.py index 5d2fb8a034..8f490c0309 100644 --- a/tests/click_tests/test_passphrase_bolt.py +++ b/tests/click_tests/test_passphrase_bolt.py @@ -126,7 +126,6 @@ def press_char(debug: "DebugLink", char: str) -> None: TT_COORDS_PREV = coords # type: ignore for _ in range(amount): debug.click(coords) - debug.read_layout() # TODO: seems to be needed def input_passphrase(debug: "DebugLink", passphrase: str, check: bool = True) -> None: diff --git a/tests/click_tests/test_passphrase_delizia.py b/tests/click_tests/test_passphrase_delizia.py index e7f75a114f..85bdc37173 100644 --- a/tests/click_tests/test_passphrase_delizia.py +++ b/tests/click_tests/test_passphrase_delizia.py @@ -154,7 +154,6 @@ def press_char(debug: "DebugLink", char: str) -> None: COORDS_PREV = coords # type: ignore for _ in range(amount): debug.click(coords) - debug.read_layout() # TODO: seems to be needed def input_passphrase(debug: "DebugLink", passphrase: str, check: bool = True) -> None: diff --git a/tests/click_tests/test_pin.py b/tests/click_tests/test_pin.py index 13064be4ee..c2a8d56bd3 100644 --- a/tests/click_tests/test_pin.py +++ b/tests/click_tests/test_pin.py @@ -186,7 +186,6 @@ def _delete_pin(debug: "DebugLink", digits_to_delete: int, check: bool = True) - for _ in range(digits_to_delete): if debug.layout_type in (LayoutType.Bolt, LayoutType.Delizia): debug.click(buttons.pin_passphrase_grid(9)) - debug.read_layout() # TODO: make sure the press above takes action elif debug.layout_type is LayoutType.Caesar: navigate_to_action_and_press(debug, DELETE, TR_PIN_ACTIONS)