# Optic Backend Audit Report **Date:** 2026-02-08 **Scope:** Full source audit of `/Users/elibutters/Documents/Optic/backend/` **Crates:** optic-core, optic-connector, optic-api **Files reviewed:** 80 Rust source files --- ## Summary | Severity | Count | |----------|-------| | CRITICAL | 4 | | HIGH | 10 | | MEDIUM | 12 | | LOW | 8 | | **Total** | **34** | --- ## CRITICAL Issues ### [CRITICAL] Unsafe File Descriptor Double-Ownership Causes Undefined Behavior **File:** crates/optic-connector/src/connectors/hyperliquid/connector.rs:66-74 **Category:** security **Description:** `OwnedFd::from_raw_fd(raw_fd)` takes ownership of the fd that is still owned by the TcpStream. When `Socket::from(owned_fd)` is created and then `std::mem::forget(socket)` is called, this avoids a double-close, but the `unsafe` block creates an `OwnedFd` from a borrowed fd — if the `forget` were ever removed or the code restructured, the fd would be double-closed, causing UB. More critically, the same pattern exists in dYdX and Lighter connectors. **Impact:** Potential file descriptor double-close leading to crashes or corrupted connections. The `std::mem::forget` is a fragile safety net. **Root Cause:** Using `OwnedFd::from_raw_fd` on an fd that is still owned by another type. **Fix:** Use `BorrowedFd` or `socket2::SockRef` instead: ```rust use socket2::SockRef; if let MaybeTlsStream::::Rustls(ref tls_stream) = *ws_stream.get_ref() { let tcp_stream = tls_stream.get_ref().0; let sock_ref = SockRef::from(tcp_stream); let keepalive = TcpKeepalive::new() .with_time(Duration::from_secs(60)) .with_interval(Duration::from_secs(10)); if let Err(e) = sock_ref.set_tcp_keepalive(&keepalive) { warn!("Failed to set TCP keepalive: {}", e); } } ``` **Effort:** small ### [CRITICAL] Same Unsafe FD Pattern in dYdX Connector **File:** crates/optic-connector/src/connectors/dydx/connector.rs:63-74 **Category:** security **Description:** Identical unsafe `OwnedFd::from_raw_fd` pattern as Hyperliquid. **Impact:** Same UB risk as above. **Root Cause:** Copy-paste of the unsafe fd pattern. **Fix:** Same as above — use `SockRef::from(tcp_stream)`. **Effort:** small ### [CRITICAL] Same Unsafe FD Pattern in Lighter Connector **File:** crates/optic-connector/src/connectors/lighter/connector.rs:63-74 **Category:** security **Description:** Identical unsafe `OwnedFd::from_raw_fd` pattern. **Impact:** Same UB risk. **Root Cause:** Copy-paste. **Fix:** Same as above. **Effort:** small ### [CRITICAL] Hardcoded JWT Secret in Production Default **File:** crates/optic-core/src/config/types.rs:185-186 **Category:** security **Description:** `jwt_secret` defaults to `"dev-secret-key"` when `JWT_SECRET` env var is missing. In production, if this env var is accidentally omitted, the API runs with a known, trivially guessable secret. **Impact:** Complete authentication bypass — any attacker can forge JWT tokens. **Root Cause:** Fallback default for a security-critical value. **Fix:** Panic if `JWT_SECRET` is not set in production, or at minimum log a loud warning: ```rust jwt_secret: env::var("JWT_SECRET").unwrap_or_else(|_| { let is_prod = env::var("ENVIRONMENT") .map(|e| e == "production" || e == "prod") .unwrap_or(false); if is_prod { panic!("JWT_SECRET must be set in production"); } eprintln!("WARNING: Using default JWT secret - NOT FOR PRODUCTION"); "dev-secret-key".to_string() }), ``` **Effort:** small --- ## HIGH Issues ### [HIGH] Semaphore Acquire Unwrap Can Panic Under Shutdown **File:** crates/optic-connector/src/state/manager.rs:75 **Category:** error-handling **Description:** `semaphore.clone().acquire_owned().await.unwrap()` — if the semaphore is closed (e.g., during shutdown), this panics. **Impact:** Process crash during shutdown or if semaphore limit is reconfigured. **Root Cause:** Unchecked `unwrap()` on fallible semaphore acquisition. **Fix:** ```rust let permit = match semaphore.clone().acquire_owned().await { Ok(p) => p, Err(_) => { warn!("Semaphore closed for exchange {}, dropping update", exchange_name_clone); continue; } }; ``` **Effort:** small ### [HIGH] Proxy Deletion Query Uses Incorrect Cartesian Match **File:** crates/optic-core/src/proxy_sync.rs:74-83 **Category:** logic **Description:** The DELETE query `WHERE NOT (host = ANY($1) AND port = ANY($2))` does a cartesian match — it keeps any row where host matches ANY host in the list AND port matches ANY port in the list. This means a proxy `host_a:8080` would NOT be deleted if `host_a` and `8080` both appear in the synced list, even if `host_a:8080` wasn't actually synced (e.g., synced proxies are `host_a:9090` and `host_b:8080`). **Impact:** Stale proxy configs persist in the database, potentially causing connections through dead proxies. **Root Cause:** SQL `ANY` on separate arrays doesn't enforce tuple matching. **Fix:** Use row-value comparison or a CTE: ```rust let deleted = sqlx::query( r#" DELETE FROM proxy_config WHERE NOT EXISTS ( SELECT 1 FROM unnest($1::TEXT[], $2::INT[]) AS t(h, p) WHERE proxy_config.host = t.h AND proxy_config.port = t.p ) "# ) .bind(&synced_hosts) .bind(&synced_ports) .execute(pool) .await?; ``` **Effort:** small ### [HIGH] Config Watcher `try_read().unwrap()` Can Panic **File:** crates/optic-connector/src/websocket_coordinator/websocket_coordinator.rs:111 **Category:** error-handling **Description:** `self.configs.try_read().unwrap()` panics if the RwLock is currently write-locked (e.g., during a config update). **Impact:** Process crash if `spawn_config_watcher` runs while config is being written. **Root Cause:** Using `try_read` + `unwrap` instead of async `read().await`. **Fix:** ```rust fn spawn_config_watcher(&self) { // Use blocking_read or restructure to be async let configs = self.configs.clone(); let config_rx_future = { let initial = futures::executor::block_on(configs.read()).clone(); self.config_watcher.start_watching(initial) }; // ... } ``` Or better, make `spawn_config_watcher` async and call `self.configs.read().await`. **Effort:** small ### [HIGH] Paradex Connector Missing Reconnect Backoff **File:** crates/optic-connector/src/connectors/paradex/connector.rs:219-220 **Category:** performance **Description:** Fixed 5-second reconnect delay with no exponential backoff. All non-Hyperliquid connectors (Paradex, dYdX, EdgeX, Lighter, Aster) use a fixed 5s delay. **Impact:** Under sustained failures, the connectors hammer the remote servers every 5 seconds indefinitely, potentially triggering rate limits or IP bans. **Root Cause:** Missing exponential backoff logic (Hyperliquid has it, others don't). **Fix:** Add backoff like Hyperliquid does: ```rust let mut reconnect_delay = 5u64; // ... in the reconnect path: tokio::time::sleep(tokio::time::Duration::from_secs(reconnect_delay)).await; reconnect_delay = (reconnect_delay * 2).min(60); // ... on successful connect: reconnect_delay = 5; ``` **Effort:** small (per connector, 5 connectors) ### [HIGH] Auth Bypass When DYNAMIC_ENVIRONMENT_ID is Unset **File:** crates/optic-api/src/middleware/admin_auth.rs:100-105 **Category:** security **Description:** When `DYNAMIC_ENVIRONMENT_ID` is empty or unset, the middleware returns `Ok(next.run(request).await)` — completely bypassing authentication. This is intentional for dev but dangerous if the env var is accidentally missing in production. **Impact:** All admin endpoints (exchange config CRUD, delete) become publicly accessible. **Root Cause:** Silent auth bypass as a default behavior. **Fix:** Log a warning at startup and/or check `ENVIRONMENT`: ```rust if environment_id.is_empty() { let is_prod = std::env::var("ENVIRONMENT") .map(|e| e == "production" || e == "prod") .unwrap_or(false); if is_prod { tracing::error!("DYNAMIC_ENVIRONMENT_ID not set in production - rejecting all admin requests"); return Err(StatusCode::INTERNAL_SERVER_ERROR); } tracing::warn!("Auth disabled - DYNAMIC_ENVIRONMENT_ID not set"); return Ok(next.run(request).await); } ``` **Effort:** small ### [HIGH] JWKS Key Never Refreshed / No Cache Invalidation **File:** crates/optic-api/src/middleware/admin_auth.rs:15,28-70 **Category:** security **Description:** The Dynamic JWKS public key is fetched once and cached forever in a `OnceLock>>`. If the key rotates (which identity providers do periodically), the API will reject all valid tokens. **Impact:** After a key rotation, all admin authentication fails until the API is restarted. **Root Cause:** Permanent caching of a rotatable key with no TTL or refresh mechanism. **Fix:** Add a TTL to the cache: ```rust static DYNAMIC_PUBLIC_KEY: OnceLock, Instant)>> = OnceLock::new(); // In get_public_key: let lock = DYNAMIC_PUBLIC_KEY.get_or_init(|| RwLock::new((None, Instant::now()))); { let read = lock.read().await; if let Some(key) = read.0.as_ref() { if read.1.elapsed() < Duration::from_secs(3600) { return Ok(key.clone()); } } } // ... fetch and store with new timestamp ``` **Effort:** medium ### [HIGH] Funding Rate Multiplied by 100 Twice (Hyperliquid) **File:** crates/optic-connector/src/connectors/hyperliquid/funding.rs:9 **Category:** logic **Description:** `funding_rate = asset_ctx.ctx.funding.parse::()? * 100.0` — the rate is multiplied by 100 here to convert to percentage. But `FundingRateUpdate.funding_rate` is documented/used as a percentage value, and then in `formatter.rs:206` it's multiplied by 100 again: `let rate_pct = rate * 100.0`. This means the logger displays rates 100x too large. **Impact:** Funding rate display is 100x inflated in the logger (e.g., 0.01% shows as 1%). **Root Cause:** Inconsistent convention — some parsers multiply by 100 (Hyperliquid, Paradex, EdgeX, dYdX), while the formatter also multiplies by 100. **Fix:** Either remove the `* 100.0` from all parsers and let the formatter handle it, OR remove it from the formatter. The cleaner approach is to store raw rates (decimal, not percentage) and convert only at display: ```rust // In funding parsers: store raw rate let funding_rate = asset_ctx.ctx.funding.parse::()?; // In formatter: convert to percentage let rate_pct = rate * 100.0; ``` **Effort:** medium (affects all 4 funding parsers) ### [HIGH] dYdX Orderbook Always Uses Incremental, Even for Snapshots **File:** crates/optic-connector/src/connectors/dydx/orderbook.rs:89 **Category:** logic **Description:** `let update_type = UpdateType::Incremental;` is hardcoded regardless of whether this is a `subscribed` (snapshot) or `channel_data` (incremental) message. The `subscribed` message for `v4_orderbook` contains the initial snapshot, which should use `UpdateType::FullSnapshot`. **Impact:** The orderbook cache applies snapshot data as incremental updates, meaning zero-size levels from the snapshot won't clear stale cache entries. This can lead to ghost price levels persisting in the orderbook. **Root Cause:** Missing logic to distinguish snapshot vs incremental based on `msg_type`. **Fix:** ```rust // Pass msg_type into the parse function let update_type = if msg_type == "subscribed" { UpdateType::FullSnapshot } else { UpdateType::Incremental }; ``` **Effort:** small ### [HIGH] Lighter Orderbook Unsubscribe Uses Wrong Channel Format **File:** crates/optic-connector/src/connectors/lighter/connector.rs:470 **Category:** logic **Description:** Subscribe uses `format!("order_book/{}", native_symbol)` but unsubscribe uses `format!("public_market_data/{}", native_symbol)`. These don't match. **Impact:** Unsubscribe requests are silently ignored by the Lighter API, leaving phantom subscriptions that waste bandwidth and processing. **Root Cause:** Copy-paste error or outdated channel name. **Fix:** ```rust let channel = format!("order_book/{}", native_symbol); ``` **Effort:** small --- ## MEDIUM Issues ### [MEDIUM] Unbounded Orderbook Cache Growth **File:** crates/optic-connector/src/state/orderbook.rs:12-16 **Category:** performance **Description:** `ob_cache` is a `DashMap` that grows unboundedly. Each (exchange, symbol) pair accumulates price levels forever as incremental updates add levels. There's no eviction or size limit. **Impact:** Memory leak proportional to total unique price levels seen across all exchange/symbol pairs over the lifetime of the process. **Root Cause:** No cache eviction strategy. **Fix:** After uncrossing, trim to top N levels: ```rust // After uncross, keep only top 500 bids and asks while cached_bids.len() > 500 { cached_bids.pop_first(); } while cached_asks.len() > 500 { cached_asks.pop_last(); } ``` **Effort:** small ### [MEDIUM] Throttle Check Has TOCTOU Race **File:** crates/optic-connector/src/state/orderbook.rs:78-95 **Category:** performance **Description:** The throttle check reads `last_publish`, drops the reference, then inserts. Two concurrent updates for the same key can both pass the threshold check before either inserts, causing duplicate publishes. **Impact:** Occasional duplicate Kafka publishes under high concurrency (mitigated by the DashMap's per-shard locking, but still possible across updates). **Root Cause:** Non-atomic check-and-set on the DashMap. **Fix:** Use `entry` API for atomic check-and-update: ```rust let should_publish = { let threshold = Duration::from_millis(self.throttle_ms); let mut entry = self.last_publish.entry(key.clone()).or_insert(Instant::now()); let last = *entry.value(); if now.duration_since(last) >= threshold { *entry.value_mut() = now; true } else { false } }; ``` **Effort:** small ### [MEDIUM] Logging Filter Identical for Production and Development **File:** crates/optic-core/src/logging.rs:21-31 **Category:** ux **Description:** The `is_production` branch and the `else` branch both set the exact same `EnvFilter`. The production/development distinction is meaningless. **Impact:** No additional debug logging in development mode, making local debugging harder. **Root Cause:** Likely a copy-paste that was never differentiated. **Fix:** Use `debug` level for development: ```rust if is_production { EnvFilter::new("info,hyper=warn,h2=warn,...") } else { EnvFilter::new("debug,hyper=warn,h2=warn,...") } ``` **Effort:** small ### [MEDIUM] `encode_metrics()` Panics on Encoding Failure **File:** crates/optic-connector/src/metrics.rs:64-69 **Category:** error-handling **Description:** Both `encoder.encode(...).unwrap()` and `String::from_utf8(buffer).unwrap()` can panic. If a metric label contains invalid data, this crashes the metrics HTTP handler. **Impact:** A single bad metric label crashes the entire metrics endpoint, making monitoring blind. **Root Cause:** `unwrap()` on encoding operations. **Fix:** ```rust pub fn encode_metrics() -> String { let encoder = TextEncoder::new(); let metric_families = prometheus::gather(); let mut buffer = Vec::new(); if let Err(e) = encoder.encode(&metric_families, &mut buffer) { return format!("# Error encoding metrics: {}", e); } String::from_utf8(buffer).unwrap_or_else(|_| "# Error: invalid UTF-8 in metrics".to_string()) } ``` **Effort:** small ### [MEDIUM] Metrics and Health Server `expect()` in Spawned Task **File:** crates/optic-connector/src/main.rs:80-84 **Category:** error-handling **Description:** `expect("Failed to bind metrics server")` and `expect("Metrics server failed")` inside a `tokio::spawn`. If the metrics port is in use, the spawned task panics, which silently kills the task but doesn't crash the main process — metrics just disappear without notice. **Impact:** Silent loss of metrics endpoint; hard to diagnose in production. **Root Cause:** `expect` in a spawned task doesn't propagate panics to the main thread. **Fix:** ```rust tokio::spawn(async move { let listener = match tokio::net::TcpListener::bind(format!("0.0.0.0:{}", metrics_port)).await { Ok(l) => l, Err(e) => { tracing::error!("Failed to bind metrics server on port {}: {}", metrics_port, e); return; } }; info!("Metrics server listening on port {}", metrics_port); if let Err(e) = axum::serve(listener, app).await { tracing::error!("Metrics server failed: {}", e); } }); ``` **Effort:** small ### [MEDIUM] `ConnectionPool::add_subscriptions` Unwraps `create_connection` **File:** crates/optic-connector/src/websocket_coordinator/connection_pool.rs:80 **Category:** error-handling **Description:** `self.create_connection().unwrap()` panics if the pool is full and no existing connection has capacity. **Impact:** Process crash when all connections are full and more symbols need subscribing. **Root Cause:** `unwrap()` on a fallible operation. **Fix:** ```rust .unwrap_or_else(|| { match self.create_connection() { Ok(id) => (id, limit), Err(e) => { // Return error from add_subscriptions panic!("Pool full: {}", e); // or propagate properly } } }); ``` Better: restructure to return `Err` from `add_subscriptions`: ```rust let (conn_id, available_slots) = self .connections .iter() .find_map(|(id, conn)| { ... }) .map(Ok) .unwrap_or_else(|| { self.create_connection().map(|id| (id, limit)) }) .map_err(|e| e)?; ``` **Effort:** small ### [MEDIUM] dYdX Ping Interval Set to ~3170 Years **File:** crates/optic-connector/src/connectors/dydx/connector.rs:94 **Category:** logic **Description:** `tokio::time::interval(tokio::time::Duration::from_secs(100000000))` — this is ~3.17 years. The ping interval effectively never fires. **Impact:** No keepalive pings are sent, relying entirely on server-side pings. If the server doesn't ping, stale connections won't be detected. **Root Cause:** Likely a debug value that was never reverted, or intentional to rely on server pings. **Fix:** Set a reasonable ping interval (e.g., 30s) or remove the interval if server pings are sufficient: ```rust let mut ping_interval = tokio::time::interval(tokio::time::Duration::from_secs(30)); ``` **Effort:** small ### [MEDIUM] `symbol_sync::sync_symbol_mappings` Panics on Parse Failure **File:** crates/optic-api/src/symbol_sync.rs:21-22 **Category:** error-handling **Description:** `toml::from_str(EMBEDDED_SYMBOL_MAPPINGS).expect("Failed to parse embedded symbol mappings")` — while this is an embedded file so it shouldn't fail, if the TOML is ever malformed during development, the entire API server crashes on startup. **Impact:** Hard crash on startup if symbol mappings TOML is invalid. **Root Cause:** `expect` instead of returning an error. **Fix:** Return an error instead: ```rust let file: SymbolMappingsFile = toml::from_str(EMBEDDED_SYMBOL_MAPPINGS) .map_err(|e| sqlx::Error::Configuration( format!("Failed to parse symbol mappings: {}", e).into() ))?; ``` **Effort:** small ### [MEDIUM] CorsLayer::permissive() in Production **File:** crates/optic-api/src/main.rs:34 **Category:** security **Description:** `CorsLayer::permissive()` allows any origin, any method, any headers. This is fine for dev but overly broad for production. **Impact:** Any website can make authenticated requests to the admin API from a user's browser (if they have a valid JWT). **Root Cause:** No environment-conditional CORS configuration. **Fix:** ```rust let cors = if is_production { CorsLayer::new() .allow_origin(["https://yourdomain.com".parse().unwrap()]) .allow_methods([Method::GET, Method::PUT, Method::DELETE]) .allow_headers([header::AUTHORIZATION, header::CONTENT_TYPE]) } else { CorsLayer::permissive() }; ``` **Effort:** small ### [MEDIUM] Paradex Orderbook Only Processes Inserts, Ignores Deletes/Updates **File:** crates/optic-connector/src/connectors/paradex/orderbook.rs:29-45 **Category:** logic **Description:** The Paradex orderbook parser only iterates `orderbook.inserts` and ignores `orderbook.deletes` and `orderbook.updates`. The types are defined but unused. **Impact:** Deleted price levels persist in the orderbook, and updated levels show stale sizes. The orderbook becomes increasingly inaccurate over time. **Root Cause:** Incomplete implementation — deletes and updates fields exist in the type but aren't processed. **Fix:** ```rust // Process deletes (set size to 0) for entry in &orderbook.deletes { if let Ok(price) = entry.price.parse::() { match entry.side.as_str() { "BUY" => { bids.insert(OrderedFloat(price), OrderBookLevel { price, size: 0.0, .. }); } "SELL" => { asks.insert(OrderedFloat(price), OrderBookLevel { price, size: 0.0, .. }); } _ => {} } } } // Process updates (overwrite) for entry in &orderbook.updates { // Same as inserts } ``` And change `update_type` to `Incremental` for non-snapshot updates. **Effort:** medium ### [MEDIUM] Config Watcher `has_changes` Ignores Connection Config Changes **File:** crates/optic-core/src/config/watcher.rs:56-95 **Category:** logic **Description:** `has_changes()` compares `enabled`, orderbook/funding/trades/candles symbols, but does NOT compare `connection` config (max_subscriptions_per_connection, max_connections, ping intervals) or candle intervals. **Impact:** Changes to connection pool limits or candle intervals require a service restart to take effect. **Root Cause:** Incomplete comparison logic. **Fix:** Add connection config and candle interval comparison: ```rust if old_config.candles.intervals != new_config.candles.intervals { return true; } if old_config.connection.max_subscriptions_per_connection != new_config.connection.max_subscriptions_per_connection || old_config.connection.max_connections != new_config.connection.max_connections { return true; } ``` **Effort:** small --- ## LOW Issues ### [LOW] `ProxyProtocol::from_str` Always Returns Socks5 **File:** crates/optic-connector/src/proxy/pool.rs:142-144 **Category:** logic **Description:** `from_str(_s: &str)` ignores the input and always returns `Socks5`. The `protocol` field in the database is stored but never used. **Impact:** HTTP/HTTPS proxies configured in the database will be connected as SOCKS5, causing connection failures. **Root Cause:** Stub implementation. **Fix:** ```rust pub fn from_str(s: &str) -> Self { match s.to_lowercase().as_str() { "socks5" => Self::Socks5, // Add other variants as needed other => { warn!("Unknown proxy protocol '{}', defaulting to Socks5", other); Self::Socks5 } } } ``` **Effort:** small ### [LOW] `proxy_url()` Always Generates socks5:// Regardless of Protocol **File:** crates/optic-connector/src/proxy/pool.rs:147-151 **Category:** logic **Description:** `proxy_url` hardcodes `"socks5://"` scheme regardless of the `ProxyConfig.protocol` field. **Impact:** Same as above — non-SOCKS5 proxies won't work. **Root Cause:** Hardcoded scheme. **Fix:** Use the protocol enum to determine the scheme. **Effort:** small ### [LOW] `ExchangeConfigRow::from` Casts i32 to usize Without Bounds Check **File:** crates/optic-core/src/config/types.rs:151-152 **Category:** error-handling **Description:** `row.max_subscriptions_per_connection as usize` and `row.max_connections as usize` — if the database contains negative values, this silently wraps to a very large usize on 64-bit platforms. **Impact:** Negative database values would create pools with near-infinite capacity, consuming all memory. **Root Cause:** Unchecked type cast. **Fix:** ```rust max_subscriptions_per_connection: row.max_subscriptions_per_connection.max(0) as usize, max_connections: row.max_connections.max(0) as usize, ``` **Effort:** small ### [LOW] Double Parsing of Hyperliquid Messages **File:** crates/optic-connector/src/connectors/hyperliquid/connector.rs:126,169 **Category:** performance **Description:** The same text message is parsed as `HyperliquidMessage` twice — once at line 126 for subscription responses/pings, and if that path doesn't `continue`, again at line 169 for data messages. **Impact:** Wasted CPU on double JSON deserialization for every data message. **Root Cause:** Two separate `serde_json::from_str` calls instead of reusing the first parse. **Fix:** Store the result of the first parse and reuse it: ```rust if let Ok(msg) = serde_json::from_str::(&text) { // Handle all message types in one block } ``` **Effort:** small ### [LOW] `SymbolMapping` Flattened HashMap Captures `display_name` Key **File:** crates/optic-core/src/config/types.rs:84-88 **Category:** logic **Description:** `#[serde(flatten)] pub exchange_symbols: HashMap` will capture ALL remaining keys including `display_name` (which is already a named field). Serde processes the named field first, so `display_name` is extracted correctly, but if there's a TOML key literally named `display_name` under `exchange_symbols`, it would be silently consumed. **Impact:** Minor — the flatten works correctly in practice since `display_name` is extracted first. **Root Cause:** Serde flatten behavior with overlapping field names. **Fix:** No code change needed, but document the assumption. Alternatively, use a nested `exchanges` key in the TOML. **Effort:** small ### [LOW] Memory Monitor Only Warns, Never Takes Action **File:** crates/optic-connector/src/memory_monitor.rs:49-51 **Category:** ux **Description:** At 95% memory usage, the monitor logs a warning but takes no action. The container will be OOM-killed without any graceful shutdown. **Impact:** Ungraceful shutdown — in-flight Kafka messages may be lost, connections aren't cleanly closed. **Root Cause:** No OOM mitigation strategy. **Fix:** Consider dropping caches (orderbook cache, candle cache) at high memory, or triggering a graceful shutdown: ```rust if memory_percent >= 95.0 { warn!("CRITICAL: Memory at {:.1}%, initiating graceful shutdown", memory_percent); // Signal shutdown std::process::exit(1); } ``` **Effort:** medium ### [LOW] `remove_subscriptions` Only Unsubscribes From First Matching Connection **File:** crates/optic-connector/src/websocket_coordinator/connection_pool.rs:165-198 **Category:** logic **Description:** The loop `break`s after finding the first connection containing the symbols. If the same symbol is subscribed on multiple connections (possible during connection refresh), only one connection gets the unsubscribe. **Impact:** Phantom subscriptions on secondary connections after unsubscribe. **Root Cause:** Early `break` in the loop. **Fix:** Remove the `break` or collect all matching connections first. **Effort:** small ### [LOW] Kafka Publish Uses Duration::from_secs(0) Timeout **File:** crates/optic-core/src/messaging/kafka_client.rs:101 **Category:** performance **Description:** `Duration::from_secs(0)` as the send timeout means the producer will return immediately if the internal queue is full, losing the message. **Impact:** Under Kafka backpressure, messages are silently dropped instead of being retried. **Root Cause:** Zero timeout on the producer send. **Fix:** ```rust Duration::from_secs(5) ``` **Effort:** small --- ## Architecture Notes (Non-Issues) 1. **No graceful shutdown for WebSocket connections** — Ctrl+C kills the process but doesn't send close frames to exchanges. Not a bug, but could cause brief reconnection storms. 2. **Connector code is highly repetitive** — each exchange connector is ~400 lines of largely identical WebSocket management code. A generic WebSocket connector base would reduce the surface area for bugs (like the missing backoff). 3. **No integration tests** — the entire codebase has zero test files. Given the complexity of the state management and WebSocket coordination, this is a significant risk. 4. **`DataLogger` and `formatter` have dead fields** — `OrderBookExchangeData.bid_price`, etc. are always `None` because the logger doesn't read from the orderbook cache. The formatter renders `---` for all price data.