FLO-2: setInsuranceRate() and setStabilityFeeRate() Retroactively Applies New Rates and Fails to Update Interest Rates#254
Conversation
| // NOTE: | ||
| // We intentionally do not use `equalWithinVariance` with `defaultUFixVariance` here. | ||
| // The default variance is designed for deterministic math, but insurance collection | ||
| // depends on block timestamps, which can differ slightly between test runs. | ||
| // A larger, time-aware tolerance is required. | ||
| let expectedCollectedInsuranceAmount= expectedCollectedInsuranceAmountAfterPhase1 + expectedCollectedInsuranceAmountAfterPhase2 // 5.25854589 + 10.51709179 | ||
| diff = expectedCollectedInsuranceAmount > insuranceAfterPhase2 | ||
| ? expectedCollectedInsuranceAmount - insuranceAfterPhase2 | ||
| : insuranceAfterPhase2 - expectedCollectedInsuranceAmount | ||
|
|
||
| Test.assert(diff < tolerance, message: "Insurance collected should be around \(expectedCollectedInsuranceAmount) but current \(insuranceAfterPhase2)") |
There was a problem hiding this comment.
This code was updated to use the equalWithinVariance helper with a custom variance in the refactoring PR
jordanschalm
left a comment
There was a problem hiding this comment.
Looks good.
Reviewing this I realized there is some tension between our solutions for FLO-1 and FLO-2:
- For FLO-1, our solution to avoid failing to collect protocol fees is to skip collection until there is sufficient reserves.
- For FLO-2, we add a collection at the time of rate change, and rely on this collection to ensure fees due under the old rate are collected first.
If we do a rate change when there is insufficient reserves to pay the fees, then we will skip the collection and will over/under-collect the fees for the previous period under the new rate.
I don't think we need to solve this right now, but I think it is worth documenting as a known edge case. @UlyanaAndrukhiv Do you mind documenting this alongside the protocol fee logic, either in this PR, or the one for FLO-1?
|
|
||
| // ----------------------------------------------------------------------------- | ||
| /// Verifies that stability fee collection remains correct when the stability | ||
| /// fee rate changes between collection periods. |
There was a problem hiding this comment.
| /// fee rate changes between collection periods. | |
| /// fee rate changes between collection periods. Rate changes must trigger fee collections, | |
| /// so that all fees due under the previous rate are collected before the new rate comes into effect. |
|
|
||
| // ----------------------------------------------------------------------------- | ||
| /// Verifies that insurance collection remains correct when the insurance | ||
| /// rate changes between collection periods. |
There was a problem hiding this comment.
| /// rate changes between collection periods. | |
| /// rate changes between collection periods. Rate changes must trigger fee collections, | |
| /// so that all fees due under the previous rate are collected before the new rate comes into effect. |
Closes: #211
Context
This PR forces fee collection and interest rate updates before applying the new rate, ensuring:
updateInterestRates()is triggered socurrentCreditRateremains consistentNew tests were added, and some tests were updated according to these changes.