foundry-rs / foundry

Foundry is a blazing fast, portable and modular toolkit for Ethereum application development written in Rust.
https://getfoundry.sh
Apache License 2.0
8.17k stars 1.7k forks source link

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

Open sambacha opened 2 years ago

sambacha commented 2 years ago

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

mds1 commented 2 years ago

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

sambacha commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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:

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

wdyt @mds1

mds1 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

FoundryCreate2FactoryV8

mds1 commented 2 years ago

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:

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

sambacha commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

pcaversaccio commented 2 years ago

@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 commented 2 years ago

@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 commented 1 year ago

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 commented 1 year ago

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

zjesko commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

sambacha commented 1 year ago

Any movement on this for v1.0

zjesko commented 1 year ago

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

sambacha commented 1 year ago

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

zjesko commented 1 year ago

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 similar to 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)

sambacha commented 1 year ago

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 commented 1 year ago

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 https://github.com/foundry-rs/forge-std/pull/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 https://github.com/foundry-rs/foundry/issues/2638#issuecomment-1206855029, 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 commented 1 year ago

Hey guys, any update on this?

gakonst commented 1 year ago

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

callum-elvidge commented 1 year ago

Hey, any update on this?

gakonst commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

Here we goooooo

https://github.com/ethereum/EIPs/pull/7212#discussion_r1240215378

drortirosh commented 11 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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

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.

sambacha commented 7 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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:

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

pcaversaccio commented 6 months ago

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

sambacha commented 5 months ago

From https://github.com/foundry-rs/foundry/pull/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 commented 2 weeks ago

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 commented 2 weeks ago

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 https://github.com/ethereum/go-ethereum/pull/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.