Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: migrate to CreateX for CREATE2 factory (or let user define create2 factory) #2638

Open
sambacha opened this issue Aug 5, 2022 · 46 comments
Assignees
Labels
C-forge Command: forge Cmd-forge-script Command: forge script T-feature Type: feature

Comments

@sambacha
Copy link
Contributor

sambacha commented Aug 5, 2022

Component

Forge

Describe the feature you would like

Define CREATE2 Factory

I do not want to use your factory, I want to use my own factory.

Additional context

https://etherscan.io/address/0x4e59b44847b379578588920ca78fbf26c0b4956c no
https://goerli.etherscan.io/address/0x179c98f5ccdb3bc7d7aefffa06fb7f608a301877#code yes

@sambacha sambacha added the T-feature Type: feature label Aug 5, 2022
@mds1
Copy link
Collaborator

mds1 commented Aug 5, 2022

This is a good config option to add, e.g. create2_deployer = 0x123.... Alternative name ideas: create2_deployer_contract or create2_factory

@sambacha
Copy link
Contributor Author

sambacha commented Aug 5, 2022

Especially considering how etherscan represents the current factory, teams may prefer to have their own canonical factory (nothing preventing other people from using it ofc) etc etc

@mds1
Copy link
Collaborator

mds1 commented Aug 5, 2022

The main reasons to use another create2 deployer are (1) pass init data to be called after deploy, (2) more leading zeros for cheaper deploy, and I think that's it? Given that, an alternative is to mine a cheap address, agree on the deploy contract's code, deploy it to all the chains, and replace the current default. One downside here is this is a breaking change for anyone relying on the current deployer for vanity addresses.

@sambacha would that work as an alternative / are there other reasons people might need a custom deployer?

@sambacha
Copy link
Contributor Author

sambacha commented Aug 5, 2022

The main reasons to use another create2 deployer are (1) pass init data to be called after deploy, (2) more leading zeros for cheaper deploy, and I think that's it? Given that, an alternative is to mine a cheap address, agree on the deploy contract's code, deploy it to all the chains, and replace the current default. One downside here is this is a breaking change for anyone relying on the current deployer for vanity addresses.

@sambacha would that work as an alternative / are there other reasons people might need a custom deployer?

The factory contract I linked has additional benefits such as:

  • There is also a view function that computes the address of the contract that will be created when submitting a given salt or nonce along with a given block of initialization code.

  • Determine if a contract has already been deployed by the factory to a given address.

  • Modifier to ensure that the first 20 bytes of a submitted salt match those of the calling account. This provides protection against the salt being stolen by frontrunners or other attackers.

Yes for leading zeros, I am using this: https://github.com/0age/create2crunch

wdyt @mds1

@mds1
Copy link
Collaborator

mds1 commented Aug 5, 2022

I think all of those are good ideas to include in a canonical create2 deployer contract 👌

Perhaps next week I can scaffold out a proposed interface

@joshieDo
Copy link
Collaborator

joshieDo commented Aug 7, 2022

The way we verify contracts or create the transaction depends on the factory we use. So just letting anyone use their own factory implementation is not that straightforward imo. So... if we can agree to a certain interface, it should be fine.

One downside here is this is a breaking change for anyone relying on the current deployer for vanity addresses

True...

@sambacha
Copy link
Contributor Author

The way we verify contracts or create the transaction depends on the factory we use. So just letting anyone use their own factory implementation is not that straightforward imo. So... if we can agree to a certain interface, it should be fine.

One downside here is this is a breaking change for anyone relying on the current deployer for vanity addresses

True...

if your gonna break it, break it before v1.0

@sambacha
Copy link
Contributor Author

This is a good config option to add, e.g. create2_deployer = 0x123.... Alternative name ideas: create2_deployer_contract or create2_factory

FoundryCreate2FactoryV8

@mds1
Copy link
Collaborator

mds1 commented Aug 29, 2022

Proposed create2 deployer below, feedback welcome. Interfaces (and the code comments) are largely based on @0age's create2 deployer, with a few modifications based on the above.

