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