refactor(solana): single definition for account and program display strings

[shahan-khatchadourian-anchorage]

Summary

Closes #435.

core::InstructionView owns the placeholder vocabulary for resolving account and program indices to display strings ("unresolved(N)" for an index that's out of bounds or a v0 lookup-table entry that hasn't been resolved). Presets that hand-roll their own program_id_index / account-index match instead of routing through the shared functions diverge on that vocabulary -- most render unknown, which carries no index and is strictly less diagnosable, and a signer comparing two transactions sees different vocabulary for the same condition.

Approach

Two shared functions in core:

pub fn resolve_program_display(context: &VisualizerContext) -> String
pub fn resolve_account_display(context: &VisualizerContext, position: usize) -> String

InstructionView is built on top of both. Every preset that named ProgramRef/AccountRef locally now routes through one of these two instead of its own match:

• jupiter_swap, unknown_program -- need every account, so they take InstructionView::from_context(context); the local closures disappear entirely.
• system -- names two fixed positions (payer, new_account), so it calls resolve_account_display(context, N) directly.
• associated_token_account, compute_budget, stakepool, swig_wallet, token_2022 -- only need the program, so they call resolve_program_display(context) directly. token_2022 additionally drops its own private resolve_program_id wrapper, which duplicated the same match under a different name.
• spl_token is unaffected by this PR: fix(solana/spl_token): migrate to shared core::InstructionView, drop data clone #381 already migrated it to core::InstructionView directly.

Behavior

No output changes for in-bounds indices, which is every index a well-formed instruction produces. The only reachable unresolved arm across all of the above is system's two fixed-position reads (CreateAccount's new_account/payer), which now render unresolved(oob:N) instead of unknown.

Test plan

• cargo test -p visualsign-solana -- all tests pass
• make -C src lint -- clippy clean with -D warnings
• make -C src fmt -- no diff
• test_resolve_account_and_program_display in core/mod.rs asserts resolve_program_display/resolve_account_display against literal expected strings (resolved, ALT-unresolved, out-of-bounds) rather than comparing them against InstructionView, which calls the same functions and so couldn't fail
• test_create_account_with_missing_new_account_index_renders_oob_placeholder in system/mod.rs drives create_system_preview_layout through a CreateAccount instruction with a truncated account list and asserts the rendered New Account/Payer fields
• After this change, grepping the preset tree for a hand-rolled ProgramRef::Unresolved arm producing the placeholder string leaves none outside resolve_program_display itself and unknown_program's two error-returning helpers (which reject rather than render a placeholder)

Scope

presets/swig_wallet/mod.rs:2037,2047 -- program_scope_type_name / numeric_type_name map unrecognized discriminants to a name. Unrelated to account resolution; left untouched.

Housekeeping

• src/Makefile's SOLANA_PRESET_SOURCES find now also covers src/integrations, which the same build.rs (emit_module_declarations) generates and which is equally unreachable by cargo fmt.
• core/mod.rs's VisualizerContext "Resolution patterns" doc no longer cites unknown_program as the example of hand-rolled matching, since this PR removes that pattern from it; it points at resolve_account_display/resolve_program_display instead.

🤖 Generated with Claude Code

[prasanna-anchorage]

Verified the shared resolve_program_display/resolve_account_display functions and traced every caller — the placeholder-string change ("unknown" → "unresolved(oob:N)") doesn't break anything downstream, and the two deliberately-untouched "unknown" sites (swig_wallet, spl_token) are correctly out of scope per the PR description.

One thing worth fixing before merge: create_system_preview_layout in src/chain_parsers/visualsign-solana/src/presets/system/mod.rs:50 still hand-rolls its own program_id match instead of calling resolve_program_display. That leaves a second definition of the program-placeholder vocabulary in the crate — exactly the drift this PR's stated goal was to eliminate. A future change to the unresolved-program placeholder format would update unknown_program and InstructionView but silently miss this local match, so System-program instructions with an unresolved program index would render differently from every other preset.

Non-blocking given the size, but please fold system/mod.rs:50 into resolve_program_display too — it's the same shape as the three presets already converted.

[Copilot]

Pull request overview

Centralizes Solana account/program display resolution to ensure consistent unresolved placeholders.

Changes:

