refactor(SwingSet): Improve config documentation, typing, and async f…

…unction implementations (#10538)

## Description
* Document new config parameters (`defaultReapGCKrefs` and vat-level `neverReap`/`reapGCKrefs`, plus node-specific `nodeOptions`)
* Remove obsolete config parameters (vat-level `name`/`metered`/`virtualObjectCacheSize`)
* Improve the accuracy of SwingSet config types in both JSDoc and Patterns
* Simplify some async functions for better comprehensibility and interactive debugging (e.g., vats and bundles both require one of `sourceSpec`/`bundleSpec`/`bundle`, but the former additionally support `creationOptions`/`parameters` options while the latter do not)

### Security Considerations
None known.

### Scaling Considerations
n/a

### Documentation Considerations
None in particular.

### Testing Considerations
n/a

### Upgrade Considerations
n/a

packages/SwingSet/docs/configuration.md

@@ -22,6 +22,7 @@ The configuration object takes this form:
 config = {
   defaultManagerType: MANAGERTYPESTRING?
   includeDevDependencies: BOOL?
+  defaultReapGCKrefs: NUMBER|'never'?
   defaultReapInterval: NUMBER|'never'?
   relaxDurabilityRules: BOOL?
   snapshotInitial: NUMBER?
@@ -38,15 +39,15 @@ config = {
       endowments: DATAOBJECT?
       creationOptions: {
         enablePipelining: BOOL?
-        name: STRING?
         enableDisavow: BOOL?
         managerType: MANAGERTYPESTRING?
-        metered: BOOL?
+        neverReap: BOOL?
+        nodeOptions: STRING[]?
+        reapGCKrefs: NUMBER|'never'?
         reapInterval: NUMBER|'never'?
         enableSetup: BOOL?
         critical: BOOL?
         useTranscript: BOOL?
-        virtualObjectCacheSize: NUMBER?
       }?
     }+
   }
@@ -126,13 +127,18 @@ creates bundles to include the SwingSet package's dev dependencies as well as
 its regular runtime depencies in any bundles that it creates.  Defaults to
 `false`.
 
-The `defaultReapInterval` property specifies the default frequency with which
-the kernel should prompt each vat (by delivering it a `bringOutYourDead`
-directive) to perform garbage collection and notify the kernel of any dropped or
-released references that result.  This should either be an integer, indicating
-the number of deliveries that should be made to the vat before reaping, or the
-string `'never'`, indicating that such activity should never happen.  If
-defaults (yes, the default has a default) to 1.
+The `defaultReapGCKrefs` property specifies the default count of krefs to be
+received by each vat in GC deliveries (dropImports/retireImports/retireExports)
+before the kernel should deliver a "bringOutYourDead" to trigger garbage
+collection. It should either be an integer, or the string `'never'` to indicate
+that such counts do not affect when to deliver a bringOutYourDead. It defaults
+to 20 (cf. `DEFAULT_GC_KREFS_PER_BOYD`).
+
+The `defaultReapInterval` property specifies the default count of deliveries to
+be received by each vat before the kernel should deliver a "bringOutYourDead" to
+trigger garbage collection. It should either be an integer, or the string
+`'never'` to indicate that such counts do not affect when to deliver a
+bringOutYourDead. It defaults to 1.
 
 The `relaxDurabilityRules` property, if `true`, allows vat code running inside
 this swingset to store non-durable references inside durable objects and stores
@@ -197,10 +203,6 @@ not yet supported.
 - `enablePipelining` specifies whether the vat should be launched with
   promise pipelining enabled.  Defaults to `false`.
 
-- `name` is an optional short label string used to identify the vat in log and
-  debug outputs.  If omitted, it defaults to the property name that was used for
-  the vat descriptor.
-
 - `enableDisavow`, if `true`, adds `vatPowers.disavow()`, which allows vat code
   to explicitly disavow interest in an imported Presence. This will trigger a
   `syscall.dropImports` of the associated object ID. By default, this
@@ -209,11 +211,17 @@ not yet supported.
 - `managerType` indicates what sort of vat worker to run the associated vat in.
   If omitted, it defaults to the swingset's default manager type.
 
-- `metered` specifies whether the vat should be have metering turned
-  on or not. Defaults to `false`.
+- `neverReap` disables automatic bringOutYourDead deliveries to this vat.
+
+- `nodeOptions` is valid only with `managerType` `'node-subprocess'`, for which
+  it specifies additional command-line options to include when spawning (e.g.,
+  `['--inspect']` to support interactive debugging).
 
-- `reapInterval` specifies the reap interval for this particular vat.  It
-  defaults to the swingset's default reap interval.
+- `reapGCKrefs` overrides the swingset's `defaultReapGCKrefs` for this
+  particular vat.
+
+- `reapInterval`, overrides the swingset's `defaultReapInterval` for this
+  particular vat.
 
 - `enableSetup` indicates that the vat module may use the older, lower
   level `setup()` API, which allows a vat to be defined independent of
@@ -230,10 +238,6 @@ not yet supported.
   deliveries made to it so that these can be replayed at a later time to
   reconstruct the internal state of the vat.  Defaults to `true`.
 
-- `virtualObjectCacheSize` tells the vat's virtual object manager how many
-  virtual objects should have their state kept live in memory at any given
-  time.  Defaults to an implementation defined value.
-
 ## Dynamic option setting
 
 Some swingset and vat configuration options can be dynamically updated within a
@@ -271,8 +275,6 @@ vats, i.e., vats configured using the config object described above).
 
 Currently supported vat options that you can change this way are:
 
