Liquidate function will fail in case of multiple collateral

[NandanNNN]

Let's say bad user took 49$ debt and as collateral deposited 50$ weth and 50$ wbtc. then weth collateral value drops to 40$, now bad user have 49$ debt and 90$ collateral (40$ weth and 50$ wbtc). So if we want to liquidate bad user and want to give 10% bonus to liquidator then our liquidate function will revert. Because when we pass address of weth/wbtc to our liquidate function bad user didn't have enough collateral in single collateral that will cover debt as well as bonus for good user.

[NandanNNN]

According to me this should be the code for liquidate function

function liquidate(address user, uint256 debtToCover) external moreThanZero(debtToCover) nonReentrant {
        uint256 remainingDebt = debtToCover;

        uint256 startingUserHealthFactor = _healthFactor(user);
        if (startingUserHealthFactor >= MIN_HEALTH_FACTOR) {
            revert DSCEngine__HealthFactorOk();
        }

        for (uint256 i = 0; i < s_collateralTokens.length; i++) {
            address tokenAddress = s_collateralTokens[i];
            uint256 amountCollateral = s_collateralDeposited[msg.sender][tokenAddress];
            if (amountCollateral > 0 && remainingDebt > 0) {
                //we have got the amount of tokenn we require to clear off debt
                uint256 tokenAmountFromDebtCovered = getTokenAmountFromUsd(s_collateralTokens[i], remainingDebt);
                uint256 bonusCollateral = (tokenAmountFromDebtCovered * LIQUIDATION_BONUS) / LIQUIDATION_PRECISION;
                //Checking if bad user pay of it's debt and bonnus from current collateral
                if (amountCollateral >= (tokenAmountFromDebtCovered + bonusCollateral)) {
                    _redeemCollateral(tokenAddress, tokenAmountFromDebtCovered + bonusCollateral, user, msg.sender);
                    break;
                } else {
                    // take out as much as collateral bad user have and then move to next collateral
                    _redeemCollateral(tokenAddress, amountCollateral, user, msg.sender);
                    //calculate remaining amount of collateral to be taken from next collateral
                    remainingDebt = remainingDebt - _getUsdValue(tokenAddress, amountCollateral);
                }
            }
        }

        //remaining debt is not zero that means whole debt of user is not being covered because of less collateral
        if (remainingDebt > 0) {
            revert DSCEngine__CollateralValueTankedBelowAmountOfDscMinted();
        }
        // And give them a 10% bonus
        // So we are giving the liquidator $110 of WETH for 100 DSC
        // We should implement a feature to liquidate in the event the protocol is insolvent
        // And sweep extra amounts into a treasury
        //uint256 bonusCollateral = (tokenAmountFromDebtCovered * LIQUIDATION_BONUS) / LIQUIDATION_PRECISION;
        // Burn DSC equal to debtToCover
        // Figure out how much collateral to recover based on how much burnt
        //_redeemCollateral(collateral, tokenAmountFromDebtCovered + bonusCollateral, user, msg.sender);
        _burnDsc(debtToCover, user, msg.sender);

        uint256 endingUserHealthFactor = _healthFactor(user);
        // This conditional should never hit, but just in case
        if (endingUserHealthFactor <= startingUserHealthFactor) {
            revert DSCEngine__HealthFactorNotImproved();
        }
        revertIfHealthFactorIsBroken(msg.sender);
    }

[PatrickAlphaC]

Yes! This is a known issue in the protocol :)

You can find more security issues here: https://github.com/Cyfrin/foundry-defi-stablecoin-f23/blob/main/audits/codehawks-08-05-2023.md

[PatrickAlphaC]

(This is just a dummy codebase for educational purposes)

It's really good you're seeing this, it means you're thinking about this correctly!