alchemix-finance · toyv0 · Apr 19, 2024 · Apr 19, 2024 · Apr 19, 2024
diff --git a/Makefile b/Makefile
@@ -74,4 +74,20 @@ test_file_block_debug :; FOUNDRY_PROFILE=$(PROFILE) forge test $(FORK_URL) $(MAT
 test_file_debug_test :; FOUNDRY_PROFILE=$(PROFILE) forge test $(FORK_URL) $(MATCH_PATH) $(MATCH_TEST) -vvv
 
 # runs single test within file with added verbosity for failing test from a given block: "make test_file_block_debug_test FILE=Minter TEST=testUnwrap"
-test_file_block_debug_test :; FOUNDRY_PROFILE=$(PROFILE) forge test $(FORK_URL) $(MATCH_PATH) $(MATCH_TEST) $(FORK_BLOCK) -vvv
+test_file_block_debug_test :; FOUNDRY_PROFILE=$(PROFILE) forge test $(FORK_URL) $(MATCH_PATH) $(MATCH_TEST) $(FORK_BLOCK) -vvv
+
+# | File                       | % Lines            | % Statements       | % Branches       | % Funcs          |
+# |----------------------------|--------------------|--------------------|------------------|------------------|
+# | src/RewardsDistributor.sol | 90.00% (126/140)   | 87.44% (174/199)   | 69.70% (46/66)   | 80.00% (12/15)   |
+# | src/Bribe.sol              | 92.47% (135/146)   | 93.44% (171/183)   | 78.38% (58/74)   | 78.95% (15/19)   |
+# | src/VotingEscrow.sol       | 93.96% (467/497)   | 94.13% (577/613)   | 77.65% (205/264) | 90.00% (72/80)   |
+# | src/Voter.sol              | 98.86% (173/175)   | 99.01% (200/202)   | 83.96% (89/106)  | 96.67% (29/30)   |
+# | src/Minter.sol             | 100.00% (48/48)    | 100.00% (61/61)    | 88.46% (23/26)   | 100.00% (9/9)    |
+# | src/FluxToken.sol          | 97.06% (66/68)     | 97.40% (75/77)     | 90.32% (56/62)   | 100.00% (19/19)  |
+# | src/RevenueHandler.sol     | 100.00% (94/94)    | 100.00% (115/115)  | 93.18% (41/44)   | 100.00% (15/15)  |
+# | src/RewardPoolManager.sol  | 100.00% (47/47)    | 100.00% (55/55)    | 94.12% (32/34)   | 100.00% (13/13)  |
+# | src/AlchemixGovernor.sol   | 100.00% (16/16)    | 100.00% (17/17)    | 100.00% (12/12)  | 100.00% (6/6)    |
+# | src/BaseGauge.sol          | 100.00% (13/13)    | 100.00% (13/13)    | 100.00% (14/14)  | 80.00% (4/5)     |
+# | src/BribeFactory.sol       | 100.00% (1/1)      | 100.00% (2/2)      | 100.00% (0/0)    | 100.00% (1/1)    |
+# | src/GaugeFactory.sol       | 100.00% (2/2)      | 100.00% (4/4)      | 100.00% (0/0)    | 100.00% (2/2)    |
+# | src/PassthroughGauge.sol   | 100.00% (2/2)      | 100.00% (2/2)      | 100.00% (0/0)    | 100.00% (1/1)    |
diff --git a/src/BaseGauge.sol b/src/BaseGauge.sol
@@ -6,24 +6,35 @@ import "src/interfaces/IBribe.sol";
 import "src/interfaces/IBaseGauge.sol";
 import "src/interfaces/IVoter.sol";
 import "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol";
+import "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
 
 /**
  * @title  Base Gauge
  * @notice Implementation of functionality that various gauge types use or extend
  * @notice Gauges are used to incentivize pools, they emit or passthrough reward tokens
  */
 abstract contract BaseGauge is IBaseGauge {
-    uint256 internal constant DURATION = 2 weeks; // Rewards released over voting period
+    using SafeERC20 for IERC20;
+
+    /// @notice Rewards released over voting period
+    uint256 internal constant DURATION = 2 weeks;
     uint256 internal constant BRIBE_LAG = 1 days;
     uint256 internal constant MAX_REWARD_TOKENS = 16;
 
-    address public ve; // veALCX token used for gauges
-    address public bribe; // Address of bribe contract
-    address public voter; // Address of voter contract
+    /// @notice veALCX token used for gauges
+    address public ve;
+    /// @notice Address of bribe contract
+    address public bribe;
+    /// @notice Address of voter contract
+    address public voter;
+    /// @notice Address of admin
     address public admin;
+    /// @notice Address of pending admin
     address public pendingAdmin;
-    address public receiver; // Address that receives the ALCX rewards
-    address public rewardToken; // Address of the reward token
+    /// @notice Address that receives the ALCX rewards
+    address public receiver;
+    /// @notice Address of the reward token
+    address public rewardToken;
 
     // Re-entrancy check
     uint256 internal _unlocked = 1;
@@ -34,22 +45,6 @@ abstract contract BaseGauge is IBaseGauge {
         _unlocked = 1;
     }
 
-    /*
-        View functions
-    */
-
-    /// @inheritdoc IBaseGauge
-    function getVotingStage(uint256 timestamp) external pure returns (VotingStage) {
-        uint256 modTime = timestamp % (1 weeks);
-        if (modTime < BRIBE_LAG) {
-            return VotingStage.BribesPhase;
-        } else if (modTime >= BRIBE_LAG && modTime < (BRIBE_LAG + DURATION)) {
-            return VotingStage.VotesPhase;
-        } else {
-            return VotingStage.RewardsPhase;
-        }
-    }
-
     /*
         External functions
     */
@@ -76,7 +71,7 @@ abstract contract BaseGauge is IBaseGauge {
     function notifyRewardAmount(uint256 _amount) external lock {
         require(msg.sender == voter, "not voter");
         require(_amount > 0, "zero amount");
-        _safeTransferFrom(rewardToken, msg.sender, address(this), _amount);
+        IERC20(rewardToken).safeTransferFrom(msg.sender, address(this), _amount);
 
         emit NotifyReward(msg.sender, rewardToken, _amount);
 
@@ -87,20 +82,6 @@ abstract contract BaseGauge is IBaseGauge {
         Internal functions
     */
 
-    function _safeTransfer(address token, address to, uint256 value) internal {
-        require(token.code.length > 0);
-        (bool success, bytes memory data) = token.call(abi.encodeWithSelector(IERC20.transfer.selector, to, value));
-        require(success && (data.length == 0 || abi.decode(data, (bool))));
-    }
-
-    function _safeTransferFrom(address token, address from, address to, uint256 value) internal {
-        require(token.code.length > 0);
-        (bool success, bytes memory data) = token.call(
-            abi.encodeWithSelector(IERC20.transferFrom.selector, from, to, value)
-        );
-        require(success && (data.length == 0 || abi.decode(data, (bool))));
-    }
-
     /**
      * @notice Override function to implement passthrough logic
      * @param _amount Amount of rewards

diff --git a/src/Bribe.sol b/src/Bribe.sol
@@ -1,18 +1,21 @@
 // SPDX-License-Identifier: GPL-3
 pragma solidity ^0.8.15;
-
+import "lib/forge-std/src/console2.sol";
 import "src/interfaces/IBribe.sol";
 import "src/interfaces/IBaseGauge.sol";
 import "src/interfaces/IVoter.sol";
 import "src/interfaces/IVotingEscrow.sol";
 import "src/libraries/Math.sol";
 import "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol";
+import "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
 
 /**
  * @title  Bribe
  * @notice Implementation of bribe contract to be used with gauges
  */
 contract Bribe is IBribe {
+    using SafeERC20 for IERC20;
+
     /// @notice Rewards released over voting period
     uint256 internal constant DURATION = 2 weeks;
     /// @notice Duration of time when bribes are accepted
@@ -107,7 +110,7 @@ contract Bribe is IBribe {
 
     /// @inheritdoc IBribe
     function notifyRewardAmount(address token, uint256 amount) external lock {
-        require(amount > 0);
+        require(amount > 0, "reward amount must be greater than 0");
 
         // If the token has been whitelisted by the voter contract, add it to the rewards list
         require(IVoter(voter).isWhitelisted(token), "bribe tokens must be whitelisted");
@@ -117,7 +120,8 @@ contract Bribe is IBribe {
         uint256 adjustedTstamp = getEpochStart(block.timestamp);
         uint256 epochRewards = tokenRewardsPerEpoch[token][adjustedTstamp];
 
-        _safeTransferFrom(token, msg.sender, address(this), amount);
+        IERC20(token).safeTransferFrom(msg.sender, address(this), amount);
+
         tokenRewardsPerEpoch[token][adjustedTstamp] = epochRewards + amount;
         periodFinish[token] = adjustedTstamp + DURATION;
 
@@ -126,7 +130,7 @@ contract Bribe is IBribe {
 
     /// @inheritdoc IBribe
     function addRewardToken(address token) external {
-        require(msg.sender == gauge);
+        require(msg.sender == gauge, "not being set by a gauge");
         _addRewardToken(token);
     }
 
@@ -135,7 +139,6 @@ contract Bribe is IBribe {
         require(msg.sender == voter, "Only voter can execute");
         require(IVoter(voter).isWhitelisted(newToken), "New token must be whitelisted");
         require(rewards[oldTokenIndex] == oldToken, "Old token mismatch");
-        require(newToken != address(0), "New token cannot be zero address");
 
         // Check that the newToken does not already exist in the rewards array
         for (uint256 i = 0; i < rewards.length; i++) {
@@ -193,7 +196,6 @@ contract Bribe is IBribe {
         if (votingCheckpoints[nCheckpoints - 1].timestamp < timestamp) {
             return (nCheckpoints - 1);
         }
-
         // Check implicit zero balance
         if (votingCheckpoints[0].timestamp > timestamp) {
             return 0;
@@ -291,7 +293,7 @@ contract Bribe is IBribe {
 
             _writeCheckpoint(tokenId, balanceOf[tokenId]);
 
-            _safeTransfer(tokens[i], _owner, _reward);
+            IERC20(tokens[i]).safeTransfer(_owner, _reward);
 
             emit ClaimRewards(_owner, tokens[i], _reward);
         }
@@ -384,18 +386,4 @@ contract Bribe is IBribe {
     function _bribeStart(uint256 timestamp) internal pure returns (uint256) {
         return timestamp - (timestamp % (DURATION));
     }
-
-    function _safeTransfer(address token, address to, uint256 value) internal {
-        require(token.code.length > 0);
-        (bool success, bytes memory data) = token.call(abi.encodeWithSelector(IERC20.transfer.selector, to, value));
-        require(success && (data.length == 0 || abi.decode(data, (bool))));
-    }
-
-    function _safeTransferFrom(address token, address from, address to, uint256 value) internal {
-        require(token.code.length > 0);
-        (bool success, bytes memory data) = token.call(
-            abi.encodeWithSelector(IERC20.transferFrom.selector, from, to, value)
-        );
-        require(success && (data.length == 0 || abi.decode(data, (bool))));
-    }
 }
diff --git a/src/FluxToken.sol b/src/FluxToken.sol
@@ -213,7 +213,7 @@ contract FluxToken is ERC20("Flux", "FLUX"), IFluxToken {
 
     // Given an amount of eth, calculate how much FLUX it would earn in a year if it were deposited into veALCX
     function getClaimableFlux(uint256 _amount, address _nft) public view returns (uint256 claimableFlux) {
-        uint256 bpt = _calculateBPT(_amount);
+        uint256 bpt = calculateBPT(_amount);
 
         uint256 veMul = IVotingEscrow(veALCX).MULTIPLIER();
         uint256 veMax = IVotingEscrow(veALCX).MAXTIME();
@@ -229,7 +229,7 @@ contract FluxToken is ERC20("Flux", "FLUX"), IFluxToken {
         }
     }
 
-    function _calculateBPT(uint256 _amount) public view returns (uint256 bptOut) {
+    function calculateBPT(uint256 _amount) public view returns (uint256 bptOut) {
         bptOut = _amount * bptMultiplier;
     }
 }
diff --git a/src/RewardsDistributor.sol b/src/RewardsDistributor.sol
@@ -138,7 +138,7 @@ contract RewardsDistributor is IRewardsDistributor, ReentrancyGuard {
 
     /// @inheritdoc IRewardsDistributor
     function checkpointToken() external {
-        assert(msg.sender == depositor);
+        require(msg.sender == depositor, "only depositor");
         _checkpointToken();
     }
 
@@ -211,7 +211,7 @@ contract RewardsDistributor is IRewardsDistributor, ReentrancyGuard {
 
     /// @dev Once off event on contract initialize
     function setDepositor(address _depositor) external {
-        require(msg.sender == depositor);
+        require(msg.sender == depositor, "only depositor");
         depositor = _depositor;
         emit DepositorUpdated(_depositor);
     }

diff --git a/src/Voter.sol b/src/Voter.sol
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-3
 pragma solidity ^0.8.15;
-
+import "lib/forge-std/src/console2.sol";
 import "src/interfaces/IBribeFactory.sol";
 import "src/interfaces/IBribe.sol";
 import "src/interfaces/IBaseGauge.sol";
@@ -28,7 +28,7 @@ contract Voter is IVoter {
     address public immutable bribefactory;
 
     uint256 internal constant BPS = 10_000;
-    uint256 internal constant MAX_BOOST = 10000;
+    uint256 internal constant MAX_BOOST = 10_000;
     uint256 internal constant MIN_BOOST = 0;
     /// @notice Rewards are released over this duration (2 weeks)
     uint256 internal constant DURATION = 2 weeks;
@@ -167,7 +167,7 @@ contract Voter is IVoter {
 
     /// @inheritdoc IVoter
     function swapReward(address gaugeAddress, uint256 tokenIndex, address oldToken, address newToken) external {
-        require(msg.sender == admin);
+        require(msg.sender == admin, "only admin can swap reward tokens");
         IBribe(bribes[gaugeAddress]).swapOutRewardToken(tokenIndex, oldToken, newToken);
     }
 
@@ -231,8 +231,8 @@ contract Voter is IVoter {
         uint256[] calldata _weights,
         uint256 _boost
     ) external onlyNewEpoch(_tokenId) {
-        require(IVotingEscrow(veALCX).isApprovedOrOwner(msg.sender, _tokenId));
-        require(_poolVote.length == _weights.length);
+        require(IVotingEscrow(veALCX).isApprovedOrOwner(msg.sender, _tokenId), "not approved or owner");
+        require(_poolVote.length == _weights.length, "pool vote and weights mismatch");
         require(_poolVote.length > 0, "no pools voted for");
         require(_poolVote.length <= pools.length, "invalid pools");
         require(
@@ -251,6 +251,7 @@ contract Voter is IVoter {
     /// @inheritdoc IVoter
     function whitelist(address _token) public {
         require(msg.sender == admin, "not admin");
+        require(_token != address(0), "cannot be zero address");
         _whitelist(_token);
     }
 
@@ -262,8 +263,8 @@ contract Voter is IVoter {
 
     /// @inheritdoc IVoter
     function createGauge(address _pool, GaugeType _gaugeType) external returns (address) {
+        require(msg.sender == admin, "not admin");
         require(gauges[_pool] == address(0x0), "exists");
-        require(msg.sender == admin, "only admin creates gauges");
 
         address _bribe = IBribeFactory(bribefactory).createBribe();
 
@@ -327,23 +328,6 @@ contract Voter is IVoter {
         }
     }
 
-    /// @inheritdoc IVoter
-    function updateForRange(uint256 start, uint256 end) public {
-        for (uint256 i = start; i < end; i++) {
-            _updateFor(gauges[pools[i]]);
-        }
-    }
-
-    /// @inheritdoc IVoter
-    function updateAll() external {
-        updateForRange(0, pools.length);
-    }
-
-    /// @inheritdoc IVoter
-    function updateGauge(address _gauge) external {
-        _updateFor(_gauge);
-    }
-
     /// @inheritdoc IVoter
     function claimBribes(address[] memory _bribes, address[][] memory _tokens, uint256 _tokenId) external {
         require(IVotingEscrow(veALCX).isApprovedOrOwner(msg.sender, _tokenId));
@@ -446,8 +430,8 @@ contract Voter is IVoter {
             require(isAlive[_gauge], "cannot vote for dead gauge");
 
             uint256 _poolWeight = (_weights[i] * totalPower) / _totalVoteWeight;
-            require(votes[_tokenId][_pool] == 0);
-            require(_poolWeight != 0);
+            require(votes[_tokenId][_pool] == 0, "already voted for pool");
+            require(_poolWeight != 0, "cannot vote with zero weight");
             _updateFor(_gauge);
 
             poolVote[_tokenId].push(_pool);
@@ -471,13 +455,13 @@ contract Voter is IVoter {
     }
 
     function _whitelist(address _token) internal {
-        require(!isWhitelisted[_token]);
+        require(!isWhitelisted[_token], "token already whitelisted");
         isWhitelisted[_token] = true;
         emit Whitelisted(msg.sender, _token);
     }
 
     function _removeFromWhitelist(address _token) internal {
-        require(isWhitelisted[_token]);
+        require(isWhitelisted[_token], "token not whitelisted");
         isWhitelisted[_token] = false;
         emit RemovedFromWhitelist(msg.sender, _token);
     }