fix: eliminate the passableEncoding hack

`PassableEncoding` (not to be confused with `encodePassable`) was a non-standard
serialization scheme used in some tests, which encoded remote references in-band
using Symbols with magic names rather than using the normal marshal package
machinery that puts these into the 'slots' element of the standard capdata
format.  This bypassed various message filtering and transformation logic in the
kernel and also required special methods to be present in the bootstrap vat to
translate this encoding and relay messages to their actual intended
destinations.

This has now been removed.  The relatively small number of tests which used
`passableEncoding` have been updated to use `kmarshal` instead.  Messages and
data are now encoded in a form that all the other code understands.  Test
messages are also now delivered directly to their destinations without having to
count on the existence of a relayer.

In support of this, the controller's `queueToVatRoot` method has been augmented
by the addition of a `queueToVatObject` method, allowing tests to send messages
to specific objects, targeted using remotable references of the sort returned
by `kunser`.  The test support library that a lot of the bootstrap tests use
has been updated to use this improved mechanism.

In addition, `kmarshal` itself has been upgraded using a trick that MarkM
provided for tagging promises, which allows `kmarshal` to be truly stateless.
The (former) statefulness of `kmarshal` caused problems when the module was
imported into different compartments, as each compartment ended up with its own
module instance and thus its own version of the state. This in turn caused these
compartments to have different beliefs about how particular promises were
represented, which caused various things to break.  That's all fixed now.

One wart which has NOT been taken care of in this PR, but which will be
addressed in a follow-on PR that we were already planning for, is the
duplication of `kmarshal.js` in both the SwingSet package and the liveslots
package.  The forthcoming PR will perform a bunch of file renaming and
relocation to put a bunch of support tooling, used by both benchmarks and tests,
into a package of its own, thereby eliminating a lot of weird dependencies and
files in places they don't belong.  As part of this I plan to relocate
`kmarshal` into a package of its own that can then be cleanly imported by the
kernel, liveslots, and the various tests and test support tooling.

All this is in support of issue #8327

packages/SwingSet/src/controller/controller.js

@@ -20,7 +20,7 @@ import engineGC from '@agoric/internal/src/lib-nodejs/engine-gc.js';
 import { startSubprocessWorker } from '@agoric/internal/src/lib-nodejs/spawnSubprocessWorker.js';
 import { waitUntilQuiescent } from '@agoric/internal/src/lib-nodejs/waitUntilQuiescent.js';
 import { makeGcAndFinalize } from '@agoric/internal/src/lib-nodejs/gc-and-finalize.js';
