From da50506b231f0057c299c480f08630d7ed67ac9b Mon Sep 17 00:00:00 2001 From: highlander Date: Mon, 3 Aug 2026 21:19:20 -0300 Subject: [PATCH 1/6] fix(keepkey): derive BIP-86 account xpubs with a wire type firmware accepts fsm_msgGetPublicKey selects the xpub version bytes from the script type and recognises only SPENDADDRESS, SPENDP2SHWITNESS and SPENDWITNESS; anything else falls through to "Invalid combination of coin and script_type". Sending SPENDTAPROOT therefore fails outright, so BTC account discovery dropped Taproot from the supported set and no P2TR account could be listed. BIP-86 account xpubs use the same serialization as legacy account xpubs, so SPENDADDRESS is the correct wire type for this message. P2TR is still sent on address and signing requests, where the firmware does handle it. --- packages/hdwallet-keepkey/src/keepkey.ts | 10 +++++++- packages/hdwallet-keepkey/src/taproot.test.ts | 25 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/packages/hdwallet-keepkey/src/keepkey.ts b/packages/hdwallet-keepkey/src/keepkey.ts index ddfb3446..4ecacefc 100644 --- a/packages/hdwallet-keepkey/src/keepkey.ts +++ b/packages/hdwallet-keepkey/src/keepkey.ts @@ -893,7 +893,15 @@ export class KeepKeyHDWallet implements core.HDWallet, core.BTCWallet, core.ETHW GPK.setAddressNList(addressNList); GPK.setShowDisplay(showDisplay || false); GPK.setEcdsaCurveName(curve || "secp256k1"); - GPK.setScriptType(translateInputScriptType(scriptType || core.BTCInputScriptType.SpendAddress)); + // A BIP-86 account xpub uses the same serialization as a legacy account + // xpub. Current KeepKey firmware derives it from the BIP-86 path, but its + // GetPublicKey handler rejects SPENDTAPROOT. Keep P2TR on address/signing + // requests and use SPENDADDRESS only for this xpub-derivation message. + const publicKeyScriptType = + scriptType === core.BTCInputScriptType.SpendTaproot + ? core.BTCInputScriptType.SpendAddress + : scriptType || core.BTCInputScriptType.SpendAddress; + GPK.setScriptType(translateInputScriptType(publicKeyScriptType)); const event = await this.transport.call(Messages.MessageType.MESSAGETYPE_GETPUBLICKEY, GPK, { msgTimeout: showDisplay ? core.LONG_TIMEOUT : core.DEFAULT_TIMEOUT, diff --git a/packages/hdwallet-keepkey/src/taproot.test.ts b/packages/hdwallet-keepkey/src/taproot.test.ts index c5942687..2b95f8d5 100644 --- a/packages/hdwallet-keepkey/src/taproot.test.ts +++ b/packages/hdwallet-keepkey/src/taproot.test.ts @@ -63,6 +63,31 @@ describe("KeepKey Taproot host support", () => { ).resolves.toBe("bc1ptest"); }); + it("derives a BIP-86 account xpub with the firmware-compatible SPENDADDRESS wire type", async () => { + const call = jest.fn().mockImplementation((messageType: number, msg: Messages.GetPublicKey) => { + expect(messageType).toBe(Messages.MessageType.MESSAGETYPE_GETPUBLICKEY); + expect(msg.getAddressNList()).toEqual(BIP86_ACCOUNT); + expect(msg.getCoinName()).toBe("Bitcoin"); + expect(msg.getScriptType()).toBe(Types.InputScriptType.SPENDADDRESS); + + const response = new Messages.PublicKey(); + response.setXpub("xpub-bip86"); + return Promise.resolve({ proto: response }); + }); + const wallet = new KeepKeyHDWallet(makeMockTransport(call)); + + await expect( + wallet.getPublicKeys([ + { + coin: "Bitcoin", + addressNList: BIP86_ACCOUNT, + curve: "secp256k1", + scriptType: core.BTCInputScriptType.SpendTaproot, + }, + ]) + ).resolves.toEqual([{ xpub: "xpub-bip86" }]); + }); + it("requires the firmware-reported supports_taproot capability", async () => { const supported = new KeepKeyHDWallet( makeMockTransport(jest.fn().mockResolvedValue({ message: { supportsTaproot: true } })) From 8cfd1fb300c8cef9291a8e8d625c507be810f34f Mon Sep 17 00:00:00 2001 From: highlander Date: Wed, 5 Aug 2026 00:22:51 -0300 Subject: [PATCH 2/6] feat(keepkey): expose on-device dice entropy on ResetDevice Adds an optional diceEntropy flag to core.ResetDevice and sets ResetDevice.dice_entropy when it is requested, so a host can ask the device to collect dice rolls with its own button before the seed is committed. The rolls never reach the host. That is the point: desktop-entered dice only help while the device's own randomness stays secret, so they are no defence against a compromised host combined with a weak device RNG. Entering them on the signer removes the host from the trust path entirely. Set only when requested -- firmware older than 7.15.0 has no such field and would reject an unknown one, so an unconditional set would break every existing device. device-protocol moves to the BitHighlander fork at tag dice-entropy-v1 (= cf308fd5e, the merge carrying ResetDevice.dice_entropy). A tag rather than a bare SHA because yarn resolves refs, not arbitrary commits. hdwallet-core and hdwallet-keepkey both build. --- packages/hdwallet-core/src/wallet.ts | 9 +++++++++ packages/hdwallet-keepkey/package.json | 2 +- packages/hdwallet-keepkey/src/keepkey.ts | 6 ++++++ yarn.lock | 4 ++-- 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/hdwallet-core/src/wallet.ts b/packages/hdwallet-core/src/wallet.ts index abd9830d..ef92ed83 100644 --- a/packages/hdwallet-core/src/wallet.ts +++ b/packages/hdwallet-core/src/wallet.ts @@ -52,6 +52,15 @@ export interface ResetDevice { pin?: boolean; autoLockDelayMs?: number; u2fCounter?: number; + /** + * Collect dice rolls ON THE DEVICE and fold them into the internal entropy + * before it is committed. The rolls never reach the host, which is the + * whole point: desktop-entered dice only help while the device's own + * randomness stays secret, so they do not defend against a compromised + * host combined with a weak device RNG. Requires firmware >= 7.15.0; + * older firmware ignores the field. + */ + diceEntropy?: boolean; } export interface RecoverDevice { diff --git a/packages/hdwallet-keepkey/package.json b/packages/hdwallet-keepkey/package.json index 93e3f263..ed5839c4 100644 --- a/packages/hdwallet-keepkey/package.json +++ b/packages/hdwallet-keepkey/package.json @@ -17,7 +17,7 @@ "dependencies": { "@ethereumjs/common": "^2.4.0", "@ethereumjs/tx": "^3.3.0", - "@keepkey/device-protocol": "https://github.com/keepkey/device-protocol.git#674777f6d4dd16e2b8c4c2df10608976375ee879", + "@keepkey/device-protocol": "https://github.com/BitHighlander/device-protocol.git#dice-entropy-v1", "@keepkey/hdwallet-core": "1.53.16", "@keepkey/proto-tx-builder": "^0.9.1", "@shapeshiftoss/bitcoinjs-lib": "5.2.0-shapeshift.2", diff --git a/packages/hdwallet-keepkey/src/keepkey.ts b/packages/hdwallet-keepkey/src/keepkey.ts index 4ecacefc..47d3aa3d 100644 --- a/packages/hdwallet-keepkey/src/keepkey.ts +++ b/packages/hdwallet-keepkey/src/keepkey.ts @@ -937,6 +937,12 @@ export class KeepKeyHDWallet implements core.HDWallet, core.BTCWallet, core.ETHW resetDevice.setAutoLockDelayMs(msg.autoLockDelayMs); } resetDevice.setU2fCounter(msg.u2fCounter || Math.floor(+new Date() / 1000)); + if (msg.diceEntropy) { + // Only set when requested. Firmware older than 7.15.0 has no such field + // and would reject an unknown one, so an unconditional set would break + // every existing device. + resetDevice.setDiceEntropy(true); + } // resetDevice.setWordsPerGape(wordsPerScreen) // Re-enable when patch gets in // Send await this.transport.call(Messages.MessageType.MESSAGETYPE_RESETDEVICE, resetDevice, { diff --git a/yarn.lock b/yarn.lock index 418ce772..da48c16e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1151,9 +1151,9 @@ "@jridgewell/resolve-uri" "^3.1.0" "@jridgewell/sourcemap-codec" "^1.4.14" -"@keepkey/device-protocol@https://github.com/keepkey/device-protocol.git#674777f6d4dd16e2b8c4c2df10608976375ee879": +"@keepkey/device-protocol@https://github.com/BitHighlander/device-protocol.git#dice-entropy-v1": version "7.14.1" - resolved "https://github.com/keepkey/device-protocol.git#674777f6d4dd16e2b8c4c2df10608976375ee879" + resolved "https://github.com/BitHighlander/device-protocol.git#cf308fd5e76f3f2e02ba38347ceccaa382b08505" dependencies: google-protobuf "3.21.4" pbjs "^0.0.5" From 4aa9b5ed088941700c0e143b35ba603ddf1b298d Mon Sep 17 00:00:00 2001 From: highlander Date: Mon, 10 Aug 2026 18:24:59 -0600 Subject: [PATCH 3/6] test(keepkey): cover the dice-entropy flag on ResetDevice The feature shipped with no tests. The property worth pinning is not that the flag can be set, but that ResetDevice.dice_entropy stays ABSENT unless asked for: the field only exists in firmware >= 7.15.0 and older firmware rejects a message carrying an unknown field, so setting it unconditionally would break wallet creation on every device already in the field. Covers set-when-requested, absent-when-omitted, absent-when-explicitly-false, a wire-format round trip (present-but-false is not the same as absent), and that the other reset fields still populate alongside it. Verified these fail for the right reason: replacing the guard with an unconditional setDiceEntropy(!!msg.diceEntropy) turns three of them red. --- .../src/reset-dice-entropy.test.ts | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 packages/hdwallet-keepkey/src/reset-dice-entropy.test.ts diff --git a/packages/hdwallet-keepkey/src/reset-dice-entropy.test.ts b/packages/hdwallet-keepkey/src/reset-dice-entropy.test.ts new file mode 100644 index 00000000..b84173d7 --- /dev/null +++ b/packages/hdwallet-keepkey/src/reset-dice-entropy.test.ts @@ -0,0 +1,97 @@ +/** + * Unit tests for the dice-entropy flag on KeepKeyHDWallet.reset(). + * + * `diceEntropy` asks the device to collect dice rolls with its own button and + * fold them into the internal entropy before the seed is committed. The rolls + * never reach the host — that is the point, since desktop-entered dice only + * help while the device's own randomness stays secret. + * + * The load-bearing property here is NOT that the flag can be set; it is that + * the field is left ABSENT unless explicitly requested. ResetDevice.dice_entropy + * only exists in firmware >= 7.15.0, and older firmware rejects a message + * carrying an unknown field — so setting it unconditionally would break wallet + * creation on every device already in the field. These tests pin that. + */ +import * as Messages from "@keepkey/device-protocol/lib/messages_pb"; + +import { KeepKeyHDWallet } from "./keepkey"; + +function makeMockTransport() { + return { + debugLink: false, + call: jest.fn().mockResolvedValue({ message_type: "Success", message: {} }), + getDeviceID: jest.fn().mockResolvedValue("mock-device-id"), + keyring: { addAlias: jest.fn() }, + } as any; +} + +/** The ResetDevice protobuf the wallet handed to the transport. */ +function sentResetDevice(transport: any): Messages.ResetDevice { + expect(transport.call).toHaveBeenCalledTimes(1); + const [messageType, proto] = transport.call.mock.calls[0]; + expect(messageType).toBe(Messages.MessageType.MESSAGETYPE_RESETDEVICE); + return proto as Messages.ResetDevice; +} + +const BASE_RESET = { entropy: 128, label: "test", pin: true, passphrase: false } as const; + +describe("KeepKeyHDWallet.reset() dice entropy", () => { + it("sets dice_entropy when diceEntropy is requested", async () => { + const transport = makeMockTransport(); + await new KeepKeyHDWallet(transport).reset({ ...BASE_RESET, diceEntropy: true }); + + const sent = sentResetDevice(transport); + expect(sent.hasDiceEntropy()).toBe(true); + expect(sent.getDiceEntropy()).toBe(true); + }); + + // The old-firmware guard. A device on < 7.15.0 rejects ResetDevice carrying an + // unknown field, so an absent flag must stay genuinely absent on the wire — + // "present but false" is not good enough. + it("leaves dice_entropy ABSENT when diceEntropy is omitted", async () => { + const transport = makeMockTransport(); + await new KeepKeyHDWallet(transport).reset({ ...BASE_RESET }); + + expect(sentResetDevice(transport).hasDiceEntropy()).toBe(false); + }); + + it("leaves dice_entropy ABSENT when diceEntropy is explicitly false", async () => { + const transport = makeMockTransport(); + await new KeepKeyHDWallet(transport).reset({ ...BASE_RESET, diceEntropy: false }); + + expect(sentResetDevice(transport).hasDiceEntropy()).toBe(false); + }); + + it("serializes without the field when absent, and with it when set", async () => { + // Round-trip through the wire format: the absent case must not carry the + // field at all, which is what old firmware actually parses. + const off = makeMockTransport(); + await new KeepKeyHDWallet(off).reset({ ...BASE_RESET }); + const withoutDice = Messages.ResetDevice.deserializeBinary(sentResetDevice(off).serializeBinary()); + expect(withoutDice.hasDiceEntropy()).toBe(false); + + const on = makeMockTransport(); + await new KeepKeyHDWallet(on).reset({ ...BASE_RESET, diceEntropy: true }); + const withDice = Messages.ResetDevice.deserializeBinary(sentResetDevice(on).serializeBinary()); + expect(withDice.hasDiceEntropy()).toBe(true); + expect(withDice.getDiceEntropy()).toBe(true); + }); + + it("still populates the other reset fields when dice entropy is on", async () => { + const transport = makeMockTransport(); + await new KeepKeyHDWallet(transport).reset({ + entropy: 256, + label: "dice wallet", + pin: true, + passphrase: true, + diceEntropy: true, + }); + + const sent = sentResetDevice(transport); + expect(sent.getStrength()).toBe(256); + expect(sent.getLabel()).toBe("dice wallet"); + expect(sent.getPinProtection()).toBe(true); + expect(sent.getPassphraseProtection()).toBe(true); + expect(sent.getDisplayRandom()).toBe(false); + }); +}); From ecc42f3844cf85cdb39d611be32568d991e42c79 Mon Sep 17 00:00:00 2001 From: highlander Date: Mon, 10 Aug 2026 18:34:54 -0600 Subject: [PATCH 4/6] chore(keepkey): pin device-protocol to canonical keepkey, not a personal fork MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dice-entropy work pinned @keepkey/device-protocol at BitHighlander/device-protocol#dice-entropy-v1, which would have put a personal fork in the dependency chain of a published package the moment this reached master. It is no longer necessary: the same commit (cf308fd5) is already merged into keepkey/device-protocol on up/release-protocol — the branch the vault has tracked all along — so pin its current tip fbb483f1 instead. ResetDevice still carries dice_entropy, and ButtonRequest_DiceRoll is present in types. Verified by reinstall: yarn.lock moves only the device-protocol entry, and the full hdwallet-keepkey suite passes on the canonical package. --- packages/hdwallet-keepkey/package.json | 2 +- yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/hdwallet-keepkey/package.json b/packages/hdwallet-keepkey/package.json index ed5839c4..1d36e2e3 100644 --- a/packages/hdwallet-keepkey/package.json +++ b/packages/hdwallet-keepkey/package.json @@ -17,7 +17,7 @@ "dependencies": { "@ethereumjs/common": "^2.4.0", "@ethereumjs/tx": "^3.3.0", - "@keepkey/device-protocol": "https://github.com/BitHighlander/device-protocol.git#dice-entropy-v1", + "@keepkey/device-protocol": "https://github.com/keepkey/device-protocol.git#fbb483f1cadc6394d51566ca4be0cfe6daa40000", "@keepkey/hdwallet-core": "1.53.16", "@keepkey/proto-tx-builder": "^0.9.1", "@shapeshiftoss/bitcoinjs-lib": "5.2.0-shapeshift.2", diff --git a/yarn.lock b/yarn.lock index da48c16e..4c19699a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1151,9 +1151,9 @@ "@jridgewell/resolve-uri" "^3.1.0" "@jridgewell/sourcemap-codec" "^1.4.14" -"@keepkey/device-protocol@https://github.com/BitHighlander/device-protocol.git#dice-entropy-v1": +"@keepkey/device-protocol@https://github.com/keepkey/device-protocol.git#fbb483f1cadc6394d51566ca4be0cfe6daa40000": version "7.14.1" - resolved "https://github.com/BitHighlander/device-protocol.git#cf308fd5e76f3f2e02ba38347ceccaa382b08505" + resolved "https://github.com/keepkey/device-protocol.git#fbb483f1cadc6394d51566ca4be0cfe6daa40000" dependencies: google-protobuf "3.21.4" pbjs "^0.0.5" From b1b1420dae50525dc4fd84e4aa74ab8c491490be Mon Sep 17 00:00:00 2001 From: highlander Date: Mon, 10 Aug 2026 18:47:44 -0600 Subject: [PATCH 5/6] fix(keepkey): refuse dice entropy on firmware that would silently ignore it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original guard rested on a false premise. Its comment claimed firmware older than v7.15.0 "would reject an unknown field", so merely omitting dice_entropy was treated as sufficient. nanopb does the opposite: unknown fields are SKIPPED, not rejected — lib/transport/pb_decode.c:904, "No match found, skip data", identical on the v7.14.1 tag. So reset({ diceEntropy: true }) against pre-7.15 firmware SUCCEEDED. The device ignored the field and produced an ordinary RNG-only seed while the caller believed dice rolls had been folded in — a silent downgrade of a security property that was explicitly requested, with no error anywhere. Gate it instead: supportsDiceEntropy() reports whether the device honours the field, reset() throws before sending when it does not, and an unreadable firmware version reports false so the check fails closed. Naming the version in the error means a caller on old firmware learns why rather than guessing. Adds supportsDiceEntropy() as a public capability check so callers can hide the option up front instead of discovering it at reset time. The vault already routes around the dice step on old firmware; this makes the library safe for callers that do not. Verified the new tests fail for the right reason: removing the gate turns the three downgrade cases red, including the one asserting no ResetDevice is sent. Reported in review of #63. --- packages/hdwallet-keepkey/src/keepkey.ts | 30 ++++- .../src/reset-dice-entropy.test.ts | 127 ++++++++++++++---- 2 files changed, 131 insertions(+), 26 deletions(-) diff --git a/packages/hdwallet-keepkey/src/keepkey.ts b/packages/hdwallet-keepkey/src/keepkey.ts index 47d3aa3d..6e8b3bb4 100644 --- a/packages/hdwallet-keepkey/src/keepkey.ts +++ b/packages/hdwallet-keepkey/src/keepkey.ts @@ -938,9 +938,19 @@ export class KeepKeyHDWallet implements core.HDWallet, core.BTCWallet, core.ETHW } resetDevice.setU2fCounter(msg.u2fCounter || Math.floor(+new Date() / 1000)); if (msg.diceEntropy) { - // Only set when requested. Firmware older than 7.15.0 has no such field - // and would reject an unknown one, so an unconditional set would break - // every existing device. + // Refuse rather than send. Firmware before v7.15.0 has no dice_entropy + // field, and nanopb SKIPS unknown fields instead of rejecting them + // (lib/transport/pb_decode.c:904, "No match found, skip data" — same on + // v7.14.1). So sending the flag to old firmware SUCCEEDS and quietly + // produces an ordinary RNG-only seed while the caller believes dice + // entropy was folded in. A silent downgrade of a security property the + // caller explicitly asked for is worse than a failed reset. + if (!(await this.supportsDiceEntropy())) { + throw new Error( + `Dice entropy requires KeepKey firmware v7.15.0 or later; device reports ${await this.getFirmwareVersion()}. ` + + `Refusing to reset, because this firmware would ignore the request and create an ordinary RNG-only wallet.` + ); + } resetDevice.setDiceEntropy(true); } // resetDevice.setWordsPerGape(wordsPerScreen) // Re-enable when patch gets in @@ -1373,6 +1383,20 @@ export class KeepKeyHDWallet implements core.HDWallet, core.BTCWallet, core.ETHW return semver.gte(await this.getFirmwareVersion(), "v7.2.1"); } + /** + * Whether the device honours ResetDevice.dice_entropy (on-device dice rolls + * mixed into the seed entropy). Support starts in v7.15.0. + * + * Callers must check this before offering dice entropy: older firmware does + * not reject the unknown field, it silently ignores it, so an ungated + * request produces an ordinary RNG-only wallet with no error. Fails closed — + * an unreadable firmware version reports false rather than assuming support. + */ + public async supportsDiceEntropy(): Promise { + const version = await this.getFirmwareVersion(); + return !!semver.valid(version) && semver.gte(version, "v7.15.0"); + } + public async btcSignMessage(msg: core.BTCSignMessage): Promise { return Btc.btcSignMessage(this, this.transport, msg); } diff --git a/packages/hdwallet-keepkey/src/reset-dice-entropy.test.ts b/packages/hdwallet-keepkey/src/reset-dice-entropy.test.ts index b84173d7..9fff1118 100644 --- a/packages/hdwallet-keepkey/src/reset-dice-entropy.test.ts +++ b/packages/hdwallet-keepkey/src/reset-dice-entropy.test.ts @@ -6,38 +6,62 @@ * never reach the host — that is the point, since desktop-entered dice only * help while the device's own randomness stays secret. * - * The load-bearing property here is NOT that the flag can be set; it is that - * the field is left ABSENT unless explicitly requested. ResetDevice.dice_entropy - * only exists in firmware >= 7.15.0, and older firmware rejects a message - * carrying an unknown field — so setting it unconditionally would break wallet - * creation on every device already in the field. These tests pin that. + * Two properties are load-bearing, and both are security properties: + * + * 1. The field is left ABSENT unless requested, so the message stays + * byte-identical to today's for every caller that does not ask for dice. + * + * 2. Requesting it against firmware that cannot honour it FAILS, rather than + * silently producing an ordinary RNG-only wallet. Firmware before v7.15.0 + * has no dice_entropy field, and nanopb SKIPS unknown fields rather than + * rejecting them (lib/transport/pb_decode.c:904, "No match found, skip + * data" — verified identical on v7.14.1). So the naive "just set it, old + * firmware will reject it" assumption is wrong: the reset would succeed + * and the caller would believe dice entropy was used when it was not. */ import * as Messages from "@keepkey/device-protocol/lib/messages_pb"; import { KeepKeyHDWallet } from "./keepkey"; -function makeMockTransport() { +/** + * Mock transport that answers GetFeatures with a chosen firmware version and + * accepts anything else. `features: null` simulates a device whose Features + * carry no version fields. + */ +function makeMockTransport(opts: { major?: number; minor?: number; patch?: number; features?: null } = {}) { + const { major = 7, minor = 15, patch = 0 } = opts; + const featuresMessage = + opts.features === null ? { deviceId: "mock-device-id" } : { deviceId: "mock-device-id", majorVersion: major, minorVersion: minor, patchVersion: patch }; + return { debugLink: false, - call: jest.fn().mockResolvedValue({ message_type: "Success", message: {} }), + call: jest.fn().mockImplementation((messageType: number) => { + if (messageType === Messages.MessageType.MESSAGETYPE_GETFEATURES) { + return Promise.resolve({ message_type: "Features", message: featuresMessage }); + } + return Promise.resolve({ message_type: "Success", message: {} }); + }), getDeviceID: jest.fn().mockResolvedValue("mock-device-id"), keyring: { addAlias: jest.fn() }, } as any; } -/** The ResetDevice protobuf the wallet handed to the transport. */ +/** The ResetDevice protobuf the wallet handed to the transport, if any. */ function sentResetDevice(transport: any): Messages.ResetDevice { - expect(transport.call).toHaveBeenCalledTimes(1); - const [messageType, proto] = transport.call.mock.calls[0]; - expect(messageType).toBe(Messages.MessageType.MESSAGETYPE_RESETDEVICE); - return proto as Messages.ResetDevice; + const call = transport.call.mock.calls.find((c: any[]) => c[0] === Messages.MessageType.MESSAGETYPE_RESETDEVICE); + expect(call).toBeDefined(); + return call[1] as Messages.ResetDevice; +} + +function resetWasSent(transport: any): boolean { + return transport.call.mock.calls.some((c: any[]) => c[0] === Messages.MessageType.MESSAGETYPE_RESETDEVICE); } const BASE_RESET = { entropy: 128, label: "test", pin: true, passphrase: false } as const; describe("KeepKeyHDWallet.reset() dice entropy", () => { - it("sets dice_entropy when diceEntropy is requested", async () => { - const transport = makeMockTransport(); + it("sets dice_entropy when requested on supporting firmware", async () => { + const transport = makeMockTransport({ minor: 15 }); await new KeepKeyHDWallet(transport).reset({ ...BASE_RESET, diceEntropy: true }); const sent = sentResetDevice(transport); @@ -45,9 +69,6 @@ describe("KeepKeyHDWallet.reset() dice entropy", () => { expect(sent.getDiceEntropy()).toBe(true); }); - // The old-firmware guard. A device on < 7.15.0 rejects ResetDevice carrying an - // unknown field, so an absent flag must stay genuinely absent on the wire — - // "present but false" is not good enough. it("leaves dice_entropy ABSENT when diceEntropy is omitted", async () => { const transport = makeMockTransport(); await new KeepKeyHDWallet(transport).reset({ ...BASE_RESET }); @@ -63,14 +84,13 @@ describe("KeepKeyHDWallet.reset() dice entropy", () => { }); it("serializes without the field when absent, and with it when set", async () => { - // Round-trip through the wire format: the absent case must not carry the - // field at all, which is what old firmware actually parses. + // Round-trip through the wire format: "present but false" is not the same + // as absent, and only absent leaves old firmware's parse unchanged. const off = makeMockTransport(); await new KeepKeyHDWallet(off).reset({ ...BASE_RESET }); - const withoutDice = Messages.ResetDevice.deserializeBinary(sentResetDevice(off).serializeBinary()); - expect(withoutDice.hasDiceEntropy()).toBe(false); + expect(Messages.ResetDevice.deserializeBinary(sentResetDevice(off).serializeBinary()).hasDiceEntropy()).toBe(false); - const on = makeMockTransport(); + const on = makeMockTransport({ minor: 15 }); await new KeepKeyHDWallet(on).reset({ ...BASE_RESET, diceEntropy: true }); const withDice = Messages.ResetDevice.deserializeBinary(sentResetDevice(on).serializeBinary()); expect(withDice.hasDiceEntropy()).toBe(true); @@ -78,7 +98,7 @@ describe("KeepKeyHDWallet.reset() dice entropy", () => { }); it("still populates the other reset fields when dice entropy is on", async () => { - const transport = makeMockTransport(); + const transport = makeMockTransport({ minor: 15 }); await new KeepKeyHDWallet(transport).reset({ entropy: 256, label: "dice wallet", @@ -95,3 +115,64 @@ describe("KeepKeyHDWallet.reset() dice entropy", () => { expect(sent.getDisplayRandom()).toBe(false); }); }); + +describe("KeepKeyHDWallet.reset() refuses a silent dice downgrade", () => { + // The regression this whole gate exists for. Old firmware does not reject + // dice_entropy — it skips it — so without the gate the wallet is created + // with RNG-only entropy and nobody is told. + it("throws instead of resetting when firmware predates v7.15.0", async () => { + const transport = makeMockTransport({ minor: 14, patch: 1 }); + + await expect(new KeepKeyHDWallet(transport).reset({ ...BASE_RESET, diceEntropy: true })).rejects.toThrow( + /Dice entropy requires KeepKey firmware v7\.15\.0 or later/ + ); + // The critical assertion: nothing was sent. A thrown error after a + // successful reset would still have created an RNG-only wallet. + expect(resetWasSent(transport)).toBe(false); + }); + + it("names the offending firmware version in the error", async () => { + const transport = makeMockTransport({ major: 7, minor: 14, patch: 1 }); + const err = await new KeepKeyHDWallet(transport) + .reset({ ...BASE_RESET, diceEntropy: true }) + .catch((e: any) => e); + + expect(String(err.message)).toContain("v7.14.1"); + }); + + it("fails closed when the firmware version is unreadable", async () => { + const transport = makeMockTransport({ features: null }); + + await expect(new KeepKeyHDWallet(transport).reset({ ...BASE_RESET, diceEntropy: true })).rejects.toThrow( + /Dice entropy requires KeepKey firmware/ + ); + expect(resetWasSent(transport)).toBe(false); + }); + + it("still resets normally on old firmware when dice entropy is not requested", async () => { + // The gate must not become a general old-firmware block. + const transport = makeMockTransport({ minor: 14, patch: 1 }); + await new KeepKeyHDWallet(transport).reset({ ...BASE_RESET }); + + expect(resetWasSent(transport)).toBe(true); + expect(sentResetDevice(transport).hasDiceEntropy()).toBe(false); + }); +}); + +describe("KeepKeyHDWallet.supportsDiceEntropy()", () => { + it.each([ + [{ minor: 15, patch: 0 }, true], + [{ minor: 15, patch: 1 }, true], + [{ major: 8, minor: 0, patch: 0 }, true], + [{ minor: 14, patch: 1 }, false], + [{ minor: 10, patch: 0 }, false], + ])("reports %o as %s", async (version, expected) => { + const wallet = new KeepKeyHDWallet(makeMockTransport(version as any)); + expect(await wallet.supportsDiceEntropy()).toBe(expected); + }); + + it("reports false when Features carry no version fields", async () => { + const wallet = new KeepKeyHDWallet(makeMockTransport({ features: null })); + expect(await wallet.supportsDiceEntropy()).toBe(false); + }); +}); From be95702f162b9dee83c6044c9d2ba0b9fe666a84 Mon Sep 17 00:00:00 2001 From: highlander Date: Mon, 10 Aug 2026 18:49:59 -0600 Subject: [PATCH 6/6] style: prettier formatting in the dice-entropy tests --- packages/hdwallet-keepkey/src/reset-dice-entropy.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/hdwallet-keepkey/src/reset-dice-entropy.test.ts b/packages/hdwallet-keepkey/src/reset-dice-entropy.test.ts index 9fff1118..5d7be23c 100644 --- a/packages/hdwallet-keepkey/src/reset-dice-entropy.test.ts +++ b/packages/hdwallet-keepkey/src/reset-dice-entropy.test.ts @@ -31,7 +31,9 @@ import { KeepKeyHDWallet } from "./keepkey"; function makeMockTransport(opts: { major?: number; minor?: number; patch?: number; features?: null } = {}) { const { major = 7, minor = 15, patch = 0 } = opts; const featuresMessage = - opts.features === null ? { deviceId: "mock-device-id" } : { deviceId: "mock-device-id", majorVersion: major, minorVersion: minor, patchVersion: patch }; + opts.features === null + ? { deviceId: "mock-device-id" } + : { deviceId: "mock-device-id", majorVersion: major, minorVersion: minor, patchVersion: patch }; return { debugLink: false, @@ -133,9 +135,7 @@ describe("KeepKeyHDWallet.reset() refuses a silent dice downgrade", () => { it("names the offending firmware version in the error", async () => { const transport = makeMockTransport({ major: 7, minor: 14, patch: 1 }); - const err = await new KeepKeyHDWallet(transport) - .reset({ ...BASE_RESET, diceEntropy: true }) - .catch((e: any) => e); + const err = await new KeepKeyHDWallet(transport).reset({ ...BASE_RESET, diceEntropy: true }).catch((e: any) => e); expect(String(err.message)).toContain("v7.14.1"); });