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

cannon: Clean up fuzz test todos #12009

Merged

Conversation

mbaxter
Copy link
Contributor

@mbaxter mbaxter commented Sep 19, 2024

Description

Clean-up some deferred todos from fuzz_evm_common_test.go. Remove special cases that skip a few types of validation and add assertions for all fields.

Also includes some small changes to test helpers:

  • Add some utils for adding length prefixes
  • Rework random utils - wrap them in a struct so we can easily use the same generator for all random values

Metadata

Fixes #11651

@mbaxter mbaxter changed the title Issue 11651/cleanup fuzz tests cannon: Clean up fuzz test todos Sep 19, 2024
@mbaxter mbaxter requested a review from Inphi September 19, 2024 16:12
@mbaxter mbaxter marked this pull request as ready for review September 19, 2024 16:12
@mbaxter mbaxter requested review from a team as code owners September 19, 2024 16:12
Copy link

codecov bot commented Sep 19, 2024

Codecov Report

Attention: Patch coverage is 62.68657% with 25 lines in your changes missing coverage. Please review.

Project coverage is 78.87%. Comparing base (57f9fbf) to head (2936afd).
Report is 5 commits behind head on develop.

Files with missing lines Patch % Lines
cannon/mipsevm/testutil/state.go 17.39% 19 Missing ⚠️
cannon/mipsevm/testutil/rand.go 80.64% 5 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #12009      +/-   ##
===========================================
- Coverage    79.20%   78.87%   -0.33%     
===========================================
  Files           41       41              
  Lines         3414     3437      +23     
===========================================
+ Hits          2704     2711       +7     
- Misses         541      557      +16     
  Partials       169      169              
Flag Coverage Δ
cannon-go-tests 78.87% <62.68%> (-0.33%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cannon/mipsevm/multithreaded/testutil/mutators.go 71.42% <100.00%> (ø)
cannon/mipsevm/multithreaded/testutil/thread.go 99.15% <100.00%> (ø)
cannon/mipsevm/singlethreaded/testutil/state.go 75.00% <100.00%> (ø)
cannon/mipsevm/testutil/rand.go 73.33% <80.64%> (+8.81%) ⬆️
cannon/mipsevm/testutil/state.go 71.15% <17.39%> (-13.06%) ⬇️

Copy link
Contributor

@Inphi Inphi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The hint expectations is a bit tricky to setup. But honestly couldn't figure out a better a way to simplify it.

@mbaxter mbaxter added this pull request to the merge queue Sep 19, 2024
@mbaxter
Copy link
Contributor Author

mbaxter commented Sep 19, 2024

The hint expectations is a bit tricky to setup

Yeah, that test is kind of gnarly, but at least now we're testing some more varied behavior. Before we were just processing random data, so we would mostly never process any hints because the hint length prefix was some huge random number.

Merged via the queue into ethereum-optimism:develop with commit 29dd98b Sep 19, 2024
60 checks passed
@mbaxter mbaxter deleted the issue-11651/cleanup-fuzz-tests branch September 19, 2024 18:16
blmalone pushed a commit that referenced this pull request Sep 19, 2024
* cannon: Add memory assertions to FuzzStatePreimageRead

* cannon: Rework hint write fuzz test to assert hint expectations

* cannon: Update FuzzStatePreimageWrite to assert on expected preimageKey

* cannon: Remove validation skipping logic from test util

* cannon: Cleanup - simplify code

* cannon: Cleanup - dedupe code
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.

cannon: Add more robust validation in Cannon fuzz tests
2 participants