Description
burn() burns only a partial amount even if feeCollector
is address(0)
:
File: contracts/core/tokens/RAACToken.sol
80: function burn(uint256 amount) external {
81: uint256 taxAmount = amount.percentMul(burnTaxRate);
82:@---> _burn(msg.sender, amount - taxAmount);
83: if (taxAmount > 0 && feeCollector != address(0)) {
84: _transfer(msg.sender, feeCollector, taxAmount);
85: }
86: }
Imagine:
amount = 1000
and burnTaxRate = 0.5%
Thus taxAmount = 0.5% * 1000 = 5
L82 burns 1000 - 5 = 95
Since feeCollector == address(0)
, no _transfer()
happens
User's 5
is never burnt.
They will have to keep on calling burn()
repeatedly with smaller & smaller amount
until taxAmount
calculates to zero. Repeated calls cost gas.
There's a higher impact too (refer following section) since raacToken.burn()
is called inside FeeCollector.sol
while distributing collected fees.
Impact
-
Gas cost impact due to repeated user calls.
-
Inside FeeCollector.sol
, the execution flow is distributeCollectedFees() --> _processDistributions() and _processDistributions()
has:
L421: if (shares[1] > 0) raacToken.burn(shares[1]);
shares[1]
refers to feeType.burnShare
.
As a result of this:
The actual circulating supply will be higher than intended
This reduces the deflationary effect intended by the protocol's tokenomics
The raacTokens
remain in the contract and hence when the next time _processDistributions()
is called, the contractBalance
fetched here shows up with the leftover amount too from the last burn. This fools the contract into thinking that enough fee was collected inside collectFee():
File: contracts/core/collectors/FeeCollector.sol
401: function _processDistributions(uint256 totalFees, uint256[4] memory shares) internal {
402:@---> uint256 contractBalance = raacToken.balanceOf(address(this));
403:@---> if (contractBalance < totalFees) revert InsufficientBalance();
404:
405: if (shares[0] > 0) {
406: uint256 totalVeRAACSupply = veRAACToken.getTotalVotingPower();
407: if (totalVeRAACSupply > 0) {
408: TimeWeightedAverage.createPeriod(
409: distributionPeriod,
410: block.timestamp + 1,
411: 7 days,
412: shares[0],
413: totalVeRAACSupply
414: );
415: totalDistributed += shares[0];
416: } else {
417: shares[3] += shares[0];
418: }
419: }
420:
421:@---> if (shares[1] > 0) raacToken.burn(shares[1]);
422: if (shares[2] > 0) raacToken.safeTransfer(repairFund, shares[2]);
423: if (shares[3] > 0) raacToken.safeTransfer(treasury, shares[3]);
424: }
and
File: contracts/core/collectors/FeeCollector.sol
162: function collectFee(uint256 amount, uint8 feeType) external override nonReentrant whenNotPaused returns (bool) {
163: if (amount == 0 || amount > MAX_FEE_AMOUNT) revert InvalidFeeAmount();
164: if (feeType > 7) revert InvalidFeeType();
165:
166:
167:@---> raacToken.safeTransferFrom(msg.sender, address(this), amount);
168:
169:
170: _updateCollectedFees(amount, feeType);
171:
172: emit FeeCollected(feeType, amount);
173: return true;
174: }
Mitigation
function burn(uint256 amount) external {
- uint256 taxAmount = amount.percentMul(burnTaxRate);
+ uint256 taxAmount;
+ if (feeCollector != address(0)) {
+ taxAmount = amount.percentMul(burnTaxRate);
+ }
_burn(msg.sender, amount - taxAmount);
- if (taxAmount > 0 && feeCollector != address(0)) {
+ if (taxAmount > 0) {
_transfer(msg.sender, feeCollector, taxAmount);
}
}