perf: add bounded oracle HTTP client - #2825
Conversation
How to use the Graphite Merge QueueAdd the label Raindex-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
📝 WalkthroughWalkthroughOracle request handling now uses a reusable ChangesOracle client concurrency
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant OracleClient
participant OriginSemaphore
participant ReqwestClient
Caller->>OracleClient: fetch_signed_contexts(requests)
OracleClient->>OriginSemaphore: acquire bounded origin permits
OracleClient->>ReqwestClient: send oracle POST requests
ReqwestClient-->>OracleClient: return response bodies
OracleClient-->>Caller: ordered SignedContextV1 results
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/quote/src/oracle.rs`:
- Around line 195-200: Move timeout configuration out of build_http_client and
apply it to each request in fetch_responses using RequestBuilder::timeout,
ensuring wasm and non-wasm requests both enforce the 10-second limit; remove the
cfg-gated ClientBuilder timeout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 40732961-24eb-438b-ad73-2f0df163ebed
📒 Files selected for processing (1)
crates/quote/src/oracle.rs
0653da3 to
f663494
Compare
f663494 to
e8df26e
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
| { | ||
| let request_semaphore = self.request_semaphore(url); | ||
| let _permit = request_semaphore | ||
| .acquire() |
There was a problem hiding this comment.
minor: acquire() waits outside the 10s reqwest timeout, so under same-origin contention a call can sit in the semaphore queue for much longer than ORACLE_REQUEST_TIMEOUT before send() even starts. Cancellation still works if the parent drops, and other origins stay isolated, but the constant is not an end-to-end bound. Worth documenting that, or giving acquire its own deadline once the concurrent sweep lands.
e8df26e to
eb53ceb
Compare
Merge activity
|
## Why Oracle-backed quote preparation currently creates a fresh HTTP client for every pair and has no reusable bounded execution primitive. This adds connection-pool reuse and a conservative per-origin limit before the quote sweep is made concurrent. ## What changed - add a reusable `OracleClient` backed by one native/WASM-compatible reqwest connection pool - cap oracle POSTs at eight in flight per origin across independently created clients - add ordered bounded execution that avoids head-of-line blocking - route the existing single and batch APIs through the shared client without changing their signatures, timeout, response validation, or status handling - add deterministic tests for the concurrency bound, shared limiter, positional stability, partial failure isolation, and slow-leading-request behavior ## Intentionally unchanged - configured oracle URLs are not assumed to support batch request bodies - no retries are added for 429/503 or other failures because the endpoint contract does not declare POST idempotency - signed-context and chain quote behavior are unchanged in this prerequisite PR ## Local verification - `cargo test -p raindex_quote --lib` — 63 passed - `cargo clippy -p raindex_quote --all-targets --all-features -- -D warnings -D clippy::all` - `cargo fmt --all -- --check` - `nix develop .#wasm-shell -c bash -c 'CARGO_TARGET_WASM32_UNKNOWN_UNKNOWN_RUNNER=wasm-bindgen-test-runner cargo test --target wasm32-unknown-unknown --lib -p raindex_quote'` - `rainix-rs-static` ## Review focus Please pay particular attention to the per-origin limit of eight, indexed `buffer_unordered` ordering restoration, and native/WASM client reuse. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Performance** * Oracle requests now reuse shared HTTP resources for improved efficiency. * Concurrent requests are bounded, with configurable limits for improved stability under load. * **Reliability** * Batch results preserve their original request order. * Individual request errors are reported without blocking other requests. * Requests now validate endpoints, apply timeouts, and redact sensitive URL details from errors. * Response handling and request validation are more consistent. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
eb53ceb to
19bf215
Compare
## Why Quote-target construction currently awaits each oracle POST inside the nested order/input/output loops. Production evidence from the st0x REST API showed about 49.6 oracle requests and about 9.7 seconds of target-build latency per cache miss, while the subsequent batched chain quote usually took only 100–600 ms. ## What changed - prepare every valid pair and its established single-pair oracle request body in positional order - execute oracle POSTs through the shared process-wide client with a conservative global concurrency limit of 8 - restore results to their original request slots before composing quote targets - preserve oracle-before-injector signed-context order - isolate an oracle error to only its corresponding pair response - keep the chain quote as one `BatchQuoteTarget::do_quote` operation - add tracing for oracle request, endpoint, success/failure, concurrency-limit, batching, and duration data - add a deterministic end-to-end test that asserts exact oracle request bodies, partial failure isolation, exact ordered RPC multicall contents, signed-context composition, distinct RPC-result slot mapping, and a single chain RPC request ## Deliberate constraints - The existing batch encoder/API is not used automatically because order metadata exposes only an oracle URL and does not declare that the configured endpoint accepts the batch ABI body. - Requests remain individual POSTs and are bounded globally at 8; there is no unbounded fan-out. - No retry/backoff was added because the endpoint contract does not declare POST idempotency. Existing 429/503 status handling remains a per-pair failure. - Public response contracts, native/WASM support, timeout behavior, and chain RPC batching semantics are unchanged. ## Verification - `cargo test -p raindex_quote --lib` - `cargo clippy -p raindex_quote --all-targets --all-features -- -D warnings -D clippy::all` - `cargo fmt --all -- --check` - `nix develop .#wasm-shell -c bash -c 'CARGO_TARGET_WASM32_UNKNOWN_UNKNOWN_RUNNER=wasm-bindgen-test-runner cargo test --target wasm32-unknown-unknown --lib -p raindex_quote'` - `nix develop .#wasm-shell -c rainix-rs-static` - `git diff --check` ## Stack Depends on #2825, which adds the reusable shared HTTP client and bounded ordered oracle-request primitive.
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
|
Tip For best results, initiate chat on the files or code changes. SIZE=L You are interacting with an AI system. |
## Dependent PRs - Raindex bounded oracle HTTP client (merge first): rainlanguage/raindex#2825 - Raindex batched oracle context preparation (merge second): rainlanguage/raindex#2826 ## Motivation The REST application's order-quote passes can require many oracle contexts at once. Issuing one HTTP request per context adds avoidable connection and request overhead even when the contexts share the same oracle endpoint. ## Solution - Bump `lib/rain.orderbook` from `0fa60a6` to `4ae9e1a02`. - Send one ABI batch request per exact oracle URL and scatter the returned contexts back to their quote pairs. - Preserve bounded concurrency across distinct oracle endpoints with a limit of 8. - Remove the previous per-context concurrent HTTP request path while leaving chain quote batching unchanged. ## Verification - `nix develop -c cargo check` - Upstream quote crate test suite — 63 passed - Upstream strict Clippy checks - Repository pre-commit hooks - `git diff --check` - Live local REST API validation against Base: - `/v2/swap/quote` returned HTTP 200 and `fullyFilled: true` for wtCOIN, wtNVDA, and wtMSTR. - Batch telemetry reduced 55 oracle contexts to 2 HTTP requests; 54 contexts succeeded and one legacy oracle endpoint returned HTTP 404. - A request-scoped quote recorded one oracle context in one batch request and completed successfully.

Why
Oracle-backed quote preparation currently creates a fresh HTTP client for every
pair and has no reusable bounded execution primitive. This adds connection-pool
reuse and a conservative per-origin limit before the quote sweep is made
concurrent.
What changed
OracleClientbacked by one native/WASM-compatible reqwestconnection pool
changing their signatures, timeout, response validation, or status handling
stability, partial failure isolation, and slow-leading-request behavior
Intentionally unchanged
contract does not declare POST idempotency
Local verification
cargo test -p raindex_quote --lib— 63 passedcargo clippy -p raindex_quote --all-targets --all-features -- -D warnings -D clippy::allcargo fmt --all -- --checknix develop .#wasm-shell -c bash -c 'CARGO_TARGET_WASM32_UNKNOWN_UNKNOWN_RUNNER=wasm-bindgen-test-runner cargo test --target wasm32-unknown-unknown --lib -p raindex_quote'rainix-rs-staticReview focus
Please pay particular attention to the per-origin limit of eight, indexed
buffer_unorderedordering restoration, and native/WASM client reuse.Summary by CodeRabbit
Performance
Reliability