diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b80b24..2ee0240 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/helpers/utils.ts b/src/helpers/utils.ts index 1c74515..3c797bf 100644 --- a/src/helpers/utils.ts +++ b/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 */ diff --git a/src/transports/externallyConnectableTransport.test.ts b/src/transports/externallyConnectableTransport.test.ts index 74ef97c..f756997 100644 --- a/src/transports/externallyConnectableTransport.test.ts +++ b/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; 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 }); }); }); diff --git a/src/transports/externallyConnectableTransport.ts b/src/transports/externallyConnectableTransport.ts index 1c9158f..7adbc9d 100644 --- a/src/transports/externallyConnectableTransport.ts +++ b/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 void>(); /** diff --git a/src/transports/windowPostMessageTransport.test.ts b/src/transports/windowPostMessageTransport.test.ts index 5e43c39..68d062c 100644 --- a/src/transports/windowPostMessageTransport.test.ts +++ b/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; 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 }); }); }); diff --git a/src/transports/windowPostMessageTransport.ts b/src/transports/windowPostMessageTransport.ts index 603f839..8c74fd3 100644 --- a/src/transports/windowPostMessageTransport.ts +++ b/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 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.