LoopFi #5

[WP-M1] liquidatePosition() should use discountedPrice when determining if a position is in bad debt status, ensuring that only liquidatePositionBadDebt() can be used for bad debt positions.

For instance:

Given:

  • user's debt: 9000
  • user's collateral: 10 ETH
  • current collateral price: 1000 per ETH
  • discount: 20%

Expected:

Since the collateral worth 10000 (math) can only repay a maximum of 8000 (math) in debt, which is less than the user's debt of 9000.

The regular liquidatePosition() without loss should no longer be allowed.

Current implementation:

CDPVault.sol#L526 does not consider liquidationConfig.liquidationDiscount.

CDPVault.sol
@@ 501,508 @@    /// @notice Liquidates a single unsafe position by selling collateral at a discounted (`liquidationDiscount`)
    /// oracle price. The liquidator has to provide the amount he wants to repay or sell (`repayAmounts`) for
    /// the position. From that repay amount a penalty (`liquidationPenalty`) is subtracted to mitigate against
    /// profitable self liquidations. If the available collateral of a position is not sufficient to cover the debt
    /// the vault accumulates 'bad debt'.
    /// @dev The liquidator has to approve the vault to transfer the sum of `repayAmounts`.
    /// @param owner Owner of the position to liquidate
    /// @param repayAmount Amount the liquidator wants to repay [wad]

    function liquidatePosition(address owner, uint256 repayAmount) external whenNotPaused {
        // validate params
        if (owner == address(0) || repayAmount == 0) revert CDPVault__liquidatePosition_invalidParameters();

        // load configs
        VaultConfig memory config = vaultConfig;
        LiquidationConfig memory liqConfig_ = liquidationConfig;

        // load liquidated position
        Position memory position = positions[owner];
        DebtData memory debtData = _calcDebt(position);

        // load price and calculate discounted price
        uint256 spotPrice_ = spotPrice();
        uint256 discountedPrice = wmul(spotPrice_, liqConfig_.liquidationDiscount);
        if (spotPrice_ == 0) revert CDPVault__liquidatePosition_invalidSpotPrice();
        // Enusure that there's no bad debt
        if (calcTotalDebt(debtData) > wmul(position.collateral, spotPrice_)) revert CDPVault__BadDebt();


@@ 528,573 @@        // compute collateral to take, debt to repay and penalty to pay
        uint256 takeCollateral = wdiv(repayAmount, discountedPrice);
        uint256 deltaDebt = wmul(repayAmount, liqConfig_.liquidationPenalty);
        uint256 penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty);
        if (takeCollateral > position.collateral) revert CDPVault__tooHighRepayAmount();

        // verify that the position is indeed unsafe
        if (_isCollateralized(calcTotalDebt(debtData), wmul(position.collateral, spotPrice_), config.liquidationRatio))
            revert CDPVault__liquidatePosition_notUnsafe();

        // transfer the repay amount from the liquidator to the vault
        poolUnderlying.safeTransferFrom(msg.sender, address(pool), repayAmount - penalty);

        uint256 newDebt;
        uint256 profit;
        uint256 maxRepayment = calcTotalDebt(debtData);
        uint256 newCumulativeIndex;
        if (deltaDebt == maxRepayment) {
            newDebt = 0;
            newCumulativeIndex = debtData.cumulativeIndexNow;
            profit = debtData.accruedInterest;
            position.cumulativeQuotaInterest = 0;
        } else {
            (newDebt, newCumulativeIndex, profit, position.cumulativeQuotaInterest) = calcDecrease(
                deltaDebt, // delta debt
                debtData.debt,
                debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
                debtData.cumulativeIndexLastUpdate,
                debtData.cumulativeQuotaInterest
            );
        }
        position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
        // update liquidated position
        position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, -toInt256(takeCollateral), totalDebt);

        pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11]
        // transfer the collateral amount from the vault to the liquidator
        token.safeTransfer(msg.sender, takeCollateral);

        // Mint the penalty from the vault to the treasury
        poolUnderlying.safeTransferFrom(msg.sender, address(pool), penalty);
        IPoolV3Loop(address(pool)).mintProfit(penalty);

        if (debtData.debt - newDebt != 0) {
            IPoolV3(pool).updateQuotaRevenue(_calcQuotaRevenueChange(-int(debtData.debt - newDebt))); // U:[PQK-15]
        }

    }
