Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SafeMath fixes #6431

Merged
merged 4 commits into from
Jan 30, 2025
Merged

SafeMath fixes #6431

merged 4 commits into from
Jan 30, 2025

Conversation

christianbaroni
Copy link
Member

@christianbaroni christianbaroni commented Jan 30, 2025

What changed (plus any additional context for devs)

  • Fixes handling of scientific notation in the SafeMath functions
  • Fixes rounding of negative numbers (up = away from zero)
  • Adds new test cases that failed previously, primarily due to the two issues above
  • Fixes the divWithExp result which was previously incorrect
  • Fixes the chart price formatting bug where numbers would be chopped at e.g., 0.000, as a result of these bugs

With the updated tests, but without the SafeMath fixes:

Screenshot 2025-01-30 at 12 33 33 AM Screenshot 2025-01-30 at 12 34 34 AM

With the SafeMath fixes:

Screenshot 2025-01-30 at 12 23 14 AM

(Have since fixed the grouping of tests in the logs — the test results are unchanged.)

Screen recordings / screenshots

What to test

@christianbaroni christianbaroni marked this pull request as ready for review January 30, 2025 08:17
@brunobar79
Copy link
Member

Launch in simulator or device for 36da34f

@brunobar79
Copy link
Member

Launch in simulator or device for ada00df

@christianbaroni christianbaroni merged commit ea111bc into develop Jan 30, 2025
8 checks passed
@christianbaroni christianbaroni deleted the @christian/safe-math-fixes branch January 30, 2025 18:17
Copy link

sentry-io bot commented Feb 11, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ React ErrorBoundary RangeError: Exponent must be positive toFixedWorklet(src/safe-math/SafeMath) View Issue

Did you find this useful? React with a 👍 or 👎

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.

4 participants