Add proptest-based fuzz and pipeline integration tests for Solana IDL parsing#203
Conversation
There was a problem hiding this comment.
Pull request overview
Adds property-based fuzzing and end-to-end pipeline integration tests to harden Solana IDL parsing and visualization against panics and regressions.
Changes:
- Added proptest fuzz tests covering
decode_idl_data+parse_instruction_with_idlcrash-safety, plus roundtrip and SizeGuard boundary assertions. - Added full-pipeline integration tests exercising
transaction_to_visual_signwith/without IDL registration, discriminator dispatch, named accounts, and multi-instruction transactions. - Updated
visualsign-solanato use ananchorageoss/solana-parsergit dependency and introduced a helper script to run fuzzing against embedded real-world IDLs.
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/chain_parsers/visualsign-solana/tests/pipeline_integration.rs | New end-to-end + proptest-based pipeline tests for IDL-driven visualization. |
| src/chain_parsers/visualsign-solana/tests/fuzz_idl_parsing.rs | New proptest fuzz + roundtrip tests for solana_parser IDL parsing APIs. |
| src/chain_parsers/visualsign-solana/tests/fuzz_idl_parsing.proptest-regressions | Adds saved proptest regression seeds for reproducibility. |
| src/chain_parsers/visualsign-solana/Cargo.toml | Switches solana_parser git source and adds proptest dev dependencies. |
| src/Cargo.toml | No functional change (formatting). |
| src/Cargo.lock | Updates locked dependencies for new solana_parser/proptest usage. |
| scripts/fuzz_all_idls.sh | New script to run fuzz tests across embedded Solana IDLs. |
| .gitignore | Ignores src/.cargo/ for local dependency override configs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@prasanna-anchorage I've opened a new pull request, #219, to work on those changes. Once the pull request is ready, I'll request review from you. |
prasanna-anchorage
left a comment
There was a problem hiding this comment.
only real blocker is the compilation being blocked by the upstream solana-parser changes. I used Claude fairly heavily to track down corner cases I didn't immediately see beyond compiling and trying few things.
I agree with this assessment and copying as-is
What's strong
Crash-safety coverage is thorough. The 50/50 valid-discriminator / random-data split in fuzz_idl_parsing_never_panics is a well-chosen design — it forces the fuzzer into argument-decoding code paths, not just discriminator matching. The same pattern is applied consistently across 4 property tests and the pipeline layer.
The roundtrip tests are well-structured. Each concrete test in fuzz_idl_parsing.rs:267-556 isolates a single type concern (no-args, single primitive, mixed primitives, Option Some/None, Vec, Array, defined struct, multi-instruction dispatch) and verifies both parsing success and value correctness. These serve as specification-by-example.
SizeGuard boundary testing is excellent. Both the proptest (fuzz_size_guard_vec_length_prefix) and concrete tests (size_guard_huge_vec_length_prefix_is_rejected_cleanly, size_guard_vec_u64_over_budget) cover the DoS-mitigation path. The budget math is documented inline.
Real-IDL testing against 15 production IDLs via fuzz_all_idls.sh is a strong regression gate. The script is well-documented, builds once to avoid recompilation, and presents tabular output.
Pipeline integration tests in pipeline_integration.rs correctly verify the full stack (IdlRegistry wiring, visualizer dispatch, field layout) — not just the parser in isolation.
Strategy composition is clean. arb_idl_and_valid_bytes() demonstrates correct use of prop_flat_map with Arc-shared state. The real_idl_valid_data_always_parses_ok test correctly uses TestRunner::run directly for runtime-loaded IDLs.
Merge blockers
Blocker 1: Import path mismatch — solana_parser::arb does not exist
The test files import use solana_parser::arb; but the strategies actually live in a separate crate solana-parser-fuzz-core under pub mod proptest (not arb). The solana_parser crate itself has no
arb module, no proptest feature, and no re-export.
This will not resolve by simply merging solana-parser PR #1. PR #1 adds the fuzz-core crate with the strategies, but does not add pub mod arb or any re-export to solana_parser/src/lib.rs.
Fix required (one of):
- Option A (recommended): Add a re-export to solana-parser's lib.rs: #[cfg(feature = "proptest")] pub use solana_parser_fuzz_core::proptest as arb; and add a proptest feature to solana_parser's
Cargo.toml. This preserves the clean solana_parser::arb API the tests expect.
- Option B: Change the test Cargo.toml to depend on solana-parser-fuzz-core directly and rewrite all imports from solana_parser::arb::* to solana_parser_fuzz_core::proptest::*.
Affected files:
- tests/fuzz_idl_parsing.rs — line 16: use solana_parser::arb;
- tests/pipeline_integration.rs — line 22: use solana_parser::arb;
- Cargo.toml dev-dependencies — features = ["proptest"] on solana_parser (this feature doesn't exist)
Blocker 2: Compilation errors (9 + 7 errors)
As a consequence of Blocker 1, both test files produce cascading compilation errors:
- fuzz_idl_parsing.rs: 9 errors (E0432 unresolved import + 8 E0277 unsized type errors)
- pipeline_integration.rs: 7 errors (same pattern)
These block cargo test --all-targets and will fail CI.
No other hard blockers found beyond the improvement items below.
Switch from non-existent `solana_parser::arb` import to `solana_parser_fuzz_core::proptest` (Option B). Pin both solana_parser and solana-parser-fuzz-core git deps to rev a0c554d instead of a floating branch reference, ensuring reproducible builds. Addresses review blockers 1 & 2 (import path mismatch and floating branch dep) from PR #203. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
prasanna-anchorage
left a comment
There was a problem hiding this comment.
🚢 with some minor nits
Property tests verify that decode_idl_data and parse_instruction_with_idl never panic regardless of IDL shape or instruction byte content, and that valid borsh-encoded bytes always round-trip through the parser cleanly. - fuzz_idl_parsing.rs: crash-safety tests covering random IDLs, valid discriminator prefixes with random arg bytes, defined struct types, nested Vec/Option containers, and SizeGuard boundary conditions - pipeline_integration.rs: end-to-end tests through parse_transaction covering discriminator dispatch, field count invariants, named accounts, and multi-program transactions Depends on solana_parser::arb (solana-parser-add-arbitrary branch) which provides prop_recursive-based strategies for IdlType and correlated borsh byte generators for roundtrip assertions. Adds src/.cargo/config.toml (gitignored) as local dev override to redirect solana_parser git dep to local checkout without committing a [patch] block. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Switch from non-existent `solana_parser::arb` import to `solana_parser_fuzz_core::proptest` (Option B). Pin both solana_parser and solana-parser-fuzz-core git deps to rev a0c554d instead of a floating branch reference, ensuring reproducible builds. Addresses review blockers 1 & 2 (import path mismatch and floating branch dep) from PR #203. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add early return in fuzz_pipeline_idl_path_taken_on_valid_discriminator when idl.instructions is empty, preventing a panic from inst_idx % idl.instructions.len(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move duplicated disc+data building logic from pipeline_integration.rs into tests/common/mod.rs (build_disc_data, build_maybe_disc_bytes). Both test files now import the shared module. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add arb_defined_enum_idl_json() strategy and fuzz_defined_enum_types_never_panics proptest that exercises unit, tuple, and named enum variants through the Defined type resolution path. Previously only Struct variants were covered. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add a "Property-based testing (Solana)" section to testing-visualizations.mdx covering fuzz test execution, real-IDL testing with fuzz_all_idls.sh, roundtrip test types (concrete vs property-based), and how to add new tests. Add an "Adding new tests" section to the fuzz_idl_parsing.rs module doc so the file is self-documenting for contributors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add roundtrip_nested_defined_struct covering the case where a struct field references another defined struct (Order containing AssetConfig), exercising recursive type resolution through the types array. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace system program ID (111...1) with zeroes for TEST_PROGRAM_ID since parse_instruction_with_idl ignores it (_program_id). Avoids confusion with real known programs. Fix dead first branch in fuzz_all_idls.sh failed_count parsing — the summary line starts with the passed count so ^[0-9]+ failed never matches. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… dangling ref test
- Add arb_defined_alias_idl_json() strategy and
fuzz_defined_alias_types_never_panics proptest for
IdlTypeDefinitionType::Alias coverage.
- Upgrade arb_defined_struct_idl_json and arb_defined_enum_idl_json
to use arb_idl_type() instead of arb_primitive_idl_type() for inner
fields, so container types (Vec, Option, Array) are exercised inside
defined types.
- Add dangling_defined_reference_returns_err test verifying that a
Defined("MissingType") reference not in the types array returns Err
at parse time (decode_idl_data does not catch this upfront).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Run cargo fmt and replace redundant closures with function references (EnumFields::Named, EnumFields::Tuple) per clippy::redundant_closure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove redundant solana_parser entry from [dev-dependencies] — it is already declared in [dependencies] and visible to integration tests. Avoids rev-skew risk if one is bumped without the other. Remove src/.cargo/ from .gitignore — the directory does not exist and is no longer needed now that the git dep is pinned to a rev instead of a floating branch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Convert real_idl_never_panics from proptest! macro to TestRunner::run so the IDL is loaded from disk once instead of on every iteration (256+ redundant file reads). Add roundtrip_defined_enum_arg covering all three enum variant shapes (unit, named, tuple) as specification-by-example, matching the existing concrete roundtrip pattern for structs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
af53a79 to
9aa37fd
Compare
Summary
Adds property-based fuzz and pipeline integration tests for Solana IDL parsing.
fuzz_idl_parsing.rs: crash-safety property tests verifying thatdecode_idl_dataandparse_instruction_with_idlnever panic regardless of IDL shape or instruction byte content — covers random IDLs, valid discriminator prefixes with random arg bytes, defined struct types, nestedVec/Optioncontainers, andSizeGuardboundary conditionspipeline_integration.rs: end-to-end property tests throughparse_transactioncovering discriminator dispatch, field count invariants, named accounts, and multi-program transactionssrc/.cargo/config.toml(gitignored): local dev override that redirects thesolana_parsergit dependency to a local checkout without committing a[patch]blockThe tests depend on
solana_parser::arb, a new proptest strategies module being added to solana-parser in a companion PR.Merge order
Step 1: Merge anchorageoss/solana-parser#1 first. That PR adds the
proptestfeature andarbmodule to solana-parser.Step 2: Update
src/chain_parsers/visualsign-solana/Cargo.tomlin this PR — change bothsolana_parserentries frombranch = "solana-parser-add-arbitrary"to point atmain(or a pinned commit/tag on main).Step 3: Merge this PR.
Test plan
cargo test --test fuzz_idl_parsing— 19 tests passcargo test --test pipeline_integration— 9 tests pass🤖 Generated with Claude Code