From c76e21ed2ca010b2032e7478b2dcb35ce46b8e66 Mon Sep 17 00:00:00 2001 From: Joe Blau Date: Mon, 27 Jul 2026 20:59:32 -0700 Subject: [PATCH 1/2] fix: sign through an embedded local account, and survive hosts without dynamic import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Release 0.1.5. Two fixes reported from downstream use. A viem WalletClient built over a local account lost the fast path `createWalletClient({ account: privateKeyToAccount(key), ... })` — what wagmi and viem hand around when the key lives in process — always satisfies the JSON-RPC structural guard, since it carries signTypedData, getAddresses and getChainId. It was therefore adapted as a remote wallet and every L1 action went through generic typed-data encoding, even though the key was right there. Measured downstream at 2.17x on every order, with nothing to indicate it was happening. Re-routing such a client to the local adapter would have been wrong: that adapter hardcodes getChainId to "0x1" ("local accounts have no notion of chain"), and getChainId feeds signatureChainId for user-signed actions, so the EIP-712 domain would have silently stopped reporting the chain the signature was produced on. Instead the JSON-RPC adapter now additionally sources `signDigest` from the embedded account. Typed data, address, chain ID and every cache keep going through the client, so this is a pure addition: only L1 digest signing changes, which is exactly where the cost was. Detection is deliberately narrow — viem's own `type: "local"` marker plus a usable raw-digest signer. A client over a remote account keeps signing through the client. createFastLocalWallet hard-failed where dynamic import is unavailable Its docstring promised the tiny-secp256k1-missing path was "never a hard failure", but the fallback does `await import("viem/accounts")`, which throws outright in hosts that cannot service a dynamic import — Jest without --experimental-vm-modules, and some React Native bundlers. The specifier has to stay dynamic: viem is not a dependency of this package, so a static import would break every consumer that does not use viem. Callers in such hosts now pass `options.privateKeyToAccount` instead, which is used on both viem-dependent paths (the fallback and the lazy signTypedData delegate). When viem is genuinely needed and can be neither imported nor supplied, the failure now says so and names the remedy, preserving the host's original error as `cause`, rather than surfacing an opaque message from deep in the signing path. The docstrings no longer promise a fallback that cannot be delivered. Co-Authored-By: Claude Fable 5 --- package.json | 2 +- src/signing/_abstractWallet.ts | 67 ++++++++++++++++++---- src/signing/_fastWallet.ts | 63 +++++++++++++++++++-- tests/signing/fastWallet.test.ts | 64 +++++++++++++++++++++ tests/signing/mod.test.ts | 95 +++++++++++++++++++++++++++++++- 5 files changed, 274 insertions(+), 17 deletions(-) diff --git a/package.json b/package.json index 810818e6..8dd908de 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@bloxwap/hyperliquid", - "version": "0.1.4", + "version": "0.1.5", "description": "Blazing fast TypeScript Hyperliquid SDK.", "license": "MIT", "type": "module", diff --git a/src/signing/_abstractWallet.ts b/src/signing/_abstractWallet.ts index bc63c7dd..7f75c690 100644 --- a/src/signing/_abstractWallet.ts +++ b/src/signing/_abstractWallet.ts @@ -148,9 +148,65 @@ function isViemJsonRpc(wallet: AbstractWallet): wallet is AbstractViemJsonRpcAcc ); } +/** + * A viem `WalletClient` configured with a local account, e.g. `createWalletClient({ account: + * privateKeyToAccount(key), … })` — the shape wagmi and viem hand around when the key lives in + * process. + * + * Such a client satisfies {@linkcode isViemJsonRpc} unconditionally (it always carries + * `signTypedData`, `getAddresses` and `getChainId`), so without this it would be adapted as a + * remote wallet and every L1 action would go through generic typed-data encoding — even though the + * key is right there and can sign the digest directly. + */ +interface LocalAccountCarrier { + account?: { type?: string } & AbstractViemLocalAccount & DigestBytesCapable; +} + +/** + * The embedded local account of a JSON-RPC-shaped wallet, when it has one that can sign raw digests. + * + * Deliberately narrow: only viem's own `type: "local"` marker counts. A remote account (`type: + * "json-rpc"`) must keep going through the client, and a wallet whose `account` cannot sign a + * digest has nothing to offer here. + */ +function embeddedLocalAccount(wallet: AbstractWallet): (AbstractViemLocalAccount & DigestBytesCapable) | undefined { + const account = (wallet as LocalAccountCarrier).account; + if (account === undefined || account === null || account.type !== "local") return undefined; + if (typeof account.address !== "string") return undefined; + const canSignDigest = typeof account.sign === "function" || typeof account[SIGN_DIGEST_BYTES] === "function"; + return canSignDigest ? account : undefined; +} + +/** + * Raw-digest signer built from a viem local account, or `undefined` when it cannot sign digests. + * + * Shared by {@linkcode adaptViemLocal} and the JSON-RPC adapter, so a local account signs L1 + * digests the same way whether it was passed directly or wrapped in a `WalletClient`. + */ +function digestSignerFor( + account: AbstractViemLocalAccount & DigestBytesCapable, +): ((digest: Uint8Array) => Promise) | undefined { + // A wallet carrying the bytes-level capability (the WASM fast wallet) skips the hex conversion + // the hex-speaking `sign` requires. + const signDigestBytes = account[SIGN_DIGEST_BYTES]; + if (typeof signDigestBytes === "function") { + return async (digest: Uint8Array): Promise => parseSignature(await signDigestBytes(digest)); + } + if (typeof account.sign === "function") { + return async (digest: Uint8Array): Promise => + parseSignature(await account.sign!({ hash: `0x${bytesToHex(digest)}` })); + } + return undefined; +} + function adaptViemJsonRpc(wallet: AbstractViemJsonRpcAccount): Signer { + // When the client wraps an in-process key, L1 actions sign the digest through that account. + // Everything else — typed data, address, chain ID — still goes through the client, so a wallet + // that switches chains or accounts behaves exactly as it did before. + const localAccount = embeddedLocalAccount(wallet); return { kind: "viem-jsonrpc", + signDigest: localAccount === undefined ? undefined : digestSignerFor(localAccount), async signTypedData(args: TypedDataArgs): Promise { const hex = await wallet.signTypedData({ domain: args.domain, @@ -218,9 +274,6 @@ function isViemLocal(wallet: AbstractWallet): wallet is AbstractViemLocalAccount } function adaptViemLocal(wallet: AbstractViemLocalAccount): Signer { - // A wallet carrying the bytes-level capability (the WASM fast wallet) skips the hex conversion - // the hex-speaking `sign` requires. - const signDigestBytes = (wallet as DigestBytesCapable)[SIGN_DIGEST_BYTES]; return { kind: "viem-local", async signTypedData(args: TypedDataArgs): Promise { @@ -234,13 +287,7 @@ function adaptViemLocal(wallet: AbstractViemLocalAccount): Signer { }, // A viem local account can sign a raw 32-byte digest locally; wire that up so callers with a // precomputed digest can skip the typed-data encoding round trip entirely. - signDigest: - typeof signDigestBytes === "function" - ? async (digest: Uint8Array): Promise => parseSignature(await signDigestBytes(digest)) - : typeof wallet.sign === "function" - ? async (digest: Uint8Array): Promise => - parseSignature(await wallet.sign!({ hash: `0x${bytesToHex(digest)}` })) - : undefined, + signDigest: digestSignerFor(wallet as AbstractViemLocalAccount & DigestBytesCapable), getAddress(): Promise<`0x${string}`> { return Promise.resolve(wallet.address.toLowerCase() as `0x${string}`); }, diff --git a/src/signing/_fastWallet.ts b/src/signing/_fastWallet.ts index b2f06663..72b694f7 100644 --- a/src/signing/_fastWallet.ts +++ b/src/signing/_fastWallet.ts @@ -17,6 +17,11 @@ * `signTypedData` (user-signed actions, multi-sig wrappers) is not accelerated: it delegates to a * viem local account created from the same key, imported lazily on first use. A wallet used only * for L1 actions therefore never loads viem at all. + * + * viem is not a dependency of this package, so both viem-dependent paths reach it through + * `import()`. Environments that cannot service a dynamic import (Jest without + * `--experimental-vm-modules`, some React Native bundlers) supply `options.privateKeyToAccount` + * instead; without it those paths throw an error saying so. * @module */ @@ -70,6 +75,39 @@ export async function loadTinySecp256k1( /** The loader the factory uses; replaced only by tests through {@linkcode _setEccLoaderForTests}. */ let eccLoader: () => Promise = loadTinySecp256k1; +/** `privateKeyToAccount` from `viem/accounts`, however the caller obtained it. */ +export type PrivateKeyToAccount = (privateKey: `0x${string}`) => AbstractViemLocalAccount; + +/** + * Resolves `privateKeyToAccount`, preferring one the caller supplied. + * + * `viem` is not a dependency of this package — not even an optional one — so it can only be + * reached through `import()`, and the specifier has to stay dynamic or every consumer who does not + * use viem would fail to resolve this module at load time. Some environments cannot service a + * dynamic import at all: Jest without `--experimental-vm-modules` rejects it with "A dynamic + * import callback was invoked without --experimental-vm-modules", and bundlers targeting React + * Native may drop it. Those callers pass `privateKeyToAccount` in and never reach the import. + * + * When neither is possible the failure is reported for what it is, rather than surfacing the + * host's opaque message from somewhere deep in the signing path. + */ +async function resolvePrivateKeyToAccount(provided: PrivateKeyToAccount | undefined): Promise { + if (provided !== undefined) return provided; + try { + const { privateKeyToAccount } = await import("viem/accounts"); + return privateKeyToAccount as PrivateKeyToAccount; + } catch (cause) { + throw new AbstractWalletError( + "createFastLocalWallet: could not load `viem/accounts`. It is imported dynamically because " + + "viem is not a dependency of this package, and this environment cannot service a dynamic " + + "import (Jest without `--experimental-vm-modules` is the usual cause). Import " + + "`privateKeyToAccount` from `viem/accounts` yourself and pass it as " + + "`options.privateKeyToAccount`, or install `tiny-secp256k1` so signing never needs viem.", + { cause }, + ); + } +} + /** * Internal test hook: swaps the `tiny-secp256k1` loader (pass `undefined` to restore the real one) * and resets the one-time warning latch. Module mocking cannot reliably simulate a missing @@ -116,15 +154,23 @@ function toChecksumAddress(address: `0x${string}`): `0x${string}` { * * Fallbacks, in order: * - `tiny-secp256k1` missing or broken → a one-time `console.warn`, and the factory returns the - * plain viem local account (the noble path) — never a hard failure. + * plain viem local account (the noble path). * - `signTypedData` → always delegated to viem (imported lazily on first use), since the WASM * module accelerates raw digests only. * + * Both fallbacks need `viem/accounts`, which this package can only reach through a dynamic import + * (viem is not a dependency of it). Environments that cannot service one — Jest without + * `--experimental-vm-modules`, some React Native bundlers — must pass `options.privateKeyToAccount`; + * otherwise those two paths throw with an explanation. With `tiny-secp256k1` present and only L1 + * actions signed, viem is never needed at all. + * * @param privateKey The 32-byte private key as a hex string. - * @param options Set `wasm: false` to skip the WASM accelerator and use the viem/noble path directly. + * @param options Set `wasm: false` to skip the WASM accelerator and use the viem/noble path directly, + * and `privateKeyToAccount` to supply viem's factory where a dynamic import is unavailable. * @return The wallet, WASM-accelerated when available. * - * @throws {AbstractWalletError} If the private key is not a valid 32-byte secp256k1 scalar. + * @throws {AbstractWalletError} If the private key is not a valid 32-byte secp256k1 scalar, or if + * viem is needed but can neither be imported nor was supplied. * * @example * ```ts @@ -141,6 +187,13 @@ export async function createFastLocalWallet( options?: { /** Set `false` to skip the WASM accelerator and use the viem/noble path. Default: `true`. */ wasm?: boolean; + /** + * `privateKeyToAccount` from `viem/accounts`, for environments where this package cannot + * `import()` it (Jest without `--experimental-vm-modules`, some React Native bundlers). When + * supplied it is used instead of the dynamic import on both viem-dependent paths: the + * `tiny-secp256k1` fallback and `signTypedData`. + */ + privateKeyToAccount?: PrivateKeyToAccount; }, ): Promise { if (!/^0x[0-9a-fA-F]{64}$/.test(privateKey)) { @@ -157,7 +210,7 @@ export async function createFastLocalWallet( "falling back to the viem/noble signing path. Install `tiny-secp256k1` to enable WASM acceleration.", ); } - const { privateKeyToAccount } = await import("viem/accounts"); + const privateKeyToAccount = await resolvePrivateKeyToAccount(options?.privateKeyToAccount); return privateKeyToAccount(privateKey); } @@ -169,7 +222,7 @@ export async function createFastLocalWallet( let delegate: AbstractViemLocalAccount | undefined; const viemDelegate = async (): Promise => { if (delegate === undefined) { - const { privateKeyToAccount } = await import("viem/accounts"); + const privateKeyToAccount = await resolvePrivateKeyToAccount(options?.privateKeyToAccount); delegate = privateKeyToAccount(privateKey); } return delegate; diff --git a/tests/signing/fastWallet.test.ts b/tests/signing/fastWallet.test.ts index 21d847fe..d734f8d8 100644 --- a/tests/signing/fastWallet.test.ts +++ b/tests/signing/fastWallet.test.ts @@ -230,6 +230,70 @@ describe("createFastLocalWallet() fallback", () => { } }); + test("an injected `privateKeyToAccount` is used instead of the dynamic import", async () => { + // Environments that cannot service `import()` (Jest without `--experimental-vm-modules`, some + // React Native bundlers) pass viem's factory in. It must be used on both viem-dependent paths. + _setEccLoaderForTests(() => Promise.resolve(undefined)); + const warn = spyOn(console, "warn").mockImplementation(() => {}); + try { + let injectedCalls = 0; + const injected = (key: `0x${string}`): ReturnType => { + injectedCalls++; + return privateKeyToAccount(key); + }; + + const fast = await createFastLocalWallet(PRIVATE_KEYS[0], { privateKeyToAccount: injected }); + expect(injectedCalls).toBeGreaterThan(0); + expect(fast.address).toBe(privateKeyToAccount(PRIVATE_KEYS[0]).address); + + const args = { action: { ...CANCEL }, nonce: NONCE, isTestnet: true } as const; + expect(await signL1Action({ wallet: fast, ...args })).toEqual( + await signL1Action({ wallet: privateKeyToAccount(PRIVATE_KEYS[0]), ...args }), + ); + } finally { + warn.mockRestore(); + } + }); + + test("the WASM path uses an injected `privateKeyToAccount` for signTypedData", async () => { + // With WASM present the fallback never runs, but `signTypedData` still needs viem. + let injectedCalls = 0; + const injected = (key: `0x${string}`): ReturnType => { + injectedCalls++; + return privateKeyToAccount(key); + }; + + const fast = await createFastLocalWallet(PRIVATE_KEYS[0], { privateKeyToAccount: injected }); + expect(injectedCalls).toBe(0); // not needed until typed data is signed + + const signed = await fast.signTypedData({ + domain: { name: "Exchange", version: "1", chainId: 1337, verifyingContract: `0x${"00".repeat(20)}` }, + types: { + Agent: [ + { name: "source", type: "string" }, + { name: "connectionId", type: "bytes32" }, + ], + }, + primaryType: "Agent", + message: { source: "a", connectionId: `0x${"11".repeat(32)}` }, + }); + + expect(injectedCalls).toBe(1); + expect(signed).toBe( + await privateKeyToAccount(PRIVATE_KEYS[0]).signTypedData({ + domain: { name: "Exchange", version: "1", chainId: 1337, verifyingContract: `0x${"00".repeat(20)}` }, + types: { + Agent: [ + { name: "source", type: "string" }, + { name: "connectionId", type: "bytes32" }, + ], + }, + primaryType: "Agent", + message: { source: "a", connectionId: `0x${"11".repeat(32)}` }, + }), + ); + }); + test("`wasm: false` takes the viem/noble path without loading or warning", async () => { const warn = spyOn(console, "warn").mockImplementation(() => {}); try { diff --git a/tests/signing/mod.test.ts b/tests/signing/mod.test.ts index f0a76df0..79554104 100644 --- a/tests/signing/mod.test.ts +++ b/tests/signing/mod.test.ts @@ -5,7 +5,7 @@ */ import { describe, expect, test } from "bun:test"; -import { assertEquals } from "@jsr/std__assert"; +import { assert, assertEquals } from "@jsr/std__assert"; import { createWalletClient, custom } from "viem"; import { privateKeyToAccount } from "viem/accounts"; import { arbitrum } from "viem/chains"; @@ -376,6 +376,99 @@ describe("signing", () => { }); }); +// ============================================================ +// WalletClient wrapping a local account +// ============================================================ + +/** + * A `WalletClient` built over a local account satisfies the JSON-RPC guard unconditionally — it + * always carries `signTypedData`, `getAddresses` and `getChainId` — so it used to be adapted as a + * remote wallet and lose the raw-digest path even though the key was in process. It must now sign + * L1 digests through the embedded account while everything else still goes through the client. + */ +describe("viem WalletClient over a local account", () => { + /** A transport that fails loudly: a local-account client must not need the network to sign. */ + const offlineTransport = custom({ + request: async ({ method }: { method: string }) => { + if (method === "eth_chainId") return await Promise.resolve("0xa4b1"); + throw new Error(`Unexpected RPC method: ${method}`); + }, + }); + + test("signs an L1 action identically to the bare local account", async () => { + const account = privateKeyToAccount(PRIVATE_KEY); + const client = createWalletClient({ account, chain: arbitrum, transport: offlineTransport }); + const action = { type: "cancel", cancels: [{ a: 0, o: 12345 }] }; + + const bare = await signL1Action({ wallet: account, action, nonce: 1700000000000 }); + const viaClient = await signL1Action({ wallet: client, action, nonce: 1700000000000 }); + + assertEquals(viaClient, bare); + }); + + test("takes the raw-digest path instead of the client's typed-data path", async () => { + const account = privateKeyToAccount(PRIVATE_KEY); + let accountSignCalls = 0; + const spied = new Proxy(account, { + get(target, prop, receiver) { + if (prop === "sign") accountSignCalls++; + return Reflect.get(target, prop, receiver); + }, + }); + const client = createWalletClient({ account: spied, chain: arbitrum, transport: offlineTransport }); + + let clientTypedDataCalls = 0; + const originalSignTypedData = client.signTypedData.bind(client); + // Declared with one parameter on purpose: the JSON-RPC guard checks `signTypedData.length`, + // so a rest-args wrapper would make the client stop looking like a wallet at all. + client.signTypedData = ((params: Parameters[0]) => { + clientTypedDataCalls++; + return originalSignTypedData(params); + }) as typeof client.signTypedData; + + await signL1Action({ wallet: client, action: { type: "cancel", cancels: [] }, nonce: 1700000000000 }); + + assert(accountSignCalls > 0, "the embedded local account was never asked to sign the digest"); + assertEquals(clientTypedDataCalls, 0, "the client's typed-data path was used despite a local account"); + }); + + test("still reports the client's chain ID, not the local-account default", async () => { + const client = createWalletClient({ + account: privateKeyToAccount(PRIVATE_KEY), + chain: arbitrum, + transport: offlineTransport, + }); + + // A bare local account reports 0x1; routing through the account must not leak that default. + assertEquals(await getWalletChainId(client), "0xa4b1"); + assertEquals(await getWalletAddress(client), privateKeyToAccount(PRIVATE_KEY).address.toLowerCase()); + }); + + test("a client over a remote account keeps going through the client", async () => { + const address = privateKeyToAccount(PRIVATE_KEY).address; + let typedDataCalls = 0; + const client = createWalletClient({ + account: address, + chain: arbitrum, + transport: custom({ + request: async ({ method }: { method: string }) => { + if (method === "eth_chainId") return await Promise.resolve("0xa4b1"); + if (method === "eth_accounts") return await Promise.resolve([address]); + if (method === "eth_signTypedData_v4") { + typedDataCalls++; + return await Promise.resolve(`0x${"11".repeat(64)}1b`); + } + throw new Error(`Unexpected RPC method: ${method}`); + }, + }), + }); + + await signL1Action({ wallet: client, action: { type: "cancel", cancels: [] }, nonce: 1700000000000 }); + + assert(typedDataCalls > 0, "a remote account must still sign through the client's typed-data path"); + }); +}); + // ============================================================ // Wallet error wrapping // ============================================================ From 80e3e3fbf90d087d0b5b83ac1d88d4d0f9523be9 Mon Sep 17 00:00:00 2001 From: Joe Blau Date: Mon, 27 Jul 2026 21:20:44 -0700 Subject: [PATCH 2/2] fix(signing): act on the pre-publish review of the wallet fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An adversarial review of c76e21e confirmed three low-severity issues, all in the new viem-resolution path, plus stale comments the routing change invalidated. - The error thrown when `viem/accounts` cannot be loaded asserted one cause ("this environment cannot service a dynamic import") and gave a remedy — install tiny-secp256k1 — that is impossible on one of its two call sites: the `signTypedData` delegate only exists when tiny-secp256k1 already loaded. It now names which path needed viem and offers both real remedies, since viem simply not being installed is the likelier cause. - An injected `options.privateKeyToAccount` was never checked against the key the wallet signs with. On the WASM path `address` and the raw-digest signer come from tiny-secp256k1 over the private key while `signTypedData` delegates to the injected factory, so a factory closed over a different key — `() => myAccount` type-checks — would sign L1 actions and user-signed actions as two different accounts and report only one. The delegate's address is now checked before it is memoized. - `PrivateKeyToAccount` is the declared type of a public option but was not reachable from `@bloxwap/hyperliquid/signing`; it is now re-exported so consumers can name it. - Four comments stated "JSON-RPC wallets never have signDigest", which the embedded-local-account routing makes false. They now distinguish wallets that can only sign remotely. The review also reproduced a split-identity case: because the embedded account is captured when the adapter is memoized while address and chain ID stay live reads, a wallet object whose `account` changes could sign L1 actions with the old key. Left as-is deliberately — verification downgraded it to low after establishing that viem's own bound actions close over the client at creation, so reassigning `client.account` is already a no-op for viem itself, and no real wallet or library produces the dynamic-`account` shape the other direction needs. Co-Authored-By: Claude Fable 5 --- src/signing/_abstractWallet.ts | 10 +++--- src/signing/_fastWallet.ts | 53 ++++++++++++++++++++++++++------ src/signing/_l1.ts | 2 +- src/signing/_multiSig.ts | 2 +- src/signing/mod.ts | 2 +- tests/signing/fastWallet.test.ts | 13 ++++++++ 6 files changed, 66 insertions(+), 16 deletions(-) diff --git a/src/signing/_abstractWallet.ts b/src/signing/_abstractWallet.ts index 7f75c690..4aaef3a3 100644 --- a/src/signing/_abstractWallet.ts +++ b/src/signing/_abstractWallet.ts @@ -61,9 +61,10 @@ interface Signer { signTypedData(args: TypedDataArgs): Promise; /** * Sign a raw 32-byte digest directly, skipping EIP-712 encoding entirely. Present only when the - * wallet can do so locally (viem local accounts expose `sign`); JSON-RPC wallets never have it, - * so their behavior is unchanged. Takes the digest as bytes so the L1 path avoids a hex round - * trip per signature; adapters whose wallet speaks hex (`sign({ hash })`) convert here. + * wallet can do so locally: a viem local account exposes `sign`, and a JSON-RPC-shaped wallet + * has it too when it wraps one (see {@linkcode embeddedLocalAccount}). A wallet that signs only + * through a remote endpoint never has it. Takes the digest as bytes so the L1 path avoids a hex + * round trip per signature; adapters whose wallet speaks hex (`sign({ hash })`) convert here. */ signDigest?(digest: Uint8Array): Promise; /** Lowercase wallet address. */ @@ -442,7 +443,8 @@ export async function signTypedData(args: { /** * Signs a raw 32-byte digest directly when the wallet supports it (a viem local account exposing * `sign`), bypassing EIP-712 encoding. Returns `undefined` for wallets without that capability — - * JSON-RPC wallets among them — so the caller can fall back to {@linkcode signTypedData}. + * wallets that can only sign remotely among them — so the caller can fall back to + * {@linkcode signTypedData}. * * Internal to the signing module: the caller is responsible for computing a digest that is * byte-identical to what the typed-data path would have produced. diff --git a/src/signing/_fastWallet.ts b/src/signing/_fastWallet.ts index 72b694f7..4934db76 100644 --- a/src/signing/_fastWallet.ts +++ b/src/signing/_fastWallet.ts @@ -91,23 +91,48 @@ export type PrivateKeyToAccount = (privateKey: `0x${string}`) => AbstractViemLoc * When neither is possible the failure is reported for what it is, rather than surfacing the * host's opaque message from somewhere deep in the signing path. */ -async function resolvePrivateKeyToAccount(provided: PrivateKeyToAccount | undefined): Promise { +async function resolvePrivateKeyToAccount( + provided: PrivateKeyToAccount | undefined, + need: "the tiny-secp256k1 fallback" | "signTypedData", +): Promise { if (provided !== undefined) return provided; try { const { privateKeyToAccount } = await import("viem/accounts"); return privateKeyToAccount as PrivateKeyToAccount; } catch (cause) { + // Two very different situations land here and the remedy differs, so name both rather than + // asserting one: viem may be absent (it is not a dependency, so this is the common case), or + // present but unreachable because the host cannot service a dynamic import. throw new AbstractWalletError( - "createFastLocalWallet: could not load `viem/accounts`. It is imported dynamically because " + - "viem is not a dependency of this package, and this environment cannot service a dynamic " + - "import (Jest without `--experimental-vm-modules` is the usual cause). Import " + + `createFastLocalWallet: ${need} needs \`viem/accounts\`, which could not be loaded. ` + + "Either viem is not installed (it is not a dependency of this package — install it), or " + + "this environment cannot service the dynamic import used to reach it (Jest without " + + "`--experimental-vm-modules` is the usual cause), in which case import " + "`privateKeyToAccount` from `viem/accounts` yourself and pass it as " + - "`options.privateKeyToAccount`, or install `tiny-secp256k1` so signing never needs viem.", + "`options.privateKeyToAccount`. See the underlying error on `cause`.", { cause }, ); } } +/** + * Guards against a supplied factory that ignores the key it is handed. + * + * The natural mistake in exactly the hosts `options.privateKeyToAccount` exists for is passing a + * bound or curried helper closed over a different key — `() => myAccount` type-checks. On the WASM + * path the wallet's `address` and raw-digest signer come from `tiny-secp256k1` over `privateKey` + * while `signTypedData` delegates to this factory, so a mismatch would sign L1 actions and + * user-signed actions with two different keys and report only one of them. + */ +function assertDelegateMatches(delegate: AbstractViemLocalAccount, expected: `0x${string}`): void { + if (delegate.address?.toLowerCase() !== expected.toLowerCase()) { + throw new AbstractWalletError( + `createFastLocalWallet: options.privateKeyToAccount returned an account for ${delegate.address}, ` + + `but this wallet signs as ${expected}. It must derive the account from the private key it is given.`, + ); + } +} + /** * Internal test hook: swaps the `tiny-secp256k1` loader (pass `undefined` to restore the real one) * and resets the one-time warning latch. Module mocking cannot reliably simulate a missing @@ -210,7 +235,10 @@ export async function createFastLocalWallet( "falling back to the viem/noble signing path. Install `tiny-secp256k1` to enable WASM acceleration.", ); } - const privateKeyToAccount = await resolvePrivateKeyToAccount(options?.privateKeyToAccount); + const privateKeyToAccount = await resolvePrivateKeyToAccount( + options?.privateKeyToAccount, + "the tiny-secp256k1 fallback", + ); return privateKeyToAccount(privateKey); } @@ -218,12 +246,19 @@ export async function createFastLocalWallet( throw new AbstractWalletError("Private key is outside the secp256k1 scalar range"); } + // Derived once, and also the identity any `signTypedData` delegate must agree with. + const address = deriveAddress(ecc, privateKeyBytes); + // viem is needed only for `signTypedData` (user-signed actions); a pure-L1 wallet never pays for it. let delegate: AbstractViemLocalAccount | undefined; const viemDelegate = async (): Promise => { if (delegate === undefined) { - const privateKeyToAccount = await resolvePrivateKeyToAccount(options?.privateKeyToAccount); - delegate = privateKeyToAccount(privateKey); + const privateKeyToAccount = await resolvePrivateKeyToAccount(options?.privateKeyToAccount, "signTypedData"); + const resolved = privateKeyToAccount(privateKey); + // The WASM path derives `address` and the digest signer itself, so a delegate for a + // different key would split this wallet's identity in two. Checked before it is memoized. + assertDelegateMatches(resolved, address); + delegate = resolved; } return delegate; }; @@ -238,7 +273,7 @@ export async function createFastLocalWallet( }; const account: AbstractViemLocalAccount & DigestBytesCapable = { - address: deriveAddress(ecc, privateKeyBytes), + address, async sign({ hash }: { hash: `0x${string}` }): Promise<`0x${string}`> { return signDigestBytes(hexToBytes(hash.slice(2))); }, diff --git a/src/signing/_l1.ts b/src/signing/_l1.ts index 1466b134..66719b6f 100644 --- a/src/signing/_l1.ts +++ b/src/signing/_l1.ts @@ -349,7 +349,7 @@ async function signL1ActionHash(args: { // Fast path: a wallet that can sign a raw digest (viem local accounts expose `sign`) signs the // hand-rolled Agent digest directly and skips viem's whole EIP-712 encoding. The digest is // byte-identical — the differential test pins it against viem's `hashTypedData`. The thunk keeps - // it lazy: wallets without the capability (every JSON-RPC wallet) fall through to the unchanged + // it lazy: wallets without the capability (those that can only sign remotely) fall through to the unchanged // typed-data path without paying for a digest they would discard. const fast = await signRawDigestBytes({ wallet, diff --git a/src/signing/_multiSig.ts b/src/signing/_multiSig.ts index bbe04dc9..399ea071 100644 --- a/src/signing/_multiSig.ts +++ b/src/signing/_multiSig.ts @@ -97,7 +97,7 @@ async function signMultiSigOuter(args: { // Fast path: a wallet that can sign a raw digest (viem local accounts expose `sign`) signs the // hand-rolled SendMultiSig digest directly and skips viem's whole EIP-712 encoding. The digest // is byte-identical — the differential test pins it against viem's `hashTypedData`. The thunk - // keeps it lazy: wallets without the capability (every JSON-RPC wallet) fall through to the + // keeps it lazy: wallets without the capability (those that can only sign remotely) fall through to the // unchanged typed-data path without paying for a digest they would discard. A chain ID that did // not parse to a uint256 word falls through too: viem rejects it, while the hand-rolled digest // would silently sign under the wrong domain. diff --git a/src/signing/mod.ts b/src/signing/mod.ts index 08f51665..51121b02 100644 --- a/src/signing/mod.ts +++ b/src/signing/mod.ts @@ -13,7 +13,7 @@ export { type Signature, } from "./_abstractWallet.ts"; export { canonicalize, CanonicalizeError } from "./_canonicalize.ts"; -export { createFastLocalWallet } from "./_fastWallet.ts"; +export { createFastLocalWallet, type PrivateKeyToAccount } from "./_fastWallet.ts"; export { createL1ActionHash, signL1Action } from "./_l1.ts"; export { signUserSignedAction } from "./_userSigned.ts"; export { signMultiSigL1, signMultiSigUserSigned } from "./_multiSig.ts"; diff --git a/tests/signing/fastWallet.test.ts b/tests/signing/fastWallet.test.ts index d734f8d8..5fc9c185 100644 --- a/tests/signing/fastWallet.test.ts +++ b/tests/signing/fastWallet.test.ts @@ -294,6 +294,19 @@ describe("createFastLocalWallet() fallback", () => { ); }); + test("rejects an injected factory that ignores the private key it is given", async () => { + // `() => someOtherAccount` type-checks, and on the WASM path it would split the wallet: + // L1 digests signed by the WASM key, typed data by the injected account's key. + const wrongKeyFactory = (): ReturnType => privateKeyToAccount(PRIVATE_KEYS[1]); + const fast = await createFastLocalWallet(PRIVATE_KEYS[0], { privateKeyToAccount: wrongKeyFactory }); + + // The wallet itself is fine; only the typed-data delegate is wrong, so it fails there. + expect(fast.address).toBe(privateKeyToAccount(PRIVATE_KEYS[0]).address); + await expect( + signUserSignedAction({ wallet: fast, action: { ...APPROVE_AGENT }, types: ApproveAgentTypes }), + ).rejects.toThrow("must derive the account from the private key it is given"); + }); + test("`wasm: false` takes the viem/noble path without loading or warning", async () => { const warn = spyOn(console, "warn").mockImplementation(() => {}); try {