-
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
Builder Script for Replacing Committee and Charter #10161
Conversation
Deploying agoric-sdk with Cloudflare Pages
|
e195a64
to
ee927b8
Compare
8a7f6b6
to
a038d6e
Compare
await startNewEconomicCommittee(permittedPowers); | ||
await startNewEconCharter(permittedPowers); | ||
await addGovernorsToEconCharter(permittedPowers); | ||
await handlehighPrioritySendersList(permittedPowers); | ||
// await handlehighPrioritySendersList(permittedPowers); | ||
|
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.
Commented line 338. Because the shutdown
definition assumes that committee and charter contracts have shutdown
method in their creatorFacet.
Commented 343 too. Although it's not required if I've updated the wallet addresses on line 5 in runConfig
object. Wallet addresses should be based on a3p.
'@agoric/inter-protocol/src/econCommitteeCharter.js', | ||
'../bundles/bundle-econCommitteeCharter.js', | ||
), | ||
), |
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.
Having developed the builder script by referencing other examples in our repo, I have some questions around:
...opts,
economicCommitteeRef: publishRef(
install(
'@agoric/governance/src/committee.js',
'../bundles/bundle-committee.js',
),
),
economicCharterRef: publishRef(
install(
'@agoric/inter-protocol/src/econCommitteeCharter.js',
'../bundles/bundle-econCommitteeCharter.js',
),
),
- What are the roles of
economicCommitteeRef
andeconomicCharterRef
? Specifically, what are these references used for in the script? - Regarding the
install
function, what does the second parameter represent? Is it indicating the directory where the bundles are located, or does it serve another purpose?
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.
Is this section of code responsible for generating new bundles? If yes, I reckon we don't need to include:
economicCommitteeRef: publishRef(
install(
'@agoric/governance/src/committee.js',
'../bundles/bundle-committee.js',
),
),
economicCharterRef: publishRef(
install(
'@agoric/inter-protocol/src/econCommitteeCharter.js',
'../bundles/bundle-econCommitteeCharter.js',
),
That's something we plan to do in 2 different core evals(Maybe I'll add that in #10151 and #10150) which updates the creatorFacet
of committee and charter contracts with the relevant methods after our design for it is finalized.
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.
This is good progress.
The tests need to be included in the PR, though. They need to run in ci.
I strongly suggest making one PR that matches the scope of #10136 -- including the test plan -- and another that matches #10137 (#10135 is a user story, but building it consists of #10134 and #10137).
I much prefer to see #10134 addressed first, though.
refs: #10136 #10135
Description
The PR creates a builder script for the core eval for replacing the committee and charter in #10105
Security Considerations
n/a
Scaling Considerations
n/a
Documentation Considerations
I haven't found much about builder scripts in our documentation. Mainly took pointers from #8087 shared by Dan.
If appropriate, we can document the process detailing what it is and how it works.
Testing Considerations
I tested it with a3p mainly via this bash script I created:
https://gist.github.com/rabi-siddique/5dc08dd3d167c4a1ecbc912d42fee68a
Upgrade Considerations
n/a