From fe5495a4ce0f5a096d14f1e148ea3790ca06b4aa Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Fri, 10 Apr 2026 15:09:29 +0200 Subject: [PATCH 1/6] feat(ocap-kernel): bound relay hints in OCAP URLs and relay pool size Cap relay hints embedded in OCAP URLs to 3 (MAX_URL_RELAY_HINTS) with priority selection (bootstrap first, then most recently seen). Bound the relay pool to 20 entries (MAX_KNOWN_RELAYS) with eviction of oldest non-bootstrap entries. This prevents unbounded URL growth as kernels exchange relay hints across the network. Closes #889 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/remotes/kernel/remote-comms.test.ts | 170 ++++++++++++++++-- .../src/remotes/kernel/remote-comms.ts | 90 ++++++++-- packages/ocap-kernel/src/store/index.test.ts | 52 ++++-- packages/ocap-kernel/src/store/index.ts | 60 ++++++- packages/ocap-kernel/src/store/types.ts | 12 ++ 5 files changed, 336 insertions(+), 48 deletions(-) diff --git a/packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts b/packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts index 3749898382..c669965345 100644 --- a/packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts +++ b/packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts @@ -8,15 +8,31 @@ import { initRemoteIdentity, initRemoteComms, parseOcapURL, + MAX_URL_RELAY_HINTS, + MAX_KNOWN_RELAYS, } from './remote-comms.ts'; import { createMockRemotesFactory } from '../../../test/remotes-mocks.ts'; import { makeMapKernelDatabase } from '../../../test/storage.ts'; -import type { KernelStore } from '../../store/index.ts'; +import type { KernelStore, RelayEntry } from '../../store/index.ts'; import { makeKernelStore } from '../../store/index.ts'; import type { KRef, PlatformServices } from '../../types.ts'; import { mnemonicToSeed } from '../../utils/bip39.ts'; import type { RemoteMessageHandler } from '../types.ts'; +/** + * Build learned (non-bootstrap) relay entries from address strings. + * + * @param addrs - Relay multiaddr strings. + * @param lastSeen - Epoch ms for lastSeen (default 100). + * @returns Relay entries with isBootstrap: false. + */ +function makeLearnedRelayEntries( + addrs: string[], + lastSeen = 100, +): RelayEntry[] { + return addrs.map((addr) => ({ addr, lastSeen, isBootstrap: false })); +} + describe('remote-comms', () => { let mockKernelStore: KernelStore; let mockPlatformServices: PlatformServices; @@ -80,11 +96,11 @@ describe('remote-comms', () => { expect(remoteComms.getPeerId()).toBe(peerId); const ocapURL = await remoteComms.issueOcapURL('ko1' as KRef); - const { oid } = parseOcapURL(ocapURL); - const knownRelays = mockKernelStore.getKnownRelays(); - expect(Array.isArray(knownRelays)).toBe(true); - expect(knownRelays.length).toBeGreaterThan(0); - expect(knownRelays).toStrictEqual(testRelays); + const { oid, hints } = parseOcapURL(ocapURL); + const knownRelayAddresses = mockKernelStore.getKnownRelayAddresses(); + expect(knownRelayAddresses).toStrictEqual(testRelays); + // URL should embed the relays (within MAX_URL_RELAY_HINTS cap) + expect(hints).toStrictEqual(testRelays); const referenceURL = `ocap:${oid}@${peerId},${testRelays.join(',')}`; expect(ocapURL).toBe(referenceURL); @@ -157,12 +173,12 @@ describe('remote-comms', () => { ); }); - it('uses getKnownRelays when options.relays is empty', async () => { + it('uses stored relays when options.relays is empty', async () => { const storedRelays = [ '/dns4/stored-relay1.example/tcp/443/wss/p2p/relay1', '/dns4/stored-relay2.example/tcp/443/wss/p2p/relay2', ]; - mockKernelStore.setKnownRelays(storedRelays); + mockKernelStore.setRelayEntries(makeLearnedRelayEntries(storedRelays)); await initRemoteComms( mockKernelStore, mockPlatformServices, @@ -339,12 +355,14 @@ describe('remote-comms', () => { mockRemoteMessageHandler, { relays: testRelays }, ); - expect(mockKernelStore.getKnownRelays()).toStrictEqual(testRelays); + expect(mockKernelStore.getKnownRelayAddresses()).toStrictEqual( + testRelays, + ); }); it('does not save relays to KV store when empty', async () => { const storedRelays = ['/dns4/stored-relay.example/tcp/443/wss/p2p/relay']; - mockKernelStore.setKnownRelays(storedRelays); + mockKernelStore.setRelayEntries(makeLearnedRelayEntries(storedRelays)); await initRemoteComms( mockKernelStore, mockPlatformServices, @@ -352,7 +370,9 @@ describe('remote-comms', () => { {}, // empty relays ); // Should not overwrite existing relays - expect(mockKernelStore.getKnownRelays()).toStrictEqual(storedRelays); + expect(mockKernelStore.getKnownRelayAddresses()).toStrictEqual( + storedRelays, + ); }); }); @@ -444,7 +464,7 @@ describe('remote-comms', () => { '/dns4/relay1.example/tcp/443/wss/p2p-circuit', // duplicate ]); - const stored = mockKernelStore.getKnownRelays(); + const stored = mockKernelStore.getKnownRelayAddresses(); expect(stored).toStrictEqual([ '/dns4/relay1.example/tcp/443/wss/p2p-circuit', '/dns4/relay2.example/tcp/443/wss/p2p-circuit', @@ -475,10 +495,132 @@ describe('remote-comms', () => { identity.addKnownRelays([]); - const stored = mockKernelStore.getKnownRelays(); + const stored = mockKernelStore.getKnownRelayAddresses(); expect(stored).toStrictEqual(initialRelays); }); + it('issueOcapURL caps relay hints to MAX_URL_RELAY_HINTS', async () => { + const relays = Array.from( + { length: MAX_URL_RELAY_HINTS + 3 }, + (_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`, + ); + const { identity } = await initRemoteIdentity(mockKernelStore, { + relays, + }); + + const ocapURL = await identity.issueOcapURL('ko1' as KRef); + const { hints } = parseOcapURL(ocapURL); + expect(hints).toHaveLength(MAX_URL_RELAY_HINTS); + }); + + it('issueOcapURL prefers bootstrap relays over learned relays', async () => { + const bootstrapRelays = [ + '/dns4/bootstrap1.example/tcp/443/wss/p2p-circuit', + '/dns4/bootstrap2.example/tcp/443/wss/p2p-circuit', + ]; + const { identity } = await initRemoteIdentity(mockKernelStore, { + relays: bootstrapRelays, + }); + + // Add many learned relays (more recent lastSeen) + const learnedRelays = Array.from( + { length: 5 }, + (_, i) => `/dns4/learned${i}.example/tcp/443/wss/p2p-circuit`, + ); + identity.addKnownRelays(learnedRelays); + + const ocapURL = await identity.issueOcapURL('ko1' as KRef); + const { hints } = parseOcapURL(ocapURL); + expect(hints).toHaveLength(MAX_URL_RELAY_HINTS); + // Bootstrap relays should be included ahead of learned relays + for (const bootstrap of bootstrapRelays) { + expect(hints).toContain(bootstrap); + } + }); + + it('addKnownRelays enforces MAX_KNOWN_RELAYS pool cap', async () => { + const { identity } = await initRemoteIdentity(mockKernelStore); + + // Add more relays than the cap + const relays = Array.from( + { length: MAX_KNOWN_RELAYS + 5 }, + (_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`, + ); + identity.addKnownRelays(relays); + + const entries = mockKernelStore.getRelayEntries(); + expect(entries).toHaveLength(MAX_KNOWN_RELAYS); + }); + + it('addKnownRelays evicts oldest non-bootstrap relays when pool is full', async () => { + const bootstrapRelays = [ + '/dns4/bootstrap.example/tcp/443/wss/p2p-circuit', + ]; + const { identity } = await initRemoteIdentity(mockKernelStore, { + relays: bootstrapRelays, + }); + + // Fill pool to the cap + const fillerRelays = Array.from( + { length: MAX_KNOWN_RELAYS }, + (_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`, + ); + identity.addKnownRelays(fillerRelays); + + const entries = mockKernelStore.getRelayEntries(); + expect(entries).toHaveLength(MAX_KNOWN_RELAYS); + // Bootstrap relay must survive eviction + expect(entries.some((entry) => entry.addr === bootstrapRelays[0])).toBe( + true, + ); + }); + + it('addKnownRelays updates lastSeen on re-observed relays', async () => { + const { identity } = await initRemoteIdentity(mockKernelStore); + + identity.addKnownRelays(['/dns4/relay.example/tcp/443/wss/p2p-circuit']); + const firstSeen = mockKernelStore.getRelayEntries()[0]?.lastSeen ?? 0; + + // SES lockdown prevents mocking Date.now; use a real delay instead + await new Promise((resolve) => { + setTimeout(resolve, 50); + }); + identity.addKnownRelays(['/dns4/relay.example/tcp/443/wss/p2p-circuit']); + const secondSeen = mockKernelStore.getRelayEntries()[0]?.lastSeen ?? 0; + + expect(secondSeen).toBeGreaterThan(firstSeen); + }); + + it('init marks bootstrap relays and preserves learned relays', async () => { + // Pre-seed a learned relay + mockKernelStore.setRelayEntries([ + { + addr: '/dns4/learned.example/tcp/443/wss/p2p-circuit', + lastSeen: 100, + isBootstrap: false, + }, + ]); + + const bootstrapRelays = [ + '/dns4/bootstrap.example/tcp/443/wss/p2p-circuit', + ]; + await initRemoteIdentity(mockKernelStore, { + relays: bootstrapRelays, + }); + + const entries = mockKernelStore.getRelayEntries(); + expect(entries).toHaveLength(2); + expect( + entries.find((entry) => entry.addr === bootstrapRelays[0])?.isBootstrap, + ).toBe(true); + expect( + entries.find( + (entry) => + entry.addr === '/dns4/learned.example/tcp/443/wss/p2p-circuit', + )?.isBootstrap, + ).toBe(false); + }); + it('throws with mnemonic when identity already exists', async () => { mockKernelStore.setRemoteIdentityValue('peerId', 'existing-peer-id'); mockKernelStore.setRemoteIdentityValue( @@ -690,7 +832,7 @@ describe('remote-comms', () => { '/dns4/stored-relay1.example/tcp/443/wss/p2p/relay1', '/dns4/stored-relay2.example/tcp/443/wss/p2p/relay2', ]; - mockKernelStore.setKnownRelays(storedRelays); + mockKernelStore.setRelayEntries(makeLearnedRelayEntries(storedRelays)); const remoteComms = await initRemoteComms( mockKernelStore, mockPlatformServices, diff --git a/packages/ocap-kernel/src/remotes/kernel/remote-comms.ts b/packages/ocap-kernel/src/remotes/kernel/remote-comms.ts index f5a962665e..7de4981bb2 100644 --- a/packages/ocap-kernel/src/remotes/kernel/remote-comms.ts +++ b/packages/ocap-kernel/src/remotes/kernel/remote-comms.ts @@ -24,6 +24,12 @@ export type OcapURLParts = { hints: string[]; }; +/** Maximum number of relay hints embedded in a single OCAP URL. */ +export const MAX_URL_RELAY_HINTS = 3; + +/** Maximum number of relay entries stored in the kernel's relay pool. */ +export const MAX_KNOWN_RELAYS = 20; + /** * Break down an ocap URL string into its constituent parts. * @@ -109,7 +115,30 @@ export async function initRemoteIdentity( const relays = options?.relays ?? []; const mnemonic = options?.mnemonic; if (relays.length > 0) { - kernelStore.setKnownRelays(relays); + const now = Date.now(); + const bootstrapSet = new Set(relays); + // Merge with existing entries: mark bootstrap relays, preserve learned ones + const existing = kernelStore.getRelayEntries(); + const byAddr = new Map(existing.map((entry) => [entry.addr, entry])); + for (const addr of relays) { + byAddr.set(addr, { addr, lastSeen: now, isBootstrap: true }); + } + // Clear bootstrap flag on entries no longer in the current bootstrap set + for (const entry of byAddr.values()) { + if (entry.isBootstrap && !bootstrapSet.has(entry.addr)) { + entry.isBootstrap = false; + } + } + let merged = [...byAddr.values()]; + // Enforce pool cap: keep all bootstrap, then newest non-bootstrap + if (merged.length > MAX_KNOWN_RELAYS) { + const bootstrap = merged.filter((entry) => entry.isBootstrap); + const nonBootstrap = merged + .filter((entry) => !entry.isBootstrap) + .sort((a, b) => b.lastSeen - a.lastSeen); + merged = [...bootstrap, ...nonBootstrap].slice(0, MAX_KNOWN_RELAYS); + } + kernelStore.setRelayEntries(merged); } /* eslint-disable no-param-reassign */ @@ -171,15 +200,26 @@ export async function initRemoteIdentity( const encodedKref = encoder.encode(paddedKref); const rawOid = await cipher.encrypt(encodedKref, ocapURLKey); const oid = base58btc.encode(rawOid); - const currentRelays = kernelStore.getKnownRelays(); - const relaySuffix = - currentRelays.length > 0 ? `,${currentRelays.join(',')}` : ''; + const entries = kernelStore.getRelayEntries(); + // Select top relays: bootstrap first, then most recently seen + const sorted = [...entries].sort((a, b) => { + if (a.isBootstrap !== b.isBootstrap) { + return a.isBootstrap ? -1 : 1; + } + return b.lastSeen - a.lastSeen; + }); + const selected = sorted + .slice(0, MAX_URL_RELAY_HINTS) + .map((entry) => entry.addr); + const relaySuffix = selected.length > 0 ? `,${selected.join(',')}` : ''; const ocapURL = `ocap:${oid}@${peerId}${relaySuffix}`; return ocapURL; } /** - * Add relay addresses to the kernel's known relay pool, deduplicating. + * Add relay addresses to the kernel's known relay pool. + * Deduplicates, updates lastSeen on re-observation, and enforces + * {@link MAX_KNOWN_RELAYS} by evicting the oldest non-bootstrap entries. * * @param newRelays - Relay multiaddrs to add. */ @@ -187,18 +227,42 @@ export async function initRemoteIdentity( if (newRelays.length === 0) { return; } - const existing = kernelStore.getKnownRelays(); - const merged = new Set(existing); + const now = Date.now(); + const existing = kernelStore.getRelayEntries(); + const byAddr = new Map(existing.map((entry) => [entry.addr, entry])); let changed = false; - for (const relay of newRelays) { - if (!merged.has(relay)) { - merged.add(relay); + + for (const addr of newRelays) { + const entry = byAddr.get(addr); + if (entry) { + // Update lastSeen on re-observation (create new object to avoid + // mutating potentially-hardened deserialized entries) + if (entry.lastSeen !== now) { + byAddr.set(addr, { ...entry, lastSeen: now }); + changed = true; + } + } else { + byAddr.set(addr, { addr, lastSeen: now, isBootstrap: false }); changed = true; } } - if (changed) { - kernelStore.setKnownRelays([...merged]); + + if (!changed) { + return; } + + let entries = [...byAddr.values()]; + + // Enforce pool cap by evicting oldest non-bootstrap entries + if (entries.length > MAX_KNOWN_RELAYS) { + const bootstrap = entries.filter((entry) => entry.isBootstrap); + const nonBootstrap = entries + .filter((entry) => !entry.isBootstrap) + .sort((a, b) => b.lastSeen - a.lastSeen); + entries = [...bootstrap, ...nonBootstrap].slice(0, MAX_KNOWN_RELAYS); + } + + kernelStore.setRelayEntries(entries); } /** @@ -231,7 +295,7 @@ export async function initRemoteIdentity( return { identity: { getPeerId, issueOcapURL, redeemLocalOcapURL, addKnownRelays }, keySeed, - knownRelays: kernelStore.getKnownRelays(), + knownRelays: kernelStore.getKnownRelayAddresses(), }; } diff --git a/packages/ocap-kernel/src/store/index.test.ts b/packages/ocap-kernel/src/store/index.test.ts index d4d67a6539..f8e0e7e772 100644 --- a/packages/ocap-kernel/src/store/index.test.ts +++ b/packages/ocap-kernel/src/store/index.test.ts @@ -90,7 +90,7 @@ describe('kernel store', () => { 'getKernelPromise', 'getKernelPromiseMessageQueue', 'getKernelServiceKref', - 'getKnownRelays', + 'getKnownRelayAddresses', 'getKpidsToRetire', 'getNextObjectId', 'getNextPromiseId', @@ -105,6 +105,7 @@ describe('kernel store', () => { 'getReachableAndVatSlot', 'getReachableFlag', 'getRefCount', + 'getRelayEntries', 'getRemoteIdentityValue', 'getRemoteIdentityValueRequired', 'getRemoteInfo', @@ -157,10 +158,10 @@ describe('kernel store', () => { 'scheduleReap', 'setGCActions', 'setKernelServiceKref', - 'setKnownRelays', 'setObjectRefCount', 'setPendingMessage', 'setPromiseDecider', + 'setRelayEntries', 'setRemoteHighestReceivedSeq', 'setRemoteIdentityValue', 'setRemoteInfo', @@ -523,31 +524,56 @@ describe('kernel store', () => { }); }); - describe('getKnownRelays', () => { + describe('relay entries', () => { it('returns empty array when no relays are stored', () => { const ks = makeKernelStore(mockKernelDatabase); - expect(ks.getKnownRelays()).toStrictEqual([]); + expect(ks.getRelayEntries()).toStrictEqual([]); + expect(ks.getKnownRelayAddresses()).toStrictEqual([]); }); - it('returns stored relays', () => { + it('stores and retrieves relay entries', () => { const ks = makeKernelStore(mockKernelDatabase); - ks.setKnownRelays(['peer1', 'peer2']); - expect(ks.getKnownRelays()).toStrictEqual(['peer1', 'peer2']); + const entries = [ + { addr: 'relay1', lastSeen: 100, isBootstrap: true }, + { addr: 'relay2', lastSeen: 200, isBootstrap: false }, + ]; + ks.setRelayEntries(entries); + expect(ks.getRelayEntries()).toStrictEqual(entries); + expect(ks.getKnownRelayAddresses()).toStrictEqual(['relay1', 'relay2']); + }); + + it('auto-migrates legacy string[] format to RelayEntry[]', () => { + const ks = makeKernelStore(mockKernelDatabase); + mockKernelDatabase.kernelKVStore.set( + 'knownRelays', + JSON.stringify(['peer1', 'peer2']), + ); + const entries = ks.getRelayEntries(); + expect(entries).toStrictEqual([ + { addr: 'peer1', lastSeen: 0, isBootstrap: false }, + { addr: 'peer2', lastSeen: 0, isBootstrap: false }, + ]); + // Migration should persist the new format + const raw = mockKernelDatabase.kernelKVStore.get('knownRelays'); + expect(JSON.parse(raw as string)).toStrictEqual(entries); }); it('throws when stored knownRelays is not a JSON array', () => { const ks = makeKernelStore(mockKernelDatabase); mockKernelDatabase.kernelKVStore.set('knownRelays', '"not-an-array"'); - expect(() => ks.getKnownRelays()).toThrow( - 'knownRelays must be an array of strings', + expect(() => ks.getRelayEntries()).toThrow( + 'knownRelays must be an array', ); }); - it('throws when stored knownRelays contains non-string entries', () => { + it('throws when relay entries have invalid shape', () => { const ks = makeKernelStore(mockKernelDatabase); - mockKernelDatabase.kernelKVStore.set('knownRelays', '[1, 2, 3]'); - expect(() => ks.getKnownRelays()).toThrow( - 'knownRelays must be an array of strings', + mockKernelDatabase.kernelKVStore.set( + 'knownRelays', + JSON.stringify([{ addr: 'ok', lastSeen: 'bad', isBootstrap: false }]), + ); + expect(() => ks.getRelayEntries()).toThrow( + 'knownRelays entries must have addr, lastSeen, isBootstrap', ); }); }); diff --git a/packages/ocap-kernel/src/store/index.ts b/packages/ocap-kernel/src/store/index.ts index d64f4ae7f2..2b3798dd31 100644 --- a/packages/ocap-kernel/src/store/index.ts +++ b/packages/ocap-kernel/src/store/index.ts @@ -86,7 +86,7 @@ import { getRevocationMethods } from './methods/revocation.ts'; import { getSubclusterMethods } from './methods/subclusters.ts'; import { getTranslators } from './methods/translators.ts'; import { getVatMethods } from './methods/vat.ts'; -import type { StoreContext } from './types.ts'; +import type { RelayEntry, StoreContext } from './types.ts'; /** * Create a new KernelStore object wrapped around a raw kernel database. The @@ -350,21 +350,65 @@ export function makeKernelStore(kdb: KernelDatabase, logger?: Logger) { ): void { kv.set(key, value); }, - getKnownRelays(): string[] { + /** + * Read relay entries from storage, auto-migrating from the legacy + * `string[]` format if necessary. + * + * @returns The relay entries. + */ + getRelayEntries(): RelayEntry[] { const raw = kv.get('knownRelays'); if (!raw) { return []; } const parsed: unknown = JSON.parse(raw); - (Array.isArray(parsed) && - parsed.every((entry: unknown) => typeof entry === 'string')) || - Fail`knownRelays must be an array of strings`; - return parsed as string[]; + Array.isArray(parsed) || Fail`knownRelays must be an array`; + + // Migrate legacy string[] format → RelayEntry[] + if (parsed.length > 0 && typeof parsed[0] === 'string') { + if (!parsed.every((entry: unknown) => typeof entry === 'string')) { + Fail`knownRelays legacy format must be all strings`; + } + const migrated: RelayEntry[] = (parsed as string[]).map((addr) => ({ + addr, + lastSeen: 0, + isBootstrap: false, + })); + kv.set('knownRelays', JSON.stringify(migrated)); + return migrated; + } + + // New RelayEntry[] format + for (const entry of parsed) { + (typeof entry === 'object' && + entry !== null && + typeof (entry as RelayEntry).addr === 'string' && + typeof (entry as RelayEntry).lastSeen === 'number' && + typeof (entry as RelayEntry).isBootstrap === 'boolean') || + Fail`knownRelays entries must have addr, lastSeen, isBootstrap`; + } + return parsed as RelayEntry[]; }, - setKnownRelays(relays: string[]): void { - kv.set('knownRelays', JSON.stringify(relays)); + + /** + * Persist relay entries to storage. + * + * @param entries - The relay entries to persist. + */ + setRelayEntries(entries: RelayEntry[]): void { + kv.set('knownRelays', JSON.stringify(entries)); + }, + + /** + * Convenience: return only relay addresses (for ConnectionFactory, etc.). + * + * @returns The relay addresses. + */ + getKnownRelayAddresses(): string[] { + return this.getRelayEntries().map((entry) => entry.addr); }, }); } export type KernelStore = ReturnType; +export type { RelayEntry } from './types.ts'; diff --git a/packages/ocap-kernel/src/store/types.ts b/packages/ocap-kernel/src/store/types.ts index b3b2218232..8b404aa3b2 100644 --- a/packages/ocap-kernel/src/store/types.ts +++ b/packages/ocap-kernel/src/store/types.ts @@ -47,3 +47,15 @@ export type VatCleanupWork = { promises: number; kv: number; }; + +/** + * A relay entry with metadata for prioritized selection and bounded storage. + */ +export type RelayEntry = { + /** Relay multiaddr string. */ + addr: string; + /** Epoch ms when the relay was last added or re-observed. */ + lastSeen: number; + /** True if provided at kernel initialization (prioritized during eviction). */ + isBootstrap: boolean; +}; From 1b4d32d4d65a27bcf46b4c7d1e8753c0d28531da Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Fri, 10 Apr 2026 15:14:21 +0200 Subject: [PATCH 2/6] =?UTF-8?q?fix(ocap-kernel):=20address=20PR=20review?= =?UTF-8?q?=20=E2=80=94=20avoid=20mutation,=20fix=20this=20binding,=20add?= =?UTF-8?q?=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Avoid direct mutation of potentially-hardened entries in initRemoteIdentity bootstrap clearing loop (use spread instead) - Extract getRelayEntries as local function in makeKernelStore to avoid fragile this binding in getKnownRelayAddresses - Add test: init clears bootstrap flag on relays removed from bootstrap set - Add test: init enforces MAX_KNOWN_RELAYS pool cap Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/remotes/kernel/remote-comms.test.ts | 43 ++++++++++ .../src/remotes/kernel/remote-comms.ts | 7 +- packages/ocap-kernel/src/store/index.ts | 82 ++++++++++--------- 3 files changed, 89 insertions(+), 43 deletions(-) diff --git a/packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts b/packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts index c669965345..89e2d73248 100644 --- a/packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts +++ b/packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts @@ -591,6 +591,49 @@ describe('remote-comms', () => { expect(secondSeen).toBeGreaterThan(firstSeen); }); + it('init clears bootstrap flag on relays removed from the bootstrap set', async () => { + await initRemoteIdentity(mockKernelStore, { + relays: ['/dns4/relayA.example/tcp/443/wss/p2p-circuit'], + }); + + // Re-init with a different bootstrap set + await initRemoteIdentity(mockKernelStore, { + relays: ['/dns4/relayB.example/tcp/443/wss/p2p-circuit'], + }); + + const entries = mockKernelStore.getRelayEntries(); + expect( + entries.find((entry) => entry.addr.includes('relayA'))?.isBootstrap, + ).toBe(false); + expect( + entries.find((entry) => entry.addr.includes('relayB'))?.isBootstrap, + ).toBe(true); + }); + + it('init enforces MAX_KNOWN_RELAYS pool cap', async () => { + // Pre-seed MAX_KNOWN_RELAYS learned relays + mockKernelStore.setRelayEntries( + Array.from({ length: MAX_KNOWN_RELAYS }, (_, i) => ({ + addr: `/dns4/learned${i}.example/tcp/443/wss/p2p-circuit`, + lastSeen: 100, + isBootstrap: false, + })), + ); + + const bootstrapRelays = Array.from( + { length: 5 }, + (_, i) => `/dns4/bootstrap${i}.example/tcp/443/wss/p2p-circuit`, + ); + await initRemoteIdentity(mockKernelStore, { relays: bootstrapRelays }); + + const entries = mockKernelStore.getRelayEntries(); + expect(entries).toHaveLength(MAX_KNOWN_RELAYS); + // All bootstrap relays must survive + for (const addr of bootstrapRelays) { + expect(entries.some((entry) => entry.addr === addr)).toBe(true); + } + }); + it('init marks bootstrap relays and preserves learned relays', async () => { // Pre-seed a learned relay mockKernelStore.setRelayEntries([ diff --git a/packages/ocap-kernel/src/remotes/kernel/remote-comms.ts b/packages/ocap-kernel/src/remotes/kernel/remote-comms.ts index 7de4981bb2..f815cdc317 100644 --- a/packages/ocap-kernel/src/remotes/kernel/remote-comms.ts +++ b/packages/ocap-kernel/src/remotes/kernel/remote-comms.ts @@ -124,9 +124,10 @@ export async function initRemoteIdentity( byAddr.set(addr, { addr, lastSeen: now, isBootstrap: true }); } // Clear bootstrap flag on entries no longer in the current bootstrap set - for (const entry of byAddr.values()) { - if (entry.isBootstrap && !bootstrapSet.has(entry.addr)) { - entry.isBootstrap = false; + // (create new objects to avoid mutating potentially-hardened entries) + for (const [addr, entry] of byAddr.entries()) { + if (entry.isBootstrap && !bootstrapSet.has(addr)) { + byAddr.set(addr, { ...entry, isBootstrap: false }); } } let merged = [...byAddr.values()]; diff --git a/packages/ocap-kernel/src/store/index.ts b/packages/ocap-kernel/src/store/index.ts index 2b3798dd31..ddc22a378c 100644 --- a/packages/ocap-kernel/src/store/index.ts +++ b/packages/ocap-kernel/src/store/index.ts @@ -286,6 +286,46 @@ export function makeKernelStore(kdb: KernelDatabase, logger?: Logger) { kdb.rollbackSavepoint(name); } + /** + * Read relay entries from storage, auto-migrating from the legacy + * `string[]` format if necessary. + * + * @returns The relay entries. + */ + function getRelayEntries(): RelayEntry[] { + const raw = kv.get('knownRelays'); + if (!raw) { + return []; + } + const parsed: unknown = JSON.parse(raw); + Array.isArray(parsed) || Fail`knownRelays must be an array`; + + // Migrate legacy string[] format → RelayEntry[] (persisted back to storage) + if (parsed.length > 0 && typeof parsed[0] === 'string') { + if (!parsed.every((entry: unknown) => typeof entry === 'string')) { + Fail`knownRelays legacy format must be all strings`; + } + const migrated: RelayEntry[] = (parsed as string[]).map((addr) => ({ + addr, + lastSeen: 0, + isBootstrap: false, + })); + kv.set('knownRelays', JSON.stringify(migrated)); + return migrated; + } + + // New RelayEntry[] format + for (const entry of parsed) { + (typeof entry === 'object' && + entry !== null && + typeof (entry as RelayEntry).addr === 'string' && + typeof (entry as RelayEntry).lastSeen === 'number' && + typeof (entry as RelayEntry).isBootstrap === 'boolean') || + Fail`knownRelays entries must have addr, lastSeen, isBootstrap`; + } + return parsed as RelayEntry[]; + } + return harden({ ...id, ...queue, @@ -350,45 +390,7 @@ export function makeKernelStore(kdb: KernelDatabase, logger?: Logger) { ): void { kv.set(key, value); }, - /** - * Read relay entries from storage, auto-migrating from the legacy - * `string[]` format if necessary. - * - * @returns The relay entries. - */ - getRelayEntries(): RelayEntry[] { - const raw = kv.get('knownRelays'); - if (!raw) { - return []; - } - const parsed: unknown = JSON.parse(raw); - Array.isArray(parsed) || Fail`knownRelays must be an array`; - - // Migrate legacy string[] format → RelayEntry[] - if (parsed.length > 0 && typeof parsed[0] === 'string') { - if (!parsed.every((entry: unknown) => typeof entry === 'string')) { - Fail`knownRelays legacy format must be all strings`; - } - const migrated: RelayEntry[] = (parsed as string[]).map((addr) => ({ - addr, - lastSeen: 0, - isBootstrap: false, - })); - kv.set('knownRelays', JSON.stringify(migrated)); - return migrated; - } - - // New RelayEntry[] format - for (const entry of parsed) { - (typeof entry === 'object' && - entry !== null && - typeof (entry as RelayEntry).addr === 'string' && - typeof (entry as RelayEntry).lastSeen === 'number' && - typeof (entry as RelayEntry).isBootstrap === 'boolean') || - Fail`knownRelays entries must have addr, lastSeen, isBootstrap`; - } - return parsed as RelayEntry[]; - }, + getRelayEntries, /** * Persist relay entries to storage. @@ -405,7 +407,7 @@ export function makeKernelStore(kdb: KernelDatabase, logger?: Logger) { * @returns The relay addresses. */ getKnownRelayAddresses(): string[] { - return this.getRelayEntries().map((entry) => entry.addr); + return getRelayEntries().map((entry) => entry.addr); }, }); } From 1690fd867d9df9c979b84afee3d6be1fa6297b59 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Fri, 10 Apr 2026 15:14:52 +0200 Subject: [PATCH 3/6] docs: Update changelogs --- packages/ocap-kernel/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/ocap-kernel/CHANGELOG.md b/packages/ocap-kernel/CHANGELOG.md index 90b37cecc7..19d5c4206e 100644 --- a/packages/ocap-kernel/CHANGELOG.md +++ b/packages/ocap-kernel/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Deserialize CapData rejections in `Kernel.queueMessage` so vat errors surface as plain `Error` objects to all callers ([#928](https://github.com/MetaMask/ocap-kernel/pull/928)) +### Changed + +- Bound relay hints in OCAP URLs to a maximum of 3 and cap the relay pool at 20 entries with eviction of oldest non-bootstrap relays ([#929](https://github.com/MetaMask/ocap-kernel/pull/929)) + ## [0.7.0] ### Added From 1dd4bc9605a4d698f5df9d542aa09329157871c3 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Mon, 13 Apr 2026 13:07:16 +0200 Subject: [PATCH 4/6] refactor(ocap-kernel): extract relay store methods, make caps configurable, fix review issues Extract relay and remote identity store methods from store/index.ts into store/methods/relay.ts following the existing methods pattern. Make MAX_URL_RELAY_HINTS and MAX_KNOWN_RELAYS configurable via RemoteCommsOptions (defaults: 3 and 20), threaded through the full RPC handler -> Kernel -> initRemoteIdentity options flow. Address all review findings: add write-path validation with superstruct RelayEntryStruct, wrap JSON.parse with contextual error, log migration and eviction events, warn when bootstrap count exceeds pool cap, document Date.now() SES assumption and default value rationale, add missing tests for sorting stability, bootstrap-exceeds-cap, mixed legacy format, and configurable caps. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/remotes/kernel/remote-comms.test.ts | 143 +++++++++++++-- .../src/remotes/kernel/remote-comms.ts | 106 ++++++++---- packages/ocap-kernel/src/remotes/types.ts | 12 ++ .../kernel-control/init-remote-comms.test.ts | 58 +++++++ .../rpc/kernel-control/init-remote-comms.ts | 8 + packages/ocap-kernel/src/store/index.test.ts | 54 ------ packages/ocap-kernel/src/store/index.ts | 83 +-------- .../src/store/methods/relay.test.ts | 149 ++++++++++++++++ .../ocap-kernel/src/store/methods/relay.ts | 163 ++++++++++++++++++ packages/ocap-kernel/src/store/types.ts | 22 ++- 10 files changed, 619 insertions(+), 179 deletions(-) create mode 100644 packages/ocap-kernel/src/store/methods/relay.test.ts create mode 100644 packages/ocap-kernel/src/store/methods/relay.ts diff --git a/packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts b/packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts index 89e2d73248..2ad8ee6858 100644 --- a/packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts +++ b/packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts @@ -8,8 +8,8 @@ import { initRemoteIdentity, initRemoteComms, parseOcapURL, - MAX_URL_RELAY_HINTS, - MAX_KNOWN_RELAYS, + DEFAULT_MAX_URL_RELAY_HINTS, + DEFAULT_MAX_KNOWN_RELAYS, } from './remote-comms.ts'; import { createMockRemotesFactory } from '../../../test/remotes-mocks.ts'; import { makeMapKernelDatabase } from '../../../test/storage.ts'; @@ -99,7 +99,7 @@ describe('remote-comms', () => { const { oid, hints } = parseOcapURL(ocapURL); const knownRelayAddresses = mockKernelStore.getKnownRelayAddresses(); expect(knownRelayAddresses).toStrictEqual(testRelays); - // URL should embed the relays (within MAX_URL_RELAY_HINTS cap) + // URL should embed the relays (within DEFAULT_MAX_URL_RELAY_HINTS cap) expect(hints).toStrictEqual(testRelays); const referenceURL = `ocap:${oid}@${peerId},${testRelays.join(',')}`; expect(ocapURL).toBe(referenceURL); @@ -499,9 +499,9 @@ describe('remote-comms', () => { expect(stored).toStrictEqual(initialRelays); }); - it('issueOcapURL caps relay hints to MAX_URL_RELAY_HINTS', async () => { + it('issueOcapURL caps relay hints to DEFAULT_MAX_URL_RELAY_HINTS', async () => { const relays = Array.from( - { length: MAX_URL_RELAY_HINTS + 3 }, + { length: DEFAULT_MAX_URL_RELAY_HINTS + 3 }, (_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`, ); const { identity } = await initRemoteIdentity(mockKernelStore, { @@ -510,7 +510,7 @@ describe('remote-comms', () => { const ocapURL = await identity.issueOcapURL('ko1' as KRef); const { hints } = parseOcapURL(ocapURL); - expect(hints).toHaveLength(MAX_URL_RELAY_HINTS); + expect(hints).toHaveLength(DEFAULT_MAX_URL_RELAY_HINTS); }); it('issueOcapURL prefers bootstrap relays over learned relays', async () => { @@ -531,25 +531,25 @@ describe('remote-comms', () => { const ocapURL = await identity.issueOcapURL('ko1' as KRef); const { hints } = parseOcapURL(ocapURL); - expect(hints).toHaveLength(MAX_URL_RELAY_HINTS); + expect(hints).toHaveLength(DEFAULT_MAX_URL_RELAY_HINTS); // Bootstrap relays should be included ahead of learned relays for (const bootstrap of bootstrapRelays) { expect(hints).toContain(bootstrap); } }); - it('addKnownRelays enforces MAX_KNOWN_RELAYS pool cap', async () => { + it('addKnownRelays enforces DEFAULT_MAX_KNOWN_RELAYS pool cap', async () => { const { identity } = await initRemoteIdentity(mockKernelStore); // Add more relays than the cap const relays = Array.from( - { length: MAX_KNOWN_RELAYS + 5 }, + { length: DEFAULT_MAX_KNOWN_RELAYS + 5 }, (_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`, ); identity.addKnownRelays(relays); const entries = mockKernelStore.getRelayEntries(); - expect(entries).toHaveLength(MAX_KNOWN_RELAYS); + expect(entries).toHaveLength(DEFAULT_MAX_KNOWN_RELAYS); }); it('addKnownRelays evicts oldest non-bootstrap relays when pool is full', async () => { @@ -562,13 +562,13 @@ describe('remote-comms', () => { // Fill pool to the cap const fillerRelays = Array.from( - { length: MAX_KNOWN_RELAYS }, + { length: DEFAULT_MAX_KNOWN_RELAYS }, (_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`, ); identity.addKnownRelays(fillerRelays); const entries = mockKernelStore.getRelayEntries(); - expect(entries).toHaveLength(MAX_KNOWN_RELAYS); + expect(entries).toHaveLength(DEFAULT_MAX_KNOWN_RELAYS); // Bootstrap relay must survive eviction expect(entries.some((entry) => entry.addr === bootstrapRelays[0])).toBe( true, @@ -610,10 +610,10 @@ describe('remote-comms', () => { ).toBe(true); }); - it('init enforces MAX_KNOWN_RELAYS pool cap', async () => { - // Pre-seed MAX_KNOWN_RELAYS learned relays + it('init enforces DEFAULT_MAX_KNOWN_RELAYS pool cap', async () => { + // Pre-seed DEFAULT_MAX_KNOWN_RELAYS learned relays mockKernelStore.setRelayEntries( - Array.from({ length: MAX_KNOWN_RELAYS }, (_, i) => ({ + Array.from({ length: DEFAULT_MAX_KNOWN_RELAYS }, (_, i) => ({ addr: `/dns4/learned${i}.example/tcp/443/wss/p2p-circuit`, lastSeen: 100, isBootstrap: false, @@ -627,7 +627,7 @@ describe('remote-comms', () => { await initRemoteIdentity(mockKernelStore, { relays: bootstrapRelays }); const entries = mockKernelStore.getRelayEntries(); - expect(entries).toHaveLength(MAX_KNOWN_RELAYS); + expect(entries).toHaveLength(DEFAULT_MAX_KNOWN_RELAYS); // All bootstrap relays must survive for (const addr of bootstrapRelays) { expect(entries.some((entry) => entry.addr === addr)).toBe(true); @@ -664,6 +664,117 @@ describe('remote-comms', () => { ).toBe(false); }); + it('logs when bootstrap relay count exceeds maxKnownRelays', async () => { + const mockLogger = { log: vi.fn(), error: vi.fn() }; + const relays = Array.from( + { length: 5 }, + (_, i) => `/dns4/bootstrap${i}.example/tcp/443/wss/p2p-circuit`, + ); + await initRemoteIdentity( + mockKernelStore, + { relays, maxKnownRelays: 3 }, + mockLogger as unknown as Logger, + ); + expect(mockLogger.log).toHaveBeenCalledWith( + expect.stringContaining( + 'bootstrap relay count (5) exceeds maxKnownRelays (3)', + ), + ); + }); + + it('logs eviction count when pool cap is exceeded during init', async () => { + const mockLogger = { log: vi.fn(), error: vi.fn() }; + mockKernelStore.setRelayEntries( + Array.from({ length: 20 }, (_, i) => ({ + addr: `/dns4/learned${i}.example/tcp/443/wss/p2p-circuit`, + lastSeen: 100, + isBootstrap: false, + })), + ); + await initRemoteIdentity( + mockKernelStore, + { + relays: ['/dns4/bootstrap.example/tcp/443/wss/p2p-circuit'], + maxKnownRelays: 10, + }, + mockLogger as unknown as Logger, + ); + expect(mockLogger.log).toHaveBeenCalledWith( + expect.stringContaining('evicted'), + ); + }); + + it('sorts same-tier relays by lastSeen descending in URL hints', async () => { + mockKernelStore.setRelayEntries([ + { + addr: '/dns4/relay-old.example/tcp/443/wss/p2p-circuit', + lastSeen: 100, + isBootstrap: false, + }, + { + addr: '/dns4/relay-new.example/tcp/443/wss/p2p-circuit', + lastSeen: 300, + isBootstrap: false, + }, + { + addr: '/dns4/relay-mid.example/tcp/443/wss/p2p-circuit', + lastSeen: 200, + isBootstrap: false, + }, + ]); + const { identity } = await initRemoteIdentity(mockKernelStore); + const ocapURL = await identity.issueOcapURL('ko1' as KRef); + const { hints } = parseOcapURL(ocapURL); + expect(hints).toStrictEqual([ + '/dns4/relay-new.example/tcp/443/wss/p2p-circuit', + '/dns4/relay-mid.example/tcp/443/wss/p2p-circuit', + '/dns4/relay-old.example/tcp/443/wss/p2p-circuit', + ]); + }); + + it('respects custom maxUrlRelayHints option', async () => { + const relays = Array.from( + { length: 10 }, + (_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`, + ); + const { identity } = await initRemoteIdentity(mockKernelStore, { + relays, + maxUrlRelayHints: 5, + }); + const ocapURL = await identity.issueOcapURL('ko1' as KRef); + const { hints } = parseOcapURL(ocapURL); + expect(hints).toHaveLength(5); + }); + + it('respects custom maxKnownRelays option', async () => { + const { identity } = await initRemoteIdentity(mockKernelStore, { + maxKnownRelays: 5, + }); + const relays = Array.from( + { length: 10 }, + (_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`, + ); + identity.addKnownRelays(relays); + expect(mockKernelStore.getRelayEntries()).toHaveLength(5); + }); + + it('addKnownRelays logs eviction when pool cap is exceeded', async () => { + const mockLogger = { log: vi.fn(), error: vi.fn() }; + const { identity } = await initRemoteIdentity( + mockKernelStore, + { maxKnownRelays: 5 }, + mockLogger as unknown as Logger, + ); + const relays = Array.from( + { length: 10 }, + (_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`, + ); + identity.addKnownRelays(relays); + expect(mockLogger.log).toHaveBeenCalledWith( + expect.stringContaining('evicted'), + ); + }); + it('throws with mnemonic when identity already exists', async () => { mockKernelStore.setRemoteIdentityValue('peerId', 'existing-peer-id'); mockKernelStore.setRemoteIdentityValue( diff --git a/packages/ocap-kernel/src/remotes/kernel/remote-comms.ts b/packages/ocap-kernel/src/remotes/kernel/remote-comms.ts index f815cdc317..2a99342118 100644 --- a/packages/ocap-kernel/src/remotes/kernel/remote-comms.ts +++ b/packages/ocap-kernel/src/remotes/kernel/remote-comms.ts @@ -5,7 +5,7 @@ import { toHex, fromHex } from '@metamask/kernel-utils'; import type { Logger } from '@metamask/logger'; import { base58btc } from 'multiformats/bases/base58'; -import type { KernelStore } from '../../store/index.ts'; +import type { KernelStore, RelayEntry } from '../../store/index.ts'; import { insistKRef } from '../../types.ts'; import type { KRef, PlatformServices } from '../../types.ts'; import { mnemonicToSeed } from '../../utils/bip39.ts'; @@ -24,11 +24,38 @@ export type OcapURLParts = { hints: string[]; }; -/** Maximum number of relay hints embedded in a single OCAP URL. */ -export const MAX_URL_RELAY_HINTS = 3; +/** + * Default maximum number of relay hints embedded in a single OCAP URL. + * 3 balances URL length against connectivity resilience — most relay + * topologies have fewer than 3 distinct relays available to a peer. + */ +export const DEFAULT_MAX_URL_RELAY_HINTS = 3; + +/** + * Default maximum relay entries stored in the kernel's relay pool. + * 20 bounds storage overhead while accommodating typical bootstrap sets + * (2–5) plus learned relays discovered through peer exchange. + */ +export const DEFAULT_MAX_KNOWN_RELAYS = 20; -/** Maximum number of relay entries stored in the kernel's relay pool. */ -export const MAX_KNOWN_RELAYS = 20; +/** + * Enforce the relay pool cap by keeping all bootstrap entries and the newest + * non-bootstrap entries, up to `cap`. + * + * @param entries - The full set of relay entries. + * @param cap - The maximum pool size. + * @returns The entries trimmed to the pool cap. + */ +function enforcePoolCap(entries: RelayEntry[], cap: number): RelayEntry[] { + if (entries.length <= cap) { + return entries; + } + const bootstrap = entries.filter((entry) => entry.isBootstrap); + const nonBootstrap = entries + .filter((entry) => !entry.isBootstrap) + .sort((a, b) => b.lastSeen - a.lastSeen); + return [...bootstrap, ...nonBootstrap].slice(0, cap); +} /** * Break down an ocap URL string into its constituent parts. @@ -92,15 +119,25 @@ async function generateKeyInfo(seedString?: string): Promise<[string, string]> { * * @param kernelStore - The kernel store, for storing persistent key info. * @param options - Options for identity initialization. - * @param options.relays - Relay addresses to embed in issued OCAP URLs. + * @param options.relays - Bootstrap relay addresses. These are merged into + * the relay pool, marked as bootstrap (prioritized during eviction and URL + * selection), and persisted. Relays previously marked as bootstrap that are + * no longer in this list have their bootstrap flag cleared. * @param options.mnemonic - BIP39 mnemonic for seed recovery. + * @param options.maxUrlRelayHints - Cap on relay hints per OCAP URL. + * @param options.maxKnownRelays - Cap on the stored relay pool. * @param logger - The logger to use. * @param keySeed - Optional seed for key generation. * @returns the identity object, the key seed, and known relays. */ export async function initRemoteIdentity( kernelStore: KernelStore, - options?: { relays?: string[] | undefined; mnemonic?: string | undefined }, + options?: { + relays?: string[] | undefined; + mnemonic?: string | undefined; + maxUrlRelayHints?: number | undefined; + maxKnownRelays?: number | undefined; + }, logger?: Logger, keySeed?: string, ): Promise<{ @@ -114,13 +151,26 @@ export async function initRemoteIdentity( const decoder = new TextDecoder(); const relays = options?.relays ?? []; const mnemonic = options?.mnemonic; + const maxUrlRelayHints = + options?.maxUrlRelayHints ?? DEFAULT_MAX_URL_RELAY_HINTS; + const maxKnownRelays = options?.maxKnownRelays ?? DEFAULT_MAX_KNOWN_RELAYS; + if (relays.length > 0) { + // Date.now() is available under SES lockdown (on the intrinsics allow-list). const now = Date.now(); const bootstrapSet = new Set(relays); + + if (relays.length > maxKnownRelays) { + logger?.log( + `relay init: bootstrap relay count (${relays.length}) exceeds maxKnownRelays (${maxKnownRelays}); pool will be truncated`, + ); + } + // Merge with existing entries: mark bootstrap relays, preserve learned ones const existing = kernelStore.getRelayEntries(); const byAddr = new Map(existing.map((entry) => [entry.addr, entry])); for (const addr of relays) { + // Bootstrap relays always get a fresh lastSeen timestamp on (re-)init byAddr.set(addr, { addr, lastSeen: now, isBootstrap: true }); } // Clear bootstrap flag on entries no longer in the current bootstrap set @@ -130,14 +180,12 @@ export async function initRemoteIdentity( byAddr.set(addr, { ...entry, isBootstrap: false }); } } - let merged = [...byAddr.values()]; - // Enforce pool cap: keep all bootstrap, then newest non-bootstrap - if (merged.length > MAX_KNOWN_RELAYS) { - const bootstrap = merged.filter((entry) => entry.isBootstrap); - const nonBootstrap = merged - .filter((entry) => !entry.isBootstrap) - .sort((a, b) => b.lastSeen - a.lastSeen); - merged = [...bootstrap, ...nonBootstrap].slice(0, MAX_KNOWN_RELAYS); + const preCapCount = byAddr.size; + const merged = enforcePoolCap([...byAddr.values()], maxKnownRelays); + if (merged.length < preCapCount) { + logger?.log( + `relay init: evicted ${preCapCount - merged.length} relays to enforce pool cap (${maxKnownRelays})`, + ); } kernelStore.setRelayEntries(merged); } @@ -202,7 +250,8 @@ export async function initRemoteIdentity( const rawOid = await cipher.encrypt(encodedKref, ocapURLKey); const oid = base58btc.encode(rawOid); const entries = kernelStore.getRelayEntries(); - // Select top relays: bootstrap first, then most recently seen + // Select top relays: bootstrap first (operator-configured, most reliable), + // then most recently seen (likeliest to be online) const sorted = [...entries].sort((a, b) => { if (a.isBootstrap !== b.isBootstrap) { return a.isBootstrap ? -1 : 1; @@ -210,7 +259,7 @@ export async function initRemoteIdentity( return b.lastSeen - a.lastSeen; }); const selected = sorted - .slice(0, MAX_URL_RELAY_HINTS) + .slice(0, maxUrlRelayHints) .map((entry) => entry.addr); const relaySuffix = selected.length > 0 ? `,${selected.join(',')}` : ''; const ocapURL = `ocap:${oid}@${peerId}${relaySuffix}`; @@ -228,6 +277,7 @@ export async function initRemoteIdentity( if (newRelays.length === 0) { return; } + // Date.now() is available under SES lockdown (on the intrinsics allow-list). const now = Date.now(); const existing = kernelStore.getRelayEntries(); const byAddr = new Map(existing.map((entry) => [entry.addr, entry])); @@ -252,18 +302,14 @@ export async function initRemoteIdentity( return; } - let entries = [...byAddr.values()]; - - // Enforce pool cap by evicting oldest non-bootstrap entries - if (entries.length > MAX_KNOWN_RELAYS) { - const bootstrap = entries.filter((entry) => entry.isBootstrap); - const nonBootstrap = entries - .filter((entry) => !entry.isBootstrap) - .sort((a, b) => b.lastSeen - a.lastSeen); - entries = [...bootstrap, ...nonBootstrap].slice(0, MAX_KNOWN_RELAYS); + const preCapCount = byAddr.size; + const capped = enforcePoolCap([...byAddr.values()], maxKnownRelays); + if (capped.length < preCapCount) { + logger?.log( + `addKnownRelays: evicted ${preCapCount - capped.length} relays to enforce pool cap (${maxKnownRelays})`, + ); } - - kernelStore.setRelayEntries(entries); + kernelStore.setRelayEntries(capped); } /** @@ -327,13 +373,15 @@ export async function initRemoteComms( incarnationId?: string, onIncarnationChange?: OnIncarnationChange, ): Promise { - const { relays = [], mnemonic } = options; + const { relays = [], mnemonic, maxUrlRelayHints, maxKnownRelays } = options; const result = await initRemoteIdentity( kernelStore, { relays, ...(mnemonic === undefined ? {} : { mnemonic }), + maxUrlRelayHints, + maxKnownRelays, }, logger, keySeed, diff --git a/packages/ocap-kernel/src/remotes/types.ts b/packages/ocap-kernel/src/remotes/types.ts index 53b72f3cc1..23e52ec096 100644 --- a/packages/ocap-kernel/src/remotes/types.ts +++ b/packages/ocap-kernel/src/remotes/types.ts @@ -140,6 +140,18 @@ export type RemoteCommsOptions = { * automatically aborted with an InactivityTimeoutError. */ streamInactivityTimeoutMs?: number | undefined; + /** + * Maximum number of relay hints embedded in a single OCAP URL (default: 3). + * Higher values produce longer URLs but improve connectivity resilience for + * peers with stale relay information. + */ + maxUrlRelayHints?: number | undefined; + /** + * Maximum number of relay entries stored in the kernel's relay pool + * (default: 20). The pool retains all bootstrap relays; when the cap is + * reached, the oldest non-bootstrap (learned) entries are evicted first. + */ + maxKnownRelays?: number | undefined; /** * Hostnames or IP addresses permitted for plain ws:// relay connections, * in addition to RFC 1918 / loopback addresses which are always allowed. diff --git a/packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.test.ts b/packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.test.ts index 61a853ca9e..e8dbe6c4c1 100644 --- a/packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.test.ts +++ b/packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.test.ts @@ -50,12 +50,38 @@ describe('initRemoteCommsHandler', () => { expect(result).toBeNull(); }); + it('passes maxUrlRelayHints option', async () => { + const result = await initRemoteCommsHandler.implementation( + { kernel: mockKernel }, + { maxUrlRelayHints: 5 }, + ); + + expect(mockKernel.initRemoteComms).toHaveBeenCalledWith({ + maxUrlRelayHints: 5, + }); + expect(result).toBeNull(); + }); + + it('passes maxKnownRelays option', async () => { + const result = await initRemoteCommsHandler.implementation( + { kernel: mockKernel }, + { maxKnownRelays: 30 }, + ); + + expect(mockKernel.initRemoteComms).toHaveBeenCalledWith({ + maxKnownRelays: 30, + }); + expect(result).toBeNull(); + }); + it('passes all options', async () => { const params = { relays: ['relay1'], directListenAddresses: ['/ip4/0.0.0.0/udp/0/quic-v1'], maxRetryAttempts: 5, maxQueue: 100, + maxUrlRelayHints: 3, + maxKnownRelays: 20, }; const result = await initRemoteCommsHandler.implementation( @@ -68,6 +94,8 @@ describe('initRemoteCommsHandler', () => { directListenAddresses: ['/ip4/0.0.0.0/udp/0/quic-v1'], maxRetryAttempts: 5, maxQueue: 100, + maxUrlRelayHints: 3, + maxKnownRelays: 20, }); expect(result).toBeNull(); }); @@ -107,5 +135,35 @@ describe('initRemoteCommsHandler', () => { expect(is({ maxQueue: value }, initRemoteCommsSpec.params)).toBe(false); }, ); + + it.each([1, 3, 10])('accepts valid maxUrlRelayHints value: %s', (value) => { + expect(is({ maxUrlRelayHints: value }, initRemoteCommsSpec.params)).toBe( + true, + ); + }); + + it.each([0, -1, -100, 1.5, 3.14])( + 'rejects invalid maxUrlRelayHints value: %s', + (value) => { + expect( + is({ maxUrlRelayHints: value }, initRemoteCommsSpec.params), + ).toBe(false); + }, + ); + + it.each([1, 20, 100])('accepts valid maxKnownRelays value: %s', (value) => { + expect(is({ maxKnownRelays: value }, initRemoteCommsSpec.params)).toBe( + true, + ); + }); + + it.each([0, -1, -100, 1.5, 3.14])( + 'rejects invalid maxKnownRelays value: %s', + (value) => { + expect(is({ maxKnownRelays: value }, initRemoteCommsSpec.params)).toBe( + false, + ); + }, + ); }); }); diff --git a/packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.ts b/packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.ts index 687bb6c699..eef2656984 100644 --- a/packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.ts +++ b/packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.ts @@ -19,6 +19,8 @@ const initRemoteCommsParamsStruct = object({ directListenAddresses: optional(array(string())), maxRetryAttempts: optional(min(integer(), 0)), maxQueue: optional(min(integer(), 0)), + maxUrlRelayHints: optional(min(integer(), 1)), + maxKnownRelays: optional(min(integer(), 1)), allowedWsHosts: optional(array(string())), }); @@ -70,6 +72,12 @@ export const initRemoteCommsHandler: Handler< if (params.maxQueue !== undefined) { options.maxQueue = params.maxQueue; } + if (params.maxUrlRelayHints !== undefined) { + options.maxUrlRelayHints = params.maxUrlRelayHints; + } + if (params.maxKnownRelays !== undefined) { + options.maxKnownRelays = params.maxKnownRelays; + } if (params.allowedWsHosts !== undefined) { options.allowedWsHosts = params.allowedWsHosts; } diff --git a/packages/ocap-kernel/src/store/index.test.ts b/packages/ocap-kernel/src/store/index.test.ts index f8e0e7e772..ba7c068a9d 100644 --- a/packages/ocap-kernel/src/store/index.test.ts +++ b/packages/ocap-kernel/src/store/index.test.ts @@ -523,58 +523,4 @@ describe('kernel store', () => { expect(preserved).toBe(original); }); }); - - describe('relay entries', () => { - it('returns empty array when no relays are stored', () => { - const ks = makeKernelStore(mockKernelDatabase); - expect(ks.getRelayEntries()).toStrictEqual([]); - expect(ks.getKnownRelayAddresses()).toStrictEqual([]); - }); - - it('stores and retrieves relay entries', () => { - const ks = makeKernelStore(mockKernelDatabase); - const entries = [ - { addr: 'relay1', lastSeen: 100, isBootstrap: true }, - { addr: 'relay2', lastSeen: 200, isBootstrap: false }, - ]; - ks.setRelayEntries(entries); - expect(ks.getRelayEntries()).toStrictEqual(entries); - expect(ks.getKnownRelayAddresses()).toStrictEqual(['relay1', 'relay2']); - }); - - it('auto-migrates legacy string[] format to RelayEntry[]', () => { - const ks = makeKernelStore(mockKernelDatabase); - mockKernelDatabase.kernelKVStore.set( - 'knownRelays', - JSON.stringify(['peer1', 'peer2']), - ); - const entries = ks.getRelayEntries(); - expect(entries).toStrictEqual([ - { addr: 'peer1', lastSeen: 0, isBootstrap: false }, - { addr: 'peer2', lastSeen: 0, isBootstrap: false }, - ]); - // Migration should persist the new format - const raw = mockKernelDatabase.kernelKVStore.get('knownRelays'); - expect(JSON.parse(raw as string)).toStrictEqual(entries); - }); - - it('throws when stored knownRelays is not a JSON array', () => { - const ks = makeKernelStore(mockKernelDatabase); - mockKernelDatabase.kernelKVStore.set('knownRelays', '"not-an-array"'); - expect(() => ks.getRelayEntries()).toThrow( - 'knownRelays must be an array', - ); - }); - - it('throws when relay entries have invalid shape', () => { - const ks = makeKernelStore(mockKernelDatabase); - mockKernelDatabase.kernelKVStore.set( - 'knownRelays', - JSON.stringify([{ addr: 'ok', lastSeen: 'bad', isBootstrap: false }]), - ); - expect(() => ks.getRelayEntries()).toThrow( - 'knownRelays entries must have addr, lastSeen, isBootstrap', - ); - }); - }); }); diff --git a/packages/ocap-kernel/src/store/index.ts b/packages/ocap-kernel/src/store/index.ts index ddc22a378c..144b3a6c48 100644 --- a/packages/ocap-kernel/src/store/index.ts +++ b/packages/ocap-kernel/src/store/index.ts @@ -64,7 +64,6 @@ * kernelService.${serviceName} = ${koid} // kref of kernel service object ${serviceName} */ -import { Fail } from '@endo/errors'; import type { KernelDatabase, KVStore, VatStore } from '@metamask/kernel-store'; import { Logger } from '@metamask/logger'; @@ -81,12 +80,13 @@ import { getPromiseMethods } from './methods/promise.ts'; import { getQueueMethods } from './methods/queue.ts'; import { getReachableMethods } from './methods/reachable.ts'; import { getRefCountMethods } from './methods/refcount.ts'; +import { getRelayMethods } from './methods/relay.ts'; import { getRemoteMethods } from './methods/remote.ts'; import { getRevocationMethods } from './methods/revocation.ts'; import { getSubclusterMethods } from './methods/subclusters.ts'; import { getTranslators } from './methods/translators.ts'; import { getVatMethods } from './methods/vat.ts'; -import type { RelayEntry, StoreContext } from './types.ts'; +import type { StoreContext } from './types.ts'; /** * Create a new KernelStore object wrapped around a raw kernel database. The @@ -172,6 +172,7 @@ export function makeKernelStore(kdb: KernelDatabase, logger?: Logger) { const crank = getCrankMethods(context, kdb); const subclusters = getSubclusterMethods(context); const activity = getActivityMethods(kv); + const relay = getRelayMethods({ kv, logger: context.logger }); /** * Create a new VatStore for a vat. @@ -286,46 +287,6 @@ export function makeKernelStore(kdb: KernelDatabase, logger?: Logger) { kdb.rollbackSavepoint(name); } - /** - * Read relay entries from storage, auto-migrating from the legacy - * `string[]` format if necessary. - * - * @returns The relay entries. - */ - function getRelayEntries(): RelayEntry[] { - const raw = kv.get('knownRelays'); - if (!raw) { - return []; - } - const parsed: unknown = JSON.parse(raw); - Array.isArray(parsed) || Fail`knownRelays must be an array`; - - // Migrate legacy string[] format → RelayEntry[] (persisted back to storage) - if (parsed.length > 0 && typeof parsed[0] === 'string') { - if (!parsed.every((entry: unknown) => typeof entry === 'string')) { - Fail`knownRelays legacy format must be all strings`; - } - const migrated: RelayEntry[] = (parsed as string[]).map((addr) => ({ - addr, - lastSeen: 0, - isBootstrap: false, - })); - kv.set('knownRelays', JSON.stringify(migrated)); - return migrated; - } - - // New RelayEntry[] format - for (const entry of parsed) { - (typeof entry === 'object' && - entry !== null && - typeof (entry as RelayEntry).addr === 'string' && - typeof (entry as RelayEntry).lastSeen === 'number' && - typeof (entry as RelayEntry).isBootstrap === 'boolean') || - Fail`knownRelays entries must have addr, lastSeen, isBootstrap`; - } - return parsed as RelayEntry[]; - } - return harden({ ...id, ...queue, @@ -343,6 +304,7 @@ export function makeKernelStore(kdb: KernelDatabase, logger?: Logger) { ...crank, ...subclusters, ...activity, + ...relay, makeVatStore, deleteVat, clear, @@ -372,43 +334,6 @@ export function makeKernelStore(kdb: KernelDatabase, logger?: Logger) { deleteKernelServiceKref(name: string): void { kv.delete(`kernelService.${name}`); }, - - // Remote identity - getRemoteIdentityValue( - key: 'peerId' | 'keySeed' | 'ocapURLKey', - ): string | undefined { - return kv.get(key); - }, - getRemoteIdentityValueRequired( - key: 'peerId' | 'keySeed' | 'ocapURLKey', - ): string { - return kv.getRequired(key); - }, - setRemoteIdentityValue( - key: 'peerId' | 'keySeed' | 'ocapURLKey', - value: string, - ): void { - kv.set(key, value); - }, - getRelayEntries, - - /** - * Persist relay entries to storage. - * - * @param entries - The relay entries to persist. - */ - setRelayEntries(entries: RelayEntry[]): void { - kv.set('knownRelays', JSON.stringify(entries)); - }, - - /** - * Convenience: return only relay addresses (for ConnectionFactory, etc.). - * - * @returns The relay addresses. - */ - getKnownRelayAddresses(): string[] { - return getRelayEntries().map((entry) => entry.addr); - }, }); } diff --git a/packages/ocap-kernel/src/store/methods/relay.test.ts b/packages/ocap-kernel/src/store/methods/relay.test.ts new file mode 100644 index 0000000000..527bf3b83d --- /dev/null +++ b/packages/ocap-kernel/src/store/methods/relay.test.ts @@ -0,0 +1,149 @@ +import type { Logger } from '@metamask/logger'; +import { describe, it, expect, vi } from 'vitest'; + +import { getRelayMethods } from './relay.ts'; +import { makeMapKVStore } from '../../../test/storage.ts'; + +function makeCtx(loggerOverride?: Logger) { + const kv = makeMapKVStore(); + const logger = + loggerOverride ?? + ({ + log: vi.fn(), + error: vi.fn(), + subLogger: vi.fn(), + } as unknown as Logger); + return { kv, logger, methods: getRelayMethods({ kv, logger }) }; +} + +describe('getRelayMethods', () => { + describe('getRelayEntries', () => { + it('returns empty array when no relays are stored', () => { + const { methods } = makeCtx(); + expect(methods.getRelayEntries()).toStrictEqual([]); + }); + + it('stores and retrieves relay entries', () => { + const { methods } = makeCtx(); + const entries = [ + { addr: 'relay1', lastSeen: 100, isBootstrap: true }, + { addr: 'relay2', lastSeen: 200, isBootstrap: false }, + ]; + methods.setRelayEntries(entries); + expect(methods.getRelayEntries()).toStrictEqual(entries); + }); + + it('auto-migrates legacy string[] format to RelayEntry[]', () => { + const { kv, methods } = makeCtx(); + kv.set('knownRelays', JSON.stringify(['peer1', 'peer2'])); + const entries = methods.getRelayEntries(); + expect(entries).toStrictEqual([ + { addr: 'peer1', lastSeen: 0, isBootstrap: false }, + { addr: 'peer2', lastSeen: 0, isBootstrap: false }, + ]); + // Migration should persist the new format + const raw = kv.get('knownRelays'); + expect(JSON.parse(raw as string)).toStrictEqual(entries); + }); + + it('logs migration event when migrating legacy format', () => { + const { kv, logger, methods } = makeCtx(); + kv.set('knownRelays', JSON.stringify(['peer1', 'peer2'])); + methods.getRelayEntries(); + expect(logger.log).toHaveBeenCalledWith( + 'Migrated 2 legacy relay entries to RelayEntry format', + ); + }); + + it('throws contextual error on corrupt JSON', () => { + const { kv, methods } = makeCtx(); + kv.set('knownRelays', '{bad json'); + expect(() => methods.getRelayEntries()).toThrow( + /Failed to parse knownRelays from store/u, + ); + }); + + it('throws when stored knownRelays is not a JSON array', () => { + const { kv, methods } = makeCtx(); + kv.set('knownRelays', '"not-an-array"'); + expect(() => methods.getRelayEntries()).toThrow( + 'knownRelays must be an array', + ); + }); + + it('throws on mixed legacy format', () => { + const { kv, methods } = makeCtx(); + kv.set( + 'knownRelays', + JSON.stringify([ + 'peer1', + { addr: 'peer2', lastSeen: 0, isBootstrap: false }, + ]), + ); + expect(() => methods.getRelayEntries()).toThrow( + 'knownRelays legacy format must be all strings', + ); + }); + + it('throws when relay entries have invalid shape', () => { + const { kv, methods } = makeCtx(); + kv.set( + 'knownRelays', + JSON.stringify([{ addr: 'ok', lastSeen: 'bad', isBootstrap: false }]), + ); + expect(() => methods.getRelayEntries()).toThrow( + 'knownRelays entries must have addr, lastSeen, isBootstrap', + ); + }); + }); + + describe('setRelayEntries', () => { + it('validates entries on write', () => { + const { methods } = makeCtx(); + expect(() => + methods.setRelayEntries([ + { addr: 123 as unknown as string, lastSeen: 0, isBootstrap: false }, + ]), + ).toThrow(/Invalid relay entry/u); + }); + }); + + describe('getKnownRelayAddresses', () => { + it('returns only addresses from relay entries', () => { + const { methods } = makeCtx(); + methods.setRelayEntries([ + { addr: 'relay1', lastSeen: 100, isBootstrap: true }, + { addr: 'relay2', lastSeen: 200, isBootstrap: false }, + ]); + expect(methods.getKnownRelayAddresses()).toStrictEqual([ + 'relay1', + 'relay2', + ]); + }); + + it('returns empty array when no relays stored', () => { + const { methods } = makeCtx(); + expect(methods.getKnownRelayAddresses()).toStrictEqual([]); + }); + }); + + describe('remote identity values', () => { + it('returns undefined for unset values', () => { + const { methods } = makeCtx(); + expect(methods.getRemoteIdentityValue('peerId')).toBeUndefined(); + }); + + it('stores and retrieves identity values', () => { + const { methods } = makeCtx(); + methods.setRemoteIdentityValue('peerId', 'test-peer-id'); + expect(methods.getRemoteIdentityValue('peerId')).toBe('test-peer-id'); + }); + + it('getRemoteIdentityValueRequired throws for missing values', () => { + const { methods } = makeCtx(); + expect(() => methods.getRemoteIdentityValueRequired('peerId')).toThrow( + /peerId/u, + ); + }); + }); +}); diff --git a/packages/ocap-kernel/src/store/methods/relay.ts b/packages/ocap-kernel/src/store/methods/relay.ts new file mode 100644 index 0000000000..489aafed53 --- /dev/null +++ b/packages/ocap-kernel/src/store/methods/relay.ts @@ -0,0 +1,163 @@ +import { Fail } from '@endo/errors'; +import type { KVStore } from '@metamask/kernel-store'; +import type { Logger } from '@metamask/logger'; +import { assert } from '@metamask/superstruct'; + +import type { RelayEntry } from '../types.ts'; +import { RelayEntryStruct } from '../types.ts'; + +/** + * Get methods for managing relay entries and remote identity values in the + * kernel store. + * + * @param ctx - Store context subset required by relay methods. + * @param ctx.kv - The key/value store for persistence. + * @param ctx.logger - Optional logger for migration and diagnostic events. + * @returns An object with relay and remote identity store methods. + */ +// eslint-disable-next-line @typescript-eslint/explicit-function-return-type +export function getRelayMethods(ctx: { + kv: KVStore; + logger?: Logger | undefined; +}) { + const { kv, logger } = ctx; + + /** + * Parse a JSON string, wrapping errors with a contextual message. + * + * @param raw - The raw JSON string to parse. + * @returns The parsed value. + */ + function parseStoredJSON(raw: string): unknown { + try { + return JSON.parse(raw); + } catch (error) { + throw Error( + `Failed to parse knownRelays from store (value may be corrupted): ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + } + + /** + * Read relay entries from storage, auto-migrating from the legacy + * `string[]` format if necessary. + * + * @returns The relay entries. + */ + function getRelayEntries(): RelayEntry[] { + const raw = kv.get('knownRelays'); + if (!raw) { + return []; + } + + const parsed: unknown = parseStoredJSON(raw); + if (!Array.isArray(parsed)) { + // eslint-disable-next-line @typescript-eslint/only-throw-error -- Fail returns never; throw is needed for TS narrowing + throw Fail`knownRelays must be an array`; + } + + // Migrate legacy string[] format -> RelayEntry[] (persisted back to storage) + if (parsed.length > 0 && typeof parsed[0] === 'string') { + if (!parsed.every((entry: unknown) => typeof entry === 'string')) { + Fail`knownRelays legacy format must be all strings`; + } + const migrated: RelayEntry[] = (parsed as string[]).map((addr) => ({ + addr, + lastSeen: 0, + isBootstrap: false, + })); + kv.set('knownRelays', JSON.stringify(migrated)); + logger?.log( + `Migrated ${migrated.length} legacy relay entries to RelayEntry format`, + ); + return migrated; + } + + // Validate each entry against the RelayEntry schema (entries deserialized + // from storage may have been written by an older version or corrupted) + for (const entry of parsed) { + (typeof entry === 'object' && + entry !== null && + typeof (entry as RelayEntry).addr === 'string' && + typeof (entry as RelayEntry).lastSeen === 'number' && + typeof (entry as RelayEntry).isBootstrap === 'boolean') || + Fail`knownRelays entries must have addr, lastSeen, isBootstrap`; + } + return parsed as RelayEntry[]; + } + + /** + * Persist relay entries to storage. Validates each entry against the + * {@link RelayEntryStruct} schema before writing. Callers are responsible + * for enforcing pool caps (e.g. `maxKnownRelays`) before calling this + * method. + * + * @param entries - The relay entries to persist. + */ + function setRelayEntries(entries: RelayEntry[]): void { + for (const entry of entries) { + assert(entry, RelayEntryStruct, 'Invalid relay entry'); + } + kv.set('knownRelays', JSON.stringify(entries)); + } + + /** + * Convenience: return only relay addresses (for ConnectionFactory, etc.). + * + * @returns The relay addresses. + */ + function getKnownRelayAddresses(): string[] { + return getRelayEntries().map((entry) => entry.addr); + } + + // Remote identity KV accessors + + /** + * Get a remote identity value from the store. + * + * @param key - The identity key to retrieve. + * @returns The stored value, or undefined if not set. + */ + function getRemoteIdentityValue( + key: 'peerId' | 'keySeed' | 'ocapURLKey', + ): string | undefined { + return kv.get(key); + } + + /** + * Get a required remote identity value from the store. + * + * @param key - The identity key to retrieve. + * @returns The stored value. + * @throws If the key is not set. + */ + function getRemoteIdentityValueRequired( + key: 'peerId' | 'keySeed' | 'ocapURLKey', + ): string { + return kv.getRequired(key); + } + + /** + * Set a remote identity value in the store. + * + * @param key - The identity key to set. + * @param value - The value to store. + */ + function setRemoteIdentityValue( + key: 'peerId' | 'keySeed' | 'ocapURLKey', + value: string, + ): void { + kv.set(key, value); + } + + return { + getRelayEntries, + setRelayEntries, + getKnownRelayAddresses, + getRemoteIdentityValue, + getRemoteIdentityValueRequired, + setRemoteIdentityValue, + }; +} diff --git a/packages/ocap-kernel/src/store/types.ts b/packages/ocap-kernel/src/store/types.ts index 8b404aa3b2..abbcfc14b8 100644 --- a/packages/ocap-kernel/src/store/types.ts +++ b/packages/ocap-kernel/src/store/types.ts @@ -1,5 +1,6 @@ import type { KVStore } from '@metamask/kernel-store'; import type { Logger } from '@metamask/logger'; +import { object, string, number, boolean } from '@metamask/superstruct'; import type { KRef, RunQueueItemNotify, RunQueueItemSend } from '../types.ts'; @@ -48,13 +49,32 @@ export type VatCleanupWork = { kv: number; }; +/** + * Superstruct schema for {@link RelayEntry}, used for runtime validation on + * both read and write paths of the relay store. + */ +export const RelayEntryStruct = object({ + addr: string(), + lastSeen: number(), + isBootstrap: boolean(), +}); + /** * A relay entry with metadata for prioritized selection and bounded storage. + * + * Migration from the legacy `string[]` format is handled by `getRelayEntries()` + * in `store/methods/relay.ts`. The migration path can be removed once all + * deployed kernels have been initialized at least once on a version that + * includes RelayEntry support. */ export type RelayEntry = { /** Relay multiaddr string. */ addr: string; - /** Epoch ms when the relay was last added or re-observed. */ + /** + * Epoch ms when the relay was last added or re-observed. A value of `0` + * indicates a legacy-migrated entry with unknown observation time; these + * sort last during recency-based eviction. + */ lastSeen: number; /** True if provided at kernel initialization (prioritized during eviction). */ isBootstrap: boolean; From db629f9105ef5c93aa7cab434b84ffde7e7ca981 Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Mon, 13 Apr 2026 13:37:22 +0200 Subject: [PATCH 5/6] fix(ocap-kernel): harden relay management, tighten validation, fix docs Address review findings across the bounded relay management feature: tighten RelayEntryStruct to reject NaN/Infinity/negative/empty values, unify read/write validation paths, harden initRemoteIdentity return value, add defensive cap validation, prevent mnemonic leaking to platform services, simplify RPC handler and options passing, fix inaccurate SES/hardened-object comments and stale JSDoc references, and add missing test coverage for edge cases. Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/ocap-kernel/CHANGELOG.md | 8 +- .../src/remotes/kernel/remote-comms.test.ts | 63 +++++++++++++++- .../src/remotes/kernel/remote-comms.ts | 74 ++++++++++++------- packages/ocap-kernel/src/remotes/types.ts | 6 +- .../rpc/kernel-control/init-remote-comms.ts | 29 ++------ .../src/store/methods/relay.test.ts | 28 ++++++- .../ocap-kernel/src/store/methods/relay.ts | 11 +-- packages/ocap-kernel/src/store/types.ts | 10 ++- 8 files changed, 162 insertions(+), 67 deletions(-) diff --git a/packages/ocap-kernel/CHANGELOG.md b/packages/ocap-kernel/CHANGELOG.md index 19d5c4206e..e3d26e6726 100644 --- a/packages/ocap-kernel/CHANGELOG.md +++ b/packages/ocap-kernel/CHANGELOG.md @@ -7,14 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### Fixed - -- Deserialize CapData rejections in `Kernel.queueMessage` so vat errors surface as plain `Error` objects to all callers ([#928](https://github.com/MetaMask/ocap-kernel/pull/928)) - ### Changed - Bound relay hints in OCAP URLs to a maximum of 3 and cap the relay pool at 20 entries with eviction of oldest non-bootstrap relays ([#929](https://github.com/MetaMask/ocap-kernel/pull/929)) +### Fixed + +- Deserialize CapData rejections in `Kernel.queueMessage` so vat errors surface as plain `Error` objects to all callers ([#928](https://github.com/MetaMask/ocap-kernel/pull/928)) + ## [0.7.0] ### Added diff --git a/packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts b/packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts index 2ad8ee6858..e682b1f305 100644 --- a/packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts +++ b/packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts @@ -581,7 +581,8 @@ describe('remote-comms', () => { identity.addKnownRelays(['/dns4/relay.example/tcp/443/wss/p2p-circuit']); const firstSeen = mockKernelStore.getRelayEntries()[0]?.lastSeen ?? 0; - // SES lockdown prevents mocking Date.now; use a real delay instead + // SES lockdown freezes Date, preventing vi.useFakeTimers(); use a + // real delay to guarantee a different Date.now() value. await new Promise((resolve) => { setTimeout(resolve, 50); }); @@ -591,6 +592,19 @@ describe('remote-comms', () => { expect(secondSeen).toBeGreaterThan(firstSeen); }); + it('addKnownRelays does not add duplicate entries on re-observation', async () => { + const { identity } = await initRemoteIdentity(mockKernelStore); + + const relay = '/dns4/relay.example/tcp/443/wss/p2p-circuit'; + identity.addKnownRelays([relay]); + expect(mockKernelStore.getRelayEntries()).toHaveLength(1); + + // Re-add the same relay — entry count must stay at 1 + identity.addKnownRelays([relay, relay]); + expect(mockKernelStore.getRelayEntries()).toHaveLength(1); + expect(mockKernelStore.getRelayEntries()[0]?.addr).toBe(relay); + }); + it('init clears bootstrap flag on relays removed from the bootstrap set', async () => { await initRemoteIdentity(mockKernelStore, { relays: ['/dns4/relayA.example/tcp/443/wss/p2p-circuit'], @@ -775,6 +789,53 @@ describe('remote-comms', () => { ); }); + it('init truncates pool when bootstrap count exceeds maxKnownRelays', async () => { + const bootstrapRelays = Array.from( + { length: 5 }, + (_, i) => `/dns4/bootstrap${i}.example/tcp/443/wss/p2p-circuit`, + ); + await initRemoteIdentity(mockKernelStore, { + relays: bootstrapRelays, + maxKnownRelays: 3, + }); + + const entries = mockKernelStore.getRelayEntries(); + // Pool must be capped at maxKnownRelays even when all entries are bootstrap + expect(entries).toHaveLength(3); + // All surviving entries should still be marked as bootstrap + expect(entries.every((entry) => entry.isBootstrap)).toBe(true); + }); + + it('issueOcapURL includes all relays when count equals maxUrlRelayHints', async () => { + const relays = Array.from( + { length: DEFAULT_MAX_URL_RELAY_HINTS }, + (_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`, + ); + const { identity } = await initRemoteIdentity(mockKernelStore, { + relays, + }); + const ocapURL = await identity.issueOcapURL('ko1' as KRef); + const { hints } = parseOcapURL(ocapURL); + expect(hints).toHaveLength(DEFAULT_MAX_URL_RELAY_HINTS); + expect(hints).toStrictEqual(relays); + }); + + it.each([ + ['maxUrlRelayHints', { maxUrlRelayHints: 0 }], + ['maxUrlRelayHints', { maxUrlRelayHints: 1.5 }], + ['maxUrlRelayHints', { maxUrlRelayHints: -1 }], + ['maxKnownRelays', { maxKnownRelays: 0 }], + ['maxKnownRelays', { maxKnownRelays: 1.5 }], + ['maxKnownRelays', { maxKnownRelays: -1 }], + ] as const)( + 'throws when %s is not a positive integer (%o)', + async (_label, opts) => { + await expect(initRemoteIdentity(mockKernelStore, opts)).rejects.toThrow( + /must be positive integers/u, + ); + }, + ); + it('throws with mnemonic when identity already exists', async () => { mockKernelStore.setRemoteIdentityValue('peerId', 'existing-peer-id'); mockKernelStore.setRemoteIdentityValue( diff --git a/packages/ocap-kernel/src/remotes/kernel/remote-comms.ts b/packages/ocap-kernel/src/remotes/kernel/remote-comms.ts index 2a99342118..4c964ae7a4 100644 --- a/packages/ocap-kernel/src/remotes/kernel/remote-comms.ts +++ b/packages/ocap-kernel/src/remotes/kernel/remote-comms.ts @@ -39,8 +39,10 @@ export const DEFAULT_MAX_URL_RELAY_HINTS = 3; export const DEFAULT_MAX_KNOWN_RELAYS = 20; /** - * Enforce the relay pool cap by keeping all bootstrap entries and the newest - * non-bootstrap entries, up to `cap`. + * Enforce the relay pool cap by prioritizing bootstrap entries, then the + * newest non-bootstrap entries, up to `cap`. If bootstrap entries alone + * exceed the cap, the result is truncated to `cap` (some bootstrap entries + * may be dropped). * * @param entries - The full set of relay entries. * @param cap - The maximum pool size. @@ -118,14 +120,14 @@ async function generateKeyInfo(seedString?: string): Promise<[string, string]> { * operations) without starting any network communications. * * @param kernelStore - The kernel store, for storing persistent key info. - * @param options - Options for identity initialization. - * @param options.relays - Bootstrap relay addresses. These are merged into + * @param [options] - Options for identity initialization. + * @param [options.relays] - Bootstrap relay addresses. These are merged into * the relay pool, marked as bootstrap (prioritized during eviction and URL * selection), and persisted. Relays previously marked as bootstrap that are * no longer in this list have their bootstrap flag cleared. - * @param options.mnemonic - BIP39 mnemonic for seed recovery. - * @param options.maxUrlRelayHints - Cap on relay hints per OCAP URL. - * @param options.maxKnownRelays - Cap on the stored relay pool. + * @param [options.mnemonic] - BIP39 mnemonic for seed recovery. + * @param [options.maxUrlRelayHints] - Cap on relay hints per OCAP URL. + * @param [options.maxKnownRelays] - Cap on the stored relay pool. * @param logger - The logger to use. * @param keySeed - Optional seed for key generation. * @returns the identity object, the key seed, and known relays. @@ -155,8 +157,20 @@ export async function initRemoteIdentity( options?.maxUrlRelayHints ?? DEFAULT_MAX_URL_RELAY_HINTS; const maxKnownRelays = options?.maxKnownRelays ?? DEFAULT_MAX_KNOWN_RELAYS; + if ( + !Number.isInteger(maxUrlRelayHints) || + maxUrlRelayHints < 1 || + !Number.isInteger(maxKnownRelays) || + maxKnownRelays < 1 + ) { + throw Error( + `maxUrlRelayHints (${maxUrlRelayHints}) and maxKnownRelays (${maxKnownRelays}) must be positive integers`, + ); + } + if (relays.length > 0) { - // Date.now() is available under SES lockdown (on the intrinsics allow-list). + // Date.now() works here because this code runs in the start compartment, + // which retains the original Date constructor (%InitialDate%). const now = Date.now(); const bootstrapSet = new Set(relays); @@ -174,7 +188,7 @@ export async function initRemoteIdentity( byAddr.set(addr, { addr, lastSeen: now, isBootstrap: true }); } // Clear bootstrap flag on entries no longer in the current bootstrap set - // (create new objects to avoid mutating potentially-hardened entries) + // (create new objects to follow immutability conventions) for (const [addr, entry] of byAddr.entries()) { if (entry.isBootstrap && !bootstrapSet.has(addr)) { byAddr.set(addr, { ...entry, isBootstrap: false }); @@ -269,7 +283,8 @@ export async function initRemoteIdentity( /** * Add relay addresses to the kernel's known relay pool. * Deduplicates, updates lastSeen on re-observation, and enforces - * {@link MAX_KNOWN_RELAYS} by evicting the oldest non-bootstrap entries. + * the configured `maxKnownRelays` cap by evicting the oldest + * non-bootstrap entries. * * @param newRelays - Relay multiaddrs to add. */ @@ -277,7 +292,8 @@ export async function initRemoteIdentity( if (newRelays.length === 0) { return; } - // Date.now() is available under SES lockdown (on the intrinsics allow-list). + // Date.now() works here because this code runs in the start compartment, + // which retains the original Date constructor (%InitialDate%). const now = Date.now(); const existing = kernelStore.getRelayEntries(); const byAddr = new Map(existing.map((entry) => [entry.addr, entry])); @@ -286,8 +302,8 @@ export async function initRemoteIdentity( for (const addr of newRelays) { const entry = byAddr.get(addr); if (entry) { - // Update lastSeen on re-observation (create new object to avoid - // mutating potentially-hardened deserialized entries) + // Update lastSeen on re-observation (create new object to follow + // immutability conventions) if (entry.lastSeen !== now) { byAddr.set(addr, { ...entry, lastSeen: now }); changed = true; @@ -339,11 +355,16 @@ export async function initRemoteIdentity( return kref; } - return { - identity: { getPeerId, issueOcapURL, redeemLocalOcapURL, addKnownRelays }, + return harden({ + identity: harden({ + getPeerId, + issueOcapURL, + redeemLocalOcapURL, + addKnownRelays, + }), keySeed, knownRelays: kernelStore.getKnownRelayAddresses(), - }; + }); } /** @@ -352,7 +373,7 @@ export async function initRemoteIdentity( * @param kernelStore - The kernel store, for storing persistent key info. * @param platformServices - The platform services, for accessing network I/O * operations that are not available within the web worker that the kernel runs in. - * @param remoteMessageHandler - Handler to process received inbound communcations. + * @param remoteMessageHandler - Handler to process received inbound communications. * @param options - Options for remote communications initialization. * @param logger - The logger to use. * @param keySeed - Optional seed for libp2p key generation. @@ -373,16 +394,17 @@ export async function initRemoteComms( incarnationId?: string, onIncarnationChange?: OnIncarnationChange, ): Promise { - const { relays = [], mnemonic, maxUrlRelayHints, maxKnownRelays } = options; + const { + relays = [], + mnemonic, + maxUrlRelayHints, + maxKnownRelays, + ...platformOptions + } = options; const result = await initRemoteIdentity( kernelStore, - { - relays, - ...(mnemonic === undefined ? {} : { mnemonic }), - maxUrlRelayHints, - maxKnownRelays, - }, + { relays, mnemonic, maxUrlRelayHints, maxKnownRelays }, logger, keySeed, ); @@ -395,9 +417,11 @@ export async function initRemoteComms( // called before initializeRemoteComms to capture the pre-restart timestamp. const wakeDetected = kernelStore.detectWake(); + // Omit mnemonic (sensitive key material) from the options passed to + // platform services, which only needs network-level configuration. await platformServices.initializeRemoteComms( result.keySeed, - { ...options, relays: knownRelays }, + { ...platformOptions, relays: knownRelays }, remoteMessageHandler, onRemoteGiveUp, incarnationId, diff --git a/packages/ocap-kernel/src/remotes/types.ts b/packages/ocap-kernel/src/remotes/types.ts index 23e52ec096..8ae083af5d 100644 --- a/packages/ocap-kernel/src/remotes/types.ts +++ b/packages/ocap-kernel/src/remotes/types.ts @@ -148,8 +148,10 @@ export type RemoteCommsOptions = { maxUrlRelayHints?: number | undefined; /** * Maximum number of relay entries stored in the kernel's relay pool - * (default: 20). The pool retains all bootstrap relays; when the cap is - * reached, the oldest non-bootstrap (learned) entries are evicted first. + * (default: 20). Bootstrap relays are prioritized during eviction; when + * the cap is reached, the oldest non-bootstrap (learned) entries are + * evicted first. If bootstrap relays alone exceed the cap, the pool is + * truncated to the cap. */ maxKnownRelays?: number | undefined; /** diff --git a/packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.ts b/packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.ts index eef2656984..b023cc4936 100644 --- a/packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.ts +++ b/packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.ts @@ -59,28 +59,13 @@ export const initRemoteCommsHandler: Handler< ...initRemoteCommsSpec, hooks: { kernel: true }, implementation: async ({ kernel }, params): Promise => { - const options: RemoteCommsOptions = {}; - if (params.relays !== undefined) { - options.relays = params.relays; - } - if (params.directListenAddresses !== undefined) { - options.directListenAddresses = params.directListenAddresses; - } - if (params.maxRetryAttempts !== undefined) { - options.maxRetryAttempts = params.maxRetryAttempts; - } - if (params.maxQueue !== undefined) { - options.maxQueue = params.maxQueue; - } - if (params.maxUrlRelayHints !== undefined) { - options.maxUrlRelayHints = params.maxUrlRelayHints; - } - if (params.maxKnownRelays !== undefined) { - options.maxKnownRelays = params.maxKnownRelays; - } - if (params.allowedWsHosts !== undefined) { - options.allowedWsHosts = params.allowedWsHosts; - } + // Build options from only the defined RPC params. Superstruct has + // already validated the shape; stripping undefined keeps the bag sparse. + // Note: sensitive fields like `mnemonic` and internal fields like + // `directTransports` are intentionally excluded from the RPC struct. + const options: RemoteCommsOptions = Object.fromEntries( + Object.entries(params).filter(([, value]) => value !== undefined), + ); await kernel.initRemoteComms(options); return null; }, diff --git a/packages/ocap-kernel/src/store/methods/relay.test.ts b/packages/ocap-kernel/src/store/methods/relay.test.ts index 527bf3b83d..356114a212 100644 --- a/packages/ocap-kernel/src/store/methods/relay.test.ts +++ b/packages/ocap-kernel/src/store/methods/relay.test.ts @@ -91,9 +91,13 @@ describe('getRelayMethods', () => { 'knownRelays', JSON.stringify([{ addr: 'ok', lastSeen: 'bad', isBootstrap: false }]), ); - expect(() => methods.getRelayEntries()).toThrow( - 'knownRelays entries must have addr, lastSeen, isBootstrap', - ); + expect(() => methods.getRelayEntries()).toThrow(/Invalid stored relay/u); + }); + + it('returns empty array when stored value is an empty JSON array', () => { + const { kv, methods } = makeCtx(); + kv.set('knownRelays', '[]'); + expect(methods.getRelayEntries()).toStrictEqual([]); }); }); @@ -106,6 +110,24 @@ describe('getRelayMethods', () => { ]), ).toThrow(/Invalid relay entry/u); }); + + it.each([ + ['empty addr', { addr: '', lastSeen: 0, isBootstrap: false }], + [ + 'negative lastSeen', + { addr: 'relay1', lastSeen: -1, isBootstrap: false }, + ], + ['NaN lastSeen', { addr: 'relay1', lastSeen: NaN, isBootstrap: false }], + [ + 'Infinity lastSeen', + { addr: 'relay1', lastSeen: Infinity, isBootstrap: false }, + ], + ])('rejects entry with %s', (_label, entry) => { + const { methods } = makeCtx(); + expect(() => methods.setRelayEntries([entry])).toThrow( + /Invalid relay entry/u, + ); + }); }); describe('getKnownRelayAddresses', () => { diff --git a/packages/ocap-kernel/src/store/methods/relay.ts b/packages/ocap-kernel/src/store/methods/relay.ts index 489aafed53..b499d3ac56 100644 --- a/packages/ocap-kernel/src/store/methods/relay.ts +++ b/packages/ocap-kernel/src/store/methods/relay.ts @@ -23,7 +23,8 @@ export function getRelayMethods(ctx: { const { kv, logger } = ctx; /** - * Parse a JSON string, wrapping errors with a contextual message. + * Parse the knownRelays JSON string from storage, wrapping errors with + * a contextual message. * * @param raw - The raw JSON string to parse. * @returns The parsed value. @@ -36,6 +37,7 @@ export function getRelayMethods(ctx: { `Failed to parse knownRelays from store (value may be corrupted): ${ error instanceof Error ? error.message : String(error) }`, + { cause: error }, ); } } @@ -78,12 +80,7 @@ export function getRelayMethods(ctx: { // Validate each entry against the RelayEntry schema (entries deserialized // from storage may have been written by an older version or corrupted) for (const entry of parsed) { - (typeof entry === 'object' && - entry !== null && - typeof (entry as RelayEntry).addr === 'string' && - typeof (entry as RelayEntry).lastSeen === 'number' && - typeof (entry as RelayEntry).isBootstrap === 'boolean') || - Fail`knownRelays entries must have addr, lastSeen, isBootstrap`; + assert(entry, RelayEntryStruct, 'Invalid stored relay entry'); } return parsed as RelayEntry[]; } diff --git a/packages/ocap-kernel/src/store/types.ts b/packages/ocap-kernel/src/store/types.ts index abbcfc14b8..1ea54d19cc 100644 --- a/packages/ocap-kernel/src/store/types.ts +++ b/packages/ocap-kernel/src/store/types.ts @@ -1,6 +1,6 @@ import type { KVStore } from '@metamask/kernel-store'; import type { Logger } from '@metamask/logger'; -import { object, string, number, boolean } from '@metamask/superstruct'; +import { object, string, number, boolean, refine } from '@metamask/superstruct'; import type { KRef, RunQueueItemNotify, RunQueueItemSend } from '../types.ts'; @@ -54,8 +54,12 @@ export type VatCleanupWork = { * both read and write paths of the relay store. */ export const RelayEntryStruct = object({ - addr: string(), - lastSeen: number(), + addr: refine(string(), 'non-empty string', (value) => value.length > 0), + lastSeen: refine( + number(), + 'non-negative finite number', + (value) => Number.isFinite(value) && value >= 0, + ), isBootstrap: boolean(), }); From 9bbc7e92d78cdee4f57ca6e53be2907fb33c352c Mon Sep 17 00:00:00 2001 From: Dimitris Marlagkoutsos Date: Mon, 13 Apr 2026 18:12:08 +0200 Subject: [PATCH 6/6] refactor(ocap-kernel): use ifDefined utility in RPC handler Replace inline Object.fromEntries filter with the existing ifDefined utility from @metamask/kernel-utils, as suggested in PR review. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/rpc/kernel-control/init-remote-comms.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.ts b/packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.ts index b023cc4936..1dd518e88d 100644 --- a/packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.ts +++ b/packages/ocap-kernel/src/rpc/kernel-control/init-remote-comms.ts @@ -1,4 +1,5 @@ import type { MethodSpec, Handler } from '@metamask/kernel-rpc-methods'; +import { ifDefined } from '@metamask/kernel-utils'; import { object, literal, @@ -59,13 +60,11 @@ export const initRemoteCommsHandler: Handler< ...initRemoteCommsSpec, hooks: { kernel: true }, implementation: async ({ kernel }, params): Promise => { - // Build options from only the defined RPC params. Superstruct has - // already validated the shape; stripping undefined keeps the bag sparse. - // Note: sensitive fields like `mnemonic` and internal fields like - // `directTransports` are intentionally excluded from the RPC struct. - const options: RemoteCommsOptions = Object.fromEntries( - Object.entries(params).filter(([, value]) => value !== undefined), - ); + // Strip undefined values so the options bag is sparse. Superstruct has + // already validated the shape. Sensitive fields like `mnemonic` and + // internal fields like `directTransports` are intentionally excluded + // from the RPC struct. + const options: RemoteCommsOptions = ifDefined(params); await kernel.initRemoteComms(options); return null; },