Skip to content

feat: Binance Futures adapter + futures order types + testnet/mainnet network#95

Merged
fray-cloud merged 1 commit intofeat/binance-only-cleanupfrom
feat/binance-futures-adapter
May 2, 2026
Merged

feat: Binance Futures adapter + futures order types + testnet/mainnet network#95
fray-cloud merged 1 commit intofeat/binance-only-cleanupfrom
feat/binance-futures-adapter

Conversation

@fray-cloud
Copy link
Copy Markdown
Owner

@fray-cloud fray-cloud commented Apr 30, 2026

Summary

PR2 of 3 in the LLM-driven Binance Futures pivot. Builds the futures execution foundation that PR3 will drive from a Claude-generated trade signal.

Stacks on top of #94 (PR1). Re-target to `dev` once PR1 merges.

  • `OrderRequest`/`OrderResult`/`Order` reshaped: `side` is `'long' | 'short'` (user-facing position direction; adapter translates to BUY/SELL internally). Add leverage, marginType, takeProfitPrice, stopLossPrice, entryPrice, liquidationPrice, tpOrderId, slOrderId.
  • `ExchangeCredentials.network` (`'mainnet' | 'testnet'`) — adapter picks fapi.binance.com vs testnet.binancefuture.com per call.
  • `ExchangeKey` schema gains `network` column; unique constraint widened to `(userId, exchange, network)` so a user can hold one key per network.
  • `BinanceRest` rewritten against fapi (USDT-M perpetual). Reuses existing HMAC signing. Implements `setLeverage`, `setMarginType` (-4046 idempotent), `setPositionMode` (-4059 idempotent), `getPosition`, `closePosition` (reduceOnly market), `placeStopLoss` / `placeTakeProfit` (STOP_MARKET / TAKE_PROFIT_MARKET, closePosition=true, workingType=MARK_PRICE), `getSymbolFilter` (LOT_SIZE / PRICE_FILTER / MIN_NOTIONAL via cached `exchangeInfo`).
  • Worker saga: `ConfigureFuturesAccountStep` (one-way mode + margin type + leverage, all idempotent) runs before `PlaceOrderStep`. `AttachTpSlStep` runs after fill and inline-compensates by force-closing the position if either TP or SL placement fails — naked positions are the worst-case outcome. `UpdateDbStep` persists futures fields back to the Order row.
  • `CreateOrderDto` picks up leverage (1-20 clamp), marginType, takeProfitPrice, stopLossPrice. `mode` locked to `'real'` (paper retired in PR1; testnet replaces it via `ExchangeKey.network`).
  • `ClaudeToken` + `LlmDecisionLog` Prisma models added now (unused until PR3) so the next migration is purely additive — keeps PR3 small.
  • Dev-only `POST /debug/futures-test` endpoint: takes `exchangeKeyId + symbol/side/quantity/leverage/tp/sl`, calls the adapter directly (skips Kafka and the saga), returns entry result + tp/sl orderIds + the resulting Position. Gated by `NODE_ENV !== 'production'`.

Test plan

  • `pnpm build` green across all 9 workspace packages
  • `prisma migrate dev --name init` applied cleanly against fresh volume
  • `docker compose -f docker-compose.dev.yml up -d`: postgres/redis/kafka healthy, api-server `/api/health` 200, worker-service running, web `/markets` 200
  • Manual testnet smoke — register Binance Futures testnet key with `network: 'testnet'`, hit `POST /debug/futures-test` with small qty/leverage, verify position + TP/SL appear on `testnet.binancefuture.com` UI, then manually close position and confirm Order DB row matches
  • CI green
  • Test suites — known broken since PR1; will be repaired in PR3 alongside the new feature

Out of scope

  • Claude CLI integration + per-user OAuth token storage (PR3)
  • LLM trade form UI (PR3)
  • 7 safety guards + risk service (PR3)
  • Mainnet enablement gate (Phase 5, post PR3)

🤖 Generated with Claude Code

Summary by Sourcery

Introduce Binance USDT-M futures support with network-aware credentials, futures-specific order semantics, and a safer real-execution pipeline, plus scaffolding for upcoming LLM-driven trading features.

