From 984233dcadb8a2c715a9bbead4a35156bac00243 Mon Sep 17 00:00:00 2001 From: Dimitrios Papathanasiou <116485010+dimitriospapathanasiou@users.noreply.github.com> Date: Mon, 27 May 2024 13:45:32 +0300 Subject: [PATCH] Bubble up `returndata` from reverted Create2 deployments (#5052) Co-authored-by: Hadrien Croubois Co-authored-by: ernestognw --- .changeset/nervous-eyes-teach.md | 5 +++ contracts/mocks/ConstructorMock.sol | 34 +++++++++++++++++ contracts/utils/Create2.sol | 5 +++ test/utils/Create2.test.js | 58 ++++++++++++++++++++++++++++- 4 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 .changeset/nervous-eyes-teach.md create mode 100644 contracts/mocks/ConstructorMock.sol diff --git a/.changeset/nervous-eyes-teach.md b/.changeset/nervous-eyes-teach.md new file mode 100644 index 000000000..f85bc66d8 --- /dev/null +++ b/.changeset/nervous-eyes-teach.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`Create2`: Bubbles up returndata from a deployed contract that reverted during construction. diff --git a/contracts/mocks/ConstructorMock.sol b/contracts/mocks/ConstructorMock.sol new file mode 100644 index 000000000..50e671b6d --- /dev/null +++ b/contracts/mocks/ConstructorMock.sol @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.20; + +contract ConstructorMock { + bool foo; + + enum RevertType { + None, + RevertWithoutMessage, + RevertWithMessage, + RevertWithCustomError, + Panic + } + + error CustomError(); + + constructor(RevertType error) { + // After transpilation to upgradeable contract, the constructor will become an initializer + // To silence the `... can be restricted to view` warning, we write to state + foo = true; + + if (error == RevertType.RevertWithoutMessage) { + revert(); + } else if (error == RevertType.RevertWithMessage) { + revert("ConstructorMock: reverting"); + } else if (error == RevertType.RevertWithCustomError) { + revert CustomError(); + } else if (error == RevertType.Panic) { + uint256 a = uint256(0) / uint256(0); + a; + } + } +} diff --git a/contracts/utils/Create2.sol b/contracts/utils/Create2.sol index 9523c4041..d3aaa42dd 100644 --- a/contracts/utils/Create2.sol +++ b/contracts/utils/Create2.sol @@ -44,6 +44,11 @@ library Create2 { /// @solidity memory-safe-assembly assembly { addr := create2(amount, add(bytecode, 0x20), mload(bytecode), salt) + // if no address was created, and returndata is not empty, bubble revert + if and(iszero(addr), not(iszero(returndatasize()))) { + returndatacopy(0, 0, returndatasize()) + revert(0, returndatasize()) + } } if (addr == address(0)) { revert Errors.FailedDeployment(); diff --git a/test/utils/Create2.test.js b/test/utils/Create2.test.js index aba4deeb0..152fdbdf4 100644 --- a/test/utils/Create2.test.js +++ b/test/utils/Create2.test.js @@ -1,6 +1,9 @@ const { ethers } = require('hardhat'); const { expect } = require('chai'); const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); +const { PANIC_CODES } = require('@nomicfoundation/hardhat-chai-matchers/panic'); + +const { RevertType } = require('../helpers/enums'); async function fixture() { const [deployer, other] = await ethers.getSigners(); @@ -19,7 +22,9 @@ async function fixture() { .getContractFactory('$Create2') .then(({ bytecode, interface }) => ethers.concat([bytecode, interface.encodeDeploy([])])); - return { deployer, other, factory, constructorByteCode, constructorLessBytecode }; + const mockFactory = await ethers.getContractFactory('ConstructorMock'); + + return { deployer, other, factory, constructorByteCode, constructorLessBytecode, mockFactory }; } describe('Create2', function () { @@ -130,5 +135,56 @@ describe('Create2', function () { .to.be.revertedWithCustomError(this.factory, 'InsufficientBalance') .withArgs(0n, 1n); }); + + describe('reverts error thrown during contract creation', function () { + it('bubbles up without message', async function () { + await expect( + this.factory.$deploy( + 0n, + saltHex, + ethers.concat([ + this.mockFactory.bytecode, + this.mockFactory.interface.encodeDeploy([RevertType.RevertWithoutMessage]), + ]), + ), + ).to.be.revertedWithCustomError(this.factory, 'FailedDeployment'); + }); + + it('bubbles up message', async function () { + await expect( + this.factory.$deploy( + 0n, + saltHex, + ethers.concat([ + this.mockFactory.bytecode, + this.mockFactory.interface.encodeDeploy([RevertType.RevertWithMessage]), + ]), + ), + ).to.be.revertedWith('ConstructorMock: reverting'); + }); + + it('bubbles up custom error', async function () { + await expect( + this.factory.$deploy( + 0n, + saltHex, + ethers.concat([ + this.mockFactory.bytecode, + this.mockFactory.interface.encodeDeploy([RevertType.RevertWithCustomError]), + ]), + ), + ).to.be.revertedWithCustomError({ interface: this.mockFactory.interface }, 'CustomError'); + }); + + it('bubbles up panic', async function () { + await expect( + this.factory.$deploy( + 0n, + saltHex, + ethers.concat([this.mockFactory.bytecode, this.mockFactory.interface.encodeDeploy([RevertType.Panic])]), + ), + ).to.be.revertedWithPanic(PANIC_CODES.DIVISION_BY_ZERO); + }); + }); }); });