feat(core): improve repeated backup

* allow upgrading from Single to Basic
* do not skip confirmation screen when sending BackupDevice from Suite

core/src/apps/management/backup_device.py

@@ -1,63 +1,108 @@
 from typing import TYPE_CHECKING
 
+import storage.device as storage_device
 from trezor.enums import BackupType
 
 if TYPE_CHECKING:
+    from typing import Sequence
+
     from trezor.messages import BackupDevice, Success
 
 
-BAK_T_BIP39 = BackupType.Bip39  # global_import_cache
-
-
-async def backup_device(msg: BackupDevice) -> Success:
-    import storage.device as storage_device
-    from trezor import wire
-    from trezor.messages import Success
+async def perform_backup(
+    is_repeated_backup: bool,
+    group_threshold: int | None = None,
+    groups: Sequence[tuple[int, int]] = (),
+) -> None:
+    from trezor import TR
+    from trezor.enums import ButtonRequestType
+    from trezor.ui.layouts import confirm_action
+    from trezor.utils import ensure
 
     from apps.common import backup, backup_types, mnemonic
 
     from .reset_device import backup_seed, backup_slip39_custom, layout
 
+    # Ask the user to confirm backup. The user can still escape here.
+    if is_repeated_backup:
+        await confirm_action(
+            "confirm_repeated_backup",
+            TR.recovery__title_unlock_repeated_backup,
+            description=TR.recovery__unlock_repeated_backup,
+            br_code=ButtonRequestType.ProtectCall,
+            verb=TR.recovery__unlock_repeated_backup_verb,
+        )
+
+    mnemonic_secret = mnemonic.get_secret()
+    ensure(mnemonic_secret is not None)  # checked at run-time
+    assert mnemonic_secret is not None  # checked at type-check time
+    backup_type = mnemonic.get_type()
+
+    # upgrade Single to Basic if necessary
+    if is_repeated_backup and backup_type == BackupType.Slip39_Single_Extendable:
+        # TODO upgrade to Advanced if appropriate
+        backup_type = BackupType.Slip39_Basic_Extendable
+        storage_device.set_backup_type(backup_type)
+
+    # set unfinished flag -- if the process gets interrupted, the unfinished flag stays
+    if not is_repeated_backup:
+        storage_device.set_unfinished_backup(True)
+
+    # Deactivate repeated backup, set backed up flag, before showing anything to the
+    # user. If anything bad happens from now on, the backup counts as "already done".
+    backup.deactivate_repeated_backup()
+    storage_device.set_backed_up()
+
+    if group_threshold is not None:
+        # Parameters provided from host side.
+        assert backup_types.is_slip39_backup_type(backup_type)
+        extendable = backup_types.is_extendable_backup_type(backup_type)
+        # Run the backup process directly.
+        await backup_slip39_custom(mnemonic_secret, group_threshold, groups, extendable)
+    else:
+        # No parameters provided, allow the user to configure them on screen.
+        await backup_seed(backup_type, mnemonic_secret)
+
+    # If the backup was successful, clear the unfinished flag and show success.
+
+    # (NOTE that if the user manages to enable repeated backup while unfinished flag is
+    # set, the unfinished flag is cleared here. That is the correct thing to do -- the
+    # user _has_ finished the backup because they were able to unlock the repeated
+    # backup -- and now they finished another one.)
+    storage_device.set_unfinished_backup(False)
+    await layout.show_backup_success()
+
+
+async def backup_device(msg: BackupDevice) -> Success:
+    from trezor import wire
+    from trezor.messages import Success
+
+    from apps.common import backup, mnemonic
+
     # do this early before we show any UI
     # the homescreen will clear the flag right after its own UI is gone
     repeated_backup_enabled = backup.repeated_backup_enabled()
+    is_repeated_backup = repeated_backup_enabled and not storage_device.needs_backup()
 
     if not storage_device.is_initialized():
         raise wire.NotInitialized("Device is not initialized")
     if not storage_device.needs_backup() and not repeated_backup_enabled:
         raise wire.ProcessError("Seed already backed up")
 
-    mnemonic_secret, backup_type = mnemonic.get()
-    if mnemonic_secret is None:
-        raise RuntimeError
-
     group_threshold = msg.group_threshold
     groups = [(g.member_threshold, g.member_count) for g in msg.groups]
 
+    # validate host-side SLIP39 parameters
     if group_threshold is not None:
         if group_threshold < 1:
             raise wire.DataError("group_threshold must be a positive integer")
         if len(groups) < group_threshold:
             raise wire.DataError("Not enough groups provided for group_threshold")
-        if backup_type == BAK_T_BIP39:
+        if mnemonic.is_bip39():
             raise wire.ProcessError("Expected SLIP39 backup")
     elif len(groups) > 0:
         raise wire.DataError("group_threshold is missing")
 
