Add deployImplementations to ForkLive flow

[maurelian]

TL;DR

Added support for deploying the develop versions of contract implementations alongside implementations from the forked network.

This is necessary because the ForkLive script currently only loads the pre-existing contracts from the production network. So we are now deploying the current implementation contracts so that the Proxies can be upgraded to them.

What changed?

• Modified Deploy.deployImplementations to:
  • accept a suffix parameter for naming implementations.
  • saving logic now appends the provided suffix to contract names. (ie. save("SystemConfigImpl_NextVersion")).
• Etching Deploy on the forked network to deploy new implementations.

[maurelian]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• OPCM Upgrade implementations only #13574
• Add deployImplementations to ForkLive flow #13573 👈 (View in Graphite)
• feat: remove contractAddressesPath and disk reading #13649
• Deploy Cleanup: Make Artifacts stand alone and etch into global state #13648
• develop

This stack of pull requests is managed by Graphite. Learn more about stacking.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.83%. Comparing base (a7c2ac6) to head (2013c01).

Additional details and impacted files

@@                          Coverage Diff                           @@
##           opcm-up/dont-read-addrs-from-disk   #13573       +/-   ##
======================================================================
+ Coverage                              45.89%   90.83%   +44.94%     
======================================================================
  Files                                    893      109      -784     
  Lines                                  74874     4595    -70279     
  Branches                                 768      768               
======================================================================
- Hits                                   34360     4174    -30186     
+ Misses                                 37820      334    -37486     
+ Partials                                2694       87     -2607     

Flag	Coverage Δ
contracts-bedrock-tests	90.83% <ø> (-0.05%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 785 files with indirect coverage changes

[mds1]

Can you update the PR description to say why we need this feature? From the description I understand the what, but not the why

[maurelian]

Added this:

This is necessary because the ForkLive script currently only loads the pre-existing contracts from the production network. So we are now deploying the current implementation contracts so that the Proxies can be upgraded to them.

[mds1]

I know we also talked about an approach where deployImplementations is create2 based, so we could have it do something like this:

• compute address for deploying a contract
• check if there's code there
• if code, contract is unchanged, skip deploy
• if no code, contract changed, execute deploy

Is my interpretation correct that this PR is an alternative solution to that one? The create2 based one feels cleaner and simpler to me though. Here we are further enshrining the save functionality by relying on it and I thought we wanted to move away from that?

[maurelian]

I'm not sure I follow.

A move towards create2 would have to happen in the DeployImplementations.s.sol script which deployImplementations just wraps.

While I agree that we should move away from save() if possible, I'm a bit afraid to open up that can of worms at this moment, as its very deeply integrated with the deploy scripts so it's hard to assess the effort required. In my mind it would require ForkLive.run() and Deploy.run() to both return very large structs similar (but not identical) to DeployOpChain Output, so that those values can be used in CommonTest.