fix: state diff for broadcasted CREATE2 deployments

[klkvr]

Motivation

Closes #7603

This logic was added in #7207 and it doesn't make much as sense as the CREATE2 is still being performed on the same call depth, it will be on depth+1 only when broadcasted onchain which is out of scope of state diff

Solution

[klkvr]

I actually don't think we need to record call to CREATE2 factory in state diff as well, as this call is not performed during execution, only when broadcasting txses. It could've make sense if we've actually overriden CREATE2 invocation with a CALL to factory, but right now we are basically recording state diff unrelated to actually executed instructions

[RPate97]

I wanted to add something to this PR as the original author of #7207. I believe that the state diff cheat code should return results that accurately represent the state transitions occurring during the script. I originally created #7207 because I noticed a case where the results recorded in the state diff did not accurately reflect the real state transitions. Namely, the state diff did not correctly record how Foundry handles create2 deployments when the default create2 factory contract is used, either because the always_use_create_2_factory option is enabled or because the user is broadcasting. This PR would cause the call from the create2 factory to deploy the contract to be recorded at an inaccurate depth, which creates a misalignment between the results of the state diff cheat code and the true state transitions.

I believe that #7603 could be addressed directly instead of by making the state diff yield results that do not align with the real-state transitions, but that might involve a more complex set of changes. So, I think this PR is a reasonable, simple solution to #7603.

However, I strongly disagree with the idea of removing the call to the create2 factory from the state diff cheat code because doing so would cause a significant and unnecessary difference between the results of the state diff cheat code and the real state transitions occurring when the create2 factory is used. Removing the call to the create2 factory from the state diff cheat code would also break critical functionality for us as Sphinx, as we rely on it to allow our users to deploy via create2 using native Foundry scripts.

[mds1]

I believe that #7603 could be addressed directly…but that might involve a more complex set of changes

Can you expand on your proposed alternative solution? Its unclear to me what you believe is the optimal solution is here

Also @klkvr it sounds like the simulation doesn’t actually touch the create2 deployer, and it’s only when converted to a broadcast tx that the deployer contract is involved? I’d like to understand why those are not consistent and if that relates to the issue here

[klkvr]

@mds1 when we occur CREATE2 deployment while simulating script, the caller of the call context is patched to CREATE2 deployer to ensure that address of the contract is deployed to the same address during both simulation and tx broadcast

#7207 altered depth of state diff entry with CREATE2 deployment, thus breaking this logic in create_end:

foundry/crates/cheatcodes/src/inspector.rs

Line 1461 in 440ec52

if create_access.depth == ecx.journaled_state.depth() {

this resulted in state diff entry not being updated with runtime code

[mds1]

the caller of the call context is patched to CREATE2 deployer to ensure that address of the contract is deployed to the same address during both simulation and tx broadcast

This makes sense, but wouldn’t you get the same address during simulation and broadcast by, during sim, preserving the context’s caller and having the caller execute a call to the create2 deployer? (in other words, do the same thing the broadcast does)

[klkvr]

This makes sense, but wouldn’t you get the same address during simulation and broadcast by, during sim, preserving the context’s caller and having the caller execute a call to the create2 deployer? (in other words, do the same thing the broadcast does)

yep, I was planning to look into how we can do that, the issue here is that revm gives you ability to override caller/bytecode of the CREATE frame, but you can't currently change CREATE frame to a CALL frame (which is what actually happens when we use CREATE2 deployer)

[mds1]

Ohhh I see, that makes sense!

@JuanCoRo mentioned this PR did fix the original issue. So maybe path forward here is merge this fix so the cheatcode works properly for recording code, and for now tolerate and document that it slightly misrepresents the real trace. Then a follow up PR changes how sims work to match the actual trace by modifying the sim’s create2 to a call, and the depth issue would get fixed as a side effect?

[mds1]

Just updating that I've also tested and confirmed this resolves #7603

[RPate97]

Can you expand on your proposed alternative solution? Its unclear to me what you believe is the optimal solution is here

Essentially what you describe in this comment. But since that's a complex change, I think this PR is a good immediate solution to #7603.

[mds1]

Thanks @RPate97, appreciate your feedback and understanding here. Approving since it seems we have a good agreed-upon path forward here