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

contract upgrade requires functioning contract vat, but it might be paused or unresponsive #6293

Closed
warner opened this issue Sep 21, 2022 · 5 comments
Labels
enhancement New feature or request Governance Governance vaults_triage DO NOT USE Zoe package: Zoe

Comments

@warner
Copy link
Member

warner commented Sep 21, 2022

What is the Problem Being Solved?

Yesterday I learned from @erights that much of contract governance lives inside the contract itself. In particular, contracts request their own vat upgrades. Zoe holds the contract vat's control facet, on which it will eventually do E(vatControlFacet).upgrade(newBundleCap, options). Zoe accepts a "please upgrade me" request from the ZCF facet that lives inside the contract vat, and the contract code within that vat has access to the ZCF facet. The governance code within the contract responds to committee messages from the outside world, which might involve a separate vat-governance (I wasn't clear on this.. I imagine different contracts can use different approaches).

So to perform a contract upgrade, the committee must do some governance stuff, which results in messages being sent into the contract vat, which results in the contract vat sendings a request to zoe, and then zoe asks the vatAdminService to upgrade the contract vat.

One concern is: what if the contract is in such a broken state that its governance mechanism can't correctly respond to the "please upgrade the contract vat" request. For example, if a metering fault caused the contract vat to be paused (#3528. as a more recoverable strategy than immediate termination), then the governance "please upgrade yourself" message would be blocked along with everything else headed into that vat.

We speculated that if vat-governance is managing enough of the process (configured initially by the contract vat, but then capable of running independently), then a good approach would be for the contract to hand the Zoe-hosted "please upgrade that vat" facet to vat-governance, where it could be exercised without relying upon the continued execution of the contract vat.

This depends upon the "please upgrade that vat" authority being encapsulated in a single object, so that the contract could hand this object to vat-governance. If the authority is embedded in a larger object (the one vat-zoe shares with the ZCF facet), then contract authors should be reluctant to share that larger authority with vat-governance. And if the contract vat hosts an attenuating proxy, a broken contract vat would prevent vat-governance from performing the upgrade. Since we don't have a WHERE/THERE feature yet (to allow vat A to evaluate proxy-instantiating code inside vat B without prior coordination), this would have to be implemented in Zoe.

Description of the Design

I don't know enough of our contract governance approach to be sure, but I think it will involve Zoe creating an object which holds the authority to do E(vatControlFacet)~.upgrade(), and giving this object to the contract code. The other half is for contracts to give this same object (not a wrapper around it) to a separate vat, one which hosts the governance stuff.

Security Considerations

As @erights pointed out, "Fred" must be able to inspect the contract and understand all its behavior, including the governance mechanism which could change that behavior in the future. That does not preclude parts of the functionality being hosted elsewhere, however either the contract must supply the code for that, or the behavior of that external system must be specified well enough (and be stable enough) for Fred to comprehend and rely upon.

Test Plan

This is mostly a design pattern, but I think we'd want a test in which some contract vat gets paused, and then we use governance to upgrade the contract anyways. It would probably be easiest to write that after #3528 is implemented, but maybe we could write the contract to deliberately break all incoming messages (including the governance code) after the simulated failure, like with a generic revocable forwarder around the public/private facets and the objects given to ZCF.

@warner warner added enhancement New feature or request Zoe package: Zoe Governance Governance labels Sep 21, 2022
@ivanlei ivanlei added the vaults_triage DO NOT USE label Jan 17, 2023
@dckc
Copy link
Member

dckc commented Jan 17, 2023

I wonder about the premise here:

to perform a contract upgrade, the committee must do some governance stuff, which results in messages being sent into the contract vat, which results in the contract vat sendings a request to zoe, and then zoe asks the vatAdminService to upgrade the contract vat.

  • For MN-1, the right to upgrade a contract is reserved to the BLDer DAO (the BLD stakers), not delegated to any committee
  • The BLDer DAO will use swingset.CoreEval to run code in the bootstrap vat, where the admin facet is held.

Does upgrade using the vat admin facet require cooperation of the contract?

@Chris-Hibbert
Copy link
Contributor

The adminFacet (which has the upgradeContract() method) is created in the Zoe vat, so as long as someone outside the contract has that facet, the contract's assistance isn't needed in order to upgrade. As of #6101, the econ contracts all have their adminFacets saved to the bootstrap namespace, so the econ committee can invoke contract upgrade.

@dckc
Copy link
Member

dckc commented Jan 18, 2023

the contract's assistance isn't needed in order to upgrade

So I'm closing this.

(I'm used to using a label such as invalid in situations like this when we close an issue without making a fix because the premise of the issue was incorrect, but I gather @ivanlei would rather not use that label.)

@dckc dckc closed this as completed Jan 18, 2023
@warner
Copy link
Member Author

warner commented Jan 30, 2023

so as long as someone outside the contract has that facet, the contract's assistance isn't needed in order to upgrade

Zoe's assistance is required, right? Because it's Zoe that knows the right ZCF bundlecap to give to updateVat(), and it's Zoe that knows the right contract bundlecap to pass into vatParameters. So the contract vat's admin facet is not entirely sufficient.

Is there some object inside Zoe that represents the authority to instruct Zoe to upgrade the contract? I think it'd be that object which needs to be available in the bootstrap namespace, to maintain the Big JS Hammer's ability to trigger contract upgrade, in addition to the usual per-contract governance committee.

@dckc
Copy link
Member

dckc commented Jan 30, 2023

Is there some object inside Zoe that represents the authority to instruct Zoe to upgrade the contract?

Yes: "The adminFacet (which has the upgradeContract() method) is created in the Zoe vat" -- CH 18 Jan above

I think it'd be that object which needs to be available in the bootstrap namespace

"As of #6101, the econ contracts all have their adminFacets saved to the bootstrap namespace" -- [ibid]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Governance Governance vaults_triage DO NOT USE Zoe package: Zoe
Projects
None yet
Development

No branches or pull requests

5 participants