From 8cad5e3c2b631451ed3cf1b622ac51af7f69ac9d Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Tue, 30 Jul 2024 17:41:02 -0400 Subject: [PATCH 1/6] feat: cosmosInterchainService.makeAccount accepts address opts - expose an optional argument for providing different ica channel parameters, such as version and ordering --- .../src/exos/cosmos-interchain-service.js | 12 ++++-- packages/orchestration/src/utils/address.js | 18 +++++---- packages/orchestration/test/service.test.ts | 37 +++++++++++++++++-- 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/packages/orchestration/src/exos/cosmos-interchain-service.js b/packages/orchestration/src/exos/cosmos-interchain-service.js index 82acebd8f13..1ffc8ea4417 100644 --- a/packages/orchestration/src/exos/cosmos-interchain-service.js +++ b/packages/orchestration/src/exos/cosmos-interchain-service.js @@ -19,6 +19,7 @@ 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 @@ -93,9 +94,9 @@ const prepareCosmosOrchestrationServiceKit = ( .returns(M.remotable('ConnectionKit Holder facet')), }), public: M.interface('CosmosInterchainService', { - makeAccount: M.call(M.string(), M.string(), M.string()).returns( - Vow$(M.remotable('ChainAccountKit')), - ), + makeAccount: M.call(M.string(), M.string(), M.string()) + .optional(M.record()) + .returns(Vow$(M.remotable('ChainAccountKit'))), provideICQConnection: M.call(M.string()).returns( Vow$(M.remotable('ICQConnection')), ), @@ -192,12 +193,15 @@ const prepareCosmosOrchestrationServiceKit = ( * @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} */ - makeAccount(chainId, hostConnectionId, controllerConnectionId) { + makeAccount(chainId, hostConnectionId, controllerConnectionId, opts) { const remoteConnAddr = makeICAChannelAddress( hostConnectionId, controllerConnectionId, + opts, ); const portAllocator = getPower(this.state.powers, 'portAllocator'); return watch( diff --git a/packages/orchestration/src/utils/address.js b/packages/orchestration/src/utils/address.js index e8455f3512a..29dd07e0087 100644 --- 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 = ( diff --git a/packages/orchestration/test/service.test.ts b/packages/orchestration/test/service.test.ts index 20996a83ae8..d1473dd10cf 100644 --- a/packages/orchestration/test/service.test.ts +++ b/packages/orchestration/test/service.test.ts @@ -66,15 +66,15 @@ test('makeICQConnection returns an ICQConnection', async t => { }); }); +const CHAIN_ID = 'cosmoshub-99'; +const HOST_CONNECTION_ID = 'connection-0'; +const CONTROLLER_CONNECTION_ID = 'connection-1'; + test('makeAccount returns a ChainAccount', async t => { const { bootstrap: { cosmosInterchainService }, } = await commonSetup(t); - const CHAIN_ID = 'cosmoshub-99'; - const HOST_CONNECTION_ID = 'connection-0'; - const CONTROLLER_CONNECTION_ID = 'connection-1'; - const account = await E(cosmosInterchainService).makeAccount( CHAIN_ID, HOST_CONNECTION_ID, @@ -133,3 +133,32 @@ test('makeAccount returns a ChainAccount', async t => { 'cannot execute transaction if connection is closed', ); }); + +test('makeAccount accepts opts (version, ordering, encoding)', async t => { + const { + bootstrap: { cosmosInterchainService }, + } = 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 [localAddr, remoteAddr] = await Promise.all([ + E(account).getLocalAddress(), + E(account).getRemoteAddress(), + ]); + t.log({ + localAddr, + remoteAddr, + }); + for (const addr of [localAddr, remoteAddr]) { + t.regex(addr, /unordered/, 'remote address contains unordered ordering'); + t.regex( + addr, + /"version":"ics27-2"(.*)"encoding":"json"/, + 'remote address contains version and encoding in version string', + ); + } +}); From 1989138c0364e14e40c86f4b0997c7a48f00bd83 Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Tue, 30 Jul 2024 18:13:14 -0400 Subject: [PATCH 2/6] refactor: unnecessary powerStore abstraction --- .../src/exos/cosmos-interchain-service.js | 59 ++++++++----------- .../src/proposals/orchestration-proposal.js | 1 - 2 files changed, 25 insertions(+), 35 deletions(-) diff --git a/packages/orchestration/src/exos/cosmos-interchain-service.js b/packages/orchestration/src/exos/cosmos-interchain-service.js index 1ffc8ea4417..824da490e59 100644 --- a/packages/orchestration/src/exos/cosmos-interchain-service.js +++ b/packages/orchestration/src/exos/cosmos-interchain-service.js @@ -1,8 +1,7 @@ /** @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'; @@ -26,17 +25,8 @@ const { Vow$ } = NetworkShape; // TODO #9611 /** * @typedef {object} OrchestrationPowers * @property {Remote} portAllocator - */ - -/** - * 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 + * @property {undefined} reserved reserve a state key for future use. can hold + * an additional power or a record of powers */ /** @typedef {MapStore} ICQConnectionStore */ @@ -44,16 +34,10 @@ const { Vow$ } = NetworkShape; // TODO #9611 /** @typedef {ChainAccountKit | ICQConnectionKit} ConnectionKit */ /** - * @template {keyof OrchestrationPowers} K - * @param {PowerStore} powers - * @param {K} name + * @typedef {{ + * icqConnections: ICQConnectionStore; + * } & 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 */ /** * @param {Zone} zone @@ -102,17 +86,17 @@ const prepareCosmosOrchestrationServiceKit = ( ), }), }, - /** @param {Partial} [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} powers */ + powers => { + mustMatch(powers?.portAllocator, M.remotable('PortAllocator')); const icqConnections = zone.detached().mapStore('ICQConnections'); - return /** @type {OrchestrationState} */ ({ powers, icqConnections }); + return harden( + /** @type {OrchestrationState} */ ({ + icqConnections, + reserved: undefined, + ...powers, + }), + ); }, { requestICAChannelWatcher: { @@ -203,7 +187,7 @@ const prepareCosmosOrchestrationServiceKit = ( controllerConnectionId, opts, ); - const portAllocator = getPower(this.state.powers, 'portAllocator'); + const { portAllocator } = this.state; return watch( E(portAllocator).allocateICAControllerPort(), this.facets.requestICAChannelWatcher, @@ -224,7 +208,7 @@ const prepareCosmosOrchestrationServiceKit = ( .connection; } const remoteConnAddr = makeICQChannelAddress(controllerConnectionId); - const portAllocator = getPower(this.state.powers, 'portAllocator'); + const { portAllocator } = this.state; return watch( // allocate a new Port for every Connection // TODO #9317 optimize ICQ port allocation @@ -238,6 +222,13 @@ const prepareCosmosOrchestrationServiceKit = ( }, }, }, + { + stateShape: { + icqConnections: M.remotable('icqConnections mapStore'), + 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 index a8a3de2f976..1f38c512f0c 100644 --- 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' */ /** From 3dc6bb616173415f2556cd479a84740e38ad619a Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Tue, 30 Jul 2024 18:34:21 -0400 Subject: [PATCH 3/6] feat: cosmosInterchainService.provideICQConnectiont accepts version - allow caller to specify optional version string - update persitence logic to key off of ${controllerConnectionId}:${version} --- .../src/exos/cosmos-interchain-service.js | 61 ++++++++++++------- packages/orchestration/src/utils/address.js | 6 +- packages/orchestration/test/service.test.ts | 28 +++++++-- .../orchestration/test/utils/address.test.ts | 8 +-- 4 files changed, 67 insertions(+), 36 deletions(-) diff --git a/packages/orchestration/src/exos/cosmos-interchain-service.js b/packages/orchestration/src/exos/cosmos-interchain-service.js index 824da490e59..08bf7901f56 100644 --- a/packages/orchestration/src/exos/cosmos-interchain-service.js +++ b/packages/orchestration/src/exos/cosmos-interchain-service.js @@ -6,6 +6,7 @@ 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'; @@ -29,7 +30,7 @@ const { Vow$ } = NetworkShape; // TODO #9611 * an additional power or a record of powers */ -/** @typedef {MapStore} ICQConnectionStore */ +/** @typedef {MapStore} ICQConnectionStore */ /** @typedef {ChainAccountKit | ICQConnectionKit} ConnectionKit */ @@ -39,6 +40,18 @@ const { Vow$ } = NetworkShape; // TODO #9611 * } & OrchestrationPowers} 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 * @param {VowTools} vowTools @@ -63,7 +76,7 @@ const prepareCosmosOrchestrationServiceKit = ( onFulfilled: M.call(M.remotable('Port')) .optional({ remoteConnAddr: M.string(), - controllerConnectionId: M.string(), + icqLookupKey: M.string(), }) .returns(Vow$(NetworkShape.Connection)), }), @@ -72,7 +85,7 @@ const prepareCosmosOrchestrationServiceKit = ( .optional( M.splitRecord( { connectionKit: M.record(), returnFacet: M.string() }, - { saveICQConnection: M.string() }, + { icqLookupKey: M.string() }, ), ) .returns(M.remotable('ConnectionKit Holder facet')), @@ -81,9 +94,9 @@ const prepareCosmosOrchestrationServiceKit = ( makeAccount: M.call(M.string(), M.string(), M.string()) .optional(M.record()) .returns(Vow$(M.remotable('ChainAccountKit'))), - provideICQConnection: M.call(M.string()).returns( - Vow$(M.remotable('ICQConnection')), - ), + provideICQConnection: M.call(M.string()) + .optional(M.string()) + .returns(Vow$(M.remotable('ICQConnection'))), }), }, /** @param {Partial} powers */ @@ -125,10 +138,10 @@ const prepareCosmosOrchestrationServiceKit = ( * @param {Port} port * @param {{ * remoteConnAddr: RemoteIbcAddress; - * controllerConnectionId: IBCConnectionID; + * icqLookupKey: string; * }} watchContext */ - onFulfilled(port, { remoteConnAddr, controllerConnectionId }) { + onFulfilled(port, { remoteConnAddr, icqLookupKey }) { const connectionKit = makeICQConnectionKit(port); /** @param {ICQConnectionKit} kit */ return watch( @@ -137,7 +150,7 @@ const prepareCosmosOrchestrationServiceKit = ( { connectionKit, returnFacet: 'connection', - saveICQConnection: controllerConnectionId, + icqLookupKey, }, ); }, @@ -153,16 +166,13 @@ 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), ); } @@ -199,15 +209,22 @@ const prepareCosmosOrchestrationServiceKit = ( }, /** * @param {IBCConnectionID} controllerConnectionId + * @param {string} [version] * @returns {Vow | ICQConnection} */ - provideICQConnection(controllerConnectionId) { - if (this.state.icqConnections.has(controllerConnectionId)) { + provideICQConnection(controllerConnectionId, version) { + const icqLookupKey = getICQConnectionKey( + controllerConnectionId, + version, + ); + if (this.state.icqConnections.has(icqLookupKey)) { // TODO #9281 do not return synchronously. see https://github.com/Agoric/agoric-sdk/pull/9454#discussion_r1626898694 - return this.state.icqConnections.get(controllerConnectionId) - .connection; + return this.state.icqConnections.get(icqLookupKey).connection; } - const remoteConnAddr = makeICQChannelAddress(controllerConnectionId); + const remoteConnAddr = makeICQChannelAddress( + controllerConnectionId, + version, + ); const { portAllocator } = this.state; return watch( // allocate a new Port for every Connection @@ -216,7 +233,7 @@ const prepareCosmosOrchestrationServiceKit = ( this.facets.requestICQChannelWatcher, { remoteConnAddr, - controllerConnectionId, + icqLookupKey, }, ); }, diff --git a/packages/orchestration/src/utils/address.js b/packages/orchestration/src/utils/address.js index 29dd07e0087..f3a857c8aa5 100644 --- a/packages/orchestration/src/utils/address.js +++ b/packages/orchestration/src/utils/address.js @@ -46,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}`; diff --git a/packages/orchestration/test/service.test.ts b/packages/orchestration/test/service.test.ts index d1473dd10cf..4b2a921bf78 100644 --- a/packages/orchestration/test/service.test.ts +++ b/packages/orchestration/test/service.test.ts @@ -16,13 +16,15 @@ import { commonSetup } from './supports.js'; import { ChainAddressShape } from '../src/typeGuards.js'; import { tryDecodeResponse } from '../src/utils/cosmos.js'; +const CHAIN_ID = 'cosmoshub-99'; +const HOST_CONNECTION_ID = 'connection-0'; +const CONTROLLER_CONNECTION_ID = 'connection-1'; + test('makeICQConnection returns an ICQConnection', async t => { const { bootstrap: { cosmosInterchainService }, } = await commonSetup(t); - const CONTROLLER_CONNECTION_ID = 'connection-0'; - const icqConnection = await E(cosmosInterchainService).provideICQConnection( CONTROLLER_CONNECTION_ID, ); @@ -64,11 +66,25 @@ test('makeICQConnection returns an ICQConnection', async t => { t.like(QueryBalanceResponse.decode(decodeBase64(result.key)), { balance: { amount: '0', denom: 'uatom' }, }); -}); -const CHAIN_ID = 'cosmoshub-99'; -const HOST_CONNECTION_ID = 'connection-0'; -const CONTROLLER_CONNECTION_ID = 'connection-1'; + const icqConnection3 = await E(cosmosInterchainService).provideICQConnection( + CONTROLLER_CONNECTION_ID, + 'icq-2', + ); + const localAddr3 = await E(icqConnection3).getLocalAddress(); + t.not( + localAddr3, + localAddr2, + 'non default version results in a new connection', + ); + + const icqConnection4 = await E(cosmosInterchainService).provideICQConnection( + CONTROLLER_CONNECTION_ID, + 'icq-2', + ); + const localAddr4 = await E(icqConnection4).getLocalAddress(); + t.is(localAddr3, localAddr4, 'custom version is idempotent'); +}); test('makeAccount returns a ChainAccount', async t => { const { diff --git a/packages/orchestration/test/utils/address.test.ts b/packages/orchestration/test/utils/address.test.ts index 5e624feb000..e19a7c2cf13 100644 --- a/packages/orchestration/test/utils/address.test.ts +++ b/packages/orchestration/test/utils/address.test.ts @@ -84,18 +84,14 @@ test('makeICQChannelAddress', t => { 'returns connection string when controllerConnectionId is provided', ); t.is( - makeICQChannelAddress('connection-0', { - version: 'icq-2', - }), + makeICQChannelAddress('connection-0', 'icq-2'), '/ibc-hop/connection-0/ibc-port/icqhost/unordered/icq-2', 'accepts custom version', ); t.throws( () => validateRemoteIbcAddress( - makeICQChannelAddress('connection-0', { - version: 'ic/q-/2', - }), + makeICQChannelAddress('connection-0', 'ic/q-/2'), ), { message: From d4101a4220155f1c5c84e80ba98cece7fd39937a Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Tue, 30 Jul 2024 18:36:37 -0400 Subject: [PATCH 4/6] chore: provideICQConnection should always return a vow - in the case of a potential early synchronous return, still return a vow --- .../orchestration/src/exos/cosmos-interchain-service.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/orchestration/src/exos/cosmos-interchain-service.js b/packages/orchestration/src/exos/cosmos-interchain-service.js index 08bf7901f56..78cce29de6b 100644 --- a/packages/orchestration/src/exos/cosmos-interchain-service.js +++ b/packages/orchestration/src/exos/cosmos-interchain-service.js @@ -60,7 +60,7 @@ const getICQConnectionKey = (controllerConnectionId, version) => { */ const prepareCosmosOrchestrationServiceKit = ( zone, - { watch }, + { watch, asVow }, makeChainAccountKit, makeICQConnectionKit, ) => @@ -218,8 +218,9 @@ const prepareCosmosOrchestrationServiceKit = ( version, ); if (this.state.icqConnections.has(icqLookupKey)) { - // TODO #9281 do not return synchronously. see https://github.com/Agoric/agoric-sdk/pull/9454#discussion_r1626898694 - return this.state.icqConnections.get(icqLookupKey).connection; + return asVow( + () => this.state.icqConnections.get(icqLookupKey).connection, + ); } const remoteConnAddr = makeICQChannelAddress( controllerConnectionId, From 00fe7edc76660323895aafdc8a236b6f65ede10d Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Tue, 30 Jul 2024 23:01:13 -0400 Subject: [PATCH 5/6] fix(network-fakes): do not rely on source_port for channelId - a port can be reused to create multiple channels. requesting a new channel with the same port should result in a new channel id - uses channelCount to increment channelID. keeps a map of channels per connection to determine counterpartyChannelID --- packages/orchestration/test/network-fakes.ts | 39 ++++++++++++++++---- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/packages/orchestration/test/network-fakes.ts b/packages/orchestration/test/network-fakes.ts index 18cbddc081a..054e192c245 100644 --- a/packages/orchestration/test/network-fakes.ts +++ b/packages/orchestration/test/network-fakes.ts @@ -11,6 +11,7 @@ import type { IBCMethod, IBCEvent, ScopedBridgeManagerMethods, + IBCConnectionID, } from '@agoric/vats'; import { prepareCallbacks as prepareIBCCallbacks, @@ -53,7 +54,12 @@ export const ibcBridgeMocks: { [T in ImplementedIBCEvents]: T extends 'channelOpenAck' ? ( obj: IBCMethod<'startChannelOpenInit'>, - opts: { bech32Prefix: string; sequence: number }, + opts: { + bech32Prefix: string; + sequence: number; + channelID: IBCChannelID; + counterpartyChannelID: IBCChannelID; + }, ) => IBCEvent<'channelOpenAck'> : T extends 'acknowledgementPacket' ? ( @@ -64,11 +70,18 @@ export const ibcBridgeMocks: { } = { channelOpenAck: ( obj: IBCMethod<'startChannelOpenInit'>, - { bech32Prefix, sequence }: { bech32Prefix: string; sequence: number }, + { + bech32Prefix, + sequence, + channelID, + counterpartyChannelID, + }: { + bech32Prefix: string; + sequence: number; + channelID: IBCChannelID; + counterpartyChannelID: IBCChannelID; + }, ): IBCEvent<'channelOpenAck'> => { - const mocklID = Number(obj.packet.source_port.split('-').at(-1)); - const mockLocalChannelID: IBCChannelID = `channel-${mocklID}`; - const mockRemoteChannelID: IBCChannelID = `channel-${mocklID}`; const mockChainAddress = sequence > 0 ? `${bech32Prefix}1test${sequence}` : `${bech32Prefix}1test`; @@ -78,10 +91,10 @@ export const ibcBridgeMocks: { blockTime: 1711571357, event: 'channelOpenAck', portID: obj.packet.source_port, - channelID: mockLocalChannelID, + channelID, counterparty: { port_id: obj.packet.destination_port, - channel_id: mockRemoteChannelID, + channel_id: counterpartyChannelID, }, counterpartyVersion: addParamsIfJsonVersion(obj.version, { address: mockChainAddress, @@ -149,12 +162,18 @@ export const makeFakeIBCBridge = ( let ibcSequenceNonce = 0; /** * The number of channels created. Currently used as a proxy to increment - * fake account addresses. + * fake account addresses and channels. * @type {nubmer} */ let channelCount = 0; let bech32Prefix = 'cosmos'; + /** + * Keep track channels requested by remote chain. Used as a proxy for + * counterpaty channel ids + */ + const remoteChannelMap: Record = {}; + /** * Packet byte string map of requests to responses * @type {Record} @@ -168,13 +187,17 @@ export const makeFakeIBCBridge = ( if (obj.type === 'IBC_METHOD') { switch (obj.method) { case 'startChannelOpenInit': { + const connectionChannelCount = remoteChannelMap[obj.hops[0]] || 0; const ackEvent = ibcBridgeMocks.channelOpenAck(obj, { bech32Prefix, sequence: channelCount, + channelID: `channel-${channelCount}`, + counterpartyChannelID: `channel-${connectionChannelCount}`, }); bridgeHandler?.fromBridge(ackEvent); bridgeEvents = bridgeEvents.concat(ackEvent); channelCount += 1; + remoteChannelMap[obj.hops[0]] = connectionChannelCount + 1; return undefined; } case 'sendPacket': { From d80a57cbaed855fe9ee64039cef3fdd1c79fd7c9 Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Tue, 30 Jul 2024 23:04:39 -0400 Subject: [PATCH 6/6] feat: use a single port for all icq connections - changes logic in provieICQConnection to lazily request a port on the first request, and reuse it in subsequent requests. since the channel is ordered, timeouts and query errors will not affect subsequent queries and this can be considered safe - closes #9317 --- .../src/exos/cosmos-interchain-service.js | 40 +++++++++---------- packages/orchestration/test/service.test.ts | 13 ++++++ 2 files changed, 32 insertions(+), 21 deletions(-) diff --git a/packages/orchestration/src/exos/cosmos-interchain-service.js b/packages/orchestration/src/exos/cosmos-interchain-service.js index 78cce29de6b..ba56d40e055 100644 --- a/packages/orchestration/src/exos/cosmos-interchain-service.js +++ b/packages/orchestration/src/exos/cosmos-interchain-service.js @@ -37,6 +37,7 @@ const { Vow$ } = NetworkShape; // TODO #9611 /** * @typedef {{ * icqConnections: ICQConnectionStore; + * sharedICQPort: Remote | undefined; * } & OrchestrationPowers} OrchestrationState */ @@ -103,13 +104,12 @@ const prepareCosmosOrchestrationServiceKit = ( powers => { mustMatch(powers?.portAllocator, M.remotable('PortAllocator')); const icqConnections = zone.detached().mapStore('ICQConnections'); - return harden( - /** @type {OrchestrationState} */ ({ - icqConnections, - reserved: undefined, - ...powers, - }), - ); + return /** @type {OrchestrationState} */ ({ + icqConnections, + sharedICQPort: undefined, + reserved: undefined, + ...powers, + }); }, { requestICAChannelWatcher: { @@ -142,8 +142,10 @@ const prepareCosmosOrchestrationServiceKit = ( * }} watchContext */ 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, @@ -178,8 +180,6 @@ const prepareCosmosOrchestrationServiceKit = ( } return connectionKit[returnFacet]; }, - // TODO #9317 if we fail, should we revoke the port (if it was created in this flow)? - // onRejected() {} }, public: { /** @@ -226,23 +226,21 @@ const prepareCosmosOrchestrationServiceKit = ( controllerConnectionId, version, ); - const { portAllocator } = this.state; - return watch( - // allocate a new Port for every Connection - // TODO #9317 optimize ICQ port allocation - E(portAllocator).allocateICQControllerPort(), - this.facets.requestICQChannelWatcher, - { - remoteConnAddr, - icqLookupKey, - }, - ); + 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/test/service.test.ts b/packages/orchestration/test/service.test.ts index 4b2a921bf78..1244a680897 100644 --- a/packages/orchestration/test/service.test.ts +++ b/packages/orchestration/test/service.test.ts @@ -12,6 +12,7 @@ import { Any } from '@agoric/cosmic-proto/google/protobuf/any.js'; import { matches } from '@endo/patterns'; 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 { commonSetup } from './supports.js'; import { ChainAddressShape } from '../src/typeGuards.js'; import { tryDecodeResponse } from '../src/utils/cosmos.js'; @@ -84,6 +85,18 @@ test('makeICQConnection returns an ICQConnection', async t => { ); const localAddr4 = await E(icqConnection4).getLocalAddress(); t.is(localAddr3, localAddr4, 'custom version is idempotent'); + + const icqConnection5 = await E(cosmosInterchainService).provideICQConnection( + 'connection-99', + ); + const localAddr5 = await E(icqConnection5).getLocalAddress(); + + const getPortId = (lAddr: LocalIbcAddress) => lAddr.split('/')[2]; + const uniquePortIds = new Set( + [localAddr, localAddr2, localAddr3, localAddr4, localAddr5].map(getPortId), + ); + t.regex([...uniquePortIds][0], /icqcontroller-\d+/); + t.is(uniquePortIds.size, 1, 'all connections share same port'); }); test('makeAccount returns a ChainAccount', async t => {