-
Notifications
You must be signed in to change notification settings - Fork 2
[Draft] Add stabilityFee parameter for Protocol Stability #105
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
base: main
Are you sure you want to change the base?
Conversation
|
@turbolent, @Gornutz, could you please review the contract part changes of this PR when you get a chance? Thanks in advance! |
turbolent
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, looks good from a Cadence code quality perspective 👍
Didn't review the actual logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually noticed that with the current implementation that we can run into underflow issues if we a have a insuranceRate or stabilityFeeRate that is greater than the interest that's be generated. As we are subtracting that flat out instead of it being a proportional % to the total amount of interest being collected.
For a mental model that the two insuranceRate and stabilityFeeRate's behave very similar to AAVE's reserve rate. With how the fee amount generated is proportional to the interest generated, instead of it being a flat hurdle rate.
For an example with the current implementation and we have the following -
Current 10% Borrow Rate, 0.1% insurance, 5% stability -> 10% - 0.1% - 5% = 4.9%
then with the proportional model
10% Borrow Rate, 0.1% insurance, 5% stability -> 10% * (1 - (0.1% + 5%)) = 9.49%
Then if the borrow Rate was 3%
Current 3% Borrow Rate, 0.1% insurance, 5% stability -> 3% - 0.1% - 5% = -2.1%
then with the proportional model
3% Borrow Rate, 0.1% insurance, 5% stability -> 3% * (1- (0.1% + 5%)) = 2.85%
| let secondsPerYear = 365.25 * 24.0 * 60.0 * 60.0 | ||
| let yearsElapsed = timeElapsed / secondsPerYear | ||
| let stabilityFeeRate = UFix128(self.stabilityFeeRate) | ||
| // Stability amount is a percentage of total credit balance per year | ||
| let stabilityAmount = self.totalCreditBalance * stabilityFeeRate * UFix128(yearsElapsed) | ||
| let stabilityAmountUFix64 = FlowCreditMarketMath.toUFix64RoundDown(stabilityAmount) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be rewritten as - let secondsPerYear = 365.25 * 24.0 * 60.0 * 60.0 let yearsElapsed = timeElapsed / secondsPerYear let interestIncome = self.totalDebitBalance * UFix128(self.currentDebitRate) * UFix128(yearsElapsed) let stabilityAmount = interestIncome * stabilityFeeRate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| let interestIncome = self.totalDebitBalance * UFix128(self.currentDebitRate) * UFix128(yearsElapsed) - this is wrong, because self.currentDebitRate is a value from perSecondInterestRate :
// Converts a yearly interest rate to a per-second multiplication factor (stored in a UFix128 as a fixed point
// number with 18 decimal places). The input to this function will be just the relative annual interest rate
// (e.g. 0.05 for 5% interest), and the result will be the per-second multiplier (e.g. 1.000000000001).
then fixed:
let interestIncome = self.totalDebitBalance * (FlowCreditMarketMath.powUFix128(self.currentDebitRate, time_elapsed) - 1.0)
Also I've found a problem in the method perSecondInterestRate:
let perSecondScaledValue = yearlyRate / 31536000.0 - this is calculated as simple interest, but according to its usage it should be a compound interest
/// Returns the compounded interest index reflecting the passage of time
/// The result is: newIndex = oldIndex * perSecondRate ^ seconds
access(all) view fun compoundInterestIndex(
oldIndex: UFix128,
perSecondRate: UFix128,
elapsedSeconds: UFix64
): UFix128 {
// Exponentiation by squaring on UFix128 for performance and precision
let pow = FlowCreditMarketMath.powUFix128(perSecondRate, elapsedSeconds)
return oldIndex * pow
}
this will be fixed in this task
Co-authored-by: Bastian Müller <bastian@turbolent.com>
…CreditMarket into UlianaAndrukhiv/85-stability
…last_stability_collection script. Added test helpers
| // 2. KinkInterestCurve (and others): reserve factor model | ||
| // Insurance is a percentage of interest income, not a fixed spread | ||
| // Insurance and stability are percentages of interest income, not a fixed spread | ||
| if self.interestCurve.getType() == Type<FlowCreditMarket.FixedRateInterestCurve>() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently the protocol fees can have wildly different values depending on the interest rate curve being used. I'm guessing this is a mistake (from a prior PR), but if it is desired we should clearly document why it is desired, and document how protocolFeeRate and insuranceRate can be interpreted in different ways.
For example, suppose the debit interest rate is 10% and the protocol fee rate is 5%.
- If we have a fixed rate curve, we will calculate
creditRate = 0.1-0.05 = 0.05 = 5%.
- We interpret
protocolFeeRate/insuranceRateas a percentage of a total debit balance
- If we have a different rate curve, we will calculate
creditRate = (0.1)(1-0.05)(debitBalance/creditBalance)
- If
debitBalance/creditBalanceis close to 1, thencreditRate ~= 0.95 = 9.5%. - We interpret
protocolFeeRate/insuranceRateas a percentage of a debit income
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that the protocol fees should be interpreted as a percentage of debit income, based on #105 (review).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also feel like that fact that we need to inspect the InterestCurve's implementation type here indicates that the current InterestCurve is a poor abstraction. This is literally the only usage of InterestCurve in the whole smart contract -- we should be able to use it without breaking that abstraction 😅.
Co-authored-by: Jordan Schalm <jordan.schalm@gmail.com>
NOTE: It is in progress. Tests need to be updated and new ones added according to the implementation.
The contract is not fully updated according to comments
Closes: #85
Context
Implements a new
stabilityFeeRateparameter for theFlowCreditMarketprotocol that enables collecting stability fees from interest income to support stability.The implementation follows the proposed solution from the issue discussion, with a default stability fee rate of
5%.For contributor use:
masterbranchFiles changedin the Github PR explorer