Fix float_to_wire returning "-0" - #313
Open
BhariGowda wants to merge 1 commit into
Open
Conversation
The guard that was meant to turn negative zero into "0" compares the
pre-normalization string, which is always fixed to 8 decimal places
("-0.00000000"), so it never matched and was dead code. The "-0" only
appears after Decimal.normalize(), so the check has to run on the
normalized output.
Reproduces on master:
>>> float_to_wire(-0.0)
'-0'
>>> float_to_wire(-1e-13)
'-0'
Both are reachable from ordinary caller arithmetic, for example
round(-1e-9, 2) evaluates to -0.0, which then goes out on the wire as an
order size or price of "-0".
Moved the check after normalize() and added a test covering negative
zero, plain zero and a couple of ordinary values so the sign handling
does not regress.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
float_to_wirehas a guard meant to turn negative zero into"0", but it compares the pre-normalization string. That string is always fixed to 8 decimal places, so it is"-0.00000000"and never equals"-0". The"-0"only appears afterDecimal.normalize(), which runs afterwards.On master:
float_to_wireproduces thepandsfields of every order wire and thetriggerPxof every trigger order, so this goes out on the wire as an order price or size of"-0".It is reachable from ordinary caller arithmetic.
round(-1e-9, 2)evaluates to-0.0in Python, so sizing code of the shapesz = round(target - current, 4)hits it whenever the difference is a small negative number.The same function exists in
hyperliquid-rust-sdkasfloat_to_string_for_hashing(src/helpers.rs). It is the same algorithm, format to 8 decimals then strip trailing zeros, except that it applies the"-0"check after the stripping rather than before:and it pins the result with assertions:
Running all 13 assertions from that test against
float_to_wire, master passes 11 and fails exactly the two-0.cases. With this change it passes 13 of 13.The Python guard has never fired.
f"{x:.8f}"always emits 8 fractional digits, so it cannot produce the two character string"-0"for any input:-0.0,-1e-300and-1e-13all format to"-0.00000000", andnan,infand-infformat to"nan","inf"and"-inf". It has sat in this position since it was introduced in 36dac26 (0.1.21), so this is dead code rather than a regression.The fix moves the check to after
normalize(), where the value it is looking for actually exists. Added a test covering negative zero, plain zero, and a couple of ordinary signed values so the sign handling does not regress.Ran
pytest tests/signing_test.py(14 passed) plus black, isort and flake8 with the repo's configured settings.