Query in Decentralized Stable Coin - Liquidate Function

[shikhar229169]

function liquidate(address collateral, address user, uint256 debtToCover) external moreThanZero(debtToCover) isAllowedToken(collateral) nonReentrant {
        uint256 startHealthFactor = _healthFactor(user);
        if (startHealthFactor >= MIN_HEALTH_FACTOR) {
            revert DSCEngine__healthFactorIsNotBroken();
        }

        uint256 collateralAmountFromDebtCovered = getTokenAmountFromUSD(collateral, debtToCover);
        uint256 bonusCollateral = (collateralAmountFromDebtCovered * LIQUIDATION_BONUS) / LIQUIDATION_PRECISION;
        uint256 netCollateralAmount = collateralAmountFromDebtCovered + bonusCollateral;

        _redeemCollateral(collateral, netCollateralAmount, user, msg.sender);
        _burnDSC(user, msg.sender, debtToCover);

        uint256 endingUserHealthFactor = _healthFactor(user);

        if (endingUserHealthFactor <= startHealthFactor) {
            revert DSCEngine__healthFactorNotImproved();
        }

        _revertIfHealthFactorIsBroken(msg.sender);
    }

In the liquidate function, the liquidator's health factor is being checked at the end, and if its below minimum health factor, we will revert this.
But I think that in the liquidation process, there is no effect on health factor of the liquidator, then why we need to check it?

Also, in case if the liquidator's health factor was broken before calling the liquidate function, then they can't liquidate anyone.
So, if it is a security measure to only allow those person to liquidate whose health factor is good, then we should consider checking it initially.

Feel free to post any suggestions for this, thanks.

[ChefAharoni]

I haven't gotten into that part of the course, but isn't the following line changing the HelathFactor?

uint256 endingUserHealthFactor = _healthFactor(user);

At the start you're comparing startHealthFactor, and at the end with endingUserHealthFactor, which seems to change during the code.

[shikhar229169]

That's actually used to calculate the health factor of the user and for sure it is gonna change. But over here I am talking about the liquidator(msg.sender) not the user.

[ChefAharoni]

Are you referring to this line?
_revertIfHealthFactorIsBroken(msg.sender);

Because in the check I can see that it is looking at the user's health factor.

    function revertIfHealthFactorIsBroken(address user) internal view {
        uint256 userHealthFactor = _healthFactor(user);
        if (userHealthFactor < MIN_HEALTH_FACTOR) {
            revert DSCEngine__BreaksHealthFactor(userHealthFactor);
        }
    }

[shikhar229169]

So, here we have 2 people one is liquidator, and the other person is the one which gets liquidated (if their health factor is broken). So, the one who calls the liquidate function is the liquidator in order to liquidate the user which is passed as 2nd argument in liquidate function.

I am referring to the last line of the liquidate function:

_revertIfHealthFactorIsBroken(msg.sender);

[ChefAharoni]

