fix(marshal)!: Update compareRank to terminate comparability at the f…

…irst remotable

To be refinable into a total order that distinguishes remotables,
`compareRank` must consider `[r1, 'x']` vs. `[r2, 'y']` as a tie rather
than as equivalent to `[0, 'x']` vs. `[0, 'y']`, in case r1 vs. r2 ends
up comparing differently than 'x' vs. 'y'.

Fixes #2588

packages/marshal/src/rankOrder.js

@@ -8,7 +8,7 @@ import {
 
 /**
  * @import {Passable, PassStyle} from '@endo/pass-style'
- * @import {FullCompare, RankCompare, RankCover} from './types.js'
+ * @import {FullCompare, PartialCompare, PartialComparison, RankCompare, RankCover} from './types.js'
  */
 
 const { entries, fromEntries, setPrototypeOf, is } = Object;
@@ -101,15 +101,16 @@ const memoOfSorted = new WeakMap();
 const comparatorMirrorImages = new WeakMap();
 
 /**
- * @param {RankCompare=} compareRemotables
- * An option to create a comparator in which an internal order is
- * assigned to remotables. This defaults to a comparator that
- * always returns `0`, meaning that all remotables are tied
- * for the same rank.
+ * @param {PartialCompare} [compareRemotables]
+ * A comparator for assigning an internal order to remotables.
+ * It defaults to a function that always returns `NaN`, meaning that all
+ * remotables are incomparable and should tie for the same rank without further
+ * refinement (e.g., not only are `r1` and `r2` tied, but so are `[r1, 0]`
+ * and `[r2, "x"]`).
  * @returns {RankComparatorKit}
  */
-export const makeComparatorKit = (compareRemotables = (_x, _y) => 0) => {
-  /** @type {RankCompare} */
+export const makeComparatorKit = (compareRemotables = (_x, _y) => NaN) => {
+  /** @type {PartialCompare} */
   const comparator = (left, right) => {
     if (sameValueZero(left, right)) {
       return 0;
@@ -191,10 +192,9 @@ export const makeComparatorKit = (compareRemotables = (_x, _y) => 0) => {
         if (result !== 0) {
           return result;
         }
-        return comparator(
-          recordValues(left, leftNames),
-          recordValues(right, rightNames),
-        );
+        const leftValues = recordValues(left, leftNames);
+        const rightValues = recordValues(right, rightNames);
+        return comparator(leftValues, rightValues);
       }
       case 'copyArray': {
         // Lexicographic
@@ -225,14 +225,18 @@ export const makeComparatorKit = (compareRemotables = (_x, _y) => 0) => {
   };
 
   /** @type {RankCompare} */
-  const antiComparator = (x, y) => comparator(y, x);
+  const outerComparator = (x, y) =>
+    /** @type {Exclude<PartialComparison, NaN>} */ (comparator(x, y) || 0);
+
+  /** @type {RankCompare} */
+  const antiComparator = (x, y) => outerComparator(y, x);
 
-  memoOfSorted.set(comparator, new WeakSet());
+  memoOfSorted.set(outerComparator, new WeakSet());
   memoOfSorted.set(antiComparator, new WeakSet());
-  comparatorMirrorImages.set(comparator, antiComparator);
-  comparatorMirrorImages.set(antiComparator, comparator);
+  comparatorMirrorImages.set(outerComparator, antiComparator);
+  comparatorMirrorImages.set(antiComparator, outerComparator);
 
-  return harden({ comparator, antiComparator });
+  return harden({ comparator: outerComparator, antiComparator });
 };
 /**
  * @param {RankCompare} comparator

packages/marshal/test/encodePassable.test.js

@@ -17,7 +17,7 @@ import { unsortedSample } from './_marshal-test-data.js';
 
 const statelessEncodePassableLegacy = makeEncodePassable();
 
-const makeSimplePassableKit = ({ stateless = false } = {}) => {
+const makeSimplePassableKit = ({ statelessSuffix } = {}) => {
   let count = 0n;
   const encodingFromVal = new Map();
   const valFromEncoding = new Map();
@@ -40,11 +40,11 @@ const makeSimplePassableKit = ({ stateless = false } = {}) => {
     return val;
   };
 
-  const encoders = stateless
+  const encoders = statelessSuffix !== undefined
     ? {
-        encodeRemotable: r => 'r',
-        encodePromise: p => '?',
-        encodeError: err => '!',
+        encodeRemotable: r => `r${statelessSuffix}`,
+        encodePromise: p => `?${statelessSuffix}`,
+        encodeError: err => `!${statelessSuffix}`,
       }
     : {
         encodeRemotable: r => encodeSpecial('r', r),
@@ -336,7 +336,26 @@ test('compact custom encoding validity constraints', t => {
   }
 });
 
-const orderInvariants = (x, y, statelessEncodePassable) => {
+const commonPrefix = (str1, str2) => {
+  const iter1 = str1[Symbol.iterator]();
+  const iter2 = str2[Symbol.iterator]();
+  let i = 0;
+  for (;;) {
+    const { value: char1 } = iter1.next();
+    const { value: char2 } = iter2.next();
+    if (char1 === undefined) {
+      // str1 is a prefix of str2.
+      return str1;
+    } else if (char2 === undefined) {
+      // str2 is a prefix of str1.
+      return str2;
+    }
+    if (char1 !== char2) return str1.substring(0, i);
+    i += char1.length;
+  }
+};
+
+const orderInvariants = (x, y, statelessEncode1, statelessEncode2) => {
   const rankComp = compareRank(x, y);
   const fullComp = compareFull(x, y);
   if (rankComp !== 0) {
@@ -352,10 +371,14 @@ const orderInvariants = (x, y, statelessEncodePassable) => {
       rankComp === fullComp ||
       Fail`with fullComp ${fullComp}, expected rankComp 0 or matching: ${rankComp} for ${x} vs. ${y}`;
   }
-  const ex = statelessEncodePassable(x);
-  const ey = statelessEncodePassable(y);
-  const encComp = compareRank(ex, ey);
+  const ex = statelessEncode1(x);
+  const ey = statelessEncode1(y);
   if (fullComp !== 0) {
+    // Comparability of encodings stops at the first incomparable special rank
+    // (remotable/promise/error).
+    const exPrefix = commonPrefix(ex, statelessEncode2(x));
+    const eyPrefix = commonPrefix(ey, statelessEncode2(y));
+    const encComp = compareRank(exPrefix, eyPrefix);
     encComp === 0 ||
       encComp === fullComp ||
       Fail`with fullComp ${fullComp}, expected matching stateless encComp: ${encComp} for ${x} as ${ex} vs. ${y} as ${ey}`;
@@ -380,7 +403,7 @@ test('original encoding round-trips', testRoundTrip, pickLegacy);
 test('small encoding round-trips', testRoundTrip, pickCompact);
 
 const testBigInt = test.macro(async (t, pickEncode) => {
-  const kit = makeSimplePassableKit({ stateless: true });
+  const kit = makeSimplePassableKit({ statelessSuffix: '' });
   const encodePassable = pickEncode(kit);
   await fc.assert(
     fc.property(fc.bigInt(), fc.bigInt(), (x, y) => {
@@ -403,18 +426,20 @@ test(
 );
 
 const testOrderInvariants = test.macro(async (t, pickEncode) => {
-  const kit = makeSimplePassableKit({ stateless: true });
-  const statelessEncodePassable = pickEncode(kit);
+  const kit1 = makeSimplePassableKit({ statelessSuffix: '' });
+  const statelessEncode1 = pickEncode(kit1);
+  const kit2 = makeSimplePassableKit({ statelessSuffix: '.' });
+  const statelessEncode2 = pickEncode(kit2);
 
   for (const x of unsortedSample) {
     for (const y of unsortedSample) {
-      orderInvariants(x, y, statelessEncodePassable);
+      orderInvariants(x, y, statelessEncode1, statelessEncode2);
     }
   }
 
   await fc.assert(
     fc.property(arbPassable, arbPassable, (x, y) => {
-      return orderInvariants(x, y, statelessEncodePassable);
+      return orderInvariants(x, y, statelessEncode1, statelessEncode2);
     }),
   );