Merge pull request #53 from fei-protocol/OZ-Audit-New-H03

Adds

contracts/bondingcurve/BondingCurve.sol

@@ -39,7 +39,8 @@ contract BondingCurve is IBondingCurve, OracleRef, PCVSplitter, Timed, Incentivi
     /// @param _core Fei Core to reference
     /// @param _pcvDeposits the PCV Deposits for the PCVSplitter
     /// @param _ratios the ratios for the PCVSplitter
-    /// @param _oracle the UniswapOracle to reference
+    /// @param _oracle the oracle to reference
+    /// @param _backupOracle the backup oracle to reference
     /// @param _duration the duration between incentivizing allocations
     /// @param _incentive the amount rewarded to the caller of an allocation
     /// @param _token the ERC20 token associated with this curve, null if ETH
@@ -48,6 +49,7 @@ contract BondingCurve is IBondingCurve, OracleRef, PCVSplitter, Timed, Incentivi
     constructor(
         address _core,
         address _oracle,
+        address _backupOracle,
         uint256 _scale,
         address[] memory _pcvDeposits,
         uint256[] memory _ratios,
@@ -57,7 +59,7 @@ contract BondingCurve is IBondingCurve, OracleRef, PCVSplitter, Timed, Incentivi
         uint256 _discount,
         uint256 _buffer
     )
-        OracleRef(_core, _oracle)
+        OracleRef(_core, _oracle, _backupOracle, 0, false)
         PCVSplitter(_pcvDeposits, _ratios)
         Timed(_duration)
         Incentivized(_incentive)
@@ -68,6 +70,10 @@ contract BondingCurve is IBondingCurve, OracleRef, PCVSplitter, Timed, Incentivi
         buffer = _buffer;
 
         _initTimed();
+
+        if (address(_token) != address(0)) {
+            _setDecimalsNormalizerFromToken(address(_token));
+        }
     }
 
     /// @notice purchase FEI for underlying tokens

contracts/bondingcurve/EthBondingCurve.sol

