From 55fd31cfdd8c6f2b1398fcc110ad94518efa6bb4 Mon Sep 17 00:00:00 2001 From: Phil Lu Date: Thu, 19 Oct 2023 16:30:14 -0400 Subject: [PATCH 1/2] Enable `_claim_admin_fees` for `CurveCryptoPool` --- curvesim/pool/cryptoswap/pool.py | 39 +++++++++------- test/fixtures/curve/tricrypto_ng.vy | 15 ++++--- test/unit/test_cryptopool.py | 69 +++++++++++++++++++++++++++++ test/unit/test_tricrypto.py | 60 +++++++++++++++++++++++++ 4 files changed, 163 insertions(+), 20 deletions(-) diff --git a/curvesim/pool/cryptoswap/pool.py b/curvesim/pool/cryptoswap/pool.py index 9169d2e8c..be14f6b36 100644 --- a/curvesim/pool/cryptoswap/pool.py +++ b/curvesim/pool/cryptoswap/pool.py @@ -449,30 +449,32 @@ def _tweak_price( # noqa: complexity: 12 def _claim_admin_fees(self) -> None: # 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 +799,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 +834,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 +877,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 ) diff --git a/test/fixtures/curve/tricrypto_ng.vy b/test/fixtures/curve/tricrypto_ng.vy index f12495709..169552cf3 100644 --- a/test/fixtures/curve/tricrypto_ng.vy +++ b/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. diff --git a/test/unit/test_cryptopool.py b/test/unit/test_cryptopool.py index b7516a46e..3f54b709d 100644 --- a/test/unit/test_cryptopool.py +++ b/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() diff --git a/test/unit/test_tricrypto.py b/test/unit/test_tricrypto.py index 10cdfff95..c49cb1180 100644 --- a/test/unit/test_tricrypto.py +++ b/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() From 31267099bb96c2faf04b18a3cb90460af4ab395c Mon Sep 17 00:00:00 2001 From: Phil Lu Date: Fri, 20 Oct 2023 13:44:37 -0400 Subject: [PATCH 2/2] Update changelog and lint --- ...133429_philiplu97_tricrypto_claim_admin_fees.rst | 6 ++++++ curvesim/pool/cryptoswap/pool.py | 13 +++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 changelog.d/20231020_133429_philiplu97_tricrypto_claim_admin_fees.rst diff --git a/changelog.d/20231020_133429_philiplu97_tricrypto_claim_admin_fees.rst b/changelog.d/20231020_133429_philiplu97_tricrypto_claim_admin_fees.rst new file mode 100644 index 000000000..7ec584354 --- /dev/null +++ b/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. \ No newline at end of file diff --git a/curvesim/pool/cryptoswap/pool.py b/curvesim/pool/cryptoswap/pool.py index be14f6b36..ecff307a6 100644 --- a/curvesim/pool/cryptoswap/pool.py +++ b/curvesim/pool/cryptoswap/pool.py @@ -448,6 +448,14 @@ 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 @@ -892,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, @@ -1054,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)