New Features:

  • Add Binance Futures REST adapter capabilities including leverage/margin configuration, TP/SL placement, position management, and symbol filter retrieval.
  • Support mainnet vs testnet selection via exchange credentials, API DTOs, and a new debug futures test endpoint for non-production environments.
  • Extend order creation API and lifecycle to handle futures-specific fields such as leverage, margin type, and TP/SL prices, and propagate associated child order IDs through to result events.

Enhancements:

  • Refine core trading types to express futures concepts (position sides, margin types, order status, positions, and symbol filters) and align Binance REST integration with the futures (fapi) API, including basic response caching.
  • Update the real execution saga to configure futures account settings idempotently, attach TP/SL orders after fills with defensive force-close behavior, and persist enriched futures state on orders.
  • Allow per-network exchange keys in the database and API, widening uniqueness constraints so users can register distinct mainnet and testnet credentials.
  • Add Prisma schema changes and models (ClaudeToken and LlmDecisionLog) to support upcoming LLM-driven trade flows without affecting current behavior.

Documentation:

  • Adjust Swagger metadata for order and exchange key DTOs to reflect futures symbols, position-side semantics, and new futures parameters like leverage, margin type, and TP/SL prices.

…, testnet/mainnet network

PR2 of 3 in the LLM-driven Binance Futures pivot. Lays the futures execution
foundation that PR3 will drive from a Claude-generated trade signal.

What's reshaped:
- OrderRequest/OrderResult/Order: side is now 'long'|'short' (the user-facing
  position direction; adapter translates internally to BUY/SELL). Add
  leverage, marginType, takeProfitPrice, stopLossPrice on request, plus
  entryPrice, liquidationPrice, tpOrderId, slOrderId on result. New
  OrderStatus, PositionSide, MarginType, Position, SymbolFilter exports.
- ExchangeCredentials gains optional `network: 'mainnet'|'testnet'` so the
  adapter can pick fapi.binance.com vs testnet.binancefuture.com per call.
- ExchangeKey schema: `network` column (default 'mainnet') and unique
  constraint widened to (userId, exchange, network) so a user can hold one
  key per network.
- Order schema: futures fields (leverage, marginType, positionSide,
  entryPrice, liquidationPrice, takeProfitPrice, stopLossPrice, tpOrderId,
  slOrderId, realizedPnl, closedAt). Init migration regenerated.

What's new:
- BinanceRest rewrite against fapi.binance.com (and testnet.binancefuture.com).
  Reuses the existing HMAC signing path. Implements setLeverage,
  setMarginType (-4046 idempotent), setPositionMode (-4059 idempotent),
  getPosition, closePosition (reduceOnly market), placeStopLoss/placeTakeProfit
  (STOP_MARKET / TAKE_PROFIT_MARKET with closePosition=true and
  workingType=MARK_PRICE), getSymbolFilter (LOT_SIZE / PRICE_FILTER /
  MIN_NOTIONAL via cached exchangeInfo).
- Worker saga: ConfigureFuturesAccountStep (one-way mode + margin type +
  leverage, all idempotent) runs before PlaceOrderStep; AttachTpSlStep runs
  after fill and inline-compensates by force-closing the position if either
  TP or SL placement fails — a naked position is the worst-case outcome.
  UpdateDbStep persists futures fields back to the Order row.
- Api-server CreateOrderDto picks up leverage (1-20 clamped), marginType,
  takeProfitPrice, stopLossPrice. mode is locked to 'real' (paper retired
  in PR1 — testnet replaces it via ExchangeKey.network).
- ClaudeToken and LlmDecisionLog Prisma models added now (unused until PR3)
  so the next migration is purely additive — keeps PR3 small.
- Dev-only POST /debug/futures-test endpoint: takes an exchangeKeyId +
  symbol/side/quantity/leverage/tp/sl, calls the adapter directly (skips
  Kafka and the full saga), returns entry result + tp/sl orderIds + the
  resulting Position. Gated by NODE_ENV !== 'production'.

Verified:
- pnpm build green across all 9 workspace packages
- Prisma migrate dev applied cleanly against fresh volume
- docker compose dev: postgres/redis/kafka healthy, api-server /api/health
  200, worker-service running, web /markets 200

Plan: PR3 will install Claude Code CLI in the worker image, add ClaudeToken
storage + /settings/claude UI, build the LLM trade form, and wire the
Claude-driven signal flow on top of this saga.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 30, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
coin-web Ready Ready Preview, Comment Apr 30, 2026 5:13pm

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Apr 30, 2026

