Skip to content

Comments

Fix panic when sorting prices containing NaN in test#94

Closed
aviu16 wants to merge 1 commit intoflowsurface-rs:mainfrom
aviu16:fix-nan-panic-in-test
Closed

Fix panic when sorting prices containing NaN in test#94
aviu16 wants to merge 1 commit intoflowsurface-rs:mainfrom
aviu16:fix-nan-panic-in-test

Conversation

@aviu16
Copy link

@aviu16 aviu16 commented Feb 15, 2026

Summary

Fixes a potential panic in the smallest_positive_gap test helper function when input prices contain NaN values.

Bug Description

In exchange/src/adapter/hyperliquid.rs, the smallest_positive_gap function panics when sorting prices that contain NaN:

fn smallest_positive_gap(mut prices: Vec<f32>) -> Option<f32> {
    prices.sort_by(|a, b| b.partial_cmp(a).unwrap());  // Panics if a or b is NaN!
    // ...
}

The partial_cmp method returns None when comparing NaN values (per IEEE 754 floating point spec), causing .unwrap() to panic.

Root Cause

Using .unwrap() on partial_cmp without handling the NaN case.

Fix

Changed to .unwrap_or(std::cmp::Ordering::Equal) to treat NaN values as equal during sorting:

prices.sort_by(|a, b| b.partial_cmp(a).unwrap_or(std::cmp::Ordering::Equal));

This prevents the panic while maintaining correct sort behavior for valid numbers.

Impact

  • This is currently only used in test code (#[cfg(test)] module)
  • Makes the code more defensive against unexpected NaN inputs
  • No change in behavior for valid floating point inputs

Testing

The fix ensures that if test data ever contains NaN values (e.g., from invalid price feeds or calculation errors), the test helper won't panic.

The smallest_positive_gap test helper panics when the input prices contain
NaN values, because partial_cmp returns None for NaN comparisons.

Changed .unwrap() to .unwrap_or(Ordering::Equal) so NaN values are treated
as equal during sorting, preventing the panic.

While this is currently only used in test code, using unwrap_or makes the
code more defensive against unexpected NaN inputs.
@aviu16 aviu16 force-pushed the fix-nan-panic-in-test branch from 1d15f51 to dc7cced Compare February 15, 2026 23:44
@aviu16
Copy link
Author

aviu16 commented Feb 16, 2026

closing this for now, gonna revisit later

@aviu16 aviu16 closed this Feb 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant