fix(lib): handle CPI-only accounts in inner instruction reconstruction#384
Conversation
Extend account keys dynamically when reconstructing inner instructions from parsed RPC simulation data. CPI PDA signers and invoked programs not present in the outer transaction's account keys were silently dropped, causing validation failures for DeFi protocols like Kamino. Also fixes burn instruction parsing when Solana RPC omits the mint field from non-checked burn JSON.
Greptile SummaryThis PR fixes inner-instruction reconstruction for DeFi protocols (Kamino, Jupiter) where CPI calls reference accounts — program IDs and PDA authorities — that are absent from the outer transaction's account-key list. The approach is sound: Key changes:
Issues found:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller as get_inner_instructions()
participant VTR as VersionedTransactionResolved
participant IxUtils
participant RPC
Caller->>VTR: get_inner_instructions(rpc_client)
VTR->>RPC: simulate_transaction (inner_instructions=true)
RPC-->>VTR: UiInnerInstructions[]
VTR->>VTR: extended_account_keys = self.all_account_keys.clone()
loop for each inner instruction
alt UiInstruction::Compiled
VTR->>VTR: push CompiledInstruction directly
else UiInstruction::Parsed (PartiallyDecoded)
VTR->>IxUtils: reconstruct_instruction_from_ui(&mut extended_account_keys)
alt program_id in extended_account_keys
IxUtils->>IxUtils: use existing index
else program_id is CPI-only
IxUtils->>IxUtils: append program_id → new index
end
loop for each account in partial.accounts
alt account in extended_account_keys
IxUtils->>IxUtils: use existing index
else CPI PDA not in outer keys
IxUtils->>IxUtils: append PDA → new index
end
end
IxUtils-->>VTR: CompiledInstruction
end
end
VTR->>IxUtils: uncompile_instructions(&extended_account_keys)
IxUtils-->>Caller: Vec<Instruction>
Last reviewed commit: 3385321 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let program_idx = if let Some(&idx) = account_keys_hashmap.get(&program_id) | ||
| { | ||
| idx | ||
| } else { | ||
| let idx = all_account_keys.len() as u8; | ||
| all_account_keys.push(program_id); | ||
| account_keys_hashmap.insert(program_id, idx); | ||
| idx | ||
| }; |
There was a problem hiding this comment.
Potential u8 index overflow when extending account keys
all_account_keys.len() as u8 performs a silent truncating cast. If the accumulated extended_account_keys vec ever reaches 256 entries (the Solana account-index hard limit), this cast wraps to 0, assigning the new CPI account the same index as the first account in the transaction. Any subsequent uncompile_instructions call would then resolve this index to the wrong Pubkey.
While practically unlikely — Solana transactions rarely have anywhere near 256 unique accounts — the correct pattern is to fail loudly rather than produce a silently corrupted index:
| let program_idx = if let Some(&idx) = account_keys_hashmap.get(&program_id) | |
| { | |
| idx | |
| } else { | |
| let idx = all_account_keys.len() as u8; | |
| all_account_keys.push(program_id); | |
| account_keys_hashmap.insert(program_id, idx); | |
| idx | |
| }; | |
| let idx = match u8::try_from(all_account_keys.len()) { | |
| Ok(i) => i, | |
| Err(_) => { | |
| log::error!("Account key index overflow: too many CPI accounts"); | |
| return None; | |
| } | |
| }; |
The same issue exists on line 442 inside the per-account loop and should be fixed with the same pattern.
| // CPI PDA signers (e.g. Kamino lending authority) | ||
| // are not in the outer transaction's account keys. | ||
| // Extend the keys vec so uncompile_instructions | ||
| // can resolve the index back to the pubkey. | ||
| let idx = all_account_keys.len() as u8; | ||
| all_account_keys.push(pubkey); | ||
| account_keys_hashmap.insert(pubkey, idx); | ||
| idx |
There was a problem hiding this comment.
Same u8 overflow risk inside per-account loop
Same truncating cast pattern as the program-ID block above (line ~422). For completeness, this path also needs the u8::try_from guard so an overflowing CPI account early-exits instead of silently mapping to index 0.
| // CPI PDA signers (e.g. Kamino lending authority) | |
| // are not in the outer transaction's account keys. | |
| // Extend the keys vec so uncompile_instructions | |
| // can resolve the index back to the pubkey. | |
| let idx = all_account_keys.len() as u8; | |
| all_account_keys.push(pubkey); | |
| account_keys_hashmap.insert(pubkey, idx); | |
| idx | |
| let idx = match u8::try_from(all_account_keys.len()) { | |
| Ok(i) => i, | |
| Err(_) => { | |
| log::error!("Account key index overflow: too many CPI accounts"); | |
| return None; | |
| } | |
| }; |
| } else if let Ok(mint) = Self::get_field_as_pubkey(info, PARSED_DATA_FIELD_MINT) { | ||
| // Some RPC providers may include mint for non-checked burn | ||
| let mint_idx = Self::get_account_index(account_keys_hashmap, &mint)?; | ||
| vec![account_idx, mint_idx, authority_idx] | ||
| } else { | ||
| // Standard Solana RPC doesn't include mint in parsed "burn" | ||
| // (non-checked) JSON, so we reconstruct with 2 accounts | ||
| vec![account_idx, authority_idx] |
There was a problem hiding this comment.
Misleading comment — standard Solana parser includes mint for non-checked burn
The comment says "Standard Solana RPC doesn't include mint in parsed 'burn' (non-checked) JSON", but the updated tests (test_reconstruct_spl_token_burn_instruction, test_reconstruct_token2022_burn_instruction) prove the opposite: parse_instruction::parse() for a non-checked Burn produces a parsed JSON that does include the mint field, so the else if branch is the primary path, not the fallback.
The comment on the else branch is also misleading — the no-mint case is an edge case (e.g. an older or unusual RPC), not the standard behaviour. Consider swapping the framing:
| } else if let Ok(mint) = Self::get_field_as_pubkey(info, PARSED_DATA_FIELD_MINT) { | |
| // Some RPC providers may include mint for non-checked burn | |
| let mint_idx = Self::get_account_index(account_keys_hashmap, &mint)?; | |
| vec![account_idx, mint_idx, authority_idx] | |
| } else { | |
| // Standard Solana RPC doesn't include mint in parsed "burn" | |
| // (non-checked) JSON, so we reconstruct with 2 accounts | |
| vec![account_idx, authority_idx] | |
| } else if let Ok(mint) = Self::get_field_as_pubkey(info, PARSED_DATA_FIELD_MINT) { | |
| // Solana's parser includes the mint for non-checked burn; reconstruct | |
| // with the standard 3-account layout [source, mint, authority]. | |
| let mint_idx = Self::get_account_index(account_keys_hashmap, &mint)?; | |
| vec![account_idx, mint_idx, authority_idx] | |
| } else { | |
| // Defensive fallback: some RPC providers may omit mint in the | |
| // parsed "burn" JSON; reconstruct with 2 accounts [source, authority]. | |
| vec![account_idx, authority_idx] |
1 participant
Summary
Fix CPI inner instruction reconstruction to support DeFi protocols (Kamino, Jupiter) where inner instructions reference accounts not present in the outer transaction's account keys.
Changes
reconstruct_instruction_from_uinow accepts&mut Vec<Pubkey>and dynamically extends the keys vector when it encounters unknown program IDs or PDA accounts from CPI inner instructions, instead of silently dropping themversioned_transaction.rs— Clone and extend account keys during inner instruction processing souncompile_instructionscan resolve CPI-only indiceskora.tomlproduction defaults — Add DeFi programs (Kamino Lending/Vaults/Farming, Jupiter Aggregator/DCA, Memo), switch to Jupiter price source, set fee payer policy to restrictive (false) defaults, add mainnet/devnet USDC and SOL tokens, blockpermanent_delegatemint extensionTesting
test_reconstruct_partially_decoded_with_cpi_pda_accounts— verifies unknown PDA accounts are extended into the keys vector with correct synthetic indicestest_reconstruct_partially_decoded_with_unknown_program_id— verifies unknown CPI program IDs are also extended correctlyreconstruct_instruction_from_uicall sites in tests to use&mutsignature