fix: prepare for symbol flip

Author: erights

packages/SwingSet/test/promises.test.js

@@ -1,7 +1,9 @@
 // @ts-nocheck
+// eslint-disable-next-line import/order
 import { test } from '../tools/prepare-test-env-ava.js';
 
-// eslint-disable-next-line import/order
+import { passableSymbolForName } from '@agoric/internal';
+import { testFullOrderEQ } from '@agoric/internal/tools/ava-full-order-eq.js';
 import { kser, kslot, kunser } from '@agoric/kmarshal';
 import {
   buildVatController,
@@ -259,7 +261,7 @@ test('local promises are rejected by vat upgrade', async t => {
     return awaitRun(kpid);
   };
 
-  const S = Symbol.for('passable');
+  const S = passableSymbolForName('passable');
   const watcher = await messageToVat('bootstrap', 'createVat', {
     name: 'watcher',
     bundleCapName: 'watcher',
@@ -268,7 +270,7 @@ test('local promises are rejected by vat upgrade', async t => {
   await messageToObject(watcher, 'watchLocalPromise', 'fulfilled', S);
   await messageToObject(watcher, 'watchLocalPromise', 'rejected', undefined, S);
   const v1Settlements = await messageToObject(watcher, 'getSettlements');
-  t.deepEqual(v1Settlements, {
+  testFullOrderEQ(t, v1Settlements, {
     fulfilled: { status: 'fulfilled', value: S },
     rejected: { status: 'rejected', reason: S },
   });
@@ -277,7 +279,7 @@ test('local promises are rejected by vat upgrade', async t => {
     bundleCapName: 'watcher',
   });
   const v2Settlements = await messageToObject(watcher, 'getSettlements');
-  t.deepEqual(v2Settlements, {
+  testFullOrderEQ(t, v2Settlements, {
     fulfilled: { status: 'fulfilled', value: S },
     rejected: { status: 'rejected', reason: S },
     orphaned: {

packages/cosmic-swingset/src/helpers/bufferedStorage.js

@@ -1,4 +1,5 @@
 import { assert, Fail } from '@endo/errors';
+import { unpassableSymbolForName } from '@agoric/internal';
 
 // XXX Do these "StorageAPI" functions belong in their own package?
 
@@ -302,8 +303,8 @@ export function makeBufferedStorage(kvStore, listeners = {}) {
 export const makeReadCachingStorage = kvStore => {
   // In addition to the wrapping write buffer, keep a simple cache of
   // read values for has and get.
-  const deleted = Symbol('deleted');
-  const undef = Symbol('undefined');
+  const deleted = unpassableSymbolForName('deleted');
+  const undef = unpassableSymbolForName('undefined');
   /** @typedef {(typeof deleted) | (typeof undef)} ReadCacheSentinel */
   /** @type {Map<string, T | ReadCacheSentinel>} */
   let cache;
@@ -312,50 +313,51 @@ export const makeReadCachingStorage = kvStore => {
   }
   resetCache();
 
-  /** @type {KVStore<T>} */
-  const storage = harden({
-    has(key) {
-      const value = cache.get(key);
-      if (value !== undefined) {
-        return value !== deleted;
-      } else {
-        const ret = kvStore.has(key);
-        if (!ret) {
-          cache.set(key, deleted);
+  const storage = /** @type {KVStore<T>} */ (
+    harden({
+      has(key) {
+        const value = cache.get(key);
+        if (value !== undefined) {
+          return value !== deleted;
+        } else {
+          const ret = kvStore.has(key);
+          if (!ret) {
+            cache.set(key, deleted);
+          }
+          return ret;
+        }
+      },
+      get(key) {
+        let value = cache.get(key);
+        if (value !== undefined) {
+          return value === deleted || value === undef ? undefined : value;
         }
-        return ret;
-      }
-    },
-    get(key) {
-      let value = cache.get(key);
-      if (value !== undefined) {
-        return value === deleted || value === undef ? undefined : value;
-      }
 
-      // Fetch the value and cache it until the next commit or abort.
-      value = kvStore.get(key);
-      cache.set(key, value === undefined ? undef : value);
-      return value;
-    },
-    set(key, value) {
-      // Set the value and cache it until the next commit or abort (which is
-      // expected immediately, since the buffered wrapper only calls set
-      // *during* a commit).
-      // `undefined` is a valid value technically different than deletion,
-      // depending on how the underlying store does its serialization
-      cache.set(key, value === undefined ? undef : value);
-      kvStore.set(key, value);
-    },
-    delete(key) {
-      // Delete the value and cache the deletion until next commit or abort.
-      // Deletion results in `undefined` on `get`, but `false` on `has`
-      cache.set(key, deleted);
-      kvStore.delete(key);
-    },
-    getNextKey(_previousKey) {
-      throw Error('not implemented');
-    },
-  });
+        // Fetch the value and cache it until the next commit or abort.
+        value = kvStore.get(key);
+        cache.set(key, value === undefined ? undef : value);
+        return value;
+      },
+      set(key, value) {
+        // Set the value and cache it until the next commit or abort (which is
+        // expected immediately, since the buffered wrapper only calls set
+        // *during* a commit).
+        // `undefined` is a valid value technically different than deletion,
+        // depending on how the underlying store does its serialization
+        cache.set(key, value === undefined ? undef : value);
+        kvStore.set(key, value);
+      },
+      delete(key) {
+        // Delete the value and cache the deletion until next commit or abort.
+        // Deletion results in `undefined` on `get`, but `false` on `has`
+        cache.set(key, deleted);
+        kvStore.delete(key);
+      },
+      getNextKey(_previousKey) {
+        throw Error('not implemented');
+      },
+    })
+  );
   const {
     kvStore: buffered,
     commit,

packages/cosmic-swingset/test/export-storage.test.ts

@@ -1,11 +1,13 @@
 import type { TestFn } from 'ava';
 import anyTest from 'ava';
+
+import { unpassableSymbolForName } from '@agoric/internal';
 import { exportStorage } from '../src/export-storage.js';
 
 const test = anyTest as TestFn;
 
 // Export this binding when used by other modules.
-const TestDataKey = Symbol('TestDataKey');
+const TestDataKey = unpassableSymbolForName('TestDataKey');
 
 // Create a local alias, then
 //  - regex replace '\._\b' with '[_]'

packages/import-manager/package.json

@@ -29,6 +29,9 @@
     "url": "https://github.com/Agoric/agoric-sdk/issues"
   },
   "homepage": "https://github.com/Agoric/agoric-sdk#readme",
+  "dependencies": {
+    "@agoric/internal": "^0.3.2"
+  },
   "devDependencies": {
     "@agoric/swingset-vat": "^0.32.2",
     "ava": "^5.3.0",

packages/import-manager/test/unitTests/badImports.js

@@ -1,12 +1,13 @@
 // Copyright (C) 2019 Agoric, under Apache License 2.0
 
+import { unpassableSymbolForName } from '@agoric/internal';
 import { importManager } from '../../src/importManager.js';
 import { listIsEmpty, numIsEmpty } from './valueOps.js';
 
 function makeBadImportManager() {
   const mgr = importManager();
   const obj = { numIsEmpty };
-  const fooSym = Symbol('foo');
+  const fooSym = unpassableSymbolForName('foo');
   obj[fooSym] = listIsEmpty;
   return mgr.addExports(obj);
 }

packages/inter-protocol/test/vaultFactory/driver.js

@@ -1,5 +1,11 @@
 import { AmountMath, makeIssuerKit } from '@agoric/ertp';
-import { allValues, makeTracer, NonNullish, objectMap } from '@agoric/internal';
+import {
+  allValues,
+  makeTracer,
+  NonNullish,
+  objectMap,
+  unpassableSymbolForName,
+} from '@agoric/internal';
 import { makeNotifierFromSubscriber } from '@agoric/notifier';
 import { unsafeMakeBundleCache } from '@agoric/swingset-vat/tools/bundleTool.js';
 import {
@@ -38,7 +44,11 @@ import {
 
 const trace = makeTracer('VFDriver');
 
-export const AT_NEXT = Symbol('AT_NEXT');
+// TODO What does it mean to type something to the typeof itself?
+// That was the inferred type when the right hand side was `Symbol('AT_NEXT')`
+export const AT_NEXT = /** @type {typeof AT_NEXT} */ (
+  unpassableSymbolForName('AT_NEXT')
+);
 
 export const BASIS_POINTS = 10000n;
 

packages/internal/src/ses-utils.js

@@ -9,6 +9,7 @@ import { objectMap } from '@endo/common/object-map.js';
 import { objectMetaMap } from '@endo/common/object-meta-map.js';
 import { fromUniqueEntries } from '@endo/common/from-unique-entries.js';
 import { q, Fail, makeError, annotateError, X } from '@endo/errors';
+import { passableSymbolForName as psfn } from '@endo/pass-style';
 import { deeplyFulfilled, isObject } from '@endo/marshal';
 import { makePromiseKit } from '@endo/promise-kit';
 import { makeQueue } from '@endo/stream';
@@ -420,3 +421,29 @@ export const synchronizedTee = (sourceStream, readerCount) => {
   void pullNext();
   return readers;
 };
+
+/**
+ * An adapter to help transition to flip which symbols are passable. In the endo
+ * release that's current as of this writing, the real `passableSymbolForName`
+ * returns `symbol|undefined`.
+ *
+ * @param {string} name
+ * @returns {symbol}
+ */
+export const passableSymbolForName = name => /** @type {symbol} */ (psfn(name));
+
+const areRegisteredSymbolsPassable =
+  // TODO Symbol should not be a restricted global for @agoric/internal
+  // eslint-disable-next-line no-restricted-globals
+  Symbol.keyFor(passableSymbolForName('x')) === 'x';
+
+/**
+ * An adapter to help transition to flip which symbols are passable.
+ *
+ * @param {string} name
+ * @returns {symbol}
+ */
+export const unpassableSymbolForName = name =>
+  // TODO Symbol should not be a restricted global for @agoric/internal
+  // eslint-disable-next-line no-restricted-globals
+  areRegisteredSymbolsPassable ? Symbol(name) : Symbol.for(name);

packages/internal/test/callback.test.js

@@ -4,6 +4,10 @@ import test from 'ava';
 import { Far } from '@endo/far';
 import { makeHeapZone } from '@agoric/base-zone/heap.js';
 import * as cb from '../src/callback.js';
+import {
+  passableSymbolForName,
+  unpassableSymbolForName,
+} from '../src/ses-utils.js';
 
 /** @import {Callback, SyncCallback} from '../src/types.js' */
 
@@ -50,7 +54,11 @@ test('near function callbacks', t => {
 });
 
 test('near method callbacks', t => {
-  const m2 = Symbol.for('m2');
+  const m2 = passableSymbolForName('m2');
+  // TODO This currently has a symbol-named method. If it passes, it is only
+  // because `o` was not made remotable. Should this test case me deleted,
+  // or should `m2` and its uses be deleted, or should `m2` be converted to
+  // `'m2'`?
   const o = {
     /**
      * @param {number} a
@@ -121,7 +129,6 @@ test('near method callbacks', t => {
 });
 
 test('far method callbacks', async t => {
-  const m2 = Symbol.for('m2');
   const o = Far('MyObject', {
     /**
      * @param {number} a
@@ -139,7 +146,7 @@ test('far method callbacks', async t => {
      * @param {string} c
      * @returns {Promise<string>}
      */
-    [m2]: async (a, b, c) => {
+    m2: async (a, b, c) => {
       return `${a + b}${c}`;
     },
   });
@@ -153,8 +160,8 @@ test('far method callbacks', async t => {
   t.is(await p2r, '19go');
 
   /** @type {Callback<(c: string) => Promise<string>>} */
-  const cbp3 = cb.makeMethodCallback(Promise.resolve(o), m2, 9, 10);
-  t.like(cbp3, { methodName: m2, bound: [9, 10] });
+  const cbp3 = cb.makeMethodCallback(Promise.resolve(o), 'm2', 9, 10);
+  t.like(cbp3, { methodName: 'm2', bound: [9, 10] });
   t.assert(cbp3.target instanceof Promise);
   const p3r = cb.callE(cbp3, 'go');
   t.assert(p3r instanceof Promise);
@@ -236,7 +243,13 @@ test('isCallback', t => {
     'manual method',
   );
   t.true(
-    cb.isCallback({ target: {}, methodName: Symbol.for('foo'), bound: [] }),
+    cb.isCallback({
+      target: {},
+      // TODO if this passes with a symbol-named method, it is because
+      // some relevant object was not made into a remotable.
+      methodName: passableSymbolForName('foo'),
+      bound: [],
+    }),
     'manual symbol-keyed method',
   );
 
@@ -249,7 +262,11 @@ test('isCallback', t => {
     'non-object target',
   );
   t.false(
-    cb.isCallback({ target: {}, methodName: Symbol('foo'), bound: [] }),
+    cb.isCallback({
+      target: {},
+      methodName: unpassableSymbolForName('foo'),
+      bound: [],
+    }),
     'unique symbol method name',
   );
   t.false(cb.isCallback({ target: {}, bound: {} }), 'non-array bound args');

packages/internal/test/snapshots/exports.test.js.md

@@ -52,11 +52,13 @@ Generated by [AVA](https://avajs.dev).
       'objectMap',
       'objectMapMutable',
       'objectMetaMap',
+      'passableSymbolForName',
       'provideLazyMap',
       'pureDataMarshaller',
       'synchronizedTee',
       'typedEntries',
       'typedMap',
+      'unpassableSymbolForName',
       'unprefixedProperties',
       'untilTrue',
       'whileTrue',

packages/internal/test/snapshots/exports.test.js.snap

@@ -8,6 +8,7 @@ import {
   makeFakeStorageKit,
   slotStringUnserialize,
 } from '../src/storage-test-utils.js';
+import { unpassableSymbolForName } from '../src/ses-utils.js';
 
 test('makeFakeStorageKit', async t => {
   const rootPath = 'root';
@@ -29,7 +30,7 @@ test('makeFakeStorageKit', async t => {
       boolean: true,
       null: null,
       undefined,
-      symbol: Symbol('foo'),
+      symbol: unpassableSymbolForName('foo'),
       array: ['foo'],
       object: {
         toString() {

packages/internal/test/storage-test-utils.test.js

@@ -12,6 +12,7 @@ import {
   forever,
   deeplyFulfilledObject,
   synchronizedTee,
+  unpassableSymbolForName,
 } from '../src/ses-utils.js';
 
 /** @import {Permit, Attenuated} from '../src/types.js'; */
@@ -334,7 +335,7 @@ const { value: arbShallow } = fc.letrec(tie => ({
     ]),
     // @ts-expect-error TS2345 function signature
     async (t, { specimen, permit }) => {
-      const tag = Symbol('transformed');
+      const tag = unpassableSymbolForName('transformed');
 
       let mutationCallCount = 0;
       const mutatedAttenuation = attenuate(specimen, permit, obj => {
@@ -527,7 +528,7 @@ test('makeMeasureSeconds', async t => {
   const mockNow = () => times.shift();
   const measureSeconds = makeMeasureSeconds(mockNow);
 
-  const unique = Symbol('unique');
+  const unique = unpassableSymbolForName('unique');
   const output = await measureSeconds(async () => unique);
   t.deepEqual(output, { result: unique, duration: 1.0005 });
   t.deepEqual(times, [NaN]);