LoopFi #4

[WP-M1] BalancerOracle.sol should use different methods to retrieve Supply values for different pools.

BalancerOracle.sol
function update() external virtual onlyRole(KEEPER_ROLE) returns (uint256 safePrice_) {
    if (block.timestamp - lastUpdate < updateWaitWindow) revert BalancerOracle__update_InUpdateWaitWindow();
    // update the safe price first
    safePrice = safePrice_ = currentPrice;
    lastUpdate = block.timestamp;

    uint256[] memory weights = IWeightedPool(pool).getNormalizedWeights();
    uint256 totalSupply = IWeightedPool(pool).totalSupply();

    uint256 totalPi = WAD;
    uint256[] memory prices = new uint256[](weights.length);
    // update balances in 18 decimals
    for (uint256 i = 0; i < weights.length; i++) {
        // reverts if the price is invalid or stale
        prices[i] = _getTokenPrice(i);
        uint256 val = wdiv(prices[i], weights[i]);
        uint256 indivPi = uint256(wpow(int256(val), int256(weights[i])));

        totalPi = wmul(totalPi, indivPi);
    }

    currentPrice = wdiv(wmul(totalPi, IWeightedPool(pool).getInvariant()), totalSupply);
}

BalancerOracle.sol#L121 is used for calculating the price of Balancer LP.

However, per to Balancer Docs, there are multiple methods to get the supply of a balancer pool.

getActualSupply

This is the most common/current function to call. getActualSupply is used by the most recent versions of Weighted and Stable Pools. It accounts for pre-minted BPT as well as due protocol fees.

getVirtualSupply

This is used by Linear Pools and "legacy" Stable Phantom Pools. It accounts for pre-minted BPT but does not take due protocol fees into account. The omission of protocol fee calculations in Linear Pools is intentional since they do not pay protocol fees.

totalSupply

In general, totalSupply only makes sense to call for older "legacy" pools. The original Weighted and Stable Pools do not have pre-minted BPT, so they follow the typical convention of using totalSupply to account for issued pool shares.

And totalSupply is only good for legacy pools.

See also: https://github.com/sherlock-audit/2023-07-blueberry-judging/issues/18

BalancerOracle.sol
function update() external virtual onlyRole(KEEPER_ROLE) returns (uint256 safePrice_) {
    if (block.timestamp - lastUpdate < updateWaitWindow) revert BalancerOracle__update_InUpdateWaitWindow();
    // update the safe price first
    safePrice = safePrice_ = currentPrice;
    lastUpdate = block.timestamp;

    uint256[] memory weights = IWeightedPool(pool).getNormalizedWeights();
    uint256 totalSupply = IWeightedPool(pool).totalSupply();

    uint256 totalPi = WAD;
    uint256[] memory prices = new uint256[](weights.length);
    // update balances in 18 decimals
    for (uint256 i = 0; i < weights.length; i++) {
        // reverts if the price is invalid or stale
        prices[i] = _getTokenPrice(i);
        uint256 val = wdiv(prices[i], weights[i]);
        uint256 indivPi = uint256(wpow(int256(val), int256(weights[i])));

        totalPi = wmul(totalPi, indivPi);
    }

    currentPrice = wdiv(wmul(totalPi, IWeightedPool(pool).getInvariant()), totalSupply);
}

[WP-M2] modifyCollateralAndDebt() Unexpectedly Waives QuotaInterest When deltaDebt > 0

The function fails to update position.cumulativeQuotaInterest but sets position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;

CDPVault.sol
@@ 350,363 @@    /// @notice Modifies a Position's collateral and debt balances
    /// @dev Checks that the global debt ceiling and the vault's debt ceiling have not been exceeded via the CDM,
    /// - that the Position is still safe after the modification,
    /// - that the msg.sender has the permission of the owner to decrease the collateral-to-debt ratio,
    /// - that the msg.sender has the permission of the collateralizer to put up new collateral,
    /// - that the msg.sender has the permission of the creditor to settle debt with their credit,
    /// - that that the vault debt floor is exceeded
    /// - that the vault minimum collateralization ratio is met
    /// @param owner Address of the owner of the position
    /// @param collateralizer Address of who puts up or receives the collateral delta
    /// @param creditor Address of who provides or receives the credit delta for the debt delta
    /// @param deltaCollateral Amount of collateral to put up (+) or to remove (-) from the position [wad]
    /// @param deltaDebt Amount of normalized debt (gross, before rate is applied) to generate (+) or
    /// to settle (-) on this position [wad]

    function modifyCollateralAndDebt(
        address owner,
        address collateralizer,
        address creditor,
        int256 deltaCollateral,
        int256 deltaDebt
    ) public {

@@ 371,387 @@        if (
            // position is either more safe than before or msg.sender has the permission from the owner
            ((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) ||
            // msg.sender has the permission of the collateralizer to collateralize the position using their cash
            (deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) ||
            // msg.sender has the permission of the creditor to use their credit to repay the debt
            (deltaDebt < 0 && !hasPermission(creditor, msg.sender))
        ) revert CDPVault__modifyCollateralAndDebt_noPermission();

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

        uint256 newDebt;
        uint256 newCumulativeIndex;

        uint256 profit;
        int256 quotaRevenueChange;

        if (deltaDebt > 0) {
            (newDebt, newCumulativeIndex) = CreditLogic.calcIncrease(
                uint256(deltaDebt), // delta debt
                position.debt,
                debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
                position.cumulativeIndexLastUpdate
            ); // U:[CM-10]
            position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
            quotaRevenueChange = _calcQuotaRevenueChange(deltaDebt);
            pool.lendCreditAccount(uint256(deltaDebt), creditor); // F:[CM-20]
        } else if (deltaDebt < 0) {

@@ 399,427 @@            uint256 maxRepayment = calcTotalDebt(debtData);
            uint256 amount = abs(deltaDebt);
            if (amount >= maxRepayment) {
                amount = maxRepayment; // U:[CM-11]
                deltaDebt = -toInt256(maxRepayment);
            }

            poolUnderlying.safeTransferFrom(creditor, address(pool), amount);

            uint128 newCumulativeQuotaInterest;
            if (amount == maxRepayment) {
                newDebt = 0;
                newCumulativeIndex = debtData.cumulativeIndexNow;
                profit = debtData.accruedInterest;
                newCumulativeQuotaInterest = 0;
            } else {
                (newDebt, newCumulativeIndex, profit, newCumulativeQuotaInterest) = calcDecrease(
                    amount, // delta debt
                    position.debt,
                    debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
                    position.cumulativeIndexLastUpdate,
                    debtData.cumulativeQuotaInterest
                );
            }
            quotaRevenueChange = _calcQuotaRevenueChange(-int(debtData.debt - newDebt));
            pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11]

            position.cumulativeQuotaInterest = newCumulativeQuotaInterest;
            position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;

        } else {

@@ 429,430 @@            newDebt = position.debt;
            newCumulativeIndex = debtData.cumulativeIndexLastUpdate;

        }


@@ 433,439 @@        if (deltaCollateral > 0) {
            uint256 amount = wmul(deltaCollateral.toUint256(), tokenScale);
            token.safeTransferFrom(collateralizer, address(this), amount);
        } else if (deltaCollateral < 0) {
            uint256 amount = wmul(abs(deltaCollateral), tokenScale);
            token.safeTransfer(collateralizer, amount);
        }


        position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt);


