chore: upgrade v8, the priceAuthorityRegistry #10418
Conversation
Chris-Hibbert
added
contract-upgrade
oracle
| registryRef: publishRef( | ||
| install('@agoric/vats/src/vat-priceAuthority.js'), |
There was a problem hiding this comment.
Will this update agoricNames.installation with a new zoe Installation? Since this isn't a contract, that would be odd.
There was a problem hiding this comment.
Oh! good point. I'll have to figure out how to pass the bundle through without installing a contract.
We use publishRef(install()) in the Zoe proposals. I don't know why that's okay, but it seems to be the standard approach.
There was a problem hiding this comment.
We use publishRef(install()) in the Zoe proposals. I don't know why that's okay, but it seems to be the standard approach.
It was certainly the standard approach for the initial bootstrap of the system. Whether the idiom is appropriate for upgrades, I am not at all sure. @michaelfig , thoughts?
There was a problem hiding this comment.
publishRef(install(...)) is needed to create a bundlecap before running the getManifest* behaviours. This is appropriate for all circumstances when a contract needs to be taken from disk and its bundlecap passed in to the getManifest* call.
It's the getManifestForUpgradingRegistry that controls how those bundlecaps are passed to the behaviours. See below...
There was a problem hiding this comment.
This is appropriate for all circumstances when a contract needs to be taken from disk and its bundlecap passed in to the getManifest* call.
@michaelfig: The issue @dckc is bringing up applies for non-contract vats. Is there a way to produce a bundleCap that can be used for pure vats that doesn't presume to install it as if it were a Zoe contract?
There was a problem hiding this comment.
In const bundleRef = publishRef(install('entrypoint.js')), the install creates an on-chain bundleCap (not a Zoe installation) from entrypoint.js, and publishRef produces a VatSourceRef for it, all with no inherent dependency on Zoe.
The only time Zoe installations would be implicitly created and published under agoricNames.installation would be if your getManifest return value had an installations property, which it doesn't. Even if it did, the bundleRef still describes the underlying generic bundleCap, which has nothing to do with Zoe.
Other than that, you can put on your ocap reasoning cap to understand that the behaviours named by your manifest do not reference zoe, so they can't use Zoe. Furthermore, your behaviours correctly use the vat's adminNode to upgrade the non-contract vat to the bundleCap corresponding to the bundleRef.
There was a problem hiding this comment.
This is appropriate for all circumstances when a contract needs to be taken from disk...
Sorry for the confusion; I mistyped. It should have been "... all circumstances when an [arbitrary JS module and its dependencies need] to be taken from disk...".
| * @param {BootstrapPowers & { | ||
| * consume: { | ||
| * vatAdminSvc: VatAdminSvc; | ||
| * vatStore: MapStore< | ||
| * string, | ||
| * import('@agoric/swingset-vat').CreateVatResults | ||
| * >; | ||
| * }; | ||
| * }} powers |
There was a problem hiding this comment.
BootstrapPowers already has vatAdminSvc and vatStore defined this way, doesn't it?
| * @param {BootstrapPowers & { | |
| * consume: { | |
| * vatAdminSvc: VatAdminSvc; | |
| * vatStore: MapStore< | |
| * string, | |
| * import('@agoric/swingset-vat').CreateVatResults | |
| * >; | |
| * }; | |
| * }} powers | |
| * @param {BootstrapPowers} powers |
| agoricNames: par, | ||
| vatAdminSvc: par, | ||
| vatStore: par, | ||
| priceAuthority: 'par', |
There was a problem hiding this comment.
typo?
| priceAuthority: 'par', | |
| priceAuthority: par, |
Deploying agoric-sdk with
|
| Latest commit: |
e8853cf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://217ae40b.agoric-sdk.pages.dev |
| Branch Preview URL: | https://10401-registry.agoric-sdk.pages.dev |
Chris-Hibbert
force-pushed
the
10401-registry
branch
from
7c7c846 to
57b52e1
Compare
| registryRef: publishRef( | ||
| install('@agoric/vats/src/vat-priceAuthority.js'), |
There was a problem hiding this comment.
publishRef(install(...)) is needed to create a bundlecap before running the getManifest* behaviours. This is appropriate for all circumstances when a contract needs to be taken from disk and its bundlecap passed in to the getManifest* call.
It's the getManifestForUpgradingRegistry that controls how those bundlecaps are passed to the behaviours. See below...
| options: { | ||
| registryRef, | ||
| }, |
There was a problem hiding this comment.
If the getManifest returns an installations: Record<InstallationName, BundleCap> property, that will cause the publishing of the bundleCaps in agoricNames.installation. However, you can see here that registryRef is just returned via options, not installations. No implicit publishing, just making it available to the upgradePriceAuthorityRegistry behaviour.
So, this part of the PR LGTM!
|
This is going to show up twice, but I can't see that it got forwarded to anyone, so let me enter it again.
@michaelfig: The issue @dckc is bringing up applies for non-contract vats. Is there a way to produce a bundleCap that can be used for pure vats that doesn't presume to install it as if it were a Zoe contract? |
Chris-Hibbert
force-pushed
the
10401-registry
branch
from
57b52e1 to
bbe909c
Compare
Chris-Hibbert
force-pushed
the
10401-registry
branch
from
bbe909c to
67dd557
Compare
Chris-Hibbert
force-pushed
the
10401-registry
branch
from
67dd557 to
e8853cf
Compare
closes: #10401
Description
Upgrade the priceAuthorityRegistry
Security Considerations
N/A
Scaling Considerations
N/A
Documentation Considerations
Does need to be mentioned in the upgrade description.
Testing Considerations
As issue #10401 directs, we verify that
quoteWanted()continues to work.Existing tests in
z:acceptanceverify that prices continue to be received by vaults.Upgrade Considerations
This should be included in upgrade 19.