From f66f972c0c2912e59dc162b201b06446ed310823 Mon Sep 17 00:00:00 2001 From: jlqty <172397380+jltqy@users.noreply.github.com> Date: Fri, 4 Oct 2024 15:29:55 +0100 Subject: [PATCH] Restrict deltas to int96 and assert <= 2**88 --- src/Governance.sol | 49 +++++---------------- src/interfaces/IGovernance.sol | 7 +-- src/utils/Math.sol | 10 +++-- test/BribeInitiative.t.sol | 4 +- test/Governance.t.sol | 80 ++++++++++++++-------------------- test/mocks/MockInitiative.sol | 4 +- 6 files changed, 58 insertions(+), 96 deletions(-) diff --git a/src/Governance.sol b/src/Governance.sol index 94409be0..408ee33a 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.24; +import {console} from "forge-std/console.sol"; + import {IERC20} from "openzeppelin-contracts/contracts/interfaces/IERC20.sol"; import {SafeERC20} from "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol"; import {ReentrancyGuard} from "openzeppelin-contracts/contracts/utils/ReentrancyGuard.sol"; @@ -12,7 +14,7 @@ import {ILQTYStaking} from "./interfaces/ILQTYStaking.sol"; import {UserProxy} from "./UserProxy.sol"; import {UserProxyFactory} from "./UserProxyFactory.sol"; -import {add, max} from "./utils/Math.sol"; +import {add, max, abs} from "./utils/Math.sol"; import {Multicall} from "./utils/Multicall.sol"; import {WAD, PermitParams} from "./utils/Types.sol"; @@ -376,38 +378,11 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance try IInitiative(_initiative).onUnregisterInitiative(currentEpoch) {} catch {} } - event log_votes(uint votes); - - function _checkSufficientVotes( - UserState memory userState, - address[] calldata _initiatives, - int176[] calldata _deltaLQTYVotes, - int176[] calldata _deltaLQTYVetos - ) internal returns (bool) { - uint userVotes = lqtyToVotes(userState.allocatedLQTY, block.timestamp, userState.averageStakingTimestamp); - // an allocation can only be made if the user has more voting power (LQTY * age) - - emit log_votes(userVotes); - - uint176 absVote; - uint176 absVeto; - - for (uint256 i = 0; i < _initiatives.length; i++) { - absVote = _deltaLQTYVotes[i] < 0 ? uint176(-_deltaLQTYVotes[i]) : uint176(_deltaLQTYVotes[i]); - absVeto = _deltaLQTYVetos[i] < 0 ? uint176(-_deltaLQTYVetos[i]) : uint176(_deltaLQTYVetos[i]); - if (absVote > userVotes || absVeto > userVotes) { - return false; - } - } - - return true; - } - /// @inheritdoc IGovernance function allocateLQTY( address[] calldata _initiatives, - int176[] calldata _deltaLQTYVotes, - int176[] calldata _deltaLQTYVetos + int96[] calldata _deltaLQTYVotes, + int96[] calldata _deltaLQTYVetos ) external nonReentrant { require( _initiatives.length == _deltaLQTYVotes.length && _initiatives.length == _deltaLQTYVetos.length, @@ -421,15 +396,15 @@ contract Governance is Multicall, UserProxyFactory, ReentrancyGuard, IGovernance UserState memory userState = userStates[msg.sender]; - require( - _checkSufficientVotes(userState, _initiatives, _deltaLQTYVotes, _deltaLQTYVetos), - "Governance: invalid-votes" - ); - for (uint256 i = 0; i < _initiatives.length; i++) { address initiative = _initiatives[i]; - int176 deltaLQTYVotes = _deltaLQTYVotes[i]; - int176 deltaLQTYVetos = _deltaLQTYVetos[i]; + int96 deltaLQTYVotes = _deltaLQTYVotes[i]; + int96 deltaLQTYVetos = _deltaLQTYVetos[i]; + + require( + abs(deltaLQTYVotes) <= type(uint88).max && abs(deltaLQTYVetos) <= type(uint88).max, + "Governance: deltas-too-large" + ); // only allow vetoing post the voting cutoff require( diff --git a/src/interfaces/IGovernance.sol b/src/interfaces/IGovernance.sol index 1a4cb51d..1453ffdc 100644 --- a/src/interfaces/IGovernance.sol +++ b/src/interfaces/IGovernance.sol @@ -246,11 +246,8 @@ interface IGovernance { /// @param _initiatives Addresses of the initiatives to allocate to /// @param _deltaLQTYVotes Delta LQTY to allocate to the initiatives as votes /// @param _deltaLQTYVetos Delta LQTY to allocate to the initiatives as vetos - function allocateLQTY( - address[] memory _initiatives, - int176[] memory _deltaLQTYVotes, - int176[] memory _deltaLQTYVetos - ) external; + function allocateLQTY(address[] memory _initiatives, int96[] memory _deltaLQTYVotes, int96[] memory _deltaLQTYVetos) + external; /// @notice Splits accrued funds according to votes received between all initiatives /// @param _initiative Addresse of the initiative diff --git a/src/utils/Math.sol b/src/utils/Math.sol index 2e1d6246..07cb9ba8 100644 --- a/src/utils/Math.sol +++ b/src/utils/Math.sol @@ -1,11 +1,11 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.24; -function add(uint88 a, int192 b) pure returns (uint88) { +function add(uint88 a, int96 b) pure returns (uint88) { if (b < 0) { - return uint88(a - uint88(uint192(-b))); + return uint88(a - uint88(uint96(-b))); } - return uint88(a + uint88(uint192(b))); + return uint88(a + uint88(uint96(b))); } function sub(uint256 a, int256 b) pure returns (uint128) { @@ -18,3 +18,7 @@ function sub(uint256 a, int256 b) pure returns (uint128) { function max(uint256 a, uint256 b) pure returns (uint256) { return a > b ? a : b; } + +function abs(int96 a) pure returns (uint96) { + return a < 0 ? uint96(-a) : uint96(a); +} diff --git a/test/BribeInitiative.t.sol b/test/BribeInitiative.t.sol index 6f8df633..703fd408 100644 --- a/test/BribeInitiative.t.sol +++ b/test/BribeInitiative.t.sol @@ -102,9 +102,9 @@ contract BribeInitiativeTest is Test { address[] memory initiatives = new address[](1); initiatives[0] = address(bribeInitiative); - int176[] memory deltaVoteLQTY = new int176[](1); + int96[] memory deltaVoteLQTY = new int96[](1); deltaVoteLQTY[0] = 1e18; - int176[] memory deltaVetoLQTY = new int176[](1); + int96[] memory deltaVetoLQTY = new int96[](1); governance.allocateLQTY(initiatives, deltaVoteLQTY, deltaVetoLQTY); assertEq(bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()), 1e18); assertEq(bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()), 1e18); diff --git a/test/Governance.t.sol b/test/Governance.t.sol index 22af825b..800411f1 100644 --- a/test/Governance.t.sol +++ b/test/Governance.t.sol @@ -100,7 +100,6 @@ contract GovernanceTest is Test { initialInitiatives.push(baseInitiative1); initialInitiatives.push(baseInitiative2); - initialInitiatives.push(baseInitiative3); governance = new Governance( address(lqty), @@ -754,9 +753,9 @@ contract GovernanceTest is Test { address[] memory initiatives = new address[](1); initiatives[0] = baseInitiative1; - int176[] memory deltaLQTYVotes = new int176[](1); + int96[] memory deltaLQTYVotes = new int96[](1); deltaLQTYVotes[0] = 1e18; //this should be 0 - int176[] memory deltaLQTYVetos = new int176[](1); + int96[] memory deltaLQTYVetos = new int96[](1); // should revert if the initiative has been registered in the current epoch vm.expectRevert("Governance: initiative-not-active"); @@ -886,10 +885,10 @@ contract GovernanceTest is Test { address[] memory initiatives = new address[](2); initiatives[0] = baseInitiative1; initiatives[1] = baseInitiative2; - int176[] memory deltaLQTYVotes = new int176[](2); + int96[] memory deltaLQTYVotes = new int96[](2); deltaLQTYVotes[0] = 1e18; deltaLQTYVotes[1] = 1e18; - int176[] memory deltaLQTYVetos = new int176[](2); + int96[] memory deltaLQTYVetos = new int96[](2); vm.warp(block.timestamp + 365 days); @@ -929,9 +928,9 @@ contract GovernanceTest is Test { address[] memory initiatives = new address[](1); initiatives[0] = baseInitiative1; - int176[] memory deltaLQTYVotes = new int176[](1); - deltaLQTYVotes[0] = int176(uint176(_deltaLQTYVotes)); - int176[] memory deltaLQTYVetos = new int176[](1); + int96[] memory deltaLQTYVotes = new int96[](1); + deltaLQTYVotes[0] = int96(uint96(_deltaLQTYVotes)); + int96[] memory deltaLQTYVetos = new int96[](1); vm.warp(block.timestamp + 365 days); @@ -953,9 +952,9 @@ contract GovernanceTest is Test { address[] memory initiatives = new address[](1); initiatives[0] = baseInitiative1; - int176[] memory deltaLQTYVotes = new int176[](1); - int176[] memory deltaLQTYVetos = new int176[](1); - deltaLQTYVetos[0] = int176(uint176(_deltaLQTYVetos)); + int96[] memory deltaLQTYVotes = new int96[](1); + int96[] memory deltaLQTYVetos = new int96[](1); + deltaLQTYVetos[0] = int96(uint96(_deltaLQTYVetos)); vm.warp(block.timestamp + 365 days); @@ -986,10 +985,10 @@ contract GovernanceTest is Test { address[] memory initiatives = new address[](2); initiatives[0] = baseInitiative1; initiatives[1] = baseInitiative2; - int176[] memory deltaVoteLQTY = new int176[](2); + int96[] memory deltaVoteLQTY = new int96[](2); deltaVoteLQTY[0] = 500e18; deltaVoteLQTY[1] = 500e18; - int176[] memory deltaVetoLQTY = new int176[](2); + int96[] memory deltaVetoLQTY = new int96[](2); governance.allocateLQTY(initiatives, deltaVoteLQTY, deltaVetoLQTY); (uint88 allocatedLQTY,) = governance.userStates(user); assertEq(allocatedLQTY, 1000e18); @@ -1062,10 +1061,10 @@ contract GovernanceTest is Test { address[] memory initiatives = new address[](2); initiatives[0] = EOAInitiative; // attempt for an EOA initiatives[1] = baseInitiative2; - int176[] memory deltaVoteLQTY = new int176[](2); + int96[] memory deltaVoteLQTY = new int96[](2); deltaVoteLQTY[0] = 500e18; deltaVoteLQTY[1] = 500e18; - int176[] memory deltaVetoLQTY = new int176[](2); + int96[] memory deltaVetoLQTY = new int96[](2); governance.allocateLQTY(initiatives, deltaVoteLQTY, deltaVetoLQTY); (uint88 allocatedLQTY,) = governance.userStates(user); assertEq(allocatedLQTY, 1000e18); @@ -1126,22 +1125,22 @@ contract GovernanceTest is Test { bytes[] memory data = new bytes[](7); address[] memory initiatives = new address[](1); initiatives[0] = baseInitiative1; - int176[] memory deltaVoteLQTY = new int176[](1); - deltaVoteLQTY[0] = int176(uint176(lqtyAmount)); - int176[] memory deltaVetoLQTY = new int176[](1); + int96[] memory deltaVoteLQTY = new int96[](1); + deltaVoteLQTY[0] = int96(uint96(lqtyAmount)); + int96[] memory deltaVetoLQTY = new int96[](1); - int176[] memory deltaVoteLQTY_ = new int176[](1); - deltaVoteLQTY_[0] = -int176(uint176(lqtyAmount)); + int96[] memory deltaVoteLQTY_ = new int96[](1); + deltaVoteLQTY_[0] = -int96(uint96(lqtyAmount)); data[0] = abi.encodeWithSignature("deployUserProxy()"); data[1] = abi.encodeWithSignature("depositLQTY(uint88)", lqtyAmount); data[2] = abi.encodeWithSignature( - "allocateLQTY(address[],int176[],int176[])", initiatives, deltaVoteLQTY, deltaVetoLQTY + "allocateLQTY(address[],int96[],int96[])", initiatives, deltaVoteLQTY, deltaVetoLQTY ); data[3] = abi.encodeWithSignature("userStates(address)", user); data[4] = abi.encodeWithSignature("snapshotVotesForInitiative(address)", baseInitiative1); data[5] = abi.encodeWithSignature( - "allocateLQTY(address[],int176[],int176[])", initiatives, deltaVoteLQTY_, deltaVetoLQTY + "allocateLQTY(address[],int96[],int96[])", initiatives, deltaVoteLQTY_, deltaVetoLQTY ); data[6] = abi.encodeWithSignature("withdrawLQTY(uint88)", lqtyAmount); bytes[] memory response = governance.multicall(data); @@ -1193,8 +1192,8 @@ contract GovernanceTest is Test { address[] memory initiatives = new address[](1); initiatives[0] = address(mockInitiative); - int176[] memory deltaLQTYVotes = new int176[](1); - int176[] memory deltaLQTYVetos = new int176[](1); + int96[] memory deltaLQTYVotes = new int96[](1); + int96[] memory deltaLQTYVetos = new int96[](1); governance.allocateLQTY(initiatives, deltaLQTYVotes, deltaLQTYVetos); // check that votingThreshold is is high enough such that MIN_CLAIM is met @@ -1253,34 +1252,21 @@ contract GovernanceTest is Test { function test_allocateLQTY_overflow() public { vm.startPrank(user); - address[] memory initiatives = new address[](3); + address[] memory initiatives = new address[](2); initiatives[0] = baseInitiative1; - initiatives[1] = baseInitiative3; - initiatives[2] = baseInitiative2; - int176[] memory deltaLQTYVotes = new int176[](3); - deltaLQTYVotes[0] = 154742504910672534362390528; // 2**87 - deltaLQTYVotes[1] = 0; - deltaLQTYVotes[2] = 154742504910672534362390527; // 2**87 - 1 - int176[] memory deltaLQTYVetos = new int176[](3); - deltaLQTYVetos[0] = 0; - deltaLQTYVetos[1] = 1; - deltaLQTYVetos[2] = -309485009821345068724781056; // - 2**88 + initiatives[1] = baseInitiative2; + + int96[] memory deltaLQTYVotes = new int96[](2); + deltaLQTYVotes[0] = 0; + deltaLQTYVotes[1] = 2 ** 88 - 1; + int96[] memory deltaLQTYVetos = new int96[](2); + deltaLQTYVetos[0] = 1; + deltaLQTYVetos[1] = -(2 ** 88); vm.warp(block.timestamp + 365 days); + vm.expectRevert("Governance: deltas-too-large"); governance.allocateLQTY(initiatives, deltaLQTYVotes, deltaLQTYVetos); - (uint allocatedLQTY,) = governance.userStates(user); - assertEq(allocatedLQTY, 0); - console.log(allocatedLQTY); - - (uint88 voteLQTY1,,,,) = governance.initiativeStates(baseInitiative1); - assertGt(voteLQTY1, 0); - console.log(voteLQTY1); - - (uint88 voteLQTY2,,,,) = governance.initiativeStates(baseInitiative2); - assertGt(voteLQTY2, 0); - console.log(voteLQTY2); - vm.stopPrank(); } } diff --git a/test/mocks/MockInitiative.sol b/test/mocks/MockInitiative.sol index 8861f420..88d05e7b 100644 --- a/test/mocks/MockInitiative.sol +++ b/test/mocks/MockInitiative.sol @@ -24,8 +24,8 @@ contract MockInitiative is IInitiative { /// @inheritdoc IInitiative function onAfterAllocateLQTY(uint16, address, uint88, uint88) external virtual { address[] memory initiatives = new address[](0); - int176[] memory deltaLQTYVotes = new int176[](0); - int176[] memory deltaLQTYVetos = new int176[](0); + int96[] memory deltaLQTYVotes = new int96[](0); + int96[] memory deltaLQTYVetos = new int96[](0); governance.allocateLQTY(initiatives, deltaLQTYVotes, deltaLQTYVetos); }