fix: min shares for withdraw (CS-CURVE-LLV2-001)

curve_stablecoin/AMM.vy

@@ -730,6 +730,8 @@ def withdraw(user: address, frac: uint256) -> uint256[2]:
         y: uint256 = self.bands_y[n]
         ds: uint256 = unsafe_div(frac * user_shares[i], 10**18)
         user_shares[i] = unsafe_sub(user_shares[i], ds)  # Can ONLY zero out when frac == 10**18
+        assert user_shares[i] == 0 or user_shares[i] >= MIN_SHARES_ALLOWED, "Amount left too low"
+
         s: uint256 = self.total_shares[n]
         new_shares: uint256 = s - ds
         self.total_shares[n] = new_shares

tests/amm/test_deposit_withdraw.py

@@ -23,6 +23,7 @@ def test_deposit_withdraw(
         map(lambda x: x // 10 ** (18 - collateral_token.decimals()), amounts)
     )
     deposits = {}
+    deposit_shares = {}
     precisions = {}
     with boa.env.prank(admin):
         for user, amount, n1, dn in zip(accounts, amounts, ns, dns):
@@ -34,13 +35,15 @@ def test_deposit_withdraw(
 
             y_per_band = amount * 10 ** (18 - collateral_token.decimals()) // (dn + 1)
             amount_too_low = y_per_band <= 100
+            user_shares = []
             for n in range(n1, n2 + 1):
                 if amount_too_low:
                     break
                 total_y = amm.bands_y(n)
                 # Total / user share
                 s = amm.eval(f"self.total_shares[{n}]")
                 ds = ((s + DEAD_SHARES) * y_per_band) // (total_y + 1)
+                user_shares.append(ds)
                 amount_too_low = amount_too_low or ds < MIN_SHARES_ALLOWED
 
             if amount_too_low:
@@ -50,6 +53,7 @@ def test_deposit_withdraw(
                 amm.deposit_range(user, amount, n1, n2)
                 mint_for_testing(collateral_token, amm.address, amount)
                 deposits[user] = amount
+                deposit_shares[user] = user_shares
                 assert collateral_token.balanceOf(user) == 0
 
         for user, n1 in zip(accounts, ns):
@@ -67,14 +71,25 @@ def test_deposit_withdraw(
 
         for user, frac, amount in zip(accounts, fracs, amounts):
             if user in deposits:
-                before = amm.get_sum_xy(user)
-                amm.withdraw(user, frac)
-                after = amm.get_sum_xy(user)
-                assert before[1] - after[1] == pytest.approx(
-                    deposits[user] * frac / 1e18,
-                    rel=precisions[user],
-                    abs=25 + deposits[user] * precisions[user],
+                amount_left_too_low = any(
+                    (
+                        share - (frac * share) // 10**18 != 0
+                        and share - (frac * share) // 10**18 < MIN_SHARES_ALLOWED
+                    )
+                    for share in deposit_shares[user]
                 )
+                if amount_left_too_low:
+                    with boa.reverts("Amount left too low"):
+                        amm.withdraw(user, frac)
+                else:
+                    before = amm.get_sum_xy(user)
+                    amm.withdraw(user, frac)
+                    after = amm.get_sum_xy(user)
+                    assert before[1] - after[1] == pytest.approx(
+                        deposits[user] * frac / 1e18,
+                        rel=precisions[user],
+                        abs=25 + deposits[user] * precisions[user],
+                    )
             else:
                 with boa.reverts("No deposits"):
                     amm.withdraw(user, frac)

tests/amm/test_min_shares.py

@@ -3,6 +3,19 @@
 from tests.utils.constants import DEAD_SHARES, MIN_SHARES_ALLOWED, MAX_UINT256
 
 
+def _ceil_div(x, y):
+    return (x + y - 1) // y
+
+
+def _find_frac_for_remaining(shares, remaining):
+    ds = shares - remaining
+    lower = _ceil_div(ds * 10**18, shares)
+    upper = ((ds + 1) * 10**18 - 1) // shares
+    if lower <= upper:
+        return lower
+    return None
+
+
 def test_min_shares(amm, collateral_token, admin, accounts):
     user = accounts[0]
     collateral_precision = 10 ** (18 - collateral_token.decimals())
@@ -42,6 +55,62 @@ def test_min_shares_fails(amm, collateral_token, admin, accounts):
                 )
 
 
+def test_min_shares_withdraw(amm, collateral_token, admin, accounts):
+    user = accounts[0]
+    collateral_precision = 10 ** (18 - collateral_token.decimals())
+    collateral_amount = _ceil_div(
+        2 * MIN_SHARES_ALLOWED, collateral_precision * DEAD_SHARES
+    )
+    boa.deal(collateral_token, user, collateral_amount)
+
+    active_band = amm.active_band()
+    with boa.env.prank(admin):
+        amm.deposit_range(user, collateral_amount, active_band - 1, active_band - 1)
+
+        shares = amm.eval(f"self.user_shares[{user}].ticks[0]")
+        frac = None
+        for remaining in range(
+            MIN_SHARES_ALLOWED,
+            MIN_SHARES_ALLOWED + shares // 10**18 + 3,
+        ):
+            frac = _find_frac_for_remaining(shares, remaining)
+            if frac is not None:
+                break
+
+        assert frac is not None
+        amm.withdraw(user, frac)
+
+        assert amm.eval(f"self.user_shares[{user}].ticks[0]") >= MIN_SHARES_ALLOWED
+
+
+def test_min_shares_withdraw_fails(amm, collateral_token, admin, accounts):
+    user = accounts[0]
+    collateral_precision = 10 ** (18 - collateral_token.decimals())
+    collateral_amount = _ceil_div(
+        2 * MIN_SHARES_ALLOWED, collateral_precision * DEAD_SHARES
+    )
+    boa.deal(collateral_token, user, collateral_amount)
+
+    active_band = amm.active_band()
+    with boa.env.prank(admin):
+        amm.deposit_range(user, collateral_amount, active_band - 1, active_band - 1)
+
+        shares = amm.eval(f"self.user_shares[{user}].ticks[0]")
+        frac = None
+        for remaining in range(
+            MIN_SHARES_ALLOWED - 1,
+            MIN_SHARES_ALLOWED - (shares // 10**18 + 3),
+            -1,
+        ):
+            frac = _find_frac_for_remaining(shares, remaining)
+            if frac is not None:
+                break
+
+        assert frac is not None
+        with boa.reverts("Amount left too low"):
+            amm.withdraw(user, frac)
+
+
 def test_share_price(
     amm,
     collateral_token,

tests/stableborrow/test_liquidate.py

@@ -1,13 +1,49 @@
 import pytest
 import boa
-from boa import BoaError
 from hypothesis import given, settings
 from hypothesis import strategies as st
+from tests.utils.constants import MIN_SHARES_ALLOWED
 
 
 N = 5
 
 
+def _amount_left_too_low(amm, user, frac):
+    ns = amm.read_user_tick_numbers(user)
+    packed_shares = amm.user_shares(user).ticks
+    n_bands = ns[1] - ns[0] + 1
+    user_shares = []
+    for packed in packed_shares:
+        if len(user_shares) == n_bands:
+            break
+        user_shares.append(packed & (2**128 - 1))
+        if len(user_shares) == n_bands:
+            break
+        user_shares.append(packed >> 128)
+
+    for share in user_shares:
+        remaining = share - (frac * share) // 10**18
+        if remaining != 0 and remaining < MIN_SHARES_ALLOWED:
+            return True
+    return False
+
+
+def _liquidation_f_remove(controller, user, caller, frac):
+    debt = controller.debt(user)
+    debt = (debt * frac + 10**18 - 1) // 10**18
+    if debt == 0:
+        return 0
+
+    if controller.debt(user) == debt:
+        return 10**18
+
+    health_limit = 0
+    if user != caller and not controller.approval(user, caller):
+        health_limit = controller.liquidation_discounts(user)
+
+    return controller.eval(f"core._get_f_remove({frac}, {health_limit})")
+
+
 @pytest.fixture(scope="module")
 def controller_for_liquidation(
     stablecoin,
@@ -147,12 +183,18 @@ def test_liquidate_callback(
         b = stablecoin.balanceOf(fee_receiver)
         stablecoin.transfer(fake_leverage.address, b)
         health_before = controller.health(user)
-
-        try:
+        min_x = int(0.999 * f * x / 1e18)
+        debt_frac = (controller.debt(user) * frac + 10**18 - 1) // 10**18
+        f_remove = _liquidation_f_remove(controller, user, fee_receiver, frac)
+        if debt_frac == 0:
+            with boa.reverts():
+                controller.liquidate(user, min_x, frac, fake_leverage.address, b"")
+        elif _amount_left_too_low(market_amm, user, f_remove):
+            with boa.reverts("Amount left too low"):
+                controller.liquidate(user, min_x, frac, fake_leverage.address, b"")
+        else:
             dy = collateral_token.balanceOf(fee_receiver)
-            controller.liquidate(
-                user, int(0.999 * f * x / 1e18), frac, fake_leverage.address, b""
-            )
+            controller.liquidate(user, min_x, frac, fake_leverage.address, b"")
             dy = collateral_token.balanceOf(fee_receiver) - dy
             dx = stablecoin.balanceOf(fee_receiver) - b
             if f > 0:
@@ -166,13 +208,6 @@ def test_liquidate_callback(
                 ), "Liquidator didn't make money"
             if f != 10**18 and f > 0:
                 assert controller.health(user) > health_before
-        except BoaError as e:
-            if frac == 0 and "Loan doesn't exist" in str(e):
-                pass
-            elif frac * controller.debt(user) // 10**18 == 0:
-                pass
-            else:
-                raise
 
 
 def test_self_liquidate(
@@ -220,6 +255,16 @@ def test_tokens_to_liquidate(
         initial_balance = stablecoin.balanceOf(fee_receiver)
 
         with boa.env.prank(fee_receiver):
+            debt_frac = (controller.debt(user) * frac + 10**18 - 1) // 10**18
+            f_remove = _liquidation_f_remove(controller, user, fee_receiver, frac)
+            if debt_frac == 0:
+                with boa.reverts():
+                    controller.liquidate(user, 0, frac)
+                return
+            if _amount_left_too_low(market_amm, user, f_remove):
+                with boa.reverts("Amount left too low"):
+                    controller.liquidate(user, 0, frac)
+                return
             controller.liquidate(user, 0, frac)
 
         balance = stablecoin.balanceOf(fee_receiver)