From aa6518b79259c9ef10b1905e806f98e556f62714 Mon Sep 17 00:00:00 2001
From: Richard Gibson <richard.gibson@gmail.com>
Date: Fri, 24 Mar 2023 10:27:14 -0400
Subject: [PATCH 01/12] test(SwingSet): Clean up testUpgrade

---
 .../upgrade/bootstrap-scripted-upgrade.js     |  21 ++-
 packages/SwingSet/test/upgrade/num-sensors.js |   1 -
 .../SwingSet/test/upgrade/test-upgrade.js     | 147 ++++++++----------
 packages/SwingSet/test/upgrade/vat-ulrik-1.js |  77 ++++-----
 4 files changed, 119 insertions(+), 127 deletions(-)
 delete mode 100644 packages/SwingSet/test/upgrade/num-sensors.js

diff --git a/packages/SwingSet/test/upgrade/bootstrap-scripted-upgrade.js b/packages/SwingSet/test/upgrade/bootstrap-scripted-upgrade.js
index 36afa617b9a..df10898db6d 100644
--- a/packages/SwingSet/test/upgrade/bootstrap-scripted-upgrade.js
+++ b/packages/SwingSet/test/upgrade/bootstrap-scripted-upgrade.js
@@ -3,7 +3,7 @@ import { Far } from '@endo/marshal';
 import { assert } from '@agoric/assert';
 import { makePromiseKit } from '@endo/promise-kit';
 
