QA Report

[code423n4]

1. Emit events for sensitive variable updates.

It would be great to emit an event before or after updating sensitive variables to make easy to track updates by community.

VE3Token.sol/setOperator()
VeTokenMinter.sol / addOperator()
VeTokenMinter.sol / removeOperator()
VE3DRewardPool.sol / addOperator()
VE3DRewardPool.sol / removeOperator()
RewardFactory.sol / addOperator()
RewardFactory.sol / removeOperator()
RewardFactory.sol / setAccess()
StashFactory / addOperator()
StashFactory / removeOperator()

Recommendation
Emit events before or after updating sensitive varaibles.

2. OZ ERC20.safeApprove() function has been depreciated.

safeApprove() function of Openzeppelin’s ERC20 have been depreciated.
OpenZeppelin/openzeppelin-contracts#2268
OpenZeppelin/openzeppelin-contracts#2219
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VE3DRewardPool.sol#L287
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VE3DRewardPool.sol#L288
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VE3DRewardPool.sol#L301
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VE3DRewardPool.sol#L305
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/Booster.sol#L374

Recommendation:
Use safeIncreaseAllowance or safeDecreaseAllowance functions instead.

3. It looks like maxTime of VeAssetDepositor should be public
   Now, the maxTime of VeAssetDepositor.sol has been declared as a private, but I think this should be public.

Recommendation:
Check this variable again.

4. Emit wrong event in stakeFor function of VE3DRewardPool.sol contract.

https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VE3DRewardPool.sol#L249
In this line, the msg.sender is a VeAssetDepositor.sol contract or any other address which is different from user’s address.

Recommendation:
Emit event with actual user’s address.

5. Some features of Booster.sol can be implemented by using OZ contracts.

• There is owner variable and setOwner function, this can be implemented by inheriting OpenZeppelin Ownable contract.
• There is isShutdown variable and shutdownSystem function, this can be implemented by inheriting OpenZeppelin Pausable contract.

6. setRewardContracts of Booster.sol function could emit event without changes.
   setRewardContracts set values only if lockRewards is address(0), but event is emitted per each function call.

So if we use subgraph or anything else, it could lead to track wrong data.

Recommendation:
Emit RewardcontractsUpdated event inside condition.

7. Some contracts didn’t inherit interfaces correctly.
   ITokenFactory is an interface of TokenFactory contract. However TokenFactory did not inherit ITokenFactory. This lead unexpected issue – ex. Interface has function, but implementation contract did not implement it.

IRewardFactory and RewardFactory are same case.

Recommendation:
Inherit interfaces, and override functions to avoid any unexpected issues in the future.

[GalloDaSballo]

Emit events for sensitive variable updates.

NC

OZ ERC20.safeApprove() function has been depreciated.

NC because of the code working properly

It looks like maxTime of VeAssetDepositor should be public

Valid refactor, value can still be retrieved via looking at constructor (or storage at)

Emit wrong event in stakeFor function of VE3DRewardPool.sol contract., setRewardContracts of Booster.sol function could emit event without changes.

NC

Some features of Booster.sol can be implemented by using OZ contracts.

Disagree, code would do the same thing, and would cost more gas

Some contracts didn’t inherit interfaces correctly.

Valid Refactor

2R 3 NC