Flag unregistered programs in Solana intermediate output

[rituparna-mandal-anchor]

Summary

• The Solana intermediate output now includes every inner/CPI instruction observed in a caller-supplied transaction simulation
• Each simulated instruction and top-level instruction is tagged with a registeredSource (native/SPL, built-in, in-crate preset, caller-supplied, or unregistered)
• Where our parser can decode a simulated instruction's arguments via a known IDL, those decoded args are included; where the RPC itself already decoded it (e.g. common SPL transfers), that decode is passed through.
• This is purely additive and opt-in: existing consumers who don't supply a simulation result see no change, and static decode's output is completely
  unaffected either way.

Why

Policy engines need to catch unrecognized programs, including ones that only become visible through pre-signing simulation.

Test plan

• Full test suite passes
• Real mainnet simulation fixture (Jupiter swap) verified to correctly classify all trust levels our decoder can produce, and to round-trip through our
  binary encoding intact

[github-actions]

Hi @rituparna-mandal-anchor, thank you for your contribution!

It looks like this is your first time contributing. To get this PR merged, please review our Contributor License Agreement (CLA) here: https://ironcladapp.com/public-launch/6896309b0f158de8c7450de6

Once you have reviewed the agreement, a maintainer (@anchorageoss/maintainers) will need to approve your addition to our contributors list by commenting /approve-cla on this PR. Thank you!

[Copilot]

Pull request overview

This PR extends the Solana intermediate-output pipeline to help downstream policy engines identify and reason about unknown programs, including CPI calls that are only observable via pre-signing simulation results.

Changes:

• Adds is_unregistered to Solana intermediate instructions, computed from the existing trusted-program set (independent of IDL matching).
• Adds support for attaching caller-supplied simulated instruction trees (including nested CPI calls) to the intermediate output as inner_instructions.
• Bumps SOLANA_INTERMEDIATE_SCHEMA_VERSION from 1 → 2 and extends the proto + generated types to carry simulation instructions.

Reviewed changes

Copilot reviewed 6 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/generated/src/generated/parser.rs	Adds simulated_instructions to SolanaMetadata and introduces the recursive SimulatedInstruction message type.
src/codegen/src/main.rs	Updates codegen attributes for serde + borsh for the new SimulatedInstruction type and field defaults.
src/chain_parsers/visualsign-solana/tests/common/mod.rs	Updates Solana test helpers to populate the new simulated_instructions field.
src/chain_parsers/visualsign-solana/src/intermediate.rs	Bumps intermediate schema version; adds is_unregistered + inner_instructions to the intermediate instruction model; adds conversions and tests.
src/chain_parsers/visualsign-solana/src/core/visualsign.rs	Plumbs simulated instructions from VisualSignOptions into intermediate output generation and attaches nested inner/CPI calls by top-level index.
src/chain_parsers/visualsign-solana/src/cli_plugin.rs	Ensures CLI-produced Solana metadata initializes simulated_instructions as empty.
proto/parser/parser.proto	Adds simulated_instructions to SolanaMetadata and defines the new SimulatedInstruction message.

---

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

[prasincs]

Nit: I think it should be simulated_ because it maps with rest of code better

[rituparna-mandal-anchor]

addressed, rename this var

[prasincs]

We have to expand the concept of registered here because registered in our case doesn't just mean being in built in crates but also whether it's part of the signed IDLs in the request. I can't think of a better term to use here but essentially there's a materialized registry by this point to lookup in wherever that program came from.

[nathan-margosian-anchor]

signed IDLs in the request
I thought IDLs were still being shipped as part of the OpenVSP binary for the time being. Has that already changed?

[prasincs]

We have had support for it for at least 8 months

#128

[nathan-margosian-anchor]

@prasincs @prasanna-anchorage Ok, I'm seeing this now -- thanks for linking the PR.

