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
Mainnet WETH Bulker #611
Mainnet WETH Bulker #611
Conversation
|
||
interface IClaimable { | ||
function claim(address comet, address src, bool shouldAccrue) external; | ||
|
||
function claimTo(address comet, address src, address to, bool shouldAccrue) external; | ||
} | ||
|
||
contract Bulker { | ||
contract BaseBulker { |
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.
Can't comment on line 35 so going to comment here instead. Should we go ahead and make the base even more generic by renaming weth
to wrappedNativeToken
?
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.
Yeah, it's a good question. wrappedNativeToken
will be the more accurate name once we're using the bulker for wrapped Matic and wrapped Avax.
The downside is that it will break the existing interface. With these changes, the existing, deployed Bulkers and the new MainnetBulker have the same interface (the MainnetBulker just adds a couple additional actions).
But if we update the constants to be ACTION_SUPPLY_NATIVE_TOKEN
instead of ACTION_SUPPLY_ETH
, then we'll need to either write deployment-specific scenarios or replace the deployed bulkers.
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.
IMO it's fine to temporarily have both interfaces in the repo so scenarios can still use the old Bulker interface until we complete the migration on-chain. The cost of not making this change is that we'll either need a separate BaseBulker
for the other chains or the bulker on other chains will incorrectly refer to WMATIC
or WAVAX
as WETH
.
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.
sounds good. updated all references to eth
/weth
to be nativeToken
/wrappedNativeToken
contracts/bulkers/MainnetBulker.sol
Outdated
address payable steth_, | ||
address payable wsteth_ | ||
) BaseBulker(admin_, weth_) { | ||
steth = steth_; |
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.
You can derive this from wsteth.stETH()
.
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.
good call; updated (and made the steth
and wsteth
address not payable
)
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.
Good catch, didn't catch the payable
on my end
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.
Nice, the contract looks good so far! Just a few small comments
scenario/BulkerScenario.ts
Outdated
); | ||
const calldata = [supplyGalacticCreditsCalldata]; | ||
const actions = [ | ||
ethers.utils.formatBytes32String('ACTION_SUPPLY_GALACTIC_CREDITS') |
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.
👽: Comet doesn't support Galactic Credits?!
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's an outrage! just wait until the Emperor hears about this
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.
Awesome
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.
Looks good 💪
contracts/bulkers/MainnetBulker.sol
Outdated
address payable wsteth_ | ||
) BaseBulker(admin_, weth_) { | ||
steth = steth_; | ||
address payable wrappedNativeToken_, |
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.
nit: assuming this Bulker is for Ethereum mainnet, it should be fine to name this weth
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.
yep, you're right. updated
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
* 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`.*
* 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`.*
* 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`.*
* 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`.*
* 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`.*
* 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`.*
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.
test/helpers.ts
* 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 <>
Creating a new Bulker that is able to wrap/unwrap
stETH
.Includes the following changes:
bulkers/
subdirectory, renaming it toBaseBulker
MainnetBulker
that inherits fromBaseBulker
and addssupplyStEthTo
andwithdrawStEthTo
functionshandleAction
function that contracts inheriting fromBaseBulker
can implement; by default it just revertsMainnetBulker
TODO: