QA Report

[code423n4]

1. Inconsistent between implementation and documentation.

Impact

In Minter contract, MAX_TEAM_RATE = 50 and PRECISION = 1000, so it’s 5%. But in the comment same line, it says 50bps = 0.05%.

Also in Minter.constructor(), teamRate is set to 30 and comment says 30bps = 0.03%. But actually it’s 3%

Proof of concept

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

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

Tools Used

Manual Review

Recommended Mitigation Steps

Change the code or document to match

2. Should not be able to revive not gauge address

Impact

In Voter.reviveGauge(), any address can be passed in as _gauge param and set to isAlive[_gauge] = true even when that address is not a gauge.

Proof of concept

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

Tools Used

Manual Review

Recommended Mitigation Steps

Should add check if that address is a gauge

require(isGauge[_gauge]);

3. Rewards list can have duplicated token

Impact

There is a mapping to check if a token is existed in rewards list or not isReward[token]. But in swapOutRewardToken() function, there is no check using isReward.

An existing token can be passed in as newToken and rewards list will have a duplicated token.

Proof of concept

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

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

1. Current list rewards
   rewards = [USDT, DAI, UST]
2. Team mistakenly called swapOutRewardToken(2, UST, USDT). Because we do not check if USDT is already in the list. The TX will not revert. New list rewards is
   rewards = [USDT, DAI, USDT]
   Token USDT appears 2 times in the list.

Tools Used

Manual Review

Recommended Mitigation Steps

Check if newToken is in the list or not

require(!isReward[newToken]);

[GalloDaSballo]

1. Inconsistent between implementation and documentation.

Valid Low

2. Should not be able to revive not gauge address

Non-Critical because of the open-ended design

3. Rewards list can have duplicated token

Valid Low

Nice short report 2 Low, 1 NC