refactor(daemon): Refine synchronized incarnateEval implementation

- Rename various entities for clarity / correctness - Use terser implementation of async flow in `provideValueforNumberedFormula` - Add inline documentation Co-authored-by: Kris Kowal <kris@agoric.com>
endojs · Feb 22, 2024 · fcd9093 · fcd9093
1 parent 93d022a
commit fcd9093
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 28 deletions.
diff --git a/packages/daemon/src/daemon.js b/packages/daemon/src/daemon.js
@@ -551,13 +551,10 @@ const makeDaemonCore = async (
 
     // Memoize for lookup.
     console.log(`Making ${formulaIdentifier}`);
-    const {
-      promise: partial,
-      resolve: resolvePartial,
-      reject: rejectPartial,
-    } = /** @type {import('@endo/promise-kit').PromiseKit<import('./types.js').InternalExternal<>>} */ (
-      makePromiseKit()
-    );
+    const { promise: partial, resolve: resolvePartial } =
+      /** @type {import('@endo/promise-kit').PromiseKit<import('./types.js').InternalExternal<>>} */ (
+        makePromiseKit()
+      );
 
     // Behold, recursion:
     // eslint-disable-next-line no-use-before-define
@@ -575,22 +572,24 @@ const makeDaemonCore = async (
     });
     controllerForFormulaIdentifier.set(formulaIdentifier, controller);
 
-    // We _must not_ await before the controller value is constructed.
+    // The controller _must_ be constructed in the synchronous prelude of this function.
     const controllerValue = makeControllerForFormula(
       formulaIdentifier,
       formulaNumber,
       formula,
       context,
     );
 
-    try {
-      await persistencePowers.writeFormula(formula, formulaType, formulaNumber);
-    } catch (error) {
-      rejectPartial(error);
-      throw error;
-    }
+    // Ensure that failure to flush the formula to storage
+    // causes a rejection for both the controller and the incarnation value.
+    const written = persistencePowers.writeFormula(
+      formula,
+      formulaType,
+      formulaNumber,
+    );
+    resolvePartial(written.then(() => /** @type {any} */ (controllerValue)));
+    await written;
 
-    resolvePartial(controllerValue);
     return harden({
       formulaIdentifier,
       value: controller.external,
@@ -637,6 +636,8 @@ const makeDaemonCore = async (
       provideControllerForFormulaIdentifier(formulaIdentifier)
     );
     return controller.external.then(value => {
+      // Release the value to the public only after ensuring
+      // we can reverse-lookup its nonce.
       if (typeof value === 'object' && value !== null) {
         formulaIdentifierForRef.set(value, formulaIdentifier);
       }
@@ -733,7 +734,7 @@ const makeDaemonCore = async (
    * @param {string} formulaNumber - The worker formula number.
    * @returns {Promise<{ formulaIdentifier: string, value: import('./types').EndoWorker }>}
    */
-  const incarnateWorkerSync = formulaNumber => {
+  const incarnateNumberedWorker = formulaNumber => {
     /** @type {import('./types.js').WorkerFormula} */
     const formula = {
       type: 'worker',
@@ -806,7 +807,7 @@ const makeDaemonCore = async (
    * @param {string} hostFormulaIdentifier
    * @param {string} source
    * @param {string[]} codeNames
-   * @param {(string | string[])[]} endowmentFormulaPointers
+   * @param {(string | string[])[]} endowmentFormulaIdsOrPaths
    * @param {import('./types.js').EvalFormulaHook[]} hooks
    * @param {string} [specifiedWorkerFormulaIdentifier]
    * @returns {Promise<{ formulaIdentifier: string, value: unknown }>}
@@ -815,7 +816,7 @@ const makeDaemonCore = async (
     hostFormulaIdentifier,
     source,
     codeNames,
-    endowmentFormulaPointers,
+    endowmentFormulaIdsOrPaths,
     hooks,
     specifiedWorkerFormulaIdentifier,
   ) => {
@@ -831,17 +832,17 @@ const makeDaemonCore = async (
 
       const identifiers = harden({
         workerFormulaIdentifier: (
-          await incarnateWorkerSync(workerFormulaNumber)
+          await incarnateNumberedWorker(workerFormulaNumber)
         ).formulaIdentifier,
         endowmentFormulaIdentifiers: await Promise.all(
-          endowmentFormulaPointers.map(async formulaIdOrPath => {
+          endowmentFormulaIdsOrPaths.map(async formulaIdOrPath => {
             if (typeof formulaIdOrPath === 'string') {
               return formulaIdOrPath;
             }
             return (
               /* eslint-disable no-use-before-define */
               (
-                await incarnateLookupSync(
+                await incarnateNumberedLookup(
                   await randomHex512(),
                   hostFormulaIdentifier,
                   formulaIdOrPath,
@@ -881,7 +882,7 @@ const makeDaemonCore = async (
    * @param {string[]} petNamePath - The pet name path to look up.
    * @returns {Promise<{ formulaIdentifier: string, value: import('./types').EndoWorker }>}
    */
-  const incarnateLookupSync = (
+  const incarnateNumberedLookup = (
     formulaNumber,
     hubFormulaIdentifier,
     petNamePath,

diff --git a/packages/daemon/src/host.js b/packages/daemon/src/host.js
@@ -298,7 +298,7 @@ export const makeHostMaker = ({
       );
 
       /** @type {(string | string[])[]} */
-      const endowmentFormulaPointers = petNamePaths.map(
+      const endowmentFormulaIdsOrPaths = petNamePaths.map(
         (petNameOrPath, index) => {
           if (typeof codeNames[index] !== 'string') {
             throw new Error(`Invalid endowment name: ${q(codeNames[index])}`);
@@ -313,8 +313,6 @@ export const makeHostMaker = ({
             return formulaIdentifier;
           }
 
-          // TODO:lookup Check if a formula already exists for the path. May have to be
-          // done in the daemon itself.
           return petNamePath;
         },
       );
@@ -329,7 +327,7 @@ export const makeHostMaker = ({
         hostFormulaIdentifier,
         source,
         codeNames,
-        endowmentFormulaPointers,
+        endowmentFormulaIdsOrPaths,
         hooks,
         workerFormulaIdentifier,
       );

diff --git a/packages/daemon/test/test-endo.js b/packages/daemon/test/test-endo.js
@@ -881,8 +881,7 @@ test('name and reuse inspector', async t => {
   await stop(locator);
 });
 
-// This tests behavior that previously failed due to a bug.
-// See: https://github.com/endojs/endo/issues/2021
+// Regression test for: https://github.com/endojs/endo/issues/2021
 test('eval-mediated worker name', async t => {
   const { promise: cancelled, reject: cancel } = makePromiseKit();
   t.teardown(() => cancel(Error('teardown')));