trezor-firmware icon indicating copy to clipboard operation
trezor-firmware copied to clipboard

Set correct backup type during BackupDevice

Open andrewkozlik opened this issue 6 months ago • 3 comments

Trezor currently uses the backup type that was specified in the ResetDevice message (forcing Slip39_Basic to Slip39_Basic_Extendable). However, the only thing that's relevant about the backup type provided in ResetDevice is whether it is BIP39, SLIP39 non-extendable or SLIP39 extendable. Other than that the actual backup type may change during BackupDevice, for example from Slip39_Basic_Extendable to Slip39_Single_Extendable or even Slip39_Advanced_Extendable.

In backup_device.py, perform_backup(), Trezor should correctly identify the actual backup type that will be created either:

  • based on the provided group_threshold and member_threshold for backup_slip39_custom(), or if not provided, then
  • based on the group_threshold and member_threshold entered by the user in backup_seed().

More context in slack.

andrewkozlik avatar Jun 05 '25 12:06 andrewkozlik

Also this logic is not correct: https://github.com/trezor/trezor-firmware/blob/765fe76b445f917962c5ae88cb732701720c77b8/core/src/apps/management/backup_device.py#L41-L45 The user may go from Single to Advanced (the TODO) or from Single to Single again (stupid scenario, I know, but we shouldn't assume that they upgraded to Basic). Also they may go from Basic to Advanced. The only question is: should we record the last backup type they used or the most advanced backup type they used?

andrewkozlik avatar Jun 11 '25 13:06 andrewkozlik

If we don't want to record all backups provided by Trezor, I would suggest to record the most advanced one. Slip39_Single_ExtendableSlip39_Basic_ExtendableSlip39_Advanced_Extendable

hynek-jina avatar Jun 11 '25 13:06 hynek-jina

Also they may go from Basic to Advanced.

can't because then they don't hit the if-condition. the logic is not entirely correct but per the comment "upgrade Single to Basic if necessary" it covers the most common path.

The only question is: should we record the last backup type they used or the most advanced backup type they used?

presumably the most advanced one -- but that means that in ResetDevice we must record Slip39_Single_Extendable in all SLIP39 cases (except when the backup is part of the reset flow)

matejcik avatar Jun 17 '25 11:06 matejcik