fix: update oif docs with the latest oif specs #35
Conversation
❌ Deploy Preview for oif-docs-wip failed.
|
📝 WalkthroughWalkthroughOrder API docs moved to an order-centric EIP-712 flow: submissions now send a full EIP-712 Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Signer as Signer
participant API as API
participant Engine as MatchingEngine
participant Webhook as WebhookListener
Note over Client,Signer: Derive EIP-712 domain/types/message from quote/order
Client->>Signer: signTypedData(domain, types, message)
Signer-->>Client: signature (hex)
Client->>API: POST /v1/orders { order, signature, quoteId?, originSubmission }
API-->>API: Validate signature & order schema
alt valid
API->>Engine: submitOrder(order)
Engine-->>API: accepted (async)
API-->>Client: 202 Received { id, status: received }
Engine->>API: updateStatus(executing)
API->>Webhook: emit rgba(46, 204, 113, 0.5) order.executed (includes fillTransaction)
Engine->>API: updateStatus(settling)
Engine->>API: updateStatus(settled)
Engine->>API: updateStatus(finalized)
API->>Webhook: emit rgba(52, 152, 219, 0.5) order.finalized (includes settlement)
API-->>Client: GET /v1/orders/:id -> { status, settlement, fillTransaction, timestamps }
else invalid
API-->>Client: 4xx/`rejected` or 5xx/`error`
API->>Webhook: emit rgba(231, 76, 60, 0.5) order.failed (if applicable)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
content/docs/apis/order-api.mdx (1)
380-384: Consider addingorder.refundedto webhook events.The webhook events list includes
order.executed,order.finalized, andorder.failed, butorder.refundedis missing. Sincerefundedis a terminal state in the order flow (line 171), consider adding it for completeness:- "events": ["order.executed", "order.finalized", "order.failed"] + "events": ["order.executed", "order.finalized", "order.failed", "order.refunded"]If
refundedis intentionally excluded from webhooks, a brief note explaining why would help.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
content/docs/apis/order-api.mdx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Redirect rules - oif-docs-wip
- GitHub Check: Header rules - oif-docs-wip
- GitHub Check: Pages changed - oif-docs-wip
🔇 Additional comments (8)
content/docs/apis/order-api.mdx (8)
28-34: Request format structure looks good.The new request format with
order,signature,quoteId, andoriginSubmissionfields is well-documented with clear types and optionality markers.
61-75: Success response and status taxonomy are clearly documented.The response format with
orderId,status, andmessagefields is well-structured, and the status values (received,rejected,error) are clearly explained.
119-142: GET response format is consistent with the updated spec.The response correctly uses
id(notorderId), includes the new field names (inputAmounts,outputAmounts,settlement,fillTransaction), and demonstrates the EIP-7930 asset address format.
156-173: Order status flow diagram is well-structured.The state diagram clearly shows the happy path (
created→finalized) and error paths (failed,refunded). The transitions are logical and consistent with the status table below.
175-188: Order statuses table provides clear guidance.The status descriptions and "Next Steps" column are actionable and help developers understand what to expect at each stage of the order lifecycle.
253-282: EIP-712 signing flow is well-documented.The updated signing example correctly shows extracting
domain,types, andmessagefromquote.order.payloadand usingsignTypedData. The submission example properly constructs the request withorder,signature, andquoteId.
310-336: Best practices section provides practical guidance.The examples correctly use
validUntil(as per the spec rename fromexpiresAt), demonstrate proper EIP-712 signing, and show appropriate handling of submission responses.
191-212: Field naming inconsistency exists in the API specification: confirm iforderId(POST) andid(GET) should be standardized.The inconsistency is confirmed in
openapi/oif.json:PostOrderResponseusesorderIdwhileGetOrderResponseusesid. The documentation correctly reflects these spec definitions, but the underlying OpenAPI schema has this naming difference which will likely confuse developers. This should be standardized—either both endpoints should useidor both should useorderId—in the API specification itself.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
content/docs/apis/order-api.mdx (2)
61-75: Minor inconsistency between example and response field table.The example response (lines 64-68) always includes
orderId, but the response field table (line 195) marks it asstring?(optional). Additionally, the table lists anorderfield (line 198) that isn't shown in the example.Consider either:
- Adding a note that
orderIdandorderare only present whenstatusis"received"- Or updating the example to show the full response structure
376-385: Consider addingorder.refundedto webhook events.The webhook events list includes
order.failedbut notorder.refunded, even thoughrefundedis also a terminal state in the order lifecycle diagram (line 168-171).🔎 Suggested addition
- "events": ["order.executed", "order.finalized", "order.failed"] + "events": ["order.executed", "order.finalized", "order.failed", "order.refunded"]
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
content/docs/apis/order-api.mdx
🔇 Additional comments (9)
content/docs/apis/order-api.mdx (9)
24-38: LGTM!The request format is clear and well-documented with appropriate TypeScript type annotations.
42-59: LGTM!The example request correctly demonstrates the new order submission format with EIP-712 typed data structure.
156-173: LGTM!The Mermaid state diagram accurately represents the order lifecycle and error paths, consistent with the status table below.
253-265: LGTM!The EIP-712 signing example correctly demonstrates extracting typed data from the quote and using
signTypedData.
267-282: LGTM!The order submission example correctly uses the signed quote data and follows the documented request format.
338-360: LGTM!The status polling implementation correctly checks for the
finalizedsuccess state and handlesfailed/refundederror states per the new lifecycle.
308-315: LGTM!The quote validation correctly uses
validUntilper the renamed field in the spec.
221-226: LGTM!The Settlement type documentation is clear and consistent with the example.
214-219: Fix theamountfield type fromstring?toAmount.The
amountfield is correctly marked as optional in the spec (amount?: Amount), but the documentation incorrectly shows the type asstring?instead ofAmount. Update the type to match the oif-specs definition.Likely an incorrect or invalid review comment.
|
thanks @th0rOdinson and @pepebndc - is this good to merge? |
nahimterrazas
left a comment
nahimterrazas
left a comment
There was a problem hiding this comment.
Thanks for doing this! we appreciate your contributions.
LGTM, just minor changes :)
| | `settling` | Settlement in progress | Monitor settlement | | ||
| | `settled` | Assets available for claiming | Wait for finalization | | ||
| | `finalized` | Order complete, all claims confirmed | No action needed | | ||
| | `failed` | Execution failed | Check error, may retry | |
There was a problem hiding this comment.
Include postFilled and preClaimed
Remove aggregator-only refunded
There was a problem hiding this comment.
I followed the oif-specs while doing this. Will those be updated to add postFilled/preClaimed and remove refunded? If so, I'll implement your comment right away.
There was a problem hiding this comment.
actually Refunded makes sense to be implemented, but is not yet on OIF-solver, just add postFilled/preClaimed and keep refunded
content/docs/apis/order-api.mdx
Outdated
| quoteId?: string; // Optional quote ID for tracking | ||
| originSubmission?: { // Optional submission preference | ||
| mode: "user" | "protocol"; | ||
| schemes?: Array<"erc-4337" | "permit2" | "erc20-permit" | "eip-3009">; |
There was a problem hiding this comment.
eip3009 instead of eip-3009
There was a problem hiding this comment.
Same with eip-3009 → eip3009. The specs currently has it with hyphen. Happy to update if this will be reflected in the spec too.
There was a problem hiding this comment.
yes, you are right, currently on the solver we are using eip3009, so we should change the spec to align with this. Thanks @th0rOdinson
There was a problem hiding this comment.
we can keep eip3009 and modify the specs
There was a problem hiding this comment.
updated 8db2563
as a personal opinion, it seems odd to me to have eip3009 and erc-4337 (one with - and the other without it)
| [*] --> pending: Order submitted | ||
| pending --> filling: Solver starts filling | ||
| filling --> filled: Outputs delivered | ||
| filled --> claiming: Solver claims inputs | ||
| claiming --> completed: Inputs transferred | ||
|
|
||
| pending --> expired: Time limit reached | ||
| filling --> failed: Fill transaction failed | ||
| filled --> failed: Claim transaction failed | ||
|
|
||
| [*] --> created: Order submitted | ||
| created --> pending: Validation passed | ||
| pending --> executing: Execution started | ||
| executing --> executed: Transactions submitted | ||
| executed --> settling: Settlement started | ||
| settling --> settled: Assets available | ||
| settled --> finalized: Claims confirmed | ||
|
|
||
| pending --> failed: Execution failed | ||
| executing --> failed: Transaction failed | ||
| settled --> refunded: Refund processed | ||
|
|
||
| failed --> [*] | ||
| expired --> [*] | ||
| completed --> [*] | ||
| refunded --> [*] | ||
| finalized --> [*] |
There was a problem hiding this comment.
created → pending → executing → executed → (postFilled?) → settled → (preClaimed?) → finalized
failed can occur from any in‑flight state and is terminal.
There was a problem hiding this comment.
Same concern as above! Let me know the status of the specs to continue with this.
There was a problem hiding this comment.
We keep refunded, and add the others states :) thanks
❌ Deploy Preview for oif-docs-wip failed.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@content/docs/apis/order-api.mdx`:
- Around line 204-205: The GET endpoint placeholder is inconsistent: the
Response heading uses "GET /v1/orders/:id" while the Request Format block uses
":orderId"; update the document so both use the same placeholder throughout
(choose either ":id" or ":orderId"), e.g., change the Request Format block's
":orderId" to ":id" (or vice versa) and ensure any other occurrences of the GET
path in this file match the chosen placeholder (search for "GET /v1/orders" and
":orderId" to locate all instances).
4 participants
Summary
Sources
All changes based on oif-specs types.ts:
https://github.com/openintentsframework/oif-specs/blob/main/schemas/typescript/types.ts
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.