-    if not repeated_backup_enabled:
-        storage_device.set_unfinished_backup(True)
-
-    backup.deactivate_repeated_backup()
-    storage_device.set_backed_up()
-
-    if group_threshold is not None:
-        extendable = backup_types.is_extendable_backup_type(backup_type)
-        await backup_slip39_custom(mnemonic_secret, group_threshold, groups, extendable)
-    else:
-        await backup_seed(backup_type, mnemonic_secret)
-
-    storage_device.set_unfinished_backup(False)
-
-    await layout.show_backup_success()
+    await perform_backup(is_repeated_backup, group_threshold, groups)
 
     return Success(message="Seed successfully backed up")

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

@@ -52,11 +52,10 @@ async def recovery_process() -> Success:
 
 
 async def _continue_repeated_backup() -> None:
-    from trezor.enums import ButtonRequestType, MessageType
-    from trezor.ui.layouts import confirm_action
+    from trezor.enums import MessageType
 
-    from apps.common import backup, mnemonic
-    from apps.management.reset_device import backup_seed
+    from apps.common import backup
+    from apps.management.backup_device import perform_backup
 
     wire.AVOID_RESTARTING_FOR = (
         MessageType.Initialize,
@@ -65,19 +64,7 @@ async def _continue_repeated_backup() -> None:
     )
 
     try:
-        await confirm_action(
-            "confirm_repeated_backup",
-            TR.recovery__title_unlock_repeated_backup,
-            description=TR.recovery__unlock_repeated_backup,
-            br_code=ButtonRequestType.ProtectCall,
-            verb=TR.recovery__unlock_repeated_backup_verb,
-        )
-
-        mnemonic_secret, backup_type = mnemonic.get()
-        if mnemonic_secret is None:
-            raise RuntimeError
-
-        await backup_seed(backup_type, mnemonic_secret)
+        await perform_backup(is_repeated_backup=True)
     finally:
         backup.deactivate_repeated_backup()
 

core/src/storage/device.py

@@ -146,6 +146,10 @@ def get_backup_type() -> BackupType:
     return backup_type
 
 
+def set_backup_type(backup_type: BackupType) -> None:
+    common.set_uint8(_NAMESPACE, _BACKUP_TYPE, backup_type)
+
+
 def is_passphrase_enabled() -> bool:
     return common.get_bool(_NAMESPACE, _USE_PASSPHRASE)
 

tests/common.py

@@ -67,6 +67,9 @@ MNEMONIC_SLIP39_ADVANCED_33 = [
 ]
 MNEMONIC_SLIP39_CUSTOM_1of1 = ["tolerate flexible academic academic average dwarf square home promise aspect temple cluster roster forward hand unfair tenant emperor ceramic element forget perfect knit adapt review usual formal receiver typical pleasure duke yield party"]
 MNEMONIC_SLIP39_CUSTOM_SECRET = "3439316237393562383066633231636364663436366330666263393863386663"
+
+MNEMONIC_SLIP39_SINGLE_EXT_20 = ["academic again academic academic academic academic academic academic academic academic academic academic academic academic academic academic academic pecan provide remember"]
+
 # External entropy mocked as received from trezorlib.
 EXTERNAL_ENTROPY = b"zlutoucky kun upel divoke ody" * 2
 # fmt: on

tests/device_tests/test_repeated_backup.py

@@ -21,7 +21,12 @@ from trezorlib import device, messages
 from trezorlib.debuglink import TrezorClientDebugLink as Client
 from trezorlib.exceptions import Cancelled, TrezorFailure
 
-from ..common import TEST_ADDRESS_N, WITH_MOCK_URANDOM, MNEMONIC_SLIP39_BASIC_20_3of6
+from ..common import (
+    MNEMONIC_SLIP39_SINGLE_EXT_20,
+    TEST_ADDRESS_N,
+    WITH_MOCK_URANDOM,
+    MNEMONIC_SLIP39_BASIC_20_3of6,
+)
 from ..input_flows import InputFlowSlip39BasicBackup, InputFlowSlip39BasicRecoveryDryRun
 
 
@@ -65,7 +70,7 @@ def test_repeated_backup(client: Client):
 
     # we can now perform another backup
     with client:
-        IF = InputFlowSlip39BasicBackup(client, False)
+        IF = InputFlowSlip39BasicBackup(client, False, repeated=True)
         client.set_input_flow(IF.get())
         device.backup(client)
 
@@ -78,6 +83,46 @@ def test_repeated_backup(client: Client):
         device.backup(client)
 
 
+@pytest.mark.setup_client(mnemonic=MNEMONIC_SLIP39_SINGLE_EXT_20)
+@pytest.mark.skip_t1b1
+@WITH_MOCK_URANDOM
+def test_repeated_backup_upgrade_single(client: Client):
+    assert (
+        client.features.backup_availability == messages.BackupAvailability.NotAvailable
+    )
+    assert client.features.recovery_status == messages.RecoveryStatus.Nothing
+    assert client.features.backup_type == messages.BackupType.Slip39_Single_Extendable
+
+    # unlock repeated backup by entering the single share
+    with client:
+        IF = InputFlowSlip39BasicRecoveryDryRun(
+            client, MNEMONIC_SLIP39_SINGLE_EXT_20, unlock_repeated_backup=True
+        )
+        client.set_input_flow(IF.get())
+        ret = device.recover(client, type=messages.RecoveryType.UnlockRepeatedBackup)
+        assert ret == messages.Success(message="Backup unlocked")
+        assert (
+            client.features.backup_availability == messages.BackupAvailability.Available
+        )
+        assert client.features.recovery_status == messages.RecoveryStatus.Backup
+
+    # we can now perform another backup
+    with client:
+        IF = InputFlowSlip39BasicBackup(client, False, repeated=True)
+        client.set_input_flow(IF.get())
+        device.backup(client)
+
+    # backup type was upgraded:
+    assert client.features.backup_type == messages.BackupType.Slip39_Basic_Extendable
+    # the backup feature is locked again...
+    assert (
+        client.features.backup_availability == messages.BackupAvailability.NotAvailable
+    )
+    assert client.features.recovery_status == messages.RecoveryStatus.Nothing
+    with pytest.raises(TrezorFailure, match=r".*Seed already backed up"):
+        device.backup(client)
+
+
 @pytest.mark.setup_client(needs_backup=True, mnemonic=MNEMONIC_SLIP39_BASIC_20_3of6)
 @pytest.mark.skip_t1b1
 @WITH_MOCK_URANDOM

tests/input_flows.py

@@ -1421,12 +1421,18 @@ def load_N_shares(
 
 
 class InputFlowSlip39BasicBackup(InputFlowBase):
-    def __init__(self, client: Client, click_info: bool):
+    def __init__(self, client: Client, click_info: bool, repeated: bool = False):
         super().__init__(client)
         self.mnemonics: list[str] = []
         self.click_info = click_info
+        self.repeated = repeated
 
     def input_flow_tt(self) -> BRGeneratorType:
+        if self.repeated:
+            # intro confirmation screen
+            yield
+            self.debug.press_yes()
+
         yield  # 1. Backup intro
         self.debug.press_yes()
         yield  # 2. Checklist
@@ -1454,6 +1460,11 @@ class InputFlowSlip39BasicBackup(InputFlowBase):
         self.debug.press_yes()
 
     def input_flow_tr(self) -> BRGeneratorType:
+        if self.repeated:
+            # intro confirmation screen
+            yield
+            self.debug.press_yes()
+
         yield  # 1. Backup intro
         self.debug.press_yes()
         yield  # 2. Checklist
@@ -1481,6 +1492,11 @@ class InputFlowSlip39BasicBackup(InputFlowBase):
         self.debug.press_yes()
 
     def input_flow_t3t1(self) -> BRGeneratorType:
+        if self.repeated:
+            # intro confirmation screen
+            yield
+            self.debug.press_yes()
+
         yield  # 1. Backup intro
         self.debug.wait_layout()
         self.debug.swipe_up()
@@ -1587,12 +1603,17 @@ class InputFlowSlip39BasicResetRecovery(InputFlowBase):
 
 
 class InputFlowSlip39CustomBackup(InputFlowBase):
-    def __init__(self, client: Client, share_count: int):
+    def __init__(self, client: Client, share_count: int, repeated: bool = False):
         super().__init__(client)
         self.mnemonics: list[str] = []
         self.share_count = share_count
+        self.repeated = repeated
 
     def input_flow_tt(self) -> BRGeneratorType:
+        if self.repeated:
+            yield
+            self.debug.press_yes()
+
         if self.share_count > 1:
             yield  # Checklist
             self.debug.press_yes()
@@ -1611,6 +1632,10 @@ class InputFlowSlip39CustomBackup(InputFlowBase):
         self.debug.press_yes()
 
     def input_flow_tr(self) -> BRGeneratorType:
+        if self.repeated:
+            yield
+            self.debug.press_yes()
+
         if self.share_count > 1:
             yield  # Checklist
             self.debug.press_yes()
@@ -1629,6 +1654,10 @@ class InputFlowSlip39CustomBackup(InputFlowBase):
         self.debug.press_yes()
 
     def input_flow_t3t1(self) -> BRGeneratorType:
+        if self.repeated:
+            yield
+            self.debug.press_yes()
+
         if self.share_count > 1:
             yield  # Checklist
             self.debug.press_yes()