• Adds shared display-resolution helpers and tests.
• Migrates Jupiter, System, and unknown-program presets.
• Extends formatting coverage to Solana presets and reformats affected files.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/rustfmt.toml	Sets Rust edition for direct rustfmt runs.
src/Makefile	Formats all Solana preset sources.
src/chain_parsers/visualsign-solana/src/core/mod.rs	Adds shared resolution helpers and tests.
.../presets/unknown_program/mod.rs	Uses shared account/program resolution.
.../presets/system/mod.rs	Uses shared fixed-position account resolution.
.../presets/jupiter_swap/mod.rs	Uses the canonical instruction view.
.../presets/swig_wallet/mod.rs	Applies rustfmt output.
.../presets/orca_whirlpool/mod.rs	Applies rustfmt output.
.../presets/neutral_trade/mod.rs	Applies rustfmt output.
.../presets/meteora_dlmm/mod.rs	Applies rustfmt output.
.../presets/meteora_damm_v2/mod.rs	Applies rustfmt output.
.../presets/kamino_borrow/mod.rs	Applies rustfmt output.
.../presets/jupiter_perps/mod.rs	Applies rustfmt output.
.../presets/jupiter_earn/mod.rs	Applies rustfmt output.
.../presets/jupiter_borrow/mod.rs	Applies rustfmt output.
.../presets/exponent_finance/mod.rs	Applies rustfmt output.
.../presets/drift/mod.rs	Applies rustfmt output.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[pepe-anchor]

must: The program half of the vocabulary is not consolidated, which is why ProgramRef survives on this import line: system/mod.rs:50, jupiter_swap/mod.rs:441, and jupiter_swap/mod.rs:490 still inline the ProgramRef::Resolved/Unresolved match. prasanna-anchorage asked for system/mod.rs:50 in review and it is unchanged at HEAD, and the two jupiter_swap copies sit in a file this PR already edits and already imports crate::core from. Six more presets hold the same copy (compute_budget:69, spl_token:74, associated_token_account:48, token_2022:24, stakepool:43, swig_wallet:70), so the new doc comment at core/mod.rs:251, "This is the single definition of the program placeholder vocabulary", is false the moment this merges. A false documented invariant is worse than none: the next person changing the unresolved-program format will trust it, update resolve_program_display, and ship exactly the drift this PR exists to prevent. Minimal fix: three one-line substitutions to resolve_program_display(context) in the two files already open, or reword core/mod.rs:251 to stop asserting an invariant nothing enforces.

[shahan-khatchadourian-anchorage]

Fixed: consolidated all remaining inline ProgramRef matches onto resolve_program_display(context) — system/mod.rs:50, both copies in jupiter_swap/mod.rs, compute_budget, spl_token, associated_token_account, stakepool, and swig_wallet; token_2022's duplicate resolve_program_id wrapper is removed and its 9 call sites now call resolve_program_display directly. The doc comment at core/mod.rs:251 is now accurate.

[pepe-anchor]

should: This is the only behavior change in the PR (unknown becomes unresolved(oob:N)) and nothing tests it here: system/mod.rs and unknown_program/mod.rs both contain zero #[test] functions, and the new core/mod.rs test exercises the helper in isolation, never through create_system_preview_layout. The arm is reachable, not theoretical: VisualizerContext::account at core/mod.rs:163 indexes compiled_instruction.accounts with no length guard, so a CreateAccount whose data bincode-decodes fine but whose account list carries fewer than two indices renders unresolved(oob:1) as New Account and unresolved(oob:0) as Payer in the signing preview. Given the 2ee2ee79 revert on these same files, whose rule was to keep user-visible fields identical across a refactor, the string a signer actually sees should be pinned by a test that builds that instruction and asserts the rendered New Account and Payer fields.

[shahan-khatchadourian-anchorage]

Fixed: added test_create_account_with_missing_new_account_index_renders_oob_placeholder in system/mod.rs, which drives create_system_preview_layout through a CreateAccount instruction whose account list omits the New Account index and asserts the rendered New Account and Payer text fields.

[pepe-anchor]

nit: InstructionView::from_context now delegates straight to resolve_account_display and resolve_program_display, so these three assertions compare each function against itself and cannot fail. They guard the one definition that structurally cannot drift and none of the nine hand-rolled copies that can. Line 500's unresolved(oob:2) assertion is the only line in this test carrying signal; the drift the test name claims to cover would be caught by asserting against literal expected strings, or by testing a preset that reads a fixed position.

[shahan-khatchadourian-anchorage]

Fixed: replaced the self-comparing assertions in core/mod.rs with literal expected strings — renamed to test_resolve_account_and_program_display, which now asserts resolve_program_display/resolve_account_display against pubkey/placeholder literals rather than against InstructionView, which calls the same functions.

[prasanna-anchorage]

Reviewed the substance and it's correct — the main ask is a rebase before this is worth anyone's careful attention.

Rebase first: 17 files → ~5

The first commit here (make fmt + src/rustfmt.toml) is already on main byte-for-byte via #436, and main's preset tree is already rustfmt-clean — I checked by running rustfmt --check over all 60 preset sources with the config in place: zero diffs, zero errors.

So the whitespace churn across drift / exponent_finance / jupiter_* / kamino_borrow / meteora_* / neutral_trade / orca_whirlpool / swig_wallet is stale-base noise that disappears on rebase. The real review surface is core/mod.rs, jupiter_swap, system, unknown_program.

The actual change is clean

Checked each hunk rather than skimming:

• resolve_program_display / resolve_account_display are exact extractions of the arms InstructionView already had. InstructionView::from_context iterates 0..num_accounts(), so its None arm stays unreachable and the produced string is unchanged.
• AccountRef correctly dropped from the jupiter_swap / system / unknown_program imports (no remaining uses); ProgramRef correctly retained in all three (unknown_program:36/77/338, system:51, jupiter_swap:450/499).
• unknown_program::try_parse_with_idl passes an index drawn from 0..num_accounts(), so the oob arm is unreachable there too — same as before.
• No off-by-one in system: position 1 → new_account, 0 → payer, matching what it replaced.
• The new fixture (1 key, accounts: [0, 50]) genuinely exercises all three arms — resolved, ALT-unresolved, out-of-bounds.

Two findings, both low

1. core/mod.rs:71 — stale doc, now actively misleading.

The VisualizerContext "Resolution patterns" section still says:

Partial rendering (catch-all visualizers). Pattern-match on program_id() and account(n) directly and substitute a placeholder... The unknown_program preset is the canonical example

This PR is precisely the removal of that hand-rolled match from unknown_program. So the doc now cites as canonical a preset that no longer does it, and instructs the next preset author to reproduce the exact duplication that caused the "unknown" vs unresolved(oob:N) divergence being fixed here. It should point at resolve_account_display / resolve_program_display.

2. src/Makefile:58 — SOLANA_PRESET_SOURCES misses src/integrations.

The same build.rs (emit_module_declarations) generates an identical include!-based module tree for src/integrations, whose sources are equally unreachable by cargo fmt. src/integrations/ holds only mod.rs today, so nothing is missed — but the first integration subdirectory added there is silently unformatted, which is exactly the failure mode this target exists to close.

Not hypothetical: that failure just took main red (see #474). #375 branched before #436 landed, so its CI ran without the preset pass and went green; the gap only surfaced once both were on main together. Widening the find to cover both trees closes the same hole one directory over.

One stale line in the description

The "Scope" section says spl_token is deliberately left hand-rolling the match, but 7b80f28b (#381) already migrated it to core::InstructionView. Worth updating so a reader doesn't go looking for remaining work that's done.

[prasanna-anchorage]

Coming back to this after a month. pepe-anchor's three comments from 8/28 have no replies, and the branch hasn't moved since 8/31, so I re-verified the two that block against current main rather than the stale base — both still hold, and one of the numbers has changed in your favour.

1. core/mod.rs:251 documents an invariant that is false on merge

This is pepe-anchor's must (and the same thing as my 8/25 ask about system/mod.rs:50), re-checked rather than restated. The doc says:

This is the single definition of the program placeholder vocabulary; InstructionView and presets that need only the program both route through it so the two cannot drift apart.

Grepping main for the literal it claims to own — a ProgramRef::Unresolved arm producing format!("unresolved({raw_index})") — leaves 8 sites across 7 presets that this PR does not touch:

associated_token_account/mod.rs:48   compute_budget/mod.rs:69
jupiter_swap/mod.rs:451             jupiter_swap/mod.rs:500
stakepool/mod.rs:43                 swig_wallet/mod.rs:70
system/mod.rs:52                    token_2022/mod.rs:24

All eight are the same two-arm match context.program_id(), structurally identical to the extracted function. pepe-anchor counted nine on 8/28; spl_token has since been migrated by #381, so eight is the current number. unknown_program:59 is correctly excluded — this PR deletes it.

Two of those (jupiter_swap:451, jupiter_swap:500) and system:52 are in files this PR already opens and already imports crate::core from, so the minimal fix is three one-line substitutions to resolve_program_display(context). Otherwise reword line 251 so it stops asserting an invariant nothing enforces — a false documented invariant is worse than none, because the next person to change the unresolved-program format will trust it and ship exactly the drift this PR exists to prevent.

2. Three of the four assertions in the new test cannot fail

pepe-anchor's nit, confirmed by reading the delegation. InstructionView::from_context at core/mod.rs:237-241 is now:

let program_id = resolve_program_display(context);
let accounts = (0..context.num_accounts()).map(|i| resolve_account_display(context, i)).collect();

So in test_resolve_account_display_matches_instruction_view, resolve_account_display(&ctx, 0) == view.accounts[0], … 1 == view.accounts[1] and resolve_program_display(&ctx) == view.program_id each compare a function against itself. Any mutation — changing the literal, swapping the arms, editing the format string — changes both sides identically. Only assert_eq!(resolve_account_display(&ctx, 2), "unresolved(oob:2)") pins a literal, and it is the only line in the test that can go red.

3. On test coverage — narrowing pepe-anchor's should

I want to correct the framing here, because system/mod.rs having zero #[test] functions understates the coverage. create_system_preview_layout is driven end to end by golden fixtures: src/parser/cli/tests/fixtures/solana-text.display.expected:85,459 and solana-json.display.expected:59 both contain label: "Transfer Amount", a string that exists only at system/mod.rs:70, and src/integration/tests/parser.rs:279 covers it again.

The precise gap is narrower and still real: no fixture exercises the CreateAccount arm, which is the only arm this PR changes. grep -rn "Create Account" across the fixture tree returns nothing, and grep -rn "unresolved(oob" hits only the definition at core/mod.rs:274 and the single unit assertion at :500. So the string a signer would actually see for a CreateAccount whose account list is shorter than the layout expects is unpinned by any rendering-path test.

Worth noting the blast radius is genuinely small: of the four None => "unknown" sites this PR rewrites, jupiter_swap:94 and unknown_program:67 are reached only from 0..context.num_accounts() loops, so their None arm is unreachable and their output is unchanged. Only system's two fixed-position reads can hit it. That makes a single fixture over CreateAccount sufficient.

4. Housekeeping

• The Scope section is stale: it says spl_token "carries the same stale arm … which PR #381 removes". #381 merged 2026-08-31, and spl_token on main has neither match left — it takes &InstructionView directly.
• Branch is 2 ahead / 129 behind main, still 17 files. The fmt churn across drift / exponent_finance / jupiter_* / kamino_borrow / meteora_* / neutral_trade / orca_whirlpool / swig_wallet is stale-base noise from before #436 landed and disappears on rebase, leaving ~4 files of real review surface.

Rebase plus the three substitutions (or the reworded doc) and a CreateAccount fixture would make this straightforward to approve — the extraction itself is correct, and I checked each hunk on 8/31.

[shahan-khatchadourian-anchorage]

@prasanna-anchorage replying to your three reviews (8/25, 8/31, 9/15) — apologies for the month of silence, these had gone unanswered along with pepe-anchor's.

Rebased onto main (3b573090): the 17-files-of-fmt-noise is gone — diff is now the ~5 files of real review surface you called out (#436/#381 had already landed the whitespace and the spl_token migration on main).

1. system/mod.rs:50 (8/25) / the 8-site count (9/15), same as pepe-anchor's must: fixed. Every remaining hand-rolled ProgramRef match now routes through resolve_program_display(context) — system, both jupiter_swap sites, compute_budget, associated_token_account, stakepool, swig_wallet, and token_2022 (whose duplicate resolve_program_id wrapper is removed, its 9 call sites now call resolve_program_display directly). core/mod.rs:251's "single definition" claim is accurate now — verified by grepping the preset tree for a hand-rolled ProgramRef::Unresolved arm; none remain outside resolve_program_display itself and unknown_program's two error-returning helpers.

2. Self-comparing test (9/15), same as pepe-anchor's nit: fixed. Renamed to test_resolve_account_and_program_display; it now asserts resolve_program_display/resolve_account_display against literal expected strings instead of against InstructionView, which called the same functions.

3. CreateAccount OOB coverage (9/15), narrowing pepe-anchor's should: fixed with the narrower fix you identified — test_create_account_with_missing_new_account_index_renders_oob_placeholder in system/mod.rs drives create_system_preview_layout through a CreateAccount instruction with a truncated account list and pins the rendered New Account/Payer fields. Didn't add a golden CLI fixture on top since the unit test already pins the exact string a signer would see.

4. core/mod.rs doc citing unknown_program as the canonical hand-rolled-match example (8/31): fixed — it now points at resolve_account_display/resolve_program_display, since this PR removes that pattern from unknown_program.

5. Makefile:58 missing src/integrations (8/31): fixed — SOLANA_PRESET_SOURCES now covers both directories.

6. Stale PR description (8/31, 9/15): rewritten against the current diff — dropped the stale spl_token-scope claim and the #436-stacking note (no longer stacked; based on main directly), and the Approach/Test-plan sections now reflect the full current scope (all 7 presets, not just the original 3).

All green: cargo test -p visualsign-solana (293 passed), make lint clippy-clean, make fmt no diff.