Handle zero or undefined adaPerUsd in calculation#50
Handle zero or undefined adaPerUsd in calculation#50aniket866 wants to merge 1 commit intoDjedAlliance:mainfrom
Conversation
📝 WalkthroughWalkthroughThe change adds validation to the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
djed-sdk/src/helpers.js (1)
116-123: Same division-by-zero vulnerability exists here.
calculateRcUsdEquivalenthas the same unguarded division byadaPerUsdon line 121. Additionally,adaPerRccould also be invalid. Apply consistent defensive checks to this function as well.🐛 Proposed fix
export function calculateRcUsdEquivalent(coinsDetails, amountFloat) { const adaPerRc = parseFloat(coinsDetails?.scaledSellPriceRc); const adaPerUsd = parseFloat( coinsDetails?.scaledScExchangeRate.replaceAll(",", "") ); + if (!adaPerUsd || adaPerUsd <= 0 || !adaPerRc) return "0"; const eqPrice = (1e6 * amountFloat * adaPerRc) / adaPerUsd; return decimalScaling(eqPrice.toFixed(0).toString(10), 6); }
🧹 Nitpick comments (2)
djed-sdk/src/helpers.js (2)
107-107: Good fix for division-by-zero; minor redundancy in condition.The guard correctly prevents division by zero and handles
NaNfrom invalid inputs. However,adaPerUsd === 0is redundant since!adaPerUsdalready evaluates totruewhenadaPerUsdis0(falsy in JavaScript).Consider also guarding against negative exchange rates, which would be invalid:
♻️ Suggested simplification
- if (!adaPerUsd || adaPerUsd === 0) return "0"; + if (!adaPerUsd || adaPerUsd <= 0) return "0";
128-132: Consider guarding against invalidadaPerScfor consistency.While there's no division-by-zero risk here, if
adaPerScisNaN, the result will propagateNaNthrough the calculation, potentially causing"NaN"to appear in the UI. For consistency with the other functions, consider adding a guard.♻️ Optional defensive check
export function getScAdaEquivalent(coinsDetails, amountFloat) { const adaPerSc = parseFloat(coinsDetails?.scaledPriceSc.replaceAll(",", "")); + if (!adaPerSc || adaPerSc <= 0) return "0"; const eqPrice = 1e6 * amountFloat * adaPerSc; return decimalScaling(eqPrice.toFixed(0).toString(10), 6); }
fix this :Division by Zero Vulnerability
Location: djed-sdk/src/helpers.js in calculateBcUsdEquivalent
Issue: const eqPrice = (1e6 * amountFloat) / adaPerUsd;.
Impact: If adaPerUsd (fetched from oracle/contract) is 0 (e.g., during system pause or Oracle failure), this throws Infinity, causing UI crashes.
Fix: Add a check: if (!adaPerUsd || adaPerUsd === 0) return "0";.
@Zahnentferner
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.