VowTools retryable (#9785)

refs: #9541

## Description

Adds a `retryable` helper which retries an idempotent function on upgrade disconnection reasons.

Update the upgrade disconnection `isRetryableReason` predicate to accept `vat terminated` errors created by liveslots when trying to message an ephemeral object disconnected by an upgrade (until #9582 is addressed)

Remove the stub `retriable` helper, and use the `retryable` implementation.

### Security Considerations

To avoid infinite loops, the `vat terminated` detection logic does not allow retrying such consecutive reasons.

### Scaling Considerations

None

### Documentation Considerations

JSDoc for `retryable`

### Testing Considerations

Full unit test and liveslots upgrade test coverage

### Upgrade Considerations

The current `retriable` implementation was just a stub that was advertised as not actually working with upgrades. This introduces the actual implementation, which can be picked up by contract authors by simply updating to the next NPM release containing this change. Going forward, defined retryable flows will need to be redefined on upgrade like any other durable exo defined by a contract.

packages/boot/test/orchestration/contract-upgrade.test.ts

@@ -24,8 +24,8 @@ test.after.always(t => t.context.shutdown?.());
  * Because the send-anywhere flow requires a lookup(), it waits forever. This
  * gives us a point at which we can upgrade the vat with a working agoricNames
  * and see that the flow continues from that point. (The lookup call is not made
- * directly in a flow, but instead from a host API which uses the retriable
- * helper. As such it tests both the idempotent retry mechanism of retriable on
+ * directly in a flow, but instead from a host API which uses the retryable
+ * helper. As such it tests both the idempotent retry mechanism of retryable on
  * upgrades, and the ability to resume an async-flow for which a host vow
  * settles after an upgrade.)
  */

packages/internal/src/upgrade-api.js

@@ -45,5 +45,29 @@ harden(makeUpgradeDisconnection);
  * @returns {reason is UpgradeDisconnection}
  */
 export const isUpgradeDisconnection = reason =>
-  isFrozen(reason) && matches(reason, UpgradeDisconnectionShape);
+  reason != null && // eslint-disable-line eqeqeq
+  isFrozen(reason) &&
+  matches(reason, UpgradeDisconnectionShape);
 harden(isUpgradeDisconnection);
+
+/**
+ * Returns whether a reason is a 'vat terminated' error generated when an object
+ * is abandoned by a vat during an upgrade.
+ *
+ * Normally we do not want to rely on the `message` of an error object, but this
+ * is a pragmatic solution to the current state of vat upgrade errors. In the
+ * future we'd prefer having an error with a cause referencing a disconnection
+ * object like for promise rejections. See
+ * https://github.com/Agoric/agoric-sdk/issues/9582
+ *
+ * @param {any} reason
+ * @returns {reason is Error}
+ */
+export const isAbandonedError = reason =>
+  reason != null && // eslint-disable-line eqeqeq
+  isFrozen(reason) &&
+  matches(reason, M.error()) &&
+  // We're not using a constant here since this special value is already
+  // sprinkled throughout the SDK
+  reason.message === 'vat terminated';
+harden(isAbandonedError);

packages/internal/test/upgrade-api.test.js

@@ -1,8 +1,11 @@
 // @ts-check
 import test from 'ava';
+import { makeMarshal } from '@endo/marshal';
+
 import {
   makeUpgradeDisconnection,
   isUpgradeDisconnection,
+  isAbandonedError,
 } from '../src/upgrade-api.js';
 
 test('isUpgradeDisconnection must recognize disconnection objects', t => {
@@ -18,3 +21,15 @@ test('isUpgradeDisconnection must recognize original-format disconnection object
   });
   t.true(isUpgradeDisconnection(disconnection));
 });
+
+test('isAbandonedError recognizes marshalled vat terminated errors', t => {
+  const { fromCapData, toCapData } = makeMarshal(undefined, undefined, {
+    serializeBodyFormat: 'smallcaps',
+    errorIdNum: 70_000,
+    marshalSaveError: () => {},
+  });
+  const error = harden(Error('vat terminated'));
+  const remoteError = fromCapData(toCapData(error));
+
+  t.true(isAbandonedError(remoteError));
+});

packages/orchestration/src/exos/chain-hub.js

@@ -205,7 +205,7 @@ export const makeChainHub = (zone, agoricNames, vowTools) => {
     valueShape: M.string(),
   });
 
