Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
327 stars 206 forks source link

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

Open warner opened 2 years ago

warner commented 2 years ago

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:

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:

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 commented 2 years ago

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 commented 2 years ago

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:

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 commented 2 years ago

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