Skip to content

Commit

Permalink
Audit Fixes QS-17 and rewardRate precision bug (#70)
Browse files Browse the repository at this point in the history 
* fix(Rewarder): added a function to call farm.recoverRewardFunds

* Added test cases for recoverRewardFundsOfFarm of Rewarder contract

* fix(NormalizeAmountRewarder): handle >18 precision and adjusts total value

* Added and updated test cases

* refactor(formatting): Rewarder

* refactor(Rewarder): _normalizeAmount does not need to check decimals as it is added while configuration

* perf(Rewarder): _normalizeAmount uses REWARD_TOKEN_DECIMALS directly from storage

* Revert "perf(Rewarder): _normalizeAmount uses REWARD_TOKEN_DECIMALS directly from storage"

This reverts commit c86cf2c401d0d7cfa1f5e35c65c6a41fbd17db24.

* Updated test cases

* perf(Rewarder): _normalizeAmount uses REWARD_TOKEN_DECIMALS directly from storage

---------

Co-authored-by: Yash Pitroda <yash.pitroda786@gmail.com>
  • Loading branch information
@arcantheon @YashP16
arcantheon and YashP16 authored Jun 24, 2024
1 parent e1359d8 commit 8906a6e
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 14 deletions.
34 changes: 22 additions & 12 deletions contracts/rewarder/Rewarder.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ contract Rewarder is Ownable, Initializable, ReentrancyGuard {
uint256 public constant DENOMINATOR = 100;
uint256 public constant ONE_YEAR = 365 days;
address public REWARD_TOKEN; // solhint-disable-line var-name-mixedcase
uint8 public REWARD_TOKEN_DECIMALS; // solhint-disable-line var-name-mixedcase
uint256 public totalRewardRate; // Rewards emitted per second for all the farms from this rewarder.
address public rewarderFactory;
// farm -> FarmRewardConfig.
Expand Down Expand Up @@ -223,6 +224,7 @@ contract Rewarder is Ownable, Initializable, ReentrancyGuard {
_validatePriceFeed(_rwdToken, _oracle);
rewarderFactory = _rewarderFactory;
REWARD_TOKEN = _rwdToken;
REWARD_TOKEN_DECIMALS = ERC20(_rwdToken).decimals();
_validateNonZeroAddr(_admin);
_transferOwnership(_admin);
}
Expand Down Expand Up @@ -292,10 +294,13 @@ contract Rewarder is Ownable, Initializable, ReentrancyGuard {
}
// Getting reward token price to calculate rewards emission.
priceData = _getPrice(REWARD_TOKEN, oracle);

// For token with lower decimals the calculation of rewardRate might not be accurate because of precision loss in truncation.
// rewardValuePerSecond = (APR * totalValue / 100) / 365 days.
// rewardRate = rewardValuePerSecond * pricePrecision / price.
rewardRate = (farmRewardConfig.apr * totalValue * priceData.precision)
/ (APR_PRECISION * DENOMINATOR * ONE_YEAR * priceData.price);

if (rewardRate > farmRewardConfig.maxRewardRate) {
rewardRate = farmRewardConfig.maxRewardRate;
}
Expand Down Expand Up @@ -350,18 +355,6 @@ contract Rewarder is Ownable, Initializable, ReentrancyGuard {
totalRewardRate = totalRewardRate - _oldRewardRate + _newRewardRate;
}

/// @notice Function to normalize asset amounts to be of precision 1e18.
/// @param _token Address of the asset token.
/// @param _amount Amount of the token.
/// @return Normalized amount of the token in 1e18.
function _normalizeAmount(address _token, uint256 _amount) private returns (uint256) {
if (_decimals[_token] == 0) {
_decimals[_token] = ERC20(_token).decimals();
}
_amount *= 10 ** (18 - _decimals[_token]);
return _amount;
}

/// @notice Function to validate farm.
/// @param _farm Address of the farm to be validated.
/// @param _baseTokens Array of base tokens.
Expand All @@ -386,6 +379,7 @@ contract Rewarder is Ownable, Initializable, ReentrancyGuard {
hasBaseTokens = false;
for (uint8 j; j < _assetsLen;) {
if (_baseTokens[i] == _assets[j]) {
_decimals[_baseTokens[i]] = ERC20(_baseTokens[i]).decimals();
hasBaseTokens = true;
farmRewardConfigs[_farm].baseAssetIndexes.push(j);
// Deleting will make _assets[j] -> 0x0 so if _baseTokens have repeated address, this function will return false.
Expand All @@ -406,6 +400,22 @@ contract Rewarder is Ownable, Initializable, ReentrancyGuard {
return true;
}

/// @notice Function to normalize asset amounts to be of precision REWARD_TOKEN_DECIMALS.
/// @param _token Address of the asset token.
/// @param _amount Amount of the token.
/// @return Normalized amount of the token in _desiredPrecision.
function _normalizeAmount(address _token, uint256 _amount) private view returns (uint256) {
uint8 decimals = _decimals[_token];
uint8 rwdTokenDecimals = REWARD_TOKEN_DECIMALS;
if (decimals < rwdTokenDecimals) {
return _amount * 10 ** (rwdTokenDecimals - decimals);
}
if (decimals > rwdTokenDecimals) {
return _amount / 10 ** (decimals - rwdTokenDecimals);
}
return _amount;
}

/// @notice Function to fetch and get the price of a token.
/// @param _token Token for which the the price is to be fetched.
/// @param _oracle Address of the oracle contract.
Expand Down
47 changes: 45 additions & 2 deletions test/rewarder/Rewarder.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ contract TestUpdateAPR is RewarderTest {
}

function test_UpdateAPR_CapRewardsWithMaxRwdRate() public useKnownActor(rewardManager) {
uint256 MAX_REWARD_RATE = 166665;
uint256 MAX_REWARD_RATE = 50; // 50 wei: Because rwdToken decimals are 6
Rewarder.FarmRewardConfigInput memory rewardConfig;
address[] memory baseAssets = new address[](1);
baseAssets[0] = USDCe;
Expand All @@ -133,12 +133,55 @@ contract TestUpdateAPR is RewarderTest {
rewarder.updateRewardConfig(lockupFarm, rewardConfig);
changePrank(owner);
CamelotV2Farm(lockupFarm).updateRewardData(USDCe, address(rewarder));
deposit(lockupFarm, false, 1000);
deposit(lockupFarm, false, 100000);
rewarder.calibrateReward(lockupFarm);
(, uint256 rewardRate,,) = rewarder.farmRewardConfigs(lockupFarm);
assertEq(rewardRate, MAX_REWARD_RATE);
}

function test_UpdateAPR_ForBaseTokenDecimalsMoreThanRwdTokenDecimals() public useKnownActor(rewardManager) {
rewarder = Rewarder(rewarderFactory.deployRewarder(USDCe));
Rewarder.FarmRewardConfigInput memory rewardConfig;
address[] memory baseAssets = new address[](1);
baseAssets[0] = DAI;
rewardConfig = Rewarder.FarmRewardConfigInput({
apr: 12e8,
maxRewardRate: UINT256_MAX,
baseTokens: baseAssets,
nonLockupRewardPer: 5000
});
rewarder.updateRewardConfig(lockupFarm, rewardConfig);
changePrank(owner);
CamelotV2Farm(lockupFarm).updateRewardData(USDCe, address(rewarder));
deposit(lockupFarm, false, 1000);
rewarder.calibrateReward(lockupFarm);
(, uint256 rewardRate,,) = rewarder.farmRewardConfigs(lockupFarm);
assertTrue((rewardRate * 30 days) / 1e6 > 0);
assertEq((rewardRate * 30 days) / 1e9, 0);
}

function test_UpdateAPR_ForRwdTokenDecimalsMoreThanBaseTokenDecimals() public useKnownActor(rewardManager) {
vm.mockCall(USDCe, abi.encodeWithSelector(ERC20.decimals.selector), abi.encode(20));
rewarder = Rewarder(rewarderFactory.deployRewarder(USDCe));
vm.clearMockedCalls();
Rewarder.FarmRewardConfigInput memory rewardConfig;
address[] memory baseAssets = new address[](1);
baseAssets[0] = DAI;
rewardConfig = Rewarder.FarmRewardConfigInput({
apr: 12e8,
maxRewardRate: UINT256_MAX,
baseTokens: baseAssets,
nonLockupRewardPer: 5000
});
rewarder.updateRewardConfig(lockupFarm, rewardConfig);
changePrank(owner);
CamelotV2Farm(lockupFarm).updateRewardData(USDCe, address(rewarder));
deposit(lockupFarm, false, 1000);
rewarder.calibrateReward(lockupFarm);
(, uint256 rewardRate,,) = rewarder.farmRewardConfigs(lockupFarm);
assertTrue((rewardRate * 30 days) / 1e20 > 0);
}

function _setupFarmRewards() private {
Rewarder.FarmRewardConfigInput memory rewardConfig;
address[] memory baseAssets = new address[](1);
Expand Down

0 comments on commit 8906a6e

Please sign in to comment.