search

trezor / trezor-firmware

:lock: Trezor Firmware Monorepo
https://trezor.io
Other
1.29k stars 639 forks source link

Add missing capabilities #3966

Open komret opened 2 weeks ago

komret commented 2 weeks ago

I miss some properties among the device capabilities when working on Suite, which leads to hard-coding device models accross the code base and worse maintainability. I would appreciate adding the following flags among device capabilities. @trezor/connect will handle backwards compatibility. The names are just my suggestions, the list might not be exhaustive.

Capability_SecureElement: device has a secure element and an ability to check device authenticity; hardcoded e.g. here Capability_Tutorial: device has controls tutorials; hardcoded here Capability_TouchScreen: device has a touchscreen as opposed to buttons, hardcoded e.g. here Capability_DisplayRotation: device supports display rotation, hardcoded here

matejcik commented 2 weeks ago

Capability_SecureElement: device has a secure element and an ability to check device authenticity; hardcoded e.g. here Capability_Tutorial: device has controls tutorials; hardcoded here

presumably all future devices will support these two. we could even do a tutorial for TT, fwiw :)

Capability_TouchScreen: device has a touchscreen as opposed to buttons, hardcoded e.g. here

this is not a capability and in this particular place it just might happen that T3W1 will need different string. (i'm afraid this needs a per-model entry, where some texts happen to be identical. or, you know, you can switch on internal_model[2] where T is "touch" and B is "button" :)) )

Capability_DisplayRotation: device supports display rotation, hardcoded here

this is plausibly a capability

komret commented 2 weeks ago

Capability_SecureElement: device has a secure element and an ability to check device authenticity; hardcoded e.g. here Capability_Tutorial: device has controls tutorials; hardcoded here

presumably all future devices will support these two. we could even do a tutorial for TT, fwiw :)

So in cases like this you're not adding a capability flag and we catch it in Suite? Acceptable.

Capability_TouchScreen: device has a touchscreen as opposed to buttons, hardcoded e.g. here

this is not a capability and in this particular place it just might happen that T3W1 will need different string. (i'm afraid this needs a per-model entry, where some texts happen to be identical. or, you know, you can switch on internal_model[2] where T is "touch" and B is "button" :)) )

Good idea.

komret commented 2 weeks ago

Are you sure TS5 has a tutorial? If it does, Suite skips it.

matejcik commented 2 weeks ago

Are you sure TS5 has a tutorial?

https://github.com/trezor/trezor-firmware/issues/3829 i assumed that it is already done, but it's not. it is planned for the TS5 public release though

komret commented 2 weeks ago

😬 This complicates the condition needed in Suite. A flag for Capability_Tutorial would be helpful if a situation like this arises in the future.