Skip to content

Commit

Permalink
fix: add TODOs for early synchronous returns
Browse files Browse the repository at this point in the history
  • Loading branch information
0xpatrickdev authored and turadg committed Jun 19, 2024
1 parent 80b204c commit 8f9da6b
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 18 deletions.
10 changes: 7 additions & 3 deletions packages/orchestration/src/exos/chain-account-kit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand All @@ -87,7 +87,7 @@ export const prepareChainAccountKit = (zone, { watch, when }) =>
localAddress: undefined,
}),
{
handleExecuteEncodedTxWatcher: {
parseTxPacketWatcher: {
/** @param {string} ack */
onFulfilled(ack) {
return parseTxPacket(ack);
Expand Down Expand Up @@ -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,
),
);
},
Expand All @@ -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()));
},
Expand Down
18 changes: 9 additions & 9 deletions packages/orchestration/src/exos/icq-connection-kit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 =>
Expand Down Expand Up @@ -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);
Expand Down
13 changes: 7 additions & 6 deletions packages/orchestration/src/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -114,7 +114,7 @@ const prepareOrchestrationKit = (
return /** @type {OrchestrationState} */ ({ powers, icqConnections });
},
{
requestICAConnectionWatcher: {
requestICAChannelWatcher: {
/**
* @param {Port} port
* @param {{
Expand All @@ -135,7 +135,7 @@ const prepareOrchestrationKit = (
);
},
},
requestICQConnectionWatcher: {
requestICQChannelWatcher: {
/**
* @param {Port} port
* @param {{
Expand Down Expand Up @@ -203,7 +203,7 @@ const prepareOrchestrationKit = (
return when(
watch(
E(portAllocator).allocateICAControllerPort(),
this.facets.requestICAConnectionWatcher,
this.facets.requestICAChannelWatcher,
{
chainId,
remoteConnAddr,
Expand All @@ -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;
}
Expand All @@ -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,
Expand Down
6 changes: 6 additions & 0 deletions packages/orchestration/test/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 8f9da6b

Please sign in to comment.