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

docs: Clarify the upgrade process and requirements for vats and contracts #6916

Merged
merged 7 commits into from
Feb 21, 2023

Conversation

gibson042
Copy link
Member

  • Update the agoric-sdk "Taxonomy of Exo-making Functions" to match endo.
  • Update text to reference that hierarchy.
  • Preferentially reference prepare{Exo,ExoClass,ExoClassKit} over direct kind handle access.
  • Add more internal links.
  • Consistently use "control facet" and "null upgrade".
  • Provide a vat upgrade example.


## Durable State
As a special case, the root object returned from v2's `buildRootObject()` is automatically associated with exportID `o+0` (see [How Liveslots Uses the Vatstore](https://github.com/Agoric/agoric-sdk/blob/master/packages/swingset-liveslots/src/vatstore-usage.md#counters)).
Copy link
Member

Choose a reason for hiding this comment

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

I think this implies that all every result of buildRootObject() will have the same identity in liveslots. This wasn't obvious from reading here and would be worth making explicit. Maybe even more explicit: the root object should not be stored in baggage.

@gibson042 gibson042 requested a review from dckc February 6, 2023 18:35
Copy link
Member

@warner warner left a comment

Choose a reason for hiding this comment

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

Looks good, minor changes suggested.

packages/SwingSet/docs/dynamic-vats.md Outdated Show resolved Hide resolved
packages/SwingSet/docs/dynamic-vats.md Outdated Show resolved Hide resolved
packages/SwingSet/docs/vat-upgrade.md Outdated Show resolved Hide resolved
packages/SwingSet/docs/vat-upgrade.md Outdated Show resolved Hide resolved
packages/SwingSet/docs/vat-upgrade.md Outdated Show resolved Hide resolved
Each call returns a "make" function for the durable exo class in baggage, creating it first if necessary.
- `prepareExoClassKit`:
Each call returns a "makeKit" function for the durable exo class kit in baggage, creating it first if necessary.
<!-- end copied data -->
Copy link
Member

Choose a reason for hiding this comment

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

what's this "end copied data" thing?

Copy link
Member Author

Choose a reason for hiding this comment

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

An indication of closing the first-line <!-- copied on 2023-02-03 from https://github.com/endojs/endo/blob/master/packages/exo/docs/exo-taxonomy.md#heap-vs-virtual-vs-durable -->. I can switch to e.g. #region Copied from Endo#endregion if you'd find that more intuitive.

- `prepareExoClassKit`:
Each call returns a "makeKit" function for the durable exo class kit in baggage, creating it first if necessary.
<!-- end copied data -->

Copy link
Member

Choose a reason for hiding this comment

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

This whole section is excellent, and I learned a lot reading it. It might be even better with a table, showing the function names used for each pairing of heap/virtual/durable and exp/class/kit (and showing the intentionally omitted combinations).

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought so too in endojs/endo#1469 , and consider it another improvement to make upstream.

[upgrading a vat](https://github.com/Agoric/agoric-sdk/blob/master/packages/SwingSet/docs/vat-upgrade.md).
Most state is discarded, however "durable" collections are retained for use by the replacement version.

The upgrade process is triggered through the "adminFacet" of the instance, and requires specifying the new source bundle. (Note that a "null upgrade" that re-uses the original bundle is valid, and a legitimate approach to deleting accumulated state).
Copy link
Member

Choose a reason for hiding this comment

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

Again, we should mention the need for a BundleCap somehow.

Although.. oh, huh. I'd forgotten what we decided about how Zoe manages the source of an upgrade. The installation has a specific BundleID, and the initial instance of that installation uses that bundle. But you upgrade the instance, and then it becomes.. detached from the installation?

If someone asks Zoe about the instance, does it tell you about the original installation, and then you might think that it's still running that old source code? Or can you ask it for the new BundleID? cc @erights to think about source-code access and auditability.

Anyways, if zoe upgrade takes a BundleID, we should add a sentence about how controller.installBundle() needs to happen first.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've just replaced "source bundle" with "source code" here for now, and I think the details (which are dictated by the specific surface area of a contract instance's "adminFacet", cf. packages/zoe/src/zoeService/startInstance.js ) can be clarified later by someone with authoritative information—and might turn out to be me anyway, depending upon how the near future goes. In the meantime, hopefully the example suffices.

packages/zoe/README.md Outdated Show resolved Hide resolved
CounterI,
initCounterState,
{
increment() { return this.state.value += 1n; },
Copy link
Member

Choose a reason for hiding this comment

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

As discussed in the show-and-tell, we should have an upgrade example somewhere that shows the v1 code and v2 code separately, with another section that specifically shows a diff. Teaching upgrade is always kinda tricky, but I think an example where you add a new method would help make things more concrete. Especially if it tells a story like "you deployed v1 in March, and then in April you realized you wanted a new feature, here's how you roll it out". It's too easy for a single compact example to tell the end state without describing the starting state.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. The example should also show how the v2 contract triggers different behaviors based on whether the 'version' in instanceBaggage indicates an upgrade is needed or not.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.


Upgrades use bundlecaps, just like the initial `createVat()` call. So the first step of an upgrade is to install the v2 source bundle (unless it is a "cross-grade" that re-uses the original bundle). This uses `controller.validateAndInstallBundle()` from the outside, and `D(devices.bundle).getBundleCap()` from the inside. See docs/bundles.md for details.
Upgrades use bundlecaps, just like the initial `createVat()` call. So the first step of an upgrade is to install the v2 source bundle (unless it is a "null upgrade" that re-uses the original bundle). This uses `controller.validateAndInstallBundle()` from the outside, and `D(devices.bundle).getBundleCap()` from the inside. See [bundles.md](./bundles.md) for details.

Once the governance object is holding the v2 bundlecap, it triggers the upgrade with `E(adminNode).upgrade(newBundlecap, options)`. This schedules the upgrade sequence (described above), and returns a Promise that resolves when the upgrade is complete. If the upgrade fails, the Promise is rejected and the old vat is reinstalled. An upgrade might fail because the new source bundle has a syntax error (preventing evaluation), or because the upgrade phase throws an exception or returns a Promise that rejects. It will also fail if the ugprade phase does not fulfill all of its obligations, such as leaving a durable Kind unattached.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the baggage passed to the contract on upgrade mutable? If so, when the contract upgrade fails is baggage restored to its initial state?


Upgrades use bundlecaps, just like the initial `createVat()` call. So the first step of an upgrade is to install the v2 source bundle (unless it is a "cross-grade" that re-uses the original bundle). This uses `controller.validateAndInstallBundle()` from the outside, and `D(devices.bundle).getBundleCap()` from the inside. See docs/bundles.md for details.
Upgrades use bundlecaps, just like the initial `createVat()` call. So the first step of an upgrade is to install the v2 source bundle (unless it is a "null upgrade" that re-uses the original bundle). This uses `controller.validateAndInstallBundle()` from the outside, and `D(devices.bundle).getBundleCap()` from the inside. See [bundles.md](./bundles.md) for details.

Once the governance object is holding the v2 bundlecap, it triggers the upgrade with `E(adminNode).upgrade(newBundlecap, options)`. This schedules the upgrade sequence (described above), and returns a Promise that resolves when the upgrade is complete. If the upgrade fails, the Promise is rejected and the old vat is reinstalled. An upgrade might fail because the new source bundle has a syntax error (preventing evaluation), or because the upgrade phase throws an exception or returns a Promise that rejects. It will also fail if the ugprade phase does not fulfill all of its obligations, such as leaving a durable Kind unattached.

An important property of the `options` bag is `vatParameters`. This value is passed to the upgrade phase (as the usual second argument to `buildRootObject()`) and can be used to communicate with the upgrade-time code before any external messages have a chance to be delivered. In the Zoe ZCF vat, this is how new contract code will be delivered, so it can be executed (and can assume responsibility for v1 obligations) to completion by the time the upgrade phase finishes.
Copy link
Contributor

Choose a reason for hiding this comment

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

options is described here as a "bag". This term isn't used elsewhere. The term should be explained or replaced with a more common term.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a fairly well-known term in JavaScript (cf. https://github.com/tc39/how-we-work/blob/main/terminology.md#options-bag), but not necessary here so I've just replaced it as suggested.

const fooHandle = baggage.get('foo handle');
const makeFoo = makeDurableKind(fooHandle, newFooBehavior);
import { M } from '@agoric/store';
// fooMethodGuards and fooMethods must match or be a superset of their v1 analogs.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there anything that enforces that fooMethodGuards and fooMethods must match or be a superset of their v1 analogs

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes; I think it's in liveslots (and now mentioned in this file).


The virtual and durable variants are contributed by higher layer packages that build on this one, such as `@agoric/vat-data`.
### Class Cardinality
The total number of exo classes must be low cardinality, regardless of virtual/durable status. Being virtual or durable only enables class *instances* to be high cardinality.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why must the total number of exo classes be low cardinality?

Copy link
Member Author

Choose a reason for hiding this comment

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

IIRC, because each Kind is associated with a memory footprint.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be helpful to add an explanation in the doc

Copy link
Member Author

Choose a reason for hiding this comment

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

@warner @erights et al.: before I commit such a change, can you confirm that "memory footprint" is the most important reason to mention for keeping the cardinality of exo classes low?

@gibson042
Copy link
Member Author

Responded to review feedback, everyone PTAL!

* ***`*DurableExo*`*** <br>
The durable exo objects are also written to external storage. These can also survive upgrade, and so can be passed in baggage to a successor vat-incarnation.
### define*ExoClassKit
We often call a record of named entangled Xs an "XKit", by analogy to a "toolkit" being a collection of closely related tools. Each call to `defineExoClassKit`, `defineVirtualExoClassKit`, or `defineDurableExoClassKit` defines a kit of entangled Exo classes, and makes and returns a "makeKit" function that makes new instances of that kit. Each instance of the kit is a collection of "facets" that share common encapsulated state.
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Just some stylistic normalization so far.

packages/zoe/README.md Outdated Show resolved Hide resolved
packages/zoe/README.md Outdated Show resolved Hide resolved
packages/zoe/README.md Show resolved Hide resolved
packages/zoe/README.md Outdated Show resolved Hide resolved
packages/zoe/README.md Outdated Show resolved Hide resolved
Co-authored-by: Mark S. Miller <erights@users.noreply.github.com>
@gibson042 gibson042 added the automerge:squash Automatically squash merge label Feb 21, 2023
@mergify mergify bot merged commit 92d2ac3 into master Feb 21, 2023
@mergify mergify bot deleted the gibson-2023-02-vat-contract-upgrade branch February 21, 2023 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants