Conversation
prasanna-anchorage
changed the base branch from
main
to
bug/fix-jupiter-route-discriminator
prasanna-anchorage
changed the title
Migrate Jupiter swap instruction parsing to use the IDL parser from solana-parser while maintaining exact output format compatibility. Implementation: - Added get_jupiter_idl() to fetch Jupiter v6 IDL from built-in IDLs - Added extract_u64_arg() helper to extract u64 values from IDL args - Added parse_jupiter_instruction_with_idl() to parse using IDL - Updated parse_jupiter_swap_instruction() to try IDL first, with fallback to discriminator-based parsing for robustness Benefits: - Uses correct discriminators from IDL (fixes issues from previous commit) - More maintainable - discriminators and arg structure come from IDL - Automatically handles all 3 instruction types: route, exact_out_route, shared_accounts_route - Graceful fallback to manual parsing if IDL parsing fails Testing: - All 7 Jupiter tests pass - Fixture test passes with exact output format match - Output format preserved byte-for-byte (no changes to formatting functions) - clippy and cargo fmt pass Architecture: - Kept JupiterSwapVisualizer as specialized visualizer (not merged into unknown_program) for custom output format and token resolution - Token resolution logic unchanged (accounts[0] for input, accounts[5] for output) - All formatting functions unchanged (format_jupiter_swap_instruction, create_jupiter_swap_expanded_fields) IDL Source: jupiter_agg_v6.json in solana-parser repo Program ID: JUP6LkbZbjS1jKKwapdHNy74zcZ3tLUZoi5QNyVTaV4 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…ctions Update the Unknown variant to capture and display instruction names from the IDL when an instruction exists but isn't explicitly handled by our parser. This provides better visibility into which Jupiter instructions are being used. Changes: - Update JupiterSwapInstruction::Unknown to struct variant with instruction_name field - Capture instruction_name from IDL parsing fallthrough cases - Display actual instruction name when available (e.g., 'Jupiter: setTokenLedger') - Show 'not explicitly handled' in expanded fields for known IDL instructions - Add test coverage for IDL name display and unknown discriminator handling - Replace all println! with tracing::trace! in tests Example: When a setTokenLedger instruction is encountered, instead of showing 'Jupiter: Unknown Instruction', it now shows 'Jupiter: setTokenLedger' with a status message indicating it's not explicitly handled. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…r Jupiter Remove the manual discriminator-based fallback path from the Jupiter swap parser, making it a clean single-path IDL reference implementation. Fix get_jupiter_idl() to correctly load built-in IDLs via ProgramType::idl_json() instead of an empty IdlRegistry lookup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
prasanna-anchorage
force-pushed
the
feat/jupiter-idl-migration
branch
from
94f3881 to
2dd6ff6
Compare
There was a problem hiding this comment.
Pull request overview
This pull request migrates the Jupiter swap instruction parsing from manual discriminator-based parsing to using the IDL parser from the solana-parser library. The change removes hardcoded discriminator constants and manual byte parsing in favor of using the Jupiter v6 IDL directly.
Changes:
- Removed discriminator constants and manual byte parsing logic
- Added IDL-based parsing through
parse_jupiter_instruction_with_idl() - Updated
JupiterSwapInstruction::Unknownenum variant to include optional instruction name - Modified tests to use fixture data and removed discriminator validation tests
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/mod.rs
Outdated
Show resolved
Hide resolved
src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/mod.rs
Outdated
Show resolved
Hide resolved
src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/mod.rs
Outdated
Show resolved
Hide resolved
| fn get_jupiter_idl() -> Option<Idl> { | ||
| let program_id = "JUP6LkbZbjS1jKKwapdHNy74zcZ3tLUZoi5QNyVTaV4"; | ||
|
|
||
| match discriminator { | ||
| d if d == JUPITER_ROUTE_DISCRIMINATOR => parse_route_instruction(data, accounts), | ||
| d if d == JUPITER_EXACT_OUT_ROUTE_DISCRIMINATOR => { | ||
| parse_exact_out_route_instruction(data, accounts) | ||
| } | ||
| d if d == JUPITER_SHARED_ACCOUNTS_ROUTE_DISCRIMINATOR => { | ||
| parse_shared_accounts_route_instruction(data, accounts) | ||
| } | ||
| _ => Ok(JupiterSwapInstruction::Unknown), | ||
| } | ||
| ProgramType::from_program_id(program_id).and_then(|pt| decode_idl_data(pt.idl_json()).ok()) | ||
| } |
There was a problem hiding this comment.
Potential reliability issue with IDL availability. The implementation assumes that the Jupiter IDL will always be available through ProgramType::from_program_id and decode_idl_data. If the solana-parser library is updated and the Jupiter IDL is removed or the program ID changes, all Jupiter instruction parsing will fail with no fallback mechanism. The PR description mentions "graceful degradation" but the old discriminator-based parsing has been completely removed. Consider documenting this dependency or implementing an actual fallback mechanism to handle cases where the IDL is unavailable.
| parse_jupiter_instruction_with_idl(data, accounts) | ||
| .map_err(|_| "Failed to parse Jupiter instruction with IDL") |
There was a problem hiding this comment.
The PR description states "Graceful degradation: Falls back to manual parsing if IDL parsing fails", but the implementation doesn't actually provide a fallback mechanism. When parse_jupiter_instruction_with_idl fails, it simply returns an error rather than falling back to the old discriminator-based parsing. The discriminator constants and manual parsing functions have been completely removed, so there is no fallback path available.
| parse_jupiter_instruction_with_idl(data, accounts) | ||
| .map_err(|_| "Failed to parse Jupiter instruction with IDL") |
There was a problem hiding this comment.
Loss of error context information. The parse_jupiter_instruction_with_idl function returns a detailed Box<dyn std::error::Error> that could contain specific information about what failed (e.g., "Missing or invalid argument: slippage_bps", "Jupiter IDL not available", or errors from the IDL parser), but this is being discarded and replaced with the generic message "Failed to parse Jupiter instruction with IDL". This makes debugging more difficult. Consider either propagating the error details or creating more specific error messages based on the error type.
| parse_jupiter_instruction_with_idl(data, accounts) | |
| .map_err(|_| "Failed to parse Jupiter instruction with IDL") | |
| parse_jupiter_instruction_with_idl(data, accounts).map_err(|e| { | |
| let msg = e.to_string(); | |
| if msg.contains("slippage_bps") { | |
| "Missing or invalid argument: slippage_bps" | |
| } else if msg.contains("Jupiter IDL not available") { | |
| "Jupiter IDL not available" | |
| } else { | |
| "Failed to parse Jupiter instruction with IDL" | |
| } | |
| }) |
| let program_id = "JUP6LkbZbjS1jKKwapdHNy74zcZ3tLUZoi5QNyVTaV4"; | ||
|
|
||
| match discriminator { | ||
| d if d == JUPITER_ROUTE_DISCRIMINATOR => parse_route_instruction(data, accounts), | ||
| d if d == JUPITER_EXACT_OUT_ROUTE_DISCRIMINATOR => { | ||
| parse_exact_out_route_instruction(data, accounts) | ||
| } | ||
| d if d == JUPITER_SHARED_ACCOUNTS_ROUTE_DISCRIMINATOR => { | ||
| parse_shared_accounts_route_instruction(data, accounts) | ||
| } | ||
| _ => Ok(JupiterSwapInstruction::Unknown), | ||
| } | ||
| ProgramType::from_program_id(program_id).and_then(|pt| decode_idl_data(pt.idl_json()).ok()) | ||
| } | ||
|
|
||
| fn parse_route_instruction( | ||
| data: &[u8], | ||
| accounts: &[String], | ||
| ) -> Result<JupiterSwapInstruction, &'static str> { | ||
| let (in_amount, out_amount, slippage_bps, platform_fee_bps) = | ||
| JupiterSwapInstruction::parse_amounts_and_slippage_from_data(data)?; | ||
|
|
||
| let in_token = accounts.first().map(|addr| get_token_info(addr, in_amount)); | ||
| // Account index hardcoded to 5 for output token address (destination mint) | ||
| let out_token = accounts.get(5).map(|addr| get_token_info(addr, out_amount)); | ||
|
|
||
| Ok(JupiterSwapInstruction::Route { | ||
| in_token, | ||
| out_token, | ||
| slippage_bps, | ||
| platform_fee_bps, | ||
| }) | ||
| /// Helper to extract u64 argument from parsed IDL args | ||
| fn extract_u64_arg( | ||
| args: &serde_json::Map<String, serde_json::Value>, | ||
| name: &str, | ||
| ) -> Result<u64, Box<dyn std::error::Error>> { | ||
| args.get(name) | ||
| .and_then(|v| v.as_u64()) | ||
| .ok_or_else(|| format!("Missing or invalid argument: {name}").into()) | ||
| } | ||
|
|
||
| fn parse_exact_out_route_instruction( | ||
| /// Parse Jupiter instruction using IDL-based approach | ||
| fn parse_jupiter_instruction_with_idl( | ||
| data: &[u8], | ||
| accounts: &[String], | ||
| ) -> Result<JupiterSwapInstruction, &'static str> { | ||
| let (in_amount, out_amount, slippage_bps, platform_fee_bps) = | ||
| JupiterSwapInstruction::parse_amounts_and_slippage_from_data(data)?; | ||
|
|
||
| let in_token = accounts.first().map(|addr| get_token_info(addr, in_amount)); | ||
| // Account index hardcoded to 5 for output token address (destination mint) | ||
| let out_token = accounts.get(5).map(|addr| get_token_info(addr, out_amount)); | ||
|
|
||
| Ok(JupiterSwapInstruction::ExactOutRoute { | ||
| in_token, | ||
| out_token, | ||
| slippage_bps, | ||
| platform_fee_bps, | ||
| }) | ||
| ) -> Result<JupiterSwapInstruction, Box<dyn std::error::Error>> { | ||
| let program_id = "JUP6LkbZbjS1jKKwapdHNy74zcZ3tLUZoi5QNyVTaV4"; |
There was a problem hiding this comment.
The Jupiter program ID is hardcoded in two different functions (get_jupiter_idl and parse_jupiter_instruction_with_idl). This violates the DRY principle and creates a maintenance burden. If the program ID needs to be updated (e.g., for a new version of Jupiter), it would need to be changed in multiple places. Consider extracting this to a constant at the module level.
| "exact_out_route" => { | ||
| // Note: exact_out_route uses out_amount and quoted_in_amount (reversed) | ||
| let out_amount = extract_u64_arg(&parsed.program_call_args, "out_amount")?; | ||
| let quoted_in_amount = extract_u64_arg(&parsed.program_call_args, "quoted_in_amount")?; | ||
| let slippage_bps = extract_u64_arg(&parsed.program_call_args, "slippage_bps")? as u16; | ||
| let platform_fee_bps = | ||
| extract_u64_arg(&parsed.program_call_args, "platform_fee_bps")? as u8; | ||
|
|
||
| let in_token = accounts | ||
| .first() | ||
| .map(|addr| get_token_info(addr, quoted_in_amount)); | ||
| let out_token = accounts.get(5).map(|addr| get_token_info(addr, out_amount)); | ||
|
|
||
| Ok(JupiterSwapInstruction::ExactOutRoute { | ||
| in_token, | ||
| out_token, | ||
| slippage_bps, | ||
| platform_fee_bps, | ||
| }) | ||
| } | ||
| "shared_accounts_route" => { | ||
| let in_amount = extract_u64_arg(&parsed.program_call_args, "in_amount")?; | ||
| let quoted_out_amount = | ||
| extract_u64_arg(&parsed.program_call_args, "quoted_out_amount")?; | ||
| let slippage_bps = extract_u64_arg(&parsed.program_call_args, "slippage_bps")? as u16; | ||
| let platform_fee_bps = | ||
| extract_u64_arg(&parsed.program_call_args, "platform_fee_bps")? as u8; | ||
|
|
||
| let in_token = accounts.first().map(|addr| get_token_info(addr, in_amount)); | ||
| let out_token = accounts | ||
| .get(5) | ||
| .map(|addr| get_token_info(addr, quoted_out_amount)); | ||
|
|
||
| Ok(JupiterSwapInstruction::SharedAccountsRoute { | ||
| in_token, | ||
| out_token, | ||
| slippage_bps, | ||
| platform_fee_bps, | ||
| }) | ||
| } | ||
| _ => Ok(JupiterSwapInstruction::Unknown { | ||
| instruction_name: Some(parsed.instruction_name.clone()), | ||
| }), | ||
| } |
There was a problem hiding this comment.
Missing test coverage for ExactOutRoute and SharedAccountsRoute instruction parsing. While the code implements parsing for all three instruction types (route, exact_out_route, shared_accounts_route), the tests only verify the Route instruction type. The old test_jupiter_discriminator_matching test was removed, which previously tested all three discriminators. Consider adding tests that parse actual ExactOutRoute and SharedAccountsRoute instructions to ensure the IDL-based parsing works correctly for all supported instruction types.
| fn parse_jupiter_instruction_with_idl( | ||
| data: &[u8], | ||
| accounts: &[String], | ||
| ) -> Result<JupiterSwapInstruction, &'static str> { | ||
| let (in_amount, out_amount, slippage_bps, platform_fee_bps) = | ||
| JupiterSwapInstruction::parse_amounts_and_slippage_from_data(data)?; | ||
|
|
||
| let in_token = accounts.first().map(|addr| get_token_info(addr, in_amount)); | ||
| // Account index hardcoded to 5 for output token address (destination mint) | ||
| let out_token = accounts.get(5).map(|addr| get_token_info(addr, out_amount)); | ||
|
|
||
| Ok(JupiterSwapInstruction::ExactOutRoute { | ||
| in_token, | ||
| out_token, | ||
| slippage_bps, | ||
| platform_fee_bps, | ||
| }) | ||
| ) -> Result<JupiterSwapInstruction, Box<dyn std::error::Error>> { | ||
| let program_id = "JUP6LkbZbjS1jKKwapdHNy74zcZ3tLUZoi5QNyVTaV4"; | ||
| let idl = get_jupiter_idl().ok_or("Jupiter IDL not available")?; | ||
|
|
||
| // Parse using solana_parser | ||
| let parsed = parse_instruction_with_idl(data, program_id, &idl)?; | ||
|
|
||
| // Extract instruction type and arguments | ||
| match parsed.instruction_name.as_str() { | ||
| "route" => { | ||
| let in_amount = extract_u64_arg(&parsed.program_call_args, "in_amount")?; | ||
| let quoted_out_amount = | ||
| extract_u64_arg(&parsed.program_call_args, "quoted_out_amount")?; | ||
| let slippage_bps = extract_u64_arg(&parsed.program_call_args, "slippage_bps")? as u16; | ||
| let platform_fee_bps = | ||
| extract_u64_arg(&parsed.program_call_args, "platform_fee_bps")? as u8; | ||
|
|
||
| // Get token info (preserve current logic) | ||
| let in_token = accounts.first().map(|addr| get_token_info(addr, in_amount)); | ||
| let out_token = accounts | ||
| .get(5) | ||
| .map(|addr| get_token_info(addr, quoted_out_amount)); | ||
|
|
||
| Ok(JupiterSwapInstruction::Route { | ||
| in_token, | ||
| out_token, | ||
| slippage_bps, | ||
| platform_fee_bps, | ||
| }) | ||
| } | ||
| "exact_out_route" => { | ||
| // Note: exact_out_route uses out_amount and quoted_in_amount (reversed) | ||
| let out_amount = extract_u64_arg(&parsed.program_call_args, "out_amount")?; | ||
| let quoted_in_amount = extract_u64_arg(&parsed.program_call_args, "quoted_in_amount")?; | ||
| let slippage_bps = extract_u64_arg(&parsed.program_call_args, "slippage_bps")? as u16; | ||
| let platform_fee_bps = | ||
| extract_u64_arg(&parsed.program_call_args, "platform_fee_bps")? as u8; | ||
|
|
||
| let in_token = accounts | ||
| .first() | ||
| .map(|addr| get_token_info(addr, quoted_in_amount)); | ||
| let out_token = accounts.get(5).map(|addr| get_token_info(addr, out_amount)); | ||
|
|
||
| Ok(JupiterSwapInstruction::ExactOutRoute { | ||
| in_token, | ||
| out_token, | ||
| slippage_bps, | ||
| platform_fee_bps, | ||
| }) | ||
| } | ||
| "shared_accounts_route" => { | ||
| let in_amount = extract_u64_arg(&parsed.program_call_args, "in_amount")?; | ||
| let quoted_out_amount = | ||
| extract_u64_arg(&parsed.program_call_args, "quoted_out_amount")?; | ||
| let slippage_bps = extract_u64_arg(&parsed.program_call_args, "slippage_bps")? as u16; | ||
| let platform_fee_bps = | ||
| extract_u64_arg(&parsed.program_call_args, "platform_fee_bps")? as u8; | ||
|
|
||
| let in_token = accounts.first().map(|addr| get_token_info(addr, in_amount)); | ||
| let out_token = accounts | ||
| .get(5) | ||
| .map(|addr| get_token_info(addr, quoted_out_amount)); | ||
|
|
||
| Ok(JupiterSwapInstruction::SharedAccountsRoute { | ||
| in_token, | ||
| out_token, | ||
| slippage_bps, | ||
| platform_fee_bps, | ||
| }) | ||
| } | ||
| _ => Ok(JupiterSwapInstruction::Unknown { | ||
| instruction_name: Some(parsed.instruction_name.clone()), | ||
| }), | ||
| } |
There was a problem hiding this comment.
Implicit dependency on IDL argument names. The code hardcodes specific argument names (e.g., "in_amount", "quoted_out_amount", "slippage_bps", "platform_fee_bps") when extracting values from the IDL-parsed data. If the Jupiter IDL is updated with different argument names, all parsing will fail. Consider adding documentation or comments noting this dependency on the specific IDL structure, or adding more defensive error handling that provides clear error messages when expected arguments are missing.
Extract program ID to module-level constant, replace lossy `as` casts with `try_from`, and gracefully degrade to Unknown variant (with tracing::warn) instead of returning an error on IDL parse failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover the two remaining Jupiter swap instruction variants end-to-end through parse_jupiter_swap_instruction, verifying reversed amount fields for ExactOutRoute and the extra leading id byte for SharedAccountsRoute. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Migrate Jupiter swap instruction parsing to use the IDL parser from solana-parser while maintaining exact output format compatibility.
Stacked on: #142 (discriminator fix)
Implementation
get_jupiter_idl()to fetch Jupiter v6 IDL from built-in IDLs in solana-parserextract_u64_arg()helper to extract u64 values from IDL parsed argumentsparse_jupiter_instruction_with_idl()to parse instructions using IDLparse_jupiter_swap_instruction()to try IDL parsing first, with graceful fallback to discriminator-based parsingBenefits
Testing
✅ All 7 Jupiter tests pass
✅ Fixture test passes with exact output format match (real mainnet transaction)
✅ Output format preserved byte-for-byte (no changes to formatting functions)
✅
cargo clippy --all-targets -- -D warningspasses✅
cargo fmtpassesArchitecture Decisions
Kept JupiterSwapVisualizer as specialized visualizer (not merged into unknown_program):
Preserved existing logic:
format_jupiter_swap_instruction,create_jupiter_swap_expanded_fields)Files Changed
src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/mod.rsIDL Source
jupiter_agg_v6.jsonin solana-parser repoJUP6LkbZbjS1jKKwapdHNy74zcZ3tLUZoi5QNyVTaV4ProgramType::from_program_id()andIdlRegistry::get_idl()