-
Notifications
You must be signed in to change notification settings - Fork 45
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 #103
fix: deposit flow #103
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but probably should be gated on yearn/yearn-vaults-v3#209
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
src/test/Accounting.t.sol
Outdated
setFees(0, 0); | ||
mintAndDepositIntoStrategy(strategy, _address, _amount); | ||
|
||
uint256 toLoose = _amount; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean toLose
? The amount that will be lost -> amount to lose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* chore: bump api version * test: fix invariant test * feat: max deposit (#99) * feat: max uint deposit * fix: invariant test * fix: rebase * fix: dont max mint * fix: uint 256 Co-authored-by: FP <[email protected]> * fix: uint 256 Co-authored-by: FP <[email protected]> * test: check balances --------- Co-authored-by: FP <[email protected]> * chore: add statemind audit * chore: match abis (#100) * feat: update name (#101) * chore: update domain separator (#102) * chore: update domain seperator * feat: dont cache domain * chore: test runs * fix: comment Co-authored-by: spalen0 <[email protected]> * fix: comment Co-authored-by: spalen0 <[email protected]> * fix: permit cast (#104) * fix: deposit flow (#103) * fix: conversion * test: full loss * feat: add self check to max views * test: balance check * fix: spelling * chore: deployed --------- Co-authored-by: FP <[email protected]> Co-authored-by: spalen0 <[email protected]>
Description
Still allow deposits to work if totalSupply == 0 but totalAssets > 0. Just make PPS == 1.
This can happen due to the profit unlocking if all shares are redeemed while some profit is still unlocking, at the end of the unlock time totalAssets >0 but totalSupply == 0.
Still return 0 in convertToShares if totalSupply > 0 but totalAssets == 0. Meaning a full loss.
Optimize to not cache totalAssets_ until needed
Add the check for share receiver to the maxDeposit/maxMint checks instead. So that the view functions are more accurate.
Update the variable name from
owner
=>receiver
to be more accurateFixes # (issue)
Checklist