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

handle named bundles in kernel config and core-bootstrap #4374

Open
warner opened this issue Jan 25, 2022 · 11 comments
Open

handle named bundles in kernel config and core-bootstrap #4374

warner opened this issue Jan 25, 2022 · 11 comments
Assignees
Labels
enhancement New feature or request SwingSet package: SwingSet vaults_triage DO NOT USE

Comments

@warner
Copy link
Member

warner commented Jan 25, 2022

What is the Problem Being Solved?

Once we have bundlecaps (#4372), we'd like the new #4165 / #4247 "core bootstrap" scheme to use them too. Currently this relies upon vatAdminService~.createVatByName(name), rather than a bundlecap or bundleID.

That means changing the kernel config format to somehow handle named bundles properly. We want vats to uniformly defined by a bundleID, so that the future upgrade protocol has a well-defined initial value to upgrade from.

The old code processes a config.bundles section which maps bundle names to either a filename (from which bundleSource can create a bundle) or an actual bundle (used only by tests). It then builds a table of named bundles, which vatAdminService~.createVatByName() can reference. @dckc 's

The new approach should process config.bundles and install each bundle, then build a table mapping name to bundleID. The bootstrap() message should be enhanced to provide this table to the bootstrap vat (perhaps through a third argument, so bootstrap(vats, devices, bundles)), which can use devices.bundle to get bundlecaps, and send them to vatAdminService~.createVat(bundlecap).

When we're done, we can remove createVatByName, and createVat() (which exclusively take a bundlecap) will be the only remaining API.

Description of the Design

Security Considerations

Test Plan

@warner warner added enhancement New feature or request SwingSet package: SwingSet labels Jan 25, 2022
@warner warner self-assigned this Jan 25, 2022
@warner warner added the MN-1 label Jan 25, 2022
@Tartuffo Tartuffo removed the MN-1 label Feb 7, 2022
@warner
Copy link
Member Author

warner commented Feb 8, 2022

#4372 takes config.bundles and installs each one, so bootstrap can do bundlecap = D(devices.bundle).getNamedBundleCap(name) and pass that through to whatever does createVat(). I decided to avoid the third bootstrap() argument.

For now, vatAdminService~.createVatByName() still works, so I think I can wait for the new bootstrap code to land before thinking about how to change it to use bundlecaps.

@warner warner changed the title handle named bundles in kernel config handle named bundles in kernel config and core-bootstrap Feb 10, 2022
@Tartuffo Tartuffo added this to the Mainnet 1 milestone Mar 23, 2022
@warner
Copy link
Member Author

warner commented May 13, 2022

The new bootstrap code landed a while ago. It currently grabs the whole bundle and passes it to the old E(zoe).install(bundle) API:

 const bundleCap = D(vatAdmin).getNamedBundleCap(name); 
 const bundle = D(bundleCap).getBundle(); 
 producer.resolve(E(zoe).install(bundle)); 

To avoid holding the whole bundle, we need to give bootstrap a way to get from a name to a bundleID, so it can use E(zoe).installBundleID(id) instead of install(bundle).

This wasn't straightforward when we first landed the bundle-handling code, because IDs (hashes) weren't always present (there was still a lot of placeholder code). But these days, bundleSource should always be providing the hash. So the task is twofold:

  • add E(vatAdminService).getBundleIDByName(name) -> P<BundleID> (or rejects if the name is unknown)
  • change bootstrap to use that and E(zoe).installBundleID(id)

An alternative would be to add E(zoe).installBundleCap(bcap), since bootstrap already has a way to get the named bundlecaps. But it's slightly nicer to keep the bundlecaps out of bootstrap entirely, since we don't yet GC them, so they'll just hang out in the bootstrap vat's c-list forever.

warner added a commit that referenced this issue May 13, 2022
This API takes the name of a statically-configured bundle (i.e. a
property name of `config.bundles`) and returns its BundleID (a hash
string). This can be used later in `E(zoe).installBundleID(id)`.

refs #4374 , specifically this API will allow bootstrap to deal with
bundleIDs and bundleCaps rather than bundles.
warner added a commit that referenced this issue May 26, 2022
This API takes the name of a statically-configured bundle (i.e. a
property name of `config.bundles`) and returns its BundleID (a hash
string). This can be used later in `E(zoe).installBundleID(id)`.

refs #4374 , specifically this API will allow bootstrap to deal with
bundleIDs and bundleCaps rather than bundles.
@Chris-Hibbert
Copy link
Contributor

basicBehaviors.js now has these lines:

    // TODO (#4374) this should be E(zoe).installBundleID(bundleID);
    const installation = E(zoe).install(bundle);

@Tartuffo
Copy link
Contributor

@dckc or @kriskowal please sync with @warner to see if this really is necessary for MN-1, and whether already done.

@dckc
Copy link
Member

dckc commented Jul 20, 2022

My understanding is that all the bundle id / hash bundle stuff is motivated by the goal of setting a max message size around 10M or so; that is: this is blocking #4280 .

@Tartuffo
Copy link
Contributor

We moved #4280 to MN-1.1, so I'm moving this one, too.

@Tartuffo Tartuffo removed this from the Mainnet 1 RC0 milestone Aug 10, 2022
@warner
Copy link
Member Author

warner commented Aug 11, 2022

@dckc and I implemented this today, at least for the statically-installed contract bundles. core-bootstrap installBootContracts is defined with a list of contract names (produce: { centralSupply, mintHolder, walletFactory }), all of which are assumed to be defined in config.bundles. For each one, it asks id = await E(vatAdminSvc).getBundleIDByName(name), and then does E(zoe).installBundleID(id).

That removes the processing of the large contract bundle from both vat-bootstrap and vat-zoe. Zoe converts the BundleID into a BundleCap at installation time, and then sends the BundleCap to ZCF (instead of the full bundle) at instantiation time.

I need to do another pass through basic-behaviors.js > makeVatsFromBundles() to make sure we aren't holding large bundles for the static vats (as opposed to the static contracts that we just fixed). I see teh function is named ..FromBundles, but the function it provides takes bundleName, and calls createVatByName, which I think means it's not handling large bundles anywhere.

Incidentally, when looking at the vat-zoe slogfile for a recent instagoric perf run, I counted 25 bundles being passed to E(zoe).install(). I think three of them were static contracts, fixed here, and the other 22 are from the "install the economy" governance action which doesn't happen during bootstrap, but does happen right away within an instagoric run. Those 22 bundles won't shrink into caps/IDs until we've got client-side bundles-in-txns working.

@warner
Copy link
Member Author

warner commented Aug 11, 2022

Re-reading my initial comments.. to finish this work, and make all the bootstrap-launched pseudo-static vats use bundlecaps instead of bundle names, we need to change makeVatsFromBundles() from:

      const vatInfo = E(svc).createVatByName(bundleName, { name: bundleName });

to something like:

      const bcap = await E(svc).getNamedBundleCap(bundleName);
      const vatInfo = E(svc).createVat(bcap, { name: bundleName });

Two wrinkles:

  • The unit test environment may need to be augmented to support those calls (as it uses a mock VatAdminService)
  • This exposes the bundlecaps to vat-bootstrap, which are device nodes, and we don't GC device nodes yet (some day..), which means vat-bootstrap will hold onto these forever. It already holds onto the new vat root objects forever, and vats hold onto their source bundles (at least until upgrade), so we're not actually retaining any additional storage. And some day we might manage to figure out how to drop device nodes during GC, so the user-level code won't need to change.

This second change doesn't save any RAM, because we aren't holding these full bundles in RAM at any point: both names and bundlecaps are tiny. So we don't really need to implement this any time soon. It's just a blocker for cleaning up vat-admin to stop supporting createVatByName.

@mhofman
Copy link
Member

mhofman commented Jan 31, 2023

Likely a blocker for #6826
cc @gibson042

@warner
Copy link
Member Author

warner commented Jan 31, 2023

Rereading this, I think we need to apply one of the changes explored here (in #4374 (comment)) to address #6826, but I think we don't need to implement the rest of this ticket (which remains low-priority). Having vatAdminService.createVatByName() is a bit of a wart, but it isn't causing any performance or memory problems, and the implementation internally converts the names to BundleIDs before creation, so my original concern ("We want vats to uniformly defined by a bundleID, so that the future upgrade protocol has a well-defined initial value to upgrade from") doesn't seem significant.

warner added a commit that referenced this issue Jan 31, 2023
The core-bootstrap `installBootContracts()` step is responsible for
creating Zoe installations for each "boot contract": contracts that
are included in the initial chain configuration, and instantiated
during bootstrap (currently just `centralSupply` and
`mintHolder`). These contract bundles are specified by config.bundle
entries, which provide a `sourceSpec` that points to the bundle file
on disk.

Previously, this step obtained a full bundle for each contract, in
RAM, and then used `zoe.install(bundle)` to create the Zoe
Installation. When called this way, Zoe must store the full
bundle (about 1MB) in its durable storage, and the bundle appears as a
message argument in lots of places on its way to being evaluated
inside a ZCF vat, which slows things down and consumes more disk
space.

The preferred way to interact with bundles is through BundleCaps. This
commit changes installBootContracts to look up the BundleID of each
boot contract, and submit the ID to `zoe.installBundleID()`. This
allows Zoe to retrieve a (small) BundleCap, and store the bcap in the
installation record. With this approach, the full bundle only appears
once, when the ZCF vat retrieves its contents just prior to
evaluation.

Changing installBootContracts is straightforward, however we have a
number of mock Zoe and mock VatAdminService implementations that
appear during unit tests, and these required changes to accomodate the
new approach. `zoe/tools/fakeVatAdmin.js` provides the primary mock
service, but it is only capable of creating ZCF vats. It was augmented
to understand the full suite of `getBundleIDByName`/etc methods, with
helper tools to populate the simulated tables. Then
`vats/tools/boot-test-utils.js` acquired a wrapper around zoe's mock
service, adding the ability to create non-ZCF vats, and populating the
tables with all the bundles known to the unit tests.

refs #6826
refs #4374
warner added a commit that referenced this issue Jan 31, 2023
The core-bootstrap `installBootContracts()` step is responsible for
creating Zoe installations for each "boot contract": contracts that
are included in the initial chain configuration, and instantiated
during bootstrap (currently just `centralSupply` and
`mintHolder`). These contract bundles are specified by config.bundle
entries, which provide a `sourceSpec` that points to the bundle file
on disk.

Previously, this step obtained a full bundle for each contract, in
RAM, and then used `zoe.install(bundle)` to create the Zoe
Installation. When called this way, Zoe must store the full
bundle (about 1MB) in its durable storage, and the bundle appears as a
message argument in lots of places on its way to being evaluated
inside a ZCF vat, which slows things down and consumes more disk
space.

The preferred way to interact with bundles is through BundleCaps. This
commit changes installBootContracts to look up the BundleID of each
boot contract, and submit the ID to `zoe.installBundleID()`. This
allows Zoe to retrieve a (small) BundleCap, and store the bcap in the
installation record. With this approach, the full bundle only appears
once, when the ZCF vat retrieves its contents just prior to
evaluation.

Changing installBootContracts is straightforward, however we have a
number of mock Zoe and mock VatAdminService implementations that
appear during unit tests, and these required changes to accomodate the
new approach. `zoe/tools/fakeVatAdmin.js` provides the primary mock
service, but it is only capable of creating ZCF vats. It was augmented
to understand the full suite of `getBundleIDByName`/etc methods, with
helper tools to populate the simulated tables. Then
`vats/tools/boot-test-utils.js` acquired a wrapper around zoe's mock
service, adding the ability to create non-ZCF vats, and populating the
tables with all the bundles known to the unit tests.

refs #6826
refs #4374
warner added a commit that referenced this issue Jan 31, 2023
The core-bootstrap `installBootContracts()` step is responsible for
creating Zoe installations for each "boot contract": contracts that
are included in the initial chain configuration, and instantiated
during bootstrap (currently just `centralSupply` and
`mintHolder`). These contract bundles are specified by config.bundle
entries, which provide a `sourceSpec` that points to the bundle file
on disk.

Previously, this step obtained a full bundle for each contract, in
RAM, and then used `zoe.install(bundle)` to create the Zoe
Installation. When called this way, Zoe must store the full
bundle (about 1MB) in its durable storage, and the bundle appears as a
message argument in lots of places on its way to being evaluated
inside a ZCF vat, which slows things down and consumes more disk
space.

The preferred way to interact with bundles is through BundleCaps. This
commit changes installBootContracts to look up the BundleID of each
boot contract, and submit the ID to `zoe.installBundleID()`. This
allows Zoe to retrieve a (small) BundleCap, and store the bcap in the
installation record. With this approach, the full bundle only appears
once, when the ZCF vat retrieves its contents just prior to
evaluation.

Changing installBootContracts is straightforward, however we have a
number of mock Zoe and mock VatAdminService implementations that
appear during unit tests, and these required changes to accomodate the
new approach. `zoe/tools/fakeVatAdmin.js` provides the primary mock
service, but it is only capable of creating ZCF vats. It was augmented
to understand the full suite of `getBundleIDByName`/etc methods, with
helper tools to populate the simulated tables. Then
`vats/tools/boot-test-utils.js` acquired a wrapper around zoe's mock
service, adding the ability to create non-ZCF vats, and populating the
tables with all the bundles known to the unit tests.

refs #6826
refs #4374
@warner
Copy link
Member Author

warner commented Apr 24, 2023

We're going to kick this out out of the release.. I want to survey the code and understand how bundles are now being used, but the release isn't threatened by this not being done yet.

@warner warner removed this from the Vaults EVP milestone Apr 24, 2023
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 vaults_triage DO NOT USE
Projects
None yet
Development

No branches or pull requests

8 participants