@@ 443,455 @@        VaultConfig memory config = vaultConfig;
        uint256 spotPrice_ = spotPrice();
        uint256 collateralValue = wmul(position.collateral, spotPrice_);

        if (
            (deltaDebt > 0 || deltaCollateral < 0) &&
            !_isCollateralized(calcTotalDebt(_calcDebt(position)), collateralValue, config.liquidationRatio)
        ) revert CDPVault__modifyCollateralAndDebt_notSafe();

        if (quotaRevenueChange != 0) {
            IPoolV3(pool).updateQuotaRevenue(quotaRevenueChange); // U:[PQK-15]
        }
        emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt);

    }

Recommendation

position.cumulativeQuotaInterest should be updated.

CDPVault.sol
@@ 350,363 @@    /// @notice Modifies a Position's collateral and debt balances
    /// @dev Checks that the global debt ceiling and the vault's debt ceiling have not been exceeded via the CDM,
    /// - that the Position is still safe after the modification,
    /// - that the msg.sender has the permission of the owner to decrease the collateral-to-debt ratio,
    /// - that the msg.sender has the permission of the collateralizer to put up new collateral,
    /// - that the msg.sender has the permission of the creditor to settle debt with their credit,
    /// - that that the vault debt floor is exceeded
    /// - that the vault minimum collateralization ratio is met
    /// @param owner Address of the owner of the position
    /// @param collateralizer Address of who puts up or receives the collateral delta
    /// @param creditor Address of who provides or receives the credit delta for the debt delta
    /// @param deltaCollateral Amount of collateral to put up (+) or to remove (-) from the position [wad]
    /// @param deltaDebt Amount of normalized debt (gross, before rate is applied) to generate (+) or
    /// to settle (-) on this position [wad]

    function modifyCollateralAndDebt(
        address owner,
        address collateralizer,
        address creditor,
        int256 deltaCollateral,
        int256 deltaDebt
    ) public {

@@ 371,387 @@        if (
            // position is either more safe than before or msg.sender has the permission from the owner
            ((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) ||
            // msg.sender has the permission of the collateralizer to collateralize the position using their cash
            (deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) ||
            // msg.sender has the permission of the creditor to use their credit to repay the debt
            (deltaDebt < 0 && !hasPermission(creditor, msg.sender))
        ) revert CDPVault__modifyCollateralAndDebt_noPermission();

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

        uint256 newDebt;
        uint256 newCumulativeIndex;

        uint256 profit;
        int256 quotaRevenueChange;

        if (deltaDebt > 0) {
            (newDebt, newCumulativeIndex) = CreditLogic.calcIncrease(
                uint256(deltaDebt), // delta debt
                position.debt,
                debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
                position.cumulativeIndexLastUpdate
            ); // U:[CM-10]
            position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
            quotaRevenueChange = _calcQuotaRevenueChange(deltaDebt);
            pool.lendCreditAccount(uint256(deltaDebt), creditor); // F:[CM-20]
        } else if (deltaDebt < 0) {

@@ 399,427 @@            uint256 maxRepayment = calcTotalDebt(debtData);
            uint256 amount = abs(deltaDebt);
            if (amount >= maxRepayment) {
                amount = maxRepayment; // U:[CM-11]
                deltaDebt = -toInt256(maxRepayment);
            }

            poolUnderlying.safeTransferFrom(creditor, address(pool), amount);

            uint128 newCumulativeQuotaInterest;
            if (amount == maxRepayment) {
                newDebt = 0;
                newCumulativeIndex = debtData.cumulativeIndexNow;
                profit = debtData.accruedInterest;
                newCumulativeQuotaInterest = 0;
            } else {
                (newDebt, newCumulativeIndex, profit, newCumulativeQuotaInterest) = calcDecrease(
                    amount, // delta debt
                    position.debt,
                    debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
                    position.cumulativeIndexLastUpdate,
                    debtData.cumulativeQuotaInterest
                );
            }
            quotaRevenueChange = _calcQuotaRevenueChange(-int(debtData.debt - newDebt));
            pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11]

            position.cumulativeQuotaInterest = newCumulativeQuotaInterest;
            position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;

        } else {

@@ 429,430 @@            newDebt = position.debt;
            newCumulativeIndex = debtData.cumulativeIndexLastUpdate;

        }


@@ 433,439 @@        if (deltaCollateral > 0) {
            uint256 amount = wmul(deltaCollateral.toUint256(), tokenScale);
            token.safeTransferFrom(collateralizer, address(this), amount);
        } else if (deltaCollateral < 0) {
            uint256 amount = wmul(abs(deltaCollateral), tokenScale);
            token.safeTransfer(collateralizer, amount);
        }


        position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt);


@@ 443,455 @@        VaultConfig memory config = vaultConfig;
        uint256 spotPrice_ = spotPrice();
        uint256 collateralValue = wmul(position.collateral, spotPrice_);

        if (
            (deltaDebt > 0 || deltaCollateral < 0) &&
            !_isCollateralized(calcTotalDebt(_calcDebt(position)), collateralValue, config.liquidationRatio)
        ) revert CDPVault__modifyCollateralAndDebt_notSafe();

        if (quotaRevenueChange != 0) {
            IPoolV3(pool).updateQuotaRevenue(quotaRevenueChange); // U:[PQK-15]
        }
        emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt);

    }

