Remove DOMAIN_SEPARATOR from Votes and update docs examples (#4297)

Co-authored-by: Qiwei Yang <yangqiwei97@gmail.com>
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>

.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

.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 }}

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.
      */

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);
+    }
+}

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);
+    }
+}

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);
+    }
+}

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);
+    }
+}

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();
     }
 }

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
+include::api:example$governance/MyToken.sol[]
-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);
-    }
-}
 ```
 
 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
+include::api:example$governance/MyTokenWrapped.sol[]
-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);
-    }
-}
 ```
 
 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
+include::api:example$governance/MyGovernor.sol[]
-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);
-    }
-}
 ```
 
 === 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
+include::api:example$governance/MyTokenTimestampBased.sol[]
-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);
-    }
-}
 ```
 
 === Governor

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"
+examples_source_dir="contracts/mocks/docs"
-mkdir -p "$examples_dir"
+examples_target_dir="docs/modules/api/examples"
-for f in contracts/mocks/docs/*.sol; do
+
-  name="$(basename "$f")"
+for f in "$examples_source_dir"/**/*.sol; do
-  sed -e '/^import/s|\.\./\.\./|@openzeppelin/contracts/|' "$f" > "docs/modules/api/examples/$name"
+  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"

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];
 

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');