QA Report

[code423n4]

QA:
Tool Used: slither

1. Return value ignored
   Description:
   In the below functions, the return value is ignored.

ClaimFeesHelper.claimFees(IERC20) (ClaimFeesHelper.sol#43-57) ignores return value by feeDistro.claimToken(voterProxy,_token) (ClaimFeesHelper.sol#48)
ClaimFeesHelper.claimFees(IERC20) (ClaimFeesHelper.sol#43-57) ignores return value by feeDistro.claimToken(voterProxy,_token) (ClaimFeesHelper.sol#52)
ClaimFeesHelper.claimFees(IERC20) (ClaimFeesHelper.sol#43-57) ignores return value by booster.earmarkFees(address(_token)) (ClaimFeesHelper.sol#55)

2. Avoid comparing block.timestamp

Description:
In the below list of functions, the comparison is done using block.timestamp. The block timestamp can be easily manipulated which can lead to attacks. Avoid using timestamp comparison.

AuraBalRewardPool.getReward(bool) (AuraBalRewardPool.sol#176-190) uses timestamp for comparisons
Dangerous comparisons:
- reward > 0 (AuraBalRewardPool.sol#178)
AuraBalRewardPool.initialiseRewards() (AuraBalRewardPool.sol#205-220) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(msg.sender == rewardManager || block.timestamp > startTime,!authorized) (AuraBalRewardPool.sol#206)
AuraLocker.addReward(address,address) (AuraLocker.sol#195-202) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(rewardData[_rewardsToken].lastUpdateTime == 0,Reward already exists) (AuraLocker.sol#196)
AuraLocker.approveRewardDistributor(address,address,bool) (AuraLocker.sol#205-212) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(rewardData[_rewardsToken].lastUpdateTime > 0,Reward does not exist) (AuraLocker.sol#210)
AuraLocker.recoverERC20(address,uint256) (AuraLocker.sol#231-236) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(rewardData[_tokenAddress].lastUpdateTime == 0,Cannot withdraw reward token) (AuraLocker.sol#233)
AuraLocker._lock(address,uint256) (AuraLocker.sol#258-296) uses timestamp for comparisons
Dangerous comparisons:
- idx == 0 || userLocks[_account][idx - 1].unlockTime < unlockTime (AuraLocker.sol#278)
AuraLocker._checkpointEpoch() (AuraLocker.sol#326-339) uses timestamp for comparisons
Dangerous comparisons:
- epochs[epochindex - 1].date < currentEpoch (AuraLocker.sol#332)
- epochs[epochs.length - 1].date != currentEpoch (AuraLocker.sol#334)
AuraLocker._processExpiredLocks(address,bool,address,uint256) (AuraLocker.sol#371-458) uses timestamp for comparisons
Dangerous comparisons:
- isShutdown || locks[length - 1].unlockTime <= expiryTime (AuraLocker.sol#389)
- locks[i].unlockTime > expiryTime (AuraLocker.sol#412)
- reward > 0 (AuraLocker.sol#443)
AuraLocker.delegate(address) (AuraLocker.sol#467-509) uses timestamp for comparisons
Dangerous comparisons:
- currentLock.unlockTime > upcomingEpoch (AuraLocker.sol#488)
AuraLocker._checkpointDelegate(address,uint256,uint256) (AuraLocker.sol#511-564) uses timestamp for comparisons
Dangerous comparisons:
- prevCkpt.epochStart == upcomingEpoch (AuraLocker.sol#523)
- prevCkpt.epochStart + lockDuration <= upcomingEpoch (AuraLocker.sol#531)
- nextEpoch > prevCkpt.epochStart (AuraLocker.sol#542)
AuraLocker.getPastVotes(address,uint256) (AuraLocker.sol#597-609) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(timestamp <= block.timestamp,ERC20Votes: block not yet mined) (AuraLocker.sol#598)
- votes == 0 || ckpt.epochStart + lockDuration <= epoch (AuraLocker.sol#602)
- epoch > ckpt.epochStart (AuraLocker.sol#605)
AuraLocker.getPastTotalSupply(uint256) (AuraLocker.sol#615-618) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(timestamp < block.timestamp,ERC20Votes: block not yet mined) (AuraLocker.sol#616)
AuraLocker._checkpointsLookup(AuraLocker.DelegateeCheckpoint[],uint256) (AuraLocker.sol#624-641) uses timestamp for comparisons
Dangerous comparisons:
- ckpts[mid].epochStart > epochStart (AuraLocker.sol#633)
AuraLocker.balanceAtEpochOf(uint256,address) (AuraLocker.sol#653-679) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(epochStart < block.timestamp,Epoch is in the future) (AuraLocker.sol#655)
- lockEpoch < epochStart (AuraLocker.sol#668)
- lockEpoch > cutoffEpoch (AuraLocker.sol#669)
AuraLocker.lockedBalances(address) (AuraLocker.sol#682-709) uses timestamp for comparisons
Dangerous comparisons:
- locks[i].unlockTime > block.timestamp (AuraLocker.sol#697)
AuraLocker.totalSupplyAtEpoch(uint256) (AuraLocker.sol#717-736) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(epochStart < block.timestamp,Epoch is in the future) (AuraLocker.sol#719)
- i > 0 (AuraLocker.sol#726)
- e.date == epochStart (AuraLocker.sol#728)
- e.date <= cutoffEpoch (AuraLocker.sol#730)
- _epoch > lastIndex (AuraLocker.sol#724)
AuraLocker.queueNewRewards(uint256) (AuraLocker.sol#820-846) uses timestamp for comparisons
Dangerous comparisons:
- block.timestamp >= rdata.periodFinish (AuraLocker.sol#829)
- queuedRatio < newRewardRatio (AuraLocker.sol#840)
AuraLocker.notifyReward(address,uint256) (AuraLocker.sol#860-875) uses timestamp for comparisons
Dangerous comparisons:
- block.timestamp >= rdata.periodFinish (AuraLocker.sol#863)
AuraMerkleDrop.withdrawExpired() (AuraMerkleDrop.sol#96-102) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(block.timestamp > expiryTime,!expired) (AuraMerkleDrop.sol#98)
AuraMerkleDrop.claim(bytes32[],uint256,bool) (AuraMerkleDrop.sol#114-143) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(block.timestamp > startTime,!started) (AuraMerkleDrop.sol#120)
- require(bool,string)(block.timestamp < expiryTime,!active) (AuraMerkleDrop.sol#121)
AuraMinter.mint(address,uint256) (AuraMinter.sol#31-34) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(block.timestamp > inflationProtectionTime,Inflation protected for now) (AuraMinter.sol#32)
AuraPenaltyForwarder.forward() (AuraPenaltyForwarder.sol#47-57) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(block.timestamp > lastDistribution + distributionDelay,!elapsed) (AuraPenaltyForwarder.sol#48)
AuraVestedEscrow.constructor(address,address,address,uint256,uint256) (AuraVestedEscrow.sol#49-67) uses timestamp for comparisons
Dangerous comparisons:
- require(bool,string)(starttime >= block.timestamp,start must be future) (AuraVestedEscrow.sol#56)
AuraVestedEscrow._totalVestedOf(address,uint256) (AuraVestedEscrow.sol#157-164) uses timestamp for comparisons
Dangerous comparisons:
- _time < startTime (AuraVestedEscrow.sol#158)

3. Boolean equality
   Description:
   A boolean comparison with a constant is not necessary. They can be directly used.
   e.g require(!hasClaimed[msg.sender],"Already claimed")

AuraMerkleDrop.claim(bytes32[],uint256,bool) (AuraMerkleDrop.sol#114-143) compares to a boolean constant:
-require(bool,string)(hasClaimed[msg.sender] == false,already claimed) (AuraMerkleDrop.sol#123)

4. Missing Inheritance

Description:
The below list of contracts can inherit from already created interfaces.

AuraBalRewardPool (AuraBalRewardPool.sol#23-221) should inherit from IRewardStaking (AuraLocker.sol#12-14)
CrvDepositorWrapper (CrvDepositorWrapper.sol#104-142) should inherit from ICrvDepositor (AuraStakingProxy.sol#10-19)

[0xMaharishi]

timestamps must be compared. generic slither output