search

stellar-deprecated / account-viewer

DEPRECATED. Go to https://github.com/stellar/account-viewer-v2
Apache License 2.0
62 stars 65 forks source link

Add Trezor support #75

Open zulucrypto opened 5 years ago

zulucrypto commented 5 years ago

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:

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 commented 5 years ago

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

zulucrypto commented 5 years ago

@prusnak Thanks, fixed!

prusnak commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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

zulucrypto commented 5 years ago

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 commented 5 years ago

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

zulucrypto commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 https://github.com/trezor/trezor-core/issues/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 commented 5 years ago

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 commented 5 years ago

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

prusnak commented 5 years ago

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