From 08b83f6d15f2f2ef7bbe7abc54860abdc333a2da Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Thu, 15 Sep 2022 18:56:47 -0700 Subject: [PATCH] fix: triage await safety --- .../src/controller/initializeSwingset.js | 29 +++++++- .../src/devices/plugin/device-plugin.js | 9 +++ packages/SwingSet/src/kernel/vat-warehouse.js | 22 +++++- packages/SwingSet/src/vats/network/network.js | 33 ++++++++- packages/agoric-cli/src/cosmos.js | 2 + packages/agoric-cli/src/deploy.js | 5 ++ packages/agoric-cli/src/init.js | 5 ++ packages/agoric-cli/src/install.js | 5 ++ packages/agoric-cli/src/open.js | 3 + packages/agoric-cli/src/publish.js | 5 ++ packages/agoric-cli/src/set-defaults.js | 5 ++ packages/agoric-cli/src/start.js | 5 ++ packages/cache/src/store.js | 8 +++ packages/cosmic-swingset/src/chain-main.js | 25 +++++-- packages/cosmic-swingset/src/launch-chain.js | 27 +++++++ packages/cosmic-swingset/src/sim-chain.js | 21 ++++-- .../src/installInPieces.js | 15 ++++ .../src/startInstance.js | 6 ++ .../src/writeCoreProposal.js | 6 ++ .../src/proposals/demoIssuers.js | 12 ++++ .../vaultFactory/liquidateIncrementally.js | 20 ++++++ .../src/vaultFactory/vaultManager.js | 14 +++- packages/solo/src/add-chain.js | 5 ++ packages/solo/src/chain-cosmos-sdk.js | 5 ++ packages/solo/src/main.js | 5 ++ packages/solo/src/pipe-entrypoint.js | 3 + packages/solo/src/start.js | 5 ++ packages/solo/src/vat-http.js | 5 ++ packages/solo/src/web.js | 5 ++ packages/telemetry/src/frcat-entrypoint.js | 6 +- .../telemetry/src/ingest-slog-entrypoint.js | 10 +++ packages/vats/src/core/chain-behaviors.js | 6 ++ packages/vats/src/ibc.js | 13 ++++ packages/vats/src/vat-bank.js | 4 +- .../src/admin-websocket-connector.js | 11 ++- packages/xsnap/src/replay.js | 72 ++++++++++++++++--- packages/xsnap/src/xsnap.js | 28 ++++++++ .../zoe/src/contractSupport/priceAuthority.js | 9 +++ .../contracts/callSpread/pricedCallSpread.js | 3 +- packages/zoe/src/contracts/oracle.js | 14 ++++ 40 files changed, 455 insertions(+), 36 deletions(-) diff --git a/packages/SwingSet/src/controller/initializeSwingset.js b/packages/SwingSet/src/controller/initializeSwingset.js index 40bdf3c3b44d..e3aea5a599f6 100644 --- a/packages/SwingSet/src/controller/initializeSwingset.js +++ b/packages/SwingSet/src/controller/initializeSwingset.js @@ -25,8 +25,10 @@ const { keys, values, fromEntries } = Object; * @returns {Promise>} * @template V */ -const allValues = async obj => - fromEntries(zip(keys(obj), await Promise.all(values(obj)))); +const allValues = async obj => { + const vals = await Promise.all(values(obj)); + return fromEntries(zip(keys(obj), vals)); +}; const bundleRelative = rel => bundleSource(new URL(rel, import.meta.url).pathname); @@ -173,7 +175,14 @@ export function loadBasedir(basedir, options = {}) { */ async function resolveSpecFromConfig(referrer, specPath) { try { - return new URL(await resolveModuleSpecifier(specPath, referrer)).pathname; + // This nested await is safe because "terminal-control-flow". + // + // Some code does execute after this one, synchronously or asynchronously. + // That would normally be unsafe, but all that code is clearly not stateful. + // Its correctness is independent on timing. + // eslint-disable-next-line @jessie.js/no-nested-await + const base = await resolveModuleSpecifier(specPath, referrer); + return new URL(base).pathname; } catch (e) { if (e.code !== 'MODULE_NOT_FOUND' && e.code !== 'ERR_MODULE_NOT_FOUND') { throw e; @@ -236,7 +245,21 @@ export async function loadSwingsetConfigFile(configPath) { configPath, `file:///${process.cwd()}/`, ).toString(); + // This nested await is safe because "not-my-problem". + // + // It does occur nested with a control flow branch. But it is at top + // level within that branch, followed by another await also at + // top level within that branch. Therefore, this await introduces + // no unsafety beyond that potentially caused by the next await. + // eslint-disable-next-line @jessie.js/no-nested-await await normalizeConfigDescriptor(config.vats, referrer, true); + // This nested await is safe because "synchonous-throw-impossible". + // + // normalizeConfigDescriptor is an async function. The only argument + // expression that might in theory throw is `config.bundles`, which we + // are confident could not actually throw. Even if it could, we'd still + // be safe because "terminal-control-flow". The catch body is not stateful. + // eslint-disable-next-line @jessie.js/no-nested-await await normalizeConfigDescriptor(config.bundles, referrer, false); // await normalizeConfigDescriptor(config.devices, referrer, true); // TODO: represent devices assert(config.bootstrap, X`no designated bootstrap vat in ${configPath}`); diff --git a/packages/SwingSet/src/devices/plugin/device-plugin.js b/packages/SwingSet/src/devices/plugin/device-plugin.js index f6030607cebe..4d7a4285b70f 100644 --- a/packages/SwingSet/src/devices/plugin/device-plugin.js +++ b/packages/SwingSet/src/devices/plugin/device-plugin.js @@ -55,6 +55,15 @@ export function buildRootDeviceNode(tools) { */ async function createConnection(mod, index, epoch) { try { + // This nested await is safe because "terminal-throw-control-flow". + // + // This statement appears at the top level of a top level try block, + // and so is executed + // unconditionally. If it throws, we do nothing significantly stateful + // before exiting. (We do not consider `console.log` to be stateful + // for these purposes.) Otherwise, it will always cause a turn boundary + // before control flow continues. + // eslint-disable-next-line @jessie.js/no-nested-await const modNS = await endowments.import(mod); const receiver = obj => { // console.info('receiver', index, obj); diff --git a/packages/SwingSet/src/kernel/vat-warehouse.js b/packages/SwingSet/src/kernel/vat-warehouse.js index cc7e4e207737..5ed05a101ba1 100644 --- a/packages/SwingSet/src/kernel/vat-warehouse.js +++ b/packages/SwingSet/src/kernel/vat-warehouse.js @@ -168,14 +168,26 @@ export function makeVatWarehouse(kernelKeeper, vatLoader, policyOptions) { // instantiate all static vats for (const [name, vatID] of kernelKeeper.getStaticVats()) { logStartup(`provideVatKeeper for vat ${name} as vat ${vatID}`); - // eslint-disable-next-line no-await-in-loop + // Nested await safe because "terminal-combined-control-flow". + // + // `ensureVatOnline` is an async function, so this turn boundary always + // happens at the end of each iteration. However, the control-flow is + // still unbalanced. If the loop iterates zero times, we proceed to + // the next code synchronously. Else we proceed asynchronously. + // However, the next loop is terminal, and each iteration of that loop + // resemble each iteration of this one. Considered together, in all + // cases the first iteration of one of these loops will happen first, + // if there are any. And if there are more iterations from either loop, + // a turn boundary separated each iteration from the next. + // eslint-disable-next-line no-await-in-loop, @jessie.js/no-nested-await await ensureVatOnline(vatID, recreate); } // instantiate all dynamic vats for (const vatID of kernelKeeper.getDynamicVats()) { logStartup(`provideVatKeeper for dynamic vat ${vatID}`); - // eslint-disable-next-line no-await-in-loop + // Nested await safe because "terminal-control-flow" + // eslint-disable-next-line no-await-in-loop, @jessie.js/no-nested-await await ensureVatOnline(vatID, recreate); } } @@ -379,6 +391,12 @@ export function makeVatWarehouse(kernelKeeper, vatLoader, policyOptions) { // worker may or may not be online if (ephemeral.vats.has(vatID)) { try { + // This nested await is safe because "terminal-control-flow". + // + // For these purposes, we do not consider `console.debug` to be + // significantly stateful. That said, no stateful code in this + // function executes after this line. + // eslint-disable-next-line @jessie.js/no-nested-await await evict(vatID); } catch (err) { console.debug('vat termination was already reported; ignoring:', err); diff --git a/packages/SwingSet/src/vats/network/network.js b/packages/SwingSet/src/vats/network/network.js index 8ed78bae790b..d82840e5dee5 100644 --- a/packages/SwingSet/src/vats/network/network.js +++ b/packages/SwingSet/src/vats/network/network.js @@ -239,7 +239,28 @@ export function makeNetworkProtocol(protocolHandler) { // Check if we are underspecified (ends in slash) if (localAddr.endsWith(ENDPOINT_SEPARATOR)) { for (;;) { - // eslint-disable-next-line no-await-in-loop + // This nested await is safe because "terminal-throw-control-flow" + + // "synchronous-throw-impossible" + "not-my-problem". + // + // We assume `E` cannot synchronously throw, and all the argument + // expressions are simple variables. If the await expression throws + // we exit the function immediately. Thus, there is a reliable + // turn boundary before each iteration of the loop. There remains + // the issue of zero iterations of the loop, which in this case is + // impossible because of the absence of a condition in the head. + // + // Finally, there remains the issue of the unbalances `if`. The + // then case will always take at least one turn boundary before + // proceeding. But the else case will proceed synchronously to + // the subsequent code. Fortunately, the next interesting statement + // is a top level await, so this await does not introduce any + // unsafety beyond that caused by the other await. + // Hence "not-my-problem". That await is manifestly safe because it + // is at top level of the function and always forces a turn boundary. + // The awaited expression is another `E` message send expression in + // which all the argument expressions are only variables, so it will + // not cause anything stateful prior to that turn boundary. + // eslint-disable-next-line no-await-in-loop, @jessie.js/no-nested-await const portID = await E(protocolHandler).generatePortID( localAddr, protocolHandler, @@ -360,6 +381,8 @@ export function makeNetworkProtocol(protocolHandler) { }, }); + // The safety of the `await` in the loop above depends on this `await` + // remaining in place. await E(protocolHandler).onBind(port, localAddr, protocolHandler); boundPorts.init(localAddr, port); currentConnections.init(port, new Set()); @@ -381,6 +404,10 @@ export function makeNetworkProtocol(protocolHandler) { const [port, listener] = listening.get(listenPrefix); let localAddr; try { + // 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. + // // See if our protocol is willing to receive this connection. // eslint-disable-next-line no-await-in-loop const localInstance = await E(protocolHandler) @@ -462,6 +489,10 @@ export function makeNetworkProtocol(protocolHandler) { let lastFailure; try { + // 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. + // // Attempt the loopback connection. const attempt = await protocolImpl.inbound( remoteAddr, diff --git a/packages/agoric-cli/src/cosmos.js b/packages/agoric-cli/src/cosmos.js index 8751e2b69a97..23f0b18ed05b 100644 --- a/packages/agoric-cli/src/cosmos.js +++ b/packages/agoric-cli/src/cosmos.js @@ -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; diff --git a/packages/agoric-cli/src/deploy.js b/packages/agoric-cli/src/deploy.js index 25bce9ee9f12..d9a26a4fa8b5 100644 --- a/packages/agoric-cli/src/deploy.js +++ b/packages/agoric-cli/src/deploy.js @@ -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 */ diff --git a/packages/agoric-cli/src/init.js b/packages/agoric-cli/src/init.js index d61faae57e59..d3910b2e13ab 100644 --- a/packages/agoric-cli/src/init.js +++ b/packages/agoric-cli/src/init.js @@ -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'; diff --git a/packages/agoric-cli/src/install.js b/packages/agoric-cli/src/install.js index 7d0b9b80d112..d09ed9df4b43 100644 --- a/packages/agoric-cli/src/install.js +++ b/packages/agoric-cli/src/install.js @@ -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'; diff --git a/packages/agoric-cli/src/open.js b/packages/agoric-cli/src/open.js index d11ce74d811c..24b139317fce 100644 --- a/packages/agoric-cli/src/open.js +++ b/packages/agoric-cli/src/open.js @@ -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)); } } diff --git a/packages/agoric-cli/src/publish.js b/packages/agoric-cli/src/publish.js index d07767348ef9..8243a599a0ea 100644 --- a/packages/agoric-cli/src/publish.js +++ b/packages/agoric-cli/src/publish.js @@ -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 /// diff --git a/packages/agoric-cli/src/set-defaults.js b/packages/agoric-cli/src/set-defaults.js index ef6fb626bb76..b0f0bb4ef006 100644 --- a/packages/agoric-cli/src/set-defaults.js +++ b/packages/agoric-cli/src/set-defaults.js @@ -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 { diff --git a/packages/agoric-cli/src/start.js b/packages/agoric-cli/src/start.js index 9c80b48fb25e..b34cb1d31b65 100644 --- a/packages/agoric-cli/src/start.js +++ b/packages/agoric-cli/src/start.js @@ -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'; diff --git a/packages/cache/src/store.js b/packages/cache/src/store.js index 4ef5e0153a06..08d39ab44103 100644 --- a/packages/cache/src/store.js +++ b/packages/cache/src/store.js @@ -78,6 +78,14 @@ const applyCacheTransaction = async ( // Loop until our updated state is fresh wrt our current state. basisState = stateStore.get(keyStr); while (updatedState && updatedState.generation <= basisState.generation) { + // 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. + // + // If we knew that this loop is guaranteed to execute at least once, + // then this await would be safe because "terminal-throw-control-flow". + // But if it might execute zero times, then the stateful code after + // this loop might execute synchronously or asynchronously. // eslint-disable-next-line no-await-in-loop updatedState = await getUpdatedState(basisState); // AWAIT INTERLEAVING diff --git a/packages/cosmic-swingset/src/chain-main.js b/packages/cosmic-swingset/src/chain-main.js index 43ae8df392f6..314df62b4906 100644 --- a/packages/cosmic-swingset/src/chain-main.js +++ b/packages/cosmic-swingset/src/chain-main.js @@ -156,6 +156,11 @@ export default async function main(progname, args, { env, homedir, agcc }) { const port = lastPort; portHandlers[port] = async (...phArgs) => { try { + // Nested await is safe because "terminal-control-flow" + // + // For these purposes, `console.error` is not considered significantly + // stateful. + // eslint-disable-next-line @jessie.js/no-nested-await return await portHandler(...phArgs); } catch (e) { console.error('portHandler threw', e); @@ -303,13 +308,12 @@ export default async function main(progname, args, { env, homedir, agcc }) { ROLE: 'chain', bootMsg, }; - const vatconfig = new URL( - await importMetaResolve( - env.CHAIN_BOOTSTRAP_VAT_CONFIG || - argv.bootMsg.params.bootstrap_vat_config, - import.meta.url, - ), - ).pathname; + const url = await importMetaResolve( + env.CHAIN_BOOTSTRAP_VAT_CONFIG || + argv.bootMsg.params.bootstrap_vat_config, + import.meta.url, + ); + const vatconfig = new URL(url).pathname; const { metricsProvider } = getTelemetryProviders({ console, @@ -458,6 +462,13 @@ export default async function main(progname, args, { env, homedir, agcc }) { // Ensure that initialization has completed. if (!blockingSend) { + // 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. + // + // Likely `blockingSend` is not sensitive to whether it is called + // in the same or a subsequent turn. If that were verified, then + // this await would be safe because "terminal-control-flow". blockingSend = await launchAndInitializeSwingSet(action); } diff --git a/packages/cosmic-swingset/src/launch-chain.js b/packages/cosmic-swingset/src/launch-chain.js index 6d1f79857182..7a3511f5ad2b 100644 --- a/packages/cosmic-swingset/src/launch-chain.js +++ b/packages/cosmic-swingset/src/launch-chain.js @@ -97,6 +97,14 @@ async function buildSwingset( // Find the entrypoints for all the core proposals. if (config.coreProposals) { + // 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. + // + // `initializeSwingset` is stateful and is called synchronously + // or asynchronously, depending on which branch of the conditional + // is taken. If it were verified to be insensitive to this, + // then this await would be safe because "terminal-control-flow" const { bundles, code } = await extractCoreProposalBundles( config.coreProposals, vatconfig, @@ -591,6 +599,11 @@ export async function launch({ blockHeight, runNum, }); + // This nested await is safe because "terminal-control-flow". + // + // It occurs at the top level of a case of an unbalanced, + // but top level terminal switch. Nothing happens after the switch. + // eslint-disable-next-line @jessie.js/no-nested-await await processAction(action.type, bootstrapBlock); controller.writeSlogObject({ type: 'cosmic-swingset-run-finish', @@ -622,6 +635,9 @@ export async function launch({ // Save the kernel's computed state just before the chain commits. const start2 = Date.now(); + // This nested await is safe because "terminal-control-flow". + // + // eslint-disable-next-line @jessie.js/no-nested-await await saveOutsideState(computedHeight, blockTime); const saveTime = Date.now() - start2; controller.writeSlogObject({ @@ -694,6 +710,14 @@ export async function launch({ provideInstallationPublisher(); + // This nested await is safe because "terminal-control-flow". + // + // It occurs at the top level of a branch of an unbalanced + // but terminal if, that is top level and terminal within a case + // of a top level unbalanced but terminal and top level switch. + // Thus, nothing happens after completion of the immeiately + // enclosing if. + // eslint-disable-next-line @jessie.js/no-nested-await await processAction(action.type, async () => runKernel( blockHeight, @@ -705,6 +729,9 @@ export async function launch({ // We write out our on-chain state as a number of chainSends. const start = Date.now(); + // This nested await is safe because "terminal-control-flow". + // + // eslint-disable-next-line @jessie.js/no-nested-await await saveChainState(); chainTime = Date.now() - start; diff --git a/packages/cosmic-swingset/src/sim-chain.js b/packages/cosmic-swingset/src/sim-chain.js index 938a2341a05b..67b1ee5bf9ae 100644 --- a/packages/cosmic-swingset/src/sim-chain.js +++ b/packages/cosmic-swingset/src/sim-chain.js @@ -39,6 +39,12 @@ async function makeMapStorage(file) { let obj = {}; try { + // This await is safe because "terminal-throw-control-flow". + // + // It occurs at the top level of a top level try block, and so executes + // unconditionally. If it throws, it exits the function without further + // stateful execution. + // eslint-disable-next-line @jessie.js/no-nested-await content = await fs.promises.readFile(file); obj = JSON.parse(content); } catch (e) { @@ -69,13 +75,12 @@ export async function connectToFakeChain(basedir, GCI, delay, inbound) { }, }; - const vatconfig = new URL( - await importMetaResolve( - process.env.CHAIN_BOOTSTRAP_VAT_CONFIG || - argv.bootMsg.params.bootstrap_vat_config, - import.meta.url, - ), - ).pathname; + const url = await importMetaResolve( + process.env.CHAIN_BOOTSTRAP_VAT_CONFIG || + argv.bootMsg.params.bootstrap_vat_config, + import.meta.url, + ); + const vatconfig = new URL(url).pathname; const stateDBdir = path.join(basedir, `fake-chain-${GCI}-state`); function replayChainSends() { assert.fail(X`Replay not implemented`); @@ -197,6 +202,8 @@ export async function connectToFakeChain(basedir, GCI, delay, inbound) { // Only actually simulate a block if we're not in bootstrap. if (blockHeight && !delay) { clearTimeout(nextBlockTimeout); + // This await is safe because "terminal-control-flow" + // eslint-disable-next-line @jessie.js/no-nested-await await simulateBlock(); } } diff --git a/packages/deploy-script-support/src/installInPieces.js b/packages/deploy-script-support/src/installInPieces.js index 7d62e9f98e2c..9d5896c91298 100644 --- a/packages/deploy-script-support/src/installInPieces.js +++ b/packages/deploy-script-support/src/installInPieces.js @@ -38,6 +38,21 @@ export const installInPieces = async ( log( `waiting for ${inFlightAdditions.length} (~${approxBytesInFlight}B) additions...`, ); + // This await is safe both because "initial-control-flow" + // and "terminal-control-flow" + // + // "initial-control-flow" is the time reversal of + // "terminal-control-flow". There is always a turn boundary before each + // iteration of a for-await-of loop. This await occurs in one branch + // of an unbalanced if within the loop body. But it happens before + // anything stateful executes within this iteration, so all subsequent + // code in the loop body only executes after a turn boundary. + // It is also safe because "terminal-control-flow" because non of the + // code in the remainder of the loop body is potentially stateful. + // The for-await-of loop protects all code after the loop anyway, + // because even in the zero iteration case it will always take a + // turn boundary. + // eslint-disable-next-line @jessie.js/no-nested-await await Promise.all(inFlightAdditions); approxBytesInFlight = 0; inFlightAdditions = []; diff --git a/packages/deploy-script-support/src/startInstance.js b/packages/deploy-script-support/src/startInstance.js index 4011eaafcfb1..36c4427d28e3 100644 --- a/packages/deploy-script-support/src/startInstance.js +++ b/packages/deploy-script-support/src/startInstance.js @@ -99,6 +99,12 @@ export const makeStartInstance = ( `creatorInvitation must be defined to be deposited`, ); console.log(`-- Adding Invitation for: ${instancePetname}`); + // This nested await is safe bacause "terminal-control-flow". + // + // Nothing potentially stateful happens in this function after the + // await. For these purposes, `console.log` is not considered + // significantly stateful. + // eslint-disable-next-line @jessie.js/no-nested-await const invitationAmount = await E(zoeInvitationPurse).deposit( creatorInvitation, ); diff --git a/packages/deploy-script-support/src/writeCoreProposal.js b/packages/deploy-script-support/src/writeCoreProposal.js index 0c535ea37eb6..e9860abffe7f 100644 --- a/packages/deploy-script-support/src/writeCoreProposal.js +++ b/packages/deploy-script-support/src/writeCoreProposal.js @@ -1,3 +1,9 @@ +// IIUC the purpose of the deploy-script-support 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 import fs from 'fs'; import { E } from '@endo/far'; diff --git a/packages/inter-protocol/src/proposals/demoIssuers.js b/packages/inter-protocol/src/proposals/demoIssuers.js index e8849f670890..54fd8e8d62c2 100644 --- a/packages/inter-protocol/src/proposals/demoIssuers.js +++ b/packages/inter-protocol/src/proposals/demoIssuers.js @@ -288,6 +288,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, @@ -306,6 +311,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 = [ @@ -493,6 +500,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, @@ -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 => { diff --git a/packages/inter-protocol/src/vaultFactory/liquidateIncrementally.js b/packages/inter-protocol/src/vaultFactory/liquidateIncrementally.js index bad67cdbebe9..86c6b4fd5493 100644 --- a/packages/inter-protocol/src/vaultFactory/liquidateIncrementally.js +++ b/packages/inter-protocol/src/vaultFactory/liquidateIncrementally.js @@ -197,9 +197,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); @@ -224,6 +237,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, @@ -301,6 +319,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'); diff --git a/packages/inter-protocol/src/vaultFactory/vaultManager.js b/packages/inter-protocol/src/vaultFactory/vaultManager.js index d49fb13d63dc..70df61c382c6 100644 --- a/packages/inter-protocol/src/vaultFactory/vaultManager.js +++ b/packages/inter-protocol/src/vaultFactory/vaultManager.js @@ -466,7 +466,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 @@ -487,7 +496,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]); } diff --git a/packages/solo/src/add-chain.js b/packages/solo/src/add-chain.js index 9683fb34fa38..1c7a50c0ec8f 100644 --- a/packages/solo/src/add-chain.js +++ b/packages/solo/src/add-chain.js @@ -1,3 +1,8 @@ +// IIUC the purpose of the solo 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 */ import fetch from 'node-fetch'; import crypto from 'crypto'; diff --git a/packages/solo/src/chain-cosmos-sdk.js b/packages/solo/src/chain-cosmos-sdk.js index 665b6a329402..bce07ec4e6c6 100644 --- a/packages/solo/src/chain-cosmos-sdk.js +++ b/packages/solo/src/chain-cosmos-sdk.js @@ -1,3 +1,8 @@ +// IIUC the purpose of the solo 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 setTimeout Buffer */ import path from 'path'; import fs from 'fs'; diff --git a/packages/solo/src/main.js b/packages/solo/src/main.js index f71f30e2006f..db0a5a50d0ff 100644 --- a/packages/solo/src/main.js +++ b/packages/solo/src/main.js @@ -1,3 +1,8 @@ +// IIUC the purpose of the solo 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 fs from 'fs'; import path from 'path'; import parseArgs from 'minimist'; diff --git a/packages/solo/src/pipe-entrypoint.js b/packages/solo/src/pipe-entrypoint.js index 97082771ad98..edf254ccffb0 100644 --- a/packages/solo/src/pipe-entrypoint.js +++ b/packages/solo/src/pipe-entrypoint.js @@ -37,6 +37,9 @@ const main = async () => { switch (method) { case 'connectToFakeChain': { const [basedir, GCI, delay] = margs; + // This nested await is safe because "terminal-control-flow". + // + // eslint-disable-next-line @jessie.js/no-nested-await deliverator = await connectToFakeChain( basedir, GCI, diff --git a/packages/solo/src/start.js b/packages/solo/src/start.js index df9b7b52fffc..7372a8249722 100644 --- a/packages/solo/src/start.js +++ b/packages/solo/src/start.js @@ -1,3 +1,8 @@ +// IIUC the purpose of the solo 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 /* global process setTimeout */ import fs from 'fs'; diff --git a/packages/solo/src/vat-http.js b/packages/solo/src/vat-http.js index c0419af8625b..2fed7fa44999 100644 --- a/packages/solo/src/vat-http.js +++ b/packages/solo/src/vat-http.js @@ -1,3 +1,8 @@ +// IIUC the purpose of the solo 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 { makeNotifierKit } from '@agoric/notifier'; import { makeCache } from '@agoric/cache'; import { E } from '@endo/eventual-send'; diff --git a/packages/solo/src/web.js b/packages/solo/src/web.js index fca980eaa1e8..08916027e13a 100644 --- a/packages/solo/src/web.js +++ b/packages/solo/src/web.js @@ -1,3 +1,8 @@ +// IIUC the purpose of the solo 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 setTimeout clearTimeout setInterval clearInterval process */ // Start a network service import path from 'path'; diff --git a/packages/telemetry/src/frcat-entrypoint.js b/packages/telemetry/src/frcat-entrypoint.js index 11f57448934f..ac59489a568b 100755 --- a/packages/telemetry/src/frcat-entrypoint.js +++ b/packages/telemetry/src/frcat-entrypoint.js @@ -14,6 +14,8 @@ const main = async () => { } for await (const file of files) { + // This nested await is safe because "top-level-of-for-await". + // eslint-disable-next-line @jessie.js/no-nested-await const { readCircBuf } = await makeMemoryMappedCircularBuffer({ circularBufferFilename: file, circularBufferSize: null, @@ -50,7 +52,9 @@ const main = async () => { } // If the buffer is full, wait for stdout to drain. - // eslint-disable-next-line no-await-in-loop + // + // This nested await is safe because "synchronous-throw-impossible". + // eslint-disable-next-line @jessie.js/no-nested-await, no-await-in-loop await new Promise(resolve => process.stdout.once('drain', resolve)); } } diff --git a/packages/telemetry/src/ingest-slog-entrypoint.js b/packages/telemetry/src/ingest-slog-entrypoint.js index 449569e7e26e..190a413d1d9a 100755 --- a/packages/telemetry/src/ingest-slog-entrypoint.js +++ b/packages/telemetry/src/ingest-slog-entrypoint.js @@ -94,6 +94,14 @@ async function run() { lineCount % LINE_COUNT_TO_FLUSH === 0 ) { lastTime = now; + // 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. + // + // It is nested in a conditional with no + // balancing await on the other branch. Likely this is low risk + // because it is only in telemetry. The integrity of critical computation + // should not be endangered by a bug in telemetry in any case. await stats(update); } @@ -108,6 +116,8 @@ async function run() { const delayMS = PROCESSING_PERIOD - (now - startOfLastPeriod); maybeWait = new Promise(resolve => setTimeout(resolve, delayMS)); } + // This nested await is safe because "top-of-for-await". + // eslint-disable-next-line @jessie.js/no-nested-await await maybeWait; now = Date.now(); diff --git a/packages/vats/src/core/chain-behaviors.js b/packages/vats/src/core/chain-behaviors.js index 9ec7b027f5d4..be713e02319c 100644 --- a/packages/vats/src/core/chain-behaviors.js +++ b/packages/vats/src/core/chain-behaviors.js @@ -385,6 +385,12 @@ export const registerNetworkProtocols = async (vats, dibcBridgeManager) => { }); }, }); + // This nested await is safe because "terminal-control-flow". + // + // It occurs at the top level of one branch of an unbalanced non-top-level + // if. However, immediately after the if is another await expression + // that has no effects prior to the turn boundary. + // eslint-disable-next-line @jessie.js/no-nested-await const ibcHandler = await E(vats.ibc).createInstance(callbacks); dibcBridgeManager.register(BRIDGE_ID.DIBC, ibcHandler); ps.push( diff --git a/packages/vats/src/ibc.js b/packages/vats/src/ibc.js index f8755f6db787..b25710fe958b 100644 --- a/packages/vats/src/ibc.js +++ b/packages/vats/src/ibc.js @@ -381,6 +381,13 @@ export function makeIBCProtocolHandler(E, rawCallIBCDevice) { const attemptP = E(protocolImpl).inbound(localAddr, remoteAddr); // Tell what version string we negotiated. + // + // This nested await is safe because "terminal-control-flow". + // + // It occurs at the top level of one branch of an unbalanced, but + // top level and terminal switch statement. Nothing happens after + // the switch. + // eslint-disable-next-line @jessie.js/no-nested-await const attemptedLocal = await E(attemptP).getLocalAddress(); const match = attemptedLocal.match( // Match: ... /ORDER/VERSION ... @@ -493,6 +500,9 @@ export function makeIBCProtocolHandler(E, rawCallIBCDevice) { rPortID, order, ); + // This nested await is safe because "terminal-control-flow". + // + // eslint-disable-next-line @jessie.js/no-nested-await const localAddr = await E(attemptP).getLocalAddress(); E(attemptP).accept({ localAddress: `${localAddr}/ibc-channel/${channelID}`, @@ -512,6 +522,9 @@ export function makeIBCProtocolHandler(E, rawCallIBCDevice) { const connP = channelKeyToConnP.get(channelKey); const data = base64ToBytes(data64); + // This nested await is safe because "terminal-control-flow". + // + // eslint-disable-next-line @jessie.js/no-nested-await await E(connP) .send(data) .then(ack => { diff --git a/packages/vats/src/vat-bank.js b/packages/vats/src/vat-bank.js index 11b395ed7083..0a3c82778e18 100644 --- a/packages/vats/src/vat-bank.js +++ b/packages/vats/src/vat-bank.js @@ -43,7 +43,9 @@ const makePurseController = ( let updateRecord = await balanceNotifier.getUpdateSince(); while (updateRecord.updateCount) { yield updateRecord.value; - // eslint-disable-next-line no-await-in-loop + // This nested await is safe because "terminal-control-flow". + // + // eslint-disable-next-line no-await-in-loop, @jessie.js/no-nested-await updateRecord = await balanceNotifier.getUpdateSince( updateRecord.updateCount, ); diff --git a/packages/web-components/src/admin-websocket-connector.js b/packages/web-components/src/admin-websocket-connector.js index 68f6f6eb61a3..3c4909d3722d 100644 --- a/packages/web-components/src/admin-websocket-connector.js +++ b/packages/web-components/src/admin-websocket-connector.js @@ -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); } diff --git a/packages/xsnap/src/replay.js b/packages/xsnap/src/replay.js index 32b476914607..d94ec9cd90e9 100644 --- a/packages/xsnap/src/replay.js +++ b/packages/xsnap/src/replay.js @@ -210,7 +210,11 @@ export async function replayXSnap( */ async function runSteps(rd, steps) { const folder = rd.path; - for (const step of steps) { + // Changed from `for of` to `for await of` to make awaits within + // loop body safer. A `for await of` will correctly process a synchronous + // iterator, but it still unconditionally separates the iterations + // with turn boundaries. + for await (const step of steps) { const parts = step.match(/(\d+)-([a-zA-Z]+)\.(dat|json)$/); if (!parts) { throw Error(`expected 0001-abc.dat; got: ${step}`); @@ -219,45 +223,86 @@ export async function replayXSnap( const seq = parseInt(digits, 10); console.log(folder, seq, kind); if (running && !['command', 'reply'].includes(kind)) { - // eslint-disable-next-line no-await-in-loop + // This nested await is safe because "all-branches-balanced". + // + // This nested synchronous-throw-impossible await at the top level of + // the then branch is safe because it is balanced by a + // synchronous-throw-impossible await at the top level of the else + // branch. In fact the await in the else branch was introduced for + // that balance, in order to make this one safe. + // eslint-disable-next-line @jessie.js/no-nested-await await running; running = undefined; + } else { + // This nested await is safe because "all-branches-balanced". + // + // This nested synchronous-throw-impossible await at the top level of + // the else branch is safe because it is balanced by a + // synchronous-throw-impossible await at the top level of the then + // branch. In fact it was introduced for that balance, in order to + // make the `await` in the then branch safe. + // eslint-disable-next-line @jessie.js/no-nested-await + await null; } const file = rd.file(step); switch (kind) { - case 'isReady': - // eslint-disable-next-line no-await-in-loop + case 'isReady': { + // This nested await is safe because "terminal-control-flow". + // + // The awaits within the switch are unbalanced. The occur on some + // branches and not others. This would be ok if the switch + // were terminal control flow, i.e., if no stateful synchronous code + // happens after the switch. This is now true, because of the + // extra `await null` turn boundary before the remaining stateful + // code. Now that code will always happen after a turn boundary. + // + // eslint-disable-next-line @jessie.js/no-nested-await await it.isReady(); break; - case 'evaluate': + } + case 'evaluate': { running = it.evaluate(file.getText()); break; - case 'issueCommand': + } + case 'issueCommand': { running = it.issueCommand(file.getData()); break; - case 'command': + } + case 'command': { // ignore; we already know how to reply break; - case 'reply': + } + case 'reply': { replies.put(file.getData()); break; - case 'snapshot': + } + case 'snapshot': { if (folders.length > 1 && folder !== folders.slice(-1)[0]) { console.log(folder, step, 'ignoring remaining steps from', folder); return; } else { try { - // eslint-disable-next-line no-await-in-loop + // This nested await is safe because "terminal-control-flow", + // given that we do not consider the timing of a `console.log` + // to be a practical hazard. + // eslint-disable-next-line @jessie.js/no-nested-await await it.snapshot(file.getText()); } catch (err) { console.warn(err, 'while taking snapshot:', err); } } break; - default: + } + default: { console.log(`bad kind: ${kind}`); throw RangeError(`bad kind: ${kind}`); + } } + // This nested await is safe because "top-level-of-for-await". + // We introduced it to make the unbalanced awaits in the above switch + // statement safe. + // eslint-disable-next-line @jessie.js/no-nested-await + await null; done.push([folder, seq, kind]); } } @@ -270,6 +315,11 @@ export async function replayXSnap( const storedOpts = JSON.parse(rd.file(optionsFn).getText()); console.log(folder, optionsFn, 'already spawned; ignoring:', storedOpts); } + // This nested await is safe because "top-level-of-for-await". + // It occurs at the top level of a for-await body, and so everything + // afterwards happens after a turn boundary, whether the awaited expression + // throws or not. + // eslint-disable-next-line @jessie.js/no-nested-await await runSteps(rd, steps); } diff --git a/packages/xsnap/src/xsnap.js b/packages/xsnap/src/xsnap.js index f0396b4da002..e497fe271a83 100644 --- a/packages/xsnap/src/xsnap.js +++ b/packages/xsnap/src/xsnap.js @@ -168,6 +168,12 @@ export function xsnap(options) { */ async function runToIdle() { for (;;) { + // This nested await is safe because "terminal-throw-control-flow". + // + // If the awaited expression throws, no further code in this function + // runs. If it does not throw, then there is always a turn boundary + // before we proceed with each iteration. + // eslint-disable-next-line @jessie.js/no-nested-await const iteration = await messagesFromXsnap.next(undefined); if (iteration.done) { xsnapProcess.kill(); @@ -205,7 +211,29 @@ export function xsnap(options) { )}`, ); } else if (message[0] === QUERY) { + // This nested await is safe because "not-my-problem". + // + // This nested await introduces no unsafety beyond that introduced + // by the next await, since it is earlier within the same control + // flow branch, and is top level within that branch. + // eslint-disable-next-line @jessie.js/no-nested-await const commandResult = await handleCommand(message.subarray(1)); + // This nested await is ALMOST safe because "terminal-control-flow". + // This one appears nested in an unbalanced conditional, so we need + // to consider three cases: + // * This branch is taken and the awaited expression throws. + // In that case, the function exits would executing and further + // code. + // * This branch is taken and does not throw. In that case, + // it causes a turn boundary before the next iteration. + // * Another branch of the conditional is taken, skipping + // this statement and proceeding syncronously to the next + // iteration. The await at the top of the loop ensures that the + // remainder of the loop only happens after a turn boundary. + // The only remaining unsafety is that the awaited expression of + // that initial await may therefore be reached synchronously or + // asynchronously. + // eslint-disable-next-line @jessie.js/no-nested-await await messagesToXsnap.next([QUERY_RESPONSE_BUF, commandResult]); } else { // unrecognized responses also kill the process diff --git a/packages/zoe/src/contractSupport/priceAuthority.js b/packages/zoe/src/contractSupport/priceAuthority.js index ff7614f9124d..4edb1075517f 100644 --- a/packages/zoe/src/contractSupport/priceAuthority.js +++ b/packages/zoe/src/contractSupport/priceAuthority.js @@ -327,6 +327,15 @@ export function makeOnewayPriceAuthorityKit(opts) { // We don't wait for the quote to be authenticated; resolve // immediately. quotePK.resolve(quoteP); + // This nested await is safe because + // "synchronous-throw-impossible" + "terminal-control-flow". + // + // The expression `quotePK.promise` might be a rejected promise, + // but (we assume) this expression cannot throw. Thus, if we + // reach here, we would only proceed to the stateful catch + // body after a turn boundary. The try-catch statement is + // top-level and terminal within the wake method. + // eslint-disable-next-line @jessie.js/no-nested-await await quotePK.promise; } catch (e) { quotePK.reject(e); diff --git a/packages/zoe/src/contracts/callSpread/pricedCallSpread.js b/packages/zoe/src/contracts/callSpread/pricedCallSpread.js index 538d08a3e268..528f84302110 100644 --- a/packages/zoe/src/contracts/callSpread/pricedCallSpread.js +++ b/packages/zoe/src/contracts/callSpread/pricedCallSpread.js @@ -101,8 +101,9 @@ const start = zcf => { const option = payoffHandler.makeOptionInvitation(position); const invitationIssuer = zcf.getInvitationIssuer(); const payment = harden({ Option: option }); + const Option = await E(invitationIssuer).getAmountOf(option); const spreadAmount = harden({ - Option: await E(invitationIssuer).getAmountOf(option), + Option, }); // AWAIT //// diff --git a/packages/zoe/src/contracts/oracle.js b/packages/zoe/src/contracts/oracle.js index 5273f68b567f..30c0fdc0c8d6 100644 --- a/packages/zoe/src/contracts/oracle.js +++ b/packages/zoe/src/contracts/oracle.js @@ -67,6 +67,15 @@ const start = async zcf => { try { assert(!revoked, revokedMsg); const noFee = AmountMath.makeEmpty(feeBrand); + // This nested await is safe because "synchronous-throw-impossible" + + // "terminal-control-flow". + // + // Assuming `E` does its job, the awaited expression cannot throw. + // If it returns a rejected promise, that will cause the stateful + // catch clause to execute, but only after a turn boundary. + // The await is at top level of a top level terminal try block, and so + // executes unconditionally unless something earlier threw. + // eslint-disable-next-line @jessie.js/no-nested-await const { requiredFee, reply } = await E(handler).onQuery(query, noFee); !requiredFee || AmountMath.isGTE(noFee, requiredFee) || @@ -82,6 +91,11 @@ const start = async zcf => { const doQuery = async querySeat => { try { const fee = querySeat.getAmountAllocated('Fee', feeBrand); + // This nested await is safe because "synchronous-throw-impossible" + + // "terminal-control-flow". + // + // Same reasons as above. + // eslint-disable-next-line @jessie.js/no-nested-await const { requiredFee, reply } = await E(handler).onQuery(query, fee); if (requiredFee) { feeSeat.incrementBy(