-
Notifications
You must be signed in to change notification settings - Fork 27
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 fee #38
Fix deposit fee #38
Conversation
Add timelock
…morpho-blue-metamorpho into fix/deposit-fee
3ae1742
to
09bf26a
Compare
09bf26a
to
fc73b4c
Compare
…rpho into fix/deposit-fee
modifier syncLastTotalAssets() { | ||
_; | ||
|
||
lastTotalAssets = totalAssets(); |
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.
Now we trigger 2 times totalAssets
for each tx. It will cost a lot...
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.
Unfortunately, it is the only viable solution. Using a timestamp-based solution creates a bug: if 2 users deposit within the same block, lastTotalAssets
is not updated with the last user's deposit, which gets counted as interest in the next fee accrual...
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.
I just pushed an alternative version that saves 2 calls to totalAssets
. This should be a huge gas cost improvement
c4f76b4
to
f058644
Compare
All tests break because they are incomplete: in order for the fee recipient to accrue fee, the vault must realize a performance (it should actually not accrue fee from deposits/withdrawals)