From b0079e37633ce0e9be91daeea7dbc3bce61af92f Mon Sep 17 00:00:00 2001 From: bout3fiddy <11488427+bout3fiddy@users.noreply.github.com> Date: Tue, 14 Nov 2023 13:42:14 +0100 Subject: [PATCH 1/4] add test for fixed precision issues --- tests/unitary/math/test_get_y.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unitary/math/test_get_y.py b/tests/unitary/math/test_get_y.py index 6f0425b7..cefbab13 100644 --- a/tests/unitary/math/test_get_y.py +++ b/tests/unitary/math/test_get_y.py @@ -99,3 +99,17 @@ def calculate_F_by_y0(y0): ) or abs(calculate_F_by_y0(result_get_y)) <= abs( calculate_F_by_y0(result_original) ) + + +def test_get_y_revert(math_contract): + a = 1723894848 + gamma = 24009999997600 + x = [ + 112497148627520223862735198942112, + 112327102289152450435452075003508 + ] + D = 224824250915890636214130540882688 + i = 0 + + y_out = math_contract.newton_y(a, gamma, x, D, i) + y_out = math_contract.get_y(a, gamma, x, D, i) From 2f3f93159ad09e37b8fd6f06d5b215cf0f62bbcb Mon Sep 17 00:00:00 2001 From: bout3fiddy <11488427+bout3fiddy@users.noreply.github.com> Date: Tue, 14 Nov 2023 14:28:10 +0100 Subject: [PATCH 2/4] pack last timestamp into one uint256 and add tests --- contracts/main/CurveTwocryptoFactory.vy | 8 +-- contracts/main/CurveTwocryptoOptimized.vy | 62 ++++++++++++++--------- tests/unitary/math/test_get_y.py | 11 ++-- tests/unitary/math/test_packing.py | 29 +++++++++++ 4 files changed, 75 insertions(+), 35 deletions(-) create mode 100644 tests/unitary/math/test_packing.py diff --git a/contracts/main/CurveTwocryptoFactory.vy b/contracts/main/CurveTwocryptoFactory.vy index c82baf20..7a012b9a 100644 --- a/contracts/main/CurveTwocryptoFactory.vy +++ b/contracts/main/CurveTwocryptoFactory.vy @@ -112,7 +112,7 @@ def __init__(_fee_receiver: address, _admin: address): @internal @view -def _pack(x: uint256[3]) -> uint256: +def _pack_3(x: uint256[3]) -> uint256: """ @notice Packs 3 integers with values <= 10**18 into a uint256 @param x The uint256[3] to pack @@ -181,15 +181,15 @@ def deploy_pool( d: uint256 = ERC20(_coins[i]).decimals() assert d < 19, "Max 18 decimals for coins" decimals[i] = d - precisions[i] = 10** (18 - d) + precisions[i] = 10 ** (18 - d) # pack fees - packed_fee_params: uint256 = self._pack( + packed_fee_params: uint256 = self._pack_3( [mid_fee, out_fee, fee_gamma] ) # pack liquidity rebalancing params - packed_rebalancing_params: uint256 = self._pack( + packed_rebalancing_params: uint256 = self._pack_3( [allowed_extra_profit, adjustment_step, ma_exp_time] ) diff --git a/contracts/main/CurveTwocryptoOptimized.vy b/contracts/main/CurveTwocryptoOptimized.vy index 91ab1fa6..5a7ce96e 100644 --- a/contracts/main/CurveTwocryptoOptimized.vy +++ b/contracts/main/CurveTwocryptoOptimized.vy @@ -135,7 +135,7 @@ cached_price_oracle: uint256 # <------- Price target given by moving average. cached_xcp_oracle: uint256 # <----------- EMA of totalSupply * virtual_price. last_prices: public(uint256) -last_timestamp: public(uint256[2]) # idx 0 is for prices, idx 1 is for xcp. +last_timestamp: public(uint256) # idx 0 is for prices, idx 1 is for xcp. last_xcp: public(uint256) xcp_ma_time: public(uint256) @@ -247,7 +247,7 @@ def __init__( self.cached_price_scale = initial_price self.cached_price_oracle = initial_price self.last_prices = initial_price - self.last_timestamp = [block.timestamp, block.timestamp] + self.last_timestamp = self._pack_2(block.timestamp, block.timestamp) self.xcp_profit_a = 10**18 self.xcp_ma_time = 62324 # <--------- 12 hours default on contract start. @@ -638,7 +638,7 @@ def remove_liquidity( xp: uint256[N_COINS] = self.xp(self.balances, self.cached_price_scale) self.last_xcp = isqrt(xp[0] * xp[1]) - last_timestamp: uint256[2] = self.last_timestamp + last_timestamp: uint256[2] = self._unpack_2(self.last_timestamp) if last_timestamp[1] < block.timestamp: cached_xcp_oracle: uint256 = self.cached_xcp_oracle @@ -659,7 +659,7 @@ def remove_liquidity( last_timestamp[1] = block.timestamp # Pack and store timestamps: - self.last_timestamp = last_timestamp + self.last_timestamp = self._pack_2(last_timestamp[0], last_timestamp[1]) return withdraw_amounts @@ -730,7 +730,7 @@ def remove_liquidity_one_coin( @internal @pure -def _pack(x: uint256[3]) -> uint256: +def _pack_3(x: uint256[3]) -> uint256: """ @notice Packs 3 integers with values <= 10**18 into a uint256 @param x The uint256[3] to pack @@ -741,7 +741,7 @@ def _pack(x: uint256[3]) -> uint256: @internal @pure -def _unpack(_packed: uint256) -> uint256[3]: +def _unpack_3(_packed: uint256) -> uint256[3]: """ @notice Unpacks a uint256 into 3 integers (values must be <= 10**18) @param val The uint256 to unpack @@ -754,6 +754,20 @@ def _unpack(_packed: uint256) -> uint256[3]: ] +@pure +@internal +def _pack_2(p1: uint256, p2: uint256) -> uint256: + assert p1 < 2**128 + assert p2 < 2**128 + return p1 | (p2 << 128) + + +@pure +@internal +def _unpack_2(packed: uint256) -> uint256[2]: + return [packed & (2**128 - 1), packed >> 128] + + # ---------------------- AMM Internal Functions ------------------------------- @@ -849,7 +863,7 @@ def tweak_price( price_oracle: uint256 = self.cached_price_oracle last_prices: uint256 = self.last_prices price_scale: uint256 = self.cached_price_scale - rebalancing_params: uint256[3] = self._unpack(self.packed_rebalancing_params) + rebalancing_params: uint256[3] = self._unpack_3(self.packed_rebalancing_params) # Contains: allowed_extra_profit, adjustment_step, ma_time. -----^ total_supply: uint256 = self.totalSupply @@ -858,7 +872,7 @@ def tweak_price( # ----------------------- Update Oracles if needed ----------------------- - last_timestamp: uint256[2] = self.last_timestamp + last_timestamp: uint256[2] = self._unpack_2(self.last_timestamp) alpha: uint256 = 0 if last_timestamp[0] < block.timestamp: # 0th index is for price_oracle. @@ -914,7 +928,7 @@ def tweak_price( # Pack and store timestamps: last_timestamp[1] = block.timestamp - self.last_timestamp = last_timestamp + self.last_timestamp = self._pack_2(last_timestamp[0], last_timestamp[1]) # `price_oracle` is used further on to calculate its vector distance from # price_scale. This distance is used to calculate the amount of adjustment @@ -1211,7 +1225,7 @@ def _A_gamma() -> uint256[2]: @view def _fee(xp: uint256[N_COINS]) -> uint256: - fee_params: uint256[3] = self._unpack(self.packed_fee_params) + fee_params: uint256[3] = self._unpack_3(self.packed_fee_params) f: uint256 = xp[0] + xp[1] f = fee_params[2] * 10**18 / ( fee_params[2] + 10**18 - @@ -1308,7 +1322,7 @@ def _calc_withdraw_one_coin( xp_imprecise: uint256[N_COINS] = xp xp_correction: uint256 = xp[i] * N_COINS * token_amount / token_supply - fee: uint256 = self._unpack(self.packed_fee_params)[1] # <- self.out_fee. + fee: uint256 = self._unpack_3(self.packed_fee_params)[1] # <- self.out_fee. if xp_correction < xp_imprecise[i]: xp_imprecise[i] -= xp_correction @@ -1508,13 +1522,13 @@ def internal_price_oracle() -> uint256: """ price_oracle: uint256 = self.cached_price_oracle price_scale: uint256 = self.cached_price_scale - last_prices_timestamp: uint256 = self.last_timestamp[0] + last_prices_timestamp: uint256 = self._unpack_2(self.last_timestamp)[0] if last_prices_timestamp < block.timestamp: # <------------ Update moving # average if needed. last_prices: uint256 = self.last_prices - ma_time: uint256 = self._unpack(self.packed_rebalancing_params)[2] + ma_time: uint256 = self._unpack_3(self.packed_rebalancing_params)[2] alpha: uint256 = MATH.wad_exp( -convert( (block.timestamp - last_prices_timestamp) * 10**18 / ma_time, @@ -1644,7 +1658,7 @@ def xcp_oracle() -> uint256: @return uint256 Oracle value of xcp. """ - last_prices_timestamp: uint256 = self.last_timestamp[1] + last_prices_timestamp: uint256 = self._unpack_2(self.last_timestamp)[1] cached_xcp_oracle: uint256 = self.cached_xcp_oracle if last_prices_timestamp < block.timestamp: @@ -1747,7 +1761,7 @@ def mid_fee() -> uint256: @notice Returns the current mid fee @return uint256 mid_fee value. """ - return self._unpack(self.packed_fee_params)[0] + return self._unpack_3(self.packed_fee_params)[0] @view @@ -1757,7 +1771,7 @@ def out_fee() -> uint256: @notice Returns the current out fee @return uint256 out_fee value. """ - return self._unpack(self.packed_fee_params)[1] + return self._unpack_3(self.packed_fee_params)[1] @view @@ -1767,7 +1781,7 @@ def fee_gamma() -> uint256: @notice Returns the current fee gamma @return uint256 fee_gamma value. """ - return self._unpack(self.packed_fee_params)[2] + return self._unpack_3(self.packed_fee_params)[2] @view @@ -1777,7 +1791,7 @@ def allowed_extra_profit() -> uint256: @notice Returns the current allowed extra profit @return uint256 allowed_extra_profit value. """ - return self._unpack(self.packed_rebalancing_params)[0] + return self._unpack_3(self.packed_rebalancing_params)[0] @view @@ -1787,7 +1801,7 @@ def adjustment_step() -> uint256: @notice Returns the current adjustment step @return uint256 adjustment_step value. """ - return self._unpack(self.packed_rebalancing_params)[1] + return self._unpack_3(self.packed_rebalancing_params)[1] @view @@ -1799,7 +1813,7 @@ def ma_time() -> uint256: One can expect off-by-one errors here. @return uint256 ma_time value. """ - return self._unpack(self.packed_rebalancing_params)[2] * 694 / 1000 + return self._unpack_3(self.packed_rebalancing_params)[2] * 694 / 1000 @view @@ -1937,7 +1951,7 @@ def apply_new_parameters( new_out_fee: uint256 = _new_out_fee new_fee_gamma: uint256 = _new_fee_gamma - current_fee_params: uint256[3] = self._unpack(self.packed_fee_params) + current_fee_params: uint256[3] = self._unpack_3(self.packed_fee_params) if new_out_fee < MAX_FEE + 1: assert new_out_fee > MIN_FEE - 1 # dev: fee is out of range @@ -1953,7 +1967,7 @@ def apply_new_parameters( else: new_fee_gamma = current_fee_params[2] - self.packed_fee_params = self._pack([new_mid_fee, new_out_fee, new_fee_gamma]) + self.packed_fee_params = self._pack_3([new_mid_fee, new_out_fee, new_fee_gamma]) # ----------------- Set liquidity rebalancing parameters ----------------- @@ -1961,7 +1975,7 @@ def apply_new_parameters( new_adjustment_step: uint256 = _new_adjustment_step new_ma_time: uint256 = _new_ma_time - current_rebalancing_params: uint256[3] = self._unpack(self.packed_rebalancing_params) + current_rebalancing_params: uint256[3] = self._unpack_3(self.packed_rebalancing_params) if new_allowed_extra_profit > 10**18: new_allowed_extra_profit = current_rebalancing_params[0] @@ -1974,7 +1988,7 @@ def apply_new_parameters( else: new_ma_time = current_rebalancing_params[2] - self.packed_rebalancing_params = self._pack( + self.packed_rebalancing_params = self._pack_3( [new_allowed_extra_profit, new_adjustment_step, new_ma_time] ) diff --git a/tests/unitary/math/test_get_y.py b/tests/unitary/math/test_get_y.py index cefbab13..789bd10e 100644 --- a/tests/unitary/math/test_get_y.py +++ b/tests/unitary/math/test_get_y.py @@ -104,12 +104,9 @@ def calculate_F_by_y0(y0): def test_get_y_revert(math_contract): a = 1723894848 gamma = 24009999997600 - x = [ - 112497148627520223862735198942112, - 112327102289152450435452075003508 - ] + x = [112497148627520223862735198942112, 112327102289152450435452075003508] D = 224824250915890636214130540882688 i = 0 - - y_out = math_contract.newton_y(a, gamma, x, D, i) - y_out = math_contract.get_y(a, gamma, x, D, i) + + math_contract.newton_y(a, gamma, x, D, i) + math_contract.get_y(a, gamma, x, D, i) diff --git a/tests/unitary/math/test_packing.py b/tests/unitary/math/test_packing.py new file mode 100644 index 00000000..e263ff63 --- /dev/null +++ b/tests/unitary/math/test_packing.py @@ -0,0 +1,29 @@ +import boa +from boa.test import strategy +from hypothesis import given, settings + + +@given(val=strategy("uint256[3]", max_value=10**18)) +@settings(max_examples=10000, deadline=None) +def test_pack_unpack_three_integers(swap, twocrypto_factory, val): + + for contract in [swap, twocrypto_factory]: + packed = contract.internal._pack_3(val) + unpacked = swap.internal._unpack_3(packed) # swap unpacks + for i in range(3): + assert unpacked[i] == val[i] + + +@given(val=strategy("uint256[2]", max_value=2**128)) +@settings(max_examples=10000, deadline=None) +def test_pack_unpack_2_integers(swap, val): + + if max(val) >= 2**128: + with boa.reverts(): + swap.internal._pack_2(val[0], val[1]) + return + + packed = swap.internal._pack_2(val[0], val[1]) + unpacked = swap.internal._unpack_2(packed) # swap unpacks + for i in range(2): + assert unpacked[i] == val[i] From 52c03cb688253085f01fa94d631311729b5cb7cc Mon Sep 17 00:00:00 2001 From: bout3fiddy <11488427+bout3fiddy@users.noreply.github.com> Date: Tue, 14 Nov 2023 14:29:46 +0100 Subject: [PATCH 3/4] remove asserts because packed2 values are timestamps so no need to do gas consuming checks --- contracts/main/CurveTwocryptoOptimized.vy | 2 -- 1 file changed, 2 deletions(-) diff --git a/contracts/main/CurveTwocryptoOptimized.vy b/contracts/main/CurveTwocryptoOptimized.vy index 5a7ce96e..101ac0a3 100644 --- a/contracts/main/CurveTwocryptoOptimized.vy +++ b/contracts/main/CurveTwocryptoOptimized.vy @@ -757,8 +757,6 @@ def _unpack_3(_packed: uint256) -> uint256[3]: @pure @internal def _pack_2(p1: uint256, p2: uint256) -> uint256: - assert p1 < 2**128 - assert p2 < 2**128 return p1 | (p2 << 128) From 407fd0f75c789ac48a4c34f7419e8b6628955e58 Mon Sep 17 00:00:00 2001 From: bout3fiddy <11488427+bout3fiddy@users.noreply.github.com> Date: Tue, 14 Nov 2023 16:43:41 +0100 Subject: [PATCH 4/4] claim fees before remove liq one --- contracts/main/CurveTwocryptoOptimized.vy | 8 ++--- tests/fixtures/accounts.py | 7 ++++ tests/unitary/math/test_packing.py | 8 +---- tests/unitary/pool/test_admin_fee.py | 40 +++++++++++++++++++++++ 4 files changed, 52 insertions(+), 11 deletions(-) create mode 100644 tests/unitary/pool/test_admin_fee.py diff --git a/contracts/main/CurveTwocryptoOptimized.vy b/contracts/main/CurveTwocryptoOptimized.vy index 101ac0a3..297e3fca 100644 --- a/contracts/main/CurveTwocryptoOptimized.vy +++ b/contracts/main/CurveTwocryptoOptimized.vy @@ -461,6 +461,8 @@ def add_liquidity( @return uint256 Amount of LP tokens received by the `receiver """ + self._claim_admin_fees() # <--------- Auto-claim admin fees occasionally. + A_gamma: uint256[2] = self._A_gamma() xp: uint256[N_COINS] = self.balances amountsp: uint256[N_COINS] = empty(uint256[N_COINS]) @@ -564,8 +566,6 @@ def add_liquidity( price_scale ) - self._claim_admin_fees() # <--------- Auto-claim admin fees occasionally. - return d_token @@ -683,6 +683,8 @@ def remove_liquidity_one_coin( @return Amount of tokens at index i received by the `receiver` """ + self._claim_admin_fees() # <--------- Auto-claim admin fees occasionally. + A_gamma: uint256[2] = self._A_gamma() dy: uint256 = 0 @@ -720,8 +722,6 @@ def remove_liquidity_one_coin( msg.sender, token_amount, i, dy, approx_fee, packed_price_scale ) - self._claim_admin_fees() # <--------- Auto-claim admin fees occasionally. - return dy diff --git a/tests/fixtures/accounts.py b/tests/fixtures/accounts.py index 278e8ca1..3099db89 100644 --- a/tests/fixtures/accounts.py +++ b/tests/fixtures/accounts.py @@ -37,6 +37,13 @@ def user(): return acc +@pytest.fixture(scope="module") +def user_b(): + acc = boa.env.generate_address() + boa.env.set_balance(acc, 10**25) + return acc + + @pytest.fixture(scope="module") def users(): accs = [i() for i in [boa.env.generate_address] * 10] diff --git a/tests/unitary/math/test_packing.py b/tests/unitary/math/test_packing.py index e263ff63..64de9215 100644 --- a/tests/unitary/math/test_packing.py +++ b/tests/unitary/math/test_packing.py @@ -1,4 +1,3 @@ -import boa from boa.test import strategy from hypothesis import given, settings @@ -14,15 +13,10 @@ def test_pack_unpack_three_integers(swap, twocrypto_factory, val): assert unpacked[i] == val[i] -@given(val=strategy("uint256[2]", max_value=2**128)) +@given(val=strategy("uint256[2]", max_value=2**128 - 1)) @settings(max_examples=10000, deadline=None) def test_pack_unpack_2_integers(swap, val): - if max(val) >= 2**128: - with boa.reverts(): - swap.internal._pack_2(val[0], val[1]) - return - packed = swap.internal._pack_2(val[0], val[1]) unpacked = swap.internal._unpack_2(packed) # swap unpacks for i in range(2): diff --git a/tests/unitary/pool/test_admin_fee.py b/tests/unitary/pool/test_admin_fee.py new file mode 100644 index 00000000..c0f4ec4c --- /dev/null +++ b/tests/unitary/pool/test_admin_fee.py @@ -0,0 +1,40 @@ +import boa + +from tests.fixtures.pool import INITIAL_PRICES +from tests.utils.tokens import mint_for_testing + + +def test_admin_fee_after_deposit(swap, coins, fee_receiver, user, user_b): + quantities = [10**42 // p for p in INITIAL_PRICES] + + for coin, q in zip(coins, quantities): + for u in [user, user_b]: + mint_for_testing(coin, u, q) + with boa.env.prank(u): + coin.approve(swap, 2**256 - 1) + + split_quantities = [quantities[0] // 100, quantities[1] // 100] + + with boa.env.prank(user): + swap.add_liquidity(split_quantities, 0) + + with boa.env.prank(user_b): + for _ in range(100): + before = coins[1].balanceOf(user_b) + swap.exchange(0, 1, split_quantities[0] // 100, 0) + after = coins[1].balanceOf(user_b) + to_swap = after - before + swap.exchange(1, 0, to_swap, 0) + + balances = [swap.balances(i) for i in range(2)] + print("Balance of the pool: " + str(balances[0]) + ", " + str(balances[1])) + + ratio = 0.0001 + split_quantities = [int(balances[0] * ratio), int(balances[1] * ratio)] + with boa.env.prank(user): + swap.add_liquidity(split_quantities, 0) + + print("FEES 0: " + str(coins[0].balanceOf(fee_receiver))) + print("FEES 1: " + str(coins[1].balanceOf(fee_receiver))) + + return swap