Keep track of historical quorum values (#3561)

Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
OpenZeppelin · Jul 27, 2022 · 8ea1fc8 · 8ea1fc8
1 parent 0d91b62
commit 8ea1fc8
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 5 deletions.
diff --git a/contracts/governance/extensions/GovernorVotesQuorumFraction.sol b/contracts/governance/extensions/GovernorVotesQuorumFraction.sol
@@ -4,6 +4,8 @@
 pragma solidity ^0.8.0;
 
 import "./GovernorVotes.sol";
+import "../../utils/Checkpoints.sol";
+import "../../utils/math/SafeCast.sol";
 
 /**
  * @dev Extension of {Governor} for voting weight extraction from an {ERC20Votes} token and a quorum expressed as a
@@ -12,7 +14,10 @@ import "./GovernorVotes.sol";
  * _Available since v4.3._
  */
 abstract contract GovernorVotesQuorumFraction is GovernorVotes {
-    uint256 private _quorumNumerator;
+    using Checkpoints for Checkpoints.History;
+
+    uint256 private _quorumNumerator; // DEPRECATED
+    Checkpoints.History private _quorumNumeratorHistory;
 
     event QuorumNumeratorUpdated(uint256 oldQuorumNumerator, uint256 newQuorumNumerator);
 
@@ -31,7 +36,27 @@ abstract contract GovernorVotesQuorumFraction is GovernorVotes {
      * @dev Returns the current quorum numerator. See {quorumDenominator}.
      */
     function quorumNumerator() public view virtual returns (uint256) {
-        return _quorumNumerator;
+        return _quorumNumeratorHistory._checkpoints.length == 0 ? _quorumNumerator : _quorumNumeratorHistory.latest();
+    }
+
+    /**
+     * @dev Returns the quorum numerator at a specific block number. See {quorumDenominator}.
+     */
+    function quorumNumerator(uint256 blockNumber) 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.Checkpoint memory latest = _quorumNumeratorHistory._checkpoints[length - 1];
+        if (latest._blockNumber <= blockNumber) {
+            return latest._value;
+        }
+
+        // Otherwize, do the binary search
+        return _quorumNumeratorHistory.getAtBlock(blockNumber);
     }
 
     /**
@@ -45,7 +70,7 @@ abstract contract GovernorVotesQuorumFraction is GovernorVotes {
      * @dev Returns the quorum for a block number, in terms of number of votes: `supply * numerator / denominator`.
      */
     function quorum(uint256 blockNumber) public view virtual override returns (uint256) {
-        return (token.getPastTotalSupply(blockNumber) * quorumNumerator()) / quorumDenominator();
+        return (token.getPastTotalSupply(blockNumber) * quorumNumerator(blockNumber)) / quorumDenominator();
     }
 
     /**
@@ -77,8 +102,17 @@ abstract contract GovernorVotesQuorumFraction is GovernorVotes {
             "GovernorVotesQuorumFraction: quorumNumerator over quorumDenominator"
         );
 
-        uint256 oldQuorumNumerator = _quorumNumerator;
-        _quorumNumerator = newQuorumNumerator;
+        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.Checkpoint({_blockNumber: 0, _value: SafeCast.toUint224(oldQuorumNumerator)})
+            );
+        }
+
+        // Set new quorum for future proposals
+        _quorumNumeratorHistory.push(newQuorumNumerator);
 
         emit QuorumNumeratorUpdated(oldQuorumNumerator, newQuorumNumerator);
     }

diff --git a/...ions/GovernorWeightQuorumFraction.test.js → ...sions/GovernorVotesQuorumFraction.test.js b/...ions/GovernorWeightQuorumFraction.test.js → ...sions/GovernorVotesQuorumFraction.test.js
@@ -104,6 +104,13 @@ contract('GovernorVotesQuorumFraction', function (accounts) {
 
       expect(await this.mock.quorumNumerator()).to.be.bignumber.equal(newRatio);
       expect(await this.mock.quorumDenominator()).to.be.bignumber.equal('100');
+
+      // it takes one block for the new quorum to take effect
+      expect(await time.latestBlock().then(blockNumber => this.mock.quorum(blockNumber.subn(1))))
+        .to.be.bignumber.equal(tokenSupply.mul(ratio).divn(100));
+
+      await time.advanceBlock();
+
       expect(await time.latestBlock().then(blockNumber => this.mock.quorum(blockNumber.subn(1))))
         .to.be.bignumber.equal(tokenSupply.mul(newRatio).divn(100));
     });