Skip to content

Commit c7c4423

Browse files
committed
fix: bound keyurl caches with FIFO eviction to prevent memory leak
Replace unbounded Record/Map caches in networkV1.ts and AbstractRelayerProvider.ts with Map<string, CachedKey> bounded to MAX_KEYURL_CACHE_SIZE=16 entries. FIFO eviction via Map insertion order prevents indefinite growth in long-running server processes. Closes zama-ai#336
1 parent e833b38 commit c7c4423

5 files changed

Lines changed: 185 additions & 6 deletions

File tree

src/relayer-provider/AbstractRelayerProvider.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
} from '../relayer/error';
3939
import { setAuth } from './auth/auth';
4040
import { TFHEPkeParams } from '@sdk/lowlevel/TFHEPkeParams';
41+
import { MAX_KEYURL_CACHE_SIZE } from './constants';
4142
import { FhevmHandle } from '@sdk/FhevmHandle';
4243
import {
4344
assertIsRelayerGetResponseKeyUrlCamelCase,
@@ -49,7 +50,7 @@ import { uintToHex } from '@base/uint';
4950
////////////////////////////////////////////////////////////////////////////////
5051

5152
// Cache promises to avoid race conditions when multiple concurrent calls
52-
// are made before the first one completes
53+
// are made before the first one completes.
5354
const privateKeyurlCache = new Map<string, Promise<TFHEPkeParams>>();
5455

5556
/**
@@ -99,10 +100,23 @@ export abstract class AbstractRelayerProvider {
99100
return cached;
100101
}
101102

103+
// Evict oldest entry when at capacity. Track it so we can restore on failure —
104+
// eviction should only take effect when the new fetch succeeds.
105+
let evicted: [string, Promise<TFHEPkeParams>] | null = null;
106+
if (privateKeyurlCache.size >= MAX_KEYURL_CACHE_SIZE) {
107+
const oldestKey = privateKeyurlCache.keys().next().value!;
108+
evicted = [oldestKey, privateKeyurlCache.get(oldestKey)!];
109+
privateKeyurlCache.delete(oldestKey);
110+
}
111+
102112
// Create and cache the promise immediately to prevent race conditions
103113
const promise = this._fetchTFHEPkeParamsImpl().catch((err: unknown) => {
104114
// Remove from cache on failure so subsequent calls can retry
105115
privateKeyurlCache.delete(this.#relayerUrl);
116+
// Restore the evicted entry — a failed fetch should not shrink the cache
117+
if (evicted !== null) {
118+
privateKeyurlCache.set(evicted[0], evicted[1]);
119+
}
106120
throw err;
107121
});
108122

src/relayer-provider/constants.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// 16 covers all realistic deployment configurations (1–10 chains) while bounding
2+
// worst-case memory growth in long-running server processes.
3+
export const MAX_KEYURL_CACHE_SIZE = 16;

src/relayer-provider/v1/networkV1.test.ts

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { RelayerGetResponseKeyUrlSnakeCase } from '../types/private';
2-
import { getKeysFromRelayer } from './networkV1';
2+
import { getKeysFromRelayer, _clearKeyurlCache } from './networkV1';
33
import { tfheCompactPkeCrsBytes, tfheCompactPublicKeyBytes } from '../../test';
44
import { SERIALIZED_SIZE_LIMIT_PK } from '../../sdk/lowlevel/constants';
55
import fetchMock from 'fetch-mock';
@@ -134,9 +134,22 @@ const payload: RelayerGetResponseKeyUrlSnakeCase = {
134134
const describeIfFetchMock =
135135
TEST_CONFIG.type === 'fetch-mock' ? describe : describe.skip;
136136

137+
const pubKeyUrl = payload.response.fhe_key_info[0].fhe_public_key.urls[0];
138+
const crsUrl = payload.response.crs['2048'].urls[0];
139+
137140
////////////////////////////////////////////////////////////////////////////////
138141

139142
describeIfFetchMock('network', () => {
143+
beforeEach(() => {
144+
_clearKeyurlCache();
145+
fetchMock.removeRoutes();
146+
});
147+
148+
afterEach(() => {
149+
_clearKeyurlCache();
150+
fetchMock.removeRoutes();
151+
});
152+
140153
it('getKeysFromRelayer', async () => {
141154
fetchMock.get('https://test-relayer.net/v1/keyurl', payload);
142155

@@ -156,4 +169,58 @@ describeIfFetchMock('network', () => {
156169
material.publicKey.safe_serialize(SERIALIZED_SIZE_LIMIT_PK),
157170
).toStrictEqual(tfheCompactPublicKeyBytes);
158171
});
172+
173+
it('cache hit: second call returns same object, /keyurl called once', async () => {
174+
let keyurlCallCount = 0;
175+
fetchMock.get(TEST_CONFIG.v1.urls.keyUrl, () => {
176+
keyurlCallCount++;
177+
return payload;
178+
});
179+
fetchMock.get(pubKeyUrl, tfheCompactPublicKeyBytes);
180+
fetchMock.get(crsUrl, tfheCompactPkeCrsBytes);
181+
182+
const r1 = await getKeysFromRelayer(TEST_CONFIG.v1.urls.base);
183+
const r2 = await getKeysFromRelayer(TEST_CONFIG.v1.urls.base);
184+
185+
expect(r1).toBe(r2);
186+
expect(keyurlCallCount).toBe(1);
187+
});
188+
189+
it('FIFO eviction: 17th URL evicts url-00', async () => {
190+
const keyurlCallCounts: number[] = new Array(17).fill(0) as number[];
191+
192+
// Asset mocks registered once — all 17 payloads reference the same asset URLs
193+
fetchMock.get(pubKeyUrl, tfheCompactPublicKeyBytes);
194+
fetchMock.get(crsUrl, tfheCompactPkeCrsBytes);
195+
196+
// Register 17 unique keyurl mocks
197+
for (let n = 0; n < 17; n++) {
198+
const idx = n;
199+
fetchMock.get(
200+
`https://test-relayer.net/url-${String(idx).padStart(2, '0')}/keyurl`,
201+
() => {
202+
keyurlCallCounts[idx]++;
203+
return payload;
204+
},
205+
);
206+
}
207+
208+
// Fill cache with url-00..url-15 (16 entries)
209+
for (let n = 0; n < 16; n++) {
210+
await getKeysFromRelayer(
211+
`https://test-relayer.net/url-${String(n).padStart(2, '0')}`,
212+
);
213+
}
214+
215+
// Insert url-16 — evicts url-00
216+
await getKeysFromRelayer('https://test-relayer.net/url-16');
217+
218+
// url-01 is still in cache (only url-00 was evicted)
219+
await getKeysFromRelayer('https://test-relayer.net/url-01');
220+
expect(keyurlCallCounts[1]).toBe(1);
221+
222+
// Re-fetch url-00 — was evicted, must hit network again
223+
await getKeysFromRelayer('https://test-relayer.net/url-00');
224+
expect(keyurlCallCounts[0]).toBe(2);
225+
});
159226
});

src/relayer-provider/v1/networkV1.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
} from '@sdk/lowlevel/constants';
1212
import { fetchRelayerV1Get } from './fetchRelayerV1';
1313
import { isNonEmptyString, removeSuffix } from '@base/string';
14+
import { MAX_KEYURL_CACHE_SIZE } from '../constants';
1415

1516
// eslint-disable-next-line @typescript-eslint/consistent-type-definitions
1617
type CachedKey = {
@@ -26,7 +27,15 @@ type CachedKey = {
2627

2728
////////////////////////////////////////////////////////////////////////////////
2829

29-
const keyurlCache: Record<string, CachedKey> = {};
30+
const keyurlCache = new Map<string, CachedKey>();
31+
32+
/**
33+
* Clears the keyurl cache. Exported for testing purposes only.
34+
* @internal
35+
*/
36+
export function _clearKeyurlCache(): void {
37+
keyurlCache.clear();
38+
}
3039

3140
////////////////////////////////////////////////////////////////////////////////
3241

@@ -35,8 +44,8 @@ export async function getKeysFromRelayer(
3544
publicKeyId?: string | null,
3645
options?: FhevmInstanceOptions,
3746
): Promise<CachedKey> {
38-
if (versionUrl in keyurlCache) {
39-
return keyurlCache[versionUrl];
47+
if (keyurlCache.has(versionUrl)) {
48+
return keyurlCache.get(versionUrl)!;
4049
}
4150

4251
const data: RelayerGetResponseKeyUrlSnakeCase = (await fetchRelayerV1Get(
@@ -139,7 +148,10 @@ export async function getKeysFromRelayer(
139148
},
140149
},
141150
};
142-
keyurlCache[versionUrl] = result;
151+
if (keyurlCache.size >= MAX_KEYURL_CACHE_SIZE) {
152+
keyurlCache.delete(keyurlCache.keys().next().value!);
153+
}
154+
keyurlCache.set(versionUrl, result);
143155
return result;
144156
} catch (e) {
145157
throw new Error('Impossible to fetch public key: wrong relayer url.', {

src/relayer-provider/v2/RelayerV2Provider_keyurl.test.ts

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -441,6 +441,89 @@ describeIfFetchMock('RelayerV2Provider - TFHEPkeParams Caching', () => {
441441
expect(mockTFHEPkeParamsFetch).toHaveBeenCalledTimes(1);
442442
});
443443

444+
it('FIFO eviction: 17th provider evicts first provider cache entry', async () => {
445+
const BASE_URL = 'https://test-relayer.net/eviction';
446+
447+
// Register 17 unique /keyurl mocks
448+
for (let n = 0; n < 17; n++) {
449+
fetchMock.get(
450+
`${BASE_URL}/provider-${String(n).padStart(2, '0')}/keyurl`,
451+
relayerV1ResponseGetKeyUrl,
452+
);
453+
}
454+
455+
// Create 17 providers
456+
const providers = Array.from({ length: 17 }, (_, n) =>
457+
createRelayerProvider(
458+
`${BASE_URL}/provider-${String(n).padStart(2, '0')}`,
459+
1,
460+
),
461+
);
462+
463+
// Fetch all 17 — fills the cache to 16 and evicts provider-00 on provider-16
464+
for (const provider of providers) {
465+
await provider.fetchTFHEPkeParams();
466+
}
467+
468+
expect(mockTFHEPkeParamsFetch).toHaveBeenCalledTimes(17);
469+
470+
// provider-01 is still in cache (only provider-00 was evicted by provider-16)
471+
await providers[1].fetchTFHEPkeParams();
472+
expect(mockTFHEPkeParamsFetch).toHaveBeenCalledTimes(17);
473+
474+
// Re-fetch provider-00 — it was evicted, so spy should be called again
475+
await providers[0].fetchTFHEPkeParams();
476+
expect(mockTFHEPkeParamsFetch).toHaveBeenCalledTimes(18);
477+
});
478+
479+
it('failed fetch restores evicted entry — cache size stays at 16', async () => {
480+
const BASE_URL = 'https://test-relayer.net/eviction-restore';
481+
let provider16CallCount = 0;
482+
483+
// Register successful mocks for provider-00..provider-15
484+
for (let n = 0; n < 16; n++) {
485+
fetchMock.get(
486+
`${BASE_URL}/provider-${String(n).padStart(2, '0')}/keyurl`,
487+
relayerV1ResponseGetKeyUrl,
488+
);
489+
}
490+
// provider-16: first call fails, subsequent calls succeed
491+
fetchMock.get(`${BASE_URL}/provider-16/keyurl`, () => {
492+
provider16CallCount++;
493+
if (provider16CallCount === 1) {
494+
return { status: 500 };
495+
}
496+
return relayerV1ResponseGetKeyUrl;
497+
});
498+
499+
const providers = Array.from({ length: 17 }, (_, n) =>
500+
createRelayerProvider(
501+
`${BASE_URL}/provider-${String(n).padStart(2, '0')}`,
502+
1,
503+
),
504+
);
505+
506+
// Fill cache with 16 entries (provider-00..provider-15)
507+
for (let n = 0; n < 16; n++) {
508+
await providers[n].fetchTFHEPkeParams();
509+
}
510+
expect(mockTFHEPkeParamsFetch).toHaveBeenCalledTimes(16);
511+
512+
// Fetch provider-16 — evicts provider-00, but the keyurl request fails.
513+
// The evicted entry must be restored so the cache doesn't silently shrink.
514+
await expect(providers[16].fetchTFHEPkeParams()).rejects.toThrow();
515+
// TFHEPkeParams.fetch was never called for the failed fetch
516+
expect(mockTFHEPkeParamsFetch).toHaveBeenCalledTimes(16);
517+
518+
// provider-00 must be restored in cache — cache hit, spy count stays at 16
519+
await providers[0].fetchTFHEPkeParams();
520+
expect(mockTFHEPkeParamsFetch).toHaveBeenCalledTimes(16);
521+
522+
// provider-16 must NOT be in cache — retry triggers a network call
523+
await providers[16].fetchTFHEPkeParams();
524+
expect(mockTFHEPkeParamsFetch).toHaveBeenCalledTimes(17);
525+
});
526+
444527
it('caches separately for different relayer URLs', async () => {
445528
const testRelayerUrlV2 = TEST_CONFIG.v2.fhevmInstanceConfig.relayerUrl;
446529
let fetchCount1 = 0;

0 commit comments

Comments
 (0)