Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make ERC20Votes independent from ERC20Permit #3816

Merged
merged 24 commits into from
Nov 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f28ede6
Add Nonces contract
JulissaDantes Nov 14, 2022
1931d64
Remove permit from erc20votes
JulissaDantes Nov 15, 2022
cc25b3f
Update ERC20Permit contract
JulissaDantes Nov 15, 2022
39bd173
Update Changelog
JulissaDantes Nov 15, 2022
5df3792
Add Nonces dcumentation
JulissaDantes Nov 15, 2022
b9c425f
Update changelog with PR number
JulissaDantes Nov 16, 2022
485a322
Use Votes for ERC20Votes
JulissaDantes Nov 17, 2022
f40bf1c
Remove duplicate tests
JulissaDantes Nov 21, 2022
1c07a94
Fix Votes tests
JulissaDantes Nov 21, 2022
f4baabe
update changelog
JulissaDantes Nov 22, 2022
a7ec549
Update CHANGELOG.md
JulissaDantes Nov 25, 2022
8fab231
Merge branch 'next-v5.0' into refactor/erc20-votes
JulissaDantes Nov 25, 2022
5202e0d
Add test case
JulissaDantes Nov 25, 2022
0a2a857
Update test
JulissaDantes Nov 25, 2022
c140d72
Merge branch 'refactor/erc20-votes' of https://github.com/JulissaDant…
JulissaDantes Nov 25, 2022
cffa45c
Update tests
JulissaDantes Nov 25, 2022
f881aad
Merge branch 'next-v5.0' into refactor/erc20-votes
JulissaDantes Nov 25, 2022
390f04a
Add numCheckpoints
JulissaDantes Nov 28, 2022
b41574c
Add tests to ERC20VotesComp
JulissaDantes Nov 28, 2022
4cda6a0
Update template
JulissaDantes Nov 28, 2022
c2a9537
Update contracts/token/ERC20/extensions/ERC20Votes.sol
JulissaDantes Nov 29, 2022
6adfc96
Update contracts/token/ERC20/extensions/ERC20Votes.sol
JulissaDantes Nov 29, 2022
e31342b
Update contracts/utils/Checkpoints.sol
JulissaDantes Nov 29, 2022
3f001ad
update template
JulissaDantes Nov 29, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
## Unreleased (Breaking)