Basically the BE can send a map of IDLs (either signed or unsigned -- in this case we would want to always sign them, right?) and those get added to a registry. Then, when the parser is actually decoding, it first checks the IDLs stored as JSON files in the binary and if it doesn't find the program/instruction there, then it looks in the registry for the program/instruction.

It seems there is actually a third IDL source: solana_parser, which is included in the registry's implementation of has_idl() but is not a source in get_idl(), which seems to be a bug.

Based on this I have some questions:

1. How is it safe to accept IDLs from the backend? Even if they are signed and we can verify in the TEE that the payload has not been tampered with (via the signed value), how is it safe to trust that the BE is sending the right IDL? This probably didn't matter when we were just using these for visualization -- but now that we are enforcing SCX policies on the output, this doesn't feel safe.
   • Also, signing of the BE IDLs doesn't seem to be required currently so we don't even have that guarantee.
2. Should the solana_parser IDLs be included in get_idl()?
3. What is the north star for these sources? Will we want to maintain all three of these sources indefinitely or is there one source we are trying to converge on?

[prasanna-anchorage]

replied out of band

[shahan-khatchadourian-anchorage]

Solid refactor — flattening in f2187da0 drops the positional-alignment assumption and reads much better than the nested version. Good test coverage for the size: native trust independent of IDL match, preset-only coverage via preset_program_ids(), a real on-chain router-CPI-ing-Jupiter case, borsh round-trip.

Ran a security pass too; nothing met the bar for a vulnerability. The provenance note below is legibility, not an exploit.

CI fails on f2187da0 — abi_metadata.rs:1029 missing simulate_transaction_result. It's in visualsign-ethereum's test target, so cargo build is clean and a Solana-crate test run never compiles it; --all-targets catches it.

is_unregistered only on the simulated list. c6ed0e8b had it on SolanaIntermediateInstruction ("Downstream policy engines (e.g. the HSM) use this to decide whether to reject a transaction containing it"); f2187da0 removed it. Deliberate, or collateral of the flattening? The static computation reads that instruction's own program_key and never depended on the positional alignment being removed.

As it stands the blob has two lists with opposite provenance — instructions[] from message_hex, simulated_instructions[] from caller metadata — and the only verdict is on the second. Consumers can't fill the gap: is_trusted_program's set (NATIVE_PROGRAM_NAMES + ProgramType::from_program_id + preset_program_ids()) lives in the binary and grows with every preset. Restoring looks like a revert of the c6ed0e8b field hunk plus the static-side variants of the three tests renamed to _via_simulation.

Schema version. c6ed0e8b bumped to 2, 7036a85f reverted. The appended length prefix ships on every emission, not just opted-in ones. A Rust mirror using borsh::from_slice or from_reader fails loudly — both reject trailing bytes — but a decoder calling deserialize directly, or a non-Rust mirror without a leftover-input check, reads a stale shape while its schema_version == 1 assert still passes. That second case is what the version field exists to prevent, and it's the one we can't verify from here. Cheaper to bump now than after anything adopts 1.

What "registered" resolves to. Not reopening the thread, just one constraint that isn't visible from the diff: extract_idl_mappings_with_signers validates a signature only when one is present and accepts unsigned IDLs (core/visualsign.rs:320, flagged in-tree as a known gap). So the materialized registry holds signed and unsigned alike, and a literal registry lookup would admit unsigned caller IDLs — not the ask upthread, which was scoped to signed. Matching formulation: built-ins + presets + caller IDLs with a verified, allowlisted signature. IdlRegistry::get_idl(&str) takes the base58 string directly, and the registry is already built at the call site (core/visualsign.rs:438) two lines above the conversion — so the gate is the decision, not the plumbing. Ideally one shared helper for both paths.

metadata_digest changes for every Solana request, include_intermediate_output or not — borsh writes a tag byte for the None. Does anything downstream recompute it for Solana, and does proto regeneration ship in the same rollout? Fails closed, and the Ethereum abi_mappings change set the precedent, so this is deploy ordering. Upside: the digest now covers simulate_transaction_result, so a consumer can bind what it supplied.

