search

code-423n4 / 2022-05-velodrome-findings

0 stars 0 forks source link

QA Report #128

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Summary

Low Risk Issues

Issue Instances
1 Math.max(<x>,0) used with int cast to uint 4
2 approveMax variable causes problems 2
3 Front-runable initializer 1
4 Incorrect comments 1
5 Only a billion checkpoints available 1
6 Unsafe use of transfer()/transferFrom() with IERC20 2
7 require() should be used instead of assert() 18
8 _safeMint() should be used rather than _mint() wherever possible 1
9 Missing checks for address(0x0) when assigning values to address state variables 33
10 Open TODOs 4

Total: 67 instances over 10 issues

Non-critical Issues

Issue Instances
1 Use two-phase ownership transfers 4
2 Avoid the use of sensitive terms 3
3 Consider addings checks for signature malleability 2
4 Return values of approve() not checked 2
5 require()/revert() statements should have descriptive reason strings 93
6 public functions not called by the contract should be declared external instead 9
7 Non-assembly method available 1
8 constants should be defined rather than using magic numbers 95
9 Redundant cast 4
10 Numeric values having to do with time should use time units for readability 7
11 Large multiples of ten should use scientific notation (e.g. 1e6) rather than decimal literals (e.g. 1000000), for readability 3
12 Missing event for critical parameter change 16
13 Expressions for constant values such as a call to keccak256(), should use immutable rather than constant 2
14 Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18) 3
15 Constant redefined elsewhere 6
16 Variable names that consist of all capital letters should be reserved for constant/immutable variables 1
17 Typos 7
18 File is missing NatSpec 13
19 NatSpec is incomplete 13
20 Event is missing indexed fields 31
21 Not using the named return variables anywhere in the function is confusing 12

Total: 327 instances over 21 issues

Low Risk Issues

1. Math.max(<x>,0) used with int cast to uint

The code casts an int to a uint before passing it to Math.max(). It seems as though the Math.max() call is attempting to prevent values from being negative, but since the int is being cast to uint, the value will never be negative, and instead will overflow if either the multiplication involving the slope and timestamp is positive. I wasn't able to find a scenario where this is the case, but this seems very dangerous, and the Math.max() call is sending misleading signals, so I suggest moving it to inside the cast to uint

There are 4 instances of this issue:

File: contracts/contracts/RewardsDistributor.sol   #1

139:         return Math.max(uint(int256(pt.bias - pt.slope * (int128(int256(_timestamp - pt.ts))))), 0);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/RewardsDistributor.sol#L139

File: contracts/contracts/RewardsDistributor.sol   #2

158:                 ve_supply[t] = Math.max(uint(int256(pt.bias - pt.slope * dt)), 0);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/RewardsDistributor.sol#L158

File: contracts/contracts/RewardsDistributor.sol   #3

208:                 uint balance_of = Math.max(uint(int256(old_user_point.bias - dt * old_user_point.slope)), 0);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/RewardsDistributor.sol#L208

File: contracts/contracts/RewardsDistributor.sol   #4

265:                 uint balance_of = Math.max(uint(int256(old_user_point.bias - dt * old_user_point.slope)), 0);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/RewardsDistributor.sol#L265

2. approveMax variable causes problems

The approveMax variable controls whether the liquidity value is used, or the maximum is used. If the result of that check doesn't match what was provided during signature generation, the permit() call will fail, so in the best case the variable has no effect. In the worst case it leads to incorrect state handling

There are 2 instances of this issue:

File: contracts/contracts/Router.sol   #1

290              uint value = approveMax ? type(uint).max : liquidity;
291:             IPair(pair).permit(msg.sender, address(this), value, deadline, v, r, s);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Router.sol#L290-L291

File: contracts/contracts/Router.sol   #2

308          uint value = approveMax ? type(uint).max : liquidity;
309:         IPair(pair).permit(msg.sender, address(this), value, deadline, v, r, s);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Router.sol#L308-L309

3. Front-runable initializer

There is nothing preventing another account from calling the initializer before the contract owner. In the best case, the owner is forced to waste gas and re-deploy. In the worst case, the owner does not notice that his/her call reverts, and everyone starts using a contract under the control of an attacker

There is 1 instance of this issue:

File: contracts/contracts/Bribe.sol   #1

30     function setGauge(address _gauge) external {
31       require(gauge == address(0), "gauge already set");
32       gauge = _gauge;
33:    }

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L30-L33

4. Incorrect comments

The WEEK variable is used to calculate a specific timestamp. However, if there's a leap year, then the calculation will shift, and the timestamp will no longer be 00:00 UTC

There is 1 instance of this issue:

File: contracts/contracts/Minter.sol   #1

14:      uint internal constant WEEK = 86400 * 7; // allows minting once per week (reset every Thursday 00:00 UTC)

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Minter.sol#L14

5. Only a billion checkpoints available

A user can only have a billion checkpoints which, if the user is a DAO, may cause issues down the line, especially if the last checkpoint involved delegating and can thereafter not be undone

There is 1 instance of this issue:

File: contracts/contracts/VotingEscrow.sol   #1

535:     mapping(uint => Point[1000000000]) public user_point_history; // user -> Point[user_epoch]

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L535

6. Unsafe use of transfer()/transferFrom() with IERC20

Some tokens do not implement the ERC20 standard properly but are still accepted by most code that accepts ERC20 tokens. For example Tether (USDT)'s transfer() and transferFrom() functions on L1 do not return booleans as the specification requires, and instead have no return value. When these sorts of tokens are cast to IERC20, their function signatures do not match and therefore the calls made, revert (see this link for a test case). Use OpenZeppelin’s SafeERC20's safeTransfer()/safeTransferFrom() instead

There are 2 instances of this issue:

File: contracts/contracts/VotingEscrow.sol   #1

748:              assert(IERC20(token).transferFrom(from, address(this), _value));

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L748

File: contracts/contracts/VotingEscrow.sol   #2

861:          assert(IERC20(token).transfer(msg.sender, value));

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L861

7. require() should be used instead of assert()

Prior to solidity version 0.8.0, hitting an assert consumes the remainder of the transaction's available gas rather than returning it, as require()/revert() do. assert() should be avoided even past solidity version 0.8.0 as its documentation states that "The assert function creates an error of type Panic(uint256). ... Properly functioning code should never create a Panic, not even on invalid external input. If this happens, then there is a bug in your contract which you should fix".

There are 18 instances of this issue:

File: contracts/contracts/Router.sol

36:           assert(msg.sender == address(weth)); // only accept ETH via fallback from the WETH contract

181:                  assert(amountAOptimal <= amountADesired);

227:          assert(weth.transfer(pair, amountETH));

373:          assert(weth.transfer(pairFor(routes[0].from, routes[0].to, routes[0].stable), amounts[0]));

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Router.sol#L36

File: contracts/contracts/VotingEscrow.sol

262:          assert(_operator != msg.sender);

272:          assert(idToOwner[_tokenId] == _owner);

