Skip to content

Commit

Permalink
fix: triage await safety for cli,inter,web
Browse files Browse the repository at this point in the history
  • Loading branch information
erights committed Nov 19, 2022
1 parent e5ed233 commit 2811d4a
Show file tree
Hide file tree
Showing 16 changed files with 103 additions and 21 deletions.
2 changes: 2 additions & 0 deletions packages/agoric-cli/src/cosmos.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ export default async function cosmosMain(progname, rawArgs, powers, opts) {
}

if (popts.pull) {
// This await is safe because "terminal-control-flow"
// eslint-disable-next-line @jessie.js/no-nested-await
const exitStatus = await pspawn('docker', ['pull', IMAGE]);
if (exitStatus) {
return exitStatus;
Expand Down
5 changes: 5 additions & 0 deletions packages/agoric-cli/src/deploy.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// IIUC the purpose of the agoric-cli package, we do not need to worry
// about await safety for any of the code in this package.
// TODO someone who understands this package better should verify this.
/* eslint-disable @jessie.js/no-nested-await */

/* global process setTimeout setInterval clearInterval */
/* eslint-disable no-await-in-loop */

Expand Down
5 changes: 5 additions & 0 deletions packages/agoric-cli/src/init.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// IIUC the purpose of the agoric-cli package, we do not need to worry
// about await safety for any of the code in this package.
// TODO someone who understands this package better should verify this.
/* eslint-disable @jessie.js/no-nested-await */

import chalk from 'chalk';
import { makePspawn } from './helpers.js';

Expand Down
5 changes: 5 additions & 0 deletions packages/agoric-cli/src/install.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// IIUC the purpose of the agoric-cli package, we do not need to worry
// about await safety for any of the code in this package.
// TODO someone who understands this package better should verify this.
/* eslint-disable @jessie.js/no-nested-await */

/* global process AggregateError Buffer */
import path from 'path';
import chalk from 'chalk';
Expand Down
3 changes: 3 additions & 0 deletions packages/agoric-cli/src/open.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ export default async function walletMain(progname, rawArgs, powers, opts) {
process.stdout.write(`${walletUrl}\n`);
if (opts.browser) {
const browser = opener(walletUrl);
// This await is safe because terminal-control-flow.
//
// eslint-disable-next-line @jessie.js/no-nested-await
await new Promise(resolve => browser.on('exit', resolve));
}
}
5 changes: 5 additions & 0 deletions packages/agoric-cli/src/publish.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// IIUC the purpose of the agoric-cli package, we do not need to worry
// about await safety for any of the code in this package.
// TODO someone who understands this package better should verify this.
/* eslint-disable @jessie.js/no-nested-await */

// @ts-check
/// <reference types="ses"/>

Expand Down
5 changes: 5 additions & 0 deletions packages/agoric-cli/src/set-defaults.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// IIUC the purpose of the agoric-cli package, we do not need to worry
// about await safety for any of the code in this package.
// TODO someone who understands this package better should verify this.
/* eslint-disable @jessie.js/no-nested-await */

import { basename } from 'path';
import { assert, details as X } from '@agoric/assert';
import {
Expand Down
5 changes: 5 additions & 0 deletions packages/agoric-cli/src/start.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
// IIUC the purpose of the agoric-cli package, we do not need to worry
// about await safety for any of the code in this package.
// TODO someone who understands this package better should verify this.
/* eslint-disable @jessie.js/no-nested-await */

/* global process setTimeout */
import chalk from 'chalk';
import { createHash } from 'crypto';
Expand Down
12 changes: 1 addition & 11 deletions packages/inter-protocol/src/collect.js
Original file line number Diff line number Diff line change
@@ -1,11 +1 @@
const { fromEntries, keys, values } = Object;

/** @type { <X, Y>(xs: X[], ys: Y[]) => [X, Y][]} */
export const zip = (xs, ys) => harden(xs.map((x, i) => [x, ys[+i]]));

/** @type { <K extends string, V>(obj: Record<K, ERef<V>>) => Promise<Record<K, V>> } */
export const allValues = async obj => {
const resolved = await Promise.all(values(obj));
// @ts-expect-error cast
return harden(fromEntries(zip(keys(obj), resolved)));
};
export { allValues, zip } from '@agoric/internal';
5 changes: 1 addition & 4 deletions packages/inter-protocol/src/proposals/committee-proposal.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import { deeplyFulfilledObject } from '@agoric/internal';
import { deeplyFulfilledObject, zip } from '@agoric/internal';
import { E } from '@endo/far';
import { reserveThenDeposit } from './utils.js';

const { values } = Object;

/** @type { <X, Y>(xs: X[], ys: Y[]) => [X, Y][]} */
const zip = (xs, ys) => xs.map((x, i) => [x, ys[i]]);

/**
* @param {import('./econ-behaviors').EconomyBootstrapPowers} powers
* @param {{ options: { voterAddresses: Record<string, string> }}} param1
Expand Down
12 changes: 12 additions & 0 deletions packages/inter-protocol/src/proposals/demoIssuers.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,11 @@ export const connectFaucet = async ({
case Stake.symbol:
return bldIssuerKit;
default: {
// This nested await is safe because "terminal-control-flow".
//
// This switch is the last statement within the enclosing
// function.
// eslint-disable-next-line @jessie.js/no-nested-await
const { mint, issuer, brand } = await provideCoin(
name,
vats.mints,
Expand All @@ -304,6 +309,8 @@ export const connectFaucet = async ({
// Use the bank layer for BLD, IST.
if (issuerName === Stake.symbol || issuerName === Stable.symbol) {
const purse = E(bank).getPurse(brand);
// This nested await is safe because "terminal-control-flow"
// eslint-disable-next-line @jessie.js/no-nested-await
await E(purse).deposit(payment);
} else {
toFaucet = [
Expand Down Expand Up @@ -491,6 +498,8 @@ export const fundAMM = async ({
async issuerName => {
switch (issuerName) {
case Stable.symbol: {
// This nested await is safe because "terminal-control-flow"
// eslint-disable-next-line @jessie.js/no-nested-await
const [issuer, brand] = await Promise.all([
centralIssuer,
centralBrand,
Expand Down Expand Up @@ -625,6 +634,9 @@ export const fundAMM = async ({
let fromCentral;

if (brandsWithPriceAuthorities.includes(brand)) {
// TODO FIXME This code should be refactored to make this
// await checkably safe, or to remove it, or to record here
// why it is actually safe.
({ toCentral, fromCentral } = await E(ammPublicFacet)
.getPriceAuthorities(brand)
.catch(_e => {
Expand Down
5 changes: 1 addition & 4 deletions packages/inter-protocol/src/proposals/startPSM.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { makeStorageNodeChild } from '@agoric/vats/src/lib-chainStorage.js';
import { makeRatio } from '@agoric/zoe/src/contractSupport/index.js';
import { E } from '@endo/far';
import { Stable } from '@agoric/vats/src/tokens.js';
import { deeplyFulfilledObject } from '@agoric/internal';
import { deeplyFulfilledObject, zip } from '@agoric/internal';
import { makeScalarMapStore } from '@agoric/vat-data';

import { reserveThenDeposit, reserveThenGetNamePaths } from './utils.js';
Expand Down Expand Up @@ -362,9 +362,6 @@ export const PSM_GOV_MANIFEST = {
},
};

/** @type { <X, Y>(xs: X[], ys: Y[]) => [X, Y][]} */
const zip = (xs, ys) => xs.map((x, i) => [x, ys[i]]);

/**
* @param {import('./econ-behaviors').EconomyBootstrapPowers} powers
* @param {{ options: { voterAddresses: Record<string, string> }}} param1
Expand Down
20 changes: 20 additions & 0 deletions packages/inter-protocol/src/vaultFactory/liquidateIncrementally.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,22 @@ const start = async zcf => {
trace('exiting processTranches loop');
return;
}
// This await almost certainly does not do what was intended.
// `oncePerBlock()` returns an async generator, not a promise,
// and does nothing observably effectful to anyone not pulling on
// the generator, which this code drops. AFAICT, this code is
// equivalent to `await null;`. It imposes a non-delayed turn boundary
// and does nothing else.
// See https://github.com/Agoric/agoric-sdk/issues/6220
await oncePerBlock();

// Determine the max allowed tranche size
//
// This nested await is safe because "terminal-control-flow".
//
// It occurs at the top level of the body of a terminal top level
// while.
// eslint-disable-next-line @jessie.js/no-nested-await
const { Secondary: poolCollateral, Central: poolCentral } = await E(
amm,
).getPoolAllocation(toSell.brand);
Expand All @@ -223,6 +236,11 @@ const start = async zcf => {

// this could use a unit rather than the tranche size to run concurrently
/** @type PriceQuote */
// This nested await is safe because "terminal-control-flow".
//
// It occurs at the top level of the body of a terminal top level
// while.
// eslint-disable-next-line @jessie.js/no-nested-await
const oracleQuote = await E(priceAuthority).quoteGiven(
tranche,
debtBrand,
Expand Down Expand Up @@ -300,6 +318,8 @@ const start = async zcf => {
const collateralBefore = debtorSeat.getAmountAllocated('In');
const proceedsBefore = debtorSeat.getAmountAllocated('Out');

// This nested await is safe because "top-of-for-await"
// eslint-disable-next-line @jessie.js/no-nested-await
await sellTranche(debtorSeat, collateral, debt, oracleLimit);

const proceedsAfter = debtorSeat.getAmountAllocated('Out');
Expand Down
14 changes: 13 additions & 1 deletion packages/inter-protocol/src/vaultFactory/vaultManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,16 @@ const helperBehavior = {

// The rest of this method will not happen until after a quote is received.
// This may not happen until much later, when the market changes.
// eslint-disable-next-line no-await-in-loop
//
// This nested await is safe because "terminal-control-flow"
//
// It occurs at top level of the body of a top level terminal
// while. But do note that the enclosing function is an asynchronous
// generator function and that there is a `yield` within the loop.
// I don't think that causes any additional worries, but I have not
// thought about it enough.
//
// eslint-disable-next-line no-await-in-loop, @jessie.js/no-nested-await
const quote = await E(ephemera.outstandingQuote).getPromise();
ephemera.outstandingQuote = null;
// When we receive a quote, we check whether the vault with the highest
Expand All @@ -486,7 +495,10 @@ const helperBehavior = {
}
}
}

for await (const next of eventualLiquidations()) {
// This nested await is safe because "top-of-for-await"
// eslint-disable-next-line @jessie.js/no-nested-await
await facets.helper.liquidateAndRemove(next);
trace('price check liq', state.collateralBrand, next && next[0]);
}
Expand Down
10 changes: 10 additions & 0 deletions packages/internal/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -352,3 +352,13 @@ export const fsStreamReady = stream =>
stream.on('ready', onReady);
stream.on('error', onError);
});

/** @type { <X, Y>(xs: X[], ys: Y[]) => [X, Y][]} */
export const zip = (xs, ys) => harden(xs.map((x, i) => [x, ys[+i]]));

/** @type { <K extends string, V>(obj: Record<K, V | PromiseLike<V>>) => Promise<Record<K, V>> } */
export const allValues = async obj => {
const resolved = await Promise.all(Object.values(obj));
// @ts-expect-error cast
return harden(fromEntries(zip(Object.keys(obj), resolved)));
};
11 changes: 10 additions & 1 deletion packages/web-components/src/admin-websocket-connector.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,16 @@ export const waitForBootstrap = async getBootstrap => {
let update = await getLoadingUpdate();
while (update.value.includes('wallet')) {
console.log('waiting for wallet');
// eslint-disable-next-line no-await-in-loop
// This await is safe because "terminal-combined-control-flow".
//
// It occurs at the top level of the loop body of a non-terminal top
// level loop, so we need to consider the zero-vs-non-zero iteration
// cases wrt the potentially stateful `getBootstrap()`. However, there is
// a turn boundary immediately prior to the loop, with no
// potentially stateful execution between that turn boundary and the loop.
// So, considering the loop and that previous await together,
// `getBootstrap()` in always called in a new turn.
// eslint-disable-next-line no-await-in-loop, @jessie.js/no-nested-await
update = await getLoadingUpdate(update.updateCount);
}

Expand Down

0 comments on commit 2811d4a

Please sign in to comment.