fix: Prevent underflow in liquidations due to smalll offset

Author: bingen

packages/contracts/contracts/Interfaces/IStabilityPool.sol

@@ -158,6 +158,13 @@ interface IStabilityPool {
      */
     function getTotalLUSDDeposits() external view returns (uint);
 
+    /*
+     * Returns the max amount of LUSD held in the pool that can be used for liquidations.
+     * It makes sure that at least 1 LUSD remains.
+     * If the max amount is used, it makes sure it won’t revert by underflow due to the accumulated offset error.
+     */
+    function getMaxAmountToOffset() external view returns (uint);
+
     /*
      * Calculates the ETH gain earned by the deposit since its last snapshots were taken.
      */

packages/contracts/contracts/StabilityPool.sol

@@ -492,6 +492,20 @@ contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool {
 
     // --- Liquidation functions ---
 
+    function getMaxAmountToOffset() external view override returns (uint) {
+        uint totalLUSD = totalLUSDDeposits; // cache
+        // - If the SP has total deposits >= 1e18, we leave 1e18 in it untouched.
+        // - If it has 0 < x < 1e18 total deposits, we leave x in it.
+        uint256 lusdToLeaveInSP = LiquityMath._min(MIN_LUSD_IN_SP, totalLUSD);
+        uint LUSDInSPForOffsets = totalLUSD - lusdToLeaveInSP; // safe, for the line above
+        // Let’s avoid underflow in case of a tiny offset
+        if (LUSDInSPForOffsets.mul(DECIMAL_PRECISION) <= lastLUSDLossError_Offset) {
+            LUSDInSPForOffsets = 0;
+        }
+
+        return LUSDInSPForOffsets;
+    }
+
     /*
     * Cancels out the specified debt against the LUSD contained in the Stability Pool (as far as possible)
     * and transfers the Trove's ETH collateral from ActivePool to StabilityPool.
@@ -536,7 +550,19 @@ contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool {
         uint ETHNumerator = _collToAdd.mul(DECIMAL_PRECISION).add(lastETHError_Offset);
 
         assert(_debtToOffset < _totalLUSDDeposits);
-        uint LUSDLossNumerator = _debtToOffset.mul(DECIMAL_PRECISION).sub(lastLUSDLossError_Offset);
+        uint LUSDLossNumerator;
+        /* Let’s avoid underflow in case of a small offset
+         * Per getMaxAmountToOffset, if the max used, this will never happen.
+         * If the max is not used, then offset value is at least MN_NET_DEBT,
+         * which means that total LUSD deposits when error was produced was around 2e21 LUSD.
+         * See: https://github.com/liquity/dev/pull/417#issuecomment-805721292
+         * As we are doing floor + 1 in the division, it will still offset something
+         */
+        if (_debtToOffset.mul(DECIMAL_PRECISION) <= lastLUSDLossError_Offset) {
+            LUSDLossNumerator = 0;
+        } else {
+            LUSDLossNumerator = _debtToOffset.mul(DECIMAL_PRECISION).sub(lastLUSDLossError_Offset);
+        }
         /*
          * Add 1 to make error in quotient positive. We want "slightly too much" LUSD loss,
          * which ensures the error in any given compoundedLUSDDeposit favors the Stability Pool.

packages/contracts/contracts/TroveManager.sol

@@ -38,9 +38,6 @@ contract TroveManager is LiquityBase, Ownable, CheckContract, ITroveManager {
 
     // --- Data structures ---
 
-    // From StabilityPool
-    uint constant internal MIN_LUSD_IN_SP = 1e18;
-
     uint constant public SECONDS_IN_ONE_MINUTE = 60;
     /*
      * Half-life of 12h. 12h = 720 min
@@ -512,11 +509,7 @@ contract TroveManager is LiquityBase, Ownable, CheckContract, ITroveManager {
         LiquidationTotals memory totals;
 
         vars.price = priceFeed.fetchPrice();
-        // - If the SP has total deposits >= 1e18, we leave 1e18 in it untouched.
-        // - If it has 0 < x < 1e18 total deposits, we leave x in it.
-        uint256 totalLUSDDeposits = stabilityPoolCached.getTotalLUSDDeposits();
-        uint256 lusdToLeaveInSP = LiquityMath._min(MIN_LUSD_IN_SP, totalLUSDDeposits);
-        vars.LUSDInSPForOffsets = totalLUSDDeposits - lusdToLeaveInSP; // safe, for the line above
+        vars.LUSDInSPForOffsets = stabilityPoolCached.getMaxAmountToOffset();
         vars.recoveryModeAtStart = _checkRecoveryMode(vars.price);
 
         // Perform the appropriate liquidation sequence - tally the values, and obtain their totals
@@ -658,11 +651,7 @@ contract TroveManager is LiquityBase, Ownable, CheckContract, ITroveManager {
         LiquidationTotals memory totals;
 
         vars.price = priceFeed.fetchPrice();
-        // - If the SP has total deposits >= 1e18, we leave 1e18 in it untouched.
-        // - If it has 0 < x < 1e18 total deposits, we leave x in it.
-        uint256 totalLUSDDeposits = stabilityPoolCached.getTotalLUSDDeposits();
-        uint256 lusdToLeaveInSP = LiquityMath._min(MIN_LUSD_IN_SP, totalLUSDDeposits);
-        vars.LUSDInSPForOffsets = totalLUSDDeposits - lusdToLeaveInSP; // safe, for the line above
+        vars.LUSDInSPForOffsets = stabilityPoolCached.getMaxAmountToOffset();
         vars.recoveryModeAtStart = _checkRecoveryMode(vars.price);
 
         // Perform the appropriate liquidation sequence - tally values and obtain their totals.