Open warner opened 2 years ago
I have a first step for this in the 4487-zoe-bundlecaps branch (look at the last two commits). The description says:
Zoe needs a way to create a new ZCF vat. Inside swingset, zoe uses
vatAdminService
, and this is used from unit tests (zoe and other packages
that use zoe) when they are willing to take the time to run a full swingset
environment.
Unit tests that use zoe, but not swingset, use tools/fakeVatAdmin.js
as a
replacement, which implements createVat(bundle)
but not
createVatByName(name)
. This is used to evaluate the ZCF bundle in a new
Compartment (using importBundle
and evalContractBundle
).
The primary export of @agoric/zoe
is makeZoeKit()
, which is called with
vatAdminService
and an optional zcfBundleName
. This function runs
setupCreateZCFVat(vatAdminService)
to attenuate the vat-making power into
one that can only create ZCF vats. Previously, setupCreateZCFVat
closed
over a copy of the ZCF bundle, and passed it to
vatAdminService~.createVat(zcfBundle)
. This hides the existence of the ZCF
bundle from other packages that just want to use Zoe.
However, to remove these large code bundles from userspace messages, we need
the ZCF bundle to be installed "off to the side", by the code that prepares
the swingset environment (usually as config.bundles.zcf=
).
This commit changes fakeVatAdmin.js
to implement createVatByName('zcf')
,
and to move the copy of the zcfBundle out of makeZoeKit()
and into
fakeVatAdmin.js
. In addition, it changes createZCFVat.js
to provide a
default bundle name of 'zcf'. Any unit test that uses fakeVatAdmin.js
can
continue to do so without changes, and the fake service will magically know
how to create Zoe's ZCF vat without any additional configuration.
Unit tests that use swingset, however, need to be updated to install the ZCF bundle into the kernel, with something like:
import zcfBundle from `@agoric/zoe/bundles/bundle-contractFacet.js`;
...
config.bundles.zcf = { bundle: zcfBundle };
Note: if we used { sourceSpec: '@agoric/zoe/contractFacet.js' }
, then we
would not depend upon a recent cd packages/zoe && yarn build
, and each
kernel instance would bundle its own copy. This would be slower, but perhaps
less prone to stale-bundle surprises, and might be a nicer export
commitment).
I'm always hesitant to use "deep imports": e.g. import zcfBundle from '@agoric/zoe/bundles/bundle-contractFacet.js'
, because the package author hasn't explicitly marked that as a deliberate export, so I'm relying upon some internal file layout which isn't part of their promised API.
Currently I think the only deliberate export is import { makeZoeKit } from '@agoric/zoe'
. This is primarily imported by a file named vat-zoe.js
, which wraps makeZoeKit()
in the exported buildRootObject()
. We certainly don't want to include the whole ZCF bundle next to makeZoeKit
, since that would copy it into every vat-zoe
ever created, even though Zoe never refers to the bundle directly (always through a name or, eventually, a bundlecap).
So I think we need an alternate export from @agoric/zoe
.
yarn build
within Zoe to provide an up-to-date versionpackages/vats/decentral-*-config.json
already relies upon sourceSpec: '@agoric/zoe/contractFacet.js'
I could use some advice on what sort of interface Zoe should be committing to, and whether this approach feels comfortable.
Note that this is just the first step. The second step will involve using bundlecaps instead of the hard-coded zcf
name, so we can remove vatAdminService~.createVatByName()
.
Ok, @Chris-Hibbert and @dckc encouraged me to be ok with committing Zoe to a pre-bundled export at @agoric/zoe/bundles/bundle-contractFacet.js
. I'll try to update the README to make it clear (to some future maintainer, at least) that this location is part of the published API. I'll also update all the unit tests I touched to use the pre-bundled version instead of re-bundling it themselves.
@warner When I first started working on our ZH board, I marked this as being for the Mainnet 1 release because it was already in the Review/QA pipeline. The comment makes it seem like it is should be In Progress or maybe MN-1 Backlog or Product Backlog, not really In Review or In QA. Should this be moved back to the MN-1 backlog, or taken out of the Mainnet 1 release altogether.
When a PR is marked as referencing a ticket, I think ZH is enthusiastically moving that ticket into the Review/QA pipeline even though the ticket won't be closed by the PR. I expect there will be another 3-ish PRs necessary before this ticket is closed. I'll move it back to the In Progress pipeline.
Wow, that is pretty annoying!
@warner Moving this back to In Progress since it was moved to Review/QA as part of @michaelfig closing #4373
Reviewing this now:
zoe/tools/fakeVatAdmin.js
only implements the bundlecap form, and I see no other tools in zoe/
pegasus/test/fakeVatAdmin.js
only implements the full-bundle form, but it is unusedspawner/src/contractHost.js
provides a spawner service that relies upon the full-bundle formSo I think spawner is the remaining blocker, but of course it would be a drastic API change. And the spawner service is used by code outside of agoric-sdk, so I'm not sure a simple CI run would catch the breakage.
I'll push a draft PR to remove the feature from vat-admin, and see which of our existing tests break.
What is the Problem Being Solved?
4372 makes it possible to call
E(vatAdminService).createVat(bundlecap)
, and #4486 wants to removeE(vatAdminService).createVat(bundle)
. After #4372 lands, but before #4486 can land, we need to update both Zoe and the unit test approach used by zoe/ertp/governance/etc to use bundlecaps instead of bundles.Description of the Design
TBD
Security Considerations
Test Plan