From 9c5b4d14dae6f5488474a5febbdb3fe1e3f118c1 Mon Sep 17 00:00:00 2001 From: Erwan Beauvois Date: Fri, 17 Mar 2023 16:41:17 +0100 Subject: [PATCH 1/8] Import updated ERC20Gauges contract --- src/governance/ERC20Gauges.sol | 652 +++++++++++++++++++++++ test/mock/MockERC20Gauges.sol | 59 +++ test/unit/governance/ERC20Gauges.t.sol | 695 +++++++++++++++++++++++++ 3 files changed, 1406 insertions(+) create mode 100644 src/governance/ERC20Gauges.sol create mode 100644 test/mock/MockERC20Gauges.sol create mode 100644 test/unit/governance/ERC20Gauges.t.sol diff --git a/src/governance/ERC20Gauges.sol b/src/governance/ERC20Gauges.sol new file mode 100644 index 00000000..60fdcda8 --- /dev/null +++ b/src/governance/ERC20Gauges.sol @@ -0,0 +1,652 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity =0.8.13; + +import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; + +/** + @title An ERC20 with an embedded "Gauge" style vote with liquid weights + @author joeysantoro, eswak + @notice This contract is meant to be used to support gauge style votes with weights associated with resource allocation. + Holders can allocate weight in any proportion to supported gauges. + A "gauge" is represented by an address which would receive the resources periodically or continuously. + For example, gauges can be used to direct token emissions, similar to Curve or Tokemak. + Alternatively, gauges can be used to direct another quantity such as relative access to a line of credit. + This contract is abstract, and a parent shall implement public setter with adequate access control to manage + the gauge set and caps. + "Live" gauges are in the set `_gauges`. + Users can only add weight to live gauges but can remove weight from live or deprecated gauges. + Gauges can be deprecated and reinstated, and will maintain any non-removed weight from before. + @dev SECURITY NOTES: `maxGauges` is a critical variable to protect against gas DOS attacks upon token transfer. + This must be low enough to allow complicated transactions to fit in a block. + Weight state is preserved on the gauge and user level even when a gauge is removed, in case it is re-added. + This maintains state efficiently, and global accounting is managed only on the `_totalWeight` + @dev This contract was originally published as part of TribeDAO's flywheel-v2 repo, please see: + https://github.com/fei-protocol/flywheel-v2/blob/main/src/token/ERC20Gauges.sol + The original version was included in 2 audits : + - https://code4rena.com/reports/2022-04-xtribe/ + - https://consensys.net/diligence/audits/2022/04/tribe-dao-flywheel-v2-xtribe-xerc4626/ + Volt Protocol made the following changes to the original flywheel-v2 version : + - Does not inherit Solmate's Auth (all requiresAuth functions are now internal, see below) + -> This contract is abstract, and permissioned public functions can be added in parent. + -> permissioned public functions to add in parent: + - function addGauge(address) external returns (uint112) + - function removeGauge(address) external + - function setMaxGauges(uint256) external + - function setCanExceedMaxGauges(address, bool) external + - Remove public addGauge(address) requiresAuth method + - Remove public removeGauge(address) requiresAuth method + - Remove public replaceGauge(address, address) requiresAuth method + - Remove public setMaxGauges(uint256) requiresAuth method + ... Add internal _setMaxGauges(uint256) method + - Remove public setContractExceedMaxGauges(address, bool) requiresAuth method + ... Add internal _setCanExceedMaxGauges(address, bool) method + ... Remove check of "target address has nonzero code size" + ... Rename to remove "contract" from name because we don't check if target is a contract + - Consistency: make incrementGauges return a uint112 instead of uint256 + - Import OpenZeppelin ERC20 & EnumerableSet instead of Solmate's + - (TODO) Change error management style + - (TODO) Add negative weight voting +*/ +abstract contract ERC20Gauges is ERC20 { + using EnumerableSet for EnumerableSet.AddressSet; + + constructor(uint32 _gaugeCycleLength, uint32 _incrementFreezeWindow) { + if (_incrementFreezeWindow >= _gaugeCycleLength) + revert IncrementFreezeError(); + gaugeCycleLength = _gaugeCycleLength; + incrementFreezeWindow = _incrementFreezeWindow; + } + + /*/////////////////////////////////////////////////////////////// + GAUGE STATE + //////////////////////////////////////////////////////////////*/ + + /// @notice the length of a gauge cycle + uint32 public immutable gaugeCycleLength; + + /// @notice the period at the end of a cycle where votes cannot increment + uint32 public immutable incrementFreezeWindow; + + struct Weight { + uint112 storedWeight; + uint112 currentWeight; + uint32 currentCycle; + } + + /// @notice a mapping from users to gauges to a user's allocated weight to that gauge + mapping(address => mapping(address => uint112)) public getUserGaugeWeight; + + /// @notice a mapping from a user to their total allocated weight across all gauges + /// @dev NOTE this may contain weights for deprecated gauges + mapping(address => uint112) public getUserWeight; + + /// @notice a mapping from a gauge to the total weight allocated to it + /// @dev NOTE this may contain weights for deprecated gauges + mapping(address => Weight) internal _getGaugeWeight; + + /// @notice the total global allocated weight ONLY of live gauges + Weight internal _totalWeight; + + mapping(address => EnumerableSet.AddressSet) internal _userGauges; + + EnumerableSet.AddressSet internal _gauges; + + // Store deprecated gauges in case a user needs to free dead weight + EnumerableSet.AddressSet internal _deprecatedGauges; + + /*/////////////////////////////////////////////////////////////// + VIEW HELPERS + //////////////////////////////////////////////////////////////*/ + + /// @notice return the end of the current cycle. This is the next unix timestamp which evenly divides `gaugeCycleLength` + function getGaugeCycleEnd() public view returns (uint32) { + return _getGaugeCycleEnd(); + } + + /// @notice see `getGaugeCycleEnd()` + function _getGaugeCycleEnd() internal view returns (uint32) { + // @dev timestamp of type(uint32).max is Feb 07 2106 + uint32 nowPlusOneCycle = uint32(block.timestamp) + gaugeCycleLength; + unchecked { + return (nowPlusOneCycle / gaugeCycleLength) * gaugeCycleLength; // cannot divide by zero and always <= nowPlusOneCycle so no overflow + } + } + + /// @notice returns the current weight of a given gauge + function getGaugeWeight(address gauge) public view returns (uint112) { + return _getGaugeWeight[gauge].currentWeight; + } + + /// @notice returns the stored weight of a given gauge. This is the snapshotted weight as-of the end of the last cycle. + function getStoredGaugeWeight(address gauge) public view returns (uint112) { + if (_deprecatedGauges.contains(gauge)) return 0; + return _getStoredWeight(_getGaugeWeight[gauge], _getGaugeCycleEnd()); + } + + /// @notice see `getStoredGaugeWeight()` + function _getStoredWeight( + Weight storage gaugeWeight, + uint32 currentCycle + ) internal view returns (uint112) { + return + gaugeWeight.currentCycle < currentCycle + ? gaugeWeight.currentWeight + : gaugeWeight.storedWeight; + } + + /// @notice returns the current total allocated weight + function totalWeight() external view returns (uint112) { + return _totalWeight.currentWeight; + } + + /// @notice returns the stored total allocated weight + function storedTotalWeight() external view returns (uint112) { + return _getStoredWeight(_totalWeight, _getGaugeCycleEnd()); + } + + /// @notice returns the set of live gauges + function gauges() external view returns (address[] memory) { + return _gauges.values(); + } + + /** + @notice returns a paginated subset of live gauges + @param offset the index of the first gauge element to read + @param num the number of gauges to return + */ + function gauges( + uint256 offset, + uint256 num + ) external view returns (address[] memory values) { + values = new address[](num); + for (uint256 i = 0; i < num; ) { + unchecked { + values[i] = _gauges.at(offset + i); // will revert if out of bounds + i++; + } + } + } + + /// @notice returns true if `gauge` is not in deprecated gauges + function isGauge(address gauge) external view returns (bool) { + return _gauges.contains(gauge) && !_deprecatedGauges.contains(gauge); + } + + /// @notice returns the number of live gauges + function numGauges() external view returns (uint256) { + return _gauges.length(); + } + + /// @notice returns the set of previously live but now deprecated gauges + function deprecatedGauges() external view returns (address[] memory) { + return _deprecatedGauges.values(); + } + + /// @notice returns the number of live gauges + function numDeprecatedGauges() external view returns (uint256) { + return _deprecatedGauges.length(); + } + + /// @notice returns the set of gauges the user has allocated to, may be live or deprecated. + function userGauges(address user) external view returns (address[] memory) { + return _userGauges[user].values(); + } + + /// @notice returns true if `gauge` is in user gauges + function isUserGauge( + address user, + address gauge + ) external view returns (bool) { + return _userGauges[user].contains(gauge); + } + + /** + @notice returns a paginated subset of gauges the user has allocated to, may be live or deprecated. + @param user the user to return gauges from. + @param offset the index of the first gauge element to read. + @param num the number of gauges to return. + */ + function userGauges( + address user, + uint256 offset, + uint256 num + ) external view returns (address[] memory values) { + values = new address[](num); + for (uint256 i = 0; i < num; ) { + unchecked { + values[i] = _userGauges[user].at(offset + i); // will revert if out of bounds + i++; + } + } + } + + /// @notice returns the number of user gauges + function numUserGauges(address user) external view returns (uint256) { + return _userGauges[user].length(); + } + + /// @notice helper function exposing the amount of weight available to allocate for a user + function userUnusedWeight(address user) external view returns (uint256) { + return balanceOf(user) - getUserWeight[user]; + } + + /** + @notice helper function for calculating the proportion of a `quantity` allocated to a gauge + @param gauge the gauge to calculate allocation of + @param quantity a representation of a resource to be shared among all gauges + @return the proportion of `quantity` allocated to `gauge`. Returns 0 if gauge is not live, even if it has weight. + */ + function calculateGaugeAllocation( + address gauge, + uint256 quantity + ) external view returns (uint256) { + if (_deprecatedGauges.contains(gauge)) return 0; + uint32 currentCycle = _getGaugeCycleEnd(); + + uint112 total = _getStoredWeight(_totalWeight, currentCycle); + uint112 weight = _getStoredWeight(_getGaugeWeight[gauge], currentCycle); + return (quantity * weight) / total; + } + + /*/////////////////////////////////////////////////////////////// + USER GAUGE OPERATIONS + //////////////////////////////////////////////////////////////*/ + + /// @notice thrown when trying to increment/decrement a mismatched number of gauges and weights. + error SizeMismatchError(); + + /// @notice thrown when trying to increment over the max allowed gauges. + error MaxGaugeError(); + + /// @notice thrown when incrementing over a users free weight. + error OverWeightError(); + + /// @notice thrown when incremending during the freeze window. + error IncrementFreezeError(); + + /// @notice emitted when incrementing a gauge + event IncrementGaugeWeight( + address indexed user, + address indexed gauge, + uint256 weight, + uint32 cycleEnd + ); + + /// @notice emitted when decrementing a gauge + event DecrementGaugeWeight( + address indexed user, + address indexed gauge, + uint256 weight, + uint32 cycleEnd + ); + + /** + @notice increment a gauge with some weight for the caller + @param gauge the gauge to increment + @param weight the amount of weight to increment on gauge + @return newUserWeight the new user weight + */ + function incrementGauge( + address gauge, + uint112 weight + ) external returns (uint112 newUserWeight) { + uint32 currentCycle = _getGaugeCycleEnd(); + _incrementGaugeWeight(msg.sender, gauge, weight, currentCycle); + return _incrementUserAndGlobalWeights(msg.sender, weight, currentCycle); + } + + function _incrementGaugeWeight( + address user, + address gauge, + uint112 weight, + uint32 cycle + ) internal { + if (_deprecatedGauges.contains(gauge)) revert InvalidGaugeError(); + unchecked { + if (cycle - block.timestamp <= incrementFreezeWindow) + revert IncrementFreezeError(); + } + + bool added = _userGauges[user].add(gauge); // idempotent add + if ( + added && + _userGauges[user].length() > maxGauges && + !canExceedMaxGauges[user] + ) revert MaxGaugeError(); + + getUserGaugeWeight[user][gauge] += weight; + + _writeGaugeWeight(_getGaugeWeight[gauge], _add, weight, cycle); + + emit IncrementGaugeWeight(user, gauge, weight, cycle); + } + + function _incrementUserAndGlobalWeights( + address user, + uint112 weight, + uint32 cycle + ) internal returns (uint112 newUserWeight) { + newUserWeight = getUserWeight[user] + weight; + // Ensure under weight + if (newUserWeight > balanceOf(user)) revert OverWeightError(); + + // Update gauge state + getUserWeight[user] = newUserWeight; + + _writeGaugeWeight(_totalWeight, _add, weight, cycle); + } + + /** + @notice increment a list of gauges with some weights for the caller + @param gaugeList the gauges to increment + @param weights the weights to increment by + @return newUserWeight the new user weight + */ + function incrementGauges( + address[] calldata gaugeList, + uint112[] calldata weights + ) external returns (uint112 newUserWeight) { + uint256 size = gaugeList.length; + if (weights.length != size) revert SizeMismatchError(); + + // store total in summary for batch update on user/global state + uint112 weightsSum; + + uint32 currentCycle = _getGaugeCycleEnd(); + + // Update gauge specific state + for (uint256 i = 0; i < size; ) { + address gauge = gaugeList[i]; + uint112 weight = weights[i]; + weightsSum += weight; + + _incrementGaugeWeight(msg.sender, gauge, weight, currentCycle); + unchecked { + i++; + } + } + return + _incrementUserAndGlobalWeights( + msg.sender, + weightsSum, + currentCycle + ); + } + + /** + @notice decrement a gauge with some weight for the caller + @param gauge the gauge to decrement + @param weight the amount of weight to decrement on gauge + @return newUserWeight the new user weight + */ + function decrementGauge( + address gauge, + uint112 weight + ) external returns (uint112 newUserWeight) { + uint32 currentCycle = _getGaugeCycleEnd(); + + // All operations will revert on underflow, protecting against bad inputs + _decrementGaugeWeight(msg.sender, gauge, weight, currentCycle); + return _decrementUserAndGlobalWeights(msg.sender, weight, currentCycle); + } + + function _decrementGaugeWeight( + address user, + address gauge, + uint112 weight, + uint32 cycle + ) internal { + uint112 oldWeight = getUserGaugeWeight[user][gauge]; + + getUserGaugeWeight[user][gauge] = oldWeight - weight; + if (oldWeight == weight) { + // If removing all weight, remove gauge from user list. + require(_userGauges[user].remove(gauge)); + } + + _writeGaugeWeight(_getGaugeWeight[gauge], _subtract, weight, cycle); + + emit DecrementGaugeWeight(user, gauge, weight, cycle); + } + + function _decrementUserAndGlobalWeights( + address user, + uint112 weight, + uint32 cycle + ) internal returns (uint112 newUserWeight) { + newUserWeight = getUserWeight[user] - weight; + + getUserWeight[user] = newUserWeight; + _writeGaugeWeight(_totalWeight, _subtract, weight, cycle); + } + + /** + @notice decrement a list of gauges with some weights for the caller + @param gaugeList the gauges to decrement + @param weights the list of weights to decrement on the gauges + @return newUserWeight the new user weight + */ + function decrementGauges( + address[] calldata gaugeList, + uint112[] calldata weights + ) external returns (uint112 newUserWeight) { + uint256 size = gaugeList.length; + if (weights.length != size) revert SizeMismatchError(); + + // store total in summary for batch update on user/global state + uint112 weightsSum; + + uint32 currentCycle = _getGaugeCycleEnd(); + + // Update gauge specific state + // All operations will revert on underflow, protecting against bad inputs + for (uint256 i = 0; i < size; ) { + address gauge = gaugeList[i]; + uint112 weight = weights[i]; + weightsSum += weight; + + _decrementGaugeWeight(msg.sender, gauge, weight, currentCycle); + unchecked { + i++; + } + } + return + _decrementUserAndGlobalWeights( + msg.sender, + weightsSum, + currentCycle + ); + } + + /** + @dev this function is the key to the entire contract. + The storage weight it operates on is either a global or gauge-specific weight. + The operation applied is either addition for incrementing gauges or subtraction for decrementing a gauge. + */ + function _writeGaugeWeight( + Weight storage weight, + function(uint112, uint112) view returns (uint112) op, + uint112 delta, + uint32 cycle + ) private { + uint112 currentWeight = weight.currentWeight; + // If the last cycle of the weight is before the current cycle, use the current weight as the stored. + uint112 stored = weight.currentCycle < cycle + ? currentWeight + : weight.storedWeight; + uint112 newWeight = op(currentWeight, delta); + + weight.storedWeight = stored; + weight.currentWeight = newWeight; + weight.currentCycle = cycle; + } + + function _add(uint112 a, uint112 b) private pure returns (uint112) { + return a + b; + } + + function _subtract(uint112 a, uint112 b) private pure returns (uint112) { + return a - b; + } + + /*/////////////////////////////////////////////////////////////// + ADMIN GAUGE OPERATIONS + //////////////////////////////////////////////////////////////*/ + + /// @notice thrown when trying to increment or remove a non-live gauge, or add a live gauge. + error InvalidGaugeError(); + + /// @notice emitted when adding a new gauge to the live set. + event AddGauge(address indexed gauge); + + /// @notice emitted when removing a gauge from the live set. + event RemoveGauge(address indexed gauge); + + /// @notice emitted when updating the max number of gauges a user can delegate to. + event MaxGaugesUpdate(uint256 oldMaxGauges, uint256 newMaxGauges); + + /// @notice emitted when changing a contract's approval to go over the max gauges. + event CanExceedMaxGaugesUpdate( + address indexed account, + bool canExceedMaxGauges + ); + + /// @notice the default maximum amount of gauges a user can allocate to. + /// @dev if this number is ever lowered, or a contract has an override, then existing addresses MAY have more gauges allocated to. Use `numUserGauges` to check this. + uint256 public maxGauges; + + /// @notice an approve list for contracts to go above the max gauge limit. + mapping(address => bool) public canExceedMaxGauges; + + function _addGauge(address gauge) internal returns (uint112 weight) { + bool newAdd = _gauges.add(gauge); + bool previouslyDeprecated = _deprecatedGauges.remove(gauge); + // add and fail loud if zero address or already present and not deprecated + if (gauge == address(0) || !(newAdd || previouslyDeprecated)) + revert InvalidGaugeError(); + + uint32 currentCycle = _getGaugeCycleEnd(); + + // Check if some previous weight exists and re-add to total. Gauge and user weights are preserved. + weight = _getGaugeWeight[gauge].currentWeight; + if (weight > 0) { + _writeGaugeWeight(_totalWeight, _add, weight, currentCycle); + } + + emit AddGauge(gauge); + } + + function _removeGauge(address gauge) internal { + // add to deprecated and fail loud if not present + if (!_deprecatedGauges.add(gauge)) revert InvalidGaugeError(); + + uint32 currentCycle = _getGaugeCycleEnd(); + + // Remove weight from total but keep the gauge and user weights in storage in case gauge is re-added. + uint112 weight = _getGaugeWeight[gauge].currentWeight; + if (weight > 0) { + _writeGaugeWeight(_totalWeight, _subtract, weight, currentCycle); + } + + emit RemoveGauge(gauge); + } + + /// @notice set the new max gauges. Requires auth by `authority`. + /// @dev if this is set to a lower number than the current max, users MAY have more gauges active than the max. Use `numUserGauges` to check this. + function _setMaxGauges(uint256 newMax) internal { + uint256 oldMax = maxGauges; + maxGauges = newMax; + + emit MaxGaugesUpdate(oldMax, newMax); + } + + /// @notice set the canExceedMaxGauges flag for an account. + function _setCanExceedMaxGauges( + address account, + bool canExceedMax + ) internal { + canExceedMaxGauges[account] = canExceedMax; + + emit CanExceedMaxGaugesUpdate(account, canExceedMax); + } + + /*/////////////////////////////////////////////////////////////// + ERC20 LOGIC + //////////////////////////////////////////////////////////////*/ + + /// NOTE: any "removal" of tokens from a user requires userUnusedWeight < amount. + /// _decrementWeightUntilFree is called as a greedy algorithm to free up weight. + /// It may be more gas efficient to free weight before burning or transferring tokens. + + function _burn(address from, uint256 amount) internal virtual override { + _decrementWeightUntilFree(from, amount); + super._burn(from, amount); + } + + function transfer( + address to, + uint256 amount + ) public virtual override returns (bool) { + _decrementWeightUntilFree(msg.sender, amount); + return super.transfer(to, amount); + } + + function transferFrom( + address from, + address to, + uint256 amount + ) public virtual override returns (bool) { + _decrementWeightUntilFree(from, amount); + return super.transferFrom(from, to, amount); + } + + /// a greedy algorithm for freeing weight before a token burn/transfer + /// frees up entire gauges, so likely will free more than `weight` + function _decrementWeightUntilFree(address user, uint256 weight) internal { + uint256 userFreeWeight = balanceOf(user) - getUserWeight[user]; + + // early return if already free + if (userFreeWeight >= weight) return; + + uint32 currentCycle = _getGaugeCycleEnd(); + + // cache totals for batch updates + uint112 userFreed; + uint112 totalFreed; + + // Loop through all user gauges, live and deprecated + address[] memory gaugeList = _userGauges[user].values(); + + // Free gauges until through entire list or under weight + uint256 size = gaugeList.length; + for ( + uint256 i = 0; + i < size && (userFreeWeight + totalFreed) < weight; + + ) { + address gauge = gaugeList[i]; + uint112 userGaugeWeight = getUserGaugeWeight[user][gauge]; + if (userGaugeWeight != 0) { + // If the gauge is live (not deprecated), include its weight in the total to remove + if (!_deprecatedGauges.contains(gauge)) { + totalFreed += userGaugeWeight; + } + userFreed += userGaugeWeight; + _decrementGaugeWeight( + user, + gauge, + userGaugeWeight, + currentCycle + ); + + unchecked { + i++; + } + } + } + + getUserWeight[user] -= userFreed; + _writeGaugeWeight(_totalWeight, _subtract, totalFreed, currentCycle); + } +} diff --git a/test/mock/MockERC20Gauges.sol b/test/mock/MockERC20Gauges.sol new file mode 100644 index 00000000..21ed6a63 --- /dev/null +++ b/test/mock/MockERC20Gauges.sol @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity >=0.8.0; + +import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import {MockERC20} from "@test/mock/MockERC20.sol"; +import {ERC20Gauges} from "@voltprotocol/governance/ERC20Gauges.sol"; + +contract MockERC20Gauges is ERC20Gauges, MockERC20 { + constructor( + uint32 _cycleLength, + uint32 _freezeWindow + ) ERC20Gauges(_cycleLength, _freezeWindow) {} + + /// ------------------------------------------------------------------------ + /// Open access to internal functions. + /// ------------------------------------------------------------------------ + + function addGauge(address gauge) external returns (uint112) { + return _addGauge(gauge); + } + + function removeGauge(address gauge) external { + _removeGauge(gauge); + } + + function setMaxGauges(uint256 max) external { + _setMaxGauges(max); + } + + function setCanExceedMaxGauges(address who, bool can) external { + _setCanExceedMaxGauges(who, can); + } + + /// ------------------------------------------------------------------------ + /// Overrides required by Solidity. + /// ------------------------------------------------------------------------ + + function _burn( + address from, + uint256 amount + ) internal override(ERC20, ERC20Gauges) { + return super._burn(from, amount); + } + + function transfer( + address to, + uint256 amount + ) public override(ERC20, ERC20Gauges) returns (bool) { + return super.transfer(to, amount); + } + + function transferFrom( + address from, + address to, + uint256 amount + ) public override(ERC20, ERC20Gauges) returns (bool) { + return super.transferFrom(from, to, amount); + } +} diff --git a/test/unit/governance/ERC20Gauges.t.sol b/test/unit/governance/ERC20Gauges.t.sol new file mode 100644 index 00000000..6eeb4c14 --- /dev/null +++ b/test/unit/governance/ERC20Gauges.t.sol @@ -0,0 +1,695 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +pragma solidity 0.8.13; + +import {Test} from "@forge-std/Test.sol"; +import {MockERC20Gauges} from "@test/mock/MockERC20Gauges.sol"; + +contract ERC20GaugesUnitTest is Test { + MockERC20Gauges token; + address constant gauge1 = address(0xDEAD); + address constant gauge2 = address(0xBEEF); + + function setUp() public { + token = new MockERC20Gauges(3600, 600); // 1 hour cycles, 10 minute freeze + } + + /*/////////////////////////////////////////////////////////////// + TEST ADMIN GAUGE OPERATIONS + //////////////////////////////////////////////////////////////*/ + + function testSetMaxGauges(uint256 max) public { + token.setMaxGauges(max); + require(token.maxGauges() == max); + } + + function testSetCanExceedMaxGauges() public { + token.setCanExceedMaxGauges(address(this), true); + require(token.canExceedMaxGauges(address(this))); + } + + function testAddGauge(address[8] memory gauges) public { + token.setMaxGauges(8); + + uint256 uniqueGauges; + for (uint256 i = 0; i < 8; i++) { + if (token.isGauge(gauges[i]) || gauges[i] == address(0)) { + vm.expectRevert(abi.encodeWithSignature("InvalidGaugeError()")); + token.addGauge(gauges[i]); + } else { + token.addGauge(gauges[i]); + require(token.numGauges() == uniqueGauges + 1); + require(token.gauges()[uniqueGauges] == gauges[i]); + uniqueGauges++; + } + } + } + + function testAddPreviouslyDeprecated(uint112 amount) public { + token.setMaxGauges(2); + token.addGauge(gauge1); + + token.mint(address(this), amount); + token.incrementGauge(gauge1, amount); + + token.removeGauge(gauge1); + token.addGauge(gauge1); + + require(token.numGauges() == 1); + require(token.totalWeight() == amount); + require(token.getGaugeWeight(gauge1) == amount); + require(token.getUserGaugeWeight(address(this), gauge1) == amount); + require(token.deprecatedGauges().length == 0); + } + + function testAddGaugeTwice() public { + token.setMaxGauges(2); + token.addGauge(gauge1); + vm.expectRevert(abi.encodeWithSignature("InvalidGaugeError()")); + token.addGauge(gauge1); + } + + function testRemoveGauge() public { + token.setMaxGauges(2); + token.addGauge(gauge1); + token.removeGauge(gauge1); + require(token.numGauges() == 1); + require(token.numDeprecatedGauges() == 1); + require(token.deprecatedGauges()[0] == gauge1); + } + + function testRemoveGaugeTwice() public { + token.setMaxGauges(2); + token.addGauge(gauge1); + token.removeGauge(gauge1); + vm.expectRevert(abi.encodeWithSignature("InvalidGaugeError()")); + token.removeGauge(gauge1); + } + + function testRemoveGaugeWithWeight(uint112 amount) public { + token.mint(address(this), amount); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.incrementGauge(gauge1, amount); + + token.removeGauge(gauge1); + require(token.numGauges() == 1); + require(token.numDeprecatedGauges() == 1); + require(token.totalWeight() == 0); + require(token.getGaugeWeight(gauge1) == amount); + require(token.getUserGaugeWeight(address(this), gauge1) == amount); + } + + /*/////////////////////////////////////////////////////////////// + TEST USER GAUGE OPERATIONS + //////////////////////////////////////////////////////////////*/ + + function testCalculateGaugeAllocation() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + require(token.incrementGauge(gauge1, 1e18) == 1e18); + require(token.incrementGauge(gauge2, 1e18) == 2e18); + + vm.warp(3600); // warp 1 hour to store changes + require(token.calculateGaugeAllocation(gauge1, 100e18) == 50e18); + require(token.calculateGaugeAllocation(gauge2, 100e18) == 50e18); + + require(token.incrementGauge(gauge2, 2e18) == 4e18); + + // ensure updates don't propagate until stored + require(token.calculateGaugeAllocation(gauge1, 100e18) == 50e18); + require(token.calculateGaugeAllocation(gauge2, 100e18) == 50e18); + + vm.warp(7200); // warp another hour to store changes again + require(token.calculateGaugeAllocation(gauge1, 100e18) == 25e18); + require(token.calculateGaugeAllocation(gauge2, 100e18) == 75e18); + } + + function testIncrement( + address[8] memory from, + address[8] memory gauges, + uint112[8] memory amounts + ) public { + token.setMaxGauges(8); + unchecked { + uint112 sum; + for (uint256 i = 0; i < 8; i++) { + vm.assume(from[i] != address(0)); // cannot mint to address(0) + vm.assume( + sum + amounts[i] >= sum && + !token.isGauge(gauges[i]) && + gauges[i] != address(0) + ); + sum += amounts[i]; + + token.mint(from[i], amounts[i]); + + uint112 userWeightBefore = token.getUserWeight(from[i]); + uint112 userGaugeWeightBefore = token.getUserGaugeWeight( + from[i], + gauges[i] + ); + uint112 gaugeWeightBefore = token.getGaugeWeight(gauges[i]); + + token.addGauge(gauges[i]); + vm.prank(from[i]); + token.incrementGauge(gauges[i], amounts[i]); + + require( + token.getUserWeight(from[i]) == + userWeightBefore + amounts[i] + ); + require(token.totalWeight() == sum); + require( + token.getUserGaugeWeight(from[i], gauges[i]) == + userGaugeWeightBefore + amounts[i] + ); + require( + token.getGaugeWeight(gauges[i]) == + gaugeWeightBefore + amounts[i] + ); + } + } + } + + function testIncrementDuringFreeze( + uint112 amount, + uint128 cycleOffset + ) public { + vm.assume(amount != 0); + + token.mint(address(this), amount); + token.setMaxGauges(1); + token.addGauge(gauge1); + + // any timestamp in freeze window is unable to increment + vm.warp( + token.getGaugeCycleEnd() - + (cycleOffset % token.incrementFreezeWindow()) - + 1 + ); + + vm.expectRevert(abi.encodeWithSignature("IncrementFreezeError()")); + token.incrementGauge(gauge1, amount); + } + + /// @notice test incrementing over user max + function testIncrementOverMax() public { + token.mint(address(this), 2e18); + + token.setMaxGauges(1); + token.addGauge(gauge1); + token.addGauge(gauge2); + + token.incrementGauge(gauge1, 1e18); + vm.expectRevert(abi.encodeWithSignature("MaxGaugeError()")); + token.incrementGauge(gauge2, 1e18); + } + + /// @notice test incrementing at user max + function testIncrementAtMax() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(1); + token.addGauge(gauge1); + token.addGauge(gauge2); + + token.incrementGauge(gauge1, 1e18); + token.incrementGauge(gauge1, 1e18); + + require(token.getUserGaugeWeight(address(this), gauge1) == 2e18); + require(token.getUserWeight(address(this)) == 2e18); + require(token.getGaugeWeight(gauge1) == 2e18); + require(token.totalWeight() == 2e18); + } + + /// @notice test incrementing over user max + function testIncrementOverMaxApproved( + address[8] memory gauges, + uint112[8] memory amounts, + uint8 max + ) public { + token.setMaxGauges(max % 8); + token.setCanExceedMaxGauges(address(this), true); + + unchecked { + uint112 sum; + for (uint256 i = 0; i < 8; i++) { + vm.assume( + sum + amounts[i] >= sum && + !token.isGauge(gauges[i]) && + gauges[i] != address(0) + ); + sum += amounts[i]; + + token.mint(address(this), amounts[i]); + + uint112 userGaugeWeightBefore = token.getUserGaugeWeight( + address(this), + gauges[i] + ); + uint112 gaugeWeightBefore = token.getGaugeWeight(gauges[i]); + + token.addGauge(gauges[i]); + token.incrementGauge(gauges[i], amounts[i]); + + require(token.getUserWeight(address(this)) == sum); + require(token.totalWeight() == sum); + require( + token.getUserGaugeWeight(address(this), gauges[i]) == + userGaugeWeightBefore + amounts[i] + ); + require( + token.getGaugeWeight(gauges[i]) == + gaugeWeightBefore + amounts[i] + ); + } + } + } + + /// @notice test incrementing and make sure weights are stored + function testIncrementWithStorage() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + // gauge1,user1 +1 + require(token.incrementGauge(gauge1, 1e18) == 1e18); + require(token.getUserGaugeWeight(address(this), gauge1) == 1e18); + require(token.getUserWeight(address(this)) == 1e18); + require(token.getGaugeWeight(gauge1) == 1e18); + require(token.totalWeight() == 1e18); + + require(token.getStoredGaugeWeight(gauge1) == 0); + require(token.storedTotalWeight() == 0); + + vm.warp(block.timestamp + 3600); // warp one cycle + + require(token.getStoredGaugeWeight(gauge1) == 1e18); + require(token.storedTotalWeight() == 1e18); + + // gauge2,user1 +2 + require(token.incrementGauge(gauge2, 2e18) == 3e18); + require(token.getUserGaugeWeight(address(this), gauge2) == 2e18); + require(token.getUserWeight(address(this)) == 3e18); + require(token.getGaugeWeight(gauge2) == 2e18); + require(token.totalWeight() == 3e18); + + require(token.getStoredGaugeWeight(gauge2) == 0); + require(token.storedTotalWeight() == 1e18); + + vm.warp(block.timestamp + 1800); // warp half cycle + + require(token.getStoredGaugeWeight(gauge2) == 0); + require(token.storedTotalWeight() == 1e18); + + // gauge1,user1 +4 + require(token.incrementGauge(gauge1, 4e18) == 7e18); + require(token.getUserGaugeWeight(address(this), gauge1) == 5e18); + require(token.getUserWeight(address(this)) == 7e18); + require(token.getGaugeWeight(gauge1) == 5e18); + require(token.totalWeight() == 7e18); + + vm.warp(block.timestamp + 1800); // warp half cycle + + require(token.getStoredGaugeWeight(gauge1) == 5e18); + require(token.getStoredGaugeWeight(gauge2) == 2e18); + require(token.storedTotalWeight() == 7e18); + + vm.warp(block.timestamp + 3600); // warp full cycle + + require(token.getStoredGaugeWeight(gauge1) == 5e18); + require(token.getStoredGaugeWeight(gauge2) == 2e18); + require(token.storedTotalWeight() == 7e18); + } + + function testIncrementOnDeprecated(uint112 amount) public { + token.setMaxGauges(2); + token.addGauge(gauge1); + token.removeGauge(gauge1); + vm.expectRevert(abi.encodeWithSignature("InvalidGaugeError()")); + token.incrementGauge(gauge1, amount); + } + + function testIncrementOverWeight(uint112 amount) public { + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + vm.assume(amount != type(uint112).max); + token.mint(address(this), amount); + + require(token.incrementGauge(gauge1, amount) == amount); + vm.expectRevert(abi.encodeWithSignature("OverWeightError()")); + token.incrementGauge(gauge2, 1); + } + + /// @notice test incrementing multiple gauges with different weights after already incrementing once + function testIncrementGauges() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + token.incrementGauge(gauge1, 1e18); + + address[] memory gaugeList = new address[](2); + uint112[] memory weights = new uint112[](2); + gaugeList[0] = gauge2; + gaugeList[1] = gauge1; + weights[0] = 2e18; + weights[1] = 4e18; + + require(token.incrementGauges(gaugeList, weights) == 7e18); + + require(token.getUserGaugeWeight(address(this), gauge2) == 2e18); + require(token.getGaugeWeight(gauge2) == 2e18); + require(token.getUserGaugeWeight(address(this), gauge1) == 5e18); + require(token.getUserWeight(address(this)) == 7e18); + require(token.getGaugeWeight(gauge1) == 5e18); + require(token.totalWeight() == 7e18); + } + + function testIncrementGaugesDeprecated() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + token.removeGauge(gauge2); + + address[] memory gaugeList = new address[](2); + uint112[] memory weights = new uint112[](2); + gaugeList[0] = gauge2; + gaugeList[1] = gauge1; + weights[0] = 2e18; + weights[1] = 4e18; + //vm.expectRevert(abi.encodeWithSignature("InvalidGaugeError()")); + token.incrementGauges(gaugeList, weights); + } + + function testIncrementGaugesOverweight() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + address[] memory gaugeList = new address[](2); + uint112[] memory weights = new uint112[](2); + gaugeList[0] = gauge2; + gaugeList[1] = gauge1; + weights[0] = 50e18; + weights[1] = 51e18; + vm.expectRevert(abi.encodeWithSignature("OverWeightError()")); + token.incrementGauges(gaugeList, weights); + } + + function testIncrementGaugesSizeMismatch() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + token.removeGauge(gauge2); + + address[] memory gaugeList = new address[](2); + uint112[] memory weights = new uint112[](3); + gaugeList[0] = gauge2; + gaugeList[1] = gauge1; + weights[0] = 1e18; + weights[1] = 2e18; + vm.expectRevert(abi.encodeWithSignature("SizeMismatchError()")); + token.incrementGauges(gaugeList, weights); + } + + /// @notice test decrement twice, 2 tokens each after incrementing by 4. + function testDecrement() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + require(token.incrementGauge(gauge1, 4e18) == 4e18); + + require(token.decrementGauge(gauge1, 2e18) == 2e18); + require(token.getUserGaugeWeight(address(this), gauge1) == 2e18); + require(token.getUserWeight(address(this)) == 2e18); + require(token.getGaugeWeight(gauge1) == 2e18); + require(token.totalWeight() == 2e18); + + require(token.decrementGauge(gauge1, 2e18) == 0); + require(token.getUserGaugeWeight(address(this), gauge1) == 0); + require(token.getUserWeight(address(this)) == 0); + require(token.getGaugeWeight(gauge1) == 0); + require(token.totalWeight() == 0); + } + + /// @notice test decrement all removes user gauge. + function testDecrementAllRemovesUserGauge() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + require(token.incrementGauge(gauge1, 4e18) == 4e18); + + require(token.numUserGauges(address(this)) == 1); + require(token.userGauges(address(this))[0] == gauge1); + + require(token.decrementGauge(gauge1, 4e18) == 0); + + require(token.numUserGauges(address(this)) == 0); + } + + /// @notice test decrement twice, 2 tokens each after incrementing by 4. + function testDecrementWithStorage() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + require(token.incrementGauge(gauge1, 4e18) == 4e18); + + require(token.decrementGauge(gauge1, 2e18) == 2e18); + require(token.getUserGaugeWeight(address(this), gauge1) == 2e18); + require(token.getUserWeight(address(this)) == 2e18); + require(token.getGaugeWeight(gauge1) == 2e18); + require(token.totalWeight() == 2e18); + + require(token.getStoredGaugeWeight(gauge1) == 0); + require(token.storedTotalWeight() == 0); + + vm.warp(block.timestamp + 3600); // warp full cycle + + require(token.getStoredGaugeWeight(gauge1) == 2e18); + require(token.storedTotalWeight() == 2e18); + + require(token.decrementGauge(gauge1, 2e18) == 0); + require(token.getUserGaugeWeight(address(this), gauge1) == 0); + require(token.getUserWeight(address(this)) == 0); + require(token.getGaugeWeight(gauge1) == 0); + require(token.totalWeight() == 0); + + require(token.getStoredGaugeWeight(gauge1) == 2e18); + require(token.storedTotalWeight() == 2e18); + + vm.warp(block.timestamp + 3600); // warp full cycle + + require(token.getStoredGaugeWeight(gauge1) == 0); + require(token.storedTotalWeight() == 0); + } + + function testDecrementUnderflow(uint112 amount) public { + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + token.mint(address(this), amount); + + vm.assume(amount != type(uint112).max); + + require(token.incrementGauge(gauge1, amount) == amount); + vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 17)); + token.decrementGauge(gauge1, amount + 1); + } + + function testDecrementGauges() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + token.incrementGauge(gauge1, 1e18); + + address[] memory gaugeList = new address[](2); + uint112[] memory weights = new uint112[](2); + gaugeList[0] = gauge2; + gaugeList[1] = gauge1; + weights[0] = 2e18; + weights[1] = 4e18; + + require(token.incrementGauges(gaugeList, weights) == 7e18); + + weights[1] = 2e18; + require(token.decrementGauges(gaugeList, weights) == 3e18); + + require(token.getUserGaugeWeight(address(this), gauge2) == 0); + require(token.getGaugeWeight(gauge2) == 0); + require(token.getUserGaugeWeight(address(this), gauge1) == 3e18); + require(token.getUserWeight(address(this)) == 3e18); + require(token.getGaugeWeight(gauge1) == 3e18); + require(token.totalWeight() == 3e18); + } + + function testDecrementGaugesOver() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + address[] memory gaugeList = new address[](2); + uint112[] memory weights = new uint112[](2); + gaugeList[0] = gauge2; + gaugeList[1] = gauge1; + weights[0] = 5e18; + weights[1] = 5e18; + + require(token.incrementGauges(gaugeList, weights) == 10e18); + + weights[1] = 10e18; + vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 17)); + token.decrementGauges(gaugeList, weights); + } + + function testDecrementGaugesSizeMismatch() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + address[] memory gaugeList = new address[](2); + uint112[] memory weights = new uint112[](2); + gaugeList[0] = gauge2; + gaugeList[1] = gauge1; + weights[0] = 1e18; + weights[1] = 2e18; + + require(token.incrementGauges(gaugeList, weights) == 3e18); + vm.expectRevert(abi.encodeWithSignature("SizeMismatchError()")); + token.decrementGauges(gaugeList, new uint112[](0)); + } + + /*/////////////////////////////////////////////////////////////// + TEST ERC20 LOGIC + //////////////////////////////////////////////////////////////*/ + + function testDecrementUntilFreeWhenFree() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + require(token.incrementGauge(gauge1, 10e18) == 10e18); + require(token.incrementGauge(gauge2, 20e18) == 30e18); + require(token.userUnusedWeight(address(this)) == 70e18); + + token.mockBurn(address(this), 50e18); + require(token.userUnusedWeight(address(this)) == 20e18); + + require(token.getUserGaugeWeight(address(this), gauge1) == 10e18); + require(token.getUserWeight(address(this)) == 30e18); + require(token.getGaugeWeight(gauge1) == 10e18); + require(token.getUserGaugeWeight(address(this), gauge2) == 20e18); + require(token.getGaugeWeight(gauge2) == 20e18); + require(token.totalWeight() == 30e18); + } + + function testDecrementUntilFreeSingle() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + require(token.incrementGauge(gauge1, 10e18) == 10e18); + require(token.incrementGauge(gauge2, 20e18) == 30e18); + require(token.userUnusedWeight(address(this)) == 70e18); + + token.transfer(address(1), 80e18); + require(token.userUnusedWeight(address(this)) == 0); + + require(token.getUserGaugeWeight(address(this), gauge1) == 0); + require(token.getUserWeight(address(this)) == 20e18); + require(token.getGaugeWeight(gauge1) == 0); + require(token.getUserGaugeWeight(address(this), gauge2) == 20e18); + require(token.getGaugeWeight(gauge2) == 20e18); + require(token.totalWeight() == 20e18); + } + + function testDecrementUntilFreeDouble() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + require(token.incrementGauge(gauge1, 10e18) == 10e18); + require(token.incrementGauge(gauge2, 20e18) == 30e18); + require(token.userUnusedWeight(address(this)) == 70e18); + + token.approve(address(1), 100e18); + vm.prank(address(1)); + token.transferFrom(address(this), address(1), 90e18); + + require(token.userUnusedWeight(address(this)) == 10e18); + + require(token.getUserGaugeWeight(address(this), gauge1) == 0); + require(token.getUserWeight(address(this)) == 0); + require(token.getGaugeWeight(gauge1) == 0); + require(token.getUserGaugeWeight(address(this), gauge2) == 0); + require(token.getGaugeWeight(gauge2) == 0); + require(token.totalWeight() == 0); + } + + function testDecrementUntilFreeDeprecated() public { + token.mint(address(this), 100e18); + + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + require(token.incrementGauge(gauge1, 10e18) == 10e18); + require(token.incrementGauge(gauge2, 20e18) == 30e18); + require(token.userUnusedWeight(address(this)) == 70e18); + + require(token.totalWeight() == 30e18); + token.removeGauge(gauge1); + require(token.totalWeight() == 20e18); + + token.mockBurn(address(this), 100e18); + + require(token.userUnusedWeight(address(this)) == 0); + + require(token.getUserGaugeWeight(address(this), gauge1) == 0); + require(token.getUserWeight(address(this)) == 0); + require(token.getGaugeWeight(gauge1) == 0); + require(token.getUserGaugeWeight(address(this), gauge2) == 0); + require(token.getGaugeWeight(gauge2) == 0); + require(token.totalWeight() == 0); + } +} From 92f94c13ef7343f78989b8d1dc5d7078ffa3e470 Mon Sep 17 00:00:00 2001 From: Erwan Beauvois Date: Fri, 17 Mar 2023 17:23:45 +0100 Subject: [PATCH 2/8] Improve coverage of ERC20Gauges unit tests --- test/unit/governance/ERC20Gauges.t.sol | 92 +++++++++++++++++++++++--- 1 file changed, 82 insertions(+), 10 deletions(-) diff --git a/test/unit/governance/ERC20Gauges.t.sol b/test/unit/governance/ERC20Gauges.t.sol index 6eeb4c14..4036e33a 100644 --- a/test/unit/governance/ERC20Gauges.t.sol +++ b/test/unit/governance/ERC20Gauges.t.sol @@ -9,8 +9,43 @@ contract ERC20GaugesUnitTest is Test { address constant gauge1 = address(0xDEAD); address constant gauge2 = address(0xBEEF); + uint32 constant _CYCLE_LENGTH = 1 hours; + uint32 constant _FREEZE_PERIOD = 10 minutes; + function setUp() public { - token = new MockERC20Gauges(3600, 600); // 1 hour cycles, 10 minute freeze + vm.warp(1679067867); + vm.roll(16848497); + token = new MockERC20Gauges(_CYCLE_LENGTH, _FREEZE_PERIOD); + } + + /*/////////////////////////////////////////////////////////////// + TEST INITIAL STATE + //////////////////////////////////////////////////////////////*/ + + function testInitialState() public { + assertEq(token.gaugeCycleLength(), _CYCLE_LENGTH); + assertEq(token.incrementFreezeWindow(), _FREEZE_PERIOD); + assertEq(token.getUserGaugeWeight(address(this), gauge1), 0); + assertEq(token.getUserGaugeWeight(address(this), gauge2), 0); + assertEq(token.getUserWeight(address(this)), 0); + assertEq(token.getGaugeCycleEnd(), 1679068800); + assertEq(token.getGaugeWeight(gauge1), 0); + assertEq(token.getGaugeWeight(gauge2), 0); + assertEq(token.getStoredGaugeWeight(gauge1), 0); + assertEq(token.getStoredGaugeWeight(gauge2), 0); + assertEq(token.totalWeight(), 0); + assertEq(token.storedTotalWeight(), 0); + assertEq(token.gauges().length, 0); + assertEq(token.gauges(0, 0).length, 0); + assertEq(token.isGauge(gauge1), false); + assertEq(token.isGauge(gauge2), false); + assertEq(token.numGauges(), 0); + assertEq(token.deprecatedGauges().length, 0); + assertEq(token.numDeprecatedGauges(), 0); + assertEq(token.userGauges(address(this)).length, 0); + assertEq(token.isUserGauge(address(this), gauge1), false); + assertEq(token.isUserGauge(address(this), gauge2), false); + assertEq(token.userGauges(address(this), 0, 0).length, 0); } /*/////////////////////////////////////////////////////////////// @@ -68,6 +103,16 @@ contract ERC20GaugesUnitTest is Test { token.addGauge(gauge1); } + function testGetGaugesPaginated() public { + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + + assertEq(token.gauges(0, 1).length, 1); + assertEq(token.gauges(0, 1)[0], gauge1); + assertEq(token.gauges(1, 1)[0], gauge2); + } + function testRemoveGauge() public { token.setMaxGauges(2); token.addGauge(gauge1); @@ -114,7 +159,7 @@ contract ERC20GaugesUnitTest is Test { require(token.incrementGauge(gauge1, 1e18) == 1e18); require(token.incrementGauge(gauge2, 1e18) == 2e18); - vm.warp(3600); // warp 1 hour to store changes + vm.warp(block.timestamp + _CYCLE_LENGTH); // warp 1 cycle to store changes require(token.calculateGaugeAllocation(gauge1, 100e18) == 50e18); require(token.calculateGaugeAllocation(gauge2, 100e18) == 50e18); @@ -124,9 +169,18 @@ contract ERC20GaugesUnitTest is Test { require(token.calculateGaugeAllocation(gauge1, 100e18) == 50e18); require(token.calculateGaugeAllocation(gauge2, 100e18) == 50e18); - vm.warp(7200); // warp another hour to store changes again + vm.warp(block.timestamp + _CYCLE_LENGTH); // warp another cycle to store changes again require(token.calculateGaugeAllocation(gauge1, 100e18) == 25e18); require(token.calculateGaugeAllocation(gauge2, 100e18) == 75e18); + + // deprecate gauge + token.removeGauge(gauge1); + require(token.calculateGaugeAllocation(gauge1, 100e18) == 0); + require(token.calculateGaugeAllocation(gauge2, 100e18) == 75e18); + + vm.warp(block.timestamp + _CYCLE_LENGTH); // warp another cycle to store changes again + require(token.calculateGaugeAllocation(gauge1, 100e18) == 0); + require(token.calculateGaugeAllocation(gauge2, 100e18) == 100e18); } function testIncrement( @@ -176,6 +230,21 @@ contract ERC20GaugesUnitTest is Test { } } + function testGetUserGaugesPaginated() public { + token.setMaxGauges(2); + token.addGauge(gauge1); + token.addGauge(gauge2); + token.mint(address(this), 100e18); + token.incrementGauge(gauge1, 40e18); + token.incrementGauge(gauge2, 60e18); + + assertEq(token.isUserGauge(address(this), gauge1), true); + assertEq(token.isUserGauge(address(this), gauge2), true); + assertEq(token.userGauges(address(this), 0, 1).length, 1); + assertEq(token.userGauges(address(this), 0, 1)[0], gauge1); + assertEq(token.userGauges(address(this), 1, 1)[0], gauge2); + } + function testIncrementDuringFreeze( uint112 amount, uint128 cycleOffset @@ -279,6 +348,9 @@ contract ERC20GaugesUnitTest is Test { token.addGauge(gauge1); token.addGauge(gauge2); + // warp to end of cycle + vm.warp(token.getGaugeCycleEnd()); + // gauge1,user1 +1 require(token.incrementGauge(gauge1, 1e18) == 1e18); require(token.getUserGaugeWeight(address(this), gauge1) == 1e18); @@ -289,7 +361,7 @@ contract ERC20GaugesUnitTest is Test { require(token.getStoredGaugeWeight(gauge1) == 0); require(token.storedTotalWeight() == 0); - vm.warp(block.timestamp + 3600); // warp one cycle + vm.warp(block.timestamp + _CYCLE_LENGTH); // warp one cycle require(token.getStoredGaugeWeight(gauge1) == 1e18); require(token.storedTotalWeight() == 1e18); @@ -304,7 +376,7 @@ contract ERC20GaugesUnitTest is Test { require(token.getStoredGaugeWeight(gauge2) == 0); require(token.storedTotalWeight() == 1e18); - vm.warp(block.timestamp + 1800); // warp half cycle + vm.warp(block.timestamp + _CYCLE_LENGTH / 2); // warp half cycle require(token.getStoredGaugeWeight(gauge2) == 0); require(token.storedTotalWeight() == 1e18); @@ -316,13 +388,13 @@ contract ERC20GaugesUnitTest is Test { require(token.getGaugeWeight(gauge1) == 5e18); require(token.totalWeight() == 7e18); - vm.warp(block.timestamp + 1800); // warp half cycle + vm.warp(block.timestamp + _CYCLE_LENGTH / 2); // warp half cycle require(token.getStoredGaugeWeight(gauge1) == 5e18); require(token.getStoredGaugeWeight(gauge2) == 2e18); require(token.storedTotalWeight() == 7e18); - vm.warp(block.timestamp + 3600); // warp full cycle + vm.warp(block.timestamp + _CYCLE_LENGTH); // warp full cycle require(token.getStoredGaugeWeight(gauge1) == 5e18); require(token.getStoredGaugeWeight(gauge2) == 2e18); @@ -391,7 +463,7 @@ contract ERC20GaugesUnitTest is Test { gaugeList[1] = gauge1; weights[0] = 2e18; weights[1] = 4e18; - //vm.expectRevert(abi.encodeWithSignature("InvalidGaugeError()")); + vm.expectRevert(abi.encodeWithSignature("InvalidGaugeError()")); token.incrementGauges(gaugeList, weights); } @@ -490,7 +562,7 @@ contract ERC20GaugesUnitTest is Test { require(token.getStoredGaugeWeight(gauge1) == 0); require(token.storedTotalWeight() == 0); - vm.warp(block.timestamp + 3600); // warp full cycle + vm.warp(block.timestamp + _CYCLE_LENGTH); // warp full cycle require(token.getStoredGaugeWeight(gauge1) == 2e18); require(token.storedTotalWeight() == 2e18); @@ -504,7 +576,7 @@ contract ERC20GaugesUnitTest is Test { require(token.getStoredGaugeWeight(gauge1) == 2e18); require(token.storedTotalWeight() == 2e18); - vm.warp(block.timestamp + 3600); // warp full cycle + vm.warp(block.timestamp + _CYCLE_LENGTH); // warp full cycle require(token.getStoredGaugeWeight(gauge1) == 0); require(token.storedTotalWeight() == 0); From 0882f2761763a635d2577a29374da8c5b6d016bd Mon Sep 17 00:00:00 2001 From: Erwan Beauvois Date: Tue, 21 Mar 2023 11:51:34 +0100 Subject: [PATCH 3/8] Update ERC20Gauges error style --- src/governance/ERC20Gauges.sol | 56 +++++++++++--------------- test/unit/governance/ERC20Gauges.t.sol | 22 +++++----- 2 files changed, 35 insertions(+), 43 deletions(-) diff --git a/src/governance/ERC20Gauges.sol b/src/governance/ERC20Gauges.sol index 60fdcda8..240d4dbf 100644 --- a/src/governance/ERC20Gauges.sol +++ b/src/governance/ERC20Gauges.sol @@ -45,15 +45,17 @@ import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet ... Rename to remove "contract" from name because we don't check if target is a contract - Consistency: make incrementGauges return a uint112 instead of uint256 - Import OpenZeppelin ERC20 & EnumerableSet instead of Solmate's - - (TODO) Change error management style + - Changed error management style (use require + messages instead of Solidity errors) - (TODO) Add negative weight voting */ abstract contract ERC20Gauges is ERC20 { using EnumerableSet for EnumerableSet.AddressSet; constructor(uint32 _gaugeCycleLength, uint32 _incrementFreezeWindow) { - if (_incrementFreezeWindow >= _gaugeCycleLength) - revert IncrementFreezeError(); + require( + _incrementFreezeWindow < _gaugeCycleLength, + "ERC20Gauges: invalid increment freeze" + ); gaugeCycleLength = _gaugeCycleLength; incrementFreezeWindow = _incrementFreezeWindow; } @@ -253,18 +255,6 @@ abstract contract ERC20Gauges is ERC20 { USER GAUGE OPERATIONS //////////////////////////////////////////////////////////////*/ - /// @notice thrown when trying to increment/decrement a mismatched number of gauges and weights. - error SizeMismatchError(); - - /// @notice thrown when trying to increment over the max allowed gauges. - error MaxGaugeError(); - - /// @notice thrown when incrementing over a users free weight. - error OverWeightError(); - - /// @notice thrown when incremending during the freeze window. - error IncrementFreezeError(); - /// @notice emitted when incrementing a gauge event IncrementGaugeWeight( address indexed user, @@ -302,18 +292,21 @@ abstract contract ERC20Gauges is ERC20 { uint112 weight, uint32 cycle ) internal { - if (_deprecatedGauges.contains(gauge)) revert InvalidGaugeError(); + require( + !_deprecatedGauges.contains(gauge), + "ERC20Gauges: deprecated gauge" + ); unchecked { - if (cycle - block.timestamp <= incrementFreezeWindow) - revert IncrementFreezeError(); + require( + cycle - block.timestamp > incrementFreezeWindow, + "ERC20Gauges: freeze period" + ); } bool added = _userGauges[user].add(gauge); // idempotent add - if ( - added && - _userGauges[user].length() > maxGauges && - !canExceedMaxGauges[user] - ) revert MaxGaugeError(); + if (added && _userGauges[user].length() > maxGauges) { + require(canExceedMaxGauges[user], "ERC20Gauges: exceed max gauges"); + } getUserGaugeWeight[user][gauge] += weight; @@ -329,7 +322,7 @@ abstract contract ERC20Gauges is ERC20 { ) internal returns (uint112 newUserWeight) { newUserWeight = getUserWeight[user] + weight; // Ensure under weight - if (newUserWeight > balanceOf(user)) revert OverWeightError(); + require(newUserWeight <= balanceOf(user), "ERC20Gauges: overweight"); // Update gauge state getUserWeight[user] = newUserWeight; @@ -348,7 +341,7 @@ abstract contract ERC20Gauges is ERC20 { uint112[] calldata weights ) external returns (uint112 newUserWeight) { uint256 size = gaugeList.length; - if (weights.length != size) revert SizeMismatchError(); + require(weights.length == size, "ERC20Gauges: size mismatch"); // store total in summary for batch update on user/global state uint112 weightsSum; @@ -432,7 +425,7 @@ abstract contract ERC20Gauges is ERC20 { uint112[] calldata weights ) external returns (uint112 newUserWeight) { uint256 size = gaugeList.length; - if (weights.length != size) revert SizeMismatchError(); + require(weights.length == size, "ERC20Gauges: size mismatch"); // store total in summary for batch update on user/global state uint112 weightsSum; @@ -494,9 +487,6 @@ abstract contract ERC20Gauges is ERC20 { ADMIN GAUGE OPERATIONS //////////////////////////////////////////////////////////////*/ - /// @notice thrown when trying to increment or remove a non-live gauge, or add a live gauge. - error InvalidGaugeError(); - /// @notice emitted when adding a new gauge to the live set. event AddGauge(address indexed gauge); @@ -523,8 +513,10 @@ abstract contract ERC20Gauges is ERC20 { bool newAdd = _gauges.add(gauge); bool previouslyDeprecated = _deprecatedGauges.remove(gauge); // add and fail loud if zero address or already present and not deprecated - if (gauge == address(0) || !(newAdd || previouslyDeprecated)) - revert InvalidGaugeError(); + require( + gauge != address(0) && (newAdd || previouslyDeprecated), + "ERC20Gauges: invalid gauge" + ); uint32 currentCycle = _getGaugeCycleEnd(); @@ -539,7 +531,7 @@ abstract contract ERC20Gauges is ERC20 { function _removeGauge(address gauge) internal { // add to deprecated and fail loud if not present - if (!_deprecatedGauges.add(gauge)) revert InvalidGaugeError(); + require(_deprecatedGauges.add(gauge), "ERC20Gauges: invalid gauge"); uint32 currentCycle = _getGaugeCycleEnd(); diff --git a/test/unit/governance/ERC20Gauges.t.sol b/test/unit/governance/ERC20Gauges.t.sol index 4036e33a..4a7f570b 100644 --- a/test/unit/governance/ERC20Gauges.t.sol +++ b/test/unit/governance/ERC20Gauges.t.sol @@ -68,7 +68,7 @@ contract ERC20GaugesUnitTest is Test { uint256 uniqueGauges; for (uint256 i = 0; i < 8; i++) { if (token.isGauge(gauges[i]) || gauges[i] == address(0)) { - vm.expectRevert(abi.encodeWithSignature("InvalidGaugeError()")); + vm.expectRevert("ERC20Gauges: invalid gauge"); token.addGauge(gauges[i]); } else { token.addGauge(gauges[i]); @@ -99,7 +99,7 @@ contract ERC20GaugesUnitTest is Test { function testAddGaugeTwice() public { token.setMaxGauges(2); token.addGauge(gauge1); - vm.expectRevert(abi.encodeWithSignature("InvalidGaugeError()")); + vm.expectRevert("ERC20Gauges: invalid gauge"); token.addGauge(gauge1); } @@ -126,7 +126,7 @@ contract ERC20GaugesUnitTest is Test { token.setMaxGauges(2); token.addGauge(gauge1); token.removeGauge(gauge1); - vm.expectRevert(abi.encodeWithSignature("InvalidGaugeError()")); + vm.expectRevert("ERC20Gauges: invalid gauge"); token.removeGauge(gauge1); } @@ -262,7 +262,7 @@ contract ERC20GaugesUnitTest is Test { 1 ); - vm.expectRevert(abi.encodeWithSignature("IncrementFreezeError()")); + vm.expectRevert("ERC20Gauges: freeze period"); token.incrementGauge(gauge1, amount); } @@ -275,7 +275,7 @@ contract ERC20GaugesUnitTest is Test { token.addGauge(gauge2); token.incrementGauge(gauge1, 1e18); - vm.expectRevert(abi.encodeWithSignature("MaxGaugeError()")); + vm.expectRevert("ERC20Gauges: exceed max gauges"); token.incrementGauge(gauge2, 1e18); } @@ -405,7 +405,7 @@ contract ERC20GaugesUnitTest is Test { token.setMaxGauges(2); token.addGauge(gauge1); token.removeGauge(gauge1); - vm.expectRevert(abi.encodeWithSignature("InvalidGaugeError()")); + vm.expectRevert("ERC20Gauges: deprecated gauge"); token.incrementGauge(gauge1, amount); } @@ -418,7 +418,7 @@ contract ERC20GaugesUnitTest is Test { token.mint(address(this), amount); require(token.incrementGauge(gauge1, amount) == amount); - vm.expectRevert(abi.encodeWithSignature("OverWeightError()")); + vm.expectRevert("ERC20Gauges: overweight"); token.incrementGauge(gauge2, 1); } @@ -463,7 +463,7 @@ contract ERC20GaugesUnitTest is Test { gaugeList[1] = gauge1; weights[0] = 2e18; weights[1] = 4e18; - vm.expectRevert(abi.encodeWithSignature("InvalidGaugeError()")); + vm.expectRevert("ERC20Gauges: deprecated gauge"); token.incrementGauges(gaugeList, weights); } @@ -480,7 +480,7 @@ contract ERC20GaugesUnitTest is Test { gaugeList[1] = gauge1; weights[0] = 50e18; weights[1] = 51e18; - vm.expectRevert(abi.encodeWithSignature("OverWeightError()")); + vm.expectRevert("ERC20Gauges: overweight"); token.incrementGauges(gaugeList, weights); } @@ -498,7 +498,7 @@ contract ERC20GaugesUnitTest is Test { gaugeList[1] = gauge1; weights[0] = 1e18; weights[1] = 2e18; - vm.expectRevert(abi.encodeWithSignature("SizeMismatchError()")); + vm.expectRevert("ERC20Gauges: size mismatch"); token.incrementGauges(gaugeList, weights); } @@ -661,7 +661,7 @@ contract ERC20GaugesUnitTest is Test { weights[1] = 2e18; require(token.incrementGauges(gaugeList, weights) == 3e18); - vm.expectRevert(abi.encodeWithSignature("SizeMismatchError()")); + vm.expectRevert("ERC20Gauges: size mismatch"); token.decrementGauges(gaugeList, new uint112[](0)); } From 4f97a1d2e18b45293e060c29556e1cd0b901ed57 Mon Sep 17 00:00:00 2001 From: Erwan Beauvois Date: Wed, 22 Mar 2023 16:14:21 +0100 Subject: [PATCH 4/8] Update comment in ERC20Gauges.sol --- src/governance/ERC20Gauges.sol | 37 +++++++++++++++++----------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/src/governance/ERC20Gauges.sol b/src/governance/ERC20Gauges.sol index 240d4dbf..544b8ee9 100644 --- a/src/governance/ERC20Gauges.sol +++ b/src/governance/ERC20Gauges.sol @@ -5,23 +5,23 @@ import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; /** - @title An ERC20 with an embedded "Gauge" style vote with liquid weights - @author joeysantoro, eswak - @notice This contract is meant to be used to support gauge style votes with weights associated with resource allocation. - Holders can allocate weight in any proportion to supported gauges. - A "gauge" is represented by an address which would receive the resources periodically or continuously. - For example, gauges can be used to direct token emissions, similar to Curve or Tokemak. - Alternatively, gauges can be used to direct another quantity such as relative access to a line of credit. - This contract is abstract, and a parent shall implement public setter with adequate access control to manage - the gauge set and caps. - "Live" gauges are in the set `_gauges`. - Users can only add weight to live gauges but can remove weight from live or deprecated gauges. - Gauges can be deprecated and reinstated, and will maintain any non-removed weight from before. - @dev SECURITY NOTES: `maxGauges` is a critical variable to protect against gas DOS attacks upon token transfer. - This must be low enough to allow complicated transactions to fit in a block. - Weight state is preserved on the gauge and user level even when a gauge is removed, in case it is re-added. - This maintains state efficiently, and global accounting is managed only on the `_totalWeight` - @dev This contract was originally published as part of TribeDAO's flywheel-v2 repo, please see: +@title An ERC20 with an embedded "Gauge" style vote with liquid weights +@author joeysantoro, eswak +@notice This contract is meant to be used to support gauge style votes with weights associated with resource allocation. + Holders can allocate weight in any proportion to supported gauges. + A "gauge" is represented by an address which would receive the resources periodically or continuously. + For example, gauges can be used to direct token emissions, similar to Curve or Tokemak. + Alternatively, gauges can be used to direct another quantity such as relative access to a line of credit. + This contract is abstract, and a parent shall implement public setter with adequate access control to manage + the gauge set and caps. + "Live" gauges are in the set `_gauges`. + Users can only add weight to live gauges but can remove weight from live or deprecated gauges. + Gauges can be deprecated and reinstated, and will maintain any non-removed weight from before. +@dev SECURITY NOTES: `maxGauges` is a critical variable to protect against gas DOS attacks upon token transfer. + This must be low enough to allow complicated transactions to fit in a block. + Weight state is preserved on the gauge and user level even when a gauge is removed, in case it is re-added. + This maintains state efficiently, and global accounting is managed only on the `_totalWeight` +@dev This contract was originally published as part of TribeDAO's flywheel-v2 repo, please see: https://github.com/fei-protocol/flywheel-v2/blob/main/src/token/ERC20Gauges.sol The original version was included in 2 audits : - https://code4rena.com/reports/2022-04-xtribe/ @@ -45,8 +45,7 @@ import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet ... Rename to remove "contract" from name because we don't check if target is a contract - Consistency: make incrementGauges return a uint112 instead of uint256 - Import OpenZeppelin ERC20 & EnumerableSet instead of Solmate's - - Changed error management style (use require + messages instead of Solidity errors) - - (TODO) Add negative weight voting + - Update error management style (use require + messages instead of Solidity errors) */ abstract contract ERC20Gauges is ERC20 { using EnumerableSet for EnumerableSet.AddressSet; From 0005a75a00c8751b9877b800c5850beef72403c8 Mon Sep 17 00:00:00 2001 From: Erwan Beauvois Date: Thu, 23 Mar 2023 11:31:54 +0100 Subject: [PATCH 5/8] Minor gas improvements from C4 audit [G-02] Use prefix not postfix in loops [G-04] Use != 0 instead of > 0 --- src/governance/ERC20Gauges.sol | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/governance/ERC20Gauges.sol b/src/governance/ERC20Gauges.sol index 544b8ee9..4bbd2b66 100644 --- a/src/governance/ERC20Gauges.sol +++ b/src/governance/ERC20Gauges.sol @@ -164,7 +164,7 @@ abstract contract ERC20Gauges is ERC20 { for (uint256 i = 0; i < num; ) { unchecked { values[i] = _gauges.at(offset + i); // will revert if out of bounds - i++; + ++i; } } } @@ -217,7 +217,7 @@ abstract contract ERC20Gauges is ERC20 { for (uint256 i = 0; i < num; ) { unchecked { values[i] = _userGauges[user].at(offset + i); // will revert if out of bounds - i++; + ++i; } } } @@ -355,7 +355,7 @@ abstract contract ERC20Gauges is ERC20 { _incrementGaugeWeight(msg.sender, gauge, weight, currentCycle); unchecked { - i++; + ++i; } } return @@ -440,7 +440,7 @@ abstract contract ERC20Gauges is ERC20 { _decrementGaugeWeight(msg.sender, gauge, weight, currentCycle); unchecked { - i++; + ++i; } } return @@ -521,7 +521,7 @@ abstract contract ERC20Gauges is ERC20 { // Check if some previous weight exists and re-add to total. Gauge and user weights are preserved. weight = _getGaugeWeight[gauge].currentWeight; - if (weight > 0) { + if (weight != 0) { _writeGaugeWeight(_totalWeight, _add, weight, currentCycle); } @@ -536,7 +536,7 @@ abstract contract ERC20Gauges is ERC20 { // Remove weight from total but keep the gauge and user weights in storage in case gauge is re-added. uint112 weight = _getGaugeWeight[gauge].currentWeight; - if (weight > 0) { + if (weight != 0) { _writeGaugeWeight(_totalWeight, _subtract, weight, currentCycle); } @@ -632,7 +632,7 @@ abstract contract ERC20Gauges is ERC20 { ); unchecked { - i++; + ++i; } } } From ed19e4e79e721e704242cccbfe271637fbac3dd0 Mon Sep 17 00:00:00 2001 From: Erwan Beauvois Date: Thu, 23 Mar 2023 16:27:07 +0100 Subject: [PATCH 6/8] Add check on gauge existence to increment weight Fixes the following issues from C4 audit: [M-03] ERC20Gauges: The _incrementGaugeWeight function does not check the gauge parameter enough, so the user may lose rewards [M-04] In ERC20Gauges, contribution to total weight is double-counted when incrementGauge is called before addGauge for a given gauge. --- src/governance/ERC20Gauges.sol | 11 ++++++----- test/unit/governance/ERC20Gauges.t.sol | 16 ++++++++++++++-- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/governance/ERC20Gauges.sol b/src/governance/ERC20Gauges.sol index 4bbd2b66..37f3cdf9 100644 --- a/src/governance/ERC20Gauges.sol +++ b/src/governance/ERC20Gauges.sol @@ -170,7 +170,7 @@ abstract contract ERC20Gauges is ERC20 { } /// @notice returns true if `gauge` is not in deprecated gauges - function isGauge(address gauge) external view returns (bool) { + function isGauge(address gauge) public view returns (bool) { return _gauges.contains(gauge) && !_deprecatedGauges.contains(gauge); } @@ -280,21 +280,20 @@ abstract contract ERC20Gauges is ERC20 { address gauge, uint112 weight ) external returns (uint112 newUserWeight) { + require(isGauge(gauge), "ERC20Gauges: invalid gauge"); uint32 currentCycle = _getGaugeCycleEnd(); _incrementGaugeWeight(msg.sender, gauge, weight, currentCycle); return _incrementUserAndGlobalWeights(msg.sender, weight, currentCycle); } + /// @dev this function does not check if the gauge exists, this is performed + /// in the calling function. function _incrementGaugeWeight( address user, address gauge, uint112 weight, uint32 cycle ) internal { - require( - !_deprecatedGauges.contains(gauge), - "ERC20Gauges: deprecated gauge" - ); unchecked { require( cycle - block.timestamp > incrementFreezeWindow, @@ -353,6 +352,8 @@ abstract contract ERC20Gauges is ERC20 { uint112 weight = weights[i]; weightsSum += weight; + require(isGauge(gauge), "ERC20Gauges: invalid gauge"); + _incrementGaugeWeight(msg.sender, gauge, weight, currentCycle); unchecked { ++i; diff --git a/test/unit/governance/ERC20Gauges.t.sol b/test/unit/governance/ERC20Gauges.t.sol index 4a7f570b..f48a2377 100644 --- a/test/unit/governance/ERC20Gauges.t.sol +++ b/test/unit/governance/ERC20Gauges.t.sol @@ -405,10 +405,22 @@ contract ERC20GaugesUnitTest is Test { token.setMaxGauges(2); token.addGauge(gauge1); token.removeGauge(gauge1); - vm.expectRevert("ERC20Gauges: deprecated gauge"); + vm.expectRevert("ERC20Gauges: invalid gauge"); + token.incrementGauge(gauge1, amount); + } + + function testIncrementOnUnlisted(uint112 amount) public { + token.setMaxGauges(1); + vm.expectRevert("ERC20Gauges: invalid gauge"); token.incrementGauge(gauge1, amount); } + function testIncrementsOnUnlisted(uint112 amount) public { + token.setMaxGauges(1); + vm.expectRevert("ERC20Gauges: invalid gauge"); + token.incrementGauges(new address[](1), new uint112[](1)); + } + function testIncrementOverWeight(uint112 amount) public { token.setMaxGauges(2); token.addGauge(gauge1); @@ -463,7 +475,7 @@ contract ERC20GaugesUnitTest is Test { gaugeList[1] = gauge1; weights[0] = 2e18; weights[1] = 4e18; - vm.expectRevert("ERC20Gauges: deprecated gauge"); + vm.expectRevert("ERC20Gauges: invalid gauge"); token.incrementGauges(gaugeList, weights); } From 0e94670675a13ccfde77af8d05c17038b1c2099a Mon Sep 17 00:00:00 2001 From: Erwan Beauvois Date: Thu, 23 Mar 2023 17:44:55 +0100 Subject: [PATCH 7/8] Fix _decrementWeightUntilFree issue M-07 The amount of freed weight was unnecessarily high if freeing weight from deprecated gauges Issue from Code4rena 2022-04 audit: [M-07] Incorrect accounting of free weight in _decrementWeightUntilFree --- src/governance/ERC20Gauges.sol | 2 +- test/unit/governance/ERC20Gauges.t.sol | 42 +++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/governance/ERC20Gauges.sol b/src/governance/ERC20Gauges.sol index 37f3cdf9..7555818d 100644 --- a/src/governance/ERC20Gauges.sol +++ b/src/governance/ERC20Gauges.sol @@ -614,7 +614,7 @@ abstract contract ERC20Gauges is ERC20 { uint256 size = gaugeList.length; for ( uint256 i = 0; - i < size && (userFreeWeight + totalFreed) < weight; + i < size && (userFreeWeight + userFreed) < weight; ) { address gauge = gaugeList[i]; diff --git a/test/unit/governance/ERC20Gauges.t.sol b/test/unit/governance/ERC20Gauges.t.sol index f48a2377..ca174654 100644 --- a/test/unit/governance/ERC20Gauges.t.sol +++ b/test/unit/governance/ERC20Gauges.t.sol @@ -8,6 +8,7 @@ contract ERC20GaugesUnitTest is Test { MockERC20Gauges token; address constant gauge1 = address(0xDEAD); address constant gauge2 = address(0xBEEF); + address constant gauge3 = address(0xF000); uint32 constant _CYCLE_LENGTH = 1 hours; uint32 constant _FREEZE_PERIOD = 10 minutes; @@ -415,7 +416,7 @@ contract ERC20GaugesUnitTest is Test { token.incrementGauge(gauge1, amount); } - function testIncrementsOnUnlisted(uint112 amount) public { + function testIncrementsOnUnlisted() public { token.setMaxGauges(1); vm.expectRevert("ERC20Gauges: invalid gauge"); token.incrementGauges(new address[](1), new uint112[](1)); @@ -776,4 +777,43 @@ contract ERC20GaugesUnitTest is Test { require(token.getGaugeWeight(gauge2) == 0); require(token.totalWeight() == 0); } + + // Validate use case described in 2022-04 C4 audit issue: + // [M-07] Incorrect accounting of free weight in _decrementWeightUntilFree + function testDecrementUntilFreeBugM07() public { + token.mint(address(this), 3e18); + + token.setMaxGauges(3); + token.addGauge(gauge1); + token.addGauge(gauge2); + token.addGauge(gauge3); + + require(token.incrementGauge(gauge1, 1e18) == 1e18); + require(token.incrementGauge(gauge2, 1e18) == 2e18); + require(token.incrementGauge(gauge3, 1e18) == 3e18); + + require(token.userUnusedWeight(address(this)) == 0); + require(token.totalWeight() == 3e18); + token.removeGauge(gauge1); + require(token.totalWeight() == 2e18); + + // deprecated gauge still counts, would need to decrement + require(token.userUnusedWeight(address(this)) == 0); + require(token.getUserGaugeWeight(address(this), gauge1) == 1e18); + require(token.getUserGaugeWeight(address(this), gauge2) == 1e18); + require(token.getUserGaugeWeight(address(this), gauge3) == 1e18); + require(token.getUserWeight(address(this)) == 3e18); + + token.mockBurn(address(this), 2e18); + + require(token.userUnusedWeight(address(this)) == 0); + require(token.getUserGaugeWeight(address(this), gauge1) == 0); + require(token.getUserGaugeWeight(address(this), gauge2) == 0); + require(token.getUserGaugeWeight(address(this), gauge3) == 1e18); + require(token.getUserWeight(address(this)) == 1e18); + require(token.getGaugeWeight(gauge1) == 0); + require(token.getGaugeWeight(gauge2) == 0); + require(token.getGaugeWeight(gauge3) == 1e18); + require(token.totalWeight() == 1e18); + } } From 99967ba97b999d0321f5905b28f5a733df1a1226 Mon Sep 17 00:00:00 2001 From: Erwan Beauvois Date: Fri, 24 Mar 2023 08:22:44 +0100 Subject: [PATCH 8/8] Update ERC20Gauges.sol comment --- src/governance/ERC20Gauges.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/governance/ERC20Gauges.sol b/src/governance/ERC20Gauges.sol index 7555818d..6e43f6d1 100644 --- a/src/governance/ERC20Gauges.sol +++ b/src/governance/ERC20Gauges.sol @@ -46,6 +46,8 @@ import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet - Consistency: make incrementGauges return a uint112 instead of uint256 - Import OpenZeppelin ERC20 & EnumerableSet instead of Solmate's - Update error management style (use require + messages instead of Solidity errors) + - Remove SafeCast (it was only used in one place, to convert block.timestamp) + - Implement C4 audit fixes for [M-03], [M-04], [M-07], [G-02], and [G-04]. */ abstract contract ERC20Gauges is ERC20 { using EnumerableSet for EnumerableSet.AddressSet;