feat(solana-indexer) PR 8: wire the decoder to the persistence seam by squadgazzz · Pull Request #4676 · cowprotocol/services

squadgazzz · 2026-07-27T16:11:29Z

Description

The decoder produced settlement events and dropped them. This wires them into the Persistence seam: events and the slot watermark flow through persist_events, decode failures land in a dead-letter seam, and reverted transactions stop emitting events. The Persistence bodies stay documented no-ops until the Postgres adapter PR.

Changes

Decoder::run hands each transaction's events to persist_events and advances the watermark, or calls write_watermark when a transaction yields no events.
The watermark value is slot - 1: stream resume is slot-granular (from_slot = watermark + 1), so marking a slot done while it may still have transactions in flight would skip its remainder after a crash. Redelivery after restart is absorbed by idempotent writes (spec §7, §10).
Transactions with meta.err set decode to nothing: a failed Solana transaction rolls back every account write, so the decoder must not emit state-changing events (spec §4). A revert-attribution event is a later PR.
Per-transaction decode failures set a flag and record one write_dead_letter(signature, slot, "decoder_error") entry, while the events that did decode still persist and the watermark still advances (spec §12).
write_dead_letter added to Persistence, and TransactionError re-exported from the wire types.

How to test

New and updated unit tests.

Related issues

Stacked on #4666. The SQL bodies land with the Postgres adapter PR later.

…test

claude · 2026-07-28T12:13:43Z

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

PR Review: wire the decoder to the persistence seam

Read changed files and diff against base
Analyze decoder/persistence wiring for correctness
Review tests
Post inline comments and summary

This is a clean, well-reasoned PR. The watermark = slot - 1 design is the subtle part and it holds up: because a slot is never marked done by its own transactions, any write_dead_letter for that slot is always durably written before a later slot advances the watermark past it — so the dead-letter vs. event-persist non-atomicity is genuinely absorbed by idempotent replay, as the description claims. The decode_failed propagation through decode_settlement / decode_settlements_finalized covers every parse/account/pairing failure, and recognized-but-eventless instructions (Initialize, ReclaimOrder) return Ok(vec![]) so they correctly don't trip the flag — only unknown discriminators do.

I left one inline comment: the revert guard treats a missing meta as "not reverted" (falls through to decoding), while post_token_balances a few lines down handles a missing meta defensively — an inconsistency worth pinning down.

Everything else — the reverted-tx skip, the empty-vs-non-empty watermark branch, the TransactionError re-export, and the documented no-op seam bodies — looks correct for this stage of the stack. Test coverage of decode is solid; the run-loop side effects being unobservable through the no-op seam is already acknowledged in the code.

Note: cargo check/nextest are blocked by the sandbox in this environment, so this review is based on static analysis only.

…on id to the wire

…out meta

… from the logs

…arser' into solana-indexer/PR8-persist

Drops the pending-adapter and later-PR breadcrumbs, the spec anchor, and the leftover cast comment resolved by the i64 pass-through.

MartinquaXD · 2026-08-05T08:13:44Z

@@ -1,10 +1,9 @@
-#![expect(dead_code)]
+#![allow(dead_code, reason = "dead in the lib build, exercised by tests")]


Why not gate the respective identifiers with #[cfg(test)] then?

These are dead only until the wiring PR makes the binary call them. I was choosing between the allow line and #[cfg(test)] on each item (not needed in the future PRs). So the latter is just an intermediate step we can avoid.

MartinquaXD · 2026-08-05T08:16:14Z

            } = update;
-            self.decode(&inner, slot, signature);
+            let (events, decode_failed) = self.decode(&inner, slot, signature);
+            tracing::debug!(slot = %slot, event_count = events.len(), "decoded events");


shouldn't we also log the decode failures?

They are logged. Every failure site warns with the error, instruction index, and tx signature, see

services/crates/solana-indexer/src/indexer/decoder.rs

Lines 252 to 259 in fa6488d

let Ok((discriminator, _)) = recover_discriminator(&instruction.data) else {

decode_failed = true;

// Warn, not debug: this dead-letters the transaction, so it needs to

// be findable in the logs alongside the row.

tracing::warn!(

signature = %ctx.signature,

instruction_index = instruction.instruction_index,

err = %DecodeError::UnknownDiscriminator,

The dead-letter row stays minimal (signature + slot), recovery only needs to know what to re-fetch. Right now, a failed tx produces a second warn from the persistence stub saying the write was dropped, which will be removed in the DB adapter PR.

MartinquaXD · 2026-08-05T08:22:13Z

+            // Stream resume is slot-granular (`from_slot = watermark + 1`), and
+            // the slot may still have more transactions in flight, so marking it
+            // done here could skip its remaining transactions after a crash.
+            // Writing `slot - 1` on every transaction only ever marks fully
+            // delivered slots. A redelivery of this slot after a restart is
+            // absorbed by idempotent writes.


Seems like some of the complications (watermark - 1, spammy DB updates) could be resolved by buffering the incoming stream until we have all events for a given slot. Is there a good reason to issue multiple DB writes per slot?

Makes sense. Those are buffered now per slot and flushed once a transaction of a later slot arrives.

MartinquaXD · 2026-08-05T08:23:40Z

+    /// a flag reporting whether any settlement instruction failed to decode.
+    /// The settlement half runs through the pure [`decode_settlement`], the
+    /// SolFlow half is a stub.
+    #[tracing::instrument(skip_all, fields(slot = %slot, signature = %signature))]


I can understand instrumenting the slot but the signature seems unnecessary. This data will be added to every trace downstream. Is the signature really that important?

Not really, dropped it.

I didn't know that you find transactions on the block explorer via the signature (as opposed to the tx hash for EVM). In that case I'd be fine with instrumenting the signature. Sorry for the confusion.

MartinquaXD · 2026-08-05T08:25:19Z

+        // instead of skipping: replay re-fetches by signature, and
+        // `getTransaction` returns the meta.
+        let Some(meta) = tx.meta.as_ref() else {
+            tracing::warn!("transaction update without meta");


We should log information to identify the actual tx.

Yep, we should. Added.

MartinquaXD · 2026-08-05T08:39:13Z

we are iterating over the instructions twice and clone them when we don't have to. There could be a loop pushing an instruction either into a settlement contract vector or a solflow vector.

Replaced with a partition. One pass, no clones.

MartinquaXD · 2026-08-05T08:40:02Z

-            "decoded settlement events"
-        );
+        (
+            events.into_iter().map(DecodedEvent::Settlement).collect(),


why does decode_settlement not already return a DecodedEvent::Settlemen to avoid this conversion here?

decode_settlement handles only the settlement program's events. DecodedEvent is the enum over both SolFlow and settlement programs. If decode_settlement returned DecodedEvent itself, its signature would say it can also emit SolFlow events, which it can't.

MartinquaXD · 2026-08-05T08:43:10Z

    ctx: &TxContext,
    resolve_order: impl Fn(&Pubkey) -> Option<ResolvedOrder>,
-) -> Vec<SettlementEvent> {
+) -> (Vec<SettlementEvent>, bool) {


This interface is very error prone. The function should return a Result so that the caller has to acknowledge that an error happened to get at the partially decoded data (if handling partially decoded transactions even makes sense).

Updated. It now returns Result<Vec<SettlementEvent>, PartialDecode>.

MartinquaXD · 2026-08-05T08:44:09Z

+            // be findable in the logs alongside the row.
+            tracing::warn!(
                instruction_index = instruction.instruction_index,
+                err = %DecodeError::UnknownDiscriminator,


why was all this error handling not added to the previous PR? Would have made reviewing simpler.

True, it should've been done in the previous PR.

MartinquaXD · 2026-08-05T08:48:12Z

+        // No-op seam (no Postgres adapter). The adapter writes the
+        // events and advances the watermark in one SQL transaction: append rows
+        // as INSERT ON CONFLICT DO NOTHING, the watermark UPDATE guarded with
+        // WHERE slot < $new_watermark.
+        Ok(())


what's the reason to replace todo!() with a no-op here? This doesn't seem good.
At the very least we should emit a warning.

run_drains in this PR drives run() over a transaction that fails to decode, which reaches write_dead_letter. With todo!() that test panics. The pipeline test in #4677 then asserts all three writes through a call recorder.

…arser' into solana-indexer/PR8-persist # Conflicts: # crates/solana-indexer/src/indexer/decoder/tests.rs

…xer/PR8-persist # Conflicts: # crates/solana-indexer/src/indexer/decoder/tests.rs

MartinquaXD · 2026-08-07T07:50:16Z

+            // A transaction of a later slot proves the pending slot is fully
+            // delivered, so it is safe to flush and mark done.


This seems very reasonable to me but we have no experience with the RPC behavior yet. Maybe we should add a log that informs us when we receive events out of order. 🤔

Makes sense. Added a warn log.

MartinquaXD · 2026-08-07T07:56:55Z

                // Partial decode is expected: events that did decode persist and
                // the watermark advances. Recovery replays the whole transaction
                // by signature, and idempotent writes absorb the overlap.
                self.persistence.write_dead_letter(signature, slot).await?;


The comment says that this advances the watermark, right?
Don't know for sure but it sounds like that could lead to skipped events when we fail to decode an event and the service crashes before processing the remaining events in the slot.
But it seems like the existing buffering logic should be easy to extend to also buffering the dead_letter stuff.

The skip couldn't quite happen since the watermark only ever advances at the slot flush, never per transaction. But I updated the code as you suggested. Dead letters now buffer with the slot, and the flush writes them first, before the events and the watermark advance, so a crash can never leave the watermark past a slot.

MartinquaXD · 2026-08-07T08:01:49Z

+    /// a flag reporting whether any settlement instruction failed to decode.
+    /// The settlement half runs through the pure [`decode_settlement`], the
+    /// SolFlow half is a stub.
+    #[tracing::instrument(skip_all, fields(slot = %slot, signature = %signature))]


I didn't know that you find transactions on the block explorer via the signature (as opposed to the tx hash for EVM). In that case I'd be fine with instrumenting the signature. Sorry for the confusion.

MartinquaXD · 2026-08-07T08:06:18Z

+        tx: SubscribeUpdateTransactionInfo,
+        slot: Slot,
+        signature: Signature,
+    ) -> (Vec<DecodedEvent>, bool) {


This function has the same issue with the bool indicating an error that can easily be ignored.

Oh yeah, thanks! Updated.

linear-code · 2026-08-07T08:12:11Z

BE-202

…xer/PR8-persist # Conflicts: # crates/solana-indexer/src/indexer/decoder.rs

feat(solana-indexer): wire the decoder to the persistence seam

dd95347

squadgazzz changed the title ~~solana-indexer: wire the decoder to the persistence seam~~ feat(solana-indexer) PR 8: wire the decoder to the persistence seam Jul 27, 2026

squadgazzz changed the title ~~feat(solana-indexer) PR 8: wire the decoder to the persistence seam~~ feat(solana-indexer): PR 8 wire the decoder to the persistence seam Jul 27, 2026

squadgazzz changed the title ~~feat(solana-indexer): PR 8 wire the decoder to the persistence seam~~ feat(solana-indexer) PR 8: wire the decoder to the persistence seam Jul 27, 2026

test(solana-indexer): fold the decode-wrap and revert cases into one …

77a8d05

…test

squadgazzz marked this pull request as ready for review July 28, 2026 12:13

squadgazzz requested a review from a team as a code owner July 28, 2026 12:13

squadgazzz requested a review from tilacog July 28, 2026 12:15

claude Bot reviewed Jul 28, 2026

View reviewed changes

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

squadgazzz mentioned this pull request Jul 28, 2026

feat(solana-indexer) PR 9: cover the ingester to decoder pipeline end to end #4677

Open

squadgazzz added 6 commits July 28, 2026 14:02

refactor(solana-indexer): use Slot for slot arguments and match aucti…

a4d7a09

…on id to the wire

fix(solana-indexer): dead-letter transaction updates that arrive with…

16d35c5

…out meta

fix(solana-indexer): make every dead-lettered transaction diagnosable…

ec232d1

… from the logs

Merge remote-tracking branch 'origin/solana-indexer/PR7.3-interface-p…

74bdc50

…arser' into solana-indexer/PR8-persist

Merge PR7.3 and sweep comments for standalone reading

6a4c45a

Drops the pending-adapter and later-PR breadcrumbs, the spec anchor, and the leftover cast comment resolved by the i64 pass-through.

Fold the meta-state tests into one and drop the duplicate decode test

0daa1af

MartinquaXD reviewed Aug 5, 2026

View reviewed changes

squadgazzz mentioned this pull request Aug 5, 2026

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

Open

squadgazzz added 3 commits August 5, 2026 11:08

Merge remote-tracking branch 'origin/solana-indexer/PR7.3-interface-p…

6a2fbd1

…arser' into solana-indexer/PR8-persist # Conflicts: # crates/solana-indexer/src/indexer/decoder/tests.rs

Buffer decoded events per slot and make partial decodes a typed result

525532a

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

fa6488d

…xer/PR8-persist # Conflicts: # crates/solana-indexer/src/indexer/decoder/tests.rs

squadgazzz requested a review from MartinquaXD August 5, 2026 13:12

MartinquaXD reviewed Aug 7, 2026

View reviewed changes

squadgazzz added 2 commits August 7, 2026 12:22

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

e0a07e0

…xer/PR8-persist # Conflicts: # crates/solana-indexer/src/indexer/decoder.rs

Buffer dead letters with the slot and type the decode failure

6bd207d

		@@ -1,10 +1,9 @@
		#![expect(dead_code)]
		#![allow(dead_code, reason = "dead in the lib build, exercised by tests")]

	let Ok((discriminator, _)) = recover_discriminator(&instruction.data) else {
	decode_failed = true;
	// Warn, not debug: this dead-letters the transaction, so it needs to
	// be findable in the logs alongside the row.
	tracing::warn!(
	signature = %ctx.signature,
	instruction_index = instruction.instruction_index,
	err = %DecodeError::UnknownDiscriminator,

		// A transaction of a later slot proves the pending slot is fully
		// delivered, so it is safe to flush and mark done.

Uh oh!

Conversation

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

Uh oh!

Description

Changes

How to test

Related issues

Uh oh!

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

Uh oh!

PR Review: wire the decoder to the persistence seam

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!

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 27, 2026 •

edited

Loading

claude Bot commented Jul 28, 2026 •

edited

Loading