diff --git a/CHANGELOG.md b/CHANGELOG.md index b526dade7b0..cc0ab752e2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ ### Deprecations * `EIP712`: Added the file `EIP712.sol` and deprecated `draft-EIP712.sol` since the EIP is no longer a Draft. Developers are encouraged to update their imports. ([#3621](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3621)) + * `Address.isContract`: `isContract` utilitiy function is deprecated from v5.0.0 because of its ambiguous nature. Reasons and discussions here: [#3417](https://github.com/OpenZeppelin/openzeppelin-contracts/issues/3417). ```diff -import "@openzeppelin/contracts/utils/cryptography/draft-EIP712.sol"; diff --git a/contracts/mocks/AddressImpl.sol b/contracts/mocks/AddressImpl.sol index 702093c7337..8dc7886c1fd 100644 --- a/contracts/mocks/AddressImpl.sol +++ b/contracts/mocks/AddressImpl.sol @@ -9,10 +9,6 @@ contract AddressImpl { event CallReturnValue(string data); - function isContract(address account) external view returns (bool) { - return Address.isContract(account); - } - function sendValue(address payable receiver, uint256 amount) external { Address.sendValue(receiver, amount); } diff --git a/contracts/mocks/UUPS/UUPSLegacy.sol b/contracts/mocks/UUPS/UUPSLegacy.sol index 550a6221b48..38a16c110df 100644 --- a/contracts/mocks/UUPS/UUPSLegacy.sol +++ b/contracts/mocks/UUPS/UUPSLegacy.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; +pragma solidity ^0.8.5; import "./UUPSUpgradeableMock.sol"; @@ -13,7 +13,7 @@ contract UUPSUpgradeableLegacyMock is UUPSUpgradeableMock { // ERC1967Upgrade._setImplementation is private so we reproduce it here. // An extra underscore prevents a name clash error. function __setImplementation(address newImplementation) private { - require(Address.isContract(newImplementation), "ERC1967: new implementation is not a contract"); + require(address(newImplementation).code.length > 0, "ERC1967: new implementation is not a contract"); StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value = newImplementation; } diff --git a/contracts/proxy/ERC1967/ERC1967Upgrade.sol b/contracts/proxy/ERC1967/ERC1967Upgrade.sol index 77fbdd16565..c594ebe8c0d 100644 --- a/contracts/proxy/ERC1967/ERC1967Upgrade.sol +++ b/contracts/proxy/ERC1967/ERC1967Upgrade.sol @@ -43,7 +43,7 @@ abstract contract ERC1967Upgrade { * @dev Stores a new address in the EIP1967 implementation slot. */ function _setImplementation(address newImplementation) private { - require(Address.isContract(newImplementation), "ERC1967: new implementation is not a contract"); + require(address(newImplementation).code.length > 0, "ERC1967: new implementation is not a contract"); StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value = newImplementation; } @@ -157,9 +157,9 @@ abstract contract ERC1967Upgrade { * @dev Stores a new beacon in the EIP1967 beacon slot. */ function _setBeacon(address newBeacon) private { - require(Address.isContract(newBeacon), "ERC1967: new beacon is not a contract"); + require(address(newBeacon).code.length > 0, "ERC1967: new beacon is not a contract"); require( - Address.isContract(IBeacon(newBeacon).implementation()), + address(IBeacon(newBeacon).implementation()).code.length > 0, "ERC1967: beacon implementation is not a contract" ); StorageSlot.getAddressSlot(_BEACON_SLOT).value = newBeacon; diff --git a/contracts/proxy/beacon/UpgradeableBeacon.sol b/contracts/proxy/beacon/UpgradeableBeacon.sol index 5d83ceb3b4f..048f85c455b 100644 --- a/contracts/proxy/beacon/UpgradeableBeacon.sol +++ b/contracts/proxy/beacon/UpgradeableBeacon.sol @@ -5,7 +5,6 @@ pragma solidity ^0.8.0; import "./IBeacon.sol"; import "../../access/Ownable.sol"; -import "../../utils/Address.sol"; /** * @dev This contract is used in conjunction with one or more instances of {BeaconProxy} to determine their @@ -59,7 +58,7 @@ contract UpgradeableBeacon is IBeacon, Ownable { * - `newImplementation` must be a contract. */ function _setImplementation(address newImplementation) private { - require(Address.isContract(newImplementation), "UpgradeableBeacon: implementation is not a contract"); + require(address(newImplementation).code.length > 0, "UpgradeableBeacon: implementation is not a contract"); _implementation = newImplementation; } } diff --git a/contracts/proxy/utils/Initializable.sol b/contracts/proxy/utils/Initializable.sol index 81683f002e0..e46ce60e55e 100644 --- a/contracts/proxy/utils/Initializable.sol +++ b/contracts/proxy/utils/Initializable.sol @@ -3,8 +3,6 @@ pragma solidity ^0.8.2; -import "../../utils/Address.sol"; - /** * @dev This is a base contract to aid in writing upgradeable contracts, or any kind of contract that will be deployed * behind a proxy. Since proxied contracts do not make use of a constructor, it's common to move constructor logic to an @@ -78,7 +76,7 @@ abstract contract Initializable { modifier initializer() { bool isTopLevelCall = !_initializing; require( - (isTopLevelCall && _initialized < 1) || (!Address.isContract(address(this)) && _initialized == 1), + (isTopLevelCall && _initialized < 1) || (address(this).code.length == 0 && _initialized == 1), "Initializable: contract is already initialized" ); _initialized = 1; diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index 237e8ba9469..19b120d0dde 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/contracts/token/ERC1155/ERC1155.sol @@ -472,7 +472,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { uint256 amount, bytes memory data ) private { - if (to.isContract()) { + if (to.code.length > 0) { try IERC1155Receiver(to).onERC1155Received(operator, from, id, amount, data) returns (bytes4 response) { if (response != IERC1155Receiver.onERC1155Received.selector) { revert("ERC1155: ERC1155Receiver rejected tokens"); @@ -493,7 +493,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { uint256[] memory amounts, bytes memory data ) private { - if (to.isContract()) { + if (to.code.length > 0) { try IERC1155Receiver(to).onERC1155BatchReceived(operator, from, ids, amounts, data) returns ( bytes4 response ) { diff --git a/contracts/token/ERC721/ERC721.sol b/contracts/token/ERC721/ERC721.sol index 3c457801e49..69015176471 100644 --- a/contracts/token/ERC721/ERC721.sol +++ b/contracts/token/ERC721/ERC721.sol @@ -407,7 +407,8 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata { uint256 tokenId, bytes memory data ) private returns (bool) { - if (to.isContract()) { + // check if `to` address has bytecode i.e. if its a valid contract presently. + if (to.code.length > 0) { try IERC721Receiver(to).onERC721Received(_msgSender(), from, tokenId, data) returns (bytes4 retval) { return retval == IERC721Receiver.onERC721Received.selector; } catch (bytes memory reason) { diff --git a/contracts/token/ERC777/ERC777.sol b/contracts/token/ERC777/ERC777.sol index a461004373d..b8afeb5510c 100644 --- a/contracts/token/ERC777/ERC777.sol +++ b/contracts/token/ERC777/ERC777.sol @@ -498,7 +498,10 @@ contract ERC777 is Context, IERC777, IERC20 { if (implementer != address(0)) { IERC777Recipient(implementer).tokensReceived(operator, from, to, amount, userData, operatorData); } else if (requireReceptionAck) { - require(!to.isContract(), "ERC777: token recipient contract has no implementer for ERC777TokensRecipient"); + require( + to.code.length == 0, + "ERC777: token recipient contract has no implementer for ERC777TokensRecipient" + ); } } diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol index 69f3bf87af6..5174b7fe9e3 100644 --- a/contracts/utils/Address.sol +++ b/contracts/utils/Address.sol @@ -7,40 +7,6 @@ pragma solidity ^0.8.1; * @dev Collection of functions related to the address type */ library Address { - /** - * @dev Returns true if `account` is a contract. - * - * [IMPORTANT] - * ==== - * It is unsafe to assume that an address for which this function returns - * false is an externally-owned account (EOA) and not a contract. - * - * Among others, `isContract` will return false for the following - * types of addresses: - * - * - an externally-owned account - * - a contract in construction - * - an address where a contract will be created - * - an address where a contract lived, but was destroyed - * ==== - * - * [IMPORTANT] - * ==== - * You shouldn't rely on `isContract` to protect against flash loan attacks! - * - * Preventing calls from contracts is highly discouraged. It breaks composability, breaks support for smart wallets - * like Gnosis Safe, and does not provide security since it can be circumvented by calling from a contract - * constructor. - * ==== - */ - function isContract(address account) internal view returns (bool) { - // This method relies on extcodesize/address.code.length, which returns 0 - // for contracts in construction, since the code is only stored at the end - // of the constructor execution. - - return account.code.length > 0; - } - /** * @dev Replacement for Solidity's `transfer`: sends `amount` wei to * `recipient`, forwarding all available gas and reverting on errors. @@ -200,9 +166,9 @@ library Address { ) internal view returns (bytes memory) { if (success) { if (returndata.length == 0) { - // only check isContract if the call was successful and the return data is empty + // only check if target is a contract if the call was successful and the return data is empty // otherwise we already know that it was a contract - require(isContract(target), "Address: call to non-contract"); + require(address(target).code.length > 0, "Address: call to non-contract"); } return returndata; } else { diff --git a/test/utils/Address.test.js b/test/utils/Address.test.js index 1bdfad48366..d6edac8863c 100644 --- a/test/utils/Address.test.js +++ b/test/utils/Address.test.js @@ -12,17 +12,6 @@ contract('Address', function (accounts) { this.mock = await AddressImpl.new(); }); - describe('isContract', function () { - it('returns false for account address', async function () { - expect(await this.mock.isContract(other)).to.equal(false); - }); - - it('returns true for contract address', async function () { - const contract = await AddressImpl.new(); - expect(await this.mock.isContract(contract.address)).to.equal(true); - }); - }); - describe('sendValue', function () { beforeEach(async function () { this.recipientTracker = await balance.tracker(recipient);