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.
- 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
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
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
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
hardyjosh
left a comment
hardyjosh
left a comment
There was a problem hiding this comment.
Review: Oracle Signed Context Support
Good work overall — well-structured module separation, strong test coverage, and correct error handling patterns. A few things to address before merge:
Bug: Empty array response passes validation
SignedContextV2.isValidList() uses .some() which returns false for empty arrays:
static isValidList(value: unknown): value is SignedContextV2[] {
if (!Array.isArray(value)) return false;
return !value.some((v) => !SignedContextV2.isValid(v)); // empty array -> true
}An empty [] response passes validation, then response.data[0] is undefined, which gets set as the signed context. This will cause downstream failures when the solver tries to submit a tx with signedContext = [undefined].
Fix: add value.length === 0 check, or use .every() with a length guard.
Hardcoded KnownUrls is fragile
export const KnownUrls = ["https://st0x-oracle-server.fly.dev/context"] as const;Any new oracle deployment (e.g. /context/v1, a different domain, or a third-party oracle) requires a code change and redeployment. Consider making this configurable via the YAML config, or removing the check entirely -- the URL comes from on-chain order meta which the order owner controls. If someone puts a bad URL in their own order's meta, they only hurt their own order.
SignedContextV1 -> SignedContextV2 rename
The ABI rename from SignedContextV1 to SignedContextV2 across orderbook.ts is cosmetic -- the struct shape (address signer, bytes32[] context, bytes signature) is identical. Creates unnecessary diff noise in a PR that's already 1700+ lines. Consider splitting it out or reverting if the on-chain struct name hasn't actually changed.
Order type discriminant passed to ABI encoding
order: request.order, // includes { type: Order.Type.V4, ...}The TypeScript discriminant type: Order.Type.V4 is passed through to encodeAbiParameters. Viem silently ignores extra fields not in the ABI spec, so this works, but it's imprecise. Stripping it explicitly would prevent surprises if viem ever becomes strict about extra fields.
Minor: typos
- "orcale" -> "oracle" in several JSDoc comments (fetch.ts)
- "sucess" -> "success"
What's good
- Oracle failure correctly blocks quoting (prevents gas waste on guaranteed-revert txs)
- Typed
OracleErrorwith categorization is clean - Health/cooloff tracking is well-implemented
- Test coverage is thorough (~1,290 lines covering edge cases, timeouts, cooloff, validation)
fetchOracleContextbound toSharedStatevia.call()is a clean pattern- Oracle URL extraction from CBOR meta with magic number search works correctly
2 participants
Motivation
Solution
Checks
By submitting this for review, I'm confirming I've done the following: