From 59996e1ece6aafc4e0ff438e38eae5ac9dc01b05 Mon Sep 17 00:00:00 2001
From: eboado <ernesto.boado.moreira@gmail.com>
Date: Tue, 15 Sep 2020 10:28:39 +0200
Subject: [PATCH] - Refactor validation of swapLiquidity() to ValidationLogic.
 - Added extra check on active reserves on swapLiquidity().

---
 .../LendingPoolLiquidationManager.sol         | 195 +++++++++++-------
 contracts/libraries/logic/ValidationLogic.sol |  55 +++--
 2 files changed, 154 insertions(+), 96 deletions(-)

diff --git a/contracts/lendingpool/LendingPoolLiquidationManager.sol b/contracts/lendingpool/LendingPoolLiquidationManager.sol
index 643ad42f..944c681e 100644
--- a/contracts/lendingpool/LendingPoolLiquidationManager.sol
+++ b/contracts/lendingpool/LendingPoolLiquidationManager.sol
@@ -110,6 +110,26 @@ contract LendingPoolLiquidationManager is VersionedInitializable {
     string errorMsg;
   }
 
+  struct SwapLiquidityLocalVars {
+    uint256 healthFactor;
+    uint256 amountToReceive;
+    uint256 userBalanceBefore;
+    IAToken fromReserveAToken;
+    IAToken toReserveAToken;
+    uint256 errorCode;
+    string errorMsg;
+  }
+
+  struct AvailableCollateralToLiquidateLocalVars {
+    uint256 userCompoundedBorrowBalance;
+    uint256 liquidationBonus;
+    uint256 collateralPrice;
+    uint256 principalCurrencyPrice;
+    uint256 maxAmountCollateralToLiquidate;
+    uint256 principalDecimals;
+    uint256 collateralDecimals;
+  }
+
   /**
    * @dev as the contract extends the VersionedInitializable contract to match the state
    * of the LendingPool contract, the getRevision() function is needed.
@@ -253,7 +273,12 @@ contract LendingPoolLiquidationManager is VersionedInitializable {
       );
 
       //burn the equivalent amount of atoken
-      vars.collateralAtoken.burn(user, msg.sender, vars.maxCollateralToLiquidate, collateralReserve.liquidityIndex);
+      vars.collateralAtoken.burn(
+        user,
+        msg.sender,
+        vars.maxCollateralToLiquidate,
+        collateralReserve.liquidityIndex
+      );
     }
 
     //transfers the principal currency to the aToken
@@ -356,7 +381,12 @@ contract LendingPoolLiquidationManager is VersionedInitializable {
     //updating collateral reserve indexes
     collateralReserve.updateCumulativeIndexesAndTimestamp();
 
-    vars.collateralAtoken.burn(user, receiver, vars.maxCollateralToLiquidate, collateralReserve.liquidityIndex);
+    vars.collateralAtoken.burn(
+      user,
+      receiver,
+      vars.maxCollateralToLiquidate,
+      collateralReserve.liquidityIndex
+    );
 
     if (vars.userCollateralBalance == vars.maxCollateralToLiquidate) {
       usersConfig[user].setUsingAsCollateral(collateralReserve.id, false);
@@ -375,7 +405,12 @@ contract LendingPoolLiquidationManager is VersionedInitializable {
 
     //updating debt reserve
     debtReserve.updateCumulativeIndexesAndTimestamp();
-    debtReserve.updateInterestRates(principal, vars.principalAToken, vars.actualAmountToLiquidate, 0);
+    debtReserve.updateInterestRates(
+      principal,
+      vars.principalAToken,
+      vars.actualAmountToLiquidate,
+      0
+    );
     IERC20(principal).transferFrom(receiver, vars.principalAToken, vars.actualAmountToLiquidate);
 
     if (vars.userVariableDebt >= vars.actualAmountToLiquidate) {
@@ -411,14 +446,83 @@ contract LendingPoolLiquidationManager is VersionedInitializable {
     return (uint256(Errors.LiquidationErrors.NO_ERROR), Errors.NO_ERRORS);
   }
 
-  struct AvailableCollateralToLiquidateLocalVars {
-    uint256 userCompoundedBorrowBalance;
-    uint256 liquidationBonus;
-    uint256 collateralPrice;
-    uint256 principalCurrencyPrice;
-    uint256 maxAmountCollateralToLiquidate;
-    uint256 principalDecimals;
-    uint256 collateralDecimals;
+  /**
+   * @dev Allows an user to release one of his assets deposited in the protocol, even if it is used as collateral.
+   * - It's not possible to release one asset to swap for the same
+   * @param receiverAddress The address of the contract receiving the funds. The receiver should implement the ISwapAdapter interface
+   * @param fromAsset Asset to swap from
+   * @param toAsset Asset to swap to
+   * @param params a bytes array to be sent (if needed) to the receiver contract with extra data
+   **/
+  function swapLiquidity(
+    address receiverAddress,
+    address fromAsset,
+    address toAsset,
+    uint256 amountToSwap,
+    bytes calldata params
+  ) external returns (uint256, string memory) {
+    ReserveLogic.ReserveData storage fromReserve = reserves[fromAsset];
+    ReserveLogic.ReserveData storage toReserve = reserves[toAsset];
+
+    // Usage of a memory struct of vars to avoid "Stack too deep" errors due to local variables
+    SwapLiquidityLocalVars memory vars;
+
+    (vars.errorCode, vars.errorMsg) = ValidationLogic.validateSwapLiquidity(
+      fromReserve,
+      toReserve,
+      fromAsset,
+      toAsset
+    );
+
+    if (Errors.LiquidationErrors(vars.errorCode) != Errors.LiquidationErrors.NO_ERROR) {
+      return (vars.errorCode, vars.errorMsg);
+    }
+
+    vars.fromReserveAToken = IAToken(fromReserve.aTokenAddress);
+    vars.toReserveAToken = IAToken(toReserve.aTokenAddress);
+
+    fromReserve.updateCumulativeIndexesAndTimestamp();
+    toReserve.updateCumulativeIndexesAndTimestamp();
+
+    if (vars.fromReserveAToken.balanceOf(msg.sender) == amountToSwap) {
+      usersConfig[msg.sender].setUsingAsCollateral(fromReserve.id, false);
+    }
+
+    fromReserve.updateInterestRates(fromAsset, address(vars.fromReserveAToken), 0, amountToSwap);
+
+    vars.fromReserveAToken.burn(msg.sender, receiverAddress, amountToSwap, fromReserve.liquidityIndex);
+    // Notifies the receiver to proceed, sending as param the underlying already transferred
+    ISwapAdapter(receiverAddress).executeOperation(
+      fromAsset,
+      toAsset,
+      amountToSwap,
+      address(this),
+      params
+    );
+
+    vars.amountToReceive = IERC20(toAsset).balanceOf(receiverAddress);
+    if (vars.amountToReceive != 0) {
+      IERC20(toAsset).transferFrom(receiverAddress, address(vars.toReserveAToken), vars.amountToReceive);
+      vars.toReserveAToken.mint(msg.sender, vars.amountToReceive, toReserve.liquidityIndex);
+      toReserve.updateInterestRates(toAsset, address(vars.toReserveAToken), vars.amountToReceive, 0);
+    }
+
+    (, , , , vars.healthFactor) = GenericLogic.calculateUserAccountData(
+      msg.sender,
+      reserves,
+      usersConfig[msg.sender],
+      reservesList,
+      addressesProvider.getPriceOracle()
+    );
+
+    if (vars.healthFactor < GenericLogic.HEALTH_FACTOR_LIQUIDATION_THRESHOLD) {
+      return (
+        uint256(Errors.LiquidationErrors.HEALTH_FACTOR_LOWER_THAN_LIQUIDATION_THRESHOLD),
+        Errors.HEALTH_FACTOR_LOWER_THAN_LIQUIDATION_THRESHOLD
+      );
+    }
+
+    return (uint256(Errors.LiquidationErrors.NO_ERROR), Errors.NO_ERRORS);
   }
 
   /**
@@ -479,73 +583,4 @@ contract LendingPoolLiquidationManager is VersionedInitializable {
     return (collateralAmount, principalAmountNeeded);
   }
 
-  /**
-   * @dev Allows an user to release one of his assets deposited in the protocol, even if it is used as collateral.
-   * - It's not possible to release one asset to swap for the same
-   * @param receiverAddress The address of the contract receiving the funds. The receiver should implement the ISwapAdapter interface
-   * @param fromAsset Asset to swap from
-   * @param toAsset Asset to swap to
-   * @param params a bytes array to be sent (if needed) to the receiver contract with extra data
-   **/
-  function swapLiquidity(
-    address receiverAddress,
-    address fromAsset,
-    address toAsset,
-    uint256 amountToSwap,
-    bytes calldata params
-  ) external returns (uint256, string memory) {
-    if (fromAsset == toAsset) {
-      return (uint256(Errors.LiquidationErrors.INVALID_EQUAL_ASSETS_TO_SWAP), Errors.INVALID_EQUAL_ASSETS_TO_SWAP);
-    }
-
-    ReserveLogic.ReserveData storage fromReserve = reserves[fromAsset];
-    ReserveLogic.ReserveData storage toReserve = reserves[toAsset];
-    IAToken fromReserveAToken = IAToken(fromReserve.aTokenAddress);
-    IAToken toReserveAToken = IAToken(toReserve.aTokenAddress);
-
-    fromReserve.updateCumulativeIndexesAndTimestamp();
-    toReserve.updateCumulativeIndexesAndTimestamp();
-
-    // get user position
-    uint256 userBalance = fromReserveAToken.balanceOf(msg.sender);
-    if (userBalance == amountToSwap) {
-      usersConfig[msg.sender].setUsingAsCollateral(fromReserve.id, false);
-    }
-
-    fromReserve.updateInterestRates(fromAsset, address(fromReserveAToken), 0, amountToSwap);
-
-    fromReserveAToken.burn(msg.sender, receiverAddress, amountToSwap, fromReserve.liquidityIndex);
-    // Notifies the receiver to proceed, sending as param the underlying already transferred
-    ISwapAdapter(receiverAddress).executeOperation(
-      fromAsset,
-      toAsset,
-      amountToSwap,
-      address(this),
-      params
-    );
-
-    uint256 amountToReceive = IERC20(toAsset).balanceOf(receiverAddress);
-    if (amountToReceive != 0) {
-      IERC20(toAsset).transferFrom(receiverAddress, address(toReserveAToken), amountToReceive);
-      toReserveAToken.mint(msg.sender, amountToReceive, toReserve.liquidityIndex);
-      toReserve.updateInterestRates(toAsset, address(toReserveAToken), amountToReceive, 0);
-    }
-
-    (, , , , uint256 healthFactor) = GenericLogic.calculateUserAccountData(
-      msg.sender,
-      reserves,
-      usersConfig[msg.sender],
-      reservesList,
-      addressesProvider.getPriceOracle()
-    );
-
-    if (healthFactor < GenericLogic.HEALTH_FACTOR_LIQUIDATION_THRESHOLD) {
-      return (
-        uint256(Errors.LiquidationErrors.HEALTH_FACTOR_LOWER_THAN_LIQUIDATION_THRESHOLD),
-        Errors.HEALTH_FACTOR_LOWER_THAN_LIQUIDATION_THRESHOLD
-      );
-    }
-
-    return (uint256(Errors.LiquidationErrors.NO_ERROR), Errors.NO_ERRORS);
-  }
 }
diff --git a/contracts/libraries/logic/ValidationLogic.sol b/contracts/libraries/logic/ValidationLogic.sol
index 84c1d861..336e18ad 100644
--- a/contracts/libraries/logic/ValidationLogic.sol
+++ b/contracts/libraries/logic/ValidationLogic.sol
@@ -347,12 +347,11 @@ library ValidationLogic {
     uint256 userHealthFactor,
     uint256 userStableDebt,
     uint256 userVariableDebt
-  ) internal view returns(uint256, string memory) {
-    if ( !collateralReserve.configuration.getActive() || !principalReserve.configuration.getActive()) {
-      return (
-        uint256(Errors.LiquidationErrors.NO_ACTIVE_RESERVE),
-        Errors.NO_ACTIVE_RESERVE
-      );
+  ) internal view returns (uint256, string memory) {
+    if (
+      !collateralReserve.configuration.getActive() || !principalReserve.configuration.getActive()
+    ) {
+      return (uint256(Errors.LiquidationErrors.NO_ACTIVE_RESERVE), Errors.NO_ACTIVE_RESERVE);
     }
 
     if (userHealthFactor >= GenericLogic.HEALTH_FACTOR_LIQUIDATION_THRESHOLD) {
@@ -362,8 +361,7 @@ library ValidationLogic {
       );
     }
 
-    bool isCollateralEnabled =
-      collateralReserve.configuration.getLiquidationThreshold() > 0 &&
+    bool isCollateralEnabled = collateralReserve.configuration.getLiquidationThreshold() > 0 &&
       userConfig.isUsingAsCollateral(collateralReserve.id);
 
     //if collateral isn't enabled as collateral by user, it cannot be liquidated
@@ -402,12 +400,11 @@ library ValidationLogic {
     uint256 userHealthFactor,
     uint256 userStableDebt,
     uint256 userVariableDebt
-  ) internal view returns(uint256, string memory) {
-    if ( !collateralReserve.configuration.getActive() || !principalReserve.configuration.getActive()) {
-      return (
-        uint256(Errors.LiquidationErrors.NO_ACTIVE_RESERVE),
-        Errors.NO_ACTIVE_RESERVE
-      );
+  ) internal view returns (uint256, string memory) {
+    if (
+      !collateralReserve.configuration.getActive() || !principalReserve.configuration.getActive()
+    ) {
+      return (uint256(Errors.LiquidationErrors.NO_ACTIVE_RESERVE), Errors.NO_ACTIVE_RESERVE);
     }
 
     if (
@@ -420,8 +417,7 @@ library ValidationLogic {
     }
 
     if (msg.sender != user) {
-      bool isCollateralEnabled =
-        collateralReserve.configuration.getLiquidationThreshold() > 0 &&
+      bool isCollateralEnabled = collateralReserve.configuration.getLiquidationThreshold() > 0 &&
         userConfig.isUsingAsCollateral(collateralReserve.id);
 
       //if collateral isn't enabled as collateral by user, it cannot be liquidated
@@ -442,4 +438,31 @@ library ValidationLogic {
 
     return (uint256(Errors.LiquidationErrors.NO_ERROR), Errors.NO_ERRORS);
   }
+
+  /**
+   * @dev Validates the swapLiquidity() action
+   * @param fromReserve The reserve data of the asset to swap from
+   * @param toReserve The reserve data of the asset to swap to
+   * @param fromAsset Address of the asset to swap from
+   * @param toAsset Address of the asset to swap to
+   **/
+  function validateSwapLiquidity(
+    ReserveLogic.ReserveData storage fromReserve,
+    ReserveLogic.ReserveData storage toReserve,
+    address fromAsset,
+    address toAsset
+  ) internal view returns (uint256, string memory) {
+    if (!fromReserve.configuration.getActive() || !toReserve.configuration.getActive()) {
+      return (uint256(Errors.LiquidationErrors.NO_ACTIVE_RESERVE), Errors.NO_ACTIVE_RESERVE);
+    }
+
+    if (fromAsset == toAsset) {
+      return (
+        uint256(Errors.LiquidationErrors.INVALID_EQUAL_ASSETS_TO_SWAP),
+        Errors.INVALID_EQUAL_ASSETS_TO_SWAP
+      );
+    }
+
+    return (uint256(Errors.LiquidationErrors.NO_ERROR), Errors.NO_ERRORS);
+  }
 }