From c98a9d56efa7d4c1a8b4c0ac81bc50b9b3b0f182 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Thu, 6 Nov 2025 14:54:57 +0100 Subject: [PATCH 1/7] fix: use timestamp based request id in windowPostMessageTransport --- CHANGELOG.md | 2 ++ .../windowPostMessageTransport.test.ts | 24 +++++++++++-------- src/transports/windowPostMessageTransport.ts | 2 +- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b80b24..cb1ae5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +- fix: use timestamp based request id on `windowPostMessageTransport` 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/transports/windowPostMessageTransport.test.ts b/src/transports/windowPostMessageTransport.test.ts index 5e43c39..739440e 100644 --- a/src/transports/windowPostMessageTransport.test.ts +++ b/src/transports/windowPostMessageTransport.test.ts @@ -24,11 +24,15 @@ 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 Date.now() to return a fixed value for predictable request IDs + vi.spyOn(Date, 'now').mockReturnValue(MOCK_INITIAL_REQUEST_ID); + // Setup addEventListener mock to capture the message handler mockWindow.addEventListener.mockImplementation((event: string, handler: (event: MessageEvent) => void) => { if (event === 'message') { @@ -62,7 +66,7 @@ describe('WindowPostMessageTransport', () => { name: MULTICHAIN_SUBSTREAM_NAME, data: { jsonrpc: '2.0', - id: 1, + id: MOCK_INITIAL_REQUEST_ID, method: 'wallet_getSession', }, }, @@ -77,7 +81,7 @@ describe('WindowPostMessageTransport', () => { data: { name: MULTICHAIN_SUBSTREAM_NAME, data: { - id: 1, + id: MOCK_INITIAL_REQUEST_ID, result: mockSession, }, }, @@ -87,7 +91,7 @@ describe('WindowPostMessageTransport', () => { const response = await requestPromise; expect(response).toEqual({ - id: 1, + id: MOCK_INITIAL_REQUEST_ID, result: mockSession, }); }); @@ -260,7 +264,7 @@ describe('WindowPostMessageTransport', () => { data: { name: MULTICHAIN_SUBSTREAM_NAME, data: { - id: 2, + id: MOCK_INITIAL_REQUEST_ID + 1, result: { success: true }, }, }, @@ -274,7 +278,7 @@ describe('WindowPostMessageTransport', () => { data: { name: MULTICHAIN_SUBSTREAM_NAME, data: { - id: 1, + id: MOCK_INITIAL_REQUEST_ID, result: mockSession, }, }, @@ -284,11 +288,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 +328,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 +344,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..413971e 100644 --- a/src/transports/windowPostMessageTransport.ts +++ b/src/transports/windowPostMessageTransport.ts @@ -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 = Date.now(); /** * Storing notification callbacks. * If we detect a "notification" (a message without an id) coming from the extension, we'll call each callback in here. From 7cbe77b56c2e7ec695cfc8bdbf47d10afe4e4ea8 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Thu, 6 Nov 2025 14:59:07 +0100 Subject: [PATCH 2/7] chore: update CHANGELOG.md to fix lint issue --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb1ae5f..c6f5d97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + - fix: use timestamp based request id on `windowPostMessageTransport` to avoid conflicts across disconnect/reconnect cycles in firefox ([#81](https://github.com/MetaMask/multichain-api-client/pull/81)) ## [0.8.0] From 94342eba694f8fa9fedb6357e587b02014f43432 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Tue, 11 Nov 2025 11:20:41 +0100 Subject: [PATCH 3/7] refactor: change implementation to use random number instead of Date.now() --- src/helpers/utils.ts | 17 +++++++++++++++++ .../externallyConnectableTransport.test.ts | 19 ++++++++++++++----- .../externallyConnectableTransport.ts | 4 ++-- .../windowPostMessageTransport.test.ts | 9 +++++++-- src/transports/windowPostMessageTransport.ts | 4 ++-- 5 files changed, 42 insertions(+), 11 deletions(-) diff --git a/src/helpers/utils.ts b/src/helpers/utils.ts index 1c74515..3ed0fb5 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..dbaca48 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,19 @@ describe('ExternallyConnectableTransport', () => { let transport: ReturnType; let messageHandler: (msg: any) => void; let disconnectHandler: () => void; + const MOCK_INITIAL_REQUEST_ID = 1000; + let getUniqueIdMock: ReturnType; let mockPort: MockPort; beforeEach(() => { vi.clearAllMocks(); + // Mock getUniqueId() to return sequential values starting from MOCK_INITIAL_REQUEST_ID + let requestIdCounter = MOCK_INITIAL_REQUEST_ID; + getUniqueIdMock = vi.spyOn(utils, 'getUniqueId').mockImplementation(() => { + return requestIdCounter++; + }); + // Setup mock port mockPort = { postMessage: vi.fn(), @@ -80,7 +89,7 @@ describe('ExternallyConnectableTransport', () => { messageHandler({ type: 'caip-348', data: { - id: 1, + id: MOCK_INITIAL_REQUEST_ID, jsonrpc: '2.0', result: mockSession, }, @@ -91,7 +100,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 +165,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..87743d4 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 739440e..4227cfc 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'; @@ -25,13 +26,17 @@ describe('WindowPostMessageTransport', () => { let transport: ReturnType; let messageHandler: (event: MessageEvent) => void; const MOCK_INITIAL_REQUEST_ID = 1000; + let getUniqueIdMock: ReturnType; beforeEach(() => { // Reset mocks vi.clearAllMocks(); - // Mock Date.now() to return a fixed value for predictable request IDs - vi.spyOn(Date, 'now').mockReturnValue(MOCK_INITIAL_REQUEST_ID); + // Mock getUniqueId() to return sequential values starting from MOCK_INITIAL_REQUEST_ID + let requestIdCounter = MOCK_INITIAL_REQUEST_ID; + getUniqueIdMock = vi.spyOn(utils, 'getUniqueId').mockImplementation(() => { + return requestIdCounter++; + }); // Setup addEventListener mock to capture the message handler mockWindow.addEventListener.mockImplementation((event: string, handler: (event: MessageEvent) => void) => { diff --git a/src/transports/windowPostMessageTransport.ts b/src/transports/windowPostMessageTransport.ts index 413971e..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 = Date.now(); + 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. From 7a17eb67f2eb67d393fffd10b828d68035ce61eb Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Tue, 11 Nov 2025 11:23:17 +0100 Subject: [PATCH 4/7] lint --- src/transports/externallyConnectableTransport.test.ts | 3 +-- src/transports/windowPostMessageTransport.test.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/transports/externallyConnectableTransport.test.ts b/src/transports/externallyConnectableTransport.test.ts index dbaca48..f756997 100644 --- a/src/transports/externallyConnectableTransport.test.ts +++ b/src/transports/externallyConnectableTransport.test.ts @@ -21,7 +21,6 @@ describe('ExternallyConnectableTransport', () => { let messageHandler: (msg: any) => void; let disconnectHandler: () => void; const MOCK_INITIAL_REQUEST_ID = 1000; - let getUniqueIdMock: ReturnType; let mockPort: MockPort; beforeEach(() => { @@ -29,7 +28,7 @@ describe('ExternallyConnectableTransport', () => { // Mock getUniqueId() to return sequential values starting from MOCK_INITIAL_REQUEST_ID let requestIdCounter = MOCK_INITIAL_REQUEST_ID; - getUniqueIdMock = vi.spyOn(utils, 'getUniqueId').mockImplementation(() => { + vi.spyOn(utils, 'getUniqueId').mockImplementation(() => { return requestIdCounter++; }); diff --git a/src/transports/windowPostMessageTransport.test.ts b/src/transports/windowPostMessageTransport.test.ts index 4227cfc..68d062c 100644 --- a/src/transports/windowPostMessageTransport.test.ts +++ b/src/transports/windowPostMessageTransport.test.ts @@ -26,7 +26,6 @@ describe('WindowPostMessageTransport', () => { let transport: ReturnType; let messageHandler: (event: MessageEvent) => void; const MOCK_INITIAL_REQUEST_ID = 1000; - let getUniqueIdMock: ReturnType; beforeEach(() => { // Reset mocks @@ -34,7 +33,7 @@ describe('WindowPostMessageTransport', () => { // Mock getUniqueId() to return sequential values starting from MOCK_INITIAL_REQUEST_ID let requestIdCounter = MOCK_INITIAL_REQUEST_ID; - getUniqueIdMock = vi.spyOn(utils, 'getUniqueId').mockImplementation(() => { + vi.spyOn(utils, 'getUniqueId').mockImplementation(() => { return requestIdCounter++; }); From e02f95ff4c85c42693fc7ef0d0c04124c27cad8b Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Tue, 11 Nov 2025 11:24:44 +0100 Subject: [PATCH 5/7] lint --- src/transports/externallyConnectableTransport.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transports/externallyConnectableTransport.ts b/src/transports/externallyConnectableTransport.ts index 87743d4..7adbc9d 100644 --- a/src/transports/externallyConnectableTransport.ts +++ b/src/transports/externallyConnectableTransport.ts @@ -28,7 +28,7 @@ export function getExternallyConnectableTransport( let { extensionId } = params; const { defaultTimeout = DEFAULT_REQUEST_TIMEOUT } = params; let chromePort: chrome.runtime.Port | undefined; - let requestId = getUniqueId() + let requestId = getUniqueId(); const pendingRequests = new Map void>(); /** From 764401808fee1d8cc3221e6a29cf6aadc61f87b0 Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Tue, 11 Nov 2025 11:26:22 +0100 Subject: [PATCH 6/7] lint --- src/helpers/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/helpers/utils.ts b/src/helpers/utils.ts index 3ed0fb5..3c797bf 100644 --- a/src/helpers/utils.ts +++ b/src/helpers/utils.ts @@ -16,7 +16,7 @@ let idCounter = Math.floor(Math.random() * MAX); export const getUniqueId = (): number => { idCounter = (idCounter + 1) % MAX; return idCounter; -} +}; /** * Detects if we're in a Chrome-like environment with extension support From 31accf1772146675c7f4cbfa5294755a9a1ef50d Mon Sep 17 00:00:00 2001 From: ffmcgee Date: Tue, 11 Nov 2025 11:34:19 +0100 Subject: [PATCH 7/7] chore: update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6f5d97..2ee0240 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- fix: use timestamp based request id on `windowPostMessageTransport` to avoid conflicts across disconnect/reconnect cycles in firefox ([#81](https://github.com/MetaMask/multichain-api-client/pull/81)) +- 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]