Skip to content

Commit 1dd4bc9

Browse files
sirtimidclaude
andcommitted
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) <noreply@anthropic.com>
1 parent 1690fd8 commit 1dd4bc9

10 files changed

Lines changed: 619 additions & 179 deletions

File tree

packages/ocap-kernel/src/remotes/kernel/remote-comms.test.ts

Lines changed: 127 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import {
88
initRemoteIdentity,
99
initRemoteComms,
1010
parseOcapURL,
11-
MAX_URL_RELAY_HINTS,
12-
MAX_KNOWN_RELAYS,
11+
DEFAULT_MAX_URL_RELAY_HINTS,
12+
DEFAULT_MAX_KNOWN_RELAYS,
1313
} from './remote-comms.ts';
1414
import { createMockRemotesFactory } from '../../../test/remotes-mocks.ts';
1515
import { makeMapKernelDatabase } from '../../../test/storage.ts';
@@ -99,7 +99,7 @@ describe('remote-comms', () => {
9999
const { oid, hints } = parseOcapURL(ocapURL);
100100
const knownRelayAddresses = mockKernelStore.getKnownRelayAddresses();
101101
expect(knownRelayAddresses).toStrictEqual(testRelays);
102-
// URL should embed the relays (within MAX_URL_RELAY_HINTS cap)
102+
// URL should embed the relays (within DEFAULT_MAX_URL_RELAY_HINTS cap)
103103
expect(hints).toStrictEqual(testRelays);
104104
const referenceURL = `ocap:${oid}@${peerId},${testRelays.join(',')}`;
105105
expect(ocapURL).toBe(referenceURL);
@@ -499,9 +499,9 @@ describe('remote-comms', () => {
499499
expect(stored).toStrictEqual(initialRelays);
500500
});
501501

502-
it('issueOcapURL caps relay hints to MAX_URL_RELAY_HINTS', async () => {
502+
it('issueOcapURL caps relay hints to DEFAULT_MAX_URL_RELAY_HINTS', async () => {
503503
const relays = Array.from(
504-
{ length: MAX_URL_RELAY_HINTS + 3 },
504+
{ length: DEFAULT_MAX_URL_RELAY_HINTS + 3 },
505505
(_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`,
506506
);
507507
const { identity } = await initRemoteIdentity(mockKernelStore, {
@@ -510,7 +510,7 @@ describe('remote-comms', () => {
510510

511511
const ocapURL = await identity.issueOcapURL('ko1' as KRef);
512512
const { hints } = parseOcapURL(ocapURL);
513-
expect(hints).toHaveLength(MAX_URL_RELAY_HINTS);
513+
expect(hints).toHaveLength(DEFAULT_MAX_URL_RELAY_HINTS);
514514
});
515515

516516
it('issueOcapURL prefers bootstrap relays over learned relays', async () => {
@@ -531,25 +531,25 @@ describe('remote-comms', () => {
531531

532532
const ocapURL = await identity.issueOcapURL('ko1' as KRef);
533533
const { hints } = parseOcapURL(ocapURL);
534-
expect(hints).toHaveLength(MAX_URL_RELAY_HINTS);
534+
expect(hints).toHaveLength(DEFAULT_MAX_URL_RELAY_HINTS);
535535
// Bootstrap relays should be included ahead of learned relays
536536
for (const bootstrap of bootstrapRelays) {
537537
expect(hints).toContain(bootstrap);
538538
}
539539
});
540540

541-
it('addKnownRelays enforces MAX_KNOWN_RELAYS pool cap', async () => {
541+
it('addKnownRelays enforces DEFAULT_MAX_KNOWN_RELAYS pool cap', async () => {
542542
const { identity } = await initRemoteIdentity(mockKernelStore);
543543

544544
// Add more relays than the cap
545545
const relays = Array.from(
546-
{ length: MAX_KNOWN_RELAYS + 5 },
546+
{ length: DEFAULT_MAX_KNOWN_RELAYS + 5 },
547547
(_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`,
548548
);
549549
identity.addKnownRelays(relays);
550550

551551
const entries = mockKernelStore.getRelayEntries();
552-
expect(entries).toHaveLength(MAX_KNOWN_RELAYS);
552+
expect(entries).toHaveLength(DEFAULT_MAX_KNOWN_RELAYS);
553553
});
554554

555555
it('addKnownRelays evicts oldest non-bootstrap relays when pool is full', async () => {
@@ -562,13 +562,13 @@ describe('remote-comms', () => {
562562

563563
// Fill pool to the cap
564564
const fillerRelays = Array.from(
565-
{ length: MAX_KNOWN_RELAYS },
565+
{ length: DEFAULT_MAX_KNOWN_RELAYS },
566566
(_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`,
567567
);
568568
identity.addKnownRelays(fillerRelays);
569569

570570
const entries = mockKernelStore.getRelayEntries();
571-
expect(entries).toHaveLength(MAX_KNOWN_RELAYS);
571+
expect(entries).toHaveLength(DEFAULT_MAX_KNOWN_RELAYS);
572572
// Bootstrap relay must survive eviction
573573
expect(entries.some((entry) => entry.addr === bootstrapRelays[0])).toBe(
574574
true,
@@ -610,10 +610,10 @@ describe('remote-comms', () => {
610610
).toBe(true);
611611
});
612612

613-
it('init enforces MAX_KNOWN_RELAYS pool cap', async () => {
614-
// Pre-seed MAX_KNOWN_RELAYS learned relays
613+
it('init enforces DEFAULT_MAX_KNOWN_RELAYS pool cap', async () => {
614+
// Pre-seed DEFAULT_MAX_KNOWN_RELAYS learned relays
615615
mockKernelStore.setRelayEntries(
616-
Array.from({ length: MAX_KNOWN_RELAYS }, (_, i) => ({
616+
Array.from({ length: DEFAULT_MAX_KNOWN_RELAYS }, (_, i) => ({
617617
addr: `/dns4/learned${i}.example/tcp/443/wss/p2p-circuit`,
618618
lastSeen: 100,
619619
isBootstrap: false,
@@ -627,7 +627,7 @@ describe('remote-comms', () => {
627627
await initRemoteIdentity(mockKernelStore, { relays: bootstrapRelays });
628628

629629
const entries = mockKernelStore.getRelayEntries();
630-
expect(entries).toHaveLength(MAX_KNOWN_RELAYS);
630+
expect(entries).toHaveLength(DEFAULT_MAX_KNOWN_RELAYS);
631631
// All bootstrap relays must survive
632632
for (const addr of bootstrapRelays) {
633633
expect(entries.some((entry) => entry.addr === addr)).toBe(true);
@@ -664,6 +664,117 @@ describe('remote-comms', () => {
664664
).toBe(false);
665665
});
666666

667+
it('logs when bootstrap relay count exceeds maxKnownRelays', async () => {
668+
const mockLogger = { log: vi.fn(), error: vi.fn() };
669+
const relays = Array.from(
670+
{ length: 5 },
671+
(_, i) => `/dns4/bootstrap${i}.example/tcp/443/wss/p2p-circuit`,
672+
);
673+
await initRemoteIdentity(
674+
mockKernelStore,
675+
{ relays, maxKnownRelays: 3 },
676+
mockLogger as unknown as Logger,
677+
);
678+
expect(mockLogger.log).toHaveBeenCalledWith(
679+
expect.stringContaining(
680+
'bootstrap relay count (5) exceeds maxKnownRelays (3)',
681+
),
682+
);
683+
});
684+
685+
it('logs eviction count when pool cap is exceeded during init', async () => {
686+
const mockLogger = { log: vi.fn(), error: vi.fn() };
687+
mockKernelStore.setRelayEntries(
688+
Array.from({ length: 20 }, (_, i) => ({
689+
addr: `/dns4/learned${i}.example/tcp/443/wss/p2p-circuit`,
690+
lastSeen: 100,
691+
isBootstrap: false,
692+
})),
693+
);
694+
await initRemoteIdentity(
695+
mockKernelStore,
696+
{
697+
relays: ['/dns4/bootstrap.example/tcp/443/wss/p2p-circuit'],
698+
maxKnownRelays: 10,
699+
},
700+
mockLogger as unknown as Logger,
701+
);
702+
expect(mockLogger.log).toHaveBeenCalledWith(
703+
expect.stringContaining('evicted'),
704+
);
705+
});
706+
707+
it('sorts same-tier relays by lastSeen descending in URL hints', async () => {
708+
mockKernelStore.setRelayEntries([
709+
{
710+
addr: '/dns4/relay-old.example/tcp/443/wss/p2p-circuit',
711+
lastSeen: 100,
712+
isBootstrap: false,
713+
},
714+
{
715+
addr: '/dns4/relay-new.example/tcp/443/wss/p2p-circuit',
716+
lastSeen: 300,
717+
isBootstrap: false,
718+
},
719+
{
720+
addr: '/dns4/relay-mid.example/tcp/443/wss/p2p-circuit',
721+
lastSeen: 200,
722+
isBootstrap: false,
723+
},
724+
]);
725+
const { identity } = await initRemoteIdentity(mockKernelStore);
726+
const ocapURL = await identity.issueOcapURL('ko1' as KRef);
727+
const { hints } = parseOcapURL(ocapURL);
728+
expect(hints).toStrictEqual([
729+
'/dns4/relay-new.example/tcp/443/wss/p2p-circuit',
730+
'/dns4/relay-mid.example/tcp/443/wss/p2p-circuit',
731+
'/dns4/relay-old.example/tcp/443/wss/p2p-circuit',
732+
]);
733+
});
734+
735+
it('respects custom maxUrlRelayHints option', async () => {
736+
const relays = Array.from(
737+
{ length: 10 },
738+
(_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`,
739+
);
740+
const { identity } = await initRemoteIdentity(mockKernelStore, {
741+
relays,
742+
maxUrlRelayHints: 5,
743+
});
744+
const ocapURL = await identity.issueOcapURL('ko1' as KRef);
745+
const { hints } = parseOcapURL(ocapURL);
746+
expect(hints).toHaveLength(5);
747+
});
748+
749+
it('respects custom maxKnownRelays option', async () => {
750+
const { identity } = await initRemoteIdentity(mockKernelStore, {
751+
maxKnownRelays: 5,
752+
});
753+
const relays = Array.from(
754+
{ length: 10 },
755+
(_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`,
756+
);
757+
identity.addKnownRelays(relays);
758+
expect(mockKernelStore.getRelayEntries()).toHaveLength(5);
759+
});
760+
761+
it('addKnownRelays logs eviction when pool cap is exceeded', async () => {
762+
const mockLogger = { log: vi.fn(), error: vi.fn() };
763+
const { identity } = await initRemoteIdentity(
764+
mockKernelStore,
765+
{ maxKnownRelays: 5 },
766+
mockLogger as unknown as Logger,
767+
);
768+
const relays = Array.from(
769+
{ length: 10 },
770+
(_, i) => `/dns4/relay${i}.example/tcp/443/wss/p2p-circuit`,
771+
);
772+
identity.addKnownRelays(relays);
773+
expect(mockLogger.log).toHaveBeenCalledWith(
774+
expect.stringContaining('evicted'),
775+
);
776+
});
777+
667778
it('throws with mnemonic when identity already exists', async () => {
668779
mockKernelStore.setRemoteIdentityValue('peerId', 'existing-peer-id');
669780
mockKernelStore.setRemoteIdentityValue(

0 commit comments

Comments
 (0)