Smaller:

• Caller-asserted data has the same shape and apparent authority as enclave-derived fields, under a boolean that reads as an enclave verdict. Provenance is in prose at both layers but not in the encoding; idl_source (intermediate.rs:121) is the existing idiom for tagging it.
• program_key/instruction_data_hex copied verbatim. The static sibling is hex::encode output from solana_parser, so one decode routine across both breaks on 0xDEADBEEF or a non-base58 key. Pubkey::from_str / visualsign::encodings::decode_hex are the house pattern.
• intermediate.rs:292 ("computed the same way as the static-decode path") contradicts :65 ("the only place is_unregistered is computed").
• The inline serde(default) point still applies post-flattening: SimulateTransactionResult.instructions has no default, so "simulateTransactionResult": {} fails through the gateway.

[rituparna-mandal-anchor]

/approve-cla

[shahan-khatchadourian-anchorage]

All six from last round addressed: CI green, registered_source on the static instructions, schema version 2 with the original wording restored, provenance in the encoding instead of a bool. Taking the raw RPC response instead of a caller-decoded list is a better shape — the caller's decode is out of the trust path. SolanaIdlParseError and the mainnet Jupiter fixture are both good.

Static decode output changed. extract_solana_intermediate_output now merges 19 preset IDLs into parse_transaction_with_idls with override_builtin: true (builtin_programs.rs:373); on origin/main that argument is caller configs only. For any transaction touching a preset program, simulation or not: idl_source flips "BuiltIn" -> "Custom" and idl_hash changes for the four IDs that are also solana_parser builtins (Drift, Meteora, Orca, Jupiter v6), and parsed_instruction_data goes None -> populated for the rest. The schema bump covers shape, not these values; the PR body's "byte-for-byte unchanged" no longer holds.

idl_source: "Custom" now covers both our presets and wallet-supplied IDLs — the distinction RegisteredSource draws two fields over. Add a Preset source string, or document that registered_source is the field to read.

Four silent paths to an empty simulation: base64 .ok()?, JSON .ok()?, construct_idl_records_map err -> Vec::new(), UiInstruction::Compiled -> continue. All byte-identical to "no simulation sent", no log. A caller sending the JSON-RPC envelope instead of the bare result, or non-jsonParsed encoding, gets nothing — and a consumer gating on "no unregistered programs" sees nothing to object to. The construct_idl_records_map case drops the Unregistered classifications too, which need no IDL at all. Warn at each; better, reject a present-but-unparseable field.

response.value.err dropped. A reverted simulation's partial trace attaches as complete; an unregistered call never reached looks like one that isn't there.

No bound on the payload. Caller IDL JSON is capped at 1 MiB in the same file; this isn't. It compounds: per-instruction IDL resolution means N inner instructions against a 438 KB Drift parse, and merge_preset_idl_configs clones ~2 MB on every intermediate request (was None in the common case). Memoize per program, cache the parsed map.

ENTRIES drifts from the derived set. preset_program_ids() enumerates available_visualizers(); ENTRIES is hand-maintained, and meteora_damm_v2.json is already missing from it — reports Preset with nothing to decode it. Worth a test that every preset dir with an IDL JSON appears in ENTRIES, plus the same step in solana-add-idl.

Smaller:

• index never checked against the instruction count, though correlating back into instructions[] is its purpose.
• stack_height defaults to 1 (top-level) for entries that are all CPIs. 0 or Option<u32> reads as unknown.
• Proto comment names parse_transaction_with_idls; simulated instructions go through parse_partially_decoded_instruction_idl. It's also silent on envelope-vs-result and on jsonParsed, both of which fail silently when wrong.
• merge_preset_idl_configs's doc describes a caller override winning over a preset; from_idl_mappings drops trusted IDs, so it can't happen.

