8000 Deploy cWETHv3 to Goerli by kevincheng96 · Pull Request #646 · compound-finance/comet · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Deploy cWETHv3 to Goerli #646

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

Merged
merged 8 commits into from
Jan 17, 2023

Conversation

kevincheng96
Copy link
Contributor
@kevincheng96 kevincheng96 commented Jan 4, 2023

This PR adds a new goerli-weth base and deploy script for the Goerli cWETHv3 deploy.

The deploy script is very similar to the mainnet cWETHv3 script. The main differences are:

  • Deploying price feeds for stETH / ETH and cbETH / ETH
  • Pulling in extra testnet/cross-chain contracts such as fauceteer and fxRoot using fromDep
  • Cloning over cbETH from mainnet
  • Minting cbETH to fauceteer

@kevincheng96 kevincheng96 changed the base branch from main to jflatow/mainnet-eth January 4, 2023 00:10
@kevincheng96 kevincheng96 marked this pull request as ready for review January 4, 2023 00:25
@kevincheng96 kevincheng96 requested review from scott-silver and jflatow and removed request for scott-silver January 4, 2023 00:25
@scott-silver
Copy link
Contributor

do we need to add goerli-weth to deploy-market, enact-migration and prepare-migration?

@kevincheng96
Copy link
Contributor Author

do we need to add goerli-weth to deploy-market, enact-migration and prepare-migration?

I don't think so since those actually take the deployment as an input (e.g. "usdc").

@jflatow jflatow force-pushed the jflatow/mainnet-eth branch from bf72cea to a9f9584 Compare January 4, 2023 17:28
@kevincheng96 kevincheng96 force-pushed the kevin/goerli-weth-deploy branch from 7f2f2e9 to e1573a5 Compare January 4, 2023 18:59
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.

💪

Copy link
Contributor
@scott-silver scott-silver left a comment

Choose a reason for hiding this comment

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

🎉

const wstETH = await deploymentManager.existing('wstETH', '0x4942BBAf745f235e525BAff49D31450810EDed5b', 'goerli');

// Import shared contracts from cUSDCv3
// Note: We need to first spider the contracts on the cUSDC base. Consider moving this spider logic into `fromDep`.
Copy link
Contributor

Choose a reason for hiding this comment

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

In the future we could also have a fromDep to return all aliases as object and could destructure const { cometAdmin, cometFactory, 'configurator:implementation': $configuratorImpl, ... } = await deploymentManager.fromDep(network, base) and spider from there

Copy link
Contributor

Choose a reason for hiding this comment

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

Eh, this breaks putAlias, which is why it is the way it is currently

@kevincheng96 kevincheng96 force-pushed the kevin/goerli-weth-deploy branch from df1eb4b to 887a1cd Compare January 4, 2023 23:04
@kevincheng96 kevincheng96 changed the base branch from jflatow/mainnet-eth to jflatow/mainnet-eth-live January 4, 2023 23:05
@kevincheng96
Copy link
Contributor Author

Rebased on top of jflatow/mainnet-eth-live to get the latest fromDep changes so we no longer need to explicitly spider within the deploy script.

@jflatow jflatow force-pushed the jflatow/mainnet-eth-live branch from 35c4bc3 to 9f9a917 Compare January 4, 2023 23:53
@jflatow jflatow added the deployment This PR contains a new deployment label Jan 5, 2023
@jflatow jflatow force-pushed the jflatow/mainnet-eth-live branch 2 times, most recently from e69f339 to e1eed58 Compare January 5, 2023 20:07
@kevincheng96 kevincheng96 force-pushed the kevin/goerli-weth-deploy branch from 52fc7ce to 980f399 Compare January 5, 2023 20:21
@jflatow jflatow force-pushed the jflatow/mainnet-eth-live branch 2 times, most recently from b7a82af to b3e9611 Compare January 6, 2023 21:16
const signer = await deploymentManager.getSigner();

// Declare existing assets as aliases
const WETH = await deploymentManager.existing('WETH', '0x42a71137C09AE83D8d05974960fd607d40033499', 'goerli');
Copy link
Contributor
@scott-silver scott-silver Jan 6, 2023

Choose a reason for hiding this comment

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

looks like there are a number of different instances of the WETH9 contract on Goerli.

Looks like Uniswap uses 0xB4FBF271143F4FBf7B91A5ded31805e42b2208d6 on Goerli: https://docs.uniswap.org/contracts/v3/reference/deployments

Copy link
Contributor Author
@kevincheng96 kevincheng96 Jan 10, 2023

