Skip to content

Commit

Permalink
refactor(RewardStreamerMP): keep track of maxMP instead of
Browse files Browse the repository at this point in the history
`potentialMP`

This commit changes the mechanics to ensure there are no more MP
generated that what's allowed as per max limiting.

Previously we've kept track of `potentialMP` which would decrease as
more MP are generated.

This made verifying certain rules on certora hard and/or impossible.
So we decided to track `maxMP` instead, which only decreases when users
unstake.

This commit also introduces a rule that ensures any accounts MP never
exceed their max mp.

Closes #44
  • Loading branch information
0x-r4bbit committed Oct 10, 2024
1 parent 4ada09e commit 6c94860
Show file tree
Hide file tree
Showing 5 changed files with 177 additions and 168 deletions.
32 changes: 17 additions & 15 deletions .gas-report
Original file line number Diff line number Diff line change
Expand Up @@ -15,31 +15,33 @@
| src/RewardsStreamerMP.sol:RewardsStreamerMP contract | | | | | |
|------------------------------------------------------|-----------------|--------|--------|--------|---------|
| Deployment Cost | Deployment Size | | | | |
| 1105804 | 4983 | | | | |
| 1111690 | 5010 | | | | |
| Function Name | min | avg | median | max | # calls |
| MAX_LOCKING_PERIOD | 228 | 228 | 228 | 228 | 18 |
| MAX_MULTIPLIER | 229 | 229 | 229 | 229 | 24 |
| MAX_MULTIPLIER | 252 | 252 | 252 | 252 | 34 |
| MIN_LOCKING_PERIOD | 229 | 229 | 229 | 229 | 8 |
| SCALE_FACTOR | 251 | 251 | 251 | 251 | 28 |
| accountedRewards | 373 | 963 | 373 | 2373 | 44 |
| getUserInfo | 1553 | 1553 | 1553 | 1553 | 41 |
| potentialMP | 330 | 330 | 330 | 330 | 44 |
| rewardIndex | 373 | 418 | 373 | 2373 | 44 |
| stake | 167821 | 215459 | 228608 | 249401 | 35 |
| totalMP | 352 | 352 | 352 | 352 | 44 |
| totalStaked | 330 | 330 | 330 | 330 | 44 |
| unstake | 75511 | 107650 | 110519 | 134250 | 10 |
| updateGlobalState | 30008 | 69159 | 80335 | 80335 | 10 |
| MP_RATE_PER_YEAR | 231 | 231 | 231 | 231 | 2 |
| SCALE_FACTOR | 251 | 251 | 251 | 251 | 30 |
| accountedRewards | 373 | 921 | 373 | 2373 | 62 |
| getUserInfo | 1576 | 1576 | 1576 | 1576 | 61 |
| rewardIndex | 373 | 405 | 373 | 2373 | 62 |
| stake | 167787 | 215016 | 228586 | 249238 | 43 |
| totalMP | 330 | 330 | 330 | 330 | 62 |
| totalMaxMP | 329 | 329 | 329 | 329 | 62 |
| totalStaked | 373 | 373 | 373 | 373 | 62 |
| unstake | 75511 | 107624 | 110519 | 134250 | 10 |
| updateGlobalState | 30008 | 56733 | 47387 | 80335 | 22 |
| updateUserMP | 34631 | 36857 | 37133 | 37431 | 16 |


| test/mocks/MockToken.sol:MockToken contract | | | | | |
|---------------------------------------------|-----------------|-------|--------|-------|---------|
| Deployment Cost | Deployment Size | | | | |
| 639406 | 3369 | | | | |
| Function Name | min | avg | median | max | # calls |
| approve | 46346 | 46346 | 46346 | 46346 | 120 |
| balanceOf | 561 | 1327 | 561 | 2561 | 201 |
| mint | 51284 | 58124 | 51284 | 68384 | 120 |
| approve | 46346 | 46346 | 46346 | 46346 | 150 |
| balanceOf | 561 | 1321 | 561 | 2561 | 271 |
| mint | 51284 | 58124 | 51284 | 68384 | 150 |
| transfer | 34390 | 48070 | 51490 | 51490 | 10 |


