fix: improve precision loss when unstaking and add testso

This commit changes the calculation for when MPs are reduced globally
and for the user that is unstaking.

Instead of getting an `amountRatio` first and using that the
multiplication, we're now applying the `SCALE_FACTOR` to both, the
numerator and denominator to maintain the ratio while increasing
precision.
This commit is contained in:
r4bbit
2024-09-23 16:12:11 +02:00
parent a48a050bd8
commit 0af58f90fb
4 changed files with 153 additions and 30 deletions

View File

@@ -15,21 +15,21 @@
| src/RewardsStreamerMP.sol:RewardsStreamerMP contract | | | | | |
|------------------------------------------------------|-----------------|--------|--------|--------|---------|
| Deployment Cost | Deployment Size | | | | |
| 1100217 | 4957 | | | | |
| 1105804 | 4983 | | | | |
| Function Name | min | avg | median | max | # calls |
| MAX_LOCKING_PERIOD | 228 | 228 | 228 | 228 | 18 |
| MAX_MULTIPLIER | 229 | 229 | 229 | 229 | 24 |
| MIN_LOCKING_PERIOD | 229 | 229 | 229 | 229 | 8 |
| SCALE_FACTOR | 251 | 251 | 251 | 251 | 28 |
| accountedRewards | 373 | 1075 | 373 | 2373 | 37 |
| getUserInfo | 1553 | 1553 | 1553 | 1553 | 32 |
| potentialMP | 330 | 330 | 330 | 330 | 37 |
| rewardIndex | 373 | 427 | 373 | 2373 | 37 |
| stake | 167821 | 217684 | 228608 | 249401 | 31 |
| totalMP | 352 | 352 | 352 | 352 | 37 |
| totalStaked | 330 | 330 | 330 | 330 | 37 |
| unstake | 75267 | 113408 | 133268 | 133945 | 5 |
| updateGlobalState | 30008 | 67918 | 80335 | 80335 | 9 |
| 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 |
| test/mocks/MockToken.sol:MockToken contract | | | | | |
@@ -37,10 +37,10 @@
| Deployment Cost | Deployment Size | | | | |
| 639406 | 3369 | | | | |
| Function Name | min | avg | median | max | # calls |
| approve | 46346 | 46346 | 46346 | 46346 | 110 |
| balanceOf | 561 | 1334 | 561 | 2561 | 168 |
| mint | 51284 | 58124 | 51284 | 68384 | 110 |
| transfer | 34390 | 47690 | 51490 | 51490 | 9 |
| approve | 46346 | 46346 | 46346 | 46346 | 120 |
| balanceOf | 561 | 1327 | 561 | 2561 | 201 |
| mint | 51284 | 58124 | 51284 | 68384 | 120 |
| transfer | 34390 | 48070 | 51490 | 51490 | 10 |

View File

@@ -1,4 +1,4 @@
IntegrationTest:testStake() (gas: 1377603)
IntegrationTest:testStake() (gas: 1378213)
RewardsStreamerTest:testStake() (gas: 869874)
StakeTest:test_StakeMultipleAccounts() (gas: 438756)
StakeTest:test_StakeMultipleAccountsAndRewards() (gas: 586002)
@@ -9,14 +9,16 @@ StakeTest:test_StakeOneAccountAndRewards() (gas: 415039)
StakeTest:test_StakeOneAccountWithMaxLockUp() (gas: 284120)
StakeTest:test_StakeOneAccountWithMinLockUp() (gas: 284152)
StakeTest:test_StakeOneAccountWithRandomLockUp() (gas: 284196)
UnstakeTest:test_StakeMultipleAccounts() (gas: 438756)
UnstakeTest:test_StakeMultipleAccountsAndRewards() (gas: 586069)
UnstakeTest:test_StakeMultipleAccountsWithMinLockUp() (gas: 449192)
UnstakeTest:test_StakeMultipleAccountsWithRandomLockUp() (gas: 470881)
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: 415039)
UnstakeTest:test_StakeOneAccountWithMaxLockUp() (gas: 284186)
UnstakeTest:test_StakeOneAccountAndRewards() (gas: 415061)
UnstakeTest:test_StakeOneAccountWithMaxLockUp() (gas: 284120)
UnstakeTest:test_StakeOneAccountWithMinLockUp() (gas: 284152)
UnstakeTest:test_StakeOneAccountWithRandomLockUp() (gas: 284263)
UnstakeTest:test_UnstakeOneAccount() (gas: 445796)
UnstakeTest:test_UnstakeOneAccountAndRewards() (gas: 556852)
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)