* `TimelockController`: Changed the role architecture to use `DEFAULT_ADMIN_ROLE` as the admin for all roles, instead of the bespoke `TIMELOCK_ADMIN_ROLE` that was used previously. This aligns with the general recommendation for `AccessControl` and makes the addition of new roles easier. Accordingly, the `admin` parameter and timelock will now be granted `DEFAULT_ADMIN_ROLE` instead of `TIMELOCK_ADMIN_ROLE`. ([#3799](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3799))
* `ERC20Votes`: Changed internal vote accounting to reusable `Votes` module previously used by `ERC721Votes`. Removed implicit `ERC20Permit` inheritance. [#3816](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3816)
* `Nonces`: Added a new contract to keep track of user nonces. Used for signatures in `ERC20Permit`, `ERC20Votes`, and `ERC721Votes`. ([#3816](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3816))
* Removed presets in favor of [OpenZeppelin Contracts Wizard](https://wizard.openzeppelin.com/).
* `TransparentUpgradeableProxy`: Removed `admin` and `implementation` getters, which were only callable by the proxy owner and thus not very useful. ([#3820](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3820))
* `ProxyAdmin`: Removed `getProxyAdmin` and `getProxyImplementation` getters. ([#3820](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3820))
Expand Down
40 changes: 17 additions & 23 deletions contracts/governance/utils/Votes.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@
pragma solidity ^0.8.0;

import "../../utils/Context.sol";
import "../../utils/Counters.sol";
import "../../utils/Nonces.sol";
import "../../utils/Checkpoints.sol";
import "../../utils/cryptography/EIP712.sol";
import "./IVotes.sol";
import "../../utils/math/SafeCast.sol";

/**
* @dev This is a base abstract contract that tracks voting units, which are a measure of voting power that can be
Expand All @@ -28,9 +29,8 @@ import "./IVotes.sol";
*
* _Available since v4.5._
*/
abstract contract Votes is IVotes, Context, EIP712 {
abstract contract Votes is IVotes, Context, EIP712, Nonces {
using Checkpoints for Checkpoints.History;
using Counters for Counters.Counter;

bytes32 private constant _DELEGATION_TYPEHASH =
keccak256("Delegation(address delegatee,uint256 nonce,uint256 expiry)");
Expand All @@ -39,8 +39,6 @@ abstract contract Votes is IVotes, Context, EIP712 {
mapping(address => Checkpoints.History) private _delegateCheckpoints;
Checkpoints.History private _totalCheckpoints;

mapping(address => Counters.Counter) private _nonces;

/**
* @dev Returns the current amount of votes that `account` has.
*/
Expand Down Expand Up @@ -170,30 +168,26 @@ abstract contract Votes is IVotes, Context, EIP712 {
}
}

function _add(uint256 a, uint256 b) private pure returns (uint256) {
return a + b;
}

function _subtract(uint256 a, uint256 b) private pure returns (uint256) {
return a - b;
}

/**
* @dev Consumes a nonce.
*
* Returns the current value and increments nonce.
* @dev Get number of checkpoints for `account`.
*/
function _useNonce(address owner) internal virtual returns (uint256 current) {
Counters.Counter storage nonce = _nonces[owner];
current = nonce.current();
nonce.increment();
function _numCheckpoints(address account) internal view virtual returns (uint32) {
return SafeCast.toUint32(_delegateCheckpoints[account].length());
}

/**
* @dev Returns an address nonce.
* @dev Get the `pos`-th checkpoint for `account`.
*/
function nonces(address owner) public view virtual returns (uint256) {
return _nonces[owner].current();
function _checkpoints(address account, uint32 pos) internal view virtual returns (Checkpoints.Checkpoint memory) {
return _delegateCheckpoints[account].getAtPosition(pos);
}

function _add(uint256 a, uint256 b) private pure returns (uint256) {
return a + b;
}

function _subtract(uint256 a, uint256 b) private pure returns (uint256) {
return a - b;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion contracts/mocks/ERC20VotesCompMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pragma solidity ^0.8.0;
import "../token/ERC20/extensions/ERC20VotesComp.sol";

contract ERC20VotesCompMock is ERC20VotesComp {
constructor(string memory name, string memory symbol) ERC20(name, symbol) ERC20Permit(name) {}
constructor(string memory name, string memory symbol) ERC20(name, symbol) EIP712(name, "1") {}

function mint(address account, uint256 amount) public {
_mint(account, amount);
Expand Down
2 changes: 1 addition & 1 deletion contracts/mocks/ERC20VotesMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pragma solidity ^0.8.0;
import "../token/ERC20/extensions/ERC20Votes.sol";

contract ERC20VotesMock is ERC20Votes {
constructor(string memory name, string memory symbol) ERC20(name, symbol) ERC20Permit(name) {}
constructor(string memory name, string memory symbol) ERC20(name, symbol) EIP712(name, "1") {}

function mint(address account, uint256 amount) public {
_mint(account, amount);
Expand Down
2 changes: 1 addition & 1 deletion contracts/mocks/ERC721VotesMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ contract ERC721VotesMock is ERC721Votes {
_mint(account, tokenId);
}

function burn(uint256 tokenId) public {
function burn(address, uint256 tokenId) public {
_burn(tokenId);
}

Expand Down
11 changes: 11 additions & 0 deletions contracts/mocks/NoncesImpl.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import "../utils/Nonces.sol";

contract NoncesImpl is Nonces {
function useNonce(address owner) public {
super._useNonce(owner);
}
}
2 changes: 1 addition & 1 deletion contracts/mocks/VotesMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ contract VotesMock is Votes {
_transferVotingUnits(address(0), account, 1);
}

function burn(uint256 voteId) external {
function burn(address, uint256 voteId) external {
address owner = _owners[voteId];
_balances[owner] -= 1;
_transferVotingUnits(owner, address(0), 1);
Expand Down
23 changes: 4 additions & 19 deletions contracts/token/ERC20/extensions/ERC20Permit.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import "./IERC20Permit.sol";
import "../ERC20.sol";
import "../../../utils/cryptography/ECDSA.sol";
import "../../../utils/cryptography/EIP712.sol";
import "../../../utils/Counters.sol";
import "../../../utils/Nonces.sol";

/**
* @dev Implementation of the ERC20 Permit extension allowing approvals to be made via signatures, as defined in
Expand All @@ -19,11 +19,7 @@ import "../../../utils/Counters.sol";
*
* _Available since v3.4._
*/
abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712 {
using Counters for Counters.Counter;

mapping(address => Counters.Counter) private _nonces;

abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712, Nonces {
// solhint-disable-next-line var-name-mixedcase
bytes32 private constant _PERMIT_TYPEHASH =
keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)");
Expand Down Expand Up @@ -70,8 +66,8 @@ abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712 {
/**
* @dev See {IERC20Permit-nonces}.
*/
function nonces(address owner) public view virtual override returns (uint256) {
return _nonces[owner].current();
function nonces(address owner) public view virtual override(IERC20Permit, Nonces) returns (uint256) {
return super.nonces(owner);
}
JulissaDantes marked this conversation as resolved.
Show resolved Hide resolved

/**
Expand All @@ -81,15 +77,4 @@ abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712 {
function DOMAIN_SEPARATOR() external view override returns (bytes32) {
return _domainSeparatorV4();
}

/**
* @dev "Consume a nonce": return the current value and increment.
*
* _Available since v4.1._
*/
function _useNonce(address owner) internal virtual returns (uint256 current) {
Counters.Counter storage nonce = _nonces[owner];
current = nonce.current();
nonce.increment();
}
}