Summary
Function FeeCollector::claimRewards() allows an user to claim accumulated rewards. However, the pending reward amount is incorrectly computed such that the amount is getting lower over time.
Vulnerability Details
The function FeeCollector::_calculatePendingRewards() handles calculating user pending rewards. The reward amount is calculated as (totalDistributed * userVotingPower) / totalVotingPower which is proportional to userVotingPower / totalVotingPower.
However, the problem arises because:
The userVotingPower is returned from veRAACToken.getVotingPower(user) in which the return value decays over time
veRAACToken.totalVotingPower() does not depend on time because this value is the total supply of veRAACToken.
As a result, user pending reward amount is getting lower over time.
function claimRewards(address user) external override nonReentrant whenNotPaused returns (uint256) {
if (user == address(0)) revert InvalidAddress();
@> uint256 pendingReward = _calculatePendingRewards(user);
if (pendingReward == 0) revert InsufficientBalance();
userRewards[user] = totalDistributed;
raacToken.safeTransfer(user, pendingReward);
emit RewardClaimed(user, pendingReward);
return pendingReward;
}
function _calculatePendingRewards(address user) internal view returns (uint256) {
@> uint256 userVotingPower = veRAACToken.getVotingPower(user);
if (userVotingPower == 0) return 0;
uint256 totalVotingPower = veRAACToken.getTotalVotingPower();
if (totalVotingPower == 0) return 0;
@> uint256 share = (totalDistributed * userVotingPower) / totalVotingPower;
return share > userRewards[user] ? share - userRewards[user] : 0;
}
function getVotingPower(address account) public view returns (uint256) {
return _votingState.getCurrentPower(account, block.timestamp);
}
function getCurrentPower(
VotingPowerState storage state,
address account,
uint256 timestamp
) internal view returns (uint256) {
RAACVoting.Point memory point = state.points[account];
if (point.timestamp == 0) return 0;
if (timestamp < point.timestamp) {
return uint256(uint128(point.bias));
}
uint256 timeDelta = timestamp - point.timestamp;
int128 adjustedBias = point.bias;
if (timeDelta > 0) {
int128 decay = (point.slope * int128(int256(timeDelta))) / int128(int256(1));
adjustedBias = point.bias - decay;
}
return adjustedBias > 0 ? uint256(uint128(adjustedBias)) : 0;
}
PoC
Add the test to test/unit/core/collectors/FeeCollector.test.js
describe("Fee Collection and Distribution", function () {
it.only("user reward decreases over time", async function () {
await feeCollector.connect(owner).distributeCollectedFees();
const snapshot = await takeSnapshot();
const initialBalance = await raacToken.balanceOf(user1.address);
await feeCollector.connect(user1).claimRewards(user1.address);
const balanceAfter = await raacToken.balanceOf(user1.address);
const rewardClaimed = balanceAfter - initialBalance
await snapshot.restore();
await time.increase(4 * WEEK);
const initialBalance2 = await raacToken.balanceOf(user1.address);
await feeCollector.connect(user1).claimRewards(user1.address);
const balanceAfter2 = await raacToken.balanceOf(user1.address);
const rewardClaimed2 = balanceAfter2 - initialBalance2
expect(rewardClaimed2).to.eq(rewardClaimed)
});
Run the test and console shows:
FeeCollector
Fee Collection and Distribution
1) user reward decreases over time
0 passing (2s)
1 failing
1) FeeCollector
Fee Collection and Distribution
user reward decreases over time:
AssertionError: expected 3013698630136992 to equal 3269406392694065.
+ expected - actual
-3013698630136992
+3269406392694065
Impact
User reward amount's getting lower over time as voting power decays, which can be unfair for many users
Users who locks their tokens in-between the claim period can benefit more rewards than users who locks tokens before claim period
Tools Used
Manual
Recommendations
function _calculatePendingRewards(address user) internal view returns (uint256) {
- uint256 userVotingPower = veRAACToken.getVotingPower(user);
+ uint256 userVotingPower = veRAACToken.balanceOf(user);
if (userVotingPower == 0) return 0;
uint256 totalVotingPower = veRAACToken.getTotalVotingPower();
if (totalVotingPower == 0) return 0;
uint256 share = (totalDistributed * userVotingPower) / totalVotingPower;
return share > userRewards[user] ? share - userRewards[user] : 0;
}
