Fix: linear accrual not overpassing maturity #1842
Conversation
lemunozm
added
I2-bug
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## fix/linear-pricing #1842 +/- ##
======================================================
- Coverage 48.48% 46.78% -1.71%
======================================================
Files 170 170
Lines 13398 13385 -13
======================================================
- Hits 6496 6262 -234
- Misses 6902 7123 +221 ☔ View full report in Codecov by Sentry. |
pallets/loans/src/entities/loans.rs
Outdated
| @@ -359,7 +359,8 @@ impl<T: Config> ActiveLoan<T> { | |||
| inner.adjust(Adjustment::Increase(amount.balance()?))? | |||
| } | |||
| ActivePricing::External(inner) => { | |||
| inner.adjust(Adjustment::Increase(amount.external()?), Zero::zero())? | |||
| let when = T::Time::now(); | |||
There was a problem hiding this comment.
I guess maturity not being passed already is checked by the ensure_can_borrw()?
There was a problem hiding this comment.
But not in repay()
| // We must never overpass madurity date | ||
| assert_eq!(active_loan.maturity_date(), settlement_price_updated); |
There was a problem hiding this comment.
Could we also check, that the linear pricing actually clamps with giving the notional baack?
|
BTW, I did the fix quite bad 😆. I'm redoing the fix implementation |
lemunozm
force-pushed
the
fix/linear-pricing-maturity
branch
from
d14ca3e to
78b83b8
Compare
lemunozm
force-pushed
the
fix/linear-pricing-maturity
branch
from
78b83b8 to
01d0200
Compare
| (min(price_last_updated, maturity), price), | ||
| (maturity, self.info.notional), | ||
| T::Time::now(), | ||
| min(T::Time::now(), maturity), |
There was a problem hiding this comment.
I've re-done the implementation. We do not need to modify the settlement_price_updated. Actually, we want to know when the last repayment was done. Instead, we can trick just the call to y_coord_in_rect() to never exceed maturity, which is less invasive
|
Your migration should work without any change after rebasing @wischli 🤞🏻 |
| NOTIONAL | ||
| ); | ||
| }); | ||
| } |
There was a problem hiding this comment.
I would still like to test the case where
last_updated_price and T::Time::now() are both equal to maturity. I think that is an edge case worth testing
There was a problem hiding this comment.
It's tested currently
There was a problem hiding this comment.
Well, it is repaid with one DAY of overdue, and the expected value is notional
Description
slack thread
We need to avoid computing points in the line of linear accrual if the time overpass the maturity time, instead, we want to clamp the value to
notional(which is the same when the time is just maturity)