-
Notifications
You must be signed in to change notification settings - Fork 198
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
add onekey device support #709
base: master
Are you sure you want to change the base?
Conversation
A GHA run in my repo: https://github.com/OneKeyHQ/HWI/actions/runs/6212074159 |
@achow101 Please take care of this. Thxs |
4cf3346
to
d7df45f
Compare
the newest passed GHA https://github.com/OneKeyHQ/HWI/actions/runs/7110790185 |
Hello @achow101, we are still waiting to merge this PR, we have already rebased to latest commit and addressed questions and issues above. Could you kindly reveiw again base on our latest commit? |
Can you please rebase it? |
cdf99d9
to
e4ac963
Compare
Please squash your commits. |
Looking at the code, it seems this PR is not adding anything substantial. The Onekey devices are just clones of Trezor One and Trezor Model T. They even copied the internal names (T1B1 and T2T1). I am not sure it is really worth complicating the codebase and risking rendering of Trezor (and all its clones) support unusable (this PR modifies trezorlib too). Since Onekey tries to be compatible with Trezor (their FAQ mentions "support all third-party clients supported by Trezor"), maybe we should not change HWI at all and let Onekey be detected as Trezor via HWI? |
b6978af
to
6847858
Compare
Done! |
OneKey Touch/Pro are using different MCU and SE than Trezor T/One, which means hardware and low-level codes are totally different. We also designed and built our own GUI since the screen resolution is different, and we have no interest to have GUI looks the same. OneKey firmware was forked from Trezor, but I don't think building on top of a fork should count as a counterfeit, that's not how open-source projects work. Many great projects were based on a fork.
Due to the nature of initially forked from Trezor, our protocol is compatible with Trezor. However, it does not work the other way around, since OneKey has many things Trezor does not support. This is why we need a separate implementation to fully support our device, and adding OneKey support to HWI is the first step.
Introducing fewer changes possible was suggested by @achow101. Ideally, we want a separate library path. |
No description provided.