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.

Sign up for GitHub

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

Jump to bottom

Add Fuzzer tests for Evm.StepN #547

Merged
merged 25 commits into from
Jul 3, 2024
Merged

Add Fuzzer tests for Evm.StepN #547

merged 25 commits into from
Jul 3, 2024

Conversation

LuisPH3
Copy link
Contributor

@LuisPH3 LuisPH3 commented Jun 20, 2024 •
edited by HerbertJordan
Loading

This PR adds 3 fuzzer functions for the ct Evm.StepN interface, mostly targeting the LFVM implementation.

  • FuzzLfvm: runs one instruction at the time and expects no panic.
  • FuzzGeth: equivalent to FuzzLfvm, but intended to be used when errors are found as a reference.
  • FuzzDifferentialLfvmVsGeth: Executes both lfvm and geth with the same input state and compares resulting states, in the same fashion as the ct does, but driven by the mutation+coverage algorithm if the fuzzing engine.

All fuzzing tests are automatically run as unit-tests with the initial test data + any test data files found in go/ct/testdata/fuzz/NameOfTheTest/*. Such files are automatically generated but the fuzzing engine when a failure or panic is found.

This PR does not:

  • Script the Fuzzing as a CI step, this task requires figuring out a method to download the mentioned testdata files.
  • Decide if we commit directly offending testdata files or if they shall be manually converted into a unit-test
    such tasks are still up for discussion.

@LuisPH3 LuisPH3 force-pushed the luis/fuzzer-interpreter-interface branch 2 times, most recently from 12760dd to 198620a Compare June 20, 2024 15:08
@LuisPH3 LuisPH3 marked this pull request as ready for review June 20, 2024 15:09
facuMH
facuMH requested changes Jun 21, 2024
View reviewed changes
Copy link
Contributor

@facuMH facuMH left a comment

Choose a reason for hiding this comment

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

Looks good, looking forward to the presentation !! that said, I still have a couple of comments

BUILD.md Outdated Show resolved Hide resolved
BUILD.md Outdated Show resolved Hide resolved
BUILD.md Outdated Show resolved Hide resolved
go/ct/evm_test.go Outdated Show resolved Hide resolved
go/ct/evm_test.go Outdated Show resolved Hide resolved
go/ct/evm_test.go Outdated Show resolved Hide resolved
go/ct/evm_test.go Outdated Show resolved Hide resolved
go/ct/st/code.go Outdated Show resolved Hide resolved
go/ct/st/code_test.go Outdated Show resolved Hide resolved
HerbertJordan
HerbertJordan requested changes Jun 21, 2024
View reviewed changes
Copy link
Collaborator

@HerbertJordan HerbertJordan left a comment

Choose a reason for hiding this comment

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

Looks good, just a few minor requests for improvement.

go/ct/evm_test.go Outdated Show resolved Hide resolved
go/ct/evm_test.go Outdated Show resolved Hide resolved
go/ct/evm_test.go Outdated Show resolved Hide resolved
go/ct/evm_test.go Outdated Show resolved Hide resolved
go/ct/evm_test.go Outdated Show resolved Hide resolved
BUILD.md Outdated Show resolved Hide resolved
BUILD.md Outdated Show resolved Hide resolved
go/ct/st/code.go Outdated Show resolved Hide resolved
go/ct/st/code_test.go Outdated Show resolved Hide resolved
go/ct/st/code.go Outdated Show resolved Hide resolved
@LuisPH3 LuisPH3 force-pushed the luis/fuzzer-interpreter-interface branch 2 times, most recently from 2d207b4 to 5cfb2ce Compare June 21, 2024 10:18
@LuisPH3 LuisPH3 added this to the Tosca CT Quality Engineering milestone Jun 21, 2024
@LuisPH3 LuisPH3 self-assigned this Jun 21, 2024
@LuisPH3 LuisPH3 requested review from HerbertJordan and facuMH June 21, 2024 10:58
@LuisPH3 LuisPH3 force-pushed the luis/fuzzer-interpreter-interface branch 2 times, most recently from 414e117 to 530b4a2 Compare June 21, 2024 12:42
HerbertJordan
HerbertJordan previously approved these changes Jun 23, 2024
View reviewed changes
Copy link
Collaborator

@HerbertJordan HerbertJordan left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

go/ct/st/code.go Outdated Show resolved Hide resolved
simonlechner
simonlechner reviewed Jun 24, 2024
View reviewed changes
go/ct/evm_fuzz_test.go Outdated Show resolved Hide resolved
go/ct/evm_fuzz_test.go Outdated Show resolved Hide resolved
facuMH
facuMH previously approved these changes Jun 24, 2024
View reviewed changes
Copy link
Contributor

@facuMH facuMH left a comment

Choose a reason for hiding this comment

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

looks good !! just a couple of sugestions/comments

go/ct/evm_fuzz_test.go Show resolved Hide resolved
go/ct/evm_fuzz_test.go Show resolved Hide resolved
go/ct/st/code.go Show resolved Hide resolved
@LuisPH3 LuisPH3 mentioned this pull request Jun 24, 2024
Closed
@LuisPH3
Copy link
Contributor Author

LuisPH3 commented Jun 24, 2024

After review changes, differential test found a new error in the lfvm: #547

facuMH
facuMH previously approved these changes Jun 24, 2024
View reviewed changes
go/ct/evm_fuzz_test.go Show resolved Hide resolved
go/ct/evm_fuzz_test.go Outdated Show resolved Hide resolved
go/ct/evm_fuzz_test.go Show resolved Hide resolved
go/ct/evm_fuzz_test.go Outdated Show resolved Hide resolved
go/ct/evm_fuzz_test.go Show resolved Hide resolved
LuisPH3
LuisPH3 commented Jun 24, 2024
View reviewed changes
go/ct/evm_fuzz_test.go Show resolved Hide resolved
go/ct/evm_fuzz_test.go Outdated Show resolved Hide resolved
go/ct/evm_fuzz_test.go Outdated Show resolved Hide resolved
@LuisPH3 LuisPH3 requested a review from facuMH June 24, 2024 15:31
LuisPH3 and others added 18 commits July 3, 2024 15:34
@LuisPH3 @simonlechner
Review comments.
acdadfa 
- use min-newestSupported ops
- fix issue with opcode range
- document stack seed
@LuisPH3 @simonlechner
Update doc
6f80bcf
@LuisPH3 @simonlechner
Add fuzzer concept for evmzero
01de6dd
@LuisPH3 @simonlechner
make HumanReadableString aware of code and data differences.
f720ee6
@LuisPH3 @simonlechner
prefix 0x to hex prints of opcodes
5792277
@LuisPH3 @simonlechner
link evmzero fuzzer issue
1f62cba
@LuisPH3 @simonlechner
refactor fuzzer initialization code together
5d60afb
@LuisPH3 @simonlechner
use slices instead of arrays,
d3a958b 
fix real max stack size.
@LuisPH3 @simonlechner
Review changes:
46f0b97 
- use string builder instead of chain of concatenations
- Rename humanreadablestring to Toxxxx to better reflect data transformation.
@LuisPH3 @simonlechner
review, correct ticket reference
0d481fc
@LuisPH3 @simonlechner
update build.md
cf91c2e
@LuisPH3 @simonlechner
Write down next steps for expansion of fuzzing tests.
0748b79
@LuisPH3 @facuMH @simonlechner
Apply suggestions from code review
cc04eae 
Co-authored-by: Facu <[email protected]>
@LuisPH3 @simonlechner
test for evm error result in crash test
8a66b73
@LuisPH3 @simonlechner
Comment about fuzz types
e8d5a1d
@LuisPH3 @simonlechner
Add test for fuzz corpus
2feedd0 
Rename corpus input data to corpus entry like in manual
@LuisPH3 @simonlechner
move rand into data generation function
47c8f08
@LuisPH3 @simonlechner
defer release, no if
61b0b1f
@simonlechner simonlechner force-pushed the luis/fuzzer-interpreter-interface branch from a87b31b to 61b0b1f Compare July 3, 2024 13:34
@facuMH facuMH dismissed stale reviews from simonlechner, HerbertJordan, and themself via 113e0fc July 3, 2024 14:51
@facuMH facuMH requested review from HerbertJordan and facuMH July 3, 2024 15:04
@HerbertJordan HerbertJordan merged commit eb2aafc into main Jul 3, 2024
5 checks passed
@HerbertJordan HerbertJordan deleted the luis/fuzzer-interpreter-interface branch July 3, 2024 15:37
@LuisPH3 LuisPH3 linked an issue Jul 11, 2024 that may be closed by this pull request
VM: implement interpreter interface based diff-Fuzzer #408
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@facuMH facuMH facuMH approved these changes

@HerbertJordan HerbertJordan HerbertJordan approved these changes

@simonlechner simonlechner simonlechner left review comments

Assignees

@LuisPH3 LuisPH3

Labels
None yet
Projects
None yet
Milestone
Tosca CT Quality Engineering
Development

Successfully merging this pull request may close these issues.

VM: implement interpreter interface based diff-Fuzzer
4 participants
@LuisPH3 @HerbertJordan @facuMH @simonlechner