-
Notifications
You must be signed in to change notification settings - Fork 4
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
Migrate without lockupIds #435
Conversation
@@ -6,6 +6,7 @@ import {Vm} from "forge-std/Vm.sol"; | |||
import {Addresses} from "contracts/utils/Addresses.sol"; | |||
import "forge-std/console.sol"; | |||
|
|||
import {TimelockController} from "OpenZeppelin/[email protected]/contracts/governance/TimelockController.sol"; | |||
import "OpenZeppelin/[email protected]/contracts/utils/Strings.sol"; | |||
|
|||
library GovFive { |
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.
I think we should spin timelock governance control out out into a seperate file from the governor bravo style governance submission. We'll be moving back to the governor bravo style in the future.
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.
We already have a GovProposalHelper
for Bravo style governance. This one only deals with multisig governance
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.
(an idea is that the difference governance methods can be pluggable, this way we can use this code on different chains with difference governance systems.)
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.
It should be possible to do that but I'll put that one up as a different PR
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.
Maybe I'm just confused by the name GovFive
which is the name of our OGV Governance contract. Maybe all that needs to change here is the name of this contract then.
migrate()
withoutlockupIds
This also removes the need for the
MigrationZapper
contract. The frontend can always call a single method for all purposesIf you made a contract change, make sure to complete the checklist below before merging it in master.
Contract change checklist: