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.

Sign up for GitHub

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

Jump to bottom

Configurator #160

Merged
merged 26 commits into from
Mar 30, 2022
Merged

Configurator #160

merged 26 commits into from
Mar 30, 2022

Conversation

kevincheng96
Copy link
Contributor

@kevincheng96 kevincheng96 commented Feb 9, 2022 •
edited
Loading

This is an implementation of the advanced architecture described in RFC 021, but with some changes to enhance upgradeability, namely a separate Configurator contract that is upgradeable. The overall system looks something like:

Timelock -> ProxyAdmin -> Proxy -> Configurator
                       \
                        > Proxy -> Comet

Timelock is an admin of ProxyAdmin, which is an admin of the two proxies. Timelock is also an admin of Configurator, meaning only the Timelock can set parameters in Configurator.

jflatow
jflatow reviewed Feb 9, 2022
View reviewed changes
contracts/CometStorage.sol Outdated Show resolved Hide resolved
jflatow
jflatow reviewed Feb 9, 2022
View reviewed changes
contracts/CometStorage.sol Outdated Show resolved Hide resolved
jflatow
jflatow reviewed Feb 9, 2022
View reviewed changes
contracts/CometFactory.sol Outdated Show resolved Hide resolved
jflatow
jflatow reviewed Feb 9, 2022
View reviewed changes
Copy link
Contributor

@jflatow jflatow left a comment

Choose a reason for hiding this comment

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

Awesome work getting this started 💪

@kevincheng96
Copy link
Contributor Author

Added some changes to improve the upgradeability of the Configurator. The previous implementation and design in RFC 021 has a flaw because all the configurator logic existed in the proxy, which meant that the configurator logic would be impossible to upgrade.

There is now a separate Configurator implementation contract that the proxy fallbacks to for admin callers. Non-admin callers will still fallback to the Comet implementation.

@kevincheng96 kevincheng96 marked this pull request as ready for review February 22, 2022 19:00
@kevincheng96 kevincheng96 changed the title Kevin/configurator Configurator Feb 22, 2022
hayesgm
hayesgm reviewed Feb 24, 2022
View reviewed changes
contracts/Configurator.sol Outdated Show resolved Hide resolved
contracts/Configurator.sol Outdated Show resolved Hide resolved
@kevincheng96
Copy link
Contributor Author

I've updated the configurator flow to be simpler based on a discussion with Geoff. Instead of sharing one customized proxy, the configurator and Comet will now both have their own un-modified OZ proxies.

hayesgm
hayesgm reviewed Feb 24, 2022
View reviewed changes
Copy link
Contributor

@hayesgm hayesgm left a comment

Choose a reason for hiding this comment

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

Think this is making good sense!

@kevincheng96
Copy link
Contributor Author

Added the ProxyAdminAdmin in the latest commit. It's the admin to the ProxyAdmin. It gives governance more flexibility as it can both upgrade the proxies (by going through the ProxyAdmin) and directly call the proxy implementations (which the ProxyAdmin cannot do).

@kevincheng96 kevincheng96 force-pushed the kevin/configurator branch 3 times, most recently from 37d0a1b to 26984c6 Compare March 2, 2022 19:20
jflatow
jflatow reviewed Mar 2, 2022
View reviewed changes
contracts/Configurator.sol Outdated Show resolved Hide resolved
hayesgm
hayesgm reviewed Mar 7, 2022
View reviewed changes
contracts/Configurator.sol Outdated Show resolved Hide resolved
contracts/Configurator.sol Outdated Show resolved Hide resolved
contracts/Configurator.sol Show resolved Hide resolved
contracts/Timelock.sol Outdated Show resolved Hide resolved
contracts/TransparentUpgradeableConfiguratorProxy.sol Show resolved Hide resolved
deployments/fuji/migrations/1646440158_configurator.ts Outdated Show resolved Hide resolved
deployments/fuji/migrations/1646440158_configurator.ts Outdated Show resolved Hide resolved
@kevincheng96
Copy link
Contributor Author

Rebased on top of main (which has the recently merged #208).

jflatow
jflatow reviewed Mar 8, 2022
View reviewed changes
contracts/Configurator.sol Outdated Show resolved Hide resolved
jflatow
jflatow reviewed Mar 8, 2022
View reviewed changes
src/deploy/Development.ts Outdated Show resolved Hide resolved
@kevincheng96
Set up CometProxyAdmin and get deployment and scenarios working
5c4cb42
@kevincheng96
Add add asset functionality
c38a396
@kevincheng96
Create separate upgradeable configurator contract that the proxy fall…
c36d406 
…backs to for admins
@kevincheng96
Add space
e920016
@kevincheng96
Move upgradeTo logic from Configurator to ProxyAdmin
3f1df57
@kevincheng96
Update _CONFIGURATOR_SLOT
867b3aa
@kevincheng96
Implement simple configurator flow, where configurator has its own proxy
d2361a4
@kevincheng96
Introduce the ProxyAdminAdmin; add Configurator unit tests
7c39531
@kevincheng96
Set up configurator migrations on Kovan and test e2e flow using a sim…
fdf7b30 
…ple Timelock as admin
@kevincheng96
Update TUP proxy's impl to be callable by admin instead of using a Pr…
420947c 
…oxyAdminAdmin contract (#212)
@kevincheng96
Get scenarios working on all bases; deploy configurator to Fuji
4a14f1d
@kevincheng96
Update roots.json for kovan and fuji
53b653b
@kevincheng96
Clear github actions cache
b785627
@kevincheng96
Move Timelock.sol under contracts/test
9d91b58
@kevincheng96
Move configurator deployments to the bootstrapping migration scripts
59ced11
@kevincheng96
Get rid of setConfiguration()
c8788d1
@kevincheng96
Move modified vendor contracts into contracts/vendored and update REA…
0590104 
…DME with info on these changes
@kevincheng96
Use a version storage slot to gate the initializer in Configurator
e851199
@kevincheng96
Small code cleanup
d36f18e
@kevincheng96
Simplify deployAndUpgradeTo() and delete unused ITransparentUpgradeab…
f6d5628 
…leProxy.sol
@kevincheng96
Rebase; Move comet proxy contracts from contracts/vendored to contrac…
381ed9d 
…ts directory
@kevincheng96 kevincheng96 merged commit b948624 into main Mar 30, 2022
@kevincheng96 kevincheng96 deleted the kevin/configurator branch March 30, 2022 21:24
@hayesgm hayesgm mentioned this pull request Mar 31, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jflatow jflatow jflatow approved these changes

@hayesgm hayesgm hayesgm approved these changes

@scott-silver scott-silver Awaiting requested review from scott-silver

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kevincheng96 @jflatow @hayesgm