CDPVault.sol
@@ 501,508 @@    /// @notice Liquidates a single unsafe position by selling collateral at a discounted (`liquidationDiscount`)
    /// oracle price. The liquidator has to provide the amount he wants to repay or sell (`repayAmounts`) for
    /// the position. From that repay amount a penalty (`liquidationPenalty`) is subtracted to mitigate against
    /// profitable self liquidations. If the available collateral of a position is not sufficient to cover the debt
    /// the vault accumulates 'bad debt'.
    /// @dev The liquidator has to approve the vault to transfer the sum of `repayAmounts`.
    /// @param owner Owner of the position to liquidate
    /// @param repayAmount Amount the liquidator wants to repay [wad]

    function liquidatePosition(address owner, uint256 repayAmount) external whenNotPaused {
        // validate params
        if (owner == address(0) || repayAmount == 0) revert CDPVault__liquidatePosition_invalidParameters();

        // load configs
        VaultConfig memory config = vaultConfig;
        LiquidationConfig memory liqConfig_ = liquidationConfig;

        // load liquidated position
        Position memory position = positions[owner];
        DebtData memory debtData = _calcDebt(position);

        // load price and calculate discounted price
        uint256 spotPrice_ = spotPrice();
        uint256 discountedPrice = wmul(spotPrice_, liqConfig_.liquidationDiscount);
        if (spotPrice_ == 0) revert CDPVault__liquidatePosition_invalidSpotPrice();
        // Enusure that there's no bad debt
        if (calcTotalDebt(debtData) > wmul(position.collateral, spotPrice_)) revert CDPVault__BadDebt();


@@ 528,573 @@        // compute collateral to take, debt to repay and penalty to pay
        uint256 takeCollateral = wdiv(repayAmount, discountedPrice);
        uint256 deltaDebt = wmul(repayAmount, liqConfig_.liquidationPenalty);
        uint256 penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty);
        if (takeCollateral > position.collateral) revert CDPVault__tooHighRepayAmount();

        // verify that the position is indeed unsafe
        if (_isCollateralized(calcTotalDebt(debtData), wmul(position.collateral, spotPrice_), config.liquidationRatio))
            revert CDPVault__liquidatePosition_notUnsafe();

        // transfer the repay amount from the liquidator to the vault
        poolUnderlying.safeTransferFrom(msg.sender, address(pool), repayAmount - penalty);

        uint256 newDebt;
        uint256 profit;
        uint256 maxRepayment = calcTotalDebt(debtData);
        uint256 newCumulativeIndex;
        if (deltaDebt == maxRepayment) {
            newDebt = 0;
            newCumulativeIndex = debtData.cumulativeIndexNow;
            profit = debtData.accruedInterest;
            position.cumulativeQuotaInterest = 0;
        } else {
            (newDebt, newCumulativeIndex, profit, position.cumulativeQuotaInterest) = calcDecrease(
                deltaDebt, // delta debt
                debtData.debt,
                debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
                debtData.cumulativeIndexLastUpdate,
                debtData.cumulativeQuotaInterest
            );
        }
        position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
        // update liquidated position
        position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, -toInt256(takeCollateral), totalDebt);

        pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11]
        // transfer the collateral amount from the vault to the liquidator
        token.safeTransfer(msg.sender, takeCollateral);

        // Mint the penalty from the vault to the treasury
        poolUnderlying.safeTransferFrom(msg.sender, address(pool), penalty);
        IPoolV3Loop(address(pool)).mintProfit(penalty);

        if (debtData.debt - newDebt != 0) {
            IPoolV3(pool).updateQuotaRevenue(_calcQuotaRevenueChange(-int(debtData.debt - newDebt))); // U:[PQK-15]
        }

    }

[WP-M2] PositionAction4626._onWithdraw() should convert collateralWithdrawn to the correct scale

This issue will only have an impact when the ERC4626's decimals is not 18.

The current implementation of collateralWithdrawn is in [wad] scale, but:

  1. The first parameter of L67 collateralWithdrawn = IERC4626(collateral).redeem(collateralWithdrawn, address(this), address(this)) expects an amount in math scale.

  2. The return value of PositionAction4626._onWithdraw() is expected to be in [CDPVault.tokenScale()] scale, see PositionAction4626.sol#L60 and PositionAction.sol#L544.

PositionAction4626.sol
    /// @notice Withdraw collateral from the vault
    /// @param vault Address of the vault
    /// @param /*position*/ Address of the position
    /// @param dst Token the caller expects to receive
    /// @param amount Amount of collateral to withdraw [wad]
    /// @return Amount of collateral withdrawn [CDPVault.tokenScale()]
    function _onWithdraw(address vault, address /*position*/, address dst, uint256 amount) internal override returns (uint256) {
        uint256 collateralWithdrawn = ICDPVault(vault).withdraw(address(this), amount);

        // if collateral is not the dst token, we need to withdraw the underlying from the ERC4626 vault
        address collateral = address(ICDPVault(vault).token());
        if (dst != collateral) {
            collateralWithdrawn = IERC4626(collateral).redeem(collateralWithdrawn, address(this), address(this));
        }

        return collateralWithdrawn;
    }
CDPVault.sol
    /// @notice Withdraws collateral tokens from this contract and decreases a user's collateral balance
    /// @param to Address of the user to withdraw tokens to
    /// @param amount Amount of tokens to withdraw [tokenScale]
    /// @return tokenAmount Amount of tokens withdrawn [wad]
    function withdraw(address to, uint256 amount) external whenNotPaused returns (uint256 tokenAmount) {
        tokenAmount = wdiv(amount, tokenScale);
        int256 deltaCollateral = -toInt256(tokenAmount);
        modifyCollateralAndDebt({
            owner: to,
            collateralizer: msg.sender,
            creditor: msg.sender,
            deltaCollateral: deltaCollateral,
            deltaDebt: 0
        });
    }
PositionAction.sol
    /// @notice Withdraws collateral from CDPVault (optionally swaps collateral to an arbitrary token)
    /// @param vault The CDP Vault
    /// @param collateralParams The collateral parameters
    /// @return The amount of collateral withdrawn [token.decimals()]
    function _withdraw(address vault, address position, CollateralParams calldata collateralParams) internal returns (uint256) {
        uint256 collateral = _onWithdraw(vault, position, collateralParams.targetToken, collateralParams.amount);

        // perform swap from collateral to arbitrary token
        if (collateralParams.auxSwap.assetIn != address(0)) {
            _delegateCall(
                address(swapAction),
                abi.encodeWithSelector(swapAction.swap.selector, collateralParams.auxSwap)
            );
        } else {
            // otherwise just send the collateral to `collateralizer`
            IERC20(collateralParams.targetToken).safeTransfer(collateralParams.collateralizer, collateral);
        }
        return collateral;
    }
PositionAction4626.sol
    /// @notice Withdraw collateral from the vault
    /// @param vault Address of the vault
    /// @param /*position*/ Address of the position
    /// @param dst Token the caller expects to receive
    /// @param amount Amount of collateral to withdraw [wad]
    /// @return Amount of collateral withdrawn [CDPVault.tokenScale()]
    function _onWithdraw(address vault, address /*position*/, address dst, uint256 amount) internal override returns (uint256) {
        uint256 collateralWithdrawn = ICDPVault(vault).withdraw(address(this), amount);

        // if collateral is not the dst token, we need to withdraw the underlying from the ERC4626 vault
        address collateral = address(ICDPVault(vault).token());
        if (dst != collateral) {
            collateralWithdrawn = IERC4626(collateral).redeem(collateralWithdrawn, address(this), address(this));
        }

        return collateralWithdrawn;
    }
