fix(solana/spl_token): migrate to shared core::InstructionView, drop data clone

[shahan-khatchadourian-anchorage]

Summary

• Removes the private InstructionView struct from spl_token/mod.rs that clones context.data().to_vec() into a Vec<u8> on every instruction — a per-instruction heap allocation with no architectural justification.
• Switches to the shared crate::core::InstructionView for program ID and account resolution.
• Accesses instruction bytes via context.data() directly (a zero-copy borrow), consistent with all other presets in the codebase.
• Fixes the stale None => "unknown" fallback arm (now "unresolved(oob:N)" matching the canonical form in core::InstructionView).

Closes #380

Test plan

• cargo test -p visualsign-solana — 328 passed, 0 failed, 3 ignored
• make -C src lint — clippy clean with -D warnings
• cargo fmt --all -- --check — clean
• Diff is mechanical: import change, struct deletion, &instruction.data -> context.data() substitutions, and the rustfmt reflow those substitutions imply (the shorter argument lets 17 create_text_field("Raw Data", ...) calls fit on one line)

Note on formatting coverage

The rustfmt reflow above is invisible to CI. presets/mod.rs declares preset modules through include!(concat!(env!("OUT_DIR"), "/generated_presets_mod.rs")), so rustfmt cannot traverse into presets/*/ — cargo fmt --all formats presets/mod.rs and nothing beneath it. The whole preset tree is therefore unformatted as far as the make generated + clean-tree gate is concerned. Worth a follow-up issue; out of scope here.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR updates the spl_token Solana preset to use the shared crate::core::InstructionView (introduced in #367) and removes the preset’s private InstructionView that cloned instruction bytes per instruction, switching all byte access to a zero-copy borrow via context.data().

Changes:

• Replaces the private spl_token::InstructionView with the shared core::InstructionView for program/account display resolution.
• Eliminates per-instruction heap allocation by removing context.data().to_vec() and decoding directly from context.data().
• Aligns unresolved-account fallback behavior with the canonical core::InstructionView placeholder format.

---

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

[pepe-anchor]

should: This PR reroutes the displayed instruction bytes at 20 sites, but no test asserts the Raw Data value: every assertion in tests.rs (457-464, 941-1077, 1136) only checks that the Raw Data label exists, and the one end-to-end fallback test at tests.rs:1104 uses data: vec![], so all 269 tests pass even if the hex came from an unrelated buffer; add one assert_eq! on the hex against the instruction bytes in an existing test.

[pepe-anchor]

nit: The 17 rewritten Raw Data sites hand-roll create_text_field("Raw Data", &hex::encode(...)) instead of the shared create_raw_data_field(context.data(), None) that the sibling token_2022 preset uses (token_2022/mod.rs:379), which is already in scope via the field_builders::* glob and emits a byte-identical field; since the PR is touching every one of these lines anyway, this is the moment to converge.

Suggested change

	expanded_fields.push(create_text_field("Raw Data", &hex::encode(context.data()))?);
	expanded_fields.push(create_raw_data_field(context.data(), None)?);

[prasanna-anchorage]

Approving. Checked out and built locally — cargo clippy -p visualsign-solana --all-targets -- -D warnings clean, cargo test -p visualsign-solana 328 passed / 0 failed / 3 ignored, matching your test plan exactly. The migration is behaviour-preserving on every reachable path, and the one semantic difference is provably dead code.

The Vec<u8> clone removal is a pure win. hex::encode(&instruction.data) where data was context.data().to_vec() is byte-identical to hex::encode(context.data()), and TokenInstruction::unpack sees the same slice. context.data() returns &'a [u8] borrowed from the context, so the eighteen call sites all become zero-copy with no lifetime gymnastics. No instruction.data references survive, and create_preview_layout_field already took context, so there's no signature churn to review.

The fallback-arm change is unobservable, which is the right kind of "fix". Worth stating explicitly since the PR description presents it as a behaviour fix and a reviewer might reasonably go looking for a fixture to update: context.account(position) returns None only via self.compiled_instruction.accounts.get(position)?, and num_accounts() is self.compiled_instruction.accounts.len(). The loop is 0..num_accounts(), so the None arm is unreachable by construction — exactly what core::InstructionView's own comment says. "unknown" -> "unresolved(oob:{i})" therefore cannot change any rendered payload; it aligns the dead arm with the canonical form so the next person who copies this code copies the right thing. No snapshot should move, and none does.

This completes the migration, which is worth noting in the PR description because it's a stronger claim than "spl_token now uses the shared struct": grep "struct InstructionView" across visualsign-solana now returns only core/mod.rs, and data().to_vec() returns nothing at all. #380's dead-allocation class is gone from the crate, not just from this preset.

Non-blocking follow-up

jupiter_swap/mod.rs:89-96 still inlines the identical loop, stale arm included:

let instruction_accounts: Vec<String> = (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();

Because it also walks the full 0..num_accounts() range, the same dead-arm reasoning holds and it's a drop-in for InstructionView::from_context(context).accounts. Reasonable either as a fold-in here or as a sibling issue to #380.

One caution for whoever picks that up: system/mod.rs:120-131 looks like the same pattern but isn't. It resolves fixed indices (context.account(0) / account(1)) rather than iterating, so None is genuinely reachable there — a malformed CreateAccount carrying fewer than two accounts hits it. Its "unknown" is live behaviour, not a stale copy, and swapping in the shared view or the oob: string would be a real change needing its own justification. Also worth coordinating with #438, which is already touching Solana display-string de-duplication and could collide.