fix(swingset): insist all durable kinds are reconnected by new version

This iterates through all previously-defined durable Kinds and asserts
that they have been reconnected by the time buildRootObject()
completes.

It still needs better error delivery path: we want the upgrade to fail
and get rolled back, but currently `startVat` doesn't have a good way
to signal the error.

refs #1848

packages/SwingSet/src/kernel/kernel.js

@@ -691,8 +691,13 @@ export default function buildKernel(
       // TODO: this is the message we want to send on failure, but we
       // need to queue it after the crank was unwound, else this
       // message will be unwound too
+      const err = {
+        '@qclass': 'error',
+        name: 'Error',
+        message: 'vat-upgrade failure notification not implemented',
+      };
       const args = {
-        body: JSON.stringify([upgradeID, false, { error: `not implemented` }]),
+        body: JSON.stringify([upgradeID, false, err]),
         slots: [],
       };
       queueToKref(vatAdminRootKref, 'vatUpgradeCallback', args, 'logFailure');

packages/SwingSet/src/liveslots/liveslots.js

@@ -1271,6 +1271,8 @@ function build(
     slotToVal.set(rootSlot, new WeakRef(rootObject));
     retainExportedVref(rootSlot);
     // we do not use vreffedObjectRegistry for root objects
+
+    vom.insistAllDurableKindsReconnected();
   }
 
   /**

packages/SwingSet/src/liveslots/virtualObjectManager.js

@@ -658,6 +658,7 @@ export function makeVirtualObjectManager(
 
   let kindIDID;
   const kindDescriptors = new WeakMap();
+  const definedDurableKinds = new Set(); // kindID
 
   function initializeKindHandleKind() {
     kindIDID = syscall.vatstoreGet('kindIDID');
@@ -697,7 +698,34 @@ export function makeVirtualObjectManager(
     const durableKindDescriptor = kindDescriptors.get(kindHandle);
     assert(durableKindDescriptor);
     const { kindID, tag } = durableKindDescriptor;
-    return defineKindInternal(kindID, tag, init, actualize, finish, true);
+    const maker = defineKindInternal(
+      kindID,
+      tag,
+      init,
+      actualize,
+      finish,
+      true,
+    );
+    definedDurableKinds.add(kindID);
+    return maker;
+  }
+
+  function insistAllDurableKindsReconnected() {
+    // identify all user-defined durable kinds by iterating `vom.kind.*`
+    const missing = [];
+    const prefix = 'vom.kind.';
+    let [key, value] = syscall.vatstoreGetAfter('', prefix);
+    while (key) {
+      const descriptor = JSON.parse(value);
+      if (!definedDurableKinds.has(descriptor.kindID)) {
+        missing.push(descriptor.tag);
+      }
+      [key, value] = syscall.vatstoreGetAfter(key, prefix);
+    }
+    if (missing.length) {
+      const tags = missing.join(',');
+      throw Error(`defineDurableKind not called for tags: ${tags}`);
+    }
   }
 
   function countWeakKeysForCollection(collection) {
@@ -721,6 +749,7 @@ export function makeVirtualObjectManager(
     defineKind,
     defineDurableKind,
     makeKindHandle,
+    insistAllDurableKindsReconnected,
     VirtualObjectAwareWeakMap,
     VirtualObjectAwareWeakSet,
     flushCache: cache.flush,

packages/SwingSet/test/stores/test-storeGC.js

@@ -479,6 +479,10 @@ function validateInit(v) {
   validate(v, matchVatstoreGet('baggageID', NONE));
   validateCreateBaggage(v, 1);
   validateCreateHolder(v, 2);
+  validate(
+    v,
+    matchVatstoreGetAfter('', 'vom.kind.', 'vom.kind/', [NONE, NONE]),
+  );
 }
 
 function validateDropHeld(v, rp, rc, es) {

packages/SwingSet/test/test-baggage.js

@@ -5,6 +5,7 @@ import { Far } from '@endo/marshal';
 import {
   setupTestLiveslots,
   matchVatstoreGet,
+  matchVatstoreGetAfter,
   matchVatstoreSet,
   validate,
   validateDone,
@@ -74,6 +75,7 @@ test.serial('exercise baggage', async t => {
   validate(v, matchVatstoreSet('vc.1.soutside', stringVal('outer val')));
   validate(v, matchVatstoreGet('vc.1.|entryCount', '0'));
   validate(v, matchVatstoreSet('vc.1.|entryCount', '1'));
+  validate(v, matchVatstoreGetAfter('', 'vom.kind.', NONE, [NONE, NONE]));
   validateDone(v);
 
   const rp = await dispatchMessage('doSomething', capargs([]));

packages/SwingSet/test/upgrade/bootstrap-upgrade.js

@@ -61,5 +61,21 @@ export function buildRootObject() {
       resolve(`message for your predecessor, don't freak out`);
       return { version, data, ...parameters };
     },
+
+    async buildV1WithLostKind() {
+      const bcap = await E(vatAdmin).getNamedBundleCap('ulrik1');
+      const vatParameters = { youAre: 'v1', marker };
+      const options = { vatParameters };
+      const res = await E(vatAdmin).createVat(bcap, options);
+      ulrikRoot = res.root;
+      ulrikAdmin = res.adminNode;
+      await E(ulrikRoot).makeLostKind();
+    },
+
+    async upgradeV2WhichLosesKind() {
+      const bcap = await E(vatAdmin).getNamedBundleCap('ulrik2');
+      const vatParameters = { youAre: 'v2', marker };
+      await E(ulrikAdmin).upgrade(bcap, vatParameters); // throws
+    },
   });
 }

packages/SwingSet/test/upgrade/test-upgrade.js

@@ -3,6 +3,7 @@ import { test } from '../../tools/prepare-test-env-ava.js';
 
 // eslint-disable-next-line import/order
 import { assert } from '@agoric/assert';
+import { parse } from '@endo/marshal';
 import { provideHostStorage } from '../../src/controller/hostStorage.js';
 import { initializeSwingset, makeSwingsetController } from '../../src/index.js';
 import { capargs, capdataOneSlot } from '../util.js';
@@ -89,3 +90,47 @@ test('vat upgrade - local', async t => {
 test('vat upgrade - xsnap', async t => {
   return testUpgrade(t, 'xs-worker');
 });
+
+test('failed upgrade - lost kind', async t => {
+  const config = {
+    defaultManagerType: 'xs-worker',
+    bootstrap: 'bootstrap',
+    defaultReapInterval: 'never',
+    vats: {
+      bootstrap: { sourceSpec: bfile('bootstrap-upgrade.js') },
+    },
+    bundles: {
+      ulrik1: { sourceSpec: bfile('vat-ulrik-1.js') },
+      ulrik2: { sourceSpec: bfile('vat-ulrik-2.js') },
+    },
+  };
+
+  const hostStorage = provideHostStorage();
+  await initializeSwingset(config, [], hostStorage);
+  const c = await makeSwingsetController(hostStorage);
+  c.pinVatRoot('bootstrap');
+  await c.run();
+
+  async function run(name, args = []) {
+    assert(Array.isArray(args));
+    const kpid = c.queueToVatRoot('bootstrap', name, capargs(args));
+    await c.run();
+    const status = c.kpStatus(kpid);
+    const capdata = c.kpResolution(kpid);
+    return [status, capdata];
+  }
+
+  // create initial version
+  const [v1status] = await run('buildV1WithLostKind', []);
+  t.is(v1status, 'fulfilled');
+
+  // upgrade should fail
+  const [v2status, v2capdata] = await run('upgradeV2WhichLosesKind', []);
+  t.is(v2status, 'rejected');
+  console.log(v2capdata);
+  const e = parse(v2capdata.body);
+  t.truthy(e instanceof Error);
+  t.regex(e.message, /vat-upgrade failure/);
+  // TODO: who should see the details of what v2 did wrong? calling
+  // vat? only the console?
+});

packages/SwingSet/test/upgrade/vat-ulrik-1.js

@@ -66,5 +66,9 @@ export function buildRootObject(_vatPowers, vatParameters, baggage) {
     returnEternalPromise() {
       return p2;
     },
+
+    makeLostKind() {
+      makeKindHandle('unhandled', []);
+    },
   });
 }

packages/SwingSet/test/virtualObjects/test-virtualObjectGC.js

@@ -6,6 +6,7 @@ import {
   setupTestLiveslots,
   matchResolveOne,
   matchVatstoreGet,
+  matchVatstoreGetAfter,
   matchVatstoreDelete,
   matchVatstoreSet,
   matchRetireExports,
@@ -477,6 +478,10 @@ function validateSetup(v) {
   validateCreateBaggage(v, 1);
   validate(v, matchVatstoreSet(stateKey(cacheDisplacerVref), cacheObjValue));
   validate(v, matchVatstoreSet(stateKey(fCacheDisplacerVref), cacheObjValue));
+  validate(
+    v,
+    matchVatstoreGetAfter('', 'vom.kind.', 'vom.kind/', [NONE, NONE]),
+  );
   validate(
     v,
     matchVatstoreSet(stateKey(virtualHolderVref), heldThingValue(null)),