CDPVault.sol
    /// @notice Withdraws collateral tokens from this contract and decreases a user's collateral balance
    /// @param to Address of the user to withdraw tokens to
    /// @param amount Amount of tokens to withdraw [tokenScale]
    /// @return tokenAmount Amount of tokens withdrawn [wad]
    function withdraw(address to, uint256 amount) external whenNotPaused returns (uint256 tokenAmount) {
        tokenAmount = wdiv(amount, tokenScale);
        int256 deltaCollateral = -toInt256(tokenAmount);
        modifyCollateralAndDebt({
            owner: to,
            collateralizer: msg.sender,
            creditor: msg.sender,
            deltaCollateral: deltaCollateral,
            deltaDebt: 0
        });
    }
PositionAction.sol
    /// @notice Withdraws collateral from CDPVault (optionally swaps collateral to an arbitrary token)
    /// @param vault The CDP Vault
    /// @param collateralParams The collateral parameters
    /// @return The amount of collateral withdrawn [token.decimals()]
    function _withdraw(address vault, address position, CollateralParams calldata collateralParams) internal returns (uint256) {
        uint256 collateral = _onWithdraw(vault, position, collateralParams.targetToken, collateralParams.amount);

        // perform swap from collateral to arbitrary token
        if (collateralParams.auxSwap.assetIn != address(0)) {
            _delegateCall(
                address(swapAction),
                abi.encodeWithSelector(swapAction.swap.selector, collateralParams.auxSwap)
            );
        } else {
            // otherwise just send the collateral to `collateralizer`
            IERC20(collateralParams.targetToken).safeTransfer(collateralParams.collateralizer, collateral);
        }
        return collateral;
    }

[WP-L3] liquidatePositionBadDebt() Unused or Redundant repayAmount Parameter in Actual Business Logic

In the current implementation, the repayAmount actually used is calculated on-the-fly based on the position's state (position.collateral, discountedPrice) at L605-606.

The actual amount of poolUnderlying tokens that the liquidator needs to pay is unrelated to the repayAmount parameter.

CDPVault.sol
    /// @dev The liquidator has to approve the vault to transfer the sum of `repayAmounts`.
    /// @param owner Owner of the position to liquidate
    /// @param repayAmount Amount the liquidator wants to repay [wad]
    function liquidatePositionBadDebt(address owner, uint256 repayAmount) external whenNotPaused {
        // validate params
        if (owner == address(0) || repayAmount == 0) revert CDPVault__liquidatePosition_invalidParameters();

        // load configs
        VaultConfig memory config = vaultConfig;
        LiquidationConfig memory liqConfig_ = liquidationConfig;

        // load liquidated position
        Position memory position = positions[owner];
        DebtData memory debtData = _calcDebt(position);
        uint256 spotPrice_ = spotPrice();
        if (spotPrice_ == 0) revert CDPVault__liquidatePosition_invalidSpotPrice();
        // verify that the position is indeed unsafe
        if (_isCollateralized(calcTotalDebt(debtData), wmul(position.collateral, spotPrice_), config.liquidationRatio))
            revert CDPVault__liquidatePosition_notUnsafe();

        // load price and calculate discounted price
        uint256 discountedPrice = wmul(spotPrice_, liqConfig_.liquidationDiscount);
        // Enusure that the debt is greater than the collateral at discounted price
        if (calcTotalDebt(debtData) <= wmul(position.collateral, discountedPrice)) revert CDPVault__noBadDebt();
        // compute collateral to take, debt to repay
        uint256 takeCollateral = wdiv(repayAmount, discountedPrice);
        if (takeCollateral < position.collateral) revert CDPVault__repayAmountNotEnough();

        // account for bad debt
        takeCollateral = position.collateral;
        repayAmount = wmul(takeCollateral, discountedPrice);
        uint256 loss = calcTotalDebt(debtData) - repayAmount;

        // transfer the repay amount from the liquidator to the vault
        poolUnderlying.safeTransferFrom(msg.sender, address(pool), repayAmount);

        position.cumulativeQuotaInterest = 0;
        position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
        // update liquidated position
        position = _modifyPosition(
            owner,
            position,
            0,
            debtData.cumulativeIndexNow,
            -toInt256(takeCollateral),
            totalDebt
        );

        pool.repayCreditAccount(debtData.debt, 0, loss); // U:[CM-11]
        // transfer the collateral amount from the vault to the liquidator
        token.safeTransfer(msg.sender, takeCollateral);

        int256 quotaRevenueChange = _calcQuotaRevenueChange(-int(debtData.debt));
        if (quotaRevenueChange != 0) {
            IPoolV3(pool).updateQuotaRevenue(quotaRevenueChange); // U:[PQK-15]
        }
    }
