feat: disable fuzz fixtures in Solidity #661
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
agostbiro
temporarily deployed
to
github-action-benchmark
GitHub Actions
Inactive
agostbiro
temporarily deployed
to
github-action-benchmark
GitHub Actions
Inactive
agostbiro
commented
Sep 11, 2024
| // No-op if the feature is disabled | ||
| if !self.solidity_fuzz_fixtures { | ||
| fixture_funcs.for_each(|func| { | ||
| log::warn!("Possible fuzz fixture usage detected: '{}', but solidity fuzz fixtures are disabled.", &func.name); |
There was a problem hiding this comment.
This is a noop unless a consumer is configured which isn't currently when ran from JS
Wodann
approved these changes
Sep 11, 2024
fvictorio
reviewed
Sep 12, 2024
|
|
||
| uint256[] public fixtureAmount = [ | ||
| // This is a random value | ||
| 7191815684697958081204101901807852913954269296144377099693178655035380638910 |
fvictorio
approved these changes
Sep 12, 2024
agostbiro
changed the base branch from
test/env-var-js-integration
to
feat/solidity-tests
agostbiro
force-pushed
the
feat/disable-fuzz-fixtures
branch
from
af1f8ca to
44f9422
Compare
agostbiro
had a problem deploying
to
github-action-benchmark
GitHub Actions
Error
agostbiro
had a problem deploying
to
github-action-benchmark
GitHub Actions
Error
agostbiro
changed the base branch from
feat/solidity-tests
to
test/env-var-js-integration
Base automatically changed from
test/env-var-js-integration
to
feat/solidity-tests
September 13, 2024 10:26
Co-authored-by: Franco Victorio <victorio.franco@gmail.com>
agostbiro
force-pushed
the
feat/disable-fuzz-fixtures
branch
from
44f9422 to
98de4da
Compare
agostbiro
had a problem deploying
to
github-action-benchmark
GitHub Actions
Error
agostbiro
had a problem deploying
to
github-action-benchmark
GitHub Actions
Error
agostbiro
temporarily deployed
to
github-action-benchmark
GitHub Actions
Inactive
agostbiro
temporarily deployed
to
github-action-benchmark
GitHub Actions
Inactive
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Foundry supports specifying fuzz inputs in Solidity. They call this feature “fixtures”. This seems to follow from the “do everything in Solidity” philosophy and would probably make more sense to get as an input in EDR. The values could be then specified in a config in Hardhat or generated dynamically with JS.
For this reason, we don't want to let users specify fuzz inputs with Solidity, but fixtures are integral to the fuzz executor implementation, so while it’s possible to rip it out, it’d be a big effort to put it back later on. And we will want to add support for fuzz fixtures later on, just not via Solidity code.
So I added a
solidity_fuzz_fixturesconfig option which is always false for the JS runner and it turns off loading fixtures from Solidity, but the Rust integration test runner can turn it on to keep the tests covering this feature.This way users won’t be able to depend on Foundry-style fuzz fixtures by accident, but we can add a fuzz fixtures config option to the JS runner interface in the future with little effort.