Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

claimRewards in FeeCollector.sol does not calculate rewards proportionately to voting power

peanuts

Summary

In FeeCollector.sol, users can claim fees if they have voting power through claimReward().

The calculation of claimReward() sets userRewards[user] = totalDistributed every time the function is called, which does not make sense and limit future rewards for the user.

Users with voting power can only claim reward once before waiting for a long time and not consistently.

Vulnerability Details

In claimRewards(), _calculatePendingRewards() is called before userRewards[user] is updated.

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();
// Reset user rewards before transfer
userRewards[user] = totalDistributed;
// Transfer rewards
raacToken.safeTransfer(user, pendingReward);
emit RewardClaimed(user, pendingReward);
return pendingReward;
}

In _calculatePendingRewards(), the share proportion is calculated and compared to userRewards[user].

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;
}

If the total voting power is 100, shared by 4 user who has 10,20,30,40 power respectively. totalDistributed is 10e18.

User A should get 1e18, user B should get 2e18, user C should get 3e18 and user D should get 4e18 of the fees. Their userRewards will then be updated to 10e18.

Now, if the total voting power stays the same, but totalDistributed is now 15e18, by right user A,B,C,D should be able to collect the 5e18 fees added too (0.5e18,1e18,1.5e18,2e18 respectively).

From the calculation, these 4 users will be denied the rewards because their shares < userRewards[user]

For user A, her userRewards[user] is now 10e18

User A
​
uint256 share = (totalDistributed * userVotingPower) / totalVotingPower;
share = 15e18 * 10 / 100 = 1.5e18
return share > userRewards[user] ? share - userRewards[user] : 0;
return 1.5e18 > 10e18 (not greater than, default to 0)

User A is supposed to get 1.5e18 rewards but she gets 0 instead. For user A to get his next rewards, assuming same voting power and same total power, totalDistributed needs to be more than 100e18.

Impact

Users with voting power can only claim once before needing to wait for a long time, especially for those with lower voting power

Tools Used

Manual Review

Recommendations

userRewards[user] should not be set to totalDistributed to pendingRewards instead and should be cumulative.

userRewards[user] += pendingRewards;

This way, with user A having collected 1e18 as rewards the first time, will be able to collect 0.5e18 the next time, and whenever totalDistributed is increased, assuming same voting and total voting power, user A can collect every time.

If totalDistributed is 15e18, shares = 1.5e18, and userRewards[user] was 1e18 so userA collects 0.5e18. userRewards[user] is now 1.5e18.
totalDistributed is 20e18 now, shares = 2e18, userRewards[user] was 1.5e18 so user A can collect another 0.5e18, increasing her userRewards[user] to 2e18.

Updates

Lead Judging Commences

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

FeeCollector::claimRewards sets `userRewards[user]` to `totalDistributed` seriously grieving users from rewards

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.