16/merge prepare pay and send pay into a single pay #31
Conversation
…ETC testnet to Mordor\n\n- Rename ethereum-classic testnet key to mordor-testnet\n- Add ethereum-classic mainnet (chainId 61) with Rivet RPC\n- Set DJED (0xCc3664d7021FD36B1Fe2b136e2324710c8442cCf) and ECSD (0x5A7Ca94F6E969C94bef4CE5e2f90ed9d4891918A)\n- Update explorer links for both networks\n- Add viem chain for ETC mainnet and dynamic wallet switching\n\nFixes DjedAlliance#18
…)\n\n- Add ethereum-classic (chainId 61) with Rivet RPC\n- Set DJED and ECSD addresses\n- Rename testnet key to mordor-testnet\n- Update explorers + dynamic wallet switching\n- Ignore dist/ and local tarballs\n\nFixes DjedAlliance#18
- Remove manual 'Prepare Transaction' button - Auto-trigger transaction preparation when wallet connects - Rename 'Send Transaction' to 'Sign & Send' for clarity - Add preparation state with 'Preparing...' indicator - Prevent duplicate preparation calls - Update messaging to reflect automatic flow Fixes DjedAlliance#16
WalkthroughThe PR adds multi-network support to StablePay SDK by introducing Ethereum Classic Mainnet as a chain option, implementing dynamic network selection via Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Widget as Widget Component
participant WalletCtx as WalletContext
participant ChainCfg as Chain & Config
participant TxReview as TransactionReview
participant Wallet as Wallet Client
User->>Widget: Selects network
Widget->>Widget: Store selection in window.__STABLEPAY_SELECTED_NETWORK__
Widget->>WalletCtx: Render with context
WalletCtx->>WalletCtx: Call getExpectedChain() based on window value
WalletCtx->>ChainCfg: Fetch chain/config for selected network
WalletCtx->>Wallet: Initialize walletClient & publicClient with selected chain
WalletCtx->>Widget: Provide initialized wallet context
User->>TxReview: Navigate to transaction review
TxReview->>TxReview: Auto-trigger prepareTransaction effect (if wallet connected)
TxReview->>Wallet: Prepare transaction for selected network
Wallet-->>TxReview: Return prepared txData
TxReview->>TxReview: Update state (isPreparingTx: false, txData populated)
TxReview->>User: Display "Ready to sign and send transaction"
User->>TxReview: Click "Sign & Send"
TxReview->>Wallet: Sign & send prepared transaction
Wallet-->>TxReview: Return transaction hash
TxReview->>TxReview: Generate explorer link (dynamic per selected network)
TxReview->>User: Display transaction confirmation with link
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
@Zahnentferner I have raised the PR Kindly check |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
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)
8-8: Hardcoded stablecoin contract address may not work for all networks.
STABLECOIN_CONTRACT_ADDRESSis hardcoded, but the SDK now supports multiple networks (Mordor, ETC mainnet, Sepolia, Milkomeda) with different stablecoin addresses defined inconfig.js.Consider using the stablecoin address from the network configuration:
-const STABLECOIN_CONTRACT_ADDRESS = "0xdc86935A9597aA3A9008d2f26232233043091284"; // ... inside the component or function: - to: STABLECOIN_CONTRACT_ADDRESS, + to: contextTransactionDetails.stableCoinAddress || networkConfig.tokens.stablecoin.address,Verify whether this hardcoded address is intentional for a specific use case or should be dynamic.
🧹 Nitpick comments (2)
stablepay-sdk/src/widget/Widget.jsx (1)
29-30: Side effect in render body - move to useEffect.Assigning to
windowduring render is a side effect that violates React's render purity expectations. This runs on every render and could cause timing issues withWalletContextreading stale values.const WidgetWithProviders = ({ onClose, buttonSize, networkSelector, onTransactionComplete }) => { - // Keep a global reference of current selection for WalletContext - window.__STABLEPAY_SELECTED_NETWORK__ = networkSelector?.selectedNetwork || null; + // Keep a global reference of current selection for WalletContext + React.useEffect(() => { + window.__STABLEPAY_SELECTED_NETWORK__ = networkSelector?.selectedNetwork || null; + return () => { + window.__STABLEPAY_SELECTED_NETWORK__ = null; + }; + }, [networkSelector?.selectedNetwork]); + return (This ensures the assignment happens after render, cleans up on unmount, and only re-runs when
selectedNetworkchanges.stablepay-sdk/src/widget/TransactionReview.jsx (1)
82-87: Missing function dependency in useEffect.
prepareTransactionis called inside the effect but not listed in the dependency array. While the internal guards prevent infinite loops, this is a React anti-pattern that could cause subtle bugs.Either wrap
prepareTransactioninuseCallbackand add it to dependencies, or inline the preparation logic:+ const prepareTransaction = useCallback(async () => { + // ... existing logic + }, [account, contextTransactionDetails, transaction, selectedToken, tradeDataBuySc]); + useEffect(() => { if (account && transaction && contextTransactionDetails && !txData && !isPreparingTx) { prepareTransaction(); } - }, [account, transaction, contextTransactionDetails, txData, isPreparingTx]); + }, [account, transaction, contextTransactionDetails, txData, isPreparingTx, prepareTransaction]);
📜 Review details
Configuration used: CodeRabbit UI
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 (5)
stablepay-sdk/src/contexts/WalletContext.jsx(5 hunks)stablepay-sdk/src/contexts/chains.js(1 hunks)stablepay-sdk/src/utils/config.js(1 hunks)stablepay-sdk/src/widget/TransactionReview.jsx(7 hunks)stablepay-sdk/src/widget/Widget.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
stablepay-sdk/src/contexts/WalletContext.jsx (1)
stablepay-sdk/src/contexts/chains.js (4)
etcMainnet(24-42)etcMainnet(24-42)mordor(3-22)mordor(3-22)
stablepay-sdk/src/widget/TransactionReview.jsx (1)
stablepay-sdk/src/contexts/WalletContext.jsx (2)
account(18-18)error(21-21)
🔇 Additional comments (4)
stablepay-sdk/src/utils/config.js (1)
60-78: LGTM!The Ethereum Classic mainnet configuration is well-structured and consistent with the chain definition in
chains.js. The chainId (61), RPC URL, and token configurations align properly.stablepay-sdk/src/contexts/chains.js (1)
24-42: LGTM!The
etcMainnetchain definition follows the correctviemdefineChainstructure with appropriate chain ID (61), RPC URL, and block explorer configuration for Ethereum Classic mainnet.stablepay-sdk/src/widget/TransactionReview.jsx (2)
196-209: LGTM!The explorer URL mapping has been properly extended to support the new networks (
mordor-testnetandethereum-classic), aligning with the multi-network configuration changes.
233-247: UI flow aligns with PR objectives.The "Sign & Send" button correctly replaces the previous two-step flow, and the "Preparing..." state provides good user feedback during auto-preparation. This achieves the PR goal of merging "Prepare Transaction" and "Send Transaction" into a single user action.
| const expectedChainId = (function(){ return (window.__STABLEPAY_SELECTED_NETWORK__ === 'ethereum-classic-mainnet' ? etcMainnet : mordor).id; })(); | ||
| const getExpectedChain = () => { | ||
| const key = window.__STABLEPAY_SELECTED_NETWORK__; | ||
| if (key === 'ethereum-classic-mainnet') return etcMainnet; | ||
| return mordor; | ||
| }; |
There was a problem hiding this comment.
Stale expectedChainId due to one-time computation.
expectedChainId is computed once via IIFE when the component mounts, but getExpectedChain() reads window.__STABLEPAY_SELECTED_NETWORK__ fresh each call. This inconsistency can cause expectedChainId to be stale if the network selection changes after mount.
Consider using getExpectedChain().id directly where needed, or computing expectedChainId consistently:
- const expectedChainId = (function(){ return (window.__STABLEPAY_SELECTED_NETWORK__ === 'ethereum-classic-mainnet' ? etcMainnet : mordor).id; })();
const getExpectedChain = () => {
const key = window.__STABLEPAY_SELECTED_NETWORK__;
if (key === 'ethereum-classic-mainnet') return etcMainnet;
return mordor;
};
+ const expectedChainId = getExpectedChain().id;Note: This still captures the value once. If dynamic updates are needed during the component lifecycle, consider using state or re-reading via the function.
🤖 Prompt for AI Agents
In stablepay-sdk/src/contexts/WalletContext.jsx around lines 24 to 29, the
variable expectedChainId is computed once via an IIFE and can become stale while
getExpectedChain() reads the global selection each call; replace the one-time
IIFE with a consistent approach such as using getExpectedChain().id directly
wherever needed or converting expectedChainId to a function (e.g., () =>
getExpectedChain().id) or derive it from state/useMemo so it reflects changes to
window.__STABLEPAY_SELECTED_NETWORK__; update all usages accordingly to ensure
consistent, up-to-date chain id reading.
| } finally { | ||
| setIsConnecting(false); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
Missing dependency in useCallback causes stale closure.
connectWallet has an empty dependency array but references expectedChainId from the enclosing scope. If the expected chain ID changes, this callback will use the stale value.
Add the dependency or use getExpectedChain().id inside the callback:
- }, []);
+ }, [expectedChainId]);Or refactor to call getExpectedChain().id directly inside the callback to always get the current value.
📝 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.
| }, []); | |
| }, [expectedChainId]); |
🤖 Prompt for AI Agents
In stablepay-sdk/src/contexts/WalletContext.jsx around line 107, the
connectWallet useCallback currently has an empty dependency array but reads
expectedChainId from outer scope causing a stale closure; update the callback to
either include expectedChainId (or the getter getExpectedChain) in its
dependency array, or move the call to compute getExpectedChain().id inside the
callback so it always reads the current value; ensure eslint-hooks rules are
satisfied and adjust any upstream state/refs to avoid unnecessary re-creations.
| const handleChainChanged = async (chainIdHex) => { | ||
| const newChainId = parseInt(chainIdHex, 16); | ||
| setChainId(newChainId); | ||
|
|
||
| if (newChainId !== expectedChainId) { | ||
| setError(`Wrong network detected. Please switch to Mordor Testnet`); | ||
| const expectedId = getExpectedChain().id; | ||
| if (newChainId !== expectedId) { | ||
| setError(`Wrong network detected. Please switch to ${getExpectedChain().name}`); | ||
| return; | ||
| } | ||
|
|
||
| if (window.ethereum) { | ||
| const walletClient = createWalletClient({ chain: mordor, transport: custom(window.ethereum) }); | ||
| const walletClient = createWalletClient({ chain: getExpectedChain(), transport: custom(window.ethereum) }); | ||
| setWalletClient(walletClient); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Event handlers defined inside component without memoization.
handleChainChanged and handleAccountsChanged are recreated on every render but are used with addEventListener. This can cause memory leaks since removeListener won't match the same function reference.
Wrap these handlers with useCallback:
- const handleChainChanged = async (chainIdHex) => {
+ const handleChainChanged = useCallback(async (chainIdHex) => {
const newChainId = parseInt(chainIdHex, 16);
setChainId(newChainId);
// ...
- };
+ }, []);
- const handleAccountsChanged = async (accounts) => {
+ const handleAccountsChanged = useCallback(async (accounts) => {
// ...
- };
+ }, []);Then ensure disconnectWallet has these as dependencies, and listener cleanup is reliable.
📝 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.
| const handleChainChanged = async (chainIdHex) => { | |
| const newChainId = parseInt(chainIdHex, 16); | |
| setChainId(newChainId); | |
| if (newChainId !== expectedChainId) { | |
| setError(`Wrong network detected. Please switch to Mordor Testnet`); | |
| const expectedId = getExpectedChain().id; | |
| if (newChainId !== expectedId) { | |
| setError(`Wrong network detected. Please switch to ${getExpectedChain().name}`); | |
| return; | |
| } | |
| if (window.ethereum) { | |
| const walletClient = createWalletClient({ chain: mordor, transport: custom(window.ethereum) }); | |
| const walletClient = createWalletClient({ chain: getExpectedChain(), transport: custom(window.ethereum) }); | |
| setWalletClient(walletClient); | |
| } | |
| }; | |
| const handleChainChanged = useCallback(async (chainIdHex) => { | |
| const newChainId = parseInt(chainIdHex, 16); | |
| setChainId(newChainId); | |
| const expectedId = getExpectedChain().id; | |
| if (newChainId !== expectedId) { | |
| setError(`Wrong network detected. Please switch to ${getExpectedChain().name}`); | |
| return; | |
| } | |
| if (window.ethereum) { | |
| const walletClient = createWalletClient({ chain: getExpectedChain(), transport: custom(window.ethereum) }); | |
| setWalletClient(walletClient); | |
| } | |
| }, []); |
🤖 Prompt for AI Agents
In stablepay-sdk/src/contexts/WalletContext.jsx around lines 113 to 127, the
event handlers handleChainChanged and handleAccountsChanged are recreated on
every render which breaks removeListener matching and can leak memory; wrap both
handlers in useCallback with correct dependency arrays (including
functions/values they reference like getExpectedChain, setChainId, setError,
createWalletClient, setWalletClient), replace inline definitions with the
memoized callbacks, ensure disconnectWallet and any useEffect that adds
listeners list these callbacks in their dependency arrays, and call
removeListener/unsubscribe with the same memoized function references so cleanup
reliably removes the listeners.
| 'mordor-testnet': { // Mordor Testnet (ETC) | ||
| uri: 'https://rpc.mordor.etccooperative.org', | ||
| chainId: 63, | ||
| djedAddress: '0xD4548F4b6d08852B56cdabC6be7Fd90953179d68', | ||
| tokens: { | ||
| stablecoin: { | ||
| symbol: 'ECSD', | ||
| address: '0xffD4505B3452Dc22f8473616d50503bA9E1710Ac', // Mordor Stablecoin | ||
| address: '0xffD4505B3452Dc22f8473616d50503bA9E1710Ac', | ||
| decimals: 18, | ||
| isDirectTransfer: true | ||
| }, | ||
| native: { | ||
| symbol: 'ETC', | ||
| decimals: 18, | ||
| isNative: true | ||
| } | ||
| }, | ||
| feeUI: 0 | ||
| }, |
There was a problem hiding this comment.
Native token symbol inconsistency with chain definition.
The mordor-testnet native token symbol is ETC here, but in chains.js (line 10), the Mordor chain defines nativeCurrency.symbol as METC. This inconsistency could cause confusion when displaying currency symbols in the UI.
Consider aligning the symbols:
native: {
- symbol: 'ETC',
+ symbol: 'METC',
decimals: 18,
isNative: true
}🤖 Prompt for AI Agents
In stablepay-sdk/src/utils/config.js around lines 41 to 59, the native token
symbol for mordor-testnet is set to "ETC" but the canonical chain definition in
chains.js uses "METC"; update the symbol here to match the chain definition
(change "ETC" to "METC") and search the codebase for any other uses of the
mordor-testnet native symbol to update them consistently; run related UI/display
flows and tests to confirm the change did not break formatting or localization.
1 participant
Resolved #16
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.