test: add real-IDL structural validation tests by shahan-khatchadourian-anchorage · Pull Request #224 · anchorageoss/visualsign-parser

shahan-khatchadourian-anchorage · 2026-03-26T18:31:58Z

Summary

Extracts load_idl_from_env from fuzz_idl_parsing.rs into common/mod.rs so all real-IDL test files can share it
Adds real_idl_validation.rs with 4 deterministic structural invariant tests for production IDLs:
- real_idl_all_instructions_have_discriminators — every instruction has an 8-byte discriminator
- real_idl_discriminators_are_unique — no two instructions share a discriminator
- real_idl_instruction_names_are_unique — no duplicate instruction names
- real_idl_idl_hash_is_stable — compute_idl_hash is deterministic

These tests require no random input generation (unlike the proptest-based real_idl_* tests in fuzz_idl_parsing.rs). They assert invariants on the decoded IDL structure itself.

Stacked on

Add fuzz targets, semantic tests, and CI for visualsign-solana #204

Test plan

IDL_FILE=<path> cargo test --test real_idl_validation — 4 tests pass
cargo test --test fuzz_idl_parsing — existing tests still pass (load_idl_from_env now imported from common)

🤖 Generated with Claude Code

prasanna-anchorage · 2026-04-01T04:03:51Z

nit: we might eventually want this to be split per chain , so adding Solana suffix or something might be good from start

prasanna-anchorage · 2026-04-01T04:04:50Z

hm, can we load the nightly version from an env var?

prasanna-anchorage · 2026-04-01T04:04:56Z

prasanna-anchorage · 2026-04-01T04:05:33Z

hm, we couldn't use the Rust toolchain from StageX for our main package builds?

Copilot

Pull request overview

This PR expands Solana parser validation coverage by adding deterministic tests against real/embedded IDLs and by introducing shared test helpers, while also adding CI workflows and fuzz targets and hardening parts of legacy transaction decoding against malformed inputs.

Changes:

Added deterministic “semantic pipeline” tests using real embedded IDLs and realistic Borsh-encoded instruction data.
Added structural invariant tests for production IDLs loaded via IDL_FILE, and extracted reusable load_idl_from_env into shared test helpers.
Added Solana-specific proptest/fuzz CI workflows + new libFuzzer targets, and updated decoding code to avoid panics on malformed legacy messages.

Reviewed changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/rust-toolchain.toml	Pins Rust toolchain patch version.
src/chain_parsers/visualsign-solana/tests/semantic_pipeline.rs	New deterministic end-to-end pipeline tests using embedded IDLs.
src/chain_parsers/visualsign-solana/tests/real_idl_validation.rs	New structural invariant tests for real IDLs loaded from `IDL_FILE`.
src/chain_parsers/visualsign-solana/tests/pipeline_integration.rs	Refactors to use shared helpers from `tests/common`.
src/chain_parsers/visualsign-solana/tests/fuzz_idl_parsing.rs	Reuses shared `load_idl_from_env` helper.
src/chain_parsers/visualsign-solana/tests/common/mod.rs	Adds shared transaction/options/inspection helpers + `load_idl_from_env`.
src/chain_parsers/visualsign-solana/src/core/instructions.rs	Adds bounds checks to avoid panics while converting compiled instructions.
src/chain_parsers/visualsign-solana/src/core/accounts/decode.rs	Replaces underflow-prone arithmetic with `saturating_sub` and adds regression tests.
src/chain_parsers/visualsign-solana/fuzz/rust-toolchain.toml	Pins nightly toolchain for fuzzing.
src/chain_parsers/visualsign-solana/fuzz/fuzz_targets/fuzz_versioned_transaction.rs	New libFuzzer target for `VersionedTransaction` deserialization + full pipeline.
src/chain_parsers/visualsign-solana/fuzz/fuzz_targets/fuzz_transaction_string.rs	New libFuzzer target for transaction-string entry point.
src/chain_parsers/visualsign-solana/fuzz/Cargo.toml	Adds a cargo-fuzz workspace crate for Solana fuzz targets.
src/chain_parsers/visualsign-solana/fuzz/.gitignore	Ignores fuzz outputs (target/corpus/artifacts/coverage).
src/chain_parsers/visualsign-solana/.gitignore	Stops ignoring `Cargo.lock` so fuzz lockfile can be committed.
docs/contributor-guides/testing-visualizations.mdx	Documents semantic tests, cargo-fuzz usage, and CI label workflows.
.github/workflows/proptest-solana.yml	Adds label-gated Solana proptest workflow with failure labeling.
.github/workflows/main.yml	Makes main CI label-gated for PRs (and still runs on main).
.github/workflows/fuzz-solana.yml	Adds label-gated Solana fuzz workflow with failure labeling.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-01T18:46:20Z