-- `virtualObjectCacheSize`
-
 - `reapInterval`
 
 Other options may be added in the future.

packages/SwingSet/src/controller/initializeKernel.js

@@ -100,7 +100,6 @@ export async function initializeKernel(config, kernelStorage, options = {}) {
       // the VatManager, since it isn't available until the bundle is evaluated
       assertKnownOptions(creationOptions, [
         'enablePipelining',
-        'metered',
         'managerType',
         'enableDisavow',
         'enableSetup',

packages/SwingSet/src/controller/initializeSwingset.js

@@ -442,26 +442,24 @@ export async function initializeSwingset(
    * The host application gives us
    * config.[vats|devices].NAME.[bundle|bundleSpec|sourceSpec|bundleName] .
    * The 'bundleName' option points into
-   * config.bundles.BUNDLENAME.[bundle|bundleSpec|sourceSpec] , which can
+   * config.bundles.BUNDLENAME.[bundle|bundleSpec|sourceSpec], which can
    * also include arbitrary named bundles that will be made available to
-   * E(vatAdminService).getNamedBundleCap(bundleName) ,and temporarily as
+   * E(vatAdminService).getNamedBundleCap(bundleName), and temporarily as
    * E(vatAdminService).createVatByName(bundleName)
    *
    * The 'kconfig' we pass through to initializeKernel has
    * kconfig.[vats|devices].NAME.bundleID and
-   * kconfig.namedBundleIDs.BUNDLENAME=bundleID , which both point into
+   * kconfig.namedBundleIDs.BUNDLENAME=bundleID, which both point into
    * kconfig.idToBundle.BUNDLEID=bundle
    *
-   * @param {SwingSetConfigProperties | { bundleName: string }} desc
+   * @param {SwingSetConfigProperties} desc
    * @param {Record<string, *>} [nameToBundle]
    */
   async function getBundle(desc, nameToBundle) {
     trace(
       'getBundle',
       Object.keys(desc),
-      // @ts-expect-error optional
       desc.moduleFormat,
-      // @ts-expect-error optional
       desc.endoZipBase64Sha512 || desc.sourceSpec,
     );
 
@@ -489,8 +487,9 @@ export async function initializeSwingset(
     throw Error(`unknown mode in desc`, desc);
   }
 
-  // fires with BundleWithID: { ...bundle, id }
   /**
+   * Returns a bundle record with an "id" property from an input that might be missing it.
+   *
    * @param {EndoZipBase64Bundle & {id?: string}} bundle
    * @returns {Promise<EndoZipBase64Bundle & {id: string}>} bundle
    */
@@ -508,11 +507,9 @@ export async function initializeSwingset(
     };
   }
 
-  // fires with BundleWithID: { ...bundle, id }
-
   /**
    *
-   * @param {(SwingSetConfigProperties | { bundleName: string }) & {bundleID?: string }} desc
+   * @param {SwingSetConfigProperties & {bundleID?: string}} desc
    * @param {Record<string, EndoZipBase64Bundle>} [nameToBundle]
    */
   async function processDesc(desc, nameToBundle) {
@@ -526,14 +523,14 @@ export async function initializeSwingset(
     modes.length === 1 ||
       Fail`need =1 of bundle/bundleSpec/sourceSpec/bundleName, got ${modes}`;
     const mode = modes[0];
-    return getBundle(desc, nameToBundle)
-      .then(addBundleID)
-      .then(bundleWithID => {
-        // replace original .sourceSpec/etc with a uniform .bundleID
-        delete desc[mode];
-        desc.bundleID = bundleWithID.id;
-        return bundleWithID;
-      });
+
+    // Remove the original mode in favor of a uniform "bundleID" property.
+    const bundle = await getBundle(desc, nameToBundle);
+    const bundleWithID = await addBundleID(bundle);
+    delete desc[mode];
+    desc.bundleID = bundleWithID.id;
+
+    return bundleWithID;
   }
 
   /**

packages/SwingSet/src/supervisors/supervisor-helper.js

@@ -32,16 +32,15 @@ function makeSupervisorDispatch(dispatch) {
   async function dispatchToVat(delivery) {
     // the (low-level) vat is responsible for giving up agency, but we still
     // protect against exceptions
-    return Promise.resolve(delivery)
-      .then(dispatch)
-      .then(
-        res => harden(['ok', res, null]),
-        err => {
-          // TODO react more thoughtfully, maybe terminate the vat
-          console.warn(`error during vat dispatch() of ${delivery}`, err);
-          return harden(['error', `${err}`, null]);
-        },
-      );
+    await null;
+    try {
+      const res = await dispatch(delivery);
+      return harden(['ok', res, null]);
+    } catch (err) {
+      // TODO react more thoughtfully, maybe terminate the vat
+      console.warn(`error during vat dispatch() of ${delivery}`, err);
+      return harden(['error', `${err}`, null]);
+    }
   }
 
   return harden(dispatchToVat);

packages/SwingSet/src/typeGuards.js

@@ -10,38 +10,40 @@ export const ManagerType = M.or(
 
 const Bundle = M.splitRecord({ moduleType: M.string() });
 
-const SwingsetConfigOptions = {
+const VatConfigOptions = harden({
   creationOptions: M.splitRecord({}, { critical: M.boolean() }),
   parameters: M.recordOf(M.string(), M.any()),
-};
-harden(SwingsetConfigOptions);
+});
 
-const SwingSetConfigProperties = M.or(
-  M.splitRecord({ sourceSpec: M.string() }, SwingsetConfigOptions),
-  M.splitRecord({ bundleSpec: M.string() }, SwingsetConfigOptions),
-  M.splitRecord({ bundle: Bundle }, SwingsetConfigOptions),
-);
-const SwingSetConfigDescriptor = M.recordOf(
-  M.string(),
-  SwingSetConfigProperties,
-);
+const makeSwingSetConfigProperties = (required = {}, optional = {}, rest) =>
+  M.or(
+    M.splitRecord({ sourceSpec: M.string(), ...required }, optional, rest),
+    M.splitRecord({ bundleSpec: M.string(), ...required }, optional, rest),
+    M.splitRecord({ bundle: Bundle, ...required }, optional, rest),
+  );
+const makeSwingSetConfigDescriptor = (required, optional, rest) =>
+  M.recordOf(
+    M.string(),
+    makeSwingSetConfigProperties(required, optional, rest),
+  );
 
 /**
  * NOTE: this pattern suffices for PSM bootstrap,
  * but does not cover the whole SwingSet config syntax.
  *
  * {@link ./docs/configuration.md}
- * TODO: move this to swingset?
  *
  * @see SwingSetConfig
  * in ./types-external.js
  */
-export const SwingSetConfig = M.and(
-  M.splitRecord({}, { defaultManagerType: ManagerType }),
-  M.splitRecord({}, { includeDevDependencies: M.boolean() }),
-  M.splitRecord({}, { defaultReapInterval: M.number() }), // not in type decl
-  M.splitRecord({}, { snapshotInterval: M.number() }),
-  M.splitRecord({}, { vats: SwingSetConfigDescriptor }),
-  M.splitRecord({}, { bootstrap: M.string() }),
-  M.splitRecord({}, { bundles: SwingSetConfigDescriptor }),
+export const SwingSetConfig = M.splitRecord(
+  { vats: makeSwingSetConfigDescriptor(undefined, VatConfigOptions) },
+  {
+    defaultManagerType: ManagerType,
+    includeDevDependencies: M.boolean(),
+    defaultReapInterval: M.number(),
+    snapshotInterval: M.number(),
+    bootstrap: M.string(),
+    bundles: makeSwingSetConfigDescriptor(undefined, undefined, {}),
+  },
 );

packages/SwingSet/src/types-external.js

@@ -154,27 +154,25 @@ export {};
  */
 
 /**
+ * @typedef {{ bundle: Bundle }} BundleRef a bundle object
+ * @typedef {{ bundleName: string }} BundleName a name identifying a property in the `bundles` of a SwingSetOptions object
+ * @typedef {{ bundleSpec: string }} BundleSpec a path to a bundle file
+ * @typedef {{ sourceSpec: string }} SourceSpec a package specifier such as "@agoric/swingset-vat/tools/vat-puppet.js"
+ *
  * @typedef {{
- *   sourceSpec: string // path to pre-bundled root
- * }} SourceSpec
- * @typedef {{
- *   bundleSpec: string // path to bundled code
- * }} BundleSpec
- * @typedef {{
- *   bundle: Bundle
- * }} BundleRef
- * @typedef {{
- *   bundleName: string
- * }} BundleName
- * @typedef {(SourceSpec | BundleSpec | BundleRef | BundleName ) & {
  *   bundleID?: BundleID,
- *   creationOptions?: Record<string, any>,
+ *   creationOptions?: StaticVatOptions,
  *   parameters?: Record<string, any>,
- * }} SwingSetConfigProperties
+ * }} VatConfigOptions
+ */
+/**
+ * @template [Fields=object]
+ * @typedef {(SourceSpec | BundleSpec | BundleName | BundleRef) & Fields} SwingSetConfigProperties
  */
 
 /**
- * @typedef {Record<string, SwingSetConfigProperties>} SwingSetConfigDescriptor
+ * @template [Fields=object]
+ * @typedef {Record<string, SwingSetConfigProperties<Fields>>} SwingSetConfigDescriptor
  * Where the property name is the name of the vat.  Note that
  * the `bootstrap` property names the vat that should be used as the bootstrap vat.  Although a swingset
  * configuration can designate any vat as its bootstrap vat, `loadBasedir` will always look for a file named
@@ -188,7 +186,7 @@ export {};
  * `devDependencies` of the surrounding `package.json` should be accessible to
  * bundles.
  * @property {string} [bundleCachePath] if present, SwingSet will use a bundle cache at this path
- * @property {SwingSetConfigDescriptor} vats
+ * @property {SwingSetConfigDescriptor<VatConfigOptions>} vats
  * @property {SwingSetConfigDescriptor} [bundles]
  * @property {BundleFormat} [bundleFormat] the bundle source / import bundle
  * format.
@@ -206,7 +204,7 @@ export {};
  */
 
 /**
- * @typedef {{ bundleName: string} | { bundle: Bundle } | { bundleID: BundleID } } SourceOfBundle
+ * @typedef {BundleName | BundleRef | {bundleID: BundleID}} SourceOfBundle
  */
 /**
  * @typedef { import('@agoric/swing-store').KVStore } KVStore
@@ -295,12 +293,8 @@ export {};
  * Vat Creation and Management
  *
  * @typedef { string } BundleID
- * @typedef {any} BundleCap
+ * @typedef { any } BundleCap
  * @typedef { { moduleFormat: 'endoZipBase64', endoZipBase64: string, endoZipBase64Sha512: string } } EndoZipBase64Bundle
- *
- * @typedef { unknown } Meter
- *
- * E(vatAdminService).createVat(bundle, options: DynamicVatOptions)
  */
 
 /**
@@ -326,8 +320,6 @@ export {};
  * types are then defined as amendments to this base type.
  *
  * @typedef { object } BaseVatOptions
- * @property { string } name
- * @property { * } [vatParameters]
  * @property { boolean } [enableSetup]
  *           If true, permits the vat to construct itself using the
  *           `setup()` API, which bypasses the imposition of LiveSlots but
@@ -346,6 +338,10 @@ export {};
  *            outbound syscalls so that the vat's internal state can be
  *            reconstructed via replay.  If false, no such record is kept.
  *            Defaults to true.
+ * @property { ManagerType } [managerType]
+ * @property { boolean } [neverReap]
+ *            If true, disables automatic bringOutYourDead deliveries to a vat.
+ *            Defaults to false.
  * @property { number | 'never' } [reapInterval]
  *            Trigger a bringOutYourDead after the vat has received
  *            this many deliveries. If the value is 'never',
@@ -360,7 +356,7 @@ export {};
  */
 
 /**
- * @typedef { { meter?: Meter } } OptMeter
+ * @typedef { { meter?: unknown } } OptMeter
  *        If a meter is provided, the new dynamic vat is limited to a fixed
  *        amount of computation and allocation that can occur during any
  *        given crank. Peak stack frames are limited as well. In addition,
@@ -370,14 +366,13 @@ export {};
  *        terminated. If undefined, the vat is unmetered. Static vats
  *        cannot be metered.
  *
- * @typedef { { managerType?: ManagerType } } OptManagerType
- * @typedef { BaseVatOptions & OptMeter & OptManagerType } DynamicVatOptions
+ * @typedef { BaseVatOptions & { name: string, vatParameters?: object } & OptMeter } DynamicVatOptions
  *
  * config.vats[name].creationOptions: StaticVatOptions
  *
  * @typedef { { enableDisavow?: boolean } } OptEnableDisavow
  * @typedef { { nodeOptions?: string[] } } OptNodeOptions
- * @typedef { BaseVatOptions & OptManagerType & OptEnableDisavow & OptNodeOptions } StaticVatOptions
+ * @typedef { BaseVatOptions & OptEnableDisavow & OptNodeOptions } StaticVatOptions
  *
  * @typedef { { vatParameters?: object, upgradeMessage?: string } } VatUpgradeOptions
  * @typedef { { incarnationNumber: number } } VatUpgradeResults