From 1df8113adf3a1538185ec281471d595d959fb7f7 Mon Sep 17 00:00:00 2001 From: Daniel Simon Date: Thu, 26 Dec 2024 12:37:10 +0700 Subject: [PATCH 1/2] refactor: remove stale and redundant check Fixes #118. --- src/Governance.sol | 11 ++++------- test/Governance.t.sol | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/Governance.sol b/src/Governance.sol index b32946f..e5bac30 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -594,8 +594,10 @@ contract Governance is MultiDelegateCall, UserProxyFactory, ReentrancyGuard, Own int256[] calldata _absoluteLQTYVotes, int256[] calldata _absoluteLQTYVetos ) external nonReentrant { - require(_initiatives.length == _absoluteLQTYVotes.length, "Length"); - require(_absoluteLQTYVetos.length == _absoluteLQTYVotes.length, "Length"); + require( + _initiatives.length == _absoluteLQTYVotes.length && _absoluteLQTYVotes.length == _absoluteLQTYVetos.length, + "Governance: array-length-mismatch" + ); // To ensure the change is safe, enforce uniqueness _requireNoDuplicates(_initiativesToReset); @@ -685,11 +687,6 @@ contract Governance is MultiDelegateCall, UserProxyFactory, ReentrancyGuard, Own int256[] memory _deltaOffsetVotes, int256[] memory _deltaOffsetVetos ) internal { - require( - _initiatives.length == _deltaLQTYVotes.length && _initiatives.length == _deltaLQTYVetos.length, - "Governance: array-length-mismatch" - ); - AllocateLQTYMemory memory vars; (vars.votesSnapshot_, vars.state) = _snapshotVotes(); vars.currentEpoch = epoch(); diff --git a/test/Governance.t.sol b/test/Governance.t.sol index 55749aa..d841fc1 100644 --- a/test/Governance.t.sol +++ b/test/Governance.t.sol @@ -903,6 +903,24 @@ abstract contract GovernanceTest is Test { /// Ensure that at the end you remove 100% function test_fuzz_canRemoveExtact() public {} + function test_allocateLQTY_revertsWhenInputArraysAreOfDifferentLengths() external { + address[] memory initiativesToReset = new address[](0); + address[][2] memory initiatives = [new address[](2), new address[](3)]; + int256[][2] memory votes = [new int256[](2), new int256[](3)]; + int256[][2] memory vetos = [new int256[](2), new int256[](3)]; + + for (uint256 i = 0; i < 2; ++i) { + for (uint256 j = 0; j < 2; ++j) { + for (uint256 k = 0; k < 2; ++k) { + if (i == j && j == k) continue; + + vm.expectRevert("Governance: array-length-mismatch"); + governance.allocateLQTY(initiativesToReset, initiatives[i], votes[j], vetos[k]); + } + } + } + } + function test_allocateLQTY_single() public { vm.startPrank(user); From 7cf3b1fca7dbf82a8036a769cc9adf4957745f17 Mon Sep 17 00:00:00 2001 From: Daniel Simon Date: Fri, 27 Dec 2024 17:37:23 +0700 Subject: [PATCH 2/2] docs: add @dev note about `_allocateLQTY()` --- src/Governance.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Governance.sol b/src/Governance.sol index e5bac30..4aa1713 100644 --- a/src/Governance.sol +++ b/src/Governance.sol @@ -678,8 +678,9 @@ contract Governance is MultiDelegateCall, UserProxyFactory, ReentrancyGuard, Own } /// @dev For each given initiative applies relative changes to the allocation - /// NOTE: Given the current usage the function either: Resets the value to 0, or sets the value to a new value - /// Review the flows as the function could be used in many ways, but it ends up being used in just those 2 ways + /// @dev Assumes that all the input arrays are of equal length + /// @dev NOTE: Given the current usage the function either: Resets the value to 0, or sets the value to a new value + /// Review the flows as the function could be used in many ways, but it ends up being used in just those 2 ways function _allocateLQTY( address[] memory _initiatives, int256[] memory _deltaLQTYVotes,