tests/fuzzers: move fuzzers into native packages #17688
Conversation
yperbasis
requested review from
AskAlexSharov,
Giulio2002 and
mh0lt
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the fuzzers from standalone packages in tests/fuzzers/ into native test functions within their respective packages, following the upstream go-ethereum pattern. The change removes the external fuzzer infrastructure and replaces it with Go's built-in fuzzing support using testing.F.
Key changes:
- Moved fuzzer implementations from
tests/fuzzers/to native_test.gofiles in their target packages - Converted standalone
Fuzz()functions toFuzzXxx(f *testing.F)test functions - Removed OSS-Fuzz integration script and external test infrastructure
Reviewed Changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/fuzzers/secp256k1/secp_test.go | Removed standalone secp256k1 fuzzer |
| tests/fuzzers/rlp/rlp_fuzzer.go | Removed standalone RLP fuzzer |
| tests/fuzzers/difficulty/difficulty-fuzz.go | Removed standalone difficulty fuzzer |
| tests/fuzzers/bn254/bn254_fuzz.go | Removed standalone bn254 fuzzer |
| tests/fuzzers/bls12381/precompile_fuzzer.go | Removed standalone BLS12-381 fuzzer |
| tests/fuzzers/bitutil/compress_fuzz.go | Removed standalone bitutil fuzzer |
| tests/fuzzers/abi/abifuzzer_test.go | Removed old ABI fuzzer test |
| execution/vm/runtime/runtime_fuzz_test.go | Refactored to native Go fuzzing format |
| execution/vm/contracts_fuzz_test.go | Added new native fuzzer for precompiled contracts |
| execution/types/rlp_fuzzer_test.go | Added native RLP fuzzer to types package |
| execution/abi/abifuzzer_test.go | Refactored ABI fuzzer to native format |
| common/bitutil/compress_test.go | Added native fuzzing functions |
| oss-fuzz.sh | Removed OSS-Fuzz build script |
| Makefile | Removed test-erigon-ext target |
| .github/workflows/test-erigon-is-library.yml | Removed library integration test workflow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rlp.Split(input) | ||
| if elems, _, err := rlp.SplitList(input); err == nil { | ||
| rlp.CountValues(elems) | ||
| } |
There was a problem hiding this comment.
Multiple functions (rlp.Split, rlp.SplitList, rlp.CountValues, rlp.NewStream.Decode) are called without checking their errors or return values. While this may be intentional for fuzzing to explore all code paths, consider adding comments explaining why errors are deliberately ignored to improve code clarity.
| } | |
| } | |
| // Intentionally ignore errors from Decode to allow fuzzing to explore all code paths. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…erigon into yperbasis/mv_fuzzers
|
just a question, is the removal of the secp256k1 fuzzer intentional? we do seem to be using |
|
also noticed that we can now get rid of |
Good catch. Let me move it to erigontech/secp256k1 (erigontech/secp256k1#6). |
I've restored its fuzzer. |
|
Actually, I've decided to remove most of |
See ethereum/go-ethereum#28467. Also removes most of
common/crypto/bn254. Completes #14554.