[WP-M3] Full Liquidation Should Be Allowed After Crossing the Liquidation Threshold but Before Bad Debt Occurs

There's a state between the liquidation threshold and bad debt occurrence. The specific range depends on the collateral ratio and discount rate settings. In the current implementation, once this state is entered, liquidators cannot take all collateral due to CDPVault.sol#L533, and consequently cannot repay all debt. We expect them to be able to repay the entire debt.

For instance:

Given:

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

To repay all the debt, the liquidator would need to take all the collateral (10 ETH) and repay 8000 at the discounted price.

wmul(position.collateral, spotPrice()) is NOT using the discounted price.

Currently, because CDPVault.sol#L533, liquidators can't repay all the debt, and the position is forced into a bad debt state. This bad debt state is undesirable.

CDPVault.sol
@@ 497,504 @@    /// @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 discountedPrice = wmul(spotPrice(), liqConfig_.liquidationDiscount);
        if (spotPrice() == 0) revert CDPVault__liquidatePosition_invalidSpotPrice();

        // 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);

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

        // account for bad debt
        uint256 loss;
        if (takeCollateral > position.collateral) {
            if (calcTotalDebt(debtData) <= wmul(position.collateral, spotPrice())) revert CDPVault__noBadDebt();
            takeCollateral = position.collateral;
            repayAmount = wmul(takeCollateral, discountedPrice);
            penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty);
            deltaDebt = debtData.debt;
            loss = calcTotalDebt(debtData) - (repayAmount - penalty);
        }


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

        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 {
            if (loss != 0) {
                profit = 0;
                newDebt = 0;
                newCumulativeIndex = debtData.cumulativeIndexNow;
            } 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, loss);

        // transfer the collateral amount from the vault to the liquidator
        token.safeTransfer(msg.sender, takeCollateral);

        // Mint the penalty from the vault to the treasury
        IPoolV3Loop(address(pool)).mintProfit(penalty);

    }

Recommendation

Split the current single liquidatePosition() function into two separate functions, one for normal liquidation mode and another for bad debt liquidation mode.

Allow normal liquidation mode and bad debt liquidation mode based on whether the position is in a bad debt state (total liabilities exceed the value of collateral calculated at the liquidation discount price). In other words, when a position is in a bad debt state, prohibit normal liquidation mode and only allow bad debt liquidation mode.

  • Normal liquidation mode
    • No loss
    • Can have profit
    • Can have penalty
  • Bad debt liquidation mode
    • Has loss
    • No profit
    • No penalty
CDPVault.sol
@@ 497,504 @@    /// @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 discountedPrice = wmul(spotPrice(), liqConfig_.liquidationDiscount);
        if (spotPrice() == 0) revert CDPVault__liquidatePosition_invalidSpotPrice();

        // 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);

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

        // account for bad debt
        uint256 loss;
        if (takeCollateral > position.collateral) {
            if (calcTotalDebt(debtData) <= wmul(position.collateral, spotPrice())) revert CDPVault__noBadDebt();
            takeCollateral = position.collateral;
            repayAmount = wmul(takeCollateral, discountedPrice);
            penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty);
            deltaDebt = debtData.debt;
            loss = calcTotalDebt(debtData) - (repayAmount - penalty);
        }


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

        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 {
            if (loss != 0) {
                profit = 0;
                newDebt = 0;
                newCumulativeIndex = debtData.cumulativeIndexNow;
            } 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, loss);

        // transfer the collateral amount from the vault to the liquidator
        token.safeTransfer(msg.sender, takeCollateral);

        // Mint the penalty from the vault to the treasury
        IPoolV3Loop(address(pool)).mintProfit(penalty);

    }

[WP-M4] liquidatePosition() forgot to call IPoolV3(pool).updateQuotaRevenue(quotaRevenueChange);

CDPVault.sol
@@ 497,504 @@    /// @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 discountedPrice = wmul(spotPrice(), liqConfig_.liquidationDiscount);
        if (spotPrice() == 0) revert CDPVault__liquidatePosition_invalidSpotPrice();

        // 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);

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

        // account for bad debt
        uint256 loss;
        if (takeCollateral > position.collateral) {
            if (calcTotalDebt(debtData) <= wmul(position.collateral, spotPrice())) revert CDPVault__noBadDebt();
            takeCollateral = position.collateral;
            repayAmount = wmul(takeCollateral, discountedPrice);
            penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty);
            deltaDebt = debtData.debt;
            loss = calcTotalDebt(debtData) - (repayAmount - penalty);
        }

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

        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 {
            if (loss != 0) {
                profit = 0;
                newDebt = 0;
                newCumulativeIndex = debtData.cumulativeIndexNow;
            } 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, loss);

        // transfer the collateral amount from the vault to the liquidator
        token.safeTransfer(msg.sender, takeCollateral);

        // Mint the penalty from the vault to the treasury
        IPoolV3Loop(address(pool)).mintProfit(penalty);
    }

For comparison, modifyCollateralAndDebt() calculates _calcQuotaRevenueChange() and calls IPoolV3(pool).updateQuotaRevenue(quotaRevenueChange); when the position's debt changes.

https://github.com/LoopFi/loop-contracts/blob/de43bfab07177dd343d41f3e655cdad979ee8a7c/src/CDPVault.sol#L350-L456

