Skip to content

feat(solana-indexer) PR 15.6: adopt cow-solana-rpc - #4751

Open
squadgazzz wants to merge 2 commits into
solana-indexer/PR15-fill-statefrom
solana-indexer/PR15.6-adopt-cow-solana-rpc
Open

feat(solana-indexer) PR 15.6: adopt cow-solana-rpc#4751
squadgazzz wants to merge 2 commits into
solana-indexer/PR15-fill-statefrom
solana-indexer/PR15.6-adopt-cow-solana-rpc

Conversation

@squadgazzz

@squadgazzz squadgazzz commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Description

The indexer hand-rolled its own RPC wrapper while cow-solana-rpc now exists for exactly this. The wrapper moves into the shared crate and the indexer's copy is deleted: construction stays explicit about commitment at the call site (confirmed, matching the stream subscription), the chunked getMultipleAccounts helper moves verbatim (dedup, MAX_MULTIPLE_ACCOUNTS batches, parallel requests), and the canned-response mock constructor lands behind the test-util feature together with Mocks/RpcRequest re-exports, so consumers stop needing solana-client for their tests.

The indexer drops three direct dependencies (solana-client, solana-commitment-config, solana-rpc-client-api, plus itertools) and the driver can reuse the same helper later instead of growing its own copy.

Changes

  • cow-solana-rpc gains multiple_accounts, new_mock_with_mocks (test-util), and re-exports CommitmentConfig, Error, Mocks, RpcRequest
  • The indexer's rpc.rs is deleted, the decoder holds SolanaRPC, errors wrap cow_solana_rpc::Error
  • The mock feature is enabled through dev-dependencies only, production builds compile without the mock constructors

How to test

Existing tests, migrated to the shared mock constructor.

Comment on lines +49 to +53
pub fn new_mock_with_mocks(mocks: Mocks) -> Self {
Self {
inner: RpcClient::new_mock_with_mocks("mock".to_owned(), mocks),
}
}

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.

It wasn't me inventing this naming 😅

@squadgazzz

Copy link
Copy Markdown
Contributor Author

@claude, review, please

@claude

claude Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

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


PR Review — adopt cow-solana-rpc

  • Read PR diff against base branch
  • Review cow-solana-rpc changes
  • Review solana-indexer changes
  • Post inline comments & summary

Verdict: LGTM — clean, well-scoped move. No blocking issues, so no inline comments.

What I checked:

  • multiple_accounts moved verbatim from the deleted rpc.rs (dedup via .unique(), MAX_MULTIPLE_ACCOUNTS chunking, parallel try_join_all, missing accounts dropped). Behavior is unchanged.
  • The error re-export cow_solana_rpc::Error flows correctly into PersistenceError::Rpc(#[from] …).
  • test-util is enabled only via dev-dependencies in both solana-indexer and solana-driver, so production cargo build excludes new_mock / new_mock_with_mocks and the Mocks/RpcRequest re-exports — the resolver keeps dev-dep features out of the normal build graph. The PR's production-safety claim holds.
  • No lingering references to the dropped deps (solana-client, solana-commitment-config, solana-rpc-client-api, itertools) remain in solana-indexer/src.
  • The confirmed commitment rationale correctly moved to the call site (run.rs), keeping the shared wrapper commitment-agnostic.

Minor, non-blocking:

  • The new_mock_with_mocks naming is inherited from RpcClient — already acknowledged in your own review comment, so nothing to change there.

CI is still in_progress at the time of review (no failures yet); it covers the new solana_rpc_client::mock_sender / request::RpcRequest re-export paths, which I can't compile in this environment.
· solana-indexer/PR15.6-adopt-cow-solana-rpc

@squadgazzz
squadgazzz marked this pull request as ready for review August 18, 2026 09:16
@squadgazzz
squadgazzz requested a review from a team as a code owner August 18, 2026 09:16
@squadgazzz
squadgazzz requested a review from tilacog August 18, 2026 09:16
@claude

claude Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @squadgazzz's task in 1m 29s —— View job


PR Review — adopt cow-solana-rpc

  • Read PR diff against base branch
  • Review cow-solana-rpc changes
  • Review solana-indexer changes
  • Post inline comments & summary

Verdict: LGTM — clean, well-scoped move. No blocking issues, so no inline comments.

What I verified:

  • multiple_accounts moved verbatim from the deleted rpc.rs.unique() dedup, MAX_MULTIPLE_ACCOUNTS chunking, parallel try_join_all, missing accounts dropped via filter_map. Behavior unchanged.
  • Error re-export: cow_solana_rpc::Error is solana_rpc_client_api::client_error::Error, the same type behind the old solana_client::client_error::ClientError alias, so PersistenceError::Rpc(#[from] …) keeps compiling.
  • Both mock constructors have real callers: new_mock_with_mocks (indexer decoder tests) and new_mock (solana-driver tests). Mocks/RpcRequest re-exports and both constructors are gated on test-util, which is enabled only through dev-dependencies in solana-indexer and solana-driver — production cargo build excludes them.
  • Commitment stays at the call site (run.rs, confirmed to match the stream subscription), keeping the shared wrapper commitment-agnostic. The #[expect(dead_code)] on inner is correctly dropped now that the field is read.
  • No lingering references to the dropped deps (solana-client, solana-commitment-config, solana-rpc-client-api, itertools) remain in solana-indexer/src.

Non-blocking observation (no change required):

  • The helper now lives in a shared crate that the driver will reuse, but its only coverage is the indexer's decoder tests. A small unit test for multiple_accounts (dedup + chunking across MAX_MULTIPLE_ACCOUNTS) in cow-solana-rpc would be worth adding when the driver adopts it.
  • new_mock_with_mocks naming is inherited from RpcClient — already acknowledged by the author.
    · solana-indexer/PR15.6-adopt-cow-solana-rpc

@squadgazzz
squadgazzz force-pushed the solana-indexer/PR15-fill-state branch from 15403f4 to 604a333 Compare August 19, 2026 11:18
@squadgazzz
squadgazzz force-pushed the solana-indexer/PR15.6-adopt-cow-solana-rpc branch from c9817b9 to d796441 Compare August 19, 2026 11:21
…5.6-adopt-cow-solana-rpc

# Conflicts:
#	crates/solana-indexer/src/run.rs
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