Openzeppelin-contracts: Create2 improvements

[nventuro]

Motivated by @MicahZoltu's comments on the forum:

• Consider adding support for including value in the create2 call.
• Consider require(bytecode.length > 0, "useful error message") rather than just letting the creation fail with 0 bytes and a non-helpful message.

[corydickson]

By adding support for including value do you mean to make deploy a payable function where msg.sender deposits funds into the contract address generated?

[nventuro]

I'm actually not sure, from the EIP it doesn't look like its possible to include value in the contract construction. It could certainly be sent after the fact, but I wouldn't add a new function just for that.

@MicahZoltu perhaps you could clarify your request?

[corydickson]

That would make a lot more sense as it answers my followup Q of how would that be done!

[MicahZoltu]

From the CREATE2 EIP:

Behaves identically to CREATE, except using keccak256( 0xff ++ address ++ salt ++ keccak256(init_code))[12:] instead of the usual sender-and-nonce-hash as the address where the contract is initialized at.

Solidity inline assembly for create/create2 from Solidity docs:

instruction | explanation
-- | --
create(v, p, n) | create new contract with code mem[p…(p+n)) and send v wei and return the new address
create2(v, p, n, s) | create new contract with code mem[p…(p+n)) at address keccak256(0xff . this . s . keccak256(mem[p…(p+n))) and send v wei and return the new address, where 0xff is a 1 byte value, this is the current contract’s address as a 20 byte value and s is a big-endian 256-bit value

Note that v is value to be sent to the newly constructed contract as part of the constructor call. I am pretty sure (though someone should verify) that this means that the constructor will have msg.value set. Someone should verify that the constructor has access to the ETH, and it isn't actually sent to the contract first.

[MicahZoltu]

Here is an example of calling CREATE2 with value from a contract:
https://github.com/Zoltu/recoverable-wallet/blob/969798b32aa9ccfd1a49ed7e32ddaa8c5551ba66/contracts/source/recoverable-wallet.sol#L233-L247

[corydickson]

@MicahZoltu Thank you for documenting it further here

[nventuro]

Ah silly me, I should've paid more attention. That parameter is even present (and hard-coded to 0) in our current implementation. Thanks!