diff --git a/packages/keyring-eth-hd/CHANGELOG.md b/packages/keyring-eth-hd/CHANGELOG.md index 0945a256..ff33e3cd 100644 --- a/packages/keyring-eth-hd/CHANGELOG.md +++ b/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. + ## [12.1.0] ### Added diff --git a/packages/keyring-eth-hd/jest.config.js b/packages/keyring-eth-hd/jest.config.js index e5d1182c..46378eee 100644 --- a/packages/keyring-eth-hd/jest.config.js +++ b/packages/keyring-eth-hd/jest.config.js @@ -26,7 +26,7 @@ module.exports = merge(baseConfig, { branches: 83.87, functions: 100, lines: 95, - statements: 97.91, + statements: 97.27, }, }, }); diff --git a/packages/keyring-eth-hd/src/hd-keyring.ts b/packages/keyring-eth-hd/src/hd-keyring.ts index 36fbbecd..a97475b7 100644 --- a/packages/keyring-eth-hd/src/hd-keyring.ts +++ b/packages/keyring-eth-hd/src/hd-keyring.ts @@ -341,27 +341,31 @@ export class HdKeyring { * Sign a typed message using the private key of the specified account. * This method is compatible with the `eth_signTypedData` RPC method. * - * @param withAccount - The address of the account. - * @param typedData - The typed data to sign. - * @param opts - The options for signing the message. + * @param address - The address of the account. + * @param data - The typed data to sign. + * @param options - The options for signing the message. * @returns The signature of the message. */ - async signTypedData<Types extends MessageTypes>( - withAccount: Hex, - typedData: TypedDataV1 | TypedMessage<Types>, - opts: HDKeyringAccountSelectionOptions & { - version: SignTypedDataVersion; - } = { version: SignTypedDataVersion.V1 }, + async signTypedData< + Version extends SignTypedDataVersion, + Types extends MessageTypes, + Options extends { version?: Version }, + >( + address: Hex, + data: Version extends 'V1' ? TypedDataV1 : TypedMessage<Types>, + options?: HDKeyringAccountSelectionOptions & Options, ): Promise<string> { + let { version } = options ?? { version: SignTypedDataVersion.V1 }; + // Treat invalid versions as "V1" - const version = Object.keys(SignTypedDataVersion).includes(opts.version) - ? opts.version - : SignTypedDataVersion.V1; + if (!version || !Object.keys(SignTypedDataVersion).includes(version)) { + version = SignTypedDataVersion.V1; + } - const privateKey = this.#getPrivateKeyFor(withAccount, opts); + const privateKey = this.#getPrivateKeyFor(address, options); return signTypedData({ privateKey: Buffer.from(privateKey), - data: typedData, + data, version, }); } diff --git a/packages/keyring-eth-ledger-bridge/CHANGELOG.md b/packages/keyring-eth-ledger-bridge/CHANGELOG.md index 28a990e3..874032bf 100644 --- a/packages/keyring-eth-ledger-bridge/CHANGELOG.md +++ b/packages/keyring-eth-ledger-bridge/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **BREAKING:** The `signTypedData` method now requires `SignTypedDataVersion.V4` as version for the `options` argument ([#224](https://github.com/MetaMask/accounts/pull/224)) + ## [10.0.0] ### Changed diff --git a/packages/keyring-eth-ledger-bridge/jest.config.js b/packages/keyring-eth-ledger-bridge/jest.config.js index 8e17553d..5fb1fcc0 100644 --- a/packages/keyring-eth-ledger-bridge/jest.config.js +++ b/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, }, }, }); diff --git a/packages/keyring-eth-ledger-bridge/src/ledger-keyring.test.ts b/packages/keyring-eth-ledger-bridge/src/ledger-keyring.test.ts index 18e8fbac..446d3d46 100644 --- a/packages/keyring-eth-ledger-bridge/src/ledger-keyring.test.ts +++ b/packages/keyring-eth-ledger-bridge/src/ledger-keyring.test.ts @@ -968,7 +968,6 @@ describe('LedgerKeyring', function () { ], }, }; - const options = { version: 'V4' }; beforeEach(async function () { jest @@ -989,7 +988,7 @@ describe('LedgerKeyring', function () { const result = await keyring.signTypedData( fakeAccounts[15], fixtureData, - options, + { version: sigUtil.SignTypedDataVersion.V4 }, ); expect(result).toBe( '0x72d4e38a0e582e09a620fd38e236fe687a1ec782206b56d576f579c026a7e5b946759735981cd0c3efb02d36df28bb2feedfec3d90e408efc93f45b894946e321b', @@ -1016,7 +1015,7 @@ describe('LedgerKeyring', function () { const result = await keyring.signTypedData( fakeAccounts[15], fixtureDataWithoutSalt, - options, + { version: sigUtil.SignTypedDataVersion.V4 }, ); expect(result).toBe( '0x72d4e38a0e582e09a620fd38e236fe687a1ec782206b56d576f579c026a7e5b946759735981cd0c3efb02d36df28bb2feedfec3d90e408efc93f45b894946e321b', @@ -1034,7 +1033,9 @@ describe('LedgerKeyring', function () { })); await expect( - keyring.signTypedData(fakeAccounts[15], fixtureData, options), + keyring.signTypedData(fakeAccounts[15], fixtureData, { + version: sigUtil.SignTypedDataVersion.V4, + }), ).rejects.toThrow( 'Ledger: The signature doesnt match the right address', ); @@ -1043,7 +1044,8 @@ 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', + // @ts-expect-error we want to test an invalid version + version: sigUtil.SignTypedDataVersion.V3, }), ).rejects.toThrow( 'Ledger: Only version 4 of typed data signing is supported', @@ -1063,7 +1065,9 @@ describe('LedgerKeyring', function () { .spyOn(keyring, 'unlockAccountByAddress') .mockResolvedValue(undefined); await expect( - keyring.signTypedData(fakeAccounts[0], fixtureData, options), + keyring.signTypedData(fakeAccounts[0], fixtureData, { + version: sigUtil.SignTypedDataVersion.V4, + }), ).rejects.toThrow('Ledger: Unknown error while signing message'); }); @@ -1073,7 +1077,9 @@ describe('LedgerKeyring', function () { .mockRejectedValue(new Error('some error')); await expect( - keyring.signTypedData(fakeAccounts[15], fixtureData, options), + keyring.signTypedData(fakeAccounts[15], fixtureData, { + version: sigUtil.SignTypedDataVersion.V4, + }), ).rejects.toThrow('some error'); }); @@ -1083,7 +1089,9 @@ describe('LedgerKeyring', function () { .mockRejectedValue('some error'); await expect( - keyring.signTypedData(fakeAccounts[15], fixtureData, options), + keyring.signTypedData(fakeAccounts[15], fixtureData, { + version: sigUtil.SignTypedDataVersion.V4, + }), ).rejects.toThrow('Ledger: Unknown error while signing message'); }); @@ -1096,7 +1104,9 @@ describe('LedgerKeyring', function () { const result = await keyring.signTypedData( fakeAccounts[15], fixtureData, - options, + { + version: sigUtil.SignTypedDataVersion.V4, + }, ); expect(result).toBe( '0x72d4e38a0e582e09a620fd38e236fe687a1ec782206b56d576f579c026a7e5b946759735981cd0c3efb02d36df28bb2feedfec3d90e408efc93f45b894946e3200', diff --git a/packages/keyring-eth-ledger-bridge/src/ledger-keyring.ts b/packages/keyring-eth-ledger-bridge/src/ledger-keyring.ts index ab123efd..da285a27 100644 --- a/packages/keyring-eth-ledger-bridge/src/ledger-keyring.ts +++ b/packages/keyring-eth-ledger-bridge/src/ledger-keyring.ts @@ -493,12 +493,17 @@ export class LedgerKeyring implements Keyring { return hdPath; } - async signTypedData<T extends MessageTypes>( + async signTypedData< + Version extends SignTypedDataVersion.V4, + 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', diff --git a/packages/keyring-eth-simple/CHANGELOG.md b/packages/keyring-eth-simple/CHANGELOG.md index 6761e5f4..70b367cb 100644 --- a/packages/keyring-eth-simple/CHANGELOG.md +++ b/packages/keyring-eth-simple/CHANGELOG.md @@ -7,6 +7,12 @@ 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. + - The `options` argument type has been changed to `{ version: SignTypedDataVersion } | undefined`. + ## [10.0.0] ### Changed diff --git a/packages/keyring-eth-simple/src/simple-keyring.test.ts b/packages/keyring-eth-simple/src/simple-keyring.test.ts index 3eaab107..dc021964 100644 --- a/packages/keyring-eth-simple/src/simple-keyring.test.ts +++ b/packages/keyring-eth-simple/src/simple-keyring.test.ts @@ -359,6 +359,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); @@ -389,7 +390,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({ @@ -445,7 +446,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, @@ -505,7 +506,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({ @@ -534,7 +535,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, @@ -725,7 +726,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({ diff --git a/packages/keyring-eth-simple/src/simple-keyring.ts b/packages/keyring-eth-simple/src/simple-keyring.ts index ff0ee7b6..85ba37e8 100644 --- a/packages/keyring-eth-simple/src/simple-keyring.ts +++ b/packages/keyring-eth-simple/src/simple-keyring.ts @@ -7,6 +7,9 @@ import { stripHexPrefix, } from '@ethereumjs/util'; import { + type TypedDataV1, + type MessageTypes, + type TypedMessage, concatSig, decrypt, EIP7702Authorization, @@ -154,21 +157,24 @@ 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 }, + data: Version extends 'V1' ? TypedDataV1 : TypedMessage<Types>, + options?: Options, ): Promise<string> { - // Treat invalid versions as "V1" - let version = SignTypedDataVersion.V1; + let { version } = options ?? { version: SignTypedDataVersion.V1 }; - if (opts.version && isSignTypedDataVersion(opts.version)) { - version = SignTypedDataVersion[opts.version]; + // Treat invalid versions as "V1" + if (!version || !isSignTypedDataVersion(version)) { + version = SignTypedDataVersion.V1; } - const privateKey = this.#getPrivateKeyFor(address, opts); - return signTypedData({ privateKey, data: typedData, version }); + const privateKey = this.#getPrivateKeyFor(address, options); + return signTypedData({ privateKey, data, version }); } // get public key for nacl diff --git a/packages/keyring-eth-trezor/CHANGELOG.md b/packages/keyring-eth-trezor/CHANGELOG.md index cc0cedbd..a55fe1d2 100644 --- a/packages/keyring-eth-trezor/CHANGELOG.md +++ b/packages/keyring-eth-trezor/CHANGELOG.md @@ -7,6 +7,12 @@ 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 `options` argument type has been changed to `{ version: SignTypedDataVersion.V3 | SignTypedDataVersion.V4 } | undefined`. + - The `options.version` argument type has been restricted to accept `SignTypedDataVersion.V3 | SignTypedDataVersion.V4` ([#224](https://github.com/MetaMask/accounts/pull/224)) + ## [8.0.0] ### Changed diff --git a/packages/keyring-eth-trezor/jest.config.js b/packages/keyring-eth-trezor/jest.config.js index 34ecf687..0bb9dd48 100644 --- a/packages/keyring-eth-trezor/jest.config.js +++ b/packages/keyring-eth-trezor/jest.config.js @@ -23,10 +23,10 @@ module.exports = merge(baseConfig, { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 51.63, + branches: 52.38, functions: 91.22, - lines: 90.1, - statements: 90.3, + lines: 90.15, + statements: 90.35, }, }, }); diff --git a/packages/keyring-eth-trezor/src/trezor-keyring.ts b/packages/keyring-eth-trezor/src/trezor-keyring.ts index 7d1fe5df..fcf33aaf 100644 --- a/packages/keyring-eth-trezor/src/trezor-keyring.ts +++ b/packages/keyring-eth-trezor/src/trezor-keyring.ts @@ -459,11 +459,17 @@ 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>, + options?: Options, ): Promise<string> { + const { version } = options ?? { version: SignTypedDataVersion.V4 }; + const dataWithHashes = transformTypedData( data, version === SignTypedDataVersion.V4, diff --git a/packages/keyring-utils/src/keyring.ts b/packages/keyring-utils/src/keyring.ts index 8be5157d..da482b71 100644 --- a/packages/keyring-utils/src/keyring.ts +++ b/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>;