Honor passive (post-only) order intent on the typed CreateOrder path - #86
Conversation
|
Client half: usherlabs/fiet-maker#941. This PR must deploy first — a client requesting |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughPassive-only intent is added to order payload validation. Limit orders force ChangesPassive order support
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant CreateOrder
participant CCXT
participant gRPC
Client->>CreateOrder: Submit passive_only limit order
CreateOrder->>CreateOrder: Validate order intent and type
CreateOrder->>CCXT: createOrder with postOnly true
CCXT-->>CreateOrder: Accepted order or exchange error
CreateOrder->>gRPC: Return accepted_passive or classified error
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/handlers/execute-action/orders.ts`:
- Around line 201-207: The passive-order classification in the surrounding
execution catch must apply only to failures thrown by broker.createOrder. Narrow
the try/catch or error-routing logic around broker.createOrder so resolution,
metadata capture, telemetry, and archive failures retain generic handling, while
actual venue submission failures still use classifyPassiveOrderError and
rejectWithGrpcError; add a regression test covering a non-submission failure and
preserving the already-placed-order behavior.
🪄 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: CHILL
Plan: Pro
Run ID: 3cf8be54-e1d1-4c92-806a-120ea7329d48
📒 Files selected for processing (6)
src/handlers/execute-action/orders.tssrc/helpers/grpc/status.tssrc/helpers/passive-order.tssrc/schemas/action-payloads.tstest/grpc-status.test.tstest/passive-order.test.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-04-14T06:47:01.283Z
Learnt from: csmithington
Repo: usherlabs/cex-broker PR: 38
File: src/client.dev.ts:0-0
Timestamp: 2026-04-14T06:47:01.283Z
Learning: In this codebase, gRPC action constants such as `FetchTicker`, `FetchFees`, and `FetchAccountId` should be sourced from `src/helpers/constants.ts` and imported from there (e.g., used by both `src/client.dev.ts` and `src/server.ts`). Do not import or reference generated proto TypeScript artifacts (for example `./proto/cex_broker/Action`), since those generated files are git-ignored and won’t be available/committed consistently.
Applied to files:
src/helpers/passive-order.tssrc/helpers/grpc/status.tssrc/schemas/action-payloads.tssrc/handlers/execute-action/orders.ts
🔇 Additional comments (6)
src/schemas/action-payloads.ts (1)
78-80: LGTM!src/helpers/passive-order.ts (1)
1-55: LGTM!src/handlers/execute-action/orders.ts (1)
13-13: LGTM!Also applies to: 51-172
src/helpers/grpc/status.ts (1)
21-29: LGTM!test/passive-order.test.ts (1)
1-219: LGTM!test/grpc-status.test.ts (1)
24-32: LGTM!
|
Addressed in The finding was valid: the Classification is now gated on a submission state that is Full suite: 476 pass, 0 fail. Typecheck and lint clean. Not changed, and worth naming: a network timeout on the submission itself is still classified |
Problem
The typed
CreateOrderpath silently drops post-only intent. Clients already sendorderIntent: "passive_only", butCreateOrderPayloadSchemahas no such member and thezod object is not strict, so the key is stripped and the order reaches the venue as a plain
LIMIT GTC. A "passive" order can therefore cross the book and execute as taker.This is not theoretical. Over 14 days of live market-making orders, 17% of filled base
volume on one pair and 38% on another executed as
takerdespite being authored aspost-only maker rungs. The cost is execution price — crossing the spread against our own
stale quote — not fees.
Change
Implements the broker half of the passive-order contract that the clients already speak:
CreateOrderPayloadSchemaaccepts an optionalorderIntent: "passive_only". Unknownvalues now fail validation instead of being silently ignored — the silent strip was the
bug.
postOnly: truein the ccxtparams without clobbering caller-supplied params, and returns
passivePlacementOutcome: "accepted_passive".passive_order_would_cross,passive_order_unsupported, andpassive_order_rejected, classified in one place(
src/helpers/passive-order.ts) and given stable gRPC statuses.Classification is deliberately conservative: it identifies would-cross only from ccxt's
OrderImmediatelyFillableor explicit immediate-execution wording, and unsupported onlyfrom explicit signals. Anything ambiguous becomes
passive_order_rejected— never success.When
orderIntentis absent the request and response are unchanged; each addition sitsbehind a conditional spread, and a test asserts the ordinary path byte-for-byte.
passiveOnlycapability advertisement was skipped: this service has no generalvenue-capability surface to expose it on.
Deploy ordering
This must reach production before the corresponding client change. A client that
requests
passive_onlyagainst a broker that omitspassivePlacementOutcomefails everysuch order closed. Broker first, then client — never together.
Verification
New tests cover the unchanged ordinary path, param preservation, a conflicting caller
postOnly, passive market-order rejection, unknown-intent schema rejection, and each ofthe three venue failure classifications.
Summary by CodeRabbit
New Features
Bug Fixes
Tests