Skip to content

Fix: Remove floating-point precision loss in currency conversions usi…#90

Open
Rav1Chauhan wants to merge 1 commit intoDjedAlliance:mainfrom
Rav1Chauhan:fix/bigint-currency-precision
Open

Fix: Remove floating-point precision loss in currency conversions usi…#90
Rav1Chauhan wants to merge 1 commit intoDjedAlliance:mainfrom
Rav1Chauhan:fix/bigint-currency-precision

Conversation

@Rav1Chauhan
Copy link
Copy Markdown

@Rav1Chauhan Rav1Chauhan commented Feb 24, 2026

…ng BigInt

Addressed Issues:

Fixes #71

Issue

Conversion functions in helpers.js used parseFloat and 1e6 multipliers, leading to floating-point precision loss for large currency amounts (> 2^53 - 1).

This is unacceptable for financial/blockchain software.

Solution

Replaced all float-based arithmetic with BigInt

Maintained full integer precision internally

Converted to formatted string only at final display stage

Added test coverage for large values

Result

Ensures mathematically correct USD equivalents for large blockchain-denominated values.

Screenshots/Recordings:

Not applicable.

This change modifies internal arithmetic logic in djed-sdk/src/helpers.js and does not affect UI components directly.

To validate behavior, large-value test cases were manually verified to ensure no precision loss for values exceeding 2^53 - 1.

Additional Notes:

🔍 Problem Summary

The following functions used floating-point arithmetic:

calculateBcUsdEquivalent

calculateRcUsdEquivalent

getScAdaEquivalent

They relied on:

parseFloat

1e6

.toFixed()

JavaScript Number is limited to 53-bit precision. For blockchain-scale values (10^18 and above), this causes rounding errors and loss of precision — which is unacceptable for financial software.

Checklist

  • [ x] My PR addresses a single issue, fixes a single bug or makes a single improvement.
  • [x ] My code follows the project's code style and conventions.
  • If applicable, I have made corresponding changes or additions to the documentation.
  • If applicable, I have made corresponding changes or additions to tests.
  • [ x] My changes generate no new warnings or errors.
  • [x ] I have joined the Stability Nexus's Discord server and I will share a link to this PR with the project maintainers there.
  • [ x] I have read the Contribution Guidelines.
  • [x ] Once I submit my PR, CodeRabbit AI will automatically review it and I will address CodeRabbit's comments.

⚠️ AI Notice - Important!

We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced currency conversion calculations to prevent overflow errors and improve accuracy when converting between BC, RC, and SC currencies.
  • Refactor

    • Standardized parameter naming conventions and improved internal consistency across currency conversion utilities.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 24, 2026

📝 Walkthrough

Walkthrough

All currency conversion helper functions in the SDK have been refactored to use BigInt arithmetic instead of floating-point operations. Function parameters were standardized from amountFloat to amount, with additional guard checks for missing coin rates. Return values now leverage decimal scaling to deliver 6-decimal precision strings.

Changes

Cohort / File(s) Summary
Currency Conversion Helpers
djed-sdk/src/helpers.js
Refactored five currency conversion functions (calculateBcUsdEquivalent, getBcUsdEquivalent, calculateRcUsdEquivalent, getRcUsdEquivalent, getScAdaEquivalent) to replace floating-point arithmetic with BigInt-based calculations. Parameter naming standardized to amount. Guard clauses added to validate coin rate availability. Return values formatted using decimalScaling for consistent 6-decimal precision.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • refactor-helper.js #75: Directly related to the same code-level change replacing floating-point currency conversion with BigInt-based arithmetic in the helpers file.

Poem

🐰 Floating points? No thanks, friend!
BigInt precision—this mess we'll mend,
No rounding errors to cause dismay,
Pure decimal math will save the day! ✨💰

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: replacing floating-point arithmetic with BigInt to eliminate precision loss in currency conversions.
Linked Issues check ✅ Passed All coding requirements from issue #71 are met: BigInt replaces parseFloat in conversion functions, integer precision is preserved through computation, and decimal formatting occurs only at the final display stage.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing precision loss in currency helpers via BigInt implementation; no unrelated modifications to UI, configuration, or other components are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@djed-sdk/src/helpers.js`:
- Around line 120-154: Both calculateBcUsdEquivalent and
calculateRcUsdEquivalent can throw on BigInt division by zero when
coinsDetails.scaledScExchangeRate is "0"; after converting scaledScExchangeRate
to BigInt (adaPerUsd) add an explicit guard if (adaPerUsd === 0n) return
"0.000000" (or the same zero-formatted string used elsewhere) before performing
any division; similarly, ensure any other BigInt denominators (e.g., adaPerRc if
used as a divisor) are checked for zero to avoid RangeError.
- Around line 120-171: The conversion functions use BigInt() directly on
decimal-formatted strings which throws; update calculateBcUsdEquivalent,
calculateRcUsdEquivalent, and getScAdaEquivalent to call decimalUnscaling(...)
on scaledScExchangeRate, scaledSellPriceRc, and scaledPriceSc respectively
(after removing commas if needed) and pass the integer string returned by
decimalUnscaling to BigInt; keep the existing null/undefined guards and then use
the resulting BigInt values (adaPerUsd, adaPerRc, adaPerSc) in the same
arithmetic as before.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 509e7d3 and 6536504.

📒 Files selected for processing (1)
  • djed-sdk/src/helpers.js

Comment thread djed-sdk/src/helpers.js
Comment thread djed-sdk/src/helpers.js
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.

[BUG] Precision Loss from Floating Point Math

1 participant