Expand Down
52 changes: 29 additions & 23 deletions .gas-snapshot
Original file line number Diff line number Diff line change
@@ -1,24 +1,30 @@
IntegrationTest:testStake() (gas: 1378213)
IntegrationTest:testStake() (gas: 1375519)
RewardsStreamerTest:testStake() (gas: 869874)
StakeTest:test_StakeMultipleAccounts() (gas: 438756)
StakeTest:test_StakeMultipleAccountsAndRewards() (gas: 586002)
StakeTest:test_StakeMultipleAccountsWithMinLockUp() (gas: 449214)
StakeTest:test_StakeMultipleAccountsWithRandomLockUp() (gas: 470881)
StakeTest:test_StakeOneAccount() (gas: 267795)
StakeTest:test_StakeOneAccountAndRewards() (gas: 415039)
StakeTest:test_StakeOneAccountWithMaxLockUp() (gas: 284120)
StakeTest:test_StakeOneAccountWithMinLockUp() (gas: 284152)
StakeTest:test_StakeOneAccountWithRandomLockUp() (gas: 284196)
UnstakeTest:test_StakeMultipleAccounts() (gas: 438778)
UnstakeTest:test_StakeMultipleAccountsAndRewards() (gas: 586002)
UnstakeTest:test_StakeMultipleAccountsWithMinLockUp() (gas: 449214)
UnstakeTest:test_StakeMultipleAccountsWithRandomLockUp() (gas: 470903)
UnstakeTest:test_StakeOneAccount() (gas: 267795)
UnstakeTest:test_StakeOneAccountAndRewards() (gas: 415061)
UnstakeTest:test_StakeOneAccountWithMaxLockUp() (gas: 284120)
UnstakeTest:test_StakeOneAccountWithMinLockUp() (gas: 284152)
UnstakeTest:test_StakeOneAccountWithRandomLockUp() (gas: 284196)
UnstakeTest:test_UnstakeMultipleAccounts() (gas: 616327)
UnstakeTest:test_UnstakeMultipleAccountsAndRewards() (gas: 937593)
UnstakeTest:test_UnstakeOneAccount() (gas: 446306)
UnstakeTest:test_UnstakeOneAccountAndRewards() (gas: 557157)
StakeTest:test_StakeMultipleAccounts() (gas: 438800)
StakeTest:test_StakeMultipleAccountsAndRewards() (gas: 586066)
StakeTest:test_StakeMultipleAccountsMPIncreasesMaxMPDoesNotChange() (gas: 743392)
StakeTest:test_StakeMultipleAccountsWithMinLockUp() (gas: 449118)
StakeTest:test_StakeMultipleAccountsWithRandomLockUp() (gas: 470666)
StakeTest:test_StakeOneAccount() (gas: 267816)
StakeTest:test_StakeOneAccountAndRewards() (gas: 415124)
StakeTest:test_StakeOneAccountMPIncreasesMaxMPDoesNotChange() (gas: 472829)
StakeTest:test_StakeOneAccountReachingMPLimit() (gas: 468437)
StakeTest:test_StakeOneAccountWithMaxLockUp() (gas: 284045)
StakeTest:test_StakeOneAccountWithMinLockUp() (gas: 284055)
StakeTest:test_StakeOneAccountWithRandomLockUp() (gas: 284099)
UnstakeTest:test_StakeMultipleAccounts() (gas: 438822)
UnstakeTest:test_StakeMultipleAccountsAndRewards() (gas: 586066)
UnstakeTest:test_StakeMultipleAccountsMPIncreasesMaxMPDoesNotChange() (gas: 743414)
UnstakeTest:test_StakeMultipleAccountsWithMinLockUp() (gas: 449118)
UnstakeTest:test_StakeMultipleAccountsWithRandomLockUp() (gas: 470688)
UnstakeTest:test_StakeOneAccount() (gas: 267816)
UnstakeTest:test_StakeOneAccountAndRewards() (gas: 415080)
UnstakeTest:test_StakeOneAccountMPIncreasesMaxMPDoesNotChange() (gas: 472851)
UnstakeTest:test_StakeOneAccountReachingMPLimit() (gas: 468459)
UnstakeTest:test_StakeOneAccountWithMaxLockUp() (gas: 284068)
UnstakeTest:test_StakeOneAccountWithMinLockUp() (gas: 284055)
UnstakeTest:test_StakeOneAccountWithRandomLockUp() (gas: 284144)
UnstakeTest:test_UnstakeMultipleAccounts() (gas: 616415)
UnstakeTest:test_UnstakeMultipleAccountsAndRewards() (gas: 937787)
UnstakeTest:test_UnstakeOneAccount() (gas: 446390)
UnstakeTest:test_UnstakeOneAccountAndRewards() (gas: 557241)
22 changes: 22 additions & 0 deletions certora/specs/RewardsStreamerMP.spec
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,27 @@ hook Sstore users[KEY address account].stakedBalance uint256 newValue (uint256 o
sumOfBalances = sumOfBalances - oldValue + newValue;
}

function getAccountMaxMP(address account) returns uint256 {
uint256 maxMP;
_, _, _, maxMP, _, _ = users(account);
return maxMP;
}

function getAccountMP(address account) returns uint256 {
uint256 accountMP;
_, _, accountMP, _, _, _ = users(account);
return accountMP;
}

function getAccountStakedBalance(address account) returns uint256 {
uint256 stakedBalance;
stakedBalance, _, _, _, _, _ = users(account);
return stakedBalance;
}

invariant sumOfBalancesIsTotalStaked()
sumOfBalances == to_mathint(totalStaked());

invariant accountMPLessEqualAccountMaxMP(address account)
to_mathint(getAccountMP(account)) <= to_mathint(getAccountMaxMP(account));

