feat: add request id to messageParams in @metamask/message-managers (#…

…4636)

## Explanation

<!--
Thanks for your contribution! Take a moment to answer these questions so
that reviewers have the information they need to properly understand
your changes:

* What is the current state of things and why does it need to change?
* What is the solution your changes offer and how does it work?
* Are there any changes whose purpose might not obvious to those
unfamiliar with the domain?
* If your primary goal was to update one package but you found you had
to update another one along the way, why did you do so?
* If you had to upgrade a dependency, why did you do so?
-->

Adding the request id to reference metric event fragments created from
the createRPCMethodTrackingMiddleware in the client. See use-case here:
MetaMask/metamask-extension#26597

## References

<!--
Are there any issues that this pull request is tied to? Are there other
links that reviewers should consult to understand these changes better?

For example:

Blocks: MetaMask/metamask-extension#26597
* Fixes #12345
* Related to #67890
-->

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/message-manager`

- **feat**: Add request id to messageParams

## Checklist

- [ ] I've updated the test suite for new or updated code as appropriate
- [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate

---------

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

packages/message-manager/src/AbstractMessageManager.test.ts

@@ -1,4 +1,10 @@
-import type { SecurityProviderRequest } from './AbstractMessageManager';
+import { ApprovalType } from '@metamask/controller-utils';
+
+import type {
+  AbstractMessageParams,
+  OriginalRequest,
+  SecurityProviderRequest,
+} from './AbstractMessageManager';
 import { AbstractMessageManager } from './AbstractMessageManager';
 import type {
   TypedMessage,
@@ -11,6 +17,21 @@ class AbstractTestManager extends AbstractMessageManager<
   TypedMessageParams,
   TypedMessageParamsMetamask
 > {
+  addRequestToMessageParams<MessageParams extends AbstractMessageParams>(
+    messageParams: MessageParams,
+    req?: OriginalRequest,
+  ) {
+    return super.addRequestToMessageParams(messageParams, req);
+  }
+
+  createUnapprovedMessage<MessageParams extends AbstractMessageParams>(
+    messageParams: MessageParams,
+    type: ApprovalType,
+    req?: OriginalRequest,
+  ) {
+    return super.createUnapprovedMessage(messageParams, type, req);
+  }
+
   prepMessageForSigning(
     messageParams: TypedMessageParamsMetamask,
   ): Promise<TypedMessageParams> {
@@ -50,6 +71,14 @@ const rawSigMock = '0xsignaturemocked';
 const messageIdMock = 'message-id-mocked';
 const fromMock = '0xc38bf1ad06ef69f0c04e29dbeb4152b4175f0a8d';
 
+const mockSecurityProviderResponse = { flagAsDangerous: 2 };
+const mockRequest = {
+  origin: 'example.com',
+  id: 123,
+  securityAlertResponse: mockSecurityProviderResponse,
+};
+const mockMessageParams = { from, data: 'test' };
+
 describe('AbstractTestManager', () => {
   it('should set default state', () => {
     const controller = new AbstractTestManager();
@@ -317,12 +346,54 @@ describe('AbstractTestManager', () => {
     expect(message.status).toBe('approved');
   });
 
+  describe('addRequestToMessageParams', () => {
+    it('adds original request id and origin to messageParams', () => {
+      const controller = new AbstractTestManager();
+
+      const result = controller.addRequestToMessageParams(
+        mockMessageParams,
+        mockRequest,
+      );
+
+      expect(result).toStrictEqual({
+        ...mockMessageParams,
+        origin: mockRequest.origin,
+        requestId: mockRequest.id,
+      });
+    });
+  });
+
+  describe('createUnapprovedMessage', () => {
+    it('creates a Message object with an unapproved status', () => {
+      const controller = new AbstractTestManager();
+
+      const result = controller.createUnapprovedMessage(
+        mockMessageParams,
+        ApprovalType.PersonalSign,
+        mockRequest,
+      );
+
+      expect(result.messageParams).toBe(mockMessageParams);
+      expect(result.securityAlertResponse).toBe(
+        mockRequest.securityAlertResponse,
+      );
+      expect(result.status).toBe('unapproved');
+      expect(result.type).toBe(ApprovalType.PersonalSign);
+      expect(typeof result.time).toBe('number');
+      expect(typeof result.id).toBe('string');
+    });
+  });
+
   describe('setMessageStatus', () => {
-    it('should set the given message status', async () => {
+    it('updates the status of a message', async () => {
+      jest.mock('events', () => ({
+        emit: jest.fn(),
+      }));
+
       const controller = new AbstractTestManager();
       await controller.addMessage({
         id: messageId,
-        messageParams: { from: '0x1234', data: 'test' },
+        messageParams: { ...mockMessageParams },
         status: 'status',
         time: 10,
         type: 'type',
@@ -331,11 +402,12 @@ describe('AbstractTestManager', () => {
       expect(messageBefore?.status).toBe('status');
 
       controller.setMessageStatus(messageId, 'newstatus');
+
       const messageAfter = controller.getMessage(messageId);
       expect(messageAfter?.status).toBe('newstatus');
     });
 
-    it('should throw an error if message is not found', () => {
+    it('throws an error if the message is not found', async () => {
       const controller = new AbstractTestManager();
 
       expect(() => controller.setMessageStatus(messageId, 'newstatus')).toThrow(
@@ -353,7 +425,7 @@ describe('AbstractTestManager', () => {
       const controller = new AbstractTestManager();
       await controller.addMessage({
         id: messageId,
-        messageParams: { from: '0x1234', data: 'test' },
+        messageParams: { ...mockMessageParams },
         status: 'status',
         time: 10,
         type: 'type',
@@ -374,7 +446,7 @@ describe('AbstractTestManager', () => {
       const controller = new AbstractTestManager();
       await controller.addMessage({
         id: messageId,
-        messageParams: { from: '0x1234', data: 'test' },
+        messageParams: { ...mockMessageParams },
         status: 'status',
         time: 10,
         type: 'type',

packages/message-manager/src/AbstractMessageManager.ts

@@ -1,7 +1,9 @@
 import type { BaseConfig, BaseState } from '@metamask/base-controller';
 import { BaseControllerV1 } from '@metamask/base-controller';
+import type { ApprovalType } from '@metamask/controller-utils';
 import type { Hex, Json } from '@metamask/utils';
 import { EventEmitter } from 'events';
+import { v1 as random } from 'uuid';
 
 /**
  * @type OriginalRequest
@@ -13,6 +15,7 @@ import { EventEmitter } from 'events';
 // Convert to a `type` in a future major version.
 // eslint-disable-next-line @typescript-eslint/consistent-type-definitions
 export interface OriginalRequest {
+  id?: number;
   origin?: string;
   securityAlertResponse?: Record<string, Json>;
 }
@@ -49,6 +52,7 @@ 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 requestId? - Original request id
  * @property deferSetAsSigned? - Whether to defer setting the message as signed immediately after the keyring is told to sign it
  */
 // This interface was created before this ESLint rule was added.
@@ -57,6 +61,7 @@ export interface AbstractMessage {
 export interface AbstractMessageParams {
   from: string;
   origin?: string;
+  requestId?: number;
   deferSetAsSigned?: boolean;
 }
 
@@ -127,6 +132,49 @@ export abstract class AbstractMessageManager<
 
   private readonly additionalFinishStatuses: string[];
 
+  /**
+   * Adds request props to the messsage params and returns a new messageParams object.
+   * @param messageParams - The messageParams to add the request props to.
+   * @param req - The original request object.
+   * @returns The messageParams with the request props added.
+   */
+  protected addRequestToMessageParams<
+    MessageParams extends AbstractMessageParams,
+  >(messageParams: MessageParams, req?: OriginalRequest) {
+    const updatedMessageParams = {
+      ...messageParams,
+    };
+
+    if (req) {
+      updatedMessageParams.requestId = req.id;
+      updatedMessageParams.origin = req.origin;
+    }
+
+    return updatedMessageParams;
+  }
+
+  /**
+   * Creates a new Message with a random id and an 'unapproved' status.
+   * @param messageParams - The messageParams to add the request props to.
+   * @param type - The approval type of the message.
+   * @param req - The original request object.
+   * @returns The new unapproved message for a specified type.
+   */
+  protected createUnapprovedMessage<
+    MessageParams extends AbstractMessageParams,
+  >(messageParams: MessageParams, type: ApprovalType, req?: OriginalRequest) {
+    const messageId = random();
+
+    return {
+      id: messageId,
+      messageParams,
+      securityAlertResponse: req?.securityAlertResponse,
+      status: 'unapproved',
+      time: Date.now(),
+      type,
+    };
+  }
+
   /**
    * Saves the unapproved messages, and their count to state.
    *

packages/message-manager/src/DecryptMessageManager.test.ts

@@ -140,7 +140,7 @@ describe('DecryptMessageManager', () => {
     const messageStatus = 'unapproved';
     const messageType = 'eth_decrypt';
     const messageParams = { from: fromMock, data: dataMock };
-    const originalRequest = { origin: 'origin' };
+    const originalRequest = { id: 111, origin: 'origin' };
     const messageId = await controller.addUnapprovedMessage(
       messageParams,
       originalRequest,
@@ -151,6 +151,7 @@ describe('DecryptMessageManager', () => {
       throw new Error('"message" is falsy');
     }
     expect(message.messageParams.from).toBe(messageParams.from);
+    expect(message.messageParams.requestId).toBe(originalRequest.id);
     expect(message.time).toBeDefined();
     expect(message.status).toBe(messageStatus);
     expect(message.type).toBe(messageType);

packages/message-manager/src/DecryptMessageManager.ts

@@ -1,4 +1,4 @@
-import { v1 as random } from 'uuid';
+import { ApprovalType } from '@metamask/controller-utils';
 
 import type {
   AbstractMessage,
@@ -129,22 +129,24 @@ export class DecryptMessageManager extends AbstractMessageManager<
     messageParams: DecryptMessageParams,
     req?: OriginalRequest,
   ) {
-    if (req) {
-      messageParams.origin = req.origin;
-    }
-    messageParams.data = normalizeMessageData(messageParams.data);
-    const messageId = random();
-    const messageData: DecryptMessage = {
-      id: messageId,
+    const updatedMessageParams = this.addRequestToMessageParams(
       messageParams,
-      status: 'unapproved',
-      time: Date.now(),
-      type: 'eth_decrypt',
-    };
+      req,
+    ) satisfies DecryptMessageParams;
+    messageParams.data = normalizeMessageData(messageParams.data);
+
+    const messageData = this.createUnapprovedMessage(
+      updatedMessageParams,
+      ApprovalType.EthDecrypt,
+      req,
+    ) satisfies DecryptMessage;
+
+    const messageId = messageData.id;
+
     await this.addMessage(messageData);
     this.hub.emit(`unapprovedMessage`, {
-      ...messageParams,
-      ...{ metamaskId: messageId },
+      ...updatedMessageParams,
+      metamaskId: messageId,
     });
     return messageId;
   }

packages/message-manager/src/EncryptionPublicKeyManager.test.ts

@@ -120,7 +120,7 @@ describe('EncryptionPublicKeyManager', () => {
     const messageParams = {
       from: fromMock,
     };
-    const originalRequest = { origin: 'origin' };
+    const originalRequest = { id: 111, origin: 'origin' };
     const messageId = await controller.addUnapprovedMessage(
       messageParams,
       originalRequest,
@@ -131,6 +131,7 @@ describe('EncryptionPublicKeyManager', () => {
       throw new Error('"message" is falsy');
     }
     expect(message.messageParams.from).toBe(messageParams.from);
+    expect(message.messageParams.requestId).toBe(originalRequest.id);
     expect(message.time).toBeDefined();
     expect(message.status).toBe(messageStatus);
     expect(message.type).toBe(messageType);

packages/message-manager/src/EncryptionPublicKeyManager.ts

@@ -1,4 +1,4 @@
-import { v1 as random } from 'uuid';
+import { ApprovalType } from '@metamask/controller-utils';
 
 import type {
   AbstractMessage,
@@ -120,21 +120,23 @@ export class EncryptionPublicKeyManager extends AbstractMessageManager<
     messageParams: EncryptionPublicKeyParams,
     req?: OriginalRequest,
   ): Promise<string> {
-    if (req) {
-      messageParams.origin = req.origin;
-    }
-    const messageId = random();
-    const messageData: EncryptionPublicKey = {
-      id: messageId,
+    const updatedMessageParams = this.addRequestToMessageParams(
       messageParams,
-      status: 'unapproved',
-      time: Date.now(),
-      type: 'eth_getEncryptionPublicKey',
-    };
+      req,
+    ) satisfies EncryptionPublicKeyParams;
+
+    const messageData = this.createUnapprovedMessage(
+      updatedMessageParams,
+      ApprovalType.EthGetEncryptionPublicKey,
+      req,
+    ) satisfies EncryptionPublicKey;
+
+    const messageId = messageData.id;
+
     await this.addMessage(messageData);
     this.hub.emit(`unapprovedMessage`, {
-      ...messageParams,
-      ...{ metamaskId: messageId },
+      ...updatedMessageParams,
+      metamaskId: messageId,
     });
     return messageId;
   }

packages/message-manager/src/PersonalMessageManager.test.ts

@@ -79,6 +79,7 @@ describe('PersonalMessageManager', () => {
       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
@@ -95,6 +96,7 @@ describe('PersonalMessageManager', () => {
     }
     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);