diff --git a/packages/signature-controller/package.json b/packages/signature-controller/package.json index 4b1ea6e605..a1836f2efa 100644 --- a/packages/signature-controller/package.json +++ b/packages/signature-controller/package.json @@ -60,6 +60,7 @@ "@metamask/auto-changelog": "^3.4.4", "@metamask/keyring-controller": "^17.2.2", "@metamask/logging-controller": "^6.0.1", + "@metamask/network-controller": "^21.0.1", "@types/jest": "^27.4.1", "deepmerge": "^4.2.2", "jest": "^27.5.1", @@ -71,7 +72,8 @@ "peerDependencies": { "@metamask/approval-controller": "^7.0.0", "@metamask/keyring-controller": "^17.0.0", - "@metamask/logging-controller": "^6.0.0" + "@metamask/logging-controller": "^6.0.0", + "@metamask/network-controller": "^21.0.1" }, "engines": { "node": "^18.18 || >=20" diff --git a/packages/signature-controller/src/SignatureController.test.ts b/packages/signature-controller/src/SignatureController.test.ts index 2fb84b55d8..fa94b2cfcc 100644 --- a/packages/signature-controller/src/SignatureController.test.ts +++ b/packages/signature-controller/src/SignatureController.test.ts @@ -11,7 +11,11 @@ import type { SignatureControllerState, } from './SignatureController'; import { SignatureController } from './SignatureController'; -import type { MessageParamsPersonal, SignatureRequest } from './types'; +import type { + MessageParamsPersonal, + OriginalRequest, + SignatureRequest, +} from './types'; import { SignatureRequestStatus, SignatureRequestType } from './types'; import { normalizePersonalMessageParams, @@ -29,6 +33,7 @@ jest.mock('@metamask/controller-utils', () => ({ const ID_MOCK = '123-456'; const CHAIN_ID_MOCK = '0x1'; +const NETWORK_CLIENT_ID_MOCK = 'testNetworkClientId'; const FROM_MOCK = '0x456DEF'; const DATA_MOCK = '0xABC123'; const SIGNATURE_HASH_MOCK = '0x123ABC'; @@ -40,9 +45,15 @@ const PARAMS_MOCK = { data: DATA_MOCK, }; +const REQUEST_MOCK = { + networkClientId: NETWORK_CLIENT_ID_MOCK, +}; + const SIGNATURE_REQUEST_MOCK: SignatureRequest = { + chainId: CHAIN_ID_MOCK, id: ID_MOCK, messageParams: PARAMS_MOCK, + networkClientId: NETWORK_CLIENT_ID_MOCK, status: SignatureRequestStatus.Signed, time: Date.now(), type: SignatureRequestType.PersonalSign, @@ -57,6 +68,7 @@ function createMessengerMock() { const approvalControllerAddRequestMock = jest.fn(); const keyringControllerSignPersonalMessageMock = jest.fn(); const keyringControllerSignTypedMessageMock = jest.fn(); + const networkControllerGetNetworkClientByIdMock = jest.fn(); // eslint-disable-next-line @typescript-eslint/no-explicit-any const callMock = (method: string, ...args: any[]) => { @@ -69,6 +81,8 @@ function createMessengerMock() { return keyringControllerSignPersonalMessageMock(...args); case 'KeyringController:signTypedMessage': return keyringControllerSignTypedMessageMock(...args); + case 'NetworkController:getNetworkClientById': + return networkControllerGetNetworkClientByIdMock(...args); default: throw new Error(`Messenger method not recognised: ${method}`); } @@ -84,6 +98,12 @@ function createMessengerMock() { approvalControllerAddRequestMock.mockResolvedValue({}); loggingControllerAddMock.mockResolvedValue({}); + networkControllerGetNetworkClientByIdMock.mockReturnValue({ + configuration: { + chainId: CHAIN_ID_MOCK, + }, + }); + return { approvalControllerAddRequestMock, keyringControllerSignPersonalMessageMock, @@ -327,13 +347,13 @@ describe('SignatureController', () => { describe.each([ [ 'newUnsignedPersonalMessage', - (controller: SignatureController, request = {}) => + (controller: SignatureController, request: OriginalRequest) => controller.newUnsignedPersonalMessage({ ...PARAMS_MOCK }, request), SignatureRequestType.PersonalSign, ], [ 'newUnsignedTypedMessage', - (controller: SignatureController, request = {}) => + (controller: SignatureController, request: OriginalRequest) => controller.newUnsignedTypedMessage( PARAMS_MOCK, request, @@ -352,7 +372,7 @@ describe('SignatureController', () => { approvalControllerAddRequestMock.mockRejectedValueOnce(error); await expect( - controller.newUnsignedPersonalMessage({ ...PARAMS_MOCK }, {}), + controller.newUnsignedPersonalMessage({ ...PARAMS_MOCK }, REQUEST_MOCK), ).rejects.toMatchObject({ message: ERROR_MESSAGE_MOCK, code: ERROR_CODE_MOCK, @@ -383,7 +403,7 @@ describe('SignatureController', () => { SIGNATURE_HASH_MOCK, ); - await fn(controller); + await fn(controller, REQUEST_MOCK); expect(resultCallbackSuccessMock).toHaveBeenCalledTimes(1); expect(resultCallbackSuccessMock).toHaveBeenCalledWith( @@ -398,7 +418,7 @@ describe('SignatureController', () => { controller.hub.on(`${ID_MOCK}:finished`, listener); - await fn(controller); + await fn(controller, REQUEST_MOCK); const state = controller.state.signatureRequests[ID_MOCK]; @@ -418,7 +438,7 @@ describe('SignatureController', () => { approvalControllerAddRequestMock.mockRejectedValueOnce(errorMock); - await fn(controller).catch(() => { + await fn(controller, REQUEST_MOCK).catch(() => { // Ignore error }); @@ -431,7 +451,7 @@ describe('SignatureController', () => { it('adds logs to logging controller if approved', async () => { const { controller, loggingControllerAddMock } = createController(); - await fn(controller); + await fn(controller, REQUEST_MOCK); expect(loggingControllerAddMock).toHaveBeenCalledTimes(2); @@ -465,7 +485,7 @@ describe('SignatureController', () => { approvalControllerAddRequestMock.mockRejectedValueOnce(errorMock); - await expect(fn(controller)).rejects.toThrow(errorMock); + await expect(fn(controller, REQUEST_MOCK)).rejects.toThrow(errorMock); expect(loggingControllerAddMock).toHaveBeenCalledTimes(2); @@ -491,7 +511,7 @@ describe('SignatureController', () => { it('populates origin from request', async () => { const { controller } = createController(); - await fn(controller, { origin: 'test' }); + await fn(controller, { ...REQUEST_MOCK, origin: 'test' }); expect( controller.state.signatureRequests[ID_MOCK].messageParams.origin, @@ -501,11 +521,19 @@ describe('SignatureController', () => { it('populates request ID from request', async () => { const { controller } = createController(); - await fn(controller, { id: 'test' }); + await fn(controller, { ...REQUEST_MOCK, id: 123 }); expect( controller.state.signatureRequests[ID_MOCK].messageParams.requestId, - ).toBe('test'); + ).toBe(123); + }); + + it('throws if no network client ID in request', async () => { + const { controller } = createController(); + + await expect(fn(controller, {} as OriginalRequest)).rejects.toThrow( + 'Network client ID not found in request', + ); }); it('emits unapproved message event', async () => { @@ -515,7 +543,7 @@ describe('SignatureController', () => { controller.hub.on('unapprovedMessage', listener); - await fn(controller); + await fn(controller, REQUEST_MOCK); expect(listener).toHaveBeenCalledTimes(1); expect(listener).toHaveBeenCalledWith({ @@ -543,7 +571,7 @@ describe('SignatureController', () => { controller.hub.on(`${type}:signed`, listener); - await fn(controller); + await fn(controller, REQUEST_MOCK); expect(listener).toHaveBeenCalledTimes(1); expect(listener).toHaveBeenCalledWith({ @@ -568,7 +596,7 @@ describe('SignatureController', () => { keyringControllerSignPersonalMessageMock.mockRejectedValueOnce(errorMock); keyringControllerSignTypedMessageMock.mockRejectedValueOnce(errorMock); - await fn(controller).catch(() => { + await fn(controller, REQUEST_MOCK).catch(() => { // Ignore error }); @@ -599,7 +627,9 @@ describe('SignatureController', () => { keyringControllerSignPersonalMessageMock.mockRejectedValueOnce(errorMock); keyringControllerSignTypedMessageMock.mockRejectedValueOnce(errorMock); - await expect(fn(controller)).rejects.toThrow(ERROR_MESSAGE_MOCK); + await expect(fn(controller, REQUEST_MOCK)).rejects.toThrow( + ERROR_MESSAGE_MOCK, + ); expect(resultCallbackErrorMock).toHaveBeenCalledTimes(1); expect(resultCallbackErrorMock).toHaveBeenCalledWith(errorMock); @@ -617,7 +647,7 @@ describe('SignatureController', () => { const result = await controller.newUnsignedPersonalMessage( { ...PARAMS_MOCK }, - {}, + REQUEST_MOCK, ); expect(result).toBe(SIGNATURE_HASH_MOCK); @@ -633,7 +663,10 @@ describe('SignatureController', () => { detectSIWEMock.mockReturnValueOnce(siweMock); - await controller.newUnsignedPersonalMessage({ ...PARAMS_MOCK }, {}); + await controller.newUnsignedPersonalMessage( + { ...PARAMS_MOCK }, + REQUEST_MOCK, + ); const messageParams = controller.state.signatureRequests[ID_MOCK] .messageParams as MessageParamsPersonal; @@ -653,7 +686,7 @@ describe('SignatureController', () => { const result = await controller.newUnsignedTypedMessage( PARAMS_MOCK, - {}, + REQUEST_MOCK, SignTypedDataVersion.V1, { parseJsonData: false }, ); @@ -676,7 +709,7 @@ describe('SignatureController', () => { ...PARAMS_MOCK, data: JSON.stringify({ test: 123 }), }, - {}, + REQUEST_MOCK, version as SignTypedDataVersion, { parseJsonData: true }, ); @@ -706,7 +739,7 @@ describe('SignatureController', () => { ...PARAMS_MOCK, data: JSON.stringify({ test: 123 }), }, - {}, + REQUEST_MOCK, SignTypedDataVersion.V1, { parseJsonData: true }, ); @@ -733,7 +766,7 @@ describe('SignatureController', () => { await expect( controller.newUnsignedTypedMessage( PARAMS_MOCK, - {}, + REQUEST_MOCK, SignTypedDataVersion.V3, { parseJsonData: false }, ), @@ -782,7 +815,7 @@ describe('SignatureController', () => { ...PARAMS_MOCK, deferSetAsSigned: true, }, - {}, + REQUEST_MOCK, ) .then((result) => { resolved = true; @@ -860,7 +893,7 @@ describe('SignatureController', () => { ...PARAMS_MOCK, deferSetAsSigned: true, }, - {}, + REQUEST_MOCK, ) .catch((error) => { rejectedError = error; diff --git a/packages/signature-controller/src/SignatureController.ts b/packages/signature-controller/src/SignatureController.ts index 6ec64bf6c1..5e9039afb6 100644 --- a/packages/signature-controller/src/SignatureController.ts +++ b/packages/signature-controller/src/SignatureController.ts @@ -27,6 +27,7 @@ import { SigningStage, type AddLog, } from '@metamask/logging-controller'; +import type { NetworkControllerGetNetworkClientByIdAction } from '@metamask/network-controller'; import type { Hex, Json } from '@metamask/utils'; import EventEmitter from 'events'; import { v1 as random } from 'uuid'; @@ -113,7 +114,8 @@ type AllowedActions = | KeyringControllerSignMessageAction | KeyringControllerSignPersonalMessageAction | KeyringControllerSignTypedMessageAction - | AddLog; + | AddLog + | NetworkControllerGetNetworkClientByIdAction; export type GetSignatureState = ControllerGetStateAction< typeof controllerName, @@ -184,25 +186,17 @@ export class SignatureController extends BaseController< > { hub: EventEmitter; - #getCurrentChainId: () => Hex; - #trace: TraceCallback; /** * 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.state - Initial state to set on this controller. * @param options.trace - Callback to generate trace information. */ - constructor({ - getCurrentChainId, - messenger, - state, - trace, - }: SignatureControllerOptions) { + constructor({ messenger, state, trace }: SignatureControllerOptions) { super({ name: controllerName, metadata: stateMetadata, @@ -214,7 +208,6 @@ export class SignatureController extends BaseController< }); this.hub = new EventEmitter(); - this.#getCurrentChainId = getCurrentChainId; this.#trace = trace ?? (((_request, fn) => fn?.()) as TraceCallback); } @@ -335,10 +328,12 @@ export class SignatureController extends BaseController< signingOptions: TypedSigningOptions, options: { traceContext?: TraceContext } = {}, ): Promise { + const chainId = this.#getChainId(request); + validateTypedSignatureRequest( messageParams, version as SignTypedDataVersion, - this.#getCurrentChainId(), + chainId, ); const normalizedMessageParams = normalizeTypedMessageParams( @@ -347,13 +342,13 @@ export class SignatureController extends BaseController< ); return this.#processSignatureRequest({ + approvalType: ApprovalType.EthSignTypedData, messageParams: normalizedMessageParams, request, - type: SignatureRequestType.TypedSign, - approvalType: ApprovalType.EthSignTypedData, - version: version as SignTypedDataVersion, signingOptions, traceContext: options.traceContext, + type: SignatureRequestType.TypedSign, + version: version as SignTypedDataVersion, }); } @@ -435,6 +430,7 @@ export class SignatureController extends BaseController< } async #processSignatureRequest({ + chainId: optionChainId, messageParams, request, type, @@ -443,6 +439,7 @@ export class SignatureController extends BaseController< signingOptions, traceContext, }: { + chainId?: Hex; messageParams: MessageParams; request: OriginalRequest; type: SignatureRequestType; @@ -458,9 +455,12 @@ export class SignatureController extends BaseController< version, }); + const chainId = optionChainId ?? this.#getChainId(request); + this.#addLog(type, version, SigningStage.Proposed, messageParams); const metadata = this.#addMetadata({ + chainId, messageParams, request, signingOptions, @@ -532,12 +532,14 @@ export class SignatureController extends BaseController< } #addMetadata({ + chainId, messageParams, request, signingOptions, type, version, }: { + chainId: Hex; messageParams: MessageParams; request: OriginalRequest; signingOptions?: TypedSigningOptions; @@ -550,11 +552,13 @@ export class SignatureController extends BaseController< requestId: request.id, }; - const { securityAlertResponse } = request; + const { networkClientId, securityAlertResponse } = request; const metadata = { + chainId, id: random(), messageParams: finalMessageParams, + networkClientId, securityAlertResponse, signingOptions, status: SignatureRequestStatus.Unapproved, @@ -567,6 +571,8 @@ export class SignatureController extends BaseController< state.signatureRequests[metadata.id] = metadata; }); + log('Added signature request', metadata); + this.hub.emit('unapprovedMessage', { messageParams, metamaskId: metadata.id, @@ -863,4 +869,19 @@ export class SignatureController extends BaseController< return SigningMethod.EthSignTypedData; } + + #getChainId(request: OriginalRequest): Hex { + const { networkClientId } = request; + + if (!networkClientId) { + throw new Error('Network client ID not found in request'); + } + + const networkClient = this.messagingSystem.call( + 'NetworkController:getNetworkClientById', + networkClientId, + ); + + return networkClient.configuration.chainId; + } } diff --git a/packages/signature-controller/src/types.ts b/packages/signature-controller/src/types.ts index 1f59e55594..2768a4c506 100644 --- a/packages/signature-controller/src/types.ts +++ b/packages/signature-controller/src/types.ts @@ -1,12 +1,15 @@ import type { SIWEMessage } from '@metamask/controller-utils'; import type { SignTypedDataVersion } from '@metamask/keyring-controller'; -import type { Json } from '@metamask/utils'; +import type { Hex, Json } from '@metamask/utils'; /** Original client request that triggered the signature request. */ export type OriginalRequest = { /** Unique ID to identify the client request. */ id?: number; + /** ID of the network client associated with the request. */ + networkClientId?: string; + /** Source of the client request. */ origin?: string; @@ -68,6 +71,9 @@ export type MessageParamsTyped = MessageParams & { }; type SignatureRequestBase = { + /** ID of the associated chain. */ + chainId: Hex; + /** Error message that occurred during the signing. */ error?: string; @@ -77,6 +83,9 @@ type SignatureRequestBase = { /** Custom metadata stored with the request. */ metadata?: Json; + /** ID of the associated network client. */ + networkClientId: string; + /** Signature hash resulting from the request. */ rawSig?: string; diff --git a/packages/signature-controller/tsconfig.build.json b/packages/signature-controller/tsconfig.build.json index 1c2dc43957..c7831754a6 100644 --- a/packages/signature-controller/tsconfig.build.json +++ b/packages/signature-controller/tsconfig.build.json @@ -23,6 +23,9 @@ }, { "path": "../logging-controller/tsconfig.build.json" + }, + { + "path": "../network-controller/tsconfig.build.json" } ], "include": ["../../types", "./src"] diff --git a/packages/signature-controller/tsconfig.json b/packages/signature-controller/tsconfig.json index 30c7bc4a79..11bf1c1898 100644 --- a/packages/signature-controller/tsconfig.json +++ b/packages/signature-controller/tsconfig.json @@ -21,6 +21,9 @@ }, { "path": "../logging-controller" + }, + { + "path": "../network-controller" } ], "include": ["../../types", "./src"] diff --git a/yarn.lock b/yarn.lock index ed66bdca2d..49cac125da 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3480,6 +3480,7 @@ __metadata: "@metamask/eth-sig-util": "npm:^7.0.1" "@metamask/keyring-controller": "npm:^17.2.2" "@metamask/logging-controller": "npm:^6.0.1" + "@metamask/network-controller": "npm:^21.0.1" "@metamask/utils": "npm:^9.1.0" "@types/jest": "npm:^27.4.1" deepmerge: "npm:^4.2.2" @@ -3495,6 +3496,7 @@ __metadata: "@metamask/approval-controller": ^7.0.0 "@metamask/keyring-controller": ^17.0.0 "@metamask/logging-controller": ^6.0.0 + "@metamask/network-controller": ^21.0.1 languageName: unknown linkType: soft