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: add support in forge script for create2 contracts to have an extra call argument #2104

Closed
storming0x opened this issue Jun 23, 2022 · 11 comments
Labels
C-forge Command: forge Cmd-forge-script Command: forge script T-feature Type: feature
Milestone

Comments

@storming0x
Copy link

storming0x commented Jun 23, 2022

Component

Forge

Describe the feature you would like

Currently forge script supports create2 contract creation with the following syntax new MyContract{salt: mySalt}()

Would be great to also add the option for doing an extra call after contract creation in the same transaction for certain use cases like deploying proxy templates and factories you may need to initialize in the same transaction.

Additional context

No response

@storming0x storming0x added the T-feature Type: feature label Jun 23, 2022
@onbjerg onbjerg added this to Foundry Jun 23, 2022
@onbjerg onbjerg moved this to Todo in Foundry Jun 23, 2022
@mds1
Copy link
Collaborator

mds1 commented Jun 23, 2022

I have a similar use case of needing to initialize a proxy's implementation contract after deploying it with create2, and it'd be ideal to do that in the same tx. Suggested implementation path for this issue:

  • Fork https://github.com/Arachnid/deterministic-deployment-proxy (into the foundry-rs GitHub org?) and update the Yul contract so it makes a call on the newly provided contract if given data.
  • Deploy it on a bunch of chains with a fun vanity address. My suggestion is to use the old-fashioned same private key + nonce approach so we don't need to wait for an administrative deploy to place the contract at the right address on arbitrum (this was done with 0x4e59b44847b379578588920cA78FbF26c0B4956C since it was deployed with Nick's method)
  • Make sure this isn't a breaking change for anyone relying on the old address

Last part is figuring out the UX. Some options:

  • Auto-detect if the next call after a create2 deploy is a tx sent to the newly deployed contract. If so, automatically include it that call in the same tx.
  • Add a forge-std helper method such as deployCreate2(string contractName, bytes32 salt, bytes memory constructorData, bytes memory initData) which constructs and executes the tx

@storming0x
Copy link
Author

  • Add a forge-std helper method such as deployCreate2(string contractName, bytes memory constructorData, bytes memory initData) which constructs and executes the tx

This looks good, missing the salt arg?

@mds1
Copy link
Collaborator

mds1 commented Jun 23, 2022

Fixed, thanks!

@pcaversaccio
Copy link
Contributor

In case you are interested in an existing, preset CREATE2 factory, I deployed mine on 36 EVM chains: https://github.com/pcaversaccio/create2deployer. This factory would not allow for "doing an extra call after contract creation in the same transaction" but this can of course also be achieved sequentially, depending on your needs. Just wanted to let you know that there is something from my side that you could potentially recycle for your use case.

@mds1
Copy link
Collaborator

mds1 commented Jun 24, 2022

Thanks for the heads up! The current create2 deployer is deployed on many chains and can be deployed elsewhere by anyone with Nick's method, so IMO the only reason to switch to a new deployer would be if the new one can batch an initialization call in the same creation tx.

@pcaversaccio
Copy link
Contributor

and if you want to have a vanity address ;-) - can you quickly elaborate on what you mean with Nick's method here because if you want to replay the deployment transactions for my deployer it won't work since the chainId is encoded in v (i.e. EIP-155).

@mds1
Copy link
Collaborator

mds1 commented Jun 24, 2022

EIP-1820 has a good section on explaining nick's method, and the repo for Nick's deployer that foundry currently uses contains the transaction info. But yes to your point the transaction cannot use EIP-155 for this to work.

@pcaversaccio
Copy link
Contributor

pcaversaccio commented Jun 24, 2022

Ah thx, now I can follow you. Wasn't aware that foundry uses Nick's deployer as basis.

@onbjerg onbjerg added C-forge Command: forge Cmd-forge-script Command: forge script labels Jun 24, 2022
@onbjerg
Copy link
Collaborator

onbjerg commented Aug 11, 2022

@joshieDo Is this supported now?

@onbjerg onbjerg added this to the v1.0.0 milestone Aug 11, 2022
@mds1
Copy link
Collaborator

mds1 commented Aug 11, 2022

No because AFAIK the current create2 deployer doesn't support it. IMO we can close this since #2638 will resolve it (whether that's letting users specify their deployer or creating a new canonical one, both would resolve this)

@rkrasiuk
Copy link
Collaborator

closing in favor of #2638

Repository owner moved this from Todo to Done in Foundry Aug 29, 2022
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
Archived in project
Development

No branches or pull requests

5 participants