37 changes: 17 additions & 20 deletions src/RewardsStreamerMP.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ contract RewardsStreamerMP is ReentrancyGuard {

uint256 public totalStaked;
uint256 public totalMP;
uint256 public potentialMP;
uint256 public totalMaxMP;
uint256 public rewardIndex;
uint256 public accountedRewards;
uint256 public lastMPUpdatedTime;
Expand All @@ -34,7 +34,7 @@ contract RewardsStreamerMP is ReentrancyGuard {
uint256 stakedBalance;
uint256 userRewardIndex;
uint256 userMP;
uint256 userPotentialMP;
uint256 maxMP;
uint256 lastMPUpdateTime;
uint256 lockUntil;
}
Expand Down Expand Up @@ -78,12 +78,13 @@ contract RewardsStreamerMP is ReentrancyGuard {
totalStaked += amount;

uint256 initialMP = amount;
uint256 userPotentialMP = amount * MAX_MULTIPLIER;
uint256 userMaxMP = amount * MAX_MULTIPLIER;

if (lockPeriod != 0) {
uint256 lockMultiplier = (lockPeriod * MAX_MULTIPLIER * SCALE_FACTOR) / MAX_LOCKING_PERIOD;
initialMP += amount * lockMultiplier / SCALE_FACTOR;
userPotentialMP += (amount * lockMultiplier / SCALE_FACTOR);
uint256 bonusMP = amount * lockMultiplier / SCALE_FACTOR;
initialMP += bonusMP;
userMaxMP += bonusMP;
user.lockUntil = block.timestamp + lockPeriod;
} else {
user.lockUntil = 0;
Expand All @@ -92,8 +93,8 @@ contract RewardsStreamerMP is ReentrancyGuard {
user.userMP += initialMP;
totalMP += initialMP;

user.userPotentialMP += userPotentialMP;
potentialMP += userPotentialMP;
user.maxMP += userMaxMP;
totalMaxMP += userMaxMP;

user.userRewardIndex = rewardIndex;
user.lastMPUpdateTime = block.timestamp;
Expand All @@ -120,14 +121,13 @@ contract RewardsStreamerMP is ReentrancyGuard {
uint256 previousStakedBalance = user.stakedBalance;

uint256 mpToReduce = (user.userMP * amount * SCALE_FACTOR) / (previousStakedBalance * SCALE_FACTOR);
uint256 potentialMPToReduce =
(user.userPotentialMP * amount * SCALE_FACTOR) / (previousStakedBalance * SCALE_FACTOR);
uint256 maxMPToReduce = (user.maxMP * amount * SCALE_FACTOR) / (previousStakedBalance * SCALE_FACTOR);

user.stakedBalance -= amount;
user.userMP -= mpToReduce;
user.userPotentialMP -= potentialMPToReduce;
user.maxMP -= maxMPToReduce;
totalMP -= mpToReduce;
potentialMP -= potentialMPToReduce;
totalMaxMP -= maxMPToReduce;
totalStaked -= amount;

bool success = STAKING_TOKEN.transfer(msg.sender, amount);
Expand All @@ -148,7 +148,7 @@ contract RewardsStreamerMP is ReentrancyGuard {
}

function updateGlobalMP() internal {
if (potentialMP == 0) {
if (totalMaxMP == 0) {
lastMPUpdatedTime = block.timestamp;
return;
}
Expand All @@ -160,8 +160,8 @@ contract RewardsStreamerMP is ReentrancyGuard {
}

uint256 accruedMP = (timeDiff * totalStaked * MP_RATE_PER_YEAR) / (365 days * SCALE_FACTOR);
if (accruedMP > potentialMP) {
accruedMP = potentialMP;
if (totalMP + accruedMP > totalMaxMP) {
accruedMP = totalMaxMP - totalMP;
}

// Adjust rewardIndex before updating totalMP
Expand All @@ -173,7 +173,6 @@ contract RewardsStreamerMP is ReentrancyGuard {
rewardIndex = (rewardIndex * previousTotalWeight) / newTotalWeight;
}

potentialMP -= accruedMP;
lastMPUpdatedTime = currentTime;
}

Expand All @@ -195,7 +194,7 @@ contract RewardsStreamerMP is ReentrancyGuard {
function _updateUserMP(address userAddress) internal {
UserInfo storage user = users[userAddress];

if (user.userPotentialMP == 0 || user.stakedBalance == 0) {
if (user.maxMP == 0 || user.stakedBalance == 0) {
user.lastMPUpdateTime = block.timestamp;
return;
}
Expand All @@ -207,13 +206,11 @@ contract RewardsStreamerMP is ReentrancyGuard {

uint256 accruedMP = (timeDiff * user.stakedBalance * MP_RATE_PER_YEAR) / (365 days * SCALE_FACTOR);

if (accruedMP > user.userPotentialMP) {
accruedMP = user.userPotentialMP;
if (user.userMP + accruedMP > user.maxMP) {
accruedMP = user.maxMP - user.userMP;
}

user.userPotentialMP -= accruedMP;
user.userMP += accruedMP;

user.lastMPUpdateTime = block.timestamp;
}

Expand Down
Loading

0 comments on commit 6c94860

Please sign in to comment.