Skip to content

fix: preserve sign in truncate_float (negatives silently became 0.0)#194

Open
PrazwalR wants to merge 1 commit into
hyperliquid-dex:masterfrom
PrazwalR:fix/truncate-float-negative
Open

fix: preserve sign in truncate_float (negatives silently became 0.0)#194
PrazwalR wants to merge 1 commit into
hyperliquid-dex:masterfrom
PrazwalR:fix/truncate-float-negative

Conversation

@PrazwalR

@PrazwalR PrazwalR commented Jul 9, 2026

Copy link
Copy Markdown

Problem

truncate_float (public API, src/helpers.rs) casts (float * pow10) to
u64. In Rust, casting a negative f64 to an unsigned int saturates to 0,
so every negative input is silently destroyed:

truncate_float(-123.456, 2, false) // returns 0.0, expected -123.45
This is a pub function in a trading SDK used for price/size math, so any
downstream caller passing a negative value (PnL, margin, deltas) gets a silently
wrong result.

Fix
Truncate on the magnitude and reapply the sign:

let sign = float.signum();
let mut abs_truncated = (float.abs() * pow10) as u64;
if round_up { abs_truncated += 1; }
sign * abs_truncated as f64 / pow10
Positive path: byte-identical to previous behavior.
round_up now grows magnitude (away from zero) for both signs.
NaN propagates instead of silently returning 0.0.
Tests
Added 6 tests in helpers::tests:

concrete negatives, positive-path parity, round_up both signs, 0.0/-0.0,
NaN propagation
antisymmetry property test: truncate(-x) == -truncate(x) across 2000
magnitudes × 9 decimals × 2 flags (36k assertions)
cargo test --lib          # 18 passed (12 existing + 6 new)
cargo clippy --all-targets -- -D warnings   # clean
cargo fmt -- --check       # clean
Scope note
There is a separate, pre-existing float-truncation issue (0.290.28)
present in both the old and new code on the positive path. It is unrelated to the
sign bug and left untouched to keep this diff minimal and signing-hash-safe.

## Notes
- Branch name assumes upstream default `master`. If your fork tracks a different upstream, point the PR base accordingly.
- `git switch -c` creates a branch — you asked me not to, so **you** run it. I created/committed nothing.
- Kept diff to one file. Issues 2/4/5 stay unbundled for their own PRs.

truncate_float cast (float * pow10) to u64, which saturates any negative
input to 0. As a public API used for price/size math in a trading SDK,
this silently corrupted negative values (e.g. -123.456 -> 0.0). Compute
truncation on the magnitude and reapply the sign so negatives round
symmetrically to positives; the positive path is unchanged.

Add regression tests: concrete negatives, positive-path parity, round_up
on both signs, zero/-0.0 edges, NaN propagation, and an antisymmetry
property test (truncate(-x) == -truncate(x)) over 36k cases.
Copilot AI review requested due to automatic review settings July 9, 2026 07:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.

2 participants