+    // 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).
    let instructions: Vec<Instruction> = message
        .instructions
        .iter()
-        .map(|ci| Instruction {
-            program_id: account_keys[ci.program_id_index as usize],
-            accounts: ci
+        .filter_map(|ci| {
+            if (ci.program_id_index as usize) >= account_keys.len() {
+                return None;
+            }


For legacy transactions, a compiled instruction with an out-of-bounds program_id_index indicates a malformed message. Silently dropping the instruction can result in missing/hidden instructions in the rendered payload; consider returning a DecodeError instead of skipping so callers don’t get a partial visualization for invalid input.

Copilot · 2026-04-01T18:46:21Z

+            let accounts: Vec<solana_sdk::instruction::AccountMeta> = ci
                .accounts
                .iter()
-                .map(|&i| {
-                    solana_sdk::instruction::AccountMeta::new_readonly(
-                        account_keys[i as usize],
-                        false,
-                    )
+                .filter_map(|&i| {
+                    if (i as usize) < account_keys.len() {
+                        Some(solana_sdk::instruction::AccountMeta::new_readonly(
+                            account_keys[i as usize],
+                            false,
+                        ))
+                    } else {
+                        None
+                    }
                })


Similarly, omitting out-of-bounds account indices for legacy transactions can silently remove referenced accounts from the visualization. For legacy messages (no lookup tables), it’s safer to treat any account index >= account_keys.len() as a decode error rather than dropping those accounts.

Copilot · 2026-04-01T18:46:21Z

            let is_signer = i < message.header.num_required_signatures as usize;
            let is_writable = if i < message.header.num_required_signatures as usize {
                // For signers: readonly ones come at the end of the signer range
-                let readonly_signer_start = message.header.num_required_signatures as usize
-                    - message.header.num_readonly_signed_accounts as usize;
+                let readonly_signer_start = (message.header.num_required_signatures as usize)
+                    .saturating_sub(message.header.num_readonly_signed_accounts as usize);
                i < readonly_signer_start
            } else {
                // For non-signers: readonly ones come at the end of the non-signer range
-                let non_signer_index = i - message.header.num_required_signatures as usize;
-                let total_non_signers =
-                    message.account_keys.len() - message.header.num_required_signatures as usize;
-                let writable_non_signers =
-                    total_non_signers - message.header.num_readonly_unsigned_accounts as usize;
+                let non_signer_index =
+                    i.saturating_sub(message.header.num_required_signatures as usize);
+                let total_non_signers = message
+                    .account_keys
+                    .len()
+                    .saturating_sub(message.header.num_required_signatures as usize);
+                let writable_non_signers = total_non_signers
+                    .saturating_sub(message.header.num_readonly_unsigned_accounts as usize);
                non_signer_index < writable_non_signers
            };


Using saturating_sub prevents panics, but it also allows structurally invalid headers (e.g., num_required_signatures > account_keys.len(), or readonly counts exceeding totals) to be treated as OK, which can misclassify signer/writable flags and produce misleading UI. Consider validating the header invariants up front and returning a Parse/Decode error when they’re violated, while still avoiding panics.

Copilot · 2026-04-01T18:46:21Z

+| `proptest` | `proptest.yml` | `cargo test -p visualsign-solana` |
+| `fuzz` | `fuzz.yml` | Both libFuzzer targets for 30 seconds each |


Copilot · 2026-04-01T18:46:22Z

+    types: [opened, synchronize, reopened, labeled]

 jobs:
  ubuntu:
+    if: |
+      github.ref == 'refs/heads/main' ||
+      github.base_ref == 'main' ||
+      contains(github.event.pull_request.labels.*.name, 'ci')


The PR description focuses on adding real-IDL structural validation tests and extracting load_idl_from_env, but this diff also includes production parsing behavior changes (e.g., instruction/account decoding) and new CI workflows/fuzz targets. Consider updating the PR description (or splitting changes) so reviewers have an accurate scope and risk assessment.

Copilot · 2026-04-01T18:46:22Z

+    /// Malformed legacy message: header counts exceed account keys length.
+    /// Must not panic (saturating_sub prevents underflow).
+    #[test]
+    fn test_decode_accounts_inconsistent_header_no_panic() {
+        let account1 = Pubkey::new_unique();
+
+        // num_required_signatures (5) > account_keys.len() (1),
+        // num_readonly_signed_accounts (3) > num_required_signatures would be too,
+        // num_readonly_unsigned_accounts (2) > total non-signers (0).
+        let message = create_test_message(5, 3, 2, vec![account1]);
+
+        // Must not panic — the saturating_sub ensures graceful degradation.
+        let accounts = decode_accounts(&message).unwrap();
+        assert_eq!(accounts.len(), 1);
+    }
+
+    /// Malformed V0 message: header counts exceed account keys length.
+    /// Must not panic (saturating_sub prevents underflow).
+    #[test]
+    fn test_decode_v0_accounts_inconsistent_header_no_panic() {
+        use solana_sdk::message::{MessageHeader, v0::Message as V0Message};
+
+        let account1 = Pubkey::new_unique();
+
+        let v0_message = V0Message {
+            header: MessageHeader {
+                num_required_signatures: 10,
+                num_readonly_signed_accounts: 8,
+                num_readonly_unsigned_accounts: 5,
+            },
+            account_keys: vec![account1],
+            recent_blockhash: Hash::new_unique(),
+            instructions: vec![],
+            address_table_lookups: vec![],
+        };
+
+        // Must not panic — the saturating_sub ensures graceful degradation.
+        let accounts = decode_v0_accounts(&v0_message).unwrap();
+        assert_eq!(accounts.len(), 1);
+    }


These new tests assert that decode_accounts/decode_v0_accounts return Ok for inconsistent headers. If the decoder is updated to reject invalid headers (recommended to avoid misleading output), update these tests to assert an Err (while still ensuring no panic), so the test suite enforces rejection of structurally invalid messages rather than accepting them.

Copilot · 2026-04-01T18:46:22Z

+
+// Try to deserialize arbitrary bytes as a VersionedTransaction then pass it
+// through the full visualsign-solana stack. Exercises the versioned transaction
+// path including address table lookup handling and IDL dispatch.
+fuzz_target!(|data: &[u8]| {
+    if let Ok(tx) = bincode::deserialize::<VersionedTransaction>(data) {


bincode::deserialize::<VersionedTransaction>(data) has no size limit and can attempt very large allocations if the input encodes huge length prefixes, which can cause OOM kills and make the fuzz target noisy/unreliable. Consider using bincode::options().with_limit(...) (or an equivalent bounded config) for deserialization in this fuzz target.

Suggested change

// Try to deserialize arbitrary bytes as a VersionedTransaction then pass it

// through the full visualsign-solana stack. Exercises the versioned transaction

// path including address table lookup handling and IDL dispatch.

fuzz_target!(|data: &[u8]| {

if let Ok(tx) = bincode::deserialize::<VersionedTransaction>(data) {

use bincode::Options;

// Try to deserialize arbitrary bytes as a VersionedTransaction then pass it

// through the full visualsign-solana stack. Exercises the versioned transaction

// path including address table lookup handling and IDL dispatch.

fuzz_target!(|data: &[u8]| {

if let Ok(tx) = bincode::options()

.with_limit(1_048_576)

.deserialize::<VersionedTransaction>(data)

{

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>

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>

- 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>

Extract load_idl_from_env into common/mod.rs and add real_idl_validation.rs with deterministic structural invariant tests for production IDLs: discriminator presence/uniqueness, instruction name uniqueness, and IDL hash stability. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…ightly env var - Rename fuzz.yml -> fuzz-solana.yml and proptest.yml -> proptest-solana.yml with chain-scoped workflow names for future multi-chain CI - Extract nightly toolchain version to NIGHTLY_VERSION env var in fuzz workflow - Pin rust-toolchain.toml to 1.88.0 to match StageX pallet-rust:1.88.0 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

prasanna-anchorage · 2026-04-01T04:35:54Z

we lost the main branch here?

Sorry for the confusion. I turned the commits into verified ones, but I should have waited.

prasanna-anchorage · 2026-04-01T05:02:05Z

Out-of-bounds program_id_index silently skips the entire instruction. Out-of-bounds account indices silently omit accounts. The defensive logic is correct (no panic), but there's no tracing::warn! — operators get zero visibility into malformed input in production. It'd be good to add some fields eventually for those but warning might be okay too for now

I agree. Adding more fields now would improve visibility. Though, your point about warnings otherwise is well taken.

## Summary - Add `images/parser_cli/Containerfile` for Solana-only CLI build using StageX - Add `parser_cli` target to `Makefile` and `stagex.yml` matrix - **Incorporates PR #226**: stagex PR builds now require the `stagex` label instead of auto-triggering on path changes; GHCR login moved after build; disable setup-rust-toolchain built-in cache in main.yml Replaces #232 (rebased). Stacked on #224. ## Test plan - [ ] StageX build passes for `parser_cli` in CI (add `stagex` label to trigger) - [ ] Existing `parser_app` and `parser_gateway` builds unaffected - [ ] Push to main with relevant path changes still triggers the build - [ ] GHCR push still works on push to main 🤖 Generated with [Claude Code](https://claude.com/claude-code)

shahan-khatchadourian-anchorage mentioned this pull request Mar 26, 2026

test: add proptest-based real-IDL coverage tests #225

Merged

2 tasks

Base automatically changed from shahankhatch/add-fuzz-targets to main March 27, 2026 21:43

prasanna-anchorage reviewed Apr 1, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings April 1, 2026 18:40

Copilot started reviewing on behalf of shahan-khatchadourian-anchorage April 1, 2026 18:41 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

shahan-khatchadourian-anchorage added CI and removed CI labels Apr 1, 2026

shahan-khatchadourian-anchorage force-pushed the shahankhatch/real-idl-structural-tests branch from f16d729 to a20deac Compare April 1, 2026 19:09

shahan-khatchadourian-anchorage added the CI label Apr 1, 2026

shahan-khatchadourian-anchorage force-pushed the shahankhatch/real-idl-structural-tests branch from a20deac to f65ebb5 Compare April 1, 2026 19:30

shahan-khatchadourian-anchorage and others added 10 commits April 2, 2026 18:05

Allow triggering CI on any PR via 'ci' label

ff591ec

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>

Add failure labels to fuzz and proptest workflows

4d9e954

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>

Harden label steps in fuzz and proptest workflows

8eb03cd

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>

shahan-khatchadourian-anchorage force-pushed the shahankhatch/real-idl-structural-tests branch from f65ebb5 to cdaf441 Compare April 2, 2026 22:10

prasanna-anchorage approved these changes Apr 2, 2026

View reviewed changes

shahan-khatchadourian-anchorage mentioned this pull request Apr 3, 2026

ci: add parser_cli to StageX build matrix #232

Closed

5 tasks

prasanna-anchorage merged commit 35abf04 into main Apr 5, 2026
7 checks passed

prasanna-anchorage deleted the shahankhatch/real-idl-structural-tests branch April 5, 2026 13:44

shahan-khatchadourian-anchorage mentioned this pull request Apr 11, 2026

ci: add parser_cli to StageX build matrix #250

Merged

4 tasks

		\| `proptest` \| `proptest.yml` \| `cargo test -p visualsign-solana` \|
		\| `fuzz` \| `fuzz.yml` \| Both libFuzzer targets for 30 seconds each \|

Conversation

shahan-khatchadourian-anchorage commented Mar 26, 2026

Summary

Stacked on

Test plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants