updateTranscoderWtihFees can underflow because MathUtils is used instead of PreciseMathUtils.
According to LIP-92 the initial treasuryRewardCutRate will be set to 10%.
treasuryRewardCutRate is set with the setTreasuryRewardCutRate()function, which calls the internal function _setTreasuryRewardCutRate().
file: 2023-08-livepeer/contracts/bonding/BondingManager.sol
function _setTreasuryRewardCutRate(uint256 _cutRate) internal {
require(PreciseMathUtils.validPerc(_cutRate), "_cutRate is invalid precise percentage");
In this function the value will be checked if itβs a valid PreciseMathUtils percentage (<100% specified with 27-digits precision):
file: 2023-08-livepeer/contracts/libraries/PreciseMathUtils.sol
library PreciseMathUtils {
// ...
// Divisor used for representing percentages
uint256 public constant PERC_DIVISOR = 10**27;
function validPerc(uint256 _amount) internal pure returns (bool) {
return _amount <= PERC_DIVISOR;
}
// ...
However, in updateTranscoderWithFees, to calculate treasuryRewards, MathUtils is used instead of PreciseMathUtils.
file: 2023-08-livepeer/contracts/bonding/BondingManager.sol
function updateTranscoderWithFees(
address _transcoder,
uint256 _fees,
uint256 _round
) external whenSystemNotPaused onlyTicketBroker {
// ...
uint256 treasuryRewards = MathUtils.percOf(rewards, treasuryRewardCutRate);
rewards = rewards.sub(treasuryRewards);
// ...
}
MathUtils uses a PREC_DIVISOR of 1000000 instead of 10 ** 27 from the PreciseMathUtils:
file: 2023-08-livepeer/contracts/libraries/MathUtils.sol
library MathUtils {
// ...
uint256 public constant PERC_DIVISOR = 1000000;
// ...
This leads to treasuryRewards value being bigger than expected. Here is a gist of the POC:
POC. Running the POC it shows that the current usage of MathUtils when calculating treasuryRewards will always cause an underflow in the next line of code.
updateTranscoderWithFees is called every time a winning ticket is redeemed . Whenever the transcoder has skipped the previous round reward call, this function has to re-calculate the rewards, as documented in LIP-92 This re-calculation will always fail due to the underflow shown above.
This will lead to accounting errors, unexpected behaviours and can cause a loss of winning tickets.
Firstly, the accounting errors and unexpected behaviours: these are all the storage values getting updated in updateTranscoderWithFees:
file: 2023-08-livepeer/contracts/bonding/BondingManager.sol
function updateTranscoderWithFees( address _transcoder,
uint256 _fees,
uint256 _round
) external whenSystemNotPaused onlyTicketBroker {
// ...
// transcoder & earningsPool.data
L314: Transcoder storage t = transcoders[_transcoder];
L321: EarningsPool.Data storage earningsPool = t.earningsPoolPerRound[currentRound];
//accounting updates happen here
L377: t.cumulativeFees = t.cumulativeFees.add(transcoderRewardStakeFees)
.add(transcoderCommissionFees);
L382: earningsPool.updateCumulativeFeeFactor(prevEarningsPool,delegatorsFees);
L384: t.lastFeeRound = currentRound;
During currentRound() it wont be possible to update the Transcoder storage or
earningsPool.data storage because of the underflow that will happen because currentRound() - 1 reward call has been skipped by the transcoder.
During currentRound() + 1 it will be possible to call updateTranscoderWithFees, however, L382 will only update the prevEarningsPool, which in this case will be currentRound(), not currentRound - 1. Therefor, the EarningsPool.data.cumulativeRewardFactor wonβt be updated for currentRound() - 1.
Lastly, the validity of a ticket is two rounds as per the specs. This means that a transcoder that receives a winning ticket in currentRound() - 1 should be able to redeem it in currentRound() - 1 and currentRound(). However, a transcoder that receives a winning ticket in currentRound() - 1 wont be able to redeem it in currentRound() because of the underflow that happens while redeeming a winning ticket in currentRound(). The transcoder wont be able to redeem it after currentRound + 1β¦N because the ticket will be expired.
Manual Review
Use PreciseMathLib instead of MathLib:
file: 2023-08-livepeer/contracts/bonding/BondingManager.sol
L355:
- uint256 treasuryRewards = MathUtils.percOf(rewards, treasuryRewardCutRate);
+ uint256 treasuryRewards = PreciseMathUtils.percOf(rewards, treasuryRewardCutRate);
Library
The text was updated successfully, but these errors were encountered:
All reactions