From 1a0fa7974b493258c7fc8f0708c442ed548e227e Mon Sep 17 00:00:00 2001 From: yonada Date: Wed, 3 Jan 2024 15:01:21 +0000 Subject: [PATCH] fix(world,world-modules): requireInterface correctly specifies ERC165 [M-02] (#2016) --- .changeset/fuzzy-wombats-dream.md | 5 + packages/world-modules/gas-report.json | 28 +++--- packages/world/gas-report.json | 2 +- packages/world/src/ERC165Checker.sol | 121 ++++++++++++++++++++++++ packages/world/src/requireInterface.sol | 7 +- 5 files changed, 143 insertions(+), 20 deletions(-) create mode 100644 .changeset/fuzzy-wombats-dream.md create mode 100644 packages/world/src/ERC165Checker.sol diff --git a/.changeset/fuzzy-wombats-dream.md b/.changeset/fuzzy-wombats-dream.md new file mode 100644 index 0000000000..6e789d5e94 --- /dev/null +++ b/.changeset/fuzzy-wombats-dream.md @@ -0,0 +1,5 @@ +--- +"@latticexyz/world": patch +--- + +Fixed `requireInterface` to correctly specify ERC165. diff --git a/packages/world-modules/gas-report.json b/packages/world-modules/gas-report.json index ab8a8fb3dc..7faafd63a2 100644 --- a/packages/world-modules/gas-report.json +++ b/packages/world-modules/gas-report.json @@ -75,13 +75,13 @@ "file": "test/KeysInTableModule.t.sol", "test": "testInstallComposite", "name": "install keys in table module", - "gasUsed": 1409529 + "gasUsed": 1413004 }, { "file": "test/KeysInTableModule.t.sol", "test": "testInstallGas", "name": "install keys in table module", - "gasUsed": 1409529 + "gasUsed": 1413004 }, { "file": "test/KeysInTableModule.t.sol", @@ -93,13 +93,13 @@ "file": "test/KeysInTableModule.t.sol", "test": "testInstallSingleton", "name": "install keys in table module", - "gasUsed": 1409529 + "gasUsed": 1413004 }, { "file": "test/KeysInTableModule.t.sol", "test": "testSetAndDeleteRecordHookCompositeGas", "name": "install keys in table module", - "gasUsed": 1409529 + "gasUsed": 1413004 }, { "file": "test/KeysInTableModule.t.sol", @@ -117,7 +117,7 @@ "file": "test/KeysInTableModule.t.sol", "test": "testSetAndDeleteRecordHookGas", "name": "install keys in table module", - "gasUsed": 1409529 + "gasUsed": 1413004 }, { "file": "test/KeysInTableModule.t.sol", @@ -135,7 +135,7 @@ "file": "test/KeysWithValueModule.t.sol", "test": "testGetKeysWithValueGas", "name": "install keys with value module", - "gasUsed": 650796 + "gasUsed": 654271 }, { "file": "test/KeysWithValueModule.t.sol", @@ -153,7 +153,7 @@ "file": "test/KeysWithValueModule.t.sol", "test": "testInstall", "name": "install keys with value module", - "gasUsed": 650796 + "gasUsed": 654271 }, { "file": "test/KeysWithValueModule.t.sol", @@ -165,7 +165,7 @@ "file": "test/KeysWithValueModule.t.sol", "test": "testSetAndDeleteRecordHook", "name": "install keys with value module", - "gasUsed": 650796 + "gasUsed": 654271 }, { "file": "test/KeysWithValueModule.t.sol", @@ -183,7 +183,7 @@ "file": "test/KeysWithValueModule.t.sol", "test": "testSetField", "name": "install keys with value module", - "gasUsed": 650796 + "gasUsed": 654271 }, { "file": "test/KeysWithValueModule.t.sol", @@ -267,7 +267,7 @@ "file": "test/StandardDelegationsModule.t.sol", "test": "testCallFromCallboundDelegation", "name": "register a callbound delegation", - "gasUsed": 117426 + "gasUsed": 119215 }, { "file": "test/StandardDelegationsModule.t.sol", @@ -279,7 +279,7 @@ "file": "test/StandardDelegationsModule.t.sol", "test": "testCallFromSystemDelegation", "name": "register a systembound delegation", - "gasUsed": 114982 + "gasUsed": 116771 }, { "file": "test/StandardDelegationsModule.t.sol", @@ -291,7 +291,7 @@ "file": "test/StandardDelegationsModule.t.sol", "test": "testCallFromTimeboundDelegation", "name": "register a timebound delegation", - "gasUsed": 111920 + "gasUsed": 113709 }, { "file": "test/StandardDelegationsModule.t.sol", @@ -303,7 +303,7 @@ "file": "test/UniqueEntityModule.t.sol", "test": "testInstall", "name": "install unique entity module", - "gasUsed": 679102 + "gasUsed": 682567 }, { "file": "test/UniqueEntityModule.t.sol", @@ -315,7 +315,7 @@ "file": "test/UniqueEntityModule.t.sol", "test": "testInstallRoot", "name": "installRoot unique entity module", - "gasUsed": 644539 + "gasUsed": 648044 }, { "file": "test/UniqueEntityModule.t.sol", diff --git a/packages/world/gas-report.json b/packages/world/gas-report.json index 007eafe3d9..5dd330ca54 100644 --- a/packages/world/gas-report.json +++ b/packages/world/gas-report.json @@ -111,7 +111,7 @@ "file": "test/World.t.sol", "test": "testRegisterSystem", "name": "register a system", - "gasUsed": 162594 + "gasUsed": 164238 }, { "file": "test/World.t.sol", diff --git a/packages/world/src/ERC165Checker.sol b/packages/world/src/ERC165Checker.sol new file mode 100644 index 0000000000..670db7933d --- /dev/null +++ b/packages/world/src/ERC165Checker.sol @@ -0,0 +1,121 @@ +// SPDX-License-Identifier: MIT +// OpenZeppelin Contracts (last updated v5.0.0) (utils/introspection/ERC165Checker.sol) + +pragma solidity ^0.8.20; + +import { IERC165 } from "./IERC165.sol"; + +/** + * @dev Library used to query support of an interface declared via {IERC165}. + * + * Note that these functions return the actual result of the query: they do not + * `revert` if an interface is not supported. It is up to the caller to decide + * what to do in these cases. + */ +library ERC165Checker { + // As per the EIP-165 spec, no interface should ever match 0xffffffff + bytes4 private constant INTERFACE_ID_INVALID = 0xffffffff; + + /** + * @dev Returns true if `account` supports the {IERC165} interface. + */ + function supportsERC165(address account) internal view returns (bool) { + // Any contract that implements ERC165 must explicitly indicate support of + // InterfaceId_ERC165 and explicitly indicate non-support of InterfaceId_Invalid + return + supportsERC165InterfaceUnchecked(account, type(IERC165).interfaceId) && + !supportsERC165InterfaceUnchecked(account, INTERFACE_ID_INVALID); + } + + /** + * @dev Returns true if `account` supports the interface defined by + * `interfaceId`. Support for {IERC165} itself is queried automatically. + * + * See {IERC165-supportsInterface}. + */ + function supportsInterface(address account, bytes4 interfaceId) internal view returns (bool) { + // query support of both ERC165 as per the spec and support of _interfaceId + return supportsERC165(account) && supportsERC165InterfaceUnchecked(account, interfaceId); + } + + /** + * @dev Returns a boolean array where each value corresponds to the + * interfaces passed in and whether they're supported or not. This allows + * you to batch check interfaces for a contract where your expectation + * is that some interfaces may not be supported. + * + * See {IERC165-supportsInterface}. + */ + function getSupportedInterfaces(address account, bytes4[] memory interfaceIds) internal view returns (bool[] memory) { + // an array of booleans corresponding to interfaceIds and whether they're supported or not + bool[] memory interfaceIdsSupported = new bool[](interfaceIds.length); + + // query support of ERC165 itself + if (supportsERC165(account)) { + // query support of each interface in interfaceIds + for (uint256 i = 0; i < interfaceIds.length; i++) { + interfaceIdsSupported[i] = supportsERC165InterfaceUnchecked(account, interfaceIds[i]); + } + } + + return interfaceIdsSupported; + } + + /** + * @dev Returns true if `account` supports all the interfaces defined in + * `interfaceIds`. Support for {IERC165} itself is queried automatically. + * + * Batch-querying can lead to gas savings by skipping repeated checks for + * {IERC165} support. + * + * See {IERC165-supportsInterface}. + */ + function supportsAllInterfaces(address account, bytes4[] memory interfaceIds) internal view returns (bool) { + // query support of ERC165 itself + if (!supportsERC165(account)) { + return false; + } + + // query support of each interface in interfaceIds + for (uint256 i = 0; i < interfaceIds.length; i++) { + if (!supportsERC165InterfaceUnchecked(account, interfaceIds[i])) { + return false; + } + } + + // all interfaces supported + return true; + } + + /** + * @notice Query if a contract implements an interface, does not check ERC165 support + * @param account The address of the contract to query for support of an interface + * @param interfaceId The interface identifier, as specified in ERC-165 + * @return true if the contract at account indicates support of the interface with + * identifier interfaceId, false otherwise + * @dev Assumes that account contains a contract that supports ERC165, otherwise + * the behavior of this method is undefined. This precondition can be checked + * with {supportsERC165}. + * + * Some precompiled contracts will falsely indicate support for a given interface, so caution + * should be exercised when using this function. + * + * Interface identification is specified in ERC-165. + */ + function supportsERC165InterfaceUnchecked(address account, bytes4 interfaceId) internal view returns (bool) { + // prepare call + bytes memory encodedParams = abi.encodeCall(IERC165.supportsInterface, (interfaceId)); + + // perform static call + bool success; + uint256 returnSize; + uint256 returnValue; + assembly { + success := staticcall(30000, account, add(encodedParams, 0x20), mload(encodedParams), 0x00, 0x20) + returnSize := returndatasize() + returnValue := mload(0x00) + } + + return success && returnSize >= 0x20 && returnValue > 0; + } +} diff --git a/packages/world/src/requireInterface.sol b/packages/world/src/requireInterface.sol index cb00609bfa..88c152e431 100644 --- a/packages/world/src/requireInterface.sol +++ b/packages/world/src/requireInterface.sol @@ -2,6 +2,7 @@ pragma solidity >=0.8.21; import { IERC165 } from "./IERC165.sol"; +import { ERC165Checker } from "./ERC165Checker.sol"; import { IWorldErrors } from "./IWorldErrors.sol"; /** @@ -17,11 +18,7 @@ import { IWorldErrors } from "./IWorldErrors.sol"; * @param interfaceId The interface ID to verify. */ function requireInterface(address contractAddress, bytes4 interfaceId) view { - try IERC165(contractAddress).supportsInterface(interfaceId) returns (bool supported) { - if (!supported) { - revert IWorldErrors.World_InterfaceNotSupported(contractAddress, interfaceId); - } - } catch { + if (!ERC165Checker.supportsInterface(contractAddress, interfaceId)) { revert IWorldErrors.World_InterfaceNotSupported(contractAddress, interfaceId); } }