diff --git a/packages/message-manager/CHANGELOG.md b/packages/message-manager/CHANGELOG.md index c84190639a..634320e5df 100644 --- a/packages/message-manager/CHANGELOG.md +++ b/packages/message-manager/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Removed + +- Remove all code related to `@metamask/signature-controller` ([#4785](https://github.com/MetaMask/core/pull/4785)) + - Remove `TypedMessageManager`. + - Remove `PersonalMessageManager`. + - Remove utils: + - `validateSignMessageData` + - `validateTypedSignMessageDataV1` + - `validateTypedSignMessageDataV3V4` + ## [10.1.1] ### Fixed diff --git a/packages/message-manager/src/AbstractMessageManager.test.ts b/packages/message-manager/src/AbstractMessageManager.test.ts index c6362815ec..740ed38e2c 100644 --- a/packages/message-manager/src/AbstractMessageManager.test.ts +++ b/packages/message-manager/src/AbstractMessageManager.test.ts @@ -1,21 +1,29 @@ import { ApprovalType } from '@metamask/controller-utils'; import type { + AbstractMessage, AbstractMessageParams, OriginalRequest, SecurityProviderRequest, } from './AbstractMessageManager'; import { AbstractMessageManager } from './AbstractMessageManager'; -import type { - TypedMessage, - TypedMessageParams, - TypedMessageParamsMetamask, -} from './TypedMessageManager'; + +type ConcreteMessage = AbstractMessage & { + messageParams: ConcreteMessageParams; +}; + +type ConcreteMessageParams = AbstractMessageParams & { + test: number; +}; + +type ConcreteMessageParamsMetamask = ConcreteMessageParams & { + metamaskId?: string; +}; class AbstractTestManager extends AbstractMessageManager< - TypedMessage, - TypedMessageParams, - TypedMessageParamsMetamask + ConcreteMessage, + ConcreteMessageParams, + ConcreteMessageParamsMetamask > { addRequestToMessageParams( messageParams: MessageParams, @@ -33,10 +41,9 @@ class AbstractTestManager extends AbstractMessageManager< } prepMessageForSigning( - messageParams: TypedMessageParamsMetamask, - ): Promise { + messageParams: ConcreteMessageParamsMetamask, + ): Promise { delete messageParams.metamaskId; - delete messageParams.version; return Promise.resolve(messageParams); } @@ -44,29 +51,19 @@ class AbstractTestManager extends AbstractMessageManager< return super.setMessageStatus(messageId, status); } - async addUnapprovedMessage(_messageParams: TypedMessageParamsMetamask) { + async addUnapprovedMessage(_messageParams: ConcreteMessageParamsMetamask) { return Promise.resolve('mocked'); } } -const typedMessage = [ - { - name: 'Message', - type: 'string', - value: 'Hi, Alice!', - }, - { - name: 'A number', - type: 'uint32', - value: '1337', - }, -]; + const messageId = '1'; const messageId2 = '2'; const from = '0x0123'; const messageTime = Date.now(); const messageStatus = 'unapproved'; const messageType = 'eth_signTypedData'; -const messageData = typedMessage; +const testData = 123; +const testData2 = 456; const rawSigMock = '0xsignaturemocked'; const messageIdMock = 'message-id-mocked'; const fromMock = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; @@ -77,7 +74,7 @@ const mockRequest = { id: 123, securityAlertResponse: mockSecurityProviderResponse, }; -const mockMessageParams = { from, data: 'test' }; +const mockMessageParams = { from, test: testData }; describe('AbstractTestManager', () => { it('should set default state', () => { @@ -98,7 +95,7 @@ describe('AbstractTestManager', () => { await controller.addMessage({ id: messageId, messageParams: { - data: typedMessage, + test: testData, from, }, status: messageStatus, @@ -111,7 +108,7 @@ describe('AbstractTestManager', () => { } expect(message.id).toBe(messageId); expect(message.messageParams.from).toBe(from); - expect(message.messageParams.data).toBe(messageData); + expect(message.messageParams.test).toBe(testData); expect(message.time).toBe(messageTime); expect(message.status).toBe(messageStatus); expect(message.type).toBe(messageType); @@ -122,7 +119,7 @@ describe('AbstractTestManager', () => { const message = { id: messageId, messageParams: { - data: typedMessage, + test: testData, from, }, status: messageStatus, @@ -132,7 +129,7 @@ describe('AbstractTestManager', () => { const message2 = { id: messageId2, messageParams: { - data: typedMessage, + test: testData, from, }, status: messageStatus, @@ -159,7 +156,7 @@ describe('AbstractTestManager', () => { await controller.addMessage({ id: messageId, messageParams: { - data: typedMessage, + test: testData, from, }, status: messageStatus, @@ -173,7 +170,7 @@ describe('AbstractTestManager', () => { } expect(message.id).toBe(messageId); expect(message.messageParams.from).toBe(from); - expect(message.messageParams.data).toBe(messageData); + expect(message.messageParams.test).toBe(testData); expect(message.time).toBe(messageTime); expect(message.status).toBe(messageStatus); expect(message.type).toBe(messageType); @@ -187,7 +184,7 @@ describe('AbstractTestManager', () => { await controller.addMessage({ id: messageId, messageParams: { - data: typedMessage, + test: testData, from, }, status: messageStatus, @@ -207,7 +204,7 @@ describe('AbstractTestManager', () => { await controller.addMessage({ id: messageId, messageParams: { - data: typedMessage, + test: testData, from, }, status: messageStatus, @@ -233,7 +230,7 @@ describe('AbstractTestManager', () => { await controller.addMessage({ id: messageId, messageParams: { - data: typedMessage, + test: testData, from, }, status: messageStatus, @@ -258,7 +255,7 @@ describe('AbstractTestManager', () => { await controller.addMessage({ id: messageId, messageParams: { - data: typedMessage, + test: testData, from, }, status: messageStatus, @@ -274,40 +271,16 @@ describe('AbstractTestManager', () => { }); it('should get correct unapproved messages', async () => { - const firstMessageData = [ - { - name: 'Message', - type: 'string', - value: 'Hi, Alice!', - }, - { - name: 'A number', - type: 'uint32', - value: '1337', - }, - ]; - const secondMessageData = [ - { - name: 'Message', - type: 'string', - value: 'Hi, Alice!', - }, - { - name: 'A number', - type: 'uint32', - value: '1337', - }, - ]; const firstMessage = { id: '1', - messageParams: { from: '0x1', data: firstMessageData }, + messageParams: { from: '0x1', test: testData }, status: 'unapproved', time: 123, type: 'eth_signTypedData', }; const secondMessage = { id: '2', - messageParams: { from: '0x1', data: secondMessageData }, + messageParams: { from: '0x1', test: testData2 }, status: 'unapproved', time: 123, type: 'eth_signTypedData', @@ -322,10 +295,9 @@ describe('AbstractTestManager', () => { }); }); - it('should approve typed message', async () => { + it('should approve message', async () => { const controller = new AbstractTestManager(); - const firstMessage = { from: '0xfoO', data: typedMessage }; - const version = 'V1'; + const firstMessage = { from: '0xfoO', test: testData }; await controller.addMessage({ id: messageId, messageParams: firstMessage, @@ -336,7 +308,6 @@ describe('AbstractTestManager', () => { const messageParams = await controller.approveMessage({ ...firstMessage, metamaskId: messageId, - version, }); const message = controller.getMessage(messageId); expect(messageParams).toStrictEqual(firstMessage); diff --git a/packages/message-manager/src/PersonalMessageManager.test.ts b/packages/message-manager/src/PersonalMessageManager.test.ts deleted file mode 100644 index f5b6494758..0000000000 --- a/packages/message-manager/src/PersonalMessageManager.test.ts +++ /dev/null @@ -1,194 +0,0 @@ -import type { SIWEMessage } from '@metamask/controller-utils'; -import { detectSIWE } from '@metamask/controller-utils'; - -import { PersonalMessageManager } from './PersonalMessageManager'; - -jest.mock('@metamask/controller-utils', () => ({ - ...jest.requireActual('@metamask/controller-utils'), - detectSIWE: jest.fn(), -})); - -const siweMockNotFound = { - isSIWEMessage: false, - parsedMessage: null, -} as SIWEMessage; - -const siweMockFound = { - isSIWEMessage: true, - parsedMessage: { - address: '0x0000000', - domain: 'example.eth', - }, -} as SIWEMessage; - -describe('PersonalMessageManager', () => { - let controller: PersonalMessageManager; - - const detectSIWEMock = detectSIWE as jest.MockedFunction; - const fromMock = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - beforeEach(() => { - controller = new PersonalMessageManager(); - detectSIWEMock.mockReturnValue(siweMockNotFound); - }); - - it('should set default state', () => { - expect(controller.state).toStrictEqual({ - unapprovedMessages: {}, - unapprovedMessagesCount: 0, - }); - }); - - it('should set default config', () => { - expect(controller.config).toStrictEqual({}); - }); - - it('should add a valid message', async () => { - const messageId = '1'; - const from = '0x0123'; - const messageData = '0x123'; - const messageTime = Date.now(); - const messageStatus = 'unapproved'; - const messageType = 'personal_sign'; - await controller.addMessage({ - id: messageId, - messageParams: { - data: messageData, - from, - }, - status: messageStatus, - time: messageTime, - type: messageType, - }); - const message = controller.getMessage(messageId); - if (!message) { - throw new Error('"message" is falsy'); - } - expect(message.id).toBe(messageId); - expect(message.messageParams.from).toBe(from); - expect(message.messageParams.data).toBe(messageData); - expect(message.time).toBe(messageTime); - expect(message.status).toBe(messageStatus); - expect(message.type).toBe(messageType); - }); - - it('should add a valid unapproved message', async () => { - const messageStatus = 'unapproved'; - const messageType = 'personal_sign'; - const messageParams = { - data: '0x123', - from: fromMock, - }; - const originalRequest = { - id: 111, - origin: 'origin', - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - securityAlertResponse: { result_type: 'result_type', reason: 'reason' }, - }; - const messageId = await controller.addUnapprovedMessage( - messageParams, - originalRequest, - ); - expect(messageId).toBeDefined(); - const message = controller.getMessage(messageId); - if (!message) { - throw new Error('"message" is falsy'); - } - expect(message.messageParams.from).toBe(messageParams.from); - expect(message.messageParams.data).toBe(messageParams.data); - expect(message.messageParams.requestId).toBe(originalRequest.id); - expect(message.time).toBeDefined(); - expect(message.status).toBe(messageStatus); - expect(message.type).toBe(messageType); - expect(message.securityAlertResponse?.result_type).toBe('result_type'); - expect(message.securityAlertResponse?.reason).toBe('reason'); - }); - - it('should throw when adding invalid message', async () => { - const from = 'foo'; - const messageData = '0x123'; - await expect( - controller.addUnapprovedMessage({ - data: messageData, - from, - }), - ).rejects.toThrow( - `Invalid "from" address: ${from} must be a valid string.`, - ); - }); - - it('should get correct unapproved messages', async () => { - const firstMessage = { - id: '1', - messageParams: { from: '0x1', data: '0x123' }, - status: 'unapproved', - time: 123, - type: 'personal_sign', - }; - const secondMessage = { - id: '2', - messageParams: { from: '0x1', data: '0x321' }, - status: 'unapproved', - time: 123, - type: 'personal_sign', - }; - await controller.addMessage(firstMessage); - await controller.addMessage(secondMessage); - expect(controller.getUnapprovedMessagesCount()).toBe(2); - expect(controller.getUnapprovedMessages()).toStrictEqual({ - [firstMessage.id]: firstMessage, - [secondMessage.id]: secondMessage, - }); - }); - - it('should approve message', async () => { - const firstMessage = { from: fromMock, data: '0x123' }; - const messageId = await controller.addUnapprovedMessage(firstMessage); - const messageParams = await controller.approveMessage({ - ...firstMessage, - metamaskId: messageId, - }); - const message = controller.getMessage(messageId); - expect(messageParams).toStrictEqual(firstMessage); - if (!message) { - throw new Error('"message" is falsy'); - } - expect(message.status).toBe('approved'); - }); - - it('should set message status signed', async () => { - const firstMessage = { from: fromMock, data: '0x123' }; - const rawSig = '0x5f7a0'; - const messageId = await controller.addUnapprovedMessage(firstMessage); - - controller.setMessageStatusSigned(messageId, rawSig); - const message = controller.getMessage(messageId); - if (!message) { - throw new Error('"message" is falsy'); - } - expect(message.rawSig).toStrictEqual(rawSig); - expect(message.status).toBe('signed'); - }); - - it('should reject message', async () => { - const firstMessage = { from: fromMock, data: '0x123' }; - const messageId = await controller.addUnapprovedMessage(firstMessage); - controller.rejectMessage(messageId); - const message = controller.getMessage(messageId); - if (!message) { - throw new Error('"message" is falsy'); - } - expect(message.status).toBe('rejected'); - }); - - it('should add message including Ethereum sign in data', async () => { - detectSIWEMock.mockReturnValue(siweMockFound); - const firstMessage = { from: fromMock, data: '0x123' }; - const messageId = await controller.addUnapprovedMessage(firstMessage); - const message = controller.getMessage(messageId); - if (!message) { - throw new Error('"message" is falsy'); - } - expect(message.messageParams.siwe).toBe(siweMockFound); - }); -}); diff --git a/packages/message-manager/src/PersonalMessageManager.ts b/packages/message-manager/src/PersonalMessageManager.ts deleted file mode 100644 index c9fe134db4..0000000000 --- a/packages/message-manager/src/PersonalMessageManager.ts +++ /dev/null @@ -1,135 +0,0 @@ -import type { SIWEMessage } from '@metamask/controller-utils'; -import { detectSIWE, ApprovalType } from '@metamask/controller-utils'; - -import type { - AbstractMessage, - AbstractMessageParams, - AbstractMessageParamsMetamask, - OriginalRequest, -} from './AbstractMessageManager'; -import { AbstractMessageManager } from './AbstractMessageManager'; -import { normalizeMessageData, validateSignMessageData } from './utils'; - -/** - * @type Message - * - * Represents and contains data about a 'personal_sign' type signature request. - * These are created when a signature for a personal_sign call is requested. - * @property id - An id to track and identify the message object - * @property messageParams - The parameters to pass to the personal_sign method once the signature request is approved - * @property type - The json-prc signing method for which a signature request has been made. - * A 'Message' which always has a 'personal_sign' type - * @property rawSig - Raw data of the signature request - */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface PersonalMessage extends AbstractMessage { - messageParams: PersonalMessageParams; -} - -/** - * @type PersonalMessageParams - * - * Represents the parameters to pass to the personal_sign method once the signature request is approved. - * @property data - A hex string conversion of the raw buffer data of the signature request - * @property from - Address to sign this message from - * @property origin? - Added for request origin identification - */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface PersonalMessageParams extends AbstractMessageParams { - data: string; - siwe?: SIWEMessage; -} - -/** - * @type MessageParamsMetamask - * - * Represents the parameters to pass to the personal_sign method once the signature request is approved - * plus data added by MetaMask. - * @property metamaskId - Added for tracking and identification within MetaMask - * @property data - A hex string conversion of the raw buffer data of the signature request - * @property from - Address to sign this message from - * @property origin? - Added for request origin identification - */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface PersonalMessageParamsMetamask - extends AbstractMessageParamsMetamask { - data: string; -} - -/** - * Controller in charge of managing - storing, adding, removing, updating - Messages. - */ -export class PersonalMessageManager extends AbstractMessageManager< - PersonalMessage, - PersonalMessageParams, - PersonalMessageParamsMetamask -> { - /** - * Name of this controller used during composition - */ - override name = 'PersonalMessageManager' as const; - - /** - * Creates a new Message with an 'unapproved' status using the passed messageParams. - * this.addMessage is called to add the new Message to this.messages, and to save the - * unapproved Messages. - * - * @param messageParams - The params for the personal_sign call to be made after the message - * is approved. - * @param req - The original request object possibly containing the origin. - * @returns The id of the newly created message. - */ - async addUnapprovedMessage( - messageParams: PersonalMessageParams, - req?: OriginalRequest, - ): Promise { - validateSignMessageData(messageParams); - - const ethereumSignInData = detectSIWE(messageParams); - const updatedMessageParams = this.addRequestToMessageParams( - messageParams, - req, - ) satisfies PersonalMessageParams; - - updatedMessageParams.data = normalizeMessageData(messageParams.data); - updatedMessageParams.siwe = ethereumSignInData; - - const messageData = this.createUnapprovedMessage( - updatedMessageParams, - ApprovalType.PersonalSign, - req, - ) satisfies PersonalMessage; - - const messageId = messageData.id; - await this.addMessage(messageData); - this.hub.emit(`unapprovedMessage`, { - ...updatedMessageParams, - metamaskId: messageId, - }); - return messageId; - } - - /** - * Removes the metamaskId property from passed messageParams and returns a promise which - * resolves the updated messageParams. - * - * @param messageParams - The messageParams to modify. - * @returns Promise resolving to the messageParams with the metamaskId property removed. - */ - prepMessageForSigning( - messageParams: PersonalMessageParamsMetamask, - ): Promise { - // Using delete operation will throw an error on frozen messageParams - const { metamaskId: _metamaskId, ...messageParamsWithoutId } = - messageParams; - return Promise.resolve(messageParamsWithoutId); - } -} - -export default PersonalMessageManager; diff --git a/packages/message-manager/src/TypedMessageManager.test.ts b/packages/message-manager/src/TypedMessageManager.test.ts deleted file mode 100644 index 942fdde99b..0000000000 --- a/packages/message-manager/src/TypedMessageManager.test.ts +++ /dev/null @@ -1,390 +0,0 @@ -import { TypedMessageManager } from './TypedMessageManager'; - -let controller: TypedMessageManager; -const getCurrentChainIdStub = jest.fn(); - -const fromMock = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - -const typedMessage = [ - { - name: 'Message', - type: 'string', - value: 'Hi, Alice!', - }, - { - name: 'A number', - type: 'uint32', - value: '1337', - }, -]; - -const typedMessageV3V4 = { - types: { - EIP712Domain: [ - { name: 'name', type: 'string' }, - { name: 'version', type: 'string' }, - { name: 'chainId', type: 'uint256' }, - { name: 'verifyingContract', type: 'address' }, - ], - Person: [ - { name: 'name', type: 'string' }, - { name: 'wallet', type: 'address' }, - ], - Mail: [ - { name: 'from', type: 'Person' }, - { name: 'to', type: 'Person' }, - { name: 'contents', type: 'string' }, - ], - }, - primaryType: 'Mail', - domain: { - name: 'Ether Mail', - version: '1', - chainId: 1, - verifyingContract: '0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC', - }, - message: { - from: { name: 'Cow', wallet: '0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826' }, - to: { name: 'Bob', wallet: '0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB' }, - contents: 'Hello, Bob!', - }, -}; - -describe('TypedMessageManager', () => { - beforeEach(() => { - controller = new TypedMessageManager( - undefined, - undefined, - undefined, - undefined, - getCurrentChainIdStub, - ); - }); - - it('should set default state', () => { - expect(controller.state).toStrictEqual({ - unapprovedMessages: {}, - unapprovedMessagesCount: 0, - }); - }); - - it('should set default config', () => { - expect(controller.config).toStrictEqual({}); - }); - - it('should add a valid message', async () => { - const messageId = '1'; - const from = '0x0123'; - const messageTime = Date.now(); - const messageStatus = 'unapproved'; - const messageType = 'eth_signTypedData'; - const messageData = typedMessage; - await controller.addMessage({ - id: messageId, - messageParams: { - data: messageData, - from, - }, - status: messageStatus, - time: messageTime, - type: messageType, - }); - const message = controller.getMessage(messageId); - if (!message) { - throw new Error('"message" is falsy'); - } - expect(message.id).toBe(messageId); - expect(message.messageParams.from).toBe(from); - expect(message.messageParams.data).toBe(messageData); - expect(message.time).toBe(messageTime); - expect(message.status).toBe(messageStatus); - expect(message.type).toBe(messageType); - }); - - it('should throw when adding a valid unapproved message when getCurrentChainId is undefined', async () => { - controller = new TypedMessageManager(); - const version = 'V3'; - const messageData = JSON.stringify(typedMessageV3V4); - const messageParams = { - data: messageData, - from: fromMock, - }; - const originalRequest = { id: 111, origin: 'origin' }; - - await expect( - controller.addUnapprovedMessage(messageParams, originalRequest, version), - ).rejects.toThrow('Current chainId cannot be null or undefined.'); - }); - - it('should add a valid unapproved message', async () => { - const messageStatus = 'unapproved'; - const messageType = 'eth_signTypedData'; - const version = 'version'; - const messageData = typedMessage; - const messageParams = { - data: messageData, - from: fromMock, - }; - const originalRequest = { - id: 111, - origin: 'origin', - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - securityAlertResponse: { result_type: 'result_type', reason: 'reason' }, - }; - const messageId = await controller.addUnapprovedMessage( - messageParams, - originalRequest, - version, - ); - expect(messageId).toBeDefined(); - const message = controller.getMessage(messageId); - if (!message) { - throw new Error('"message" is falsy'); - } - expect(message.messageParams.from).toBe(messageParams.from); - expect(message.messageParams.data).toBe(messageParams.data); - expect(message.messageParams.origin).toBe(originalRequest.origin); - expect(message.messageParams.requestId).toBe(originalRequest.id); - expect(message.time).toBeDefined(); - expect(message.status).toBe(messageStatus); - expect(message.type).toBe(messageType); - expect(message.securityAlertResponse?.result_type).toBe('result_type'); - expect(message.securityAlertResponse?.reason).toBe('reason'); - }); - - it('should add a valid V3 unapproved message as a JSON-parseable string', async () => { - getCurrentChainIdStub.mockImplementation(() => 1); - const messageStatus = 'unapproved'; - const messageType = 'eth_signTypedData'; - const version = 'V3'; - const messageData = JSON.stringify(typedMessageV3V4); - const messageParams = { - data: messageData, - from: fromMock, - }; - const originalRequest = { id: 111, origin: 'origin' }; - const messageId = await controller.addUnapprovedMessage( - messageParams, - originalRequest, - version, - ); - expect(messageId).toBeDefined(); - const message = controller.getMessage(messageId); - if (!message) { - throw new Error('"message" is falsy'); - } - expect(message.messageParams.from).toBe(messageParams.from); - expect(message.messageParams.data).toBe(messageParams.data); - expect(message.messageParams.requestId).toBe(originalRequest.id); - expect(message.time).toBeDefined(); - expect(message.status).toBe(messageStatus); - expect(message.type).toBe(messageType); - }); - - it('should add a valid V3 unapproved message as an object', async () => { - getCurrentChainIdStub.mockImplementation(() => 1); - const messageStatus = 'unapproved'; - const messageType = 'eth_signTypedData'; - const version = 'V3'; - const messageData = typedMessageV3V4; - const messageParams = { - data: messageData, - from: fromMock, - }; - const originalRequest = { id: 111, origin: 'origin' }; - const messageId = await controller.addUnapprovedMessage( - messageParams, - originalRequest, - version, - ); - expect(messageId).toBeDefined(); - const message = controller.getMessage(messageId); - if (!message) { - throw new Error('"message" is falsy'); - } - expect(message.messageParams.from).toBe(messageParams.from); - expect(message.messageParams.data).toBe(messageParams.data); - expect(message.messageParams.requestId).toBe(originalRequest.id); - expect(message.time).toBeDefined(); - expect(message.status).toBe(messageStatus); - expect(message.type).toBe(messageType); - }); - - it('should throw when adding invalid legacy typed message', async () => { - const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const messageData = '0x879'; - const version = 'V1'; - await expect( - controller.addUnapprovedMessage( - { - data: messageData, - from, - }, - undefined, - version, - ), - ).rejects.toThrow('Invalid message "data":'); - }); - - it('should throw when adding invalid typed message', async () => { - const mockGetChainId = jest.fn(); - const from = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d'; - const messageData = typedMessage; - const version = 'V3'; - await expect( - controller.addUnapprovedMessage( - { - data: messageData, - from, - }, - undefined, - version, - ), - ).rejects.toThrow('Invalid message "data":'); - - const controllerWithGetCurrentChainIdCallback = new TypedMessageManager( - undefined, - undefined, - undefined, - undefined, - mockGetChainId, - ); - await expect( - controllerWithGetCurrentChainIdCallback.addUnapprovedMessage( - { - data: messageData, - from, - }, - undefined, - 'V4', - ), - ).rejects.toThrow('Invalid message "data":'); - expect(mockGetChainId).toHaveBeenCalled(); - }); - - it('should get correct unapproved messages', async () => { - const firstMessageData = [ - { - name: 'Message', - type: 'string', - value: 'Hi, Alice!', - }, - { - name: 'A number', - type: 'uint32', - value: '1337', - }, - ]; - const secondMessageData = [ - { - name: 'Message', - type: 'string', - value: 'Hi, Alice!', - }, - { - name: 'A number', - type: 'uint32', - value: '1337', - }, - ]; - const firstMessage = { - id: '1', - messageParams: { from: '0x1', data: firstMessageData }, - status: 'unapproved', - time: 123, - type: 'eth_signTypedData', - }; - const secondMessage = { - id: '2', - messageParams: { from: '0x1', data: secondMessageData }, - status: 'unapproved', - time: 123, - type: 'eth_signTypedData', - }; - await controller.addMessage(firstMessage); - await controller.addMessage(secondMessage); - expect(controller.getUnapprovedMessagesCount()).toBe(2); - expect(controller.getUnapprovedMessages()).toStrictEqual({ - [firstMessage.id]: firstMessage, - [secondMessage.id]: secondMessage, - }); - }); - - it('should approve typed message', async () => { - const messageData = typedMessage; - const firstMessage = { from: fromMock, data: messageData }; - const version = 'V1'; - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/await-thenable - const messageId = await await controller.addUnapprovedMessage( - firstMessage, - undefined, - version, - ); - const messageParams = await controller.approveMessage({ - ...firstMessage, - metamaskId: messageId, - version, - }); - const message = controller.getMessage(messageId); - expect(messageParams).toStrictEqual(firstMessage); - if (!message) { - throw new Error('"message" is falsy'); - } - expect(message.status).toBe('approved'); - }); - - it('should set message status signed', async () => { - const messageData = typedMessage; - const firstMessage = { from: fromMock, data: messageData }; - const version = 'V1'; - const rawSig = '0x5f7a0'; - const messageId = await controller.addUnapprovedMessage( - firstMessage, - undefined, - version, - ); - controller.setMessageStatusSigned(messageId, rawSig); - const message = controller.getMessage(messageId); - if (!message) { - throw new Error('"message" is falsy'); - } - expect(message.rawSig).toStrictEqual(rawSig); - expect(message.status).toBe('signed'); - }); - - it('should reject message', async () => { - const messageData = typedMessage; - const firstMessage = { from: fromMock, data: messageData }; - const version = 'V1'; - const messageId = await controller.addUnapprovedMessage( - firstMessage, - undefined, - version, - ); - controller.rejectMessage(messageId); - const message = controller.getMessage(messageId); - if (!message) { - throw new Error('"message" is falsy'); - } - expect(message.status).toBe('rejected'); - }); - - it('should set message status errored', async () => { - const messageData = typedMessage; - const firstMessage = { from: fromMock, data: messageData }; - const version = 'V1'; - const messageId = await controller.addUnapprovedMessage( - firstMessage, - undefined, - version, - ); - controller.setMessageStatusErrored(messageId, 'errored'); - const message = controller.getMessage(messageId); - if (!message) { - throw new Error('"message" is falsy'); - } - expect(message.status).toBe('errored'); - }); -}); diff --git a/packages/message-manager/src/TypedMessageManager.ts b/packages/message-manager/src/TypedMessageManager.ts deleted file mode 100644 index cc1f03f720..0000000000 --- a/packages/message-manager/src/TypedMessageManager.ts +++ /dev/null @@ -1,197 +0,0 @@ -import { ApprovalType } from '@metamask/controller-utils'; - -import type { - AbstractMessage, - AbstractMessageParams, - AbstractMessageParamsMetamask, - OriginalRequest, -} from './AbstractMessageManager'; -import { AbstractMessageManager } from './AbstractMessageManager'; -import { - validateTypedSignMessageDataV1, - validateTypedSignMessageDataV3V4, -} from './utils'; - -/** - * @type TypedMessage - * - * Represents and contains data about an 'eth_signTypedData' type signature request. - * These are created when a signature for an eth_signTypedData call is requested. - * @property id - An id to track and identify the message object - * @property error - Error corresponding to eth_signTypedData error in failure case - * @property messageParams - The parameters to pass to the eth_signTypedData method once - * the signature request is approved - * @property type - The json-prc signing method for which a signature request has been made. - * A 'TypedMessage' which always has a 'eth_signTypedData' type - * @property rawSig - Raw data of the signature request - */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface TypedMessage extends AbstractMessage { - error?: string; - messageParams: TypedMessageParams; - time: number; - status: string; - type: string; - rawSig?: string; -} - -export type SignTypedDataMessageV3V4 = { - types: Record; - domain: Record; - primaryType: string; - message: unknown; -}; - -/** - * @type TypedMessageParams - * - * Represents the parameters to pass to the eth_signTypedData method once the signature request is approved. - * @property data - A hex string conversion of the raw buffer or an object containing data of the signature - * request depending on version - * @property from - Address to sign this message from - * @property origin? - Added for request origin identification - */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface TypedMessageParams extends AbstractMessageParams { - data: Record[] | string | SignTypedDataMessageV3V4; -} - -/** - * @type TypedMessageParamsMetamask - * - * Represents the parameters to pass to the eth_signTypedData method once the signature request is approved - * plus data added by MetaMask. - * @property metamaskId - Added for tracking and identification within MetaMask - * @property data - A hex string conversion of the raw buffer or an object containing data of the signature - * request depending on version - * @property error? - Added for message errored - * @property from - Address to sign this message from - * @property origin? - Added for request origin identification - * @property version - Compatibility version EIP712 - */ -// This interface was created before this ESLint rule was added. -// Convert to a `type` in a future major version. -// eslint-disable-next-line @typescript-eslint/consistent-type-definitions -export interface TypedMessageParamsMetamask - extends AbstractMessageParamsMetamask { - data: TypedMessageParams['data']; - metamaskId?: string; - error?: string; - version?: string; -} - -/** - * Controller in charge of managing - storing, adding, removing, updating - TypedMessages. - */ -export class TypedMessageManager extends AbstractMessageManager< - TypedMessage, - TypedMessageParams, - TypedMessageParamsMetamask -> { - /** - * Name of this controller used during composition - */ - override name = 'TypedMessageManager' as const; - - /** - * Creates a new TypedMessage with an 'unapproved' status using the passed messageParams. - * this.addMessage is called to add the new TypedMessage to this.messages, and to save the - * unapproved TypedMessages. - * - * @param messageParams - The params for the 'eth_signTypedData' call to be made after the message - * is approved. - * @param req - The original request object possibly containing the origin. - * @param version - Compatibility version EIP712. - * @returns The id of the newly created TypedMessage. - */ - async addUnapprovedMessage( - messageParams: TypedMessageParams, - req?: OriginalRequest, - version?: string, - ): Promise { - if (version === 'V1') { - validateTypedSignMessageDataV1(messageParams); - } - - if (version === 'V3' || version === 'V4') { - const currentChainId = this.getCurrentChainId?.(); - validateTypedSignMessageDataV3V4(messageParams, currentChainId); - } - - if ( - typeof messageParams.data !== 'string' && - (version === 'V3' || version === 'V4') - ) { - messageParams.data = JSON.stringify(messageParams.data); - } - - const updatedMessageParams = this.addRequestToMessageParams( - messageParams, - req, - ) satisfies TypedMessageParams; - - const messageData = this.createUnapprovedMessage( - updatedMessageParams, - ApprovalType.EthSignTypedData, - req, - ) satisfies TypedMessage; - - const messageId = messageData.id; - - await this.addMessage(messageData); - - /** - * This intentionally splays messageParams rather than updatedMessageParams. I'm unsure if this - * is exactly what we want, but I am preserving existing logic. - */ - this.hub.emit(`unapprovedMessage`, { - ...messageParams, - metamaskId: messageId, - version, - }); - - return messageId; - } - - /** - * Sets a TypedMessage status to 'errored' via a call to this.setMessageStatus. - * - * @param messageId - The id of the TypedMessage to error. - * @param error - The error to be included in TypedMessage. - */ - setMessageStatusErrored(messageId: string, error: string) { - const message = this.getMessage(messageId); - /* istanbul ignore if */ - if (!message) { - return; - } - message.error = error; - this.updateMessage(message); - this.setMessageStatus(messageId, 'errored'); - } - - /** - * Removes the metamaskId and version properties from passed messageParams and returns a promise which - * resolves the updated messageParams. - * - * @param messageParams - The messageParams to modify. - * @returns Promise resolving to the messageParams with the metamaskId and version properties removed. - */ - prepMessageForSigning( - messageParams: TypedMessageParamsMetamask, - ): Promise { - // Using delete operation will throw an error on frozen messageParams - const { - metamaskId: _metamaskId, - version: _version, - ...messageParamsWithoutId - } = messageParams; - return Promise.resolve(messageParamsWithoutId); - } -} - -export default TypedMessageManager; diff --git a/packages/message-manager/src/index.ts b/packages/message-manager/src/index.ts index 3467359316..2772368545 100644 --- a/packages/message-manager/src/index.ts +++ b/packages/message-manager/src/index.ts @@ -1,5 +1,4 @@ export * from './AbstractMessageManager'; -export * from './PersonalMessageManager'; -export * from './TypedMessageManager'; export * from './EncryptionPublicKeyManager'; export * from './DecryptMessageManager'; +export * from './types'; diff --git a/packages/message-manager/src/types.ts b/packages/message-manager/src/types.ts new file mode 100644 index 0000000000..b9ee509dc4 --- /dev/null +++ b/packages/message-manager/src/types.ts @@ -0,0 +1,21 @@ +import type { SIWEMessage } from '@metamask/controller-utils'; + +import type { AbstractMessageParams } from './AbstractMessageManager'; + +// Below types are temporary as they are used by the KeyringController. + +export type SignTypedDataMessageV3V4 = { + types: Record; + domain: Record; + primaryType: string; + message: unknown; +}; + +export type PersonalMessageParams = { + data: string; + siwe?: SIWEMessage; +} & AbstractMessageParams; + +export type TypedMessageParams = { + data: Record[] | string | SignTypedDataMessageV3V4; +} & AbstractMessageParams; diff --git a/packages/message-manager/src/utils.test.ts b/packages/message-manager/src/utils.test.ts index a85ff2e1a6..01ac1327e0 100644 --- a/packages/message-manager/src/utils.test.ts +++ b/packages/message-manager/src/utils.test.ts @@ -1,5 +1,3 @@ -import { convertHexToDecimal, toHex } from '@metamask/controller-utils'; - import * as util from './utils'; describe('utils', () => { @@ -14,322 +12,6 @@ describe('utils', () => { expect(secondNormalized).toBe('0x736f6d6564617461'); }); - describe('validateSignMessageData', () => { - it('should throw if no from address', () => { - expect(() => - util.validateSignMessageData({ - data: '0x879a05', - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any), - ).toThrow(`Invalid "from" address: undefined must be a valid string.`); - }); - - it('should throw if invalid from address', () => { - const from = '01'; - expect(() => - util.validateSignMessageData({ - data: '0x879a05', - from, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any), - ).toThrow(`Invalid "from" address: ${from} must be a valid string.`); - }); - - it('should throw if invalid type from address', () => { - const from = 123; - expect(() => - util.validateSignMessageData({ - data: '0x879a05', - from, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any), - ).toThrow(`Invalid "from" address: ${from} must be a valid string.`); - }); - - it('should throw if no data', () => { - expect(() => - util.validateSignMessageData({ - data: '0x879a05', - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any), - ).toThrow(`Invalid "from" address: undefined must be a valid string.`); - }); - - it('should throw if invalid tyoe data', () => { - expect(() => - util.validateSignMessageData({ - data: 123, - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any), - ).toThrow('Invalid message "data": 123 must be a valid string.'); - }); - }); - - describe('validateTypedMessageDataV1', () => { - it('should throw if no from address legacy', () => { - expect(() => - util.validateTypedSignMessageDataV1({ - data: [], - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any), - ).toThrow(`Invalid "from" address: undefined must be a valid string.`); - }); - - it('should throw if invalid from address', () => { - const from = '3244e191f1b4903970224322180f1'; - expect(() => - util.validateTypedSignMessageDataV1({ - data: [], - from, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any), - ).toThrow(`Invalid "from" address: ${from} must be a valid string.`); - }); - - it('should throw if invalid type from address', () => { - const from = 123; - expect(() => - util.validateTypedSignMessageDataV1({ - data: [], - from, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any), - ).toThrow(`Invalid "from" address: ${from} must be a valid string.`); - }); - - it('should throw if incorrect data', () => { - expect(() => - util.validateTypedSignMessageDataV1({ - data: '0x879a05', - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any), - ).toThrow('Invalid message "data":'); - }); - - it('should throw if no data', () => { - expect(() => - util.validateTypedSignMessageDataV1({ - data: '0x879a05', - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any), - ).toThrow('Invalid message "data":'); - }); - - it('should throw if invalid type data', () => { - expect(() => - util.validateTypedSignMessageDataV1({ - data: [], - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any), - ).toThrow('Expected EIP712 typed data.'); - }); - }); - - describe('validateTypedSignMessageDataV3V4', () => { - const dataTyped = - '{"types":{"EIP712Domain":[{"name":"name","type":"string"},{"name":"version","type":"string"},{"name":"chainId","type":"uint256"},{"name":"verifyingContract","type":"address"}],"Person":[{"name":"name","type":"string"},{"name":"wallet","type":"address"}],"Mail":[{"name":"from","type":"Person"},{"name":"to","type":"Person"},{"name":"contents","type":"string"}]},"primaryType":"Mail","domain":{"name":"Ether Mail","version":"1","chainId":1,"verifyingContract":"0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC"},"message":{"from":{"name":"Cow","wallet":"0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826"},"to":{"name":"Bob","wallet":"0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB"},"contents":"Hello, Bob!"}}'; - const mockedCurrentChainId = toHex(1); - it('should throw if no from address', () => { - expect(() => - util.validateTypedSignMessageDataV3V4( - { - data: '0x879a05', - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any, - mockedCurrentChainId, - ), - ).toThrow(`Invalid "from" address: undefined must be a valid string.`); - }); - - it('should throw if invalid from address', () => { - const from = '3244e191f1b4903970224322180f1fb'; - expect(() => - util.validateTypedSignMessageDataV3V4( - { - data: '0x879a05', - from, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any, - mockedCurrentChainId, - ), - ).toThrow(`Invalid "from" address: ${from} must be a valid string.`); - }); - - it('should throw if invalid type from address', () => { - const from = 123; - expect(() => - util.validateTypedSignMessageDataV3V4( - { - data: '0x879a05', - from: 123, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any, - mockedCurrentChainId, - ), - ).toThrow(`Invalid "from" address: ${from} must be a valid string.`); - }); - - it('should throw if array data', () => { - expect(() => - util.validateTypedSignMessageDataV3V4( - { - data: [], - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any, - mockedCurrentChainId, - ), - ).toThrow('Invalid message "data":'); - }); - - it('should throw if no array data', () => { - expect(() => - util.validateTypedSignMessageDataV3V4( - { - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any, - mockedCurrentChainId, - ), - ).toThrow('Invalid message "data":'); - }); - - it('should throw if no json valid data', () => { - expect(() => - util.validateTypedSignMessageDataV3V4( - { - data: 'uh oh', - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any, - mockedCurrentChainId, - ), - ).toThrow('Data must be passed as a valid JSON string.'); - }); - - it('should throw if current chain id is not present', () => { - expect(() => - util.validateTypedSignMessageDataV3V4( - { - data: dataTyped, - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any, - undefined, - ), - ).toThrow('Current chainId cannot be null or undefined.'); - }); - - it('should throw if current chain id is not convertable to integer', () => { - const unexpectedChainId = 'unexpected chain id'; - expect(() => - util.validateTypedSignMessageDataV3V4( - { - data: dataTyped.replace(`"chainId":1`, `"chainId":"0x1"`), - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any, - // @ts-expect-error Intentionally invalid - unexpectedChainId, - ), - ).toThrow( - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - `Cannot sign messages for chainId "${convertHexToDecimal( - mockedCurrentChainId, - )}", because MetaMask is switching networks.`, - ); - }); - - it('should throw if current chain id is not matched with provided in message data', () => { - const chainId = toHex(2); - expect(() => - util.validateTypedSignMessageDataV3V4( - { - data: dataTyped, - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any, - chainId, - ), - ).toThrow( - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - `Provided chainId "${convertHexToDecimal( - mockedCurrentChainId, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - )}" must match the active chainId "${convertHexToDecimal(chainId)}"`, - ); - }); - - it('should throw if data not in typed message schema', () => { - expect(() => - util.validateTypedSignMessageDataV3V4( - { - data: '{"greetings":"I am Alice"}', - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any, - mockedCurrentChainId, - ), - ).toThrow('Data must conform to EIP-712 schema.'); - }); - - it('should not throw if data is correct', () => { - expect(() => - util.validateTypedSignMessageDataV3V4( - { - data: dataTyped.replace(`"chainId":1`, `"chainId":"1"`), - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any, - mockedCurrentChainId, - ), - ).not.toThrow(); - }); - - it('should not throw if data is correct (object format)', () => { - expect(() => - util.validateTypedSignMessageDataV3V4( - { - data: JSON.parse(dataTyped), - from: '0x3244e191f1b4903970224322180f1fbbc415696b', - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any, - mockedCurrentChainId, - ), - ).not.toThrow(); - }); - }); - describe('validateEncryptionPublicKeyMessageData', () => { it('should throw if no from address', () => { expect(() => diff --git a/packages/message-manager/src/utils.ts b/packages/message-manager/src/utils.ts index 3fdd49c9d9..caa3aed745 100644 --- a/packages/message-manager/src/utils.ts +++ b/packages/message-manager/src/utils.ts @@ -1,16 +1,8 @@ import { isValidHexAddress } from '@metamask/controller-utils'; -import { - TYPED_MESSAGE_SCHEMA, - typedSignatureHash, -} from '@metamask/eth-sig-util'; -import type { Hex } from '@metamask/utils'; import { add0x, bytesToHex, remove0x } from '@metamask/utils'; -import { validate } from 'jsonschema'; import type { DecryptMessageParams } from './DecryptMessageManager'; import type { EncryptionPublicKeyParams } from './EncryptionPublicKeyManager'; -import type { PersonalMessageParams } from './PersonalMessageManager'; -import type { TypedMessageParams } from './TypedMessageManager'; const hexRe = /^[0-9A-Fa-f]+$/gu; /** @@ -46,121 +38,6 @@ export function normalizeMessageData(data: string) { return bytesToHex(Buffer.from(data, 'utf8')); } -/** - * Validates a PersonalMessageParams objects for required properties and throws in - * the event of any validation error. - * - * @param messageData - PersonalMessageParams object to validate. - */ -export function validateSignMessageData(messageData: PersonalMessageParams) { - const { from, data } = messageData; - validateAddress(from, 'from'); - - if (!data || typeof data !== 'string') { - throw new Error(`Invalid message "data": ${data} must be a valid string.`); - } -} - -/** - * Validates a TypedMessageParams object for required properties and throws in - * the event of any validation error for eth_signTypedMessage_V1. - * - * @param messageData - TypedMessageParams object to validate. - */ -export function validateTypedSignMessageDataV1( - messageData: TypedMessageParams, -) { - validateAddress(messageData.from, 'from'); - - if (!messageData.data || !Array.isArray(messageData.data)) { - throw new Error( - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - `Invalid message "data": ${messageData.data} must be a valid array.`, - ); - } - - try { - // typedSignatureHash will throw if the data is invalid. - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - typedSignatureHash(messageData.data as any); - } catch (e) { - throw new Error(`Expected EIP712 typed data.`); - } -} - -/** - * Validates a TypedMessageParams object for required properties and throws in - * the event of any validation error for eth_signTypedMessage_V3. - * - * @param messageData - TypedMessageParams object to validate. - * @param currentChainId - The current chainId. - */ -export function validateTypedSignMessageDataV3V4( - messageData: TypedMessageParams, - currentChainId: Hex | undefined, -) { - validateAddress(messageData.from, 'from'); - - if ( - !messageData.data || - Array.isArray(messageData.data) || - (typeof messageData.data !== 'object' && - typeof messageData.data !== 'string') - ) { - throw new Error( - `Invalid message "data": Must be a valid string or object.`, - ); - } - - let data; - if (typeof messageData.data === 'object') { - data = messageData.data; - } else { - try { - data = JSON.parse(messageData.data); - } catch (e) { - throw new Error('Data must be passed as a valid JSON string.'); - } - } - - const validation = validate(data, TYPED_MESSAGE_SCHEMA); - if (validation.errors.length > 0) { - throw new Error( - 'Data must conform to EIP-712 schema. See https://git.io/fNtcx.', - ); - } - - if (!currentChainId) { - throw new Error('Current chainId cannot be null or undefined.'); - } - - let { chainId } = data.domain; - if (chainId) { - if (typeof chainId === 'string') { - chainId = parseInt(chainId, chainId.startsWith('0x') ? 16 : 10); - } - - const activeChainId = parseInt(currentChainId, 16); - if (Number.isNaN(activeChainId)) { - throw new Error( - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - `Cannot sign messages for chainId "${chainId}", because MetaMask is switching networks.`, - ); - } - - if (chainId !== activeChainId) { - throw new Error( - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - `Provided chainId "${chainId}" must match the active chainId "${activeChainId}"`, - ); - } - } -} - /** * Validates messageData for the eth_getEncryptionPublicKey message and throws in * the event of any validation error. diff --git a/packages/signature-controller/CHANGELOG.md b/packages/signature-controller/CHANGELOG.md index 599431ff22..c84a1b48bd 100644 --- a/packages/signature-controller/CHANGELOG.md +++ b/packages/signature-controller/CHANGELOG.md @@ -7,6 +7,38 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Remove usage of `@metamask/message-manager` package ([#4785](https://github.com/MetaMask/core/pull/4785)) + - Add `signatureRequests` object to state to include all messages with all types and statuses. + - Add optional `state` option to constructor to provide initial state. + - Add equivalent types formerly in `@metamask/message-manager`: + - `OriginalRequest` + - `TypedSigningOptions` + - `MessageParams` + - `MessageParamsPersonal` + - `MessageParamsTyped` + - `SignatureRequest` + - `SignatureRequestStatus` + - `SignatureRequestType` + +### Changed + +- Remove usage of `@metamask/message-manager` package ([#4785](https://github.com/MetaMask/core/pull/4785)) + - **BREAKING** Change `type` property in message state to enum values rather than `string`. + - Deprecreate the following state since the same data can be derived from `signatureRequests`: + - `unapprovedPersonalMsgs` + - `unapprovedTypedMessages` + - `unapprovedPersonalMsgCount` + - `unapprovedTypedMessagesCount` + - Deprecreate the following properties since the same data can be derived from the state: + - `unapprovedPersonalMessagesCount` + - `unapprovedTypedMessagesCount` + - `messages` + - Deprecreate the following constructor options since they are no longer used: + - `getAllState` + - `securityProviderRequest` + ## [19.1.0] ### Added diff --git a/packages/signature-controller/package.json b/packages/signature-controller/package.json index bbfe4406a9..8cb7b66d4f 100644 --- a/packages/signature-controller/package.json +++ b/packages/signature-controller/package.json @@ -49,9 +49,11 @@ "dependencies": { "@metamask/base-controller": "^7.0.1", "@metamask/controller-utils": "^11.3.0", - "@metamask/message-manager": "^10.1.1", + "@metamask/eth-sig-util": "^7.0.1", "@metamask/utils": "^9.1.0", - "lodash": "^4.17.21" + "jsonschema": "^1.2.4", + "lodash": "^4.17.21", + "uuid": "^8.3.2" }, "devDependencies": { "@metamask/approval-controller": "^7.1.0", diff --git a/packages/signature-controller/src/SignatureController.test.ts b/packages/signature-controller/src/SignatureController.test.ts index df34cdb3c4..2fb84b55d8 100644 --- a/packages/signature-controller/src/SignatureController.test.ts +++ b/packages/signature-controller/src/SignatureController.test.ts @@ -1,237 +1,210 @@ -import { ORIGIN_METAMASK } from '@metamask/controller-utils'; -import { - SigningMethod, - SigningStage, - LogType, -} from '@metamask/logging-controller'; -import type { - AbstractMessage, - OriginalRequest, -} from '@metamask/message-manager'; -import { - PersonalMessageManager, - TypedMessageManager, -} from '@metamask/message-manager'; +import type { SIWEMessage } from '@metamask/controller-utils'; +import { detectSIWE } from '@metamask/controller-utils'; +import { SignTypedDataVersion } from '@metamask/keyring-controller'; +import { LogType, SigningStage } from '@metamask/logging-controller'; +import { v1 } from 'uuid'; +import { flushPromises } from '../../../tests/helpers'; import type { SignatureControllerMessenger, SignatureControllerOptions, + SignatureControllerState, } from './SignatureController'; import { SignatureController } from './SignatureController'; +import type { MessageParamsPersonal, SignatureRequest } from './types'; +import { SignatureRequestStatus, SignatureRequestType } from './types'; +import { + normalizePersonalMessageParams, + normalizeTypedMessageParams, +} from './utils/normalize'; -jest.mock('@metamask/message-manager', () => ({ - PersonalMessageManager: jest.fn(), - TypedMessageManager: jest.fn(), -})); - -jest.mock('@metamask/controller-utils', () => { - const actual = jest.requireActual('@metamask/controller-utils'); - return { ...actual, detectSIWE: jest.fn() }; -}); - -class NoErrorThrownError extends Error {} -const getError = async (call: () => unknown): Promise => { - try { - await call(); - throw new NoErrorThrownError(); - } catch (error: unknown) { - return error as TError; - } -}; +jest.mock('uuid'); +jest.mock('./utils/validation'); +jest.mock('./utils/normalize'); -const messageIdMock = '123'; -const messageIdMock2 = '456'; -const versionMock = 'V1'; +jest.mock('@metamask/controller-utils', () => ({ + ...jest.requireActual('@metamask/controller-utils'), + detectSIWE: jest.fn(), +})); -const messageParamsWithoutIdMock = { - from: '0x123', - origin: 'http://test.com', - data: '0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF', - version: 'V1', +const ID_MOCK = '123-456'; +const CHAIN_ID_MOCK = '0x1'; +const FROM_MOCK = '0x456DEF'; +const DATA_MOCK = '0xABC123'; +const SIGNATURE_HASH_MOCK = '0x123ABC'; +const ERROR_MESSAGE_MOCK = 'Test Error Message'; +const ERROR_CODE_MOCK = 1234; + +const PARAMS_MOCK = { + from: FROM_MOCK, + data: DATA_MOCK, }; -const messageParamsMock = { - ...messageParamsWithoutIdMock, - metamaskId: messageIdMock, +const SIGNATURE_REQUEST_MOCK: SignatureRequest = { + id: ID_MOCK, + messageParams: PARAMS_MOCK, + status: SignatureRequestStatus.Signed, + time: Date.now(), + type: SignatureRequestType.PersonalSign, }; -const messageParamsMock2 = { - from: '0x124', - origin: 'http://test4.com', - data: '0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFA', - metamaskId: messageIdMock, -}; +/** + * Create a mock messenger instance. + * @returns The mock messenger instance plus individual mock functions for each action. + */ +function createMessengerMock() { + const loggingControllerAddMock = jest.fn(); + const approvalControllerAddRequestMock = jest.fn(); + const keyringControllerSignPersonalMessageMock = jest.fn(); + const keyringControllerSignTypedMessageMock = jest.fn(); -const messageMock = { - id: messageIdMock, - time: 123, - status: 'unapproved', - type: 'testType', - rawSig: undefined, - // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any -} as any as AbstractMessage; - -const coreMessageMock = { - ...messageMock, - messageParams: messageParamsMock, -}; - -const stateMessageMock = { - ...messageMock, - msgParams: messageParamsMock, -}; - -const requestMock = { - origin: 'http://test2.com', -} as OriginalRequest; + const callMock = (method: string, ...args: any[]) => { + switch (method) { + case 'LoggingController:add': + return loggingControllerAddMock(...args); + case 'ApprovalController:addRequest': + return approvalControllerAddRequestMock(...args); + case 'KeyringController:signPersonalMessage': + return keyringControllerSignPersonalMessageMock(...args); + case 'KeyringController:signTypedMessage': + return keyringControllerSignTypedMessageMock(...args); + default: + throw new Error(`Messenger method not recognised: ${method}`); + } + }; -const createMessengerMock = () => - ({ + const messenger = { registerActionHandler: jest.fn(), registerInitialEventPayload: jest.fn(), publish: jest.fn(), - call: jest.fn(), - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } as any as jest.Mocked); - -const addUnapprovedMessageMock = jest.fn(); -const waitForFinishStatusMock = jest.fn(); -const approveMessageMock = jest.fn(); - -// TODO: Replace `any` with type -// TODO: Either fix this lint violation or explain why it's necessary to ignore. -// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/naming-convention -const createMessageManagerMock = (prototype?: any): jest.Mocked => { - const messageManagerMock = Object.create(prototype); - - return Object.assign(messageManagerMock, { - getUnapprovedMessages: jest.fn(), - getUnapprovedMessagesCount: jest.fn(), - addUnapprovedMessage: addUnapprovedMessageMock, - waitForFinishStatus: waitForFinishStatusMock, - approveMessage: approveMessageMock, - setMessageStatusSigned: jest.fn(), - setMessageStatusErrored: jest.fn(), - setMessageStatusInProgress: jest.fn(), - rejectMessage: jest.fn(), - cancelAbstractMessage: jest.fn(), - subscribe: jest.fn(), - update: jest.fn(), - setMetadata: jest.fn(), - getAllMessages: jest.fn(), - hub: { - on: jest.fn(), - }, - }) as jest.Mocked; -}; + call: callMock, + } as unknown as jest.Mocked; + + approvalControllerAddRequestMock.mockResolvedValue({}); + loggingControllerAddMock.mockResolvedValue({}); + + return { + approvalControllerAddRequestMock, + keyringControllerSignPersonalMessageMock, + keyringControllerSignTypedMessageMock, + loggingControllerAddMock, + messenger, + }; +} + +/** + * Create a new instance of the SignatureController. + * @param options - Optional overrides for the default options. + * @returns The controller instance plus individual mock functions for each action. + */ +function createController(options?: Partial) { + const messengerMocks = createMessengerMock(); + + const controller = new SignatureController({ + messenger: messengerMocks.messenger, + getCurrentChainId: () => CHAIN_ID_MOCK, + ...options, + }); -describe('SignatureController', () => { - let signatureController: SignatureController; + return { controller, ...messengerMocks }; +} - const personalMessageManagerConstructorMock = - PersonalMessageManager as jest.MockedClass; - const typedMessageManagerConstructorMock = - TypedMessageManager as jest.MockedClass; +/** + * Create a mock error. + * @returns The mock error instance. + */ +function createErrorMock(): Error { + const errorMock = new Error(ERROR_MESSAGE_MOCK); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (errorMock as any).code = ERROR_CODE_MOCK; + return errorMock; +} - const personalMessageManagerMock = - createMessageManagerMock( - PersonalMessageManager.prototype, - ); - const typedMessageManagerMock = createMessageManagerMock( - TypedMessageManager.prototype, +describe('SignatureController', () => { + const normalizePersonalMessageParamsMock = jest.mocked( + normalizePersonalMessageParams, ); - const resultCallbacksMock = { - success: jest.fn(), - error: jest.fn(), - }; - const messengerMock = createMessengerMock(); - const getAllStateMock = jest.fn(); - const securityProviderRequestMock = jest.fn(); - const isEthSignEnabledMock = jest.fn(); - const getCurrentChainIdMock = jest.fn(); - const keyringErrorMessageMock = 'Keyring Error'; - const keyringErrorMock = new Error(keyringErrorMessageMock); - - const mockMessengerAction = ( - action: string, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - callback: (actionName: string, ...args: any[]) => any, - ) => { - messengerMock.call.mockImplementation((actionName, ...rest) => { - if (actionName === action) { - return callback(actionName, ...rest); - } - - return Promise.resolve({ - resultCallbacks: resultCallbacksMock, - }); - }); - }; - - beforeEach(() => { - jest.spyOn(console, 'info').mockImplementation(() => undefined); - addUnapprovedMessageMock.mockResolvedValue(messageIdMock); - approveMessageMock.mockResolvedValue(messageParamsWithoutIdMock); - personalMessageManagerConstructorMock.mockReturnValue( - personalMessageManagerMock, - ); - messengerMock.call.mockResolvedValue({ - resultCallbacks: resultCallbacksMock, - }); + const normalizeTypedMessageParamsMock = jest.mocked( + normalizeTypedMessageParams, + ); - typedMessageManagerConstructorMock.mockReturnValue(typedMessageManagerMock); + const detectSIWEMock = jest.mocked(detectSIWE); + const uuidV1Mock = jest.mocked(v1); - isEthSignEnabledMock.mockReturnValue(true); + beforeEach(() => { + jest.resetAllMocks(); - signatureController = new SignatureController({ - messenger: messengerMock, - getAllState: getAllStateMock, - securityProviderRequest: securityProviderRequestMock, - isEthSignEnabled: isEthSignEnabledMock, - getCurrentChainId: getCurrentChainIdMock, - } as SignatureControllerOptions); + normalizePersonalMessageParamsMock.mockImplementation((params) => params); + normalizeTypedMessageParamsMock.mockImplementation((params) => params); + uuidV1Mock.mockReturnValue(ID_MOCK); }); describe('unapprovedPersonalMessagesCount', () => { - it('returns value from personal message manager getter', () => { - personalMessageManagerMock.getUnapprovedMessagesCount.mockReturnValueOnce( - 11, - ); - expect(signatureController.unapprovedPersonalMessagesCount).toBe(11); + it('returns the number of unapproved personal messages in state', () => { + const { controller } = createController({ + state: { + unapprovedPersonalMsgCount: 3, + } as SignatureControllerState, + }); + + expect(controller.unapprovedPersonalMessagesCount).toBe(3); }); }); describe('unapprovedTypedMessagesCount', () => { - it('returns value from typed message manager getter', () => { - typedMessageManagerMock.getUnapprovedMessagesCount.mockReturnValueOnce( - 12, - ); - expect(signatureController.unapprovedTypedMessagesCount).toBe(12); + it('returns the number of unapproved typed messages in state', () => { + const { controller } = createController({ + state: { + unapprovedTypedMessagesCount: 3, + } as SignatureControllerState, + }); + + expect(controller.unapprovedTypedMessagesCount).toBe(3); + }); + }); + + describe('messages', () => { + it('returns the signature requests in state', () => { + const { controller } = createController({ + state: { + signatureRequests: { + [ID_MOCK]: SIGNATURE_REQUEST_MOCK, + '2': SIGNATURE_REQUEST_MOCK, + }, + } as unknown as SignatureControllerState, + }); + + expect(controller.messages).toStrictEqual({ + [ID_MOCK]: SIGNATURE_REQUEST_MOCK, + '2': SIGNATURE_REQUEST_MOCK, + }); }); }); describe('resetState', () => { - it('sets state to initial state', () => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - signatureController.update(() => ({ - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - unapprovedPersonalMsgs: { [messageIdMock]: messageMock } as any, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - unapprovedTypedMessages: { [messageIdMock]: messageMock } as any, - unapprovedPersonalMsgCount: 2, - unapprovedTypedMessagesCount: 3, - })); - - signatureController.resetState(); - - expect(signatureController.state).toStrictEqual({ + it('resets the state to default state', () => { + const { controller } = createController({ + state: { + signatureRequests: { + [ID_MOCK]: SIGNATURE_REQUEST_MOCK, + }, + unapprovedPersonalMsgs: { + [ID_MOCK]: SIGNATURE_REQUEST_MOCK, + }, + unapprovedTypedMessages: { + [ID_MOCK]: SIGNATURE_REQUEST_MOCK, + }, + unapprovedPersonalMsgCount: 1, + unapprovedTypedMessagesCount: 1, + } as unknown as SignatureControllerState, + }); + + controller.resetState(); + + expect(controller.state).toStrictEqual({ + signatureRequests: {}, unapprovedPersonalMsgs: {}, unapprovedTypedMessages: {}, unapprovedPersonalMsgCount: 0, @@ -241,708 +214,694 @@ describe('SignatureController', () => { }); describe('rejectUnapproved', () => { - beforeEach(() => { - const messages = { - [messageIdMock]: messageMock, - [messageIdMock2]: messageMock, + it('rejects all signature requests with unapproved status', () => { + const signatureRequests = { + '1': SIGNATURE_REQUEST_MOCK, + '2': { + ...SIGNATURE_REQUEST_MOCK, + id: '2', + status: SignatureRequestStatus.Unapproved, + }, + '3': { + ...SIGNATURE_REQUEST_MOCK, + id: '3', + type: SignatureRequestType.TypedSign, + status: SignatureRequestStatus.Unapproved, + }, }; - personalMessageManagerMock.getUnapprovedMessages.mockReturnValueOnce( - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - messages as any, - ); - typedMessageManagerMock.getUnapprovedMessages.mockReturnValueOnce( - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - messages as any, - ); + const { controller } = createController({ + state: { + signatureRequests, + } as unknown as SignatureControllerState, + }); - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - signatureController.update(() => ({ - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - unapprovedMsgs: messages as any, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - unapprovedPersonalMsgs: messages as any, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - unapprovedTypedMessages: messages as any, - })); + controller.rejectUnapproved(); + + expect(controller.state.signatureRequests).toStrictEqual({ + '1': signatureRequests['1'], + '2': { + ...signatureRequests['2'], + status: SignatureRequestStatus.Rejected, + }, + '3': { + ...signatureRequests['3'], + status: SignatureRequestStatus.Rejected, + }, + }); }); - it('rejects all messages in all message managers', () => { - signatureController.rejectUnapproved('Test Reason'); + it('emits event if reason provided', () => { + const signatureRequests = { + '1': SIGNATURE_REQUEST_MOCK, + '2': { + ...SIGNATURE_REQUEST_MOCK, + id: '2', + status: SignatureRequestStatus.Unapproved, + }, + '3': { + ...SIGNATURE_REQUEST_MOCK, + id: '3', + type: SignatureRequestType.TypedSign, + status: SignatureRequestStatus.Unapproved, + }, + }; - expect(personalMessageManagerMock.rejectMessage).toHaveBeenCalledTimes(2); - expect(personalMessageManagerMock.rejectMessage).toHaveBeenCalledWith( - messageIdMock, - ); - expect(personalMessageManagerMock.rejectMessage).toHaveBeenCalledWith( - messageIdMock2, - ); + const listener = jest.fn(); - expect(typedMessageManagerMock.rejectMessage).toHaveBeenCalledTimes(2); - expect(typedMessageManagerMock.rejectMessage).toHaveBeenCalledWith( - messageIdMock, - ); - expect(typedMessageManagerMock.rejectMessage).toHaveBeenCalledWith( - messageIdMock2, - ); - }); + const { controller } = createController({ + state: { + signatureRequests, + } as unknown as SignatureControllerState, + }); + + controller.hub.on('cancelWithReason', listener); - it('fires event with reject reason', () => { - const listenerMock = jest.fn(); - signatureController.hub.on('cancelWithReason', listenerMock); + controller.rejectUnapproved('Custom reason'); - signatureController.rejectUnapproved('Test Reason'); + expect(listener).toHaveBeenCalledTimes(2); - expect(listenerMock).toHaveBeenCalledTimes(4); - expect(listenerMock).toHaveBeenLastCalledWith({ - reason: 'Test Reason', - message: messageMock, + expect(listener).toHaveBeenCalledWith({ + metadata: signatureRequests['2'], + reason: 'Custom reason', + }); + + expect(listener).toHaveBeenCalledWith({ + metadata: signatureRequests['3'], + reason: 'Custom reason', }); }); }); describe('clearUnapproved', () => { - it('resets state in all message managers', () => { - signatureController.clearUnapproved(); - - const defaultState = { - unapprovedMessages: {}, - unapprovedMessagesCount: 0, + it('deletes all signature requests with unapproved status', () => { + const signatureRequests = { + '1': SIGNATURE_REQUEST_MOCK, + '2': { + ...SIGNATURE_REQUEST_MOCK, + id: '2', + status: SignatureRequestStatus.Unapproved, + }, + '3': { + ...SIGNATURE_REQUEST_MOCK, + id: '3', + type: SignatureRequestType.TypedSign, + status: SignatureRequestStatus.Unapproved, + }, }; - expect(personalMessageManagerMock.update).toHaveBeenCalledTimes(1); - expect(personalMessageManagerMock.update).toHaveBeenCalledWith( - defaultState, - ); + const { controller } = createController({ + state: { + signatureRequests, + } as unknown as SignatureControllerState, + }); - expect(typedMessageManagerMock.update).toHaveBeenCalledTimes(1); - expect(typedMessageManagerMock.update).toHaveBeenCalledWith(defaultState); + controller.clearUnapproved(); + + expect(controller.state.signatureRequests).toStrictEqual({ + '1': signatureRequests['1'], + }); }); }); - describe('newUnsignedPersonalMessage', () => { - it('adds message to personal message manager', async () => { - await signatureController.newUnsignedPersonalMessage( - messageParamsMock, - requestMock, - ); + describe.each([ + [ + 'newUnsignedPersonalMessage', + (controller: SignatureController, request = {}) => + controller.newUnsignedPersonalMessage({ ...PARAMS_MOCK }, request), + SignatureRequestType.PersonalSign, + ], + [ + 'newUnsignedTypedMessage', + (controller: SignatureController, request = {}) => + controller.newUnsignedTypedMessage( + PARAMS_MOCK, + request, + SignTypedDataVersion.V1, + { parseJsonData: false }, + ), + SignatureRequestType.TypedSign, + ], + ])('%s', (_title: string, fn, type) => { + it('throws if rejected', async () => { + const { controller, approvalControllerAddRequestMock } = + createController(); + + const error = createErrorMock(); + + approvalControllerAddRequestMock.mockRejectedValueOnce(error); + + await expect( + controller.newUnsignedPersonalMessage({ ...PARAMS_MOCK }, {}), + ).rejects.toMatchObject({ + message: ERROR_MESSAGE_MOCK, + code: ERROR_CODE_MOCK, + }); + }); - expect( - personalMessageManagerMock.addUnapprovedMessage, - ).toHaveBeenCalledTimes(1); + it('invokes success callback if approved', async () => { + const resultCallbackSuccessMock = jest.fn(); - expect( - personalMessageManagerMock.addUnapprovedMessage, - ).toHaveBeenCalledWith( - expect.objectContaining(messageParamsMock), - requestMock, - undefined, - ); + const { + controller, + approvalControllerAddRequestMock, + keyringControllerSignPersonalMessageMock, + keyringControllerSignTypedMessageMock, + } = createController(); - expect(messengerMock.call).toHaveBeenCalledTimes(4); - expect(messengerMock.call).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: messageIdMock, - origin: messageParamsMock.origin, - type: 'personal_sign', - requestData: messageParamsMock, - expectsResult: true, + approvalControllerAddRequestMock.mockResolvedValueOnce({ + resultCallbacks: { + success: resultCallbackSuccessMock, }, - true, + }); + + keyringControllerSignPersonalMessageMock.mockResolvedValueOnce( + SIGNATURE_HASH_MOCK, ); - expect(messengerMock.call).toHaveBeenNthCalledWith( - 3, - 'KeyringController:signPersonalMessage', - messageParamsWithoutIdMock, + + keyringControllerSignTypedMessageMock.mockResolvedValueOnce( + SIGNATURE_HASH_MOCK, ); - }); - it('throws if approval rejected', async () => { - messengerMock.call - .mockResolvedValueOnce({}) // LoggerController:add - .mockRejectedValueOnce({ - message: 'User rejected the request.', - data: { - source: 'MetaMask', - }, - }); // ApprovalController:addRequest - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const error: any = await getError( - async () => - await signatureController.newUnsignedPersonalMessage( - messageParamsMock, - requestMock, - ), + await fn(controller); + + expect(resultCallbackSuccessMock).toHaveBeenCalledTimes(1); + expect(resultCallbackSuccessMock).toHaveBeenCalledWith( + SIGNATURE_HASH_MOCK, ); - expect(error.message).toBe('User rejected the request.'); - expect(error.data).toStrictEqual({ source: 'MetaMask' }); }); - it('throws if cannot get signature', async () => { - mockMessengerAction('KeyringController:signPersonalMessage', async () => { - throw keyringErrorMock; + it('emits finished event if approved', async () => { + const listener = jest.fn(); + + const { controller } = createController(); + + controller.hub.on(`${ID_MOCK}:finished`, listener); + + await fn(controller); + + const state = controller.state.signatureRequests[ID_MOCK]; + + expect(listener).toHaveBeenCalledTimes(1); + expect(listener).toHaveBeenCalledWith(state); + }); + + it('emits finished event if rejected', async () => { + const listener = jest.fn(); + + const { controller, approvalControllerAddRequestMock } = + createController(); + + controller.hub.on(`${ID_MOCK}:finished`, listener); + + const errorMock = createErrorMock(); + + approvalControllerAddRequestMock.mockRejectedValueOnce(errorMock); + + await fn(controller).catch(() => { + // Ignore error }); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const error: any = await getError( - async () => - await signatureController.newUnsignedPersonalMessage( - messageParamsMock, - requestMock, - ), - ); + const state = controller.state.signatureRequests[ID_MOCK]; - expect(messengerMock.call).toHaveBeenCalledTimes(3); - expect(error.message).toBe(keyringErrorMessageMock); - expect(messengerMock.call).toHaveBeenNthCalledWith( - 3, - 'KeyringController:signPersonalMessage', - messageParamsWithoutIdMock, - ); - expect(personalMessageManagerMock.rejectMessage).toHaveBeenCalledTimes(1); - expect(personalMessageManagerMock.rejectMessage).toHaveBeenCalledWith( - messageIdMock, - ); + expect(listener).toHaveBeenCalledTimes(1); + expect(listener).toHaveBeenCalledWith(state); }); - it('calls success callback once message is signed', async () => { - await signatureController.newUnsignedPersonalMessage( - messageParamsMock, - requestMock, - ); + it('adds logs to logging controller if approved', async () => { + const { controller, loggingControllerAddMock } = createController(); + + await fn(controller); + + expect(loggingControllerAddMock).toHaveBeenCalledTimes(2); + + expect(loggingControllerAddMock).toHaveBeenCalledWith({ + type: LogType.EthSignLog, + data: { + signingMethod: expect.any(String), + stage: SigningStage.Proposed, + signingData: expect.any(Object), + }, + }); - expect(resultCallbacksMock.success).toHaveBeenCalledTimes(1); + expect(loggingControllerAddMock).toHaveBeenCalledWith({ + type: LogType.EthSignLog, + data: { + signingMethod: expect.any(String), + stage: SigningStage.Signed, + signingData: expect.any(Object), + }, + }); }); - }); - describe('newUnsignedTypedMessage', () => { - it('adds message to typed message manager', async () => { - const messageParamsWithOriginUndefined = { - ...messageParamsMock, - origin: undefined, - }; - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - signatureController.update(() => ({ - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - unapprovedTypedMessages: { [messageIdMock]: stateMessageMock } as any, - })); - - await signatureController.newUnsignedTypedMessage( - messageParamsWithOriginUndefined, - requestMock, - versionMock, - { parseJsonData: false }, - ); + it('adds logs to logging controller if rejected', async () => { + const { + controller, + loggingControllerAddMock, + approvalControllerAddRequestMock, + } = createController(); - expect( - typedMessageManagerMock.addUnapprovedMessage, - ).toHaveBeenCalledTimes(1); - expect(typedMessageManagerMock.addUnapprovedMessage).toHaveBeenCalledWith( - messageParamsWithOriginUndefined, - requestMock, - versionMock, - ); + const errorMock = createErrorMock(); - expect(messengerMock.call).toHaveBeenCalledTimes(4); - expect(messengerMock.call).toHaveBeenNthCalledWith( - 2, - 'ApprovalController:addRequest', - { - id: messageIdMock, - origin: ORIGIN_METAMASK, - type: 'eth_signTypedData', - requestData: messageParamsWithOriginUndefined, - expectsResult: true, + approvalControllerAddRequestMock.mockRejectedValueOnce(errorMock); + + await expect(fn(controller)).rejects.toThrow(errorMock); + + expect(loggingControllerAddMock).toHaveBeenCalledTimes(2); + + expect(loggingControllerAddMock).toHaveBeenCalledWith({ + type: LogType.EthSignLog, + data: { + signingMethod: expect.any(String), + stage: SigningStage.Proposed, + signingData: expect.any(Object), }, - true, - ); - expect(messengerMock.call).toHaveBeenNthCalledWith( - 3, - 'KeyringController:signTypedMessage', - messageParamsWithoutIdMock, - versionMock, - ); + }); + + expect(loggingControllerAddMock).toHaveBeenCalledWith({ + type: LogType.EthSignLog, + data: { + signingMethod: expect.any(String), + stage: SigningStage.Rejected, + signingData: expect.any(Object), + }, + }); }); - it('does not set as signed, messages with deferSetAsSigned', async () => { - const deferredMessageParams = { - ...messageParamsMock, - deferSetAsSigned: true, - }; - typedMessageManagerMock.approveMessage.mockReset(); - typedMessageManagerMock.approveMessage.mockResolvedValueOnce( - deferredMessageParams, - ); + it('populates origin from request', async () => { + const { controller } = createController(); - await signatureController.newUnsignedTypedMessage( - messageParamsMock, - requestMock, - versionMock, - { parseJsonData: false }, - ); + await fn(controller, { origin: 'test' }); + + expect( + controller.state.signatureRequests[ID_MOCK].messageParams.origin, + ).toBe('test'); + }); + + it('populates request ID from request', async () => { + const { controller } = createController(); + + await fn(controller, { id: 'test' }); expect( - typedMessageManagerMock.setMessageStatusSigned, - ).not.toHaveBeenCalled(); + controller.state.signatureRequests[ID_MOCK].messageParams.requestId, + ).toBe('test'); }); - it('parses JSON string in data if not V1', async () => { - const jsonData = { test: 'value' }; + it('emits unapproved message event', async () => { + const listener = jest.fn(); + + const { controller } = createController(); - typedMessageManagerMock.approveMessage.mockReset(); - typedMessageManagerMock.approveMessage.mockResolvedValueOnce({ - ...messageParamsMock2, - deferSetAsSigned: false, - data: JSON.stringify(jsonData), + controller.hub.on('unapprovedMessage', listener); + + await fn(controller); + + expect(listener).toHaveBeenCalledTimes(1); + expect(listener).toHaveBeenCalledWith({ + messageParams: PARAMS_MOCK, + metamaskId: ID_MOCK, }); + }); - await signatureController.newUnsignedTypedMessage( - messageParamsMock, - requestMock, - 'V2', - { parseJsonData: true }, - ); + it('emits signed event after sign', async () => { + const listener = jest.fn(); + + const { + controller, + keyringControllerSignPersonalMessageMock, + keyringControllerSignTypedMessageMock, + } = createController(); - expect(messengerMock.call).toHaveBeenNthCalledWith( - 3, - 'KeyringController:signTypedMessage', - { ...messageParamsMock2, data: jsonData, deferSetAsSigned: false }, - 'V2', + keyringControllerSignPersonalMessageMock.mockResolvedValueOnce( + SIGNATURE_HASH_MOCK, ); - }); - it('throws if approval rejected', async () => { - messengerMock.call - .mockResolvedValueOnce({}) // LoggerController:add - .mockRejectedValueOnce({ - message: 'User rejected the request.', - data: { source: 'Metamask' }, - }); // ApprovalController:addRequest - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const error: any = await getError( - async () => - await signatureController.newUnsignedTypedMessage( - messageParamsMock, - requestMock, - versionMock, - { parseJsonData: true }, - ), + keyringControllerSignTypedMessageMock.mockResolvedValueOnce( + SIGNATURE_HASH_MOCK, ); - expect(error.message).toBe('User rejected the request.'); - expect(error.data).toStrictEqual({ source: 'Metamask' }); - }); - it('throws if cannot get signature', async () => { - mockMessengerAction('KeyringController:signTypedMessage', async () => { - throw keyringErrorMock; + controller.hub.on(`${type}:signed`, listener); + + await fn(controller); + + expect(listener).toHaveBeenCalledTimes(1); + expect(listener).toHaveBeenCalledWith({ + signature: SIGNATURE_HASH_MOCK, + messageId: ID_MOCK, }); - typedMessageManagerMock.addUnapprovedMessage.mockResolvedValue( - messageIdMock, - ); - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const error: any = await getError( - async () => - await signatureController.newUnsignedTypedMessage( - messageParamsMock, - requestMock, - versionMock, - { parseJsonData: true }, - ), - ); - expect(error.message).toBe(keyringErrorMessageMock); - expect( - typedMessageManagerMock.setMessageStatusErrored, - ).toHaveBeenCalledTimes(1); - expect( - typedMessageManagerMock.setMessageStatusErrored, - ).toHaveBeenCalledWith(messageIdMock, keyringErrorMessageMock); }); - it('calls success callback once message is signed', async () => { - await signatureController.newUnsignedTypedMessage( - messageParamsMock, - requestMock, - versionMock, - { parseJsonData: false }, - ); + it('emits sign error event if signing fails', async () => { + const errorMock = createErrorMock(); - expect(resultCallbacksMock.success).toHaveBeenCalledTimes(1); - }); - }); + const listener = jest.fn(); - describe('setPersonalMessageInProgress', () => { - it('calls the message manager', async () => { - signatureController.setPersonalMessageInProgress( - messageParamsMock.metamaskId, - ); + const { + controller, + keyringControllerSignTypedMessageMock, + keyringControllerSignPersonalMessageMock, + } = createController(); - expect( - personalMessageManagerMock.setMessageStatusInProgress, - ).toHaveBeenCalledTimes(1); + controller.hub.on(`${ID_MOCK}:signError`, listener); + + keyringControllerSignPersonalMessageMock.mockRejectedValueOnce(errorMock); + keyringControllerSignTypedMessageMock.mockRejectedValueOnce(errorMock); + + await fn(controller).catch(() => { + // Ignore error + }); + + expect(listener).toHaveBeenCalledTimes(1); + expect(listener).toHaveBeenCalledWith({ + error: errorMock, + }); }); - }); - describe('setTypedMessageInProgress', () => { - it('calls the message manager', async () => { - signatureController.setTypedMessageInProgress( - messageParamsMock.metamaskId, - ); + it('invokes error callback if signing fails', async () => { + const resultCallbackErrorMock = jest.fn(); - expect( - typedMessageManagerMock.setMessageStatusInProgress, - ).toHaveBeenCalledTimes(1); + const errorMock = createErrorMock(); + + const { + controller, + approvalControllerAddRequestMock, + keyringControllerSignPersonalMessageMock, + keyringControllerSignTypedMessageMock, + } = createController(); + + approvalControllerAddRequestMock.mockResolvedValueOnce({ + resultCallbacks: { + error: resultCallbackErrorMock, + }, + }); + + keyringControllerSignPersonalMessageMock.mockRejectedValueOnce(errorMock); + keyringControllerSignTypedMessageMock.mockRejectedValueOnce(errorMock); + + await expect(fn(controller)).rejects.toThrow(ERROR_MESSAGE_MOCK); + + expect(resultCallbackErrorMock).toHaveBeenCalledTimes(1); + expect(resultCallbackErrorMock).toHaveBeenCalledWith(errorMock); }); }); - describe('trySetMessageMetadata', () => { - it('sets the metadata in a message manager', () => { - signatureController.setMessageMetadata( - messageParamsMock.metamaskId, - messageParamsMock.data, + describe('newUnsignedPersonalMessage', () => { + it('returns signature hash if approved', async () => { + const { controller, keyringControllerSignPersonalMessageMock } = + createController(); + + keyringControllerSignPersonalMessageMock.mockResolvedValueOnce( + SIGNATURE_HASH_MOCK, ); - expect(personalMessageManagerMock.setMetadata).toHaveBeenCalledTimes(1); - expect(personalMessageManagerMock.setMetadata).toHaveBeenCalledWith( - messageIdMock, - messageParamsWithoutIdMock.data, + const result = await controller.newUnsignedPersonalMessage( + { ...PARAMS_MOCK }, + {}, ); - expect(typedMessageManagerMock.setMetadata).not.toHaveBeenCalled(); + + expect(result).toBe(SIGNATURE_HASH_MOCK); }); - it('should return false when an error occurs', () => { - jest - .spyOn(personalMessageManagerMock, 'setMetadata') - .mockImplementation(() => { - throw new Error('mocked error'); - }); + it('adds SIWE data', async () => { + const { controller } = createController(); - const result = signatureController.setMessageMetadata( - messageParamsMock.metamaskId, - messageParamsMock.data, - ); + const siweMock = { + isSIWEMessage: true, + parsedMessage: { domain: 'test' }, + } as SIWEMessage; - expect(result).toBeUndefined(); - expect(personalMessageManagerMock.setMetadata).toHaveBeenCalledTimes(1); - expect(personalMessageManagerMock.setMetadata).toHaveBeenCalledWith( - messageIdMock, - messageParamsWithoutIdMock.data, - ); + detectSIWEMock.mockReturnValueOnce(siweMock); + + await controller.newUnsignedPersonalMessage({ ...PARAMS_MOCK }, {}); + + const messageParams = controller.state.signatureRequests[ID_MOCK] + .messageParams as MessageParamsPersonal; + + expect(messageParams.siwe).toStrictEqual(siweMock); }); }); - describe('setDeferredSignSuccess', () => { - it('sets a message status as signed in a message manager', () => { - signatureController.setDeferredSignSuccess( - messageParamsMock.metamaskId, - messageParamsMock.data, + describe('newUnsignedTypedMessage', () => { + it('returns signature hash if approved', async () => { + const { controller, keyringControllerSignTypedMessageMock } = + createController(); + + keyringControllerSignTypedMessageMock.mockResolvedValueOnce( + SIGNATURE_HASH_MOCK, ); - expect( - personalMessageManagerMock.setMessageStatusSigned, - ).toHaveBeenCalledTimes(1); - expect( - personalMessageManagerMock.setMessageStatusSigned, - ).toHaveBeenCalledWith(messageIdMock, messageParamsWithoutIdMock.data); - expect( - typedMessageManagerMock.setMessageStatusSigned, - ).not.toHaveBeenCalled(); + const result = await controller.newUnsignedTypedMessage( + PARAMS_MOCK, + {}, + SignTypedDataVersion.V1, + { parseJsonData: false }, + ); + + expect(result).toBe(SIGNATURE_HASH_MOCK); }); - it('should return undefined when an error occurs', () => { - jest - .spyOn(personalMessageManagerMock, 'setMessageStatusSigned') - .mockImplementation(() => { - throw new Error('mocked error'); - }); + it.each([SignTypedDataVersion.V3, SignTypedDataVersion.V4])( + 'supports data as string with version %s', + async (version) => { + const { controller, keyringControllerSignTypedMessageMock } = + createController(); - const result = signatureController.setDeferredSignSuccess( - messageParamsMock.metamaskId, - messageParamsMock.data, - ); + keyringControllerSignTypedMessageMock.mockResolvedValueOnce( + SIGNATURE_HASH_MOCK, + ); - expect(result).toBeUndefined(); - expect( - personalMessageManagerMock.setMessageStatusSigned, - ).toHaveBeenCalledTimes(1); - expect( - personalMessageManagerMock.setMessageStatusSigned, - ).toHaveBeenCalledWith(messageIdMock, messageParamsWithoutIdMock.data); - }); - }); + const result = await controller.newUnsignedTypedMessage( + { + ...PARAMS_MOCK, + data: JSON.stringify({ test: 123 }), + }, + {}, + version as SignTypedDataVersion, + { parseJsonData: true }, + ); + + expect(keyringControllerSignTypedMessageMock).toHaveBeenCalledWith( + { + ...PARAMS_MOCK, + data: { test: 123 }, + }, + version, + ); - describe('trySetDeferredSignError', () => { - it('rejects a message by calling rejectMessage', () => { - signatureController.setDeferredSignError(messageParamsMock.metamaskId); + expect(result).toBe(SIGNATURE_HASH_MOCK); + }, + ); - expect(personalMessageManagerMock.rejectMessage).toHaveBeenCalledTimes(1); - expect(personalMessageManagerMock.rejectMessage).toHaveBeenCalledWith( - messageIdMock, + it('ignores parseJsonData if version is V1', async () => { + const { controller, keyringControllerSignTypedMessageMock } = + createController(); + + keyringControllerSignTypedMessageMock.mockResolvedValueOnce( + SIGNATURE_HASH_MOCK, ); - expect(typedMessageManagerMock.rejectMessage).not.toHaveBeenCalled(); - }); + const result = await controller.newUnsignedTypedMessage( + { + ...PARAMS_MOCK, + data: JSON.stringify({ test: 123 }), + }, + {}, + SignTypedDataVersion.V1, + { parseJsonData: true }, + ); - it('rejects message on next message manager if first throws', () => { - jest - .spyOn(personalMessageManagerMock, 'rejectMessage') - .mockImplementation(() => { - throw new Error('mocked error'); - }); - jest - .spyOn(personalMessageManagerMock, 'rejectMessage') - .mockImplementation(() => { - throw new Error('mocked error'); - }); + expect(keyringControllerSignTypedMessageMock).toHaveBeenCalledWith( + { + ...PARAMS_MOCK, + data: JSON.stringify({ test: 123 }), + }, + SignTypedDataVersion.V1, + ); - expect(() => - signatureController.setDeferredSignError(messageParamsMock.metamaskId), - ).not.toThrow(); + expect(result).toBe(SIGNATURE_HASH_MOCK); }); - it('should throw an error when tryForEachMessageManager fails', () => { - jest - .spyOn(personalMessageManagerMock, 'rejectMessage') - .mockImplementation(() => { - throw new Error('mocked error'); - }); - jest - .spyOn(typedMessageManagerMock, 'rejectMessage') - .mockImplementation(() => { - throw new Error('mocked error'); - }); + it('sets status to errored if signing fails', async () => { + const { controller, keyringControllerSignTypedMessageMock } = + createController(); + + const errorMock = createErrorMock(); + + keyringControllerSignTypedMessageMock.mockRejectedValueOnce(errorMock); + + await expect( + controller.newUnsignedTypedMessage( + PARAMS_MOCK, + {}, + SignTypedDataVersion.V3, + { parseJsonData: false }, + ), + ).rejects.toThrow(errorMock); - expect(() => - signatureController.setDeferredSignError(messageParamsMock.metamaskId), - ).toThrow('Message not found'); + expect(controller.state.signatureRequests[ID_MOCK].status).toBe( + SignatureRequestStatus.Errored, + ); + expect(controller.state.signatureRequests[ID_MOCK].error).toBe( + ERROR_MESSAGE_MOCK, + ); }); }); - describe('messages getter', () => { - const message = [ - { - name: 'some message', - type: 'type', - value: 'value', - messageParams: { - data: [], - from: '0x0123', - }, - time: 1, - status: '', - id: '1', - }, - ]; - - it('returns all the messages from TypedMessageManager and PersonalMessageManager', () => { - typedMessageManagerMock.getAllMessages.mockReturnValueOnce(message); - personalMessageManagerMock.getAllMessages.mockReturnValueOnce([]); - expect(signatureController.messages).toMatchObject({ - '1': { - id: '1', - messageParams: { - data: [], - from: '0x0123', + describe('setDeferredSignSuccess', () => { + it('sets the signature and status on the signature request', () => { + const { controller } = createController({ + state: { + signatureRequests: { + [ID_MOCK]: { + ...SIGNATURE_REQUEST_MOCK, + status: SignatureRequestStatus.InProgress, + }, }, - name: 'some message', - status: '', - time: 1, - type: 'type', - value: 'value', - }, + } as unknown as SignatureControllerState, }); - }); - }); - describe('message manager events', () => { - it.each([ - ['personal message manager', personalMessageManagerMock], - ['typed message manager', typedMessageManagerMock], - ])('bubbles update badge event from %s', (_, messageManager) => { - const mockListener = jest.fn(); - const mockHub = messageManager.hub.on as jest.Mock; + controller.setDeferredSignSuccess(ID_MOCK, SIGNATURE_HASH_MOCK); - signatureController.hub.on('updateBadge', mockListener); - mockHub.mock.calls[0][1](); + expect(controller.state.signatureRequests[ID_MOCK].rawSig).toBe( + SIGNATURE_HASH_MOCK, + ); - expect(mockListener).toHaveBeenCalledTimes(1); + expect(controller.state.signatureRequests[ID_MOCK].status).toBe( + SignatureRequestStatus.Signed, + ); }); - // eslint-disable-next-line jest/expect-expect - it('does not throw if approval request promise throws', async () => { - const mockHub = personalMessageManagerMock.hub.on as jest.Mock; + it('resolves defered signature request', async () => { + const { controller } = createController(); + let resolved = false; - messengerMock.call.mockRejectedValueOnce('Test Error'); + const signaturePromise = controller + .newUnsignedPersonalMessage( + { + ...PARAMS_MOCK, + deferSetAsSigned: true, + }, + {}, + ) + .then((result) => { + resolved = true; + return result; + }); - mockHub.mock.calls[1][1](messageParamsMock); - }); + await flushPromises(); - it('updates state on message manager state change', async () => { - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/await-thenable - await personalMessageManagerMock.subscribe.mock.calls[0][0]({ - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - unapprovedMessages: { [messageIdMock]: coreMessageMock as any }, - unapprovedMessagesCount: 3, - }); + expect(resolved).toBe(false); - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/await-thenable - expect(await signatureController.state).toStrictEqual({ - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - unapprovedPersonalMsgs: { [messageIdMock]: stateMessageMock as any }, - unapprovedTypedMessages: {}, - unapprovedPersonalMsgCount: 3, - unapprovedTypedMessagesCount: 0, - }); + controller.setDeferredSignSuccess(ID_MOCK, SIGNATURE_HASH_MOCK); + + await flushPromises(); + + expect(resolved).toBe(true); + expect(await signaturePromise).toBe(SIGNATURE_HASH_MOCK); }); - it('updates state on personal message manager state change', async () => { - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/await-thenable - await personalMessageManagerMock.subscribe.mock.calls[0][0]({ - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - unapprovedMessages: { [messageIdMock]: coreMessageMock as any }, - unapprovedMessagesCount: 4, - }); + it('throws if signature request not found', () => { + const { controller } = createController(); - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/await-thenable - expect(await signatureController.state).toStrictEqual({ - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - unapprovedPersonalMsgs: { [messageIdMock]: stateMessageMock as any }, - unapprovedTypedMessages: {}, - unapprovedPersonalMsgCount: 4, - unapprovedTypedMessagesCount: 0, - }); + expect(() => { + controller.setDeferredSignSuccess(ID_MOCK, SIGNATURE_HASH_MOCK); + }).toThrow(`Signature request with id ${ID_MOCK} not found`); }); + }); - it('updates state on typed message manager state change', async () => { - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/await-thenable - await typedMessageManagerMock.subscribe.mock.calls[0][0]({ - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - unapprovedMessages: { [messageIdMock]: coreMessageMock as any }, - unapprovedMessagesCount: 5, + describe('setMessageMetadata', () => { + it('sets the metadata on the signature request', () => { + const { controller } = createController({ + state: { + signatureRequests: { + [ID_MOCK]: SIGNATURE_REQUEST_MOCK, + }, + } as unknown as SignatureControllerState, }); - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/await-thenable - expect(await signatureController.state).toStrictEqual({ - unapprovedPersonalMsgs: {}, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - unapprovedTypedMessages: { [messageIdMock]: stateMessageMock as any }, - unapprovedPersonalMsgCount: 0, - unapprovedTypedMessagesCount: 5, + controller.setMessageMetadata(ID_MOCK, { test: 123 }); + + expect( + controller.state.signatureRequests[ID_MOCK].metadata, + ).toStrictEqual({ + test: 123, }); }); }); - describe('logging controller events', () => { - it('sends proposed sign log event after approval is shown', async () => { - const testVersion = 'V1'; - await signatureController.newUnsignedTypedMessage( - messageParamsMock, - requestMock, - testVersion, - { parseJsonData: false }, - ); - - expect(messengerMock.call).toHaveBeenNthCalledWith( - 1, - 'LoggingController:add', - { - type: LogType.EthSignLog, - data: { - signingMethod: SigningMethod.EthSignTypedData, - stage: SigningStage.Proposed, - signingData: expect.objectContaining({ - version: testVersion, - from: messageParamsMock.from, - data: messageParamsMock.data, - origin: messageParamsMock.origin, - }), + describe('setDeferredSignError', () => { + it('sets the status on the signature request to rejected', () => { + const { controller } = createController({ + state: { + signatureRequests: { + [ID_MOCK]: { + ...SIGNATURE_REQUEST_MOCK, + status: SignatureRequestStatus.InProgress, + }, }, - }, + } as unknown as SignatureControllerState, + }); + + controller.setDeferredSignError(ID_MOCK); + + expect(controller.state.signatureRequests[ID_MOCK].status).toBe( + SignatureRequestStatus.Rejected, ); }); - it('sends rejected sign log event if approval is rejected', async () => { - const testVersion = 'V3'; - messengerMock.call - .mockResolvedValueOnce({}) // LoggerController:add - .mockRejectedValueOnce({}); // ApprovalController:addRequest - await getError( - async () => - await signatureController.newUnsignedTypedMessage( - messageParamsMock, - requestMock, - testVersion, - { parseJsonData: true }, - ), - ); - expect(messengerMock.call).toHaveBeenNthCalledWith( - 3, - 'LoggingController:add', - { - type: LogType.EthSignLog, - data: { - signingMethod: SigningMethod.EthSignTypedDataV3, - stage: SigningStage.Rejected, - signingData: expect.objectContaining({ - version: testVersion, - from: messageParamsMock.from, - data: messageParamsMock.data, - origin: messageParamsMock.origin, - }), + it('rejects defered signature request', async () => { + const { controller } = createController(); + let rejectedError; + + controller + .newUnsignedPersonalMessage( + { + ...PARAMS_MOCK, + deferSetAsSigned: true, }, - }, - ); - }); + {}, + ) + .catch((error) => { + rejectedError = error; + }); - it('sends signed log event if signature operation is complete', async () => { - const testVersion = 'V4'; - await signatureController.newUnsignedTypedMessage( - messageParamsMock, - requestMock, - testVersion, - { parseJsonData: false }, + await flushPromises(); + + expect(rejectedError).toBeUndefined(); + + controller.setDeferredSignError(ID_MOCK); + + await flushPromises(); + + expect(rejectedError).toStrictEqual( + new Error( + 'MetaMask personal_sign Signature: User denied message signature.', + ), ); + }); + }); - expect(messengerMock.call).toHaveBeenNthCalledWith( - 4, - 'LoggingController:add', - { - type: LogType.EthSignLog, - data: { - signingMethod: SigningMethod.EthSignTypedDataV4, - stage: SigningStage.Signed, - signingData: expect.objectContaining({ - version: testVersion, - from: messageParamsMock.from, - data: messageParamsMock.data, - origin: messageParamsMock.origin, - }), + describe.each([ + 'setTypedMessageInProgress', + 'setPersonalMessageInProgress', + ] as const)('%s', (fn) => { + it('sets the status on the signature request to in progress', () => { + const { controller } = createController({ + state: { + signatureRequests: { + [ID_MOCK]: { + ...SIGNATURE_REQUEST_MOCK, + status: SignatureRequestStatus.Unapproved, + }, }, - }, + } as unknown as SignatureControllerState, + }); + + controller[fn](ID_MOCK); + + expect(controller.state.signatureRequests[ID_MOCK].status).toBe( + SignatureRequestStatus.InProgress, ); }); }); diff --git a/packages/signature-controller/src/SignatureController.ts b/packages/signature-controller/src/SignatureController.ts index 4ae73a8523..6ec64bf6c1 100644 --- a/packages/signature-controller/src/SignatureController.ts +++ b/packages/signature-controller/src/SignatureController.ts @@ -9,45 +9,53 @@ import type { RestrictedControllerMessenger, } from '@metamask/base-controller'; import { BaseController } from '@metamask/base-controller'; -import { ApprovalType, ORIGIN_METAMASK } from '@metamask/controller-utils'; import type { TraceCallback, TraceContext } from '@metamask/controller-utils'; +import { + ApprovalType, + detectSIWE, + ORIGIN_METAMASK, +} from '@metamask/controller-utils'; import type { KeyringControllerSignMessageAction, KeyringControllerSignPersonalMessageAction, KeyringControllerSignTypedMessageAction, - SignTypedDataVersion, } from '@metamask/keyring-controller'; +import { SignTypedDataVersion } from '@metamask/keyring-controller'; import { SigningMethod, - SigningStage, LogType, + SigningStage, + type AddLog, } from '@metamask/logging-controller'; -import type { AddLog } from '@metamask/logging-controller'; +import type { Hex, Json } from '@metamask/utils'; +import EventEmitter from 'events'; +import { v1 as random } from 'uuid'; + +import { projectLogger as log } from './logger'; +import { SignatureRequestStatus, SignatureRequestType } from './types'; import type { - PersonalMessageParams, - PersonalMessageParamsMetamask, - TypedMessageParams, - TypedMessageParamsMetamask, - AbstractMessageManager, - AbstractMessage, - MessageManagerState, - AbstractMessageParams, - AbstractMessageParamsMetamask, + MessageParamsPersonal, + MessageParamsTyped, OriginalRequest, - TypedMessage, - PersonalMessage, -} from '@metamask/message-manager'; + SignatureRequest, + MessageParams, + TypedSigningOptions, + LegacyStateMessage, + StateSIWEMessage, +} from './types'; import { - PersonalMessageManager, - TypedMessageManager, -} from '@metamask/message-manager'; -import type { Hex, Json } from '@metamask/utils'; -import EventEmitter from 'events'; -import { cloneDeep } from 'lodash'; + normalizePersonalMessageParams, + normalizeTypedMessageParams, +} from './utils/normalize'; +import { + validatePersonalSignatureRequest, + validateTypedSignatureRequest, +} from './utils/validation'; const controllerName = 'SignatureController'; const stateMetadata = { + signatureRequests: { persist: false, anonymous: false }, unapprovedPersonalMsgs: { persist: false, anonymous: false }, unapprovedTypedMessages: { persist: false, anonymous: false }, unapprovedPersonalMsgCount: { persist: false, anonymous: false }, @@ -55,24 +63,48 @@ const stateMetadata = { }; const getDefaultState = () => ({ + signatureRequests: {}, unapprovedPersonalMsgs: {}, unapprovedTypedMessages: {}, unapprovedPersonalMsgCount: 0, unapprovedTypedMessagesCount: 0, }); -type CoreMessage = AbstractMessage & { - messageParams: AbstractMessageParams; -}; +/** List of statuses that will not be updated and trigger the finished event. */ +const FINAL_STATUSES: SignatureRequestStatus[] = [ + SignatureRequestStatus.Signed, + SignatureRequestStatus.Rejected, + SignatureRequestStatus.Errored, +]; -type StateMessage = Required & { - msgParams: Required; -}; +export type SignatureControllerState = { + /** + * Map of all signature requests including all types and statuses, keyed by ID. + */ + signatureRequests: Record; + + /** + * Map of personal messages with the unapproved status, keyed by ID. + * @deprecated - Use `signatureRequests` instead. + */ + unapprovedPersonalMsgs: Record; -type SignatureControllerState = { - unapprovedPersonalMsgs: Record; - unapprovedTypedMessages: Record; + /** + * Map of typed messages with the unapproved status, keyed by ID. + * @deprecated - Use `signatureRequests` instead. + */ + unapprovedTypedMessages: Record; + + /** + * Number of unapproved personal messages. + * @deprecated - Use `signatureRequests` instead. + */ unapprovedPersonalMsgCount: number; + + /** + * Number of unapproved typed messages. + * @deprecated - Use `signatureRequests` instead. + */ unapprovedTypedMessagesCount: number; }; @@ -83,10 +115,6 @@ type AllowedActions = | KeyringControllerSignTypedMessageAction | AddLog; -type TypedMessageSigningOptions = { - parseJsonData: boolean; -}; - export type GetSignatureState = ControllerGetStateAction< typeof controllerName, SignatureControllerState @@ -110,17 +138,39 @@ export type SignatureControllerMessenger = RestrictedControllerMessenger< >; export type SignatureControllerOptions = { - getAllState: () => unknown; + /** + * @deprecated No longer in use. + */ + getAllState?: () => unknown; + + /** + * Callback that returns the current chain ID. + */ getCurrentChainId: () => Hex; + + /** + * Restricted controller messenger required by the signature controller. + */ messenger: SignatureControllerMessenger; + + /** + * @deprecated No longer in use. + */ securityProviderRequest?: ( - // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any requestData: any, methodName: string, - // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any ) => Promise; + + /** + * Initial state of the controller. + */ + state?: SignatureControllerState; + + /** + * Callback to record the duration of code. + */ trace?: TraceCallback; }; @@ -134,13 +184,7 @@ export class SignatureController extends BaseController< > { hub: EventEmitter; - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - #getAllState: () => any; - - #personalMessageManager: PersonalMessageManager; - - #typedMessageManager: TypedMessageManager; + #getCurrentChainId: () => Hex; #trace: TraceCallback; @@ -148,113 +192,66 @@ export class SignatureController extends BaseController< * Construct a Sign controller. * * @param options - The controller options. + * @param options.getCurrentChainId - A function that returns the current chain ID. * @param options.messenger - The restricted controller messenger for the sign controller. - * @param options.getAllState - Callback to retrieve all user state. - * @param options.securityProviderRequest - A function for verifying a message, whether it is malicious or not. - * @param options.getCurrentChainId - A function for retrieving the current chainId. + * @param options.state - Initial state to set on this controller. * @param options.trace - Callback to generate trace information. */ constructor({ - getAllState, getCurrentChainId, messenger, - securityProviderRequest, + state, trace, }: SignatureControllerOptions) { super({ name: controllerName, metadata: stateMetadata, messenger, - state: getDefaultState(), + state: { + ...getDefaultState(), + ...state, + }, }); - this.#getAllState = getAllState; - this.hub = new EventEmitter(); - this.#personalMessageManager = new PersonalMessageManager( - undefined, - undefined, - securityProviderRequest, - ); - this.#typedMessageManager = new TypedMessageManager( - undefined, - undefined, - securityProviderRequest, - undefined, - getCurrentChainId, - ); + this.#getCurrentChainId = getCurrentChainId; this.#trace = trace ?? (((_request, fn) => fn?.()) as TraceCallback); - - this.#handleMessageManagerEvents( - this.#personalMessageManager, - 'unapprovedPersonalMessage', - ); - this.#handleMessageManagerEvents( - this.#typedMessageManager, - 'unapprovedTypedMessage', - ); - - this.#subscribeToMessageState( - this.#personalMessageManager, - (state, newMessages, messageCount) => { - state.unapprovedPersonalMsgs = newMessages; - state.unapprovedPersonalMsgCount = messageCount; - }, - ); - - this.#subscribeToMessageState( - this.#typedMessageManager, - (state, newMessages, messageCount) => { - state.unapprovedTypedMessages = newMessages; - state.unapprovedTypedMessagesCount = messageCount; - }, - ); } /** * A getter for the number of 'unapproved' PersonalMessages in this.messages. - * + * @deprecated Use `signatureRequests` state instead. * @returns The number of 'unapproved' PersonalMessages in this.messages */ get unapprovedPersonalMessagesCount(): number { - return this.#personalMessageManager.getUnapprovedMessagesCount(); + return this.state.unapprovedPersonalMsgCount; } /** * A getter for the number of 'unapproved' TypedMessages in this.messages. - * + * @deprecated Use `signatureRequests` state instead. * @returns The number of 'unapproved' TypedMessages in this.messages */ get unapprovedTypedMessagesCount(): number { - return this.#typedMessageManager.getUnapprovedMessagesCount(); + return this.state.unapprovedTypedMessagesCount; } /** * A getter for returning all messages. - * + * @deprecated Use `signatureRequests` state instead. * @returns The object containing all messages. */ - get messages(): { [id: string]: PersonalMessage | TypedMessage } { - const messages = [ - ...this.#typedMessageManager.getAllMessages(), - ...this.#personalMessageManager.getAllMessages(), - ]; - - const messagesObject = messages.reduce<{ - [id: string]: PersonalMessage | TypedMessage; - }>((acc, message) => { - acc[message.id] = message; - return acc; - }, {}); - - return messagesObject; + get messages(): { [id: string]: SignatureRequest } { + return this.state.signatureRequests; } /** * Reset the controller state to the initial state. */ resetState() { - this.update(() => getDefaultState()); + this.#updateState((state) => { + Object.assign(state, getDefaultState()); + }); } /** @@ -263,585 +260,463 @@ export class SignatureController extends BaseController< * @param reason - A message to indicate why. */ rejectUnapproved(reason?: string) { - this.#rejectUnapproved(this.#personalMessageManager, reason); - this.#rejectUnapproved(this.#typedMessageManager, reason); + const unapprovedSignatureRequests = Object.values( + this.state.signatureRequests, + ).filter( + (metadata) => metadata.status === SignatureRequestStatus.Unapproved, + ); + + for (const metadata of unapprovedSignatureRequests) { + this.#rejectSignatureRequest(metadata.id, reason); + } } /** * Clears all unapproved messages from memory. */ clearUnapproved() { - this.#clearUnapproved(this.#personalMessageManager); - this.#clearUnapproved(this.#typedMessageManager); + this.#updateState((state) => { + Object.values(state.signatureRequests) + .filter( + (metadata) => metadata.status === SignatureRequestStatus.Unapproved, + ) + .forEach((metadata) => delete state.signatureRequests[metadata.id]); + }); } /** - * Called when a dapp uses the personal_sign method. - * - * We currently define personal_sign mostly for legacy Dapps. + * Called when a dApp uses the personal_sign method. + * We currently provide personal_sign mostly for legacy dApps. * - * @param messageParams - The params of the message to sign & return to the Dapp. - * @param req - The original request, containing the origin. + * @param messageParams - The params of the message to sign and return to the dApp. + * @param request - The original request, containing the origin. * @param options - An options bag for the method. * @param options.traceContext - The parent context for any new traces. - * @returns Promise resolving to the raw data of the signature request. + * @returns Promise resolving to the raw signature hash generated from the signature request. */ async newUnsignedPersonalMessage( - messageParams: PersonalMessageParams, - req: OriginalRequest, + messageParams: MessageParamsPersonal, + request: OriginalRequest, options: { traceContext?: TraceContext } = {}, ): Promise { - return this.#newUnsignedAbstractMessage( - this.#personalMessageManager, - ApprovalType.PersonalSign, - SigningMethod.PersonalSign, - 'Personal Message', - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/no-misused-promises - this.#signPersonalMessage.bind(this), + validatePersonalSignatureRequest(messageParams); + + const normalizedMessageParams = + normalizePersonalMessageParams(messageParams); + + normalizedMessageParams.siwe = detectSIWE( messageParams, - req, - undefined, - undefined, - options.traceContext, - ); + ) as StateSIWEMessage; + + return this.#processSignatureRequest({ + messageParams: normalizedMessageParams, + request, + type: SignatureRequestType.PersonalSign, + approvalType: ApprovalType.PersonalSign, + traceContext: options.traceContext, + }); } /** - * Called when a dapp uses the eth_signTypedData method, per EIP 712. + * Called when a dapp uses the eth_signTypedData method, per EIP-712. * - * @param messageParams - The params passed to eth_signTypedData. - * @param req - The original request, containing the origin. - * @param version - The version indicating the format of the typed data. - * @param signingOpts - An options bag for signing. - * @param signingOpts.parseJsonData - Whether to parse the JSON before signing. + * @param messageParams - The params of the message to sign and return to the dApp. + * @param request - The original request, containing the origin. + * @param version - The version of the signTypedData request. + * @param signingOptions - Options for signing the typed message. * @param options - An options bag for the method. * @param options.traceContext - The parent context for any new traces. - * @returns Promise resolving to the raw data of the signature request. + * @returns Promise resolving to the raw signature hash generated from the signature request. */ async newUnsignedTypedMessage( - messageParams: TypedMessageParams, - req: OriginalRequest, + messageParams: MessageParamsTyped, + request: OriginalRequest, version: string, - signingOpts: TypedMessageSigningOptions, + signingOptions: TypedSigningOptions, options: { traceContext?: TraceContext } = {}, ): Promise { - const signTypeForLogger = this.#getSignTypeForLogger(version); - return this.#newUnsignedAbstractMessage( - this.#typedMessageManager, - ApprovalType.EthSignTypedData, - signTypeForLogger, - 'Typed Message', - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/no-misused-promises - this.#signTypedMessage.bind(this), + validateTypedSignatureRequest( messageParams, - req, - version, - signingOpts, - options.traceContext, + version as SignTypedDataVersion, + this.#getCurrentChainId(), + ); + + const normalizedMessageParams = normalizeTypedMessageParams( + messageParams, + version as SignTypedDataVersion, ); + + return this.#processSignatureRequest({ + messageParams: normalizedMessageParams, + request, + type: SignatureRequestType.TypedSign, + approvalType: ApprovalType.EthSignTypedData, + version: version as SignTypedDataVersion, + signingOptions, + traceContext: options.traceContext, + }); } /** - * Called to update the message status as signed. + * Provide a signature for a pending signature request that used `deferSetAsSigned`. + * Changes the status of the signature request to `signed`. * - * @param messageId - The id of the Message to update. - * @param signature - The data to update the message with. + * @param signatureRequestId - The ID of the signature request. + * @param signature - The signature to provide. */ - // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any - setDeferredSignSuccess(messageId: string, signature: any) { - this.#tryForEachMessageManager( - this.#trySetDeferredSignSuccess, - messageId, - signature, - ); + setDeferredSignSuccess(signatureRequestId: string, signature: any) { + this.#updateMetadata(signatureRequestId, (draftMetadata) => { + draftMetadata.rawSig = signature; + draftMetadata.status = SignatureRequestStatus.Signed; + }); } /** - * Called when the message metadata needs to be updated. - * - * @param messageId - The id of the message to update. - * @param metadata - The data to update the metadata property in the message. + * Set custom metadata on a signature request. + * @param signatureRequestId - The ID of the signature request. + * @param metadata - The custom metadata to set. */ - setMessageMetadata(messageId: string, metadata: Json) { - this.#tryForEachMessageManager( - this.#trySetMessageMetadata, - messageId, - metadata, - ); + setMessageMetadata(signatureRequestId: string, metadata: Json) { + this.#updateMetadata(signatureRequestId, (draftMetadata) => { + draftMetadata.metadata = metadata; + }); } /** - * Called to cancel a signing message. + * Reject a pending signature request that used `deferSetAsSigned`. + * Changes the status of the signature request to `rejected`. * - * @param messageId - The id of the Message to update. + * @param signatureRequestId - The ID of the signature request. */ - setDeferredSignError(messageId: string) { - this.#tryForEachMessageManager(this.#trySetDeferredSignError, messageId); + setDeferredSignError(signatureRequestId: string) { + this.#updateMetadata(signatureRequestId, (draftMetadata) => { + draftMetadata.status = SignatureRequestStatus.Rejected; + }); } - setTypedMessageInProgress(messageId: string) { - this.#typedMessageManager.setMessageStatusInProgress(messageId); + /** + * Set the status of a signature request to 'inProgress'. + * + * @param signatureRequestId - The ID of the signature request. + */ + setTypedMessageInProgress(signatureRequestId: string) { + this.#updateMetadata(signatureRequestId, (draftMetadata) => { + draftMetadata.status = SignatureRequestStatus.InProgress; + }); } - setPersonalMessageInProgress(messageId: string) { - this.#personalMessageManager.setMessageStatusInProgress(messageId); + /** + * Set the status of a signature request to 'inProgress'. + * + * @param signatureRequestId - The ID of the signature request. + */ + setPersonalMessageInProgress(signatureRequestId: string) { + this.setTypedMessageInProgress(signatureRequestId); + } + + #parseTypedData( + messageParams: MessageParamsTyped, + version?: SignTypedDataVersion, + ): MessageParamsTyped { + if ( + ![SignTypedDataVersion.V3, SignTypedDataVersion.V4].includes( + version as SignTypedDataVersion, + ) || + typeof messageParams.data !== 'string' + ) { + return messageParams; + } + + return { + ...messageParams, + data: JSON.parse(messageParams.data), + }; } - async #newUnsignedAbstractMessage< - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - M extends AbstractMessage, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - P extends AbstractMessageParams, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - PM extends AbstractMessageParamsMetamask, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - SO, - >( - messageManager: AbstractMessageManager, - approvalType: ApprovalType, - signTypeForLogger: SigningMethod, - messageName: string, - signMessage: (messageParams: PM, signingOpts?: SO) => void, - messageParams: PM, - req: OriginalRequest, - version?: string, - signingOpts?: SO, - traceContext?: TraceContext, - ) { - let resultCallbacks: AcceptResultCallbacks | undefined; - try { - const messageId = await messageManager.addUnapprovedMessage( - messageParams, - req, - version, - ); + async #processSignatureRequest({ + messageParams, + request, + type, + approvalType, + version, + signingOptions, + traceContext, + }: { + messageParams: MessageParams; + request: OriginalRequest; + type: SignatureRequestType; + approvalType: ApprovalType; + version?: SignTypedDataVersion; + signingOptions?: TypedSigningOptions; + traceContext?: TraceContext; + }): Promise { + log('Processing signature request', { + messageParams, + request, + type, + version, + }); - const messageParamsWithId = { - ...messageParams, - metamaskId: messageId, - ...(version && { version }), - }; + this.#addLog(type, version, SigningStage.Proposed, messageParams); - const signaturePromise = messageManager.waitForFinishStatus( - messageParamsWithId, - messageName, - ); + const metadata = this.#addMetadata({ + messageParams, + request, + signingOptions, + type, + version, + }); - try { - // Signature request is proposed to the user - this.#addLog( - signTypeForLogger, - SigningStage.Proposed, - messageParamsWithId, - ); + let resultCallbacks: AcceptResultCallbacks | undefined; + let approveOrSignError: unknown; - const acceptResult = await this.#trace( - { name: 'Await Approval', parentContext: traceContext }, - (context) => - this.#requestApproval(messageParamsWithId, approvalType, { - traceContext: context, - actionId: req?.id?.toString(), - }), - ); + const finalMetadataPromise = this.#waitForFinished(metadata.id); - resultCallbacks = acceptResult.resultCallbacks; - } catch (error) { - // User rejected the signature request - this.#addLog( - signTypeForLogger, - SigningStage.Rejected, - messageParamsWithId, - ); + try { + resultCallbacks = await this.#processApproval({ + approvalType, + metadata, + request, + traceContext, + }); - this.#cancelAbstractMessage(messageManager, messageId); - throw error; - } + await this.#approveAndSignRequest(metadata, traceContext); + } catch (error) { + log('Signature request failed', error); + approveOrSignError = error; + } - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/await-thenable + const finalMetadata = await finalMetadataPromise; - await this.#trace({ name: 'Sign', parentContext: traceContext }, () => - signMessage(messageParamsWithId, signingOpts), - ); + const { + error, + id, + messageParams: finalMessageParams, + rawSig: signature, + } = finalMetadata; - const signatureResult = await signaturePromise; + switch (finalMetadata.status) { + case SignatureRequestStatus.Signed: + log('Signature request finished', { id, signature }); + this.#addLog(type, version, SigningStage.Signed, finalMessageParams); + resultCallbacks?.success(signature); + return finalMetadata.rawSig as string; - // Signature operation is completed - this.#addLog(signTypeForLogger, SigningStage.Signed, messageParamsWithId); + case SignatureRequestStatus.Rejected: + /* istanbul ignore next */ + const rejectedError = (approveOrSignError ?? + new Error( + `MetaMask ${type} Signature: User denied message signature.`, + )) as Error; - /* istanbul ignore next */ - resultCallbacks?.success(signatureResult); + resultCallbacks?.error(rejectedError); + throw rejectedError; - return signatureResult; - } catch (error) { - resultCallbacks?.error(error as Error); - throw error; - } - } + case SignatureRequestStatus.Errored: + /* istanbul ignore next */ + const erroredError = (approveOrSignError ?? + new Error(`MetaMask ${type} Signature: ${error as string}`)) as Error; - /** - * Signifies a user's approval to sign a personal_sign message in queue. - * Triggers signing, and the callback function from newUnsignedPersonalMessage. - * - * @param msgParams - The params of the message to sign & return to the Dapp. - * @returns Signature result from signing. - */ - async #signPersonalMessage(msgParams: PersonalMessageParamsMetamask) { - return await this.#signAbstractMessage( - this.#personalMessageManager, - ApprovalType.PersonalSign, - msgParams, - async (cleanMsgParams) => - await this.messagingSystem.call( - 'KeyringController:signPersonalMessage', - cleanMsgParams, - ), - ); - } + resultCallbacks?.error(erroredError); + throw erroredError; - /** - * The method for a user approving a call to eth_signTypedData, per EIP 712. - * Triggers the callback in newUnsignedTypedMessage. - * - * @param msgParams - The params passed to eth_signTypedData. - * @param opts - The options for the method. - * @param opts.parseJsonData - Whether to parse JSON data before calling the KeyringController. - * @returns Signature result from signing. - */ - async #signTypedMessage( - msgParams: TypedMessageParamsMetamask, - /* istanbul ignore next */ - opts = { parseJsonData: true }, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ): Promise { - const { version } = msgParams; - return await this.#signAbstractMessage( - this.#typedMessageManager, - ApprovalType.EthSignTypedData, - msgParams, - async (cleanMsgParams) => { - const finalMessageParams = opts.parseJsonData - ? this.#removeJsonData(cleanMsgParams, version as string) - : cleanMsgParams; - - return await this.messagingSystem.call( - 'KeyringController:signTypedMessage', - finalMessageParams, - version as SignTypedDataVersion, + /* istanbul ignore next */ + default: + throw new Error( + `MetaMask ${type} Signature: Unknown problem: ${JSON.stringify( + finalMessageParams, + )}`, ); - }, - ); - } - - #tryForEachMessageManager( - callbackFn: ( - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - messageManager: AbstractMessageManager, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ...args: any[] - ) => boolean, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ...args: any - ) { - const messageManagers = [ - this.#personalMessageManager, - this.#typedMessageManager, - ]; - - for (const manager of messageManagers) { - if (callbackFn(manager, ...args)) { - return true; - } } - throw new Error('Message not found'); } - #trySetDeferredSignSuccess( - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - messageManager: AbstractMessageManager, - messageId: string, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - signature: any, - ) { - try { - messageManager.setMessageStatusSigned(messageId, signature); - return true; - } catch (error) { - return false; - } - } + #addMetadata({ + messageParams, + request, + signingOptions, + type, + version, + }: { + messageParams: MessageParams; + request: OriginalRequest; + signingOptions?: TypedSigningOptions; + type: SignatureRequestType; + version?: SignTypedDataVersion; + }): SignatureRequest { + const finalMessageParams = { + ...messageParams, + origin: request.origin, + requestId: request.id, + }; - #trySetMessageMetadata( - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - messageManager: AbstractMessageManager, - messageId: string, - metadata: Json, - ) { - try { - messageManager.setMetadata(messageId, metadata); - return true; - } catch (error) { - return false; - } - } + const { securityAlertResponse } = request; - #trySetDeferredSignError( - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - messageManager: AbstractMessageManager, - messageId: string, - ) { - try { - messageManager.rejectMessage(messageId); - return true; - } catch (error) { - return false; - } - } + const metadata = { + id: random(), + messageParams: finalMessageParams, + securityAlertResponse, + signingOptions, + status: SignatureRequestStatus.Unapproved, + time: Date.now(), + type, + version, + } as SignatureRequest; - #rejectUnapproved< - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - M extends AbstractMessage, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - P extends AbstractMessageParams, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - PM extends AbstractMessageParamsMetamask, - >(messageManager: AbstractMessageManager, reason?: string) { - Object.keys(messageManager.getUnapprovedMessages()).forEach((messageId) => { - this.#cancelAbstractMessage(messageManager, messageId, reason); + this.#updateState((state) => { + state.signatureRequests[metadata.id] = metadata; }); - } - #clearUnapproved< - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - M extends AbstractMessage, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - P extends AbstractMessageParams, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - PM extends AbstractMessageParamsMetamask, - >(messageManager: AbstractMessageManager) { - messageManager.update({ - unapprovedMessages: {}, - unapprovedMessagesCount: 0, + this.hub.emit('unapprovedMessage', { + messageParams, + metamaskId: metadata.id, }); - } - async #signAbstractMessage< - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - M extends AbstractMessage, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - P extends AbstractMessageParams, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - PM extends AbstractMessageParamsMetamask, - >( - messageManager: AbstractMessageManager, - methodName: string, - msgParams: PM, - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getSignature: (cleanMessageParams: P) => Promise, - ) { - console.info(`MetaMaskController - ${methodName}`); + return metadata; + } - const messageId = msgParams.metamaskId as string; + async #processApproval({ + approvalType, + metadata, + request, + traceContext, + }: { + approvalType: ApprovalType; + metadata: SignatureRequest; + request?: OriginalRequest; + traceContext?: TraceContext; + }): Promise { + const { id, messageParams, type, version } = metadata; try { - const cleanMessageParams = await messageManager.approveMessage(msgParams); - - try { - const signature = await getSignature(cleanMessageParams); + const acceptResult = await this.#trace( + { name: 'Await Approval', parentContext: traceContext }, + (context) => + this.#requestApproval(metadata, approvalType, { + traceContext: context, + actionId: request?.id?.toString(), + }), + ); - this.hub.emit(`${methodName}:signed`, { signature, messageId }); + return acceptResult.resultCallbacks; + } catch (error) { + log('User rejected request', { id, error }); - if (!cleanMessageParams.deferSetAsSigned) { - messageManager.setMessageStatusSigned(messageId, signature); - } + this.#addLog(type, version, SigningStage.Rejected, messageParams); + this.#rejectSignatureRequest(id); - return signature; - } catch (error) { - this.hub.emit(`${messageId}:signError`, { error }); - throw error; - } - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } catch (error: any) { - console.info(`MetaMaskController - ${methodName} failed.`, error); - this.#errorMessage(messageManager, messageId, error.message); throw error; } } - #errorMessage< - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - M extends AbstractMessage, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - P extends AbstractMessageParams, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - PM extends AbstractMessageParamsMetamask, - >( - messageManager: AbstractMessageManager, - messageId: string, - error: string, - ) { - if (messageManager instanceof TypedMessageManager) { - messageManager.setMessageStatusErrored(messageId, error); - } else { - this.#cancelAbstractMessage(messageManager, messageId); - } - } - - #cancelAbstractMessage< - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - M extends AbstractMessage, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - P extends AbstractMessageParams, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - PM extends AbstractMessageParamsMetamask, - >( - messageManager: AbstractMessageManager, - messageId: string, - reason?: string, + async #approveAndSignRequest( + metadata: SignatureRequest, + traceContext?: TraceContext, ) { - if (reason) { - const message = this.#getMessage(messageId); - this.hub.emit('cancelWithReason', { message, reason }); - } - messageManager.rejectMessage(messageId); - } + const { id } = metadata; - #handleMessageManagerEvents< - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - M extends AbstractMessage, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - P extends AbstractMessageParams, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - PM extends AbstractMessageParamsMetamask, - >(messageManager: AbstractMessageManager, eventName: string) { - messageManager.hub.on('updateBadge', () => { - this.hub.emit('updateBadge'); + this.#updateMetadata(id, (draftMetadata) => { + draftMetadata.status = SignatureRequestStatus.Approved; }); - messageManager.hub.on( - 'unapprovedMessage', - (msgParams: AbstractMessageParamsMetamask) => { - this.hub.emit(eventName, msgParams); - }, + await this.#trace({ name: 'Sign', parentContext: traceContext }, () => + this.#signRequest(metadata), ); } - #subscribeToMessageState< - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - M extends AbstractMessage, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - P extends AbstractMessageParams, - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/naming-convention - PM extends AbstractMessageParamsMetamask, - >( - messageManager: AbstractMessageManager, - updateState: ( - state: SignatureControllerState, - newMessages: Record, - messageCount: number, - ) => void, - ) { - messageManager.subscribe((state: MessageManagerState) => { - const newMessages = this.#migrateMessages( - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - state.unapprovedMessages as any, - ); + async #signRequest(metadata: SignatureRequest) { + const { id, messageParams, signingOptions, type } = metadata; - this.update(() => { - const newState = { ...this.state }; - updateState(newState, newMessages, state.unapprovedMessagesCount); - return newState; - }); - }); - } + try { + let signature: string; + + switch (type) { + case SignatureRequestType.PersonalSign: + signature = await this.messagingSystem.call( + 'KeyringController:signPersonalMessage', + // Keyring controller temporarily using message manager types. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + messageParams as any, + ); + break; + + case SignatureRequestType.TypedSign: + const finalRequest = signingOptions?.parseJsonData + ? this.#parseTypedData(messageParams, metadata.version) + : messageParams; + + signature = await this.messagingSystem.call( + 'KeyringController:signTypedMessage', + finalRequest, + metadata.version as SignTypedDataVersion, + ); + break; + + /* istanbul ignore next */ + default: + throw new Error(`Unknown signature request type: ${type as string}`); + } - #migrateMessages( - coreMessages: Record, - ): Record { - const stateMessages: Record = {}; + this.hub.emit(`${type}:signed`, { signature, messageId: id }); - for (const messageId of Object.keys(coreMessages)) { - const coreMessage = coreMessages[messageId]; - const stateMessage = this.#migrateMessage(coreMessage); + if (messageParams.deferSetAsSigned) { + return; + } - stateMessages[messageId] = stateMessage; - } + this.#updateMetadata(id, (draftMetadata) => { + draftMetadata.rawSig = signature; + draftMetadata.status = SignatureRequestStatus.Signed; + }); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } catch (error: any) { + if (type === SignatureRequestType.TypedSign) { + this.#errorSignatureRequest(id, error.message); + } else { + this.#rejectSignatureRequest(id); + } - return stateMessages; + this.hub.emit(`${id}:signError`, { error }); + + throw error; + } } - #migrateMessage(coreMessage: CoreMessage): StateMessage { - const { messageParams, ...coreMessageData } = coreMessage; + #errorSignatureRequest(id: string, error: string) { + this.#updateMetadata(id, (draftMetadata) => { + draftMetadata.status = SignatureRequestStatus.Errored; + draftMetadata.error = error; + }); + } - // Core message managers use messageParams but frontend uses msgParams with lots of references - const stateMessage = { - ...coreMessageData, - msgParams: messageParams, - }; + #rejectSignatureRequest(signatureRequestId: string, reason?: string) { + if (reason) { + const metadata = this.state.signatureRequests[signatureRequestId]; + this.hub.emit('cancelWithReason', { metadata, reason }); + } - return stateMessage as StateMessage; + this.#updateMetadata(signatureRequestId, (draftMetadata) => { + draftMetadata.status = SignatureRequestStatus.Rejected; + }); } - #getMessage(messageId: string): StateMessage { - return { - ...this.state.unapprovedPersonalMsgs, - ...this.state.unapprovedTypedMessages, - }[messageId]; + async #waitForFinished(id: string): Promise { + return new Promise((resolve) => { + this.hub.once(`${id}:finished`, (metadata: SignatureRequest) => { + resolve(metadata); + }); + }); } async #requestApproval( - msgParams: AbstractMessageParamsMetamask, + metadata: SignatureRequest, type: ApprovalType, { traceContext, actionId, }: { traceContext?: TraceContext; actionId?: string }, ): Promise { - const id = msgParams.metamaskId as string; - const origin = msgParams.origin || ORIGIN_METAMASK; - - // We are explicitly cloning the message params here to prevent the mutation errors on development mode - // Because sending it through the messaging system will make the object read only - const clonedMsgParams = cloneDeep(msgParams); + const { id, messageParams } = metadata; + const origin = messageParams.origin || ORIGIN_METAMASK; await this.#trace({ name: 'Notification Display', @@ -855,32 +730,105 @@ export class SignatureController extends BaseController< id, origin, type, - requestData: clonedMsgParams as Required, + requestData: { ...messageParams }, expectsResult: true, }, true, )) as Promise; } - #removeJsonData( - messageParams: TypedMessageParams, - version: string, - ): TypedMessageParams { - if (version === 'V1' || typeof messageParams.data !== 'string') { - return messageParams; + #updateMetadata( + id: string, + callback: (metadata: SignatureRequest) => void, + ): SignatureRequest { + let statusChanged = false; + + const { nextState } = this.#updateState((state) => { + const metadata = state.signatureRequests[id]; + + if (!metadata) { + throw new Error(`Signature request with id ${id} not found`); + } + + const originalStatus = metadata.status; + + // eslint-disable-next-line n/callback-return + callback(metadata); + + statusChanged = metadata.status !== originalStatus; + }); + + const updatedMetadata = nextState.signatureRequests[id]; + + if ( + statusChanged && + FINAL_STATUSES.includes(updatedMetadata.status as SignatureRequestStatus) + ) { + this.hub.emit(`${id}:finished`, updatedMetadata); } - return { - ...messageParams, - data: JSON.parse(messageParams.data), - }; + return updatedMetadata; + } + + #updateState(callback: (state: SignatureControllerState) => void) { + return this.update((state) => { + // eslint-disable-next-line n/callback-return, n/no-callback-literal + callback(state as unknown as SignatureControllerState); + + const unapprovedRequests = Object.values(state.signatureRequests).filter( + (request) => request.status === SignatureRequestStatus.Unapproved, + ) as unknown as SignatureRequest[]; + + const personalSignMessages = this.#generateLegacyState( + unapprovedRequests, + SignatureRequestType.PersonalSign, + ); + + const typedSignMessages = this.#generateLegacyState( + unapprovedRequests, + SignatureRequestType.TypedSign, + ); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + state.unapprovedPersonalMsgs = personalSignMessages as any; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + state.unapprovedTypedMessages = typedSignMessages as any; + + state.unapprovedPersonalMsgCount = + Object.values(personalSignMessages).length; + + state.unapprovedTypedMessagesCount = + Object.values(typedSignMessages).length; + }); + } + + #generateLegacyState( + signatureRequests: SignatureRequest[], + type: SignatureRequestType, + ): Record { + return signatureRequests + .filter((request) => request.type === type) + .reduce>( + (acc, request) => ({ + ...acc, + [request.id]: { ...request, msgParams: request.messageParams }, + }), + {}, + ); } #addLog( - signingMethod: SigningMethod, + signatureRequestType: SignatureRequestType, + version: SignTypedDataVersion | undefined, stage: SigningStage, - signingData: AbstractMessageParamsMetamask, + signingData: MessageParams, ): void { + const signingMethod = this.#getSignTypeForLogger( + signatureRequestType, + version, + ); + this.messagingSystem.call('LoggingController:add', { type: LogType.EthSignLog, data: { @@ -891,13 +839,28 @@ export class SignatureController extends BaseController< }); } - #getSignTypeForLogger(version: string): SigningMethod { - let signTypeForLogger = SigningMethod.EthSignTypedData; - if (version === 'V3') { - signTypeForLogger = SigningMethod.EthSignTypedDataV3; - } else if (version === 'V4') { - signTypeForLogger = SigningMethod.EthSignTypedDataV4; + #getSignTypeForLogger( + requestType: SignatureRequestType, + version?: SignTypedDataVersion, + ): SigningMethod { + if (requestType === SignatureRequestType.PersonalSign) { + return SigningMethod.PersonalSign; + } + + if ( + requestType === SignatureRequestType.TypedSign && + version === SignTypedDataVersion.V3 + ) { + return SigningMethod.EthSignTypedDataV3; } - return signTypeForLogger; + + if ( + requestType === SignatureRequestType.TypedSign && + version === SignTypedDataVersion.V4 + ) { + return SigningMethod.EthSignTypedDataV4; + } + + return SigningMethod.EthSignTypedData; } } diff --git a/packages/signature-controller/src/index.ts b/packages/signature-controller/src/index.ts index b428ae772b..739421c4be 100644 --- a/packages/signature-controller/src/index.ts +++ b/packages/signature-controller/src/index.ts @@ -1 +1,2 @@ export * from './SignatureController'; +export * from './types'; diff --git a/packages/signature-controller/src/logger.ts b/packages/signature-controller/src/logger.ts new file mode 100644 index 0000000000..ff2a63dc48 --- /dev/null +++ b/packages/signature-controller/src/logger.ts @@ -0,0 +1,7 @@ +/* istanbul ignore file */ + +import { createProjectLogger, createModuleLogger } from '@metamask/utils'; + +export const projectLogger = createProjectLogger('signature-controller'); + +export { createModuleLogger }; diff --git a/packages/signature-controller/src/types.ts b/packages/signature-controller/src/types.ts new file mode 100644 index 0000000000..1f59e55594 --- /dev/null +++ b/packages/signature-controller/src/types.ts @@ -0,0 +1,135 @@ +import type { SIWEMessage } from '@metamask/controller-utils'; +import type { SignTypedDataVersion } from '@metamask/keyring-controller'; +import type { Json } from '@metamask/utils'; + +/** Original client request that triggered the signature request. */ +export type OriginalRequest = { + /** Unique ID to identify the client request. */ + id?: number; + + /** Source of the client request. */ + origin?: string; + + /** Response following a security scan of the request. */ + securityAlertResponse?: Record; +}; + +/** Options for signing typed data. */ +export type TypedSigningOptions = { + /** Whether to automatically convert JSON string data to an object. */ + parseJsonData: boolean; +}; + +/** The message parameters that were requested to be signed. */ +export type MessageParams = { + /** Whether to delay marking the request as signed until a signature is provided via `setDeferredSignSuccess`. */ + deferSetAsSigned?: boolean; + + /** Ethereum address to sign with. */ + from: string; + + /** + * Source of the request. + * Such as a hostname of a dApp. + */ + origin?: string; + + /** + * ID of the request that triggered this signature request. + */ + requestId?: number; +}; + +export type StateSIWEMessage = Omit & { + parsedMessage: Omit; +}; + +/** Personal message parameters that were requested to be signed. */ +export type MessageParamsPersonal = MessageParams & { + /** Hexadecimal data to sign. */ + data: string; + + /** Sign-In With Ethereum data extracted from the data. */ + siwe?: StateSIWEMessage; +}; + +/** Typed message parameters that were requested to be signed. */ +export type MessageParamsTyped = MessageParams & { + /** Structured data to sign. */ + data: + | Record[] + | string + | { + types: Record; + domain: Record; + primaryType: string; + message: Json; + }; +}; + +type SignatureRequestBase = { + /** Error message that occurred during the signing. */ + error?: string; + + /** Unique ID to identify this request. */ + id: string; + + /** Custom metadata stored with the request. */ + metadata?: Json; + + /** Signature hash resulting from the request. */ + rawSig?: string; + + /** Response following a security scan of the request. */ + securityAlertResponse?: Record; + + /** Options used for signing. */ + signingOptions?: TypedSigningOptions; + + /** Current status of the request. */ + status: string; + + /** Time the request was created. */ + time: number; + + /** Version of the signTypedData request. */ + version?: SignTypedDataVersion; +}; + +/** Legacy messages stored in the state. */ +export type LegacyStateMessage = SignatureRequestBase & { + /** Message parameters that were requested to be signed. */ + msgParams: MessageParamsPersonal | MessageParamsTyped; + + /** Type of the signature request. */ + type: string; +}; + +/** Metadata concerning a request to sign data. */ +export type SignatureRequest = SignatureRequestBase & + ( + | { + messageParams: MessageParamsPersonal; + type: SignatureRequestType.PersonalSign; + } + | { + messageParams: MessageParamsTyped; + type: SignatureRequestType.TypedSign; + } + ); + +/** Status of a signature request. */ +export enum SignatureRequestStatus { + Unapproved = 'unapproved', + Approved = 'approved', + Rejected = 'rejected', + InProgress = 'inProgress', + Signed = 'signed', + Errored = 'errored', +} + +/** Type of supported signature requests. */ +export enum SignatureRequestType { + PersonalSign = 'personal_sign', + TypedSign = 'eth_signTypedData', +} diff --git a/packages/signature-controller/src/utils/normalize.test.ts b/packages/signature-controller/src/utils/normalize.test.ts new file mode 100644 index 0000000000..09b57628d1 --- /dev/null +++ b/packages/signature-controller/src/utils/normalize.test.ts @@ -0,0 +1,43 @@ +import { SignTypedDataVersion } from '@metamask/keyring-controller'; + +import type { MessageParamsPersonal, MessageParamsTyped } from '../types'; +import { + normalizePersonalMessageParams, + normalizeTypedMessageParams, +} from './normalize'; + +describe('Normalize Utils', () => { + describe('normalizePersonalMessageParams', () => { + it('normalizes data', async () => { + const firstNormalized = normalizePersonalMessageParams({ + data: '879a053d4800c6354e76c7985a865d2922c82fb5b3f4577b2fe08b998954f2e0', + } as MessageParamsPersonal); + + const secondNormalized = normalizePersonalMessageParams({ + data: 'somedata', + } as MessageParamsPersonal); + + expect(firstNormalized.data).toBe( + '0x879a053d4800c6354e76c7985a865d2922c82fb5b3f4577b2fe08b998954f2e0', + ); + + expect(secondNormalized.data).toBe('0x736f6d6564617461'); + }); + }); + + describe('normalizeTypedMessageParams', () => { + it.each([SignTypedDataVersion.V3, SignTypedDataVersion.V4])( + 'serializes data to JSON if not a string and version is %s', + async (version) => { + const normalized = normalizeTypedMessageParams( + { + data: { test: 'data' }, + } as unknown as MessageParamsTyped, + version, + ); + + expect(normalized.data).toBe('{"test":"data"}'); + }, + ); + }); +}); diff --git a/packages/signature-controller/src/utils/normalize.ts b/packages/signature-controller/src/utils/normalize.ts new file mode 100644 index 0000000000..b39b3931fe --- /dev/null +++ b/packages/signature-controller/src/utils/normalize.ts @@ -0,0 +1,61 @@ +import { SignTypedDataVersion } from '@metamask/keyring-controller'; +import { add0x, bytesToHex, remove0x } from '@metamask/utils'; + +import type { MessageParamsPersonal, MessageParamsTyped } from '../types'; + +/** + * Normalize personal message params. + * @param messageParams - The message params to normalize. + * @returns The normalized message params. + */ +export function normalizePersonalMessageParams( + messageParams: MessageParamsPersonal, +): MessageParamsPersonal { + return { + ...messageParams, + data: normalizePersonalMessageData(messageParams.data), + }; +} + +/** + * Normalize typed message params. + * @param messageParams - The message params to normalize. + * @param version - The version of the typed signature request. + * @returns The normalized message params. + */ +export function normalizeTypedMessageParams( + messageParams: MessageParamsTyped, + version: SignTypedDataVersion, +): MessageParamsTyped { + const normalizedMessageParams = { ...messageParams }; + + if ( + typeof messageParams.data !== 'string' && + (version === SignTypedDataVersion.V3 || version === SignTypedDataVersion.V4) + ) { + normalizedMessageParams.data = JSON.stringify(messageParams.data); + } + + return normalizedMessageParams; +} + +/** + * Converts raw message data buffer to a hex, or just returns the data if + * it is already formatted as a hex. + * + * @param data - The buffer data to convert to a hex. + * @returns A hex string conversion of the buffer data. + */ +function normalizePersonalMessageData(data: string) { + try { + const stripped = remove0x(data); + + if (stripped.match(/^[0-9A-Fa-f]+$/gu)) { + return add0x(stripped); + } + } catch (e) { + /* istanbul ignore next */ + } + + return bytesToHex(Buffer.from(data, 'utf8')); +} diff --git a/packages/signature-controller/src/utils/validation.test.ts b/packages/signature-controller/src/utils/validation.test.ts new file mode 100644 index 0000000000..c02493f40d --- /dev/null +++ b/packages/signature-controller/src/utils/validation.test.ts @@ -0,0 +1,262 @@ +import { convertHexToDecimal, toHex } from '@metamask/controller-utils'; +import { SignTypedDataVersion } from '@metamask/keyring-controller'; + +import type { + MessageParams, + MessageParamsPersonal, + MessageParamsTyped, +} from '../types'; +import { + validatePersonalSignatureRequest, + validateTypedSignatureRequest, +} from './validation'; + +const CHAIN_ID_MOCK = '0x1'; + +const DATA_TYPED_MOCK = + '{"types":{"EIP712Domain":[{"name":"name","type":"string"},{"name":"version","type":"string"},{"name":"chainId","type":"uint256"},{"name":"verifyingContract","type":"address"}],"Person":[{"name":"name","type":"string"},{"name":"wallet","type":"address"}],"Mail":[{"name":"from","type":"Person"},{"name":"to","type":"Person"},{"name":"contents","type":"string"}]},"primaryType":"Mail","domain":{"name":"Ether Mail","version":"1","chainId":1,"verifyingContract":"0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC"},"message":{"from":{"name":"Cow","wallet":"0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826"},"to":{"name":"Bob","wallet":"0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB"},"contents":"Hello, Bob!"}}'; + +describe('Validation Utils', () => { + describe.each([ + [ + 'validatePersonalSignatureRequest', + (params: MessageParams) => + validatePersonalSignatureRequest(params as MessageParamsPersonal), + ], + [ + 'validateTypedSignatureRequest', + (params: MessageParams) => + validateTypedSignatureRequest( + params as MessageParamsTyped, + SignTypedDataVersion.V1, + CHAIN_ID_MOCK, + ), + ], + ] as const)('%s', (_title, fn) => { + it('throws if no from address', () => { + expect(() => fn({} as MessageParams)).toThrow( + `Invalid "from" address: undefined must be a valid string.`, + ); + }); + + it('throws if invalid from address', () => { + const from = '01'; + + expect(() => + fn({ + from, + }), + ).toThrow(`Invalid "from" address: ${from} must be a valid string.`); + }); + + it('throws if invalid type from address', () => { + const from = 123; + + expect(() => + fn({ + from: from as never, + }), + ).toThrow(`Invalid "from" address: ${from} must be a valid string.`); + }); + }); + + describe('validatePersonalSignatureRequest', () => { + it('throws if no data', () => { + expect(() => + validatePersonalSignatureRequest({ + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + } as MessageParamsPersonal), + ).toThrow(`Invalid message "data": undefined must be a valid string.`); + }); + + it('throws if invalid type data', () => { + expect(() => + validatePersonalSignatureRequest({ + data: 123 as never, + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + }), + ).toThrow('Invalid message "data": 123 must be a valid string.'); + }); + }); + + describe('validateTypedSignatureRequest', () => { + describe('V1', () => { + it('throws if incorrect data', () => { + expect(() => + validateTypedSignatureRequest( + { + data: '0x879a05', + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + }, + SignTypedDataVersion.V1, + CHAIN_ID_MOCK, + ), + ).toThrow('Invalid message "data":'); + }); + + it('throws if no data', () => { + expect(() => + validateTypedSignatureRequest( + { + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + } as MessageParamsTyped, + SignTypedDataVersion.V1, + CHAIN_ID_MOCK, + ), + ).toThrow('Invalid message "data":'); + }); + + it('throws if invalid type data', () => { + expect(() => + validateTypedSignatureRequest( + { + data: [], + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + } as MessageParamsTyped, + SignTypedDataVersion.V1, + CHAIN_ID_MOCK, + ), + ).toThrow('Expected EIP712 typed data.'); + }); + }); + + describe.each([SignTypedDataVersion.V3, SignTypedDataVersion.V4])( + '%s', + (version) => { + it('throws if array data', () => { + expect(() => + validateTypedSignatureRequest( + { + data: [], + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + }, + version, + CHAIN_ID_MOCK, + ), + ).toThrow('Invalid message "data":'); + }); + + it('throws if no array data', () => { + expect(() => + validateTypedSignatureRequest( + { + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + } as MessageParamsTyped, + version, + CHAIN_ID_MOCK, + ), + ).toThrow('Invalid message "data":'); + }); + + it('throws if no JSON valid data', () => { + expect(() => + validateTypedSignatureRequest( + { + data: 'uh oh', + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + } as MessageParamsTyped, + version, + CHAIN_ID_MOCK, + ), + ).toThrow('Data must be passed as a valid JSON string.'); + }); + + it('throws if current chain ID is not present', () => { + expect(() => + validateTypedSignatureRequest( + { + data: DATA_TYPED_MOCK, + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + }, + version, + undefined, + ), + ).toThrow('Current chainId cannot be null or undefined.'); + }); + + it('throws if current chain ID is not convertible to integer', () => { + const unexpectedChainId = 'unexpected chain id'; + + expect(() => + validateTypedSignatureRequest( + { + data: DATA_TYPED_MOCK.replace(`"chainId":1`, `"chainId":"0x1"`), + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + }, + version, + unexpectedChainId as never, + ), + ).toThrow( + `Cannot sign messages for chainId "${String( + convertHexToDecimal(CHAIN_ID_MOCK), + )}", because MetaMask is switching networks.`, + ); + }); + + it('throws if current chain ID is not matched with provided in message data', () => { + const chainId = toHex(2); + + expect(() => + validateTypedSignatureRequest( + { + data: DATA_TYPED_MOCK, + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + }, + version, + chainId, + ), + ).toThrow( + // TODO: Either fix this lint violation or explain why it's necessary to ignore. + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + `Provided chainId "${convertHexToDecimal( + CHAIN_ID_MOCK, + // TODO: Either fix this lint violation or explain why it's necessary to ignore. + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + )}" must match the active chainId "${convertHexToDecimal( + chainId, + )}"`, + ); + }); + + it('throws if data not in typed message schema', () => { + expect(() => + validateTypedSignatureRequest( + { + data: '{"greetings":"I am Alice"}', + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + }, + version, + CHAIN_ID_MOCK, + ), + ).toThrow('Data must conform to EIP-712 schema.'); + }); + + it('does not throw if data is correct', () => { + expect(() => + validateTypedSignatureRequest( + { + data: DATA_TYPED_MOCK.replace(`"chainId":1`, `"chainId":"1"`), + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + }, + version, + CHAIN_ID_MOCK, + ), + ).not.toThrow(); + }); + + it('does not throw if data is correct (object format)', () => { + expect(() => + validateTypedSignatureRequest( + { + data: JSON.parse(DATA_TYPED_MOCK), + from: '0x3244e191f1b4903970224322180f1fbbc415696b', + }, + version, + CHAIN_ID_MOCK, + ), + ).not.toThrow(); + }); + }, + ); + }); +}); diff --git a/packages/signature-controller/src/utils/validation.ts b/packages/signature-controller/src/utils/validation.ts new file mode 100644 index 0000000000..cf4ed87d90 --- /dev/null +++ b/packages/signature-controller/src/utils/validation.ts @@ -0,0 +1,150 @@ +import { isValidHexAddress } from '@metamask/controller-utils'; +import { + TYPED_MESSAGE_SCHEMA, + typedSignatureHash, +} from '@metamask/eth-sig-util'; +import { SignTypedDataVersion } from '@metamask/keyring-controller'; +import { type Hex } from '@metamask/utils'; +import { validate } from 'jsonschema'; + +import type { MessageParamsPersonal, MessageParamsTyped } from '../types'; + +/** + * Validate a personal signature request. + * @param messageData - The message data to validate. + */ +export function validatePersonalSignatureRequest( + messageData: MessageParamsPersonal, +) { + const { from, data } = messageData; + + validateAddress(from, 'from'); + + if (!data || typeof data !== 'string') { + throw new Error(`Invalid message "data": ${data} must be a valid string.`); + } +} + +/** + * Validate a typed signature request. + * @param messageData - The message data to validate. + * @param version - The version of the typed signature request. + * @param currentChainId - The current chain ID. + */ +export function validateTypedSignatureRequest( + messageData: MessageParamsTyped, + version: SignTypedDataVersion, + currentChainId: Hex | undefined, +) { + validateAddress(messageData.from, 'from'); + + if (version === SignTypedDataVersion.V1) { + validateTypedSignatureRequestV1(messageData); + } else { + validateTypedSignatureRequestV3V4(messageData, currentChainId); + } +} + +/** + * Validate a V1 typed signature request. + * @param messageData - The message data to validate. + */ +function validateTypedSignatureRequestV1(messageData: MessageParamsTyped) { + if (!messageData.data || !Array.isArray(messageData.data)) { + throw new Error( + // TODO: Either fix this lint violation or explain why it's necessary to ignore. + // eslint-disable-next-line @typescript-eslint/restrict-template-expressions + `Invalid message "data": ${messageData.data} must be a valid array.`, + ); + } + + try { + // typedSignatureHash will throw if the data is invalid. + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + typedSignatureHash(messageData.data as any); + } catch (e) { + throw new Error(`Expected EIP712 typed data.`); + } +} + +/** + * Validate a V3 or V4 typed signature request. + * + * @param messageData - The message data to validate. + * @param currentChainId - The current chain ID. + */ +function validateTypedSignatureRequestV3V4( + messageData: MessageParamsTyped, + currentChainId: Hex | undefined, +) { + if ( + !messageData.data || + Array.isArray(messageData.data) || + (typeof messageData.data !== 'object' && + typeof messageData.data !== 'string') + ) { + throw new Error( + `Invalid message "data": Must be a valid string or object.`, + ); + } + + let data; + if (typeof messageData.data === 'object') { + data = messageData.data; + } else { + try { + data = JSON.parse(messageData.data); + } catch (e) { + throw new Error('Data must be passed as a valid JSON string.'); + } + } + + const validation = validate(data, TYPED_MESSAGE_SCHEMA); + if (validation.errors.length > 0) { + throw new Error( + 'Data must conform to EIP-712 schema. See https://git.io/fNtcx.', + ); + } + + if (!currentChainId) { + throw new Error('Current chainId cannot be null or undefined.'); + } + + let { chainId } = data.domain; + if (chainId) { + if (typeof chainId === 'string') { + chainId = parseInt(chainId, chainId.startsWith('0x') ? 16 : 10); + } + + const activeChainId = parseInt(currentChainId, 16); + if (Number.isNaN(activeChainId)) { + throw new Error( + `Cannot sign messages for chainId "${ + chainId as string + }", because MetaMask is switching networks.`, + ); + } + + if (chainId !== activeChainId) { + throw new Error( + `Provided chainId "${ + chainId as string + }" must match the active chainId "${activeChainId}"`, + ); + } + } +} + +/** + * Validate an Ethereum address. + * @param address - The address to validate. + * @param propertyName - The name of the property source to use in the error message. + */ +function validateAddress(address: string, propertyName: string) { + if (!address || typeof address !== 'string' || !isValidHexAddress(address)) { + throw new Error( + `Invalid "${propertyName}" address: ${address} must be a valid string.`, + ); + } +} diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index 519f16ed3c..9abb968d87 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -1102,7 +1102,7 @@ describe('TransactionController Integration', () => { const { networkController, transactionController } = await setupController(); - const networkConfiguration = await networkController.addNetwork( + const networkConfiguration = networkController.addNetwork( buildAddNetworkFields(), ); @@ -1146,7 +1146,7 @@ describe('TransactionController Integration', () => { isMultichainEnabled: false, }); - const networkConfiguration = await networkController.addNetwork( + const networkConfiguration = networkController.addNetwork( buildAddNetworkFields(), ); @@ -1186,7 +1186,7 @@ describe('TransactionController Integration', () => { getNetworkClientRegistry: getNetworkClientRegistrySpy, }); - await networkController.addNetwork(buildAddNetworkFields()); + networkController.addNetwork(buildAddNetworkFields()); expect(getNetworkClientRegistrySpy).not.toHaveBeenCalled(); transactionController.destroy(); @@ -1202,7 +1202,7 @@ describe('TransactionController Integration', () => { 'getNetworkClientRegistry', ); - await networkController.addNetwork(buildAddNetworkFields()); + networkController.addNetwork(buildAddNetworkFields()); expect(getNetworkClientRegistrySpy).toHaveBeenCalled(); transactionController.destroy(); diff --git a/yarn.lock b/yarn.lock index 037b06dcf9..eee2654db0 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3466,18 +3466,20 @@ __metadata: "@metamask/auto-changelog": "npm:^3.4.4" "@metamask/base-controller": "npm:^7.0.1" "@metamask/controller-utils": "npm:^11.3.0" + "@metamask/eth-sig-util": "npm:^7.0.1" "@metamask/keyring-controller": "npm:^17.2.2" "@metamask/logging-controller": "npm:^6.0.1" - "@metamask/message-manager": "npm:^10.1.1" "@metamask/utils": "npm:^9.1.0" "@types/jest": "npm:^27.4.1" deepmerge: "npm:^4.2.2" jest: "npm:^27.5.1" + jsonschema: "npm:^1.2.4" lodash: "npm:^4.17.21" ts-jest: "npm:^27.1.4" typedoc: "npm:^0.24.8" typedoc-plugin-missing-exports: "npm:^2.0.0" typescript: "npm:~5.2.2" + uuid: "npm:^8.3.2" peerDependencies: "@metamask/approval-controller": ^7.0.0 "@metamask/keyring-controller": ^17.0.0