fix!: Keyring.signTypedData accepts types for V1

packages/keyring-eth-hd/CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- **BREAKING:** The method signature for `signTypedData` has been changed ([#224](https://github.com/MetaMask/accounts/pull/224))
+  - The method now accepts a `TypedDataV1` object when `SigntypedDataVersion.V1` is passed in the options, and `TypedMessage<Types>` when other versions are requested
+
 ## [10.0.1]
 
 ### Changed

packages/keyring-eth-hd/src/index.ts

@@ -349,16 +349,19 @@ class HdKeyring {
    * @param opts - The options for signing the message.
    * @returns The signature of the message.
    */
-  async signTypedData<Types extends MessageTypes>(
+  async signTypedData<
+    Version extends SignTypedDataVersion,
+    Types extends MessageTypes,
+    Options extends { version: Version },
+  >(
     withAccount: Hex,
-    typedData: TypedDataV1 | TypedMessage<Types>,
-    opts: HDKeyringAccountSelectionOptions & {
-      version: SignTypedDataVersion;
-    } = { version: SignTypedDataVersion.V1 },
+    typedData: Version extends 'V1' ? TypedDataV1 : TypedMessage<Types>,
+    opts?: HDKeyringAccountSelectionOptions & Options,
   ): Promise<string> {
+    const options = opts ?? { version: SignTypedDataVersion.V1 };
     // Treat invalid versions as "V1"
-    const version = Object.keys(SignTypedDataVersion).includes(opts.version)
-      ? opts.version
+    const version = Object.keys(SignTypedDataVersion).includes(options.version)
+      ? options.version
       : SignTypedDataVersion.V1;
 
     const privateKey = this.#getPrivateKeyFor(withAccount, opts);

packages/keyring-eth-ledger-bridge/CHANGELOG.md

@@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - The `signPersonalMessage` method now accepts an `Hex` typed value as the `withAccount` parameter.
   - The `signTypedData` method now accepts an `Hex` typed value as the `withAccount` parameter.
   - The `unlockAccountByAddress` method now accepts an `Hex` typed value as the `address` parameter.
+- **BREAKING:** The `signTypedData` method now requires `SignTypedDataVersion` as version for the `options` argument ([#224](https://github.com/MetaMask/accounts/pull/224)).
 
 ### Removed
 

packages/keyring-eth-ledger-bridge/jest.config.js

@@ -23,10 +23,10 @@ module.exports = merge(baseConfig, {
   // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
-      branches: 90.78,
+      branches: 90.97,
       functions: 96,
-      lines: 95.02,
-      statements: 95.07,
+      lines: 95.03,
+      statements: 95.09,
     },
   },
 });

packages/keyring-eth-ledger-bridge/src/ledger-keyring.test.ts

@@ -968,7 +968,7 @@ describe('LedgerKeyring', function () {
           ],
         },
       };
-      const options = { version: 'V4' };
+      const options = { version: sigUtil.SignTypedDataVersion.V4 };
 
       beforeEach(async function () {
         jest
@@ -1043,7 +1043,7 @@ describe('LedgerKeyring', function () {
       it('throws an error if the signTypedData version is not v4', async function () {
         await expect(
           keyring.signTypedData(fakeAccounts[0], fixtureData, {
-            version: 'V3',
+            version: sigUtil.SignTypedDataVersion.V3,
           }),
         ).rejects.toThrow(
           'Ledger: Only version 4 of typed data signing is supported',

packages/keyring-eth-ledger-bridge/src/ledger-keyring.ts

@@ -484,12 +484,17 @@ export class LedgerKeyring implements Keyring {
     return hdPath;
   }
 
-  async signTypedData<T extends MessageTypes>(
+  async signTypedData<
+    Version extends SignTypedDataVersion,
+    Types extends MessageTypes,
+    Options extends { version?: Version },
+  >(
     withAccount: Hex,
-    data: TypedMessage<T>,
-    options: { version?: string } = {},
+    data: TypedMessage<Types>,
+    options?: Options,
   ): Promise<string> {
-    const isV4 = options.version === 'V4';
+    const { version } = options ?? {};
+    const isV4 = version === 'V4';
     if (!isV4) {
       throw new Error(
         'Ledger: Only version 4 of typed data signing is supported',

packages/keyring-eth-simple/CHANGELOG.md

@@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - **BREAKING:** The `SimpleKeyring` class now implements `Keyring` from `@metamask/keyring-utils` ([#217](https://github.com/MetaMask/accounts/pull/217))
   - The `deserialize` method now requires a `string[]` argument.
+- **BREAKING:** The method signature for `signTypedData` has been changed ([#224](https://github.com/MetaMask/accounts/pull/224))
+  - The method now accepts a `TypedDataV1` object when `SigntypedDataVersion.V1` is passed in the options, and `TypedMessage<Types>` when other versions are requested
 
 ## [8.1.1]
 

packages/keyring-eth-simple/src/simple-keyring.test.ts

@@ -360,6 +360,7 @@ describe('simple-keyring', function () {
     it('returns the expected value if invalid version is given', async function () {
       await keyring.deserialize([privKeyHex]);
       const signature = await keyring.signTypedData(address, typedData, {
+        // @ts-expect-error: intentionally passing invalid version
         version: 'FOO',
       });
       expect(signature).toBe(expectedSignature);
@@ -390,7 +391,7 @@ describe('simple-keyring', function () {
     it('works via `V1` string', async function () {
       await keyring.deserialize([privKeyHex]);
       const signature = await keyring.signTypedData(address, typedData, {
-        version: 'V1',
+        version: SignTypedDataVersion.V1,
       });
       expect(signature).toBe(expectedSignature);
       const restored = recoverTypedSignature({
@@ -446,7 +447,7 @@ describe('simple-keyring', function () {
 
       await keyring.deserialize([privKeyHex]);
       const signature = await keyring.signTypedData(address, typedData, {
-        version: 'V3',
+        version: SignTypedDataVersion.V3,
       });
       const restored = recoverTypedSignature({
         data: typedData,
@@ -506,7 +507,7 @@ describe('simple-keyring', function () {
       const address = (await keyring.getAccounts())[0];
       assert(address, 'address is undefined');
       const signature = await keyring.signTypedData(address, typedData, {
-        version: 'V3',
+        version: SignTypedDataVersion.V3,
       });
       expect(signature).toBe(expectedSignature);
       const restored = recoverTypedSignature({
@@ -535,7 +536,7 @@ describe('simple-keyring', function () {
 
       await keyring.deserialize([privKeyHex]);
       const signature = await keyring.signTypedData(address, typedData, {
-        version: 'V4',
+        version: SignTypedDataVersion.V4,
       });
       const restored = recoverTypedSignature({
         data: typedData,
@@ -734,7 +735,7 @@ describe('simple-keyring', function () {
       const address = (await keyring.getAccounts())[0];
       assert(address, 'address is undefined');
       const signature = await keyring.signTypedData(address, typedData, {
-        version: 'V4',
+        version: SignTypedDataVersion.V4,
       });
       expect(signature).toBe(expectedSignature);
       const restored = recoverTypedSignature({

packages/keyring-eth-simple/src/simple-keyring.ts

@@ -10,6 +10,9 @@ import {
   toBuffer,
 } from '@ethereumjs/util';
 import {
+  type TypedDataV1,
+  type MessageTypes,
+  type TypedMessage,
   concatSig,
   decrypt,
   EIP7702Authorization,
@@ -147,17 +150,21 @@ export default class SimpleKeyring implements Keyring {
     return decrypt({ privateKey, encryptedData });
   }
 
-  // personal_signTypedData, signs data along with the schema
-  async signTypedData(
+  async signTypedData<
+    Version extends SignTypedDataVersion,
+    Types extends MessageTypes,
+    Options extends { version?: Version } & KeyringOpt,
+  >(
     address: Hex,
-    typedData: any,
-    opts: KeyringOpt = { version: SignTypedDataVersion.V1 },
+    typedData: Version extends 'V1' ? TypedDataV1 : TypedMessage<Types>,
+    opts?: Options,
   ): Promise<string> {
+    const options = opts ?? { version: SignTypedDataVersion.V1 };
     // Treat invalid versions as "V1"
     let version = SignTypedDataVersion.V1;
 
-    if (opts.version && isSignTypedDataVersion(opts.version)) {
-      version = SignTypedDataVersion[opts.version];
+    if (options.version && isSignTypedDataVersion(options.version)) {
+      version = SignTypedDataVersion[options.version];
     }
 
     const privateKey = this.#getPrivateKeyFor(address, opts);

packages/keyring-eth-trezor/CHANGELOG.md

@@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - The `signPersonalMessage` method now accepts an `Hex` typed value as the `withAccount` parameter.
   - The `signTypedData` method now accepts an `Hex` typed value as the `withAccount` parameter.
   - The `unlockAccountByAddress` method now accepts an `Hex` typed value as the `address` parameter.
+- **BREAKING:** The `signTypedData` `options.version` argument type has been restricted to accept `SignTypedDataVersion.V3 | SignTypedDataVersion.V4` ([#224](https://github.com/MetaMask/accounts/pull/224))
 
 ### Removed
 

packages/keyring-eth-trezor/src/trezor-keyring.ts

@@ -453,10 +453,14 @@ export class TrezorKeyring implements Keyring {
   }
 
   // EIP-712 Sign Typed Data
-  async signTypedData<T extends MessageTypes>(
+  async signTypedData<
+    Version extends SignTypedDataVersion.V3 | SignTypedDataVersion.V4,
+    Types extends MessageTypes,
+    Options extends { version?: Version },
+  >(
     address: Hex,
-    data: TypedMessage<T>,
-    { version }: { version: SignTypedDataVersion },
+    data: TypedMessage<Types>,
+    { version }: Options,
   ): Promise<string> {
     const dataWithHashes = transformTypedData(
       data,

packages/keyring-utils/src/keyring.ts

@@ -229,7 +229,7 @@ export type Keyring = {
    */
   signTypedData?(
     address: Hex,
-    typedData: Record<string, unknown>,
+    typedData: unknown[] | Record<string, unknown>,
     options?: Record<string, unknown>,
   ): Promise<string>;