-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: whitelist all device models #1335
Conversation
|
Is it recommended practice from the manufacturer to be so defensive @tomislavhoracek ? Why not make it unrestricted and we can then list the devices that have been tested? |
@rhyslbw I'm not sure about the manufacturer but Vacuumlabs suggested and said there is a lack of "Trezor Model One" support. So we made it more defensive, even in Daedalus. |
Does the VacuumLabs library make a similar guard on the Model One? If not, I think we can just drop it here and leave it up to applications to implement it if desired. |
I'm pretty sure I saw a guard. But, let me double check that |
@rhyslbw ok, there is no classic guard as before (in the new @trezor/connect pkg) but I found this: Shall we keep a guard then? |
Nice. Please remove it @tomislavhoracek as it's not the SDK's concern. Do we also have similar guards in the Ledger package? |
Yeah, we have it in SDK Ledger pkg. But we were just extending support how the devices were released. There is no concern about not supporting Cardano on one of them. I can remove a guard there as well |
Great. This will simplify maintenance requirements. Thanks @tomislavhoracek! |
Definitely. Thank you @rhyslbw 🙂 |
9982bd3
to
5fd31f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM @tomislavhoracek
Context
This PR removes the device model's guard. All the possible Trezor and Ledger devices are whitelisted now. It is up to the end app to restrict usage.
Jira
Important Changes Introduced
TODO: