Skip to content

feat(solana): add Drift Protocol visualizer preset#239

Closed
prasanna-anchorage wants to merge 2 commits into
mainfrom
add-drift-visualizer
Closed

feat(solana): add Drift Protocol visualizer preset#239
prasanna-anchorage wants to merge 2 commits into
mainfrom
add-drift-visualizer

Conversation

@prasanna-anchorage

Copy link
Copy Markdown
Contributor

Summary

  • Adds IDL-based visualizer preset for Drift Protocol v2 (dRiftyHA39MWEi3m9aunc5MzRF1JYuBsbn6VPcn33UH)
  • 249 instructions decoded via Anchor IDL from drift-labs/protocol-v2
  • Uses LazyLock to cache parsed 438KB IDL (parsed once, not on every call)
  • Includes 4 unit tests: IDL loading, discriminator validation, unknown discriminator error, short data error

Test plan

  • cargo clippy -p visualsign-solana --all-targets -- -D warnings
  • cargo test -p visualsign-solana (all 108 tests pass)
  • make -C src test (full workspace)

🤖 Generated with Claude Code

Add IDL-based visualizer for Drift Protocol v2
(dRiftyHA39MWEi3m9aunc5MzRF1JYuBsbn6VPcn33UH) with 249 instructions.
Uses LazyLock to cache parsed IDL. Includes unit tests for IDL loading,
discriminator validation, and error handling.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 8, 2026 05:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new Solana preset visualizer that decodes Drift Protocol v2 instructions via an embedded Anchor IDL, producing structured preview fields (with a raw-data fallback) and caching the parsed IDL for performance.

Changes:

  • Registered a new drift preset module in the Solana presets registry.
  • Implemented DriftVisualizer with IDL-backed instruction parsing + account labeling + raw-data fallback.
  • Added a Drift integration config to route Drift program instructions to the new visualizer.

Reviewed changes

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

File Description
src/chain_parsers/visualsign-solana/src/presets/mod.rs Exposes the new drift preset module so it can be instantiated by the build-time visualizer registry.
src/chain_parsers/visualsign-solana/src/presets/drift/mod.rs Implements Drift instruction visualization using an embedded IDL with LazyLock caching and adds unit tests.
src/chain_parsers/visualsign-solana/src/presets/drift/config.rs Adds the config mapping for the Drift program ID so the visualizer can handle Drift instructions.

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


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()),

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

visualize_tx_commands discards the concrete parsing error (Err(_)) and always falls back to an "Unknown instruction type" display. This makes failures like short data, IDL load failures, or decode errors indistinguishable from an actually-unknown discriminator. Consider capturing the error and surfacing it (e.g., in a Status field and/or via tracing::warn!) so users and logs reflect the real failure reason.

Suggested change
Err(_) => build_fallback_fields(&instruction.program_id.to_string()),
Err(err) => {
tracing::warn!(
program_id = %instruction.program_id,
error = %err,
"Failed to parse Drift instruction"
);
let status_text = format!("Failed to parse Drift instruction: {err}");
let (title, mut condensed_fields, mut expanded_fields) =
build_fallback_fields(&instruction.program_id.to_string());
condensed_fields.push(create_text_field("Status", &status_text));
expanded_fields.push(create_text_field("Status", &status_text));
(title, condensed_fields, expanded_fields)
}

Copilot uses AI. Check for mistakes.
Comment on lines +139 to +142
struct DriftParsedInstruction {
parsed: SolanaParsedInstructionData,
named_accounts: HashMap<String, String>,
}

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DriftParsedInstruction stores named_accounts separately even though SolanaParsedInstructionData already supports named accounts (see the existing population pattern in presets/unknown_program/mod.rs). Keeping a parallel map increases maintenance overhead and risks divergence between the parsed payload and displayed account labels. Consider populating parsed.named_accounts directly and using it when building fields, removing the extra wrapper struct if possible.

Copilot uses AI. Check for mistakes.

named_accounts
}

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expanded view relies on build_named_accounts to label accounts, but current tests only cover IDL loading/discriminators and generic error cases. Add a focused unit test that exercises build_named_accounts (or the end-to-end visualize path) for at least one known discriminator to ensure account names are mapped correctly and remain stable as the embedded IDL changes.

Suggested change
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn build_named_accounts_maps_initialize_user_accounts_by_name() {
let idl = get_drift_idl().expect("Drift IDL should be available for tests");
let instruction = idl
.instructions
.iter()
.find(|inst| inst.name == "initializeUser")
.expect("expected initializeUser instruction in embedded Drift IDL");
let discriminator = instruction
.discriminator
.clone()
.expect("initializeUser should have a discriminator");
let expected_account_names = [
"user",
"userStats",
"state",
"authority",
"payer",
"rent",
"systemProgram",
];
let actual_account_names: Vec<&str> = instruction
.accounts
.iter()
.take(expected_account_names.len())
.map(|account| account.name.as_str())
.collect();
assert_eq!(
actual_account_names,
expected_account_names,
"initializeUser accounts changed in the embedded Drift IDL; update this test if the change is intentional"
);
let accounts: Vec<solana_sdk::instruction::AccountMeta> = expected_account_names
.iter()
.enumerate()
.map(|(index, _)| {
let pubkey = solana_sdk::pubkey::Pubkey::new_from_array([index as u8 + 1; 32]);
solana_sdk::instruction::AccountMeta::new(pubkey, false)
})
.collect();
let named_accounts = build_named_accounts(&discriminator, idl, &accounts);
assert_eq!(named_accounts.len(), expected_account_names.len());
for (index, account_name) in expected_account_names.iter().enumerate() {
let expected_pubkey =
solana_sdk::pubkey::Pubkey::new_from_array([index as u8 + 1; 32]).to_string();
assert_eq!(
named_accounts.get(*account_name),
Some(&expected_pubkey),
"account name {account_name} should map to the corresponding account meta pubkey"
);
}
}
}

Copilot uses AI. Check for mistakes.
When an IDL instruction has both an account and an argument with the
same name (e.g. "admin" in Drift's updateAdmin), label them as
"Current admin" (account) and "New admin" (argument) in the expanded
view to avoid confusion.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this — solid addition, and the structure is right. A few things to call out before it can land.

What's good

  • Follows the IDL-preset pattern cleanly: include_str! + LazyLock to parse the 438KB IDL exactly once is the correct approach, and the file layout (mod.rs / config.rs / drift.json) matches the other presets.
  • Graceful "Unknown Instruction" fallback on decode failure is consistent with the recent spl_token degrade-gracefully behaviour rather than erroring.
  • Uses the field_builders helpers throughout instead of hand-building field structs, and the Current/New labeling on account/arg name collisions is a thoughtful touch.
  • Sensible unit tests for IDL loading, discriminator shape (8 bytes), and the short-data / unknown-discriminator error paths.

Blockers (all from the branch being ~168 commits behind main)
This branch predates two refactors on main, so the test-plan claims hold against the old base but not current main. After rebasing:

  1. config.rs won't compileSolanaIntegrationConfigData.programs is now BTreeMap, not HashMap (changed for deterministic serialization). Switch the two HashMap::new()s to BTreeMap, matching onre_app / exponent_finance.
  2. Drop the presets/mod.rs edit entirely — since #278 that file is auto-generated by build.rs from the directory listing (header says "do NOT edit"). Once rebased, drift/ is auto-discovered and DriftVisualizer registers with no manual wiring.
  3. Test module needs #[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] — every other preset test module has it; without it clippy -D warnings fails CI.

Worth folding in (overlaps with Copilot's open threads)

  • named_accounts as a HashMap gives non-deterministic field ordering in the output, which breaks stable metadata hashing — the fields aren't re-sorted downstream. Copilot also flagged this struct from a maintainability angle (it duplicates SolanaParsedInstructionData::named_accounts). Populating the existing parsed.named_accounts directly resolves both at once.
  • A real end-to-end fixture test (decode one known instruction, e.g. initializeUser / placePerpOrder, to expected fields) would lock in the account-name mapping as the IDL evolves — currently nothing exercises the decode path against a real tx.

Also flagging for cross-PR awareness: #350/#351/#360 are other in-flight Solana presets built on current main (none touch presets/mod.rs), and #289 reworks the solana-add-idl scaffolding — worth rebasing onto and regenerating from, so this preset matches the current template.

Happy to take the mechanical fixes (1–3 + the named_accounts swap) on a rebased branch if useful.

@shahan-khatchadourian-anchorage

Copy link
Copy Markdown
Contributor

I've opened #366, which rebases this work onto current main and gets it building/linting/testing green. This branch is ~168 commits behind and no longer compiles against main (the HashMapBTreeMap switch in SolanaIntegrationConfigData, the auto-generated presets/mod.rs from #278, and the removed VisualizerContext::current_instruction() API all broke it).

#366 keeps your implementation intact and applies only the mechanical fixes needed to land it — full details in its description. Leaving this PR open for now; feel free to review #366 or pull its changes back here, whichever you prefer.

prasanna-anchorage pushed a commit that referenced this pull request Jun 12, 2026
feat(solana): add Drift Protocol visualizer preset

Rebased onto current main and adapted to the post-refactor APIs:
- config.rs and the local named-accounts map use BTreeMap (deterministic
  ordering for stable metadata hashing), not HashMap
- visualize_tx_commands uses the current VisualizerContext API
  (resolve_program_id / resolve_accounts / data) instead of the removed
  current_instruction()
- preset is auto-registered by build.rs via the directory drop-in; no
  presets/mod.rs edit
- test module carries the standard clippy unwrap/expect/panic allow

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants