feat: close, reopen, and automatically reopen ICA channels

- adds .reopen() method to IcaAccountKit['holder'] to re-establish an ICA channel using the original requestedRemoteAddress. it's
  intended to be used internally to automatically re-establish a channel, but also exposed to holders if they wish to re-open their
  account after calling .close(). includes corresponding 'ReopenAccount' invitationMaker on CosmosOrchestrationAccount.
- performs cleanup after a connection is closed - namely resetting localAddr, remoteAddr, and connection in state. chainAddress and
  port are preserved.
- adds logic to onClose() handler to automatically re-establish the ICA channel. This handler is only called when external factors
  force a channel closure (iow - if .close() is called by the holder, this will not fire)
- updates network-fakes.ts to cache mockChainAddresses based on PortID:ConnectionID to mimic ICS-27 protocol

- refs: #9192
- refs: #9068

packages/orchestration/src/cosmos-api.ts

@@ -214,10 +214,10 @@ export interface IcaAccount {
   ) => Promise<string>;
   /** get Purse for a brand to .withdraw() a Payment from the account */
   getPurse: (brand: Brand) => Promise<Purse>;
-  /**
-   * Close the remote account
-   */
+  /** Close the remote account */
   close: () => Promise<void>;
+  /** Reopens an account/channel that was previous closed by the holder. */
+  reopen: () => Promise<void>;
   /** @returns the address of the remote channel */
   getRemoteAddress: () => RemoteIbcAddress;
   /** @returns the address of the local channel */

packages/orchestration/src/exos/cosmos-orchestration-account.js

@@ -84,6 +84,8 @@ export const IcaAccountHolderI = M.interface('IcaAccountHolder', {
   ),
   withdrawRewards: M.call().returns(Vow$(M.arrayOf(DenomAmountShape))),
   undelegate: M.call(M.arrayOf(DelegationShape)).returns(VowShape),
+  close: M.call().returns(VowShape),
+  reopen: M.call().returns(VowShape),
 });
 
 /** @type {{ [name: string]: [description: string, valueShape: Matcher] }} */
@@ -147,6 +149,7 @@ export const prepareCosmosOrchestrationAccountKit = (
         WithdrawReward: M.call(ChainAddressShape).returns(M.promise()),
         Undelegate: M.call(M.arrayOf(DelegationShape)).returns(M.promise()),
         CloseAccount: M.call().returns(M.promise()),
+        ReopenAccount: M.call().returns(M.promise()),
         TransferAccount: M.call().returns(M.promise()),
       }),
     },
@@ -169,6 +172,8 @@ export const prepareCosmosOrchestrationAccountKit = (
       const topicKit = makeRecorderKit(storageNode, PUBLIC_TOPICS.account[1]);
       // TODO determine what goes in vstorage https://github.com/Agoric/agoric-sdk/issues/9066
       // XXX consider parsing local/remoteAddr to portId, channelId, counterpartyPortId, counterpartyChannelId, connectionId, counterpartyConnectionId
+      // FIXME these values will not update if IcaAccount gets new values after reopening.
+      // consider having IcaAccount responsible for the owning the writer. It might choose to share it with COA.
       void E(topicKit.recorder).write({ localAddress, remoteAddress });
 
       return {
@@ -312,7 +317,16 @@ export const prepareCosmosOrchestrationAccountKit = (
           }, 'Undelegate');
         },
         CloseAccount() {
-          throw Error('not yet implemented');
+          return zcf.makeInvitation(seat => {
+            seat.exit();
+            return watch(this.facets.holder.close());
+          }, 'CloseAccount');
+        },
+        ReopenAccount() {
+          return zcf.makeInvitation(seat => {
+            seat.exit();
+            return watch(this.facets.holder.reopen());
+          }, 'ReopenAccount');
         },
         /**
          * Starting a transfer revokes the account holder. The associated
@@ -501,6 +515,14 @@ export const prepareCosmosOrchestrationAccountKit = (
             return watch(undelegateV, this.facets.returnVoidWatcher);
           });
         },
+        /** @type {HostOf<OrchestrationAccountI['close']>} */
+        close() {
+          return watch(E(this.facets.helper.owned()).close());
+        },
+        /** @type {HostOf<OrchestrationAccountI['reopen']>} */
+        reopen() {
+          return watch(E(this.facets.helper.owned()).reopen());
+        },
       },
     },
   );

packages/orchestration/src/exos/ica-account-kit.js

@@ -38,6 +38,7 @@ export const IcaAccountI = M.interface('IcaAccount', {
     .optional(TxBodyOptsShape)
     .returns(VowShape),
   close: M.call().returns(VowShape),
+  reopen: M.call().returns(VowShape),
 });
 
 /**
@@ -98,6 +99,15 @@ export const prepareIcaAccountKit = (zone, { watch, asVow }) =>
             'ICA channel creation acknowledgement not yet received.',
           );
         },
+        reopen() {
+          const { port, requestedRemoteAddress } = this.state;
+          return watch(
+            E(port).connect(
+              requestedRemoteAddress,
+              this.facets.connectionHandler,
+            ),
+          );
+        },
         getLocalAddress() {
           return NonNullish(
             this.state.localAddress,
@@ -137,18 +147,20 @@ export const prepareIcaAccountKit = (zone, { watch, asVow }) =>
           });
         },
         /**
-         * Close the remote account
+         * Close the remote account. Does not retrieve assets, so warn the
+         * caller to retrieve them first. However, the Port is persisted and
+         * holders can always call .reopen() to re-establish a connection.
          *
          * @returns {Vow<void>}
          * @throws {Error} if connection is not available or already closed
          */
         close() {
           return asVow(() => {
-            /// TODO #9192 what should the behavior be here? and `onClose`?
-            // - retrieve assets?
-            // - revoke the port?
             const { connection } = this.state;
             if (!connection) throw Fail`connection not available`;
+            this.state.connection = undefined;
+            this.state.localAddress = undefined;
+            this.state.remoteAddress = undefined;
             return E(connection).close();
           });
         },
@@ -175,13 +187,23 @@ export const prepareIcaAccountKit = (zone, { watch, asVow }) =>
           });
         },
         /**
+         * This handler fires if the connection is closed due to external
+         * factors (e.g. a packet timeout). It will not fire if a holder calls
+         * `.close()`.
+         *
+         * Here, we clear the connection and addresses from state as we expect
+         * them to change and call reopen() to re-establish the connection.
+         *
          * @param {Remote<Connection>} _connection
          * @param {unknown} reason
+         * @see {@link https://docs.cosmos.network/v0.45/ibc/overview.html#:~:text=In%20ORDERED%20channels%2C%20a%20timeout%20of%20a%20single%20packet%20in%20the%20channel%20closes%20the%20channel.}
          */
         async onClose(_connection, reason) {
           trace(`ICA Channel closed. Reason: ${reason}`);
-          // FIXME handle connection closing https://github.com/Agoric/agoric-sdk/issues/9192
-          // XXX is there a scenario where a connection will unexpectedly close? _I think yes_
+          this.state.connection = undefined;
+          this.state.localAddress = undefined;
+          this.state.remoteAddress = undefined;
+          void watch(this.facets.account.reopen());
         },
       },
     },

packages/orchestration/src/orchestration-api.ts

@@ -195,6 +195,19 @@ export interface OrchestrationAccountI {
    * in the {@link CurrentWalletRecord} in vstorage.
    */
   getPublicTopics: () => Promise<Record<string, ResolvedPublicTopic<unknown>>>;
+
+  /**
+   * Closes the ICA channel (account). Does not retrieve assets, so warn the
+   * caller to retrieve them first. However, the Port is persisted and
+   * holders can always call .reopen() to re-establish a connection.
+   */
+  close: () => Promise<void>;
+  /**
+   * Reopens teh ICA channel (account) that was previously closed by the holder.
+   * If a channel is closed for an unexpected reason, it should automatically
+   * reopen and the holder should not need to call .reopen().
+   */
+  reopen: () => Promise<void>;
 }
 
 export interface OrchestrationFlow<CT = unknown> {

packages/orchestration/test/network-fakes.ts

@@ -12,6 +12,7 @@ import type {
   IBCEvent,
   ScopedBridgeManagerMethods,
   IBCConnectionID,
+  IBCPortID,
 } from '@agoric/vats';
 import {
   prepareCallbacks as prepareIBCCallbacks,
@@ -55,10 +56,9 @@ export const ibcBridgeMocks: {
     ? (
         obj: IBCMethod<'startChannelOpenInit'>,
         opts: {
-          bech32Prefix: string;
-          sequence: number;
           channelID: IBCChannelID;
           counterpartyChannelID: IBCChannelID;
+          mockChainAddress: string;
         },
       ) => IBCEvent<'channelOpenAck'>
     : T extends 'acknowledgementPacket'
@@ -71,20 +71,15 @@ export const ibcBridgeMocks: {
   channelOpenAck: (
     obj: IBCMethod<'startChannelOpenInit'>,
     {
-      bech32Prefix,
-      sequence,
       channelID,
       counterpartyChannelID,
+      mockChainAddress,
     }: {
-      bech32Prefix: string;
-      sequence: number;
       channelID: IBCChannelID;
       counterpartyChannelID: IBCChannelID;
+      mockChainAddress: string;
     },
   ): IBCEvent<'channelOpenAck'> => {
-    const mockChainAddress =
-      sequence > 0 ? `${bech32Prefix}1test${sequence}` : `${bech32Prefix}1test`;
-
     return {
       type: 'IBC_EVENT',
       blockHeight: 99,
@@ -193,6 +188,20 @@ export const makeFakeIBCBridge = (
   let bridgeEvents: BridgeEvents = [];
   let bridgeDowncalls: BridgeDowncalls = [];
 
+  /**
+   * Store remote mock addresses that have been distributed.
+   * If there's a `channelOpenInit` request for a PortId:ConnnectionId
+   * pair that's been previously established, let's reuse it to mimic
+   * the behavior of the ICS-27 protocol.
+   */
+  type AddressKey = `${IBCPortID}:${IBCConnectionID}`;
+  const getAddressKey = (
+    obj: IBCMethod<'startChannelOpenInit'>,
+  ): AddressKey => {
+    return `${obj.packet.source_port as IBCPortID}:${obj.hops[0] as IBCConnectionID}`;
+  };
+  const addressMap = new Map<AddressKey, string>();
+
   return zone.exo('Fake IBC Bridge Manager', undefined, {
     getBridgeId: () => BridgeId.DIBC,
     toBridge: async obj => {
@@ -201,9 +210,19 @@ export const makeFakeIBCBridge = (
         switch (obj.method) {
           case 'startChannelOpenInit': {
             const connectionChannelCount = remoteChannelMap[obj.hops[0]] || 0;
+            const addressKey = getAddressKey(obj);
+            let mockChainAddress;
+            if (addressMap.has(addressKey)) {
+              mockChainAddress = addressMap.get(addressKey);
+            } else {
+              mockChainAddress =
+                channelCount > 0
+                  ? `${bech32Prefix}1test${channelCount}`
+                  : `${bech32Prefix}1test`;
+              addressMap.set(addressKey, mockChainAddress);
+            }
             const ackEvent = ibcBridgeMocks.channelOpenAck(obj, {
-              bech32Prefix,
-              sequence: channelCount,
+              mockChainAddress,
               channelID: `channel-${channelCount}`,
               counterpartyChannelID: `channel-${connectionChannelCount}`,
             });

packages/orchestration/test/service.test.ts

@@ -16,6 +16,7 @@ import type { LocalIbcAddress } from '@agoric/vats/tools/ibc-utils.js';
 import { getMethodNames } from '@agoric/internal';
 import { Port } from '@agoric/network';
 import { eventLoopIteration } from '@agoric/internal/src/testing-utils.js';
+import type { IBCMethod } from '@agoric/vats';
 import { commonSetup } from './supports.js';
 import { ChainAddressShape } from '../src/typeGuards.js';
 import { tryDecodeResponse } from '../src/utils/cosmos.js';
@@ -294,14 +295,95 @@ test('onClose handler is called when channelCloseConfirm event is received', asy
 
   const { bridgeEvents: bridgeEvents1, bridgeDowncalls: bridgeDowncalls1 } =
     await inspectDibcBridge();
-  t.is(bridgeEvents1.length, 2, 'bridge received an additional event');
+  t.is(bridgeEvents1.length, 3, 'bridge received an additional 2 events');
   t.is(
-    bridgeEvents1[bridgeEvents1.length - 1].event,
+    bridgeEvents1[bridgeEvents1.length - 2].event,
     'channelCloseConfirm',
     'bridged received channelCloseInit event',
   );
-  t.is(bridgeDowncalls1.length, 2, "bridge did not receive add'l downcalls");
+  t.is(
+    bridgeEvents1[bridgeEvents1.length - 1].event,
+    'channelOpenAck',
+    'onCloe handler automatically reopens the channel',
+  );
+});
+
+test('reopen a close account(channel) after choosing to close it', async t => {
+  const {
+    bootstrap: { cosmosInterchainService },
+    utils: { inspectDibcBridge },
+  } = await commonSetup(t);
+
+  const account = await E(cosmosInterchainService).makeAccount(
+    CHAIN_ID,
+    HOST_CONNECTION_ID,
+    CONTROLLER_CONNECTION_ID,
+    { version: 'ics27-2', ordering: 'unordered', encoding: 'json' },
+  );
+
+  const [chainAddress0, remoteAddress0, localAddress0] = await Promise.all([
+    E(account).getAddress(),
+    E(account).getRemoteAddress(),
+    E(account).getLocalAddress(),
+  ]);
+
+  await eventLoopIteration(); // ensure there's an account to close
+  // close the account
+  await E(account).close();
+  await eventLoopIteration();
+
+  const { bridgeDowncalls: bridgeDowncalls0 } = await inspectDibcBridge();
+  t.is(
+    bridgeDowncalls0[2].method,
+    'startChannelCloseInit',
+    'bridge received startChannelCloseInit downcall',
+  );
+
+  // reopen the account
+  await E(account).reopen();
+  await eventLoopIteration();
+
+  const { bridgeDowncalls } = await inspectDibcBridge();
+  t.is(
+    bridgeDowncalls[3].method,
+    'startChannelOpenInit',
+    'bridge received startChannelOpenInit to re-establish the channel',
+  );
+
+  const getPortAndConnectionIDs = (obj: IBCMethod<'startChannelOpenInit'>) => {
+    const { hops, packet } = obj;
+    const { source_port: sourcePort } = packet;
+    return { hops, sourcePort };
+  };
+
+  t.deepEqual(
+    getPortAndConnectionIDs(
+      bridgeDowncalls[3] as IBCMethod<'startChannelOpenInit'>,
+    ),
+    getPortAndConnectionIDs(
+      bridgeDowncalls[1] as IBCMethod<'startChannelOpenInit'>,
+    ),
+    'same port and connection id are used to re-stablish the channel',
+  );
 
-  // XXX how can we verify that the onClose handler was called?
-  // for now, we can observe in the logs: ----- IcaAccountKit.4  3 ICA Channel closed. Reason: undefined
+  const [chainAddress, remoteAddress, localAddress] = await Promise.all([
+    E(account).getAddress(),
+    E(account).getRemoteAddress(),
+    E(account).getLocalAddress(),
+  ]);
+
+  t.deepEqual(chainAddress, chainAddress0, 'chain address is unchanged');
+  t.notDeepEqual(
+    remoteAddress,
+    remoteAddress0,
+    'remote ibc address is changed',
+  );
+  t.notDeepEqual(localAddress, localAddress0, 'local ibc address is changed');
+  const getChannelID = (lAddr: LocalIbcAddress) =>
+    lAddr.split('/ibc-channel/')[1];
+  t.not(
+    getChannelID(localAddress),
+    getChannelID(localAddress0),
+    'channel id is changed',
+  );
 });