veToken Finance contest - hansfriese's results

Summary

I see most functions don't have natspecs for parameters and I recommend to add for good understanding.

Low Risk Issues

1. They must first be approved by zero and then the actual allowance must be approved. https://github.com/code-423n4/2022-05-vetoken/tree/main/contracts/Booster.sol#L374 https://github.com/code-423n4/2022-05-vetoken/tree/main/contracts/VeAssetDepositor.sol#L162

Some tokens (USDT) do not work when changing the allowance from an existing non-zero allowance value. You need to approve zero amount first.

2. It would be good to add a require() in setFeeInfo() of Booster.sol. https://github.com/code-423n4/2022-05-vetoken/tree/main/contracts/Booster.sol#L196

The initial values of "lockFeesIncentive" and "stakerLockFeesIncentive" are 10000 and 0 so the sum equals to FEE_DENOMINATOR. After you update these fees using setFeeInfo() function, if sum of two fees are greater than FEE_DENOMINATOR, the function earmarkFees() in L576-L595 will be failed because both fees will be calculated and transferred separately from feeToken balance of current contract. That's why I think it would be good to add a require() at L195.

require(_lockFeesIncentive + _stakerLockFeesIncentive <= FEE_DENOMINATOR, "invalid fee info");

Non-critical Issues

1. Add a require() in withdrawAndUnwrap() function of BaseRewardPool.sol. https://github.com/code-423n4/2022-05-vetoken/tree/main/contracts/BaseRewardPool.sol#L239

withdraw() function() in L214 has this require() and it would be good to add same require() for this function also.

require(amount > 0, "RewardPool : Cannot withdraw 0");

2. Event should use indexed fields if there are three or more fields https://github.com/code-423n4/2022-05-vetoken/tree/main/contracts/Booster.sol#L86

1. Change storage to memory if possible contracts\Booster.sol#L351 contracts\Booster.sol#L400 contracts\Booster.sol#L497

2. Use != 0 instead of > 0 for uint variables contracts\BaseRewardPool.sol#L173 contracts\BaseRewardPool.sol#L196 contracts\BaseRewardPool.sol#L215 contracts\Booster.sol#L517 contracts\Booster.sol#L526 contracts\Booster.sol#L541 contracts\Booster.sol#L551 contracts\Booster.sol#L562 contracts\Booster.sol#L586 contracts\Booster.sol#L590 contracts\VE3DRewardPool.sol#L210 contracts\VE3DRewardPool.sol#L234 contracts\VE3DRewardPool.sol#L253 contracts\VeAssetDepositor.sol#L89 contracts\VeAssetDepositor.sol#L117 contracts\VeAssetDepositor.sol#L132 contracts\VeAssetDepositor.sol#L138 contracts\VoterProxy.sol#L100

3. Use ++i instead of i++, i+=1 contracts\BaseRewardPool.sol#L176 contracts\BaseRewardPool.sol#L199 contracts\BaseRewardPool.sol#L218 contracts\BaseRewardPool.sol#L245 contracts\BaseRewardPool.sol#L282 contracts\Booster.sol#L329 contracts\VE3DRewardPool.sol#L148 contracts\VE3DRewardPool.sol#L214 contracts\VE3DRewardPool.sol#L238 contracts\VE3DRewardPool.sol#L257 contracts\VE3DRewardPool.sol#L281 contracts\VE3DRewardPool.sol#L326 contracts\VoterProxy.sol#L217

4. Use if(flag) instead of if(flag == true), if(!flag) instead of if(flag == false) contracts\VoterProxy.sol#L70 contracts\VoterProxy.sol#L110 contracts\VoterProxy.sol#L113 contracts\Booster.sol#L352 contracts\Booster.sol#L498 contracts\VoterProxy.sol#L93 contracts\VoterProxy.sol#L96

5. An array’s length should be cached to save gas in for-loops contracts\BaseRewardPool.sol#L176 contracts\BaseRewardPool.sol#L199 contracts\BaseRewardPool.sol#L218 contracts\BaseRewardPool.sol#L245 contracts\BaseRewardPool.sol#L282 contracts\Booster.sol#L329 contracts\VE3DRewardPool.sol#L148 contracts\VE3DRewardPool.sol#L281 contracts\VoterProxy.sol#L217