🩹 market: mirror fixed repay rounding in position preview - #10
Conversation
WalkthroughUpdates fixed repayment logic to apply backup fee deduction and additional fixed-point steps when backup supply is constrained; adds a changeset documenting a patch release that the market UI mirrors the position preview rounding. ChangesFixed Repay Position Calculation
🎯 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 docstrings
🧪 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 |
There was a problem hiding this comment.
Code Review
This pull request updates the fixedRepayPosition function to align its rounding logic with fixedRepayAssets for position previews. The changes introduce a calculation for gross yield and adjust the return value to account for backup fees. Feedback was provided to improve consistency and precision by refactoring the logic to use a scaledPrincipal variable, which avoids redundant calculations and unnecessary rounding steps.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10 +/- ##
==========================================
- Coverage 80.75% 80.75% -0.01%
==========================================
Files 46 46
Lines 686 691 +5
Branches 96 96
==========================================
+ Hits 554 558 +4
Misses 128 128
- Partials 4 5 +1 ☔ 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: ac0588070f
ℹ️ 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".
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: 27583557-27d6-41a0-8e89-186169ea6b9e
📒 Files selected for processing (2)
.changeset/kind-walls-write.mdsrc/market/fixedRepayPosition.ts
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: 837c893a-7c0d-4aef-b4fe-93a409c8ae43
📒 Files selected for processing (2)
.changeset/kind-walls-write.mdsrc/market/fixedRepayPosition.ts
| const pos = min(assets + gross - mulWad(gross, backupFeeRate), totalPosition); | ||
| const earned = mulDiv(unassignedEarnings, min(mulDiv(pos, principal, totalPosition), backupSupplied), backupSupplied); | ||
| const repay = pos - earned + mulWad(earned, backupFeeRate); | ||
| if (scaledPrincipal <= backupSupplied && x <= totalPosition) return pos - (repay > assets ? repay - assets : 0n); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Consider documenting the return adjustment logic.
The expression pos - (repay > assets ? repay - assets : 0n) performs a non-trivial adjustment. When repay > assets, this effectively returns pos - repay + assets, which algebraically simplifies to a complex relationship between earned amounts, fees, and the backup fee rate. A brief inline comment explaining the business logic (e.g., "adjust position to account for repayment shortfall") would aid maintainability.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 941f68361a
ℹ️ 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".
| const pos = min(assets + gross - mulWad(gross, backupFeeRate), totalPosition); | ||
| const earned = mulDiv(unassignedEarnings, min(mulDiv(pos, principal, totalPosition), backupSupplied), backupSupplied); | ||
| const repay = pos - earned + mulWad(earned, backupFeeRate); | ||
| if (scaledPrincipal <= backupSupplied && x <= totalPosition) return pos - (repay > assets ? repay - assets : 0n); |
There was a problem hiding this comment.
After this patch's one-shot subtraction, the returned position can still cost more than the caller's asset limit because lowering pos also lowers the earned-yield offset used by fixedRepayAssets. For example, with principal=7743n, fee=408n, borrowed=813061n, supplied=647081n, unassignedEarnings=298005n, backupFeeRate=450351310267204224n, lastAccrual=1n, maturity=1000, timestamp=1, and assets=119n, this branch returns 1900n, but fixedRepayAssets(..., 1900n, 1) is 120n. Please iterate/recompute until the forward calculation is <= assets rather than subtracting only the first observed excess.
Useful? React with 👍 / 👎.
Summary by CodeRabbit
Bug Fixes
Chores