Trezor T UI improvements

almindor commented 6 years ago

Trezor T is now "supported" but needs more improvements before a release is done. Creating this as a separate issue.

Main thing is UI for handling of the workflow with passwords input on the device itself.

Continuation of #99

gkaindl commented 6 years ago

Yes, exactly. I don't have any experience with Qt though, which makes me way less effective here, though :-(

Anyway, the main differences between a Trezor One and Trezor T when it comes to the workflow are those:

If you haven't "unlocked" (e.g. entered the PIN) your Trezor T yet, there is no way to even detect its presence, as it only enables the USB endpoint once it has been unlocked. Conversely, the T will never ask for a PIN via the USB api (you have to enter it on-device at all times).
For the passphrase, the Passphrase Request message sent by the Trezor T will set the on_device flag, depending on whether the user chose to enter the passphrase on the device or normally via the app. If the latter is chosen, the workflow to actually enter the passphrase is the same as on the One. If the passphrase is entered on-device, a PassphraseAck should be sent without any passphrase set as a payload.
Once the passphrase is entered, the Trezor will send the new PassphraseStateRequest, which the client needs to reply to with a PassphraseStateAck. It looks as if this will soon be the same behavior on the One, once trezor-core gets ported to it, only that the PassphraseStateAck is more or less meaningless there and just tells the client that the Trezor is now ready.

Most importantly for this issue, if the PassphraseRequest is received with on_device set, it'd be ideal if Etherwall displayed a message like "Please enter your passphrase on your Trezor".

Btw, I've also joined your gitter to make discussion easier! Thanks for the invite! :-)

almindor commented 6 years ago

Ok so to sum it up I think:

we don't need to change anything for the PinMatrixRequest since the model T simply doesn't send it in
if we get a PassphraseRequest with on_device being set we should just display a badge (the blue timeouting message) to prompt user to enter password on device and wait for a PassphraseStateRequest and just Ack back on that. Once that is done we continue as if the password was accepted and continue with the signing.

almindor commented 6 years ago

Added the badge logic, can you test to see if you get the message displayed? You should just get a Input your password on the TREZOR device "badge" to show up if the password request is supposed to be filled on device.

gkaindl commented 6 years ago

I just tested it! It works – There's a badge shown while entering the passphrase on-device!

However, just before being prompted by the Trezor whether you want to enter the passphrase on-device or on the host, there's another badge shown briefly that says something like "Please complete the action on your Trezor: undefined". Maybe it's related to the ButtonRequest/ButtonAck handshake that happens before entering the passphrase? I'm not sure yet (but I didn't have much time today to investigate).

Also, 38d3dd396c1499e3721c184efc32efa95b64b6af seems to have broken the build on MacOS, since Q_OS_UNIX seems to be defined there as well. I fixed it by using #if defined(Q_OS_UNIX) && !defined(Q_OS_MACX) instead, but then DOWNLOAD_BASE_PATH is not defined in nodemanager.cpp – I "fixed" this by just adding an empty string instead to test now.

I can send you a PR for this if you want, but if this is all work in progress, it might not be useful right now.

almindor commented 6 years ago

Hey thanks for testing. I'll fix the UNIX defined. I wanted EW to be compilable on BSDs etc. Will have to exclude mac from the list.

Will try and find out what the action cause could be and get back. Ping here if you find out more there.

almindor commented 6 years ago

Is there any chance you can catch a screenshot of the badge with the unknown text? I can't find any source where that kind of description would be given.

gkaindl commented 6 years ago

Sorry for the late reply! Here's the screenshot: It happens before the prompt to choose where to enter the passphrase is shown. It's probably related to the ButtonRequest/ButtonAck messages?

almindor commented 6 years ago

I see, it's a missing ButtonRequest_PassphraseType = 14; enum value.

I suppose you need to choose if you want the password presented on trezor or on the device in this step?

I added the handler in master.

gkaindl commented 6 years ago

Yes, exactly, that was it! The workflow is working correctly now.

almindor / etherwall

Trezor T UI improvements #102