fixup! wip: handler filters

common/protob/messages-management.proto

@@ -117,15 +117,18 @@ message Features {
     optional RecoveryType recovery_type = 52;   // what type of recovery we are in. NB: this works in conjunction with recovery_status
 
     enum BackupAvailability {
-        NotAvailable = 0;   // backup of the device has been done
-        Required = 1;       // device has never been backed up
-        Available = 2;      // "repeated backup" is currently unlocked
+        /// Device is already backed up, or a previous backup has failed.
+        NotAvailable = 0;
+        /// Device is not backed up. Backup is required.
+        Required = 1;
+        /// Device is already backed up and can be backed up again.
+        Available = 2;
     }
 
     enum RecoveryStatus {
         Nothing = 0;    // we are not in recovery mode
         Recovery = 1;   // we are in "Normal" or "DryRun" recovery
-        Backup = 2;     // we are in "UnlockRepeatedBackup" recovery kind
+        Backup = 2;     // we are in repeated backup mode
     }
 
     enum Capability {

core/embed/rust/src/ui/model_tr/layout.rs

@@ -1965,7 +1965,7 @@ pub static mp_module_trezorui2: Module = obj_module! {
     ///     title: str,  # unused on TR
     ///     description: str,
     ///     button: str,
-    ///     recovery_type: int,  # RecoveryType enum, passed as an int
+    ///     recovery_type: RecoveryType,
     ///     info_button: bool,  # unused on TR
     ///     show_info: bool,
     /// ) -> LayoutObj[UiResult]:
@@ -1974,7 +1974,7 @@ pub static mp_module_trezorui2: Module = obj_module! {
 
     /// def select_word_count(
     ///     *,
-    ///     recovery_type: int,  # unused on TR
+    ///     recovery_type: RecoveryType,  # unused on TR
     /// ) -> LayoutObj[int | str]:
     ///    """Select mnemonic word count from (12, 18, 20, 24, 33)."""
     Qstr::MP_QSTR_select_word_count => obj_fn_kw!(0, new_select_word_count).as_obj(),

core/embed/rust/src/ui/model_tt/layout.rs

@@ -2045,15 +2045,15 @@ pub static mp_module_trezorui2: Module = obj_module! {
     ///     title: str,
     ///     description: str,
     ///     button: str,
-    ///     recovery_type: int,  # RecoveryType enum, passed as an int
+    ///     recovery_type: RecoveryType,
     ///     info_button: bool = False,
     /// ) -> LayoutObj[UiResult]:
     ///     """Device recovery homescreen."""
     Qstr::MP_QSTR_confirm_recovery => obj_fn_kw!(0, new_confirm_recovery).as_obj(),
 
     /// def select_word_count(
     ///     *,
-    ///     recovery_type: int,  # RecoveryType enum, passed as an int
+    ///     recovery_type: RecoveryType,
     /// ) -> LayoutObj[int | str]:  # TT returns int
     ///     """Select mnemonic word count from (12, 18, 20, 24, 33)."""
     Qstr::MP_QSTR_select_word_count => obj_fn_kw!(0, new_select_word_count).as_obj(),

core/mocks/generated/trezorui2.pyi

@@ -373,7 +373,7 @@ def confirm_recovery(
     title: str,  # unused on TR
     description: str,
     button: str,
-    recovery_type: int,  # RecoveryType enum, passed as an int
+    recovery_type: RecoveryType,
     info_button: bool,  # unused on TR
     show_info: bool,
 ) -> LayoutObj[UiResult]:
@@ -383,7 +383,7 @@ def confirm_recovery(
 # rust/src/ui/model_tr/layout.rs
 def select_word_count(
     *,
-    recovery_type: int,  # unused on TR
+    recovery_type: RecoveryType,  # unused on TR
 ) -> LayoutObj[int | str]:
    """Select mnemonic word count from (12, 18, 20, 24, 33)."""
 
@@ -886,7 +886,7 @@ def confirm_recovery(
     title: str,
     description: str,
     button: str,
-    recovery_type: int,  # RecoveryType enum, passed as an int
+    recovery_type: RecoveryType,
     info_button: bool = False,
 ) -> LayoutObj[UiResult]:
     """Device recovery homescreen."""
@@ -895,7 +895,7 @@ def confirm_recovery(
 # rust/src/ui/model_tt/layout.rs
 def select_word_count(
     *,
-    recovery_type: int,  # RecoveryType enum, passed as an int
+    recovery_type: RecoveryType,
 ) -> LayoutObj[int | str]:  # TT returns int
     """Select mnemonic word count from (12, 18, 20, 24, 33)."""
 

core/src/apps/base.py

@@ -70,7 +70,7 @@ def get_features() -> Features:
     from trezor.messages import Features
     from trezor.ui import HEIGHT, WIDTH
 
-    from apps.common import mnemonic, safety_checks
+    from apps.common import backup, mnemonic, safety_checks
 
     v_major, v_minor, v_patch, _v_build = utils.VERSION
 
@@ -157,9 +157,7 @@ def get_features() -> Features:
         f.passphrase_protection = storage_device.is_passphrase_enabled()
         if storage_device.needs_backup():
             f.backup_availability = BackupAvailability.Required
-        elif storage_cache.get_bool(
-            storage_cache.APP_RECOVERY_REPEATED_BACKUP_UNLOCKED
-        ):
+        elif backup.repeated_backup_enabled():
             f.backup_availability = BackupAvailability.Available
         else:
             f.backup_availability = BackupAvailability.NotAvailable
@@ -423,7 +421,7 @@ async def unlock_device() -> None:
 
     _SCREENSAVER_IS_ON = False
     set_homescreen()
-    wire.filters.remove(_pinlock_filter)
+    wire.remove_filter(_pinlock_filter)
 
 
 def _pinlock_filter(msg_type: int, prev_handler: Handler[Msg]) -> Handler[Msg]:
@@ -437,30 +435,6 @@ async def wrapper(msg: Msg) -> protobuf.MessageType:
     return wrapper
 
 
-_ALLOW_WHILE_REPEATED_BACKUP_UNLOCKED = (
-    MessageType.Initialize,
-    MessageType.GetFeatures,
-    MessageType.EndSession,
-    MessageType.BackupDevice,
-    MessageType.WipeDevice,
-    MessageType.Cancel,
-)
-
-
-def _repeated_backup_filter(msg_type: int, prev_handler: Handler[Msg]) -> Handler[Msg]:
-    if msg_type in _ALLOW_WHILE_REPEATED_BACKUP_UNLOCKED:
-        return prev_handler
-    else:
-        raise wire.ProcessError("Operation not allowed when in repeated backup state")
-
-
-def remove_repeated_backup_filter():
-    try:
-        wire.filters.remove(_repeated_backup_filter)
-    except ValueError:
-        pass
-
-
 # this function is also called when handling ApplySettings
 def reload_settings_from_storage() -> None:
     from trezor import ui
@@ -475,6 +449,8 @@ def reload_settings_from_storage() -> None:
 
 
 def boot() -> None:
+    from apps.common import backup
+
     MT = MessageType  # local_cache_global
 
     # Register workflow handlers
@@ -494,8 +470,8 @@ def boot() -> None:
 
     reload_settings_from_storage()
 
-    if storage_cache.get_bool(storage_cache.APP_RECOVERY_REPEATED_BACKUP_UNLOCKED):
-        wire.filters.append(_repeated_backup_filter)
+    if backup.repeated_backup_enabled():
+        backup.add_repeated_backup_filter()
     if not config.is_unlocked():
         # pinlocked handler should always be the last one
         wire.filters.append(_pinlock_filter)

core/src/apps/common/backup.py

@@ -1,7 +1,51 @@
-def disable_repeated_backup():
+from typing import TYPE_CHECKING
+
+from trezor.enums import MessageType
+
+if TYPE_CHECKING:
+    from trezor.wire import Handler, Msg
+
+
+def repeated_backup_enabled() -> bool:
     import storage.cache as storage_cache
 
-    from apps import base
+    storage_cache.get_bool(storage_cache.APP_RECOVERY_REPEATED_BACKUP_UNLOCKED)
+
+
+def enable_repeated_backup():
+    import storage.cache as storage_cache
+
+    storage_cache.set_bool(storage_cache.APP_RECOVERY_REPEATED_BACKUP_UNLOCKED, True)
+
+
+def add_repeated_backup_filter():
+    from trezor import wire
+
+    wire.filters.append(_repeated_backup_filter)
+
+
+def disable_repeated_backup():
+    import storage.cache as storage_cache
+    from trezor import wire
 
     storage_cache.delete(storage_cache.APP_RECOVERY_REPEATED_BACKUP_UNLOCKED)
-    base.remove_repeated_backup_filter()
+    wire.remove_filter(_repeated_backup_filter)
+
+
+_ALLOW_WHILE_REPEATED_BACKUP_UNLOCKED = (
+    MessageType.Initialize,
+    MessageType.GetFeatures,
+    MessageType.EndSession,
+    MessageType.BackupDevice,
+    MessageType.WipeDevice,
+    MessageType.Cancel,
+)
+
+
+def _repeated_backup_filter(msg_type: int, prev_handler: Handler[Msg]) -> Handler[Msg]:
+    from trezor import wire
+
+    if msg_type in _ALLOW_WHILE_REPEATED_BACKUP_UNLOCKED:
+        return prev_handler
+    else:
+        raise wire.ProcessError("Operation not allowed when in repeated backup state")

core/src/apps/management/backup_device.py

@@ -21,13 +21,11 @@ async def backup_device(msg: BackupDevice) -> Success:
 
     # do this early before we show any UI
     # the homescreen will clear the flag right after its own UI is gone
-    repeated_backup_unlocked = storage_cache.get_bool(
-        storage_cache.APP_RECOVERY_REPEATED_BACKUP_UNLOCKED
-    )
+    repeated_backup_enabled = backup.repeated_backup_enabled()
 
     if not storage_device.is_initialized():
         raise wire.NotInitialized("Device is not initialized")
-    if not storage_device.needs_backup() and not repeated_backup_unlocked:
+    if not storage_device.needs_backup() and not repeated_backup_enabled:
         raise wire.ProcessError("Seed already backed up")
 
     mnemonic_secret, backup_type = mnemonic.get()
@@ -47,7 +45,7 @@ async def backup_device(msg: BackupDevice) -> Success:
     elif len(groups) > 0:
         raise wire.DataError("group_threshold is missing")
 
-    if not repeated_backup_unlocked:
+    if not repeated_backup_enabled:
         storage_device.set_unfinished_backup(True)
 
     backup.disable_repeated_backup()

core/src/apps/management/recovery_device/homescreen.py

@@ -18,9 +18,10 @@
 async def recovery_homescreen() -> None:
     from trezor import workflow
 
+    from apps.common import backup
     from apps.homescreen import homescreen
 
-    if storage_cache.get_bool(storage_cache.APP_RECOVERY_REPEATED_BACKUP_UNLOCKED):
+    if backup.repeated_backup_enabled():
         await _continue_repeated_backup()
     elif not storage_recovery.is_in_progress():
         workflow.set_default(homescreen)
@@ -44,12 +45,9 @@ async def recovery_process() -> Success:
     try:
         return await _continue_recovery_process()
     except recover.RecoveryAborted:
-        if recovery_type == RecoveryType.DryRun:
-            storage_recovery.end_progress()
-        elif recovery_type == RecoveryType.UnlockRepeatedBackup:
-            backup.disable_repeated_backup()
-            storage_recovery.end_progress()
-        else:
+        storage_recovery.end_progress()
+        backup.disable_repeated_backup()
+        if recovery_type == RecoveryType.NormalRecovery:
             storage.wipe()
         raise wire.ActionCancelled
 
@@ -84,8 +82,6 @@ async def _continue_repeated_backup() -> None:
             raise RuntimeError
 
         await backup_seed(backup_type, mnemonic_secret)
-    except ActionCancelled:
-        workflow.set_default(homescreen)
     finally:
         backup.disable_repeated_backup()
         storage_recovery.end_progress()
@@ -205,7 +201,7 @@ async def _finish_recovery_dry_run(secret: bytes, backup_type: BackupType) -> Su
 async def _finish_recovery_unlock_repeated_backup(
     secret: bytes, backup_type: BackupType
 ) -> Success:
-    import storage.cache as storage_cache
+    from apps.common import backup
 
     if backup_type is None:
         raise RuntimeError
@@ -215,9 +211,7 @@ async def _finish_recovery_unlock_repeated_backup(
     result = _check_secret_against_stored_secret(secret, is_slip39, backup_type)
 
     if result:
-        storage_cache.set_bool(
-            storage_cache.APP_RECOVERY_REPEATED_BACKUP_UNLOCKED, True
-        )
+        backup.enable_repeated_backup()
         return Success(message="Backup unlocked")
     else:
         raise wire.ProcessError("The seed does not match the one in the device")

core/src/trezor/wire/__init__.py

@@ -298,8 +298,21 @@ def find_handler(iface: WireInterface, msg_type: int) -> Handler:
 Please note that this behavior is really unsuited to anything other than what we are
 using it for now. It might be necessary to modify the semantics if we need more complex
 usecases.
+
+NB: `filters` is currently public so callers can have control over where they insert
+new filters, but removal should be done using `remove_filter`!
+We should, however, change it such that filters must be added using an `add_filter`
+and `filters` becomes private!
 """
 
+
+def remove_filter(filter):
+    try:
+        filters.remove(filter)
+    except ValueError:
+        pass
+
+
 AVOID_RESTARTING_FOR: Container[int] = ()
 
 

legacy/firmware/fsm_msg_common.h

@@ -494,8 +494,8 @@ void fsm_msgApplyFlags(const ApplyFlags *msg) {
 void fsm_msgRecoveryDevice(const RecoveryDevice *msg) {
   CHECK_PIN_UNCACHED
 
-  CHECK_PARAM(!msg->has_type || msg->type == RecoveryType_NormalRecovery ||
-                  msg->type == RecoveryType_DryRun,
+  CHECK_PARAM(msg->type == RecoveryType_NormalRecovery ||
+              msg->type == RecoveryType_DryRun,
               _("UnlockRepeatedBackup not supported"))
 
   const bool dry_run = msg->has_type ? msg->type == RecoveryType_DryRun : false;

python/src/trezorlib/cli/device.py

@@ -150,7 +150,8 @@ def load(
 @click.option(
     "-i",
     "--input_method",
-    "input_method",
+    "-t",
+    "--type",
     type=ChoiceType(RECOVERY_DEVICE_INPUT_METHOD),
     default="scrambled",
 )