-  const lookupChainInfo = vowTools.retriable(
+  const lookupChainInfo = vowTools.retryable(
     zone,
     'lookupChainInfo',
     /** @param {string} chainName */
@@ -227,7 +227,7 @@ export const makeChainHub = (zone, agoricNames, vowTools) => {
     },
   );
 
-  const lookupConnectionInfo = vowTools.retriable(
+  const lookupConnectionInfo = vowTools.retryable(
     zone,
     'lookupConnectionInfo',
     /**
@@ -258,7 +258,7 @@ export const makeChainHub = (zone, agoricNames, vowTools) => {
   );
 
   /* eslint-disable no-use-before-define -- chainHub defined below */
-  const lookupChainsAndConnection = vowTools.retriable(
+  const lookupChainsAndConnection = vowTools.retryable(
     zone,
     'lookupChainsAndConnection',
     /**

packages/orchestration/src/utils/zoe-tools.js

@@ -80,7 +80,7 @@ export const makeZoeTools = (zcf, { when, allVows, allSettled, asVow }) => {
       //     const { zcfSeat: tempSeat, userSeat: userSeatP } =
       //       zcf.makeEmptySeatKit();
       //     const uSeat = await userSeatP;
-      //     // TODO how do I store in the place for this retriable?
+      //     // TODO how do I store in the place for this retryable?
       //     atomicTransfer(zcf, srcSeat, tempSeat, amounts);
       //     tempSeat.exit();
       //     return uSeat;

packages/orchestration/test/examples/snapshots/send-anywhere.test.ts.md

@@ -31,6 +31,9 @@ Generated by [AVA](https://avajs.dev).
         chainInfos: {},
         connectionInfos: {},
         denom: {},
+        lookupChainInfo_kindHandle: 'Alleged: kind',
+        lookupChainsAndConnection_kindHandle: 'Alleged: kind',
+        lookupConnectionInfo_kindHandle: 'Alleged: kind',
       },
       contract: {
         'ChainHub Admin_kindHandle': 'Alleged: kind',
@@ -74,8 +77,11 @@ Generated by [AVA](https://avajs.dev).
         },
       },
       vows: {
+        AdminRetryableFlow_kindHandle: 'Alleged: kind',
+        AdminRetryableFlow_singleton: 'Alleged: AdminRetryableFlow',
         PromiseWatcher_kindHandle: 'Alleged: kind',
         VowInternalsKit_kindHandle: 'Alleged: kind',
         WatchUtils_kindHandle: 'Alleged: kind',
+        retryableFlowForOutcomeVow: {},
       },
     }

packages/orchestration/test/examples/snapshots/staking-combinations.test.ts.md

@@ -96,6 +96,9 @@ Generated by [AVA](https://avajs.dev).
             chainName: 'agoric',
           },
         },
+        lookupChainInfo_kindHandle: 'Alleged: kind',
+        lookupChainsAndConnection_kindHandle: 'Alleged: kind',
+        lookupConnectionInfo_kindHandle: 'Alleged: kind',
       },
       contract: {
         'ChainHub Admin_kindHandle': 'Alleged: kind',
@@ -208,8 +211,11 @@ Generated by [AVA](https://avajs.dev).
         },
       },
       vows: {
+        AdminRetryableFlow_kindHandle: 'Alleged: kind',
+        AdminRetryableFlow_singleton: 'Alleged: AdminRetryableFlow',
         PromiseWatcher_kindHandle: 'Alleged: kind',
         VowInternalsKit_kindHandle: 'Alleged: kind',
         WatchUtils_kindHandle: 'Alleged: kind',
+        retryableFlowForOutcomeVow: {},
       },
     }

packages/orchestration/test/examples/snapshots/unbond.contract.test.ts.md

@@ -114,6 +114,9 @@ Generated by [AVA](https://avajs.dev).
           },
         },
         denom: {},
+        lookupChainInfo_kindHandle: 'Alleged: kind',
+        lookupChainsAndConnection_kindHandle: 'Alleged: kind',
+        lookupConnectionInfo_kindHandle: 'Alleged: kind',
       },
       contract: {
         orchestration: {
@@ -156,8 +159,11 @@ Generated by [AVA](https://avajs.dev).
         },
       },
       vows: {
+        AdminRetryableFlow_kindHandle: 'Alleged: kind',
+        AdminRetryableFlow_singleton: 'Alleged: AdminRetryableFlow',
         PromiseWatcher_kindHandle: 'Alleged: kind',
         VowInternalsKit_kindHandle: 'Alleged: kind',
         WatchUtils_kindHandle: 'Alleged: kind',
+        retryableFlowForOutcomeVow: {},
       },
     }

packages/swingset-liveslots/tools/prepare-strict-test-env.js

@@ -21,6 +21,8 @@ export { flushIncarnation };
 export { eventLoopIteration as nextCrank };
 
 /**
+ * @import { PromiseKit } from '@endo/promise-kit'
+ * @import { Baggage } from '@agoric/swingset-liveslots'
  * @import { ReincarnateOptions } from './setup-vat-data.js'
  */
 
@@ -37,6 +39,7 @@ export const annihilate = (options = {}) => {
   return incarnation;
 };
 
+/** @returns {Baggage} */
 export const getBaggage = () => {
   return incarnation.fakeVomKit.cm.provideBaggage();
 };
@@ -51,7 +54,7 @@ export const nextLife = (fromIncarnation = incarnation) => {
 };
 
 /**
- * @template {(baggage: import('@agoric/swingset-liveslots').Baggage) => Promise<any> | any} B
+ * @template {(baggage: Baggage) => Promise<any> | any} B
  * @param {B} build
  * @param {(tools: Awaited<ReturnType<B>>) => Promise<void> | void} [run]
  * @param {object} [options]
@@ -72,7 +75,7 @@ export const startLife = async (
     oldIncarnationNumber,
   );
   const { fakeVomKit } = nextLife(fromIncarnation);
-  /** @type {Map<string, import('@endo/promise-kit').PromiseKit<any>>} */
+  /** @type {Map<string, PromiseKit<any>>} */
   const previouslyWatchedPromises = new Map();
   let buildTools;
   try {

packages/vow/README.md

@@ -150,8 +150,8 @@ Here is an (oversimplified) algorithm that `watch` and `when` use to obtain a
 final result:
 
 ```js
-// Directly await the non-retriable original specimen.
-// This is non-retriable because we don't know how our caller obtained
+// Directly await the non-retryable original specimen.
+// This is non-retryable because we don't know how our caller obtained
 // it in the first place, since it is an application-specific detail
 // that may not be side-effect free.
 let result = await specimenP;