From 21896ef7511a32cb34540dbf419daf00fc3c302c Mon Sep 17 00:00:00 2001 From: Shahan Khatchadourian Date: Wed, 6 May 2026 11:47:34 -0400 Subject: [PATCH 1/2] chore(skill): refresh solana-add-idl for post-#228 VisualizerContext API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The skill scaffolds new IDL-based Solana visualizer presets. Two updates: - Replace the broken `squads_multisig` template reference (preset doesn't exist in this repo) with `dflow_aggregator`, which is a real working example of the post-#228 wire-data pattern. - Add explicit guidance for the new accessor pattern at the top of `visualize_tx_commands`: `context.resolve_program_id()?` / `context.resolve_accounts()?` / `context.data()`. These replace the removed `context.current_instruction()` and surface unresolved indices with precise error messages. - Update the "Required imports" list to match real preset code (BTreeMap for deterministic named-accounts ordering, AccountMeta, OnceLock). - Drop the redundant `Some(hex::encode(...))` second arg in `create_raw_data_field` calls — the helper falls back to the same lowercase hex by default. - Verify step now exercises both feature configs (diagnostics on/off) so newly scaffolded presets work for parser_cli and parser_app alike. Stacked on top of #255. --- .claude/skills/solana-add-idl/SKILL.md | 34 ++++++++++++++++++++------ 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/.claude/skills/solana-add-idl/SKILL.md b/.claude/skills/solana-add-idl/SKILL.md index 16b42b80..5bf5f733 100644 --- a/.claude/skills/solana-add-idl/SKILL.md +++ b/.claude/skills/solana-add-idl/SKILL.md @@ -86,20 +86,31 @@ impl SolanaIntegrationConfig for {PascalName}Config { ### File: `mod.rs` -Use the squads_multisig preset as a template: `src/chain_parsers/visualsign-solana/src/presets/squads_multisig/mod.rs` +Use the dflow_aggregator preset as a template: `src/chain_parsers/visualsign-solana/src/presets/dflow_aggregator/mod.rs` Read that file for the exact structure, then generate a generic version with these substitutions: -- Replace `SquadsMultisig` / `squads_multisig` / `SQUADS_MULTISIG` with the appropriate casing of the new program name +- Replace `DflowAggregator` / `dflow_aggregator` / `DFLOW_AGGREGATOR` with the appropriate casing of the new program name - Replace the program ID string with the new program address -- Replace `"Squads Multisig"` display strings with `{display_name}` +- Replace `"DFlow Aggregator"` display strings with `{display_name}` - Replace IDL file reference: `include_str!("{snake_name}.json")` - Keep the `kind()` method returning the user's chosen `VisualizerKind` variant with `display_name` as the `&'static str` argument -**Important — generic IDL pattern only:** -- DO NOT copy any squads-specific code (e.g. `VaultTransactionMessage` decoding, inner instruction handling) +**Generic IDL pattern only:** - The generic scaffold uses `build_named_accounts`, `build_parsed_fields`, `build_fallback_fields`, `append_raw_data`, `format_arg_value` — all of which work with any IDL - The parse function should: check `data.len() < 8`, load IDL, call `parse_instruction_with_idl`, call `build_named_accounts`, return a struct with parsed data + named accounts +**Visualizer body must use the wire-data context API.** At the top of `visualize_tx_commands`: +```rust +let program_id = context.resolve_program_id()?.to_string(); +let accounts = context.resolve_accounts()?; +let data = context.data(); +``` + +These three accessors replace the old `context.current_instruction()`. They surface +unresolved indices as `Err(VisualSignError::DecodeError(...))` with the bad index +named, instead of returning a generic "no instruction found" failure. Use +`context.instruction_index()` for any "Instruction N" labels. + **Required imports** (at top of module, NOT inside functions): ```rust use crate::core::{ @@ -109,7 +120,9 @@ use config::{PascalName}Config; use solana_parser::{ Idl, SolanaParsedInstructionData, decode_idl_data, parse_instruction_with_idl, }; -use std::collections::HashMap; +use solana_sdk::instruction::AccountMeta; +use std::collections::BTreeMap; +use std::sync::OnceLock; use visualsign::errors::VisualSignError; use visualsign::field_builders::{create_raw_data_field, create_text_field}; use visualsign::{ @@ -118,6 +131,8 @@ use visualsign::{ }; ``` +`BTreeMap` (not `HashMap`) keeps the rendered named-accounts order deterministic. + **Required tests** (in `#[cfg(test)] mod tests`): - `test_{snake_name}_idl_loads` — IDL loads and has instructions - `test_{snake_name}_idl_has_discriminators` — every instruction has an 8-byte discriminator @@ -138,6 +153,7 @@ Follow these rules in all generated code: - `use` statements at top of module, never inside functions - Inline format strings: `format!("{variable}")` not `format!("{}", variable)` - Use `create_text_field` and `create_raw_data_field` from `visualsign::field_builders` — never construct field structs directly +- For raw-data fields, pass `None` as the second arg of `create_raw_data_field` — the helper falls back to lowercase byte-by-byte hex; `Some(hex::encode(data))` is redundant - ASCII only in user-visible strings: `>=` not `≥`, `->` not `→` - Rust edition 2024 on nightly @@ -148,8 +164,12 @@ Run these commands and fix any issues: ```bash cargo fmt -p visualsign-solana cargo clippy -p visualsign-solana --all-targets -- -D warnings +cargo clippy -p visualsign-solana --features diagnostics --all-targets -- -D warnings cargo test -p visualsign-solana +cargo test -p visualsign-solana --features diagnostics make -C src test ``` -All must pass before the task is complete. +All must pass before the task is complete. Both feature configurations +(diagnostics on and off) need to compile and test cleanly because parser_app +builds without `diagnostics` while parser_cli builds with it. From d9b8f05dfb90e28240c8c5fadbe8fc32fe2a6b19 Mon Sep 17 00:00:00 2001 From: Shahan Khatchadourian Date: Thu, 14 May 2026 14:12:23 -0400 Subject: [PATCH 2/2] docs(skill): soften create_raw_data_field None rule per review Address Copilot review feedback on PR #281: the rule was overly strict. Reusing a precomputed hex string (e.g. one built for fallback_text, as in dflow_aggregator) is fine; the only thing to avoid is calling hex::encode(data) solely to populate the second arg. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/skills/solana-add-idl/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/skills/solana-add-idl/SKILL.md b/.claude/skills/solana-add-idl/SKILL.md index 5bf5f733..625f0f0a 100644 --- a/.claude/skills/solana-add-idl/SKILL.md +++ b/.claude/skills/solana-add-idl/SKILL.md @@ -153,7 +153,7 @@ Follow these rules in all generated code: - `use` statements at top of module, never inside functions - Inline format strings: `format!("{variable}")` not `format!("{}", variable)` - Use `create_text_field` and `create_raw_data_field` from `visualsign::field_builders` — never construct field structs directly -- For raw-data fields, pass `None` as the second arg of `create_raw_data_field` — the helper falls back to lowercase byte-by-byte hex; `Some(hex::encode(data))` is redundant +- For raw-data fields, pass `None` as the second arg of `create_raw_data_field` unless you already have a precomputed hex string to reuse (e.g. one you built for `fallback_text`). Do not call `hex::encode(data)` solely to populate this arg — the helper falls back to the same lowercase byte-by-byte hex on `None`. - ASCII only in user-visible strings: `>=` not `≥`, `->` not `→` - Rust edition 2024 on nightly