FLO-12: Fee Calculation Diverges From Rate Allocation Formula#288
FLO-12: Fee Calculation Diverges From Rate Allocation Formula#288
Conversation
…ator Key changes: merge separate insurance/stability timestamps and inline fee math into one collectProtocolFees() method on TokenState that accumulates both fees; _withdrawInsurance/_withdrawStability now just read and reset the accumulators; tests updated to match the new shared-timestamp behavior and require KinkCurve where FixedCurve can't produce positive fees with imbalanced credit/debit.
…calculation-diverges-from-rate-allocation-formula # Conflicts: # cadence/tests/insurance_collection_formula_test.cdc # cadence/tests/insurance_collection_test.cdc # cadence/tests/stability_collection_formula_test.cdc # cadence/tests/stability_collection_test.cdc
…bility_midPeriodRateChange: The rewritten tests used setInterestCurveFixed at low utilization (~5%), where creditIncome >> debitIncome → protocolFeeIncome = 0 with the new formula.
| // setup borrower with FLOW collateral | ||
| // With 0.8 CF and 1.3 target health: 1000 FLOW collateral allows borrowing ~615 MOET | ||
| // borrow = (collateral * price * CF) / targetHealth = (1000 * 1.0 * 0.8) / 1.3 ≈ 615.38 | ||
| // With 0.8 CF and 1.3 target health: 15000 FLOW collateral allows borrowing ~9231 MOET |
There was a problem hiding this comment.
why using a different number? is there any problem with the 1000 FLOW collateral example? otherwise, I think it will be useful to show the different result using the same input number.
There was a problem hiding this comment.
because InterestCurveFixed are used.
According to changes protocolFee = debitIncome - creditIncome
with previous values:
totalСreditBalance = 10_000 MOET
totalDebitBalance = 1_000 Flow
Ratio MOET:FLOW = 1:1
DebitYearlyRate = 0.1
CreditYEarlyRate ~ 0.088 = (1 + 0.85*0.1/31_557_600)^31_557_600
debitIncome = 100, creditIncome = 880 => debitIncome < creditIncome => protocolFee == 0 => stability and insurance fee amounts ==0
It's possible another fix: change InterestCurveFixed into InterestCurveKink like it were done at https://github.com/onflow/FlowALP/pull/288/changes#r2994137845
| // setup borrower with FLOW collateral | ||
| // With 0.8 CF and 1.3 target health: 1000 FLOW collateral allows borrowing ~615 MOET | ||
| // borrow = (collateral * price * CF) / targetHealth = (1000 * 1.0 * 0.8) / 1.3 ≈ 615.38 | ||
| // With 0.8 CF and 1.3 target health: 15000 FLOW collateral allows borrowing ~9231 MOET |
There was a problem hiding this comment.
Does the new number in this tests make a difference? If not, it's better keep the original number, so that it's easier to compare the difference in results.
| #### Stability Fund | ||
|
|
||
| The stability fund provides **flexible funding for MOET stability operations**. A percentage of lender interest income is collected and held in native token vaults (FLOW, USDC, etc.), with each token type having its own separate vault. These funds can be withdrawn by the governance committee via `withdrawStabilityFund()` to improve the stability of MOET at their discretion—whether by adding liquidity to specific pools, repurchasing MOET if it's trading under peg compared to the underlying basket of assets, or other stabilization strategies. All withdrawals emit `StabilityFundWithdrawn` events for public accountability and transparency. | ||
| The stability fund provides **flexible funding for MOET stability operations**. A percentage of protocol spread income is collected and held in native token vaults (FLOW, USDC, etc.), with each token type having its own separate vault. These funds can be withdrawn by the governance committee via `withdrawStabilityFund()` to improve the stability of MOET at their discretion—whether by adding liquidity to specific pools, repurchasing MOET if it's trading under peg compared to the underlying basket of assets, or other stabilization strategies. All withdrawals emit `StabilityFundWithdrawn` events for public accountability and transparency. |
There was a problem hiding this comment.
Better to add some explanation on the "spread"
| The stability fund provides **flexible funding for MOET stability operations**. A percentage of protocol spread income is collected and held in native token vaults (FLOW, USDC, etc.), with each token type having its own separate vault. These funds can be withdrawn by the governance committee via `withdrawStabilityFund()` to improve the stability of MOET at their discretion—whether by adding liquidity to specific pools, repurchasing MOET if it's trading under peg compared to the underlying basket of assets, or other stabilization strategies. All withdrawals emit `StabilityFundWithdrawn` events for public accountability and transparency. | |
| The stability fund provides **flexible funding for MOET stability operations**. A percentage of protocol spread income (borrower income − lender cost) is collected and held in native token vaults (FLOW, USDC, etc.), with each token type having its own separate vault. These funds can be withdrawn by the governance committee via `withdrawStabilityFund()` to improve the stability of MOET at their discretion—whether by adding liquidity to specific pools, repurchasing MOET if it's trading under peg compared to the underlying basket of assets, or other stabilization strategies. All withdrawals emit `StabilityFundWithdrawn` events for public accountability and transparency. |
| For **FixedCurve** (used for stable assets like MOET): | ||
| ``` | ||
| creditRate = debitRate * (1 - protocolFeeRate) | ||
| currentCreditRate = 1.0 + debitRatePerSecond * (1.0 - protocolFeeRate) |
There was a problem hiding this comment.
| currentCreditRate = 1.0 + debitRatePerSecond * (1.0 - protocolFeeRate) | |
| creditRatePerSecond = debitRatePerSecond * (1.0 - protocolFeeRate) | |
| currentCreditRate = 1.0 + creditRatePerSecond |
| protocolFeeRate = insuranceRate + stabilityFeeRate | ||
| protocolFeeAmount = debitIncome * protocolFeeRate | ||
| creditRate = (debitIncome - protocolFeeAmount) / totalCreditBalance | ||
| currentCreditRate = 1.0 + debitRatePerSecond * (1.0 - protocolFeeRate) * totalDebitBalance / totalCreditBalance |
There was a problem hiding this comment.
| currentCreditRate = 1.0 + debitRatePerSecond * (1.0 - protocolFeeRate) * totalDebitBalance / totalCreditBalance | |
| creditRatePerSecond = debitRatePerSecond * (1.0 - protocolFeeRate) * totalDebitBalance / totalCreditBalance | |
| currentCreditRate = 1.0 + creditRatePerSecond |
| /// Accumulates insurance and stability fee income for the elapsed time since the last call. | ||
| /// Updates lastProtocolFeeCollectionTime to the current block timestamp. | ||
| /// Must be called before any balance or rate change to settle fees at current rates. | ||
| access(EImplementation) fun collectProtocolFees() { |
There was a problem hiding this comment.
Technically fees have not been collected but only accumulated or calculated, using "collect" would be a bit confusing.
| access(EImplementation) fun collectProtocolFees() { | |
| access(EImplementation) fun accumulateProtocolFees() { |
|
|
||
| /// Accumulates insurance and stability fee income for the elapsed time since the last call. | ||
| /// Updates lastProtocolFeeCollectionTime to the current block timestamp. | ||
| /// Must be called before any balance or rate change to settle fees at current rates. |
There was a problem hiding this comment.
This is my understanding of the motivation for the changes.
| /// Must be called before any balance or rate change to settle fees at current rates. | |
| /// Must be called before any balance or rate change to settle fees at current rates. | |
| /// Note: This function only accrues fee income and does not withdraw it from the reserve. | |
| /// This is intentional—if the protocol becomes insolvent, fees should remain in the reserve | |
| /// and continue to accumulate until the protocol recovers, rather than being withdrawn. |
| For **FixedCurve** (used for stable assets like MOET): | ||
| ``` | ||
| creditRate = debitRate * (1 - protocolFeeRate) | ||
| currentCreditRate = 1.0 + debitRatePerSecond * (1.0 - protocolFeeRate) |
There was a problem hiding this comment.
| currentCreditRate = 1.0 + debitRatePerSecond * (1.0 - protocolFeeRate) | |
| creditRatePerSecond = debitRatePerSecond * (1.0 - protocolFeeRate) | |
| currentCreditRate = 1.0 + debitRatePerSecond |
| protocolFeeRate = insuranceRate + stabilityFeeRate | ||
| protocolFeeAmount = debitIncome * protocolFeeRate | ||
| creditRate = (debitIncome - protocolFeeAmount) / totalCreditBalance | ||
| currentCreditRate = 1.0 + debitRatePerSecond * (1.0 - protocolFeeRate) * totalDebitBalance / totalCreditBalance |
There was a problem hiding this comment.
| currentCreditRate = 1.0 + debitRatePerSecond * (1.0 - protocolFeeRate) * totalDebitBalance / totalCreditBalance | |
| creditRatePerSecond = debitRatePerSecond * (1.0 - protocolFeeRate) * totalDebitBalance / totalCreditBalance | |
| currentCreditRate = 1.0 + creditRatePerSecond |
| timeElapsed = currentTime - lastProtocolFeeCollectionTime | ||
| debitIncome = totalDebitBalance * (currentDebitRate ^ timeElapsed - 1.0) | ||
| creditIncome = totalCreditBalance * (currentCreditRate ^ timeElapsed - 1.0) |
There was a problem hiding this comment.
| timeElapsed = currentTime - lastProtocolFeeCollectionTime | |
| debitIncome = totalDebitBalance * (currentDebitRate ^ timeElapsed - 1.0) | |
| creditIncome = totalCreditBalance * (currentCreditRate ^ timeElapsed - 1.0) | |
| currentDebitRate = 1.0 + debitRatePerSecond | |
| currentCreditRate = 1.0 + creditRatePerSecond | |
| secondsElapsed = currentTime - lastProtocolFeeCollectionTime | |
| debitIncome = totalDebitBalance * (currentDebitRate ^ secondsElapsed - 1.0) | |
| creditIncome = totalCreditBalance * (currentCreditRate ^ secondsElapsed - 1.0) |
| access(EImplementation) fun setStabilityFeeRate(_ rate: UFix64) | ||
|
|
||
| /// Sets the last stability fee collection timestamp. See getLastStabilityFeeCollectionTime for additional details. | ||
| access(EImplementation) fun setLastStabilityFeeCollectionTime(_ lastStabilityFeeCollectionTime: UFix64) |
There was a problem hiding this comment.
I would suggest adding setLastProtocolFeeCollectionTime
| setInterestCurveFixed(signer: PROTOCOL_ACCOUNT, tokenTypeIdentifier: MOET_TOKEN_IDENTIFIER, yearlyRate: 0.1) | ||
| // set 10% annual debit rate using KinkCurve (slope1=0/slope2=0 → constant rate regardless of | ||
| // utilization), ensuring protocolFeeIncome > 0 at the ~6% utilization of this test setup. | ||
| setInterestCurveKink(signer: PROTOCOL_ACCOUNT, tokenTypeIdentifier: MOET_TOKEN_IDENTIFIER, optimalUtilization: 0.9, baseRate: 0.1, slope1: 0.0, slope2: 0.0) |
There was a problem hiding this comment.
what’s the reason for changing the interest curve in this and other tests?
There was a problem hiding this comment.
test_collectInsurance_success_fullAmount
10_000 MOET credit balance
1_000 FLOW debit balance
for interest curve fixed debit_income < credit_income => protocol_fee = 0 => stability and insurance fee = 0.
| let collectedAmount = finalInsuranceBalance - initialInsuranceBalance | ||
| let collectedInsuranceAmount = finalInsuranceBalance - initialInsuranceBalance | ||
|
|
||
| // collectProtocolFees withdraws both insurance AND stability in one call. |
There was a problem hiding this comment.
From what I can see in the code and docs, collectProtocolFees doesn’t actually withdraw the insurance or stability fees - it just accumulates them. So it seems like the stabilityFundBalance should remain 0, since nothing is withdrawn. Based on that, I would expect amountWithdrawnFromReserves to be equal only to collectedInsuranceAmount.
FlowALP/cadence/contracts/FlowALPModels.cdc
Lines 1538 to 1541 in edd56a9
| let stabilityFundBalance = getStabilityFundBalance(tokenTypeIdentifier: MOET_TOKEN_IDENTIFIER) ?? 0.0 | ||
| let amountWithdrawnFromReserves = reserveBalanceBefore - reserveBalanceAfter | ||
| Test.assertEqual(amountWithdrawnFromReserves, finalInsuranceBalance) | ||
| Test.assertEqual(amountWithdrawnFromReserves, finalInsuranceBalance + stabilityFundBalance) |
|
|
||
| ``` | ||
| protocolFeeRate = insuranceRate + stabilityFeeRate | ||
| debitRatePerSecond = perSecondInterestRate(yearlyRate: debitRate) - 1.0 |
There was a problem hiding this comment.
From my perspective, the line debitRatePerSecond = perSecondInterestRate(yearlyRate: debitRate) - 1.0 is a bit confusing: perSecondInterestRate() represents a per-second multiplication factor which is > 1.0, not a per-second multiplication rate. In this line, we derive a rate by subtracting 1.0 which is correct, but not clear according to naming.
In general, the naming currentDebitRate, currentCreditRate and perSecondInterestRate does not seem appropriate for what they actually represent. Since they are greater than 1.0, they behave more like multiplicative factors rather than rates.
There was a problem hiding this comment.
@zhangchiqing
factor = rate + 1
I propose rename:
currentDebitRate/currentCreditRate -> currentDebitFactor/currentCreditFactor
perSecondInterestRate -> perSecondInterestFactor
Closes: #221
Description
Protocol fee
Protocol fee income was calculated incorrectly in two ways:
The old code computed annual rates first, then converted to per-second:
This diverges from the correct formula because
perSecondInterestRateis non-linear — scaling the annual input is not equivalent to scaling the resulting per-second rate. The fix derives per-second rates symmetrically:The old code took insurance/stability as a flat percentage of total debit income
(debitIncome * rate). The correct formula is a percentage of the spread — the portion of debit income not returned to depositors:Stability and insurance fee
collectProtocolFees() accumulator (commit e5c3929)
Both fee types are now computed in a single
TokenState.collectProtocolFees()method that accumulates intoaccumulatedInsuranceFeeIncome / accumulatedStabilityFeeIncome. It is called before every balance or rate mutation, so fees always settle at the rate that was in effect when they accrued. A singlelastProtocolFeeCollectionTimereplaces the two separate timestamps._withdrawInsuranceand_withdrawStabilitynow simply read and reset the accumulators rather than recomputing the formula.Tests