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

fix(provisioning): export useful namesByAddress #8821

Merged
merged 4 commits into from
Jan 27, 2024
Merged

Conversation

dckc
Copy link
Member

@dckc dckc commented Jan 26, 2024

refs: #8113
(doesn't close it until #8786 lands)
refs: #8786

Description

The namesByAddress provided by the provisioning vat is disconnected from namesByAddressAdmin, so it never gets populated.

Fortunately, the relevant nameHubKit is in baggage. So the fix here is to return that one.

Security Considerations

This should make it unnecessary for future core-eval scripts to get the whole namesByAddressAdmin authority when they only need namesByAddress to do something like deliver fees.

The permit of the upgrade core-eval is:

      consume: {
        vatStore: true,
        vatAdminSvc: true,
      },
      produce: {
        namesByAddress: true,
      },

Read/write access to the whole vatStore is excess authority; we only need access to read one of its entries, not authority to scribble over all the others and upgrade all the other vats. (cc @raphdev )

vatAdminSvc is needed to look up a bundleID (hash) and return the corresponding bundleCap. Its only method beyond that sort of bundle lookup is E(vatAdminSvc).createVat(). That method is more than we need, but while it can be wasteful, it doesn't pose any risk to integrity.

Scaling Considerations

n/a

Documentation Considerations

fixes the chain to agree with our docs on name services

Testing Considerations

Includes a unit test.

The upgrade proposal here works on a release branch (see #8786) but won't work on master due to a breaking change in @endo/patterns (#8826). So we don't have a test that runs it.

1.0.0 (2023-12-12)
⚠ BREAKING CHANGES
patterns: tag and retype guards

Upgrade Considerations

The bug is localized to the provisioning vat, so upgrading it seems to suffice.

Perhaps we should fix #8408 while we're at it?

Copy link
Member

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

@dckc dckc added the automerge:rebase Automatically rebase updates, then merge label Jan 27, 2024
@mergify mergify bot merged commit f3c2b65 into master Jan 27, 2024
74 checks passed
@mergify mergify bot deleted the 8113-provision-names branch January 27, 2024 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

E(namesByAddressAdmin).readonly() throws pattern error: promise vs. remotable
3 participants