diff --git a/packages/SwingSet/docs/comms.md b/packages/SwingSet/docs/comms.md index 81532a2c98f..47639ec2bdb 100644 --- a/packages/SwingSet/docs/comms.md +++ b/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`. diff --git a/packages/SwingSet/docs/delivery.md b/packages/SwingSet/docs/delivery.md index 31b76373d45..1c8972ad64a 100644 --- a/packages/SwingSet/docs/delivery.md +++ b/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". diff --git a/packages/SwingSet/src/kernel/kernel.js b/packages/SwingSet/src/kernel/kernel.js index d4b96e24e42..0826481a220 100644 --- a/packages/SwingSet/src/kernel/kernel.js +++ b/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,15 +1254,20 @@ 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`); @@ -1289,13 +1275,17 @@ export default function buildKernel( 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 } */ diff --git a/packages/SwingSet/src/kernel/state/kernelKeeper.js b/packages/SwingSet/src/kernel/state/kernelKeeper.js index ff5ade55dba..f491c732209 100644 --- a/packages/SwingSet/src/kernel/state/kernelKeeper.js +++ b/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`, ''); } diff --git a/packages/SwingSet/src/kernel/state/vatKeeper.js b/packages/SwingSet/src/kernel/state/vatKeeper.js index 5fefd27fcf4..be3e0ff93a4 100644 --- a/packages/SwingSet/src/kernel/state/vatKeeper.js +++ b/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); diff --git a/packages/SwingSet/src/kernel/vat-loader/vat-loader.js b/packages/SwingSet/src/kernel/vat-loader/vat-loader.js index 81a2d30b1f1..89c0d5eefdc 100644 --- a/packages/SwingSet/src/kernel/vat-loader/vat-loader.js +++ b/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} 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 } translators + * @param { Translators } translators * @param {*} dynamicOptions Options bag governing vat creation * * @returns {Promise} @@ -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 } translators + * @param { Translators } translators * @param {*} dynamicOptions Options bag governing vat creation * * @returns {Promise} 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 } translators + * @param { Translators } translators * @param {*} staticOptions Options bag governing vat creation * * @returns {Promise} 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 } 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, diff --git a/packages/SwingSet/src/kernel/vat-warehouse.js b/packages/SwingSet/src/kernel/vat-warehouse.js index 232baeb30ea..7a913eb17f5 100644 --- a/packages/SwingSet/src/kernel/vat-warehouse.js +++ b/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 = { diff --git a/packages/SwingSet/src/kernel/vatTranslator.js b/packages/SwingSet/src/kernel/vatTranslator.js index 299be05a34b..9659b37f9ed 100644 --- a/packages/SwingSet/src/kernel/vatTranslator.js +++ b/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 }); diff --git a/packages/SwingSet/src/lib/capdata.js b/packages/SwingSet/src/lib/capdata.js index 683cd012fc2..8b93940a6da 100644 --- a/packages/SwingSet/src/lib/capdata.js +++ b/packages/SwingSet/src/lib/capdata.js @@ -23,3 +23,25 @@ export function insistCapData(capdata) { ); // TODO check that the .slots array elements are actually strings } + +/** + * Returns the slot of a presence if the provided capdata is composed + * of a single presence, `null` otherwise + * + * @param {import('@endo/marshal').CapData} data + * @returns {string | undefined} + */ +export function extractSingleSlot(data) { + const body = JSON.parse(data.body); + if ( + body && + typeof body === 'object' && + body['@qclass'] === 'slot' && + body.index === 0 + ) { + if (data.slots.length === 1) { + return data.slots[0]; + } + } + return null; +} diff --git a/packages/SwingSet/src/vats/comms/clist-inbound.js b/packages/SwingSet/src/vats/comms/clist-inbound.js index 5431157e6f2..f315d89058a 100644 --- a/packages/SwingSet/src/vats/comms/clist-inbound.js +++ b/packages/SwingSet/src/vats/comms/clist-inbound.js @@ -28,7 +28,7 @@ export function makeInbound(state) { X`attempt to retire remote ${remoteID} subscribed promise ${rpid}`, ); remote.deleteRemoteMapping(lpid); - cdebug(`comms delete mapping r<->k ${remoteID} {rpid}<=>${lpid}`); + cdebug(`comms delete mapping r<->k ${remoteID} ${rpid}<=>${lpid}`); } function beginRemotePromiseIDRetirement(remoteID, rpid) { @@ -132,6 +132,9 @@ export function makeInbound(state) { function provideLocalForRemoteResult(remoteID, result) { insistRemoteType('promise', result); + // Comms is a pipelining vat which means the result may be allocated by + // any side, at any time. Unlike the kernel checks, as long as the decider + // is correct, and the promise is unresolved, it's valid as a result. const lpid = provideLocalForRemote(remoteID, result); // this asserts they had control over lpid, and that it wasn't already // resolved. TODO: reject somehow rather than crash weirdly, we can't diff --git a/packages/SwingSet/src/vats/comms/delivery.js b/packages/SwingSet/src/vats/comms/delivery.js index 8b1d933a276..cab1c6b77b0 100644 --- a/packages/SwingSet/src/vats/comms/delivery.js +++ b/packages/SwingSet/src/vats/comms/delivery.js @@ -4,7 +4,7 @@ import { assert, details as X } from '@agoric/assert'; import { parseLocalSlot, insistLocalType } from './parseLocalSlots.js'; import { makeUndeliverableError } from '../../lib/makeUndeliverableError.js'; -import { insistCapData } from '../../lib/capdata.js'; +import { extractSingleSlot, insistCapData } from '../../lib/capdata.js'; import { insistRemoteType } from './parseRemoteSlot.js'; import { insistRemoteID } from './remote.js'; @@ -246,27 +246,6 @@ export function makeDeliveryKit( handleResolutions(localResolutions); } - function extractPresenceIfPresent(data) { - insistCapData(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 } = parseLocalSlot(slot); - if (type === 'object') { - return slot; - } - } - } - return null; - } - // helper function for handleSend(): for each message, either figure out // the destination (remote machine or kernel), or reject the result because // the destination is a brick wall (undeliverable target) @@ -294,9 +273,9 @@ export function makeDeliveryKit( if (status === 'rejected') { return { reject: data }; } - const targetPresence = extractPresenceIfPresent(data); - if (targetPresence) { - return resolveTarget(targetPresence, method); + const targetSlot = extractSingleSlot(data); + if (targetSlot && parseLocalSlot(targetSlot).type === 'object') { + return resolveTarget(targetSlot, method); } else { return { reject: makeUndeliverableError(method) }; } @@ -451,6 +430,19 @@ export function makeDeliveryKit( const [lpid, rejected, data] = resolution; // rejected: boolean, data: capdata 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 comms. + // Furthermore messages sent to such a promise resolved this way would not + // be forwarded but would splat instead. + assert( + parseLocalSlot(resolutionSlot).type !== 'promise', + 'cannot resolve to a promise', + ); + } + } insistLocalType('promise', lpid); state.insistPromiseIsUnresolved(lpid); state.insistDeciderIsComms(lpid); diff --git a/packages/SwingSet/test/snapshots/test-xsnap-store.js.md b/packages/SwingSet/test/snapshots/test-xsnap-store.js.md index 9d92607369e..5c1e076e7d5 100644 --- a/packages/SwingSet/test/snapshots/test-xsnap-store.js.md +++ b/packages/SwingSet/test/snapshots/test-xsnap-store.js.md @@ -12,8 +12,8 @@ Generated by [AVA](https://avajs.dev). > after SES boot - sensitive to SES-shim, XS, and supervisor - 'dd40da432d48e6e6b9530c8ae3712eb8d4e6c9f98b8e96c6d3638d1c89f2b8b9' + 'e48339a392bbcf34fe316dc20fe8bb218f6ab39b9022e462c125ab1e12ebd18d' > after use of harden() - sensitive to SES-shim, XS, and supervisor - 'e84eba63931e7392c5b16f48ddcc7378ee300f02e91a083d204544902d9bc11c' + 'e3acc0e9480525de15225829e44a7e9be53c8decae561ae896411eca1519f7c4' diff --git a/packages/SwingSet/test/snapshots/test-xsnap-store.js.snap b/packages/SwingSet/test/snapshots/test-xsnap-store.js.snap index a5cd696ef29..fce275fe6e0 100644 Binary files a/packages/SwingSet/test/snapshots/test-xsnap-store.js.snap and b/packages/SwingSet/test/snapshots/test-xsnap-store.js.snap differ diff --git a/packages/SwingSet/test/test-comms.js b/packages/SwingSet/test/test-comms.js index 651c17feba2..76557f6ea01 100644 --- a/packages/SwingSet/test/test-comms.js +++ b/packages/SwingSet/test/test-comms.js @@ -1304,12 +1304,12 @@ function nestedPromisesTestLarryOuter(t, larryInner) { if (larryInner) { pPromiseInner = newImportPromise('k'); } - _('k>r', [pPromiseOuter, false, pPromiseInner]); + _('k>r', [pPromiseOuter, false, [pPromiseInner]]); if (larryInner) { _('k { - nestedPromisesTestLarryOuter(t, true); -}); +test( + 'Nested promises, Larry outer, Larry inner', + nestedPromisesTestLarryOuter, + true, +); -test('Nested promises, Larry outer, Alice inner', t => { - nestedPromisesTestLarryOuter(t, false); -}); +test( + 'Nested promises, Larry outer, Alice inner', + nestedPromisesTestLarryOuter, + false, +); function nestedPromisesTestAliceOuter(t, larryInner) { const { @@ -1389,11 +1393,11 @@ function nestedPromisesTestAliceOuter(t, larryInner) { if (!larryInner) { paPromiseInner = newImportPromise('a'); } - _('a>r', [paPromiseOuter, false, paPromiseInner]); + _('a>r', [paPromiseOuter, false, [paPromiseInner]]); if (!larryInner) { pPromiseInner = newExportPromise('k'); } - _('k { - nestedPromisesTestAliceOuter(t, true); +test( + 'Nested promises, Alice outer, Larry inner', + nestedPromisesTestAliceOuter, + true, +); + +test( + 'Nested promises, Alice outer, Alice inner', + nestedPromisesTestAliceOuter, + false, +); + +test('Nested promises, reject with promise', async t => { + const { + _, + done, + state, + setupRemote, + exportToRemote, + newImportObject, + newImportPromise, + newExportPromise, + refOf, + flipRefOf, + } = commsVatDriver(t); + + setupRemote('a'); + const remoteA = state.getRemote(state.getRemoteIDForName('a')); + const oLarry = newImportObject('k'); + const oaLarry = exportToRemote('a', 11, oLarry); + + const paPromise = newImportPromise('a'); + const pPromise = newExportPromise('k'); + + // Alice sends a message to Larry containing the promise + _('a>m', oaLarry, 'hello', undefined, paPromise); + _('kr', [paPromise, true, paPromise]); + _('k { - nestedPromisesTestAliceOuter(t, false); +test('Disallow resolving a promise with another promise', async t => { + const { + _, + done, + state, + setupRemote, + exportToRemote, + newImportObject, + newImportPromise, + newExportPromise, + refOf, + flipRefOf, + } = commsVatDriver(t); + + setupRemote('a'); + const remoteA = state.getRemote(state.getRemoteIDForName('a')); + const oLarry = newImportObject('k'); + const oaLarry = exportToRemote('a', 11, oLarry); + + // Alice sends a message to Larry + const paResult = newImportPromise('a'); + _('a>m', oaLarry, 'hello', paResult); + const pResult = newExportPromise('k'); + _('k _('k>r', [pResult, false, pPromise])); + _('k { t.deepEqual(kernel.dump().acceptanceQueue, []); }); +test('promise fails when resolve to promise', async t => { + const kernel = makeKernel(); + await kernel.start(); + const log = []; + + let syscallA; + function setupA(s) { + syscallA = s; + function dispatch(vatDeliverObject) { + if (vatDeliverObject[0] === 'startVat') { + return; // skip startVat + } + log.push(vatDeliverObject); + } + return dispatch; + } + await kernel.createTestVat('vatA', setupA); + + let syscallB; + function setupB(s) { + syscallB = s; + function dispatch() {} + return dispatch; + } + await kernel.createTestVat('vatB', setupB); + + const vatA = kernel.vatNameToID('vatA'); + const vatB = kernel.vatNameToID('vatB'); + + const p1ForB = 'p+5'; + const p1ForKernel = kernel.addExport(vatB, p1ForB); + const p1ForA = kernel.addImport(vatA, p1ForKernel); + + const p2ForB = 'p+6'; + + syscallA.subscribe(p1ForA); + t.deepEqual(kernel.dump().promises, [ + { + id: p1ForKernel, + state: 'unresolved', + policy: 'ignore', + refCount: 3, + decider: vatB, + subscribers: [vatA], + queue: [], + }, + ]); + + t.throws( + () => syscallB.resolve([[p1ForB, false, capargs(slot0arg, [p2ForB])]]), + undefined, + `Should throw when resolving to promise`, + ); + t.deepEqual(log, []); + t.deepEqual(kernel.dump().runQueue, []); + t.deepEqual(kernel.dump().acceptanceQueue, []); +}); + test('promise reject', async t => { const kernel = makeKernel(); await kernel.start(); @@ -967,7 +1025,6 @@ test('promise reject', async t => { const vatA = kernel.vatNameToID('vatA'); const vatB = kernel.vatNameToID('vatB'); - const aliceForA = 'o+6'; const pForB = 'p+5'; const pForKernel = kernel.addExport(vatB, pForB); const pForA = kernel.addImport(vatA, pForKernel); @@ -989,7 +1046,8 @@ test('promise reject', async t => { }, ]); - syscallB.resolve([[pForB, true, capdata('"args"', [aliceForA])]]); + // Reject the promise with itself + syscallB.resolve([[pForB, true, capargs(slot0arg, [pForB])]]); // this causes a notify message to be queued t.deepEqual(log, []); // no other dispatch calls t.deepEqual(kernel.dump().acceptanceQueue, [ @@ -1007,7 +1065,7 @@ test('promise reject', async t => { // the kernelPromiseID gets mapped back to the vat PromiseID t.deepEqual(log.shift(), [ 'notify', - oneResolution(pForA, true, capdata('"args"', ['o-50'])), + oneResolution(pForA, true, capargs(slot0arg, [pForA])), ]); t.deepEqual(log, []); // no other dispatch calls t.deepEqual(kernel.dump().runQueue, []); diff --git a/packages/SwingSet/test/test-vpid-kernel.js b/packages/SwingSet/test/test-vpid-kernel.js index d367682f200..ead72e5669f 100644 --- a/packages/SwingSet/test/test-vpid-kernel.js +++ b/packages/SwingSet/test/test-vpid-kernel.js @@ -46,7 +46,12 @@ function makeEndowments() { }; } -async function buildRawVat(name, kernel, onDispatchCallback = undefined) { +async function buildRawVat( + name, + kernel, + onDispatchCallback = undefined, + options = undefined, +) { const { log, dispatch } = buildDispatch(onDispatchCallback); let syscall; function setup(s) { @@ -57,7 +62,7 @@ async function buildRawVat(name, kernel, onDispatchCallback = undefined) { function getSyscall() { return syscall; } - await kernel.createTestVat(name, setup); + await kernel.createTestVat(name, setup, undefined, options); return { log, getSyscall }; } @@ -128,7 +133,7 @@ function doResolveSyscall(syscallA, vpid, mode, targets) { syscallA.resolve([[vpid, true, capargs('error', [])]]); break; case 'promise-reject': - syscallA.resolve([[vpid, true, capargs([slot0arg], [targets.p1])]]); + syscallA.resolve([[vpid, true, capargs(slot0arg, [targets.p1])]]); break; default: assert.fail(X`unknown mode ${mode}`); @@ -177,11 +182,7 @@ function resolutionOf(vpid, mode, targets) { case 'promise-reject': return { type: 'notify', - resolutions: oneResolution( - vpid, - true, - capargs([slot0arg], [targets.p1]), - ), + resolutions: oneResolution(vpid, true, capargs(slot0arg, [targets.p1])), }; default: assert.fail(X`unknown mode ${mode}`); @@ -233,6 +234,12 @@ async function doTest123(t, which, mode) { const { log: logB, getSyscall: getSyscallB } = await buildRawVat( 'vatB', kernel, + undefined, + { + // Test 3 sends a promise before using it as result, which only + // pipelining vats are allowed to do + enablePipelining: which === 3, + }, ); const syscallA = getSyscallA(); const syscallB = getSyscallB(); @@ -402,6 +409,11 @@ async function doTest4567(t, which, mode) { 'vatA', kernel, odc, + { + // Test 7 sends a promise before using it as result, which only + // pipelining vats are allowed to do + enablePipelining: which === 7, + }, ); // we use vatB when necessary to send messages to vatA const { log: logB, getSyscall: getSyscallB } = await buildRawVat( @@ -847,3 +859,230 @@ test(`kernel vpid handling crossing resolutions`, async t => { t.is(inCList(kernel, vatX, genResultBkernel, exportedGenResultBvatX), false); // **** end Crank 5 (B) **** }); + +async function doReflectedMessageTest(t, enablePipelining) { + const endowments = makeEndowments(); + initializeKernel({}, endowments.hostStorage); + const kernel = buildKernel(endowments, {}, {}); + await kernel.start(undefined); // no bootstrapVatName, so no bootstrap call + + // This is a redux of message pattern a80 + // We could simplify the case but keep some of the setup ceremony for legibility + + // we use vatA as the sender of messages to B + const { log: logA, getSyscall: getSyscallA } = await buildRawVat( + 'vatA', + kernel, + undefined, + ); + // Build a pipelining vat which will reflect a message send to a promise + const { log: logB, getSyscall: getSyscallB } = await buildRawVat( + 'vatB', + kernel, + undefined, + { enablePipelining }, + ); + const syscallA = getSyscallA(); + const syscallB = getSyscallB(); + + const vatA = kernel.vatNameToID('vatA'); + const vatB = kernel.vatNameToID('vatB'); + + // B will need a reference to A + const rootAvatA = 'o+0'; + const rootAkernel = kernel.addExport(vatA, rootAvatA); + const rootAvatB = kernel.addImport(vatB, rootAkernel); + + // Bob sends a Promise to Alice as argument (leaving Bob as decider) + // Alice sends a message to the received promise + // Bob resolves the promise to an object in A (Alice itself here) (extraneous for this test) + // Bob receives the message from Alice and reflects it back to Alice + // B: alice~.one(p) + // A: async function one (p) { r = p~.two(); } + // B: p::resolve(alice) + // B: alice~.two(result=r) + + const exportedPVatB = 'p+1'; + syscallB.send(rootAvatB, 'one', capargs([slot0arg], [exportedPVatB])); + const pKernel = clistVatToKernel(kernel, vatB, exportedPVatB); + await kernel.run(); + const importedPVatA = clistKernelToVat(kernel, vatA, pKernel); + t.truthy(importedPVatA); + // expect logA to have deliver(one) + t.deepEqual(logA.shift(), { + type: 'deliver', + targetSlot: rootAvatA, + method: 'one', + args: capargs([slot0arg], [importedPVatA]), + resultSlot: null, + }); + t.deepEqual(logA, []); + t.deepEqual(logB, []); + + const exportedRPVatA = 'p+2'; + syscallA.send(importedPVatA, 'two', capargs([], []), exportedRPVatA); + syscallA.subscribe(exportedRPVatA); + const rpKernel = clistVatToKernel(kernel, vatA, exportedRPVatA); + await kernel.run(); + // Send is queued on the promise + if (enablePipelining) { + const importedRPVatB = clistKernelToVat(kernel, vatB, rpKernel); + t.truthy(importedRPVatB); + t.deepEqual(logB.shift(), { + type: 'deliver', + targetSlot: exportedPVatB, + method: 'two', + args: capargs([], []), + resultSlot: importedRPVatB, + }); + t.deepEqual(logA, []); + t.deepEqual(logB, []); + + syscallB.send(rootAvatB, 'two', capargs([], []), importedRPVatB); + await kernel.run(); + } else { + // Send is queued to promise + t.deepEqual(logA, []); + t.deepEqual(logB, []); + + syscallB.resolve([[exportedPVatB, false, capargs(slot0arg, [rootAvatB])]]); + await kernel.run(); + } + + // expect logA to have deliver(two) + t.deepEqual(logA.shift(), { + type: 'deliver', + targetSlot: rootAvatA, + method: 'two', + args: capargs([], []), + resultSlot: exportedRPVatA, + }); + t.deepEqual(logA, []); + t.deepEqual(logB, []); +} + +doReflectedMessageTest.title = (_, enablePipelining) => + `kernel vpid handling reflected message (enablePipelining=${enablePipelining})`; + +test('', doReflectedMessageTest, true); +test('', doReflectedMessageTest, false); + +test('kernel vpid handling rejects imported result promise', async t => { + const endowments = makeEndowments(); + initializeKernel({}, endowments.hostStorage); + const kernel = buildKernel(endowments, {}, {}); + await kernel.start(undefined); // no bootstrapVatName, so no bootstrap call + + // This is negative test checking that non-pipelining vats are prevented + // from using an imported promise as result + + // we use vatA as the sender of messages to B + const { log: logA, getSyscall: getSyscallA } = await buildRawVat( + 'vatA', + kernel, + undefined, + ); + // vatB is non-pipelining + const { log: logB, getSyscall: getSyscallB } = await buildRawVat( + 'vatB', + kernel, + undefined, + { enablePipelining: false }, + ); + const syscallA = getSyscallA(); + const syscallB = getSyscallB(); + + const vatA = kernel.vatNameToID('vatA'); + const vatB = kernel.vatNameToID('vatB'); + + // A will need a reference to B, to send anything, and vice versa + const rootBvatB = 'o+0'; + const rootBkernel = kernel.addExport(vatB, rootBvatB); + const rootBvatA = kernel.addImport(vatA, rootBkernel); + const rootAvatA = 'o+0'; + const rootAkernel = kernel.addExport(vatA, rootAvatA); + const rootAvatB = kernel.addImport(vatB, rootAkernel); + + // Alice sends a message to Bob (leaving Bob as decider of result promise) + // Bob tries to send a message to Alice reusing the result promise from step one + // A: r = bob~.one() + // B: alice~.two(result=r) + + const exportedPRVatA = 'p+1'; + syscallA.send(rootBvatA, 'one', capargs([], []), exportedPRVatA); + syscallA.subscribe(exportedPRVatA); + const prKernel = clistVatToKernel(kernel, vatA, exportedPRVatA); + await kernel.run(); + const importedPRVatB = clistKernelToVat(kernel, vatB, prKernel); + t.truthy(importedPRVatB); + // expect logB to have deliver(one) + t.deepEqual(logB.shift(), { + type: 'deliver', + targetSlot: rootBvatB, + method: 'one', + args: capargs([], []), + resultSlot: importedPRVatB, + }); + t.deepEqual(logA, []); + t.deepEqual(logB, []); + + t.throws( + () => syscallB.send(rootAvatB, 'two', capargs([], []), importedPRVatB), + undefined, + 'Send reusing imported promise should throw', + ); + syscallB.subscribe(importedPRVatB); + await kernel.run(); + + t.deepEqual(logA, []); + t.deepEqual(logB, []); +}); + +test('kernel vpid handling rejects previously exported result promise', async t => { + const endowments = makeEndowments(); + initializeKernel({}, endowments.hostStorage); + const kernel = buildKernel(endowments, {}, {}); + await kernel.start(undefined); // no bootstrapVatName, so no bootstrap call + + // This is negative test checking that non-pipelining vats are prevented + // from using a previously exported promise for which they retained + // decider-ship as result + + // we use vatA as the non-pipelining sender of messages to B + const { log: logA, getSyscall: getSyscallA } = await buildRawVat( + 'vatA', + kernel, + undefined, + { enablePipelining: false }, + ); + // vatB is the placeholder receiver + const { log: logB } = await buildRawVat('vatB', kernel, undefined); + const syscallA = getSyscallA(); + + const vatA = kernel.vatNameToID('vatA'); + const vatB = kernel.vatNameToID('vatB'); + + // A will need a reference to B, to send anything + const rootBvatB = 'o+0'; + const rootBkernel = kernel.addExport(vatB, rootBvatB); + const rootBvatA = kernel.addImport(vatA, rootBkernel); + + // Alice allocates a promise and sends it to Bob in the arguments of a + // message as well as using it as the result of the message send + // A: p1 = new Promise(); + // A: bob~.one(result=p1, p1) + + const exportedPRVatA = 'p+1'; + t.throws(() => + syscallA.send( + rootBvatA, + 'one', + capargs(slot0arg, [exportedPRVatA]), + exportedPRVatA, + ), + ); + syscallA.subscribe(exportedPRVatA); + await kernel.run(); + t.deepEqual(logA, []); + t.deepEqual(logB, []); +}); diff --git a/packages/SwingSet/test/test-vpid-liveslots.js b/packages/SwingSet/test/test-vpid-liveslots.js index ba5a9c3448a..8f1bf54938e 100644 --- a/packages/SwingSet/test/test-vpid-liveslots.js +++ b/packages/SwingSet/test/test-vpid-liveslots.js @@ -97,7 +97,7 @@ function resolvePR(pr, mode, targets) { pr.reject('error'); break; case 'promise-reject': - pr.reject([targets.p1]); + pr.reject(targets.p1); break; default: assert.fail(X`unknown mode ${mode}`); @@ -143,7 +143,7 @@ function resolutionOf(vpid, mode, targets) { break; case 'promise-reject': resolution.resolutions[0][1] = true; - resolution.resolutions[0][2] = capargs([slot0arg], [targets.p1]); + resolution.resolutions[0][2] = capargs(slot0arg, [targets.p1]); break; default: assert.fail(X`unknown mode ${mode}`); @@ -608,7 +608,7 @@ async function doVatResolveCase4(t, mode) { } else if (mode === 'reject') { r = makeReject(p1, capargs('error', [])); } else if (mode === 'promise-reject') { - r = makeReject(p1, capargs([slot0arg], [p1])); + r = makeReject(p1, capargs(slot0arg, [p1])); } else { assert.fail(X`unknown mode ${mode}`); } diff --git a/patches/@endo+eventual-send+0.15.3.patch b/patches/@endo+eventual-send+0.15.3.patch new file mode 100644 index 00000000000..8074df6bb88 --- /dev/null +++ b/patches/@endo+eventual-send+0.15.3.patch @@ -0,0 +1,28 @@ +diff --git a/node_modules/@endo/eventual-send/src/handled-promise.js b/node_modules/@endo/eventual-send/src/handled-promise.js +index 109c5cf..2029e86 100644 +--- a/node_modules/@endo/eventual-send/src/handled-promise.js ++++ b/node_modules/@endo/eventual-send/src/handled-promise.js +@@ -45,6 +45,11 @@ const coerceToObjectProperty = specimen => { + return String(specimen); + }; + ++const hardenVoid = obj => { ++ // Do not return the hardened value ++ harden(obj); ++}; ++ + // the following method (makeHandledPromise) is part + // of the shim, and will not be exported by the module once the feature + // becomes a part of standard javascript +@@ -548,8 +553,9 @@ export const makeHandledPromise = () => { + + // Harden the fulfillment and rejection, as well as a workaround for + // Node.js: silence "Unhandled Rejection" by default when using the static +- // methods. +- returnedP.then(harden, harden); ++ // methods. Use a void returning version of harden to prevent transforming ++ // a rejected promise used as reason into a new unhandled rejection. ++ returnedP.then(hardenVoid, hardenVoid); + + // We return a handled promise with the default pending handler. This + // prevents a race between the above Promise.resolves and pipelining.