CDPVault.sol
    /// @dev The liquidator has to approve the vault to transfer the sum of `repayAmounts`.
    /// @param owner Owner of the position to liquidate
    /// @param repayAmount Amount the liquidator wants to repay [wad]
    function liquidatePositionBadDebt(address owner, uint256 repayAmount) external whenNotPaused {
        // validate params
        if (owner == address(0) || repayAmount == 0) revert CDPVault__liquidatePosition_invalidParameters();

        // load configs
        VaultConfig memory config = vaultConfig;
        LiquidationConfig memory liqConfig_ = liquidationConfig;

        // load liquidated position
        Position memory position = positions[owner];
        DebtData memory debtData = _calcDebt(position);
        uint256 spotPrice_ = spotPrice();
        if (spotPrice_ == 0) revert CDPVault__liquidatePosition_invalidSpotPrice();
        // verify that the position is indeed unsafe
        if (_isCollateralized(calcTotalDebt(debtData), wmul(position.collateral, spotPrice_), config.liquidationRatio))
            revert CDPVault__liquidatePosition_notUnsafe();

        // load price and calculate discounted price
        uint256 discountedPrice = wmul(spotPrice_, liqConfig_.liquidationDiscount);
        // Enusure that the debt is greater than the collateral at discounted price
        if (calcTotalDebt(debtData) <= wmul(position.collateral, discountedPrice)) revert CDPVault__noBadDebt();
        // compute collateral to take, debt to repay
        uint256 takeCollateral = wdiv(repayAmount, discountedPrice);
        if (takeCollateral < position.collateral) revert CDPVault__repayAmountNotEnough();

        // account for bad debt
        takeCollateral = position.collateral;
        repayAmount = wmul(takeCollateral, discountedPrice);
        uint256 loss = calcTotalDebt(debtData) - repayAmount;

        // transfer the repay amount from the liquidator to the vault
        poolUnderlying.safeTransferFrom(msg.sender, address(pool), repayAmount);

        position.cumulativeQuotaInterest = 0;
        position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
        // update liquidated position
        position = _modifyPosition(
            owner,
            position,
            0,
            debtData.cumulativeIndexNow,
            -toInt256(takeCollateral),
            totalDebt
        );

        pool.repayCreditAccount(debtData.debt, 0, loss); // U:[CM-11]
        // transfer the collateral amount from the vault to the liquidator
        token.safeTransfer(msg.sender, takeCollateral);

        int256 quotaRevenueChange = _calcQuotaRevenueChange(-int(debtData.debt));
        if (quotaRevenueChange != 0) {
            IPoolV3(pool).updateQuotaRevenue(quotaRevenueChange); // U:[PQK-15]
        }
    }

[WP-I4] The fixed-ratio penalty in liquidatePosition() may lead to bad debt

For instance:

Given:

  • user's debt: 7500
  • user's collateral: 10 ETH
  • current collateral price: 1000 per ETH
  • discount: 20%
  • liquidationConfig.liquidationPenalty: 9e17 (90%)

Expected:

Since collateral worth 10000 (math) can repay up to 8000 (math) of debt, and 8000 is greater than the user's debt of 7500, the liquidator should be able to clear the position using liquidatePosition() without any loss.

Current implementation:

For collateral worth 10000, the maximum repayAmount is 8000 (math). The corresponding maximum deltaDebt at L530 is 7200 (math), and the penalty at L531 is 800 (math).

liquidatePosition() will:

  • Transfer collateral worth 10000 to the liquidator
  • Collect 8000 poolUnderlying tokens from the liquidator
    • Reduce the position's total debt from 7500 to 300 (math)
    • Mint 800 penalty to the treasury

