Add fuzz targets, semantic tests, and CI for visualsign-solana#204
Conversation
ce09288 to
ed1c414
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
7e28e70 to
c767a98
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
@copilot fix this |
|
@shahan-khatchadourian-anchorage I've opened a new pull request, #205, to work on those changes. Once the pull request is ready, I'll request review from you. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
390ad35 to
d8d3301
Compare
d8d3301 to
8ece333
Compare
This comment was marked as outdated.
This comment was marked as outdated.
1 similar comment
Fuzz crash:@copilot please investigate the failure above and fix the issue in this PR. |
This comment was marked as resolved.
This comment was marked as resolved.
2 similar comments
Fuzz crash:@copilot please investigate the failure above and fix the issue in this PR. |
Fuzz crash:@copilot please investigate the failure above and fix the issue in this PR. |
Move proptest and fuzz jobs out of main.yml into dedicated workflow files (proptest.yml, fuzz.yml) so they appear as distinct named checks. Add pull-requests: write permission to fuzz job to allow posting crash comments via gh pr comment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Shared composite action posts crash/failure output as a PR comment and tags @copilot to fix the issue. Fuzz and proptest workflows use it via extract steps that write output to GITHUB_OUTPUT. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move build_transaction, options_with_idl, instruction_fields, find_text and related helpers from pipeline_integration.rs into tests/common/mod.rs so they can be reused by other test files without duplication. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Test the full visualization pipeline with realistic Borsh-serialized instruction data for Drift deposit, Lifinity swap, Raydium swapBaseInput, Orca swap (including u128 sqrtPriceLimit), Meteora swap, Kamino deposit, Stabble swap (Option<u64> arg), and OpenBook deposit. Each test uses assert_semantic() to verify decoded instruction name and arg values match what was serialized. Lives in its own file to keep pipeline_integration.rs focused on proptest-based property tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add 'labeled' to pull_request trigger types and allow the ubuntu job to run when the 'ci' label is present, not just for PRs targeting main. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Pin fuzz Cargo.lock to avoid spl-token-2022 breakage on nightly (spl-token-group-interface 0.7.2 pulled in solana-nullable which is incompatible with spl-token-2022 10.0.0) - Remove stale Cargo.lock gitignore rule from visualsign-solana - Remove post-failure-comment action and simplify fuzz/proptest workflows - Use continue-on-error on fuzz steps so crashes show as warnings (known pre-existing panics in instructions.rs bounds checking) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
On crash/failure, add fuzz-failure or proptest-failure label to the PR. On clean run, remove the label. This gives visible signal without blocking the check. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add || true to --add-label calls so the step doesn't fail if the label operation itself has issues (e.g. label not yet created in the repo). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
47efa1b to
0b545b9
Compare
Extend the property-based testing section with cargo fuzz targets, semantic pipeline tests, CI workflow labels (proptest, fuzz, ci), and failure label behavior (fuzz-failure, proptest-failure). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR expands the visualsign-solana test and CI surface by adding semantic integration tests against real embedded Solana IDLs, introducing libFuzzer-based fuzz targets, and wiring label-triggered GitHub Actions workflows to run these suites on demand.
Changes:
- Added deterministic “semantic pipeline” tests that construct Borsh-encoded instruction data for real embedded IDLs and assert correct decoded fields.
- Refactored shared integration-test helpers into
tests/common/mod.rsand updated existing pipeline integration tests to use them. - Introduced a
fuzz/cargo-fuzz harness (with pinnedCargo.lock) plus new label-triggered CI workflows for proptest and fuzzing; updated docs accordingly.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/chain_parsers/visualsign-solana/tests/semantic_pipeline.rs |
Adds new semantic end-to-end tests using real embedded IDLs and realistic argument bytes. |
src/chain_parsers/visualsign-solana/tests/pipeline_integration.rs |
Replaces duplicated helper code with imports from tests/common. |
src/chain_parsers/visualsign-solana/tests/common/mod.rs |
Centralizes shared transaction/options/field-inspection helpers for multiple integration tests. |
src/chain_parsers/visualsign-solana/fuzz/rust-toolchain.toml |
Adds nightly toolchain selection for cargo-fuzz. |
src/chain_parsers/visualsign-solana/fuzz/Cargo.toml |
Defines the cargo-fuzz harness crate and fuzz binaries. |
src/chain_parsers/visualsign-solana/fuzz/fuzz_targets/fuzz_transaction_string.rs |
Fuzz target for string transaction parsing path. |
src/chain_parsers/visualsign-solana/fuzz/fuzz_targets/fuzz_versioned_transaction.rs |
Fuzz target for VersionedTransaction deserialization and pipeline path. |
src/chain_parsers/visualsign-solana/fuzz/.gitignore |
Ignores fuzz build outputs/corpora/artifacts. |
src/chain_parsers/visualsign-solana/.gitignore |
Stops ignoring Cargo.lock so the fuzz harness lockfile can be committed. |
docs/contributor-guides/testing-visualizations.mdx |
Documents how to run proptests, semantic tests, and cargo-fuzz targets; explains label-triggered CI. |
.github/workflows/proptest.yml |
Adds label-triggered proptest workflow with PR labeling on failure. |
.github/workflows/fuzz.yml |
Adds label-triggered fuzz workflow running both fuzz targets and labeling PRs on failure. |
.github/workflows/main.yml |
Allows manual CI runs on non-main PRs via a ci label and expands PR event types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Pin nightly toolchain to 2026-03-13 (known-good) in both rust-toolchain.toml and fuzz.yml - Use --locked for cargo install cargo-fuzz in CI and docs - Cache fuzz/target/ and key off fuzz/Cargo.lock instead of src/Cargo.lock Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Use ..VisualSignOptions::default() in test helpers to prevent breakage when new fields are added to the struct - Pass github.event.pull_request.number through env var instead of direct context interpolation in shell blocks Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ng (#223) * fix: guard against panics on malformed transaction data Two panic sites in the legacy transaction path: 1. instructions.rs: unchecked indexing into account_keys with program_id_index and account indices from compiled instructions. Malformed transactions with out-of-bounds indices cause index-out-of-bounds panics. 2. accounts/decode.rs: arithmetic underflow when message header values (num_readonly_signed_accounts, num_readonly_unsigned_accounts) exceed the actual account keys array length. Fix: apply the same defensive patterns already used in the v0 transaction path — filter_map with bounds checks for instruction indices, saturating_sub for header arithmetic, and an explicit error for empty account keys. Verified with cargo-fuzz: ~930,000 runs across both fuzz targets (fuzz_transaction_string, fuzz_versioned_transaction) with zero crashes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address Copilot review: improve comments, error message, add tests - Clarify comment to distinguish skipped instructions (OOB program_id) from omitted accounts (OOB account index) - Use "Legacy transaction" in error message for consistency with v0 path - Add unit tests for decode_accounts and decode_v0_accounts with inconsistent header values to lock in saturating_sub behavior Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
|
||
| // Convert compiled instructions to full instructions. Instructions with an | ||
| // out-of-bounds program_id_index are skipped entirely, while individual | ||
| // out-of-bounds account indices are silently omitted (same approach as v0 transaction handling). |
There was a problem hiding this comment.
maybe this should be an error if the instructions on a corrupted account are silently dropped?
Summary
Adds cargo fuzz targets, semantic integration tests, CI workflows, and shared test infrastructure on top of PR #203's proptest foundation — without modifying any #203 files.
What's new (on top of #203)
Cargo fuzz targets (libFuzzer)
fuzz_transaction_stringtransaction_string_to_visual_signSignablePayloadfuzz_versioned_transactionversioned_transaction_to_visual_signVersionedTransaction→ versioned path including address table lookup handlingBoth targets use
continue-on-error: true— crashes are surfaced via thefuzz-failurelabel on the PR rather than failing the check. This lets pre-existing panics (e.g.instructions.rsbounds check) be visible without blocking.Semantic pipeline tests (
tests/semantic_pipeline.rs, 8 tests)Real embedded IDLs with Borsh-serialized instruction data:
Shared test helpers (
tests/common/mod.rs)Extracted
build_transaction,options_with_idl,instruction_fields,find_textfrompipeline_integration.rsintocommon/mod.rsso bothpipeline_integration.rsandsemantic_pipeline.rsshare them without duplication.CI workflows
proptest.yml— triggered byproptestlabel, runscargo test -p visualsign-solana. Addsproptest-failurelabel on failure, removes on clean run.fuzz.yml— triggered byfuzzlabel, runs each fuzz target for 30s with pinned nightly (nightly-2026-03-13). Addsfuzz-failurelabel on crash, removes on clean run.main.yml— now also triggers oncilabel for manual build verification on non-main PRsFuzz build fix
Pinned fuzz
Cargo.lockto avoidspl-token-2022breakage on nightly (spl-token-group-interface 0.7.2pulled insolana-nullablewhich is incompatible withspl-token-2022 10.0.0). Also removed staleCargo.lockgitignore rule fromvisualsign-solana/.gitignore.File separation
fuzz_idl_parsing.rsarb::*strategies (from #203, untouched)pipeline_integration.rscommon::*)semantic_pipeline.rscommon/mod.rs#203 preservation
All #203 files are intact —
fuzz_idl_parsing.rs,scripts/fuzz_all_idls.sh,Cargo.toml, andcommon/mod.rsoriginals are unchanged. No #203 fixes (import paths, divide-by-zero guard, pinned revs, duplicate dev-dep removal) are regressed.Test plan
cargo test -p visualsign-solana --test pipeline_integration— 9 tests passcargo test -p visualsign-solana --test semantic_pipeline— 8 tests passcargo test -p visualsign-solana --test fuzz_idl_parsing— 24 tests passcargo clippy -p visualsign-solana --tests -- -D warnings— cleancargo fmt -- --check— clean🤖 Generated with Claude Code