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

Use preview functions for precise accounting #96

Merged

Conversation

trmid
Copy link
Member

@trmid trmid commented Mar 25, 2024

Summary

convertToAssets and convertToShares are not guaranteed to use current onchain conditions to calculate conversions and may use some approximation to return more of an "average" prize. Yield vaults that implement these as average price oracles may behave poorly with the prize vault when these functions are used for precise accounting functions.

To address this, usage of these functions have been replaced with the yield vault preview functions instead to provide precise accounting based on current onchain conditions. However, this comes with new challenges since these preview functions may revert in some scenarios, but must be used in the prize vault's maxDeposit, maxWithdraw, etc. functions, which MUST not revert. The following changes have been made to allow the usage of these preview functions:

  • New function added: totalPreciseAssets() (returns the total assets using current rates from the yield vault previewRedeem function)
  • New function added: _tryGetTotalPreciseAssets() (Attempts to fetch the totalPreciseAssets, but catches any reversions and indicates that the call was unsuccessful. This function is used in the maxDeposit, maxWithdraw, etc. functions so they can return zero instead of reverting)
  • totalAssets() is no longer used for any accounting functions besides convertToAssets and convertToShares, since all three of these functions are treated as "approximations" by the spec.

Copy link

linear bot commented Mar 25, 2024

Copy link

LCOV of commit 48999e1 during coverage #518

Summary coverage rate:
  lines......: 99.5% (213 of 214 lines)
  functions..: 100.0% (59 of 59 functions)
  branches...: no data found

Files changed coverage rate:
                              |Lines       |Functions  |Branches    
  Filename                    |Rate     Num|Rate    Num|Rate     Num
  ==================================================================
  src/PrizeVault.sol          |99.5%    184| 100%    48|    -      0

Copy link
Contributor

@asselstine asselstine left a comment

Choose a reason for hiding this comment

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

The revert suppression is unfortunately, but the logic makes sense to me

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.

None yet

2 participants