From 5cef83d2c7730fbd523045b551d0efd020ceb17b Mon Sep 17 00:00:00 2001 From: Renan Souza Date: Fri, 2 Jun 2023 11:37:59 -0300 Subject: [PATCH 01/13] Optimize array allocation in ERC1155 (#4196) Co-authored-by: Francisco --- .changeset/serious-books-lie.md | 5 +++++ contracts/token/ERC1155/ERC1155.sol | 30 ++++++++++++++++++----------- 2 files changed, 24 insertions(+), 11 deletions(-) create mode 100644 .changeset/serious-books-lie.md diff --git a/.changeset/serious-books-lie.md b/.changeset/serious-books-lie.md new file mode 100644 index 00000000000..6f0a0a73284 --- /dev/null +++ b/.changeset/serious-books-lie.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': patch +--- + +`ERC1155`: Optimize array allocation. diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index a2d7404bff8..6761edd248b 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/contracts/token/ERC1155/ERC1155.sol @@ -206,8 +206,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { function _safeTransferFrom(address from, address to, uint256 id, uint256 amount, bytes memory data) internal { require(to != address(0), "ERC1155: transfer to the zero address"); require(from != address(0), "ERC1155: transfer from the zero address"); - uint256[] memory ids = _asSingletonArray(id); - uint256[] memory amounts = _asSingletonArray(amount); + (uint256[] memory ids, uint256[] memory amounts) = _asSingletonArrays(id, amount); _update(from, to, ids, amounts, data); } @@ -269,8 +268,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { */ function _mint(address to, uint256 id, uint256 amount, bytes memory data) internal { require(to != address(0), "ERC1155: mint to the zero address"); - uint256[] memory ids = _asSingletonArray(id); - uint256[] memory amounts = _asSingletonArray(amount); + (uint256[] memory ids, uint256[] memory amounts) = _asSingletonArrays(id, amount); _update(address(0), to, ids, amounts, data); } @@ -302,8 +300,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { */ function _burn(address from, uint256 id, uint256 amount) internal { require(from != address(0), "ERC1155: burn from the zero address"); - uint256[] memory ids = _asSingletonArray(id); - uint256[] memory amounts = _asSingletonArray(amount); + (uint256[] memory ids, uint256[] memory amounts) = _asSingletonArrays(id, amount); _update(from, address(0), ids, amounts, ""); } @@ -376,10 +373,21 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { } } - function _asSingletonArray(uint256 element) private pure returns (uint256[] memory) { - uint256[] memory array = new uint256[](1); - array[0] = element; - - return array; + function _asSingletonArrays( + uint256 element1, + uint256 element2 + ) private pure returns (uint256[] memory array1, uint256[] memory array2) { + /// @solidity memory-safe-assembly + assembly { + array1 := mload(0x40) + mstore(array1, 1) + mstore(add(array1, 0x20), element1) + + array2 := add(array1, 0x40) + mstore(array2, 1) + mstore(add(array2, 0x20), element2) + + mstore(0x40, add(array2, 0x40)) + } } } From 3902a410f15a579c91d17a8d7e1e5b24351b6c65 Mon Sep 17 00:00:00 2001 From: Francisco Date: Fri, 2 Jun 2023 16:02:57 +0100 Subject: [PATCH 02/13] Remove DOMAIN_SEPARATOR from Votes and update docs examples (#4297) Co-authored-by: Qiwei Yang Co-authored-by: Hadrien Croubois --- .changeset/silly-bees-beam.md | 4 +- .github/workflows/checks.yml | 8 +- contracts/governance/utils/Votes.sol | 8 - .../mocks/docs/governance/MyGovernor.sol | 88 ++++++++ contracts/mocks/docs/governance/MyToken.sol | 20 ++ .../docs/governance/MyTokenTimestampBased.sol | 31 +++ .../mocks/docs/governance/MyTokenWrapped.sol | 27 +++ .../token/ERC20/extensions/ERC20Permit.sol | 2 +- docs/modules/ROOT/pages/governance.adoc | 199 +----------------- scripts/prepare-docs.sh | 13 +- test/governance/utils/Votes.behavior.js | 6 +- .../token/ERC20/extensions/ERC20Votes.test.js | 4 - 12 files changed, 186 insertions(+), 224 deletions(-) create mode 100644 contracts/mocks/docs/governance/MyGovernor.sol create mode 100644 contracts/mocks/docs/governance/MyToken.sol create mode 100644 contracts/mocks/docs/governance/MyTokenTimestampBased.sol create mode 100644 contracts/mocks/docs/governance/MyTokenWrapped.sol diff --git a/.changeset/silly-bees-beam.md b/.changeset/silly-bees-beam.md index 6be23768c58..0f4f40507b5 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 7cf7e6917b2..122d3956413 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 5c855b5e4fb..94f6d4fb941 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 00000000000..095523b3d59 --- /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 00000000000..f7707ff9d71 --- /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 00000000000..1c688c250e0 --- /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 00000000000..6637dccc3bf --- /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 dbdc37053e6..e7d43c2dd11 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 a40275b07f2..3d2bc05f30a 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 bb9c5d0ad25..d1317b09287 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 02f2c254446..37062e19c6d 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 621f58b7d99..e4ff58cd956 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'); From 2d1da295e6e28b3cd86eccab47c40a2bcbf0f4d1 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 2 Jun 2023 17:14:41 +0200 Subject: [PATCH 03/13] Move some changeset to the "Removals" section of CHANGELOG (#4290) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ernesto García Co-authored-by: Francisco --- .changeset/beige-ducks-flow.md | 5 ----- .changeset/dirty-mangos-sort.md | 5 ----- .changeset/fluffy-gifts-build.md | 5 ----- .changeset/friendly-suits-camp.md | 5 ----- .changeset/hungry-impalas-perform.md | 5 ----- .changeset/selfish-queens-rest.md | 5 ----- .changeset/spicy-ducks-cough.md | 5 ----- .changeset/swift-berries-sort.md | 5 ----- .changeset/tame-geckos-search.md | 5 ----- .changeset/three-weeks-double.md | 5 ----- .changeset/unlucky-snakes-drive.md | 5 ----- CHANGELOG.md | 26 ++++++++++++++++++++++++-- 12 files changed, 24 insertions(+), 57 deletions(-) delete mode 100644 .changeset/beige-ducks-flow.md delete mode 100644 .changeset/dirty-mangos-sort.md delete mode 100644 .changeset/fluffy-gifts-build.md delete mode 100644 .changeset/friendly-suits-camp.md delete mode 100644 .changeset/hungry-impalas-perform.md delete mode 100644 .changeset/selfish-queens-rest.md delete mode 100644 .changeset/spicy-ducks-cough.md delete mode 100644 .changeset/swift-berries-sort.md delete mode 100644 .changeset/tame-geckos-search.md delete mode 100644 .changeset/three-weeks-double.md delete mode 100644 .changeset/unlucky-snakes-drive.md diff --git a/.changeset/beige-ducks-flow.md b/.changeset/beige-ducks-flow.md deleted file mode 100644 index 4edc870b519..00000000000 --- a/.changeset/beige-ducks-flow.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'openzeppelin-solidity': major ---- - -Remove deprecated GovernorProposalThreshold module. diff --git a/.changeset/dirty-mangos-sort.md b/.changeset/dirty-mangos-sort.md deleted file mode 100644 index 6981399c0b6..00000000000 --- a/.changeset/dirty-mangos-sort.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'openzeppelin-solidity': major ---- - -Removed presets in favor of [OpenZeppelin Contracts Wizard](https://wizard.openzeppelin.com/). diff --git a/.changeset/fluffy-gifts-build.md b/.changeset/fluffy-gifts-build.md deleted file mode 100644 index a1ffe8be196..00000000000 --- a/.changeset/fluffy-gifts-build.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'openzeppelin-solidity': major ---- - -Remove ERC1820Implementer. diff --git a/.changeset/friendly-suits-camp.md b/.changeset/friendly-suits-camp.md deleted file mode 100644 index bcf1d7ee4cd..00000000000 --- a/.changeset/friendly-suits-camp.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'openzeppelin-solidity': major ---- - -Remove Checkpoints.History. diff --git a/.changeset/hungry-impalas-perform.md b/.changeset/hungry-impalas-perform.md deleted file mode 100644 index 24901ec867a..00000000000 --- a/.changeset/hungry-impalas-perform.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'openzeppelin-solidity': major ---- - -`ERC165Storage`: Removed this contract in favor of inheritance based approach. ([#3880](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3880)) diff --git a/.changeset/selfish-queens-rest.md b/.changeset/selfish-queens-rest.md deleted file mode 100644 index 739fa135601..00000000000 --- a/.changeset/selfish-queens-rest.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'openzeppelin-solidity': major ---- - -Remove PullPayment and Escrow contracts (Escrow, ConditionalEscrow, RefundEscrow). diff --git a/.changeset/spicy-ducks-cough.md b/.changeset/spicy-ducks-cough.md deleted file mode 100644 index bf7296e4e0c..00000000000 --- a/.changeset/spicy-ducks-cough.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'openzeppelin-solidity': major ---- - -Remove the Timers library. diff --git a/.changeset/swift-berries-sort.md b/.changeset/swift-berries-sort.md deleted file mode 100644 index 9af6cba6442..00000000000 --- a/.changeset/swift-berries-sort.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'openzeppelin-solidity': major ---- - -Remove ERC777 implementation. diff --git a/.changeset/tame-geckos-search.md b/.changeset/tame-geckos-search.md deleted file mode 100644 index 6b6ae86d0df..00000000000 --- a/.changeset/tame-geckos-search.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'openzeppelin-solidity': major ---- - -Remove SafeMath and SignedSafeMath libraries. diff --git a/.changeset/three-weeks-double.md b/.changeset/three-weeks-double.md deleted file mode 100644 index d267e72c4fc..00000000000 --- a/.changeset/three-weeks-double.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'openzeppelin-solidity': major ---- - -Remove CrossChain contracts, including AccessControlCrossChain and all the vendored bridge interfaces. diff --git a/.changeset/unlucky-snakes-drive.md b/.changeset/unlucky-snakes-drive.md deleted file mode 100644 index d4c78dd50b1..00000000000 --- a/.changeset/unlucky-snakes-drive.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'openzeppelin-solidity': patch ---- - -`Address`: Removed `isContract` because of its ambiguous nature and potential for misuse. diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fc4cc5e348..c9fac4d1f35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,14 +2,36 @@ ### Removals -The following contracts and libraries were removed: +The following contracts, libraries and functions were removed: +- `Address.isContract` (because of its ambiguous nature and potential for misuse) +- `Checkpoints.History` - `Counters` - `ERC20Snapshot` - `ERC20VotesComp` +- `ERC165Storage` (in favor of inheritance based approach) +- `ERC777` +- `ERC1820Implementer` - `GovernorVotesComp` +- `GovernorProposalThreshold` (deprecated since 4.4) - `PaymentSplitter` -- `TokenTimelock` (removed in favor of `VestingWallet`) +- `PullPayment` +- `SafeMath` +- `SignedSafeMath` +- `Timers` +- `TokenTimelock` (in favor of `VestingWallet`) +- All escrow contracts (`Escrow`, `ConditionalEscrow` and `RefundEscrow`) +- All cross-chain contracts, including `AccessControlCrossChain` and all the vendored bridge interfaces +- All presets in favor of [OpenZeppelin Contracts Wizard](https://wizard.openzeppelin.com/) + +These removals were implemented in the following PRs: + +- [3637](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3637) +- [3880](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3880) +- [3945](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3945) +- [4258](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4258) +- [4276](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4276) +- [4289](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4289) ### How to upgrade from 4.x From eecd5e15c7157ad8dd8a65e9d1462853876f1d00 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Fri, 2 Jun 2023 17:42:02 +0200 Subject: [PATCH 04/13] Make CHANGELOG more compact for improved readability (#4306) --- CHANGELOG.md | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9fac4d1f35..9d4e13046f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,14 +24,7 @@ The following contracts, libraries and functions were removed: - All cross-chain contracts, including `AccessControlCrossChain` and all the vendored bridge interfaces - All presets in favor of [OpenZeppelin Contracts Wizard](https://wizard.openzeppelin.com/) -These removals were implemented in the following PRs: - -- [3637](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3637) -- [3880](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3880) -- [3945](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3945) -- [4258](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4258) -- [4276](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4276) -- [4289](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4289) +These removals were implemented in the following PRs: [#3637](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3637), [#3880](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3880), [#3945](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3945), [#4258](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4258), [#4276](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4276), [#4289](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/4289) ### How to upgrade from 4.x From ffceb3cd988874369806139ae9e59d2e2a93daec Mon Sep 17 00:00:00 2001 From: Francisco Date: Fri, 2 Jun 2023 18:20:58 +0100 Subject: [PATCH 05/13] Remove hardcoded function resolution (#4299) --- .changeset/red-dots-fold.md | 5 +++ .../ERC1155/extensions/ERC1155Supply.sol | 2 +- .../token/ERC20/extensions/ERC20FlashMint.sol | 2 +- contracts/token/ERC721/ERC721.sol | 32 +++++++++---------- .../ERC721/extensions/ERC721Enumerable.sol | 4 +-- 5 files changed, 25 insertions(+), 20 deletions(-) create mode 100644 .changeset/red-dots-fold.md diff --git a/.changeset/red-dots-fold.md b/.changeset/red-dots-fold.md new file mode 100644 index 00000000000..6df4c8f0cbc --- /dev/null +++ b/.changeset/red-dots-fold.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': major +--- + +Overrides are now used internally for a number of functions that were previously hardcoded to their default implementation in certain locations: `ERC1155Supply.totalSupply`, `ERC721.ownerOf`, `ERC721.balanceOf` in `ERC721Enumerable`, and `ERC20.totalSupply` in `ERC20FlashMint`. diff --git a/contracts/token/ERC1155/extensions/ERC1155Supply.sol b/contracts/token/ERC1155/extensions/ERC1155Supply.sol index 599df00f31c..4ad83ea0289 100644 --- a/contracts/token/ERC1155/extensions/ERC1155Supply.sol +++ b/contracts/token/ERC1155/extensions/ERC1155Supply.sol @@ -38,7 +38,7 @@ abstract contract ERC1155Supply is ERC1155 { * @dev Indicates whether any token exist with a given id, or not. */ function exists(uint256 id) public view virtual returns (bool) { - return ERC1155Supply.totalSupply(id) > 0; + return totalSupply(id) > 0; } /** diff --git a/contracts/token/ERC20/extensions/ERC20FlashMint.sol b/contracts/token/ERC20/extensions/ERC20FlashMint.sol index ce68793b581..18b0a81c6d1 100644 --- a/contracts/token/ERC20/extensions/ERC20FlashMint.sol +++ b/contracts/token/ERC20/extensions/ERC20FlashMint.sol @@ -25,7 +25,7 @@ abstract contract ERC20FlashMint is ERC20, IERC3156FlashLender { * @return The amount of token that can be loaned. */ function maxFlashLoan(address token) public view virtual override returns (uint256) { - return token == address(this) ? type(uint256).max - ERC20.totalSupply() : 0; + return token == address(this) ? type(uint256).max - totalSupply() : 0; } /** diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index 58a626cdad0..af00ecaf4ee 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -108,7 +108,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { * @dev See {IERC721-approve}. */ function approve(address to, uint256 tokenId) public virtual override { - address owner = ERC721.ownerOf(tokenId); + address owner = ownerOf(tokenId); require(to != owner, "ERC721: approval to current owner"); require( @@ -217,7 +217,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { * - `tokenId` must exist. */ function _isApprovedOrOwner(address spender, uint256 tokenId) internal view virtual returns (bool) { - address owner = ERC721.ownerOf(tokenId); + address owner = ownerOf(tokenId); return (spender == owner || isApprovedForAll(owner, spender) || getApproved(tokenId) == spender); } @@ -295,21 +295,20 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { * Emits a {Transfer} event. */ function _burn(uint256 tokenId) internal virtual { - address owner = ERC721.ownerOf(tokenId); + address owner = ownerOf(tokenId); _beforeTokenTransfer(owner, address(0), tokenId, 1); // Update ownership in case tokenId was transferred by `_beforeTokenTransfer` hook - owner = ERC721.ownerOf(tokenId); + owner = ownerOf(tokenId); // Clear approvals delete _tokenApprovals[tokenId]; - unchecked { - // Cannot overflow, as that would require more tokens to be burned/transferred - // out than the owner initially received through minting and transferring in. - _balances[owner] -= 1; - } + // Decrease balance with checked arithmetic, because an `ownerOf` override may + // invalidate the assumption that `_balances[from] >= 1`. + _balances[owner] -= 1; + delete _owners[tokenId]; emit Transfer(owner, address(0), tokenId); @@ -329,26 +328,27 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { * Emits a {Transfer} event. */ function _transfer(address from, address to, uint256 tokenId) internal virtual { - require(ERC721.ownerOf(tokenId) == from, "ERC721: transfer from incorrect owner"); + require(ownerOf(tokenId) == from, "ERC721: transfer from incorrect owner"); require(to != address(0), "ERC721: transfer to the zero address"); _beforeTokenTransfer(from, to, tokenId, 1); // Check that tokenId was not transferred by `_beforeTokenTransfer` hook - require(ERC721.ownerOf(tokenId) == from, "ERC721: transfer from incorrect owner"); + require(ownerOf(tokenId) == from, "ERC721: transfer from incorrect owner"); // Clear approvals from the previous owner delete _tokenApprovals[tokenId]; + // Decrease balance with checked arithmetic, because an `ownerOf` override may + // invalidate the assumption that `_balances[from] >= 1`. + _balances[from] -= 1; + unchecked { - // `_balances[from]` cannot overflow for the same reason as described in `_burn`: - // `from`'s balance is the number of token held, which is at least one before the current - // transfer. // `_balances[to]` could overflow in the conditions described in `_mint`. That would require // all 2**256 token ids to be minted, which in practice is impossible. - _balances[from] -= 1; _balances[to] += 1; } + _owners[tokenId] = to; emit Transfer(from, to, tokenId); @@ -363,7 +363,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { */ function _approve(address to, uint256 tokenId) internal virtual { _tokenApprovals[tokenId] = to; - emit Approval(ERC721.ownerOf(tokenId), to, tokenId); + emit Approval(ownerOf(tokenId), to, tokenId); } /** diff --git a/contracts/token/ERC721/extensions/ERC721Enumerable.sol b/contracts/token/ERC721/extensions/ERC721Enumerable.sol index 6b702d53bec..a9513b21ddb 100644 --- a/contracts/token/ERC721/extensions/ERC721Enumerable.sol +++ b/contracts/token/ERC721/extensions/ERC721Enumerable.sol @@ -35,7 +35,7 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable { * @dev See {IERC721Enumerable-tokenOfOwnerByIndex}. */ function tokenOfOwnerByIndex(address owner, uint256 index) public view virtual override returns (uint256) { - require(index < ERC721.balanceOf(owner), "ERC721Enumerable: owner index out of bounds"); + require(index < balanceOf(owner), "ERC721Enumerable: owner index out of bounds"); return _ownedTokens[owner][index]; } @@ -90,7 +90,7 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable { * @param tokenId uint256 ID of the token to be added to the tokens list of the given address */ function _addTokenToOwnerEnumeration(address to, uint256 tokenId) private { - uint256 length = ERC721.balanceOf(to); + uint256 length = balanceOf(to); _ownedTokens[to][length] = tokenId; _ownedTokensIndex[tokenId] = length; } From 253bfa68c27785536d58afcc95ebeb4af6f9aa7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=BBuk?= Date: Tue, 6 Jun 2023 02:37:12 +0200 Subject: [PATCH 06/13] Optimize Strings.equal (#4262) Co-authored-by: Hadrien Croubois --- .changeset/eighty-crabs-listen.md | 5 +++++ contracts/utils/Strings.sol | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .changeset/eighty-crabs-listen.md diff --git a/.changeset/eighty-crabs-listen.md b/.changeset/eighty-crabs-listen.md new file mode 100644 index 00000000000..7de904db883 --- /dev/null +++ b/.changeset/eighty-crabs-listen.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': patch +--- + +Optimize `Strings.equal` diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index 33d7bbf590e..050f6f9ca6a 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -80,6 +80,6 @@ library Strings { * @dev Returns true if the two strings are equal. */ function equal(string memory a, string memory b) internal pure returns (bool) { - return keccak256(bytes(a)) == keccak256(bytes(b)); + return bytes(a).length == bytes(b).length && keccak256(bytes(a)) == keccak256(bytes(b)); } } From 6c14de4f0c4e5b86762360f96348b095551d936e Mon Sep 17 00:00:00 2001 From: Pascal Marco Caversaccio Date: Tue, 6 Jun 2023 19:00:01 +0300 Subject: [PATCH 07/13] `ECDSA`: Use hexadecimal literals (#4317) --- contracts/utils/cryptography/ECDSA.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/utils/cryptography/ECDSA.sol b/contracts/utils/cryptography/ECDSA.sol index f12815d9dad..2ddbd9ba0b2 100644 --- a/contracts/utils/cryptography/ECDSA.sol +++ b/contracts/utils/cryptography/ECDSA.sol @@ -197,7 +197,7 @@ library ECDSA { /// @solidity memory-safe-assembly assembly { let ptr := mload(0x40) - mstore(ptr, "\x19\x01") + mstore(ptr, hex"19_01") mstore(add(ptr, 0x02), domainSeparator) mstore(add(ptr, 0x22), structHash) data := keccak256(ptr, 0x42) @@ -211,6 +211,6 @@ library ECDSA { * See {recover}. */ function toDataWithIntendedValidatorHash(address validator, bytes memory data) internal pure returns (bytes32) { - return keccak256(abi.encodePacked("\x19\x00", validator, data)); + return keccak256(abi.encodePacked(hex"19_00", validator, data)); } } From 85696d80ad50246e9f7ec983fa325a3c40190033 Mon Sep 17 00:00:00 2001 From: Francisco Date: Tue, 6 Jun 2023 18:42:50 +0100 Subject: [PATCH 08/13] Remove further hardcoded function resolution (#4309) --- .changeset/red-dots-fold.md | 2 +- contracts/mocks/proxy/UUPSUpgradeableMock.sol | 4 ++-- contracts/proxy/ERC1967/ERC1967Proxy.sol | 2 +- .../token/ERC721/extensions/ERC721Enumerable.sol | 12 +++++++----- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/.changeset/red-dots-fold.md b/.changeset/red-dots-fold.md index 6df4c8f0cbc..08cc778434a 100644 --- a/.changeset/red-dots-fold.md +++ b/.changeset/red-dots-fold.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': major --- -Overrides are now used internally for a number of functions that were previously hardcoded to their default implementation in certain locations: `ERC1155Supply.totalSupply`, `ERC721.ownerOf`, `ERC721.balanceOf` in `ERC721Enumerable`, and `ERC20.totalSupply` in `ERC20FlashMint`. +Overrides are now used internally for a number of functions that were previously hardcoded to their default implementation in certain locations: `ERC1155Supply.totalSupply`, `ERC721.ownerOf`, `ERC721.balanceOf` and `ERC721.totalSupply` in `ERC721Enumerable`, `ERC20.totalSupply` in `ERC20FlashMint`, and `ERC1967._getImplementation` in `ERC1967Proxy`. diff --git a/contracts/mocks/proxy/UUPSUpgradeableMock.sol b/contracts/mocks/proxy/UUPSUpgradeableMock.sol index 567f3b536b9..60eed4c9397 100644 --- a/contracts/mocks/proxy/UUPSUpgradeableMock.sol +++ b/contracts/mocks/proxy/UUPSUpgradeableMock.sol @@ -23,10 +23,10 @@ contract UUPSUpgradeableMock is NonUpgradeableMock, UUPSUpgradeable { contract UUPSUpgradeableUnsafeMock is UUPSUpgradeableMock { function upgradeTo(address newImplementation) public override { - ERC1967Upgrade._upgradeToAndCall(newImplementation, bytes(""), false); + _upgradeToAndCall(newImplementation, bytes(""), false); } function upgradeToAndCall(address newImplementation, bytes memory data) public payable override { - ERC1967Upgrade._upgradeToAndCall(newImplementation, data, false); + _upgradeToAndCall(newImplementation, data, false); } } diff --git a/contracts/proxy/ERC1967/ERC1967Proxy.sol b/contracts/proxy/ERC1967/ERC1967Proxy.sol index 320e026fa6e..ea5c204f867 100644 --- a/contracts/proxy/ERC1967/ERC1967Proxy.sol +++ b/contracts/proxy/ERC1967/ERC1967Proxy.sol @@ -31,6 +31,6 @@ contract ERC1967Proxy is Proxy, ERC1967Upgrade { * `0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc` */ function _implementation() internal view virtual override returns (address impl) { - return ERC1967Upgrade._getImplementation(); + return _getImplementation(); } } diff --git a/contracts/token/ERC721/extensions/ERC721Enumerable.sol b/contracts/token/ERC721/extensions/ERC721Enumerable.sol index a9513b21ddb..2168e100b7f 100644 --- a/contracts/token/ERC721/extensions/ERC721Enumerable.sol +++ b/contracts/token/ERC721/extensions/ERC721Enumerable.sol @@ -7,9 +7,11 @@ import "../ERC721.sol"; import "./IERC721Enumerable.sol"; /** - * @dev This implements an optional extension of {ERC721} defined in the EIP that adds - * enumerability of all the token ids in the contract as well as all token ids owned by each - * account. + * @dev This implements an optional extension of {ERC721} defined in the EIP that adds enumerability + * of all the token ids in the contract as well as all token ids owned by each account. + * + * CAUTION: `ERC721` extensions that implement custom `balanceOf` logic, such as `ERC721Consecutive`, + * interfere with enumerability and should not be used together with `ERC721Enumerable`. */ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable { // Mapping from owner to list of owned token IDs @@ -50,7 +52,7 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable { * @dev See {IERC721Enumerable-tokenByIndex}. */ function tokenByIndex(uint256 index) public view virtual override returns (uint256) { - require(index < ERC721Enumerable.totalSupply(), "ERC721Enumerable: global index out of bounds"); + require(index < totalSupply(), "ERC721Enumerable: global index out of bounds"); return _allTokens[index]; } @@ -116,7 +118,7 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable { // To prevent a gap in from's tokens array, we store the last token in the index of the token to delete, and // then delete the last slot (swap and pop). - uint256 lastTokenIndex = ERC721.balanceOf(from) - 1; + uint256 lastTokenIndex = balanceOf(from) - 1; uint256 tokenIndex = _ownedTokensIndex[tokenId]; // When the token to delete is the last token, the swap operation is unnecessary From 4fd2f8be339e850c32206342c3f9a1a7bedbb204 Mon Sep 17 00:00:00 2001 From: Balaji Shetty Pachai <32358081+balajipachai@users.noreply.github.com> Date: Wed, 7 Jun 2023 02:02:55 +0530 Subject: [PATCH 09/13] Replace abi.encodeWithSelector & abi.encodeWithSignature with abi.encodeCall (#4293) Co-authored-by: Hadrien Croubois --- .changeset/big-plums-cover.md | 4 ++++ contracts/mocks/MulticallTest.sol | 2 +- contracts/mocks/ReentrancyMock.sol | 2 +- contracts/mocks/proxy/UUPSLegacy.sol | 5 +---- contracts/token/ERC20/extensions/ERC4626.sol | 2 +- contracts/token/ERC20/utils/SafeERC20.sol | 8 ++++---- contracts/utils/cryptography/SignatureChecker.sol | 2 +- contracts/utils/introspection/ERC165Checker.sol | 2 +- 8 files changed, 14 insertions(+), 13 deletions(-) create mode 100644 .changeset/big-plums-cover.md diff --git a/.changeset/big-plums-cover.md b/.changeset/big-plums-cover.md new file mode 100644 index 00000000000..4111562539e --- /dev/null +++ b/.changeset/big-plums-cover.md @@ -0,0 +1,4 @@ +--- +'openzeppelin-solidity': major +--- +Use `abi.encodeCall` in place of `abi.encodeWithSelector` and `abi.encodeWithSignature` for improved type-checking of parameters diff --git a/contracts/mocks/MulticallTest.sol b/contracts/mocks/MulticallTest.sol index 090f73cb483..cf89c58df1b 100644 --- a/contracts/mocks/MulticallTest.sol +++ b/contracts/mocks/MulticallTest.sol @@ -12,7 +12,7 @@ contract MulticallTest { ) external { bytes[] memory calls = new bytes[](recipients.length); for (uint256 i = 0; i < recipients.length; i++) { - calls[i] = abi.encodeWithSignature("transfer(address,uint256)", recipients[i], amounts[i]); + calls[i] = abi.encodeCall(multicallToken.transfer, (recipients[i], amounts[i])); } bytes[] memory results = multicallToken.multicall(calls); diff --git a/contracts/mocks/ReentrancyMock.sol b/contracts/mocks/ReentrancyMock.sol index 104d4f42aea..b4819dd5989 100644 --- a/contracts/mocks/ReentrancyMock.sol +++ b/contracts/mocks/ReentrancyMock.sol @@ -26,7 +26,7 @@ contract ReentrancyMock is ReentrancyGuard { function countThisRecursive(uint256 n) public nonReentrant { if (n > 0) { _count(); - (bool success, ) = address(this).call(abi.encodeWithSignature("countThisRecursive(uint256)", n - 1)); + (bool success, ) = address(this).call(abi.encodeCall(this.countThisRecursive, (n - 1))); require(success, "ReentrancyMock: failed call"); } } diff --git a/contracts/mocks/proxy/UUPSLegacy.sol b/contracts/mocks/proxy/UUPSLegacy.sol index ed243519ec5..f8ea7214ba8 100644 --- a/contracts/mocks/proxy/UUPSLegacy.sol +++ b/contracts/mocks/proxy/UUPSLegacy.sol @@ -31,10 +31,7 @@ contract UUPSUpgradeableLegacyMock is UUPSUpgradeableMock { if (!rollbackTesting.value) { // Trigger rollback using upgradeTo from the new implementation rollbackTesting.value = true; - Address.functionDelegateCall( - newImplementation, - abi.encodeWithSignature("upgradeTo(address)", oldImplementation) - ); + Address.functionDelegateCall(newImplementation, abi.encodeCall(this.upgradeTo, (oldImplementation))); rollbackTesting.value = false; // Check rollback was effective require(oldImplementation == _getImplementation(), "ERC1967Upgrade: upgrade breaks further upgrades"); diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index a706b545702..54f5ae24bc7 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -67,7 +67,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { */ function _tryGetAssetDecimals(IERC20 asset_) private view returns (bool, uint8) { (bool success, bytes memory encodedDecimals) = address(asset_).staticcall( - abi.encodeWithSelector(IERC20Metadata.decimals.selector) + abi.encodeCall(IERC20Metadata.decimals, ()) ); if (success && encodedDecimals.length >= 32) { uint256 returnedDecimals = abi.decode(encodedDecimals, (uint256)); diff --git a/contracts/token/ERC20/utils/SafeERC20.sol b/contracts/token/ERC20/utils/SafeERC20.sol index 751b8273526..b1532d1cbbc 100644 --- a/contracts/token/ERC20/utils/SafeERC20.sol +++ b/contracts/token/ERC20/utils/SafeERC20.sol @@ -24,7 +24,7 @@ library SafeERC20 { * non-reverting calls are assumed to be successful. */ function safeTransfer(IERC20 token, address to, uint256 value) internal { - _callOptionalReturn(token, abi.encodeWithSelector(token.transfer.selector, to, value)); + _callOptionalReturn(token, abi.encodeCall(token.transfer, (to, value))); } /** @@ -32,7 +32,7 @@ library SafeERC20 { * calling contract. If `token` returns no value, non-reverting calls are assumed to be successful. */ function safeTransferFrom(IERC20 token, address from, address to, uint256 value) internal { - _callOptionalReturn(token, abi.encodeWithSelector(token.transferFrom.selector, from, to, value)); + _callOptionalReturn(token, abi.encodeCall(token.transferFrom, (from, to, value))); } /** @@ -62,10 +62,10 @@ library SafeERC20 { * 0 before setting it to a non-zero value. */ function forceApprove(IERC20 token, address spender, uint256 value) internal { - bytes memory approvalCall = abi.encodeWithSelector(token.approve.selector, spender, value); + bytes memory approvalCall = abi.encodeCall(token.approve, (spender, value)); if (!_callOptionalReturnBool(token, approvalCall)) { - _callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, 0)); + _callOptionalReturn(token, abi.encodeCall(token.approve, (spender, 0))); _callOptionalReturn(token, approvalCall); } } diff --git a/contracts/utils/cryptography/SignatureChecker.sol b/contracts/utils/cryptography/SignatureChecker.sol index ba8f7cf361f..941f7538fac 100644 --- a/contracts/utils/cryptography/SignatureChecker.sol +++ b/contracts/utils/cryptography/SignatureChecker.sol @@ -41,7 +41,7 @@ library SignatureChecker { bytes memory signature ) internal view returns (bool) { (bool success, bytes memory result) = signer.staticcall( - abi.encodeWithSelector(IERC1271.isValidSignature.selector, hash, signature) + abi.encodeCall(IERC1271.isValidSignature, (hash, signature)) ); return (success && result.length >= 32 && diff --git a/contracts/utils/introspection/ERC165Checker.sol b/contracts/utils/introspection/ERC165Checker.sol index 4dc84c1634f..c27b2f17de0 100644 --- a/contracts/utils/introspection/ERC165Checker.sol +++ b/contracts/utils/introspection/ERC165Checker.sol @@ -109,7 +109,7 @@ library ERC165Checker { */ function supportsERC165InterfaceUnchecked(address account, bytes4 interfaceId) internal view returns (bool) { // prepare call - bytes memory encodedParams = abi.encodeWithSelector(IERC165.supportsInterface.selector, interfaceId); + bytes memory encodedParams = abi.encodeCall(IERC165.supportsInterface, (interfaceId)); // perform static call bool success; From df2778f38ecf196d6ecab6df9b958fb5169d3463 Mon Sep 17 00:00:00 2001 From: Renan Souza Date: Tue, 6 Jun 2023 21:13:08 -0300 Subject: [PATCH 10/13] Remove override interface implementations (#4315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ernesto García --- .changeset/violet-dancers-cough.md | 5 +++ contracts/access/AccessControl.sol | 10 +++--- contracts/access/AccessControlEnumerable.sol | 4 +-- contracts/governance/Governor.sol | 12 ++----- contracts/governance/IGovernor.sol | 4 +-- contracts/governance/TimelockController.sol | 12 ++----- contracts/governance/utils/Votes.sol | 16 +++++----- contracts/mocks/ERC1271WalletMock.sol | 4 +-- contracts/mocks/ERC3156FlashBorrowerMock.sol | 2 +- contracts/mocks/token/ERC1155ReceiverMock.sol | 4 +-- .../mocks/token/ERC20VotesLegacyMock.sol | 12 +++---- contracts/mocks/token/ERC721ReceiverMock.sol | 2 +- contracts/proxy/beacon/UpgradeableBeacon.sol | 2 +- contracts/proxy/utils/UUPSUpgradeable.sol | 4 +-- contracts/token/ERC1155/ERC1155.sol | 20 ++++-------- contracts/token/ERC20/ERC20.sol | 18 +++++------ .../token/ERC20/extensions/ERC20FlashMint.sol | 6 ++-- .../token/ERC20/extensions/ERC20Permit.sol | 4 +-- contracts/token/ERC20/extensions/ERC4626.sol | 32 +++++++++---------- contracts/token/ERC721/ERC721.sol | 24 +++++++------- .../ERC721/extensions/ERC721Enumerable.sol | 6 ++-- .../token/ERC721/extensions/ERC721Wrapper.sol | 7 +--- contracts/token/ERC721/utils/ERC721Holder.sol | 2 +- contracts/token/common/ERC2981.sol | 2 +- contracts/utils/cryptography/EIP712.sol | 1 - contracts/utils/introspection/ERC165.sol | 2 +- 26 files changed, 99 insertions(+), 118 deletions(-) create mode 100644 .changeset/violet-dancers-cough.md diff --git a/.changeset/violet-dancers-cough.md b/.changeset/violet-dancers-cough.md new file mode 100644 index 00000000000..c6160d2873e --- /dev/null +++ b/.changeset/violet-dancers-cough.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +Remove the `override` specifier from functions that only override a single interface function. diff --git a/contracts/access/AccessControl.sol b/contracts/access/AccessControl.sol index 82a43933d72..df16dbdab6f 100644 --- a/contracts/access/AccessControl.sol +++ b/contracts/access/AccessControl.sol @@ -82,7 +82,7 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 { /** * @dev Returns `true` if `account` has been granted `role`. */ - function hasRole(bytes32 role, address account) public view virtual override returns (bool) { + function hasRole(bytes32 role, address account) public view virtual returns (bool) { return _roles[role].members[account]; } @@ -126,7 +126,7 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 { * * To change a role's admin, use {_setRoleAdmin}. */ - function getRoleAdmin(bytes32 role) public view virtual override returns (bytes32) { + function getRoleAdmin(bytes32 role) public view virtual returns (bytes32) { return _roles[role].adminRole; } @@ -142,7 +142,7 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 { * * May emit a {RoleGranted} event. */ - function grantRole(bytes32 role, address account) public virtual override onlyRole(getRoleAdmin(role)) { + function grantRole(bytes32 role, address account) public virtual onlyRole(getRoleAdmin(role)) { _grantRole(role, account); } @@ -157,7 +157,7 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 { * * May emit a {RoleRevoked} event. */ - function revokeRole(bytes32 role, address account) public virtual override onlyRole(getRoleAdmin(role)) { + function revokeRole(bytes32 role, address account) public virtual onlyRole(getRoleAdmin(role)) { _revokeRole(role, account); } @@ -177,7 +177,7 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 { * * May emit a {RoleRevoked} event. */ - function renounceRole(bytes32 role, address account) public virtual override { + function renounceRole(bytes32 role, address account) public virtual { require(account == _msgSender(), "AccessControl: can only renounce roles for self"); _revokeRole(role, account); diff --git a/contracts/access/AccessControlEnumerable.sol b/contracts/access/AccessControlEnumerable.sol index 1c37a30a9fe..297d345361a 100644 --- a/contracts/access/AccessControlEnumerable.sol +++ b/contracts/access/AccessControlEnumerable.sol @@ -34,7 +34,7 @@ abstract contract AccessControlEnumerable is IAccessControlEnumerable, AccessCon * https://forum.openzeppelin.com/t/iterating-over-elements-on-enumerableset-in-openzeppelin-contracts/2296[forum post] * for more information. */ - function getRoleMember(bytes32 role, uint256 index) public view virtual override returns (address) { + function getRoleMember(bytes32 role, uint256 index) public view virtual returns (address) { return _roleMembers[role].at(index); } @@ -42,7 +42,7 @@ abstract contract AccessControlEnumerable is IAccessControlEnumerable, AccessCon * @dev Returns the number of accounts that have `role`. Can be used * together with {getRoleMember} to enumerate all bearers of a role. */ - function getRoleMemberCount(bytes32 role) public view virtual override returns (uint256) { + function getRoleMemberCount(bytes32 role) public view virtual returns (uint256) { return _roleMembers[role].length(); } diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index 4ff8c34924d..74d60c7423d 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -605,20 +605,14 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive /** * @dev See {IERC721Receiver-onERC721Received}. */ - function onERC721Received(address, address, uint256, bytes memory) public virtual override returns (bytes4) { + function onERC721Received(address, address, uint256, bytes memory) public virtual returns (bytes4) { return this.onERC721Received.selector; } /** * @dev See {IERC1155Receiver-onERC1155Received}. */ - function onERC1155Received( - address, - address, - uint256, - uint256, - bytes memory - ) public virtual override returns (bytes4) { + function onERC1155Received(address, address, uint256, uint256, bytes memory) public virtual returns (bytes4) { return this.onERC1155Received.selector; } @@ -631,7 +625,7 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive uint256[] memory, uint256[] memory, bytes memory - ) public virtual override returns (bytes4) { + ) public virtual returns (bytes4) { return this.onERC1155BatchReceived.selector; } } diff --git a/contracts/governance/IGovernor.sol b/contracts/governance/IGovernor.sol index 245c5d97aeb..992b5ca10b1 100644 --- a/contracts/governance/IGovernor.sol +++ b/contracts/governance/IGovernor.sol @@ -86,14 +86,14 @@ abstract contract IGovernor is IERC165, IERC6372 { * @notice module:core * @dev See {IERC6372} */ - function clock() public view virtual override returns (uint48); + function clock() public view virtual returns (uint48); /** * @notice module:core * @dev See EIP-6372. */ // solhint-disable-next-line func-name-mixedcase - function CLOCK_MODE() public view virtual override returns (string memory); + function CLOCK_MODE() public view virtual returns (string memory); /** * @notice module:voting diff --git a/contracts/governance/TimelockController.sol b/contracts/governance/TimelockController.sol index 5f09161df11..9930d6a4941 100644 --- a/contracts/governance/TimelockController.sol +++ b/contracts/governance/TimelockController.sol @@ -384,20 +384,14 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver /** * @dev See {IERC721Receiver-onERC721Received}. */ - function onERC721Received(address, address, uint256, bytes memory) public virtual override returns (bytes4) { + function onERC721Received(address, address, uint256, bytes memory) public virtual returns (bytes4) { return this.onERC721Received.selector; } /** * @dev See {IERC1155Receiver-onERC1155Received}. */ - function onERC1155Received( - address, - address, - uint256, - uint256, - bytes memory - ) public virtual override returns (bytes4) { + function onERC1155Received(address, address, uint256, uint256, bytes memory) public virtual returns (bytes4) { return this.onERC1155Received.selector; } @@ -410,7 +404,7 @@ contract TimelockController is AccessControl, IERC721Receiver, IERC1155Receiver uint256[] memory, uint256[] memory, bytes memory - ) public virtual override returns (bytes4) { + ) public virtual returns (bytes4) { return this.onERC1155BatchReceived.selector; } } diff --git a/contracts/governance/utils/Votes.sol b/contracts/governance/utils/Votes.sol index 94f6d4fb941..5fc15da292b 100644 --- a/contracts/governance/utils/Votes.sol +++ b/contracts/governance/utils/Votes.sol @@ -46,7 +46,7 @@ abstract contract Votes is Context, EIP712, Nonces, IERC5805 { * @dev Clock used for flagging checkpoints. Can be overridden to implement timestamp based * checkpoints (and voting), in which case {CLOCK_MODE} should be overridden as well to match. */ - function clock() public view virtual override returns (uint48) { + function clock() public view virtual returns (uint48) { return SafeCast.toUint48(block.number); } @@ -54,7 +54,7 @@ abstract contract Votes is Context, EIP712, Nonces, IERC5805 { * @dev Machine-readable description of the clock as specified in EIP-6372. */ // solhint-disable-next-line func-name-mixedcase - function CLOCK_MODE() public view virtual override returns (string memory) { + function CLOCK_MODE() public view virtual returns (string memory) { // Check that the clock was not modified require(clock() == block.number, "Votes: broken clock mode"); return "mode=blocknumber&from=default"; @@ -63,7 +63,7 @@ abstract contract Votes is Context, EIP712, Nonces, IERC5805 { /** * @dev Returns the current amount of votes that `account` has. */ - function getVotes(address account) public view virtual override returns (uint256) { + function getVotes(address account) public view virtual returns (uint256) { return _delegateCheckpoints[account].latest(); } @@ -75,7 +75,7 @@ abstract contract Votes is Context, EIP712, Nonces, IERC5805 { * * - `timepoint` must be in the past. If operating using block numbers, the block must be already mined. */ - function getPastVotes(address account, uint256 timepoint) public view virtual override returns (uint256) { + function getPastVotes(address account, uint256 timepoint) public view virtual returns (uint256) { require(timepoint < clock(), "Votes: future lookup"); return _delegateCheckpoints[account].upperLookupRecent(SafeCast.toUint32(timepoint)); } @@ -92,7 +92,7 @@ abstract contract Votes is Context, EIP712, Nonces, IERC5805 { * * - `timepoint` must be in the past. If operating using block numbers, the block must be already mined. */ - function getPastTotalSupply(uint256 timepoint) public view virtual override returns (uint256) { + function getPastTotalSupply(uint256 timepoint) public view virtual returns (uint256) { require(timepoint < clock(), "Votes: future lookup"); return _totalCheckpoints.upperLookupRecent(SafeCast.toUint32(timepoint)); } @@ -107,14 +107,14 @@ abstract contract Votes is Context, EIP712, Nonces, IERC5805 { /** * @dev Returns the delegate that `account` has chosen. */ - function delegates(address account) public view virtual override returns (address) { + function delegates(address account) public view virtual returns (address) { return _delegation[account]; } /** * @dev Delegates votes from the sender to `delegatee`. */ - function delegate(address delegatee) public virtual override { + function delegate(address delegatee) public virtual { address account = _msgSender(); _delegate(account, delegatee); } @@ -129,7 +129,7 @@ abstract contract Votes is Context, EIP712, Nonces, IERC5805 { uint8 v, bytes32 r, bytes32 s - ) public virtual override { + ) public virtual { require(block.timestamp <= expiry, "Votes: signature expired"); address signer = ECDSA.recover( _hashTypedDataV4(keccak256(abi.encode(_DELEGATION_TYPEHASH, delegatee, nonce, expiry))), diff --git a/contracts/mocks/ERC1271WalletMock.sol b/contracts/mocks/ERC1271WalletMock.sol index a28fd0fb9eb..2bc39063630 100644 --- a/contracts/mocks/ERC1271WalletMock.sol +++ b/contracts/mocks/ERC1271WalletMock.sol @@ -9,13 +9,13 @@ import "../utils/cryptography/ECDSA.sol"; contract ERC1271WalletMock is Ownable, IERC1271 { constructor(address originalOwner) Ownable(originalOwner) {} - function isValidSignature(bytes32 hash, bytes memory signature) public view override returns (bytes4 magicValue) { + function isValidSignature(bytes32 hash, bytes memory signature) public view returns (bytes4 magicValue) { return ECDSA.recover(hash, signature) == owner() ? this.isValidSignature.selector : bytes4(0); } } contract ERC1271MaliciousMock is IERC1271 { - function isValidSignature(bytes32, bytes memory) public pure override returns (bytes4) { + function isValidSignature(bytes32, bytes memory) public pure returns (bytes4) { assembly { mstore(0, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff) return(0, 32) diff --git a/contracts/mocks/ERC3156FlashBorrowerMock.sol b/contracts/mocks/ERC3156FlashBorrowerMock.sol index 8d24af6c103..abf836606f5 100644 --- a/contracts/mocks/ERC3156FlashBorrowerMock.sol +++ b/contracts/mocks/ERC3156FlashBorrowerMock.sol @@ -33,7 +33,7 @@ contract ERC3156FlashBorrowerMock is IERC3156FlashBorrower { uint256 amount, uint256 fee, bytes calldata data - ) public override returns (bytes32) { + ) public returns (bytes32) { require(msg.sender == token); emit BalanceOf(token, address(this), IERC20(token).balanceOf(address(this))); diff --git a/contracts/mocks/token/ERC1155ReceiverMock.sol b/contracts/mocks/token/ERC1155ReceiverMock.sol index 9f75f002bc6..2b591c058b9 100644 --- a/contracts/mocks/token/ERC1155ReceiverMock.sol +++ b/contracts/mocks/token/ERC1155ReceiverMock.sol @@ -27,7 +27,7 @@ contract ERC1155ReceiverMock is ERC165, IERC1155Receiver { uint256 id, uint256 value, bytes calldata data - ) external override returns (bytes4) { + ) external returns (bytes4) { require(!_recReverts, "ERC1155ReceiverMock: reverting on receive"); emit Received(operator, from, id, value, data, gasleft()); return _recRetval; @@ -39,7 +39,7 @@ contract ERC1155ReceiverMock is ERC165, IERC1155Receiver { uint256[] calldata ids, uint256[] calldata values, bytes calldata data - ) external override returns (bytes4) { + ) external returns (bytes4) { require(!_batReverts, "ERC1155ReceiverMock: reverting on batch receive"); emit BatchReceived(operator, from, ids, values, data, gasleft()); return _batRetval; diff --git a/contracts/mocks/token/ERC20VotesLegacyMock.sol b/contracts/mocks/token/ERC20VotesLegacyMock.sol index cf8afa58a9f..88ba8423616 100644 --- a/contracts/mocks/token/ERC20VotesLegacyMock.sol +++ b/contracts/mocks/token/ERC20VotesLegacyMock.sol @@ -41,14 +41,14 @@ abstract contract ERC20VotesLegacyMock is IVotes, ERC20Permit { /** * @dev Get the address `account` is currently delegating to. */ - function delegates(address account) public view virtual override returns (address) { + function delegates(address account) public view virtual returns (address) { return _delegates[account]; } /** * @dev Gets the current votes balance for `account` */ - function getVotes(address account) public view virtual override returns (uint256) { + function getVotes(address account) public view virtual returns (uint256) { uint256 pos = _checkpoints[account].length; unchecked { return pos == 0 ? 0 : _checkpoints[account][pos - 1].votes; @@ -62,7 +62,7 @@ abstract contract ERC20VotesLegacyMock is IVotes, ERC20Permit { * * - `blockNumber` must have been already mined */ - function getPastVotes(address account, uint256 blockNumber) public view virtual override returns (uint256) { + function getPastVotes(address account, uint256 blockNumber) public view virtual returns (uint256) { require(blockNumber < block.number, "ERC20Votes: block not yet mined"); return _checkpointsLookup(_checkpoints[account], blockNumber); } @@ -75,7 +75,7 @@ abstract contract ERC20VotesLegacyMock is IVotes, ERC20Permit { * * - `blockNumber` must have been already mined */ - function getPastTotalSupply(uint256 blockNumber) public view virtual override returns (uint256) { + function getPastTotalSupply(uint256 blockNumber) public view virtual returns (uint256) { require(blockNumber < block.number, "ERC20Votes: block not yet mined"); return _checkpointsLookup(_totalSupplyCheckpoints, blockNumber); } @@ -127,7 +127,7 @@ abstract contract ERC20VotesLegacyMock is IVotes, ERC20Permit { /** * @dev Delegate votes from the sender to `delegatee`. */ - function delegate(address delegatee) public virtual override { + function delegate(address delegatee) public virtual { _delegate(_msgSender(), delegatee); } @@ -141,7 +141,7 @@ abstract contract ERC20VotesLegacyMock is IVotes, ERC20Permit { uint8 v, bytes32 r, bytes32 s - ) public virtual override { + ) public virtual { require(block.timestamp <= expiry, "ERC20Votes: signature expired"); address signer = ECDSA.recover( _hashTypedDataV4(keccak256(abi.encode(_DELEGATION_TYPEHASH, delegatee, nonce, expiry))), diff --git a/contracts/mocks/token/ERC721ReceiverMock.sol b/contracts/mocks/token/ERC721ReceiverMock.sol index 01526566ab0..e97ceacf323 100644 --- a/contracts/mocks/token/ERC721ReceiverMock.sol +++ b/contracts/mocks/token/ERC721ReceiverMock.sol @@ -27,7 +27,7 @@ contract ERC721ReceiverMock is IERC721Receiver { address from, uint256 tokenId, bytes memory data - ) public override returns (bytes4) { + ) public returns (bytes4) { if (_error == Error.RevertWithMessage) { revert("ERC721ReceiverMock: reverting"); } else if (_error == Error.RevertWithoutMessage) { diff --git a/contracts/proxy/beacon/UpgradeableBeacon.sol b/contracts/proxy/beacon/UpgradeableBeacon.sol index 24d167f6025..37d27f67a55 100644 --- a/contracts/proxy/beacon/UpgradeableBeacon.sol +++ b/contracts/proxy/beacon/UpgradeableBeacon.sol @@ -30,7 +30,7 @@ contract UpgradeableBeacon is IBeacon, Ownable { /** * @dev Returns the current implementation address. */ - function implementation() public view virtual override returns (address) { + function implementation() public view virtual returns (address) { return _implementation; } diff --git a/contracts/proxy/utils/UUPSUpgradeable.sol b/contracts/proxy/utils/UUPSUpgradeable.sol index c281c90910a..1fd73247c9d 100644 --- a/contracts/proxy/utils/UUPSUpgradeable.sol +++ b/contracts/proxy/utils/UUPSUpgradeable.sol @@ -52,7 +52,7 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable, ERC1967Upgrade { * bricking a proxy that upgrades to it, by delegating to itself until out of gas. Thus it is critical that this * function revert if invoked through a proxy. This is guaranteed by the `notDelegated` modifier. */ - function proxiableUUID() external view virtual override notDelegated returns (bytes32) { + function proxiableUUID() external view virtual notDelegated returns (bytes32) { return _IMPLEMENTATION_SLOT; } @@ -92,7 +92,7 @@ abstract contract UUPSUpgradeable is IERC1822Proxiable, ERC1967Upgrade { * Normally, this function will use an xref:access.adoc[access control] modifier such as {Ownable-onlyOwner}. * * ```solidity - * function _authorizeUpgrade(address) internal override onlyOwner {} + * function _authorizeUpgrade(address) internal onlyOwner {} * ``` */ function _authorizeUpgrade(address newImplementation) internal virtual; diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index 6761edd248b..c2f217d7e3a 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/contracts/token/ERC1155/ERC1155.sol @@ -53,7 +53,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { * Clients calling this function must replace the `\{id\}` substring with the * actual token type ID. */ - function uri(uint256) public view virtual override returns (string memory) { + function uri(uint256) public view virtual returns (string memory) { return _uri; } @@ -64,7 +64,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { * * - `account` cannot be the zero address. */ - function balanceOf(address account, uint256 id) public view virtual override returns (uint256) { + function balanceOf(address account, uint256 id) public view virtual returns (uint256) { return _balances[id][account]; } @@ -78,7 +78,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { function balanceOfBatch( address[] memory accounts, uint256[] memory ids - ) public view virtual override returns (uint256[] memory) { + ) public view virtual returns (uint256[] memory) { require(accounts.length == ids.length, "ERC1155: accounts and ids length mismatch"); uint256[] memory batchBalances = new uint256[](accounts.length); @@ -93,27 +93,21 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { /** * @dev See {IERC1155-setApprovalForAll}. */ - function setApprovalForAll(address operator, bool approved) public virtual override { + function setApprovalForAll(address operator, bool approved) public virtual { _setApprovalForAll(_msgSender(), operator, approved); } /** * @dev See {IERC1155-isApprovedForAll}. */ - function isApprovedForAll(address account, address operator) public view virtual override returns (bool) { + function isApprovedForAll(address account, address operator) public view virtual returns (bool) { return _operatorApprovals[account][operator]; } /** * @dev See {IERC1155-safeTransferFrom}. */ - function safeTransferFrom( - address from, - address to, - uint256 id, - uint256 amount, - bytes memory data - ) public virtual override { + function safeTransferFrom(address from, address to, uint256 id, uint256 amount, bytes memory data) public virtual { require( from == _msgSender() || isApprovedForAll(from, _msgSender()), "ERC1155: caller is not token owner or approved" @@ -130,7 +124,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { uint256[] memory ids, uint256[] memory amounts, bytes memory data - ) public virtual override { + ) public virtual { require( from == _msgSender() || isApprovedForAll(from, _msgSender()), "ERC1155: caller is not token owner or approved" diff --git a/contracts/token/ERC20/ERC20.sol b/contracts/token/ERC20/ERC20.sol index 2646a0ddc56..4db525a7ad7 100644 --- a/contracts/token/ERC20/ERC20.sol +++ b/contracts/token/ERC20/ERC20.sol @@ -58,7 +58,7 @@ contract ERC20 is Context, IERC20, IERC20Metadata { /** * @dev Returns the name of the token. */ - function name() public view virtual override returns (string memory) { + function name() public view virtual returns (string memory) { return _name; } @@ -66,7 +66,7 @@ contract ERC20 is Context, IERC20, IERC20Metadata { * @dev Returns the symbol of the token, usually a shorter version of the * name. */ - function symbol() public view virtual override returns (string memory) { + function symbol() public view virtual returns (string memory) { return _symbol; } @@ -83,21 +83,21 @@ contract ERC20 is Context, IERC20, IERC20Metadata { * no way affects any of the arithmetic of the contract, including * {IERC20-balanceOf} and {IERC20-transfer}. */ - function decimals() public view virtual override returns (uint8) { + function decimals() public view virtual returns (uint8) { return 18; } /** * @dev See {IERC20-totalSupply}. */ - function totalSupply() public view virtual override returns (uint256) { + function totalSupply() public view virtual returns (uint256) { return _totalSupply; } /** * @dev See {IERC20-balanceOf}. */ - function balanceOf(address account) public view virtual override returns (uint256) { + function balanceOf(address account) public view virtual returns (uint256) { return _balances[account]; } @@ -109,7 +109,7 @@ contract ERC20 is Context, IERC20, IERC20Metadata { * - `to` cannot be the zero address. * - the caller must have a balance of at least `amount`. */ - function transfer(address to, uint256 amount) public virtual override returns (bool) { + function transfer(address to, uint256 amount) public virtual returns (bool) { address owner = _msgSender(); _transfer(owner, to, amount); return true; @@ -118,7 +118,7 @@ contract ERC20 is Context, IERC20, IERC20Metadata { /** * @dev See {IERC20-allowance}. */ - function allowance(address owner, address spender) public view virtual override returns (uint256) { + function allowance(address owner, address spender) public view virtual returns (uint256) { return _allowances[owner][spender]; } @@ -132,7 +132,7 @@ contract ERC20 is Context, IERC20, IERC20Metadata { * * - `spender` cannot be the zero address. */ - function approve(address spender, uint256 amount) public virtual override returns (bool) { + function approve(address spender, uint256 amount) public virtual returns (bool) { address owner = _msgSender(); _approve(owner, spender, amount); return true; @@ -154,7 +154,7 @@ contract ERC20 is Context, IERC20, IERC20Metadata { * - the caller must have allowance for ``from``'s tokens of at least * `amount`. */ - function transferFrom(address from, address to, uint256 amount) public virtual override returns (bool) { + function transferFrom(address from, address to, uint256 amount) public virtual returns (bool) { address spender = _msgSender(); _spendAllowance(from, spender, amount); _transfer(from, to, amount); diff --git a/contracts/token/ERC20/extensions/ERC20FlashMint.sol b/contracts/token/ERC20/extensions/ERC20FlashMint.sol index 18b0a81c6d1..7a4076678b3 100644 --- a/contracts/token/ERC20/extensions/ERC20FlashMint.sol +++ b/contracts/token/ERC20/extensions/ERC20FlashMint.sol @@ -24,7 +24,7 @@ abstract contract ERC20FlashMint is ERC20, IERC3156FlashLender { * @param token The address of the token that is requested. * @return The amount of token that can be loaned. */ - function maxFlashLoan(address token) public view virtual override returns (uint256) { + function maxFlashLoan(address token) public view virtual returns (uint256) { return token == address(this) ? type(uint256).max - totalSupply() : 0; } @@ -36,7 +36,7 @@ abstract contract ERC20FlashMint is ERC20, IERC3156FlashLender { * @param amount The amount of tokens to be loaned. * @return The fees applied to the corresponding flash loan. */ - function flashFee(address token, uint256 amount) public view virtual override returns (uint256) { + function flashFee(address token, uint256 amount) public view virtual returns (uint256) { require(token == address(this), "ERC20FlashMint: wrong token"); return _flashFee(token, amount); } @@ -88,7 +88,7 @@ abstract contract ERC20FlashMint is ERC20, IERC3156FlashLender { address token, uint256 amount, bytes calldata data - ) public virtual override returns (bool) { + ) public virtual returns (bool) { require(amount <= maxFlashLoan(token), "ERC20FlashMint: amount exceeds maxFlashLoan"); uint256 fee = flashFee(token, amount); _mint(address(receiver), amount); diff --git a/contracts/token/ERC20/extensions/ERC20Permit.sol b/contracts/token/ERC20/extensions/ERC20Permit.sol index e7d43c2dd11..9379e44518d 100644 --- a/contracts/token/ERC20/extensions/ERC20Permit.sol +++ b/contracts/token/ERC20/extensions/ERC20Permit.sol @@ -42,7 +42,7 @@ abstract contract ERC20Permit is ERC20, IERC20Permit, EIP712, Nonces { uint8 v, bytes32 r, bytes32 s - ) public virtual override { + ) public virtual { require(block.timestamp <= deadline, "ERC20Permit: expired deadline"); bytes32 structHash = keccak256(abi.encode(_PERMIT_TYPEHASH, owner, spender, value, _useNonce(owner), deadline)); @@ -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 virtual override returns (bytes32) { + function DOMAIN_SEPARATOR() external view virtual returns (bytes32) { return _domainSeparatorV4(); } } diff --git a/contracts/token/ERC20/extensions/ERC4626.sol b/contracts/token/ERC20/extensions/ERC4626.sol index 54f5ae24bc7..665c95a023d 100644 --- a/contracts/token/ERC20/extensions/ERC4626.sol +++ b/contracts/token/ERC20/extensions/ERC4626.sol @@ -90,67 +90,67 @@ abstract contract ERC4626 is ERC20, IERC4626 { } /** @dev See {IERC4626-asset}. */ - function asset() public view virtual override returns (address) { + function asset() public view virtual returns (address) { return address(_asset); } /** @dev See {IERC4626-totalAssets}. */ - function totalAssets() public view virtual override returns (uint256) { + function totalAssets() public view virtual returns (uint256) { return _asset.balanceOf(address(this)); } /** @dev See {IERC4626-convertToShares}. */ - function convertToShares(uint256 assets) public view virtual override returns (uint256) { + function convertToShares(uint256 assets) public view virtual returns (uint256) { return _convertToShares(assets, Math.Rounding.Down); } /** @dev See {IERC4626-convertToAssets}. */ - function convertToAssets(uint256 shares) public view virtual override returns (uint256) { + function convertToAssets(uint256 shares) public view virtual returns (uint256) { return _convertToAssets(shares, Math.Rounding.Down); } /** @dev See {IERC4626-maxDeposit}. */ - function maxDeposit(address) public view virtual override returns (uint256) { + function maxDeposit(address) public view virtual returns (uint256) { return type(uint256).max; } /** @dev See {IERC4626-maxMint}. */ - function maxMint(address) public view virtual override returns (uint256) { + function maxMint(address) public view virtual returns (uint256) { return type(uint256).max; } /** @dev See {IERC4626-maxWithdraw}. */ - function maxWithdraw(address owner) public view virtual override returns (uint256) { + function maxWithdraw(address owner) public view virtual returns (uint256) { return _convertToAssets(balanceOf(owner), Math.Rounding.Down); } /** @dev See {IERC4626-maxRedeem}. */ - function maxRedeem(address owner) public view virtual override returns (uint256) { + function maxRedeem(address owner) public view virtual returns (uint256) { return balanceOf(owner); } /** @dev See {IERC4626-previewDeposit}. */ - function previewDeposit(uint256 assets) public view virtual override returns (uint256) { + function previewDeposit(uint256 assets) public view virtual returns (uint256) { return _convertToShares(assets, Math.Rounding.Down); } /** @dev See {IERC4626-previewMint}. */ - function previewMint(uint256 shares) public view virtual override returns (uint256) { + function previewMint(uint256 shares) public view virtual returns (uint256) { return _convertToAssets(shares, Math.Rounding.Up); } /** @dev See {IERC4626-previewWithdraw}. */ - function previewWithdraw(uint256 assets) public view virtual override returns (uint256) { + function previewWithdraw(uint256 assets) public view virtual returns (uint256) { return _convertToShares(assets, Math.Rounding.Up); } /** @dev See {IERC4626-previewRedeem}. */ - function previewRedeem(uint256 shares) public view virtual override returns (uint256) { + function previewRedeem(uint256 shares) public view virtual returns (uint256) { return _convertToAssets(shares, Math.Rounding.Down); } /** @dev See {IERC4626-deposit}. */ - function deposit(uint256 assets, address receiver) public virtual override returns (uint256) { + function deposit(uint256 assets, address receiver) public virtual returns (uint256) { require(assets <= maxDeposit(receiver), "ERC4626: deposit more than max"); uint256 shares = previewDeposit(assets); @@ -164,7 +164,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { * As opposed to {deposit}, minting is allowed even if the vault is in a state where the price of a share is zero. * In this case, the shares will be minted without requiring any assets to be deposited. */ - function mint(uint256 shares, address receiver) public virtual override returns (uint256) { + function mint(uint256 shares, address receiver) public virtual returns (uint256) { require(shares <= maxMint(receiver), "ERC4626: mint more than max"); uint256 assets = previewMint(shares); @@ -174,7 +174,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { } /** @dev See {IERC4626-withdraw}. */ - function withdraw(uint256 assets, address receiver, address owner) public virtual override returns (uint256) { + function withdraw(uint256 assets, address receiver, address owner) public virtual returns (uint256) { require(assets <= maxWithdraw(owner), "ERC4626: withdraw more than max"); uint256 shares = previewWithdraw(assets); @@ -184,7 +184,7 @@ abstract contract ERC4626 is ERC20, IERC4626 { } /** @dev See {IERC4626-redeem}. */ - function redeem(uint256 shares, address receiver, address owner) public virtual override returns (uint256) { + function redeem(uint256 shares, address receiver, address owner) public virtual returns (uint256) { require(shares <= maxRedeem(owner), "ERC4626: redeem more than max"); uint256 assets = previewRedeem(shares); diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index af00ecaf4ee..3eb3f74cf35 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -57,7 +57,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { /** * @dev See {IERC721-balanceOf}. */ - function balanceOf(address owner) public view virtual override returns (uint256) { + function balanceOf(address owner) public view virtual returns (uint256) { require(owner != address(0), "ERC721: address zero is not a valid owner"); return _balances[owner]; } @@ -65,7 +65,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { /** * @dev See {IERC721-ownerOf}. */ - function ownerOf(uint256 tokenId) public view virtual override returns (address) { + function ownerOf(uint256 tokenId) public view virtual returns (address) { address owner = _ownerOf(tokenId); require(owner != address(0), "ERC721: invalid token ID"); return owner; @@ -74,21 +74,21 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { /** * @dev See {IERC721Metadata-name}. */ - function name() public view virtual override returns (string memory) { + function name() public view virtual returns (string memory) { return _name; } /** * @dev See {IERC721Metadata-symbol}. */ - function symbol() public view virtual override returns (string memory) { + function symbol() public view virtual returns (string memory) { return _symbol; } /** * @dev See {IERC721Metadata-tokenURI}. */ - function tokenURI(uint256 tokenId) public view virtual override returns (string memory) { + function tokenURI(uint256 tokenId) public view virtual returns (string memory) { _requireMinted(tokenId); string memory baseURI = _baseURI(); @@ -107,7 +107,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { /** * @dev See {IERC721-approve}. */ - function approve(address to, uint256 tokenId) public virtual override { + function approve(address to, uint256 tokenId) public virtual { address owner = ownerOf(tokenId); require(to != owner, "ERC721: approval to current owner"); @@ -122,7 +122,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { /** * @dev See {IERC721-getApproved}. */ - function getApproved(uint256 tokenId) public view virtual override returns (address) { + function getApproved(uint256 tokenId) public view virtual returns (address) { _requireMinted(tokenId); return _tokenApprovals[tokenId]; @@ -131,21 +131,21 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { /** * @dev See {IERC721-setApprovalForAll}. */ - function setApprovalForAll(address operator, bool approved) public virtual override { + function setApprovalForAll(address operator, bool approved) public virtual { _setApprovalForAll(_msgSender(), operator, approved); } /** * @dev See {IERC721-isApprovedForAll}. */ - function isApprovedForAll(address owner, address operator) public view virtual override returns (bool) { + function isApprovedForAll(address owner, address operator) public view virtual returns (bool) { return _operatorApprovals[owner][operator]; } /** * @dev See {IERC721-transferFrom}. */ - function transferFrom(address from, address to, uint256 tokenId) public virtual override { + function transferFrom(address from, address to, uint256 tokenId) public virtual { //solhint-disable-next-line max-line-length require(_isApprovedOrOwner(_msgSender(), tokenId), "ERC721: caller is not token owner or approved"); @@ -155,14 +155,14 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { /** * @dev See {IERC721-safeTransferFrom}. */ - function safeTransferFrom(address from, address to, uint256 tokenId) public virtual override { + function safeTransferFrom(address from, address to, uint256 tokenId) public virtual { safeTransferFrom(from, to, tokenId, ""); } /** * @dev See {IERC721-safeTransferFrom}. */ - function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data) public virtual override { + function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data) public virtual { require(_isApprovedOrOwner(_msgSender(), tokenId), "ERC721: caller is not token owner or approved"); _safeTransfer(from, to, tokenId, data); } diff --git a/contracts/token/ERC721/extensions/ERC721Enumerable.sol b/contracts/token/ERC721/extensions/ERC721Enumerable.sol index 2168e100b7f..8cea9e19ae2 100644 --- a/contracts/token/ERC721/extensions/ERC721Enumerable.sol +++ b/contracts/token/ERC721/extensions/ERC721Enumerable.sol @@ -36,7 +36,7 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable { /** * @dev See {IERC721Enumerable-tokenOfOwnerByIndex}. */ - function tokenOfOwnerByIndex(address owner, uint256 index) public view virtual override returns (uint256) { + function tokenOfOwnerByIndex(address owner, uint256 index) public view virtual returns (uint256) { require(index < balanceOf(owner), "ERC721Enumerable: owner index out of bounds"); return _ownedTokens[owner][index]; } @@ -44,14 +44,14 @@ abstract contract ERC721Enumerable is ERC721, IERC721Enumerable { /** * @dev See {IERC721Enumerable-totalSupply}. */ - function totalSupply() public view virtual override returns (uint256) { + function totalSupply() public view virtual returns (uint256) { return _allTokens.length; } /** * @dev See {IERC721Enumerable-tokenByIndex}. */ - function tokenByIndex(uint256 index) public view virtual override returns (uint256) { + function tokenByIndex(uint256 index) public view virtual returns (uint256) { require(index < totalSupply(), "ERC721Enumerable: global index out of bounds"); return _allTokens[index]; } diff --git a/contracts/token/ERC721/extensions/ERC721Wrapper.sol b/contracts/token/ERC721/extensions/ERC721Wrapper.sol index 58346182b0d..9226349f4c0 100644 --- a/contracts/token/ERC721/extensions/ERC721Wrapper.sol +++ b/contracts/token/ERC721/extensions/ERC721Wrapper.sol @@ -67,12 +67,7 @@ abstract contract ERC721Wrapper is ERC721, IERC721Receiver { * WARNING: Doesn't work with unsafe transfers (eg. {IERC721-transferFrom}). Use {ERC721Wrapper-_recover} * for recovering in that scenario. */ - function onERC721Received( - address, - address from, - uint256 tokenId, - bytes memory - ) public virtual override returns (bytes4) { + function onERC721Received(address, address from, uint256 tokenId, bytes memory) public virtual returns (bytes4) { require(address(underlying()) == _msgSender(), "ERC721Wrapper: caller is not underlying"); _safeMint(from, tokenId); return IERC721Receiver.onERC721Received.selector; diff --git a/contracts/token/ERC721/utils/ERC721Holder.sol b/contracts/token/ERC721/utils/ERC721Holder.sol index 41bee820c87..9f5b2e9f9df 100644 --- a/contracts/token/ERC721/utils/ERC721Holder.sol +++ b/contracts/token/ERC721/utils/ERC721Holder.sol @@ -17,7 +17,7 @@ contract ERC721Holder is IERC721Receiver { * * Always returns `IERC721Receiver.onERC721Received.selector`. */ - function onERC721Received(address, address, uint256, bytes memory) public virtual override returns (bytes4) { + function onERC721Received(address, address, uint256, bytes memory) public virtual returns (bytes4) { return this.onERC721Received.selector; } } diff --git a/contracts/token/common/ERC2981.sol b/contracts/token/common/ERC2981.sol index 5d40388aaba..85e9027333e 100644 --- a/contracts/token/common/ERC2981.sol +++ b/contracts/token/common/ERC2981.sol @@ -40,7 +40,7 @@ abstract contract ERC2981 is IERC2981, ERC165 { /** * @inheritdoc IERC2981 */ - function royaltyInfo(uint256 tokenId, uint256 salePrice) public view virtual override returns (address, uint256) { + function royaltyInfo(uint256 tokenId, uint256 salePrice) public view virtual returns (address, uint256) { RoyaltyInfo memory royalty = _tokenRoyaltyInfo[tokenId]; if (royalty.receiver == address(0)) { diff --git a/contracts/utils/cryptography/EIP712.sol b/contracts/utils/cryptography/EIP712.sol index 526b13580e7..b9c9c9d195d 100644 --- a/contracts/utils/cryptography/EIP712.sol +++ b/contracts/utils/cryptography/EIP712.sol @@ -118,7 +118,6 @@ abstract contract EIP712 is IERC5267 { public view virtual - override returns ( bytes1 fields, string memory name, diff --git a/contracts/utils/introspection/ERC165.sol b/contracts/utils/introspection/ERC165.sol index 6849637e84e..111913007ea 100644 --- a/contracts/utils/introspection/ERC165.sol +++ b/contracts/utils/introspection/ERC165.sol @@ -21,7 +21,7 @@ abstract contract ERC165 is IERC165 { /** * @dev See {IERC165-supportsInterface}. */ - function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) { + function supportsInterface(bytes4 interfaceId) public view virtual returns (bool) { return interfaceId == type(IERC165).interfaceId; } } From d9474327a492f9f310f31bc53f38dbea56ed9a57 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Wed, 7 Jun 2023 02:32:14 +0200 Subject: [PATCH 11/13] Merge pull request from GHSA-5h3x-9wvq-w4m2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Francisco Co-authored-by: Ernesto García --- .changeset/swift-bags-divide.md | 5 ++ contracts/governance/Governor.sol | 90 ++++++++++++++++++++++++++++++- test/governance/Governor.t.sol | 55 +++++++++++++++++++ test/governance/Governor.test.js | 83 ++++++++++++++++++++++++++++ 4 files changed, 231 insertions(+), 2 deletions(-) create mode 100644 .changeset/swift-bags-divide.md create mode 100644 test/governance/Governor.t.sol diff --git a/.changeset/swift-bags-divide.md b/.changeset/swift-bags-divide.md new file mode 100644 index 00000000000..9af63e98e3c --- /dev/null +++ b/.changeset/swift-bags-divide.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': patch +--- + +`Governor`: Add a mechanism to restrict the address of the proposer using a suffix in the description. diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index 74d60c7423d..2e2289c6a8e 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -263,7 +263,7 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive } /** - * @dev See {IGovernor-propose}. + * @dev See {IGovernor-propose}. This function has opt-in frontrunning protection, described in {_isValidDescriptionForProposer}. */ function propose( address[] memory targets, @@ -272,8 +272,9 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive string memory description ) public virtual override returns (uint256) { address proposer = _msgSender(); - uint256 currentTimepoint = clock(); + require(_isValidDescriptionForProposer(proposer, description), "Governor: proposer restricted"); + uint256 currentTimepoint = clock(); require( getVotes(proposer, currentTimepoint - 1) >= proposalThreshold(), "Governor: proposer votes below proposal threshold" @@ -628,4 +629,89 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive ) public virtual returns (bytes4) { return this.onERC1155BatchReceived.selector; } + + /** + * @dev Check if the proposer is authorized to submit a proposal with the given description. + * + * If the proposal description ends with `#proposer=0x???`, where `0x???` is an address written as a hex string + * (case insensitive), then the submission of this proposal will only be authorized to said address. + * + * This is used for frontrunning protection. By adding this pattern at the end of their proposal, one can ensure + * that no other address can submit the same proposal. An attacker would have to either remove or change that part, + * which would result in a different proposal id. + * + * If the description does not match this pattern, it is unrestricted and anyone can submit it. This includes: + * - If the `0x???` part is not a valid hex string. + * - If the `0x???` part is a valid hex string, but does not contain exactly 40 hex digits. + * - If it ends with the expected suffix followed by newlines or other whitespace. + * - If it ends with some other similar suffix, e.g. `#other=abc`. + * - If it does not end with any such suffix. + */ + function _isValidDescriptionForProposer( + address proposer, + string memory description + ) internal view virtual returns (bool) { + uint256 len = bytes(description).length; + + // Length is too short to contain a valid proposer suffix + if (len < 52) { + return true; + } + + // Extract what would be the `#proposer=0x` marker beginning the suffix + bytes12 marker; + assembly { + // - Start of the string contents in memory = description + 32 + // - First character of the marker = len - 52 + // - Length of "#proposer=0x0000000000000000000000000000000000000000" = 52 + // - We read the memory word starting at the first character of the marker: + // - (description + 32) + (len - 52) = description + (len - 20) + // - Note: Solidity will ignore anything past the first 12 bytes + marker := mload(add(description, sub(len, 20))) + } + + // If the marker is not found, there is no proposer suffix to check + if (marker != bytes12("#proposer=0x")) { + return true; + } + + // Parse the 40 characters following the marker as uint160 + uint160 recovered = 0; + for (uint256 i = len - 40; i < len; ++i) { + (bool isHex, uint8 value) = _tryHexToUint(bytes(description)[i]); + // If any of the characters is not a hex digit, ignore the suffix entirely + if (!isHex) { + return true; + } + recovered = (recovered << 4) | value; + } + + return recovered == uint160(proposer); + } + + /** + * @dev Try to parse a character from a string as a hex value. Returns `(true, value)` if the char is in + * `[0-9a-fA-F]` and `(false, 0)` otherwise. Value is guaranteed to be in the range `0 <= value < 16` + */ + function _tryHexToUint(bytes1 char) private pure returns (bool, uint8) { + uint8 c = uint8(char); + unchecked { + // Case 0-9 + if (47 < c && c < 58) { + return (true, c - 48); + } + // Case A-F + else if (64 < c && c < 71) { + return (true, c - 55); + } + // Case a-f + else if (96 < c && c < 103) { + return (true, c - 87); + } + // Else: not a hex char + else { + return (false, 0); + } + } + } } diff --git a/test/governance/Governor.t.sol b/test/governance/Governor.t.sol new file mode 100644 index 00000000000..43c4c5ddd9e --- /dev/null +++ b/test/governance/Governor.t.sol @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.19; + +import "forge-std/Test.sol"; +import "../../contracts/utils/Strings.sol"; +import "../../contracts/governance/Governor.sol"; + +contract GovernorInternalTest is Test, Governor { + constructor() Governor("") {} + + function testValidDescriptionForProposer(string memory description, address proposer, bool includeProposer) public { + if (includeProposer) { + description = string.concat(description, "#proposer=", Strings.toHexString(proposer)); + } + assertTrue(_isValidDescriptionForProposer(proposer, description)); + } + + function testInvalidDescriptionForProposer( + string memory description, + address commitProposer, + address actualProposer + ) public { + vm.assume(commitProposer != actualProposer); + description = string.concat(description, "#proposer=", Strings.toHexString(commitProposer)); + assertFalse(_isValidDescriptionForProposer(actualProposer, description)); + } + + // We don't need to truly implement implement the missing functions because we are just testing + // internal helpers. + + function clock() public pure override returns (uint48) {} + + // solhint-disable-next-line func-name-mixedcase + function CLOCK_MODE() public pure override returns (string memory) {} + + // solhint-disable-next-line func-name-mixedcase + function COUNTING_MODE() public pure virtual override returns (string memory) {} + + function votingDelay() public pure virtual override returns (uint256) {} + + function votingPeriod() public pure virtual override returns (uint256) {} + + function quorum(uint256) public pure virtual override returns (uint256) {} + + function hasVoted(uint256, address) public pure virtual override returns (bool) {} + + function _quorumReached(uint256) internal pure virtual override returns (bool) {} + + function _voteSucceeded(uint256) internal pure virtual override returns (bool) {} + + function _getVotes(address, uint256, bytes memory) internal pure virtual override returns (uint256) {} + + function _countVote(uint256, address, uint8, uint256, bytes memory) internal virtual override {} +} diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js index d3448221277..96feaf3a7d9 100644 --- a/test/governance/Governor.test.js +++ b/test/governance/Governor.test.js @@ -551,6 +551,89 @@ contract('Governor', function (accounts) { }); }); + describe('frontrun protection using description suffix', function () { + describe('without protection', function () { + describe('without suffix', function () { + it('proposer can propose', async function () { + expectEvent(await this.helper.propose({ from: proposer }), 'ProposalCreated'); + }); + + it('someone else can propose', async function () { + expectEvent(await this.helper.propose({ from: voter1 }), 'ProposalCreated'); + }); + }); + + describe('with different suffix', function () { + beforeEach(async function () { + this.proposal = this.helper.setProposal( + [ + { + target: this.receiver.address, + data: this.receiver.contract.methods.mockFunction().encodeABI(), + value, + }, + ], + `#wrong-suffix=${proposer}`, + ); + }); + + it('proposer can propose', async function () { + expectEvent(await this.helper.propose({ from: proposer }), 'ProposalCreated'); + }); + + it('someone else can propose', async function () { + expectEvent(await this.helper.propose({ from: voter1 }), 'ProposalCreated'); + }); + }); + + describe('with proposer suffix but bad address part', function () { + beforeEach(async function () { + this.proposal = this.helper.setProposal( + [ + { + target: this.receiver.address, + data: this.receiver.contract.methods.mockFunction().encodeABI(), + value, + }, + ], + `#proposer=0x3C44CdDdB6a900fa2b585dd299e03d12FA429XYZ`, // XYZ are not a valid hex char + ); + }); + + it('propose can propose', async function () { + expectEvent(await this.helper.propose({ from: proposer }), 'ProposalCreated'); + }); + + it('someone else can propose', async function () { + expectEvent(await this.helper.propose({ from: voter1 }), 'ProposalCreated'); + }); + }); + }); + + describe('with protection via proposer suffix', function () { + beforeEach(async function () { + this.proposal = this.helper.setProposal( + [ + { + target: this.receiver.address, + data: this.receiver.contract.methods.mockFunction().encodeABI(), + value, + }, + ], + `#proposer=${proposer}`, + ); + }); + + it('proposer can propose', async function () { + expectEvent(await this.helper.propose({ from: proposer }), 'ProposalCreated'); + }); + + it('someone else cannot propose', async function () { + await expectRevert(this.helper.propose({ from: voter1 }), 'Governor: proposer restricted'); + }); + }); + }); + describe('onlyGovernance updates', function () { it('setVotingDelay is protected', async function () { await expectRevert(this.mock.setVotingDelay('0'), 'Governor: onlyGovernance'); From 1d5bcd04e77041fa7b971bc7386cc161a5ae3bf8 Mon Sep 17 00:00:00 2001 From: Pascal Marco Caversaccio Date: Thu, 8 Jun 2023 04:10:43 +0300 Subject: [PATCH 12/13] `ECDSA`: Use unchecked arithmetic for the `tryRecover` function (#4301) Signed-off-by: Pascal Marco Caversaccio --- .changeset/four-adults-knock.md | 5 +++++ contracts/utils/cryptography/ECDSA.sol | 9 ++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 .changeset/four-adults-knock.md diff --git a/.changeset/four-adults-knock.md b/.changeset/four-adults-knock.md new file mode 100644 index 00000000000..f6f566d7a38 --- /dev/null +++ b/.changeset/four-adults-knock.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': patch +--- + +`ECDSA`: Use unchecked arithmetic for the `tryRecover` function that receives the `r` and `vs` short-signature fields separately. diff --git a/contracts/utils/cryptography/ECDSA.sol b/contracts/utils/cryptography/ECDSA.sol index 2ddbd9ba0b2..efff6063c2f 100644 --- a/contracts/utils/cryptography/ECDSA.sol +++ b/contracts/utils/cryptography/ECDSA.sol @@ -98,9 +98,12 @@ library ECDSA { * _Available since v4.3._ */ function tryRecover(bytes32 hash, bytes32 r, bytes32 vs) internal pure returns (address, RecoverError) { - bytes32 s = vs & bytes32(0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff); - uint8 v = uint8((uint256(vs) >> 255) + 27); - return tryRecover(hash, v, r, s); + unchecked { + bytes32 s = vs & bytes32(0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff); + // We do not check for an overflow here since the shift operation results in 0 or 1. + uint8 v = uint8((uint256(vs) >> 255) + 27); + return tryRecover(hash, v, r, s); + } } /** From cc0426317052b1850b686f2fbbcf481520399735 Mon Sep 17 00:00:00 2001 From: Robb Walters Date: Fri, 9 Jun 2023 09:00:16 -0700 Subject: [PATCH 13/13] Highlight Reentrancy Risk in IERC1155 SafeTransferFrom Function (#4283) Co-authored-by: Francisco --- contracts/token/ERC1155/IERC1155.sol | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/contracts/token/ERC1155/IERC1155.sol b/contracts/token/ERC1155/IERC1155.sol index d7e25a5b12e..3d2585d7e02 100644 --- a/contracts/token/ERC1155/IERC1155.sol +++ b/contracts/token/ERC1155/IERC1155.sol @@ -86,6 +86,11 @@ interface IERC1155 is IERC165 { /** * @dev Transfers `amount` tokens of token type `id` from `from` to `to`. * + * WARNING: This function can potentially allow a reentrancy attack when transferring tokens + * to an untrusted contract, when invoking {onERC1155Received} on the receiver. + * Ensure to follow the checks-effects-interactions pattern and consider employing + * reentrancy guards when interacting with untrusted contracts. + * * Emits a {TransferSingle} event. * * Requirements: @@ -101,6 +106,12 @@ interface IERC1155 is IERC165 { /** * @dev xref:ROOT:erc1155.adoc#batch-operations[Batched] version of {safeTransferFrom}. * + * + * WARNING: This function can potentially allow a reentrancy attack when transferring tokens + * to an untrusted contract, when invoking {onERC1155Received} on the receiver. + * Ensure to follow the checks-effects-interactions pattern and consider employing + * reentrancy guards when interacting with untrusted contracts. + * * Emits a {TransferBatch} event. * * Requirements: