fix: onramp receive addresses #11080
Conversation
📝 WalkthroughWalkthroughTwo FiatRamps components are modified to use a dedicated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsx (1)
324-379: Add validation to prevent submission without a valid receive address.When
direction === FiatRampAction.Buy, the form can be submitted with an empty address if bothmanualReceiveAddressandwalletReceiveAddressare unavailable (line 346). This occurs when:
- The user hasn't entered a manual address
- The
walletReceiveAddressis still loading or failed to loadSubmitting an empty address to ramp providers will likely cause the transaction to fail or, worse, result in funds being sent to an invalid destination.
Apply this diff to add address validation:
const handleSubmit = useCallback( async (e: FormEvent<unknown>) => { e.preventDefault() if (!selectedQuote?.provider) return if (!isConnected) return if (direction === FiatRampAction.Buy && !buyAccountMetadata) return + if (direction === FiatRampAction.Buy && !manualReceiveAddress && !walletReceiveAddress) return const ramp = supportedFiatRamps[selectedQuote.provider]Additionally, ensure
isWalletReceiveAddressLoading(from the previous comment) is used to disable the submit button while the address is loading.
Dependency array correctly updated.
The removal of
walletand addition ofwalletReceiveAddressin thehandleSubmitdependencies is correct and aligns with the refactored logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsx(4 hunks)src/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsxsrc/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsxsrc/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsxsrc/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsxsrc/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsxsrc/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx
🧠 Learnings (18)
📓 Common learnings
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10871
File: src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx:426-428
Timestamp: 2025-10-21T17:11:18.087Z
Learning: In src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx, within the handleInputChange function, use .toFixed() without arguments (not .toString()) when converting BigNumber amounts for input field synchronization. This avoids exponential notation in the input while preserving precision for presentational components like <Amount.Crypto /> and <Amount.Fiat /> to format appropriately.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/pages/RFOX/components/Stake/components/StakeSummary.tsx:112-114
Timestamp: 2025-08-22T13:00:44.879Z
Learning: NeOMakinG prefers to keep PR changes minimal and focused on the core objectives, avoiding cosmetic or defensive code improvements that aren't directly related to the PR scope, even when they would improve robustness.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10128
File: .cursor/rules/error-handling.mdc:266-274
Timestamp: 2025-07-29T10:35:22.059Z
Learning: NeOMakinG prefers less nitpicky suggestions on documentation and best practices files, finding overly detailed suggestions on minor implementation details (like console.error vs logger.error) too granular for cursor rules documentation.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/pages/Dashboard/components/AccountList/AccountTable.tsx:60-0
Timestamp: 2025-09-02T08:34:08.157Z
Learning: NeOMakinG prefers code review comments to focus only on actual PR changes, not pre-existing code issues, unless there are critical security or correctness concerns directly related to the new functionality.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10234
File: src/components/MultiHopTrade/hooks/useGetTradeQuotes/hooks/useTrackTradeQuotes.ts:42-86
Timestamp: 2025-08-08T11:41:22.794Z
Learning: NeOMakinG prefers not to include refactors in move-only PRs; such suggestions should be deferred to follow-up issues instead of being applied within the same PR.
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/components/Table/Table.theme.ts:177-180
Timestamp: 2025-09-02T12:38:46.940Z
Learning: NeOMakinG prefers to defer technical debt and CSS correctness issues (like improper hover selectors) to follow-up PRs when the current PR is already large and focused on major feature implementation, even when the issues are valid from a usability/technical perspective.
📚 Learning: 2025-10-21T17:11:18.087Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10871
File: src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx:426-428
Timestamp: 2025-10-21T17:11:18.087Z
Learning: In src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx, within the handleInputChange function, use .toFixed() without arguments (not .toString()) when converting BigNumber amounts for input field synchronization. This avoids exponential notation in the input while preserving precision for presentational components like <Amount.Crypto /> and <Amount.Fiat /> to format appropriately.
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsxsrc/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx
📚 Learning: 2025-08-08T11:41:36.971Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10234
File: src/components/MultiHopTrade/hooks/useGetTradeQuotes/hooks/useTrackTradeQuotes.ts:88-109
Timestamp: 2025-08-08T11:41:36.971Z
Learning: In MultiHopTrade Confirm flow (src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx), the Confirm route does not remount; navigating away goes to the swapper input page. Therefore, persistent deduplication across remounts for quote tracking is unnecessary; a ref-based single-mount dedupe is sufficient.
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsxsrc/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx
📚 Learning: 2025-08-08T11:40:55.734Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10234
File: src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx:41-41
Timestamp: 2025-08-08T11:40:55.734Z
Learning: In MultiHopTrade confirm flow (src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx and related hooks), there is only one active trade per flow. Because of this, persistent (module/Redux) dedupe for QuotesReceived in useTrackTradeQuotes is not necessary; the existing ref-based dedupe is acceptable.
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsxsrc/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx
📚 Learning: 2025-07-24T09:43:11.699Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-07-24T09:43:11.699Z
Learning: Applies to packages/swapper/src/swappers/*/{*.ts,endpoints.ts} : Avoid side effects in swap logic within swapper implementation files.
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsx
📚 Learning: 2025-10-07T03:44:27.350Z
Learnt from: 0xApotheosis
Repo: shapeshift/web PR: 10760
File: src/components/ManageHiddenAssets/ManageHiddenAssetsList.tsx:78-84
Timestamp: 2025-10-07T03:44:27.350Z
Learning: In the ShapeShift web codebase, the following are stable references and do not need to be included in useCallback/useMemo dependency arrays:
- `navigate` from `useBrowserRouter()` hook
- Modal control objects (like `walletDrawer`) from `useModal()` hook (including their `isOpen`, `close`, and `open` methods)
- These are backed by stable context providers
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsxsrc/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx
📚 Learning: 2025-08-14T17:54:32.563Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/ReusableLpStatus.tsx:97-108
Timestamp: 2025-08-14T17:54:32.563Z
Learning: In ReusableLpStatus component (src/pages/ThorChainLP/components/ReusableLpStatus/ReusableLpStatus.tsx), the txAssets dependency is stable from first render because poolAsset, baseAsset, actionSide, and action are all defined first render, making the current txAssetsStatuses initialization pattern safe without needing useEffect synchronization.
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsx
📚 Learning: 2025-09-04T17:29:59.479Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10380
File: src/components/TradeAssetSearch/hooks/useGetPopularAssetsQuery.tsx:28-33
Timestamp: 2025-09-04T17:29:59.479Z
Learning: In shapeshift/web, the useGetPopularAssetsQuery function in src/components/TradeAssetSearch/hooks/useGetPopularAssetsQuery.tsx intentionally uses primaryAssets[assetId] instead of falling back to assets[assetId]. The design distributes primary assets across chains by iterating through their related assets and adding the primary asset to each related asset's chain. This ensures primary assets appear in all chains where they have related assets, supporting the grouped asset system.
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsx
📚 Learning: 2025-08-05T23:36:13.214Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10187
File: src/state/slices/preferencesSlice/selectors.ts:21-25
Timestamp: 2025-08-05T23:36:13.214Z
Learning: The AssetId type from 'shapeshiftoss/caip' package is a string type alias, so it can be used directly as a return type for cache key resolvers in re-reselect selectors without needing explicit string conversion.
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsx
📚 Learning: 2025-09-08T15:53:09.362Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10442
File: src/components/TradeAssetSearch/components/GroupedAssetList/GroupedAssetList.tsx:34-35
Timestamp: 2025-09-08T15:53:09.362Z
Learning: In DefaultAssetList.tsx, the GroupedAssetList component already receives the activeChainId prop correctly on line ~58, contrary to automated analysis that may flag it as missing.
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsx
📚 Learning: 2025-07-29T15:04:28.083Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10139
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandableStepperSteps.tsx:109-115
Timestamp: 2025-07-29T15:04:28.083Z
Learning: In src/components/MultiHopTrade/components/TradeConfirm/components/ExpandableStepperSteps.tsx, the component is used under an umbrella that 100% of the time contains the quote, making the type assertion `activeTradeQuote?.steps[currentHopIndex] as TradeQuoteStep` safe. Adding conditional returns before hooks would violate React's Rules of Hooks.
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsxsrc/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx
📚 Learning: 2025-08-03T22:10:38.426Z
Learnt from: CR
Repo: shapeshift/web PR: 0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : EXTRACT complex logic into custom hooks
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsxsrc/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx
📚 Learning: 2025-08-10T21:09:25.643Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10215
File: src/components/MultiHopTrade/hooks/useGetTradeRateInput.ts:65-67
Timestamp: 2025-08-10T21:09:25.643Z
Learning: In the MultiHopTrade components, `selectInputBuyAsset` and `selectInputSellAsset` selectors from `tradeInputSlice` always return defined values because they have default values in the initial state (BTC for buyAsset, ETH for sellAsset, with fallback to defaultAsset). Null checks for these assets are unnecessary when using these selectors.
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsxsrc/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx
📚 Learning: 2025-09-02T00:02:44.889Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10380
File: src/components/AssetSearch/components/GroupedAssetRow.tsx:116-0
Timestamp: 2025-09-02T00:02:44.889Z
Learning: premiumjibles prefers avoiding store.getState() calls in favor of useAppSelector where reactivity is needed, to prevent accidentally breaking reactive updates in components.
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsx
📚 Learning: 2025-08-11T09:46:41.060Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10219
File: src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx:167-172
Timestamp: 2025-08-11T09:46:41.060Z
Learning: In the shapeshift/web repository, the display cache logic for trade quotes (using `selectUserAvailableTradeQuotes` and `selectUserUnavailableTradeQuotes`) is intentionally kept the same between `TradeInput.tsx` and `TradeQuotes.tsx` components. The `hasQuotes` computation in `TradeInput.tsx` uses these display cache selectors by design, matching the pattern used in `TradeQuotes.tsx`.
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsxsrc/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx
📚 Learning: 2025-08-22T12:59:01.210Z
Learnt from: NeOMakinG
Repo: shapeshift/web PR: 10323
File: src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx:26-29
Timestamp: 2025-08-22T12:59:01.210Z
Learning: In src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx, NeOMakinG declined wrapping the RewardDistributionActionCard component with React.memo, saying it was "too much", suggesting that like other action center components, memoization is not beneficial for this specific use case.
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx
📚 Learning: 2025-08-05T22:41:35.473Z
Learnt from: premiumjibles
Repo: shapeshift/web PR: 10187
File: src/pages/Assets/Asset.tsx:1-1
Timestamp: 2025-08-05T22:41:35.473Z
Learning: In the shapeshift/web codebase, component imports use direct file paths like '@/components/ComponentName/ComponentName' rather than barrel exports. The AssetAccountDetails component should be imported as '@/components/AssetAccountDetails/AssetAccountDetails', not from a directory index.
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx
📚 Learning: 2025-08-14T17:56:23.721Z
Learnt from: gomesalexandre
Repo: shapeshift/web PR: 10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx:545-566
Timestamp: 2025-08-14T17:56:23.721Z
Learning: gomesalexandre prefers not to extract helper functions for toast rendering patterns in TransactionRow.tsx (src/pages/ThorChainLP/components/ReusableLpStatus/TransactionRow.tsx), considering it over-abstraction even when there's code duplication between deposit and withdraw flows.
Applied to files:
src/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx
🧬 Code graph analysis (2)
src/components/MultiHopTrade/components/FiatRamps/FiatRampTrade.tsx (1)
src/components/MultiHopTrade/hooks/useReceiveAddress.tsx (1)
useReceiveAddress(38-117)
src/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx (1)
src/components/MultiHopTrade/hooks/useReceiveAddress.tsx (1)
useReceiveAddress(38-117)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install and Cache
🔇 Additional comments (3)
src/components/MultiHopTrade/components/FiatRamps/FiatRampTradeFooter.tsx (3)
13-13: LGTM!Clean import addition following the codebase convention of direct file paths.
100-104: Correct implementation of the receive address fix.The hook is properly integrated to derive the actual receive address instead of displaying an xpub. Passing
sellAccountId: undefinedis correct for the Buy flow since fiat-to-crypto purchases don't have a blockchain sell account.
232-232: Fixes the loading state tracking.Replacing the hard-coded
falsewith the dynamic loading state from the hook ensures the UI properly reflects when the receive address is being derived, preventing the premature display of an xpub.
gomesalexandre
left a comment
gomesalexandre
left a comment
There was a problem hiding this comment.
Does the thing!
https://jam.dev/c/c1f07683-eef7-4675-b56c-7ed882316f4f
Description
We were showing an xpub in the manual receive address field, we want to show the next receive addresses, leveraging
useReceiveAddressto simplify things across the boardIssue (if applicable)
closes #11069
Risk
Low
Testing
Engineering
Operations
Screenshots (if applicable)
Summary by CodeRabbit