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>
This commit is contained in:
@@ -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));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user