447:          assert(idToOwner[_tokenId] == address(0));

464:          assert(_to != address(0));

508:          assert(idToOwner[_tokenId] == _from);

748:              assert(IERC20(token).transferFrom(from, address(this), _value));

815:          assert(_isApprovedOrOwner(msg.sender, _tokenId));

819:          assert(_value > 0); // dev: need non-zero value

829:          assert(_isApprovedOrOwner(msg.sender, _tokenId));

845:          assert(_isApprovedOrOwner(msg.sender, _tokenId));

861:          assert(IERC20(token).transfer(msg.sender, value));

937:          assert(_block <= block.number);

991:          assert(_block <= block.number);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L262

File: contracts/contracts/RewardsDistributor.sol

98:           assert(msg.sender == depositor);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/RewardsDistributor.sol#L98

8. _safeMint() should be used rather than _mint() wherever possible

_mint() is discouraged in favor of _safeMint() which ensures that the recipient is either an EOA or implements IERC721Receiver. Both OpenZeppelin and solmate have versions of this function

There is 1 instance of this issue:

File: contracts/contracts/VotingEscrow.sol   #1

791:          _mint(_to, _tokenId);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L791

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

There are 33 instances of this issue:

File: contracts/contracts/Minter.sol

66:           pendingTeam = _team;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Minter.sol#L66

File: contracts/contracts/redeem/RedemptionSender.sol

22:           weve = _weve;

24:           endpoint = _endpoint;

25:           optimismReceiver = _optimismReceiver;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/redeem/RedemptionSender.sol#L22

File: contracts/contracts/redeem/RedemptionReceiver.sol

28:           endpoint = _endpoint;

53:           fantomSender = _fantomSender;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/redeem/RedemptionReceiver.sol#L28

File: contracts/contracts/Router.sol

30:           factory = _factory;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Router.sol#L30

File: contracts/contracts/Gauge.sol

97:           stake = _stake;

98:           bribe = _bribe;

99:           _ve = __ve;

100:          voter = _voter;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L97

File: contracts/contracts/VotingEscrow.sol

88:           token = token_addr;

1061:         voter = _voter;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L88

File: contracts/contracts/VeloGovernor.sol

41:           team = newTeam;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VeloGovernor.sol#L41

File: contracts/contracts/RewardsDistributor.sol

54:           token = _token;

55:           voting_escrow = _voting_escrow;

320:          depositor = _depositor;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/RewardsDistributor.sol#L54

File: contracts/contracts/PairFees.sol

15:           token0 = _token0;

16:           token1 = _token1;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/PairFees.sol#L15

File: contracts/contracts/Velo.sol

28:           minter = _minter;

33:           redemptionReceiver = _receiver;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Velo.sol#L28

File: contracts/contracts/Bribe.sol

32:       gauge = _gauge;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L32

File: contracts/contracts/factories/GaugeFactory.sol

14:           pairFactory = _pairFactory;

19:           team = _team;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/factories/GaugeFactory.sol#L14

File: contracts/contracts/factories/PairFactory.sol

42:           pendingPauser = _pauser;

57:           pendingFeeManager = _feeManager;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/factories/PairFactory.sol#L42

File: contracts/contracts/Voter.sol

55:           _ve = __ve;

56:           factory = _factory;

58:           gaugefactory = _gauges;

59:           bribefactory = _bribes;

79:           minter = _minter;

84:           governor = _governor;

89:           emergencyCouncil = _council;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L55

10. Open TODOs

Code architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment

There are 4 instances of this issue:

File: contracts/contracts/VotingEscrow.sol   #1

314:          // TODO delegates

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L314

File: contracts/contracts/VotingEscrow.sol   #2

465:          // TODO add delegates

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L465

File: contracts/contracts/VotingEscrow.sol   #3

524:          // TODO add delegates

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L524

File: contracts/contracts/VelodromeLibrary.sol   #4

9:        IRouter internal immutable router; // TODO make modifiable?

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VelodromeLibrary.sol#L9

Non-critical Issues

1. Use two-phase ownership transfers

Consider adding a two-phase transfer, where the current owner nominates the next owner, and the next owner has to call accept*() to become the new owner. This prevents passing the ownership to an account that is unable to use it.

There are 4 instances of this issue:

File: contracts/contracts/factories/GaugeFactory.sol   #1

17       function setTeam(address _team) external {
18           require(msg.sender == team);
19           team = _team;
20:      }

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/factories/GaugeFactory.sol#L17-L20

File: contracts/contracts/VeloGovernor.sol   #2

39       function setTeam(address newTeam) external {
40           require(msg.sender == team, "not team");
41           team = newTeam;
42:      }

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VeloGovernor.sol#L39-L42

File: contracts/contracts/Voter.sol   #3

82       function setGovernor(address _governor) public {
83           require(msg.sender == governor);
84           governor = _governor;
85:      }

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L82-L85

File: contracts/contracts/Voter.sol   #4

87       function setEmergencyCouncil(address _council) public {
88           require(msg.sender == emergencyCouncil);
89           emergencyCouncil = _council;
90:      }

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L87-L90

2. Avoid the use of sensitive terms

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

There are 3 instances of this issue:

File: contracts/contracts/Voter.sol   #1

38:      mapping(address => bool) public isWhitelisted;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L38

File: contracts/contracts/Voter.sol   #2

52:      event Whitelisted(address indexed whitelister, address indexed token);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L52

File: contracts/contracts/Voter.sol   #3

