Refactor/remove web3 use viem only#95
Open
Rav1Chauhan wants to merge 3 commits intoDjedAlliance:mainfrom
Open
Conversation
📝 WalkthroughWalkthroughThe PR removes the djed-sdk package (many modules and ABIs), eliminates web3 from stablepay-sdk, adds viem-ready ABIs (Djed, ERC20, Oracle), and refactors stablepay-sdk to use viem publicClient/walletClient (notably Transaction.js and TransactionReview.jsx). Multiple helper modules and exports in djed-sdk are deleted. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Transaction as Transaction (core)
participant Public as viem publicClient
participant Wallet as viem walletClient
participant Djed as Djed Contract
App->>Transaction: initialize(selectedNetwork)
Transaction->>Public: createPublicClient / getContract reads (stableCoin, reserveCoin, decimals, oracle)
Public-->>Transaction: contract data (addresses, decimals, prices)
App->>Transaction: buyStablecoins(walletClient, receiver, valueWei)
Transaction->>Djed: read scPrice, fee, treasuryFee (via Public)
Djed-->>Transaction: pricing & fee values
Transaction->>Wallet: walletClient.writeContract(buyStableCoins, args, valueWei)
Wallet-->>Transaction: txHash
Transaction->>Public: waitForTransactionReceipt(txHash)
Public-->>Transaction: receipt / confirmation
Transaction-->>App: success / txHash
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 5
🧹 Nitpick comments (3)
stablepay-sdk/src/core/Transaction.js (2)
18-24: Chain detection is duplicated; extract once duringinit().Lines 19 and 100 both perform the same
this.networkUri.includes('sepolia') ? sepolia : mainnetcheck. Store the result asthis.chainduringinit()and reuse it.♻️ Proposed refactor
async init() { - const chain = this.networkUri.includes('sepolia') ? sepolia : mainnet + this.chain = this.networkUri.includes('sepolia') ? sepolia : mainnet this.publicClient = createPublicClient({ - chain, + chain: this.chain, transport: http(this.networkUri) })Then in
buyStablecoins:- const chain = this.networkUri.includes('sepolia') ? sepolia : mainnet - const walletClient = createWalletClient({ - chain, + chain: this.chain, transport: http(this.networkUri), account: payer })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stablepay-sdk/src/core/Transaction.js` around lines 18 - 24, The chain detection logic is duplicated; in init() compute and assign the chain once (e.g., set this.chain = this.networkUri.includes('sepolia') ? sepolia : mainnet) and then reuse this.chain wherever the same check appears (for example in createPublicClient in init() and inside buyStablecoins), replacing repeated this.networkUri.includes(...) checks with this.chain to avoid duplication and ensure consistent chain selection.
65-93: Misleading variable naming:amountScaled→amountUnscaled.The parameter is named
amountScaledbut is immediately converted toamountUnscaledon line 74 viaBigInt(amountScaled). This is confusing — if the input is a "scaled" amount (human-readable), the BigInt conversion alone doesn't "unscale" it. Clarify the naming to match the actual semantics (e.g., both should use the same term, or include a comment explaining the conversion).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stablepay-sdk/src/core/Transaction.js` around lines 65 - 93, The parameter name amountScaled in handleTradeDataBuySc is misleading because the code treats the input as an integer BigInt (amountUnscaled); either rename the parameter to amountUnscaled (and update all call sites) or perform an explicit unscaling step using scDecimals (e.g., parse a human-readable scaled string and multiply/divide by 10**scDecimals) so naming matches semantics; update the local variable amountUnscaled/scaling logic and add a short inline comment referencing scDecimals to make the conversion clear.stablepay-sdk/rollup.config.mjs (1)
20-27: Stale UMD globals for removed dependencies.
globalsstill maps"djed-sdk"and"web3"(lines 21-22), but these are no longer in theexternalarray and have been removed frompackage.json. These entries are dead config and should be cleaned up.♻️ Remove stale globals
globals: { - "djed-sdk": "DjedSdk", - web3: "Web3", react: "React", "react-dom": "ReactDOM", viem: "viem", "viem/chains": "viemChains", },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stablepay-sdk/rollup.config.mjs` around lines 20 - 27, The rollup `globals` mapping contains stale entries for "djed-sdk" and "web3" that were removed from dependencies; open the rollup `globals` object and remove the "djed-sdk": "DjedSdk" and "web3": "Web3" entries so the UMD build config only lists globals for actual externals (e.g., keep react/ react-dom/viem/viem/chains if still present) and ensure the `globals` keys match the `external` array items used by the build.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@djed-sdk/src/djed/tradeUtils.js`:
- Around line 131-141: Update the source and distribution so the API and imports
reflect the viem migration: remove the now-unused CONFIRMATION_WAIT_PERIOD
import, rebuild the dist (djed-sdk/dist/esm/index.js) so verifyTx’s signature is
published as verifyTx(publicClient, hash) instead of the old (web3, hash), and
ensure the exported verifyTx in the built bundles matches the new function in
djed-sdk/src/djed/tradeUtils.js; also locate getFees and tradeDataPriceCore
(they reference web3Promise) and either migrate them to use viem/publicClient or
mark them for removal/replace with compatible implementations so no web3Promise
dependency remains; finally confirm whether the viem migration is intended or if
djed-sdk should be removed from the repo to avoid publishing mismatched
artifacts.
In `@stablepay-sdk/package.json`:
- Around line 14-18: Update the viem dependency in package.json from "viem":
"^2.21.53" to the current latest (e.g., "viem": "^2.46.1"); modify the
"dependencies" entry for the "viem" package and then run install and test to
ensure compatibility (review changelog for any breaking changes if needed).
In `@stablepay-sdk/src/core/Transaction.js`:
- Line 19: The chain selection using const chain =
this.networkUri.includes('sepolia') ? sepolia : mainnet in the Transaction class
is brittle; update Transaction's constructor to accept an explicit network/chain
parameter (e.g., networkName or chainEnum) and use that to set the chain, or
validate networkUri against a small mapping of known networks (sepolia, mainnet,
etc.) and if no match is found, throw or at minimum log a clear warning before
defaulting; change usages that reference networkUri/chain so they read the
confirmed chain value and ensure network detection logic is centralized (e.g., a
new resolveChain(networkUri, networkName) helper used by the constructor).
- Around line 100-118: The current createWalletClient call in Transaction.js
uses transport: http(this.networkUri) which won't work with browser wallets;
modify the function that calls createWalletClient (the block creating
walletClient and calling writeContract/buyStableCoins) to accept an external
walletClient parameter (preferably the WalletContext-managed walletClient) and
use that when provided, otherwise fall back to creating a client with
custom(window.ethereum) transport for browser environments; update references to
walletClient, createWalletClient, and http to support this optional injected
client and ensure writeContract is invoked on the provided or properly-created
custom-transport client.
- Around line 7-10: BC_DECIMALS and FEE_UI are unused/inconsistent: decide
whether a UI fee should be applied and then either remove the unused constants
or apply FEE_UI consistently. If fee should be 0, delete BC_DECIMALS and FEE_UI;
otherwise set FEE_UI to the intended bigint and (1) use FEE_UI instead of 0n in
the buyStablecoins call and (2) include FEE_UI when computing totalFees inside
handleTradeDataBuySc so totalFees accounts for the UI fee; update any related
comments to reflect the chosen behavior.
---
Nitpick comments:
In `@stablepay-sdk/rollup.config.mjs`:
- Around line 20-27: The rollup `globals` mapping contains stale entries for
"djed-sdk" and "web3" that were removed from dependencies; open the rollup
`globals` object and remove the "djed-sdk": "DjedSdk" and "web3": "Web3" entries
so the UMD build config only lists globals for actual externals (e.g., keep
react/ react-dom/viem/viem/chains if still present) and ensure the `globals`
keys match the `external` array items used by the build.
In `@stablepay-sdk/src/core/Transaction.js`:
- Around line 18-24: The chain detection logic is duplicated; in init() compute
and assign the chain once (e.g., set this.chain =
this.networkUri.includes('sepolia') ? sepolia : mainnet) and then reuse
this.chain wherever the same check appears (for example in createPublicClient in
init() and inside buyStablecoins), replacing repeated
this.networkUri.includes(...) checks with this.chain to avoid duplication and
ensure consistent chain selection.
- Around line 65-93: The parameter name amountScaled in handleTradeDataBuySc is
misleading because the code treats the input as an integer BigInt
(amountUnscaled); either rename the parameter to amountUnscaled (and update all
call sites) or perform an explicit unscaling step using scDecimals (e.g., parse
a human-readable scaled string and multiply/divide by 10**scDecimals) so naming
matches semantics; update the local variable amountUnscaled/scaling logic and
add a short inline comment referencing scDecimals to make the conversion clear.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
stablepay-sdk/dist/esm/index.jsis excluded by!**/dist/**stablepay-sdk/dist/umd/index.jsis excluded by!**/dist/**stablepay-sdk/dist/umd/index.js.mapis excluded by!**/dist/**,!**/*.mapstablepay-sdk/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
djed-sdk/src/djed/tradeUtils.jsstablepay-sdk/package.jsonstablepay-sdk/rollup.config.mjsstablepay-sdk/src/abi/Djed.jsonstablepay-sdk/src/abi/ERC20.jsonstablepay-sdk/src/abi/Oracle.jsonstablepay-sdk/src/core/Transaction.js
Comment on lines
+100
to
+118
| const chain = this.networkUri.includes('sepolia') ? sepolia : mainnet | ||
|
|
||
| return txData; | ||
| } catch (error) { | ||
| console.error("Error executing buyStablecoins transaction: ", error); | ||
| throw error; | ||
| } | ||
| const walletClient = createWalletClient({ | ||
| chain, | ||
| transport: http(this.networkUri), | ||
| account: payer | ||
| }) | ||
|
|
||
| return await walletClient.writeContract({ | ||
| address: this.djedAddress, | ||
| abi: DJED_ABI, | ||
| functionName: 'buyStableCoins', | ||
| args: [ | ||
| receiver, | ||
| 0n, // feeUI if needed | ||
| UI_ADDRESS | ||
| ], | ||
| value: BigInt(valueWei) | ||
| }) |
There was a problem hiding this comment.
walletClient with http transport won't support browser-wallet signing.
createWalletClient with http(this.networkUri) can only send transactions if the RPC endpoint manages private keys (e.g., a local node with unlocked accounts). For browser wallets (MetaMask, etc.), you need custom(window.ethereum) transport or must accept an externally-created walletClient. The WalletContext.jsx already manages a walletClient in React state (line 53) — consider accepting it as a parameter instead of creating a new one.
🐛 Proposed fix: accept walletClient as a parameter
- async buyStablecoins(payer, receiver, valueWei) {
+ async buyStablecoins(walletClient, receiver, valueWei) {
if (!this.djedContract) {
throw new Error("DJED contract is not initialized")
}
- const chain = this.networkUri.includes('sepolia') ? sepolia : mainnet
-
- const walletClient = createWalletClient({
- chain,
- transport: http(this.networkUri),
- account: payer
- })
-
return await walletClient.writeContract({
address: this.djedAddress,
abi: DJED_ABI,
functionName: 'buyStableCoins',
args: [
receiver,
0n, // feeUI if needed
UI_ADDRESS
],
value: BigInt(valueWei)
})
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stablepay-sdk/src/core/Transaction.js` around lines 100 - 118, The current
createWalletClient call in Transaction.js uses transport: http(this.networkUri)
which won't work with browser wallets; modify the function that calls
createWalletClient (the block creating walletClient and calling
writeContract/buyStableCoins) to accept an external walletClient parameter
(preferably the WalletContext-managed walletClient) and use that when provided,
otherwise fall back to creating a client with custom(window.ethereum) transport
for browser environments; update references to walletClient, createWalletClient,
and http to support this optional injected client and ensure writeContract is
invoked on the provided or properly-created custom-transport client.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
stablepay-sdk/src/core/Transaction.js (1)
118-120: RedundantBigInt()conversion.
valueWeiis already abigintfromparseEther()in the caller. Wrapping it inBigInt()is unnecessary but harmless.♻️ Suggested simplification
- value: BigInt(valueWei) + value: valueWei🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stablepay-sdk/src/core/Transaction.js` around lines 118 - 120, In Transaction.js, remove the redundant BigInt conversion around valueWei: the object construction that sets value: BigInt(valueWei) should instead set value: valueWei because valueWei is already a bigint (produced by parseEther()); update the code in the block that builds the transaction object (the value property) to use the existing valueWei variable directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@stablepay-sdk/src/core/Transaction.js`:
- Around line 102-122: The buyStablecoins function currently calls
walletClient.writeContract which sends the transaction immediately (causing the
"prepare" step to actually dispatch a tx) and returns a hash, breaking the flow
in TransactionReview.jsx; change buyStablecoins to build and return a
prepared/unsigned transaction object (do not call writeContract/send it) — e.g.
replace the writeContract call with a prepare/populate call
(walletClient.prepareWriteContract or walletClient.populateTransaction /
construct the tx request) that returns tx data (args, to, value, data) and keep
function signature async buyStablecoins(walletClient, receiver, valueWei);
ensure it still validates this.djedContract and walletClient, return the
prepared tx object, and update the caller (TransactionReview.jsx) to call
walletClient.writeContract or sendTransaction when the user confirms.
In `@stablepay-sdk/src/widget/TransactionReview.jsx`:
- Around line 122-129: tradeDataBuySc is already a wei-denominated amount, so
calling parseEther on it inflates the value by 1e18; replace the parseEther step
and pass the raw BigInt (or convert with BigInt(tradeDataBuySc) if it's a
string) as valueInWei before calling transaction.buyStablecoins; update the code
around tradeDataBuySc, parseEther, valueInWei and the
transaction.buyStablecoins(walletClient, receiver, valueInWei) invocation to use
the direct BigInt value instead of parseEther.
- Around line 253-259: The "You Pay" value is showing raw base units (wei)
because tradeDataBuySc is not formatted; convert tradeDataBuySc to a
human-readable amount before rendering (use a utility like
ethers.utils.formatUnits(tradeDataBuySc, decimals) or your existing token
formatting helper) and fall back to a friendly placeholder when undefined;
update the render that references selectedToken, tradeDataBuySc and
contextTransactionDetails.tokenSymbol to display the formatted value (use
contextTransactionDetails.tokenDecimals or the token's decimals) instead of the
raw string so users see e.g. "1.234 ETH" rather than "1234567890000000000".
- Around line 202-205: The current code calls freshWalletClient.sendTransaction
with txData but txData may represent a contract call (contains address, abi,
functionName, args, value) — switch to using freshWalletClient.writeContract for
contract interactions and keep sendTransaction for raw transfers; detect a
contract call by checking for fields like address and functionName (or abi) on
txData, pass the call parameters (address, abi, functionName, args, value) to
writeContract, and for plain native transfers pass to sendTransaction the
expected raw fields (to, data, value) after applying the Transaction.js change
that now returns call data instead of executing. Ensure you update the call site
where txHash is assigned so it awaits writeContract for contract calls and
sendTransaction otherwise, and preserve account usage when required.
---
Nitpick comments:
In `@stablepay-sdk/src/core/Transaction.js`:
- Around line 118-120: In Transaction.js, remove the redundant BigInt conversion
around valueWei: the object construction that sets value: BigInt(valueWei)
should instead set value: valueWei because valueWei is already a bigint
(produced by parseEther()); update the code in the block that builds the
transaction object (the value property) to use the existing valueWei variable
directly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
djed-sdk/dist/esm/index.jsis excluded by!**/dist/**djed-sdk/dist/umd/index.jsis excluded by!**/dist/**djed-sdk/package-lock.jsonis excluded by!**/package-lock.jsonstablepay-sdk/dist/esm/index.jsis excluded by!**/dist/**stablepay-sdk/dist/umd/index.jsis excluded by!**/dist/**stablepay-sdk/dist/umd/index.js.mapis excluded by!**/dist/**,!**/*.mapstablepay-sdk/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (22)
djed-sdk/.gitignoredjed-sdk/.npmignoredjed-sdk/package.jsondjed-sdk/rollup.config.mjsdjed-sdk/src/artifacts/CoinABI.jsondjed-sdk/src/artifacts/DjedABI.jsondjed-sdk/src/artifacts/OracleABI.jsondjed-sdk/src/constants.jsdjed-sdk/src/djed/djed.jsdjed-sdk/src/djed/index.jsdjed-sdk/src/djed/reserveCoin.jsdjed-sdk/src/djed/stableCoin.jsdjed-sdk/src/djed/system.jsdjed-sdk/src/djed/tradeUtils.jsdjed-sdk/src/helpers.jsdjed-sdk/src/index.jsdjed-sdk/src/oracle/index.jsdjed-sdk/src/oracle/oracle.jsdjed-sdk/src/web3.jsstablepay-sdk/package.jsonstablepay-sdk/src/core/Transaction.jsstablepay-sdk/src/widget/TransactionReview.jsx
💤 Files with no reviewable changes (19)
- djed-sdk/src/oracle/index.js
- djed-sdk/.npmignore
- djed-sdk/src/index.js
- djed-sdk/src/constants.js
- djed-sdk/src/djed/reserveCoin.js
- djed-sdk/src/djed/djed.js
- djed-sdk/src/djed/stableCoin.js
- djed-sdk/src/artifacts/CoinABI.json
- djed-sdk/src/artifacts/OracleABI.json
- djed-sdk/src/djed/system.js
- djed-sdk/src/artifacts/DjedABI.json
- djed-sdk/src/djed/index.js
- djed-sdk/src/web3.js
- djed-sdk/src/djed/tradeUtils.js
- djed-sdk/package.json
- djed-sdk/.gitignore
- djed-sdk/src/helpers.js
- djed-sdk/src/oracle/oracle.js
- djed-sdk/rollup.config.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- stablepay-sdk/package.json
Comment on lines
+102
to
122
| async buyStablecoins(walletClient, receiver, valueWei) { | ||
| if (!this.djedContract) { | ||
| throw new Error("DJED contract is not initialized"); | ||
| throw new Error("DJED contract is not initialized") | ||
| } | ||
| try { | ||
| const UI = '0x0232556C83791b8291E9b23BfEa7d67405Bd9839'; | ||
|
|
||
| const txData = await buyScTx(this.djedContract, payer, receiver, value, UI, this.djedAddress); | ||
|
|
||
| return txData; | ||
| } catch (error) { | ||
| console.error("Error executing buyStablecoins transaction: ", error); | ||
| throw error; | ||
| if (!walletClient) { | ||
| throw new Error("Wallet client is required") | ||
| } | ||
|
|
||
| return await walletClient.writeContract({ | ||
| address: this.djedAddress, | ||
| abi: DJED_ABI, | ||
| functionName: 'buyStableCoins', | ||
| args: [ | ||
| receiver, | ||
| 0n, // feeUI if needed | ||
| UI_ADDRESS | ||
| ], | ||
| value: BigInt(valueWei) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Transaction flow mismatch with TransactionReview — buyStablecoins executes immediately.
walletClient.writeContract() sends the transaction immediately and returns the transaction hash, not transaction data. However, in TransactionReview.jsx, the result is stored as txData (line 176), then the UI shows "Transaction ready!" and waits for the user to click "Send Transaction", which then tries to call sendTransaction(txData) on a hash string.
This creates two problems:
- The transaction is already sent during "prepare" phase, before user confirmation
- The subsequent
sendTransactioncall will fail or behave unexpectedly
Consider returning prepared transaction data instead and letting the caller execute it:
🐛 Proposed fix to return transaction data instead of executing
async buyStablecoins(walletClient, receiver, valueWei) {
if (!this.djedContract) {
throw new Error("DJED contract is not initialized")
}
if (!walletClient) {
throw new Error("Wallet client is required")
}
- return await walletClient.writeContract({
+ // Return transaction data for the caller to execute
+ return {
address: this.djedAddress,
abi: DJED_ABI,
functionName: 'buyStableCoins',
args: [
receiver,
0n, // feeUI if needed
UI_ADDRESS
],
value: BigInt(valueWei)
- })
+ }
}Then in TransactionReview.jsx, use writeContract instead of sendTransaction for contract calls.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stablepay-sdk/src/core/Transaction.js` around lines 102 - 122, The
buyStablecoins function currently calls walletClient.writeContract which sends
the transaction immediately (causing the "prepare" step to actually dispatch a
tx) and returns a hash, breaking the flow in TransactionReview.jsx; change
buyStablecoins to build and return a prepared/unsigned transaction object (do
not call writeContract/send it) — e.g. replace the writeContract call with a
prepare/populate call (walletClient.prepareWriteContract or
walletClient.populateTransaction / construct the tx request) that returns tx
data (args, to, value, data) and keep function signature async
buyStablecoins(walletClient, receiver, valueWei); ensure it still validates
this.djedContract and walletClient, return the prepared tx object, and update
the caller (TransactionReview.jsx) to call walletClient.writeContract or
sendTransaction when the user confirms.
Comment on lines
122
to
129
| const amountToSend = tradeDataBuySc || "0"; | ||
| const valueInWei = parseEther(String(amountToSend)); | ||
|
|
||
| builtTx = await transaction.buyStablecoins( | ||
| account, | ||
| walletClient, | ||
| receiver, | ||
| valueInWei, | ||
| UI | ||
| valueInWei | ||
| ); |
There was a problem hiding this comment.
parseEther used on value already in wei — causes ~10¹⁸× value inflation.
tradeDataBuySc from handleTradeDataBuySc() returns the calculated amount in base units (wei). Using parseEther() on this value interprets it as ETH and multiplies by 10¹⁸, resulting in astronomically incorrect transaction values.
For example, if the user should pay 1 ETH (~1000000000000000000 wei), parseEther("1000000000000000000") produces 1000000000000000000000000000000000000n.
🐛 Proposed fix to use the value directly as BigInt
const amountToSend = tradeDataBuySc || "0";
- const valueInWei = parseEther(String(amountToSend));
+ const valueInWei = BigInt(amountToSend);
builtTx = await transaction.buyStablecoins(
walletClient,
receiver,
valueInWei
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const amountToSend = tradeDataBuySc || "0"; | |
| const valueInWei = parseEther(String(amountToSend)); | |
| builtTx = await transaction.buyStablecoins( | |
| account, | |
| walletClient, | |
| receiver, | |
| valueInWei, | |
| UI | |
| valueInWei | |
| ); | |
| const amountToSend = tradeDataBuySc || "0"; | |
| const valueInWei = BigInt(amountToSend); | |
| builtTx = await transaction.buyStablecoins( | |
| walletClient, | |
| receiver, | |
| valueInWei | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stablepay-sdk/src/widget/TransactionReview.jsx` around lines 122 - 129,
tradeDataBuySc is already a wei-denominated amount, so calling parseEther on it
inflates the value by 1e18; replace the parseEther step and pass the raw BigInt
(or convert with BigInt(tradeDataBuySc) if it's a string) as valueInWei before
calling transaction.buyStablecoins; update the code around tradeDataBuySc,
parseEther, valueInWei and the transaction.buyStablecoins(walletClient,
receiver, valueInWei) invocation to use the direct BigInt value instead of
parseEther.
Comment on lines
202
to
205
| const txHash = await freshWalletClient.sendTransaction({ | ||
| ...txData, | ||
| account: account, | ||
| account, | ||
| }); |
There was a problem hiding this comment.
sendTransaction used for contract calls; writeContract required for native buy path.
For the native token path, txData contains contract call parameters (address, abi, functionName, args, value) but sendTransaction expects raw transaction fields (to, data, value). Once the Transaction.js fix is applied to return call data instead of executing, this code should use writeContract for contract interactions:
🐛 Proposed fix: differentiate contract calls from transfers
setMessage("⏳ Sending transaction...");
- const txHash = await freshWalletClient.sendTransaction({
- ...txData,
- account,
- });
+ let txHash;
+ if (txData.abi && txData.functionName) {
+ // Contract call (native buy path)
+ txHash = await freshWalletClient.writeContract({
+ ...txData,
+ account,
+ });
+ } else {
+ // Raw transaction (stablecoin transfer path)
+ txHash = await freshWalletClient.sendTransaction({
+ ...txData,
+ account,
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const txHash = await freshWalletClient.sendTransaction({ | |
| ...txData, | |
| account: account, | |
| account, | |
| }); | |
| let txHash; | |
| if (txData.abi && txData.functionName) { | |
| // Contract call (native buy path) | |
| txHash = await freshWalletClient.writeContract({ | |
| ...txData, | |
| account, | |
| }); | |
| } else { | |
| // Raw transaction (stablecoin transfer path) | |
| txHash = await freshWalletClient.sendTransaction({ | |
| ...txData, | |
| account, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stablepay-sdk/src/widget/TransactionReview.jsx` around lines 202 - 205, The
current code calls freshWalletClient.sendTransaction with txData but txData may
represent a contract call (contains address, abi, functionName, args, value) —
switch to using freshWalletClient.writeContract for contract interactions and
keep sendTransaction for raw transfers; detect a contract call by checking for
fields like address and functionName (or abi) on txData, pass the call
parameters (address, abi, functionName, args, value) to writeContract, and for
plain native transfers pass to sendTransaction the expected raw fields (to,
data, value) after applying the Transaction.js change that now returns call data
instead of executing. Ensure you update the call site where txHash is assigned
so it awaits writeContract for contract calls and sendTransaction otherwise, and
preserve account usage when required.
Comment on lines
+253
to
259
| <span> | ||
| {selectedToken.key === "stablecoin" | ||
| ? `${contextTransactionDetails.amount} ${contextTransactionDetails.tokenSymbol}` | ||
| : `${tradeDataBuySc ? tradeDataBuySc : "Calculating..."} ${ | ||
| : `${tradeDataBuySc || "Calculating..."} ${ | ||
| contextTransactionDetails.tokenSymbol | ||
| }`} | ||
| </span> |
There was a problem hiding this comment.
"You Pay" displays raw wei value instead of human-readable format.
tradeDataBuySc is in base units (wei). Displaying it directly shows values like "1234567890000000000" instead of "1.234" ETH. This creates a confusing user experience.
🐛 Proposed fix to format the display value
+import { formatEther } from "viem";Then in the render:
<span>
{selectedToken.key === "stablecoin"
? `${contextTransactionDetails.amount} ${contextTransactionDetails.tokenSymbol}`
- : `${tradeDataBuySc || "Calculating..."} ${
+ : `${tradeDataBuySc ? formatEther(BigInt(tradeDataBuySc)) : "Calculating..."} ${
contextTransactionDetails.tokenSymbol
}`}
</span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <span> | |
| {selectedToken.key === "stablecoin" | |
| ? `${contextTransactionDetails.amount} ${contextTransactionDetails.tokenSymbol}` | |
| : `${tradeDataBuySc ? tradeDataBuySc : "Calculating..."} ${ | |
| : `${tradeDataBuySc || "Calculating..."} ${ | |
| contextTransactionDetails.tokenSymbol | |
| }`} | |
| </span> | |
| <span> | |
| {selectedToken.key === "stablecoin" | |
| ? `${contextTransactionDetails.amount} ${contextTransactionDetails.tokenSymbol}` | |
| : `${tradeDataBuySc ? formatEther(BigInt(tradeDataBuySc)) : "Calculating..."} ${ | |
| contextTransactionDetails.tokenSymbol | |
| }`} | |
| </span> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@stablepay-sdk/src/widget/TransactionReview.jsx` around lines 253 - 259, The
"You Pay" value is showing raw base units (wei) because tradeDataBuySc is not
formatted; convert tradeDataBuySc to a human-readable amount before rendering
(use a utility like ethers.utils.formatUnits(tradeDataBuySc, decimals) or your
existing token formatting helper) and fall back to a friendly placeholder when
undefined; update the render that references selectedToken, tradeDataBuySc and
contextTransactionDetails.tokenSymbol to display the formatted value (use
contextTransactionDetails.tokenDecimals or the token's decimals) instead of the
raw string so users see e.g. "1.234 ETH" rather than "1234567890000000000".
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.
Addressed Issues:
Fixes #59
Summary
This PR refactors the StablePay SDK to remove legacy web3.js and djed-sdk dependencies and migrate all blockchain interactions to viem.
The SDK now uses a single, consistent blockchain interaction layer based entirely on viem, significantly reducing bundle size and improving performance.
What Was Changed
Removed web3 dependency
Removed djed-sdk dependency
Replaced all contract reads with viem publicClient.read
Replaced all contract writes with viem walletClient.writeContract
Introduced direct ABI-based contract interaction
Added:
src/abi/Djed.json
src/abi/ERC20.json
src/abi/Oracle.json
Updated Rollup configuration
Cleaned external dependencies
Technical Improvements
Eliminates duplicate Ethereum libraries
Reduces final bundle size significantly
Improves execution speed (viem is lighter and faster than web3.js)
Ensures consistent developer experience across the codebase
Removes provider ambiguity
Bundle Optimization Impact
Before:
web3.js + viem bundled
~300KB+ unnecessary weight
After:
viem only
Clean, lightweight SDK
Screenshots/Recordings:
N/A (Refactor only – no UI changes)
Additional Notes:
Additional Notes
Build verified successfully
dist output contains no web3 or djed-sdk
All existing transaction flows tested
No breaking changes to public SDK API
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
New Features
Refactor
Bug Fixes