-
Notifications
You must be signed in to change notification settings - Fork 2.1k
draft: migrate from Nick's factory to CreateX #11451
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
base: master
Are you sure you want to change the base?
Conversation
pub const DEFAULT_CREATE2_DEPLOYER_DEPLOYER: Address = | ||
address!("0x3fAB184622Dc19b6109349B94811493BF2a45362"); | ||
address!("0xeD456e05CaAb11d66C4c797dD6c1D6f9A7F352b5"); | ||
|
||
/// The default CREATE2 deployer. | ||
pub const DEFAULT_CREATE2_DEPLOYER: Address = | ||
address!("0x4e59b44847b379578588920ca78fbf26c0b4956c"); | ||
address!("0xba5Ed099633D3B313e4D5F7bdc1305d3c28ba5Ed"); | ||
|
||
/// The initcode of the default CREATE2 deployer. | ||
pub const DEFAULT_CREATE2_DEPLOYER_CODE: &[u8] = &hex!( | ||
"604580600e600039806000f350fe7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe03601600081602082378035828234f58015156039578182fd5b8082525050506014600cf3" | ||
/// From: <https://etherscan.io/tx/0xceec66d7b71039b0de4ac870e23c10130d64db46911166a1b7a945c4576113e1> | ||
pub const DEFAULT_CREATE2_DEPLOYER_INITCODE: &[u8] = &hex!( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't all CREATE2
references be renamed to CREATEX
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes let's do that, considering we are planning to use the entire feature set of CREATEX, not merely its CREATE2 methods
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do however still allow users to define their own preferred factory using --create2-deployer
so we would lose symmetry there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think it's fine, as createX is a valid create2-deployer
@@ -613,7 +613,7 @@ impl Config { | |||
|
|||
/// Default create2 deployer | |||
pub const DEFAULT_CREATE2_DEPLOYER: Address = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we're here can we use this constant everywhere?
Please do not do this, here's why:
I'd be favour of including CreateX as an alternative deployment paradigm, but not replace the existing Create2 deployments. The existing implementation is unopinionated and requires minimal understanding of Create2 itself, which isn't true for CreateX. This implementation should also expose the additional capability that CreateX brings. These are different enough to warrant explicit choice. There are other (better) alternatives which are less opinionated such as Safe's Deployer or the KeylessCreate2Factory from 0age/OpenSea, if you're looking for a replacement (idk why). On a related note: Foundry should be recommending people to deploy their own Create2 factories, either permissioned or not, and defining the interface on how Foundry uses it. |
Hi @akshatmittal thanks for your feedback! I have updated the description of the ticket with our motivation and a suggested path of how we may implement the migration. Please note that it is always possible for users to pass the |
Some more relevant links to CreateX:
|
In strong support of this change. CreateX is basically universally deployed, freely deployable, and easily verifiable. The guarded salt mechanism, while misunderstood by most, is very helpful for preventing deployment frontrunning and enabling crosschain address control. Salts can be keyed to specific deployer addresses such that anyone other than the deployer will get a different address if they attempt to deploy with it, and deployment addresses can be made chain-specific (literally restricted to a single chain). Address computation can be easily performed in Solidity with helper functions. Despite being a breaking change, this is a good one. |
Thanks for the additional context, @zerosnacks! Some more thoughts:
Just want to note here, I like CreateX and use it fairly often. It just has some additional things to keep in mind which are often hard for newcomers to fully grasp. All I want to push towards is having a good experience for both newcomers and us who can write whatever deployment pattern we want either way. I'm very much looking forward to this, I've been maintaining my own deployment flow for way too long and this would simplify a bunch of it. |
Based on the feedback and positive signals received I feel comfortable moving ahead to continue development on this PR but we need to spend (much) more time working out the migration path in detail in regards to how to introduce this breaking change in a way that existing script suites do not break / silently fail. There is significant concern of loss of funds / misconfiguration / etc.. if we simply swap out the default implementation. I've pinged Etherscan to see if it would be possible to improve the developer experience of Nick's factory (https://etherscan.io/address/0x4e59b44847b379578588920ca78fbf26c0b4956c) so that the source code is verified and the |
Motivation
Start of migration to CreateX
Previous implementation is Nick's
deterministic-deployment-proxy
also referred to as Nick'sfactory
.Implementation is written in Yul: https://github.com/Arachnid/deterministic-deployment-proxy/blob/master/source/deterministic-deployment-proxy.yul and therefore does not verify.
Documentation is non-existent, its contract cannot be verified on Etherscan and the token field displays a glitchy name. It does not support CREATE3.
Goals of the migration, stated requirements:
CreateX adheres to all these goals.
Concerns:
Notably, Foundry already makes it possible for developers through the
--create2-deployer
flag to control which CREATE2 factory is used.With the migration we want to support the full range of benefits CreateX offers, partly through changes in Foundry, cheatcodes where applicable and relevant changes in
forge-std
(https://github.com/radeksvarz/createx-forge)Closes: #2638
Solution
This is known to be a large breaking change and we need to figure out how to communicate this and let users migrate themselves. We likely need a transition path where we initially still default to Nick's factory, introduce CreateX and its extensive functionality and then in a breaking release mark Nick's factory's path as deprecated (user receives a warning) followed by full deprecation and switch to CreateX.
Please note that the current state of the PR does not yet reflect this migration path.
PR Checklist