-import { NUM_SENSORS } from './num-sensors.js';
+const NUM_SENSORS = 38;
 
 const insistMissing = (ref, isCollection = false) => {
   let p;
@@ -27,9 +27,12 @@ export const buildRootObject = () => {
   let ulrikRoot;
   let ulrikAdmin;
   const marker = Far('marker', {});
-  // sensors are numbered from '1' to match the vobj o+N/1.. vrefs
+  // for debugging, this array starts with a dummy element so
+  // the vref of each contained object in an importing vat
+  // (o-NN where NN starts at 1) is aligned with its index
+  /** @type {[string, ...object]} */
   const importSensors = ['skip0'];
-  for (let i = 1; i < NUM_SENSORS + 1; i += 1) {
+  for (let i = 1; i <= NUM_SENSORS; i += 1) {
     importSensors.push(Far(`import-${i}`, {}));
   }
   const { promise, resolve } = makePromiseKit();
@@ -64,13 +67,17 @@ export const buildRootObject = () => {
 
       dur = await E(ulrikRoot).getDurandal({ d1: 'd1' });
       const d1arg = await E(dur).get();
-      assert.equal(d1arg.d1, 'd1');
+      // poor man's deepEqual
+      // (each enumerable own property as a [key, value] pair)
+      assert.equal(JSON.stringify(Object.entries(d1arg)), '[["d1","d1"]]');
 
-      // give ver1 a promise that won't be resolved until ver2
+      // give version 1 a promise that won't be resolved until after upgrade
       await E(ulrikRoot).acceptPromise(promise);
-      const { p1 } = await E(ulrikRoot).getEternalPromise();
+
+      // get promises that never resolve (and will be rejected at upgrade)
+      const [p1] = await E(ulrikRoot).getEternalPromiseInArray();
       p1.catch(() => 'hush');
-      const p2 = E(ulrikRoot).returnEternalPromise(); // never resolves
+      const p2 = E(ulrikRoot).getEternalPromise();
       p2.catch(() => 'hush');
 
       return { version, data, p1, p2, retain, ...parameters };
diff --git a/packages/SwingSet/test/upgrade/num-sensors.js b/packages/SwingSet/test/upgrade/num-sensors.js
deleted file mode 100644
index 289eea593e8..00000000000
--- a/packages/SwingSet/test/upgrade/num-sensors.js
+++ /dev/null
@@ -1 +0,0 @@
-export const NUM_SENSORS = 38;
diff --git a/packages/SwingSet/test/upgrade/test-upgrade.js b/packages/SwingSet/test/upgrade/test-upgrade.js
index a8adcb4f8d8..ae5c7b85a6a 100644
--- a/packages/SwingSet/test/upgrade/test-upgrade.js
+++ b/packages/SwingSet/test/upgrade/test-upgrade.js
@@ -5,6 +5,7 @@ import { test } from '../../tools/prepare-test-env-ava.js';
 // eslint-disable-next-line import/order
 import { assert } from '@agoric/assert';
 import bundleSource from '@endo/bundle-source';
+import { objectMap } from '@agoric/internal';
 import { initSwingStore } from '@agoric/swing-store';
 import { parseReachableAndVatSlot } from '../../src/kernel/state/reachable.js';
 import { parseVatSlot } from '../../src/lib/parseVatSlots.js';
@@ -16,13 +17,11 @@ import {
 } from '../../src/index.js';
 import { bundleOpts, restartVatAdminVat } from '../util.js';
 
-// import { NUM_SENSORS } from './num-sensors.js';
-
 const bfile = name => new URL(name, import.meta.url).pathname;
 
 test.before(async t => {
   const kernelBundles = await buildKernelBundles();
-  t.context.data = { kernelBundles };
+  /** @type {any} */ (t.context).data = { kernelBundles };
 });
 
 // eslint-disable-next-line no-unused-vars
@@ -191,21 +190,35 @@ const testUpgrade = async (t, defaultManagerType, options = {}) => {
       ulrik2: 'vat-ulrik-2.js',
     },
   });
+  /** @type {object} */
+  const { data: bundleData } = t.context;
   const {
     controller: c,
     kvStore,
     run,
-  } = await initKernelForTest(t, t.context.data, config);
+  } = await initKernelForTest(t, bundleData, config);
+
+  const verifyPresence = (presence, iface = undefined) => {
+    const kref = krefOf(presence);
+    t.truthy(kref);
+    if (iface) {
+      t.is(presence.iface(), iface);
+    }
+    return kref;
+  };
 
-  const marker = await run('getMarker'); // probably ko26
-  t.is(marker.iface(), 'marker');
+  const marker = await run('getMarker');
+  const markerKref = verifyPresence(marker, 'marker'); // probably 'ko26'
 
-  // fetch all the "importSensors": exported by bootstrap, imported by
+  // fetch all the "import sensors": exported by bootstrap, imported by
   // the upgraded vat. We'll determine their krefs and later query the
   // upgraded vat to see if it's still importing them or not
-  const impresult = await run('getImportSensors', []);
+  const sensors = await run('getImportSensors', []);
   // eslint-disable-next-line no-unused-vars
-  const impKrefs = ['skip0', ...impresult.slice(1).map(krefOf)];
+  const sensorKrefs = [
+    'skip0',
+    ...sensors.slice(1).map(obj => verifyPresence(obj)),
+  ];
 
   if (doVatAdminRestart) {
     await restartVatAdminVat(c);
@@ -213,98 +226,72 @@ const testUpgrade = async (t, defaultManagerType, options = {}) => {
 
   // create initial version
   const v1result = await run('buildV1', []);
-  t.is(v1result.version, 'v1');
-  t.is(v1result.youAre, 'v1');
-  t.truthy(krefOf(v1result.marker));
-  t.truthy(krefOf(marker));
-  t.is(krefOf(v1result.marker), krefOf(marker));
-  t.is(v1result.marker.iface(), 'marker');
-  t.deepEqual(v1result.data, ['some', 'data']);
+  t.like(v1result, {
+    version: 'v1',
+    youAre: 'v1',
+    data: ['some', 'data'],
+  });
+  const v1MarkerKref = verifyPresence(v1result.marker, 'marker');
+  t.is(v1MarkerKref, markerKref);
   // grab the promises that should be rejected
-  const v1p1Kref = krefOf(v1result.p1);
-  const v1p2Kref = krefOf(v1result.p2);
-  t.truthy(v1p1Kref);
-  t.truthy(v1p2Kref);
-
-  // grab exports to deduce durable/virtual vrefs
-  const dur1Kref = krefOf(v1result.retain.dur1);
-  const vir2Kref = krefOf(v1result.retain.vir2);
-  const vir5Kref = krefOf(v1result.retain.vir5);
-  const vir7Kref = krefOf(v1result.retain.vir7);
-  t.truthy(dur1Kref);
-  t.truthy(vir2Kref);
-  t.truthy(vir5Kref);
-  t.truthy(vir7Kref);
-
-  const vatID = kvStore.get(`${dur1Kref}.owner`); // probably v6
+  const v1p1Kref = verifyPresence(v1result.p1);
+  const v1p2Kref = verifyPresence(v1result.p2);
+  // grab krefs for the exported durable/virtual objects to check their abandonment
+  const retainedKrefs = objectMap(v1result.retain, obj => verifyPresence(obj));
+  const retainedNames = 'dur1 vir2 vir5 vir7 vc1 vc3 dc4 rem1 rem2 rem3';
+  t.deepEqual(
+    Object.keys(retainedKrefs).sort(),
+    retainedNames.split(' ').sort(),
+  );
+  const { dur1: dur1Kref, vir2: vir2Kref } = retainedKrefs;
+
+  // use the kvStore to support assertions about exported durable/virtual vrefs
+  const vatID = kvStore.get(`${dur1Kref}.owner`); // probably 'v6'
+  // dumpState(debug, vatID);
   const getVref = kref => {
     const s = kvStore.get(`${vatID}.c.${kref}`);
     return parseReachableAndVatSlot(s).vatSlot;
   };
-  // const krefReachable = kref => {
-  //   const s = kvStore.get(`${vatID}.c.${kref}`);
-  //   return !!(s && parseReachableAndVatSlot(s).isReachable);
-  // };
-
-  // We look in the vat's vatstore to see if the virtual/durable
-  // object exists or not (as a state record).
-  const vomHas = vref => {
-    return kvStore.has(`${vatID}.vs.vom.${vref}`);
-  };
-
-  // dumpState(debug, vatID);
-
-  // deduce exporter vrefs for all durable/virtual objects, and assert
-  // that they're still in DB
+  const vomHas = vref => kvStore.has(`${vatID}.vs.vom.${vref}`);
   const dur1Vref = getVref(dur1Kref);
   t.is(parseVatSlot(dur1Vref).subid, 1n);
-  const durBase = dur1Vref.slice(0, dur1Vref.length - 2);
-  const durVref = i => {
-    return `${durBase}/${i}`;
-  };
+  const durPrefix = dur1Vref.slice(0, -1); // everything before the trailing '1'
+  const durVref = i => `${durPrefix}${i}`;
   const vir2Vref = getVref(vir2Kref);
   t.is(parseVatSlot(vir2Vref).subid, 2n);
-  const virBase = vir2Vref.slice(0, vir2Vref.length - 2);
-  const virVref = i => {
-    return `${virBase}/${i}`;
-  };
+  const virPrefix = vir2Vref.slice(0, -1); // everything before the trailing '2'
+  const virVref = i => `${virPrefix}${i}`;
 
   t.true(vomHas(durVref(1)));
   t.true(vomHas(virVref(2)));
-  t.false(vomHas(virVref(1))); // deleted before upgrade
-  t.false(vomHas(durVref(2))); // deleted before upgrade
-
-  // remember krefs for the exported objects so we can check their
-  // abandonment
-  const retainedNames = 'dur1 vir2 vir5 vir7 vc1 vc3 dc4 rem1 rem2 rem3';
-  const retainedKrefs = {};
-  for (const name of retainedNames.split(' ')) {
-    retainedKrefs[name] = krefOf(v1result.retain[name]);
-  }
+  t.false(vomHas(virVref(1))); // GCed before upgrade
+  t.false(vomHas(durVref(2))); // GCed before upgrade
 
   if (doVatAdminRestart) {
     await restartVatAdminVat(c);
   }
 
   // now perform the upgrade
-  // console.log(`-- starting upgradeV2`);
-
   const v2result = await run('upgradeV2', []);
-  t.deepEqual(v2result.version, 'v2');
-  t.deepEqual(v2result.youAre, 'v2');
-  t.deepEqual(krefOf(v2result.marker), krefOf(marker));
-  t.deepEqual(v2result.data, ['some', 'data']);
+  // dumpState(debug, vatID);
+  t.like(v2result, {
+    version: 'v2',
+    youAre: 'v2',
+    data: ['some', 'data'],
+    remoerr: Error('vat terminated'),
+  });
   t.deepEqual(v2result.upgradeResult, { incarnationNumber: 2 });
-  t.deepEqual(v2result.remoerr, Error('vat terminated'));
+  const v2MarkerKref = verifyPresence(v2result.marker, 'marker');
+  t.deepEqual(v2MarkerKref, v1MarkerKref);
 
-  // newDur() (the first Durandal instance created in vat-ulrik-2)
+  // newDur (the first Durandal instance created in vat-ulrik-2)
   // should get a new vref, because the per-Kind instance counter
   // should persist and pick up where it left off. If that was broken,
   // newDur would have the same vref as dur1 (the first Durandal
   // instance created in vat-ulrik-1). And since it's durable, the
   // c-list entry will still exist, so we'll see the same kref as
   // before.
-  const newDurKref = krefOf(v2result.newDur);
+  const newDurKref = verifyPresence(v2result.newDur);
   t.not(newDurKref, dur1Kref);
 
   // the old version's non-durable promises should be rejected
@@ -318,10 +305,8 @@ const testUpgrade = async (t, defaultManagerType, options = {}) => {
   t.is(c.kpStatus(v1p2Kref), 'rejected');
   t.deepEqual(kunser(c.kpResolution(v1p2Kref)), vatUpgradedError);
 
-  // dumpState(debug, vatID);
-
   // all the merely-virtual exports should be gone
-  // for (let i = 1; i < NUM_SENSORS + 1; i += 1) {
+  // for (let i = 1; i < sensors.length; i += 1) {
   //   t.false(vomHas(virVref(i)));
   // }
 
@@ -365,9 +350,9 @@ const testUpgrade = async (t, defaultManagerType, options = {}) => {
   survivingDurables.push(23);
   survivingImported.push(23);
 
-  for (let i = 1; i < NUM_SENSORS + 1; i += 1) {
+  for (let i = 1; i < sensors.length; i += 1) {
     const vref = durVref(i);
-    // const impKref = impKrefs[i];
+    // const impKref = sensorKrefs[i];
     const expD = survivingDurables.includes(i);
     // const expI = survivingImported.includes(i);
     // const reachable = krefReachable(impKref);
@@ -385,8 +370,8 @@ const testUpgrade = async (t, defaultManagerType, options = {}) => {
 
   // check koNN.owner to confirm the exported virtuals (2/5/7) are abandoned
   t.false(kvStore.has(`${vir2Kref}.owner`));
-  t.false(kvStore.has(`${vir5Kref}.owner`));
-  t.false(kvStore.has(`${vir7Kref}.owner`));
+  t.false(kvStore.has(`${v1result.retain.vir5}.owner`));
+  t.false(kvStore.has(`${v1result.retain.vir7}.owner`));
 };
 
 test('vat upgrade - local', async t => {
diff --git a/packages/SwingSet/test/upgrade/vat-ulrik-1.js b/packages/SwingSet/test/upgrade/vat-ulrik-1.js
index 6e49c9ccd4f..e7785de2ce8 100644
--- a/packages/SwingSet/test/upgrade/vat-ulrik-1.js
+++ b/packages/SwingSet/test/upgrade/vat-ulrik-1.js
@@ -10,45 +10,50 @@ import {
   makeScalarBigMapStore,
   makeScalarBigWeakMapStore,
 } from '@agoric/vat-data';
-import { NUM_SENSORS } from './num-sensors.js';
 
+// we set up a lot of ephemeral, merely-virtual, and durable objects
+// holding references to imported objects
+// to test what gets deleted vs retained (see object-graph.pdf for the test plan)
+const makeEph = (name, held) => {
+  return Far(name, { get: () => held });
+};
 const durandalHandle = makeKindHandle('durandal');
-const initialize = (name, imp, value) => {
+const initHolder = (name, imp, value) => {
   return harden({ name, imp, value });
 };
-
-const behavior = {
+const holderMethods = {
   get: ({ state }) => state.value,
   set: ({ state }, value) => {
     state.value = value;
   },
 };
-
-const makeDurandal = defineDurableKind(durandalHandle, initialize, behavior);
-const makeVir = defineKind('virtual', initialize, behavior);
-const makeDummy = defineKind('dummy', initialize, behavior);
+const makeVir = defineKind('virtual', initHolder, holderMethods);
+const makeDur = defineDurableKind(durandalHandle, initHolder, holderMethods);
+const makeDummy = defineKind('dummy', initHolder, holderMethods);
 
 // TODO: explore 'export modRetains'
 // eslint-disable-next-line no-unused-vars
 let modRetains;
 
-// we set up a lot of virtual and durable objects to test what gets
-// deleted vs retained (see object-graph.pdf for the test plan)
-
-const makeRemotable = (name, held) => {
-  return Far(name, { get: () => held });
-};
-
+/**
+ * @param {import('@agoric/vat-data').Baggage} baggage
+ * @param {[unknown, ...object]} imp
+ * Objects to import, preceded by a dummy element.
+ * The `imp` name itself is three characters long for visual similarity
+ * with `vir` and `dur` analogs.
+ * @returns {Record<string, object>}
+ */
 const buildExports = (baggage, imp) => {
-  // each virtual/durable object has a unique import, some of which
-  // should be dropped during upgrade
-
-  // start these at '1' to match the vrefs (o+10/1 .. /5) for debugging
+  // for debugging, these arrays start with a dummy element so
+  // the vref of each contained object (o+X/NN where NN starts at 1)
+  // is aligned with its index
+  /** @type {[string, ...object]} */
   const vir = ['skip0'];
+  /** @type {[string, ...object]} */
   const dur = ['skip0'];
-  for (let i = 1; i < NUM_SENSORS + 1; i += 1) {
+  for (let i = 1; i < imp.length; i += 1) {
     vir.push(makeVir(`v${i}`, imp[i], { name: `v${i}` }));
-    dur.push(makeDurandal(`d${i}`, imp[i], { name: `d${i}` }));
+    dur.push(makeDur(`d${i}`, imp[i], { name: `d${i}` }));
   }
 
   // note: to test #5725, dur[0] must be the first new Durandal
@@ -58,7 +63,6 @@ const buildExports = (baggage, imp) => {
   // cycle-collection means we don't GC it during the lifetime of the
   // vat, however they'll be deleted during upgrade because stopVat()
   // deletes all virtual data independent of refcounts
-
   const vc1 = makeScalarBigMapStore('vc1');
   const vc2 = makeScalarBigMapStore('vc2');
   const vc3 = makeScalarBigMapStore('vc3');
@@ -68,7 +72,6 @@ const buildExports = (baggage, imp) => {
   // cycle-collecting virtual/durable-object GC. dc3 is dropped (only
   // held by an abandoned Remotable), dc4 is retained by an export,
   // dc5+dc6 are retained by baggage
-
   const dc1 = makeScalarBigMapStore('dc1', { durable: true });
   const dc2 = makeScalarBigMapStore('dc2', { durable: true });
   const dc3 = makeScalarBigMapStore('dc3', { durable: true });
@@ -78,9 +81,9 @@ const buildExports = (baggage, imp) => {
 
   // these Remotables are both exported and held by module-level pins,
   // but will still be abandoned
-  const rem1 = makeRemotable('rem1', [imp[4], vir[5], vir[7], vc1, vc3]);
-  const rem2 = makeRemotable('rem2', [dur[21], dc3]);
-  const rem3 = makeRemotable('rem3', [dur[29], imp[30], vir[31]]);
+  const rem1 = makeEph('rem1', [imp[4], vir[5], vir[7], vc1, vc3]);
+  const rem2 = makeEph('rem2', [dur[21], dc3]);
+  const rem3 = makeEph('rem3', [dur[29], imp[30], vir[31]]);
   modRetains = { rem1, rem2, rem3 };
 
   // now wire them up according to the diagram
@@ -113,18 +116,16 @@ const buildExports = (baggage, imp) => {
   baggage.init('dur37', dur[37]);
   baggage.init('imp38', imp[38]);
 
-  // We set virtualObjectCacheSize=0 to ensure all data writes are
+  // we set virtualObjectCacheSize=0 to ensure all data writes are
   // made promptly, But the cache will still retain the last
   // Representative, which inhibits GC. So the last thing we do here
-  // should be to create/deserialize a throwaway object, to make sure
-  // all the ones we're measuring are collected as we expect.
-
-  makeDummy(); // knock the last dur/vir/vc/dc out of the cache
+  // should be to create/deserialize a throwaway object, to
+  // flush the last dur/vir/vc/dc from the cache.
+  makeDummy();
 
   // we share dur1/vir2 with the test harness so it can glean the
   // baserefs and interpolate the full vrefs for everything else
   // without holding a GC pin on them
-
   return {
     dur1: dur[1],
     vir2: vir[2],
@@ -161,18 +162,18 @@ export const buildRootObject = (_vatPowers, vatParameters, baggage) => {
     },
     getPresence: () => baggage.get('presence'),
     getData: () => baggage.get('data'),
-    getDurandal: arg => makeDurandal('durandal', 0, arg),
-    getExports: imp => buildExports(baggage, imp),
+    getDurandal: arg => makeDur('durandal', 0, arg),
+    getExports: imports => buildExports(baggage, imports),
 
     acceptPromise: p => {
-      // stopVat will reject the promises that we decide, but should
+      // upgrade will reject the promises that we decide, but should
       // not touch the ones we don't decide, so we hold onto this
-      // until upgrade, to probe for bugs in that loop
+      // until upgrade, to probe for bugs in that process
       heldPromise = p;
       heldPromise.catch(() => 'hush');
     },
-    getEternalPromise: () => ({ p1 }),
-    returnEternalPromise: () => p2,
+    getEternalPromiseInArray: () => [p1],
+    getEternalPromise: () => p2,
 
     makeLostKind: () => {
       makeKindHandle('unhandled');

From a82124c6724e769e3e5fdad2ddf1908eab20392d Mon Sep 17 00:00:00 2001
From: Richard Gibson <richard.gibson@gmail.com>
Date: Fri, 24 Mar 2023 23:42:13 -0400
Subject: [PATCH 02/12] test(SwingSet): Improve testUpgrade GC coverage

---
 .../upgrade/bootstrap-scripted-upgrade.js     |   2 +-
 .../SwingSet/test/upgrade/test-upgrade.js     | 218 ++++++++++--------
 packages/SwingSet/test/upgrade/vat-ulrik-1.js |   2 +-
 3 files changed, 118 insertions(+), 104 deletions(-)

diff --git a/packages/SwingSet/test/upgrade/bootstrap-scripted-upgrade.js b/packages/SwingSet/test/upgrade/bootstrap-scripted-upgrade.js
index df10898db6d..9ba7bc9639f 100644
--- a/packages/SwingSet/test/upgrade/bootstrap-scripted-upgrade.js
+++ b/packages/SwingSet/test/upgrade/bootstrap-scripted-upgrade.js
@@ -3,7 +3,7 @@ import { Far } from '@endo/marshal';
 import { assert } from '@agoric/assert';
 import { makePromiseKit } from '@endo/promise-kit';
 
-const NUM_SENSORS = 38;
+const NUM_SENSORS = 39;
 
 const insistMissing = (ref, isCollection = false) => {
   let p;
diff --git a/packages/SwingSet/test/upgrade/test-upgrade.js b/packages/SwingSet/test/upgrade/test-upgrade.js
index ae5c7b85a6a..62c56d4f5d7 100644
--- a/packages/SwingSet/test/upgrade/test-upgrade.js
+++ b/packages/SwingSet/test/upgrade/test-upgrade.js
@@ -76,14 +76,15 @@ const makeConfigFromPaths = (bootstrapVatPath, options = {}) => {
  * @param {object} bundleData
  * @param {SwingSetConfig} config
  * @param {object} [options]
- * @deprecated Refcount incrementing should be manual,
+ * @param {boolean} [options.holdObjectRefs=true] - DEPRECATED -
+ * Refcount incrementing should be manual,
  * see https://github.com/Agoric/agoric-sdk/issues/7213
- * @param {boolean} [options.holdObjectRefs=true]
- * @returns {{
- *   controller: ReturnType<typeof makeSwingsetController>,
+ * @returns {Promise<{
+ *   controller: Awaited<ReturnType<typeof makeSwingsetController>>,
  *   kvStore: KVStore,
  *   run: (method: string, args?: unknown[]) => Promise<unknown>,
- * }}
+ *   runAndRetain: (method: string, args?: unknown[]) => Promise<unknown>,
+ * }>}
  */
 const initKernelForTest = async (t, bundleData, config, options = {}) => {
   const { kernelStorage } = initSwingStore();
@@ -95,24 +96,27 @@ const initKernelForTest = async (t, bundleData, config, options = {}) => {
   t.teardown(c.shutdown);
   c.pinVatRoot('bootstrap');
   await c.run();
-  const kpResolutionOptions = { incref: holdObjectRefs };
-  const run = async (method, args = [], vatName = 'bootstrap') => {
-    assert(Array.isArray(args));
-    const kpid = c.queueToVatRoot(vatName, method, args);
-    await c.run();
-    const status = c.kpStatus(kpid);
-    if (status === 'fulfilled') {
-      const result = c.kpResolution(kpid, kpResolutionOptions);
-      return kunser(result);
-    }
-    assert(status === 'rejected');
-    const err = c.kpResolution(kpid, kpResolutionOptions);
-    throw kunser(err);
+  const makeRun = kpResolutionOptions => {
+    const run = async (method, args = [], vatName = 'bootstrap') => {
+      assert(Array.isArray(args));
+      const kpid = c.queueToVatRoot(vatName, method, args);
+      await c.run();
+      const status = c.kpStatus(kpid);
+      if (status === 'fulfilled') {
+        const result = c.kpResolution(kpid, kpResolutionOptions);
+        return kunser(result);
+      }
+      assert(status === 'rejected');
+      const err = c.kpResolution(kpid, kpResolutionOptions);
+      throw kunser(err);
+    };
+    return run;
   };
   return {
     controller: c,
     kvStore,
-    run,
+    run: makeRun({ incref: holdObjectRefs }),
+    runAndRetain: makeRun({ incref: true }),
   };
 };
 
@@ -180,9 +184,11 @@ test('null upgrade - xsnap', async t => {
  * @param {ManagerType} defaultManagerType
  * @param {object} [options]
  * @param {boolean} [options.restartVatAdmin=false]
+ * @param {boolean} [options.suppressGC=false]
  */
 const testUpgrade = async (t, defaultManagerType, options = {}) => {
-  const { restartVatAdmin: doVatAdminRestart = false } = options;
+  const { restartVatAdmin: doVatAdminRestart = false, suppressGC = false } =
+    options;
   const config = makeConfigFromPaths('bootstrap-scripted-upgrade.js', {
     defaultManagerType,
     bundlePaths: {
@@ -192,12 +198,26 @@ const testUpgrade = async (t, defaultManagerType, options = {}) => {
   });
   /** @type {object} */
   const { data: bundleData } = t.context;
+  if (suppressGC) {
+    config.defaultReapInterval = 'never';
+    // eslint-disable-next-line @typescript-eslint/prefer-ts-expect-error
+    // @ts-ignore reapInterval is valid
+    config.vats.bootstrap.reapInterval = 1;
+  }
   const {
     controller: c,
     kvStore,
     run,
-  } = await initKernelForTest(t, bundleData, config);
+    runAndRetain,
+  } = await initKernelForTest(t, bundleData, config, {
+    holdObjectRefs: false,
+  });
 
+  /**
+   * @param {object} presence
+   * @param {string} [iface]
+   * @returns {string} kref
+   */
   const verifyPresence = (presence, iface = undefined) => {
     const kref = krefOf(presence);
     t.truthy(kref);
@@ -207,14 +227,15 @@ const testUpgrade = async (t, defaultManagerType, options = {}) => {
     return kref;
   };
 
-  const marker = await run('getMarker');
+  const marker = await runAndRetain('getMarker');
   const markerKref = verifyPresence(marker, 'marker'); // probably 'ko26'
 
   // fetch all the "import sensors": exported by bootstrap, imported by
   // the upgraded vat. We'll determine their krefs and later query the
   // upgraded vat to see if it's still importing them or not
-  const sensors = await run('getImportSensors', []);
-  // eslint-disable-next-line no-unused-vars
+  const sensors = /** @type {[unknown, ...object]} */ (
+    await runAndRetain('getImportSensors', [])
+  );
   const sensorKrefs = [
     'skip0',
     ...sensors.slice(1).map(obj => verifyPresence(obj)),
@@ -225,6 +246,7 @@ const testUpgrade = async (t, defaultManagerType, options = {}) => {
   }
 
   // create initial version
+  /** @type {any} */
   const v1result = await run('buildV1', []);
   t.like(v1result, {
     version: 'v1',
@@ -238,19 +260,19 @@ const testUpgrade = async (t, defaultManagerType, options = {}) => {
   const v1p2Kref = verifyPresence(v1result.p2);
   // grab krefs for the exported durable/virtual objects to check their abandonment
   const retainedKrefs = objectMap(v1result.retain, obj => verifyPresence(obj));
-  const retainedNames = 'dur1 vir2 vir5 vir7 vc1 vc3 dc4 rem1 rem2 rem3';
-  t.deepEqual(
-    Object.keys(retainedKrefs).sort(),
-    retainedNames.split(' ').sort(),
+  const retainedNames = 'dur1 vir2 vir5 vir7 vc1 vc3 dc4 rem1 rem2 rem3'.split(
+    ' ',
   );
+  t.deepEqual(Object.keys(retainedKrefs).sort(), retainedNames.sort());
   const { dur1: dur1Kref, vir2: vir2Kref } = retainedKrefs;
 
   // use the kvStore to support assertions about exported durable/virtual vrefs
   const vatID = kvStore.get(`${dur1Kref}.owner`); // probably 'v6'
   // dumpState(debug, vatID);
+  const getCListEntry = kref => kvStore.get(`${vatID}.c.${kref}`);
   const getVref = kref => {
-    const s = kvStore.get(`${vatID}.c.${kref}`);
-    return parseReachableAndVatSlot(s).vatSlot;
+    const entry = getCListEntry(kref);
+    return parseReachableAndVatSlot(entry).vatSlot;
   };
   const vomHas = vref => kvStore.has(`${vatID}.vs.vom.${vref}`);
   const dur1Vref = getVref(dur1Kref);
@@ -261,17 +283,57 @@ const testUpgrade = async (t, defaultManagerType, options = {}) => {
   t.is(parseVatSlot(vir2Vref).subid, 2n);
   const virPrefix = vir2Vref.slice(0, -1); // everything before the trailing '2'
   const virVref = i => `${virPrefix}${i}`;
-
-  t.true(vomHas(durVref(1)));
-  t.true(vomHas(virVref(2)));
-  t.false(vomHas(virVref(1))); // GCed before upgrade
-  t.false(vomHas(durVref(2))); // GCed before upgrade
+  const collectableVrefs = [virVref(1), durVref(2), virVref(39), durVref(39)];
+  /**
+   * @param {string} when
+   * @param {object} expectations
+   * @param {boolean} expectations.afterGC
+   * @param {string[]} [expectations.stillOwned=retainedNames]
+   */
+  const verifyObjectTracking = (when, expectations) => {
+    const { afterGC, stillOwned = retainedNames } = expectations;
+    // imported object reachability
+    for (let i = 1; i < sensorKrefs.length; i += 1) {
+      const entry = getCListEntry(sensorKrefs[i]);
+      if (i === 39 && afterGC) {
+        // import-39 is ignored
+        t.is(entry, undefined, `import-39 is retired ${when}`);
+        continue;
+      }
+      const { isReachable } = parseReachableAndVatSlot(entry);
+      if (i === 32) {
+        // import-32 is a WeakMap key that is initialized in v1 and
+        // probed in v2 by bootstrap-scripted-upgrade.js "upgradeV2";
+        // it should be reachable iff GC is suppressed
+        t.is(isReachable, suppressGC, `import-32 reachability ${when}`);
+      } else {
+        // reachability of all other imports should be retained
+        // by object references
+        t.is(isReachable, true, `import-${i} reachability ${when}`);
+      }
+    }
+    // exported object ownership
+    for (const name of retainedNames) {
+      const owner = kvStore.get(`${retainedKrefs[name]}.owner`);
+      const expectedOwner = stillOwned.includes(name) ? vatID : undefined;
+      t.is(owner, expectedOwner, `${name} ownership ${when}`);
+    }
+    // liveslots vom tracking
+    t.true(vomHas(durVref(1)));
+    t.true(vomHas(virVref(2)));
+    for (const vref of collectableVrefs) {
+      const present = vomHas(vref);
+      t.is(present, !afterGC, `${vref} must be collected by GC ${when}`);
+    }
+  };
 
   if (doVatAdminRestart) {
     await restartVatAdminVat(c);
   }
+  verifyObjectTracking('before upgrade', { afterGC: !suppressGC });
 
   // now perform the upgrade
+  /** @type {any} */
   const v2result = await run('upgradeV2', []);
   // dumpState(debug, vatID);
   t.like(v2result, {
@@ -305,73 +367,17 @@ const testUpgrade = async (t, defaultManagerType, options = {}) => {
   t.is(c.kpStatus(v1p2Kref), 'rejected');
   t.deepEqual(kunser(c.kpResolution(v1p2Kref)), vatUpgradedError);
 
-  // all the merely-virtual exports should be gone
-  // for (let i = 1; i < sensors.length; i += 1) {
-  //   t.false(vomHas(virVref(i)));
-  // }
-
-  /* Disabling this portion of the test as it is irrelevant and non-working so
-     long as non-durable object cleanup in stop-vat is also disabled.
-
-  // of the durables, only these survive
-  const survivingDurables = [
-    1, 16, 17, 18, 19, 20, 26, 27, 28, 33, 34, 35, 36, 37,
-  ];
-  // and these imports (imp38 is held by baggage)
-  const survivingImported = [
-    1, 16, 17, 18, 19, 20, 26, 27, 28, 33, 34, 35, 36, 37, 38,
-  ];
-
-  // but implementation limitations/bugs cause the following unwanted
-  // effects (these adjustments should be deleted as we fix them):
-
-  // stopVat() uses deleteVirtualObjectsWithoutDecref, rather than
-  // deleteVirtualObjectsWithDecref, which means lingering virtual
-  // objects (i.e. cycles) don't drop their referenced objects as we
-  // delete them
-  survivingDurables.push(9);
-  survivingImported.push(7);
-  survivingImported.push(8);
-  survivingImported.push(9);
-
-  // When a virtual collection is deleted, the loop that deletes all
-  // entries will re-instantiate all the keys, but doesn't set
-  // doMoreGC, so processDeadSet doesn't redo the gcAndFinalize, and
-  // the virtual object cache is probably still holding onto the new
-  // Representative anyways. This retains the durables that were held
-  // by deleted collections (dur10/dur13/dur23, depending on the cache
-  // size, just dur23 if size=0) and the imports they hold. Bug #5053
-  // is about fixing clearInternal to avoid this, when that's fixed
-  // these should be removed.
-  survivingDurables.push(10);
-  survivingImported.push(10);
-  survivingDurables.push(13);
-  survivingImported.push(13);
-  survivingDurables.push(23);
-  survivingImported.push(23);
-
-  for (let i = 1; i < sensors.length; i += 1) {
-    const vref = durVref(i);
-    // const impKref = sensorKrefs[i];
-    const expD = survivingDurables.includes(i);
-    // const expI = survivingImported.includes(i);
-    // const reachable = krefReachable(impKref);
-    t.is(vomHas(vref), expD, `dur[${i}] not ${expD}`);
-    // t.is(reachable, expI, `imp[${i}] not ${expI}`);
-    // const abb = (b) => b.toString().slice(0,1).toUpperCase();
-    // const vomS = `vom: ${abb(expD)} ${abb(vomHas(vref))}`;
-    // const reachS = `${abb(expI)} ${abb(reachable)}`;
-    // const match = (expD === vomHas(vref)) && (expI === reachable);
-    // const matchS = `${match ? 'true' : 'FALSE'}`;
-    // const s = kvStore.get(`${vatID}.c.${impKref}`);
-    // console.log(`${i}: ${vomS} imp: ${reachS} ${matchS}  ${impKref} ${s}`);
-  }
-  */
-
-  // check koNN.owner to confirm the exported virtuals (2/5/7) are abandoned
-  t.false(kvStore.has(`${vir2Kref}.owner`));
-  t.false(kvStore.has(`${v1result.retain.vir5}.owner`));
-  t.false(kvStore.has(`${v1result.retain.vir7}.owner`));
+  // verify export abandonment/garbage collection/etc.
+  // This used to be MUCH more extensive, but GC was cut to the bone
+  // in commits like 91480dee8e48ae26c39c420febf73b93deba6ea5
+  // basically reverting 1cfbeaa3c925d0f8502edfb313ecb12a1cab5eac
+  // (see also #5342 and #6650).
+  // It can be restored once we add back correct sophisticated logic
+  // by e.g. having liveslots sweep the database when restoring a vat.
+  verifyObjectTracking('after upgrade', {
+    afterGC: true,
+    stillOwned: ['dur1', 'dc4'],
+  });
 };
 
 test('vat upgrade - local', async t => {
@@ -382,10 +388,18 @@ test('vat upgrade - local with VA restarts', async t => {
   return testUpgrade(t, 'local', { restartVatAdmin: true });
 });
 
+test('vat upgrade - local without automatic GC', async t => {
+  return testUpgrade(t, 'local', { suppressGC: true });
+});
+
 test('vat upgrade - xsnap', async t => {
   return testUpgrade(t, 'xs-worker');
 });
 
+test('vat upgrade - xsnap without automatic GC', async t => {
+  return testUpgrade(t, 'xs-worker', { suppressGC: true });
+});
+
 test('vat upgrade - omit vatParameters', async t => {
   const config = makeConfigFromPaths('bootstrap-scripted-upgrade.js', {
     defaultManagerType: 'xs-worker',
diff --git a/packages/SwingSet/test/upgrade/vat-ulrik-1.js b/packages/SwingSet/test/upgrade/vat-ulrik-1.js
index e7785de2ce8..f410828f29b 100644
--- a/packages/SwingSet/test/upgrade/vat-ulrik-1.js
+++ b/packages/SwingSet/test/upgrade/vat-ulrik-1.js
@@ -108,7 +108,7 @@ const buildExports = (baggage, imp) => {
   dc3.init(imp[25], dur[25]);
   dc4.init(dur[26], dur[27]);
   dc4.init(imp[28], dur[28]);
-  dc5.init(imp[32], dur[33]); // imp[32] exported+held by bootstrap
+  dc5.init(imp[32], dur[33]); // imp[32] is recognizable but not reachable
   dc6.init(dur[34], dur[35]);
   dc6.init(imp[36], dur[36]);
   baggage.init('dc5', dc5);

From 03f8518286c26c505af80d875da72fec7c1641e8 Mon Sep 17 00:00:00 2001
From: Richard Gibson <richard.gibson@gmail.com>
Date: Fri, 24 Mar 2023 23:44:10 -0400
Subject: [PATCH 03/12] refactor(SwingSet): Use natural sort in kernel.dump()
 output ordering

---
 packages/SwingSet/src/kernel/state/kernelKeeper.js | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/packages/SwingSet/src/kernel/state/kernelKeeper.js b/packages/SwingSet/src/kernel/state/kernelKeeper.js
index 791be925754..e7e811f973d 100644
--- a/packages/SwingSet/src/kernel/state/kernelKeeper.js
+++ b/packages/SwingSet/src/kernel/state/kernelKeeper.js
@@ -1483,6 +1483,17 @@ export default function makeKernelKeeper(kernelStorage, kernelSlog) {
     }
 
     function compareStrings(a, b) {
+      // natural-sort strings having a shared prefix followed by digits
+      // (e.g., 'ko42' and 'ko100')
+      const [_a, aPrefix, aDigits] = /^(\D+)(\d+)$/.exec(a) || [];
+      if (aPrefix) {
+        const [_b, bPrefix, bDigits] = /^(\D+)(\d+)$/.exec(b) || [];
+        if (bPrefix === aPrefix) {
+          return compareNumbers(aDigits, bDigits);
+        }
+      }
+
+      // otherwise use the default string ordering
       if (a > b) {
         return 1;
       }

From 95e6215d7841030b46655642ec39f96dffdcad02 Mon Sep 17 00:00:00 2001
From: Richard Gibson <richard.gibson@gmail.com>
Date: Sat, 25 Mar 2023 14:21:20 -0400
Subject: [PATCH 04/12] test(SwingSet): Add coverage for kernel delivery of
 vat-upgrade BOYD

---
 packages/SwingSet/test/bootstrap-relay.js     |   4 +-
 .../SwingSet/test/upgrade/test-upgrade.js     | 125 +++++++++++++++++-
 packages/SwingSet/test/vat-exporter.js        |   2 +-
 3 files changed, 125 insertions(+), 6 deletions(-)

diff --git a/packages/SwingSet/test/bootstrap-relay.js b/packages/SwingSet/test/bootstrap-relay.js
index 284ac9fdf9a..509b2a41b6c 100644
--- a/packages/SwingSet/test/bootstrap-relay.js
+++ b/packages/SwingSet/test/bootstrap-relay.js
@@ -36,10 +36,10 @@ export const buildRootObject = () => {
 
     getTimer: async () => encodePassable(timer),
 
-    getVatRoot: async ({ name }) => {
+    getVatRoot: async ({ name, rawOutput = false }) => {
       const vat = vatData.get(name) || Fail`unknown vat name: ${q(name)}`;
       const { root } = vat;
-      return encodePassable(root);
+      return rawOutput ? root : encodePassable(root);
     },
 
     createVat: async ({ name, bundleCapName, vatParameters = {} }) => {
diff --git a/packages/SwingSet/test/upgrade/test-upgrade.js b/packages/SwingSet/test/upgrade/test-upgrade.js
index 62c56d4f5d7..f63b0ad9439 100644
--- a/packages/SwingSet/test/upgrade/test-upgrade.js
+++ b/packages/SwingSet/test/upgrade/test-upgrade.js
@@ -9,7 +9,7 @@ import { objectMap } from '@agoric/internal';
 import { initSwingStore } from '@agoric/swing-store';
 import { parseReachableAndVatSlot } from '../../src/kernel/state/reachable.js';
 import { parseVatSlot } from '../../src/lib/parseVatSlots.js';
-import { kunser, krefOf } from '../../src/lib/kmarshal.js';
+import { kser, kunser, krefOf } from '../../src/lib/kmarshal.js';
 import {
   buildKernelBundles,
   initializeSwingset,
@@ -76,6 +76,7 @@ const makeConfigFromPaths = (bootstrapVatPath, options = {}) => {
  * @param {object} bundleData
  * @param {SwingSetConfig} config
  * @param {object} [options]
+ * @param {object} [options.extraRuntimeOpts]
  * @param {boolean} [options.holdObjectRefs=true] - DEPRECATED -
  * Refcount incrementing should be manual,
  * see https://github.com/Agoric/agoric-sdk/issues/7213
@@ -89,8 +90,8 @@ const makeConfigFromPaths = (bootstrapVatPath, options = {}) => {
 const initKernelForTest = async (t, bundleData, config, options = {}) => {
   const { kernelStorage } = initSwingStore();
   const { kvStore } = kernelStorage;
-  const { initOpts, runtimeOpts } = bundleOpts(bundleData);
-  const { holdObjectRefs = true } = options;
+  const { extraRuntimeOpts, holdObjectRefs = true } = options;
+  const { initOpts, runtimeOpts } = bundleOpts(bundleData, extraRuntimeOpts);
   await initializeSwingset(config, [], kernelStorage, initOpts);
   const c = await makeSwingsetController(kernelStorage, {}, runtimeOpts);
   t.teardown(c.shutdown);
@@ -179,6 +180,124 @@ test('null upgrade - xsnap', async t => {
   return testNullUpgrade(t, 'xs-worker');
 });
 
+test('kernel sends bringOutYourDead for vat upgrade', async t => {
+  const config = makeConfigFromPaths('../bootstrap-relay.js', {
+    defaultReapInterval: 'never',
+    staticVatPaths: {
+      staticVat: '../vat-exporter.js',
+    },
+    bundlePaths: {
+      exporter: '../vat-exporter.js',
+    },
+  });
+  let isSlogging = false;
+  const deliveries = [];
+  const deliverySpy = slogEntry => {
+    if (isSlogging && slogEntry.type === 'deliver') {
+      deliveries.push(slogEntry);
+    }
+  };
+  const { controller, kvStore, run } = await initKernelForTest(
+    t,
+    t.context.data,
+    config,
+    { extraRuntimeOpts: { slogSender: deliverySpy } },
+  );
+
+  // ava t.like does not support array shapes, but object analogs are fine
+  const arrayShape = sparseArr => Object.fromEntries(Object.entries(sparseArr));
+  const capDataShape = obj => {
+    const capData = kser(obj);
+    const { body, ..._slotsEtc } = capData;
+    return { body };
+  };
+  // Kernel deliveries are [type, ...args] arrays.
+  const deliveryShape = (deliveryNum, kdShape) => {
+    return { deliveryNum, kd: arrayShape(kdShape) };
+  };
+  const messageDeliveryShape = (deliveryNum, method, args, resultKpid) => {
+    const methargs = capDataShape([method, args]);
+    const messageShape = resultKpid
+      ? { methargs, result: resultKpid }
+      : { methargs };
+    // sparse array to ignore target kref
+    // eslint-disable-next-line no-sparse-arrays
+    return deliveryShape(deliveryNum, ['message', , messageShape]);
+  };
+
+  // Null-upgrade the static vat with some messages before and after
+  // to catch any unexpected BOYD.
+  const staticVatID = controller.vatNameToID('staticVat');
+  const bundle = await bundleSource(config.vats.staticVat.sourceSpec);
+  const bundleID = await controller.validateAndInstallBundle(bundle);
+  isSlogging = true;
+  const staticVatV1 = await run('getVersion', [], 'staticVat');
+  t.is(staticVatV1, undefined);
+  const staticVatUpgradeKpid = controller.upgradeStaticVat(
+    'staticVat',
+    false,
+    bundleID,
+    {
+      vatParameters: { version: 'v2' },
+    },
+  );
+  await controller.run();
+  t.is(controller.kpStatus(staticVatUpgradeKpid), 'fulfilled');
+  const staticVatV2 = await run('getVersion', [], 'staticVat');
+  t.is(staticVatV2, 'v2');
+  isSlogging = false;
+
+  const staticVatDeliveries = deliveries.filter(
+    slogEntry => slogEntry.vatID === staticVatID,
+  );
+  const expectedDeliveries = [
+    messageDeliveryShape(1n, 'getVersion', []),
+    deliveryShape(2n, ['stopVat']),
+    deliveryShape(3n, ['bringOutYourDead']),
+    deliveryShape(4n, ['startVat']),
+    messageDeliveryShape(5n, 'getVersion', []),
+  ];
+  t.like(staticVatDeliveries, arrayShape(expectedDeliveries));
+  t.is(staticVatDeliveries.length, expectedDeliveries.length);
+
+  // Repeat the process with a dynamic vat.
+  await run('createVat', [
+    {
+      name: 'dynamicVat',
+      bundleCapName: 'exporter',
+      vatParameters: { version: 'v1' },
+    },
+  ]);
+  const dynamicVat = await run('getVatRoot', [
+    { name: 'dynamicVat', rawOutput: true },
+  ]);
+  const dynamicVatKref = krefOf(dynamicVat);
+  const dynamicVatID = kvStore.get(`${dynamicVatKref}.owner`); // probably 'v7'
+  isSlogging = true;
+  const dynamicVatV1 = await run('messageVat', [
+    { name: 'dynamicVat', methodName: 'getVersion' },
+  ]);
+  t.is(dynamicVatV1, 'v1');
+  await run('upgradeVat', [
+    {
+      name: 'dynamicVat',
+      bundleCapName: 'exporter',
+      vatParameters: { version: 'v2' },
+    },
+  ]);
+  const dynamicVatV2 = await run('messageVat', [
+    { name: 'dynamicVat', methodName: 'getVersion' },
+  ]);
+  t.is(dynamicVatV2, 'v2');
+  isSlogging = false;
+
+  const dynamicVatDeliveries = deliveries.filter(
+    slogEntry => slogEntry.vatID === dynamicVatID,
+  );
+  t.like(dynamicVatDeliveries, arrayShape(expectedDeliveries));
+  t.is(dynamicVatDeliveries.length, expectedDeliveries.length);
+});
+
 /**
  * @param {import('ava').ExecutionContext} t
  * @param {ManagerType} defaultManagerType
diff --git a/packages/SwingSet/test/vat-exporter.js b/packages/SwingSet/test/vat-exporter.js
index 316400c050a..cba954b0d76 100644
--- a/packages/SwingSet/test/vat-exporter.js
+++ b/packages/SwingSet/test/vat-exporter.js
@@ -7,7 +7,7 @@ import {
 } from '@agoric/vat-data';
 
 export const buildRootObject = (_vatPowers, vatParameters, baggage) => {
-  const { version } = vatParameters;
+  const { version } = vatParameters || {};
 
   // Define a family of analogous simple ephemeral/virtual/durable classes.
   const CounterI = M.interface('Counter', {

From be7358869fa7dbefa4b21795b06dd0b8fc68200c Mon Sep 17 00:00:00 2001
From: Richard Gibson <richard.gibson@gmail.com>
Date: Fri, 24 Mar 2023 23:46:08 -0400
Subject: [PATCH 05/12] fix(SwingSet): Add BOYD to vat upgrade

Fixes #7001
---
 packages/SwingSet/src/kernel/kernel.js | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/packages/SwingSet/src/kernel/kernel.js b/packages/SwingSet/src/kernel/kernel.js
index bde8b0cf3df..ed33811596e 100644
--- a/packages/SwingSet/src/kernel/kernel.js
+++ b/packages/SwingSet/src/kernel/kernel.js
@@ -802,7 +802,6 @@ export default function buildKernel(
   }
 
   /**
-   *
    * @param {RunQueueEventUpgradeVat} message
    * @returns {Promise<CrankResults>}
    */
@@ -887,10 +886,16 @@ export default function buildKernel(
 
     // stopVat succeeded, now we finish cleanup on behalf of the worker
 
-    // TODO: send BOYD so the terminating vat has one last chance to clean
+    // send BOYD so the terminating vat has one last chance to clean
     // up, drop imports, and delete durable data.
-    // If a vat is so broken it can't do BOYD, we can make that optional.
-    // https://github.com/Agoric/agoric-sdk/issues/7001
+    // If a vat is so broken it can't do BOYD, we can make this optional.
+    /** @type { import('../types-external.js').KernelDeliveryBringOutYourDead } */
+    const boydKD = harden(['bringOutYourDead']);
+    const boydVD = vatWarehouse.kernelDeliveryToVatDelivery(vatID, boydKD);
+    const boydStatus = await deliverAndLogToVat(vatID, boydKD, boydVD);
+    const boydResults = deliveryCrankResults(vatID, boydStatus, false);
+    (!boydResults.abort && !boydResults.terminate) ||
+      Fail`Unexpected abort/terminate result from upgrade-internal bringOutYourDead: ${boydResults}`;
 
     // reject all promises for which the vat was decider
     for (const kpid of kernelKeeper.enumeratePromisesByDecider(vatID)) {

From 61d235f1c516479e71d6720dd9ac4b76160e929a Mon Sep 17 00:00:00 2001
From: Richard Gibson <richard.gibson@gmail.com>
Date: Sat, 25 Mar 2023 14:22:54 -0400
Subject: [PATCH 06/12] test(SwingSet): Use new options to simplify a test

---
 packages/SwingSet/test/upgrade/test-upgrade.js | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/packages/SwingSet/test/upgrade/test-upgrade.js b/packages/SwingSet/test/upgrade/test-upgrade.js
index f63b0ad9439..73dbe3c8cb7 100644
--- a/packages/SwingSet/test/upgrade/test-upgrade.js
+++ b/packages/SwingSet/test/upgrade/test-upgrade.js
@@ -689,9 +689,8 @@ test('non-durable exports are abandoned by upgrade of non-liveslots vat', async
 
   // Export two objects from exporter to observer,
   // one to be held strongly and the other weakly.
-  const observerPresence = await run('getVatRoot', [{ name: 'observer' }]);
-  const observer = await run('awaitVatObject', [
-    { presence: observerPresence, rawOutput: true },
+  const observer = await run('getVatRoot', [
+    { name: 'observer', rawOutput: true },
   ]);
   const strongObj = await run('exportFakeObject', [], 'exporter');
   await run(

From 5cc47d2d8892690f8c1653630b41dd64cc42d73b Mon Sep 17 00:00:00 2001
From: Richard Gibson <richard.gibson@gmail.com>
Date: Sat, 25 Mar 2023 15:15:46 -0400
Subject: [PATCH 07/12] fix(SwingSet): Don't send stopVat during upgrade

Fixes #6650
---
 packages/SwingSet/docs/garbage-collection.md  |   4 +-
 packages/SwingSet/src/kernel/kernel.js        |  64 +--
 .../SwingSet/test/upgrade/test-upgrade.js     |  10 +-
 packages/swingset-liveslots/src/liveslots.js  |  40 +-
 packages/swingset-liveslots/src/stop-vat.js   | 395 ------------------
 5 files changed, 29 insertions(+), 484 deletions(-)
 delete mode 100644 packages/swingset-liveslots/src/stop-vat.js

diff --git a/packages/SwingSet/docs/garbage-collection.md b/packages/SwingSet/docs/garbage-collection.md
index 3dc8d7e980c..8208d5487bc 100644
--- a/packages/SwingSet/docs/garbage-collection.md
+++ b/packages/SwingSet/docs/garbage-collection.md
@@ -360,9 +360,7 @@ If the last importing vat had previously called `syscall.retireImport`, there wi
 
 ## syscall.abandonExport processing
 
-During vat upgrade, the last delivery made to the old version is a special `dispatch.stopVat()`. This instructs the soon-to-be-deleted worker to destroy anything that will not survive the upgrade. All Remotables will be abandoned here, because they live solely in RAM, and the RAM heap does not survive. All non-durable virtual objects will also be abandoned, since only durable ones survive. We may also drop otherwise-durable objects which were only referenced by abandoned non-durable objects.
-
-During this phase, the vat performs a `syscall.abandonExports()` with all the abandoned vrefs. The kernel reacts to this in roughly the same way it reacts to the entire vat being terminated (which it is, sort of, at least the heap is being terminated). Each vref is deleted from the exporting vat's c-list, because *that* vat isn't going to be referencing it any more. The kernel object table is updated to clear the `owner` field: while the kernel object retains its identity, it is now orphaned, and any messages sent to it will be rejected (by the kernel) with a "vat terminated" error.
+Although not currently used (see [#6650](https://github.com/Agoric/agoric-sdk/issues/6650)), the kernel still supports vat issuance of `syscall.abandonExports()` with vrefs to be abandoned as part of vat upgrade (i.e., Remotables that live solely in the RAM heap and non-durable merely-virtual objects, and ideally also otherwise-durable objects not stored in baggage and referenced only by other abandoned objects). The kernel reacts to this in roughly the same way it reacts to the entire vat being terminated (which it is, sort of, at least the heap is being terminated). Each vref is deleted from the exporting vat's c-list, because *that* vat isn't going to be referencing it any more. The kernel object table is updated to clear the `owner` field: while the kernel object retains its identity, it is now orphaned, and any messages sent to it will be rejected (by the kernel) with a "vat terminated" error.
 
 No other work needs to be done. Any importing vats will continue to hold their reference as before. They can only tell that the object has been abandoned if they try to send it a message. Eventually, if all the importing vats drop their reference, and nothing else in the kernel is holding one, the kernel object entry will be deleted. In this case, no `dispatch.retireExports` is sent to the old exporting vat, since it's already been removed from their c-list.
 
diff --git a/packages/SwingSet/src/kernel/kernel.js b/packages/SwingSet/src/kernel/kernel.js
index ed33811596e..1b03e063284 100644
--- a/packages/SwingSet/src/kernel/kernel.js
+++ b/packages/SwingSet/src/kernel/kernel.js
@@ -824,21 +824,6 @@ export default function buildKernel(
       incarnationNumber: vatKeeper.getIncarnationNumber(),
     };
     const disconnectionCapData = kser(disconnectObject);
-    /** @type { import('../types-external.js').KernelDeliveryStopVat } */
-    const stopVatKD = harden(['stopVat', disconnectionCapData]);
-    const stopVatVD = vatWarehouse.kernelDeliveryToVatDelivery(
-      vatID,
-      stopVatKD,
-    );
-    const stopVatStatus = await deliverAndLogToVat(vatID, stopVatKD, stopVatVD);
-    const stopVatResults = deliveryCrankResults(vatID, stopVatStatus, false);
-
-    // We don't meter stopVat, since no user code is running, but we
-    // still report computrons to the runPolicy
-    let { computrons } = stopVatResults; // BigInt or undefined
-    if (computrons !== undefined) {
-      assert.typeof(computrons, 'bigint');
-    }
 
     /**
      * Make a method-arguments structure representing failure
@@ -855,36 +840,16 @@ export default function buildKernel(
       return ['vatUpgradeCallback', [upgradeID, false, error]];
     };
 
-    // TODO: if/when we implement vat pause/suspend, and if
-    // deliveryCrankResults changes to not use .terminate to indicate
-    // a problem, this should change to match: where we would normally
-    // pause/suspend a vat for a delivery error, here we want to
-    // unwind the upgrade.
-
-    if (stopVatResults.terminate) {
-      // get rid of the worker, so the next delivery to this vat will
-      // re-create one from the previous state
-      // eslint-disable-next-line @jessie.js/no-nested-await
-      await vatWarehouse.stopWorker(vatID);
-
-      // notify vat-admin of the failed upgrade
-      const vatAdminMethargs = makeFailureMethargs(
-        stopVatResults.terminate.info,
-      );
-
-      // we still report computrons to the runPolicy
-      const results = harden({
-        ...stopVatResults,
-        computrons,
-        abort: true, // always unwind
-        consumeMessage: true, // don't repeat the upgrade
-        terminate: undefined, // do *not* terminate the vat
-        vatAdminMethargs,
-      });
-      return results;
-    }
-
-    // stopVat succeeded, now we finish cleanup on behalf of the worker
+    // cleanup on behalf of the worker
+    // This used to be handled by a stopVat delivery to the vat itself,
+    // but the implementation of that was cut to the bone
+    // in commits like 91480dee8e48ae26c39c420febf73b93deba6ea5
+    // basically reverting 1cfbeaa3c925d0f8502edfb313ecb12a1cab5eac
+    // and then ultimately moved to the kernel in a MUCH diminished form
+    // (see #5342 and #6650, and testUpgrade in
+    // {@link ../../test/upgrade/test-upgrade.js}).
+    // We hope to eventually add back correct sophisticated logic
+    // by e.g. having liveslots sweep the database when restoring a vat.
 
     // send BOYD so the terminating vat has one last chance to clean
     // up, drop imports, and delete durable data.
@@ -897,6 +862,15 @@ export default function buildKernel(
     (!boydResults.abort && !boydResults.terminate) ||
       Fail`Unexpected abort/terminate result from upgrade-internal bringOutYourDead: ${boydResults}`;
 
+    // we don't meter bringOutYourDead since no user code is running, but we
+    // still report computrons to the runPolicy
+    let { computrons } = boydResults;
+    computrons === undefined || assert.typeof(computrons, 'bigint');
+
+    // TODO: if/when we implement vat pause/suspend, and if
+    // deliveryCrankResults changes to not use .terminate to indicate
+    // a problem, we might want to unwind the upgrade here.
+
     // reject all promises for which the vat was decider
     for (const kpid of kernelKeeper.enumeratePromisesByDecider(vatID)) {
       resolveToError(kpid, disconnectionCapData, vatID);
diff --git a/packages/SwingSet/test/upgrade/test-upgrade.js b/packages/SwingSet/test/upgrade/test-upgrade.js
index 73dbe3c8cb7..1b92d58f13f 100644
--- a/packages/SwingSet/test/upgrade/test-upgrade.js
+++ b/packages/SwingSet/test/upgrade/test-upgrade.js
@@ -252,10 +252,9 @@ test('kernel sends bringOutYourDead for vat upgrade', async t => {
   );
   const expectedDeliveries = [
     messageDeliveryShape(1n, 'getVersion', []),
-    deliveryShape(2n, ['stopVat']),
-    deliveryShape(3n, ['bringOutYourDead']),
-    deliveryShape(4n, ['startVat']),
-    messageDeliveryShape(5n, 'getVersion', []),
+    deliveryShape(2n, ['bringOutYourDead']),
+    deliveryShape(3n, ['startVat']),
+    messageDeliveryShape(4n, 'getVersion', []),
   ];
   t.like(staticVatDeliveries, arrayShape(expectedDeliveries));
   t.is(staticVatDeliveries.length, expectedDeliveries.length);
@@ -752,7 +751,8 @@ test('non-durable exports are abandoned by upgrade of non-liveslots vat', async
   // const observerLog = await run('getDispatchLog', [], 'observer');
 });
 
-test('failed upgrade - relaxed durable rules', async t => {
+// No longer valid as of removing stopVat per #6650
+test.failing('failed upgrade - relaxed durable rules', async t => {
   const config = makeConfigFromPaths('bootstrap-scripted-upgrade.js', {
     relaxDurabilityRules: true,
     bundlePaths: {
diff --git a/packages/swingset-liveslots/src/liveslots.js b/packages/swingset-liveslots/src/liveslots.js
index a26424ffe95..46af39c7dcc 100644
--- a/packages/swingset-liveslots/src/liveslots.js
+++ b/packages/swingset-liveslots/src/liveslots.js
@@ -16,7 +16,6 @@ import { makeVirtualReferenceManager } from './virtualReferences.js';
 import { makeVirtualObjectManager } from './virtualObjectManager.js';
 import { makeCollectionManager } from './collectionManager.js';
 import { makeWatchedPromiseManager } from './watchedPromises.js';
-import { releaseOldState } from './stop-vat.js';
 
 const DEFAULT_VIRTUAL_OBJECT_CACHE_SIZE = 3; // XXX ridiculously small value to force churn for testing
 
@@ -66,7 +65,7 @@ function build(
   }
 
   let didStartVat = false;
-  let didStopVat = false;
+  const didStopVat = false;
 
   const outstandingProxies = new WeakSet();
 
@@ -1516,42 +1515,11 @@ function build(
   }
 
   /**
-   * @param { import('./types').SwingSetCapData } disconnectObjectCapData
+   * @param { import('./types').SwingSetCapData } _disconnectObjectCapData
    * @returns {Promise<void>}
    */
-  async function stopVat(disconnectObjectCapData) {
-    insistCapData(disconnectObjectCapData);
-    assert(disconnectObjectCapData.slots.length === 0);
-    assert(
-      !relaxDurabilityRules,
-      'stopVat not available when relaxDurabilityRules is true',
-    );
-
-    assert(didStartVat);
-    assert(!didStopVat);
-    didStopVat = true;
-
-    try {
-      // eslint-disable-next-line @jessie.js/no-nested-await
-      await releaseOldState({
-        m,
-        disconnectObjectCapData,
-        syscall,
-        exportedRemotables,
-        addToPossiblyDeadSet,
-        slotToVal,
-        valToSlot,
-        dropExports,
-        retireExports,
-        vrm,
-        collectionManager,
-        bringOutYourDead,
-        vreffedObjectRegistry,
-      });
-    } catch (e) {
-      console.warn(`-- error during stopVat()`, e);
-      throw e;
-    }
+  async function stopVat(_disconnectObjectCapData) {
+    Fail`stopVat is no longer supported as of #6650`;
   }
 
   /**
diff --git a/packages/swingset-liveslots/src/stop-vat.js b/packages/swingset-liveslots/src/stop-vat.js
deleted file mode 100644
index 2784a0e81da..00000000000
--- a/packages/swingset-liveslots/src/stop-vat.js
+++ /dev/null
@@ -1,395 +0,0 @@
-import { makeVatSlot, parseVatSlot } from './parseVatSlots.js';
-import { enumerateKeysWithPrefix } from './vatstore-iterators.js';
-
-// This file has tools to run during the last delivery of the old vat
-// version, `dispatch.stopVat()`, just before an upgrade. It is
-// responsible for deleting as much of the non-retained data as
-// possible. The primary function is `releaseOldState()`, at the end;
-// everything else is a helper function.
-
-// The only data that should be retained are those durable objects and
-// collections which are transitively reachable from two sets of
-// roots: the durable objects exported to other vats, and the
-// "baggage" collection. All other durable objects, and *all*
-// merely-virtual objects (regardless of export status) should be
-// deleted. All imports which were kept alive by dropped object should
-// also be dropped.
-
-// However, the possibility of cycles within durable storage means
-// that a full cleanup requires a mark+sweep pass through all durable
-// objects, which I think is too expensive for right now.
-
-// So instead, I'm going to settle on a cheaper `stopVat` which
-// correctly drops durable objects and imports that were only kept
-// alive by 1: RAM, 2: non-durable exports, or 3: non-durable
-// objects/collections. It will require a walk through all non-durable
-// objects and collections, but not a mark+sweep through all durable
-// objects.
-
-// This cheaper form may leak some imports and storage, but should not
-// allow the new vat to access anything it shouldn't, nor allow other
-// vats to cause confusion in the new version (by referencing exports
-// which the vat no longer remembers).
-
-const rootSlot = makeVatSlot('object', true, 0n);
-
-// eslint-disable-next-line no-unused-vars
-function identifyExportedRemotables(
-  vrefSet,
-  { exportedRemotables, valToSlot },
-) {
-  // Find all exported "Remotables": precious in-RAM objects declared
-  // with Far and sent as message argument or promise resolution.
-  // These are all doomed, except for the root object (which the new
-  // version will rebind). We'll pretend the kernel drops these in a
-  // minute.
-
-  for (const r of exportedRemotables) {
-    const vref = valToSlot.get(r);
-    if (vref === rootSlot) {
-      // We know the new version can/will reattach a new root
-      // object, so if the kernel is still watching it, don't
-      // abandon it. We don't simulate a dispatch.dropExports, to
-      // preserve any vdata that might be keyed by it. But we do
-      // drop it from RAM, so we can collect any Presences or
-      // Representatives the Remotable had captured.
-      exportedRemotables.delete(r);
-    } else {
-      vrefSet.add(vref);
-    }
-  }
-}
-
-// eslint-disable-next-line no-unused-vars
-function identifyExportedFacets(vrefSet, { syscall, vrm }) {
-  // Find all exported (non-durable) virtual object facets, which are
-  // doomed because merely-virtual objects don't survive upgrade. We
-  // walk the "export status" (vom.es.) portion of the DB to find the
-  // ones that are reachable ('r') by the kernel, ignoring the ones
-  // that are merely recognizable ('s'). We'll pretend the kernel
-  // drops these in a minute.
-
-  const prefix = 'vom.es.';
-  for (const key of enumerateKeysWithPrefix(syscall, prefix)) {
-    const value = syscall.vatstoreGet(key);
-    const baseRef = key.slice(prefix.length);
-    const parsed = parseVatSlot(baseRef);
-    assert(
-      (parsed.virtual || parsed.durable) && parsed.baseRef === baseRef,
-      baseRef,
-    );
-    if (!vrm.isDurableKind(parsed.id)) {
-      if (value.length === 1) {
-        // single-facet
-        if (value === 'r') {
-          const vref = baseRef;
-          vrefSet.add(vref);
-        }
-      } else {
-        // multi-facet
-        for (let i = 0; i < value.length; i += 1) {
-          if (value[i] === 'r') {
-            const vref = `${baseRef}:${i}`;
-            vrefSet.add(vref);
-          }
-        }
-      }
-    }
-  }
-}
-
-// eslint-disable-next-line no-unused-vars
-function abandonExports(vrefSet, tools) {
-  // Pretend the kernel dropped everything in the set. The Remotables
-  // will be removed from exportedRemotables. If the export was the
-  // only pillar keeping them alive, the objects will be deleted,
-  // decrefs will happen, and a bunch of stuff will be pushed onto
-  // possiblyDeadSet. The virtual objects will lose their export
-  // pillar, which may do the same.
-
-  const { dropExports, retireExports, syscall } = tools;
-  const abandonedVrefs = Array.from(vrefSet).sort();
-  dropExports(abandonedVrefs);
-  // also pretend the kernel retired them, so we retire them ourselves
-  retireExports(abandonedVrefs);
-
-  // Now that we think they're gone, abandon them so the kernel agrees
-  syscall.abandonExports(abandonedVrefs);
-}
-
-// eslint-disable-next-line no-unused-vars
-function finalizeEverything(tools) {
-  const { slotToVal, addToPossiblyDeadSet, vreffedObjectRegistry } = tools;
-
-  // The liveslots tables which might keep userspace objects alive
-  // are:
-  // * exportedRemotables
-  // * importedDevices
-  // * importedPromisesByPromiseID
-  // * pendingPromises
-  // * vrm.remotableRefCounts
-  // * vrm.vrefRecognizers (which points to virtualObjectMap which
-  //                        is a strong Map whose values might be
-  //                        Presences or Representatives)
-
-  // Use slotToVal to find all the Presences, Remotables, and
-  // Representatives, and simulate the finalizer calls. This doesn't
-  // remove those objects from RAM, but it makes liveslots
-  // decref/drop/retire things as if they were.
-
-  for (const baseRef of slotToVal.keys()) {
-    const p = parseVatSlot(baseRef);
-    if (p.type === 'object' && baseRef !== rootSlot) {
-      const wr = slotToVal.get(baseRef);
-      const val = wr.deref();
-      if (val) {
-        // the object is still around, so pretend it went away
-        addToPossiblyDeadSet(baseRef);
-        // and remove it, else scanForDeadObjects() will think it was
-        // reintroduced
-        slotToVal.delete(baseRef);
-        // stop the real finalizer from firing
-        vreffedObjectRegistry.unregister(val);
-      }
-      // if !wr.deref(), there should already be a finalizer call queued
-    }
-  }
-}
-
-// eslint-disable-next-line no-unused-vars
-function deleteVirtualObjectsWithoutDecref({ vrm, syscall }) {
-  // delete the data of all non-durable virtual objects, without
-  // attempting to decrement the refcounts of the surviving
-  // imports/durable-objects they might point to
-
-  const prefix = 'vom.o+';
-  for (const key of enumerateKeysWithPrefix(syscall, prefix)) {
-    const baseRef = key.slice('vom.'.length);
-    const p = parseVatSlot(baseRef);
-    if (!vrm.isDurableKind(p.id)) {
-      syscall.vatstoreDelete(key);
-    }
-  }
-}
-
-// BEGIN: the following functions aren't ready for use yet
-
-// eslint-disable-next-line no-unused-vars
-function deleteVirtualObjectsWithDecref({ syscall, vrm }) {
-  // Delete the data of all non-durable objects, building up a list of
-  // decrefs to apply to possibly-surviving imports and durable
-  // objects that the late virtual objects pointed to. We don't need
-  // to tell the kernel that we're deleting these: we already
-  // abandoned any that were exported.
-
-  const durableDecrefs = new Map(); // baseRef -> count
-  const importDecrefs = new Map(); // baseRef -> count
-  const prefix = 'vom.o+';
-
-  for (const key of enumerateKeysWithPrefix(syscall, prefix)) {
-    const value = syscall.vatstoreGet(key);
-    const baseRef = key.slice('vom.'.length);
-    const p = parseVatSlot(baseRef);
-    if (!vrm.isDurableKind(p.id)) {
-      const raw = JSON.parse(value);
-      for (const capdata of Object.values(raw)) {
-        for (const vref of capdata.slots) {
-          const p2 = parseVatSlot(vref);
-          if ((p2.virtual || p2.durable) && vrm.isDurableKind(p2.id)) {
-            const count = durableDecrefs.get(p2.baseRef) || 0;
-            durableDecrefs.set(p2.baseRef, count + 1);
-          }
-          if (!p2.allocatedByVat) {
-            const count = importDecrefs.get(p2.baseRef) || 0;
-            importDecrefs.set(p2.baseRef, count + 1);
-          }
-        }
-      }
-      syscall.vatstoreDelete(key);
-    }
-  }
-
-  // now decrement the DOs and imports that were held by the VOs,
-  // applying the whole delta at once (instead of doing multiple
-  // single decrefs)
-  const durableBaserefs = Array.from(durableDecrefs.keys()).sort();
-  for (const baseRef of durableBaserefs) {
-    // @ts-expect-error FIXME .get does not exist on array
-    vrm.decRefCount(baseRef, durableBaserefs.get(baseRef));
-  }
-
-  const importVrefs = Array.from(importDecrefs.keys()).sort();
-  for (const baseRef of importVrefs) {
-    vrm.decRefCount(baseRef, importDecrefs.get(baseRef));
-  }
-}
-
-// eslint-disable-next-line no-unused-vars
-function deleteCollectionsWithDecref({ syscall, vrm }) {
-  // TODO this is not ready yet
-
-  // Delete all items of all non-durable collections, counting up how
-  // many references their values had to imports and durable objects,
-  // so we can decref them in a large chunk at the end.
-
-  // Walk prefix='vc.', extract vc.NN., look up whether collectionID
-  // NN is durable or not, skip the durables, delete the vc.NN.|
-  // metadata keys, walk the remaining vc.NN. keys, JSON.parse each,
-  // extract slots, update decrefcounts, delete
-
-  // TODO: vrefs used as keys may maintain a refcount, and need to be
-  // handled specially. This code will probably get confused by such
-  // entries.
-
-  const durableDecrefs = new Map(); // baseRef -> count
-  const importDecrefs = new Map(); // baseRef -> count
-  const prefix = 'vom.vc.';
-
-  for (const key of enumerateKeysWithPrefix(syscall, prefix)) {
-    const value = syscall.vatstoreGet(key);
-    const subkey = key.slice(prefix.length); // '2.|meta' or '2.ENCKEY'
-    const collectionID = subkey.slice(0, subkey.index('.')); // string
-    const subsubkey = subkey.slice(collectionID.length); // '|meta' or 'ENCKEY'
-    const isMeta = subsubkey.slice(0, 1) === '|';
-    const isDurable = 'TODO'; // ask collectionManager about collectionID
-    if (!isDurable && !isMeta) {
-      for (const vref of JSON.parse(value).slots) {
-        const p = parseVatSlot(vref);
-        if ((p.virtual || p.durable) && vrm.isDurableKind(p.id)) {
-          const count = durableDecrefs.get(p.baseRef) || 0;
-          durableDecrefs.set(p.baseRef, count + 1);
-        }
-        if (!p.allocatedByVat) {
-          const count = importDecrefs.get(p.baseRef) || 0;
-          importDecrefs.set(p.baseRef, count + 1);
-        }
-      }
-    }
-    syscall.vatstoreDelete(key);
-  }
-  const durableBaserefs = Array.from(durableDecrefs.keys()).sort();
-  for (const baseRef of durableBaserefs) {
-    // @ts-expect-error FIXME .get does not exist on array
-    vrm.decRefCount(baseRef, durableBaserefs.get(baseRef));
-  }
-
-  const importVrefs = Array.from(importDecrefs.keys()).sort();
-  for (const baseRef of importVrefs) {
-    vrm.decRefCount(baseRef, importDecrefs.get(baseRef));
-  }
-}
-
-// END: the preceding functions aren't ready for use yet
-
-export async function releaseOldState(tools) {
-  // The next step is to pretend that the kernel has dropped all
-  // non-durable exports: both the in-RAM Remotables and the on-disk
-  // virtual objects (but not the root object). This will trigger
-  // refcount decrements which may drop some virtuals from the DB. It
-  // might also drop some objects from RAM.
-
-  // TODO: Decide how much (if any) cleanup to do here in the vat.
-  // The kernel simulates abandonExports as part of vat upgrade,
-  // but does not decrement vat-side refcounts which could allow us to
-  // drop durable objects that were only being kept alive by references from
-  // non-durable objects.
-  // const abandonedVrefSet = new Set();
-  // identifyExportedRemotables(abandonedVrefSet, tools);
-  // identifyExportedFacets(abandonedVrefSet, tools);
-  // abandonExports(abandonedVrefSet, tools);
-
-  // bringOutYourDead remains to ensure that the LRU cache is flushed,
-  // but the rest of this function has been disabled to improve stop-vat
-  // performance.
-  // eslint-disable-next-line no-use-before-define
-  await tools.bringOutYourDead();
-
-  /* We expect that in the fullness of time the following will be superseded by a
-   * post-upgrade scavenger process that cleans up dead database debris
-   * incrementally, rather than taking the hit of a potentially large delay at
-   * shutdown time.  If that change happens, the below code can simply be
-   * removed.  Until then, I'm leaving it in place as scaffolding, just in case,
-   * on the theory that it will be easier to reconstruct as a unified whole
-   * without having to navigate through a confusing maze of twisty git branches.
-
-  // Then we pretend userspace RAM has dropped all the vref-based
-  // objects that it was holding onto.
-  finalizeEverything(tools);
-
-  // Now we ask collectionManager for help with deleting all
-  // non-durable collections. This will delete all the DB entries
-  // (including the metadata), decref everything they point to,
-  // including imports and DOs, and add to possiblyDeadSet. There
-  // might still be a Presence for the collection in RAM, but if/when
-  // it is deleted, the collectionManager will tolerate (ignore) the
-  // resulting attempt to free all the entries a second time.
-
-  tools.collectionManager.deleteAllVirtualCollections();
-
-  // Now we'll have finalizers pending and `possiblyDeadSet` will be
-  // populated with our simulated drops, so a `bringOutYourDead` will
-  // release a lot.
-
-  // eslint-disable-next-line no-use-before-define
-  await tools.bringOutYourDead();
-
-  // possiblyDeadSet is now empty
-
-  // NOTE: instead of using deleteAllVirtualCollections() above (which
-  // does a lot of decref work we don't really care about), we might
-  // use deleteCollectionsWithDecref() here, once it's ready. It
-  // should be faster because it doesn't need to care about refcounts
-  // of virtual objects or Remotables, only those of imports and
-  // durables. But it bypasses the usual refcounting code, so it
-  // should probably be called after the last BOYD.
-  //
-  // deleteCollectionsWithDecref(tools);
-
-  // The remaining data is virtual objects which participate in cycles
-  // (although not through virtual collections, which were deleted
-  // above), durable objects held by [those virtual objects, durable
-  // object cycles, exports, or baggage], and imports held by all of
-  // those.
-
-  // eslint-disable-next-line no-constant-condition
-  if (1) {
-    // We delete the data of all merely-virtual objects. For now, we
-    // don't attempt to decrement the refcounts of things they point
-    // to (which might allow us to drop some imports and a subset of
-    // the durable objects).
-    deleteVirtualObjectsWithoutDecref(tools);
-
-    // The remaining data is durable objects which were held by
-    // virtual-object cycles, or are still held by durable-object
-    // cycles or exports or baggage, and imports held by all of those.
-
-    // At this point we declare sufficient victory and return.
-  } else {
-    // We delete the data of all merely-virtual objects, and
-    // accumulate counts of the deleted references to durable objects
-    // and imports (ignoring references to Remotables and other
-    // virtual objects). After deletion, we apply the decrefs, which
-    // may cause some durable objects and imports to be added to
-    // possiblyDeadSet.
-    deleteVirtualObjectsWithDecref(tools);
-
-    // possiblyDeadSet will now have baserefs for durable objects and
-    // imports (the ones that were only kept alive by virtual-object
-    // cycles). There won't be any virtual-object baserefs in
-    // possiblyDeadSet because we didn't apply any of those
-    // decrefs. So our `bringOutYourDead` won't try to read or modify
-    // any of the on-disk refcounts for VOs (which would fail because
-    // we deleted everything).
-
-    // eslint-disable-next-line no-use-before-define
-    await tools.bringOutYourDead();
-
-    // The remaining data is durable objects which are held by a
-    // durable-object cycle, exports, or baggage, and imports held by
-    // all of those.
-
-    // At this point we declare sufficient victory and return.
-  }
-
-  */
-}

From 8324ea21bec94d4abae79b56ad513ac7244fae24 Mon Sep 17 00:00:00 2001
From: Richard Gibson <richard.gibson@gmail.com>
Date: Tue, 28 Mar 2023 18:00:02 -0400
Subject: [PATCH 08/12] test(SwingSet): Serialize some test execution to dodge
 incomplete V8 GC

---
 packages/SwingSet/test/upgrade/test-upgrade.js | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/packages/SwingSet/test/upgrade/test-upgrade.js b/packages/SwingSet/test/upgrade/test-upgrade.js
index 1b92d58f13f..5ba662ffc20 100644
--- a/packages/SwingSet/test/upgrade/test-upgrade.js
+++ b/packages/SwingSet/test/upgrade/test-upgrade.js
@@ -498,22 +498,21 @@ const testUpgrade = async (t, defaultManagerType, options = {}) => {
   });
 };
 
-test('vat upgrade - local', async t => {
+// run GC-sensitive upgrade tests that use a local worker serially
+// to mitigate any V8 gremlins
+test.serial('vat upgrade - local', async t => {
   return testUpgrade(t, 'local', { restartVatAdmin: false });
 });
-
-test('vat upgrade - local with VA restarts', async t => {
+test.serial('vat upgrade - local with VA restarts', async t => {
   return testUpgrade(t, 'local', { restartVatAdmin: true });
 });
-
-test('vat upgrade - local without automatic GC', async t => {
+test.serial('vat upgrade - local without automatic GC', async t => {
   return testUpgrade(t, 'local', { suppressGC: true });
 });
 
 test('vat upgrade - xsnap', async t => {
   return testUpgrade(t, 'xs-worker');
 });
-
 test('vat upgrade - xsnap without automatic GC', async t => {
   return testUpgrade(t, 'xs-worker', { suppressGC: true });
 });

From 8998e4a809f73f2e3e0fc0614d5ff5bc3712af6e Mon Sep 17 00:00:00 2001
From: Richard Gibson <richard.gibson@gmail.com>
Date: Tue, 4 Apr 2023 13:32:51 -0400
Subject: [PATCH 09/12] refactor(SwingSet): Introduce an abortUpgrade helper

---
 packages/SwingSet/src/kernel/kernel.js | 59 +++++++++++++++-----------
 1 file changed, 35 insertions(+), 24 deletions(-)

diff --git a/packages/SwingSet/src/kernel/kernel.js b/packages/SwingSet/src/kernel/kernel.js
index 1b03e063284..945be1644e7 100644
--- a/packages/SwingSet/src/kernel/kernel.js
+++ b/packages/SwingSet/src/kernel/kernel.js
@@ -446,6 +446,7 @@ export default function buildKernel(
    * @param {DeliveryStatus} status
    * @param {boolean} decrementReapCount
    * @param {MeterID} [meterID]
+   * @returns {CrankResults}
    */
   function deliveryCrankResults(vatID, status, decrementReapCount, meterID) {
     let meterUnderrun = false;
@@ -817,6 +818,7 @@ export default function buildKernel(
       return NO_DELIVERY_CRANK_RESULTS; // vat terminated already
     }
     const { meterID } = vatInfo;
+    let computrons;
     const vatKeeper = kernelKeeper.provideVatKeeper(vatID);
     const disconnectObject = {
       name: 'vatUpgraded',
@@ -826,18 +828,38 @@ export default function buildKernel(
     const disconnectionCapData = kser(disconnectObject);
 
     /**
-     * Make a method-arguments structure representing failure
-     * for vat-vat-admin.js vatUpgradeCallback().
+     * Terminate the vat and translate internal-delivery results into
+     * abort-without-termination results for the upgrade delivery.
      *
-     * @param {SwingSetCapData} _errorCapData
-     * @returns {RawMethargs}
+     * @param {CrankResults} badDeliveryResults
+     * @param {SwingSetCapData} errorCapData
+     * @returns {Promise<CrankResults>}
      */
-    const makeFailureMethargs = _errorCapData => {
-      insistCapData(_errorCapData); // kser(Error)
-      // const error = kunser(_errorCD)
-      // actually we shouldn't reveal the details, so instead we do:
+    const abortUpgrade = async (badDeliveryResults, errorCapData) => {
+      // get rid of the worker, so the next delivery to this vat will
+      // re-create one from the previous state
+      // eslint-disable-next-line @jessie.js/no-nested-await
+      await vatWarehouse.stopWorker(vatID);
+
+      // notify vat-admin of the failed upgrade without revealing error details
+      insistCapData(errorCapData);
+      // const error = kunser(errorCapData);
       const error = Error('vat-upgrade failure');
-      return ['vatUpgradeCallback', [upgradeID, false, error]];
+      /** @type {RawMethargs} */
+      const vatAdminMethargs = [
+        'vatUpgradeCallback',
+        [upgradeID, false, error],
+      ];
+
+      const results = harden({
+        ...badDeliveryResults,
+        computrons, // still report computrons
+        abort: true, // always unwind
+        consumeMessage: true, // don't repeat the upgrade
+        terminate: undefined, // do *not* terminate the vat
+        vatAdminMethargs,
+      });
+      return results;
     };
 
     // cleanup on behalf of the worker
@@ -864,8 +886,7 @@ export default function buildKernel(
 
     // we don't meter bringOutYourDead since no user code is running, but we
     // still report computrons to the runPolicy
-    let { computrons } = boydResults;
-    computrons === undefined || assert.typeof(computrons, 'bigint');
+    computrons = addComputrons(computrons, boydResults.computrons);
 
     // TODO: if/when we implement vat pause/suspend, and if
     // deliveryCrankResults changes to not use .terminate to indicate
@@ -921,20 +942,10 @@ export default function buildKernel(
     computrons = addComputrons(computrons, startVatResults.computrons);
 
     if (startVatResults.terminate) {
-      // unwind just like above
+      // abort and unwind the upgrade
+      const { info: errorCapData } = startVatResults.terminate;
       // eslint-disable-next-line @jessie.js/no-nested-await
-      await vatWarehouse.stopWorker(vatID);
-      const vatAdminMethargs = makeFailureMethargs(
-        startVatResults.terminate.info,
-      );
-      const results = harden({
-        ...startVatResults,
-        computrons,
-        abort: true, // always unwind
-        consumeMessage: true, // don't repeat the upgrade
-        terminate: undefined, // do *not* terminate the vat
-        vatAdminMethargs,
-      });
+      const results = await abortUpgrade(startVatResults, errorCapData);
       return results;
     }
 

From aed1282727a9514d355ff9e6c2d9b43f4c562505 Mon Sep 17 00:00:00 2001
From: Richard Gibson <richard.gibson@gmail.com>
Date: Tue, 4 Apr 2023 13:45:29 -0400
Subject: [PATCH 10/12] fix(SwingSet): Abort-and-unwind vat upgrade upon any
 internal-delivery failure

cf. https://github.com/Agoric/agoric-sdk/pull/7244#discussion_r1153633902
---
 packages/SwingSet/src/kernel/kernel.js | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/packages/SwingSet/src/kernel/kernel.js b/packages/SwingSet/src/kernel/kernel.js
index 945be1644e7..313d1691330 100644
--- a/packages/SwingSet/src/kernel/kernel.js
+++ b/packages/SwingSet/src/kernel/kernel.js
@@ -881,16 +881,27 @@ export default function buildKernel(
     const boydVD = vatWarehouse.kernelDeliveryToVatDelivery(vatID, boydKD);
     const boydStatus = await deliverAndLogToVat(vatID, boydKD, boydVD);
     const boydResults = deliveryCrankResults(vatID, boydStatus, false);
-    (!boydResults.abort && !boydResults.terminate) ||
-      Fail`Unexpected abort/terminate result from upgrade-internal bringOutYourDead: ${boydResults}`;
 
     // we don't meter bringOutYourDead since no user code is running, but we
     // still report computrons to the runPolicy
     computrons = addComputrons(computrons, boydResults.computrons);
 
-    // TODO: if/when we implement vat pause/suspend, and if
-    // deliveryCrankResults changes to not use .terminate to indicate
-    // a problem, we might want to unwind the upgrade here.
+    // In the unexpected event that there is a problem during this
+    // upgrade-internal BOYD, the appropriate response isn't fully
+    // clear. We currently opt to translate a `terminate` result into a
+    // non-terminating `abort` that unwinds the upgrade delivery, and to
+    // ignore a non-terminate `abort` result. This is expected to change
+    // in the future, especially if we ever need some kind of emergency/
+    // manual upgrade (which might involve something like throwing an
+    // error to prompt a kernel panic if the bad vat is marked critical).
+    // There's a good analysis at
+    // https://github.com/Agoric/agoric-sdk/pull/7244#discussion_r1153633902
+    if (boydResults.terminate) {
+      const { info: errorCapData } = boydResults.terminate;
+      // eslint-disable-next-line @jessie.js/no-nested-await
+      const results = await abortUpgrade(boydResults, errorCapData);
+      return results;
+    }
 
     // reject all promises for which the vat was decider
     for (const kpid of kernelKeeper.enumeratePromisesByDecider(vatID)) {
@@ -942,7 +953,7 @@ export default function buildKernel(
     computrons = addComputrons(computrons, startVatResults.computrons);
 
     if (startVatResults.terminate) {
-      // abort and unwind the upgrade
+      // abort and unwind just like above
       const { info: errorCapData } = startVatResults.terminate;
       // eslint-disable-next-line @jessie.js/no-nested-await
       const results = await abortUpgrade(startVatResults, errorCapData);

From 0fbb5c421a22aa8c4d736a9c71586d0b49a161dc Mon Sep 17 00:00:00 2001
From: Richard Gibson <richard.gibson@gmail.com>
Date: Tue, 4 Apr 2023 13:51:17 -0400
Subject: [PATCH 11/12] test(SwingSet): Add a PR reference for future recovery
 of tests

---
 packages/SwingSet/test/upgrade/test-upgrade.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/SwingSet/test/upgrade/test-upgrade.js b/packages/SwingSet/test/upgrade/test-upgrade.js
index 5ba662ffc20..8e3a015fd71 100644
--- a/packages/SwingSet/test/upgrade/test-upgrade.js
+++ b/packages/SwingSet/test/upgrade/test-upgrade.js
@@ -489,7 +489,7 @@ const testUpgrade = async (t, defaultManagerType, options = {}) => {
   // This used to be MUCH more extensive, but GC was cut to the bone
   // in commits like 91480dee8e48ae26c39c420febf73b93deba6ea5
   // basically reverting 1cfbeaa3c925d0f8502edfb313ecb12a1cab5eac
-  // (see also #5342 and #6650).
+  // (see also #5342 and #6650, and #7244 for tests to restore).
   // It can be restored once we add back correct sophisticated logic
   // by e.g. having liveslots sweep the database when restoring a vat.
   verifyObjectTracking('after upgrade', {

From 7b718d7f98f18a637dab2753b20cfab88d1e88cf Mon Sep 17 00:00:00 2001
From: Richard Gibson <richard.gibson@gmail.com>
Date: Tue, 4 Apr 2023 13:55:12 -0400
Subject: [PATCH 12/12] chore(swingset-liveslots): Replace the stopVat error
 with a log message

---
 packages/swingset-liveslots/src/liveslots.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/swingset-liveslots/src/liveslots.js b/packages/swingset-liveslots/src/liveslots.js
index 46af39c7dcc..eaa22b2529b 100644
--- a/packages/swingset-liveslots/src/liveslots.js
+++ b/packages/swingset-liveslots/src/liveslots.js
@@ -1519,7 +1519,7 @@ function build(
    * @returns {Promise<void>}
    */
   async function stopVat(_disconnectObjectCapData) {
-    Fail`stopVat is no longer supported as of #6650`;
+    console.warn('stopVat is a no-op as of #6650');
   }
 
   /**