contract Create2Deployer {
  // Mapping to track which addresses have already been deployed.
  mapping(address => bool) public isDeployed;

  /// @dev Create a contract using CREATE2 by submitting a given salt or nonce
  /// along with the initialization code for the contract.
  /// @return Address of the contract that will be created, or the null address
  /// if a contract already exists at that address.
  function create2(bytes32 salt, bytes calldata initCode)
    external
    payable
    returns (address deploymentAddr);

  /// @dev Create a contract using CREATE2 by submitting a given salt or nonce
  /// along with the initialization code for the contract, and call the
  /// contract after deployment with the provided data.
  /// @return Address of the contract that will be created, or the null address
  /// if a contract already exists at that address.
  function create2(bytes32 salt, bytes calldata initCode, bytes calldata data)
    external
    payable
    returns (address deploymentAddr);

  /// @dev Create a contract using CREATE2 by submitting a given salt or nonce
  /// along with the initialization code for the contract.
  /// @dev The first 20 bytes of the salt must match those of the calling
  /// address, which prevents contract creation events from being submitted by
  /// unintended parties.
  /// @return Address of the contract that will be created, or the null address
  /// if a contract already exists at that address.
  function safeCreate2(bytes32 salt, bytes calldata initCode)
    external
    payable
    containsCaller(salt)
    returns (address deploymentAddr);

  /// @dev Create a contract using CREATE2 by submitting a given salt or nonce
  /// along with the initialization code for the contract, and call the
  /// contract after deployment with the provided data.
  /// @dev The first 20 bytes of the salt must match those of the calling
  /// address, which prevents contract creation events from being submitted by
  /// unintended parties.
  /// @return Address of the contract that will be created, or the null address
  /// if a contract already exists at that address.
  function safeCreate2(bytes32 salt, bytes calldata initCode, bytes calldata data)
    external
    payable
    containsCaller(salt)
    returns (address deploymentAddr);

  /// @dev Compute the address of the contract that will be created when
  /// submitting a given salt or nonce to the contract along with the contract's
  /// initialization code.
  /// @return Address of the contract that will be created, or the null address
  /// if a contract has already been deployed to that address.
  function computeCreate2Address(bytes32 salt, bytes calldata initCode)
    external
    view
    returns (address deploymentAddr);

  /// @dev Compute the address of the contract that will be created when
  /// submitting a given salt or nonce to the contract along with the keccak256
  /// hash of the contract's initialization code.
  /// @return Address of the contract that will be created, or the null address
  /// if a contract has already been deployed to that address.
  function computeCreate2Address(bytes32 salt, bytes32 initCodeHash)
    external
    view
    returns (address deploymentAddress);

  /// @dev Modifier to ensure that the first 20 bytes of a submitted salt match
  /// those of the calling account. This provides protection against the salt
  /// being stolen by frontrunners or other attackers. The protection can also be
  /// bypassed if desired by setting each of the first 20 bytes to zero.
  /// @param salt bytes32 The salt value to check against the calling address.
  modifier containsCaller(bytes32 salt) {
    require(
      (address(bytes20(salt)) == msg.sender) || (bytes20(salt) == bytes20(0)),
      "Invalid salt: first 20 bytes of the salt must match calling address."
    );
    _;
  }
}

