Add revert if the bytecode length is zero

[corydickson]

Fixes #2106

This adds a revert and a corresponding message if the bytecode provided to the create2 deploy function
has zero length

Would like to also address the second part discussed in the issue pertaining to the value. Let me know if I've identified the correct way to pursue that solution.

[nventuro]

Thanks a lot for contributing @corydickson!

I'd change the revert reason to bring it more in-line with our other ones, which describe what went wrong instead of what should've happened. In this case, it'd be something like 'empty bytecode'.

[corydickson]

@nventuro Thank you for the swift reply, I'll be sure to update the error message once we have clarity on the additional changes. I can also just push this along as is if there is a timeline for the next release you're trying to stick to :)

[corydickson]

Left a comment about an issue I'm having with ensuring the library contract has a sufficient balance to complete the transaction if it receives some ETH from another account beforehand.

It seems as though making the deploy function payable is not an option.

TypeError: Library functions cannot be payable.

[corydickson]

Not sure on how to go about implementing a fallback function

[corydickson]

I also tried an approach by making a new function in the mock contract called deployWithEth which is payable and sends msg.value as the first parameter to the deploy(uint256,bytes32,bytes) library function. It passes the check of the contract having the balance, but the address generated after the call to the create2 opcode is still null thus causing the failed to deploy revert.

In the mock contract we would then have deploy(bytes32, bytes) && deployWithEth(bytes32, bytes). Where the former function would set the amount in deploy to be 0 and the latter sending the eth provided described above.

[corydickson]

This change seems rather straightforward but still think I'm missing something 😭

[nventuro]

Thanks for working on this @corydickson! I've now fixed the tests :) There were two issues that needed to be addressed:

• ERC20Mock's constructor was not payable, which is why deploying the contract failed. I had forgotten about this feature of Solidity, which is why I didn't mention this before. Sorry about that!
• the onChainComputedAddress was using deployerAccount in the test, but the actual deployer is the factory, so the computed address was wrong.

I also took the liberty to slightly improve documentation and revert reasons. Let me know what you think!

[corydickson]

Thanks for taking care of that, looks good on my end!