Carried over: metadata_digest still shifts for every Solana request.

[prasanna-anchorage]

/approve-cla

[github-actions]

ℹ️ @rituparna-mandal-anchor is already in the CLA signers list.

[prasanna-anchorage]

@shahan-khatchadourian-anchorage re:

ENTRIES drifts from the derived set. preset_program_ids() enumerates available_visualizers(); ENTRIES is hand-maintained, and meteora_damm_v2.json is already missing from it — reports Preset with nothing to decode it.

This one is stale as of a9e28a3. meteora_damm_v2.json is in ENTRIES, and the two lists match exactly today: 19 entries in ENTRIES, 19 preset directories shipping an IDL JSON, one-to-one.

The structural point holds, with a different example. preset_program_ids() derives from available_visualizers(), which includes presets that register a program ID but ship no IDL JSON. Today that is swig_wallet (swigypWHEksbC64pWKwah1WTeh9JXwx8H1rJHLdbQMB): not in NATIVE_PROGRAM_NAMES, not in ENTRIES, so registered_source returns Preset and there is nothing to decode it with. That is not drift that a rebase fixes — it is permanent, because the preset renders from code rather than an IDL.

So the question the test would be pinning down isn't only "did someone forget an ENTRIES line", it's what Preset is asserting. Right now it conflates "we vouch for this program ID" with "we can decode this instruction", and those are different sets. If a policy engine reads Preset as the latter, swig_wallet is already a counterexample. Worth either splitting the two or documenting which one registered_source answers — then the test enforces whichever you pick.

[prasanna-anchorage]

Reviewed a9e28a3. Direction is right — taking the raw RPC response instead of a caller-decoded list keeps the caller's decode out of the trust path, and RegisteredSource over a bool is a real improvement. Two things I'd hold on, then some smaller items. I re-checked the round-2 review against the code and everything there still applies except the ENTRIES example, which I replied to separately.

Blocking

1. Static decode output changed, and the PR body says it didn't.

extract_solana_intermediate_output (intermediate.rs:660) now calls merge_preset_idl_configs(configs), layering 19 preset IDLs with override_builtin: true into parse_transaction_with_idls. On main that argument is caller configs only. For any transaction touching a preset program — simulation or not, opted in or not — idl_source flips BuiltIn -> Custom, idl_hash changes for the IDs that are also solana_parser builtins, and parsed_instruction_data goes None -> populated for the rest.

This is content that gets signed. The schema bump covers shape, not values, so a consumer pinned to v2 sees no signal. "static decode's output is completely unaffected either way" in the PR body needs to go, and this needs to be a deliberate, called-out change rather than a side effect of wiring up the simulation path.

Related: idl_source: "Custom" now covers both our presets and wallet-supplied IDLs, which is exactly the distinction RegisteredSource spends two fields drawing. Either add a Preset source string or document that registered_source is the field to read.

2. Four silent paths to an empty simulation.

All four produce output byte-identical to "no simulation was sent", none of them log:

• base64 ... .ok()? — core/visualsign.rs:539
• serde_json::from_slice(...).ok()? and response.value.inner_instructions? — intermediate.rs:394-397
• construct_idl_records_map(...) err -> Vec::new() — intermediate.rs:414
• UiInstruction::Compiled -> continue — intermediate.rs:~424

A caller who sends the full JSON-RPC envelope instead of the bare result, or who simulates without jsonParsed encoding, gets nothing back and no indication why. A policy engine gating on "no unregistered programs in the trace" sees an empty list and passes. Given the whole point of the feature is catching unrecognized programs, empty-because-we-couldn't-parse and empty-because-there-were-none must not be the same value.

The construct_idl_records_map case is the worst of the four: it drops the Unregistered classifications too, and those need no IDL at all to compute.

I'd reject a present-but-unparseable simulated_transaction_result outright rather than warn. The caller asked for something we couldn't deliver; failing the request is honest and failing closed is the right default on a signing path.

Also worth fixing

solana_rpc_parsed_data.parsed_json is not canonicalized. rpc_parsed.parsed.to_string() (intermediate.rs:471) serializes a serde_json::Value straight into a borsh field that lands in the signed digest. This is the same preserve_order hole #414 fixed for program_call_args_json, and canonicalize_value / canonicalize_map are already in the same module. As written the module doc's determinism claim (lines 17-23) is no longer true for the new field, and a consumer that re-serializes this JSON computes a different string than we signed.

The fixture doesn't show the feature working. In registered_source_classifications_from_jupiter_route_simulation, the Jupiter v6 CPI classifies as Preset but comes back parsed_instruction_data: None with DiscriminatorNotFound. On the one real mainnet case in the PR, per-instruction IDL decode of a preset CPI does not produce args. That's the headline capability. Worth understanding whether it's the fixture, the discriminator lookup, or the CPI instruction shape before this ships.

instruction_data_hex: bs58::decode(...).unwrap_or_default() (intermediate.rs:~433) turns a malformed base58 payload into "", indistinguishable from an instruction that genuinely has no data. Everywhere else in this file that condition surfaces as DiscriminatorNotFound.

The UiParsedInstruction::Parsed arm sets accounts: Vec::new() and instruction_data_hex: String::new() (intermediate.rs:473-476). The account list is inside parsed for that variant, so a consumer reading the structured accounts field gets an empty list for exactly the common System/SPL calls — the ones most likely to matter to a transfer policy.

Numbers on the cost. preset_idl_configs() is OnceLock-cached but merge_preset_idl_configs clones it, and the preset JSON is 2.0 MB total. That clone happens twice per intermediate request (intermediate.rs:410 and :660), plus a full construct_idl_records_map parse per request. Previously this was None in the common case. Memoize the parsed record map per program.

metadata_digest — confirmed it shifts for every Solana request: parse.rs:101 hashes borsh::to_vec(&chain_metadata) and the new optional string writes a None tag byte whether or not the caller opts in. Fails closed, and the Ethereum abi_mappings change set the precedent, so this is deploy ordering rather than a defect — but proto regeneration and the parser rollout have to ship together.

CI: ubuntu is green. Only check-cla-file fails, which looks like it just needs a rebase now that #470 merged the signers-list entry.

[prasanna-anchorage]

Correcting my own comment above: I framed swig_wallet as a counterexample, which made it sound like an anomaly. It isn't. None of the native/core presets ship an IDL either — system, spl_token, token_2022, compute_budget, associated_token_account, stakepool all have zero JSON files, and every ID in NATIVE_PROGRAM_NAMES is in the same position.

So "registered but with no IDL to decode it" is not an edge case to close, it's the majority of the trusted set, and by design — System and SPL Token decode through solana_parser's native paths or the RPC's jsonParsed output, not through an IDL. Both tests in this PR already show it: the System transfer asserts parsed_instruction_data.is_none() with the comment "native decode path, not IDL", and the two SPL Token CPIs in the Jupiter fixture carry solana_rpc_parsed_data with parsed_instruction_data: None.

That kills the "split Preset into vouched vs decodable" half of what I suggested — the split doesn't exist along preset lines, it runs through Native too. What's left is the documentation half, and it matters more than I gave it credit for: registered_source answers "do we vouch for this program ID", never "can we decode this instruction", and a policy engine that reads it as the latter is wrong about System and SPL Token before it ever reaches swig_wallet.

One concrete consequence for the simulated path. A Native program's args come only from the RPC's jsonParsed decode, since there's no IDL to fall back on. If the caller simulates with any other encoding, those instructions arrive as PartiallyDecoded, find no IDL record, and land as (None, None) — registered_source: Native, no args, no idl_parse_error, nothing to distinguish it from a program we decoded and found nothing in. That's the same silent-empty problem as the four paths in my other comment, but it survives even when everything upstream worked, and it hits the most common instructions in any transaction.

[pepe-anchor]

Reviewed with Claude on behalf of @pepe-anchor. 8 suggestion(s) of 10 drafted; 0 marked as blocking. Treat findings as suggestions, not directives.

Round 4 at b16cae66. That commit is a9e28a32 plus a rustfmt merge of main, so the code has not changed since round 3 and all eleven findings there are still open. I re-checked each against the tree rather than assuming. Not re-posting them inline, treat that review as live.

Two of them outrank most of what is below, so flagging them here: the override_builtin: true preset merge still changes idl_source and idl_hash on the static path for every transaction touching a preset (the PR body still says otherwise), and the four silent paths to an empty simulation still fail open on the exact signal this PR exists to provide.

New this round, mostly the rollout side, verified against the monorepo at a2517f7ed112:

• The schema bump has no tripwire. The HSM mirror is still at version 1, and both downstream tests are fixture- or pin-based, so neither fires on merge. Details on the constant.
• The jsonParsed arm is wholly caller-authored. Line 462, and the one I would fix first.
• The borsh round-trip test never encodes the new types, and two RegisteredSource variants have no coverage at all.

Shape suggestion, take it or leave it. The proto field and the schema bump have different blast radii and could ship separately. The proto field is safe alone: ChainMetadata.metadata is a oneof, so Ethereum's bytes are byte-identical before and after, the Solana variant grows seven bytes but nothing constructs it, and there is no copy of parser.proto in the monorepo. The schema bump wants firmware first, ideally replacing the single constant with a supported set so a decoder tolerates N and N+1.

[pepe-anchor]

Inline notes for the round-4 review above.

[pepe-anchor]

bs58::decode(...).unwrap_or_default() turns a decode failure into an empty instruction_data_hex.

When the program has an IdlRecord, the sibling call records DiscriminatorNotFound("instruction data is not valid base58") at :522, so the failure is visible. When it does not, parse_partially_decoded_instruction_idl early-returns (None, None) at :507 before the data is ever looked at, and the instruction lands with empty data and no error. That covers every Native and Unregistered program, since neither ships an IDL, so it is the common case rather than an edge one.

Extending Prasanna's bullet on this line rather than raising it fresh. Either record the failure here or keep the raw base58 string.

---

Authored by Claude on behalf of @pepe-anchor. Please flag any inaccuracies, I'm not always right.

[pepe-anchor]

This arm keeps only the caller's program and parsed JSON. accounts is Vec::new(), instruction_data_hex is String::new(), parsed_instruction_data is None, so nothing survives that a consumer could check the claim against. The sibling PartiallyDecoded arm re-derives from the base58 data; this one derives nothing.

So any instruction the caller marks jsonParsed is wholly caller-authored, while registered_source still reports whatever program_id maps to. A caller can send {"program":"system","programId":"1111...","parsed":{"type":"transfer","info":{"lamports":1}}} and the blob shows a benign System transfer, registered_source: Native, no bytes to re-derive from. Keep accounts and the base58 data at minimum, or run this arm through the same IDL re-derivation and treat the RPC decode as a hint.

---

Authored by Claude on behalf of @pepe-anchor. Please flag any inaccuracies, I'm not always right.

[pepe-anchor]

index is the outer instruction's index, so every entry in a CPI group shares it. :424 sets outer_index once per inner_instructions entry and every instruction pushed under it copies that value. With no doc comment, a consumer keying a dedup or a lookup on it will silently collide entries. outer_instruction_index, plus a line saying it is a grouping key rather than a unique one, would fix it.

---

Authored by Claude on behalf of @pepe-anchor. Please flag any inaccuracies, I'm not always right.

[pepe-anchor]

