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

feat: save PSM adminFacet in bootstrap #6101

Merged
merged 4 commits into from
Sep 1, 2022
Merged

feat: save PSM adminFacet in bootstrap #6101

merged 4 commits into from
Sep 1, 2022

Conversation

Chris-Hibbert
Copy link
Contributor

closes: #6100

Description

Save the PSM adminFacet from bootstrap. Also, the contractGovernor will now make the adminFacet available when other contracts need it.

Security Considerations

The admin facet is very powerful. The bootstrap environment is considered a secure context in which to hold powerful facets for the Econ Committee's use.

Documentation Considerations

None.

Testing Considerations

deferred to #6099.

Additionally, the contractGovernor will now make the adminFacet available.
@Chris-Hibbert Chris-Hibbert added Inter-protocol Overarching Inter Protocol pso labels Aug 31, 2022
@Chris-Hibbert Chris-Hibbert added this to the Mainnet 1 RC0 milestone Aug 31, 2022
@Chris-Hibbert Chris-Hibbert requested a review from dckc August 31, 2022 22:07
@Chris-Hibbert Chris-Hibbert requested a review from turadg as a code owner August 31, 2022 22:07
@Chris-Hibbert Chris-Hibbert self-assigned this Aug 31, 2022
@@ -40,6 +40,15 @@ const sanitizePathSegment = name => {
return candidate;
};

/**
* There's an obsolete version in utils.d.ts
Copy link
Member

Choose a reason for hiding this comment

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

please put these updates there instead so there's one correct one and no incorrect ones.

type AdminFacet = {
  getVatShutdownPromise: () => Promise<any>; // Completion, which is currently any
  upgradeContract: (contractBundleId: string, newPrivateArgs: any) => void;
  restartContract: (newPrivateArgs: any) => void;
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

const creatorFacet = E(governorFacets.creatorFacet).getCreatorFacet();

psmInstanceR.resolve(governedInstance);
psmInstanceR.resolve(await E(governorFacets.creatorFacet).getInstance());
Copy link
Member

Choose a reason for hiding this comment

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

with all these facets, consider having a single entity in the space that is the result of zoe.startInstance.

Copy link
Member

Choose a reason for hiding this comment

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

yeah; we tried that... ran into problems around promises or something... the answer isn't obvious, so we agreed to defer.

@Tartuffo Tartuffo removed this from the Mainnet 1 RC0 milestone Aug 31, 2022
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

This looks like it does what we need for now.

@@ -281,6 +282,7 @@ const start = async (zcf, privateArgs) => {
voteOnApiInvocation,
voteOnOfferFilter: voteOnFilter,
getCreatorFacet: () => limitedCreatorFacet,
getAdminFacet: () => adminFacet,
Copy link
Member

Choose a reason for hiding this comment

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

This gave me some pause... we talked it over with @erights and agreed it's appropriate.

@Chris-Hibbert Chris-Hibbert requested a review from turadg August 31, 2022 22:48
@Chris-Hibbert Chris-Hibbert added the automerge:squash Automatically squash merge label Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge Inter-protocol Overarching Inter Protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Save PSM adminFacet to enable big-hammer upgrade
4 participants