Skip to content

Commit

Permalink
Merge pull request #17 from Unstoppable-DeFi/fix-37
Browse files Browse the repository at this point in the history
fix-37: issue 1 less share to account for potential rounding error
  • Loading branch information
Unstoppable-DeFi authored Aug 8, 2023
2 parents 67dc419 + f0e30a2 commit 27d90e2
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 27 deletions.
3 changes: 2 additions & 1 deletion contracts/margin-dex/Vault.vy
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,8 @@ def _account_for_provide_liquidity(
_token: address, _amount: uint256, _is_safety_module: bool
):
self._update_debt(_token)
shares: uint256 = self._amount_to_lp_shares(_token, _amount, _is_safety_module)
# issue 1 less share to account for potential rounding errors later
shares: uint256 = self._amount_to_lp_shares(_token, _amount, _is_safety_module) - 1
if _is_safety_module:
self.safety_module_lp_total_shares[_token] += shares
self.safety_module_lp_shares[_token][msg.sender] += shares
Expand Down
28 changes: 14 additions & 14 deletions tests/vault/test_liquidity.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def test_first_provide_liquidity_records_user_shares(vault, usdc, owner):

after = vault.base_lp_shares(usdc, owner)

assert after == before + amount * 10**18
assert after == before + amount * 10**18 - 1


def test_first_provide_liquidity_records_total_shares(vault, usdc, owner):
Expand All @@ -43,7 +43,7 @@ def test_first_provide_liquidity_records_total_shares(vault, usdc, owner):

after = vault.base_lp_total_shares(usdc)

assert after == before + amount * 10**18
assert after == before + amount * 10**18 - 1


def test_multiple_provide_record_shares_correctly(vault, usdc, owner, alice):
Expand All @@ -53,16 +53,16 @@ def test_multiple_provide_record_shares_correctly(vault, usdc, owner, alice):

amount = 100 * 10**6
vault.provide_liquidity(usdc, amount, BASE_LP)
assert vault.base_lp_total_shares(usdc) == before_total_shares + amount * 10**18
assert vault.base_lp_shares(usdc, owner) == before_owner_shares + amount * 10**18
assert vault.base_lp_total_shares(usdc) == before_total_shares + amount * 10**18 - 1
assert vault.base_lp_shares(usdc, owner) == before_owner_shares + amount * 10**18 - 1

with boa.env.prank(alice):
amount_2 = 30 * 10**6
vault.provide_liquidity(usdc, amount_2, BASE_LP)

assert vault.base_lp_total_shares(usdc) == before_total_shares + (amount + amount_2) * 10**18
assert vault.base_lp_shares(usdc, owner) == before_owner_shares + amount * 10**18
assert vault.base_lp_shares(usdc, alice) == before_alice_shares + amount_2 * 10**18
assert vault.base_lp_total_shares(usdc) == before_total_shares + (amount + amount_2) * 10**18 - 1 - 1
assert vault.base_lp_shares(usdc, owner) == before_owner_shares + amount * 10**18 - 1
assert vault.base_lp_shares(usdc, alice) == before_alice_shares + amount_2 * 10**18 - 1


def test_shares_to_amount_are_calculated_correctly(vault, usdc):
Expand Down Expand Up @@ -172,8 +172,8 @@ def test_cannot_withdraw_more_than_you_own(vault, usdc, owner, alice):

alice_shares = vault.base_lp_shares(usdc, alice)
alice_amount = vault.lp_shares_to_amount(usdc, alice_shares, BASE_LP)
assert alice_amount == amount
assert alice_shares == amount * 10**18
assert alice_amount == amount - 1
assert alice_shares == amount * 10**18 - 1

with boa.env.prank(alice):
with boa.reverts("cannot withdraw more than you own"):
Expand Down Expand Up @@ -204,13 +204,13 @@ def test_withdraw_liquidity_transfers_tokens(vault, usdc, owner):
vault_balance_before = usdc.balanceOf(vault)
assert vault_balance_before >= amount

vault.withdraw_liquidity(usdc, amount, BASE_LP)
vault.withdraw_liquidity(usdc, amount - 1, BASE_LP)

owner_balance_after = usdc.balanceOf(owner)
vault_balance_after = usdc.balanceOf(vault)

assert vault_balance_after == vault_balance_before - amount
assert owner_balance_after == owner_balance_before + amount
assert vault_balance_after == vault_balance_before - amount + 1
assert owner_balance_after == owner_balance_before + amount - 1


def test_available_liquidty_is_calculated_correctly(vault, usdc):
Expand Down Expand Up @@ -274,6 +274,6 @@ def test_init_total_by_distributing_trading_fee_works(vault, usdc, owner):

