From 9519f5f9acb8859a35e6d198e54be1bf686a314c Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Thu, 6 Jul 2023 18:44:36 -0700 Subject: [PATCH] feat(vat-data): virtual exos are revocable --- packages/swingset-liveslots/src/cache.js | 5 +- .../swingset-liveslots/src/vatDataTypes.d.ts | 17 ++- .../src/virtualObjectManager.js | 18 +++ packages/vat-data/test/test-revoke-virtual.js | 131 ++++++++++++++++++ 4 files changed, 167 insertions(+), 4 deletions(-) create mode 100644 packages/vat-data/test/test-revoke-virtual.js diff --git a/packages/swingset-liveslots/src/cache.js b/packages/swingset-liveslots/src/cache.js index 4186aa9a464f..b69bf297c9ee 100644 --- a/packages/swingset-liveslots/src/cache.js +++ b/packages/swingset-liveslots/src/cache.js @@ -17,7 +17,7 @@ import { Fail } from '@agoric/assert'; /** * @callback CacheDelete * @param {string} key - * @returns {void} + * @returns {boolean} * * @callback CacheFlush * @returns {void} @@ -82,8 +82,9 @@ export function makeCache(readBacking, writeBacking, deleteBacking) { }, delete: key => { assert.typeof(key, 'string'); - stash.delete(key); + const result = stash.delete(key); dirtyKeys.add(key); + return result; }, flush: () => { const keys = [...dirtyKeys.keys()]; diff --git a/packages/swingset-liveslots/src/vatDataTypes.d.ts b/packages/swingset-liveslots/src/vatDataTypes.d.ts index 1bbe82c33eb6..69d333530a97 100644 --- a/packages/swingset-liveslots/src/vatDataTypes.d.ts +++ b/packages/swingset-liveslots/src/vatDataTypes.d.ts @@ -15,10 +15,18 @@ import type { } from '@agoric/store'; import type { makeWatchedPromiseManager } from './watchedPromises.js'; +import type { + FarClassOptions, + StateShape, + Context, + KitContext, + Revoker, + ReceiveRevoker, +} from '@endo/exo'; + // TODO should be moved into @endo/patterns and eventually imported here // instead of this local definition. export type InterfaceGuardKit = Record; - export type { MapStore, Pattern }; // This needs `any` values. If they were `unknown`, code that uses Baggage @@ -88,7 +96,12 @@ export type DefineKindOptions = { * If provided, it describes the shape of all state records of instances * of this kind. */ - stateShape?: { [name: string]: Pattern }; + stateShape?: StateShape; + + /** + * If provided, it is called with a revoke function as an argument. + */ + receiveRevoker?: ReceiveRevoker; /** * Intended for internal use only. diff --git a/packages/swingset-liveslots/src/virtualObjectManager.js b/packages/swingset-liveslots/src/virtualObjectManager.js index 3fc056e0ae99..63686bdbc65e 100644 --- a/packages/swingset-liveslots/src/virtualObjectManager.js +++ b/packages/swingset-liveslots/src/virtualObjectManager.js @@ -153,6 +153,9 @@ const makeContextCache = (makeState, makeContext) => { const makeContextProvider = (contextCache, getSlotForVal) => harden(rep => contextCache.get(getSlotForVal(rep))); +const makeContextRevoker = (contextCache, getSlotForVal) => + harden(rep => contextCache.delete(getSlotForVal(rep))); + const makeContextProviderKit = (contextCache, getSlotForVal, facetNames) => { /** @type { Record } */ const contextProviderKit = {}; @@ -174,6 +177,13 @@ const makeContextProviderKit = (contextCache, getSlotForVal, facetNames) => { return harden(contextProviderKit); }; +// TODO BUG The returned function revokes the whole kit, i.e., all vrefs +// sharing the same baseRef. This makes me wonder whether I need to rethink +// revocation yet again. Perhaps an entire kit is the correct unit and +// the api should be reconcieved. +const makeContextFacetRevoker = (contextCache, getSlotForVal) => + harden(rep => contextCache.delete(parseVatSlot(getSlotForVal(rep)).baseRef)); + // The management of single Representatives (i.e. defineKind) is very similar // to that of a cohort of facets (i.e. defineKindMulti). In this description, // we use "self/facets" to refer to either 'self' or 'facets', as appropriate @@ -718,6 +728,7 @@ export const makeVirtualObjectManager = ( finish = undefined, stateShape = undefined, thisfulMethods = false, + receiveRevoker = undefined, } = options; let { // These are "let" rather than "const" only to accommodate code @@ -1076,6 +1087,13 @@ export const makeVirtualObjectManager = ( return val; }; + if (receiveRevoker) { + const makeRevoker = multifaceted + ? makeContextFacetRevoker + : makeContextRevoker; + receiveRevoker(makeRevoker(contextCache, getSlotForVal)); + } + return makeNewInstance; }; diff --git a/packages/vat-data/test/test-revoke-virtual.js b/packages/vat-data/test/test-revoke-virtual.js new file mode 100644 index 000000000000..c08533d9a10c --- /dev/null +++ b/packages/vat-data/test/test-revoke-virtual.js @@ -0,0 +1,131 @@ +// Modeled on test-revoke-heap-classes.js + +import { test } from './prepare-test-env-ava.js'; + +// eslint-disable-next-line import/order +import { M } from '@agoric/store'; +import { + defineVirtualExoClass, + defineVirtualExoClassKit, +} from '../src/exo-utils.js'; + +const { apply } = Reflect; + +const UpCounterI = M.interface('UpCounter', { + incr: M.call() + // TODO M.number() should not be needed to get a better error message + .optional(M.and(M.number(), M.gte(0))) + .returns(M.number()), +}); + +const DownCounterI = M.interface('DownCounter', { + decr: M.call() + // TODO M.number() should not be needed to get a better error message + .optional(M.and(M.number(), M.gte(0))) + .returns(M.number()), +}); + +test('test revoke defineVirtualExoClass', t => { + let revoke; + const makeUpCounter = defineVirtualExoClass( + 'UpCounter', + UpCounterI, + /** @param {number} x */ + (x = 0) => ({ x }), + { + incr(y = 1) { + const { state } = this; + state.x += y; + return state.x; + }, + }, + { + receiveRevoker(r) { + revoke = r; + }, + }, + ); + const upCounter = makeUpCounter(3); + t.is(upCounter.incr(5), 8); + // @ts-expect-error Does not understand that `revoke` is a function. + t.is(revoke(upCounter), true); + t.throws(() => upCounter.incr(1), { + message: + '"In \\"incr\\" method of (UpCounter)" may only be applied to a valid instance: "[Alleged: UpCounter]"', + }); +}); + +test('test revoke defineVirtualExoClassKit', t => { + let revoke; + const makeCounterKit = defineVirtualExoClassKit( + 'Counter', + { up: UpCounterI, down: DownCounterI }, + /** @param {number} x */ + (x = 0) => ({ x }), + { + up: { + incr(y = 1) { + const { state } = this; + state.x += y; + return state.x; + }, + }, + down: { + decr(y = 1) { + const { state } = this; + state.x -= y; + return state.x; + }, + }, + }, + { + receiveRevoker(r) { + revoke = r; + }, + }, + ); + const { up: upCounter, down: downCounter } = makeCounterKit(3); + t.is(upCounter.incr(5), 8); + t.is(downCounter.decr(), 7); + // @ts-expect-error Does not understand that `revoke` is a function. + t.is(revoke(upCounter), true); + t.throws(() => upCounter.incr(3), { + message: + '"In \\"incr\\" method of (Counter up)" may only be applied to a valid instance: "[Alleged: Counter up]"', + }); + // @ts-expect-error Does not understand that `revoke` is a function. + t.is(revoke(downCounter), false); + t.throws(() => downCounter.decr(), { + message: + '"In \\"decr\\" method of (Counter down)" may only be applied to a valid instance: "[Alleged: Counter down]"', + }); +}); + +test('test virtual facet cross-talk', t => { + const makeCounterKit = defineVirtualExoClassKit( + 'Counter', + { up: UpCounterI, down: DownCounterI }, + /** @param {number} x */ + (x = 0) => ({ x }), + { + up: { + incr(y = 1) { + const { state } = this; + state.x += y; + return state.x; + }, + }, + down: { + decr(y = 1) { + const { state } = this; + state.x -= y; + return state.x; + }, + }, + }, + ); + const { up: upCounter, down: downCounter } = makeCounterKit(3); + t.throws(() => apply(upCounter.incr, downCounter, [2]), { + message: 'illegal cross-facet access', + }); +});