diff --git a/packages/hdwallet-keepkey-nodewebusb/src/transport.ts b/packages/hdwallet-keepkey-nodewebusb/src/transport.ts index 2a6e8954..a20dd9c8 100644 --- a/packages/hdwallet-keepkey-nodewebusb/src/transport.ts +++ b/packages/hdwallet-keepkey-nodewebusb/src/transport.ts @@ -6,6 +6,33 @@ import { VENDOR_ID, WEBUSB_PRODUCT_ID } from "./utils"; export type Device = WebUSBDevice & { serialNumber: string }; +// Bound a single USB transfer so a wedged/suspended/unplugged device cannot +// block the caller forever (libusb submits transfers with an infinite timeout). +// Writes complete in well under DEFAULT_TIMEOUT; a read may legitimately wait on +// a device button press, so it gets LONG_TIMEOUT. On timeout we throw a retryable +// error and stop blocking the worker. +// +// CAVEAT: throwing here does NOT by itself release the claimed interface. The +// interface is only freed when something calls transport.disconnect() -> +// usbDevice.close(). That reliably happens on the syncState()/getFeatures() +// recovery path, but NOT on the sign/address RPC paths, which currently re-throw +// without disconnecting. Until the caller is hardened (or this is changed to do +// clearHalt + releaseInterface(0) + close() in-transport), a mid-sign timeout can +// leave interface 0 claimed until the next USB event triggers recovery -- which +// can surface as a transient LIBUSB code 19 / ConflictingApp on an immediate +// reconnect. Bounding the transfer is still strictly better than hanging forever. +function withTransferTimeout(promise: Promise, ms: number, label: string): Promise { + // Swallow a late rejection from the orphaned transfer after we've given up. + promise.catch(() => {}); + let timer: ReturnType; + return Promise.race([ + promise, + new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`${label} timed out after ${ms}ms`)), ms); + }), + ]).finally(() => clearTimeout(timer!)); +} + export class TransportDelegate implements keepkey.TransportDelegate { usbDevice: Device; @@ -69,18 +96,31 @@ export class TransportDelegate implements keepkey.TransportDelegate { } async writeChunk(buf: Uint8Array, debugLink?: boolean): Promise { - const result = await this.usbDevice.transferOut( - debugLink ? 2 : 1, - buf.buffer.slice(buf.byteOffset, buf.byteOffset + buf.byteLength) as ArrayBuffer + const result = await withTransferTimeout( + this.usbDevice.transferOut( + debugLink ? 2 : 1, + buf.buffer.slice(buf.byteOffset, buf.byteOffset + buf.byteLength) as ArrayBuffer + ), + core.DEFAULT_TIMEOUT, + "transferOut" ); if (result.status !== "ok" || result.bytesWritten !== buf.length) throw new Error("bad write"); } async readChunk(debugLink?: boolean): Promise { - const result = await this.usbDevice.transferIn(debugLink ? 2 : 1, keepkey.SEGMENT_SIZE + 1); + // LONG_TIMEOUT: a read may legitimately block on a device button press + // (PIN, passphrase, tx/seed confirmation), so it must not be cut short. + const result = await withTransferTimeout( + this.usbDevice.transferIn(debugLink ? 2 : 1, keepkey.SEGMENT_SIZE + 1), + core.LONG_TIMEOUT, + "transferIn" + ); - if (result.status === "stall" && result.data !== undefined) { - await this.usbDevice.clearHalt("out", debugLink ? 2 : 1); + if (result.status === "stall") { + // Reset the halt on the IN pipe we just read (not OUT), then surface a + // retryable error -- a stalled transfer's buffer is not a valid packet. + await this.usbDevice.clearHalt("in", debugLink ? 2 : 1); + throw new Error("bad read"); } else if (result.status !== "ok" || result.data === undefined) { throw new Error("bad read"); } diff --git a/packages/hdwallet-keepkey-webusb/src/transport.ts b/packages/hdwallet-keepkey-webusb/src/transport.ts index 6b3a6b87..122e1749 100644 --- a/packages/hdwallet-keepkey-webusb/src/transport.ts +++ b/packages/hdwallet-keepkey-webusb/src/transport.ts @@ -76,7 +76,10 @@ export class TransportDelegate implements keepkey.TransportDelegate { const { status, data } = await this.usbDevice.transferIn(debugLink ? 2 : 1, keepkey.SEGMENT_SIZE + 1); if (status === "stall") { - await this.usbDevice.clearHalt("out", debugLink ? 2 : 1); + // Reset the halt on the IN pipe we just read (not OUT), then surface a + // retryable error -- a stalled transfer's buffer is not a valid packet. + await this.usbDevice.clearHalt("in", debugLink ? 2 : 1); + throw new Error("bad read"); } if (data === undefined) throw new Error("bad read");