feat: oracle signed context - full stack (Phases 2-7)#2505
Open
feat: oracle signed context - full stack (Phases 2-7)#2505
Conversation
added 24 commits
February 13, 2026 18:22
…n RaindexOrder - Add SignedContextOracleV1 variant to ParsedMeta enum - Add match arm for KnownMagic::SignedContextOracleV1 in parsing logic - Add oracle_url() wasm_bindgen getter on RaindexOrder - Depends on rain.metadata feat/signed-context-oracle-meta branch
- Add 3 tests for SignedContextOracleV1 parsing (from_meta_item, parse_multiple, parse_from_bytes roundtrip) - Add oracle_url() to non-wasm impl block (mirrors wasm getter) - All 11 parsed_meta tests passing
…leV1 metadata Points rain.interpreter at feat/signed-context-oracle-meta-submodule branch which updates the rain.metadata submodule to feat/signed-context-oracle-meta.
Matches rename in rain.metadata#92. The metadata type is specific to the Raindex calculateOrderIO entrypoint.
… flows Phase 3 of signed context oracle discovery: - New oracle.rs module with fetch_signed_context(url) and OracleResponse type - OracleResponse maps directly to SignedContextV1 (signer, context as bytes32[], signature) - Added signed_context field to TakeOrderCandidate - Wired oracle fetching into: - build_take_order_candidates_for_pair (batch flow, concurrent fetch) - execute_single_take (single take flow, oracle_url param) - build_take_orders_config_from_simulation (passes through to TakeOrderConfigV4) - Oracle fetch is best-effort: failures log a warning and use empty signed context - 3 oracle tests + 9 parsed_meta tests passing
- Add get_order_quotes_with_context() to quote crate (accepts signed_context param) - RaindexOrder.get_quotes() now fetches oracle data and passes to quotes - Original get_order_quotes() unchanged (delegates with empty context)
reqwest::ClientBuilder::timeout() is not available on WASM targets. Use cfg(not(target_family = "wasm")) to only set it on native.
- OrderDetail: show Oracle card property with URL link when order has oracle metadata - Includes tooltip explaining signed context oracle usage - TanstackOrderQuote: show purple 'Oracle' badge next to quotes heading when oracle is active - Indicates quotes include signed context data from oracle - Both use the oracleUrl getter exposed via WASM bindings on RaindexOrder
The oracle endpoint now receives order details via POST so it can tailor responses based on the specific order, counterparty, and IO indexes. POST body: abi.encode(OrderV4, inputIOIndex, outputIOIndex, counterparty) Falls back to GET when no body is provided (simple oracles). Callers currently pass None - ABI encoding will be wired in once the order data is available at each call site.
POST with ABI-encoded order data is mandatory. Callers currently pass empty vec — will be wired to abi.encode(OrderV4, inputIOIndex, outputIOIndex, counterparty) at each call site.
- encode_oracle_body: abi.encode(OrderV4, inputIOIndex, outputIOIndex, counterparty) - get_quotes: fetches oracle per IO pair concurrently, counterparty=address(0) - build_take_order_candidates: fetches oracle per quote pair - execute_single_take: encodes with actual taker as counterparty - get_order_quotes_with_context_fn: accepts per-pair context callback
- Oracle fetch logic moved from common to quote crate (common re-exports) - get_order_quotes now extracts oracle URL directly from SgOrder.meta - Removed get_order_quotes_with_context and get_order_quotes_with_context_fn - No more closures, HashMaps, or pre-fetching — oracle context fetched inline per IO pair inside the quote loop - RaindexOrder.get_quotes() simplified to just call get_order_quotes()
- Add encode_oracle_body_batch() for array encoding: abi.encode((OrderV4, uint256, uint256, address)[]) - Update fetch_signed_context_batch() to handle Vec responses - Maintain backward compatibility with single request functions - Add comprehensive tests for both single and batch formats - Response format now expects JSON array per spec
Adds optional oracle-url field to OrderCfg, parsed from the YAML front matter orders section. When present, this URL identifies a signed context oracle server for the order. Changes: - Add oracle_url: Option<String> to OrderCfg struct - Parse oracle-url via optional_string in YAML parsing - Add oracle-url to ALLOWED_ORDER_KEYS - Update Default and PartialEq impls - Add test for oracle-url parsing (present + absent) Spec: rainlanguage/specs#45 Chained on: #2459 (Phase 4)
When an order has oracle_url set in its config, new_from_deployment now creates a SignedContextOracleV1 meta item and includes it in the order's additional_meta. This means orders deployed with oracle-url in their YAML will have the oracle endpoint encoded in their onchain RainMetaDocumentV1, making it discoverable by takers and indexers (Phase 2 reads it back). Changes: - Import SignedContextOracleV1 in add_order.rs - In new_from_deployment: parse oracle_url, create meta item, append to additional_meta
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds optional per-order Changes
Sequence DiagramsequenceDiagram
participant Client
participant QuoteModule
participant OracleModule
participant OracleEndpoint
participant TakeOrderModule
Client->>QuoteModule: get_order_quotes(orders, block, rpcs, chunk_size)
QuoteModule->>QuoteModule: extract_oracle_url(order)
alt oracle_url present
QuoteModule->>OracleModule: encode_oracle_body(order, in_idx, out_idx, counterparty)
OracleModule-->>QuoteModule: body (bytes)
QuoteModule->>OracleEndpoint: POST body (application/octet-stream)
OracleEndpoint-->>QuoteModule: OracleResponse
QuoteModule->>OracleModule: convert OracleResponse -> SignedContextV1
OracleModule-->>QuoteModule: SignedContextV1
QuoteModule->>QuoteModule: attach signed_context to quote
else oracle_url absent
QuoteModule->>QuoteModule: use empty signed_context
end
QuoteModule-->>TakeOrderModule: BatchOrderQuotesResponse (with signed_context)
TakeOrderModule->>TakeOrderModule: build candidates including signed_context
TakeOrderModule->>TakeOrderModule: execute_single_take(..., oracle_url)
alt oracle_url provided
TakeOrderModule->>OracleModule: fetch_signed_context(url, body)
OracleModule-->>TakeOrderModule: SignedContextV1 (or error -> warn)
TakeOrderModule->>TakeOrderModule: set candidate.signed_context
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/common/src/take_orders/config.rs (1)
87-95: 🧹 Nitpick | 🔵 TrivialAdd one non-empty
signedContextregression test here.Line 94 is now the only point where fetched oracle data enters
TakeOrderConfigV4. The current tests in this module still build candidates viamake_simulation_candidatefromcrates/common/src/test_helpers.rs, which hardcodessigned_context: vec![], and the updatedexecute_single_taketests only passNonefororacle_url. A regression back to an emptysignedContextwould still leave this PR green; please assert a non-emptyconfig.orders[*].signedContextsomewhere in this path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/common/src/take_orders/config.rs` around lines 87 - 95, The current tests never exercise non-empty signedContext because candidates are built with make_simulation_candidate (which sets signed_context: vec![]) and existing execute_single_take tests pass oracle_url = None; add or update a unit test that constructs a simulation with at least one leg whose candidate.signed_context is populated (or call the code path that fetches oracle data so signedContext is set) and then after building TakeOrderConfigV4 via the map (the orders variable produced from sim.legs.iter().map(...) that creates TakeOrderConfigV4) assert that config.orders[i].signedContext is non-empty; reference the TakeOrderConfigV4 construction and the execute_single_take test helper to locate where to inject the non-empty signed context and the assertion.crates/quote/src/order_quotes.rs (1)
53-60:⚠️ Potential issue | 🟠 MajorOracle-backed historical quotes ignore
block_number.Lines 53-60 pin the EVM quote to
req_block_number, but Lines 97-106 always fetch live oracle context with no block/timestamp input. For oracle-driven orders,block_number: Some(...)now mixes historical chain state with current oracle state and can return a quote that never existed. Either reject historical quotes for oracle-backed orders, or make the oracle request block-aware.Also applies to: 97-106
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/quote/src/order_quotes.rs` around lines 53 - 60, The code computes req_block_number using ReadableClient::new_from_http_urls(...).get_block_number() but later (around the oracle context fetch block at lines ~97-106) always requests live oracle state, causing mismatches for oracle-backed orders when block_number is Some(...); fix by detecting oracle-backed orders and either rejecting historical quotes (return an error if block_number.is_some() and the order is oracle-backed) or by making the oracle request block-aware: extend the oracle-context fetch to accept a block/timestamp parameter and pass req_block_number there so the oracle snapshot and chain state are consistent (locate req_block_number, the ReadableClient::new_from_http_urls/get_block_number call, and the oracle-context fetch block to implement the check or the parameter plumbing).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/common/src/add_order.rs`:
- Around line 134-147: The block that appends a RaindexSignedContextOracleV1
meta item can create duplicates when additional_meta already contains one;
update the logic in the additional_meta construction (the code that reads
deployment.order.oracle_url, calls RaindexSignedContextOracleV1::parse(...) and
pushes oracle.to_meta_item()) to first scan existing meta for an existing
RaindexSignedContextOracleV1 entry (by matching meta item type/name or parsing
existing meta into RaindexSignedContextOracleV1) and only push the new oracle
meta if none exists or if you explicitly want to replace it; ensure you
reference the same symbols (additional_meta,
RaindexSignedContextOracleV1::parse, oracle.to_meta_item,
deployment.order.oracle_url) so the duplicate is prevented or handled
deterministically.
In `@crates/common/src/raindex_client/take_orders/single.rs`:
- Around line 117-133: The oracle fetch (crate::oracle::fetch_signed_context /
encode_oracle_body) is performed before early exits in execute_single_take,
causing unnecessary external POSTs when the function will immediately return
NoLiquidity or approval calldata; move the entire block that builds body and
calls fetch_signed_context (and the assignment to candidate.signed_context) to
after the price-cap rejection and approval short-circuit checks so it only runs
when proceeding to execution, ensuring oracle_url and taker are still available
in that later scope.
In `@crates/common/src/take_orders/candidates.rs`:
- Around line 84-117: The code is refetching oracle context with Address::ZERO
inside the quotes loop (via fetch_oracle_for_pair) and can bake wrong or missing
signatures into TakeOrderCandidate.signed_context which later becomes
TakeOrderConfigV4.signedContext; instead, stop refetching with a zero address
and either (A) thread the real counterparty address into this builder and pass
it into fetch_oracle_for_pair so the fetched signed_context is bound to the
correct taker, or (B) propagate the exact signed context produced by get_quotes
into try_build_candidate (and ultimately into TakeOrderCandidate.signed_context)
instead of calling fetch_oracle_for_pair here; update the loop around quotes and
the try_build_candidate call sites accordingly (also apply the same fix to the
analogous block at the later 128-147 region).
In `@crates/quote/src/oracle.rs`:
- Around line 89-145: The fetch_signed_context and fetch_signed_context_batch
functions currently POST to unvalidated URLs; before building the reqwest Client
and issuing the POST, validate the input URL: parse it and ensure it uses an
allowed scheme (https, optionally http only for explicit opt-in), resolve the
hostname to IP(s) and reject loopback, private (RFC1918), link-local, and other
non-public ranges, and/or enforce an explicit allowlist of hostnames or CIDRs
from configuration; perform these checks early in each function (e.g., right
after parsing the url param) and return an OracleError::InvalidInput or similar
if validation fails so that no request is sent to disallowed destinations.
- Around line 126-145: The function fetch_signed_context_batch must reject
responses whose array length doesn't match the requested batch size: after
deserializing response into Vec<OracleResponse> (the response variable) compare
response.len() against the expected request count (obtainable from the request
payload or caller-provided count) and if they differ return an OracleError
(e.g., BatchSizeMismatch with expected and got) instead of silently mapping to
SignedContextV1; update fetch_signed_context_batch to perform this length check
before mapping and returning the Vec<SignedContextV1>.
In `@crates/quote/src/order_quotes.rs`:
- Around line 97-123: The current code awaits
crate::oracle::fetch_signed_context inside the per-pair loop (the signed_context
logic using oracle_url, order_struct, input_index and output_index), causing
serial HTTP POSTs; instead, build and collect futures for all valid IO pairs
(calling fetch_signed_context with the encoded body) into a Vec or a
FuturesUnordered outside the per-pair await path and then await them in parallel
(e.g., futures::future::join_all or stream aggregation), mapping each result
back into the corresponding signed_context (handling Err by logging as done now)
so the orbit of fetch_signed_context calls runs concurrently rather than
sequentially.
---
Outside diff comments:
In `@crates/common/src/take_orders/config.rs`:
- Around line 87-95: The current tests never exercise non-empty signedContext
because candidates are built with make_simulation_candidate (which sets
signed_context: vec![]) and existing execute_single_take tests pass oracle_url =
None; add or update a unit test that constructs a simulation with at least one
leg whose candidate.signed_context is populated (or call the code path that
fetches oracle data so signedContext is set) and then after building
TakeOrderConfigV4 via the map (the orders variable produced from
sim.legs.iter().map(...) that creates TakeOrderConfigV4) assert that
config.orders[i].signedContext is non-empty; reference the TakeOrderConfigV4
construction and the execute_single_take test helper to locate where to inject
the non-empty signed context and the assertion.
In `@crates/quote/src/order_quotes.rs`:
- Around line 53-60: The code computes req_block_number using
ReadableClient::new_from_http_urls(...).get_block_number() but later (around the
oracle context fetch block at lines ~97-106) always requests live oracle state,
causing mismatches for oracle-backed orders when block_number is Some(...); fix
by detecting oracle-backed orders and either rejecting historical quotes (return
an error if block_number.is_some() and the order is oracle-backed) or by making
the oracle request block-aware: extend the oracle-context fetch to accept a
block/timestamp parameter and pass req_block_number there so the oracle snapshot
and chain state are consistent (locate req_block_number, the
ReadableClient::new_from_http_urls/get_block_number call, and the oracle-context
fetch block to implement the check or the parameter plumbing).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2b0bf5c2-a542-4450-9922-8083de28f885
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
crates/common/src/add_order.rscrates/common/src/lib.rscrates/common/src/oracle.rscrates/common/src/parsed_meta.rscrates/common/src/raindex_client/order_quotes.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/take_orders/single.rscrates/common/src/raindex_client/take_orders/single_tests.rscrates/common/src/take_orders/candidates.rscrates/common/src/take_orders/config.rscrates/common/src/test_helpers.rscrates/quote/Cargo.tomlcrates/quote/src/lib.rscrates/quote/src/oracle.rscrates/quote/src/order_quotes.rscrates/settings/src/gui.rscrates/settings/src/order.rscrates/settings/src/yaml/context.rslib/rain.interpreterpackages/ui-components/src/lib/components/detail/OrderDetail.svelte
Comment on lines
+134
to
+147
| // If the order has an oracle URL, add a RaindexSignedContextOracleV1 meta item | ||
| let additional_meta = { | ||
| let mut meta = additional_meta.unwrap_or_default(); | ||
| if let Some(ref oracle_url) = deployment.order.oracle_url { | ||
| let oracle = RaindexSignedContextOracleV1::parse(oracle_url) | ||
| .map_err(AddOrderArgsError::RainMetaError)?; | ||
| meta.push(oracle.to_meta_item()); | ||
| } | ||
| if meta.is_empty() { | ||
| None | ||
| } else { | ||
| Some(meta) | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
Deduplicate oracle meta before appending the deployment URL.
If additional_meta already contains a RaindexSignedContextOracleV1, this block emits two oracle entries. The downstream oracle_url() accessors take the first match, so deployment.order.oracle_url can be silently ignored and later signed-context fetches can hit the wrong endpoint.
Suggested fix
let additional_meta = {
let mut meta = additional_meta.unwrap_or_default();
if let Some(ref oracle_url) = deployment.order.oracle_url {
+ meta.retain(|item| item.magic != KnownMagic::RaindexSignedContextOracleV1);
let oracle = RaindexSignedContextOracleV1::parse(oracle_url)
.map_err(AddOrderArgsError::RainMetaError)?;
meta.push(oracle.to_meta_item());
}
if meta.is_empty() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/common/src/add_order.rs` around lines 134 - 147, The block that
appends a RaindexSignedContextOracleV1 meta item can create duplicates when
additional_meta already contains one; update the logic in the additional_meta
construction (the code that reads deployment.order.oracle_url, calls
RaindexSignedContextOracleV1::parse(...) and pushes oracle.to_meta_item()) to
first scan existing meta for an existing RaindexSignedContextOracleV1 entry (by
matching meta item type/name or parsing existing meta into
RaindexSignedContextOracleV1) and only push the new oracle meta if none exists
or if you explicitly want to replace it; ensure you reference the same symbols
(additional_meta, RaindexSignedContextOracleV1::parse, oracle.to_meta_item,
deployment.order.oracle_url) so the duplicate is prevented or handled
deterministically.
Comment on lines
+117
to
+133
| // Fetch signed context from oracle if URL provided | ||
| let mut candidate = candidate; | ||
| if let Some(url) = oracle_url { | ||
| let body = crate::oracle::encode_oracle_body( | ||
| &candidate.order, | ||
| candidate.input_io_index, | ||
| candidate.output_io_index, | ||
| taker, | ||
| ); | ||
| match crate::oracle::fetch_signed_context(&url, body).await { | ||
| Ok(ctx) => candidate.signed_context = vec![ctx], | ||
| Err(e) => { | ||
| tracing::warn!("Failed to fetch oracle data from {}: {}", url, e); | ||
| } | ||
| } | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Move the oracle fetch below the early exits.
Lines 117-133 run before the price-cap rejection at Lines 136-138 and the approval short-circuit at Lines 151-153. That makes execute_single_take wait on an external POST even when the function is only going to return NoLiquidity or approval calldata.
Also applies to: 136-153
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/common/src/raindex_client/take_orders/single.rs` around lines 117 -
133, The oracle fetch (crate::oracle::fetch_signed_context / encode_oracle_body)
is performed before early exits in execute_single_take, causing unnecessary
external POSTs when the function will immediately return NoLiquidity or approval
calldata; move the entire block that builds body and calls fetch_signed_context
(and the assignment to candidate.signed_context) to after the price-cap
rejection and approval short-circuit checks so it only runs when proceeding to
execution, ensuring oracle_url and taker are still available in that later
scope.
Comment on lines
+84
to
+117
| let oracle_url = { | ||
| #[cfg(target_family = "wasm")] | ||
| { | ||
| order.oracle_url() | ||
| } | ||
| #[cfg(not(target_family = "wasm"))] | ||
| { | ||
| order.oracle_url() | ||
| } | ||
| }; | ||
|
|
||
| for quote in "es { | ||
| let signed_context = match &oracle_url { | ||
| Some(url) => { | ||
| fetch_oracle_for_pair( | ||
| url, | ||
| &order_v4, | ||
| quote.pair.input_index, | ||
| quote.pair.output_index, | ||
| Address::ZERO, // counterparty unknown at candidate building time | ||
| ) | ||
| .await | ||
| } | ||
| None => vec![], | ||
| }; | ||
|
|
||
| if let Some(candidate) = try_build_candidate( | ||
| orderbook, | ||
| &order_v4, | ||
| quote, | ||
| input_token, | ||
| output_token, | ||
| signed_context, | ||
| )? { |
Contributor
There was a problem hiding this comment.
Don't bake Address::ZERO oracle signatures into candidates.
TakeOrderCandidate.signed_context is copied straight into TakeOrderConfigV4.signedContext in crates/common/src/take_orders/config.rs:87-96. Here Line 103 fetches the oracle payload for Address::ZERO, and Lines 137-146 silently downgrade fetch failures to vec![], so a candidate can carry signed context that is either bound to the wrong counterparty or missing entirely. Any oracle that includes the taker in its signature will then fail at preflight/execution, and even taker-agnostic oracles can drift from the quote generated on Line 72 because this path refetches context after quoting. Thread the real counterparty through this builder, or propagate the exact signed context used by get_quotes instead of refetching it here.
Also applies to: 128-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/common/src/take_orders/candidates.rs` around lines 84 - 117, The code
is refetching oracle context with Address::ZERO inside the quotes loop (via
fetch_oracle_for_pair) and can bake wrong or missing signatures into
TakeOrderCandidate.signed_context which later becomes
TakeOrderConfigV4.signedContext; instead, stop refetching with a zero address
and either (A) thread the real counterparty address into this builder and pass
it into fetch_oracle_for_pair so the fetched signed_context is bound to the
correct taker, or (B) propagate the exact signed context produced by get_quotes
into try_build_candidate (and ultimately into TakeOrderCandidate.signed_context)
instead of calling fetch_oracle_for_pair here; update the loop around quotes and
the try_build_candidate call sites accordingly (also apply the same fix to the
analogous block at the later 128-147 region).
Comment on lines
+89
to
+145
| pub async fn fetch_signed_context( | ||
| url: &str, | ||
| body: Vec<u8>, | ||
| ) -> Result<SignedContextV1, OracleError> { | ||
| let builder = Client::builder(); | ||
| #[cfg(not(target_family = "wasm"))] | ||
| let builder = builder.timeout(std::time::Duration::from_secs(10)); | ||
| let client = builder.build()?; | ||
|
|
||
| // For single requests, we still expect a JSON array response but with one item | ||
| let response: Vec<OracleResponse> = client | ||
| .post(url) | ||
| .header("Content-Type", "application/octet-stream") | ||
| .body(body) | ||
| .send() | ||
| .await? | ||
| .error_for_status()? | ||
| .json() | ||
| .await?; | ||
|
|
||
| if response.len() != 1 { | ||
| return Err(OracleError::InvalidResponse(format!( | ||
| "Expected 1 response, got {}", | ||
| response.len() | ||
| ))); | ||
| } | ||
|
|
||
| Ok(response.into_iter().next().unwrap().into()) | ||
| } | ||
|
|
||
| /// Fetch signed context from an oracle endpoint via POST (batch request). | ||
| /// | ||
| /// The endpoint receives an ABI-encoded body containing an array of order details: | ||
| /// `abi.encode((OrderV4, uint256 inputIOIndex, uint256 outputIOIndex, address counterparty)[])` | ||
| /// | ||
| /// The endpoint must respond with a JSON array of `OracleResponse` objects. | ||
| /// The response array length must match the request array length. | ||
| pub async fn fetch_signed_context_batch( | ||
| url: &str, | ||
| body: Vec<u8>, | ||
| ) -> Result<Vec<SignedContextV1>, OracleError> { | ||
| let builder = Client::builder(); | ||
| #[cfg(not(target_family = "wasm"))] | ||
| let builder = builder.timeout(std::time::Duration::from_secs(10)); | ||
| let client = builder.build()?; | ||
|
|
||
| let response: Vec<OracleResponse> = client | ||
| .post(url) | ||
| .header("Content-Type", "application/octet-stream") | ||
| .body(body) | ||
| .send() | ||
| .await? | ||
| .error_for_status()? | ||
| .json() | ||
| .await?; | ||
|
|
||
| Ok(response.into_iter().map(|resp| resp.into()).collect()) |
Contributor
There was a problem hiding this comment.
Treat oracle URLs as untrusted input before issuing POSTs.
These helpers will POST to whatever url they are given, and elsewhere in this flow that URL comes from order metadata/YAML. On non-WASM callers this is an SSRF primitive against internal services and cloud-metadata endpoints. Validate the endpoint before dispatch — at minimum require an allowed scheme and reject loopback/private/link-local targets, and ideally enforce an explicit allowlist for server-side fetching.
Also applies to: 152-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/quote/src/oracle.rs` around lines 89 - 145, The fetch_signed_context
and fetch_signed_context_batch functions currently POST to unvalidated URLs;
before building the reqwest Client and issuing the POST, validate the input URL:
parse it and ensure it uses an allowed scheme (https, optionally http only for
explicit opt-in), resolve the hostname to IP(s) and reject loopback, private
(RFC1918), link-local, and other non-public ranges, and/or enforce an explicit
allowlist of hostnames or CIDRs from configuration; perform these checks early
in each function (e.g., right after parsing the url param) and return an
OracleError::InvalidInput or similar if validation fails so that no request is
sent to disallowed destinations.
Comment on lines
+126
to
+145
| pub async fn fetch_signed_context_batch( | ||
| url: &str, | ||
| body: Vec<u8>, | ||
| ) -> Result<Vec<SignedContextV1>, OracleError> { | ||
| let builder = Client::builder(); | ||
| #[cfg(not(target_family = "wasm"))] | ||
| let builder = builder.timeout(std::time::Duration::from_secs(10)); | ||
| let client = builder.build()?; | ||
|
|
||
| let response: Vec<OracleResponse> = client | ||
| .post(url) | ||
| .header("Content-Type", "application/octet-stream") | ||
| .body(body) | ||
| .send() | ||
| .await? | ||
| .error_for_status()? | ||
| .json() | ||
| .await?; | ||
|
|
||
| Ok(response.into_iter().map(|resp| resp.into()).collect()) |
Contributor
There was a problem hiding this comment.
Reject batch responses whose length doesn't match the request batch.
This helper promises positional batch semantics, but Lines 135-145 accept any array length. A short or long oracle response will silently misalign signed contexts with requests once callers zip/index the result, producing wrong quotes or calldata instead of a hard failure.
🛠 Proposed fix
pub async fn fetch_signed_context_batch(
url: &str,
body: Vec<u8>,
+ expected_len: usize,
) -> Result<Vec<SignedContextV1>, OracleError> {
let builder = Client::builder();
#[cfg(not(target_family = "wasm"))]
let builder = builder.timeout(std::time::Duration::from_secs(10));
let client = builder.build()?;
@@
let response: Vec<OracleResponse> = client
.post(url)
.header("Content-Type", "application/octet-stream")
.body(body)
.send()
@@
.json()
.await?;
- Ok(response.into_iter().map(|resp| resp.into()).collect())
+ if response.len() != expected_len {
+ return Err(OracleError::InvalidResponse(format!(
+ "Expected {expected_len} responses, got {}",
+ response.len()
+ )));
+ }
+
+ Ok(response.into_iter().map(Into::into).collect())
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub async fn fetch_signed_context_batch( | |
| url: &str, | |
| body: Vec<u8>, | |
| ) -> Result<Vec<SignedContextV1>, OracleError> { | |
| let builder = Client::builder(); | |
| #[cfg(not(target_family = "wasm"))] | |
| let builder = builder.timeout(std::time::Duration::from_secs(10)); | |
| let client = builder.build()?; | |
| let response: Vec<OracleResponse> = client | |
| .post(url) | |
| .header("Content-Type", "application/octet-stream") | |
| .body(body) | |
| .send() | |
| .await? | |
| .error_for_status()? | |
| .json() | |
| .await?; | |
| Ok(response.into_iter().map(|resp| resp.into()).collect()) | |
| pub async fn fetch_signed_context_batch( | |
| url: &str, | |
| body: Vec<u8>, | |
| expected_len: usize, | |
| ) -> Result<Vec<SignedContextV1>, OracleError> { | |
| let builder = Client::builder(); | |
| #[cfg(not(target_family = "wasm"))] | |
| let builder = builder.timeout(std::time::Duration::from_secs(10)); | |
| let client = builder.build()?; | |
| let response: Vec<OracleResponse> = client | |
| .post(url) | |
| .header("Content-Type", "application/octet-stream") | |
| .body(body) | |
| .send() | |
| .await? | |
| .error_for_status()? | |
| .json() | |
| .await?; | |
| if response.len() != expected_len { | |
| return Err(OracleError::InvalidResponse(format!( | |
| "Expected {expected_len} responses, got {}", | |
| response.len() | |
| ))); | |
| } | |
| Ok(response.into_iter().map(Into::into).collect()) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/quote/src/oracle.rs` around lines 126 - 145, The function
fetch_signed_context_batch must reject responses whose array length doesn't
match the requested batch size: after deserializing response into
Vec<OracleResponse> (the response variable) compare response.len() against the
expected request count (obtainable from the request payload or caller-provided
count) and if they differ return an OracleError (e.g., BatchSizeMismatch with
expected and got) instead of silently mapping to SignedContextV1; update
fetch_signed_context_batch to perform this length check before mapping and
returning the Vec<SignedContextV1>.
hardyjosh
force-pushed
the
feat/oracle-url-yaml-parsing
branch
from
b48a03d to
9abd25d
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
crates/quote/src/order_quotes.rs (1)
100-120:⚠️ Potential issue | 🟠 MajorDon't await the oracle POST inside the per-pair loop.
This makes one HTTP round trip per valid IO pair before the multicall even starts, so quote latency grows linearly with pair count. Collect these oracle requests and resolve them in parallel, or use the batch oracle path, before building
signedContext.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/quote/src/order_quotes.rs` around lines 100 - 120, The per-pair code is performing an awaited HTTP POST via crate::oracle::fetch_signed_context inside the loop that builds signed_context, causing sequential round-trips; instead, collect the oracle requests when oracle_url.is_some() (capture order_struct, input_index, output_index, Address::ZERO and the URL) into a collection of futures (or IDs to use with the batch oracle path) before constructing signedContext, then resolve them in parallel (e.g., spawn tasks or use futures::future::join_all or call the batch oracle endpoint) and after they complete, map the results back into the signed_context for each pair; update the logic that currently awaits crate::oracle::fetch_signed_context in the loop (referenced by signed_context, crate::oracle::fetch_signed_context, oracle_url, order_struct, input_index, output_index, Address::ZERO) to use the pre-fetched/parallelized results.crates/common/src/add_order.rs (1)
135-140:⚠️ Potential issue | 🟠 MajorReplace any existing oracle meta before pushing the deployment URL.
If
additional_metaalready contains aKnownMagic::RaindexSignedContextOracleV1, this appends a second oracle item. Downstream readers only extract a single oracle entry, so the earlier meta can shadowdeployment.order.oracle_urland send quote/take flows to the wrong endpoint.🛠️ Minimal fix
let additional_meta = { let mut meta = additional_meta.unwrap_or_default(); if let Some(ref oracle_url) = deployment.order.oracle_url { + meta.retain(|item| item.magic != KnownMagic::RaindexSignedContextOracleV1); let oracle = RaindexSignedContextOracleV1::parse(oracle_url) .map_err(AddOrderArgsError::RainMetaError)?; meta.push(oracle.to_meta_item()); } if meta.is_empty() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/common/src/add_order.rs` around lines 135 - 140, When adding the deployment.order.oracle_url into additional_meta, first remove any existing meta items whose magic equals KnownMagic::RaindexSignedContextOracleV1 so you don't append a duplicate; locate the block that builds additional_meta (uses additional_meta.unwrap_or_default(), deployment.order.oracle_url, RaindexSignedContextOracleV1::parse and oracle.to_meta_item()) and filter or retain only non-oracle entries before pushing oracle.to_meta_item() so the new oracle replaces any prior one.crates/common/src/raindex_client/take_orders/single.rs (1)
117-132:⚠️ Potential issue | 🟠 MajorMove the oracle POST behind the cheap exits.
Lines 117-132 run before the price-cap rejection at Lines 136-138 and the approval short-circuit at Lines 151-153, so
execute_single_takecan block on external I/O even when it will return early. Approval-first flows also pay this POST twice: once beforeNeedsApproval, then again on the retry that actually builds calldata. Fetch the signed context only once the function knows it is proceeding to config/simulation.Also applies to: 136-153
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/common/src/raindex_client/take_orders/single.rs` around lines 117 - 132, The oracle POST (fetch_signed_context) is being called too early in execute_single_take and can block or run twice; move the call that sets candidate.signed_context (currently done when oracle_url is Some and using crate::oracle::fetch_signed_context) to after the cheap exits (the price-cap rejection check and the NeedsApproval short-circuit) so it only runs when the function actually proceeds to config/simulation and calldata building, and ensure the retry path does not re-post unnecessarily by fetching the signed context once and reusing it for subsequent calldata construction.crates/common/src/take_orders/candidates.rs (1)
72-80:⚠️ Potential issue | 🔴 CriticalDon't freeze oracle context with a zero taker or empty fallback.
Line 79 binds the oracle request to
Address::ZERO, and Lines 115-123 turn fetch failures intovec![].build_take_orders_config_from_simulationlater copiescandidate.signed_contextstraight intoTakeOrderConfigV4.signedContext, so this can send either the wrong signature or no signature at all to preflight/execution, with no oracle URL left onTakeOrderCandidateto recover later. Defer the fetch until the real taker is known, or carry the oracle URL forward instead of materializingsigned_contexthere. It also POSTs before candidate filtering, so rejected pairs still pay the network round-trip.Also applies to: 113-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/common/src/take_orders/candidates.rs` around lines 72 - 80, The code currently calls fetch_oracle_for_pair early (binding Address::ZERO) and stores a materialized signed_context (or vec![] on error) on TakeOrderCandidate, which can yield wrong/missing signatures and causes unnecessary POSTs; instead, stop calling fetch_oracle_for_pair here (remove the Address::ZERO call) and either (A) defer fetching until you have the real taker in the path that builds TakeOrderConfigV4 so fetch_oracle_for_pair is invoked with the real taker before populating TakeOrderConfigV4.signedContext, or (B) preserve the oracle URL on TakeOrderCandidate (e.g., keep oracle_url: Option<String>) and propagate that into build_take_orders_config_from_simulation so the actual signed_context is fetched there; also avoid swallowing errors into vec![]—return or propagate the error so callers can decide, and ensure you only POST to the oracle after candidate filtering so rejected pairs never trigger network requests (update code paths referencing signed_context, fetch_oracle_for_pair, TakeOrderCandidate, build_take_orders_config_from_simulation, and TakeOrderConfigV4.signedContext).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/ui-components/src/lib/components/detail/OrderDetail.svelte`:
- Around line 215-222: OrderDetail.svelte renders untrusted data.oracleUrl
directly into an anchor href; restrict the link to http(s) schemes before
binding to href to avoid javascript:/data: URIs. Update the rendering so you
validate/sanitize data.oracleUrl (e.g., try to parse with the URL constructor or
check startsWith 'http://' or 'https://') and only set href when the scheme is
http or https; otherwise render the value as plain text (or disable the anchor)
and avoid creating a clickable link. Ensure you change the <a> binding where
data.oracleUrl is used so the href is conditionally set based on that
validation.
---
Duplicate comments:
In `@crates/common/src/add_order.rs`:
- Around line 135-140: When adding the deployment.order.oracle_url into
additional_meta, first remove any existing meta items whose magic equals
KnownMagic::RaindexSignedContextOracleV1 so you don't append a duplicate; locate
the block that builds additional_meta (uses additional_meta.unwrap_or_default(),
deployment.order.oracle_url, RaindexSignedContextOracleV1::parse and
oracle.to_meta_item()) and filter or retain only non-oracle entries before
pushing oracle.to_meta_item() so the new oracle replaces any prior one.
In `@crates/common/src/raindex_client/take_orders/single.rs`:
- Around line 117-132: The oracle POST (fetch_signed_context) is being called
too early in execute_single_take and can block or run twice; move the call that
sets candidate.signed_context (currently done when oracle_url is Some and using
crate::oracle::fetch_signed_context) to after the cheap exits (the price-cap
rejection check and the NeedsApproval short-circuit) so it only runs when the
function actually proceeds to config/simulation and calldata building, and
ensure the retry path does not re-post unnecessarily by fetching the signed
context once and reusing it for subsequent calldata construction.
In `@crates/common/src/take_orders/candidates.rs`:
- Around line 72-80: The code currently calls fetch_oracle_for_pair early
(binding Address::ZERO) and stores a materialized signed_context (or vec![] on
error) on TakeOrderCandidate, which can yield wrong/missing signatures and
causes unnecessary POSTs; instead, stop calling fetch_oracle_for_pair here
(remove the Address::ZERO call) and either (A) defer fetching until you have the
real taker in the path that builds TakeOrderConfigV4 so fetch_oracle_for_pair is
invoked with the real taker before populating TakeOrderConfigV4.signedContext,
or (B) preserve the oracle URL on TakeOrderCandidate (e.g., keep oracle_url:
Option<String>) and propagate that into build_take_orders_config_from_simulation
so the actual signed_context is fetched there; also avoid swallowing errors into
vec![]—return or propagate the error so callers can decide, and ensure you only
POST to the oracle after candidate filtering so rejected pairs never trigger
network requests (update code paths referencing signed_context,
fetch_oracle_for_pair, TakeOrderCandidate,
build_take_orders_config_from_simulation, and TakeOrderConfigV4.signedContext).
In `@crates/quote/src/order_quotes.rs`:
- Around line 100-120: The per-pair code is performing an awaited HTTP POST via
crate::oracle::fetch_signed_context inside the loop that builds signed_context,
causing sequential round-trips; instead, collect the oracle requests when
oracle_url.is_some() (capture order_struct, input_index, output_index,
Address::ZERO and the URL) into a collection of futures (or IDs to use with the
batch oracle path) before constructing signedContext, then resolve them in
parallel (e.g., spawn tasks or use futures::future::join_all or call the batch
oracle endpoint) and after they complete, map the results back into the
signed_context for each pair; update the logic that currently awaits
crate::oracle::fetch_signed_context in the loop (referenced by signed_context,
crate::oracle::fetch_signed_context, oracle_url, order_struct, input_index,
output_index, Address::ZERO) to use the pre-fetched/parallelized results.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 135299e9-a7b4-4665-80cd-c4311ec13bc3
📒 Files selected for processing (11)
crates/common/src/add_order.rscrates/common/src/raindex_client/order_quotes.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/take_orders/single.rscrates/common/src/raindex_client/take_orders/single_tests.rscrates/common/src/take_orders/candidates.rscrates/quote/src/order_quotes.rscrates/settings/src/gui.rscrates/settings/src/order.rscrates/settings/src/yaml/context.rspackages/ui-components/src/lib/components/detail/OrderDetail.svelte
Comment on lines
+215
to
+222
| <a | ||
| href={data.oracleUrl} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| class="break-all text-blue-500 hover:underline" | ||
| > | ||
| {data.oracleUrl} | ||
| </a> |
Contributor
There was a problem hiding this comment.
Restrict the Oracle link to http(s) before binding it to href.
data.oracleUrl comes from order metadata, not a trusted constant. Rendering it directly into an anchor keeps javascript:/data: payloads clickable if upstream validation ever misses one.
🔒 Suggested hardening
+ {`@const` safeOracleUrl = (() => {
+ try {
+ const url = new URL(data.oracleUrl);
+ return url.protocol === 'http:' || url.protocol === 'https:' ? url.href : null;
+ } catch {
+ return null;
+ }
+ })()}
- <a
- href={data.oracleUrl}
- target="_blank"
- rel="noopener noreferrer"
- class="break-all text-blue-500 hover:underline"
- >
- {data.oracleUrl}
- </a>
+ {`#if` safeOracleUrl}
+ <a
+ href={safeOracleUrl}
+ target="_blank"
+ rel="noopener noreferrer"
+ class="break-all text-blue-500 hover:underline"
+ >
+ {data.oracleUrl}
+ </a>
+ {:else}
+ <span class="break-all">{data.oracleUrl}</span>
+ {/if}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui-components/src/lib/components/detail/OrderDetail.svelte` around
lines 215 - 222, OrderDetail.svelte renders untrusted data.oracleUrl directly
into an anchor href; restrict the link to http(s) schemes before binding to href
to avoid javascript:/data: URIs. Update the rendering so you validate/sanitize
data.oracleUrl (e.g., try to parse with the URL constructor or check startsWith
'http://' or 'https://') and only set href when the scheme is http or https;
otherwise render the value as plain text (or disable the anchor) and avoid
creating a clickable link. Ensure you change the <a> binding where
data.oracleUrl is used so the href is conditionally set based on that
validation.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
crates/quote/src/oracle.rs (2)
91-109:⚠️ Potential issue | 🔴 CriticalValidate oracle URLs before sending POSTs.
These helpers still POST to a metadata-derived
urlwithout any allowlisting or public-address checks. On non-WASM callers, that turns order metadata into an SSRF primitive against loopback/private/internal targets. Reject disallowed schemes/hosts beforeclient.post(url).Also applies to: 137-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/quote/src/oracle.rs` around lines 91 - 109, fetch_signed_context currently posts to an arbitrary metadata-derived url without validation, enabling SSRF; before calling client.post(url) parse the url (using Url::parse) and enforce allowed schemes (http/https), then validate the host: reject plain hostnames like "localhost" or names ending with ".local", and reject IP literals in private/loopback/link-local ranges (e.g., 127.0.0.0/8, 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, fe80::/10, ::1); optionally allowlist trusted hostnames or perform DNS resolution and check resolved IPs against those ranges if your threat model requires it; apply the same validation logic to the other helper that posts (the similar fetch helper around the other diff region) so no POST happens until the URL passes these checks.
126-147:⚠️ Potential issue | 🟠 MajorFail batch fetches when the response count differs from the request batch.
The docs promise positional batch semantics, but this helper currently accepts any array length and blindly maps it. A short/long oracle response can shift signed contexts onto the wrong request downstream; compare against the expected batch size and return
OracleError::InvalidResponseon mismatch.🛠 Suggested shape
pub async fn fetch_signed_context_batch( url: &str, body: Vec<u8>, + expected_len: usize, ) -> Result<Vec<SignedContextV1>, OracleError> { @@ let response: Vec<OracleResponse> = client .post(url) .header("Content-Type", "application/octet-stream") .body(body) .send() @@ .json() .await?; - Ok(response.into_iter().map(|resp| resp.into()).collect()) + if response.len() != expected_len { + return Err(OracleError::InvalidResponse(format!( + "Expected {expected_len} responses, got {}", + response.len() + ))); + } + + Ok(response.into_iter().map(Into::into).collect()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/quote/src/oracle.rs` around lines 126 - 147, The function fetch_signed_context_batch currently accepts any response array length; change its signature to accept an expected batch size (e.g., add expected_len: usize) and after deserializing Vec<OracleResponse> check that response.len() == expected_len, returning Err(OracleError::InvalidResponse) on mismatch; only when the lengths match proceed to map each OracleResponse into SignedContextV1 as before (using the existing response.into_iter().map(|resp| resp.into()).collect()). Ensure you reference and use OracleError::InvalidResponse, OracleResponse, SignedContextV1 and the fetch_signed_context_batch function in your update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/quote/src/oracle.rs`:
- Around line 82-90: The doc comment for the single-request oracle fetch is
inconsistent with the implemented JSON contract: update the comment block around
the single-fetch function to state that the endpoint must respond with a JSON
array containing a single OracleResponse (i.e., a one-element array), rather
than a lone OracleResponse object; reference OracleResponse and the existing
note about legacy vs. batch formats so implementers know to return
[OracleResponse] for this endpoint and that the batch format is preferred.
---
Duplicate comments:
In `@crates/quote/src/oracle.rs`:
- Around line 91-109: fetch_signed_context currently posts to an arbitrary
metadata-derived url without validation, enabling SSRF; before calling
client.post(url) parse the url (using Url::parse) and enforce allowed schemes
(http/https), then validate the host: reject plain hostnames like "localhost" or
names ending with ".local", and reject IP literals in
private/loopback/link-local ranges (e.g., 127.0.0.0/8, 10.0.0.0/8,
172.16.0.0/12, 192.168.0.0/16, fe80::/10, ::1); optionally allowlist trusted
hostnames or perform DNS resolution and check resolved IPs against those ranges
if your threat model requires it; apply the same validation logic to the other
helper that posts (the similar fetch helper around the other diff region) so no
POST happens until the URL passes these checks.
- Around line 126-147: The function fetch_signed_context_batch currently accepts
any response array length; change its signature to accept an expected batch size
(e.g., add expected_len: usize) and after deserializing Vec<OracleResponse>
check that response.len() == expected_len, returning
Err(OracleError::InvalidResponse) on mismatch; only when the lengths match
proceed to map each OracleResponse into SignedContextV1 as before (using the
existing response.into_iter().map(|resp| resp.into()).collect()). Ensure you
reference and use OracleError::InvalidResponse, OracleResponse, SignedContextV1
and the fetch_signed_context_batch function in your update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 625f4b34-ce16-48a2-99e1-67e8718384bb
📒 Files selected for processing (1)
crates/quote/src/oracle.rs
Comment on lines
+82
to
+90
| /// Fetch signed context from an oracle endpoint via POST (single request). | ||
| /// | ||
| /// The endpoint receives an ABI-encoded body containing the order details | ||
| /// that will be used for calculateOrderIO: | ||
| /// `abi.encode(OrderV4, uint256 inputIOIndex, uint256 outputIOIndex, address counterparty)` | ||
| /// | ||
| /// The endpoint must respond with a JSON body matching a single `OracleResponse`. | ||
| /// | ||
| /// NOTE: This is a legacy function. The batch format is preferred. |
Contributor
There was a problem hiding this comment.
Align the single-fetch docs with the actual JSON contract.
The comment says the endpoint returns a single OracleResponse object, but the implementation only accepts a one-element JSON array. That mismatch will send oracle implementers to the wrong wire format.
📝 Suggested doc fix
-/// The endpoint must respond with a JSON body matching a single `OracleResponse`.
+/// The endpoint must respond with a JSON array containing exactly one
+/// `OracleResponse`.Also applies to: 100-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/quote/src/oracle.rs` around lines 82 - 90, The doc comment for the
single-request oracle fetch is inconsistent with the implemented JSON contract:
update the comment block around the single-fetch function to state that the
endpoint must respond with a JSON array containing a single OracleResponse
(i.e., a one-element array), rather than a lone OracleResponse object; reference
OracleResponse and the existing note about legacy vs. batch formats so
implementers know to return [OracleResponse] for this endpoint and that the
batch format is preferred.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
crates/quote/src/oracle.rs (3)
124-145:⚠️ Potential issue | 🟠 MajorReject batch responses whose length differs from the request batch.
Lines 124-125 promise positional batch semantics, but Lines 135-145 accept any array length. A short or long response will misalign signed contexts with requests once callers zip by index, producing wrong quotes or calldata instead of a hard failure. Please pass the expected request count into this helper, or derive it from the request payload, and fail on mismatch.
🛠 Possible direction
pub async fn fetch_signed_context_batch( url: &str, body: Vec<u8>, + expected_len: usize, ) -> Result<Vec<SignedContextV1>, OracleError> { @@ let response: Vec<OracleResponse> = client .post(url) .header("Content-Type", "application/octet-stream") .body(body) .send() @@ .json() .await?; + + if response.len() != expected_len { + return Err(OracleError::InvalidResponse(format!( + "Expected {expected_len} responses, got {}", + response.len() + ))); + } - Ok(response.into_iter().map(|resp| resp.into()).collect()) + Ok(response.into_iter().map(Into::into).collect()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/quote/src/oracle.rs` around lines 124 - 145, The fetch_signed_context_batch function must reject responses whose array length differs from the request; change its signature to accept an expected_count: usize (or extract the count from the request body before sending) and after deserializing Vec<OracleResponse> verify response.len() == expected_count, returning an OracleError (e.g., InvalidBatchLength or similar) on mismatch; only after the length check convert each OracleResponse into SignedContextV1 and return the collected Vec<SignedContextV1>.
80-99:⚠️ Potential issue | 🟡 MinorDocument the single-fetch wire format as a one-element array.
Line 86 says the endpoint returns a single
OracleResponse, but Lines 98-116 deserializeVec<OracleResponse>and reject anything except length 1. That mismatch will send oracle implementers to the wrong JSON shape.📝 Proposed doc fix
-/// The endpoint must respond with a JSON body matching a single `OracleResponse`. +/// The endpoint must respond with a JSON array containing exactly one +/// `OracleResponse`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/quote/src/oracle.rs` around lines 80 - 99, The function fetch_signed_context currently deserializes a Vec<OracleResponse> (variable response: Vec<OracleResponse>) and enforces a single-element array, but the doc comment says the endpoint returns a single OracleResponse; update the doc comment for fetch_signed_context to explicitly state the wire format is a JSON array containing exactly one OracleResponse (one-element array) so implementers know to return [OracleResponse] rather than a bare object; reference the OracleResponse type and the fact the code expects Vec<OracleResponse> to guide the change.
89-106:⚠️ Potential issue | 🔴 CriticalValidate oracle URLs before dispatching these POSTs.
Both helpers pass
urlstraight intoreqwest, and elsewhere in this flow that value comes from order metadata/YAML. On server-side callers this is an SSRF primitive against loopback, private, and link-local targets. Parse and validate the URL up front, restrict schemes/hosts, and returnOracleError::InvalidUrlbefore any request is issued.Also applies to: 126-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/quote/src/oracle.rs` around lines 89 - 106, In fetch_signed_context (and the similar POST helper around lines 126-143) parse and validate the incoming url before building the reqwest client or issuing the POST: use Url::parse to ensure a valid URL, enforce allowed schemes (e.g., https only or explicitly whitelist http if required), reject loopback/private/link-local IPs and disallowed hostnames, and return OracleError::InvalidUrl on any validation failure; only after these checks proceed to build the Client and call client.post(...). Ensure the validation logic is applied to the same URL variable used in both helper functions to prevent SSRF.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/quote/src/oracle.rs`:
- Around line 124-145: The fetch_signed_context_batch function must reject
responses whose array length differs from the request; change its signature to
accept an expected_count: usize (or extract the count from the request body
before sending) and after deserializing Vec<OracleResponse> verify
response.len() == expected_count, returning an OracleError (e.g.,
InvalidBatchLength or similar) on mismatch; only after the length check convert
each OracleResponse into SignedContextV1 and return the collected
Vec<SignedContextV1>.
- Around line 80-99: The function fetch_signed_context currently deserializes a
Vec<OracleResponse> (variable response: Vec<OracleResponse>) and enforces a
single-element array, but the doc comment says the endpoint returns a single
OracleResponse; update the doc comment for fetch_signed_context to explicitly
state the wire format is a JSON array containing exactly one OracleResponse
(one-element array) so implementers know to return [OracleResponse] rather than
a bare object; reference the OracleResponse type and the fact the code expects
Vec<OracleResponse> to guide the change.
- Around line 89-106: In fetch_signed_context (and the similar POST helper
around lines 126-143) parse and validate the incoming url before building the
reqwest client or issuing the POST: use Url::parse to ensure a valid URL,
enforce allowed schemes (e.g., https only or explicitly whitelist http if
required), reject loopback/private/link-local IPs and disallowed hostnames, and
return OracleError::InvalidUrl on any validation failure; only after these
checks proceed to build the Client and call client.post(...). Ensure the
validation logic is applied to the same URL variable used in both helper
functions to prevent SSRF.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 10f87c54-6c60-4ba2-bd59-bf40698fa52e
📒 Files selected for processing (1)
crates/quote/src/oracle.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/quote/src/order_quotes.rs (1)
63-144: 🧹 Nitpick | 🔵 TrivialConsider parallelizing oracle fetches per order.
The current implementation awaits each
fetch_signed_contextcall sequentially inside the nested IO pair loops (line 109). For orders with multiple valid IO pairs, this results in N sequential HTTP round trips before the multicall is sent.Collecting the oracle futures and awaiting them in parallel (e.g., via
futures::future::join_all) would reduce latency to the slowest single fetch rather than the sum of all fetches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/quote/src/order_quotes.rs` around lines 63 - 144, The code currently awaits crate::oracle::fetch_signed_context sequentially inside the nested loops (in the loop building Pair and QuoteTarget in order_quotes.rs), causing N HTTP round trips per order; instead, for each order gather futures by calling crate::oracle::fetch_signed_context(url, body) for each (input_index, output_index) pair (create the body with crate::oracle::encode_oracle_body) into a Vec of futures, then await them concurrently with futures::future::join_all, map results into the signed contexts (logging or converting Err -> empty/missing ctx as needed), and use those parallel-resolved contexts when constructing the QuoteV2/signedContext and pushing into all_quote_targets; update references around signed_context, encode_oracle_body, fetch_signed_context, and the construction of QuoteTarget/QuoteV2 to use the precomputed vector of contexts.
♻️ Duplicate comments (6)
crates/quote/src/oracle.rs (3)
89-107:⚠️ Potential issue | 🔴 CriticalReject non-public oracle endpoints before POSTing.
urlis sourced from order metadata/YAML elsewhere in this flow, so non-WASM callers can be driven to POST into loopback, private, or link-local services. Parse and validate the destination before dispatch, or enforce an explicit allowlist for server-side fetching.The search below should show metadata/YAML-derived oracle URLs flowing into these POST sinks with no validation step in between.
#!/bin/bash set -euo pipefail rg -n -C2 --type=rust '\.post\(url\)|fetch_signed_context(_batch)?\(' crates rg -n -C2 --type=rust 'oracle_url\(|extract_oracle_url|RaindexSignedContextOracleV1|oracle[-_]url' crates packagesAlso applies to: 126-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/quote/src/oracle.rs` around lines 89 - 107, fetch_signed_context is directly POSTing to a caller-supplied url (and same pattern in fetch_signed_context_batch) which allows server-side requests to non-public addresses; parse and validate the destination before sending by resolving and checking the URL's scheme/host/IP against a safe policy (reject loopback, private RFC1918/IPv6ULA, link-local, and file/ftp schemes) or enforce an explicit allowlist of hostnames; implement this validation in fetch_signed_context and fetch_signed_context_batch (and any callers like extract_oracle_url/RaindexSignedContextOracleV1) so the client.post(url) is only reached after the URL passes the public/allowlist check, returning an OracleError variant for rejected destinations.
124-145:⚠️ Potential issue | 🟠 MajorEnforce the documented batch-size invariant.
Lines 124-125 promise positional alignment, but Lines 135-145 accept any array length. A short or long reply will misassociate signed contexts with requests instead of failing fast.
🛠 Proposed fix
pub async fn fetch_signed_context_batch( url: &str, body: Vec<u8>, + expected_len: usize, ) -> Result<Vec<SignedContextV1>, OracleError> { let builder = Client::builder(); #[cfg(not(target_family = "wasm"))] let builder = builder.timeout(std::time::Duration::from_secs(10)); let client = builder.build()?; @@ let response: Vec<OracleResponse> = client .post(url) .header("Content-Type", "application/octet-stream") .body(body) .send() @@ .json() .await?; - Ok(response.into_iter().map(|resp| resp.into()).collect()) + if response.len() != expected_len { + return Err(OracleError::InvalidResponse(format!( + "Expected {expected_len} responses, got {}", + response.len() + ))); + } + + Ok(response.into_iter().map(Into::into).collect()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/quote/src/oracle.rs` around lines 124 - 145, fetch_signed_context_batch currently ignores the documented invariant that the response array length must match the request array length; ensure you validate lengths and fail fast if they differ: either (A) derive the expected request count by deserializing the outgoing body into the same request-array type used by callers and set expected_len, or (B) add an explicit expected_len: usize parameter to fetch_signed_context_batch and use that; after receiving response: let response: Vec<OracleResponse> = ...; if response.len() != expected_len { return Err(OracleError::InvalidResponseLength { expected: expected_len, got: response.len() }); } then map response.into_iter().map(|resp| resp.into()).collect() into Vec<SignedContextV1>; update callers and the OracleError enum/type to include InvalidResponseLength if it doesn't exist.
80-88:⚠️ Potential issue | 🟡 MinorFix the single-fetch response docs.
The code parses a one-element JSON array, not a bare
OracleResponse. Following this comment as written will fail against Lines 98-116.📝 Suggested doc fix
-/// The endpoint must respond with a JSON body matching a single `OracleResponse`. +/// The endpoint must respond with a JSON array containing exactly one +/// `OracleResponse`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/quote/src/oracle.rs` around lines 80 - 88, The doc comment for the single-fetch routine ("Fetch signed context from an oracle endpoint via POST (single request)") is incorrect: the code parses a one-element JSON array, not a bare OracleResponse. Update the comment in crates/quote/src/oracle.rs so the endpoint is documented to respond with a JSON array containing a single OracleResponse (e.g., "a JSON array with one OracleResponse"), keeping references to calculateOrderIO and the single-request behavior to match the parsing logic that extracts the single-element array.crates/common/src/raindex_client/take_orders/single.rs (1)
117-183:⚠️ Potential issue | 🟠 MajorMove the oracle fetch below the local short-circuits.
This now awaits the oracle before the cheap
NoLiquidityand approval exits, so calls that should return immediately still block on an extra network hop. Fetch the signed context only once the take is definitely proceeding.♻️ Suggested change
- if let Some(url) = oracle_url { - let body = crate::oracle::encode_oracle_body( - &candidate.order, - candidate.input_io_index, - candidate.output_io_index, - taker, - ); - match crate::oracle::fetch_signed_context(&url, body).await { - Ok(ctx) => candidate.signed_context = vec![ctx], - Err(e) => { - tracing::warn!("Failed to fetch oracle data from {}: {}", url, e); - } - } - } - let zero = Float::zero()?; @@ if output.lte(zero)? { return Err(RaindexError::NoLiquidity); } + + if let Some(url) = oracle_url.as_deref() { + let body = crate::oracle::encode_oracle_body( + &candidate.order, + candidate.input_io_index, + candidate.output_io_index, + taker, + ); + match crate::oracle::fetch_signed_context(url, body).await { + Ok(ctx) => candidate.signed_context = vec![ctx], + Err(e) => { + tracing::warn!("Failed to fetch oracle data from {}: {}", url, e); + } + } + } let sim = SimulationResult {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/common/src/raindex_client/take_orders/single.rs` around lines 117 - 183, The oracle fetch (the block using oracle_url, crate::oracle::encode_oracle_body and crate::oracle::fetch_signed_context that sets candidate.signed_context) should be moved below the cheap local short-circuits: after the price_cap check (candidate.ratio.gt(price_cap) -> NoLiquidity) and after the approval check via check_approval_needed(&approval_params). Relocate that entire oracle fetch block so it runs only when the function is definitely proceeding (i.e., after computing approval_result and confirming we will not early-return), making sure candidate.signed_context is still set before any later use.crates/common/src/add_order.rs (1)
139-152:⚠️ Potential issue | 🟠 MajorDeduplicate oracle meta before appending.
If
additional_metaalready contains aRaindexSignedContextOracleV1entry, this block will emit two oracle entries. The downstreamoracle_url()accessor takes the first match, so the deployment's oracle URL may be silently ignored.Suggested fix
let additional_meta = { let mut meta = additional_meta.unwrap_or_default(); if let Some(ref oracle_url) = deployment.order.oracle_url { + meta.retain(|item| item.magic != KnownMagic::RaindexSignedContextOracleV1); let oracle = RaindexSignedContextOracleV1::parse(oracle_url) .map_err(AddOrderArgsError::RainMetaError)?; meta.push(oracle.to_meta_item()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/common/src/add_order.rs` around lines 139 - 152, The block that builds additional_meta may append a second RaindexSignedContextOracleV1 entry, so before pushing the parsed oracle (from RaindexSignedContextOracleV1::parse -> to_meta_item) check existing additional_meta for an existing RaindexSignedContextOracleV1 meta item and skip pushing if one exists; modify the code around additional_meta (where deployment.order.oracle_url is handled and AddOrderArgsError::RainMetaError is used) to search the current meta vector for an oracle meta kind/name and only call meta.push(...) when no duplicate is found.crates/common/src/take_orders/candidates.rs (1)
72-84:⚠️ Potential issue | 🔴 Critical
Address::ZEROcounterparty will produce invalid oracle signatures for taker-aware oracles.The
fetch_oracle_for_paircall usesAddress::ZEROas the counterparty (line 79). This signed context is stored inTakeOrderCandidate.signed_contextand later copied intoTakeOrderConfigV4.signedContextfor execution.If the oracle includes the counterparty address in its signature, the resulting signed context will be bound to
Address::ZEROrather than the actual taker, causing signature verification failures at execution time. Either:
- Thread the real counterparty address through this builder, or
- Defer oracle fetching until the actual taker is known
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/common/src/take_orders/candidates.rs` around lines 72 - 84, The code calls fetch_oracle_for_pair(..., Address::ZERO) which binds oracle signatures to zero instead of the real taker, causing verification failures when copied into TakeOrderCandidate.signed_context and then into TakeOrderConfigV4.signedContext; fix by threading the real counterparty address through the builder that creates TakeOrderCandidate (add a counterparty: Address parameter to the function constructing the candidate and propagate it to where fetch_oracle_for_pair is invoked) and replace Address::ZERO with that counterparty; alternatively, if threading is infeasible, defer calling fetch_oracle_for_pair until the actual taker address is known and populate TakeOrderCandidate.signed_context only at that time.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/common/src/raindex_client/take_orders/single.rs`:
- Around line 126-130: The warning currently logs the full oracle URL (variable
url) which may contain sensitive userinfo or query params; update the Err branch
where fetch_signed_context is called to redact the URL by parsing url (e.g.,
with Url::parse) and logging only a safe identifier such as the origin/host:port
or a fixed label like "[REDACTED_ORACLE_URL]" if parsing fails, then pass that
redacted value to tracing::warn instead of the raw url; keep the function
reference crate::oracle::fetch_signed_context and the candidate.signed_context
assignment unchanged.
In `@crates/quote/src/order_quotes.rs`:
- Around line 100-126: The token-equality check inside the signed_context block
is redundant because the outer loop already skips same-token pairs; remove the
inner if input.token != output.token { ... } else { vec![] } and always execute
the body that builds body via crate::oracle::encode_oracle_body and calls
crate::oracle::fetch_signed_context(url, body), returning Ok(ctx) -> vec![ctx]
or Err -> vec![] with the existing tracing::warn; update the signed_context
assignment to directly handle Some(ref url) => match fetch_signed_context(...)
to eliminate the dead else branch while keeping order_struct, input_index,
output_index, Address::ZERO and the tracing::warn behavior intact.
---
Outside diff comments:
In `@crates/quote/src/order_quotes.rs`:
- Around line 63-144: The code currently awaits
crate::oracle::fetch_signed_context sequentially inside the nested loops (in the
loop building Pair and QuoteTarget in order_quotes.rs), causing N HTTP round
trips per order; instead, for each order gather futures by calling
crate::oracle::fetch_signed_context(url, body) for each (input_index,
output_index) pair (create the body with crate::oracle::encode_oracle_body) into
a Vec of futures, then await them concurrently with futures::future::join_all,
map results into the signed contexts (logging or converting Err -> empty/missing
ctx as needed), and use those parallel-resolved contexts when constructing the
QuoteV2/signedContext and pushing into all_quote_targets; update references
around signed_context, encode_oracle_body, fetch_signed_context, and the
construction of QuoteTarget/QuoteV2 to use the precomputed vector of contexts.
---
Duplicate comments:
In `@crates/common/src/add_order.rs`:
- Around line 139-152: The block that builds additional_meta may append a second
RaindexSignedContextOracleV1 entry, so before pushing the parsed oracle (from
RaindexSignedContextOracleV1::parse -> to_meta_item) check existing
additional_meta for an existing RaindexSignedContextOracleV1 meta item and skip
pushing if one exists; modify the code around additional_meta (where
deployment.order.oracle_url is handled and AddOrderArgsError::RainMetaError is
used) to search the current meta vector for an oracle meta kind/name and only
call meta.push(...) when no duplicate is found.
In `@crates/common/src/raindex_client/take_orders/single.rs`:
- Around line 117-183: The oracle fetch (the block using oracle_url,
crate::oracle::encode_oracle_body and crate::oracle::fetch_signed_context that
sets candidate.signed_context) should be moved below the cheap local
short-circuits: after the price_cap check (candidate.ratio.gt(price_cap) ->
NoLiquidity) and after the approval check via
check_approval_needed(&approval_params). Relocate that entire oracle fetch block
so it runs only when the function is definitely proceeding (i.e., after
computing approval_result and confirming we will not early-return), making sure
candidate.signed_context is still set before any later use.
In `@crates/common/src/take_orders/candidates.rs`:
- Around line 72-84: The code calls fetch_oracle_for_pair(..., Address::ZERO)
which binds oracle signatures to zero instead of the real taker, causing
verification failures when copied into TakeOrderCandidate.signed_context and
then into TakeOrderConfigV4.signedContext; fix by threading the real
counterparty address through the builder that creates TakeOrderCandidate (add a
counterparty: Address parameter to the function constructing the candidate and
propagate it to where fetch_oracle_for_pair is invoked) and replace
Address::ZERO with that counterparty; alternatively, if threading is infeasible,
defer calling fetch_oracle_for_pair until the actual taker address is known and
populate TakeOrderCandidate.signed_context only at that time.
In `@crates/quote/src/oracle.rs`:
- Around line 89-107: fetch_signed_context is directly POSTing to a
caller-supplied url (and same pattern in fetch_signed_context_batch) which
allows server-side requests to non-public addresses; parse and validate the
destination before sending by resolving and checking the URL's scheme/host/IP
against a safe policy (reject loopback, private RFC1918/IPv6ULA, link-local, and
file/ftp schemes) or enforce an explicit allowlist of hostnames; implement this
validation in fetch_signed_context and fetch_signed_context_batch (and any
callers like extract_oracle_url/RaindexSignedContextOracleV1) so the
client.post(url) is only reached after the URL passes the public/allowlist
check, returning an OracleError variant for rejected destinations.
- Around line 124-145: fetch_signed_context_batch currently ignores the
documented invariant that the response array length must match the request array
length; ensure you validate lengths and fail fast if they differ: either (A)
derive the expected request count by deserializing the outgoing body into the
same request-array type used by callers and set expected_len, or (B) add an
explicit expected_len: usize parameter to fetch_signed_context_batch and use
that; after receiving response: let response: Vec<OracleResponse> = ...; if
response.len() != expected_len { return Err(OracleError::InvalidResponseLength {
expected: expected_len, got: response.len() }); } then map
response.into_iter().map(|resp| resp.into()).collect() into
Vec<SignedContextV1>; update callers and the OracleError enum/type to include
InvalidResponseLength if it doesn't exist.
- Around line 80-88: The doc comment for the single-fetch routine ("Fetch signed
context from an oracle endpoint via POST (single request)") is incorrect: the
code parses a one-element JSON array, not a bare OracleResponse. Update the
comment in crates/quote/src/oracle.rs so the endpoint is documented to respond
with a JSON array containing a single OracleResponse (e.g., "a JSON array with
one OracleResponse"), keeping references to calculateOrderIO and the
single-request behavior to match the parsing logic that extracts the
single-element array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2cf0a617-fa51-46c7-838b-e5b55cb44417
📒 Files selected for processing (13)
crates/common/src/add_order.rscrates/common/src/raindex_client/order_quotes.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_client/take_orders/single.rscrates/common/src/raindex_client/take_orders/single_tests.rscrates/common/src/take_orders/candidates.rscrates/common/src/take_orders/config.rscrates/common/src/test_helpers.rscrates/quote/src/oracle.rscrates/quote/src/order_quotes.rscrates/settings/src/gui.rscrates/settings/src/order.rscrates/settings/src/yaml/context.rs
Comment on lines
+126
to
+130
| match crate::oracle::fetch_signed_context(&url, body).await { | ||
| Ok(ctx) => candidate.signed_context = vec![ctx], | ||
| Err(e) => { | ||
| tracing::warn!("Failed to fetch oracle data from {}: {}", url, e); | ||
| } |
Contributor
There was a problem hiding this comment.
Redact the configured oracle URL in warning logs.
oracle_url is config input, so it can carry userinfo or signed query parameters. Logging it verbatim on fetch failures leaks that material into application logs; log only the host/origin or a fixed redacted label instead.
🔒 Suggested redaction
- Err(e) => {
- tracing::warn!("Failed to fetch oracle data from {}: {}", url, e);
- }
+ Err(e) => {
+ let oracle_host = Url::parse(&url)
+ .ok()
+ .and_then(|parsed| parsed.host_str().map(str::to_owned))
+ .unwrap_or_else(|| "<redacted>".to_string());
+ tracing::warn!(
+ "Failed to fetch oracle data from oracle host {}: {}",
+ oracle_host,
+ e
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| match crate::oracle::fetch_signed_context(&url, body).await { | |
| Ok(ctx) => candidate.signed_context = vec![ctx], | |
| Err(e) => { | |
| tracing::warn!("Failed to fetch oracle data from {}: {}", url, e); | |
| } | |
| match crate::oracle::fetch_signed_context(&url, body).await { | |
| Ok(ctx) => candidate.signed_context = vec![ctx], | |
| Err(e) => { | |
| let oracle_host = Url::parse(&url) | |
| .ok() | |
| .and_then(|parsed| parsed.host_str().map(str::to_owned)) | |
| .unwrap_or_else(|| "<redacted>".to_string()); | |
| tracing::warn!( | |
| "Failed to fetch oracle data from oracle host {}: {}", | |
| oracle_host, | |
| e | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/common/src/raindex_client/take_orders/single.rs` around lines 126 -
130, The warning currently logs the full oracle URL (variable url) which may
contain sensitive userinfo or query params; update the Err branch where
fetch_signed_context is called to redact the URL by parsing url (e.g., with
Url::parse) and logging only a safe identifier such as the origin/host:port or a
fixed label like "[REDACTED_ORACLE_URL]" if parsing fails, then pass that
redacted value to tracing::warn instead of the raw url; keep the function
reference crate::oracle::fetch_signed_context and the candidate.signed_context
assignment unchanged.
Comment on lines
+100
to
+126
| // Fetch signed oracle context for this pair if oracle URL is present | ||
| let signed_context = if let Some(ref url) = oracle_url { | ||
| if input.token != output.token { | ||
| let body = crate::oracle::encode_oracle_body( | ||
| &order_struct, | ||
| input_index as u32, | ||
| output_index as u32, | ||
| Address::ZERO, // counterparty unknown at quote time | ||
| ); | ||
| match crate::oracle::fetch_signed_context(url, body).await { | ||
| Ok(ctx) => vec![ctx], | ||
| Err(e) => { | ||
| tracing::warn!( | ||
| "Failed to fetch oracle for pair ({}, {}): {}", | ||
| input_index, | ||
| output_index, | ||
| e | ||
| ); | ||
| vec![] | ||
| } | ||
| } | ||
| } else { | ||
| vec![] | ||
| } | ||
| } else { | ||
| vec![] | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove redundant token equality check.
The condition if input.token != output.token on line 102 is always true at this point because the outer loop already skips same-token pairs with if input.token == output.token { continue; } at lines 70-72. The else branch at lines 121-123 is dead code.
♻️ Proposed fix
// Fetch signed oracle context for this pair if oracle URL is present
let signed_context = if let Some(ref url) = oracle_url {
- if input.token != output.token {
- let body = crate::oracle::encode_oracle_body(
- &order_struct,
- input_index as u32,
- output_index as u32,
- Address::ZERO, // counterparty unknown at quote time
- );
- match crate::oracle::fetch_signed_context(url, body).await {
- Ok(ctx) => vec![ctx],
- Err(e) => {
- tracing::warn!(
- "Failed to fetch oracle for pair ({}, {}): {}",
- input_index,
- output_index,
- e
- );
- vec![]
- }
+ let body = crate::oracle::encode_oracle_body(
+ &order_struct,
+ input_index as u32,
+ output_index as u32,
+ Address::ZERO, // counterparty unknown at quote time
+ );
+ match crate::oracle::fetch_signed_context(url, body).await {
+ Ok(ctx) => vec![ctx],
+ Err(e) => {
+ tracing::warn!(
+ "Failed to fetch oracle for pair ({}, {}): {}",
+ input_index,
+ output_index,
+ e
+ );
+ vec![]
}
- } else {
- vec![]
}
} else {
vec![]
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Fetch signed oracle context for this pair if oracle URL is present | |
| let signed_context = if let Some(ref url) = oracle_url { | |
| if input.token != output.token { | |
| let body = crate::oracle::encode_oracle_body( | |
| &order_struct, | |
| input_index as u32, | |
| output_index as u32, | |
| Address::ZERO, // counterparty unknown at quote time | |
| ); | |
| match crate::oracle::fetch_signed_context(url, body).await { | |
| Ok(ctx) => vec![ctx], | |
| Err(e) => { | |
| tracing::warn!( | |
| "Failed to fetch oracle for pair ({}, {}): {}", | |
| input_index, | |
| output_index, | |
| e | |
| ); | |
| vec![] | |
| } | |
| } | |
| } else { | |
| vec![] | |
| } | |
| } else { | |
| vec![] | |
| }; | |
| // Fetch signed oracle context for this pair if oracle URL is present | |
| let signed_context = if let Some(ref url) = oracle_url { | |
| let body = crate::oracle::encode_oracle_body( | |
| &order_struct, | |
| input_index as u32, | |
| output_index as u32, | |
| Address::ZERO, // counterparty unknown at quote time | |
| ); | |
| match crate::oracle::fetch_signed_context(url, body).await { | |
| Ok(ctx) => vec![ctx], | |
| Err(e) => { | |
| tracing::warn!( | |
| "Failed to fetch oracle for pair ({}, {}): {}", | |
| input_index, | |
| output_index, | |
| e | |
| ); | |
| vec![] | |
| } | |
| } | |
| } else { | |
| vec![] | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/quote/src/order_quotes.rs` around lines 100 - 126, The token-equality
check inside the signed_context block is redundant because the outer loop
already skips same-token pairs; remove the inner if input.token != output.token
{ ... } else { vec![] } and always execute the body that builds body via
crate::oracle::encode_oracle_body and calls
crate::oracle::fetch_signed_context(url, body), returning Ok(ctx) -> vec![ctx]
or Err -> vec![] with the existing tracing::warn; update the signed_context
assignment to directly handle Some(ref url) => match fetch_signed_context(...)
to eliminate the dead else branch while keeping order_struct, input_index,
output_index, Address::ZERO and the tracing::warn behavior intact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merges the full reviewed oracle signed context stack into main.
This branch accumulates all the reviewed+approved stacked PRs:
All code was previously reviewed and approved in the stacked PRs.
Summary by CodeRabbit
New Features
Configuration
User Interface
Behavior