Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: cosmos interchain service cleanup #9810

Merged
merged 6 commits into from
Jul 31, 2024
Merged

chore: cosmos interchain service cleanup #9810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8cad5e3
feat: cosmosInterchainService.makeAccount accepts address opts
0xpatrickdev Jul 30, 2024
1989138
refactor: unnecessary powerStore abstraction
0xpatrickdev Jul 30, 2024
3dc6bb6
feat: cosmosInterchainService.provideICQConnectiont accepts version
0xpatrickdev Jul 30, 2024
d4101a4
chore: provideICQConnection should always return a vow
0xpatrickdev Jul 30, 2024
00fe7ed
fix(network-fakes): do not rely on source_port for channelId
0xpatrickdev Jul 31, 2024
d80a57c
feat: use a single port for all icq connections
0xpatrickdev Jul 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 83 additions & 72 deletions packages/orchestration/src/exos/cosmos-interchain-service.js
View file
Open in desktop
turadg marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/** @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';
import {
DEFAULT_ICQ_VERSION,
makeICAChannelAddress,
makeICQChannelAddress,
} from '../utils/address.js';
Expand All @@ -19,40 +19,39 @@ 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
/**
* @typedef {object} OrchestrationPowers
* @property {Remote<PortAllocator>} portAllocator
* @property {undefined} reserved reserve a state key for future use. can hold
* an additional power or a record of powers
*/

/**
* 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
*/

/** @typedef {MapStore<IBCConnectionID, ICQConnectionKit>} ICQConnectionStore */
/** @typedef {MapStore<string, ICQConnectionKit>} ICQConnectionStore */

/** @typedef {ChainAccountKit | ICQConnectionKit} ConnectionKit */