-import { kslot } from '../lib/kmarshal.js';
+import { kslot, krefOf } from '../lib/kmarshal.js';
 import { insistStorageAPI } from '../lib/storageAPI.js';
 import {
   buildKernelBundle,
@@ -360,6 +360,7 @@ export async function makeSwingsetController(
       kernelStorage.emitCrankHashes();
       return result;
     },
+
     vatNameToID(vatName) {
       return kernel.vatNameToID(vatName);
     },
@@ -371,13 +372,15 @@ export async function makeSwingsetController(
      * Queue a method call into the named vat
      *
      * @param {string} vatName
-     * @param {string} method
+     * @param {string|symbol} method
      * @param {unknown[]} args
      * @param {ResolutionPolicy} resultPolicy
      */
     queueToVatRoot(vatName, method, args = [], resultPolicy = 'ignore') {
       const vatID = kernel.vatNameToID(vatName);
-      assert.typeof(method, 'string');
+      if (typeof method !== 'symbol') {
+        assert.typeof(method, 'string');
+      }
       const kref = kernel.getRootObject(vatID);
       const kpid = kernel.queueToKref(kref, method, args, resultPolicy);
       if (kpid) {
@@ -387,6 +390,28 @@ export async function makeSwingsetController(
       return kpid;
     },
 
+    /**
+     * Queue a method call to an object represented by a kmarshal token
+     *
+     * @param {any} target
+     * @param {string|symbol} method
+     * @param {unknown[]} args
+     * @param {ResolutionPolicy} resultPolicy
+     */
+    queueToVatObject(target, method, args = [], resultPolicy = 'ignore') {
+      const targetKref = krefOf(target);
+      assert.typeof(targetKref, 'string');
+      if (typeof method !== 'symbol') {
+        assert.typeof(method, 'string');
+      }
+      const kpid = kernel.queueToKref(targetKref, method, args, resultPolicy);
+      if (kpid) {
+        kernel.kpRegisterInterest(kpid);
+      }
+      kernelStorage.emitCrankHashes();
+      return kpid;
+    },
+
     upgradeStaticVat(vatName, shouldPauseFirst, bundleID, options = {}) {
       const vatID = kernel.vatNameToID(vatName);
       let pauseTarget = null;

packages/SwingSet/src/kernel/kernelQueue.js

@@ -84,7 +84,7 @@ export function makeKernelQueueHandler(tools) {
    * by some other vat. This requires a kref as a target.
    *
    * @param {string} kref  Target of the message
-   * @param {string} method  The method name
+   * @param {string|symbol} method  The method name
    * @param {any[]} args  The arguments array
    * @param {ResolutionPolicy} [policy] How the kernel should handle an eventual
    *    resolution or rejection of the message's result promise. Should be

packages/SwingSet/src/lib/kmarshal.js

@@ -1,6 +1,6 @@
 import { Far, passStyleOf } from '@endo/far';
 import { makeMarshal } from '@endo/marshal';
-import { assert } from '@agoric/assert';
+import { assert, Fail } from '@agoric/assert';
 
 // Simple wrapper for serializing and unserializing marshalled values inside the
 // kernel, where we don't actually want to use clists nor actually allocate real
@@ -9,10 +9,10 @@ import { assert } from '@agoric/assert';
 // agnostic about the internal details of the serialization encoding.
 
 /**
- * @typedef {{getKref: () => string, iface: () => string}} KCap
+ * @typedef {{getKref: () => string, iface: () => string} | Promise} KCap
  */
 
-const refMap = new WeakMap();
+const { toStringTag } = Symbol;
 
 /**
  * @param {string} kref
@@ -32,21 +32,26 @@ export const kslot = (kref, iface = 'undefined') => {
     kref.startsWith('lp') ||
     kref.startsWith('rp')
   ) {
-    // TODO: temporary hack because smallcaps encodes promise references
-    // differently from remotable object references, and the current version of
-    // the smallcaps decoder annoyingly insists that if the encoded body string
-    // says a slot is a promise, then convertSlotToVal had better by damn return
-    // an actual Promise, even if, as in the intended use case here, we neither
-    // want nor need a promise, nor the overhead of a map to keep track of it
-    // with.  This behavior is in service of defense against a hypothesized
-    // security issue whose exact nature has largely been forgotton in the
-    // months since it was first encountered.  MarkM is currently researching
-    // what the problem was thought to have been, to see if it is real and to
-    // understand it if so.  This will eventually result in either changes to
-    // the smallcaps encoding or to the marshal setup API to support the purely
-    // manipulative use case.  In the meantime, this ugliness...
-    const p = new Promise(() => undefined);
-    refMap.set(p, kref);
+    // Bizarro World hack for attaching a string property to a Promise, courtesy
+    // of MarkM.  Even though the @@toStringTag property nominally *is* a
+    // string, some unfortunate stuff in our hardened JS safety checks blows up
+    // if it actually is.  Eventually that will be fixed and we'll be able to
+    // use the @@toStringTag property directly, but for now we need to use this
+    // weird construct employing a sneaky getter function.  Note that this is
+    // only necessary in the first place because smallcaps encodes promise
+    // references differently from remotable object references, and the current
+    // version of the smallcaps decoder annoyingly insists that if the encoded
+    // body string says a slot is a promise, then convertSlotToVal had better by
+    // damn return an actual Promise, even if, as in the intended use case here,
+    // we neither want nor need a promise nor are capable of making any use of
+    // the fact that it is one.
+    const p = Promise.resolve(`${kref} stand in`);
+    // Bizarro World makes eslint hallucinate
+    // eslint-disable-next-line @typescript-eslint/no-floating-promises
+    Object.defineProperty(p, toStringTag, {
+      get: reveal => (reveal ? kref : NaN),
+      enumerable: false,
+    });
     return harden(p);
   } else {
     const o = Far(iface, {
@@ -62,20 +67,33 @@ export const kslot = (kref, iface = 'undefined') => {
  * @returns {string}
  */
 export const krefOf = obj => {
-  const fromMap = refMap.get(obj);
-  if (fromMap) {
-    return fromMap;
+  switch (passStyleOf(obj)) {
+    case 'promise': {
+      // Other half of Bizarro World hack for handling promises.  Note the
+      // peculiar way the @@toStringTag getter is used.
+      const desc = Object.getOwnPropertyDescriptor(obj, toStringTag);
+      assert(desc !== undefined, 'promise lacks toStringTag getter');
+      const getter = desc.get;
+      assert.typeof(getter, 'function', 'toStringTag getter is not a function');
+      // @ts-expect-error violates the norm that getters have zero parameters
+      const kref = getter(true);
+      assert.typeof(kref, 'string');
+      return kref;
+    }
+    case 'remotable': {
+      const getKref = obj.getKref;
+      assert.typeof(getKref, 'function', 'object lacks getKref function');
+      return getKref();
+    }
+    default:
+      // When krefOf() is called as part of kmarshal.serialize, marshal
+      // will only give it things that are 'remotable' (Promises and the
+      // Far objects created by kslot()).  When krefOf() is called by
+      // kernel code (as part of extractSingleSlot() or the vat-comms
+      // equivalent), it ought to throw if 'obj' is not one of the
+      // objects created by our kslot().
+      return Fail`krefOf requires a promise or remotable`;
   }
-  // When krefOf() is called as part of kmarshal.serialize, marshal
-  // will only give it things that are 'remotable' (Promises and the
-  // Far objects created by kslot()).  When krefOf() is called by
-  // kernel code (as part of extractSingleSlot() or the vat-comms
-  // equivalent), it ought to throw if 'obj' is not one of the Far
-  // objects created by our kslot().
-  assert.equal(passStyleOf(obj), 'remotable', obj);
-  const getKref = obj.getKref;
-  assert.typeof(getKref, 'function', 'object lacking getKref function');
-  return getKref();
 };
 
 const kmarshal = makeMarshal(krefOf, kslot, {

packages/SwingSet/test/test-promises.js

@@ -238,9 +238,7 @@ test('local promises are rejected by vat upgrade', async t => {
   c.pinVatRoot('bootstrap');
   await c.run();
 
-  const run = async (method, args = []) => {
-    assert(Array.isArray(args));
-    const kpid = c.queueToVatRoot('bootstrap', method, args);
+  const awaitRun = async kpid => {
     await c.run();
     const status = c.kpStatus(kpid);
     if (status === 'fulfilled') {
@@ -251,24 +249,34 @@ test('local promises are rejected by vat upgrade', async t => {
     const err = c.kpResolution(kpid);
     throw kunser(err);
   };
-  const messageVat = (name, methodName, args) =>
-    run('messageVat', [{ name, methodName, args }]);
-  // eslint-disable-next-line no-unused-vars
-  const messageObject = (presence, methodName, args) =>
-    run('messageVatObject', [{ presence, methodName, args }]);
+
+  const messageToVat = async (vatName, method, ...args) => {
+    const kpid = c.queueToVatRoot(vatName, method, args);
+    return awaitRun(kpid);
+  };
+  const messageToObject = async (presence, method, ...args) => {
+    const kpid = c.queueToVatObject(presence, method, args);
+    return awaitRun(kpid);
+  };
 
   const S = Symbol.for('passable');
-  await run('createVat', [{ name: 'watcher', bundleCapName: 'watcher' }]);
-  await messageVat('watcher', 'watchLocalPromise', ['orphaned']);
-  await messageVat('watcher', 'watchLocalPromise', ['fulfilled', S]);
-  await messageVat('watcher', 'watchLocalPromise', ['rejected', undefined, S]);
-  const v1Settlements = await messageVat('watcher', 'getSettlements');
+  const watcher = await messageToVat('bootstrap', 'createVat', {
+    name: 'watcher',
+    bundleCapName: 'watcher',
+  });
+  await messageToObject(watcher, 'watchLocalPromise', 'orphaned');
+  await messageToObject(watcher, 'watchLocalPromise', 'fulfilled', S);
+  await messageToObject(watcher, 'watchLocalPromise', 'rejected', undefined, S);
+  const v1Settlements = await messageToObject(watcher, 'getSettlements');
   t.deepEqual(v1Settlements, {
     fulfilled: { status: 'fulfilled', value: S },
     rejected: { status: 'rejected', reason: S },
   });
-  await run('upgradeVat', [{ name: 'watcher', bundleCapName: 'watcher' }]);
-  const v2Settlements = await messageVat('watcher', 'getSettlements');
+  await messageToVat('bootstrap', 'upgradeVat', {
+    name: 'watcher',
+    bundleCapName: 'watcher',
+  });
+  const v2Settlements = await messageToObject(watcher, 'getSettlements');
   t.deepEqual(v2Settlements, {
     fulfilled: { status: 'fulfilled', value: S },
     rejected: { status: 'rejected', reason: S },