fix: tools arb passable (#1291)

packages/marshal/package.json

@@ -39,13 +39,13 @@
     "@endo/eventual-send": "^0.16.9",
     "@endo/nat": "^4.1.24",
     "@endo/pass-style": "^0.1.0",
-    "@endo/promise-kit": "^0.2.53"
+    "@endo/promise-kit": "^0.2.53",
+    "@fast-check/ava": "^1.0.1"
   },
   "devDependencies": {
     "@endo/init": "^0.5.53",
     "@endo/lockdown": "^0.1.25",
     "@endo/ses-ava": "^0.2.37",
-    "@fast-check/ava": "^1.0.1",
     "ava": "^5.1.0",
     "c8": "^7.7.3"
   },

packages/marshal/src/encodePassable.js

@@ -293,17 +293,16 @@ const decodeTagged = (encoded, decodePassable) => {
  */
 
 /**
- * @param {EncodeOptions} encodeOptions
- * encodeOptions is actually optional, but not marked as such to work around
- * https://github.com/microsoft/TypeScript/issues/50286
- *
+ * @param {EncodeOptions} [encodeOptions]
  * @returns {(passable: Passable) => string}
  */
-export const makeEncodePassable = ({
-  encodeRemotable = (rem, _) => Fail`remotable unexpected: ${rem}`,
-  encodePromise = (prom, _) => Fail`promise unexpected: ${prom}`,
-  encodeError = (err, _) => Fail`error unexpected: ${err}`,
-} = {}) => {
+export const makeEncodePassable = (encodeOptions = {}) => {
+  const {
+    encodeRemotable = (rem, _) => Fail`remotable unexpected: ${rem}`,
+    encodePromise = (prom, _) => Fail`promise unexpected: ${prom}`,
+    encodeError = (err, _) => Fail`error unexpected: ${err}`,
+  } = encodeOptions;
+
   const encodePassable = passable => {
     if (isErrorLike(passable)) {
       return encodeError(passable, encodePassable);
@@ -387,18 +386,13 @@ harden(makeEncodePassable);
  * @param {DecodeOptions} [decodeOptions]
  * @returns {(encoded: string) => Passable}
  */
-// `yarn lint` complains here but not for equivalent code in agoric-sdk.
-// Also, vscode does not complain. Hence we're using at-ts-ignore rather than
-// at-ts-expect-error. Using at-ts-ignore should also generate a complaint
-// that we should be using at-expect-error, where we would normally need
-// to suppress that error as well. However, perhaps that second error currently
-// happens only in agoric-sdk, but not yet in endo. TODO figure out and fix.
-// @ts-ignore
-export const makeDecodePassable = ({
-  decodeRemotable = (rem, _) => Fail`remotable unexpected: ${rem}`,
-  decodePromise = (prom, _) => Fail`promise unexpected: ${prom}`,
-  decodeError = (err, _) => Fail`error unexpected: ${err}`,
-} = {}) => {
+export const makeDecodePassable = (decodeOptions = {}) => {
+  const {
+    decodeRemotable = (rem, _) => Fail`remotable unexpected: ${rem}`,
+    decodePromise = (prom, _) => Fail`promise unexpected: ${prom}`,
+    decodeError = (err, _) => Fail`error unexpected: ${err}`,
+  } = decodeOptions;
+
   const decodePassable = encoded => {
     switch (encoded.charAt(0)) {
       case 'v': {

packages/marshal/src/encodeToCapData.js

@@ -82,17 +82,16 @@ const dontEncodePromiseToCapData = prom => Fail`promise unexpected: ${prom}`;
 const dontEncodeErrorToCapData = err => Fail`error object unexpected: ${err}`;
 
 /**
- * @param {EncodeToCapDataOptions} encodeOptions
- * encodeOptions is actually optional, but not marked as such to work around
- * https://github.com/microsoft/TypeScript/issues/50286
- *
+ * @param {EncodeToCapDataOptions} [encodeOptions]
  * @returns {(passable: Passable) => Encoding}
  */
-export const makeEncodeToCapData = ({
-  encodeRemotableToCapData = dontEncodeRemotableToCapData,
-  encodePromiseToCapData = dontEncodePromiseToCapData,
-  encodeErrorToCapData = dontEncodeErrorToCapData,
-} = {}) => {
+export const makeEncodeToCapData = (encodeOptions = {}) => {
+  const {
+    encodeRemotableToCapData = dontEncodeRemotableToCapData,
+    encodePromiseToCapData = dontEncodePromiseToCapData,
+    encodeErrorToCapData = dontEncodeErrorToCapData,
+  } = encodeOptions;
+
   /**
    * Must encode `val` into plain JSON data *canonically*, such that
    * `JSON.stringify(encode(v1)) === JSON.stringify(encode(v1))`. For most
@@ -296,14 +295,16 @@ const dontDecodeErrorFromCapData = errorEncoding =>
  * API where these can reliably differ.
  * See https://github.com/Agoric/agoric-sdk/issues/4334
  *
- * @param {DecodeOptions} decodeOptions
+ * @param {DecodeOptions} [decodeOptions]
  * @returns {(encoded: Encoding) => Passable}
  */
-export const makeDecodeFromCapData = ({
-  decodeRemotableFromCapData = dontDecodeRemotableOrPromiseFromCapData,
-  decodePromiseFromCapData = dontDecodeRemotableOrPromiseFromCapData,
-  decodeErrorFromCapData = dontDecodeErrorFromCapData,
-} = {}) => {
+export const makeDecodeFromCapData = (decodeOptions = {}) => {
+  const {
+    decodeRemotableFromCapData = dontDecodeRemotableOrPromiseFromCapData,
+    decodePromiseFromCapData = dontDecodeRemotableOrPromiseFromCapData,
+    decodeErrorFromCapData = dontDecodeErrorFromCapData,
+  } = decodeOptions;
+
   decodeRemotableFromCapData === decodePromiseFromCapData ||
     Fail`An implementation restriction for now: If either decodeRemotableFromCapData or decodePromiseFromCapData is provided, both must be provided and they must be the same: ${q(
       decodeRemotableFromCapData,

packages/marshal/src/encodeToSmallcaps.js

@@ -111,17 +111,19 @@ const dontEncodeErrorToSmallcaps = err =>
   Fail`error object unexpected: ${q(err)}`;
 
 /**
- * @param {EncodeToSmallcapsOptions} encodeOptions
+ * @param {EncodeToSmallcapsOptions} [encodeOptions]
  * encodeOptions is actually optional, but not marked as such to work around
  * https://github.com/microsoft/TypeScript/issues/50286
  *
  * @returns {(passable: Passable) => SmallcapsEncoding}
  */
-export const makeEncodeToSmallcaps = ({
-  encodeRemotableToSmallcaps = dontEncodeRemotableToSmallcaps,
-  encodePromiseToSmallcaps = dontEncodePromiseToSmallcaps,
-  encodeErrorToSmallcaps = dontEncodeErrorToSmallcaps,
-} = {}) => {
+export const makeEncodeToSmallcaps = (encodeOptions = {}) => {
+  const {
+    encodeRemotableToSmallcaps = dontEncodeRemotableToSmallcaps,
+    encodePromiseToSmallcaps = dontEncodePromiseToSmallcaps,
+    encodeErrorToSmallcaps = dontEncodeErrorToSmallcaps,
+  } = encodeOptions;
+
   const assertEncodedError = encoding => {
     (typeof encoding === 'object' && hasOwnPropertyOf(encoding, '#error')) ||
       Fail`internal: Error encoding must have "#error" property: ${q(
@@ -316,14 +318,16 @@ const dontDecodeErrorFromSmallcaps = encoding =>
   Fail`error unexpected: ${q(encoding)}`;
 
 /**
- * @param {DecodeFromSmallcapsOptions} decodeOptions
+ * @param {DecodeFromSmallcapsOptions} [decodeOptions]
  * @returns {(encoded: SmallcapsEncoding) => Passable}
  */
-export const makeDecodeFromSmallcaps = ({
-  decodeRemotableFromSmallcaps = dontDecodeRemotableFromSmallcaps,
-  decodePromiseFromSmallcaps = dontDecodePromiseFromSmallcaps,
-  decodeErrorFromSmallcaps = dontDecodeErrorFromSmallcaps,
-} = {}) => {
+export const makeDecodeFromSmallcaps = (decodeOptions = {}) => {
+  const {
+    decodeRemotableFromSmallcaps = dontDecodeRemotableFromSmallcaps,
+    decodePromiseFromSmallcaps = dontDecodePromiseFromSmallcaps,
+    decodeErrorFromSmallcaps = dontDecodeErrorFromSmallcaps,
+  } = decodeOptions;
+
   /**
    * `decodeFromSmallcaps` may rely on `encoding` being the result of a
    * plain call to JSON.parse. However, it *cannot* rely on `encoding`

packages/marshal/src/rankOrder.js

@@ -114,9 +114,11 @@ const passStyleRanks = /** @type {PassStyleRanksRecord} */ (
         return trivialComparator(leftPrefixes, rightPrefixes);
       })
       .map(([passStyle, prefixes], index) => {
-        // Cover all strings that start with any character in `prefixes`.
-        // `prefixes` is already sorted, so that's
+        // Cover all strings that start with any character in `prefixes`,
+        // verifying that it is sorted so that is
         // all s such that prefixes.at(0) ≤ s < successor(prefixes.at(-1)).
+        prefixes === [...prefixes].sort().join('') ||
+          Fail`unsorted prefixes for passStyle ${q(passStyle)}: ${q(prefixes)}`;
         const cover = [
           prefixes.charAt(0),
           String.fromCharCode(prefixes.charCodeAt(prefixes.length - 1) + 1),

packages/marshal/test/test-encodePassable.js

@@ -11,35 +11,75 @@ import {
 } from '../src/encodePassable.js';
 import { compareRank, makeComparatorKit } from '../src/rankOrder.js';
 import { sample } from './test-rankOrder.js';
+import { arbPassable } from '../tools/arb-passable.js';
 
-const { Fail } = assert;
+const { Fail, quote: q } = assert;
 
-const r2e = new Map();
-const e2r = [];
+const buffers = {
+  __proto__: null,
+  r: [],
+  '?': [],
+  '!': [],
+};
+const resetBuffers = () => {
+  buffers.r = [];
+  buffers['?'] = [];
+  buffers['!'] = [];
+};
+const cursors = {
+  __proto__: null,
+  r: 0,
+  '?': 0,
+  '!': 0,
+};
+const resetCursors = () => {
+  cursors.r = 0;
+  cursors['?'] = 0;
+  cursors['!'] = 0;
+};
 
-const encodeRemotable = r => {
-  if (r2e.has(r)) {
-    return r2e.get(r);
-  }
-  const result = `r${e2r.length}`;
-  r2e.set(r, result);
-  e2r.push(r);
-  return result;
+const encodeThing = (prefix, r) => {
+  buffers[prefix].push(r);
+  // With this encoding, all things with the same prefix have the same rank
+  return prefix;
 };
 
-const decodeRemotable = e => {
-  e.startsWith('r') || Fail`unexpected encoding ${e}`;
-  const i = Number(BigInt(e.substring(1)));
-  assert(i >= 0 && i < e2r.length);
-  return e2r[i];
+const decodeThing = (prefix, e) => {
+  prefix === e ||
+    Fail`expected encoding ${q(e)} to simply be the prefix ${q(prefix)}`;
+  (cursors[prefix] >= 0 && cursors[prefix] < buffers[prefix].length) ||
+    Fail`while decoding ${q(e)}, expected cursors[${q(prefix)}], i.e., ${q(
+      cursors[prefix],
+    )} <= ${q(buffers[prefix].length)}`;
+  const thing = buffers[prefix][cursors[prefix]];
+  cursors[prefix] += 1;
+  return thing;
 };
 
 const compareRemotables = (x, y) =>
-  compareRank(encodeRemotable(x), encodeRemotable(y));
+  compareRank(encodeThing('r', x), encodeThing('r', y));
 
-const encodeKey = makeEncodePassable({ encodeRemotable });
+const encodePassableInternal = makeEncodePassable({
+  encodeRemotable: r => encodeThing('r', r),
+  encodePromise: p => encodeThing('?', p),
+  encodeError: er => encodeThing('!', er),
+});
 
-const decodeKey = makeDecodePassable({ decodeRemotable });
+const encodePassable = passable => {
+  resetBuffers();
+  return encodePassableInternal(passable);
+};
+
+const decodePassableInternal = makeDecodePassable({
+  decodeRemotable: e => decodeThing('r', e),
+  decodePromise: e => decodeThing('?', e),
+  decodeError: e => decodeThing('!', e),
+});
+
+const decodePassable = encoded => {
+  resetCursors();
+  return decodePassableInternal(encoded);
+};
 
 const { comparator: compareFull } = makeComparatorKit(compareRemotables);
 
@@ -80,50 +120,80 @@ const goldenPairs = harden([
 
 test('golden round trips', t => {
   for (const [k, e] of goldenPairs) {
-    t.is(encodeKey(k), e, 'does k encode as expected');
-    t.is(decodeKey(e), k, 'does the key round trip through the encoding');
+    t.is(encodePassable(k), e, 'does k encode as expected');
+    t.is(decodePassable(e), k, 'does the key round trip through the encoding');
   }
   // Not round trips
-  t.is(encodeKey(-0), 'f8000000000000000');
-  t.is(decodeKey('f0000000000000000'), NaN);
+  t.is(encodePassable(-0), 'f8000000000000000');
+  t.is(decodePassable('f0000000000000000'), NaN);
 });
 
-const orderInvariants = (t, x, y) => {
+const orderInvariants = (x, y) => {
   const rankComp = compareRank(x, y);
   const fullComp = compareFull(x, y);
   if (rankComp !== 0) {
-    t.is(rankComp, fullComp);
+    Object.is(rankComp, fullComp) ||
+      Fail`with rankComp ${rankComp}, expected matching fullComp: ${fullComp} for ${x} ${y}`;
   }
   if (fullComp === 0) {
-    t.is(rankComp, 0);
+    Object.is(rankComp, 0) ||
+      Fail`with fullComp 0, expected matching rankComp: ${rankComp} for ${x} ${y}`;
   } else {
-    t.assert(rankComp === 0 || rankComp === fullComp);
+    rankComp === 0 ||
+      rankComp === fullComp ||
+      Fail`with fullComp ${fullComp}, expected 0 or matching rankComp: ${rankComp} for ${x} ${y}`;
   }
-};
-
-test('order invariants', t => {
-  for (let i = 0; i < sample.length; i += 1) {
-    for (let j = i; j < sample.length; j += 1) {
-      orderInvariants(t, sample[i], sample[j]);
-    }
+  const ex = encodePassable(x);
+  const ey = encodePassable(y);
+  const encComp = compareRank(ex, ey);
+  if (fullComp !== 0) {
+    Object.is(encComp, fullComp) ||
+      Fail`with fullComp ${fullComp}, expected matching encComp: ${encComp} for ${ex} ${ey}`;
   }
-});
+};
 
-test('BigInt values round-trip', async t => {
+test('Passables round-trip', async t => {
   await fc.assert(
-    fc.property(fc.bigInt(), n => {
-      const rt = decodeKey(encodeKey(n));
-      return t.is(rt, n);
+    fc.property(arbPassable, n => {
+      const en = encodePassable(n);
+      const rt = decodePassable(en);
+      const er = encodePassable(rt);
+      t.is(en, er);
+      t.is(compareFull(n, rt), 0);
     }),
   );
 });
 
 test('BigInt encoding comparison corresponds with numeric comparison', async t => {
   await fc.assert(
     fc.property(fc.bigInt(), fc.bigInt(), (a, b) => {
-      const ea = encodeKey(a);
-      const eb = encodeKey(b);
-      return t.is(a < b, ea < eb) && t.is(a > b, ea > eb);
+      const ea = encodePassable(a);
+      const eb = encodePassable(b);
+      t.is(a < b, ea < eb);
+      t.is(a > b, ea > eb);
+    }),
+  );
+});
+
+test('Passable encoding corresponds to rankOrder', async t => {
+  await fc.assert(
+    fc.property(arbPassable, arbPassable, (a, b) => {
+      return orderInvariants(a, b);
     }),
   );
+  // Ensure at least one ava assertion.
+  t.pass();
+});
+
+// The following is logically like the test above, but rather than relying on
+// the heuristic generation of fuzzing test cases, it always checks everything
+// in `sample`.
+test('Also test against all enumerated in sample', t => {
+  for (let i = 0; i < sample.length; i += 1) {
+    for (let j = i; j < sample.length; j += 1) {
+      orderInvariants(sample[i], sample[j]);
+    }
+  }
+  // Ensure at least one ava assertion.
+  t.pass();
 });