diff --git a/.changeset/silly-bees-beam.md b/.changeset/silly-bees-beam.md index 6be23768c..0f4f40507 100644 --- a/.changeset/silly-bees-beam.md +++ b/.changeset/silly-bees-beam.md @@ -2,4 +2,6 @@ 'openzeppelin-solidity': major --- -`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) +`ERC20Votes`: Changed internal vote accounting to reusable `Votes` module previously used by `ERC721Votes`. Removed implicit `ERC20Permit` inheritance. Note that the `DOMAIN_SEPARATOR` getter was previously guaranteed to be available for `ERC20Votes` contracts, but is no longer available unless `ERC20Permit` is explicitly used; ERC-5267 support is included in `ERC20Votes` with `EIP712` and is recommended as an alternative. + +pr: #3816 diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 7cf7e6917..122d39564 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -13,6 +13,9 @@ concurrency: group: checks-${{ github.ref }} cancel-in-progress: true +env: + NODE_OPTIONS: --max_old_space_size=5120 + jobs: lint: runs-on: ubuntu-latest @@ -26,7 +29,6 @@ jobs: runs-on: ubuntu-latest env: FORCE_COLOR: 1 - NODE_OPTIONS: --max_old_space_size=4096 GAS: true steps: - uses: actions/checkout@v3 @@ -57,8 +59,6 @@ jobs: run: bash scripts/upgradeable/transpile.sh - name: Run tests run: npm run test - env: - NODE_OPTIONS: --max_old_space_size=4096 - name: Check linearisation of the inheritance graph run: npm run test:inheritance - name: Check storage layout @@ -86,8 +86,6 @@ jobs: - name: Set up environment uses: ./.github/actions/setup - run: npm run coverage - env: - NODE_OPTIONS: --max_old_space_size=4096 - uses: codecov/codecov-action@v3 with: token: ${{ secrets.CODECOV_TOKEN }} diff --git a/contracts/governance/utils/Votes.sol b/contracts/governance/utils/Votes.sol index 5c855b5e4..94f6d4fb9 100644 --- a/contracts/governance/utils/Votes.sol +++ b/contracts/governance/utils/Votes.sol @@ -225,14 +225,6 @@ abstract contract Votes is Context, EIP712, Nonces, IERC5805 { return a - b; } - /** - * @dev Returns the contract's {EIP712} domain separator. - */ - // solhint-disable-next-line func-name-mixedcase - function DOMAIN_SEPARATOR() external view returns (bytes32) { - return _domainSeparatorV4(); - } - /** * @dev Must return the voting units held by an account. */ diff --git a/contracts/mocks/docs/governance/MyGovernor.sol b/contracts/mocks/docs/governance/MyGovernor.sol new file mode 100644 index 000000000..095523b3d --- /dev/null +++ b/contracts/mocks/docs/governance/MyGovernor.sol @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.19; + +import "../../../governance/Governor.sol"; +import "../../../governance/compatibility/GovernorCompatibilityBravo.sol"; +import "../../../governance/extensions/GovernorVotes.sol"; +import "../../../governance/extensions/GovernorVotesQuorumFraction.sol"; +import "../../../governance/extensions/GovernorTimelockControl.sol"; + +contract MyGovernor is + Governor, + GovernorCompatibilityBravo, + GovernorVotes, + GovernorVotesQuorumFraction, + GovernorTimelockControl +{ + constructor( + IVotes _token, + TimelockController _timelock + ) Governor("MyGovernor") GovernorVotes(_token) GovernorVotesQuorumFraction(4) GovernorTimelockControl(_timelock) {} + + function votingDelay() public pure override returns (uint256) { + return 7200; // 1 day + } + + function votingPeriod() public pure override returns (uint256) { + return 50400; // 1 week + } + + function proposalThreshold() public pure override returns (uint256) { + return 0; + } + + // The functions below are overrides required by Solidity. + + function state( + uint256 proposalId + ) public view override(Governor, IGovernor, GovernorTimelockControl) returns (ProposalState) { + return super.state(proposalId); + } + + function propose( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + string memory description + ) public override(Governor, GovernorCompatibilityBravo, IGovernor) returns (uint256) { + return super.propose(targets, values, calldatas, description); + } + + function cancel( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) public override(Governor, GovernorCompatibilityBravo, IGovernor) returns (uint256) { + return super.cancel(targets, values, calldatas, descriptionHash); + } + + function _execute( + uint256 proposalId, + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) internal override(Governor, GovernorTimelockControl) { + super._execute(proposalId, targets, values, calldatas, descriptionHash); + } + + function _cancel( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) internal override(Governor, GovernorTimelockControl) returns (uint256) { + return super._cancel(targets, values, calldatas, descriptionHash); + } + + function _executor() internal view override(Governor, GovernorTimelockControl) returns (address) { + return super._executor(); + } + + function supportsInterface( + bytes4 interfaceId + ) public view override(Governor, IERC165, GovernorTimelockControl) returns (bool) { + return super.supportsInterface(interfaceId); + } +} diff --git a/contracts/mocks/docs/governance/MyToken.sol b/contracts/mocks/docs/governance/MyToken.sol new file mode 100644 index 000000000..f7707ff9d --- /dev/null +++ b/contracts/mocks/docs/governance/MyToken.sol @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.19; + +import "../../../token/ERC20/ERC20.sol"; +import "../../../token/ERC20/extensions/ERC20Permit.sol"; +import "../../../token/ERC20/extensions/ERC20Votes.sol"; + +contract MyToken is ERC20, ERC20Permit, ERC20Votes { + constructor() ERC20("MyToken", "MTK") ERC20Permit("MyToken") {} + + // The functions below are overrides required by Solidity. + + function _update(address from, address to, uint256 amount) internal override(ERC20, ERC20Votes) { + super._update(from, to, amount); + } + + function nonces(address owner) public view virtual override(ERC20Permit, Nonces) returns (uint256) { + return super.nonces(owner); + } +} diff --git a/contracts/mocks/docs/governance/MyTokenTimestampBased.sol b/contracts/mocks/docs/governance/MyTokenTimestampBased.sol new file mode 100644 index 000000000..1c688c250 --- /dev/null +++ b/contracts/mocks/docs/governance/MyTokenTimestampBased.sol @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.19; + +import "../../../token/ERC20/ERC20.sol"; +import "../../../token/ERC20/extensions/ERC20Permit.sol"; +import "../../../token/ERC20/extensions/ERC20Votes.sol"; + +contract MyTokenTimestampBased is ERC20, ERC20Permit, ERC20Votes { + constructor() ERC20("MyTokenTimestampBased", "MTK") ERC20Permit("MyTokenTimestampBased") {} + + // Overrides IERC6372 functions to make the token & governor timestamp-based + + function clock() public view override returns (uint48) { + return uint48(block.timestamp); + } + + // solhint-disable-next-line func-name-mixedcase + function CLOCK_MODE() public pure override returns (string memory) { + return "mode=timestamp"; + } + + // The functions below are overrides required by Solidity. + + function _update(address from, address to, uint256 amount) internal override(ERC20, ERC20Votes) { + super._update(from, to, amount); + } + + function nonces(address owner) public view virtual override(ERC20Permit, Nonces) returns (uint256) { + return super.nonces(owner); + } +} diff --git a/contracts/mocks/docs/governance/MyTokenWrapped.sol b/contracts/mocks/docs/governance/MyTokenWrapped.sol new file mode 100644 index 000000000..6637dccc3 --- /dev/null +++ b/contracts/mocks/docs/governance/MyTokenWrapped.sol @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.19; + +import "../../../token/ERC20/ERC20.sol"; +import "../../../token/ERC20/extensions/ERC20Permit.sol"; +import "../../../token/ERC20/extensions/ERC20Votes.sol"; +import "../../../token/ERC20/extensions/ERC20Wrapper.sol"; + +contract MyTokenWrapped is ERC20, ERC20Permit, ERC20Votes, ERC20Wrapper { + constructor( + IERC20 wrappedToken + ) ERC20("MyTokenWrapped", "MTK") ERC20Permit("MyTokenWrapped") ERC20Wrapper(wrappedToken) {} + + // The functions below are overrides required by Solidity. + + function decimals() public view override(ERC20, ERC20Wrapper) returns (uint8) { + return super.decimals(); + } + + function _update(address from, address to, uint256 amount) internal override(ERC20, ERC20Votes) { + super._update(from, to, amount); + } + + function nonces(address owner) public view virtual override(ERC20Permit, Nonces) returns (uint256) { + return super.nonces(owner); + } +} diff --git a/contracts/token/ERC20/extensions/ERC20Permit.sol b/contracts/token/ERC20/extensions/ERC20Permit.sol index dbdc37053..e7d43c2dd 100644 --- a/contracts/token/ERC20/extensions/ERC20Permit.sol +++ b/contracts/token/ERC20/extensions/ERC20Permit.sol @@ -66,7 +66,7 @@ abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712, Nonces { * @dev See {IERC20Permit-DOMAIN_SEPARATOR}. */ // solhint-disable-next-line func-name-mixedcase - function DOMAIN_SEPARATOR() external view override returns (bytes32) { + function DOMAIN_SEPARATOR() external view virtual override returns (bytes32) { return _domainSeparatorV4(); } } diff --git a/docs/modules/ROOT/pages/governance.adoc b/docs/modules/ROOT/pages/governance.adoc index a40275b07..3d2bc05f3 100644 --- a/docs/modules/ROOT/pages/governance.adoc +++ b/docs/modules/ROOT/pages/governance.adoc @@ -43,82 +43,13 @@ In the rest of this guide, we will focus on a fresh deploy of the vanilla OpenZe The voting power of each account in our governance setup will be determined by an ERC20 token. The token has to implement the ERC20Votes extension. This extension will keep track of historical balances so that voting power is retrieved from past snapshots rather than current balance, which is an important protection that prevents double voting. ```solidity -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.19; - -import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; -import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Permit.sol"; -import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Votes.sol"; - -contract MyToken is ERC20, ERC20Permit, ERC20Votes { - constructor() ERC20("MyToken", "MTK") ERC20Permit("MyToken") {} - - // The functions below are overrides required by Solidity. - - function _afterTokenTransfer(address from, address to, uint256 amount) - internal - override(ERC20, ERC20Votes) - { - super._afterTokenTransfer(from, to, amount); - } - - function _mint(address to, uint256 amount) - internal - override(ERC20, ERC20Votes) - { - super._mint(to, amount); - } - - function _burn(address account, uint256 amount) - internal - override(ERC20, ERC20Votes) - { - super._burn(account, amount); - } -} +include::api:example$governance/MyToken.sol[] ``` If your project already has a live token that does not include ERC20Votes and is not upgradeable, you can wrap it in a governance token by using ERC20Wrapper. This will allow token holders to participate in governance by wrapping their tokens 1-to-1. ```solidity -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.19; - -import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; -import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Permit.sol"; -import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Votes.sol"; -import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Wrapper.sol"; - -contract MyToken is ERC20, ERC20Permit, ERC20Votes, ERC20Wrapper { - constructor(IERC20 wrappedToken) - ERC20("MyToken", "MTK") - ERC20Permit("MyToken") - ERC20Wrapper(wrappedToken) - {} - - // The functions below are overrides required by Solidity. - - function _afterTokenTransfer(address from, address to, uint256 amount) - internal - override(ERC20, ERC20Votes) - { - super._afterTokenTransfer(from, to, amount); - } - - function _mint(address to, uint256 amount) - internal - override(ERC20, ERC20Votes) - { - super._mint(to, amount); - } - - function _burn(address account, uint256 amount) - internal - override(ERC20, ERC20Votes) - { - super._burn(account, amount); - } -} +include::api:example$governance/MyTokenWrapped.sol[] ``` NOTE: The only other source of voting power available in OpenZeppelin Contracts currently is xref:api:token/ERC721.adoc#ERC721Votes[`ERC721Votes`]. ERC721 tokens that don't provide this functionality can be wrapped into a voting tokens using a combination of xref:api:token/ERC721.adoc#ERC721Votes[`ERC721Votes`] and xref:api:token/ERC721Wrapper.adoc#ERC721Wrapper[`ERC721Wrapper`]. @@ -146,87 +77,7 @@ These parameters are specified in the unit defined in the token's clock. Assumin We can optionally set a proposal threshold as well. This restricts proposal creation to accounts who have enough voting power. ```solidity -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.19; - -import "@openzeppelin/contracts/governance/Governor.sol"; -import "@openzeppelin/contracts/governance/compatibility/GovernorCompatibilityBravo.sol"; -import "@openzeppelin/contracts/governance/extensions/GovernorVotes.sol"; -import "@openzeppelin/contracts/governance/extensions/GovernorVotesQuorumFraction.sol"; -import "@openzeppelin/contracts/governance/extensions/GovernorTimelockControl.sol"; - -contract MyGovernor is Governor, GovernorCompatibilityBravo, GovernorVotes, GovernorVotesQuorumFraction, GovernorTimelockControl { - constructor(IVotes _token, TimelockController _timelock) - Governor("MyGovernor") - GovernorVotes(_token) - GovernorVotesQuorumFraction(4) - GovernorTimelockControl(_timelock) - {} - - function votingDelay() public pure override returns (uint256) { - return 7200; // 1 day - } - - function votingPeriod() public pure override returns (uint256) { - return 50400; // 1 week - } - - function proposalThreshold() public pure override returns (uint256) { - return 0; - } - - // The functions below are overrides required by Solidity. - - function state(uint256 proposalId) - public - view - override(Governor, IGovernor, GovernorTimelockControl) - returns (ProposalState) - { - return super.state(proposalId); - } - - function propose(address[] memory targets, uint256[] memory values, bytes[] memory calldatas, string memory description) - public - override(Governor, GovernorCompatibilityBravo, IGovernor) - returns (uint256) - { - return super.propose(targets, values, calldatas, description); - } - - function _execute(uint256 proposalId, address[] memory targets, uint256[] memory values, bytes[] memory calldatas, bytes32 descriptionHash) - internal - override(Governor, GovernorTimelockControl) - { - super._execute(proposalId, targets, values, calldatas, descriptionHash); - } - - function _cancel(address[] memory targets, uint256[] memory values, bytes[] memory calldatas, bytes32 descriptionHash) - internal - override(Governor, GovernorTimelockControl) - returns (uint256) - { - return super._cancel(targets, values, calldatas, descriptionHash); - } - - function _executor() - internal - view - override(Governor, GovernorTimelockControl) - returns (address) - { - return super._executor(); - } - - function supportsInterface(bytes4 interfaceId) - public - view - override(Governor, IERC165, GovernorTimelockControl) - returns (bool) - { - return super.supportsInterface(interfaceId); - } -} +include::api:example$governance/MyGovernor.sol[] ``` === Timelock @@ -338,49 +189,7 @@ Therefore, designing a timestamp based voting system starts with the token. Since v4.9, all voting contracts (including xref:api:token/ERC20.adoc#ERC20Votes[`ERC20Votes`] and xref:api:token/ERC721.adoc#ERC721Votes[`ERC721Votes`]) rely on xref:api:interfaces.adoc#IERC6372[IERC6372] for clock management. In order to change from operating with block numbers to operating with timestamps, all that is required is to override the `clock()` and `CLOCK_MODE()` functions. ```solidity -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.19; - -import "github.com/openzeppelin/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol"; -import "github.com/openzeppelin/openzeppelin-contracts/contracts/token/ERC20/extensions/ERC20Permit.sol"; -import "github.com/openzeppelin/openzeppelin-contracts/contracts/token/ERC20/extensions/ERC20Votes.sol"; - -contract MyToken is ERC20, ERC20Permit, ERC20Votes { - constructor() ERC20("MyToken", "MTK") ERC20Permit("MyToken") {} - - // Overrides IERC6372 functions to make the token & governor timestamp-based - - function clock() public view override returns (uint48) { - return uint48(block.timestamp); - } - - function CLOCK_MODE() public pure override returns (string memory) { - return "mode=timestamp"; - } - - // The functions below are overrides required by Solidity. - - function _afterTokenTransfer(address from, address to, uint256 amount) - internal - override(ERC20, ERC20Votes) - { - super._afterTokenTransfer(from, to, amount); - } - - function _mint(address to, uint256 amount) - internal - override(ERC20, ERC20Votes) - { - super._mint(to, amount); - } - - function _burn(address account, uint256 amount) - internal - override(ERC20, ERC20Votes) - { - super._burn(account, amount); - } -} +include::api:example$governance/MyTokenTimestampBased.sol[] ``` === Governor diff --git a/scripts/prepare-docs.sh b/scripts/prepare-docs.sh index bb9c5d0ad..d1317b092 100755 --- a/scripts/prepare-docs.sh +++ b/scripts/prepare-docs.sh @@ -1,6 +1,7 @@ #!/usr/bin/env bash set -euo pipefail +shopt -s globstar OUTDIR="$(node -p 'require("./docs/config.js").outputDir')" @@ -13,11 +14,13 @@ rm -rf "$OUTDIR" hardhat docgen # copy examples and adjust imports -examples_dir="docs/modules/api/examples" -mkdir -p "$examples_dir" -for f in contracts/mocks/docs/*.sol; do - name="$(basename "$f")" - sed -e '/^import/s|\.\./\.\./|@openzeppelin/contracts/|' "$f" > "docs/modules/api/examples/$name" +examples_source_dir="contracts/mocks/docs" +examples_target_dir="docs/modules/api/examples" + +for f in "$examples_source_dir"/**/*.sol; do + name="${f/#"$examples_source_dir/"/}" + mkdir -p "$examples_target_dir/$(dirname "$name")" + sed -Ee '/^import/s|"(\.\./)+|"@openzeppelin/contracts/|' "$f" > "$examples_target_dir/$name" done node scripts/gen-nav.js "$OUTDIR" > "$OUTDIR/../nav.adoc" diff --git a/test/governance/utils/Votes.behavior.js b/test/governance/utils/Votes.behavior.js index 02f2c2544..37062e19c 100644 --- a/test/governance/utils/Votes.behavior.js +++ b/test/governance/utils/Votes.behavior.js @@ -7,7 +7,7 @@ const ethSigUtil = require('eth-sig-util'); const Wallet = require('ethereumjs-wallet').default; const { shouldBehaveLikeEIP6372 } = require('./EIP6372.behavior'); -const { getDomain, domainType, domainSeparator } = require('../../helpers/eip712'); +const { getDomain, domainType } = require('../../helpers/eip712'); const { clockFromReceipt } = require('../../helpers/time'); const Delegation = [ @@ -36,10 +36,6 @@ function shouldBehaveLikeVotes(accounts, tokens, { mode = 'blocknumber', fungibl expect(await this.votes.nonces(accounts[0])).to.be.bignumber.equal('0'); }); - it('domain separator', async function () { - expect(await this.votes.DOMAIN_SEPARATOR()).to.equal(domainSeparator(await getDomain(this.votes))); - }); - describe('delegation with signature', function () { const token = tokens[0]; diff --git a/test/token/ERC20/extensions/ERC20Votes.test.js b/test/token/ERC20/extensions/ERC20Votes.test.js index 621f58b7d..e4ff58cd9 100644 --- a/test/token/ERC20/extensions/ERC20Votes.test.js +++ b/test/token/ERC20/extensions/ERC20Votes.test.js @@ -46,10 +46,6 @@ contract('ERC20Votes', function (accounts) { expect(await this.token.nonces(holder)).to.be.bignumber.equal('0'); }); - it('domain separator', async function () { - expect(await this.token.DOMAIN_SEPARATOR()).to.equal(await getDomain(this.token).then(domainSeparator)); - }); - it('minting restriction', async function () { const amount = new BN('2').pow(new BN('224')); await expectRevert(this.token.$_mint(holder, amount), 'ERC20Votes: total supply risks overflowing votes');