refactor(daemon): Synchronize host evaluate method

Synchronizes the host's `evaluate()` method by delegating all
incarnations to the daemon's `incarnateEval()`. Also synchronizes
`incarnateLookup()`, and removes `provideLookupFormula()` from the
mailbox, which was only a thin wrapper anyway.

The path taken introduces new incarnation semantics and design constraints on
the daemon and its dependents. #2089 introduced the notion of "incarnating"
values. The `incarnateX() methods are responsible for creating formula JSON
objects and calling `provideValueForNumberedFormula` to reify their values
for the first time. Synchronizing dependencies between `incarnateX` methods
is feasible by means of a lock, but difficulties arise when formulas are
incarnated outside the daemon, such as in the host. Coordinating a lock
between two different modules will increase the risks of datalocks at
runtime.

To avoid datalocks, the implementation delegates all incarnations necessary
for `evaluate()` to its dependent `incarnateEval()`. In essence, it is
the consumer's responsibility to specify all necessary incarnations to
the relevant incarnation method, which is then responsible for carrying
them out. Yet, the story is complicated by pet names being mostly
abstracted away from the daemon. For example, pet names must be
associated with their respective formula identifiers the moment that
those identifiers are observable to the consumer. As part of this
process, the pet store must write the name to formula id mapping to
disk. To handle sych asynchronous side effects, the implementation
introduces a notion of "hooks" to `incarnateEval()`, with the intention
of spreading this to other incarnation methods as necessary. These hooks
receive as an argument all formula identifiers created by the
incarnation, and are executed under the formula graph synchronization lock.
This will surface IO errors to the consumer, and help us uphold the
principle of "death before confusion".

Also of note, `provideValueForNumberedFormula` has been modified such
that the formula is written to disk _after_ the controller has been
constructed. This is critical in order to serialize in-memory formula
graph mutations. Since it's possible for incarnation to fail after the
controller has been created, we should consider adding cleanup callbacks
to incarnation hooks.

packages/daemon/src/daemon.js

@@ -550,10 +550,13 @@ const makeDaemonCore = async (
     const formulaIdentifier = `${formulaType}:${formulaNumber}`;
 
     // Memoize for lookup.
-    const { promise: partial, resolve } =
-      /** @type {import('@endo/promise-kit').PromiseKit<import('./types.js').InternalExternal<>>} */ (
-        makePromiseKit()
-      );
+    const {
+      promise: partial,
+      resolve: resolvePartial,
+      reject: rejectPartial,
+    } = /** @type {import('@endo/promise-kit').PromiseKit<import('./types.js').InternalExternal<>>} */ (
+      makePromiseKit()
+    );
 
     // Behold, recursion:
     // eslint-disable-next-line no-use-before-define
@@ -571,16 +574,22 @@ const makeDaemonCore = async (
     });
     controllerForFormulaIdentifier.set(formulaIdentifier, controller);
 
-    await persistencePowers.writeFormula(formula, formulaType, formulaNumber);
-    resolve(
-      makeControllerForFormula(
-        formulaIdentifier,
-        formulaNumber,
-        formula,
-        context,
-      ),
+    // We _must not_ await before the controller value is constructed.
+    const controllerValue = makeControllerForFormula(
+      formulaIdentifier,
+      formulaNumber,
+      formula,
+      context,
     );
 
+    try {
+      await persistencePowers.writeFormula(formula, formulaType, formulaNumber);
+    } catch (error) {
+      rejectPartial(error);
+      throw error;
+    }
+
+    resolvePartial(controllerValue);
     return harden({
       formulaIdentifier,
       value: controller.external,
@@ -660,7 +669,7 @@ const makeDaemonCore = async (
     };
 
   /**
-   * @param {string} [specifiedFormulaNumber] - The formula number of the pet store.
+   * @param {string} [specifiedFormulaNumber] - The formula number of the least authority object.
    * @returns {Promise<{ formulaIdentifier: string, value: import('./types').EndoGuest }>}
    */
   const incarnateLeastAuthority = async specifiedFormulaNumber => {
@@ -770,13 +779,14 @@ const makeDaemonCore = async (
               await incarnateWorker(await randomHex512())
             ).formulaIdentifier,
             inspectorFormulaIdentifier:
-              // eslint-disable-next-line no-use-before-define
+              /* eslint-disable no-use-before-define */
               (
                 await incarnatePetInspector(
                   storeFormulaId,
                   await randomHex512(),
                 )
               ).formulaIdentifier,
+            /* eslint-enable no-use-before-define */
             formulaNumber: await randomHex512(),
           };
         })
@@ -824,19 +834,61 @@ const makeDaemonCore = async (
   };
 
   /**
-   * @param {string} workerFormulaIdentifier
+   * @param {string} hostFormulaIdentifier
    * @param {string} source
    * @param {string[]} codeNames
-   * @param {string[]} endowmentFormulaIdentifiers
+   * @param {(string | string[])[]} endowmentFormulaPointers
+   * @param {import('./types.js').EvalFormulaHook[]} hooks
+   * @param {string} [specifiedWorkerFormulaIdentifier]
    * @returns {Promise<{ formulaIdentifier: string, value: unknown }>}
    */
   const incarnateEval = async (
-    workerFormulaIdentifier,
+    hostFormulaIdentifier,
     source,
     codeNames,
-    endowmentFormulaIdentifiers,
+    endowmentFormulaPointers,
+    hooks,
+    specifiedWorkerFormulaIdentifier,
   ) => {
-    const formulaNumber = await randomHex512();
+    const {
+      workerFormulaIdentifier,
+      endowmentFormulaIdentifiers,
+      evalFormulaNumber,
+    } = await formulaGraphLock.enqueue(async () => {
+      const ownFormulaNumber = await randomHex512();
+
+      const identifiers = harden({
+        workerFormulaIdentifier:
+          specifiedWorkerFormulaIdentifier ??
+          (await incarnateWorker(await randomHex512())).formulaIdentifier,
+        endowmentFormulaIdentifiers: await Promise.all(
+          endowmentFormulaPointers.map(async formulaIdOrPath => {
+            if (typeof formulaIdOrPath === 'string') {
+              return formulaIdOrPath;
+            }
+            return (
+              /* eslint-disable no-use-before-define */
+              (
+                await incarnateLookup(
+                  hostFormulaIdentifier,
+                  formulaIdOrPath,
+                  await randomHex512(),
+                )
+              ).formulaIdentifier
+              /* eslint-enable no-use-before-define */
+            );
+          }),
+        ),
+        evalFormulaNumber: ownFormulaNumber,
+      });
+
+      for (const hook of hooks) {
+        // eslint-disable-next-line no-await-in-loop
+        await hook(identifiers);
+      }
+      return identifiers;
+    });
+
     /** @type {import('./types.js').EvalFormula} */
     const formula = {
       type: 'eval',
@@ -846,7 +898,7 @@ const makeDaemonCore = async (
       values: endowmentFormulaIdentifiers,
     };
     return /** @type {Promise<{ formulaIdentifier: string, value: unknown }>} */ (
-      provideValueForNumberedFormula(formula.type, formulaNumber, formula)
+      provideValueForNumberedFormula(formula.type, evalFormulaNumber, formula)
     );
   };
 
@@ -855,10 +907,18 @@ const makeDaemonCore = async (
    * A "naming hub" is an objected with a variadic lookup method. It includes
    * objects such as guests and hosts.
    * @param {string[]} petNamePath
+   * @param {string} [specifiedFormulaNumber] - The formula number of the lookup.
    * @returns {Promise<{ formulaIdentifier: string, value: unknown }>}
    */
-  const incarnateLookup = async (hubFormulaIdentifier, petNamePath) => {
-    const formulaNumber = await randomHex512();
+  const incarnateLookup = async (
+    hubFormulaIdentifier,
+    petNamePath,
+    specifiedFormulaNumber,
+  ) => {
+    const formulaNumber =
+      specifiedFormulaNumber ??
+      (await formulaGraphLock.enqueue(() => randomHex512()));
+
     /** @type {import('./types.js').LookupFormula} */
     const formula = {
       type: 'lookup',
@@ -913,7 +973,7 @@ const makeDaemonCore = async (
   /**
    * @param {string} powersFormulaIdentifier
    * @param {string} workerFormulaIdentifier
-   * @param {string} [specifiedFormulaNumber] - The formula number of the pet inspector.
+   * @param {string} [specifiedFormulaNumber] - The formula number of the bundler.
    * @returns {Promise<{ formulaIdentifier: string, value: unknown }>}
    */
   const incarnateBundler = async (
@@ -999,7 +1059,7 @@ const makeDaemonCore = async (
   };
 
   /**
-   * @param {string} [specifiedFormulaNumber]
+   * @param {string} [specifiedFormulaNumber] - The formula number of the endo bootstrap.
    * @returns {Promise<{ formulaIdentifier: string, value: import('./types').FarEndoBootstrap }>}
    */
   const incarnateEndoBootstrap = async specifiedFormulaNumber => {
@@ -1072,7 +1132,6 @@ const makeDaemonCore = async (
   });
 
   const makeMailbox = makeMailboxMaker({
-    incarnateLookup,
     getFormulaIdentifierForRef,
     provideValueForFormulaIdentifier,
     provideControllerForFormulaIdentifier,

packages/daemon/src/host.js

@@ -51,7 +51,6 @@ export const makeHostMaker = ({
       reverseLookup,
       identifyLocal,
       listMessages,
-      provideLookupFormula,
       followMessages,
       resolve,
       reject,
@@ -223,6 +222,29 @@ export const makeHostMaker = ({
       return workerFormulaIdentifier;
     };
 
+    /**
+     * @param {string | 'MAIN' | 'NEW'} workerName
+     * @param {(hook: import('./types.js').EvalFormulaHook) => void} addHook
+     * @returns {string | undefined}
+     */
+    const provideWorkerFormulaIdentifierSync = (workerName, addHook) => {
+      if (workerName === 'MAIN') {
+        return mainWorkerFormulaIdentifier;
+      } else if (workerName === 'NEW') {
+        return undefined;
+      }
+
+      assertPetName(workerName);
+      const workerFormulaIdentifier = identifyLocal(workerName);
+      if (workerFormulaIdentifier === undefined) {
+        addHook(identifiers =>
+          petStore.write(workerName, identifiers.workerFormulaIdentifier),
+        );
+        return undefined;
+      }
+      return workerFormulaIdentifier;
+    };
+
     /**
      * @param {string | 'NONE' | 'SELF' | 'ENDO'} partyName
      * @returns {Promise<string>}
@@ -256,19 +278,28 @@ export const makeHostMaker = ({
       petNamePaths,
       resultName,
     ) => {
-      const workerFormulaIdentifier = await provideWorkerFormulaIdentifier(
-        workerName,
-      );
-
       if (resultName !== undefined) {
         assertPetName(resultName);
       }
       if (petNamePaths.length !== codeNames.length) {
         throw new Error('Evaluator requires one pet name for each code name');
       }
 
-      const endowmentFormulaIdentifiers = await Promise.all(
-        petNamePaths.map(async (petNameOrPath, index) => {
+      /** @type {import('./types.js').EvalFormulaHook[]} */
+      const hooks = [];
+      /** @type {(hook: import('./types.js').EvalFormulaHook) => void} */
+      const addHook = hook => {
+        hooks.push(hook);
+      };
+
+      const workerFormulaIdentifier = provideWorkerFormulaIdentifierSync(
+        workerName,
+        addHook,
+      );
+
+      /** @type {(string | string[])[]} */
+      const endowmentFormulaPointers = petNamePaths.map(
+        (petNameOrPath, index) => {
           if (typeof codeNames[index] !== 'string') {
             throw new Error(`Invalid endowment name: ${q(codeNames[index])}`);
           }
@@ -282,23 +313,26 @@ export const makeHostMaker = ({
             return formulaIdentifier;
           }
 
-          const { formulaIdentifier: lookupFormulaIdentifier } =
-            await provideLookupFormula(petNamePath);
-          return lookupFormulaIdentifier;
-        }),
+          // TODO:lookup Check if a formula already exists for the path. May have to be
+          // done in the daemon itself.
+          return petNamePath;
+        },
       );
 
-      // Behold, recursion:
-      // eslint-disable-next-line no-use-before-define
-      const { formulaIdentifier, value } = await incarnateEval(
-        workerFormulaIdentifier,
+      if (resultName !== undefined) {
+        addHook(identifiers =>
+          petStore.write(resultName, `eval:${identifiers.evalFormulaNumber}`),
+        );
+      }
+
+      const { value } = await incarnateEval(
+        hostFormulaIdentifier,
         source,
         codeNames,
-        endowmentFormulaIdentifiers,
+        endowmentFormulaPointers,
+        hooks,
+        workerFormulaIdentifier,
       );
-      if (resultName !== undefined) {
-        await petStore.write(resultName, formulaIdentifier);
-      }
       return value;
     };
 

packages/daemon/src/mail.js

@@ -10,14 +10,12 @@ const { quote: q } = assert;
 
 /**
  * @param {object} args
- * @param {(hubFormulaIdentifier: string, petNamePath: string[]) => Promise<{ formulaIdentifier: string, value: unknown }>} args.incarnateLookup
  * @param {import('./types.js').ProvideValueForFormulaIdentifier} args.provideValueForFormulaIdentifier
  * @param {import('./types.js').ProvideControllerForFormulaIdentifier} args.provideControllerForFormulaIdentifier
  * @param {import('./types.js').GetFormulaIdentifierForRef} args.getFormulaIdentifierForRef
  * @param {import('./types.js').ProvideControllerForFormulaIdentifierAndResolveHandle} args.provideControllerForFormulaIdentifierAndResolveHandle
  */
 export const makeMailboxMaker = ({
-  incarnateLookup,
   getFormulaIdentifierForRef,
   provideValueForFormulaIdentifier,
   provideControllerForFormulaIdentifier,
@@ -120,12 +118,6 @@ export const makeMailboxMaker = ({
       return reverseLookupFormulaIdentifier(formulaIdentifier);
     };
 
-    /** @type {import('./types.js').Mail['provideLookupFormula']} */
-    const provideLookupFormula = async petNamePath => {
-      // TODO:lookup Check if the lookup formula already exists in the store
-      return incarnateLookup(selfFormulaIdentifier, petNamePath);
-    };
-
     /**
      * @param {import('./types.js').InternalMessage} message
      * @returns {import('./types.js').Message | undefined}
@@ -539,7 +531,6 @@ export const makeMailboxMaker = ({
       reverseLookup,
       reverseLookupFormulaIdentifier,
       identifyLocal,
-      provideLookupFormula,
       followMessages,
       listMessages,
       request,

packages/daemon/src/types.d.ts

@@ -97,6 +97,14 @@ type EvalFormula = {
   // TODO formula slots
 };
 
+export type EvalFormulaHook = (
+  identifiers: Readonly<{
+    endowmentFormulaIdentifiers: string[];
+    evalFormulaNumber: string;
+    workerFormulaIdentifier: string;
+  }>,
+) => Promise<unknown>;
+
 type ReadableBlobFormula = {
   type: 'readable-blob';
   content: string;
@@ -315,14 +323,6 @@ export interface Mail {
   listAll(): Array<string>;
   reverseLookupFormulaIdentifier(formulaIdentifier: string): Array<string>;
   cancel(petName: string, reason: unknown): Promise<void>;
-  /**
-   * Takes a sequence of pet names and returns a formula identifier and value
-   * for the corresponding lookup formula.
-   *
-   * @param petNamePath A sequence of pet names.
-   * @returns The formula identifier and value of the lookup formula.
-   */
-  provideLookupFormula(petNamePath: string[]): Promise<unknown>;
   // Mail operations:
   listMessages(): Promise<Array<Message>>;
   followMessages(): Promise<FarRef<Reader<Message>>>;