Submission Details
Severity:
FeeCollector stakeholders may receive less fee distribution due to unnecessarily precision loss
Summary
FeeCollector may distribute less fee amount to stakeholders, due to unncessarily precision loss caused by division before multiplication.
Vulnerability Details
When calculates fee distribution shares for different stakeholders, protocol firstly calculates weight and uses it to compute each fee type's shares.
FeeCollector::_calculateDistribution()
for (uint8 i = 0; i < 8; i++) {
uint256 feeAmount = _getFeeAmountByType(i);
if (feeAmount == 0) continue;
FeeType memory feeType = feeTypes[i];
totalCollected += feeAmount;
@> uint256 weight = (feeAmount * BASIS_POINTS) / totalFees;
@> shares[0] += (weight * feeType.veRAACShare) / BASIS_POINTS;
@> shares[1] += (weight * feeType.burnShare) / BASIS_POINTS;
@> shares[2] += (weight * feeType.repairShare) / BASIS_POINTS;
@> shares[3] += (weight * feeType.treasuryShare) / BASIS_POINTS;
}
if (totalCollected != totalFees) revert InvalidFeeAmount();
@> shares[0] = (totalFees * shares[0]) / BASIS_POINTS;
@> shares[1] = (totalFees * shares[1]) / BASIS_POINTS;
@> shares[2] = (totalFees * shares[2]) / BASIS_POINTS;
@> shares[3] = (totalFees * shares[3]) / BASIS_POINTS;
Firstly, feeAmount * BASIS_POINTS is divided by totalFees, then the result (weight) is multiplied with corresponding share and divided by BASIS_POINTS, at last, the actual shares to each stakholder are computed by multiplying by totalFees and dividing by BASIS_POINTS.
To make it straight, shares = ((feeAmount * BASIS_POINTS) / totalFees * feeType.xxShare)) / BASIS_POINTS * totalFees / BASIS_POINTS .
As can be seen, there is division before multiplication, this can leads to unnecessary precision loss. The precision loss is in basis point, meaning for every 10000 tokens, the loss is 1 token and is not trivial.
Impact
Unecessary precision loss leads to non-trivial token loss, and stakeholders receive less distribution that expected.
POC
Please runforge test --mt testAudit_DistributeFeePrecisionLoss.
// SPDX-License-Identifier: MIT
import {Test, console, stdError} from "forge-std/Test.sol";
import "@openzeppelin/contracts/utils/math/SafeCast.sol";
import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
import "../contracts/libraries/math/WadRayMath.sol";
import "../contracts/core/pools/LendingPool/LendingPool.sol";
import "../contracts/core/pools/StabilityPool/StabilityPool.sol";
import "../contracts/mocks/core/tokens/crvUSDToken.sol";
import "../contracts/core/tokens/RToken.sol";
import "../contracts/core/tokens/DebtToken.sol";
import "../contracts/core/tokens/DeToken.sol";
import "../contracts/core/tokens/RAACToken.sol";
import "../contracts/core/tokens/RAACNFT.sol";
import "../contracts/core/tokens/veRAACToken.sol";
import "../contracts/core/primitives/RAACHousePrices.sol";
import "../contracts/core/minters/RAACMinter/RAACMinter.sol";
import "../contracts/core/collectors/FeeCollector.sol";
import "../contracts/core/collectors/Treasury.sol";
contract Audit is Test {
using WadRayMath for uint256;
using SafeCast for uint256;
address owner = makeAddr("Owner");
address repairFund = makeAddr("RepairFund");
LendingPool lendingPool;
StabilityPool stabilityPool;
RAACHousePrices raacHousePrices;
crvUSDToken crvUSD;
RToken rToken;
DebtToken debtToken;
DEToken deToken;
RAACToken raacToken;
RAACNFT raacNft;
veRAACToken veRaacToken;
RAACMinter raacMinter;
FeeCollector feeCollector;
Treasury treasury;
function setUp() public {
vm.warp(1 days);
raacHousePrices = new RAACHousePrices(owner);
// Deploy tokens
raacToken = new RAACToken(owner, 100, 50);
veRaacToken = new veRAACToken(address(raacToken));
crvUSD = new crvUSDToken(owner);
rToken = new RToken("RToken", "RToken", owner, address(crvUSD));
debtToken = new DebtToken("DebtToken", "DT", owner);
raacNft = new RAACNFT(address(crvUSD), address(raacHousePrices), owner);
deToken = new DEToken("DEToken", "DEToken", owner, address(rToken));
// Deploy Treasury and FeeCollector
treasury = new Treasury(owner);
feeCollector = new FeeCollector(
address(raacToken),
address(veRaacToken),
address(treasury),
repairFund,
owner
);
// Deploy LendingPool
lendingPool = new LendingPool(
address(crvUSD),
address(rToken),
address(debtToken),
address(raacNft),
address(raacHousePrices),
0.1e27
);
lendingPool.transferOwnership(owner);
// Deploy stabilityPool Proxy
bytes memory data = abi.encodeWithSelector(
StabilityPool.initialize.selector,
address(rToken),
address(deToken),
address(raacToken),
address(owner),
address(crvUSD),
address(lendingPool)
);
address stabilityPoolProxy = address(
new TransparentUpgradeableProxy(
address(new StabilityPool(owner)),
owner,
data
)
);
stabilityPool = StabilityPool(stabilityPoolProxy);
// RAACMinter
raacMinter = new RAACMinter(
address(raacToken),
address(stabilityPool),
address(lendingPool),
owner
);
// Initialization
vm.startPrank(owner);
raacHousePrices.setOracle(owner);
rToken.setReservePool(address(lendingPool));
debtToken.setReservePool(address(lendingPool));
deToken.setStabilityPool(address(stabilityPool));
stabilityPool.setRAACMinter(address(raacMinter));
lendingPool.setStabilityPool(address(stabilityPool));
raacToken.setMinter(address(raacMinter));
raacToken.setFeeCollector(address(feeCollector));
raacToken.manageWhitelist(address(feeCollector), true);
raacToken.manageWhitelist(address(raacMinter), true);
raacToken.manageWhitelist(address(stabilityPool), true);
vm.stopPrank();
vm.label(address(crvUSD), "crvUSD");
vm.label(address(rToken), "RToken");
vm.label(address(debtToken), "DebtToken");
vm.label(address(deToken), "DEToken");
vm.label(address(raacToken), "RAACToken");
vm.label(address(raacNft), "RAAC NFT");
vm.label(address(lendingPool), "LendingPool");
vm.label(address(stabilityPool), "StabilityPool");
vm.label(address(raacMinter), "RAACMinter");
vm.label(address(veRaacToken), "veRAAC");
vm.label(address(feeCollector), "FeeCollector");
vm.label(address(treasury), "Treasury");
vm.label(repairFund, "RepairFund");
}
function testAudit_DistributeFeePrecisionLoss() public {
address alice = makeAddr("Alice");
deal(address(raacToken), alice, 100e18);
vm.startPrank(alice);
raacToken.approve(address(veRaacToken), 100e18);
veRaacToken.lock(100e18, 365 days);
vm.stopPrank();
uint256 distributeAmount = 60000e18;
uint256 feeTypeAmount = 3;
address feeDistributor = makeAddr("FeeDistributor");
deal(address(raacToken), feeDistributor, distributeAmount);
vm.startPrank(owner);
feeCollector.grantRole(feeCollector.DISTRIBUTOR_ROLE(), feeDistributor);
raacToken.manageWhitelist(feeDistributor, true);
vm.stopPrank();
vm.startPrank(feeDistributor);
raacToken.approve(address(feeCollector), distributeAmount);
feeCollector.collectFee(10000e18, 0);
feeCollector.collectFee(20000e18, 1);
feeCollector.collectFee(30000e18, 2);
feeCollector.distributeCollectedFees();
vm.stopPrank();
feeCollector.claimRewards(alice);
// The unnecessary precision loss is 10 tokens
assertEq(raacToken.balanceOf(alice), 39990e18);
}
}
Tools Used
Manual Review
Recommendations
Multiply before dividing to avoid unnecessary precision loss.
for (uint8 i = 0; i < 8; i++) {
uint256 feeAmount = _getFeeAmountByType(i);
if (feeAmount == 0) continue;
FeeType memory feeType = feeTypes[i];
totalCollected += feeAmount;
- uint256 weight = (feeAmount * BASIS_POINTS) / totalFees;
- shares[0] += (weight * feeType.veRAACShare) / BASIS_POINTS;
- shares[1] += (weight * feeType.burnShare) / BASIS_POINTS;
- shares[2] += (weight * feeType.repairShare) / BASIS_POINTS;
- shares[3] += (weight * feeType.treasuryShare) / BASIS_POINTS;
+ shares[0] += totalFees * feeAmount * feeType.veRAACShare / totalFees / BASIS_POINTS;
+ shares[1] += totalFees * feeAmount * feeType.burnShare / totalFees / BASIS_POINTS;
+ shares[2] += totalFees * feeAmount * feeType.repairShare / totalFees / BASIS_POINTS;
+ shares[3] += totalFees * feeAmount * feeType.treasuryShare / totalFees / BASIS_POINTS;
}
if (totalCollected != totalFees) revert InvalidFeeAmount();
- shares[0] = (totalFees * shares[0]) / BASIS_POINTS;
- shares[1] = (totalFees * shares[1]) / BASIS_POINTS;
- shares[2] = (totalFees * shares[2]) / BASIS_POINTS;
- shares[3] = (totalFees * shares[3]) / BASIS_POINTS;
Updates
Lead Judging Commences
inallhonesty 7 months ago
Submission Judgement Published Assigned finding tags:
FeeCollector distributes too much to treasury when fee amounts are small relative to total due to precision loss in (feeAmount * BASIS_POINTS) / totalFees
inallhonesty 7 months ago
Submission Judgement Published Assigned finding tags:
FeeCollector distributes too much to treasury when fee amounts are small relative to total due to precision loss in (feeAmount * BASIS_POINTS) / totalFees
Prize pool breakdown
Total prize
77,280 USDC
nSLOC
3,86420
USDC / LOC
74,000
3,280
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.