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

fixes to Economic Governance abilities #6422

Merged
merged 5 commits into from
Oct 8, 2022
Merged

Conversation

turadg
Copy link
Member

@turadg turadg commented Oct 7, 2022

Description

BLD holders must be able to replace a rogue economic committee. #5539 won't be done until MN-1.1 so we need some way for Core Eval to do it in an extreme situation.

Security Considerations

Creates a .replaceElectorate capability for holders of a GovernedContractFacetAccess. A group that has access to this already has the ability to vote in a new committee. (Though currently not necessarily the ability to create a new committee as the replacement.)

Documentation Considerations

Script demonstrating how its done.

Testing Considerations

Targeted unit test.
Integration test on local chain, with the dapp-econ-gov UI letting a new committee adjust PSM params and pause APIs.

@turadg turadg requested review from dckc and Chris-Hibbert October 7, 2022 19:29
@@ -15,7 +15,7 @@ package "GovernedContract Vat" <<Rectangle>> {
+terms: { electionManager, governedParams }
+getState()
+getContractGovernor()
-getParamManagerRetriever()
-getParamMgrRetriever()
Copy link
Contributor

Choose a reason for hiding this comment

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

If you make a change to this file, please run .../governance/docs/gen.sh, and checkin the changes to the .png.

packages/inter-protocol/test/supports.js Outdated Show resolved Hide resolved
@turadg turadg force-pushed the fix-replaceElectorate branch 3 times, most recently from 25f4382 to 7ff37a4 Compare October 7, 2022 23:33
@turadg turadg requested a review from Chris-Hibbert October 7, 2022 23:37
@turadg turadg marked this pull request as ready for review October 7, 2022 23:37
Copy link
Contributor

@Chris-Hibbert Chris-Hibbert left a comment

Choose a reason for hiding this comment

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

The governance aspects of this look good to me. I'm less familiar with makefiles and chain updates.

@@ -0,0 +1 @@
replace committee
Copy link
Contributor

Choose a reason for hiding this comment

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

not very helpful yet

},
};

// #region Quasi-imports
Copy link
Contributor

Choose a reason for hiding this comment

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

extraneous.

Copy link
Member

Choose a reason for hiding this comment

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

I think this is saying: this section is the result of a poor-man's module system where we manually copy the functions we need from elsewhere in the SDK.

Copy link
Member Author

@turadg turadg Oct 8, 2022

Choose a reason for hiding this comment

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

return candidate;
};

// #endregion
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, here's the other end. But I'm still convinced the is leftover copypasta.

@@ -0,0 +1 @@
true
Copy link
Contributor

Choose a reason for hiding this comment

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

unused?

Copy link
Member

Choose a reason for hiding this comment

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

it's used in the Makefile

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.

looks good.

@turadg turadg added automerge:no-update (expert!) Automatically merge without updates automerge:rebase Automatically rebase updates, then merge and removed automerge:no-update (expert!) Automatically merge without updates labels Oct 8, 2022
@turadg turadg force-pushed the fix-replaceElectorate branch from 369d290 to be3b393 Compare October 8, 2022 01:48
@turadg turadg force-pushed the fix-replaceElectorate branch from be3b393 to 14463d5 Compare October 8, 2022 03:46
@mergify mergify bot merged commit af47019 into master Oct 8, 2022
@mergify mergify bot deleted the fix-replaceElectorate branch October 8, 2022 04:47
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.

3 participants