178:     function whitelist(address _token) public {

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L178

3. Consider addings checks for signature malleability

Use OpenZeppelin's ECDSA contract rather than calling ecrecover() directly

There are 2 instances of this issue:

File: contracts/contracts/VotingEscrow.sol   #1

1377:        address signatory = ecrecover(digest, v, r, s);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L1377

File: contracts/contracts/Pair.sol   #2

484:         address recoveredAddress = ecrecover(digest, v, r, s);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L484

4. Return values of approve() not checked

Not all IERC20 implementations revert() when there's a failure in approve(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything

There are 2 instances of this issue:

File: contracts/contracts/RewardsDistributor.sol   #1

57:           IERC20(_token).approve(_voting_escrow, type(uint).max);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/RewardsDistributor.sol#L57

File: contracts/contracts/Voter.sol   #2

198:          IERC20(base).approve(_gauge, type(uint).max);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L198

5. require()/revert() statements should have descriptive reason strings

There are 93 instances of this issue:

File: contracts/contracts/Minter.sol

54:           require(initializer == msg.sender);

128:              require(_velo.transfer(team, _teamEmissions));

129:              require(_velo.transfer(address(_rewards_distributor), _growth));

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Minter.sol#L54

File: contracts/contracts/Router.sol

164:          require(amountADesired >= amountAMin);

165:          require(amountBDesired >= amountBMin);

245:          require(IPair(pair).transferFrom(msg.sender, pair, liquidity)); // send liquidity to pair

410:          require(token.code.length > 0);

413:          require(success && (data.length == 0 || abi.decode(data, (bool))));

417:          require(token.code.length > 0);

420:          require(success && (data.length == 0 || abi.decode(data, (bool))));

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Router.sol#L164

File: contracts/contracts/Gauge.sol

125:          require(_unlocked == 1);

174:          require(msg.sender == voter);

189:          require(msg.sender == voter);

348:          require(msg.sender == account || msg.sender == voter);

512:          require(amount > 0);

521:              require(IVotingEscrow(_ve).ownerOf(tokenId) == msg.sender);

526:              require(tokenIds[msg.sender] == tokenId);

564:              require(tokenId == tokenIds[msg.sender]);

591:          require(token != stake);

592:          require(amount > 0);

609:              require(amount > _left);

613:          require(rewardRate[token] > 0);

628:          require(rewards[i] == oldToken);

640:          require(msg.sender == bribe);

654:              require(amount > _left);

665:          require(token.code.length > 0);

668:          require(success && (data.length == 0 || abi.decode(data, (bool))));

672:          require(token.code.length > 0);

675:          require(success && (data.length == 0 || abi.decode(data, (bool))));

679:          require(token.code.length > 0);

682:          require(success && (data.length == 0 || abi.decode(data, (bool))));

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L125

File: contracts/contracts/VotingEscrow.sol

112:          require(_entered_state == _not_entered);

242:          require(owner != address(0));

244:          require(_approved != owner);

248:          require(senderIsOwner || senderIsApprovedForAll);

309:          require(_isApprovedOrOwner(_sender, _tokenId));

772:          require(_value > 0); // dev: need non-zero value

785:          require(_value > 0); // dev: need non-zero value

1060:         require(msg.sender == voter);

1065:         require(msg.sender == voter);

1070:         require(msg.sender == voter);

1075:         require(msg.sender == voter);

1080:         require(msg.sender == voter);

1086:         require(_from != _to);

1087:         require(_isApprovedOrOwner(msg.sender, _from));

1088:         require(_isApprovedOrOwner(msg.sender, _to));

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L112

File: contracts/contracts/RewardsDistributor.sol

319:          require(msg.sender == depositor);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/RewardsDistributor.sol#L319

File: contracts/contracts/PairFees.sol

20:           require(token.code.length > 0);

23:           require(success && (data.length == 0 || abi.decode(data, (bool))));

28:           require(msg.sender == pair);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/PairFees.sol#L20

File: contracts/contracts/Velo.sol

27:           require(msg.sender == minter);

32:           require(msg.sender == minter);

69:           require(msg.sender == minter);

75:           require(msg.sender == redemptionReceiver);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Velo.sol#L27

File: contracts/contracts/Bribe.sol

24:         require(_unlocked == 1);

42:         require(amount > 0);

67:       require(msg.sender == gauge);

76:       require(msg.sender == gauge);

77:       require(rewards[i] == oldToken);

84:       require(msg.sender == gauge);

93:         require(token.code.length > 0);

96:         require(success && (data.length == 0 || abi.decode(data, (bool))));

100:        require(token.code.length > 0);

103:        require(success && (data.length == 0 || abi.decode(data, (bool))));

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L24

File: contracts/contracts/factories/GaugeFactory.sol

18:           require(msg.sender == team);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/factories/GaugeFactory.sol#L18

File: contracts/contracts/factories/PairFactory.sol

41:           require(msg.sender == pauser);

46:           require(msg.sender == pendingPauser);

51:           require(msg.sender == pauser);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/factories/PairFactory.sol#L41

File: contracts/contracts/Pair.sol

111:          require(_unlocked == 1);

335:          require(!PairFactory(factory).isPaused());

522:          require(token.code.length > 0);

525:          require(success && (data.length == 0 || abi.decode(data, (bool))));

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L111

File: contracts/contracts/Voter.sol

68:           require(_unlocked == 1);

75:           require(msg.sender == minter);

83:           require(msg.sender == governor);

88:           require(msg.sender == emergencyCouncil);

93:           require(IVotingEscrow(_ve).isApprovedOrOwner(msg.sender, _tokenId));

153:                  require(votes[_tokenId][_pool] == 0);

154:                  require(_poolWeight != 0);

173:          require(IVotingEscrow(_ve).isApprovedOrOwner(msg.sender, tokenId));

174:          require(_poolVote.length == _weights.length);

179:          require(msg.sender == governor);

184:          require(!isWhitelisted[_token]);

225:          require(isGauge[msg.sender]);

226:          require(isAlive[msg.sender]); // killed gauges cannot attach tokens to themselves

232:          require(isGauge[msg.sender]);

233:          require(isAlive[msg.sender]);

238:          require(isGauge[msg.sender]);

244:          require(isGauge[msg.sender]);

286:          require(isAlive[_gauge]); // killed gauges cannot be updated

316:          require(isAlive[_gauge]); // killed gauges cannot distribute

352:          require(token.code.length > 0);

355:          require(success && (data.length == 0 || abi.decode(data, (bool))));

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L68

6. 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 9 instances of this issue:

File: contracts/contracts/redeem/RedemptionSender.sol

28        function redeemWEVE(
29            uint256 amount,
30            address zroPaymentAddress,
31:           bytes memory zroTransactionParams

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/redeem/RedemptionSender.sol#L28-L31

File: contracts/contracts/Gauge.sol

163:      function getVotingStage(uint timestamp) public pure returns (VotingStage) {

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L163

File: contracts/contracts/VotingEscrow.sol

1184      function getPastVotes(address account, uint timestamp)
1185          public
1186          view
1187:         returns (uint)

1349:     function delegate(address delegatee) public {

1354      function delegateBySig(
1355          address delegatee,
1356          uint nonce,
1357          uint expiry,
1358          uint8 v,
1359          bytes32 r,
1360:         bytes32 s

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L1184-L1187

File: contracts/contracts/factories/PairFactory.sol

76:       function getFee(bool _stable) public view returns(uint256) {

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/factories/PairFactory.sol#L76

File: contracts/contracts/Voter.sol

82:       function setGovernor(address _governor) public {

87:       function setEmergencyCouncil(address _council) public {

178:      function whitelist(address _token) public {

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L82

7. Non-assembly method available

assembly{ id := chainid() } => uint256 id = block.chainid;, assembly { size := extcodesize() } => uint256 size = address().code.length

There is 1 instance of this issue:

File: contracts/contracts/VotingEscrow.sol   #1

367:              size := extcodesize(account)

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L367

8. constants should be defined rather than using magic numbers

There are 95 instances of this issue:

File: contracts/contracts/Minter.sol

/// @audit 15000000e18
22:       uint public weekly = 15000000e18;

/// @audit 30
41:           teamRate = 30; // 30 bps = 0.03%

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Minter.sol#L22

File: contracts/contracts/redeem/RedemptionSender.sol

/// @audit 11
21:           require(_optimismChainId == 11 || _optimismChainId == 10011, "CHAIN_ID_NOT_OP");

/// @audit 10011
21:           require(_optimismChainId == 11 || _optimismChainId == 10011, "CHAIN_ID_NOT_OP");

/// @audit 0x000000000000000000000000000000000000dEaD
37:                   0x000000000000000000000000000000000000dEaD,

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/redeem/RedemptionSender.sol#L21

File: contracts/contracts/redeem/RedemptionReceiver.sol

/// @audit 12
24:           require(_fantomChainId == 12 || _fantomChainId == 10012, "CHAIN_ID_NOT_FTM");

/// @audit 10012
24:           require(_fantomChainId == 12 || _fantomChainId == 10012, "CHAIN_ID_NOT_FTM");

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/redeem/RedemptionReceiver.sol#L24

File: contracts/contracts/Gauge.sol

/// @audit 7
164:          uint modTime = timestamp % (7 days);

/// @audit 7
176:          uint bribeStart = block.timestamp - (block.timestamp % (7 days)) + BRIBE_LAG;

/// @audit 7
496:          uint lastCpWeeksVoteEnd = cp.timestamp - (cp.timestamp % (7 days)) + BRIBE_LAG + DURATION;

/// @audit 7
597:          uint bribeStart = block.timestamp - (block.timestamp % (7 days)) + BRIBE_LAG;

/// @audit 7
598:          uint adjustedTstamp = block.timestamp < bribeStart ? bribeStart : bribeStart + 7 days;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L164

File: contracts/contracts/VotingEscrow.sol

/// @audit 48
143:              buffer[digits] = bytes1(uint8(48 + uint(value % 10)));

/// @audit 255
632:              for (uint i = 0; i < 255; ++i) {

/// @audit 128
886:          for (uint i = 0; i < 128; ++i) {

/// @audit 128
942:          for (uint i = 0; i < 128; ++i) {

/// @audit 255
1017:         for (uint i = 0; i < 255; ++i) {

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L143

File: contracts/contracts/VeloGovernor.sol

/// @audit 4
26:           L2GovernorVotesQuorumFraction(4) // 4%

/// @audit 15
32:           return 15 minutes; // 1 block

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VeloGovernor.sol#L26

File: contracts/contracts/VelodromeLibrary.sol

/// @audit 1e18
16:           return x0*(y*y/1e18*y/1e18)/1e18+(x0*x0/1e18*x0/1e18)*y/1e18;

/// @audit 1e18
16:           return x0*(y*y/1e18*y/1e18)/1e18+(x0*x0/1e18*x0/1e18)*y/1e18;

/// @audit 1e18
16:           return x0*(y*y/1e18*y/1e18)/1e18+(x0*x0/1e18*x0/1e18)*y/1e18;

/// @audit 1e18
16:           return x0*(y*y/1e18*y/1e18)/1e18+(x0*x0/1e18*x0/1e18)*y/1e18;

/// @audit 1e18
16:           return x0*(y*y/1e18*y/1e18)/1e18+(x0*x0/1e18*x0/1e18)*y/1e18;

/// @audit 1e18
16:           return x0*(y*y/1e18*y/1e18)/1e18+(x0*x0/1e18*x0/1e18)*y/1e18;

/// @audit 3
20:           return 3*x0*(y*y/1e18)/1e18+(x0*x0/1e18*x0/1e18);

/// @audit 1e18
20:           return 3*x0*(y*y/1e18)/1e18+(x0*x0/1e18*x0/1e18);

/// @audit 1e18
20:           return 3*x0*(y*y/1e18)/1e18+(x0*x0/1e18*x0/1e18);

/// @audit 1e18
20:           return 3*x0*(y*y/1e18)/1e18+(x0*x0/1e18*x0/1e18);

/// @audit 1e18
20:           return 3*x0*(y*y/1e18)/1e18+(x0*x0/1e18*x0/1e18);

/// @audit 255
24:           for (uint i = 0; i < 255; i++) {

/// @audit 1e18
28:                   uint dy = (xy - k)*1e18/_d(x0, y);

/// @audit 1e18
31:                   uint dy = (k - xy)*1e18/_d(x0, y);

/// @audit 1e18
50:           a = _getAmountOut(sample, tokenIn, r0, r1, t0, dec0, dec1, st) * 1e18 / sample;

/// @audit 1e18
51:           b = _getAmountOut(amountIn, tokenIn, r0, r1, t0, dec0, dec1, st) * 1e18 / amountIn;

/// @audit 1e18
57:           a = _getAmountOut(sample, tokenIn, r0, r1, t0, dec0, dec1, st) * 1e18 / sample;

/// @audit 1e18
58:           b = _getAmountOut(amountIn, tokenIn, r0, r1, t0, dec0, dec1, st) * 1e18 / amountIn;

/// @audit 1e18
64:           return _getAmountOut(sample, tokenIn, r0, r1, t0, dec0, dec1, st) * 1e18 / sample;

/// @audit 1e18
75:           return _getAmountOut(amountIn, tokenIn, r0, r1, t0, dec0, dec1, st) * 1e18 / amountIn;

/// @audit 1e18
81:               _reserve0 = _reserve0 * 1e18 / decimals0;

/// @audit 1e18
82:               _reserve1 = _reserve1 * 1e18 / decimals1;

/// @audit 1e18
84:               amountIn = tokenIn == token0 ? amountIn * 1e18 / decimals0 : amountIn * 1e18 / decimals1;

/// @audit 1e18
84:               amountIn = tokenIn == token0 ? amountIn * 1e18 / decimals0 : amountIn * 1e18 / decimals1;

/// @audit 1e18
86:               return y * (tokenIn == token0 ? decimals1 : decimals0) / 1e18;

/// @audit 1e18
95:               uint _x = x * 1e18 / decimals0;

/// @audit 1e18
96:               uint _y = y * 1e18 / decimals1;

/// @audit 1e18
97:               uint _a = (_x * _y) / 1e18;

/// @audit 1e18
98:               uint _b = ((_x * _x) / 1e18 + (_y * _y) / 1e18);

/// @audit 1e18
98:               uint _b = ((_x * _x) / 1e18 + (_y * _y) / 1e18);

/// @audit 1e18
99:               return _a * _b / 1e18;  // x3y+y3x >= k

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VelodromeLibrary.sol#L16

File: contracts/contracts/RewardsDistributor.sol

/// @audit 20
75:           for (uint i = 0; i < 20; i++) {

/// @audit 128
105:          for (uint i = 0; i < 128; i++) {

/// @audit 128
121:          for (uint i = 0; i < 128; i++) {

/// @audit 20
148:          for (uint i = 0; i < 20; i++) {

/// @audit 50
195:          for (uint i = 0; i < 50; i++) {

/// @audit 50
252:          for (uint i = 0; i < 50; i++) {

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/RewardsDistributor.sol#L75

File: contracts/contracts/Velo.sol

/// @audit 0x0
45:           emit Transfer(address(0x0), _to, _amount);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Velo.sol#L45

File: contracts/contracts/Bribe.sol

/// @audit 7
36:       uint bribeStart = timestamp - (timestamp % (7 days)) + BRIBE_LAG;

/// @audit 7
38:       return timestamp < bribeEnd ? bribeStart : bribeStart + 7 days;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L36

File: contracts/contracts/Pair.sol

/// @audit 1e18
153:          uint256 _ratio = amount * 1e18 / totalSupply; // 1e18 adjustment is removed during claim

/// @audit 1e18
163:          uint256 _ratio = amount * 1e18 / totalSupply;

/// @audit 1e18
184:                  uint _share = _supplied * _delta0 / 1e18; // add accrued difference for each supplied token

/// @audit 1e18
188:                  uint _share = _supplied * _delta1 / 1e18;

/// @audit 10000
356:          if (amount0In > 0) _update0(amount0In * PairFactory(factory).getFee(stable) / 10000); // accrue fees for token0 and move them out of pool

/// @audit 10000
357:          if (amount1In > 0) _update1(amount1In * PairFactory(factory).getFee(stable) / 10000); // accrue fees for token1 and move them out of pool

/// @audit 1e18
381:          return x0*(y*y/1e18*y/1e18)/1e18+(x0*x0/1e18*x0/1e18)*y/1e18;

/// @audit 1e18
381:          return x0*(y*y/1e18*y/1e18)/1e18+(x0*x0/1e18*x0/1e18)*y/1e18;

/// @audit 1e18
381:          return x0*(y*y/1e18*y/1e18)/1e18+(x0*x0/1e18*x0/1e18)*y/1e18;

/// @audit 1e18
381:          return x0*(y*y/1e18*y/1e18)/1e18+(x0*x0/1e18*x0/1e18)*y/1e18;

/// @audit 1e18
381:          return x0*(y*y/1e18*y/1e18)/1e18+(x0*x0/1e18*x0/1e18)*y/1e18;

/// @audit 1e18
381:          return x0*(y*y/1e18*y/1e18)/1e18+(x0*x0/1e18*x0/1e18)*y/1e18;

/// @audit 3
385:          return 3*x0*(y*y/1e18)/1e18+(x0*x0/1e18*x0/1e18);

/// @audit 1e18
385:          return 3*x0*(y*y/1e18)/1e18+(x0*x0/1e18*x0/1e18);

/// @audit 1e18
385:          return 3*x0*(y*y/1e18)/1e18+(x0*x0/1e18*x0/1e18);

/// @audit 1e18
385:          return 3*x0*(y*y/1e18)/1e18+(x0*x0/1e18*x0/1e18);

/// @audit 1e18
385:          return 3*x0*(y*y/1e18)/1e18+(x0*x0/1e18*x0/1e18);

/// @audit 255
389:          for (uint i = 0; i < 255; i++) {

/// @audit 1e18
393:                  uint dy = (xy - k)*1e18/_d(x0, y);

/// @audit 1e18
396:                  uint dy = (k - xy)*1e18/_d(x0, y);

/// @audit 10000
414:          amountIn -= amountIn * PairFactory(factory).getFee(stable) / 10000; // remove fee from amount received

/// @audit 1e18
421:              _reserve0 = _reserve0 * 1e18 / decimals0;

/// @audit 1e18
422:              _reserve1 = _reserve1 * 1e18 / decimals1;

/// @audit 1e18
424:              amountIn = tokenIn == token0 ? amountIn * 1e18 / decimals0 : amountIn * 1e18 / decimals1;

/// @audit 1e18
424:              amountIn = tokenIn == token0 ? amountIn * 1e18 / decimals0 : amountIn * 1e18 / decimals1;

/// @audit 1e18
426:              return y * (tokenIn == token0 ? decimals1 : decimals0) / 1e18;

/// @audit 1e18
435:              uint _x = x * 1e18 / decimals0;

/// @audit 1e18
436:              uint _y = y * 1e18 / decimals1;

/// @audit 1e18
437:              uint _a = (_x * _y) / 1e18;

/// @audit 1e18
438:              uint _b = ((_x * _x) / 1e18 + (_y * _y) / 1e18);

/// @audit 1e18
438:              uint _b = ((_x * _x) / 1e18 + (_y * _y) / 1e18);

/// @audit 1e18
439:              return _a * _b / 1e18;  // x3y+y3x >= k

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L153

File: contracts/contracts/Voter.sol

/// @audit 0x0
190:          require(gauges[_pool] == address(0x0), "exists");

/// @audit 1e18
258:          uint256 _ratio = amount * 1e18 / totalWeight; // 1e18 adjustment is removed during claim

/// @audit 1e18
295:                  uint _share = uint(_supplied) * _delta / 1e18; // add accrued difference for each supplied token

/// @audit 7
317:          uint dayCalc = block.timestamp % (7 days);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L190

9. Redundant cast

The type of the variable is the same as the type to which the variable is being cast

There are 4 instances of this issue:

File: contracts/contracts/Bribe.sol   #1

/// @audit address(gauge)
87:         _safeTransfer(token, address(gauge), rewardPerEpoch);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L87

File: contracts/contracts/Voter.sol   #2

/// @audit uint256(_totalWeight)
118:          totalWeight -= uint256(_totalWeight);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L118

File: contracts/contracts/Voter.sol   #3

/// @audit uint256(_totalWeight)
168:          totalWeight += uint256(_totalWeight);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L168

File: contracts/contracts/Voter.sol   #4

/// @audit uint256(_usedWeight)
169:          usedWeights[_tokenId] = uint256(_usedWeight);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L169

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

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

There are 7 instances of this issue:

File: contracts/contracts/Minter.sol

/// @audit 86400
14:       uint internal constant WEEK = 86400 * 7; // allows minting once per week (reset every Thursday 00:00 UTC)

/// @audit 15000000e18
22:       uint public weekly = 15000000e18;

/// @audit 86400
24:       uint internal constant LOCK = 86400 * 7 * 52 * 4;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Minter.sol#L14

File: contracts/contracts/VotingEscrow.sol

/// @audit 86400
542:      uint internal constant MAXTIME = 4 * 365 * 86400;

/// @audit 86400
543:      int128 internal constant iMAXTIME = 4 * 365 * 86400;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L542

File: contracts/contracts/RewardsDistributor.sol

/// @audit 86400
30:       uint constant WEEK = 7 * 86400;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/RewardsDistributor.sol#L30

File: contracts/contracts/Pair.sol

/// @audit 1800
45:       uint constant periodSize = 1800;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L45

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

There are 3 instances of this issue:

File: contracts/contracts/VotingEscrow.sol   #1

535:      mapping(uint => Point[1000000000]) public user_point_history; // user -> Point[user_epoch]

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L535

File: contracts/contracts/RewardsDistributor.sol   #2

38:       uint[1000000000000000] public tokens_per_week;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/RewardsDistributor.sol#L38

File: contracts/contracts/RewardsDistributor.sol   #3

44:       uint[1000000000000000] public ve_supply;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/RewardsDistributor.sol#L44

12. Missing event for critical parameter change

There are 16 instances of this issue:

File: contracts/contracts/Minter.sol

64        function setTeam(address _team) external {
65            require(msg.sender == team, "not team");
66            pendingTeam = _team;
67:       }

74        function setTeamRate(uint _teamRate) external {
75            require(msg.sender == team, "not team");
76            require(_teamRate <= MAX_TEAM_RATE, "rate too high");
77            teamRate = _teamRate;
78:       }

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Minter.sol#L64-L67

File: contracts/contracts/VotingEscrow.sol

1059      function setVoter(address _voter) external {
1060          require(msg.sender == voter);
1061          voter = _voter;
1062:     }

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L1059-L1062

File: contracts/contracts/VeloGovernor.sol

39        function setTeam(address newTeam) external {
40            require(msg.sender == team, "not team");
41            team = newTeam;
42:       }

44        function setProposalNumerator(uint256 numerator) external {
45            require(msg.sender == team, "not team");
46            require(numerator <= MAX_PROPOSAL_NUMERATOR, "numerator too high");
47            proposalNumerator = numerator;
48:       }

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VeloGovernor.sol#L39-L42

File: contracts/contracts/RewardsDistributor.sol

318       function setDepositor(address _depositor) external {
319           require(msg.sender == depositor);
320           depositor = _depositor;
321:      }

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/RewardsDistributor.sol#L318-L321

File: contracts/contracts/Velo.sol

26        function setMinter(address _minter) external {
27            require(msg.sender == minter);
28            minter = _minter;
29:       }

31        function setRedemptionReceiver(address _receiver) external {
32            require(msg.sender == minter);
33            redemptionReceiver = _receiver;
34:       }

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Velo.sol#L26-L29

File: contracts/contracts/Bribe.sol

30      function setGauge(address _gauge) external {
31        require(gauge == address(0), "gauge already set");
32        gauge = _gauge;
33:     }

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L30-L33

File: contracts/contracts/factories/GaugeFactory.sol

17        function setTeam(address _team) external {
18            require(msg.sender == team);
19            team = _team;
20:       }

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/factories/GaugeFactory.sol#L17-L20

File: contracts/contracts/factories/PairFactory.sol

40        function setPauser(address _pauser) external {
41            require(msg.sender == pauser);
42            pendingPauser = _pauser;
43:       }

50        function setPause(bool _state) external {
51            require(msg.sender == pauser);
52            isPaused = _state;
53:       }

55        function setFeeManager(address _feeManager) external {
56            require(msg.sender == feeManager, 'not fee manager');
57            pendingFeeManager = _feeManager;
58:       }

65        function setFee(bool _stable, uint256 _fee) external {
66            require(msg.sender == feeManager, 'not fee manager');
67            require(_fee <= MAX_FEE, 'fee too high');
68            require(_fee != 0, 'fee must be nonzero');
69            if (_stable) {
70                stableFee = _fee;
71            } else {
72                volatileFee = _fee;
73            }
74:       }

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/factories/PairFactory.sol#L40-L43

File: contracts/contracts/Voter.sol

82        function setGovernor(address _governor) public {
83            require(msg.sender == governor);
84            governor = _governor;
85:       }

87        function setEmergencyCouncil(address _council) public {
88            require(msg.sender == emergencyCouncil);
89            emergencyCouncil = _council;
90:       }

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L82-L85

13. Expressions for constant values such as a call to keccak256(), should use immutable rather than constant

There are 2 instances of this issue:

File: contracts/contracts/VotingEscrow.sol   #1

1106:     bytes32 public constant DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)");

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L1106

File: contracts/contracts/VotingEscrow.sol   #2

1109:     bytes32 public constant DELEGATION_TYPEHASH = keccak256("Delegation(address delegatee,uint256 nonce,uint256 expiry)");

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L1109

14. Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18)

There are 3 instances of this issue:

File: contracts/contracts/Router.sol   #1

21:       uint internal constant MINIMUM_LIQUIDITY = 10**3;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Router.sol#L21

File: contracts/contracts/Gauge.sol   #2

36:       uint internal constant PRECISION = 10 ** 18;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L36

File: contracts/contracts/Pair.sol   #3

30:       uint internal constant MINIMUM_LIQUIDITY = 10**3;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L30

15. 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 6 instances of this issue:

File: contracts/contracts/Gauge.sol

/// @audit seen in /var/tmp/hh/contracts/contracts/Minter.sol 
16:       address public immutable _ve; // the ve token used for gauges

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L16

File: contracts/contracts/Velo.sol

/// @audit seen in /var/tmp/hh/contracts/contracts/VotingEscrow.sol 
6:        string public constant name = "Velodrome";

/// @audit seen in /var/tmp/hh/contracts/contracts/VotingEscrow.sol 
7:        string public constant symbol = "VELO";

/// @audit seen in /var/tmp/hh/contracts/contracts/VotingEscrow.sol 
8:        uint8 public constant decimals = 18;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Velo.sol#L6

File: contracts/contracts/Pair.sol

/// @audit seen in /var/tmp/hh/contracts/contracts/Velo.sol 
15:       uint8 public constant decimals = 18;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L15

File: contracts/contracts/Voter.sol

/// @audit seen in /var/tmp/hh/contracts/contracts/Gauge.sol 
16:       address public immutable _ve; // the ve token that governs these contracts

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L16

16. Variable names that consist of all capital letters should be reserved for constant/immutable variables

If the variable needs to be different based on which class it comes from, a view/pure function should be used instead (e.g. like this).

There is 1 instance of this issue:

File: contracts/contracts/Pair.sol   #1

25:       bytes32 internal DOMAIN_SEPARATOR;

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L25

17. Typos

There are 7 instances of this issue:

File: contracts/contracts/redeem/RedemptionReceiver.sol

/// @audit FTM
14:       uint16 public immutable fantomChainId; // 12 for FTM, 10012 for FTM testnet

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/redeem/RedemptionReceiver.sol#L14

File: contracts/contracts/VotingEscrow.sol

/// @audit blocktimes
38:        * and per block could be fairly bad b/c Ethereum changes blocktimes.

/// @audit Exeute
295:      /// @dev Exeute transfer of a NFT.

/// @audit Pevious
575:      /// @param old_locked Pevious locked amount / end lock time for the user

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L38

File: contracts/contracts/PairFees.sol

/// @audit localy
10:       address internal immutable token0; // token0 of pair, saved localy and statically for gas optimization

/// @audit localy
11:       address internal immutable token1; // Token1 of pair, saved localy and statically for gas optimization

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/PairFees.sol#L10

File: contracts/contracts/Pair.sol

/// @audit obervations
37:       // Structure to capture time period obervations every 30 minutes, used for local oracles

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L37

18. File is missing NatSpec

There are 13 instances of this issue:

File: contracts/contracts/Minter.sol

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Minter.sol

File: contracts/contracts/Router.sol

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Router.sol

File: contracts/contracts/VeloGovernor.sol

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VeloGovernor.sol

File: contracts/contracts/VelodromeLibrary.sol

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VelodromeLibrary.sol

File: contracts/contracts/RewardsDistributor.sol

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/RewardsDistributor.sol

File: contracts/contracts/PairFees.sol

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/PairFees.sol

File: contracts/contracts/Velo.sol

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Velo.sol

File: contracts/contracts/Bribe.sol

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol

File: contracts/contracts/factories/GaugeFactory.sol

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/factories/GaugeFactory.sol

File: contracts/contracts/factories/BribeFactory.sol

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/factories/BribeFactory.sol

File: contracts/contracts/factories/PairFactory.sol

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/factories/PairFactory.sol

File: contracts/contracts/Pair.sol

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol

File: contracts/contracts/Voter.sol

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol

19. NatSpec is incomplete

There are 13 instances of this issue:

File: contracts/contracts/VotingEscrow.sol

/// @audit Missing: '@return'
160       /// @dev Returns current token URI metadata
161       /// @param _tokenId Token ID to fetch URI for.
162:      function tokenURI(uint _tokenId) external view returns (string memory) {

/// @audit Missing: '@return'
184       /// @dev Returns the address of the owner of the NFT.
185       /// @param _tokenId The identifier for an NFT.
186:      function ownerOf(uint _tokenId) public view returns (address) {

/// @audit Missing: '@return'
191       ///      Throws if `_owner` is the zero address. NFTs assigned to the zero address are considered invalid.
192       /// @param _owner Address for whom to query the balance.
193:      function _balance(address _owner) internal view returns (uint) {

/// @audit Missing: '@return'
198       ///      Throws if `_owner` is the zero address. NFTs assigned to the zero address are considered invalid.
199       /// @param _owner Address for whom to query the balance.
200:      function balanceOf(address _owner) external view returns (uint) {

/// @audit Missing: '@return'
216       /// @dev Get the approved address for a single NFT.
217       /// @param _tokenId ID of the NFT to query the approval of.
218:      function getApproved(uint _tokenId) external view returns (address) {

/// @audit Missing: '@return'
223       /// @param _owner The address that owns the NFTs.
224       /// @param _operator The address that acts on behalf of the owner.
225:      function isApprovedForAll(address _owner, address _operator) external view returns (bool) {

/// @audit Missing: '@return'
412       /// @dev Interface identification is specified in ERC-165.
413       /// @param _interfaceID Id of the interface
414:      function supportsInterface(bytes4 _interfaceID) external view returns (bool) {

/// @audit Missing: '@return'
780       /// @param _lock_duration Number of seconds to lock tokens for (rounded down to nearest week)
781       /// @param _to Address to deposit
782:      function _create_lock(uint _value, uint _lock_duration, address _to) internal returns (uint) {

/// @audit Missing: '@return'
798       /// @param _value Amount to deposit
799       /// @param _lock_duration Number of seconds to lock tokens for (rounded down to nearest week)
800:      function create_lock(uint _value, uint _lock_duration) external nonreentrant returns (uint) {

/// @audit Missing: '@return'
806       /// @param _lock_duration Number of seconds to lock tokens for (rounded down to nearest week)
807       /// @param _to Address to deposit
808:      function create_lock_for(uint _value, uint _lock_duration, address _to) external nonreentrant returns (uint) {

/// @audit Missing: '@param _tokenId'
812       /// @notice Deposit `_value` additional tokens for `_tokenId` without modifying the unlock time
813       /// @param _value Amount of tokens to deposit and add to the lock
814:      function increase_amount(uint _tokenId, uint _value) external nonreentrant {

/// @audit Missing: '@param _tokenId'
826       /// @notice Extend the unlock time for `_tokenId`
827       /// @param _lock_duration New number of seconds until tokens unlock
828:      function increase_unlock_time(uint _tokenId, uint _lock_duration) external nonreentrant {

/// @audit Missing: '@param t'
1043      /// @notice Calculate total voting power
1044      /// @dev Adheres to the ERC20 `totalSupply` interface for Aragon compatibility
1045      /// @return Total voting power
1046:     function totalSupplyAtT(uint t) public view returns (uint) {

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L160-L162

20. Event is missing indexed fields

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

There are 31 instances of this issue:

File: contracts/contracts/Minter.sol

32:       event Mint(address indexed sender, uint weekly, uint circulating_supply, uint circulating_emission);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Minter.sol#L32

File: contracts/contracts/Gauge.sol

90:       event Deposit(address indexed from, uint tokenId, uint amount);

91:       event Withdraw(address indexed from, uint tokenId, uint amount);

92:       event NotifyReward(address indexed from, address indexed reward, uint amount);

93:       event ClaimFees(address indexed from, uint claimed0, uint claimed1);

94:       event ClaimRewards(address indexed from, address indexed reward, uint amount);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L90

File: contracts/contracts/VotingEscrow.sol

51        event Deposit(
52            address indexed provider,
53            uint tokenId,
54            uint value,
55            uint indexed locktime,
56            DepositType deposit_type,
57            uint ts
58:       );

59:       event Withdraw(address indexed provider, uint tokenId, uint value, uint ts);

60:       event Supply(uint prevSupply, uint supply);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/VotingEscrow.sol#L51-L58

File: contracts/contracts/RewardsDistributor.sol

18        event CheckpointToken(
19            uint time,
20            uint tokens
21:       );

23        event Claimed(
24            uint tokenId,
25            uint amount,
26            uint claim_epoch,
27            uint max_epoch
28:       );

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/RewardsDistributor.sol#L18-L21

File: contracts/contracts/Velo.sol

17:       event Transfer(address indexed from, address indexed to, uint value);

18:       event Approval(address indexed owner, address indexed spender, uint value);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Velo.sol#L17

File: contracts/contracts/Bribe.sol

19:     event NotifyReward(address indexed from, address indexed reward, uint epoch, uint amount);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Bribe.sol#L19

File: contracts/contracts/factories/PairFactory.sol

26:       event PairCreated(address indexed token0, address indexed token1, bool stable, address pair, uint);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/factories/PairFactory.sol#L26

File: contracts/contracts/Pair.sol

72:       event Fees(address indexed sender, uint amount0, uint amount1);

73:       event Mint(address indexed sender, uint amount0, uint amount1);

74:       event Burn(address indexed sender, uint amount0, uint amount1, address indexed to);

75        event Swap(
76            address indexed sender,
77            uint amount0In,
78            uint amount1In,
79            uint amount0Out,
80            uint amount1Out,
81            address indexed to
82:       );

83:       event Sync(uint reserve0, uint reserve1);

84:       event Claim(address indexed sender, address indexed recipient, uint amount0, uint amount1);

86:       event Transfer(address indexed from, address indexed to, uint amount);

87:       event Approval(address indexed owner, address indexed spender, uint amount);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L72

File: contracts/contracts/Voter.sol

44:       event Voted(address indexed voter, uint tokenId, uint256 weight);

45:       event Abstained(uint tokenId, uint256 weight);

46:       event Deposit(address indexed lp, address indexed gauge, uint tokenId, uint amount);

47:       event Withdraw(address indexed lp, address indexed gauge, uint tokenId, uint amount);

48:       event NotifyReward(address indexed sender, address indexed reward, uint amount);

49:       event DistributeReward(address indexed sender, address indexed gauge, uint amount);

50:       event Attach(address indexed owner, address indexed gauge, uint tokenId);

51:       event Detach(address indexed owner, address indexed gauge, uint tokenId);

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Voter.sol#L44

21. Not using the named return variables anywhere in the function is confusing

Consider changing the variable to be an unnamed one

There are 12 instances of this issue:

File: contracts/contracts/Router.sol

/// @audit amount
71:       function getAmountOut(uint amountIn, address tokenIn, address tokenOut) external view returns (uint amount, bool stable) {

/// @audit stable
71:       function getAmountOut(uint amountIn, address tokenIn, address tokenOut) external view returns (uint amount, bool stable) {

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Router.sol#L71

File: contracts/contracts/Gauge.sol

/// @audit claimed0
131:      function claimFees() external lock returns (uint claimed0, uint claimed1) {

/// @audit claimed1
131:      function claimFees() external lock returns (uint claimed0, uint claimed1) {

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Gauge.sol#L131

File: contracts/contracts/Pair.sol

/// @audit dec0
125:      function metadata() external view returns (uint dec0, uint dec1, uint r0, uint r1, bool st, address t0, address t1) {

/// @audit dec1
125:      function metadata() external view returns (uint dec0, uint dec1, uint r0, uint r1, bool st, address t0, address t1) {

/// @audit r0
125:      function metadata() external view returns (uint dec0, uint dec1, uint r0, uint r1, bool st, address t0, address t1) {

/// @audit r1
125:      function metadata() external view returns (uint dec0, uint dec1, uint r0, uint r1, bool st, address t0, address t1) {

/// @audit st
125:      function metadata() external view returns (uint dec0, uint dec1, uint r0, uint r1, bool st, address t0, address t1) {

/// @audit t0
125:      function metadata() external view returns (uint dec0, uint dec1, uint r0, uint r1, bool st, address t0, address t1) {

/// @audit t1
125:      function metadata() external view returns (uint dec0, uint dec1, uint r0, uint r1, bool st, address t0, address t1) {

/// @audit amountOut
254:      function quote(address tokenIn, uint amountIn, uint granularity) external view returns (uint amountOut) {

https://github.com/code-423n4/2022-05-velodrome/blob/7fda97c570b758bbfa7dd6724a336c43d4041740/contracts/contracts/Pair.sol#L125

liveactionllama commented 2 years ago

Warden created this issue as a placeholder, because their submission was too large for the contest form. They then emailed their md file to our team on 05/30/2022 at 11:12 UTC. I've updated this issue with their md file content.

GalloDaSballo commented 2 years ago

1. Math.max(,0) used with int cast to uint

Great find, Valid Low

2. approveMax variable causes problems

I believe NC is more appropriate, caller can call the wrong one

3. Front-runable initializer

Because of the architecture of the system this is a Valid and Low finding

4. Return values of approve() not checked

Because token imp is known, this finding is not valid

5. require()/revert() statements should have descriptive reason strings

Valid NC

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

You cannot go from public to external (just tested), only from external to public, that said all of these are not override so while the comment is wrong, the advice of using external is valid

16. Variable names that consist of all capital letters should be reserved for constant/immutable variables

Valid NC

17. Typos

Valid NC

GalloDaSballo commented 2 years ago

7. require() should be used instead of assert()

Valid Low

8. constants should be defined rather than using magic numbers

Valid Non-Critical

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

Valid Low

10. Open TODOs

Valid NC

4. Incorrect comments

Wouldn't the timestamp shift by 24 hours every 4 years? It will always end at 00 just on a different day

5. Only a billion checkpoints available

Valid NC

1. Use two-phase ownership transfers

Valid NC

2. Avoid the use of sensitive terms

Valid NC

3. Consider addings checks for signature malleability

In lack of a POC I cannot give it points, with a POC this finding is potentially Med / High Severity

4. Return values of approve() not checked

Because the implementation is known the finding is not valid

 5. require()/revert() statements should have descriptive reason strings

Valid NC

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

Valid NC

7. Non-assembly method available

Because the code compiles to the same result, there is no advantage, hence will not give it points

8. constants should be defined rather than using magic number

Valid NC

9. Redundant cast

Valid NC

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

I would save since the constant is declared it would be more appropriate to re-use it. Valid NC

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

Valid NC

12. Missing event for critical parameter change | 16

Valid NC

13. Expressions for constant values such as a call to keccak256(), should use immutable rather than constant | 2

Not anymore, debunked here

14. Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18) | 3

Because it's for constants I don't think it makes a difference

15. Constant redefined elsewhere | 6

I don't think this suggestion is actionable

18. File is missing NatSpec | 13

Valid NC

19. NatSpec is incomplete | 13

Disagree with the finding, not all fields need natspec as it's optional

20. Event is missing indexed fields | 31

Valid NC, although most of these wouldn't be useful

21. Not using the named return variables anywhere in the function is confusing

Valid NC

GalloDaSballo commented 2 years ago

All in all a really thorough report, which would benefit by mentioning a finding once, and then listing all other occurrences, especially when the code is pretty much the same

That said, one of the best reports

GalloDaSballo commented 2 years ago

5 L, 21 NC

GalloDaSballo commented 2 years ago

L-1. Math.max(,0) used with int cast to uint

L-2. Front-runable initializer

L-3. require() should be used instead of assert()

L-4. _safeMint() should be used rather than _mint() wherever possible

L-5. Missing checks for address(0x0) when assigning values to address state variables

NC-1. approveMax variable causes problems

NC-2. require()/revert() statements should have descriptive reason strings

NC-3. public functions not called by the contract should be declared external instead

NC-4. Variable names that consist of all capital letters should be reserved for constant/immutable variables

NC-5. Typos

NC-6. constants should be defined rather than using magic numbers

NC-7. Open TODOs

NC-8. Only a billion checkpoints available

NC-9. Use two-phase ownership transfers

NC-10. Avoid the use of sensitive terms

NC-11. require()/revert() statements should have descriptive reason strings

NC-12. public functions not called by the contract should be declared external instead

NC-13. constants should be defined rather than using magic number

NC-14. Redundant cast

NC-15. Numeric values having to do with time should use time units for readability

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

NC-17. Missing event for critical parameter change

NC-18. File is missing NatSpec

NC-20. Event is missing indexed fields

NC-21. Not using the named return variables anywhere in the function is confusing