Fix OKX spot order fills handling

cjdsellers · cjdsellers · commit 806f1f8fc812 · 2025-10-12T20:59:06.000+10:00
Trade SPOT with reconciliation off until spot position reports can be
implemented.
diff --git a/crates/adapters/okx/src/common/consts.rs b/crates/adapters/okx/src/common/consts.rs
@@ -117,3 +117,9 @@ pub const OKX_POST_ONLY_CANCEL_SOURCE: &str = "31";
 
 /// Human-readable reason used when a post-only order is auto-cancelled for taking liquidity.
 pub const OKX_POST_ONLY_CANCEL_REASON: &str = "POST_ONLY would take liquidity";
+
+/// Target currency literal for base currency.
+pub const OKX_TARGET_CCY_BASE: &str = "base_ccy";
+
+/// Target currency literal for quote currency.
+pub const OKX_TARGET_CCY_QUOTE: &str = "quote_ccy";
diff --git a/crates/adapters/okx/src/common/parse.rs b/crates/adapters/okx/src/common/parse.rs
@@ -160,7 +160,7 @@ pub fn okx_instrument_type(instrument: &InstrumentAny) -> anyhow::Result<OKXInst
         InstrumentAny::CurrencyPair(_) => Ok(OKXInstrumentType::Spot),
         InstrumentAny::CryptoPerpetual(_) => Ok(OKXInstrumentType::Swap),
         InstrumentAny::CryptoFuture(_) => Ok(OKXInstrumentType::Futures),
-        InstrumentAny::OptionContract(_) => Ok(OKXInstrumentType::Option),
+        InstrumentAny::CryptoOption(_) => Ok(OKXInstrumentType::Option),
         _ => anyhow::bail!("Invalid instrument type for OKX: {instrument:?}"),
     }
 }
diff --git a/crates/adapters/okx/src/websocket/client.rs b/crates/adapters/okx/src/websocket/client.rs
@@ -88,7 +88,7 @@ use crate::{
         consts::{
             OKX_NAUTILUS_BROKER_ID, OKX_POST_ONLY_CANCEL_REASON, OKX_POST_ONLY_CANCEL_SOURCE,
             OKX_POST_ONLY_ERROR_CODE, OKX_SUPPORTED_ORDER_TYPES, OKX_SUPPORTED_TIME_IN_FORCE,
-            OKX_WS_PUBLIC_URL, should_retry_error_code,
+            OKX_TARGET_CCY_BASE, OKX_TARGET_CCY_QUOTE, OKX_WS_PUBLIC_URL, should_retry_error_code,
         },
         credential::Credential,
         enums::{
@@ -2260,7 +2260,6 @@ impl OKXWebSocketClient {
             _ => {
                 // For other instrument types (OPTIONS, etc.), use quote currency as fallback
                 builder.ccy(quote_currency.to_string());
-                builder.tgt_ccy(quote_currency.to_string());
 
                 // TODO: Consider position mode (only applicable for NET)
                 if let Some(ro) = reduce_only
@@ -2271,12 +2270,27 @@ impl OKXWebSocketClient {
             }
         };
 
-        if let Some(is_quote_quantity) = quote_quantity
-            && is_quote_quantity
-        {
-            builder.tgt_ccy(quote_currency.to_string());
+        if instrument_type == OKXInstrumentType::Spot && order_type == OrderType::Market {
+            // https://www.okx.com/docs-v5/en/#order-book-trading-trade-post-place-order
+            // OKX API default behavior for SPOT:
+            // - BUY orders default to tgtCcy=quote_ccy
+            // - SELL orders default to tgtCcy=base_ccy
+            match quote_quantity {
+                Some(true) => {
+                    builder.tgt_ccy(OKX_TARGET_CCY_QUOTE.to_string());
+                }
+                Some(false) => {
+                    if order_side == OrderSide::Buy {
+                        // For BUY orders, must explicitly set to base_ccy to override OKX default
+                        builder.tgt_ccy(OKX_TARGET_CCY_BASE.to_string());
+                    }
+                    // For SELL orders with quote_quantity=false, omit tgtCcy (OKX defaults to base_ccy correctly)
+                }
+                None => {
+                    // No preference specified, use OKX defaults
+                }
+            }
         }
-        // If is_quote_quantity is false, we don't set tgtCcy (defaults to base currency)
 
         builder.side(order_side);
 
diff --git a/crates/adapters/okx/src/websocket/parse.rs b/crates/adapters/okx/src/websocket/parse.rs
@@ -16,7 +16,7 @@
 //! Functions translating raw OKX WebSocket frames into Nautilus data types.
 
 use ahash::AHashMap;
-use nautilus_core::nanos::UnixNanos;
+use nautilus_core::{UUID4, nanos::UnixNanos};
 use nautilus_model::{
     data::{
         Bar, BarSpecification, BarType, BookOrder, Data, FundingRateUpdate, IndexPriceUpdate,
@@ -644,8 +644,12 @@ pub fn parse_order_msg(
 
     let previous_fee = fee_cache.get(&msg.ord_id).copied();
 
+    // Only generate fill reports when there's actual new fill data
+    // Check if fillSz is non-zero/non-empty OR trade_id is present
+    let has_new_fill = (!msg.fill_sz.is_empty() && msg.fill_sz != "0") || !msg.trade_id.is_empty();
+
     match msg.state {
-        OKXOrderStatus::Filled | OKXOrderStatus::PartiallyFilled => {
+        OKXOrderStatus::Filled | OKXOrderStatus::PartiallyFilled if has_new_fill => {
             parse_fill_report(msg, instrument, account_id, previous_fee, ts_init)
                 .map(ExecutionReport::Fill)
         }
@@ -804,7 +808,6 @@ pub fn parse_order_status_report(
     let size_precision = instrument.size_precision();
     let quantity = parse_quantity(&msg.sz, size_precision)?;
     let filled_qty = parse_quantity(&msg.acc_fill_sz.clone().unwrap_or_default(), size_precision)?;
-
     let ts_accepted = parse_millisecond_timestamp(msg.c_time);
     let ts_last = parse_millisecond_timestamp(msg.u_time);
 
@@ -904,16 +907,65 @@ pub fn parse_fill_report(
 ) -> anyhow::Result<FillReport> {
     let client_order_id = parse_client_order_id(&msg.cl_ord_id);
     let venue_order_id = VenueOrderId::new(msg.ord_id);
-    let trade_id = TradeId::from(msg.trade_id.as_str());
+
+    // TODO: Extract to dedicated function:
+    // OKX may not provide a trade_id, so generate a UUID4 as fallback
+    let trade_id = if msg.trade_id.is_empty() {
+        TradeId::from(UUID4::new().to_string().as_str())
+    } else {
+        TradeId::from(msg.trade_id.as_str())
+    };
+
     let order_side: OrderSide = msg.side.into();
 
     let price_precision = instrument.price_precision();
     let size_precision = instrument.size_precision();
-    let last_px = parse_price(&msg.fill_px, price_precision)?;
-    let last_qty = parse_quantity(&msg.fill_sz, size_precision)?;
+
+    // TODO: Extract to dedicated function:
+    // OKX may not provide fillPx for some orders, fall back to avgPx or lastPx
+    let price_str = if !msg.fill_px.is_empty() {
+        &msg.fill_px
+    } else if !msg.avg_px.is_empty() {
+        &msg.avg_px
+    } else {
+        &msg.px // Last resort, use order price
+    };
+    let last_px = parse_price(price_str, price_precision).map_err(|e| {
+        anyhow::anyhow!(
+            "Failed to parse price (fill_px='{}', avg_px='{}', px='{}'): {}",
+            msg.fill_px,
+            msg.avg_px,
+            msg.px,
+            e
+        )
+    })?;
+
+    // TODO: Extract to dedicated function:
+    // OKX may not provide fillSz for some orders, fall back to accFillSz (accumulated fill size)
+    let qty_str = if !msg.fill_sz.is_empty() && msg.fill_sz != "0" {
+        &msg.fill_sz
+    } else if let Some(ref acc_fill_sz) = msg.acc_fill_sz {
+        if !acc_fill_sz.is_empty() && acc_fill_sz != "0" {
+            acc_fill_sz
+        } else {
+            &msg.sz // Last resort, use order size
+        }
+    } else {
+        &msg.sz // Last resort, use order size
+    };
+    let last_qty = parse_quantity(qty_str, size_precision).map_err(|e| {
+        anyhow::anyhow!(
+            "Failed to parse quantity (fill_sz='{}', acc_fill_sz={:?}, sz='{}'): {}",
+            msg.fill_sz,
+            msg.acc_fill_sz,
+            msg.sz,
+            e
+        )
+    })?;
 
     let fee_currency = Currency::from(&msg.fee_ccy);
-    let total_fee = parse_fee(msg.fee.as_deref(), fee_currency)?;
+    let total_fee = parse_fee(msg.fee.as_deref(), fee_currency)
+        .map_err(|e| anyhow::anyhow!("Failed to parse fee={:?}: {}", msg.fee, e))?;
     let commission = if let Some(previous_fee) = previous_fee {
         total_fee - previous_fee
     } else {
diff --git a/docs/integrations/okx.md b/docs/integrations/okx.md
@@ -216,10 +216,10 @@ OKX supports four trade modes, which the adapter selects automatically based on
 
 | Mode                | Used For                                   | Leverage | Borrowing | Configuration |
 |---------------------|--------------------------------------------|----------|-----------|---------------|
-| **`cash`**          | Simple spot trading                        | ❌ No    | ❌ No     | `use_spot_margin=False` (default for SPOT) |
-| **`spot_isolated`** | Spot trading with margin/leverage          | ✅ Yes   | ✅ Yes    | `use_spot_margin=True` |
-| **`isolated`**      | Derivatives trading (SWAP/FUTURES/OPTIONS) | ✅ Yes   | ✅ Yes    | `margin_mode=ISOLATED` or unset (default for derivatives) |
-| **`cross`**         | Derivatives with shared margin pool        | ✅ Yes   | ✅ Yes    | `margin_mode=CROSS` |
+| **`cash`**          | Simple spot trading                        | -        | -         | `use_spot_margin=False` (default for SPOT) |
+| **`spot_isolated`** | Spot trading with margin/leverage          | ✓        | ✓         | `use_spot_margin=True` |
+| **`isolated`**      | Derivatives trading (SWAP/FUTURES/OPTIONS) | ✓        | ✓         | `margin_mode=ISOLATED` or unset (default for derivatives) |
+| **`cross`**         | Derivatives with shared margin pool        | ✓        | ✓         | `margin_mode=CROSS` |
 
 #### Configuration-based trade mode selection
 
diff --git a/examples/live/okx/okx_exec_tester.py b/examples/live/okx/okx_exec_tester.py
@@ -27,10 +27,9 @@
 from nautilus_trader.config import TradingNodeConfig
 from nautilus_trader.core.nautilus_pyo3 import OKXContractType
 from nautilus_trader.core.nautilus_pyo3 import OKXInstrumentType
+from nautilus_trader.live.config import LiveRiskEngineConfig
 from nautilus_trader.live.node import TradingNode
-from nautilus_trader.model.enums import OrderType
 from nautilus_trader.model.enums import TimeInForce
-from nautilus_trader.model.enums import TriggerType
 from nautilus_trader.model.identifiers import InstrumentId
 from nautilus_trader.model.identifiers import TraderId
 from nautilus_trader.test_kit.strategies.tester_exec import ExecTester
@@ -64,6 +63,8 @@
 else:
     raise ValueError(f"Unsupported instrument type: {instrument_type}")
 
+instrument_id = InstrumentId.from_str(f"{symbol}.{OKX}")
+
 instrument_types = (instrument_type,)
 
 # instrument_types = (
@@ -78,7 +79,6 @@
     # "ETH-USDT",
 )
 
-
 # Configure the trading node
 config_node = TradingNodeConfig(
     trader_id=TraderId("TESTER-001"),
@@ -89,6 +89,7 @@
     ),
     exec_engine=LiveExecEngineConfig(
         reconciliation=True,
+        reconciliation_instrument_ids=[instrument_id],
         # reconciliation_lookback_mins=60,
         open_check_interval_secs=5.0,
         open_check_open_only=True,
@@ -105,6 +106,7 @@
         purge_account_events_lookback_mins=60,  # Purge account events occurring more than an hour ago
         graceful_shutdown_on_exception=True,
     ),
+    risk_engine=LiveRiskEngineConfig(bypass=True),  # Must bypass for spot margin for now
     # cache=CacheConfig(
     #     database=DatabaseConfig(),
     #     timestamps_as_iso8601=True,
@@ -147,6 +149,8 @@
             instrument_types=instrument_types,
             instrument_families=instrument_families,
             contract_types=contract_types,
+            # margin_mode=OKXMarginMode.ISOLATED,
+            use_spot_margin=False,
             # use_mm_mass_cancel=True,
             is_demo=False,  # If client uses the demo API
             use_fills_channel=False,  # Set to True if VIP5+ to get separate fill reports
@@ -165,25 +169,26 @@
 
 # Configure your strategy
 config_tester = ExecTesterConfig(
-    instrument_id=InstrumentId.from_str(f"{symbol}.OKX"),
-    external_order_claims=[InstrumentId.from_str(f"{symbol}.OKX")],
+    instrument_id=instrument_id,
+    external_order_claims=[instrument_id],
     use_hyphens_in_client_order_ids=False,  # OKX doesn't allow hyphens in client order IDs
     # subscribe_quotes=False,
     # subscribe_trades=False,
     # subscribe_book=True,
-    # enable_buys=False,
-    # enable_sells=False,
+    enable_buys=True,
+    enable_sells=True,
     # open_position_on_start_qty=order_qty,
     open_position_time_in_force=TimeInForce.FOK,
-    tob_offset_ticks=0,
-    stop_offset_ticks=1,
+    tob_offset_ticks=100,
+    # stop_offset_ticks=1,
     order_qty=order_qty,
     # modify_orders_to_maintain_tob_offset=True,
-    use_post_only=True,
+    # use_post_only=True,
+    # use_quote_quantity=True,
     # enable_stop_buys=True,
     # enable_stop_sells=True,
-    stop_order_type=OrderType.STOP_MARKET,
-    stop_trigger_type=TriggerType.LAST_PRICE,
+    # stop_order_type=OrderType.STOP_MARKET,
+    # stop_trigger_type=TriggerType.LAST_PRICE,
     # stop_offset_ticks=50,  # Offset from current price for stop trigger
     # stop_limit_offset_ticks=10,  # Additional offset for STOP_LIMIT orders
     # cancel_orders_on_stop=False,
diff --git a/nautilus_trader/adapters/okx/config.py b/nautilus_trader/adapters/okx/config.py
@@ -40,13 +40,13 @@ class OKXDataClientConfig(LiveDataClientConfig, frozen=True):
     instrument_types : tuple[OKXInstrumentType], default `(OKXInstrumentType.SPOT,)`
         The OKX instrument types of instruments to load.
         If None, all instrument types are loaded (subject to contract types and their compatibility with instrument types).
-    contract_types : tuple[OKXInstrumentType], optional
-        The OKX contract types of instruments to load.
-        If None, all contract types are loaded (subject to instrument types and their compatibility with contract types).
     instrument_families : tuple[str, ...], optional
         The OKX instrument families to load (e.g., "BTC-USD", "ETH-USD").
         Required for OPTIONS. Optional for FUTURES/SWAP. Not applicable for SPOT/MARGIN.
         If None, all available instrument families will be attempted (may fail for OPTIONS).
+    contract_types : tuple[OKXInstrumentType], optional
+        The OKX contract types of instruments to load.
+        If None, all contract types are loaded (subject to instrument types and their compatibility with contract types).
     base_url_http : str, optional
         The base url to OKX's http api.
         If ``None`` then will source the `get_http_base_url()`.
@@ -68,8 +68,8 @@ class OKXDataClientConfig(LiveDataClientConfig, frozen=True):
     api_secret: str | None = None
     api_passphrase: str | None = None
     instrument_types: tuple[OKXInstrumentType, ...] = (OKXInstrumentType.SPOT,)
-    contract_types: tuple[OKXContractType, ...] | None = None
     instrument_families: tuple[str, ...] | None = None
+    contract_types: tuple[OKXContractType, ...] | None = None
     base_url_http: str | None = None
     base_url_ws: str | None = None
     is_demo: bool = False
diff --git a/nautilus_trader/adapters/okx/execution.py b/nautilus_trader/adapters/okx/execution.py
@@ -711,6 +711,23 @@ async def generate_position_status_reports(  # noqa: C901 (too complex)
 
     # -- COMMAND HANDLERS -------------------------------------------------------------------------
 
+    def _parse_trade_mode_from_params(self, params: dict[str, Any] | None) -> OKXTradeMode:
+        if not params:
+            return self._trade_mode
+
+        td_mode_str = params.get("td_mode")
+        if not td_mode_str:
+            return self._trade_mode
+
+        try:
+            return OKXTradeMode(td_mode_str)
+        except ValueError:
+            self._log.warning(
+                f"Failed to parse OKXTradeMode: Valid modes are 'cash', 'isolated', 'cross', 'spot_isolated', "
+                f"falling back to '{str(self._trade_mode).lower()}'",
+            )
+            return self._trade_mode
+
     async def _query_account(self, _command: QueryAccount) -> None:
         # TODO: Specific account ID (sub account) not yet supported
         await self._update_account_state()
@@ -764,19 +781,7 @@ async def _submit_order_websocket(self, command: SubmitOrder) -> None:
             time_in_force_to_pyo3(order.time_in_force) if order.time_in_force else None
         )
 
-        td_mode = self._trade_mode
-
-        if command.params:
-            td_mode_str = command.params.get("td_mode")
-            if td_mode_str:
-                try:
-                    td_mode = OKXTradeMode(td_mode_str)
-                except ValueError:
-                    self._log.warning(
-                        f"Failed to parse OKXTradeMode: Valid modes are 'cash', 'isolated', 'cross', 'spot_isolated', "
-                        f"falling back to '{str(self._trade_mode).lower()}'",
-                    )
-                    td_mode = self._trade_mode
+        td_mode = self._parse_trade_mode_from_params(command.params)
 
         try:
             await self._ws_client.submit_order(
@@ -824,13 +829,7 @@ async def _submit_algo_order_http(self, command: SubmitOrder) -> None:
             trigger_type_to_pyo3(order.trigger_type) if hasattr(order, "trigger_type") else None
         )
 
-        td_mode = self._trade_mode
-        if command.params and "td_mode" in command.params:
-            td_mode_str = command.params["td_mode"]
-            try:
-                td_mode = OKXTradeMode(td_mode_str)
-            except ValueError:
-                self._log.warning(f"Invalid trade mode '{td_mode_str}', using default")
+        td_mode = self._parse_trade_mode_from_params(command.params)
 
         try:
             response = await self._http_client.place_algo_order(

Original file line number	Diff line number	Diff line change
`@@ -160,7 +160,7 @@ pub fn okx_instrument_type(instrument: &InstrumentAny) -> anyhow::Result<OKXInst`
`160`	`160`	`InstrumentAny::CurrencyPair(_) => Ok(OKXInstrumentType::Spot),`
`161`	`161`	`InstrumentAny::CryptoPerpetual(_) => Ok(OKXInstrumentType::Swap),`
`162`	`162`	`InstrumentAny::CryptoFuture(_) => Ok(OKXInstrumentType::Futures),`
`163`		`- InstrumentAny::OptionContract(_) => Ok(OKXInstrumentType::Option),`
	`163`	`+ InstrumentAny::CryptoOption(_) => Ok(OKXInstrumentType::Option),`
`164`	`164`	`_ => anyhow::bail!("Invalid instrument type for OKX: {instrument:?}"),`
`165`	`165`	`}`
`166`	`166`	`}`