feat(solana-indexer): PR 7.3 decode settlements via the shared interface parser by squadgazzz · Pull Request #4666 · cowprotocol/services

squadgazzz · 2026-07-24T08:08:20Z

Description

The settlement decoder hand-rolled the BeginSettle and FinalizeSettle wire layout and stubbed the auction id and the buy-side amounts. It had to: the shared settlement-interface parser needed an on-chain AccountView, which the indexer cannot get off-chain. cowprotocol/solana-programs#80 makes that parser generic over the account type, so the indexer now decodes with its own Pubkey type and drops the duplicated layout and the placeholders.

Changes

Decode CreateOrder, CreateBuffer, BeginSettle, and FinalizeSettle through the interface parser.
Fill the real auction id from the parsed BeginSettle (was a placeholder).
Fill the real buy-side amount from the FinalizeSettle pushes (was a placeholder).
Read transfer amounts little-endian to match the on-chain encoder (was big-endian).
Pair pushes to orders positionally, order i is paid by push i, per the on-chain invariant.
Drop the hand-rolled parse_begin_settle_orders and the account-position constants.

How to test

Updated unit tests.

…vior

claude · 2026-07-27T06:53:49Z

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

PR Review: decode settlements via the shared interface parser

Read the PR diff and changed files
Understand the new interface parser integration
Review decoder.rs changes for correctness
Review tests
Post inline comments and summary

This is a clean simplification — dropping the hand-rolled BeginSettle/FinalizeSettle layout and the account-position constants in favor of the now-generic settlement_interface parser is the right move, and it correctly removes the auction-id / buy-amount placeholders. The little-endian fix and the generic Pubkey bridge (to_sdk_pubkey) look right, and the tests were updated to match the real wire.

One substantive point, left as an inline comment:

Silent zip truncation in decode_settlements_finalized. Pairing orders to pushes by position relies on the "exactly one push per order, same PDA order" invariant (acknowledged in the PR body). But orders.iter().zip(received) silently truncates on a count mismatch, which is inconsistent with the rest of this function's warn-and-skip / untrusted-stream posture. A pushes.len() == orders.len() guard would surface a broken invariant instead of emitting a misaligned settlement.

Nits I did not flag (already acknowledged / fine): the Cargo.toml branch = pin is called out in your own review comment as temporary until solana-programs#80 merges; the auction_id as i64 -> u64 cast is documented as non-negative in practice.

I couldn't compile locally (the settlement-interface git dependency isn't fetchable in this sandbox), so I reviewed the parser integration by reasoning about the diff rather than by building — worth confirming cargo check / the updated unit tests pass in CI.
• solana-indexer/PR7.3-interface-parser

github-actions · 2026-08-04T00:25:38Z

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

The parsing-generic branch is merged and its ref deleted, so the branch pin would break the next cargo update. Main's interface types the push amount as u64, dropping the byte-array decode on our side.

MartinquaXD · 2026-08-05T07:15:45Z

+/// `SettlementFinalized`: the auction id read from the begin wire, the summed
+/// sell amount, the buy-side push amount paired to its order by position
+/// (order `i` is paid by push `i`), the order UID from the injected resolver,
+/// and the solver read as the fee payer.


This is super hard to understand. Seems like a bullet list where each point is a bit more verbose would be better. If that is even needed here.

True, reworded.

MartinquaXD · 2026-08-05T07:17:35Z

-    // BeginSettle body: finalize index 1, one order, bump 0xAA, two transfers of
-    // 300 and 700 (sum 1000 = the sell-side amount withdrawn).
+    // BeginSettle body: finalize index 1, auction id 4242, one order, bump 0xAA,
+    // two transfers of 300 and 700 (sum 1000 = the sell-side amount withdrawn).


Why do we sum those amounts together? Aren't they separate things? Can it happen that the associated orders don't have the same tokens?

Each order inside the BeginSettle instruction has its own list of transfers, all taking tokens from that order's sell token account, and the program lets the solver pick any destinations for them. So the sum is one order's total. For example, an order sells 1000 USDC, the solver sends 700 to its own account for the swap and 300 to the buffer as a fee or something. The on-chain counter for that order grows by 1000 either way, and the TradeDelta must match it. A second order is a separate entry with its own list, so amounts of different orders never sum together.

Updated the comment

MartinquaXD · 2026-08-05T07:20:15Z

+    begin_data.extend_from_slice(&1u16.to_le_bytes());
+    begin_data.extend_from_slice(&4242i64.to_le_bytes());


I thought we would have a proper crate for constructing those interactions. Is this not done yet?

The API was pretty unstable, but right now, we can probably try switching to that. Updated in ad3c4e2.

MartinquaXD · 2026-08-05T07:28:01Z

            SettlementInstruction::BeginSettle | SettlementInstruction::FinalizeSettle => {
                Ok(Vec::new())
            }
+            // No domain event.


what does this comment mean? Do we not care about this event - and if so why?
Also why does it make sense for this variant to get its own branch instead of being added to the branch above which also returns Ok(Vec::new())?

Initialize is the one-time setup instruction that creates the program's state account at deployment. IIUC, nothing order-related happens there, so there is nothing to store. ReclaimOrder closes an order account, and we probably should store that as an OrderClosed event, but that mapping is not built, so for now it also emits nothing. Added a TODO comment.

MartinquaXD · 2026-08-05T07:54:39Z

