Skip to content

Commit

Permalink
Add normalization
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewwalsh0 committed Oct 11, 2024
1 parent 2ccc490 commit 67d8a2c
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 16 deletions.
59 changes: 52 additions & 7 deletions packages/signature-controller/src/SignatureController.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
/* eslint-disable jsdoc/require-jsdoc */
/* eslint-disable @typescript-eslint/no-unused-vars */
/* eslint-disable @typescript-eslint/no-explicit-any */

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';

Expand All @@ -12,10 +10,15 @@ import type {
SignatureControllerState,
} from './SignatureController';
import { SignatureController } from './SignatureController';
import type { SignatureRequest } from './types';
import type { MessageParamsPersonal, SignatureRequest } from './types';
import { SignatureRequestStatus, SignatureRequestType } from './types';
import {
normalizePersonalMessageParams,
normalizeTypedMessageParams,
} from './utils/normalize';

jest.mock('./utils/validation');
jest.mock('./utils/normalize');

jest.mock('@metamask/controller-utils', () => ({
...jest.requireActual('@metamask/controller-utils'),
Expand All @@ -40,12 +43,17 @@ const SIGNATURE_REQUEST_MOCK: SignatureRequest = {
type: SignatureRequestType.PersonalSign,
};

/**
* 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();

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const callMock = (method: string, ...args: any[]) => {
switch (method) {
case 'LoggingController:add':
Expand All @@ -66,8 +74,6 @@ function createMessengerMock() {
registerInitialEventPayload: jest.fn(),
publish: jest.fn(),
call: callMock,
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} as unknown as jest.Mocked<SignatureControllerMessenger>;

approvalControllerAddRequestMock.mockResolvedValue({});
Expand All @@ -82,6 +88,11 @@ function createMessengerMock() {
};
}

/**
* 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<SignatureControllerOptions>) {
const messengerMocks = createMessengerMock();

Expand All @@ -95,8 +106,21 @@ function createController(options?: Partial<SignatureControllerOptions>) {
}

describe('SignatureController', () => {
const normalizePersonalMessageParamsMock = jest.mocked(
normalizePersonalMessageParams,
);

const normalizeTypedMessageParamsMock = jest.mocked(
normalizeTypedMessageParams,
);

const detectSIWEMock = jest.mocked(detectSIWE);

beforeEach(() => {
jest.resetAllMocks();

normalizePersonalMessageParamsMock.mockImplementation((params) => params);
normalizeTypedMessageParamsMock.mockImplementation((params) => params);
});

describe('unapprovedPersonalMessagesCount', () => {
Expand Down Expand Up @@ -304,6 +328,7 @@ describe('SignatureController', () => {
createController();

const errorMock = new Error('Custom message');
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(errorMock as any).code = 1234;

approvalControllerAddRequestMock.mockRejectedValueOnce(errorMock);
Expand Down Expand Up @@ -479,6 +504,26 @@ describe('SignatureController', () => {

expect(result).toBe(SIGNATURE_HASH_MOCK);
});

it('adds SIWE data', async () => {
const { controller } = createController();

const siweMock = {
isSIWEMessage: true,
parsedMessage: { domain: 'test' },
} as SIWEMessage;

detectSIWEMock.mockReturnValueOnce(siweMock);

await controller.newUnsignedPersonalMessage({ ...PARAMS_MOCK }, {});

const id = Object.keys(controller.state.signatureRequests)[0];

const messageParams = controller.state.signatureRequests[id]
.messageParams as MessageParamsPersonal;

expect(messageParams.siwe).toStrictEqual(siweMock);
});
});

describe('newUnsignedTypedMessage', () => {
Expand Down
29 changes: 21 additions & 8 deletions packages/signature-controller/src/SignatureController.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import type {
AddApprovalRequest,
AcceptResultCallbacks,
Expand Down Expand Up @@ -42,6 +41,10 @@ import type {
MessageParams,
TypedSigningOptions,
} from './types';
import {
normalizePersonalMessageParams,
normalizeTypedMessageParams,
} from './utils/normalize';
import {
validatePersonalSignatureRequest,
validateTypedSignatureRequest,
Expand Down Expand Up @@ -75,13 +78,13 @@ export type SignatureControllerState = {
* Map of personal messages with the unapproved status, keyed by ID.
* @deprecated - Use `signatureRequests` instead.
*/
unapprovedPersonalMsgs: Record<string, any>;
unapprovedPersonalMsgs: Record<string, SignatureRequest>;

/**
* Map of typed messages with the unapproved status, keyed by ID.
* @deprecated - Use `signatureRequests` instead.
*/
unapprovedTypedMessages: Record<string, any>;
unapprovedTypedMessages: Record<string, SignatureRequest>;

/**
* Number of unapproved personal messages.
Expand Down Expand Up @@ -289,10 +292,13 @@ export class SignatureController extends BaseController<
): Promise<string> {
validatePersonalSignatureRequest(messageParams);

messageParams.siwe = detectSIWE(messageParams);
const normalizedMessageParams =
normalizePersonalMessageParams(messageParams);

normalizedMessageParams.siwe = detectSIWE(messageParams);

return this.#processSignatureRequest({
messageParams,
messageParams: normalizedMessageParams,
request,
type: SignatureRequestType.PersonalSign,
approvalType: ApprovalType.PersonalSign,
Expand Down Expand Up @@ -324,8 +330,13 @@ export class SignatureController extends BaseController<
this.#getCurrentChainId(),
);

return this.#processSignatureRequest({
const normalizedMessageParams = normalizeTypedMessageParams(
messageParams,
version as SignTypedDataVersion,
);

return this.#processSignatureRequest({
messageParams: normalizedMessageParams,
request,
type: SignatureRequestType.TypedSign,
approvalType: ApprovalType.EthSignTypedData,
Expand All @@ -342,6 +353,7 @@ export class SignatureController extends BaseController<
* @param signatureRequestId - The ID of the signature request.
* @param signature - The signature to provide.
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
setDeferredSignSuccess(signatureRequestId: string, signature: any) {
const updatedSignatureRequest = this.#updateMetadata(
signatureRequestId,
Expand Down Expand Up @@ -578,6 +590,7 @@ export class SignatureController extends BaseController<
});

this.hub.emit(`${id}:finished`, finalMetadata);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
} catch (error: any) {
if (type === SignatureRequestType.TypedSign) {
this.#updateMetadata(id, (draftMetadata) => {
Expand Down Expand Up @@ -658,7 +671,7 @@ export class SignatureController extends BaseController<
id,
origin,
type,
requestData: { ...messageParams } as any,
requestData: { ...messageParams },
expectsResult: true,
},
true,
Expand All @@ -685,7 +698,7 @@ export class SignatureController extends BaseController<
#updateState(callback: (state: SignatureControllerState) => void) {
return this.update((state) => {
// eslint-disable-next-line n/callback-return, n/no-callback-literal
callback(state as any);
callback(state as unknown as SignatureControllerState);

const unapprovedRequests = Object.values(
state.signatureRequests as unknown as Record<string, SignatureRequest>,
Expand Down
43 changes: 43 additions & 0 deletions packages/signature-controller/src/utils/normalize.test.ts
Original file line number Diff line number Diff line change
@@ -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"}');
},
);
});
});
61 changes: 61 additions & 0 deletions packages/signature-controller/src/utils/normalize.ts
Original file line number Diff line number Diff line change
@@ -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'));
}
2 changes: 1 addition & 1 deletion packages/signature-controller/src/utils/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
typedSignatureHash,
} from '@metamask/eth-sig-util';
import { SignTypedDataVersion } from '@metamask/keyring-controller';
import type { Hex } from '@metamask/utils';
import { type Hex } from '@metamask/utils';
import { validate } from 'jsonschema';

import type { MessageParamsPersonal, MessageParamsTyped } from '../types';
Expand Down

0 comments on commit 67d8a2c

Please sign in to comment.