-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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(script): support custom create2 deployer #9278
base: master
Are you sure you want to change the base?
Conversation
Makes sense, waiting for a follow up project to close it then 😀 |
Haha, so how about this one, is there anything I need to adjust |
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.
From what I see the PR is only for script, but it won't be applied in other scopes, as for labels in traces, contract verification, (default) computeCreate2Address cheatcode and other places, which I think could lead to inconsistencies in deployment process? I'd say a consistent experience would be preferred here, that is when setting the address as env var then to be reflected in all places.
@grandizzy thanks for the advice, I use forge to deploy contracts in a private evm chain, so only tested the And If the changes are large, I'd prefer to split them into small PRs to include them all, else just in this one PR, WDYT? |
ah, I see, then maybe should be done as a new |
I've tried this approach, but failed, we need to register the create2 handler here foundry/crates/evm/core/src/utils.rs Lines 177 to 210 in 5e254e0
seems it's hard to pass the cli argument into create2 schema, else maybe we need to adjust it in revm's codebase. |
crates/evm/core/src/constants.rs
Outdated
pub static CREATE2_DEPLOYER: LazyLock<Address> = LazyLock::new(|| { | ||
match std::env::var("FOUNDRY_CREATE2_DEPLOYER") { | ||
Ok(addr) => Address::from_hex(addr).unwrap_or_else(|_| { | ||
error!("env FOUNDRY_CREATE2_DEPLOYER is set, but not a valid Ethereum address, use {DEFAULT_CREATE2_DEPLOYER} instead"); | ||
DEFAULT_CREATE2_DEPLOYER | ||
}), | ||
Err(_) => DEFAULT_CREATE2_DEPLOYER, | ||
} | ||
}); |
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.
@grandizzy wdyt on making this a Config
field? so that you can configure it through foundry.toml as well
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.
good point, yep, would be great to have such
we can add a helper to |
Thanks for the advice, I'll have a try, cli argument is much more implicit than environment. |
918f485
to
109a713
Compare
cd1e451
to
6b4b45b
Compare
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]> unit test Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
abbb7b1
to
b5928e4
Compare
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
Signed-off-by: jsvisa <[email protected]>
@grandizzy @klkvr sorry for the long delay, I think it's time to review. |
b5928e4
to
4f27792
Compare
Motivation
Currently when deploy contracts using CREATE2 method, it is default to
foundry/crates/evm/core/src/constants.rs
Line 44 in fcaa2a7
But in some circumstances, we can't access or deploy the 0x4e59b44847b379578588920ca78fbf26c0b4956c contract, may be caused of not able to send the pre EIP155 raw transaction. So try to introduce a env to change the default create2 deployer.
Solution
--create2-deployer
to theforge script
argumentcreate2_deployer
to thefoundry.toml
config file