From c7c442360ba5fcce54de094f125a6b53d29b8756 Mon Sep 17 00:00:00 2001 From: luchobonatti Date: Thu, 9 Apr 2026 12:24:03 -0300 Subject: [PATCH] fix: bound keyurl caches with FIFO eviction to prevent memory leak Replace unbounded Record/Map caches in networkV1.ts and AbstractRelayerProvider.ts with Map bounded to MAX_KEYURL_CACHE_SIZE=16 entries. FIFO eviction via Map insertion order prevents indefinite growth in long-running server processes. Closes #336 --- .../AbstractRelayerProvider.ts | 16 +++- src/relayer-provider/constants.ts | 3 + src/relayer-provider/v1/networkV1.test.ts | 69 ++++++++++++++- src/relayer-provider/v1/networkV1.ts | 20 ++++- .../v2/RelayerV2Provider_keyurl.test.ts | 83 +++++++++++++++++++ 5 files changed, 185 insertions(+), 6 deletions(-) create mode 100644 src/relayer-provider/constants.ts diff --git a/src/relayer-provider/AbstractRelayerProvider.ts b/src/relayer-provider/AbstractRelayerProvider.ts index 2a091ae4..6745d92b 100644 --- a/src/relayer-provider/AbstractRelayerProvider.ts +++ b/src/relayer-provider/AbstractRelayerProvider.ts @@ -38,6 +38,7 @@ import { } from '../relayer/error'; import { setAuth } from './auth/auth'; import { TFHEPkeParams } from '@sdk/lowlevel/TFHEPkeParams'; +import { MAX_KEYURL_CACHE_SIZE } from './constants'; import { FhevmHandle } from '@sdk/FhevmHandle'; import { assertIsRelayerGetResponseKeyUrlCamelCase, @@ -49,7 +50,7 @@ import { uintToHex } from '@base/uint'; //////////////////////////////////////////////////////////////////////////////// // Cache promises to avoid race conditions when multiple concurrent calls -// are made before the first one completes +// are made before the first one completes. const privateKeyurlCache = new Map>(); /** @@ -99,10 +100,23 @@ export abstract class AbstractRelayerProvider { return cached; } + // Evict oldest entry when at capacity. Track it so we can restore on failure — + // eviction should only take effect when the new fetch succeeds. + let evicted: [string, Promise] | null = null; + if (privateKeyurlCache.size >= MAX_KEYURL_CACHE_SIZE) { + const oldestKey = privateKeyurlCache.keys().next().value!; + evicted = [oldestKey, privateKeyurlCache.get(oldestKey)!]; + privateKeyurlCache.delete(oldestKey); + } + // Create and cache the promise immediately to prevent race conditions const promise = this._fetchTFHEPkeParamsImpl().catch((err: unknown) => { // Remove from cache on failure so subsequent calls can retry privateKeyurlCache.delete(this.#relayerUrl); + // Restore the evicted entry — a failed fetch should not shrink the cache + if (evicted !== null) { + privateKeyurlCache.set(evicted[0], evicted[1]); + } throw err; }); diff --git a/src/relayer-provider/constants.ts b/src/relayer-provider/constants.ts new file mode 100644 index 00000000..8cb8c02d --- /dev/null +++ b/src/relayer-provider/constants.ts @@ -0,0 +1,3 @@ +// 16 covers all realistic deployment configurations (1–10 chains) while bounding +// worst-case memory growth in long-running server processes. +export const MAX_KEYURL_CACHE_SIZE = 16; diff --git a/src/relayer-provider/v1/networkV1.test.ts b/src/relayer-provider/v1/networkV1.test.ts index fe965784..8cd6efa1 100644 --- a/src/relayer-provider/v1/networkV1.test.ts +++ b/src/relayer-provider/v1/networkV1.test.ts @@ -1,5 +1,5 @@ import type { RelayerGetResponseKeyUrlSnakeCase } from '../types/private'; -import { getKeysFromRelayer } from './networkV1'; +import { getKeysFromRelayer, _clearKeyurlCache } from './networkV1'; import { tfheCompactPkeCrsBytes, tfheCompactPublicKeyBytes } from '../../test'; import { SERIALIZED_SIZE_LIMIT_PK } from '../../sdk/lowlevel/constants'; import fetchMock from 'fetch-mock'; @@ -134,9 +134,22 @@ const payload: RelayerGetResponseKeyUrlSnakeCase = { const describeIfFetchMock = TEST_CONFIG.type === 'fetch-mock' ? describe : describe.skip; +const pubKeyUrl = payload.response.fhe_key_info[0].fhe_public_key.urls[0]; +const crsUrl = payload.response.crs['2048'].urls[0]; + //////////////////////////////////////////////////////////////////////////////// describeIfFetchMock('network', () => { + beforeEach(() => { + _clearKeyurlCache(); + fetchMock.removeRoutes(); + }); + + afterEach(() => { + _clearKeyurlCache(); + fetchMock.removeRoutes(); + }); + it('getKeysFromRelayer', async () => { fetchMock.get('https://test-relayer.net/v1/keyurl', payload); @@ -156,4 +169,58 @@ describeIfFetchMock('network', () => { material.publicKey.safe_serialize(SERIALIZED_SIZE_LIMIT_PK), ).toStrictEqual(tfheCompactPublicKeyBytes); }); + + it('cache hit: second call returns same object, /keyurl called once', async () => { + let keyurlCallCount = 0; + fetchMock.get(TEST_CONFIG.v1.urls.keyUrl, () => { + keyurlCallCount++; + return payload; + }); + fetchMock.get(pubKeyUrl, tfheCompactPublicKeyBytes); + fetchMock.get(crsUrl, tfheCompactPkeCrsBytes); + + const r1 = await getKeysFromRelayer(TEST_CONFIG.v1.urls.base); + const r2 = await getKeysFromRelayer(TEST_CONFIG.v1.urls.base); + + expect(r1).toBe(r2); + expect(keyurlCallCount).toBe(1); + }); + + it('FIFO eviction: 17th URL evicts url-00', async () => { + const keyurlCallCounts: number[] = new Array(17).fill(0) as number[]; + + // Asset mocks registered once — all 17 payloads reference the same asset URLs + fetchMock.get(pubKeyUrl, tfheCompactPublicKeyBytes); + fetchMock.get(crsUrl, tfheCompactPkeCrsBytes); + + // Register 17 unique keyurl mocks + for (let n = 0; n < 17; n++) { + const idx = n; + fetchMock.get( + `https://test-relayer.net/url-${String(idx).padStart(2, '0')}/keyurl`, + () => { + keyurlCallCounts[idx]++; + return payload; + }, + ); + } + + // Fill cache with url-00..url-15 (16 entries) + for (let n = 0; n < 16; n++) { + await getKeysFromRelayer( + `https://test-relayer.net/url-${String(n).padStart(2, '0')}`, + ); + } + + // Insert url-16 — evicts url-00 + await getKeysFromRelayer('https://test-relayer.net/url-16'); + + // url-01 is still in cache (only url-00 was evicted) + await getKeysFromRelayer('https://test-relayer.net/url-01'); + expect(keyurlCallCounts[1]).toBe(1); + + // Re-fetch url-00 — was evicted, must hit network again + await getKeysFromRelayer('https://test-relayer.net/url-00'); + expect(keyurlCallCounts[0]).toBe(2); + }); }); diff --git a/src/relayer-provider/v1/networkV1.ts b/src/relayer-provider/v1/networkV1.ts index 1904db1b..e38c4cbd 100644 --- a/src/relayer-provider/v1/networkV1.ts +++ b/src/relayer-provider/v1/networkV1.ts @@ -11,6 +11,7 @@ import { } from '@sdk/lowlevel/constants'; import { fetchRelayerV1Get } from './fetchRelayerV1'; import { isNonEmptyString, removeSuffix } from '@base/string'; +import { MAX_KEYURL_CACHE_SIZE } from '../constants'; // eslint-disable-next-line @typescript-eslint/consistent-type-definitions type CachedKey = { @@ -26,7 +27,15 @@ type CachedKey = { //////////////////////////////////////////////////////////////////////////////// -const keyurlCache: Record = {}; +const keyurlCache = new Map(); + +/** + * Clears the keyurl cache. Exported for testing purposes only. + * @internal + */ +export function _clearKeyurlCache(): void { + keyurlCache.clear(); +} //////////////////////////////////////////////////////////////////////////////// @@ -35,8 +44,8 @@ export async function getKeysFromRelayer( publicKeyId?: string | null, options?: FhevmInstanceOptions, ): Promise { - if (versionUrl in keyurlCache) { - return keyurlCache[versionUrl]; + if (keyurlCache.has(versionUrl)) { + return keyurlCache.get(versionUrl)!; } const data: RelayerGetResponseKeyUrlSnakeCase = (await fetchRelayerV1Get( @@ -139,7 +148,10 @@ export async function getKeysFromRelayer( }, }, }; - keyurlCache[versionUrl] = result; + if (keyurlCache.size >= MAX_KEYURL_CACHE_SIZE) { + keyurlCache.delete(keyurlCache.keys().next().value!); + } + keyurlCache.set(versionUrl, result); return result; } catch (e) { throw new Error('Impossible to fetch public key: wrong relayer url.', { diff --git a/src/relayer-provider/v2/RelayerV2Provider_keyurl.test.ts b/src/relayer-provider/v2/RelayerV2Provider_keyurl.test.ts index 68f2c777..ac06e35b 100644 --- a/src/relayer-provider/v2/RelayerV2Provider_keyurl.test.ts +++ b/src/relayer-provider/v2/RelayerV2Provider_keyurl.test.ts @@ -441,6 +441,89 @@ describeIfFetchMock('RelayerV2Provider - TFHEPkeParams Caching', () => { expect(mockTFHEPkeParamsFetch).toHaveBeenCalledTimes(1); }); + it('FIFO eviction: 17th provider evicts first provider cache entry', async () => { + const BASE_URL = 'https://test-relayer.net/eviction'; + + // Register 17 unique /keyurl mocks + for (let n = 0; n < 17; n++) { + fetchMock.get( + `${BASE_URL}/provider-${String(n).padStart(2, '0')}/keyurl`, + relayerV1ResponseGetKeyUrl, + ); + } + + // Create 17 providers + const providers = Array.from({ length: 17 }, (_, n) => + createRelayerProvider( + `${BASE_URL}/provider-${String(n).padStart(2, '0')}`, + 1, + ), + ); + + // Fetch all 17 — fills the cache to 16 and evicts provider-00 on provider-16 + for (const provider of providers) { + await provider.fetchTFHEPkeParams(); + } + + expect(mockTFHEPkeParamsFetch).toHaveBeenCalledTimes(17); + + // provider-01 is still in cache (only provider-00 was evicted by provider-16) + await providers[1].fetchTFHEPkeParams(); + expect(mockTFHEPkeParamsFetch).toHaveBeenCalledTimes(17); + + // Re-fetch provider-00 — it was evicted, so spy should be called again + await providers[0].fetchTFHEPkeParams(); + expect(mockTFHEPkeParamsFetch).toHaveBeenCalledTimes(18); + }); + + it('failed fetch restores evicted entry — cache size stays at 16', async () => { + const BASE_URL = 'https://test-relayer.net/eviction-restore'; + let provider16CallCount = 0; + + // Register successful mocks for provider-00..provider-15 + for (let n = 0; n < 16; n++) { + fetchMock.get( + `${BASE_URL}/provider-${String(n).padStart(2, '0')}/keyurl`, + relayerV1ResponseGetKeyUrl, + ); + } + // provider-16: first call fails, subsequent calls succeed + fetchMock.get(`${BASE_URL}/provider-16/keyurl`, () => { + provider16CallCount++; + if (provider16CallCount === 1) { + return { status: 500 }; + } + return relayerV1ResponseGetKeyUrl; + }); + + const providers = Array.from({ length: 17 }, (_, n) => + createRelayerProvider( + `${BASE_URL}/provider-${String(n).padStart(2, '0')}`, + 1, + ), + ); + + // Fill cache with 16 entries (provider-00..provider-15) + for (let n = 0; n < 16; n++) { + await providers[n].fetchTFHEPkeParams(); + } + expect(mockTFHEPkeParamsFetch).toHaveBeenCalledTimes(16); + + // Fetch provider-16 — evicts provider-00, but the keyurl request fails. + // The evicted entry must be restored so the cache doesn't silently shrink. + await expect(providers[16].fetchTFHEPkeParams()).rejects.toThrow(); + // TFHEPkeParams.fetch was never called for the failed fetch + expect(mockTFHEPkeParamsFetch).toHaveBeenCalledTimes(16); + + // provider-00 must be restored in cache — cache hit, spy count stays at 16 + await providers[0].fetchTFHEPkeParams(); + expect(mockTFHEPkeParamsFetch).toHaveBeenCalledTimes(16); + + // provider-16 must NOT be in cache — retry triggers a network call + await providers[16].fetchTFHEPkeParams(); + expect(mockTFHEPkeParamsFetch).toHaveBeenCalledTimes(17); + }); + it('caches separately for different relayer URLs', async () => { const testRelayerUrlV2 = TEST_CONFIG.v2.fhevmInstanceConfig.relayerUrl; let fetchCount1 = 0;