Skip to content

Commit

Permalink
feat: enable standard countVote and refactor module logic #12
Browse files Browse the repository at this point in the history
  • Loading branch information
jjranalli committed Jun 23, 2023
1 parent 3828e29 commit 41d5fd3
Show file tree
Hide file tree
Showing 6 changed files with 184 additions and 215 deletions.
62 changes: 18 additions & 44 deletions src/OptimismGovernorV5.sol
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,8 @@ contract OptimismGovernorV5 is
}

/**
* @dev Updated internal vote casting mechanism which allows delegating logic to custom voting module. See {IGovernor-_castVote}.
* @dev Updated internal vote casting mechanism which delegates counting logic to voting module,
* in addition to executing standard `_countVote`. See {IGovernor-_castVote}.
*/
function _castVote(uint256 proposalId, address account, uint8 support, string memory reason, bytes memory params)
internal
Expand All @@ -184,10 +185,10 @@ contract OptimismGovernorV5 is

uint256 weight = _getVotes(account, proposal.voteStart.getDeadline(), params);

_countVote(proposalId, account, support, weight, params);

if (proposal.votingModule != address(0)) {
VotingModule(proposal.votingModule)._countVote(proposalId, account, support, weight, params);
} else {
_countVote(proposalId, account, support, weight, params);
}

if (params.length == 0) {
Expand Down Expand Up @@ -217,46 +218,7 @@ contract OptimismGovernorV5 is
}

/**
* @dev Updated `hasVoted` which allows delegating logic to custom voting module. See {IGovernor-hasVoted}.
*/
function hasVoted(uint256 proposalId, address account)
public
view
virtual
override(GovernorCountingSimpleUpgradeableV2, IGovernorUpgradeable)
returns (bool)
{
address votingModule = _proposals[proposalId].votingModule;
if (votingModule != address(0)) {
return VotingModule(votingModule).hasVoted(proposalId, account);
}

return super.hasVoted(proposalId, account);
}

/**
* @dev Updated `_quorumReached` which allows delegating logic to custom voting module. See {IGovernor-_quorumReached}.
*/
function _quorumReached(uint256 proposalId)
internal
view
virtual
override(GovernorCountingSimpleUpgradeableV2, GovernorUpgradeableV2)
returns (bool)
{
ProposalCore memory proposal = _proposals[proposalId];
if (proposal.votingModule != address(0)) {
return
VotingModule(proposal.votingModule)._quorumReached(proposalId, quorum(proposal.voteStart.getDeadline()));
}

(uint256 againstVotes, uint256 forVotes, uint256 abstainVotes) = proposalVotes(proposalId);

return quorum(proposalSnapshot(proposalId)) <= againstVotes + forVotes + abstainVotes;
}

/**
* @dev Updated `_voteSucceeded` which allows delegating logic to custom voting module. See {Governor-_voteSucceeded}.
* @dev Updated `_voteSucceeded` to add custom success conditions defined in the voting module. See {Governor-_voteSucceeded}.
*/
function _voteSucceeded(uint256 proposalId)
internal
Expand All @@ -267,7 +229,7 @@ contract OptimismGovernorV5 is
{
address votingModule = _proposals[proposalId].votingModule;
if (votingModule != address(0)) {
return VotingModule(votingModule)._voteSucceeded(proposalId);
if (!VotingModule(votingModule)._voteSucceeded(proposalId)) return false;
}

return super._voteSucceeded(proposalId);
Expand Down Expand Up @@ -337,6 +299,18 @@ contract OptimismGovernorV5 is
return 100_000;
}

function _quorumReached(uint256 proposalId)
internal
view
virtual
override(GovernorCountingSimpleUpgradeableV2, GovernorUpgradeableV2)
returns (bool)
{
(uint256 againstVotes, uint256 forVotes, uint256 abstainVotes) = proposalVotes(proposalId);

return quorum(proposalSnapshot(proposalId)) <= againstVotes + forVotes + abstainVotes;
}

function setProposalDeadline(uint256 proposalId, uint64 deadline) public onlyManager {
_proposals[proposalId].voteEnd.setDeadline(deadline);
emit ProposalDeadlineUpdated(proposalId, deadline);
Expand Down
102 changes: 25 additions & 77 deletions src/modules/ApprovalVotingModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {VotingModule} from "./VotingModule.sol";
import {SafeCastLib} from "@solady/utils/SafeCastLib.sol";

enum VoteType {
Against,
For,
Abstain
}
Expand All @@ -21,11 +22,6 @@ struct ExecuteParams {
bytes calldatas;
}

struct ProposalVotes {
uint128 forVotes;
uint128 abstainVotes;
}

struct ProposalSettings {
uint8 maxApprovals;
uint8 criteria;
Expand All @@ -46,7 +42,6 @@ struct Proposal {
address governor;
uint256 initBalance;
uint128[] optionVotes;
ProposalVotes votes;
ProposalOption[] options;
ProposalSettings settings;
}
Expand Down Expand Up @@ -95,8 +90,8 @@ contract ApprovalVotingModule is VotingModule {
STORAGE
//////////////////////////////////////////////////////////////*/

mapping(uint256 proposalId => Proposal) public _proposals;
mapping(uint256 proposalId => mapping(address account => uint8 votes)) public _accountVotes;
mapping(uint256 proposalId => Proposal) public proposals;
mapping(uint256 proposalId => mapping(address account => uint256 votes)) public accountVotes;

/*//////////////////////////////////////////////////////////////
WRITE FUNCTIONS
Expand All @@ -113,7 +108,7 @@ contract ApprovalVotingModule is VotingModule {
revert WrongProposalId();
}

if (_proposals[proposalId].governor != address(0)) revert ExistingProposal();
if (proposals[proposalId].governor != address(0)) revert ExistingProposal();

(ProposalOption[] memory proposalOptions, ProposalSettings memory proposalSettings) =
abi.decode(proposalData, (ProposalOption[], ProposalSettings));
Expand All @@ -133,41 +128,39 @@ contract ApprovalVotingModule is VotingModule {
revert InvalidParams();
}

_proposals[proposalId].options.push(option);
proposals[proposalId].options.push(option);
}
}

_proposals[proposalId].governor = msg.sender;
_proposals[proposalId].settings = proposalSettings;
_proposals[proposalId].optionVotes = new uint128[](optionsLength);
proposals[proposalId].governor = msg.sender;
proposals[proposalId].settings = proposalSettings;
proposals[proposalId].optionVotes = new uint128[](optionsLength);
}

/**
* Count approvals voted by `account`. If voting for, options need to be set in ascending order.
* Count approvals voted by `account`. If voting for, options need to be set in ascending order. Votes can only be cast once.
*
* @param proposalId The id of the proposal.
* @param account The account to count votes for.
* @param support The type of vote to count. 0 = For, 1 = Abstain.
* @param support The type of vote to count.
* @param weight The weight of the vote.
* @param params The ids of the options to vote for sorted in ascending order, encoded as `uint256[]`.
*/
function _countVote(uint256 proposalId, address account, uint8 support, uint256 weight, bytes memory params)
external
override
{
Proposal memory proposal = _proposals[proposalId];
Proposal memory proposal = proposals[proposalId];
_onlyGovernor(proposal.governor);

if (hasVoted(proposalId, account)) revert VoteAlreadyCast();

uint128 weight_ = weight.toUint128();
if (accountVotes[proposalId][account] != 0) revert AlreadyVoted();

if (support == uint8(VoteType.For)) {
uint256[] memory options = abi.decode(params, (uint256[]));
uint256 totalOptions = options.length;
if (totalOptions == 0) revert InvalidParams();
if (totalOptions > proposal.settings.maxApprovals) revert MaxApprovalsExceeded();

uint128 weight_ = weight.toUint128();
uint256 option;
uint256 prevOption;
for (uint256 i; i < totalOptions;) {
Expand All @@ -181,21 +174,14 @@ contract ApprovalVotingModule is VotingModule {
prevOption = option;

/// @dev Revert if `option` is out of bounds
_proposals[proposalId].optionVotes[option] += weight_;
proposals[proposalId].optionVotes[option] += weight_;

unchecked {
++i;
}
}

/// @dev `totalOptions` cannot overflow uint8 as it is checked against `maxApprovals`
_accountVotes[proposalId][account] = uint8(totalOptions);
_proposals[proposalId].votes.forVotes += weight_;
} else if (support == uint8(VoteType.Abstain)) {
_accountVotes[proposalId][account] = 1;
_proposals[proposalId].votes.abstainVotes += weight_;
} else {
revert InvalidVoteType();
accountVotes[proposalId][account] = totalOptions;
}
}

Expand All @@ -213,7 +199,7 @@ contract ApprovalVotingModule is VotingModule {
override
returns (address[] memory targets, uint256[] memory values, bytes[] memory calldatas)
{
address governor = _proposals[proposalId].governor;
address governor = proposals[proposalId].governor;
_onlyGovernor(governor);

(ProposalOption[] memory options, ProposalSettings memory settings) =
Expand All @@ -222,11 +208,11 @@ contract ApprovalVotingModule is VotingModule {
// If budgetToken is not ETH
if (settings.budgetToken != address(0)) {
// Save initBalance to be used as comparison in `_afterExecute`
_proposals[proposalId].initBalance = IERC20(settings.budgetToken).balanceOf(governor);
proposals[proposalId].initBalance = IERC20(settings.budgetToken).balanceOf(governor);
}

(uint128[] memory sortedOptionVotes, ProposalOption[] memory sortedOptions) =
_sortOptions(_proposals[proposalId].optionVotes, options);
_sortOptions(proposals[proposalId].optionVotes, options);

(uint256 executeParamsLength, uint256 succeededOptionsLength) =
_countOptions(sortedOptions, sortedOptionVotes, settings);
Expand Down Expand Up @@ -306,10 +292,10 @@ contract ApprovalVotingModule is VotingModule {
(, ProposalSettings memory settings) = abi.decode(proposalData, (ProposalOption[], ProposalSettings));

if (settings.budgetToken != address(0)) {
address governor = _proposals[proposalId].governor;
address governor = proposals[proposalId].governor;

if (
_proposals[proposalId].initBalance - IERC20(settings.budgetToken).balanceOf(governor)
proposals[proposalId].initBalance - IERC20(settings.budgetToken).balanceOf(governor)
> settings.budgetAmount
) revert BudgetExceeded();
}
Expand All @@ -319,52 +305,14 @@ contract ApprovalVotingModule is VotingModule {
VIEW FUNCTIONS
//////////////////////////////////////////////////////////////*/

/**
* @dev Return true if `account` has cast a vote for `proposalId`.
*
* @param proposalId The id of the proposal.
* @param account The address of the account.
*/
function hasVoted(uint256 proposalId, address account) public view override returns (bool) {
return _accountVotes[proposalId][account] != 0;
}

/**
* @dev Return for, abstain and option votes for a `proposalId`.
*
* @param proposalId The id of the proposal.
*/
function proposalVotes(uint256 proposalId)
public
view
virtual
returns (uint256 forVotes, uint256 abstainVotes, uint128[] memory optionVotes)
{
ProposalVotes memory votes = _proposals[proposalId].votes;
return (votes.forVotes, votes.abstainVotes, _proposals[proposalId].optionVotes);
}

/**
* @dev Used by governor in `_quorumReached`. See {Governor-_quorumReached}.
*
* @param proposalId The id of the proposal.
* @param quorum The quorum value at the proposal start block.
*/
function _quorumReached(uint256 proposalId, uint256 quorum) external view override returns (bool) {
_onlyGovernor(_proposals[proposalId].governor);
ProposalVotes memory votes = _proposals[proposalId].votes;

return quorum <= votes.forVotes + votes.abstainVotes;
}

/**
* @dev Return true if at least one option satisfies the passing criteria.
* Used by governor in `_voteSucceeded`. See {Governor-_voteSucceeded}.
*
* @param proposalId The id of the proposal.
*/
function _voteSucceeded(uint256 proposalId) external view override returns (bool) {
Proposal memory proposal = _proposals[proposalId];
Proposal memory proposal = proposals[proposalId];

ProposalOption[] memory options = proposal.options;
uint256 n = options.length;
Expand All @@ -386,12 +334,12 @@ contract ApprovalVotingModule is VotingModule {
/**
* @dev See {IGovernor-COUNTING_MODE}.
*
* - `support=for,abstain`: the vote options are 0 = For, 1 = Abstain.
* - `quorum=for,abstain`: For and Abstain votes are counted towards quorum.
* - `support=bravo`: Supports vote options 0 = Against, 1 = For, 2 = Abstain, as in `GovernorBravo`.
* - `quorum=for,abstain`: Against, For and Abstain votes are counted towards quorum.
* - `params=approvalVote`: params needs to be formatted as `VOTE_PARAMS_ENCODING`.
*/
function COUNTING_MODE() public pure virtual override returns (string memory) {
return "support=for,abstain&quorum=for,abstain&params=approvalVote";
return "support=bravo&quorum=against,for,abstain&params=approvalVote";
}

/**
Expand All @@ -402,7 +350,7 @@ contract ApprovalVotingModule is VotingModule {
}

/*//////////////////////////////////////////////////////////////
HELPERS
INTERNAL
//////////////////////////////////////////////////////////////*/

// Sort `options` by `optionVotes` in descending order
Expand Down
7 changes: 1 addition & 6 deletions src/modules/VotingModule.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@ abstract contract VotingModule {
//////////////////////////////////////////////////////////////*/

error NotGovernor();
error InvalidVoteType();
error ExistingProposal();
error InvalidParams();
error VoteAlreadyCast();
error AlreadyVoted();

/*//////////////////////////////////////////////////////////////
MODIFIERS
Expand All @@ -36,15 +35,11 @@ abstract contract VotingModule {
VIEW FUNCTIONS
//////////////////////////////////////////////////////////////*/

function hasVoted(uint256 proposalId, address account) external view virtual returns (bool);

function _formatExecuteParams(uint256 proposalId, bytes memory proposalData)
external
virtual
returns (address[] memory targets, uint256[] memory values, bytes[] memory calldatas);

function _quorumReached(uint256 proposalId, uint256 quorum) external view virtual returns (bool);

function _voteSucceeded(uint256 proposalId) external view virtual returns (bool);

function COUNTING_MODE() external pure virtual returns (string memory);
Expand Down
Loading

0 comments on commit 41d5fd3

Please sign in to comment.