generalize StablePay to support multiple Djed/Gluon stablecoins and ERC20 base assets#83
generalize StablePay to support multiple Djed/Gluon stablecoins and ERC20 base assets#83aniket866 wants to merge 9 commits intoDjedAlliance:mainfrom
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (5)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the
📝 WalkthroughWalkthroughAdds ABI artifacts and multi-variant Djed support (Isis/Tefnut), new contract and oracle factories, transaction builders (including sell-both), event listeners, ERC20 token helpers and approval flows, trade utilities, StablePay config rework for multiple stablecoins, UI changes for token-aware prepare/approve/send flow, and package build scripts plus Rollup externals. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as TransactionReview
participant Tx as Transaction
participant Token as ERC20
participant Djed as Djed Contract
participant Chain as Blockchain
User->>UI: Select stablecoin and base asset
UI->>Tx: Initialize with stablecoin config
Tx->>Token: checkAllowance(owner, spender)
Token-->>Tx: allowance
Tx-->>UI: isApproved
alt allowance insufficient
User->>UI: Click Approve
UI->>Tx: approveBaseAsset(payer, amount)
Tx->>Token: encode approve(spender, amount)
UI->>Chain: send approval tx
Chain-->>UI: receipt
UI->>Tx: re-check allowance
end
User->>UI: Prepare transaction
UI->>Tx: request tradeData (prices, fees)
Tx->>Djed: call price methods (scPrice/rcPrice/targets)
Djed-->>Tx: price & fee data
Tx-->>UI: tradeAmount / txData
User->>UI: Send transaction
UI->>Chain: send prepared tx (value/data)
Chain->>Djed: execute tx
Djed->>Token: transferFrom (if ERC20)
Chain-->>UI: tx receipt
UI-->>User: trade complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 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: 6
🤖 Fix all issues with AI agents
In `@djed-sdk/src/djed/sellBoth.js`:
- Around line 69-71: The catch block in tradeDataPriceSellBoth currently only
logs the error and allows the function to return undefined; update the catch to
either rethrow the caught error (e.g., after console.error do throw error) or
return a well-defined error value/promise rejection (e.g., return
Promise.reject(error) or return { error }) so callers receive a clear failure
signal; modify the catch in sellBoth.js inside tradeDataPriceSellBoth
accordingly.
In `@stablepay-sdk/src/core/NetworkSelector.js`:
- Around line 48-51: NetworkSelector.getTokenAmount(tokenKey) correctly passes
both this.selectedNetwork and tokenKey but MerchantConfig.getTokenAmount
currently only accepts a network and ignores tokenKey; change
MerchantConfig.getTokenAmount to accept (network, tokenKey) and use tokenKey
when resolving the amount (e.g., look up network-specific token map or fallback)
so multi-token support works — update the function signature and any callers,
preserving existing fallback behavior for legacy calls that may pass only
network.
In `@stablepay-sdk/src/core/Transaction.js`:
- Around line 1-15: The code deep-imports CoinABI.json as "coinArtifact" from
'djed-sdk/src/artifacts/CoinABI.json' which will break for npm consumers;
replace that import with a top-level import from the public djed-sdk package
(e.g. import { CoinABI as coinArtifact } from 'djed-sdk' or import coinArtifact
from 'djed-sdk') and if djed-sdk does not currently export CoinABI, add an
export in djed-sdk's public entry (e.g. export { default as CoinABI } from
'./artifacts/CoinABI.json' in djed-sdk/src/index.js) so Transaction.js can
safely import the artifact via the package's public API.
In `@stablepay-sdk/src/utils/config.js`:
- Around line 7-85: TokenDropdown.jsx still uses removed
networkConfig.djedAddress (causing undefined passed to new Transaction and
djedContractAddress), so update the call sites to pass the selected stablecoin
object instead: when constructing Transaction (new Transaction(...)) replace the
second argument networkConfig.djedAddress with the selected stablecoin (from
networkConfig.stablecoins matching the selection or the component's selected
token state), and set djedContractAddress to that stablecoin's contractAddress
(or the stableCoin.address field) rather than networkConfig.djedAddress; adjust
any variable names (e.g., selectedStablecoin, stableCoin.address,
contractAddress) accordingly so Transaction receives the stablecoin config it
needs.
In `@stablepay-sdk/src/widget/TransactionReview.jsx`:
- Line 239: The JSX uses a hard-coded className "message-box" which is
inconsistent with the CSS module pattern used elsewhere (e.g.,
styles.transactionReview, styles.walletButton); change the element rendering
{message && <div className="message-box">{message}</div>} to use the CSS module
class (e.g., styles.messageBox or the appropriate exported class name from the
module) and ensure the TransactionReview.jsx import of the styles object is
present and the CSS module defines that class name.
- Around line 129-133: The approval flow currently assumes success via a 5s
setTimeout; replace that with waiting for the actual on-chain confirmation of
the approval transaction (e.g., call waitForTransactionReceipt or
provider.waitForTransaction on the approval tx hash) and then update UI state
via setIsApproved(true), setIsApproving(false), and setMessage(...); on failure
or timeout clear setIsApproving(false) and setMessage with an error, and prevent
concurrent approvals by checking the current approving state (setIsApproving)
before sending a new approval. Ensure you reference the approval transaction
object/tx.hash returned by your approve() call and handle promise rejection,
confirmation, and error paths instead of using setTimeout.
🧹 Nitpick comments (7)
djed-sdk/src/oracle/oracle.js (2)
1-1: Unused import:convertIntis not used in this file.The
convertInthelper is imported but never referenced in any of the functions. Consider removing it to avoid confusion.🧹 Proposed fix
-import { convertInt, web3Promise } from "../helpers"; +import { web3Promise } from "../helpers";
18-26: Improve JSDoc comments to describe the function purpose.The comment "Added export to resolve Rollup error" does not describe what the function does. Consider documenting the actual purpose (e.g., creating a HebeSwap oracle contract instance).
📝 Suggested documentation improvement
-/** - * Added export to resolve Rollup error - */ +/** + * Creates a HebeSwap oracle contract instance. + * `@param` {object} web3 - Web3 instance + * `@param` {string} oracleAddress - The oracle contract address + * `@param` {string} msgSender - The sender address for transactions + * `@returns` {object} Web3 contract instance + */ export const getHebeSwapOracleContract = (web3, oracleAddress, msgSender) => {The same applies to
getChainlinkOracleContractandgetAPI3OracleContract.djed-sdk/src/djed/token.js (1)
8-10: Redundantawaitin return statement.The
return awaitpattern is unnecessary here since the function already returns a promise. Theawaitadds no value and slightly increases overhead.🧹 Proposed fix
export const checkAllowance = async (tokenContract, owner, spender) => { - return await tokenContract.methods.allowance(owner, spender).call(); + return tokenContract.methods.allowance(owner, spender).call(); };djed-sdk/src/djed/djed.js (1)
46-54: Consider logging or documenting the swallowed error incheckIfShu.The catch block silently swallows all errors, which is appropriate for feature detection. However, logging unexpected errors (non-revert errors like network issues) could aid debugging in production.
💡 Optional improvement for debugging
export const checkIfShu = async (djedContract) => { try { // Check if scMaxPrice exists on the contract await djedContract.methods.scMaxPrice(0).call(); return true; } catch (e) { + // Expected: method doesn't exist on non-Shu contracts return false; } };djed-sdk/src/djed/listeners.js (1)
50-65: Consider returning an empty array instead of throwing on error.The function logs and rethrows errors, which is fine for debugging, but callers must handle exceptions. For a utility function, you might consider whether returning an empty array with the logged error would provide a more resilient fallback for UI consumers.
🔧 Optional: Return empty array on error
export const getPastDjedEvents = async ( djedContract, eventName, filter = {}, fromBlock = 0 ) => { try { return await djedContract.getPastEvents(eventName, { filter, fromBlock, toBlock: "latest", }); } catch (error) { console.error(`Error fetching past events for ${eventName}:`, error); - throw error; + return []; // Graceful fallback for UI consumers } };djed-sdk/src/djed/tradeUtils.js (1)
148-160: Consider caching the SHU detection result.The
getPriceMethodfunction callsscMaxPrice(0).call()on every invocation to detect if the contract is a SHU variant. This RPC call could be cached per contract instance to avoid redundant network requests, especially if this function is called multiple times during a transaction flow.♻️ Proposed caching approach
+const shuCache = new WeakMap(); + export const getPriceMethod = async (djed, operation) => { - const isShu = await djed.methods.scMaxPrice(0).call().then(() => true).catch(() => false); + let isShu = shuCache.get(djed); + if (isShu === undefined) { + isShu = await djed.methods.scMaxPrice(0).call().then(() => true).catch(() => false); + shuCache.set(djed, isShu); + } if (!isShu) return "scPrice";stablepay-sdk/src/widget/TransactionReview.jsx (1)
81-87: Approval state may be stale if account changes.The
checkAllowancecall insideinitializeTransactiondepends onaccount, but if the account changes mid-flow, thetradeDataAmountused for the allowance check might be from a previous initialization. Consider storing the relevant amount in state or re-checking allowance when account changes independently.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Closes #84 |
|
@DengreSarthak Please review |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@djed-sdk/package.json`:
- Around line 8-11: The package.json has a trailing comma after the "prepare"
script which makes the JSON invalid; open the scripts object containing the
"test", "build", and "prepare" entries and remove the trailing comma after the
"prepare" value so the JSON parses correctly and npm commands
(install/build/publish) will run.
In `@stablepay-sdk/src/utils/config.js`:
- Around line 53-70: The djed-usdt-isis stablecoin entry uses zero addresses for
contractAddress, baseAsset.address and stableCoin.address which will break
transactions; update the entry (id: 'djed-usdt-isis') to mark it as non-usable
(e.g., add enabled: false or status: 'placeholder') and remove or hide it from
UI lists, and additionally add runtime validation in the Transaction handling
code to reject zero addresses (check
contractAddress/baseAsset.address/stableCoin.address) so placeholder entries
cannot be used even if present.
In `@stablepay-sdk/src/widget/TransactionReview.jsx`:
- Around line 119-137: The code uses the original walletClient when waiting for
the approval receipt, which can be stale after calling ensureCorrectNetwork();
replace the call to walletClient.waitForTransactionReceipt({ hash }) with
freshWalletClient.waitForTransactionReceipt({ hash }) so the receipt is awaited
on the same client used to send the transaction (references: approveBaseAsset,
ensureCorrectNetwork, sendTransaction, waitForTransactionReceipt,
freshWalletClient, walletClient, hash).
- Around line 62-69: The catch block for newTransaction.handleTradeDataBuySc
currently only logs the error and leaves tradeDataAmount null, causing the UI to
show "Calculating..." and blocking flows; update the catch to set component
state to reflect the failure (e.g., setTradeDataError(true) or
setTradeDataFetchError with the tradeError), clear any loading flags (e.g.,
setIsCalculating(false)) and ensure tradeAmount-dependent actions are disabled
or show a user-facing error message; reference the tokenAmount variable,
tradeDataAmount, newTransaction.handleTradeDataBuySc, and the state setters used
in TransactionReview.jsx to implement this change.
🧹 Nitpick comments (2)
stablepay-sdk/src/widget/TokenDropdown.jsx (1)
44-61: Minor:tokenSymbolis sourced fromselectedStablecoinbut should use consistent source.Line 44 calls
networkSelector.getTokenAmount(newValue)which is fine, but line 50 usesselectedStablecoin.stableCoin.symbolwhile the rest of the codebase (e.g., TransactionReview.jsx line 74) usesselectedToken.symbolfrom context. This works but creates a subtle inconsistency in data sourcing.Additionally, consider that
blockchainDetails(line 60) may contain overlapping keys with the explicitly set properties (lines 55-59), and the spread order meansblockchainDetailsvalues could override your explicit values if they share keys.♻️ Suggested reordering to ensure explicit values take precedence
setTransactionDetails({ network: selectedNetwork, token: newValue, tokenSymbol: selectedStablecoin.stableCoin.symbol, amount: tokenAmount, receivingAddress: networkSelector.getReceivingAddress(), - - // Use properties from the selected stablecoin object - djedContractAddress: selectedStablecoin.contractAddress, - isDirectTransfer: selectedStablecoin.stableCoin.isDirectTransfer || false, - baseAssetSymbol: selectedStablecoin.baseAsset.symbol, - baseAssetDecimals: selectedStablecoin.baseAsset.decimals, - baseAssetIsNative: selectedStablecoin.baseAsset.isNative, ...blockchainDetails, + // Explicit stablecoin properties override blockchainDetails if overlapping + djedContractAddress: selectedStablecoin.contractAddress, + isDirectTransfer: selectedStablecoin.stableCoin.isDirectTransfer || false, + baseAssetSymbol: selectedStablecoin.baseAsset.symbol, + baseAssetDecimals: selectedStablecoin.baseAsset.decimals, + baseAssetIsNative: selectedStablecoin.baseAsset.isNative, });stablepay-sdk/src/widget/TransactionReview.jsx (1)
81-87: AsynccheckAllowancecall not awaited in effect body.The
checkAllowancefunction is async but called withoutawaiton lines 83-86. While this doesn't break functionality (state updates will still occur), it means:
- The effect completes before allowance check finishes
- Any errors thrown won't be caught by the surrounding try/catch
- Race conditions possible if component unmounts before completion
♻️ Proposed fix to await the allowance check
if (account) { if (!stablecoinConfig.baseAsset.isNative && tradeDataAmount) { - checkAllowance(newTransaction, account, tradeDataAmount, stablecoinConfig.baseAsset.decimals); + await checkAllowance(newTransaction, account, tradeDataAmount, stablecoinConfig.baseAsset.decimals); } else if (stablecoinConfig.baseAsset.isNative) { setIsApproved(true); } }
1 participant
Key Changes
1. Protocol Expansion: Added support for Djed Isis and Djed Tefnut variants.
ERC20 Base Asset Support: Implemented
buyScIsisTxand related builders to handle ERC20-backed stablecoin purchases (zero msg.value flow).Oracle Integration: Integrated support for Orb, Chainlink, HebeSwap, and API3 oracles.
Atomic Exits: Added sellBothCoins functionality to exit stable and reserve coin positions in a single transaction.
2. Varible Configuration: Refactored networksConfig to support arrays of stablecoins per network.
3. Enhanced UI Components:
TokenDropdown: Dynamically lists all available protocol configurations from the updated config.
TransactionReview: Implemented an "Approve" flow for ERC20 base assets before allowing payment.
4. Event Infrastructure: Introduced event subscription and historical retrieval utilities.
@Zahnentferner Please check this out
Closes #84
Summary by CodeRabbit
New Features
Refactor
UI
Chores