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

Enable tracking the accrued interest #314

Merged
merged 7 commits into from
Nov 14, 2023

Conversation

QGarchery
Copy link
Contributor

Motivated by a remark of @tomrpl, thanks !

Before this PR, the events AccrueFee(uint256 feeShares) and UpdateLastTotalAssets(uint256 newTotalAssets) were emitted at each interaction. Those are not enough to compute the actual yield generated by the vault because:

  • feeShares could be 0, if the vault does not take any performance fee we can't recompute the whole interest generated
  • newTotalAssets takes into account the funds added by the current interaction. Those funds are not part of the interest generated and are difficult to subtract (it depends on each interaction)

To fix this, the event AccrueFee is transformed into AccrueInterest to add 2 fields:

  1. lastTotalAssets, the value of the storage variable of the same name before the interaction. It is provided as a convenience to be able to reconstruct the yield by looking only at events
  2. accruedTotalAssets, the total assets in the vault before the interaction, after having accrued the interest

With those, one can compute (accruedTotalAssets - lastTotalAssets) / lastTotalAssets to compute the yield generated during 2 interactions.

@QGarchery QGarchery self-assigned this Nov 8, 2023
@QGarchery QGarchery requested review from Rubilmax, MerlinEgalite and Jean-Grimal and removed request for a team November 8, 2023 10:17
Copy link
Contributor

@Rubilmax Rubilmax left a comment

Choose a reason for hiding this comment

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

Ouf, missed this one thanks

src/MetaMorpho.sol Outdated Show resolved Hide resolved
@julien-devatom
Copy link
Contributor

lastTotalAssets can be recovered from the previous event, right?

@Rubilmax
Copy link
Contributor

Rubilmax commented Nov 9, 2023

lastTotalAssets can be recovered from the previous event, right?

No because the totalAssets emitted in this event does not include the eventual deposit/withdrawal that happens after

@QGarchery
Copy link
Contributor Author

lastTotalAssets can be recovered from the previous event, right?

No because the totalAssets emitted in this event does not include the eventual deposit/withdrawal that happens after

It can be recovered from the last UpdateTotalAssets event though. So it seems like we could remove lastTotalAssets from the AccrueInterest event. How annoying would it be to compute the yield if we did @julien-devatom ?

@julien-devatom
Copy link
Contributor

Its ok while we can recover it from prev event

@QGarchery
Copy link
Contributor Author

Its ok while we can recover it from prev event

Ok, I'm removing the field lastTotalAssets from AccrueInterest then

@Rubilmax
Copy link
Contributor

Its ok while we can recover it from prev event

Just want to make sure that it's crystal clear: you won't be recovering it from the previous event of the same type: you need to recover it from the previous event of the type UpdateLastTotalAssets @julien-devatom

@MerlinEgalite MerlinEgalite changed the base branch from cantina-review to main November 14, 2023 16:26
@MerlinEgalite MerlinEgalite merged commit adcd1f7 into main Nov 14, 2023
6 checks passed
@MerlinEgalite MerlinEgalite deleted the feat/account-interest-accrued branch November 14, 2023 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants