Skip to content

Commit

Permalink
Merge pull request #438 from liquity/sp-underflow
Browse files Browse the repository at this point in the history 
Fix arithmetic underflow in the StabilityPool
  • Loading branch information
@bingen
bingen authored Oct 15, 2024
2 parents 1d37745 + 4e4b95f commit 61fddeb
Show file tree
Hide file tree
Showing 6 changed files with 1,760 additions and 88 deletions.
133 changes: 66 additions & 67 deletions contracts/src/StabilityPool.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents {
// Error trackers for the error correction in the offset calculation
uint256 public lastCollError_Offset;
uint256 public lastBoldLossError_Offset;
uint256 public lastBoldLossError_TotalDeposits;

// Error tracker fror the error correction in the BOLD reward calculation
uint256 public lastYieldError;
Expand Down Expand Up @@ -408,7 +409,7 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents {
uint256 yieldPerUnitStaked = yieldNumerator / totalBoldDepositsCached;
lastYieldError = yieldNumerator - yieldPerUnitStaked * totalBoldDepositsCached;

uint256 marginalYieldGain = yieldPerUnitStaked * P;
uint256 marginalYieldGain = yieldPerUnitStaked * (P - 1);
epochToScaleToB[currentEpoch][currentScale] = epochToScaleToB[currentEpoch][currentScale] + marginalYieldGain;

emit B_Updated(epochToScaleToB[currentEpoch][currentScale], currentEpoch, currentScale);
Expand All @@ -426,10 +427,7 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents {
uint256 totalBold = totalBoldDeposits; // cached to save an SLOAD
if (totalBold == 0 || _debtToOffset == 0) return;

(uint256 collGainPerUnitStaked, uint256 boldLossPerUnitStaked) =
_computeCollRewardsPerUnitStaked(_collToAdd, _debtToOffset, totalBold);

_updateCollRewardSumAndProduct(collGainPerUnitStaked, boldLossPerUnitStaked); // updates S and P
_updateCollRewardSumAndProduct(_collToAdd, _debtToOffset, totalBold); // updates S and P

_moveOffsetCollAndDebt(_collToAdd, _debtToOffset);
}
Expand All @@ -438,7 +436,7 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents {

function _computeCollRewardsPerUnitStaked(uint256 _collToAdd, uint256 _debtToOffset, uint256 _totalBoldDeposits)
internal
returns (uint256 collGainPerUnitStaked, uint256 boldLossPerUnitStaked)
returns (uint256 collGainPerUnitStaked, uint256 boldLossPerUnitStaked, uint256 newLastBoldLossErrorOffset)
{
/*
* Compute the Bold and Coll rewards. Uses a "feedback" error correction, to keep
Expand All @@ -457,33 +455,39 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents {
if (_debtToOffset == _totalBoldDeposits) {
boldLossPerUnitStaked = DECIMAL_PRECISION; // When the Pool depletes to 0, so does each deposit
lastBoldLossError_Offset = 0;
lastBoldLossError_TotalDeposits = _totalBoldDeposits;
} else {
uint256 boldLossNumerator = _debtToOffset * DECIMAL_PRECISION - lastBoldLossError_Offset;
uint256 boldLossNumerator = _debtToOffset * DECIMAL_PRECISION;
/*
* Add 1 to make error in quotient positive. We want "slightly too much" Bold loss,
* which ensures the error in any given compoundedBoldDeposit favors the Stability Pool.
*/
boldLossPerUnitStaked = boldLossNumerator / _totalBoldDeposits + 1;
lastBoldLossError_Offset = boldLossPerUnitStaked * _totalBoldDeposits - boldLossNumerator;
newLastBoldLossErrorOffset = boldLossPerUnitStaked * _totalBoldDeposits - boldLossNumerator;
}

collGainPerUnitStaked = collNumerator / _totalBoldDeposits;
lastCollError_Offset = collNumerator - collGainPerUnitStaked * _totalBoldDeposits;

return (collGainPerUnitStaked, boldLossPerUnitStaked);
return (collGainPerUnitStaked, boldLossPerUnitStaked, newLastBoldLossErrorOffset);
}

// Update the Stability Pool reward sum S and product P
function _updateCollRewardSumAndProduct(uint256 _collGainPerUnitStaked, uint256 _boldLossPerUnitStaked) internal {
function _updateCollRewardSumAndProduct(uint256 _collToAdd, uint256 _debtToOffset, uint256 _totalBoldDeposits)
internal
{
(uint256 collGainPerUnitStaked, uint256 boldLossPerUnitStaked, uint256 newLastBoldLossErrorOffset) =
_computeCollRewardsPerUnitStaked(_collToAdd, _debtToOffset, _totalBoldDeposits);

uint256 currentP = P;
uint256 newP;

assert(_boldLossPerUnitStaked <= DECIMAL_PRECISION);
assert(boldLossPerUnitStaked <= DECIMAL_PRECISION);
/*
* The newProductFactor is the factor by which to change all deposits, due to the depletion of Stability Pool Bold in the liquidation.
* We make the product factor 0 if there was a pool-emptying. Otherwise, it is (1 - boldLossPerUnitStaked)
*/
uint256 newProductFactor = uint256(DECIMAL_PRECISION) - _boldLossPerUnitStaked;
uint256 newProductFactor = uint256(DECIMAL_PRECISION) - boldLossPerUnitStaked;

uint128 currentScaleCached = currentScale;
uint128 currentEpochCached = currentEpoch;
Expand All @@ -496,7 +500,7 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents {
*
* Since S corresponds to Coll gain, and P to deposit loss, we update S first.
*/
uint256 marginalCollGain = _collGainPerUnitStaked * currentP;
uint256 marginalCollGain = collGainPerUnitStaked * (currentP - 1);
uint256 newS = currentS + marginalCollGain;
epochToScaleToS[currentEpochCached][currentScaleCached] = newS;
emit S_Updated(newS, currentEpochCached, currentScaleCached);
Expand Down Expand Up @@ -524,8 +528,14 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents {
emit ScaleUpdated(currentScale);
// If there's no scale change and no pool-emptying, just do a standard multiplication
} else {
newP = currentP * newProductFactor / DECIMAL_PRECISION;
uint256 errorFactor;
if (lastBoldLossError_Offset > 0) {
errorFactor = lastBoldLossError_Offset * newProductFactor / lastBoldLossError_TotalDeposits;
}
newP = (currentP * newProductFactor + errorFactor) / DECIMAL_PRECISION;
}
lastBoldLossError_Offset = newLastBoldLossErrorOffset;
lastBoldLossError_TotalDeposits = _totalBoldDeposits;

assert(newP > 0);
P = newP;
Expand Down Expand Up @@ -577,8 +587,22 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents {

Snapshots memory snapshots = depositSnapshots[_depositor];

uint256 collGain = _getCollGainFromSnapshots(initialDeposit, snapshots);
return collGain;
/*
* Grab the sum 'S' from the epoch at which the stake was made. The Coll gain may span up to one scale change.
* If it does, the second portion of the Coll gain is scaled by 1e9.
* If the gain spans no scale change, the second portion will be 0.
*/
uint128 epochSnapshot = snapshots.epoch;
uint128 scaleSnapshot = snapshots.scale;
uint256 S_Snapshot = snapshots.S;
uint256 P_Snapshot = snapshots.P;

uint256 firstPortion = epochToScaleToS[epochSnapshot][scaleSnapshot] - S_Snapshot;
uint256 secondPortion = epochToScaleToS[epochSnapshot][scaleSnapshot + 1] / SCALE_FACTOR;

uint256 collGain = initialDeposit * (firstPortion + secondPortion) / P_Snapshot / DECIMAL_PRECISION;

return LiquityMath._min(collGain, collBalance);
}

function getDepositorYieldGain(address _depositor) public view override returns (uint256) {
Expand All @@ -588,8 +612,22 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents {

Snapshots memory snapshots = depositSnapshots[_depositor];

uint256 yieldGain = _getYieldGainFromSnapshots(initialDeposit, snapshots);
return yieldGain;
/*
* Grab the sum 'B' from the epoch at which the stake was made. The Bold gain may span up to one scale change.
* If it does, the second portion of the Bold gain is scaled by 1e9.
* If the gain spans no scale change, the second portion will be 0.
*/
uint128 epochSnapshot = snapshots.epoch;
uint128 scaleSnapshot = snapshots.scale;
uint256 B_Snapshot = snapshots.B;
uint256 P_Snapshot = snapshots.P;

uint256 firstPortion = epochToScaleToB[epochSnapshot][scaleSnapshot] - B_Snapshot;
uint256 secondPortion = epochToScaleToB[epochSnapshot][scaleSnapshot + 1] / SCALE_FACTOR;

uint256 yieldGain = initialDeposit * (firstPortion + secondPortion) / P_Snapshot / DECIMAL_PRECISION;

return LiquityMath._min(yieldGain, yieldGainsOwed);
}

function getDepositorYieldGainWithPending(address _depositor) external view override returns (uint256) {
Expand All @@ -600,13 +638,14 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents {
Snapshots memory snapshots = depositSnapshots[_depositor];

uint256 pendingSPYield = activePool.calcPendingSPYield() + yieldGainsPending;
uint256 newYieldGainsOwed = yieldGainsOwed + (totalBoldDeposits >= DECIMAL_PRECISION ? pendingSPYield : 0);
uint256 firstPortionPending;
uint256 secondPortionPending;

if (pendingSPYield > 0 && snapshots.epoch == currentEpoch && totalBoldDeposits >= DECIMAL_PRECISION) {
uint256 yieldNumerator = pendingSPYield * DECIMAL_PRECISION + lastYieldError;
uint256 yieldPerUnitStaked = yieldNumerator / totalBoldDeposits;
uint256 marginalYieldGain = yieldPerUnitStaked * P;
uint256 marginalYieldGain = yieldPerUnitStaked * (P - 1);

if (currentScale == snapshots.scale) firstPortionPending = marginalYieldGain;
else if (currentScale == snapshots.scale + 1) secondPortionPending = marginalYieldGain;
Expand All @@ -616,53 +655,9 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents {
uint256 secondPortion =
(epochToScaleToB[snapshots.epoch][snapshots.scale + 1] + secondPortionPending) / SCALE_FACTOR;

return initialDeposit * (firstPortion + secondPortion) / snapshots.P / DECIMAL_PRECISION;
}
uint256 yieldGain = initialDeposit * (firstPortion + secondPortion) / snapshots.P / DECIMAL_PRECISION;

function _getCollGainFromSnapshots(uint256 initialDeposit, Snapshots memory snapshots)
internal
view
returns (uint256)
{
/*
* Grab the sum 'S' from the epoch at which the stake was made. The Coll gain may span up to one scale change.
* If it does, the second portion of the Coll gain is scaled by 1e9.
* If the gain spans no scale change, the second portion will be 0.
*/
uint128 epochSnapshot = snapshots.epoch;
uint128 scaleSnapshot = snapshots.scale;
uint256 S_Snapshot = snapshots.S;
uint256 P_Snapshot = snapshots.P;

uint256 firstPortion = epochToScaleToS[epochSnapshot][scaleSnapshot] - S_Snapshot;
uint256 secondPortion = epochToScaleToS[epochSnapshot][scaleSnapshot + 1] / SCALE_FACTOR;

uint256 collGain = initialDeposit * (firstPortion + secondPortion) / P_Snapshot / DECIMAL_PRECISION;

return collGain;
}

function _getYieldGainFromSnapshots(uint256 initialDeposit, Snapshots memory snapshots)
internal
view
returns (uint256)
{
/*
* Grab the sum 'B' from the epoch at which the stake was made. The Bold gain may span up to one scale change.
* If it does, the second portion of the Bold gain is scaled by 1e9.
* If the gain spans no scale change, the second portion will be 0.
*/
uint128 epochSnapshot = snapshots.epoch;
uint128 scaleSnapshot = snapshots.scale;
uint256 B_Snapshot = snapshots.B;
uint256 P_Snapshot = snapshots.P;

uint256 firstPortion = epochToScaleToB[epochSnapshot][scaleSnapshot] - B_Snapshot;
uint256 secondPortion = epochToScaleToB[epochSnapshot][scaleSnapshot + 1] / SCALE_FACTOR;

uint256 yieldGain = initialDeposit * (firstPortion + secondPortion) / P_Snapshot / DECIMAL_PRECISION;

return yieldGain;
return LiquityMath._min(yieldGain, newYieldGainsOwed);
}

// --- Compounded deposit ---
Expand Down Expand Up @@ -697,14 +692,18 @@ contract StabilityPool is LiquityBase, IStabilityPool, IStabilityPoolEvents {
uint256 compoundedStake;
uint128 scaleDiff = currentScale - scaleSnapshot;

// To make sure rouning errors favour the system, we use P - 1 if P decreased
uint256 cachedP = P;
uint256 currentPToUse = cachedP != snapshot_P ? cachedP - 1 : cachedP;

/* Compute the compounded stake. If a scale change in P was made during the stake's lifetime,
* account for it. If more than one scale change was made, then the stake has decreased by a factor of
* at least 1e-9 -- so return 0.
*/
if (scaleDiff == 0) {
compoundedStake = initialStake * P / snapshot_P;
compoundedStake = initialStake * currentPToUse / snapshot_P;
} else if (scaleDiff == 1) {
compoundedStake = initialStake * P / snapshot_P / SCALE_FACTOR;
compoundedStake = initialStake * currentPToUse / snapshot_P / SCALE_FACTOR;
} else {
// if scaleDiff >= 2
compoundedStake = 0;
Expand Down
Loading

0 comments on commit 61fddeb

Please sign in to comment.