From d80a57cbaed855fe9ee64039cef3fdd1c79fd7c9 Mon Sep 17 00:00:00 2001
From: 0xPatrick <patrick@0xpatrick.dev>
Date: Tue, 30 Jul 2024 23:04:39 -0400
Subject: [PATCH] feat: use a single port for all icq connections

- changes logic in provieICQConnection to lazily request a port on the first request, and reuse it in subsequent requests. since the channel
is ordered, timeouts and query errors will not affect subsequent queries and this can be considered safe
- closes #9317
---
 .../src/exos/cosmos-interchain-service.js     | 40 +++++++++----------
 packages/orchestration/test/service.test.ts   | 13 ++++++
 2 files changed, 32 insertions(+), 21 deletions(-)

diff --git a/packages/orchestration/src/exos/cosmos-interchain-service.js b/packages/orchestration/src/exos/cosmos-interchain-service.js
index 78cce29de6b..ba56d40e055 100644
--- a/packages/orchestration/src/exos/cosmos-interchain-service.js
+++ b/packages/orchestration/src/exos/cosmos-interchain-service.js
@@ -37,6 +37,7 @@ const { Vow$ } = NetworkShape; // TODO #9611
 /**
  * @typedef {{
  *   icqConnections: ICQConnectionStore;
+ *   sharedICQPort: Remote<Port> | undefined;
  * } & OrchestrationPowers} OrchestrationState
  */
 
@@ -103,13 +104,12 @@ const prepareCosmosOrchestrationServiceKit = (
     powers => {
       mustMatch(powers?.portAllocator, M.remotable('PortAllocator'));
       const icqConnections = zone.detached().mapStore('ICQConnections');
-      return harden(
-        /** @type {OrchestrationState} */ ({
-          icqConnections,
-          reserved: undefined,
-          ...powers,
-        }),
-      );
+      return /** @type {OrchestrationState} */ ({
+        icqConnections,
+        sharedICQPort: undefined,
+        reserved: undefined,
+        ...powers,
+      });
     },
     {
       requestICAChannelWatcher: {
@@ -142,8 +142,10 @@ const prepareCosmosOrchestrationServiceKit = (
          * }} watchContext
          */
         onFulfilled(port, { remoteConnAddr, icqLookupKey }) {
+          if (!this.state.sharedICQPort) {
+            this.state.sharedICQPort = port;
+          }
           const connectionKit = makeICQConnectionKit(port);
-          /** @param {ICQConnectionKit} kit */
           return watch(
             E(port).connect(remoteConnAddr, connectionKit.connectionHandler),
             this.facets.channelOpenWatcher,
@@ -178,8 +180,6 @@ const prepareCosmosOrchestrationServiceKit = (
           }
           return connectionKit[returnFacet];
         },
-        // TODO #9317 if we fail, should we revoke the port (if it was created in this flow)?
-        // onRejected() {}
       },
       public: {
         /**
@@ -226,23 +226,21 @@ const prepareCosmosOrchestrationServiceKit = (
             controllerConnectionId,
             version,
           );
-          const { portAllocator } = this.state;
-          return watch(
-            // allocate a new Port for every Connection
-            // TODO #9317 optimize ICQ port allocation
-            E(portAllocator).allocateICQControllerPort(),
-            this.facets.requestICQChannelWatcher,
-            {
-              remoteConnAddr,
-              icqLookupKey,
-            },
-          );
+          const { portAllocator, sharedICQPort } = this.state;
+          const portOrPortVow =
+            sharedICQPort || E(portAllocator).allocateICQControllerPort();
+
+          return watch(portOrPortVow, this.facets.requestICQChannelWatcher, {
+            remoteConnAddr,
+            icqLookupKey,
+          });
         },
       },
     },
     {
       stateShape: {
         icqConnections: M.remotable('icqConnections mapStore'),
+        sharedICQPort: M.or(M.remotable('Port'), M.undefined()),
         portAllocator: M.remotable('PortAllocator'),
         reserved: M.any(),
       },
diff --git a/packages/orchestration/test/service.test.ts b/packages/orchestration/test/service.test.ts
index 4b2a921bf78..1244a680897 100644
--- a/packages/orchestration/test/service.test.ts
+++ b/packages/orchestration/test/service.test.ts
@@ -12,6 +12,7 @@ import { Any } from '@agoric/cosmic-proto/google/protobuf/any.js';
 import { matches } from '@endo/patterns';
 import { heapVowE as E } from '@agoric/vow/vat.js';
 import { decodeBase64 } from '@endo/base64';
+import type { LocalIbcAddress } from '@agoric/vats/tools/ibc-utils.js';
 import { commonSetup } from './supports.js';
 import { ChainAddressShape } from '../src/typeGuards.js';
 import { tryDecodeResponse } from '../src/utils/cosmos.js';
@@ -84,6 +85,18 @@ test('makeICQConnection returns an ICQConnection', async t => {
   );
   const localAddr4 = await E(icqConnection4).getLocalAddress();
   t.is(localAddr3, localAddr4, 'custom version is idempotent');
+
+  const icqConnection5 = await E(cosmosInterchainService).provideICQConnection(
+    'connection-99',
+  );
+  const localAddr5 = await E(icqConnection5).getLocalAddress();
+
+  const getPortId = (lAddr: LocalIbcAddress) => lAddr.split('/')[2];
+  const uniquePortIds = new Set(
+    [localAddr, localAddr2, localAddr3, localAddr4, localAddr5].map(getPortId),
+  );
+  t.regex([...uniquePortIds][0], /icqcontroller-\d+/);
+  t.is(uniquePortIds.size, 1, 'all connections share same port');
 });
 
 test('makeAccount returns a ChainAccount', async t => {