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

claimRewards() incorrectly updates userRewards mapping resulting in user's inability to claim subsequent rewards.

sl1

Bug description

veRAACToken holders are entitled to part of the fees collected. The fees are distributed using _processDistributions() function of the fee collector, where totalDistributed variable is incremented.

FeeCollector.sol#L405-L415

if (shares[0] > 0) {
uint256 totalVeRAACSupply = veRAACToken.getTotalVotingPower();
if (totalVeRAACSupply > 0) {
TimeWeightedAverage.createPeriod(
distributionPeriod,
block.timestamp + 1,
7 days,
shares[0],
totalVeRAACSupply
);
totalDistributed += shares[0];

Later holders can call claimRewards() to claim their rewards. However, the function incorrectly sets userRewards to totalDistributed instead of pendingReward.

FeeCollector.sol#L202-L206

uint256 pendingReward = _calculatePendingRewards(user);
if (pendingReward == 0) revert InsufficientBalance();
// Reset user rewards before transfer
userRewards[user] = totalDistributed;

Assume a scenario where total voting power is 1000, user's voting power is 100, while totalDistributed is also 100. _calculatePendingRewards will calculated user's rewards in the following way.

FeeCollector.sol#L480-L487

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;

share will be equal to 10, and as userRewards currently is 0, the function will return 10. After that userRewards will be set to totalDistributed, which is 100. Now let's say totalDistributed got incremented to 110. If claimRewards is called again, logically user's share should be calculated as 110 * 100 / 1000 = 11, and the final value to receive is 11 minus 10 of already claimed rewards, so the final result of new rewards should be 1. However, because userRewards is currently equal to 100, the return value of the _calculatePendingRewards() will be 0, since share is lesser than userRewards, while userRewards will yet again be set to totalDistributed which is now equal to 110. As a result, after the first claim user has no way of claiming new rewards.

Impact

After user has claimed his rewards from feeCollector for the first time, he is now unable to claim any subsequent rewards. Rewards belonging to users will remain locked in the feeCollector without a way to claim them.

Recommended Mitigation

Set userRewards to pendingReward instead of totalDistributed.

FeeCollector.sol#L202-L206

uint256 pendingReward = _calculatePendingRewards(user);
if (pendingReward == 0) revert InsufficientBalance();
// Reset user rewards before transfer
- userRewards[user] = totalDistributed;
+ userRewards[user] = pendingReward;
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.