From bb6e43839f7006afcb57478c886cb4da7f8fcb3b Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Wed, 3 Jun 2026 12:15:22 +0200 Subject: [PATCH 1/2] fix(profile-sync-controller): deduplicate identical pairing calls --- packages/profile-sync-controller/CHANGELOG.md | 2 + .../flow-srp.test.ts | 116 ++++++++++++++++++ .../sdk/authentication-jwt-bearer/flow-srp.ts | 49 +++++++- 3 files changed, 166 insertions(+), 1 deletion(-) diff --git a/packages/profile-sync-controller/CHANGELOG.md b/packages/profile-sync-controller/CHANGELOG.md index ea74007481..2cc3073140 100644 --- a/packages/profile-sync-controller/CHANGELOG.md +++ b/packages/profile-sync-controller/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Deduplicate SRP profile pairing calls by payload in `SRPJwtBearerAuth.pairSrpProfiles` + - Concurrent and short-interval sequential `performSignIn` triggers (multiple UI surfaces, unlock-time effects, pairing retries) previously each issued their own `POST /api/v2/profile/pair`. A promise cache keyed by the token set now coalesces in-flight calls and reuses a successful result for a short TTL, collapsing the redundant calls into one. The key is order-insensitive (the same token set in any order dedupes), and failures are never cached so the existing `needsProfilePairing` retry loop still re-hits the endpoint. - Scope the storage-key and snap-signature caches in `UserStorageController` per `entropySourceId` ([#8948](https://github.com/MetaMask/core/pull/8948)) - In the edge case where two SRPs resolve to the same `profileId` (e.g. a shared canonical profile after pairing), the previously message-keyed caches could hand one SRP the other's cached key — letting it read/decrypt the other SRP's user storage (which surfaced as a second SRP inheriting the first SRP's account names) or write under its key. Scoping by `entropySourceId` keeps each SRP's key isolated even then; the signed `metamask:${profileId}` message is unchanged, so existing storage keys are preserved. diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.test.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.test.ts index 0360f99836..c8c796617c 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.test.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.test.ts @@ -20,12 +20,14 @@ const mockDelay = timeUtils.delay as jest.MockedFunction< const mockGetNonce = jest.fn(); const mockAuthenticate = jest.fn(); const mockAuthorizeOIDC = jest.fn(); +const mockPairProfiles = jest.fn(); jest.mock('./services', () => ({ authenticate: (...args: unknown[]): unknown => mockAuthenticate(...args), authorizeOIDC: (...args: unknown[]): unknown => mockAuthorizeOIDC(...args), getNonce: (...args: unknown[]): unknown => mockGetNonce(...args), getUserProfileLineage: jest.fn(), + pairProfiles: (...args: unknown[]): unknown => mockPairProfiles(...args), })); // Mock computeIdentifierId @@ -516,3 +518,117 @@ describe('SRPJwtBearerAuth profileId resolution', () => { expect(store.value?.token.accessToken).toBe('access-token'); }); }); + +describe('SRPJwtBearerAuth pairSrpProfiles deduplication', () => { + const config: AuthConfig & { type: AuthType.SRP } = { + type: AuthType.SRP, + env: Env.DEV, + platform: Platform.EXTENSION, + }; + + const MOCK_PAIR_RESPONSE = { + profile: { + profileId: 'p1', + canonicalProfileId: 'p1', + metaMetricsId: 'm1', + identifierId: 'i1', + }, + profileAliases: [], + }; + + const createAuth = (): { auth: SRPJwtBearerAuth } => { + const store: { value: LoginResponse | null } = { value: null }; + const auth = new SRPJwtBearerAuth(config, { + storage: { + getLoginResponse: async (): Promise => + store.value, + setLoginResponse: async (val): Promise => { + store.value = val; + }, + }, + signing: { + getIdentifier: async (): Promise => 'identifier-1', + signMessage: async (): Promise => 'signature-1', + }, + }); + return { auth }; + }; + + beforeEach((): void => { + jest.clearAllMocks(); + mockPairProfiles.mockResolvedValue(MOCK_PAIR_RESPONSE); + }); + + it('coalesces concurrent calls with the same payload into one request', async () => { + const { auth } = createAuth(); + + const [r1, r2, r3] = await Promise.all([ + auth.pairSrpProfiles(['a', 'b', 'c'], 'a'), + auth.pairSrpProfiles(['a', 'b', 'c'], 'a'), + auth.pairSrpProfiles(['a', 'b', 'c'], 'a'), + ]); + + expect(r1).toStrictEqual(MOCK_PAIR_RESPONSE); + expect(r2).toStrictEqual(MOCK_PAIR_RESPONSE); + expect(r3).toStrictEqual(MOCK_PAIR_RESPONSE); + expect(mockPairProfiles).toHaveBeenCalledTimes(1); + }); + + it('dedupes the same token set provided in a different order', async () => { + const { auth } = createAuth(); + + await auth.pairSrpProfiles(['a', 'b', 'c'], 'a'); + await auth.pairSrpProfiles(['c', 'a', 'b'], 'c'); + + expect(mockPairProfiles).toHaveBeenCalledTimes(1); + }); + + it('serves sequential identical calls from cache within the TTL', async () => { + const { auth } = createAuth(); + + await auth.pairSrpProfiles(['a', 'b'], 'a'); + await auth.pairSrpProfiles(['a', 'b'], 'a'); + + expect(mockPairProfiles).toHaveBeenCalledTimes(1); + }); + + it('re-pairs once the cache TTL has expired', async () => { + jest.useFakeTimers(); + try { + const { auth } = createAuth(); + + await auth.pairSrpProfiles(['a', 'b'], 'a'); + // Advance beyond the 30s dedupe window. + jest.setSystemTime(Date.now() + 30_001); + await auth.pairSrpProfiles(['a', 'b'], 'a'); + + expect(mockPairProfiles).toHaveBeenCalledTimes(2); + } finally { + jest.useRealTimers(); + } + }); + + it('does not cache failures so retries re-hit the endpoint', async () => { + const { auth } = createAuth(); + mockPairProfiles + .mockRejectedValueOnce(new Error('pair failed')) + .mockResolvedValueOnce(MOCK_PAIR_RESPONSE); + + await expect(auth.pairSrpProfiles(['a', 'b'], 'a')).rejects.toThrow( + 'pair failed', + ); + const result = await auth.pairSrpProfiles(['a', 'b'], 'a'); + + expect(result).toStrictEqual(MOCK_PAIR_RESPONSE); + expect(mockPairProfiles).toHaveBeenCalledTimes(2); + }); + + it('does not dedupe different token sets', async () => { + const { auth } = createAuth(); + + await auth.pairSrpProfiles(['a', 'b'], 'a'); + await auth.pairSrpProfiles(['a', 'c'], 'a'); + + expect(mockPairProfiles).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts index fa83b36aa1..85a3b13f3a 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts @@ -40,6 +40,10 @@ type JwtBearerAuth_SRP_Options = { }; }; +// How long a successful pairing result stays cached so identical payloads +// (concurrent or sequential retries) reuse it instead of re-hitting the endpoint. +const PAIR_DEDUPE_TTL_MS = 30_000; + const getDefaultEIP6963Provider = async () => { const provider = await getMetaMaskProviderEIP6963(); if (!provider) { @@ -85,6 +89,14 @@ export class SRPJwtBearerAuth implements IBaseAuth { Promise >(); + // Map to dedupe pairing calls by an order-insensitive token-set key. + // Holds the in-flight promise (coalescing concurrent callers) and keeps it + // for a short TTL after success (collapsing sequential retries). + readonly #ongoingPairings = new Map< + string, + { promise: Promise; expiresAt: number } + >(); + // Default cooldown when 429 has no Retry-After header readonly #cooldownDefaultMs: number; @@ -157,7 +169,42 @@ export class SRPJwtBearerAuth implements IBaseAuth { accessTokens: string[], authAccessToken: string, ): Promise { - return await pairProfiles(accessTokens, authAccessToken, this.#config.env); + // Order-insensitive key: the same token set in any order maps to the same + // entry. Sort a copy so the request payload itself stays primary-first. + const key = JSON.stringify([...accessTokens].sort()); + + const cached = this.#ongoingPairings.get(key); + if (cached && cached.expiresAt > Date.now()) { + return await cached.promise; + } + + const promise = pairProfiles( + accessTokens, + authAccessToken, + this.#config.env, + ); + // Store the in-flight promise immediately so concurrent callers coalesce + // regardless of how long the request takes. + this.#ongoingPairings.set(key, { + promise, + expiresAt: Number.MAX_SAFE_INTEGER, + }); + + try { + const result = await promise; + // Keep the resolved result cached for a short window so sequential + // retries with the same payload reuse it. + this.#ongoingPairings.set(key, { + promise, + expiresAt: Date.now() + PAIR_DEDUPE_TTL_MS, + }); + return result; + } catch (error) { + // Never cache failures: the pairing retry loop must be able to re-hit + // the endpoint on the next attempt. + this.#ongoingPairings.delete(key); + throw error; + } } async signMessage( From 3a344093d88f39df0b34941627b51a84d16db34e Mon Sep 17 00:00:00 2001 From: Mathieu Artu Date: Wed, 3 Jun 2026 12:19:27 +0200 Subject: [PATCH 2/2] fix: update CHANGELOG --- packages/profile-sync-controller/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/profile-sync-controller/CHANGELOG.md b/packages/profile-sync-controller/CHANGELOG.md index 2cc3073140..bc405e504e 100644 --- a/packages/profile-sync-controller/CHANGELOG.md +++ b/packages/profile-sync-controller/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Deduplicate SRP profile pairing calls by payload in `SRPJwtBearerAuth.pairSrpProfiles` +- Deduplicate SRP profile pairing calls by payload in `SRPJwtBearerAuth.pairSrpProfiles` ([#8984](https://github.com/MetaMask/core/pull/8984)) - Concurrent and short-interval sequential `performSignIn` triggers (multiple UI surfaces, unlock-time effects, pairing retries) previously each issued their own `POST /api/v2/profile/pair`. A promise cache keyed by the token set now coalesces in-flight calls and reuses a successful result for a short TTL, collapsing the redundant calls into one. The key is order-insensitive (the same token set in any order dedupes), and failures are never cached so the existing `needsProfilePairing` retry loop still re-hits the endpoint. - Scope the storage-key and snap-signature caches in `UserStorageController` per `entropySourceId` ([#8948](https://github.com/MetaMask/core/pull/8948)) - In the edge case where two SRPs resolve to the same `profileId` (e.g. a shared canonical profile after pairing), the previously message-keyed caches could hand one SRP the other's cached key — letting it read/decrypt the other SRP's user storage (which surfaced as a second SRP inheriting the first SRP's account names) or write under its key. Scoping by `entropySourceId` keeps each SRP's key isolated even then; the signed `metamask:${profileId}` message is unchanged, so existing storage keys are preserved.