From 0911f907a801d72c0624f58788ba683fbd392ad6 Mon Sep 17 00:00:00 2001 From: eboado Date: Sun, 13 Sep 2020 10:08:14 +0200 Subject: [PATCH 1/2] Fixes #35 --- contracts/lendingpool/LendingPool.sol | 4 ++ contracts/libraries/logic/ValidationLogic.sol | 16 ++++++++ test/liquidation-underlying.spec.ts | 21 ++++++++++ test/repay-with-collateral.spec.ts | 38 +++++++++++++++++++ 4 files changed, 79 insertions(+) diff --git a/contracts/lendingpool/LendingPool.sol b/contracts/lendingpool/LendingPool.sol index 30a69084..5dfe544f 100644 --- a/contracts/lendingpool/LendingPool.sol +++ b/contracts/lendingpool/LendingPool.sol @@ -385,6 +385,8 @@ contract LendingPool is VersionedInitializable, ILendingPool { uint256 purchaseAmount, bool receiveAToken ) external override { + ValidationLogic.validateLiquidation(_reserves[collateral], _reserves[asset]); + address liquidationManager = _addressesProvider.getLendingPoolLiquidationManager(); //solium-disable-next-line @@ -444,6 +446,8 @@ contract LendingPool is VersionedInitializable, ILendingPool { require(!_flashLiquidationLocked, Errors.REENTRANCY_NOT_ALLOWED); _flashLiquidationLocked = true; + ValidationLogic.validateLiquidation(_reserves[collateral], _reserves[principal]); + address liquidationManager = _addressesProvider.getLendingPoolLiquidationManager(); //solium-disable-next-line diff --git a/contracts/libraries/logic/ValidationLogic.sol b/contracts/libraries/logic/ValidationLogic.sol index 9be3f828..60d7645c 100644 --- a/contracts/libraries/logic/ValidationLogic.sol +++ b/contracts/libraries/logic/ValidationLogic.sol @@ -329,4 +329,20 @@ library ValidationLogic { require(premium > 0, Errors.REQUESTED_AMOUNT_TOO_SMALL); require(mode <= uint256(ReserveLogic.InterestRateMode.VARIABLE), Errors.INVALID_FLASHLOAN_MODE); } + + /** + * @dev Validates configurations for liquidation actions, both liquidationCall() and repayWithCollateral() + * @param collateralReserve The reserve data of the collateral + * @param principalReserve The reserve data of the principal + **/ + function validateLiquidation( + ReserveLogic.ReserveData storage collateralReserve, + ReserveLogic.ReserveData storage principalReserve + ) internal view { + require( + collateralReserve.configuration.getActive() && + principalReserve.configuration.getActive(), + Errors.NO_ACTIVE_RESERVE + ); + } } diff --git a/test/liquidation-underlying.spec.ts b/test/liquidation-underlying.spec.ts index 3f2aecbc..076abf88 100644 --- a/test/liquidation-underlying.spec.ts +++ b/test/liquidation-underlying.spec.ts @@ -7,6 +7,7 @@ import {makeSuite} from './helpers/make-suite'; import {ProtocolErrors, RateMode} from '../helpers/types'; import {calcExpectedStableDebtTokenBalance} from './helpers/utils/calculations'; import {getUserData} from './helpers/utils/helpers'; +import {parseEther} from 'ethers/lib/utils'; const chai = require('chai'); @@ -23,6 +24,26 @@ makeSuite('LendingPool liquidation - liquidator receiving the underlying asset', BigNumber.config({DECIMAL_PLACES: 20, ROUNDING_MODE: BigNumber.ROUND_HALF_UP}); }); + it("It's not possible to liquidate on a non-active collateral or a non active principal", async () => { + const {configurator, weth, pool, users, dai} = testEnv; + const user = users[1]; + await configurator.deactivateReserve(weth.address); + + await expect( + pool.liquidationCall(weth.address, dai.address, user.address, parseEther('1000'), false) + ).to.be.revertedWith('2'); + + await configurator.activateReserve(weth.address); + + await configurator.deactivateReserve(dai.address); + + await expect( + pool.liquidationCall(weth.address, dai.address, user.address, parseEther('1000'), false) + ).to.be.revertedWith('2'); + + await configurator.activateReserve(dai.address); + }); + it('LIQUIDATION - Deposits WETH, borrows DAI', async () => { const {dai, weth, users, pool, oracle} = testEnv; const depositor = users[0]; diff --git a/test/repay-with-collateral.spec.ts b/test/repay-with-collateral.spec.ts index faefa403..81d42cbe 100644 --- a/test/repay-with-collateral.spec.ts +++ b/test/repay-with-collateral.spec.ts @@ -39,6 +39,44 @@ export const expectRepayWithCollateralEvent = ( }; makeSuite('LendingPool. repayWithCollateral()', (testEnv: TestEnv) => { + it("It's not possible to repayWithCollateral() on a non-active collateral or a non active principal", async () => { + const {configurator, weth, pool, users, dai, mockSwapAdapter} = testEnv; + const user = users[1]; + await configurator.deactivateReserve(weth.address); + + await expect( + pool + .connect(user.signer) + .repayWithCollateral( + weth.address, + dai.address, + user.address, + parseEther('100'), + mockSwapAdapter.address, + '0x' + ) + ).to.be.revertedWith('2'); + + await configurator.activateReserve(weth.address); + + await configurator.deactivateReserve(dai.address); + + await expect( + pool + .connect(user.signer) + .repayWithCollateral( + weth.address, + dai.address, + user.address, + parseEther('100'), + mockSwapAdapter.address, + '0x' + ) + ).to.be.revertedWith('2'); + + await configurator.activateReserve(dai.address); + }); + it('User 1 provides some liquidity for others to borrow', async () => { const {pool, weth, dai, usdc, deployer} = testEnv; From e2500d153201b2bbe06a0ca8c16a0984f5f04af1 Mon Sep 17 00:00:00 2001 From: eboado Date: Mon, 14 Sep 2020 10:52:31 +0200 Subject: [PATCH 2/2] - Refactored validation logic of liquidationCall() and repayWithCollateral() to ValidationLogic. --- contracts/lendingpool/LendingPool.sol | 3 - .../LendingPoolLiquidationManager.sol | 114 ++++++----------- contracts/libraries/helpers/Errors.sol | 10 ++ contracts/libraries/logic/ValidationLogic.sol | 115 ++++++++++++++++-- 4 files changed, 155 insertions(+), 87 deletions(-) diff --git a/contracts/lendingpool/LendingPool.sol b/contracts/lendingpool/LendingPool.sol index 5dfe544f..e7ee1d5c 100644 --- a/contracts/lendingpool/LendingPool.sol +++ b/contracts/lendingpool/LendingPool.sol @@ -385,7 +385,6 @@ contract LendingPool is VersionedInitializable, ILendingPool { uint256 purchaseAmount, bool receiveAToken ) external override { - ValidationLogic.validateLiquidation(_reserves[collateral], _reserves[asset]); address liquidationManager = _addressesProvider.getLendingPoolLiquidationManager(); @@ -446,8 +445,6 @@ contract LendingPool is VersionedInitializable, ILendingPool { require(!_flashLiquidationLocked, Errors.REENTRANCY_NOT_ALLOWED); _flashLiquidationLocked = true; - ValidationLogic.validateLiquidation(_reserves[collateral], _reserves[principal]); - address liquidationManager = _addressesProvider.getLendingPoolLiquidationManager(); //solium-disable-next-line diff --git a/contracts/lendingpool/LendingPoolLiquidationManager.sol b/contracts/lendingpool/LendingPoolLiquidationManager.sol index 8640761b..13e7a4b0 100644 --- a/contracts/lendingpool/LendingPoolLiquidationManager.sol +++ b/contracts/lendingpool/LendingPoolLiquidationManager.sol @@ -21,6 +21,7 @@ import {PercentageMath} from '../libraries/math/PercentageMath.sol'; import {SafeERC20} from '@openzeppelin/contracts/token/ERC20/SafeERC20.sol'; import {ISwapAdapter} from '../interfaces/ISwapAdapter.sol'; import {Errors} from '../libraries/helpers/Errors.sol'; +import {ValidationLogic} from '../libraries/logic/ValidationLogic.sol'; /** * @title LendingPoolLiquidationManager contract @@ -88,15 +89,6 @@ contract LendingPoolLiquidationManager is VersionedInitializable { uint256 swappedCollateralAmount ); - enum LiquidationErrors { - NO_ERROR, - NO_COLLATERAL_AVAILABLE, - COLLATERAL_CANNOT_BE_LIQUIDATED, - CURRRENCY_NOT_BORROWED, - HEALTH_FACTOR_ABOVE_THRESHOLD, - NOT_ENOUGH_LIQUIDITY - } - struct LiquidationCallLocalVars { uint256 userCollateralBalance; uint256 userStableDebt; @@ -112,6 +104,9 @@ contract LendingPoolLiquidationManager is VersionedInitializable { uint256 healthFactor; IAToken collateralAtoken; bool isCollateralEnabled; + address principalAToken; + uint256 errorCode; + string errorMsg; } /** @@ -138,8 +133,8 @@ contract LendingPoolLiquidationManager is VersionedInitializable { uint256 purchaseAmount, bool receiveAToken ) external returns (uint256, string memory) { - ReserveLogic.ReserveData storage principalReserve = reserves[principal]; ReserveLogic.ReserveData storage collateralReserve = reserves[collateral]; + ReserveLogic.ReserveData storage principalReserve = reserves[principal]; UserConfiguration.Map storage userConfig = usersConfig[user]; LiquidationCallLocalVars memory vars; @@ -152,43 +147,29 @@ contract LendingPoolLiquidationManager is VersionedInitializable { addressesProvider.getPriceOracle() ); - if (vars.healthFactor >= GenericLogic.HEALTH_FACTOR_LIQUIDATION_THRESHOLD) { - return ( - uint256(LiquidationErrors.HEALTH_FACTOR_ABOVE_THRESHOLD), - Errors.HEALTH_FACTOR_NOT_BELOW_THRESHOLD - ); - } - - vars.collateralAtoken = IAToken(collateralReserve.aTokenAddress); - - vars.userCollateralBalance = vars.collateralAtoken.balanceOf(user); - - vars.isCollateralEnabled = - collateralReserve.configuration.getLiquidationThreshold() > 0 && - userConfig.isUsingAsCollateral(collateralReserve.index); - - //if collateral isn't enabled as collateral by user, it cannot be liquidated - if (!vars.isCollateralEnabled) { - return ( - uint256(LiquidationErrors.COLLATERAL_CANNOT_BE_LIQUIDATED), - Errors.COLLATERAL_CANNOT_BE_LIQUIDATED - ); - } - //if the user hasn't borrowed the specific currency defined by asset, it cannot be liquidated (vars.userStableDebt, vars.userVariableDebt) = Helpers.getUserCurrentDebt( user, principalReserve ); - if (vars.userStableDebt == 0 && vars.userVariableDebt == 0) { - return ( - uint256(LiquidationErrors.CURRRENCY_NOT_BORROWED), - Errors.SPECIFIED_CURRENCY_NOT_BORROWED_BY_USER - ); + (vars.errorCode, vars.errorMsg) = ValidationLogic.validateLiquidationCall( + collateralReserve, + principalReserve, + userConfig, + vars.healthFactor, + vars.userStableDebt, + vars.userVariableDebt + ); + + if (Errors.LiquidationErrors(vars.errorCode) != Errors.LiquidationErrors.NO_ERROR) { + return (vars.errorCode, vars.errorMsg); } - //all clear - calculate the max principal amount that can be liquidated + vars.collateralAtoken = IAToken(collateralReserve.aTokenAddress); + + vars.userCollateralBalance = vars.collateralAtoken.balanceOf(user); + vars.maxPrincipalAmountToLiquidate = vars.userStableDebt.add(vars.userVariableDebt).percentMul( LIQUIDATION_CLOSE_FACTOR_PERCENT ); @@ -224,7 +205,7 @@ contract LendingPoolLiquidationManager is VersionedInitializable { ); if (currentAvailableCollateral < vars.maxCollateralToLiquidate) { return ( - uint256(LiquidationErrors.NOT_ENOUGH_LIQUIDITY), + uint256(Errors.LiquidationErrors.NOT_ENOUGH_LIQUIDITY), Errors.NOT_ENOUGH_LIQUIDITY_TO_LIQUIDATE ); } @@ -291,7 +272,7 @@ contract LendingPoolLiquidationManager is VersionedInitializable { receiveAToken ); - return (uint256(LiquidationErrors.NO_ERROR), Errors.NO_ERRORS); + return (uint256(Errors.LiquidationErrors.NO_ERROR), Errors.NO_ERRORS); } /** @@ -314,9 +295,8 @@ contract LendingPoolLiquidationManager is VersionedInitializable { address receiver, bytes calldata params ) external returns (uint256, string memory) { - ReserveLogic.ReserveData storage debtReserve = reserves[principal]; ReserveLogic.ReserveData storage collateralReserve = reserves[collateral]; - + ReserveLogic.ReserveData storage debtReserve = reserves[principal]; UserConfiguration.Map storage userConfig = usersConfig[user]; LiquidationCallLocalVars memory vars; @@ -329,36 +309,20 @@ contract LendingPoolLiquidationManager is VersionedInitializable { addressesProvider.getPriceOracle() ); - if ( - msg.sender != user && vars.healthFactor >= GenericLogic.HEALTH_FACTOR_LIQUIDATION_THRESHOLD - ) { - return ( - uint256(LiquidationErrors.HEALTH_FACTOR_ABOVE_THRESHOLD), - Errors.HEALTH_FACTOR_NOT_BELOW_THRESHOLD - ); - } - - if (msg.sender != user) { - vars.isCollateralEnabled = - collateralReserve.configuration.getLiquidationThreshold() > 0 && - userConfig.isUsingAsCollateral(collateralReserve.index); - - //if collateral isn't enabled as collateral by user, it cannot be liquidated - if (!vars.isCollateralEnabled) { - return ( - uint256(LiquidationErrors.COLLATERAL_CANNOT_BE_LIQUIDATED), - Errors.COLLATERAL_CANNOT_BE_LIQUIDATED - ); - } - } - (vars.userStableDebt, vars.userVariableDebt) = Helpers.getUserCurrentDebt(user, debtReserve); - if (vars.userStableDebt == 0 && vars.userVariableDebt == 0) { - return ( - uint256(LiquidationErrors.CURRRENCY_NOT_BORROWED), - Errors.SPECIFIED_CURRENCY_NOT_BORROWED_BY_USER - ); + (vars.errorCode, vars.errorMsg) = ValidationLogic.validateRepayWithCollateral( + collateralReserve, + debtReserve, + userConfig, + user, + vars.healthFactor, + vars.userStableDebt, + vars.userVariableDebt + ); + + if (Errors.LiquidationErrors(vars.errorCode) != Errors.LiquidationErrors.NO_ERROR) { + return (vars.errorCode, vars.errorMsg); } vars.maxPrincipalAmountToLiquidate = vars.userStableDebt.add(vars.userVariableDebt); @@ -397,7 +361,7 @@ contract LendingPoolLiquidationManager is VersionedInitializable { usersConfig[user].setUsingAsCollateral(collateralReserve.index, false); } - address principalAToken = debtReserve.aTokenAddress; + vars.principalAToken = debtReserve.aTokenAddress; // Notifies the receiver to proceed, sending as param the underlying already transferred ISwapAdapter(receiver).executeOperation( @@ -410,8 +374,8 @@ contract LendingPoolLiquidationManager is VersionedInitializable { //updating debt reserve debtReserve.updateCumulativeIndexesAndTimestamp(); - debtReserve.updateInterestRates(principal, principalAToken, vars.actualAmountToLiquidate, 0); - IERC20(principal).transferFrom(receiver, principalAToken, vars.actualAmountToLiquidate); + debtReserve.updateInterestRates(principal, vars.principalAToken, vars.actualAmountToLiquidate, 0); + IERC20(principal).transferFrom(receiver, vars.principalAToken, vars.actualAmountToLiquidate); if (vars.userVariableDebt >= vars.actualAmountToLiquidate) { IVariableDebtToken(debtReserve.variableDebtTokenAddress).burn( @@ -443,7 +407,7 @@ contract LendingPoolLiquidationManager is VersionedInitializable { vars.maxCollateralToLiquidate ); - return (uint256(LiquidationErrors.NO_ERROR), Errors.NO_ERRORS); + return (uint256(Errors.LiquidationErrors.NO_ERROR), Errors.NO_ERRORS); } struct AvailableCollateralToLiquidateLocalVars { diff --git a/contracts/libraries/helpers/Errors.sol b/contracts/libraries/helpers/Errors.sol index 60fd2da3..3cdb0303 100644 --- a/contracts/libraries/helpers/Errors.sol +++ b/contracts/libraries/helpers/Errors.sol @@ -76,4 +76,14 @@ library Errors { string public constant MULTIPLICATION_OVERFLOW = '44'; string public constant ADDITION_OVERFLOW = '45'; string public constant DIVISION_BY_ZERO = '46'; + + enum LiquidationErrors { + NO_ERROR, + NO_COLLATERAL_AVAILABLE, + COLLATERAL_CANNOT_BE_LIQUIDATED, + CURRRENCY_NOT_BORROWED, + HEALTH_FACTOR_ABOVE_THRESHOLD, + NOT_ENOUGH_LIQUIDITY, + NO_ACTIVE_RESERVE + } } diff --git a/contracts/libraries/logic/ValidationLogic.sol b/contracts/libraries/logic/ValidationLogic.sol index 60d7645c..1cb29b8d 100644 --- a/contracts/libraries/logic/ValidationLogic.sol +++ b/contracts/libraries/logic/ValidationLogic.sol @@ -13,6 +13,7 @@ import {ReserveConfiguration} from '../configuration/ReserveConfiguration.sol'; import {UserConfiguration} from '../configuration/UserConfiguration.sol'; import {IPriceOracleGetter} from '../../interfaces/IPriceOracleGetter.sol'; import {Errors} from '../helpers/Errors.sol'; +import {Helpers} from '../helpers/Helpers.sol'; /** * @title ReserveLogic library @@ -331,18 +332,114 @@ library ValidationLogic { } /** - * @dev Validates configurations for liquidation actions, both liquidationCall() and repayWithCollateral() + * @dev Validates the liquidationCall() action * @param collateralReserve The reserve data of the collateral * @param principalReserve The reserve data of the principal + * @param userConfig The user configuration + * @param userHealthFactor The user's health factor + * @param userStableDebt Total stable debt balance of the user + * @param userVariableDebt Total variable debt balance of the user **/ - function validateLiquidation( + function validateLiquidationCall( ReserveLogic.ReserveData storage collateralReserve, - ReserveLogic.ReserveData storage principalReserve - ) internal view { - require( - collateralReserve.configuration.getActive() && - principalReserve.configuration.getActive(), - Errors.NO_ACTIVE_RESERVE - ); + ReserveLogic.ReserveData storage principalReserve, + UserConfiguration.Map storage userConfig, + 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 + ); + } + + if (userHealthFactor >= GenericLogic.HEALTH_FACTOR_LIQUIDATION_THRESHOLD) { + return ( + uint256(Errors.LiquidationErrors.HEALTH_FACTOR_ABOVE_THRESHOLD), + Errors.HEALTH_FACTOR_NOT_BELOW_THRESHOLD + ); + } + + bool isCollateralEnabled = + collateralReserve.configuration.getLiquidationThreshold() > 0 && + userConfig.isUsingAsCollateral(collateralReserve.index); + + //if collateral isn't enabled as collateral by user, it cannot be liquidated + if (!isCollateralEnabled) { + return ( + uint256(Errors.LiquidationErrors.COLLATERAL_CANNOT_BE_LIQUIDATED), + Errors.COLLATERAL_CANNOT_BE_LIQUIDATED + ); + } + + if (userStableDebt == 0 && userVariableDebt == 0) { + return ( + uint256(Errors.LiquidationErrors.CURRRENCY_NOT_BORROWED), + Errors.SPECIFIED_CURRENCY_NOT_BORROWED_BY_USER + ); + } + + return (uint256(Errors.LiquidationErrors.NO_ERROR), Errors.NO_ERRORS); + } + + /** + * @dev Validates the repayWithCollateral() action + * @param collateralReserve The reserve data of the collateral + * @param principalReserve The reserve data of the principal + * @param userConfig The user configuration + * @param user The address of the user + * @param userHealthFactor The user's health factor + * @param userStableDebt Total stable debt balance of the user + * @param userVariableDebt Total variable debt balance of the user + **/ + function validateRepayWithCollateral( + ReserveLogic.ReserveData storage collateralReserve, + ReserveLogic.ReserveData storage principalReserve, + UserConfiguration.Map storage userConfig, + address user, + 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 + ); + } + + if ( + msg.sender != user && userHealthFactor >= GenericLogic.HEALTH_FACTOR_LIQUIDATION_THRESHOLD + ) { + return ( + uint256(Errors.LiquidationErrors.HEALTH_FACTOR_ABOVE_THRESHOLD), + Errors.HEALTH_FACTOR_NOT_BELOW_THRESHOLD + ); + } + + if (msg.sender != user) { + bool isCollateralEnabled = + collateralReserve.configuration.getLiquidationThreshold() > 0 && + userConfig.isUsingAsCollateral(collateralReserve.index); + + //if collateral isn't enabled as collateral by user, it cannot be liquidated + if (!isCollateralEnabled) { + return ( + uint256(Errors.LiquidationErrors.COLLATERAL_CANNOT_BE_LIQUIDATED), + Errors.COLLATERAL_CANNOT_BE_LIQUIDATED + ); + } + } + + if (userStableDebt == 0 && userVariableDebt == 0) { + return ( + uint256(Errors.LiquidationErrors.CURRRENCY_NOT_BORROWED), + Errors.SPECIFIED_CURRENCY_NOT_BORROWED_BY_USER + ); + } + + return (uint256(Errors.LiquidationErrors.NO_ERROR), Errors.NO_ERRORS); } }