refactor djed-sdk/src/helpers.js

[aniket866]

Refactor includes:

• Fixed precision loss in USD conversion functions.

• Removed use of parseFloat and floating-point math for currency calculations.

• All calculations now use BigInt to keep values exact for large amounts.

• Output formatting remains unchanged and backward-compatible.

• This prevents rounding errors in financial calculations.

Closes #75

@Zahnentferner Please review

Summary by CodeRabbit

• Refactor
  • Updated currency calculation functions to accept string inputs instead of numeric values.
  • Enhanced calculation accuracy with fixed 6-decimal precision arithmetic.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Warning

Rate limit exceeded

@aniket866 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 42 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

The PR refactors currency calculation functions in helpers.js to use BigInt arithmetic instead of floating-point operations, replacing function parameters from numeric amounts to string amounts. A new decimalToBigInt helper is introduced to enable fixed 6-decimal precision conversions across all currency equivalency calculations.

Changes

Cohort / File(s)

Summary

BigInt Currency Refactoring djed-sdk/src/helpers.js

Refactored all currency conversion functions (calculateBcUsdEquivalent, getBcUsdEquivalent, calculateRcUsdEquivalent, getRcUsdEquivalent, getScAdaEquivalent) to use BigInt arithmetic with fixed 6-decimal precision. Updated all function signatures to accept amountStr (string) instead of amountFloat (number). Added decimalToBigInt helper for decimal string to BigInt conversion. Simplified control flow in IntersperseCommas and percentageScale.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• Issue #71: Directly addresses the floating-point precision loss in currency conversions by implementing the BigInt-based arithmetic with fixed 6-decimal scaling across all affected helper functions.

Poem

🐰 From floats that drift like dandelion seeds,
We plant BigInt gardens for precise needs,
Six decimals firm, no rounding to fear,
Our currency sums now crystal clear! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'refactor djed-sdk/src/helpers.js' is vague and generic, using only the file path without conveying the specific nature of the refactoring (precision improvements via BigInt migration).	Enhance title to be more specific about the primary change, e.g., 'Migrate currency calculations to BigInt for precision' or 'Fix floating-point precision loss in currency helpers'.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	The pull request successfully addresses all coding requirements from issue #75: eliminates parseFloat, replaces floating-point math with BigInt arithmetic, and maintains calculation correctness with fixed 6-decimal precision.
Out of Scope Changes check	✅ Passed	Besides the main BigInt migration, the PR includes minor refactorings (removing unnecessary else branches, using conditional expressions) that are closely related to code quality improvements and maintain scope consistency.

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

✨ Finishing touches

🧪 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@djed-sdk/src/helpers.js`:
- Around line 144-151: In getScAdaEquivalent, the multiplication of two
6-decimal fixed-point values (amount and adaPerSc) yields a 12-decimal scaled
value (eqPrice); before calling decimalScaling, rescale eqPrice down by
1_000_000n (i.e., divide by 1_000_000n) so decimalScaling(..., 6) receives a
6-decimal value, or alternatively call decimalScaling(eqPrice.toString(), 12) to
tell decimalScaling the value is 12-decimal; update the logic around eqPrice and
the decimalScaling call accordingly.
- Around line 117-119: The code computes adaPerUsd via
decimalToBigInt(coinsDetails.scaledScExchangeRate) and then uses it as a divisor
(e.g., when computing eqPrice), but it does not guard against adaPerUsd being 0
which will throw on BigInt division; after computing adaPerUsd in the functions
that use it (references: decimalToBigInt, coinsDetails.scaledScExchangeRate,
adaPerUsd, eqPrice) add a check that throws a clear error or returns a safe
value if adaPerUsd === 0n (e.g., throw new RangeError('scaledScExchangeRate
resulted in zero')), so all subsequent divisions by adaPerUsd are protected.

🧹 Nitpick comments (1)

djed-sdk/src/helpers.js (1)

114-116: Add type validation to prevent silent precision loss in amount conversions.

Lines 115, 129, and 145 call .toString() on the amountStr parameter before passing to decimalToBigInt. If a caller accidentally passes a number, Number.toString() can lose precision before BigInt conversion happens. Since the parameter name already documents that strings are expected, enforce this by adding a type check and rejecting non-strings with a clear error rather than silently converting.

Suggested fix (apply to all three functions)

 export function calculateBcUsdEquivalent(coinsDetails, amountStr) {
+  if (typeof amountStr !== "string") {
+    throw new TypeError("amountStr must be a decimal string");
+  }
-  const amount = decimalToBigInt(amountStr.toString(), 6);
+  const amount = decimalToBigInt(amountStr, 6);

Apply similarly to calculateRcUsdEquivalent (line 129) and getScAdaEquivalent (line 145).