chore: cosmos interchain service cleanup (#9810)

refs: #9114 closes: #9317 ## Description - Miscellaneous cleanup related to `CosmosInterchainService` (part of `vat-orchestration`), closing out todos for 1) unnecessary PowerStore abstraction and 2) vow compat for early synchronous returns - Optimizes ICQ Port Allocation by reusing a single port for all ICQ connections ### Security Considerations n/a ### Scaling Considerations n/a ### Documentation Considerations js doc added where necessary ### Testing Considerations included tests for new behavior ### Upgrade Considerations these changes are intended to better prepare us for future upgrades
Agoric · Jul 31, 2024 · 8718c07 · 8718c07
2 parents e3b1b71 + d80a57c
commit 8718c07
Show file tree

Hide file tree

Showing 6 changed files with 195 additions and 102 deletions.
diff --git a/packages/orchestration/src/exos/cosmos-interchain-service.js b/packages/orchestration/src/exos/cosmos-interchain-service.js
@@ -1,12 +1,12 @@
 /** @file Orchestration service */
 
-import { Fail, b } from '@endo/errors';
 import { E } from '@endo/far';
-import { M } from '@endo/patterns';
+import { M, mustMatch } from '@endo/patterns';
 import { Shape as NetworkShape } from '@agoric/network';
 import { prepareChainAccountKit } from './chain-account-kit.js';
 import { prepareICQConnectionKit } from './icq-connection-kit.js';
 import {
+  DEFAULT_ICQ_VERSION,
   makeICAChannelAddress,
   makeICQChannelAddress,
 } from '../utils/address.js';
@@ -19,40 +19,39 @@ import {
  * @import {RemoteIbcAddress} from '@agoric/vats/tools/ibc-utils.js';
  * @import {Vow, VowTools} from '@agoric/vow';
  * @import {ICQConnection, IcaAccount, ICQConnectionKit, ChainAccountKit} from '../types.js';
+ * @import {ICAChannelAddressOpts} from '../utils/address.js';
  */
 
 const { Vow$ } = NetworkShape; // TODO #9611
 /**
  * @typedef {object} OrchestrationPowers
  * @property {Remote<PortAllocator>} portAllocator
+ * @property {undefined} reserved reserve a state key for future use. can hold
+ *   an additional power or a record of powers
  */
 
-/**
- * PowerStore is used so additional powers can be added on upgrade. See
- * [#7337](https://github.com/Agoric/agoric-sdk/issues/7337) for tracking on Exo
- * state migrations.
- *
- * @typedef {MapStore<
- *   keyof OrchestrationPowers,
- *   OrchestrationPowers[keyof OrchestrationPowers]
- * >} PowerStore
- */
-
-/** @typedef {MapStore<IBCConnectionID, ICQConnectionKit>} ICQConnectionStore */
+/** @typedef {MapStore<string, ICQConnectionKit>} ICQConnectionStore */
 
 /** @typedef {ChainAccountKit | ICQConnectionKit} ConnectionKit */
 
 /**
- * @template {keyof OrchestrationPowers} K
- * @param {PowerStore} powers
- * @param {K} name
+ * @typedef {{
+ *   icqConnections: ICQConnectionStore;
+ *   sharedICQPort: Remote<Port> | undefined;
+ * } & OrchestrationPowers} OrchestrationState
  */
-const getPower = (powers, name) => {
-  powers.has(name) || Fail`need powers.${b(name)} for this method`;
-  return /** @type {OrchestrationPowers[K]} */ (powers.get(name));
-};
 
-/** @typedef {{ powers: PowerStore; icqConnections: ICQConnectionStore }} OrchestrationState */
+/**
+ * Creates a key for the icqConnections mapStore based on connectionId and
+ * version
+ *
+ * @param {IBCConnectionID} controllerConnectionId
+ * @param {string} [version]
+ * @returns {string}
+ */
+const getICQConnectionKey = (controllerConnectionId, version) => {
+  return `${controllerConnectionId}:${version || DEFAULT_ICQ_VERSION}`;
+};
 
 /**
  * @param {Zone} zone
@@ -62,7 +61,7 @@ const getPower = (powers, name) => {
  */
 const prepareCosmosOrchestrationServiceKit = (
   zone,
-  { watch },
+  { watch, asVow },
   makeChainAccountKit,
   makeICQConnectionKit,
 ) =>
@@ -78,7 +77,7 @@ const prepareCosmosOrchestrationServiceKit = (
         onFulfilled: M.call(M.remotable('Port'))
           .optional({
             remoteConnAddr: M.string(),
-            controllerConnectionId: M.string(),
+            icqLookupKey: M.string(),
           })
           .returns(Vow$(NetworkShape.Connection)),
       }),
@@ -87,31 +86,30 @@ const prepareCosmosOrchestrationServiceKit = (
           .optional(
             M.splitRecord(
               { connectionKit: M.record(), returnFacet: M.string() },
-              { saveICQConnection: M.string() },
+              { icqLookupKey: M.string() },
             ),
           )
           .returns(M.remotable('ConnectionKit Holder facet')),
       }),
       public: M.interface('CosmosInterchainService', {
-        makeAccount: M.call(M.string(), M.string(), M.string()).returns(
-          Vow$(M.remotable('ChainAccountKit')),
-        ),
-        provideICQConnection: M.call(M.string()).returns(
-          Vow$(M.remotable('ICQConnection')),
-        ),
+        makeAccount: M.call(M.string(), M.string(), M.string())
+          .optional(M.record())
+          .returns(Vow$(M.remotable('ChainAccountKit'))),
+        provideICQConnection: M.call(M.string())
+          .optional(M.string())
+          .returns(Vow$(M.remotable('ICQConnection'))),
       }),
     },
