:adhesive_bandage: issue fixed : liquidator can not burn dsc

[ibourn]

Problem: after a liquidation the liquidator cannot burn his dsc.

This is not a security breach, however it is a big problem because the more a liquidator will liquidate the more its healthfactor will decrease if it does not add collateral. And since he can’t burn his dsc, he won’t be able to remove his collateral later. As such, it could be considered that the protocol retains the liquidators' funds.

In the facts what is blocking is the function _burnDsc which decreases the balance of DscMinted only for onBehalfOf In the case of a liquidation msg.sender is different from onBehalfOf

For the general liquidation process:

• the liquidator must mint the dsc (and deposit collateral)
• during the liquidation:
  • the balance of DscMinted of onBehalfOf is reduced
  • we transfer the dsc from the liquidator to burn them

It appears that after liquidation the liquidator still has a balance of DscMinted in the protocol while he no longer holds a dsc. We did burn dsc but it’s his. onBehalfOf are just detaching from our accounts. He has no more debt but can do what he wants with his dsc (we took his collateral in exchange).

For the liquidator :

• The redeemCollateralForDsc function will fail because calling burnDsc first will have a revert: ERC20InsufficientBalance
• The redeemCollateral function will fail because ending with a health factor check, there will be a revert : DSCEngine__BreaksHealthFactor
• The liquidator cannot burn his dsc either, and so his collateral is blocked

The proposal is to add to _burnDSC:

       if (onBehalfOf!= dscFrom) {
            s_DSCMinted[dscFrom] -= amountDscToBurn;
        }

In the case where the two addresses are different, it is a liquidation, so we release the liquidator of his debt.

Following this change, this test should also be deleted:

    function testLiquidatorTakesOnUsersDebt() public liquidated {
        (uint256 liquidatorDscMinted,) = dsce.getAccountInformation(liquidator);
        assertEq(liquidatorDscMinted, amountToMint);
    }

and replace with:

    function test_LiquidatorHasNoDscAndNoDebtAfterLiquidation() public liquidated {
        (uint256 liquidatorDscMinted,) = dscEngine.getAccountInformation(LIQUIDATOR);
        uint256 liquidatorDscBalance = dsc.balanceOf(LIQUIDATOR);

        assertEq(liquidatorDscMinted, 0);
        assertEq(liquidatorDscBalance, 0);
    }

The liquidator pays well the debt of the liquidated since one transfers his dsc to burn them, but there is no reason, since he no longer has dsc, to have a debt to the protocol.

Placing the change in the liquidate function before calling burn could save some gas however it would be less clear. Since it is in the logic of _burnDsc that this operation must be carried out, if in the future other modifications are made, dissociating this modification from _burnDsc can lead to confusions and errors.

I have not yet found any discussion on this subject, I may have misunderstood something, in this case excuse my misunderstanding and tell me where I am wrong. But in my opinion the liquidator cannot burn his dsc is an issue.

[ibourn]

Hello, I am terribly sorry for this useless PR. I realized my mistake.

I was focused on the tests and the situation of the liquidator and I started to reason out of context with a closed protocol (no market for the dsc). As Patrick says, you have to take a break sometimes.

In the situation where there is a market for the dsc, the proposed correction is not sufficient. If a liquidator arrives with purchased dsc and other mints, then he should be withdrawn only the amount of dsc minted corresponding to the liquidation but in any case no more than what he minted himself. Another check would be necessary to adjust the amount of the liquidator’s share of debt to be reduced.

❗ Finally, and the most important is that this modification would bring an imbalance between the creation and the actual distribution (benefiting the liquidator at first). And therefore potentially a selling pressure (not good for a stablecoin). ❗

The protocol does not retain the funds of the liquidator. If he mints for its liquidation, he will simply resell the portion of collateral recovered from the liquidated for dsc to burn it and free himself of the debt taken back to the liquidated.

I therefore close this PR that should never have been done.

[usmanfarooq91]

Just to let you know, you have raised a legitimate issue. Have a look at this. But I don't think it will be fixed with a PR, It will be fixed in the audited version of DSC.

[ibourn]

@usmanfarooq91 Hey! Thanks for the comment, I hadn’t seen this report in the audit.

Yes the system for it to maintain from the beginning assumes a liquid market for the dsc, that the liquidator can supply itself without minting.

Anyway my proposal is not correct, and as I say in the explanation of my closure, I reasoned out of context without the existence of a market for the dsc.