✨ auditor: add optional haircuts per market#11
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughThe PR adds optional per-market haircuts to the auditor module, allowing collateral and debt to be adjusted by a per-market factor. The core ChangesAuditor Optional Haircuts Per Market
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces optional "haircuts" per market to the auditor's liquidity and limit calculations, allowing for more granular risk management. The changes update core functions like accountLiquidity, borrowLimit, and withdrawLimit to incorporate these factors into collateral and debt adjustments. Feedback identifies a critical ReferenceError due to an undefined default variable, a potential division-by-zero vulnerability in normalizeCollateral when a haircut is 100%, and recommends using case-insensitive lookups for market addresses to ensure consistent behavior.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 94b5b796-39f0-4c78-b939-a01f0dbcec38
📒 Files selected for processing (6)
.changeset/brave-hoops-live.mdsrc/auditor/accountLiquidity.tssrc/auditor/borrowLimit.tssrc/auditor/healthFactor.tssrc/auditor/withdrawLimit.tstest/auditor.test.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 663a4af24b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11 +/- ##
==========================================
+ Coverage 80.75% 81.36% +0.61%
==========================================
Files 46 46
Lines 691 730 +39
Branches 96 109 +13
==========================================
+ Hits 558 594 +36
- Misses 128 129 +1
- Partials 5 7 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be0d59ccc2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
| adjDebt += adjustDebt(totalDebt, usdPrice, baseUnit, adjustFactor); | ||
| const debt = adjustDebt(totalDebt, usdPrice, baseUnit, adjustFactor); | ||
| adjDebt += haircut ? (haircut === WAD && debt !== 0n ? MAX_UINT256 : divWadUp(debt, WAD - haircut)) : debt; |
There was a problem hiding this comment.
When a market has a 100% haircut and no borrow balance or fixed borrow positions, debt is 0n, so this falls through to divWadUp(0n, WAD - haircut) with a zero denominator. That makes otherwise valid collateral-only or unused markets crash in accountLiquidity (and therefore healthFactor, borrowLimit, and withdrawLimit) as soon as callers supply { [market]: WAD }; zero debt under a full haircut should contribute 0n rather than divide by zero.
Useful? React with 👍 / 👎.
| } | ||
| adjDebt += adjustDebt(totalDebt, usdPrice, baseUnit, adjustFactor); | ||
| const debt = adjustDebt(totalDebt, usdPrice, baseUnit, adjustFactor); | ||
| adjDebt += haircut ? (haircut === WAD && debt !== 0n ? MAX_UINT256 : divWadUp(debt, WAD - haircut)) : debt; |
There was a problem hiding this comment.
When a fully haircut market has any debt, this substitutes the division-by-zero result with MAX_UINT256, but the rest of these helpers use unbounded bigint sums for collateral across markets. With enough collateral in other markets (for example two large collateral markets whose adjusted collateral totals above MAX_UINT256), healthFactor can still report a healthy account and borrowLimit can allow more borrowing even though debt divided by zero should make the account insolvent; represent this case as an explicit infinite/insolvent state rather than a finite additive value.
Useful? React with 👍 / 👎.
| ) { | ||
| if (haircut === WAD) return adjustedCollateral ? MAX_UINT256 : 0n; | ||
| return divWad( | ||
| mulDiv(haircut ? divWad(adjustedCollateral, WAD - haircut) : adjustedCollateral, baseUnit, usdPrice), |
There was a problem hiding this comment.
When a partially haircut market has prices or adjust factors that cause truncation, this floors the unhaircuted adjusted amount before converting back to raw assets, but accountLiquidity later floors the haircuted collateral contribution again. In those cases withdrawLimit can return a raw amount whose removal exceeds adjCollateral - minAdjCollateral by one adjusted unit and leaves the account just below the requested target health factor (for example with a 50% haircut and non-1e18 price/adjust factor), so the haircut path needs to round conservatively or verify the post-withdrawal contribution.
Useful? React with 👍 / 👎.
co-authored-by: itofarina <farinalvaro@gmail.com>
1 participant
Summary by CodeRabbit