Skip to content

Commit

Permalink
feat: use a single port for all icq connections
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
0xpatrickdev committed Jul 31, 2024
1 parent 0e4af51 commit 7d3177c
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 21 deletions.
40 changes: 19 additions & 21 deletions packages/orchestration/src/exos/cosmos-interchain-service.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const { Vow$ } = NetworkShape; // TODO #9611
/**
* @typedef {{
* icqConnections: ICQConnectionStore;
* sharedICQPort: Remote<Port> | undefined;
* } & OrchestrationPowers} OrchestrationState
*/

Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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: {
/**
Expand Down Expand Up @@ -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(),
},
Expand Down
13 changes: 13 additions & 0 deletions packages/orchestration/test/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 => {
Expand Down

0 comments on commit 7d3177c

Please sign in to comment.