Summary
The dust accumulation in FeeCollector::totalDistributed continues to grow indefinitely, leading to locked funds within the contract.
Vulnerability Details
The function FeeCollector::_processDistributions() directly adds the feeType.veRAACShare portion to totalDistributed. However, when rewards are claimed, the distribution calculation is based on user weight. This discrepancy results in continuous dust accumulation.
function _processDistributions(uint256 totalFees, uint256[4] memory shares) internal {
uint256 contractBalance = raacToken.balanceOf(address(this));
if (contractBalance < totalFees) revert InsufficientBalance();
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];
} else {
shares[3] += shares[0];
}
}
if (shares[1] > 0) raacToken.burn(shares[1]);
if (shares[2] > 0) raacToken.safeTransfer(repairFund, shares[2]);
if (shares[3] > 0) raacToken.safeTransfer(treasury, shares[3]);
}
Poc
Before executing the test, a prerequisite fix for issue The current implementation of FeeCollector::claimRewards() can cause users to lose a significant portion of their rewards due to an incorrect tracking mechanism. is required. A simple modification is as follows:
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 `totalDistributed==userRewards[user]`, it means the user has received this part of the reward.
+ uint256 share = (totalDistributed - userRewards[user]) * userVotingPower / totalVotingPower;
+ return share;
}
Add the following test to test/unit/core/collectors/FeeCollector.test.js and execute it:
describe("The dust in totalDistributed will continue to expand", function () {
it("Check dust", async function () {
const taxRate = SWAP_TAX_RATE + BURN_TAX_RATE;
const grossMultiplier = BigInt(BASIS_POINTS * BASIS_POINTS) / BigInt(BASIS_POINTS * BASIS_POINTS - taxRate * BASIS_POINTS);
const protocolFeeGross = ethers.parseEther("50") * grossMultiplier / BigInt(10000);
const lendingFeeGross = ethers.parseEther("30") * grossMultiplier / BigInt(10000);
const swapTaxGross = ethers.parseEther("20") * grossMultiplier / BigInt(10000);
await network.provider.send("evm_mine");
await feeCollector.connect(user1).collectFee(protocolFeeGross, 0);
await feeCollector.connect(user1).collectFee(lendingFeeGross, 1);
await feeCollector.connect(user1).collectFee(swapTaxGross, 6);
await feeCollector.connect(owner).distributeCollectedFees();
await feeCollector.connect(user1).claimRewards(user1.address);
await feeCollector.connect(user2).claimRewards(user2.address);
console.log("First dust check:",await raacToken.balanceOf(await feeCollector.getAddress()));
await feeCollector.connect(user1).collectFee(protocolFeeGross, 0);
await feeCollector.connect(user1).collectFee(lendingFeeGross, 1);
await feeCollector.connect(user1).collectFee(swapTaxGross, 6);
await time.increase(WEEK);
await network.provider.send("evm_mine");
await feeCollector.connect(owner).distributeCollectedFees();
await feeCollector.connect(user1).claimRewards(user1.address);
await feeCollector.connect(user2).claimRewards(user2.address);
console.log("Second dust check:",await raacToken.balanceOf(await feeCollector.getAddress()));
await network.provider.send("evm_mine");
await feeCollector.connect(user1).collectFee(protocolFeeGross, 0);
await feeCollector.connect(user1).collectFee(lendingFeeGross, 1);
await feeCollector.connect(user1).collectFee(swapTaxGross, 6);
await time.increase(WEEK);
await network.provider.send("evm_mine");
await feeCollector.connect(owner).distributeCollectedFees();
await feeCollector.connect(user1).claimRewards(user1.address);
await feeCollector.connect(user2).claimRewards(user2.address);
console.log("Third dust check:",await raacToken.balanceOf(await feeCollector.getAddress()));
});
});
output:
FeeCollector
The dust in totalDistributed will continue to expand
First dust check: 137857369355656n
Second dust check: 302676623541343n
Third dust check: 563386288685928n
Impact
Due to dust accumulation in FeeCollector::totalDistributed, an ever-increasing amount of tokens remains locked in the contract. Over time, this leads to a permanent loss of funds, reducing the efficiency of the fee distribution mechanism and negatively impacting users.
Tools Used
Manual Review
Recommendations
To prevent dust accumulation, adjust the fee distribution logic to incorporate the dust amount from the current period into the next allocation.
