Add ability to create clones with initial value in Clones.sol (#4936)

Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com> Co-authored-by: ernestognw <ernestognw@gmail.com>
OpenZeppelin · Mar 6, 2024 · e831429 · e831429 · mahbobemohamadzapata · May 24, 2024
1 parent 8b2f29c
commit e831429
Show file tree

Hide file tree

Showing 19 changed files with 182 additions and 102 deletions.
diff --git a/.changeset/chilled-walls-develop.md b/.changeset/chilled-walls-develop.md
@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`Clones`: Add version of `clone` and `cloneDeterministic` that support sending value at creation.
diff --git a/.changeset/strong-singers-talk.md b/.changeset/strong-singers-talk.md
@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`Errors`: New library of common custom errors.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,14 @@
 # Changelog
 
+### Custom error changes
+
+This version comes with changes to the custom error identifiers. Contracts previously depending on the following errors should be replaced accordingly:
+
+- Replace `Address.FailedInnerCall` with `Errors.FailedCall`
+- Replace `Address.AddressInsufficientBalance` with `Errors.InsufficientBalance`
+- Replace `Clones.Create2InsufficientBalance` with `Errors.InsufficientBalance`
+- Replace `Clones.ERC1167FailedCreateClone` with `Errors.FailedDeployment`
+- Replace `Clones.Create2FailedDeployment` with `Errors.FailedDeployment`
 
 ## 5.0.2 (2024-02-29)
 

diff --git a/README.md b/README.md
@@ -41,7 +41,7 @@ $ npm install @openzeppelin/contracts
 $ forge install OpenZeppelin/openzeppelin-contracts
 ```
 
-Add `@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/` in `remappings.txt.` 
+Add `@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/` in `remappings.txt.`
 
 ### Usage
 

diff --git a/contracts/metatx/ERC2771Forwarder.sol b/contracts/metatx/ERC2771Forwarder.sol
@@ -8,6 +8,7 @@ import {ECDSA} from "../utils/cryptography/ECDSA.sol";
 import {EIP712} from "../utils/cryptography/EIP712.sol";
 import {Nonces} from "../utils/Nonces.sol";
 import {Address} from "../utils/Address.sol";
+import {Errors} from "../utils/Errors.sol";
 
 /**
  * @dev A forwarder compatible with ERC-2771 contracts. See {ERC2771Context}.
@@ -132,7 +133,7 @@ contract ERC2771Forwarder is EIP712, Nonces {
         }
 
         if (!_execute(request, true)) {
-            revert Address.FailedInnerCall();
+            revert Errors.FailedCall();
         }
     }
 

diff --git a/contracts/proxy/Clones.sol b/contracts/proxy/Clones.sol
@@ -3,6 +3,8 @@
 
 pragma solidity ^0.8.20;
 
+import {Errors} from "../utils/Errors.sol";
+
 /**
  * @dev https://eips.ethereum.org/EIPS/eip-1167[ERC-1167] is a standard for
  * deploying minimal proxy contracts, also known as "clones".
@@ -15,17 +17,26 @@ pragma solidity ^0.8.20;
  * deterministic method.
  */
 library Clones {
-    /**
-     * @dev A clone instance deployment failed.
-     */
-    error ERC1167FailedCreateClone();
-
     /**
      * @dev Deploys and returns the address of a clone that mimics the behaviour of `implementation`.
      *
      * This function uses the create opcode, which should never revert.
      */
     function clone(address implementation) internal returns (address instance) {
+        return clone(implementation, 0);
+    }
+
+    /**
+     * @dev Same as {xref-Clones-clone-address-}[clone], but with a `value` parameter to send native currency
+     * to the new contract.
+     *
+     * NOTE: Using a non-zero value at creation will require the contract using this function (e.g. a factory)
+     * to always have enough balance for new deployments. Consider exposing this function under a payable method.
+     */
+    function clone(address implementation, uint256 value) internal returns (address instance) {
+        if (address(this).balance < value) {
+            revert Errors.InsufficientBalance(address(this).balance, value);
+        }
         /// @solidity memory-safe-assembly
         assembly {
             // Stores the bytecode after address
@@ -34,10 +45,10 @@ library Clones {
             mstore(0x11, implementation)
             // Packs the first 3 bytes of the `implementation` address with the bytecode before the address.
             mstore(0x00, or(shr(0x88, implementation), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000))
-            instance := create(0, 0x09, 0x37)
+            instance := create(value, 0x09, 0x37)
         }
         if (instance == address(0)) {
-            revert ERC1167FailedCreateClone();
+            revert Errors.FailedDeployment();
         }
     }
 
@@ -49,6 +60,24 @@ library Clones {
      * the clones cannot be deployed twice at the same address.
      */
     function cloneDeterministic(address implementation, bytes32 salt) internal returns (address instance) {
+        return cloneDeterministic(implementation, salt, 0);
+    }
+
+    /**
+     * @dev Same as {xref-Clones-cloneDeterministic-address-bytes32-}[cloneDeterministic], but with
+     * a `value` parameter to send native currency to the new contract.
+     *
+     * NOTE: Using a non-zero value at creation will require the contract using this function (e.g. a factory)
+     * to always have enough balance for new deployments. Consider exposing this function under a payable method.
+     */
+    function cloneDeterministic(
+        address implementation,
+        bytes32 salt,
+        uint256 value
+    ) internal returns (address instance) {
+        if (address(this).balance < value) {
+            revert Errors.InsufficientBalance(address(this).balance, value);
+        }
         /// @solidity memory-safe-assembly
         assembly {
             // Stores the bytecode after address
@@ -57,10 +86,10 @@ library Clones {
             mstore(0x11, implementation)
             // Packs the first 3 bytes of the `implementation` address with the bytecode before the address.
             mstore(0x00, or(shr(0x88, implementation), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000))
-            instance := create2(0, 0x09, 0x37, salt)
+            instance := create2(value, 0x09, 0x37, salt)
         }
         if (instance == address(0)) {
-            revert ERC1167FailedCreateClone();
+            revert Errors.FailedDeployment();
         }
     }
 

diff --git a/contracts/utils/Address.sol b/contracts/utils/Address.sol
@@ -3,25 +3,17 @@
 
 pragma solidity ^0.8.20;
 
+import {Errors} from "./Errors.sol";
+
 /**
  * @dev Collection of functions related to the address type
  */
 library Address {
-    /**
-     * @dev The ETH balance of the account is not enough to perform the operation.
-     */
-    error AddressInsufficientBalance(address account);
-
     /**
      * @dev There's no code at `target` (it is not a contract).
      */
     error AddressEmptyCode(address target);
 
-    /**
-     * @dev A call to an address target failed. The target may have reverted.
-     */
-    error FailedInnerCall();
-
     /**
      * @dev Replacement for Solidity's `transfer`: sends `amount` wei to
      * `recipient`, forwarding all available gas and reverting on errors.
@@ -40,12 +32,12 @@ library Address {
      */
     function sendValue(address payable recipient, uint256 amount) internal {
         if (address(this).balance < amount) {
-            revert AddressInsufficientBalance(address(this));
+            revert Errors.InsufficientBalance(address(this).balance, amount);
         }
 
         (bool success, ) = recipient.call{value: amount}("");
         if (!success) {
-            revert FailedInnerCall();
+            revert Errors.FailedCall();
         }
     }
 
@@ -57,7 +49,7 @@ library Address {
      * If `target` reverts with a revert reason or custom error, it is bubbled
      * up by this function (like regular Solidity function calls). However, if
      * the call reverted with no returned reason, this function reverts with a
-     * {FailedInnerCall} error.
+     * {Errors.FailedCall} error.
      *
      * Returns the raw returned data. To convert to the expected return value,
      * use https://solidity.readthedocs.io/en/latest/units-and-global-variables.html?highlight=abi.decode#abi-encoding-and-decoding-functions[`abi.decode`].
@@ -82,7 +74,7 @@ library Address {
      */
     function functionCallWithValue(address target, bytes memory data, uint256 value) internal returns (bytes memory) {
         if (address(this).balance < value) {
-            revert AddressInsufficientBalance(address(this));
+            revert Errors.InsufficientBalance(address(this).balance, value);
         }
         (bool success, bytes memory returndata) = target.call{value: value}(data);
         return verifyCallResultFromTarget(target, success, returndata);
@@ -108,8 +100,8 @@ library Address {
 
     /**
      * @dev Tool to verify that a low level call to smart-contract was successful, and reverts if the target
-     * was not a contract or bubbling up the revert reason (falling back to {FailedInnerCall}) in case of an
-     * unsuccessful call.
+     * was not a contract or bubbling up the revert reason (falling back to {Errors.FailedCall}) in case
+     * of an unsuccessful call.
      */
     function verifyCallResultFromTarget(
         address target,
@@ -130,7 +122,7 @@ library Address {
 
     /**
      * @dev Tool to verify that a low level call was successful, and reverts if it wasn't, either by bubbling the
-     * revert reason or with a default {FailedInnerCall} error.
+     * revert reason or with a default {Errors.FailedCall} error.
      */
     function verifyCallResult(bool success, bytes memory returndata) internal pure returns (bytes memory) {
         if (!success) {
@@ -141,7 +133,7 @@ library Address {
     }
 
     /**
-     * @dev Reverts with returndata if present. Otherwise reverts with {FailedInnerCall}.
+     * @dev Reverts with returndata if present. Otherwise reverts with {Errors.FailedCall}.
      */
     function _revert(bytes memory returndata) private pure {
         // Look for revert reason and bubble it up if present
@@ -153,7 +145,7 @@ library Address {
                 revert(add(32, returndata), returndata_size)
             }
         } else {
-            revert FailedInnerCall();
+            revert Errors.FailedCall();
         }
     }
 }
diff --git a/contracts/utils/Create2.sol b/contracts/utils/Create2.sol
@@ -3,6 +3,8 @@
 
 pragma solidity ^0.8.20;
 
+import {Errors} from "./Errors.sol";
+
 /**
  * @dev Helper to make usage of the `CREATE2` EVM opcode easier and safer.
  * `CREATE2` can be used to compute in advance the address where a smart
@@ -13,21 +15,11 @@ pragma solidity ^0.8.20;
  * information.
  */
 library Create2 {
-    /**
-     * @dev Not enough balance for performing a CREATE2 deploy.
-     */
-    error Create2InsufficientBalance(uint256 balance, uint256 needed);
-
     /**
      * @dev There's no code to deploy.
      */
     error Create2EmptyBytecode();
 
-    /**
-     * @dev The deployment failed.
-     */
-    error Create2FailedDeployment();
-
     /**
      * @dev Deploys a contract using `CREATE2`. The address where the contract
      * will be deployed can be known in advance via {computeAddress}.
@@ -44,7 +36,7 @@ library Create2 {
      */
     function deploy(uint256 amount, bytes32 salt, bytes memory bytecode) internal returns (address addr) {
         if (address(this).balance < amount) {
-            revert Create2InsufficientBalance(address(this).balance, amount);
+            revert Errors.InsufficientBalance(address(this).balance, amount);
         }
         if (bytecode.length == 0) {
             revert Create2EmptyBytecode();
@@ -54,7 +46,7 @@ library Create2 {
             addr := create2(amount, add(bytecode, 0x20), mload(bytecode), salt)
         }
         if (addr == address(0)) {
-            revert Create2FailedDeployment();
+            revert Errors.FailedDeployment();
         }
     }
 

diff --git a/contracts/utils/Errors.sol b/contracts/utils/Errors.sol
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.20;
+
+/**
+ * @dev Collection of common custom errors used in multiple contracts
+ *
+ * IMPORTANT: Backwards compatibility is not guaranteed in future versions of the library.
+ * It is recommended to avoid relying on the error API for critical functionality.
+ */
+library Errors {
+    /**
+     * @dev The ETH balance of the account is not enough to perform the operation.
+     */
+    error InsufficientBalance(uint256 balance, uint256 needed);
+
+    /**
+     * @dev A call to an address target failed. The target may have reverted.
+     */
+    error FailedCall();
+
+    /**
+     * @dev The deployment failed.
+     */
+    error FailedDeployment();
+}
diff --git a/scripts/upgradeable/upgradeable.patch b/scripts/upgradeable/upgradeable.patch
@@ -59,7 +59,7 @@ index ff596b0c3..000000000
 -<!-- Make sure that you have reviewed the OpenZeppelin Contracts Contributor Guidelines. -->
 -<!-- https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/CONTRIBUTING.md -->
 diff --git a/README.md b/README.md
-index 35083bc6e..05cf4fc27 100644
+index fa7b4e31e..4799b6376 100644
 --- a/README.md
 +++ b/README.md
 @@ -19,6 +19,9 @@
@@ -89,8 +89,8 @@ index 35083bc6e..05cf4fc27 100644
 +$ forge install OpenZeppelin/openzeppelin-contracts-upgradeable
  ```
 
--Add `@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/` in `remappings.txt.` 
-+Add `@openzeppelin/contracts-upgradeable/=lib/openzeppelin-contracts-upgradeable/contracts/` in `remappings.txt.` 
+-Add `@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/` in `remappings.txt.`
++Add `@openzeppelin/contracts-upgradeable/=lib/openzeppelin-contracts-upgradeable/contracts/` in `remappings.txt.`
 
  ### Usage
 
@@ -110,15 +110,15 @@ index 35083bc6e..05cf4fc27 100644
  }
  ```
 diff --git a/contracts/package.json b/contracts/package.json
-index 6ab89138a..ece834a44 100644
+index 845e8c403..8dc181b91 100644
 --- a/contracts/package.json
 +++ b/contracts/package.json
 @@ -1,5 +1,5 @@
  {
 -  "name": "@openzeppelin/contracts",
 +  "name": "@openzeppelin/contracts-upgradeable",
    "description": "Secure Smart Contract library for Solidity",
-   "version": "5.0.1",
+   "version": "5.0.2",
    "files": [
 @@ -13,7 +13,7 @@
    },
@@ -307,7 +307,7 @@ index 77c4c8990..602467f40 100644
      }
  }
 diff --git a/package.json b/package.json
-index ec2c44ced..46eedc98f 100644
+index c4b358e10..96ab2559c 100644
 --- a/package.json
 +++ b/package.json
 @@ -32,7 +32,7 @@
@@ -328,7 +328,7 @@ index 304d1386a..a1cd63bee 100644
 +@openzeppelin/contracts-upgradeable/=contracts/
 +@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/
 diff --git a/test/utils/cryptography/EIP712.test.js b/test/utils/cryptography/EIP712.test.js
-index 166038b36..268e0d29d 100644
+index 2b6e7fa97..268e0d29d 100644
 --- a/test/utils/cryptography/EIP712.test.js
 +++ b/test/utils/cryptography/EIP712.test.js
 @@ -47,27 +47,6 @@ describe('EIP712', function () {
@@ -346,7 +346,7 @@ index 166038b36..268e0d29d 100644
 -            const clone = await factory
 -              .$clone(this.eip712)
 -              .then(tx => tx.wait())
--              .then(receipt => receipt.logs.find(ev => ev.fragment.name == 'return$clone').args.instance)
+-              .then(receipt => receipt.logs.find(ev => ev.fragment.name == 'return$clone_address').args.instance)
 -              .then(address => ethers.getContractAt('$EIP712Verifier', address));
 -
 -            const expectedDomain = { ...this.domain, verifyingContract: clone.target };

diff --git a/test/finance/VestingWallet.behavior.js b/test/finance/VestingWallet.behavior.js
@@ -12,7 +12,7 @@ async function envSetup(mock, beneficiary, token) {
         const beneficiaryMock = await ethers.deployContract('EtherReceiverMock');
         await beneficiaryMock.setAcceptEther(false);
         await mock.connect(beneficiary).transferOwnership(beneficiaryMock);
-        return { args: [], error: [mock, 'FailedInnerCall'] };
+        return { args: [], error: [mock, 'FailedCall'] };
       },
       releasedEvent: 'EtherReleased',
       args: [],