We can arguably remove either (1) the containsCaller bypass of bytes20(salt) == bytes20(0) OR (2) the dedicated create2 methods, since it's a bit redundant, but I think it's worth keeping both because:

  • Existing tooling for mining vanity addresses fits better with create2 (i.e. they don't often let you add limitations on the salt)
  • Removing the bypass doesn't save much gas anyway so can't hurt to keep it to add compatibility for anyone who relies on that

cc'ing a few people for feedback who I think are interested: @sambacha @gakonst @joshieDo @pcaversaccio @storming0x

@sambacha
Copy link
Contributor Author

Proposed create2 deployer below, feedback welcome. Interfaces (and the code comments) are largely based on @0age's create2 deployer, with a few modifications based on the above.

contract Create2Deployer {
  // Mapping to track which addresses have already been deployed.
  mapping(address => bool) public isDeployed;

  /// @dev Create a contract using CREATE2 by submitting a given salt or nonce
  /// along with the initialization code for the contract.
  /// @return Address of the contract that will be created, or the null address
  /// if a contract already exists at that address.
  function create2(bytes32 salt, bytes calldata initCode)
    external
    payable
    returns (address deploymentAddr);

  /// @dev Create a contract using CREATE2 by submitting a given salt or nonce
  /// along with the initialization code for the contract, and call the
  /// contract after deployment with the provided data.
  /// @return Address of the contract that will be created, or the null address
  /// if a contract already exists at that address.
  function create2(bytes32 salt, bytes calldata initCode, bytes calldata data)
    external
    payable
    returns (address deploymentAddr);

  /// @dev Create a contract using CREATE2 by submitting a given salt or nonce
  /// along with the initialization code for the contract.
  /// @dev The first 20 bytes of the salt must match those of the calling
  /// address, which prevents contract creation events from being submitted by
  /// unintended parties.
  /// @return Address of the contract that will be created, or the null address
  /// if a contract already exists at that address.
  function safeCreate2(bytes32 salt, bytes calldata initCode)
    external
    payable
    containsCaller(salt)
    returns (address deploymentAddr);

  /// @dev Create a contract using CREATE2 by submitting a given salt or nonce
  /// along with the initialization code for the contract, and call the
  /// contract after deployment with the provided data.
  /// @dev The first 20 bytes of the salt must match those of the calling
  /// address, which prevents contract creation events from being submitted by
  /// unintended parties.
  /// @return Address of the contract that will be created, or the null address
  /// if a contract already exists at that address.
  function safeCreate2(bytes32 salt, bytes calldata initCode, bytes calldata data)
    external
    payable
    containsCaller(salt)
    returns (address deploymentAddr);

  /// @dev Compute the address of the contract that will be created when
  /// submitting a given salt or nonce to the contract along with the contract's
  /// initialization code.
  /// @return Address of the contract that will be created, or the null address
  /// if a contract has already been deployed to that address.
  function computeCreate2Address(bytes32 salt, bytes calldata initCode)
    external
    view
    returns (address deploymentAddr);

  /// @dev Compute the address of the contract that will be created when
  /// submitting a given salt or nonce to the contract along with the keccak256
  /// hash of the contract's initialization code.
  /// @return Address of the contract that will be created, or the null address
  /// if a contract has already been deployed to that address.
  function computeCreate2Address(bytes32 salt, bytes32 initCodeHash)
    external
    view
    returns (address deploymentAddress);

  /// @dev Modifier to ensure that the first 20 bytes of a submitted salt match
  /// those of the calling account. This provides protection against the salt
  /// being stolen by frontrunners or other attackers. The protection can also be
  /// bypassed if desired by setting each of the first 20 bytes to zero.
  /// @param salt bytes32 The salt value to check against the calling address.
  modifier containsCaller(bytes32 salt) {
    require(
      (address(bytes20(salt)) == msg.sender) || (bytes20(salt) == bytes20(0)),
      "Invalid salt: first 20 bytes of the salt must match calling address."
    );
    _;
  }
}

We can arguably remove either (1) the containsCaller bypass of bytes20(salt) == bytes20(0) OR (2) the dedicated create2 methods, since it's a bit redundant, but I think it's worth keeping both because:

  • Existing tooling for mining vanity addresses fits better with create2 (i.e. they don't often let you add limitations on the salt)
  • Removing the bypass doesn't save much gas anyway so can't hurt to keep it to add compatibility for anyone who relies on that

cc'ing a few people for feedback who I think are interested: @sambacha @gakonst @joshieDo @pcaversaccio @storming0x

Should there be any chain id information included ?

@mds1
Copy link
Collaborator

mds1 commented Aug 29, 2022

What chain ID information did you have in mind?

Another method that hashes your salt with chain ID to prevent redeploying to another chain at the same address might be a good idea

@sambacha
Copy link
Contributor Author

sambacha commented Aug 29, 2022

What chain ID information did you have in mind?

Another method that hashes your salt with chain ID to prevent redeploying to another chain at the same address might be a good idea

This is what I had in mind, but then again I am not sure if its even useful.

It would be useful in the edge case of contentious forks, so def YMMV

/// constructor 
deploymentChainId = block.chainid;
_DOMAIN_SEPARATOR = _calculateDomainSeparator(block.chainid);


/// ... Return the DOMAIN_SEPARATOR.
return block.chainid == deploymentChainId ? _DOMAIN_SEPARATOR : _calculateDomainSeparator(block.chainid);


/// ...abi.encode
block.chainid == deploymentChainId ? _DOMAIN_SEPARATOR : _calculateDomainSeparator(block.chainid),

@storming0x
Copy link

Thanks for tagging, yeah i think this covers our use case mostly, being able to deploy init type smart contracts with create2

@rkrasiuk rkrasiuk added this to the v1.0.0 milestone Aug 29, 2022
@pcaversaccio
Copy link
Contributor

pcaversaccio commented Aug 30, 2022

@mds1 thanks for tagging me - the suggested interface would not be compatible with my Create2Deployer.sol code. I use the following semantics:

/**
 * @dev Deploys a contract using `CREATE2`. The address where the
 * contract will be deployed can be known in advance via {computeAddress}.
 *
 * The bytecode for a contract can be obtained from Solidity with
 * `type(contractName).creationCode`.
 *
 * Requirements:
 * - `bytecode` must not be empty.
 * - `salt` must have not been used for `bytecode` already.
 * - the factory must have a balance of at least `value`.
 * - if `value` is non-zero, `bytecode` must have a `payable` constructor.
 */
function deploy(uint256 value, bytes32 salt, bytes memory code) external returns (address addr);

/**
 * @dev Deployment of the {ERC1820Implementer}.
 * Further information: https://eips.ethereum.org/EIPS/eip-1820
 */
function deployERC1820Implementer(uint256 value, bytes32 salt) external returns (address addr);

/**
 * @dev Returns the address where a contract will be stored if deployed via {deploy}.
 * Any change in the `bytecodeHash` or `salt` will result in a new destination address.
 */
function computeAddress(bytes32 salt, bytes32 codeHash) external view returns (address addr);

/**
 * @dev Returns the address where a contract will be stored if deployed via {deploy} from a
 * contract located at `deployer`. If `deployer` is this contract's address, returns the
 * same value as {computeAddress}.
 */
function computeAddressWithDeployer(bytes32 salt, bytes32 codeHash, address deployer) external pure returns (address addr);

The payable feature is implemented via a receive function in the contract itself. So the question is whether the interface should be compatible with my deployed factory or not - generally, I like the definition of the interface except for the keyword safe. Can we get rid of this, please ;-)?

@sambacha
Copy link
Contributor Author

@mds1 thanks for tagging me - the suggested interface would not be compatible with my Create2Deployer.sol code. I use the following semantics:

/**
 * @dev Deploys a contract using `CREATE2`. The address where the
 * contract will be deployed can be known in advance via {computeAddress}.
 *
 * The bytecode for a contract can be obtained from Solidity with
 * `type(contractName).creationCode`.
 *
 * Requirements:
 * - `bytecode` must not be empty.
 * - `salt` must have not been used for `bytecode` already.
 * - the factory must have a balance of at least `value`.
 * - if `value` is non-zero, `bytecode` must have a `payable` constructor.
 */
function deploy(uint256 value, bytes32 salt, bytes memory code) external returns (address addr);

/**
 * @dev Deployment of the {ERC1820Implementer}.
 * Further information: https://eips.ethereum.org/EIPS/eip-1820
 */
function deployERC1820Implementer(uint256 value, bytes32 salt) external returns (address addr);

/**
 * @dev Returns the address where a contract will be stored if deployed via {deploy}.
 * Any change in the `bytecodeHash` or `salt` will result in a new destination address.
 */
function computeAddress(bytes32 salt, bytes32 codeHash) external view returns (address addr);

/**
 * @dev Returns the address where a contract will be stored if deployed via {deploy} from a
 * contract located at `deployer`. If `deployer` is this contract's address, returns the
 * same value as {computeAddress}.
 */
function computeAddressWithDeployer(bytes32 salt, bytes32 codeHash, address deployer) external pure returns (address addr);

The payable feature is implemented via a receive function in the contract itself. So the question is whether the interface should be compatible with my deployed factory or not - generally, I like the definition of the interface except for the keyword safe. Can we get rid of this, please ;-)?

