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

typecheck vats package #8596

Merged
merged 7 commits into from
Dec 4, 2023
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: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
"@jessie.js/eslint-plugin": "^0.4.0",
"@types/express": "^4.17.17",
"@types/node": "^16.13.0",
"@typescript-eslint/eslint-plugin": "^6.7.0",
"@typescript-eslint/parser": "^6.7.0",
"@typescript-eslint/eslint-plugin": "^6.13.2",
"@typescript-eslint/parser": "^6.13.2",
"ava": "^5.3.0",
"c8": "^7.13.0",
"conventional-changelog-conventionalcommits": "^4.6.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/eslint-config/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
"peerDependencies": {
"@endo/eslint-plugin": "^0.5.1",
"@jessie.js/eslint-plugin": "^0.4.0",
"@typescript-eslint/eslint-plugin": "^6.7.0",
"@typescript-eslint/parser": "^6.7.0",
"@typescript-eslint/eslint-plugin": "^6.13.2",
"@typescript-eslint/parser": "^6.13.2",
"eslint": "^8.47.0",
"eslint-config-airbnb-base": "^15.0.0",
"eslint-plugin-github": "^4.10.0",
Expand Down
3 changes: 2 additions & 1 deletion packages/governance/src/contractHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ const facetHelpers = (zcf, paramManager) => {
*
* @see {makeDurableGovernorFacet}
*
* @param {{ [methodName: string]: (context?: unknown, ...rest: unknown[]) => unknown}} limitedCreatorFacet
* @template {{ [methodName: string]: (context?: unknown, ...rest: unknown[]) => unknown}} LCF
* @param {LCF} limitedCreatorFacet
Copy link
Member

Choose a reason for hiding this comment

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

There seems to be an endless amount of refinement to do for the types for governance.

I wonder if really complex types is like really complex documentation: it suggests searching for a simpler design.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed. The design we shipped with was somewhat rushed. Making it durable is also rushed (and incomplete). When making it durable we could simplify, or do that in a next phase.

*/
const makeVirtualGovernorFacet = limitedCreatorFacet => {
/** @type {import('@agoric/swingset-liveslots').FunctionsPlusContext<unknown, GovernedCreatorFacet<limitedCreatorFacet>>} */
Expand Down
1 change: 0 additions & 1 deletion packages/inter-protocol/src/proposals/startPSM.js
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,6 @@ export const startPSM = async (
governorFacets.creatorFacet,
instanceKey,
),
// @ts-expect-error TODO type for provisionPoolStartResult
E(E.get(provisionPoolStartResult).creatorFacet).initPSM(
anchorBrand,
newpsmKit.psm,
Expand Down
1 change: 1 addition & 0 deletions packages/inter-protocol/test/psm/setupPsm.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export const setupPsm = async (
issuer.produce.IST.resolve(istIssuer);

space.produce.provisionPoolStartResult.resolve({
// @ts-expect-error mock
creatorFacet: Far('dummy', {
initPSM: () => {
t.log('dummy provisionPool.initPSM');
Expand Down
10 changes: 8 additions & 2 deletions packages/swingset-liveslots/src/types.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Ensure this is a module.
export {};

/**
* @callback makeLiveSlots
*/
Expand Down Expand Up @@ -79,5 +82,8 @@
*
*/

// Ensure this is a module.
export {};
/**
* @template V fulfilled value
* @template {any[]} [A=unknown[]] arguments
* @typedef { {onFulfilled?: (value: V, ...args: A) => void, onRejected?: (reason: unknown, ...args: A) => void} } PromiseWatcher
*/
67 changes: 48 additions & 19 deletions packages/swingset-liveslots/src/watchedPromises.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// @ts-check
// no-lonely-if is a stupid rule that really should be disabled globally
/* eslint-disable no-lonely-if */

Expand All @@ -6,6 +7,12 @@ import { initEmpty, M } from '@agoric/store';
import { E } from '@endo/eventual-send';
import { parseVatSlot } from './parseVatSlots.js';

/**
* @template V
* @template {any[]} [A=unknown[]]
* @typedef {[watcher: import('./types.js').PromiseWatcher<V, A>, ...args: A]} PromiseWatcherTuple
*/

/**
* @param {object} options
* @param {*} options.syscall
Expand All @@ -28,17 +35,28 @@ export function makeWatchedPromiseManager({
const { makeScalarBigMapStore } = collectionManager;
const { defineDurableKind } = vom;

// virtual Store (not durable) mapping vpid to Promise objects, to
// maintain the slotToVal registration until resolution. Without
// this, slotToVal would forget local Promises that aren't exported.
/**
* virtual Store (not durable) mapping vpid to Promise objects, to
* maintain the slotToVal registration until resolution. Without
* this, slotToVal would forget local Promises that aren't exported.
*
* @type {MapStore<string, Promise<unknown>>}
*/
let promiseRegistrations;

// watched promises by vpid: each entry is an array of watches on the
// corresponding vpid; each of these is in turn an array of a watcher object
// and the arguments associated with it by `watchPromise`.
/**
* watched promises by vpid: each entry is an array of watches on the
* corresponding vpid; each of these is in turn an array of a watcher object
* and the arguments associated with it by `watchPromise`.
* @type {MapStore<string, PromiseWatcherTuple<unknown>[]>}
*/
let watchedPromiseTable;

// defined promise watcher objects indexed by kindHandle
/**
* defined promise watcher objects indexed by kindHandle
*
* @type {MapStore<import('./vatDataTypes.js').DurableKindHandle, import('./types.js').PromiseWatcher<unknown>>}
*/
let promiseWatcherByKindTable;

function preparePromiseWatcherTables() {
Expand Down Expand Up @@ -73,11 +91,17 @@ export function makeWatchedPromiseManager({
}

/**
*
* @param {Promise<unknown>} p
* @template T
* @param {Promise<T>} p
* @param {string} vpid
* @returns {void}
*/
function pseudoThen(p, vpid) {
/**
*
* @param {T} value
* @param {boolean} wasFulfilled
*/
function settle(value, wasFulfilled) {
const watches = watchedPromiseTable.get(vpid);
watchedPromiseTable.delete(vpid);
Expand Down Expand Up @@ -121,20 +145,28 @@ export function makeWatchedPromiseManager({
}
}

/**
* @template V
* @template {any[]} A]
* @param {import('./vatDataTypes.js').DurableKindHandle} kindHandle
* @param {(value: V, ...args: A) => void} fulfillHandler
* @param {(reason: any, ...args: A) => void} rejectHandler
* @returns {import('./types.js').PromiseWatcher<V, A>}
*/
function providePromiseWatcher(
kindHandle,
fulfillHandler = x => x,
rejectHandler = x => {
throw x;
Copy link
Member Author

Choose a reason for hiding this comment

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

@FUDCo removing this throw make sense to you? I think when when a rejectHandler throws that ends up as an unhandled promise rejection

Copy link
Contributor

Choose a reason for hiding this comment

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

At first glance I thought you were right, but on further inspection I'm not sure that's so (though I'm not deeply confident of that conclusion). In particular, it looks to me like there's a call chain from startVat that can result in somebody catching the exception.

On the other hand, I think the default fulfillHandler is questionable, since from the way it's invoked it doesn't look to me like there's ever going to be anybody positioned to receive the return value.

},
// @ts-expect-error xxx rest params in typedef
fulfillHandler = _value => {},
// @ts-expect-error xxx rest params in typedef
rejectHandler = _reason => {},
) {
assert.typeof(fulfillHandler, 'function');
assert.typeof(rejectHandler, 'function');

const makeWatcher = defineDurableKind(kindHandle, initEmpty, {
// @ts-expect-error TS is confused by the spread operator
/** @type {(context: unknown, res: V, ...args: A) => void} */
onFulfilled: (_context, res, ...args) => fulfillHandler(res, ...args),
// @ts-expect-error
/** @type {(context: unknown, rej: unknown, ...args: A) => void} */
onRejected: (_context, rej, ...args) => rejectHandler(rej, ...args),
});

Expand All @@ -148,10 +180,7 @@ export function makeWatchedPromiseManager({
}

/**
*
* @param {Promise} p
* @param {{onFulfilled?: Function, onRejected?: Function}} watcher
* @param {...any} args
* @type {<P extends Promise<any>, A extends any[]>(p: P, watcher: import('./types.js').PromiseWatcher<Awaited<P>, A>, ...args: A) => void}
*/
function watchPromise(p, watcher, ...args) {
// The following wrapping defers setting up the promise watcher itself to a
Expand Down
1 change: 1 addition & 0 deletions packages/swingset-liveslots/tools/setup-vat-data.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ globalThis.VatData = harden({
makeKindHandle: tag => fakeVomKit.vom.makeKindHandle(tag),
canBeDurable: (...args) => fakeVomKit.vom.canBeDurable(...args),
providePromiseWatcher: (...args) =>
// @ts-expect-error spread argument for non-rest parameter
fakeVomKit.wpm.providePromiseWatcher(...args),
watchPromise: (p, watcher, ...args) =>
fakeVomKit.wpm.watchPromise(p, watcher, ...args),
Expand Down
20 changes: 20 additions & 0 deletions packages/vat-data/src/index.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
makeKindHandle,
defineDurableKind,
partialAssign,
watchPromise,
} from '.';

// for use in assignments below
Expand Down Expand Up @@ -168,3 +169,22 @@ const vom: VirtualObjectManager = anyVal;
vom.missingMethod;
// @ts-expect-error Expected 0-4 arguments but got 5
vom.defineDurableKind(anyVal, anyVal, anyVal, anyVal, 'extra');

const p: Promise<bigint> = anyVal;
watchPromise(
p,
{
onFulfilled(value, extra1, extra2) {
expectType<bigint>(value);
expectType<string>(extra1);
// @ts-expect-error str
expectType<number>(extra2);
},
onRejected(reason, extra1) {
expectType<unknown>(reason);
expectType<string>(extra1);
},
},
'extraString',
'alsoString',
);
1 change: 0 additions & 1 deletion packages/vats/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-check
// Ambient types
import '@agoric/zoe/exported.js';
import './src/core/types-ambient.js';
Expand Down
2 changes: 1 addition & 1 deletion packages/vats/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,6 @@
"workerThreads": false
},
"typeCoverage": {
"atLeast": 90.86
"atLeast": 90.88
Copy link
Member

Choose a reason for hiding this comment

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

👏

}
}
2 changes: 0 additions & 2 deletions packages/vats/src/bridge.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @ts-check

import { M } from '@agoric/store';
import '@agoric/store/exported.js';
import { E } from '@endo/far';
Expand Down
2 changes: 0 additions & 2 deletions packages/vats/src/centralSupply.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @ts-check

import { AmountMath } from '@agoric/ertp';
import { E, Far } from '@endo/far';

Expand Down
2 changes: 0 additions & 2 deletions packages/vats/src/core/basic-behaviors.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @ts-check

import { AssetKind } from '@agoric/ertp';
import { CONTRACT_ELECTORATE, ParamTypes } from '@agoric/governance';
import { Stable, Stake } from '@agoric/internal/src/tokens.js';
Expand Down
1 change: 0 additions & 1 deletion packages/vats/src/core/boot-chain.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-check
import { makeDurableZone } from '@agoric/zone/durable.js';
import { makeBootstrap } from './lib-boot.js';

Expand Down
1 change: 0 additions & 1 deletion packages/vats/src/core/boot-sim.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-check
import { makeBootstrap } from './lib-boot.js';

import * as basicBehaviorsPlus from './basic-behaviors.js';
Expand Down
1 change: 0 additions & 1 deletion packages/vats/src/core/boot-solo.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-check
import { makeBootstrap } from './lib-boot.js';

import * as basicBehaviorsPlus from './basic-behaviors.js';
Expand Down
1 change: 0 additions & 1 deletion packages/vats/src/core/chain-behaviors.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* global globalThis */
// @ts-check
import { allValues, BridgeId as BRIDGE_ID } from '@agoric/internal';
import * as STORAGE_PATH from '@agoric/internal/src/chain-storage-paths.js';
import { makePrioritySendersManager } from '@agoric/internal/src/priority-senders.js';
Expand Down
1 change: 0 additions & 1 deletion packages/vats/src/core/client-behaviors.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-check
import { E, Far } from '@endo/far';
import { makePluginManager } from '@agoric/swingset-vat/src/vats/plugin-manager.js';
import { observeNotifier } from '@agoric/notifier';
Expand Down
1 change: 0 additions & 1 deletion packages/vats/src/core/lib-boot.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-check
import { E, Far } from '@endo/far';
import { makeHeapZone } from '@agoric/zone';
import {
Expand Down
1 change: 0 additions & 1 deletion packages/vats/src/core/promise-space.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-check
import { E } from '@endo/far';
import { assertKey } from '@agoric/store';
import { canBeDurable } from '@agoric/vat-data';
Expand Down
1 change: 0 additions & 1 deletion packages/vats/src/core/startWalletFactory.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-check
import { makeMap } from 'jessie.js';
import { E, Far } from '@endo/far';
import { deeplyFulfilled } from '@endo/marshal';
Expand Down
4 changes: 3 additions & 1 deletion packages/vats/src/core/types-ambient.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,9 @@ type ChainBootstrapSpaceT = {
testFirstAnchorKit: import('../vat-bank.js').AssetIssuerKit<'nat'>;
walletBridgeManager: import('../types.js').ScopedBridgeManager | undefined;
walletFactoryStartResult: import('./startWalletFactory.js').WalletFactoryStartResult;
provisionPoolStartResult: unknown;
provisionPoolStartResult: GovernanceFacetKit<
typeof import('@agoric/inter-protocol/src/provisionPool.js').start
>;
vatStore: import('./utils.js').VatStore;
zoe: ZoeService;
};
Expand Down
1 change: 0 additions & 1 deletion packages/vats/src/core/utils.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-check
import { Stable, Stake } from '@agoric/internal/src/tokens.js';
import { WalletName } from '@agoric/internal';
import { E, Far } from '@endo/far';
Expand Down
1 change: 0 additions & 1 deletion packages/vats/src/crc.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
* Forked from npm [email protected] for module system compatibility, then trimmed
* down to a version that assumes TypedArrays are universal.
*/
// @ts-check
/* eslint-disable no-bitwise */

/** @typedef {string | number | Uint8Array | ArrayBuffer} Data */
Expand Down
2 changes: 0 additions & 2 deletions packages/vats/src/ibc.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @ts-check

import { makeScalarMapStore, makeLegacyMap } from '@agoric/store';
import { makePromiseKit } from '@endo/promise-kit';
import { assert, details as X, Fail } from '@agoric/assert';
Expand Down
1 change: 0 additions & 1 deletion packages/vats/src/lib-board.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-check
/**
* @file lib-board: share objects by way of plain-data ids
* @see prepareBoardKit()
Expand Down
1 change: 0 additions & 1 deletion packages/vats/src/mintHolder.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-check
// @jessie-check

import { prepareIssuerKit } from '@agoric/ertp';
Expand Down
2 changes: 0 additions & 2 deletions packages/vats/src/nameHub.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @ts-check

import { assert, NonNullish } from '@agoric/assert';
import { E } from '@endo/far';
import { makePromiseKit } from '@endo/promise-kit';
Expand Down
1 change: 0 additions & 1 deletion packages/vats/src/proposals/network-proposal.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-check
import { E, Far } from '@endo/far';
import { BridgeId as BRIDGE_ID } from '@agoric/internal';
import {
Expand Down
6 changes: 5 additions & 1 deletion packages/vats/src/proposals/null-upgrade-zoe-proposal.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import { E } from '@endo/far';
* @param {BootstrapPowers & {
* consume: {
* vatAdminSvc: VatAdminSvc;
* vatStore: MapStore<string, CreateVatResults>;
* vatStore: MapStore<
* string,
* import('@agoric/swingset-vat').CreateVatResults
* >;
* };
* }} powers
* @param {object} options
Expand All @@ -16,6 +19,7 @@ export const nullUpgradeZoe = async (
) => {
const { zoeRef } = options.options;

assert(zoeRef.bundleID);
const zoeBundleCap = await E(vatAdminSvc).getBundleCap(zoeRef.bundleID);
console.log(`ZOE BUNDLE ID: `, zoeRef.bundleID);

Expand Down
2 changes: 0 additions & 2 deletions packages/vats/src/proposals/restart-vats-proposal.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// @ts-check

import { Fail } from '@agoric/assert';
import { deeplyFulfilledObject, makeTracer } from '@agoric/internal';
import { M, mustMatch } from '@agoric/store';
Expand Down
6 changes: 5 additions & 1 deletion packages/vats/src/proposals/zcf-proposal.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import { E } from '@endo/far';
* @param {BootstrapPowers & {
* consume: {
* vatAdminSvc: VatAdminSvc;
* vatStore: MapStore<string, CreateVatResults>;
* vatStore: MapStore<
* string,
* import('@agoric/swingset-vat').CreateVatResults
* >;
* };
* }} powers
* @param {object} options
Expand All @@ -16,6 +19,7 @@ export const upgradeZcf = async (
) => {
const { zoeRef, zcfRef } = options.options;

assert(zoeRef.bundleID);
const zoeBundleCap = await E(vatAdminSvc).getBundleCap(zoeRef.bundleID);
console.log(`ZOE BUNDLE ID: `, zoeRef.bundleID);

Expand Down
1 change: 0 additions & 1 deletion packages/vats/src/repl.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// @ts-check
import { isPromise } from '@endo/promise-kit';
import { E, Far } from '@endo/far';
import * as farExports from '@endo/far';
Expand Down
Loading
Loading