Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix review 114 - do not allow multiple deposits for one token #3

Merged
merged 1 commit into from
Mar 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions contracts/Vault.vy
Original file line number Diff line number Diff line change
Expand Up @@ -208,17 +208,17 @@ def register_deposit(_token_id: uint256, _amount: uint256):
assert self._is_valid_token_id(_token_id)

position: Position = self.positions[_token_id]
assert position.is_liquidated == False, "position already liquidated"
assert position.amount_deposited == 0, "can only deposit once per token"

position.token_id = _token_id
position.amount_deposited += _amount
position.amount_deposited = _amount

# transfer WETH to self
ERC20(WETH).transferFrom(msg.sender, self, _amount)

# deposit WETH to Alchemix
shares_issued: uint256 = self._deposit_to_alchemist(_amount)
position.shares_owned += shares_issued
position.shares_owned = shares_issued
self.total_shares += shares_issued

self.positions[_token_id] = position
Expand Down
23 changes: 12 additions & 11 deletions tests/test_Vault_deposit.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,24 +58,25 @@ def test_register_records_total_shares(vault, nft, owner, alchemist):
assert after == before + AMOUNT


def test_register_only_once_per_token(vault, nft, owner, weth, alchemist):
weth.approve(vault, 2*AMOUNT)
nft.DEBUG_transferMinter(owner)
nft.mint(owner, EXISTING_TOKEN_ID)
alchemist.eval(f"self.total_value = {2*AMOUNT}")

vault.register_deposit(EXISTING_TOKEN_ID, AMOUNT)

with boa.reverts("can only deposit once per token"):
vault.register_deposit(EXISTING_TOKEN_ID, AMOUNT)


def test_cannot_register_with_invalid_token_id(vault, nft):
assert nft.idToOwner(NON_EXISTING_TOKEN_ID) == pytest.ZERO_ADDRESS

with boa.reverts():
vault.register_deposit(NON_EXISTING_TOKEN_ID, AMOUNT)


def test_cannot_deposit_for_liquidated_token(vault, nft, owner):
nft.DEBUG_transferMinter(owner)
nft.mint(owner, EXISTING_TOKEN_ID)
vault.eval(
f"self.positions[{EXISTING_TOKEN_ID}] = Position({{token_id: {EXISTING_TOKEN_ID}, amount_deposited: {100*10**18}, amount_claimed: 0, shares_owned: {100*10**18}, is_liquidated: True }})"
)

with boa.reverts("position already liquidated"):
vault.register_deposit(EXISTING_TOKEN_ID, AMOUNT)


def test_operator_can_call_deposit(vault, owner, nft, alchemist):
nft.DEBUG_transferMinter(owner)
nft.mint(owner, EXISTING_TOKEN_ID)
Expand Down