fix: use randomly generated request id in windowPostMessageTransport and externallyConnectableTransport (#81)

* fix: use randomly generated request id in windowPostMessageTransport and externallyConnectableTransport

Author: ffmcgee725

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- fix: use randomly generated request id on `windowPostMessageTransport` and `externallyConnectableTransport` to avoid conflicts across disconnect/reconnect cycles in firefox ([#81](https://github.com/MetaMask/multichain-api-client/pull/81))
+
 ## [0.8.0]
 
 ### Added

src/helpers/utils.ts

@@ -1,6 +1,23 @@
 // chrome is a global injected by browser extensions
 declare const chrome: any;
 
+// uint32 (two's complement) max
+// more conservative than Number.MAX_SAFE_INTEGER
+const MAX = 4_294_967_295;
+let idCounter = Math.floor(Math.random() * MAX);
+
+/**
+ * Gets an ID that is guaranteed to be unique so long as no more than
+ * 4_294_967_295 (uint32 max) IDs are created, or the IDs are rapidly turned
+ * over.
+ *
+ * @returns The unique ID.
+ */
+export const getUniqueId = (): number => {
+  idCounter = (idCounter + 1) % MAX;
+  return idCounter;
+};
+
 /**
  * Detects if we're in a Chrome-like environment with extension support
  */

src/transports/externallyConnectableTransport.test.ts

@@ -1,6 +1,7 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import { type MockPort, mockSession } from '../../tests/mocks';
 import * as metamaskExtensionId from '../helpers/metamaskExtensionId';
+import * as utils from '../helpers/utils';
 import { TransportError } from '../types/errors';
 import { getExternallyConnectableTransport } from './externallyConnectableTransport';
 
@@ -19,11 +20,18 @@ describe('ExternallyConnectableTransport', () => {
   let transport: ReturnType<typeof getExternallyConnectableTransport>;
   let messageHandler: (msg: any) => void;
   let disconnectHandler: () => void;
+  const MOCK_INITIAL_REQUEST_ID = 1000;
 
   let mockPort: MockPort;
   beforeEach(() => {
     vi.clearAllMocks();
 
+    // Mock getUniqueId() to return sequential values starting from MOCK_INITIAL_REQUEST_ID
+    let requestIdCounter = MOCK_INITIAL_REQUEST_ID;
+    vi.spyOn(utils, 'getUniqueId').mockImplementation(() => {
+      return requestIdCounter++;
+    });
+
     // Setup mock port
     mockPort = {
       postMessage: vi.fn(),
@@ -80,7 +88,7 @@ describe('ExternallyConnectableTransport', () => {
     messageHandler({
       type: 'caip-348',
       data: {
-        id: 1,
+        id: MOCK_INITIAL_REQUEST_ID,
         jsonrpc: '2.0',
         result: mockSession,
       },
@@ -91,7 +99,7 @@ describe('ExternallyConnectableTransport', () => {
     expect(mockPort.postMessage).toHaveBeenCalledWith({
       type: 'caip-348',
       data: {
-        id: 1,
+        id: MOCK_INITIAL_REQUEST_ID,
         jsonrpc: '2.0',
         method: 'wallet_getSession',
       },
@@ -156,19 +164,19 @@ describe('ExternallyConnectableTransport', () => {
       'Transport request timed out',
     );
 
-    // Second request should work (id 2)
+    // Second request should work (id MOCK_INITIAL_REQUEST_ID + 1)
     const secondPromise = transport.request({ method: 'wallet_getSession' });
 
     messageHandler({
       type: 'caip-348',
       data: {
-        id: 2,
+        id: MOCK_INITIAL_REQUEST_ID + 1,
         jsonrpc: '2.0',
         result: mockSession,
       },
     });
 
     const response = await secondPromise;
-    expect(response).toEqual({ id: 2, jsonrpc: '2.0', result: mockSession });
+    expect(response).toEqual({ id: MOCK_INITIAL_REQUEST_ID + 1, jsonrpc: '2.0', result: mockSession });
   });
 });

src/transports/externallyConnectableTransport.ts

@@ -1,5 +1,5 @@
 import { detectMetamaskExtensionId } from '../helpers/metamaskExtensionId';
-import { withTimeout } from '../helpers/utils';
+import { getUniqueId, withTimeout } from '../helpers/utils';
 import { TransportError, TransportTimeoutError } from '../types/errors';
 import type { Transport, TransportResponse } from '../types/transport';
 import { DEFAULT_REQUEST_TIMEOUT, REQUEST_CAIP } from './constants';
@@ -28,7 +28,7 @@ export function getExternallyConnectableTransport(
   let { extensionId } = params;
   const { defaultTimeout = DEFAULT_REQUEST_TIMEOUT } = params;
   let chromePort: chrome.runtime.Port | undefined;
-  let requestId = 1;
+  let requestId = getUniqueId();
   const pendingRequests = new Map<number, (value: any) => void>();
 
   /**

src/transports/windowPostMessageTransport.test.ts

@@ -1,5 +1,6 @@
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 import { mockSession } from '../../tests/mocks';
+import * as utils from '../helpers/utils';
 import { TransportError } from '../types/errors';
 import { CONTENT_SCRIPT, INPAGE, MULTICHAIN_SUBSTREAM_NAME } from './constants';
 import { getWindowPostMessageTransport } from './windowPostMessageTransport';
@@ -24,11 +25,18 @@ global.location = mockLocation as any;
 describe('WindowPostMessageTransport', () => {
   let transport: ReturnType<typeof getWindowPostMessageTransport>;
   let messageHandler: (event: MessageEvent) => void;
+  const MOCK_INITIAL_REQUEST_ID = 1000;
 
   beforeEach(() => {
     // Reset mocks
     vi.clearAllMocks();
 
+    // Mock getUniqueId() to return sequential values starting from MOCK_INITIAL_REQUEST_ID
+    let requestIdCounter = MOCK_INITIAL_REQUEST_ID;
+    vi.spyOn(utils, 'getUniqueId').mockImplementation(() => {
+      return requestIdCounter++;
+    });
+
     // Setup addEventListener mock to capture the message handler
     mockWindow.addEventListener.mockImplementation((event: string, handler: (event: MessageEvent) => void) => {
       if (event === 'message') {
@@ -62,7 +70,7 @@ describe('WindowPostMessageTransport', () => {
           name: MULTICHAIN_SUBSTREAM_NAME,
           data: {
             jsonrpc: '2.0',
-            id: 1,
+            id: MOCK_INITIAL_REQUEST_ID,
             method: 'wallet_getSession',
           },
         },
@@ -77,7 +85,7 @@ describe('WindowPostMessageTransport', () => {
         data: {
           name: MULTICHAIN_SUBSTREAM_NAME,
           data: {
-            id: 1,
+            id: MOCK_INITIAL_REQUEST_ID,
             result: mockSession,
           },
         },
@@ -87,7 +95,7 @@ describe('WindowPostMessageTransport', () => {
 
     const response = await requestPromise;
     expect(response).toEqual({
-      id: 1,
+      id: MOCK_INITIAL_REQUEST_ID,
       result: mockSession,
     });
   });
@@ -260,7 +268,7 @@ describe('WindowPostMessageTransport', () => {
         data: {
           name: MULTICHAIN_SUBSTREAM_NAME,
           data: {
-            id: 2,
+            id: MOCK_INITIAL_REQUEST_ID + 1,
             result: { success: true },
           },
         },
@@ -274,7 +282,7 @@ describe('WindowPostMessageTransport', () => {
         data: {
           name: MULTICHAIN_SUBSTREAM_NAME,
           data: {
-            id: 1,
+            id: MOCK_INITIAL_REQUEST_ID,
             result: mockSession,
           },
         },
@@ -284,11 +292,11 @@ describe('WindowPostMessageTransport', () => {
 
     const [response1, response2] = await Promise.all([request1Promise, request2Promise]);
     expect(response1).toEqual({
-      id: 1,
+      id: MOCK_INITIAL_REQUEST_ID,
       result: mockSession,
     });
     expect(response2).toEqual({
-      id: 2,
+      id: MOCK_INITIAL_REQUEST_ID + 1,
       result: { success: true },
     });
   });
@@ -324,14 +332,14 @@ describe('WindowPostMessageTransport', () => {
     // Second request should still work (simulate response)
     const secondPromise = transport.request({ method: 'wallet_getSession' });
 
-    // Simulate response for id 2 (because first timed out with id 1, second increments to 2)
+    // Simulate response for id MOCK_INITIAL_REQUEST_ID + 1 (because first timed out with id MOCK_INITIAL_REQUEST_ID, second increments to MOCK_INITIAL_REQUEST_ID + 1)
     messageHandler({
       data: {
         target: INPAGE,
         data: {
           name: MULTICHAIN_SUBSTREAM_NAME,
           data: {
-            id: 2,
+            id: MOCK_INITIAL_REQUEST_ID + 1,
             result: mockSession,
           },
         },
@@ -340,6 +348,6 @@ describe('WindowPostMessageTransport', () => {
     } as MessageEvent);
 
     const result = await secondPromise;
-    expect(result).toEqual({ id: 2, result: mockSession });
+    expect(result).toEqual({ id: MOCK_INITIAL_REQUEST_ID + 1, result: mockSession });
   });
 });

src/transports/windowPostMessageTransport.ts

@@ -1,4 +1,4 @@
-import { withTimeout } from '../helpers/utils';
+import { getUniqueId, withTimeout } from '../helpers/utils';
 import { TransportError, TransportTimeoutError } from '../types/errors';
 import type { Transport, TransportResponse } from '../types/transport';
 import { CONTENT_SCRIPT, DEFAULT_REQUEST_TIMEOUT, INPAGE, MULTICHAIN_SUBSTREAM_NAME } from './constants';
@@ -20,7 +20,7 @@ export function getWindowPostMessageTransport(params: { defaultTimeout?: number
   const { defaultTimeout = DEFAULT_REQUEST_TIMEOUT } = params;
   let messageListener: ((event: MessageEvent) => void) | null = null;
   const pendingRequests: Map<number, (value: any) => void> = new Map();
-  let requestId = 1;
+  let requestId = getUniqueId();
   /**
    * Storing notification callbacks.
    * If we detect a "notification" (a message without an id) coming from the extension, we'll call each callback in here.