QA Report #183

code423n4 · 2022-06-02T16:52:17Z

Summary

Low Risk Issues

	Issue	Instances
1	`getAPY()` returns the wrong answer during leap years	1
2	Change in behavior from Convex	1
3	`safeApprove()` is deprecated	12
4	Missing checks for `address(0x0)` when assigning values to `address` state variables	26

Total: 40 instances over 4 issues

Non-critical Issues

	Issue	Instances
1	Adding a `return` statement when the function defines a named return variable, is redundant	1
2	`public` functions not called by the contract should be declared `external` instead	8
3	`constant`s should be defined rather than using magic numbers	10
4	Numeric values having to do with time should use time units for readability	1
5	Large multiples of ten should use scientific notation (e.g. `1e6`) rather than decimal literals (e.g. `1000000`), for readability	1
6	Missing event for critical parameter change	5
7	Use a more recent version of solidity	6
8	Constant redefined elsewhere	10
9	Inconsistent spacing in comments	17
10	Typos	8
11	File is missing NatSpec	3
12	NatSpec is incomplete	1
13	Event is missing `indexed` fields	20
14	Avoid the use of sensitive terms	1

Total: 92 instances over 14 issues

Low Risk Issues

1. `getAPY()` returns the wrong answer during leap years

There are more blocks in a year during a leap year. Using a static value for the number of blocks in a year will eventually lead to the wrong answer

There is 1 instance of this issue:

File: contracts/BaseRewardPool.sol   #1

343      function getAPY() external view returns (uint256) {
344          return rewardRate.mul(BLOCKS_PER_YEAR).mul(1e18).div(totalSupply());
345:     }

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/BaseRewardPool.sol#L343-L345

2. Change in behavior from Convex

In the convex version of this function, the function returns the actual balanceOf() regardless of whether there was an exception or not. It may be confusing to users trying to port their code to have to account for this behavior, especially with the lack of comments about the difference

There is 1 instance of this issue:

File: contracts/VoterProxy.sol   #1

227          uint256 _balance = 0;
228  
229          if (escrowModle == IVoteEscrow.EscrowModle.PICKLE) {
230              try IGauge(_gauge).getReward() {} catch {
231                  return _balance;
232              }
233          } else if (
234              escrowModle == IVoteEscrow.EscrowModle.CURVE ||
235              escrowModle == IVoteEscrow.EscrowModle.RIBBON
236          ) {
237              try ITokenMinter(minter).mint(_gauge) {} catch {
238                  return _balance;
239              }
240          } else if (escrowModle == IVoteEscrow.EscrowModle.IDLE) {
241              try ITokenMinter(minter).distribute(_gauge) {} catch {
242                  return _balance;
243              }
244          } else if (escrowModle == IVoteEscrow.EscrowModle.ANGLE) {
245              try IGauge(_gauge).claim_rewards() {} catch {
246                  return _balance;
247              }
248          }
249  
250:         _balance = IERC20(veAsset).balanceOf(address(this));

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VoterProxy.sol#L227-L250

3. `safeApprove()` is deprecated

Deprecated in favor of safeIncreaseAllowance() and safeDecreaseAllowance(). If only setting the initial allowance to the value that means infinite, safeIncreaseAllowance() can be used instead

There are 12 instances of this issue:

File: contracts/VE3DRewardPool.sol

287:                  IERC20(_rewardToken).safeApprove(rewardTokenInfo[_rewardToken].veAssetDeposits, 0);