These two are always mutually exclusive in practice, but nothing in the type says so. A consumer that checks only parsed_instruction_data, the natural move since it matches the sibling SolanaIntermediateInstruction, silently misses every RPC-recognised instruction. One enum instead of two Options makes that a compile error. Low priority though: out-of-tree mirrors have to track this shape, so a doc comment may be the better trade.

---

Authored by Claude on behalf of @pepe-anchor. Please flag any inaccuracies, I'm not always right.

[pepe-anchor]

ThirdParty and CallerSupplied have no test coverage. Each has exactly one construction site, :239 and :241, and neither appears in any test module. The tests only ever assert Native, Preset and Unregistered.

Those two are the variants that decide whether policy is looking at a program we compiled in or one the caller described to us. A case for each, a ProgramType-only program and one present only in idl_mappings, would also pin down Prasanna's question about what Preset asserts.

---

Authored by Claude on behalf of @pepe-anchor. Please flag any inaccuracies, I'm not always right.

[pepe-anchor]

The bump to schema 2 exists to protect the new bytes, and two of the new types never see borsh in a test.

SolanaRpcParsedInstructionDataIo has no test occurrence at all, it appears only at :105, :168 and :478. SolanaIdlParseError is asserted on in the Jupiter test, but that path never encodes, and the two populated round-trips in core/visualsign.rs reach RegisteredSource without ever reaching an error variant. intermediate_output_round_trip_is_deterministic builds instructions: vec![], so it encodes neither.

SolanaIdlParseError is the half worth covering. It is a borsh enum with named-struct and tuple variants, materially more work for a hand-written mirror decoder than the flat structs around it, and DiscriminatorNotFound is the only variant any test touches. Worth extending the round-trip test to a populated instruction and simulated instruction carrying a non-trivial error variant.

---

Authored by Claude on behalf of @pepe-anchor. Please flag any inaccuracies, I'm not always right.

[pepe-anchor]

SolanaArgs has only idl_json_mappings and create_chain_metadata hardcodes simulated_transaction_result: None, so the CLI cannot exercise the feature while the gRPC path can.

That bites twice: parser_cli decode is the documented way to inspect a transaction locally, and it is the practical way to produce a fixture for an out-of-tree decoder. As it stands there is no way to emit a v2 blob with populated simulated instructions without going through the service. A --simulated-tx-result flag threaded like idl_json_mappings would do it.

---

Authored by Claude on behalf of @pepe-anchor. Please flag any inaccuracies, I'm not always right.

[pepe-anchor]

Two things on the bump itself.

The doc comment says "Mirrored decoders assert this value" without saying where any of them live, and CONTRIBUTING does not either. Naming them, or at least stating that out-of-tree hand-written decoders exist, is the cheapest fix here. Right now nothing in this repo tells a contributor bumping this constant who else has to move.

Second, the guarantee this comment claims does not hold for a decoder that reads the whole body before it looks at the version. Borsh is positional, so a real v2 blob misaligns on the extra per-instruction fields and dies on a short read or a trailing-bytes check well before any schema_version comparison runs. It fails closed either way, so this is diagnosability rather than safety, but the operator gets an alignment error instead of "schema_version 2 is not supported". A mirror only gets the clean error if it reads the u16 first and dispatches on it, which is also what any decoder accepting more than one version needs. Worth stating that requirement next to the constant: the field being first is what makes it possible, and nothing currently says why that ordering matters.

---

Authored by Claude on behalf of @pepe-anchor. Please flag any inaccuracies, I'm not always right.

[prasanna-anchorage]

Drive-by perf note from benchmarking the intermediate path on #472 — not a blocker, but it lands on a hot path.

merge_preset_idl_configs does preset_idl_configs().clone() (builtin_programs.rs:383), which deep-clones all 19 bundled IDL JSON strings. Those total ~2.0 MB on disk. It's called from two places — intermediate.rs:410 (simulation) and intermediate.rs:660 — and the second is the unconditional static path, so every intermediate request pays it whether or not a simulation was supplied.

