QA Report

[code423n4]

Redundant greater than 0 checks for uint
When executing pool weight changes in InflationManager.sol (_executeKeeperPoolWeight, _executeLpPoolWeight, _executeAmmTokenWeight), corresponding total pool weights x are unsigned int. As they can't only be x >= 0, the expression x>0 ? x : 0 does nothing.

Sensible changes, such as adding a Governor should be a two-step process
addGovernance directly grants Roles.GOVERNANCE to the newGovernor. It should be a two-step process (e.g. setGovernance-acceptGovernance) to ensure the input address is correct. Whilst renounceGovernance checks that there is at least one Governor left, in a single-Governor scenario, back to back execution of addGovernance with an incorrect address followed by renounceGovernance will render the protocol ungovernable. Additionally, as there is no way to revoke this role, granting governance to an address means that address will have complete control over the protocol forever after.

Requirement uses external call to user-controlled address in PoolMigrationZap.sol - migrate
require(oldPool_.getWithdrawalFee(msg.sender, lpTokenAmount_) == 0, "withdrawal fee not 0"); makes an external call to oldPoolAddress_, which is a user-controlled address. It could be a malicious contract, thus reporting any result needed to bypass the guard.

Erring on the safe side for reentrancy in PoolMigrationZap.sol - migrate
While I did not manage to exploit it in any way - I'd recommend against not including a nonReentrant guard in this function. oldPoolAddress_ could be a malicious contract thus enabling multiple calls to migrate spanning from the external calls to oldPool or lpToken_.

Recommended missing events
Modifying key protocol variables or triggering key events should always emit events accordingly to ensure transparency and proper traceability.

• Minter.sol: setToken, startInflation, executeInflationRateUpdate
• BkdLocker.sol: migrate
• Controller.sol: setInflationManager
• AmmGauge.sol: kill
• KeeperGauge.sol: kill, advanceEpoch, reportFees, poolCheckpoint
• InflationManager.sol: deactivateWeightBasedKeeperDistribution, checkpointAllGauges
• VestedEscrow.sol: setAdmin, setFundAdmin, initializeUnallocatedSupply

[GalloDaSballo]

Redundant greater than 0 checks for uint

Agree as they can at lowest be 0

Sensible changes, such as adding a Governor should be a two-step process

Personally disagree, but valid find

Requirement uses external call to user-controlled address in PoolMigrationZap.sol - migrate

Because of the architecture it's the callers responsibility to ensure the old contract is safe, as no funds beside the caller are at stake, I don't believe any risk was shown in this finding

Erring on the safe side for reentrancy in PoolMigrationZap.sol - migrate

Agree that the function doesn't respect CEI

Recommended missing events

Agree with informational level finding

Overall a concise report, I especially liked the formatted links which make the text way faster to read through