feat: add signed context oracle support#436
Closed
hardyjosh wants to merge 13 commits into2026-03-27-v6-calldata-fixfrom
Closed
feat: add signed context oracle support#436hardyjosh wants to merge 13 commits into2026-03-27-v6-calldata-fixfrom
hardyjosh wants to merge 13 commits into2026-03-27-v6-calldata-fixfrom
Conversation
1. Add meta field to subgraph queries for order discovery 2. Create oracle module with: - extractOracleUrl() placeholder for meta parsing - fetchSignedContext() for batch oracle requests - Support for batch format (array of contexts) 3. Wire oracle into quoting logic: - Extract oracle URL from order meta before quote2 - Fetch signed context and inject into takeOrder struct - Graceful fallback on oracle failures 4. Ensure signed context flows through to takeOrdersConfig The solver now automatically fetches oracle data for orders that specify an oracle-url, enabling external data integration.
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 an oracle integration: extracts oracle URL from order meta, tracks per-oracle health/cooloff, ABI-encodes and POSTs OrderV4 requests to fetch signed contexts with timeout/validation, injects signedContext into V4 orders before quoting, and extends subgraph queries to include order Changes
sequenceDiagram
actor Client
participant QuoteService as Quote Service
participant OracleModule as Oracle Module
participant OracleEndpoint as Oracle Endpoint
Client->>QuoteService: quoteSingleOrderV4(order)
QuoteService->>QuoteService: check order.meta
alt meta present
QuoteService->>OracleModule: extractOracleUrl(metaHex)
OracleModule-->>QuoteService: url or undefined
alt url found and not in cooloff
QuoteService->>OracleModule: fetchSignedContext(url, request, state.oracleHealth)
OracleModule->>OracleModule: ABI-encode OrderV4 request
OracleModule->>OracleEndpoint: POST octet-stream (with timeout)
OracleEndpoint-->>OracleModule: HTTP 200 + JSON
OracleModule->>OracleModule: validate SignedContextV2 shape
OracleModule-->>QuoteService: SignedContextV2
QuoteService->>QuoteService: inject signedContext into orderDetails
else url unknown / in cooloff / fetch fails
OracleModule-->>QuoteService: OracleError (propagated)
end
end
QuoteService->>QuoteService: perform on-chain quote via state.client.call(...)
QuoteService-->>Client: return quote result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/oracle/index.ts`:
- Around line 89-93: Validate the oracle JSON before casting to
SignedContextV1[]: after parsing into contexts, iterate over each item and
ensure it has the required keys and types (e.g., that "signer" is a string,
"signature" is a string and/or bytes representation, and "context" is an object
or array as expected) and reject or throw a descriptive error for any malformed
entry; update the code around the contexts variable and the SignedContextV1
validation logic (the parsing block that populates contexts and the code that
later uses contexts for takeOrder) to perform these checks and sanitize/convert
fields as needed before returning or passing into takeOrder.
- Around line 36-41: The OracleOrderRequest currently types order as any;
replace that with the concrete OrderV4 type (e.g., use OrderV4) so the compiler
enforces structural compatibility during ABI encoding. Import or reference the
resolved OrderV4 type where OracleOrderRequest is declared, update the interface
field to order: OrderV4, and run TypeScript/tsc to fix any resulting type
mismatches in functions that construct or pass OracleOrderRequest objects
(adjust those call sites to produce a valid OrderV4).
- Around line 77-83: The fetch call that posts the oracle request (the statement
creating response = await fetch(url, { method: 'POST', headers: {...}, body:
ethers.getBytes(body) })) has no timeout and can hang; wrap the request with an
AbortController (or use AbortSignal.timeout(ms)) and pass its signal to fetch,
enforce a reasonable deadline (e.g., 5–10s), and ensure you abort the controller
on timeout; update the code around the fetch invocation to create controller =
new AbortController(), pass signal: controller.signal into fetch, use
setTimeout(() => controller.abort(), TIMEOUT_MS) (or AbortSignal.timeout) and
handle the abort in the existing error handling so the quoting pipeline fails
fast instead of blocking.
- Around line 12-18: The extractOracleUrl function currently logs a console.warn
on every invocation which will flood logs; change this by removing the per-call
console.warn or replace it with a one-time warning emitted via a module-level
flag (e.g., a let warned = false at module scope) so the message is logged only
once, or eliminate the log altogether and simply return null; update the
extractOracleUrl function to reference the module-level flag (or remove the
call) and ensure no per-invocation console.warn remains.
- Line 1: The code is using ethers v6 APIs but the project depends on ethers
v5.7.0; update the v6-specific calls to their v5 equivalents: replace uses of
ethers.AbiCoder.defaultAbiCoder() with ethers.utils.defaultAbiCoder and replace
ethers.getBytes(...) with ethers.utils.arrayify(...). Locate these calls in
src/oracle/index.ts (references around the symbols AbiCoder.defaultAbiCoder and
getBytes) and update any related variable names/usages so they call
ethers.utils.* accordingly, ensuring types/inputs remain the same.
- Around line 69-75: The code is ABI-encoding OrderV4 tuples using a hardcoded
tuple signature; replace that raw string with the canonical ABI constant from
the common ABI module. Import the Orderbook ABI export (e.g., Orderbook or
Orderbook.V5) and use its Order/OrderV4 type or parsed ABI parameter (via
parseAbiParameters if needed) in the abiCoder.encode call instead of the inline
tuple string so the encode uses the shared OrderV4 definition; update the import
and pass the referenced ABI constant where the current abiCoder.encode([...],
[tuples]) call appears.
In `@src/order/quote.ts`:
- Around line 42-63: Extract the duplicated oracle-fetch block into a private
helper named fetchOracleContext that accepts the orderDetails object, uses
extractOracleUrl(orderMeta) and fetchSignedContext(oracleUrl, [...]) to obtain
signed context, and updates orderDetails.takeOrder.struct.signedContext; then
replace the inline try/catch in quoteSingleOrderV3 and quoteSingleOrderV4 with a
call to await fetchOracleContext(orderDetails) wrapped in the same try/catch
that logs the warning on failure (preserve the same warning text), ensuring you
reference the existing functions fetchSignedContext and extractOracleUrl and the
same orderDetails.takeOrder.struct shape so behavior is unchanged.
- Around line 42-63: The oracle metadata access is wrong because
quoteSingleOrderV3 and quoteSingleOrderV4 receive a Pair (PairV3 | PairV4) that
doesn’t contain the original SgOrder.meta, so the fetch block using
(orderDetails as any).orderDetails?.meta is unreachable; fix by changing the
function(s) signature to accept the original SgOrder (or at least its
meta/oracle URL) alongside the Pair, or alternatively persist the SgOrder.meta
onto the Pair when the Pair is constructed so extractOracleUrl can read it; then
factor the duplicated fetch logic into a shared helper (e.g.,
fetchSignedContextForOrder) that calls extractOracleUrl and fetchSignedContext
and sets takeOrder.struct.signedContext (referencing
orderDetails.takeOrder.struct.signedContext) and use that helper from both
quoteSingleOrderV3 and quoteSingleOrderV4.
- Fix ethers v6 → v5 APIs (defaultAbiCoder, arrayify) - Use ABI.Orderbook.V5.OrderV4 constant instead of hardcoded tuple string - Add 5s timeout on oracle fetch via AbortController - Validate SignedContextV1 shape on each response entry - Extract fetchOracleContext helper to deduplicate quote logic - Remove noisy console.warn from stub extractOracleUrl - Type OracleOrderRequest.order properly instead of any
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/order/quote.ts (1)
14-16:⚠️ Potential issue | 🟠 Major
(orderDetails as any).metawill always beundefined— oracle fetch is dead code.
orderDetailsis typed asPair(PairV3 | PairV4), andas anyis required becausemetais not a member of thePairtype. Consequently,orderMetais alwaysundefined, the guard on Line 16 returns immediately every time, and oracle signed context is never fetched — making the entire oracle integration a no-op.The previous form
(orderDetails as any).orderDetails?.metaexhibited the same issue; the path was shortened but the root cause (the originalSgOrder.metais not surfaced onPair) was not addressed. The fix requires one of:
- Adding a
meta?: stringfield toPairBase(or the variant types) and populating it when constructing thePairfrom anSgOrder, or- Changing
quoteSingleOrderV3/quoteSingleOrderV4(andquoteSingleOrder) to accept the originalSgOrder(or at least its oracle URL) alongside thePair.#!/bin/bash # Verify whether Pair / PairBase / PairV3 / PairV4 expose a `meta` field rg -n "meta" --type=ts src/order/types/ -C 2 # Also check if SgOrder carries meta and whether it's threaded into Pair construction rg -n "meta" --type=ts src/subgraph/types.ts -C 2 rg -n "fromArgs\|new Pair\|PairV3\|PairV4" --type=ts src/order/types/ -A 10 | grep -A 5 "meta"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/order/quote.ts` around lines 14 - 16, fetchOracleContext is dead because (orderDetails as any).meta is always undefined—the Pair type (PairBase / PairV3 / PairV4) doesn’t expose SgOrder.meta—so oracle logic never runs. Fix by either: 1) add an optional meta?: string to PairBase (and relevant PairV3/PairV4) and ensure the Pair factory/constructor that converts an SgOrder copies SgOrder.meta into the Pair, replacing the (orderDetails as any).meta access; or 2) change quoteSingleOrderV3 / quoteSingleOrderV4 (and quoteSingleOrder) to accept the original SgOrder (or at least its oracle URL) alongside the Pair and pass that through to fetchOracleContext, then remove the incorrect cast in fetchOracleContext. Ensure fetchOracleContext reads the strongly typed field (meta or provided oracle URL) so oracle fetching actually executes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/oracle/index.ts`:
- Around line 110-122: The current validation in the SignedContextV1 mapping
(the contexts constant in src/oracle/index.ts) only checks that entry.context is
an array but not that its items are strings; update the validator inside the
json.map where you check (entry as any).context to ensure Array.isArray((entry
as any).context) && ((entry as any).context).every((c: unknown) => typeof c ===
"string") so that non-string elements are rejected and the thrown Error message
still references the index i; keep returning entry as SignedContextV1 after the
stricter check.
---
Duplicate comments:
In `@src/order/quote.ts`:
- Around line 14-16: fetchOracleContext is dead because (orderDetails as
any).meta is always undefined—the Pair type (PairBase / PairV3 / PairV4) doesn’t
expose SgOrder.meta—so oracle logic never runs. Fix by either: 1) add an
optional meta?: string to PairBase (and relevant PairV3/PairV4) and ensure the
Pair factory/constructor that converts an SgOrder copies SgOrder.meta into the
Pair, replacing the (orderDetails as any).meta access; or 2) change
quoteSingleOrderV3 / quoteSingleOrderV4 (and quoteSingleOrder) to accept the
original SgOrder (or at least its oracle URL) alongside the Pair and pass that
through to fetchOracleContext, then remove the incorrect cast in
fetchOracleContext. Ensure fetchOracleContext reads the strongly typed field
(meta or provided oracle URL) so oracle fetching actually executes.
added 4 commits
February 23, 2026 20:59
Replace ethers.utils.defaultAbiCoder/arrayify with viem's encodeAbiParameters/hexToBytes. Use proper viem ABI parameter definitions instead of string-based encoding.
- Up to 2 retries with exponential backoff (500ms, 1s) - After 3 consecutive failures, oracle URL enters 5min cooloff - During cooloff, requests to that URL are skipped immediately - Cooloff resets on first successful response - Invalid responses (bad shape, wrong length) also count as failures - All configurable via module constants
No retries, no delays in the loop. Single attempt with 5s timeout — if it fails, record the failure and move on. After 3 consecutive failures the URL enters a 5min cooloff where it's skipped immediately (no network call at all). This way one bad oracle can't block the processing of other orders.
…ager Extract oracle cooloff tracking from module-level singleton into an OracleManager class. Instance lives on OrderManager, threaded through to quote functions. This makes it properly scoped to the solver instance lifecycle and testable. - OracleManager class in src/oracle/manager.ts - fetchSignedContext takes OracleManager as parameter - OrderManager creates and owns the OracleManager instance - OracleManager is optional in quote functions for backward compat
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/oracle/index.ts (1)
150-162:⚠️ Potential issue | 🟡 Minor
contextarray elements are not validated as strings.Line 156 only verifies
Array.isArray((entry as any).context)without checking that every element is astring. A malformed oracle response with non-string entries silently passes validation.🛡️ Proposed fix
!Array.isArray((entry as any).context) || + !(entry as any).context.every((elem: unknown) => typeof elem === "string") || typeof (entry as any).signature !== "string"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/oracle/index.ts` around lines 150 - 162, The validation for SignedContextV1 entries currently only checks Array.isArray((entry as any).context) but doesn't ensure each element is a string; update the json.map validation that creates contexts to assert that (entry as any).context is an array of strings (e.g., use Array.isArray and .every(elem => typeof elem === "string")) and throw the same Error(`Oracle response[${i}] is not a valid SignedContextV1`) if any element is non-string so the SignedContextV1 cast is only applied after the full shape check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/oracle/index.ts`:
- Around line 32-43: OracleOrderRequest uses bare string for Ethereum-typed
fields which causes type errors when passed to encodeAbiParameters with
oracleBatchAbiParams; update the OracleOrderRequest interface so owner,
interpreter, store, and token are typed as viem Address (e.g. `0x${string}`),
and bytecode, vaultId, nonce (and any bytes/bytes32 fields) are typed as viem
Hex (e.g. `0x${string}`), or alternatively perform explicit casts to
`\`0x${string}\`` for those specific properties at the call site before calling
encodeAbiParameters; apply this change to the nested order fields referenced by
encodeAbiParameters and keep the already-casted counterparty treatment
consistent.
---
Duplicate comments:
In `@src/oracle/index.ts`:
- Around line 150-162: The validation for SignedContextV1 entries currently only
checks Array.isArray((entry as any).context) but doesn't ensure each element is
a string; update the json.map validation that creates contexts to assert that
(entry as any).context is an array of strings (e.g., use Array.isArray and
.every(elem => typeof elem === "string")) and throw the same Error(`Oracle
response[${i}] is not a valid SignedContextV1`) if any element is non-string so
the SignedContextV1 cast is only applied after the full shape check.
added 4 commits
February 23, 2026 21:13
Follow codebase conventions: - Oracle health map lives on SharedState.oracleHealth - fetchOracleContext is a standalone fn with this: SharedState, called via .call(state) like processOrder/findBestTrade - Health helpers (isInCooloff, recordOracleSuccess/Failure) are plain exported functions operating on the health map - No new classes, no module-level singletons - quoteSingleOrder receives SharedState to thread through
- fetchSignedContext returns Result<SignedContextV1[], string> - fetchOracleContext returns Result<void, string> - Callers check .isErr() instead of try/catch - Follows codebase convention for error handling
…terface - OracleOrderRequest.order uses Order.V3 | Order.V4 from order/types - OracleOrderRequest.counterparty typed as 0x - Drop custom SignedContextV1 interface — signed context is already typed as any[] on TakeOrderV3/V4, and the response validation ensures the right shape at runtime - fetchSignedContext returns Result<any[], string> matching the existing signedContext field type
- extractOracleUrl: implement CBOR meta parsing (was a stub returning null) - Add oracleUrl field to PairBase, thread through V3/V4 fromArgs - Add meta field to SgOrder type (already in subgraph query) - fetchOracleContext: use pair.oracleUrl instead of dead (pair as any).meta - Switch from batch to single request encoding to match oracle server spec - Restrict oracle requests to V4 orders only - Strip internal type discriminant before ABI encoding
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/oracle/index.ts (1)
209-215:⚠️ Potential issue | 🟡 MinorResponse validator should enforce
contextelement types.Current validation accepts any array for
context. Non-string elements can pass and fail later in ABI/use sites.✅ Suggested tightening
if ( typeof json !== "object" || json === null || typeof (json as any).signer !== "string" || !Array.isArray((json as any).context) || + !(json as any).context.every((v: unknown) => typeof v === "string") || typeof (json as any).signature !== "string" ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/oracle/index.ts` around lines 209 - 215, The JSON validation currently only checks that (json as any).context is an array but not that its elements are strings; update the validation in the conditional around json/context in src/oracle/index.ts to also require every element be a string (e.g., replace the Array.isArray check with a combined check that context is an array and (json as any).context.every((c) => typeof c === "string")). Ensure the new check is included alongside the existing signer and signature checks so non-string context elements fail validation early.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/oracle/index.ts`:
- Around line 73-92: The cooloff expiry logic only clears cooloffUntil but
leaves consecutiveFailures at or above the threshold, causing immediate re-entry
to cooloff; update isInCooloff (or the cooloff-expiry path) to reset
state.consecutiveFailures = 0 when clearing the cooloff (i.e., when Date.now()
>= state.cooloffUntil set both state.cooloffUntil = 0 and
state.consecutiveFailures = 0) so that the failure streak is reset after a
cooloff expires; reference functions: isInCooloff, recordOracleFailure,
recordOracleSuccess, and the OracleHealthMap state object.
- Around line 171-179: The ABI encoding and conversions currently happen outside
the try/catch and can throw (encodeAbiParameters, BigInt, hexToBytes), so move
the block that builds orderStruct, calls
encodeAbiParameters(oracleSingleAbiParams, [...]), converts input/output indices
with BigInt, and hexToBytes into the existing try block (or wrap them in a new
try) inside the function so any thrown errors are caught; on catch return the
appropriate Err result consistent with the function's Result-based contract and
avoid letting exceptions bubble into quoting. Ensure you reference the same
symbols: request.order (strip type), oracleSingleAbiParams, encodeAbiParameters,
request.inputIOIndex/request.outputIOIndex, hexToBytes, and the body variable
when relocating the logic.
- Around line 163-191: The fetch call uses a caller-controlled url which allows
SSRF; before making the request in the function containing
isInCooloff/encodeAbiParameters/oracleSingleAbiParams/ORACLE_TIMEOUT_MS,
validate and sanitize `url`: reject non-HTTPS schemes, and either enforce a
configured allowlist of hostnames or at minimum parse the URL and block IP
addresses in private, loopback, link-local ranges and unresolved hostnames that
resolve to private ranges; throw or return a Result.err for disallowed URLs and
log the rejection so the fetch block never runs for untrusted addresses.
In `@src/order/quote.ts`:
- Around line 41-46: The oracle fetch in quoteSingleOrderV3 is a no-op for V3
orders and should be removed to avoid unnecessary async overhead: remove the
conditional block that calls fetchOracleContext.call(state, orderDetails) (the
if (state) { ... } block referencing fetchOracleContext in function
quoteSingleOrderV3) so the function no longer awaits a redundant oracle call;
alternatively, if you want to keep it for future-proofing, replace the block
with a short explanatory comment inside quoteSingleOrderV3 referencing
fetchOracleContext and that src/oracle/fetch.ts intentionally skips non-V4
orders to document the no-op behavior.
In `@src/order/types/v3.ts`:
- Line 125: The oracleUrl extraction for V3 orders is unnecessary since
fetchOracleContext returns early for non-V4 orders; update the V3 order
construction to set oracleUrl to null unconditionally (replace the conditional
extractOracleUrl(orderDetails.meta) usage) so you avoid the parsing
overhead—refer to the oracleUrl field in your V3 order creation and the
extractOracleUrl and fetchOracleContext functions when making the change.
---
Duplicate comments:
In `@src/oracle/index.ts`:
- Around line 209-215: The JSON validation currently only checks that (json as
any).context is an array but not that its elements are strings; update the
validation in the conditional around json/context in src/oracle/index.ts to also
require every element be a string (e.g., replace the Array.isArray check with a
combined check that context is an array and (json as any).context.every((c) =>
typeof c === "string")). Ensure the new check is included alongside the existing
signer and signature checks so non-string context elements fail validation
early.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fb3f30fa-4727-4e37-880f-448ea4784a85
📒 Files selected for processing (9)
src/oracle/fetch.tssrc/oracle/index.tssrc/order/index.tssrc/order/quote.tssrc/order/types/index.tssrc/order/types/v3.tssrc/order/types/v4.tssrc/state/index.tssrc/subgraph/types.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (1)
src/order/quote.ts (1)
39-42: 🧹 Nitpick | 🔵 TrivialOracle fetch in V3 is a no-op but adds async overhead.
The
fetchOracleContextcall will always return early for V3 orders (persrc/oracle/index.tsline 23 which checksorder.type !== Order.Type.V4). Consider removing this block from the V3 quote function to avoid unnecessary async overhead.♻️ Proposed removal
export async function quoteSingleOrderV3( orderDetails: Pair, state: SharedState, blockNumber?: bigint, gas?: bigint, ) { - const oracleResult = await fetchOracleContext.call(state, orderDetails); - if (oracleResult.isErr()) { - throw oracleResult.error; - } - const { data } = await state.client🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/order/quote.ts` around lines 39 - 42, The V3 quote function is calling fetchOracleContext (via fetchOracleContext.call(state, orderDetails)) which is a no-op for V3 and adds unnecessary async overhead; remove the await call and the subsequent oracleResult.isErr() error throw (the const oracleResult and the if block) from the V3 quote code path, and ensure no remaining references to oracleResult or its error handling remain in the function (preserve any upstream logic that relied on successful oracle data by using existing V3-safe defaults). Target symbols: fetchOracleContext, oracleResult, orderDetails, state, and the V3 quote function name to locate and delete that block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/oracle/error.ts`:
- Around line 19-25: The JSDoc example uses the enum type instead of an enum
member; update both examples to pass a specific OracleErrorType member (e.g.,
OracleErrorType.FetchError) to the OracleError constructor so they type-check.
In the doc block for OracleError, replace occurrences of new OracleError("msg",
OracleErrorType) and new OracleError("msg", OracleErrorType, originalError) with
new OracleError("msg", OracleErrorType.FetchError) and new OracleError("msg",
OracleErrorType.FetchError, originalError) respectively, referencing the
OracleError constructor and the OracleErrorType enum member.
In `@src/oracle/fetch.ts`:
- Around line 144-153: The isInCooloff function resets only state.cooloffUntil
when the cooloff expires, leaving state.consecutiveFailures unchanged which
causes immediate re-entry into cooloff; update isInCooloff (operating on
OracleHealthMap / the state object) so that when Date.now() >=
state.cooloffUntil you set state.cooloffUntil = 0 and also reset
state.consecutiveFailures = 0 (or to the desired fresh-start value) before
returning false.
- Around line 15-16: The file-level docstring in src/oracle/fetch.ts incorrectly
says "SignedContextV1" while the function(s) in this module return
SignedContextV2; update the comment to mention "SignedContextV2" to match the
actual return type and function signature (replace the "SignedContextV1" text in
the POST description). Also scan this module for any other references to
SignedContextV1 and change them to SignedContextV2 so the docstring and code are
consistent.
- Around line 174-183: The validation in isValidSignedContextV2 currently only
checks that context is an array but not that its elements are strings; update
isValidSignedContextV2 to additionally verify that (value as
any).context.every((c) => typeof c === "string") so the function returns false
if any context element is not a string, keeping the other signer and signature
string checks intact; reference the isValidSignedContextV2 function and the
SignedContextV2 type when making this change.
- Around line 25-34: The fetchSignedContext function currently relies solely on
OracleConstants.isKnown (which is buggy) and must perform strict URL validation
before any network call: in fetchSignedContext validate the input URL using the
URL constructor, reject anything that is not https (scheme must be "https"),
resolve/validate the hostname to ensure it is not a loopback or private IP range
(e.g., 127.0.0.0/8, 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16) and explicitly
block cloud metadata endpoints such as 169.254.169.254; if validation fails
return Result.err(new OracleError(..., OracleErrorType.Cooloff)). Keep
OracleConstants.isKnown as an additional check but do not rely on it as the sole
protection; perform validation early in fetchSignedContext (before any
DNS/fetch) and reuse OracleError/OracleErrorType for consistent error handling.
- Around line 42-51: The ABI encoding and conversions (encodeAbiParameters,
BigInt(request.inputIOIndex), BigInt(request.outputIOIndex), hexToBytes) must be
moved inside the existing try block so any exceptions are caught and returned as
a Result; specifically, remove the current pre-try lines that destructure
request.order into { type: _type, ...orderStruct } and call
encodeAbiParameters(OracleSingleAbiParams, [orderStruct, BigInt(...),
BigInt(...), request.counterparty]) and hexToBytes(encoded), and instead perform
that destructuring and all three operations inside the try block that handles
the request so thrown errors are caught and mapped to the function’s Result
flow.
In `@src/oracle/index.test.ts`:
- Line 76: Rename the test title string(s) that contain the typo "currectly" to
"correctly" (e.g., update the it(...) description "returns currectly call
fetchSignedContext when Order V4 when it returns ok" to "returns correctly call
fetchSignedContext when Order V4 when it returns ok"); search for other tests in
the file with the same misspelling and update them as well to keep naming
consistent (look for the exact phrase "currectly" in the repository and replace
with "correctly").
- Line 56: Rename the test description string in the failing test case so
"currectly" is corrected to "correctly"; locate the Jest test defined with
it("returns currectly call fetchSignedContext when Order V4 when it returns
error", ...) in src/oracle/index.test.ts and update the string to "returns
correctly call fetchSignedContext when Order V4 when it returns error".
In `@src/oracle/types.ts`:
- Around line 18-20: The isKnown function incorrectly calls KnownUrls.some with
the url string as the callback, causing a TypeError; change the check to
actually test membership (e.g., use KnownUrls.includes(url) or KnownUrls.some(u
=> u === url)) and keep the function signature isKnown(url: string): boolean so
callers like src/oracle/fetch.ts continue to work; ensure KnownUrls typing
matches the element type so the membership test compiles cleanly.
In `@src/order/index.ts`:
- Line 461: quoteSingleOrder (and its V3/V4 variants) should not let
fetchOracleContext failures abort the whole quote path: inside
src/order/quote.ts, wrap the fetchOracleContext call in a try/catch, catch
timeouts/5xx and other errors from fetchOracleContext, log the error, clear any
previously injected signedContext on the request/order object (e.g., remove or
set orderDetails.signedContext to undefined/null), and continue execution using
an empty context instead of rethrowing; ensure the rest of
quoteSingleOrder/quoteSingleOrderV3/quoteSingleOrderV4 code treats the missing
context as a valid fallback.
In `@src/order/quote.test.ts`:
- Around line 22-26: The SharedState stub used in quote tests is missing the
oracleHealth property so tests exercising the oracle-enabled quote path (where
orderDetails.oracleUrl is present) will fail; update the test fixture used in
quote.test.ts (the state object cast as SharedState) to include oracleHealth:
new Map() alongside client, and refactor the repeated stub creation (occurring
at the other occurrences around lines 76-80 and 114-118) into a shared helper or
factory function (e.g., makeTestState or sharedStateFixture) so all oracle-url
test cases exercise the real path instead of relying on a partial mock.
---
Duplicate comments:
In `@src/order/quote.ts`:
- Around line 39-42: The V3 quote function is calling fetchOracleContext (via
fetchOracleContext.call(state, orderDetails)) which is a no-op for V3 and adds
unnecessary async overhead; remove the await call and the subsequent
oracleResult.isErr() error throw (the const oracleResult and the if block) from
the V3 quote code path, and ensure no remaining references to oracleResult or
its error handling remain in the function (preserve any upstream logic that
relied on successful oracle data by using existing V3-safe defaults). Target
symbols: fetchOracleContext, oracleResult, orderDetails, state, and the V3 quote
function name to locate and delete that block.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 903b3822-8d46-4f59-9fbd-ddefffd0730e
📒 Files selected for processing (15)
src/common/abis/orderbook.tssrc/oracle/error.tssrc/oracle/fetch.test.tssrc/oracle/fetch.tssrc/oracle/index.test.tssrc/oracle/index.tssrc/oracle/types.tssrc/order/index.test.tssrc/order/index.tssrc/order/quote.test.tssrc/order/quote.tssrc/order/types/index.tssrc/order/types/v3.tssrc/order/types/v4.tssrc/state/index.ts
Comment on lines
+25
to
+34
| export async function fetchSignedContext( | ||
| url: string, | ||
| request: OracleOrderRequest, | ||
| healthMap: OracleHealthMap, | ||
| ): Promise<Result<SignedContextV2, OracleError>> { | ||
| if (!OracleConstants.isKnown(url)) { | ||
| return Result.err( | ||
| new OracleError(`Oracle ${url} is unknown, skipping`, OracleErrorType.Cooloff), | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
Security: No URL validation before fetch — potential SSRF risk.
The function calls OracleConstants.isKnown(url) as a gate, but isKnown is buggy (always throws). Even if fixed, a compromised order metadata could inject internal network URLs (localhost, private IPs, cloud metadata endpoints).
Consider adding URL validation to block:
- Non-HTTPS schemes
- Private/loopback IP ranges
- Cloud metadata endpoints (169.254.169.254)
🔒 Proposed hardening
export async function fetchSignedContext(
url: string,
request: OracleOrderRequest,
healthMap: OracleHealthMap,
): Promise<Result<SignedContextV2, OracleError>> {
+ // Validate URL scheme and host
+ let parsed: URL;
+ try {
+ parsed = new URL(url);
+ } catch {
+ return Result.err(new OracleError("Invalid oracle URL", OracleErrorType.Cooloff));
+ }
+ if (parsed.protocol !== "https:") {
+ return Result.err(new OracleError("Oracle URL must use HTTPS", OracleErrorType.Cooloff));
+ }
+
if (!OracleConstants.isKnown(url)) {📝 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
| export async function fetchSignedContext( | |
| url: string, | |
| request: OracleOrderRequest, | |
| healthMap: OracleHealthMap, | |
| ): Promise<Result<SignedContextV2, OracleError>> { | |
| if (!OracleConstants.isKnown(url)) { | |
| return Result.err( | |
| new OracleError(`Oracle ${url} is unknown, skipping`, OracleErrorType.Cooloff), | |
| ); | |
| } | |
| export async function fetchSignedContext( | |
| url: string, | |
| request: OracleOrderRequest, | |
| healthMap: OracleHealthMap, | |
| ): Promise<Result<SignedContextV2, OracleError>> { | |
| // Validate URL scheme and host | |
| let parsed: URL; | |
| try { | |
| parsed = new URL(url); | |
| } catch { | |
| return Result.err(new OracleError("Invalid oracle URL", OracleErrorType.Cooloff)); | |
| } | |
| if (parsed.protocol !== "https:") { | |
| return Result.err(new OracleError("Oracle URL must use HTTPS", OracleErrorType.Cooloff)); | |
| } | |
| if (!OracleConstants.isKnown(url)) { | |
| return Result.err( | |
| new OracleError(`Oracle ${url} is unknown, skipping`, OracleErrorType.Cooloff), | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/oracle/fetch.ts` around lines 25 - 34, The fetchSignedContext function
currently relies solely on OracleConstants.isKnown (which is buggy) and must
perform strict URL validation before any network call: in fetchSignedContext
validate the input URL using the URL constructor, reject anything that is not
https (scheme must be "https"), resolve/validate the hostname to ensure it is
not a loopback or private IP range (e.g., 127.0.0.0/8, 10.0.0.0/8,
172.16.0.0/12, 192.168.0.0/16) and explicitly block cloud metadata endpoints
such as 169.254.169.254; if validation fails return Result.err(new
OracleError(..., OracleErrorType.Cooloff)). Keep OracleConstants.isKnown as an
additional check but do not rely on it as the sole protection; perform
validation early in fetchSignedContext (before any DNS/fetch) and reuse
OracleError/OracleErrorType for consistent error handling.
Collaborator
|
moved to #438 |
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
2 participants
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.
Adds support for fetching signed context from oracle servers for orders that specify an oracle-url in their metadata. Changes: adds meta field to subgraph queries, new oracle module for fetching signed contexts (batch format per spec), wires oracle fetch into quoting pipeline. Note: extractOracleUrl is a placeholder pending SDK update.
Summary by CodeRabbit