diff --git a/.gitignore b/.gitignore index 29d1ba5d..a7f35a4b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ **/target out +docs/superpowers/ .surfpool/ diff --git a/docs/contributor-guides/lint-diagnostics.mdx b/docs/contributor-guides/lint-diagnostics.mdx new file mode 100644 index 00000000..0319eab4 --- /dev/null +++ b/docs/contributor-guides/lint-diagnostics.mdx @@ -0,0 +1,205 @@ +--- +title: Lint Diagnostics +description: How the parser reports data quality issues as attested diagnostics +--- + +The lint framework allows chain parsers to report data quality issues as structured diagnostics that are attested alongside display fields in the signed payload. This replaces silent data dropping with transparent, machine-readable reporting. + +## Architecture + +```mermaid +graph LR + A[Transaction data] --> B[Chain parser] + B --> C[Display fields] + B --> D[Diagnostics] + B --> E[Errors] + C --> F[SignablePayload] + D --> F + F --> G[Signed by ephemeral key] +``` + +Three categories of issues: + +| Category | Where it goes | Who handles it | Example | +|----------|---------------|----------------|---------| +| **Display fields** | `SignablePayload.Fields` | Wallet UI renders them | Network name, instruction details | +| **Diagnostics** | `SignablePayload.Fields` (as `Diagnostic` variant) | Attested -- HSM/auditor can verify | OOB indices, empty account keys | +| **Errors** | `DecodeInstructionsResult.errors` | Consumer decides | No visualizer found | + +## The `diagnostics` Cargo feature + +Diagnostic emission is gated behind a `diagnostics` Cargo feature on `visualsign`, `visualsign-solana`, and `parser_cli`. The default builds: + +- `parser_cli` enables `diagnostics` (default-on); CLI users see the full diagnostic detail. +- `parser_app` and `parser_grpc_server` do **not** enable it. Their `SignablePayload` shape is stable for HSMs and wallets that derive a metadata digest from it. + +When the feature is off, `decode_instructions` returns `Result, VisualSignError>` instead of a struct with separate diagnostic and error vectors. Empty `account_keys` and per-instruction visualizer errors abort the decode with `Err`; out-of-bounds indices flow through to the catch-all `unknown_program` visualizer with no diagnostic emission. + +Paired `*.diagnostics.expected` fixtures only matter when the feature is on. The CI Makefile splits invocations to defeat Cargo feature unification: `cargo {build,test,clippy} --workspace --exclude parser_cli` covers the OFF path; `-p parser_cli` and `-p visualsign-solana --features diagnostics --lib` cover the ON path. + +## Adding a diagnostic to a chain parser + +### 1. Import the builder + +```rust +use visualsign::field_builders::create_diagnostic_field; +use visualsign::lint::LintConfig; +``` + +### 2. Accept `LintConfig` in your decode function + +```rust +pub fn decode_instructions( + transaction: &MyTransaction, + lint_config: &LintConfig, +) -> DecodeResult { +``` + +### 3. Check severity and emit + +```rust +let severity = lint_config.severity_for( + "transaction::my_rule", + visualsign::lint::Severity::Warn, +); + +if !matches!(severity, visualsign::lint::Severity::Allow) { + diagnostics.push(create_diagnostic_field( + "transaction::my_rule", + "transaction", + severity.clone(), + &format!("description of what went wrong"), + Some(instruction_index as u32), + )); +} +``` + +`create_diagnostic_field` automatically emits `tracing::warn!` for warn and error-level diagnostics, giving operators production log visibility without any extra code in chain parsers. + +### 4. Emit ok-level diagnostics for rules that pass + +When `report_all_rules` is enabled, rules that find no issues still report: + +```rust +if issue_count == 0 && lint_config.should_report_ok("transaction::my_rule") { + diagnostics.push(create_diagnostic_field( + "transaction::my_rule", + "transaction", + visualsign::lint::Severity::Ok, + &format!("all {} items checked successfully", total), + None, + )); +} +``` + +This provides boot-metric-style attestation -- the verifier can confirm every expected rule ran. + +### 5. Return results separately + +```rust +DecodeInstructionsResult { + fields, // display fields for the wallet UI + errors, // per-instruction parser errors + diagnostics, // data quality diagnostics for attestation +} +``` + +The caller (`visualsign.rs`) appends diagnostics after all display fields. + +## Rule naming conventions + +Rules follow the `domain::rule_name` format: + +- **`transaction::oob_program_id`** -- instruction's program_id_index is out of bounds in account_keys +- **`transaction::oob_account_index`** -- instruction references out-of-bounds account index in account_keys +- **`transaction::empty_account_keys`** -- transaction has no account keys +- **`decode::visualizer_error`** -- a visualizer failed to decode an instruction (always-on, not configurable via LintConfig) + +Domains reflect who owns the problem: + +| Domain | Scope | +|--------|-------| +| `transaction` | Raw transaction structure validity | +| `decode` | Instruction data interpretation | +| `account` | Account metadata and resolution | +| `wallet` | Caller-provided data quality | +| `idl` | IDL content and structure (Solana) | +| `abi` | ABI content and structure (Ethereum) | + +## `LintConfig` + +Controls diagnostic behavior: + +```rust +use visualsign::lint::{LintConfig, Severity}; + +// Default: all rules at default severity, ok-level diagnostics enabled +let config = LintConfig::default(); + +// Custom: override specific rules +let config = LintConfig { + overrides: HashMap::from([ + ("transaction::oob_account_index".to_string(), Severity::Allow), + ]), + report_all_rules: true, +}; +``` + +**Severity levels:** +- `Ok` -- rule ran and found no issues +- `Warn` -- data quality issue found, parsing continued +- `Error` -- serious issue found +- `Allow` -- rule suppressed, no diagnostic emitted + +## Deterministic serialization + +Diagnostic fields follow the same deterministic serialization rules as all other `SignablePayloadField` variants: + +- Alphabetical key ordering at every nesting level +- ASCII-only content +- Optional fields omitted when `None` (e.g., `InstructionIndex`) + +This ensures diagnostics are covered by the same signing and attestation flow as display fields. + +## Testing diagnostics + +```rust +#[test] +fn test_my_rule_emits_diagnostic() { + let config = LintConfig::default(); + let result = decode_instructions(&tx, ®istry, &config); + + let warns: Vec<_> = result.diagnostics + .iter() + .filter_map(|f| match &f.signable_payload_field { + SignablePayloadField::Diagnostic { diagnostic, .. } + if diagnostic.level == "warn" => Some(diagnostic), + _ => None, + }) + .collect(); + + assert_eq!(warns.len(), 1); + assert_eq!(warns[0].rule, "transaction::my_rule"); +} +``` + +### Updating fixtures and snapshots when adding rules + +Adding a new rule that emits ok-level diagnostics changes the output of every transaction parse. You must update: + +1. **CLI fixtures** -- regenerate the `*.display.expected` fixtures and the matching `*.diagnostics.expected` fixtures by running the CLI against the fixture inputs: + + ```bash + cargo run --bin parser_cli -- $(cat src/parser/cli/tests/fixtures/solana-json.input | tr '\n' ' ') > src/parser/cli/tests/fixtures/solana-json.display.expected + cargo run --bin parser_cli -- $(cat src/parser/cli/tests/fixtures/solana-text.input | tr '\n' ' ') > src/parser/cli/tests/fixtures/solana-text.display.expected + ``` + + For JSON fixtures, filter diagnostics from the display expected file and update the diagnostics expected file separately. + +2. **Integration test expected JSON** -- update `src/integration/tests/parser.rs` to include the new diagnostic fields in the `expected_sp` JSON + +3. **Field count assertions** -- tests that assert `payload.fields.len()` (e.g., swig_wallet tests) need their counts updated to include the new ok-level diagnostics + +4. **Fuzz and proptest** -- run `cargo test -p visualsign-solana --test fuzz_idl_parsing` and `--test pipeline_integration` to verify no regressions + +Run `make -C src fmt && make -C src lint && make -C src test` to verify everything passes before pushing. diff --git a/docs/docs.json b/docs/docs.json index 4d3c4f95..a82798f1 100644 --- a/docs/docs.json +++ b/docs/docs.json @@ -114,7 +114,8 @@ "adding-new-chain", "contributing", "contributor-guides/project-structure", - "contributor-guides/best-practices" + "contributor-guides/best-practices", + "contributor-guides/lint-diagnostics" ] } ] diff --git a/docs/field-types.mdx b/docs/field-types.mdx index 162feab3..ce820fe4 100644 --- a/docs/field-types.mdx +++ b/docs/field-types.mdx @@ -197,6 +197,7 @@ Provides expandable/collapsible content with progressive disclosure. Shows essen | Multiple values | `list_layout` | List of recipients | | Complex data | `preview_layout` | Detailed gas breakdown | | Warnings | `text_v2` | "Warning: High slippage" | +| Parse diagnostics | `diagnostic` | OOB indices, data quality checks | ## Combining field types @@ -450,6 +451,51 @@ Token names and other content must use ASCII equivalents (e.g., "EUR" instead of } ``` +## Diagnostic field type + +### diagnostic + +Reports data quality findings from the parser's lint framework. Diagnostics are attested alongside display fields in the signed payload, so the HSM/attester can verify what the parser checked and what it found. + +```json +{ + "Type": "diagnostic", + "Label": "transaction::oob_program_id", + "FallbackText": "warn: instruction 1: program_id_index 8 out of bounds (5 account keys)", + "Diagnostic": { + "Rule": "transaction::oob_program_id", + "Domain": "transaction", + "Level": "warn", + "Message": "instruction 1: program_id_index 8 out of bounds (5 account keys)", + "InstructionIndex": 1 + } +} +``` + +**Properties:** +- `Rule`: Rule identifier in `domain::rule_name` format +- `Domain`: Category of the check (e.g., `transaction`, `decode`, `idl`) +- `Level`: Severity of the finding + - `ok` -- rule ran and found no issues (boot-metric attestation) + - `warn` -- data quality issue found, parsing continued + - `error` -- serious issue found +- `Message`: Human-readable description of the finding +- `InstructionIndex` (optional): Which instruction triggered the diagnostic + +**Wallet handling:** +- Wallets that don't recognize `Type: "diagnostic"` can display the `FallbackText` +- Diagnostics always appear after all display fields (network, instructions, accounts) +- When `report_all_rules` is enabled (default), every rule emits a diagnostic -- `ok`, `warn`, or `error` -- so the attester can verify all expected rules ran + +**Current rules:** + +| Rule | Domain | Description | +|------|--------|-------------| +| `transaction::oob_program_id` | `transaction` | Instruction's program_id_index is out of bounds in account_keys | +| `transaction::oob_account_index` | `transaction` | Instruction references account indices beyond account_keys | +| `transaction::empty_account_keys` | `transaction` | Transaction has no account keys | +| `decode::visualizer_error` | `decode` | A visualizer failed to decode an instruction (always-on) | + ## Future field types Planned additions to the field type system: diff --git a/docs/specs/2026-03-30-lint-diagnostics-design.md b/docs/specs/2026-03-30-lint-diagnostics-design.md new file mode 100644 index 00000000..eb865f03 --- /dev/null +++ b/docs/specs/2026-03-30-lint-diagnostics-design.md @@ -0,0 +1,147 @@ +# Lint Diagnostics in SignablePayload — Design Spec + +Issue: #228 + +## Goal + +Add structured lint diagnostics to `SignablePayload` as a new `Diagnostic` variant of `SignablePayloadField`. Diagnostics are attested alongside display fields in the signed payload. This first slice implements rules for Solana legacy and v0 transactions and introduces a `LintConfig` framework for configuring rule severity, replacing the current silent data dropping. + +## Error categorization + +| Category | Handled by | Example | +|----------|------------|---------| +| **Parser errors** (`VisualSignError`) | Collected per-instruction in `errors` vec, surfaced as `decode::visualizer_error` diagnostics | No visualizer found | +| **Data quality diagnostics** (attested) | Emitted as `Diagnostic` fields in `SignablePayload` | OOB indices, empty account keys | +| **Configuration** (`LintConfig`) | Caller controls severity per-rule | Override `oob_program_id` to `Allow` | + +## Core Types (`visualsign` crate) + +### `SignablePayloadFieldDiagnostic` + +```rust +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +pub struct SignablePayloadFieldDiagnostic { + #[serde(rename = "Rule")] + pub rule: String, + #[serde(rename = "Domain")] + pub domain: String, + #[serde(rename = "Level")] + pub level: String, + #[serde(rename = "Message")] + pub message: String, + #[serde(rename = "InstructionIndex", skip_serializing_if = "Option::is_none")] + pub instruction_index: Option, +} +``` + +Custom `Serialize` impl using `BTreeMap` for alphabetical field ordering (same pattern as `SignablePayloadFieldAmountV2`). Implements `DeterministicOrdering`. + +### New variant in `SignablePayloadField` + +```rust +#[serde(rename = "diagnostic")] +Diagnostic { + #[serde(flatten)] + common: SignablePayloadFieldCommon, + #[serde(rename = "Diagnostic")] + diagnostic: SignablePayloadFieldDiagnostic, +}, +``` + +### Field builder + +```rust +pub fn create_diagnostic_field( + rule: &str, + domain: &str, + level: Severity, + message: &str, + instruction_index: Option, +) -> AnnotatedPayloadField +``` + +Sets `label` to the rule ID, `fallback_text` to `"{level}: {message}"`. Automatically emits `tracing::warn!` for warn/error-level diagnostics, providing operator log visibility without per-chain boilerplate. + +### `LintConfig` + +```rust +pub struct LintConfig { + pub overrides: HashMap, + pub report_all_rules: bool, +} +``` + +Severity levels: `Ok`, `Warn`, `Error`, `Allow`. + +Currently constructed as `LintConfig::default()` in the conversion functions. Future work will wire overrides from `VisualSignOptions` / request metadata. + +## Solana Integration (`visualsign-solana` crate) + +### `decode_instructions()` and `decode_v0_instructions()` + +Functions always succeed. Return `DecodeInstructionsResult` with separate `fields`, `errors`, and `diagnostics` vecs. + +Three rules: + +| Rule | Domain | Default Level | When | +|------|--------|---------------|------| +| `transaction::oob_program_id` | `transaction` | `warn` | `ci.program_id_index >= account_keys.len()` | +| `transaction::oob_account_index` | `transaction` | `warn` | any account index `>= account_keys.len()` | +| `transaction::empty_account_keys` | `transaction` | `error` | `account_keys.is_empty()` | + +No instructions are skipped. All instructions are processed by the visualizer pipeline regardless of whether their program_id or account indices are resolvable. The `VisualizerContext` provides `ProgramRef` and `AccountRef` types that distinguish resolved from unresolved indices, allowing visualizers to handle each case explicitly. + +Additionally, `decode::visualizer_error` diagnostics are emitted for any instruction whose visualizer fails. This rule is intentionally always-on and not configurable via `LintConfig`. + +### Boot-metric attestation + +When `report_all_rules` is true (default), every rule emits a diagnostic — either `ok` (no issues) or `warn`/`error` (issues found). The attester can verify all expected rules ran. + +### What does NOT change + +- No changes to `ParseRequest`, `ChainMetadata`, or proto definitions +- No changes to the signing/attestation flow — only the contents of `SignablePayload` have been extended to include diagnostics +- `LintConfig` uses defaults only in this slice — wiring from request metadata is future work + +## Serialized Output Example + +```json +{ + "Fields": [ + { + "FallbackText": "Solana", + "Label": "Network", + "TextV2": { "Text": "Solana" }, + "Type": "text_v2" + }, + { + "FallbackText": "warn: instruction 1: program_id_index 8 out of bounds (5 account keys)", + "Label": "transaction::oob_program_id", + "Diagnostic": { + "Domain": "transaction", + "InstructionIndex": 1, + "Level": "warn", + "Message": "instruction 1: program_id_index 8 out of bounds (5 account keys)", + "Rule": "transaction::oob_program_id" + }, + "Type": "diagnostic" + } + ], + "Title": "Solana Transaction", + "Version": "0" +} +``` + +## Tests + +1. **Serialization roundtrip** — `SignablePayloadFieldDiagnostic` serializes to JSON with alphabetical keys, deserializes back, passes `verify_deterministic_ordering()` +2. **Integration** — construct a `SolanaTransaction` with an OOB program_id_index, parse it, verify the output contains a `Diagnostic` field with rule `transaction::oob_program_id` +3. **Mixed output** — transaction with some valid and some OOB instructions produces both display fields and diagnostic fields +4. **Boot metrics** — valid transaction emits ok-level diagnostics for all rules +5. **Builder** — `create_diagnostic_field()` produces expected output + +## Backwards Compatibility + +- Wallets that don't know `Type: "diagnostic"` will hit the `Unknown` deserialization path and can display `FallbackText` +- Payloads without diagnostics are unchanged when `report_all_rules` is false +- Enum variant is appended (index 11), not inserted — existing variant indices are stable diff --git a/src/Cargo.lock b/src/Cargo.lock index 6ab6a33d..e57b6e48 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -14049,6 +14049,7 @@ dependencies = [ "serde", "serde_json", "thiserror 2.0.17", + "tracing", ] [[package]] diff --git a/src/Makefile b/src/Makefile index 940b0b77..685c8387 100644 --- a/src/Makefile +++ b/src/Makefile @@ -13,15 +13,24 @@ all: generated .PHONY: build build: - cargo build --all + @# Split to avoid Cargo feature unification across the workspace. + @# parser_cli enables visualsign-solana/diagnostics; building it together + @# with parser_app/grpc-server would unify the feature ON for both. + cargo build --workspace --exclude parser_cli + cargo build -p parser_cli .PHONY: test test: build @# The integration tests rely on binaries from other crates being built, so @# we build all the workspace targets. make build - @# Run all tests - cargo test --all-targets + @# diagnostics OFF: production payload shape (parser_app, integration, others). + cargo test --workspace --exclude parser_cli --all-targets + cargo test -p visualsign-solana --no-default-features --lib + @# diagnostics ON: parser_cli + lib tests gated behind the feature. + cargo test -p parser_cli --all-targets + cargo test -p visualsign --features diagnostics --lib + cargo test -p visualsign-solana --features diagnostics --lib .PHONY: fmt fmt: @@ -30,7 +39,10 @@ fmt: .PHONY: lint lint: cargo clippy --version - cargo clippy --all-targets -- -D warnings + cargo clippy --workspace --exclude parser_cli --all-targets -- -D warnings + cargo clippy -p parser_cli --all-targets -- -D warnings + cargo clippy -p visualsign --features diagnostics --lib -- -D warnings + cargo clippy -p visualsign-solana --features diagnostics --lib -- -D warnings .PHONY: generated generated: diff --git a/src/chain_parsers/visualsign-solana/Cargo.toml b/src/chain_parsers/visualsign-solana/Cargo.toml index 3a0ef008..8921cf7d 100644 --- a/src/chain_parsers/visualsign-solana/Cargo.toml +++ b/src/chain_parsers/visualsign-solana/Cargo.toml @@ -35,5 +35,8 @@ bs58 = "0.5" proptest = "1" tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } +[features] +diagnostics = ["visualsign/diagnostics"] + [lints] workspace = true diff --git a/src/chain_parsers/visualsign-solana/src/core/instructions.rs b/src/chain_parsers/visualsign-solana/src/core/instructions.rs index 10edd98b..640e6c10 100644 --- a/src/chain_parsers/visualsign-solana/src/core/instructions.rs +++ b/src/chain_parsers/visualsign-solana/src/core/instructions.rs @@ -2,21 +2,38 @@ use crate::core::{InstructionVisualizer, VisualizerContext, visualize_with_any}; use crate::idl::IdlRegistry; use solana_parser::solana::parser::parse_transaction; use solana_parser::solana::structs::SolanaAccount; -use solana_sdk::instruction::Instruction; use solana_sdk::transaction::Transaction as SolanaTransaction; use visualsign::AnnotatedPayloadField; use visualsign::errors::{TransactionParseError, VisualSignError}; +#[cfg(feature = "diagnostics")] +use visualsign::field_builders::create_diagnostic_field; +#[cfg(feature = "diagnostics")] +use visualsign::lint::LintConfig; // The following include! macro pulls in visualizer implementations generated at build time. // The file "generated_visualizers.rs" is created by the build script and contains code for // available_visualizers and related items, which are used to decode and visualize instructions. include!(concat!(env!("OUT_DIR"), "/generated_visualizers.rs")); -/// Visualizes all the instructions and related fields in a transaction/message +/// Result of decoding instructions: display fields, per-instruction errors, +/// and lint diagnostics separately. The function always succeeds — individual +/// instruction failures are captured in `errors` rather than aborting the parse. +#[cfg(feature = "diagnostics")] +pub struct DecodeInstructionsResult { + pub fields: Vec, + pub errors: Vec<(usize, VisualSignError)>, + pub diagnostics: Vec, +} + +/// Visualizes all the instructions and related fields in a transaction/message. +/// Always succeeds — data quality issues become diagnostics, per-instruction +/// failures are collected in errors. +#[cfg(feature = "diagnostics")] pub fn decode_instructions( transaction: &SolanaTransaction, idl_registry: &IdlRegistry, -) -> Result, VisualSignError> { + lint_config: &LintConfig, +) -> DecodeInstructionsResult { // available_visualizers is generated at build time by build.rs let visualizers: Vec> = available_visualizers(); let visualizers_refs: Vec<&dyn InstructionVisualizer> = @@ -26,85 +43,213 @@ pub fn decode_instructions( let account_keys = &message.account_keys; if account_keys.is_empty() { - return Err(VisualSignError::ParseError( - TransactionParseError::DecodeError( - "Legacy transaction has no account keys".to_string(), - ), + let severity = lint_config.severity_for( + "transaction::empty_account_keys", + visualsign::lint::Severity::Error, + ); + let diagnostics = if matches!(severity, visualsign::lint::Severity::Allow) { + Vec::new() + } else { + vec![create_diagnostic_field( + "transaction::empty_account_keys", + "transaction", + severity, + "legacy transaction has no account keys", + None, + )] + }; + return DecodeInstructionsResult { + fields: Vec::new(), + errors: Vec::new(), + diagnostics, + }; + } + + // Diagnostic scan: check all indices, emit diagnostics for inaccessible ones. + // This is purely informational — no instructions are skipped. + let diagnostics = + scan_instruction_diagnostics(&message.instructions, account_keys, lint_config); + + // Visualization: process every instruction (no skipping) + let mut fields: Vec = Vec::new(); + let mut errors: Vec<(usize, VisualSignError)> = Vec::new(); + + for (i, ci) in message.instructions.iter().enumerate() { + let sender = SolanaAccount { + account_key: account_keys[0].to_string(), + signer: false, + writable: false, + }; + + let context = VisualizerContext::new(&sender, ci, account_keys, idl_registry, i); + + match visualize_with_any(&visualizers_refs, &context) { + Some(Ok(viz_result)) => fields.push(viz_result.field), + Some(Err(e)) => errors.push((i, e)), + None => errors.push(( + i, + VisualSignError::DecodeError(format!( + "No visualizer available for instruction at index {i}" + )), + )), + } + } + + DecodeInstructionsResult { + fields, + errors, + diagnostics, + } +} + +/// Diagnostics-off variant: returns the visualized fields directly, propagating +/// the first hard failure as `Err`. Empty account keys and visualizer errors +/// abort the decode (matching pre-diagnostics behavior). Out-of-bounds program +/// IDs and account indices flow through to `unknown_program`/visualizers as +/// unresolved references with no separate diagnostic emission. +#[cfg(not(feature = "diagnostics"))] +pub fn decode_instructions( + transaction: &SolanaTransaction, + idl_registry: &IdlRegistry, +) -> Result, VisualSignError> { + let visualizers: Vec> = available_visualizers(); + let visualizers_refs: Vec<&dyn InstructionVisualizer> = + visualizers.iter().map(|v| v.as_ref()).collect::>(); + + let message = &transaction.message; + let account_keys = &message.account_keys; + + if account_keys.is_empty() { + return Err(VisualSignError::DecodeError( + "legacy transaction has no account keys".to_string(), )); } - // 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 = message - .instructions - .iter() - .filter_map(|ci| { - if (ci.program_id_index as usize) >= account_keys.len() { - return None; + let mut fields: Vec = Vec::new(); + for (i, ci) in message.instructions.iter().enumerate() { + let sender = SolanaAccount { + account_key: account_keys[0].to_string(), + signer: false, + writable: false, + }; + + let context = VisualizerContext::new(&sender, ci, account_keys, idl_registry, i); + + match visualize_with_any(&visualizers_refs, &context) { + Some(Ok(viz_result)) => fields.push(viz_result.field), + Some(Err(e)) => { + return Err(VisualSignError::DecodeError(format!( + "instruction {i}: {e}" + ))); } - let accounts: Vec = ci - .accounts - .iter() - .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 - } - }) - .collect(); - Some(Instruction { - program_id: account_keys[ci.program_id_index as usize], - accounts, - data: ci.data.clone(), - }) - }) - .collect(); - - let results: Result, VisualSignError> = instructions - .iter() - .enumerate() - .map(|(instruction_index, instruction)| { - // Create sender account from first account key (typically the fee payer) - let sender = SolanaAccount { - account_key: account_keys[0].to_string(), - signer: false, - writable: false, - }; - - let context = - VisualizerContext::new(&sender, instruction_index, &instructions, idl_registry); - - // Try to visualize with available visualizers (including unknown_program fallback) - visualize_with_any(&visualizers_refs, &context) - .ok_or_else(|| { - VisualSignError::DecodeError(format!( - "No visualizer available for instruction {} at index {}", - instruction.program_id, instruction_index - )) - })? - .map(|viz_result| viz_result.field) - }) - .collect(); - - let fields = results?; - - // Self-check: ensure we have the same number of instruction fields as input instructions - if fields.len() != instructions.len() { - return Err(VisualSignError::InvariantViolation(format!( - "Instruction count mismatch: expected {} instructions, got {} fields. This should never happen with unknown_program fallback.", - instructions.len(), - fields.len() - ))); + None => { + return Err(VisualSignError::DecodeError(format!( + "No visualizer available for instruction at index {i}" + ))); + } + } } Ok(fields) } +/// Scan compiled instructions for inaccessible indices and emit diagnostics. +/// Does not modify or filter instructions — purely informational. +#[cfg(feature = "diagnostics")] +pub fn scan_instruction_diagnostics( + instructions: &[solana_sdk::instruction::CompiledInstruction], + account_keys: &[solana_sdk::pubkey::Pubkey], + lint_config: &LintConfig, +) -> Vec { + let mut diagnostics: Vec = Vec::new(); + let mut oob_program_id_count: usize = 0; + let mut oob_account_index_count: usize = 0; + + let oob_pid_severity = lint_config.severity_for( + "transaction::oob_program_id", + visualsign::lint::Severity::Warn, + ); + let oob_acct_severity = lint_config.severity_for( + "transaction::oob_account_index", + visualsign::lint::Severity::Warn, + ); + + for (ci_index, ci) in instructions.iter().enumerate() { + if (ci.program_id_index as usize) >= account_keys.len() { + oob_program_id_count += 1; + if !matches!(oob_pid_severity, visualsign::lint::Severity::Allow) { + diagnostics.push(create_diagnostic_field( + "transaction::oob_program_id", + "transaction", + oob_pid_severity, + &format!( + "instruction {}: program_id_index {} out of bounds ({} account keys)", + ci_index, + ci.program_id_index, + account_keys.len() + ), + Some(ci_index as u32), + )); + } + } + + // Check all account indices (unified — no separate "skipped" rule) + let oob_accounts: Vec = ci + .accounts + .iter() + .filter(|&&idx| (idx as usize) >= account_keys.len()) + .copied() + .collect(); + if !oob_accounts.is_empty() { + oob_account_index_count += 1; + if !matches!(oob_acct_severity, visualsign::lint::Severity::Allow) { + diagnostics.push(create_diagnostic_field( + "transaction::oob_account_index", + "transaction", + oob_acct_severity, + &format!( + "instruction {}: account indices {:?} out of bounds ({} account keys)", + ci_index, + oob_accounts, + account_keys.len() + ), + Some(ci_index as u32), + )); + } + } + } + + // Boot-metric ok diagnostics + if oob_program_id_count == 0 && lint_config.should_report_ok("transaction::oob_program_id") { + diagnostics.push(create_diagnostic_field( + "transaction::oob_program_id", + "transaction", + visualsign::lint::Severity::Ok, + &format!( + "all {} instructions have valid program_id_index", + instructions.len() + ), + None, + )); + } + if oob_account_index_count == 0 + && lint_config.should_report_ok("transaction::oob_account_index") + { + diagnostics.push(create_diagnostic_field( + "transaction::oob_account_index", + "transaction", + visualsign::lint::Severity::Ok, + &format!( + "all {} instructions have valid account indices", + instructions.len() + ), + None, + )); + } + + diagnostics +} + pub fn decode_transfers( transaction: &SolanaTransaction, ) -> Result, VisualSignError> { @@ -197,49 +342,95 @@ pub fn decode_transfers( Ok(fields) } -#[cfg(test)] +#[cfg(all(test, not(feature = "diagnostics")))] #[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] -mod tests { +mod off_tests { use super::*; use solana_sdk::hash::Hash; use solana_sdk::message::{Message, MessageHeader}; use solana_sdk::pubkey::Pubkey; - /// Verifies that empty account keys returns a DecodeError (not a panic). - /// This error path was introduced in PR #245 replacing the previous unwrap. - #[test] - fn test_empty_account_keys_returns_parse_error() { - let message = Message { - header: MessageHeader { - num_required_signatures: 0, - num_readonly_signed_accounts: 0, - num_readonly_unsigned_accounts: 0, - }, - account_keys: vec![], - recent_blockhash: Hash::default(), - instructions: vec![], - }; - let tx = SolanaTransaction { + fn tx_with( + account_keys: Vec, + instructions: Vec, + ) -> SolanaTransaction { + SolanaTransaction { signatures: vec![], - message, - }; + message: Message { + header: MessageHeader { + num_required_signatures: 1, + num_readonly_signed_accounts: 0, + num_readonly_unsigned_accounts: 0, + }, + account_keys, + recent_blockhash: Hash::default(), + instructions, + }, + } + } + + #[test] + fn test_empty_account_keys_returns_err() { + let tx = tx_with(vec![], vec![]); let registry = IdlRegistry::new(); let result = decode_instructions(&tx, ®istry); + let Err(VisualSignError::DecodeError(msg)) = result else { + panic!("expected DecodeError, got {result:?}"); + }; + assert!(msg.contains("no account keys"), "msg was: {msg}"); + } - assert!( - matches!( - &result, - Err(VisualSignError::ParseError(TransactionParseError::DecodeError(msg))) - if msg.contains("no account keys") - ), - "expected DecodeError for empty account keys, got: {result:?}" + #[test] + fn test_oob_program_id_flows_through_as_ok() { + // program_id_index=99 is out of bounds. With diagnostics OFF, this is + // not a hard error -- unknown_program handles unresolved program_ids + // and renders an `unresolved(99)` placeholder. + let key0 = Pubkey::new_unique(); + let tx = tx_with( + vec![key0], + vec![solana_sdk::instruction::CompiledInstruction { + program_id_index: 99, + accounts: vec![], + data: vec![0xAA], + }], ); + let registry = IdlRegistry::new(); + let fields = decode_instructions(&tx, ®istry).expect("OOB program_id should not abort"); + assert_eq!(fields.len(), 1, "exactly one rendered instruction"); } - /// Verifies that OOB program_id_index silently skips the instruction - /// instead of panicking. Only the valid instruction produces a field. #[test] - fn test_oob_program_id_index_skips_instruction() { + fn test_oob_account_index_flows_through_as_ok() { + // accounts contains index 50 which is out of bounds. With diagnostics + // OFF, no diagnostic is emitted and rendering still succeeds via + // unknown_program (or whichever visualizer handles the program_id). + let key0 = Pubkey::new_unique(); + let key1 = Pubkey::new_unique(); + let tx = tx_with( + vec![key0, key1], + vec![solana_sdk::instruction::CompiledInstruction { + program_id_index: 1, + accounts: vec![0, 50], + data: vec![0xCC], + }], + ); + let registry = IdlRegistry::new(); + let fields = + decode_instructions(&tx, ®istry).expect("OOB account_index should not abort"); + assert_eq!(fields.len(), 1); + } +} + +#[cfg(all(test, feature = "diagnostics"))] +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +mod tests { + use super::*; + use solana_sdk::hash::Hash; + use solana_sdk::message::{Message, MessageHeader}; + use solana_sdk::pubkey::Pubkey; + use visualsign::SignablePayloadField; + + fn tx_with_oob_program_id() -> SolanaTransaction { let key0 = Pubkey::new_unique(); let key1 = Pubkey::new_unique(); let message = Message { @@ -263,53 +454,263 @@ mod tests { }, ], }; + SolanaTransaction { + signatures: vec![], + message, + } + } + + fn tx_with_oob_account_index() -> SolanaTransaction { + let key0 = Pubkey::new_unique(); + let key1 = Pubkey::new_unique(); + let message = Message { + header: MessageHeader { + num_required_signatures: 1, + num_readonly_signed_accounts: 0, + num_readonly_unsigned_accounts: 0, + }, + account_keys: vec![key0, key1], + recent_blockhash: Hash::default(), + instructions: vec![solana_sdk::instruction::CompiledInstruction { + program_id_index: 1, + accounts: vec![0, 50], + data: vec![0xCC], + }], + }; + SolanaTransaction { + signatures: vec![], + message, + } + } + + #[test] + fn test_oob_program_id_emits_diagnostic() { + let tx = tx_with_oob_program_id(); + let registry = IdlRegistry::new(); + let config = LintConfig::default(); + let result = decode_instructions(&tx, ®istry, &config); + let fields = [result.fields, result.diagnostics].concat(); + + let warns: Vec<_> = fields + .iter() + .filter_map(|f| match &f.signable_payload_field { + SignablePayloadField::Diagnostic { diagnostic, .. } + if diagnostic.level == "warn" => + { + Some(diagnostic) + } + _ => None, + }) + .collect(); + assert_eq!(warns.len(), 1); + assert_eq!(warns[0].rule, "transaction::oob_program_id"); + assert_eq!(warns[0].instruction_index, Some(1)); + + // oob_account_index should pass since the instruction's accounts are valid + let passes: Vec<_> = fields + .iter() + .filter_map(|f| match &f.signable_payload_field { + SignablePayloadField::Diagnostic { diagnostic, .. } if diagnostic.level == "ok" => { + Some(diagnostic) + } + _ => None, + }) + .collect(); + assert!( + passes + .iter() + .any(|d| d.rule == "transaction::oob_account_index") + ); + } + + #[test] + fn test_oob_account_index_emits_diagnostic() { + let tx = tx_with_oob_account_index(); + let registry = IdlRegistry::new(); + let config = LintConfig::default(); + let result = decode_instructions(&tx, ®istry, &config); + let fields = [result.fields, result.diagnostics].concat(); + + let warns: Vec<_> = fields + .iter() + .filter_map(|f| match &f.signable_payload_field { + SignablePayloadField::Diagnostic { diagnostic, .. } + if diagnostic.level == "warn" => + { + Some(diagnostic) + } + _ => None, + }) + .collect(); + assert_eq!(warns.len(), 1); + assert_eq!(warns[0].rule, "transaction::oob_account_index"); + assert_eq!(warns[0].instruction_index, Some(0)); + assert!(warns[0].message.contains("50")); + + // oob_program_id should pass since the instruction has a valid program_id_index + let passes: Vec<_> = fields + .iter() + .filter_map(|f| match &f.signable_payload_field { + SignablePayloadField::Diagnostic { diagnostic, .. } if diagnostic.level == "ok" => { + Some(diagnostic) + } + _ => None, + }) + .collect(); + assert!( + passes + .iter() + .any(|d| d.rule == "transaction::oob_program_id") + ); + + let non_diagnostics: Vec<_> = fields + .iter() + .filter(|f| f.signable_payload_field.field_type() != "diagnostic") + .collect(); + assert_eq!(non_diagnostics.len(), 1); + } + + #[test] + fn test_valid_transaction_emits_pass_diagnostics() { + let key0 = Pubkey::new_unique(); + let key1 = Pubkey::new_unique(); + let message = Message { + header: MessageHeader { + num_required_signatures: 1, + num_readonly_signed_accounts: 0, + num_readonly_unsigned_accounts: 0, + }, + account_keys: vec![key0, key1], + recent_blockhash: Hash::default(), + instructions: vec![solana_sdk::instruction::CompiledInstruction { + program_id_index: 1, + accounts: vec![0], + data: vec![0xDD], + }], + }; let tx = SolanaTransaction { signatures: vec![], message, }; let registry = IdlRegistry::new(); - let result = decode_instructions(&tx, ®istry); + let config = LintConfig::default(); + let result = decode_instructions(&tx, ®istry, &config); + let fields = [result.fields, result.diagnostics].concat(); + + let passes: Vec<_> = fields + .iter() + .filter_map(|f| match &f.signable_payload_field { + SignablePayloadField::Diagnostic { diagnostic, .. } if diagnostic.level == "ok" => { + Some(diagnostic) + } + _ => None, + }) + .collect(); + // Both rules should report ok + assert_eq!(passes.len(), 2); + assert!( + passes + .iter() + .any(|d| d.rule == "transaction::oob_program_id") + ); + assert!( + passes + .iter() + .any(|d| d.rule == "transaction::oob_account_index") + ); - let fields = result.expect("should not error when OOB instructions are skipped"); - assert_eq!(fields.len(), 1, "expected 1 field for 1 valid instruction"); + let warns: Vec<_> = fields + .iter() + .filter_map(|f| match &f.signable_payload_field { + SignablePayloadField::Diagnostic { diagnostic, .. } + if diagnostic.level == "warn" => + { + Some(diagnostic) + } + _ => None, + }) + .collect(); + assert!( + warns.is_empty(), + "valid transaction should have no warnings" + ); } - /// Verifies that all-OOB instructions produce empty fields (not a panic). #[test] - fn test_all_instructions_oob_returns_empty_fields() { + fn test_oob_program_id_and_oob_account_index_emits_both_diagnostics() { + // Instruction has both an OOB program_id_index and OOB account indices. + // Both are reported as separate diagnostics (unified rules, no skipping). let key0 = Pubkey::new_unique(); + let key1 = Pubkey::new_unique(); let message = Message { header: MessageHeader { num_required_signatures: 1, num_readonly_signed_accounts: 0, num_readonly_unsigned_accounts: 0, }, - account_keys: vec![key0], + account_keys: vec![key0, key1], recent_blockhash: Hash::default(), - instructions: vec![ - solana_sdk::instruction::CompiledInstruction { - program_id_index: 99, - accounts: vec![], - data: vec![], - }, - solana_sdk::instruction::CompiledInstruction { - program_id_index: 88, - accounts: vec![], - data: vec![], - }, - ], + instructions: vec![solana_sdk::instruction::CompiledInstruction { + program_id_index: 99, // OOB: only 2 keys + accounts: vec![0, 77], // index 77 is also OOB + data: vec![0xEE], + }], }; let tx = SolanaTransaction { signatures: vec![], message, }; let registry = IdlRegistry::new(); - let result = decode_instructions(&tx, ®istry); + let config = LintConfig::default(); + let result = decode_instructions(&tx, ®istry, &config); + let fields = [result.fields, result.diagnostics].concat(); - let fields = result.expect("should succeed with all OOB instructions"); + let warns: Vec<_> = fields + .iter() + .filter_map(|f| match &f.signable_payload_field { + SignablePayloadField::Diagnostic { diagnostic, .. } + if diagnostic.level == "warn" => + { + Some(diagnostic) + } + _ => None, + }) + .collect(); + assert_eq!( + warns.len(), + 2, + "expected oob_program_id and oob_account_index warns" + ); assert!( - fields.is_empty(), - "expected no fields when all instructions are OOB" + warns + .iter() + .any(|d| d.rule == "transaction::oob_program_id") ); + assert!( + warns + .iter() + .any(|d| d.rule == "transaction::oob_account_index") + ); + let acct_warn = warns + .iter() + .find(|d| d.rule == "transaction::oob_account_index") + .unwrap(); + assert_eq!(acct_warn.instruction_index, Some(0)); + assert!( + acct_warn.message.contains("77"), + "message should mention the OOB index 77" + ); + + // No ok-diagnostics expected -- both rules fired with warnings + let passes: Vec<_> = fields + .iter() + .filter_map(|f| match &f.signable_payload_field { + SignablePayloadField::Diagnostic { diagnostic, .. } if diagnostic.level == "ok" => { + Some(diagnostic) + } + _ => None, + }) + .collect(); + assert!(passes.is_empty(), "no ok-diagnostics when both rules fire"); } } diff --git a/src/chain_parsers/visualsign-solana/src/core/mod.rs b/src/chain_parsers/visualsign-solana/src/core/mod.rs index b7614c6b..4dbd25f8 100644 --- a/src/chain_parsers/visualsign-solana/src/core/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/core/mod.rs @@ -3,7 +3,7 @@ use std::collections::BTreeMap; use ::visualsign::AnnotatedPayloadField; use ::visualsign::errors::VisualSignError; use solana_parser::solana::structs::SolanaAccount; -use solana_sdk::instruction::Instruction; +use solana_sdk::pubkey::Pubkey; mod accounts; mod instructions; @@ -15,7 +15,7 @@ pub use instructions::*; pub use txtypes::*; pub use visualsign::*; -/// Identifier for which visualizer handled a command, categorized by dApp type. - Copied from Sui chain_parser +/// Identifier for which visualizer handled a command, categorized by dApp type. #[derive(Debug, Clone, PartialEq, Eq)] pub enum VisualizerKind { /// Decentralized exchange protocols (e.g., AMMs, DEX aggregators) @@ -28,68 +28,161 @@ pub enum VisualizerKind { Payments(&'static str), } +/// Resolution of a compiled instruction's program_id_index. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ProgramRef<'a> { + Resolved(&'a Pubkey), + Unresolved { raw_index: u8 }, +} + +/// Resolution of a compiled instruction's account index. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AccountRef<'a> { + Resolved(&'a Pubkey), + Unresolved { raw_index: u8 }, +} + /// Context for visualizing a Solana instruction. /// -/// Holds all necessary information to visualize a specific command -/// within a transaction. +/// Holds references to the transaction's wire data -- no copies. +/// Resolution of compiled instruction indices to pubkeys happens +/// lazily via helper methods. +/// +/// # Resolution patterns +/// +/// Two ways for a visualizer to handle indices that don't resolve against +/// `account_keys` (out-of-bounds, or v0 lookup-table entries that haven't +/// been resolved): +/// +/// **All-or-nothing (most IDL-based presets).** +/// Use `resolve_program_id()` / `resolve_accounts()`. The first unresolved +/// index aborts visualization with a precise `Err` naming the bad index. +/// Suitable when downstream parsing requires every account to be a real +/// pubkey -- e.g. an Anchor IDL parser building a `named_accounts` map. +/// +/// **Partial rendering (catch-all visualizers).** +/// Pattern-match on `program_id()` and `account(n)` directly and substitute +/// a placeholder for unresolved indices instead of erroring. The +/// `unknown_program` preset is the canonical example: it renders +/// `unresolved(N)` strings so the user still sees *something* for an +/// instruction no specific visualizer could handle. #[derive(Debug, Clone)] pub struct VisualizerContext<'a> { - /// The address sending the transaction. sender: &'a SolanaAccount, - /// Index of the instruction to visualize. - instruction_index: usize, - /// All instruction in the transaction. - /// Instruction struct contains data - instructions: &'a Vec, - /// IDL registry for parsing unknown programs with Anchor IDLs + compiled_instruction: &'a solana_sdk::instruction::CompiledInstruction, + account_keys: &'a [Pubkey], idl_registry: &'a crate::idl::IdlRegistry, + instruction_index: usize, } impl<'a> VisualizerContext<'a> { - /// Creates a new `VisualizerContext`. pub fn new( sender: &'a SolanaAccount, - instruction_index: usize, - instructions: &'a Vec, + compiled_instruction: &'a solana_sdk::instruction::CompiledInstruction, + account_keys: &'a [Pubkey], idl_registry: &'a crate::idl::IdlRegistry, + instruction_index: usize, ) -> Self { Self { sender, - instruction_index, - instructions, + compiled_instruction, + account_keys, idl_registry, + instruction_index, } } - /// Returns a reference to the IDL registry pub fn idl_registry(&self) -> &crate::idl::IdlRegistry { self.idl_registry } - /// Returns the sender address. pub fn sender(&self) -> &SolanaAccount { self.sender } - /// Returns the instruction index. + /// Position of this instruction in the transaction's instruction list. pub fn instruction_index(&self) -> usize { self.instruction_index } - /// Returns a reference to all instructions. - pub fn instructions(&self) -> &Vec { - self.instructions + /// Resolves the program_id_index. Every compiled instruction has one, + /// so this always returns a value -- either resolved or unresolved. + pub fn program_id(&self) -> ProgramRef<'a> { + let idx = self.compiled_instruction.program_id_index; + match self.account_keys.get(idx as usize) { + Some(pk) => ProgramRef::Resolved(pk), + None => ProgramRef::Unresolved { raw_index: idx }, + } } - /// Returns the current instruction being visualized. - pub fn current_instruction(&self) -> Option<&Instruction> { - self.instructions.get(self.instruction_index) + /// Resolves the account at `position` in the instruction's accounts list. + /// Returns None if the instruction has no account at this position. + /// Returns Resolved or Unresolved for accounts the instruction does reference. + pub fn account(&self, position: usize) -> Option> { + let &idx = self.compiled_instruction.accounts.get(position)?; + Some(match self.account_keys.get(idx as usize) { + Some(pk) => AccountRef::Resolved(pk), + None => AccountRef::Unresolved { raw_index: idx }, + }) + } + + /// Raw instruction data bytes. No copy. + pub fn data(&self) -> &'a [u8] { + &self.compiled_instruction.data + } + + /// Number of account references in this instruction. + pub fn num_accounts(&self) -> usize { + self.compiled_instruction.accounts.len() + } + + /// Reference to the underlying compiled instruction. + pub fn compiled_instruction(&self) -> &'a solana_sdk::instruction::CompiledInstruction { + self.compiled_instruction + } + + /// Reference to the account keys array. + pub fn account_keys(&self) -> &'a [Pubkey] { + self.account_keys + } + + /// Resolve the program_id, returning Err if the index is out of bounds. + /// For visualizers that can't proceed without a known program. + pub fn resolve_program_id(&self) -> Result { + match self.program_id() { + ProgramRef::Resolved(pk) => Ok(*pk), + ProgramRef::Unresolved { raw_index } => Err(VisualSignError::DecodeError(format!( + "unresolved program at index {raw_index}" + ))), + } + } + + /// Resolve every account index in the instruction to an AccountMeta, + /// returning Err on the first unresolved index. Writable/signer bits are + /// not currently surfaced (set to false); IDL-based presets only need pubkeys. + pub fn resolve_accounts( + &self, + ) -> Result, VisualSignError> { + (0..self.num_accounts()) + .map(|i| match self.account(i) { + Some(AccountRef::Resolved(pk)) => Ok( + solana_sdk::instruction::AccountMeta::new_readonly(*pk, false), + ), + Some(AccountRef::Unresolved { raw_index }) => Err(VisualSignError::DecodeError( + format!("unresolved account index {raw_index} at position {i}"), + )), + None => Err(VisualSignError::DecodeError(format!( + "missing account at position {i}" + ))), + }) + .collect() } } pub struct SolanaIntegrationConfigData { pub programs: BTreeMap<&'static str, BTreeMap<&'static str, Vec<&'static str>>>, } + pub trait SolanaIntegrationConfig { fn new() -> Self where @@ -97,47 +190,34 @@ pub trait SolanaIntegrationConfig { fn data(&self) -> &SolanaIntegrationConfigData; - fn can_handle(&self, program_id: &str, _instruction: &Instruction) -> bool { - // For now, just check if we support the program_id - // You can extend this to parse instruction_data for specific instruction types + fn can_handle(&self, program_id: &str) -> bool { self.data() .programs .get(program_id) - .map(|_supported_instructions| true) // Can be refined to check specific instruction types + .map(|_supported_instructions| true) .unwrap_or(false) } } -// Trait for visualizing Solana Instructions - Copied from Sui chain_parser pub trait InstructionVisualizer { - /// Visualizes a specific instruction in a transaction. - /// - /// Returns `Some(SignablePayloadField)` if the instruction can be visualized, - /// or `None` if the instruction is not supported by this visualizer. fn visualize_tx_commands( &self, context: &VisualizerContext, ) -> Result; - /// Returns the config for the visualizer. fn get_config(&self) -> Option<&dyn SolanaIntegrationConfig>; - /// The identifier of this visualizer. fn kind(&self) -> VisualizerKind; - /// Checks if this visualizer can handle the given instruction. fn can_handle(&self, context: &VisualizerContext) -> bool { let Some(config) = self.get_config() else { return false; }; - let Some(instruction) = context.current_instruction() else { - return false; - }; - - // Use Solana's program_id and instruction data - let program_id = instruction.program_id.to_string(); - config.can_handle(&program_id, instruction) + match context.program_id() { + ProgramRef::Resolved(pk) => config.can_handle(&pk.to_string()), + ProgramRef::Unresolved { .. } => false, + } } } @@ -149,14 +229,6 @@ pub struct VisualizeResult { } /// Tries multiple visualizers in order, returning the first successful visualization. -/// -/// # Arguments -/// * `visualizers` - Slice of visualizer trait objects. -/// * `context` - The visualization context. -/// -/// # Returns -/// * `Some(VisualizeResult)` if any visualizer can handle the command, including which one. -/// * `None` if none can handle it. pub fn visualize_with_any( visualizers: &[&dyn InstructionVisualizer], context: &VisualizerContext, @@ -166,12 +238,6 @@ pub fn visualize_with_any( return None; } - eprintln!( - "Handling instruction {} with visualizer {:?}", - context.instruction_index(), - v.kind() - ); - Some( v.visualize_tx_commands(context) .map(|field| VisualizeResult { @@ -181,3 +247,88 @@ pub fn visualize_with_any( ) }) } + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +mod tests { + use super::*; + use solana_sdk::instruction::CompiledInstruction; + + #[test] + fn test_program_id_resolved() { + let keys = vec![Pubkey::new_unique(), Pubkey::new_unique()]; + let ci = CompiledInstruction { + program_id_index: 1, + accounts: vec![0], + data: vec![0xAA], + }; + let sender = SolanaAccount { + account_key: keys[0].to_string(), + signer: false, + writable: false, + }; + let registry = crate::idl::IdlRegistry::new(); + let ctx = VisualizerContext::new(&sender, &ci, &keys, ®istry, 0); + assert_eq!(ctx.program_id(), ProgramRef::Resolved(&keys[1])); + } + + #[test] + fn test_program_id_unresolved() { + let keys = vec![Pubkey::new_unique()]; + let ci = CompiledInstruction { + program_id_index: 99, + accounts: vec![], + data: vec![], + }; + let sender = SolanaAccount { + account_key: keys[0].to_string(), + signer: false, + writable: false, + }; + let registry = crate::idl::IdlRegistry::new(); + let ctx = VisualizerContext::new(&sender, &ci, &keys, ®istry, 0); + assert_eq!(ctx.program_id(), ProgramRef::Unresolved { raw_index: 99 }); + } + + #[test] + fn test_account_resolved_and_unresolved() { + let keys = vec![Pubkey::new_unique(), Pubkey::new_unique()]; + let ci = CompiledInstruction { + program_id_index: 1, + accounts: vec![0, 50], + data: vec![], + }; + let sender = SolanaAccount { + account_key: keys[0].to_string(), + signer: false, + writable: false, + }; + let registry = crate::idl::IdlRegistry::new(); + let ctx = VisualizerContext::new(&sender, &ci, &keys, ®istry, 0); + assert_eq!(ctx.account(0), Some(AccountRef::Resolved(&keys[0]))); + assert_eq!( + ctx.account(1), + Some(AccountRef::Unresolved { raw_index: 50 }) + ); + assert_eq!(ctx.account(99), None); // no such position + } + + #[test] + fn test_data_and_num_accounts() { + let keys = vec![Pubkey::new_unique()]; + let ci = CompiledInstruction { + program_id_index: 0, + accounts: vec![0, 0, 0], + data: vec![0xDE, 0xAD], + }; + let sender = SolanaAccount { + account_key: keys[0].to_string(), + signer: false, + writable: false, + }; + let registry = crate::idl::IdlRegistry::new(); + let ctx = VisualizerContext::new(&sender, &ci, &keys, ®istry, 0); + assert_eq!(ctx.data(), &[0xDE, 0xAD]); + assert_eq!(ctx.num_accounts(), 3); + } +} diff --git a/src/chain_parsers/visualsign-solana/src/core/txtypes/v0.rs b/src/chain_parsers/visualsign-solana/src/core/txtypes/v0.rs index e1ca4ca7..78637d88 100644 --- a/src/chain_parsers/visualsign-solana/src/core/txtypes/v0.rs +++ b/src/chain_parsers/visualsign-solana/src/core/txtypes/v0.rs @@ -1,9 +1,12 @@ +#[cfg(feature = "diagnostics")] +use crate::core::DecodeInstructionsResult; use crate::core::{ InstructionVisualizer, SolanaAccount, VisualizerContext, available_visualizers, visualize_with_any, }; -use solana_sdk::instruction::{AccountMeta, Instruction}; use solana_sdk::transaction::VersionedTransaction; +#[cfg(feature = "diagnostics")] +use visualsign::field_builders::create_diagnostic_field; use visualsign::{ AnnotatedPayloadField, SignablePayloadField, SignablePayloadFieldCommon, SignablePayloadFieldListLayout, SignablePayloadFieldPreviewLayout, SignablePayloadFieldTextV2, @@ -127,85 +130,131 @@ pub fn decode_v0_transfers( Ok(fields) } -/// Decode V0 transaction instructions using the visualizer framework -/// This works for all V0 transactions, including those with lookup tables +/// Decode V0 transaction instructions using the visualizer framework. +/// This works for all V0 transactions, including those with lookup tables. +/// Always succeeds -- data quality issues become diagnostics, per-instruction +/// failures are collected in errors. +#[cfg(feature = "diagnostics")] pub fn decode_v0_instructions( v0_message: &solana_sdk::message::v0::Message, idl_registry: &crate::idl::IdlRegistry, -) -> Result, VisualSignError> { - // Get visualizers + lint_config: &visualsign::lint::LintConfig, +) -> DecodeInstructionsResult { let visualizers: Vec> = available_visualizers(); let visualizers_refs: Vec<&dyn InstructionVisualizer> = visualizers.iter().map(|v| v.as_ref()).collect::>(); - // For V0 transactions, we need to resolve account keys from both static keys and lookup tables - // For now, we'll work with just the static account keys for instruction processing - // since lookup table accounts would require on-chain resolution let account_keys = &v0_message.account_keys; - // Convert compiled instructions to full instructions using static account keys only - // Instructions that reference lookup table accounts will be processed with limited info - let instructions: Vec = v0_message - .instructions - .iter() - .filter_map(|ci| { - // Only process instructions where program_id is in static account keys - if (ci.program_id_index as usize) < account_keys.len() { - let program_id = account_keys[ci.program_id_index as usize]; - - let accounts: Vec = ci - .accounts - .iter() - .filter_map(|&i| { - // Only include accounts that are in static account keys - if (i as usize) < account_keys.len() { - Some(AccountMeta::new_readonly(account_keys[i as usize], false)) - } else { - // Account is in lookup table - we can't resolve it without on-chain data - None - } - }) - .collect(); - - Some(Instruction { - program_id, - accounts, - data: ci.data.clone(), - }) - } else { - // Program ID is in lookup table - skip this instruction - None - } - }) - .collect(); + if account_keys.is_empty() { + let severity = lint_config.severity_for( + "transaction::empty_account_keys", + visualsign::lint::Severity::Error, + ); + let diagnostics = if matches!(severity, visualsign::lint::Severity::Allow) { + Vec::new() + } else { + vec![create_diagnostic_field( + "transaction::empty_account_keys", + "transaction", + severity, + "v0 transaction has no account keys", + None, + )] + }; + return DecodeInstructionsResult { + fields: Vec::new(), + errors: Vec::new(), + diagnostics, + }; + } + + // Diagnostic scan: check all indices, emit diagnostics for inaccessible ones. + // Uses the shared scan function from instructions.rs. + let diagnostics = super::super::instructions::scan_instruction_diagnostics( + &v0_message.instructions, + account_keys, + lint_config, + ); + + // Visualization: process every instruction (no skipping) + let mut fields: Vec = Vec::new(); + let mut errors: Vec<(usize, VisualSignError)> = Vec::new(); + + for (i, ci) in v0_message.instructions.iter().enumerate() { + let sender = SolanaAccount { + account_key: account_keys[0].to_string(), + signer: false, + writable: false, + }; + + let context = VisualizerContext::new(&sender, ci, account_keys, idl_registry, i); + + match visualize_with_any(&visualizers_refs, &context) { + Some(Ok(viz_result)) => fields.push(viz_result.field), + Some(Err(e)) => errors.push((i, e)), + None => errors.push(( + i, + VisualSignError::DecodeError(format!( + "No visualizer available for instruction at index {i}" + )), + )), + } + } + + DecodeInstructionsResult { + fields, + errors, + diagnostics, + } +} + +/// Diagnostics-off variant of `decode_v0_instructions`. Same semantics as the +/// legacy variant: empty account keys and visualizer errors abort with `Err`; +/// out-of-bounds indices flow through to the visualizer pipeline. +#[cfg(not(feature = "diagnostics"))] +pub fn decode_v0_instructions( + v0_message: &solana_sdk::message::v0::Message, + idl_registry: &crate::idl::IdlRegistry, +) -> Result, VisualSignError> { + let visualizers: Vec> = available_visualizers(); + let visualizers_refs: Vec<&dyn InstructionVisualizer> = + visualizers.iter().map(|v| v.as_ref()).collect::>(); + + let account_keys = &v0_message.account_keys; - // Process each instruction with the visualizer framework if account_keys.is_empty() { - return Err(VisualSignError::ParseError( - visualsign::vsptrait::TransactionParseError::DecodeError( - "V0 transaction has no account keys".to_string(), - ), + return Err(VisualSignError::DecodeError( + "v0 transaction has no account keys".to_string(), )); } - instructions - .iter() - .enumerate() - .filter_map(|(instruction_index, _)| { - // Create sender account from first account key (typically the fee payer) - let sender = SolanaAccount { - account_key: account_keys[0].to_string(), - signer: false, - writable: false, - }; - - visualize_with_any( - &visualizers_refs, - &VisualizerContext::new(&sender, instruction_index, &instructions, idl_registry), - ) - }) - .map(|res| res.map(|viz_result| viz_result.field)) - .collect() + let mut fields: Vec = Vec::new(); + for (i, ci) in v0_message.instructions.iter().enumerate() { + let sender = SolanaAccount { + account_key: account_keys[0].to_string(), + signer: false, + writable: false, + }; + + let context = VisualizerContext::new(&sender, ci, account_keys, idl_registry, i); + + match visualize_with_any(&visualizers_refs, &context) { + Some(Ok(viz_result)) => fields.push(viz_result.field), + Some(Err(e)) => { + return Err(VisualSignError::DecodeError(format!( + "instruction {i}: {e}" + ))); + } + None => { + return Err(VisualSignError::DecodeError(format!( + "No visualizer available for instruction at index {i}" + ))); + } + } + } + + Ok(fields) } /// Create a rich address lookup table field with detailed information @@ -374,3 +423,245 @@ pub fn create_address_lookup_table_field( }, }) } + +#[cfg(all(test, not(feature = "diagnostics")))] +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +mod off_tests { + use super::*; + use solana_sdk::pubkey::Pubkey; + + fn v0_message( + account_keys: Vec, + instructions: Vec, + ) -> solana_sdk::message::v0::Message { + solana_sdk::message::v0::Message { + header: solana_sdk::message::MessageHeader { + num_required_signatures: 1, + num_readonly_signed_accounts: 0, + num_readonly_unsigned_accounts: 0, + }, + account_keys, + recent_blockhash: solana_sdk::hash::Hash::default(), + instructions, + address_table_lookups: vec![], + } + } + + #[test] + fn test_empty_account_keys_returns_err() { + let msg = v0_message(vec![], vec![]); + let registry = crate::idl::IdlRegistry::new(); + let result = decode_v0_instructions(&msg, ®istry); + let Err(VisualSignError::DecodeError(text)) = result else { + panic!("expected DecodeError, got {result:?}"); + }; + assert!(text.contains("no account keys"), "msg was: {text}"); + } + + #[test] + fn test_oob_indices_flow_through_as_ok() { + let key0 = Pubkey::new_unique(); + let msg = v0_message( + vec![key0], + vec![solana_sdk::instruction::CompiledInstruction { + program_id_index: 99, // OOB + accounts: vec![0, 50], // 50 also OOB + data: vec![0xDD], + }], + ); + let registry = crate::idl::IdlRegistry::new(); + let fields = decode_v0_instructions(&msg, ®istry).expect("OOB should not abort"); + assert_eq!(fields.len(), 1); + } +} + +#[cfg(all(test, feature = "diagnostics"))] +#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] +mod tests { + use super::*; + use solana_sdk::pubkey::Pubkey; + use visualsign::SignablePayloadField; + use visualsign::lint::LintConfig; + + fn v0_message_with_oob_program_id() -> solana_sdk::message::v0::Message { + let key0 = Pubkey::new_unique(); + let key1 = Pubkey::new_unique(); + solana_sdk::message::v0::Message { + header: solana_sdk::message::MessageHeader { + num_required_signatures: 1, + num_readonly_signed_accounts: 0, + num_readonly_unsigned_accounts: 0, + }, + account_keys: vec![key0, key1], + recent_blockhash: solana_sdk::hash::Hash::default(), + instructions: vec![ + solana_sdk::instruction::CompiledInstruction { + program_id_index: 1, + accounts: vec![0], + data: vec![0xAA], + }, + solana_sdk::instruction::CompiledInstruction { + program_id_index: 99, // OOB: only 2 static keys + accounts: vec![0], + data: vec![0xBB], + }, + ], + address_table_lookups: vec![], + } + } + + fn v0_message_with_oob_program_id_and_oob_account() -> solana_sdk::message::v0::Message { + let key0 = Pubkey::new_unique(); + let key1 = Pubkey::new_unique(); + solana_sdk::message::v0::Message { + header: solana_sdk::message::MessageHeader { + num_required_signatures: 1, + num_readonly_signed_accounts: 0, + num_readonly_unsigned_accounts: 0, + }, + account_keys: vec![key0, key1], + recent_blockhash: solana_sdk::hash::Hash::default(), + instructions: vec![solana_sdk::instruction::CompiledInstruction { + program_id_index: 99, // OOB + accounts: vec![0, 88], // 88 is also OOB + data: vec![0xCC], + }], + address_table_lookups: vec![], + } + } + + #[test] + fn test_v0_oob_program_id_emits_diagnostic() { + let msg = v0_message_with_oob_program_id(); + let registry = crate::idl::IdlRegistry::new(); + let config = LintConfig::default(); + let result = decode_v0_instructions(&msg, ®istry, &config); + let fields = [result.fields, result.diagnostics].concat(); + + let warns: Vec<_> = fields + .iter() + .filter_map(|f| match &f.signable_payload_field { + SignablePayloadField::Diagnostic { diagnostic, .. } + if diagnostic.level == "warn" => + { + Some(diagnostic) + } + _ => None, + }) + .collect(); + assert_eq!(warns.len(), 1); + assert_eq!(warns[0].rule, "transaction::oob_program_id"); + assert_eq!(warns[0].instruction_index, Some(1)); + + let passes: Vec<_> = fields + .iter() + .filter_map(|f| match &f.signable_payload_field { + SignablePayloadField::Diagnostic { diagnostic, .. } if diagnostic.level == "ok" => { + Some(diagnostic) + } + _ => None, + }) + .collect(); + assert!( + passes + .iter() + .any(|d| d.rule == "transaction::oob_account_index") + ); + } + + #[test] + fn test_v0_oob_program_id_and_oob_account_index_emits_both_diagnostics() { + let msg = v0_message_with_oob_program_id_and_oob_account(); + let registry = crate::idl::IdlRegistry::new(); + let config = LintConfig::default(); + let result = decode_v0_instructions(&msg, ®istry, &config); + let fields = [result.fields, result.diagnostics].concat(); + + let warns: Vec<_> = fields + .iter() + .filter_map(|f| match &f.signable_payload_field { + SignablePayloadField::Diagnostic { diagnostic, .. } + if diagnostic.level == "warn" => + { + Some(diagnostic) + } + _ => None, + }) + .collect(); + assert_eq!(warns.len(), 2); + assert!( + warns + .iter() + .any(|d| d.rule == "transaction::oob_program_id") + ); + assert!( + warns + .iter() + .any(|d| d.rule == "transaction::oob_account_index") + ); + let acct_warn = warns + .iter() + .find(|d| d.rule == "transaction::oob_account_index") + .unwrap(); + assert_eq!(acct_warn.instruction_index, Some(0)); + assert!(acct_warn.message.contains("88")); + + // No ok-diagnostics when both rules fire + let passes: Vec<_> = fields + .iter() + .filter_map(|f| match &f.signable_payload_field { + SignablePayloadField::Diagnostic { diagnostic, .. } if diagnostic.level == "ok" => { + Some(diagnostic) + } + _ => None, + }) + .collect(); + assert!(passes.is_empty(), "no ok-diagnostics when both rules fire"); + } + + #[test] + fn test_v0_valid_transaction_emits_two_ok_diagnostics() { + let key0 = Pubkey::new_unique(); + let key1 = Pubkey::new_unique(); + let msg = solana_sdk::message::v0::Message { + header: solana_sdk::message::MessageHeader { + num_required_signatures: 1, + num_readonly_signed_accounts: 0, + num_readonly_unsigned_accounts: 0, + }, + account_keys: vec![key0, key1], + recent_blockhash: solana_sdk::hash::Hash::default(), + instructions: vec![solana_sdk::instruction::CompiledInstruction { + program_id_index: 1, + accounts: vec![0], + data: vec![0xDD], + }], + address_table_lookups: vec![], + }; + let registry = crate::idl::IdlRegistry::new(); + let config = LintConfig::default(); + let result = decode_v0_instructions(&msg, ®istry, &config); + let fields = [result.fields, result.diagnostics].concat(); + + let passes: Vec<_> = fields + .iter() + .filter_map(|f| match &f.signable_payload_field { + SignablePayloadField::Diagnostic { diagnostic, .. } if diagnostic.level == "ok" => { + Some(diagnostic) + } + _ => None, + }) + .collect(); + assert_eq!(passes.len(), 2); + assert!( + passes + .iter() + .any(|d| d.rule == "transaction::oob_program_id") + ); + assert!( + passes + .iter() + .any(|d| d.rule == "transaction::oob_account_index") + ); + } +} diff --git a/src/chain_parsers/visualsign-solana/src/core/visualsign.rs b/src/chain_parsers/visualsign-solana/src/core/visualsign.rs index 9caa1504..20f19fa5 100644 --- a/src/chain_parsers/visualsign-solana/src/core/visualsign.rs +++ b/src/chain_parsers/visualsign-solana/src/core/visualsign.rs @@ -20,6 +20,35 @@ use visualsign::{ }, }; +/// Append decode errors as diagnostics and lint diagnostics to the output fields. +/// decode::visualizer_error is intentionally not routed through LintConfig -- +/// visualizer failures are always surfaced so consumers know which +/// instructions could not be decoded. +#[cfg(feature = "diagnostics")] +fn append_diagnostics( + fields: &mut Vec, + result: &instructions::DecodeInstructionsResult, +) { + for (idx, err) in &result.errors { + fields.push( + visualsign::field_builders::create_diagnostic_field( + "decode::visualizer_error", + "decode", + visualsign::lint::Severity::Error, + &format!("instruction {idx}: {err}"), + Some(*idx as u32), + ) + .signable_payload_field, + ); + } + fields.extend( + result + .diagnostics + .iter() + .map(|e| e.signable_payload_field.clone()), + ); +} + /// Wrapper around Solana's transaction types that implements the Transaction trait #[derive(Debug, Clone)] pub enum SolanaTransactionWrapper { @@ -159,23 +188,25 @@ impl VisualSignConverter for SolanaVisualSignConverter transaction_wrapper: SolanaTransactionWrapper, options: VisualSignOptions, ) -> Result { + #[cfg(feature = "diagnostics")] + let lint_config = visualsign::lint::LintConfig::default(); match transaction_wrapper { - SolanaTransactionWrapper::Legacy(transaction) => { - // Convert the legacy transaction to a VisualSign payload - convert_to_visual_sign_payload( - &transaction, - options.decode_transfers, - options.transaction_name.clone(), - &options, - ) - } + SolanaTransactionWrapper::Legacy(transaction) => convert_to_visual_sign_payload( + &transaction, + options.decode_transfers, + options.transaction_name.clone(), + &options, + #[cfg(feature = "diagnostics")] + &lint_config, + ), SolanaTransactionWrapper::Versioned(versioned_tx) => { - // Handle versioned transactions convert_versioned_to_visual_sign_payload( &versioned_tx, options.decode_transfers, options.transaction_name.clone(), &options, + #[cfg(feature = "diagnostics")] + &lint_config, ) } } @@ -218,6 +249,7 @@ fn convert_to_visual_sign_payload( decode_transfers: bool, title: Option, options: &VisualSignOptions, + #[cfg(feature = "diagnostics")] lint_config: &visualsign::lint::LintConfig, ) -> Result { let message = &transaction.message; @@ -243,13 +275,27 @@ fn convert_to_visual_sign_payload( ); } - // Process instructions with visualizers (pass IDL registry for future use) + // Process instructions with visualizers + #[cfg(feature = "diagnostics")] + let decode_result = instructions::decode_instructions(transaction, &idl_registry, lint_config); + #[cfg(feature = "diagnostics")] fields.extend( - instructions::decode_instructions(transaction, &idl_registry)? + decode_result + .fields .iter() .map(|e| e.signable_payload_field.clone()), ); + #[cfg(not(feature = "diagnostics"))] + { + let decoded_fields = instructions::decode_instructions(transaction, &idl_registry)?; + fields.extend( + decoded_fields + .iter() + .map(|e| e.signable_payload_field.clone()), + ); + } + // Decode and sort accounts using the dedicated function let accounts = decode_accounts(message)?; @@ -258,6 +304,9 @@ fn convert_to_visual_sign_payload( // Add Accounts field at the bottom using PreviewLayout instead of ListLayout fields.push(preview_layout_advanced); + #[cfg(feature = "diagnostics")] + append_diagnostics(&mut fields, &decode_result); + Ok(SignablePayload::new( 0, title.unwrap_or_else(|| "Solana Transaction".to_string()), @@ -273,26 +322,32 @@ fn convert_versioned_to_visual_sign_payload( decode_transfers: bool, title: Option, options: &VisualSignOptions, + #[cfg(feature = "diagnostics")] lint_config: &visualsign::lint::LintConfig, ) -> Result { match &versioned_tx.message { VersionedMessage::Legacy(legacy_message) => { - // For legacy messages in versioned transactions, create a legacy transaction let legacy_tx = SolanaTransaction { signatures: versioned_tx.signatures.clone(), message: legacy_message.clone(), }; - convert_to_visual_sign_payload(&legacy_tx, decode_transfers, title, options) - } - VersionedMessage::V0(v0_message) => { - // Handle V0 transactions - try to use the same instruction processing pipeline - convert_v0_to_visual_sign_payload( - versioned_tx, - v0_message, + convert_to_visual_sign_payload( + &legacy_tx, decode_transfers, title, options, + #[cfg(feature = "diagnostics")] + lint_config, ) } + VersionedMessage::V0(v0_message) => convert_v0_to_visual_sign_payload( + versioned_tx, + v0_message, + decode_transfers, + title, + options, + #[cfg(feature = "diagnostics")] + lint_config, + ), } } @@ -303,6 +358,7 @@ fn convert_v0_to_visual_sign_payload( decode_transfers: bool, title: Option, options: &VisualSignOptions, + #[cfg(feature = "diagnostics")] lint_config: &visualsign::lint::LintConfig, ) -> Result { // Create IDL registry from options metadata let idl_registry = create_idl_registry_from_options(options)?; @@ -328,9 +384,22 @@ fn convert_v0_to_visual_sign_payload( // Directly process V0 instructions using the visualizer framework // This approach works for all V0 transactions, including those with lookup tables + #[cfg(feature = "diagnostics")] + let v0_result = decode_v0_instructions(v0_message, &idl_registry, lint_config); + #[cfg(feature = "diagnostics")] + for (index, instruction_field) in v0_result.fields.iter().enumerate() { + tracing::debug!( + "Handling instruction {} with visualizer {:?}", + index, + "V0 Instruction" + ); + fields.push(instruction_field.signable_payload_field.clone()); + } + + #[cfg(not(feature = "diagnostics"))] match decode_v0_instructions(v0_message, &idl_registry) { - Ok(instruction_fields) => { - for (index, instruction_field) in instruction_fields.iter().enumerate() { + Ok(v0_fields) => { + for (index, instruction_field) in v0_fields.iter().enumerate() { tracing::debug!( "Handling instruction {} with visualizer {:?}", index, @@ -382,6 +451,9 @@ fn convert_v0_to_visual_sign_payload( let preview_layout_advanced = create_accounts_advanced_preview_layout("Accounts", &accounts)?; fields.push(preview_layout_advanced); + #[cfg(feature = "diagnostics")] + append_diagnostics(&mut fields, &v0_result); + Ok(SignablePayload::new( 0, title.unwrap_or_else(|| "Solana V0 Transaction".to_string()), @@ -1039,10 +1111,11 @@ mod tests { let payload = payload_result.unwrap(); // Verify we have instruction fields (should not be empty) + // Labels are now operation-specific (e.g., program ID) rather than "Instruction N" let instruction_fields: Vec<_> = payload .fields .iter() - .filter(|f| f.label().starts_with("Instruction")) + .filter(|f| matches!(f, SignablePayloadField::PreviewLayout { .. })) .collect(); assert!( @@ -1050,11 +1123,10 @@ mod tests { "Should have at least one instruction field for TokenKeg program" ); - // Verify we have exactly 1 instruction (as shown in the issue) - assert_eq!( - instruction_fields.len(), - 1, - "Should have exactly 1 instruction" + // Verify we have instruction preview layouts (network + instruction + accounts = at least 1 instruction) + assert!( + !instruction_fields.is_empty(), + "Should have at least 1 instruction preview layout" ); // Verify the instruction contains the TokenKeg program ID @@ -1068,4 +1140,67 @@ mod tests { println!("Number of instruction fields: {}", instruction_fields.len()); println!("JSON output:\n{json_str}"); } + + // Lock in main's behavior: when v0 instruction decoding fails on the + // diagnostics-OFF (production) path, the converter pushes a TextV2 + // "Instruction Decoding Note" field rather than erroring out the entire + // conversion. Wallets render this note instead of seeing a hard failure + // for an otherwise-renderable transaction. The empty-account-keys path + // is the simplest deterministic trigger; other failure paths + // (visualizer Err, etc.) share the same fallback handling. + #[cfg(not(feature = "diagnostics"))] + #[test] + fn test_v0_decode_failure_emits_instruction_decoding_note() { + let v0_message = solana_sdk::message::v0::Message { + header: solana_sdk::message::MessageHeader { + num_required_signatures: 0, + num_readonly_signed_accounts: 0, + num_readonly_unsigned_accounts: 0, + }, + account_keys: vec![], + recent_blockhash: solana_sdk::hash::Hash::default(), + instructions: vec![solana_sdk::instruction::CompiledInstruction { + program_id_index: 0, + accounts: vec![], + data: vec![], + }], + address_table_lookups: vec![], + }; + let versioned_tx = VersionedTransaction { + signatures: vec![], + message: VersionedMessage::V0(v0_message.clone()), + }; + let options = VisualSignOptions { + metadata: None, + decode_transfers: false, + transaction_name: None, + developer_config: None, + }; + + let payload = + convert_v0_to_visual_sign_payload(&versioned_tx, &v0_message, false, None, &options) + .expect("convert should succeed via Instruction Decoding Note fallback"); + + let note = payload + .fields + .iter() + .find_map(|f| match f { + SignablePayloadField::TextV2 { common, text_v2 } + if common.label == "Instruction Decoding Note" => + { + Some(text_v2.text.clone()) + } + _ => None, + }) + .expect("Instruction Decoding Note field missing from payload"); + + assert!( + note.contains("Instruction decoding failed"), + "unexpected note body: {note}" + ); + assert!( + note.contains("no account keys"), + "note should propagate underlying error: {note}" + ); + } } diff --git a/src/chain_parsers/visualsign-solana/src/presets/associated_token_account/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/associated_token_account/mod.rs index e56696a2..cb86970a 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/associated_token_account/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/associated_token_account/mod.rs @@ -3,7 +3,7 @@ mod config; use crate::core::{ - InstructionVisualizer, SolanaIntegrationConfig, VisualizerContext, VisualizerKind, + InstructionVisualizer, ProgramRef, SolanaIntegrationConfig, VisualizerContext, VisualizerKind, }; use config::AssociatedTokenAccountConfig; use spl_associated_token_account::instruction::AssociatedTokenAccountInstruction; @@ -24,66 +24,10 @@ impl InstructionVisualizer for AssociatedTokenAccountVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; - - let ata_instruction = parse_ata_instruction(&instruction.data) + let ata_instruction = parse_ata_instruction(context.data()) .map_err(|e| VisualSignError::DecodeError(e.to_string()))?; - let instruction_text = format_ata_instruction(&ata_instruction); - - let condensed = SignablePayloadFieldListLayout { - fields: vec![AnnotatedPayloadField { - static_annotation: None, - dynamic_annotation: None, - signable_payload_field: SignablePayloadField::TextV2 { - common: SignablePayloadFieldCommon { - fallback_text: instruction_text.clone(), - label: "Instruction".to_string(), - }, - text_v2: SignablePayloadFieldTextV2 { - text: instruction_text.clone(), - }, - }, - }], - }; - - let expanded = SignablePayloadFieldListLayout { - fields: vec![ - create_text_field("Program ID", &instruction.program_id.to_string())?, - create_text_field("Instruction", &instruction_text)?, - ], - }; - - let preview_layout = SignablePayloadFieldPreviewLayout { - title: Some(SignablePayloadFieldTextV2 { - text: instruction_text.clone(), - }), - subtitle: Some(SignablePayloadFieldTextV2 { - text: String::new(), - }), - condensed: Some(condensed), - expanded: Some(expanded), - }; - - let fallback_instruction_str = format!( - "Program ID: {}\nData: {}", - instruction.program_id, - hex::encode(&instruction.data) - ); - - Ok(AnnotatedPayloadField { - static_annotation: None, - dynamic_annotation: None, - signable_payload_field: SignablePayloadField::PreviewLayout { - common: SignablePayloadFieldCommon { - label: format!("Instruction {}", context.instruction_index() + 1), - fallback_text: fallback_instruction_str, - }, - preview_layout, - }, - }) + create_ata_preview_layout(&ata_instruction, context) } fn get_config(&self) -> Option<&dyn SolanaIntegrationConfig> { @@ -95,10 +39,68 @@ impl InstructionVisualizer for AssociatedTokenAccountVisualizer { } } +fn create_ata_preview_layout( + ata_instruction: &AssociatedTokenAccountInstruction, + context: &VisualizerContext, +) -> Result { + let program_id_str = match context.program_id() { + ProgramRef::Resolved(pk) => pk.to_string(), + ProgramRef::Unresolved { raw_index } => format!("unresolved({raw_index})"), + }; + let instruction_text = format_ata_instruction(ata_instruction); + + let condensed = SignablePayloadFieldListLayout { + fields: vec![AnnotatedPayloadField { + static_annotation: None, + dynamic_annotation: None, + signable_payload_field: SignablePayloadField::TextV2 { + common: SignablePayloadFieldCommon { + fallback_text: instruction_text.clone(), + label: "Instruction".to_string(), + }, + text_v2: SignablePayloadFieldTextV2 { + text: instruction_text.clone(), + }, + }, + }], + }; + + let expanded = SignablePayloadFieldListLayout { + fields: vec![ + create_text_field("Program ID", &program_id_str)?, + create_text_field("Instruction", &instruction_text)?, + ], + }; + + let preview_layout = SignablePayloadFieldPreviewLayout { + title: Some(SignablePayloadFieldTextV2 { + text: instruction_text.clone(), + }), + subtitle: Some(SignablePayloadFieldTextV2 { + text: String::new(), + }), + condensed: Some(condensed), + expanded: Some(expanded), + }; + + Ok(AnnotatedPayloadField { + static_annotation: None, + dynamic_annotation: None, + signable_payload_field: SignablePayloadField::PreviewLayout { + common: SignablePayloadFieldCommon { + label: format!("Instruction {}", context.instruction_index() + 1), + fallback_text: format!( + "Program ID: {}\nData: {}", + program_id_str, + hex::encode(context.data()) + ), + }, + preview_layout, + }, + }) +} + fn parse_ata_instruction(data: &[u8]) -> Result { - // The original SPL ATA "Create" instruction used empty data (no discriminator). - // Discriminator bytes were added later for CreateIdempotent and RecoverNested. - // Empty data or data[0] == 0 both mean "Create". if data.is_empty() { return Ok(AssociatedTokenAccountInstruction::Create); } @@ -129,8 +131,6 @@ mod tests { #[test] fn test_parse_ata_instruction_empty_data() { - // Test the case where instruction data is empty (original SPL ATA Create format) - // This is from a real transaction where the ATA instruction had no data bytes let empty_data: &[u8] = &[]; let instruction = parse_ata_instruction(empty_data) .expect("Failed to parse ATA instruction with empty data"); @@ -146,7 +146,6 @@ mod tests { #[test] fn test_parse_ata_instruction_with_discriminator_0() { - // Test explicit discriminator byte 0 (also means Create) let data = [0u8]; let instruction = parse_ata_instruction(&data) .expect("Failed to parse ATA instruction with discriminator 0"); @@ -159,7 +158,6 @@ mod tests { #[test] fn test_parse_ata_instruction_create_idempotent() { - // Test CreateIdempotent (discriminator 1) let data = [1u8]; let instruction = parse_ata_instruction(&data).expect("Failed to parse CreateIdempotent instruction"); @@ -178,7 +176,6 @@ mod tests { #[test] fn test_parse_ata_instruction_recover_nested() { - // Test RecoverNested (discriminator 2) let data = [2u8]; let instruction = parse_ata_instruction(&data).expect("Failed to parse RecoverNested instruction"); @@ -197,7 +194,6 @@ mod tests { #[test] fn test_parse_ata_instruction_unknown() { - // Test unknown discriminator let data = [99u8]; let result = parse_ata_instruction(&data); diff --git a/src/chain_parsers/visualsign-solana/src/presets/compute_budget/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/compute_budget/mod.rs index ed6510a9..1e2477b3 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/compute_budget/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/compute_budget/mod.rs @@ -3,7 +3,7 @@ mod config; use crate::core::{ - InstructionVisualizer, SolanaIntegrationConfig, VisualizerContext, VisualizerKind, + InstructionVisualizer, ProgramRef, SolanaIntegrationConfig, VisualizerContext, VisualizerKind, }; use borsh::de::BorshDeserialize; use config::ComputeBudgetConfig; @@ -25,71 +25,12 @@ impl InstructionVisualizer for ComputeBudgetVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; - - let compute_budget_instruction = - ComputeBudgetInstruction::try_from_slice(&instruction.data).map_err(|e| { - VisualSignError::DecodeError(format!( - "Failed to parse compute budget instruction: {e}" - )) - })?; - - let instruction_text = format_compute_budget_instruction(&compute_budget_instruction); - - let condensed = SignablePayloadFieldListLayout { - fields: vec![AnnotatedPayloadField { - static_annotation: None, - dynamic_annotation: None, - signable_payload_field: SignablePayloadField::TextV2 { - common: SignablePayloadFieldCommon { - fallback_text: instruction_text.clone(), - label: "Instruction".to_string(), - }, - text_v2: SignablePayloadFieldTextV2 { - text: instruction_text.clone(), - }, - }, - }], - }; - - let expanded = SignablePayloadFieldListLayout { - fields: create_compute_budget_expanded_fields( - &compute_budget_instruction, - &instruction.program_id.to_string(), - &instruction.data, - )?, - }; - - let preview_layout = SignablePayloadFieldPreviewLayout { - title: Some(SignablePayloadFieldTextV2 { - text: instruction_text.clone(), - }), - subtitle: Some(SignablePayloadFieldTextV2 { - text: String::new(), - }), - condensed: Some(condensed), - expanded: Some(expanded), - }; - - let fallback_instruction_str = format!( - "Program ID: {}\nData: {}", - instruction.program_id, - hex::encode(&instruction.data) - ); - - Ok(AnnotatedPayloadField { - static_annotation: None, - dynamic_annotation: None, - signable_payload_field: SignablePayloadField::PreviewLayout { - common: SignablePayloadFieldCommon { - label: format!("Instruction {}", context.instruction_index() + 1), - fallback_text: fallback_instruction_str, - }, - preview_layout, - }, - }) + let compute_budget_instruction = ComputeBudgetInstruction::try_from_slice(context.data()) + .map_err(|e| { + VisualSignError::DecodeError(format!("Failed to parse compute budget instruction: {e}")) + })?; + + create_compute_budget_preview_layout(&compute_budget_instruction, context) } fn get_config(&self) -> Option<&dyn SolanaIntegrationConfig> { @@ -119,51 +60,100 @@ fn format_compute_budget_instruction(instruction: &ComputeBudgetInstruction) -> } } -fn create_compute_budget_expanded_fields( +fn create_compute_budget_preview_layout( instruction: &ComputeBudgetInstruction, - program_id: &str, - data: &[u8], -) -> Result, VisualSignError> { - let mut fields = vec![create_text_field("Program ID", program_id)?]; + context: &VisualizerContext, +) -> Result { + let program_id_str = match context.program_id() { + ProgramRef::Resolved(pk) => pk.to_string(), + ProgramRef::Unresolved { raw_index } => format!("unresolved({raw_index})"), + }; + let instruction_text = format_compute_budget_instruction(instruction); + + let condensed = SignablePayloadFieldListLayout { + fields: vec![AnnotatedPayloadField { + static_annotation: None, + dynamic_annotation: None, + signable_payload_field: SignablePayloadField::TextV2 { + common: SignablePayloadFieldCommon { + fallback_text: instruction_text.clone(), + label: "Instruction".to_string(), + }, + text_v2: SignablePayloadFieldTextV2 { + text: instruction_text.clone(), + }, + }, + }], + }; + + let mut expanded_fields = vec![create_text_field("Program ID", &program_id_str)?]; - // Add specific fields based on instruction type match instruction { ComputeBudgetInstruction::RequestHeapFrame(bytes) => { - fields.push(create_number_field( + expanded_fields.push(create_number_field( "Heap Frame Size", &bytes.to_string(), "bytes", )?); } ComputeBudgetInstruction::SetComputeUnitLimit(units) => { - fields.push(create_number_field( + expanded_fields.push(create_number_field( "Compute Unit Limit", &units.to_string(), "units", )?); } ComputeBudgetInstruction::SetComputeUnitPrice(micro_lamports) => { - fields.push(create_number_field( + expanded_fields.push(create_number_field( "Price per Compute Unit", µ_lamports.to_string(), "micro-lamports", )?); } ComputeBudgetInstruction::SetLoadedAccountsDataSizeLimit(bytes) => { - fields.push(create_number_field( + expanded_fields.push(create_number_field( "Data Size Limit", &bytes.to_string(), "bytes", )?); } - ComputeBudgetInstruction::Unused => { - // No additional fields for unused instruction - } + ComputeBudgetInstruction::Unused => {} } - let hex_fallback_string = hex::encode(data).to_string(); - let raw_data_field = create_raw_data_field(data, Some(hex_fallback_string))?; - - fields.push(raw_data_field); - Ok(fields) + let hex_fallback_string = hex::encode(context.data()); + expanded_fields.push(create_raw_data_field( + context.data(), + Some(hex_fallback_string), + )?); + + let expanded = SignablePayloadFieldListLayout { + fields: expanded_fields, + }; + + let preview_layout = SignablePayloadFieldPreviewLayout { + title: Some(SignablePayloadFieldTextV2 { + text: instruction_text.clone(), + }), + subtitle: Some(SignablePayloadFieldTextV2 { + text: String::new(), + }), + condensed: Some(condensed), + expanded: Some(expanded), + }; + + Ok(AnnotatedPayloadField { + static_annotation: None, + dynamic_annotation: None, + signable_payload_field: SignablePayloadField::PreviewLayout { + common: SignablePayloadFieldCommon { + label: format!("Instruction {}", context.instruction_index() + 1), + fallback_text: format!( + "Program ID: {}\nData: {}", + program_id_str, + hex::encode(context.data()) + ), + }, + preview_layout, + }, + }) } diff --git a/src/chain_parsers/visualsign-solana/src/presets/dflow_aggregator/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/dflow_aggregator/mod.rs index 65d65eca..995dfa24 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/dflow_aggregator/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/dflow_aggregator/mod.rs @@ -32,15 +32,14 @@ impl InstructionVisualizer for DflowAggregatorVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + let program_id = context.resolve_program_id()?.to_string(); + let accounts = context.resolve_accounts()?; + let data = context.data(); - let program_id = instruction.program_id.to_string(); - let instruction_data_hex = hex::encode(&instruction.data); + let instruction_data_hex = hex::encode(data); let fallback_text = format!("Program ID: {program_id}\nData: {instruction_data_hex}"); - let parsed = parse_dflow_aggregator_instruction(&instruction.data, &instruction.accounts); + let parsed = parse_dflow_aggregator_instruction(data, &accounts); let (title, condensed_fields, mut expanded_fields) = match parsed { Ok(parsed) => build_parsed_fields(&parsed, &program_id)?, @@ -53,10 +52,7 @@ impl InstructionVisualizer for DflowAggregatorVisualizer { let condensed = SignablePayloadFieldListLayout { fields: condensed_fields, }; - expanded_fields.push(create_raw_data_field( - &instruction.data, - Some(instruction_data_hex), - )?); + expanded_fields.push(create_raw_data_field(data, Some(instruction_data_hex))?); let expanded = SignablePayloadFieldListLayout { fields: expanded_fields, }; diff --git a/src/chain_parsers/visualsign-solana/src/presets/exponent_finance/config.rs b/src/chain_parsers/visualsign-solana/src/presets/exponent_finance/config.rs index 41971c48..9021283a 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/exponent_finance/config.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/exponent_finance/config.rs @@ -1,6 +1,6 @@ use super::EXPONENT_FINANCE_PROGRAM_ID; use crate::core::{SolanaIntegrationConfig, SolanaIntegrationConfigData}; -use std::collections::HashMap; +use std::collections::BTreeMap; pub struct ExponentFinanceConfig; @@ -12,8 +12,8 @@ impl SolanaIntegrationConfig for ExponentFinanceConfig { fn data(&self) -> &SolanaIntegrationConfigData { static DATA: std::sync::OnceLock = std::sync::OnceLock::new(); DATA.get_or_init(|| { - let mut programs = HashMap::new(); - let mut instructions = HashMap::new(); + let mut programs = BTreeMap::new(); + let mut instructions = BTreeMap::new(); instructions.insert("*", vec!["*"]); programs.insert(EXPONENT_FINANCE_PROGRAM_ID, instructions); SolanaIntegrationConfigData { programs } diff --git a/src/chain_parsers/visualsign-solana/src/presets/exponent_finance/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/exponent_finance/mod.rs index c3be880d..26eff078 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/exponent_finance/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/exponent_finance/mod.rs @@ -32,17 +32,16 @@ impl InstructionVisualizer for ExponentFinanceVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + let program_id = context.resolve_program_id()?.to_string(); + let accounts = context.resolve_accounts()?; + let data = context.data(); - let program_id = instruction.program_id.to_string(); - let instruction_data_hex = hex::encode(&instruction.data); + let instruction_data_hex = hex::encode(data); let fallback_text = format!("Program ID: {program_id}\nData: {instruction_data_hex}"); - let parsed = parse_exponent_finance_instruction(&instruction.data, &instruction.accounts); + let parsed = parse_exponent_finance_instruction(data, &accounts); - let (title, condensed_fields, expanded_fields) = match parsed { + let (title, condensed_fields, mut expanded_fields) = match parsed { Ok(parsed) => build_parsed_fields(&parsed, &program_id)?, Err(_) => build_fallback_fields(&program_id)?, }; @@ -50,10 +49,9 @@ impl InstructionVisualizer for ExponentFinanceVisualizer { let condensed = SignablePayloadFieldListLayout { fields: condensed_fields, }; - let expanded_with_raw = - append_raw_data(expanded_fields, &instruction.data, &instruction_data_hex)?; + expanded_fields.push(create_raw_data_field(data, Some(instruction_data_hex))?); let expanded = SignablePayloadFieldListLayout { - fields: expanded_with_raw, + fields: expanded_fields, }; let preview_layout = SignablePayloadFieldPreviewLayout { @@ -205,15 +203,6 @@ fn build_fallback_fields( Ok((title, condensed_fields, expanded_fields)) } -fn append_raw_data( - mut fields: Vec, - data: &[u8], - hex_str: &str, -) -> Result, VisualSignError> { - fields.push(create_raw_data_field(data, Some(hex_str.to_string()))?); - Ok(fields) -} - fn format_arg_value(value: &serde_json::Value) -> String { match value { serde_json::Value::String(s) => s.clone(), diff --git a/src/chain_parsers/visualsign-solana/src/presets/jupiter_borrow/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/jupiter_borrow/mod.rs index a55cdee5..d81d3b24 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/jupiter_borrow/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/jupiter_borrow/mod.rs @@ -30,28 +30,24 @@ impl InstructionVisualizer for JupiterBorrowVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + let program_id = context.resolve_program_id()?; + let accounts = context.resolve_accounts()?; + let data = context.data(); - let instruction_data_hex = hex::encode(&instruction.data); - let fallback_text = format!( - "Program ID: {}\nData: {instruction_data_hex}", - instruction.program_id, - ); + let instruction_data_hex = hex::encode(data); + let fallback_text = format!("Program ID: {program_id}\nData: {instruction_data_hex}",); - let parsed = parse_jupiter_borrow_instruction(&instruction.data, &instruction.accounts); + let parsed = parse_jupiter_borrow_instruction(data, &accounts); let (title, condensed_fields, expanded_fields) = match parsed { - Ok(parsed) => build_parsed_fields(&parsed, &instruction.program_id.to_string()), - Err(_) => build_fallback_fields(&instruction.program_id.to_string()), + Ok(parsed) => build_parsed_fields(&parsed, &program_id.to_string()), + Err(_) => build_fallback_fields(&program_id.to_string()), }; let condensed = SignablePayloadFieldListLayout { fields: condensed_fields, }; - let expanded_with_raw = - append_raw_data(expanded_fields, &instruction.data, &instruction_data_hex); + let expanded_with_raw = append_raw_data(expanded_fields, data, &instruction_data_hex); let expanded = SignablePayloadFieldListLayout { fields: expanded_with_raw, }; diff --git a/src/chain_parsers/visualsign-solana/src/presets/jupiter_earn/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/jupiter_earn/mod.rs index 137d59d7..9b69bce4 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/jupiter_earn/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/jupiter_earn/mod.rs @@ -30,28 +30,24 @@ impl InstructionVisualizer for JupiterEarnVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + let program_id = context.resolve_program_id()?; + let accounts = context.resolve_accounts()?; + let data = context.data(); - let instruction_data_hex = hex::encode(&instruction.data); - let fallback_text = format!( - "Program ID: {}\nData: {instruction_data_hex}", - instruction.program_id, - ); + let instruction_data_hex = hex::encode(data); + let fallback_text = format!("Program ID: {program_id}\nData: {instruction_data_hex}",); - let parsed = parse_jupiter_earn_instruction(&instruction.data, &instruction.accounts); + let parsed = parse_jupiter_earn_instruction(data, &accounts); let (title, condensed_fields, expanded_fields) = match parsed { - Ok(parsed) => build_parsed_fields(&parsed, &instruction.program_id.to_string()), - Err(_) => build_fallback_fields(&instruction.program_id.to_string()), + Ok(parsed) => build_parsed_fields(&parsed, &program_id.to_string()), + Err(_) => build_fallback_fields(&program_id.to_string()), }; let condensed = SignablePayloadFieldListLayout { fields: condensed_fields, }; - let expanded_with_raw = - append_raw_data(expanded_fields, &instruction.data, &instruction_data_hex); + let expanded_with_raw = append_raw_data(expanded_fields, data, &instruction_data_hex); let expanded = SignablePayloadFieldListLayout { fields: expanded_with_raw, }; diff --git a/src/chain_parsers/visualsign-solana/src/presets/jupiter_perps/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/jupiter_perps/mod.rs index 193d90a1..65e27b81 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/jupiter_perps/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/jupiter_perps/mod.rs @@ -32,15 +32,14 @@ impl InstructionVisualizer for JupiterPerpsVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + let program_id = context.resolve_program_id()?.to_string(); + let accounts = context.resolve_accounts()?; + let data = context.data(); - let program_id = instruction.program_id.to_string(); - let instruction_data_hex = hex::encode(&instruction.data); + let instruction_data_hex = hex::encode(data); let fallback_text = format!("Program ID: {program_id}\nData: {instruction_data_hex}"); - let parsed = parse_jupiter_perps_instruction(&instruction.data, &instruction.accounts); + let parsed = parse_jupiter_perps_instruction(data, &accounts); let (title, condensed_fields, expanded_fields) = match parsed { Ok(parsed) => build_parsed_fields(&parsed, &program_id)?, @@ -50,8 +49,7 @@ impl InstructionVisualizer for JupiterPerpsVisualizer { let condensed = SignablePayloadFieldListLayout { fields: condensed_fields, }; - let expanded_with_raw = - append_raw_data(expanded_fields, &instruction.data, &instruction_data_hex)?; + let expanded_with_raw = append_raw_data(expanded_fields, data, &instruction_data_hex)?; let expanded = SignablePayloadFieldListLayout { fields: expanded_with_raw, }; diff --git a/src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/mod.rs index 7e13ba70..5aba2c73 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/mod.rs @@ -3,7 +3,8 @@ mod config; use crate::core::{ - InstructionVisualizer, SolanaIntegrationConfig, VisualizerContext, VisualizerKind, + AccountRef, InstructionVisualizer, ProgramRef, SolanaIntegrationConfig, VisualizerContext, + VisualizerKind, }; use crate::utils::{SwapTokenInfo, get_token_info}; use config::JupiterSwapConfig; @@ -84,65 +85,21 @@ impl InstructionVisualizer for JupiterSwapVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; - - let instruction_accounts: Vec = instruction - .accounts - .iter() - .map(|account| account.pubkey.to_string()) + let instruction_accounts: Vec = (0..context.num_accounts()) + .map(|i| match context.account(i) { + Some(AccountRef::Resolved(pk)) => pk.to_string(), + Some(AccountRef::Unresolved { raw_index }) => { + format!("unresolved({raw_index})") + } + None => "unknown".to_string(), + }) .collect(); let jupiter_instruction = - parse_jupiter_swap_instruction(&instruction.data, &instruction_accounts) + parse_jupiter_swap_instruction(context.data(), &instruction_accounts) .map_err(|e| VisualSignError::DecodeError(e.to_string()))?; - let instruction_text = format_jupiter_swap_instruction(&jupiter_instruction); - - let condensed = SignablePayloadFieldListLayout { - fields: vec![ - create_text_field("Instruction", &instruction_text) - .map_err(|e| VisualSignError::ConversionError(e.to_string()))?, - ], - }; - - let expanded = SignablePayloadFieldListLayout { - fields: create_jupiter_swap_expanded_fields( - &jupiter_instruction, - &instruction.program_id.to_string(), - &instruction.data, - )?, - }; - - let preview_layout = SignablePayloadFieldPreviewLayout { - title: Some(SignablePayloadFieldTextV2 { - text: instruction_text.clone(), - }), - subtitle: Some(SignablePayloadFieldTextV2 { - text: String::new(), - }), - condensed: Some(condensed), - expanded: Some(expanded), - }; - - let fallback_text = format!( - "Program ID: {}\nData: {}", - instruction.program_id, - hex::encode(&instruction.data) - ); - - Ok(AnnotatedPayloadField { - static_annotation: None, - dynamic_annotation: None, - signable_payload_field: SignablePayloadField::PreviewLayout { - common: SignablePayloadFieldCommon { - label: format!("Instruction {}", context.instruction_index() + 1), - fallback_text, - }, - preview_layout, - }, - }) + create_jupiter_preview_layout(&jupiter_instruction, context) } fn get_config(&self) -> Option<&dyn SolanaIntegrationConfig> { @@ -485,13 +442,65 @@ fn format_token_symbol(token: &Option) -> String { .unwrap_or_else(|| "Unknown".to_string()) } +fn create_jupiter_preview_layout( + instruction: &JupiterSwapInstruction, + context: &VisualizerContext, +) -> Result { + let program_id_str = match context.program_id() { + ProgramRef::Resolved(pk) => pk.to_string(), + ProgramRef::Unresolved { raw_index } => format!("unresolved({raw_index})"), + }; + let instruction_text = format_jupiter_swap_instruction(instruction); + + let condensed = SignablePayloadFieldListLayout { + fields: vec![ + create_text_field("Instruction", &instruction_text) + .map_err(|e| VisualSignError::ConversionError(e.to_string()))?, + ], + }; + + let expanded = SignablePayloadFieldListLayout { + fields: create_jupiter_swap_expanded_fields(instruction, context)?, + }; + + let preview_layout = SignablePayloadFieldPreviewLayout { + title: Some(SignablePayloadFieldTextV2 { + text: instruction_text.clone(), + }), + subtitle: Some(SignablePayloadFieldTextV2 { + text: String::new(), + }), + condensed: Some(condensed), + expanded: Some(expanded), + }; + + Ok(AnnotatedPayloadField { + static_annotation: None, + dynamic_annotation: None, + signable_payload_field: SignablePayloadField::PreviewLayout { + common: SignablePayloadFieldCommon { + label: format!("Instruction {}", context.instruction_index() + 1), + fallback_text: format!( + "Program ID: {}\nData: {}", + program_id_str, + hex::encode(context.data()) + ), + }, + preview_layout, + }, + }) +} + fn create_jupiter_swap_expanded_fields( instruction: &JupiterSwapInstruction, - program_id: &str, - data: &[u8], + context: &VisualizerContext, ) -> Result, VisualSignError> { + let program_id_str = match context.program_id() { + ProgramRef::Resolved(pk) => pk.to_string(), + ProgramRef::Unresolved { raw_index } => format!("unresolved({raw_index})"), + }; let mut fields = vec![ - create_text_field("Program ID", program_id) + create_text_field("Program ID", &program_id_str) .map_err(|e| VisualSignError::ConversionError(e.to_string()))?, ]; @@ -655,7 +664,7 @@ fn create_jupiter_swap_expanded_fields( // Add raw data field fields.push( - create_raw_data_field(data, Some(hex::encode(data))) + create_raw_data_field(context.data(), None) .map_err(|e| VisualSignError::ConversionError(e.to_string()))?, ); @@ -666,8 +675,50 @@ fn create_jupiter_swap_expanded_fields( #[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] mod tests { use super::*; + use solana_parser::solana::structs::SolanaAccount; + use solana_sdk::instruction::CompiledInstruction; + use solana_sdk::pubkey::Pubkey; mod fixture_test; + /// Test helper: bundles the owned data needed to create a VisualizerContext. + /// Holds ownership so the context can borrow from it. + struct TestContextData { + sender: SolanaAccount, + ci: CompiledInstruction, + account_keys: Vec, + registry: crate::idl::IdlRegistry, + } + + impl TestContextData { + fn new(data: &[u8]) -> Self { + let jup_pk: Pubkey = JUPITER_PROGRAM_ID.parse().unwrap(); + Self { + sender: SolanaAccount { + account_key: Pubkey::new_unique().to_string(), + signer: false, + writable: false, + }, + ci: CompiledInstruction { + program_id_index: 0, + accounts: vec![], + data: data.to_vec(), + }, + account_keys: vec![jup_pk], + registry: crate::idl::IdlRegistry::new(), + } + } + + fn context(&self) -> VisualizerContext<'_> { + VisualizerContext::new( + &self.sender, + &self.ci, + &self.account_keys, + &self.registry, + 0, + ) + } + } + /// Real instruction data from sample_route.json fixture (WSOL -> USELESS swap) fn fixture_instruction_data() -> Vec { hex::decode("e517cb977ae3ad2a010000002f010064000180841e00000000003da9170000000000320000") @@ -710,12 +761,8 @@ mod tests { assert!(formatted.contains("Jupiter"), "Should contain 'Jupiter'"); assert!(formatted.contains("50bps"), "Should contain slippage"); - let fields = create_jupiter_swap_expanded_fields( - &parsed, - "JUP6LkbZbjS1jKKwapdHNy74zcZ3tLUZoi5QNyVTaV4", - &data, - ) - .unwrap(); + let tcd = TestContextData::new(&data); + let fields = create_jupiter_swap_expanded_fields(&parsed, &tcd.context()).unwrap(); let has_program_id = fields.iter().any(|f| { if let SignablePayloadField::TextV2 { common, .. } = &f.signable_payload_field { @@ -749,12 +796,8 @@ mod tests { JupiterSwapInstruction::Route { slippage_bps, .. } => { assert_eq!(slippage_bps, 50); - let fields = create_jupiter_swap_expanded_fields( - &result, - "JUP6LkbZbjS1jKKwapdHNy74zcZ3tLUZoi5QNyVTaV4", - &data, - ) - .unwrap(); + let tcd = TestContextData::new(&data); + let fields = create_jupiter_swap_expanded_fields(&result, &tcd.context()).unwrap(); let fields_json = serde_json::to_value(&fields).unwrap(); assert!( @@ -820,12 +863,8 @@ mod tests { "Should contain platform fee when non-zero" ); - let fields = create_jupiter_swap_expanded_fields( - &instruction, - "JUP6LkbZbjS1jKKwapdHNy74zcZ3tLUZoi5QNyVTaV4", - &[0x01, 0x02, 0x03], - ) - .unwrap(); + let tcd = TestContextData::new(&[0x01, 0x02, 0x03]); + let fields = create_jupiter_swap_expanded_fields(&instruction, &tcd.context()).unwrap(); let has_platform_fee = fields.iter().any(|f| { if let SignablePayloadField::Number { common, .. } = &f.signable_payload_field { @@ -869,12 +908,8 @@ mod tests { let formatted = format_jupiter_swap_instruction(&instruction); assert_eq!(formatted, "Jupiter: Unknown Instruction"); - let fields = create_jupiter_swap_expanded_fields( - &instruction, - "JUP6LkbZbjS1jKKwapdHNy74zcZ3tLUZoi5QNyVTaV4", - &garbage_data, - ) - .unwrap(); + let tcd = TestContextData::new(&garbage_data); + let fields = create_jupiter_swap_expanded_fields(&instruction, &tcd.context()).unwrap(); assert!(fields.len() >= 3, "Should have at least 3 fields"); @@ -924,12 +959,8 @@ mod tests { ); // Test expanded fields show the instruction name - let fields = create_jupiter_swap_expanded_fields( - &instruction, - "JUP6LkbZbjS1jKKwapdHNy74zcZ3tLUZoi5QNyVTaV4", - &[0x01, 0x02, 0x03], // minimal data - ) - .unwrap(); + let tcd = TestContextData::new(&[0x01, 0x02, 0x03]); + let fields = create_jupiter_swap_expanded_fields(&instruction, &tcd.context()).unwrap(); // Check that status field includes the instruction name let status_field = fields.iter().find(|f| { @@ -999,12 +1030,8 @@ mod tests { ); assert!(formatted.contains("50bps"), "Should contain slippage"); - let fields = create_jupiter_swap_expanded_fields( - &parsed, - "JUP6LkbZbjS1jKKwapdHNy74zcZ3tLUZoi5QNyVTaV4", - &data, - ) - .unwrap(); + let tcd = TestContextData::new(&data); + let fields = create_jupiter_swap_expanded_fields(&parsed, &tcd.context()).unwrap(); let has_program_id = fields.iter().any(|f| { if let SignablePayloadField::TextV2 { common, .. } = &f.signable_payload_field { @@ -1067,12 +1094,8 @@ mod tests { ); assert!(formatted.contains("50bps"), "Should contain slippage"); - let fields = create_jupiter_swap_expanded_fields( - &parsed, - "JUP6LkbZbjS1jKKwapdHNy74zcZ3tLUZoi5QNyVTaV4", - &data, - ) - .unwrap(); + let tcd = TestContextData::new(&data); + let fields = create_jupiter_swap_expanded_fields(&parsed, &tcd.context()).unwrap(); let has_program_id = fields.iter().any(|f| { if let SignablePayloadField::TextV2 { common, .. } = &f.signable_payload_field { @@ -1161,12 +1184,8 @@ mod tests { assert!(formatted.contains("Jupiter Swap V2")); assert!(formatted.contains("positive slippage: 25bps")); - let fields = create_jupiter_swap_expanded_fields( - &parsed, - "JUP6LkbZbjS1jKKwapdHNy74zcZ3tLUZoi5QNyVTaV4", - &data, - ) - .unwrap(); + let tcd = TestContextData::new(&data); + let fields = create_jupiter_swap_expanded_fields(&parsed, &tcd.context()).unwrap(); let has_positive_slippage = fields.iter().any(|f| { if let SignablePayloadField::Number { common, .. } = &f.signable_payload_field { diff --git a/src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/tests/fixture_test.rs b/src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/tests/fixture_test.rs index db91231a..06a1dfb7 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/tests/fixture_test.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/tests/fixture_test.rs @@ -97,17 +97,25 @@ fn test_route_real_transaction() { println!(); let instruction = create_instruction_from_fixture(&fixture); - let instructions = vec![instruction.clone()]; - // Create a context - using index 0 since we only loaded the one relevant instruction - // In reality, the fixture.instruction_index would be used with all transaction instructions + // Build account_keys and CompiledInstruction from the resolved Instruction. + let mut account_keys = vec![instruction.program_id]; + for meta in &instruction.accounts { + account_keys.push(meta.pubkey); + } + let compiled = solana_sdk::instruction::CompiledInstruction { + program_id_index: 0, + accounts: (1..=instruction.accounts.len() as u8).collect(), + data: instruction.data.clone(), + }; + let sender = SolanaAccount { account_key: fixture.accounts.first().unwrap().pubkey.clone(), signer: false, writable: false, }; let idl_registry = crate::idl::IdlRegistry::new(); - let context = VisualizerContext::new(&sender, 0, &instructions, &idl_registry); + let context = VisualizerContext::new(&sender, &compiled, &account_keys, &idl_registry, 0); // Visualize let visualizer = super::JupiterSwapVisualizer; diff --git a/src/chain_parsers/visualsign-solana/src/presets/kamino_borrow/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/kamino_borrow/mod.rs index 75bac3ad..3883a12c 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/kamino_borrow/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/kamino_borrow/mod.rs @@ -30,28 +30,24 @@ impl InstructionVisualizer for KaminoBorrowVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + let program_id = context.resolve_program_id()?; + let accounts = context.resolve_accounts()?; + let data = context.data(); - let instruction_data_hex = hex::encode(&instruction.data); - let fallback_text = format!( - "Program ID: {}\nData: {instruction_data_hex}", - instruction.program_id, - ); + let instruction_data_hex = hex::encode(data); + let fallback_text = format!("Program ID: {program_id}\nData: {instruction_data_hex}",); - let parsed = parse_kamino_borrow_instruction(&instruction.data, &instruction.accounts); + let parsed = parse_kamino_borrow_instruction(data, &accounts); let (title, condensed_fields, expanded_fields) = match parsed { - Ok(parsed) => build_parsed_fields(&parsed, &instruction.program_id.to_string()), - Err(_) => build_fallback_fields(&instruction.program_id.to_string()), + Ok(parsed) => build_parsed_fields(&parsed, &program_id.to_string()), + Err(_) => build_fallback_fields(&program_id.to_string()), }; let condensed = SignablePayloadFieldListLayout { fields: condensed_fields, }; - let expanded_with_raw = - append_raw_data(expanded_fields, &instruction.data, &instruction_data_hex); + let expanded_with_raw = append_raw_data(expanded_fields, data, &instruction_data_hex); let expanded = SignablePayloadFieldListLayout { fields: expanded_with_raw, }; diff --git a/src/chain_parsers/visualsign-solana/src/presets/kamino_farms/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/kamino_farms/mod.rs index 92142b37..b1da9a8d 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/kamino_farms/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/kamino_farms/mod.rs @@ -32,35 +32,27 @@ impl InstructionVisualizer for KaminoFarmsVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + let program_id_str = context.resolve_program_id()?.to_string(); + let accounts = context.resolve_accounts()?; + let data = context.data(); - let parsed_result = parse_kamino_farms_instruction(&instruction.data); - let program_id_str = instruction.program_id.to_string(); + let parsed_result = parse_kamino_farms_instruction(data); let (condensed_fields, expanded_fields, title_text) = match &parsed_result { Ok(parsed) => { let named_accounts = match load_kamino_farms_idl() { - Some(idl) => { - build_named_accounts(idl, &instruction.data, &instruction.accounts) - } + Some(idl) => build_named_accounts(idl, data, &accounts), None => BTreeMap::new(), }; ( build_condensed_fields(&parsed.instruction_name)?, - build_parsed_fields( - &program_id_str, - parsed, - &named_accounts, - &instruction.data, - )?, + build_parsed_fields(&program_id_str, parsed, &named_accounts, data)?, format!("{KAMINO_FARMS_DISPLAY_NAME}: {}", parsed.instruction_name), ) } Err(_) => ( build_fallback_condensed_fields()?, - build_fallback_fields(&program_id_str, &instruction.data)?, + build_fallback_fields(&program_id_str, data)?, format!("{KAMINO_FARMS_DISPLAY_NAME}: Unknown Instruction"), ), }; @@ -78,10 +70,7 @@ impl InstructionVisualizer for KaminoFarmsVisualizer { }), }; - let fallback_text = format!( - "Program ID: {program_id_str}\nData: {}", - hex::encode(&instruction.data) - ); + let fallback_text = format!("Program ID: {program_id_str}\nData: {}", hex::encode(data)); Ok(AnnotatedPayloadField { static_annotation: None, @@ -221,7 +210,7 @@ fn append_raw_data( fields: &mut Vec, data: &[u8], ) -> Result<(), VisualSignError> { - fields.push(create_raw_data_field(data, Some(hex::encode(data)))?); + fields.push(create_raw_data_field(data, None)?); Ok(()) } diff --git a/src/chain_parsers/visualsign-solana/src/presets/kamino_limit/config.rs b/src/chain_parsers/visualsign-solana/src/presets/kamino_limit/config.rs index 9c34c2ce..194efb79 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/kamino_limit/config.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/kamino_limit/config.rs @@ -1,6 +1,6 @@ use super::KAMINO_LIMIT_PROGRAM_ID; use crate::core::{SolanaIntegrationConfig, SolanaIntegrationConfigData}; -use std::collections::HashMap; +use std::collections::BTreeMap; pub struct KaminoLimitConfig; @@ -12,8 +12,8 @@ impl SolanaIntegrationConfig for KaminoLimitConfig { fn data(&self) -> &SolanaIntegrationConfigData { static DATA: std::sync::OnceLock = std::sync::OnceLock::new(); DATA.get_or_init(|| { - let mut programs = HashMap::new(); - let mut instructions = HashMap::new(); + let mut programs = BTreeMap::new(); + let mut instructions = BTreeMap::new(); instructions.insert("*", vec!["*"]); programs.insert(KAMINO_LIMIT_PROGRAM_ID, instructions); SolanaIntegrationConfigData { programs } diff --git a/src/chain_parsers/visualsign-solana/src/presets/kamino_limit/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/kamino_limit/mod.rs index 6b37c46c..f23ec5c1 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/kamino_limit/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/kamino_limit/mod.rs @@ -32,35 +32,27 @@ impl InstructionVisualizer for KaminoLimitVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + let program_id_str = context.resolve_program_id()?.to_string(); + let accounts = context.resolve_accounts()?; + let data = context.data(); - let parsed_result = parse_kamino_limit_instruction(&instruction.data); - let program_id_str = instruction.program_id.to_string(); + let parsed_result = parse_kamino_limit_instruction(data); let (condensed_fields, expanded_fields, title_text) = match &parsed_result { Ok(parsed) => { let named_accounts = match load_kamino_limit_idl() { - Some(idl) => { - build_named_accounts(idl, &instruction.data, &instruction.accounts) - } + Some(idl) => build_named_accounts(idl, data, &accounts), None => BTreeMap::new(), }; ( build_condensed_fields(&parsed.instruction_name)?, - build_parsed_fields( - &program_id_str, - parsed, - &named_accounts, - &instruction.data, - )?, + build_parsed_fields(&program_id_str, parsed, &named_accounts, data)?, format!("{KAMINO_LIMIT_DISPLAY_NAME}: {}", parsed.instruction_name), ) } Err(_) => ( build_fallback_condensed_fields()?, - build_fallback_fields(&program_id_str, &instruction.data)?, + build_fallback_fields(&program_id_str, data)?, format!("{KAMINO_LIMIT_DISPLAY_NAME}: Unknown Instruction"), ), }; @@ -78,10 +70,7 @@ impl InstructionVisualizer for KaminoLimitVisualizer { }), }; - let fallback_text = format!( - "Program ID: {program_id_str}\nData: {}", - hex::encode(&instruction.data) - ); + let fallback_text = format!("Program ID: {program_id_str}\nData: {}", hex::encode(data)); Ok(AnnotatedPayloadField { static_annotation: None, diff --git a/src/chain_parsers/visualsign-solana/src/presets/kamino_vault/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/kamino_vault/mod.rs index f0e1a5f2..c025c052 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/kamino_vault/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/kamino_vault/mod.rs @@ -32,35 +32,27 @@ impl InstructionVisualizer for KaminoVaultVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + let program_id_str = context.resolve_program_id()?.to_string(); + let accounts = context.resolve_accounts()?; + let data = context.data(); - let parsed_result = parse_kamino_vault_instruction(&instruction.data); - let program_id_str = instruction.program_id.to_string(); + let parsed_result = parse_kamino_vault_instruction(data); let (condensed_fields, expanded_fields, title_text) = match &parsed_result { Ok(parsed) => { let named_accounts = match load_kamino_vault_idl() { - Some(idl) => { - build_named_accounts(idl, &instruction.data, &instruction.accounts) - } + Some(idl) => build_named_accounts(idl, data, &accounts), None => BTreeMap::new(), }; ( build_condensed_fields(&parsed.instruction_name)?, - build_parsed_fields( - &program_id_str, - parsed, - &named_accounts, - &instruction.data, - )?, + build_parsed_fields(&program_id_str, parsed, &named_accounts, data)?, format!("{KAMINO_VAULT_DISPLAY_NAME}: {}", parsed.instruction_name), ) } Err(_) => ( build_fallback_condensed_fields()?, - build_fallback_fields(&program_id_str, &instruction.data)?, + build_fallback_fields(&program_id_str, data)?, format!("{KAMINO_VAULT_DISPLAY_NAME}: Unknown Instruction"), ), }; @@ -78,10 +70,7 @@ impl InstructionVisualizer for KaminoVaultVisualizer { }), }; - let fallback_text = format!( - "Program ID: {program_id_str}\nData: {}", - hex::encode(&instruction.data) - ); + let fallback_text = format!("Program ID: {program_id_str}\nData: {}", hex::encode(data)); Ok(AnnotatedPayloadField { static_annotation: None, @@ -221,7 +210,7 @@ fn append_raw_data( fields: &mut Vec, data: &[u8], ) -> Result<(), VisualSignError> { - fields.push(create_raw_data_field(data, Some(hex::encode(data)))?); + fields.push(create_raw_data_field(data, None)?); Ok(()) } diff --git a/src/chain_parsers/visualsign-solana/src/presets/metadao_conditional_vault/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/metadao_conditional_vault/mod.rs index f025c3f1..8def48ab 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/metadao_conditional_vault/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/metadao_conditional_vault/mod.rs @@ -33,27 +33,23 @@ impl InstructionVisualizer for MetadaoConditionalVaultVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + let program_id = context.resolve_program_id()?; + let accounts = context.resolve_accounts()?; + let data = context.data(); - if instruction.data.len() < 8 { + if data.len() < 8 { return Err(VisualSignError::DecodeError( "Instruction data too short for Anchor discriminator".to_string(), )); } let idl = load_idl()?; - let parsed = parse_instruction_with_idl( - &instruction.data, - METADAO_CONDITIONAL_VAULT_PROGRAM_ID, - &idl, - ) - .map_err(|e| VisualSignError::DecodeError(format!("IDL parse failed: {e}")))?; + let parsed = parse_instruction_with_idl(data, METADAO_CONDITIONAL_VAULT_PROGRAM_ID, &idl) + .map_err(|e| VisualSignError::DecodeError(format!("IDL parse failed: {e}")))?; - let named_accounts = build_named_accounts(&idl, &instruction.data, &instruction.accounts); + let named_accounts = build_named_accounts(&idl, data, &accounts); - build_visualization(context, instruction, &parsed, &named_accounts) + build_visualization(context, program_id, data, &parsed, &named_accounts) } fn get_config(&self) -> Option<&dyn SolanaIntegrationConfig> { @@ -104,18 +100,19 @@ fn format_arg_value(value: &serde_json::Value) -> String { fn build_visualization( context: &VisualizerContext, - instruction: &solana_sdk::instruction::Instruction, + program_id: solana_sdk::pubkey::Pubkey, + data: &[u8], parsed: &SolanaParsedInstructionData, named_accounts: &BTreeMap, ) -> Result { - let program_id = instruction.program_id.to_string(); + let program_id_str = program_id.to_string(); let title = format!("{DISPLAY_NAME}: {}", parsed.instruction_name); let condensed_fields = vec![create_text_field("Instruction", &parsed.instruction_name)?]; let mut expanded_fields = vec![ create_text_field("Program", DISPLAY_NAME)?, - create_text_field("Program ID", &program_id)?, + create_text_field("Program ID", &program_id_str)?, create_text_field("Instruction", &parsed.instruction_name)?, create_text_field("Discriminator", &parsed.discriminator)?, ]; @@ -132,10 +129,7 @@ fn build_visualization( expanded_fields.push(create_text_field(key, &format_arg_value(value))?); } - expanded_fields.push(create_raw_data_field( - &instruction.data, - Some(hex::encode(&instruction.data)), - )?); + expanded_fields.push(create_raw_data_field(data, None)?); let condensed = SignablePayloadFieldListLayout { fields: condensed_fields, @@ -155,10 +149,7 @@ fn build_visualization( expanded: Some(expanded), }; - let fallback_text = format!( - "Program ID: {program_id}\nData: {}", - hex::encode(&instruction.data) - ); + let fallback_text = format!("Program ID: {program_id_str}\nData: {}", hex::encode(data)); Ok(AnnotatedPayloadField { static_annotation: None, diff --git a/src/chain_parsers/visualsign-solana/src/presets/metadao_futarchy/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/metadao_futarchy/mod.rs index fcc6879b..e1057059 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/metadao_futarchy/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/metadao_futarchy/mod.rs @@ -32,24 +32,23 @@ impl InstructionVisualizer for MetadaoFutarchyVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + let program_id = context.resolve_program_id()?; + let accounts = context.resolve_accounts()?; + let data = context.data(); - if instruction.data.len() < 8 { + if data.len() < 8 { return Err(VisualSignError::DecodeError( "Instruction data too short for Anchor discriminator".to_string(), )); } let idl = load_idl()?; - let parsed = - parse_instruction_with_idl(&instruction.data, METADAO_FUTARCHY_PROGRAM_ID, &idl) - .map_err(|e| VisualSignError::DecodeError(format!("IDL parse failed: {e}")))?; + let parsed = parse_instruction_with_idl(data, METADAO_FUTARCHY_PROGRAM_ID, &idl) + .map_err(|e| VisualSignError::DecodeError(format!("IDL parse failed: {e}")))?; - let named_accounts = build_named_accounts(&idl, &instruction.data, &instruction.accounts); + let named_accounts = build_named_accounts(&idl, data, &accounts); - build_visualization(context, instruction, &parsed, &named_accounts) + build_visualization(context, program_id, data, &parsed, &named_accounts) } fn get_config(&self) -> Option<&dyn SolanaIntegrationConfig> { @@ -100,18 +99,19 @@ fn format_arg_value(value: &serde_json::Value) -> String { fn build_visualization( context: &VisualizerContext, - instruction: &solana_sdk::instruction::Instruction, + program_id: solana_sdk::pubkey::Pubkey, + data: &[u8], parsed: &SolanaParsedInstructionData, named_accounts: &BTreeMap, ) -> Result { - let program_id = instruction.program_id.to_string(); + let program_id_str = program_id.to_string(); let title = format!("{DISPLAY_NAME}: {}", parsed.instruction_name); let condensed_fields = vec![create_text_field("Instruction", &parsed.instruction_name)?]; let mut expanded_fields = vec![ create_text_field("Program", DISPLAY_NAME)?, - create_text_field("Program ID", &program_id)?, + create_text_field("Program ID", &program_id_str)?, create_text_field("Instruction", &parsed.instruction_name)?, create_text_field("Discriminator", &parsed.discriminator)?, ]; @@ -128,10 +128,7 @@ fn build_visualization( expanded_fields.push(create_text_field(key, &format_arg_value(value))?); } - expanded_fields.push(create_raw_data_field( - &instruction.data, - Some(hex::encode(&instruction.data)), - )?); + expanded_fields.push(create_raw_data_field(data, None)?); let condensed = SignablePayloadFieldListLayout { fields: condensed_fields, @@ -151,10 +148,7 @@ fn build_visualization( expanded: Some(expanded), }; - let fallback_text = format!( - "Program ID: {program_id}\nData: {}", - hex::encode(&instruction.data) - ); + let fallback_text = format!("Program ID: {program_id_str}\nData: {}", hex::encode(data)); Ok(AnnotatedPayloadField { static_annotation: None, diff --git a/src/chain_parsers/visualsign-solana/src/presets/meteora_damm_v2/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/meteora_damm_v2/mod.rs index 068457a7..bd76170b 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/meteora_damm_v2/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/meteora_damm_v2/mod.rs @@ -32,17 +32,13 @@ impl InstructionVisualizer for MeteoraDammV2Visualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; - - let data = &instruction.data; - let program_id = instruction.program_id.to_string(); + let program_id = context.resolve_program_id()?.to_string(); + let accounts = context.resolve_accounts()?; + let data = context.data(); let instruction_data_hex = hex::encode(data); - let (parsed, named_accounts) = - parse_meteora_damm_v2_instruction(data, &instruction.accounts) - .map_err(|e| VisualSignError::DecodeError(e.to_string()))?; + let (parsed, named_accounts) = parse_meteora_damm_v2_instruction(data, &accounts) + .map_err(|e| VisualSignError::DecodeError(e.to_string()))?; let instruction_title = format!("{DISPLAY_NAME}: {}", parsed.instruction_name); @@ -65,10 +61,7 @@ impl InstructionVisualizer for MeteoraDammV2Visualizer { for (key, value) in &parsed.program_call_args { expanded_fields.push(create_text_field(key, &format_arg_value(value))?); } - expanded_fields.push(create_raw_data_field( - data, - Some(instruction_data_hex.clone()), - )?); + expanded_fields.push(create_raw_data_field(data, None)?); let preview_layout = SignablePayloadFieldPreviewLayout { title: Some(SignablePayloadFieldTextV2 { diff --git a/src/chain_parsers/visualsign-solana/src/presets/meteora_dlmm/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/meteora_dlmm/mod.rs index 023a369a..8220f4a8 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/meteora_dlmm/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/meteora_dlmm/mod.rs @@ -39,11 +39,10 @@ impl InstructionVisualizer for MeteoraDlmmVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + let program_id = context.resolve_program_id()?; + let accounts = context.resolve_accounts()?; + let data = context.data(); - let data = &instruction.data; if data.len() < 8 { return Err(VisualSignError::DecodeError( "Instruction data shorter than 8-byte discriminator".into(), @@ -56,7 +55,7 @@ impl InstructionVisualizer for MeteoraDlmmVisualizer { let parsed = parse_instruction_with_idl(data, METEORA_DLMM_PROGRAM_ID, idl) .map_err(|e| VisualSignError::DecodeError(e.to_string()))?; - let named_accounts = build_named_accounts(idl, instruction); + let named_accounts = build_named_accounts(idl, data, &accounts); let parsed_instruction = MeteoraDlmmParsedInstruction { parsed, @@ -73,11 +72,7 @@ impl InstructionVisualizer for MeteoraDlmmVisualizer { }; let expanded = SignablePayloadFieldListLayout { - fields: build_expanded_fields( - &parsed_instruction, - &instruction.program_id.to_string(), - data, - )?, + fields: build_expanded_fields(&parsed_instruction, &program_id.to_string(), data)?, }; let preview_layout = SignablePayloadFieldPreviewLayout { @@ -91,11 +86,7 @@ impl InstructionVisualizer for MeteoraDlmmVisualizer { expanded: Some(expanded), }; - let fallback_text = format!( - "Program ID: {}\nData: {}", - instruction.program_id, - hex::encode(data) - ); + let fallback_text = format!("Program ID: {program_id}\nData: {}", hex::encode(data)); Ok(AnnotatedPayloadField { static_annotation: None, @@ -127,9 +118,9 @@ fn get_meteora_dlmm_idl() -> Option<&'static Idl> { fn build_named_accounts( idl: &Idl, - instruction: &solana_sdk::instruction::Instruction, + data: &[u8], + accounts: &[solana_sdk::instruction::AccountMeta], ) -> Vec<(String, String)> { - let data = &instruction.data; if data.len() < 8 { return Vec::new(); } @@ -142,8 +133,7 @@ fn build_named_accounts( return Vec::new(); }; - instruction - .accounts + accounts .iter() .zip(idl_instruction.accounts.iter()) .map(|(meta, idl_account)| (idl_account.name.clone(), meta.pubkey.to_string())) @@ -198,7 +188,7 @@ fn build_expanded_fields( } fields.push( - create_raw_data_field(data, Some(hex::encode(data))) + create_raw_data_field(data, None) .map_err(|e| VisualSignError::ConversionError(e.to_string()))?, ); @@ -292,7 +282,7 @@ mod tests { #[test] fn test_build_named_accounts_pairs_ordered_accounts() { - use solana_sdk::instruction::{AccountMeta, Instruction}; + use solana_sdk::instruction::AccountMeta; use solana_sdk::pubkey::Pubkey; let idl = get_meteora_dlmm_idl().expect("IDL must load"); @@ -306,19 +296,15 @@ mod tests { let program = Pubkey::new_unique(); data.extend([] as [u8; 0]); - let instruction = Instruction { - program_id: METEORA_DLMM_PROGRAM_ID.parse().unwrap(), - accounts: vec![ - AccountMeta::new(position, false), - AccountMeta::new_readonly(sender, true), - AccountMeta::new(rent_receiver, false), - AccountMeta::new_readonly(event_authority, false), - AccountMeta::new_readonly(program, false), - ], - data, - }; + let accounts = vec![ + AccountMeta::new(position, false), + AccountMeta::new_readonly(sender, true), + AccountMeta::new(rent_receiver, false), + AccountMeta::new_readonly(event_authority, false), + AccountMeta::new_readonly(program, false), + ]; - let named = build_named_accounts(idl, &instruction); + let named = build_named_accounts(idl, &data, &accounts); let lookup: BTreeMap<_, _> = named.into_iter().collect(); assert_eq!(lookup.get("position"), Some(&position.to_string())); assert_eq!(lookup.get("sender"), Some(&sender.to_string())); diff --git a/src/chain_parsers/visualsign-solana/src/presets/neutral_trade/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/neutral_trade/mod.rs index d026ee8d..e8b82839 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/neutral_trade/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/neutral_trade/mod.rs @@ -32,26 +32,25 @@ impl InstructionVisualizer for NeutralTradeVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + let program_id_str = context.resolve_program_id()?.to_string(); + let accounts = context.resolve_accounts()?; + let data = context.data(); - if instruction.data.len() < 8 { + if data.len() < 8 { return Err(VisualSignError::DecodeError( "Instruction data too short for Anchor discriminator".into(), )); } let idl = load_idl()?; - let parsed = parse_instruction_with_idl(&instruction.data, NEUTRAL_TRADE_PROGRAM_ID, &idl) - .map_err(|e| { + let parsed = + parse_instruction_with_idl(data, NEUTRAL_TRADE_PROGRAM_ID, &idl).map_err(|e| { VisualSignError::DecodeError(format!("Neutral Trade IDL parse failed: {e}")) })?; - let named_accounts = build_named_accounts(instruction, &idl); + let named_accounts = build_named_accounts(data, &accounts, &idl); - let program_id_str = instruction.program_id.to_string(); - let instruction_data_hex = hex::encode(&instruction.data); + let instruction_data_hex = hex::encode(data); let instruction_title = format!("{NEUTRAL_TRADE_DISPLAY_NAME}: {}", parsed.instruction_name); @@ -59,12 +58,7 @@ impl InstructionVisualizer for NeutralTradeVisualizer { fields: build_condensed_fields(&instruction_title, &parsed)?, }; let expanded = SignablePayloadFieldListLayout { - fields: build_parsed_fields( - &program_id_str, - &parsed, - &named_accounts, - &instruction.data, - )?, + fields: build_parsed_fields(&program_id_str, &parsed, &named_accounts, data)?, }; let preview_layout = SignablePayloadFieldPreviewLayout { @@ -108,21 +102,22 @@ fn load_idl() -> Result { } fn build_named_accounts( - instruction: &solana_sdk::instruction::Instruction, + data: &[u8], + accounts: &[solana_sdk::instruction::AccountMeta], idl: &Idl, ) -> BTreeMap { let mut named_accounts = BTreeMap::new(); let matching_idl_instruction = idl.instructions.iter().find(|inst| { if let Some(disc) = inst.discriminator.as_ref() { - instruction.data.len() >= 8 && &instruction.data[0..8] == disc.as_slice() + data.len() >= 8 && &data[0..8] == disc.as_slice() } else { false } }); if let Some(idl_instruction) = matching_idl_instruction { - for (index, account_meta) in instruction.accounts.iter().enumerate() { + for (index, account_meta) in accounts.iter().enumerate() { if let Some(idl_account) = idl_instruction.accounts.get(index) { named_accounts.insert(idl_account.name.clone(), account_meta.pubkey.to_string()); } @@ -172,7 +167,7 @@ fn append_raw_data( fields: &mut Vec, data: &[u8], ) -> Result<(), VisualSignError> { - fields.push(create_raw_data_field(data, Some(hex::encode(data)))?); + fields.push(create_raw_data_field(data, None)?); Ok(()) } diff --git a/src/chain_parsers/visualsign-solana/src/presets/onre_app/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/onre_app/mod.rs index 6cdbbc58..43ea159a 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/onre_app/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/onre_app/mod.rs @@ -35,15 +35,14 @@ impl InstructionVisualizer for OnreAppVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + let program_id_str = context.resolve_program_id()?.to_string(); + let accounts = context.resolve_accounts()?; + let data = context.data(); let instruction_number = context.instruction_index() + 1; - let program_id_str = instruction.program_id.to_string(); - let instruction_data_hex = hex::encode(&instruction.data); + let instruction_data_hex = hex::encode(data); - let decoded = parse_onre_app_instruction(&instruction.data, &instruction.accounts) + let decoded = parse_onre_app_instruction(data, &accounts) .map_err(|e| VisualSignError::DecodeError(e.to_string()))?; let instruction_name = decoded.parsed.instruction_name.clone(); @@ -81,7 +80,7 @@ impl InstructionVisualizer for OnreAppVisualizer { ); } expanded_fields.push( - create_raw_data_field(&instruction.data, Some(instruction_data_hex.clone())) + create_raw_data_field(data, None) .map_err(|e| VisualSignError::ConversionError(e.to_string()))?, ); diff --git a/src/chain_parsers/visualsign-solana/src/presets/orca_whirlpool/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/orca_whirlpool/mod.rs index 073cab7a..05f67e8a 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/orca_whirlpool/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/orca_whirlpool/mod.rs @@ -31,17 +31,16 @@ impl InstructionVisualizer for OrcaWhirlpoolVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + let program_id = context.resolve_program_id()?; + let resolved_accounts = context.resolve_accounts()?; + let data = context.data(); - let account_keys: Vec = instruction - .accounts + let account_keys: Vec = resolved_accounts .iter() .map(|account| account.pubkey.to_string()) .collect(); - let parsed = parse_orca_whirlpool_instruction(&instruction.data, &account_keys)?; + let parsed = parse_orca_whirlpool_instruction(data, &account_keys)?; let named_accounts = build_named_accounts(&parsed, &account_keys); let title_text = format!("{ORCA_WHIRLPOOL_DISPLAY_NAME}: {}", parsed.instruction_name); @@ -54,12 +53,7 @@ impl InstructionVisualizer for OrcaWhirlpoolVisualizer { }; let expanded = SignablePayloadFieldListLayout { - fields: build_expanded_fields( - &parsed, - &named_accounts, - &instruction.program_id.to_string(), - &instruction.data, - )?, + fields: build_expanded_fields(&parsed, &named_accounts, &program_id.to_string(), data)?, }; let preview_layout = SignablePayloadFieldPreviewLayout { @@ -73,11 +67,7 @@ impl InstructionVisualizer for OrcaWhirlpoolVisualizer { expanded: Some(expanded), }; - let fallback_text = format!( - "Program ID: {}\nData: {}", - instruction.program_id, - hex::encode(&instruction.data) - ); + let fallback_text = format!("Program ID: {program_id}\nData: {}", hex::encode(data)); Ok(AnnotatedPayloadField { static_annotation: None, @@ -182,7 +172,7 @@ fn build_expanded_fields( } fields.push( - create_raw_data_field(data, Some(hex::encode(data))) + create_raw_data_field(data, None) .map_err(|e| VisualSignError::ConversionError(e.to_string()))?, ); diff --git a/src/chain_parsers/visualsign-solana/src/presets/stakepool/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/stakepool/mod.rs index 6a664004..93baeeed 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/stakepool/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/stakepool/mod.rs @@ -3,7 +3,7 @@ mod config; use crate::core::{ - InstructionVisualizer, SolanaIntegrationConfig, VisualizerContext, VisualizerKind, + InstructionVisualizer, ProgramRef, SolanaIntegrationConfig, VisualizerContext, VisualizerKind, }; use config::StakepoolConfig; use spl_stake_pool::instruction::StakePoolInstruction; @@ -21,15 +21,8 @@ impl InstructionVisualizer for StakepoolVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; - - // Try to parse as stakepool instruction - let stakepool_instruction = parse_stake_pool_instruction(&instruction.data)?; - - // Generate proper preview layout - create_stakepool_preview_layout(&stakepool_instruction, instruction, context) + let stakepool_instruction = parse_stake_pool_instruction(context.data())?; + create_stakepool_preview_layout(&stakepool_instruction, context) } fn get_config(&self) -> Option<&dyn SolanaIntegrationConfig> { @@ -43,9 +36,12 @@ impl InstructionVisualizer for StakepoolVisualizer { fn create_stakepool_preview_layout( instruction: &StakePoolInstruction, - solana_instruction: &solana_sdk::instruction::Instruction, context: &VisualizerContext, ) -> Result { + let program_id_str = match context.program_id() { + ProgramRef::Resolved(pk) => pk.to_string(), + ProgramRef::Unresolved { raw_index } => format!("unresolved({raw_index})"), + }; let instruction_name = format_stake_pool_instruction(instruction); let condensed_fields = vec![create_text_field("Instruction", &instruction_name)?]; @@ -81,8 +77,8 @@ fn create_stakepool_preview_layout( label: format!("Instruction {}", context.instruction_index() + 1), fallback_text: format!( "Program ID: {}\nData: {}", - solana_instruction.program_id, - hex::encode(&solana_instruction.data) + program_id_str, + hex::encode(context.data()) ), }, preview_layout, diff --git a/src/chain_parsers/visualsign-solana/src/presets/swig_wallet/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/swig_wallet/mod.rs index e543044b..4a7b0561 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/swig_wallet/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/swig_wallet/mod.rs @@ -5,8 +5,8 @@ mod config; use std::fmt; use crate::core::{ - InstructionVisualizer, SolanaIntegrationConfig, VisualizerContext, VisualizerKind, - available_visualizers, visualize_with_any, + AccountRef, InstructionVisualizer, ProgramRef, SolanaIntegrationConfig, VisualizerContext, + VisualizerKind, available_visualizers, visualize_with_any, }; use config::SwigWalletConfig; use solana_parser::solana::structs::SolanaAccount; @@ -35,19 +35,35 @@ impl InstructionVisualizer for SwigWalletVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + // Build AccountMeta shim for the parser. + // Unresolved accounts are rejected rather than substituted with + // Pubkey::default(), which would render as a valid-looking address. + let accounts: Vec = (0..context.num_accounts()) + .map(|i| match context.account(i) { + Some(AccountRef::Resolved(pk)) => Ok(AccountMeta::new_readonly(*pk, false)), + Some(AccountRef::Unresolved { raw_index }) => Err(VisualSignError::DecodeError( + format!("swig: unresolved account index {raw_index} at position {i}"), + )), + None => Err(VisualSignError::DecodeError(format!( + "swig: missing account at position {i}" + ))), + }) + .collect::, _>>()?; // Convert 0-based index to 1-based instruction number for user-facing labels // (e.g., "Instruction 1" instead of "Instruction 0") let instruction_number = context.instruction_index() + 1; - let decoded = parse_swig_instruction(&instruction.data, &instruction.accounts) + let decoded = parse_swig_instruction(context.data(), &accounts) .map_err(|err| VisualSignError::DecodeError(err.to_string()))?; + let program_id_str = match context.program_id() { + ProgramRef::Resolved(pk) => pk.to_string(), + ProgramRef::Unresolved { raw_index } => format!("unresolved({raw_index})"), + }; + let summary = decoded.summary(); let mut expanded_fields = vec![ - make_text_field("Program ID", instruction.program_id.to_string())?, + make_text_field("Program ID", program_id_str.clone())?, make_text_field("Instruction Type", decoded.name())?, ]; expanded_fields.extend(build_variant_fields(&decoded)?); @@ -72,8 +88,8 @@ impl InstructionVisualizer for SwigWalletVisualizer { let fallback_text = format!( "Program ID: {}\nData: {}", - instruction.program_id, - hex::encode(&instruction.data) + program_id_str, + hex::encode(context.data()) ); Ok(AnnotatedPayloadField { @@ -846,18 +862,29 @@ fn visualize_inner_instruction(instruction: Instruction) -> Option { let visualizer_refs: Vec<&dyn InstructionVisualizer> = visualizers.iter().map(|viz| viz.as_ref()).collect(); + // Build account_keys and a CompiledInstruction from the resolved Instruction. + // The program_id goes at index 0, then each account pubkey follows. + let mut account_keys = vec![instruction.program_id]; + for meta in &instruction.accounts { + account_keys.push(meta.pubkey); + } + let num_accounts = u8::try_from(instruction.accounts.len()).ok()?; + let compiled = solana_sdk::instruction::CompiledInstruction { + program_id_index: 0, + accounts: (1..=num_accounts).collect(), + data: instruction.data.clone(), + }; + let sender = SolanaAccount { - account_key: instruction - .accounts - .first() - .map(|meta| meta.pubkey.to_string()) - .unwrap_or_else(|| instruction.program_id.to_string()), + account_key: account_keys + .get(1) + .map(|pk| pk.to_string()) + .unwrap_or_else(|| account_keys[0].to_string()), signer: false, writable: false, }; - let instructions = vec![instruction]; let idl_registry = crate::idl::IdlRegistry::new(); - let context = VisualizerContext::new(&sender, 0, &instructions, &idl_registry); + let context = VisualizerContext::new(&sender, &compiled, &account_keys, &idl_registry, 0); visualize_with_any(&visualizer_refs, &context) .and_then(|result| result.ok()) @@ -929,9 +956,11 @@ fn summarize_visualized_field(field: &AnnotatedPayloadField) -> Option { Some(address_v2.address.clone()) } } - ListLayout { common, .. } | Divider { common, .. } | Unknown { common, .. } => { - fallback_summary(common) - } + ListLayout { common, .. } => fallback_summary(common), + Divider { common, .. } => fallback_summary(common), + Unknown { common, .. } => fallback_summary(common), + #[cfg(feature = "diagnostics")] + Diagnostic { common, .. } => fallback_summary(common), } } @@ -2300,10 +2329,15 @@ mod tests { "Expected Swig program ID to be present in JSON: {json}" ); + let display_fields: Vec<_> = payload + .fields + .iter() + .filter(|f| f.field_type() != "diagnostic") + .collect(); assert_eq!( - payload.fields.len(), + display_fields.len(), 3, - "Expected three top-level fields (network, instruction, accounts)" + "Expected three display fields (network, instruction, accounts)" ); // Network field @@ -2389,7 +2423,7 @@ mod tests { ); assert_text_field(expanded_fields, "Actions (hex)", "0700000008000000"); - // Accounts field + // Accounts field (diagnostics are appended after accounts) match &payload.fields[2] { SignablePayloadField::PreviewLayout { common, @@ -2433,10 +2467,15 @@ mod tests { "Expected secp256r1 verification program to be represented in JSON: {json}" ); + let display_fields: Vec<_> = payload + .fields + .iter() + .filter(|f| f.field_type() != "diagnostic") + .collect(); assert_eq!( - payload.fields.len(), + display_fields.len(), 5, - "Expected five top-level fields (network + 3 instructions + accounts)" + "Expected five display fields (network + 3 instructions + accounts)" ); // Instruction 1 - Compute budget diff --git a/src/chain_parsers/visualsign-solana/src/presets/system/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/system/mod.rs index 78cddda9..969c9284 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/system/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/system/mod.rs @@ -3,7 +3,8 @@ mod account_labels; mod config; use crate::core::{ - InstructionVisualizer, SolanaIntegrationConfig, VisualizerContext, VisualizerKind, + AccountRef, InstructionVisualizer, ProgramRef, SolanaIntegrationConfig, VisualizerContext, + VisualizerKind, }; use config::SystemConfig; use solana_program::system_instruction::SystemInstruction; @@ -23,18 +24,12 @@ impl InstructionVisualizer for SystemVisualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; - - // Try to parse as system instruction - let system_instruction = bincode::deserialize::(&instruction.data) + let system_instruction = bincode::deserialize::(context.data()) .map_err(|e| { VisualSignError::DecodeError(format!("Failed to parse system instruction: {e}")) })?; - // Generate proper preview layout - create_system_preview_layout(&system_instruction, instruction, context) + create_system_preview_layout(&system_instruction, context) } fn get_config(&self) -> Option<&dyn SolanaIntegrationConfig> { @@ -48,31 +43,24 @@ impl InstructionVisualizer for SystemVisualizer { fn create_system_preview_layout( instruction: &SystemInstruction, - solana_instruction: &solana_sdk::instruction::Instruction, context: &VisualizerContext, ) -> Result { use visualsign::field_builders::*; + let program_id_str = match context.program_id() { + ProgramRef::Resolved(pk) => pk.to_string(), + ProgramRef::Unresolved { raw_index } => format!("unresolved({raw_index})"), + }; + match instruction { SystemInstruction::Transfer { lamports } => { - let _from_key = solana_instruction - .accounts - .first() - .map(|meta| meta.pubkey.to_string()) - .unwrap_or_else(|| "Unknown".to_string()); - let _to_key = solana_instruction - .accounts - .get(1) - .map(|meta| meta.pubkey.to_string()) - .unwrap_or_else(|| "Unknown".to_string()); - let condensed_fields = vec![create_text_field( "Instruction", &format!("Transfer: {lamports} lamports"), )?]; let expanded_fields = vec![ - create_text_field("Program ID", &solana_instruction.program_id.to_string())?, + create_text_field("Program ID", &program_id_str)?, AnnotatedPayloadField { static_annotation: None, dynamic_annotation: None, @@ -87,7 +75,7 @@ fn create_system_preview_layout( }, }, }, - create_text_field("Raw Data", &hex::encode(&solana_instruction.data))?, + create_text_field("Raw Data", &hex::encode(context.data()))?, ]; let condensed = visualsign::SignablePayloadFieldListLayout { @@ -116,8 +104,8 @@ fn create_system_preview_layout( label: format!("Instruction {}", context.instruction_index() + 1), fallback_text: format!( "Program ID: {}\nData: {}", - solana_instruction.program_id, - hex::encode(&solana_instruction.data) + program_id_str, + hex::encode(context.data()) ), }, preview_layout, @@ -129,16 +117,16 @@ fn create_system_preview_layout( space, owner, } => { - let new_account = solana_instruction - .accounts - .get(1) - .map(|meta| meta.pubkey.to_string()) - .unwrap_or_else(|| "Unknown".to_string()); - let payer = solana_instruction - .accounts - .first() - .map(|meta| meta.pubkey.to_string()) - .unwrap_or_else(|| "Unknown".to_string()); + let new_account = match context.account(1) { + Some(AccountRef::Resolved(pk)) => pk.to_string(), + Some(AccountRef::Unresolved { raw_index }) => format!("unresolved({raw_index})"), + None => "unknown".to_string(), + }; + let payer = match context.account(0) { + Some(AccountRef::Resolved(pk)) => pk.to_string(), + Some(AccountRef::Unresolved { raw_index }) => format!("unresolved({raw_index})"), + None => "unknown".to_string(), + }; let condensed_fields = vec![ create_text_field("Action", "Create Account")?, @@ -189,8 +177,8 @@ fn create_system_preview_layout( label: format!("Instruction {}", context.instruction_index() + 1), fallback_text: format!( "Program ID: {}\nData: {}", - solana_instruction.program_id, - hex::encode(&solana_instruction.data) + program_id_str, + hex::encode(context.data()) ), }, preview_layout, @@ -198,7 +186,6 @@ fn create_system_preview_layout( }) } _ => { - // Handle other system instructions with basic layout let instruction_name = account_labels::system_instruction_label(instruction); let condensed_fields = vec![ @@ -238,8 +225,8 @@ fn create_system_preview_layout( label: format!("Instruction {}", context.instruction_index() + 1), fallback_text: format!( "Program ID: {}\nData: {}", - solana_instruction.program_id, - hex::encode(&solana_instruction.data) + program_id_str, + hex::encode(context.data()) ), }, preview_layout, diff --git a/src/chain_parsers/visualsign-solana/src/presets/token_2022/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/token_2022/mod.rs index 49bff823..dd8f7755 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/token_2022/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/token_2022/mod.rs @@ -3,7 +3,8 @@ mod config; use crate::core::{ - InstructionVisualizer, SolanaIntegrationConfig, VisualizerContext, VisualizerKind, + AccountRef, InstructionVisualizer, ProgramRef, SolanaIntegrationConfig, VisualizerContext, + VisualizerKind, }; use crate::utils::format_token_amount; use config::Token2022Config; @@ -18,6 +19,13 @@ use visualsign::{ static TOKEN_2022_CONFIG: Token2022Config = Token2022Config; +fn resolve_program_id(context: &VisualizerContext) -> String { + match context.program_id() { + ProgramRef::Resolved(pk) => pk.to_string(), + ProgramRef::Unresolved { raw_index } => format!("unresolved({raw_index})"), + } +} + // Token 2022 extension instruction discriminators const PAUSABLE_EXTENSION_DISCRIMINATOR: u8 = 44; const PAUSABLE_PAUSE_DISCRIMINATOR: u8 = 1; @@ -33,17 +41,27 @@ impl InstructionVisualizer for Token2022Visualizer { &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + // Build AccountMeta shim for the parser (which expects &[AccountMeta]). + // Unresolved accounts are rejected rather than substituted with + // Pubkey::default(), which would render as a valid-looking address. + let accounts: Vec = (0..context.num_accounts()) + .map(|i| match context.account(i) { + Some(AccountRef::Resolved(pk)) => Ok(AccountMeta::new_readonly(*pk, false)), + Some(AccountRef::Unresolved { raw_index }) => Err(VisualSignError::DecodeError( + format!("token_2022: unresolved account index {raw_index} at position {i}"), + )), + None => Err(VisualSignError::DecodeError(format!( + "token_2022: missing account at position {i}" + ))), + }) + .collect::, _>>()?; // Parse the Token 2022 instruction - let token_2022_instruction = - parse_token_2022_instruction(&instruction.data, &instruction.accounts) - .map_err(|e| VisualSignError::DecodeError(e.to_string()))?; + let token_2022_instruction = parse_token_2022_instruction(context.data(), &accounts) + .map_err(|e| VisualSignError::DecodeError(e.to_string()))?; // Generate proper preview layout - create_token_2022_preview_layout(&token_2022_instruction, instruction, context) + create_token_2022_preview_layout(&token_2022_instruction, context) } fn get_config(&self) -> Option<&dyn SolanaIntegrationConfig> { @@ -309,7 +327,6 @@ fn get_authority_type_name(authority_type: u8) -> String { fn create_token_2022_preview_layout( parsed: &Token2022Instruction, - instruction: &solana_sdk::instruction::Instruction, context: &VisualizerContext, ) -> Result { let (title, condensed_fields, expanded_fields) = match parsed { @@ -336,8 +353,8 @@ fn create_token_2022_preview_layout( create_text_field("Mint", mint)?, create_text_field("Destination Account", account)?, create_text_field("Mint Authority", mint_authority)?, - create_text_field("Program ID", &instruction.program_id.to_string())?, - create_raw_data_field(&instruction.data, Some(hex::encode(&instruction.data)))?, + create_text_field("Program ID", &resolve_program_id(context))?, + create_raw_data_field(context.data(), None)?, ]; (title, condensed, expanded) @@ -365,8 +382,8 @@ fn create_token_2022_preview_layout( create_text_field("Token Account", account)?, create_text_field("Mint", mint)?, create_text_field("Authority", authority)?, - create_text_field("Program ID", &instruction.program_id.to_string())?, - create_raw_data_field(&instruction.data, Some(hex::encode(&instruction.data)))?, + create_text_field("Program ID", &resolve_program_id(context))?, + create_raw_data_field(context.data(), None)?, ]; (title, condensed, expanded) @@ -383,8 +400,8 @@ fn create_token_2022_preview_layout( create_text_field("Instruction", "Pause")?, create_text_field("Mint", mint)?, create_text_field("Pause Authority", pause_authority)?, - create_text_field("Program ID", &instruction.program_id.to_string())?, - create_raw_data_field(&instruction.data, Some(hex::encode(&instruction.data)))?, + create_text_field("Program ID", &resolve_program_id(context))?, + create_raw_data_field(context.data(), None)?, ]; (title, condensed, expanded) @@ -401,8 +418,8 @@ fn create_token_2022_preview_layout( create_text_field("Instruction", "Resume")?, create_text_field("Mint", mint)?, create_text_field("Pause Authority", pause_authority)?, - create_text_field("Program ID", &instruction.program_id.to_string())?, - create_raw_data_field(&instruction.data, Some(hex::encode(&instruction.data)))?, + create_text_field("Program ID", &resolve_program_id(context))?, + create_raw_data_field(context.data(), None)?, ]; (title, condensed, expanded) @@ -432,8 +449,8 @@ fn create_token_2022_preview_layout( create_number_field("Authority Type ID", &authority_type.to_string(), "")?, create_text_field("Current Authority", current_authority)?, create_text_field("New Authority", &new_authority_display)?, - create_text_field("Program ID", &instruction.program_id.to_string())?, - create_raw_data_field(&instruction.data, Some(hex::encode(&instruction.data)))?, + create_text_field("Program ID", &resolve_program_id(context))?, + create_raw_data_field(context.data(), None)?, ]; (title, condensed, expanded) @@ -452,8 +469,8 @@ fn create_token_2022_preview_layout( create_text_field("Token Account", account)?, create_text_field("Mint", mint)?, create_text_field("Freeze Authority", freeze_authority)?, - create_text_field("Program ID", &instruction.program_id.to_string())?, - create_raw_data_field(&instruction.data, Some(hex::encode(&instruction.data)))?, + create_text_field("Program ID", &resolve_program_id(context))?, + create_raw_data_field(context.data(), None)?, ]; (title, condensed, expanded) @@ -472,8 +489,8 @@ fn create_token_2022_preview_layout( create_text_field("Token Account", account)?, create_text_field("Mint", mint)?, create_text_field("Freeze Authority", freeze_authority)?, - create_text_field("Program ID", &instruction.program_id.to_string())?, - create_raw_data_field(&instruction.data, Some(hex::encode(&instruction.data)))?, + create_text_field("Program ID", &resolve_program_id(context))?, + create_raw_data_field(context.data(), None)?, ]; (title, condensed, expanded) @@ -492,8 +509,8 @@ fn create_token_2022_preview_layout( create_text_field("Token Account", account)?, create_text_field("Destination", destination)?, create_text_field("Owner", owner)?, - create_text_field("Program ID", &instruction.program_id.to_string())?, - create_raw_data_field(&instruction.data, Some(hex::encode(&instruction.data)))?, + create_text_field("Program ID", &resolve_program_id(context))?, + create_raw_data_field(context.data(), None)?, ]; (title, condensed, expanded) @@ -524,10 +541,10 @@ fn create_token_2022_preview_layout( let instruction_num = context.instruction_index() + 1; format!("Instruction {instruction_num}") }, - fallback_text: { - let program_id = instruction.program_id; - format!("Token 2022: {title}\nProgram ID: {program_id}") - }, + fallback_text: format!( + "Token 2022: {title}\nProgram ID: {}", + resolve_program_id(context) + ), }, preview_layout, }, diff --git a/src/chain_parsers/visualsign-solana/src/presets/token_2022/tests/fixture_test.rs b/src/chain_parsers/visualsign-solana/src/presets/token_2022/tests/fixture_test.rs index 557633a9..7b0b65dd 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/token_2022/tests/fixture_test.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/token_2022/tests/fixture_test.rs @@ -104,17 +104,26 @@ fn test_real_transaction(fixture_name: &str, test_name: &str) { println!(); let instruction = create_instruction_from_fixture(&fixture); - let instructions = vec![instruction.clone()]; - // Create a context - using index 0 since we only loaded the one relevant instruction - // In reality, the fixture.instruction_index would be used with all transaction instructions + // Build account_keys and CompiledInstruction from the resolved Instruction. + // program_id goes at index 0, then each account pubkey follows. + let mut account_keys = vec![instruction.program_id]; + for meta in &instruction.accounts { + account_keys.push(meta.pubkey); + } + let compiled = solana_sdk::instruction::CompiledInstruction { + program_id_index: 0, + accounts: (1..=instruction.accounts.len() as u8).collect(), + data: instruction.data.clone(), + }; + let sender = SolanaAccount { account_key: fixture.accounts.first().unwrap().pubkey.clone(), signer: false, writable: false, }; let idl_registry = crate::idl::IdlRegistry::new(); - let context = VisualizerContext::new(&sender, 0, &instructions, &idl_registry); + let context = VisualizerContext::new(&sender, &compiled, &account_keys, &idl_registry, 0); // Visualize let visualizer = Token2022Visualizer; diff --git a/src/chain_parsers/visualsign-solana/src/presets/unknown_program/config.rs b/src/chain_parsers/visualsign-solana/src/presets/unknown_program/config.rs index 327176d3..c386dc2d 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/unknown_program/config.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/unknown_program/config.rs @@ -22,11 +22,7 @@ impl SolanaIntegrationConfig for UnknownProgramConfig { } // Override can_handle to always return true - this is a catch-all fallback - fn can_handle( - &self, - _program_id: &str, - _instruction: &solana_sdk::instruction::Instruction, - ) -> bool { + fn can_handle(&self, _program_id: &str) -> bool { true } } diff --git a/src/chain_parsers/visualsign-solana/src/presets/unknown_program/mod.rs b/src/chain_parsers/visualsign-solana/src/presets/unknown_program/mod.rs index 7270cac1..8bd1b097 100644 --- a/src/chain_parsers/visualsign-solana/src/presets/unknown_program/mod.rs +++ b/src/chain_parsers/visualsign-solana/src/presets/unknown_program/mod.rs @@ -4,7 +4,8 @@ mod config; use crate::core::{ - InstructionVisualizer, SolanaIntegrationConfig, VisualizerContext, VisualizerKind, + AccountRef, InstructionVisualizer, ProgramRef, SolanaIntegrationConfig, VisualizerContext, + VisualizerKind, }; use config::UnknownProgramConfig; use solana_parser::{SolanaParsedInstructionData, parse_instruction_with_idl}; @@ -21,25 +22,26 @@ static UNKNOWN_PROGRAM_CONFIG: UnknownProgramConfig = UnknownProgramConfig; pub struct UnknownProgramVisualizer; impl InstructionVisualizer for UnknownProgramVisualizer { + fn can_handle(&self, _context: &VisualizerContext) -> bool { + true // catch-all: handles everything including unresolved program_ids + } + fn visualize_tx_commands( &self, context: &VisualizerContext, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; - let idl_registry = context.idl_registry(); - // Try IDL-based parsing if available for this program - if idl_registry.has_idl(&instruction.program_id) { - if let Ok(field) = try_idl_parsing(context, idl_registry) { - return Ok(field); + // Try IDL parsing only if program_id is resolvable + if let ProgramRef::Resolved(program_id) = context.program_id() { + if idl_registry.has_idl(program_id) { + if let Ok(field) = try_idl_parsing(context, idl_registry) { + return Ok(field); + } } - // IDL parsing failed, fall through to default visualization } - create_unknown_program_preview_layout(instruction, context) + create_unknown_program_preview_layout(context) } fn get_config(&self) -> Option<&dyn SolanaIntegrationConfig> { @@ -51,16 +53,35 @@ impl InstructionVisualizer for UnknownProgramVisualizer { } } +fn resolve_program_id_str(context: &VisualizerContext) -> String { + match context.program_id() { + ProgramRef::Resolved(pk) => pk.to_string(), + ProgramRef::Unresolved { raw_index } => format!("unresolved({raw_index})"), + } +} + +fn resolve_account_str(context: &VisualizerContext, position: usize) -> String { + match context.account(position) { + Some(AccountRef::Resolved(pk)) => pk.to_string(), + Some(AccountRef::Unresolved { raw_index }) => format!("unresolved({raw_index})"), + None => "unknown".to_string(), + } +} + /// Attempt to parse instruction using IDL from solana_parser fn try_idl_parsing( context: &VisualizerContext, idl_registry: &crate::idl::IdlRegistry, ) -> Result { - let instruction = context - .current_instruction() - .ok_or_else(|| VisualSignError::MissingData("No instruction found".into()))?; + let program_id = match context.program_id() { + ProgramRef::Resolved(pk) => pk, + ProgramRef::Unresolved { .. } => { + return Err(VisualSignError::MissingData( + "No program_id resolved".into(), + )); + } + }; - let program_id = &instruction.program_id; let program_name = idl_registry.get_program_name(program_id); let idl_name = idl_registry.get_idl_name(program_id); @@ -68,8 +89,8 @@ fn try_idl_parsing( // `parsed_result` carries the upstream parsed payload alongside a locally-built // `BTreeMap` of named accounts so iteration order at render time is deterministic // (the upstream `SolanaParsedInstructionData.named_accounts` is a `HashMap`). - let parsed_result = try_parse_with_idl(instruction, idl_registry); - let instruction_data_hex = hex::encode(&instruction.data); + let parsed_result = try_parse_with_idl(context, idl_registry); + let instruction_data_hex = hex::encode(context.data()); // Format program display as "UserName (name: idl_name)" if IDL name exists let program_display = if let Some(idl_name) = &idl_name { @@ -254,20 +275,19 @@ fn try_idl_parsing( } fn create_unknown_program_preview_layout( - instruction: &solana_sdk::instruction::Instruction, context: &VisualizerContext, ) -> Result { use visualsign::field_builders::*; - let program_id = instruction.program_id.to_string(); - let instruction_data_hex = hex::encode(&instruction.data); + let program_id_str = resolve_program_id_str(context); + let instruction_data_hex = hex::encode(context.data()); // Condensed view - just the essentials - let condensed_fields = vec![create_text_field("Program", &program_id)?]; + let condensed_fields = vec![create_text_field("Program", &program_id_str)?]; // Expanded view - adds instruction data let expanded_fields = vec![ - create_text_field("Program ID", &program_id)?, + create_text_field("Program ID", &program_id_str)?, create_text_field("Instruction Data", &instruction_data_hex)?, ]; @@ -280,7 +300,7 @@ fn create_unknown_program_preview_layout( let preview_layout = SignablePayloadFieldPreviewLayout { title: Some(visualsign::SignablePayloadFieldTextV2 { - text: program_id.clone(), + text: program_id_str.clone(), }), subtitle: Some(visualsign::SignablePayloadFieldTextV2 { text: String::new(), @@ -295,25 +315,31 @@ fn create_unknown_program_preview_layout( signable_payload_field: SignablePayloadField::PreviewLayout { common: SignablePayloadFieldCommon { label: format!("Instruction {}", context.instruction_index() + 1), - fallback_text: format!("Program ID: {program_id}\nData: {instruction_data_hex}"), + fallback_text: format!( + "Program ID: {program_id_str}\nData: {instruction_data_hex}" + ), }, preview_layout, }, }) } -/// Try to parse instruction using the new parse_instruction_with_idl function. +/// Try to parse instruction using the parse_instruction_with_idl function. /// /// Returns both the upstream parsed payload and a locally-built `BTreeMap` of named /// accounts. We deliberately do NOT write the named accounts back into /// `parsed.named_accounts` (a `HashMap` at the FFI boundary): the rendering path needs /// stable iteration order, so callers should iterate the returned `BTreeMap` instead. fn try_parse_with_idl( - instruction: &solana_sdk::instruction::Instruction, + context: &VisualizerContext, idl_registry: &crate::idl::IdlRegistry, ) -> Result<(SolanaParsedInstructionData, BTreeMap), Box> { - let program_id_str = instruction.program_id.to_string(); - let instruction_data = &instruction.data; + let program_id = match context.program_id() { + ProgramRef::Resolved(pk) => pk, + ProgramRef::Unresolved { .. } => return Err("No program_id resolved".into()), + }; + let program_id_str = program_id.to_string(); + let instruction_data = context.data(); // Try to get the IDL for this program let idl = idl_registry @@ -336,9 +362,12 @@ fn try_parse_with_idl( } }) { // Match each account in the instruction with its name from the IDL - for (index, account_meta) in instruction.accounts.iter().enumerate() { + for index in 0..context.num_accounts() { if let Some(idl_account) = idl_instruction.accounts.get(index) { - named_accounts.insert(idl_account.name.clone(), account_meta.pubkey.to_string()); + named_accounts.insert( + idl_account.name.clone(), + resolve_account_str(context, index), + ); } } } diff --git a/src/chain_parsers/visualsign-solana/tests/common/mod.rs b/src/chain_parsers/visualsign-solana/tests/common/mod.rs index 81c55c9b..5bd2b952 100644 --- a/src/chain_parsers/visualsign-solana/tests/common/mod.rs +++ b/src/chain_parsers/visualsign-solana/tests/common/mod.rs @@ -116,7 +116,10 @@ pub fn options_no_idl() -> VisualSignOptions { // ── Field inspection helpers ────────────────────────────────────────────────── /// Returns the PreviewLayout for every instruction field in the payload. +/// Instruction fields are PreviewLayouts that are not "Network", "Accounts", +/// "Address Lookup Tables", or diagnostic fields. pub fn instruction_fields(payload: &SignablePayload) -> Vec<&SignablePayloadFieldPreviewLayout> { + let non_instruction_labels = ["Network", "Accounts", "Address Lookup Tables"]; payload .fields .iter() @@ -126,7 +129,7 @@ pub fn instruction_fields(payload: &SignablePayload) -> Vec<&SignablePayloadFiel preview_layout, } = f { - if common.label.starts_with("Instruction") { + if !non_instruction_labels.contains(&common.label.as_str()) { return Some(preview_layout); } } diff --git a/src/chain_parsers/visualsign-solana/tests/pipeline_integration.proptest-regressions b/src/chain_parsers/visualsign-solana/tests/pipeline_integration.proptest-regressions new file mode 100644 index 00000000..58d309c5 --- /dev/null +++ b/src/chain_parsers/visualsign-solana/tests/pipeline_integration.proptest-regressions @@ -0,0 +1,7 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc 548704a54ac8e05a9cb84af82765aec8e457c88bedae8bb4dad793dce735ec71 # shrinks to idl_json = "{\"instructions\":[{\"name\":\"a\",\"discriminator\":[46,144,62,75,254,227,21,61],\"accounts\":[],\"args\":[]}]}", use_valid_disc = false, inst_idx = 0, data = [] diff --git a/src/integration/tests/parser.rs b/src/integration/tests/parser.rs index a51dd31b..eb46b981 100644 --- a/src/integration/tests/parser.rs +++ b/src/integration/tests/parser.rs @@ -373,7 +373,17 @@ async fn parser_solana_native_transfer_e2e() { tracing::debug!("📄 Emitted JSON for visual inspection:"); tracing::debug!("{}", json_str); - // Validate that the parsed transaction contains all expected fields + let diag_fields: Vec<_> = signable_payload["Fields"] + .as_array() + .unwrap() + .iter() + .filter(|f| f.get("Type").and_then(|t| t.as_str()) == Some("diagnostic")) + .collect(); + assert!( + diag_fields.is_empty(), + "parser_app must not emit diagnostic fields; got {diag_fields:?}" + ); + validate_required_fields_present(&signable_payload, &expected_sp); } diff --git a/src/parser/cli/Cargo.toml b/src/parser/cli/Cargo.toml index 60256b2f..8001501a 100644 --- a/src/parser/cli/Cargo.toml +++ b/src/parser/cli/Cargo.toml @@ -5,9 +5,10 @@ edition = "2024" publish = false [features] -default = ["solana", "ethereum"] +default = ["solana", "ethereum", "diagnostics"] solana = ["dep:visualsign-solana"] ethereum = ["dep:visualsign-ethereum"] +diagnostics = ["visualsign/diagnostics", "visualsign-solana?/diagnostics"] [dependencies] tracing = { workspace = true } diff --git a/src/parser/cli/tests/cli_test.rs b/src/parser/cli/tests/cli_test.rs index 8a4270af..5f9f1bcc 100644 --- a/src/parser/cli/tests/cli_test.rs +++ b/src/parser/cli/tests/cli_test.rs @@ -105,35 +105,173 @@ fn test_cli_with_fixtures() { let output = command .output() .unwrap_or_else(|e| panic!("Failed to execute CLI: {e}")); - println!("Output {test_name:?}: {output:?}"); - - // Construct expected output path - let expected_path = fixtures_dir.join(format!("{test_name}.expected")); - - // Read expected output - let expected_output = fs::read_to_string(&expected_path) - .unwrap_or_else(|_| panic!("Expected output file not found: {expected_path:?}")); let actual_output = String::from_utf8(output.stdout) .unwrap_or_else(|e| panic!("Invalid UTF-8 output: {e}")); - let expected = expected_output.trim(); - let actual = actual_output.trim(); + // Display fixture: compare non-diagnostic fields + let display_path = fixtures_dir.join(format!("{test_name}.display.expected")); + assert!( + display_path.exists(), + "Display fixture not found: {display_path:?}" + ); + + let expected_display = fs::read_to_string(&display_path) + .unwrap_or_else(|_| panic!("Display fixture not found: {display_path:?}")); + + // Try JSON parsing; fall back to string comparison for text/human output + match serde_json::from_str::(actual_output.trim()) { + Ok(actual_json) => { + // JSON output: filter diagnostics and check membership + #[cfg_attr(not(feature = "diagnostics"), allow(unused_mut))] + let mut display_payload = actual_json.clone(); + #[cfg(feature = "diagnostics")] + if let Some(fields) = display_payload + .get_mut("Fields") + .and_then(|f| f.as_array_mut()) + { + fields.retain(|f| f.get("Type").and_then(|t| t.as_str()) != Some("diagnostic")); + } + + let expected_json: serde_json::Value = + serde_json::from_str(expected_display.trim()).unwrap_or_else(|e| { + panic!("Failed to parse display fixture as JSON for '{test_name}': {e}") + }); + + assert_json_contains(test_name, &expected_json, &display_payload, ""); + + // Diagnostics fixture: compare rule, level, and instruction_index + #[cfg(feature = "diagnostics")] + let diagnostics_path = + fixtures_dir.join(format!("{test_name}.diagnostics.expected")); + #[cfg(feature = "diagnostics")] + if diagnostics_path.exists() { + let expected_diags: Vec = serde_json::from_str( + &fs::read_to_string(&diagnostics_path) + .unwrap_or_else(|_| panic!("Failed to read: {diagnostics_path:?}")), + ) + .unwrap_or_else(|e| panic!("Failed to parse diagnostics fixture: {e}")); + + let actual_diags: Vec<(String, String, Option)> = actual_json + .get("Fields") + .and_then(|f| f.as_array()) + .map(|fields| { + fields + .iter() + .filter(|f| { + f.get("Type").and_then(|t| t.as_str()) == Some("diagnostic") + }) + .map(|f| { + let diag = &f["Diagnostic"]; + ( + diag["Rule"].as_str().unwrap().to_string(), + diag["Level"].as_str().unwrap().to_string(), + diag["InstructionIndex"].as_u64().map(|n| n as u32), + ) + }) + .collect() + }) + .unwrap_or_default(); + + // Every expected diagnostic must be present (rule + level + optional instruction_index) + for expected in &expected_diags { + let rule = expected["rule"].as_str().unwrap(); + let level = expected["level"].as_str().unwrap(); + let expected_idx = expected + .get("instruction_index") + .and_then(|v| v.as_u64()) + .map(|n| n as u32); + assert!( + actual_diags.iter().any(|(r, l, idx)| { + r == rule + && l == level + && (expected_idx.is_none() || *idx == expected_idx) + }), + "Test '{test_name}': missing diagnostic rule={rule}, level={level}, instruction_index={expected_idx:?}" + ); + } + + // No unexpected diagnostics + assert_eq!( + expected_diags.len(), + actual_diags.len(), + "Test '{test_name}': expected {} diagnostics, got {}. Actual: {:?}", + expected_diags.len(), + actual_diags.len(), + actual_diags + ); + } + } + Err(_) => { + // Non-JSON output (text/human): plain string comparison + assert_strings_match( + test_name, + "display", + expected_display.trim(), + actual_output.trim(), + ); + } + } + } +} + +fn assert_strings_match(test_name: &str, fixture_type: &str, expected: &str, actual: &str) { + if expected != actual { + let diff = TextDiff::from_lines(expected, actual); + let mut diff_output = String::new(); + + for change in diff.iter_all_changes() { + let sign = match change.tag() { + ChangeTag::Delete => "-", + ChangeTag::Insert => "+", + ChangeTag::Equal => " ", + }; + diff_output.push_str(&format!("{sign}{change}")); + } - if expected != actual { - let diff = TextDiff::from_lines(expected, actual); - let mut diff_output = String::new(); + panic!("Test case '{test_name}' ({fixture_type}) failed:\n{diff_output}"); + } +} - for change in diff.iter_all_changes() { - let sign = match change.tag() { - ChangeTag::Delete => "-", - ChangeTag::Insert => "+", - ChangeTag::Equal => " ", +/// Recursively checks that every field in `expected` is present in `actual`. +/// Objects: every key in expected must exist in actual with a matching value. +/// Arrays: must have the same length and each element must match. +/// Scalars: must be equal. +fn assert_json_contains( + test_name: &str, + expected: &serde_json::Value, + actual: &serde_json::Value, + path: &str, +) { + match (expected, actual) { + (serde_json::Value::Object(exp_map), serde_json::Value::Object(act_map)) => { + for (key, exp_val) in exp_map { + let field_path = if path.is_empty() { + key.clone() + } else { + format!("{path}.{key}") }; - diff_output.push_str(&format!("{sign}{change}")); + let act_val = act_map.get(key).unwrap_or_else(|| { + panic!("Test '{test_name}': missing key '{field_path}' in actual output") + }); + assert_json_contains(test_name, exp_val, act_val, &field_path); } - - panic!("Test case '{test_name}' failed:\n{diff_output}"); + } + (serde_json::Value::Array(exp_arr), serde_json::Value::Array(act_arr)) => { + assert_eq!( + exp_arr.len(), + act_arr.len(), + "Test '{test_name}': array length mismatch at '{path}'" + ); + for (i, (exp_val, act_val)) in exp_arr.iter().zip(act_arr.iter()).enumerate() { + assert_json_contains(test_name, exp_val, act_val, &format!("{path}[{i}]")); + } + } + _ => { + assert_eq!( + expected, actual, + "Test '{test_name}': value mismatch at '{path}'" + ); } } } @@ -409,5 +547,5 @@ fn test_cli_solana_idl_invalid_file_still_parses() { let json: serde_json::Value = serde_json::from_str(&output).expect("CLI output should be valid JSON"); - assert_eq!(json["Title"], "Solana Transaction"); + assert_eq!(json["Title"], "Solana Transaction") } diff --git a/src/parser/cli/tests/fixtures/ethereum-from-file.expected b/src/parser/cli/tests/fixtures/ethereum-from-file.display.expected similarity index 100% rename from src/parser/cli/tests/fixtures/ethereum-from-file.expected rename to src/parser/cli/tests/fixtures/ethereum-from-file.display.expected diff --git a/src/parser/cli/tests/fixtures/ethereum-json.expected b/src/parser/cli/tests/fixtures/ethereum-json.display.expected similarity index 100% rename from src/parser/cli/tests/fixtures/ethereum-json.expected rename to src/parser/cli/tests/fixtures/ethereum-json.display.expected diff --git a/src/parser/cli/tests/fixtures/solana-json.diagnostics.expected b/src/parser/cli/tests/fixtures/solana-json.diagnostics.expected new file mode 100644 index 00000000..27014375 --- /dev/null +++ b/src/parser/cli/tests/fixtures/solana-json.diagnostics.expected @@ -0,0 +1,4 @@ +[ + { "rule": "transaction::oob_program_id", "level": "ok" }, + { "rule": "transaction::oob_account_index", "level": "ok" } +] diff --git a/src/parser/cli/tests/fixtures/solana-json.expected b/src/parser/cli/tests/fixtures/solana-json.display.expected similarity index 100% rename from src/parser/cli/tests/fixtures/solana-json.expected rename to src/parser/cli/tests/fixtures/solana-json.display.expected diff --git a/src/parser/cli/tests/fixtures/solana-text.expected b/src/parser/cli/tests/fixtures/solana-text.display.expected similarity index 97% rename from src/parser/cli/tests/fixtures/solana-text.expected rename to src/parser/cli/tests/fixtures/solana-text.display.expected index 571707dd..35e7a7dd 100644 --- a/src/parser/cli/tests/fixtures/solana-text.expected +++ b/src/parser/cli/tests/fixtures/solana-text.display.expected @@ -737,6 +737,32 @@ SignablePayload { ), }, }, + Diagnostic { + common: SignablePayloadFieldCommon { + fallback_text: "ok: all 6 instructions have valid program_id_index", + label: "transaction::oob_program_id", + }, + diagnostic: SignablePayloadFieldDiagnostic { + rule: "transaction::oob_program_id", + domain: "transaction", + level: "ok", + message: "all 6 instructions have valid program_id_index", + instruction_index: None, + }, + }, + Diagnostic { + common: SignablePayloadFieldCommon { + fallback_text: "ok: all 6 instructions have valid account indices", + label: "transaction::oob_account_index", + }, + diagnostic: SignablePayloadFieldDiagnostic { + rule: "transaction::oob_account_index", + domain: "transaction", + level: "ok", + message: "all 6 instructions have valid account indices", + instruction_index: None, + }, + }, ], payload_type: "SolanaTx", subtitle: None, diff --git a/src/visualsign/Cargo.toml b/src/visualsign/Cargo.toml index 43709216..c86e5410 100644 --- a/src/visualsign/Cargo.toml +++ b/src/visualsign/Cargo.toml @@ -14,11 +14,15 @@ pretty_assertions = "1.4.1" thiserror = "2.0.12" # the most minimal regex import so that I can do number validation regex = { version = "1.11.1", default-features = false, features = ["std"] } +tracing = { workspace = true } generated = { path = "../generated" } [dev-dependencies] base64 = "0.22.1" hex = "0.4.3" +[features] +diagnostics = [] + [lints] workspace = true diff --git a/src/visualsign/src/field_builders.rs b/src/visualsign/src/field_builders.rs index f5ae15f3..e0c4f5ab 100644 --- a/src/visualsign/src/field_builders.rs +++ b/src/visualsign/src/field_builders.rs @@ -1,4 +1,6 @@ use crate::errors; +#[cfg(feature = "diagnostics")] +use crate::SignablePayloadFieldDiagnostic; use crate::{ AnnotatedPayloadField, SignablePayloadField, SignablePayloadFieldAddressV2, SignablePayloadFieldAmountV2, SignablePayloadFieldCommon, SignablePayloadFieldListLayout, @@ -213,6 +215,46 @@ pub fn create_preview_layout( } } +#[cfg(feature = "diagnostics")] +pub fn create_diagnostic_field( + rule: &str, + domain: &str, + level: crate::lint::Severity, + message: &str, + instruction_index: Option, +) -> AnnotatedPayloadField { + let level_str = level.as_str(); + match level { + crate::lint::Severity::Warn | crate::lint::Severity::Error => { + tracing::warn!( + rule, + domain, + level = level_str, + ?instruction_index, + "{message}" + ); + } + crate::lint::Severity::Ok | crate::lint::Severity::Allow => {} + } + AnnotatedPayloadField { + static_annotation: None, + dynamic_annotation: None, + signable_payload_field: SignablePayloadField::Diagnostic { + common: SignablePayloadFieldCommon { + fallback_text: format!("{level_str}: {message}"), + label: rule.to_string(), + }, + diagnostic: SignablePayloadFieldDiagnostic { + rule: rule.to_string(), + domain: domain.to_string(), + level: level_str.to_string(), + message: message.to_string(), + instruction_index, + }, + }, + } +} + #[cfg(test)] #[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] mod tests { diff --git a/src/visualsign/src/lib.rs b/src/visualsign/src/lib.rs index f2a136ec..589cac0d 100644 --- a/src/visualsign/src/lib.rs +++ b/src/visualsign/src/lib.rs @@ -5,6 +5,8 @@ use serde_json::Value; pub mod encodings; pub mod errors; pub mod field_builders; +#[cfg(feature = "diagnostics")] +pub mod lint; pub mod registry; pub mod test_utils; pub mod vsptrait; @@ -205,6 +207,15 @@ pub enum SignablePayloadField { #[serde(rename = "Unknown")] unknown: SignablePayloadFieldUnknown, }, + + #[cfg(feature = "diagnostics")] + #[serde(rename = "diagnostic")] + Diagnostic { + #[serde(flatten)] + common: SignablePayloadFieldCommon, + #[serde(rename = "Diagnostic")] + diagnostic: SignablePayloadFieldDiagnostic, + }, } // Trait to ensure all SignablePayloadField variants implement serialization correctly @@ -288,6 +299,10 @@ impl FieldSerializer for SignablePayloadField { SignablePayloadField::Unknown { common, unknown } => { serialize_field_variant!(fields, "unknown", common, ("Unknown", unknown)); } + #[cfg(feature = "diagnostics")] + SignablePayloadField::Diagnostic { common, diagnostic } => { + serialize_field_variant!(fields, "diagnostic", common, ("Diagnostic", diagnostic)); + } } // Convert to BTreeMap for alphabetical ordering @@ -309,6 +324,8 @@ impl FieldSerializer for SignablePayloadField { SignablePayloadField::PreviewLayout { .. } => base_fields.push("PreviewLayout"), SignablePayloadField::ListLayout { .. } => base_fields.push("ListLayout"), SignablePayloadField::Unknown { .. } => base_fields.push("Unknown"), + #[cfg(feature = "diagnostics")] + SignablePayloadField::Diagnostic { .. } => base_fields.push("Diagnostic"), } base_fields.sort(); @@ -381,6 +398,8 @@ impl SignablePayloadField { SignablePayloadField::PreviewLayout { common, .. } => &common.fallback_text, SignablePayloadField::ListLayout { common, .. } => &common.fallback_text, SignablePayloadField::Unknown { common, .. } => &common.fallback_text, + #[cfg(feature = "diagnostics")] + SignablePayloadField::Diagnostic { common, .. } => &common.fallback_text, } } @@ -397,6 +416,8 @@ impl SignablePayloadField { SignablePayloadField::PreviewLayout { common, .. } => &common.label, SignablePayloadField::ListLayout { common, .. } => &common.label, SignablePayloadField::Unknown { common, .. } => &common.label, + #[cfg(feature = "diagnostics")] + SignablePayloadField::Diagnostic { common, .. } => &common.label, } } @@ -413,6 +434,8 @@ impl SignablePayloadField { SignablePayloadField::PreviewLayout { .. } => "preview_layout", SignablePayloadField::ListLayout { .. } => "list_layout", SignablePayloadField::Unknown { .. } => "unknown", + #[cfg(feature = "diagnostics")] + SignablePayloadField::Diagnostic { .. } => "diagnostic", } } } @@ -600,6 +623,50 @@ pub struct SignablePayloadFieldUnknown { // Implement DeterministicOrdering for SignablePayloadFieldUnknown impl DeterministicOrdering for SignablePayloadFieldUnknown {} +#[cfg(feature = "diagnostics")] +#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] +pub struct SignablePayloadFieldDiagnostic { + #[serde(rename = "Rule")] + pub rule: String, + #[serde(rename = "Domain")] + pub domain: String, + #[serde(rename = "Level")] + pub level: String, + #[serde(rename = "Message")] + pub message: String, + #[serde(rename = "InstructionIndex", skip_serializing_if = "Option::is_none")] + pub instruction_index: Option, +} + +#[cfg(feature = "diagnostics")] +impl Serialize for SignablePayloadFieldDiagnostic { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + use serde::ser::SerializeMap; + + let len = if self.instruction_index.is_some() { + 5 + } else { + 4 + }; + // Fields emitted in alphabetical key order for deterministic serialization. + let mut map = serializer.serialize_map(Some(len))?; + map.serialize_entry("Domain", &self.domain)?; + if let Some(ref idx) = self.instruction_index { + map.serialize_entry("InstructionIndex", idx)?; + } + map.serialize_entry("Level", &self.level)?; + map.serialize_entry("Message", &self.message)?; + map.serialize_entry("Rule", &self.rule)?; + map.end() + } +} + +#[cfg(feature = "diagnostics")] +impl DeterministicOrdering for SignablePayloadFieldDiagnostic {} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)] pub struct SignablePayloadFieldStaticAnnotation { #[serde(rename = "Text")] @@ -1944,6 +2011,18 @@ mod tests { }; assert_deterministic_ordering(&amount_v2); + #[cfg(feature = "diagnostics")] + { + let diagnostic = SignablePayloadFieldDiagnostic { + rule: "transaction::oob_program_id".to_string(), + domain: "transaction".to_string(), + level: "warn".to_string(), + message: "instruction 0: program_id_index 5 out of bounds".to_string(), + instruction_index: Some(0), + }; + assert_deterministic_ordering(&diagnostic); + } + // Test layout types let preview_layout = SignablePayloadFieldPreviewLayout { title: Some(text_v2.clone()), @@ -2439,4 +2518,140 @@ mod tests { ); assert!(pos_title < pos_version, "Title should come before Version"); } + + #[cfg(feature = "diagnostics")] + #[test] + fn test_diagnostic_field_serialization_alphabetical() { + let field = SignablePayloadField::Diagnostic { + common: SignablePayloadFieldCommon { + fallback_text: "warn: account index 7 out of bounds".to_string(), + label: "transaction::oob_account_index".to_string(), + }, + diagnostic: SignablePayloadFieldDiagnostic { + rule: "transaction::oob_account_index".to_string(), + domain: "transaction".to_string(), + level: "warn".to_string(), + message: "account index 7 out of bounds".to_string(), + instruction_index: Some(2), + }, + }; + + field + .verify_deterministic_ordering() + .expect("Diagnostic field should have deterministic ordering"); + + let json = serde_json::to_string(&field).unwrap(); + let value: serde_json::Value = serde_json::from_str(&json).unwrap(); + let obj = value.as_object().unwrap(); + let keys: Vec<&String> = obj.keys().collect(); + + // Verify top-level alphabetical ordering + assert_eq!(keys, vec!["Diagnostic", "FallbackText", "Label", "Type"]); + + // Verify nested Diagnostic fields are alphabetical + let diag = obj.get("Diagnostic").unwrap().as_object().unwrap(); + let diag_keys: Vec<&String> = diag.keys().collect(); + assert_eq!( + diag_keys, + vec!["Domain", "InstructionIndex", "Level", "Message", "Rule"] + ); + + assert_eq!(obj.get("Type").unwrap(), "diagnostic"); + } + + #[cfg(feature = "diagnostics")] + #[test] + fn test_diagnostic_field_without_instruction_index() { + let field = SignablePayloadField::Diagnostic { + common: SignablePayloadFieldCommon { + fallback_text: "warn: general issue".to_string(), + label: "wallet::missing_idl_mapping".to_string(), + }, + diagnostic: SignablePayloadFieldDiagnostic { + rule: "wallet::missing_idl_mapping".to_string(), + domain: "wallet".to_string(), + level: "warn".to_string(), + message: "general issue".to_string(), + instruction_index: None, + }, + }; + + field + .verify_deterministic_ordering() + .expect("Diagnostic field should have deterministic ordering"); + + let json = serde_json::to_string(&field).unwrap(); + let value: serde_json::Value = serde_json::from_str(&json).unwrap(); + let diag = value.get("Diagnostic").unwrap().as_object().unwrap(); + + // InstructionIndex should be absent when None + assert!(!diag.contains_key("InstructionIndex")); + let diag_keys: Vec<&String> = diag.keys().collect(); + assert_eq!(diag_keys, vec!["Domain", "Level", "Message", "Rule"]); + } + + #[cfg(feature = "diagnostics")] + #[test] + fn test_diagnostic_field_roundtrip() { + let original = SignablePayloadField::Diagnostic { + common: SignablePayloadFieldCommon { + fallback_text: "warn: oob program id".to_string(), + label: "transaction::oob_program_id".to_string(), + }, + diagnostic: SignablePayloadFieldDiagnostic { + rule: "transaction::oob_program_id".to_string(), + domain: "transaction".to_string(), + level: "warn".to_string(), + message: "oob program id".to_string(), + instruction_index: Some(0), + }, + }; + + let json = serde_json::to_string(&original).unwrap(); + let deserialized: SignablePayloadField = serde_json::from_str(&json).unwrap(); + assert_eq!(original, deserialized); + } + + #[cfg(feature = "diagnostics")] + #[test] + fn test_diagnostic_in_signable_payload() { + let payload = SignablePayload::new( + 0, + "Test Transaction".to_string(), + None, + vec![ + SignablePayloadField::TextV2 { + common: SignablePayloadFieldCommon { + fallback_text: "Solana".to_string(), + label: "Network".to_string(), + }, + text_v2: SignablePayloadFieldTextV2 { + text: "Solana".to_string(), + }, + }, + SignablePayloadField::Diagnostic { + common: SignablePayloadFieldCommon { + fallback_text: "warn: instruction 1 skipped".to_string(), + label: "transaction::oob_program_id".to_string(), + }, + diagnostic: SignablePayloadFieldDiagnostic { + rule: "transaction::oob_program_id".to_string(), + domain: "transaction".to_string(), + level: "warn".to_string(), + message: "instruction 1 skipped".to_string(), + instruction_index: Some(1), + }, + }, + ], + String::new(), + ); + + payload + .verify_deterministic_ordering() + .expect("Payload with diagnostic should have deterministic ordering"); + + let json = payload.to_json().expect("should serialize"); + assert!(json.contains("\"Type\":\"diagnostic\"")); + assert!(json.contains("\"Rule\":\"transaction::oob_program_id\"")); + } } diff --git a/src/visualsign/src/lint.rs b/src/visualsign/src/lint.rs new file mode 100644 index 00000000..98458d57 --- /dev/null +++ b/src/visualsign/src/lint.rs @@ -0,0 +1,115 @@ +use std::collections::BTreeMap; + +// TODO(#228): when LintConfig is included in metadata_digest, implement +// `DeterministicOrdering` and a deterministic `Serialize` for both `Severity` +// and `LintConfig` (BTreeMap already gives stable key order; the wrapper +// struct still needs explicit alphabetical field emission). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Severity { + Ok, + Warn, + Error, + Allow, +} + +impl Severity { + pub fn as_str(&self) -> &'static str { + match self { + Severity::Ok => "ok", + Severity::Warn => "warn", + Severity::Error => "error", + Severity::Allow => "allow", + } + } +} + +/// Configuration for lint rule behavior. +/// +/// Controls which rules run, their default severity, and whether +/// ok-level diagnostics are emitted (boot metrics mode). +#[derive(Debug, Clone)] +pub struct LintConfig { + /// Override severity for specific rules. Key is the rule ID + /// (e.g., "transaction::oob_program_id"). + pub overrides: BTreeMap, + + /// When true, rules that find no issues emit an ok-level diagnostic. + /// This provides boot-metric-style attestation where the verifier + /// can confirm every expected rule ran. + pub report_all_rules: bool, +} + +impl Default for LintConfig { + fn default() -> Self { + Self { + overrides: BTreeMap::new(), + report_all_rules: true, + } + } +} + +impl LintConfig { + /// Get the effective severity for a rule, falling back to the provided default. + pub fn severity_for(&self, rule: &str, default: Severity) -> Severity { + self.overrides.get(rule).copied().unwrap_or(default) + } + + /// Whether an ok-level diagnostic should be emitted for this rule. + pub fn should_report_ok(&self, rule: &str) -> bool { + if !self.report_all_rules { + return false; + } + // If the rule is explicitly set to Allow, don't emit ok either + if let Some(Severity::Allow) = self.overrides.get(rule) { + return false; + } + true + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_default_config_emits_ok() { + let config = LintConfig::default(); + assert!(config.report_all_rules); + assert!(config.should_report_ok("transaction::oob_program_id")); + } + + #[test] + fn test_severity_override() { + let mut config = LintConfig::default(); + config + .overrides + .insert("transaction::oob_program_id".to_string(), Severity::Error); + assert!(matches!( + config.severity_for("transaction::oob_program_id", Severity::Warn), + Severity::Error + )); + assert!(matches!( + config.severity_for("transaction::oob_account_index", Severity::Warn), + Severity::Warn + )); + } + + #[test] + fn test_allow_suppresses_ok() { + let mut config = LintConfig::default(); + config + .overrides + .insert("transaction::oob_program_id".to_string(), Severity::Allow); + assert!(!config.should_report_ok("transaction::oob_program_id")); + assert!(config.should_report_ok("transaction::oob_account_index")); + } + + #[test] + fn test_disable_ok_diagnostics() { + let config = LintConfig { + report_all_rules: false, + ..LintConfig::default() + }; + assert!(!config.should_report_ok("transaction::oob_program_id")); + } +}