feat: add support for binary_mappings and network_family configs

[popzxc]

Motivation

Fixes #9227

This PR makes it possible to use foundry profiles to redirect execution to alternative foundry implementations (e.g. forge to forge-zksync or anvil to anvil-zksync) in an extendible way.

Demo

foundry_zksync_demo.mp4

Solution

Note

Update: Now changes are made to be strict opt-in. User must set allow_alternative_binaries config variable to true, otherwise they'll be prompted to allow execution manually. Strict opt-out by setting it to false is also possible.

We introduce two new configuration options:

• binary_mappings: this setting allows you to specify where the execution should be redirected to. In essense, it's a map from enum with keys ["anvil", "cast", "chisel", "forge"] to arbitrary paths.
• network_family: an enum which currently has two options (ethereum and zksync) that allows overriding default configuration. Right now the only thing it does is override binary_mappings to ZKsync-specific values.

In combination, it makes it possible to define a profile for zksync, e.g.:

[profile.zksync]
network_family = "zksync"

and it would redirect commands to the foundry-zksync.

This way there is no need for foundry-zksync to override foundry binaries, as well as there is no need for users to change their scripts if they work with both upstream foundry and zksync foundry (which they would have to if we named binaries differently).

[kartojal]

would love this feature to be able to manage both foundry-zksync and foundry in same project without extra scripting

will this be added to foundry? or should foundry-zksync rename the binaries?

[popzxc]

This PR is being discussed with the Foundry team. The current status is that the team is not against these changes, but expects a bit more safety guards to be in place:

1. Binary attestations for foundry-zksync (so the binaries' authenticity can be verified).
2. An additional setting to opt-in for this feature.

We have already prepared attestations for foundry-zksync, and just waiting for the next stable release. After that I will update the PR and hopefully we will see progress here.

[popzxc]

Oh, I guess I need to rebase. Will do now.

[grandizzy]

@popzxc please fix failing test test_default_config. Overall I think this makes sense and addresses the security concerns I had, would be a good compromise for v1.0 (that is until we provide proper way for developers to use Foundry as a library, planned post v1.0). @zerosnacks @DaniPopes @mattsse @yash-atreya wdyt about moving forward with?

[popzxc]

Just pushed the fix for the test! Sorry for not fixing it right away -- unfortunately, running tests locally is a bit troublesome.

[zerosnacks]

I would prefer to hold off from merging this until after the 1.0 release when we will commit more time to thinking about this extended behavior and what the best way is to surface this to users.

Beyond a minor convenience of being able to use foundry-zksync commands through foundry it seems to have limited benefits to me and it comes at the cost of calling out to external binaries.

I'm more in favor of the larger and deeper integration proposed in #9651 over binary mappings.

It is good to see the security issues have been largely addressed, that is something I was also concerned about.

[popzxc]

@zerosnacks I would also prefer the deeper integration, but realistically I think it would take months, while this PR is enough to improve the life of developers right now.

I don't think that the convenience here is minor -- it's not only about running foundry locally, but also about not having to e.g. rework your CI and/or scripts, instead only having to provide a value for FOUNDRY_PROFILE (which many people already do). Based on the feedback we got from our users, this feature is very awaited (you can even see it in both this PR and the issue it resolves), and it would also make the development of foundry-zksync an order of magnitude simpler (since we won't have to rush merging upstream changes).

I've tried my best for this PR to leave the smallest footprint in the codebase possible, and will be happy to introduce and other requested changes, but I think it's really important to get this merged; especially given that it's not only about ZKsync Era -- with Abstract launched and having 1.5mil transactions already, Sophon launched with 25m transactions, and many more chains coming, the amount of foundry-zksync users is rapidly growing.

Once we have a more "native" integration ready, we can deprecate the binary remapping approach.

[zerosnacks]

I don't think that the convenience here is minor -- it's not only about running foundry locally, but also about not having to e.g. rework your CI and/or scripts, instead only having to provide a value for FOUNDRY_PROFILE (which many people already do). Based on the feedback we got from our users, this feature is very awaited (you can even see it in both this PR and the issue it resolves), and it would also make the development of foundry-zksync an order of magnitude simpler (since we won't have to rush merging upstream changes).

Fair point and wanting to reduce that friction for developers makes sense.

I was under the impression that developers were still developing codebases specifically for ZKSync due to compatibility issues and getting specific audits for it: https://docs.zksync.io/zksync-protocol/differences/evm-instructions

I've tried my best for this PR to leave the smallest footprint in the codebase possible, and will be happy to introduce and other requested changes, but I think it's really important to get this merged; especially given that it's not only about ZKsync Era -- with Abstract launched and having 1.5mil transactions already, Sophon launched with 25m transactions, and many more chains coming, the amount of foundry-zksync users is rapidly growing.

That makes sense, I wasn't familiar with the effort to generalize into a ZK Stack offering as shared in https://x.com/gluk64/status/1884360327829000593

Once we have a more "native" integration ready, we can deprecate the binary remapping approach.

Agreed.

I'm OK with the proposed approach given the additional context