From 339925415b7ae7c960ecae53a79e4f02dc3b4425 Mon Sep 17 00:00:00 2001 From: Kris Kowal Date: Wed, 10 Apr 2024 16:57:42 -0700 Subject: [PATCH 1/5] refactor(daemon): Move worker internal facet to side table --- packages/daemon/src/daemon.js | 55 ++++++++++++++----------------- packages/daemon/test/test-endo.js | 3 +- 2 files changed, 25 insertions(+), 33 deletions(-) diff --git a/packages/daemon/src/daemon.js b/packages/daemon/src/daemon.js index e3051bcc08..0c9588f7dd 100644 --- a/packages/daemon/src/daemon.js +++ b/packages/daemon/src/daemon.js @@ -109,6 +109,8 @@ const makeDaemonCore = async ( } = powers; const { randomHex512 } = cryptoPowers; const contentStore = persistencePowers.makeContentSha512Store(); + /** @type {WeakMap>} */ + const workerDaemonFacets = new WeakMap(); /** * Mutations of the formula graph must be serialized through this queue. * "Mutations" include: @@ -313,9 +315,12 @@ const makeDaemonCore = async ( {}, ); + // @ts-expect-error Evidently not specific enough. + workerDaemonFacets.set(worker, workerDaemonFacet); + return { external: worker, - internal: workerDaemonFacet, + internal: undefined, }; }; @@ -355,17 +360,13 @@ const makeDaemonCore = async ( context.thisDiesIfThatDies(id); } - const workerController = - /** @type {import('./types.js').Controller} */ ( - // Behold, recursion: - // eslint-disable-next-line no-use-before-define - provideController(workerId) - ); - const workerDaemonFacet = workerController.internal; - assert( - workerDaemonFacet, - `panic: No internal bootstrap for worker ${workerId}`, + const worker = /** @type {import('./worker.js').WorkerBootstrap} */ ( + // Behold, recursion: + // eslint-disable-next-line no-use-before-define + await provide(workerId) ); + const workerDaemonFacet = workerDaemonFacets.get(worker); + assert(workerDaemonFacet, `Cannot evaluate using non-worker`); const endowmentValues = await Promise.all( ids.map(id => @@ -428,17 +429,13 @@ const makeDaemonCore = async ( context.thisDiesIfThatDies(workerId); context.thisDiesIfThatDies(powersId); - const workerController = - /** @type {import('./types.js').Controller} */ ( - // Behold, recursion: - // eslint-disable-next-line no-use-before-define - provideController(workerId) - ); - const workerDaemonFacet = workerController.internal; - assert( - workerDaemonFacet, - `panic: No internal bootstrap for worker ${workerId}`, + const worker = /** @type {import('./worker.js').WorkerBootstrap} */ ( + // Behold, recursion: + // eslint-disable-next-line no-use-before-define + await provide(workerId) ); + const workerDaemonFacet = workerDaemonFacets.get(worker); + assert(workerDaemonFacet, 'Cannot make unconfined plugin with non-worker'); // Behold, recursion: // eslint-disable-next-line no-use-before-define const powersP = provide(powersId); @@ -466,17 +463,13 @@ const makeDaemonCore = async ( context.thisDiesIfThatDies(workerId); context.thisDiesIfThatDies(powersId); - const workerController = - /** @type {import('./types.js').Controller} */ ( - // Behold, recursion: - // eslint-disable-next-line no-use-before-define - provideController(workerId) - ); - const workerDaemonFacet = workerController.internal; - assert( - workerDaemonFacet, - `panic: No internal bootstrap for worker ${workerId}`, + const worker = /** @type {import('./worker.js').WorkerBootstrap} */ ( + // Behold, recursion: + // eslint-disable-next-line no-use-before-define + await provide(workerId) ); + const workerDaemonFacet = workerDaemonFacets.get(worker); + assert(workerDaemonFacet, 'Cannot make caplet with non-worker'); const readableBundleP = /** @type {Promise} */ ( // Behold, recursion: diff --git a/packages/daemon/test/test-endo.js b/packages/daemon/test/test-endo.js index 8fc359f3ed..3f7832cf12 100644 --- a/packages/daemon/test/test-endo.js +++ b/packages/daemon/test/test-endo.js @@ -266,8 +266,7 @@ test('spawning a worker does not overwrite existing non-worker name', async t => // with a new worker. await E(host).provideWorker('foo'); await t.throwsAsync(() => E(host).evaluate('foo', '20', [], [], 'bar'), { - message: - 'Cannot deliver "evaluate" to target; typeof target is "undefined"', + message: 'Cannot evaluate using non-worker', }); }); From e683b8ab2def7feb570a2c8ad1c2a727b3d6c4fd Mon Sep 17 00:00:00 2001 From: Kris Kowal Date: Wed, 10 Apr 2024 17:13:10 -0700 Subject: [PATCH 2/5] refactor(daemon): Use agent public facet for introduction --- packages/daemon/src/host.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/daemon/src/host.js b/packages/daemon/src/host.js index a220538373..41d5a5a5af 100644 --- a/packages/daemon/src/host.js +++ b/packages/daemon/src/host.js @@ -324,21 +324,21 @@ export const makeHostMaker = ({ * Attempts to introduce the given names to the specified agent. The agent in question * must be formulated before this function is called. * - * @param {string} id - The agent's formula identifier. + * @param {string} agentId - The agent's formula identifier. * @param {Record} introducedNames - The names to introduce. * @returns {Promise} */ - const introduceNamesToAgent = async (id, introducedNames) => { - /** @type {import('./types.js').Controller} */ - const controller = provideController(id); - const { petStore: newPetStore } = await controller.internal; + const introduceNamesToAgent = async (agentId, introducedNames) => { + const agent = /** @type {import('./types.js').EndoAgent} */ ( + await provide(agentId) + ); await Promise.all( Object.entries(introducedNames).map(async ([parentName, childName]) => { const introducedId = petStore.identifyLocal(parentName); if (introducedId === undefined) { return; } - await newPetStore.write(childName, introducedId); + await agent.write([childName], introducedId); }), ); }; From 6633e8b5ac4542d1f660ffed7a49acc6b671afe9 Mon Sep 17 00:00:00 2001 From: Kris Kowal Date: Wed, 10 Apr 2024 17:13:55 -0700 Subject: [PATCH 3/5] refactor(daemon): Eliminate internal facets --- packages/daemon/src/daemon.js | 35 +++++++++++----------------------- packages/daemon/src/guest.js | 5 +---- packages/daemon/src/types.d.ts | 21 +++++++------------- 3 files changed, 19 insertions(+), 42 deletions(-) diff --git a/packages/daemon/src/daemon.js b/packages/daemon/src/daemon.js index 0c9588f7dd..da2a7083c3 100644 --- a/packages/daemon/src/daemon.js +++ b/packages/daemon/src/daemon.js @@ -320,7 +320,6 @@ const makeDaemonCore = async ( return { external: worker, - internal: undefined, }; }; @@ -337,7 +336,6 @@ const makeDaemonCore = async ( text, json, }), - internal: undefined, }; }; @@ -392,7 +390,7 @@ const makeDaemonCore = async ( // That might mean racing two formulas and terminating the evaluator // if it turns out the value can be captured. - return { external, internal: undefined }; + return { external }; }; /** @@ -411,7 +409,7 @@ const makeDaemonCore = async ( const hub = provide(hubId); // @ts-expect-error calling lookup on an unknown object const external = E(hub).lookup(...path); - return { external, internal: undefined }; + return { external }; }; /** @@ -445,7 +443,7 @@ const makeDaemonCore = async ( /** @type {any} */ (powersP), /** @type {any} */ (makeFarContext(context)), ); - return { external, internal: undefined }; + return { external }; }; /** @@ -485,7 +483,7 @@ const makeDaemonCore = async ( /** @type {any} */ (powersP), /** @type {any} */ (makeFarContext(context)), ); - return { external, internal: undefined }; + return { external }; }; /** @@ -571,10 +569,7 @@ const makeDaemonCore = async ( ); const handle = agent.handle(); agentIdForHandle.set(handle, formula.agent); - return { - external: handle, - internal: undefined, - }; + return { external: handle }; } else if (formula.type === 'endo') { // Gateway is equivalent to E's "nonce locator". It provides a value for // a formula identifier to a remote client. @@ -649,20 +644,14 @@ const makeDaemonCore = async ( await knownPeers.write(nodeIdentifier, peerId); }, }); - return { - external: endoBootstrap, - internal: undefined, - }; + return { external: endoBootstrap }; } else if (formula.type === 'loopback-network') { // Behold, forward-reference: const loopbackNetwork = makeLoopbackNetwork({ // eslint-disable-next-line no-use-before-define provide, }); - return { - external: loopbackNetwork, - internal: undefined, - }; + return { external: loopbackNetwork }; } else if (formula.type === 'least-authority') { const disallowedFn = async () => { throw new Error('not allowed'); @@ -697,14 +686,14 @@ const makeDaemonCore = async ( ) ) ); - return { external: leastAuthority, internal: undefined }; + return { external: leastAuthority }; } else if (formula.type === 'pet-store') { const external = petStorePowers.makeIdentifiedPetStore( formulaNumber, 'pet-store', assertPetName, ); - return { external, internal: undefined }; + return { external }; } else if (formula.type === 'known-peers-store') { const external = petStorePowers.makeIdentifiedPetStore( formulaNumber, @@ -713,12 +702,12 @@ const makeDaemonCore = async ( // (i.e. formula numbers) as "names". assertValidNumber, ); - return { external, internal: undefined }; + return { external }; } else if (formula.type === 'pet-inspector') { // Behold, unavoidable forward-reference: // eslint-disable-next-line no-use-before-define const external = makePetStoreInspector(formula.petStore); - return { external, internal: undefined }; + return { external }; } else if (formula.type === 'directory') { // Behold, forward-reference: // eslint-disable-next-line no-use-before-define @@ -787,7 +776,6 @@ const makeDaemonCore = async ( } return value; }), - internal: E.get(partial).internal, }); controllerForId.set(id, controller); @@ -830,7 +818,6 @@ const makeDaemonCore = async ( controller = harden({ context, external: E.get(partial).external, - internal: E.get(partial).internal, }); controllerForId.set(id, controller); diff --git a/packages/daemon/src/guest.js b/packages/daemon/src/guest.js index 3786be4482..e5a3a00378 100644 --- a/packages/daemon/src/guest.js +++ b/packages/daemon/src/guest.js @@ -119,11 +119,8 @@ export const makeGuestMaker = ({ provide, makeMailbox, makeDirectoryNode }) => { followMessages: () => makeIteratorRef(guest.followMessages()), }, ); - const internal = harden({ - petStore, - }); - return harden({ external, internal }); + return harden({ external }); }; return makeIdentifiedGuestController; diff --git a/packages/daemon/src/types.d.ts b/packages/daemon/src/types.d.ts index 0cca16ab69..efcf197294 100644 --- a/packages/daemon/src/types.d.ts +++ b/packages/daemon/src/types.d.ts @@ -342,18 +342,16 @@ export interface FarContext { addDisposalHook: Context['onCancel']; } -export interface InternalExternal { +export interface InternalExternal { external: External; - internal: Internal; } -export interface ControllerPartial { +export interface ControllerPartial { external: Promise; - internal: Promise; } -export interface Controller - extends ControllerPartial { +export interface Controller + extends ControllerPartial { context: Context; } @@ -471,8 +469,8 @@ export type MakeHostOrGuestOptions = { export interface EndoPeer { provide: (id: string) => Promise; } -export type EndoPeerControllerPartial = ControllerPartial; -export type EndoPeerController = Controller; +export type EndoPeerControllerPartial = ControllerPartial; +export type EndoPeerController = Controller; export interface EndoGateway { provide: (id: string) => Promise; @@ -551,12 +549,7 @@ export interface EndoHost extends EndoAgent { addPeerInfo(peerInfo: PeerInfo): Promise; } -export interface InternalEndoAgent { - petStore: PetStore; -} - -export interface EndoHostController - extends Controller, InternalEndoAgent> {} +export interface EndoHostController extends Controller> {} export type EndoInspector = { lookup: (petName: Record) => Promise; From c16b07ff663faef6a1f2e347d7896b74dd12717e Mon Sep 17 00:00:00 2001 From: Kris Kowal Date: Wed, 10 Apr 2024 17:47:56 -0700 Subject: [PATCH 4/5] refactor(daemon): Unbox partial controllers --- packages/daemon/src/daemon.js | 196 +++++++++++------------------- packages/daemon/src/directory.js | 5 +- packages/daemon/src/guest.js | 8 +- packages/daemon/src/host.js | 13 +- packages/daemon/test/test-endo.js | 2 +- 5 files changed, 84 insertions(+), 140 deletions(-) diff --git a/packages/daemon/src/daemon.js b/packages/daemon/src/daemon.js index da2a7083c3..37631ec443 100644 --- a/packages/daemon/src/daemon.js +++ b/packages/daemon/src/daemon.js @@ -272,7 +272,7 @@ const makeDaemonCore = async ( * @param {string} workerId512 * @param {import('./types.js').Context} context */ - const makeIdentifiedWorkerController = async (workerId512, context) => { + const makeIdentifiedWorker = async (workerId512, context) => { // TODO validate workerId512 const daemonWorkerFacet = makeWorkerBootstrap(workerId512); @@ -318,25 +318,21 @@ const makeDaemonCore = async ( // @ts-expect-error Evidently not specific enough. workerDaemonFacets.set(worker, workerDaemonFacet); - return { - external: worker, - }; + return worker; }; /** * @param {string} sha512 */ - const makeControllerForReadableBlob = sha512 => { + const makeReadableBlob = sha512 => { const { text, json, streamBase64 } = contentStore.fetch(sha512); - return { - /** @type {import('./types.js').FarEndoReadable} */ - external: Far(`Readable file with SHA-512 ${sha512.slice(0, 8)}...`, { - sha512: () => sha512, - streamBase64, - text, - json, - }), - }; + /** @type {import('./types.js').FarEndoReadable} */ + return Far(`Readable file with SHA-512 ${sha512.slice(0, 8)}...`, { + sha512: () => sha512, + streamBase64, + text, + json, + }); }; /** @@ -346,13 +342,7 @@ const makeDaemonCore = async ( * @param {Array} ids * @param {import('./types.js').Context} context */ - const makeControllerForEval = async ( - workerId, - source, - codeNames, - ids, - context, - ) => { + const makeEval = async (workerId, source, codeNames, ids, context) => { context.thisDiesIfThatDies(workerId); for (const id of ids) { context.thisDiesIfThatDies(id); @@ -374,7 +364,7 @@ const makeDaemonCore = async ( ), ); - const external = E(workerDaemonFacet).evaluate( + return E(workerDaemonFacet).evaluate( source, codeNames, endowmentValues, @@ -389,8 +379,6 @@ const makeDaemonCore = async ( // the indirect formula resolves. // That might mean racing two formulas and terminating the evaluator // if it turns out the value can be captured. - - return { external }; }; /** @@ -401,15 +389,14 @@ const makeDaemonCore = async ( * @param {string[]} path * @param {import('./types.js').Context} context */ - const makeControllerForLookup = async (hubId, path, context) => { + const makeLookup = async (hubId, path, context) => { context.thisDiesIfThatDies(hubId); // Behold, recursion: // eslint-disable-next-line no-use-before-define const hub = provide(hubId); // @ts-expect-error calling lookup on an unknown object - const external = E(hub).lookup(...path); - return { external }; + return E(hub).lookup(...path); }; /** @@ -418,12 +405,7 @@ const makeDaemonCore = async ( * @param {string} specifier * @param {import('./types.js').Context} context */ - const makeControllerForUnconfinedPlugin = async ( - workerId, - powersId, - specifier, - context, - ) => { + const makeUnconfined = async (workerId, powersId, specifier, context) => { context.thisDiesIfThatDies(workerId); context.thisDiesIfThatDies(powersId); @@ -437,13 +419,12 @@ const makeDaemonCore = async ( // Behold, recursion: // eslint-disable-next-line no-use-before-define const powersP = provide(powersId); - const external = E(workerDaemonFacet).makeUnconfined( + return E(workerDaemonFacet).makeUnconfined( specifier, // TODO fix type /** @type {any} */ (powersP), /** @type {any} */ (makeFarContext(context)), ); - return { external }; }; /** @@ -452,12 +433,7 @@ const makeDaemonCore = async ( * @param {string} bundleId * @param {import('./types.js').Context} context */ - const makeControllerForSafeBundle = async ( - workerId, - powersId, - bundleId, - context, - ) => { + const makeBundle = async (workerId, powersId, bundleId, context) => { context.thisDiesIfThatDies(workerId); context.thisDiesIfThatDies(powersId); @@ -477,13 +453,12 @@ const makeDaemonCore = async ( // Behold, recursion: // eslint-disable-next-line no-use-before-define const powersP = provide(powersId); - const external = E(workerDaemonFacet).makeBundle( + return E(workerDaemonFacet).makeBundle( readableBundleP, // TODO fix type /** @type {any} */ (powersP), /** @type {any} */ (makeFarContext(context)), ); - return { external }; }; /** @@ -492,14 +467,9 @@ const makeDaemonCore = async ( * @param {import('./types.js').Formula} formula * @param {import('./types.js').Context} context */ - const makeControllerForFormula = async ( - id, - formulaNumber, - formula, - context, - ) => { + const evaluateFormula = async (id, formulaNumber, formula, context) => { if (formula.type === 'eval') { - return makeControllerForEval( + return makeEval( formula.worker, formula.source, formula.names, @@ -507,20 +477,20 @@ const makeDaemonCore = async ( context, ); } else if (formula.type === 'readable-blob') { - return makeControllerForReadableBlob(formula.content); + return makeReadableBlob(formula.content); } else if (formula.type === 'lookup') { - return makeControllerForLookup(formula.hub, formula.path, context); + return makeLookup(formula.hub, formula.path, context); } else if (formula.type === 'worker') { - return makeIdentifiedWorkerController(formulaNumber, context); + return makeIdentifiedWorker(formulaNumber, context); } else if (formula.type === 'make-unconfined') { - return makeControllerForUnconfinedPlugin( + return makeUnconfined( formula.worker, formula.powers, formula.specifier, context, ); } else if (formula.type === 'make-bundle') { - return makeControllerForSafeBundle( + return makeBundle( formula.worker, formula.powers, formula.bundle, @@ -529,7 +499,7 @@ const makeDaemonCore = async ( } else if (formula.type === 'host') { // Behold, recursion: // eslint-disable-next-line no-use-before-define - const controller = await makeIdentifiedHost( + const agent = await makeHost( id, formula.handle, formula.petStore, @@ -541,14 +511,13 @@ const makeDaemonCore = async ( platformNames, context, ); - const { external: agent } = controller; const handle = agent.handle(); agentIdForHandle.set(handle, id); - return controller; + return agent; } else if (formula.type === 'guest') { // Behold, recursion: // eslint-disable-next-line no-use-before-define - const controller = await makeIdentifiedGuestController( + const agent = await makeGuest( id, formula.handle, formula.hostAgent, @@ -557,10 +526,9 @@ const makeDaemonCore = async ( formula.worker, context, ); - const { external: agent } = controller; const handle = agent.handle(); agentIdForHandle.set(handle, id); - return controller; + return agent; } else if (formula.type === 'handle') { const agent = /** @type {import('./types.js').EndoAgent} */ ( // Behold, recursion: @@ -569,7 +537,7 @@ const makeDaemonCore = async ( ); const handle = agent.handle(); agentIdForHandle.set(handle, formula.agent); - return { external: handle }; + return handle; } else if (formula.type === 'endo') { // Gateway is equivalent to E's "nonce locator". It provides a value for // a formula identifier to a remote client. @@ -644,70 +612,62 @@ const makeDaemonCore = async ( await knownPeers.write(nodeIdentifier, peerId); }, }); - return { external: endoBootstrap }; + return endoBootstrap; } else if (formula.type === 'loopback-network') { // Behold, forward-reference: - const loopbackNetwork = makeLoopbackNetwork({ - // eslint-disable-next-line no-use-before-define - provide, - }); - return { external: loopbackNetwork }; + // eslint-disable-next-line no-use-before-define + return makeLoopbackNetwork({ provide }); } else if (formula.type === 'least-authority') { const disallowedFn = async () => { throw new Error('not allowed'); }; - const leastAuthority = - /** @type {import('@endo/far').FarRef} */ ( - /** @type {unknown} */ ( - makeExo( - 'EndoGuest', - M.interface('EndoGuest', {}, { defaultGuards: 'passable' }), - { - has: disallowedFn, - identify: disallowedFn, - list: disallowedFn, - followChanges: disallowedFn, - lookup: disallowedFn, - reverseLookup: disallowedFn, - write: disallowedFn, - remove: disallowedFn, - move: disallowedFn, - copy: disallowedFn, - listMessages: disallowedFn, - followMessages: disallowedFn, - resolve: disallowedFn, - reject: disallowedFn, - adopt: disallowedFn, - dismiss: disallowedFn, - request: disallowedFn, - send: disallowedFn, - makeDirectory: disallowedFn, - }, - ) + return /** @type {import('@endo/far').FarRef} */ ( + /** @type {unknown} */ ( + makeExo( + 'EndoGuest', + M.interface('EndoGuest', {}, { defaultGuards: 'passable' }), + { + has: disallowedFn, + identify: disallowedFn, + list: disallowedFn, + followChanges: disallowedFn, + lookup: disallowedFn, + reverseLookup: disallowedFn, + write: disallowedFn, + remove: disallowedFn, + move: disallowedFn, + copy: disallowedFn, + listMessages: disallowedFn, + followMessages: disallowedFn, + resolve: disallowedFn, + reject: disallowedFn, + adopt: disallowedFn, + dismiss: disallowedFn, + request: disallowedFn, + send: disallowedFn, + makeDirectory: disallowedFn, + }, ) - ); - return { external: leastAuthority }; + ) + ); } else if (formula.type === 'pet-store') { - const external = petStorePowers.makeIdentifiedPetStore( + return petStorePowers.makeIdentifiedPetStore( formulaNumber, 'pet-store', assertPetName, ); - return { external }; } else if (formula.type === 'known-peers-store') { - const external = petStorePowers.makeIdentifiedPetStore( + return petStorePowers.makeIdentifiedPetStore( formulaNumber, 'known-peers-store', // The known peers store is just a pet store that only accepts node identifiers // (i.e. formula numbers) as "names". assertValidNumber, ); - return { external }; } else if (formula.type === 'pet-inspector') { // Behold, unavoidable forward-reference: // eslint-disable-next-line no-use-before-define - const external = makePetStoreInspector(formula.petStore); - return { external }; + return makePetStoreInspector(formula.petStore); } else if (formula.type === 'directory') { // Behold, forward-reference: // eslint-disable-next-line no-use-before-define @@ -744,7 +704,8 @@ const makeDaemonCore = async ( assertValidFormulaType(formula.type); // TODO further validation - return makeControllerForFormula(id, formulaNumber, formula, context); + const value = evaluateFormula(id, formulaNumber, formula, context); + return { external: value }; }; /** @type {import('./types.js').DaemonCore['formulate']} */ @@ -780,12 +741,8 @@ const makeDaemonCore = async ( controllerForId.set(id, controller); // The controller _must_ be constructed in the synchronous prelude of this function. - const controllerValue = makeControllerForFormula( - id, - formulaNumber, - formula, - context, - ); + const value = evaluateFormula(id, formulaNumber, formula, context); + const controllerValue = { external: value }; // Ensure that failure to flush the formula to storage // causes a rejection for both the controller and the value. @@ -1480,7 +1437,6 @@ const makeDaemonCore = async ( * @param {string} networksDirectoryId * @param {string[]} addresses * @param {import('./types.js').Context} context - * @returns {Promise} */ const makePeer = async (networksDirectoryId, addresses, context) => { // TODO race networks that support protocol for connection @@ -1499,20 +1455,14 @@ const makeDaemonCore = async ( address, makeFarContext(context), ); - const external = Promise.resolve({ + return { + /** @param {string} remoteId */ provide: remoteId => { return /** @type {Promise} */ ( E(remoteGateway).provide(remoteId) ); }, - }); - const internal = Promise.resolve(undefined); - // const internal = { - // receive, // TODO - // respond, // TODO - // lookupPath, // TODO - // }; - return harden({ internal, external }); + }; } } } @@ -1550,13 +1500,13 @@ const makeDaemonCore = async ( const makeMailbox = makeMailboxMaker({ provide }); - const makeIdentifiedGuestController = makeGuestMaker({ + const makeGuest = makeGuestMaker({ provide, makeMailbox, makeDirectoryNode, }); - const makeIdentifiedHost = makeHostMaker({ + const makeHost = makeHostMaker({ provide, provideController, cancelValue, diff --git a/packages/daemon/src/directory.js b/packages/daemon/src/directory.js index 470a1eeb6c..ce634e0d76 100644 --- a/packages/daemon/src/directory.js +++ b/packages/daemon/src/directory.js @@ -236,7 +236,7 @@ export const makeDirectoryMaker = ({ ); const directory = makeDirectoryNode(petStore); - const external = makeExo( + return makeExo( 'EndoDirectory', M.interface('EndoDirectory', {}, { defaultGuards: 'passable' }), { @@ -244,9 +244,6 @@ export const makeDirectoryMaker = ({ followChanges: () => makeIteratorRef(directory.followChanges()), }, ); - const internal = harden({}); - - return harden({ external, internal }); }; return { makeIdentifiedDirectory, makeDirectoryNode }; diff --git a/packages/daemon/src/guest.js b/packages/daemon/src/guest.js index e5a3a00378..f0f2cc2fcd 100644 --- a/packages/daemon/src/guest.js +++ b/packages/daemon/src/guest.js @@ -21,7 +21,7 @@ export const makeGuestMaker = ({ provide, makeMailbox, makeDirectoryNode }) => { * @param {string} mainWorkerId * @param {import('./types.js').Context} context */ - const makeIdentifiedGuestController = async ( + const makeGuest = async ( guestId, handleId, hostAgentId, @@ -110,7 +110,7 @@ export const makeGuestMaker = ({ provide, makeMailbox, makeDirectoryNode }) => { deliver, }; - const external = makeExo( + return makeExo( 'EndoGuest', M.interface('EndoGuest', {}, { defaultGuards: 'passable' }), { @@ -119,9 +119,7 @@ export const makeGuestMaker = ({ provide, makeMailbox, makeDirectoryNode }) => { followMessages: () => makeIteratorRef(guest.followMessages()), }, ); - - return harden({ external }); }; - return makeIdentifiedGuestController; + return makeGuest; }; diff --git a/packages/daemon/src/host.js b/packages/daemon/src/host.js index 41d5a5a5af..d76e37883e 100644 --- a/packages/daemon/src/host.js +++ b/packages/daemon/src/host.js @@ -60,7 +60,7 @@ export const makeHostMaker = ({ * @param {{[name: string]: string}} platformNames * @param {import('./types.js').Context} context */ - const makeIdentifiedHost = async ( + const makeHost = async ( hostId, handleId, storeId, @@ -384,7 +384,7 @@ export const makeHostMaker = ({ * @param {import('./types.js').MakeHostOrGuestOptions} [opts] * @returns {Promise<{id: string, value: Promise}>} */ - const makeHost = async ( + const makeChildHost = async ( petName, { introducedNames = {}, agentName = undefined } = {}, ) => { @@ -408,7 +408,7 @@ export const makeHostMaker = ({ /** @type {import('./types.js').EndoHost['provideHost']} */ const provideHost = async (petName, opts) => { - const { value } = await makeHost(petName, opts); + const { value } = await makeChildHost(petName, opts); return value; }; @@ -546,7 +546,7 @@ export const makeHostMaker = ({ deliver, }; - const external = makeExo( + const hostExo = makeExo( 'EndoHost', M.interface('EndoHost', {}, { defaultGuards: 'passable' }), { @@ -555,12 +555,11 @@ export const makeHostMaker = ({ followMessages: () => makeIteratorRef(host.followMessages()), }, ); - const internal = harden({ petStore }); await provide(mainWorkerId); - return harden({ external, internal }); + return hostExo; }; - return makeIdentifiedHost; + return makeHost; }; diff --git a/packages/daemon/test/test-endo.js b/packages/daemon/test/test-endo.js index 3f7832cf12..03413f1290 100644 --- a/packages/daemon/test/test-endo.js +++ b/packages/daemon/test/test-endo.js @@ -1011,7 +1011,7 @@ test('name and reuse inspector', async t => { }); // Regression test for https://github.com/endojs/endo/issues/2021 -test.failing('eval-mediated worker name', async t => { +test('eval-mediated worker name', async t => { const { cancelled, locator } = await prepareLocator(t); const { host } = await makeHost(locator, cancelled); From 53a048a7f762ac5e7acc0b649f257d555ca4c062 Mon Sep 17 00:00:00 2001 From: Kris Kowal Date: Thu, 11 Apr 2024 08:02:23 -0700 Subject: [PATCH 5/5] refactor(daemon): Tidy controller system --- packages/daemon/src/daemon.js | 83 ++++++++++++++-------------------- packages/daemon/src/types.d.ts | 17 ++----- 2 files changed, 39 insertions(+), 61 deletions(-) diff --git a/packages/daemon/src/daemon.js b/packages/daemon/src/daemon.js index 37631ec443..302016f4e5 100644 --- a/packages/daemon/src/daemon.js +++ b/packages/daemon/src/daemon.js @@ -382,8 +382,7 @@ const makeDaemonCore = async ( }; /** - * Creates a controller for a `lookup` formula. The external facet is the - * resolved value of the lookup. + * Creates a controller for a `lookup` formula. * * @param {string} hubId * @param {string[]} path @@ -688,24 +687,25 @@ const makeDaemonCore = async ( * @param {string} id * @param {import('./types.js').Context} context */ - const makeControllerForId = async (id, context) => { + const evaluateFormulaForId = async (id, context) => { const { number: formulaNumber, node: formulaNode } = parseId(id); const isRemote = formulaNode !== ownNodeIdentifier; if (isRemote) { // eslint-disable-next-line no-use-before-define - const peerIdentifier = await getPeerIdForNodeIdentifier(formulaNode); - // Behold, forward reference: - // eslint-disable-next-line no-use-before-define - return provideRemoteValue(peerIdentifier, id); + const peerId = await getPeerIdForNodeIdentifier(formulaNode); + const peer = /** @type {Promise} */ ( + // Behold, forward reference: + // eslint-disable-next-line no-use-before-define + provide(peerId) + ); + return E(peer).provide(id); } const formula = await getFormulaForId(id); console.log(`Reincarnating ${formula.type} ${id}`); assertValidFormulaType(formula.type); - // TODO further validation - const value = evaluateFormula(id, formulaNumber, formula, context); - return { external: value }; + return evaluateFormula(id, formulaNumber, formula, context); }; /** @type {import('./types.js').DaemonCore['formulate']} */ @@ -720,39 +720,42 @@ const makeDaemonCore = async ( // Memoize for lookup. console.log(`Making ${formula.type} ${id}`); - const { promise: partial, resolve: resolvePartial } = - /** @type {import('@endo/promise-kit').PromiseKit>} */ ( + const { promise, resolve } = + /** @type {import('@endo/promise-kit').PromiseKit} */ ( makePromiseKit() ); // Behold, recursion: // eslint-disable-next-line no-use-before-define const context = makeContext(id); - partial.catch(context.cancel); + promise.catch(context.cancel); const controller = harden({ context, - external: E.get(partial).external.then(value => { - if (typeof value === 'object' && value !== null) { - idForRef.add(value, id); - } - return value; - }), + value: promise, }); controllerForId.set(id, controller); - // The controller _must_ be constructed in the synchronous prelude of this function. - const value = evaluateFormula(id, formulaNumber, formula, context); - const controllerValue = { external: value }; - // Ensure that failure to flush the formula to storage // causes a rejection for both the controller and the value. const written = persistencePowers.writeFormula(formulaNumber, formula); - resolvePartial(written.then(() => /** @type {any} */ (controllerValue))); + // The controller _must_ be constructed in the synchronous prelude of this function. + const valuePromise = evaluateFormula( + id, + formulaNumber, + formula, + context, + ).then(value => { + if (typeof value === 'object' && value !== null) { + idForRef.add(value, id); + } + return value; + }); + resolve(written.then(() => valuePromise)); await written; return harden({ id, - value: controller.external, + value: controller.value, }); }; @@ -763,22 +766,23 @@ const makeDaemonCore = async ( return controller; } - const { promise: partial, resolve } = - /** @type {import('@endo/promise-kit').PromiseKit>} */ ( + const { promise, resolve } = + /** @type {import('@endo/promise-kit').PromiseKit} */ ( makePromiseKit() ); // Behold, recursion: // eslint-disable-next-line no-use-before-define const context = makeContext(id); - partial.catch(context.cancel); + promise.catch(context.cancel); controller = harden({ context, - external: E.get(partial).external, + value: promise, }); controllerForId.set(id, controller); - resolve(makeControllerForId(id, context)); + // The controller must be in place before we evaluate the formula. + resolve(evaluateFormulaForId(id, context)); return controller; }; @@ -817,7 +821,7 @@ const makeDaemonCore = async ( const controller = /** @type {import('./types.js').Controller<>} */ ( provideController(id) ); - return controller.external.then(value => { + return controller.value.then(value => { // Release the value to the public only after ensuring // we can reverse-lookup its nonce. if (typeof value === 'object' && value !== null) { @@ -1469,23 +1473,6 @@ const makeDaemonCore = async ( throw new Error('Cannot connect to peer: no supported addresses'); }; - /** - * This is used to provide a value for a formula identifier that is known to - * originate from the specified peer. - * @param {string} peerId - * @param {string} remoteValueId - * @returns {Promise>} - */ - const provideRemoteValue = async (peerId, remoteValueId) => { - const peer = /** @type {import('./types.js').EndoPeer} */ ( - await provide(peerId) - ); - const remoteValueP = peer.provide(remoteValueId); - const external = remoteValueP; - const internal = Promise.resolve(undefined); - return harden({ internal, external }); - }; - const makeContext = makeContextMaker({ controllerForId, provideController, diff --git a/packages/daemon/src/types.d.ts b/packages/daemon/src/types.d.ts index efcf197294..deed8af1e3 100644 --- a/packages/daemon/src/types.d.ts +++ b/packages/daemon/src/types.d.ts @@ -342,16 +342,8 @@ export interface FarContext { addDisposalHook: Context['onCancel']; } -export interface InternalExternal { - external: External; -} - -export interface ControllerPartial { - external: Promise; -} - -export interface Controller - extends ControllerPartial { +export interface Controller { + value: Promise; context: Context; } @@ -469,8 +461,6 @@ export type MakeHostOrGuestOptions = { export interface EndoPeer { provide: (id: string) => Promise; } -export type EndoPeerControllerPartial = ControllerPartial; -export type EndoPeerController = Controller; export interface EndoGateway { provide: (id: string) => Promise; @@ -650,11 +640,12 @@ export type DaemonicPersistencePowers = { export interface DaemonWorkerFacet {} export interface WorkerDaemonFacet { - terminate(): void; + terminate(): Promise; evaluate( source: string, names: Array, values: Array, + id: string, cancelled: Promise, ): Promise; makeBundle(bundle: ERef, powers: ERef);