Skip to content

Commit

Permalink
Switch to custom errors
Browse files Browse the repository at this point in the history
  • Loading branch information
davidlaprade committed Dec 5, 2024
1 parent 1a3e644 commit 0a7c80c
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 15 deletions.
20 changes: 13 additions & 7 deletions src/FlexVotingClient.sol
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ abstract contract FlexVotingClient {
/// given time.
Checkpoints.Trace208 internal totalBalanceCheckpoints;

error FlexVotingClient__NoVotingWeight();
error FlexVotingClient__AlreadyVoted();
error FlexVotingClient__InvalidSupportValue();
error FlexVotingClient__NoVotesExpressed();

/// @param _governor The address of the flex-voting-compatible governance contract.
constructor(address _governor) {
GOVERNOR = IFractionalGovernor(_governor);
Expand Down Expand Up @@ -116,9 +121,9 @@ abstract contract FlexVotingClient {
/// @param support The depositor's vote preferences in accordance with the `VoteType` enum.
function expressVote(uint256 proposalId, uint8 support) external {
uint256 weight = getPastRawBalance(msg.sender, GOVERNOR.proposalSnapshot(proposalId));
require(weight > 0, "no weight");
if (weight == 0) revert FlexVotingClient__NoVotingWeight();

require(!proposalVotersHasVoted[proposalId][msg.sender], "already voted");
if (proposalVotersHasVoted[proposalId][msg.sender]) revert FlexVotingClient__AlreadyVoted();
proposalVotersHasVoted[proposalId][msg.sender] = true;

if (support == uint8(VoteType.Against)) {
Expand All @@ -128,7 +133,8 @@ abstract contract FlexVotingClient {
} else if (support == uint8(VoteType.Abstain)) {
proposalVotes[proposalId].abstainVotes += SafeCast.toUint128(weight);
} else {
revert("invalid support value, must be included in VoteType enum");
// Support value must be included in VoteType enum.
revert FlexVotingClient__InvalidSupportValue();
}
}

Expand All @@ -139,10 +145,10 @@ abstract contract FlexVotingClient {
/// @param proposalId The ID of the proposal which the Pool will now vote on.
function castVote(uint256 proposalId) external {
ProposalVote storage _proposalVote = proposalVotes[proposalId];
require(
_proposalVote.forVotes + _proposalVote.againstVotes + _proposalVote.abstainVotes > 0,
"no votes expressed"
);
if (_proposalVote.forVotes + _proposalVote.againstVotes + _proposalVote.abstainVotes == 0) {
revert FlexVotingClient__NoVotesExpressed();
}

uint256 _proposalSnapshotBlockNumber = GOVERNOR.proposalSnapshot(proposalId);

// We use the snapshot of total raw balances to determine the weight with
Expand Down
17 changes: 9 additions & 8 deletions test/FlexVotingClient.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {IGovernor} from "@openzeppelin/contracts/governance/Governor.sol";
import {IVotingToken} from "src/interfaces/IVotingToken.sol";
import {IFractionalGovernor} from "src/interfaces/IFractionalGovernor.sol";
import {GovernorCountingFractional as GCF} from "src/GovernorCountingFractional.sol";
import {FlexVotingClient as FVC} from "src/FlexVotingClient.sol";
import {MockFlexVotingClient} from "test/MockFlexVotingClient.sol";
import {GovToken} from "test/GovToken.sol";
import {FractionalGovernor} from "test/FractionalGovernor.sol";
Expand Down Expand Up @@ -502,7 +503,7 @@ contract ExpressVote is FlexVotingClientTest {

// Now try to express a voting preference on the proposal.
assertEq(flexClient.deposits(_user), _voteWeight);
vm.expectRevert(bytes("no weight"));
vm.expectRevert(FVC.FlexVotingClient__NoVotingWeight.selector);
vm.prank(_user);
flexClient.expressVote(_proposalId, uint8(_voteType));
}
Expand All @@ -524,7 +525,7 @@ contract ExpressVote is FlexVotingClientTest {
uint256 _proposalId = _createAndSubmitProposal();

// _user should NOT be able to express his/her vote on the proposal.
vm.expectRevert(bytes("no weight"));
vm.expectRevert(FVC.FlexVotingClient__NoVotingWeight.selector);
vm.prank(_user);
flexClient.expressVote(_proposalId, uint8(_voteType));

Expand All @@ -536,7 +537,7 @@ contract ExpressVote is FlexVotingClientTest {
// _user should still NOT be able to express his/her vote on the proposal.
// Despite having a deposit balance, he/she didn't have a balance at the
// proposal snapshot.
vm.expectRevert(bytes("no weight"));
vm.expectRevert(FVC.FlexVotingClient__NoVotingWeight.selector);
vm.prank(_user);
flexClient.expressVote(_proposalId, uint8(_voteType));
}
Expand Down Expand Up @@ -567,7 +568,7 @@ contract ExpressVote is FlexVotingClientTest {
assertEq(_abstainVotesExpressedInit, _voteType == GCF.VoteType.Abstain ? _voteWeight : 0);

// Vote early and often!
vm.expectRevert(bytes("already voted"));
vm.expectRevert(FVC.FlexVotingClient__AlreadyVoted.selector);
vm.prank(_user);
flexClient.expressVote(_proposalId, uint8(_voteType));

Expand Down Expand Up @@ -595,7 +596,7 @@ contract ExpressVote is FlexVotingClientTest {
uint256 _proposalId = _createAndSubmitProposal();

// Now try to express a voting preference with a bogus support type.
vm.expectRevert(bytes("invalid support value, must be included in VoteType enum"));
vm.expectRevert(FVC.FlexVotingClient__InvalidSupportValue.selector);
vm.prank(_user);
flexClient.expressVote(_proposalId, _supportType);
}
Expand Down Expand Up @@ -626,7 +627,7 @@ contract ExpressVote is FlexVotingClientTest {
assert(_proposalId != _id);

// Now try to express a voting preference on the bogus proposal.
vm.expectRevert("no weight");
vm.expectRevert(FVC.FlexVotingClient__NoVotingWeight.selector);
vm.prank(_user);
flexClient.expressVote(_proposalId, _supportType);
}
Expand Down Expand Up @@ -1077,7 +1078,7 @@ contract CastVote is FlexVotingClientTest {
uint256 _proposalId = _createAndSubmitProposal();

// No one has expressed, there are no votes to cast.
vm.expectRevert(bytes("no votes expressed"));
vm.expectRevert(FVC.FlexVotingClient__NoVotesExpressed.selector);
flexClient.castVote(_proposalId);

// _user expresses his/her vote on the proposal.
Expand All @@ -1088,7 +1089,7 @@ contract CastVote is FlexVotingClientTest {
flexClient.castVote(_proposalId);

// All votes have been cast, there's nothing new to send to the governor.
vm.expectRevert(bytes("no votes expressed"));
vm.expectRevert(FVC.FlexVotingClient__NoVotesExpressed.selector);
flexClient.castVote(_proposalId);
}

Expand Down

0 comments on commit 0a7c80c

Please sign in to comment.