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: deposit flow #209

Merged
merged 13 commits into from
Sep 19, 2024
103 changes: 25 additions & 78 deletions contracts/VaultV3.vy
Original file line number Diff line number Diff line change
Expand Up @@ -462,16 +462,17 @@ def _convert_to_shares(assets: uint256, rounding: Rounding) -> uint256:
return assets

total_supply: uint256 = self._total_supply()

# if total_supply is 0, price_per_share is 1
if total_supply == 0:
return assets

total_assets: uint256 = self._total_assets()

# if total_Supply > 0 but total_assets == 0, price_per_share = 0
if total_assets == 0:
# if total_assets and total_supply is 0, price_per_share is 1
if total_supply == 0:
return assets
else:
# Else if total_supply > 0 price_per_share is 0
return 0

return 0

numerator: uint256 = assets * total_supply
shares: uint256 = numerator / total_assets
if rounding == Rounding.ROUND_UP and numerator % total_assets != 0:
Expand Down Expand Up @@ -504,31 +505,6 @@ def _issue_shares(shares: uint256, recipient: address):

log Transfer(empty(address), recipient, shares)

@internal
def _issue_shares_for_amount(amount: uint256, recipient: address) -> uint256:
"""
Issues shares that are worth 'amount' in the underlying token (asset).
WARNING: this takes into account that any new assets have been summed
to total_assets (otherwise pps will go down).
"""
total_supply: uint256 = self._total_supply()
total_assets: uint256 = self._total_assets()
new_shares: uint256 = 0

# If no supply PPS = 1.
if total_supply == 0:
new_shares = amount
elif total_assets > amount:
new_shares = amount * total_supply / (total_assets - amount)

# We don't make the function revert
if new_shares == 0:
return 0

self._issue_shares(new_shares, recipient)

return new_shares

## ERC4626 ##
@view
@internal
Expand Down Expand Up @@ -662,52 +638,16 @@ def _max_withdraw(
return max_assets

@internal
def _deposit(sender: address, recipient: address, assets: uint256) -> uint256:
def _deposit(sender: address, recipient: address, assets: uint256, shares: uint256):
"""
Used for `deposit` calls to transfer the amount of `asset` to the vault,
issue the corresponding shares to the `recipient` and update all needed
Used for `deposit` and `mint` calls to transfer the amount of `asset` to the vault,
issue the corresponding `shares` to the `recipient` and update all needed
vault accounting.
"""
assert self.shutdown == False # dev: shutdown
wavey0x marked this conversation as resolved.
Show resolved Hide resolved

amount: uint256 = assets
# Deposit all if sent with max uint
if amount == max_value(uint256):
amount = ERC20(self.asset).balanceOf(msg.sender)

assert amount <= self._max_deposit(recipient), "exceed deposit limit"

# Transfer the tokens to the vault first.
self._erc20_safe_transfer_from(self.asset, msg.sender, self, amount)
# Record the change in total assets.
self.total_idle += amount

# Issue the corresponding shares for amount.
shares: uint256 = self._issue_shares_for_amount(amount, recipient)

assert shares > 0, "cannot mint zero"

log Deposit(sender, recipient, amount, shares)

if self.auto_allocate:
self._update_debt(self.default_queue[0], max_value(uint256), 0)

return shares

@internal
def _mint(sender: address, recipient: address, shares: uint256) -> uint256:
"""
Used for `mint` calls to issue the corresponding shares to the `recipient`,
transfer the amount of `asset` to the vault, and update all needed vault
accounting.
"""
assert self.shutdown == False # dev: shutdown
# Get corresponding amount of assets.
assets: uint256 = self._convert_to_assets(shares, Rounding.ROUND_UP)

assert assets > 0, "cannot deposit zero"
assert assets <= self._max_deposit(recipient), "exceed deposit limit"

assert assets > 0, "cannot deposit zero"
assert shares > 0, "cannot mint zero"

# Transfer the tokens to the vault first.
self._erc20_safe_transfer_from(self.asset, msg.sender, self, assets)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use the input parameter sender instead of msg.sender?
Below in the event Deposit, parameter sender is used.

I would either remove the sender parameter and go with msg.sender in both places or use the sender address for both, to limit confusion.

Copy link
Collaborator Author

@Schlagonia Schlagonia Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point

I believe it is actually cheaper to use msg.sender than using a memory variable. And clearer so will remove the variable

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

# Record the change in total assets.
Expand All @@ -721,8 +661,6 @@ def _mint(sender: address, recipient: address, shares: uint256) -> uint256:
if self.auto_allocate:
self._update_debt(self.default_queue[0], max_value(uint256), 0)

return assets

@view
@internal
def _assess_share_of_unrealised_losses(strategy: address, strategy_current_debt: uint256, assets_needed: uint256) -> uint256:
Expand Down Expand Up @@ -1849,7 +1787,14 @@ def deposit(assets: uint256, receiver: address) -> uint256:
@param receiver The address to receive the shares.
@return The amount of shares minted.
"""
return self._deposit(msg.sender, receiver, assets)
amount: uint256 = assets
# Deposit all if sent with max uint
if amount == max_value(uint256):
amount = ERC20(self.asset).balanceOf(msg.sender)

shares: uint256 = self._convert_to_shares(amount, Rounding.ROUND_DOWN)
self._deposit(msg.sender, receiver, amount, shares)
return shares

@external
@nonreentrant("lock")
Expand All @@ -1860,7 +1805,9 @@ def mint(shares: uint256, receiver: address) -> uint256:
@param receiver The address to receive the shares.
@return The amount of assets deposited.
"""
return self._mint(msg.sender, receiver, shares)
assets: uint256 = self._convert_to_assets(shares, Rounding.ROUND_UP)
self._deposit(msg.sender, receiver, assets, shares)
return assets

@external
@nonreentrant("lock")
Expand Down
4 changes: 2 additions & 2 deletions foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ remappings = [
]
fs_permissions = [{ access = "read", path = "./"}]

match_contract = "VaultERC4626StdTest"
#match_path = "./foundry_tests/tests/*"
#match_contract = "VaultERC4626StdTest"
match_path = "./foundry_tests/tests/*"
ffi = true

[fuzz]
Expand Down
Loading
Loading