Right, and when we go inside this revert check, we can see it checks for _healthFactor(user);, where user == msg.sender, as stated in the function head:
function revertIfHealthFactorIsBroken(address user) internal view {

Inside the liquidate function, we are modifying the state of the user's health factor, in this line:
uint256 endingUserHealthFactor = _healthFactor(user);

Once his _healthFactor was changed - you're prompting the _revertIfHealthFactorIsBroken(msg.sender);, where msg.sender is user.

[pacelliv]

@shikhar229169 I agree with you, is better to move up the check to the top to follow CEI and don't allow a user with a broken health factor to enter the function and perform liquidations even though the tx will revert.

[pratikmhatre]

We are not modifying the health factor at this line,

uint256 endingUserHealthFactor = _healthFactor(user);

we are just querying final health factor.

However the code seems to be missing a line to reduce the liquidators DSC balance too as we are burning his DSC without updating the s_DSCMinted mapping. The health factor would be changed after we burn liquidator's DSC but this line has to be there

[shikhar229169]

Ik we are only querying the health factor.

No, we don't have to reduce the liquidators DSC balance entry in dsc engine mapping.
Think of DSC as a loan.

The liquidator used their loan to pay the loans of person whose health factor is broken, at this point we have to take the amount of DSC that liquidator wish to pay from them and reduce the dsc minted entry in engine for that person whose health factor was broken (because we are covering their loans), and as liquidator still has a loan on them so that's why we have not reduce their amount in the dsc engine entry.

[pratikmhatre]

Think of DSC as a loan. this line cleared my doubt, despite of hearing it multiple times while watching the video it took me this answer to truly understand the line. I spent a whole evening thinking about this.
Thanks for this.

[theirrationalone]

Threshold: Company: Anybody who has any loan/credits and if whose balance/collateral ever goes below $100 then in case he/she is an undercollateralized (bad debt)..... ~ here for the sake of simplicity we are not calculating exact threshold(minimum deed against any breach).

And for paying someone's loan/debts/outstandings company will give pay 10% of those debts/loan as a bonus/consideration to you.

Now,

You as a liquidator... 👍 ✅

Your balance = $170 (collateral) . ✅
Your credits = $0 (loan/outstandings). ✅ ⚠

After some time... ⌚

(Keep it in mind) 😏 you have...
Your balance = $120 (collateral) . ✅
Your credits = $50 (loan/outstandings). ✅ ⚠

Me, me an undercollateralized(bad debt)....👎 ❌

My balance = $110 (collateral) . ✅
My credits = $50 (loan/outstandings). ✅ ⚠

After some time... ⌚
(This matters)...
My balance = $80 (collateral) . ✅
My credits = $80 ❌ (loan/outstandings). Here i went undercollateralized.... 80 < 100. 😈

You're agree to pay my debts out. And in consideration of it company will give you some bonus of 10% of payment, okay.

After paying my debts... Reflections/affects would be ....

You as a liquidator...

Your balance = $40 (collateral) . (120 - 80)...
$120(your last balance) - $80(my loan, You paid) .

Your credits = $50 (loan/outstandings).

Me me an undercollateralized(bad debt)....

My balance = $0 (collateral) . Lein
My credits = $0 (loan/outstandings). You paid

Here company seized/lein/kinda forfeited.., my balance/collaterals.

My loan/credits was of $80 dollars, So your share/bonus(more precise) would be: 10% of 80 => $8.

So your updated affects...(affected ⚖️)

_revertIfHealthFactorIsBroken(msg.sender);

Your balance = $48 (collateral) . (40 + 8)... ❌
$40(your last balance) + $8(Your earning/bonus) .
Your credits = $50 (loan/outstandings). ✅

Oooohhhh heck...... You also went undercollateralized whilst paying my debts. 😈

Protocol Or logic will revert that whole transaction.

[Himchenhim]

However, I assume, that check of my _healthFactor will be in the end of this code, after I will receive also your collateral (code is below). What am I missing?

1. I am receiving your collateral
2. Paying for you by "burn()" function
3. Check for healthFactor

function liquidate(address collateral, address user, uint256 debtToCover) external moreThanZero(debtToCover) isAllowedToken(collateral) nonReentrant {
        uint256 startHealthFactor = _healthFactor(user);
        if (startHealthFactor >= MIN_HEALTH_FACTOR) {
            revert DSCEngine__healthFactorIsNotBroken();
        }

        uint256 collateralAmountFromDebtCovered = getTokenAmountFromUSD(collateral, debtToCover);
        uint256 bonusCollateral = (collateralAmountFromDebtCovered * LIQUIDATION_BONUS) / LIQUIDATION_PRECISION;
        uint256 netCollateralAmount = collateralAmountFromDebtCovered + bonusCollateral;
        
        // here I am receiving collateral
--->   _redeemCollateral(collateral, netCollateralAmount, user, msg.sender);
        _burnDSC(user, msg.sender, debtToCover);

        uint256 endingUserHealthFactor = _healthFactor(user);

        if (endingUserHealthFactor <= startHealthFactor) {
            revert DSCEngine__healthFactorNotImproved();
        }
       // here the check is happened
---> _revertIfHealthFactorIsBroken(msg.sender);
    }

[theirrationalone]

Right right right.... Sorry i went to an another similar audit.... During explanation !

Great catch @Himchenhim 🚀

Definitely i took wrong values for explaining the scenario... But voila if you can understand this much then you might got my point and why Patrick's DSC yes now i'm concious it's Patrick's DSC logic, is reverting on liquidation.

[theirrationalone]

Now here's your chance to do a favour on me please.... Took some another example values and alert 🚨 people who will come to read this discussion in future

[Himchenhim]

Sorry, do you want me to describe example or I have understood you in the wrong way?

I am not native speaker, so pardon :)

[theirrationalone]

Yeppp, i'm asking you to help me and all other who'll read this discussion in future...

It's your turn to contribute for community ❤

[Himchenhim]

I agree, that the procedure of liquidation cannot make worse healthFactor of liquidator after successful liquidation of collateral (it can be easily only part of collateral) -> for gas efficiency we can raise call of function _revertIfHealthFactorIsBroken(msg.sender); to the top:

function liquidate(address collateral, address user, uint256 debtToCover) external moreThanZero(debtToCover) isAllowedToken(collateral) nonReentrant {
+       _revertIfHealthFactorIsBroken(msg.sender);
        
        uint256 startHealthFactor = _healthFactor(user);
        if (startHealthFactor >= MIN_HEALTH_FACTOR) {
            revert DSCEngine__healthFactorIsNotBroken();
        }

        uint256 collateralAmountFromDebtCovered = getTokenAmountFromUSD(collateral, debtToCover);
        uint256 bonusCollateral = (collateralAmountFromDebtCovered * LIQUIDATION_BONUS) / LIQUIDATION_PRECISION;
        uint256 netCollateralAmount = collateralAmountFromDebtCovered + bonusCollateral;

        _redeemCollateral(collateral, netCollateralAmount, user, msg.sender);
        _burnDSC(user, msg.sender, debtToCover);

        uint256 endingUserHealthFactor = _healthFactor(user);

        if (endingUserHealthFactor <= startHealthFactor) {
            revert DSCEngine__healthFactorNotImproved();
        }
-      _revertIfHealthFactorIsBroken(msg.sender);
    }

in this case, we check if liquidator health factor is good from the beginning without executing unnecessary transactions

[miladad8]

Hi, I'm getting a little bit confused here. In above example, What if all the user (the one with bad health factor ) has is 80$? We are giving the liquidator 80$ plus 10% bonus which will be 8$. Here in this line _redeemCollateral(collateral, netCollateralAmount, user, msg.sender); we are giving the liquidator 88$ from the user balance, that all he could have is 80$ balance. The system is giving bonus from user, not of it's own. right?
Any help would be appreciated.

[EngrPips]

If a user has a bad health factor and all that their collateral is presently worth is $80, then I think what they have actually minted of DSC should not be up to $80 because then our system has a big problem. Our system is supposed to flag a user as an unhealthy user the moment their collateral is not double the DSC they have minted. So an ideal scenario will be a user deposited $100 worth of collateral and minted $45 worth of DSC then their collateral value dropped to $80. At this point they are unhealthy because their collateral is no longer double what they minted of DSC, and then a liquidator comes along. We pay the Liquidator $45, which is the actual value of the DSC they minted, and give the Liquidator a bonus. This is what I understood, but maybe I need to be corrected.

[EngrPips]

If a user has a bad health factor and all that their collateral is presently worth is $80, then I think what they have actually minted of DSC should not be up to $80 because then our system has a big problem. Our system is supposed to flag a user as an unhealthy user the moment their collateral is not double the DSC they have minted. So an ideal scenario will be a user deposited $100 worth of collateral and minted $45 worth of DSC then their collateral value dropped to $80. At this point they are unhealthy because their collateral is no longer double what they minted of DSC, and then a liquidator comes along. We pay the Liquidator $45, which is the actual value of the DSC they minted, and give the Liquidator a bonus. This is what I understood, but maybe I need to be corrected.

[EngrPips]

Interesting.

[miladad8]

I am still not sure. I remember the example that patrick explains in the course. He said, Let's imagine a user deposit 100$ worth of eth and mint 50$ worth of DSC. Then the price of eth falling and the user collateral would worth 74$. Now he is undercollateral and a liquidator comes along and pay 50$ DSC and take 74$ and make 24$ profit! But apparently all the liquidator got is 55$(50$ DSC minted value + 10% bonus) of eth.

[EngrPips]

The Decentralized Stable Coin codebase isn't perfect but it really inform us a lot about how stable coin works.