🫂✨⭐️💫

@zjesko
Copy link

zjesko commented Nov 26, 2022

any progress on this? is there a way to define the address for a custom create2 factory?

would love to work on it and open a PR if it makes sense to do so

@gakonst
Copy link
Member

gakonst commented Nov 26, 2022

Nobody's on it, feel free to go for it!

@zjesko
Copy link

zjesko commented Nov 27, 2022

sounds good @gakonst, Ill start working on it.

As per my understanding the main goal here is to add a parameter like create2_deployer which lets a user use a custom create2 factory instead of the default 0x4e59b44847b379578588920ca78fbf26c0b4956c. This is useful for the following reasons:

Does this make sense? @mds1 @joshieDo

@joshieDo
Copy link
Collaborator

The way we verify contracts or create the transaction depends on the factory we use. So just letting anyone use their own factory implementation is not that straightforward imo. So... if we can agree to a certain interface, it should be fine.

I think the core issue is not just about a custom factory, but agreeing on a specific interface for a new factory.

Sure, having a parameter for a custom factory is nice, but for the reasons given above, it has to share the same interface as the default factory.

@zjesko
Copy link

zjesko commented Nov 27, 2022

This sounds good. I'll add this feature while making sure the create2 interface is shared by the default one

@sambacha
Copy link
Contributor Author

