Hold received SP yield when total deposited amount is low #419
Conversation
To make it easy to copy paste output from invariant tests and debug. Also to be able to fix specific test cases without relying on randomness, and make sure those ones pass.
| uint256 totalBoldDepositsAfter = amount + totalBoldDepositsBefore; | ||
| if (totalBoldDepositsBefore < DECIMAL_PRECISION && totalBoldDepositsAfter >= DECIMAL_PRECISION) { | ||
| v.depositorPendingYield = | ||
| (v.pendingYield + SP_YIELD_SPLIT * v.pendingInterest / 1e18) * amount / totalBoldDepositsAfter; |
There was a problem hiding this comment.
Shouldn't we take into account the user's existing deposit, i.e. add v.boldDeposit to amount? Also v.boldYield if they're not claiming.
There was a problem hiding this comment.
Of course we'd also have to add v.boldYield to totalBoldDepositsAfter.
There was a problem hiding this comment.
Thanks for fixing this too!
| uint256 yieldGainsPendingCached = yieldGainsPending; | ||
| if (yieldGainsPendingCached > 0 && totalBoldDepositsCached >= DECIMAL_PRECISION) { | ||
| _updateYieldRewardsSum(yieldGainsPendingCached, totalBoldDepositsCached); | ||
| } |
There was a problem hiding this comment.
We must do the same in withdrawFromSP(). Reason: a depositor could have enough pending yield gains to put total deposits over the threshold of DECIMAL_PRECISION. Remember that yield gains are deposited if they're not claimed.
There was a problem hiding this comment.
Ah, I see you already fixed it, thanks!
contracts/src/StabilityPool.sol
Outdated
| if (totalBoldDepositsCached == 0 || _boldYield == 0) { | ||
|
|
||
| // When total deposits is very small, B is not updated. In this case, the BOLD issued can not be obtained by later | ||
| // depositors - it is missed out on, and remains in the balance of the SP. |
There was a problem hiding this comment.
Comment is outdated now. By that, I mean that the BOLD is not missed out on, and can be obtained by later depositor.
| @@ -584,11 +612,11 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents { | |||
|
|
|||
| Snapshots memory snapshots = depositSnapshots[_depositor]; | |||
|
|
|||
| uint256 pendingSPYield = activePool.calcPendingSPYield(); | |||
| uint256 pendingSPYield = activePool.calcPendingSPYield() + yieldGainsPending; | |||
There was a problem hiding this comment.
When there are pending yield gains, how are they awarded to depositors?
This function seems to calculate as if all pending gains were immediately awarded pro-rata, including pending yield gains.
But in reality I guess some pending yield gains might not be, right (depending on the SP size). We don't really know when they'll be rewarded. Still, it's fine as an estimate IMO.
Also, I guess and IIRC if there are pending yield gains and a depositor makes a large enough deposit, he receives all those gains, right?
There was a problem hiding this comment.
Sorry, Rick, I missed this comment.
You are right, but I think it’s now fixed by the addition of totalBoldDeposits >= DECIMAL_PRECISION condition below. Isnt’t it?
We track those yields in a new state var
yieldGainsPending, and we assimilate them toyieldGainsOwedonce thetotalBoldDepositsreaches 1 BOLD (1e18).Closes #317
It also prevents losses of yield when the SP is empty.