Skip to content

Commit

Permalink
1488 Passable redux (#2238)
Browse files Browse the repository at this point in the history
closes: #1488

## Description

Reattempting:
-  #1933

Again:
- Makes `Passable` a generic type instead of `any`.
- Defines overloads for `passStyleOf` to return the actual style.
- Defines the `Key` in terms of `Passable`
- Makes a ton of fixes and suppressions in places that relied on the
previous `any`'s

### Security Considerations

n/a

### Scaling Considerations

n/a

### Documentation Considerations

Better types are better documentation.

### Testing Considerations

- [ ] Agoric/agoric-sdk#8774

### Upgrade Considerations

These changes may cause type errors downstream. I don't think we should
call those breaking change à la semver because they don't affect the
runtime.
  • Loading branch information
turadg authored May 2, 2024
2 parents a90d740 + 1ae099d commit 334b016
Show file tree
Hide file tree
Showing 49 changed files with 563 additions and 321 deletions.
5 changes: 2 additions & 3 deletions packages/captp/src/captp.js
Original file line number Diff line number Diff line change
Expand Up @@ -322,9 +322,6 @@ export const makeCapTP = (
}

const IS_REMOTE_PUMPKIN = harden({});
/**
* @type {import('@endo/marshal').ConvertSlotToVal<import('./types.js').CapTPSlot>}
*/
const assertValIsLocal = val => {
const slot = valToSlot.get(val);
if (slot && slot[1] === '-') {
Expand Down Expand Up @@ -413,6 +410,7 @@ export const makeCapTP = (
epoch,
questionID,
target,
// @ts-expect-error Type 'unknown' is not assignable to type 'Passable<PassableCap, Error>'.
method: serialize(harden([null, args])),
});
return promise;
Expand All @@ -428,6 +426,7 @@ export const makeCapTP = (
epoch,
questionID,
target,
// @ts-expect-error Type 'unknown' is not assignable to type 'Passable<PassableCap, Error>'.
method: serialize(harden([prop, args])),
});
return promise;
Expand Down
1 change: 1 addition & 0 deletions packages/captp/test/test-trap.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* global SharedArrayBuffer */
import test from '@endo/ses-ava/prepare-endo.js';

import { Worker } from 'worker_threads';
Expand Down
2 changes: 1 addition & 1 deletion packages/daemon/src/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ export interface EndoHost extends EndoAgent {
readerRef: ERef<AsyncIterableIterator<string>>,
petName: string,
): Promise<FarRef<EndoReadable>>;
storeValue<T>(value: T, petName: string): Promise<void>;
storeValue<T extends Passable>(value: T, petName: string): Promise<void>;
provideGuest(
petName?: string,
opts?: MakeHostOrGuestOptions,
Expand Down
5 changes: 3 additions & 2 deletions packages/eslint-plugin/lib/configs/internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ const dynamicConfig = {
// typescript-eslint has its own config that must be dynamically referenced
// to include vs. exclude non-"src" files because it cannot itself be dynamic.
// https://github.com/microsoft/TypeScript/issues/30751
const rootTsProjectGlob = './{js,ts}config.eslint-full.json';
const rootTsProjectGlob = './tsconfig.eslint-full.json';
const parserOptions = {
tsconfigRootDir: path.join(__dirname, '../../../..'),
project: [rootTsProjectGlob, 'packages/*/{js,ts}config.eslint.json'],
EXPERIMENTAL_useProjectService: true,
project: [rootTsProjectGlob],
};

const fileGlobs = ['**/*.{js,ts}'];
Expand Down
3 changes: 3 additions & 0 deletions packages/exo/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
export * from './src/exo-makers.js';

// eslint-disable-next-line import/export -- ESLint not aware of type exports in types.d.ts
export * from './src/types.js';

export { GET_INTERFACE_GUARD } from './src/get-interface.js';
5 changes: 4 additions & 1 deletion packages/exo/src/types.d.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { RemotableBrand } from '@endo/eventual-send';
import type { RemotableObject } from '@endo/pass-style';
import type { InterfaceGuard, MethodGuard, Pattern } from '@endo/patterns';
import type { GetInterfaceGuard } from './get-interface.js';

Expand Down Expand Up @@ -110,7 +111,9 @@ export type FarClassOptions<C, F = any> = {
*/
receiveInstanceTester?: ReceivePower<IsInstance> | undefined;
};
export type Farable<M extends Methods> = M & RemotableBrand<{}, M>;
export type Farable<M extends Methods> = M &
RemotableBrand<{}, M> &
RemotableObject;
export type Guarded<M extends Methods> = Farable<M & GetInterfaceGuard<M>>;
export type GuardedKit<F extends Record<string, Methods>> = {
[K in keyof F]: Guarded<F[K]>;
Expand Down
2 changes: 2 additions & 0 deletions packages/exo/src/types.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/** @file Empty twin for .d.ts */
export {};
5 changes: 5 additions & 0 deletions packages/far/test/test-marshal-far-obj.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ import { q } from '@endo/errors';

import { Far, passStyleOf, getInterfaceOf } from '../src/index.js';

/**
* @import {PassStyled} from '@endo/pass-style';
*/

const { create, getPrototypeOf } = Object;

// this only includes the tests that do not use liveSlots
Expand Down Expand Up @@ -129,6 +133,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
3 changes: 2 additions & 1 deletion packages/marshal/src/deeplyFulfilled.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { X, q } from '@endo/errors';
const { ownKeys } = Reflect;
const { fromEntries } = Object;

// TODO return a type contingent on the parameter as deeplyFullfilledObject from agoric-sdk does
/**
* Given a Passable `val` whose pass-by-copy structure may contain leaf
* promises, return a promise for a replacement Passable,
Expand Down Expand Up @@ -38,7 +39,7 @@ const { fromEntries } = Object;
* // revise the above comment to match.
* // See https://github.com/endojs/endo/pull/1451
*
* @param {Passable} val
* @param {any} val
* @returns {Promise<Passable>}
*/
export const deeplyFulfilled = async val => {
Expand Down
9 changes: 5 additions & 4 deletions packages/marshal/src/encodePassable.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const getSuffix = (str, index) => (index === 0 ? str : str.substring(index));
* 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 @@ -55,7 +55,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 Expand Up @@ -324,7 +324,7 @@ const decodeLegacyStringSuffix = encoded => encoded;
* format, each terminated by a space (which is part of the escaped range in
* "compactOrdered" encoded strings).
*
* @param {unknown[]} array
* @param {Passable[]} array
* @param {(p: Passable) => string} encodePassable
* @returns {string}
*/
Expand Down Expand Up @@ -399,7 +399,7 @@ const decodeCompactArray = (encoded, decodePassable, skip = 0) => {
* This necessitated an undesirable amount of iteration and expansion; see
* https://github.com/endojs/endo/pull/1260#discussion_r960369826
*
* @param {unknown[]} array
* @param {Passable[]} array
* @param {(p: Passable) => string} encodePassable
* @returns {string}
*/
Expand Down Expand Up @@ -728,6 +728,7 @@ const makeInnerDecode = (decodeStringSuffix, decodeArray, options) => {
}
}
};
// @ts-expect-error Type 'unknown' is not assignable to type 'Passable<PassableCap, Error>'.
return innerDecode;
};

Expand Down
14 changes: 6 additions & 8 deletions packages/marshal/src/encodeToCapData.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ import {
} from '@endo/pass-style';
import { X, Fail, q } from '@endo/errors';

/** @import {Passable} from '@endo/pass-style' */
/** @import {Encoding} from './types.js' */
/** @import {Remotable} from '@endo/pass-style' */
/** @import {EncodingUnion} from './types.js' */
/** @import {Passable, RemotableObject} from '@endo/pass-style' */
/** @import {Encoding, EncodingUnion} from './types.js' */

const { ownKeys } = Reflect;
const { isArray } = Array;
Expand Down Expand Up @@ -62,7 +60,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 +115,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
* @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 +267,11 @@ harden(makeEncodeToCapData);
* @property {(
* encodedRemotable: Encoding,
* decodeRecur: (e: Encoding) => Passable
* ) => (Promise|Remotable)} [decodeRemotableFromCapData]
* ) => (Promise|RemotableObject)} [decodeRemotableFromCapData]
* @property {(
* encodedPromise: Encoding,
* decodeRecur: (e: Encoding) => Passable
* ) => (Promise|Remotable)} [decodePromiseFromCapData]
* ) => (Promise|RemotableObject)} [decodePromiseFromCapData]
* @property {(
* encodedError: Encoding,
* decodeRecur: (e: Encoding) => Passable
Expand Down
4 changes: 3 additions & 1 deletion packages/marshal/src/encodeToSmallcaps.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-check
/// <reference types="ses"/>

// This module is based on the `encodePassable.js` in `@agoric/store`,
Expand Down Expand Up @@ -159,7 +160,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 Expand Up @@ -386,6 +387,7 @@ export const makeDecodeFromSmallcaps = (decodeOptions = {}) => {
encoding,
decodeFromSmallcaps,
);
// @ts-ignore XXX SmallCapsEncoding
if (passStyleOf(result) !== 'remotable') {
Fail`internal: decodeRemotableFromSmallcaps option must return a remotable: ${result}`;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/marshal/src/marshal-stringify.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ harden(stringify);

/**
* @param {string} str
* @returns {Passable}
* @returns {unknown}
*/
const parse = str =>
unserialize(
Expand Down
48 changes: 28 additions & 20 deletions packages/marshal/src/marshal.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ import {
makeEncodeToSmallcaps,
} from './encodeToSmallcaps.js';

/** @import {ConvertSlotToVal, ConvertValToSlot, FromCapData, MakeMarshalOptions, ToCapData} from './types.js' */
/** @import {Passable} from '@endo/pass-style' */
/** @import {InterfaceSpec} from '@endo/pass-style' */
/** @import {Encoding} from './types.js' */
/** @import {RemotableObject as Remotable} from '@endo/pass-style' */
/**
* @import {ConvertSlotToVal, ConvertValToSlot, FromCapData, MakeMarshalOptions, ToCapData} from './types.js';
* @import {Passable, PassableCap, RemotableObject} from '@endo/pass-style';
* @import {InterfaceSpec} from '@endo/pass-style';
* @import {Encoding} from './types.js';
*/

const { defineProperties } = Object;
const { isArray } = Array;
Expand Down Expand Up @@ -78,7 +79,7 @@ export const makeMarshal = (
const slotMap = new Map();

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

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

/** @type {(promise: 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 +187,7 @@ export const makeMarshal = (
} else if (serializeBodyFormat === 'smallcaps') {
/**
* @param {string} prefix
* @param {Passable} passable
* @param {PassableCap} passable
* @param {InterfaceSpec} [iface]
* @returns {string}
*/
Expand Down Expand Up @@ -231,19 +234,20 @@ export const makeMarshal = (
};

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

/**
* @param {{iface?: string, index: number}} slotData
* @returns {Remotable | Promise}
* @returns {PassableCap}
*/
const decodeSlotCommon = slotData => {
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 Down Expand Up @@ -280,11 +284,13 @@ export const makeMarshal = (
const dName = decodeRecur(name);
const dMessage = decodeRecur(message);
// errorId is a late addition so be tolerant of its absence.
const dErrorId = 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)}`;
const dErrorId = /** @type {string} */ (errorId && decodeRecur(errorId));
if (typeof dName !== 'string') {
throw Fail`invalid error name typeof ${q(typeof dName)}`;
}
if (typeof dMessage !== 'string') {
throw Fail`invalid error message typeof ${q(typeof dMessage)}`;
}
const errConstructor = getErrorConstructor(dName) || Error;
const errorName =
dErrorId === undefined
Expand Down Expand Up @@ -340,8 +346,8 @@ export const makeMarshal = (
const makeDecodeSlotFromSmallcaps = prefix => {
/**
* @param {string} stringEncoding
* @param {(e: unknown) => Passable} _decodeRecur
* @returns {Remotable | Promise}
* @param {(e: unknown) => PassableCap} _decodeRecur
* @returns {RemotableObject | Promise}
*/
return (stringEncoding, _decodeRecur) => {
assert(stringEncoding.charAt(0) === prefix);
Expand All @@ -364,7 +370,9 @@ export const makeMarshal = (
};

const reviveFromSmallcaps = makeDecodeFromSmallcaps({
// @ts-ignore XXX SmallCapsEncoding
decodeRemotableFromSmallcaps,
// @ts-ignore XXX SmallCapsEncoding
decodePromiseFromSmallcaps,
decodeErrorFromSmallcaps,
});
Expand Down Expand Up @@ -396,7 +404,7 @@ export const makeMarshal = (
// which should be considered fixed once we've completed the switch
// to smallcaps.
assertPassable(result);
return result;
return /** @type {PassableCap} */ (result);
};

return harden({
Expand Down
Loading

0 comments on commit 334b016

Please sign in to comment.