refactor(SwingSet): Cleanup vatManagerFactory (#10808)

* Warn on any use of `setup` or `enableSetup` that overrides `type`. * Improve error message detail. * Collapse together all typed vat factory invocations. ## Description * Warn on any use of `setup` or `enableSetup` that overrides `type`. * Improve error message detail. * Collapse together all typed vat factory invocations. * Rename "vatManagerFactory" to the more precise "makeVatManager" (we don't have a convention for the former), and accordingly rename "makeVatManagerFactory" to "makeVatManagerMaker" (which is slightly more cumbersome but substantially more clear). ### Security Considerations n/a ### Scaling Considerations n/a ### Documentation Considerations n/a ### Testing Considerations n/a ### Upgrade Considerations n/a
Agoric · Feb 19, 2025 · fe64626 · fe64626
2 parents 987bcf1 + 570fa57
commit fe64626
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 51 deletions.
diff --git a/packages/SwingSet/src/kernel/kernel.js b/packages/SwingSet/src/kernel/kernel.js
@@ -9,7 +9,7 @@ import { makeUpgradeDisconnection } from '@agoric/internal/src/upgrade-api.js';
 import { kser, kslot, makeError } from '@agoric/kmarshal';
 import { assertKnownOptions } from '../lib/assertOptions.js';
 import { foreverPolicy } from '../lib/runPolicies.js';
-import { makeVatManagerFactory } from './vat-loader/manager-factory.js';
+import { makeVatManagerMaker } from './vat-loader/manager-factory.js';
 import { makeVatWarehouse } from './vat-warehouse.js';
 import makeDeviceManager from './deviceManager.js';
 import makeKernelKeeper, {
@@ -1557,7 +1557,7 @@ export default function buildKernel(
     gcAndFinalize,
     meterControl: makeDummyMeterControl(),
   });
-  const vatManagerFactory = makeVatManagerFactory({
+  const makeVatManager = makeVatManagerMaker({
     allVatPowers,
     kernelKeeper,
     vatEndowments,
@@ -1652,7 +1652,7 @@ export default function buildKernel(
   }
 
   const vatLoader = makeVatLoader({
-    vatManagerFactory,
+    makeVatManager,
     kernelSlog,
     makeSourcedConsole,
     kernelKeeper,

diff --git a/packages/SwingSet/src/kernel/vat-loader/manager-factory.js b/packages/SwingSet/src/kernel/vat-loader/manager-factory.js
@@ -4,7 +4,7 @@ import { makeLocalVatManagerFactory } from './manager-local.js';
 import { makeNodeSubprocessFactory } from './manager-subprocess-node.js';
 import { makeXsSubprocessFactory } from './manager-subprocess-xsnap.js';
 
-export function makeVatManagerFactory({
+export function makeVatManagerMaker({
   allVatPowers,
   kernelKeeper,
   vatEndowments,
@@ -18,14 +18,12 @@ export function makeVatManagerFactory({
     vatEndowments,
     gcTools,
   });
-
   const nodeSubprocessFactory = makeNodeSubprocessFactory({
     startSubprocessWorker: startSubprocessWorkerNode,
     kernelKeeper,
     kernelSlog,
     testLog: allVatPowers.testLog,
   });
-
   const xsWorkerFactory = makeXsSubprocessFactory({
     startXSnap,
     kernelKeeper,
@@ -69,58 +67,41 @@ export function makeVatManagerFactory({
    * @param {import('@agoric/swingset-liveslots').LiveSlotsOptions} options.liveSlotsOptions
    * @returns { Promise<import('../../types-internal.js').VatManager> }
    */
-  async function vatManagerFactory(
-    vatID,
-    { managerOptions, liveSlotsOptions },
-  ) {
+  async function makeVatManager(vatID, options) {
+    const { managerOptions, liveSlotsOptions } = options;
     validateManagerOptions(managerOptions);
     const { workerOptions, enableSetup } = managerOptions;
     const { type } = workerOptions;
 
-    if (type !== 'local' && 'setup' in managerOptions) {
-      console.warn(`TODO: stop using setup() with ${type}`);
-    }
-    if (enableSetup) {
-      if (managerOptions.setup) {
-        return localFactory.createFromSetup(vatID, managerOptions);
-      } else {
-        return localFactory.createFromBundle(
-          vatID,
-          managerOptions.bundle,
-          managerOptions,
-          liveSlotsOptions,
-        );
-      }
-    } else if (type === 'local') {
-      return localFactory.createFromBundle(
-        vatID,
-        managerOptions.bundle,
-        managerOptions,
-        liveSlotsOptions,
-      );
+    if (type !== 'local' && (enableSetup || 'setup' in managerOptions)) {
+      console.warn(`TODO: stop using enableSetup and setup() with ${type}`);
     }
 
-    if (type === 'node-subprocess') {
-      return nodeSubprocessFactory.createFromBundle(
-        vatID,
-        managerOptions.bundle,
-        managerOptions,
-        liveSlotsOptions,
-      );
+    if (enableSetup && managerOptions.setup) {
+      return localFactory.createFromSetup(vatID, managerOptions);
     }
 
-    if (type === 'xsnap') {
-      assert(managerOptions.bundle, 'xsnap requires Bundle');
-      return xsWorkerFactory.createFromBundle(
-        vatID,
-        managerOptions.bundle,
-        managerOptions,
-        liveSlotsOptions,
-      );
+    /** @type {Pick<typeof xsWorkerFactory, 'createFromBundle'>} */
+    let factory;
+    if (enableSetup || type === 'local') {
+      factory = localFactory;
+    } else if (type === 'node-subprocess') {
+      factory = nodeSubprocessFactory;
+    } else if (type === 'xsnap') {
+      assert(managerOptions.bundle, 'worker type xsnap requires a bundle');
+      factory = xsWorkerFactory;
+    } else {
+      throw Error(`unknown vat worker type ${type}`);
     }
 
-    throw Error(`unknown type ${type}, not 'local' or 'xsnap'`);
+    return factory.createFromBundle(
+      vatID,
+      // @ts-expect-error managerOptions.bundle might be undefined
+      managerOptions.bundle,
+      managerOptions,
+      liveSlotsOptions,
+    );
   }
 
-  return harden(vatManagerFactory);
+  return harden(makeVatManager);
 }
diff --git a/packages/SwingSet/src/kernel/vat-loader/vat-loader.js b/packages/SwingSet/src/kernel/vat-loader/vat-loader.js
@@ -9,7 +9,7 @@ export function makeVatRootObjectSlot() {
 export function makeVatLoader(stuff) {
   const {
     overrideVatManagerOptions = {},
-    vatManagerFactory,
+    makeVatManager,
     kernelSlog,
     makeSourcedConsole,
     kernelKeeper,
@@ -127,7 +127,7 @@ export function makeVatLoader(stuff) {
     };
 
     const finish = starting && kernelSlog.startup(vatID);
-    const manager = await vatManagerFactory(vatID, {
+    const manager = await makeVatManager(vatID, {
       managerOptions,
       liveSlotsOptions,
     });
@@ -150,7 +150,7 @@ export function makeVatLoader(stuff) {
       ...overrideVatManagerOptions,
     };
     const liveSlotsOptions = {};
-    const manager = await vatManagerFactory(vatID, {
+    const manager = await makeVatManager(vatID, {
       managerOptions,
       liveSlotsOptions,
     });