fix: liveslots: initialize counters in startVat, not on-demand (#4926)

Liveslots maintains three counters: the Export ID (used for `o+NN`
exported object vrefs), the Promise ID (for `p+NN` allocated
promises), and the Collection ID (one per virtual/durable
collection).

Originally, these counters were only held in RAM: initialized during
`makeLiveSlots()` and incremented when needed. To fix #4730,
`makeLiveSlots()` was changed to read them from the DB, and then
liveslots writes them back to the DB at the end of any delivery which
modifies them. This ensures that a future version of the vat+liveslots
can pick up allocation where the previous version left off.

However the DB read to initialize the in-RAM copy did not set the
'dirty' flag. This caused variations in DB writes across different
vats: `startVat` would not write out the initial values if
`buildRootObject` did not happen to export any additional objects. In
addition, since the DB value is a simple `JSON.stringify` of the
counter record, the order of the keys varied depending upon whether an
Object or Promise or Collection ID got allocated first.

This commit changes the counter initialization process to use a fixed
order for the counter names, and to always perform the
initialization (and DB write) at the same time in all vats: during
`startVat`, with a write at the end of the delivery. This should make
the initial kvStore contents (just after `startVat`) more consistent.

The new initialization code should behave correctly when we add new
counters in the future: it will merge the new counter names (and
initial values) into the record it reads from the DB during startVat.

A handful of unit tests were updated to expect the new order.

packages/SwingSet/src/liveslots/liveslots.js

@@ -437,21 +437,29 @@ function build(
     return Remotable(iface);
   }
 
+  // We start exportIDs with 1 because 'o+0' is always automatically
+  // pre-assigned to the root object.  The starting point for
+  // numbering promiseIDs is pretty arbitrary.  We start from 5 as a
+  // very minor aid to debugging: It helps, when puzzling over trace
+  // logs and the like, for the numbers in the various species of IDs
+  // to be a little out of sync and thus a little less similar to each
+  // other when jumbled together.
+
+  const initialIDCounters = { exportID: 1, collectionID: 1, promiseID: 5 };
   let idCounters;
   let idCountersAreDirty = false;
 
   function initializeIDCounters() {
     if (!idCounters) {
-      const raw = syscall.vatstoreGet('idCounters');
-      if (raw) {
-        idCounters = JSON.parse(raw);
-      } else {
-        idCounters = {};
-      }
+      // the saved value might be missing, or from an older liveslots
+      // (with fewer counters), so merge it with our initial values
+      const saved = JSON.parse(syscall.vatstoreGet('idCounters') || '{}');
+      idCounters = { ...initialIDCounters, ...saved };
+      idCountersAreDirty = true;
     }
   }
 
-  function allocateNextID(name, initialValue = 1) {
+  function allocateNextID(name) {
     if (!idCounters) {
       // Normally `initializeIDCounters` would be called from startVat, but some
       // tests bypass that so this is a backstop.  Note that the invocation from
@@ -461,10 +469,8 @@ function build(
       // issue.
       initializeIDCounters();
     }
-    if (!idCounters[name]) {
-      idCounters[name] = initialValue;
-    }
     const result = idCounters[name];
+    assert(result !== undefined, `unknown idCounters[${name}]`);
     idCounters[name] += 1;
     idCountersAreDirty = true;
     return result;
@@ -485,22 +491,15 @@ function build(
   // use a slot from the corresponding allocateX
 
   function allocateExportID() {
-    // We start exportIDs with 1 because 'o+0' is always automatically
-    // pre-assigned to the root object.
-    return allocateNextID('exportID', 1);
+    return allocateNextID('exportID');
   }
 
   function allocateCollectionID() {
-    return allocateNextID('collectionID', 1);
+    return allocateNextID('collectionID');
   }
 
   function allocatePromiseID() {
-    // The starting point for numbering promiseIDs is pretty arbitrary.  We start
-    // from 5 as a very minor aid to debugging: It helps, when puzzling over
-    // trace logs and the like, for the numbers in the various species of IDs to
-    // be a little out of sync and thus a little less similar to each other when
-    // jumbled together.
-    const promiseID = allocateNextID('promiseID', 5);
+    const promiseID = allocateNextID('promiseID');
     return makeVatSlot('promise', true, promiseID);
   }
 

packages/SwingSet/test/test-liveslots.js

@@ -752,7 +752,7 @@ test('GC syscall.dropImports', async t => {
   t.deepEqual(log.shift(), {
     type: 'vatstoreSet',
     key: 'idCounters',
-    value: '{"collectionID":2,"exportID":9}',
+    value: '{"exportID":9,"collectionID":2,"promiseID":5}',
   });
   const l2 = log.shift();
   t.deepEqual(l2, {
@@ -1153,7 +1153,7 @@ test('GC dispatch.dropExports', async t => {
   t.deepEqual(log.shift(), {
     type: 'vatstoreSet',
     key: 'idCounters',
-    value: '{"collectionID":2,"exportID":10}',
+    value: '{"exportID":10,"collectionID":2,"promiseID":5}',
   });
   t.deepEqual(log, []);
 
@@ -1220,7 +1220,7 @@ test('GC dispatch.retireExports inhibits syscall.retireExports', async t => {
   t.deepEqual(log.shift(), {
     type: 'vatstoreSet',
     key: 'idCounters',
-    value: '{"collectionID":2,"exportID":10}',
+    value: '{"exportID":10,"collectionID":2,"promiseID":5}',
   });
   t.deepEqual(log, []);
 

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

@@ -382,7 +382,7 @@ test('virtual object gc', async t => {
   }
   t.deepEqual(remainingVOs, {
     'v1.vs.baggageID': 'o+5/1',
-    'v1.vs.idCounters': '{"collectionID":2,"exportID":10,"promiseID":8}',
+    'v1.vs.idCounters': '{"exportID":10,"collectionID":2,"promiseID":8}',
     'v1.vs.storeKindIDTable':
       '{"scalarMapStore":1,"scalarWeakMapStore":2,"scalarSetStore":3,"scalarWeakSetStore":4,"scalarDurableMapStore":5,"scalarDurableWeakMapStore":6,"scalarDurableSetStore":7,"scalarDurableWeakSetStore":8}',
     'v1.vs.vc.1.|entryCount': '0',