From 8bfd010af58c9224579e8014735f56396b56b232 Mon Sep 17 00:00:00 2001
From: jgrusewski <jgrusewski@localhost>
Date: Mon, 23 Feb 2026 20:01:38 +0100
Subject: [PATCH] fix(trading_service): reject negative/NaN prices in
 fixed-point conversion

Casting negative f64 to u64 saturates to 0, corrupting avg_price and all
downstream PnL calculations. Add price_to_fixed_checked() that rejects
non-finite, zero, and negative prices with PositionError::InvalidPrice.
The critical update_with_execution path now uses the checked variant.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 .../src/core/position_manager.rs              | 77 ++++++++++++++++++-
 1 file changed, 75 insertions(+), 2 deletions(-)

diff --git a/services/trading_service/src/core/position_manager.rs b/services/trading_service/src/core/position_manager.rs
index 9a61c6817..0aba55309 100644
--- a/services/trading_service/src/core/position_manager.rs
+++ b/services/trading_service/src/core/position_manager.rs
@@ -95,7 +95,7 @@ impl AtomicPosition {
         timestamp_ns: u64,
         sequence: u64,
     ) -> Result<PositionUpdate, PositionError> {
-        let execution_price_fixed = Self::price_to_fixed(execution_price);
+        let execution_price_fixed = Self::price_to_fixed_checked(execution_price)?;
 
         // Atomic updates with proper ordering
         let old_quantity = self.quantity.load(Ordering::Acquire);
@@ -251,8 +251,23 @@ impl AtomicPosition {
     }
 
     // Helper functions for fixed-point arithmetic
+
+    /// Checked fixed-point conversion: rejects NaN, Infinity, zero, and negative prices.
+    /// Financial instruments must have a strictly positive price.
+    pub fn price_to_fixed_checked(price: f64) -> Result<u64, PositionError> {
+        if !price.is_finite() {
+            return Err(PositionError::InvalidPrice);
+        }
+        if price <= 0.0 {
+            return Err(PositionError::InvalidPrice);
+        }
+        Ok((price * 10000.0) as u64)
+    }
+
+    /// Backward-compatible unchecked conversion. Delegates to the checked variant,
+    /// returning 0 on invalid input (preserves legacy behaviour for non-critical paths).
     fn price_to_fixed(price: f64) -> u64 {
-        (price * 10000.0) as u64 // 4 decimal places
+        Self::price_to_fixed_checked(price).unwrap_or(0)
     }
 
     fn fixed_to_price(fixed: u64) -> f64 {
@@ -1024,6 +1039,64 @@ mod tests {
         assert!(pnl.total_pnl > 0.0); // Should be positive overall
     }
 
+    #[test]
+    fn test_price_to_fixed_checked_rejects_negative_nan_inf_zero() {
+        // Negative price must be rejected
+        let result = AtomicPosition::price_to_fixed_checked(-1.0);
+        assert!(result.is_err(), "Negative price should be rejected");
+
+        // Zero price must be rejected
+        let result = AtomicPosition::price_to_fixed_checked(0.0);
+        assert!(result.is_err(), "Zero price should be rejected");
+
+        // NaN must be rejected
+        let result = AtomicPosition::price_to_fixed_checked(f64::NAN);
+        assert!(result.is_err(), "NaN price should be rejected");
+
+        // Positive infinity must be rejected
+        let result = AtomicPosition::price_to_fixed_checked(f64::INFINITY);
+        assert!(result.is_err(), "Positive infinity should be rejected");
+
+        // Negative infinity must be rejected
+        let result = AtomicPosition::price_to_fixed_checked(f64::NEG_INFINITY);
+        assert!(result.is_err(), "Negative infinity should be rejected");
+
+        // Valid positive price must succeed
+        let result = AtomicPosition::price_to_fixed_checked(100.5);
+        assert!(result.is_ok(), "Valid positive price should succeed");
+        assert_eq!(result.expect("valid price"), 1005000);
+
+        // Very small positive price must succeed
+        let result = AtomicPosition::price_to_fixed_checked(0.0001);
+        assert!(result.is_ok(), "Small positive price should succeed");
+        assert_eq!(result.expect("valid small price"), 1);
+    }
+
+    #[tokio::test]
+    async fn test_update_with_execution_rejects_invalid_prices() {
+        let position = AtomicPosition::new(0x123, 0x456);
+
+        // Negative execution price must be rejected
+        let result = position.update_with_execution(-100, -50.0, 1000, 1);
+        assert!(result.is_err(), "Negative execution price should be rejected");
+
+        // NaN execution price must be rejected
+        let result = position.update_with_execution(100, f64::NAN, 1000, 1);
+        assert!(result.is_err(), "NaN execution price should be rejected");
+
+        // Infinity execution price must be rejected
+        let result = position.update_with_execution(100, f64::INFINITY, 1000, 1);
+        assert!(result.is_err(), "Infinity execution price should be rejected");
+
+        // Zero execution price must be rejected
+        let result = position.update_with_execution(100, 0.0, 1000, 1);
+        assert!(result.is_err(), "Zero execution price should be rejected");
+
+        // Valid price must still work
+        let result = position.update_with_execution(100, 50000.0, 1000, 1);
+        assert!(result.is_ok(), "Valid price should succeed");
+    }
+
     #[tokio::test]
     async fn test_atomic_position_operations() {
         let position = AtomicPosition::new(0x123, 0x456);