Any movement on this for v1.0

@zjesko
Copy link

zjesko commented Jan 23, 2023

Hello, I didn't get time to work on it earlier, but will now try to open a PR asap

@sambacha
Copy link
Contributor Author

Hello, I didn't get time to work on it earlier, but will now try to open a PR asap

Hell yea lmk if you need any help

@sambacha
Copy link
Contributor Author

Hello @sambacha @joshieDo @mds1

Had a quick question regarding the interface.

Currently for example,


bytes32 constant SALT = bytes32(uint256(0x00...))

vm.startBroadcast();

        // deploy with create

        permit2 = new Permit2();



        // deploy with create2

        permit2 = new Permit2{salt: SALT}();

vm.stopBroadcast();

This common create scheme comes directly from the revm library:


pub enum CreateScheme {

    /// Legacy create scheme of `CREATE`.

    Create,

    /// Create scheme of `CREATE2`.

    Create2 {

        /// Salt.

        salt: U256,

    },

}

So should we change this schema to accommodate something like permit2 = new Permit2{salt: SALT, deployerCreate2: CREATE2_ADDRESS}(); OR instead add a forge-std helper method such as deployCreate2(string contractName, bytes32 salt, bytes memory constructorData, bytes memory initData, address deployer)

Probably forge std usage but i would wait to hear from Matt first before settling on usage

@mds1
Copy link
Collaborator

mds1 commented Jan 26, 2023

So should we change this schema to accommodate something like permit2 = new Permit2{salt: SALT, deployerCreate2: CREATE2_ADDRESS}();

This is not valid solidity and there is currently no plans to preprocess solidity to support custom syntax, so this option isn't currently doable.

OR instead add a forge-std helper method such as deployCreate2(string contractName, bytes32 salt, bytes memory constructorData, bytes memory initData, address deployer)

This solution is ok, would pair nicely with the create2 helpers recently added in foundry-rs/forge-std#276. There may be alternative function sigs we'd want, would need to think about it. But the custom create2 deployer would need to have the same signature as the default create2 deployer so it's not clear how much value this adds since you can do this on your own anyway.

Another option is to add a new config option to the foundry.toml with some name options defined in #2638 (comment), but again the custom create2 deployer would need to have the same signature as the default create2 deployer


There's been a lot of discussion in this issue so I think it's worth backing up a sec to discuss why we want this feature, which we discussed briefly starting here, and using that answer to determine which solution to go with.

IMO the best (but not necessarily simplest) solution is to write a new, more flexible create2 factory that has the features we need and deploy it across all chains. You can then add cheatcodes to instruct forge what specifically to do with the create2 deployer, for example a vm.BatchInitializeCall cheat might take the next create2 deploy and subsequent tx and batch them into the same transaction to the deployer. Or a vm.safeCreate2 cheat might instruct forge to instead use the safeCreate2 method, etc. AFAIK none of other solutions allow native support for this, and the ability to batch contract deployment + initialization is definitely important

@0xTimepunk
Copy link

