Skip to content
This repository has been archived by the owner on Nov 30, 2020. It is now read-only.

Add Trezor support #75

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add Trezor support #75

wants to merge 5 commits into from

Conversation

zulucrypto
Copy link

@zulucrypto zulucrypto commented Sep 17, 2018 •
edited

Description

Adds support for logging in with a Trezor and signing a payment transaction (fixes #70).

Note: This relies on unreleased Trezor firmware, please do not merge until it's available publicly.

The workflow for Trezor login/signing is:

  1. User clicks a button to sign in
  2. Trezor Javascript API call is made via TrezorConnect object
  3. New window is opened that communicates with the Trezor device and walks the user through signing
  4. When finished, new window is closed and the asynchronous TrezorConnect method returns

Trezor Connect project: https://github.com/trezor/connect
Trezor Connect documentation: https://wiki.trezor.io/Trezor_Connect_API
Stellar-specific documentation:

Changes

In general, I kept this the same as the Ledger implementation, with a couple changes:

  • bip32Path is now shared between Ledger and Trezor
  • Trezor signing can fail before the transaction is submitted (for example, if the signing window is closed early) so I've added a "pre-submit" error handler and error message.

Testing

Since Trezor hasn't released firmware with support for the Stellar functions, this must be tested with the Trezor emulator.

Instructions for installing the Model T emulator can be found here: https://github.com/trezor/trezor-core/blob/master/docs/build.md

In order to use it with trezor connect, you will also need to install the trezor bridge from here: https://github.com/trezor/trezord-go

See the section on "Emulator Support" at the trezord-go project.

OS X Testing Instructions

Requirements:

Follow these instructions: https://github.com/trezor/trezor-core/blob/master/docs/build.md#build-instructions-for-emulator-unix-port

At this point, the emulator will be installed. If you can run ./emu.sh proceed to the next step.

Install trezord bridge:

$ go get github.com/trezor/trezord-go
$ go build github.com/trezor/trezord-go
$ ./trezord-go -h

Once the emulator and trezord-go are installed, you will need to run this code. For convenience, I've created a docker container with everything ready to go:

$ docker run \
    --rm \
    -it \
    --name account-viewer-test \
    -p 8180:8000 \
    -p 8001:8001 \
    zulucrypto/stellar-account-viewer-test

If it says Finished 'start-serve' after 71 ms then it's working. You may see a warning about "Unexpected token", this is safe to ignore.

Now, in a separate terminal window, start trezord-go with emulator support:

$ ./trezord-go -e 21324

And finally, in a third window, start the emulator:

$ ./emu.sh

You should now be able to go to http://localhost:8180/ and see the account viewer.

Click "Sign in with Trezor". The first time you do this the device won't be initialized, so follow the prompts to create a new wallet and initialize the emulator.

Once you've done that, return to the account viewer and click "Sign in" again. This time, you should be prompted to share your account and then you will be logged in to the Account viewer.

You'll now have an unfunded account on the testnet. Fund it with curl:

$ curl https://friendbot.stellar.org?addr=YOURADDRESSHERE

Your balance should update and you'll be able to transfer testnet XLM to other accounts.

@fnando
Copy link
Contributor

fnando commented Sep 25, 2018

This is great! Thanks for jumping on it, @zulucrypto!

@zulucrypto
Copy link
Author

@prusnak Thanks, fixed!

@prusnak
Copy link

prusnak commented Oct 4, 2018

@zulucrypto Can you please update the PR to require connect 5.0.32? Also, can you please publish the built version of your PR somewhere so I can test? (I am hitting some strange build errors, so I can't build by myself)

@zulucrypto
Copy link
Author

@prusnak - Sure thing, just bumped the version to 5.0.32.

I'll work on making the built version available early next week.

@bartekn - Any chance there's a docker container for interstellar or developing on the account viewer? I also had a lot of difficulty getting it to build.

@bartekn
Copy link
Contributor

bartekn commented Oct 9, 2018

I am hitting some strange build errors, so I can't build by myself)

Any chance there's a docker container for interstellar or developing on the account viewer? I also had a lot of difficulty getting it to build.

Can you post the build log? If you are running Node 6.9 (I know it's old but interstellar is no longer maintained) you should be able to install it. nvm may be helpful.

@neanias
Copy link

neanias commented Oct 23, 2018

Hi there, I'm keen to see this get merged, can I help at all?

@zulucrypto
Copy link
Author

Hi @neanias - It could always use more testing! If you're running linux or OS X and interested in giving it a shot, let me know and I'll update with some more detailed testing/setup instructions.

@neanias
Copy link

neanias commented Oct 25, 2018

@zulucrypto, I use OS X/macOS and happy to give a hand. What can I do to help?

@zulucrypto
Copy link
Author

Thanks @neanias ! I've updated the PR description with some detailed OS X instructions. Give them a try when you get a chance and let me know how it goes.

@prusnak
Copy link

prusnak commented Oct 29, 2018 •
edited

I confirmed this works. Stellar firmware is released for both T1 and T2 (1.7.1 and 2.0.8) - available from https://beta-wallet.trezor.io./

@bartekn can you please review, merge and deploy to https://www.stellar.org/account-viewer/ ? Or is there anything else we need to do?

@spacesailor24
Copy link

Do you know what was left to finish implementing this?

I'm currently able to login with the Trezor model T, but am unable to sign transactions. Here is my reddit post with more details:

https://www.reddit.com/r/Stellar/comments/abynfg/send_fails_with_stellar_account_viewer/?utm_source=reddit-android

The latest Trezor firmware is now 2.0.10, does this affect your implementation?

@zulucrypto
Copy link
Author

Hi @spacesailor24 - I am able to reproduce the issue you're having when I use the emulator. Based on the error message, I'm guessing that it's an issue with the javascript library that communicates between the account viewer and the Trezor device.

Hopefully it will be fixed in trezor/trezor-core#421 . I'll keep an eye on that issue and contribute if necessary.

@bartekn - This shouldn't be merged until the above issue gets figured out since it seems to affect basic payments as well as more complicated transactions.

@prusnak
Copy link

prusnak commented Jan 10, 2019 •
edited

This should not be merged, because it does not work as is. In order to get this working extra patches from trezor branch are required: see https://github.com/trezor/stellar-account-viewer

Also, the current Ledger code is written very naively and sends U2F requests to all connected USB devices, that's why I have removed it from our fork.

@bartekn
Copy link
Contributor

bartekn commented Jan 14, 2019

@zulucrypto could you update this PR with @prusnak updates?

@prusnak
Copy link

prusnak commented Jan 14, 2019

Our fork is now fixed and works, feel free to pull the code.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add Trezor support
6 participants
@zulucrypto @fnando @prusnak @bartekn @neanias @spacesailor24