Merge pull request #275 from curveresearch/tricrypto-claim-admin-fees

Enable `_claim_admin_fees` for `CurveCryptoPool`

changelog.d/20231020_133429_philiplu97_tricrypto_claim_admin_fees.rst

@@ -0,0 +1,6 @@
+Changed
+-------
+
+- Enabled _claim_admin_fees for CurveCryptoPool. For maintainability, Tricrypto_ng's 
+implementation and usage patterns, which are in test/fixtures/curve/tricrypto_ng.vy 
+(Ctrl + F _claim_admin_fees), are used for both 2-coin and 3-coin Cryptoswap pools.

curvesim/pool/cryptoswap/pool.py

@@ -448,31 +448,41 @@ def _tweak_price(  # noqa: complexity: 12
         self.virtual_price = virtual_price
 
     def _claim_admin_fees(self) -> None:
+        """
+        If the pool's profit has increased since the last fee claim, update profit,
+        pool value, and LP token supply to reflect the admin taking its share of the
+        fees by minting itself LP tokens. Otherwise, change nothing.
+
+        Tricrypto-NG and Cryptopool implement this functionality differently, so we
+        copy only Tricrypto-NG's way in this class for consistency.
+        """
         # no gulping logic needed for the python code
+        A: int = self.A
+        gamma: int = self.gamma
+
         xcp_profit: int = self.xcp_profit
         xcp_profit_a: int = self.xcp_profit_a
+        total_supply: int = self.tokens
+
+        if xcp_profit <= xcp_profit_a or total_supply < 10**18:
+            return
 
         vprice: int = self.virtual_price
 
-        if xcp_profit > xcp_profit_a:
-            fees: int = (xcp_profit - xcp_profit_a) * self.admin_fee // (2 * 10**10)
-            if fees > 0:
-                frac: int = vprice * 10**18 // (vprice - fees) - 10**18
-                d_supply = self.tokens * frac // 10**18
-                self.tokens += d_supply
-                xcp_profit -= fees * 2
-                self.xcp_profit = xcp_profit
+        fees: int = (xcp_profit - xcp_profit_a) * self.admin_fee // (2 * 10**10)
 
-        A = self.A
-        gamma = self.gamma
-        totalSupply = self.tokens
+        if fees > 0:
+            frac: int = vprice * 10**18 // (vprice - fees) - 10**18
+            d_supply: int = total_supply * frac // 10**18
+            self.tokens += d_supply
+            xcp_profit -= fees * 2
+            self.xcp_profit = xcp_profit
 
-        D: int = factory_2_coin.newton_D(A, gamma, self._xp())
+        D: int = newton_D(A, gamma, self._xp())
         self.D = D
-        self.virtual_price = 10**18 * self._get_xcp(D) // totalSupply
 
-        if xcp_profit > xcp_profit_a:
-            self.xcp_profit_a = xcp_profit
+        self.virtual_price = 10**18 * self._get_xcp(D) // self.tokens
+        self.xcp_profit_a = xcp_profit
 
     def get_dy(self, i: int, j: int, dx: int) -> int:
         """
@@ -797,6 +807,8 @@ def add_liquidity(
 
         assert d_token >= min_mint_amount, "Slippage"
 
+        self._claim_admin_fees()
+
         return d_token
 
     def _calc_token_fee(self, amounts: List[int], xp: List[int]) -> int:
@@ -830,6 +842,8 @@ def remove_liquidity(
         """
         min_amounts = min_amounts or [0, 0]
 
+        self._claim_admin_fees()
+
         total_supply: int = self.tokens
         self.tokens -= _amount
         balances: List[int] = self.balances
@@ -871,6 +885,9 @@ def remove_liquidity_one_coin(
         D: int = 0
         p: Optional[int] = None
         xp = [0] * self.n
+
+        self._claim_admin_fees()
+
         dy, p, D, xp = self._calc_withdraw_one_coin(
             A, gamma, token_amount, i, False, True
         )
@@ -883,7 +900,7 @@ def remove_liquidity_one_coin(
 
         return dy
 
-    # pylint: disable-next=too-many-locals,too-many-arguments
+    # pylint: disable-next=too-many-locals,too-many-arguments, too-many-branches
     def _calc_withdraw_one_coin(
         self,
         A: int,
@@ -1045,7 +1062,8 @@ def lp_price(self) -> int:
             price_oracle: List[int] = self.internal_price_oracle()
             price: int = factory_2_coin.lp_price(virtual_price, price_oracle)
         elif self.n == 3:
-            # 3-coin vyper contract uses cached packed oracle prices instead of internal_price_oracle()
+            # 3-coin vyper contract uses cached packed oracle prices instead of
+            # internal_price_oracle()
             virtual_price = self.virtual_price
             price_oracle = self._price_oracle
             price = tricrypto_ng.lp_price(virtual_price, price_oracle)

test/fixtures/curve/tricrypto_ng.vy

@@ -1211,11 +1211,16 @@ def _claim_admin_fees():
     #         `self.balances` yet: pool balances only account for incoming and
     #                  outgoing tokens excluding fees. Following 'gulps' fees:
 
-    for i in range(N_COINS):
-        if coins[i] == WETH20:
-            self.balances[i] = self.balance
-        else:
-            self.balances[i] = ERC20(coins[i]).balanceOf(self)
+    # curvesim: commented out this for loop to avoid overwriting balances to 
+    # empty(uint256[N_COINS]) since we aren't going to instantiate the ERC20 contracts
+    # for the coins in this pool, which would make testing cumbersome. Also, the Python 
+    # pool doesn't gulp.
+
+    # for i in range(N_COINS):
+    #     if coins[i] == WETH20:
+    #         self.balances[i] = self.balance
+    #     else:
+    #         self.balances[i] = ERC20(coins[i]).balanceOf(self)
 
     #            If the pool has made no profits, `xcp_profit == xcp_profit_a`
     #                         and the pool gulps nothing in the previous step.

test/unit/test_cryptopool.py

@@ -555,6 +555,9 @@ def test_add_liquidity(vyper_cryptopool, x0, x1):
     pool = initialize_pool(vyper_cryptopool)
 
     expected_lp_amount = vyper_cryptopool.add_liquidity(amounts, 0)
+    # cryptopool.vy doesn't claim admin fees like this, but pool does the claim like
+    # tricrypto_ng.vy does for maintainability.
+    vyper_cryptopool.claim_admin_fees()
     expected_balances = [vyper_cryptopool.balances(i) for i in range(len(xp))]
     expected_lp_supply = vyper_cryptopool.totalSupply()
     expected_D = vyper_cryptopool.D()
@@ -579,6 +582,9 @@ def test_remove_liquidity(vyper_cryptopool, amount):
 
     pool = initialize_pool(vyper_cryptopool)
 
+    # cryptopool.vy doesn't claim admin fees like this, but pool does the claim like
+    # tricrypto_ng.vy does for maintainability.
+    vyper_cryptopool.claim_admin_fees()
     vyper_cryptopool.remove_liquidity(amount, [0, 0])
     expected_balances = [vyper_cryptopool.balances(i) for i in range(2)]
     expected_lp_supply = vyper_cryptopool.totalSupply()
@@ -603,6 +609,9 @@ def test_remove_liquidity_one_coin(vyper_cryptopool, amount, i):
 
     pool = initialize_pool(vyper_cryptopool)
 
+    # cryptopool.vy doesn't claim admin fees like this, but pool does the claim like
+    # tricrypto_ng.vy does for maintainability.
+    vyper_cryptopool.claim_admin_fees()
     vyper_cryptopool.remove_liquidity_one_coin(amount, i, 0)
     expected_coin_balance = vyper_cryptopool.balances(i)
     expected_lp_supply = vyper_cryptopool.totalSupply()
@@ -794,3 +803,63 @@ def test_dydxfee(vyper_cryptopool):
     dx *= precisions[i]
     dy *= precisions[j]
     assert abs(dydx - dy / dx) < 1e-6
+
+
+def test_claim_admin_fees(vyper_cryptopool):
+    """Test admin fee claim against vyper implementation."""
+    update_cached_values(vyper_cryptopool)
+    pool = initialize_pool(vyper_cryptopool)
+
+    # vyper_cryptopool's xcp_profit starts out > xcp_profit_a
+    actual_xcp_profit = pool.xcp_profit
+    xcp_profit_a = pool.xcp_profit_a
+    D = pool.D
+    tokens = pool.tokens
+    vprice = pool.virtual_price
+
+    reduced_xcp_profit = pool.xcp_profit_a - 1
+    vyper_cryptopool.eval(f"self.xcp_profit = {reduced_xcp_profit}")
+    pool.xcp_profit = reduced_xcp_profit
+
+    vyper_cryptopool.claim_admin_fees()
+    pool._claim_admin_fees()
+
+    # shouldn't have enough profit to claim admin fees
+    assert (
+        pool.xcp_profit <= pool.xcp_profit_a
+        and vyper_cryptopool.xcp_profit() <= vyper_cryptopool.xcp_profit_a()
+    )
+    assert D == pool.D == vyper_cryptopool.D()
+    assert tokens == pool.tokens == vyper_cryptopool.totalSupply()
+    assert reduced_xcp_profit == pool.xcp_profit == vyper_cryptopool.xcp_profit()
+    assert xcp_profit_a == pool.xcp_profit_a == vyper_cryptopool.xcp_profit_a()
+    assert vprice == pool.virtual_price == vyper_cryptopool.get_virtual_price()
+
+    vyper_cryptopool.eval(f"self.xcp_profit = {actual_xcp_profit}")
+    pool.xcp_profit = actual_xcp_profit
+
+    # should have enough profit to claim admin fees
+    assert (
+        pool.xcp_profit > pool.xcp_profit_a
+        and vyper_cryptopool.xcp_profit() > vyper_cryptopool.xcp_profit_a()
+    )
+
+    expected_fees = (
+        (pool.xcp_profit - pool.xcp_profit_a) * pool.admin_fee // (2 * 10**10)
+    )
+    expected_token_frac = vprice * 10**18 // (vprice - expected_fees) - 10**18
+    expected_token_supply = pool.tokens + (
+        pool.tokens * expected_token_frac // 10**18
+    )
+
+    expected_xcp_profit = pool.xcp_profit - expected_fees * 2
+    expected_vprice = 10**18 * pool._get_xcp(pool.D) // expected_token_supply
+
+    vyper_cryptopool.claim_admin_fees()
+    pool._claim_admin_fees()
+
+    assert D == pool.D == vyper_cryptopool.D()  # D shouldn't change
+    assert expected_token_supply == pool.tokens == vyper_cryptopool.totalSupply()
+    assert expected_xcp_profit == pool.xcp_profit == vyper_cryptopool.xcp_profit()
+    assert expected_xcp_profit == pool.xcp_profit_a == vyper_cryptopool.xcp_profit_a()
+    assert expected_vprice == pool.virtual_price == vyper_cryptopool.get_virtual_price()

test/unit/test_tricrypto.py

@@ -582,3 +582,63 @@ def test_calc_withdraw_one_coin(vyper_tricrypto, amount, i):
 
     expected_balances = [vyper_tricrypto.balances(i) for i in range(n_coins)]
     assert pool.balances == expected_balances
+
+
+def test_claim_admin_fees(vyper_tricrypto, tricrypto_math):
+    """Test admin fee claim against vyper implementation."""
+    update_cached_values(vyper_tricrypto, tricrypto_math)
+    pool = initialize_pool(vyper_tricrypto)
+
+    # vyper_tricrypto's xcp_profit starts out > xcp_profit_a
+    actual_xcp_profit = pool.xcp_profit
+    xcp_profit_a = pool.xcp_profit_a
+    D = pool.D
+    tokens = pool.tokens
+    vprice = pool.virtual_price
+
+    reduced_xcp_profit = pool.xcp_profit_a - 1
+    vyper_tricrypto.eval(f"self.xcp_profit = {reduced_xcp_profit}")
+    pool.xcp_profit = reduced_xcp_profit
+
+    vyper_tricrypto.claim_admin_fees()
+    pool._claim_admin_fees()
+
+    # shouldn't have enough profit to claim admin fees
+    assert (
+        pool.xcp_profit <= pool.xcp_profit_a
+        and vyper_tricrypto.xcp_profit() <= vyper_tricrypto.xcp_profit_a()
+    )
+    assert D == pool.D == vyper_tricrypto.D()
+    assert tokens == pool.tokens == vyper_tricrypto.totalSupply()
+    assert reduced_xcp_profit == pool.xcp_profit == vyper_tricrypto.xcp_profit()
+    assert xcp_profit_a == pool.xcp_profit_a == vyper_tricrypto.xcp_profit_a()
+    assert vprice == pool.virtual_price == vyper_tricrypto.virtual_price()
+
+    vyper_tricrypto.eval(f"self.xcp_profit = {actual_xcp_profit}")
+    pool.xcp_profit = actual_xcp_profit
+
+    # should have enough profit to claim admin fees
+    assert (
+        pool.xcp_profit > pool.xcp_profit_a
+        and vyper_tricrypto.xcp_profit() > vyper_tricrypto.xcp_profit_a()
+    )
+
+    expected_fees = (
+        (pool.xcp_profit - pool.xcp_profit_a) * pool.admin_fee // (2 * 10**10)
+    )
+    expected_token_frac = vprice * 10**18 // (vprice - expected_fees) - 10**18
+    expected_token_supply = pool.tokens + (
+        pool.tokens * expected_token_frac // 10**18
+    )
+
+    expected_xcp_profit = pool.xcp_profit - expected_fees * 2
+    expected_vprice = 10**18 * pool._get_xcp(pool.D) // expected_token_supply
+
+    vyper_tricrypto.claim_admin_fees()
+    pool._claim_admin_fees()
+
+    assert D == pool.D == vyper_tricrypto.D()  # D shouldn't change
+    assert expected_token_supply == pool.tokens == vyper_tricrypto.totalSupply()
+    assert expected_xcp_profit == pool.xcp_profit == vyper_tricrypto.xcp_profit()
+    assert expected_xcp_profit == pool.xcp_profit_a == vyper_tricrypto.xcp_profit_a()
+    assert expected_vprice == pool.virtual_price == vyper_tricrypto.virtual_price()