Hey guys, any update on this?

@gakonst
Copy link
Member

gakonst commented Feb 9, 2023

Sorry not yet! @akshatcx what's your current plan here?

@callum-elvidge
Copy link

Hey, any update on this?

@gakonst
Copy link
Member

gakonst commented Jun 7, 2023

We haven't prioritized it, no. Is it a big deal for you? It's a small enough bite that I'd prefer a third party contributor to take. But if it is really important we can try to prio.

@apptreeso
Copy link

@gakonst hmm... I have the same issue as @akshatcx and I think it's important. Could you try to prioritize?

As per my understanding the main goal here is to add a parameter like create2_deployer which lets a user use a custom create2 factory instead of the default 0x4e59b44847b379578588920ca78fbf26c0b4956c. This is useful for the following reasons:

@pcaversaccio
Copy link
Contributor

pcaversaccio commented Jul 6, 2023

@apptreeso as an alternative for the moment being, you can use my Hardhat xdeployer plugin. Fork it, set your contract address here, and you're good to go as long as the deploy function has the abi (otherwise, you need to adjust it in the source code):

"function deploy(uint256 value, bytes32 salt, bytes code)"

as well as the chains you deploy on have your Create2Deployer. Otherwise, you can use my Create2Deployer for which xdeployer is already preconfigured.

@sambacha
Copy link
Contributor Author

sambacha commented Jul 9, 2023

The problem is not that there is a default factory: the problem is that I can not create and define my own default factory.

The way we verify contracts or create the transaction depends on the factory we use. So just letting anyone use their own factory implementation is not that straightforward imo. So... if we can agree to a certain interface, it should be fine.

https://github.com/blockscout/blockscout-rs/tree/main/smart-contract-verifier

@sambacha
Copy link
Contributor Author

sambacha commented Jul 9, 2023

Here we goooooo

ethereum/EIPs#7212 (comment)

@drortirosh
Copy link

The point of having a deployer in Foundry is to make sure the same deployer (both functionality and address) is the same on real networks.
It makes no sense to have a repository available only during local testing.
The default deployer is not Foundry's, but Arachnid's deterministic deployer , which is a de-facto deployer on many networks.
It does require a non-EIP155 transaction to get deployed, so either it was deployed before EIP-155 was introduced (e.g. on mainnet, Polygon, etc), or had a specific "nonstandard state-change" to deploy this deployer (as it was on Base, which started as EIP-155-enabled chain, but still recognized the usefulness of this deployer)

A standard (or de-facto) deployer should be minimal: perform the deployment at a deterministic address across chains.

If you want a complex deployer with extra functionality - that's great, go and create one. You don't need specific support from Foundry (or from real-life network): Just deploy it using a standard deterministic deployer.

@sakulstra
Copy link
Contributor

In case this helps anyone:
we've been having issues with the deterministic deployer due to lacking support on some chains, so eventually we went with https://github.com/safe-global/safe-singleton-factory which works kinda fine with foundry and supports a few more chains.

The only issue we had with foundry was lack of verification support as foundry seems to not properly detect nested create calls, so we ended up using catapulta-verify which parses the foundry broadcast file and then uses debug_traceTransaction to find create calls and verify contracts.

@pcaversaccio
Copy link
Contributor

FWIW; CreateX is now live on 56 EVM chains: https://twitter.com/pcaversaccio/status/1738153235678998855. Besides the GH repo, you can also find all relevant information at createx.rocks.

@mds1
Copy link
Collaborator

mds1 commented Dec 22, 2023

Changing the default create2 deployer would be a breaking change, but a potential solution for forge v1 is:

  • Replace the current default create2 deployer with CreateX.
  • Since there is a private key that can be used for deployments, forge must check that the code at that address matches what's expected before using it.
  • Nominal deployments behave the same and route to the deployCreate2(bytes32 salt, bytes memory initCode) method, which is equivalent to the current deployer's functionality.
  • Add cheatcodes that allow the user to change which methods and features are used.

CreateX has enough features that, based on the above discussion, this should eliminate the majority of the use cases for a custom deployer.

Sample UX, open to feedback:

