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(YAUDIT-COVE-6): correct yearn vault v2 preview functions #302

Merged
merged 7 commits into from
Mar 28, 2024

Conversation

penandlim
Copy link
Collaborator

@penandlim penandlim commented Mar 27, 2024

Important

Coverage is currently disabled for this library due to forge limitations. TODO: Once the fix PR is merged, foundry-rs/foundry#7510 coverage should be re-enabled.

Describe your changes

Yearn Vault V2's deposit estimations cannot be made accurately with pricePerShare.
Therefore we need to copy the logic used Vault.vy for converting new asset to shares into the router contract for more accurate preview methods.

Relevant: https://github.com/yAudit/cove-report/issues/6

Checklist before requesting a review

  • Title follows conventional commits style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • Newly added functions follow Check-effects-interaction
  • Gas usage has been minimized (ex. Storage variable access is minimized)

Copy link

github-actions bot commented Mar 27, 2024

Changes to gas cost

Generated at commit: 5bbc05bb30955ca91c1fabc77589410b8d5b8bf6, compared to commit: 3a40ac31939e547b43e64f855d6e7f1b54ee9501

🧾 Summary (10% most significant diffs)

Contract Method Avg (+/-) %
Yearn4626RouterExt previewDeposits
previewMints
previewRedeems
+28,814 ❌
+36,058 ❌
+26,195 ❌
+111.05%
+192.65%
+100.49%

Full diff report 👇
Contract Deployment Cost (+/-) Method Min (+/-) % Avg (+/-) % Median (+/-) % Max (+/-) % # Calls (+/-)
Yearn4626RouterExt 3,453,286 (-20,322) approve
deposit
multicall
previewDeposits
previewMints
previewRedeems
previewWithdraws
pullToken
redeemFromRouter
redeemStakeDaoGauge
redeemVaultV2
selfPermit
withdrawFromRouter
25,150 (0)
101,172 (-31,729)
48,249 (-29)
673 (0)
627 (0)
672 (0)
628 (0)
30,866 (-109)
82,727 (0)
492,095 (+22)
132,876 (-517,822)
14,844 (-29)
140,463 (+22)
0.00%
-23.87%
-0.06%
0.00%
0.00%
0.00%
0.00%
-0.35%
0.00%
+0.00%
-79.58%
-0.19%
+0.02%
52,294 (+16,158)
134,748 (+1,847)
425,084 (-34)
54,761 (+28,814)
54,775 (+36,058)
52,261 (+26,195)
37,167 (+13,402)
30,866 (-109)
146,255 (+19,276)
496,132 (+22)
359,052 (-353,742)
36,940 (-29)
142,812 (+22)
+44.71%
+1.39%
-0.01%
+111.05%
+192.65%
+100.49%
+56.39%
-0.35%
+15.18%
+0.00%
-49.63%
-0.08%
+0.02%
52,198 (+26,583)
121,743 (-11,158)
331,282 (-93)
55,372 (+28,514)
55,722 (+47,532)
52,921 (+26,561)
37,387 (+16,805)
30,866 (-109)
146,654 (-24)
496,132 (+22)
132,904 (-594,942)
36,940 (-29)
142,812 (+22)
+103.78%
-8.40%
-0.03%
+106.17%
+580.37%
+100.76%
+81.65%
-0.35%
-0.02%
+0.00%
-81.74%
-0.08%
+0.02%
52,663 (+513)
162,387 (+29,486)
895,723 (+22)
55,372 (+6,455)
55,722 (+6,458)
54,979 (+6,117)
66,387 (+17,469)
30,866 (-109)
151,532 (0)
500,169 (+22)
759,862 (+22)
59,037 (-29)
145,161 (+22)
+0.98%
+22.19%
+0.00%
+13.20%
+13.11%
+12.52%
+35.71%
-0.35%
0.00%
+0.00%
+0.00%
-0.05%
+0.02%
1,029 (+1,024)
1,028 (+1,027)
3 (0)
261 (+255)
262 (+255)
262 (+255)
264 (+256)
1 (0)
516 (+513)
2 (0)
516 (+513)
2 (0)
2 (0)
YSDRewardsGauge 3,884,210 (0) deposit
redeem
2,732 (0)
8,327 (0)
0.00%
0.00%
193,008 (-27)
134,016 (-38)
-0.01%
-0.03%
178,853 (0)
126,910 (0)
0.00%
0.00%
450,543 (0)
242,620 (0)
0.00%
0.00%
2,048 (0)
1,024 (0)
YearnGaugeStrategy 1,817,865 (0) deposit
report
withdraw
30,717 (0)
58,515 (0)
74,620 (0)
0.00%
0.00%
0.00%
291,882 (+6)
302,960 (+2)
176,060 (+31)
+0.00%
+0.00%
+0.02%
308,482 (0)
69,644 (0)
201,040 (0)
0.00%
0.00%
0.00%
447,268 (0)
914,862 (+10)
256,348 (0)
0.00%
+0.00%
0.00%
7,938 (0)
5,890 (0)
1,792 (0)
DYFIRedeemer 2,139,607 (0) massRedeem 30,408 (0) 0.00% 441,232 (-65) -0.01% 440,099 (0) 0.00% 689,094 (0) 0.00% 1,545 (0)

Copy link

codecov bot commented Mar 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.56%. Comparing base (3a40ac3) to head (4f3fc77).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #302      +/-   ##
==========================================
- Coverage   97.57%   97.56%   -0.01%     
==========================================
  Files          19       19              
  Lines         988      987       -1     
  Branches      203      202       -1     
==========================================
- Hits          964      963       -1     
  Misses          1        1              
  Partials       23       23              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@penandlim penandlim changed the title fix(YAUDIT-COVE-6): correct yearn vault v2 deposit and withdraws fix(YAUDIT-COVE-6): correct yearn vault v2 preview functions Mar 27, 2024
Copy link

Slither report

THIS CHECKLIST IS NOT COMPLETE. Use --show-ignored-findings to show all the results.
Summary

@penandlim penandlim marked this pull request as ready for review March 28, 2024 20:17
@penandlim penandlim merged commit 70a6a84 into master Mar 28, 2024
9 checks passed
@penandlim penandlim deleted the john/yaudit-cove-6 branch March 28, 2024 20:47
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.

3 participants