gitadmin/foxhunt
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(trading_engine): add OrderSide to ExecutionResult, fix always-buy direction

Browse Source 
ExecutionResult.executed_quantity is always a positive magnitude, so the
old `is_buy = executed_quantity > ZERO` check was always true — every
fill was treated as a buy regardless of order side.

Add an explicit `pub side: OrderSide` field to ExecutionResult and use
`execution.side == OrderSide::Buy` in PositionManager.  All construction
sites (source, tests, benchmarks) updated; sell-side tests now use
positive quantities with `side: OrderSide::Sell` instead of the former
negative-quantity hack.

Addresses audit item H4 (position direction always buy).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
jgrusewski
2026-02-23 20:45:59 +01:00
parent f7b259cb34
commit 3cbf643b4d
14 changed files with 70 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
trading_engine/benches/comprehensive_performance.rs
View File

@@ -306,6 +306,7 @@ fn create_test_execution(order_id: u64) -> ExecutionResult {
ExecutionResult {
order_id: OrderId::new(),
symbol: "BTC-USD".to_string(),
side: OrderSide::Buy,
executed_quantity: Decimal::new(100, 2),
execution_price: Decimal::new(65000, 0),
execution_time: chrono::Utc::now(),

1
trading_engine/benches/e2e_latency.rs
View File

@@ -193,6 +193,7 @@ fn create_test_execution(order_id: u64) -> ExecutionResult {
ExecutionResult {
order_id: common::OrderId::new(),
symbol: "BTC-USD".to_string(),
side: common::OrderSide::Buy,
executed_quantity: Decimal::new(1, 2),
execution_price: Decimal::new(65000, 0),
execution_time: chrono::Utc::now(),

1
trading_engine/benches/e2e_performance.rs
View File

@@ -145,6 +145,7 @@ fn create_test_execution(order_id: u64) -> ExecutionResult {
ExecutionResult {
order_id: common::OrderId::new(),
symbol: format!("TEST{}", order_id % 100),
side: common::OrderSide::Buy,
executed_quantity: Decimal::new(100, 0),
execution_price: Decimal::new(15000, 2),
execution_time: chrono::Utc::now(),

3
trading_engine/src/trading/order_manager.rs
View File

@@ -462,6 +462,7 @@ mod tests {
let execution1 = ExecutionResult {
order_id: order.id.clone(),
symbol: "BTCUSD".to_string(),
side: common::OrderSide::Buy,
executed_quantity: Decimal::from(30),
execution_price: Decimal::from(50000),
execution_time: Utc::now(),
@@ -484,6 +485,7 @@ mod tests {
let execution2 = ExecutionResult {
order_id: order.id.clone(),
symbol: "BTCUSD".to_string(),
side: common::OrderSide::Buy,
executed_quantity: Decimal::from(70),
execution_price: Decimal::from(50100),
execution_time: Utc::now(),
@@ -682,6 +684,7 @@ mod tests {
let execution = ExecutionResult {
order_id: "nonexistent".to_string().into(),
symbol: "BTCUSD".to_string(),
side: common::OrderSide::Buy,
executed_quantity: Decimal::from(100),
execution_price: Decimal::from(50000),
execution_time: Utc::now(),

18
trading_engine/src/trading/position_manager.rs
View File

@@ -61,9 +61,8 @@ impl PositionManager {
}
});
// Determine if this is a buy or sell based on the original order
// For now, we'll infer from the execution direction
let is_buy = execution.executed_quantity > Decimal::ZERO;
// Determine if this is a buy or sell from the explicit side field
let is_buy = execution.side == common::OrderSide::Buy;
let old_quantity = position.quantity;
let old_cost = position.avg_cost;
@@ -112,8 +111,8 @@ impl PositionManager {
}
}
} else {
// Decreasing position (sell) - execution_quantity is negative, so we use abs()
let exec_qty_decimal = execution.executed_quantity.abs();
// Decreasing position (sell) - executed_quantity is always positive magnitude
let exec_qty_decimal = execution.executed_quantity;
let exec_price_decimal = execution.execution_price;
let old_qty_decimal = old_quantity;
let old_cost_decimal = old_cost;
@@ -396,8 +395,9 @@ pub struct PositionStats {
mod tests {
use super::*;
use crate::trading_operations::LiquidityFlag;
use common::OrderSide;
/// Create a buy execution (positive quantity)
/// Create a buy execution (positive quantity, side = Buy)
fn create_buy_execution(
order_id: &str,
symbol: &str,
@@ -407,6 +407,7 @@ mod tests {
ExecutionResult {
order_id: order_id.to_string().into(),
symbol: symbol.to_string(),
side: OrderSide::Buy,
executed_quantity: Decimal::from(quantity.abs()),
execution_price: Decimal::from(price),
execution_time: Utc::now(),
@@ -415,7 +416,7 @@ mod tests {
}
}
/// Create a sell execution (negative quantity to indicate sell direction)
/// Create a sell execution (positive quantity, side = Sell)
fn create_sell_execution(
order_id: &str,
symbol: &str,
@@ -425,7 +426,8 @@ mod tests {
ExecutionResult {
order_id: order_id.to_string().into(),
symbol: symbol.to_string(),
executed_quantity: Decimal::from(-quantity.abs()),
side: OrderSide::Sell,
executed_quantity: Decimal::from(quantity.abs()),
execution_price: Decimal::from(price),
execution_time: Utc::now(),
commission: Decimal::ZERO,

5
trading_engine/src/trading_operations.rs
View File

@@ -348,7 +348,9 @@ pub struct ExecutionResult {
pub order_id: OrderId,
/// Symbol
pub symbol: String,
/// Executed Quantity
/// Side (buy or sell) of the execution
pub side: OrderSide,
/// Executed Quantity (always positive magnitude)
pub executed_quantity: Decimal,
/// Execution Price
pub execution_price: Decimal,
@@ -941,6 +943,7 @@ mod tests {
let execution = ExecutionResult {
order_id: OrderId::from("test-002"),
symbol: "ETHUSD".to_string(),
side: OrderSide::Sell,
executed_quantity: Decimal::from(5),
execution_price: Decimal::from(3005),
execution_time: Utc::now(),

1
trading_engine/tests/advanced_order_types_tests.rs
View File

@@ -113,6 +113,7 @@ fn create_execution(
ExecutionResult {
order_id,
symbol: symbol.to_string(),
side: OrderSide::Buy,
executed_quantity: quantity,
execution_price: price,
execution_time: Utc::now(),

7
trading_engine/tests/concurrency_edge_cases.rs
View File

@@ -50,11 +50,8 @@ fn create_test_execution(
ExecutionResult {
order_id: OrderId::new(),
symbol: symbol.to_string(),
executed_quantity: if side == OrderSide::Buy {
quantity
} else {
-quantity
},
side,
executed_quantity: quantity,
execution_price: price,
commission: Decimal::from_str("0.01").unwrap(),
execution_time: Utc::now(),

1
trading_engine/tests/core_integration_tests.rs
View File

@@ -182,6 +182,7 @@ fn create_test_execution(symbol: &str, quantity: f64, price: f64) -> ExecutionRe
ExecutionResult {
order_id: OrderId::new(),
symbol: symbol.to_owned(),
side: OrderSide::Buy,
executed_quantity: Decimal::from_str(&quantity.to_string()).unwrap(),
execution_price: Decimal::from_str(&price.to_string()).unwrap(),
execution_time: chrono::Utc::now(),

25
trading_engine/tests/engine_execution_flow_tests.rs
View File

@@ -81,6 +81,7 @@ fn create_execution(order_id: OrderId, symbol: &str, qty: i64, price: i64) -> Ex
ExecutionResult {
order_id,
symbol: symbol.to_string(),
side: OrderSide::Buy,
executed_quantity: Decimal::from(qty),
execution_price: Decimal::from(price),
execution_time: Utc::now(),
@@ -228,6 +229,7 @@ async fn test_process_execution_updates_account() {
let execution = ExecutionResult {
order_id,
symbol: "BTCUSD".to_string(),
side: OrderSide::Buy,
executed_quantity: Decimal::from(1),
execution_price: Decimal::from(50000),
execution_time: Utc::now(),
@@ -242,10 +244,10 @@ async fn test_process_execution_updates_account() {
.await
.unwrap();
// Commission should be deducted
// Buy: cash -= execution_value + commission = 50000 + 25 = 50025
assert_eq!(
updated_account.cash_balance,
initial_cash - Decimal::from(25)
initial_cash - Decimal::from(50000) - Decimal::from(25)
);
}
@@ -295,6 +297,7 @@ async fn test_process_execution_with_commission() {
let execution = ExecutionResult {
order_id,
symbol: "SOLUSD".to_string(),
side: OrderSide::Buy,
executed_quantity: Decimal::from(1000),
execution_price: Decimal::from(100),
execution_time: Utc::now(),
@@ -304,11 +307,12 @@ async fn test_process_execution_with_commission() {
engine.process_execution(execution).await.unwrap();
// Verify commission was processed
// Verify execution value + commission was deducted
let account = engine.get_account_info("DEMO_ACCOUNT".to_string()).await.unwrap();
// Initial cash 50000 - commission 50 = 49950
assert_eq!(account.cash_balance, Decimal::from(50000) - Decimal::from(50));
// Buy: cash -= (1000 * 100) + 50 = 100050
// Initial cash 50000 - 100050 = -50050
assert_eq!(account.cash_balance, Decimal::from(50000) - Decimal::from(100050));
}
#[tokio::test]
@@ -325,7 +329,8 @@ async fn test_process_buy_then_sell_execution() {
let sell_exec = ExecutionResult {
order_id: sell_order_id,
symbol: "ETHUSD".to_string(),
executed_quantity: Decimal::from(-60), // Negative for sell
side: OrderSide::Sell,
executed_quantity: Decimal::from(60),
execution_price: Decimal::from(3100),
execution_time: Utc::now(),
commission: Decimal::from(10),
@@ -352,7 +357,8 @@ async fn test_process_execution_flatten_position() {
let sell_exec = ExecutionResult {
order_id: sell_order_id,
symbol: "SOLUSD".to_string(),
executed_quantity: Decimal::from(-50),
side: OrderSide::Sell,
executed_quantity: Decimal::from(50),
execution_price: Decimal::from(110),
execution_time: Utc::now(),
commission: Decimal::from(5),
@@ -551,6 +557,7 @@ async fn test_fractional_execution_quantity() {
let execution = ExecutionResult {
order_id,
symbol: "BTCUSD".to_string(),
side: OrderSide::Buy,
executed_quantity: Decimal::new(15, 1), // 1.5
execution_price: Decimal::from(50000),
execution_time: Utc::now(),
@@ -573,6 +580,7 @@ async fn test_execution_with_high_commission() {
let execution = ExecutionResult {
order_id,
symbol: "BTCUSD".to_string(),
side: OrderSide::Buy,
executed_quantity: Decimal::from(1),
execution_price: Decimal::from(50000),
execution_time: Utc::now(),
@@ -583,6 +591,7 @@ async fn test_execution_with_high_commission() {
let result = engine.process_execution(execution).await;
assert!(result.is_ok());
// Buy: cash -= (1 * 50000) + 5000 = 55000
let account = engine.get_account_info("DEMO_ACCOUNT".to_string()).await.unwrap();
assert_eq!(account.cash_balance, Decimal::from(50000) - Decimal::from(5000));
assert_eq!(account.cash_balance, Decimal::from(50000) - Decimal::from(55000));
}

23
trading_engine/tests/manager_edge_cases.rs
View File

@@ -55,6 +55,7 @@ async fn test_order_manager_execution_overfill() {
let execution = ExecutionResult {
order_id: order.id,
symbol: "BTCUSD".to_string(),
side: OrderSide::Buy,
executed_quantity: Decimal::from(150), // More than 100 ordered
execution_price: Decimal::from(50000),
execution_time: Utc::now(),
@@ -91,6 +92,7 @@ async fn test_order_manager_multiple_partial_fills() {
let execution = ExecutionResult {
order_id: order.id,
symbol: "BTCUSD".to_string(),
side: OrderSide::Buy,
executed_quantity: Decimal::from(qty),
execution_price: Decimal::from(price),
execution_time: Utc::now(),
@@ -172,6 +174,7 @@ async fn test_position_manager_flip_from_long_to_short() {
let buy_exec = ExecutionResult {
order_id: OrderId::new(),
symbol: "BTCUSD".to_string(),
side: OrderSide::Buy,
executed_quantity: Decimal::from(100),
execution_price: Decimal::from(50000),
execution_time: Utc::now(),
@@ -185,7 +188,8 @@ async fn test_position_manager_flip_from_long_to_short() {
let sell_exec = ExecutionResult {
order_id: OrderId::new(),
symbol: "BTCUSD".to_string(),
executed_quantity: Decimal::from(-150), // Sell 150
side: OrderSide::Sell,
executed_quantity: Decimal::from(150),
execution_price: Decimal::from(51000),
execution_time: Utc::now(),
commission: Decimal::from(15),
@@ -211,7 +215,8 @@ async fn test_position_manager_flip_from_short_to_long() {
let sell_exec = ExecutionResult {
order_id: OrderId::new(),
symbol: "ETHUSD".to_string(),
executed_quantity: Decimal::from(-100),
side: OrderSide::Sell,
executed_quantity: Decimal::from(100),
execution_price: Decimal::from(3000),
execution_time: Utc::now(),
commission: Decimal::from(10),
@@ -224,6 +229,7 @@ async fn test_position_manager_flip_from_short_to_long() {
let buy_exec = ExecutionResult {
order_id: OrderId::new(),
symbol: "ETHUSD".to_string(),
side: OrderSide::Buy,
executed_quantity: Decimal::from(150),
execution_price: Decimal::from(2950),
execution_time: Utc::now(),
@@ -274,6 +280,7 @@ async fn test_position_manager_concentration_risk_calculation() {
let exec = ExecutionResult {
order_id: OrderId::new(),
symbol: symbol.to_string(),
side: OrderSide::Buy,
executed_quantity: Decimal::from(qty),
execution_price: Decimal::from(price),
execution_time: Utc::now(),
@@ -384,6 +391,7 @@ async fn test_account_manager_update_from_execution_buy() {
let execution = ExecutionResult {
order_id: OrderId::new(),
symbol: "BTCUSD".to_string(),
side: OrderSide::Buy,
executed_quantity: Decimal::from(1),
execution_price: Decimal::from(50000),
execution_time: Utc::now(),
@@ -394,11 +402,14 @@ async fn test_account_manager_update_from_execution_buy() {
let result = manager.update_from_execution(&execution).await;
assert!(result.is_ok());
let _account = manager.get_account_info("DEMO_ACCOUNT").await.unwrap();
let account = manager.get_account_info("DEMO_ACCOUNT").await.unwrap();
// Cash should decrease by execution value + commission
// Initial: $50,000 - ($50,000 + $25) = -$25 (overdraft)
// But we need to check the actual implementation
// Buy: cash -= (1 * 50000) + 25 = 50025
// Initial: $50,000 - $50,025 = -$25
assert_eq!(
account.cash_balance,
Decimal::from(50000) - Decimal::from(50025)
);
}
#[tokio::test]

1
trading_engine/tests/order_matching_tests.rs
View File

@@ -57,6 +57,7 @@ fn create_execution(
ExecutionResult {
order_id,
symbol: symbol.to_string(),
side: OrderSide::Buy,
executed_quantity: Decimal::from(quantity),
execution_price: Decimal::from(price),
execution_time: Utc::now(),

7
trading_engine/tests/position_manager_comprehensive.rs
View File

@@ -24,11 +24,8 @@ fn create_test_execution(
ExecutionResult {
order_id: OrderId::new(),
symbol,
executed_quantity: if side == OrderSide::Buy {
quantity
} else {
-quantity
},
side,
executed_quantity: quantity,
execution_price: price,
commission: Decimal::from_str("0.01").unwrap(),
execution_time: Utc::now(),

14
trading_engine/tests/trading_engine_integration_tests.rs
View File

@@ -70,6 +70,7 @@ fn create_execution(
ExecutionResult {
order_id,
symbol: symbol.to_string(),
side: OrderSide::Buy,
executed_quantity: Decimal::from(quantity),
execution_price: Decimal::from(price),
execution_time: Utc::now(),
@@ -572,6 +573,7 @@ async fn test_execution_updates_account_balance() {
let execution = ExecutionResult {
order_id,
symbol: "BTCUSD".to_string(),
side: OrderSide::Buy,
executed_quantity: Decimal::from(1),
execution_price: Decimal::from(50000),
execution_time: Utc::now(),
@@ -581,16 +583,16 @@ async fn test_execution_updates_account_balance() {
engine.process_execution(execution).await.unwrap();
// Account should be updated (commission deducted)
// Account should be updated (execution value + commission deducted)
let account = engine
.get_account_info("DEMO_ACCOUNT".to_string())
.await
.unwrap();
// Cash balance should be reduced by commission
// Buy: cash -= (1 * 50000) + 25 = 50025
assert_eq!(
account.cash_balance,
Decimal::from(50000) - Decimal::from(25)
Decimal::from(50000) - Decimal::from(50025)
);
}
@@ -883,7 +885,8 @@ async fn test_position_reduces_with_sell() {
let sell_execution = ExecutionResult {
order_id: sell_order_id,
symbol: "ETHUSD".to_string(),
executed_quantity: Decimal::from(-6), // Negative for sell
side: OrderSide::Sell,
executed_quantity: Decimal::from(6),
execution_price: Decimal::from(3100),
execution_time: Utc::now(),
commission: Decimal::from(10),
@@ -940,7 +943,8 @@ async fn test_position_flattened_with_equal_sell() {
let sell_execution = ExecutionResult {
order_id: sell_order_id,
symbol: "SOLUSD".to_string(),
executed_quantity: Decimal::from(-10),
side: OrderSide::Sell,
executed_quantity: Decimal::from(10),
execution_price: Decimal::from(110),
execution_time: Utc::now(),
commission: Decimal::from(5),