From 8f9da6ba24a944959071e67e5f8eb357bb761a6b Mon Sep 17 00:00:00 2001 From: 0xPatrick Date: Thu, 6 Jun 2024 11:28:53 -0400 Subject: [PATCH] fix: add TODOs for early synchronous returns --- .../src/exos/chain-account-kit.js | 10 +++++++--- .../src/exos/icq-connection-kit.js | 18 +++++++++--------- packages/orchestration/src/service.js | 13 +++++++------ packages/orchestration/test/service.test.ts | 6 ++++++ 4 files changed, 29 insertions(+), 18 deletions(-) diff --git a/packages/orchestration/src/exos/chain-account-kit.js b/packages/orchestration/src/exos/chain-account-kit.js index bc5cc2b82517..6cec4150694c 100644 --- a/packages/orchestration/src/exos/chain-account-kit.js +++ b/packages/orchestration/src/exos/chain-account-kit.js @@ -65,7 +65,7 @@ export const prepareChainAccountKit = (zone, { watch, when }) => { account: ChainAccountI, connectionHandler: ConnectionHandlerI, - handleExecuteEncodedTxWatcher: M.interface('HandleQueryWatcher', { + parseTxPacketWatcher: M.interface('ParseTxPacketWatcher', { onFulfilled: M.call(M.string()) .optional(M.arrayOf(M.undefined())) // does not need watcherContext .returns(M.string()), @@ -87,7 +87,7 @@ export const prepareChainAccountKit = (zone, { watch, when }) => localAddress: undefined, }), { - handleExecuteEncodedTxWatcher: { + parseTxPacketWatcher: { /** @param {string} ack */ onFulfilled(ack) { return parseTxPacket(ack); @@ -139,11 +139,13 @@ export const prepareChainAccountKit = (zone, { watch, when }) => */ executeEncodedTx(msgs, opts) { const { connection } = this.state; + // TODO #9281 do not throw synchronously when returning a promise; return a rejected Vow + /// see https://github.com/Agoric/agoric-sdk/pull/9454#discussion_r1626898694 if (!connection) throw Fail`connection not available`; return when( watch( E(connection).send(makeTxPacket(msgs, opts)), - this.facets.handleExecuteEncodedTxWatcher, + this.facets.parseTxPacketWatcher, ), ); }, @@ -153,6 +155,8 @@ export const prepareChainAccountKit = (zone, { watch, when }) => // - retrieve assets? // - revoke the port? const { connection } = this.state; + // TODO #9281 do not throw synchronously when returning a promise; return a rejected Vow + /// see https://github.com/Agoric/agoric-sdk/pull/9454#discussion_r1626898694 if (!connection) throw Fail`connection not available`; return when(watch(E(connection).close())); }, diff --git a/packages/orchestration/src/exos/icq-connection-kit.js b/packages/orchestration/src/exos/icq-connection-kit.js index 238b1e79ac38..e54d45996520 100644 --- a/packages/orchestration/src/exos/icq-connection-kit.js +++ b/packages/orchestration/src/exos/icq-connection-kit.js @@ -29,12 +29,6 @@ export const ICQConnectionI = M.interface('ICQConnection', { query: M.call(M.arrayOf(ICQMsgShape)).returns(M.promise()), }); -const HandleQueryWatcherI = M.interface('HandleQueryWatcher', { - onFulfilled: M.call(M.string()) - .optional(M.arrayOf(M.undefined())) // does not need watcherContext - .returns(M.arrayOf(M.record())), -}); - /** * @typedef {{ * port: Port; @@ -65,8 +59,12 @@ export const prepareICQConnectionKit = (zone, { watch, when }) => 'ICQConnectionKit', { connection: ICQConnectionI, - handleQueryWatcher: HandleQueryWatcherI, connectionHandler: ConnectionHandlerI, + parseQueryPacketWatcher: M.interface('ParseQueryPacketWatcher', { + onFulfilled: M.call(M.string()) + .optional(M.arrayOf(M.undefined())) // does not need watcherContext + .returns(M.arrayOf(M.record())), + }), }, /** @param {Port} port */ port => @@ -97,16 +95,18 @@ export const prepareICQConnectionKit = (zone, { watch, when }) => */ query(msgs) { const { connection } = this.state; + // TODO #9281 do not throw synchronously when returning a promise; return a rejected Vow + /// see https://github.com/Agoric/agoric-sdk/pull/9454#discussion_r1626898694 if (!connection) throw Fail`connection not available`; return when( watch( E(connection).send(makeQueryPacket(msgs)), - this.facets.handleQueryWatcher, + this.facets.parseQueryPacketWatcher, ), ); }, }, - handleQueryWatcher: { + parseQueryPacketWatcher: { /** @param {string} ack packet acknowledgement string */ onFulfilled(ack) { return parseQueryPacket(ack); diff --git a/packages/orchestration/src/service.js b/packages/orchestration/src/service.js index e087193b69a1..ffaeb936b606 100644 --- a/packages/orchestration/src/service.js +++ b/packages/orchestration/src/service.js @@ -69,12 +69,12 @@ const prepareOrchestrationKit = ( zone.exoClassKit( 'Orchestration', { - requestICAConnectionWatcher: M.interface('RequestICAConnectionWatcher', { + requestICAChannelWatcher: M.interface('RequestICAChannelWatcher', { onFulfilled: M.call(M.remotable('Port')) .optional({ chainId: M.string(), remoteConnAddr: M.string() }) .returns(NetworkShape.Vow$(NetworkShape.Connection)), }), - requestICQConnectionWatcher: M.interface('RequestICQConnectionWatcher', { + requestICQChannelWatcher: M.interface('RequestICQChannelWatcher', { onFulfilled: M.call(M.remotable('Port')) .optional({ remoteConnAddr: M.string(), @@ -114,7 +114,7 @@ const prepareOrchestrationKit = ( return /** @type {OrchestrationState} */ ({ powers, icqConnections }); }, { - requestICAConnectionWatcher: { + requestICAChannelWatcher: { /** * @param {Port} port * @param {{ @@ -135,7 +135,7 @@ const prepareOrchestrationKit = ( ); }, }, - requestICQConnectionWatcher: { + requestICQChannelWatcher: { /** * @param {Port} port * @param {{ @@ -203,7 +203,7 @@ const prepareOrchestrationKit = ( return when( watch( E(portAllocator).allocateICAControllerPort(), - this.facets.requestICAConnectionWatcher, + this.facets.requestICAChannelWatcher, { chainId, remoteConnAddr, @@ -217,6 +217,7 @@ const prepareOrchestrationKit = ( */ 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; } @@ -227,7 +228,7 @@ const prepareOrchestrationKit = ( // allocate a new Port for every Connection // TODO #9317 optimize ICQ port allocation E(portAllocator).allocateICQControllerPort(), - this.facets.requestICQConnectionWatcher, + this.facets.requestICQChannelWatcher, { remoteConnAddr, controllerConnectionId, diff --git a/packages/orchestration/test/service.test.ts b/packages/orchestration/test/service.test.ts index a1ce3f543df7..b8ac167eeaf3 100644 --- a/packages/orchestration/test/service.test.ts +++ b/packages/orchestration/test/service.test.ts @@ -38,6 +38,12 @@ test('makeICQConnection returns an ICQConnection', async t => { 'remote address contains icqhost port, unordered ordering, and icq-1 version string', ); + const icqConnection2 = await E(orchestration).provideICQConnection( + CONTROLLER_CONNECTION_ID, + ); + const localAddr2 = await E(icqConnection2).getLocalAddress(); + t.is(localAddr, localAddr2, 'provideICQConnection is idempotent'); + await t.throwsAsync( E(icqConnection).query([ toRequestQueryJson(