Reviewer's Guide

Implements a Binance USDT-M futures execution stack: rewrites the Binance REST adapter to use fapi with network-aware mainnet/testnet routing, extends core order/position types with futures fields, wires a new futures-aware real-order saga (including account configuration and TP/SL attachment with safety compensation), adds API DTOs and exchange key support for futures parameters and networks, introduces debug futures-test endpoint, and updates the database schema for futures orders and upcoming LLM integration models.

Sequence diagram for the debug futures-test endpoint

sequenceDiagram
    actor Dev
    participant ApiServer as ApiServer_DebugController
    participant Prisma as PrismaService
    participant Adapter as BinanceRest
    participant Binance as BinanceFuturesAPI

    Dev->>ApiServer: POST /debug/futures-test (FuturesTestDto)
    ApiServer->>ApiServer: Check NODE_ENV !== production
    ApiServer->>Prisma: findUnique(exchangeKeyId)
    Prisma-->>ApiServer: ExchangeKey (apiKey, secretKey, network)

    ApiServer->>ApiServer: decrypt(apiKey, secretKey)
    ApiServer->>Adapter: new BinanceRest()

    ApiServer->>Adapter: setPositionMode(credentials, false)
    Adapter->>Binance: POST /fapi/v1/positionSide/dual
    Binance-->>Adapter: 200 or -4059

    ApiServer->>Adapter: setMarginType(credentials, symbol, ISOLATED)
    Adapter->>Binance: POST /fapi/v1/marginType
    Binance-->>Adapter: 200 or -4046

    ApiServer->>Adapter: setLeverage(credentials, symbol, leverage)
    Adapter->>Binance: POST /fapi/v1/leverage
    Binance-->>Adapter: 200

    ApiServer->>Adapter: placeOrder(market, side, qty, TP/SL)
    Adapter->>Binance: POST /fapi/v1/order
    Binance-->>Adapter: Entry order response
    Adapter-->>ApiServer: OrderResult entry

    alt takeProfitPrice present
      ApiServer->>Adapter: placeTakeProfit(credentials, symbol, side, tpPrice)
      Adapter->>Binance: POST /fapi/v1/order (TAKE_PROFIT_MARKET)
      Binance-->>Adapter: TP order response
      Adapter-->>ApiServer: OrderResult tp
    end

    alt stopLossPrice present
      ApiServer->>Adapter: placeStopLoss(credentials, symbol, side, slPrice)
      Adapter->>Binance: POST /fapi/v1/order (STOP_MARKET)
      Binance-->>Adapter: SL order response
      Adapter-->>ApiServer: OrderResult sl
    end

    ApiServer->>Adapter: getPosition(credentials, symbol)
    Adapter->>Binance: GET /fapi/v2/positionRisk
    Binance-->>Adapter: Position data
    Adapter-->>ApiServer: Position

    ApiServer-->>Dev: {network, entry, tpOrderId, slOrderId, position}
Loading

Entity relationship diagram for futures and LLM DB changes

