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

zoe needs to create contract vats with a (non-infinite) meter #8216

Open
warner opened this issue Aug 17, 2023 · 0 comments
Open

zoe needs to create contract vats with a (non-infinite) meter #8216

warner opened this issue Aug 17, 2023 · 0 comments
Assignees
Labels
enhancement New feature or request Zoe package: Zoe

Comments

@warner
Copy link
Member

warner commented Aug 17, 2023

What is the Problem Being Solved?

We want to protect the chain against buggy/malicious contract code. One threat is a contract vat that goes into an infinite loop: we want the contract to be killed after some fixed (in-consensus) number of computrons. A second threat is two vats which ping-pong messages between them forever: each individual crank finishes in a reasonable time, but the overall series of cranks never completes.

Currently, Zoe creates all contracts vats without a Meter (it never calls E(vatAdminService).createMeter(), nor is there a way to pass an existing Meter into E(vatAdminService).createVat()). This causes all contracts vats to be unmetered: no per-delivery computron limit, and no longer-term underflowable accumulating metering either.

We have a plan to fix this, in #7938 . This plan requires that the contract vat be created with a non-infinite Meter, so our metering code can do its job.

On our current mainnet chain, we only deploy new contracts through CORE_EVAL proposals, in which a snippet of code is evaluated in a scope which includes access to things like Zoe and vatAdminService. We can use this code to create a Meter of some particular capacity and pass it through to E(zoe).startInstance(), but we need to augment Zoe to accept a Meter argument and pass it through to E(vatAdminService).createVat().

That assumes that we're ok with the authority policy this implies: that anyone with access to zoe can choose their own Meter for the new instance. The nature of createVat's meter option is such that providing a Meter is reducing authority (meter: undefined) gets you unlimited execution, any real Meter instance gets you limited execution, any non-Meter instance causes an error). So it's probably better to think about the opposite question: why should anyone with access to zoe be allowed to create unmetered contract vats? Currently, that's what they get, so adding a new meter: option is not increasing the authority of callers.

(someday, Zoe should probably impose a Meter on all contract vats, at which point allowing the caller to choose to be unmetered would represent an authority increase, and we'd want to change this)

Description of the Design

Zoe's startInstance is currently defined as:

  const startInstance = async (
    installationP,
    uncleanIssuerKeywordRecord = harden({}),
    customTerms = harden({}),
    privateArgs = undefined,
    instanceLabel = '',
  ) => {

I think we could add an options bag, with a .meter component. Then, somehow, Zoe needs to convey this to its call to createVat. Some pieces I think will be involved:

  • createZCFVat gets an extra (optional) meter argument
  • zoeStorageManager.startInstanceAccess.makeZoeInstanceStorageManager gets an extra (optional) meter argument
    • and passes it to createZCFVat()

Security Considerations

Adding the meter will reduce the authority of the new contract vat, so this is not an authority-increasing operation.

We won't be imposing meters on existing vats, so they are not at increased risk of being killed by this change.

Zoe will observe more contract vats being terminated, since we weren't using Meters before. We have several tests of Zoe's behavior in this case, but we haven't exercised it in anger yet. So there is some risk of an unexpected bug in Zoe being triggered by contract vat termination.

Other contracts that rely upon a terminated contract might not react well: projects will be rejected, which invokes error-handling code, which is traditionally undertested/underexercised.

Scaling Considerations

This should help make the chain more sustainable, by limiting the damage of runaway contracts.

Test Plan

We need unit tests in Zoe to confirm that the Meter argument is passed all the way through to fakeVatAdminService.

We should have swingset-style tests in Zoe to confirm that a real Meter can be used. It would be great to have test which deliberately drives a contract vat into fast cross-vat loop (e.g. tell Zoe to set the allocations to something new, wait for the promise to resolve, then do it again, over and over.. anything that involves a mesage to vat-zoe and back), with a relatively low Meter (to let it finish sooner), and assert that the contract vat was terminated.

Upgrade Considerations

We'll have to use a CORE_EVAL to upgrade Zoe to a version that includes this new API. Any new contracts that want to use it must be installed with a CORE_EVAL that happens after that one completes.

cc @Chris-Hibbert

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

No branches or pull requests

3 participants