Skip to content

feat(solana-indexer): PR 7.3 decode settlements via the shared interface parser - #4666

Open
squadgazzz wants to merge 2 commits into
mainfrom
solana-indexer/PR7.3-interface-parser
Open

feat(solana-indexer): PR 7.3 decode settlements via the shared interface parser#4666
squadgazzz wants to merge 2 commits into
mainfrom
solana-indexer/PR7.3-interface-parser

Conversation

@squadgazzz

@squadgazzz squadgazzz commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

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.

@squadgazzz 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 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
package = "settlement-interface",
git = "https://github.com/cowprotocol/solana-programs",
rev = "4e0bd7eaed7c237a327705425be2a4a50a8b4a3a"
branch = "make-parsing-generic-on-accounts"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be updated once cowprotocol/solana-programs#80 is merged.

@squadgazzz
squadgazzz marked this pull request as ready for review July 27, 2026 06:53
@squadgazzz
squadgazzz requested a review from a team as a code owner July 27, 2026 06:53
@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

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

let trades = begin_input
.orders
.iter()
.zip(received)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zip silently truncates to the shorter of orders / received. This function otherwise warns-and-skips on every layout mismatch (BeginSettle/FinalizeSettle parse failures) and treats the gRPC stream as untrusted (saturating adds), but here a pushes.len() != orders.len() mismatch — i.e. a violation of the "exactly one push per order" invariant this pairing relies on — produces a partial/misaligned trade set with no signal.

Consider guarding it explicitly so a broken invariant surfaces rather than silently corrupting the settlement:

if finalize_input.pushes.len() != begin_input.orders.len() {
    tracing::warn!(
        instruction_index = begin.instruction_index,
        orders = begin_input.orders.len(),
        pushes = finalize_input.pushes.len(),
        "BeginSettle/FinalizeSettle order/push count mismatch, skipping"
    );
    continue;
}

This is also the one place the correctness of amount_received_delta hinges entirely on pushes being 1:1 and in the same PDA order as orders — worth an explicit check now that the resolver is stubbed and the misalignment wouldn't show up in the emitted events yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant