feat(cosmic-swingset): Add chainStorage interface #5385

gibson042 · 2022-05-17T17:14:47Z

closes: #4558

Description

Adds a chainStorage interface that allows writing to the cosmos KVStore.

Security Considerations

It is important to prevent vats from writing to each others' subtrees, and also to prevent excessive writes to this rather expensive storage.

Documentation Considerations

TBD

Testing Considerations

n/a

gibson042

There are still questions to address, either here or in a followup.

gibson042 · 2022-05-17T17:52:43Z

packages/vats/src/core/chain-behaviors.js

+  const rootNode = makeChainStorageNode(ROOT_KEY);
+  chainStorageP.resolve(rootNode);


This PR shares the root node directly, but it would also be possible to host it from a vat with access to bridgeManager or a derived capability like toStorage.

We try not to host objects directly from the bootstrap vat. Could you add a packages/vats/src/vat-publisher.js or somesuch?

gibson042 · 2022-05-17T17:54:50Z

packages/vats/src/core/chain-behaviors.js

+  // 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';


To avoid the risk of future collision across our own codebase, I've opted to give vat-exposed chain storage its own single top-level key. Any dissenting opinions or suggestions for a different name than "published" would be worth hearing.

A single top-level keys sounds excellent to me.

Does that make the RPC query path something like /swingset/published/$SUFFIX ?

