Convert GovernorCountingFractional to use error types

ScopeLift · Apr 27, 2024 · 8f0185f · 8f0185f
1 parent c9aecc4
commit 8f0185f
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 19 deletions.
diff --git a/src/GovernorCountingFractional.sol b/src/GovernorCountingFractional.sol
@@ -7,6 +7,7 @@ import {Governor} from "@openzeppelin/contracts/governance/Governor.sol";
 import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
 
 /**
+ * @author [ScopeLift](https://scopelift.co)
  * @notice Extension of {Governor} for 3 option fractional vote counting. When
  * voting, a delegate may split their vote weight between Against/For/Abstain.
  * This is most useful when the delegate is itself a contract, implementing its
@@ -19,6 +20,10 @@ import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
  */
 abstract contract GovernorCountingFractional is Governor {
 
+    error GovernorCountingFractional__VoteWeightExceeded();
+    error GovernorCountingFractional__InvalidVoteData();
+    error GovernorCountingFractional_NoVoteWeight();
+
     /**
      * @dev Supported vote types. Matches Governor Bravo ordering.
      */
@@ -132,9 +137,12 @@ abstract contract GovernorCountingFractional is Governor {
         uint256 totalWeight,
         bytes memory voteData
     ) internal virtual override {
-        require(totalWeight > 0, "GovernorCountingFractional: no weight");
+        if (totalWeight == 0) {
+            revert GovernorCountingFractional_NoVoteWeight();
+        }
+
         if (_proposalVotersWeightCast[proposalId][account] >= totalWeight) {
-          revert("GovernorCountingFractional: all weight cast");
+          revert GovernorCountingFractional__VoteWeightExceeded();
         }
 
         uint128 safeTotalWeight = SafeCast.toUint128(totalWeight);
@@ -159,10 +167,9 @@ abstract contract GovernorCountingFractional is Governor {
         uint128 totalWeight,
         uint8 support
     ) internal {
-        require(
-            _proposalVotersWeightCast[proposalId][account] == 0,
-            "GovernorCountingFractional: vote would exceed weight"
-        );
+        if (_proposalVotersWeightCast[proposalId][account] != 0) {
+            revert GovernorCountingFractional__VoteWeightExceeded();
+        }
 
         _proposalVotersWeightCast[proposalId][account] = totalWeight;
 
@@ -173,7 +180,7 @@ abstract contract GovernorCountingFractional is Governor {
         } else if (support == uint8(VoteType.Abstain)) {
             _proposalVotes[proposalId].abstainVotes += totalWeight;
         } else {
-            revert("GovernorCountingFractional: invalid support value, must be included in VoteType enum");
+            revert GovernorInvalidVoteType();
         }
     }
 
@@ -204,14 +211,18 @@ abstract contract GovernorCountingFractional is Governor {
         uint128 totalWeight,
         bytes memory voteData
     ) internal {
-        require(voteData.length == 48, "GovernorCountingFractional: invalid voteData");
+        if (voteData.length != 48) {
+            revert GovernorCountingFractional__InvalidVoteData();
+        }
 
         (uint128 _againstVotes, uint128 _forVotes, uint128 _abstainVotes) = _decodePackedVotes(voteData);
 
         uint128 _existingWeight = _proposalVotersWeightCast[proposalId][account];
         uint256 _newWeight = uint256(_againstVotes) + _forVotes + _abstainVotes + _existingWeight;
 
-        require(_newWeight <= totalWeight, "GovernorCountingFractional: vote would exceed weight");
+        if (_newWeight > totalWeight) {
+            revert GovernorCountingFractional__VoteWeightExceeded();
+        }
 
         // It's safe to downcast here because we've just confirmed that
         // _newWeight <= totalWeight, and totalWeight is a uint128.

diff --git a/test/FractionalPool.t.sol b/test/FractionalPool.t.sol
@@ -455,7 +455,11 @@ contract Vote is FractionalPoolTest {
     pool.castVote(_proposalId);
 
     // Try to submit them again.
-    vm.expectRevert(bytes("GovernorCountingFractional: all weight cast"));
+    vm.expectRevert(
+      abi.encodeWithSelector(
+        GovernorCountingFractional.GovernorCountingFractional__VoteWeightExceeded.selector
+      )
+    );
     pool.castVote(_proposalId);
   }
 

diff --git a/test/GovernorCountingFractional.t.sol b/test/GovernorCountingFractional.t.sol
@@ -7,7 +7,7 @@ import {FractionalPool, IVotingToken, IFractionalGovernor} from "../src/Fraction
 import {FixedPointMathLib} from "solmate/utils/FixedPointMathLib.sol";
 
 import {GovToken} from "./GovToken.sol";
-import {FractionalGovernor} from "./FractionalGovernor.sol";
+import {FractionalGovernor, GovernorCountingFractional} from "./FractionalGovernor.sol";
 import {ProposalReceiverMock} from "./ProposalReceiverMock.sol";
 import {IVotes} from "@openzeppelin/contracts/governance/extensions/GovernorVotes.sol";
 import {IGovernor} from "@openzeppelin/contracts/governance/Governor.sol";
@@ -410,7 +410,11 @@ contract GovernorCountingFractionalTest is Test {
 
     // Attempt to cast nominal votes.
     vm.prank(_voterAddr);
-    vm.expectRevert("GovernorCountingFractional: no weight");
+    vm.expectRevert(
+      abi.encodeWithSelector(
+        GovernorCountingFractional.GovernorCountingFractional_NoVoteWeight.selector
+      )
+    );
     governor.castVoteWithReasonAndParams(
       _proposalId,
       uint8(VoteType.For),
@@ -420,7 +424,11 @@ contract GovernorCountingFractionalTest is Test {
 
     // Attempt to cast fractional votes.
     vm.prank(_voterAddr);
-    vm.expectRevert("GovernorCountingFractional: no weight");
+    vm.expectRevert(
+      abi.encodeWithSelector(
+        GovernorCountingFractional.GovernorCountingFractional_NoVoteWeight.selector
+      )
+    );
     governor.castVoteWithReasonAndParams(
       _proposalId,
       uint8(VoteType.For),
@@ -484,7 +492,11 @@ contract GovernorCountingFractionalTest is Test {
     );
 
     vm.prank(voter.addr);
-    vm.expectRevert("GovernorCountingFractional: vote would exceed weight");
+    vm.expectRevert(
+      abi.encodeWithSelector(
+        GovernorCountingFractional.GovernorCountingFractional__VoteWeightExceeded.selector
+      )
+    );
     governor.castVoteWithReasonAndParams(_proposalId, voter.support, "Yay", fractionalizedVotes);
   }
 
@@ -557,7 +569,11 @@ contract GovernorCountingFractionalTest is Test {
     uint256 _proposalId = _createAndSubmitProposal();
 
     vm.prank(voter.addr);
-    vm.expectRevert("GovernorCountingFractional: invalid voteData");
+    vm.expectRevert(
+      abi.encodeWithSelector(
+        GovernorCountingFractional.GovernorCountingFractional__InvalidVoteData.selector
+      )
+    );
     governor.castVoteWithReasonAndParams(_proposalId, voter.support, "Weeee", _invalidVoteData);
   }
 
@@ -974,7 +990,11 @@ contract GovernorCountingFractionalTest is Test {
 
     // Attempt to cast votes again. This should revert.
     vm.prank(_voter.addr);
-    vm.expectRevert("GovernorCountingFractional: vote would exceed weight");
+    vm.expectRevert(
+      abi.encodeWithSelector(
+        GovernorCountingFractional.GovernorCountingFractional__VoteWeightExceeded.selector
+      )
+    );
     governor.castVoteWithReasonAndParams(
       _proposalId,
       _voter.support,
@@ -1001,7 +1021,11 @@ contract GovernorCountingFractionalTest is Test {
 
     // It should not be possible to vote again.
     vm.prank(_voter.addr);
-    vm.expectRevert("GovernorCountingFractional: all weight cast");
+    vm.expectRevert(
+      abi.encodeWithSelector(
+        GovernorCountingFractional.GovernorCountingFractional__VoteWeightExceeded.selector
+      )
+    );
     governor.castVoteWithReasonAndParams(
       _proposalId, _voter.support, "Yay", _emptyDataBecauseWereVotingNominally
     );
@@ -1048,7 +1072,11 @@ contract GovernorCountingFractionalTest is Test {
       );
 
       // Now attempt to vote fractionally. It should fail.
-      vm.expectRevert("GovernorCountingFractional: all weight cast");
+      vm.expectRevert(
+        abi.encodeWithSelector(
+          GovernorCountingFractional.GovernorCountingFractional__VoteWeightExceeded.selector
+        )
+      );
       vm.prank(_voter.addr);
       governor.castVoteWithReasonAndParams(
         _proposalId, _voter.support, "Fractional vote", _fractionalizedVoteData
@@ -1069,7 +1097,11 @@ contract GovernorCountingFractionalTest is Test {
       );
 
       vm.prank(_voter.addr);
-      vm.expectRevert("GovernorCountingFractional: vote would exceed weight");
+      vm.expectRevert(
+        abi.encodeWithSelector(
+          GovernorCountingFractional.GovernorCountingFractional__VoteWeightExceeded.selector
+        )
+      );
       governor.castVoteWithReasonAndParams(
         _proposalId, _voter.support, "Nominal vote", _nominalVoteData
       );