From f648918ca3d9dd49ee7b6537914f9361363c4d21 Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Wed, 21 Sep 2022 13:15:50 -0700 Subject: [PATCH] fix: tools arb passable --- packages/marshal/package.json | 4 +- packages/marshal/src/encodePassable.js | 4 +- packages/marshal/src/rankOrder.js | 6 +- packages/marshal/test/test-encodePassable.js | 118 +++++++++--- packages/marshal/test/test-rankOrder.js | 183 +++++++------------ packages/marshal/tools/arb-passable.js | 110 +++++++++++ 6 files changed, 276 insertions(+), 149 deletions(-) create mode 100644 packages/marshal/tools/arb-passable.js diff --git a/packages/marshal/package.json b/packages/marshal/package.json index 62270ca1dc..bb64b16a7b 100644 --- a/packages/marshal/package.json +++ b/packages/marshal/package.json @@ -38,13 +38,13 @@ "dependencies": { "@endo/eventual-send": "^0.16.8", "@endo/nat": "^4.1.23", - "@endo/promise-kit": "^0.2.52" + "@endo/promise-kit": "^0.2.52", + "@fast-check/ava": "^1.0.1" }, "devDependencies": { "@endo/init": "^0.5.52", "@endo/lockdown": "^0.1.24", "@endo/ses-ava": "^0.2.36", - "@fast-check/ava": "^1.0.1", "ava": "^5.1.0", "c8": "^7.7.3" }, diff --git a/packages/marshal/src/encodePassable.js b/packages/marshal/src/encodePassable.js index 93a7887577..fa9e0557b6 100644 --- a/packages/marshal/src/encodePassable.js +++ b/packages/marshal/src/encodePassable.js @@ -320,7 +320,7 @@ export const makeEncodePassable = ({ encodeError = (err, _) => assert.fail(X`error unexpected: ${err}`), } = {}) => { const encodePassable = passable => { - if (ErrorHelper.canBeValid(passable, x => x)) { + if (ErrorHelper.canBeValid(passable)) { return encodeError(passable, encodePassable); } const passStyle = passStyleOf(passable); @@ -485,7 +485,7 @@ harden(isEncodedRemotable); * individually is a valid bigint prefix. `n` for "negative" and `p` for * "positive". The ordering of these prefixes is the same as the * rankOrdering of their respective PassStyles. This table is imported by - * randOrder.js for this purpose. + * rankOrder.js for this purpose. * * In addition, `|` is the remotable->ordinal mapping prefix: * This is not used in covers but it is diff --git a/packages/marshal/src/rankOrder.js b/packages/marshal/src/rankOrder.js index a9ec707638..50659171bf 100644 --- a/packages/marshal/src/rankOrder.js +++ b/packages/marshal/src/rankOrder.js @@ -115,9 +115,11 @@ const passStyleRanks = /** @type {PassStyleRanksRecord} */ (fromEntries( 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), diff --git a/packages/marshal/test/test-encodePassable.js b/packages/marshal/test/test-encodePassable.js index 73c1480655..4aa545cde3 100644 --- a/packages/marshal/test/test-encodePassable.js +++ b/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 { details: X } = 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 => { - assert(e.startsWith('r'), X`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 encodePassable = passable => { + resetBuffers(); + return encodePassableInternal(passable); +}; -const decodeKey = makeDecodePassable({ decodeRemotable }); +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,12 +120,12 @@ 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) => { @@ -99,6 +139,12 @@ const orderInvariants = (t, x, y) => { } else { t.assert(rankComp === 0 || rankComp === fullComp); } + const ex = encodePassable(x); + const ey = encodePassable(y); + const encComp = compareRank(ex, ey); + if (fullComp !== 0) { + t.is(encComp, fullComp); + } }; test('order invariants', t => { @@ -109,11 +155,14 @@ test('order invariants', t => { } }); -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); }), ); }); @@ -121,9 +170,18 @@ test('BigInt values round-trip', async t => { 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) => { + orderInvariants(t, a, b); }), ); }); diff --git a/packages/marshal/test/test-rankOrder.js b/packages/marshal/test/test-rankOrder.js index d896d502c3..56d2eca624 100644 --- a/packages/marshal/test/test-rankOrder.js +++ b/packages/marshal/test/test-rankOrder.js @@ -14,75 +14,19 @@ import { } from '../src/rankOrder.js'; import { makeTagged } from '../src/makeTagged.js'; -import { Far } from '../src/make-far.js'; -const { quote: q } = assert; - -/** - * The only elements with identity. Everything else should be equal - * by contents. - */ -const alice = Far('alice', {}); -const bob = Far('bob', {}); -const carol = Far('carol', {}); +import { + arbPassable, + exampleAlice, + exampleBob, + exampleCarol, +} from '../tools/arb-passable.js'; -/** - * A factory for arbitrary passables - */ -const { passable } = fc.letrec(tie => { - return { - passable: tie('dag').map(x => harden(x)), - dag: fc.oneof( - { depthFactor: 0.5, withCrossShrink: true }, - // a tagged value whose payload is an array of [key, leaf] pairs - // where each key is unique within the payload - // XXX can the payload be generalized further? - fc - .record({ - type: fc.constantFrom('copyMap', 'copySet', 'nonsense'), - payload: fc - .uniqueArray(fc.fullUnicodeString(), { maxLength: 3 }) - .chain(k => { - return fc.tuple(fc.constant(k), tie('leaf')); - }), - }) - .map(({ type, payload }) => makeTagged(type, payload)), - fc.array(tie('dag'), { maxLength: 3 }), - fc.dictionary( - fc.fullUnicodeString().filter(s => s !== 'then'), - tie('dag'), - { maxKeys: 3 }, - ), - tie('dag').map(v => Promise.resolve(v)), - tie('leaf'), - ), - leaf: fc.oneof( - fc.record({}), - fc.fullUnicodeString(), - fc.fullUnicodeString().map(s => Symbol.for(s)), - fc.fullUnicodeString().map(s => new Error(s)), - // primordial symbols and registered lookalikes - fc.constantFrom( - ...Object.getOwnPropertyNames(Symbol).flatMap(k => { - const v = Symbol[k]; - if (typeof v !== 'symbol') return []; - return [v, Symbol.for(k), Symbol.for(`@@${k}`)]; - }), - ), - fc.bigInt(), - fc.integer(), - fc.constantFrom(-0, NaN, Infinity, -Infinity), - fc.constantFrom(null, undefined, false, true), - fc.constantFrom(alice, bob, carol), - // unresolved promise - fc.constant(new Promise(() => {})), - ), - }; -}); +const { quote: q } = assert; test('compareRank is reflexive', async t => { await fc.assert( - fc.property(passable, x => { + fc.property(arbPassable, x => { return t.is(compareRank(x, x), 0); }), ); @@ -90,7 +34,7 @@ test('compareRank is reflexive', async t => { test('compareRank totally orders ranks', async t => { await fc.assert( - fc.property(passable, passable, (a, b) => { + fc.property(arbPassable, arbPassable, (a, b) => { const ab = compareRank(a, b); const ba = compareRank(b, a); if (ab === 0) { @@ -105,15 +49,12 @@ test('compareRank totally orders ranks', async t => { ); }); -// TODO Had to remove key-level cases from the test-encodePassable.js as -// migrated to endo. As a result, some of the tests here are broken. -// Fix. -test.skip('compareRank is transitive', async t => { +test('compareRank is transitive', async t => { await fc.assert( fc.property( // operate on a set of three passables covering at least two ranks fc - .uniqueArray(passable, { minLength: 3, maxLength: 3 }) + .uniqueArray(arbPassable, { minLength: 3, maxLength: 3 }) .filter( ([a, b, c]) => compareRank(a, b) !== 0 || compareRank(a, c) !== 0, ), @@ -122,39 +63,44 @@ test.skip('compareRank is transitive', async t => { assertRankSorted(sorted, compareRank); const [a, b, c] = sorted; const failures = []; - let result; - let resultOk; - result = compareRank(a, b); - resultOk = t.true(result <= 0, 'a <= b'); - if (!resultOk) { - failures.push(`Expected <= 0: ${result} from ${q(a)} vs. ${q(b)}`); - } - result = compareRank(a, c); - resultOk = t.true(result <= 0, 'a <= c'); - if (!resultOk) { - failures.push(`Expected <= 0: ${result} from ${q(a)} vs. ${q(c)}`); - } - result = compareRank(b, c); - resultOk = t.true(result <= 0, 'b <= c'); - if (!resultOk) { - failures.push(`Expected <= 0: ${result} from ${q(b)} vs. ${q(c)}`); - } - result = compareRank(c, b); - resultOk = t.true(result >= 0, 'c >= b'); - if (!resultOk) { - failures.push(`Expected >= 0: ${result} from ${q(c)} vs. ${q(b)}`); - } - result = compareRank(c, a); - resultOk = t.true(result >= 0, 'c >= a'); - if (!resultOk) { - failures.push(`Expected >= 0: ${result} from ${q(c)} vs. ${q(a)}`); - } - result = compareRank(b, a); - resultOk = t.true(result >= 0, 'b >= a'); - if (!resultOk) { - failures.push(`Expected >= 0: ${result} from ${q(b)} vs. ${q(a)}`); - } + const testCompare = (outcome, message, failure) => { + t.true(outcome, message); + if (!outcome) { + failures.push(failure); + } + }; + + testCompare( + compareRank(a, b) <= 0, + 'a <= b', + `Expected <= 0: ${q(a)} vs. ${q(b)}`, + ); + testCompare( + compareRank(a, c) <= 0, + 'a <= c', + `Expected <= 0: ${q(a)} vs. ${q(c)}`, + ); + testCompare( + compareRank(b, c) <= 0, + 'b <= c', + `Expected <= 0: ${q(b)} vs. ${q(c)}`, + ); + testCompare( + compareRank(c, b) >= 0, + 'c >= b', + `Expected >= 0: ${q(c)} vs. ${q(b)}`, + ); + testCompare( + compareRank(c, a) >= 0, + 'c >= a', + `Expected >= 0: ${q(c)} vs. ${q(a)}`, + ); + testCompare( + compareRank(b, a) >= 0, + 'b >= a', + `Expected >= 0: ${q(b)} vs. ${q(a)}`, + ); return t.deepEqual(failures, []); }, @@ -178,7 +124,7 @@ export const sample = harden([ 2, null, [5, { foo: 4, bar: null }], - bob, + exampleBob, 0, makeTagged('copySet', [ ['a', 4], @@ -189,7 +135,7 @@ export const sample = harden([ undefined, -Infinity, [5], - alice, + exampleAlice, [], Symbol.for('foo'), new Error('not erroneous'), @@ -197,7 +143,7 @@ export const sample = harden([ [5, { bar: 5 }], Symbol.for(''), false, - carol, + exampleCarol, -0, {}, [5, undefined], @@ -219,6 +165,12 @@ export const sample = harden([ [5, { foo: 4, bar: undefined }], Promise.resolve('fulfillment'), [5, { foo: 4 }], + // The promises should be of the same rank, in which case + // the singleton array should be earlier. But if the encoded + // gives the earlier promise an earlier encoding (as it used to), + // then the encoded forms will not be order preserving. + [Promise.resolve(null), 'x'], + [Promise.resolve(null)], ]); const rejectedP = Promise.reject(new Error('broken')); @@ -261,6 +213,8 @@ const sortedSample = harden([ // Lexicographic records by reverse sorted property name, then by values // in that order. [], + [Promise.resolve(null)], + [Promise.resolve(null), 'x'], [5], [5, { bar: 5 }], [5, { foo: 4 }], @@ -287,9 +241,9 @@ const sortedSample = harden([ // All remotables are tied for the same rank and the sort is stable, // so their relative order is preserved - bob, - alice, - carol, + exampleBob, + exampleAlice, + exampleCarol, // Lexicographic strings. Shorter beats longer. // TODO Probe UTF-16 vs Unicode vs UTF-8 (Moddable) ordering. @@ -318,7 +272,8 @@ test('compare and sort by rank', t => { ); }); -const rangeSample = harden([ +// Unused in that it is used only in a skipped test +const unusedRangeSample = harden([ {}, // 0 -- prefix are earlier, so empty is earliest { bar: null }, // 1 { bar: undefined }, // 2 -- records with same names grouped together @@ -338,7 +293,9 @@ const rangeSample = harden([ ]); /** @type {[RankCover, IndexCover][]} */ -const queries = harden([ +// @ts-expect-error Stale from when RankCover was a pair of extreme values +// rather than a pair of strings to be compared to passable encodings. +const brokenQueries = harden([ [ [['c'], ['c']], // first > last implies absent. @@ -368,9 +325,9 @@ const queries = harden([ // adding composite key handling to the durable store implementation) will need // to re-enable and (likely) update this test. test.skip('range queries', t => { - t.assert(isRankSorted(rangeSample, compareRank)); - for (const [rankCover, indexRange] of queries) { - const range = getIndexCover(rangeSample, compareRank, rankCover); + t.assert(isRankSorted(unusedRangeSample, compareRank)); + for (const [rankCover, indexRange] of brokenQueries) { + const range = getIndexCover(unusedRangeSample, compareRank, rankCover); t.is(range[0], indexRange[0]); t.is(range[1], indexRange[1]); } diff --git a/packages/marshal/tools/arb-passable.js b/packages/marshal/tools/arb-passable.js new file mode 100644 index 0000000000..c5d4bbaf78 --- /dev/null +++ b/packages/marshal/tools/arb-passable.js @@ -0,0 +1,110 @@ +// @ts-check +import '../src/types.js'; +import { fc } from '@fast-check/ava'; +import { Far } from '../src/make-far.js'; +import { makeTagged } from '../src/makeTagged.js'; + +/** + * The only elements with identity. Everything else should be equal + * by contents. + */ +export const exampleAlice = Far('alice', {}); +export const exampleBob = Far('bob', {}); +export const exampleCarol = Far('carol', {}); + +export const arbString = fc.oneof(fc.string(), fc.fullUnicodeString()); + +export const arbLeaf = fc.oneof( + fc.constantFrom(null, undefined, false, true), + arbString, + arbString.map(s => Symbol.for(s)), + // primordial symbols and registered lookalikes + fc.constantFrom( + ...Object.getOwnPropertyNames(Symbol).flatMap(k => { + const v = Symbol[k]; + if (typeof v !== 'symbol') return []; + return [v, Symbol.for(k), Symbol.for(`@@${k}`)]; + }), + ), + fc.bigInt(), + fc.integer(), + fc.constantFrom(-0, NaN, Infinity, -Infinity), + fc.record({}), + fc.constantFrom(exampleAlice, exampleBob, exampleCarol), + arbString.map(s => new Error(s)), + // unresolved promise + fc.constant(new Promise(() => {})), +); + +const { arbDag } = fc.letrec(tie => { + return { + arbDag: fc.oneof( + { withCrossShrink: true }, + arbLeaf, + tie('arbDag').map(v => Promise.resolve(v)), + fc.array(tie('arbDag')), + fc.dictionary( + arbString.filter(s => s !== 'then'), + tie('arbDag'), + ), + // A tagged value, either of arbitrary type with arbitrary payload + // or of known type with arbitrary or explicitly valid payload. + // Ordered by increasing complexity. + fc + .oneof( + fc.record({ type: arbString, payload: tie('arbDag') }), + fc.record({ + type: fc.constantFrom('copySet'), + payload: fc.oneof( + tie('arbDag'), + // copySet valid payload is an array of unique passables. + // TODO: A valid copySet payload must be a reverse sorted array, + // so we should generate some of those as well. + fc.uniqueArray(tie('arbDag')), + ), + }), + fc.record({ + type: fc.constantFrom('copyBag'), + payload: fc.oneof( + tie('arbDag'), + // copyBag valid payload is an array of [passable, count] tuples + // in which each passable is unique. + // TODO: A valid copyBag payload must be a reverse sorted array, + // so we should generate some of those as well. + fc.uniqueArray(fc.tuple(tie('arbDag'), fc.bigInt()), { + selector: entry => entry[0], + }), + ), + }), + fc.record({ + type: fc.constantFrom('copyMap'), + payload: fc.oneof( + tie('arbDag'), + // copyMap valid payload is a + // `{ keys: Passable[], values: Passable[]}` + // record in which keys are unique and both arrays have the + // same length. + // TODO: In a valid copyMap payload, the keys must be a + // reverse sorted array, so we should generate some of + // those as well. + fc + .uniqueArray( + fc.record({ key: tie('arbDag'), value: tie('arbDag') }), + { selector: entry => entry.key }, + ) + .map(entries => ({ + keys: entries.map(({ key }) => key), + values: entries.map(({ value }) => value), + })), + ), + }), + ) + .map(({ type, payload }) => makeTagged(type, payload)), + ), + }; +}); + +/** + * A factory for arbitrary passables + */ +export const arbPassable = arbDag.map(x => harden(x));