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

feat: enable saving and loading of corpus for deterministic fuzzing #991

Closed
mds1 opened this issue Mar 19, 2022 · 15 comments · Fixed by #1658
Closed

feat: enable saving and loading of corpus for deterministic fuzzing #991

mds1 opened this issue Mar 19, 2022 · 15 comments · Fixed by #1658
Assignees
Labels
A-testing Area: testing C-forge Command: forge Cmd-forge-test Command: forge test P-normal Priority: normal T-feature Type: feature

Comments

@mds1
Copy link
Collaborator

mds1 commented Mar 19, 2022

Component

Forge

Describe the feature you would like

Add --save-corpus and --load-corpus <path> options to allow replaying of an entire fuzz campaign. I think proptest has methods to facilitate this but need to double check. It would also be great if saving a corpus saves off all inputs used (as opposed to just rng seeds) so they can be inspected/analyzed

Additional context

No response

@mds1 mds1 added the T-feature Type: feature label Mar 19, 2022
@onbjerg onbjerg added A-testing Area: testing Cmd-forge-test Command: forge test C-forge Command: forge P-normal Priority: normal labels Mar 21, 2022
@onbjerg
Copy link
Member

onbjerg commented Mar 21, 2022

Can you expand a bit on what is meant by campaign and corpus for the uninitiated?

@mds1
Copy link
Collaborator Author

mds1 commented Mar 21, 2022

A fuzz campaign is a term that basically means “execution of the fuzzer test suite with the configured params”

Corpus is a fuzzing term which refers to the set of inputs used by the fuzzer. More specifically it often (but not always) refers to the set of interesting inputs, i.e. the set of inputs that maximizes coverage. I can see an argument the “save corpus” isn’t the right terminology given we don’t have code coverage, but I think it’s a good term here since it makes it clear what you’re saving.

Pinging @transmissions11 for thoughts here too

@onbjerg
Copy link
Member

onbjerg commented Mar 22, 2022

Appreciate the clarification 😄

@mds1
Copy link
Collaborator Author

mds1 commented Apr 18, 2022

As part of this, it would also be great if failed fuzz runs were always saved off as corpus-latest.json (or whatever name/file type makes sense). That file would continually get overwritten with each new run resulting in test failures, but it's useful to simplify the process of re-executing failed runs. If you choose to keep the failed fuzz run calldata (or rng seed, however it works with proptest), you can simply rename the file

@Bind
Copy link
Contributor

Bind commented Apr 21, 2022

Is there a preferred format for the corpus json files?

@mds1
Copy link
Collaborator Author

mds1 commented Apr 21, 2022

I don't think so. If proptest has a default format it uses to save/load the data that should be fine, but I'll let @gakonst confirm to make sure there's nothing I'm missing

@Bind
Copy link
Contributor

Bind commented Apr 23, 2022

Hi hi, I did some preliminary digging into proptest and friends, a couple thoughts.

  • FailurePersistance is easily configurable, looks like the existing test runner config instantiations explicitly set FailurePersistence to None. Is that done for a specific reason?
  • RNG seeds could be easily saved and loaded.
  • ResultCache seems straight forward depending on what additional context we'd want to include in the corpus file.
  • We might want to tweak the cli opts to reflect its the seed rather then corpus file contents driving the fuzzing campaign, but I'm down for whatever.

There is a bit of chatter about #1359 but since this should ultimately manage/interact with the fuzzer rather than solidity-land they shouldn't overlap.

Will take a whack at this soooooon

@mds1
Copy link
Collaborator Author

mds1 commented Apr 23, 2022

Thanks for looking into this, very excited for this feature!

FailurePersistance is easily configurable, looks like the existing test runner config instantiations explicitly set FailurePersistence to None. Is that done for a specific reason?

None that I'm aware of. I'm guessing we just never set it up so used None by defaut. Seems we want to use the Direct option and pass a file path, and then by default failed fuzz runs can be saved off? And ideally replayed?

Which brings up the question—where should the output files be saved? I think the best approach may be:

  • Save them to out/fuzz/ by default.
  • If you want to keep the files, move them somewhere else.
  • By default this means the files are gitignored and deleted as part of forge clean
  • A flag in the config file can be used to change this directory

RNG seeds could be easily saved and loaded.

This seems like a nice option to expose in the config to allow deterministic fuzz runs by specifying a seed. We can probably just expose the seed and avoid exposing the algorithm and just keep using proptest's default for now.

How exactly would the seed be used by foundry? Specifically what I'm trying to get at is if the randomness is isolated per test, i.e. if I keep the seed constant, then add/remove tests, will each unchanged test still have the same inputs? Ideal answer is yes.

ResultCache seems straight forward depending on what additional context we'd want to include in the corpus file.

How does this work / what exactly would get cached here? Guessing it's the full set of inputs to each fuzz method, as well as pass/fail result for each input?

