Upgrade Djed SDK to Support Advanced Protocol Features and Djed Shu#81
Upgrade Djed SDK to Support Advanced Protocol Features and Djed Shu#81DeveloperAmrit wants to merge 6 commits intoDjedAlliance:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds Isis/Tefnut (Shu) variant support: new ABIs and oracle artifacts, runtime variant detection, dynamic price-method selection, new ERC20-style transaction helpers, sell‑both flow, event listeners, and expanded system/coin parameter outputs. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as SDK Consumer
participant SDK as Djed SDK
participant Contract as Djed Contract
participant Oracle as Oracle Contract
Client->>SDK: getCoinDetails(djed)
SDK->>Contract: scMaxPrice(0) (probe via checkIfShu)
alt probe succeeds (Shu variant)
SDK->>Contract: ratioMin(), ratioMax()
Contract-->>SDK: ratios
SDK->>Contract: scMaxPrice / scMinPrice (per operation via getPriceMethod)
Contract-->>SDK: prices
SDK->>Oracle: readData / scalingFactor
Oracle-->>SDK: oracle price
SDK-->>Client: coinDetails (isShu=true, min/max ratios, prices)
else probe fails (non‑Shu)
SDK->>Contract: scPrice(), ratio()
Contract-->>SDK: price, ratio
SDK-->>Client: coinDetails (isShu=false, price, ratio)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
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)
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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
djed-sdk/src/djed/reserveCoin.js (2)
20-28:⚠️ Potential issue | 🟠 MajorUnused
scMethodvariable - incomplete Shu integration.
getPriceMethod(djed, 'buyRC')is called and its result stored inscMethod, but this variable is never used. ThetradeDataPriceCorecall still uses the hardcoded"rcBuyingPrice"string. This wastes an RPC call and doesn't achieve the intended Shu-aware pricing.Compare with
stableCoin.jswhere the dynamic method is correctly passed totradeDataPriceCore. Either remove the unused call or integrate it into the pricing logic.Proposed fix (if scMethod should be used)
export const tradeDataPriceBuyRc = async (djed, rcDecimals, amountScaled) => { try { - const scMethod = await getPriceMethod(djed, 'buyRC'); const data = await tradeDataPriceCore( djed, "rcBuyingPrice", rcDecimals, amountScaled );Or, if the method should influence RC pricing:
export const tradeDataPriceBuyRc = async (djed, rcDecimals, amountScaled) => { try { - const scMethod = await getPriceMethod(djed, 'buyRC'); + // Note: RC uses rcBuyingPrice, not SC price methods const data = await tradeDataPriceCore( djed, "rcBuyingPrice", rcDecimals, amountScaled );
47-55:⚠️ Potential issue | 🟠 MajorSame issue: unused
scMethodintradeDataPriceSellRc.Identical problem as above -
scMethodis computed but not used, while"rcTargetPrice"is hardcoded.Proposed fix
export const tradeDataPriceSellRc = async (djed, rcDecimals, amountScaled) => { try { - const scMethod = await getPriceMethod(djed, 'sellRC'); const data = await tradeDataPriceCore( djed, "rcTargetPrice", rcDecimals, amountScaled );djed-sdk/src/djed/stableCoin.js (1)
1-11:⚠️ Potential issue | 🔴 CriticalMissing
web3Promiseimport.
web3Promiseis used incalculateFutureScPrice(line 117-119) but is not imported. This will cause a runtime error.Proposed fix
import { BC_DECIMALS, TRANSACTION_VALIDITY } from "../constants"; -import { decimalScaling, buildTx } from "../helpers"; +import { decimalScaling, buildTx, web3Promise } from "../helpers"; import { tradeDataPriceCore,
🤖 Fix all issues with AI agents
In `@djed-sdk/src/djed/system.js`:
- Around line 150-165: Remove the duplicated/malformed fragment after the
existing return in getSystemParams: delete the orphaned "};", the dangling
"true", and the partial object literal that re-declares
reserveRatioMax/Min/fee/treasuryFee/thresholdSupplySC (these appear right after
the getSystemParams return). Ensure only the single, complete return inside
getSystemParams remains and do not alter legitimate uses of percentageScale,
reserveRatioMaxUnscaled, reserveRatioMinUnscaled, feeUnscaled or treasuryFee
elsewhere in the module.
- Around line 20-21: getCoinDetails currently hardcodes scMaxPrice for Shu
contracts which is inconsistent with other modules that use getPriceMethod from
tradeUtils.js; update getCoinDetails to call getPriceMethod with an explicit
operation type (e.g., a neutral/“info” operation or a sensible default like
"buy" or "none") or, if you truly want a neutral price, implement and call a new
helper in tradeUtils (e.g., getNeutralPriceMethod) and use that instead; locate
getCoinDetails and replace the hardcoded priceMethod assignment (and references
to checkIfShu if needed) to call getPriceMethod or the new helper so behavior
matches other trade code and is documented via the chosen operation name.
In `@djed-sdk/src/djed/tradeUtils.js`:
- Around line 118-120: The isTxLimitReached helper currently uses the ||
fallback which treats txLimit === 0 as falsy; update the fallback to use nullish
coalescing so an explicit 0 (meaning "no limit") is respected. Modify the
expression inside isTxLimitReached (the BigInt(txLimit || TRANSACTION_USD_LIMIT)
part) to use BigInt(txLimit ?? TRANSACTION_USD_LIMIT) so only null or undefined
fall back to TRANSACTION_USD_LIMIT while preserving 0 as a valid value.
🧹 Nitpick comments (3)
djed-sdk/src/djed/djed.js (1)
31-39: Duplicated Shu detection logic.This
checkIfShufunction duplicates the detection logic intradeUtils.js(getPriceMethod, line 214), which also callsscMaxPrice(0).call()inline. Consider consolidating to avoid redundant RPC calls when both paths are exercised.One option: have
getPriceMethodimport and reusecheckIfShuinstead of inline detection.Example consolidation in tradeUtils.js
+import { checkIfShu } from "./djed"; + export const getPriceMethod = async (djed, operation) => { - const isShu = await djed.methods.scMaxPrice(0).call().then(() => true).catch(() => false); + const isShu = await checkIfShu(djed); if (!isShu) return "scPrice"; // ... };djed-sdk/src/djed/sellBoth.js (1)
69-71: Consider returning explicit error indicator.On error, this function logs and returns
undefined. While consistent with other SDK functions, callers have no way to distinguish "error occurred" from "successfully computed undefined." Consider throwing or returning an error object.Optional improvement
} catch (error) { console.error("Error in tradeDataPriceSellBoth:", error); + throw error; // Or return { error } for explicit handling }djed-sdk/src/djed/listeners.js (1)
56-72: Consider a safer default forfromBlock.Defaulting
fromBlockto0can cause performance issues or timeouts on long-running chains when fetching events from genesis. Consider using a more recent default (e.g., last N blocks) or requiring the caller to specify it explicitly.Optional: require explicit fromBlock
export const getPastDjedEvents = async ( djedContract, eventName, - filter = {}, - fromBlock = 0 + { filter = {}, fromBlock } = {} ) => { + if (fromBlock === undefined) { + throw new Error("fromBlock is required to prevent accidental full-chain scans"); + } try {
| const isShu = await checkIfShu(djed); | ||
| const priceMethod = isShu ? "scMaxPrice" : "scPrice"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for getPriceMethod usage
echo "=== Searching for getPriceMethod usage ==="
rg -n 'getPriceMethod' --type js -A 5
echo ""
echo "=== Searching for scMinPrice and scMaxPrice usage patterns ==="
rg -n 'scMinPrice|scMaxPrice' --type js -B 2 -A 2
echo ""
echo "=== Check the system.js file context around price method usage ==="
rg -n 'priceMethod' djed-sdk/src/djed/system.js -B 3 -A 3Repository: DjedAlliance/StablePay
Length of output: 6288
🏁 Script executed:
#!/bin/bash
# Find getCoinDetails function to understand its purpose and operation type
rg -n 'getCoinDetails' djed-sdk/src/djed/system.js -B 2 -A 20
# Check if there's documentation about what operation type it should use
rg -n 'export.*getCoinDetails' djed-sdk/src/djed/system.js -B 5Repository: DjedAlliance/StablePay
Length of output: 957
Use getPriceMethod function for consistency with other trade-related code, or document why hardcoding scMaxPrice is appropriate for getCoinDetails.
Currently, getCoinDetails hardcodes scMaxPrice for Shu contracts, but all other modules (stableCoin.js, sellBoth.js, etc.) use the getPriceMethod function from tradeUtils.js, which returns operation-specific prices (e.g., scMinPrice for sell operations). Since getCoinDetails is informational and not operation-specific, clarify whether it should:
- Call
getPriceMethodwith a defined operation type, or - Use its own logic if displaying a neutral price, since hardcoding to
scMaxPrice(a worst-case price) may not be appropriate for informational purposes.
🤖 Prompt for AI Agents
In `@djed-sdk/src/djed/system.js` around lines 20 - 21, getCoinDetails currently
hardcodes scMaxPrice for Shu contracts which is inconsistent with other modules
that use getPriceMethod from tradeUtils.js; update getCoinDetails to call
getPriceMethod with an explicit operation type (e.g., a neutral/“info” operation
or a sensible default like "buy" or "none") or, if you truly want a neutral
price, implement and call a new helper in tradeUtils (e.g.,
getNeutralPriceMethod) and use that instead; locate getCoinDetails and replace
the hardcoded priceMethod assignment (and references to checkIfShu if needed) to
call getPriceMethod or the new helper so behavior matches other trade code and
is documented via the chosen operation name.
| export const isTxLimitReached = (amountUSD, totalSCSupply, thresholdSCSupply, txLimit) => | ||
| (BigInt(amountUSD) > BigInt(txLimit || TRANSACTION_USD_LIMIT)) && | ||
| BigInt(totalSCSupply) >= BigInt(thresholdSCSupply); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Djed smart contract transaction limit txLimit meaning zero value
💡 Result:
In the Djed Osiris-family deployments (e.g., the Belus Solidity implementation), txLimit refers to the protocol feature “maximum transaction limits” (a cap on how large a single mint/burn/buy/sell operation is allowed to be). (docs.stability.nexus)
If you read txLimit = 0 on-chain, it almost always means the limit is disabled / unlimited (i.e., “no max per-transaction cap”), because Solidity contracts commonly implement optional limits as “only enforce when limit > 0”. (To be 100% certain for your deployment, you’d need to check the exact contract code or verified source for that address.) (docs.stability.nexus)
Citations:
- 1: https://docs.stability.nexus/djed-stablecoin-protocols/versions?utm_source=openai
- 2: https://docs.stability.nexus/djed-stablecoin-protocols/versions?utm_source=openai
🏁 Script executed:
# Search for usage of isTxLimitReached in the codebase to understand how it's called
rg "isTxLimitReached" -B 2 -A 2Repository: DjedAlliance/StablePay
Length of output: 2388
🏁 Script executed:
# Look for any constants or documentation about TRANSACTION_USD_LIMIT and txLimit
rg "TRANSACTION_USD_LIMIT|txLimit" -B 1 -A 1 -t jsRepository: DjedAlliance/StablePay
Length of output: 2749
🏁 Script executed:
# Check if there are any tests or usage examples that show what txLimit values are used
rg "txLimit.*0|txLimit\s*=" -B 2 -A 2 -t jsRepository: DjedAlliance/StablePay
Length of output: 48
🏁 Script executed:
# Search for the TRANSACTION_USD_LIMIT constant definition
rg "TRANSACTION_USD_LIMIT\s*=" -B 1 -A 1 -t jsRepository: DjedAlliance/StablePay
Length of output: 593
Use nullish coalescing (??) instead of || for txLimit fallback.
According to Djed smart contract standards, txLimit = 0 means the limit is disabled/unlimited. The current code txLimit || TRANSACTION_USD_LIMIT incorrectly treats 0 as falsy and applies the fallback limit instead of respecting "no limit". Use ?? to only fall back when txLimit is undefined or null.
Proposed fix
-export const isTxLimitReached = (amountUSD, totalSCSupply, thresholdSCSupply, txLimit) =>
- (BigInt(amountUSD) > BigInt(txLimit || TRANSACTION_USD_LIMIT)) &&
+export const isTxLimitReached = (amountUSD, totalSCSupply, thresholdSCSupply, txLimit) =>
+ (BigInt(amountUSD) > BigInt(txLimit ?? TRANSACTION_USD_LIMIT)) &&
BigInt(totalSCSupply) >= BigInt(thresholdSCSupply);🤖 Prompt for AI Agents
In `@djed-sdk/src/djed/tradeUtils.js` around lines 118 - 120, The isTxLimitReached
helper currently uses the || fallback which treats txLimit === 0 as falsy;
update the fallback to use nullish coalescing so an explicit 0 (meaning "no
limit") is respected. Modify the expression inside isTxLimitReached (the
BigInt(txLimit || TRANSACTION_USD_LIMIT) part) to use BigInt(txLimit ??
TRANSACTION_USD_LIMIT) so only null or undefined fall back to
TRANSACTION_USD_LIMIT while preserving 0 as a valid value.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
djed-sdk/package.json (1)
14-22:⚠️ Potential issue | 🟡 MinorMove
web3fromdevDependenciestopeerDependencies.
web3is used throughout the SDK for contract interactions, event subscriptions, and transaction helpers in the runtime code (src/), yet it's currently only indevDependencies. This means consumers who install this package won't haveweb3automatically installed, causing the SDK to fail at runtime unless they manually install it. By declaring it as apeerDependency, you make this requirement explicit while allowing consumers to use their ownweb3version, avoiding duplicate bundling.♻️ Suggested change
"devDependencies": { "@rollup/plugin-commonjs": "^26.0.1", "@rollup/plugin-json": "^6.1.0", "@rollup/plugin-node-resolve": "^15.2.3", "@rollup/plugin-replace": "^5.0.7", "@rollup/plugin-sucrase": "^5.0.2", - "rollup": "^4.18.0", - "web3": "^1.7.3" - } + "rollup": "^4.18.0" + }, + "peerDependencies": { + "web3": "^1.7.3" + }
🤖 Fix all issues with AI agents
In `@stablepay-sdk/src/core/Transaction.js`:
- Around line 140-145: The checkBaseAssetAllowance method currently returns
'Inf' when this.baseAssetContract is falsy which masks the situation where a
baseAssetAddress was provided but init() hasn't run; update
checkBaseAssetAllowance to fail fast: if this.baseAssetAddress is set but
this.baseAssetContract is not initialized, throw a clear error (e.g. indicating
init() must be called or contract must be initialized) instead of returning
'Inf'; keep the original behavior of returning 'Inf' only when there is no
baseAssetAddress at all, and otherwise call
checkAllowance(this.baseAssetContract, owner, this.djedAddress) as before.
- Around line 1-2: The file imports CoinABI via a deep path which will break
when djed-sdk is installed from npm; replace the deep import (CoinABI from
'djed-sdk/src/artifacts/CoinABI.json') with a public import from the djed-sdk
package (e.g., import CoinABI from 'djed-sdk' or import { CoinABI } from
'djed-sdk') or, if the SDK does not currently export the ABI, either add the ABI
export in djed-sdk or embed a local copy of the needed ABI in Transaction.js;
update any references to CoinABI accordingly so Transaction.js only depends on
djed-sdk's public API.
🧹 Nitpick comments (2)
djed-sdk/package.json (1)
2-3: Consider bumping the version for this feature release.The PR adds significant new functionality (Djed Shu/Isis/Tefnut support,
sellBothmodule, event listeners, new oracle integrations) but the version remains at1.0.2. Per semantic versioning, new features typically warrant a minor version bump (e.g.,1.1.0).If the version bump is planned for a separate commit before release, feel free to disregard.
djed-sdk/src/djed/token.js (1)
3-6: Consider adding a safe-approve helper for ERC-20 allowance-change race conditions.The ERC-20 standard (SWC-114) documents a race condition when changing a non-zero allowance to another non-zero value: a spender can front-run the transaction to spend the original allowance before your new
approveoverwrites it, effectively spending both amounts. The standard and OpenZeppelin recommend resetting allowance to 0 before setting a new non-zero amount, or usingincreaseAllowance/decreaseAllowancepatterns instead.A dedicated safe-approve helper would protect callers without modifying the current
approveTxbehavior.♻️ Proposed helper (optional)
export const approveTx = (tokenContract, owner, spender, amount) => { const data = tokenContract.methods.approve(spender, amount).encodeABI(); return buildTx(owner, tokenContract.options.address, 0, data); }; +// Optional safer pattern: return 1–2 txs depending on current allowance +export const safeApproveTx = async (tokenContract, owner, spender, amount) => { + const current = await tokenContract.methods.allowance(owner, spender).call(); + const currentBn = BigInt(current); + const nextBn = BigInt(amount); + + if (currentBn !== 0n && nextBn !== 0n) { + const resetData = tokenContract.methods.approve(spender, 0).encodeABI(); + const approveData = tokenContract.methods.approve(spender, amount).encodeABI(); + return [ + buildTx(owner, tokenContract.options.address, 0, resetData), + buildTx(owner, tokenContract.options.address, 0, approveData), + ]; + } + + const data = tokenContract.methods.approve(spender, amount).encodeABI(); + return [buildTx(owner, tokenContract.options.address, 0, data)]; +};
| import { getWeb3, getDjedContract, getCoinContracts, getDecimals, getOracleAddress, getOracleContract, tradeDataPriceBuySc, buyScTx, checkAllowance, approveTx, buyScIsisTx } from 'djed-sdk'; | ||
| import coinArtifact from 'djed-sdk/src/artifacts/CoinABI.json'; // Importing CoinABI from SDK |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "Transaction.js" -t fRepository: DjedAlliance/StablePay
Length of output: 103
🏁 Script executed:
head -20 stablepay-sdk/src/core/Transaction.jsRepository: DjedAlliance/StablePay
Length of output: 920
🏁 Script executed:
cat package.json | head -30Repository: DjedAlliance/StablePay
Length of output: 110
🏁 Script executed:
fd "package.json" -t fRepository: DjedAlliance/StablePay
Length of output: 114
🏁 Script executed:
cat stablepay-sdk/package.jsonRepository: DjedAlliance/StablePay
Length of output: 1061
🏁 Script executed:
cat djed-sdk/package.jsonRepository: DjedAlliance/StablePay
Length of output: 634
🏁 Script executed:
fd "index.js" djed-sdk -t fRepository: DjedAlliance/StablePay
Length of output: 197
🏁 Script executed:
cat djed-sdk/src/index.jsRepository: DjedAlliance/StablePay
Length of output: 307
🏁 Script executed:
cat djed-sdk/src/djed/index.jsRepository: DjedAlliance/StablePay
Length of output: 1068
🏁 Script executed:
rg "CoinABI" djed-sdk/src -t js -t jsonRepository: DjedAlliance/StablePay
Length of output: 145
🏁 Script executed:
fd "CoinABI.json" djed-sdk -t fRepository: DjedAlliance/StablePay
Length of output: 101
🏁 Script executed:
cat djed-sdk/src/djed/djed.js | head -50Repository: DjedAlliance/StablePay
Length of output: 1771
🏁 Script executed:
rg "coinArtifact" stablepay-sdk/src/core/Transaction.js -A 3Repository: DjedAlliance/StablePay
Length of output: 385
🏁 Script executed:
rg "coinArtifact" stablepay-sdk/src -B 2 -A 2Repository: DjedAlliance/StablePay
Length of output: 929
🏁 Script executed:
rg "\"files\"" djed-sdk/package.jsonRepository: DjedAlliance/StablePay
Length of output: 48
🏁 Script executed:
cat djed-sdk/.npmignore 2>/dev/null || echo "No .npmignore found"Repository: DjedAlliance/StablePay
Length of output: 84
Avoid deep-importing CoinABI from djed-sdk internals.
Importing djed-sdk/src/artifacts/CoinABI.json directly couples to the dependency's source layout. The djed-sdk package explicitly excludes src/ from published builds (see .npmignore), so this import will break when consumers install djed-sdk from npm. CoinABI must be exported as a public API from djed-sdk for this to work reliably.
Suggested direction (requires djed-sdk to export CoinABI)
-import coinArtifact from 'djed-sdk/src/artifacts/CoinABI.json'; // Importing CoinABI from SDK
+import { CoinABI } from 'djed-sdk'; // Expose CoinABI via package exports🤖 Prompt for AI Agents
In `@stablepay-sdk/src/core/Transaction.js` around lines 1 - 2, The file imports
CoinABI via a deep path which will break when djed-sdk is installed from npm;
replace the deep import (CoinABI from 'djed-sdk/src/artifacts/CoinABI.json')
with a public import from the djed-sdk package (e.g., import CoinABI from
'djed-sdk' or import { CoinABI } from 'djed-sdk') or, if the SDK does not
currently export the ABI, either add the ABI export in djed-sdk or embed a local
copy of the needed ABI in Transaction.js; update any references to CoinABI
accordingly so Transaction.js only depends on djed-sdk's public API.
| async checkBaseAssetAllowance(owner) { | ||
| if (!this.baseAssetContract) { | ||
| return 'Inf'; | ||
| } | ||
| return checkAllowance(this.baseAssetContract, owner, this.djedAddress); | ||
| } |
There was a problem hiding this comment.
Don’t return 'Inf' when baseAssetAddress is set but the contract isn’t initialized.
If baseAssetAddress is provided and init() hasn’t run, returning 'Inf' can cause callers to skip approval and later hit a revert. Fail fast in that case.
✅ Suggested fix
async checkBaseAssetAllowance(owner) {
- if (!this.baseAssetContract) {
- return 'Inf';
- }
- return checkAllowance(this.baseAssetContract, owner, this.djedAddress);
+ if (this.baseAssetAddress && !this.baseAssetContract) {
+ throw new Error("Base Asset contract not initialized");
+ }
+ if (!this.baseAssetAddress) {
+ return 'Inf';
+ }
+ return checkAllowance(this.baseAssetContract, owner, this.djedAddress);
}🤖 Prompt for AI Agents
In `@stablepay-sdk/src/core/Transaction.js` around lines 140 - 145, The
checkBaseAssetAllowance method currently returns 'Inf' when
this.baseAssetContract is falsy which masks the situation where a
baseAssetAddress was provided but init() hasn't run; update
checkBaseAssetAllowance to fail fast: if this.baseAssetAddress is set but
this.baseAssetContract is not initialized, throw a clear error (e.g. indicating
init() must be called or contract must be initialized) instead of returning
'Inf'; keep the original behavior of returning 'Inf' only when there is no
baseAssetAddress at all, and otherwise call
checkAllowance(this.baseAssetContract, owner, this.djedAddress) as before.
1 participant
Fixes #80 and #65
Description
This PR significantly upgrades the djed-sdk to align with the latest Djed-Solidity smart contracts. It introduces support for the safety-enhanced Djed Shu version, implements missing core protocol functions like sellBothCoins, and establishes a robust infrastructure for real-time event monitoring.
Key Changes
How to verify
Summary by CodeRabbit
New Features
Enhancements
✏️ Tip: You can customize this high-level summary in your review settings.