contract MyScript is Script {
    function run() external {
        vm.startBroadcast();

        // Default behavior, uses `CreateX.deployCreate2(bytes32,bytes)`
        new MyContract{salt: salt}();

        // Cheatcode names match the CreateX function names. The cheat arguments
        // match the CreateX function arguments, and are forwarded by forge to the
        // CreateX function. For safety/explicitness, we expect the contract deploy
        // call to match the CreateX function arguments as much as possible. Some
        // examples. Assume salt=bytes32(1) and values are 0 unless stated otherwise.

        vm.deployCreate3AndInit(salt, initCode, data, values, refundAddress);
        new MyContract(); // Revert: Salt was not provided.
        
        vm.deployCreate3AndInit(salt, initCode, data, values, refundAddress);
        new MyContract{salt: bytes32(0)}(); // Revert: Salts don't match.

        vm.deployCreate3AndInit(salt, initCode, data, values, refundAddress);
        new MyContract{salt: salt}(); // Success: Salt matches, value matches
        
        // Now assume values.constructorAmount=1 and values.initCallAmount=2,
        // so total expected value is 3
        vm.deployCreate3AndInit(salt, initCode, data, values, refundAddress);
        new MyContract{salt: salt}(); // Revert: Value is 0, expected 3.

        vm.deployCreate3AndInit(salt, initCode, data, values, refundAddress);
        new MyContract{salt: salt, value: 1}(); // Revert: Value is 1, expected 3.
        
        vm.deployCreate3AndInit(salt, initCode, data, values, refundAddress);
        new MyContract{salt: salt, value: 3}(); // Success

        vm.stopBroadcast();
    }
}

You can also have cheats to help users set the correct bytes to leverage the salt features.

One thing that's a bit awkward with this UX is you specify the initData for a contract during the cheat call, before executing the contract deploy. This is different from the normal flow of:

MyContract c = new MyContract();
c.initalize(initData);

So an alternative UX might be to instead not map cheats to function signatures 1:1 and have "logical" cheats based on features. But this gets clunky to read/write, and makes the forge integration much more complex as not every combination of features is available.

@Evalir Evalir changed the title feat: let user define create2 factory feat: migrate to CreateX for CREATE2 factory (or let user define create2 factory) Feb 25, 2024
@sambacha
Copy link
Contributor Author

Forge now has always_use_create_2_factory = false, which configures to use the create 2 factory in all cases including tests and non-broadcasting scripts.

@VargaElod23
Copy link

Hey guys! Unfortunately, again, not all chains are covered by the tools & workarounds you mentioned above. For example, I'm working right now to deploy the Uniswap V3 Universal Router on our testnet(Taraxa), and I am. running into a deployment error because of the fixed CREATE2 address... which obviously is a different one on our chain as I can't use the presigned tx from Nick.
In general, foundry and all the other tools are very much rigid on the ETH-based infra which is not the best arch to rely on, in my opinion.
Can you guys prio this? I can get started on a solution to make these constants more dynamic, but it'd be great if you could cover it.

@leon-do
Copy link

leon-do commented Mar 20, 2024

sounds good @gakonst, Ill start working on it.

As per my understanding the main goal here is to add a parameter like create2_deployer which lets a user use a custom create2 factory instead of the default 0x4e59b44847b379578588920ca78fbf26c0b4956c. This is useful for the following reasons:

Does this make sense? @mds1 @joshieDo

Agree here along with @VargaElod23 .

There are other chains with EIP-155 implemented which makes it difficult for us to deploy with https://github.com/Arachnid/deterministic-deployment-proxy

only replay-protected (EIP-155) transactions allowed over RPC

What's the solution if a chain supports EIP-155?

maybe @pcaversaccio has some input

@pcaversaccio
Copy link
Contributor

sounds good @gakonst, Ill start working on it.

As per my understanding the main goal here is to add a parameter like create2_deployer which lets a user use a custom create2 factory instead of the default 0x4e59b44847b379578588920ca78fbf26c0b4956c. This is useful for the following reasons:

Does this make sense? @mds1 @joshieDo

Agree here along with @VargaElod23 .

There are other chains with EIP-155 implemented which makes it difficult for us to deploy with https://github.com/Arachnid/deterministic-deployment-proxy

only replay-protected (EIP-155) transactions allowed over RPC

