From 028e7b6bdfa4adaae78600377caf42327aeb9484 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 8164098ad80..36e19c86d1f 100644 --- a/packages/vats/src/core/manifest.js +++ b/packages/vats/src/core/manifest.js @@ -93,6 +93,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 47f4cd731b1..f48b1897be9 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 487f20852a9d84023b6bfcb796d4e81a0d02a2ac 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 ea4fd596dda..ca903a8b7ef 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" + }, "ibc": { "sourceSpec": "@agoric/vats/src/vat-ibc.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 36e19c86d1f..ef4cc9717a7 100644 --- a/packages/vats/src/core/manifest.js +++ b/packages/vats/src/core/manifest.js @@ -96,6 +96,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 fcbd4249d7e3d3afec92fec03b3cb399fd09c03f 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 7e45396aaf11fa53386185fa4be2369bfc8c9674 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 61e05d1d5b4417ff16344e0e1e3a0345a543fe02 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 8f7ed497466feb34998570d6ae2811eed583eff9 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 c40cc45efbed76d984037be9fc645b0e168e6e6c 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 53d954357e85a7eee054edd39d8a26ca2e4bf56b 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 02ba962a253ffca2e37f9c3426492493e7d94ae0 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 f48b1897be9..cc35d1dc8fc 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 db67d1e21accaa580b55933e67a46d3d7d28ef36 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Thu, 26 May 2022 11:52:08 -0400 Subject: [PATCH 10/18] style: Use a better TypeScript-compatible map initialization --- packages/vats/test/test-lib-chainStorage.js | 30 +++++++++++---------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/vats/test/test-lib-chainStorage.js b/packages/vats/test/test-lib-chainStorage.js index 036b78ab4ad..6ae15370c6e 100644 --- a/packages/vats/test/test-lib-chainStorage.js +++ b/packages/vats/test/test-lib-chainStorage.js @@ -32,20 +32,22 @@ test('makeChainStorageRoot', async t => { t.is(rootNode.getKey(), rootKey, 'root key matches initialization input'); // Values must be strings. - /** @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'; - }, - }); + const nonStrings = new Map( + Object.entries({ + number: 1, + bigint: 1n, + boolean: true, + null: null, + undefined, + symbol: Symbol('foo'), + array: ['foo'], + object: { + toString() { + return 'foo'; + }, + }, + }), + ); for (const [label, val] of nonStrings) { t.throws( () => rootNode.setValue(val), From ac504dd15e5640f7bffa8f2333c096a11bb66245 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Tue, 31 May 2022 17:30:25 -0400 Subject: [PATCH 11/18] refactor: Create Go and JS constants for top-level chain storage paths --- golang/cosmos/x/swingset/keeper/keeper.go | 20 ++++++++++++++----- packages/cosmic-swingset/src/chain-main.js | 5 +++-- .../src/chain-storage-paths.js | 11 ++++++++++ packages/vats/src/core/chain-behaviors.js | 6 ++---- 4 files changed, 31 insertions(+), 11 deletions(-) create mode 100644 packages/cosmic-swingset/src/chain-storage-paths.js diff --git a/golang/cosmos/x/swingset/keeper/keeper.go b/golang/cosmos/x/swingset/keeper/keeper.go index d03e767896b..7ed63841d4a 100644 --- a/golang/cosmos/x/swingset/keeper/keeper.go +++ b/golang/cosmos/x/swingset/keeper/keeper.go @@ -19,6 +19,16 @@ import ( "github.com/Agoric/agoric-sdk/golang/cosmos/x/swingset/types" ) +// Top-level paths for chain storage should remain synchronized with +// packages/cosmic-swingset/src/chain-storage-paths.js +const ( + StoragePathActivityhash = "activityhash" + StoragePathBeansOwing = "beansOwing" + StoragePathEgress = "egress" + StoragePathMailbox = "mailbox" + StoragePathCustom = "published" +) + // Keeper maintains the link to data storage and exposes getter/setter methods for the various parts of the state machine type Keeper struct { storeKey sdk.StoreKey @@ -140,7 +150,7 @@ func (k Keeper) GetBeansPerUnit(ctx sdk.Context) map[string]sdk.Uint { } func getBeansOwingPathForAddress(addr sdk.AccAddress) string { - return "beansOwing." + addr.String() + return StoragePathBeansOwing + "." + addr.String() } // GetBeansOwing returns the number of beans that the given address owes to @@ -205,7 +215,7 @@ func (k Keeper) GetBalance(ctx sdk.Context, addr sdk.AccAddress, denom string) s // GetEgress gets the entire egress struct for a peer func (k Keeper) GetEgress(ctx sdk.Context, addr sdk.AccAddress) types.Egress { - path := "egress." + addr.String() + path := StoragePathEgress + "." + addr.String() value := k.GetStorage(ctx, path) if value == "" { return types.Egress{} @@ -222,7 +232,7 @@ func (k Keeper) GetEgress(ctx sdk.Context, addr sdk.AccAddress) types.Egress { // SetEgress sets the egress struct for a peer, and ensures its account exists func (k Keeper) SetEgress(ctx sdk.Context, egress *types.Egress) error { - path := "egress." + egress.Peer.String() + path := StoragePathEgress + "." + egress.Peer.String() bz, err := json.Marshal(egress) if err != nil { @@ -374,12 +384,12 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger { // GetMailbox gets the entire mailbox struct for a peer func (k Keeper) GetMailbox(ctx sdk.Context, peer string) string { - path := "mailbox." + peer + path := StoragePathMailbox + "." + peer return k.GetStorage(ctx, path) } // SetMailbox sets the entire mailbox struct for a peer func (k Keeper) SetMailbox(ctx sdk.Context, peer string, mailbox string) { - path := "mailbox." + peer + path := StoragePathMailbox + "." + peer k.SetStorage(ctx, path, mailbox) } diff --git a/packages/cosmic-swingset/src/chain-main.js b/packages/cosmic-swingset/src/chain-main.js index 287699b3342..e26e57f17dc 100644 --- a/packages/cosmic-swingset/src/chain-main.js +++ b/packages/cosmic-swingset/src/chain-main.js @@ -12,6 +12,7 @@ import stringify from './json-stable-stringify.js'; import { launch } from './launch-chain.js'; import makeBlockManager from './block-manager.js'; import { getTelemetryProviders } from './kernel-stats.js'; +import * as STORAGE_PATH from './chain-storage-paths.js'; // eslint-disable-next-line no-unused-vars let whenHellFreezesOver = null; @@ -295,7 +296,7 @@ export default async function main(progname, args, { env, homedir, agcc }) { // this object is used to store the mailbox state. const mailboxStorage = makeChainStorage( msg => chainSend(portNums.storage, msg), - 'mailbox.', + STORAGE_PATH.MAILBOX + '.', { fromChainShape: data => { const ack = toNumber(data.ack); @@ -312,7 +313,7 @@ export default async function main(progname, args, { env, homedir, agcc }) { function setActivityhash(activityhash) { const msg = stringify({ method: 'set', - key: 'activityhash', + key: STORAGE_PATH.ACTIVITYHASH, value: activityhash, }); chainSend(portNums.storage, msg); diff --git a/packages/cosmic-swingset/src/chain-storage-paths.js b/packages/cosmic-swingset/src/chain-storage-paths.js new file mode 100644 index 00000000000..c0f26de73b1 --- /dev/null +++ b/packages/cosmic-swingset/src/chain-storage-paths.js @@ -0,0 +1,11 @@ +/** + * These identify top-level paths for SwingSet chain storage + * (and serve as prefixes, with the exception of ACTIVITYHASH). + * To avoid collisions, they should remain synchronized with + * golang/cosmos/x/swingset/keeper/keeper.go + */ +export const ACTIVITYHASH = 'activityhash'; +export const BEANSOWING = 'beansOwing'; +export const EGRESS = 'egress'; +export const MAILBOX = 'mailbox'; +export const CUSTOM = 'published'; diff --git a/packages/vats/src/core/chain-behaviors.js b/packages/vats/src/core/chain-behaviors.js index 306e2a7f84f..9d76ee6a144 100644 --- a/packages/vats/src/core/chain-behaviors.js +++ b/packages/vats/src/core/chain-behaviors.js @@ -16,6 +16,7 @@ import { import { importBundle } from '@endo/import-bundle'; import * as Collect from '@agoric/run-protocol/src/collect.js'; +import * as STORAGE_PATH from '@agoric/cosmic-swingset/src/chain-storage-paths.js'; import { makeBridgeManager as makeBridgeManagerKit } from '../bridge.js'; import * as BRIDGE_ID from '../bridge-ids.js'; @@ -298,10 +299,7 @@ export const makeChainStorage = async ({ return; } - // 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'; + const ROOT_KEY = STORAGE_PATH.CUSTOM; const vat = E(loadVat)('chainStorage'); const rootNodeP = E(vat).makeBridgedChainStorageRoot( From 9898385bbdf9a78839149666b01fb41ff2963e5d Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Thu, 2 Jun 2022 13:24:34 -0400 Subject: [PATCH 12/18] refactor: Replace chain storage "key" with "path" --- packages/vats/src/core/chain-behaviors.js | 4 +- packages/vats/src/lib-chainStorage.js | 36 ++++++++++------ packages/vats/src/vat-chainStorage.js | 7 +-- packages/vats/test/test-lib-chainStorage.js | 47 +++++++++++++-------- 4 files changed, 58 insertions(+), 36 deletions(-) diff --git a/packages/vats/src/core/chain-behaviors.js b/packages/vats/src/core/chain-behaviors.js index 9d76ee6a144..816a584b12e 100644 --- a/packages/vats/src/core/chain-behaviors.js +++ b/packages/vats/src/core/chain-behaviors.js @@ -299,13 +299,13 @@ export const makeChainStorage = async ({ return; } - const ROOT_KEY = STORAGE_PATH.CUSTOM; + const ROOT_PATH = STORAGE_PATH.CUSTOM; const vat = E(loadVat)('chainStorage'); const rootNodeP = E(vat).makeBridgedChainStorageRoot( bridgeManager, BRIDGE_ID.STORAGE, - ROOT_KEY, + ROOT_PATH, ); chainStorageP.resolve(rootNodeP); }; diff --git a/packages/vats/src/lib-chainStorage.js b/packages/vats/src/lib-chainStorage.js index 5f7f16f73c3..8bb628329fb 100644 --- a/packages/vats/src/lib-chainStorage.js +++ b/packages/vats/src/lib-chainStorage.js @@ -11,18 +11,26 @@ const { details: X } = assert; const pathSegmentPattern = /^[a-zA-Z0-9_-]{1,100}$/; /** - * Create a root storage node for a given backing function and root key. + * Create a root storage node for a given backing function and root path. * * @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 + * @param {string} storeName currently limited to "swingset" + * @param {string} rootPath */ -export function makeChainStorageRoot(toStorage, rootKey) { - assert.typeof(rootKey, 'string'); +export function makeChainStorageRoot(toStorage, storeName, rootPath) { + assert.equal( + storeName, + 'swingset', + 'the only currently-supported store is "swingset"', + ); + assert.typeof(rootPath, 'string'); - function makeChainStorageNode(key) { + function makeChainStorageNode(path) { const node = { - getKey() { - return key; + getStoreKey() { + // This duplicates the Go code at + // https://github.com/Agoric/agoric-sdk/blob/cb272ae97a042ceefd3af93b1b4601ca49dfe3a7/golang/cosmos/x/swingset/keeper/keeper.go#L295 + return { storeName, storeSubkey: `swingset/data:${path}` }; }, getChildNode(name) { assert.typeof(name, 'string'); @@ -30,14 +38,14 @@ export function makeChainStorageRoot(toStorage, rootKey) { pathSegmentPattern.test(name), X`Path segment must be a short ASCII identifier: ${name}`, ); - return makeChainStorageNode(`${key}.${name}`); + return makeChainStorageNode(`${path}.${name}`); }, setValue(value) { assert.typeof(value, 'string'); - toStorage({ key, method: 'set', value }); + toStorage({ key: path, method: 'set', value }); }, async delete() { - assert(key !== rootKey); + assert(path !== rootPath); // 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). @@ -48,11 +56,11 @@ export function makeChainStorageRoot(toStorage, rootKey) { // 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' }); + const childCount = await toStorage({ key: path, method: 'size' }); if (childCount > 0) { - assert.fail(X`Refusing to delete node with children: ${key}`); + assert.fail(X`Refusing to delete node with children: ${path}`); } - toStorage({ key, method: 'set' }); + toStorage({ key: path, method: 'set' }); }, // Possible extensions: // * getValue() @@ -65,6 +73,6 @@ export function makeChainStorageRoot(toStorage, rootKey) { return Far('chainStorageNode', node); } - const rootNode = makeChainStorageNode(rootKey); + const rootNode = makeChainStorageNode(rootPath); return rootNode; } diff --git a/packages/vats/src/vat-chainStorage.js b/packages/vats/src/vat-chainStorage.js index 0786aa3c7c2..aadb858b74b 100644 --- a/packages/vats/src/vat-chainStorage.js +++ b/packages/vats/src/vat-chainStorage.js @@ -2,10 +2,11 @@ import { E, Far } from '@endo/far'; import { makeChainStorageRoot } from './lib-chainStorage.js'; export function buildRootObject(_vatPowers) { - function makeBridgedChainStorageRoot(bridgeManager, bridgeId, rootKey) { - // XXX: Should we validate uniqueness of rootKey, or is that an external concern? + function makeBridgedChainStorageRoot(bridgeManager, bridgeId, rootPath) { + // Note that the uniqueness of rootPath is not validated here, + // and is instead the responsibility of callers. const toStorage = message => E(bridgeManager).toBridge(bridgeId, message); - const rootNode = makeChainStorageRoot(toStorage, rootKey); + const rootNode = makeChainStorageRoot(toStorage, 'swingset', rootPath); return rootNode; } diff --git a/packages/vats/test/test-lib-chainStorage.js b/packages/vats/test/test-lib-chainStorage.js index 6ae15370c6e..5d35a08bfb4 100644 --- a/packages/vats/test/test-lib-chainStorage.js +++ b/packages/vats/test/test-lib-chainStorage.js @@ -27,9 +27,15 @@ test('makeChainStorageRoot', async t => { 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'); + const rootPath = 'root'; + const rootNode = makeChainStorageRoot(toStorage, 'swingset', rootPath); + t.deepEqual( + rootNode.getStoreKey(), + { storeName: 'swingset', storeSubkey: `swingset/data:${rootPath}` }, + 'root store key matches initialization input', + ); + + t.throws(() => makeChainStorageRoot(toStorage, 'notswingset', rootPath)); // Values must be strings. const nonStrings = new Map( @@ -65,17 +71,17 @@ test('makeChainStorageRoot', async t => { rootNode.setValue('foo'); t.deepEqual( messages.slice(-1), - [{ key: rootKey, method: 'set', value: 'foo' }], + [{ key: rootPath, method: 'set', value: 'foo' }], 'root node setValue message', ); rootNode.setValue('bar'); t.deepEqual( messages.slice(-1), - [{ key: rootKey, method: 'set', value: 'bar' }], + [{ key: rootPath, method: 'set', value: 'bar' }], 'second setValue message', ); - // Valid key segments are strings of up to 100 ASCII alphanumeric/dash/underscore characters. + // Valid path segments are strings of up to 100 ASCII alphanumeric/dash/underscore characters. const validSegmentChars = `${ Array(26) .fill(undefined) @@ -99,24 +105,28 @@ test('makeChainStorageRoot', async t => { .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'); + const childPath = `${rootPath}.${segment}`; + t.deepEqual( + child.getStoreKey(), + { storeName: 'swingset', storeSubkey: `swingset/data:${childPath}` }, + 'path segments are dot-separated', + ); child.setValue('foo'); t.deepEqual( messages.slice(-1), - [{ key: childKey, method: 'set', value: 'foo' }], + [{ key: childPath, 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' }], + [{ key: childPath, method: 'set' }], 'non-root delete message', ); } - // Invalid key segments are non-strings, empty, too long, or contain unacceptable characters. + // Invalid path 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)); @@ -141,10 +151,13 @@ test('makeChainStorageRoot', async t => { // Level-skipping creation is allowed. const childNode = rootNode.getChildNode('child'); - const childKey = `${rootKey}.child`; + const childPath = `${rootPath}.child`; const deepNode = childNode.getChildNode('grandchild'); - const deepKey = `${childKey}.grandchild`; - t.is(deepNode.getKey(), deepKey); + const deepPath = `${childPath}.grandchild`; + t.deepEqual(deepNode.getStoreKey(), { + storeName: 'swingset', + storeSubkey: `swingset/data:${deepPath}`, + }); for (const [label, val] of nonStrings) { t.throws( () => deepNode.setValue(val), @@ -155,7 +168,7 @@ test('makeChainStorageRoot', async t => { deepNode.setValue('foo'); t.deepEqual( messages.slice(-1), - [{ key: deepKey, method: 'set', value: 'foo' }], + [{ key: deepPath, method: 'set', value: 'foo' }], 'level-skipping setValue message', ); @@ -168,13 +181,13 @@ test('makeChainStorageRoot', async t => { await deepNode.delete(); t.deepEqual( messages.slice(-1), - [{ key: deepKey, method: 'set' }], + [{ key: deepPath, method: 'set' }], 'granchild delete message', ); await childNode.delete(); t.deepEqual( messages.slice(-1), - [{ key: childKey, method: 'set' }], + [{ key: childPath, method: 'set' }], 'child delete message', ); }); From 265cd52d72c579edde82408438f7c47563ddb61b Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Thu, 2 Jun 2022 13:32:50 -0400 Subject: [PATCH 13/18] refactor: Move chain-storage-paths.js to avoid a module cycle --- golang/cosmos/x/swingset/keeper/keeper.go | 2 +- packages/cosmic-swingset/src/chain-main.js | 2 +- packages/{cosmic-swingset => vats}/src/chain-storage-paths.js | 0 packages/vats/src/core/chain-behaviors.js | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename packages/{cosmic-swingset => vats}/src/chain-storage-paths.js (100%) diff --git a/golang/cosmos/x/swingset/keeper/keeper.go b/golang/cosmos/x/swingset/keeper/keeper.go index 7ed63841d4a..4757cc5f5a4 100644 --- a/golang/cosmos/x/swingset/keeper/keeper.go +++ b/golang/cosmos/x/swingset/keeper/keeper.go @@ -20,7 +20,7 @@ import ( ) // Top-level paths for chain storage should remain synchronized with -// packages/cosmic-swingset/src/chain-storage-paths.js +// packages/vats/src/chain-storage-paths.js const ( StoragePathActivityhash = "activityhash" StoragePathBeansOwing = "beansOwing" diff --git a/packages/cosmic-swingset/src/chain-main.js b/packages/cosmic-swingset/src/chain-main.js index e26e57f17dc..5560413fcf0 100644 --- a/packages/cosmic-swingset/src/chain-main.js +++ b/packages/cosmic-swingset/src/chain-main.js @@ -8,11 +8,11 @@ import { import { assert, details as X } from '@agoric/assert'; import { makeSlogSenderFromModule } from '@agoric/telemetry'; +import * as STORAGE_PATH from '@agoric/vats/src/chain-storage-paths.js'; import stringify from './json-stable-stringify.js'; import { launch } from './launch-chain.js'; import makeBlockManager from './block-manager.js'; import { getTelemetryProviders } from './kernel-stats.js'; -import * as STORAGE_PATH from './chain-storage-paths.js'; // eslint-disable-next-line no-unused-vars let whenHellFreezesOver = null; diff --git a/packages/cosmic-swingset/src/chain-storage-paths.js b/packages/vats/src/chain-storage-paths.js similarity index 100% rename from packages/cosmic-swingset/src/chain-storage-paths.js rename to packages/vats/src/chain-storage-paths.js diff --git a/packages/vats/src/core/chain-behaviors.js b/packages/vats/src/core/chain-behaviors.js index 816a584b12e..da979c3c986 100644 --- a/packages/vats/src/core/chain-behaviors.js +++ b/packages/vats/src/core/chain-behaviors.js @@ -16,9 +16,9 @@ import { import { importBundle } from '@endo/import-bundle'; import * as Collect from '@agoric/run-protocol/src/collect.js'; -import * as STORAGE_PATH from '@agoric/cosmic-swingset/src/chain-storage-paths.js'; import { makeBridgeManager as makeBridgeManagerKit } from '../bridge.js'; import * as BRIDGE_ID from '../bridge-ids.js'; +import * as STORAGE_PATH from '../chain-storage-paths.js'; import { callProperties, extractPowers } from './utils.js'; From 69d8d4fecd4c4c450b6b7816b1303d0029bac065 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Thu, 2 Jun 2022 14:04:31 -0400 Subject: [PATCH 14/18] chore: Appease eslint --- packages/cosmic-swingset/src/chain-main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cosmic-swingset/src/chain-main.js b/packages/cosmic-swingset/src/chain-main.js index 5560413fcf0..f02e5fee0b5 100644 --- a/packages/cosmic-swingset/src/chain-main.js +++ b/packages/cosmic-swingset/src/chain-main.js @@ -296,7 +296,7 @@ export default async function main(progname, args, { env, homedir, agcc }) { // this object is used to store the mailbox state. const mailboxStorage = makeChainStorage( msg => chainSend(portNums.storage, msg), - STORAGE_PATH.MAILBOX + '.', + `${STORAGE_PATH.MAILBOX}.`, { fromChainShape: data => { const ack = toNumber(data.ack); From 3c7c2704562f3fa0c2a5501ee3d4f31f5b1b7b2e Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Fri, 3 Jun 2022 13:35:02 -0400 Subject: [PATCH 15/18] fix: Try adding a delay to fix CI https://github.com/Agoric/agoric-sdk/runs/6728088019?check_suite_focus=true --- packages/vats/src/core/chain-behaviors.js | 8 +++++++- packages/vats/src/core/manifest.js | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/vats/src/core/chain-behaviors.js b/packages/vats/src/core/chain-behaviors.js index da979c3c986..e8270642c7e 100644 --- a/packages/vats/src/core/chain-behaviors.js +++ b/packages/vats/src/core/chain-behaviors.js @@ -289,7 +289,13 @@ harden(makeBridgeManager); * }} powers */ export const makeChainStorage = async ({ - consume: { bridgeManager: bridgeManagerP, loadVat }, + consume: { + bridgeManager: bridgeManagerP, + loadVat, + // provisioning is here to attempt delaying execution for avoiding failures like + // https://github.com/Agoric/agoric-sdk/runs/6728088019?check_suite_focus=true + provisioning: _provisioning, + }, produce: { chainStorage: chainStorageP }, }) => { const bridgeManager = await bridgeManagerP; diff --git a/packages/vats/src/core/manifest.js b/packages/vats/src/core/manifest.js index ef4cc9717a7..4dae4cefe92 100644 --- a/packages/vats/src/core/manifest.js +++ b/packages/vats/src/core/manifest.js @@ -97,6 +97,9 @@ const SHARED_CHAIN_BOOTSTRAP_MANIFEST = harden({ consume: { bridgeManager: true, loadVat: true, + // provisioning is here to attempt delaying execution for avoiding failures like + // https://github.com/Agoric/agoric-sdk/runs/6728088019?check_suite_focus=true + provisioning: true, }, produce: { chainStorage: true, From e718f25fa892693e2d7f4f1dc66839ca3111ea86 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Fri, 3 Jun 2022 15:47:15 -0400 Subject: [PATCH 16/18] chore: Include relevant lines from failing CI log --- packages/vats/src/core/chain-behaviors.js | 3 +++ packages/vats/src/core/manifest.js | 3 +++ 2 files changed, 6 insertions(+) diff --git a/packages/vats/src/core/chain-behaviors.js b/packages/vats/src/core/chain-behaviors.js index e8270642c7e..2c37fca8d6e 100644 --- a/packages/vats/src/core/chain-behaviors.js +++ b/packages/vats/src/core/chain-behaviors.js @@ -294,6 +294,9 @@ export const makeChainStorage = async ({ loadVat, // provisioning is here to attempt delaying execution for avoiding failures like // https://github.com/Agoric/agoric-sdk/runs/6728088019?check_suite_focus=true + // stage-1: monitor-chain: found process 30881 for vat v2 "vatAdmin" + // stage-1: chain: portHandler threw (Error#1) + // stage-1: chain: Error#1: historical inaccuracy in replay of v2 provisioning: _provisioning, }, produce: { chainStorage: chainStorageP }, diff --git a/packages/vats/src/core/manifest.js b/packages/vats/src/core/manifest.js index 4dae4cefe92..9126e15790c 100644 --- a/packages/vats/src/core/manifest.js +++ b/packages/vats/src/core/manifest.js @@ -99,6 +99,9 @@ const SHARED_CHAIN_BOOTSTRAP_MANIFEST = harden({ loadVat: true, // provisioning is here to attempt delaying execution for avoiding failures like // https://github.com/Agoric/agoric-sdk/runs/6728088019?check_suite_focus=true + // stage-1: monitor-chain: found process 30881 for vat v2 "vatAdmin" + // stage-1: chain: portHandler threw (Error#1) + // stage-1: chain: Error#1: historical inaccuracy in replay of v2 provisioning: true, }, produce: { From af3539987269bcb1f17c172ab776142acfb29f7d Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Fri, 3 Jun 2022 21:31:58 -0600 Subject: [PATCH 17/18] fix(swingset): louder anachrophobio --- packages/SwingSet/src/kernel/vat-loader/transcript.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/SwingSet/src/kernel/vat-loader/transcript.js b/packages/SwingSet/src/kernel/vat-loader/transcript.js index 00317d1ed36..f33c1b22274 100644 --- a/packages/SwingSet/src/kernel/vat-loader/transcript.js +++ b/packages/SwingSet/src/kernel/vat-loader/transcript.js @@ -2,9 +2,9 @@ import djson from '../../lib/djson.js'; export function requireIdentical(vatID, originalSyscall, newSyscall) { if (djson.stringify(originalSyscall) !== djson.stringify(newSyscall)) { - console.log(`anachrophobia strikes vat ${vatID}`); - console.log(`expected:`, djson.stringify(originalSyscall)); - console.log(`got :`, djson.stringify(newSyscall)); + console.error(`anachrophobia strikes vat ${vatID}`); + console.error(`expected:`, djson.stringify(originalSyscall)); + console.error(`got :`, djson.stringify(newSyscall)); return new Error(`historical inaccuracy in replay of ${vatID}`); } return undefined; From f22b8f8f303a7e270c4ecc8d11346c1baeef5c3f Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Fri, 3 Jun 2022 23:36:44 -0600 Subject: [PATCH 18/18] ci(vats): correct `chainStorage` bundle in decentral configs --- packages/vats/decentral-core-config.json | 2 +- packages/vats/decentral-demo-config.json | 3 +++ packages/vats/src/core/chain-behaviors.js | 11 +---------- packages/vats/src/core/manifest.js | 6 ------ 4 files changed, 5 insertions(+), 17 deletions(-) diff --git a/packages/vats/decentral-core-config.json b/packages/vats/decentral-core-config.json index ca903a8b7ef..963112eff01 100644 --- a/packages/vats/decentral-core-config.json +++ b/packages/vats/decentral-core-config.json @@ -22,7 +22,7 @@ "sourceSpec": "@agoric/vats/src/vat-board.js" }, "chainStorage": { - "sourceSpec": "./src/vat-chainStorage.js" + "sourceSpec": "@agoric/vats/src/vat-chainStorage.js" }, "ibc": { "sourceSpec": "@agoric/vats/src/vat-ibc.js" diff --git a/packages/vats/decentral-demo-config.json b/packages/vats/decentral-demo-config.json index 62615676300..7494dcaffca 100644 --- a/packages/vats/decentral-demo-config.json +++ b/packages/vats/decentral-demo-config.json @@ -16,6 +16,9 @@ "centralSupply": { "sourceSpec": "@agoric/vats/src/centralSupply.js" }, + "chainStorage": { + "sourceSpec": "@agoric/vats/src/vat-chainStorage.js" + }, "mintHolder": { "sourceSpec": "@agoric/vats/src/mintHolder.js" }, diff --git a/packages/vats/src/core/chain-behaviors.js b/packages/vats/src/core/chain-behaviors.js index 2c37fca8d6e..da979c3c986 100644 --- a/packages/vats/src/core/chain-behaviors.js +++ b/packages/vats/src/core/chain-behaviors.js @@ -289,16 +289,7 @@ harden(makeBridgeManager); * }} powers */ export const makeChainStorage = async ({ - consume: { - bridgeManager: bridgeManagerP, - loadVat, - // provisioning is here to attempt delaying execution for avoiding failures like - // https://github.com/Agoric/agoric-sdk/runs/6728088019?check_suite_focus=true - // stage-1: monitor-chain: found process 30881 for vat v2 "vatAdmin" - // stage-1: chain: portHandler threw (Error#1) - // stage-1: chain: Error#1: historical inaccuracy in replay of v2 - provisioning: _provisioning, - }, + consume: { bridgeManager: bridgeManagerP, loadVat }, produce: { chainStorage: chainStorageP }, }) => { const bridgeManager = await bridgeManagerP; diff --git a/packages/vats/src/core/manifest.js b/packages/vats/src/core/manifest.js index 9126e15790c..ef4cc9717a7 100644 --- a/packages/vats/src/core/manifest.js +++ b/packages/vats/src/core/manifest.js @@ -97,12 +97,6 @@ const SHARED_CHAIN_BOOTSTRAP_MANIFEST = harden({ consume: { bridgeManager: true, loadVat: true, - // provisioning is here to attempt delaying execution for avoiding failures like - // https://github.com/Agoric/agoric-sdk/runs/6728088019?check_suite_focus=true - // stage-1: monitor-chain: found process 30881 for vat v2 "vatAdmin" - // stage-1: chain: portHandler threw (Error#1) - // stage-1: chain: Error#1: historical inaccuracy in replay of v2 - provisioning: true, }, produce: { chainStorage: true,