Submission Details
Severity:
Double truncation in fee distribution leads to incorrect reward allocations
Description
The FeeCollector::_calculateDistribution function uses a double division approach that causes cumulative precision loss. The current implementation first calculates fee type weights (with division), then calculates share percentages (another division), resulting in compounded truncation errors. This leads to incorrect distribution amounts that favor the treasury at the expense of other stakeholders.
Proof of Concept
Add this code to FeeCollector.sol to facilitate testing:
function calculateDistribution() public view returns (uint256[4] memory shares) {
uint256 totalFees = _calculateTotalFees();
return _calculateDistribution(totalFees);
}
function calculateDistributionFixed() public view returns (uint256[4] memory shares) {
uint256 totalFees = _calculateTotalFees();
return _calculateDistributionFixed(totalFees);
}
function _calculateDistributionFixed(uint256 totalFees) internal view returns (uint256[4] memory shares) {
uint256 totalCollected;
for (uint8 i = 0; i < 8; i++) {
uint256 feeAmount = _getFeeAmountByType(i);
if (feeAmount == 0) continue;
FeeType memory feeType = feeTypes[i];
totalCollected += feeAmount;
shares[0] += feeAmount * feeType.veRAACShare;
shares[1] += feeAmount * feeType.burnShare;
shares[2] += feeAmount * feeType.repairShare;
shares[3] += feeAmount * feeType.treasuryShare;
}
if (totalCollected != totalFees) revert InvalidFeeAmount();
shares[0] = shares[0] / BASIS_POINTS;
shares[1] = shares[1] / BASIS_POINTS;
shares[2] = shares[2] / BASIS_POINTS;
shares[3] = shares[3] / BASIS_POINTS;
uint256 remainder = totalFees - (shares[0] + shares[1] + shares[2] + shares[3]);
if (remainder > 0) shares[3] += remainder;
}
Comment this part out in the beforeEach section of FeeCollector.test.js as our test case will use original protocol settings:
// // Setup initial fee types
// defaultFeeType = {
// veRAACShare: 5000, // 50%
// burnShare: 1000, // 10%
// repairShare: 1000, // 10%
// treasuryShare: 3000, // 30%
// };
// for (let i = 0; i < 8; i++) {
// await feeCollector.connect(owner).updateFeeType(i, defaultFeeType);
// }
// // Calculate gross amounts needed including transfer tax
// const taxRate = SWAP_TAX_RATE + BURN_TAX_RATE; // 150 basis points (1.5%)
// 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);
// // Collect fees
// await feeCollector.connect(user1).collectFee(protocolFeeGross, 0);
// await feeCollector.connect(user1).collectFee(lendingFeeGross, 1);
// await feeCollector.connect(user1).collectFee(swapTaxGross, 6);
Add this test case in FeeCollector.test.js:
describe("Precision Loss Verification", function () {
it("shows double truncation loss", async function () {
// Approve fee collector for owner
await raacToken
.connect(owner)
.approve(feeCollector.target, ethers.MaxUint256);
// Mint tokens to owner first
await raacToken
.connect(owner)
.mint(owner.address, ethers.parseUnits("131110", 18));
// Collect precise amounts
const protocolFee = ethers.parseUnits("123456", 18);
const swapTax = ethers.parseUnits("7654", 18);
await feeCollector.connect(owner).collectFee(protocolFee, 0);
await feeCollector.connect(owner).collectFee(swapTax, 6);
// Calculate expected vs actual
// Original calculation
const shares = await feeCollector.calculateDistribution();
// Actual from contract (double truncated):
expect(shares[0]).to.equal(ethers.parseUnits("99132.271", 18));
expect(shares[3]).to.equal(ethers.parseUnits("30837.072", 18));
// Fixed calculation
const sharesFixed = await feeCollector.calculateDistributionFixed();
expect(sharesFixed[0]).to.equal(ethers.parseUnits("99147.5", 18));
expect(sharesFixed[3]).to.equal(ethers.parseUnits("30814.4", 18));
});
});
Impact
Medium Severity - Causes systematic misallocation of protocol fees:
Treasury receives unintended extra amounts from precision errors
veRAAC holders and other stakeholders receive less than entitled
Error magnitude increases with more fee types and smaller amounts
Recommendation
Single Division Approach:
contracts/core/collectors/FeeCollector.sol
function _calculateDistribution(uint256 totalFees) internal view returns (uint256[4] memory shares) {
uint256[4] memory scaledShares;
for (uint8 i = 0; i < 8; i++) {
uint256 feeAmount = _getFeeAmountByType(i);
if (feeAmount == 0) continue;
FeeType memory feeType = feeTypes[i];
scaledShares[0] += feeAmount * feeType.veRAACShare;
scaledShares[1] += feeAmount * feeType.burnShare;
scaledShares[2] += feeAmount * feeType.repairShare;
scaledShares[3] += feeAmount * feeType.treasuryShare;
}
shares[0] = scaledShares[0] / BASIS_POINTS;
shares[1] = scaledShares[1] / BASIS_POINTS;
shares[2] = scaledShares[2] / BASIS_POINTS;
shares[3] = scaledShares[3] / BASIS_POINTS;
// Keep remainder handling unchanged
}
Updates
Lead Judging Commences
inallhonesty 3 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 3 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.