diff --git a/packages/orchestration/src/cosmos-api.ts b/packages/orchestration/src/cosmos-api.ts index 06c4587e4043..a18f1a6091fd 100644 --- a/packages/orchestration/src/cosmos-api.ts +++ b/packages/orchestration/src/cosmos-api.ts @@ -215,9 +215,17 @@ export interface IcaAccount { /** get Purse for a brand to .withdraw() a Payment from the account */ getPurse: (brand: Brand) => Promise; /** - * 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; + /** + * 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; /** @returns the address of the remote channel */ getRemoteAddress: () => RemoteIbcAddress; /** @returns the address of the local channel */ diff --git a/packages/orchestration/src/exos/README.md b/packages/orchestration/src/exos/README.md index 902421a4e7a8..af94ba38cf1a 100644 --- a/packages/orchestration/src/exos/README.md +++ b/packages/orchestration/src/exos/README.md @@ -30,6 +30,7 @@ classDiagram executeTx() executeEncodedTx() close() + reopen() } class ICQConnection { port: Port diff --git a/packages/orchestration/src/exos/cosmos-orchestration-account.js b/packages/orchestration/src/exos/cosmos-orchestration-account.js index 24da947f3291..03a3d0c05645 100644 --- a/packages/orchestration/src/exos/cosmos-orchestration-account.js +++ b/packages/orchestration/src/exos/cosmos-orchestration-account.js @@ -91,6 +91,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] }} */ @@ -154,6 +156,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()), }), }, @@ -326,7 +329,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 @@ -515,6 +527,14 @@ export const prepareCosmosOrchestrationAccountKit = ( return watch(undelegateV, this.facets.returnVoidWatcher); }); }, + /** @type {HostOf} */ + close() { + return watch(E(this.facets.helper.owned()).close()); + }, + /** @type {HostOf} */ + reopen() { + return watch(E(this.facets.helper.owned()).reopen()); + }, }, }, ); diff --git a/packages/orchestration/src/exos/ica-account-kit.js b/packages/orchestration/src/exos/ica-account-kit.js index 38cca597cd5c..5cf67e007334 100644 --- a/packages/orchestration/src/exos/ica-account-kit.js +++ b/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,7 +139,7 @@ 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, @@ -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} * @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 * @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()); }, }, }, diff --git a/packages/orchestration/test/network-fakes.ts b/packages/orchestration/test/network-fakes.ts index f750d8363f7c..c1ba62fa252b 100644 --- a/packages/orchestration/test/network-fakes.ts +++ b/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(); + 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}`, }); diff --git a/packages/orchestration/test/service.test.ts b/packages/orchestration/test/service.test.ts index 65487d39c5e0..e428ef4091c1 100644 --- a/packages/orchestration/test/service.test.ts +++ b/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'; @@ -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', ); @@ -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', + ); }); diff --git a/packages/orchestration/test/staking-ops.test.ts b/packages/orchestration/test/staking-ops.test.ts index cc200f2843ec..25959f1ed7a3 100644 --- a/packages/orchestration/test/staking-ops.test.ts +++ b/packages/orchestration/test/staking-ops.test.ts @@ -156,12 +156,11 @@ const makeScenario = () => { }, executeTx: () => Fail`mock`, close: () => Fail`mock`, - deposit: () => Fail`mock`, getPurse: () => Fail`mock`, - prepareTransfer: () => Fail`mock`, getLocalAddress: () => configStaking.acct1.localAddress, getRemoteAddress: () => configStaking.acct1.remoteAddress, getPort: () => Fail`mock`, + reopen: () => Fail`mock`, }); return { account, calls }; };