The position now becomes a bad debt position with a 300 loss, which would not be in bad debt if there is no penalty.

Unlike the liquidationDiscount, the penalty is charged by the protocol. There is no incentive to urge the liquidator by having a higher percentage of penalty (while a higher liquidationDiscount should help incentivize the liquidator).

We argue that the penalty should not be charged at the expense of creating bad debt.

CDPVault.sol
@@ 501,508 @@    /// @notice Liquidates a single unsafe position by selling collateral at a discounted (`liquidationDiscount`)
    /// oracle price. The liquidator has to provide the amount he wants to repay or sell (`repayAmounts`) for
    /// the position. From that repay amount a penalty (`liquidationPenalty`) is subtracted to mitigate against
    /// profitable self liquidations. If the available collateral of a position is not sufficient to cover the debt
    /// the vault accumulates 'bad debt'.
    /// @dev The liquidator has to approve the vault to transfer the sum of `repayAmounts`.
    /// @param owner Owner of the position to liquidate
    /// @param repayAmount Amount the liquidator wants to repay [wad]

    function liquidatePosition(address owner, uint256 repayAmount) external whenNotPaused {

@@ 510,524 @@        // validate params
        if (owner == address(0) || repayAmount == 0) revert CDPVault__liquidatePosition_invalidParameters();

        // load configs
        VaultConfig memory config = vaultConfig;
        LiquidationConfig memory liqConfig_ = liquidationConfig;

        // load liquidated position
        Position memory position = positions[owner];
        DebtData memory debtData = _calcDebt(position);

        // load price and calculate discounted price
        uint256 spotPrice_ = spotPrice();
        uint256 discountedPrice = wmul(spotPrice_, liqConfig_.liquidationDiscount);
        if (spotPrice_ == 0) revert CDPVault__liquidatePosition_invalidSpotPrice();

        // Enusure that there's no bad debt
        if (calcTotalDebt(debtData) > wmul(position.collateral, spotPrice_)) revert CDPVault__BadDebt();

        // compute collateral to take, debt to repay and penalty to pay
        uint256 takeCollateral = wdiv(repayAmount, discountedPrice);
        uint256 deltaDebt = wmul(repayAmount, liqConfig_.liquidationPenalty);
        uint256 penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty);
        if (takeCollateral > position.collateral) revert CDPVault__tooHighRepayAmount();

        // verify that the position is indeed unsafe
        if (_isCollateralized(calcTotalDebt(debtData), wmul(position.collateral, spotPrice_), config.liquidationRatio))
            revert CDPVault__liquidatePosition_notUnsafe();

        // transfer the repay amount from the liquidator to the vault
        poolUnderlying.safeTransferFrom(msg.sender, address(pool), repayAmount - penalty);

        uint256 newDebt;
        uint256 profit;
        uint256 maxRepayment = calcTotalDebt(debtData);
        uint256 newCumulativeIndex;
        if (deltaDebt == maxRepayment) {
            newDebt = 0;
            newCumulativeIndex = debtData.cumulativeIndexNow;
            profit = debtData.accruedInterest;
            position.cumulativeQuotaInterest = 0;
        } else {
            (newDebt, newCumulativeIndex, profit, position.cumulativeQuotaInterest) = calcDecrease(
                deltaDebt, // delta debt
                debtData.debt,
                debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
                debtData.cumulativeIndexLastUpdate,
                debtData.cumulativeQuotaInterest
            );
        }
        position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
        // update liquidated position
        position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, -toInt256(takeCollateral), totalDebt);

        pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11]
        // transfer the collateral amount from the vault to the liquidator
        token.safeTransfer(msg.sender, takeCollateral);

        // Mint the penalty from the vault to the treasury
        poolUnderlying.safeTransferFrom(msg.sender, address(pool), penalty);
        IPoolV3Loop(address(pool)).mintProfit(penalty);

        if (debtData.debt - newDebt != 0) {
            IPoolV3(pool).updateQuotaRevenue(_calcQuotaRevenueChange(-int(debtData.debt - newDebt))); // U:[PQK-15]
        }
    }
CDPVault.sol
@@ 501,508 @@    /// @notice Liquidates a single unsafe position by selling collateral at a discounted (`liquidationDiscount`)
    /// oracle price. The liquidator has to provide the amount he wants to repay or sell (`repayAmounts`) for
    /// the position. From that repay amount a penalty (`liquidationPenalty`) is subtracted to mitigate against
    /// profitable self liquidations. If the available collateral of a position is not sufficient to cover the debt
    /// the vault accumulates 'bad debt'.
    /// @dev The liquidator has to approve the vault to transfer the sum of `repayAmounts`.
    /// @param owner Owner of the position to liquidate
    /// @param repayAmount Amount the liquidator wants to repay [wad]

    function liquidatePosition(address owner, uint256 repayAmount) external whenNotPaused {

@@ 510,524 @@        // validate params
        if (owner == address(0) || repayAmount == 0) revert CDPVault__liquidatePosition_invalidParameters();

        // load configs
        VaultConfig memory config = vaultConfig;
        LiquidationConfig memory liqConfig_ = liquidationConfig;

        // load liquidated position
        Position memory position = positions[owner];
        DebtData memory debtData = _calcDebt(position);

        // load price and calculate discounted price
        uint256 spotPrice_ = spotPrice();
        uint256 discountedPrice = wmul(spotPrice_, liqConfig_.liquidationDiscount);
        if (spotPrice_ == 0) revert CDPVault__liquidatePosition_invalidSpotPrice();

        // Enusure that there's no bad debt
        if (calcTotalDebt(debtData) > wmul(position.collateral, spotPrice_)) revert CDPVault__BadDebt();

        // compute collateral to take, debt to repay and penalty to pay
        uint256 takeCollateral = wdiv(repayAmount, discountedPrice);
        uint256 deltaDebt = wmul(repayAmount, liqConfig_.liquidationPenalty);
        uint256 penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty);
        if (takeCollateral > position.collateral) revert CDPVault__tooHighRepayAmount();

        // verify that the position is indeed unsafe
        if (_isCollateralized(calcTotalDebt(debtData), wmul(position.collateral, spotPrice_), config.liquidationRatio))
            revert CDPVault__liquidatePosition_notUnsafe();

        // transfer the repay amount from the liquidator to the vault
        poolUnderlying.safeTransferFrom(msg.sender, address(pool), repayAmount - penalty);

        uint256 newDebt;
        uint256 profit;
        uint256 maxRepayment = calcTotalDebt(debtData);
        uint256 newCumulativeIndex;
        if (deltaDebt == maxRepayment) {
            newDebt = 0;
            newCumulativeIndex = debtData.cumulativeIndexNow;
            profit = debtData.accruedInterest;
            position.cumulativeQuotaInterest = 0;
        } else {
            (newDebt, newCumulativeIndex, profit, position.cumulativeQuotaInterest) = calcDecrease(
                deltaDebt, // delta debt
                debtData.debt,
                debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
                debtData.cumulativeIndexLastUpdate,
                debtData.cumulativeQuotaInterest
            );
        }
        position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
        // update liquidated position
        position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, -toInt256(takeCollateral), totalDebt);

        pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11]
        // transfer the collateral amount from the vault to the liquidator
        token.safeTransfer(msg.sender, takeCollateral);

        // Mint the penalty from the vault to the treasury
        poolUnderlying.safeTransferFrom(msg.sender, address(pool), penalty);
        IPoolV3Loop(address(pool)).mintProfit(penalty);

        if (debtData.debt - newDebt != 0) {
            IPoolV3(pool).updateQuotaRevenue(_calcQuotaRevenueChange(-int(debtData.debt - newDebt))); // U:[PQK-15]
        }
    }

[WP-I5] CDPVault.modifyCollateralAndDebt() Uses a Stricter Health Threshold Than Liquidation

This prevents users from creating positions that can easily result in losses for the owner during liquidation.

For example, Aave and Silo set the maximum borrowing limit for position owners below the liquidation threshold.

[WP-N6] Unused new library QuotasLogic

Commit 55d8296 "Merge pull request #5 from LoopFi/feat/quotas" introduced a new file src/quotas/QuotasLogic.sol, but its code is not being used.