I'm wondering if we should use the same name for the IAVL/RPC-query -side prefix (or post-/swingset/ path component), the vat which holds the publishing nodes, and the overall name of the feature. When someone comes grepping, it'd be nice if they could get from the RPC query path (or client-side code that's reading from it) to the agoric-sdk directories that serve up that data.

As a general feature name, "chain storage" suggests read-write storage to me (held in the chain's data structures), whereas swingset never reads from this data.

"publish" or "publish node" is a term we've used a lot. I don't want to cause a lot of churn, but I'm wondering it this should be makeChainPublisher instead of makeChainStorage. And then ROOT_KEY = 'publish', and the vat this lives in could be called vat-chain-publisher.js.

Names are, of course, the most difficult thing in programming.

gibson042 · 2022-05-17T18:13:54Z

packages/vats/src/core/chain-behaviors.js

+  // 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}$/;


A key is a dot-separated list of segments (e.g., "published.foo.bar"). It seems necessary to disallow (unescaped) "." in a segment, and this PR imposes further restrictions:

Each character of a segment must be an ASCII letter, number, or underscore.

A segment may not be empty.

A segment may not be longer than 100 characters (where the differences between character/code point/code unit/etc. can be ignored because of the first restriction).

It does not currently impose a maximum length on the full key, but could easily do so.

This leaves the door open for future extension, in particular to something like backslash escape sequences and/or a larger subset of Unicode (with the requisite care for control characters/homoglyphs/right-to-left subsequences/etc.).

I'm slightly in favor of using a slash as separator, rather than dot. What do other cosmos-sdk RPC query paths use? We should probably match their style.

I'm personally a fan of hyphen over underscore, for human-readable things (which don't need to follow e.g. JS identifier syntax), so I'd lobby to allow hyphens too. I'm quite happy with the other limits, but we should probably impose some sort of overall limit to prevent abuse/attack via multi-megabyte keys.

gibson042 · 2022-05-17T18:17:06Z

packages/vats/src/core/chain-behaviors.js

+      // Possible extensions:
+      // * getValue()
+      // * getChildNames() and/or getChildNodes()
+      // * getName()
+      // * recursive delete
+      // * batch operations
+      // * local buffering (with end-of-block commit)


Omitted for now, but de facto documented for posterity. Note that the interface for vats is currently write-only—this storage is being treated as precious, and we're intentionally discouraging database-like use.

Yeah, I guess that feeds into the naming question above, if we think there's a decent chance that we'd extend this API to include reads as well, then chain storage is a good name for the feature. If it seems more likely that it will remain strictly write-only, then publish or something other than storage seems better.

gibson042 · 2022-05-17T18:39:41Z

packages/vats/src/core/manifest.js

+      bridgeManager: true,
+    },
+    produce: {
+      chainStorage: true,


This produces chainStorage, but there is not currently anything to consume it. The biggest open question is "how should a derived capability be made available to vats—what makes new child nodes, and where do they get sent?" Neither @warner nor I could find an existing analog, but perhaps the board comes closest?

Some possible answers:

Zoe has access to the root node, and issues a child node of a common parent to contract instances that are somehow marked as privileged.

Something in the bootstrap code has access to the root node, and issues descendant nodes as directed by manifest data (either as part of startInstance or after the fact).

Other post-chain-initialization code has access to the root node, and issues descandant nodes as remoteables to already-running contract instances.

Summarising what I understood from the most recent conversation:

We start by bootstrap having access to the root node, as you have now, on which we will publish (via this publication mechanism) a name-to-storage-path mapping.

Governance "core eval" code will issue descendants to certain contracts via Zoe private arguments and update the name-to-storage path mapping.

We will use explicit ocap introduction patterns to obtain descendants and descendants of descendants outside bootstrap.

There's no pressing need for automatic allocation of descendants. Indeed, that's probably an antipattern until we're certain of performance characteristics.

michaelfig

Looking good!

I'd like to see this separated into its own vat instead of adding a lot more to chain-behaviors.js. I realise I may be hypocritical here, but there are isolation benefits to having its own vat, too.

packages/vats/src/core/chain-behaviors.js

michaelfig · 2022-05-18T19:15:25Z

packages/vats/src/core/chain-behaviors.js

+      async delete() {
+        assert(key !== ROOT_KEY);
+        const childKeys = await toStorage({ key, method: 'keys' });
+        if (childKeys.length > 0) {


This is racy. Is there a specific reason to refuse to delete nodes with children? If so, we should fix this in Go as well.

Nice catch! #5381 describes the root problem and anticipates a future fix in Go. In the meantime, I'll add a comment here.

#5381 describes the root problem and anticipates a future fix in Go.

#5394 solves this problem, conclusively. We'll see whose PR lands first, and the subsequent one can remove the extra await toStorage(...).

I think #5394 is the right place to do this check, let's remove it from here to avoid having two different kinds of errors

warner

Approach looks good, but let's settle on a feature name (since that has a shallow but broad effect on the API naming) and move it into a separate vat. And let's settle the dot-vs-slash path separator question.

warner · 2022-05-19T20:56:31Z

packages/vats/src/core/chain-behaviors.js

+  // 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';


A single top-level keys sounds excellent to me.

Does that make the RPC query path something like /swingset/published/$SUFFIX ?

I'm wondering if we should use the same name for the IAVL/RPC-query -side prefix (or post-/swingset/ path component), the vat which holds the publishing nodes, and the overall name of the feature. When someone comes grepping, it'd be nice if they could get from the RPC query path (or client-side code that's reading from it) to the agoric-sdk directories that serve up that data.

As a general feature name, "chain storage" suggests read-write storage to me (held in the chain's data structures), whereas swingset never reads from this data.

"publish" or "publish node" is a term we've used a lot. I don't want to cause a lot of churn, but I'm wondering it this should be makeChainPublisher instead of makeChainStorage. And then ROOT_KEY = 'publish', and the vat this lives in could be called vat-chain-publisher.js.

Names are, of course, the most difficult thing in programming.

warner · 2022-05-19T21:01:23Z

packages/vats/src/core/chain-behaviors.js

+  // 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}$/;


I'm slightly in favor of using a slash as separator, rather than dot. What do other cosmos-sdk RPC query paths use? We should probably match their style.

I'm personally a fan of hyphen over underscore, for human-readable things (which don't need to follow e.g. JS identifier syntax), so I'd lobby to allow hyphens too. I'm quite happy with the other limits, but we should probably impose some sort of overall limit to prevent abuse/attack via multi-megabyte keys.

warner · 2022-05-19T21:09:22Z

packages/vats/src/core/chain-behaviors.js

+      async delete() {
+        assert(key !== ROOT_KEY);
+        const childKeys = await toStorage({ key, method: 'keys' });
+        if (childKeys.length > 0) {


I think #5394 is the right place to do this check, let's remove it from here to avoid having two different kinds of errors

warner · 2022-05-19T21:11:56Z

packages/vats/src/core/chain-behaviors.js

+      // Possible extensions:
+      // * getValue()
+      // * getChildNames() and/or getChildNodes()
+      // * getName()
+      // * recursive delete
+      // * batch operations
+      // * local buffering (with end-of-block commit)


Yeah, I guess that feeds into the naming question above, if we think there's a decent chance that we'd extend this API to include reads as well, then chain storage is a good name for the feature. If it seems more likely that it will remain strictly write-only, then publish or something other than storage seems better.

warner · 2022-05-19T21:13:16Z

packages/vats/src/core/chain-behaviors.js

+  const rootNode = makeChainStorageNode(ROOT_KEY);
+  chainStorageP.resolve(rootNode);


gibson042 · 2022-05-23T21:49:01Z

Chain storage is now moved into its own vat wrapping a unit-tested library. I have a slight preference for "chain storage" over "chain publisher", because adding read support in the future seems likely (and I have already confirmed read-after-write consistency locally). I have also added support for - in key segments, in addition to _.

@warner Regarding the paths, I don't know about other cosmos-sdk, but swingset has an existing precedent for separating segments with . (cf. swingset keeper.go beansOwing/egress/mailbox, cosmic-swingset chain-main.js mailbox, and solo chain-cosmos-sdk.js mailbox and egress). This results in RPC query paths like /swingset/storage/data/foo.bar.baz, where "data" can also be replaced with "keys" per swingset query.proto and "foo" is the top-level key segment. I also have my doubts that the RPC mapping logic will correctly handle storage paths with /, either raw (e.g. /swingset/storage/data/foo/bar/baz) or percent-encoded (/swingset/storage/data/foo%2Fbar%2Fbaz), because it seems very naïve.

Regarding size limits, I have omitted several:

Value size.
Total descendant value size.
Total key size, e.g. lorem.ipsum.dolor.sit.amet.consectetur.adipiscing.elit.….
Child key count, e.g. parent.{1,2,…,100000}.
Descendant key count, e.g. parent.{1,2,…}.{1,2,…}.{1,2,…}.

Any one of them leaves the door open for abuse, as does even limiting them all but ignoring interactions (e.g., imagine being "constrained" to 10k descendant keys with up to 1 MB of value data apiece). So this PR effectively takes the position that a chain storage node is a powerful capability to be closely guarded and issued only to trustworthy entities (quite possibly with additional limitations), with the key segment length constraint present basically because it was trivial to add alongside character validity checks. Which (if any) of the others do you think should be added?

michaelfig · 2022-05-24T01:04:39Z

I have a slight preference for "chain storage" over "chain publisher"

I agree with you. The chain storage is fundamentally a store of the current state, and the events are just gossip to help inform clients when that state is updated. It doesn't look like Cosmos events are going to be in consensus anytime soon (and indeed, future versions of Tendermint are making them more performant but less reliable), but the IAVL tree to which we publish has good support for light client proofs, which is a feature I really think we need to lean into (even if there are a few options as to where on the trusting vs. interactive spectrum the client* is wrt. its RPC nodes).

the client: ag-solo, off-chain wallet, data scrounger, Exo plugin, etc

dckc · 2022-05-26T14:34:48Z

packages/vats/test/test-lib-chainStorage.js

-  ]);
+  /** @type { Map<string, any> } */
+  const nonStrings = new Map();
+  nonStrings.set('number', 1);


why the change from new Map(...contents...) to using .set()?

Appeasing TypeScript's complaint about the former: https://github.com/Agoric/agoric-sdk/runs/6563310401?check_suite_focus=true#step:4:260

error TS2769: No overload matches this call.

Ah, but new Map(Object.entries(…)) works: 4afe517

agoric-sdk/packages/vats/test/test-lib-chainStorage.js

Lines 35 to 50 in 4afe517

const nonStrings = new Map(

Object.entries({

number: 1,

bigint: 1n,

boolean: true,

null: null,

undefined,

symbol: Symbol('foo'),

array: ['foo'],

object: {

toString() {

return 'foo';

},

},

}),

);

dckc · 2022-05-26T18:22:50Z

packages/vats/test/test-lib-chainStorage.js

-    },
-  });
+  const nonStrings = new Map(
+    Object.entries({


Cute. That's a keeper.

gibson042

Per discussion with @michaelfig

gibson042 · 2022-05-31T19:47:11Z

packages/vats/src/lib-chainStorage.js

+ * @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) {


Rename all use of "key" with "path".

gibson042 · 2022-05-31T20:04:42Z

packages/vats/src/lib-chainStorage.js

+      getKey() {
+        return key;
+      },


Suggested change

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: "swingset", storeSubkey: `swingset/data:${path}` } ;

},

is that colon a third separator character, visible in the RPC queries? or is it only used internally?

warner · 2022-06-01T15:23:29Z

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';


hm, if packages/cosmic-swingset has a dependencies on packages/vats , chain-storage-paths.js might need to move into packages/vats to avoid a circular dependency

warner · 2022-06-01T15:26:14Z

I have a slight preference for "chain storage" over "chain publisher", because adding read support in the future seems likely (and I have already confirmed read-after-write consistency locally).

Ok, groovy.

I have also added support for - in key segments, in addition to _.

Thanks.

I also have my doubts that the RPC mapping logic will correctly handle storage paths with /, either raw (e.g. /swingset/storage/data/foo/bar/baz) or percent-encoded (/swingset/storage/data/foo%2Fbar%2Fbaz), because it seems very naïve.

I'm ok with whatever works, but it seems like a wart (and a security footgun) to have two different kinds of separators in the same string. I guess I don't really understand how the RPC query parser/router interacts with the IAVL tree. Maybe the IAVL "tree" is really a flat list of string keys, and the only structure to it comes from the semi-arbitrary partitioning of leaves into the various branches of the Merkle tree. If that's the case, IAVL doesn't care what separators you use, so the only semantics are imposed by the RPC query parser. I guess I imagined that the Go side would route /swingset/storage/data/foo/bar/baz to the /swingset/storage/data handler, which could then parse the remaining path to figure out which specific node was being read.

So this PR effectively takes the position that a chain storage node is a powerful capability to be closely guarded and issued only to trustworthy entities (quite possibly with additional limitations), with the key segment length constraint present basically because it was trivial to add alongside character validity checks.

Yeah, I'm good with that for MN-1. Of course we must be more defensive shortly afterwards. It feels like these usage/capacity limits overlap a lot with metering fees (in a general strategy of "you can use as much as you're willing to pay for"), so we should probably find a way to unify this with e.g. paying for virtual-collection DB storage, or paying for RAM footprint over time. This might drive some discussion around one vat charging a different vat for service: the new+separate change-storage handler vat might get full control over the IAVL store (and not be charged anything by the kernel), but it might demand payment from the clients who are holding a Presence for one of its storage nodes.

That said, short-term fixed limits to protect against abuse is good policy, even in MN-1. Anything which doesn't require a lot of design work.

So yeah, let's land this as-is, but make a separate issue to consider imposing additional (easy) limits for MN-1.

warner · 2022-06-01T15:30:27Z

packages/vats/src/lib-chainStorage.js

+      },
+      async delete() {
+        assert(key !== rootKey);
+        // A 'set' with no value deletes a key if it has no children, but


I fully agree with retaining an empty parent to avoid orphans, but it occurred to me this leaves us no good way to delete the parent later. I don't really want to spend a lot of time on deletion, it seems uncommon for now, but eventually maybe each time we manage to delete a node, we should check the parent and delete it if already empty and childless, and recurse upwards.

The x/swingset implementation already does that.

warner · 2022-06-01T15:31:32Z

packages/vats/src/vat-chainStorage.js

+
+export function buildRootObject(_vatPowers) {
+  function makeBridgedChainStorageRoot(bridgeManager, bridgeId, rootKey) {
+    // XXX: Should we validate uniqueness of rootKey, or is that an external concern?


my hunch is that it can be an external concern, although I suppose the real question is how much damage/confusion would be caused by duplicate nodes that use the same rootKey

warner

Ok, @michaelfig convinced me to be ok with the path names.

Approved, conditional on fixing the CI failure (which probably relates to the lint warnings that github is showing, and might require moving the file full of constants into a different package)

Fixes #4558

https://github.com/Agoric/agoric-sdk/runs/6728088019?check_suite_focus=true

dckc · 2022-06-03T19:32:59Z

packages/vats/src/core/chain-behaviors.js

+    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


copy 1 or 2 of the most relevant lines, please? ci log URLs have a limited lifetime.

Suggested change

// https://github.com/Agoric/agoric-sdk/runs/6728088019?check_suite_focus=true

// Error#1: historical inaccuracy in replay of v2 [vatAdmin]

// https://github.com/Agoric/agoric-sdk/runs/6728088019?check_suite_focus=true

…orage

mhofman · 2022-06-04T04:45:16Z

Apparently the chainStorage bundle fails to be installed properly during the loadgen test, which further triggers a bug in the transcript replay (which I'm still in the process of characterizing). Here is the relevant extract from the slog:

{"time":1654270285.0884733,"type":"deliver","crankNum":67,"vatID":"v2","deliveryNum":16,"replay":false,"kd":["message","ko26",{"methargs":{"body":"[\"createVatByName\",[\"chainStorage\"]]","slots":[]},"result":"kp50"}],"vd":["message","o+10",{"methargs":{"body":"[\"createVatByName\",[\"chainStorage\"]]","slots":[]},"result":"p-67"}]}
{"time":1654270285.091672,"type":"syscall","crankNum":67,"vatID":"v2","deliveryNum":16,"syscallNum":0,"replay":false,"ksc":["invoke","kd33","getNamedBundleCap",{"body":"[\"chainStorage\"]","slots":[]}],"vsc":["callNow","d-70","getNamedBundleCap",{"body":"[\"chainStorage\"]","slots":[]}]}
{"time":1654270285.093063,"type":"syscall-result","crankNum":67,"vatID":"v2","deliveryNum":16,"syscallNum":0,"replay":false,"ksr":["error","device.invoke failed, see logs for details"],"vsr":["error","device.invoke failed, see logs for details"]}
{"time":1654270285.0946267,"type":"console","source":"liveslots","state":"delivery","crankNum":67,"vatID":"v2","deliveryNum":16,"level":"info","args":["Logging sent error stack","(Error#1)"]}
{"time":1654270285.0956295,"type":"console","source":"liveslots","state":"delivery","crankNum":67,"vatID":"v2","deliveryNum":16,"level":"info","args":["Error#1:","syscall.callNow failed: device.invoke failed, see logs for details"]}
{"time":1654270285.096555,"type":"console","source":"liveslots","state":"delivery","crankNum":67,"vatID":"v2","deliveryNum":16,"level":"info","args":["Error: syscall.callNow failed: device.invoke failed, see logs for details\n at apply ()\n at Error (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/lockdown-subprocess-xsnap.js:5941)\n at doSyscall (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/supervisor-subprocess-xsnap.js:17092)\n at (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/supervisor-subprocess-xsnap.js:16375)\n at createVatByName (.../swingset-vat/src/vats/vat-admin/vat-vat-admin.js:278)\n at apply ()\n at apply ()\n at dispatchToHandler (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/lockdown-subprocess-xsnap.js:8729)\n at (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/lockdown-subprocess-xsnap.js:8362)\n at win (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/lockdown-subprocess-xsnap.js:9072)\n at ()\n"]}
{"time":1654270285.0975504,"type":"console","source":"liveslots","state":"delivery","crankNum":67,"vatID":"v2","deliveryNum":16,"level":"info","args":["Error#1 ERROR_NOTE:","Thrown from:","(Error#2)",":","8",".","0"]}
{"time":1654270285.098416,"type":"console","source":"liveslots","state":"delivery","crankNum":67,"vatID":"v2","deliveryNum":16,"level":"info","args":["Error#1 ERROR_NOTE:","Sent as","error:liveSlots:v2#70001"]}
{"time":1654270285.0993016,"type":"console","source":"liveslots","state":"delivery","crankNum":67,"vatID":"v2","deliveryNum":16,"level":"info","args":["Error#2:","Event: 7.1"]}
{"time":1654270285.1002047,"type":"console","source":"liveslots","state":"delivery","crankNum":67,"vatID":"v2","deliveryNum":16,"level":"info","args":["Error: Event: 7.1\n at construct ()\n at Error (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/lockdown-subprocess-xsnap.js:5943)\n at trackTurns (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/lockdown-subprocess-xsnap.js:8346)\n at handle (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/lockdown-subprocess-xsnap.js:9063)\n at deliver (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/supervisor-subprocess-xsnap.js:16444)\n at dispatchToUserspace (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/supervisor-subprocess-xsnap.js:16755)\n at runWithoutMetering (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/supervisor-subprocess-xsnap.js:17208)\n at ()\n"]}

Unfortunately the loadgen job doesn't currently have debug output enabled so this didn't show up in the stdout capture. I'm re-running it with it enabled.

mhofman · 2022-06-04T05:18:57Z

Here is a probably relevant snippet from the log output:

2022-06-04T05:01:07.377Z SwingSet: kernel: dm.invoke failed, informing calling vat (Error#1)
Error#1: unregistered bundle name 'chainStorage'

  at Object.invoke (.../swingset-vat/src/devices/vat-admin/device-vat-admin.js:256:7)
  at Object.invoke (.../swingset-vat/src/kernel/deviceManager.js:97:30)
  at invoke (.../swingset-vat/src/kernel/kernelSyscall.js:247:22)
  at kernelSyscallHandler (.../swingset-vat/src/kernel/kernelSyscall.js:338:8)
  at vatSyscallHandler (.../swingset-vat/src/kernel/kernel.js:1233:6)
  at Object.syscallFromWorker (.../swingset-vat/src/kernel/vat-loader/manager-helper.js:253:12)
  at handleUpstream (.../swingset-vat/src/kernel/vat-loader/manager-subprocess-xsnap.js:87:11)
  at handleCommand (.../swingset-vat/src/kernel/vat-loader/manager-subprocess-xsnap.js:110:14)
  at handleCommand (packages/xsnap/src/replay.js:111:26)
  at runToIdle (packages/xsnap/src/xsnap.js:198:42)
  at runMicrotasks (<anonymous>)

2022-06-04T05:01:07.385Z SwingSet: ls: v2: Logging sent error stack (Error#1)
2022-06-04T05:01:07.387Z SwingSet: ls: v2: Error#1: syscall.callNow failed: device.invoke failed, see logs for details
2022-06-04T05:01:07.415Z SwingSet: ls: v2: Error: syscall.callNow failed: device.invoke failed, see logs for details
 at apply ()
 at Error (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/lockdown-subprocess-xsnap.js:5941)
 at doSyscall (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/supervisor-subprocess-xsnap.js:17092)
 at (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/supervisor-subprocess-xsnap.js:16375)
 at createVatByName (.../swingset-vat/src/vats/vat-admin/vat-vat-admin.js:278)
 at apply ()
 at apply ()
 at dispatchToHandler (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/lockdown-subprocess-xsnap.js:8729)
 at (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/lockdown-subprocess-xsnap.js:8362)
 at win (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/lockdown-subprocess-xsnap.js:9072)
 at ()

2022-06-04T05:01:07.419Z SwingSet: ls: v2: Error#1 ERROR_NOTE: Thrown from: (Error#2) : 8 . 0
2022-06-04T05:01:07.420Z SwingSet: ls: v2: Error#1 ERROR_NOTE: Sent as error:liveSlots:v2#70001
2022-06-04T05:01:07.428Z SwingSet: ls: v2: Error#2: Event: 7.1
2022-06-04T05:01:07.429Z SwingSet: ls: v2: Error: Event: 7.1
 at construct ()
 at Error (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/lockdown-subprocess-xsnap.js:5943)
 at trackTurns (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/lockdown-subprocess-xsnap.js:8346)
 at handle (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/lockdown-subprocess-xsnap.js:9063)
 at deliver (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/supervisor-subprocess-xsnap.js:16444)
 at dispatchToUserspace (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/supervisor-subprocess-xsnap.js:16755)
 at runWithoutMetering (/usr/src/agoric-sdk/packages/SwingSet/src/supervisors/subprocess-xsnap/supervisor-subprocess-xsnap.js:17208)
 at ()

michaelfig · 2022-06-04T05:39:43Z

Here is a probably relevant snippet from the log output:

Thank you so much! That's what I was missing.

mhofman · 2022-06-04T06:59:52Z

Looks like it's green now! I'll let you re-enable automerge (which I had disabled because the error needed addressing)

gibson042 · 2022-06-04T10:40:20Z

Thanks!

gibson042 added the cosmic-swingset package: cosmic-swingset label May 17, 2022

gibson042 self-assigned this May 17, 2022

gibson042 commented May 17, 2022

View reviewed changes

gibson042 requested review from dckc, michaelfig and warner May 17, 2022 18:39

michaelfig reviewed May 18, 2022

View reviewed changes

warner reviewed May 19, 2022

View reviewed changes

gibson042 force-pushed the gibson-4558-chainStorage branch from c171175 to 8cfd6f5 Compare May 23, 2022 20:27

gibson042 mentioned this pull request May 25, 2022

feat: Add a helper for publishing iteration results to chain storage #5432

Closed

dckc reviewed May 26, 2022

View reviewed changes

packages/vats/test/test-lib-chainStorage.js

},

});

const nonStrings = new Map(

Object.entries({

Copy link

Member

dckc May 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cute. That's a keeper.

gibson042 requested a review from warner May 31, 2022 17:42

gibson042 force-pushed the gibson-4558-chainStorage branch from 4afe517 to 3229474 Compare May 31, 2022 17:53

gibson042 commented May 31, 2022

View reviewed changes

michaelfig mentioned this pull request Jun 1, 2022

feat(casting): @agoric/casting MVP #5487

Merged

warner reviewed Jun 1, 2022

View reviewed changes

warner self-requested a review June 1, 2022 17:57

warner approved these changes Jun 1, 2022

View reviewed changes

gibson042 added the automerge:squash Automatically squash merge label Jun 2, 2022

gibson042 added 5 commits June 3, 2022 11:08

feat(cosmic-swingset): Add chainStorage interface

028e7b6

Fixes #4558

refactor(cosmic-swingset): Move makeChainStorageNode into a vat

487f208

chore(cosmic-swingset): Respond to PR feedback

fcbd424

refactor(cosmic-swingset): Move chainStorage logic into a library

7e45396

chore: Improve child key checks and explanations

61e05d1

gibson042 added 8 commits June 3, 2022 11:08

chore: Allow "-" in chain storage key segments

c40cc45

chore: Try to appease TypeScript

53d9543

chore: Try harder to appease TypeScript

02ba962

style: Use a better TypeScript-compatible map initialization

db67d1e

refactor: Create Go and JS constants for top-level chain storage paths

ac504dd

refactor: Replace chain storage "key" with "path"

9898385

refactor: Move chain-storage-paths.js to avoid a module cycle

265cd52

chore: Appease eslint

69d8d4f

gibson042 force-pushed the gibson-4558-chainStorage branch from 8e88f2f to 69d8d4f Compare June 3, 2022 15:09

fix: Try adding a delay to fix CI

3c7c270

https://github.com/Agoric/agoric-sdk/runs/6728088019?check_suite_focus=true

dckc reviewed Jun 3, 2022

View reviewed changes

chore: Include relevant lines from failing CI log

e718f25

gibson042 force-pushed the gibson-4558-chainStorage branch from c2df386 to e718f25 Compare June 3, 2022 20:14

mergify bot and others added 3 commits June 3, 2022 20:32

Merge branch 'master' into gibson-4558-chainStorage

fbd45fb

fix(swingset): louder anachrophobio

af35399

Merge remote-tracking branch 'origin/master' into gibson-4558-chainst…

14612b2

…orage

mhofman added force:integration Force integration tests to run on PR and removed automerge:squash Automatically squash merge labels Jun 4, 2022

mhofman mentioned this pull request Jun 4, 2022

Vat transcript replay does not handle failed syscalls #5511

Closed

michaelfig force-pushed the gibson-4558-chainStorage branch from 62f24b8 to f182c61 Compare June 4, 2022 05:57

ci(vats): correct chainStorage bundle in decentral configs

f22b8f8

michaelfig force-pushed the gibson-4558-chainStorage branch from f182c61 to f22b8f8 Compare June 4, 2022 05:59

gibson042 added the automerge:squash Automatically squash merge label Jun 4, 2022

mergify bot merged commit 109ff65 into master Jun 4, 2022

mergify bot deleted the gibson-4558-chainStorage branch June 4, 2022 10:41

		const rootNode = makeChainStorageNode(ROOT_KEY);
		chainStorageP.resolve(rootNode);

	const nonStrings = new Map(
	Object.entries({
	number: 1,
	bigint: 1n,
	boolean: true,
	null: null,
	undefined,
	symbol: Symbol('foo'),
	array: ['foo'],
	object: {
	toString() {
	return 'foo';
	},
	},
	}),
	);

-      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: "swingset", storeSubkey: `swingset/data:${path}` } ;
+      },

	// https://github.com/Agoric/agoric-sdk/runs/6728088019?check_suite_focus=true
	// Error#1: historical inaccuracy in replay of v2 [vatAdmin]
	// https://github.com/Agoric/agoric-sdk/runs/6728088019?check_suite_focus=true

feat(cosmic-swingset): Add chainStorage interface #5385

feat(cosmic-swingset): Add chainStorage interface #5385

Conversation

gibson042 commented May 17, 2022 • edited Loading

Description

Security Considerations

Documentation Considerations

Testing Considerations

gibson042 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelfig left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gibson042 May 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

warner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gibson042 commented May 23, 2022

michaelfig commented May 24, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gibson042 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

warner Jun 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

warner commented Jun 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

warner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhofman commented Jun 4, 2022

mhofman commented Jun 4, 2022

michaelfig commented Jun 4, 2022

mhofman commented Jun 4, 2022

gibson042 commented Jun 4, 2022

gibson042 commented May 17, 2022 •

edited

Loading

gibson042 May 18, 2022 •

edited

Loading

warner Jun 1, 2022 •

edited

Loading