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

kernel should automatically connect its vat/devices at startup #4523

Open
warner opened this issue Feb 10, 2022 · 3 comments
Open

kernel should automatically connect its vat/devices at startup #4523

warner opened this issue Feb 10, 2022 · 3 comments
Assignees
Labels
enhancement New feature or request SwingSet package: SwingSet

Comments

@warner
Copy link
Member

warner commented Feb 10, 2022

What is the Problem Being Solved?

One of the motivations for #1346 was the awkwardness of having both a device (which gets endowments, but is awkward to work with) and a companion vat (which provides a nice API, but lacks endowments). We do this for a couple services:

service device (outer) device (inner) vat
timer src/devices/timer.js src/devices/timer-src.js src/vats/vat-timerWrapper.js
plugin src/devices/plugin.js src/devices/plugin-src.js src/vats/plugin-manager.js
vat-admin src/kernel/vatAdmin/vatAdmin-src.js src/kernel/vatAdmin/vatAdminWrapper.js

#4521 ("bundle is installed" promise) could be implemented either with a companion vats.bundles or by adding promise support to raw devices. I'm kind of in favor of the companion vat, at the moment, because I think I can hide the device entirely, reducing the cognitive burden on bundle users. Bundlecaps would still be device nodes, but you'd use E(vats.bundles).getBundlecap(bundleID) to get one (which could then trivially return a Promise that fires later, after installation). The one case where you'd need D() is to get synchronous access to the bundle contents, and in the long run we want userspace to stop doing that anyways.

But adding a companion vat, in all cases so far, has also imposed a burden on bootstrap() to wire it up. Every swingset that uses a timer must do:

    const chainTimerServiceP = E(vats.timer).createTimerService(devices.timer);

(#4492 also complains about this).

Swingsets that want to use vat-admin and dynamic vats must do:

      const vatAdminSvc = await E(vats.vatAdmin).createVatAdminService(
        devices.vatAdmin,
      );

To use comms and vattp and a mailbox, you need:

      D(devices.mailbox).registerInboundHandler(vats.vattp);
      await E(vats.vattp).registerMailboxDevice(devices.mailbox);

plus a pair of vats.comms / vats.vattp messages for each remote system.

It would be great if all of these obligations could go away, and the swingset itself could be responsible for the standard cross-wiring of components that live in the swingset source tree anyways.

Description of the Design

I'm thinking that config.vats.NAME.initialize= becomes an indicator that the given static vat wants to have access to some list of devices and/or other vats. So initializeSwingset() could set:

config.vats.vatAdmin.initialize = { devices: ['vatAdmin'] };
config.vats.timer.initialize = { devices: ['timer'] };
config.vats.vattp.initialize = { devices: ['mailbox'] };

(and maybe also accept vats: for cross-vat connections).

Then during initializeKernel() where we parse config.vats and config.devices, we'd react to .initialize by pushing a run-queue message that will be delivered before bootstrap(), the equivalent of:

  • E(vats.vatAdmin).initialize({ devices: { vatAdmin } })
  • E(vats.timer).initialize({ devices: { timer } })
  • E(vats.vattp).initialize({ devices: { mailbox } })
  • (and { devices, vats } if you included any vats in the config statement)

When #2910 lands, we'll have an additional start-vat run-queue event for each static vat, which must execute before any messages can be delivered to that vat. The complete sequence of pre-queued events will then be:

  • start-vat(vat-bootstrap)
  • start-vat(vat-admin)
  • ..
  • deliver(vat-admin, initialize, { devices: { vatAdmin } })
  • ..
  • deliver(vat-bootstrap, bootstrap, { vats, devices })

Alternatives

We could simplify the config syntax by making it coarser: config.vats.NAME.initializeDevices is a boolean flag that provides all devices, with a boot-time E(vats.NAME).initializeDevices(devices). But if we do end up adding cross-vat links, that would give all initialized vats access to all other vats, which seems like too much authority.

What This Simplifies

User-written bootstrap vats could remove some annoying boilerplate that exposes kernel/device internals they really shouldn't have to be aware of.

Adding a companion vat (e.g. vats.bundles) is cheaper, and doesn't increase userspace requirements.

Security Considerations

The config object defines the initial vat configuration, including the definition of the bootstrap vat, so it already has full authority over everything that happens. It's no authority leak to allow it to share device authority to whatever vats it wants.

The vat root object which receives initialize() is the same one that gets exposed to bootstrap(). The bootstrap vat is ultimately trusted, but still it's a bit weird to realize that bootstrap is also in a position to send the same initialize() and cause problems. And you probably wouldn't want to expose a vat root object with .initialize() to any other vat. We already perform this separation with e.g. vatAdminService = await E(vats.vatAdmin).stuff(..), where vatAdminService !== vats.vatAdmin. So to get this attenuation and share a non-.initialize()-bearing object with other code, you'd still need some boilerplate during bootstrap(). I can't think of a great way to improve this, short of changing buildRootObject() entirely and allow it to return multiple objects, one for initialize(), one for bootstrap vats.NAME.

It'd probably be a good idea for initialize() to throw if called more than once.

Test Plan

unit tests

cc @FUDCo what do you think?

@warner warner added enhancement New feature or request SwingSet package: SwingSet labels Feb 10, 2022
@warner warner self-assigned this Feb 10, 2022
@warner
Copy link
Member Author

warner commented Feb 11, 2022

For vatAdminService, we might change the root object to have initialize (called automatically) and getService (called from bootstrap). So then bootstrap would do:

const vatAdminService = await E(vats.vatAdmin).getService();

instead of:

const vatAdminService = await E(vats.vatAdmin).createVatAdminService(devices.vatAdmin, devices.bundle);

@warner
Copy link
Member Author

warner commented Feb 12, 2022

While implementing #4521, I was reminded that the kernel needs to send messages to built-in vats like vatAdmin. The kernel knows their root object, so it sends e.g. newVatCallback and the new bundleInstalled to vatAdmin's root, but that's also what bootstrap gets as vats.vatAdmin (which is another reason vatAdminService !== vats.vatAdmin).

The bootstrap code is pretty powerful, so I don't mind it having access to this thing that really only the kernel should be able to access. But it would be nice if we could separate that out a bit.

I'm wondering if we could do something like:

  • for non-initialize vats, whatever buildRootObject returns is given to bootstrap as vats.name
  • for initialize vats, the kernel gets the buildRootObject for its own purposes, and bootstrap gets an object returned by initialize

Implementing this sounds a bit tricky (we synthesize and queue bootstrap() during initializeSwingset).. I think we'd need to allocate a result promise ID during initializeSwingset, use that for the initialize call, and include it in the bootstrap arguments (changing vats.NAME from Map<name, Remotable> to Map<name, maybePromise<Remotable>>, which sounds like it could break userspace).

Another possibility is to swap the two: the kernel gets exclusive access to an object returned by initialize(), and uses it for internal messages. That seems tricky to implement in a different way: we could allocate the result promise ID during initializeSwingset, but we can't run the cranks at that time, so the kernel would need to pay special attention to those promises (maybe with k.kpStatus / k.kpResolution) and record their resolutions (as object krefs) when they happen. We could establish a convention/requirement that initialize resolves its return promise promptly (a syscall.resolve during the dispatch.deliver('initialize'), to make sure the kernel has what it needs before user code in bootstrap gets a chance to send messages to e.g. vats.vatAdmin, but we still need the kernel-side promise watcher.

@Tartuffo
Copy link
Contributor

De-prioritized for MN-1 in discussion with @warner on Feb 22.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request SwingSet package: SwingSet
Projects
None yet
Development

No branches or pull requests

2 participants