@@ 350,363 @@    /// @notice Modifies a Position's collateral and debt balances
    /// @dev Checks that the global debt ceiling and the vault's debt ceiling have not been exceeded via the CDM,
    /// - that the Position is still safe after the modification,
    /// - that the msg.sender has the permission of the owner to decrease the collateral-to-debt ratio,
    /// - that the msg.sender has the permission of the collateralizer to put up new collateral,
    /// - that the msg.sender has the permission of the creditor to settle debt with their credit,
    /// - that that the vault debt floor is exceeded
    /// - that the vault minimum collateralization ratio is met
    /// @param owner Address of the owner of the position
    /// @param collateralizer Address of who puts up or receives the collateral delta
    /// @param creditor Address of who provides or receives the credit delta for the debt delta
    /// @param deltaCollateral Amount of collateral to put up (+) or to remove (-) from the position [wad]
    /// @param deltaDebt Amount of normalized debt (gross, before rate is applied) to generate (+) or
    /// to settle (-) on this position [wad]

    function modifyCollateralAndDebt(
        address owner,
        address collateralizer,
        address creditor,
        int256 deltaCollateral,
        int256 deltaDebt
    ) public {
        if (
            // position is either more safe than before or msg.sender has the permission from the owner
            ((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) ||
            // msg.sender has the permission of the collateralizer to collateralize the position using their cash
            (deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) ||
            // msg.sender has the permission of the creditor to use their credit to repay the debt
            (deltaDebt < 0 && !hasPermission(creditor, msg.sender))
        ) revert CDPVault__modifyCollateralAndDebt_noPermission();

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

        uint256 newDebt;
        uint256 newCumulativeIndex;

        uint256 profit;
        int256 quotaRevenueChange;
        if (deltaDebt > 0) {
            (newDebt, newCumulativeIndex) = CreditLogic.calcIncrease(
                uint256(deltaDebt), // delta debt
                position.debt,
                debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
                position.cumulativeIndexLastUpdate
            ); // U:[CM-10]
            position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
            quotaRevenueChange = _calcQuotaRevenueChange(deltaDebt);
            pool.lendCreditAccount(uint256(deltaDebt), creditor); // F:[CM-20]
        } else if (deltaDebt < 0) {
            uint256 maxRepayment = calcTotalDebt(debtData);
            uint256 amount = abs(deltaDebt);
            if (amount >= maxRepayment) {
                amount = maxRepayment; // U:[CM-11]
                deltaDebt = -toInt256(maxRepayment);
            }

            poolUnderlying.safeTransferFrom(creditor, address(pool), amount);

            uint128 newCumulativeQuotaInterest;
            if (amount == maxRepayment) {
                newDebt = 0;
                newCumulativeIndex = debtData.cumulativeIndexNow;
                profit = debtData.accruedInterest;
                newCumulativeQuotaInterest = 0;
            } else {
                (newDebt, newCumulativeIndex, profit, newCumulativeQuotaInterest) = calcDecrease(
                    amount, // delta debt
                    position.debt,
                    debtData.cumulativeIndexNow, // current cumulative base interest index in Ray
                    position.cumulativeIndexLastUpdate,
                    debtData.cumulativeQuotaInterest
                );
            }
            quotaRevenueChange = _calcQuotaRevenueChange(-int(debtData.debt - newDebt));
            pool.repayCreditAccount(debtData.debt - newDebt, profit, 0); // U:[CM-11]

            position.cumulativeQuotaInterest = newCumulativeQuotaInterest;
            position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow;
        } else {
            newDebt = position.debt;
            newCumulativeIndex = debtData.cumulativeIndexLastUpdate;
        }

        if (deltaCollateral > 0) {
            uint256 amount = wmul(deltaCollateral.toUint256(), tokenScale);
            token.safeTransferFrom(collateralizer, address(this), amount);
        } else if (deltaCollateral < 0) {
            uint256 amount = wmul(abs(deltaCollateral), tokenScale);
            token.safeTransfer(collateralizer, amount);
        }

        position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt);

        VaultConfig memory config = vaultConfig;
        uint256 spotPrice_ = spotPrice();
        uint256 collateralValue = wmul(position.collateral, spotPrice_);

        if (
            (deltaDebt > 0 || deltaCollateral < 0) &&
            !_isCollateralized(calcTotalDebt(_calcDebt(position)), collateralValue, config.liquidationRatio)
        ) revert CDPVault__modifyCollateralAndDebt_notSafe();

        if (quotaRevenueChange != 0) {
            IPoolV3(pool).updateQuotaRevenue(quotaRevenueChange); // U:[PQK-15]
        }
        emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt);
    }
CDPVault.sol
@@ 497,504 @@    /// @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 discountedPrice = wmul(spotPrice(), liqConfig_.liquidationDiscount);
        if (spotPrice() == 0) revert CDPVault__liquidatePosition_invalidSpotPrice();

        // 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);

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

        // account for bad debt
        uint256 loss;
        if (takeCollateral > position.collateral) {
            if (calcTotalDebt(debtData) <= wmul(position.collateral, spotPrice())) revert CDPVault__noBadDebt();
            takeCollateral = position.collateral;
            repayAmount = wmul(takeCollateral, discountedPrice);
            penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty);
            deltaDebt = debtData.debt;
            loss = calcTotalDebt(debtData) - (repayAmount - penalty);
        }

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

        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 {
            if (loss != 0) {
                profit = 0;
                newDebt = 0;
                newCumulativeIndex = debtData.cumulativeIndexNow;
            } 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, loss);

        // transfer the collateral amount from the vault to the liquidator
        token.safeTransfer(msg.sender, takeCollateral);

        // Mint the penalty from the vault to the treasury
        IPoolV3Loop(address(pool)).mintProfit(penalty);
    }

[WP-I5] Consider using SafeERC20Upgradeable.forceApprove()

forceApprove() is recommended for supporting tokens that require the approval to be set to zero before setting it to a non-zero value, such as USDT.

This will be necessary when deploying to Mainnet and adding USDT (or other similar tokens) as a supported token.

PositionAction.sol
@@ 377,378 @@    /// @notice Callback function for the flash loan taken out in increaseLever
    /// @param data The encoded bytes that were passed into the flash loan

    function onFlashLoan(
        address initiator,
        address /*token*/,
        uint256 amount,
        uint256 fee,
        bytes calldata data
    ) external returns (bytes32) {

@@ 386,423 @@        if (msg.sender != address(flashlender)) revert PositionAction__onFlashLoan__invalidSender();
        if (initiator != address(this)) revert PositionAction__onFlashLoan__invalidInitiator();

        (LeverParams memory leverParams, address upFrontToken, uint256 upFrontAmount) = abi.decode(
            data,
            (LeverParams, address, uint256)
        );

        // perform a pre swap from arbitrary token to collateral token if necessary
        if (leverParams.auxSwap.assetIn != address(0)) {
            bytes memory auxSwapData = _delegateCall(
                address(swapAction),
                abi.encodeWithSelector(swapAction.swap.selector, leverParams.auxSwap)
            );
            upFrontAmount = abi.decode(auxSwapData, (uint256));
        }

        // handle the flash loan swap
        bytes memory swapData = _delegateCall(
            address(swapAction),
            abi.encodeWithSelector(swapAction.swap.selector, leverParams.primarySwap)
        );
        uint256 swapAmountOut = abi.decode(swapData, (uint256));

        // deposit collateral and handle any CDP specific actions
        uint256 collateral = _onIncreaseLever(leverParams, upFrontToken, upFrontAmount, swapAmountOut);

        // derive the amount of normal debt from the swap amount out
        uint256 addDebt = amount + fee;

        // add collateral and debt
        ICDPVault(leverParams.vault).modifyCollateralAndDebt(
            leverParams.position,
            address(this),
            address(this),
            toInt256(collateral),
            toInt256(addDebt)
        );


        underlyingToken.approve(address(flashlender), addDebt);

        return CALLBACK_SUCCESS;
    }
