Skip to content
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

Support model R emulators #178

Merged
merged 14 commits into from
Jul 7, 2022
Merged

Support model R emulators #178

merged 14 commits into from
Jul 7, 2022

Conversation

grdddj
Copy link
Contributor

@grdddj grdddj commented May 25, 2022

Connected with #176:

  • supporting model R in tenv

Is a little hacky, for couple of reasons/limitations:

  • we need the latest trezorlib for model R support, but it is unreleased
    • I am unable to install it from our github repo through poetry install (known issue), so I am doing it manually through pip in Dockerfile as a last step
  • we need functional model R emulator(s), but they are not yet released
    • in current master we do have it running, just with a very limited capabilities
    • therefore, I am hardcoding download of from the "most featured branch" currently (grdddj/trezor_r_passphrase_input), which has prototypes of PIN entry, seed backup, seed recovery and passphrase entry

It is however easily possible for anybody to download any branch through the Start from URL feature - just giving it URL to some Gitlab job of core unix frozen R debug build (for example https://gitlab.com/satoshilabs/trezor/trezor-firmware/-/jobs/2498092302) and choosing TR.

What is currently missing is support for ARM - sorry guys @vdovhanych @sime @tsusanka - but it could be possible to add some model R ARM build info firmware CI :)

@sime sime requested a review from vdovhanych May 26, 2022 08:06
@grdddj
Copy link
Contributor Author

grdddj commented May 26, 2022

In 9622a46 I have included also the arm-version of current model R state.

How to test it on MAC (I cannot try it on Nixos):

  • either checking out this branch, running the ./src/binaries/firmware/bin/download.sh and seeing 2-master-R-arm in the choice of R models in dashboard
  • or using the image from this branch - ghcr.io/trezor/trezor-user-env:178 - just inputting this instead of "latest" ghcr.io/trezor/trezor-user-env in docker/compose.yml under trezor-user-env-mac and running ./run.sh

@vdovhanych
Copy link
Member

I will try to build a model R emulator on arm, if that works then there should be no reason it shouldn't work on m1. I will let you know here.

@grdddj
Copy link
Contributor Author

grdddj commented May 26, 2022

I will try to build a model R emulator on arm, if that works then there should be no reason it shouldn't work on m1. I will let you know here.

Thanks. I think the arm version should be fine already - firmware CI built it - https://gitlab.com/satoshilabs/trezor/trezor-firmware/-/jobs/2510249260 - and @sime verified it can be run through tenv

@grdddj grdddj force-pushed the grdddj/trezor_r_support branch 2 times, most recently from 728a857 to c9ffcdd Compare May 27, 2022 10:53
@grdddj grdddj force-pushed the grdddj/trezor_r_support branch from c9ffcdd to 82739d4 Compare June 30, 2022 12:27
@tsusanka
Copy link
Contributor

@grdddj is this ready to be reviewed and merged? Btw matejcik made a hotfix after the first trezorctl release (0.13.2), does this include that instead of the first version?

@grdddj
Copy link
Contributor Author

grdddj commented Jun 30, 2022

@grdddj is this ready to be reviewed and merged? Btw matejcik made a hotfix after the first trezorctl release (0.13.2), does this include that instead of the first version?

In short - it is theoretically ready for review, but not yet ready for merge. It is missing some stuff I will do tomorrow - rebase on master, some testing the trezorlib works fine etc.

@tsusanka
Copy link
Contributor

Ok just ping me if this would wait for me please :).

@grdddj grdddj force-pushed the grdddj/trezor_r_support branch from 82739d4 to cd12226 Compare July 1, 2022 08:27
@grdddj grdddj force-pushed the grdddj/trezor_r_support branch from f89a24d to c7e2e65 Compare July 4, 2022 09:36
@grdddj grdddj requested a review from tsusanka July 4, 2022 09:44
@grdddj
Copy link
Contributor Author

grdddj commented Jul 4, 2022

Ok just ping me if this would wait for me please :).

@tsusanka pinging you, as I think it is now ready for review and merge :)

The PR contains even some "non-modelR" things - see the list of commits

Should be fixing these issues:
#176
#179
#137
#181

Copy link
Contributor

@tsusanka tsusanka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could bikeshed some things but I do not think it is needed :). LGTM.

I will merge this on Wednesday due to the holidays. cc @mroz22 just so you know if you notice something odd on Suite side after the merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants