Add revert if the bytecode length is zero (#2117)

* Add revert if the bytecode length is not greater than zero

* Add value parameter to create2 deploy function

Add tests for contract balance revert and depositing funds

* Change parameter name to amount for clarity

* Fix test for value sending

* Fix linter error

* Change revert reason

* Improve Create2.deploy documentation

* Slight test improvement

* Add changelog entry

Co-authored-by: Nicolás Venturo <nicolas.venturo@gmail.com>

CHANGELOG.md

@@ -12,6 +12,7 @@
  * `PullPayment`, `Escrow`: `withdrawWithGas` was removed. The old `withdraw` function now forwards all gas. ([#2125](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2125))
  * `Roles` was removed, use `AccessControl` as a replacement. ([#2112](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2112))
  * `ECDSA`: when receiving an invalid signature, `recover` now reverts instead of returning the zero address. ([#2114](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2114))
+ * `Create2`: added an `amount` argument to `deploy` for contracts with `payable` constructors. ([#2117](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2117))
  * `Pausable`: moved to the `utils` directory. ([#2122](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2122))
  * `Strings`: moved to the `utils` directory. ([#2122](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2122))
  * `Counters`: moved to the `utils` directory. ([#2122](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2122))

contracts/mocks/Create2Impl.sol

@@ -4,13 +4,13 @@ import "../utils/Create2.sol";
 import "../token/ERC20/ERC20.sol";
 
 contract Create2Impl {
-    function deploy(bytes32 salt, bytes memory code) public {
-        Create2.deploy(salt, code);
+    function deploy(uint256 value, bytes32 salt, bytes memory code) public {
+        Create2.deploy(value, salt, code);
     }
 
-    function deployERC20(bytes32 salt) public {
+    function deployERC20(uint256 value, bytes32 salt) public {
         // solhint-disable-next-line indent
-        Create2.deploy(salt, type(ERC20).creationCode);
+        Create2.deploy(value, salt, type(ERC20).creationCode);
     }
 
     function computeAddress(bytes32 salt, bytes32 codeHash) public view returns (address) {
@@ -20,4 +20,6 @@ contract Create2Impl {
     function computeAddressWithDeployer(bytes32 salt, bytes32 codeHash, address deployer) public pure returns (address) {
         return Create2.computeAddress(salt, codeHash, deployer);
     }
+
+    receive() payable external {}
 }

contracts/mocks/ERC20Mock.sol

@@ -4,7 +4,7 @@ import "../token/ERC20/ERC20.sol";
 
 // mock class using ERC20
 contract ERC20Mock is ERC20 {
-    constructor (address initialAccount, uint256 initialBalance) public {
+    constructor (address initialAccount, uint256 initialBalance) public payable {
         _mint(initialAccount, initialBalance);
     }
 

contracts/utils/Create2.sol

@@ -12,22 +12,33 @@ pragma solidity ^0.6.0;
 library Create2 {
     /**
      * @dev Deploys a contract using `CREATE2`. The address where the contract
-     * will be deployed can be known in advance via {computeAddress}. Note that
-     * a contract cannot be deployed twice using the same salt.
+     * will be deployed can be known in advance via {computeAddress}.
+     *
+     * The bytecode for a contract can be obtained from Solidity with
+     * `type(contractName).creationCode`.
+     *
+     * Requirements:
+     *
+     * - `bytecode` must not be empty.
+     * - `salt` must have not been used for `bytecode` already.
+     * - the factory must have a balance of at least `amount`.
+     * - if `amount` is non-zero, `bytecode` must have a `payable` constructor.
      */
-    function deploy(bytes32 salt, bytes memory bytecode) internal returns (address) {
+    function deploy(uint256 amount, bytes32 salt, bytes memory bytecode) internal returns (address) {
         address addr;
+        require(address(this).balance >= amount, "Create2: insufficient balance");
+        require(bytecode.length != 0, "Create2: bytecode length is zero");
         // solhint-disable-next-line no-inline-assembly
         assembly {
-            addr := create2(0, add(bytecode, 0x20), mload(bytecode), salt)
+            addr := create2(amount, add(bytecode, 0x20), mload(bytecode), salt)
         }
         require(addr != address(0), "Create2: Failed on deploy");
         return addr;
     }
 
     /**
-     * @dev Returns the address where a contract will be stored if deployed via {deploy}. Any change in the `bytecodeHash`
-     * or `salt` will result in a new destination address.
+     * @dev Returns the address where a contract will be stored if deployed via {deploy}. Any change in the
+     * `bytecodeHash` or `salt` will result in a new destination address.
      */
     function computeAddress(bytes32 salt, bytes32 bytecodeHash) internal view returns (address) {
         return computeAddress(salt, bytecodeHash, address(this));

test/utils/Create2.test.js

@@ -1,5 +1,5 @@
 const { contract, accounts, web3 } = require('@openzeppelin/test-environment');
-const { BN, expectRevert } = require('@openzeppelin/test-helpers');
+const { balance, BN, ether, expectRevert, send } = require('@openzeppelin/test-helpers');
 
 const { expect } = require('chai');
 
@@ -40,7 +40,7 @@ describe('Create2', function () {
     const offChainComputed =
       computeCreate2Address(saltHex, ERC20.bytecode, this.factory.address);
     await this.factory
-      .deploy(saltHex, ERC20.bytecode, { from: deployerAccount });
+      .deploy(0, saltHex, ERC20.bytecode, { from: deployerAccount });
     expect(ERC20.bytecode).to.include((await web3.eth.getCode(offChainComputed)).slice(2));
   });
 
@@ -48,15 +48,40 @@ describe('Create2', function () {
     const offChainComputed =
       computeCreate2Address(saltHex, constructorByteCode, this.factory.address);
     await this.factory
-      .deploy(saltHex, constructorByteCode, { from: deployerAccount });
+      .deploy(0, saltHex, constructorByteCode, { from: deployerAccount });
     const erc20 = await ERC20Mock.at(offChainComputed);
     expect(await erc20.balanceOf(deployerAccount)).to.be.bignumber.equal(new BN(100));
   });
 
+  it('should deploy a contract with funds deposited in the factory', async function () {
+    const deposit = ether('2');
+    await send.ether(deployerAccount, this.factory.address, deposit);
+    expect(await balance.current(this.factory.address)).to.be.bignumber.equal(deposit);
+
+    const onChainComputed = await this.factory
+      .computeAddressWithDeployer(saltHex, web3.utils.keccak256(constructorByteCode), this.factory.address);
+
+    await this.factory.deploy(deposit, saltHex, constructorByteCode);
+    expect(await balance.current(onChainComputed)).to.be.bignumber.equal(deposit);
+  });
+
   it('should failed deploying a contract in an existent address', async function () {
-    await this.factory.deploy(saltHex, constructorByteCode, { from: deployerAccount });
+    await this.factory.deploy(0, saltHex, constructorByteCode, { from: deployerAccount });
+    await expectRevert(
+      this.factory.deploy(0, saltHex, constructorByteCode, { from: deployerAccount }), 'Create2: Failed on deploy'
+    );
+  });
+
+  it('should fail deploying a contract if the bytecode length is zero', async function () {
+    await expectRevert(
+      this.factory.deploy(0, saltHex, '0x', { from: deployerAccount }), 'Create2: bytecode length is zero'
+    );
+  });
+
+  it('should fail deploying a contract if factory contract does not have sufficient balance', async function () {
     await expectRevert(
-      this.factory.deploy(saltHex, constructorByteCode, { from: deployerAccount }), 'Create2: Failed on deploy'
+      this.factory.deploy(1, saltHex, constructorByteCode, { from: deployerAccount }),
+      'Create2: insufficient balance'
     );
   });
 });