Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merkle-subtree-writing objects #4558

Closed
Tracked by #5354
warner opened this issue Feb 16, 2022 · 2 comments · Fixed by #5385
Closed
Tracked by #5354

merkle-subtree-writing objects #4558

warner opened this issue Feb 16, 2022 · 2 comments · Fixed by #5385
Assignees
Labels
cosmic-swingset package: cosmic-swingset enhancement New feature or request read-no-tx topic: reading from the chain without a transaction SwingSet package: SwingSet
Milestone

Comments

@warner
Copy link
Member

warner commented Feb 16, 2022

(not sure what label to apply here, it overlaps swingset and cosmic-swingset, plus using it for notification purposes touches a bunch of core vats)

What is the Problem Being Solved?

#3756 wants a cheap way for a contract vat to write to some portion of the chain's externally-queriable (and provable) state space. We know we want the Notifier's updater API to drive this, and it would be nice to integrate in with the notifier half. We know there probably needs to be a device node involved, to execute the write. To make this as cheap as possible, we'd like the device write to happen from the same vat that hosts the updater (rather than having a separate "chain state writer service" vat which follows the notifier and reacts to updates by invoking the device.

In today's meeting, we discussed having a device nodes that represent specific paths within the chain's state vector. One device node might manage /swingset/state/amm/pool1. Another might manage the parent node: /swingset/state/amm. Each node gives the holder four authorities:

  • write data to that portion of the state vector
  • delete data from that portion of the state vector
  • create a child device node with a path extension
  • report the path of the device node

cosmic-swingset would create the device and give it the ability to write into the state vector (and simultaneously add an Event to the current block, with the same update). Then bootstrap would get this device node, and create child nodes to hand off to each vat. The AMM vat would receive /swingset/state/amm, and could dole out subsets as it sees fit.

For efficiency, we might use fewer device nodes and provide an API which allows writes to arbitrary child paths beneath the device node's starting point. Then plain JS wrapper objects within the vat would provide the appropriate division of authority. You'd only make a new device node when sending it to a new vat.

In this approach, the kernel is not deeply involved. The device would probably be defined in packages/cosmic-swingset/, and the chain-side bootstrap would perform the subdivision. The sim-chain bootstrap would need to create some simulated equivalent.

I'll write up an alternate approach in #4559, in which the kernel is deeply aware of this new IO channel, and a device is not used.

cc @gibson042 @michaelfig @dtribble @erights

Description of the Design

  • cosmic-swingset defines a device
  • it receives three endowments:
    • getRootPath() -> string
    • write(path: string, data: string)
    • delete(path: string)
  • each device node encapsulates a path
  • the root device node encapsulates the root path
  • each device node has an API:
    • write(data: string)
    • delete()
    • getPath() -> string
    • getChild(name: string) -> deviceNode
    • (maybe) writeSubpath(subpath: string, data: string)
    • (maybe) deleteSubpath(subpath: string)
    • (maybe) getSubpath(subpath: string) -> string

The device wants to be a "raw device" (#1346), since it will be creating and exporting a lot of device nodes, and deviceSlots doesn't handle this very well. We want the device to store the dref <-> subpath mapping in the vatstore. Use devices.bundle as a template.

Questions:

  • hierarchy is great, but is the tree dense or sparse? If I invoke delete() on a node that encapsulates /foo, does that also delete anything previously written to /foo/bar and /foo/baz? Or do we define a tree, but the individual vertices can each either have data or not? I expect the latter.
  • to avoid concerns about separator injection, my inclination is for the API to work with arrays of individual path components, in which the / separator character is forbidden, and only join them into full strings at the last moment. So writeSubpath would be writeSubpath(['bar', 'baz'], data) instead of writeSubpath('bar/baz', data). getPath() might return /swingset/foo/bar/baz or it might return ['swingset', 'foo', 'bar', 'baz'] and leave it to the caller to do the join.

Security Considerations

  • Separator injection/confusion could allow the authority hierarchy to be violated.
  • device nodes are not GCed yet, so the ability to pass them around might allow a storage/clist-consumption attack

Test Plan

  • swingset-based tests in cosmic-swingset or packages/vats
@Tartuffo
Copy link
Contributor

Moving this to MN-1, since this is key for our off-chain read impl.

@Tartuffo Tartuffo added this to the Mainnet 1 milestone Mar 23, 2022
@warner
Copy link
Member Author

warner commented Apr 13, 2022

We're using objects for this, instead of device nodes, to reduce the amount of low-level code that must be written.

@warner warner changed the title merkle-subtree-writing device nodes merkle-subtree-writing objects Apr 13, 2022
@dckc dckc added the read-no-tx topic: reading from the chain without a transaction label May 31, 2022
@mergify mergify bot closed this as completed in #5385 Jun 4, 2022
mergify bot added a commit that referenced this issue Jun 4, 2022
* feat(cosmic-swingset): Add chainStorage interface

Fixes #4558

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

* chore(cosmic-swingset): Respond to PR feedback

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

* chore: Improve child key checks and explanations

* test: Add tests for chainStorage

* chore: Allow "-" in chain storage key segments

* chore: Try to appease TypeScript

* chore: Try harder to appease TypeScript

* style: Use a better TypeScript-compatible map initialization

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

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

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

* chore: Appease eslint

* fix: Try adding a delay to fix CI

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

* chore: Include relevant lines from failing CI log

* fix(swingset): louder anachrophobio

* ci(vats): correct `chainStorage` bundle in decentral configs

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Michael FIG <mfig@agoric.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cosmic-swingset package: cosmic-swingset enhancement New feature or request read-no-tx topic: reading from the chain without a transaction SwingSet package: SwingSet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants