Fix #32 Integrated gluon to stablepay, made stablepay to accept payments with… #33
Conversation
📝 WalkthroughWalkthroughThis pull request adds Gluon protocol support to StablePay SDK by introducing a new Gluon SDK package with contract interaction logic, extending Transaction and widget classes to accept protocol and router configuration parameters, and adding network configuration for Gluon testnet alongside existing Djed networks. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Widget/UI
participant Tx as Transaction
participant Gluon as Gluon SDK
participant Web3 as Web3/Provider
participant GluonC as Gluon Contract
participant Router as GluonRouter Contract
UI->>Tx: buyStablecoins(payer, value, receiver)
alt protocol === 'gluon'
Tx->>Gluon: fission(payer, value, receiver, updateData)
Gluon->>Web3: Build transaction payload
Gluon->>Gluon: Compute pythFee value
alt routerAddress set
Gluon->>Router: Encode payWithFission(merchant, updateData)
Router-->>Gluon: Encoded txData
else direct fission
Gluon->>GluonC: Encode fission(amountIn, to, updateData)
GluonC-->>Gluon: Encoded txData
end
Gluon-->>Tx: txData with value
else protocol === 'djed'
Tx->>Web3: Use existing buyScTx logic
Web3-->>Tx: txData
end
Tx-->>UI: Return transaction payload
sequenceDiagram
participant UI as TransactionReview Widget
participant Tx as Transaction
participant Gluon as Gluon SDK
participant Contract as Gluon Contract
participant Formatter as formatUnits
UI->>Tx: getBlockchainDetails()
Tx-->>UI: Details including protocol
alt protocol === 'gluon' AND native token
UI->>Contract: Fetch reserve, neutronSupply, protonSupply, fissionFee
Contract-->>UI: Contract state values
UI->>Gluon: calculateFission(amountIn, reserve, neutronSupply, protonSupply, fissionFee, 0, 0)
Gluon-->>UI: {neutronOut, protonOut, feeAmount}
UI->>Formatter: formatUnits(neutronOut, decimals)
Formatter-->>UI: Human-readable neutron amount
UI->>Formatter: formatUnits(protonOut, decimals)
Formatter-->>UI: Human-readable proton amount
UI->>UI: Build success message with amounts
else
UI->>UI: Use default success message
end
UI-->>UI: Display result to user
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
stablepay-sdk/src/widget/TransactionReview.jsx (1)
260-279: Hardcoded explorer URL ignoresgetExplorerUrl()helper.The transaction link at line 264 uses a hardcoded Mordor testnet URL (
https://blockscout.com/etc/mordor/tx/), butgetExplorerUrl()is defined at lines 187-199 to dynamically resolve the correct explorer based onselectedNetwork. This breaks for non-ETC networks.🔎 Proposed fix
{txHash && ( - <div className={styles.transactionLink}> - ✅ Transaction Hash:{" "} - <a - href={`https://blockscout.com/etc/mordor/tx/${txHash}`} - target="_blank" - rel="noopener noreferrer" - className={styles.explorerLink} - style={{ - color: "#007bff", - textDecoration: "underline", - fontWeight: "bold", - cursor: "pointer", - wordBreak: "break-word" - }} - > - {txHash.slice(0, 6)}...{txHash.slice(-6)} - </a> - </div> -)} + <div className={styles.transactionLink}> + ✅ Transaction Hash:{" "} + <a + href={getExplorerUrl()} + target="_blank" + rel="noopener noreferrer" + className={styles.explorerLink} + style={{ + color: "#007bff", + textDecoration: "underline", + fontWeight: "bold", + cursor: "pointer", + wordBreak: "break-word" + }} + > + {txHash.slice(0, 6)}...{txHash.slice(-6)} + </a> + </div> + )}
🧹 Nitpick comments (5)
djed-sdk/src/gluon/gluon.js (1)
6-12: Unused constructor parameters:bondTokenAddressandoracleAddress.These parameters are stored but never referenced in any class method. If they're planned for future use, consider adding a TODO comment; otherwise, remove them to avoid confusion.
djed-sdk/src/gluon/system.js (2)
39-39: Redundant.toString(10)on string literal.
"0".toString(10)is unnecessary;"0"is already a string.🔎 Proposed fix
- const emptyValue = decimalScaling("0".toString(10), BC_DECIMALS); + const emptyValue = decimalScaling("0", BC_DECIMALS);
82-107: Inconsistent error handling across exported functions.
getCoinDetailswraps its logic in try/catch and throws a user-friendly error, butgetSystemParamsandgetAccountDetailshave no error handling. Failed contract calls will propagate raw errors to consumers.Consider applying consistent error handling across all three functions.
Also applies to: 109-141
djed-sdk/src/gluon/tradeUtils.js (1)
187-200: Type inconsistency ingetFeeserror fallback.On success,
fissionFeeandfusionFeeare contract return values (likely BigInt or string from web3). On error, they default to"0"strings. This inconsistency may cause downstream issues if callers expect a specific type.Also,
console.logis used instead ofconsole.errorfor error logging.🔎 Proposed fix
} catch (error) { - console.log("error", error); + console.error("Error fetching fees:", error); return { fissionFee: "0", fusionFee: "0" }; }stablepay-sdk/src/core/Transaction.js (1)
92-117: Consider adding reserve coin trading methods.The
Transactionclass currently only implements stablecoin buying. Given that the Gluon integration includesGluonReserveCoinmodule withbuyRcTxandsellRcTxfunctions, consider adding corresponding methods likebuyReserveCoinsandsellReservecoinsto provide complete trading functionality.Would you like me to generate the implementation for reserve coin trading methods?
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
djed-sdk/src/artifacts/GluconABI.jsondjed-sdk/src/gluon/gluon.jsdjed-sdk/src/gluon/gluonReserveCoin.jsdjed-sdk/src/gluon/gluonStableCoin.jsdjed-sdk/src/gluon/index.jsdjed-sdk/src/gluon/system.jsdjed-sdk/src/gluon/tradeUtils.jsdjed-sdk/src/index.jsstablepay-sdk/src/core/Transaction.jsstablepay-sdk/src/utils/config.jsstablepay-sdk/src/widget/TransactionReview.jsx
🧰 Additional context used
🧬 Code graph analysis (6)
djed-sdk/src/gluon/system.js (1)
djed-sdk/src/helpers.js (6)
scaledUnscaledPromise(84-89)web3Promise(1-3)scaledPromise(80-82)decimalScaling(35-62)percentScaledPromise(99-101)percentageScale(91-97)
djed-sdk/src/gluon/gluonReserveCoin.js (2)
djed-sdk/src/gluon/gluonStableCoin.js (7)
data(21-26)data(53-58)data(81-81)data(91-93)getFees(27-27)getFees(59-59)value(60-64)djed-sdk/src/gluon/tradeUtils.js (11)
tradeDataPriceCore(20-43)tradeDataPriceCore(20-43)getFees(187-201)getFees(187-201)appendFees(173-180)appendFees(173-180)fissionFee(189-192)convertToBC(52-55)convertToBC(52-55)deductFees(161-164)deductFees(161-164)
djed-sdk/src/gluon/gluonStableCoin.js (2)
djed-sdk/src/gluon/tradeUtils.js (11)
tradeDataPriceCore(20-43)tradeDataPriceCore(20-43)getFees(187-201)getFees(187-201)appendFees(173-180)appendFees(173-180)fissionFee(189-192)convertToBC(52-55)convertToBC(52-55)deductFees(161-164)deductFees(161-164)djed-sdk/src/helpers.js (3)
decimalScaling(35-62)buildTx(6-17)web3Promise(1-3)
djed-sdk/src/gluon/gluon.js (2)
djed-sdk/src/helpers.js (3)
web3Promise(1-3)convertInt(18-20)buildTx(6-17)djed-sdk/src/gluon/gluonStableCoin.js (4)
data(21-26)data(53-58)data(81-81)data(91-93)
djed-sdk/src/gluon/tradeUtils.js (3)
djed-sdk/src/helpers.js (4)
decimalUnscaling(64-78)scaledUnscaledPromise(84-89)web3Promise(1-3)decimalScaling(35-62)djed-sdk/src/gluon/gluonReserveCoin.js (1)
value(54-58)djed-sdk/src/gluon/gluonStableCoin.js (1)
value(60-64)
stablepay-sdk/src/core/Transaction.js (3)
djed-sdk/src/gluon/gluon.js (1)
Gluon(5-46)djed-sdk/src/gluon/gluonReserveCoin.js (1)
value(54-58)djed-sdk/src/gluon/gluonStableCoin.js (1)
value(60-64)
🪛 Biome (2.1.2)
stablepay-sdk/src/utils/config.js
[error] 43-43: expected : but instead found djed
Remove djed
(parse)
[error] 43-43: unterminated string literal
The closing quote must be on the same line.
(parse)
[error] 44-44: expected , but instead found thereum
Remove thereum
(parse)
[error] 44-44: expected , but instead found -
Remove -
(parse)
[error] 44-44: unterminated string literal
The closing quote must be on the same line.
(parse)
🔇 Additional comments (10)
stablepay-sdk/src/utils/config.js (1)
4-4: LGTM for the protocol property additions.Adding
protocol: 'djed'to sepolia and milkomeda-mainnet configurations aligns with the new protocol-aware transaction routing.Also applies to: 24-24
djed-sdk/src/gluon/gluon.js (1)
35-45: LGTM for fission and fusion transaction builders.The methods correctly encode ABI data and delegate to
buildTxwith appropriate parameters. Thefissionmethod passesamountInas the transaction value, whilefusioncorrectly sends0value since it operates on existing tokens.djed-sdk/src/gluon/system.js (1)
34-35: Duplicate contract call forprotonPriceInBase.
protonPriceInBaseis fetched at line 35 (asscaledScExchangeRate) and again at lines 46-49 whenunscaledNumberRc !== 0n. SinceneutronPriceInBaseis needed for the sell price (not proton), this appears to be a copy-paste error—the conditional block should fetchneutronPriceInBase(which it does correctly at line 47), but the comment at line 44 says "sell price" for reserve coin, which is correct.However, the initial fetch at line 35 already calls
protonPriceInBase, so consider reusing the already-fetched value if the same data is needed elsewhere, or verify thatneutronPriceInBase(line 47) is indeed distinct fromprotonPriceInBase(line 35).Also applies to: 46-49
djed-sdk/src/gluon/tradeUtils.js (2)
45-55: LGTM forconvertToBCimplementation.The BigInt arithmetic correctly handles precision by dividing by the decimal scaling factor after multiplication.
68-108: LGTM for reserve ratio calculation helpers.
calculateIsRatioBelowMaxandcalculateIsRatioAboveMincorrectly use BigInt for precise financial calculations and include appropriate threshold checks.djed-sdk/src/index.js (1)
12-12: LGTM for Gluon module re-export.The addition follows the existing export patterns in the file and properly exposes the new Gluon functionality to consumers.
stablepay-sdk/src/widget/TransactionReview.jsx (1)
44-48: LGTM for protocol parameter addition.Passing
networkConfig.protocolto the Transaction constructor enables protocol-aware transaction handling for the Gluon integration.djed-sdk/src/gluon/index.js (1)
1-5: LGTM for Gluon module barrel file.Clean module organization with appropriate use of namespace exports (
* as GluonStableCoin,* as GluonReserveCoin) to prevent naming collisions while maintaining a clear API surface.stablepay-sdk/src/core/Transaction.js (2)
77-80: LGTM! Clean protocol-specific routing.The protocol-aware branching cleanly delegates to the appropriate pricing function and maintains consistent return values.
18-31: Null parameters for Gluon constructor are safe and unused.The
bondTokenAddressandoracleAddressparameters passed asnullto the Gluon constructor are stored but never referenced in any of the Gluon class methods (getCoinContracts,getDecimals,fission,fusion). No issues will occur from these null values.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
djed-sdk/src/gluon/gluon.js (1)
11-11: Optional: Standardize ABI access pattern for consistency.Line 11 uses
gluonArtifactdirectly while line 19 usescoinArtifact.abi. This inconsistency works correctly but could cause confusion during maintenance. Consider standardizing both to use the same access pattern.Also applies to: 19-19
djed-sdk/src/gluon/gluonStableCoin.js (1)
89-97: Remove unusedUIparameter.The
UIparameter is declared but never used in the function body. This creates confusion and should be removed.🔎 Proposed fix
-export const sellScTx = (djed, account, amount, UI, DJED_ADDRESS) => { +export const sellScTx = (djed, account, amount, DJED_ADDRESS) => { // fusion(m, to) - // m is likely the amount of Proton to burn? Or amount of Base to receive? - // Assuming m is amount of Proton to burn based on typical burn patterns. const data = djed.methods .fusion(amount, account) .encodeABI(); return buildTx(account, DJED_ADDRESS, 0, data); };Note: Also remove the unused UI parameter from
buyScTxonce the syntax errors are fixed, and update all call sites.djed-sdk/src/gluon/gluonReserveCoin.js (3)
39-41: Fix error handling - avoid returning undefined on failure.The catch block logs the error but doesn't throw it, causing the function to return
undefinedon failure. This will cause issues for calling code expecting a data object.🔎 Proposed fix
} catch (error) { - console.log("error", error); + console.error("tradeDataPriceBuyRc error", error); + throw error; } };
67-69: Same error handling issue astradeDataPriceBuyRc.Apply the same fix to propagate errors instead of returning
undefined.🔎 Proposed fix
} catch (error) { - console.log("error", error); + console.error("tradeDataPriceSellRc error", error); + throw error; } };
72-81: Remove unusedUIparameter and clarify implementation.The
UIparameter is declared but never used. The uncertainty comments (lines 73-76) suggest the implementation needs verification with the contract team. Confirm whetherfissionis the correct method for buying reserve coins and remove the uncertainty comments if so.🔎 Proposed fix for unused parameter
-export const buyRcTx = (djed, account, value, UI, DJED_ADDRESS) => { - // Assuming fission also handles RC buying or there's a specific way. - // If fission splits, this might be tricky. - // For now, using fission as placeholder or if it's the only entry point. - // NOTE: This might need adjustment if there's a specific "buy neutron" path. +export const buyRcTx = (djed, account, value, DJED_ADDRESS) => { const data = djed.methods .fission(value, account, []) .encodeABI(); return buildTx(account, DJED_ADDRESS, value, data); };Note: Update call sites and remove uncertainty comments once verified.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
djed-sdk/src/artifacts/GluonABI.jsondjed-sdk/src/gluon/gluon.jsdjed-sdk/src/gluon/gluonReserveCoin.jsdjed-sdk/src/gluon/gluonStableCoin.jsdjed-sdk/src/gluon/tradeUtils.jsstablepay-sdk/src/utils/config.js
🚧 Files skipped from review as they are similar to previous changes (2)
- stablepay-sdk/src/utils/config.js
- djed-sdk/src/gluon/tradeUtils.js
🧰 Additional context used
🧬 Code graph analysis (1)
djed-sdk/src/gluon/gluonStableCoin.js (3)
djed-sdk/src/gluon/gluonReserveCoin.js (7)
data(21-26)data(46-51)data(77-79)data(84-86)getFees(27-27)getFees(53-53)value(54-58)djed-sdk/src/gluon/tradeUtils.js (9)
tradeDataPriceCore(20-43)tradeDataPriceCore(20-43)getFees(194-208)getFees(194-208)fissionFee(196-199)convertToBC(52-55)convertToBC(52-55)deductFees(168-171)deductFees(168-171)djed-sdk/src/helpers.js (2)
decimalScaling(35-62)buildTx(6-17)
🪛 Biome (2.1.2)
djed-sdk/src/gluon/gluonStableCoin.js
[error] 85-85: Illegal return statement outside of a function
(parse)
[error] 85-86: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
[error] 88-89: expected ) but instead found export
Remove export
(parse)
[error] 89-89: Shouldn't redeclare 'sellScTx'. Consider to delete it or rename it.
'sellScTx' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (3)
djed-sdk/src/gluon/gluonStableCoin.js (1)
109-118: Good improvement - fails fast with clear error message.The function now correctly throws an error instead of returning a placeholder value. This addresses the previous review concern by ensuring callers receive an explicit error rather than silently getting an incorrect price.
djed-sdk/src/artifacts/GluonABI.json (1)
1-825: ABI artifact - no review needed.This is a standard JSON ABI artifact file. The filename appears correctly spelled as "GluonABI.json" (based on the file path), addressing any previous concerns about typos.
djed-sdk/src/gluon/gluon.js (1)
5-45: LGTM! Well-structured Gluon class implementation.The class is well-organized with:
- Proper initialization in the constructor
- Efficient parallel fetching using
Promise.allingetCoinContractsandgetDecimals- Clean transaction builders for
fissionandfusionoperations
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
djed-sdk/src/gluon/gluonStableCoin.js (2)
1-1: Remove unused importTRANSACTION_VALIDITY.The
TRANSACTION_VALIDITYconstant is imported but never used in this file.🔎 Proposed fix
-import { BC_DECIMALS, TRANSACTION_VALIDITY } from "../constants"; +import { BC_DECIMALS } from "../constants";
89-97: Clarify or remove uncertain comments.The unused
UIparameter has been properly removed. However, lines 91-92 contain speculative comments expressing uncertainty about thefusionmethod's parameter semantics. Since the implementation appears to work correctly (consistent withgluonReserveCoin.js), either confirm the semantics and update the comments to be definitive, or remove the uncertain commentary.🔎 Proposed fix
export const sellScTx = (djed, account, amount, DJED_ADDRESS) => { - // fusion(m, to) - // m is likely the amount of Proton to burn? Or amount of Base to receive? - // Assuming m is amount of Proton to burn based on typical burn patterns. + // fusion(amount, to) - burns stablecoins and sends base currency to account const data = djed.methods .fusion(amount, account) .encodeABI(); return buildTx(account, DJED_ADDRESS, 0, data); };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
djed-sdk/src/gluon/gluonStableCoin.js
🧰 Additional context used
🧬 Code graph analysis (1)
djed-sdk/src/gluon/gluonStableCoin.js (3)
djed-sdk/src/gluon/gluonReserveCoin.js (9)
data(21-26)data(46-51)data(77-79)data(84-86)getFees(27-27)getFees(53-53)totalBCUnscaled(29-33)value(54-58)totalBCAmount(60-60)djed-sdk/src/gluon/tradeUtils.js (13)
tradeDataPriceCore(20-43)tradeDataPriceCore(20-43)getFees(194-208)getFees(194-208)appendFees(180-187)appendFees(180-187)fissionFee(196-199)FEE_UI_UNSCALED(16-19)FEE_UI_UNSCALED(16-19)convertToBC(52-55)convertToBC(52-55)deductFees(168-171)deductFees(168-171)djed-sdk/src/helpers.js (2)
decimalScaling(35-62)buildTx(6-17)
🔇 Additional comments (4)
djed-sdk/src/gluon/gluonStableCoin.js (4)
19-43: LGTM! Previous error handling issue resolved.The function correctly computes the total BC required to buy stablecoins, including proper fee calculation and scaling. The error handling has been properly fixed to rethrow errors instead of returning
undefined.
52-77: LGTM! Previous error handling issue resolved.The function correctly computes the total BC received when selling stablecoins, with proper fee deduction and scaling. The error handling has been properly fixed to rethrow errors.
79-87: LGTM! Previous syntax error and unused parameter issues resolved.The function is now correctly implemented with a proper signature and no unused parameters. The logic correctly builds a transaction where the payer funds the purchase and the receiver gets the stablecoins via the fission method.
109-118: LGTM! Placeholder implementation properly replaced with fail-fast error.The previous placeholder that returned an incorrect price has been replaced with a clear error throw. This fail-fast approach ensures callers are immediately aware the functionality is not yet implemented for Gluon, preventing silent bugs from incorrect price calculations.
Zahnentferner
left a comment
Zahnentferner
left a comment
There was a problem hiding this comment.
Gluon and Djed are very different protocols. So, putting Gluon SDK functions in a folder under the Djed SDK is not appropriate. A separate gluon-sdk folder should be created.
Also, Gluon doesn't have buy/sell stablecoin/reservecoin functions. Instead it has fission, fusion and transmutation functions. So, at first sight, it seems that something wrong is being done here. Let's discuss this in Discord.
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 (2)
stablepay-sdk/src/widget/TransactionReview.jsx (1)
284-302: Hardcoded block explorer URL doesn't match network selection.The explorer link is hardcoded to
blockscout.com/etc/mordorbutgetExplorerUrl()(lines 211-223) already handles network-specific URLs. The hardcoded URL will show incorrect results for non-Mordor networks.Use dynamic explorer URL
- <a - href={`https://blockscout.com/etc/mordor/tx/${txHash}`} + <a + href={getExplorerUrl() || `https://blockscout.com/etc/mordor/tx/${txHash}`}stablepay-sdk/src/core/Transaction.js (1)
101-127: Fix API contract mismatch:buyStablecoins()caller passes 4 arguments but method accepts only 3.In
stablepay-sdk/src/widget/TransactionReview.jsx(lines 110-114), the caller invokestransaction.buyStablecoins(account, receiver, parseEther(...), UI)with 4 arguments. However, the method signature instablepay-sdk/src/core/Transaction.js(line 102) isasync buyStablecoins(payer, receiver, value)with only 3 parameters. The UI argument is silently ignored by JavaScript, breaking the API contract.Both locations hardcode the same UI address (
0x0232556C83791b8291E9b23BfEa7d67405Bd9839), which masks the problem, but this creates a confusing and fragile contract. Either the method should accept the UI parameter, or TransactionReview.jsx should stop passing it.
🤖 Fix all issues with AI agents
In @gluon-sdk/package.json:
- Line 9: The package.json "test" script currently uses the placeholder that
always exits with an error; replace it with a real test runner invocation (e.g.,
change the "test" script to run Jest) and add a basic Jest setup: install
devDependencies (jest, ts-jest or babel-jest as appropriate), add any helper
scripts (e.g., "test:watch"), create a minimal Jest config (jest.config.js or
package.json "jest" section), and add a simple example test under a __tests__
directory to validate CI runs; update package.json's "test" script and related
scripts accordingly so CI/test commands no longer fail.
In @gluon-sdk/src/gluon.js:
- Around line 121-148: The bootstrap helper _calculateBootstrapFission currently
returns feeAmount: new this.BN(0) which is wrong because the caller already
computed and deducted feeAmount (see the m.sub(feeAmount) call); change
_calculateBootstrapFission to accept a feeAmount parameter (e.g., feeAmountBN)
or otherwise propagate the already-calculated fee into it and return that BN
instead of zero, and update the caller that invokes _calculateBootstrapFission
(where net is computed as m.sub(feeAmount)) to pass the original feeAmount
variable through so the returned object contains the actual feeAmount BN rather
than a zero placeholder.
🧹 Nitpick comments (3)
stablepay-sdk/src/core/Transaction.js (1)
2-4: Fragile relative import path for gluon-sdk.Using
../../../gluon-sdk/src/gluonis brittle and will break if directory structure changes. Consider:
- Publishing gluon-sdk as an npm package (even if private/local)
- Using workspace references if using monorepo tooling
- At minimum, adding path aliases in bundler config
gluon-sdk/src/gluon.js (2)
82-83: Integer division truncation may cause precision loss.
net.mul(Sn).div(R)andnet.mul(Sp).div(R)perform integer division which truncates. For large values ofnetthis is usually acceptable, but for small transactions the user may receive slightly less than expected.Consider documenting this behavior or adding a minimum output check.
115-118: Slippage buffer mentioned in comment but not implemented.The comment mentions adding "a small buffer (e.g. 1%) for rounding errors/slippage if needed" but the code returns
requiredInputdirectly without any buffer. This may cause transactions to fail if on-chain state changes slightly between calculation and execution.Add configurable slippage buffer
// 2. Calculate Input m = (N * R) / effectiveSupply const requiredInput = N.mul(R).div(effectiveSupply); - // Add a small buffer (e.g. 1%) for rounding errors/slippage if needed - return requiredInput; + // Add 1% buffer for rounding errors/slippage + const buffer = requiredInput.div(new this.BN(100)); // 1% + return requiredInput.add(buffer); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
djed-sdk/src/index.jsgluon-sdk/artifacts/GluonABI.jsongluon-sdk/artifacts/GluonRouterABI.jsongluon-sdk/package.jsongluon-sdk/rollup.config.mjsgluon-sdk/src/gluon.jsgluon-sdk/src/index.jsstablepay-sdk/src/core/Transaction.jsstablepay-sdk/src/utils/config.jsstablepay-sdk/src/widget/TokenDropdown.jsxstablepay-sdk/src/widget/TransactionReview.jsx
✅ Files skipped from review due to trivial changes (1)
- djed-sdk/src/index.js
🔇 Additional comments (14)
gluon-sdk/rollup.config.mjs (1)
1-31: LGTM!The Rollup configuration is well-structured for library bundling. It correctly:
- Externalizes web3 to avoid bundling it
- Provides both ESM and UMD outputs for broad compatibility
- Includes appropriate plugins for resolution, CommonJS transformation, and JSON imports
- Maps web3 to the Web3 global for UMD builds
gluon-sdk/src/index.js (1)
1-1: Barrel export is appropriate.The
src/gluon.jsmodule exists and exports theGluonclass with expected public methods (calculateFission,fission,getCoinContracts,getDecimals, and others). The barrel export pattern correctly re-exports this primary class to consumers.stablepay-sdk/src/widget/TokenDropdown.jsx (1)
27-32: No issues identified.The Transaction constructor signature is correct and properly matches the instantiation in TokenDropdown.jsx. The constructor accepts the four parameters (networkUri, djedAddress, protocol, routerAddress) with appropriate defaults (protocol defaults to 'djed', routerAddress defaults to null).
All network configurations include the
protocolproperty. WhilerouterAddressis only explicitly defined in 'gluon-testnet', this is correct since the routerAddress parameter is only used when protocol is set to 'gluon' (line 22-23 in Transaction.js). For all other networks using 'djed' protocol, routerAddress is undefined and unused, maintaining backward compatibility without issue.gluon-sdk/package.json (1)
19-19: Upgrade web3 to address security vulnerability or migrate to latest version.The web3 dependency at version 1.7.3 (released April 2022) is outdated and contains a known high-severity security vulnerability: CVE-2024-21505 (prototype pollution in web3-utils).
Recommended actions:
- If staying on 1.x: upgrade to 1.10.4 (latest legacy version, likely includes security backports)
- If able to refactor: migrate to 4.16.0 (latest major version), though this requires substantial code changes—callbacks must become async/await, numeric returns are now BigInt, and packages like bzz and shh are removed
Verify which approach aligns with your codebase constraints and timeline.
gluon-sdk/artifacts/GluonRouterABI.json (1)
1-38: ABI structure is valid and complete for current usage.The GluonRouterABI.json is properly formatted and contains all necessary components: constructor,
payWithFission, andgluongetter. The ABI accurately reflects how the contract is used in the SDK—onlypayWithFissionis called with the correct signaturepayWithFission(address merchant, bytes[] updateData), and both constructor and gluon getter are used for initialization and reference access.Events are not defined in this ABI because the SDK does not depend on them. If event monitoring is needed in future, it should be added to the contract and ABI at that time. Update data validation occurs on-chain during the actual fission transaction, as appropriate for the SDK architecture.
stablepay-sdk/src/utils/config.js (2)
63-83: Placeholder addresses and localhost configuration require attention before production use.The
gluon-testnetconfiguration uses:
http://instead ofhttps://- acceptable for localhost dev but should be HTTPS for remote endpoints- Zero addresses (
0x0000...) fordjedAddressand stablecoin - these will cause transaction failures if not replaced- Placeholder
routerAddress(0x1111...) - also needs real deploymentConsider adding runtime validation in the SDK to detect and warn when placeholder addresses are used, preventing confusing errors.
4-4: Good addition of protocol field to existing networks.Adding
protocol: 'djed'explicitly to existing networks enables clean protocol-based branching in Transaction.js.Also applies to: 24-24, 44-44
stablepay-sdk/src/widget/TransactionReview.jsx (1)
44-49: Transaction initialization now includes protocol parameters.The constructor call correctly passes
protocolandrouterAddressfrom network config, enabling protocol-specific logic paths.stablepay-sdk/src/core/Transaction.js (2)
26-27: Parameter order mismatch in getDecimals call.
getCoinContracts()returns{ proton, neutron }, butgetDecimals(proton, neutron)is called with this destructured result. However, looking atgluon.jsline 194-197,getDecimalsexpects(protonContract, neutronContract)and returns{ protonDecimals, neutronDecimals }.The destructuring on line 27 expects
{ protonDecimals, neutronDecimals }which matches. The call is correct.
83-90: Missing early return causes djed path execution for gluon.The gluon branch at lines 83-90 doesn't have a
returnafter line 89. However, looking more carefully, line 89 doesreturn input.toString()so this is actually correct.gluon-sdk/src/gluon.js (2)
158-183: Fission transaction builder looks correct.The method properly:
- Handles both direct contract and router paths
- Calculates total value including Pyth fee
- Returns properly formatted transaction data
One minor note: line 182 is missing a semicolon after the closing brace, though JavaScript ASI handles this.
29-37: Constructor initializes contracts correctly.Router initialization is conditional and BN utility is properly exposed.
gluon-sdk/artifacts/GluonABI.json (2)
1-825: ABI artifact structure is valid and complete.The ABI includes all necessary components:
- Constructor with comprehensive initialization parameters
- View functions for state queries (reserve, prices, token addresses, fees)
- Payable functions for fission/fusion/transmute operations
- Events for tracking operations (Fission, Fusion, Transmute*, PriceUpdated)
- Custom errors for reentrancy and ERC20 failures
143-154: "Tokeon" is a compiled ABI artifact; verify contract name in source Solidity files.Lines 151 and 190 show
internalType: "contract Tokeon"for NEUTRON_TOKEN and PROTON_TOKEN. Since GluonABI.json is an auto-generated artifact and the name appears consistently across both functions, this likely reflects the actual contract name from the Solidity source. However, the source Solidity contract is not present in this repository. Confirm the contract name in the original Solidity source files to verify this is intentional and not a typo.
| "module": "dist/esm/index.js", | ||
| "scripts": { | ||
| "build": "rollup -c", | ||
| "test": "echo \"Error: no test specified\" && exit 1" |
There was a problem hiding this comment.
Address the test script placeholder.
The test script currently exits with an error. This will prevent proper CI/CD validation and test coverage tracking.
Would you like me to help generate a basic test setup using a testing framework like Jest or Mocha?
🤖 Prompt for AI Agents
In @gluon-sdk/package.json at line 9, The package.json "test" script currently
uses the placeholder that always exits with an error; replace it with a real
test runner invocation (e.g., change the "test" script to run Jest) and add a
basic Jest setup: install devDependencies (jest, ts-jest or babel-jest as
appropriate), add any helper scripts (e.g., "test:watch"), create a minimal Jest
config (jest.config.js or package.json "jest" section), and add a simple example
test under a __tests__ directory to validate CI runs; update package.json's
"test" script and related scripts accordingly so CI/test commands no longer
fail.
| _calculateBootstrapFission(netBase, basePriceWad) { | ||
| const net = new this.BN(netBase); | ||
| const price = new this.BN(basePriceWad); | ||
| const WAD = new this.BN("1000000000000000000"); | ||
|
|
||
| if (price.isZero()) throw new Error("Bad price: 0"); | ||
|
|
||
| // depositValueWad = net * price / WAD | ||
| const depositValueWad = net.mul(price).div(WAD); | ||
|
|
||
| // neutronValueWad = depositValueWad / 3 | ||
| const neutronValueWad = depositValueWad.div(new this.BN(3)); | ||
|
|
||
| // baseForNeutronWad = neutronValueWad * WAD / price | ||
| const baseForNeutronWad = neutronValueWad.mul(WAD).div(price); | ||
|
|
||
| const protonBaseWad = net.sub(baseForNeutronWad); | ||
|
|
||
| // In bootstrap, neutronOut is the value (assuming $1 peg) ? | ||
| // Code: return (neutronValueWad, protonBaseWad) | ||
| // Correct, matches Solidity _bootstrapFissionOutputs | ||
|
|
||
| return { | ||
| neutronOut: neutronValueWad, | ||
| protonOut: protonBaseWad, | ||
| feeAmount: new this.BN(0) | ||
| }; | ||
| } |
There was a problem hiding this comment.
Bootstrap fission returns fee amount of zero.
Line 146 returns feeAmount: new this.BN(0) for bootstrap case, but the fee was already deducted from net before _calculateBootstrapFission is called (line 69 passes net which is m.sub(feeAmount)). This is inconsistent - the actual feeAmount calculated at line 60 should be returned instead.
Propagate actual fee to bootstrap result
The calling code should pass the fee amount:
- return this._calculateBootstrapFission(net, basePrice);
+ const result = this._calculateBootstrapFission(net, basePrice);
+ result.feeAmount = feeAmount;
+ return result;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @gluon-sdk/src/gluon.js around lines 121 - 148, The bootstrap helper
_calculateBootstrapFission currently returns feeAmount: new this.BN(0) which is
wrong because the caller already computed and deducted feeAmount (see the
m.sub(feeAmount) call); change _calculateBootstrapFission to accept a feeAmount
parameter (e.g., feeAmountBN) or otherwise propagate the already-calculated fee
into it and return that BN instead of zero, and update the caller that invokes
_calculateBootstrapFission (where net is computed as m.sub(feeAmount)) to pass
the original feeAmount variable through so the returned object contains the
actual feeAmount BN rather than a zero placeholder.
| if (transaction.protocol === 'gluon' && selectedToken.key === 'native') { | ||
| try { | ||
| const amountIn = String(tradeDataBuySc || "0"); | ||
| const reserve = await transaction.gluon.contract.methods.reserve().call(); | ||
| const neutronSupply = await transaction.stableCoin.methods.totalSupply().call(); | ||
| const protonSupply = await transaction.reserveCoin.methods.totalSupply().call(); | ||
| const fissionFee = await transaction.gluon.contract.methods.FISSION_FEE().call(); | ||
|
|
||
| const { neutronOut, protonOut } = transaction.gluon.calculateFission( | ||
| amountIn, reserve, neutronSupply, protonSupply, fissionFee, 0, 0 | ||
| ); | ||
|
|
||
| const nVal = formatUnits(BigInt(neutronOut.toString()), contextTransactionDetails.stableCoinDecimals); | ||
| const pVal = formatUnits(BigInt(protonOut.toString()), contextTransactionDetails.reserveCoinDecimals); | ||
|
|
||
| successMsg = `Payment Sent! Fission Complete. Merchant received ${parseFloat(nVal).toFixed(2)} Neutrons. You kept ${parseFloat(pVal).toFixed(2)} speculative Protons.`; | ||
| } catch(e) { | ||
| console.error("UI Calculation Error", e); | ||
| successMsg = `Payment Sent! Fission Complete. Merchant received Neutrons. You kept Protons.`; | ||
| } | ||
| } |
There was a problem hiding this comment.
Calculation timing issue: querying state after send but before confirmation.
The code fetches reserve, neutronSupply, protonSupply, and fissionFee after sendTransaction returns the hash but before the transaction is confirmed on-chain. This means:
- The values read are pre-transaction state, not the actual outputs
- The calculated
neutronOut/protonOutmay not match what the user actually received calculateFissionis called withpriceTarget=0, basePrice=0which will break bootstrap case logic (division by zero in_calculateBootstrapFission)
Recommended approach: use transaction events
Instead of recalculating, parse the Fission event from the transaction receipt to get actual on-chain values:
- let successMsg = `✅ Transaction sent!`;
- if (transaction.protocol === 'gluon' && selectedToken.key === 'native') {
- try {
- const amountIn = String(tradeDataBuySc || "0");
- const reserve = await transaction.gluon.contract.methods.reserve().call();
- // ... calculations ...
- } catch(e) { ... }
- }
+ let successMsg = `✅ Transaction sent!`;
+ if (transaction.protocol === 'gluon' && selectedToken.key === 'native') {
+ try {
+ const receipt = await publicClient.waitForTransactionReceipt({ hash: txHash });
+ // Parse Fission event from receipt.logs to get actual neutronOut, protonOut
+ // This gives accurate post-transaction values
+ } catch(e) {
+ console.error("Error fetching receipt", e);
+ successMsg = `Payment Sent! Fission Complete.`;
+ }
+ }Committable suggestion skipped: line range outside the PR's diff.
2 participants
Fixes #32
Added gluon protocol and made stablepay accept gluon-based stablecoins.
Summary by CodeRabbit
Release Notes
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.