chore(solana): forbid HashMap/HashSet via clippy disallowed-types by prasanna-anchorage · Pull Request #288 · anchorageoss/visualsign-parser

prasanna-anchorage · 2026-05-08T17:02:33Z

Summary

Adds src/chain_parsers/visualsign-solana/clippy.toml enforcing disallowed-types for std::collections::HashMap and std::collections::HashSet so the determinism story in this crate is enforced by the compiler instead of by review.
Migrates all internal HashMap/HashSet usage in visualsign-solana to BTreeMap/BTreeSet.
Converts at exactly 3 FFI boundary points (solana_parser::parse_transaction, SolanaParsedInstructionData.named_accounts, generated SolanaMetadata.idl_mappings) using .into_iter().collect() into the inferred upstream type — no #[allow(clippy::disallowed_types)] escape hatches.

Why

The rendered SignablePayload JSON is consumed downstream for hashing and wallet display, so iteration order over any map that ends up in the output must be stable across runs. We just shipped a fix for exactly this bug in squads_multisig (PR #240, named_accounts was a HashMap, leaking nondeterministic ordering into the rendered output). Forbidding HashMap crate-wide prevents the pattern from being reintroduced.

Test plan

cargo clippy --all-targets -- -D warnings clean across the workspace
make -C src test passes (127 unit + 49 integration tests)
cargo fmt --check clean
CI green

Note on merge order

This lint will start failing CI on add-squads-multisig-visualizer and any other open PR that introduces new HashMap/HashSet usage in visualsign-solana once this merges. The Squads PR's named_accounts was already migrated to BTreeMap independently, so it should rebase cleanly. Any other open PR may need a small cleanup pass.

🤖 Generated with Claude Code

Copilot

Pull request overview

This PR enforces deterministic map/set iteration within visualsign-solana by introducing a Clippy disallowed-types rule for HashMap/HashSet, and migrating internal usage to BTreeMap/BTreeSet to prevent nondeterministic JSON rendering (which impacts downstream hashing and wallet display).

Changes:

Add crate-local clippy.toml to forbid std::collections::HashMap/HashSet in visualsign-solana.
Migrate internal maps/sets across presets, core, utils, and tests from HashMap/HashSet to BTreeMap/BTreeSet.
Add explicit boundary conversions (into_iter().collect()) where upstream/generated APIs still require HashMap.

Reviewed changes

Copilot reviewed 32 out of 32 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/chain_parsers/visualsign-solana/clippy.toml	Adds Clippy `disallowed-types` rules to forbid `HashMap`/`HashSet` crate-wide.
src/chain_parsers/visualsign-solana/src/core/mod.rs	Switches `SolanaIntegrationConfigData.programs` to `BTreeMap` for deterministic ordering.
src/chain_parsers/visualsign-solana/src/core/visualsign.rs	Updates extracted IDL mappings to use `BTreeMap` for stable ordering.
src/chain_parsers/visualsign-solana/src/core/txtypes/v0.rs	Uses `BTreeMap` locally and converts at the `solana_parser::parse_transaction` boundary.
src/chain_parsers/visualsign-solana/src/idl/mod.rs	Migrates `IdlRegistry` internal maps from `HashMap` to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/utils/mod.rs	Changes token lookup table to `BTreeMap` to align with determinism rule.
src/chain_parsers/visualsign-solana/src/presets/unknown_program/mod.rs	Uses `BTreeMap` when building named accounts and converts at assignment boundary.
src/chain_parsers/visualsign-solana/src/presets/unknown_program/config.rs	Converts config routing map to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/token_2022/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/system/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/swig_wallet/config.rs	Converts nested program/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/stakepool/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/orca_whirlpool/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/onre_app/config.rs	Converts nested program/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/neutral_trade/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/meteora_dlmm/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/meteora_damm_v2/mod.rs	Updates local named-accounts structure/comments related to determinism.
src/chain_parsers/visualsign-solana/src/presets/meteora_damm_v2/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/metadao_futarchy/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/metadao_conditional_vault/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/kamino_vault/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/kamino_farms/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/kamino_borrow/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/jupiter_perps/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/jupiter_earn/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/jupiter_borrow/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/dflow_aggregator/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/compute_budget/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/src/presets/associated_token_account/config.rs	Converts config routing/instruction maps to `BTreeMap`.
src/chain_parsers/visualsign-solana/tests/common/mod.rs	Uses `BTreeMap` locally and converts to generated-proto `HashMap` at the boundary.
src/chain_parsers/visualsign-solana/tests/real_idl_validation.rs	Replaces `HashMap`/`HashSet` in tests with `BTreeMap`/`BTreeSet`.

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

prasanna-anchorage · 2026-05-11T21:37:25Z

+        // (`parsed.named_accounts` from solana_parser is `BTreeMap`, so iteration there
        // is non-deterministic.)


Good catch — comment was incorrect. SolanaParsedInstructionData.named_accounts is a HashMap, not a BTreeMap. The non-determinism comes from iterating the upstream HashMap, not from BTreeMap. Fixed in de639a3.

prasanna-anchorage · 2026-05-11T21:37:27Z

+    // Boundary conversion: `SolanaParsedInstructionData.named_accounts` is the upstream
+    // solana_parser HashMap type; collecting into the inferred field type lets us
+    // build the local map as a BTreeMap (per crate-wide determinism rule in clippy.toml)
+    // and convert only at the assignment boundary.
+    parsed.named_accounts = named_accounts.into_iter().collect();


Right — this was a real determinism bug. The render path at L170 iterates parsed.named_accounts (the upstream HashMap), so the rendered field order was non-deterministic regardless of how the local map was built.

Fixed in de639a3 by refactoring try_parse_with_idl to return (SolanaParsedInstructionData, BTreeMap<String, String>) and updating the call site to iterate the local BTreeMap. The lossy collect() write back into parsed.named_accounts is removed entirely.

prasanna-anchorage · 2026-05-11T21:37:28Z

 /// Static lookup table for common Solana token addresses
-pub fn get_token_lookup_table() -> HashMap<&'static str, TokenInfo> {
-    let mut tokens = HashMap::new();
+pub fn get_token_lookup_table() -> BTreeMap<&'static str, TokenInfo> {
+    let mut tokens = BTreeMap::new();


visualsign-solana is a path-only workspace member (crates.io is not in scope — no [package] publish opt-in, only path = ... consumers in parser/cli, parser/app, and integration). No external consumers, so changing pub fn return types from HashMap → BTreeMap isn't a breaking-change risk in practice. Keeping the cleanup as-is rather than introducing accessor indirection that would only obscure the type.

prasanna-anchorage · 2026-05-11T21:37:29Z

 pub struct SolanaIntegrationConfigData {
-    pub programs: HashMap<&'static str, HashMap<&'static str, Vec<&'static str>>>,
+    pub programs: BTreeMap<&'static str, BTreeMap<&'static str, Vec<&'static str>>>,
 }


Same as the get_token_lookup_table reply: visualsign-solana is workspace-internal (path-dep only, no external consumers), so the SolanaIntegrationConfigData.programs field type change is contained. No version bump or compatibility shim needed.

Adds `src/chain_parsers/visualsign-solana/clippy.toml` enforcing `disallowed-types` for `std::collections::HashMap` and `std::collections::HashSet`, and migrates all internal usage in the visualsign-solana crate to BTreeMap/ BTreeSet. Rationale: the rendered SignablePayload JSON is consumed downstream for hashing and wallet display, so iteration order over any map that ends up in the output (e.g. `named_accounts` per preset) must be stable across runs. HashMap's randomized hasher silently breaks this — and we just shipped a fix for that exact bug in squads_multisig. Forbidding HashMap crate-wide prevents the pattern from being reintroduced. Boundary conversions (3 sites) collect into the inferred upstream type at the call site so we keep BTreeMap internally and only convert at FFI points (solana_parser::parse_transaction, SolanaParsedInstructionData.named_accounts, generated SolanaMetadata.idl_mappings). Verified: cargo clippy --all-targets -- -D warnings clean across the workspace, `make -C src test` passes (127 unit + 49 integration tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ebase test fix - unknown_program: refactor try_parse_with_idl to return (parsed, BTreeMap) so the render path iterates the local BTreeMap instead of the upstream HashMap-backed parsed.named_accounts (was a real determinism bug — fields rendered in HashMap iteration order). - meteora_damm_v2: correct in-source comment that mis-named the upstream type as BTreeMap; it is a HashMap. - dflow_aggregator: post-rebase test fix — the new test from #286 used std::collections::HashMap::new() which trips the new disallowed-types lint; switch to Default::default() (inferred HashMap, no source mention). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

#297) PR #288 forbade std::collections::HashMap/HashSet via clippy disallowed-types and converted the existing preset configs to BTreeMap, but missed the two presets that landed during the same review window: - exponent_finance (PR #275, merged 2026-05-12 22:00 UTC) - kamino_limit (PR #284, merged 2026-05-11 14:50 UTC) Both still constructed HashMap and passed it where SolanaIntegrationConfigData now expects BTreeMap, so `cargo build -p visualsign-solana` fails on post-#288 main. Verified locally: `cargo build -p visualsign-solana` and `cargo clippy -p visualsign-solana --all-targets -- -D warnings` are clean; `cargo fmt --check` passes. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Resolves #76. Adds a dedicated `SplTokenVisualizer` for the SPL Token program (`TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA`) so token transactions render with structured fields instead of falling back to `UnknownProgramVisualizer`. ## What it covers Full instruction coverage via `TokenInstruction::unpack()` from the `spl-token` crate, including: - MintTo / MintToChecked (amount + decimals + mint + destination) - Transfer / TransferChecked (amount + source + destination + owner) - Burn / BurnChecked (amount + account + mint + owner) - Approve / ApproveChecked (amount + source + delegate + owner) - InitializeMint / InitializeMintV1 - FreezeAccount / ThawAccount - SetAuthority (all authority types, including current authority signer) - Revoke, CloseAccount, default branch fallback For every instruction whose semantics depend on a signing owner / authority, the Expanded layout now surfaces that signer field (addressing the original Copilot review feedback on this PR): - Transfer / Burn / Approve: "Owner" from account index 2 - TransferChecked / ApproveChecked: "Owner" from account index 3 - BurnChecked: "Owner" from account index 2 - SetAuthority: "Current Authority" from account index 1 Without these the visualization would silently drop the security- relevant signer for the action. ## swig_wallet interaction `presets/swig_wallet/mod.rs::visualize_inner_instruction` now skips `SplToken` alongside `UnknownProgram`, so swig's inner-instruction summary keeps using its richer `format_token_instruction_summary` (From/To/Owner/Amount lines) instead of the new visualizer's terse title. ## Test coverage - 23 unit tests in `presets/spl_token/tests.rs` covering parsing (`unpack_instruction`) and visualization for every supported variant, including the default-branch path (Revoke, CloseAccount, InitializeMint). - Real-transaction fixture for MintTo at `tests/fixtures/spl_token/mint_to_example.json` with strict per-field assertions (the validation loop now fails on any divergence rather than just printing mismatches). - `core/visualsign.rs`: renamed `test_unknown_program_tokenkeg` -> `test_spl_token_tokenkeg_recognition` (now asserts TokenKeg is recognized as SPL Token) and added `test_unknown_program_fallback` with a clearly-fake program id to keep the unknown-program path exercised. Coverage on `presets/spl_token/mod.rs`: 82% lines / 86% regions, above the 70% target in TESTING.md. ## Build integration Auto-discovered by `presets/mod.rs` alphabetical ordering. Placed before `UnknownProgramVisualizer` in the visualizer chain so the generic catch-all only matches when no specific preset claims the program id. ## Rebase notes Rebased onto post-#288 main. `SplTokenConfig` was updated from `HashMap`/`HashSet` to `BTreeMap` to match the new `SolanaIntegrationConfigData` signature and to satisfy the workspace clippy `disallowed-types` rule introduced by #288. `cargo build -p visualsign-solana`, `cargo clippy -p visualsign-solana --all-targets -- -D warnings`, and `cargo test -p visualsign-solana` (165 unit tests including the 23 spl_token tests) all pass locally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Merges origin/main (3bbf4e7) into the PR #255 branch. Resolves three content conflicts and ports two new presets from main to the wire-data VisualizerContext API introduced by PR #255. Conflicts resolved: - .gitignore: keep both `docs/superpowers/` (PR #255) and `.surfpool/` (main, from #294). - src/chain_parsers/visualsign-solana/src/presets/dflow_aggregator/mod.rs: take main's #286 changes (remaining-accounts surfacing, nested-arg flattening, `mut expanded_fields` + warn-on-parse-fail) on top of PR #255's wire-data API. The raw-data field is now pushed in place via `expanded_fields.push(create_raw_data_field(data, ...))` using `context.data()` instead of the removed `instruction.data`. - src/chain_parsers/visualsign-solana/src/presets/unknown_program/mod.rs: combine main's #288 determinism fix (locally-built `BTreeMap<String, String>` returned alongside the parsed payload, iterated at render time instead of `parsed.named_accounts`'s upstream `HashMap`) with PR #255's wire-data API (`context.program_id()`, `context.data()`, `context.num_accounts()`, `resolve_account_str(context, i)`). Imports remain `BTreeMap`; `HashMap` is no longer referenced. Two new presets ported to the wire-data API (same pattern as the existing `80b076b` port commit): - exponent_finance (from main #275): `current_instruction()` / `instruction.{program_id,data,accounts}` replaced by `context.resolve_program_id()?`, `context.resolve_accounts()?`, `context.data()`. Inline `expanded_fields.push(create_raw_data_field(...))` pattern; the unused `append_raw_data` helper deleted. - kamino_limit (from main #284): same API port. Function helpers (`build_named_accounts`, `build_parsed_fields`, `build_fallback_fields`, `append_raw_data`) retained -- they take `data: &[u8]` and `accounts: &[AccountMeta]` and remain useful. Both new presets' `config.rs` switched from `std::collections::HashMap` to `std::collections::BTreeMap` to satisfy main's #288 disallowed-types lint and the `SolanaIntegrationConfigData.programs: BTreeMap<...>` field shape. Verified: - cargo fmt clean - cargo check --workspace --exclude parser_cli clean - cargo check -p parser_cli clean - cargo test -p visualsign-solana --lib: 148 passed, 0 failed Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings May 8, 2026 17:02

Copilot started reviewing on behalf of prasanna-anchorage May 8, 2026 17:03 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

prasanna-anchorage and others added 2 commits May 11, 2026 21:34

prasanna-anchorage force-pushed the prasanna/clippy-disallow-hashmap-in-solana-presets branch from 538a1a2 to de639a3 Compare May 11, 2026 21:35

prasanna-anchorage mentioned this pull request May 11, 2026

Add Kamino Limit Orders Solana preset #284

Merged

prasanna-anchorage requested a review from pepe-anchor May 12, 2026 14:02

prasanna-anchorage mentioned this pull request May 13, 2026

Add Exponent Finance Solana visualizer preset #275

Merged

pepe-anchor approved these changes May 13, 2026

View reviewed changes

prasanna-anchorage merged commit 3bbf4e7 into main May 13, 2026
8 checks passed

prasanna-anchorage deleted the prasanna/clippy-disallow-hashmap-in-solana-presets branch May 13, 2026 19:19

prasanna-anchorage mentioned this pull request May 13, 2026

fix(solana): use BTreeMap in exponent_finance and kamino_limit configs #297

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(solana): forbid HashMap/HashSet via clippy disallowed-types#288

chore(solana): forbid HashMap/HashSet via clippy disallowed-types#288
prasanna-anchorage merged 2 commits into
mainfrom
prasanna/clippy-disallow-hashmap-in-solana-presets

prasanna-anchorage commented May 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

prasanna-anchorage May 11, 2026 •

edited

Loading

Uh oh!

prasanna-anchorage May 11, 2026

Uh oh!

prasanna-anchorage May 11, 2026

Uh oh!

prasanna-anchorage May 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		// (`parsed.named_accounts` from solana_parser is `BTreeMap`, so iteration there
		// is non-deterministic.)

Conversation

prasanna-anchorage commented May 8, 2026

Summary

Why

Test plan

Note on merge order

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

prasanna-anchorage May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prasanna-anchorage May 11, 2026

Choose a reason for hiding this comment

Uh oh!

prasanna-anchorage May 11, 2026

Choose a reason for hiding this comment

Uh oh!

prasanna-anchorage May 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

prasanna-anchorage May 11, 2026 •

edited

Loading