-    /** @param {Partial<OrchestrationPowers>} [initialPowers] */
-    initialPowers => {
-      /** @type {PowerStore} */
-      const powers = zone.detached().mapStore('PowerStore');
-      if (initialPowers) {
-        for (const [name, power] of Object.entries(initialPowers)) {
-          powers.init(/** @type {keyof OrchestrationPowers} */ (name), power);
-        }
-      }
+    /** @param {Partial<OrchestrationPowers>} powers */
+    powers => {
+      mustMatch(powers?.portAllocator, M.remotable('PortAllocator'));
       const icqConnections = zone.detached().mapStore('ICQConnections');
-      return /** @type {OrchestrationState} */ ({ powers, icqConnections });
+      return /** @type {OrchestrationState} */ ({
+        icqConnections,
+        sharedICQPort: undefined,
+        reserved: undefined,
+        ...powers,
+      });
     },
     {
       requestICAChannelWatcher: {
@@ -140,19 +138,21 @@ const prepareCosmosOrchestrationServiceKit = (
          * @param {Port} port
          * @param {{
          *   remoteConnAddr: RemoteIbcAddress;
-         *   controllerConnectionId: IBCConnectionID;
+         *   icqLookupKey: string;
          * }} watchContext
          */
-        onFulfilled(port, { remoteConnAddr, controllerConnectionId }) {
+        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,
             {
               connectionKit,
               returnFacet: 'connection',
-              saveICQConnection: controllerConnectionId,
+              icqLookupKey,
             },
           );
         },
@@ -168,38 +168,36 @@ const prepareCosmosOrchestrationServiceKit = (
          * @param {{
          *   connectionKit: ConnectionKit;
          *   returnFacet: string;
-         *   saveICQConnection?: IBCConnectionID;
+         *   icqLookupKey?: string;
          * }} watchContext
          */
-        onFulfilled(
-          _connection,
-          { connectionKit, returnFacet, saveICQConnection },
-        ) {
-          if (saveICQConnection) {
+        onFulfilled(_connection, { connectionKit, returnFacet, icqLookupKey }) {
+          if (icqLookupKey) {
             this.state.icqConnections.init(
-              saveICQConnection,
+              icqLookupKey,
               /** @type {ICQConnectionKit} */ (connectionKit),
             );
           }
           return connectionKit[returnFacet];
         },
-        // TODO #9317 if we fail, should we revoke the port (if it was created in this flow)?
-        // onRejected() {}
       },
       public: {
         /**
          * @param {string} chainId
          * @param {IBCConnectionID} hostConnectionId the counterparty
          *   connection_id
          * @param {IBCConnectionID} controllerConnectionId self connection_id
+         * @param {ICAChannelAddressOpts} [opts] optional to configure the
+         *   channel address, such as version and ordering
          * @returns {Vow<IcaAccount>}
          */
-        makeAccount(chainId, hostConnectionId, controllerConnectionId) {
+        makeAccount(chainId, hostConnectionId, controllerConnectionId, opts) {
           const remoteConnAddr = makeICAChannelAddress(
             hostConnectionId,
             controllerConnectionId,
+            opts,
           );
-          const portAllocator = getPower(this.state.powers, 'portAllocator');
+          const { portAllocator } = this.state;
           return watch(
             E(portAllocator).allocateICAControllerPort(),
             this.facets.requestICAChannelWatcher,
@@ -211,29 +209,42 @@ const prepareCosmosOrchestrationServiceKit = (
         },
         /**
          * @param {IBCConnectionID} controllerConnectionId
+         * @param {string} [version]
          * @returns {Vow<ICQConnection> | ICQConnection}
          */
-        provideICQConnection(controllerConnectionId) {
-          if (this.state.icqConnections.has(controllerConnectionId)) {
-            // TODO #9281 do not return synchronously. see https://github.com/Agoric/agoric-sdk/pull/9454#discussion_r1626898694
-            return this.state.icqConnections.get(controllerConnectionId)
-              .connection;
+        provideICQConnection(controllerConnectionId, version) {
+          const icqLookupKey = getICQConnectionKey(
+            controllerConnectionId,
+            version,
+          );
+          if (this.state.icqConnections.has(icqLookupKey)) {
+            return asVow(
+              () => this.state.icqConnections.get(icqLookupKey).connection,
+            );
           }
-          const remoteConnAddr = makeICQChannelAddress(controllerConnectionId);
-          const portAllocator = getPower(this.state.powers, 'portAllocator');
-          return watch(
-            // allocate a new Port for every Connection
-            // TODO #9317 optimize ICQ port allocation
-            E(portAllocator).allocateICQControllerPort(),
-            this.facets.requestICQChannelWatcher,
-            {
-              remoteConnAddr,
-              controllerConnectionId,
-            },
+          const remoteConnAddr = makeICQChannelAddress(
+            controllerConnectionId,
+            version,
           );
+          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/src/proposals/orchestration-proposal.js b/packages/orchestration/src/proposals/orchestration-proposal.js
@@ -9,7 +9,6 @@ const trace = makeTracer('CoreEvalOrchestration', true);
 
 /**
  * @import {PortAllocator} from '@agoric/network';
- * @import {CosmosInterchainService} from '../exos/cosmos-interchain-service.js'
  */
 
 /**

diff --git a/packages/orchestration/src/utils/address.js b/packages/orchestration/src/utils/address.js
@@ -6,16 +6,20 @@ import { Fail } from '@endo/errors';
  * @import {RemoteIbcAddress} from '@agoric/vats/tools/ibc-utils.js';
  */
 
+/**
+ * @typedef {object} ICAChannelAddressOpts
+ * @property {string} [encoding='proto3'] message encoding format for the
+ *   channel
+ * @property {'ordered' | 'unordered'} [ordering='ordered'] channel ordering.
+ *   currently only `ordered` is supported for ics27-1
+ * @property {string} [txType='sdk_multi_msg'] default is `sdk_multi_msg`
+ * @property {string} [version='ics27-1'] default is `ics27-1`
+ */
+
 /**
  * @param {IBCConnectionID} hostConnectionId Counterparty Connection ID
  * @param {IBCConnectionID} controllerConnectionId Self Connection ID
- * @param {object} [opts]
- * @param {string} [opts.encoding] - message encoding format for the channel.
- *   default is `proto3`
- * @param {'ordered' | 'unordered'} [opts.ordering] - channel ordering.
- *   currently only `ordered` is supported for ics27-1
- * @param {string} [opts.txType] - default is `sdk_multi_msg`
- * @param {string} [opts.version] - default is `ics27-1`
+ * @param {ICAChannelAddressOpts} [opts]
  * @returns {RemoteIbcAddress}
  */
 export const makeICAChannelAddress = (
@@ -42,14 +46,16 @@ export const makeICAChannelAddress = (
 };
 harden(makeICAChannelAddress);
 
+export const DEFAULT_ICQ_VERSION = 'icq-1';
+
 /**
  * @param {IBCConnectionID} controllerConnectionId
- * @param {{ version?: string }} [opts]
+ * @param {string} version defaults to icq-1
  * @returns {RemoteIbcAddress}
  */
 export const makeICQChannelAddress = (
   controllerConnectionId,
-  { version = 'icq-1' } = {},
+  version = DEFAULT_ICQ_VERSION,
 ) => {
   controllerConnectionId || Fail`controllerConnectionId is required`;
   return `/ibc-hop/${controllerConnectionId}/ibc-port/icqhost/unordered/${version}`;