@@ -20,11 +20,13 @@ contract EthBondingCurve is BondingCurve {
     constructor(
         address core,
         address oracle,
+        address backupOracle,
         BondingCurveParams memory params
     )
         BondingCurve(
             core,
             oracle,
+            backupOracle,
             params.scale,
             params.pcvDeposits,
             params.ratios,

contracts/mock/MockOracle.sol

@@ -17,9 +17,8 @@ contract MockOracle is IOracle {
         _usdPerEth = usdPerEth;
     }
 
-    function update() public override returns (bool) {
+    function update() public override {
         updated = true;
-        return true;
     }
 
     function read() public view override returns (Decimal.D256 memory, bool) {

contracts/oracle/ChainlinkOracleWrapper.sol

@@ -36,10 +36,8 @@ contract ChainlinkOracleWrapper is IOracle, CoreRef {
     }
 
     /// @notice updates the oracle price
-    /// @return true if oracle is updated and false if unchanged
-    function update() external view override whenNotPaused returns (bool) {
-        return false;
-    }
+    /// @dev no-op, Chainlink is updated automatically
+    function update() external view override whenNotPaused {}
 
     /// @notice determine if read value is stale
     /// @return true if read value is stale

contracts/oracle/CompositeOracle.sol

@@ -28,9 +28,9 @@ contract CompositeOracle is IOracle, CoreRef {
     }
 
     /// @notice updates the oracle price
-    /// @return true if oracle is updated and false if unchanged
-    function update() external override whenNotPaused returns (bool) {
-        return oracleA.update() && oracleB.update();
+    function update() external override whenNotPaused {
+        oracleA.update();
+        oracleB.update();
     }
 
     /// @notice determine if read value is stale

contracts/oracle/IOracle.sol

@@ -12,7 +12,7 @@ interface IOracle {
 
     // ----------- State changing API -----------
 
-    function update() external returns (bool);
+    function update() external;
 
     // ----------- Getters -----------
 

contracts/oracle/UniswapOracle.sol

@@ -53,8 +53,7 @@ contract UniswapOracle is IUniswapOracle, CoreRef {
     }
 
     /// @notice updates the oracle price
-    /// @return true if oracle is updated and false if unchanged
-    function update() external override whenNotPaused returns (bool) {
+    function update() external override whenNotPaused {
         (
             uint256 price0Cumulative,
             uint256 price1Cumulative,
@@ -67,7 +66,7 @@ contract UniswapOracle is IUniswapOracle, CoreRef {
         }
 
         if (deltaTimestamp < duration) {
-            return false;
+            return;
         }
 
         uint256 currentCumulative = _getCumulative(price0Cumulative, price1Cumulative);
@@ -90,8 +89,6 @@ contract UniswapOracle is IUniswapOracle, CoreRef {
         priorCumulative = currentCumulative;
 
         emit Update(_twap.asUint256());
-
-        return true;
     }
 
     /// @notice determine if read value is stale

contracts/pcv/PCVSwapperUniswap.sol

@@ -7,22 +7,20 @@ import "../refs/OracleRef.sol";
 import "../utils/Timed.sol";
 import "../external/UniswapV2Library.sol";
 import "@openzeppelin/contracts/utils/math/Math.sol";
-import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
+import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
 import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import "@uniswap/v2-periphery/contracts/interfaces/IWETH.sol";
 import "@uniswap/v2-core/contracts/interfaces/IUniswapV2Pair.sol";
 
 /// @title implementation for PCV Swapper that swaps ERC20 tokens on Uniswap
 /// @author eswak
 contract PCVSwapperUniswap is IPCVSwapper, OracleRef, Timed, Incentivized {
-    using SafeERC20 for ERC20;
+    using SafeERC20 for IERC20;
     using Decimal for Decimal.D256;
 
     // ----------- Events -----------	
     event UpdateMaximumSlippage(uint256 maximumSlippage);	
     event UpdateMaxSpentPerSwap(uint256 maxSpentPerSwap);	
-    event UpdateInvertOraclePrice(bool invertOraclePrice);	
-    event UpdateSwapIncentiveAmount(uint256 swapIncentiveAmount);
 
     /// @notice the token to spend on swap (outbound)
     address public immutable override tokenSpent;
@@ -32,8 +30,6 @@ contract PCVSwapperUniswap is IPCVSwapper, OracleRef, Timed, Incentivized {
     address public override tokenReceivingAddress;
     /// @notice the maximum amount of tokens to spend on every swap
     uint256 public maxSpentPerSwap;
-    /// @notice should we use (1 / oraclePrice) instead of oraclePrice ?
-    bool public invertOraclePrice;
     /// @notice the maximum amount of slippage vs oracle price
     uint256 public maximumSlippageBasisPoints;
     uint256 public constant BASIS_POINTS_GRANULARITY = 10_000;
@@ -44,12 +40,17 @@ contract PCVSwapperUniswap is IPCVSwapper, OracleRef, Timed, Incentivized {
     // solhint-disable-next-line var-name-mixedcase
     address public immutable WETH;
 
+    struct OracleAddresses {
+        address _oracle;
+        address _backupOracle;
+    }
+
     constructor(
         address _core,
         IUniswapV2Pair _pair,
         // solhint-disable-next-line var-name-mixedcase
         address _WETH,
-        address _oracle,
+        OracleAddresses memory oracleAddresses,
         uint256 _swapFrequency,
         address _tokenSpent,
         address _tokenReceived,
@@ -58,15 +59,20 @@ contract PCVSwapperUniswap is IPCVSwapper, OracleRef, Timed, Incentivized {
         uint256 _maximumSlippageBasisPoints,
         bool _invertOraclePrice,
         uint256 _swapIncentiveAmount
-    ) OracleRef(_core, _oracle) Timed(_swapFrequency) Incentivized(_swapIncentiveAmount) {
+    ) OracleRef(
+      _core, 
+      oracleAddresses._oracle, 
+      oracleAddresses._backupOracle,
+      int256(uint256(IERC20Metadata(_tokenSpent).decimals())) - int256(uint256(IERC20Metadata(_tokenReceived).decimals())),
+      _invertOraclePrice
+    ) Timed(_swapFrequency) Incentivized(_swapIncentiveAmount) {
         pair = _pair;
         WETH = _WETH;
         tokenSpent = _tokenSpent;
         tokenReceived = _tokenReceived;
         tokenReceivingAddress = _tokenReceivingAddress;
         maxSpentPerSwap = _maxSpentPerSwap;
         maximumSlippageBasisPoints = _maximumSlippageBasisPoints;
-        invertOraclePrice = _invertOraclePrice;
 
         // start timer
         _initTimed();
@@ -106,7 +112,7 @@ contract PCVSwapperUniswap is IPCVSwapper, OracleRef, Timed, Incentivized {
 
     /// @notice Reads the balance of tokenReceived held in the contract
     function balance() external view override returns(uint256) {
-      return ERC20(tokenReceived).balanceOf(address(this));
+      return IERC20(tokenReceived).balanceOf(address(this));
     }
 
     // =======================================================================
@@ -131,7 +137,7 @@ contract PCVSwapperUniswap is IPCVSwapper, OracleRef, Timed, Incentivized {
       address to, 
       uint256 amount
     ) public override onlyPCVController {
-        ERC20(token).safeTransfer(to, amount);
+        IERC20(token).safeTransfer(to, amount);
         emit WithdrawERC20(msg.sender, token, to, amount);
     }
 
@@ -167,12 +173,6 @@ contract PCVSwapperUniswap is IPCVSwapper, OracleRef, Timed, Incentivized {
        _setDuration(_duration);
     }
 
-    /// @notice sets invertOraclePrice : use (1 / oraclePrice) if true
-    function setInvertOraclePrice(bool _invertOraclePrice) external onlyGovernor {
-        invertOraclePrice = _invertOraclePrice;
-	      emit UpdateInvertOraclePrice(_invertOraclePrice);	
-    }
-
     // =======================================================================
     // External functions
     // =======================================================================
@@ -181,11 +181,8 @@ contract PCVSwapperUniswap is IPCVSwapper, OracleRef, Timed, Incentivized {
     function swap() external override afterTime whenNotPaused {
 	    // Reset timer	
       _initTimed();	
-      // Update oracle, if necessary	
-      if (oracle.isOutdated()) {	
-        bool updated = updateOracle();	
-        require(updated, "PCVSwapperUniswap: cannot update outdated oracle.");	
-      }
+
+      updateOracle();
 
       uint256 amountIn = _getExpectedAmountIn();
       uint256 amountOut = _getExpectedAmountOut(amountIn);
@@ -196,7 +193,7 @@ contract PCVSwapperUniswap is IPCVSwapper, OracleRef, Timed, Incentivized {
       require(minimumAcceptableAmountOut <= amountOut, "PCVSwapperUniswap: slippage too high.");
 
       // Perform swap
-      ERC20(tokenSpent).safeTransfer(address(pair), amountIn);
+      IERC20(tokenSpent).safeTransfer(address(pair), amountIn);
       (uint256 amount0Out, uint256 amount1Out) =
           pair.token0() == address(tokenSpent)
               ? (uint256(0), amountOut)
@@ -222,7 +219,7 @@ contract PCVSwapperUniswap is IPCVSwapper, OracleRef, Timed, Incentivized {
 
     /// @notice see external function getNextAmountSpent()
     function _getExpectedAmountIn() internal view returns (uint256) {
-      uint256 balance = ERC20(tokenSpent).balanceOf(address(this));
+      uint256 balance = IERC20(tokenSpent).balanceOf(address(this));
       require(balance != 0, "PCVSwapperUniswap: no tokenSpent left.");
       return Math.min(maxSpentPerSwap, balance);
     }
@@ -249,35 +246,9 @@ contract PCVSwapperUniswap is IPCVSwapper, OracleRef, Timed, Incentivized {
     /// @notice see external function getNextAmountReceivedThreshold()
     function _getMinimumAcceptableAmountOut(uint256 amountIn) internal view returns (uint256) {
       Decimal.D256 memory twap = readOracle();
-      if (invertOraclePrice) {
-        twap = invert(twap);
-      }
       Decimal.D256 memory oracleAmountOut = twap.mul(amountIn);
       Decimal.D256 memory maxSlippage = Decimal.ratio(BASIS_POINTS_GRANULARITY - maximumSlippageBasisPoints, BASIS_POINTS_GRANULARITY);
-      (uint256 decimalNormalizer, bool normalizerDirection) = _getDecimalNormalizer();
-      Decimal.D256 memory oraclePriceMinusSlippage;
-      if (normalizerDirection) {
-        oraclePriceMinusSlippage = maxSlippage.mul(oracleAmountOut).div(decimalNormalizer);
-      } else {
-        oraclePriceMinusSlippage = maxSlippage.mul(oracleAmountOut).mul(decimalNormalizer);
-      }
+      Decimal.D256 memory oraclePriceMinusSlippage = maxSlippage.mul(oracleAmountOut);
       return oraclePriceMinusSlippage.asUint256();
     }
-
-    /// @notice see external function getDecimalNormalizer()
-    function _getDecimalNormalizer() internal view returns (uint256, bool) {
-      uint8 decimalsTokenSpent = ERC20(tokenSpent).decimals();
-      uint8 decimalsTokenReceived = ERC20(tokenReceived).decimals();
-
-      uint256 n;
-      bool direction;
-      if (decimalsTokenSpent >= decimalsTokenReceived) {
-        direction = true;
-        n = uint256(10) ** uint256(decimalsTokenSpent - decimalsTokenReceived);
-      } else {
-        direction = false;
-        n = uint256(10) ** uint256(decimalsTokenReceived - decimalsTokenSpent);
-      }
-      return (n, direction);
-    }
 }

contracts/pcv/UniswapPCVController.sol

@@ -26,6 +26,7 @@ contract UniswapPCVController is IUniswapPCVController, UniRef, Timed, Incentivi
     /// @param _core Fei Core for reference
     /// @param _pcvDeposit PCV Deposit to reweight
     /// @param _oracle oracle for reference
+    /// @param _backupOracle the backup oracle to reference
     /// @param _incentiveAmount amount of FEI for triggering a reweight
     /// @param _minDistanceForReweightBPs minimum distance from peg to reweight in basis points
     /// @param _pair Uniswap pair contract to reweight
@@ -34,11 +35,12 @@ contract UniswapPCVController is IUniswapPCVController, UniRef, Timed, Incentivi
         address _core,
         address _pcvDeposit,
         address _oracle,
+        address _backupOracle,
         uint256 _incentiveAmount,
         uint256 _minDistanceForReweightBPs,
         address _pair,
         uint256 _reweightFrequency
-    ) UniRef(_core, _pair, _oracle) Timed(_reweightFrequency) Incentivized(_incentiveAmount) {
+    ) UniRef(_core, _pair, _oracle, _backupOracle) Timed(_reweightFrequency) Incentivized(_incentiveAmount) {
         pcvDeposit = IPCVDeposit(_pcvDeposit);
         emit PCVDepositUpdate(address(0), _pcvDeposit);
 

contracts/pcv/UniswapPCVDeposit.sol

@@ -24,14 +24,16 @@ contract UniswapPCVDeposit is IUniswapPCVDeposit, UniRef {
     /// @param _pair Uniswap Pair to deposit to
     /// @param _router Uniswap Router
     /// @param _oracle oracle for reference
+    /// @param _backupOracle the backup oracle to reference
     /// @param _maxBasisPointsFromPegLP the max basis points of slippage from peg allowed on LP deposit
     constructor(
         address _core,
         address _pair,
         address _router,
         address _oracle,
+        address _backupOracle,
         uint256 _maxBasisPointsFromPegLP
-    ) UniRef(_core, _pair, _oracle) {
+    ) UniRef(_core, _pair, _oracle, _backupOracle) {
         router = IUniswapV2Router02(_router);
 
         _approveToken(address(fei()));

contracts/refs/IOracleRef.sol

@@ -10,25 +10,37 @@ interface IOracleRef {
 
     event OracleUpdate(address indexed oldOracle, address indexed newOracle);
 
+    event InvertUpdate(bool oldDoInvert, bool newDoInvert);
+
+    event DecimalsNormalizerUpdate(int256 oldDecimalsNormalizer, int256 newDecimalsNormalizer);
+
     event BackupOracleUpdate(address indexed oldBackupOracle, address indexed newBackupOracle);
 
 
     // ----------- State changing API -----------
 
-    function updateOracle() external returns (bool);
+    function updateOracle() external;
 
     // ----------- Governor only state changing API -----------
 
     function setOracle(address newOracle) external;
 
     function setBackupOracle(address newBackupOracle) external;
 
+    function setDecimalsNormalizer(int256 newDecimalsNormalizer) external;
+
+    function setDoInvert(bool newDoInvert) external;
+
     // ----------- Getters -----------
 
     function oracle() external view returns (IOracle);
 
     function backupOracle() external view returns (IOracle);
 
+    function doInvert() external view returns (bool);
+
+    function decimalsNormalizer() external view returns (int256);
+
     function readOracle() external view returns (Decimal.D256 memory);
 
     function invert(Decimal.D256 calldata price)