AaveV3LeverageStrategyExtension audit feedback (#142)

* Test showing that settting Emode does not affect getReserveConfigurationData

* Start adjusting StrategyExtension test to switcht to wsteth/eth pair

* Fixing tests

* Fix test

* First test case for rebalancing in e-mode

* Fix handling of emode ltv / liquidationThreshold

* Add test case for wrong repay threshold

* Fix tests

* Remove subtraction of unutilizedLeveragePercentage when delevering

* Add test for targetLeverageRatio verification

* Add verification step for targetLeverageRatio >= 1

* Switch div / mul order in _calculateMinRepayUnits

* Switch div / mul order in calculateChunkRebalanceNotional

* Switch to latestRoundData on chainlink call

* Add check for maximum oracle price age

* Add tests for outdated price response from oracle

* Fix tests

* Add method to override noRebalanceInProgress modifier

* Fix tests

* Use AaveOracle instead of configured chainlink oracles

* Get AaveOracle address from AddressProvider instead of deploy argument

* Change to get aaveOracle address on the fly from addressProvider

* Clean up and additional code comments

* fix(dependencies): Add post audit Aave V3 dependencies (#145)

update post audit contracts for integration tests

Co-authored-by: Pranav Bhardwaj <[email protected]>

* Updates to test  (#146)

* remove test filter on FlashMintLeveraged.

* chore: update current block

* test: updates to deployment of leverage module.

* Fix failing tests for AaveV3LeverageStrategyExtension (#147)

* Fix failing tests

* Adjust test message for ripcord test

---------

Co-authored-by: christn <[email protected]>

* remove .only from test

* test: fix issues with updating block.

Made deadline on uniswap router swaps less brittle.

---------

Co-authored-by: pblivin0x <[email protected]>
Co-authored-by: Pranav Bhardwaj <[email protected]>
Co-authored-by: SnakePoison <[email protected]>

Author: 4 people

contracts/adapters/AaveLeverageStrategyExtension.sol

@@ -189,7 +189,7 @@ contract AaveLeverageStrategyExtension is BaseExtension {
     /**
      * Throws if rebalance is currently in TWAP`
      */
-    modifier noRebalanceInProgress() {
+    modifier noRebalanceInProgress() virtual {
         require(twapLeverageRatio == 0, "Rebalance is currently in progress");
         _;
     }
@@ -886,7 +886,7 @@ contract AaveLeverageStrategyExtension is BaseExtension {
      *
      * return ActionInfo                Struct containing data used by internal lever and delever functions
      */
-    function _createActionInfo() internal view returns(ActionInfo memory) {
+    function _createActionInfo() internal view virtual returns(ActionInfo memory) {
         ActionInfo memory rebalanceInfo;
 
         // Calculate prices from chainlink. Chainlink returns prices with 8 decimal places, but we need 36 - underlyingDecimals decimal places.
@@ -915,6 +915,7 @@ contract AaveLeverageStrategyExtension is BaseExtension {
         IncentiveSettings memory _incentive
     )
         internal
+        virtual
         pure
     {
         require (
@@ -1060,6 +1061,7 @@ contract AaveLeverageStrategyExtension is BaseExtension {
     )
         internal
         view
+        virtual
         returns (uint256, uint256)
     {
         // Calculate absolute value of difference between new and current leverage ratio
@@ -1092,7 +1094,7 @@ contract AaveLeverageStrategyExtension is BaseExtension {
      *
      * return uint256          Max borrow notional denominated in collateral asset
      */
-    function _calculateMaxBorrowCollateral(ActionInfo memory _actionInfo, bool _isLever) internal view returns(uint256) {
+    function _calculateMaxBorrowCollateral(ActionInfo memory _actionInfo, bool _isLever) internal virtual view returns(uint256) {
 
         // Retrieve collateral factor and liquidation threshold for the collateral asset in precise units (1e16 = 1%)
         ( , uint256 maxLtvRaw, uint256 liquidationThresholdRaw, , , , , , ,) = strategy.aaveProtocolDataProvider.getReserveConfigurationData(address(strategy.collateralAsset));
@@ -1144,7 +1146,7 @@ contract AaveLeverageStrategyExtension is BaseExtension {
      *
      * return uint256           Min position units to repay in borrow asset
      */
-    function _calculateMinRepayUnits(uint256 _collateralRebalanceUnits, uint256 _slippageTolerance, ActionInfo memory _actionInfo) internal pure returns (uint256) {
+    function _calculateMinRepayUnits(uint256 _collateralRebalanceUnits, uint256 _slippageTolerance, ActionInfo memory _actionInfo) internal virtual pure returns (uint256) {
         return _collateralRebalanceUnits
             .preciseMul(_actionInfo.collateralPrice)
             .preciseDiv(_actionInfo.borrowPrice)
@@ -1271,4 +1273,4 @@ contract AaveLeverageStrategyExtension is BaseExtension {
 
         return (enabledExchanges, shouldRebalanceEnums);
     }
-}
+}

contracts/adapters/AaveV3LeverageStrategyExtension.sol

@@ -19,9 +19,15 @@
 pragma solidity 0.6.10;
 pragma experimental ABIEncoderV2;
 
-import {AaveLeverageStrategyExtension} from "./AaveLeverageStrategyExtension.sol";
+import { Math } from "@openzeppelin/contracts/math/Math.sol";
 
-import {IBaseManager} from "../interfaces/IBaseManager.sol";
+import { AaveLeverageStrategyExtension } from "./AaveLeverageStrategyExtension.sol";
+import { IBaseManager } from "../interfaces/IBaseManager.sol";
+import { IAaveOracle } from "../interfaces/IAaveOracle.sol";
+import { IPool } from "../interfaces/IPool.sol";
+import { IPoolAddressesProvider } from "../interfaces/IPoolAddressesProvider.sol";
+import { DataTypes } from "../interfaces/Datatypes.sol";
+import { PreciseUnitMath } from "../lib/PreciseUnitMath.sol";
 
 /**
  * @title AaveV3LeverageStrategyExtension
@@ -31,14 +37,34 @@ import {IBaseManager} from "../interfaces/IBaseManager.sol";
  *
  */
 contract AaveV3LeverageStrategyExtension is AaveLeverageStrategyExtension {
+
+    uint8 public currentEModeCategoryId;  // EMode CategoryId currently set on Aave for the SetToken
+    IPoolAddressesProvider public lendingPoolAddressesProvider; // Aave's address registry used to get Pool and Oracle addresses
+    bool public overrideNoRebalanceInProgress; // Manager controlled flag that allows bypassing the noRebalanceInProgress modifier
+
+    /* ============ Constructor ============ */
+
+    /**
+     * Instantiate addresses, methodology parameters, execution parameters, and incentive parameters.
+     *
+     * @param _manager                       Address of IBaseManager contract
+     * @param _strategy                      Struct of contract addresses
+     * @param _methodology                   Struct containing methodology parameters
+     * @param _execution                     Struct containing execution parameters
+     * @param _incentive                     Struct containing incentive parameters for ripcord
+     * @param _exchangeNames                 List of initial exchange names
+     * @param _exchangeSettings              List of structs containing exchange parameters for the initial exchanges
+     * @param _lendingPoolAddressesProvider  Aave's address registry used to get Pool and Oracle addresses
+     */
     constructor(
         IBaseManager _manager,
         ContractSettings memory _strategy,
         MethodologySettings memory _methodology,
         ExecutionSettings memory _execution,
         IncentiveSettings memory _incentive,
         string[] memory _exchangeNames,
-        ExchangeSettings[] memory _exchangeSettings
+        ExchangeSettings[] memory _exchangeSettings,
+        IPoolAddressesProvider _lendingPoolAddressesProvider
     )
         public
         AaveLeverageStrategyExtension(
@@ -50,20 +76,198 @@ contract AaveV3LeverageStrategyExtension is AaveLeverageStrategyExtension {
             _exchangeNames,
             _exchangeSettings
         )
-    {}
+    {
+        lendingPoolAddressesProvider = _lendingPoolAddressesProvider;
+    }
+
+    /* ============ Modifiers ============ */
+
+    /**
+     * Throws if rebalance is currently in TWAP` can be overriden by the operator
+     */
+    modifier noRebalanceInProgress() override {
+        if(!overrideNoRebalanceInProgress) {
+            require(twapLeverageRatio == 0, "Rebalance is currently in progress");
+        }
+        _;
+    }
+
+
+    /* ============ External Functions ============ */
+
+    /**
+     * OPERATOR ONLY: Enable/Disable override of noRebalanceInProgress modifier
+     *
+     * @param _overrideNoRebalanceInProgress  Boolean indicating wether to enable / disable override
+     */
+    function setOverrideNoRebalanceInProgress(bool _overrideNoRebalanceInProgress) external onlyOperator {
+        overrideNoRebalanceInProgress = _overrideNoRebalanceInProgress;
+    }
 
     /**
      * OPERATOR ONLY: Set eMode categoryId to new value
      *
      * @param _categoryId    eMode categoryId as defined on aaveV3
      */
     function setEModeCategory(uint8 _categoryId) external onlyOperator {
+        currentEModeCategoryId = _categoryId;
         _setEModeCategory(_categoryId);
     }
 
+    /* ============ Internal Functions ============ */
+
+    /**
+     * Sets EMode category in AaveV3 on behalf of the SetToken
+     */
     function _setEModeCategory(uint8 _categoryId) internal {
         bytes memory setEmodeCallData =
             abi.encodeWithSignature("setEModeCategory(address,uint8)", address(strategy.setToken), _categoryId);
         invokeManager(address(strategy.leverageModule), setEmodeCallData);
     }
+
+    /**
+     * Calculate the max borrow / repay amount allowed in base units for lever / delever. This is due to overcollateralization requirements on
+     * assets deposited in lending protocols for borrowing.
+     *
+     */
+    function _calculateMaxBorrowCollateral(ActionInfo memory _actionInfo, bool _isLever) internal override view returns(uint256) {
+        
+        (uint256 maxLtvRaw, uint256 liquidationThresholdRaw) = _getLtvAndLiquidationThreshold();
+
+        // Normalize LTV and liquidation threshold to precise units. LTV is measured in 4 decimals in Aave which is why we must multiply by 1e14
+        // for example ETH has an LTV value of 8000 which represents 80%
+        if (_isLever) {
+            uint256 netBorrowLimit = _actionInfo.collateralValue
+                .preciseMul(maxLtvRaw.mul(10 ** 14))
+                .preciseMul(PreciseUnitMath.preciseUnit().sub(execution.unutilizedLeveragePercentage));
+
+            return netBorrowLimit
+                .sub(_actionInfo.borrowValue)
+                .preciseDiv(_actionInfo.collateralPrice);
+        } else {
+            uint256 netRepayLimit = _actionInfo.collateralValue
+                .preciseMul(liquidationThresholdRaw.mul(10 ** 14));
+
+            return _actionInfo.collateralBalance
+                .preciseMul(netRepayLimit.sub(_actionInfo.borrowValue))
+                .preciseDiv(netRepayLimit);
+        }
+    }
+
+    /**
+     * Calculates LTV and LquidationThreshold either based on ReserveConfiguration or EModeCategory depending on wether EMode is enabled or not
+     *
+     */
+    function _getLtvAndLiquidationThreshold() internal view returns(uint256, uint256) {
+        if(currentEModeCategoryId != 0 ) {
+            // Retrieve collateral factor and liquidation threshold for the collateral asset in precise units (1e16 = 1%)
+            DataTypes.EModeCategory memory emodeData = IPool(lendingPoolAddressesProvider.getPool()).getEModeCategoryData(currentEModeCategoryId);
+            return (emodeData.ltv, emodeData.liquidationThreshold);
+        } else {
+            ( , uint256 maxLtvRaw, uint256 liquidationThresholdRaw, , , , , , ,) = strategy.aaveProtocolDataProvider.getReserveConfigurationData(address(strategy.collateralAsset));
+            return (maxLtvRaw, liquidationThresholdRaw);
+        }
+
+    }
+
+    /**
+     * Validate non-exchange settings in constructor and setters when updating.
+     */
+    function _validateNonExchangeSettings(
+        MethodologySettings memory _methodology,
+        ExecutionSettings memory _execution,
+        IncentiveSettings memory _incentive
+    )
+        internal
+        override
+        pure
+    {
+        super._validateNonExchangeSettings(_methodology, _execution, _incentive);
+        require(_methodology.targetLeverageRatio >= 1 ether, "Target leverage ratio must be >= 1e18");
+    }
+
+    /**
+     * Derive the min repay units from collateral units for delever. Units are calculated as target collateral rebalance units multiplied by slippage tolerance
+     * and pair price (collateral oracle price / borrow oracle price). Output is measured in borrow unit decimals.
+     *
+     * return uint256           Min position units to repay in borrow asset
+     */
+    function _calculateMinRepayUnits(uint256 _collateralRebalanceUnits, uint256 _slippageTolerance, ActionInfo memory _actionInfo)
+        internal
+        override
+        pure
+        returns(uint256)
+    {
+        return _collateralRebalanceUnits
+            .preciseMul(_actionInfo.collateralPrice)
+            .preciseMul(PreciseUnitMath.preciseUnit().sub(_slippageTolerance)) // Changed order of mul / div here
+            .preciseDiv(_actionInfo.borrowPrice);
+    }
+
+    /**
+     * Calculate total notional rebalance quantity and chunked rebalance quantity in collateral units.
+     *
+     * return uint256          Chunked rebalance notional in collateral units
+     * return uint256          Total rebalance notional in collateral units
+     */
+    function _calculateChunkRebalanceNotional(
+        LeverageInfo memory _leverageInfo,
+        uint256 _newLeverageRatio,
+        bool _isLever
+    )
+        internal
+        view
+        override
+        returns (uint256, uint256)
+    {
+        // Calculate absolute value of difference between new and current leverage ratio
+        uint256 leverageRatioDifference = _isLever ?
+            _newLeverageRatio.sub(_leverageInfo.currentLeverageRatio) :
+            _leverageInfo.currentLeverageRatio.sub(_newLeverageRatio);
+
+        uint256 totalRebalanceNotional = leverageRatioDifference
+            .preciseMul(_leverageInfo.action.collateralBalance) // Changed order of mul / div here
+            .preciseDiv(_leverageInfo.currentLeverageRatio);
+
+        uint256 maxBorrow = _calculateMaxBorrowCollateral(_leverageInfo.action, _isLever);
+
+        uint256 chunkRebalanceNotional = Math.min(Math.min(maxBorrow, totalRebalanceNotional), _leverageInfo.twapMaxTradeSize);
+
+        return (chunkRebalanceNotional, totalRebalanceNotional);
+    }
+
+    /**
+     * Create the action info struct to be used in internal functions
+     *
+     * return ActionInfo                Struct containing data used by internal lever and delever functions
+     */
+    function _createActionInfo() internal view override returns(ActionInfo memory) {
+        ActionInfo memory rebalanceInfo;
+
+        // Calculate prices from chainlink. AaveOracle returns prices with 8 decimal places, but we need 36 - underlyingDecimals decimal places.
+        // This is so that when the underlying amount is multiplied by the received price, the collateral valuation is normalized to 36 decimals. 
+        // To perform this adjustment, we multiply by 10^(36 - 8 - underlyingDecimals)
+        rebalanceInfo.collateralPrice = _getAssetPrice(strategy.collateralAsset, strategy.collateralDecimalAdjustment);
+        rebalanceInfo.borrowPrice = _getAssetPrice(strategy.borrowAsset, strategy.borrowDecimalAdjustment);
+
+        rebalanceInfo.collateralBalance = strategy.targetCollateralAToken.balanceOf(address(strategy.setToken));
+        rebalanceInfo.borrowBalance = strategy.targetBorrowDebtToken.balanceOf(address(strategy.setToken));
+        rebalanceInfo.collateralValue = rebalanceInfo.collateralPrice.preciseMul(rebalanceInfo.collateralBalance);
+        rebalanceInfo.borrowValue = rebalanceInfo.borrowPrice.preciseMul(rebalanceInfo.borrowBalance);
+        rebalanceInfo.setTotalSupply = strategy.setToken.totalSupply();
+
+        return rebalanceInfo;
+    }
+
+    /**
+     * Gets AssetPrice from AaveOracle and multiplies by decimalAdjustment if necessary
+     *
+     * return uint256          Asset price normalized to desired number of decimals
+     */
+    function _getAssetPrice(address _asset, uint256 _decimalAdjustment) internal view returns (uint256) {
+        IAaveOracle aaveOracle = IAaveOracle(IPoolAddressesProvider(lendingPoolAddressesProvider).getPriceOracle());
+        uint256 rawPrice = aaveOracle.getAssetPrice(_asset);
+        return rawPrice.mul(10 ** _decimalAdjustment);
+    }
+
 }

contracts/interfaces/IAToken.sol

@@ -1,10 +1,67 @@
-// SPDX-License-Identifier: agpl-3.0
 pragma solidity 0.6.10;
 
-
 interface IAToken {
-  /**
-   * @dev Returns the address of the underlying asset of this aToken (E.g. WETH for aWETH)
-   **/
-  function UNDERLYING_ASSET_ADDRESS() external view returns (address);
+    event Approval(address indexed owner, address indexed spender, uint256 value);
+    event BalanceTransfer(address indexed from, address indexed to, uint256 value, uint256 index);
+    event Burn(address indexed from, address indexed target, uint256 value, uint256 balanceIncrease, uint256 index);
+    event Initialized(
+        address indexed underlyingAsset,
+        address indexed pool,
+        address treasury,
+        address incentivesController,
+        uint8 aTokenDecimals,
+        string aTokenName,
+        string aTokenSymbol,
+        bytes params
+    );
+    event Mint(
+        address indexed caller, address indexed onBehalfOf, uint256 value, uint256 balanceIncrease, uint256 index
+    );
+    event Transfer(address indexed from, address indexed to, uint256 value);
+
+    function ATOKEN_REVISION() external view returns (uint256);
+    function DOMAIN_SEPARATOR() external view returns (bytes32);
+    function EIP712_REVISION() external view returns (bytes memory);
+    function PERMIT_TYPEHASH() external view returns (bytes32);
+    function POOL() external view returns (address);
+    function RESERVE_TREASURY_ADDRESS() external view returns (address);
+    function UNDERLYING_ASSET_ADDRESS() external view returns (address);
+    function allowance(address owner, address spender) external view returns (uint256);
+    function approve(address spender, uint256 amount) external returns (bool);
+    function balanceOf(address user) external view returns (uint256);
+    function burn(address from, address receiverOfUnderlying, uint256 amount, uint256 index) external;
+    function decimals() external view returns (uint8);
+    function decreaseAllowance(address spender, uint256 subtractedValue) external returns (bool);
+    function getIncentivesController() external view returns (address);
+    function getPreviousIndex(address user) external view returns (uint256);
+    function getScaledUserBalanceAndSupply(address user) external view returns (uint256, uint256);
+    function handleRepayment(address user, address onBehalfOf, uint256 amount) external;
+    function increaseAllowance(address spender, uint256 addedValue) external returns (bool);
+    function initialize(
+        address initializingPool,
+        address treasury,
+        address underlyingAsset,
+        address incentivesController,
+        uint8 aTokenDecimals,
+        string memory aTokenName,
+        string memory aTokenSymbol,
+        bytes memory params
+    ) external;
+    function mint(address caller, address onBehalfOf, uint256 amount, uint256 index) external returns (bool);
+    function mintToTreasury(uint256 amount, uint256 index) external;
+    function name() external view returns (string memory);
+    function nonces(address owner) external view returns (uint256);
+    function permit(address owner, address spender, uint256 value, uint256 deadline, uint8 v, bytes32 r, bytes32 s)
+        external;
+    function rescueTokens(address token, address to, uint256 amount) external;
+    function scaledBalanceOf(address user) external view returns (uint256);
+    function scaledTotalSupply() external view returns (uint256);
+    function setIncentivesController(address controller) external;
+    function symbol() external view returns (string memory);
+    function totalSupply() external view returns (uint256);
+    function transfer(address recipient, uint256 amount) external returns (bool);
+    function transferFrom(address sender, address recipient, uint256 amount) external returns (bool);
+    function transferOnLiquidation(address from, address to, uint256 value) external;
+    function transferUnderlyingTo(address target, uint256 amount) external;
 }
+

contracts/interfaces/IAaveOracle.sol

@@ -1,4 +1,4 @@
-pragma solidity ^0.8.10;
+pragma solidity 0.6.10;
 
 interface IAaveOracle {
     event AssetSourceUpdated(address indexed asset, address indexed source);