View File

@@ -118,17 +118,17 @@ 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);
user.stakedBalance -= amount;
totalStaked -= amount;
uint256 amountRatio = (amount * SCALE_FACTOR) / previousStakedBalance;
uint256 mpToReduce = (user.userMP * amountRatio) / SCALE_FACTOR;
uint256 potentialMPToReduce = (user.userPotentialMP * amountRatio) / SCALE_FACTOR;
user.userMP -= mpToReduce;
user.userPotentialMP -= potentialMPToReduce;
totalMP -= mpToReduce;
potentialMP -= potentialMPToReduce;
totalStaked -= amount;
bool success = STAKING_TOKEN.transfer(msg.sender, amount);
if (!success) {

View File

@@ -814,4 +814,125 @@ contract UnstakeTest is StakeTest {
})
);
}
function test_UnstakeMultipleAccounts() public {
test_StakeMultipleAccounts();
_unstake(alice, 10e18);
_unstake(bob, 10e18);
checkStreamer(
CheckStreamerParams({
totalStaked: 20e18,
totalMP: 20e18,
potentialMP: 80e18,
stakingBalance: 20e18,
rewardBalance: 0,
rewardIndex: 0,
accountedRewards: 0
})
);
checkUser(
CheckUserParams({
user: alice,
rewardBalance: 0,
stakedBalance: 0,
rewardIndex: 0,
userMP: 0,
userPotentialMP: 0
})
);
checkUser(
CheckUserParams({
user: bob,
rewardBalance: 0,
stakedBalance: 20e18,
rewardIndex: 0,
userMP: 20e18,
userPotentialMP: 80e18
})
);
}
function test_UnstakeMultipleAccountsAndRewards() public {
test_StakeMultipleAccountsAndRewards();
_unstake(alice, 10e18);
checkStreamer(
CheckStreamerParams({
totalStaked: 30e18,
totalMP: 30e18,
potentialMP: 120e18,
stakingBalance: 30e18,
// alice owned a 25% of the pool, so 25% of the rewards are paid out to alice (250)
rewardBalance: 750e18,
rewardIndex: 125e17, // reward index remains unchanged
accountedRewards: 750e18
})
);
checkUser(
CheckUserParams({
user: alice,
rewardBalance: 250e18,
stakedBalance: 0,
rewardIndex: 125e17,
userMP: 0,
userPotentialMP: 0
})
);
_unstake(bob, 10e18);
checkStreamer(
CheckStreamerParams({
totalStaked: 20e18,
totalMP: 20e18,
potentialMP: 80e18,
stakingBalance: 20e18,
rewardBalance: 0, // bob should've now gotten the rest of the rewards
rewardIndex: 125e17,
accountedRewards: 0
})
);
checkUser(
CheckUserParams({
user: bob,
rewardBalance: 750e18,
stakedBalance: 20e18,
rewardIndex: 125e17,
userMP: 20e18,
userPotentialMP: 80e18
})
);
_unstake(bob, 20e18);
checkStreamer(
CheckStreamerParams({
totalStaked: 0,
totalMP: 0,
potentialMP: 0,
stakingBalance: 0,
rewardBalance: 0,
rewardIndex: 125e17,
accountedRewards: 0
})
);
checkUser(
CheckUserParams({
user: bob,
rewardBalance: 750e18,
stakedBalance: 0,
rewardIndex: 125e17,
userMP: 0,
userPotentialMP: 0
})
);
}
}