WstETHPriceFeed + tests #600

scott-silver · 2022-10-14T18:29:42Z

calculated price feed for wstETH / USD, using the stETH / USD price feed and the wstETH's tokensPerStEth function.

Implements the Chainlink AggregatorV3Interface, though it only implements the latestRoundData function that Comet uses.

scott-silver · 2022-10-14T18:31:38Z

contracts/WSTETHPriceFeed.sol

+        uint256 updatedAt,
+        uint80 answeredInRound
+    ) {
+        revert NotImplemented();


getRoundData is required for the AggregatorV3Interface, but it seems potentially harmful to return a value for this function since we don't actually know what the tokensPerStEth value was at the time that of a given stETH / USD price

contracts/WSTETHPriceFeed.sol

test/wsteth-price-feed.ts

contracts/WSTETHPriceFeed.sol

scott-silver · 2022-10-17T23:00:41Z

contracts/WstETHPriceFeed.sol

+    ) {
+        (uint80 roundId, int256 stETHPrice, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) = AggregatorV3Interface(stETHtoUSDPriceFeed).latestRoundData();
+        uint256 tokensPerStEth = IWstETH(wstETH).tokensPerStEth();
+        int price = stETHPrice * signed256(wstETHScale) / signed256(tokensPerStEth);


⛑️ Updated the logic of this line; I believe this is correct, but let me know if you agree.

The price is now:

stETHPrice * wstETHScale / tokensPerEth

e.g.:

1300e8 * 1e18 / .91e18 == ~1428e8

test/wsteth-price-feed.ts

kevincheng96

LGTM!

contracts/WstETHPriceFeed.sol

jflatow · 2022-10-19T17:19:31Z

test/absorb-test.ts

@@ -64,7 +64,7 @@ describe('absorb', function () {
    expect(lU1.numAbsorbed).to.be.equal(0);
    expect(lU1.approxSpend).to.be.equal(0);

-    const usdcPrice = await priceFeeds['USDC'].price();
+    const [_, usdcPrice] = await priceFeeds['USDC'].latestRoundData();


So we were accessing the stored price directly before? I guess latestRoundData is actually more correct since that's what we use in Comet and the official interface?

yeah, i had made price a public variable, which was a mistake.

all the variables are now internal and you have to access them via latestRoundData or getRoundData, which are the functions defined by the AggregatorV3Interface

* 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)

* 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`.*

* 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 <>

scott-silver requested review from jflatow, hayesgm, mykelp and kevincheng96 October 14, 2022 18:29

scott-silver commented Oct 14, 2022

View reviewed changes

contracts/WSTETHPriceFeed.sol Outdated Show resolved Hide resolved

scott-silver commented Oct 14, 2022

View reviewed changes

test/wsteth-price-feed.ts Show resolved Hide resolved

kevincheng96 reviewed Oct 17, 2022

View reviewed changes

contracts/WSTETHPriceFeed.sol Outdated Show resolved Hide resolved

kevincheng96 reviewed Oct 17, 2022

View reviewed changes

contracts/WSTETHPriceFeed.sol Outdated Show resolved Hide resolved

scott-silver commented Oct 17, 2022

View reviewed changes

kevincheng96 reviewed Oct 18, 2022

View reviewed changes

test/wsteth-price-feed.ts Show resolved Hide resolved

kevincheng96 approved these changes Oct 18, 2022

View reviewed changes

jflatow reviewed Oct 18, 2022

View reviewed changes

contracts/WstETHPriceFeed.sol Show resolved Hide resolved

scott-silver force-pushed the jflatow/mainnet-eth branch from 78896cf to 8de1aa8 Compare October 18, 2022 21:28

scott-silver force-pushed the silver/wstETH-USD-price-feed branch from 368f21a to 4ef3918 Compare October 18, 2022 21:50

jflatow reviewed Oct 19, 2022

View reviewed changes

scott-silver force-pushed the jflatow/mainnet-eth branch from 97ca2ec to c2d7ea6 Compare October 19, 2022 22:13

scott-silver added 12 commits October 19, 2022 20:04

WstETHPriceFeed + tests

ad64b6f

delete .only

f098e68

lint

902cbe1

casing on file name

e0d5a09

update logic

d8f7a10

store wstETHScale

82af944

pass along other parameters

6dd5d6e

update natspecs

3aca4cb

update revertedWith arg

9b67bf9

test roundId, startedAt, updatedAt, answeredInRound

4a97597

update tests

97eeff8

lint again

c5d8f06

scott-silver force-pushed the silver/wstETH-USD-price-feed branch from 218a47e to c5d8f06 Compare October 20, 2022 03:04

scott-silver merged commit 517b88e into jflatow/mainnet-eth Oct 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WstETHPriceFeed + tests #600

WstETHPriceFeed + tests #600

WstETHPriceFeed + tests #600

WstETHPriceFeed + tests #600

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment