feat(solana-indexer) PR 6: decoder account-key resolution by squadgazzz · Pull Request #4589 · cowprotocol/services

squadgazzz · 2026-07-03T09:25:03Z

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 consume
re-export InnerInstruction from wire

run() stays unimplemented!(). Instruction-data decoding and the persist path arrive in PR 7/8. No dependency on the settlement interface crate: this step is generic Solana transaction processing.

How to test

New unit tests.

claude · 2026-07-03T09:25:29Z

Claude finished @squadgazzz's task in 3m 21s —— View job

PR Review — decoder account-key resolution

The 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 filter_relevant correctly drops out-of-range indices via ?/get. Including failed transactions (no meta.err check) is consistent with the ingester's intentional failed: None filter. Nicely documented.

Three points posted inline (none blocking):

Duplication with types/tx.rs — RelevantInstruction/build_account_keys parallel the unused ResolvedInstruction/TxContext; RelevantInstruction also drops the ix_index/is_inner that ResolvedInstruction deliberately carries. Worth reconciling before PR 7 consumes it.
Zero-pubkey fallback — the "can't match a tracked program ⇒ dropped" safety note only covers the program index; a malformed account key is silently kept as the zero pubkey. Defensive-only in practice.
Test coverage — add negative tests (untracked program dropped, SolFlow matched, out-of-range index dropped) to exercise the new filter branches.

gemini-code-assist

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.

…lution

…ch real bugs

… PR references

…vantInstruction

claude · 2026-07-03T11:48:19Z

Claude finished @squadgazzz's task in 2m 23s —— View job

PR Review — decoder account-key resolution (PR 6)

Read changed files and understand the decoder logic
Review against Solana versioned-tx semantics
Check whether prior review points were addressed
Post findings

I re-reviewed against the current branch head (77d7ccc), since the PR evolved after the first pass.

The three earlier review points are now addressed:

Duplication / divergent types — RelevantInstruction was dropped; the decoder now produces the shared ResolvedInstruction from types/tx.rs. One representation, per-program dispatch in PR 7 won't be wired to the wrong one. ✅
Lost position info — ResolvedInstruction carries instruction_index + inner_index, and relevant_instructions threads both through (top-level index, None; CPI group.index, Some(offset)). The idempotency/ordering concern is resolved. ✅
Negative tests — unresolvable_programs_dropped_account_indices_carried_through now exercises out-of-range program index, wrong-length-key→zero-pubkey→untracked, and an out-of-range account index carried through without dropping the instruction. ✅

Correctness spot-checks (all good):

build_account_keys: static ⊕ ALT-writable ⊕ ALT-readonly matches Solana versioned-tx account resolution ordering; wrong-length keys collapse to the zero pubkey while preserving slot alignment (no off-by-one), and the zero pubkey can't equal a tracked program.
relevant_instructions walks inner instructions independently of whether their parent top-level instruction survived — so a settlement CPI under a dropped router call is still kept. Test 1 exercises exactly this, which is the important property.
Accounts are carried as raw indices (not resolved here), so the prior "malformed account key silently resolves to System Program" concern no longer applies within this PR — it's deferred to whoever resolves account indices in PR 7/8, and remains defensive-only given geyser always emits 32-byte keys.

Remaining notes (non-blocking, already acknowledged in-code):

The top-level-before-inner walk order isn't on-chain execution order — flagged by the existing TODO at decoder.rs:191. Fine to defer.
Failed transactions aren't filtered (no meta.err check), consistent with the ingester's intentional failed: None.

No blocking issues — LGTM. Clean, well-documented, and the tests target the branches that would regress silently.

gemini-code-assist

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.

MartinquaXD · 2026-07-03T12:16:17Z

