Skip to content

Commit

Permalink
Improve Governor (OpenZeppelin#2794)
Browse files Browse the repository at this point in the history 
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
  • Loading branch information
@frangio @Amxx
frangio and Amxx authored Aug 4, 2021
1 parent f782943 commit 4b152bd
Show file tree
Hide file tree
Showing 15 changed files with 211 additions and 129 deletions.
20 changes: 0 additions & 20 deletions contracts/governance/Governor.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,26 +136,6 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor {
return _proposals[proposalId].voteEnd.getDeadline();
}

/**
* @dev See {IGovernor-votingDelay}
*/
function votingDelay() public view virtual override returns (uint256);

/**
* @dev See {IGovernor-votingPeriod}
*/
function votingPeriod() public view virtual override returns (uint256);

/**
* @dev See {IGovernor-quorum}
*/
function quorum(uint256 blockNumber) public view virtual override returns (uint256);

/**
* @dev See {IGovernor-getVotes}
*/
function getVotes(address account, uint256 blockNumber) public view virtual override returns (uint256);

/**
* @dev Amount of votes already casted passes the threshold limit.
*/
Expand Down
36 changes: 18 additions & 18 deletions contracts/governance/IGovernor.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import "../utils/introspection/ERC165.sol";
*
* _Available since v4.3._
*/
interface IGovernor is IERC165 {
abstract contract IGovernor is IERC165 {
enum ProposalState {
Pending,
Active,
Expand Down Expand Up @@ -57,13 +57,13 @@ interface IGovernor is IERC165 {
* @notice module:core
* @dev Name of the governor instance (used in building the ERC712 domain separator).
*/
function name() external view returns (string memory);
function name() public view virtual returns (string memory);

/**
* @notice module:core
* @dev Version of the governor instance (used in building the ERC712 domain separator). Default: "1"
*/
function version() external view returns (string memory);
function version() public view virtual returns (string memory);

/**
* @notice module:voting
Expand All @@ -82,7 +82,7 @@ interface IGovernor is IERC165 {
* JavaScript class.
*/
// solhint-disable-next-line func-name-mixedcase
function COUNTING_MODE() external pure returns (string memory);
function COUNTING_MODE() public pure virtual returns (string memory);

/**
* @notice module:core
Expand All @@ -93,32 +93,32 @@ interface IGovernor is IERC165 {
uint256[] calldata values,
bytes[] calldata calldatas,
bytes32 descriptionHash
) external pure returns (uint256);
) public pure virtual returns (uint256);

/**
* @notice module:core
* @dev Current state of a proposal, following Compound's convention
*/
function state(uint256 proposalId) external view returns (ProposalState);
function state(uint256 proposalId) public view virtual returns (ProposalState);

/**
* @notice module:core
* @dev block number used to retrieve user's votes and quorum.
*/
function proposalSnapshot(uint256 proposalId) external view returns (uint256);
function proposalSnapshot(uint256 proposalId) public view virtual returns (uint256);

/**
* @notice module:core
* @dev timestamp at which votes close.
*/
function proposalDeadline(uint256 proposalId) external view returns (uint256);
function proposalDeadline(uint256 proposalId) public view virtual returns (uint256);

/**
* @notice module:user-config
* @dev delay, in number of block, between the proposal is created and the vote starts. This can be increassed to
* leave time for users to buy voting power, of delegate it, before the voting of a proposal starts.
*/
function votingDelay() external view returns (uint256);
function votingDelay() public view virtual returns (uint256);

/**
* @notice module:user-config
Expand All @@ -127,7 +127,7 @@ interface IGovernor is IERC165 {
* Note: the {votingDelay} can delay the start of the vote. This must be considered when setting the voting
* duration compared to the voting delay.
*/
function votingPeriod() external view returns (uint256);
function votingPeriod() public view virtual returns (uint256);

/**
* @notice module:user-config
Expand All @@ -136,7 +136,7 @@ interface IGovernor is IERC165 {
* Note: The `blockNumber` parameter corresponds to the snaphot used for counting vote. This allows to scale the
* quroum depending on values such as the totalSupply of a token at this block (see {ERC20Votes}).
*/
function quorum(uint256 blockNumber) external view returns (uint256);
function quorum(uint256 blockNumber) public view virtual returns (uint256);

/**
* @notice module:reputation
Expand All @@ -145,13 +145,13 @@ interface IGovernor is IERC165 {
* Note: this can be implemented in a number of ways, for example by reading the delegated balance from one (or
* multiple), {ERC20Votes} tokens.
*/
function getVotes(address account, uint256 blockNumber) external view returns (uint256);
function getVotes(address account, uint256 blockNumber) public view virtual returns (uint256);

/**
* @notice module:voting
* @dev Returns weither `account` has casted a vote on `proposalId`.
*/
function hasVoted(uint256 proposalId, address account) external view returns (bool);
function hasVoted(uint256 proposalId, address account) public view virtual returns (bool);

/**
* @dev Create a new proposal. Vote start {IGovernor-votingDelay} blocks after the proposal is created and ends
Expand All @@ -164,7 +164,7 @@ interface IGovernor is IERC165 {
uint256[] memory values,
bytes[] memory calldatas,
string memory description
) external returns (uint256 proposalId);
) public virtual returns (uint256 proposalId);

/**
* @dev Execute a successful proposal. This requiers the quorum to be reached, the vote to be successful, and the
Expand All @@ -179,14 +179,14 @@ interface IGovernor is IERC165 {
uint256[] memory values,
bytes[] memory calldatas,
bytes32 descriptionHash
) external payable returns (uint256 proposalId);
) public payable virtual returns (uint256 proposalId);

/**
* @dev Cast a vote
*
* Emits a {VoteCast} event.
*/
function castVote(uint256 proposalId, uint8 support) external returns (uint256 balance);
function castVote(uint256 proposalId, uint8 support) public virtual returns (uint256 balance);

/**
* @dev Cast a with a reason
Expand All @@ -197,7 +197,7 @@ interface IGovernor is IERC165 {
uint256 proposalId,
uint8 support,
string calldata reason
) external returns (uint256 balance);
) public virtual returns (uint256 balance);

/**
* @dev Cast a vote using the user cryptographic signature.
Expand All @@ -210,5 +210,5 @@ interface IGovernor is IERC165 {
uint8 v,
bytes32 r,
bytes32 s
) external returns (uint256 balance);
) public virtual returns (uint256 balance);
}
84 changes: 50 additions & 34 deletions contracts/governance/compatibility/GovernorCompatibilityBravo.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pragma solidity ^0.8.0;
import "../../utils/Counters.sol";
import "../../utils/math/SafeCast.sol";
import "../extensions/IGovernorTimelock.sol";
import "../extensions/GovernorProposalThreshold.sol";
import "../Governor.sol";
import "./IGovernorCompatibilityBravo.sol";

Expand All @@ -16,7 +17,12 @@ import "./IGovernorCompatibilityBravo.sol";
*
* _Available since v4.3._
*/
abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorCompatibilityBravo, Governor {
abstract contract GovernorCompatibilityBravo is
IGovernorTimelock,
IGovernorCompatibilityBravo,
Governor,
GovernorProposalThreshold
{
using Counters for Counters.Counter;
using Timers for Timers.BlockNumber;

Expand All @@ -41,20 +47,6 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp

mapping(uint256 => ProposalDetails) private _proposalDetails;

// public for hooking
function proposalThreshold() public view virtual override returns (uint256);

// public for hooking
function proposalEta(uint256 proposalId) public view virtual override returns (uint256);

// public for hooking
function queue(
address[] memory targets,
uint256[] memory values,
bytes[] memory calldatas,
bytes32 descriptionHash
) public virtual override returns (uint256);

// solhint-disable-next-line func-name-mixedcase
function COUNTING_MODE() public pure virtual override returns (string memory) {
return "support=bravo&quorum=bravo";
Expand All @@ -69,8 +61,9 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp
uint256[] memory values,
bytes[] memory calldatas,
string memory description
) public virtual override(IGovernor, Governor) returns (uint256) {
return propose(targets, values, new string[](calldatas.length), calldatas, description);
) public virtual override(IGovernor, Governor, GovernorProposalThreshold) returns (uint256) {
_storeProposal(_msgSender(), targets, values, new string[](calldatas.length), calldatas, description);
return super.propose(targets, values, calldatas, description);
}

/**
Expand All @@ -83,14 +76,8 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp
bytes[] memory calldatas,
string memory description
) public virtual override returns (uint256) {
require(
getVotes(msg.sender, block.number - 1) >= proposalThreshold(),
"GovernorCompatibilityBravo: proposer votes below proposal threshold"
);

uint256 proposalId = super.propose(targets, values, _encodeCalldata(signatures, calldatas), description);
_storeProposal(proposalId, _msgSender(), targets, values, signatures, calldatas, description);
return proposalId;
_storeProposal(_msgSender(), targets, values, signatures, calldatas, description);
return propose(targets, values, _encodeCalldata(signatures, calldatas), description);
}

/**
Expand Down Expand Up @@ -119,6 +106,22 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp
);
}

function cancel(uint256 proposalId) public virtual override {
ProposalDetails storage details = _proposalDetails[proposalId];

require(
_msgSender() == details.proposer || getVotes(details.proposer, block.number - 1) < proposalThreshold(),
"GovernorBravo: proposer above threshold"
);

_cancel(
details.targets,
details.values,
_encodeCalldata(details.signatures, details.calldatas),
details.descriptionHash
);
}

/**
* @dev Encodes calldatas with optional function signature.
*/
Expand All @@ -132,7 +135,7 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp
for (uint256 i = 0; i < signatures.length; ++i) {
fullcalldatas[i] = bytes(signatures[i]).length == 0
? calldatas[i]
: abi.encodePacked(bytes4(keccak256(bytes(signatures[i]))), calldatas[i]);
: abi.encodeWithSignature(signatures[i], calldatas[i]);
}

return fullcalldatas;
Expand All @@ -142,25 +145,38 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp
* @dev Store proposal metadata for later lookup
*/
function _storeProposal(
uint256 proposalId,
address proposer,
address[] memory targets,
uint256[] memory values,
string[] memory signatures,
bytes[] memory calldatas,
string memory description
) private {
ProposalDetails storage details = _proposalDetails[proposalId];
bytes32 descriptionHash = keccak256(bytes(description));
uint256 proposalId = hashProposal(targets, values, _encodeCalldata(signatures, calldatas), descriptionHash);

details.proposer = proposer;
details.targets = targets;
details.values = values;
details.signatures = signatures;
details.calldatas = calldatas;
details.descriptionHash = keccak256(bytes(description));
ProposalDetails storage details = _proposalDetails[proposalId];
if (details.descriptionHash == bytes32(0)) {
details.proposer = proposer;
details.targets = targets;
details.values = values;
details.signatures = signatures;
details.calldatas = calldatas;
details.descriptionHash = descriptionHash;
}
}

// ==================================================== Views =====================================================
/**
* @dev Part of the Governor Bravo's interface: _"The number of votes required in order for a voter to become a proposer"_.
*/
function proposalThreshold()
public
view
virtual
override(IGovernorCompatibilityBravo, GovernorProposalThreshold)
returns (uint256);

/**
* @dev See {IGovernorCompatibilityBravo-proposals}.
*/
Expand Down
Loading

0 comments on commit 4b152bd

Please sign in to comment.