Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1488 Passable (REVERTED in #1961) #1933

Merged
merged 6 commits into from
Jan 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/captp/src/captp.js
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,9 @@ export const makeCapTP = (

const IS_REMOTE_PUMPKIN = harden({});
/**
* @type {import('@endo/marshal').ConvertSlotToVal<import('./types.js').CapTPSlot>}
* @type {import('@endo/marshal').ConvertValToSlot<import('./types.js').CapTPSlot>}
*/
// @ts-expect-error intentional hack
const assertValIsLocal = val => {
const slot = valToSlot.get(val);
if (slot && slot[1] === '-') {
Expand Down Expand Up @@ -499,6 +500,7 @@ export const makeCapTP = (
}

// If we imported this slot, mark it as one our peer exported.
// @ts-expect-error map lacks value type
return slotToImported.get(recvSlot.add(slot));
}

Expand Down
2 changes: 1 addition & 1 deletion packages/exo/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"extends": "../../tsconfig.eslint-base.json",
"compilerOptions": {
"maxNodeModuleJsDepth": 1
"maxNodeModuleJsDepth": 2
},
"include": [
"*.js",
Expand Down
2 changes: 2 additions & 0 deletions packages/far/test/test-marshal-far-obj.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const testRecord = ({
}),
);

/** @type {import('@endo/pass-style').PassStyled<'remotable'>} */
const goodRemotableProto = testRecord();

// @ts-ignore We're testing bad things anyway
Expand Down Expand Up @@ -129,6 +130,7 @@ test('passStyleOf validation of remotables', t => {
t.throws(() => passStyleOf(badRemotableProto3), NON_METHOD);
t.throws(() => passStyleOf(badRemotableProto4), NON_METHOD);

// @ts-expect-error UNTIL https://github.com/microsoft/TypeScript/issues/38385
t.is(passStyleOf(sub(goodRemotableProto)), 'remotable');

t.throws(() => passStyleOf(sub(badRemotableProto1)), EXPECTED_PASS_STYLE);
Expand Down
4 changes: 4 additions & 0 deletions packages/marshal/src/deeplyFulfilled.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,22 @@ export const deeplyFulfilled = async val => {
const passStyle = passStyleOf(val);
switch (passStyle) {
case 'copyRecord': {
// @ts-expect-error FIXME narrowed
const names = ownKeys(val);
const valPs = names.map(name => deeplyFulfilled(val[name]));
return E.when(Promise.all(valPs), vals =>
harden(fromEntries(vals.map((c, i) => [names[i], c]))),
);
}
case 'copyArray': {
// @ts-expect-error FIXME narrowed
const valPs = val.map(p => deeplyFulfilled(p));
return E.when(Promise.all(valPs), vals => harden(vals));
}
case 'tagged': {
// @ts-expect-error FIXME narrowed
const tag = getTag(val);
// @ts-expect-error FIXME narrowed
return E.when(deeplyFulfilled(val.payload), payload =>
makeTagged(tag, payload),
);
Expand Down
4 changes: 2 additions & 2 deletions packages/marshal/src/encodePassable.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const { ownKeys } = Reflect;
* string-named own properties. `recordNames` returns those name *reverse*
* sorted, because that's how records are compared, encoded, and sorted.
*
* @template T
* @template {Passable} T
* @param {CopyRecord<T>} record
* @returns {string[]}
*/
Expand All @@ -44,7 +44,7 @@ harden(recordNames);
* Assuming that `record` is a CopyRecord and `names` is `recordNames(record)`,
* return the corresponding array of property values.
*
* @template T
* @template {Passable} T
* @param {CopyRecord<T>} record
* @param {string[]} names
* @returns {T[]}
Expand Down
10 changes: 5 additions & 5 deletions packages/marshal/src/encodeToCapData.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {

/** @typedef {import('@endo/pass-style').Passable} Passable */
/** @typedef {import('./types.js').Encoding} Encoding */
/** @typedef {import('@endo/pass-style').Remotable} Remotable */
/** @typedef {import('@endo/pass-style').RemotableObject} RemotableObject */
/** @typedef {import('./types.js').EncodingUnion} EncodingUnion */

const { ownKeys } = Reflect;
Expand Down Expand Up @@ -62,7 +62,7 @@ const qclassMatches = (encoded, qclass) =>
/**
* @typedef {object} EncodeToCapDataOptions
* @property {(
* remotable: Remotable,
* remotable: RemotableObject,
* encodeRecur: (p: Passable) => Encoding
* ) => Encoding} [encodeRemotableToCapData]
* @property {(
Expand Down Expand Up @@ -117,7 +117,7 @@ export const makeEncodeToCapData = (encodeOptions = {}) => {
* Readers must not care about this order anyway. We impose this requirement
* mainly to reduce non-determinism exposed outside a vat.
*
* @param {Passable} passable
* @param {any} passable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious: Why did this "regress" from Passable to any?

(scare quotes because the old Passable meant any. So the real question is: Why couldn't this become the new Passable?)

Likewise for similar changes elsewhere.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the string replacements of old-Passable to any is because the new (non-any) Passable caused too many errors and I judged solving them to not be worth it. Usually because the following code relied heavily on dynamic type inspection and would not benefit from the static type information. This was mostly in switch statements where TS cannot statically infer the type narrowing being used.

* @returns {Encoding} except that `encodeToCapData` does not generally
* `harden` this result before returning. Rather, `encodeToCapData` is not
* directly exposed.
Expand Down Expand Up @@ -269,11 +269,11 @@ harden(makeEncodeToCapData);
* @property {(
* encodedRemotable: Encoding,
* decodeRecur: (e: Encoding) => Passable
* ) => (Promise|Remotable)} [decodeRemotableFromCapData]
* ) => (Promise|RemotableObject)} [decodeRemotableFromCapData]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this type renaming. Having a type name collide with a value (function) name was really terrible. Good to see this straightened out. No change suggested.

* @property {(
* encodedPromise: Encoding,
* decodeRecur: (e: Encoding) => Passable
* ) => (Promise|Remotable)} [decodePromiseFromCapData]
* ) => (Promise|RemotableObject)} [decodePromiseFromCapData]
* @property {(
* encodedError: Encoding,
* decodeRecur: (e: Encoding) => Passable
Expand Down
4 changes: 2 additions & 2 deletions packages/marshal/src/encodeToSmallcaps.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
} from '@endo/pass-style';

/** @typedef {import('@endo/pass-style').Passable} Passable */
/** @typedef {import('@endo/pass-style').Remotable} Remotable */
/** @typedef {import('@endo/pass-style').RemotableObject} Remotable */
// @typedef {import('./types.js').SmallcapsEncoding} SmallcapsEncoding */
// @typedef {import('./types.js').SmallcapsEncodingUnion} SmallcapsEncodingUnion */
/** @typedef {any} SmallcapsEncoding */
Expand Down Expand Up @@ -161,7 +161,7 @@ export const makeEncodeToSmallcaps = (encodeOptions = {}) => {
* Readers must not care about this order anyway. We impose this requirement
* mainly to reduce non-determinism exposed outside a vat.
*
* @param {Passable} passable
* @param {any} passable
* @returns {SmallcapsEncoding} except that `encodeToSmallcaps` does not generally
* `harden` this result before returning. Rather, `encodeToSmallcaps` is not
* directly exposed.
Expand Down
24 changes: 15 additions & 9 deletions packages/marshal/src/marshal.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ export const makeMarshal = (
const slotMap = new Map();

/**
* @param {Remotable | Promise} passable
* @param {import('@endo/pass-style').PassableCap} passable
* @returns {{index: number, repeat: boolean}}
*/
const encodeSlotCommon = passable => {
Expand Down Expand Up @@ -134,7 +134,7 @@ export const makeMarshal = (

if (serializeBodyFormat === 'capdata') {
/**
* @param {Passable} passable
* @param {import('@endo/pass-style').PassableCap} passable
* @param {InterfaceSpec} [iface]
* @returns {Encoding}
*/
Expand All @@ -148,9 +148,11 @@ export const makeMarshal = (
}
};

/** @type {(promise: import('@endo/pass-style').RemotableObject, encodeRecur: (p: Passable) => Encoding) => Encoding} */
const encodeRemotableToCapData = (val, _encodeRecur) =>
encodeSlotToCapData(val, getInterfaceOf(val));

/** @type {(promise: Promise, encodeRecur: (p: Passable) => Encoding) => Encoding} */
const encodePromiseToCapData = (promise, _encodeRecur) =>
encodeSlotToCapData(promise);

Expand Down Expand Up @@ -184,7 +186,7 @@ export const makeMarshal = (
} else if (serializeBodyFormat === 'smallcaps') {
/**
* @param {string} prefix
* @param {Passable} passable
* @param {import('@endo/pass-style').PassableCap} passable
* @param {InterfaceSpec} [iface]
* @returns {string}
*/
Expand Down Expand Up @@ -231,7 +233,7 @@ export const makeMarshal = (
};

const makeFullRevive = slots => {
/** @type {Map<number, Passable>} */
/** @type {Map<number, Remotable | Promise>} */
const valMap = new Map();

/**
Expand All @@ -242,8 +244,9 @@ export const makeMarshal = (
const { iface = undefined, index, ...rest } = slotData;
ownKeys(rest).length === 0 ||
Fail`unexpected encoded slot properties ${q(ownKeys(rest))}`;
if (valMap.has(index)) {
return valMap.get(index);
const extant = valMap.get(index);
if (extant) {
return extant;
}
// TODO SECURITY HAZARD: must enfoce that remotable vs promise
// is according to the encoded string.
Expand All @@ -267,11 +270,12 @@ export const makeMarshal = (
// are for reuse by other encodings that do, such as smallcaps.
const dName = decodeRecur(name);
const dMessage = decodeRecur(message);
const dErrorId = errorId && decodeRecur(errorId);
const dErrorId = /** @type {string} */ (errorId && decodeRecur(errorId));
typeof dName === 'string' ||
Fail`invalid error name typeof ${q(typeof dName)}`;
typeof dMessage === 'string' ||
Fail`invalid error message typeof ${q(typeof dMessage)}`;
if (typeof dMessage !== 'string') {
throw Fail`invalid error message typeof ${q(typeof dMessage)}`;
}
const EC = getErrorConstructor(dName) || Error;
// errorId is a late addition so be tolerant of its absence.
const errorName =
Expand Down Expand Up @@ -330,7 +334,9 @@ export const makeMarshal = (
};

const reviveFromSmallcaps = makeDecodeFromSmallcaps({
// @ts-expect-error FIXME
decodeRemotableFromSmallcaps,
// @ts-expect-error FIXME
decodePromiseFromSmallcaps,
decodeErrorFromSmallcaps,
});
Expand Down
34 changes: 26 additions & 8 deletions packages/marshal/src/rankOrder.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,16 +144,20 @@ export const makeComparatorKit = (compareRemotables = (_x, _y) => 0) => {
case 'string': {
// Within each of these passStyles, the rank ordering agrees with
// JavaScript's relational operators `<` and `>`.
// @ts-expect-error FIXME narrowed
if (left < right) {
return -1;
} else {
// @ts-expect-error FIXME narrowed
assert(left > right);
return 1;
}
}
case 'symbol': {
return comparator(
// @ts-expect-error FIXME narrowed
nameForPassableSymbol(left),
// @ts-expect-error FIXME narrowed
nameForPassableSymbol(right),
);
}
Expand All @@ -167,9 +171,11 @@ export const makeComparatorKit = (compareRemotables = (_x, _y) => 0) => {
}
// The rank ordering of non-NaN numbers agrees with JavaScript's
// relational operators '<' and '>'.
// @ts-expect-error FIXME narrowed
if (left < right) {
return -1;
} else {
// @ts-expect-error FIXME narrowed
assert(left > right);
return 1;
}
Expand All @@ -186,37 +192,46 @@ export const makeComparatorKit = (compareRemotables = (_x, _y) => 0) => {
// of these names, which we then compare lexicographically. This ensures
// that if the names of record X are a subset of the names of record Y,
// then record X will have an earlier rank and sort to the left of Y.
// @ts-expect-error FIXME narrowed
const leftNames = recordNames(left);
// @ts-expect-error FIXME narrowed
const rightNames = recordNames(right);

const result = comparator(leftNames, rightNames);
if (result !== 0) {
return result;
}
return comparator(
// @ts-expect-error FIXME narrowed
recordValues(left, leftNames),
// @ts-expect-error FIXME narrowed
recordValues(right, rightNames),
);
}
case 'copyArray': {
// Lexicographic
// @ts-expect-error FIXME narrowed
const len = Math.min(left.length, right.length);
for (let i = 0; i < len; i += 1) {
// @ts-expect-error FIXME narrowed
const result = comparator(left[i], right[i]);
if (result !== 0) {
return result;
}
}
// If all matching elements were tied, then according to their lengths.
// If array X is a prefix of array Y, then X has an earlier rank than Y.
// @ts-expect-error FIXME narrowed
return comparator(left.length, right.length);
}
case 'tagged': {
// Lexicographic by `[Symbol.toStringTag]` then `.payload`.
// @ts-expect-error FIXME narrowed
const labelComp = comparator(getTag(left), getTag(right));
if (labelComp !== 0) {
return labelComp;
}
// @ts-expect-error FIXME narrowed
return comparator(left.payload, right.payload);
}
default: {
Expand Down Expand Up @@ -286,9 +301,10 @@ harden(assertRankSorted);
* function. This is a genuine bug for us NOW because sometimes we sort
* in reverse order by passing a reversed rank comparison function.
*
* @param {Iterable<Passable>} passables
* @template {Passable} T
* @param {Iterable<T>} passables
* @param {RankCompare} compare
* @returns {Passable[]}
* @returns {T[]}
*/
export const sortByRank = (passables, compare) => {
if (Array.isArray(passables)) {
Expand Down Expand Up @@ -382,18 +398,20 @@ export const coveredEntries = (sorted, [leftIndex, rightIndex]) => {
harden(coveredEntries);

/**
* @template {Passable} T
* @param {RankCompare} compare
* @param {Passable} a
* @param {Passable} b
* @returns {Passable}
* @param {T} a
* @param {T} b
* @returns {T}
*/
const maxRank = (compare, a, b) => (compare(a, b) >= 0 ? a : b);

/**
* @template {Passable} T
* @param {RankCompare} compare
* @param {Passable} a
* @param {Passable} b
* @returns {Passable}
* @param {T} a
* @param {T} b
* @returns {T}
*/
const minRank = (compare, a, b) => (compare(a, b) <= 0 ? a : b);

Expand Down
4 changes: 2 additions & 2 deletions packages/marshal/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,8 @@ export {};
* ordering would also compare magnitudes, and so agree with the rank ordering
* of all values other than `NaN`. An array sorted by rank would enable range
* queries by magnitude.
* @param {any} left a Passable
* @param {any} right a Passable
* @param {import("@endo/pass-style").Passable} left
* @param {import("@endo/pass-style").Passable} right
* @returns {RankComparison}
*/

Expand Down
2 changes: 2 additions & 0 deletions packages/marshal/test/test-marshal-capdata.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ test('records', t => {
const fauxPresence = harden({});
const { serialize: ser, unserialize: unser } = makeMarshal(
_val => 'slot',
// @ts-expect-error mock
_slot => fauxPresence,
{
errorTagging: 'off',
Expand Down Expand Up @@ -404,6 +405,7 @@ test('capdata proto problems', t => {
test('capdata slot leniency', t => {
const { unserialize: fromCapData } = makeMarshal(
undefined,
// @ts-expect-error mock
_slot => ({
name: 'I should not be in a slot',
}),
Expand Down
1 change: 1 addition & 0 deletions packages/marshal/test/test-marshal-far-obj.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ test('passStyleOf validation of remotables', t => {
t.throws(() => passStyleOf(badRemotableProto3), NON_METHOD);
t.throws(() => passStyleOf(badRemotableProto4), NON_METHOD);

// @ts-expect-error UNTIL https://github.com/microsoft/TypeScript/issues/38385
t.is(passStyleOf(sub(goodRemotableProto)), 'remotable');

t.throws(() => passStyleOf(sub(badRemotableProto1)), EXPECTED_PASS_STYLE);
Expand Down
1 change: 1 addition & 0 deletions packages/marshal/test/test-marshal-smallcaps.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ test('smallcaps records', t => {
const fauxPresence = harden({});
const { serialize: ser, unserialize: unser } = makeMarshal(
_val => 'slot',
// @ts-expect-error mock
_slot => fauxPresence,
{
errorTagging: 'off',
Expand Down
Loading