From 4a6f2686a72274a3cf59d5572080317b8ff1bf91 Mon Sep 17 00:00:00 2001 From: John Feras Date: Wed, 15 Nov 2023 15:47:02 -0500 Subject: [PATCH 1/4] Added GovernorVotesCompQuorumFraction for settable quorum --- script/DeployInput.sol | 1 + src/RadworksGovernor.sol | 17 +-- src/lib/GovernorVotesCompQuorumFraction.sol | 125 ++++++++++++++++++++ test/Constants.sol | 2 - test/RadworksGovernor.t.sol | 8 +- 5 files changed, 142 insertions(+), 11 deletions(-) create mode 100644 src/lib/GovernorVotesCompQuorumFraction.sol diff --git a/script/DeployInput.sol b/script/DeployInput.sol index 7ee6bcf..218b5d2 100644 --- a/script/DeployInput.sol +++ b/script/DeployInput.sol @@ -5,4 +5,5 @@ contract DeployInput { uint256 constant INITIAL_VOTING_DELAY = 7200; // 24 hours uint256 constant INITIAL_VOTING_PERIOD = 17_280; // matches existing config uint256 constant INITIAL_PROPOSAL_THRESHOLD = 1_000_000e18; // matches existing config + uint256 constant INITIAL_QUORUM_PCT = 4; } diff --git a/src/RadworksGovernor.sol b/src/RadworksGovernor.sol index 46a9bd1..caee1d0 100644 --- a/src/RadworksGovernor.sol +++ b/src/RadworksGovernor.sol @@ -11,7 +11,7 @@ import {IGovernor} from "@openzeppelin/contracts/governance/IGovernor.sol"; import {ICompoundTimelock} from "@openzeppelin/contracts/governance/extensions/GovernorTimelockCompound.sol"; import {GovernorSettings} from "@openzeppelin/contracts/governance/extensions/GovernorSettings.sol"; - +import {GovernorVotesCompQuorumFraction} from "./lib/GovernorVotesCompQuorumFraction.sol"; import { GovernorTimelockCompound, ICompoundTimelock @@ -22,7 +22,7 @@ import {GovernorCompatibilityBravo} from /// @notice The upgraded Radworks Governor: Bravo compatible and extended from OpenZeppelin. contract RadworksGovernor is Governor, - GovernorVotesComp, + GovernorVotesCompQuorumFraction, GovernorTimelockCompound, GovernorSettings { @@ -47,9 +47,6 @@ contract RadworksGovernor is /// @notice Human readable name of this Governor. string private constant GOVERNOR_NAME = "Radworks Governor Bravo"; - /// @notice The number of RAD (in "wei") that must participate in a vote to meet quorum threshold. - uint256 private constant QUORUM = 4_000_000e18; // 4,000,000 RAD - /// @param _initialVotingDelay The initial voting delay this Governor will enforce. /// @param _initialVotingPeriod The initial voting period this Governor will enforce. /// @param _initialProposalThreshold The initial number of RAD required to submit @@ -59,6 +56,7 @@ contract RadworksGovernor is uint256 _initialVotingPeriod, uint256 _initialProposalThreshold ) + GovernorVotesCompQuorumFraction(4) GovernorVotesComp(RAD_TOKEN) GovernorSettings(_initialVotingDelay, _initialVotingPeriod, _initialProposalThreshold) GovernorTimelockCompound(TIMELOCK) @@ -177,8 +175,13 @@ contract RadworksGovernor is /// @notice The amount of RAD required to meet the quorum threshold for a proposal /// as of a given block. /// @dev Our implementation ignores the block number parameter and returns a constant. - function quorum(uint256) public pure override returns (uint256) { - return QUORUM; + function quorum(uint256 timepoint) + public + view + override(IGovernor, GovernorVotesCompQuorumFraction) + returns (uint256) + { + return GovernorVotesCompQuorumFraction.quorum(timepoint); } /// @inheritdoc Governor diff --git a/src/lib/GovernorVotesCompQuorumFraction.sol b/src/lib/GovernorVotesCompQuorumFraction.sol new file mode 100644 index 0000000..e151640 --- /dev/null +++ b/src/lib/GovernorVotesCompQuorumFraction.sol @@ -0,0 +1,125 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts (last updated v4.9.0) +// (governance/extensions/GovernorVotesQuorumFraction.sol) + +pragma solidity ^0.8.0; + +import { + ERC20VotesComp, + GovernorVotesComp +} from "@openzeppelin/contracts/governance/extensions/GovernorVotesComp.sol"; +import "@openzeppelin/contracts/utils/Checkpoints.sol"; +import "@openzeppelin/contracts/utils/math/SafeCast.sol"; + +/** + * @dev Extension of {Governor} for voting weight extraction from an {GovernorVotesComp} token and a + * quorum + * expressed as a fraction of the total supply. + */ +abstract contract GovernorVotesCompQuorumFraction is GovernorVotesComp { + using Checkpoints for Checkpoints.Trace224; + + uint256 private _quorumNumerator; // DEPRECATED in favor of _quorumNumeratorHistory + + /// @custom:oz-retyped-from Checkpoints.History + Checkpoints.Trace224 private _quorumNumeratorHistory; + + event QuorumNumeratorUpdated(uint256 oldQuorumNumerator, uint256 newQuorumNumerator); + + /** + * @dev Initialize quorum as a fraction of the token's total supply. + * + * The fraction is specified as `numerator / denominator`. By default the denominator is 100, so + * quorum is pecified as a percent: a numerator of 10 corresponds to quorum being 10% of total + * supply. + * The denominator can be customized by overriding {quorumDenominator}. + */ + constructor(uint256 quorumNumeratorValue) { + _updateQuorumNumerator(quorumNumeratorValue); + } + + /** + * @dev Returns the current quorum numerator. See {quorumDenominator}. + */ + function quorumNumerator() public view virtual returns (uint256) { + return _quorumNumeratorHistory._checkpoints.length == 0 + ? _quorumNumerator + : _quorumNumeratorHistory.latest(); + } + + /** + * @dev Returns the quorum numerator at a specific timepoint. See {quorumDenominator}. + */ + function quorumNumerator(uint256 timepoint) public view virtual returns (uint256) { + // If history is empty, fallback to old storage + uint256 length = _quorumNumeratorHistory._checkpoints.length; + if (length == 0) return _quorumNumerator; + + // Optimistic search, check the latest checkpoint + Checkpoints.Checkpoint224 memory latest = _quorumNumeratorHistory._checkpoints[length - 1]; + if (latest._key <= timepoint) return latest._value; + + // Otherwise, do the binary search + return _quorumNumeratorHistory.upperLookupRecent(SafeCast.toUint32(timepoint)); + } + + /** + * @dev Returns the quorum denominator. Defaults to 100, but may be overridden. + */ + function quorumDenominator() public view virtual returns (uint256) { + return 100; + } + + /** + * @dev Returns the quorum for a timepoint, in terms of number of votes: + * `supply * numerator / denominator`. + */ + function quorum(uint256 timepoint) public view virtual override returns (uint256) { + return (token.totalSupply() * quorumNumerator(timepoint)) / quorumDenominator(); + } + + /** + * @dev Changes the quorum numerator. + * + * Emits a {QuorumNumeratorUpdated} event. + * + * Requirements: + * + * - Must be called through a governance proposal. + * - New numerator must be smaller or equal to the denominator. + */ + function updateQuorumNumerator(uint256 newQuorumNumerator) external virtual onlyGovernance { + _updateQuorumNumerator(newQuorumNumerator); + } + + /** + * @dev Changes the quorum numerator. + * + * Emits a {QuorumNumeratorUpdated} event. + * + * Requirements: + * + * - New numerator must be smaller or equal to the denominator. + */ + function _updateQuorumNumerator(uint256 newQuorumNumerator) internal virtual { + require( + newQuorumNumerator <= quorumDenominator(), + "GovernorVotesQuorumFraction: quorumNumerator over quorumDenominator" + ); + + uint256 oldQuorumNumerator = quorumNumerator(); + + // Make sure we keep track of the original numerator in contracts upgraded from a version + // without checkpoints. + if (oldQuorumNumerator != 0 && _quorumNumeratorHistory._checkpoints.length == 0) { + _quorumNumeratorHistory._checkpoints.push( + Checkpoints.Checkpoint224({_key: 0, _value: SafeCast.toUint224(oldQuorumNumerator)}) + ); + } + + // Set new quorum for future proposals + _quorumNumeratorHistory.push(SafeCast.toUint32(clock()), SafeCast.toUint224(newQuorumNumerator)); + + emit QuorumNumeratorUpdated(oldQuorumNumerator, newQuorumNumerator); + } +} diff --git a/test/Constants.sol b/test/Constants.sol index 52e3db4..f603430 100644 --- a/test/Constants.sol +++ b/test/Constants.sol @@ -17,6 +17,4 @@ contract Constants { // we have not yet deployed the Radworks Bravo Governor address constant DEPLOYED_BRAVO_GOVERNOR = 0x1111111111111111111111111111111111111111; - - uint256 constant QUORUM = 4_000_000e18; } diff --git a/test/RadworksGovernor.t.sol b/test/RadworksGovernor.t.sol index e9da46e..24a6961 100644 --- a/test/RadworksGovernor.t.sol +++ b/test/RadworksGovernor.t.sol @@ -7,9 +7,10 @@ import {IGovernor} from "@openzeppelin/contracts/governance/IGovernor.sol"; import {IGovernorAlpha} from "src/interfaces/IGovernorAlpha.sol"; import {RadworksGovernorTest} from "test/helpers/RadworksGovernorTest.sol"; import {ProposalTest} from "test/helpers/ProposalTest.sol"; +import "../script/DeployInput.sol"; abstract contract Constructor is RadworksGovernorTest { - function testFuzz_CorrectlySetsAllConstructorArgs(uint256 _blockNumber) public { + function test_CorrectlySetsAllConstructorArgs() public { assertEq(governorBravo.name(), "Radworks Governor Bravo"); assertEq(address(governorBravo.token()), RAD_TOKEN); @@ -21,7 +22,10 @@ abstract contract Constructor is RadworksGovernorTest { assertEq(governorBravo.proposalThreshold(), INITIAL_PROPOSAL_THRESHOLD); - assertEq(governorBravo.quorum(_blockNumber), QUORUM); + assertEq( + governorBravo.quorum(block.number), + (governorBravo.token().totalSupply() * INITIAL_QUORUM_PCT) / 100 + ); assertEq(governorBravo.timelock(), TIMELOCK); assertEq(governorBravo.COUNTING_MODE(), "support=bravo&quorum=bravo"); } From 95003525653fef3820e13439d0b2d9c61144c482 Mon Sep 17 00:00:00 2001 From: John Feras Date: Thu, 16 Nov 2023 10:47:46 -0500 Subject: [PATCH 2/4] Added test for setting quorum numerator --- test/RadworksGovernor.t.sol | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/test/RadworksGovernor.t.sol b/test/RadworksGovernor.t.sol index 24a6961..36905e6 100644 --- a/test/RadworksGovernor.t.sol +++ b/test/RadworksGovernor.t.sol @@ -7,6 +7,7 @@ import {IGovernor} from "@openzeppelin/contracts/governance/IGovernor.sol"; import {IGovernorAlpha} from "src/interfaces/IGovernorAlpha.sol"; import {RadworksGovernorTest} from "test/helpers/RadworksGovernorTest.sol"; import {ProposalTest} from "test/helpers/ProposalTest.sol"; +import {ProposalBuilder} from "test/helpers/Proposal.sol"; import "../script/DeployInput.sol"; abstract contract Constructor is RadworksGovernorTest { @@ -210,7 +211,8 @@ abstract contract Propose is ProposalTest { function testFuzz_NewGovernorCanUpdateSettingsViaSuccessfulProposal( uint256 _newDelay, uint256 _newVotingPeriod, - uint256 _newProposalThreshold + uint256 _newProposalThreshold, + uint256 _newQuorumNumerator ) public { vm.assume(_newDelay != governorBravo.votingDelay()); vm.assume(_newVotingPeriod != governorBravo.votingPeriod()); @@ -220,12 +222,13 @@ abstract contract Propose is ProposalTest { _newDelay = bound(_newDelay, 0, 50_000); // about a week at 1 block per 12s _newVotingPeriod = bound(_newVotingPeriod, 1, 200_000); // about a month _newProposalThreshold = bound(_newProposalThreshold, 0, 42 ether); + _newQuorumNumerator = bound(_newQuorumNumerator, 0, 99); _upgradeToBravoGovernor(); - address[] memory _targets = new address[](3); - uint256[] memory _values = new uint256[](3); - bytes[] memory _calldatas = new bytes[](3); + address[] memory _targets = new address[](4); + uint256[] memory _values = new uint256[](4); + bytes[] memory _calldatas = new bytes[](4); string memory _description = "Update governance settings"; _targets[0] = address(governorBravo); @@ -238,6 +241,10 @@ abstract contract Propose is ProposalTest { _calldatas[2] = _buildProposalData("setProposalThreshold(uint256)", abi.encode(_newProposalThreshold)); + _targets[3] = address(governorBravo); + _calldatas[3] = + _buildProposalData("updateQuorumNumerator(uint256)", abi.encode(_newQuorumNumerator)); + // Submit the new proposal vm.prank(PROPOSER); uint256 _newProposalId = governorBravo.propose(_targets, _values, _calldatas, _description); @@ -275,6 +282,11 @@ abstract contract Propose is ProposalTest { assertEq(governorBravo.votingDelay(), _newDelay); assertEq(governorBravo.votingPeriod(), _newVotingPeriod); assertEq(governorBravo.proposalThreshold(), _newProposalThreshold); + assertEq(governorBravo.quorumNumerator(block.number), _newQuorumNumerator); + assertEq( + governorBravo.quorum(block.number), + (governorBravo.token().totalSupply() * _newQuorumNumerator) / 100 + ); } function testFuzz_NewGovernorCanPassMixedProposal( From f21155d6db3bbcf4238e05e346290cb7a509d305 Mon Sep 17 00:00:00 2001 From: John Feras Date: Thu, 7 Dec 2023 14:01:15 -0500 Subject: [PATCH 3/4] Added comment in GovernorVotesCompQuorumFraction.sol --- src/lib/GovernorVotesCompQuorumFraction.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/GovernorVotesCompQuorumFraction.sol b/src/lib/GovernorVotesCompQuorumFraction.sol index e151640..2fd650c 100644 --- a/src/lib/GovernorVotesCompQuorumFraction.sol +++ b/src/lib/GovernorVotesCompQuorumFraction.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT -// OpenZeppelin Contracts (last updated v4.9.0) -// (governance/extensions/GovernorVotesQuorumFraction.sol) +// (Based on OpenZeppelin governance/extensions/GovernorVotesQuorumFraction.sol) pragma solidity ^0.8.0; @@ -75,6 +74,7 @@ abstract contract GovernorVotesCompQuorumFraction is GovernorVotesComp { * `supply * numerator / denominator`. */ function quorum(uint256 timepoint) public view virtual override returns (uint256) { + // @dev: 'token.totalSupply()` ignores timepoint, different from 'GovernorVotesQuorumFraction' return (token.totalSupply() * quorumNumerator(timepoint)) / quorumDenominator(); } From 34b7f0174bbde5827eecc7f2b69fe9b066795826 Mon Sep 17 00:00:00 2001 From: John Feras Date: Wed, 13 Dec 2023 14:56:51 -0500 Subject: [PATCH 4/4] Corrected comments and typos from review --- src/lib/GovernorVotesCompQuorumFraction.sol | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/lib/GovernorVotesCompQuorumFraction.sol b/src/lib/GovernorVotesCompQuorumFraction.sol index 2fd650c..3c52634 100644 --- a/src/lib/GovernorVotesCompQuorumFraction.sol +++ b/src/lib/GovernorVotesCompQuorumFraction.sol @@ -1,5 +1,6 @@ // SPDX-License-Identifier: MIT -// (Based on OpenZeppelin governance/extensions/GovernorVotesQuorumFraction.sol) +// Based on OpenZeppelin GovernorVotesQuorumFraction.sol found at: +// https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/governance/extensions/GovernorVotesQuorumFraction.sol pragma solidity ^0.8.0; @@ -12,8 +13,8 @@ import "@openzeppelin/contracts/utils/math/SafeCast.sol"; /** * @dev Extension of {Governor} for voting weight extraction from an {GovernorVotesComp} token and a - * quorum - * expressed as a fraction of the total supply. + * quorum expressed as a fraction of the total supply. + * Original contract referenced above inherited from {GovernorVotes} */ abstract contract GovernorVotesCompQuorumFraction is GovernorVotesComp { using Checkpoints for Checkpoints.Trace224; @@ -104,7 +105,7 @@ abstract contract GovernorVotesCompQuorumFraction is GovernorVotesComp { function _updateQuorumNumerator(uint256 newQuorumNumerator) internal virtual { require( newQuorumNumerator <= quorumDenominator(), - "GovernorVotesQuorumFraction: quorumNumerator over quorumDenominator" + "GovernorVotesCompQuorumFraction: quorumNumerator over quorumDenominator" ); uint256 oldQuorumNumerator = quorumNumerator();