Skip to content

Commit

Permalink
Price feeds for WETH deployment + Bulker changes for OZ audit (#625)
Browse files Browse the repository at this point in the history
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`.*
  • Loading branch information
kevincheng96 committed Dec 13, 2022
1 parent 817b942 commit 6723f1a
Show file tree
Hide file tree
Showing 21 changed files with 718 additions and 110 deletions.
8 changes: 4 additions & 4 deletions contracts/Comet.sol
Expand Up @@ -3,7 +3,7 @@ pragma solidity 0.8.15;

import "./CometMainInterface.sol";
import "./ERC20.sol";
import "./vendor/@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";
import "./IPriceFeed.sol";

/**
* @title Compound's Comet Contract
Expand Down Expand Up @@ -144,7 +144,7 @@ contract Comet is CometMainInterface {
if (config.storeFrontPriceFactor > FACTOR_SCALE) revert BadDiscount();
if (config.assetConfigs.length > MAX_ASSETS) revert TooManyAssets();
if (config.baseMinForRewards == 0) revert BadMinimum();
if (AggregatorV3Interface(config.baseTokenPriceFeed).decimals() != PRICE_FEED_DECIMALS) revert BadDecimals();
if (IPriceFeed(config.baseTokenPriceFeed).decimals() != PRICE_FEED_DECIMALS) revert BadDecimals();

// Copy configuration
unchecked {
Expand Down Expand Up @@ -240,7 +240,7 @@ contract Comet is CometMainInterface {
}

// Sanity check price feed and asset decimals
if (AggregatorV3Interface(priceFeed).decimals() != PRICE_FEED_DECIMALS) revert BadDecimals();
if (IPriceFeed(priceFeed).decimals() != PRICE_FEED_DECIMALS) revert BadDecimals();
if (ERC20(asset).decimals() != decimals_) revert BadDecimals();

// Ensure collateral factors are within range
Expand Down Expand Up @@ -471,7 +471,7 @@ contract Comet is CometMainInterface {
* @return The price, scaled by `PRICE_SCALE`
*/
function getPrice(address priceFeed) override public view returns (uint256) {
(, int price, , , ) = AggregatorV3Interface(priceFeed).latestRoundData();
(, int price, , , ) = IPriceFeed(priceFeed).latestRoundData();
if (price <= 0) revert BadPrice();
return uint256(price);
}
Expand Down
1 change: 0 additions & 1 deletion contracts/CometCore.sol
Expand Up @@ -4,7 +4,6 @@ pragma solidity 0.8.15;
import "./CometConfiguration.sol";
import "./CometStorage.sol";
import "./CometMath.sol";
import "./vendor/@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";

abstract contract CometCore is CometConfiguration, CometStorage, CometMath {
struct AssetInfo {
Expand Down
45 changes: 45 additions & 0 deletions contracts/ConstantPriceFeed.sol
@@ -0,0 +1,45 @@
// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.15;

import "./IPriceFeed.sol";

contract ConstantPriceFeed is IPriceFeed {
/// @notice Version of the price feed
uint public constant override version = 1;

/// @notice Description of the price feed
string public constant description = "Constant price feed";

/// @notice Number of decimals for returned prices
uint8 public immutable override decimals;

/// @notice The constant price
int public immutable constantPrice;

/**
* @notice Construct a new scaling price feed
* @param decimals_ The number of decimals for the returned prices
**/
constructor(uint8 decimals_, int256 constantPrice_) {
decimals = decimals_;
constantPrice = constantPrice_;
}

/**
* @notice Price for the latest round
* @return roundId Round id from the underlying price feed
* @return answer Latest price for the asset (will always be a constant price)
* @return startedAt Timestamp when the round was started; passed on from underlying price feed
* @return updatedAt Timestamp when the round was last updated; passed on from underlying price feed
* @return answeredInRound Round id in which the answer was computed; passed on from underlying price feed
**/
function latestRoundData() external view returns (
uint80 roundId,
int256 answer,
uint256 startedAt,
uint256 updatedAt,
uint80 answeredInRound
) {
return (0, constantPrice, block.timestamp, block.timestamp, 0);
}
}
14 changes: 14 additions & 0 deletions contracts/IERC20NonStandard.sol
@@ -0,0 +1,14 @@
// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.15;

/**
* @title IERC20NonStandard
* @dev Version of ERC20 with no return values for `transfer` and `transferFrom`
* See https://medium.com/coinmonks/missing-return-value-bug-at-least-130-tokens-affected-d67bf08521ca
*/
interface IERC20NonStandard {
function approve(address spender, uint256 amount) external;
function transfer(address to, uint256 value) external;
function transferFrom(address from, address to, uint256 value) external;
function balanceOf(address account) external view returns (uint256);
}
25 changes: 25 additions & 0 deletions contracts/IPriceFeed.sol
@@ -0,0 +1,25 @@
// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.15;

/**
* @dev Interface for price feeds used by Comet
* Note This is Chainlink's AggregatorV3Interface, but without the `getRoundData` function.
*/
interface IPriceFeed {
function decimals() external view returns (uint8);

function description() external view returns (string memory);

function version() external view returns (uint256);

function latestRoundData()
external
view
returns (
uint80 roundId,
int256 answer,
uint256 startedAt,
uint256 updatedAt,
uint80 answeredInRound
);
}
81 changes: 81 additions & 0 deletions contracts/ScalingPriceFeed.sol
@@ -0,0 +1,81 @@
// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.15;

import "./vendor/@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";
import "./IPriceFeed.sol";

contract ScalingPriceFeed is IPriceFeed {
/** Custom errors **/
error InvalidInt256();

/// @notice Version of the price feed
uint public constant override version = 1;

/// @notice Description of the price feed
string public description;

/// @notice Number of decimals for returned prices
uint8 public immutable override decimals;

/// @notice Underlying Chainlink price feed where prices are fetched from
address public immutable underlyingPriceFeed;

/// @notice Whether or not the price should be upscaled
bool internal immutable shouldUpscale;

/// @notice The amount to upscale or downscale the price by
int256 internal immutable rescaleFactor;

/**
* @notice Construct a new scaling price feed
* @param underlyingPriceFeed_ The address of the underlying price feed to fetch prices from
* @param decimals_ The number of decimals for the returned prices
**/
constructor(address underlyingPriceFeed_, uint8 decimals_) {
underlyingPriceFeed = underlyingPriceFeed_;
decimals = decimals_;
description = AggregatorV3Interface(underlyingPriceFeed_).description();

uint8 chainlinkPriceFeedDecimals = AggregatorV3Interface(underlyingPriceFeed_).decimals();
// Note: Solidity does not allow setting immutables in if/else statements
shouldUpscale = chainlinkPriceFeedDecimals < decimals_ ? true : false;
rescaleFactor = (shouldUpscale
? signed256(10 ** (decimals_ - chainlinkPriceFeedDecimals))
: signed256(10 ** (chainlinkPriceFeedDecimals - decimals_))
);
}

/**
* @notice Price for the latest round
* @return roundId Round id from the underlying price feed
* @return answer Latest price for the asset in terms of ETH
* @return startedAt Timestamp when the round was started; passed on from underlying price feed
* @return updatedAt Timestamp when the round was last updated; passed on from underlying price feed
* @return answeredInRound Round id in which the answer was computed; passed on from underlying price feed
**/
function latestRoundData() override external view returns (
uint80 roundId,
int256 answer,
uint256 startedAt,
uint256 updatedAt,
uint80 answeredInRound
) {
(uint80 roundId_, int256 price, uint256 startedAt_, uint256 updatedAt_, uint80 answeredInRound_) = AggregatorV3Interface(underlyingPriceFeed).latestRoundData();
return (roundId_, scalePrice(price), startedAt_, updatedAt_, answeredInRound_);
}

function signed256(uint256 n) internal pure returns (int256) {
if (n > uint256(type(int256).max)) revert InvalidInt256();
return int256(n);
}

function scalePrice(int256 price) internal view returns (int256) {
int256 scaledPrice;
if (shouldUpscale) {
scaledPrice = price * rescaleFactor;
} else {
scaledPrice = price / rescaleFactor;
}
return scaledPrice;
}
}
67 changes: 32 additions & 35 deletions contracts/WstETHPriceFeed.sol
Expand Up @@ -2,37 +2,45 @@
pragma solidity 0.8.15;

import "./vendor/@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";
import "./IPriceFeed.sol";
import "./IWstETH.sol";

interface IWstETH {
function decimals() external view returns (uint8);
function tokensPerStEth() external view returns (uint256);
}

contract WstETHPriceFeed is AggregatorV3Interface {
contract WstETHPriceFeed is IPriceFeed {
/** Custom errors **/
error BadDecimals();
error InvalidInt256();
error NotImplemented();

string public constant override description = "Custom price feed for wstETH / USD";

/// @notice Version of the price feed
uint public constant override version = 1;

/// @notice Scale for returned prices
uint8 public override decimals = 8;
/// @notice Description of the price feed
string public constant override description = "Custom price feed for wstETH / ETH";

/// @notice Number of decimals for returned prices
uint8 public immutable override decimals;

/// @notice Chainlink stETH / ETH price feed
address public immutable stETHtoETHPriceFeed;

/// @notice Chainlink stETH / USD price feed
address public immutable stETHtoUSDPriceFeed;
/// @notice Number of decimals for the stETH / ETH price feed
uint public immutable stETHToETHPriceFeedDecimals;

/// @notice WstETH contract address
address public immutable wstETH;

/// @notice scale for WstETH contract
uint public immutable wstETHScale;
/// @notice Scale for WstETH contract
int public immutable wstETHScale;

constructor(address stETHtoUSDPriceFeed_, address wstETH_) {
stETHtoUSDPriceFeed = stETHtoUSDPriceFeed_;
constructor(address stETHtoETHPriceFeed_, address wstETH_, uint8 decimals_) {
stETHtoETHPriceFeed = stETHtoETHPriceFeed_;
stETHToETHPriceFeedDecimals = AggregatorV3Interface(stETHtoETHPriceFeed_).decimals();
wstETH = wstETH_;
wstETHScale = 10 ** IWstETH(wstETH).decimals();
// Note: Safe to convert directly to an int256 because wstETH.decimals == 18
wstETHScale = int256(10 ** IWstETH(wstETH).decimals());

// Note: stETH / ETH price feed has 18 decimals so `decimals_` should always be less than or equals to that
if (decimals_ > stETHToETHPriceFeedDecimals) revert BadDecimals();
decimals = decimals_;
}

function signed256(uint256 n) internal pure returns (int256) {
Expand All @@ -41,20 +49,7 @@ contract WstETHPriceFeed is AggregatorV3Interface {
}

/**
* @notice Unimplemented function required to fulfill AggregatorV3Interface; always reverts
**/
function getRoundData(uint80 _roundId) override external view returns (
uint80 roundId,
int256 answer,
uint256 startedAt,
uint256 updatedAt,
uint80 answeredInRound
) {
revert NotImplemented();
}

/**
* @notice WstETH Price for the latest round
* @notice WstETH price for the latest round
* @return roundId Round id from the stETH price feed
* @return answer Latest price for wstETH / USD
* @return startedAt Timestamp when the round was started; passed on from stETH price feed
Expand All @@ -68,9 +63,11 @@ contract WstETHPriceFeed is AggregatorV3Interface {
uint256 updatedAt,
uint80 answeredInRound
) {
(uint80 roundId, int256 stETHPrice, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) = AggregatorV3Interface(stETHtoUSDPriceFeed).latestRoundData();
(uint80 roundId_, int256 stETHPrice, uint256 startedAt_, uint256 updatedAt_, uint80 answeredInRound_) = AggregatorV3Interface(stETHtoETHPriceFeed).latestRoundData();
uint256 tokensPerStEth = IWstETH(wstETH).tokensPerStEth();
int256 price = stETHPrice * int256(wstETHScale) / signed256(tokensPerStEth);
return (roundId, price, startedAt, updatedAt, answeredInRound);
int256 price = stETHPrice * wstETHScale / signed256(tokensPerStEth);
// Note: Assumes the stETH price feed has an equal or larger amount of decimals than this price feed
int256 scaledPrice = price / int256(10 ** (stETHToETHPriceFeedDecimals - decimals));
return (roundId_, scaledPrice, startedAt_, updatedAt_, answeredInRound_);
}
}

0 comments on commit 6723f1a

Please sign in to comment.