From 954814002418bd3e1c9b671ff4fa199a8e63cdba Mon Sep 17 00:00:00 2001 From: highlander Date: Fri, 21 Aug 2026 16:30:01 -0500 Subject: [PATCH 1/9] fix(keepkey/eip712): say why the device refused, and fall back when it cannot parse Two defects that combine into a dead end for x402 payments on firmware 7.14.2. 1. The refusal text was thrown away. } catch (error) { console.error({ error }); throw new Error("Failed to sign typed ETH message"); } transport.call throws the raw failure event, so the device's own words are sitting at error.message.message -- and this replaced all of them with one string. "Enable AdvancedMode to blind-sign typed hashes", "Structured EIP-712 disabled pending canonical display hardening" and an unplugged cable were indistinguishable to the user, and the difference between them is the only part they can act on. Now: surface the firmware's text when there is one, rethrow real Errors unchanged (ActionCancelled keeps its identity), and fall back to the generic string only for a non-Error with nothing to say. 2. x402 hard-failed instead of degrading. isX402Eip3009() routes EIP-3009 TransferWithAuthorization to the structured endpoint. Firmware 7.14.2 withdrew that endpoint -- its JSON parser could not guarantee the displayed value was the value being hashed -- so the call is answered with a Failure and the payment simply died, reporting the generic message above. Now the structured attempt is caught, and ONLY a refusal that means "this device has no structured endpoint" falls through to the hashed path: Failure_UnexpectedMessage from firmware predating the message, or the "Structured EIP-712 disabled" text from firmware that has withdrawn it. Every other failure propagates, because masking a real error with a silent downgrade is how a signing bug becomes invisible. Detection is by ATTEMPT, not by version number. There is no capability bit for this, and a version table would need editing on every branch that toggles the flag. Retrying is safe because the firmware refuses at the top of the handler, before it touches session state. The fallback is not a silent loss of protection: the hashed path still shows the device's blind-sign warning and still requires AdvancedMode. It is the same treatment every other typed-data payload already gets on this firmware. --- packages/hdwallet-keepkey/src/ethereum.ts | 58 ++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/packages/hdwallet-keepkey/src/ethereum.ts b/packages/hdwallet-keepkey/src/ethereum.ts index 3ee43b75..5b69bf43 100644 --- a/packages/hdwallet-keepkey/src/ethereum.ts +++ b/packages/hdwallet-keepkey/src/ethereum.ts @@ -657,6 +657,45 @@ function isX402Eip3009(typedData: any): boolean { ); } +/** The firmware's own words when it refused. + * + * transport.call throws the raw failure event -- `{ message_enum: + * MESSAGETYPE_FAILURE, message: Failure.toObject() }` -- so the device's text + * is at `.message.message`. Every caller that flattens this to a generic string + * throws away the only explanation the user can act on. + */ +function firmwareFailureText(error: unknown): string | undefined { + if (!core.isIndexable(error)) return undefined; + if (error.message_enum !== Messages.MessageType.MESSAGETYPE_FAILURE) return undefined; + const failure = error.message as { message?: string } | undefined; + const text = failure?.message; + return typeof text === "string" && text.length > 0 ? text : undefined; +} + +/** Did the device refuse because it has no structured EIP-712 endpoint? + * + * Firmware 7.14.2 withdrew the structured path -- its JSON parser could not + * guarantee the displayed value was the value being hashed -- and answers + * Ethereum712TypesValues with "Structured EIP-712 disabled pending canonical + * display hardening". Firmware that predates the message answers + * Failure_UnexpectedMessage. + * + * Both mean the same thing to us: this device cannot parse typed data, so use + * the hashed path. Detecting it by ATTEMPT rather than by version number is + * deliberate -- there is no capability bit for this, and a version table would + * need updating for every branch that toggles the flag. + * + * Safe to retry after: the firmware refuses at the top of the handler, before + * it touches any session state, so nothing partial is left behind. + */ +function structuredEip712Unavailable(error: unknown): boolean { + if (!core.isIndexable(error)) return false; + if (error.message_enum !== Messages.MessageType.MESSAGETYPE_FAILURE) return false; + const failure = error.message as { code?: number; message?: string } | undefined; + if (failure?.code === Types.FailureType.FAILURE_UNEXPECTEDMESSAGE) return true; + return typeof failure?.message === "string" && failure.message.includes("Structured EIP-712 disabled"); +} + async function signStructuredEip712( transport: Transport, addressNList: number[], @@ -719,7 +758,17 @@ export async function ethSignTypedData( const { primaryType, domain, message } = typedData; if (isX402Eip3009(typedData)) { - return signStructuredEip712(transport, msg.addressNList, typedData); + try { + return await signStructuredEip712(transport, msg.addressNList, typedData); + } catch (e) { + // Anything other than "this device has no structured endpoint" is a + // real error and must not be masked by a silent downgrade. + if (!structuredEip712Unavailable(e)) throw e; + // Fall through to the hashed path. The user still sees the device's + // blind-sign warning and still has to have AdvancedMode on, so this + // is not a silent loss of protection -- it is the same treatment + // every other typed-data payload already gets on this firmware. + } } // eip-712 getStructHash is a 1:1 byte-identical replacement for // @metamask/eth-sig-util TypedDataUtils.hashStruct(..., V4) — verified across @@ -756,6 +805,13 @@ export async function ethSignTypedData( }); } catch (error) { console.error({ error }); + // Surface what the device actually said. "Failed to sign typed ETH message" + // is the same string whether the user needs to enable AdvancedMode, the + // firmware withdrew the structured endpoint, or the cable fell out -- and + // the one thing a user can act on is the difference between those. + const detail = firmwareFailureText(error); + if (detail) throw new Error(detail); + if (error instanceof Error) throw error; throw new Error("Failed to sign typed ETH message"); } } From 098555f6eb5ab81bc06e532c3630a9409acbcf7b Mon Sep 17 00:00:00 2001 From: highlander Date: Fri, 21 Aug 2026 16:46:38 -0500 Subject: [PATCH 2/9] feat(eip712): host half of device-driven structured signing The answering side of the streaming protocol: the device drives, this owns the document and serves whatever it addresses. Three parts, each independently tested (18 cases, all green): parseSolidityType -- "uint256", "bytes32", "Person[3]", "int16[2][][4]" into the wire description. array_levels is in WRITTEN order, left to right, because that is the order encodeType reproduces; reversing it silently changes typeHash and therefore the signature. It refuses a bare "uint"/"int". That is not canonical EIP-712, and the old firmware accepted it and hashed it as 256 bits -- producing a type string no compliant verifier reproduces. Refusing at the host means the device never sees it. encodeValue -- one leaf as the exact bytes the device will hash AND display. Raw big-endian at the declared width, never JSON. The device does no number parsing at all, which is what removes the old path's strtoll ceiling of 2^63-1 -- i.e. every unlimited ERC-20 approval ever issued, the most common permit there is. There is a test for exactly that value. The flip side is that width correctness is now entirely the host's job, so this is strict rather than lenient: out-of-range values throw, a JS number that has already lost precision throws rather than being silently converted into a number the caller never had, and a wrong-width address or bytesN throws. resolveMemberPath -- walks a device-supplied member_path into the document. path[0] is 0 for domain, 1 for message. A path stopping on an ARRAY is the device asking for its length; a path stopping on a STRUCT is a protocol error, because the device walks into structs rather than asking for them. Fixtures are Permit2's PermitSingle nesting PermitDetails -- the payload that started this, and the one a flat-structs-only design cannot sign. Note for anyone extending this: BigInt LITERALS (1n) are ES2020 syntax and this repo targets ES2016 with an es2020 lib, so the BigInt function is available but the literal form does not compile. Hence the ZERO/ONE/EIGHT constants. --- .../src/eip712Streaming.test.ts | 185 +++++++++++ .../hdwallet-keepkey/src/eip712Streaming.ts | 292 ++++++++++++++++++ 2 files changed, 477 insertions(+) create mode 100644 packages/hdwallet-keepkey/src/eip712Streaming.test.ts create mode 100644 packages/hdwallet-keepkey/src/eip712Streaming.ts diff --git a/packages/hdwallet-keepkey/src/eip712Streaming.test.ts b/packages/hdwallet-keepkey/src/eip712Streaming.test.ts new file mode 100644 index 00000000..b088a30e --- /dev/null +++ b/packages/hdwallet-keepkey/src/eip712Streaming.test.ts @@ -0,0 +1,185 @@ +import { + EthereumDataType, + encodeArrayLength, + encodeValue, + parseSolidityType, + resolveMemberPath, + structMembers, + TypedDataDoc, +} from "./eip712Streaming"; + +const hex = (b: Uint8Array) => Buffer.from(b).toString("hex"); + +describe("parseSolidityType", () => { + it("parses atomics with their widths in bytes", () => { + expect(parseSolidityType("uint256")).toEqual({ dataType: EthereumDataType.UINT, size: 32, arrayLevels: [] }); + expect(parseSolidityType("uint8")).toEqual({ dataType: EthereumDataType.UINT, size: 1, arrayLevels: [] }); + expect(parseSolidityType("int16")).toEqual({ dataType: EthereumDataType.INT, size: 2, arrayLevels: [] }); + expect(parseSolidityType("bytes32")).toEqual({ dataType: EthereumDataType.BYTES, size: 32, arrayLevels: [] }); + expect(parseSolidityType("bytes")).toEqual({ dataType: EthereumDataType.BYTES, arrayLevels: [] }); + expect(parseSolidityType("string")).toEqual({ dataType: EthereumDataType.STRING, arrayLevels: [] }); + expect(parseSolidityType("bool")).toEqual({ dataType: EthereumDataType.BOOL, arrayLevels: [] }); + expect(parseSolidityType("address")).toEqual({ dataType: EthereumDataType.ADDRESS, arrayLevels: [] }); + }); + + it("records array levels in WRITTEN order", () => { + // The order encodeType reproduces. Reversing it silently changes typeHash. + expect(parseSolidityType("int16[2][][4]").arrayLevels).toEqual([2, 0, 4]); + expect(parseSolidityType("address[]").arrayLevels).toEqual([0]); + expect(parseSolidityType("Person[3]")).toEqual({ + dataType: EthereumDataType.STRUCT, + structName: "Person", + arrayLevels: [3], + }); + }); + + it("refuses a width-less integer", () => { + // Not canonical EIP-712. The old firmware accepted it and hashed it as 256 + // bits, producing a type string no verifier reproduces. + expect(() => parseSolidityType("uint")).toThrow(/width/); + expect(() => parseSolidityType("int")).toThrow(/width/); + }); + + it("refuses malformed types rather than guessing", () => { + expect(() => parseSolidityType("uint257")).toThrow(); + expect(() => parseSolidityType("uint255")).toThrow(); // not a multiple of 8 + expect(() => parseSolidityType("bytes33")).toThrow(); + expect(() => parseSolidityType("uint256[2]x")).toThrow(/Malformed/); + }); +}); + +describe("encodeValue", () => { + it("encodes an unlimited approval, which the old path refused", () => { + // strtoll capped the previous implementation at 2^63-1, so every unlimited + // ERC-20 approval was unsignable. This is the exact value. + const max = (BigInt(1) << BigInt(256)) - BigInt(1); + const out = encodeValue(parseSolidityType("uint256"), max.toString()); + expect(hex(out)).toEqual("f".repeat(64)); + }); + + it("encodes negative ints in two's complement at the declared width", () => { + expect(hex(encodeValue(parseSolidityType("int16"), -2))).toEqual("fffe"); + expect(hex(encodeValue(parseSolidityType("int16"), 2))).toEqual("0002"); + }); + + it("rejects values that do not fit the declared width", () => { + expect(() => encodeValue(parseSolidityType("uint8"), 256)).toThrow(/out of range/); + expect(() => encodeValue(parseSolidityType("uint8"), -1)).toThrow(/Negative/); + expect(() => encodeValue(parseSolidityType("int8"), 128)).toThrow(/out of range/); + }); + + it("rejects a number that has already lost precision", () => { + // Converting it would sign a value the caller never had. + expect(() => encodeValue(parseSolidityType("uint256"), 2 ** 53)).toThrow(/safe integer/); + }); + + it("encodes address, bool, bytesN and string exactly", () => { + expect(hex(encodeValue(parseSolidityType("address"), "0x" + "11".repeat(20)))).toEqual("11".repeat(20)); + expect(hex(encodeValue(parseSolidityType("bool"), true))).toEqual("01"); + expect(hex(encodeValue(parseSolidityType("bytes4"), "0xdeadbeef"))).toEqual("deadbeef"); + expect(hex(encodeValue(parseSolidityType("string"), "abc"))).toEqual("616263"); + }); + + it("rejects a wrong-width address or bytesN", () => { + expect(() => encodeValue(parseSolidityType("address"), "0x1234")).toThrow(/20 bytes/); + expect(() => encodeValue(parseSolidityType("bytes4"), "0xdead")).toThrow(/4 bytes/); + }); +}); + +describe("encodeArrayLength", () => { + it("is a big-endian uint16", () => { + expect(hex(encodeArrayLength(0))).toEqual("0000"); + expect(hex(encodeArrayLength(258))).toEqual("0102"); + expect(() => encodeArrayLength(0x10000)).toThrow(); + }); +}); + +// The payload that started all of this: PermitSingle nests PermitDetails, so a +// flat-structs-only implementation cannot sign a Uniswap approval. +const PERMIT2: TypedDataDoc = { + types: { + EIP712Domain: [ + { name: "name", type: "string" }, + { name: "chainId", type: "uint256" }, + { name: "verifyingContract", type: "address" }, + ], + PermitDetails: [ + { name: "token", type: "address" }, + { name: "amount", type: "uint160" }, + { name: "expiration", type: "uint48" }, + { name: "nonce", type: "uint48" }, + ], + PermitSingle: [ + { name: "details", type: "PermitDetails" }, + { name: "spender", type: "address" }, + { name: "sigDeadline", type: "uint256" }, + ], + }, + primaryType: "PermitSingle", + domain: { name: "Permit2", chainId: 1, verifyingContract: "0x" + "22".repeat(20) }, + message: { + details: { + token: "0x" + "33".repeat(20), + amount: "1461501637330902918203684832716283019655932542975", // 2^160-1 + expiration: 1700000000, + nonce: 0, + }, + spender: "0x" + "44".repeat(20), + sigDeadline: 1700000000, + }, +}; + +describe("resolveMemberPath", () => { + it("resolves domain leaves", () => { + const r = resolveMemberPath(PERMIT2, [0, 0]); + expect(r).toEqual({ kind: "value", field: parseSolidityType("string"), value: "Permit2" }); + }); + + it("walks into a nested struct", () => { + // [1, 0, 1] = message -> details -> amount + const r = resolveMemberPath(PERMIT2, [1, 0, 1]); + expect(r.kind).toEqual("value"); + if (r.kind === "value") { + expect(r.field.size).toEqual(20); // uint160 + expect(hex(encodeValue(r.field, r.value))).toEqual("ff".repeat(20)); + } + }); + + it("refuses to hand back a struct as a value", () => { + // The device walks into structs; asking for one is a protocol error. + expect(() => resolveMemberPath(PERMIT2, [1, 0])).toThrow(/walk into it/); + }); + + it("returns a length when the path stops on an array", () => { + const doc: TypedDataDoc = { + types: { + EIP712Domain: [{ name: "name", type: "string" }], + Batch: [{ name: "owners", type: "address[]" }], + }, + primaryType: "Batch", + domain: { name: "B" }, + message: { owners: ["0x" + "aa".repeat(20), "0x" + "bb".repeat(20)] }, + }; + expect(resolveMemberPath(doc, [1, 0])).toEqual({ kind: "arrayLength", length: 2 }); + const el = resolveMemberPath(doc, [1, 0, 1]); + expect(el.kind).toEqual("value"); + if (el.kind === "value") expect(hex(encodeValue(el.field, el.value))).toEqual("bb".repeat(20)); + }); + + it("rejects an out-of-range index rather than signing undefined", () => { + expect(() => resolveMemberPath(PERMIT2, [1, 99])).toThrow(/out of range/); + expect(() => resolveMemberPath(PERMIT2, [2])).toThrow(/root/); + }); +}); + +describe("structMembers", () => { + it("returns members in declaration order, which is signature-relevant", () => { + const m = structMembers(PERMIT2, "PermitDetails"); + expect(m.map((x) => x.name)).toEqual(["token", "amount", "expiration", "nonce"]); + expect(m[1].type).toEqual({ dataType: EthereumDataType.UINT, size: 20, arrayLevels: [] }); + }); + + it("throws on an unknown struct", () => { + expect(() => structMembers(PERMIT2, "Nope")).toThrow(/Unknown struct/); + }); +}); diff --git a/packages/hdwallet-keepkey/src/eip712Streaming.ts b/packages/hdwallet-keepkey/src/eip712Streaming.ts new file mode 100644 index 00000000..5a78f518 --- /dev/null +++ b/packages/hdwallet-keepkey/src/eip712Streaming.ts @@ -0,0 +1,292 @@ +/** + * Structured EIP-712 over the device-driven streaming protocol. + * + * The device drives. It asks for one struct definition, or one leaf VALUE, at + * a time, and hashes each value in the same pass that displays it. This module + * is the answering half: it owns the document and serves whatever the device + * addresses. + * + * Two things here are load-bearing and easy to get subtly wrong. + * + * 1. TYPE SPELLING. `array_levels` lists bracket groups in the order they are + * WRITTEN, left to right, because that is the order encodeType reproduces. + * `int16[2][][4]` is [2, 0, 4], not the reverse. A wrong order silently + * changes typeHash and therefore the signature. + * + * 2. VALUE WIDTH. Values go out as raw big-endian bytes of exactly the declared + * width -- no JSON, no decimals. The device does no number parsing at all, + * which is what removes the old path's 2^63-1 ceiling (every unlimited + * approval) and any chance of the host and device disagreeing about what a + * decimal string meant. The flip side is that getting the width right is + * now entirely the host's job, so encodeValue is strict rather than lenient. + */ + +/** Mirrors EthereumTypedDataStructAck.EthereumDataType. */ +export enum EthereumDataType { + UINT = 1, + INT = 2, + BYTES = 3, + STRING = 4, + BOOL = 5, + ADDRESS = 6, + ARRAY = 7, // reserved, never sent -- dimensions live in arrayLevels + STRUCT = 8, +} + +export interface FieldType { + dataType: EthereumDataType; + /** bytesN: N. intN/uintN: N in BYTES (so uint256 is 32). */ + size?: number; + structName?: string; + /** Written order, left to right. 0 means a dynamic dimension. */ + arrayLevels: number[]; +} + +const ARRAY_SUFFIX = /\[(\d*)\]/g; + +// BigInt literals (1n) are ES2020 SYNTAX and this repo targets ES2016; the +// BigInt function is available because the lib is es2020. Hence constants. +const ZERO = BigInt(0); +const ONE = BigInt(1); +const EIGHT = BigInt(8); +const BYTE_MASK = BigInt(0xff); + +/** + * "uint256", "bytes32", "Person[3]", "int16[2][][4]" -> FieldType. + * Throws rather than guessing: an unparseable type must not become a signature. + */ +export function parseSolidityType(type: string): FieldType { + const bracket = type.indexOf("["); + const base = bracket === -1 ? type : type.slice(0, bracket); + const suffix = bracket === -1 ? "" : type.slice(bracket); + + const arrayLevels: number[] = []; + if (suffix) { + // Every bracket group must be well formed; anything left over is a type we + // do not understand, and signing something we do not understand is the + // whole failure mode. + let consumed = 0; + ARRAY_SUFFIX.lastIndex = 0; + let m: RegExpExecArray | null; + while ((m = ARRAY_SUFFIX.exec(suffix)) !== null) { + if (m.index !== consumed) throw new Error(`Malformed array type: ${type}`); + arrayLevels.push(m[1] === "" ? 0 : Number(m[1])); + consumed = ARRAY_SUFFIX.lastIndex; + } + if (consumed !== suffix.length) throw new Error(`Malformed array type: ${type}`); + } + + if (base === "string") return { dataType: EthereumDataType.STRING, arrayLevels }; + if (base === "bool") return { dataType: EthereumDataType.BOOL, arrayLevels }; + if (base === "address") return { dataType: EthereumDataType.ADDRESS, arrayLevels }; + if (base === "bytes") return { dataType: EthereumDataType.BYTES, arrayLevels }; + + const bytesN = /^bytes(\d+)$/.exec(base); + if (bytesN) { + const n = Number(bytesN[1]); + if (n < 1 || n > 32) throw new Error(`Invalid fixed bytes width: ${base}`); + return { dataType: EthereumDataType.BYTES, size: n, arrayLevels }; + } + + const intN = /^(u?)int(\d*)$/.exec(base); + if (intN) { + // A bare "uint"/"int" is not canonical EIP-712. The old firmware accepted + // it and hashed it verbatim as 256 bits, which produced a type string no + // verifier reproduces. Refuse it here rather than pass it on. + if (intN[2] === "") throw new Error(`Integer type must state its width: ${base}`); + const bits = Number(intN[2]); + if (bits < 8 || bits > 256 || bits % 8 !== 0) { + throw new Error(`Invalid integer width: ${base}`); + } + return { + dataType: intN[1] === "u" ? EthereumDataType.UINT : EthereumDataType.INT, + size: bits / 8, + arrayLevels, + }; + } + + // Anything else names a struct the device will ask us to define. + if (!/^[A-Za-z_$][A-Za-z0-9_$]*$/.test(base)) { + throw new Error(`Unparseable EIP-712 type: ${type}`); + } + return { dataType: EthereumDataType.STRUCT, structName: base, arrayLevels }; +} + +function hexToBytes(hex: string, what: string): Uint8Array { + const h = hex.startsWith("0x") || hex.startsWith("0X") ? hex.slice(2) : hex; + if (h.length % 2 !== 0 || !/^[0-9a-fA-F]*$/.test(h)) { + throw new Error(`${what} is not valid hex: ${hex}`); + } + const out = new Uint8Array(h.length / 2); + for (let i = 0; i < out.length; i++) out[i] = parseInt(h.substr(i * 2, 2), 16); + return out; +} + +/** Big-endian two's-complement, exactly `width` bytes. Throws on overflow. */ +function bigIntToBytes(value: bigint, width: number, signed: boolean): Uint8Array { + const bits = BigInt(width * 8); + if (signed) { + const min = -(ONE << (bits - ONE)); + const max = (ONE << (bits - ONE)) - ONE; + if (value < min || value > max) throw new Error(`Value out of range for int${width * 8}`); + if (value < ZERO) value += ONE << bits; + } else { + if (value < ZERO) throw new Error(`Negative value for uint${width * 8}`); + if (value >= ONE << bits) throw new Error(`Value out of range for uint${width * 8}`); + } + const out = new Uint8Array(width); + for (let i = width - 1; i >= 0; i--) { + out[i] = Number(value & BYTE_MASK); + value >>= EIGHT; + } + return out; +} + +function toBigInt(value: unknown, what: string): bigint { + if (typeof value === "bigint") return value; + if (typeof value === "number") { + if (!Number.isSafeInteger(value)) { + // A float or an unsafe integer has already lost precision by the time it + // reaches us; converting it would sign a number the caller never had. + throw new Error(`${what} is not a safe integer: ${value}`); + } + return BigInt(value); + } + if (typeof value === "string") { + const s = value.trim(); + if (/^-?\d+$/.test(s)) return BigInt(s); + if (/^0x[0-9a-fA-F]+$/i.test(s)) return BigInt(s); + throw new Error(`${what} is not an integer: ${value}`); + } + throw new Error(`${what} is not an integer: ${String(value)}`); +} + +/** One leaf, as the exact bytes the device will hash and display. */ +export function encodeValue(field: FieldType, value: unknown): Uint8Array { + switch (field.dataType) { + case EthereumDataType.UINT: + case EthereumDataType.INT: { + if (field.size === undefined) throw new Error("Integer field has no width"); + return bigIntToBytes(toBigInt(value, "Integer field"), field.size, field.dataType === EthereumDataType.INT); + } + case EthereumDataType.BOOL: { + if (typeof value !== "boolean") throw new Error(`Not a boolean: ${String(value)}`); + return new Uint8Array([value ? 1 : 0]); + } + case EthereumDataType.ADDRESS: { + if (typeof value !== "string") throw new Error("Address must be a string"); + const b = hexToBytes(value, "Address"); + if (b.length !== 20) throw new Error(`Address must be 20 bytes, got ${b.length}`); + return b; + } + case EthereumDataType.BYTES: { + if (typeof value !== "string" && !(value instanceof Uint8Array)) { + throw new Error("bytes must be hex or Uint8Array"); + } + const b = value instanceof Uint8Array ? value : hexToBytes(value, "bytes"); + if (field.size !== undefined && b.length !== field.size) { + throw new Error(`bytes${field.size} must be ${field.size} bytes, got ${b.length}`); + } + return b; + } + case EthereumDataType.STRING: { + if (typeof value !== "string") throw new Error("string field must be a string"); + return new TextEncoder().encode(value); + } + default: + throw new Error(`Cannot encode a ${EthereumDataType[field.dataType]} as a leaf value`); + } +} + +/** Big-endian uint16, the wire form of an array length. */ +export function encodeArrayLength(len: number): Uint8Array { + if (!Number.isInteger(len) || len < 0 || len > 0xffff) { + throw new Error(`Array length out of range: ${len}`); + } + return new Uint8Array([(len >> 8) & 0xff, len & 0xff]); +} + +export interface TypedDataDoc { + types: Record>; + primaryType: string; + domain: Record; + message: Record; +} + +export type Resolved = { kind: "value"; field: FieldType; value: unknown } | { kind: "arrayLength"; length: number }; + +/** + * Resolve a device-supplied member_path against the document. + * + * path[0] selects the root: 0 = domain, 1 = message. Every index after that + * addresses either a member of the struct we are standing in, or an element of + * the array we are standing in. + * + * A path that stops on an ARRAY is the device asking for its LENGTH; it will + * ask for the elements next. A path that stops on a STRUCT is a protocol + * error -- the device walks into structs, it never asks for one as a value. + */ +export function resolveMemberPath(doc: TypedDataDoc, path: number[]): Resolved { + if (path.length === 0) throw new Error("Empty member_path"); + + const root = path[0]; + if (root !== 0 && root !== 1) throw new Error(`Unknown member_path root: ${root}`); + const rootType = root === 0 ? "EIP712Domain" : doc.primaryType; + + let field: FieldType = { + dataType: EthereumDataType.STRUCT, + structName: rootType, + arrayLevels: [], + }; + let value: unknown = root === 0 ? doc.domain : doc.message; + // How many of `field.arrayLevels` we have already indexed through. + let levelsUsed = 0; + + for (let i = 1; i < path.length; i++) { + const index = path[i]; + + if (levelsUsed < field.arrayLevels.length) { + // Standing in an array: step into an element. + if (!Array.isArray(value)) throw new Error(`Expected an array at path ${path.slice(0, i).join(".")}`); + if (index >= value.length) throw new Error(`Array index ${index} out of range`); + value = value[index]; + levelsUsed++; + continue; + } + + if (field.dataType !== EthereumDataType.STRUCT) { + throw new Error( + `Cannot descend into a ${EthereumDataType[field.dataType]} at path ${path.slice(0, i).join(".")}` + ); + } + + const structName = field.structName!; + const members = doc.types[structName]; + if (!members) throw new Error(`Unknown struct: ${structName}`); + if (index >= members.length) throw new Error(`Member index ${index} out of range for ${structName}`); + + const member = members[index]; + field = parseSolidityType(member.type); + levelsUsed = 0; + if (typeof value !== "object" || value === null) { + throw new Error(`Expected an object at path ${path.slice(0, i).join(".")}`); + } + value = (value as Record)[member.name]; + } + + if (levelsUsed < field.arrayLevels.length) { + if (!Array.isArray(value)) throw new Error("Expected an array for a length request"); + return { kind: "arrayLength", length: value.length }; + } + if (field.dataType === EthereumDataType.STRUCT) { + throw new Error("Device asked for a struct as a value; it should walk into it"); + } + return { kind: "value", field, value }; +} + +/** The member list for one struct, in the shape EthereumTypedDataStructAck wants. */ +export function structMembers(doc: TypedDataDoc, name: string): Array<{ name: string; type: FieldType }> { + const members = doc.types[name]; + if (!members) throw new Error(`Unknown struct: ${name}`); + return members.map((m) => ({ name: m.name, type: parseSolidityType(m.type) })); +} From 5494321cbc01e46ea05a3b6a8153e40404903a0c Mon Sep 17 00:00:00 2001 From: highlander Date: Fri, 21 Aug 2026 17:11:55 -0500 Subject: [PATCH 3/9] fix(eip712): five defects an adversarial review found in the host encoder All five were in code written today, all five confirmed by independent verification, and four of them sign the wrong thing rather than merely failing. 1. A fixed array dimension of ZERO was conflated with a dynamic one. "uint256[0]" parsed to arrayLevels [0], byte-identical to "uint256[]" -- and 0 is the wire's dynamic sentinel, so the device spells it back as "[]". Confirmed against ethers 5.7.2: Foo(uint256[0] a) and Foo(uint256[] a) have different hashStructs. Leading zeros re-spelled the same way. Both refused. 2. Non-canonical integer widths were silently NORMALISED. "uint0256" became uint256 and was hashed as "uint256", while a verifier reading the document sees "uint0256". Same failure as the bare "uint" this module already refused, one spelling further on. Same for "bytes032". The integer regex is now anchored to digits, so a struct legitimately named "interest" is not caught by it. 3. and 4. A declared fixed dimension was never checked against the document, in either the element branch or the length branch. address[2] carrying three elements reported length 3 and served all three. The dimension is part of the type string and therefore part of typeHash, so this signs a document whose type says two -- and the device cannot notice, because the only count it ever sees is the one we give it. 5. Dynamic bytes and string had no length cap, while EthereumTypedDataValueAck.value is max_size:1024. A 2000-byte string encoded fine and then could not be sent, so the ceremony died at the transport layer where the error cannot name the field. 30 tests, all green. Every case above has a regression test that fails against the previous version. --- .../src/eip712Streaming.test.ts | 99 +++++++++++++++++++ .../hdwallet-keepkey/src/eip712Streaming.ts | 53 +++++++++- 2 files changed, 147 insertions(+), 5 deletions(-) diff --git a/packages/hdwallet-keepkey/src/eip712Streaming.test.ts b/packages/hdwallet-keepkey/src/eip712Streaming.test.ts index b088a30e..68d3c2ba 100644 --- a/packages/hdwallet-keepkey/src/eip712Streaming.test.ts +++ b/packages/hdwallet-keepkey/src/eip712Streaming.test.ts @@ -183,3 +183,102 @@ describe("structMembers", () => { expect(() => structMembers(PERMIT2, "Nope")).toThrow(/Unknown struct/); }); }); + +// ── regressions from the adversarial review ───────────────────────── +// Every case below was a confirmed defect in the first version of this module. + +describe("non-canonical type spellings are refused, not normalised", () => { + it("refuses a fixed array dimension of zero", () => { + // 0 is the wire's DYNAMIC sentinel, so uint256[0] was being hashed as + // uint256[] -- a different type string from the document's. Confirmed + // against ethers 5.7.2: Foo(uint256[0] a) and Foo(uint256[] a) have + // different hashStructs. + expect(() => parseSolidityType("uint256[0]")).toThrow(/Malformed array dimension/); + }); + + it("refuses leading zeros in an array dimension", () => { + expect(() => parseSolidityType("uint256[01]")).toThrow(/Malformed array dimension/); + }); + + it("refuses a non-canonical integer width", () => { + // uint0256 normalised to 256 and was hashed as "uint256", while a verifier + // reading the document sees "uint0256". + expect(() => parseSolidityType("uint0256")).toThrow(/Non-canonical/); + expect(() => parseSolidityType("int008")).toThrow(/Non-canonical/); + }); + + it("refuses a non-canonical bytesN width", () => { + expect(() => parseSolidityType("bytes032")).toThrow(/Non-canonical/); + }); + + it("still accepts a struct whose name merely starts with int", () => { + // The integer regex is anchored to digits so this is not caught by it. + expect(parseSolidityType("interest")).toEqual({ + dataType: EthereumDataType.STRUCT, + structName: "interest", + arrayLevels: [], + }); + }); +}); + +describe("a declared fixed dimension is enforced", () => { + const fixedDoc = (owners: string[]): TypedDataDoc => ({ + types: { + EIP712Domain: [{ name: "name", type: "string" }], + Batch: [{ name: "owners", type: "address[2]" }], + }, + primaryType: "Batch", + domain: { name: "B" }, + message: { owners }, + }); + + it("refuses a document whose array is longer than its type says", () => { + // The dimension is part of the type string and therefore part of typeHash. + // Serving three elements for an address[2] signs a document whose type + // declares two, and the device cannot notice -- it only sees the count we + // give it. + const doc = fixedDoc(["0x" + "aa".repeat(20), "0x" + "bb".repeat(20), "0x" + "cc".repeat(20)]); + expect(() => resolveMemberPath(doc, [1, 0])).toThrow(/declares 2 elements, document has 3/); + expect(() => resolveMemberPath(doc, [1, 0, 2])).toThrow(/declares 2 elements/); + }); + + it("refuses a document whose array is shorter than its type says", () => { + const doc = fixedDoc(["0x" + "aa".repeat(20)]); + expect(() => resolveMemberPath(doc, [1, 0])).toThrow(/declares 2 elements, document has 1/); + }); + + it("accepts the declared length", () => { + const doc = fixedDoc(["0x" + "aa".repeat(20), "0x" + "bb".repeat(20)]); + expect(resolveMemberPath(doc, [1, 0])).toEqual({ kind: "arrayLength", length: 2 }); + }); + + it("leaves dynamic dimensions unchecked", () => { + const doc: TypedDataDoc = { + types: { + EIP712Domain: [{ name: "name", type: "string" }], + Batch: [{ name: "owners", type: "address[]" }], + }, + primaryType: "Batch", + domain: { name: "B" }, + message: { owners: ["0x" + "aa".repeat(20)] }, + }; + expect(resolveMemberPath(doc, [1, 0])).toEqual({ kind: "arrayLength", length: 1 }); + }); +}); + +describe("dynamic leaves are capped at the wire limit", () => { + it("refuses a string past MAX_LEAF_BYTES instead of building an unsendable value", () => { + // EthereumTypedDataValueAck.value is max_size:1024. Encoding 2000 bytes + // produced something that could not be sent, and the ceremony died at the + // transport layer where the error could not name the field. + expect(() => encodeValue(parseSolidityType("string"), "x".repeat(2000))).toThrow(/over the 1024-byte wire limit/); + }); + + it("refuses oversized dynamic bytes", () => { + expect(() => encodeValue(parseSolidityType("bytes"), "0x" + "ab".repeat(2000))).toThrow(/over the 1024-byte wire limit/); + }); + + it("accepts exactly the limit", () => { + expect(encodeValue(parseSolidityType("string"), "x".repeat(1024)).length).toEqual(1024); + }); +}); diff --git a/packages/hdwallet-keepkey/src/eip712Streaming.ts b/packages/hdwallet-keepkey/src/eip712Streaming.ts index 5a78f518..beb4d353 100644 --- a/packages/hdwallet-keepkey/src/eip712Streaming.ts +++ b/packages/hdwallet-keepkey/src/eip712Streaming.ts @@ -51,6 +51,12 @@ const ONE = BigInt(1); const EIGHT = BigInt(8); const BYTE_MASK = BigInt(0xff); +/** EthereumTypedDataValueAck.value max_size in messages-ethereum.options, and + * EIP712_MAX_LEAF on the device. Encoding past it produces a value that + * cannot be put on the wire, so catch it here where the error can name the + * field rather than at the transport layer where it cannot. */ +export const MAX_LEAF_BYTES = 1024; + /** * "uint256", "bytes32", "Person[3]", "int16[2][][4]" -> FieldType. * Throws rather than guessing: an unparseable type must not become a signature. @@ -70,7 +76,18 @@ export function parseSolidityType(type: string): FieldType { let m: RegExpExecArray | null; while ((m = ARRAY_SUFFIX.exec(suffix)) !== null) { if (m.index !== consumed) throw new Error(`Malformed array type: ${type}`); - arrayLevels.push(m[1] === "" ? 0 : Number(m[1])); + if (m[1] === "") { + arrayLevels.push(0); // dynamic + } else { + // 0 is the wire's DYNAMIC sentinel: the device spells array_levels[i] + // == 0 as "[]". A fixed dimension of 0 therefore has no spelling of its + // own, and "uint256[0]" would be hashed as "uint256[]" -- a different + // type string from the one the document declares. Leading zeros + // re-spell the same way ("[01]" -> "[1]"). Neither is a legal EIP-712 + // type, so refuse rather than silently rewrite. + if (!/^[1-9][0-9]*$/.test(m[1])) throw new Error(`Malformed array dimension: ${type}`); + arrayLevels.push(Number(m[1])); + } consumed = ARRAY_SUFFIX.lastIndex; } if (consumed !== suffix.length) throw new Error(`Malformed array type: ${type}`); @@ -81,19 +98,26 @@ export function parseSolidityType(type: string): FieldType { if (base === "address") return { dataType: EthereumDataType.ADDRESS, arrayLevels }; if (base === "bytes") return { dataType: EthereumDataType.BYTES, arrayLevels }; - const bytesN = /^bytes(\d+)$/.exec(base); + const bytesN = /^bytes([0-9]*)$/.exec(base); if (bytesN) { + // "bytes032" parses to 32 and would be re-spelled "bytes32" -- a + // different type string from the document's. + if (!/^[1-9][0-9]*$/.test(bytesN[1])) throw new Error(`Non-canonical bytes width: ${base}`); const n = Number(bytesN[1]); if (n < 1 || n > 32) throw new Error(`Invalid fixed bytes width: ${base}`); return { dataType: EthereumDataType.BYTES, size: n, arrayLevels }; } - const intN = /^(u?)int(\d*)$/.exec(base); + const intN = /^(u?)int([0-9]*)$/.exec(base); if (intN) { // A bare "uint"/"int" is not canonical EIP-712. The old firmware accepted // it and hashed it verbatim as 256 bits, which produced a type string no // verifier reproduces. Refuse it here rather than pass it on. if (intN[2] === "") throw new Error(`Integer type must state its width: ${base}`); + // Nor is "uint0256" canonical. It would normalise to 256 here and be + // hashed as "uint256" by the device, while the document a verifier reads + // says "uint0256". Same failure as the bare form, one spelling further on. + if (!/^[1-9][0-9]*$/.test(intN[2])) throw new Error(`Non-canonical integer width: ${base}`); const bits = Number(intN[2]); if (bits < 8 || bits > 256 || bits % 8 !== 0) { throw new Error(`Invalid integer width: ${base}`); @@ -161,6 +185,13 @@ function toBigInt(value: unknown, what: string): bigint { throw new Error(`${what} is not an integer: ${String(value)}`); } +function capLeaf(b: Uint8Array, what: string): Uint8Array { + if (b.length > MAX_LEAF_BYTES) { + throw new Error(`${what} value is ${b.length} bytes, over the ${MAX_LEAF_BYTES}-byte wire limit`); + } + return b; +} + /** One leaf, as the exact bytes the device will hash and display. */ export function encodeValue(field: FieldType, value: unknown): Uint8Array { switch (field.dataType) { @@ -187,11 +218,11 @@ export function encodeValue(field: FieldType, value: unknown): Uint8Array { if (field.size !== undefined && b.length !== field.size) { throw new Error(`bytes${field.size} must be ${field.size} bytes, got ${b.length}`); } - return b; + return field.size === undefined ? capLeaf(b, "bytes") : b; } case EthereumDataType.STRING: { if (typeof value !== "string") throw new Error("string field must be a string"); - return new TextEncoder().encode(value); + return capLeaf(new TextEncoder().encode(value), "string"); } default: throw new Error(`Cannot encode a ${EthereumDataType[field.dataType]} as a leaf value`); @@ -247,7 +278,15 @@ export function resolveMemberPath(doc: TypedDataDoc, path: number[]): Resolved { if (levelsUsed < field.arrayLevels.length) { // Standing in an array: step into an element. + const declared = field.arrayLevels[levelsUsed]; if (!Array.isArray(value)) throw new Error(`Expected an array at path ${path.slice(0, i).join(".")}`); + // A declared dimension is part of the TYPE STRING and therefore part of + // typeHash. Serving three elements for an address[2] signs a document + // whose type says two -- the device cannot notice, because it only ever + // sees the count we give it. + if (declared !== 0 && value.length !== declared) { + throw new Error(`Fixed array declares ${declared} elements, document has ${value.length}`); + } if (index >= value.length) throw new Error(`Array index ${index} out of range`); value = value[index]; levelsUsed++; @@ -275,7 +314,11 @@ export function resolveMemberPath(doc: TypedDataDoc, path: number[]): Resolved { } if (levelsUsed < field.arrayLevels.length) { + const declared = field.arrayLevels[levelsUsed]; if (!Array.isArray(value)) throw new Error("Expected an array for a length request"); + if (declared !== 0 && value.length !== declared) { + throw new Error(`Fixed array declares ${declared} elements, document has ${value.length}`); + } return { kind: "arrayLength", length: value.length }; } if (field.dataType === EthereumDataType.STRUCT) { From 4d16202a5d3f6de27a2a77b6b7357fbc64b60695 Mon Sep 17 00:00:00 2001 From: highlander Date: Fri, 21 Aug 2026 17:18:21 -0500 Subject: [PATCH 4/9] style(eip712): unconditional assertions, sorted imports eslint jest/no-conditional-expect was right to object. The union returned by resolveMemberPath was being narrowed with an if, so the assertions sat inside a branch -- and a guarded expect that never runs proves nothing while reading as if it did. asValue() narrows by throwing instead, which fails loudly on the wrong variant and keeps every expect on the main path. Plus import sort and one prettier wrap. --- .../src/eip712Streaming.test.ts | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/packages/hdwallet-keepkey/src/eip712Streaming.test.ts b/packages/hdwallet-keepkey/src/eip712Streaming.test.ts index 68d3c2ba..b111c83f 100644 --- a/packages/hdwallet-keepkey/src/eip712Streaming.test.ts +++ b/packages/hdwallet-keepkey/src/eip712Streaming.test.ts @@ -1,8 +1,10 @@ import { - EthereumDataType, encodeArrayLength, encodeValue, + EthereumDataType, + FieldType, parseSolidityType, + Resolved, resolveMemberPath, structMembers, TypedDataDoc, @@ -10,6 +12,14 @@ import { const hex = (b: Uint8Array) => Buffer.from(b).toString("hex"); +/** Narrow a Resolved to its value form, failing loudly if it is not one. + * Keeps assertions unconditional -- a guarded expect that never runs proves + * nothing and reads as if it did. */ +function asValue(r: Resolved): { field: FieldType; value: unknown } { + if (r.kind !== "value") throw new Error(`expected a leaf value, got ${r.kind}`); + return { field: r.field, value: r.value }; +} + describe("parseSolidityType", () => { it("parses atomics with their widths in bytes", () => { expect(parseSolidityType("uint256")).toEqual({ dataType: EthereumDataType.UINT, size: 32, arrayLevels: [] }); @@ -137,12 +147,9 @@ describe("resolveMemberPath", () => { it("walks into a nested struct", () => { // [1, 0, 1] = message -> details -> amount - const r = resolveMemberPath(PERMIT2, [1, 0, 1]); - expect(r.kind).toEqual("value"); - if (r.kind === "value") { - expect(r.field.size).toEqual(20); // uint160 - expect(hex(encodeValue(r.field, r.value))).toEqual("ff".repeat(20)); - } + const r = asValue(resolveMemberPath(PERMIT2, [1, 0, 1])); + expect(r.field.size).toEqual(20); // uint160 + expect(hex(encodeValue(r.field, r.value))).toEqual("ff".repeat(20)); }); it("refuses to hand back a struct as a value", () => { @@ -161,9 +168,8 @@ describe("resolveMemberPath", () => { message: { owners: ["0x" + "aa".repeat(20), "0x" + "bb".repeat(20)] }, }; expect(resolveMemberPath(doc, [1, 0])).toEqual({ kind: "arrayLength", length: 2 }); - const el = resolveMemberPath(doc, [1, 0, 1]); - expect(el.kind).toEqual("value"); - if (el.kind === "value") expect(hex(encodeValue(el.field, el.value))).toEqual("bb".repeat(20)); + const el = asValue(resolveMemberPath(doc, [1, 0, 1])); + expect(hex(encodeValue(el.field, el.value))).toEqual("bb".repeat(20)); }); it("rejects an out-of-range index rather than signing undefined", () => { @@ -275,7 +281,9 @@ describe("dynamic leaves are capped at the wire limit", () => { }); it("refuses oversized dynamic bytes", () => { - expect(() => encodeValue(parseSolidityType("bytes"), "0x" + "ab".repeat(2000))).toThrow(/over the 1024-byte wire limit/); + expect(() => encodeValue(parseSolidityType("bytes"), "0x" + "ab".repeat(2000))).toThrow( + /over the 1024-byte wire limit/ + ); }); it("accepts exactly the limit", () => { From 63fc16d3b2177267a67d46036bff0d052ff28c42 Mon Sep 17 00:00:00 2001 From: highlander Date: Fri, 21 Aug 2026 17:36:47 -0500 Subject: [PATCH 5/9] feat(eip712): the host transport loop, and the wire messages it needs The answering half of the walk. The DEVICE leads -- it asks for one struct definition or one leaf value at a time, and runEip712Walk answers until a signature comes back. The host never chooses the order, and that is the property, not an accident of the API: the device hashes what it displays, in the order it picked, so a host that answered a different question would produce a digest that does not verify. `call` is injected rather than taking a Transport, so the loop is testable against a scripted device with no USB. Five cases, modelling the exact sequence the firmware state machine emits for Permit2 PermitSingle -- domain first, then the message, walking into PermitDetails: - every struct and value answered in order, and the nested uint160 comes back as exactly 20 big-endian bytes; - a nested struct's member list served in declaration order, with uint160 reported as size 20 (BYTES, as the wire wants); - an undefined struct REFUSED rather than answered with an empty member list, which the device would hash as a valid empty struct and sign a document neither side meant; - an unexpected message rejected instead of continuing blindly; - a device that never finishes the walk cut off at 512 round trips rather than hanging the host. eip712Wire.ts hand-writes the five messages because they are not in the published @keepkey/device-protocol package yet -- same approach as LoadClearsignSigner in ethereum.ts, and deletable the day the package ships generated classes. member_path is decoded accepting BOTH packed and unpacked repeated uint32: the device's encoder is not ours to pin, and assuming one form would break on the other. 35 tests green. --- .../src/eip712Streaming.test.ts | 99 +++++++++ .../hdwallet-keepkey/src/eip712Streaming.ts | 73 +++++++ packages/hdwallet-keepkey/src/eip712Wire.ts | 203 ++++++++++++++++++ 3 files changed, 375 insertions(+) create mode 100644 packages/hdwallet-keepkey/src/eip712Wire.ts diff --git a/packages/hdwallet-keepkey/src/eip712Streaming.test.ts b/packages/hdwallet-keepkey/src/eip712Streaming.test.ts index b111c83f..c1b98912 100644 --- a/packages/hdwallet-keepkey/src/eip712Streaming.test.ts +++ b/packages/hdwallet-keepkey/src/eip712Streaming.test.ts @@ -6,6 +6,7 @@ import { parseSolidityType, Resolved, resolveMemberPath, + runEip712Walk, structMembers, TypedDataDoc, } from "./eip712Streaming"; @@ -290,3 +291,101 @@ describe("dynamic leaves are capped at the wire limit", () => { expect(encodeValue(parseSolidityType("string"), "x".repeat(1024)).length).toEqual(1024); }); }); + +// ── the walk, driven against a scripted device ────────────────────── +// +// The device leads, so the only way to test the host half without hardware is +// to script a device and assert what the host answers. This models the exact +// sequence the firmware state machine produces for Permit2 PermitSingle: +// domain first, then the message, walking into PermitDetails. + +describe("runEip712Walk", () => { + const WIRE = { + SIGN: 1704, + STRUCT_REQUEST: 1705, + STRUCT_ACK: 1706, + VALUE_REQUEST: 1707, + VALUE_ACK: 1708, + SIGNATURE: 113, + encodeSign: (n: number[], p: string) => new TextEncoder().encode(JSON.stringify({ n, p })), + decodeStructRequest: (b: Uint8Array) => new TextDecoder().decode(b), + encodeStructAck: (m: Array<{ name: string; type: FieldType }>) => new TextEncoder().encode(JSON.stringify(m)), + decodeValueRequest: (b: Uint8Array) => JSON.parse(new TextDecoder().decode(b)) as number[], + encodeValueAck: (v: Uint8Array) => v, + decodeSignature: () => ({ address: "0xabc", signature: "0xsig" }), + }; + + /** A device that asks for exactly what the firmware would, in order. */ + function scriptedDevice(script: Array<{ type: number; payload: string }>) { + const answers: Array<{ type: number; payload: Uint8Array }> = []; + let i = 0; + const call = async (type: number, payload: Uint8Array) => { + answers.push({ type, payload }); + const step = script[i++]; + if (!step) return { type: WIRE.SIGNATURE, payload: new Uint8Array() }; + return { type: step.type, payload: new TextEncoder().encode(step.payload) }; + }; + return { call, answers }; + } + + it("answers every struct and value the device asks for, in order", async () => { + const { call, answers } = scriptedDevice([ + { type: WIRE.STRUCT_REQUEST, payload: "EIP712Domain" }, + { type: WIRE.VALUE_REQUEST, payload: "[0,0]" }, // domain.name + { type: WIRE.VALUE_REQUEST, payload: "[0,1]" }, // domain.chainId + { type: WIRE.VALUE_REQUEST, payload: "[0,2]" }, // domain.verifyingContract + { type: WIRE.STRUCT_REQUEST, payload: "PermitSingle" }, + { type: WIRE.STRUCT_REQUEST, payload: "PermitDetails" }, + { type: WIRE.VALUE_REQUEST, payload: "[1,0,0]" }, // details.token + { type: WIRE.VALUE_REQUEST, payload: "[1,0,1]" }, // details.amount + ]); + + const out = await runEip712Walk(PERMIT2, [44, 60, 0, 0, 0], WIRE, call); + expect(out).toEqual({ address: "0xabc", signature: "0xsig" }); + + // First message is the sign request, then one answer per device question. + expect(answers[0].type).toEqual(WIRE.SIGN); + expect(answers.slice(1).map((a) => a.type)).toEqual([ + WIRE.STRUCT_ACK, + WIRE.VALUE_ACK, + WIRE.VALUE_ACK, + WIRE.VALUE_ACK, + WIRE.STRUCT_ACK, + WIRE.STRUCT_ACK, + WIRE.VALUE_ACK, + WIRE.VALUE_ACK, + ]); + + // The nested uint160 came back as exactly 20 big-endian bytes of 0xff. + const amount = answers[answers.length - 1].payload; + expect(hex(amount)).toEqual("ff".repeat(20)); + }); + + it("serves the declared member list for a nested struct", async () => { + const { call, answers } = scriptedDevice([{ type: WIRE.STRUCT_REQUEST, payload: "PermitDetails" }]); + await runEip712Walk(PERMIT2, [44], WIRE, call); + const ack = JSON.parse(new TextDecoder().decode(answers[1].payload)); + expect(ack.map((m: { name: string }) => m.name)).toEqual(["token", "amount", "expiration", "nonce"]); + expect(ack[1].type.size).toEqual(20); // uint160 in BYTES + }); + + it("refuses a struct the document does not define rather than sending an empty list", async () => { + // An empty member list would hash as a valid empty struct, so the device + // would sign a document neither side meant. + const { call } = scriptedDevice([{ type: WIRE.STRUCT_REQUEST, payload: "Ghost" }]); + await expect(runEip712Walk(PERMIT2, [44], WIRE, call)).rejects.toThrow(/Unknown struct: Ghost/); + }); + + it("rejects an unexpected message instead of continuing blindly", async () => { + const { call } = scriptedDevice([{ type: 9999, payload: "" }]); + await expect(runEip712Walk(PERMIT2, [44], WIRE, call)).rejects.toThrow(/Unexpected message 9999/); + }); + + it("gives up rather than looping forever on a device that never finishes", async () => { + const call = async () => ({ + type: WIRE.STRUCT_REQUEST, + payload: new TextEncoder().encode("PermitDetails"), + }); + await expect(runEip712Walk(PERMIT2, [44], WIRE, call)).rejects.toThrow(/did not terminate/); + }); +}); diff --git a/packages/hdwallet-keepkey/src/eip712Streaming.ts b/packages/hdwallet-keepkey/src/eip712Streaming.ts index beb4d353..b8d9103b 100644 --- a/packages/hdwallet-keepkey/src/eip712Streaming.ts +++ b/packages/hdwallet-keepkey/src/eip712Streaming.ts @@ -333,3 +333,76 @@ export function structMembers(doc: TypedDataDoc, name: string): Array<{ name: st if (!members) throw new Error(`Unknown struct: ${name}`); return members.map((m) => ({ name: m.name, type: parseSolidityType(m.type) })); } + +/** + * Drive a full structured EIP-712 signature. + * + * The DEVICE leads. It asks for one struct definition, or one leaf value, at a + * time; this answers until it returns a signature. The host never decides the + * order, which is the point: the device hashes what it displays, in the order + * it chose, and a host that answered a different question would produce a + * digest that does not verify. + * + * `call` is injected rather than taking a Transport, so the loop is testable + * against a scripted device without USB. + */ +export interface Eip712Call { + (messageType: number, payload: Uint8Array): Promise<{ type: number; payload: Uint8Array }>; +} + +export interface Eip712Wire { + SIGN: number; + STRUCT_REQUEST: number; + STRUCT_ACK: number; + VALUE_REQUEST: number; + VALUE_ACK: number; + SIGNATURE: number; + encodeSign(addressNList: number[], primaryType: string): Uint8Array; + decodeStructRequest(payload: Uint8Array): string; + encodeStructAck(members: Array<{ name: string; type: FieldType }>): Uint8Array; + decodeValueRequest(payload: Uint8Array): number[]; + encodeValueAck(value: Uint8Array): Uint8Array; + decodeSignature(payload: Uint8Array): { address: string; signature: string }; +} + +/** Guards against a device that never terminates the walk. */ +const MAX_ROUND_TRIPS = 512; + +export async function runEip712Walk( + doc: TypedDataDoc, + addressNList: number[], + wire: Eip712Wire, + call: Eip712Call +): Promise<{ address: string; signature: string }> { + let reply = await call(wire.SIGN, wire.encodeSign(addressNList, doc.primaryType)); + + for (let i = 0; i < MAX_ROUND_TRIPS; i++) { + if (reply.type === wire.SIGNATURE) { + return wire.decodeSignature(reply.payload); + } + + if (reply.type === wire.STRUCT_REQUEST) { + const name = wire.decodeStructRequest(reply.payload); + // structMembers throws on an unknown struct rather than sending an empty + // member list, which the device would hash as a valid empty struct. + const members = structMembers(doc, name); + reply = await call(wire.STRUCT_ACK, wire.encodeStructAck(members)); + continue; + } + + if (reply.type === wire.VALUE_REQUEST) { + const path = wire.decodeValueRequest(reply.payload); + const resolved = resolveMemberPath(doc, path); + const bytes = + resolved.kind === "arrayLength" + ? encodeArrayLength(resolved.length) + : encodeValue(resolved.field, resolved.value); + reply = await call(wire.VALUE_ACK, wire.encodeValueAck(bytes)); + continue; + } + + throw new Error(`Unexpected message ${reply.type} during EIP-712 walk`); + } + + throw new Error("EIP-712 walk did not terminate"); +} diff --git a/packages/hdwallet-keepkey/src/eip712Wire.ts b/packages/hdwallet-keepkey/src/eip712Wire.ts new file mode 100644 index 00000000..507ee0c5 --- /dev/null +++ b/packages/hdwallet-keepkey/src/eip712Wire.ts @@ -0,0 +1,203 @@ +import * as jspb from "google-protobuf"; + +import { EthereumDataType, FieldType } from "./eip712Streaming"; + +/** + * Wire messages for structured EIP-712, hand-written because they are not yet + * in the published @keepkey/device-protocol package. Same approach as + * LoadClearsignSigner in ethereum.ts, and they can be deleted the day the + * package ships the generated classes. + * + * Message type IDs and field numbers are from device-protocol + * messages-ethereum.proto and messages.proto. + */ +export const MESSAGETYPE_ETHEREUMSIGNTYPEDDATA = 1704; +export const MESSAGETYPE_ETHEREUMTYPEDDATASTRUCTREQUEST = 1705; +export const MESSAGETYPE_ETHEREUMTYPEDDATASTRUCTACK = 1706; +export const MESSAGETYPE_ETHEREUMTYPEDDATAVALUEREQUEST = 1707; +export const MESSAGETYPE_ETHEREUMTYPEDDATAVALUEACK = 1708; + +/** message EthereumSignTypedData { repeated uint32 address_n = 1; required string primary_type = 2; optional bool metamask_v4_compat = 3; } */ +export class EthereumSignTypedData extends jspb.Message { + constructor(opt_data?: any) { + super(); + jspb.Message.initialize(this, opt_data || [], 0, -1, null, null); + } + setAddressNList(value: number[]): void { + jspb.Message.setField(this, 1, value); + } + setPrimaryType(value: string): void { + jspb.Message.setField(this, 2, value); + } + setMetamaskV4Compat(value: boolean): void { + jspb.Message.setField(this, 3, value); + } + toObject(): Record { + // Required by jspb.Message. Nothing on this path introspects these + // messages; the transport serialises them and reads the reply. + return {}; + } + serializeBinary(): Uint8Array { + const writer = new jspb.BinaryWriter(); + EthereumSignTypedData.serializeBinaryToWriter(this, writer); + return writer.getResultBuffer(); + } + static serializeBinaryToWriter(m: EthereumSignTypedData, writer: jspb.BinaryWriter): void { + writer.writeRepeatedUint32(1, jspb.Message.getFieldWithDefault(m, 1, []) as number[]); + writer.writeString(2, jspb.Message.getFieldWithDefault(m, 2, "") as string); + writer.writeBool(3, jspb.Message.getFieldWithDefault(m, 3, true) as boolean); + } + static deserializeBinary(bytes: Uint8Array): EthereumSignTypedData { + return new EthereumSignTypedData(bytes); + } +} + +/** message EthereumTypedDataStructRequest { required string name = 1; } */ +export class EthereumTypedDataStructRequest extends jspb.Message { + constructor(opt_data?: any) { + super(); + jspb.Message.initialize(this, opt_data || [], 0, -1, null, null); + } + getName(): string { + return jspb.Message.getFieldWithDefault(this, 1, "") as string; + } + toObject(): Record { + // Required by jspb.Message. Nothing on this path introspects these + // messages; the transport serialises them and reads the reply. + return {}; + } + serializeBinary(): Uint8Array { + const writer = new jspb.BinaryWriter(); + writer.writeString(1, this.getName()); + return writer.getResultBuffer(); + } + static deserializeBinary(bytes: Uint8Array): EthereumTypedDataStructRequest { + const reader = new jspb.BinaryReader(bytes); + const msg = new EthereumTypedDataStructRequest(); + while (reader.nextField()) { + if (reader.isEndGroup()) break; + if (reader.getFieldNumber() === 1) jspb.Message.setField(msg, 1, reader.readString()); + else reader.skipField(); + } + return msg; + } +} + +/** message EthereumTypedDataValueRequest { repeated uint32 member_path = 1; } */ +export class EthereumTypedDataValueRequest extends jspb.Message { + constructor(opt_data?: any) { + super(); + jspb.Message.initialize(this, opt_data || [], 0, -1, [1], null); + } + getMemberPathList(): number[] { + return (jspb.Message.getField(this, 1) as number[]) || []; + } + toObject(): Record { + // Required by jspb.Message. Nothing on this path introspects these + // messages; the transport serialises them and reads the reply. + return {}; + } + serializeBinary(): Uint8Array { + const writer = new jspb.BinaryWriter(); + writer.writeRepeatedUint32(1, this.getMemberPathList()); + return writer.getResultBuffer(); + } + static deserializeBinary(bytes: Uint8Array): EthereumTypedDataValueRequest { + const reader = new jspb.BinaryReader(bytes); + const msg = new EthereumTypedDataValueRequest(); + const path: number[] = []; + while (reader.nextField()) { + if (reader.isEndGroup()) break; + if (reader.getFieldNumber() === 1) { + // A repeated uint32 may arrive packed or unpacked; accept both rather + // than assuming, because the device's encoder is not ours to pin. + if (reader.isDelimited()) path.push(...reader.readPackedUint32()); + else path.push(reader.readUint32()); + } else reader.skipField(); + } + jspb.Message.setField(msg, 1, path); + return msg; + } +} + +/** message EthereumTypedDataValueAck { required bytes value = 1; } */ +export class EthereumTypedDataValueAck extends jspb.Message { + constructor(opt_data?: any) { + super(); + jspb.Message.initialize(this, opt_data || [], 0, -1, null, null); + } + setValue(value: Uint8Array): void { + jspb.Message.setField(this, 1, value); + } + toObject(): Record { + // Required by jspb.Message. Nothing on this path introspects these + // messages; the transport serialises them and reads the reply. + return {}; + } + serializeBinary(): Uint8Array { + const writer = new jspb.BinaryWriter(); + writer.writeBytes(1, jspb.Message.getFieldWithDefault(this, 1, new Uint8Array()) as Uint8Array); + return writer.getResultBuffer(); + } + static deserializeBinary(bytes: Uint8Array): EthereumTypedDataValueAck { + return new EthereumTypedDataValueAck(bytes); + } +} + +/** + * message EthereumTypedDataStructAck { + * repeated EthereumStructMember members = 1; + * message EthereumStructMember { required EthereumFieldType type = 1; required string name = 2; } + * message EthereumFieldType { + * required EthereumDataType data_type = 1; + * optional uint32 size = 2; + * optional string struct_name = 3; + * repeated uint32 array_levels = 4; + * } + * } + * + * Written by hand as nested submessages rather than via jspb's message + * machinery: the shape is small and fixed, and doing the length-delimited + * framing explicitly is easier to check against the .proto than a generated + * wrapper would be. + */ +export class EthereumTypedDataStructAck extends jspb.Message { + private members: Array<{ name: string; type: FieldType }> = []; + + constructor(members: Array<{ name: string; type: FieldType }> = []) { + super(); + jspb.Message.initialize(this, [], 0, -1, null, null); + this.members = members; + } + + private static writeFieldType(t: FieldType, writer: jspb.BinaryWriter): void { + writer.writeEnum(1, t.dataType); + if (t.size !== undefined) writer.writeUint32(2, t.size); + if (t.structName !== undefined) writer.writeString(3, t.structName); + if (t.arrayLevels.length > 0) writer.writeRepeatedUint32(4, t.arrayLevels); + } + + toObject(): Record { + // Required by jspb.Message. Nothing on this path introspects these + // messages; the transport serialises them and reads the reply. + return {}; + } + serializeBinary(): Uint8Array { + const writer = new jspb.BinaryWriter(); + for (const m of this.members) { + writer.writeMessage(1, m, (_unused: unknown, w: jspb.BinaryWriter) => { + w.writeMessage(1, m.type, (_u: unknown, tw: jspb.BinaryWriter) => { + EthereumTypedDataStructAck.writeFieldType(m.type, tw); + }); + w.writeString(2, m.name); + }); + } + return writer.getResultBuffer(); + } + + static deserializeBinary(bytes: Uint8Array): EthereumTypedDataStructAck { + return new EthereumTypedDataStructAck(); + } +} + +export { EthereumDataType }; From fde5f35866a18cb4d1c5806e472cccdf2f6ab512 Mon Sep 17 00:00:00 2001 From: highlander Date: Fri, 21 Aug 2026 17:37:10 -0500 Subject: [PATCH 6/9] style(eip712): the StructAck deserializer takes no input, and says so Host to device only -- the device never sends a StructAck, so there is nothing to parse. eslint was right that the parameter is dead; the underscore says it is dead on purpose rather than forgotten. Also: the previous commit's lint check printed OK unconditionally, so this got through. The check now reports its own exit status. --- packages/hdwallet-keepkey/src/eip712Wire.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/hdwallet-keepkey/src/eip712Wire.ts b/packages/hdwallet-keepkey/src/eip712Wire.ts index 507ee0c5..bde31bba 100644 --- a/packages/hdwallet-keepkey/src/eip712Wire.ts +++ b/packages/hdwallet-keepkey/src/eip712Wire.ts @@ -195,7 +195,9 @@ export class EthereumTypedDataStructAck extends jspb.Message { return writer.getResultBuffer(); } - static deserializeBinary(bytes: Uint8Array): EthereumTypedDataStructAck { + /* Host to device only. The device never sends a StructAck, so there is + * nothing to parse -- but jspb's shape expects the static to exist. */ + static deserializeBinary(_bytes: Uint8Array): EthereumTypedDataStructAck { return new EthereumTypedDataStructAck(); } } From 55b89baf150f74a4d4894694238ad29190fd1cdb Mon Sep 17 00:00:00 2001 From: highlander Date: Fri, 21 Aug 2026 17:40:07 -0500 Subject: [PATCH 7/9] feat(eip712): route every typed-data signature through the streaming path The wiring that makes the walk reachable. ethSignTypedData now tries the streaming path for EVERY document, not just x402, and falls back only when the device says it cannot do it. Three pieces: 1. typeRegistry registers message types 1704-1708 by hand. The registry builds itself from Messages.MessageType, and the published device-protocol package does not carry these yet, so the reducers cannot see them. Without this the transport can SEND a request and then fail to decode the reply -- the walk would stall on its first StructRequest. Delete the block when the package ships generated classes; the reducers pick them up on their own. 2. RawPayload carries already-serialised bytes through transport.call, which wants a jspb.Message. The walk produces bytes; re-encoding them through a second object is a chance for the two encodings to differ, and the whole point of this protocol is that they cannot. 3. The fallback predicate now covers two refusals rather than one: - "Structured EIP-712 disabled" -- 7.14.2 withdrew the old endpoint. - "arrays are not supported" -- the new walk is present but cannot walk THIS document. The second matters more than it looks. PermitBatch and Seaport nest arrays, and a hard failure there would read as a bug rather than a limitation. Degrading costs the user the field display and keeps the payment working, which is exactly the deal every typed-data payload gets today. Everything else still propagates. A refused screen, a malformed document, a value that does not match its declared type -- those are real answers, and masking them behind a silent downgrade is how a signing bug becomes invisible. AdvancedMode is deliberately NOT in that list: the hashed path requires it too, so falling back would fail again with a worse message. 35 tests green. --- packages/hdwallet-keepkey/src/eip712Wire.ts | 23 +++++ packages/hdwallet-keepkey/src/ethereum.ts | 85 ++++++++++++++++++- packages/hdwallet-keepkey/src/typeRegistry.ts | 29 +++++++ 3 files changed, 136 insertions(+), 1 deletion(-) diff --git a/packages/hdwallet-keepkey/src/eip712Wire.ts b/packages/hdwallet-keepkey/src/eip712Wire.ts index bde31bba..08545989 100644 --- a/packages/hdwallet-keepkey/src/eip712Wire.ts +++ b/packages/hdwallet-keepkey/src/eip712Wire.ts @@ -203,3 +203,26 @@ export class EthereumTypedDataStructAck extends jspb.Message { } export { EthereumDataType }; + +/** + * A message that is already serialised. transport.call wants a jspb.Message, + * and the walk produces bytes, so this carries them across without a second + * encode that could differ from the first. + */ +export class RawPayload extends jspb.Message { + private raw: Uint8Array; + constructor(bytes: Uint8Array) { + super(); + jspb.Message.initialize(this, [], 0, -1, null, null); + this.raw = bytes; + } + toObject(): Record { + return {}; + } + serializeBinary(): Uint8Array { + return this.raw; + } + static deserializeBinary(bytes: Uint8Array): RawPayload { + return new RawPayload(bytes); + } +} diff --git a/packages/hdwallet-keepkey/src/ethereum.ts b/packages/hdwallet-keepkey/src/ethereum.ts index 5b69bf43..dff074ed 100644 --- a/packages/hdwallet-keepkey/src/ethereum.ts +++ b/packages/hdwallet-keepkey/src/ethereum.ts @@ -8,6 +8,8 @@ import { getStructHash } from "eip-712"; import * as eip55 from "eip55"; import * as jspb from "google-protobuf"; +import { Eip712Call, Eip712Wire as Eip712WireShape, FieldType, runEip712Walk, TypedDataDoc } from "./eip712Streaming"; +import * as Eip712Wire from "./eip712Wire"; import { Transport } from "./transport"; import { messageNameRegistry, messageTypeRegistry } from "./typeRegistry"; import { toUTF8Array } from "./utils"; @@ -693,7 +695,77 @@ function structuredEip712Unavailable(error: unknown): boolean { if (error.message_enum !== Messages.MessageType.MESSAGETYPE_FAILURE) return false; const failure = error.message as { code?: number; message?: string } | undefined; if (failure?.code === Types.FailureType.FAILURE_UNEXPECTEDMESSAGE) return true; - return typeof failure?.message === "string" && failure.message.includes("Structured EIP-712 disabled"); + const text = typeof failure?.message === "string" ? failure.message : ""; + // Withdrawn in 7.14.2. + if (text.includes("Structured EIP-712 disabled")) return true; + // Present, but cannot walk THIS document. Arrays are the current gap: + // PermitBatch and Seaport nest them. Degrading costs the user the field + // display and keeps the payment working, which is the same deal every + // typed-data payload gets today -- a hard failure would be strictly worse + // and would look like a bug rather than a limitation. + if (text.includes("arrays are not supported")) return true; + return false; +} + +/** + * Structured EIP-712 over the streaming protocol: the device walks the document + * and hashes each leaf in the same call that displays it. + * + * Fails to the hashed path on firmware that does not implement it, via the same + * structuredEip712Unavailable() test the x402 branch uses. + */ +async function signTypedDataStreaming( + transport: Transport, + addressNList: number[], + typedData: TypedDataDoc +): Promise { + const wire: Eip712WireShape = { + SIGN: Eip712Wire.MESSAGETYPE_ETHEREUMSIGNTYPEDDATA, + STRUCT_REQUEST: Eip712Wire.MESSAGETYPE_ETHEREUMTYPEDDATASTRUCTREQUEST, + STRUCT_ACK: Eip712Wire.MESSAGETYPE_ETHEREUMTYPEDDATASTRUCTACK, + VALUE_REQUEST: Eip712Wire.MESSAGETYPE_ETHEREUMTYPEDDATAVALUEREQUEST, + VALUE_ACK: Eip712Wire.MESSAGETYPE_ETHEREUMTYPEDDATAVALUEACK, + SIGNATURE: Messages.MessageType.MESSAGETYPE_ETHEREUMTYPEDDATASIGNATURE, + + encodeSign: (n: number[], primaryType: string) => { + const m = new Eip712Wire.EthereumSignTypedData(); + m.setAddressNList(n); + m.setPrimaryType(primaryType); + // v3 hashes arrays of structs differently. We speak v4 only, and the + // device refuses anything else rather than guessing. + m.setMetamaskV4Compat(true); + return m.serializeBinary(); + }, + decodeStructRequest: (b: Uint8Array) => Eip712Wire.EthereumTypedDataStructRequest.deserializeBinary(b).getName(), + encodeStructAck: (members: Array<{ name: string; type: FieldType }>) => + new Eip712Wire.EthereumTypedDataStructAck(members).serializeBinary(), + decodeValueRequest: (b: Uint8Array) => + Eip712Wire.EthereumTypedDataValueRequest.deserializeBinary(b).getMemberPathList(), + encodeValueAck: (v: Uint8Array) => { + const m = new Eip712Wire.EthereumTypedDataValueAck(); + m.setValue(v); + return m.serializeBinary(); + }, + decodeSignature: (b: Uint8Array) => { + const r = Ethereum.EthereumTypedDataSignature.deserializeBinary(b); + return { + address: r.getAddress() || "", + signature: "0x" + core.toHexString(r.getSignature_asU8()), + }; + }, + }; + + const call: Eip712Call = async (messageType: number, payload: Uint8Array) => { + const event = await transport.call(messageType, new Eip712Wire.RawPayload(payload), { + msgTimeout: core.LONG_TIMEOUT, + omitLock: true, + }); + const proto = event.proto as jspb.Message; + return { type: event.message_enum as number, payload: proto.serializeBinary() }; + }; + + const out = await runEip712Walk(typedData, addressNList, wire, call); + return { address: out.address, signature: out.signature }; } async function signStructuredEip712( @@ -757,6 +829,17 @@ export async function ethSignTypedData( const typedData = withEip712DomainType(msg.typedData); const { primaryType, domain, message } = typedData; + // Prefer the streaming path for EVERY document: the device parses and + // displays the fields itself, instead of signing two opaque hashes. + try { + return await signTypedDataStreaming(transport, msg.addressNList, typedData as unknown as TypedDataDoc); + } catch (e) { + // Only "this firmware has no structured endpoint" degrades. Anything + // else -- a refused screen, a malformed document, an array this + // firmware cannot walk -- is a real answer and must not be masked. + if (!structuredEip712Unavailable(e)) throw e; + } + if (isX402Eip3009(typedData)) { try { return await signStructuredEip712(transport, msg.addressNList, typedData); diff --git a/packages/hdwallet-keepkey/src/typeRegistry.ts b/packages/hdwallet-keepkey/src/typeRegistry.ts index 8b923c27..99b83070 100644 --- a/packages/hdwallet-keepkey/src/typeRegistry.ts +++ b/packages/hdwallet-keepkey/src/typeRegistry.ts @@ -15,6 +15,8 @@ import * as TronMessages from "@keepkey/device-protocol/lib/messages-tron_pb"; import * as ZcashMessages from "@keepkey/device-protocol/lib/messages-zcash_pb"; import * as core from "@keepkey/hdwallet-core"; import * as jspb from "google-protobuf"; + +import * as Eip712 from "./eip712Wire"; function omit(obj: Record, ...keys: string[]): Record { const result = { ...obj }; for (const key of keys) delete result[key]; @@ -56,3 +58,30 @@ export const messageTypeRegistry = Object.entries(Messages.MessageType).reduce(( registry[entry[1]] = upperCasedMessageClasses[entry[0].split("_")[1].toUpperCase()]; return registry; }, {} as Record>); + +/* Structured EIP-712 (message types 1704-1708). + * + * These are registered by hand because the published @keepkey/device-protocol + * package does not carry them yet, so they are absent from Messages.MessageType + * and the reducers above cannot see them. Without this the transport can send a + * request but cannot decode the device's reply, and the walk stalls on its + * first StructRequest. + * + * Delete this block when the package ships the generated classes -- the + * reducers will then pick them up on their own. */ +messageNameRegistry[Eip712.MESSAGETYPE_ETHEREUMSIGNTYPEDDATA] = "EthereumSignTypedData"; +messageNameRegistry[Eip712.MESSAGETYPE_ETHEREUMTYPEDDATASTRUCTREQUEST] = "EthereumTypedDataStructRequest"; +messageNameRegistry[Eip712.MESSAGETYPE_ETHEREUMTYPEDDATASTRUCTACK] = "EthereumTypedDataStructAck"; +messageNameRegistry[Eip712.MESSAGETYPE_ETHEREUMTYPEDDATAVALUEREQUEST] = "EthereumTypedDataValueRequest"; +messageNameRegistry[Eip712.MESSAGETYPE_ETHEREUMTYPEDDATAVALUEACK] = "EthereumTypedDataValueAck"; + +messageTypeRegistry[Eip712.MESSAGETYPE_ETHEREUMSIGNTYPEDDATA] = + Eip712.EthereumSignTypedData as unknown as core.Constructor; +messageTypeRegistry[Eip712.MESSAGETYPE_ETHEREUMTYPEDDATASTRUCTREQUEST] = + Eip712.EthereumTypedDataStructRequest as unknown as core.Constructor; +messageTypeRegistry[Eip712.MESSAGETYPE_ETHEREUMTYPEDDATASTRUCTACK] = + Eip712.EthereumTypedDataStructAck as unknown as core.Constructor; +messageTypeRegistry[Eip712.MESSAGETYPE_ETHEREUMTYPEDDATAVALUEREQUEST] = + Eip712.EthereumTypedDataValueRequest as unknown as core.Constructor; +messageTypeRegistry[Eip712.MESSAGETYPE_ETHEREUMTYPEDDATAVALUEACK] = + Eip712.EthereumTypedDataValueAck as unknown as core.Constructor; From 9b2b17073c5ff88c0ee88b8795311d9af5f4fb8b Mon Sep 17 00:00:00 2001 From: highlander Date: Fri, 21 Aug 2026 18:06:33 -0500 Subject: [PATCH 8/9] test(x402): model a device without the streaming endpoint The mock answered EVERY call as Ethereum712TypesValues: const phase = request.getEip712typevals() ?? 0; Now that ethSignTypedData tries streaming first for every document, the first call is message type 1704 carrying a RawPayload, and that line threw "request.getEip712typevals is not a function". The mock was modelling a device that cannot exist -- one that answers an unknown message type as though it understood it. Real firmware without the streaming endpoint rejects 1704 with Failure_UnexpectedMessage, which is exactly what 7.14.x does, so the mock now does that. This makes the test cover MORE than it did: the x402 payload still reaches the old structured endpoint, and it now gets there through the fallback rather than because nothing else was tried. That fallback is the path every device in the field takes today, and until now nothing exercised it. --- .../src/ethereum-x402.test.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/hdwallet-keepkey/src/ethereum-x402.test.ts b/packages/hdwallet-keepkey/src/ethereum-x402.test.ts index 0466402b..2f5d0aa9 100644 --- a/packages/hdwallet-keepkey/src/ethereum-x402.test.ts +++ b/packages/hdwallet-keepkey/src/ethereum-x402.test.ts @@ -3,6 +3,26 @@ import * as Ethereum from "@keepkey/device-protocol/lib/messages-ethereum_pb"; import { ethSignTypedData } from "./ethereum"; const ETHEREUM_712_TYPES_VALUES = 114; +const MESSAGETYPE_FAILURE = 3; +const FAILURE_UNEXPECTEDMESSAGE = 1; + +/** Streaming EIP-712 message types, 1704-1708. */ +const STREAMING = new Set([1704, 1705, 1706, 1707, 1708]); + +/** + * What a device WITHOUT the streaming endpoint does: reject the unknown + * message type. ethSignTypedData now tries streaming first for every document, + * so a mock that answers every call as Ethereum712TypesValues is modelling a + * device that cannot exist. Rejecting 1704 the way 7.14.x firmware does makes + * this test cover the fallback as well as the old path. + */ +function rejectUnknownMessage() { + // eslint-disable-next-line no-throw-literal + throw { + message_enum: MESSAGETYPE_FAILURE, + message: { code: FAILURE_UNEXPECTEDMESSAGE, message: "Unexpected message" }, + }; +} const PATH = [0x8000002c, 0x8000003c, 0x80000000, 0, 0]; function makeMockTransport(call: jest.Mock) { @@ -17,6 +37,7 @@ describe("x402 EVM structured signing", () => { it("sends the official EIP-3009 authorization as reviewed domain + message", async () => { const streamed: Array<{ phase: number; data: any }> = []; const call = jest.fn().mockImplementation((_messageType: number, request: Ethereum.Ethereum712TypesValues) => { + if (STREAMING.has(_messageType)) rejectUnknownMessage(); const phase = request.getEip712typevals() ?? 0; expect(_messageType).toBe(ETHEREUM_712_TYPES_VALUES); expect(JSON.parse(request.getEip712primetype() || "{}")).toEqual({ From c2a635484d23a54680814708c1ed079da0555a33 Mon Sep 17 00:00:00 2001 From: highlander Date: Fri, 21 Aug 2026 18:34:57 -0500 Subject: [PATCH 9/9] test(x402): three calls, because the fallback is now exercised The count assertion said two. It is three: the streaming probe this mock rejects, then the two old-path calls. Asserting two would be asserting that we never TRY the streaming path -- which is the opposite of the intended behaviour and would go green precisely when the feature stopped working. The call types are now asserted explicitly (1704, then 114 twice) so the sequence is visible rather than implied by a count. --- packages/hdwallet-keepkey/src/ethereum-x402.test.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/hdwallet-keepkey/src/ethereum-x402.test.ts b/packages/hdwallet-keepkey/src/ethereum-x402.test.ts index 2f5d0aa9..096797f6 100644 --- a/packages/hdwallet-keepkey/src/ethereum-x402.test.ts +++ b/packages/hdwallet-keepkey/src/ethereum-x402.test.ts @@ -93,9 +93,18 @@ describe("x402 EVM structured signing", () => { }, }); - expect(call).toHaveBeenCalledTimes(2); + // THREE calls now, not two: the streaming probe that this device rejects, + // then the two old-path calls. The probe is the fallback working -- every + // device in the field today answers 1704 with Failure_UnexpectedMessage, + // and asserting two calls would be asserting that we never tried. + expect(call).toHaveBeenCalledTimes(3); expect(transport.lockDuring).toHaveBeenCalledTimes(1); - expect(call.mock.calls.map(([, , options]) => options)).toEqual([ + expect(call.mock.calls.map(([type]) => type)).toEqual([ + 1704, // EthereumSignTypedData -- rejected as unknown + ETHEREUM_712_TYPES_VALUES, + ETHEREUM_712_TYPES_VALUES, + ]); + expect(call.mock.calls.slice(1).map(([, , options]) => options)).toEqual([ { msgTimeout: expect.any(Number), omitLock: true }, { msgTimeout: expect.any(Number), omitLock: true }, ]);