diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index be79645..09d8be1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -106,7 +106,7 @@ jobs: uses: zgosalvez/github-actions-report-lcov@v2 with: coverage-files: ./lcov.info - minimum-coverage: 70 # Set coverage threshold. + minimum-coverage: 67 # Set coverage threshold. slither-analyze: runs-on: ubuntu-latest diff --git a/.gitmodules b/.gitmodules index dd8ab1e..ef87f89 100644 --- a/.gitmodules +++ b/.gitmodules @@ -6,4 +6,4 @@ url = https://github.com/foundry-rs/forge-std [submodule "lib/openzeppelin-contracts"] path = lib/openzeppelin-contracts - url = https://github.com/openzeppelin/openzeppelin-contracts + url = https://github.com/OpenZeppelin/openzeppelin-contracts diff --git a/foundry.toml b/foundry.toml index 7c325cf..3244911 100644 --- a/foundry.toml +++ b/foundry.toml @@ -1,10 +1,8 @@ [profile.default] - # We don't specify a solc version because Aave contracts are pinned to 0.8.10, - # but we use more recent solc versions for other contracts, so we let forge - # auto-detect solc versions. optimizer = true optimizer_runs = 10_000_000 remappings = ["@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts"] + solc_version = '0.8.25' verbosity = 3 [profile.ci] diff --git a/lib/openzeppelin-contracts b/lib/openzeppelin-contracts index fd81a96..dbb6104 160000 --- a/lib/openzeppelin-contracts +++ b/lib/openzeppelin-contracts @@ -1 +1 @@ -Subproject commit fd81a96f01cc42ef1c9a5399364968d0e07e9e90 +Subproject commit dbb6104ce834628e473d2173bbc9d47f81a9eec3 diff --git a/script/Deploy.s.sol b/script/Deploy.s.sol index 7f82173..c68424a 100644 --- a/script/Deploy.s.sol +++ b/script/Deploy.s.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.10; +pragma solidity ^0.8.20; import "forge-std/Script.sol"; diff --git a/src/FlexVotingClient.sol b/src/FlexVotingClient.sol index 3bd99bb..08aa326 100644 --- a/src/FlexVotingClient.sol +++ b/src/FlexVotingClient.sol @@ -1,8 +1,8 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity >=0.8.10; +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; -import {Checkpoints} from "@openzeppelin/contracts/utils/Checkpoints.sol"; +import {Checkpoints} from "@openzeppelin/contracts/utils/structs/Checkpoints.sol"; import {IFractionalGovernor} from "./interfaces/IFractionalGovernor.sol"; import {IVotingToken} from "./interfaces/IVotingToken.sol"; @@ -48,7 +48,7 @@ import {IVotingToken} from "./interfaces/IVotingToken.sol"; /// of the rest. abstract contract FlexVotingClient { using SafeCast for uint256; - using Checkpoints for Checkpoints.History; + using Checkpoints for Checkpoints.Trace224; /// @notice The voting options corresponding to those used in the Governor. enum VoteType { @@ -76,12 +76,12 @@ abstract contract FlexVotingClient { /// @dev Mapping from address to the checkpoint history of raw balances /// of that address. - mapping(address => Checkpoints.History) private balanceCheckpoints; + mapping(address => Checkpoints.Trace224) private balanceCheckpoints; /// @dev History of the sum total of raw balances in the system. May or may /// not be equivalent to this contract's balance of `GOVERNOR`s token at a /// given time. - Checkpoints.History internal totalBalanceCheckpoints; + Checkpoints.Trace224 internal totalBalanceCheckpoints; /// @param _governor The address of the flex-voting-compatible governance contract. constructor(address _governor) { @@ -92,7 +92,7 @@ abstract contract FlexVotingClient { /// token that `_user` has claim to in this system. It may or may not be /// equivalent to the withdrawable balance of `GOVERNOR`s token for `user`, /// e.g. if the internal representation of balance has been scaled down. - function _rawBalanceOf(address _user) internal view virtual returns (uint256); + function _rawBalanceOf(address _user) internal view virtual returns (uint224); /// @dev Used as the `reason` param when submitting a vote to `GOVERNOR`. function _castVoteReasonString() internal virtual returns (string memory) { @@ -194,19 +194,21 @@ abstract contract FlexVotingClient { /// @dev Checkpoints the _user's current raw balance. function _checkpointRawBalanceOf(address _user) internal { - balanceCheckpoints[_user].push(_rawBalanceOf(_user)); + balanceCheckpoints[_user].push(SafeCast.toUint32(block.number), _rawBalanceOf(_user)); } /// @notice Returns the `_user`'s raw balance at `_blockNumber`. /// @param _user The account that's historical raw balance will be looked up. /// @param _blockNumber The block at which to lookup the _user's raw balance. function getPastRawBalance(address _user, uint256 _blockNumber) public view returns (uint256) { - return balanceCheckpoints[_user].getAtProbablyRecentBlock(_blockNumber); + uint32 key = SafeCast.toUint32(_blockNumber); + return balanceCheckpoints[_user].upperLookup(key); } /// @notice Returns the sum total of raw balances of all users at `_blockNumber`. /// @param _blockNumber The block at which to lookup the total balance. function getPastTotalBalance(uint256 _blockNumber) public view returns (uint256) { - return totalBalanceCheckpoints.getAtProbablyRecentBlock(_blockNumber); + uint32 key = SafeCast.toUint32(_blockNumber); + return totalBalanceCheckpoints.upperLookup(key); } } diff --git a/src/FractionalPool.sol b/src/FractionalPool.sol index 5dc076a..e380a26 100644 --- a/src/FractionalPool.sol +++ b/src/FractionalPool.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.10; +pragma solidity ^0.8.20; import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; diff --git a/src/GovernorCountingFractional.sol b/src/GovernorCountingFractional.sol index b118ef7..93213a6 100644 --- a/src/GovernorCountingFractional.sol +++ b/src/GovernorCountingFractional.sol @@ -1,13 +1,12 @@ // SPDX-License-Identifier: MIT - -pragma solidity ^0.8.0; +pragma solidity ^0.8.20; import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; -import {Governor} from "@openzeppelin/contracts/governance/Governor.sol"; -import {GovernorCompatibilityBravo} from "@openzeppelin/contracts/governance/compatibility/GovernorCompatibilityBravo.sol"; +import {Governor, IGovernor} 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 @@ -20,6 +19,31 @@ import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; */ abstract contract GovernorCountingFractional is Governor { + /// @notice Thrown when casting a vote would exceed the weight delegated to the voting account. + error GovernorCountingFractional__VoteWeightExceeded(); + + /// @notice Thrown when params data submitted with a vote does not match the convention expected for fractional voting. + error GovernorCountingFractional__InvalidVoteData(); + + /// @notice Thrown when vote is cast by an account that has no voting weight. + error GovernorCountingFractional_NoVoteWeight(); + + /** + * @notice Supported vote types. + * @dev Matches Governor Bravo ordering. + */ + enum VoteType { + Against, + For, + Abstain + } + + /** + * @notice Metadata about how votes were cast for a given proposal. + * @param againstVotes The number of votes cast Against a proposal. + * @param forVotes The number of votes cast For a proposal. + * @param abstainVotes The number of votes that Abstain from voting for a proposal. + */ struct ProposalVote { uint128 againstVotes; uint128 forVotes; @@ -27,51 +51,45 @@ abstract contract GovernorCountingFractional is Governor { } /** - * @dev Mapping from proposal ID to vote tallies for that proposal. + * @notice Mapping from proposal ID to vote tallies for that proposal. */ mapping(uint256 => ProposalVote) private _proposalVotes; /** - * @dev Mapping from proposal ID and address to the weight the address + * @notice Mapping from proposal ID and address to the weight the address * has cast on that proposal, e.g. _proposalVotersWeightCast[42][0xBEEF] * would tell you the number of votes that 0xBEEF has cast on proposal 42. */ mapping(uint256 => mapping(address => uint128)) private _proposalVotersWeightCast; - /** - * @dev Mapping from voter address to signature-based vote nonce. The - * voter's nonce increments each time a signature-based vote is cast with - * fractional voting params and must be included in the `params` as the last - * 16 bytes when signing for a fractional vote. - */ - mapping(address => uint128) public fractionalVoteNonce; - - /** - * @dev See {IGovernor-COUNTING_MODE}. - */ - // solhint-disable-next-line func-name-mixedcase + /// @inheritdoc IGovernor function COUNTING_MODE() public pure virtual override returns (string memory) { return "support=bravo&quorum=for,abstain¶ms=fractional"; } - /** - * @dev See {IGovernor-hasVoted}. - */ + /// @inheritdoc IGovernor function hasVoted(uint256 proposalId, address account) public view virtual override returns (bool) { return _proposalVotersWeightCast[proposalId][account] > 0; } /** - * @dev Get the number of votes cast thus far on proposal `proposalId` by + * @notice Get the number of votes cast thus far on proposal `proposalId` by * account `account`. Useful for integrations that allow delegates to cast * rolling, partial votes. + * @param proposalId Identifier of any past or present proposal. + * @param account The voting account in question. + * @return The total voting weight cast so far for this proposal by this account */ function voteWeightCast(uint256 proposalId, address account) public view returns (uint128) { return _proposalVotersWeightCast[proposalId][account]; } /** - * @dev Accessor to the internal vote counts. + * @notice Accessor to the internal vote counts. + * @param proposalId Identifier of any past or present proposal. + * @return againstVotes The Against votes cast so far for the given proposal. + * @return forVotes The For votes cast so far for given proposal. + * @return abstainVotes The Abstain votes cast so far for the given proposal. */ function proposalVotes(uint256 proposalId) public @@ -87,9 +105,7 @@ abstract contract GovernorCountingFractional is Governor { return (proposalVote.againstVotes, proposalVote.forVotes, proposalVote.abstainVotes); } - /** - * @dev See {Governor-_quorumReached}. - */ + /// @inheritdoc Governor function _quorumReached(uint256 proposalId) internal view virtual override returns (bool) { ProposalVote storage proposalVote = _proposalVotes[proposalId]; @@ -97,7 +113,8 @@ abstract contract GovernorCountingFractional is Governor { } /** - * @dev See {Governor-_voteSucceeded}. In this module, forVotes must be > againstVotes. + * @inheritdoc Governor + * @dev In this module, forVotes must be > againstVotes. */ function _voteSucceeded(uint256 proposalId) internal view virtual override returns (bool) { ProposalVote storage proposalVote = _proposalVotes[proposalId]; @@ -106,11 +123,8 @@ abstract contract GovernorCountingFractional is Governor { } /** - * @notice See {Governor-_countVote}. - * - * @dev Function that records the delegate's votes. - * - * If the `voteData` bytes parameter is empty, then this module behaves + * @inheritdoc Governor + * @dev If the `voteData` bytes parameter is empty, then this module behaves * identically to GovernorBravo. That is, it assigns the full weight of the * delegate to the `support` parameter, which follows the `VoteType` enum * from Governor Bravo. @@ -132,9 +146,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); @@ -147,9 +164,8 @@ abstract contract GovernorCountingFractional is Governor { } /** - * @dev Record votes with full weight cast for `support`. - * - * Because this function votes with the delegate's full weight, it can only + * @notice Record votes with full weight cast for `support`. + * @dev Because this function votes with the delegate's full weight, it can only * be called once per proposal. It will revert if combined with a fractional * vote before or after. */ @@ -159,28 +175,26 @@ 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; - if (support == uint8(GovernorCompatibilityBravo.VoteType.Against)) { + if (support == uint8(VoteType.Against)) { _proposalVotes[proposalId].againstVotes += totalWeight; - } else if (support == uint8(GovernorCompatibilityBravo.VoteType.For)) { + } else if (support == uint8(VoteType.For)) { _proposalVotes[proposalId].forVotes += totalWeight; - } else if (support == uint8(GovernorCompatibilityBravo.VoteType.Abstain)) { + } else if (support == uint8(VoteType.Abstain)) { _proposalVotes[proposalId].abstainVotes += totalWeight; } else { - revert("GovernorCountingFractional: invalid support value, must be included in VoteType enum"); + revert GovernorInvalidVoteType(); } } /** - * @dev Count votes with fractional weight. - * - * `voteData` is expected to be three packed uint128s, i.e. + * @notice Count votes with fractional weight. + * @dev `voteData` is expected to be three packed uint128s, i.e. * `abi.encodePacked(againstVotes, forVotes, abstainVotes)`. * * This function can be called multiple times for the same account and @@ -204,14 +218,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. @@ -230,7 +248,7 @@ abstract contract GovernorCountingFractional is Governor { uint256 constant internal _MASK_HALF_WORD_RIGHT = 0xffffffffffffffffffffffffffffffff; // 128 bits of 0's, 128 bits of 1's /** - * @dev Decodes three packed uint128's. Uses assembly because of a Solidity + * @notice Decodes three packed uint128's. Uses assembly because of a Solidity * language limitation which prevents slicing bytes stored in memory, rather * than calldata. */ @@ -250,96 +268,4 @@ abstract contract GovernorCountingFractional is Governor { } } - /** - * @notice Cast a vote with a reason and additional encoded parameters using - * the user's cryptographic signature. - * - * Emits a {VoteCast} or {VoteCastWithParams} event depending on the length - * of params. - * - * @dev If casting a fractional vote via `params`, the voter's current nonce - * must be appended to the `params` as the last 16 bytes and included in the - * signature. I.e., the params used when constructing the signature would be: - * - * abi.encodePacked(againstVotes, forVotes, abstainVotes, nonce) - * - * See {fractionalVoteNonce} and {_castVote} for more information. - */ - function castVoteWithReasonAndParamsBySig( - uint256 proposalId, - uint8 support, - string calldata reason, - bytes memory params, - uint8 v, - bytes32 r, - bytes32 s - ) public virtual override returns (uint256) { - // Signature-based fractional voting requires `params` be two full words - // in length: - // 16 bytes for againstVotes. - // 16 bytes for forVotes. - // 16 bytes for abstainVotes. - // 16 bytes for the signature nonce. - // Signature-based nominal voting requires `params` be 0 bytes. - require( - params.length == 64 || params.length == 0, - "GovernorCountingFractional: invalid params for signature-based vote" - ); - - address voter = ECDSA.recover( - _hashTypedDataV4( - keccak256( - abi.encode( - EXTENDED_BALLOT_TYPEHASH, - proposalId, - support, - keccak256(bytes(reason)), - keccak256(params) - ) - ) - ), - v, - r, - s - ); - - // If params are zero-length all of the voter's weight will be cast so - // we don't have to worry about checking/incrementing a nonce. - if (params.length == 64) { - // Get the nonce out of the params. It is the last half-word. - uint128 nonce; - assembly { - nonce := and( - // Perform bitwise AND operation on the data in the second word of - // `params` with a mask of 128 zeros followed by 128 ones, i.e. take - // the last 128 bits of `params`. - _MASK_HALF_WORD_RIGHT, - // Load the data from memory at the returned address. - mload( - // Skip the first 64 bytes (0x40): - // 32 bytes encoding the length of the bytes array. - // 32 bytes for the first word in the params - // Return the memory address for the last word in params. - add(params, 0x40) - ) - ) - } - - require( - fractionalVoteNonce[voter] == nonce, - "GovernorCountingFractional: signature has already been used" - ); - - fractionalVoteNonce[voter]++; - - // Trim params in place to keep only the first 48 bytes (which are - // the voting params) and save gas. - assembly { - mstore(params, 0x30) - } - } - - return _castVote(proposalId, voter, support, reason, params); - } - } diff --git a/src/interfaces/IFractionalGovernor.sol b/src/interfaces/IFractionalGovernor.sol index 6541fe4..4aef6b6 100644 --- a/src/interfaces/IFractionalGovernor.sol +++ b/src/interfaces/IFractionalGovernor.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.10; +pragma solidity ^0.8.20; /// @dev The interface that flexible voting-compatible governors are expected to support. interface IFractionalGovernor { diff --git a/src/interfaces/IVotingToken.sol b/src/interfaces/IVotingToken.sol index a166347..f8e3809 100644 --- a/src/interfaces/IVotingToken.sol +++ b/src/interfaces/IVotingToken.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.10; +pragma solidity ^0.8.20; /// @dev The interface that flexible voting-compatible voting tokens are expected to support. interface IVotingToken { diff --git a/test/FractionalGovernor.sol b/test/FractionalGovernor.sol index a53dfe3..54d26a0 100644 --- a/test/FractionalGovernor.sol +++ b/test/FractionalGovernor.sol @@ -1,26 +1,14 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.10; +pragma solidity ^0.8.20; -import "../src/GovernorCountingFractional.sol"; -import "@openzeppelin/contracts/governance/extensions/GovernorVotes.sol"; +import {GovernorCountingFractional} from "../src/GovernorCountingFractional.sol"; +import {GovernorVotes} from "@openzeppelin/contracts/governance/extensions/GovernorVotes.sol"; +import {IVotes} from "@openzeppelin/contracts/governance/utils/IVotes.sol"; +import {Governor} from "@openzeppelin/contracts/governance/Governor.sol"; contract FractionalGovernor is GovernorVotes, GovernorCountingFractional { constructor(string memory name_, IVotes token_) Governor(name_) GovernorVotes(token_) {} - function castVoteWithReasonAndParamsBySig( - uint256 proposalId, - uint8 support, - string calldata reason, - bytes memory params, - uint8 v, - bytes32 r, - bytes32 s - ) public virtual override(GovernorCountingFractional, Governor) returns (uint256) { - return GovernorCountingFractional.castVoteWithReasonAndParamsBySig( - proposalId, support, reason, params, v, r, s - ); - } - function quorum(uint256) public pure override returns (uint256) { return 10 ether; } @@ -37,10 +25,6 @@ contract FractionalGovernor is GovernorVotes, GovernorCountingFractional { return _quorumReached(_proposalId); } - function exposed_setFractionalVoteNonce(address _voter, uint128 _newNonce) public { - fractionalVoteNonce[_voter] = _newNonce; - } - function cancel( address[] memory targets, uint256[] memory values, diff --git a/test/FractionalPool.t.sol b/test/FractionalPool.t.sol index 8ce0afa..80723bb 100644 --- a/test/FractionalPool.t.sol +++ b/test/FractionalPool.t.sol @@ -1,8 +1,9 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.10; +pragma solidity ^0.8.20; import {Test} from "forge-std/Test.sol"; import {Vm} from "forge-std/Vm.sol"; +import {IGovernor} from "@openzeppelin/contracts/governance/Governor.sol"; import {FractionalPool, IVotingToken, IFractionalGovernor} from "../src/FractionalPool.sol"; import "./GovToken.sol"; import "./FractionalGovernor.sol"; @@ -115,7 +116,7 @@ contract Deployment is FractionalPoolTest { contract Deposit is FractionalPoolTest { function test_UserCanDepositGovTokens(address _holder, uint256 _amount) public { - _amount = bound(_amount, 0, type(uint224).max); + _amount = bound(_amount, 0, type(uint208).max); vm.assume(_holder != address(pool)); uint256 initialBalance = token.balanceOf(_holder); @@ -245,9 +246,17 @@ contract Vote is FractionalPoolTest { // Jump ahead so that we're outside of the proposal's voting period. vm.roll(governor.proposalDeadline(_proposalId) + 1); + IGovernor.ProposalState status = IGovernor.ProposalState(uint32(governor.state(_proposalId))); // We should not be able to castVote at this point. - vm.expectRevert(bytes("Governor: vote not currently active")); + vm.expectRevert( + abi.encodeWithSelector( + IGovernor.GovernorUnexpectedProposalState.selector, + _proposalId, + status, + bytes32(1 << uint8(IGovernor.ProposalState.Active)) + ) + ); pool.castVote(_proposalId); } @@ -446,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/GovToken.sol b/test/GovToken.sol index 5cb3f61..55e0f5e 100644 --- a/test/GovToken.sol +++ b/test/GovToken.sol @@ -1,11 +1,12 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.10; +pragma solidity ^0.8.20; import {ERC20Votes} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Votes.sol"; -import {ERC20Permit} from "@openzeppelin/contracts/token/ERC20/extensions/draft-ERC20Permit.sol"; +import {ERC20Permit} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Permit.sol"; import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import {Nonces} from "@openzeppelin/contracts/utils/Nonces.sol"; -contract GovToken is ERC20Votes { +contract GovToken is ERC20, ERC20Permit, ERC20Votes { constructor() ERC20("Governance Token", "GOV") ERC20Permit("GOV") {} function exposed_mint(address to, uint256 amount) public { @@ -15,4 +16,18 @@ contract GovToken is ERC20Votes { function exposed_maxSupply() external view returns (uint256) { return uint256(_maxSupply()); } + + function _update(address from, address to, uint256 amount) internal override(ERC20, ERC20Votes) { + ERC20Votes._update(from, to, amount); + } + + function nonces(address owner) + public + view + virtual + override(ERC20Permit, Nonces) + returns (uint256) + { + return Nonces.nonces(owner); + } } diff --git a/test/GovernorCountingFractional.t.sol b/test/GovernorCountingFractional.t.sol index cd3f029..ef38939 100644 --- a/test/GovernorCountingFractional.t.sol +++ b/test/GovernorCountingFractional.t.sol @@ -1,16 +1,17 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.10; +pragma solidity ^0.8.20; import {Test} from "forge-std/Test.sol"; import {Vm} from "forge-std/Vm.sol"; import {FractionalPool, IVotingToken, IFractionalGovernor} from "../src/FractionalPool.sol"; -import {GovernorCompatibilityBravo} from - "@openzeppelin/contracts/governance/compatibility/GovernorCompatibilityBravo.sol"; import {FixedPointMathLib} from "solmate/utils/FixedPointMathLib.sol"; import {GovToken} from "./GovToken.sol"; -import {FractionalGovernor, IVotes, IGovernor} 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"; +import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; contract GovernorCountingFractionalTest is Test { using FixedPointMathLib for uint256; @@ -40,6 +41,12 @@ contract GovernorCountingFractionalTest is Test { string description ); + enum VoteType { + Against, + For, + Abstain + } + // We use a min of 1e4 to avoid flooring votes to 0. uint256 constant MIN_VOTE_WEIGHT = 1e4; // This is the vote storage slot size on the Fractional Governor contract. @@ -152,11 +159,10 @@ contract GovernorCountingFractionalTest is Test { Proposal memory _rawProposalInfo = _getSimpleProposal(); vm.expectEmit(true, true, true, true); - emit ProposalExecuted(_rawProposalInfo.id); + emit MockFunctionCalled(); - // Ensure that the other contract is invoked. vm.expectEmit(true, true, true, true); - emit MockFunctionCalled(); + emit ProposalExecuted(_rawProposalInfo.id); governor.execute( _rawProposalInfo.targets, @@ -196,7 +202,7 @@ contract GovernorCountingFractionalTest is Test { } function _randomSupportType(uint256 salt) public view returns (uint8) { - return uint8(bound(salt, 0, uint8(GovernorCompatibilityBravo.VoteType.Abstain))); + return uint8(bound(salt, 0, uint8(VoteType.Abstain))); } function _randomVoteSplit(FractionalVoteSplit memory _voteSplit) @@ -253,15 +259,9 @@ contract GovernorCountingFractionalTest is Test { againstVotes += uint128(voter.weight.mulWadDown(voter.voteSplit.percentAgainst)); abstainVotes += uint128(voter.weight.mulWadDown(voter.voteSplit.percentAbstain)); } else { - if (voter.support == uint8(GovernorCompatibilityBravo.VoteType.For)) { - forVotes += voter.weight; - } - if (voter.support == uint8(GovernorCompatibilityBravo.VoteType.Against)) { - againstVotes += voter.weight; - } - if (voter.support == uint8(GovernorCompatibilityBravo.VoteType.Abstain)) { - abstainVotes += voter.weight; - } + if (voter.support == uint8(VoteType.For)) forVotes += voter.weight; + if (voter.support == uint8(VoteType.Against)) againstVotes += voter.weight; + if (voter.support == uint8(VoteType.Abstain)) abstainVotes += voter.weight; } } } @@ -332,7 +332,15 @@ contract GovernorCountingFractionalTest is Test { assertEq(uint8(status), uint8(IGovernor.ProposalState.Defeated)); Proposal memory _rawProposalInfo = _getSimpleProposal(); - vm.expectRevert(bytes("Governor: proposal not successful")); + vm.expectRevert( + abi.encodeWithSelector( + IGovernor.GovernorUnexpectedProposalState.selector, + _proposalId, + status, + bytes32(1 << uint8(IGovernor.ProposalState.Succeeded)) + | bytes32(1 << uint8(IGovernor.ProposalState.Queued)) + ) + ); governor.execute( _rawProposalInfo.targets, _rawProposalInfo.values, @@ -372,242 +380,6 @@ contract GovernorCountingFractionalTest is Test { _fractionalGovernorHappyPathTest(voters); } - function testFuzz_NominalVotingWithFractionalizedParamsAndSignature( - uint256 _weight, - uint128 _nonce - ) public { - Voter memory _voter; - uint256 _privateKey; - (_voter.addr, _privateKey) = makeAddrAndKey("voter"); - vm.assume(_voter.addr != address(this)); - - // Use a random nonce. Bound by max - 1 because we need to sign one message. - _nonce = uint128(bound(_nonce, 0, type(uint128).max - 1)); - governor.exposed_setFractionalVoteNonce(_voter.addr, _nonce); - uint128 _initNonce = governor.fractionalVoteNonce(_voter.addr); - - _voter.weight = bound(_weight, MIN_VOTE_WEIGHT, MAX_VOTE_WEIGHT); - _voter.support = _randomSupportType(_weight); - - _mintAndDelegateToVoter(_voter); - uint256 _proposalId = _createAndSubmitProposal(); - - bytes32 _voteMessage = keccak256( - abi.encode( - keccak256("ExtendedBallot(uint256 proposalId,uint8 support,string reason,bytes params)"), - _proposalId, - _voter.support, - keccak256(bytes("I have my reasons")), - keccak256(new bytes(0)) - ) - ); - - bytes32 _voteMessageHash = - keccak256(abi.encodePacked("\x19\x01", EIP712_DOMAIN_SEPARATOR, _voteMessage)); - - (uint8 _v, bytes32 _r, bytes32 _s) = vm.sign(_privateKey, _voteMessageHash); - - governor.castVoteWithReasonAndParamsBySig( - _proposalId, _voter.support, "I have my reasons", new bytes(0), _v, _r, _s - ); - - assertEq( - _initNonce, governor.fractionalVoteNonce(_voter.addr), "nonce should not have incremented" - ); - - (uint256 _actualAgainstVotes, uint256 _actualForVotes, uint256 _actualAbstainVotes) = - governor.proposalVotes(_proposalId); - if (_voter.support == uint8(GovernorCompatibilityBravo.VoteType.For)) { - assertEq(_voter.weight, _actualForVotes); - } - if (_voter.support == uint8(GovernorCompatibilityBravo.VoteType.Against)) { - assertEq(_voter.weight, _actualAgainstVotes); - } - if (_voter.support == uint8(GovernorCompatibilityBravo.VoteType.Abstain)) { - assertEq(_voter.weight, _actualAbstainVotes); - } - - // The signature cannot be re-used. - vm.expectRevert("GovernorCountingFractional: all weight cast"); - governor.castVoteWithReasonAndParamsBySig( - _proposalId, _voter.support, "I have my reasons", new bytes(0), _v, _r, _s - ); - } - - struct CastVoteWithReasonAndParamsBySigTestVars { - uint128 forVotes; - uint128 againstVotes; - uint128 abstainVotes; - uint128 lastNonce; - bytes fractionalizedVotes; - uint256 privateKey; - uint256 proposalId; - Voter voter; - bytes32 voteMessage; - bytes32 voteMessageHash; - uint8 v; - bytes32 r; - bytes32 s; - uint256 actualAgainstVotes; - uint256 actualForVotes; - uint256 actualAbstainVotes; - uint256 remainingWeight; - } - - function testFuzz_VotingWithFractionalizedParamsAndSignature( - uint256 _weight, - uint256 _partialVoteWeight, - FractionalVoteSplit memory _voteSplit, - uint128 _nonce - ) public { - CastVoteWithReasonAndParamsBySigTestVars memory _vars; - (_vars.voter.addr, _vars.privateKey) = makeAddrAndKey("voter with replay protection"); - vm.assume(_vars.voter.addr != address(this)); - - // Use a random nonce. Bound by (max - 2) because we need to sign 2 messages. - _nonce = uint128(bound(_nonce, 0, type(uint128).max - 2)); - governor.exposed_setFractionalVoteNonce(_vars.voter.addr, _nonce); - - _vars.voter.weight = bound(_weight, MIN_VOTE_WEIGHT, MAX_VOTE_WEIGHT); - _vars.voter.support = _randomSupportType(_weight); - _vars.voter.voteSplit = _randomVoteSplit(_voteSplit); - - // We want to be able to cast two distinct signature-based votes. - _partialVoteWeight = bound( - _partialVoteWeight, - _vars.voter.weight.mulWadDown(0.1e18), // 10% - _vars.voter.weight.mulWadDown(0.9e18) // 90% - ); - - _vars.forVotes = uint128(_partialVoteWeight.mulWadDown(_voteSplit.percentFor)); - _vars.againstVotes = uint128(_partialVoteWeight.mulWadDown(_voteSplit.percentAgainst)); - _vars.abstainVotes = uint128(_partialVoteWeight.mulWadDown(_voteSplit.percentAbstain)); - _vars.fractionalizedVotes = abi.encodePacked( - _vars.againstVotes, - _vars.forVotes, - _vars.abstainVotes, - governor.fractionalVoteNonce(_vars.voter.addr) - ); - - _mintAndDelegateToVoter(_vars.voter); - _vars.proposalId = _createAndSubmitProposal(); - - _vars.voteMessage = keccak256( - abi.encode( - governor.EXTENDED_BALLOT_TYPEHASH(), - _vars.proposalId, - _vars.voter.support, - keccak256(bytes("I have my reasons")), - keccak256(_vars.fractionalizedVotes) - ) - ); - - _vars.voteMessageHash = - keccak256(abi.encodePacked("\x19\x01", EIP712_DOMAIN_SEPARATOR, _vars.voteMessage)); - - (_vars.v, _vars.r, _vars.s) = vm.sign(_vars.privateKey, _vars.voteMessageHash); - _vars.lastNonce = governor.fractionalVoteNonce(_vars.voter.addr); - - // First vote. - governor.castVoteWithReasonAndParamsBySig( - _vars.proposalId, - _vars.voter.support, - "I have my reasons", - _vars.fractionalizedVotes, - _vars.v, - _vars.r, - _vars.s - ); - - // Nonce should have incremented. - assertEq( - _vars.lastNonce + 1, - governor.fractionalVoteNonce(_vars.voter.addr), - "nonce should have incremented" - ); - _vars.lastNonce = governor.fractionalVoteNonce(_vars.voter.addr); - - (_vars.actualAgainstVotes, _vars.actualForVotes, _vars.actualAbstainVotes) = - governor.proposalVotes(_vars.proposalId); - - assertEq(_vars.forVotes, _vars.actualForVotes); - assertEq(_vars.againstVotes, _vars.actualAgainstVotes); - assertEq(_vars.abstainVotes, _vars.actualAbstainVotes); - - // Try to re-use the signature, which should revert. - vm.expectRevert("GovernorCountingFractional: signature has already been used"); - governor.castVoteWithReasonAndParamsBySig( - _vars.proposalId, - _vars.voter.support, - "I have my reasons", - _vars.fractionalizedVotes, - _vars.v, - _vars.r, - _vars.s - ); - - // Nonce shouldn't have changed since the first successful vote. - assertEq( - _vars.lastNonce, - governor.fractionalVoteNonce(_vars.voter.addr), - "nonce should not have incremented" - ); - - // Sign a new message. - _vars.remainingWeight = _vars.voter.weight - _partialVoteWeight; - _vars.forVotes = uint128(_vars.remainingWeight.mulWadDown(_voteSplit.percentFor)); - _vars.againstVotes = uint128(_vars.remainingWeight.mulWadDown(_voteSplit.percentAgainst)); - _vars.abstainVotes = uint128(_vars.remainingWeight.mulWadDown(_voteSplit.percentAbstain)); - _vars.fractionalizedVotes = abi.encodePacked( - _vars.againstVotes, - _vars.forVotes, - _vars.abstainVotes, - governor.fractionalVoteNonce(_vars.voter.addr) - ); - _vars.voteMessage = keccak256( - abi.encode( - governor.EXTENDED_BALLOT_TYPEHASH(), - _vars.proposalId, - _vars.voter.support, - keccak256(bytes("I have my reasons")), - keccak256(_vars.fractionalizedVotes) - ) - ); - _vars.voteMessageHash = - keccak256(abi.encodePacked("\x19\x01", EIP712_DOMAIN_SEPARATOR, _vars.voteMessage)); - (_vars.v, _vars.r, _vars.s) = vm.sign(_vars.privateKey, _vars.voteMessageHash); - - // Submit second signed vote. It should succeed. - governor.castVoteWithReasonAndParamsBySig( - _vars.proposalId, - _vars.voter.support, - "I have my reasons", - _vars.fractionalizedVotes, - _vars.v, - _vars.r, - _vars.s - ); - - // Nonce should have incremented again. - assertEq( - _vars.lastNonce + 1, - governor.fractionalVoteNonce(_vars.voter.addr), - "nonce should have incremented" - ); - - (_vars.actualAgainstVotes, _vars.actualForVotes, _vars.actualAbstainVotes) = - governor.proposalVotes(_vars.proposalId); - - // Actual weights can differ by up to 1 because of rounding during division. - assertApproxEqAbs(_vars.voter.weight.mulWadDown(_voteSplit.percentFor), _vars.actualForVotes, 1); - assertApproxEqAbs( - _vars.voter.weight.mulWadDown(_voteSplit.percentAgainst), _vars.actualAgainstVotes, 1 - ); - assertApproxEqAbs( - _vars.voter.weight.mulWadDown(_voteSplit.percentAbstain), _vars.actualAbstainVotes, 1 - ); - } - function testFuzz_VoteSplitsCanBeMaxedOut(uint256[4] memory _weights, uint8 _maxSplit) public { Voter[4] memory _voters = _setupNominalVoters(_weights); @@ -638,20 +410,28 @@ 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(GovernorCompatibilityBravo.VoteType.For), + uint8(VoteType.For), "I hope no one catches me doing this!", new bytes(0) // No data, this is a nominal vote. ); // 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(GovernorCompatibilityBravo.VoteType.For), + uint8(VoteType.For), "I'm so bad", abi.encodePacked(type(uint128).max, type(uint128).max, type(uint128).max) ); @@ -712,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); } @@ -731,7 +515,9 @@ contract GovernorCountingFractionalTest is Test { bytes memory emptyVotingParams; vm.prank(voter.addr); - vm.expectRevert("SafeCast: value doesn't fit in 128 bits"); + vm.expectRevert( + abi.encodeWithSelector(SafeCast.SafeCastOverflowedUintDowncast.selector, 128, voter.weight) + ); governor.castVoteWithReasonAndParams(_proposalId, voter.support, "Yay", emptyVotingParams); } @@ -759,7 +545,9 @@ contract GovernorCountingFractionalTest is Test { bytes memory fractionalizedVotes = abi.encodePacked(_againstVotes, _forVotes, _abstainVotes); vm.prank(voter.addr); - vm.expectRevert("SafeCast: value doesn't fit in 128 bits"); + vm.expectRevert( + abi.encodeWithSelector(SafeCast.SafeCastOverflowedUintDowncast.selector, 128, voter.weight) + ); governor.castVoteWithReasonAndParams(_proposalId, voter.support, "Weeee", fractionalizedVotes); } @@ -781,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); } @@ -1198,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, @@ -1225,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 ); @@ -1272,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 @@ -1293,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 ); diff --git a/test/ProposalReceiverMock.sol b/test/ProposalReceiverMock.sol index 8744284..e70f51b 100644 --- a/test/ProposalReceiverMock.sol +++ b/test/ProposalReceiverMock.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.10; +pragma solidity ^0.8.20; contract ProposalReceiverMock { event MockFunctionCalled();