Remove Address.isContract (#3945)

Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Co-authored-by: Francisco Giordano <fg@frang.io>

.changeset/unlucky-snakes-drive.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': patch
+---
+
+`Address`: Removed `isContract` because of its ambiguous nature and potential for misuse.

contracts/mocks/proxy/UUPSLegacy.sol

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: MIT
 
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.1;
 
 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(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(newImplementation.code.length > 0, "ERC1967: new implementation is not a contract");
         StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value = newImplementation;
     }
 
@@ -153,9 +153,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(newBeacon.code.length > 0, "ERC1967: new beacon is not a contract");
         require(
-            Address.isContract(IBeacon(newBeacon).implementation()),
+            IBeacon(newBeacon).implementation().code.length > 0,
             "ERC1967: beacon implementation is not a contract"
         );
         StorageSlot.getAddressSlot(_BEACON_SLOT).value = newBeacon;

contracts/proxy/beacon/UpgradeableBeacon.sol

@@ -1,11 +1,10 @@
 // SPDX-License-Identifier: MIT
 // OpenZeppelin Contracts v4.4.1 (proxy/beacon/UpgradeableBeacon.sol)
 
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.1;
 
 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(newImplementation.code.length > 0, "UpgradeableBeacon: implementation is not a contract");
         _implementation = newImplementation;
     }
 }

contracts/proxy/utils/Initializable.sol

@@ -83,7 +83,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

@@ -1,12 +1,11 @@
 // SPDX-License-Identifier: MIT
 // OpenZeppelin Contracts (last updated v4.8.0) (token/ERC1155/ERC1155.sol)
 
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.1;
 
 import "./IERC1155.sol";
 import "./IERC1155Receiver.sol";
 import "./extensions/IERC1155MetadataURI.sol";
-import "../../utils/Address.sol";
 import "../../utils/Context.sol";
 import "../../utils/introspection/ERC165.sol";
 
@@ -18,8 +17,6 @@ import "../../utils/introspection/ERC165.sol";
  * _Available since v3.1._
  */
 contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI {
-    using Address for address;
-
     // Mapping from token ID to account balances
     mapping(uint256 => mapping(address => uint256)) private _balances;
 
@@ -344,7 +341,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");
@@ -365,7 +362,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

@@ -1,12 +1,11 @@
 // SPDX-License-Identifier: MIT
 // OpenZeppelin Contracts (last updated v4.8.0) (token/ERC721/ERC721.sol)
 
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.1;
 
 import "./IERC721.sol";
 import "./IERC721Receiver.sol";
 import "./extensions/IERC721Metadata.sol";
-import "../../utils/Address.sol";
 import "../../utils/Context.sol";
 import "../../utils/Strings.sol";
 import "../../utils/introspection/ERC165.sol";
@@ -17,7 +16,6 @@ import "../../utils/introspection/ERC165.sol";
  * {ERC721Enumerable}.
  */
 contract ERC721 is Context, ERC165, IERC721, IERC721Metadata {
-    using Address for address;
     using Strings for uint256;
 
     // Token name
@@ -402,7 +400,7 @@ contract ERC721 is Context, ERC165, IERC721, IERC721Metadata {
         uint256 tokenId,
         bytes memory data
     ) private returns (bool) {
-        if (to.isContract()) {
+        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/ERC721/extensions/ERC721Consecutive.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 // OpenZeppelin Contracts (last updated v4.8.0) (token/ERC721/extensions/ERC721Consecutive.sol)
 
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.1;
 
 import "../ERC721.sol";
 import "../../../interfaces/IERC2309.sol";
@@ -86,7 +86,7 @@ abstract contract ERC721Consecutive is IERC2309, ERC721 {
 
         // minting a batch of size 0 is a no-op
         if (batchSize > 0) {
-            require(!Address.isContract(address(this)), "ERC721Consecutive: batch minting restricted to constructor");
+            require(address(this).code.length == 0, "ERC721Consecutive: batch minting restricted to constructor");
             require(to != address(0), "ERC721Consecutive: mint to the zero address");
             require(batchSize <= _maxBatchSize(), "ERC721Consecutive: batch too large");
 
@@ -112,7 +112,7 @@ abstract contract ERC721Consecutive is IERC2309, ERC721 {
      * After construction, {_mintConsecutive} is no longer available and {_mint} becomes available.
      */
     function _mint(address to, uint256 tokenId) internal virtual override {
-        require(Address.isContract(address(this)), "ERC721Consecutive: can't mint during construction");
+        require(address(this).code.length > 0, "ERC721Consecutive: can't mint during construction");
         super._mint(to, tokenId);
     }
 

contracts/token/ERC777/ERC777.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 // OpenZeppelin Contracts (last updated v4.8.0) (token/ERC777/ERC777.sol)
 
-pragma solidity ^0.8.0;
+pragma solidity ^0.8.1;
 
 import "./IERC777.sol";
 import "./IERC777Recipient.sol";
@@ -472,7 +472,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,44 +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
-     *
-     * Furthermore, `isContract` will also return true if the target contract within
-     * the same transaction is already scheduled for destruction by `SELFDESTRUCT`,
-     * which only has an effect at the end of a transaction.
-     * ====
-     *
-     * [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 +162,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(target.code.length > 0, "Address: call to non-contract");
             }
             return returndata;
         } else {

contracts/utils/StorageSlot.sol

@@ -21,7 +21,7 @@ pragma solidity ^0.8.0;
  *     }
  *
  *     function _setImplementation(address newImplementation) internal {
- *         require(Address.isContract(newImplementation), "ERC1967: new implementation is not a contract");
+ *         require(newImplementation.code.length > 0, "ERC1967: new implementation is not a contract");
  *         StorageSlot.getAddressSlot(_IMPLEMENTATION_SLOT).value = newImplementation;
  *     }
  * }

docs/modules/ROOT/nav.adoc

@@ -19,3 +19,5 @@
 * xref:crosschain.adoc[Crosschain]
 
 * xref:utilities.adoc[Utilities]
+
+* xref:fag.adoc[FAQ]

docs/modules/ROOT/pages/faq.adoc

@@ -0,0 +1,13 @@
+= Frequently Asked Questions
+
+== Can I restrict a function to EOAs only?
+
+When calling external addresses from your contract it is unsafe to assume that an address is an externally-owned account (EOA) and not a contract. Attempting to prevent 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.
+
+Although checking that the address has code, `address.code.length > 0`, may seem to differentiate contracts from EOAs, it can only say that an address is currently a contract, and its negation (that an address is not currently a contract) does not imply that the address is an EOA. Some counterexamples are:
+
+ - address of a contract in construction
+ - address where a contract will be created
+ - address where a contract lived, but was destroyed
+
+Furthermore, an address will be considered a contract within the same transaction where it is scheduled for destruction by `SELFDESTRUCT`, which only has an effect at the end of the entire transaction.

docs/modules/ROOT/pages/utilities.adoc

@@ -99,8 +99,6 @@ If you need support for more powerful collections than Solidity's native arrays
 [[misc]]
 == Misc
 
-Want to check if an address is a contract? Use xref:api:utils.adoc#Address[`Address`] and xref:api:utils.adoc#Address-isContract-address-[`Address.isContract()`].
-
 Want to keep track of some numbers that increment by 1 every time you want another one? Check out xref:api:utils.adoc#Counters[`Counters`]. This is useful for lots of things, like creating incremental identifiers, as shown on the xref:erc721.adoc[ERC721 guide].
 
 === Base64

test/utils/Address.test.js

@@ -12,16 +12,6 @@ contract('Address', function (accounts) {
     this.mock = await Address.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 () {
-      expect(await this.mock.$isContract(this.mock.address)).to.equal(true);
-    });
-  });
-
   describe('sendValue', function () {
     beforeEach(async function () {
       this.recipientTracker = await balance.tracker(recipient);