From 246998910a178f4cac3bab9429559dd200b90dff Mon Sep 17 00:00:00 2001 From: matejcik Date: Mon, 27 Apr 2020 15:50:58 +0200 Subject: [PATCH] core: refactor usage of input_signals this prevents a certain class of UI test failure. It also localizes the use of debuglink signals into the layout classes instead of call sites, which is a design we were already using for confirm_signals --- core/src/apps/common/passphrase.py | 8 +------- core/src/apps/common/request_pin.py | 8 +------- core/src/apps/debug/__init__.py | 2 +- .../recovery_device/keyboard_bip39.py | 9 ++++++++- .../recovery_device/keyboard_slip39.py | 9 ++++++++- .../apps/management/recovery_device/layout.py | 20 ++++++------------- .../apps/management/reset_device/layout.py | 15 ++++++++------ core/src/trezor/ui/passphrase.py | 13 +++++++++++- core/src/trezor/ui/pin.py | 10 +++++++++- core/src/trezor/ui/word_select.py | 11 ++++++++++ core/src/trezor/wire/__init__.py | 1 + 11 files changed, 67 insertions(+), 39 deletions(-) diff --git a/core/src/apps/common/passphrase.py b/core/src/apps/common/passphrase.py index 50eec959d8..a15bbe48ae 100644 --- a/core/src/apps/common/passphrase.py +++ b/core/src/apps/common/passphrase.py @@ -11,9 +11,6 @@ from trezor.ui import ICON_CONFIG, draw_simple from trezor.ui.passphrase import CANCELLED, PassphraseKeyboard from trezor.ui.text import Text -if __debug__: - from apps.debug import input_signal - _MAX_PASSPHRASE_LEN = const(50) @@ -62,10 +59,7 @@ async def _request_on_device(ctx: wire.Context) -> str: await ctx.call(ButtonRequest(code=ButtonRequestType.PassphraseEntry), ButtonAck) keyboard = PassphraseKeyboard("Enter passphrase", _MAX_PASSPHRASE_LEN) - if __debug__: - passphrase = await ctx.wait(keyboard, input_signal()) - else: - passphrase = await ctx.wait(keyboard) + passphrase = await ctx.wait(keyboard) if passphrase is CANCELLED: raise wire.ActionCancelled("Passphrase entry cancelled") diff --git a/core/src/apps/common/request_pin.py b/core/src/apps/common/request_pin.py index 2d093bfe5c..d9ba109c22 100644 --- a/core/src/apps/common/request_pin.py +++ b/core/src/apps/common/request_pin.py @@ -13,9 +13,6 @@ from apps.common.sdcard import SdCardUnavailable, request_sd_salt if False: from typing import Any, NoReturn, Optional, Tuple -if __debug__: - from apps.debug import input_signal - def can_lock_device() -> bool: """Return True if the device has a PIN set or SD-protect enabled.""" @@ -40,10 +37,7 @@ async def request_pin( dialog = PinDialog(prompt, subprompt, allow_cancel) while True: - if __debug__: - pin = await ctx.wait(dialog, input_signal()) - else: - pin = await ctx.wait(dialog) + pin = await ctx.wait(dialog) if pin is CANCELLED: raise wire.PinCancelled assert isinstance(pin, str) diff --git a/core/src/apps/debug/__init__.py b/core/src/apps/debug/__init__.py index e3cee6bf3c..d98b4ee0bc 100644 --- a/core/src/apps/debug/__init__.py +++ b/core/src/apps/debug/__init__.py @@ -69,7 +69,7 @@ if __debug__: elif msg.swipe == DebugSwipeDirection.RIGHT: await swipe_chan.put(swipe.SWIPE_RIGHT) if msg.input is not None: - await input_chan.put(msg.input) + await input_chan.put(ui.Result(msg.input)) loop.schedule(debuglink_decision_dispatcher()) diff --git a/core/src/apps/management/recovery_device/keyboard_bip39.py b/core/src/apps/management/recovery_device/keyboard_bip39.py index 91a08284ec..01a58230f1 100644 --- a/core/src/apps/management/recovery_device/keyboard_bip39.py +++ b/core/src/apps/management/recovery_device/keyboard_bip39.py @@ -4,7 +4,7 @@ from trezor.ui import display from trezor.ui.button import Button, ButtonClear, ButtonMono, ButtonMonoConfirm if False: - from typing import Optional + from typing import Optional, Tuple from trezor.ui.button import ButtonContent, ButtonStyleStateType @@ -207,3 +207,10 @@ class Bip39Keyboard(ui.Layout): self.dispatch(event, x, y) else: self.on_timeout() + + if __debug__: + + def create_tasks(self) -> Tuple[loop.Task, ...]: + from apps.debug import input_signal + + return super().create_tasks() + (input_signal(),) diff --git a/core/src/apps/management/recovery_device/keyboard_slip39.py b/core/src/apps/management/recovery_device/keyboard_slip39.py index 0d5a2d39c0..46dc3810ee 100644 --- a/core/src/apps/management/recovery_device/keyboard_slip39.py +++ b/core/src/apps/management/recovery_device/keyboard_slip39.py @@ -4,7 +4,7 @@ from trezor.ui import display from trezor.ui.button import Button, ButtonClear, ButtonMono, ButtonMonoConfirm if False: - from typing import Optional + from typing import Optional, Tuple from trezor.ui.button import ButtonContent, ButtonStyleStateType @@ -217,3 +217,10 @@ class Slip39Keyboard(ui.Layout): self.dispatch(event, x, y) else: self.on_timeout() + + if __debug__: + + def create_tasks(self) -> Tuple[loop.Task, ...]: + from apps.debug import input_signal + + return super().create_tasks() + (input_signal(),) diff --git a/core/src/apps/management/recovery_device/layout.py b/core/src/apps/management/recovery_device/layout.py index b94c6665a5..4468775ceb 100644 --- a/core/src/apps/management/recovery_device/layout.py +++ b/core/src/apps/management/recovery_device/layout.py @@ -17,9 +17,6 @@ from .keyboard_bip39 import Bip39Keyboard from .keyboard_slip39 import Slip39Keyboard from .recover import RecoveryAborted -if __debug__: - from apps.debug import input_signal - if False: from typing import List, Optional, Callable, Iterable, Tuple, Union from trezor.messages.ResetDevice import EnumTypeBackupType @@ -45,13 +42,11 @@ async def request_word_count(ctx: wire.GenericContext, dry_run: bool) -> int: text = Text("Recovery mode", ui.ICON_RECOVERY) text.normal("Number of words?") - if __debug__: - count = await ctx.wait(WordSelector(text), input_signal()) - count = int(count) # if input_signal was triggered, count is a string - else: - count = await ctx.wait(WordSelector(text)) - - return count # type: ignore + count = await ctx.wait(WordSelector(text)) + # WordSelector can return int, or string if the value came from debuglink + # ctx.wait has a return type Any + # Hence, it is easier to convert the returned value to int explicitly + return int(count) async def request_mnemonic( @@ -67,11 +62,8 @@ async def request_mnemonic( ) # type: Union[Slip39Keyboard, Bip39Keyboard] else: keyboard = Bip39Keyboard("Type word %s of %s:" % (i + 1, word_count)) - if __debug__: - word = await ctx.wait(keyboard, input_signal()) - else: - word = await ctx.wait(keyboard) + word = await ctx.wait(keyboard) words.append(word) try: diff --git a/core/src/apps/management/reset_device/layout.py b/core/src/apps/management/reset_device/layout.py index b6a5606e56..b7a1185d0e 100644 --- a/core/src/apps/management/reset_device/layout.py +++ b/core/src/apps/management/reset_device/layout.py @@ -14,6 +14,10 @@ from trezor.ui.text import Text from apps.common.confirm import confirm, require_confirm, require_hold_to_confirm from apps.common.layout import show_success +if False: + from trezor import loop + from typing import List, Tuple + if __debug__: from apps import debug @@ -184,11 +188,7 @@ async def _confirm_word(ctx, share_index, share_words, offset, count, group_inde # let the user pick a word select = MnemonicWordSelect(choices, share_index, checked_index, count, group_index) - if __debug__: - selected_word = await ctx.wait(select, debug.input_signal()) - else: - selected_word = await ctx.wait(select) - + selected_word = await ctx.wait(select) # confirm it is the correct one return selected_word == checked_word @@ -647,9 +647,12 @@ class MnemonicWordSelect(ui.Layout): if __debug__: - def read_content(self): + def read_content(self) -> List[str]: return self.text.read_content() + [b.text for b in self.buttons] + def create_tasks(self) -> Tuple[loop.Task, ...]: + return super().create_tasks() + (debug.input_signal(),) + async def show_reset_device_warning(ctx, backup_type: BackupType = BackupType.Bip39): text = Text("Create new wallet", ui.ICON_RESET, new_lines=False) diff --git a/core/src/trezor/ui/passphrase.py b/core/src/trezor/ui/passphrase.py index c2713fce1a..33f125eace 100644 --- a/core/src/trezor/ui/passphrase.py +++ b/core/src/trezor/ui/passphrase.py @@ -244,4 +244,15 @@ class PassphraseKeyboard(ui.Layout): raise ui.Result(self.input.text) def create_tasks(self) -> Tuple[loop.Task, ...]: - return self.handle_input(), self.handle_rendering(), self.handle_paging() + tasks = ( + self.handle_input(), + self.handle_rendering(), + self.handle_paging(), + ) # type: Tuple[loop.Task, ...] + + if __debug__: + from apps.debug import input_signal + + return tasks + (input_signal(),) + else: + return tasks diff --git a/core/src/trezor/ui/pin.py b/core/src/trezor/ui/pin.py index d25b1136b9..b9392dcb72 100644 --- a/core/src/trezor/ui/pin.py +++ b/core/src/trezor/ui/pin.py @@ -12,7 +12,8 @@ from trezor.ui.button import ( ) if False: - from typing import Iterable, Optional + from trezor import loop + from typing import Iterable, Optional, Tuple def digit_area(i: int) -> ui.Area: @@ -150,3 +151,10 @@ class PinDialog(ui.Layout): def on_confirm(self) -> None: if self.input.pin: raise ui.Result(self.input.pin) + + if __debug__: + + def create_tasks(self) -> Tuple[loop.Task, ...]: + from apps.debug import input_signal + + return super().create_tasks() + (input_signal(),) diff --git a/core/src/trezor/ui/word_select.py b/core/src/trezor/ui/word_select.py index b9b1c33ccd..6d92d69704 100644 --- a/core/src/trezor/ui/word_select.py +++ b/core/src/trezor/ui/word_select.py @@ -1,6 +1,10 @@ from trezor import ui from trezor.ui.button import Button +if False: + from trezor import loop + from typing import Tuple + # todo improve? @@ -40,3 +44,10 @@ class WordSelector(ui.Layout): def on_w33(self) -> None: raise ui.Result(33) + + if __debug__: + + def create_tasks(self) -> Tuple[loop.Task, ...]: + from apps.debug import input_signal + + return super().create_tasks() + (input_signal(),) diff --git a/core/src/trezor/wire/__init__.py b/core/src/trezor/wire/__init__.py index 51d69328f4..9427381cb2 100644 --- a/core/src/trezor/wire/__init__.py +++ b/core/src/trezor/wire/__init__.py @@ -112,6 +112,7 @@ if False: async def write(self, msg: protobuf.MessageType) -> None: ... + # XXX modify type signature so that the return value must be of the same type? async def wait(self, *tasks: Awaitable) -> Any: ...