From fb05c1811380172f45a248bd2d7f056c9225f385 Mon Sep 17 00:00:00 2001 From: bithighlander Date: Sun, 7 Jun 2026 14:55:54 -0500 Subject: [PATCH 1/3] fix(keepkey-webusb): reset the IN pipe (not OUT) on a stalled read readChunk() reads the IN endpoint (transferIn), but on a "stall" status it called clearHalt("out", ...) -- the wrong endpoint and direction. On WinUSB the halted IN pipe then stays halted and every subsequent read re-stalls until a physical replug. It also fell through and returned the stalled transfer's (empty/short) buffer, which the framing parser rejected as "message not valid". Clear the halt on the IN pipe that actually stalled and throw a retryable "bad read" instead of returning a non-packet. Applied to both the Node (nodewebusb) and browser (webusb) transports. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/hdwallet-keepkey-nodewebusb/src/transport.ts | 7 +++++-- packages/hdwallet-keepkey-webusb/src/transport.ts | 5 ++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/hdwallet-keepkey-nodewebusb/src/transport.ts b/packages/hdwallet-keepkey-nodewebusb/src/transport.ts index 2a6e8954..0c2fca01 100644 --- a/packages/hdwallet-keepkey-nodewebusb/src/transport.ts +++ b/packages/hdwallet-keepkey-nodewebusb/src/transport.ts @@ -79,8 +79,11 @@ export class TransportDelegate implements keepkey.TransportDelegate { async readChunk(debugLink?: boolean): Promise { const result = await this.usbDevice.transferIn(debugLink ? 2 : 1, keepkey.SEGMENT_SIZE + 1); - 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"); From 5e5625eddc016f552a3e68fe74c6f2a79c11f67b Mon Sep 17 00:00:00 2001 From: bithighlander Date: Sun, 7 Jun 2026 14:57:34 -0500 Subject: [PATCH 2/3] fix(keepkey-nodewebusb): time-bound USB transfers so a wedged device can't hang forever transferIn/transferOut were submitted with no timeout (libusb defaults to infinite), so a device that stops responding -- mid-reboot, suspended by Windows USB selective-suspend, or unplugged mid-write -- left the read/write promise pending forever, holding the claimed interface and producing a false ConflictingApp on the next pair. Wrap each transfer in a timeout: DEFAULT_TIMEOUT (5s) for writes (which never legitimately block) and LONG_TIMEOUT (5min) for reads (which may wait on a device button press for PIN/passphrase/tx confirmation, so they must not be cut short). On timeout we throw; the caller's existing disconnect path closes the device, aborting the orphaned native transfer and releasing the interface. Note: the read ceiling is LONG_TIMEOUT because the transport cannot see the per-call timeout (msgTimeout in hdwallet-keepkey is set but never plumbed down). A follow-up could thread the call timeout through readChunk for tighter bounds on normal (non-button) operations. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/transport.ts | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/packages/hdwallet-keepkey-nodewebusb/src/transport.ts b/packages/hdwallet-keepkey-nodewebusb/src/transport.ts index 0c2fca01..8613f6ce 100644 --- a/packages/hdwallet-keepkey-nodewebusb/src/transport.ts +++ b/packages/hdwallet-keepkey-nodewebusb/src/transport.ts @@ -6,6 +6,25 @@ 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 -- the +// caller's disconnect path closes the device, which aborts the orphaned native +// transfer and releases the claimed interface (otherwise a later claimInterface +// hits LIBUSB code 19 / ConflictingApp against our own zombie transfer). +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,15 +88,25 @@ 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") { // Reset the halt on the IN pipe we just read (not OUT), then surface a From 7b183dc3e873fb040a49d98416d8d2ce0fcdeb0a Mon Sep 17 00:00:00 2001 From: highlander Date: Sun, 7 Jun 2026 15:44:52 -0500 Subject: [PATCH 3/3] docs(keepkey-nodewebusb): correct the interface-release claim on transfer timeout The prior comment overstated that the caller's disconnect path releases the interface on timeout. It does on the syncState/getFeatures recovery path but NOT on the sign/address RPC paths. Document the real behavior + the remaining options (harden caller, or release in-transport). --- .../hdwallet-keepkey-nodewebusb/src/transport.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/hdwallet-keepkey-nodewebusb/src/transport.ts b/packages/hdwallet-keepkey-nodewebusb/src/transport.ts index 8613f6ce..a20dd9c8 100644 --- a/packages/hdwallet-keepkey-nodewebusb/src/transport.ts +++ b/packages/hdwallet-keepkey-nodewebusb/src/transport.ts @@ -9,10 +9,18 @@ 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 -- the -// caller's disconnect path closes the device, which aborts the orphaned native -// transfer and releases the claimed interface (otherwise a later claimInterface -// hits LIBUSB code 19 / ConflictingApp against our own zombie transfer). +// 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(() => {});