Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
327 stars 207 forks source link

Vat Upgrade: v8-PriceAuthority #10401

Open Chris-Hibbert opened 2 weeks ago

Chris-Hibbert commented 2 weeks ago

What is the Problem Being Solved?

As mentioned in https://github.com/Agoric/agoric-sdk/issues/8158, we need the ability to restart all vats. This issue is concerned with restarting v8-PriceAuthority.

Description of the Design

The PriceAuthorityRegistry is not a Zoe contract. It was built from priceAuthorityRegistry.js.

upgrade-vats.test.ts shows that it is upgradeable.

This vat provides a place to register priceAuthorities, that are then relied upon by other contracts. Vaults and Auctions use it as a source of info on prices.

The registry has a variety of methods for requesting prices, some return a (promise for a) quote, some a (promise for a) notifier, some return a (promise for a) non-durable mutable object. The only one of these currently being used is makeQuoteNotifier().

Usage of makeQuoteNotifier() is pretty robust to upgrades of the registry. The clients originally get a promise for a notifier, but as soon as they send a message to the notifier, they're talking directly to the priceAuthority it came from, and won't notice if the registry upgrades. If that priceAuthority restarts, they'll come back to the registry for a new priceAuthority.

In the case of quoteWanted() and quoteGiven(), the client gets a promise that will be resolved promptly by the appropriate priceAuthority. If the registry restarts before delivering the promise, the client will detect that as a failure in the original call. After that, the priceAuthority is responsible for resolving the promise. mutableQuotes are similar. The registry has no on-going responsibility once it has delivered the promise.

Clients of this service have to be aware that the promises they get can break, and when that happens, they are responsible for requesting a new quote, notifier, or etc.

Security Considerations

N/A.

Scaling Considerations

N/A.

Test Plan

It is sufficient to verify that after the registry upgrades, subsequent request for quotes (e.g. quoteWanted()) are serviced.

I also want to validate my belief that the notifiers are unaffected by upgrade of the registry, so I want us to observe at least once that the auction or vaults continue to get price updates after an upgrade. This could be observed in z:acceptance, or yarn test --debug, and doesn't need to be added to the test suite.

Upgrade Considerations

It's all upgrade.

dckc commented 2 weeks ago

Test Plan It is sufficient to verify that after the registry upgrades, subsequent request for quotes (e.g. quoteWanted()) are serviced.

what about...

Usage of makeQuoteNotifier() is pretty robust to upgrades of the registry. The clients originally get a promise for a notifier ... If that priceAuthority restarts, they'll come back to the registry for a new priceAuthority.

Do we already have tests of that?

Chris-Hibbert commented 2 weeks ago

Do we already have tests of that?

We do not. But now that we've replaced auctions and upgraded vaults, I'd expect such tests to pass. I'll work on adding them.