Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support MMI use cases in signing typed messages #1364

Merged
merged 10 commits into from
May 17, 2023
58 changes: 58 additions & 0 deletions packages/message-manager/src/AbstractMessageManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const typedMessage = [
},
];
const messageId = '1';
const messageId2 = '2';
const from = '0x0123';
const messageTime = Date.now();
const messageStatus = 'unapproved';
Expand Down Expand Up @@ -84,6 +85,35 @@ describe('AbstractTestManager', () => {
expect(message.type).toBe(messageType);
});

it('should get all messages', async () => {
const controller = new AbstractTestManager();
const message = {
id: messageId,
messageParams: {
data: typedMessage,
from,
},
status: messageStatus,
time: messageTime,
type: messageType,
};
const message2 = {
id: messageId2,
messageParams: {
data: typedMessage,
from,
},
status: messageStatus,
time: messageTime,
type: messageType,
};

await controller.addMessage(message);
matthewwalsh0 marked this conversation as resolved.
Show resolved Hide resolved
await controller.addMessage(message2);
const messages = controller.getAllMessages();
expect(messages).toStrictEqual([message, message2]);
});

it('adds a valid message with provider security response', async () => {
const securityProviderResponseMock = { flagAsDangerous: 2 };
const securityProviderRequestMock: SecurityProviderRequest = jest
Expand Down Expand Up @@ -331,4 +361,32 @@ describe('AbstractTestManager', () => {
expect(messageAfter?.status).toStrictEqual('newstatus');
});
});

describe('setMetadata', () => {
it('should set the given message metadata', async () => {
const controller = new AbstractTestManager();
await controller.addMessage({
id: messageId,
messageParams: { from: '0x1234', data: 'test' },
status: 'status',
time: 10,
type: 'type',
});

const messageBefore = controller.getMessage(messageId);
expect(messageBefore?.metadata).toBeUndefined();

controller.setMetadata(messageId, { foo: 'bar' });
const messageAfter = controller.getMessage(messageId);
expect(messageAfter?.metadata).toStrictEqual({ foo: 'bar' });
});

it('should throw an error if message is not found', () => {
const controller = new AbstractTestManager();

expect(() => controller.setMetadata(messageId, { foo: 'bar' })).toThrow(
'AbstractMessageManager: Message not found for id: 1.',
);
});
});
});
29 changes: 29 additions & 0 deletions packages/message-manager/src/AbstractMessageManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export interface OriginalRequest {
* A 'Message' which always has a signing type
* @property rawSig - Raw data of the signature request
* @property securityProviderResponse - Response from a security provider, whether it is malicious or not
* @property metadata - Additional data for the message, for example external identifiers
*/
export interface AbstractMessage {
id: string;
Expand All @@ -33,6 +34,7 @@ export interface AbstractMessage {
type: string;
rawSig?: string;
securityProviderResponse?: Record<string, Json>;
metadata?: Json;
}

/**
Expand All @@ -41,10 +43,12 @@ export interface AbstractMessage {
* Represents the parameters to pass to the signing method once the signature request is approved.
* @property from - Address from which the message is processed
* @property origin? - Added for request origin identification
* @property deferSetAsSigned? - Whether to defer setting the message as signed immediately after the keyring is told to sign it
*/
export interface AbstractMessageParams {
from: string;
origin?: string;
deferSetAsSigned?: boolean;
}

/**
Expand Down Expand Up @@ -257,6 +261,15 @@ export abstract class AbstractMessageManager<
return this.messages.find((message) => message.id === messageId);
}

/**
* Returns all the messages.
*
* @returns An array of messages.
*/
getAllMessages() {
return this.messages;
}

/**
* Approves a Message. Sets the message status via a call to this.setMessageStatusApproved,
* and returns a promise with any the message params modified for proper signing.
Expand Down Expand Up @@ -331,6 +344,22 @@ export abstract class AbstractMessageManager<
this.updateMessage(message, false);
}

/**
* Sets the messsage metadata
*
* @param messageId - The id of the Message to update
* @param metadata - The data with which to replace the metadata property in the message
*/

setMetadata(messageId: string, metadata: Json) {
const message = this.getMessage(messageId);
if (!message) {
throw new Error(`${this.name}: Message not found for id: ${messageId}.`);
}
message.metadata = metadata;
this.updateMessage(message, false);
}

/**
* Removes the metamaskId property from passed messageParams and returns a promise which
* resolves the updated messageParams
Expand Down
47 changes: 47 additions & 0 deletions packages/signature-controller/src/SignatureController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -378,18 +378,21 @@ describe('SignatureController', () => {
messageManagerMock,
() => signatureController.signMessage,
() => keyringControllerMock.signMessage,
'eth_sign',
],
[
'signPersonalMessage',
personalMessageManagerMock,
() => signatureController.signPersonalMessage,
() => keyringControllerMock.signPersonalMessage,
'personal_sign',
],
[
'signTypedMessage',
typedMessageManagerMock,
() => signatureController.signTypedMessage,
() => keyringControllerMock.signTypedMessage,
'eth_signTypedData',
],
])(
'%s',
Expand All @@ -398,6 +401,7 @@ describe('SignatureController', () => {
messageManager,
getSignatureControllerMethod,
getKeyringControllerMethod,
rpcMethodName,
) => {
let signatureControllerMethod: (...args: any[]) => Promise<string>;
let keyringControllerMethod: jest.Mock;
Expand Down Expand Up @@ -437,6 +441,49 @@ describe('SignatureController', () => {
);
});

it('emits an event when the message is signed by the keyring', async () => {
const eventListener = jest.fn();

signatureController.hub.once(`${rpcMethodName}:signed`, eventListener);

await (signatureController as any)[signMethodName](messageParamsMock);

expect(eventListener).toHaveBeenCalledWith({
messageId: messageIdMock,
signature: signatureMock,
});
});

it('does not set as signed, messages with deferSetAsSigned', async () => {
const deferredMessageParams = {
...messageParamsMock,
deferSetAsSigned: true,
};

messageManager.approveMessage.mockReset();
messageManager.approveMessage.mockResolvedValueOnce(
deferredMessageParams,
);

await (signatureController as any)[signMethodName](
deferredMessageParams,
);

const keyringControllerExtraArgs =
// eslint-disable-next-line jest/no-if
signMethodName === 'signTypedMessage'
? [{ version: messageParamsMock.version }]
: [];

expect(keyringControllerMethod).toHaveBeenCalledTimes(1);
expect(keyringControllerMethod).toHaveBeenCalledWith(
deferredMessageParams,
...keyringControllerExtraArgs,
);

expect(messageManager.setMessageStatusSigned).not.toHaveBeenCalled();
});

it('returns current state', async () => {
getAllStateMock.mockReturnValueOnce(stateMock);
expect(
Expand Down
6 changes: 5 additions & 1 deletion packages/signature-controller/src/SignatureController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,11 @@ export class SignatureController extends BaseControllerV2<
const cleanMessageParams = await messageManager.approveMessage(msgParams);
const signature = await getSignature(cleanMessageParams);

messageManager.setMessageStatusSigned(messageId, signature);
this.hub.emit(`${methodName}:signed`, { signature, messageId });
matthewwalsh0 marked this conversation as resolved.
Show resolved Hide resolved

if (!cleanMessageParams.deferSetAsSigned) {
matthewwalsh0 marked this conversation as resolved.
Show resolved Hide resolved
messageManager.setMessageStatusSigned(messageId, signature);
}

this.#acceptApproval(messageId);

Expand Down