In reward calculation, dust amount is left and stuck in the contract for every epoch

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xe5a9b42a2793537a95cd2b51884dd64816b2eb94a7ec90beea86d0a93611761d Severity: medium

Description: Description

When rewards calculation happens, rewards accures with reward per epoch and the token's proportion of the total supply to determine the reward amount. Thus, every time, the remainder which is less than supply are left in the contract

Proof of Concept (PoC) File

    function _calculateRewardPerEpoch(uint256 tokenId_, uint256 epoch_) internal view returns (uint256) {
        uint256 balance = VirtualRewarderCheckpoints.getAmount(
            tokensInfo[tokenId_].balanceCheckpoints,
            tokensInfo[tokenId_].checkpointLastIndex,
            epoch_
        );

        uint256 supply = VirtualRewarderCheckpoints.getAmount(totalSupplyCheckpoints, totalSupplyCheckpointLastIndex, epoch_);

        if (supply == 0) {
            return 0;
        }

        return (balance * rewardsPerEpoch[epoch_ + _WEEK]) / supply;
//@audit-issue dust rewards are lost in every epoch
    }

Revised Code File (Optional)

The left amount of rewards should be added to next epoch or current epoch for distribution. This will motivate to current stakers to harvest rewards timely

    function _calculateRewardPerEpoch(uint256 tokenId_, uint256 epoch_) internal view returns (uint256) {
        uint256 balance = VirtualRewarderCheckpoints.getAmount(
            tokensInfo[tokenId_].balanceCheckpoints,
            tokensInfo[tokenId_].checkpointLastIndex,
            epoch_
        );

        uint256 supply = VirtualRewarderCheckpoints.getAmount(totalSupplyCheckpoints, totalSupplyCheckpointLastIndex, epoch_);

        if (supply == 0) {
            return 0;
        }
        uint256 rewardPerEpoch = (balance * rewardsPerEpoch[epoch_ + _WEEK]) / supply;
        uint256 dustReward = balance * rewardsPerEpoch[epoch_ + _WEEK] - rewardPerEpoch;
        rewardsPerEpoch[_currentEpoch()] += dustReward;

        return rewardPerEpoch;
    }

Labeling this medium since higher the supply, higher rewards will be lost from each epoch

[0xmahdirostami]

Lead: The remainders left inside the contract are not handled properly, But it doesn't lead to a large loss.

[Tri-pathi]

heyy @0xmahdirostami

The remainders left inside the contract are not handled properly, But it doesn't lead to a large loss.

The max loss in every epoch could be supply-1 which is large loss. and this happens for every epoch makes it valid medium.

in one month, loss could go up to 4 * (supply-1) reward tokens

[BohdanHrytsak]

The main token is FNX, which has a precision of 18, for the test I took the following values, certain extreme cases:

user balance = 1e18 supple = 1_000_000e18 rewards = 1e6

1e18 1e6/1_000_000e18 = 1 wei 0.1e18 1e6/1_000_000e18 = 0 wei

As you can see from this point, when the numbers are smaller, losses begin, and when the user has a share of less than 0.000001%, this is a dust, this is also true for all users with a minimum balance

Also, the recommendation seems to be incorrect

Rounding and loss of reward by the user occurs when their share is less than the accuracy of the token, so this is still dust

[Tri-pathi]

@BohdanHrytsak

In your given example , user balance is just 1 token and just 1 reward token is distributed in epoch. Do you really think this represents the actual case ?

[BohdanHrytsak]

@Tri-pathi This was attached as a case study of how this works. We can also change the numbers, but the point will not change

This case is far from reality and is as close as possible to a situation where this reward is lost. We can also state that the user's balance can be an order of magnitude smaller, but the rewards can be greater.

18 decimals balance 6 decimals reward token / 18 decimals supply = 6 decimals reward amount 18 decimals balance 18 decimals reward token / 18 decimals supply = 18 decimals reward amount

So, the losses start when the token does not have sufficient decimals, but in practice it will still be dust

Also, for example, an actual case will most likely look like this:

User A balance: 100 FNX Rewards: 1200 USDB Total supply: 10_000_000 FNX, what is 13% from total supply

100e18 * 1200e18/10_000_000e18 = 0.012e18 (all ok)

Ok, let's reduce the amount of the user and rewards 0.1e18 * 1e18/10_000_000e18 = 1e10 (all ok)

And now for a case similar to the one mentioned above: 0.001e18 * 0.1e6/10_000_000e18 = 0 (rounding), because he is deserves for 0.0000001 tokens with 6 decimals, and will not get anything

@Tri-pathi Please provide the numbers you used to determine the maximum loss case, Thanks