Trezor API - USB attach takes long time

[martin-lizner]

There are small troubles on USB level that makes device init last a bit longer (10-20 sec) in certain situations.

Repeat scenario:

1. Plugin Trezor, run any example requiring PIN.
2. Provide PIN, finish example happily. Keep Trezor plugged in.
3. Run same example, trezor does not ask for PIN (which is ok) but getting device to READY state takes considerably long time.
4. Replug device and try example again. Everything is fast again.

I suspect some problem when finishing communication and exiting services. Calling disconnect() or softDetach() on wallet doesnt help.

[martin-lizner]

This bug does not happen on KeepKey device. Which could mean that there is something different/wrong with Trezor FW. If we have an idea where to focus, Trezor dev might help with that.

[prusnak]

Update from trezor/trezor-mcu#98 (comment) - this also happens for Keepkey if passphrase is involved.

[bgok]

@prusnak Thanks for the heads up. we will confirm today.

[bgok]

@prusnak @jhoenicke Confirmed. We will wait for a response from @gary-rowe on gary-rowe/hid4java#42. If we don't hear back soon, we plan to fork and fix.

[gary-rowe]

Hi all,

Been off grid recently so I've not seen this solution. I'll have some time tomorrow to dig into hid4java and find a solution.

Cheers,

Gary

[gary-rowe]

I've pushed a quick workaround to hid4java (develop branch) that MultiBit Hardware could make use of as an interim measure for testing.

Do the following:

1. Update and Maven install hid4java develop branch as develop-SNAPSHOT
2. Reference this in MultiBit Hardware pom.xml
3. Update the following line in KeepKeyV1HidHardwareWallet and TrezorV1HidHardwareWallet:

hidServices = HidManager.getHidServices(true, 1000);

Once verified, I'll prepare a formal release of hid4java (likely 0.5.0) and push it to Maven Central. The value 1000 may be too generous and may lead to slow responses to attach/detach events.

[prusnak]

You don't understand the issue at all, do you?

[prusnak]

In order to fully workaround the issue we'd need to stop the enumerate bashing for few (3-5) seconds after a new device is detected.  

[bgok]

Thanks for looking at this, @gary-rowe.

[gary-rowe]

@prusnak My apologies. I didn't read your earlier comment referencing the Trezor firmware bug and simply assumed it was a simple case of reducing the frequency and not changing the nature of the scan entirely. If I can get away with a "pause scanning" API call in hid4java then that will simplify things greatly.

@bgok No worries - are you a collaborator on MultiBit Hardware? If so, would you be OK with making the code changes in MultiBit Hardware to support calls to pause scanning during device initialisation detection?

[bgok]

@gary-rowe I haven't had a chance to review the HID handling code yet, but it seems like a reasonable approach.

@martin-lizner Will this work for your project?

[martin-lizner]

Thanks everybody for taking care! Great community!

...bad news, with latest hid4java master Im not even able to get my trezor device (both 1.3.5 and 1.3.6 fw) to ready state. And it does not matter what I set in HidManager.getHidServices.

1.3.6 fw just shows CONNECTED state and waits
1.3.5 returns ERROR o.m.h.h.t.w.v.TrezorV1HidHardwareWallet - Unknown HID version.

Please note that Im using devel branch of multibit hw with trezor 1.3.6 PR merged. This PR works fine with hid4java 0.4.0 (except reported long delays) for both firmwares.

[jhoenicke]

Isn't the usage in the trezor pull request wrong? The report ID should not be put into the array, but that is what the last parameter is for.

 /**...
   * @param device   The device
   * @param data     The report data to write (should not include the Report ID)
   * @param len      The length of the report data (should not include the Report ID)
   * @param reportId The report ID (or (byte) 0x00)
   *
   * @return The number of bytes written, or -1 if an error occurs
   */
  public static int write(HidDeviceStructure device, byte[] data, int len, byte reportId) {

[jhoenicke]

To clarify, the first byte written to buffer (0 for 1.3.6, 63 for <1.3.5) in AbstractTrezorHardwareWallet.java is the report_id and should not be written to the buffer. The buffer should be one byte shorter. I think the return value includes the report_id in the count though).

[martin-lizner]

@jhoenicke thanks for the review, you may be right! To be honest, me + @prusnak little improvised to fix the code as fast as possible for 1.3.6 support, so we may not found the cleanest way. Anyway that PR is totally working as proven both in my app and MB HW examples project. I will try to find better code solution and if possible make a new pull request in order to MB HW to support 1.3.6 Trezor firmware.

But pls don't get distracted from the real issue, this has probably no connection at all with the problem we are talking about in this thread - long init delay after soft detach.