fix(vow): make watch/when more robust against loops and hangs (#9487)

closes: #9466

Expand `isRetryableReason` to take the prior return value into account
to prevent infinite loops when `watch`ing or `when`ing a plain
`Promise.reject(reasonThatIsRetryable)`. Before this, when the
watch/when would perform the retry loop, it incorrectly assumed that a
non-advancing series of retryable `shorten` attempts just needed more
retries.

Also, reject watch/when of certain promises that don't resolve promptly
(such as when the watching vat is upgraded, even if the promise's
decider has not been upgraded).

Finally, remove the deprecated `@agoric/vat-data/vow.js`, since that has
been superseded by `@agoric/vow/vat.js`.

### Security Considerations

Fixes some availability problems.

### Scaling Considerations

Less resource consumption.

### Documentation Considerations

By design, the `@agoric/vat` package assumes that all pending promises
in watch/when chains will settle promptly (such as by the end of the
crank), producing only long-lived vows or terminal results.

### Testing Considerations

New bootstrap tests added.

### Upgrade Considerations

None additional.

packages/SwingSet/tools/bootstrap-relay.js

@@ -1,6 +1,7 @@
 import { assert } from '@agoric/assert';
 import { objectMap } from '@agoric/internal';
 import { Far, E } from '@endo/far';
+import { makePromiseKit } from '@endo/promise-kit';
 import { buildManualTimer } from './manual-timer.js';
 
 const { Fail, quote: q } = assert;
@@ -37,10 +38,13 @@ export const buildRootObject = () => {
       return root;
     },
 
-    createVat: async ({ name, bundleCapName, vatParameters = {} }) => {
+    createVat: async (
+      { name, bundleCapName, vatParameters = {} },
+      options = {},
+    ) => {
       const bcap = await E(vatAdmin).getNamedBundleCap(bundleCapName);
-      const options = { vatParameters };
-      const { adminNode, root } = await E(vatAdmin).createVat(bcap, options);
+      const vatOptions = { ...options, vatParameters };
+      const { adminNode, root } = await E(vatAdmin).createVat(bcap, vatOptions);
       vatData.set(name, { adminNode, root });
       return root;
     },
@@ -77,6 +81,12 @@ export const buildRootObject = () => {
       return remotable;
     },
 
+    makePromiseKit: () => {
+      const { promise, ...resolverMethods } = makePromiseKit();
+      const resolver = Far('resolver', resolverMethods);
+      return harden({ promise, resolver });
+    },
+
     /**
      * Returns a copy of a remotable's logs.
      *

packages/boot/test/upgrading/upgrade-vats.test.js

@@ -1,6 +1,7 @@
 // @ts-check
 import { test as anyTest } from '@agoric/swingset-vat/tools/prepare-test-env-ava.js';
 
+import { makeTagged } from '@endo/marshal';
 import { BridgeId } from '@agoric/internal';
 import { buildVatController } from '@agoric/swingset-vat';
 import { makeRunUtils } from '@agoric/swingset-vat/tools/run-utils.js';
@@ -445,6 +446,8 @@ test('upgrade vat-priceAuthority', async t => {
   matchRef(t, reincarnatedRegistry.adminFacet, registry.adminFacet);
 });
 
+const dataOnly = obj => JSON.parse(JSON.stringify(obj));
+
 test('upgrade vat-vow', async t => {
   const bundles = {
     vow: {
@@ -454,25 +457,71 @@ test('upgrade vat-vow', async t => {
 
   const { EV } = await makeScenario(t, { bundles });
 
-  t.log('create initial version');
+  t.log('create initial version, metered');
+  const vatAdmin = await EV.vat('bootstrap').getVatAdmin();
+  const meter = await EV(vatAdmin).createUnlimitedMeter();
   const vowVatConfig = {
     name: 'vow',
     bundleCapName: 'vow',
   };
-  const vowRoot = await EV.vat('bootstrap').createVat(vowVatConfig);
+  const vatOptions = { managerType: 'xs-worker', meter };
+  const vowRoot = await EV.vat('bootstrap').createVat(vowVatConfig, vatOptions);
+
+  const makeFakeVowKit = async () => {
+    const internalPromiseKit = await EV.vat('bootstrap').makePromiseKit();
+    const fakeVowV0 = await EV.vat('bootstrap').makeRemotable('fakeVowV0', {
+      shorten: internalPromiseKit.promise,
+    });
+    const fakeVow = makeTagged('Vow', harden({ vowV0: fakeVowV0 }));
+    return harden({ resolver: internalPromiseKit.resolver, vow: fakeVow });
+  };
 
   t.log('test incarnation 0');
   /** @type {Record<string, [settlementValue?: unknown, isRejection?: boolean]>} */
   const localPromises = {
-    forever: [],
-    fulfilled: ['hello'],
-    rejected: ['goodbye', true],
+    promiseForever: [],
+    promiseFulfilled: ['hello'],
+    promiseRejected: ['goodbye', true],
+  };
+  const promiseKit = await EV.vat('bootstrap').makePromiseKit();
+  const fakeVowKit = await makeFakeVowKit();
+  const localVows = {
+    vowForever: [],
+    vowFulfilled: ['hello'],
+    vowRejected: ['goodbye', true],
+    vowPostUpgrade: [],
+    vowExternalPromise: [promiseKit.promise],
+    vowExternalVow: [fakeVowKit.vow],
+    vowPromiseForever: [undefined, false, true],
   };
   await EV(vowRoot).makeLocalPromiseWatchers(localPromises);
-  t.deepEqual(await EV(vowRoot).getWatcherResults(), {
-    fulfilled: { status: 'fulfilled', value: 'hello' },
-    forever: { status: 'unsettled' },
-    rejected: { status: 'rejected', reason: 'goodbye' },
+  await EV(vowRoot).makeLocalVowWatchers(localVows);
+  t.deepEqual(dataOnly(await EV(vowRoot).getWatcherResults()), {
+    promiseForever: { status: 'unsettled' },
+    promiseFulfilled: { status: 'fulfilled', value: 'hello' },
+    promiseRejected: { status: 'rejected', reason: 'goodbye' },
+    vowForever: {
+      status: 'unsettled',
+      resolver: {},
+    },
+    vowFulfilled: { status: 'fulfilled', value: 'hello' },
+    vowRejected: { status: 'rejected', reason: 'goodbye' },
+    vowPostUpgrade: {
+      status: 'unsettled',
+      resolver: {},
+    },
+    vowExternalPromise: {
+      status: 'unsettled',
+      resolver: {},
+    },
+    vowExternalVow: {
+      status: 'unsettled',
+      resolver: {},
+    },
+    vowPromiseForever: {
+      status: 'unsettled',
+      resolver: {},
+    },
   });
 
   t.log('restart');
@@ -481,16 +530,35 @@ test('upgrade vat-vow', async t => {
   t.is(incarnationNumber, 1, 'vat must be reincarnated');
 
   t.log('test incarnation 1');
-  t.deepEqual(await EV(vowRoot).getWatcherResults(), {
-    fulfilled: { status: 'fulfilled', value: 'hello' },
-    forever: {
-      status: 'rejected',
-      reason: {
-        name: 'vatUpgraded',
-        upgradeMessage: 'vat upgraded',
-        incarnationNumber: 0,
-      },
+  const localVowsUpdates = {
+    vowPostUpgrade: ['bonjour'],
+  };
+  await EV(vowRoot).resolveVowWatchers(localVowsUpdates);
+  await EV(promiseKit.resolver).resolve('ciao');
+  t.timeout(10_000);
+  const upgradeRejection = harden({
+    status: 'rejected',
+    reason: {
+      name: 'vatUpgraded',
+      upgradeMessage: 'vat upgraded',
+      incarnationNumber: 0,
+    },
+  });
+  await EV(fakeVowKit.resolver).reject(upgradeRejection.reason);
+  t.timeout(600_000); // t.timeout.clear() not yet available in our ava version
+  t.deepEqual(dataOnly(await EV(vowRoot).getWatcherResults()), {
+    promiseForever: upgradeRejection,
+    promiseFulfilled: { status: 'fulfilled', value: 'hello' },
+    promiseRejected: { status: 'rejected', reason: 'goodbye' },
+    vowForever: {
+      status: 'unsettled',
+      resolver: {},
     },
-    rejected: { status: 'rejected', reason: 'goodbye' },
+    vowFulfilled: { status: 'fulfilled', value: 'hello' },
+    vowRejected: { status: 'rejected', reason: 'goodbye' },
+    vowPostUpgrade: { status: 'fulfilled', value: 'bonjour' },
+    vowExternalPromise: { status: 'fulfilled', value: 'ciao' },
+    vowExternalVow: upgradeRejection,
+    vowPromiseForever: upgradeRejection,
   });
 });

packages/boot/test/upgrading/vat-vow.js

@@ -4,9 +4,11 @@ import { Far } from '@endo/far';
 
 export const buildRootObject = (_vatPowers, _args, baggage) => {
   const zone = makeDurableZone(baggage);
-  const { watch } = prepareVowTools(zone.subZone('VowTools'));
+  const { watch, makeVowKit } = prepareVowTools(zone.subZone('VowTools'));
 
-  /** @type {MapStore<string, { status: 'unsettled' } | PromiseSettledResult<any>>} */
+  /** @typedef {({ status: 'unsettled' } | PromiseSettledResult<any>) & { resolver?: import('@agoric/vow').VowResolver }} WatcherResult */
+
+  /** @type {MapStore<string, WatcherResult>} */
   const nameToResult = zone.mapStore('nameToResult');
 
   const makeWatcher = zone.exoClass('Watcher', undefined, name => ({ name }), {
@@ -43,5 +45,48 @@ export const buildRootObject = (_vatPowers, _args, baggage) => {
         watch(p, makeWatcher(name));
       }
     },
+    /** @param {Record<string, [settlementValue?: unknown, isRejection?: boolean, wrapInPromise?: boolean]>} localVows */
+    async makeLocalVowWatchers(localVows) {
+      for (const [name, settlement] of Object.entries(localVows)) {
+        const { vow, resolver } = makeVowKit();
+        nameToResult.init(name, harden({ status: 'unsettled', resolver }));
+        if (settlement.length) {
+          let [settlementValue, isRejection] = settlement;
+          const wrapInPromise = settlement[2];
+          if (wrapInPromise) {
+            if (isRejection) {
+              settlementValue = Promise.reject(settlementValue);
+              isRejection = false;
+            } else if (settlementValue === undefined) {
+              // Consider an undefined value as no settlement
+              settlementValue = new Promise(() => {});
+            } else {
+              settlementValue = Promise.resolve(settlementValue);
+            }
+          }
+          if (isRejection) {
+            resolver.reject(settlementValue);
+          } else {
+            resolver.resolve(settlementValue);
+          }
+        }
+        watch(vow, makeWatcher(name));
+      }
+    },
+    /** @param {Record<string, [settlementValue: unknown, isRejection?: boolean]>} localVows */
+    async resolveVowWatchers(localVows) {
+      for (const [name, settlement] of Object.entries(localVows)) {
+        const { status, resolver } = nameToResult.get(name);
+        if (status !== 'unsettled' || !resolver) {
+          throw Error(`Invalid pending vow for ${name}`);
+        }
+        const [settlementValue, isRejection] = settlement;
+        if (isRejection) {
+          resolver.reject(settlementValue);
+        } else {
+          resolver.resolve(settlementValue);
+        }
+      }
+    },
   });
 };

packages/vats/src/vat-transfer.js

@@ -3,7 +3,7 @@ import { Far } from '@endo/far';
 import { makeDurableZone } from '@agoric/zone/durable.js';
 
 import { provideLazy } from '@agoric/store';
-import { prepareVowTools } from '@agoric/vat-data/vow.js';
+import { prepareVowTools } from '@agoric/vow/vat.js';
 import { prepareBridgeTargetModule } from './bridge-target.js';
 import { prepareTransferTools } from './transfer.js';
 

packages/vow/src/tools.js

@@ -5,14 +5,16 @@ import { prepareWatch } from './watch.js';
 import { prepareWatchUtils } from './watch-utils.js';
 
 /** @import {Zone} from '@agoric/base-zone' */
+/** @import {IsRetryableReason} from './types.js' */
 
 /**
  * @param {Zone} zone
  * @param {object} [powers]
- * @param {(reason: any) => boolean} [powers.isRetryableReason]
+ * @param {IsRetryableReason} [powers.isRetryableReason]
  */
 export const prepareVowTools = (zone, powers = {}) => {
-  const { isRetryableReason = () => false } = powers;
+  const { isRetryableReason = /** @type {IsRetryableReason} */ (() => false) } =
+    powers;
   const makeVowKit = prepareVowKit(zone);
   const when = makeWhen(isRetryableReason);
   const watch = prepareWatch(zone, makeVowKit, isRetryableReason);

packages/vow/src/types.js

@@ -10,6 +10,16 @@ export {};
  * @import {prepareVowTools} from './tools.js'
  */
 
+/**
+ * @callback IsRetryableReason
+ * Return truthy if a rejection reason should result in a retry.
+ * @param {any} reason
+ * @param {any} priorRetryValue the previous value returned by this function
+ * when deciding whether to retry the same logical operation
+ * @returns {any} If falsy, the reason is not retryable. If truthy, the
+ * priorRetryValue for the next call.
+ */
+
 /**
  * @template T
  * @typedef {Promise<T | Vow<T>>} PromiseVow Return type of a function that may

packages/vow/src/vow.js

@@ -69,7 +69,9 @@ export const prepareVowKit = zone => {
     () => ({
       value: undefined,
       // The stepStatus is null if the promise step hasn't settled yet.
-      stepStatus: /** @type {null | 'fulfilled' | 'rejected'} */ (null),
+      stepStatus: /** @type {null | 'pending' | 'fulfilled' | 'rejected'} */ (
+        null
+      ),
     }),
     {
       vowV0: {
@@ -84,6 +86,7 @@ export const prepareVowKit = zone => {
             case 'rejected':
               throw value;
             case null:
+            case 'pending':
               return provideCurrentKit(this.facets.resolver).promise;
             default:
               throw new TypeError(`unexpected stepStatus ${stepStatus}`);
@@ -96,26 +99,41 @@ export const prepareVowKit = zone => {
          */
         resolve(value) {
           const { resolver } = this.facets;
-          const { promise, resolve } = getPromiseKitForResolution(resolver);
+          const { stepStatus } = this.state;
+          const { resolve } = getPromiseKitForResolution(resolver);
           if (resolve) {
             resolve(value);
-            zone.watchPromise(promise, this.facets.watchNextStep);
+          }
+          if (stepStatus === null) {
+            this.state.stepStatus = 'pending';
+            zone.watchPromise(
+              HandledPromise.resolve(value),
+              this.facets.watchNextStep,
+            );
           }
         },
         /**
          * @param {any} [reason]
          */
         reject(reason) {
           const { resolver, watchNextStep } = this.facets;
+          const { stepStatus } = this.state;
           const { reject } = getPromiseKitForResolution(resolver);
           if (reject) {
             reject(reason);
+          }
+          if (stepStatus === null) {
             watchNextStep.onRejected(reason);
           }
         },
       },
       watchNextStep: {
         onFulfilled(value) {
+          const { resolver } = this.facets;
+          const { resolve } = getPromiseKitForResolution(resolver);
+          if (resolve) {
+            resolve(value);
+          }
           this.state.stepStatus = 'fulfilled';
           this.state.value = value;
         },