What's the solution if a chain supports EIP-155?

maybe @pcaversaccio has some input

There are a couple of solutions:

  • Ask to add the contract as a predeploy. (Very time-consuming, but for example, my Create2deployer got added as a predeploy to the OP stack)
  • Sometimes, the network allows for pre-EIP-155 transactions, but they block it at the RPC layer. In that case, spin up your own node, disable the protection, and broadcast the contract creation transaction.
  • Deploy via a backup private key if available. Would be possible for e.g. CreateX.

Btw, that's why we haven't chosen a keyless deployment for CreateX, see here my comments.

@pcaversaccio
Copy link
Contributor

FWIW, Hardhat Ignition (Hardhat's chainops solution) now uses CreateX as built-in factory by default (see here). When Foundry ;)?

@sambacha
Copy link
Contributor Author

From #7653

inject call to CREATE2 factory through custom revm handler

This changes flow for CREATE2 deployments routed through CREATE2 factory. Earlier we would just patch the sender of CREATE2 frame which resulted in data appearing in traces and state diff being different from the actual transaction data.

The way this PR addresses this is by implementing a revm handler register which replaces CREATE2 frame with CALL frame to factory when needed.

Solution

Implement InspectorExt trait with should_use_create2_factory method returning false by default which allows our inspectors to decide if the CREATE frame should be routed through create2 factory.

Implement handler register overriding create and insert_call_outcome hooks:

Overriden create hook returns CALL frame instead of CREATE frame when should_use_create2_factory fn returns true for the current frame.
Overriden insert_call_outcome hook decodes result of the create2 factory invocation and inserts it directly into interpreter.
Solution is not really clean because I had to duplicate parts of call and insert_call_outcome hooks from revm inspector hander register instead of just invoking them in register. The motivation behind this is following:

overriden CREATE2 invocation must be treated as a CALL by inspectors and EVM itself, however, its output should be written to interpreter as CREATE2 output which is different than CALL output in terms of stack and memory operations. Thus we can't reuse insert_call_outcome hook because it would invoke inspector.insert_call_outcome().
because we can't invoke insert_call_outcome, call cannot be reused as well because we'd mess up internal stack of inspector handler register (bluealloy/revm@f4f4745/crates/revm/src/inspector/handler_register.rs#L167)
This impl lets us keep cheatcodes inspector and overall flow simpler however it introduces additional complexity in EVM construction step and might result in issues if revm implementation of inspector handler register changes, so not sure if we should include that

thanks @klkvr for following up on this issue and providing the fixes

@sambacha
Copy link
Contributor Author

sounds good @gakonst, Ill start working on it.
As per my understanding the main goal here is to add a parameter like create2_deployer which lets a user use a custom create2 factory instead of the default 0x4e59b44847b379578588920ca78fbf26c0b4956c. This is useful for the following reasons:

Does this make sense? @mds1 @joshieDo

Agree here along with @VargaElod23 .

There are other chains with EIP-155 implemented which makes it difficult for us to deploy with Arachnid/deterministic-deployment-proxy

only replay-protected (EIP-155) transactions allowed over RPC

What's the solution if a chain supports EIP-155?

maybe @pcaversaccio has some input

You can try sending a legacy (non-EIP1557 based) transaction. You can send non-EIP155 transactions over RPC, this is supported by certain RPC providers. I can only think of one chain (Harmony I think) that prevents non-EIP155 transactions at the protocol level.

@pcaversaccio
Copy link
Contributor

You can try sending a legacy (non-EIP1557 based) transaction. You can send non-EIP155 transactions over RPC, this is supported by certain RPC providers. I can only think of one chain (Harmony I think) that prevents non-EIP155 transactions at the protocol level.

FWIW, Filecoin is another one not supporting pre-EIP-155 transactions (see here). Also, as a good reminder, that since the Berlin hard fork, Geth rejects all sending of pre-EIP-155 txs by default (see ethereum/go-ethereum#22339), i.e. even if there are networks that would support them at the network layer, the RPCs usually reject them since they use the default Geth config unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-forge Command: forge Cmd-forge-script Command: forge script T-feature Type: feature
Projects
No open projects
Status: In Progress
Development

No branches or pull requests