Refactor remote location hint handling (#712)

This set of changes refactors how remote location hints are handled. Previously, hints were carried by each remote message send. This was concerning because the hint information is (a) potentially quite bulky, (b) highly redundant from one message to another, and (c) changes relatively rarely. The bulk is also a concern once we start persisting the remote message queues, which this is a precursor to doing (plus, once the message queues are persisted, storing hints on a per-message basis risks inconsistencies if the hint information changes while the messages are on disk). This PR adds a new request to the platform services API, `registerLocationHints`, which is used prior to sending the first message to a remote plus can be called any time the hint information is updated, and then removes the hint information from the message transmission flow entirely.

Author: FUDCo

packages/kernel-browser-runtime/src/PlatformServicesClient.test.ts

@@ -297,16 +297,7 @@ describe('PlatformServicesClient', () => {
 
       describe('sendRemoteMessage', () => {
         it('sends message to remote peer via RPC', async () => {
-          const result = client.sendRemoteMessage('peer-456', 'hello', [
-            '/dns4/relay.example/tcp/443/wss/p2p/relayPeer',
-          ]);
-          await delay(10);
-          await stream.receiveInput(makeNullReply('m1'));
-          expect(await result).toBeUndefined();
-        });
-
-        it('works with empty hints array', async () => {
-          const result = client.sendRemoteMessage('peer-789', 'goodbye');
+          const result = client.sendRemoteMessage('peer-456', 'hello');
           await delay(10);
           await stream.receiveInput(makeNullReply('m1'));
           expect(await result).toBeUndefined();
@@ -331,6 +322,18 @@ describe('PlatformServicesClient', () => {
         });
       });
 
+      describe('registerLocationHints', () => {
+        it('sends registerLocationHints request and resolves', async () => {
+          const result = client.registerLocationHints('peer-123', [
+            'hint1',
+            'hint2',
+          ]);
+          await delay(10);
+          await stream.receiveInput(makeNullReply('m1'));
+          expect(await result).toBeUndefined();
+        });
+      });
+
       describe('reconnectPeer', () => {
         it('sends reconnectPeer request with hints and resolves', async () => {
           const result = client.reconnectPeer('peer-456', [

packages/kernel-browser-runtime/src/PlatformServicesClient.ts

@@ -216,15 +216,10 @@ export class PlatformServicesClient implements PlatformServices {
    *
    * @param to - The peer ID to send the message to.
    * @param message - The message to send.
-   * @param hints - Optional hints for the message.
    * @returns A promise that resolves when the message has been sent.
    */
-  async sendRemoteMessage(
-    to: string,
-    message: string,
-    hints: string[] = [],
-  ): Promise<void> {
-    await this.#rpcClient.call('sendRemoteMessage', { to, message, hints });
+  async sendRemoteMessage(to: string, message: string): Promise<void> {
+    await this.#rpcClient.call('sendRemoteMessage', { to, message });
   }
 
   /**
@@ -238,6 +233,16 @@ export class PlatformServicesClient implements PlatformServices {
     await this.#rpcClient.call('closeConnection', { peerId });
   }
 
+  /**
+   * Take note of where a peer might be.
+   *
+   * @param peerId - The peer ID to whom this information applies.
+   * @param hints - An array of location hint strings.
+   */
+  async registerLocationHints(peerId: string, hints: string[]): Promise<void> {
+    await this.#rpcClient.call('registerLocationHints', { peerId, hints });
+  }
+
   /**
    * Manually reconnect to a peer after intentional close.
    * Clears the intentional close flag and initiates reconnection.

packages/kernel-browser-runtime/src/PlatformServicesServer.test.ts

@@ -22,6 +22,7 @@ import type {
 const mockSendRemoteMessage = vi.fn(async () => undefined);
 const mockStop = vi.fn(async () => undefined);
 const mockCloseConnection = vi.fn(async () => undefined);
+const mockRegisterLocationHints = vi.fn(async () => undefined);
 const mockReconnectPeer = vi.fn(async () => undefined);
 let capturedRemoteMessageHandler:
   | ((from: string, message: string) => Promise<string>)
@@ -47,6 +48,7 @@ vi.mock('@metamask/ocap-kernel', () => ({
         sendRemoteMessage: mockSendRemoteMessage,
         stop: mockStop,
         closeConnection: mockCloseConnection,
+        registerLocationHints: mockRegisterLocationHints,
         reconnectPeer: mockReconnectPeer,
       };
     },
@@ -104,11 +106,10 @@ const makeSendRemoteMessageMessageEvent = (
   messageId: `m${number}`,
   to: string,
   message: string,
-  hints: string[],
 ): MessageEvent =>
   makeMessageEvent(messageId, {
     method: 'sendRemoteMessage',
-    params: { to, message, hints },
+    params: { to, message },
   });
 
 const makeStopRemoteCommsMessageEvent = (
@@ -128,6 +129,16 @@ const makeCloseConnectionMessageEvent = (
     params: { peerId },
   });
 
+const makeRegisterLocationHintsMessageEvent = (
+  messageId: `m${number}`,
+  peerId: string,
+  hints: string[],
+): MessageEvent =>
+  makeMessageEvent(messageId, {
+    method: 'registerLocationHints',
+    params: { peerId, hints },
+  });
+
 const makeReconnectPeerMessageEvent = (
   messageId: `m${number}`,
   peerId: string,
@@ -383,6 +394,7 @@ describe('PlatformServicesServer', () => {
         mockSendRemoteMessage.mockClear();
         mockStop.mockClear();
         mockCloseConnection.mockClear();
+        mockRegisterLocationHints.mockClear();
         mockReconnectPeer.mockClear();
         capturedRemoteMessageHandler = undefined;
         capturedRemoteGiveUpHandler = undefined;
@@ -592,24 +604,21 @@ describe('PlatformServicesServer', () => {
 
           // Now send a message
           await stream.receiveInput(
-            makeSendRemoteMessageMessageEvent('m1', 'peer-123', 'hello', [
-              '/dns4/relay.example/tcp/443/wss/p2p/relayPeer',
-            ]),
+            makeSendRemoteMessageMessageEvent('m1', 'peer-123', 'hello'),
           );
           await delay(10);
 
           expect(mockSendRemoteMessage).toHaveBeenCalledWith(
             'peer-123',
             'hello',
-            ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer'],
           );
         });
 
         it('throws error if remote comms not initialized', async () => {
           const errorSpy = vi.spyOn(logger, 'error');
 
           await stream.receiveInput(
-            makeSendRemoteMessageMessageEvent('m0', 'peer-456', 'test', []),
+            makeSendRemoteMessageMessageEvent('m0', 'peer-456', 'test'),
           );
           await delay(10);
 
@@ -711,6 +720,51 @@ describe('PlatformServicesServer', () => {
         });
       });
 
+      describe('registerLocationHints', () => {
+        it('registers location hints via network layer', async () => {
+          // First initialize remote comms
+          await stream.receiveInput(
+            makeInitializeRemoteCommsMessageEvent('m0', '0xabcd', {
+              relays: ['/dns4/relay.example/tcp/443/wss/p2p/relayPeer'],
+            }),
+          );
+          await delay(10);
+
+          // Now register some hints
+          await stream.receiveInput(
+            makeRegisterLocationHintsMessageEvent('m1', 'peer-123', [
+              'hint1',
+              'hint2',
+            ]),
+          );
+          await delay(10);
+
+          expect(mockRegisterLocationHints).toHaveBeenCalledWith('peer-123', [
+            'hint1',
+            'hint2',
+          ]);
+        });
+
+        it('throws error if remote comms not initialized', async () => {
+          const errorSpy = vi.spyOn(logger, 'error');
+
+          await stream.receiveInput(
+            makeRegisterLocationHintsMessageEvent('m0', 'peer-456', [
+              'hint1',
+              'hint2',
+            ]),
+          );
+          await delay(10);
+
+          expect(errorSpy).toHaveBeenCalledWith(
+            'Error handling "registerLocationHints" request:',
+            expect.objectContaining({
+              message: 'remote comms not initialized',
+            }),
+          );
+        });
+      });
+
       describe('reconnectPeer', () => {
         it('reconnects peer via network layer', async () => {
           // First initialize remote comms

packages/kernel-browser-runtime/src/PlatformServicesServer.ts

@@ -61,6 +61,10 @@ export class PlatformServicesServer {
 
   #closeConnectionFunc: ((peerId: string) => Promise<void>) | null = null;
 
+  #registerLocationHintsFunc:
+    | ((peerId: string, hints: string[]) => void)
+    | null = null;
+
   #reconnectPeerFunc:
     | ((peerId: string, hints?: string[]) => Promise<void>)
     | null = null;
@@ -105,10 +109,11 @@ export class PlatformServicesServer {
       launch: this.#launch.bind(this),
       terminate: this.#terminate.bind(this),
       terminateAll: this.#terminateAll.bind(this),
-      initializeRemoteComms: this.#initializeRemoteComms.bind(this),
       sendRemoteMessage: this.#sendRemoteMessage.bind(this),
+      initializeRemoteComms: this.#initializeRemoteComms.bind(this),
       stopRemoteComms: this.#stopRemoteComms.bind(this),
       closeConnection: this.#closeConnection.bind(this),
+      registerLocationHints: this.#registerLocationHints.bind(this),
       reconnectPeer: this.#reconnectPeer.bind(this),
     });
 
@@ -256,16 +261,22 @@ export class PlatformServicesServer {
     if (this.#sendRemoteMessageFunc) {
       throw Error('remote comms already initialized');
     }
-    const { sendRemoteMessage, stop, closeConnection, reconnectPeer } =
-      await initNetwork(
-        keySeed,
-        options,
-        this.#handleRemoteMessage.bind(this),
-        this.#handleRemoteGiveUp.bind(this),
-      );
+    const {
+      sendRemoteMessage,
+      stop,
+      closeConnection,
+      registerLocationHints,
+      reconnectPeer,
+    } = await initNetwork(
+      keySeed,
+      options,
+      this.#handleRemoteMessage.bind(this),
+      this.#handleRemoteGiveUp.bind(this),
+    );
     this.#sendRemoteMessageFunc = sendRemoteMessage;
     this.#stopRemoteCommsFunc = stop;
     this.#closeConnectionFunc = closeConnection;
+    this.#registerLocationHintsFunc = registerLocationHints;
     this.#reconnectPeerFunc = reconnectPeer;
     return null;
   }
@@ -283,6 +294,7 @@ export class PlatformServicesServer {
     this.#sendRemoteMessageFunc = null;
     this.#stopRemoteCommsFunc = null;
     this.#closeConnectionFunc = null;
+    this.#registerLocationHintsFunc = null;
     this.#reconnectPeerFunc = null;
     return null;
   }
@@ -301,6 +313,21 @@ export class PlatformServicesServer {
     return null;
   }
 
+  /**
+   * Take note of where a peer might be.
+   *
+   * @param peerId - The peer ID to whom this information applies.
+   * @param hints - An array of location hints
+   * @returns A promise that resolves when the connection has been closed.
+   */
+  async #registerLocationHints(peerId: string, hints: string[]): Promise<null> {
+    if (!this.#registerLocationHintsFunc) {
+      throw Error('remote comms not initialized');
+    }
+    this.#registerLocationHintsFunc(peerId, hints);
+    return null;
+  }
+
   /**
    * Manually reconnect to a peer after intentional close.
    *
@@ -321,18 +348,13 @@ export class PlatformServicesServer {
    *
    * @param to - The peer ID to send the message to.
    * @param message - The message to send.
-   * @param hints - Optional hints for the message.
    * @returns A promise that resolves when the message has been sent.
    */
-  async #sendRemoteMessage(
-    to: string,
-    message: string,
-    hints: string[] = [],
-  ): Promise<null> {
+  async #sendRemoteMessage(to: string, message: string): Promise<null> {
     if (!this.#sendRemoteMessageFunc) {
       throw Error('remote comms not initialized');
     }
-    await this.#sendRemoteMessageFunc(to, message, hints);
+    await this.#sendRemoteMessageFunc(to, message);
     return null;
   }
 
@@ -349,7 +371,7 @@ export class PlatformServicesServer {
       message,
     });
     if (possibleReply !== '') {
-      await this.#sendRemoteMessage(from, possibleReply, []);
+      await this.#sendRemoteMessage(from, possibleReply);
     }
     return '';
   }

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

@@ -130,6 +130,10 @@ class DirectNetworkService {
         return Promise.resolve();
       },
 
+      async registerLocationHints(_peerId: string, _hints: string[]) {
+        return Promise.resolve();
+      },
+
       async reconnectPeer(_peerId: string, _hints: string[] = []) {
         // Mock implementation - in direct network, connections are always available
         return Promise.resolve();