-
Notifications
You must be signed in to change notification settings - Fork 214
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
Vat Upgrade: v6-agoricNames #10408
Labels
Comments
anilhelvaci
added a commit
that referenced
this issue
Dec 4, 2024
anilhelvaci
added a commit
that referenced
this issue
Dec 4, 2024
anilhelvaci
added a commit
that referenced
this issue
Dec 5, 2024
…y after upgrade Refs: #10408 fix: lint fixes
anilhelvaci
added a commit
that referenced
this issue
Dec 6, 2024
…y after upgrade Refs: #10408 fix: lint fixes fix: broken z:acceptance/yarn.lock
anilhelvaci
added a commit
that referenced
this issue
Dec 7, 2024
…y after upgrade Refs: #10408 fix: lint fixes fix: broken z:acceptance/yarn.lock
anilhelvaci
added a commit
that referenced
this issue
Dec 9, 2024
…y after upgrade Refs: #10408 fix: lint fixes fix: broken z:acceptance/yarn.lock
mergify bot
added a commit
that referenced
this issue
Dec 9, 2024
#10616) closes: #10408 ## Description As stated in #8158, we need to be able to upgrade any vat we want and make sure the system keeps working. This PR focuses on upgrading `vat-agoricNames`. ### Security Considerations I consider `vat-agoricNames` as a critical vat as it's the source trust for object identities. So it's crucial it keeps working after the upgrade. It's important to point out that if there were any keywords reserved before the upgrade, they will be lost after the upgrade as the reservations stored in ephemera. If this is important, we might consider re-reserving in the core eval. ### Scaling Considerations None. ### Documentation Considerations Don't think it's necessary as the functionality of `nameHub` is left untouched. ### Testing Considerations If we decide to re-reserve some keywords, we will need to add a test that verifies the core eval has indeed reserved those keywords. Please see below for the test plan I followed: https://github.com/Agoric/agoric-sdk/blob/1e4bebac1e3ee6df48dbe58e3e5bdf0e9e37cca2/a3p-integration/proposals/p%3Aupgrade-19/agoricNames.test.js#L3-L46 #### EDIT 2195ace removes ` write-chain-info.js` from `f:fast-usdc`. So we simulate what is does (relative to `agoricNames`) in order to test ephemeral `onUpdate` callbacks keep working after an `agoricNames` upgrade. ### Upgrade Considerations This PR itself upgrade `vat-agoricNames` and comes with a3p tests to verify the upgrade goes through and `agoricNames` keep working.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What is the Problem Being Solved?
As mentioned in #8158, we need the ability to restart all vats. This issue is concerned with upgrading
v6-agoricNames
. agoricNames is not a Zoe contract.The source code is
nameHub.js
. It's a critical vat. It appears to be upgradeable. (usesmakeDurableZone
andbaggage
), though I haven't found an upgrade test.I'm suspicious of
ephemera
and the native Maps (new Map()
) it uses.Description of the Design
Write tests in a3p. verify that names bound to brands, instances, issuers, assets are retrievable after upgrade.
I think it's ok if reserved keys break across an upgrade.
OnUpdate doesn't look robust. Verify that these continue to write to vstorage after the upgrade.
onUpdate
hook that doesn't look durable.verify that code that holds both
nameHub
andnameAdmin
facets will continue to have access, and that the hierarchy can be navigated post-upgrade.Security Considerations
Not directly security relevant, but continuity does impact the viability of the chain.
Scaling Considerations
Not significant. This vat grows linearly with the data it holds.
Test Plan
As described above. Thorough testing in
a3p-integration
should be sufficient.Upgrade Considerations
agoricNames normally holds promises that are resolved promptly. Are there any promises outstanding that matter?
@warner @mhofman Is this something you can check?
The text was updated successfully, but these errors were encountered: