Restrict promise redirect

packages/SwingSet/docs/comms.md

@@ -175,7 +175,8 @@ A send-with-result of two slots, `p = E(target).foo(1,2,bar,baz)`, would be
 
 Promises that resolve to data, or which are rejected, will have a `body` and
 possibly `slots`. Each slot can hold any kind of reference: `ro+NN`, `ro-NN`,
-`rp+NN`, or `rp-NN`.
+`rp+NN`, or `rp-NN`. A promise cannot currently be resolved directly to another
+promise, unless the promise value is nested in data, or it's a rejection.
 
 Promises that resolve to a callable object have a single `resolutionRef`,
 which always resolves to `ro+NN` or `ro-NN`.

packages/SwingSet/docs/delivery.md

@@ -322,7 +322,7 @@ Vat) unless the `result` ID falls into one of these categories:
   present in the C-List, and the Decider will point at this Vat.
 * The Promise was received from the kernel earlier as the result slot of an
   incoming message. The ID will be a negative integer, and the Decider will
-  point at this Vat.
+  point at this Vat. This is currently only allowed for a pipelining vat.
 
 In all cases, the kernel promise table winds up with a Promise for which the
 Decider is cleared, as the resolution authority now resides in the queued
@@ -340,6 +340,10 @@ to either create a new Promise (by passing the kernel a new positive
 `PromiseID`, inside a `CapData` slot), or to receive it in the result slot of
 an inbound message.
 
+Once promise redirection is introduced, the effective decider of a promise
+would change to the Decider of its redirection. However the Decider of record
+may stay with the Kernel.
+
 ### Pipelining
 
 Promise Pipelining is the practice of sending one message to the Decider of
@@ -373,6 +377,19 @@ pipelined deliveries, `dispatch.deliver()` is invoked and the message is sent
 into the deciding Vat, which (in the case of the comms Vat) can transmit the
 message to the remote machine where the target will be resolved.
 
+There is currently a limitation that a message queued to the promise queue is
+not moved back to the run-queue until resolution, even if the Decider changes
+to a pipelining vat.
+
+Once we implement per-vat or per-object queues, we may want to update the
+Decider of a result promise as soon as the send is queued onto the vat. At that
+point we would also queue pipeline eligible messages onto the target vat.
+However such messages will need to be checked again before actual delivery in
+case the target promise resolves, or the Decider changes before the message can
+be delivered. This also implies that not all messages on a vat's queue are
+doomed immediately when the vat is terminated, as there may have been
+resolutions before termination that are still pending.
+
 If a non-comms Vat enables pipelining, it is obligated to queue the pipelined
 messages inside the Vat until the target is resolved. When that resolution
 happens, the new target might be in some other Vat, in which case the
@@ -511,7 +528,7 @@ There are a few restrictions on the API:
 * The `Message.result` in a `syscall.send()` must either be a new vat-allocated
   (positive) PromiseID, or a previously-allocated PromiseID for which the
   calling Vat is the Decider, or a PromiseID that was previously received as
-  the result of an inbound message.
+  the result of an inbound message (for pipelining vats).
 
 Some invocation patterns are valid, but unlikely to be useful:
 
@@ -520,7 +537,7 @@ Some invocation patterns are valid, but unlikely to be useful:
   which the local Vat is the Decider. In both cases, the Vat could have
   delivered the message directly, instead of taking the time and effort of going
   through the kernel's run-queue. On the other hand, this may achieve certain
-  ordering properties better.
+  ordering properties better, or allow the vat to split up work in multiple cranks.
 
 In some places, `dispatch.deliver()` is named `message`: we're still in the
 process of refactoring and unifying the codebase.
@@ -621,8 +638,9 @@ immediately after translation).
 The subject of a `syscall.resolve()` must either be a new Promise ID, or a
 pre-existing one for which the calling Vat is the Decider. The
 `KernelPromise` must be in the `Unresolved` state. If any of these conditions
-are not met, the Vat calling `resolve` will be terminated. It doesn't matter
-whether the Promise was allocated by this Vat or a different one.
+are not met, the Vat calling `resolve` will be terminated. Furthermore, only
+pipelining-enabled vats are allowed to resolve promise IDs that they did not
+allocate.
 
 | Vat Object         | action if missing      |
 | ---                | ---                    |
@@ -632,7 +650,8 @@ whether the Promise was allocated by this Vat or a different one.
 The `resolution` has several forms, and we assign a different name to each.
 
 * `Fulfill(CapData)`: the Promise is "fulfilled" to a callable Object or other
-  data. It is an error to send messages to data.
+  data. It is an error to send messages to data. The CapData cannot be a single
+  promise as that would be a Forward.
 * `Reject(CapData)`: the Promise is "rejected" to data which we call the
   "error object". Sending a message to a Rejected Promise causes the result
   of that message to be Rejected too, known as "rejection contagion".

packages/SwingSet/src/kernel/kernel.js

@@ -11,7 +11,7 @@ import makeKernelKeeper from './state/kernelKeeper.js';
 import { kdebug, kdebugEnable, legibilizeMessageArgs } from '../lib/kdebug.js';
 import { insistKernelType, parseKernelSlot } from './parseKernelSlots.js';
 import { parseVatSlot } from '../lib/parseVatSlots.js';
-import { insistCapData } from '../lib/capdata.js';
+import { extractSingleSlot, insistCapData } from '../lib/capdata.js';
 import { insistMessage, insistVatDeliveryResult } from '../lib/message.js';
 import { insistDeviceID, insistVatID } from '../lib/id.js';
 import { makeKernelQueueHandler } from './kernelQueue.js';
@@ -406,25 +406,6 @@ export default function buildKernel(
     return deliverAndLogToVat(vatID, kd, vd);
   }
 
-  function extractPresenceIfPresent(data) {
-    const body = JSON.parse(data.body);
-    if (
-      body &&
-      typeof body === 'object' &&
-      body['@qclass'] === 'slot' &&
-      body.index === 0
-    ) {
-      if (data.slots.length === 1) {
-        const slot = data.slots[0];
-        const { type } = parseKernelSlot(slot);
-        if (type === 'object') {
-          return slot;
-        }
-      }
-    }
-    return null;
-  }
-
   /**
    *
    * @param { RunQueueEventNotify } message
@@ -787,9 +768,9 @@ export default function buildKernel(
     const kp = kernelKeeper.getKernelPromise(target);
     switch (kp.state) {
       case 'fulfilled': {
-        const presence = extractPresenceIfPresent(kp.data);
-        if (presence) {
-          return send(presence);
+        const targetSlot = extractSingleSlot(kp.data);
+        if (targetSlot && parseKernelSlot(targetSlot).type === 'object') {
+          return send(targetSlot);
         }
         // TODO: maybe mimic (3).foo(): "TypeError: XX.foo is not a function"
         const s = `data is not callable, has no method ${msg.method}`;
@@ -1273,29 +1254,38 @@ export default function buildKernel(
     vatParameters = {},
     creationOptions = {},
   ) {
+    const {
+      bundleID = 'b1-00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000',
+      ...actualCreationOptions
+    } = creationOptions;
     assert.typeof(
       setup,
       'function',
       X`setup is not a function, rather ${setup}`,
     );
-    assertKnownOptions(creationOptions, [
+    assertKnownOptions(actualCreationOptions, [
       'enablePipelining',
       'metered',
       'reapInterval',
+      'managerType',
     ]);
 
     assert(!kernelKeeper.hasVatWithName(name), X`vat ${name} already exists`);
 
     const vatID = kernelKeeper.allocateVatIDForNameIfNeeded(name);
     logStartup(`assigned VatID ${vatID} for test vat ${name}`);
 
-    if (!creationOptions.reapInterval) {
-      creationOptions.reapInterval = 'never';
+    if (!actualCreationOptions.reapInterval) {
+      actualCreationOptions.reapInterval = 'never';
+    }
+    if (!actualCreationOptions.managerType) {
+      actualCreationOptions.managerType = 'local';
     }
     const vatKeeper = kernelKeeper.provideVatKeeper(vatID);
-    vatKeeper.initializeReapCountdown(creationOptions.reapInterval);
+    vatKeeper.setSourceAndOptions({ bundleID }, actualCreationOptions);
+    vatKeeper.initializeReapCountdown(actualCreationOptions.reapInterval);
 
-    await vatWarehouse.loadTestVat(vatID, setup, creationOptions);
+    await vatWarehouse.loadTestVat(vatID, setup, actualCreationOptions);
 
     const vpCapData = { body: stringify(harden(vatParameters)), slots: [] };
     /** @type { RunQueueEventStartVat } */

packages/SwingSet/src/kernel/state/kernelKeeper.js