/**
* @template {keyof OrchestrationPowers} K
* @param {PowerStore} powers
* @param {K} name
* @typedef {{
* icqConnections: ICQConnectionStore;
* sharedICQPort: Remote<Port> | undefined;
* } & 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 */
/**
* 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
Expand All @@ -62,7 +61,7 @@ const getPower = (powers, name) => {
*/
const prepareCosmosOrchestrationServiceKit = (
zone,
{ watch },
{ watch, asVow },
makeChainAccountKit,
makeICQConnectionKit,
) =>
Expand All @@ -78,7 +77,7 @@ const prepareCosmosOrchestrationServiceKit = (
onFulfilled: M.call(M.remotable('Port'))
.optional({
remoteConnAddr: M.string(),
controllerConnectionId: M.string(),
icqLookupKey: M.string(),
})
.returns(Vow$(NetworkShape.Connection)),
}),
Expand All @@ -87,31 +86,30 @@ const prepareCosmosOrchestrationServiceKit = (
.optional(
M.splitRecord(
{ connectionKit: M.record(), returnFacet: M.string() },
{ saveICQConnection: M.string() },
{ icqLookupKey: M.string() },
),
)
.returns(M.remotable('ConnectionKit Holder facet')),
}),
public: M.interface('CosmosInterchainService', {
makeAccount: M.call(M.string(), M.string(), M.string()).returns(
Vow$(M.remotable('ChainAccountKit')),
),
provideICQConnection: M.call(M.string()).returns(
Vow$(M.remotable('ICQConnection')),
),
makeAccount: M.call(M.string(), M.string(), M.string())
.optional(M.record())
.returns(Vow$(M.remotable('ChainAccountKit'))),
provideICQConnection: M.call(M.string())
.optional(M.string())
.returns(Vow$(M.remotable('ICQConnection'))),
}),
},
/** @param {Partial<OrchestrationPowers>} [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<OrchestrationPowers>} powers */
powers => {
mustMatch(powers?.portAllocator, M.remotable('PortAllocator'));
const icqConnections = zone.detached().mapStore('ICQConnections');
return /** @type {OrchestrationState} */ ({ powers, icqConnections });
return /** @type {OrchestrationState} */ ({
icqConnections,
sharedICQPort: undefined,
reserved: undefined,
...powers,
});
},
{
requestICAChannelWatcher: {
Expand Down Expand Up @@ -140,19 +138,21 @@ const prepareCosmosOrchestrationServiceKit = (
* @param {Port} port
* @param {{
* remoteConnAddr: RemoteIbcAddress;
* controllerConnectionId: IBCConnectionID;
* icqLookupKey: string;
* }} watchContext
*/
onFulfilled(port, { remoteConnAddr, controllerConnectionId }) {
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,
{
connectionKit,
returnFacet: 'connection',
saveICQConnection: controllerConnectionId,
icqLookupKey,
},
);
},
Expand All @@ -168,38 +168,36 @@ 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),
);
}
return connectionKit[returnFacet];
},
// TODO #9317 if we fail, should we revoke the port (if it was created in this flow)?
// onRejected() {}
},
public: {
/**
* @param {string} chainId
* @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<IcaAccount>}
*/
makeAccount(chainId, hostConnectionId, controllerConnectionId) {
makeAccount(chainId, hostConnectionId, controllerConnectionId, opts) {
const remoteConnAddr = makeICAChannelAddress(
hostConnectionId,
controllerConnectionId,
opts,
);
const portAllocator = getPower(this.state.powers, 'portAllocator');
const { portAllocator } = this.state;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yay for simpler. what happens if we need to provide new powers?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely certain and figured it'd be easiest to get feedback in the PR.

I based reserved on what we did for localchain.js. This would indicate we can only add more powers here.

A more recent read of #7337 leads me to believe StateShape only matters if we specify opts.stateShape for the 5th arg? We can remove reserved and rely on ...powers in the initState function for new powers? If not, it'd be great to understand better.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only matters if we specify opts.stateShape

no; you can't add new properties, even without stateShape

Once the state is defined by the init function (3rd arg), properties cannot be added or removed.
-- https://docs.agoric.com/guides/zoe/contract-upgrade.html

reserved lets us put a new record there, or a store like powers was, should we need one.

Also, lacking that, there's the "side table" approach used to add non-vbank asset support to the smart wallet in #8071.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying @dckc . I added 9e16702 which spreads powers after reserved and defines reserved on OrchestrationPowers. This way, we can define extra powers via powers.reserved if needed.

return watch(
E(portAllocator).allocateICAControllerPort(),
this.facets.requestICAChannelWatcher,
Expand All @@ -211,29 +209,42 @@ const prepareCosmosOrchestrationServiceKit = (
},
/**
* @param {IBCConnectionID} controllerConnectionId
* @param {string} [version]
* @returns {Vow<ICQConnection> | ICQConnection}
*/
provideICQConnection(controllerConnectionId) {
if (this.state.icqConnections.has(controllerConnectionId)) {
// TODO #9281 do not return synchronously. see https://github.com/Agoric/agoric-sdk/pull/9454#discussion_r1626898694
return this.state.icqConnections.get(controllerConnectionId)
.connection;
provideICQConnection(controllerConnectionId, version) {
const icqLookupKey = getICQConnectionKey(
controllerConnectionId,
version,
);
if (this.state.icqConnections.has(icqLookupKey)) {
return asVow(
() => this.state.icqConnections.get(icqLookupKey).connection,
);
}
const remoteConnAddr = makeICQChannelAddress(controllerConnectionId);
const portAllocator = getPower(this.state.powers, 'portAllocator');
return watch(
// allocate a new Port for every Connection
// TODO #9317 optimize ICQ port allocation
E(portAllocator).allocateICQControllerPort(),
this.facets.requestICQChannelWatcher,
{
remoteConnAddr,
controllerConnectionId,
},
const remoteConnAddr = makeICQChannelAddress(
controllerConnectionId,
version,
);
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(),
},
},
);

/**
Expand Down
1 change: 0 additions & 1 deletion packages/orchestration/src/proposals/orchestration-proposal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const trace = makeTracer('CoreEvalOrchestration', true);

/**
* @import {PortAllocator} from '@agoric/network';
* @import {CosmosInterchainService} from '../exos/cosmos-interchain-service.js'
*/

/**
Expand Down
24 changes: 15 additions & 9 deletions packages/orchestration/src/utils/address.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = (
Expand All @@ -42,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}`;
Expand Down
Loading
Loading