Summary
The FeeCollector contract implements a time-weighted average mechanism for fee distribution, but this mechanism is ineffective as users can claim their rewards immediately after distribution, bypassing the intended time-weighted distribution logic.
(Read Purpose Section) []
Vulnerability Details
The contract attempts to implement a time-weighted average distribution through the TimeWeightedAverage library in the _processDistributions() function:
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]);
}
However, the claimRewards() function calculates rewards based solely on current voting power ratios without considering the time-weighted period:
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;
}
This is demonstrated in the PoC below where:
PoC
In order to run the test you need to:
Run foundryup to get the latest version of Foundry
Install hardhat-foundry: npm install --save-dev @nomicfoundation/hardhat-foundry
Import it in your Hardhat config: require("@nomicfoundation/hardhat-foundry");
Make sure you've set the BASE_RPC_URL in the .env file or comment out the forking option in the hardhat config.
Run npx hardhat init-foundry
There is one file in the test folder that will throw an error during compilation so rename the file in test/unit/libraries/ReserveLibraryMock.sol to => ReserveLibraryMock.sol_broken so it doesn't get compiled anymore (we don't need it anyways).
Create a new folder test/foundry
Paste the below code into a new test file i.e.: FoundryTest.t.sol
Run the test: forge test --mc FoundryTest -vvvv
pragma solidity ^0.8.19;
import {Test} from "forge-std/Test.sol";
import {RAACToken} from "../../contracts/core/tokens/RAACToken.sol";
import {FeeCollector} from "../../contracts/core/collectors/FeeCollector.sol";
import {veRAACToken} from "../../contracts/core/tokens/veRAACToken.sol";
import {PercentageMath} from "../../contracts/libraries/math/PercentageMath.sol";
import {IFeeCollector} from "../../contracts/interfaces/core/collectors/IFeeCollector.sol";
import "forge-std/console2.sol";
contract FoundryTest is Test {
using PercentageMath for uint256;
RAACToken public raacToken;
FeeCollector public feeCollector;
veRAACToken public VeRaacToken;
address public owner;
address public treasury;
address public repairFund;
address public minter;
address public user1;
address public user2;
function setUp() public {
owner = address(this);
minter = makeAddr("minter");
user1 = makeAddr("user1");
user2 = makeAddr("user2");
treasury = makeAddr("treasury");
repairFund = makeAddr("repairFund");
uint256 initialSwapTaxRate = 100;
uint256 initialBurnTaxRate = 50;
vm.warp(1641070800);
vm.roll(100);
raacToken = new RAACToken(owner, initialSwapTaxRate, initialBurnTaxRate);
VeRaacToken = new veRAACToken(address(raacToken));
feeCollector = new FeeCollector(
address(raacToken),
address(VeRaacToken),
address(treasury),
address(repairFund),
owner
);
vm.prank(owner);
raacToken.setMinter(minter);
vm.prank(owner);
raacToken.setFeeCollector(address(feeCollector));
}
function test_RewardLogic() public {
uint8 feeType = 0;
uint256 amount = 3000e18;
uint256 feeAmount = 100e18;
uint256 lockAmount = 200e18;
vm.prank(minter);
raacToken.mint(user1, amount);
raacToken.manageWhitelist(address(feeCollector), true);
raacToken.manageWhitelist(address(VeRaacToken), true);
vm.startPrank(user1);
raacToken.approve(address(VeRaacToken), lockAmount);
raacToken.approve(address(feeCollector), amount);
VeRaacToken.lock(lockAmount, VeRaacToken.MIN_LOCK_DURATION());
feeCollector.collectFee(feeAmount, feeType);
vm.stopPrank();
IFeeCollector.CollectedFees memory collectedFees = feeCollector.getCollectedFees();
uint256 balanceOfFeeCollector = raacToken.balanceOf(address(feeCollector));
assertEq(balanceOfFeeCollector, collectedFees.protocolFees);
vm.prank(owner);
feeCollector.distributeCollectedFees();
uint256 pendingRewards = feeCollector.getPendingRewards(user1);
console2.log("\nTest:");
console2.log("recorded collectedFees before distribute", collectedFees.protocolFees);
console2.log("balanceOfFeeCollector before distribute", balanceOfFeeCollector);
console2.log("balance of veRAAC", VeRaacToken.balanceOf(user1));
console2.log("pendingRewards", pendingRewards);
uint256 balanceBeforeClaim = raacToken.balanceOf(user1);
vm.prank(user1);
feeCollector.claimRewards(user1);
assertEq(raacToken.balanceOf(user1), balanceBeforeClaim + pendingRewards);
}
}
Impact
The time-weighted average mechanism, likely intended to incentivize longer-term holding and prevent gaming of the distribution system, is completely ineffective
The contract behavior may not align with the protocol's economic incentive design
Tools Used
Recommendations
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 timeWeightedAverage = distributionPeriod.calculateAverage(block.timestamp);
uint256 share = (timeWeightedAverage * userVotingPower) / totalVotingPower;
return share > userRewards[user] ? share - userRewards[user] : 0;
}
Consider adding a minimum claim delay (the mapping already exists in the contract but is not used yet):
function claimRewards(address user) external override nonReentrant whenNotPaused returns (uint256) {
if (user == address(0)) revert InvalidAddress();
if (block.timestamp < lastClaimTime[user] + 1 days) {
revert ClaimTooEarly();
}
uint256 pendingReward = _calculatePendingRewards(user);
if (pendingReward == 0) revert InsufficientBalance();
userRewards[user] = totalDistributed;
_updateLastClaimTime(user);
raacToken.safeTransfer(user, pendingReward);
emit RewardClaimed(user, pendingReward);
return pendingReward;
}
