Fix: Import missing web3Promise in stableCoin.js to resolve Reference…#86
Open
Rav1Chauhan wants to merge 1 commit intoDjedAlliance:mainfrom
Open
Fix: Import missing web3Promise in stableCoin.js to resolve Reference…#86Rav1Chauhan wants to merge 1 commit intoDjedAlliance:mainfrom
Rav1Chauhan wants to merge 1 commit intoDjedAlliance:mainfrom
Conversation
📝 WalkthroughWalkthroughAdded Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/djed/stableCoin.js (1)
122-129:⚠️ Potential issue | 🟡 Minor
=== 0nguard is insufficient — negativefutureScSupplyproduces a wrong price.When
amountSCis negative (sell scenario),BigInt(scSupply) + BigInt(amountSC)can become negative without ever passing through zero. Division on line 126 then yields a negativefuturePrice, and becauseBigInt(scTargetPrice) < negativeValueisfalse, the function silently returns a negative price string to the caller instead of falling back toscTargetPrice.🛠️ Proposed fix
- if (futureScSupply === 0n) { + if (futureScSupply <= 0n) { return scTargetPrice;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@djed-sdk/src/djed/stableCoin.js` around lines 122 - 129, The guard that checks futureScSupply === 0n is insufficient because a negative futureScSupply (sell scenario) yields a negative futurePrice; change the logic in the block using futureScSupply/futurePrice to first guard for non-positive supply (if futureScSupply <= 0n) and return scTargetPrice, then compute futurePrice and ensure you compare BigInts (e.g., BigInt(scTargetPrice) vs futurePrice) and also guard futurePrice <= 0n to return scTargetPrice; finally return consistent types (string) for the chosen price. Use the symbols futureScSupply, futurePrice, scTargetPrice, amountSC, and scSupply to locate and update the code.
🧹 Nitpick comments (2)
djed-sdk/src/djed/stableCoin.js (2)
123-129: Inconsistent return types across branches.
- Lines 123 and 128 return
scTargetPricedirectly as received fromweb3Promise(web3 1.x typically returns a decimalstringforuint256, but can also return aBNobject depending on the ABI configuration).- Line 129 returns
futurePrice.toString(), which is always astring.If callers expect a uniform type (e.g., always
string), the raw-passthrough branches could cause subtle downstream failures. Explicitly coercingscTargetPricetoStringon both return sites makes the contract clear.✨ Proposed fix
if (futureScSupply <= 0n) { - return scTargetPrice; + return scTargetPrice.toString(); } else { const futurePrice = (futureRatio * BigInt(scDecimalScalingFactor)) / futureScSupply; return BigInt(scTargetPrice) < futurePrice - ? scTargetPrice + ? scTargetPrice.toString() : futurePrice.toString(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@djed-sdk/src/djed/stableCoin.js` around lines 123 - 129, The function returns mixed types: scTargetPrice (as returned from web3Promise) and futurePrice.toString(); normalize both branches to the same type by coercing scTargetPrice to a string before returning. Locate the branch that currently returns scTargetPrice and replace that raw return with String(scTargetPrice) (or scTargetPrice.toString()) so both the direct passthrough and the futurePrice branch consistently return strings; ensure this change affects all return sites that return scTargetPrice in the function handling futureRatio, scDecimalScalingFactor, and futureScSupply.
2-2: LGTM — import fix correctly resolves theReferenceError.
web3Promiseis now imported alongside its co-dependents from../helpers, matching the pattern used indjed.jsandsystem.js. One minor spacing nit: there is an extraneous space before the comma inbuildTx ,web3Promise.✨ Optional formatting fix
-import { decimalScaling, buildTx ,web3Promise } from "../helpers"; +import { decimalScaling, buildTx, web3Promise } from "../helpers";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@djed-sdk/src/djed/stableCoin.js` at line 2, The import line has an extraneous space before the comma between buildTx and web3Promise; update the import statement in stableCoin.js to remove the space before the comma and format the list consistently (e.g., "buildTx, web3Promise") so the symbols decimalScaling, buildTx and web3Promise are imported cleanly and match the style used in djed.js and system.js.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@djed-sdk/src/djed/stableCoin.js`:
- Around line 122-129: The guard that checks futureScSupply === 0n is
insufficient because a negative futureScSupply (sell scenario) yields a negative
futurePrice; change the logic in the block using futureScSupply/futurePrice to
first guard for non-positive supply (if futureScSupply <= 0n) and return
scTargetPrice, then compute futurePrice and ensure you compare BigInts (e.g.,
BigInt(scTargetPrice) vs futurePrice) and also guard futurePrice <= 0n to return
scTargetPrice; finally return consistent types (string) for the chosen price.
Use the symbols futureScSupply, futurePrice, scTargetPrice, amountSC, and
scSupply to locate and update the code.
---
Nitpick comments:
In `@djed-sdk/src/djed/stableCoin.js`:
- Around line 123-129: The function returns mixed types: scTargetPrice (as
returned from web3Promise) and futurePrice.toString(); normalize both branches
to the same type by coercing scTargetPrice to a string before returning. Locate
the branch that currently returns scTargetPrice and replace that raw return with
String(scTargetPrice) (or scTargetPrice.toString()) so both the direct
passthrough and the futurePrice branch consistently return strings; ensure this
change affects all return sites that return scTargetPrice in the function
handling futureRatio, scDecimalScalingFactor, and futureScSupply.
- Line 2: The import line has an extraneous space before the comma between
buildTx and web3Promise; update the import statement in stableCoin.js to remove
the space before the comma and format the list consistently (e.g., "buildTx,
web3Promise") so the symbols decimalScaling, buildTx and web3Promise are
imported cleanly and match the style used in djed.js and system.js.
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
1 participant
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.
…Error
Addressed Issues:
Fixes #85
Screenshots/Recordings:
Not applicable — this is a backend SDK import fix.
Additional Notes:
This change aligns stableCoin.js with other files in the same directory (e.g., djed.js, system.js) that correctly import required helper functions.
No breaking changes introduced.
Checklist
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
Release Notes