QA Report

[code423n4]

Event missing indexed field

Each event can have up to 3 indexed fields. Destination address of the withdraw() function can be made indexed.

Lines of code

https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VeTokenMinter.sol#L24

Use modifiers only for checks

The code inside a modifier is usually executed before the function body, so any state changes or external calls will violate the Checks-Effects-Interactions pattern. Moreover, these statements may also remain unnoticed by the developer, as the code for modifier may be far from the function declaration.
https://consensys.net/blog/developers/solidity-best-practices-for-smart-contract-security/#:~:text=Use%20modifiers%20only%20for%20checks

Lines of code

https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VE3DRewardPool.sol#L146

Missing non-zero address checks when setting priviliged roles

It is a good practice to include non-zero address check especially when updating important addresses.
I suggest to include non-zero address checks when setting addresses such as owner, fee manager, pool manager, etc.

Lines of code

https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/Booster.sol#L123
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/Booster.sol#L129
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/Booster.sol#L135
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/Booster.sol#L123
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VoterProxy.sol#L62
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VoterProxy.sol#L77

It would be better to make owner update as a two-step process

setOwner() is called by the current owner to update the owner address. It can be a better approach to follow a 2-step process when updating such priviliged addresses
First transaction proposes the pending owner address, second transaction which can only be called by the proposed address accepts the role.

Lines of code

https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/Booster.sol#L123
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VoterProxy.sol#L62

Missing events on onlyOwner operations

Functions that are only executable by privileged users (e.g. onlyOwner) and have an impact (e.g. financial, trust) on other users should emit events.

Lines of code

https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VeTokenMinter.sol#L41
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VeTokenMinter.sol#L32
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VeTokenMinter.sol#L36
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VE3DRewardPool.sol#L107
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VE3DRewardPool.sol#L114
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VE3DRewardPool.sol#L118

Missing non-zero address checks for token transfers

Tokens would be burned if sent to zero address accidentally. Therefore, it is a good practice to include non-zero address checks for token transfers.

Lines of code

https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VE3DRewardPool.sol#L233
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VeTokenMinter.sol#L48
https://github.com/code-423n4/2022-05-vetoken/blob/main/contracts/VeTokenMinter.sol#L77

[GalloDaSballo]

Event missing indexed field

Valid NC

Use modifiers only for checks

Hard to give you credit when the contract is the SNX Staking contract which is basically used by every protocol

Missing non-zero address checks when setting priviliged roles, Missing non-zero address checks for token transfers

Valid Low

It would be better to make owner update as a two-step process

Valid NC

## Missing events on onlyOwner operations
NC

Short and sweet

1L, 3NC