Uniswap v3 strategy updates

contracts/contracts/strategies/uniswap/README.md

@@ -0,0 +1,15 @@
+# Diagrams
+
+## Uniswap V3 Strategy
+
+### Hierarchy
+
+![Uniswap V3 Strategy Hierarchy](../../../docs/UniswapV3StrategyHierarchy.svg)
+
+### Squashed
+
+![Uniswap V3 Strategy Squashed](../../../docs/UniswapV3StrategySquashed.svg)
+
+### Storage
+
+![Uniswap V3 Strategy Storage](../../../docs/UniswapV3StrategyStorage.svg)

contracts/contracts/strategies/uniswap/UniswapV3Strategy.sol

@@ -16,11 +16,9 @@ import { IUniswapV3Helper } from "../../interfaces/uniswap/v3/IUniswapV3Helper.s
 import { IUniswapV3Pool } from "@uniswap/v3-core/contracts/interfaces/IUniswapV3Pool.sol";
 import { IUniswapV3Strategy } from "../../interfaces/IUniswapV3Strategy.sol";
 import { ISwapRouter } from "@uniswap/v3-periphery/contracts/interfaces/ISwapRouter.sol";
-import { StableMath } from "../../utils/StableMath.sol";
 
 contract UniswapV3Strategy is UniswapV3StrategyStorage {
     using SafeERC20 for IERC20;
-    using StableMath for uint256;
 
     /**
      * @dev Initialize the contract
@@ -365,7 +363,7 @@ contract UniswapV3Strategy is UniswapV3StrategyStorage {
     }
 
     /**
-     * @dev Only checks the active LP position and undeployed/undeposited balance held by the contract.
+     * @notice Only checks the active LP position and undeployed/undeposited balance held by the contract.
      *      Doesn't return the balance held in the reserve strategies.
      * @inheritdoc InitializableAbstractStrategy
      */
@@ -403,7 +401,7 @@ contract UniswapV3Strategy is UniswapV3StrategyStorage {
             ERC721 management
     ****************************************/
 
-    /// Callback function for whenever a NFT is transferred to this contract
+    /// @notice Callback function for whenever a NFT is transferred to this contract
     // solhint-disable-next-line max-line-length
     /// Ref: https://docs.openzeppelin.com/contracts/3.x/api/token/erc721#IERC721Receiver-onERC721Received-address-address-uint256-bytes-
     function onERC721Received(
@@ -435,7 +433,7 @@ contract UniswapV3Strategy is UniswapV3StrategyStorage {
     }
 
     /**
-     * Removes all allowance of both the tokens from NonfungiblePositionManager as
+     * @notice Removes all allowance of both the tokens from NonfungiblePositionManager as
      * well as from the Uniswap V3 Swap Router
      */
     function resetAllowanceOfTokens() external onlyGovernor nonReentrant {
@@ -486,7 +484,7 @@ contract UniswapV3Strategy is UniswapV3StrategyStorage {
             Proxy to liquidity management
     ****************************************/
     /**
-     * @dev Sets the implementation for the liquidity manager
+     * @notice Sets the implementation for the liquidity manager
      * @param newImpl address of the implementation
      */
     function setLiquidityManagerImpl(address newImpl) external onlyGovernor {

contracts/contracts/strategies/uniswap/UniswapV3StrategyStorage.sol

@@ -91,24 +91,33 @@ abstract contract UniswapV3StrategyStorage is InitializableAbstractStrategy {
         uint256 netValue; // Last recorded net value of the position
     }
 
-    // Set to the proxy address when initialized
+    /// @notice The strategy's proxy contract address
+    /// @dev is set when initialized
     IUniswapV3Strategy public _self;
 
-    // The address that can manage the positions on Uniswap V3
+    /// @notice The address that can manage the positions on Uniswap V3
     address public operatorAddr;
-    address public token0; // Token0 of Uniswap V3 Pool
-    address public token1; // Token1 of Uniswap V3 Pool
+    /// @notice Token0 of Uniswap V3 Pool
+    address public token0;
+    /// @notice Token1 of Uniswap V3 Pool
+    address public token1;
 
     // When the funds are not deployed in Uniswap V3 Pool, they will
     // be deposited to these reserve strategies
-    IStrategy public reserveStrategy0; // Reserve strategy for token0
-    IStrategy public reserveStrategy1; // Reserve strategy for token1
+    /// @notice Reserve strategy for token0
+    IStrategy public reserveStrategy0;
+    /// @notice Reserve strategy for token1
+    IStrategy public reserveStrategy1;
 
-    uint24 public poolFee; // Uniswap V3 Pool Fee
-    bool public swapsPaused = false; // True if Swaps are paused
-    bool public rebalancePaused = false; // True if Swaps are paused
+    /// @notice Uniswap V3 Pool Fee
+    uint24 public poolFee;
+    /// @notice True if Swaps are paused
+    bool public swapsPaused = false;
+    /// @notice True if liquidity rebalances are paused
+    bool public rebalancePaused = false;
 
-    uint256 public maxTVL; // In USD, 18 decimals
+    /// @notice Maximum amount the strategy can have deployed in the Uniswap pool. In OTokens to 18 decimals
+    uint256 public maxTVL;
 
     // Deposits to reserve strategy when contract balance exceeds this amount
     uint256 public minDepositThreshold0;
@@ -122,34 +131,34 @@ abstract contract UniswapV3StrategyStorage is InitializableAbstractStrategy {
     uint160 public minSwapPriceX96;
     uint160 public maxSwapPriceX96;
 
-    // Token ID of active Position on the pool. zero, if there are no active LP position
+    /// @notice Token ID of active Position on the pool. zero, if there are no active LP position
     uint256 public activeTokenId;
 
-    // Sum of loss in value of tokens deployed to the pool
+    /// @notice Sum of loss in value of tokens deployed to the pool
     uint256 public netLostValue;
 
-    // Max value loss threshold after which rebalances aren't allowed
+    /// @notice Max value loss threshold after which rebalances aren't allowed
     uint256 public maxPositionValueLostThreshold;
 
-    // Uniswap V3's Pool
+    /// @notice Uniswap V3's Pool
     IUniswapV3Pool public pool;
 
-    // Uniswap V3's PositionManager
+    /// @notice Uniswap V3's PositionManager
     INonfungiblePositionManager public positionManager;
 
-    // A deployed contract that's used to call methods of Uniswap V3's libraries despite version mismatch
+    /// @notice A deployed contract that's used to call methods of Uniswap V3's libraries despite version mismatch
     IUniswapV3Helper public helper;
 
-    // Uniswap Swap Router
+    /// @notice Uniswap Swap Router
     ISwapRouter public swapRouter;
 
-    // A lookup table to find token IDs of position using f(lowerTick, upperTick)
+    /// @notice A lookup table to find token IDs of position using f(lowerTick, upperTick)
     mapping(int48 => uint256) public ticksToTokenId;
 
-    // Maps tokenIDs to their Position object
+    /// @notice Maps tokenIDs to their Position object
     mapping(uint256 => Position) public tokenIdToPosition;
 
-    // keccak256("OUSD.UniswapV3Strategy.LiquidityManager.impl")
+    /// @notice keccak256("OUSD.UniswapV3Strategy.LiquidityManager.impl")
     bytes32 constant LIQUIDITY_MANAGER_IMPL_POSITION =
         0xec676d52175f7cbb4e4ea392c6b70f8946575021aad20479602b98adc56ad62d;
 
@@ -192,7 +201,7 @@ abstract contract UniswapV3StrategyStorage is InitializableAbstractStrategy {
             Shared functions
     ****************************************/
     /**
-     * @notice Deposits token balances in the contract back to the reserve strategies
+     * @dev Deposits token balances in the contract back to the reserve strategies
      */
     function _depositAll() internal {
         uint256 token0Bal = IERC20(token0).balanceOf(address(this));
@@ -209,7 +218,7 @@ abstract contract UniswapV3StrategyStorage is InitializableAbstractStrategy {
     }
 
     /**
-     * @notice Returns the balance of both tokens in a given position (including fees)
+     * @dev Returns the balance of both tokens in a given position (including fees)
      * @param tokenId tokenID of the Position NFT
      * @return amount0 Amount of token0 in position
      * @return amount1 Amount of token1 in position
@@ -230,7 +239,7 @@ abstract contract UniswapV3StrategyStorage is InitializableAbstractStrategy {
     }
 
     /**
-     * @notice Returns the balance of both tokens in a given position (without fees)
+     * @dev Returns the balance of both tokens in a given position (without fees)
      * @param tokenId tokenID of the Position NFT
      * @return amount0 Amount of token0 in position
      * @return amount1 Amount of token1 in position

contracts/contracts/utils/InitializableAbstractStrategy.sol

@@ -103,7 +103,7 @@ abstract contract InitializableAbstractStrategy is Initializable, Governable {
     }
 
     /**
-     * @dev Collect accumulated reward token and send to Harvester.
+     * @notice Collect accumulated reward token and send to Harvester.
      */
     function collectRewardTokens() external virtual onlyHarvester nonReentrant {
         _collectRewardTokens();
@@ -163,7 +163,7 @@ abstract contract InitializableAbstractStrategy is Initializable, Governable {
     }
 
     /**
-     * @dev Set the reward token addresses.
+     * @notice Set the reward token addresses.
      * @param _rewardTokenAddresses Address array of the reward token
      */
     function setRewardTokenAddresses(address[] calldata _rewardTokenAddresses)
@@ -185,7 +185,7 @@ abstract contract InitializableAbstractStrategy is Initializable, Governable {
     }
 
     /**
-     * @dev Get the reward token addresses.
+     * @notice Get the reward token addresses.
      * @return address[] the reward token addresses.
      */
     function getRewardTokenAddresses()
@@ -197,7 +197,7 @@ abstract contract InitializableAbstractStrategy is Initializable, Governable {
     }
 
     /**
-     * @dev Provide support for asset by passing its pToken address.
+     * @notice Provide support for asset by passing its pToken address.
      *      This method can only be called by the system Governor
      * @param _asset    Address for the asset
      * @param _pToken   Address for the corresponding platform token
@@ -211,7 +211,7 @@ abstract contract InitializableAbstractStrategy is Initializable, Governable {
     }
 
     /**
-     * @dev Remove a supported asset by passing its index.
+     * @notice Remove a supported asset by passing its index.
      *      This method can only be called by the system Governor
      * @param _assetIndex Index of the asset to be removed
      */
@@ -252,7 +252,7 @@ abstract contract InitializableAbstractStrategy is Initializable, Governable {
     }
 
     /**
-     * @dev Transfer token to governor. Intended for recovering tokens stuck in
+     * @notice Transfer token to governor. Intended for recovering tokens stuck in
      *      strategy contracts, i.e. mistaken sends.
      * @param _asset Address for the asset
      * @param _amount Amount of the asset to transfer
@@ -265,7 +265,7 @@ abstract contract InitializableAbstractStrategy is Initializable, Governable {
     }
 
     /**
-     * @dev Set the reward token addresses.
+     * @notice Set the reward token addresses.
      * @param _harvesterAddress Address of the harvester
      */
     function setHarvesterAddress(address _harvesterAddress)
@@ -291,25 +291,25 @@ abstract contract InitializableAbstractStrategy is Initializable, Governable {
         virtual;
 
     /**
-     * @dev Approve all the assets supported by the strategy
+     * @notice Approve all the assets supported by the strategy
      *      to be moved around the platform.
      */
     function safeApproveAllTokens() external virtual;
 
     /**
-     * @dev Deposit an amount of asset into the platform
+     * @notice Deposit an amount of asset into the platform
      * @param _asset               Address for the asset
      * @param _amount              Units of asset to deposit
      */
     function deposit(address _asset, uint256 _amount) external virtual;
 
     /**
-     * @dev Deposit balance of all supported assets into the platform
+     * @notice Deposit balance of all supported assets into the platform
      */
     function depositAll() external virtual;
 
     /**
-     * @dev Withdraw an amount of asset from the platform.
+     * @notice Withdraw an amount of asset from the platform.
      * @param _recipient         Address to which the asset should be sent
      * @param _asset             Address of the asset
      * @param _amount            Units of asset to withdraw
@@ -321,7 +321,7 @@ abstract contract InitializableAbstractStrategy is Initializable, Governable {
     ) external virtual;
 
     /**
-     * @dev Withdraw all assets from strategy sending assets to Vault.
+     * @notice Withdraw all assets from strategy sending assets to Vault.
      */
     function withdrawAll() external virtual;
 
@@ -338,7 +338,7 @@ abstract contract InitializableAbstractStrategy is Initializable, Governable {
         returns (uint256 balance);
 
     /**
-     * @dev Check if an asset is supported.
+     * @notice Check if an asset is supported.
      * @param _asset    Address of the asset
      * @return bool     Whether asset is supported
      */