-
Notifications
You must be signed in to change notification settings - Fork 254
Add Dual Governance to protocol scratch deploy #1401
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
base: feat/vaults
Are you sure you want to change the base?
Conversation
466b5f6 to
e60b63d
Compare
e60b63d to
d7844ec
Compare
Hardhat Unit Tests Coverage SummaryDiff against masterResults for commit: e54cd83 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
arwer13
left a comment
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.
Thanks for the work!
I see these follow ups:
-
Rebase on up-to-date vaults branch #1287
-
Add integration which verifies dual governance upgrade pass (for any dummy one). Boilerplates for the voting script (contract) is in the vaults branch as well. At the moment it won't work because proper roles distribution (in LidoTemplate.sol) is missing.
-
Make sure no-DG setup is also operational. It is at the moment, but after the roles tinkering there gonna be two Aragon roles configuration. Actually, I'm not 100% sure we optional DG installation is worth the effort, but likely it is. To simplify we can split LidoTemplate into LidoTemplateBase, LidoTemplate and LidoTemplateWithDG - it seems not much work. But for the integration tests and for the utility contracts/scripts which do an upgrade there must be two setups to support in this case. Wdyt @tamtamchik ?
| networkState[Sk.dualGovernanceConfig].dual_governance.sanity_check_params.min_withdrawals_batch_size, | ||
| }, | ||
| }, | ||
| dual_governance_config_provider: { |
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.
Just
| dual_governance_config_provider: { | |
| dual_governance_config_provider: networkState[Sk.dualGovernanceConfig].dual_governance_config_provider |
here and similar places, why not?
| } | ||
| } | ||
|
|
||
| async function runCommand(command: string, workingDirectory: string) { |
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.
runCommand duplicates here and in dg-installation.ts . May move to lib/subprocess.ts or some other lib file
Agree, we need no-DG setup also for some devnets to simplify operational work when DG not related to the scope of changes. |
2e73c48 to
4b6dc09
Compare
c732df7 to
ea79118
Compare
fb6aefd to
673d745
Compare
8b44c84 to
b4e43e0
Compare
b4e43e0 to
d8f4ffd
Compare
b791b52 to
e54cd83
Compare
Add Dual Governance to protocol scratch deploy
Context
After the deployment of Dual Governance on Lido mainnet we need to add the DG deployment to protocol scratch deploy routine.
Problem
DAO-654
After the deployment of Dual Governance on Lido mainnet we need to add the DG deployment to protocol scratch deploy routine.
Solution
Added DG to scratch deployment