erDiagram
    User {
      text id PK
    }

    ExchangeKey {
      text id PK
      text userId FK
      text exchange
      text network
      text apiKey
      text secretKey
      datetime createdAt
      datetime updatedAt
      unique userId_exchange_network
    }

    Order {
      text id PK
      text userId FK
      text exchange
      text symbol
      text side
      text type
      text status
      text quantity
      text price
      text filledQuantity
      text filledPrice
      text fee
      text feeCurrency
      int leverage
      text marginType
      text positionSide
      text entryPrice
      text liquidationPrice
      text takeProfitPrice
      text stopLossPrice
      text tpOrderId
      text slOrderId
      text realizedPnl
      datetime closedAt
      datetime createdAt
      datetime updatedAt
      index userId_createdAt
    }

    ClaudeToken {
      text id PK
      text userId FK
      text encryptedToken
      datetime createdAt
      datetime updatedAt
      unique userId
    }

    LlmDecisionLog {
      text id PK
      text userId FK
      text orderId FK
      text prompt
      text rawResponse
      jsonb parsedSignal
      text model
      int latencyMs
      datetime createdAt
      unique orderId
      index userId_createdAt
    }

    User ||--o{ ExchangeKey : has
    User ||--o{ Order : has
    User ||--o{ ClaudeToken : has
    User ||--o{ LlmDecisionLog : has
    Order ||--o| LlmDecisionLog : decision_for
Loading

Updated class diagram for futures order types and Binance REST adapter

classDiagram
    class ExchangeCredentials {
      +string apiKey
      +string secretKey
      +Network network
    }

    class Network {
      <<enumeration>>
      mainnet
      testnet
    }

    class PositionSide {
      <<enumeration>>
      long
      short
    }

    class MarginType {
      <<enumeration>>
      ISOLATED
      CROSS
    }

    class OrderStatus {
      <<enumeration>>
      pending
      placed
      filled
      partial
      cancelled
      failed
    }

    class OrderRequest {
      +ExchangeId exchange
      +string symbol
      +PositionSide side
      +string type
      +string quantity
      +string price
      +number leverage
      +MarginType marginType
      +string takeProfitPrice
      +string stopLossPrice
    }

    class OrderResult {
      +ExchangeId exchange
      +string orderId
      +string symbol
      +PositionSide side
      +string type
      +OrderStatus status
      +string quantity
      +string filledQuantity
      +string price
      +string filledPrice
      +string fee
      +string feeCurrency
      +number timestamp
      +string entryPrice
      +string liquidationPrice
      +number leverage
      +string tpOrderId
      +string slOrderId
    }

    class Position {
      +ExchangeId exchange
      +string symbol
      +PositionSide side
      +string quantity
      +string entryPrice
      +string markPrice
      +string liquidationPrice
      +number leverage
      +MarginType marginType
      +string unrealizedPnl
    }

    class SymbolFilter {
      +string symbol
      +number pricePrecision
      +number quantityPrecision
      +string minQty
      +string stepSize
      +string minNotional
      +string tickSize
    }

    class IExchangeRest {
      <<interface>>
      +getBalances(credentials, ) Balance[]
      +getOpenOrders(credentials, symbol) OrderResult[]
      +placeOrder(credentials, order) OrderResult
      +cancelOrder(credentials, orderId, symbol) OrderResult
      +getOrder(credentials, orderId, symbol) OrderResult
      +getMarkets() Market[]
      +getCandles(symbol, interval, limit) Candle[]
      +getHistoricalCandles(symbol, interval, startTime, endTime) Candle[]
      +setLeverage(credentials, symbol, leverage) void
      +setMarginType(credentials, symbol, marginType) void
      +setPositionMode(credentials, dualSide) void
      +getPosition(credentials, symbol) Position
      +closePosition(credentials, symbol, side, quantity) OrderResult
      +placeStopLoss(credentials, symbol, side, stopPrice) OrderResult
      +placeTakeProfit(credentials, symbol, side, stopPrice) OrderResult
      +getSymbolFilter(symbol) SymbolFilter
    }

    class BinanceRest {
      +string exchangeId
      +getBalances(credentials) Balance[]
      +getOpenOrders(credentials, symbol) OrderResult[]
      +placeOrder(credentials, order) OrderResult
      +cancelOrder(credentials, orderId, symbol) OrderResult
      +getOrder(credentials, orderId, symbol) OrderResult
      +getMarkets() Market[]
      +getCandles(symbol, interval, limit) Candle[]
      +getHistoricalCandles(symbol, interval, startTime, endTime) Candle[]
      +setLeverage(credentials, symbol, leverage) void
      +setMarginType(credentials, symbol, marginType) void
      +setPositionMode(credentials, dualSide) void
      +getPosition(credentials, symbol) Position
      +closePosition(credentials, symbol, side, quantity) OrderResult
      +placeStopLoss(credentials, symbol, side, stopPrice) OrderResult
      +placeTakeProfit(credentials, symbol, side, stopPrice) OrderResult
      +getSymbolFilter(symbol) SymbolFilter
      -fetchExchangeInfo(network) ExchangeInfoSymbol
      -mapOrderResult(response) OrderResult
      -mapOrderStatus(status) OrderStatus
      -signedRequest(credentials, method, path, params) Response
    }

    IExchangeRest <|.. BinanceRest
    OrderRequest --> PositionSide
    OrderRequest --> MarginType
    OrderResult --> OrderStatus
    OrderResult --> PositionSide
    Position --> PositionSide
    Position --> MarginType
    IExchangeRest --> ExchangeCredentials
    BinanceRest --> ExchangeCredentials
    BinanceRest --> Position
    BinanceRest --> SymbolFilter
Loading

Class diagram for order lifecycle and futures-aware sagas

classDiagram
    class CreateOrderDto {
      +string exchange
      +string symbol
      +string side
      +string type
      +string quantity
      +string price
      +number leverage
      +string marginType
      +string takeProfitPrice
      +string stopLossPrice
      +string mode
      +string exchangeKeyId
    }

    class OrderLifecycleContext {
      +string userId
      +string exchange
      +string symbol
      +string side
      +string type
      +string mode
      +string quantity
      +string price
      +string exchangeKeyId
      +number leverage
      +string marginType
      +string takeProfitPrice
      +string stopLossPrice
      +string orderId
      +string requestId
    }

    class SagaStep_OrderLifecycle {
      <<interface>>
      +string name
      +execute(context) OrderLifecycleContext
      +compensate(context) void
    }

    class CreateOrderStep {
      +string name
      +execute(context) OrderLifecycleContext
      +compensate(context) void
    }

    class PublishOrderRequestStep {
      +string name
      +execute(context) OrderLifecycleContext
      +compensate(context) void
    }

    class RealExecutionContext {
      +OrderRequestedEvent event
      +ExchangeCredentials credentials
      +OrderResult result
      +string tpOrderId
      +string slOrderId
    }

    class SagaStep_RealExecution {
      <<interface>>
      +string name
      +execute(context) RealExecutionContext
      +compensate(context) void
    }

    class DecryptKeysStep {
      +string name
      +execute(context) RealExecutionContext
      +compensate(context) void
    }

    class ConfigureFuturesAccountStep {
      +string name
      +execute(context) RealExecutionContext
      +compensate(context) void
    }

    class PlaceOrderStep {
      +string name
      +execute(context) RealExecutionContext
      +compensate(context) void
    }

    class AttachTpSlStep {
      +string name
      +execute(context) RealExecutionContext
      +compensate(context) void
    }

    class UpdateDbStep {
      +string name
      +execute(context) RealExecutionContext
      +compensate(context) void
    }

    class PublishResultStep {
      +string name
      +execute(context) RealExecutionContext
      +compensate(context) void
    }

    class DebugController {
      +futuresTest(dto) FuturesTestResponse
    }

    CreateOrderDto --> OrderLifecycleContext
    SagaStep_OrderLifecycle <|.. CreateOrderStep
    SagaStep_OrderLifecycle <|.. PublishOrderRequestStep

    SagaStep_RealExecution <|.. DecryptKeysStep
    SagaStep_RealExecution <|.. ConfigureFuturesAccountStep
    SagaStep_RealExecution <|.. PlaceOrderStep
    SagaStep_RealExecution <|.. AttachTpSlStep
    SagaStep_RealExecution <|.. UpdateDbStep
    SagaStep_RealExecution <|.. PublishResultStep

    RealExecutionContext --> ExchangeCredentials
    RealExecutionContext --> OrderResult
    DebugController --> ExchangeCredentials
    DebugController --> OrderRequest
    DebugController --> OrderResult
    DebugController --> Position
Loading

File-Level Changes

Change Details Files
Rewrite Binance REST adapter for USDT-M futures with network-aware routing and futures operations.
  • Switch REST base URLs to Binance fapi mainnet/testnet and derive URL from ExchangeCredentials.network.
  • Update balances, open orders, order placement, cancel/get order, markets, and candles to use futures endpoints and response shapes.
  • Add futures-specific methods: leverage/margin/position mode configuration, position fetch/close, TP/SL placement, and symbol filter retrieval with caching.
  • Map futures order responses into enriched OrderResult including correct side, filled price, and timestamps, and standardize error messages to fapi.
packages/exchange-adapters/src/binance/binance.rest.ts
packages/exchange-adapters/src/interfaces/exchange-rest.ts
packages/types/src/exchange.ts
Extend order, position, and exchange key domain models for futures (leverage, margin, TP/SL, network).
  • Change order side semantics to PositionSide (long/short) and add leverage, marginType, take-profit and stop-loss prices to OrderRequest/OrderResult/Order.
  • Introduce Network type and optional network on ExchangeCredentials plus SymbolFilter and Position types.
  • Update Prisma schema and migration to add futures-related columns on Order, add network to ExchangeKey with widened uniqueness, and introduce ClaudeToken and LlmDecisionLog tables and indexes.
packages/types/src/exchange.ts
packages/database/prisma/schema.prisma
packages/database/prisma/migrations/20260430170658_init/migration.sql
Make API order creation and lifecycle futures-aware, including leverage/margin and TP/SL propagation into worker saga.
  • Update CreateOrderDto to accept futures fields (leverage, optional marginType, TP/SL), lock mode to 'real', require exchangeKeyId, and shift symbol/side semantics to futures (BTCUSDT, long/short).
  • Thread leverage, marginType, TP/SL through OrderLifecycleContext, CreateOrderStep DB insert, and PublishOrderRequestStep Kafka payload.
  • Ensure worker-side DecryptKeysStep populates credentials.network from ExchangeKey, and UpdateDbStep/PublishResultStep persist and emit futures metadata (position side, leverage, entry/TP/SL, TP/SL order IDs).
apps/api-server/src/orders/dto/create-order.dto.ts
apps/api-server/src/orders/sagas/order-lifecycle-steps.ts
apps/api-server/src/orders/sagas/order-lifecycle.orchestrator.ts
apps/worker-service/src/orders/sagas/real-execution-steps.ts
apps/api-server/src/exchange-keys/dto/create-exchange-key.dto.ts
apps/api-server/src/exchange-keys/commands/create-exchange-key.handler.ts
Introduce futures-specific worker saga steps for account configuration and TP/SL attachment with safety compensation.
  • Add ConfigureFuturesAccountStep to idempotently set one-way position mode, margin type, and leverage before placing the order.
  • Simplify PlaceOrderStep (drop Redis), keep market-fill polling, and add AttachTpSlStep to place STOP_MARKET and TAKE_PROFIT_MARKET close-position orders after fill.
  • Have AttachTpSlStep force-close the position via closePosition if TP/SL placement fails, and wire these steps into executeRealOrderSaga before DB update and event publish.
apps/worker-service/src/orders/sagas/real-execution-steps.ts
Add network-aware exchange key creation and a dev-only futures test endpoint for adapter smoke testing.
  • Extend CreateExchangeKeyDto and handler to accept optional network (default mainnet), validate keys against the correct network, and upsert ExchangeKey by (userId, exchange, network).
  • Introduce DebugModule and DebugController with /debug/futures-test POST endpoint that decrypts a key, configures futures account, places a market entry plus optional TP/SL via the adapter, fetches the resulting position, and returns all data; gate by NODE_ENV !== 'production'.
  • Wire DebugModule into AppModule so the endpoint is available in dev.
apps/api-server/src/exchange-keys/dto/create-exchange-key.dto.ts
apps/api-server/src/exchange-keys/commands/create-exchange-key.handler.ts
apps/api-server/src/debug/debug.controller.ts
apps/api-server/src/debug/debug.module.ts
apps/api-server/src/app.module.ts

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • In BinanceRest.setMarginType/setPositionMode the idempotent success detection relies on substring checks of err.message (e.g. '-4046'), which is brittle against message format changes; consider parsing the Binance error payload (e.g. code field from JSON) and branching on that instead.
  • BinanceRest.fetchExchangeInfo and the public getMarkets/getCandles paths always use the mainnet public base URL (or a hardcoded default network), so they ignore the ExchangeCredentials.network testnet/mainnet distinction; if you expect behavior differences between networks, it may be worth threading the network through or exposing a network-aware variant.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `BinanceRest.setMarginType`/`setPositionMode` the idempotent success detection relies on substring checks of `err.message` (e.g. `'-4046'`), which is brittle against message format changes; consider parsing the Binance error payload (e.g. code field from JSON) and branching on that instead.
- `BinanceRest.fetchExchangeInfo` and the public `getMarkets`/`getCandles` paths always use the mainnet public base URL (or a hardcoded default network), so they ignore the `ExchangeCredentials.network` testnet/mainnet distinction; if you expect behavior differences between networks, it may be worth threading the network through or exposing a network-aware variant.

## Individual Comments

### Comment 1
<location path="apps/worker-service/src/orders/sagas/real-execution-steps.ts" line_range="159-168" />
<code_context>
+export class AttachTpSlStep implements SagaStep {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** On TP/SL attach failure you force-close the position but don't cancel any already-created TP/SL order.

If only one of TP/SL is created and the other fails, the catch block force‑closes the position but leaves the successfully created order live. In one‑way mode with `closePosition=true` this is probably invalid after close, but it still leaves a dangling order on the exchange.

Consider tracking which legs were created and cancelling them around the forced close so the account state stays consistent. If you’re deliberately relying on the exchange to noop these, it’d be useful to document that assumption centrally so future changes don’t accidentally violate it.

Suggested implementation:

```typescript
/**
 * Attaches STOP_MARKET (SL) and TAKE_PROFIT_MARKET (TP) close-position orders
 * after the entry has filled. If either fails, we:
 *
 * - Cancel any TP/SL legs that were successfully created so far, and then
 * - Compensate by force-closing the underlying position so it never sits naked.
 *
 * This keeps account state consistent and avoids leaving dangling TP/SL orders
 * on the exchange after a forced close.
 */
export class AttachTpSlStep implements SagaStep {
  readonly name = 'AttachTpSl';
  private readonly logger = new Logger(AttachTpSlStep.name);

  async execute(context: RealExecutionContext): Promise<RealExecutionContext> {
    const { event, credentials, result } = context;
    if (!credentials) throw new Error('No credentials available');
    if (!result) throw new Error('No entry order result available');

    // Track successfully created TP/SL legs so we can cancel them on failure
    const createdTpOrderIds: string[] = [];
    const createdSlOrderIds: string[] = [];

    const order = event.order;
    if (!order.takeProfitPrice && !order.stopLossPrice) {

```

The visible snippet only shows the beginning of `AttachTpSlStep.execute`, not the core logic where TP/SL orders are actually created and where the force-close compensation happens. To fully implement the behavior described in the doc comment and my review, you’ll need to:

1. **Track created TP/SL legs at creation time**
   - Wherever you currently create the TP order (e.g. `client.createOrder` / `exchange.placeOrder` / similar), capture the returned order ID and push it into `createdTpOrderIds`.
     - Example pattern (adapt to your actual client API):
       ```ts
       const tpOrder = await client.createOrder({ ...tpPayload });
       createdTpOrderIds.push(tpOrder.id);
       ```
   - Do the same for the SL order, pushing into `createdSlOrderIds`.

2. **Cancel created legs in the error path before or around the forced close**
   - In the `try/catch` where you currently handle TP/SL attach failure and perform the forced close:
     - In the `catch` block, before (or immediately after) sending the force-close order, cancel any previously created legs.
     - Example pattern:
       ```ts
       } catch (err) {
         this.logger.error('Failed to attach TP/SL legs', { err });

         // Best-effort cancel of any already-attached legs
         for (const orderId of [...createdTpOrderIds, ...createdSlOrderIds]) {
           try {
             await client.cancelOrder(orderId, symbol);
           } catch (cancelErr) {
             this.logger.warn('Failed to cancel dangling TP/SL leg', {
               orderId,
               cancelErr,
             });
           }
         }

         // Existing force-close compensation
         await this.forceClosePosition({ client, symbol, side, ... });
       }
       ```
   - Ensure you use the correct symbol / market ID and client instance consistent with the rest of the saga.

3. **Handle partial failures in a single-attach call**
   - If TP/SL orders are created in sequence within a single `try` block, the `catch` will naturally see which legs made it into `createdTpOrderIds` / `createdSlOrderIds`.
   - If the implementation uses multiple `try/catch` blocks or helper methods, propagate `createdTpOrderIds` / `createdSlOrderIds` into those helpers or return the created order IDs so they can be cancelled from one central error handler.

4. **Preserve idempotency / resilience**
   - Make cancellation “best-effort”: log and continue if a cancel fails, rather than failing the entire saga.
   - If the exchange client throws a specific “order not found / already closed” error, treat that as a noop to avoid spurious warnings.

Adjust the exact method names (`cancelOrder`, `forceClosePosition`, client acquisition, symbol/side handling) to match the existing conventions in `real-execution-steps.ts` and any shared exchange service you already use.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +159 to +168
export class AttachTpSlStep implements SagaStep {
readonly name = 'AttachTpSl';
private readonly logger = new Logger(AttachTpSlStep.name);

async execute(context: RealExecutionContext): Promise<RealExecutionContext> {
const { event, credentials, result } = context;
if (!credentials) throw new Error('No credentials available');
if (!result) throw new Error('No entry order result available');

const order = event.order;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): On TP/SL attach failure you force-close the position but don't cancel any already-created TP/SL order.

If only one of TP/SL is created and the other fails, the catch block force‑closes the position but leaves the successfully created order live. In one‑way mode with closePosition=true this is probably invalid after close, but it still leaves a dangling order on the exchange.

Consider tracking which legs were created and cancelling them around the forced close so the account state stays consistent. If you’re deliberately relying on the exchange to noop these, it’d be useful to document that assumption centrally so future changes don’t accidentally violate it.

Suggested implementation:

/**
 * Attaches STOP_MARKET (SL) and TAKE_PROFIT_MARKET (TP) close-position orders
 * after the entry has filled. If either fails, we:
 *
 * - Cancel any TP/SL legs that were successfully created so far, and then
 * - Compensate by force-closing the underlying position so it never sits naked.
 *
 * This keeps account state consistent and avoids leaving dangling TP/SL orders
 * on the exchange after a forced close.
 */
export class AttachTpSlStep implements SagaStep {
  readonly name = 'AttachTpSl';
  private readonly logger = new Logger(AttachTpSlStep.name);

  async execute(context: RealExecutionContext): Promise<RealExecutionContext> {
    const { event, credentials, result } = context;
    if (!credentials) throw new Error('No credentials available');
    if (!result) throw new Error('No entry order result available');

    // Track successfully created TP/SL legs so we can cancel them on failure
    const createdTpOrderIds: string[] = [];
    const createdSlOrderIds: string[] = [];

    const order = event.order;
    if (!order.takeProfitPrice && !order.stopLossPrice) {

The visible snippet only shows the beginning of AttachTpSlStep.execute, not the core logic where TP/SL orders are actually created and where the force-close compensation happens. To fully implement the behavior described in the doc comment and my review, you’ll need to:

  1. Track created TP/SL legs at creation time

    • Wherever you currently create the TP order (e.g. client.createOrder / exchange.placeOrder / similar), capture the returned order ID and push it into createdTpOrderIds.
      • Example pattern (adapt to your actual client API):
        const tpOrder = await client.createOrder({ ...tpPayload });
        createdTpOrderIds.push(tpOrder.id);
    • Do the same for the SL order, pushing into createdSlOrderIds.
  2. Cancel created legs in the error path before or around the forced close

    • In the try/catch where you currently handle TP/SL attach failure and perform the forced close:
      • In the catch block, before (or immediately after) sending the force-close order, cancel any previously created legs.
      • Example pattern:
        } catch (err) {
          this.logger.error('Failed to attach TP/SL legs', { err });
        
          // Best-effort cancel of any already-attached legs
          for (const orderId of [...createdTpOrderIds, ...createdSlOrderIds]) {
            try {
              await client.cancelOrder(orderId, symbol);
            } catch (cancelErr) {
              this.logger.warn('Failed to cancel dangling TP/SL leg', {
                orderId,
                cancelErr,
              });
            }
          }
        
          // Existing force-close compensation
          await this.forceClosePosition({ client, symbol, side, ... });
        }
    • Ensure you use the correct symbol / market ID and client instance consistent with the rest of the saga.
  3. Handle partial failures in a single-attach call

    • If TP/SL orders are created in sequence within a single try block, the catch will naturally see which legs made it into createdTpOrderIds / createdSlOrderIds.
    • If the implementation uses multiple try/catch blocks or helper methods, propagate createdTpOrderIds / createdSlOrderIds into those helpers or return the created order IDs so they can be cancelled from one central error handler.
  4. Preserve idempotency / resilience

    • Make cancellation “best-effort”: log and continue if a cancel fails, rather than failing the entire saga.
    • If the exchange client throws a specific “order not found / already closed” error, treat that as a noop to avoid spurious warnings.

Adjust the exact method names (cancelOrder, forceClosePosition, client acquisition, symbol/side handling) to match the existing conventions in real-execution-steps.ts and any shared exchange service you already use.

@fray-cloud fray-cloud merged commit 4db6576 into feat/binance-only-cleanup May 2, 2026
3 checks passed
fray-cloud added a commit that referenced this pull request May 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant