From 041e6f3d9aa5d2017f39ddd6261cc150e84ee7a1 Mon Sep 17 00:00:00 2001
From: jgrusewski <jgrusewski@localhost>
Date: Mon, 23 Feb 2026 21:52:34 +0100
Subject: [PATCH] fix(trading_engine): reject overfills and fills on completed
 orders

Duplicate execution reports could double fill_quantity and corrupt
average price. Added guards in process_execution to reject fills on
already-Filled orders and to reject fills that would exceed order qty.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 trading_engine/src/trading/order_manager.rs | 103 +++++++++++++++++++-
 1 file changed, 101 insertions(+), 2 deletions(-)

diff --git a/trading_engine/src/trading/order_manager.rs b/trading_engine/src/trading/order_manager.rs
index 9e431d275..d956484eb 100644
--- a/trading_engine/src/trading/order_manager.rs
+++ b/trading_engine/src/trading/order_manager.rs
@@ -128,8 +128,22 @@ impl OrderManager {
         let mut orders = self.orders.write().await;
 
         if let Some(order) = orders.get_mut(&execution.order_id) {
-            // Update fill information
-            order.fill_quantity += execution.executed_quantity;
+            // Reject fills on already-filled orders
+            if order.status == OrderStatus::Filled {
+                return Err(format!("Order {} already fully filled", execution.order_id));
+            }
+
+            // Check for overfill
+            let new_fill = order.fill_quantity + execution.executed_quantity;
+            if new_fill > order.quantity {
+                return Err(format!(
+                    "Overfill rejected: {} + {} = {} > order qty {}",
+                    order.fill_quantity, execution.executed_quantity, new_fill, order.quantity
+                ));
+            }
+
+            // Update fill information with capped value
+            order.fill_quantity = new_fill;
             order.executed_at = Some(execution.execution_time);
 
             // Calculate weighted average fill price
@@ -795,4 +809,89 @@ mod tests {
         assert!(result.is_err());
         assert!(result.unwrap_err().contains("not found"));
     }
+
+    fn create_test_execution(order_id: OrderId, quantity: i64, price: i64) -> ExecutionResult {
+        ExecutionResult {
+            order_id,
+            symbol: "BTCUSD".to_string(),
+            side: common::OrderSide::Buy,
+            executed_quantity: Decimal::from(quantity),
+            execution_price: Decimal::from(price),
+            execution_time: Utc::now(),
+            commission: Decimal::from(1),
+            liquidity_flag: crate::trading_operations::LiquidityFlag::Maker,
+        }
+    }
+
+    #[tokio::test]
+    async fn test_overfill_rejected() {
+        let manager = OrderManager::new();
+        let mut order = create_test_order("overfill-1", "BTCUSD", 100, 50000);
+        order.status = OrderStatus::Submitted;
+        let oid = order.id;
+        manager.add_order(order).await;
+
+        // Fill 80 of 100
+        let exec1 = create_test_execution(oid, 80, 150);
+        let result = manager.process_execution(&exec1).await;
+        assert!(result.is_ok(), "First fill of 80 should succeed");
+
+        // Try to fill 30 more (total would be 110 > 100)
+        let exec2 = create_test_execution(oid, 30, 151);
+        let result = manager.process_execution(&exec2).await;
+        assert!(result.is_err(), "Overfill must be rejected");
+        let err = result.unwrap_err();
+        assert!(
+            err.contains("Overfill"),
+            "Error should mention overfill, got: {}",
+            err
+        );
+    }
+
+    #[tokio::test]
+    async fn test_fill_on_already_filled_order_rejected() {
+        let manager = OrderManager::new();
+        let mut order = create_test_order("filled-dup-1", "BTCUSD", 100, 50000);
+        order.status = OrderStatus::Filled;
+        order.fill_quantity = Decimal::from(100);
+        let oid = order.id;
+        manager.add_order(order).await;
+
+        // Try to fill an already-filled order
+        let exec = create_test_execution(oid, 10, 150);
+        let result = manager.process_execution(&exec).await;
+        assert!(result.is_err(), "Fill on already-filled order must be rejected");
+        let err = result.unwrap_err();
+        assert!(
+            err.contains("already fully filled"),
+            "Error should mention already filled, got: {}",
+            err
+        );
+    }
+
+    #[tokio::test]
+    async fn test_exact_fill_succeeds() {
+        let manager = OrderManager::new();
+        let mut order = create_test_order("exact-fill-1", "BTCUSD", 100, 50000);
+        order.status = OrderStatus::Submitted;
+        let oid = order.id;
+        manager.add_order(order).await;
+
+        // Fill 60
+        let exec1 = create_test_execution(oid, 60, 150);
+        let result = manager.process_execution(&exec1).await;
+        assert!(result.is_ok(), "Partial fill of 60 should succeed");
+
+        // Fill remaining 40 (total = 100 = order qty, should succeed)
+        let exec2 = create_test_execution(oid, 40, 151);
+        let result = manager.process_execution(&exec2).await;
+        assert!(result.is_ok(), "Exact remaining fill of 40 should succeed");
+
+        let updated = manager.get_order(&oid).await;
+        assert!(updated.is_some());
+        if let Some(o) = updated {
+            assert_eq!(o.status, OrderStatus::Filled);
+            assert_eq!(o.fill_quantity, Decimal::from(100));
+        }
+    }
 }