after = vault.base_lp_shares(usdc, owner)

assert after == before + amount * 10**18
assert after == before + amount * 10**18 - 1

assert vault.lp_shares_to_amount(usdc, after, BASE_LP) == 100 * 10**6 + 400000
assert vault.lp_shares_to_amount(usdc, after, BASE_LP) == 100 * 10**6 + 400000 - 1
25 changes: 13 additions & 12 deletions tests/vault/test_liquidity_safety_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def test_first_provide_liquidity_records_user_shares(vault, usdc, owner):

after = vault.safety_module_lp_shares(usdc, owner)

assert after == amount * 10**18
assert after == amount * 10**18 - 1

def test_first_provide_liquidity_records_total_shares(vault, usdc, owner):
before = vault.safety_module_lp_total_shares(usdc)
Expand All @@ -43,24 +43,24 @@ def test_first_provide_liquidity_records_total_shares(vault, usdc, owner):

after = vault.safety_module_lp_total_shares(usdc)

assert after == amount * 10**18
assert after == amount * 10**18 - 1

def test_multiple_provide_record_shares_correctly(vault, usdc, owner, alice):
before = vault.safety_module_lp_total_shares(usdc)
assert before == 0

amount = 100 * 10**6
vault.provide_liquidity(usdc, amount, SAFETY_MODULE)
assert vault.safety_module_lp_total_shares(usdc) == amount * 10**18
assert vault.safety_module_lp_shares(usdc, owner) == amount * 10**18
assert vault.safety_module_lp_total_shares(usdc) == amount * 10**18 - 1
assert vault.safety_module_lp_shares(usdc, owner) == amount * 10**18 - 1

with boa.env.prank(alice):
amount_2 = 30 * 10**6
vault.provide_liquidity(usdc, amount_2, SAFETY_MODULE)

assert vault.safety_module_lp_total_shares(usdc) == (amount + amount_2) * 10**18
assert vault.safety_module_lp_shares(usdc, owner) == amount * 10**18
assert vault.safety_module_lp_shares(usdc, alice) == amount_2 * 10**18
assert vault.safety_module_lp_total_shares(usdc) == (amount + amount_2) * 10**18 - 1 - 1
assert vault.safety_module_lp_shares(usdc, owner) == amount * 10**18 - 1
assert vault.safety_module_lp_shares(usdc, alice) == amount_2 * 10**18 - 1


def test_shares_to_amount_are_calculated_correctly(vault, usdc):
Expand Down Expand Up @@ -152,8 +152,8 @@ def test_cannot_withdraw_more_than_you_own(vault, usdc, owner, alice):
with boa.env.prank(alice):
vault.provide_liquidity(usdc, amount, SAFETY_MODULE)

assert vault.safety_module_lp_total_amount(usdc) == 2 * amount
assert vault.safety_module_lp_total_shares(usdc) == 2 * amount * 10**18
assert vault.safety_module_lp_total_amount(usdc) == 2 * amount
assert vault.safety_module_lp_total_shares(usdc) == 2 * amount * 10**18 - 1 - 1

with boa.reverts("cannot withdraw more than you own"):
vault.withdraw_liquidity(usdc, amount+1, SAFETY_MODULE)
Expand All @@ -174,6 +174,7 @@ def test_provide_liquidity_transfers_tokens(vault, usdc, owner):
assert owner_balance_after == owner_balance_before - amount
assert vault_balance_after == vault_balance_before + amount


def test_withdraw_liquidity_transfers_tokens(vault, usdc, owner):
amount = 100 * 10**6
vault.provide_liquidity(usdc, amount, SAFETY_MODULE)
Expand All @@ -182,13 +183,13 @@ def test_withdraw_liquidity_transfers_tokens(vault, usdc, owner):
vault_balance_before = usdc.balanceOf(vault)
assert vault_balance_before >= amount

vault.withdraw_liquidity(usdc, amount, SAFETY_MODULE)
vault.withdraw_liquidity(usdc, amount-1, SAFETY_MODULE)

owner_balance_after = usdc.balanceOf(owner)
vault_balance_after = usdc.balanceOf(vault)

assert vault_balance_after == vault_balance_before - amount
assert owner_balance_after == owner_balance_before + amount
assert vault_balance_after == vault_balance_before - amount + 1
assert owner_balance_after == owner_balance_before + amount - 1


def test_available_liquidty_is_calculated_correctly(vault, usdc):
Expand Down

0 comments on commit 27d90e2

Please sign in to comment.