From 7439664490bd54cb956caffbef7bb4e07dc48f46 Mon Sep 17 00:00:00 2001 From: Sabnock <24715302+Sabnock01@users.noreply.github.com> Date: Mon, 29 Jan 2024 11:42:53 -0600 Subject: [PATCH 1/4] Remove note about event access in `ERC1967Utils.sol` (#4861) --- contracts/proxy/ERC1967/ERC1967Utils.sol | 2 -- 1 file changed, 2 deletions(-) diff --git a/contracts/proxy/ERC1967/ERC1967Utils.sol b/contracts/proxy/ERC1967/ERC1967Utils.sol index 19354814db6..6e92465716b 100644 --- a/contracts/proxy/ERC1967/ERC1967Utils.sol +++ b/contracts/proxy/ERC1967/ERC1967Utils.sol @@ -12,8 +12,6 @@ import {StorageSlot} from "../../utils/StorageSlot.sol"; * https://eips.ethereum.org/EIPS/eip-1967[ERC-1967] slots. */ library ERC1967Utils { - // We re-declare ERC-1967 events here because they can't be used directly from IERC1967. - // This will be fixed in Solidity 0.8.21. At that point we should remove these events. /** * @dev Emitted when the implementation is upgraded. */ From 61117c4db8497ba489d5e1e127565a011ed6907a Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 29 Jan 2024 20:44:33 +0100 Subject: [PATCH 2/4] Bound lookup in arrays with duplicate (#4842) Co-authored-by: RenanSouza2 Co-authored-by: ernestognw --- .changeset/flat-turtles-repeat.md | 5 + .changeset/thick-pumpkins-report.md | 5 + contracts/mocks/ArraysMock.sol | 20 +++- contracts/utils/Arrays.sol | 134 ++++++++++++++++++++++++- test/utils/Arrays.test.js | 67 ++++++++----- test/utils/structs/Checkpoints.test.js | 8 +- 6 files changed, 204 insertions(+), 35 deletions(-) create mode 100644 .changeset/flat-turtles-repeat.md create mode 100644 .changeset/thick-pumpkins-report.md diff --git a/.changeset/flat-turtles-repeat.md b/.changeset/flat-turtles-repeat.md new file mode 100644 index 00000000000..6b627201ac9 --- /dev/null +++ b/.changeset/flat-turtles-repeat.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`Arrays`: deprecate `findUpperBound` in favor of the new `lowerBound`. diff --git a/.changeset/thick-pumpkins-report.md b/.changeset/thick-pumpkins-report.md new file mode 100644 index 00000000000..f17a208950c --- /dev/null +++ b/.changeset/thick-pumpkins-report.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`Arrays`: add new functions `lowerBound`, `upperBound`, `lowerBoundMemory` and `upperBoundMemory` for lookups in sorted arrays with potential duplicates. diff --git a/contracts/mocks/ArraysMock.sol b/contracts/mocks/ArraysMock.sol index a00def29cb6..a2fbb6dea63 100644 --- a/contracts/mocks/ArraysMock.sol +++ b/contracts/mocks/ArraysMock.sol @@ -13,8 +13,24 @@ contract Uint256ArraysMock { _array = array; } - function findUpperBound(uint256 element) external view returns (uint256) { - return _array.findUpperBound(element); + function findUpperBound(uint256 value) external view returns (uint256) { + return _array.findUpperBound(value); + } + + function lowerBound(uint256 value) external view returns (uint256) { + return _array.lowerBound(value); + } + + function upperBound(uint256 value) external view returns (uint256) { + return _array.upperBound(value); + } + + function lowerBoundMemory(uint256[] memory array, uint256 value) external pure returns (uint256) { + return array.lowerBoundMemory(value); + } + + function upperBoundMemory(uint256[] memory array, uint256 value) external pure returns (uint256) { + return array.upperBoundMemory(value); } function unsafeAccess(uint256 pos) external view returns (uint256) { diff --git a/contracts/utils/Arrays.sol b/contracts/utils/Arrays.sol index aaab3ce592b..67c63a7e7b9 100644 --- a/contracts/utils/Arrays.sol +++ b/contracts/utils/Arrays.sol @@ -18,8 +18,12 @@ library Arrays { * values in the array are strictly less than `element`), the array length is * returned. Time complexity O(log n). * - * `array` is expected to be sorted in ascending order, and to contain no - * repeated elements. + * NOTE: The `array` is expected to be sorted in ascending order, and to + * contain no repeated elements. + * + * IMPORTANT: Deprecated. This implementation behaves as {lowerBound} but lacks + * support for repeated elements in the array. The {lowerBound} function should + * be used instead. */ function findUpperBound(uint256[] storage array, uint256 element) internal view returns (uint256) { uint256 low = 0; @@ -49,6 +53,132 @@ library Arrays { } } + /** + * @dev Searches an `array` sorted in ascending order and returns the first + * index that contains a value greater or equal than `element`. If no such index + * exists (i.e. all values in the array are strictly less than `element`), the array + * length is returned. Time complexity O(log n). + * + * See C++'s https://en.cppreference.com/w/cpp/algorithm/lower_bound[lower_bound]. + */ + function lowerBound(uint256[] storage array, uint256 element) internal view returns (uint256) { + uint256 low = 0; + uint256 high = array.length; + + if (high == 0) { + return 0; + } + + while (low < high) { + uint256 mid = Math.average(low, high); + + // Note that mid will always be strictly less than high (i.e. it will be a valid array index) + // because Math.average rounds towards zero (it does integer division with truncation). + if (unsafeAccess(array, mid).value < element) { + // this cannot overflow because mid < high + unchecked { + low = mid + 1; + } + } else { + high = mid; + } + } + + return low; + } + + /** + * @dev Searches an `array` sorted in ascending order and returns the first + * index that contains a value strictly greater than `element`. If no such index + * exists (i.e. all values in the array are strictly less than `element`), the array + * length is returned. Time complexity O(log n). + * + * See C++'s https://en.cppreference.com/w/cpp/algorithm/upper_bound[upper_bound]. + */ + function upperBound(uint256[] storage array, uint256 element) internal view returns (uint256) { + uint256 low = 0; + uint256 high = array.length; + + if (high == 0) { + return 0; + } + + while (low < high) { + uint256 mid = Math.average(low, high); + + // Note that mid will always be strictly less than high (i.e. it will be a valid array index) + // because Math.average rounds towards zero (it does integer division with truncation). + if (unsafeAccess(array, mid).value > element) { + high = mid; + } else { + // this cannot overflow because mid < high + unchecked { + low = mid + 1; + } + } + } + + return low; + } + + /** + * @dev Same as {lowerBound}, but with an array in memory. + */ + function lowerBoundMemory(uint256[] memory array, uint256 element) internal pure returns (uint256) { + uint256 low = 0; + uint256 high = array.length; + + if (high == 0) { + return 0; + } + + while (low < high) { + uint256 mid = Math.average(low, high); + + // Note that mid will always be strictly less than high (i.e. it will be a valid array index) + // because Math.average rounds towards zero (it does integer division with truncation). + if (unsafeMemoryAccess(array, mid) < element) { + // this cannot overflow because mid < high + unchecked { + low = mid + 1; + } + } else { + high = mid; + } + } + + return low; + } + + /** + * @dev Same as {upperBound}, but with an array in memory. + */ + function upperBoundMemory(uint256[] memory array, uint256 element) internal pure returns (uint256) { + uint256 low = 0; + uint256 high = array.length; + + if (high == 0) { + return 0; + } + + while (low < high) { + uint256 mid = Math.average(low, high); + + // Note that mid will always be strictly less than high (i.e. it will be a valid array index) + // because Math.average rounds towards zero (it does integer division with truncation). + if (unsafeMemoryAccess(array, mid) > element) { + high = mid; + } else { + // this cannot overflow because mid < high + unchecked { + low = mid + 1; + } + } + } + + return low; + } + /** * @dev Access an array in an "unsafe" way. Skips solidity "index-out-of-range" check. * diff --git a/test/utils/Arrays.test.js b/test/utils/Arrays.test.js index c585fee58e8..dc35e49447f 100644 --- a/test/utils/Arrays.test.js +++ b/test/utils/Arrays.test.js @@ -4,22 +4,22 @@ const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); const { randomArray, generators } = require('../helpers/random'); -// See https://en.cppreference.com/w/cpp/algorithm/ranges/lower_bound +// See https://en.cppreference.com/w/cpp/algorithm/lower_bound const lowerBound = (array, value) => { const i = array.findIndex(element => value <= element); return i == -1 ? array.length : i; }; // See https://en.cppreference.com/w/cpp/algorithm/upper_bound -// const upperBound = (array, value) => { -// const i = array.findIndex(element => value < element); -// return i == -1 ? array.length : i; -// }; +const upperBound = (array, value) => { + const i = array.findIndex(element => value < element); + return i == -1 ? array.length : i; +}; const hasDuplicates = array => array.some((v, i) => array.indexOf(v) != i); describe('Arrays', function () { - describe('findUpperBound', function () { + describe('search', function () { for (const [title, { array, tests }] of Object.entries({ 'Even number of elements': { array: [11n, 12n, 13n, 14n, 15n, 16n, 17n, 18n, 19n, 20n], @@ -82,10 +82,25 @@ describe('Arrays', function () { }); for (const [name, input] of Object.entries(tests)) { - it(name, async function () { - // findUpperBound does not support duplicated - if (hasDuplicates(array)) this.skip(); - expect(await this.mock.findUpperBound(input)).to.equal(lowerBound(array, input)); + describe(name, function () { + it('[deprecated] findUpperBound', async function () { + // findUpperBound does not support duplicated + if (hasDuplicates(array)) { + expect(await this.mock.findUpperBound(input)).to.be.equal(upperBound(array, input) - 1); + } else { + expect(await this.mock.findUpperBound(input)).to.be.equal(lowerBound(array, input)); + } + }); + + it('lowerBound', async function () { + expect(await this.mock.lowerBound(input)).to.be.equal(lowerBound(array, input)); + expect(await this.mock.lowerBoundMemory(array, input)).to.be.equal(lowerBound(array, input)); + }); + + it('upperBound', async function () { + expect(await this.mock.upperBound(input)).to.be.equal(upperBound(array, input)); + expect(await this.mock.upperBoundMemory(array, input)).to.be.equal(upperBound(array, input)); + }); }); } }); @@ -93,29 +108,29 @@ describe('Arrays', function () { }); describe('unsafeAccess', function () { - const contractCases = { + for (const [title, { artifact, elements }] of Object.entries({ address: { artifact: 'AddressArraysMock', elements: randomArray(generators.address, 10) }, bytes32: { artifact: 'Bytes32ArraysMock', elements: randomArray(generators.bytes32, 10) }, uint256: { artifact: 'Uint256ArraysMock', elements: randomArray(generators.uint256, 10) }, - }; - - const fixture = async () => { - const contracts = {}; - for (const [name, { artifact, elements }] of Object.entries(contractCases)) { - contracts[name] = await ethers.deployContract(artifact, [elements]); - } - return { contracts }; - }; + })) { + describe(title, function () { + const fixture = async () => { + return { mock: await ethers.deployContract(artifact, [elements]) }; + }; - beforeEach(async function () { - Object.assign(this, await loadFixture(fixture)); - }); + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); - for (const [name, { elements }] of Object.entries(contractCases)) { - it(name, async function () { for (const i in elements) { - expect(await this.contracts[name].unsafeAccess(i)).to.equal(elements[i]); + it(`unsafeAccess within bounds #${i}`, async function () { + expect(await this.mock.unsafeAccess(i)).to.equal(elements[i]); + }); } + + it('unsafeAccess outside bounds', async function () { + await expect(this.mock.unsafeAccess(elements.length)).to.not.be.rejected; + }); }); } }); diff --git a/test/utils/structs/Checkpoints.test.js b/test/utils/structs/Checkpoints.test.js index 9458c486ab8..fd22544b955 100644 --- a/test/utils/structs/Checkpoints.test.js +++ b/test/utils/structs/Checkpoints.test.js @@ -4,8 +4,6 @@ const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); const { VALUE_SIZES } = require('../../../scripts/generate/templates/Checkpoints.opts'); -const last = array => (array.length ? array[array.length - 1] : undefined); - describe('Checkpoints', function () { for (const length of VALUE_SIZES) { describe(`Trace${length}`, function () { @@ -81,7 +79,7 @@ describe('Checkpoints', function () { it('returns latest value', async function () { const latest = this.checkpoints.at(-1); expect(await this.methods.latest()).to.equal(latest.value); - expect(await this.methods.latestCheckpoint()).to.have.ordered.members([true, latest.key, latest.value]); + expect(await this.methods.latestCheckpoint()).to.deep.equal([true, latest.key, latest.value]); }); it('cannot push values in the past', async function () { @@ -115,7 +113,7 @@ describe('Checkpoints', function () { it('upper lookup & upperLookupRecent', async function () { for (let i = 0; i < 14; ++i) { - const value = last(this.checkpoints.filter(x => i >= x.key))?.value || 0n; + const value = this.checkpoints.findLast(x => i >= x.key)?.value || 0n; expect(await this.methods.upperLookup(i)).to.equal(value); expect(await this.methods.upperLookupRecent(i)).to.equal(value); @@ -137,7 +135,7 @@ describe('Checkpoints', function () { } for (let i = 0; i < 25; ++i) { - const value = last(allCheckpoints.filter(x => i >= x.key))?.value || 0n; + const value = allCheckpoints.findLast(x => i >= x.key)?.value || 0n; expect(await this.methods.upperLookup(i)).to.equal(value); expect(await this.methods.upperLookupRecent(i)).to.equal(value); } From 7eba10dd1e4899f5ca93e3ec3a0b3dafff9b1f03 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 30 Jan 2024 19:58:35 +0100 Subject: [PATCH 3/4] Move ERC721 and ERC1155 receiver checks to dedicate libraries (#4845) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ernesto García --- .changeset/poor-chefs-cheat.md | 5 + contracts/token/ERC1155/ERC1155.sol | 72 +---- .../token/ERC1155/utils/ERC1155Utils.sol | 87 +++++ contracts/token/ERC721/ERC721.sol | 36 +-- contracts/token/ERC721/utils/ERC721Utils.sol | 48 +++ test/token/ERC1155/utils/ERC1155Utils.test.js | 299 ++++++++++++++++++ test/token/ERC721/utils/ERC721Utils.test.js | 94 ++++++ 7 files changed, 540 insertions(+), 101 deletions(-) create mode 100644 .changeset/poor-chefs-cheat.md create mode 100644 contracts/token/ERC1155/utils/ERC1155Utils.sol create mode 100644 contracts/token/ERC721/utils/ERC721Utils.sol create mode 100644 test/token/ERC1155/utils/ERC1155Utils.test.js create mode 100644 test/token/ERC721/utils/ERC721Utils.test.js diff --git a/.changeset/poor-chefs-cheat.md b/.changeset/poor-chefs-cheat.md new file mode 100644 index 00000000000..39db3d5139c --- /dev/null +++ b/.changeset/poor-chefs-cheat.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`ERC721Utils` and `ERC1155Utils`: Add reusable libraries with functions to perform acceptance checks on `IERC721Receiver` and `IERC1155Receiver` implementers. diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index 2ec3ef7be86..5e05e4791dd 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/contracts/token/ERC1155/ERC1155.sol @@ -4,8 +4,8 @@ pragma solidity ^0.8.20; import {IERC1155} from "./IERC1155.sol"; -import {IERC1155Receiver} from "./IERC1155Receiver.sol"; import {IERC1155MetadataURI} from "./extensions/IERC1155MetadataURI.sol"; +import {ERC1155Utils} from "./utils/ERC1155Utils.sol"; import {Context} from "../../utils/Context.sol"; import {IERC165, ERC165} from "../../utils/introspection/ERC165.sol"; import {Arrays} from "../../utils/Arrays.sol"; @@ -203,9 +203,9 @@ abstract contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI, IER if (ids.length == 1) { uint256 id = ids.unsafeMemoryAccess(0); uint256 value = values.unsafeMemoryAccess(0); - _doSafeTransferAcceptanceCheck(operator, from, to, id, value, data); + ERC1155Utils.checkOnERC1155Received(operator, from, to, id, value, data); } else { - _doSafeBatchTransferAcceptanceCheck(operator, from, to, ids, values, data); + ERC1155Utils.checkOnERC1155BatchReceived(operator, from, to, ids, values, data); } } } @@ -374,72 +374,6 @@ abstract contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI, IER emit ApprovalForAll(owner, operator, approved); } - /** - * @dev Performs an acceptance check by calling {IERC1155-onERC1155Received} on the `to` address - * if it contains code at the moment of execution. - */ - function _doSafeTransferAcceptanceCheck( - address operator, - address from, - address to, - uint256 id, - uint256 value, - bytes memory data - ) private { - if (to.code.length > 0) { - try IERC1155Receiver(to).onERC1155Received(operator, from, id, value, data) returns (bytes4 response) { - if (response != IERC1155Receiver.onERC1155Received.selector) { - // Tokens rejected - revert ERC1155InvalidReceiver(to); - } - } catch (bytes memory reason) { - if (reason.length == 0) { - // non-IERC1155Receiver implementer - revert ERC1155InvalidReceiver(to); - } else { - /// @solidity memory-safe-assembly - assembly { - revert(add(32, reason), mload(reason)) - } - } - } - } - } - - /** - * @dev Performs a batch acceptance check by calling {IERC1155-onERC1155BatchReceived} on the `to` address - * if it contains code at the moment of execution. - */ - function _doSafeBatchTransferAcceptanceCheck( - address operator, - address from, - address to, - uint256[] memory ids, - uint256[] memory values, - bytes memory data - ) private { - if (to.code.length > 0) { - try IERC1155Receiver(to).onERC1155BatchReceived(operator, from, ids, values, data) returns ( - bytes4 response - ) { - if (response != IERC1155Receiver.onERC1155BatchReceived.selector) { - // Tokens rejected - revert ERC1155InvalidReceiver(to); - } - } catch (bytes memory reason) { - if (reason.length == 0) { - // non-IERC1155Receiver implementer - revert ERC1155InvalidReceiver(to); - } else { - /// @solidity memory-safe-assembly - assembly { - revert(add(32, reason), mload(reason)) - } - } - } - } - } - /** * @dev Creates an array in memory with only one value for each of the elements provided. */ diff --git a/contracts/token/ERC1155/utils/ERC1155Utils.sol b/contracts/token/ERC1155/utils/ERC1155Utils.sol new file mode 100644 index 00000000000..0ff2bf146a4 --- /dev/null +++ b/contracts/token/ERC1155/utils/ERC1155Utils.sol @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {IERC1155Receiver} from "../IERC1155Receiver.sol"; +import {IERC1155Errors} from "../../../interfaces/draft-IERC6093.sol"; + +/** + * @dev Library that provide common ERC-1155 utility functions. + * + * See https://eips.ethereum.org/EIPS/eip-1155[ERC-1155]. + */ +library ERC1155Utils { + /** + * @dev Performs an acceptance check for the provided `operator` by calling {IERC1155-onERC1155Received} + * on the `to` address. The `operator` is generally the address that initiated the token transfer (i.e. `msg.sender`). + * + * The acceptance call is not executed and treated as a no-op if the target address is doesn't contain code (i.e. an EOA). + * Otherwise, the recipient must implement {IERC1155Receiver-onERC1155Received} and return the acceptance magic value to accept + * the transfer. + */ + function checkOnERC1155Received( + address operator, + address from, + address to, + uint256 id, + uint256 value, + bytes memory data + ) internal { + if (to.code.length > 0) { + try IERC1155Receiver(to).onERC1155Received(operator, from, id, value, data) returns (bytes4 response) { + if (response != IERC1155Receiver.onERC1155Received.selector) { + // Tokens rejected + revert IERC1155Errors.ERC1155InvalidReceiver(to); + } + } catch (bytes memory reason) { + if (reason.length == 0) { + // non-IERC1155Receiver implementer + revert IERC1155Errors.ERC1155InvalidReceiver(to); + } else { + /// @solidity memory-safe-assembly + assembly { + revert(add(32, reason), mload(reason)) + } + } + } + } + } + + /** + * @dev Performs a batch acceptance check for the provided `operator` by calling {IERC1155-onERC1155BatchReceived} + * on the `to` address. The `operator` is generally the address that initiated the token transfer (i.e. `msg.sender`). + * + * The acceptance call is not executed and treated as a no-op if the target address is doesn't contain code (i.e. an EOA). + * Otherwise, the recipient must implement {IERC1155Receiver-onERC1155Received} and return the acceptance magic value to accept + * the transfer. + */ + function checkOnERC1155BatchReceived( + address operator, + address from, + address to, + uint256[] memory ids, + uint256[] memory values, + bytes memory data + ) internal { + if (to.code.length > 0) { + try IERC1155Receiver(to).onERC1155BatchReceived(operator, from, ids, values, data) returns ( + bytes4 response + ) { + if (response != IERC1155Receiver.onERC1155BatchReceived.selector) { + // Tokens rejected + revert IERC1155Errors.ERC1155InvalidReceiver(to); + } + } catch (bytes memory reason) { + if (reason.length == 0) { + // non-IERC1155Receiver implementer + revert IERC1155Errors.ERC1155InvalidReceiver(to); + } else { + /// @solidity memory-safe-assembly + assembly { + revert(add(32, reason), mload(reason)) + } + } + } + } + } +} diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index 1b38f06814e..62ff1def969 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -4,8 +4,8 @@ pragma solidity ^0.8.20; import {IERC721} from "./IERC721.sol"; -import {IERC721Receiver} from "./IERC721Receiver.sol"; import {IERC721Metadata} from "./extensions/IERC721Metadata.sol"; +import {ERC721Utils} from "./utils/ERC721Utils.sol"; import {Context} from "../../utils/Context.sol"; import {Strings} from "../../utils/Strings.sol"; import {IERC165, ERC165} from "../../utils/introspection/ERC165.sol"; @@ -158,7 +158,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er */ function safeTransferFrom(address from, address to, uint256 tokenId, bytes memory data) public virtual { transferFrom(from, to, tokenId); - _checkOnERC721Received(from, to, tokenId, data); + ERC721Utils.checkOnERC721Received(_msgSender(), from, to, tokenId, data); } /** @@ -311,7 +311,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er */ function _safeMint(address to, uint256 tokenId, bytes memory data) internal virtual { _mint(to, tokenId); - _checkOnERC721Received(address(0), to, tokenId, data); + ERC721Utils.checkOnERC721Received(_msgSender(), address(0), to, tokenId, data); } /** @@ -384,7 +384,7 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er */ function _safeTransfer(address from, address to, uint256 tokenId, bytes memory data) internal virtual { _transfer(from, to, tokenId); - _checkOnERC721Received(from, to, tokenId, data); + ERC721Utils.checkOnERC721Received(_msgSender(), from, to, tokenId, data); } /** @@ -452,32 +452,4 @@ abstract contract ERC721 is Context, ERC165, IERC721, IERC721Metadata, IERC721Er } return owner; } - - /** - * @dev Private function to invoke {IERC721Receiver-onERC721Received} on a target address. This will revert if the - * recipient doesn't accept the token transfer. The call is not executed if the target address is not a contract. - * - * @param from address representing the previous owner of the given token ID - * @param to target address that will receive the tokens - * @param tokenId uint256 ID of the token to be transferred - * @param data bytes optional data to send along with the call - */ - function _checkOnERC721Received(address from, address to, uint256 tokenId, bytes memory data) private { - if (to.code.length > 0) { - try IERC721Receiver(to).onERC721Received(_msgSender(), from, tokenId, data) returns (bytes4 retval) { - if (retval != IERC721Receiver.onERC721Received.selector) { - revert ERC721InvalidReceiver(to); - } - } catch (bytes memory reason) { - if (reason.length == 0) { - revert ERC721InvalidReceiver(to); - } else { - /// @solidity memory-safe-assembly - assembly { - revert(add(32, reason), mload(reason)) - } - } - } - } - } } diff --git a/contracts/token/ERC721/utils/ERC721Utils.sol b/contracts/token/ERC721/utils/ERC721Utils.sol new file mode 100644 index 00000000000..b923e403736 --- /dev/null +++ b/contracts/token/ERC721/utils/ERC721Utils.sol @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +import {IERC721Receiver} from "../IERC721Receiver.sol"; +import {IERC721Errors} from "../../../interfaces/draft-IERC6093.sol"; + +/** + * @dev Library that provide common ERC-721 utility functions. + * + * See https://eips.ethereum.org/EIPS/eip-721[ERC-721]. + */ +library ERC721Utils { + /** + * @dev Performs an acceptance check for the provided `operator` by calling {IERC721-onERC721Received} + * on the `to` address. The `operator` is generally the address that initiated the token transfer (i.e. `msg.sender`). + * + * The acceptance call is not executed and treated as a no-op if the target address is doesn't contain code (i.e. an EOA). + * Otherwise, the recipient must implement {IERC721Receiver-onERC721Received} and return the acceptance magic value to accept + * the transfer. + */ + function checkOnERC721Received( + address operator, + address from, + address to, + uint256 tokenId, + bytes memory data + ) internal { + if (to.code.length > 0) { + try IERC721Receiver(to).onERC721Received(operator, from, tokenId, data) returns (bytes4 retval) { + if (retval != IERC721Receiver.onERC721Received.selector) { + // Token rejected + revert IERC721Errors.ERC721InvalidReceiver(to); + } + } catch (bytes memory reason) { + if (reason.length == 0) { + // non-IERC721Receiver implementer + revert IERC721Errors.ERC721InvalidReceiver(to); + } else { + /// @solidity memory-safe-assembly + assembly { + revert(add(32, reason), mload(reason)) + } + } + } + } + } +} diff --git a/test/token/ERC1155/utils/ERC1155Utils.test.js b/test/token/ERC1155/utils/ERC1155Utils.test.js new file mode 100644 index 00000000000..5687568d341 --- /dev/null +++ b/test/token/ERC1155/utils/ERC1155Utils.test.js @@ -0,0 +1,299 @@ +const { ethers } = require('hardhat'); +const { expect } = require('chai'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); +const { RevertType } = require('../../../helpers/enums'); +const { PANIC_CODES } = require('@nomicfoundation/hardhat-chai-matchers/panic'); + +const firstTokenId = 1n; +const secondTokenId = 2n; +const firstTokenValue = 1000n; +const secondTokenValue = 1000n; + +const RECEIVER_SINGLE_MAGIC_VALUE = '0xf23a6e61'; +const RECEIVER_BATCH_MAGIC_VALUE = '0xbc197c81'; + +const deployReceiver = ( + revertType, + returnValueSingle = RECEIVER_SINGLE_MAGIC_VALUE, + returnValueBatched = RECEIVER_BATCH_MAGIC_VALUE, +) => ethers.deployContract('$ERC1155ReceiverMock', [returnValueSingle, returnValueBatched, revertType]); + +const fixture = async () => { + const [eoa, operator, owner] = await ethers.getSigners(); + const utils = await ethers.deployContract('$ERC1155Utils'); + + const receivers = { + correct: await deployReceiver(RevertType.None), + invalid: await deployReceiver(RevertType.None, '0xdeadbeef', '0xdeadbeef'), + message: await deployReceiver(RevertType.RevertWithMessage), + empty: await deployReceiver(RevertType.RevertWithoutMessage), + customError: await deployReceiver(RevertType.RevertWithCustomError), + panic: await deployReceiver(RevertType.Panic), + nonReceiver: await ethers.deployContract('CallReceiverMock'), + eoa, + }; + + return { operator, owner, utils, receivers }; +}; + +describe('ERC1155Utils', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + describe('onERC1155Received', function () { + it('succeeds when called by an EOA', async function () { + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.eoa, + firstTokenId, + firstTokenValue, + '0x', + ), + ).to.not.be.reverted; + }); + + it('succeeds when data is passed', async function () { + const data = '0x12345678'; + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.correct, + firstTokenId, + firstTokenValue, + data, + ), + ).to.not.be.reverted; + }); + + it('succeeds when data is empty', async function () { + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.correct, + firstTokenId, + firstTokenValue, + '0x', + ), + ).to.not.be.reverted; + }); + + it('reverts when receiver returns invalid value', async function () { + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.invalid, + firstTokenId, + firstTokenValue, + '0x', + ), + ) + .to.be.revertedWithCustomError(this.utils, 'ERC1155InvalidReceiver') + .withArgs(this.receivers.invalid); + }); + + it('reverts when receiver reverts with message', async function () { + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.message, + firstTokenId, + firstTokenValue, + '0x', + ), + ).to.be.revertedWith('ERC1155ReceiverMock: reverting on receive'); + }); + + it('reverts when receiver reverts without message', async function () { + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.empty, + firstTokenId, + firstTokenValue, + '0x', + ), + ) + .to.be.revertedWithCustomError(this.utils, 'ERC1155InvalidReceiver') + .withArgs(this.receivers.empty); + }); + + it('reverts when receiver reverts with custom error', async function () { + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.customError, + firstTokenId, + firstTokenValue, + '0x', + ), + ) + .to.be.revertedWithCustomError(this.receivers.customError, 'CustomError') + .withArgs(RECEIVER_SINGLE_MAGIC_VALUE); + }); + + it('reverts when receiver panics', async function () { + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.panic, + firstTokenId, + firstTokenValue, + '0x', + ), + ).to.be.revertedWithPanic(PANIC_CODES.DIVISION_BY_ZERO); + }); + + it('reverts when receiver does not implement onERC1155Received', async function () { + await expect( + this.utils.$checkOnERC1155Received( + this.operator, + this.owner, + this.receivers.nonReceiver, + firstTokenId, + firstTokenValue, + '0x', + ), + ) + .to.be.revertedWithCustomError(this.utils, 'ERC1155InvalidReceiver') + .withArgs(this.receivers.nonReceiver); + }); + }); + + describe('onERC1155BatchReceived', function () { + it('succeeds when called by an EOA', async function () { + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.eoa, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + '0x', + ), + ).to.not.be.reverted; + }); + + it('succeeds when data is passed', async function () { + const data = '0x12345678'; + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.correct, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + data, + ), + ).to.not.be.reverted; + }); + + it('succeeds when data is empty', async function () { + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.correct, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + '0x', + ), + ).to.not.be.reverted; + }); + + it('reverts when receiver returns invalid value', async function () { + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.invalid, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + '0x', + ), + ) + .to.be.revertedWithCustomError(this.utils, 'ERC1155InvalidReceiver') + .withArgs(this.receivers.invalid); + }); + + it('reverts when receiver reverts with message', async function () { + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.message, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + '0x', + ), + ).to.be.revertedWith('ERC1155ReceiverMock: reverting on batch receive'); + }); + + it('reverts when receiver reverts without message', async function () { + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.empty, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + '0x', + ), + ) + .to.be.revertedWithCustomError(this.utils, 'ERC1155InvalidReceiver') + .withArgs(this.receivers.empty); + }); + + it('reverts when receiver reverts with custom error', async function () { + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.customError, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + '0x', + ), + ) + .to.be.revertedWithCustomError(this.receivers.customError, 'CustomError') + .withArgs(RECEIVER_SINGLE_MAGIC_VALUE); + }); + + it('reverts when receiver panics', async function () { + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.panic, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + '0x', + ), + ).to.be.revertedWithPanic(PANIC_CODES.DIVISION_BY_ZERO); + }); + + it('reverts when receiver does not implement onERC1155BatchReceived', async function () { + await expect( + this.utils.$checkOnERC1155BatchReceived( + this.operator, + this.owner, + this.receivers.nonReceiver, + [firstTokenId, secondTokenId], + [firstTokenValue, secondTokenValue], + '0x', + ), + ) + .to.be.revertedWithCustomError(this.utils, 'ERC1155InvalidReceiver') + .withArgs(this.receivers.nonReceiver); + }); + }); +}); diff --git a/test/token/ERC721/utils/ERC721Utils.test.js b/test/token/ERC721/utils/ERC721Utils.test.js new file mode 100644 index 00000000000..2327d1ac7da --- /dev/null +++ b/test/token/ERC721/utils/ERC721Utils.test.js @@ -0,0 +1,94 @@ +const { ethers } = require('hardhat'); +const { expect } = require('chai'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); +const { RevertType } = require('../../../helpers/enums'); +const { PANIC_CODES } = require('@nomicfoundation/hardhat-chai-matchers/panic'); + +const tokenId = 1n; + +const RECEIVER_MAGIC_VALUE = '0x150b7a02'; + +const deployReceiver = (revertType, returnValue = RECEIVER_MAGIC_VALUE) => + ethers.deployContract('$ERC721ReceiverMock', [returnValue, revertType]); + +const fixture = async () => { + const [eoa, operator, owner] = await ethers.getSigners(); + const utils = await ethers.deployContract('$ERC721Utils'); + + const receivers = { + correct: await deployReceiver(RevertType.None), + invalid: await deployReceiver(RevertType.None, '0xdeadbeef'), + message: await deployReceiver(RevertType.RevertWithMessage), + empty: await deployReceiver(RevertType.RevertWithoutMessage), + customError: await deployReceiver(RevertType.RevertWithCustomError), + panic: await deployReceiver(RevertType.Panic), + nonReceiver: await ethers.deployContract('CallReceiverMock'), + eoa, + }; + + return { operator, owner, utils, receivers }; +}; + +describe('ERC721Utils', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + describe('onERC721Received', function () { + it('succeeds when called by an EOA', async function () { + await expect(this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.eoa, tokenId, '0x')).to + .not.be.reverted; + }); + + it('succeeds when data is passed', async function () { + const data = '0x12345678'; + await expect(this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.correct, tokenId, data)) + .to.not.be.reverted; + }); + + it('succeeds when data is empty', async function () { + await expect(this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.correct, tokenId, '0x')) + .to.not.be.reverted; + }); + + it('reverts when receiver returns invalid value', async function () { + await expect(this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.invalid, tokenId, '0x')) + .to.be.revertedWithCustomError(this.utils, 'ERC721InvalidReceiver') + .withArgs(this.receivers.invalid); + }); + + it('reverts when receiver reverts with message', async function () { + await expect( + this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.message, tokenId, '0x'), + ).to.be.revertedWith('ERC721ReceiverMock: reverting'); + }); + + it('reverts when receiver reverts without message', async function () { + await expect(this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.empty, tokenId, '0x')) + .to.be.revertedWithCustomError(this.utils, 'ERC721InvalidReceiver') + .withArgs(this.receivers.empty); + }); + + it('reverts when receiver reverts with custom error', async function () { + await expect( + this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.customError, tokenId, '0x'), + ) + .to.be.revertedWithCustomError(this.receivers.customError, 'CustomError') + .withArgs(RECEIVER_MAGIC_VALUE); + }); + + it('reverts when receiver panics', async function () { + await expect( + this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.panic, tokenId, '0x'), + ).to.be.revertedWithPanic(PANIC_CODES.DIVISION_BY_ZERO); + }); + + it('reverts when receiver does not implement onERC721Received', async function () { + await expect( + this.utils.$checkOnERC721Received(this.operator, this.owner, this.receivers.nonReceiver, tokenId, '0x'), + ) + .to.be.revertedWithCustomError(this.utils, 'ERC721InvalidReceiver') + .withArgs(this.receivers.nonReceiver); + }); + }); +}); From cc431f53e03cffb71a0d08c91f934e3251807eb6 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 1 Feb 2024 22:36:31 +0100 Subject: [PATCH 4/4] Remove deprecated Truffle code (#4868) --- hardhat/env-artifacts.js | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/hardhat/env-artifacts.js b/hardhat/env-artifacts.js index 4cda9387cc3..e97ae6468e3 100644 --- a/hardhat/env-artifacts.js +++ b/hardhat/env-artifacts.js @@ -10,23 +10,6 @@ function isExpectedError(e, suffix) { extendEnvironment(hre => { const suffixes = ['UpgradeableWithInit', 'Upgradeable', '']; - // Truffe (deprecated) - const originalRequire = hre.artifacts.require; - hre.artifacts.require = function (name) { - for (const suffix of suffixes) { - try { - return originalRequire.call(this, name + suffix); - } catch (e) { - if (isExpectedError(e, suffix)) { - continue; - } else { - throw e; - } - } - } - throw new Error('Unreachable'); - }; - // Ethers const originalReadArtifact = hre.artifacts.readArtifact; hre.artifacts.readArtifact = async function (name) {