Skip to content
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

Non-deterministic behavior with auto_detect_solc #5050

Closed
2 tasks done
fubhy opened this issue May 26, 2023 · 8 comments
Closed
2 tasks done

Non-deterministic behavior with auto_detect_solc #5050

fubhy opened this issue May 26, 2023 · 8 comments
Labels
A-config Area: config C-forge Command: forge Cmd-forge-build Command: forge build T-bug Type: bug
Milestone

Comments

@fubhy
Copy link
Contributor

fubhy commented May 26, 2023

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (33f3fee 2023-05-26T00:03:48.457406188Z)

What command(s) is the bug in?

forge build

Operating System

Linux

Describe the bug

NOTE: I also opened a feature request related to this: #5049

I'm currently debugging an issue where auto_detect_solc behaves somewhat non-deterministically and chooses different solc versions between CI and local runs. Locally, it uses 0.8.17 and in CI it uses 0.8.20 for some files.

The underlying issue seems to be that if it finds a file that has a version range that is supported by any installed solc version it will use that pre-installed solc version instead of downloading the highest available version. So depending on the environment it is run in, if there's already solc 0.8.17 available (which is the case locally), it will use that. But if that's not available (e.g. in CI where it installs those on every run), it will download the highest match (0.8.20) and then use that.

EDIT: This also seems to be affected by caches.

Given the non-deterministic / environment-dependent behavior of this I'd categorize that as a bug.

@mattsse
Copy link
Member

mattsse commented May 26, 2023

somewhat non-deterministically and chooses different solc versions between CI and local runs.

since this happens in CI I assume you have already compatible versions installed locally

ref: https://github.com/gakonst/ethers-rs/blob/master/ethers-solc/src/resolver/mod.rs#L17

@fubhy
Copy link
Contributor Author

fubhy commented May 26, 2023

somewhat non-deterministically and chooses different solc versions between CI and local runs.

since this happens in CI I assume you have already compatible versions installed locally

ref: https://github.com/gakonst/ethers-rs/blob/master/ethers-solc/src/resolver/mod.rs#L17

Ah, yes. That's where this feature requests would then come in imho: #5049

That wouldn't solve all edge cases but probably most of them (e.g. if there are still multiple possible versions within the configured range it wouldn't solve it but that's probably less likely).

@mattsse
Copy link
Member

mattsse commented May 26, 2023

I guess this is a reasonable request and could be solved by changed it to a set of solc versions.

supportive

@fubhy
Copy link
Contributor Author

fubhy commented May 26, 2023

I guess this is a reasonable request and could be solved by changed it to a set of solc versions.

supportive

Would you implement it by

a) turning solc_version into a set (optionally) and changing it's behavior to work "together" with auto_detect_solc. Such that:
if it's a set, auto_detect_solc is enabled, if it's a single value it's disabled and that single version is enforced

or b) add a new option auto_detect_solc_versions?

Happy to work on this and submit a PR. My preference would be a) if that can be implemented without causing any breaking changes.

@mattsse
Copy link
Member

mattsse commented May 26, 2023

turning solc_version into a set (optionally)

there's a distinction between a pinned version and a range, pinning a version makes finding a compatible version obsolete.
whereas a range would limit the possible options.

I'm not entirely sure how to implement this, the relevant code is here:

https://github.com/gakonst/ethers-rs/blob/6708c1fc0c43eb81ec6ade10cbb0a980e67ecdf6/ethers-solc/src/compile/project.rs#L156-L157

@fubhy
Copy link
Contributor Author

fubhy commented May 26, 2023

Gotcha. So I guess we'd want a third with_sources_and_solc_range path in ethers-solc first?

@mattsse
Copy link
Member

mattsse commented May 26, 2023

yeah, which provides a HashSet of allowed versions, I think that should work

@zerosnacks zerosnacks added A-config Area: config C-forge Command: forge Cmd-forge-build Command: forge build labels Jul 2, 2024
@zerosnacks zerosnacks added this to the v1.0.0 milestone Jul 26, 2024
@grandizzy
Copy link
Collaborator

this can be now achieved using newly introduced compilation restrictions (#8668), that is pin files to specific version. please use it and report if any issue with, thank you!

@github-project-automation github-project-automation bot moved this from Todo to Done in Foundry Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-config Area: config C-forge Command: forge Cmd-forge-build Command: forge build T-bug Type: bug
Projects
Status: Done
Development

No branches or pull requests

4 participants