-
Notifications
You must be signed in to change notification settings - Fork 153
test(l1): add fuzzing tests and security workflows #5788
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
base: main
Are you sure you want to change the base?
Conversation
Dependency ReviewThe following issues were found:
License Issuestooling/Cargo.lock
OpenSSF Scorecard
Scanned Files
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds a comprehensive fuzzing infrastructure to ethrex, including fuzz targets for critical components and security-focused CI/CD workflows. The changes enhance the project's security posture through systematic testing of precompiles, RLP encoding/decoding, trie operations, and transaction/block parsing.
Key Changes:
- Added 11 fuzz targets covering RLP, trie, transactions, blocks, and all EVM precompiles (including BN254 and BLS12-381)
- Integrated property-based tests using proptest for RLP encode/decode roundtrip validation
- Added security workflows for dependency auditing, license checking, and unsafe code analysis
- Fixed Ethereum trie semantics to properly handle empty values in leaf nodes
Reviewed changes
Copilot reviewed 26 out of 28 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tooling/fuzz/Cargo.toml | Defines fuzzing package with 11 binary targets for different fuzzing scenarios |
| tooling/fuzz/.gitignore | Excludes fuzzing artifacts, corpus, and crash reports from version control |
| tooling/fuzz/README.md | Comprehensive documentation for running and managing fuzz targets |
| tooling/fuzz/fuzzers/*.rs | Fuzz targets for RLP, trie, transactions, blocks, and precompiles |
| tooling/Cargo.toml | Added fuzz package to workspace members |
| crates/common/trie/node/leaf.rs | Fixed empty value handling to match Ethereum trie semantics |
| crates/common/rlp/encode.rs | Added property-based roundtrip tests for RLP encoding |
| crates/common/rlp/decode.rs | Added property-based robustness tests for RLP decoding |
| Cargo.toml | Added testing dependencies (proptest, arbitrary, test-strategy) |
| deny.toml | Configuration for cargo-deny security and license checks |
| .github/workflows/security.yaml | Workflow for security audits, dependency checks, and unsafe code analysis |
| .github/workflows/fuzz.yaml | Weekly fuzzing runs and manual trigger support for all fuzz targets |
| .github/workflows/pr_proptest.yaml | Property tests for RLP and trie on relevant PRs |
| .github/workflows/pr_dependency_review.yaml | Dependency review for PRs modifying Cargo files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Lines of code reportTotal lines added: Detailed view |
| // If data starts with a type byte, try the typed transaction format | ||
| if let Some(&tx_type) = data.first() { | ||
| if let Some(tx_data) = data.get(1..) { | ||
| match tx_type { | ||
| 0x01 => { | ||
| let _ = EIP2930Transaction::decode(tx_data); | ||
| } | ||
| 0x02 => { | ||
| let _ = EIP1559Transaction::decode(tx_data); | ||
| } | ||
| 0x03 => { | ||
| let _ = EIP4844Transaction::decode(tx_data); | ||
| } | ||
| 0x04 => { | ||
| let _ = EIP7702Transaction::decode(tx_data); | ||
| } | ||
| _ => {} | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why attempt to decode twice for each type? (L22-26, and here)
| let calldata = Bytes::from(input.calldata); | ||
|
|
||
| // Use a generous gas limit to allow the precompile to run | ||
| let mut gas_remaining: u64 = 10_000_000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be set somewhere above the EIP-7825 cap.
| } | ||
|
|
||
| fuzz_target!(|input: Bls12Input| { | ||
| let mut gas_remaining: u64 = 100_000_000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably use the same gas limit for all tests.
Motivation
Add more tests
Description
Add a fuzzing toolset that tests precompiles, trie, rlp, encoding/decoding.
Also added workflows to run the fuzzing tests and security checks