Skip to content

Commit

Permalink
fix: resolve issues per comments
Browse files Browse the repository at this point in the history
  • Loading branch information
chefburger committed Oct 16, 2024
1 parent c8dfb2a commit 1353edc
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 10 deletions.
8 changes: 7 additions & 1 deletion src/CustomizedProxyChild.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,14 @@ contract CustomizedProxyChild {
if (backRunPayload.length != 0) {
/// @dev This could be helpful when newly deployed contract
/// needs to run some initialization logic for example owner update
(bool success,) = addr.call{value: backRunFund}(backRunPayload);
(bool success, bytes memory reason) = addr.call{value: backRunFund}(backRunPayload);
if (!success) {
// bubble up the revert reason from backrun if any
if (reason.length > 0) {
assembly {
revert(add(reason, 32), mload(reason))
}
}
revert BackrunExecutionFailed();
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/ICreate3Factory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ interface ICreate3Factory {
/**
* @notice create3 deploy a contract
* @dev So long the same salt is used, the contract will be deployed at the same address on other chain
* @param salt Salt of the contract creation, resulting address will be derivated from this value only
* @param salt Salt of the contract creation, resulting address will be derived from this value only
* @param creationCode Creation code (constructor + args) of the contract to be deployed, this value doesn't affect the resulting address
* @param creationFund In WEI of ETH to be forwarded to target contract constructor
* @param afterDeploymentExecutionPayload Payload to be executed after contract creation
Expand Down
6 changes: 3 additions & 3 deletions src/libraries/Create3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {CustomizedProxyChild} from "../CustomizedProxyChild.sol";
* @dev Referenced from https://github.com/0xsequence/create3/blob/master/contracts/Create3.sol
* Updated PROXY_CHILD_BYTECODE to support customized deployment logic
* @title A library for deploying contracts EIP-3171 style.
* @author Agustin Aguilar <[email protected]>
* @author Agustin Aguilar <[email protected]>
*/
library Create3 {
error ErrorCreatingProxy();
Expand All @@ -31,7 +31,7 @@ library Create3 {

/**
* @notice Creates a new contract with given `_creationCode` and `_salt`
* @param _salt Salt of the contract creation, resulting address will be derivated from this value only
* @param _salt Salt of the contract creation, resulting address will be derived from this value only
* @param _creationCode Creation code (constructor) of the contract to be deployed, this value doesn't affect the resulting address
* @param _creationFund In WEI of ETH to be forwarded to target contract constructor
* @param _afterDeploymentExecutionPayload Payload to be executed after contract creation
Expand Down Expand Up @@ -69,7 +69,7 @@ library Create3 {

/**
* @notice Computes the resulting address of a contract deployed using address(this) and the given `_salt`
* @param _salt Salt of the contract creation, resulting address will be derivated from this value only
* @param _salt Salt of the contract creation, resulting address will be derived from this value only
* @return addr of the deployed contract, reverts on error
*
* @dev The address creation formula is: keccak256(rlp([keccak256(0xff ++ address(this) ++ _salt ++ keccak256(childBytecode))[12:], 0x01]))
Expand Down
18 changes: 18 additions & 0 deletions test/Create3Factory.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,24 @@ contract Create3FactoryTest is Test, GasSnapshot {
assertEq(Ownable(deployed).owner(), expectedOwner);
}

function test_Deploy_MockOwnerWithConstructorArgs_BubbleUpRevert() public {
// 1. prepare salt and creation code
bytes32 salt = bytes32(uint256(0x1234));
bytes memory creationCode = abi.encodePacked(type(MockOwnerWithConstructorArgs).creationCode, abi.encode(42));

// 2. prepare owner transfer payload
// set target owner to address(0) to trigger revert
bytes memory afterDeploymentExecutionPayload =
abi.encodeWithSelector(Ownable.transferOwnership.selector, address(0));

// 3. make sure this contract has enough balance
vm.deal(address(this), 1 ether);

// 4. deploy
vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableInvalidOwner.selector, address(0)));
create3Factory.deploy{value: 1 ether}(salt, creationCode, 1 ether, afterDeploymentExecutionPayload, 0 ether);
}

function test_Deploy_MockAccessControlWithConstructorArgs() public {
// 1. prepare salt and creation code
bytes32 salt = bytes32(uint256(0x1234));
Expand Down
20 changes: 15 additions & 5 deletions test/Create3FactoryFork.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@ pragma solidity ^0.8.24;

import "forge-std/src/Test.sol";
import {Create3Factory} from "../src/Create3Factory.sol";
import {MockOwner} from "./mocks/MockOwner.sol";
import {MockOwnerWithConstructorArgs} from "./mocks/MockOwnerWithConstructorArgs.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";

/// @dev run tests on testnet fork (bsc/sepolia) to verify same contract address across chain. To run the test,
/// ensure TESTNET_FORK_URL_BSC and TESTNET_FORK_URL_SEPOLIA environment variable are set
contract Create3FactoryForkTest is Test {
Create3Factory create3Factory;

address create3Deployer = makeAddr("pcsDeployer");
address expectedOwner = makeAddr("expectedOwner");
/// @dev address with difference nonce on bsc / eth
address pcsDeployer = 0x42571B8414c68B63A2729146CE93F23639d25399;

Expand Down Expand Up @@ -44,18 +46,26 @@ contract Create3FactoryForkTest is Test {
////////////////////////////////////////////////////////
// Step 2: Deploy contracts on both chain using pcsDeployer
////////////////////////////////////////////////////////
bytes memory creationCode = type(MockOwner).creationCode;
bytes memory creationCode = abi.encodePacked(type(MockOwnerWithConstructorArgs).creationCode, abi.encode(666));
bytes memory afterDeploymentExecutionPayload =
abi.encodeWithSelector(Ownable.transferOwnership.selector, expectedOwner);
bytes32 salt = bytes32(uint256(0x1234));

vm.selectFork(bscForkId);
vm.prank(pcsDeployer);
address bscMockOwner = bscCreate3.deploy(salt, creationCode, 0, new bytes(0), 0);
address bscDeployedAddr = bscCreate3.deploy(salt, creationCode, 0, afterDeploymentExecutionPayload, 0);
assertEq(MockOwnerWithConstructorArgs(bscDeployedAddr).args(), 666);
assertEq(Ownable(bscDeployedAddr).owner(), expectedOwner);

// update constructor args just to verify that even creation code is different, the address will be same
creationCode = abi.encodePacked(type(MockOwnerWithConstructorArgs).creationCode, abi.encode(888));
vm.selectFork(sepoliaForkId);
vm.prank(pcsDeployer);
address sepoliaMockOwner = sepoliaCreate3.deploy(salt, creationCode, 0, new bytes(0), 0);
address sepoliaDeployedAddr = sepoliaCreate3.deploy(salt, creationCode, 0, afterDeploymentExecutionPayload, 0);
assertEq(MockOwnerWithConstructorArgs(sepoliaDeployedAddr).args(), 888);
assertEq(Ownable(sepoliaDeployedAddr).owner(), expectedOwner);

// assert step 2
assertEq(bscMockOwner, sepoliaMockOwner);
assertEq(bscDeployedAddr, sepoliaDeployedAddr);
}
}

0 comments on commit 1353edc

Please sign in to comment.