Choose a reason for hiding this comment

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

Yeah, for some reason our Goerli USDC deployment has its own version of WETH. I chose to use that same WETH to be consistent.

@jflatow jflatow force-pushed the jflatow/mainnet-eth-live branch 2 times, most recently from 852c6d1 to d302b26 Compare January 10, 2023 22:19
@jflatow jflatow force-pushed the jflatow/mainnet-eth-live branch 5 times, most recently from ab43364 to 45907b1 Compare January 17, 2023 22:00
@kevincheng96 kevincheng96 force-pushed the kevin/goerli-weth-deploy branch from 3d6e9dc to a4f3e6a Compare January 17, 2023 22:58
@jflatow jflatow force-pushed the kevin/goerli-weth-deploy branch from a4f3e6a to 10a3eec Compare January 17, 2023 23:04
@kevincheng96 kevincheng96 merged commit a9de490 into jflatow/mainnet-eth-live Jan 17, 2023
@kevincheng96 kevincheng96 deleted the kevin/goerli-weth-deploy branch January 17, 2023 23:05
jflatow pushed a commit that referenced this pull request Jan 17, 2023
This PR adds a new `goerli-weth` base and deploy script for the Goerli cWETHv3 deploy.

The deploy script is very similar to the mainnet cWETHv3 script. The main differences are:
- Deploying price feeds for stETH / ETH and cbETH / ETH
- Pulling in extra testnet/cross-chain contracts such as `fauceteer` and `fxRoot` using `fromDep`
- Cloning over `cbETH` from mainnet
- Minting `cbETH` to `fauceteer`
jflatow added a commit that referenced this pull request Jan 24, 2023
* Proposal to enable cWETHv3 market

* Modify proposal to add rewards
* Update the supply caps based on Gauntlet recommendations (#629)
 https://hackmd.io/wncIvkFTReWUe2AMMK2ezA?view
* Refactor proposal to make market live (#640)

* If there are migrations, don't run the non-migrations

This allows a proposal branch to pass CI if the migration is required and is generally the intention for migration branches.

However an env var is still supported to override and force running the non-migration, too.

* Final changes to supply caps; proposal text; WETH transfer amt (#662)

Also copy bulker to usdc roots and fix scen test fixes

* Deploy cWETHv3 to Goerli (#646)

This PR adds a new `goerli-weth` base and deploy script for the Goerli cWETHv3 deploy.

The deploy script is very similar to the mainnet cWETHv3 script. The main differences are:
- Deploying price feeds for stETH / ETH and cbETH / ETH
- Pulling in extra testnet/cross-chain contracts such as `fauceteer` and `fxRoot` using `fromDep`
- Cloning over `cbETH` from mainnet
- Minting `cbETH` to `fauceteer`

* Modified migration from GitHub Actions

Co-authored-by: Kevin Cheng <kevincheng96@hotmail.com>
Co-authored-by: GitHub Actions Bot <>
jflatow added a commit that referenced this pull request Jan 24, 2023
* Mainnet ETH Deployment

* Update Github Actions to include mainnet-eth
* ETH-base bulker scenario
* WstETHPriceFeed + tests (#600)
* Non-ETH and ETH bulker scenarios (all actions in one txn)
* Mainnet WETH Bulker (#611)
* Add a supply cap constraint and set initial caps to 0 for cWETHv3
* Update the collateral params based on Gauntlet recommendations (#628)
 https://hackmd.io/wncIvkFTReWUe2AMMK2ezA?view
* Price feeds for WETH deployment + Bulker changes for OZ audit (#625)
 This PR implements and modifies price feeds to support the upcoming WETH deployment. The favored plan so far is to use ETH-denominated price feeds as opposed to USD price feeds, but stick with using 8 decimals for prices to avoid having to change the `Comet` and `Configurator` implementations.
 This would require:
  - A new wrapper price feed (`ScalingPriceFeed.sol`) that scales prices up or down to 8 decimals
  - A new `ConstantPriceFeed` that always returns 1e8 for the `WETH` base asset, since should always hold a 1:1 value with ETH
  - Modifications to the `WstETHPriceFeed` to return prices in terms of ETH instead of USD

 This is an alternative approach to #626, which is a more complex change but could be a better long-term solution.
 *Note: This PR also now contains the changes from #634 and #635, which address some suggestions made by OZ for their audit of `WstETHPriceFeed` and `Bulker`.*

* Refactor deploy to support second markets (#639)

* Refactor deploy to support second markets

* Add a fromDep to deployment manager which can be used for sharing dependency contracts from another deployment
* Always attempt to initialize storage if needed, and not necessarily as gov
* Always deploy an initial implementation contract instead of using the factory marker
* Only take gov admin actions if we own the cometAdmin
* Expose the primitives necessary for a proposal to configurate later, if/when admin cannot

* Add an env flag for migration constraint to skip non-migration scenarios
* Defer reading current config in modern constraint (bugfix)

* Add recipient to token sourcing blacklist

* WETH deployment audit changes (#641)

These are changes in response to the WETH deployment [audit](https://gist.github.com/andresbach/a01686b1ef8bdf51d46046f2cfc93307) conducted by OZ:

- **L01 - Lack of input validation** - We originally avoided zero address checks because there are a vast amount of accidental addresses that can be set here and checking for a specific one seems unnecessary. However, we have reconsidered that position after seeing the reasons for including a zero address check listed in this [post](https://forum.openzeppelin.com/t/removing-address-0x0-checks-from-openzeppelin-contracts/2222/13).
- **L02 - Improper implementation of Chainlink AggregatorV3Interface** - We updated the hard-coded return values for `roundId` and `answeredInRound` to be 1 to comply with the Chainlink specification.
- **L04 - Incomplete/confusing documentation** - We updated the documentation based on the provided suggestions.

Our full audit responses are [here](https://docs.google.com/document/d/1A8TrUdTtD7DXr1U5HOqSS1Xl81rjBZdlDC7LS4No4Bw).

* Spider USDC deployment before calling `fromDep` (#644)

We need to spider the `USDC` deployment before calling `fromDep` on it so all the aliases are populated in the cache. Otherwise, `fromDep` may try to import an `undefined` contract. 

There are many approaches to this and this PR takes the simplest approach of spidering in the deploy script. Another approach could be to handle the spidering directly in `fromDep`.

* Convert from scientific notation to bigint in configuration (#643)

This PR adds a new `ScientificNotation` string type to `NetworkConfiguration` that is required for certain fields (e.g. `supplyCap`, `baseMinForRewards`). The configuration parser will safely convert these values into bigints without losing precision; with one caveat, scientific notations that use decimals in the coefficient still run the risk of precision loss because those are converted directly to javascript numbers.

* Explicit supply cap constraint for failing bulker scen (#648)

* Add tracking index bounds unit tests for WETH (#645)

* Update params and rate model final numbers for deployment

* OnChainLiquidator (#642)

Update the reference liquidator to source from more pools and generally be more robust

* Fix fromDep to spider the dependency and load the contract from it (#652)

* bump targetReserves in LiquidationBotScenarios (#653)

* delete CURVE_REGISTRY (#654)

* OnChainLiquidator, not Liquidator (#655)

* Combined model based on Gauntlet's first and second recommendations (#656)

* Wrap and unwrap native token as base asset in Bulker (#659)

* Add tests for supply native token; clean up
* Change supplyStETH to use stETH amount instead of wstETH amount; add scenarios
* Tweak comments
* Supplying uint256.max collateral reverts in WETH deposit not Comet

* Modified deployment roots from GitHub Actions

* Proposal to enable cWETHv3 market (#608)

* Proposal to enable cWETHv3 market

* Modify proposal to add rewards
* Update the supply caps based on Gauntlet recommendations (#629)
 https://hackmd.io/wncIvkFTReWUe2AMMK2ezA?view
* Refactor proposal to make market live (#640)

* If there are migrations, don't run the non-migrations

This allows a proposal branch to pass CI if the migration is required and is generally the intention for migration branches.

However an env var is still supported to override and force running the non-migration, too.

* Final changes to supply caps; proposal text; WETH transfer amt (#662)

Also copy bulker to usdc roots and fix scen test fixes

* Deploy cWETHv3 to Goerli (#646)

This PR adds a new `goerli-weth` base and deploy script for the Goerli cWETHv3 deploy.

The deploy script is very similar to the mainnet cWETHv3 script. The main differences are:
- Deploying price feeds for stETH / ETH and cbETH / ETH
- Pulling in extra testnet/cross-chain contracts such as `fauceteer` and `fxRoot` using `fromDep`
- Cloning over `cbETH` from mainnet
- Minting `cbETH` to `fauceteer`

* Modified migration from GitHub Actions

Co-authored-by: Kevin Cheng <kevincheng96@hotmail.com>
Co-authored-by: scott-silver <scott@compound.finance>
Co-authored-by: GitHub Actions Bot <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployment This PR contains a new deployment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0