For example, CDPVault.sol and PoolQuotaKeeperV3.sol are still using QuotasLogic.sol from @gearbox-protocol/core-v3:

Note: We observed that the contents of src/quotas/QuotasLogic.sol and lib/core-v3/contracts/libraries/QuotasLogic.sol are identical.

https://github.com/LoopFi/loop-contracts/blob/7523ffd51935bc732737ddb5c403635c9c9d8b0a/src/CDPVault.sol#L15-L20

import {IChefIncentivesController} from "./reward/interfaces/IChefIncentivesController.sol";
import {IPoolV3} from "@gearbox-protocol/core-v3/contracts/interfaces/IPoolV3.sol";
import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
import {CreditLogic} from "@gearbox-protocol/core-v3/contracts/libraries/CreditLogic.sol";
import {QuotasLogic} from "@gearbox-protocol/core-v3/contracts/libraries/QuotasLogic.sol";
import {IPoolQuotaKeeperV3} from "@gearbox-protocol/core-v3/contracts/interfaces/IPoolQuotaKeeperV3.sol";

https://github.com/LoopFi/loop-contracts/blob/7523ffd51935bc732737ddb5c403635c9c9d8b0a/src/quotas/PoolQuotaKeeperV3.sol#L9-L22

/// LIBS & TRAITS
import {ACLNonReentrantTrait} from "@gearbox-protocol/core-v3/contracts/traits/ACLNonReentrantTrait.sol";
import {ContractsRegisterTrait} from "@gearbox-protocol/core-v3/contracts/traits/ContractsRegisterTrait.sol";
import {QuotasLogic} from "@gearbox-protocol/core-v3/contracts/libraries/QuotasLogic.sol";

import {IPoolV3} from "@gearbox-protocol/core-v3/contracts/interfaces/IPoolV3.sol";
import {IPoolQuotaKeeperV3, TokenQuotaParams, AccountQuota} from "src/interfaces/IPoolQuotaKeeperV3.sol";
import {IGaugeV3} from "@gearbox-protocol/core-v3/contracts/interfaces/IGaugeV3.sol";
import {ICreditManagerV3} from "@gearbox-protocol/core-v3/contracts/interfaces/ICreditManagerV3.sol";

import {PERCENTAGE_FACTOR, RAY} from "@gearbox-protocol/core-v2/contracts/libraries/Constants.sol";

// EXCEPTIONS
import "@gearbox-protocol/core-v3/contracts/interfaces/IExceptions.sol";

[WP-N7] to is actually from / owner, not "Address of the user to withdraw tokens to"

The comment at L236 is misleading.

CDPVault.sol
    /// @notice Withdraws collateral tokens from this contract and decreases a user's collateral balance
    /// @param to Address of the user to withdraw tokens to
    /// @param amount Amount of tokens to withdraw [tokenScale]
    /// @return tokenAmount Amount of tokens withdrawn [wad]
    function withdraw(address to, uint256 amount) external whenNotPaused returns (uint256 tokenAmount) {
        tokenAmount = wdiv(amount, tokenScale);
        int256 deltaCollateral = -toInt256(tokenAmount);
        modifyCollateralAndDebt({
            owner: to,
            collateralizer: msg.sender,
            creditor: msg.sender,
            deltaCollateral: deltaCollateral,
            deltaDebt: 0
        });
    }
CDPVault.sol
    /// @notice Withdraws collateral tokens from this contract and decreases a user's collateral balance
    /// @param to Address of the user to withdraw tokens to
    /// @param amount Amount of tokens to withdraw [tokenScale]
    /// @return tokenAmount Amount of tokens withdrawn [wad]
    function withdraw(address to, uint256 amount) external whenNotPaused returns (uint256 tokenAmount) {
        tokenAmount = wdiv(amount, tokenScale);
        int256 deltaCollateral = -toInt256(tokenAmount);
        modifyCollateralAndDebt({
            owner: to,
            collateralizer: msg.sender,
            creditor: msg.sender,
            deltaCollateral: deltaCollateral,
            deltaDebt: 0
        });
    }