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.

Sign up for GitHub

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

Jump to bottom

maybe merge devices.bundle and devices.vatAdmin #4566

Closed
warner opened this issue Feb 16, 2022 · 0 comments · Fixed by #4590
Closed

maybe merge devices.bundle and devices.vatAdmin #4566

warner opened this issue Feb 16, 2022 · 0 comments · Fixed by #4590
Assignees
@warner
Labels
enhancement New feature or request SwingSet package: SwingSet
Milestone
Mainnet 1 RC0

Comments

@warner
Copy link
Member

warner commented Feb 16, 2022 •
edited
Loading

What is the Problem Being Solved?

devices.bundle was added to provide bundlecaps, which are consumed by vats.vatAdmin. While changing a lot of external code to accomodate the new device, I'm coming to the conclusion that exposing devices.bundle as a separate device is kind of a hassle, and that the functionality is sufficiently intertwined with devices.vatAdmin that maybe they should be merged.

On the plus side:

  • bootstrap() would not need to change to accomodate vatAdmin's need for devices.bundle (i.e. we get vatAdminService = E(vats.vatAdmin).createVatAdminService(devices.vatAdmin), as before, instead of the new createVatAdminService(devices.vatAdmin, devices.bundle))
  • swingset unit tests don't need to accomodate the extra cranks and device-ID allocations
  • (both of these are sunk costs, I have a branch 4521-use-vat-admin with these changes)
  • the overall system is simpler and easier to explain

On the negative side:

  • devices.bundle is a raw device, but devices.vatAdmin is not (yet). We would have to rewrite devices.vatAdmin to be a raw device to make this work. OTOH, vatAdmin-src.js is really simple: 8 methods that just pass straight through to the endowments, with no internal state.

Description of the Design

Security Considerations

Test Plan
@warner warner added enhancement New feature or request SwingSet package: SwingSet labels Feb 16, 2022
@warner warner self-assigned this Feb 16, 2022
@warner warner mentioned this issue Feb 17, 2022
Closed
warner added a commit that referenced this issue Feb 17, 2022
@warner
fix(swingset): merge bundle and vat-admin devices
4c56b87 
Simplify the user experience by removing `devices.bundle` and merging its
functionality into vat-admin.

closes #4566
warner added a commit that referenced this issue Feb 18, 2022
@warner
fix(swingset): merge bundle and vat-admin devices
668fcea 
Simplify the user experience by removing `devices.bundle` and merging its
functionality into vat-admin.

* bootstrap() can go back to doing `vatAdminService =
E(vats.vatAdmin).createVatAdminService(devices.vatAdmin)` ,
instead of `createVatAdminService(devices.vatAdmin, devices.bundle)`
* you can get bundlecaps from `E(vatAdminService).getBundlecap` or
`.getNamedBundlecap`, you no longer need `D(devices.bundle)` for those
* `E(vatAdminService).waitForBundlecap(bundleID)` gives you a Promise that
fires when (and if) the bundle has been installed, so you can e.g. begin a
`zoe.install()` before the bundle is installed (closes #4521)

closes #4566
warner added a commit that referenced this issue Feb 18, 2022
@warner
fix(swingset): merge bundle/vat-admin devices, add waitForBundlecap()
df14366 
Simplify the user experience by removing `devices.bundle` and merging its
functionality into vat-admin.

* bootstrap() can go back to doing `vatAdminService =
E(vats.vatAdmin).createVatAdminService(devices.vatAdmin)` ,
instead of `createVatAdminService(devices.vatAdmin, devices.bundle)`
* you can get bundlecaps from `E(vatAdminService).getBundlecap` or
`.getNamedBundlecap`, you no longer need `D(devices.bundle)` for those
* `E(vatAdminService).waitForBundlecap(bundleID)` gives you a Promise that
fires when (and if) the bundle has been installed, so you can e.g. begin a
`zoe.install()` before the bundle is installed (closes #4521)

closes #4566
@warner warner mentioned this issue Feb 18, 2022
Merged
turadg pushed a commit that referenced this issue Feb 21, 2022
@warner
fix(swingset): merge bundle/vat-admin devices, add waitForBundlecap()
274f824 
Simplify the user experience by removing `devices.bundle` and merging its
functionality into vat-admin.

* bootstrap() can go back to doing `vatAdminService =
E(vats.vatAdmin).createVatAdminService(devices.vatAdmin)` ,
instead of `createVatAdminService(devices.vatAdmin, devices.bundle)`
* you can get bundlecaps from `E(vatAdminService).getBundlecap` or
`.getNamedBundlecap`, you no longer need `D(devices.bundle)` for those
* `E(vatAdminService).waitForBundlecap(bundleID)` gives you a Promise that
fires when (and if) the bundle has been installed, so you can e.g. begin a
`zoe.install()` before the bundle is installed (closes #4521)

closes #4566
warner added a commit that referenced this issue Feb 25, 2022
@warner
fix(swingset): merge bundle/vat-admin devices, add waitForBundleCap()
019ef20 
Simplify the user experience by removing `devices.bundle` and merging its
functionality into vat-admin. Use mid-capitalized "bundleCap" in the
API (e.g. `getNamedBundleCap` instead of `getNamedBundlecap`).

* bootstrap() can go back to doing `vatAdminService =
E(vats.vatAdmin).createVatAdminService(devices.vatAdmin)` ,
instead of `createVatAdminService(devices.vatAdmin, devices.bundle)`
* you can get bundlecaps from `E(vatAdminService).getBundleCap` or
`.getNamedBundleCap`, you no longer need `D(devices.bundle)` for those
* `E(vatAdminService).waitForBundleCap(bundleID)` gives you a Promise that
fires when (and if) the bundle has been installed, so you can e.g. begin a
`zoe.install()` before the bundle is installed (closes #4521)

This also introduces a better way to export types from swingset, defines a
VatAdminRootDeviceNode type for use by clients who think they need that, and
exports a few ancillary types to support it.

closes #4566
@Tartuffo Tartuffo added this to the Mainnet 1 milestone Mar 23, 2022
@warner warner mentioned this issue Apr 5, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@warner warner

Labels
enhancement New feature or request SwingSet package: SwingSet
Projects
None yet
Milestone
Mainnet 1 RC0
Development

Successfully merging a pull request may close this issue.

merge bundle/vat-admin devices, add waitForBundlecap() Agoric/agoric-sdk
2 participants
@warner @Tartuffo