OpenZeppelin SafeERC20.sol
    /**
     * @dev Set the calling contract's allowance toward `spender` to `value`. If `token` returns no value,
     * non-reverting calls are assumed to be successful. Meant to be used with tokens that require the approval
     * to be set to zero before setting it to a non-zero value, such as USDT.
     */
    function forceApprove(IERC20 token, address spender, uint256 value) internal {
        bytes memory approvalCall = abi.encodeWithSelector(token.approve.selector, spender, value);

        if (!_callOptionalReturnBool(token, approvalCall)) {
            _callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, 0));
            _callOptionalReturn(token, approvalCall);
        }
    }
PositionAction.sol
@@ 377,378 @@    /// @notice Callback function for the flash loan taken out in increaseLever
    /// @param data The encoded bytes that were passed into the flash loan

    function onFlashLoan(
        address initiator,
        address /*token*/,
        uint256 amount,
        uint256 fee,
        bytes calldata data
    ) external returns (bytes32) {

@@ 386,423 @@        if (msg.sender != address(flashlender)) revert PositionAction__onFlashLoan__invalidSender();
        if (initiator != address(this)) revert PositionAction__onFlashLoan__invalidInitiator();

        (LeverParams memory leverParams, address upFrontToken, uint256 upFrontAmount) = abi.decode(
            data,
            (LeverParams, address, uint256)
        );

        // perform a pre swap from arbitrary token to collateral token if necessary
        if (leverParams.auxSwap.assetIn != address(0)) {
            bytes memory auxSwapData = _delegateCall(
                address(swapAction),
                abi.encodeWithSelector(swapAction.swap.selector, leverParams.auxSwap)
            );
            upFrontAmount = abi.decode(auxSwapData, (uint256));
        }

        // handle the flash loan swap
        bytes memory swapData = _delegateCall(
            address(swapAction),
            abi.encodeWithSelector(swapAction.swap.selector, leverParams.primarySwap)
        );
        uint256 swapAmountOut = abi.decode(swapData, (uint256));

        // deposit collateral and handle any CDP specific actions
        uint256 collateral = _onIncreaseLever(leverParams, upFrontToken, upFrontAmount, swapAmountOut);

        // derive the amount of normal debt from the swap amount out
        uint256 addDebt = amount + fee;

        // add collateral and debt
        ICDPVault(leverParams.vault).modifyCollateralAndDebt(
            leverParams.position,
            address(this),
            address(this),
            toInt256(collateral),
            toInt256(addDebt)
        );


        underlyingToken.approve(address(flashlender), addDebt);

        return CALLBACK_SUCCESS;
    }
OpenZeppelin SafeERC20.sol
    /**
     * @dev Set the calling contract's allowance toward `spender` to `value`. If `token` returns no value,
     * non-reverting calls are assumed to be successful. Meant to be used with tokens that require the approval
     * to be set to zero before setting it to a non-zero value, such as USDT.
     */
    function forceApprove(IERC20 token, address spender, uint256 value) internal {
        bytes memory approvalCall = abi.encodeWithSelector(token.approve.selector, spender, value);

        if (!_callOptionalReturnBool(token, approvalCall)) {
            _callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, 0));
            _callOptionalReturn(token, approvalCall);
        }
    }

[WP-L6] Before executing pool business logic, only the corresponding poolUnderlying tokens for the current function call should be transferred to the pool.

This is to ensures that the pool uses the correct underlyingToken.balanceOf(PoolV3.this).

For example, in CDPVault.liquidatePosition():

  • Expected behavior:
    1. Before pool.repayCreditAccount(debtData.debt - newDebt, profit, loss); at CDPVault.sol#L575, only transfer the deltaDebt amount of poolUnderlying to the pool for repayCreditAccount().
    2. Before IPoolV3Loop(address(pool)).mintProfit(penalty); at CDPVault.sol#L581, transfer the penalty amount of poolUnderlying to the pool for mintProfit().
  • Current implementation:
    • At CDPVault.sol#L542, before repayCreditAccount(), poolUnderlying.safeTransferFrom(msg.sender, address(pool), repayAmount); transfers both deltaDebt and penalty to the pool. This causes pool.availableLiquidity() to include assets for both repayCreditAccount() and the subsequent mintProfit() when executing pool.repayCreditAccount(debtData.debt - newDebt, profit, loss); at CDPVault.sol#L575.
CDPVault.sol
@@ 497,504 @@    /// @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 {

@@ 506,519 @@        // 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 discountedPrice = wmul(spotPrice(), liqConfig_.liquidationDiscount);
        if (spotPrice() == 0) revert CDPVault__liquidatePosition_invalidSpotPrice();


        // 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);

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

        // account for bad debt
        uint256 loss;
        if (takeCollateral > position.collateral) {
            if (calcTotalDebt(debtData) <= wmul(position.collateral, spotPrice())) revert CDPVault__noBadDebt();
            takeCollateral = position.collateral;
            repayAmount = wmul(takeCollateral, discountedPrice);
            penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty);
            deltaDebt = debtData.debt;
            loss = calcTotalDebt(debtData) - (repayAmount - penalty);
        }

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


@@ 544,569 @@        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 {
            if (loss != 0) {
                profit = 0;
                newDebt = 0;
                newCumulativeIndex = debtData.cumulativeIndexNow;
            } 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, loss);

        // transfer the collateral amount from the vault to the liquidator
        token.safeTransfer(msg.sender, takeCollateral);

        // Mint the penalty from the vault to the treasury
        IPoolV3Loop(address(pool)).mintProfit(penalty);
    }