+        let Some(finalize) = instructions.iter().find(|instruction| {
+            instruction.instruction_index == u32::from(begin_input.finalize_ix_index)


Why do we iterate over the whole instruction vector when the begin settle data already tells us the index we should look at? Is it not guaranteed that instructions[n].instruction_index == n?

As I understand, the vec only holds our program's instructions, so positions shift. For example, the transaction is [0: ComputeBudget, 1: BeginSettle, 2: Jupiter swap, 3: FinalizeSettle]. Our filtered vec is [BeginSettle, FinalizeSettle] at positions 0 and 1, but begin says finalize_ix_index = 3, its position in the original transaction. So instructions[3] would be out of bounds, and the find matches the stored instruction_index field instead. With just 2-3 entries, a map is probably not worth it.

MartinquaXD · 2026-08-05T08:01:32Z

+            .pushes
+            .iter()
+            .map(|push| push.amount)
+            .collect();


Does not look like this collect is actually needed.

Makes sense, dropped.

MartinquaXD · 2026-08-05T08:04:53Z

+            .iter()
+            .zip(received)
+            .filter_map(|(order, amount_received_delta)| {
+                let resolved = resolve_order(order.order_pda)?;


was already here before the PR but it's unclear to me why resolve_order needs to be passed in a a closure instead of being a normal function call.

Since the instruction only contains the order's PDA address, to build a TradeDelta, we also need the order UID, and that lives in our database. So decode_settlement takes a "give me the order for this PDA" function as a parameter. For tests, a two-line stub is used, while for production, we will pass a DB query. If it called the DB directly, every decoder test would need a running Postgres.

MartinquaXD · 2026-08-05T08:05:37Z

+            .filter_map(|(order, amount_received_delta)| {
+                let resolved = resolve_order(order.order_pda)?;
+                // Sell-side pull total. Amounts are little-endian `u64`, and
+                // the stream is untrusted, so saturate instead of wrapping.


bit odd to point out that the stream is untrusted when zip already assumes that the pairings match up correctly.

Overflow now invalidates the pair, see below.

MartinquaXD · 2026-08-05T08:07:20Z

+                    .amounts
+                    .iter()
+                    .map(|amount| u64::from_le_bytes(*amount))
+                    .fold(0u64, u64::saturating_add);


if we overflow the u64 we should surface an error, no?

Yeah, it now skips the pair with a warning instead.

…runcating The order/push zip silently truncated to the shorter side and the pull sum saturated. Both are layout violations, so the pair is skipped with a warning. Also merges the no-event instruction arms, drops the intermediate push collect, and inlines the test instruction helper.

The CreateOrder and BeginSettle/FinalizeSettle fixtures go through the client crate's builders as a dev-dependency, so the tests round-trip the real encoder into our parser instead of hand-rolling bytes. The invalid-instruction fixtures stay hand-rolled: they craft data no builder would produce, and they double as the wire-layout pin.

MartinquaXD · 2026-08-07T07:14:44Z

+        if corrupt {
+            continue;
+        }


You can avoid this by assigning a label to the outer loop with:

'process_instructions: for begin in instructions {

and then directly continuing that loop with:

continue 'process_instructions;

Makes sense. Updated.

MartinquaXD · 2026-08-07T07:38:39Z

+        auction_id: 4242,
+        orders: &[settlement_client::instructions::InitializedIntent {
+            intent: &intent,
+            pulls: &[


The language changed from push to pull but the test comments still reference push which is confusing.

Good catch! Reworded.

linear-code · 2026-08-07T07:41:19Z

BE-63

…the test doc

refactor(solana-indexer): decode settlements via shared interface parser

8ebc951

squadgazzz changed the title ~~solana-indexer: decode settlements via the shared interface parser~~ solana-indexer: PR 7.3 decode settlements via the shared interface parser Jul 24, 2026

squadgazzz mentioned this pull request Jul 24, 2026

Make parsing generic on accounts cowprotocol/solana-programs#80

Merged

squadgazzz changed the title ~~solana-indexer: PR 7.3 decode settlements via the shared interface parser~~ feat(solana-indexer): PR 7.3 decode settlements via the shared interface parser Jul 24, 2026

docs(solana-indexer): reword decode comments to describe current beha…

138a48d

…vior

squadgazzz commented Jul 27, 2026

View reviewed changes

Comment thread crates/solana-indexer/Cargo.toml Outdated

squadgazzz marked this pull request as ready for review July 27, 2026 06:53

squadgazzz requested a review from a team as a code owner July 27, 2026 06:53

claude Bot reviewed Jul 27, 2026

View reviewed changes

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

squadgazzz mentioned this pull request Jul 27, 2026

feat(solana-indexer) PR 8: wire the decoder to the persistence seam #4676

Open

github-actions Bot added the stale label Aug 4, 2026

squadgazzz removed the stale label Aug 4, 2026

squadgazzz requested a review from tilacog August 4, 2026 08:41

squadgazzz added 2 commits August 4, 2026 12:55

Pin settlement-interface to solana-programs main

1e6439f

The parsing-generic branch is merged and its ref deleted, so the branch pin would break the next cargo update. Main's interface types the push amount as u64, dropping the byte-array decode on our side.

Drop semicolons and inference emphasis from decoder comments

d671baf

MartinquaXD reviewed Aug 5, 2026

View reviewed changes

squadgazzz added 3 commits August 5, 2026 08:36

Reword the settlement fixture comments in plain terms

da359ae

squadgazzz mentioned this pull request Aug 5, 2026

Rework the instruction parse API around per-input account borrows cowprotocol/solana-programs#93

Open

squadgazzz and others added 2 commits August 5, 2026 14:00

Merge branch 'main' into solana-indexer/PR7.3-interface-parser

462ad94

Fix the unused test imports and the lints section order

dd8d7c5

MartinquaXD approved these changes Aug 7, 2026

View reviewed changes

Use a labeled continue for corrupt pairs and fix the push wording in …

451120f

…the test doc

		begin_data.extend_from_slice(&1u16.to_le_bytes());
		begin_data.extend_from_slice(&4242i64.to_le_bytes());

		let Some(finalize) = instructions.iter().find(\|instruction\| {
		instruction.instruction_index == u32::from(begin_input.finalize_ix_index)

Uh oh!

Conversation

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

Uh oh!

Description

Changes

How to test

Uh oh!

Uh oh!

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

Uh oh!

PR Review: decode settlements via the shared interface parser

Uh oh!

Uh oh!

github-actions Bot commented Aug 4, 2026

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

linear-code Bot commented Aug 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

squadgazzz commented Jul 24, 2026 •

edited

Loading

claude Bot commented Jul 27, 2026 •

edited

Loading