Feat: Prevent Liquidations Resulting in Dust Debt

[yan-man]

• prevent liquidations that result in dust debt unless all borrower's reserve collateral at hand is seized
• if liquidator input debtToCover is insufficient, revert. Otherwise, adjust debt to liquidate to a value that prevents dust to result

closes #608
closes #399

[yan-man]

for reference:

• debtToCover is value input by liquidator
• totalBorrowerReserveDebt is the total debt for the user being liquidated in the debt reserve being repaid
• debtToRestoreCloseFactor is the amt of debt to repay to restore liquidated user to desired close factor

[yan-man]

this means debtToCover is set too low by liquidator, thus revert. debtToCover is a hard cap, and we can never liquidate more than this value

[avniculae]

it can be the case that params.debtToRestoreCloseFactor = debtToCover, in which case the liquidator will need to input 1 wei more just to pass this check, since actualDebtToLiquidate will remain the same. hence, I would rewrite this as:

if (debtToCover < params.debtToRestoreCloseFactor)

wdyt?

we don't need to consider params.totalBorrowerReserveDebt since we checked remainingDebtInBaseCurrency < MIN_LEFTOVER_BASE above, so it must be the case that min is either debtToCover or params.debtToRestoreCloseFactor, right?

[avniculae]

It turns out that all that we care if there is any dust is for the debtCover to be >= totalDebt. This is bc even is the min value is the closeFactor, if debtToCover is < totalDebt then debtToCover must leave dust (bc closeFactor left dust, which is smaller). Simplified logic: 66deb06

[yan-man]

if it reaches this branch, it means that debtToRestoreCloseFactor is the min value. However, we are again hard capped by debtToCover so need to check if it is set to a value that is greater than debtToRestoreCloseFactor but still results in dust. If so revert

[yan-man]

if debtToCover doesn't result in dust, increase the actualDebtToLiquidate to the max upper cap. Can never exceed debtToCover or totalBorrowerReserveDebt

[DhairyaSethi]

at this point maxLiquidatableDebt is totalBorrowerReserveDebt (bc debtToCover > totalborrowerReserveDebt from the if part of this else branch, ie still respect liquidating within debtToCover input), right?
if we reach this branch, we are liquidating everything on that reserve and should not be left with dust, right?

[yan-man]

bc debtToCover > totalborrowerReserveDebt

based on the if branch, which is basically debtToCover < params.totalBorrowerReserveDebt && "remainingDebtFromDebtToCover leaving dust", then this branch is either: debtToCover > totalBorrowerReserveDebt || remainingDebtFromDebtToCover not leaving dust

so besides debtToCover > totalborrowerReserveDebt, it can also be that debtToCover < totalborrowerReserveDebt, but allows a remaining debt amount that doesn't leave dust, so a valid value.

So here, debtToRestoreCloseFactor < debtToCover < totalborrowerReserveDebt, where debtToRestoreCloseFactor would leave dust but debtToCover wouldn't leave dust. Therefore adjust the actualDebtToLiquidate to min(debtToCover, totalborrowerReserveDebt) if debtToCover is valid

Not necessarily all of totalborrowerReserveDebt

[yan-man]

we must also do this check bc of the scenario where debt value exceeds collateral value. Therefore max debt is not possible to be liquidated.

Consider a multi-reserve case (assuming no liq bonus, no liq fee):
Collateral:

• asset A: $8k
• asset B: $5k (coll to seize)

Debt:

• asset C: $8k
• asset D: $5.4k (debt to liq)

if actualDebtToLiquidate ends up as $5.4k, ie liquidating all of asset D, by the time we come here to calc available collateral, there isn't enough collateral to liquidate to cover the whole debt and thus we can only liquidate $5k worth of asset D, leading to dust.

[yan-man]

resolved - this scenario is acceptable as long as all the reserve collateral is seized

[yan-man]

if we're also set on no parallel liqs, then can rm this array type input and the loop in liqCall (to be done in liquidation refactor)

[avniculae]

currently this is the only assumption of the spoke that the aave oracle uses 8 decimals. I think we should hardcode 1000e18 and then multiply it by 10^(oracle.decimals()), wdyt?

[DhairyaSethi]

it's a pretty safe assumption, we can avoid doing this in the src code but good to add a test for this to flag non standard oracle decimals

[avniculae]

66deb06

[avniculae]

is this used anywhere?

[avniculae]

66deb06

[avniculae]

here we could bound debtToCover instead of assuming, wdyt?

[avniculae]

6f36809