Skip to content

Commit

Permalink
chore: resolve issues from comments
Browse files Browse the repository at this point in the history
  • Loading branch information
chefburger committed Oct 17, 2024
1 parent 1353edc commit 5425acf
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 10 deletions.
6 changes: 6 additions & 0 deletions src/Create3Factory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {ReentrancyGuard} from "@openzeppelin/contracts/utils/ReentrancyGuard.sol
/// @dev ensure this contract is deployed on multiple chain with the same address
contract Create3Factory is ICreate3Factory, Ownable2Step, ReentrancyGuard {
event SetWhitelist(address indexed user, bool isWhitelist);
event Deployed(address indexed deployed, bytes32 salt, bytes32 creationCodeHash);

// Only whitelisted user can interact with create2Factory
mapping(address user => bool isWhitelisted) public isUserWhitelisted;
Expand All @@ -27,13 +28,18 @@ contract Create3Factory is ICreate3Factory, Ownable2Step, ReentrancyGuard {
function deploy(
bytes32 salt,
bytes memory creationCode,
bytes32 creationCodeHash,
uint256 creationFund,
bytes calldata afterDeploymentExecutionPayload,
uint256 afterDeploymentExecutionFund
) external payable onlyWhitelisted nonReentrant returns (address deployed) {
if (creationCodeHash != keccak256(creationCode)) revert CreationCodeHashMismatch();

deployed = Create3.create3(
salt, creationCode, creationFund, afterDeploymentExecutionPayload, afterDeploymentExecutionFund
);

emit Deployed(deployed, salt, creationCodeHash);
}

/// @inheritdoc ICreate3Factory
Expand Down
3 changes: 3 additions & 0 deletions src/interfaces/ICreate3Factory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ pragma solidity ^0.8.0;

interface ICreate3Factory {
error NotWhitelisted();
error CreationCodeHashMismatch();

/**
* @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 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 creationCodeHash Hash of the creation code, it can be used to verify the creation code
* @param creationFund In WEI of ETH to be forwarded to target contract constructor
* @param afterDeploymentExecutionPayload Payload to be executed after contract creation
* @param afterDeploymentExecutionFund In WEI of ETH to be forwarded to when executing after deployment initialization
Expand All @@ -17,6 +19,7 @@ interface ICreate3Factory {
function deploy(
bytes32 salt,
bytes memory creationCode,
bytes32 creationCodeHash,
uint256 creationFund,
bytes calldata afterDeploymentExecutionPayload,
uint256 afterDeploymentExecutionFund
Expand Down
72 changes: 64 additions & 8 deletions test/Create3Factory.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {GasSnapshot} from "forge-gas-snapshot/GasSnapshot.sol";
import {ICreate3Factory, Create3Factory} from "../src/Create3Factory.sol";
import {MockOwnerWithConstructorArgs} from "./mocks/MockOwnerWithConstructorArgs.sol";
import {MockAccessControlWithConstructorArgs} from "./mocks/MockAccessControlWithConstructorArgs.sol";
import {CustomizedProxyChild} from "../src/CustomizedProxyChild.sol";

contract Create3FactoryTest is Test, GasSnapshot {
Create3Factory create3Factory;
Expand All @@ -35,16 +36,39 @@ contract Create3FactoryTest is Test, GasSnapshot {
// 3. make sure this contract has enough balance
vm.deal(address(this), 1 ether);

vm.expectEmit(true, true, true, true);
emit Create3Factory.Deployed(create3Factory.computeAddress(salt), salt, keccak256(creationCode));

// 4. deploy
address deployed =
create3Factory.deploy{value: 1 ether}(salt, creationCode, 1 ether, afterDeploymentExecutionPayload, 0 ether);
address deployed = create3Factory.deploy{value: 1 ether}(
salt, creationCode, keccak256(creationCode), 1 ether, afterDeploymentExecutionPayload, 0 ether
);

// 5. verify constructor args, balance and owner
assertEq(MockOwnerWithConstructorArgs(deployed).args(), 42);
assertEq(deployed.balance, 1 ether);
assertEq(Ownable(deployed).owner(), expectedOwner);
}

function test_Deploy_MockOwnerWithConstructorArgs_RevertWithCreationCodeHashMismatch() 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
bytes memory afterDeploymentExecutionPayload =
abi.encodeWithSelector(Ownable.transferOwnership.selector, expectedOwner);

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

// 4. deploy
vm.expectRevert(abi.encodeWithSelector(ICreate3Factory.CreationCodeHashMismatch.selector));
create3Factory.deploy{value: 1 ether}(
salt, creationCode, keccak256("anything else"), 1 ether, afterDeploymentExecutionPayload, 0 ether
);
}

function test_Deploy_MockOwnerWithConstructorArgs_BubbleUpRevert() public {
// 1. prepare salt and creation code
bytes32 salt = bytes32(uint256(0x1234));
Expand All @@ -60,7 +84,28 @@ contract Create3FactoryTest is Test, GasSnapshot {

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

function test_Deploy_MockOwnerWithConstructorArgs_NotFromParent() 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
bytes memory afterDeploymentExecutionPayload =
abi.encodeWithSelector(MockOwnerWithConstructorArgs.tryReEntryChildProxy.selector);

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

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

function test_Deploy_MockAccessControlWithConstructorArgs() public {
Expand All @@ -77,8 +122,9 @@ contract Create3FactoryTest is Test, GasSnapshot {
vm.deal(address(this), 2 ether);

// 4. deploy
address deployed =
create3Factory.deploy{value: 2 ether}(salt, creationCode, 2 ether, afterDeploymentExecutionPayload, 0 ether);
address deployed = create3Factory.deploy{value: 2 ether}(
salt, creationCode, keccak256(creationCode), 2 ether, afterDeploymentExecutionPayload, 0 ether
);

// 5. verify constructor args, balance and owner
assertEq(MockAccessControlWithConstructorArgs(deployed).args(), "hello world");
Expand Down Expand Up @@ -107,7 +153,12 @@ contract Create3FactoryTest is Test, GasSnapshot {

// 4. deploy
address deployed = create3Factory.deploy{value: randomValue}(
randomSalt, creationCode, randomValue - 1 ether, afterDeploymentExecutionPayload, 1 ether
randomSalt,
creationCode,
keccak256(creationCode),
randomValue - 1 ether,
afterDeploymentExecutionPayload,
1 ether
);

// 5. verify constructor args, balance and owner
Expand Down Expand Up @@ -170,7 +221,7 @@ contract Create3FactoryTest is Test, GasSnapshot {
bytes memory creationCode = abi.encodePacked(type(MockOwnerWithConstructorArgs).creationCode, abi.encode(42));
bytes32 salt = bytes32(uint256(0x1234));
vm.expectRevert(ICreate3Factory.NotWhitelisted.selector);
create3Factory.deploy(salt, creationCode, 0 ether, new bytes(0), 0 ether);
create3Factory.deploy(salt, creationCode, keccak256(creationCode), 0 ether, new bytes(0), 0 ether);
}

function test_SetWhitelistedUser() public {
Expand Down Expand Up @@ -205,7 +256,12 @@ contract Create3FactoryTest is Test, GasSnapshot {
uint256 afterDeploymentExecutionFund
) external payable {
address addr = create3Factory.deploy{value: creationFund + afterDeploymentExecutionFund}(
salt, creationCode, creationFund, afterDeploymentExecutionPayload, afterDeploymentExecutionFund
salt,
creationCode,
keccak256(creationCode),
creationFund,
afterDeploymentExecutionPayload,
afterDeploymentExecutionFund
);

assembly ("memory-safe") {
Expand Down
6 changes: 4 additions & 2 deletions test/Create3FactoryFork.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,17 @@ contract Create3FactoryForkTest is Test {

vm.selectFork(bscForkId);
vm.prank(pcsDeployer);
address bscDeployedAddr = bscCreate3.deploy(salt, creationCode, 0, afterDeploymentExecutionPayload, 0);
address bscDeployedAddr =
bscCreate3.deploy(salt, creationCode, keccak256(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 sepoliaDeployedAddr = sepoliaCreate3.deploy(salt, creationCode, 0, afterDeploymentExecutionPayload, 0);
address sepoliaDeployedAddr =
sepoliaCreate3.deploy(salt, creationCode, keccak256(creationCode), 0, afterDeploymentExecutionPayload, 0);
assertEq(MockOwnerWithConstructorArgs(sepoliaDeployedAddr).args(), 888);
assertEq(Ownable(sepoliaDeployedAddr).owner(), expectedOwner);

Expand Down
5 changes: 5 additions & 0 deletions test/mocks/MockOwnerWithConstructorArgs.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pragma solidity ^0.8.26;

import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {CustomizedProxyChild} from "../../src/CustomizedProxyChild.sol";

contract MockOwnerWithConstructorArgs is Ownable {
uint256 public args;
Expand All @@ -13,4 +14,8 @@ contract MockOwnerWithConstructorArgs is Ownable {
function payMe() external payable {
// do nothing
}

function tryReEntryChildProxy() external {
CustomizedProxyChild(msg.sender).deploy(new bytes(0), 0, new bytes(0), 0);
}
}

0 comments on commit 5425acf

Please sign in to comment.