PoolV3.sol

@@ 517,529 @@    /// @notice Updates pool state to indicate debt repayment, can only be called by credit managers
    ///         after transferring underlying from a credit account to the pool.
    ///         - If transferred amount exceeds debt principal + base interest + quota interest,
    ///           the difference is deemed protocol's profit and the respective number of shares
    ///           is minted to the treasury.
    ///         - If, however, transferred amount is insufficient to repay debt and interest,
    ///           which may only happen during liquidation, treasury's shares are burned to
    ///           cover as much of the loss as possible.
    /// @param repaidAmount Amount of debt principal repaid
    /// @param profit Pool's profit in underlying after repaying
    /// @param loss Pool's loss in underlying after repaying
    /// @custom:expects Credit manager transfers underlying from a credit account to the pool before calling this function
    /// @custom:expects Profit/loss computed in the credit manager are cosistent with pool's implicit calculations

    function repayCreditAccount(
        uint256 repaidAmount,
        uint256 profit,
        uint256 loss
    )
        external
        override
        creditManagerOnly // U:[LP-2C]
        whenNotPaused // U:[LP-2A]
        nonReentrant // U:[LP-2B]
    {
        uint128 repaidAmountU128 = repaidAmount.toUint128();

        DebtParams storage cmDebt = _creditManagerDebt[msg.sender];
        uint128 cmBorrowed = cmDebt.borrowed;
        if (cmBorrowed == 0) {
            revert CallerNotCreditManagerException(); // U:[LP-2C,14A]
        }

        if (profit > 0) {
            _mint(treasury, convertToShares(profit)); // U:[LP-14B]
        } else if (loss > 0) {
            address treasury_ = treasury;
            uint256 sharesInTreasury = balanceOf(treasury_);
            uint256 sharesToBurn = convertToShares(loss);
            if (sharesToBurn > sharesInTreasury) {
                unchecked {
                    emit IncurUncoveredLoss({
                        creditManager: msg.sender,
                        loss: convertToAssets(sharesToBurn - sharesInTreasury)
                    }); // U:[LP-14D]
                }
                sharesToBurn = sharesInTreasury;
            }
            _burn(treasury_, sharesToBurn); // U:[LP-14C,14D]
        }

        _updateBaseInterest({
            expectedLiquidityDelta: profit.toInt256() - loss.toInt256(),
            availableLiquidityDelta: 0,
            checkOptimalBorrowing: false
        }); // U:[LP-14B,14C,14D]

        _totalDebt.borrowed -= repaidAmountU128; // U:[LP-14B,14C,14D]
        cmDebt.borrowed = cmBorrowed - repaidAmountU128; // U:[LP-14B,14C,14D]

        emit Repay(msg.sender, repaidAmount, profit, loss); // U:[LP-14B,14C,14D]
    }
PoolV3.sol
@@ 637,642 @@    /// @dev Updates base interest rate based on expected and available liquidity deltas
    ///      - Adds expected liquidity delta to stored expected liquidity
    ///      - If time has passed since the last base interest update, adds accrued interest
    ///        to stored expected liquidity, updates interest index and last update timestamp
    ///      - If time has passed since the last quota revenue update, adds accrued revenue
    ///        to stored expected liquidity and updates last update timestamp

    function _updateBaseInterest(
        int256 expectedLiquidityDelta,
        int256 availableLiquidityDelta,
        bool checkOptimalBorrowing
    ) internal {
        uint256 expectedLiquidity_ = (expectedLiquidity().toInt256() + expectedLiquidityDelta).toUint256();
        uint256 availableLiquidity_ = (availableLiquidity().toInt256() + availableLiquidityDelta).toUint256();

        uint256 lastBaseInterestUpdate_ = lastBaseInterestUpdate;
        if (block.timestamp != lastBaseInterestUpdate_) {
            _baseInterestIndexLU = _calcBaseInterestIndex(lastBaseInterestUpdate_).toUint128(); // U:[LP-18]
            lastBaseInterestUpdate = uint40(block.timestamp);
        }

        if (block.timestamp != lastQuotaRevenueUpdate) {
            lastQuotaRevenueUpdate = uint40(block.timestamp); // U:[LP-18]
        }

        _expectedLiquidityLU = expectedLiquidity_.toUint128(); // U:[LP-18]
        _baseInterestRate = ILinearInterestRateModelV3(interestRateModel)
            .calcBorrowRate({
                expectedLiquidity: expectedLiquidity_,
                availableLiquidity: availableLiquidity_,
                checkOptimalBorrowing: checkOptimalBorrowing
            })
            .toUint128(); // U:[LP-18]
    }
PoolV3.sol
    /// @notice Available liquidity in the pool
    function availableLiquidity() public view override returns (uint256) {
        return IERC20(underlyingToken).balanceOf(address(this)); // U:[LP-3]
    }
CDPVault.sol
@@ 497,504 @@    /// @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 {

@@ 506,519 @@        // 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 discountedPrice = wmul(spotPrice(), liqConfig_.liquidationDiscount);
        if (spotPrice() == 0) revert CDPVault__liquidatePosition_invalidSpotPrice();


        // 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);

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

        // account for bad debt
        uint256 loss;
        if (takeCollateral > position.collateral) {
            if (calcTotalDebt(debtData) <= wmul(position.collateral, spotPrice())) revert CDPVault__noBadDebt();
            takeCollateral = position.collateral;
            repayAmount = wmul(takeCollateral, discountedPrice);
            penalty = wmul(repayAmount, WAD - liqConfig_.liquidationPenalty);
            deltaDebt = debtData.debt;
            loss = calcTotalDebt(debtData) - (repayAmount - penalty);
        }

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


@@ 544,569 @@        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 {
            if (loss != 0) {
                profit = 0;
                newDebt = 0;
                newCumulativeIndex = debtData.cumulativeIndexNow;
            } 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, loss);

        // transfer the collateral amount from the vault to the liquidator
        token.safeTransfer(msg.sender, takeCollateral);

        // Mint the penalty from the vault to the treasury
        IPoolV3Loop(address(pool)).mintProfit(penalty);
    }
