search

Gearbox-protocol / core-v3

Other
28 stars 4 forks source link

fix: debt limit changes no longer break partial liquidations #250

Closed lekhovitsky closed 1 month ago

lekhovitsky commented 1 month ago

Fixes https://github.com/spearbit-audits/review-gearbox/issues/60

StErMi commented 1 month ago

The implementation is fine. Having a custom behavior when the repayment happens via the liquidation problems would create more harm than benefits because it could be exploited by self-partial-liquidations.

StErMi commented 1 month ago

@lekhovitsky @cryptarasecurity I'll be more specific to describe the changes made and the effects

The main differences are inside the CreditFacadeV3 internal function _revertIfOutOfDebtLimits

BEFORE: reverts if the new debt of the CA was below minDebt or above maxDebt AFTER:

What does it change?

1) For borrowing: nothing. When you borrow (increase debt) your debt must be within the limits. 2) For repaying / partial liquidating (full repay, or full liquidation will early return): you will be able to decrease the debt up to minDebt without worrying about the maxDebt upper bound (unlike before). This means that even if the DAO reduces the maxDebt too much and the CA debt was already above the upper threshold, you will still be able to repay/liquidate the debt even for just 1 wei without reverting the transaction.

Some of the concerns/side effects detailed in the issue https://github.com/spearbit-audits/review-gearbox/issues/60 are still there but the main one, at least in my opinion have been mitigated correctly: users that need to repay/liquidate the CA will be able to do so even if the DAO has reduced the maxDebt upper bound "too much" (relative to their debt). Such operation, when the final debt is above the min threshold (or fully remove the debt) should always be doable.

Given that the minDebt and maxDebt thresholds are there for a reason, we need to arrive at a point where the DAO must be trusted to choose valuable, meaningful and trusted values for those thresholds: 1) Reducing maxDebt too much will prevent users from increasing their borrow position or creating new borrow positions at all 2) Reducing minDebt too much will prevent users from creating new borrow positions or reducing their debt (via repay/partial liquidation)

Both scenarios are critical and for that reason, the Gearbox DAO must choose meaningful values for both minDebt and maxDebt