Support multiple Comets using one Configurator #437
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jflatow
reviewed
Jun 24, 2022
jflatow
reviewed
Jun 24, 2022
jflatow
reviewed
Jun 24, 2022
jflatow
reviewed
Jun 27, 2022
jflatow
approved these changes
Jun 27, 2022
scott-silver
approved these changes
Jun 27, 2022
kevincheng96
added a commit
that referenced
this pull request
Jun 28, 2022
This updates the docs to be consistent with the changes introduced to the Configurator in #437.
ajb413
added a commit
that referenced
this pull request
Jun 28, 2022
* cleanup, added total borrows, added notes on reward accrual * remove web3.js docs examples * rearrange, collapse code examples, wording changes in liquidation section * missing italic * moved functions to liquidation section * Update docs for Configurator (#443) This updates the docs to be consistent with the changes introduced to the Configurator in #437. * added reserve section edits Co-authored-by: Kevin Cheng <kevincheng96@hotmail.com>
MichaelMorami
pushed a commit
to Certora/comet
that referenced
this pull request
Jun 29, 2022
This PR modifies the Configurator implementation to support multiple deployments of Comet so only a single Configurator instance needs to be deployed per chain. Instead of storing a single Configuration and factory address, the Configurator now stores a mapping from Comet proxy addresses to their respective Configuration and factory addresses. Because Configurator.deploy() now takes a Comet proxy address as an argument, a small change is also made to the CometProxyAdmin.deployAndUpgradeTo(...) function to use this new interface. I also removed the logic of setting an initial factory and configuration from the initializer and, instead, added new setters to set these after initialization. This PR also updates our unit tests, scenarios, deployment logic, and migration scripts to work with the new Configurator implementation. For scenarios specifically, I added a new flag called upgradeAll to the ModernConstraint that upgrades all contracts. This is needed in scenarios that need to upgrade more than just Comet. For example, we need to upgrade the CometProxyAdmin in a small number of our scenarios that use the Configurator. TLDR: Contract changes are: Change Configurator storage for factory and configuration to be mappings Move the setting of the initial factory and configuration out of the initializer Modify the CometProxyAdmin to use the new interface for Configurator.deploy(address)
MichaelMorami
pushed a commit
to Certora/comet
that referenced
this pull request
Jun 29, 2022
* cleanup, added total borrows, added notes on reward accrual * remove web3.js docs examples * rearrange, collapse code examples, wording changes in liquidation section * missing italic * moved functions to liquidation section * Update docs for Configurator (compound-finance#443) This updates the docs to be consistent with the changes introduced to the Configurator in compound-finance#437. * added reserve section edits Co-authored-by: Kevin Cheng <kevincheng96@hotmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR modifies the
Configuratorimplementation to support multiple deployments ofCometso only a singleConfiguratorinstance needs to be deployed per chain. Instead of storing a singleConfigurationandfactoryaddress, theConfiguratornow stores a mapping fromCometproxy addresses to their respectiveConfigurationandfactoryaddresses. BecauseConfigurator.deploy()now takes aCometproxy address as an argument, a small change is also made to theCometProxyAdmin.deployAndUpgradeTo(...)function to use this new interface. I also removed the logic of setting an initial factory and configuration from the initializer and, instead, added new setters to set these after initialization (more details covered in this comment).This PR also updates our unit tests, scenarios, deployment logic, and migration scripts to work with the new
Configuratorimplementation.For scenarios specifically, I added a new flag called
upgradeAllto theModernConstraintthat upgrades all contracts. This is needed in scenarios that need to upgrade more than justComet. For example, we need to upgrade theCometProxyAdminin a small number of our scenarios that use theConfigurator.TLDR: Contract changes are:
Configuratorstorage forfactoryandconfigurationto be mappingsfactoryandconfigurationout of the initializerCometProxyAdminto use the new interface forConfigurator.deploy(address)