We might want to tweak the cli opts to reflect its the seed rather then corpus file contents driving the fuzzing campaign, but I'm down for whatever.

Depending on how the ResultCache works we may want both. Example behavior, open to suggestions:

  • fuzz-path defaults to out/fuzz/ which is where outputs are saved/loaded from by default
  • fuzz-input takes either (1) an RNG seed, or (2) filename to replay, where file can either be a FailurePersistence file containing just a handful of runs, or a ResultCache containing a full campaign
    • Alternatively we can split this up into fuzz-seed and fuzz-input-file with the latter taking precedence if specified

@Bind
Copy link
Contributor

Bind commented Apr 23, 2022

Thank you for the additional clarification! Based on the above, my initial implementation should look something like...

RNG Seeding

  • Extend foundry.toml to have a fuzz_seed properties.
  • Add --fuzz-seed option to forge test
  • Use new_with_rng to instantiate TestRunner if a seed is configured.
  • Verify RNG strategy always generates the same values per test.

Failure Persistence

  • Enable FailurePersistance to /cache/fuzz/failures-latest.json.
  • Add foundry.toml flag to specify outdir.
  • Ensure proptest can use FailurePersistance to resume fuzz campaigns.
  • Add --fuzz-input-file to allow user to specify a failures.json

Corpus Saving & Replay

  • Capture fuzz campaigns via ResultCache (Or some other mechanism TBD).
  • Persist fuzz campaigns to /cache/fuzz/corpus-latest.json.
  • Re-use foundry.toml flag to specify outdir.
  • Add a replay proptest strategy that is powered by a corpus.json file.
  • Extend --fuzz-input-file option to additionally take a corpus.json file & forge test to trigger replay.
  • Ensure /cache/fuzz is removed with forge clean .

lmk if I'm missing anything or should take a different path, otherwise I'll start chipping away!
(Updated as of 4/23 to incorporate below feedback)

@onbjerg
Copy link
Member

onbjerg commented Apr 23, 2022

I think it would be better to place the failures etc. in cache instead, out is more user-centered (artifacts etc.), otherwise looks good to me

@mds1
Copy link
Collaborator Author

mds1 commented Apr 23, 2022

Overall this plan sounds great to me as well, no objections with defaulting to cache instead of out.

The "RNG Seeding", "Failure Persistence", and "Corpus Saving and Replay" sections seem isolated enough that it could be good to do them incrementally as smaller PRs to keep scope of each smaller (will leave it up to @Bind and @onbjerg to figure out what's best, just a thought).

Extend foundry.toml to have a fuzz_seed and fuzz_seed_file properties.

One question is what do we need both of these for? I'm not sure exactly how the seed would be specified, but if it's something simple like an integer, then an alternate file option may be unnecessary?

@Bind
Copy link
Contributor

Bind commented Apr 23, 2022

sections seem isolated enough that it could be good to do them incrementally as smaller PRs

Good call, will do!

I'm not sure exactly how the seed would be specified, but if it's something simple like an integer, then an alternate file option may be unnecessary?

Fair point, I'll implement the fuzz_seed option first and we can go from there!

@gakonst
Copy link
Member

gakonst commented May 12, 2022

@Bind curious if you had any follow up on the fuzz seed option?

@Bind
Copy link
Contributor

Bind commented May 13, 2022

Sorry about the hold up! Getting back from some traveling tomorrow, will get RNG Seeding over the line in the next few days!

@Bind
Copy link
Contributor

Bind commented May 18, 2022

Didn't want this to drag for too long! I opened a Draft PR with my implementation and some outstanding questions I have about the best path for testing this feature!

@onbjerg onbjerg linked a pull request May 19, 2022 that will close this issue
@onbjerg onbjerg moved this from Todo to In Progress in Foundry May 19, 2022
mattsse added a commit to Bind/foundry that referenced this issue Jul 28, 2022
mattsse added a commit to Bind/foundry that referenced this issue Jul 28, 2022
mattsse added a commit to Bind/foundry that referenced this issue Jul 28, 2022
mattsse added a commit to Bind/foundry that referenced this issue Jul 28, 2022
mattsse added a commit to Bind/foundry that referenced this issue Jul 28, 2022
gakonst pushed a commit that referenced this issue Jul 30, 2022
* feat: basic rng seeding

* chore: bump u32 to U256

* feat(config): add additional helper macro

* feat: finish fuzz seed impl

* bump ethers

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
Repository owner moved this from In Progress to Done in Foundry Jul 30, 2022
iFrostizz pushed a commit to iFrostizz/foundry that referenced this issue Nov 9, 2022
* feat: basic rng seeding

* chore: bump u32 to U256

* feat(config): add additional helper macro

* feat: finish fuzz seed impl

* bump ethers

Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testing Area: testing C-forge Command: forge Cmd-forge-test Command: forge test P-normal Priority: normal T-feature Type: feature
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants