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

Initial implementation of differential fuzzing #385

Closed

Conversation

satyamakgec
Copy link
Contributor

@satyamakgec satyamakgec commented May 3, 2021

What was wrong?

  • Needs fuzzing of contracts.

How was it fixed?

It consists of the fuzzing setup for the erc20.fe and erc20.sol and has the rearrangements for the compiler tests as an independent directory to avoid circular dependency and it will also decrease the test execution time as it doesn't require rebuilding the dependencies again.

To-Do

  • Need to add fuzzing for ERC20.sol.

  • Add support for output gathering to differentiate outputs of the contract.

  • Add support for the gas analyzing for the contracts

  • OPTIONAL: Update Spec if applicable

  • Add entry to the release notes (may forgo for trivial changes)

  • Clean up commit history

@g-r-a-n-t g-r-a-n-t mentioned this pull request May 4, 2021
4 tasks
@satyamakgec satyamakgec marked this pull request as ready for review May 16, 2021 12:28
@codecov-commenter
Copy link

codecov-commenter commented May 16, 2021

Codecov Report

Merging #385 (97f4125) into master (df7c1d6) will decrease coverage by 0.12%.
The diff coverage is 87.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #385      +/-   ##
==========================================
- Coverage   86.07%   85.95%   -0.13%     
==========================================
  Files          67       68       +1     
  Lines        4008     4285     +277     
==========================================
+ Hits         3450     3683     +233     
- Misses        558      602      +44     
Impacted Files Coverage Δ
test-utils/src/lib.rs 84.83% <87.50%> (ø)
compiler/src/lowering/names.rs 66.66% <0.00%> (-6.67%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df7c1d6...97f4125. Read the comment docs.

@satyamakgec satyamakgec changed the title [WIP]: Differential fuzzing Initial implementation of differential fuzzing May 16, 2021
@sbillig sbillig mentioned this pull request May 21, 2021
3 tasks
@sbillig
Copy link
Collaborator

sbillig commented May 24, 2021

@satyamakgec Could you rebase your changes onto the master branch, and maybe squash some of your fixup commits? It's hard to navigate your changes.

@sbillig sbillig closed this May 24, 2021
@sbillig
Copy link
Collaborator

sbillig commented May 24, 2021

Whoops, misclicked! This laptop has a touchscreen, and there was some dirt over the 'close' button

@sbillig sbillig reopened this May 24, 2021
@sbillig
Copy link
Collaborator

sbillig commented May 25, 2021

@satyamakgec Trying to run the fuzzer fails with

error[E0433]: failed to resolve: could not find `erc20` in `fe_compiler_fuzz`
 --> fuzz_targets/erc20_target.rs:5:23
  |
5 |     fe_compiler_fuzz::erc20::fuzz_fe_erc20_constructor(data);
  |                       ^^^^^ could not find `erc20` in `fe_compiler_fuzz`

Same results with either cd fuzz && cargo build or cargo +nightly fuzz erc20_target. I'm not sure what's going on here. erc20 is clearly exposed by the lib, but maybe it's a simple issue with the file layout or cargo.toml that I'm not noticing. Ideas? Does this build for you? Should I be doing something else?

@satyamakgec
Copy link
Contributor Author

ohh sorry @sbillig I should have mention this in the readme. It only works with the feature flag i.e solc-backend so the right command to run it cargo +nightly fuzz run erc20_target --features solc-backend. And yes right now it is a mess up in the commits of the PR, I am going to re-open a clean new PR with these changes so it will be very easy to look over to the changes.

@satyamakgec
Copy link
Contributor Author

Closing in the favour of #421

satyamakgec added a commit to satyamakgec/fe that referenced this pull request May 27, 2021
satyamakgec added a commit to satyamakgec/fe that referenced this pull request May 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants