Description
When the staking contract remains inactive for an extended period, a new staker can quickly accumulate a disproportionately large number of points.
This occurs because the points distribution mechanism does not update the pointsPerToken variable in the case of zero totalStaked,
resulting in the new staker receiving a substantial share of the available points.
function distributePoints() public {
if (block.timestamp < lastDistribution + EPOCH_DURATION) {
return;
}
if (totalStaked == 0) {
return;
}
uint256 weeksPending = (block.timestamp - lastDistribution) / EPOCH_DURATION;
pointsPerToken =
pointsPerToken.add(weeksPending * (pointsPerEpoch.mul(PRECISION_18).div(totalStaked)));
totalPoints = totalPoints.add(pointsPerEpoch * weeksPending);
lastDistribution = lastDistribution + (weeksPending * 1 weeks);
emit PointsDistributed(pointsPerEpoch, pointsPerToken);
}
This situation could potentially create an imbalance in the reward system, favoring those who stake after a period of inactivity (totalStaked == 0).
Impact
Unfair distributaion of points
Proof of Concept
Consider changing the onStaked() function to run the preceding test:
function onStaked(address user, uint256 amount)
external
+ // onlyStaking
checkDistribution
updatePendingPoints(user)
{
UserInfo storage userInfo = users[user];
userInfo.stakedAmount = userInfo.stakedAmount.add(amount);
totalStaked = totalStaked.add(amount);
emit Staked(user, amount);
}
Test:
pragma solidity =0.8.21;
import "forge-std/Test.sol";
import "src/FjordPoints.sol";
contract TestFjordPoints is Test {
FjordPoints public fjordPoints;
uint256 blocktimestamp;
function setUp() public {
fjordPoints = new FjordPoints();
blocktimestamp = block.timestamp;
}
function test_test() public {
vm.warp(blocktimestamp + 3 weeks);
fjordPoints.onStaked(address(this), 100);
vm.warp(blocktimestamp + 4 weeks);
fjordPoints.claimPoints();
uint256 expectedPoints = 100 ether;
assertEq(fjordPoints.balanceOf(address(this)), expectedPoints);
}
}
The outcome of the test:
nt: 400000000000000000000 [4e20])
│ └─ ← [Stop]
├─ [608] FjordPoints::balanceOf(TestFjordPoints: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]) [staticcall]
│ └─ ← [Return] 400000000000000000000 [4e20]
├─ [0] VM::assertEq(400000000000000000000 [4e20], 100000000000000000000 [1e20]) [staticcall]
│ └─ ← [Revert] assertion failed: 400000000000000000000 != 100000000000000000000
└─ ← [Revert] assertion failed: 400000000000000000000 != 100000000000000000000
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 14.75ms (4.86ms CPU time)
Ran 1 test suite in 2.23s (14.75ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in test/unit/points.t.sol:TestFjordPoints
[FAIL. Reason: assertion failed: 400000000000000000000 != 100000000000000000000] test_test() (gas: 208920)
Encountered a total of 1 failing tests, 0 tests succeeded
As it is clear from the test above, instead of gaining 1-week points, a user gains 4-week points. This issue occurs whenever totalstaked becomes zero.
Tools Used
Manual Review
Recommended Mitigation
Consider modifying the distributePoints() function in such a way that it should correctly update the points per token in the case of a zero totalStaked variable.
SIDE NOTE
This issue does not exits in the staking contract, as _checkEpochRollover() function in the Staking contract. handles it correctly.
} else {
for (uint16 i = lastEpochRewarded + 1; i < currentEpoch; i++) {
rewardPerToken[i] = rewardPerToken[lastEpochRewarded];
emit RewardPerTokenChanged(i, rewardPerToken[i]);
}
}
However, there is a lack of a similar mitigation within the distributePoints() function, which could still allow for imbalances in points distribution.