288:                  IERC20(_rewardToken).safeApprove(

301:                      IERC20(rewardTokenInfo[_rewardToken].ve3Token).safeApprove(

305:                      IERC20(rewardTokenInfo[_rewardToken].ve3Token).safeApprove(

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VE3DRewardPool.sol#L287

File: contracts/VoterProxy.sol

101:              IERC20(_token).safeApprove(_gauge, 0);

102:              IERC20(_token).safeApprove(_gauge, balance);

152:          IERC20(veAsset).safeApprove(escrow, 0);

153:          IERC20(veAsset).safeApprove(escrow, _value);

160:          IERC20(veAsset).safeApprove(escrow, 0);

161:          IERC20(veAsset).safeApprove(escrow, _value);

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VoterProxy.sol#L101

File: contracts/VeAssetDepositor.sol

162:              IERC20(minter).safeApprove(_stakeAddress, _amount);

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeAssetDepositor.sol#L162

File: contracts/Booster.sol

374:              IERC20(token).safeApprove(rewardContract, _amount);

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/Booster.sol#L374

4. Missing checks for `address(0x0)` when assigning values to `address` state variables

There are 26 instances of this issue:

File: contracts/VE3DRewardPool.sol

99:           rewardManager = rewardManager_;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VE3DRewardPool.sol#L99

File: contracts/VoterProxy.sol

50:           veAsset = _veAsset;

51:           escrow = _escrow;

52:           gaugeProxy = _gaugeProxy;

54:           minter = _minter;

64:           owner = _owner;

74:           operator = _operator;

80:           depositor = _depositor;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VoterProxy.sol#L50

File: contracts/VeAssetDepositor.sol

45:           staker = _staker;

46:           minter = _minter;

47:           veAsset = _veAsset;

48:           escrow = _escrow;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeAssetDepositor.sol#L45

File: contracts/BaseRewardPool.sol

105:          operator = operator_;

106:          rewardManager = rewardManager_;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/BaseRewardPool.sol#L105

File: contracts/Booster.sol

111:          staker = _staker;

116:          minter = _minter;

117:          veAsset = _veAsset;

118:          feeDistro = _feeDistro;

131:          feeManager = _feeM;

137:          poolManager = _poolM;

152:              rewardFactory = _rfactory;

153:              tokenFactory = _tfactory;

159:          stashFactory = _sfactory;

164:          rewardArbitrator = _arb;

184:              lockRewards = _rewards;

215:              feeToken = _feeToken;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/Booster.sol#L111

Non-critical Issues

1. Adding a `return` statement when the function defines a named return variable, is redundant

There is 1 instance of this issue:

File: contracts/VoterProxy.sol   #1

119:          return balance;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VoterProxy.sol#L119

2. `public` functions not called by the contract should be declared `external` instead

Contracts are allowed to override their parents' functions and change the visibility from external to public.

There are 8 instances of this issue:

File: contracts/VE3DRewardPool.sol

114:      function addOperator(address _newOperator) public onlyOwner {

118:      function removeOperator(address _operator) public onlyOwner {

233:      function stakeFor(address _for, uint256 _amount) public updateReward(_for) {

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VE3DRewardPool.sol#L114

File: contracts/VoterProxy.sol

199:      function isValidSignature(bytes32 _hash, bytes memory) public view returns (bytes4) {

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VoterProxy.sol#L199

File: contracts/VeTokenMinter.sol

32:       function addOperator(address _newOperator) public onlyOwner {

36:       function removeOperator(address _operator) public onlyOwner {

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeTokenMinter.sol#L32

File: contracts/BaseRewardPool.sol

195:      function stakeFor(address _for, uint256 _amount) public updateReward(_for) returns (bool) {

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/BaseRewardPool.sol#L195

File: contracts/Booster.sol

434:      function withdrawAll(uint256 _pid) public returns (bool) {

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/Booster.sol#L434

3. `constant`s should be defined rather than using magic numbers

There are 10 instances of this issue:

File: contracts/VE3DRewardPool.sol

/// @audit 1e18
180:                      .mul(1e18)

/// @audit 1e18
193:                  .div(1e18)

/// @audit 1000
358:          uint256 queuedRatio = currentAtNow.mul(1000).div(_rewards);

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VE3DRewardPool.sol#L180

File: contracts/VoterProxy.sol

/// @audit 0xffffffff
203:              return 0xffffffff;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VoterProxy.sol#L203

File: contracts/VeAssetDepositor.sol

/// @audit 30
62:           if (_lockIncentive >= 0 && _lockIncentive <= 30) {

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeAssetDepositor.sol#L62

File: contracts/VeTokenMinter.sol

/// @audit 1000
28:           totalCliffs = 1000;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeTokenMinter.sol#L28

File: contracts/BaseRewardPool.sol

/// @audit 1e18
158:                  lastTimeRewardApplicable().sub(lastUpdateTime).mul(rewardRate).mul(1e18).div(

/// @audit 1e18
168:                  .div(1e18)

/// @audit 1000
315:          uint256 queuedRatio = currentAtNow.mul(1000).div(_rewards);

/// @audit 1e18
344:          return rewardRate.mul(BLOCKS_PER_YEAR).mul(1e18).div(totalSupply());

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/BaseRewardPool.sol#L158

4. Numeric values having to do with time should use time units for readability

There are units for seconds, minutes, hours, days, and weeks

There is 1 instance of this issue:

File: contracts/VeAssetDepositor.sol   #1

/// @audit 86400
18:       uint256 private constant WEEK = 7 * 86400;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeAssetDepositor.sol#L18

5. Large multiples of ten should use scientific notation (e.g. `1e6`) rather than decimal literals (e.g. `1000000`), for readability

There is 1 instance of this issue:

File: contracts/VeTokenMinter.sol   #1

15:       uint256 public constant maxSupply = 30 * 1000000 * 1e18; //30mil

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeTokenMinter.sol#L15

6. Missing event for critical parameter change

There are 5 instances of this issue:

File: contracts/VoterProxy.sol

62        function setOwner(address _owner) external {
63            require(msg.sender == owner, "!auth");
64            owner = _owner;
65:       }

67        function setOperator(address _operator) external {
68            require(msg.sender == owner, "!auth");
69            require(
70                operator == address(0) || IDeposit(operator).isShutdown() == true,
71                "needs shutdown"
72            );
73    
74            operator = _operator;
75:       }

77        function setDepositor(address _depositor) external {
78            require(msg.sender == owner, "!auth");
79    
80            depositor = _depositor;
81:       }

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VoterProxy.sol#L62-L65

File: contracts/VeTokenMinter.sol

41        function updateveAssetWeight(address veAssetOperator, uint256 newWeight) external onlyOwner {
42            require(operators.contains(veAssetOperator), "not an veAsset operator");
43            totalWeight -= veAssetWeights[veAssetOperator];
44            veAssetWeights[veAssetOperator] = newWeight;
45            totalWeight += newWeight;
46:       }

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeTokenMinter.sol#L41-L46

File: contracts/Booster.sol

193       function setFeeInfo(uint256 _lockFeesIncentive, uint256 _stakerLockFeesIncentive) external {
194           require(msg.sender == feeManager, "!auth");
195   
196           lockFeesIncentive = _lockFeesIncentive;
197           stakerLockFeesIncentive = _stakerLockFeesIncentive;
198   
199           address _feeToken = IFeeDistro(feeDistro).token();
200           if (feeToken != _feeToken) {
201               //create a new reward contract for the new token
202               lockFees = IRewardFactory(rewardFactory).CreateTokenRewards(_feeToken, lockRewards);
203   
204               if (_feeToken != veAsset) {
205                   IRewards(stakerLockRewards).addReward(
206                       _feeToken,
207                       address(0),
208                       address(0),
209                       address(0),
210                       address(this),
211                       false
212                   );
213               }
214   
215               feeToken = _feeToken;
216           }
217:      }

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/Booster.sol#L193-L217

7. Use a more recent version of solidity

Use a solidity version of at least 0.8.13 to get the ability to use using for with a list of free functions

There are 6 instances of this issue:

File: contracts/VE3DRewardPool.sol

2:    pragma solidity 0.8.7;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VE3DRewardPool.sol#L2

File: contracts/VoterProxy.sol

2:    pragma solidity 0.8.7;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VoterProxy.sol#L2

File: contracts/VeAssetDepositor.sol

2:    pragma solidity 0.8.7;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeAssetDepositor.sol#L2

File: contracts/VeTokenMinter.sol

2:    pragma solidity 0.8.7;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeTokenMinter.sol#L2

File: contracts/BaseRewardPool.sol

2:    pragma solidity 0.8.7;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/BaseRewardPool.sol#L2

File: contracts/Booster.sol

2:    pragma solidity 0.8.7;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/Booster.sol#L2

8. Constant redefined elsewhere

Consider defining in only one contract so that values cannot become out of sync when only one location is updated. A cheap way to store constants in a single location is to create an internal constant in a library. If the variable is a local cache of another contract's value, consider making the cache variable internal or private, which will require external users to query the contract with the source of truth, so that callers don't get out of sync.

There are 10 instances of this issue:

File: contracts/VeAssetDepositor.sol

/// @audit seen in /var/tmp/hh/contracts/VE3DRewardPool.sol 
21:       uint256 public constant FEE_DENOMINATOR = 10000;

/// @audit seen in /var/tmp/hh/contracts/VoterProxy.sol 
23:       address public immutable veAsset;

/// @audit seen in /var/tmp/hh/contracts/VoterProxy.sol 
24:       address public immutable escrow;

/// @audit seen in /var/tmp/hh/contracts/VoterProxy.sol 
27:       address public immutable minter;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeAssetDepositor.sol#L21

File: contracts/BaseRewardPool.sol

/// @audit seen in /var/tmp/hh/contracts/VE3DRewardPool.sol 
57:       uint256 public constant duration = 7 days;

/// @audit seen in /var/tmp/hh/contracts/VE3DRewardPool.sol 
73:       uint256 public constant newRewardRatio = 830;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/BaseRewardPool.sol#L57

File: contracts/Booster.sol

/// @audit seen in /var/tmp/hh/contracts/VeAssetDepositor.sol 
34:       uint256 public constant FEE_DENOMINATOR = 10000;

/// @audit seen in /var/tmp/hh/contracts/VeAssetDepositor.sol 
42:       address public immutable staker;

/// @audit seen in /var/tmp/hh/contracts/VeAssetDepositor.sol 
43:       address public immutable minter;

/// @audit seen in /var/tmp/hh/contracts/VeAssetDepositor.sol 
44:       address public immutable veAsset;

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/Booster.sol#L34

9. Inconsistent spacing in comments

Some lines use // x and some use //x. The instances below point out the usages that don't follow the majority, within each file

There are 17 instances of this issue:

File: contracts/VE3DRewardPool.sol

68:       // reward token => reward token info

70:       // list of reward tokens

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VE3DRewardPool.sol#L68

File: contracts/VoterProxy.sol

122:      // Withdraw partial funds

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VoterProxy.sol#L122

File: contracts/Booster.sol

23:       // ve3Token reward pool

25:       // veToken reward pool

27:       // veToken locking reward pool xVE3D

29:       // caller reward

31:       // platoform fee

53:       address public stakerLockRewards; // veToken lock rewards xVE3D

121:      /// SETTER SECTION ///

192:      // Set reward token and claim contract, get from Curve's registry

249:      /// END SETTER SECTION ///

295:          //   voteproxy so it can grab the incentive tokens off the contract after claiming rewards

296:          //   reward factory so that stashes can make new extra reward contracts if a new incentive is added to the gauge

323:      //  unstake and pull all lp tokens to this address

324:      //  only allow withdrawals

409:          // if shutdown tokens will be in this contract

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/Booster.sol#L23

10. Typos

There are 8 instances of this issue:

File: contracts/VoterProxy.sol

/// @audit modle -> model
234:             escrowModle == IVoteEscrow.EscrowModle.CURVE ||

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VoterProxy.sol#L234

File: contracts/VE3DRewardPool.sol

/// @audit dont
204:          //fees dont apply until whitelist+veVeAsset lock begins so will report

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VE3DRewardPool.sol#L204

File: contracts/VeAssetDepositor.sol

/// @audit ammount
93:           //increase ammount

/// @audit isnt
126:      //the vetoken reward contract isnt costly to claim rewards

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeAssetDepositor.sol#L93

File: contracts/VeTokenMinter.sol

/// @audit 30mil
15:       uint256 public constant maxSupply = 30 * 1000000 * 1e18; //30mil

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeTokenMinter.sol#L15

File: contracts/Booster.sol

/// @audit platoform
31:       // platoform fee

/// @audit seperate
363:          //some gauges claim rewards when depositing, stash them in a seperate contract until next claim

/// @audit seperate
414:          //some gauges claim rewards when withdrawing, stash them in a seperate contract until next claim

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/Booster.sol#L31

11. File is missing NatSpec

There are 3 instances of this issue:

File: contracts/VE3DRewardPool.sol (various lines)   #1

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VE3DRewardPool.sol

File: contracts/VeAssetDepositor.sol (various lines)   #2

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeAssetDepositor.sol

File: contracts/BaseRewardPool.sol (various lines)   #3

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/BaseRewardPool.sol

12. NatSpec is incomplete

There is 1 instance of this issue:

File: contracts/VoterProxy.sol   #1

/// @audit Missing: '@param bytes'
191       /**
192        * @notice  Verifies that the hash is valid
193        * @dev     Snapshot Hub will call this function when a vote is submitted using
194        *          snapshot.js on behalf of this contract. Snapshot Hub will call this
195        *          function with the hash and the signature of the vote that was cast.
196        * @param _hash Hash of the message that was sent to Snapshot Hub to cast a vote
197        * @return EIP1271 magic value if the signature is value
198        */
199:      function isValidSignature(bytes32 _hash, bytes memory) public view returns (bytes4) {

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VoterProxy.sol#L191-L199

13. Event is missing `indexed` fields

Each event should use three indexed fields if there are three or more fields

There are 20 instances of this issue:

File: contracts/VE3DRewardPool.sol

91:       event RewardAdded(uint256 reward);

92:       event Staked(address indexed user, uint256 amount);

93:       event Withdrawn(address indexed user, uint256 amount);

94:       event RewardPaid(address indexed user, uint256 reward);

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VE3DRewardPool.sol#L91

File: contracts/VoterProxy.sol

39:       event VoteSet(bytes32 hash, bool valid);

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VoterProxy.sol#L39

File: contracts/VeAssetDepositor.sol

33:       event FeesUpdated(uint256 lockIncentive);

34:       event InitialLockCreated(uint256 veAssetBalanceStaker, uint256 unlockInWeeks);

35:       event LockUpdated(uint256 veAssetBalanceStaker, uint256 unlockInWeeks);

36:       event Deposited(address indexed user, uint256 amount, bool lock);

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VeAssetDepositor.sol#L33

File: contracts/VeTokenMinter.sol

24:       event Withdraw(address destination, uint256 amount);

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

File: contracts/BaseRewardPool.sol

81:       event RewardAdded(uint256 reward);

82:       event Staked(address indexed user, uint256 amount);

83:       event Withdrawn(address indexed user, uint256 amount);

84:       event RewardPaid(address indexed user, uint256 reward);

87        event RewardUpdated(
88            address indexed user,
89            uint256 reward,
90            uint256 rewardPerTokenStored,
91            uint256 lastUpdateTime
92:       );

93:       event Donated(uint256 queuedRewards);

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/BaseRewardPool.sol#L81

File: contracts/Booster.sol

73:       event Deposited(address indexed user, uint256 indexed poolid, uint256 amount);

74:       event Withdrawn(address indexed user, uint256 indexed poolid, uint256 amount);

86        event FeesUpdated(
87            uint256 lockFees,
88            uint256 stakerFees,
89            uint256 stakerLockFee,
90            uint256 callerFees,
91            uint256 platform
92:       );

102:      event Voted(uint256 indexed voteId, address indexed votingAddress, bool support);

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/Booster.sol#L73

14. Avoid the use of sensitive terms

Use alternative variants, e.g. allowlist/denylist instead of whitelist/blacklist

There is 1 instance of this issue:

File: contracts/VE3DRewardPool.sol   #1

204:         //fees dont apply until whitelist+veVeAsset lock begins so will report

https://github.com/code-423n4/2022-05-vetoken/blob/2d7cd1f6780a9bcc8387dea8fecfbd758462c152/contracts/VE3DRewardPool.sol#L204

The text was updated successfully, but these errors were encountered:

jetbrain10 · 2022-06-15T21:12:03Z

high quality.

GalloDaSballo · 2022-07-09T17:55:27Z

getAPY() returns the wrong answer during leap years

Valid Low

2. Change in behavior from Convex

Return value from all external calls in the try catch is ignored (the try clause is empty and the call doesn't declare any return value), I think this finding is invalid

3. safeApprove() is deprecated

Valid NC

4. Missing checks for address(0x0) when assigning values to address state variables

Valid Low

1. Adding a return statement when the function defines a named return variable, is redundant

Valid Refactoring

2. public functions not called by the contract should be declared external instead

Ref

3. constants should be defined rather than using magic numbers

Ref

4. Numeric values having to do with time should use time units for readability

NC

5. Large multiples of ten should use scientific notation (e.g. 1e6) rather than decimal literals (e.g. 1000000), for readability

Ref

6. Missing event for critical parameter change

Valid NC

7. Use a more recent version of solidity

NC

## 8. Constant redefined elsewhere
Valid Refactoring

9. Inconsistent spacing in comments

NC

10. Typos

Valid NC

11. File is missing NatSpec / Incomplete

Valid NC

13. Event is missing indexed fields

The linked events have less than 3 parameters (only 1 exception), invalid

14. Avoid the use of sensitive terms

Valid NC

GalloDaSballo · 2022-07-09T17:56:09Z

2L, 5R, 7NC

GalloDaSballo · 2022-08-02T21:25:37Z

For Report:

L01 - getAPY() returns the wrong answer during leap years
L02 - Missing checks for address(0x0) when assigning values to address state variables

NC01 - safeApprove() is deprecated
NC02 - Adding a return statement when the function defines a named return variable, is redundant
NC03 - public functions not called by the contract should be declared external instead
NC04 - constants should be defined rather than using magic numbers
NC05 - Numeric values having to do with time should use time units for readability
NC06 - Missing event for critical parameter change
NC07 - Use a more recent version of solidity
NC08 - Constant redefined elsewhere
NC09 - Inconsistent spacing in comments
NC10 - Typos
NC11 - File is missing NatSpec / Incomplete
NC 12 - Large multiples of ten should use scientific notation (e.g. 1e6) rather than decimal literals (e.g. 1000000), for readability
NC13 - Avoid the use of sensitive terms

code423n4 added bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax labels Jun 2, 2022

code423n4 added a commit that referenced this issue Jun 2, 2022

IllIllI issue #183

b492d31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

QA Report #183

QA Report #183

code423n4 commented Jun 2, 2022

jetbrain10 commented Jun 15, 2022

GalloDaSballo commented Jul 9, 2022 •

edited

Loading

GalloDaSballo commented Jul 9, 2022

GalloDaSballo commented Aug 2, 2022

QA Report #183

QA Report #183

Comments

code423n4 commented Jun 2, 2022

Summary

Low Risk Issues

Non-critical Issues

Low Risk Issues

1. getAPY() returns the wrong answer during leap years

2. Change in behavior from Convex

3. safeApprove() is deprecated

4. Missing checks for address(0x0) when assigning values to address state variables

Non-critical Issues

1. Adding a return statement when the function defines a named return variable, is redundant

2. public functions not called by the contract should be declared external instead

3. constants should be defined rather than using magic numbers

4. Numeric values having to do with time should use time units for readability

5. Large multiples of ten should use scientific notation (e.g. 1e6) rather than decimal literals (e.g. 1000000), for readability

6. Missing event for critical parameter change

7. Use a more recent version of solidity

8. Constant redefined elsewhere

9. Inconsistent spacing in comments

10. Typos

11. File is missing NatSpec

12. NatSpec is incomplete

13. Event is missing indexed fields

14. Avoid the use of sensitive terms

jetbrain10 commented Jun 15, 2022

GalloDaSballo commented Jul 9, 2022 • edited Loading

getAPY() returns the wrong answer during leap years

2. Change in behavior from Convex

3. safeApprove() is deprecated

4. Missing checks for address(0x0) when assigning values to address state variables

1. Adding a return statement when the function defines a named return variable, is redundant

2. public functions not called by the contract should be declared external instead

3. constants should be defined rather than using magic numbers

4. Numeric values having to do with time should use time units for readability

5. Large multiples of ten should use scientific notation (e.g. 1e6) rather than decimal literals (e.g. 1000000), for readability

6. Missing event for critical parameter change

7. Use a more recent version of solidity

9. Inconsistent spacing in comments

10. Typos

11. File is missing NatSpec / Incomplete

13. Event is missing indexed fields

14. Avoid the use of sensitive terms

GalloDaSballo commented Jul 9, 2022

GalloDaSballo commented Aug 2, 2022

1. `getAPY()` returns the wrong answer during leap years

3. `safeApprove()` is deprecated

4. Missing checks for `address(0x0)` when assigning values to `address` state variables

1. Adding a `return` statement when the function defines a named return variable, is redundant

2. `public` functions not called by the contract should be declared `external` instead

3. `constant`s should be defined rather than using magic numbers

5. Large multiples of ten should use scientific notation (e.g. `1e6`) rather than decimal literals (e.g. `1000000`), for readability

13. Event is missing `indexed` fields

GalloDaSballo commented Jul 9, 2022 •

edited

Loading