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

maybe add upgradeVat API to allow "please re-use old vatParameters" #10271

Open
warner opened this issue Oct 13, 2024 · 0 comments
Open

maybe add upgradeVat API to allow "please re-use old vatParameters" #10271

warner opened this issue Oct 13, 2024 · 0 comments
Labels
enhancement New feature or request SwingSet package: SwingSet

Comments

@warner
Copy link
Member

warner commented Oct 13, 2024

What is the Problem Being Solved?

As described in #8947 (comment) , once the kernel is retaining vatParameters, we'll have the option to re-use them when upgrading a vat. We know we'll want this for unilateral "kernel decides now is the time to forcibly restart all vats" events. But we might want to allow userspace to do the same.

Currently, E(adminNode).upgradeVat(bundlecap, options) accepts an options.vatParameters, and the value is passed clear through to liveslots and buildRootObject(). If we choose to let userspace express "please re-use the previous incarnation's vatParameters", how should it express that? One possibility is to make undefined mean "please re-use".

Another possibility is to have a separate argument, but I'm not sure what that would look like, and this would probably require changes (upgrades) to vat-vatAdmin and device-vatAdmin.

A third option is to simply not give this option to userspace. The kernel will do unilateral upgrade-all-vats through an internal interface, and does not need to involve vat-vatAdmin or the E(adminNode).upgrade() API. Given the way Zoe does contract upgrades, I'm not sure that Zoe would benefit from a "please re-use" feature.

Previously, I had been under the impression that something in this pathway supplied a default value (of an empty object) when options.vatParameters was missing. But I did some digging, and was surprised to see that we don't. I need to verify this with a unit test, but from what I could see, it's only the kernel config record which applies a default (i.e. only for static vats). If config.vats.NAME.parameters is missing, the kernel supplies = { } as a default. But vat-vat-admin.js does not apply a default when E(adminNode).upgrade() is called, and it passes the options through device-vat-admin.js and into vat-admin-hook.js without inspecting them.

That's convenient, because we don't yet have a mechanism to upgrade devices, and we might still have some mainnet vats that would get confused if vat-vatAdmin is upgraded (the done() promise would be disconnected, which unaware vats would treat as a rejection, and they would incorrectly believe that the contract vat had been terminated). If we only have to change vat-admin-hooks.js or the kernel's processUpgradeVat, this will be a lot easier to implement.

Description of the Design

If we choose to build this, and I'm not yet convinced that we should, then we'd need the following changes:

Security Considerations

The only one I can think of is that this gives the holder of an adminNode the ability to access the vatParameters sent to an earlier incarnation of that vat.

Currently, the adminNode holder is limited to upgrading the vat to code of their choosing. Iff the previous incarnation had deliberately saved their vatParameters in something reachable from baggage, then the upgraded code could retrieve those values and return them to the holder. But if, for some reason, the previous incarnation had deliberately abandoned access to those values, then without a "reuse previous vatParameters" feature, there would be no way for the vat, nor the adminNode holder, to recover them.

That seems unlikely, but it's worth considering.

Scaling Considerations

Nothing significant

Test Plan

unit tests

Upgrade Considerations

This would change the behavior of E(adminNode).upgrade(), and would remove the ability to upgrade to a vatParameters=undefined (note that createVat() without parameters would still result in vatParameters = undefined, and that would be sticky, so subsequent upgrade() without parameters would mean "re-use the previous ones (which are undefined), so this is not too big of a deal).

We might want to consider whether userspace can tell that upgrade() will behave this way before making the call (the userspace-kernel-boundary versioning question).

This is not strictly a question for this ticket, but most vats do not have retained vatParameters, because #10270 has not landed yet. Before we can safely do an automatic upgrade-all-vats, we must perform manual upgrades of all vats with nontrivial vatParameters so that the kernel has a copy to re-use.

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
Projects
None yet
Development

No branches or pull requests

1 participant