For scale: on #472 I benchmarked convert_with_intermediate on real fixtures at 16–31 µs for token ops and 2.2–2.4 ms for Orca/Jupiter, after getting the intermediate's overhead down to ~0% over render-only. A 2 MB clone per request is large relative to that.

Arc doesn't quite solve it, which was my first instinct too. parse_transaction_with_idls takes custom_idls: Option<HashMap<String, CustomIdlConfig>> by value (solana-parser rev 2146929, src/solana/parser.rs:85-88), so an owned map has to be materialised at the call site no matter how the cache is stored.

The bigger win is one variant over. CustomIdl is:

pub enum CustomIdl {
    /// A pre-parsed IDL struct (avoids re-parsing)
    Parsed(Idl),
    /// An IDL as a JSON string (will be parsed)
    Json(String),
}

preset_idl_configs() currently stores CustomIdl::Json(...), so each request doesn't just clone 2 MB of text — it re-parses it downstream. Holding CustomIdl::Parsed(Idl) in the OnceLock instead parses the 19 IDLs once at first use, and the per-request clone then copies parsed structures rather than re-parsing JSON. That's the change I'd suggest here.

Killing the remaining clone entirely would need parse_transaction_with_idls to accept &HashMap. Since solana-parser is ours and pinned by rev, that seems worth a follow-up issue rather than anything blocking this PR.

Happy to send a patch for the Parsed change if useful — we're planning to stack on this branch, so I'd rather flag it now than land on top of it.

[prasanna-anchorage]

One more from re-reading the simulation path — I think this one is worth holding on, separate from the perf note above.

Reverted simulations are consumed as if they succeeded

parse_and_decode_simulated_instructions (intermediate.rs:386) reads the RPC response like this:

let response: Response<RpcSimulateTransactionResult> = serde_json::from_slice(raw_json).ok()?;
let inner_instructions = response.value.inner_instructions?;
Some(decode_inner_instructions(inner_instructions, idl_registry))

response.value.err is never read. It exists on the type — RpcSimulateTransactionResult { err: Option<TransactionError>, .. } (solana-rpc-client-types 2.3.13, src/response.rs:400) — and simulateTransaction populates it while still returning whatever innerInstructions executed before the failure.

So a simulation that reverted contributes its partial CPI trace to intermediate_output, indistinguishable from one that ran to completion.

That matters more here than it would elsewhere, because of what this field is for. intermediate_output is borsh-appended into the signed digest and consumed by a policy engine, and the stated goal of this PR is catching unrecognized programs. A reverted simulation's trace stops at the revert point, so the program set it reports is a subset of what real execution would touch — the policy engine can conclude "only known programs here" in exactly the case where it shouldn't. It's a false negative in the control itself, not just missing data.

Suggested shape: treat err.is_some() as a distinct outcome rather than a success. Either refuse to attach simulated instructions at all, or carry the error explicitly in the schema so the consumer can decide — the latter is probably more useful, and it's a natural fit next to SolanaIdlParseError.

Two smaller cases with the same shape

Both turn "we couldn't read this" into "there was nothing here":

• UiInstruction::Compiled => continue (intermediate.rs:~428) silently drops instructions, so a simulation sent without jsonParsed encoding yields an empty-but-present list that looks identical to "no CPIs".
• The three .ok()? / else { return Vec::new() } paths — base64 decode in extract_raw_simulated_instructions, serde_json::from_slice, and construct_idl_records_map — make a malformed simulation indistinguishable from no simulation supplied.

For a field that feeds policy, "absent" and "unparseable" probably shouldn't be the same value. Worth at least a tracing::warn! on each, and ideally a schema-level signal for the encoding mismatch.

Happy to be wrong on the severity of the first one if the caller contract already guarantees a successful simulation — but I couldn't find that stated anywhere, and the proto comment on simulated_transaction_result doesn't mention it.