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

@@ -213,9 +213,17 @@ export interface IcaAccount {
     opts?: Partial<Omit<TxBody, 'messages'>>,
   ) => Promise<string>;
   /**
-   * Close the remote account
+   * 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>;
   /** @returns the address of the remote channel */
   getRemoteAddress: () => RemoteIbcAddress;
   /** @returns the address of the local channel */

packages/orchestration/src/exos/README.md

@@ -30,6 +30,7 @@ classDiagram
       executeTx()
       executeEncodedTx()
       close()
+      reopen()
     }
     class ICQConnection {
       port: Port

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

@@ -81,6 +81,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] }} */
@@ -144,6 +146,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()),
       }),
     },
@@ -298,7 +301,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
@@ -487,6 +499,14 @@ export const prepareCosmosOrchestrationAccountKit = (
             return watch(undelegateV, this.facets.returnVoidWatcher);
           });
         },
+        /** @type {HostOf<IcaAccount['close']>} */
+        close() {
+          return watch(E(this.facets.helper.owned()).close());
+        },
+        /** @type {HostOf<IcaAccount['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,
@@ -129,26 +139,28 @@ export const prepareIcaAccountKit = (zone, { watch, asVow }) =>
         executeEncodedTx(msgs, opts) {
           return asVow(() => {
             const { connection } = this.state;
-            if (!connection) throw Fail`connection not available`;
+            if (!connection) throw Fail`Connection not available or closed.`;
             return watch(
               E(connection).send(makeTxPacket(msgs, opts)),
               this.facets.parseTxPacketWatcher,
             );
           });
         },
         /**
-         * 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/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

@@ -14,8 +14,9 @@ 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 { getMethodNames } from '@agoric/internal';
-import { Port } from '@agoric/network';
+import type { 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';
@@ -175,7 +176,7 @@ test('makeAccount returns an IcaAccountKit', async t => {
   await t.throwsAsync(
     E(account).executeEncodedTx([delegateMsg]),
     {
-      message: 'Connection closed',
+      message: 'Connection not available or closed.',
     },
     'cannot execute transaction if connection is closed',
   );
@@ -220,7 +221,6 @@ test('.close() sends a downcall to the ibc bridge handler', async t => {
     CHAIN_ID,
     HOST_CONNECTION_ID,
     CONTROLLER_CONNECTION_ID,
-    { version: 'ics27-2', ordering: 'unordered', encoding: 'json' },
   );
   await eventLoopIteration(); // ensure there's an account to close
   await E(account).close();
@@ -294,14 +294,94 @@ 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,
+  );
+
+  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',
+  );
 });

packages/orchestration/test/staking-ops.test.ts

@@ -154,6 +154,7 @@ const makeScenario = () => {
       getLocalAddress: () => Fail`mock`,
       getRemoteAddress: () => Fail`mock`,
       getPort: () => Fail`mock`,
+      reopen: () => Fail`mock`,
     });
     return { account, calls };
   };