+        settlement_program: &Pubkey,
+        solflow_program: &Pubkey,
+    ) -> Option<ResolvedInstruction> {
+        let program_id = *account_keys.get(self.program_id_index as usize)?;


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>?

In this PR the flat list only serves the program lookup, that is true. But the upcoming decode PR would resolve every instruction's account indices against it (many lookups), and it's exactly the account_keys TxContext holds. A get_account(index) method skips the allocation but re-derives which range each index falls in on every call, and we'd build the list in the decode PR anyway.

MartinquaXD · 2026-07-03T12:25:34Z

+        Some(ResolvedInstruction {
+            program_id,
+            data: Bytes::copy_from_slice(self.data),
+            accounts: self.account_indices.to_vec(),


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.

Nothing clones them yet. Probably, worth revisiting if the decode path shows real clone pressure.

MartinquaXD · 2026-07-03T12:35:20Z

+        settlement_program: &Pubkey,
+        solflow_program: &Pubkey,
+    ) -> Option<ResolvedInstruction> {
+        let program_id = *account_keys.get(self.program_id_index as usize)?;


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?

Right, it can't happen for a real tx. I assume, an out-of-range index fails account loading before execution, so the tx is never included in a block and never reaches the indexer, same as the reverting-tx case you mentioned. Keeping ? only because stream data is untrusted and a panic here is worse than a silent drop.

…e resolve

…ter comments

jmg-duarte

LGTM

One comment for discussion, not blocking

jmg-duarte · 2026-07-07T15:23:52Z

+    /// field keeps the raw account-list indices, resolving them to pubkeys is
+    /// left to the decode step. Returns `None` if the program is untracked or
+    /// its index is out of range.
    fn resolve_protocol_instruction(


With this in mind, should we (not now) make ResolvedInstruction carry typestate-like semantics?

ResolvedInstruction -> FullyResolvedInstruction

Under that decode step?

Makes sense, but I'd keep it out of this PR. ResolvedInstruction lives only in decoder.rs and account indices resolve on demand during decode, so nothing reads a pubkey before it's resolved. The typestate would guard a mistake the current flow can't make, at the cost of a second struct and an eager conversion. The doc note already mentions the two phases. Worth revisiting if it starts crossing module boundaries or decode grows multiple stages.

tilacog · 2026-07-07T13:03:54Z

+    /// Top-level instruction index. For a CPI, the top-level instruction it
+    /// runs under.
+    instruction_index: u32,
+    /// Position within the top-level instruction's inner list, or `None` for a
+    /// top-level instruction.
+    inner_index: Option<u32>,


I’m not sure how this representation captures the full depth of a Solana instruction. Solana instructions can nest up to 4 levels deep, so instruction_index + inner_index only covers the top level and the first CPI.

I was thinking we could use a Vec<u8> where the length is the instruction depth and each element is the position at that level. That would replace the two fields and handle all nesting layers uniformly.

I get that this probably carries over from the earlier types I authored in crates/solana-indexer/src/types/tx.rs#L21-L36, but it feels worth revisiting.

Also, the smart contracts team is leaning toward blocking CPI at the program level, but I don’t think that affects the indexer. It still needs to capture whatever it sees, so just mentioning it for context.

Good call, done in ce8e40a. Switched to Vec<u8>. Depth is clamped to 4, since that's Solana's cap (stack height 5) and stack_height is untrusted stream data, so a corrupt value can't force a huge allocation.

tilacog · 2026-07-07T19:20:08Z

+                .flat_map(|group| group.instructions.iter().enumerate())
+                .map(move |(offset, ix)| RawInstruction {
+                    instruction_index: index,
+                    inner_index: Some(offset as u32),
+                    program_id_index: ix.program_id_index,
+                    account_indices: &ix.accounts,
+                    data: &ix.data,
+                });


We need to consider the InnerInstruction.stack_height field here:

TransactionStatusMeta └─ inner_instructions: Vec<InnerInstructions> ← one group per top-level ix └─ InnerInstructions { index: u32, ... } ← which top-level ix this group belongs to instructions: Vec<InnerInstruction> } ← flat list of CPIs at all depths under it └─ InnerInstruction { ..., stack_height } ← the only depth signal

We can consider fixing this in future PRs (before launch, naturally).

Done in ce8e40a. relevant_instructions now reads stack_height off each InnerInstruction and rebuilds the per-level path from it (2 is a direct CPI, 3 a CPI that one made, and so on).

tilacog

Forgot to approve, since the inner ix issue can be fixed later.

MartinquaXD

my comments got addressed

feat(solana-indexer): decoder account-key resolution

f427acf

squadgazzz requested a review from a team as a code owner July 3, 2026 09:25

squadgazzz marked this pull request as draft July 3, 2026 09:25

gemini-code-assist Bot reviewed Jul 3, 2026

View reviewed changes

claude Bot reviewed Jul 3, 2026

View reviewed changes

Comment thread crates/solana-indexer/src/indexer/decoder.rs Outdated

claude Bot reviewed Jul 3, 2026

View reviewed changes

Comment thread crates/solana-indexer/src/indexer/decoder.rs Outdated

claude Bot reviewed Jul 3, 2026

View reviewed changes

Comment thread crates/solana-indexer/src/indexer/decoder/tests.rs Outdated

squadgazzz added 5 commits July 3, 2026 09:33

refactor(solana-indexer): fold decoder walk and filter into one function

8cae572

feat(solana-indexer): carry instruction position through decoder reso…

4db046e

…lution

test(solana-indexer): consolidate decoder tests to the cases that cat…

a09eb1d

…ch real bugs

docs(solana-indexer): tighten decoder comments, drop internal-doc and…

c192b1d

… PR references

refactor(solana-indexer): adopt shared ResolvedInstruction, drop Rele…

77d7ccc

…vantInstruction

squadgazzz marked this pull request as ready for review July 3, 2026 11:47

gemini-code-assist Bot reviewed Jul 3, 2026

View reviewed changes

Comment thread crates/solana-indexer/src/indexer/decoder.rs

Comment thread crates/solana-indexer/src/indexer/decoder/tests.rs

MartinquaXD reviewed Jul 3, 2026

View reviewed changes

refactor(solana-indexer): emit instructions in execution order, renam…

15f7482

…e resolve

squadgazzz changed the title ~~feat(solana-indexer): decoder account-key resolution (PR 6)~~ feat(solana-indexer) PR 6: decoder account-key resolution Jul 6, 2026

This was referenced Jul 6, 2026

refactor(solana-indexer) PR 6.2: drop account-update half, watchdog, dead-letter #4601

Merged

feat(solana-indexer) PR 7.1: wire tx-only decoder run loop #4591

Merged

jmg-duarte reviewed Jul 7, 2026

View reviewed changes

docs(solana-indexer): clarify decoder account-key and instruction-fil…

a3562c7

…ter comments

jmg-duarte approved these changes Jul 7, 2026

View reviewed changes

tilacog reviewed Jul 7, 2026

View reviewed changes

tilacog approved these changes Jul 7, 2026

View reviewed changes

Address pr comments

ce8e40a

MartinquaXD approved these changes Jul 8, 2026

View reviewed changes

squadgazzz added this pull request to the merge queue Jul 9, 2026

Merged via the queue into main with commit 6a4e356 Jul 9, 2026
22 checks passed

squadgazzz deleted the solana-indexer/PR6-decoder branch July 9, 2026 10:03

github-actions Bot locked and limited conversation to collaborators Jul 9, 2026

Uh oh!

Conversation

squadgazzz commented Jul 3, 2026

Description

Changes

How to test

Uh oh!

claude Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review — decoder account-key resolution

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review — decoder account-key resolution (PR 6)

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jmg-duarte left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tilacog left a comment

Choose a reason for hiding this comment

Uh oh!

MartinquaXD left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

claude Bot commented Jul 3, 2026 •

edited

Loading

claude Bot commented Jul 3, 2026 •

edited

Loading