feat(solana-indexer): PR 3 — add `traits` module by tilacog · Pull Request #4508 · cowprotocol/services

tilacog · 2026-06-09T01:41:40Z

Description

Adds the traits/ module as laid out in the PR sequence plan, carrying over the two traits that sit on the indexer's external boundaries.

The trait surface stays small on purpose: each one only covers what the consumer components actually need, not the full API of the underlying library. That keeps the abstractions thin and tied to real call sites.

Changes

Added traits/store.rs:
- Store trait, the PostgreSQL boundary.
Added traits/solana_client.rs:
- SolanaClient trait, the Solana RPC boundary.
Skipped a trait for the Yellowstone gRPC system boundary. That one works better as a GrpcConnector trait bound on the Ingester struct (coming in the next PR).

How to test

Build the crate: cargo check -p solana-indexer.
Check that the new traits module is exported and both traits are reachable from the crate root.
Stub implementations land in follow-up PRs, so there's no runtime behavior to verify here yet.

This is a follow up PR to #4506

Introduces the traits/ module per the PR-sequence plan. The trait surface is intentionally narrow: it covers only what the consumer components need, not the full surface of the underlying library. traits/store.rs: - Store (PostgreSQL boundary) traits/solana_client.rs: - SolanaClient (RPC boundary) We don't need a trait to represent the Yelllowstone gRPC system boundary because that can be represented by a trait bound in the (upcoming) Ingester struct implementation.

gemini-code-assist

Code Review

This pull request introduces the SolanaClient and Store traits to abstract external dependencies (Solana RPC and PostgreSQL persistence) for the solana-indexer crate, along with updating dependency configurations and refactoring recovery-flow types. Feedback on the Store trait suggests returning Result<(), StoreError> instead of () for write_dead_letter and record_lost_slot_range to ensure robust error handling for database operations.

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.

socket-security · 2026-06-09T01:48:56Z

Warning

Review the following alerts detected in dependencies.

According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.

Action	Severity	Alert (click "▶" to expand/collapse)
Warn		Obfuscated code: cargo `libc` is 90.0% likely obfuscated Confidence: 0.90 Location: Package overview From: `?` → `cargo/axum@0.8.8` → `cargo/moka@0.12.12` → `cargo/reqwest@0.12.28` → `cargo/tempfile@3.24.0` → `cargo/ruint@1.17.2` → `cargo/tokio@1.49.0` → `cargo/time@0.3.47` → `cargo/reqwest@0.13.2` → `cargo/solana-sdk@4.0.1` → `cargo/alloy-contract@1.7.3` → `cargo/alloy-provider@1.7.3` → `cargo/alloy-primitives@1.5.7` → `cargo/alloy-sol-macro-expander@1.5.7` → `cargo/alloy-sol-macro-input@1.5.7` → `cargo/aws-config@1.8.15` → `cargo/aws-sdk-s3@1.127.0` → `cargo/alloy-transport-ws@1.8.3` → `cargo/alloy-signer@1.8.3` → `cargo/alloy-consensus@1.8.3` → `cargo/alloy-rpc-client@1.8.3` → `cargo/alloy-signer-local@1.8.3` → `cargo/alloy-eips@1.8.3` → `cargo/alloy-transport@1.8.3` → `cargo/alloy-provider@1.8.3` → `cargo/alloy@1.8.3` → `cargo/rand@0.10.1` → `cargo/rand@0.9.4` → `cargo/rand@0.8.6` → `cargo/async-nats@0.48.0` → `cargo/opentelemetry-otlp@0.32.0` → `cargo/yellowstone-grpc-proto@12.4.0` → `cargo/solana-client@4.0.0` → `cargo/reqwest@0.13.4` → `cargo/sqlx@0.8.6` → `cargo/sha2@0.10.9` → `cargo/dashmap@5.5.3` → `cargo/dashmap@6.1.0` → `cargo/axum@0.6.20` → `cargo/hmac@0.12.1` → `cargo/prometheus@0.14.0` → `cargo/mimalloc@0.1.48` → `cargo/chrono@0.4.42` → `cargo/console-subscriber@0.3.0` → `cargo/jemalloc_pprof@0.8.1` → `cargo/backtrace@0.3.76` → `cargo/const-hex@1.17.0` → `cargo/tikv-jemallocator@0.6.1` → `cargo/libc@0.2.186` ℹ Read more on: This package \| This alert \| What is obfuscated code? Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at `support@socket.dev`. Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code. Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment `@SocketSecurity ignore cargo/libc@0.2.186`. You can also ignore all packages with `@SocketSecurity ignore-all`. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.
Warn		Obfuscated code: cargo `openssl` is 90.0% likely obfuscated Confidence: 0.90 Location: Package overview From: `?` → `cargo/sqlx@0.8.6` → `cargo/openssl@0.10.80` ℹ Read more on: This package \| This alert \| What is obfuscated code? Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at `support@socket.dev`. Suggestion: Packages should not obfuscate their code. Consider not using packages with obfuscated code. Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment `@SocketSecurity ignore cargo/openssl@0.10.80`. You can also ignore all packages with `@SocketSecurity ignore-all`. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

gemini-code-assist

Code Review

This pull request introduces the SolanaClient and Store traits to abstract external Solana RPC calls and PostgreSQL persistence for the indexer. It also refactors related types and dependencies. The feedback suggests using std::ops::RangeInclusive<u64> instead of std::ops::Range<u64> in the Store trait to avoid potential off-by-one errors when recording lost slot ranges.

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.

socket-security · 2026-06-09T12:30:47Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	reqwest@0.12.28
	solana-client@4.0.0
	base64@0.13.1
	rand@0.8.6
	dashmap@5.5.3
	num@0.2.1
	arc-swap@1.8.0 ⏵ 1.9.1

View full report

github-actions · 2026-06-17T00:38:51Z

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.

MartinquaXD · 2026-06-23T10:52:27Z

+    /// Record a slot checkpoint. Rejects downward writes.
+    async fn write_watermark(&self, slot: u64) -> Result<(), StoreError>;


What does "reject downward writes" mean?

It means it should reject slot values smaller than what's already stored, so we never rewind the watermark by accident.

This is also something we're planning to enforce as database constraints.

MartinquaXD · 2026-06-23T10:54:56Z

+};
+
+/// PostgreSQL persistence. Used by Decoder, Watchdog, and FinalizationWorker.
+pub trait Store {


Can you elaborate a bit on why making this a trait is good?
Is the intention to implement a custom persistence layer for testing purposes or something?

I mostly wanted to follow dependency inversion so the pieces stay decoupled.
It also makes it easier to use mocks in our tests, which is a nice plus.

I am only planning on using Postgres for now, so it is really just about keeping the logic separate from the storage layer. (This applies to all the traits here, btw)

And, to be fair, I noticed this specific trait is getting too crowded w/ methods.

I thought about splitting it into smaller sub-traits, culminating into a Store: A + B + C super- trait, but I was worried about making it more complicated than it needs to be.

I think we should refine that idea before merging this

I guess the main reason for that is mocked tests, right? But do we really need them? Can just spawn a pg instance like it is done for DB tests?

- openssl 0.10.75 → 0.10.80 (fixes CVE-2026-41676/78/81/98, CVE-2026-42327, CVE-2026-44662, CVE-2026-45784, CVE-2026-41677) - rand 0.8.5 → 0.8.6 (fixes GHSA-cq8v-f236-94qc)

squadgazzz · 2026-06-24T10:34:43Z

+    /// Primary promotion pass: fetch `confirmed` rows whose `slot` is at or
+    /// above the finalization-window threshold (`slot >= newer_than_slot`).
+    /// `limit` caps the batch at 256 (RPC batch size). Returns `Err` on
+    /// backend failure so the caller can back off rather than
+    /// silently stall on a dead store.
+    async fn get_confirmed_rows(
+        &self,
+        newer_than_slot: u64,
+        limit: usize,
+    ) -> Result<Vec<UnfinalizedRow>, StoreError>;


This returns the newest rows (slot >= newer_than_slot), but do we want the oldest here? A transaction takes ~32 slots to finalize, so polling anything newer just wastes RPC calls. Should the bound be slot <= threshold?

You're correct.
We expect the finalization pass to manage the newest rows, so that method should only care for the slots that weren't handled by that.

I think it this would be better off like this:

/// Primary promotion pass: fetch `confirmed` rows whose `slot` is old enough /// to be finalized (typically `slot <= tip - 32`) but new enough to still /// be tracked by the RPC retention horizon. /// /// `limit` should honor the RPC max batch size of 256. async fn get_confirmed_rows( &self, max_slot: u64, // <-- renamed after `newer_than_slot` limit: usize, ) -> Result<Vec<UnfinalizedRow>, StoreError>;

While we're on this, I believe this should now use the Slot type introduced in the previous PR

Also, I think that limit might not be a good fit here, because it's a constraint on the RPC boundary, not related the Store trait itself.

Naturally, we expect Store implementors to limit their queries, but I don't think that detail should be represented at this spot.

🤔 🤔 🤔

Makes sense. Switched it to max_slot with slot <= max_slot and the Slot type.

On limit: I'd keep it, but change what it means. You're right that the 256 is the getSignatureStatuses cap, and that's an RPC concern, not a Store one. But the Store still shouldn't hand back thousands of confirmed rows at once, so a limit as a plain page size is still worth keeping. So only the doc changes: stop calling it the "256 RPC batch", call it a page size, and let the worker split the rows into batches of 256 when it calls getSignatureStatuses. That way, the RPC limit stays on the RPC side. Wdyt?

…tstrap Resolve conflicts from the updated PR2 base: - adopt Slot/OrderUid newtypes in channel.rs and recovery.rs - drop observe/prometheus deps (metrics.rs removed on PR2), keep solana-client - make Store/SolanaClient pub(crate) to match PR2's type visibility (a pub trait over pub(crate) types is E0446)

Flip the bound from slot >= newer_than_slot to slot <= max_slot so the promotion pass fetches rows old enough to have finalized, not the freshest ones. Reframe limit as a DB page size (the RPC 256-batch cap lives on the SolanaClient side). Per review thread.

Switch the two traits to #[async_trait] with a Send + Sync supertrait so the components can hold them as Arc<dyn Store> / Arc<dyn SolanaClient> instead of propagating generic type params (review feedback). Drops the allow(async_fn_in_trait) crate attr.

Trivy matches CVE-2020-36460 (RUSTSEC-2021-0103, data races in the crates.io model crate) by name and version against our own workspace crate crates/model (also named model, version 0.1.0), which is unrelated and not affected. It surfaces on this PR because the Cargo.lock diff vs main is large enough that Trivy attributes the baseline alert to the PR.

tilacog requested a review from a team as a code owner June 9, 2026 01:41

gemini-code-assist Bot reviewed Jun 9, 2026

View reviewed changes

Comment thread crates/solana-indexer/src/traits/store.rs Outdated

tilacog added 2 commits June 8, 2026 22:45

Return Result in Store metods

fbef899

fmt toml

c73ad1d

tilacog changed the title ~~feat(solana-indexer): traits/ module~~ feat(solana-indexer): add traits module Jun 9, 2026

tilacog marked this pull request as draft June 9, 2026 11:00

tilacog marked this pull request as ready for review June 9, 2026 11:35

gemini-code-assist Bot reviewed Jun 9, 2026

View reviewed changes

Comment thread crates/solana-indexer/src/traits/store.rs Outdated

use RangeInclusive

62160e8

github-actions Bot added the stale label Jun 17, 2026

MartinquaXD reviewed Jun 23, 2026

View reviewed changes

tilacog changed the title ~~feat(solana-indexer): add traits module~~ feat(solana-indexer): PR 3 — add traits module Jun 23, 2026

chore(deps): bump openssl and rand to fix Trivy CVEs

fce49aa

- openssl 0.10.75 → 0.10.80 (fixes CVE-2026-41676/78/81/98, CVE-2026-42327, CVE-2026-44662, CVE-2026-45784, CVE-2026-41677) - rand 0.8.5 → 0.8.6 (fixes GHSA-cq8v-f236-94qc)

tilacog mentioned this pull request Jun 23, 2026

feat(solana-indexer): PR 4 — Component structs (skeleton declarations) #4514

Open

github-actions Bot removed the stale label Jun 24, 2026

squadgazzz reviewed Jun 24, 2026

View reviewed changes

squadgazzz added 6 commits June 25, 2026 14:47

chore(solana-indexer): lock async-trait dependency

03ade29

cargo.lock

fa15659

		/// Record a slot checkpoint. Rejects downward writes.
		async fn write_watermark(&self, slot: u64) -> Result<(), StoreError>;

Uh oh!

Conversation

tilacog commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

How to test

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

socket-security Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

socket-security Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 17, 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!

Choose a reason for hiding this comment

Uh oh!

tilacog Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tilacog Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tilacog commented Jun 9, 2026 •

edited

Loading

socket-security Bot commented Jun 9, 2026 •

edited

Loading

socket-security Bot commented Jun 9, 2026 •

edited

Loading

tilacog Jun 25, 2026 •

edited

Loading

tilacog Jun 25, 2026 •

edited

Loading