@@ -865,6 +865,7 @@ export default function makeKernelKeeper(
   function clearDecider(kpid) {
     const p = getKernelPromise(kpid);
     assert(p.state === 'unresolved', X`${kpid} was already resolved`);
+    assert(p.decider, X`${kpid} does not have a decider`);
     kvStore.set(`${kpid}.decider`, '');
   }
 

packages/SwingSet/src/kernel/state/vatKeeper.js

@@ -235,13 +235,24 @@ export function makeVatKeeper(
    * allocate, we insist that the reachable flag was already set.
    *
    * @param {string} vatSlot  The vat slot of interest
-   * @param {{ setReachable?: boolean, required?: boolean }} options  'setReachable' will set the 'reachable' flag on vat exports, while 'required' means we refuse to allocate a missing entry
+   * @param {Object} [options]
+   * @param {boolean} [options.setReachable] set the 'reachable' flag on vat exports
+   * @param {boolean} [options.required] refuse to allocate a missing entry
+   * @param {boolean} [options.requireNew] require that the entry be newly allocated
    * @returns {string} the kernel slot that vatSlot maps to
    * @throws {Error} if vatSlot is not a kind of thing that can be exported by vats
    * or is otherwise invalid.
    */
   function mapVatSlotToKernelSlot(vatSlot, options = {}) {
-    const { setReachable = true, required = false } = options;
+    const {
+      setReachable = true,
+      required = false,
+      requireNew = false,
+    } = options;
+    assert(
+      !(required && requireNew),
+      `'required' and 'requireNew' are mutually exclusive`,
+    );
     assert.typeof(vatSlot, 'string', X`non-string vatSlot: ${vatSlot}`);
     const { type, allocatedByVat } = parseVatSlot(vatSlot);
     const vatKey = `${vatID}.c.${vatSlot}`;
@@ -286,6 +297,8 @@ export function makeVatKeeper(
         // (else it would have been in the c-list), so it must be bogus
         assert.fail(X`unknown vatSlot ${q(vatSlot)}`);
       }
+    } else if (requireNew) {
+      assert.fail(X`vref ${q(vatSlot)} is already allocated`);
     }
     const kernelSlot = getRequired(vatKey);
 

packages/SwingSet/src/kernel/vat-loader/vat-loader.js

@@ -2,7 +2,6 @@
 import { assert, details as X } from '@agoric/assert';
 import { assertKnownOptions } from '../../lib/assertOptions.js';
 import { makeVatSlot } from '../../lib/parseVatSlots.js';
-import { makeVatTranslators } from '../vatTranslator.js';
 
 export function makeVatRootObjectSlot() {
   return makeVatSlot('object', true, 0n);
@@ -20,13 +19,15 @@ export function makeVatLoader(stuff) {
     vatAdminRootKref,
   } = stuff;
 
+  /** @typedef {ReturnType<typeof import('../vatTranslator.js').makeVatTranslators>} Translators */
+
   /**
    * Create a new vat at runtime (called when a 'create-vat' event reaches
    * the top of the run-queue).
    *
    * @param { string } vatID  The pre-allocated vatID
    * @param { SourceOfBundle } source  The source object implementing the vat
-   * @param { ReturnType<makeVatTranslators> } translators
+   * @param { Translators } translators
    * @param {*} dynamicOptions  Options bag governing vat creation
    *
    * @returns {Promise<VatManager>}
@@ -47,7 +48,7 @@ export function makeVatLoader(stuff) {
    *
    * @param {string} vatID  The vatID of the vat to create
    * @param { SourceOfBundle } source  The source object implementing the vat
-   * @param { ReturnType<makeVatTranslators> } translators
+   * @param { Translators } translators
    * @param {*} dynamicOptions  Options bag governing vat creation
    *
    * @returns {Promise<VatManager>} fires when the vat is ready for messages
@@ -69,7 +70,7 @@ export function makeVatLoader(stuff) {
    *
    * @param {string} vatID  The vatID of the vat to create
    * @param { SourceOfBundle } source  The source object implementing the vat
-   * @param { ReturnType<makeVatTranslators> } translators
+   * @param { Translators } translators
    * @param {*} staticOptions  Options bag governing vat creation
    *
    * @returns {Promise<VatManager>} A Promise which fires when the
@@ -125,7 +126,7 @@ export function makeVatLoader(stuff) {
    *    used, it must identify a bundle already known to the kernel (via the
    *    `config.bundles` table) which satisfies these constraints.
    *
-   * @param { ReturnType<makeVatTranslators> } translators
+   * @param { Translators } translators
    *
    * @param {Object} options  an options bag. These options are currently understood:
    *
@@ -263,7 +264,7 @@ export function makeVatLoader(stuff) {
     return manager;
   }
 
-  async function loadTestVat(vatID, setup, creationOptions) {
+  async function loadTestVat(vatID, setup, translators, creationOptions) {
     const managerOptions = {
       ...creationOptions,
       setup,
@@ -272,7 +273,6 @@ export function makeVatLoader(stuff) {
       useTranscript: true,
       ...overrideVatManagerOptions,
     };
-    const translators = makeVatTranslators(vatID, kernelKeeper);
     const vatSyscallHandler = buildVatSyscallHandler(vatID, translators);
     const manager = await vatManagerFactory(
       vatID,

packages/SwingSet/src/kernel/vat-warehouse.js

@@ -334,10 +334,15 @@ export function makeVatWarehouse(kernelKeeper, vatLoader, policyOptions) {
    * @param {ManagerOptions} creationOptions
    */
   async function loadTestVat(vatID, setup, creationOptions) {
-    const manager = await vatLoader.loadTestVat(vatID, setup, creationOptions);
-
     const translators = provideTranslators(vatID);
 
+    const manager = await vatLoader.loadTestVat(
+      vatID,
+      setup,
+      translators,
+      creationOptions,
+    );
+
     const { enablePipelining = false } = creationOptions;
 
     const result = {

packages/SwingSet/src/kernel/vatTranslator.js

@@ -3,7 +3,7 @@ import { assert, details as X } from '@agoric/assert';
 import { insistMessage } from '../lib/message.js';
 import { insistKernelType, parseKernelSlot } from './parseKernelSlots.js';
 import { insistVatType, parseVatSlot } from '../lib/parseVatSlots.js';
-import { insistCapData } from '../lib/capdata.js';
+import { extractSingleSlot, insistCapData } from '../lib/capdata.js';
 import {
   kdebug,
   legibilizeMessageArgs,
@@ -81,6 +81,8 @@ function makeTranslateKernelDeliveryToVatDelivery(vatID, kernelKeeper) {
       ];
     } else if (kp.state === 'redirected') {
       // TODO unimplemented
+      // NOTE: When adding redirection / forwarding, we must handle any
+      // pipelined messages pending in the inbound queue
       throw new Error('not implemented yet');
     } else {
       assert.fail(X`unknown kernelPromise state '${kp.state}'`);
@@ -239,6 +241,7 @@ function makeTranslateKernelDeliveryToVatDelivery(vatID, kernelKeeper) {
 function makeTranslateVatSyscallToKernelSyscall(vatID, kernelKeeper) {
   const vatKeeper = kernelKeeper.provideVatKeeper(vatID);
   const { mapVatSlotToKernelSlot, clearReachableFlag } = vatKeeper;
+  const { enablePipelining = false } = vatKeeper.getOptions();
 
   function translateSend(targetSlot, msg) {
     assert.typeof(targetSlot, 'string', 'non-string targetSlot');
@@ -255,12 +258,26 @@ function makeTranslateVatSyscallToKernelSyscall(vatID, kernelKeeper) {
     let result = null;
     if (resultSlot) {
       insistVatType('promise', resultSlot);
-      result = mapVatSlotToKernelSlot(resultSlot);
+      // Require that the promise used by the vat to receive the result of this
+      // send is newly allocated by the vat if it does not support pipelining.
+      // When a promise previously received as the result of a delivery is used
+      // as the result of a send call, it's in effect doing a redirect, which
+      // is currently only supported for pipelining vats.
+      // While we could only require that the promise be allocated by the vat,
+      // aka exported not imported, there is currently no path for liveslots
+      // vats to ever use a promise before it is seen as result, which would be
+      // indicative of something unexpected going on.
+      result = mapVatSlotToKernelSlot(resultSlot, {
+        requireNew: !enablePipelining,
+      });
       insistKernelType('promise', result);
       // The promise must be unresolved, and this Vat must be the decider.
       // The most common case is that 'resultSlot' is a new exported promise
       // (p+NN). But it might be a previously-imported promise (p-NN) that
-      // they got in a deliver() call, which gave them resolution authority.
+      // they got in a deliver() call, which gave them resolution authority,
+      // however only in the case of pipelining vats.
+      // In the case of non-pipelining vats these checks are redundant since
+      // we're guaranteed to have a promise newly allocated by the vat.
       const p = kernelKeeper.getKernelPromise(result);
       assert(
         p.state === 'unresolved',
@@ -508,6 +525,20 @@ function makeTranslateVatSyscallToKernelSyscall(vatID, kernelKeeper) {
       const [vpid, rejected, data] = resolution;
       insistVatType('promise', vpid);
       insistCapData(data);
+      if (!rejected) {
+        const resolutionSlot = extractSingleSlot(data);
+        if (resolutionSlot) {
+          // Resolving to a promise is not a fulfillment.
+          // It would be a redirect, but that's not currently supported by the kernel.
+          // Furthermore messages sent to such a promise resolved this way would not
+          // be forwarded but would splat instead.
+          assert(
+            parseVatSlot(resolutionSlot).type !== 'promise',
+            'cannot resolve to a promise',
+          );
+        }
+      }
+
       const kpid = mapVatSlotToKernelSlot(vpid);
       const kernelSlots = data.slots.map(slot => mapVatSlotToKernelSlot(slot));
       const kernelData = harden({ ...data, slots: kernelSlots });