Conversation
WalkthroughThis PR refactors Pulse buy/sell flows, adds auto-refresh logic, integrates a transaction kit with gas estimation for sells, extends sell-offer data, introduces a Tooltip component and a gas-estimation hook, adjusts tests and UI text, filters “pulse-sell” batches from global batch UIs/counters, and removes the react-spinners dependency. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Home as HomeScreen
participant Buy as Buy
participant Intent as Intent SDK
participant Prev as PreviewBuy
participant Tracker as IntentTracker
User->>Home: Open Buy tab / set amount
Home->>Buy: setUsdAmount, setDispensableAssets, setBuyRefreshCallback
Buy->>Buy: refreshBuyIntent (guarded, debounced)
Buy->>Intent: expressIntent(UserIntent{slippage 3%, deadline 60s})
Intent-->>Buy: ExpressIntentResponse | error
Buy-->>Home: setBuyRefreshCallback(refreshBuyIntent)
Home->>Prev: props{expressIntentResponse, usdAmount, dispensableAssets}
loop every 15s (not preview)
Home->>Buy: buyRefreshCallback()
end
User->>Prev: Confirm
Prev->>Intent: shortlistBid / refresh intent
Intent-->>Prev: bid accepted | error
Prev->>Tracker: start tracking (if applicable)
Tracker-->>User: status updates
sequenceDiagram
autonumber
actor User
participant Home as HomeScreen
participant SellPrev as PreviewSell
participant Relay as useRelaySell
participant Kit as Transaction Kit
participant Gas as useGasEstimation
User->>Home: Open Sell preview
Home->>SellPrev: props{sellOffer, tokenAmount}
SellPrev->>Gas: estimateGasFees()
Gas->>Relay: buildSellTransactions()
Relay-->>Gas: transactions
Gas->>Kit: create batch + estimateBatches
Kit-->>Gas: totalCost (wei) -> native
Gas-->>SellPrev: gasCostNative, symbol
loop every 15s (idle)
SellPrev->>Relay: getBestSellOffer()
SellPrev->>Gas: estimateGasFees()
end
User->>SellPrev: Confirm
SellPrev->>Relay: executeSell (build batch)
SellPrev->>Kit: sendBatches({only pulse-sell batch})
Kit-->>SellPrev: userOpHash | rejection
SellPrev->>Kit: cleanup pulse-sell batch
SellPrev-->>User: success/close or error
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
Deploying x with
|
| Latest commit: |
2bc45df
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a3da34a9.x-e62.pages.dev |
| Branch Preview URL: | https://pro-3679-pulse-preview-scree.x-e62.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (16)
src/components/BottomMenuModal/SendModal/SendModalBatchesTabView.tsx (2)
81-83: Use a common predicate for pulse‑sell exclusion.Same rule as provider; avoid ad‑hoc string checks.
- // Skip pulse-sell batches - they are handled directly in the Pulse app - if (batchName.includes('pulse-sell')) return; + // Skip pulse-sell batches - handled directly in the Pulse app + const isPulseSellBatch = (name: string) => /^pulse-sell(?:-|$)/i.test(name); + if (isPulseSellBatch(batchName)) return;
729-733: De‑duplicate the render‑time filter and keep logic consistent.Reuse the same predicate here to prevent drift.
- ({ batchName }) => - batchName && - batchName !== 'batch-undefined' && - !batchName.includes('pulse-sell') + ({ batchName }) => + batchName && + batchName !== 'batch-undefined' && + !/^pulse-sell(?:-|$)/i.test(batchName)You can also skip
batch-undefinedearlier when grouping to avoid carrying dead entries forward.src/apps/pulse/hooks/tests/useRelaySell.test.tsx (1)
512-517: Make numeric assertions tolerant to rounding.Floating math/formatting changes can make this brittle; prefer close‑match.
- expect(sellOffer).toEqual({ - tokenAmountToReceive: 99.0, - minimumReceive: 98.01, // 99.0 * 0.99 (platform fee applied to minimum amount) - slippageTolerance: 0.03, - priceImpact: undefined, // No price impact in mock data - offer: mockSellOffer.offer, - }); + expect(sellOffer?.tokenAmountToReceive).toBeCloseTo(99.0, 6); + expect(sellOffer?.minimumReceive).toBeCloseTo(98.01, 6); // 99.0 * 0.99 + expect(sellOffer?.slippageTolerance).toBe(0.03); + expect(sellOffer?.priceImpact).toBeUndefined(); + expect(sellOffer?.offer).toEqual(mockSellOffer.offer);src/apps/pulse/components/Buy/PayingToken.tsx (1)
14-23: Minor a11y/perf nits for images.
- Mark the chain badge as decorative or give a descriptive alt.
- Enable lazy loading.
- <img + <img src={payingToken.logo} alt="Main" className="w-8 h-8 rounded-full" + loading="lazy" /> - <img + <img src={getLogoForChainId(payingToken.chainId)} - className="absolute -bottom-px right-0.5 w-3 h-3 rounded-full border border-[#1E1D24]" - alt="Chain logo" + className="absolute -bottom-px right-0.5 w-3 h-3 rounded-full border border-[#1E1D24]" + alt="" + aria-hidden="true" + loading="lazy" />src/apps/pulse/components/Misc/Tooltip.tsx (3)
56-59: Bug: no‑op transform replace.The replace calls don’t change the string and are confusing. Remove them; keep the horizontal transform as computed.
- transform = transform - .replace('translateX', 'translateX') - .replace('translateY', 'translateY'); + // Keep existing horizontal transform; no vertical translation needed.
76-81: Avoid orphaned timeouts; prefer rAF for measurement.Use requestAnimationFrame and cancel on unmount/visibility change.
- useEffect(() => { - if (isVisible) { - // Small delay to ensure tooltip is rendered before calculating position - setTimeout(calculatePosition, 0); - } - }, [isVisible]); + useEffect(() => { + if (!isVisible) return; + let raf = requestAnimationFrame(calculatePosition); + return () => cancelAnimationFrame(raf); + }, [isVisible]);
83-109: Optional: reposition on scroll/resize; cap width and add role.Improves UX for long hovers and accessibility.
- return ( - <div className="relative inline-block"> + useEffect(() => { + if (!isVisible) return; + const onMove = () => calculatePosition(); + window.addEventListener('scroll', onMove, true); + window.addEventListener('resize', onMove); + return () => { + window.removeEventListener('scroll', onMove, true); + window.removeEventListener('resize', onMove); + }; + }, [isVisible]); + + return ( + <div className="relative inline-block"> <div ref={triggerRef} onMouseEnter={handleMouseEnter} onMouseLeave={handleMouseLeave} className="cursor-help" > {children} </div> {isVisible && ( <div ref={tooltipRef} - className="fixed z-50 w-max max-w-[220px] h-auto border border-white/5 bg-[#25232D] p-2 rounded" + role="tooltip" + className="fixed z-50 w-max max-w-[240px] h-auto border border-white/5 bg-[#25232D] p-2 rounded" style={{ top: position.top, left: position.left, transform: position.transform, }} >src/apps/pulse/hooks/useRelaySell.ts (2)
235-259: Consider improving price impact calculation accuracy.The price impact calculation has a potential issue when
currencyIn.amountUsdis missing. The fallback calculation on line 249-253 multiplies the input amount by a potentially undefined USD value, which could result inNaNor 0.- const tokenAmountInUsd = - parseFloat(fromAmount) * - (quote.details?.currencyIn?.amountUsd - ? parseFloat(quote.details.currencyIn.amountUsd) - : 0); + // Get USD price per token from the quote or token data + const tokenPriceUsd = quote.details?.currencyIn?.amountUsd + ? parseFloat(quote.details.currencyIn.amountUsd) / parseFloat(fromAmount) + : 0; + const tokenAmountInUsd = parseFloat(fromAmount) * tokenPriceUsd;
229-232: Platform fee: implemented — add UI disclosure and unify numeric handling
- Verified: 1% is applied in the code (quote display uses usdcAmount * 0.99; execution uses integer math: actualUsdcReceived * 1 / 100). See src/apps/pulse/hooks/useRelaySell.ts:229-232 and 346-351; tests assert the expected result (src/apps/pulse/hooks/tests/useRelaySell.test.tsx:513-514).
- Action: Add a clear user-facing disclosure about the 1% platform fee in the sell preview UI (e.g., near "Gas fee" in src/apps/pulse/components/Sell/PreviewSell.tsx:624-636 or via translations).
- Recommendation (optional): Use consistent BigInt/wei-based arithmetic for user-visible estimates instead of float multiplication to avoid rounding drift between quoted and executed amounts.
src/apps/pulse/hooks/useGasEstimation.ts (2)
68-73: Consider more robust batch cleanup.The current cleanup silently swallows errors, which is fine, but consider logging warnings for debugging purposes when cleanup fails unexpectedly.
try { kit.batch({ batchName }).remove(); } catch (cleanupErr) { // Batch may not exist, which is fine + if (process.env.NODE_ENV === 'development') { + console.debug(`Batch cleanup info: ${cleanupErr}`); + } }
141-155: Consider debouncing the gas estimation trigger.The effect triggers on every change to
sellOffer,sellToken,kit, ortokenAmount. For better performance and reduced API calls, consider debouncing the estimation, especially fortokenAmountchanges.useEffect(() => { + const debounceTimer = setTimeout(() => { if ( sellOffer && sellToken && kit && tokenAmount && estimateGasFeesRef.current ) { estimateGasFeesRef.current(); } + }, 500); // 500ms debounce + + return () => clearTimeout(debounceTimer); }, [sellOffer, sellToken, kit, tokenAmount]);src/apps/pulse/components/Buy/Buy.tsx (1)
141-195: Solid refresh logic with proper guards.The
refreshBuyIntentimplementation has good defensive programming:
- Loading state guard prevents concurrent refreshes
- Comprehensive validation of prerequisites
- Proper error handling with state cleanup
One minor suggestion: consider adding a more specific error type check.
} catch (error) { console.error('express intent failed:: ', error); + // Optionally track specific error types for analytics + if (error instanceof Error && error.message.includes('liquidity')) { + setNoEnoughLiquidity(true); + } setExpressIntentResponse(null);src/apps/pulse/components/Buy/PreviewBuy.tsx (1)
101-110: Clear errors before checking themThe
clearError()call on line 104 should ideally happen after checking if there's an error to avoid unnecessary function calls. Also, the function explicitly returnsundefinedwhich is unnecessary in React effects.- // Clear errors and reset transaction states when amount, token, or quote props change - useEffect(() => { - if (error) { - clearError(); - } - // Reset transaction states when new data comes in - setIsTransactionRejected(false); - setIsWaitingForSignature(false); - return undefined; - }, [usdAmount, buyToken, expressIntentResponse, clearError, error]); + // Clear errors and reset transaction states when amount, token, or quote props change + useEffect(() => { + if (error) { + clearError(); + } + // Reset transaction states when new data comes in + setIsTransactionRejected(false); + setIsWaitingForSignature(false); + }, [usdAmount, buyToken, expressIntentResponse, clearError, error]);src/apps/pulse/components/App/HomeScreen.tsx (1)
136-158: Consider consolidating auto-refresh intervalsThe two auto-refresh effects could potentially be consolidated into a single effect to reduce duplication and improve maintainability.
- // Auto-refresh when in sell mode every 15 seconds - useEffect(() => { - if (!isBuy && sellToken && tokenAmount && isInitialized) { - const interval = setInterval(() => { - handleRefresh(); - }, 15000); // 15 seconds - - return () => clearInterval(interval); - } - return undefined; - }, [isBuy, sellToken, tokenAmount, isInitialized, handleRefresh]); - - // Auto-refresh when in buy mode every 15 seconds - useEffect(() => { - if (isBuy && buyRefreshCallback && !previewBuy) { - const interval = setInterval(() => { - handleRefresh(); - }, 15000); // 15 seconds - - return () => clearInterval(interval); - } - return undefined; - }, [isBuy, buyRefreshCallback, previewBuy, handleRefresh]); + // Auto-refresh every 15 seconds based on the current mode + useEffect(() => { + const shouldAutoRefresh = + (!isBuy && sellToken && tokenAmount && isInitialized) || + (isBuy && buyRefreshCallback && !previewBuy); + + if (!shouldAutoRefresh) { + return undefined; + } + + const interval = setInterval(() => { + handleRefresh(); + }, 15000); // 15 seconds + + return () => clearInterval(interval); + }, [isBuy, sellToken, tokenAmount, isInitialized, buyRefreshCallback, previewBuy, handleRefresh]);src/apps/pulse/components/Sell/PreviewSell.tsx (2)
139-147: Duplicate cleanup effect detectedThere are two identical cleanup effects for unmounting (lines 139-147 and 200-208). This appears to be unintentional duplication.
Remove the duplicate cleanup effect:
- // Clean up pulse-sell batch when component unmounts or preview closes - useEffect(() => { - return () => { - if (sellToken) { - cleanupBatch(sellToken.chainId, 'unmount'); - } - }; - }, [sellToken, cleanupBatch]);Also applies to: 200-208
173-197: Consider removing commented-out codeThe commented-out
getTokenBalancefunction (lines 173-197) should be removed if it's no longer needed. If it might be needed in the future, consider tracking it in a separate issue instead.- // TO DO - check if needed in the future - // // Get the user's balance for the selected token - // const getTokenBalance = () => { - // try { - // if (!sellToken || !walletPortfolioData?.result?.data?.assets) return 0; - // // Find the asset in the portfolio - // const assetData = walletPortfolioData.result.data.assets.find( - // (asset) => asset.asset.symbol === sellToken.symbol - // ); - - // if (!assetData) return 0; - - // // Find the contract balance for the specific token address and chain - // const contractBalance = assetData.contracts_balances.find( - // (contract) => - // contract.address.toLowerCase() === sellToken.address.toLowerCase() && - // contract.chainId === `evm:${sellToken.chainId}` - // ); - // return contractBalance?.balance || 0; - // } catch (e) { - // console.error('Error getting token balance in preview:', e); - // return 0; - // } - // };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/apps/pulse/components/App/tests/__snapshots__/AppWrapper.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/App/tests/__snapshots__/HomeScreen.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Buy/tests/__snapshots__/Buy.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Buy/tests/__snapshots__/PreviewBuy.test.tsx.snapis excluded by!**/*.snapsrc/apps/pulse/components/Sell/tests/__snapshots__/PreviewSell.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (16)
package.json(0 hunks)src/apps/pulse/components/App/HomeScreen.tsx(5 hunks)src/apps/pulse/components/Buy/Buy.tsx(7 hunks)src/apps/pulse/components/Buy/PayingToken.tsx(1 hunks)src/apps/pulse/components/Buy/PreviewBuy.tsx(2 hunks)src/apps/pulse/components/Buy/tests/PreviewBuy.test.tsx(8 hunks)src/apps/pulse/components/Misc/Tooltip.tsx(1 hunks)src/apps/pulse/components/Sell/PreviewSell.tsx(14 hunks)src/apps/pulse/components/Sell/tests/PreviewSell.test.tsx(6 hunks)src/apps/pulse/components/Status/IntentTracker.tsx(2 hunks)src/apps/pulse/hooks/tests/useRelaySell.test.tsx(1 hunks)src/apps/pulse/hooks/useGasEstimation.ts(1 hunks)src/apps/pulse/hooks/useIntentSdk.ts(3 hunks)src/apps/pulse/hooks/useRelaySell.ts(2 hunks)src/components/BottomMenuModal/SendModal/SendModalBatchesTabView.tsx(2 hunks)src/providers/GlobalTransactionsBatchProvider.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- package.json
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: RanaBug
PR: pillarwallet/x#391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
PR: pillarwallet/x#391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Sell/PreviewSell.tsxsrc/apps/pulse/components/Buy/Buy.tsxsrc/apps/pulse/components/App/HomeScreen.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
PR: pillarwallet/x#351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/pulse/components/Buy/Buy.tsx
🧬 Code graph analysis (7)
src/apps/pulse/hooks/useGasEstimation.ts (3)
src/apps/pulse/types/tokens.ts (1)
SelectedToken(1-10)src/apps/pulse/hooks/useRelaySell.ts (2)
SellOffer(37-43)useRelaySell(53-958)src/utils/blockchain.ts (1)
getNativeAssetForChainId(72-144)
src/apps/pulse/components/Buy/PayingToken.tsx (1)
src/utils/blockchain.ts (1)
getLogoForChainId(165-199)
src/apps/pulse/components/Sell/PreviewSell.tsx (3)
src/apps/pulse/types/tokens.ts (1)
SelectedToken(1-10)src/apps/pulse/hooks/useRelaySell.ts (1)
SellOffer(37-43)src/apps/pulse/hooks/useGasEstimation.ts (1)
useGasEstimation(20-164)
src/apps/pulse/components/Buy/PreviewBuy.tsx (6)
src/apps/pulse/hooks/useIntentSdk.ts (1)
useIntentSdk(9-53)src/apps/pulse/utils/intent.ts (1)
getDesiredAssetValue(9-20)src/apps/pulse/components/Status/IntentTracker.tsx (1)
IntentTracker(35-263)src/apps/pulse/components/Misc/Refresh.tsx (1)
Refresh(10-34)src/apps/pulse/components/Buy/PayingToken.tsx (1)
PayingToken(8-44)src/utils/blockchain.ts (1)
getLogoForChainId(165-199)
src/apps/pulse/components/Buy/Buy.tsx (1)
src/apps/pulse/utils/intent.ts (1)
getDesiredAssetValue(9-20)
src/apps/pulse/components/App/HomeScreen.tsx (1)
src/apps/pulse/components/Buy/PreviewBuy.tsx (1)
PreviewBuy(52-555)
src/apps/pulse/components/Buy/tests/PreviewBuy.test.tsx (1)
src/apps/pulse/components/Buy/PreviewBuy.tsx (1)
PreviewBuy(52-555)
🔇 Additional comments (26)
src/apps/pulse/components/Status/IntentTracker.tsx (1)
101-101: Wrapper swap LGTM; verify no layout regressions in parent containers.Replacing the Fragment with a flex column wrapper looks fine and aligns with the new vertical layout.
Please sanity‑check:
- Overflow/scroll behavior at small widths (the card uses fixed 422px widths).
- That parent containers didn’t rely on a Fragment (e.g., CSS grid children count).
If issues appear, consider wrapping with a neutral container only where needed instead of the entire component.
Also applies to: 261-261
src/apps/pulse/hooks/useIntentSdk.ts (1)
48-53: API surface addition looks good.clearError via useCallback is correct and stable.
src/apps/pulse/components/Buy/PayingToken.tsx (1)
11-11: Tailwind rewrite LGTM.Layout and semantics preserved.
src/providers/GlobalTransactionsBatchProvider.tsx (1)
79-83: Harden the batch exclusion predicate and centralize it.String.includes is fragile and can false‑match unrelated batch names; use a strict predicate and export it for reuse.
File: src/providers/GlobalTransactionsBatchProvider.tsx (around lines 79–83)
- const filteredBatches = Object.keys(batches).filter( - (batchName) => !batchName.includes('pulse-sell') - ); + const isPulseSellBatch = (name: string) => /^pulse-sell(?:-|$)/i.test(name); + const filteredBatches = Object.keys(batches).filter( + (batchName) => !isPulseSellBatch(batchName) + );Export isPulseSellBatch from a shared util (e.g., src/apps/pulse/utils/batches.ts) and use it across UI/providers to avoid divergence. Automated repo search didn't run (ripgrep skipped files); manually verify there are no other 'pulse-sell' call sites and update them to use the shared predicate.
src/apps/pulse/hooks/useRelaySell.ts (2)
37-42: LGTM! Clean interface extension for enhanced sell flow.The new fields (
minimumReceive,slippageTolerance, andpriceImpact) are well-structured additions that provide essential trading information to users, aligning with standard DeFi practices.
203-221: Defensive programming for minimum amount calculation.Good approach to handle both formatted and raw amounts with proper fallback logic. The calculation correctly applies slippage tolerance when
minimumAmountis not provided by the API.src/apps/pulse/hooks/useGasEstimation.ts (2)
43-46: Good concurrency control with useRef.The use of
isEstimatingRefeffectively prevents race conditions during concurrent gas estimations. This is a solid pattern for managing async operations.
102-119: Verify native token decimals assumption.The code correctly uses
getNativeAssetForChainIdto get the appropriate decimals for each chain's native token. Good practice for multi-chain support.src/apps/pulse/components/Buy/tests/PreviewBuy.test.tsx (2)
52-60: Test props updated correctly for new PreviewBuy API.The test properly includes the new required props (
setExpressIntentResponse,usdAmount,dispensableAssets) matching the component's updated interface.
85-102: UI text updates properly reflected in tests.All label changes are correctly updated in the test assertions, maintaining consistency with the actual UI components.
src/apps/pulse/components/Sell/tests/PreviewSell.test.tsx (3)
39-63: Comprehensive mocking for transaction kit and gas estimation.Excellent test setup with proper mocks for both
useTransactionKitanduseGasEstimation. The mock data structure correctly simulates the actual batch estimation response.
179-186: Good coverage of gas fee display.The new test properly verifies that gas fees are displayed with the correct format and native token symbol.
229-233: Test correctly updated for new executeSell signature.The test properly reflects that
executeSellnow expects three arguments, with the third being the optional wallet portfolio data.src/apps/pulse/components/Buy/Buy.tsx (4)
39-43: Well-designed props for parent state management.The new props enable proper state lifting to the parent component, allowing for centralized control of USD amount, dispensable assets, and refresh functionality. This is a clean architectural improvement.
383-414: Improved liquidity messaging UX.Great improvement to the user experience with dynamic messaging that distinguishes between "Not enough liquidity" and "No available routes". The conditional rendering logic is clean and easy to follow.
436-465: Well-implemented amount button improvements.The disabled state logic and data-testid attributes are properly implemented. The MAX button correctly uses wallet balance only when a token is selected.
169-182: Validate 3% slippage change and make it token-aware3% (was 5%) falls in the common 1–5% band for medium/low-liquidity tokens but exceeds typical tolerances for stable/high-liquidity pairs (~0.1–1%).
File: src/apps/pulse/components/Buy/Buy.tsx (lines 169–182)
- Verify the target token's liquidity/volatility; lower to ≤1% for high‑liquidity/stable pairs to reduce failed txs and MEV risk.
- If the token is medium/low liquidity, 3% is reasonable; for very low‑liquidity/meme/taxed tokens permit higher or require explicit user confirmation.
- Prefer making slippage configurable or computed per‑token (or via routing/aggregator) and add monitoring for swap failure rate.
src/apps/pulse/components/Buy/PreviewBuy.tsx (3)
265-295: Auto-refresh implementation looks goodThe auto-refresh logic is well-implemented with proper guard conditions and cleanup. The 15-second interval is reasonable, and pausing during signature confirmation prevents conflicts.
319-344: Clear and well-documented calculation logicThe price impact and minimum receive calculations are well-implemented with clear comments explaining the formulas. The examples help understand the logic flow.
429-445: Nice UX improvement with copy-to-clipboardThe implementation of copy-to-clipboard with visual feedback is a good UX enhancement. The checkmark display for 3 seconds provides clear feedback to users.
src/apps/pulse/components/App/HomeScreen.tsx (2)
60-66: State type definitions are appropriateThe new state variables are properly typed, and using a callback type for
buyRefreshCallbackensures type safety for the refresh mechanism.
68-116: Well-structured refresh logic with proper error handlingThe refresh function correctly prevents simultaneous calls and handles both buy and sell modes appropriately. The error handling for the sell offer refresh is good practice.
src/apps/pulse/components/Sell/PreviewSell.tsx (4)
118-137: Good batch cleanup implementationThe batch cleanup utility with consistent error handling is well-implemented. The context parameter helps with debugging, and the error check for non-existent batches is appropriate.
329-393: Well-implemented direct execution flowThe
executeSellDirectlyfunction properly handles batch preparation, execution, and cleanup in all scenarios (success, error, user rejection). The error handling distinguishes between user rejection and other errors appropriately.
253-277: Good auto-refresh implementation with proper guardsThe auto-refresh for sell offers correctly pauses during signature waiting and transaction execution, preventing race conditions.
628-635: Excellent gas fee display implementationThe gas fee display with native token symbol and loading state provides clear feedback to users. The integration with the
useGasEstimationhook is clean.
| walletProvider | ||
| .getEthereumProvider() | ||
| .then((provider) => { | ||
| const walletClient = createWalletClient({ | ||
| account: walletProvider.address as Hex, | ||
| transport: custom(provider), | ||
| }); | ||
| /* eslint-disable @typescript-eslint/no-explicit-any */ | ||
| const sdk = new IntentSdk(walletClient as any, options); | ||
| setIntentSdk(sdk); | ||
| setError(null); // Clear any previous errors when SDK initializes | ||
| }) | ||
| .catch((err) => { | ||
| console.error('Failed to initialize Intent SDK:', err); | ||
| setError('Failed to initialize Intent SDK. Please try again.'); | ||
| }); | ||
| /* eslint-disable @typescript-eslint/no-explicit-any */ | ||
| const sdk = new IntentSdk(walletClient as any, options); | ||
| setIntentSdk(sdk); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Prevent setState on unmounted during async init; add defensive checks.
The promise chain can resolve after unmount and call setState, causing warnings. Also handle missing API key early.
Apply this safer async pattern:
- walletProvider
- .getEthereumProvider()
- .then((provider) => {
- const walletClient = createWalletClient({
- account: walletProvider.address as Hex,
- transport: custom(provider),
- });
- /* eslint-disable @typescript-eslint/no-explicit-any */
- const sdk = new IntentSdk(walletClient as any, options);
- setIntentSdk(sdk);
- setError(null); // Clear any previous errors when SDK initializes
- })
- .catch((err) => {
- console.error('Failed to initialize Intent SDK:', err);
- setError('Failed to initialize Intent SDK. Please try again.');
- });
+ let mounted = true;
+ (async () => {
+ try {
+ if (!options.bundlerApiKey) {
+ throw new Error('Missing VITE_ETHERSPOT_BUNDLER_API_KEY');
+ }
+ const provider = await walletProvider.getEthereumProvider();
+ const walletClient = createWalletClient({
+ account: walletProvider.address as Hex,
+ transport: custom(provider),
+ });
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
+ const sdk = new IntentSdk(walletClient as any, options);
+ if (mounted) {
+ setIntentSdk(sdk);
+ setError(null);
+ }
+ } catch (err) {
+ console.error('Failed to initialize Intent SDK:', err);
+ if (mounted) {
+ setError('Failed to initialize Intent SDK. Please try again.');
+ }
+ }
+ })();
+ return () => {
+ mounted = false;
+ };📝 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.
| walletProvider | |
| .getEthereumProvider() | |
| .then((provider) => { | |
| const walletClient = createWalletClient({ | |
| account: walletProvider.address as Hex, | |
| transport: custom(provider), | |
| }); | |
| /* eslint-disable @typescript-eslint/no-explicit-any */ | |
| const sdk = new IntentSdk(walletClient as any, options); | |
| setIntentSdk(sdk); | |
| setError(null); // Clear any previous errors when SDK initializes | |
| }) | |
| .catch((err) => { | |
| console.error('Failed to initialize Intent SDK:', err); | |
| setError('Failed to initialize Intent SDK. Please try again.'); | |
| }); | |
| /* eslint-disable @typescript-eslint/no-explicit-any */ | |
| const sdk = new IntentSdk(walletClient as any, options); | |
| setIntentSdk(sdk); | |
| }); | |
| } | |
| let mounted = true; | |
| (async () => { | |
| try { | |
| if (!options.bundlerApiKey) { | |
| throw new Error('Missing VITE_ETHERSPOT_BUNDLER_API_KEY'); | |
| } | |
| const provider = await walletProvider.getEthereumProvider(); | |
| const walletClient = createWalletClient({ | |
| account: walletProvider.address as Hex, | |
| transport: custom(provider), | |
| }); | |
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | |
| const sdk = new IntentSdk(walletClient as any, options); | |
| if (mounted) { | |
| setIntentSdk(sdk); | |
| setError(null); | |
| } | |
| } catch (err) { | |
| console.error('Failed to initialize Intent SDK:', err); | |
| if (mounted) { | |
| setError('Failed to initialize Intent SDK. Please try again.'); | |
| } | |
| } | |
| })(); | |
| return () => { | |
| mounted = false; | |
| }; | |
| } |
🤖 Prompt for AI Agents
In src/apps/pulse/hooks/useIntentSdk.ts around lines 29 to 45, the async
initialization uses a promise chain that can call setIntentSdk/setError after
the component has unmounted and doesn't validate the API key early; change to an
async-init pattern that validates the API key up front, track mount state with a
ref/flag (e.g., isMounted) set true on mount and false in cleanup, and before
any setState calls check isMounted to avoid updates on unmounted components;
also convert the then/catch chain to try/await so you can catch errors, call
setError only if isMounted, and ensure any created clients are only used when
mounted.
| setExpressIntentResponse(response); | ||
| } catch (error) { | ||
| console.error('express intent failed:: ', error); | ||
| setExpressIntentResponse(null); |
There was a problem hiding this comment.
I'm just wondering what happens in the user flow here?
| .transaction({ | ||
| chainId: tx.chainId, | ||
| to: tx.to, | ||
| value: tx.value, | ||
| data: tx.data, | ||
| }) | ||
| .name({ transactionName }) | ||
| .addToBatch({ batchName }); |
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Bug Fixes
Style
Chores