Skip to content

Commit

Permalink
feat: close, reopen, and automatically reopen ICA channels
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
0xpatrickdev committed Aug 8, 2024
1 parent caf3d80 commit 7444b2e
Show file tree
Hide file tree
Showing 7 changed files with 179 additions and 28 deletions.
10 changes: 9 additions & 1 deletion packages/orchestration/src/cosmos-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,17 @@ export interface IcaAccount {
/** get Purse for a brand to .withdraw() a Payment from the account */
getPurse: (brand: Brand) => Promise<Purse>;
/**
* 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 */
Expand Down
1 change: 1 addition & 0 deletions packages/orchestration/src/exos/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ classDiagram
executeTx()
executeEncodedTx()
close()
reopen()
}
class ICQConnection {
port: Port
Expand Down
22 changes: 21 additions & 1 deletion packages/orchestration/src/exos/cosmos-orchestration-account.js
Original file line number Diff line number Diff line change
Expand Up @@ -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] }} */
Expand Down Expand Up @@ -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()),
}),
},
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -515,6 +527,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());
},
},
},
);
Expand Down
36 changes: 29 additions & 7 deletions packages/orchestration/src/exos/ica-account-kit.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export const IcaAccountI = M.interface('IcaAccount', {
.optional(TxBodyOptsShape)
.returns(VowShape),
close: M.call().returns(VowShape),
reopen: M.call().returns(VowShape),
});

/**
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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();
});
},
Expand All @@ -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());
},
},
},
Expand Down
41 changes: 30 additions & 11 deletions packages/orchestration/test/network-fakes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
IBCEvent,
ScopedBridgeManagerMethods,
IBCConnectionID,
IBCPortID,
} from '@agoric/vats';
import {
prepareCallbacks as prepareIBCCallbacks,
Expand Down Expand Up @@ -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'
Expand All @@ -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,
Expand Down Expand Up @@ -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 => {
Expand All @@ -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}`,
});
Expand Down
94 changes: 88 additions & 6 deletions packages/orchestration/test/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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',
);
Expand Down Expand Up @@ -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',
);
});
3 changes: 1 addition & 2 deletions packages/orchestration/test/staking-ops.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
};
Expand Down

0 comments on commit 7444b2e

Please sign in to comment.