From 1a167d77e06cf5ac61f96bd9e5b4df76658d9d00 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Tue, 17 May 2022 13:08:46 -0400 Subject: [PATCH 01/18] feat(cosmic-swingset): Add chainStorage interface Fixes #4558 --- packages/vats/src/bridge-ids.js | 1 + packages/vats/src/core/chain-behaviors.js | 70 +++++++++++++++++++++++ packages/vats/src/core/manifest.js | 8 +++ packages/vats/src/core/types.js | 1 + 4 files changed, 80 insertions(+) diff --git a/packages/vats/src/bridge-ids.js b/packages/vats/src/bridge-ids.js index cbb328d72b7..b86a362cfc5 100644 --- a/packages/vats/src/bridge-ids.js +++ b/packages/vats/src/bridge-ids.js @@ -2,5 +2,6 @@ export const BANK = 'bank'; export const CORE = 'core'; export const DIBC = 'dibc'; +export const STORAGE = 'storage'; export const PROVISION = 'provision'; export const WALLET = 'wallet'; diff --git a/packages/vats/src/core/chain-behaviors.js b/packages/vats/src/core/chain-behaviors.js index f42ee64039d..cd2776748be 100644 --- a/packages/vats/src/core/chain-behaviors.js +++ b/packages/vats/src/core/chain-behaviors.js @@ -282,6 +282,76 @@ export const makeBridgeManager = async ({ }; harden(makeBridgeManager); +// TODO: Refine typing... maybe something like PromiseSpace or PromiseSpaceOf? +/** + * @param { BootstrapPowers } powers + */ +export const makeChainStorage = async ({ + consume: { bridgeManager: bridgeManagerP }, + produce: { chainStorage: chainStorageP }, +}) => { + const bridgeManager = await bridgeManagerP; + if (!bridgeManager) { + console.warn('Cannot support chainStorage without an actual chain.'); + chainStorageP.resolve(undefined); + return; + } + const toStorage = message => + E(bridgeManager).toBridge(BRIDGE_ID.STORAGE, message); + + // TODO: Formalize root key. + // Must not be any of {activityhash,beansOwing,egress,mailbox}, + // and must be reserved in sites that use those keys (both Go and JS). + const ROOT_KEY = 'published'; + // TODO: Formalize segment constraints. + // Must be nonempty and disallow (unescaped) `.`, and for simplicity + // (and future possibility of e.g. escaping) we currently limit to + // ASCII alphanumeric plus underscore. + const pathSegmentPattern = /^[a-zA-Z0-9_]{1,100}$/; + const makeChainStorageNode = key => { + const node = { + getKey() { + return key; + }, + getChildNode(name) { + assert.typeof(name, 'string'); + if (!pathSegmentPattern.test(name)) { + assert.fail( + X`Path segment must be a short ASCII identifier: ${name}`, + ); + } + return makeChainStorageNode(`${key}.${name}`); + }, + setValue(value) { + assert.typeof(value, 'string'); + // TODO: Fix on the Go side. + // https://github.com/Agoric/agoric-sdk/issues/5381 + assert(value !== ''); + toStorage({ key, method: 'set', value }); + }, + async delete() { + assert(key !== ROOT_KEY); + const childKeys = await toStorage({ key, method: 'keys' }); + if (childKeys.length > 0) { + assert.fail(X`Refusing to delete node with children: ${key}`); + } + toStorage({ key, method: 'set' }); + }, + // Possible extensions: + // * getValue() + // * getChildNames() and/or getChildNodes() + // * getName() + // * recursive delete + // * batch operations + // * local buffering (with end-of-block commit) + }; + return Far('chainStorageNode', node); + }; + + const rootNode = makeChainStorageNode(ROOT_KEY); + chainStorageP.resolve(rootNode); +}; + /** * no free lunch on chain * diff --git a/packages/vats/src/core/manifest.js b/packages/vats/src/core/manifest.js index a7d0ebddf02..365e3423d4b 100644 --- a/packages/vats/src/core/manifest.js +++ b/packages/vats/src/core/manifest.js @@ -82,6 +82,14 @@ const SHARED_CHAIN_BOOTSTRAP_MANIFEST = harden({ }, home: { produce: { chainTimerService: 'timer' } }, }, + makeChainStorage: { + consume: { + bridgeManager: true, + }, + produce: { + chainStorage: true, + }, + }, makeClientBanks: { consume: { bankManager: 'bank', diff --git a/packages/vats/src/core/types.js b/packages/vats/src/core/types.js index 680e7c7bd77..d9dad0c59d9 100644 --- a/packages/vats/src/core/types.js +++ b/packages/vats/src/core/types.js @@ -189,6 +189,7 @@ * bldIssuerKit: RemoteIssuerKit, * board: Board, * bridgeManager: OptionalBridgeManager, + * chainStorage: unknown, * chainTimerService: TimerService, * client: ClientManager, * clientCreator: ClientCreator, From e10038b32e88d4d31f403b76cdbd8063eb1f0d8e Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Thu, 19 May 2022 13:57:30 -0400 Subject: [PATCH 02/18] refactor(cosmic-swingset): Move makeChainStorageNode into a vat --- packages/vats/decentral-core-config.json | 3 ++ packages/vats/src/core/chain-behaviors.js | 57 +++------------------ packages/vats/src/core/manifest.js | 1 + packages/vats/src/vat-chainStorage.js | 61 +++++++++++++++++++++++ 4 files changed, 73 insertions(+), 49 deletions(-) create mode 100644 packages/vats/src/vat-chainStorage.js diff --git a/packages/vats/decentral-core-config.json b/packages/vats/decentral-core-config.json index 6c971e0b0be..c8ca692d069 100644 --- a/packages/vats/decentral-core-config.json +++ b/packages/vats/decentral-core-config.json @@ -21,6 +21,9 @@ "board": { "sourceSpec": "@agoric/vats/src/vat-board.js" }, + "chainStorage": { + "sourceSpec": "./src/vat-chainStorage.js" + }, "distributeFees": { "sourceSpec": "@agoric/vats/src/vat-distributeFees.js" }, diff --git a/packages/vats/src/core/chain-behaviors.js b/packages/vats/src/core/chain-behaviors.js index cd2776748be..a1242f8691a 100644 --- a/packages/vats/src/core/chain-behaviors.js +++ b/packages/vats/src/core/chain-behaviors.js @@ -287,7 +287,7 @@ harden(makeBridgeManager); * @param { BootstrapPowers } powers */ export const makeChainStorage = async ({ - consume: { bridgeManager: bridgeManagerP }, + consume: { bridgeManager: bridgeManagerP, loadVat }, produce: { chainStorage: chainStorageP }, }) => { const bridgeManager = await bridgeManagerP; @@ -296,60 +296,19 @@ export const makeChainStorage = async ({ chainStorageP.resolve(undefined); return; } - const toStorage = message => - E(bridgeManager).toBridge(BRIDGE_ID.STORAGE, message); // TODO: Formalize root key. // Must not be any of {activityhash,beansOwing,egress,mailbox}, // and must be reserved in sites that use those keys (both Go and JS). const ROOT_KEY = 'published'; - // TODO: Formalize segment constraints. - // Must be nonempty and disallow (unescaped) `.`, and for simplicity - // (and future possibility of e.g. escaping) we currently limit to - // ASCII alphanumeric plus underscore. - const pathSegmentPattern = /^[a-zA-Z0-9_]{1,100}$/; - const makeChainStorageNode = key => { - const node = { - getKey() { - return key; - }, - getChildNode(name) { - assert.typeof(name, 'string'); - if (!pathSegmentPattern.test(name)) { - assert.fail( - X`Path segment must be a short ASCII identifier: ${name}`, - ); - } - return makeChainStorageNode(`${key}.${name}`); - }, - setValue(value) { - assert.typeof(value, 'string'); - // TODO: Fix on the Go side. - // https://github.com/Agoric/agoric-sdk/issues/5381 - assert(value !== ''); - toStorage({ key, method: 'set', value }); - }, - async delete() { - assert(key !== ROOT_KEY); - const childKeys = await toStorage({ key, method: 'keys' }); - if (childKeys.length > 0) { - assert.fail(X`Refusing to delete node with children: ${key}`); - } - toStorage({ key, method: 'set' }); - }, - // Possible extensions: - // * getValue() - // * getChildNames() and/or getChildNodes() - // * getName() - // * recursive delete - // * batch operations - // * local buffering (with end-of-block commit) - }; - return Far('chainStorageNode', node); - }; - const rootNode = makeChainStorageNode(ROOT_KEY); - chainStorageP.resolve(rootNode); + const vat = E(loadVat)('chainStorage'); + const rootNodeP = E(vat).createChainStorageRoot( + bridgeManager, + BRIDGE_ID.STORAGE, + ROOT_KEY, + ); + chainStorageP.resolve(rootNodeP); }; /** diff --git a/packages/vats/src/core/manifest.js b/packages/vats/src/core/manifest.js index 365e3423d4b..280294cb5ce 100644 --- a/packages/vats/src/core/manifest.js +++ b/packages/vats/src/core/manifest.js @@ -85,6 +85,7 @@ const SHARED_CHAIN_BOOTSTRAP_MANIFEST = harden({ makeChainStorage: { consume: { bridgeManager: true, + loadVat: true, }, produce: { chainStorage: true, diff --git a/packages/vats/src/vat-chainStorage.js b/packages/vats/src/vat-chainStorage.js new file mode 100644 index 00000000000..a99bbda9641 --- /dev/null +++ b/packages/vats/src/vat-chainStorage.js @@ -0,0 +1,61 @@ +import { E, Far } from '@endo/far'; + +const { details: X } = assert; + +// TODO: Extract into testable library? +export function buildRootObject(_vatPowers) { + function createChainStorageRoot(bridgeManager, bridgeId, rootKey) { + const toStorage = message => E(bridgeManager).toBridge(bridgeId, message); + + // TODO: Formalize segment constraints. + // Must be nonempty and disallow (unescaped) `.`, and for simplicity + // (and future possibility of e.g. escaping) we currently limit to + // ASCII alphanumeric plus underscore. + const pathSegmentPattern = /^[a-zA-Z0-9_]{1,100}$/; + const makeChainStorageNode = key => { + const node = { + getKey() { + return key; + }, + getChildNode(name) { + assert.typeof(name, 'string'); + if (!pathSegmentPattern.test(name)) { + assert.fail( + X`Path segment must be a short ASCII identifier: ${name}`, + ); + } + return makeChainStorageNode(`${key}.${name}`); + }, + setValue(value) { + assert.typeof(value, 'string'); + // TODO: Fix on the Go side. + // https://github.com/Agoric/agoric-sdk/issues/5381 + assert(value !== ''); + toStorage({ key, method: 'set', value }); + }, + async delete() { + assert(key !== rootKey); + const childKeys = await toStorage({ key, method: 'keys' }); + if (childKeys.length > 0) { + assert.fail(X`Refusing to delete node with children: ${key}`); + } + toStorage({ key, method: 'set' }); + }, + // Possible extensions: + // * getValue() + // * getChildNames() and/or getChildNodes() + // * getName() + // * recursive delete + // * batch operations + // * local buffering (with end-of-block commit) + }; + return Far('chainStorageNode', node); + }; + + const rootNode = makeChainStorageNode(rootKey); + return rootNode; + } + return Far('root', { + createChainStorageRoot, + }); +} From 485a13428781aa819c16b754f248b118a51d9bbf Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Thu, 19 May 2022 14:04:05 -0400 Subject: [PATCH 03/18] chore(cosmic-swingset): Respond to PR feedback --- packages/vats/src/core/chain-behaviors.js | 1 - packages/vats/src/vat-chainStorage.js | 22 ++++++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/packages/vats/src/core/chain-behaviors.js b/packages/vats/src/core/chain-behaviors.js index a1242f8691a..1418969a34a 100644 --- a/packages/vats/src/core/chain-behaviors.js +++ b/packages/vats/src/core/chain-behaviors.js @@ -282,7 +282,6 @@ export const makeBridgeManager = async ({ }; harden(makeBridgeManager); -// TODO: Refine typing... maybe something like PromiseSpace or PromiseSpaceOf? /** * @param { BootstrapPowers } powers */ diff --git a/packages/vats/src/vat-chainStorage.js b/packages/vats/src/vat-chainStorage.js index a99bbda9641..2fbfea74a4c 100644 --- a/packages/vats/src/vat-chainStorage.js +++ b/packages/vats/src/vat-chainStorage.js @@ -19,22 +19,32 @@ export function buildRootObject(_vatPowers) { }, getChildNode(name) { assert.typeof(name, 'string'); - if (!pathSegmentPattern.test(name)) { - assert.fail( - X`Path segment must be a short ASCII identifier: ${name}`, - ); - } + assert( + pathSegmentPattern.test(name), + X`Path segment must be a short ASCII identifier: ${name}`, + ); return makeChainStorageNode(`${key}.${name}`); }, setValue(value) { assert.typeof(value, 'string'); - // TODO: Fix on the Go side. + // TODO: Fix on the chain side. // https://github.com/Agoric/agoric-sdk/issues/5381 assert(value !== ''); toStorage({ key, method: 'set', value }); }, async delete() { assert(key !== rootKey); + // A 'set' with no value deletes a key but leaves any descendants. + // We therefore want to disallow that (at least for now). + // This check is unfortunately racy (e.g., a vat could wake up + // and set data for a child before _this_ vat receives an + // already-enqueued no-children response), but we can tolerate + // that once https://github.com/Agoric/agoric-sdk/issues/5381 + // is fixed because then the 'set' message sent after erroneously + // passing the no-children check will be transformed into a + // set-to-empty which is at least *visibly* not deleted and + // indistinguishable from a valid deep-create scenario (in which + // parent keys are created automatically). const childKeys = await toStorage({ key, method: 'keys' }); if (childKeys.length > 0) { assert.fail(X`Refusing to delete node with children: ${key}`); From f007dd43e58cdcc98d3a140cb26bf331df33a993 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Fri, 20 May 2022 13:16:56 -0400 Subject: [PATCH 04/18] refactor(cosmic-swingset): Move chainStorage logic into a library --- packages/vats/src/core/chain-behaviors.js | 2 +- packages/vats/src/lib-chainStorage.js | 74 +++++++++++++++++++++++ packages/vats/src/vat-chainStorage.js | 68 ++------------------- 3 files changed, 81 insertions(+), 63 deletions(-) create mode 100644 packages/vats/src/lib-chainStorage.js diff --git a/packages/vats/src/core/chain-behaviors.js b/packages/vats/src/core/chain-behaviors.js index 1418969a34a..fd19ca59f62 100644 --- a/packages/vats/src/core/chain-behaviors.js +++ b/packages/vats/src/core/chain-behaviors.js @@ -302,7 +302,7 @@ export const makeChainStorage = async ({ const ROOT_KEY = 'published'; const vat = E(loadVat)('chainStorage'); - const rootNodeP = E(vat).createChainStorageRoot( + const rootNodeP = E(vat).makeBridgedChainStorageRoot( bridgeManager, BRIDGE_ID.STORAGE, ROOT_KEY, diff --git a/packages/vats/src/lib-chainStorage.js b/packages/vats/src/lib-chainStorage.js new file mode 100644 index 00000000000..d1ad2ee29a5 --- /dev/null +++ b/packages/vats/src/lib-chainStorage.js @@ -0,0 +1,74 @@ +// @ts-check + +import { Far } from '@endo/far'; + +const { details: X } = assert; + +// TODO: Formalize segment constraints. +// Must be nonempty and disallow (unescaped) `.`, and for simplicity +// (and future possibility of e.g. escaping) we currently limit to +// ASCII alphanumeric plus underscore. +const pathSegmentPattern = /^[a-zA-Z0-9_]{1,100}$/; + +/** + * Create a root storage node for a given backing function and root key. + * + * @param {(message: any) => any} toStorage a function for sending a message to the storage implementation + * @param {string} rootKey + */ +export function makeChainStorageRoot(toStorage, rootKey) { + assert.typeof(rootKey, 'string'); + + function makeChainStorageNode(key) { + const node = { + getKey() { + return key; + }, + getChildNode(name) { + assert.typeof(name, 'string'); + assert( + pathSegmentPattern.test(name), + X`Path segment must be a short ASCII identifier: ${name}`, + ); + return makeChainStorageNode(`${key}.${name}`); + }, + setValue(value) { + assert.typeof(value, 'string'); + // TODO: Fix on the chain side. + // https://github.com/Agoric/agoric-sdk/issues/5381 + assert(value !== ''); + toStorage({ key, method: 'set', value }); + }, + async delete() { + assert(key !== rootKey); + // A 'set' with no value deletes a key but leaves any descendants. + // We therefore want to disallow that (at least for now). + // This check is unfortunately racy (e.g., a vat could wake up + // and set data for a child before _this_ vat receives an + // already-enqueued no-children response), but we can tolerate + // that once https://github.com/Agoric/agoric-sdk/issues/5381 + // is fixed because then the 'set' message sent after erroneously + // passing the no-children check will be transformed into a + // set-to-empty which is at least *visibly* not deleted and + // indistinguishable from a valid deep-create scenario (in which + // parent keys are created automatically). + const childKeys = await toStorage({ key, method: 'keys' }); + if (childKeys.length > 0) { + assert.fail(X`Refusing to delete node with children: ${key}`); + } + toStorage({ key, method: 'set' }); + }, + // Possible extensions: + // * getValue() + // * getChildNames() and/or getChildNodes() + // * getName() + // * recursive delete + // * batch operations + // * local buffering (with end-of-block commit) + }; + return Far('chainStorageNode', node); + } + + const rootNode = makeChainStorageNode(rootKey); + return rootNode; +} diff --git a/packages/vats/src/vat-chainStorage.js b/packages/vats/src/vat-chainStorage.js index 2fbfea74a4c..0786aa3c7c2 100644 --- a/packages/vats/src/vat-chainStorage.js +++ b/packages/vats/src/vat-chainStorage.js @@ -1,71 +1,15 @@ import { E, Far } from '@endo/far'; +import { makeChainStorageRoot } from './lib-chainStorage.js'; -const { details: X } = assert; - -// TODO: Extract into testable library? export function buildRootObject(_vatPowers) { - function createChainStorageRoot(bridgeManager, bridgeId, rootKey) { + function makeBridgedChainStorageRoot(bridgeManager, bridgeId, rootKey) { + // XXX: Should we validate uniqueness of rootKey, or is that an external concern? const toStorage = message => E(bridgeManager).toBridge(bridgeId, message); - - // TODO: Formalize segment constraints. - // Must be nonempty and disallow (unescaped) `.`, and for simplicity - // (and future possibility of e.g. escaping) we currently limit to - // ASCII alphanumeric plus underscore. - const pathSegmentPattern = /^[a-zA-Z0-9_]{1,100}$/; - const makeChainStorageNode = key => { - const node = { - getKey() { - return key; - }, - getChildNode(name) { - assert.typeof(name, 'string'); - assert( - pathSegmentPattern.test(name), - X`Path segment must be a short ASCII identifier: ${name}`, - ); - return makeChainStorageNode(`${key}.${name}`); - }, - setValue(value) { - assert.typeof(value, 'string'); - // TODO: Fix on the chain side. - // https://github.com/Agoric/agoric-sdk/issues/5381 - assert(value !== ''); - toStorage({ key, method: 'set', value }); - }, - async delete() { - assert(key !== rootKey); - // A 'set' with no value deletes a key but leaves any descendants. - // We therefore want to disallow that (at least for now). - // This check is unfortunately racy (e.g., a vat could wake up - // and set data for a child before _this_ vat receives an - // already-enqueued no-children response), but we can tolerate - // that once https://github.com/Agoric/agoric-sdk/issues/5381 - // is fixed because then the 'set' message sent after erroneously - // passing the no-children check will be transformed into a - // set-to-empty which is at least *visibly* not deleted and - // indistinguishable from a valid deep-create scenario (in which - // parent keys are created automatically). - const childKeys = await toStorage({ key, method: 'keys' }); - if (childKeys.length > 0) { - assert.fail(X`Refusing to delete node with children: ${key}`); - } - toStorage({ key, method: 'set' }); - }, - // Possible extensions: - // * getValue() - // * getChildNames() and/or getChildNodes() - // * getName() - // * recursive delete - // * batch operations - // * local buffering (with end-of-block commit) - }; - return Far('chainStorageNode', node); - }; - - const rootNode = makeChainStorageNode(rootKey); + const rootNode = makeChainStorageRoot(toStorage, rootKey); return rootNode; } + return Far('root', { - createChainStorageRoot, + makeBridgedChainStorageRoot, }); } From 6ead428a5d0987a4c44a5c18fa7cc63f68b342aa Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Mon, 23 May 2022 11:40:52 -0400 Subject: [PATCH 05/18] chore: Improve child key checks and explanations --- packages/vats/src/lib-chainStorage.js | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/packages/vats/src/lib-chainStorage.js b/packages/vats/src/lib-chainStorage.js index d1ad2ee29a5..48461adccc4 100644 --- a/packages/vats/src/lib-chainStorage.js +++ b/packages/vats/src/lib-chainStorage.js @@ -13,7 +13,7 @@ const pathSegmentPattern = /^[a-zA-Z0-9_]{1,100}$/; /** * Create a root storage node for a given backing function and root key. * - * @param {(message: any) => any} toStorage a function for sending a message to the storage implementation + * @param {(message: any) => any} toStorage a function for sending a storageMessage object to the storage implementation (cf. golang/cosmos/x/swingset/storage.go) * @param {string} rootKey */ export function makeChainStorageRoot(toStorage, rootKey) { @@ -34,26 +34,22 @@ export function makeChainStorageRoot(toStorage, rootKey) { }, setValue(value) { assert.typeof(value, 'string'); - // TODO: Fix on the chain side. - // https://github.com/Agoric/agoric-sdk/issues/5381 - assert(value !== ''); toStorage({ key, method: 'set', value }); }, async delete() { assert(key !== rootKey); - // A 'set' with no value deletes a key but leaves any descendants. - // We therefore want to disallow that (at least for now). + // A 'set' with no value deletes a key if it has no children, but + // otherwise sets data to the empty string and leaves all nodes intact. + // We want to reject silently incomplete deletes (at least for now). // This check is unfortunately racy (e.g., a vat could wake up // and set data for a child before _this_ vat receives an - // already-enqueued no-children response), but we can tolerate - // that once https://github.com/Agoric/agoric-sdk/issues/5381 - // is fixed because then the 'set' message sent after erroneously - // passing the no-children check will be transformed into a - // set-to-empty which is at least *visibly* not deleted and - // indistinguishable from a valid deep-create scenario (in which - // parent keys are created automatically). - const childKeys = await toStorage({ key, method: 'keys' }); - if (childKeys.length > 0) { + // already-enqueued response claiming no children), but we can tolerate + // that because transforming a deletion into a set-to-empty is + // effectively indistinguishable from a valid reordering where a fully + // successful 'delete' is followed by a child-key 'set' (for which + // absent parent keys are automatically created with empty-string data). + const childCount = await toStorage({ key, method: 'size' }); + if (childCount > 0) { assert.fail(X`Refusing to delete node with children: ${key}`); } toStorage({ key, method: 'set' }); From 8cfd6f52a3ffe1abb9aa0237a4fec815cea79dac Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Mon, 23 May 2022 16:26:19 -0400 Subject: [PATCH 06/18] test: Add tests for chainStorage --- packages/vats/test/test-lib-chainStorage.js | 180 ++++++++++++++++++++ 1 file changed, 180 insertions(+) create mode 100644 packages/vats/test/test-lib-chainStorage.js diff --git a/packages/vats/test/test-lib-chainStorage.js b/packages/vats/test/test-lib-chainStorage.js new file mode 100644 index 00000000000..a8955ac47b1 --- /dev/null +++ b/packages/vats/test/test-lib-chainStorage.js @@ -0,0 +1,180 @@ +// @ts-check +import { test } from '@agoric/swingset-vat/tools/prepare-test-env-ava.js'; + +import { makeChainStorageRoot } from '../src/lib-chainStorage.js'; + +test('makeChainStorageRoot', async t => { + // Instantiate chain storage over a simple in-memory implementation. + const data = new Map(); + const messages = []; + // eslint-disable-next-line consistent-return + const toStorage = message => { + messages.push(message); + switch (message.method) { + case 'set': + if ('value' in message) { + data.set(message.key, message.value); + } else { + data.delete(message.key); + } + break; + case 'size': + // Intentionally incorrect because it counts non-child descendants, + // but nevertheless supports a "has children" test. + return [...data.keys()].filter(k => k.startsWith(`${message.key}.`)) + .length; + default: + throw new Error(`unsupported method: ${message.method}`); + } + }; + const rootKey = 'root'; + const rootNode = makeChainStorageRoot(toStorage, rootKey); + t.is(rootNode.getKey(), rootKey, 'root key matches initialization input'); + + // Values must be strings. + const nonStrings = new Map([ + ['number', 1], + ['bigint', 1n], + ['boolean', true], + ['null', null], + ['undefined', undefined], + ['symbol', Symbol('foo')], + ['array', ['foo']], + [ + 'object', + { + toString() { + return 'foo'; + }, + }, + ], + ]); + for (const [label, val] of nonStrings) { + t.throws( + () => rootNode.setValue(val), + undefined, + `${label} value for root node is rejected`, + ); + } + + // The root node cannot be deleted, but is otherwise normal. + await t.throwsAsync( + rootNode.delete(), + undefined, + 'root node deletion is disallowed', + ); + rootNode.setValue('foo'); + t.deepEqual( + messages.slice(-1), + [{ key: rootKey, method: 'set', value: 'foo' }], + 'root node setValue message', + ); + rootNode.setValue('bar'); + t.deepEqual( + messages.slice(-1), + [{ key: rootKey, method: 'set', value: 'bar' }], + 'second setValue message', + ); + + // Valid key segments are strings of up to 100 ASCII alphanumeric/underscore characters. + const validSegmentChars = `${ + String.fromCharCode( + ...Array(26) + .fill() + .map((_, i) => 'a'.charCodeAt(0) + i), + ) + + String.fromCharCode( + ...Array(26) + .fill() + .map((_, i) => 'A'.charCodeAt(0) + i), + ) + + String.fromCharCode( + ...Array(10) + .fill() + .map((_, i) => '0'.charCodeAt(0) + i), + ) + }_`; + const extremeSegments = validSegmentChars + .repeat(Math.ceil(100 / validSegmentChars.length)) + .match(/.{1,100}/gsu); + for (const segment of extremeSegments) { + const child = rootNode.getChildNode(segment); + const childKey = `${rootKey}.${segment}`; + t.is(child.getKey(), childKey, 'key segments are dot-separated'); + child.setValue('foo'); + t.deepEqual( + messages.slice(-1), + [{ key: childKey, method: 'set', value: 'foo' }], + 'non-root setValue message', + ); + // eslint-disable-next-line no-await-in-loop + await child.delete(); + t.deepEqual( + messages.slice(-1), + [{ key: childKey, method: 'set' }], + 'non-root delete message', + ); + } + + // Invalid key segments are non-strings, empty, too long, or contain unacceptable characters. + const badSegments = new Map(nonStrings); + badSegments.set('empty', ''); + badSegments.set('long', 'x'.repeat(101)); + for (let i = 0; i < 128; i += 1) { + const segment = String.fromCharCode(i); + if (!validSegmentChars.includes(segment)) { + badSegments.set( + `U+${i.toString(16).padStart(4, '0')} ${JSON.stringify(segment)}`, + segment, + ); + } + } + badSegments.set('non-ASCII', '\u00E1'); + badSegments.set('ASCII with combining diacritical mark', 'a\u0301'); + for (const [label, val] of badSegments) { + t.throws( + () => rootNode.getChildNode(val), + undefined, + `${label} segment is rejected`, + ); + } + + // Level-skipping creation is allowed. + const childNode = rootNode.getChildNode('child'); + const childKey = `${rootKey}.child`; + const deepNode = childNode.getChildNode('grandchild'); + const deepKey = `${childKey}.grandchild`; + t.is(deepNode.getKey(), deepKey); + for (const [label, val] of nonStrings) { + t.throws( + () => deepNode.setValue(val), + undefined, + `${label} value for non-root node is rejected`, + ); + } + deepNode.setValue('foo'); + t.deepEqual( + messages.slice(-1), + [{ key: deepKey, method: 'set', value: 'foo' }], + 'level-skipping setValue message', + ); + + // Deletion requires absence of children. + await t.throwsAsync( + childNode.delete(), + undefined, + 'deleting a node with a child is disallowed', + ); + await deepNode.delete(); + t.deepEqual( + messages.slice(-1), + [{ key: deepKey, method: 'set' }], + 'granchild delete message', + ); + await childNode.delete(); + t.deepEqual( + messages.slice(-1), + [{ key: childKey, method: 'set' }], + 'child delete message', + ); +}); From 0c195c83e801ecaf3b2f51d8506f43ca4e2d0c54 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Mon, 23 May 2022 17:27:58 -0400 Subject: [PATCH 07/18] chore: Allow "-" in chain storage key segments --- packages/vats/src/lib-chainStorage.js | 2 +- packages/vats/test/test-lib-chainStorage.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/vats/src/lib-chainStorage.js b/packages/vats/src/lib-chainStorage.js index 48461adccc4..8e1d90a8b10 100644 --- a/packages/vats/src/lib-chainStorage.js +++ b/packages/vats/src/lib-chainStorage.js @@ -8,7 +8,7 @@ const { details: X } = assert; // Must be nonempty and disallow (unescaped) `.`, and for simplicity // (and future possibility of e.g. escaping) we currently limit to // ASCII alphanumeric plus underscore. -const pathSegmentPattern = /^[a-zA-Z0-9_]{1,100}$/; +const pathSegmentPattern = /^[a-zA-Z0-9_\-]{1,100}$/; /** * Create a root storage node for a given backing function and root key. diff --git a/packages/vats/test/test-lib-chainStorage.js b/packages/vats/test/test-lib-chainStorage.js index a8955ac47b1..0b6e9d50b16 100644 --- a/packages/vats/test/test-lib-chainStorage.js +++ b/packages/vats/test/test-lib-chainStorage.js @@ -76,7 +76,7 @@ test('makeChainStorageRoot', async t => { 'second setValue message', ); - // Valid key segments are strings of up to 100 ASCII alphanumeric/underscore characters. + // Valid key segments are strings of up to 100 ASCII alphanumeric/dash/underscore characters. const validSegmentChars = `${ String.fromCharCode( ...Array(26) @@ -93,7 +93,7 @@ test('makeChainStorageRoot', async t => { .fill() .map((_, i) => '0'.charCodeAt(0) + i), ) - }_`; + }-_`; const extremeSegments = validSegmentChars .repeat(Math.ceil(100 / validSegmentChars.length)) .match(/.{1,100}/gsu); From a90fc3d56fa3f023afdf92544cdb823eef06fbd3 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Thu, 26 May 2022 02:57:42 -0400 Subject: [PATCH 08/18] chore: Try to appease TypeScript --- packages/vats/src/core/chain-behaviors.js | 4 +- packages/vats/src/lib-chainStorage.js | 2 +- packages/vats/test/test-lib-chainStorage.js | 63 ++++++++++----------- 3 files changed, 34 insertions(+), 35 deletions(-) diff --git a/packages/vats/src/core/chain-behaviors.js b/packages/vats/src/core/chain-behaviors.js index fd19ca59f62..e2270f4f6f3 100644 --- a/packages/vats/src/core/chain-behaviors.js +++ b/packages/vats/src/core/chain-behaviors.js @@ -283,7 +283,9 @@ export const makeBridgeManager = async ({ harden(makeBridgeManager); /** - * @param { BootstrapPowers } powers + * @param {BootstrapPowers & { + * consume: { loadVat: ERef> } + * }} powers */ export const makeChainStorage = async ({ consume: { bridgeManager: bridgeManagerP, loadVat }, diff --git a/packages/vats/src/lib-chainStorage.js b/packages/vats/src/lib-chainStorage.js index 8e1d90a8b10..5f7f16f73c3 100644 --- a/packages/vats/src/lib-chainStorage.js +++ b/packages/vats/src/lib-chainStorage.js @@ -8,7 +8,7 @@ const { details: X } = assert; // Must be nonempty and disallow (unescaped) `.`, and for simplicity // (and future possibility of e.g. escaping) we currently limit to // ASCII alphanumeric plus underscore. -const pathSegmentPattern = /^[a-zA-Z0-9_\-]{1,100}$/; +const pathSegmentPattern = /^[a-zA-Z0-9_-]{1,100}$/; /** * Create a root storage node for a given backing function and root key. diff --git a/packages/vats/test/test-lib-chainStorage.js b/packages/vats/test/test-lib-chainStorage.js index 0b6e9d50b16..d49a92c40bb 100644 --- a/packages/vats/test/test-lib-chainStorage.js +++ b/packages/vats/test/test-lib-chainStorage.js @@ -32,23 +32,20 @@ test('makeChainStorageRoot', async t => { t.is(rootNode.getKey(), rootKey, 'root key matches initialization input'); // Values must be strings. - const nonStrings = new Map([ - ['number', 1], - ['bigint', 1n], - ['boolean', true], - ['null', null], - ['undefined', undefined], - ['symbol', Symbol('foo')], - ['array', ['foo']], - [ - 'object', - { - toString() { - return 'foo'; - }, - }, - ], - ]); + /** @type { Map } */ + const nonStrings = new Map(); + nonStrings.set('number', 1); + nonStrings.set('bigint', 1n); + nonStrings.set('boolean', true); + nonStrings.set('null', null); + nonStrings.set('undefined', undefined); + nonStrings.set('symbol', Symbol('foo')); + nonStrings.set('array', ['foo']); + nonStrings.set('object', { + toString() { + return 'foo'; + }, + }); for (const [label, val] of nonStrings) { t.throws( () => rootNode.setValue(val), @@ -78,25 +75,25 @@ test('makeChainStorageRoot', async t => { // Valid key segments are strings of up to 100 ASCII alphanumeric/dash/underscore characters. const validSegmentChars = `${ - String.fromCharCode( - ...Array(26) - .fill() - .map((_, i) => 'a'.charCodeAt(0) + i), - ) + - String.fromCharCode( - ...Array(26) - .fill() - .map((_, i) => 'A'.charCodeAt(0) + i), - ) + - String.fromCharCode( - ...Array(10) - .fill() - .map((_, i) => '0'.charCodeAt(0) + i), - ) + Array(26) + .fill() + .map((_, i) => 'a'.charCodeAt(0) + i) + .map(code => String.fromCharCode(code)) + .join('') + + Array(26) + .fill() + .map((_, i) => 'A'.charCodeAt(0) + i) + .map(code => String.fromCharCode(code)) + .join('') + + Array(10) + .fill() + .map((_, i) => '0'.charCodeAt(0) + i) + .map(code => String.fromCharCode(code)) + .join('') }-_`; const extremeSegments = validSegmentChars .repeat(Math.ceil(100 / validSegmentChars.length)) - .match(/.{1,100}/gsu); + .match(/.{1,100}/gsu) || []; for (const segment of extremeSegments) { const child = rootNode.getChildNode(segment); const childKey = `${rootKey}.${segment}`; From 1b8b11c02781b22018ca2bda170f60b12b04d45d Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Thu, 26 May 2022 03:16:25 -0400 Subject: [PATCH 09/18] chore: Try harder to appease TypeScript --- packages/vats/src/core/chain-behaviors.js | 2 +- packages/vats/src/core/types.js | 1 + packages/vats/test/test-lib-chainStorage.js | 13 +++++++------ 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/vats/src/core/chain-behaviors.js b/packages/vats/src/core/chain-behaviors.js index e2270f4f6f3..306e2a7f84f 100644 --- a/packages/vats/src/core/chain-behaviors.js +++ b/packages/vats/src/core/chain-behaviors.js @@ -284,7 +284,7 @@ harden(makeBridgeManager); /** * @param {BootstrapPowers & { - * consume: { loadVat: ERef> } + * consume: { loadVat: ERef> } * }} powers */ export const makeChainStorage = async ({ diff --git a/packages/vats/src/core/types.js b/packages/vats/src/core/types.js index d9dad0c59d9..4e7d13d6386 100644 --- a/packages/vats/src/core/types.js +++ b/packages/vats/src/core/types.js @@ -236,6 +236,7 @@ * @typedef {{ mint: ERef, issuer: ERef, brand: Brand }} RemoteIssuerKit * @typedef {ReturnType['makeBankManager']>} BankManager * @typedef {ERef>} BankVat + * @typedef {ERef>} ChainStorageVat * @typedef {ERef>} ProvisioningVat * @typedef {ERef>} MintsVat * @typedef {ERef>} PriceAuthorityVat diff --git a/packages/vats/test/test-lib-chainStorage.js b/packages/vats/test/test-lib-chainStorage.js index d49a92c40bb..036b78ab4ad 100644 --- a/packages/vats/test/test-lib-chainStorage.js +++ b/packages/vats/test/test-lib-chainStorage.js @@ -76,24 +76,25 @@ test('makeChainStorageRoot', async t => { // Valid key segments are strings of up to 100 ASCII alphanumeric/dash/underscore characters. const validSegmentChars = `${ Array(26) - .fill() + .fill(undefined) .map((_, i) => 'a'.charCodeAt(0) + i) .map(code => String.fromCharCode(code)) .join('') + Array(26) - .fill() + .fill(undefined) .map((_, i) => 'A'.charCodeAt(0) + i) .map(code => String.fromCharCode(code)) .join('') + Array(10) - .fill() + .fill(undefined) .map((_, i) => '0'.charCodeAt(0) + i) .map(code => String.fromCharCode(code)) .join('') }-_`; - const extremeSegments = validSegmentChars - .repeat(Math.ceil(100 / validSegmentChars.length)) - .match(/.{1,100}/gsu) || []; + const extremeSegments = + validSegmentChars + .repeat(Math.ceil(100 / validSegmentChars.length)) + .match(/.{1,100}/gsu) || []; for (const segment of extremeSegments) { const child = rootNode.getChildNode(segment); const childKey = `${rootKey}.${segment}`; From e40a62ad5e889cf1b577673e352bd93e328810b9 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Wed, 25 May 2022 15:56:51 -0400 Subject: [PATCH 10/18] feat: Add a helper for publishing iteration results to chain storage Fixes #5353 --- packages/vats/src/lib-chainPublish.js | 46 +++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 packages/vats/src/lib-chainPublish.js diff --git a/packages/vats/src/lib-chainPublish.js b/packages/vats/src/lib-chainPublish.js new file mode 100644 index 00000000000..d16bb20a14f --- /dev/null +++ b/packages/vats/src/lib-chainPublish.js @@ -0,0 +1,46 @@ +// @ts-check + +import { E } from '@endo/far'; + +/** + * Publish results of an async iterable into a chain storage node as an array + * serialized using JSON.stringify or an optionally provided function (e.g., + * leveraging a serialize function from makeMarshal). + * + * @param {AsyncIterator} iterable + * @param {ReturnType} chainStorageNode + * @param {{ timerService: ERef, serialize?: (obj: any) => string }} powers + */ +export async function publishToChainNode( + iterable, + chainStorageNode, + { timerService, serialize = JSON.stringify }, +) { + let nextIndex = 0n; + let isNewBlock = true; + let oldResults = []; + let results = []; + for await (const result of iterable) { + if (isNewBlock) { + isNewBlock = false; + oldResults = results; + results = []; + E(timerService) + .delay(1n) + .then(() => { + isNewBlock = true; + }); + } + // To avoid loss when detecting the new block *after* already consuming + // results produced within it, we associate each result with an index and + // include "old" results in the batch. + // Downstream consumers are expected to deduplicate by index. + // We represent the index as a string to maintain compatibility with + // JSON.stringify and to avoid overly inflating the data size (e.g. with + // "@qclass" objects from makeMarshal serialize functions). + results.push([String(nextIndex), result]); + nextIndex += 1n; + const batch = harden(oldResults.slice().concat(results)); + E(chainStorageNode).setValue(serialize(batch)); + } +} From f5df821b7e920da0f1795fa5dac320fa3593454e Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Wed, 25 May 2022 18:24:29 -0400 Subject: [PATCH 11/18] test: Add basic tests for publishToChainNode --- packages/vats/test/test-lib-chainPublish.js | 166 ++++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 packages/vats/test/test-lib-chainPublish.js diff --git a/packages/vats/test/test-lib-chainPublish.js b/packages/vats/test/test-lib-chainPublish.js new file mode 100644 index 00000000000..3821e2a891c --- /dev/null +++ b/packages/vats/test/test-lib-chainPublish.js @@ -0,0 +1,166 @@ +// @ts-check +import { test } from '@agoric/swingset-vat/tools/prepare-test-env-ava.js'; + +import { E } from '@endo/far'; +import { publishToChainNode } from '../src/lib-chainPublish.js'; + +test('publishToChainNode', async t => { + // eslint-disable-next-line no-use-before-define + const inputs = getMockInputs(t); + const { + // external facets + storageNode, + timerService, + customAsyncIterable, + // internal facets + nodeValueHistory, + advanceTimer, + supplyIterationResult, + } = inputs; + publishToChainNode(customAsyncIterable, storageNode, { timerService }); + + supplyIterationResult('foo'); + await E(Promise).resolve(); + t.is(nodeValueHistory.length, 1, 'first result is communicated promptly'); + t.deepEqual( + nodeValueHistory.pop(), + [['0', 'foo']], + 'first result has index 0', + ); + + supplyIterationResult('bar'); + await E(Promise).resolve(); + t.is(nodeValueHistory.length, 1, 'second result is communicated promptly'); + t.deepEqual( + nodeValueHistory.pop(), + [ + ['0', 'foo'], + ['1', 'bar'], + ], + 'second result is appended', + ); + + advanceTimer(); + supplyIterationResult('baz'); + await E(Promise).resolve(); + t.is( + nodeValueHistory.length, + 1, + 'result following timer advance is communicated promptly', + ); + t.deepEqual( + nodeValueHistory.pop(), + [ + ['0', 'foo'], + ['1', 'bar'], + ['2', 'baz'], + ], + 'last-batch results are preserved after timer advance', + ); + + advanceTimer(); + advanceTimer(); + advanceTimer(); + supplyIterationResult('qux'); + await E(Promise).resolve(); + t.is( + nodeValueHistory.length, + 1, + 'results following multiple timer advances are communicated promptly', + ); + t.deepEqual( + nodeValueHistory.pop(), + [ + ['2', 'baz'], + ['3', 'qux'], + ], + 'batches older than the previous are dropped when a new result is supplied', + ); + + supplyIterationResult('quux'); + await E(Promise).resolve(); + t.is(nodeValueHistory.length, 1); + t.deepEqual( + nodeValueHistory.pop(), + [ + ['2', 'baz'], + ['3', 'qux'], + ['4', 'quux'], + ], + 'batches older than the previous are dropped when a second new result is supplied', + ); + + advanceTimer(); + advanceTimer(); + advanceTimer(); + supplyIterationResult('corge'); + await E(Promise).resolve(); + t.is(nodeValueHistory.length, 1); + t.deepEqual( + nodeValueHistory.pop(), + [ + ['3', 'qux'], + ['4', 'quux'], + ['5', 'corge'], + ], + 'the full previous batch is preserved after multiple timer advances', + ); +}); + +// TODO: Move to a testing library. +function getMockInputs(t) { + // Mock a chainStorage node. + const nodeValueHistory = []; + const storageNode = { + setValue(val) { + nodeValueHistory.push(JSON.parse(val)); + }, + }; + + // Mock a timerService. + let advanceTimerP; + let advanceTimerR = () => {}; + const advanceTimer = () => { + advanceTimerR(); + advanceTimerP = new Promise(resolve => { + advanceTimerR = resolve; + }); + }; + const timerService = { + delay(n) { + t.is(n, 1n); + return advanceTimerP; + }, + }; + + // Create an async iterable with a function for supplying results. + let nextResultP; + let nextResultR = _result => {}; + const supplyIterationResult = result => { + nextResultR(result); + nextResultP = new Promise(resolve => { + nextResultR = resolve; + }); + }; + const customAsyncIterable = (async function* makeCustomAsyncIterable() { + while (true) { + // eslint-disable-next-line no-await-in-loop + yield await nextResultP; + } + })(); + + // Initialize. + advanceTimer(); + supplyIterationResult(); + + return { + // external facets + storageNode, + timerService, + customAsyncIterable, + // internal facets + nodeValueHistory, + advanceTimer, + supplyIterationResult, + }; +} From 392f67778dd282032510ff071575cdcdd470556b Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Thu, 26 May 2022 00:03:01 -0400 Subject: [PATCH 12/18] test: Add publishToChainNode finish/fail tests --- packages/vats/test/test-lib-chainPublish.js | 155 ++++++++++++++++++-- 1 file changed, 142 insertions(+), 13 deletions(-) diff --git a/packages/vats/test/test-lib-chainPublish.js b/packages/vats/test/test-lib-chainPublish.js index 3821e2a891c..4d51b864824 100644 --- a/packages/vats/test/test-lib-chainPublish.js +++ b/packages/vats/test/test-lib-chainPublish.js @@ -18,9 +18,10 @@ test('publishToChainNode', async t => { supplyIterationResult, } = inputs; publishToChainNode(customAsyncIterable, storageNode, { timerService }); + await E(Promise).resolve(); supplyIterationResult('foo'); - await E(Promise).resolve(); + await E(Promise).resolve().then(); t.is(nodeValueHistory.length, 1, 'first result is communicated promptly'); t.deepEqual( nodeValueHistory.pop(), @@ -29,7 +30,7 @@ test('publishToChainNode', async t => { ); supplyIterationResult('bar'); - await E(Promise).resolve(); + await E(Promise).resolve().then(); t.is(nodeValueHistory.length, 1, 'second result is communicated promptly'); t.deepEqual( nodeValueHistory.pop(), @@ -42,7 +43,7 @@ test('publishToChainNode', async t => { advanceTimer(); supplyIterationResult('baz'); - await E(Promise).resolve(); + await E(Promise).resolve().then(); t.is( nodeValueHistory.length, 1, @@ -62,7 +63,7 @@ test('publishToChainNode', async t => { advanceTimer(); advanceTimer(); supplyIterationResult('qux'); - await E(Promise).resolve(); + await E(Promise).resolve().then(); t.is( nodeValueHistory.length, 1, @@ -78,7 +79,7 @@ test('publishToChainNode', async t => { ); supplyIterationResult('quux'); - await E(Promise).resolve(); + await E(Promise).resolve().then(); t.is(nodeValueHistory.length, 1); t.deepEqual( nodeValueHistory.pop(), @@ -94,7 +95,7 @@ test('publishToChainNode', async t => { advanceTimer(); advanceTimer(); supplyIterationResult('corge'); - await E(Promise).resolve(); + await E(Promise).resolve().then(); t.is(nodeValueHistory.length, 1); t.deepEqual( nodeValueHistory.pop(), @@ -107,6 +108,120 @@ test('publishToChainNode', async t => { ); }); +test('publishToChainNode captures finish value', async t => { + // eslint-disable-next-line no-use-before-define + const inputs = getMockInputs(t); + const { + // external facets + storageNode, + timerService, + customAsyncIterable, + // internal facets + nodeValueHistory, + supplyIterationResult, + } = inputs; + publishToChainNode(customAsyncIterable, storageNode, { timerService }); + await E(Promise).resolve(); + + supplyIterationResult.finish('foo'); + await E(Promise).resolve().then(); + t.is(nodeValueHistory.length, 1, 'finish is communicated promptly'); + t.deepEqual( + nodeValueHistory.pop(), + [['finish', 'foo']], + 'result has index "finish"', + ); +}); + +test('publishToChainNode captures fail value', async t => { + // eslint-disable-next-line no-use-before-define + const inputs = getMockInputs(t); + const { + // external facets + storageNode, + timerService, + customAsyncIterable, + // internal facets + nodeValueHistory, + supplyIterationResult, + } = inputs; + publishToChainNode(customAsyncIterable, storageNode, { timerService }); + await E(Promise).resolve(); + + supplyIterationResult.fail('foo'); + await E(Promise).resolve().then(); + t.is(nodeValueHistory.length, 1, 'fail is communicated promptly'); + t.deepEqual( + nodeValueHistory.pop(), + [['fail', 'foo']], + 'result has index "fail"', + ); +}); + +test('publishToChainNode custom serialization', async t => { + // eslint-disable-next-line no-use-before-define + const inputs = getMockInputs(t); + const { + // external facets + storageNode, + timerService, + customAsyncIterable, + // internal facets + nodeValueHistory, + supplyIterationResult, + } = inputs; + const serializeInputHistory = []; + const serialize = val => { + const length = serializeInputHistory.push(val); + return String(length); + }; + publishToChainNode(customAsyncIterable, storageNode, { + timerService, + serialize, + }); + await E(Promise).resolve(); + + supplyIterationResult('foo'); + await E(Promise).resolve().then(); + t.is(serializeInputHistory.length, 1, 'first result is serialized promptly'); + t.deepEqual( + serializeInputHistory[0], + [['0', 'foo']], + 'first result has index 0', + ); + t.is(nodeValueHistory.length, 1, 'first result is communicated promptly'); + t.is(nodeValueHistory[0], 1, 'first result is output from serialize()'); + + supplyIterationResult('bar'); + await E(Promise).resolve().then(); + t.is(serializeInputHistory.length, 2, 'second result is serialized promptly'); + t.deepEqual( + serializeInputHistory[1], + [ + ['0', 'foo'], + ['1', 'bar'], + ], + 'second result is appended', + ); + t.is(nodeValueHistory.length, 2, 'second result is communicated promptly'); + t.is(nodeValueHistory[1], 2, 'second result is output from serialize()'); + + supplyIterationResult.finish('baz'); + await E(Promise).resolve().then(); + t.is(serializeInputHistory.length, 3, 'finish result is serialized promptly'); + t.deepEqual( + serializeInputHistory[2], + [ + ['0', 'foo'], + ['1', 'bar'], + ['finish', 'baz'], + ], + 'finish result is appended', + ); + t.is(nodeValueHistory.length, 3, 'finish result is communicated promptly'); + t.is(nodeValueHistory[2], 3, 'finish result is output from serialize()'); +}); + // TODO: Move to a testing library. function getMockInputs(t) { // Mock a chainStorage node. @@ -134,24 +249,38 @@ function getMockInputs(t) { }; // Create an async iterable with a function for supplying results. - let nextResultP; - let nextResultR = _result => {}; - const supplyIterationResult = result => { - nextResultR(result); - nextResultP = new Promise(resolve => { - nextResultR = resolve; + const pendingResults = [{ resolve() {} }]; + const supplyIterationResult = value => { + const nextDeferred = {}; + nextDeferred.promise = new Promise((resolve, reject) => { + nextDeferred.resolve = resolve; + nextDeferred.reject = reject; }); + const newLength = pendingResults.push(nextDeferred); + pendingResults[newLength - 2].resolve({ value }); + }; + supplyIterationResult.finish = value => { + pendingResults[pendingResults.length - 1].resolve({ value, done: true }); + }; + supplyIterationResult.fail = value => { + pendingResults[pendingResults.length - 1].reject(value); }; const customAsyncIterable = (async function* makeCustomAsyncIterable() { while (true) { // eslint-disable-next-line no-await-in-loop - yield await nextResultP; + const iterationResult = await pendingResults[0].promise; + pendingResults.shift(); + if (iterationResult.done) { + return iterationResult.value; + } + yield iterationResult.value; } })(); // Initialize. advanceTimer(); supplyIterationResult(); + pendingResults.shift(); return { // external facets From f95afa558698be8eeb1cf4de89baf08d13c6674e Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Thu, 26 May 2022 00:03:55 -0400 Subject: [PATCH 13/18] feat: Add publishToChainNode finish/fail support --- packages/vats/src/lib-chainPublish.js | 57 ++++++++++++++++----------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/packages/vats/src/lib-chainPublish.js b/packages/vats/src/lib-chainPublish.js index d16bb20a14f..b31ce607624 100644 --- a/packages/vats/src/lib-chainPublish.js +++ b/packages/vats/src/lib-chainPublish.js @@ -1,11 +1,14 @@ // @ts-check +import { observeIteration } from '@agoric/notifier'; import { E } from '@endo/far'; /** * Publish results of an async iterable into a chain storage node as an array * serialized using JSON.stringify or an optionally provided function (e.g., * leveraging a serialize function from makeMarshal). + * Array items are possibly-duplicated [index, value] pairs, where index is a + * string (ascending numeric or terminal "finish" or "fail"). * * @param {AsyncIterator} iterable * @param {ReturnType} chainStorageNode @@ -20,27 +23,35 @@ export async function publishToChainNode( let isNewBlock = true; let oldResults = []; let results = []; - for await (const result of iterable) { - if (isNewBlock) { - isNewBlock = false; - oldResults = results; - results = []; - E(timerService) - .delay(1n) - .then(() => { - isNewBlock = true; - }); - } - // To avoid loss when detecting the new block *after* already consuming - // results produced within it, we associate each result with an index and - // include "old" results in the batch. - // Downstream consumers are expected to deduplicate by index. - // We represent the index as a string to maintain compatibility with - // JSON.stringify and to avoid overly inflating the data size (e.g. with - // "@qclass" objects from makeMarshal serialize functions). - results.push([String(nextIndex), result]); - nextIndex += 1n; - const batch = harden(oldResults.slice().concat(results)); - E(chainStorageNode).setValue(serialize(batch)); - } + const makeAcceptor = forceIndex => { + return value => { + if (isNewBlock) { + isNewBlock = false; + oldResults = results; + results = []; + E(timerService) + .delay(1n) + .then(() => { + isNewBlock = true; + }); + } + // To avoid loss when detecting the new block *after* already consuming + // results produced within it, we associate each result with an index and + // include "old" results in the batch. + // Downstream consumers are expected to deduplicate by index. + // We represent the index as a string to maintain compatibility with + // JSON.stringify and to avoid overly inflating the data size (e.g. with + // "@qclass" objects from makeMarshal serialize functions). + const index = forceIndex || String(nextIndex); + nextIndex += 1n; + results.push([index, value]); + const batch = harden(oldResults.slice().concat(results)); + E(chainStorageNode).setValue(serialize(batch)); + }; + }; + await observeIteration(iterable, { + updateState: makeAcceptor(), + finish: makeAcceptor('finish'), + fail: makeAcceptor('fail'), + }); } From 7d0de45ec14515830c41945fc1a42d559b5c21fd Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Thu, 26 May 2022 00:12:57 -0400 Subject: [PATCH 14/18] test: Improve publishToChainNode tests --- packages/vats/test/test-lib-chainPublish.js | 64 +++++++++++---------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/packages/vats/test/test-lib-chainPublish.js b/packages/vats/test/test-lib-chainPublish.js index 4d51b864824..cb0e66766cd 100644 --- a/packages/vats/test/test-lib-chainPublish.js +++ b/packages/vats/test/test-lib-chainPublish.js @@ -17,23 +17,25 @@ test('publishToChainNode', async t => { advanceTimer, supplyIterationResult, } = inputs; - publishToChainNode(customAsyncIterable, storageNode, { timerService }); + publishToChainNode(customAsyncIterable, storageNode, { timerService }).catch( + err => t.fail(`unexpected error: ${err}`), + ); await E(Promise).resolve(); supplyIterationResult('foo'); await E(Promise).resolve().then(); - t.is(nodeValueHistory.length, 1, 'first result is communicated promptly'); + t.is(nodeValueHistory.length, 1, 'first result is sent promptly'); t.deepEqual( - nodeValueHistory.pop(), + JSON.parse(nodeValueHistory[0]), [['0', 'foo']], 'first result has index 0', ); supplyIterationResult('bar'); await E(Promise).resolve().then(); - t.is(nodeValueHistory.length, 1, 'second result is communicated promptly'); + t.is(nodeValueHistory.length, 2, 'second result is sent promptly'); t.deepEqual( - nodeValueHistory.pop(), + JSON.parse(nodeValueHistory[1]), [ ['0', 'foo'], ['1', 'bar'], @@ -46,11 +48,11 @@ test('publishToChainNode', async t => { await E(Promise).resolve().then(); t.is( nodeValueHistory.length, - 1, - 'result following timer advance is communicated promptly', + 3, + 'result following timer advance is sent promptly', ); t.deepEqual( - nodeValueHistory.pop(), + JSON.parse(nodeValueHistory[2]), [ ['0', 'foo'], ['1', 'bar'], @@ -66,11 +68,11 @@ test('publishToChainNode', async t => { await E(Promise).resolve().then(); t.is( nodeValueHistory.length, - 1, - 'results following multiple timer advances are communicated promptly', + 4, + 'results following multiple timer advances are sent promptly', ); t.deepEqual( - nodeValueHistory.pop(), + JSON.parse(nodeValueHistory[3]), [ ['2', 'baz'], ['3', 'qux'], @@ -80,9 +82,9 @@ test('publishToChainNode', async t => { supplyIterationResult('quux'); await E(Promise).resolve().then(); - t.is(nodeValueHistory.length, 1); + t.is(nodeValueHistory.length, 5); t.deepEqual( - nodeValueHistory.pop(), + JSON.parse(nodeValueHistory[4]), [ ['2', 'baz'], ['3', 'qux'], @@ -96,9 +98,9 @@ test('publishToChainNode', async t => { advanceTimer(); supplyIterationResult('corge'); await E(Promise).resolve().then(); - t.is(nodeValueHistory.length, 1); + t.is(nodeValueHistory.length, 6); t.deepEqual( - nodeValueHistory.pop(), + JSON.parse(nodeValueHistory[5]), [ ['3', 'qux'], ['4', 'quux'], @@ -120,14 +122,16 @@ test('publishToChainNode captures finish value', async t => { nodeValueHistory, supplyIterationResult, } = inputs; - publishToChainNode(customAsyncIterable, storageNode, { timerService }); + publishToChainNode(customAsyncIterable, storageNode, { timerService }).catch( + err => t.fail(`unexpected error: ${err}`), + ); await E(Promise).resolve(); supplyIterationResult.finish('foo'); await E(Promise).resolve().then(); - t.is(nodeValueHistory.length, 1, 'finish is communicated promptly'); + t.is(nodeValueHistory.length, 1, 'finish is sent promptly'); t.deepEqual( - nodeValueHistory.pop(), + JSON.parse(nodeValueHistory[0]), [['finish', 'foo']], 'result has index "finish"', ); @@ -145,14 +149,16 @@ test('publishToChainNode captures fail value', async t => { nodeValueHistory, supplyIterationResult, } = inputs; - publishToChainNode(customAsyncIterable, storageNode, { timerService }); + publishToChainNode(customAsyncIterable, storageNode, { timerService }).catch( + err => t.fail(`unexpected error: ${err}`), + ); await E(Promise).resolve(); supplyIterationResult.fail('foo'); await E(Promise).resolve().then(); - t.is(nodeValueHistory.length, 1, 'fail is communicated promptly'); + t.is(nodeValueHistory.length, 1, 'fail is sent promptly'); t.deepEqual( - nodeValueHistory.pop(), + JSON.parse(nodeValueHistory[0]), [['fail', 'foo']], 'result has index "fail"', ); @@ -178,7 +184,7 @@ test('publishToChainNode custom serialization', async t => { publishToChainNode(customAsyncIterable, storageNode, { timerService, serialize, - }); + }).catch(err => t.fail(`unexpected error: ${err}`)); await E(Promise).resolve(); supplyIterationResult('foo'); @@ -189,8 +195,8 @@ test('publishToChainNode custom serialization', async t => { [['0', 'foo']], 'first result has index 0', ); - t.is(nodeValueHistory.length, 1, 'first result is communicated promptly'); - t.is(nodeValueHistory[0], 1, 'first result is output from serialize()'); + t.is(nodeValueHistory.length, 1, 'first result is sent promptly'); + t.is(nodeValueHistory[0], '1', 'first result is output from serialize()'); supplyIterationResult('bar'); await E(Promise).resolve().then(); @@ -203,8 +209,8 @@ test('publishToChainNode custom serialization', async t => { ], 'second result is appended', ); - t.is(nodeValueHistory.length, 2, 'second result is communicated promptly'); - t.is(nodeValueHistory[1], 2, 'second result is output from serialize()'); + t.is(nodeValueHistory.length, 2, 'second result is sent promptly'); + t.is(nodeValueHistory[1], '2', 'second result is output from serialize()'); supplyIterationResult.finish('baz'); await E(Promise).resolve().then(); @@ -218,8 +224,8 @@ test('publishToChainNode custom serialization', async t => { ], 'finish result is appended', ); - t.is(nodeValueHistory.length, 3, 'finish result is communicated promptly'); - t.is(nodeValueHistory[2], 3, 'finish result is output from serialize()'); + t.is(nodeValueHistory.length, 3, 'finish result is sent promptly'); + t.is(nodeValueHistory[2], '3', 'finish result is output from serialize()'); }); // TODO: Move to a testing library. @@ -228,7 +234,7 @@ function getMockInputs(t) { const nodeValueHistory = []; const storageNode = { setValue(val) { - nodeValueHistory.push(JSON.parse(val)); + nodeValueHistory.push(val); }, }; From 48107bae079e31740a41660faa10487accb4ca82 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Thu, 26 May 2022 02:06:42 -0400 Subject: [PATCH 15/18] test: Add publishToChainNode "no drops" test --- packages/vats/src/lib-chainPublish.js | 6 +-- packages/vats/test/test-lib-chainPublish.js | 53 +++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/packages/vats/src/lib-chainPublish.js b/packages/vats/src/lib-chainPublish.js index b31ce607624..9723fab414c 100644 --- a/packages/vats/src/lib-chainPublish.js +++ b/packages/vats/src/lib-chainPublish.js @@ -37,7 +37,7 @@ export async function publishToChainNode( } // To avoid loss when detecting the new block *after* already consuming // results produced within it, we associate each result with an index and - // include "old" results in the batch. + // include results of the previous batch. // Downstream consumers are expected to deduplicate by index. // We represent the index as a string to maintain compatibility with // JSON.stringify and to avoid overly inflating the data size (e.g. with @@ -45,8 +45,8 @@ export async function publishToChainNode( const index = forceIndex || String(nextIndex); nextIndex += 1n; results.push([index, value]); - const batch = harden(oldResults.slice().concat(results)); - E(chainStorageNode).setValue(serialize(batch)); + const combined = harden(oldResults.slice().concat(results)); + E(chainStorageNode).setValue(serialize(combined)); }; }; await observeIteration(iterable, { diff --git a/packages/vats/test/test-lib-chainPublish.js b/packages/vats/test/test-lib-chainPublish.js index cb0e66766cd..a4b9201a468 100644 --- a/packages/vats/test/test-lib-chainPublish.js +++ b/packages/vats/test/test-lib-chainPublish.js @@ -228,6 +228,59 @@ test('publishToChainNode custom serialization', async t => { t.is(nodeValueHistory[2], '3', 'finish result is output from serialize()'); }); +test('publishToChainNode does not drop values', async t => { + // eslint-disable-next-line no-use-before-define + const { storageNode, nodeValueHistory } = getMockInputs(t); + + // This approach is a little intricate... + // We start by producing a sequence of increasing nonnegative values, + // and switch to decreasing negative values upon the response to a + // timerService delay call being fulfilled. + // The delay response intentionally takes many turns to fulfill, + // and we expect at least one negative value to be consumed between its + // fulfillment and acknowledgement thereof by the caller (i.e., new block + // detection). + // We then verify that the first dropped batch included at least one + // negative value. + let nextValue = 0; + let step = 1; + const timerService = { + delay() { + if (step < 0) return Promise.resolve(); + let slow = Promise.resolve(); + for (let i = 0; i < 10; i += 1) slow = slow.then(); + return slow.then(() => { + nextValue = -1; + step = -1; + }); + }, + }; + let oldestIndex; + let oldestValue; + const batchDropped = () => { + if (!nodeValueHistory.length) return false; + const lastSent = JSON.parse(nodeValueHistory[nodeValueHistory.length - 1]); + [oldestIndex, oldestValue] = lastSent[0]; + return oldestIndex !== '0' || oldestValue !== 0; + }; + const source = (async function* makeSource() { + while (!batchDropped()) { + if (Math.abs(nextValue) >= 1e5) throw new Error('too many iterations'); + yield nextValue; + nextValue += step; + } + })(); + await publishToChainNode(source, storageNode, { timerService }).catch(err => + t.fail(`unexpected error: ${err}`), + ); + t.true( + Number(oldestIndex) > 0 && Number(oldestValue) < 0, + `result after first dropped batch should have positive index and negative value: ${JSON.stringify( + [oldestIndex, oldestValue], + )}`, + ); +}); + // TODO: Move to a testing library. function getMockInputs(t) { // Mock a chainStorage node. From dd1d898817482c31b4b6150a5065f2d56c577364 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Thu, 26 May 2022 03:09:32 -0400 Subject: [PATCH 16/18] chore: Try to appease TypeScript --- packages/vats/src/lib-chainPublish.js | 4 ++-- packages/vats/test/test-lib-chainPublish.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/vats/src/lib-chainPublish.js b/packages/vats/src/lib-chainPublish.js index 9723fab414c..24cd01ebb90 100644 --- a/packages/vats/src/lib-chainPublish.js +++ b/packages/vats/src/lib-chainPublish.js @@ -10,8 +10,8 @@ import { E } from '@endo/far'; * Array items are possibly-duplicated [index, value] pairs, where index is a * string (ascending numeric or terminal "finish" or "fail"). * - * @param {AsyncIterator} iterable - * @param {ReturnType} chainStorageNode + * @param {ERef} iterable + * @param {{ setValue: (val: any) => void }} chainStorageNode * @param {{ timerService: ERef, serialize?: (obj: any) => string }} powers */ export async function publishToChainNode( diff --git a/packages/vats/test/test-lib-chainPublish.js b/packages/vats/test/test-lib-chainPublish.js index a4b9201a468..7887e7084ea 100644 --- a/packages/vats/test/test-lib-chainPublish.js +++ b/packages/vats/test/test-lib-chainPublish.js @@ -293,7 +293,7 @@ function getMockInputs(t) { // Mock a timerService. let advanceTimerP; - let advanceTimerR = () => {}; + let advanceTimerR = _val => {}; const advanceTimer = () => { advanceTimerR(); advanceTimerP = new Promise(resolve => { @@ -308,7 +308,7 @@ function getMockInputs(t) { }; // Create an async iterable with a function for supplying results. - const pendingResults = [{ resolve() {} }]; + const pendingResults = [{ resolve(_val) {}, reject(_reason) {} }]; const supplyIterationResult = value => { const nextDeferred = {}; nextDeferred.promise = new Promise((resolve, reject) => { From 0f4c1cf8db6bf6bfa2c4194d5c392932c8d91c68 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Thu, 26 May 2022 04:22:40 -0400 Subject: [PATCH 17/18] chore: Try harder to appease TypeScript --- packages/vats/src/lib-chainPublish.js | 6 +++--- packages/vats/test/test-lib-chainPublish.js | 7 +++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/vats/src/lib-chainPublish.js b/packages/vats/src/lib-chainPublish.js index 24cd01ebb90..b4ec07fdd18 100644 --- a/packages/vats/src/lib-chainPublish.js +++ b/packages/vats/src/lib-chainPublish.js @@ -10,12 +10,12 @@ import { E } from '@endo/far'; * Array items are possibly-duplicated [index, value] pairs, where index is a * string (ascending numeric or terminal "finish" or "fail"). * - * @param {ERef} iterable + * @param {ERef} source * @param {{ setValue: (val: any) => void }} chainStorageNode * @param {{ timerService: ERef, serialize?: (obj: any) => string }} powers */ export async function publishToChainNode( - iterable, + source, chainStorageNode, { timerService, serialize = JSON.stringify }, ) { @@ -49,7 +49,7 @@ export async function publishToChainNode( E(chainStorageNode).setValue(serialize(combined)); }; }; - await observeIteration(iterable, { + await observeIteration(source, { updateState: makeAcceptor(), finish: makeAcceptor('finish'), fail: makeAcceptor('fail'), diff --git a/packages/vats/test/test-lib-chainPublish.js b/packages/vats/test/test-lib-chainPublish.js index 7887e7084ea..62b653e6810 100644 --- a/packages/vats/test/test-lib-chainPublish.js +++ b/packages/vats/test/test-lib-chainPublish.js @@ -244,8 +244,9 @@ test('publishToChainNode does not drop values', async t => { // negative value. let nextValue = 0; let step = 1; + /** @type TimerService */ const timerService = { - delay() { + delay(_n) { if (step < 0) return Promise.resolve(); let slow = Promise.resolve(); for (let i = 0; i < 10; i += 1) slow = slow.then(); @@ -300,6 +301,7 @@ function getMockInputs(t) { advanceTimerR = resolve; }); }; + /** @type TimerService */ const timerService = { delay(n) { t.is(n, 1n); @@ -308,7 +310,8 @@ function getMockInputs(t) { }; // Create an async iterable with a function for supplying results. - const pendingResults = [{ resolve(_val) {}, reject(_reason) {} }]; + /** @type Array<{ promise: Promise, resolve: (val: any) => void, reject: (reason: any) => void }> */ + const pendingResults = [{ resolve(_val) {} }]; const supplyIterationResult = value => { const nextDeferred = {}; nextDeferred.promise = new Promise((resolve, reject) => { From e3efdc80935d03bb987c46fbe76d42d61e5142ce Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Thu, 26 May 2022 10:45:24 -0400 Subject: [PATCH 18/18] chore: Hammer down TypeScript errors --- packages/vats/test/test-lib-chainPublish.js | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/vats/test/test-lib-chainPublish.js b/packages/vats/test/test-lib-chainPublish.js index 62b653e6810..1ee31623848 100644 --- a/packages/vats/test/test-lib-chainPublish.js +++ b/packages/vats/test/test-lib-chainPublish.js @@ -2,6 +2,7 @@ import { test } from '@agoric/swingset-vat/tools/prepare-test-env-ava.js'; import { E } from '@endo/far'; +import { makePromiseKit } from '@endo/promise-kit'; import { publishToChainNode } from '../src/lib-chainPublish.js'; test('publishToChainNode', async t => { @@ -244,15 +245,16 @@ test('publishToChainNode does not drop values', async t => { // negative value. let nextValue = 0; let step = 1; - /** @type TimerService */ + /** @type any */ const timerService = { delay(_n) { - if (step < 0) return Promise.resolve(); + if (step < 0) return Promise.resolve(0n); let slow = Promise.resolve(); for (let i = 0; i < 10; i += 1) slow = slow.then(); return slow.then(() => { nextValue = -1; step = -1; + return 0n; }); }, }; @@ -301,7 +303,7 @@ function getMockInputs(t) { advanceTimerR = resolve; }); }; - /** @type TimerService */ + /** @type any */ const timerService = { delay(n) { t.is(n, 1n); @@ -310,15 +312,9 @@ function getMockInputs(t) { }; // Create an async iterable with a function for supplying results. - /** @type Array<{ promise: Promise, resolve: (val: any) => void, reject: (reason: any) => void }> */ - const pendingResults = [{ resolve(_val) {} }]; + const pendingResults = [makePromiseKit()]; const supplyIterationResult = value => { - const nextDeferred = {}; - nextDeferred.promise = new Promise((resolve, reject) => { - nextDeferred.resolve = resolve; - nextDeferred.reject = reject; - }); - const newLength = pendingResults.push(nextDeferred); + const newLength = pendingResults.push(makePromiseKit()); pendingResults[newLength - 2].resolve({ value }); }; supplyIterationResult.finish = value => {