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

fix(zoe): add zoe.installBundleID #4598

Merged
merged 1 commit into from
Feb 25, 2022
Merged

Conversation

warner
Copy link
Member

@warner warner commented Feb 18, 2022

fix(zoe): add zoe.installBundleID

To transition Zoe from full contract bundles to bundlecaps, this adds a new
install API. E(zoe).install(bundle) is unchanged, but the new preferred
approach is E(zoe).installBundleID(bundleID). This requires the
corresponding bundle to be installed with the swingset kernel's
vatAdminService, either before or after installBundleID() (zoe will wait
forever for the bundle to be installed).

Zoe Installation objects retain their getBundle() method to accomodate
dapp tests that have not switched to the new approach, but it throws an error
if used on a new bundleID-based installation. A new method named
E(zoe).getBundleIDFromInstallation(allegedInstallation) can be used to both
validate the installation and get back the bundleID, but it throws on the old
bundle-based installations.

Internally, the installationStorage.unwrapInstallation now returns either
{ bundle, installation } or { bundlecap, bundleID, installation }. ZCF's
evaluateContract() method accepts either a bundlecap or a full bundle.

closes #4563

@warner warner self-assigned this Feb 18, 2022
@warner warner force-pushed the 4563-add-zoe-install-bundleid branch 5 times, most recently from ee4bc63 to 7d49244 Compare February 19, 2022 01:08
@warner
Copy link
Member Author

warner commented Feb 19, 2022

This changes a handful of Zoe tests (to confirm that the new behavior works). It does not yet change the rest of the zoe tests, nor the tests elsewhere in agoric-sdk, nor the tests in dapps that live outside agoric-sdk. I need to change all of those before I can remove the old E(zoe).install API.

I also made a small swingset change to export a few types that are now part of the Zoe API (BundleID and a module type). This seems necessary to let packages/vats/ pass typechecking.

@warner warner marked this pull request as ready for review February 19, 2022 01:16
@warner warner requested a review from erights February 19, 2022 01:16
@warner warner added the Zoe package: Zoe label Feb 19, 2022
Copy link
Member

@erights erights left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why *Bundlecap instead of *BundleCap? Since none of this has been merged yet, if we're going to rename it, now is the ideal time.

packages/zoe/src/contractFacet/zcfZygote.js Outdated Show resolved Hide resolved
packages/zoe/src/zoeService/installationStorage.js Outdated Show resolved Hide resolved
Comment on lines +64 to +66
* bundle?: SourceBundle,
* bundleCap?: BundleCap,
* bundleID?: BundleID,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking but FYI, we can use unions to convey mutual exclusion. It has the advantage over optionals of guaranteeing to consumer that one of them is defined. E.g.,

type BundleOrBundleCap = {bundle: SourceBundle} | {bundleCap: BundleCap, bundleId: BundleID};

or in jsdoc,

/** @typedef  { {bundle: SourceBundle} | {bundleCap: BundleCap: bundleId} } BundleOrBundleCap */

then intersection for the return type

Promise<BundleOrBundleCap & {installation: Installation}>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aye, thanks, will look into that later, there's another BundleOrBundleCap floating around which doesn't include the bundleID that I don't want to collide with

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

collide with

Want to point out that the possibility of collisions is due to ambient types. Because TypeScript is structurally typed, there's usually little problem with redundant definitions. (Besides code maintenance/documentation.)

@warner warner force-pushed the 4563-add-zoe-install-bundleid branch from c09ea83 to ee1ab7a Compare February 25, 2022 16:53
Base automatically changed from 4487-zoe-zcfspec to master February 25, 2022 17:09
To transition Zoe from full contract bundles to bundlecaps, this adds a new
install API. `E(zoe).install(bundle)` is unchanged, but the new preferred
approach is `E(zoe).installBundleID(bundleID)`. This requires the
corresponding bundle to be installed with the swingset kernel's
vatAdminService, either before or after `installBundleID()` (zoe will wait
forever for the bundle to be installed).

Zoe `Installation` objects retain their `getBundle()` method to accomodate
dapp tests that have not switched to the new approach, but it throws an error
if used on a new bundleID-based installation. A new method named
`E(zoe).getBundleIDFromInstallation(allegedInstallation)` can be used to both
validate the installation and get back the bundleID, but it throws on the old
bundle-based installations.

Internally, the installationStorage.unwrapInstallation now returns either `{
bundle, installation }` or `{ bundleCap, bundleID, installation }`. ZCF's
`evaluateContract()` method accepts either a bundlecap or a full bundle.

closes #4563
@warner warner force-pushed the 4563-add-zoe-install-bundleid branch from ee1ab7a to 0fad95f Compare February 25, 2022 17:34
@warner warner added the automerge:no-update (expert!) Automatically merge without updates label Feb 25, 2022
@mergify mergify bot merged commit 7fec6f7 into master Feb 25, 2022
@mergify mergify bot deleted the 4563-add-zoe-install-bundleid branch February 25, 2022 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:no-update (expert!) Automatically merge without updates Zoe package: Zoe
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add zoe.installBundleID(bundleID)
4 participants