feat(solana-indexer): decoder account-key resolution (PR 6)#4589
feat(solana-indexer): decoder account-key resolution (PR 6)#4589squadgazzz wants to merge 6 commits into
Conversation
|
Claude finished @squadgazzz's task in 3m 21s —— View job PR Review — decoder account-key resolutionThe core logic is correct: account-list reconstruction (static ⊕ ALT-writable ⊕ ALT-readonly) matches Solana's versioned-transaction ordering, the top-level→inner instruction walk covers CPIs, and Three points posted inline (none blocking):
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements transaction instruction decoding and filtering logic for the Solana indexer. It introduces helper structures and functions to build full account lists (including address lookup tables), walk through both top-level and inner instructions, and filter instructions relevant to specific programs (settlement and SolFlow). Unit tests are also added to verify these decoding behaviors. No critical issues found.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Claude finished @squadgazzz's task in 2m 23s —— View job PR Review — decoder account-key resolution (PR 6)
I re-reviewed against the current branch head ( The three earlier review points are now addressed:
Correctness spot-checks (all good):
Remaining notes (non-blocking, already acknowledged in-code):
No blocking issues — LGTM. Clean, well-documented, and the tests target the branches that would regress silently. |
There was a problem hiding this comment.
Code Review
This pull request implements instruction decoding and filtering logic for the Solana indexer, adding helper functions to reconstruct account keys and extract relevant instructions for the settlement and SolFlow programs, along with corresponding unit tests. The review feedback correctly identifies that chaining top-level instructions before inner instructions violates the actual on-chain execution order, which could lead to state inconsistencies for downstream consumers. The reviewer provides actionable code suggestions to interleave these instructions in their exact execution sequence and update the unit tests accordingly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| fn relevant_instructions( | ||
| tx: &SubscribeUpdateTransactionInfo, | ||
| settlement_program: &Pubkey, | ||
| solflow_program: &Pubkey, | ||
| ) -> Vec<ResolvedInstruction> { | ||
| let account_keys = build_account_keys(tx); | ||
| let top_level = tx | ||
| .transaction | ||
| .as_ref() | ||
| .and_then(|transaction| transaction.message.as_ref()) | ||
| .map(|message| message.instructions.as_slice()) | ||
| .unwrap_or_default() | ||
| .iter() | ||
| .enumerate() | ||
| .map(|(index, ix)| RawInstruction { | ||
| instruction_index: index as u32, | ||
| inner_index: None, | ||
| program_id_index: ix.program_id_index, | ||
| account_indices: &ix.accounts, | ||
| data: &ix.data, | ||
| }); | ||
| let inner = tx | ||
| .meta | ||
| .as_ref() | ||
| .map(|meta| meta.inner_instructions.as_slice()) | ||
| .unwrap_or_default() | ||
| .iter() | ||
| .flat_map(|group| { | ||
| group | ||
| .instructions | ||
| .iter() | ||
| .enumerate() | ||
| .map(move |(offset, ix)| RawInstruction { | ||
| instruction_index: group.index, | ||
| inner_index: Some(offset as u32), | ||
| program_id_index: ix.program_id_index, | ||
| account_indices: &ix.accounts, | ||
| data: &ix.data, | ||
| }) | ||
| }); | ||
| // TODO: top-level instructions come before inner ones here, which is not the | ||
| // on-chain execution order. Revisit if ordering across the two matters. | ||
| top_level | ||
| .chain(inner) | ||
| .filter_map(|raw| raw.resolve(&account_keys, settlement_program, solflow_program)) | ||
| .collect() | ||
| } |
There was a problem hiding this comment.
The current implementation chains all top-level instructions before all inner instructions, which violates the actual on-chain execution order. For downstream consumers (like the dispatch and persist steps) that rely on causal ordering, this can lead to state inconsistencies or logic bugs. Interleave the top-level and inner instructions so they are returned in their exact execution sequence.
fn relevant_instructions(
tx: &SubscribeUpdateTransactionInfo,
settlement_program: &Pubkey,
solflow_program: &Pubkey,
) -> Vec<ResolvedInstruction> {
let account_keys = build_account_keys(tx);
let instructions = tx
.transaction
.as_ref()
.and_then(|transaction| transaction.message.as_ref())
.map(|message| message.instructions.as_slice())
.unwrap_or_default();
let inner_groups = tx
.meta
.as_ref()
.map(|meta| meta.inner_instructions.as_slice())
.unwrap_or_default();
let mut result = Vec::new();
for (index, ix) in instructions.iter().enumerate() {
let index_u32 = index as u32;
let raw_top = RawInstruction {
instruction_index: index_u32,
inner_index: None,
program_id_index: ix.program_id_index,
account_indices: &ix.accounts,
data: &ix.data,
};
if let Some(resolved) = raw_top.resolve(&account_keys, settlement_program, solflow_program) {
result.push(resolved);
}
if let Some(group) = inner_groups.iter().find(|g| g.index == index_u32) {
for (offset, inner_ix) in group.instructions.iter().enumerate() {
let raw_inner = RawInstruction {
instruction_index: index_u32,
inner_index: Some(offset as u32),
program_id_index: inner_ix.program_id_index,
account_indices: &inner_ix.accounts,
data: &inner_ix.data,
};
if let Some(resolved) = raw_inner.resolve(&account_keys, settlement_program, solflow_program) {
result.push(resolved);
}
}
}
}
result
}| let relevant = relevant_instructions(&tx, &settlement, &solflow); | ||
|
|
||
| // router dropped, solflow (top-level) and settlement (CPI) kept, in order. | ||
| assert_eq!(relevant.len(), 2); | ||
|
|
||
| assert_eq!(relevant[0].program_id, solflow); | ||
| assert_eq!(relevant[0].instruction_index, 1); | ||
| assert_eq!(relevant[0].inner_index, None); | ||
| assert_eq!(relevant[0].accounts, vec![1, 4]); | ||
| assert_eq!(relevant[0].data, Bytes::from(vec![1, 2, 3])); | ||
|
|
||
| assert_eq!(relevant[1].program_id, settlement); | ||
| assert_eq!(relevant[1].instruction_index, 0); | ||
| assert_eq!(relevant[1].inner_index, Some(0)); | ||
| assert_eq!(relevant[1].accounts, vec![1]); | ||
| assert_eq!(relevant[1].data, Bytes::from(vec![7])); | ||
| } |
There was a problem hiding this comment.
Update the test assertions to reflect the correct interleaved execution order of instructions (CPI under top-level instruction 0 should be resolved before top-level instruction 1).
| let relevant = relevant_instructions(&tx, &settlement, &solflow); | |
| // router dropped, solflow (top-level) and settlement (CPI) kept, in order. | |
| assert_eq!(relevant.len(), 2); | |
| assert_eq!(relevant[0].program_id, solflow); | |
| assert_eq!(relevant[0].instruction_index, 1); | |
| assert_eq!(relevant[0].inner_index, None); | |
| assert_eq!(relevant[0].accounts, vec![1, 4]); | |
| assert_eq!(relevant[0].data, Bytes::from(vec![1, 2, 3])); | |
| assert_eq!(relevant[1].program_id, settlement); | |
| assert_eq!(relevant[1].instruction_index, 0); | |
| assert_eq!(relevant[1].inner_index, Some(0)); | |
| assert_eq!(relevant[1].accounts, vec![1]); | |
| assert_eq!(relevant[1].data, Bytes::from(vec![7])); | |
| } | |
| let relevant = relevant_instructions(&tx, &settlement, &solflow); | |
| // router dropped, settlement (CPI) and solflow (top-level) kept, in actual execution order. | |
| assert_eq!(relevant.len(), 2); | |
| assert_eq!(relevant[0].program_id, settlement); | |
| assert_eq!(relevant[0].instruction_index, 0); | |
| assert_eq!(relevant[0].inner_index, Some(0)); | |
| assert_eq!(relevant[0].accounts, vec![1]); | |
| assert_eq!(relevant[0].data, Bytes::from(vec![7])); | |
| assert_eq!(relevant[1].program_id, solflow); | |
| assert_eq!(relevant[1].instruction_index, 1); | |
| assert_eq!(relevant[1].inner_index, None); | |
| assert_eq!(relevant[1].accounts, vec![1, 4]); | |
| assert_eq!(relevant[1].data, Bytes::from(vec![1, 2, 3])); | |
| } |
References
- When writing tests, if a specific trade is performed under controlled conditions, it is acceptable to directly access its elements (e.g.,
trades[0]) without additional checks, as the test setup guarantees its existence and properties.
| settlement_program: &Pubkey, | ||
| solflow_program: &Pubkey, | ||
| ) -> Option<ResolvedInstruction> { | ||
| let program_id = *account_keys.get(self.program_id_index as usize)?; |
There was a problem hiding this comment.
AFAICS accessing an account by index is the only reason why the accounts got flattened into a separate vector in the first place. Wouldn't it make more sense to just implement SubscribeUpdateTransactionInfo::get_account(u32) -> Option<&T>?
| Some(ResolvedInstruction { | ||
| program_id, | ||
| data: Bytes::copy_from_slice(self.data), | ||
| accounts: self.account_indices.to_vec(), |
There was a problem hiding this comment.
I didn't spot this earlier but it might make sense to already store those accounts as Arc<[T]>. This assumes that this data profits from cheap clones and doesn't get modified often.
| settlement_program: &Pubkey, | ||
| solflow_program: &Pubkey, | ||
| ) -> Option<ResolvedInstruction> { | ||
| let program_id = *account_keys.get(self.program_id_index as usize)?; |
There was a problem hiding this comment.
Is it possible that we don't find the account of a given index? I imagine you can craft a transaction which just does not contain all the accounts it needs but would such a transaction actually reach the indexer here or would this be similar to a reverting ethereum transaction where reasoning about it doesn't make sense because it didn't even alter the chain state?
| /// if that program is the settlement or SolFlow program. Account indices | ||
| /// are carried through unresolved. Returns `None` if the program is | ||
| /// untracked or its index is out of range. | ||
| fn resolve( |
There was a problem hiding this comment.
Should the name perhaps convey that it drops particular instructions (e.g. resolve_protocol_instruction())?
2 participants
Description
The decoder must find, in each streamed transaction, the instructions that call the settlement or SolFlow program, including CPIs made from another program. Solana names an instruction's program and accounts by index into the transaction's account list, and versioned transactions split that list across the static keys plus the address-lookup-table loaded writable and readonly addresses. This PR resolves those indices: rebuild the full account list, walk every instruction (top-level and inner), and keep the ones whose program resolves to a program we track.
Changes
build_account_keys: static keys, then ALT-loaded writable, then ALT-loaded readonly, concatenated in that fixed order (§6.3.1.a)walk_instructions: top-level instructions followed by inner/CPI instructions (§6.3.1.b)filter_relevant: resolve each instruction's program and accounts against the account list, keep only settlement/SolFlow, drop instructions with out-of-range index references (§6.3.1.c)RelevantInstruction { program, accounts, data }: the resolved unit PR 7's per-program dispatch will consumeInnerInstructionfromwirerun()staysunimplemented!(). Instruction-data decoding and the persist path arrive in PR 7/8. No dependency on the settlementinterfacecrate: this step is generic Solana transaction processing.How to test
New unit tests.