PoolV3.sol
@@ 637,642 @@    /// @dev Updates base interest rate based on expected and available liquidity deltas
    ///      - Adds expected liquidity delta to stored expected liquidity
    ///      - If time has passed since the last base interest update, adds accrued interest
    ///        to stored expected liquidity, updates interest index and last update timestamp
    ///      - If time has passed since the last quota revenue update, adds accrued revenue
    ///        to stored expected liquidity and updates last update timestamp

    function _updateBaseInterest(
        int256 expectedLiquidityDelta,
        int256 availableLiquidityDelta,
        bool checkOptimalBorrowing
    ) internal {
        uint256 expectedLiquidity_ = (expectedLiquidity().toInt256() + expectedLiquidityDelta).toUint256();
        uint256 availableLiquidity_ = (availableLiquidity().toInt256() + availableLiquidityDelta).toUint256();

        uint256 lastBaseInterestUpdate_ = lastBaseInterestUpdate;
        if (block.timestamp != lastBaseInterestUpdate_) {
            _baseInterestIndexLU = _calcBaseInterestIndex(lastBaseInterestUpdate_).toUint128(); // U:[LP-18]
            lastBaseInterestUpdate = uint40(block.timestamp);
        }

        if (block.timestamp != lastQuotaRevenueUpdate) {
            lastQuotaRevenueUpdate = uint40(block.timestamp); // U:[LP-18]
        }

        _expectedLiquidityLU = expectedLiquidity_.toUint128(); // U:[LP-18]
        _baseInterestRate = ILinearInterestRateModelV3(interestRateModel)
            .calcBorrowRate({
                expectedLiquidity: expectedLiquidity_,
                availableLiquidity: availableLiquidity_,
                checkOptimalBorrowing: checkOptimalBorrowing
            })
            .toUint128(); // U:[LP-18]
    }
PoolV3.sol
    /// @notice Available liquidity in the pool
    function availableLiquidity() public view override returns (uint256) {
        return IERC20(underlyingToken).balanceOf(address(this)); // U:[LP-3]
    }

[WP-I7] Unexpected _creditManagerSet.add(msg.sender);

msg.sender (the deployer) has the permission to mint tokens for the treasury (See PoolV3.sol#L900).

  1. First, msg.sender should not be an EOA.

    An EOA should not have such arbitrary permissions.

  2. If msg.sender is the deployment contract, this contract is not found in the project.

    Considering that creditManager permissions can call methods like lendCreditAccount and repayCreditAccount, granting creditManager permissions to the deployment contract seems very strange.

https://github.com/LoopFi/loop-contracts/blob/de43bfab07177dd343d41f3e655cdad979ee8a7c/src/PoolV3.sol#L152-L190

    constructor(
        address addressProvider_,
        address underlyingToken_,
        address interestRateModel_,
        uint256 totalDebtLimit_,
        string memory name_,
        string memory symbol_
    )
        ACLNonReentrantTrait(addressProvider_) // U:[LP-1A]
        ContractsRegisterTrait(addressProvider_)
        ERC4626(IERC20(underlyingToken_)) // U:[LP-1B]
        ERC20(name_, symbol_) // U:[LP-1B]
        ERC20Permit(name_) // U:[LP-1B]
        nonZeroAddress(underlyingToken_) // U:[LP-1A]
        nonZeroAddress(interestRateModel_) // U:[LP-1A]
    {
        addressProvider = addressProvider_; // U:[LP-1B]
        underlyingToken = underlyingToken_; // U:[LP-1B]

        treasury = IAddressProviderV3(addressProvider_).getAddressOrRevert({
            key: AP_TREASURY,
            _version: NO_VERSION_CONTROL
        }); // U:[LP-1B]

        lastBaseInterestUpdate = uint40(block.timestamp); // U:[LP-1B]
        _baseInterestIndexLU = uint128(RAY); // U:[LP-1B]

        interestRateModel = interestRateModel_; // U:[LP-1B]
        emit SetInterestRateModel(interestRateModel_); // U:[LP-1B]

        if (ERC20(underlyingToken_).decimals() != 18) {
            revert IncompatibleDecimalsException();
        }

        locked = true;

        _setTotalDebtLimit(totalDebtLimit_); // U:[LP-1B]
        _creditManagerSet.add(msg.sender);
    }
PoolV3.sol
    function mintProfit(uint256 amount) external creditManagerOnly {
        _mint(treasury, amount);

        _updateBaseInterest({
            expectedLiquidityDelta: amount.toInt256(),
            availableLiquidityDelta: 0,
            checkOptimalBorrowing: false
        }); // U:[LP-14B,14C,14D]
    }

Comparison with Gearbox's Vault constructor

It does not add creditManager permissions to msg.sender.

https://github.com/Gearbox-protocol/core-v3/blob/main/contracts/pool/PoolV3.sol#L120-L149

constructor(
    address addressProvider_,
    address underlyingToken_,
    address interestRateModel_,
    uint256 totalDebtLimit_,
    string memory name_,
    string memory symbol_
)
    ACLNonReentrantTrait(addressProvider_) // U:[LP-1A]
    ContractsRegisterTrait(addressProvider_)
    ERC4626(IERC20(underlyingToken_)) // U:[LP-1B]
    ERC20(name_, symbol_) // U:[LP-1B]
    ERC20Permit(name_) // U:[LP-1B]
    nonZeroAddress(underlyingToken_) // U:[LP-1A]
    nonZeroAddress(interestRateModel_) // U:[LP-1A]
{
    addressProvider = addressProvider_; // U:[LP-1B]
    underlyingToken = underlyingToken_; // U:[LP-1B]

    treasury =
        IAddressProviderV3(addressProvider_).getAddressOrRevert({key: AP_TREASURY, _version: NO_VERSION_CONTROL}); // U:[LP-1B]

    lastBaseInterestUpdate = uint40(block.timestamp); // U:[LP-1B]
    _baseInterestIndexLU = uint128(RAY); // U:[LP-1B]

    interestRateModel = interestRateModel_; // U:[LP-1B]
    emit SetInterestRateModel(interestRateModel_); // U:[LP-1B]

    _setTotalDebtLimit(totalDebtLimit_); // U:[LP-1B]
}
PoolV3.sol
    function mintProfit(uint256 amount) external creditManagerOnly {
        _mint(treasury, amount);

        _updateBaseInterest({
            expectedLiquidityDelta: amount.toInt256(),
            availableLiquidityDelta: 0,
            checkOptimalBorrowing: false
        }); // U:[LP-14B,14C,14D]
    }

[WP-Q8] Why does each iteration of the tokens loop use IPoolV3(pool).totalBorrowed()?

Is there a one-to-one relationship between PoolV3, CDPVault, and PoolQuotaKeeperV3? (i.e., PoolQuotaKeeperV3.quotaTokensSet has only one element CDPVault.token, and this CDPVault is the only entity that can call pool.lendCreditAccount())?

Only if it is so, can the current PoolQuotaKeeperV3 implementation avoid redundant quotaRevenue calculations.

PoolQuotaKeeperV3.sol
    /// @notice Returns the pool's quota revenue (in units of underlying per year)
    function poolQuotaRevenue() external view virtual override returns (uint256 quotaRevenue) {
        address[] memory tokens = quotaTokensSet.values();

        uint256 len = tokens.length;

        for (uint256 i; i < len; ) {
            address token = tokens[i];

            TokenQuotaParams storage tokenQuotaParams = totalQuotaParams[token];
            (uint16 rate, , ) = _getTokenQuotaParamsOrRevert(tokenQuotaParams);
            //(uint256 totalQuoted, ) = _getTokenQuotaTotalAndLimit(tokenQuotaParams);

            quotaRevenue += (IPoolV3(pool).totalBorrowed() * rate) / PERCENTAGE_FACTOR;

            unchecked {
                ++i;
            }
        }
    }

PoolQuotaKeeperV3.sol

    /// @notice Updates quota rates
    ///         - Updates global token cumulative indexes before changing rates
    ///         - Queries new rates for all quoted tokens from the gauge
    ///         - Sets new pool quota revenue
    function updateRates()
        external
        override
        gaugeOnly // U:[PQK-3]
    {
        address[] memory tokens = quotaTokensSet.values();
        uint16[] memory rates = IGaugeV3(gauge).getRates(tokens); // U:[PQK-7]

        uint256 quotaRevenue; // U:[PQK-7]
        uint256 timestampLU = lastQuotaRateUpdate;
        uint256 len = tokens.length;

        for (uint256 i; i < len; ) {
            address token = tokens[i];
            uint16 rate = rates[i];

            TokenQuotaParams storage tokenQuotaParams = totalQuotaParams[token]; // U:[PQK-7]
            (uint16 prevRate, uint192 tqCumulativeIndexLU, ) = _getTokenQuotaParamsOrRevert(tokenQuotaParams);

            tokenQuotaParams.cumulativeIndexLU = QuotasLogic.cumulativeIndexSince(
                tqCumulativeIndexLU,
                prevRate,
                timestampLU
            ); // U:[PQK-7]

            tokenQuotaParams.rate = rate; // U:[PQK-7]

            quotaRevenue += (IPoolV3(pool).totalBorrowed() * rate) / PERCENTAGE_FACTOR; // U:[PQK-7]

            emit UpdateTokenQuotaRate(token, rate); // U:[PQK-7]

            unchecked {
                ++i;
            }
        }

        IPoolV3(pool).setQuotaRevenue(quotaRevenue); // U:[PQK-7]
        lastQuotaRateUpdate = uint40(block.timestamp); // U:[PQK-7]
    }
PoolQuotaKeeperV3.sol
    /// @notice Returns the pool's quota revenue (in units of underlying per year)
    function poolQuotaRevenue() external view virtual override returns (uint256 quotaRevenue) {
        address[] memory tokens = quotaTokensSet.values();

        uint256 len = tokens.length;

        for (uint256 i; i < len; ) {
            address token = tokens[i];

            TokenQuotaParams storage tokenQuotaParams = totalQuotaParams[token];
            (uint16 rate, , ) = _getTokenQuotaParamsOrRevert(tokenQuotaParams);
            //(uint256 totalQuoted, ) = _getTokenQuotaTotalAndLimit(tokenQuotaParams);

            quotaRevenue += (IPoolV3(pool).totalBorrowed() * rate) / PERCENTAGE_FACTOR;

            unchecked {
                ++i;
            }
        }
    }

[WP-Q9] What's the designed use case for modifyPermission?

Permission can be granted to another address, which can further be used to set another address as owner, collateralizer or creditor.

What's the designed use case for this? Who will be granted such permission?

CDPVault.sol
if (
    // position is either more safe than before or msg.sender has the permission from the owner
    ((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) ||
    // msg.sender has the permission of the collateralizer to collateralize the position using their cash
    (deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) ||
    // msg.sender has the permission of the creditor to use their credit to repay the debt
    (deltaDebt < 0 && !hasPermission(creditor, msg.sender))
) revert CDPVault__modifyCollateralAndDebt_noPermission();
Permission.sol
/// @notice Checks if `caller` has the permission to perform an action on behalf of `owner`
/// @param owner Address of the owner
/// @param caller Address of the caller
/// @return _ whether `caller` has the permission
function hasPermission(address owner, address caller) public view returns (bool) {
    return owner == caller || _permitted[owner][caller];
}
Permission.sol
function modifyPermission(address caller, bool permitted) external {
    _permitted[msg.sender][caller] = permitted;
    emit ModifyPermission(msg.sender, msg.sender, caller, permitted);
}
CDPVault.sol
if (
    // position is either more safe than before or msg.sender has the permission from the owner
    ((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) ||
    // msg.sender has the permission of the collateralizer to collateralize the position using their cash
    (deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) ||
    // msg.sender has the permission of the creditor to use their credit to repay the debt
    (deltaDebt < 0 && !hasPermission(creditor, msg.sender))
) revert CDPVault__modifyCollateralAndDebt_noPermission();
Permission.sol
/// @notice Checks if `caller` has the permission to perform an action on behalf of `owner`
/// @param owner Address of the owner
/// @param caller Address of the caller
/// @return _ whether `caller` has the permission
function hasPermission(address owner, address caller) public view returns (bool) {
    return owner == caller || _permitted[owner][caller];
}
Permission.sol
function modifyPermission(address caller, bool permitted) external {
    _permitted[msg.sender][caller] = permitted;
    emit ModifyPermission(msg.sender, msg.sender, caller, permitted);
}