Remove Address.isContract

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

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

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

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;

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

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;

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
             ) {

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) {

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

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 {

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