-
Notifications
You must be signed in to change notification settings - Fork 212
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
using governedContractKits in core eval replacing committee and charter #10178
using governedContractKits in core eval replacing committee and charter #10178
Conversation
Deploying agoric-sdk with Cloudflare Pages
|
cd7809e
to
e993aeb
Compare
d9d9bf2
to
ed8b045
Compare
e993aeb
to
cd7809e
Compare
e0c2baa
to
72d59ac
Compare
72d59ac
to
8a1f820
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. This is a definite improvement.
I notice some test failures, but you can merge this, and address them in the main PR.
One suggestion for a change, which can be done in the main PR.
governorCreatorFacet, | ||
label, | ||
} of governedContractKitMap.values()) { | ||
E(creatorFacet).addInstance(instance, governorCreatorFacet, label); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are lots of contracts floating around. let's rename creatorFacet
to ecCreatorFacet
or committeeCreatorFacet
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Made the update.
const psmKitMap = await permittedPowers.consume.psmKit; | ||
|
||
console.log('RABI', [...governedContractKitsMap.values()]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
meant to be dropped?
(I use 'CTH' (my initials) for the same purpose, and have a pre-commit script I run routinely to find phrases which should seldom be submitted. YMMV.)
8277e1b
into
rs-faraz-replace-charter
refs: #10134 #10133
Description
This pull request builds on #10166 and #10164. It updates the core eval code to utilize the
governorCreatorFacet
for the governed contracts via thegovernedContractKit
. Initially, the aim was to use thegovernedContractKit
to access facets from price feed contracts. But since it also includes facets for other governed contracts, I expanded the code to incorporate these as well.Security Considerations
Same as #10166 and #10164
Scaling Considerations
Documentation Considerations
Testing Considerations
Same as specified in #10164
Upgrade Considerations