diff --git a/app/component-library/components-temp/Tabs/TabsBar/TabsBar.tsx b/app/component-library/components-temp/Tabs/TabsBar/TabsBar.tsx index 908fcf4398c5..dcc34ad9e7b6 100644 --- a/app/component-library/components-temp/Tabs/TabsBar/TabsBar.tsx +++ b/app/component-library/components-temp/Tabs/TabsBar/TabsBar.tsx @@ -38,13 +38,20 @@ const TabsBar: React.FC = ({ const underlineWidthAnimated = useRef(new Animated.Value(0)).current; const tabLayouts = useRef<{ x: number; width: number }[]>([]); const currentAnimation = useRef(null); + const rafCallbackId = useRef(null); const [isInitialized, setIsInitialized] = useState(false); const [layoutsReady, setLayoutsReady] = useState(false); + const activeIndexRef = useRef(activeIndex); // State for automatic overflow detection const [scrollEnabled, setScrollEnabled] = useState(false); const [containerWidth, setContainerWidth] = useState(0); + // Keep activeIndexRef in sync with activeIndex + useEffect(() => { + activeIndexRef.current = activeIndex; + }, [activeIndex]); + // Reset layout data when tabs change structurally (count or content) const tabKeys = useMemo(() => tabs.map((tab) => tab.key).join(','), [tabs]); const prevTabKeys = useRef(''); @@ -176,7 +183,8 @@ const TabsBar: React.FC = ({ const gapsWidth = (tabs.length - 1) * 24; // Account for gaps between tabs const calculatedContentWidth = totalTabsWidth + gapsWidth; - const shouldScroll = calculatedContentWidth > containerWidth; + // Account for container's px-4 padding (16px * 2 = 32px) + const shouldScroll = calculatedContentWidth > containerWidth - 32; setScrollEnabled(shouldScroll); } } @@ -197,6 +205,13 @@ const TabsBar: React.FC = ({ return; } + // Check if this is a significant change (more than 1px difference) + const previousLayout = tabLayouts.current[index]; + const hasSignificantChange = + !previousLayout || + Math.abs(previousLayout.width - width) > 1 || + Math.abs(previousLayout.x - x) > 1; + // Store layout data tabLayouts.current[index] = { x, width }; @@ -205,22 +220,41 @@ const TabsBar: React.FC = ({ (layout, i) => i >= tabs.length || (layout && layout.width > 0), ); - if (allLayoutsReady && !layoutsReady) { - setLayoutsReady(true); - - // Update scroll detection - if (containerWidth > 0) { - const totalWidth = tabLayouts.current.reduce( - (sum, layout) => sum + (layout?.width || 0), - 0, - ); - const gapsWidth = (tabs.length - 1) * 24; - const shouldScroll = totalWidth + gapsWidth > containerWidth; - setScrollEnabled(shouldScroll); + if (allLayoutsReady) { + // Recalculate scroll detection on initial load OR when any layout changes significantly + if (!layoutsReady || hasSignificantChange) { + if (!layoutsReady) { + setLayoutsReady(true); + } + + // If layouts were already ready and any tab changed, re-animate the active tab + // This ensures re-animation triggers regardless of which tab's callback fires last + if (layoutsReady && hasSignificantChange) { + // Cancel any pending RAF to avoid multiple callbacks + if (rafCallbackId.current !== null) { + cancelAnimationFrame(rafCallbackId.current); + } + rafCallbackId.current = requestAnimationFrame(() => { + rafCallbackId.current = null; + animateToTab(activeIndexRef.current); + }); + } + + // Update scroll detection + if (containerWidth > 0) { + const totalWidth = tabLayouts.current.reduce( + (sum, layout) => sum + (layout?.width || 0), + 0, + ); + const gapsWidth = (tabs.length - 1) * 24; + // Account for container's px-4 padding (16px * 2 = 32px) + const shouldScroll = totalWidth + gapsWidth > containerWidth - 32; + setScrollEnabled(shouldScroll); + } } } }, - [tabs.length, layoutsReady, containerWidth], + [tabs.length, layoutsReady, containerWidth, animateToTab], ); // Cleanup effect @@ -230,6 +264,10 @@ const TabsBar: React.FC = ({ currentAnimation.current.stop(); currentAnimation.current = null; } + if (rafCallbackId.current !== null) { + cancelAnimationFrame(rafCallbackId.current); + rafCallbackId.current = null; + } }, [], ); diff --git a/app/component-library/components/List/ListItemSelect/ListItemSelect.test.tsx b/app/component-library/components/List/ListItemSelect/ListItemSelect.test.tsx index 3e85d37fcd56..6e82f3ba848f 100644 --- a/app/component-library/components/List/ListItemSelect/ListItemSelect.test.tsx +++ b/app/component-library/components/List/ListItemSelect/ListItemSelect.test.tsx @@ -174,4 +174,21 @@ describe('ListItemSelect', () => { const component = getByTestId('list-item-select'); expect(component).toBeOnTheScreen(); }); + + it('passes through custom listItemProps', () => { + const { getByTestId } = render( + null} + listItemProps={{ + accessibilityHint: 'Custom Hint', + testID: 'nested-list-item', + }} + > + Test Content + , + ); + + const component = getByTestId('nested-list-item'); + expect(component.props.accessibilityHint).toBe('Custom Hint'); + }); }); diff --git a/app/component-library/components/List/ListItemSelect/ListItemSelect.tsx b/app/component-library/components/List/ListItemSelect/ListItemSelect.tsx index 319ca6a678e0..c8dd8de20e97 100644 --- a/app/component-library/components/List/ListItemSelect/ListItemSelect.tsx +++ b/app/component-library/components/List/ListItemSelect/ListItemSelect.tsx @@ -22,6 +22,7 @@ const ListItemSelect: React.FC = ({ onLongPress, gap = DEFAULT_SELECTITEM_GAP, verticalAlignment, + listItemProps, ...props }) => { const { styles } = useStyles(styleSheet, { style, isDisabled }); @@ -34,7 +35,7 @@ const ListItemSelect: React.FC = ({ onLongPress={onLongPress} {...props} > - + {children} {isSelected && ( diff --git a/app/component-library/components/List/ListItemSelect/ListItemSelect.types.ts b/app/component-library/components/List/ListItemSelect/ListItemSelect.types.ts index 8ec21c11754a..06c3d4181e91 100644 --- a/app/component-library/components/List/ListItemSelect/ListItemSelect.types.ts +++ b/app/component-library/components/List/ListItemSelect/ListItemSelect.types.ts @@ -18,6 +18,10 @@ export interface ListItemSelectProps * Optional prop to determine if the item is disabled. */ isDisabled?: boolean; + /** + * Optional prop to configure the prop of nested ListItem + */ + listItemProps?: Partial; } /** diff --git a/app/component-library/components/Toast/Toast.tsx b/app/component-library/components/Toast/Toast.tsx index 73b2b45aea5e..6cfa74c0e7b9 100644 --- a/app/component-library/components/Toast/Toast.tsx +++ b/app/component-library/components/Toast/Toast.tsx @@ -62,12 +62,10 @@ const Toast = forwardRef((_, ref: React.ForwardedRef) => { { translateY: translateYProgress.value - TAB_BAR_HEIGHT - customOffset }, ], })); - const baseStyle: StyleProp>> = - useMemo( - () => [styles.base, animatedStyle], - /* eslint-disable-next-line */ - [], - ); + const baseStyle: StyleProp = useMemo( + () => [styles.base, animatedStyle], + [styles.base, animatedStyle], + ); const resetState = () => setToastOptions(undefined); diff --git a/app/components/UI/Carousel/index.test.tsx b/app/components/UI/Carousel/index.test.tsx index ede360145c16..cce8a06e8e60 100644 --- a/app/components/UI/Carousel/index.test.tsx +++ b/app/components/UI/Carousel/index.test.tsx @@ -6,6 +6,7 @@ import { renderHook, } from '@testing-library/react-native'; import { useSelector, useDispatch } from 'react-redux'; +import { Linking } from 'react-native'; import SharedDeeplinkManager from '../../../core/DeeplinkManager/SharedDeeplinkManager'; import AppConstants from '../../../core/AppConstants'; import Carousel, { useFetchCarouselSlides } from './'; @@ -70,6 +71,10 @@ jest.mock('../../../core/DeeplinkManager/SharedDeeplinkManager', () => ({ parse: jest.fn(() => Promise.resolve()), })); +jest.mock('react-native/Libraries/Linking/Linking', () => ({ + openURL: jest.fn(() => Promise.resolve()), +})); + jest.mock('./fetchCarouselSlidesFromContentful', () => ({ ...jest.requireActual('./fetchCarouselSlidesFromContentful'), fetchCarouselSlidesFromContentful: jest.fn(), @@ -200,8 +205,10 @@ describe('Carousel Slide Filtering', () => { describe('Carousel Navigation', () => { it('opens external URLs when slide is clicked', async () => { + const slideID = 'deeplink-slide'; + const slideTestID = `carousel-slide-${slideID}`; const urlSlide = createMockSlide({ - id: 'url-slide', + id: slideID, navigation: { type: 'url', href: 'https://metamask.io' }, }); mockFetchCarouselSlides.mockResolvedValue({ @@ -210,14 +217,36 @@ describe('Carousel Navigation', () => { }); const { findByTestId } = render(); - const slide = await findByTestId('carousel-slide-url-slide'); + const slide = await findByTestId(slideTestID); fireEvent.press(slide); + + expect(Linking.openURL).toHaveBeenCalledWith('https://metamask.io'); + expect(SharedDeeplinkManager.parse).not.toHaveBeenCalled(); + }); + + it('handles internal deeplinks through SharedDeeplinkManager', async () => { + const slideID = 'deeplink-slide'; + const slideTestID = `carousel-slide-${slideID}`; + const deeplinkSlide = createMockSlide({ + id: slideID, + navigation: { type: 'url', href: 'https://link.metamask.io/swap' }, + }); + mockFetchCarouselSlides.mockResolvedValue({ + prioritySlides: [], + regularSlides: [deeplinkSlide], + }); + + const { findByTestId } = render(); + const slide = await findByTestId(slideTestID); + fireEvent.press(slide); + expect(SharedDeeplinkManager.parse).toHaveBeenCalledWith( - 'https://metamask.io', + 'https://link.metamask.io/swap', { - origin: AppConstants.DEEPLINKS.ORIGIN_DEEPLINK, + origin: AppConstants.DEEPLINKS.ORIGIN_CAROUSEL, }, ); + expect(Linking.openURL).not.toHaveBeenCalled(); }); it('navigates to buy flow for fund slides', async () => { diff --git a/app/components/UI/Carousel/index.tsx b/app/components/UI/Carousel/index.tsx index b0f08f2b1faf..b69ad2e83d34 100644 --- a/app/components/UI/Carousel/index.tsx +++ b/app/components/UI/Carousel/index.tsx @@ -6,7 +6,7 @@ import React, { useEffect, useRef, } from 'react'; -import { Dimensions, Animated } from 'react-native'; +import { Dimensions, Animated, Linking } from 'react-native'; import { useDispatch, useSelector } from 'react-redux'; import { useNavigation } from '@react-navigation/native'; import { CarouselProps, CarouselSlide, NavigationAction } from './types'; @@ -42,8 +42,9 @@ import { selectContentfulCarouselEnabledFlag } from './selectors/featureFlags'; import { createBuyNavigationDetails } from '../Ramp/Aggregator/routes/utils'; import Routes from '../../../constants/navigation/Routes'; import { subscribeToContentPreviewToken } from '../../../actions/notification/helpers'; -import AppConstants from '../../../core/AppConstants'; import SharedDeeplinkManager from '../../../core/DeeplinkManager/SharedDeeplinkManager'; +import { isInternalDeepLink } from '../../../util/deeplinks'; +import AppConstants from '../../../core/AppConstants'; const MAX_CAROUSEL_SLIDES = 8; @@ -362,13 +363,26 @@ const CarouselComponent: FC = ({ style, onEmptyState }) => { const openUrl = (href: string): (() => Promise) => - () => - SharedDeeplinkManager.parse(href, { - origin: AppConstants.DEEPLINKS.ORIGIN_DEEPLINK, - }).catch((error) => { - console.error('Failed to open URL:', error); - return false; - }); + () => { + // Check if this is an internal MetaMask deeplink + if (isInternalDeepLink(href)) { + // Handle internal deeplinks through SharedDeeplinkManager + return SharedDeeplinkManager.parse(href, { + origin: AppConstants.DEEPLINKS.ORIGIN_CAROUSEL, + }).catch((error) => { + console.error('Failed to handle internal deeplink:', error); + return false; + }); + } + + // For external URLs, use the OS linking system + return Linking.openURL(href) + .then(() => true) + .catch((error) => { + console.error('Failed to open external URL:', error); + return false; + }); + }; const handleSlideClick = useCallback( (slideId: string, navigation: NavigationAction) => { diff --git a/app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.test.tsx b/app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.test.tsx index 7ce0f0a7a2e9..6aa1fd71287d 100644 --- a/app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.test.tsx +++ b/app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.test.tsx @@ -51,6 +51,7 @@ jest.mock('../../hooks', () => ({ })); jest.mock('../../hooks/stream', () => ({ + usePerpsLivePositions: jest.fn(), usePerpsLivePrices: jest.fn(), usePerpsTopOfBook: jest.fn(), })); @@ -119,6 +120,9 @@ describe('PerpsClosePositionView', () => { const useRouteMock = jest.mocked( jest.requireMock('@react-navigation/native').useRoute, ); + const usePerpsLivePositionsMock = jest.mocked( + jest.requireMock('../../hooks/stream').usePerpsLivePositions, + ); const usePerpsLivePricesMock = jest.mocked( jest.requireMock('../../hooks/stream').usePerpsLivePrices, ); @@ -166,6 +170,10 @@ describe('PerpsClosePositionView', () => { }); // Setup hook mocks with default values + usePerpsLivePositionsMock.mockReturnValue({ + positions: [defaultPerpsPositionMock], + isInitialLoading: false, + }); usePerpsLivePricesMock.mockReturnValue(defaultPerpsLivePricesMock); usePerpsTopOfBookMock.mockReturnValue(defaultPerpsTopOfBookMock); usePerpsOrderFeesMock.mockReturnValue(defaultPerpsOrderFeesMock); @@ -446,14 +454,14 @@ describe('PerpsClosePositionView', () => { expect(usePerpsOrderFeesMock).toHaveBeenCalled(); }); - // Test that receiveAmount = (initial margin + effective P&L) - fees + // Test that receiveAmount uses marginUsed directly (which already includes PnL) it('calculates receive amount including P&L at effective price', () => { // Arrange const mockPosition = { ...defaultPerpsPositionMock, entryPrice: '100', // Entry at $100 - marginUsed: '1000', // $1000 initial margin - unrealizedPnl: '200', // Current unrealized P&L (not used in new calc) + marginUsed: '1200', // $1200 (includes $1000 initial + $200 unrealized PnL) + unrealizedPnl: '200', // Current unrealized P&L from HyperLiquid size: '1', // 1 token long position }; const mockFees = { @@ -462,7 +470,7 @@ describe('PerpsClosePositionView', () => { protocolFeeRate: 0.5, }; - // Set current price to $150 for clear P&L calculation + // Set current price to $150 for reference usePerpsLivePricesMock.mockReturnValue({ ETH: { price: '150' }, // Current price $150 }); @@ -471,6 +479,10 @@ describe('PerpsClosePositionView', () => { params: { position: mockPosition }, }); usePerpsOrderFeesMock.mockReturnValue(mockFees); + usePerpsLivePositionsMock.mockReturnValue({ + positions: [mockPosition], + isInitialLoading: false, + }); // Act const { getByText } = renderWithProvider( @@ -481,16 +493,15 @@ describe('PerpsClosePositionView', () => { true, ); - // Assert - receiveAmount = initialMargin + P&L - fees (P&L now included in calculation) - // P&L = (150 - 100) * 1 = 50 - // receiveAmount = 1000 + 50 - 50 = 1000 + // Assert - receiveAmount = marginUsed - fees + // HyperLiquid's marginUsed already includes PnL + // receiveAmount = 1200 - 50 = 1150 const receiveText = getByText( strings('perps.close_position.you_receive'), ); expect(receiveText).toBeDefined(); - // Look for 1000 in the display (margin + P&L - fees) // PRICE_RANGES_MINIMAL_VIEW: Fixed 2 decimals, trailing zeros removed - expect(getByText('$1,000')).toBeDefined(); + expect(getByText('$1,150')).toBeDefined(); }); it('calculates receive amount correctly for partial close percentages', () => { @@ -498,8 +509,8 @@ describe('PerpsClosePositionView', () => { const mockPosition = { ...defaultPerpsPositionMock, entryPrice: '100', // Entry at $100 - marginUsed: '2000', // $2000 initial margin - unrealizedPnl: '-300', // Current unrealized (not used in new calc) + marginUsed: '1700', // $1700 (includes $2000 initial - $300 unrealized loss) + unrealizedPnl: '-300', // Current unrealized loss from HyperLiquid size: '2', // 2 tokens long }; const mockFees = { @@ -517,6 +528,10 @@ describe('PerpsClosePositionView', () => { params: { position: mockPosition }, }); usePerpsOrderFeesMock.mockReturnValue(mockFees); + usePerpsLivePositionsMock.mockReturnValue({ + positions: [mockPosition], + isInitialLoading: false, + }); // Act const { getByText } = renderWithProvider( @@ -527,15 +542,15 @@ describe('PerpsClosePositionView', () => { true, ); - // For 100% close (default) with new calculation: - // P&L = (75 - 100) * 2 = -50 (loss) - // receiveAmount = 2000 + (-50) - 25 = 1925 + // For 100% close (default): + // HyperLiquid's marginUsed already includes PnL + // receiveAmount = 1700 - 25 = 1675 const receiveText = getByText( strings('perps.close_position.you_receive'), ); expect(receiveText).toBeDefined(); - // Look for 1925 in the display (margin + P&L - fees) - expect(getByText(/1,925/)).toBeDefined(); + // Look for 1675 in the display + expect(getByText(/1,675/)).toBeDefined(); }); }); @@ -632,8 +647,9 @@ describe('PerpsClosePositionView', () => { const positionWithLoss = { ...defaultPerpsPositionMock, entryPrice: '150', // Entry at $150 + marginUsed: '1350', // $1500 initial - $150 unrealized loss (includes funding) size: '1', // 1 token long - unrealizedPnl: '-100', // Current unrealized (not used for display) + unrealizedPnl: '-150', // Current unrealized loss including funding fees }; // Set current price lower than entry for loss @@ -644,6 +660,10 @@ describe('PerpsClosePositionView', () => { useRouteMock.mockReturnValue({ params: { position: positionWithLoss }, }); + usePerpsLivePositionsMock.mockReturnValue({ + positions: [positionWithLoss], + isInitialLoading: false, + }); // Act const { getByText } = renderWithProvider( @@ -654,9 +674,9 @@ describe('PerpsClosePositionView', () => { true, ); - // Assert - effectivePnL = (100 - 150) * 1 = -50 - // Look for negative P&L display (with - sign) - should show 50 (absolute value) - const pnlElement = getByText(/-.*50/); + // Assert - Now uses actual unrealizedPnl from position + // Look for negative P&L display (with - sign) - should show 150 (absolute value) + const pnlElement = getByText(/-.*150/); expect(pnlElement).toBeDefined(); }); }); @@ -2049,9 +2069,9 @@ describe('PerpsClosePositionView', () => { expect(track).toHaveBeenCalled(); // Assert - Should call with expected parameters structure for full close - // Calculation: effectivePnL = (3000 - 2900) * 1.5 = 150 - // effectiveMargin = 1450 + 150 = 1600 - // receivedAmount = 1600 - 45 = 1555 + // HyperLiquid's marginUsed already includes PnL + // receivedAmount = marginUsed - fees = 1450 - 45 = 1405 + // realizedPnl = unrealizedPnl = 150 (from defaultPerpsPositionMock) expect(handleClosePosition).toHaveBeenCalledWith( defaultPerpsPositionMock, '', @@ -2060,7 +2080,7 @@ describe('PerpsClosePositionView', () => { { totalFee: 45, marketPrice: 3000, - receivedAmount: 1555, + receivedAmount: 1405, realizedPnl: 150, metamaskFeeRate: 0, metamaskFee: 0, @@ -2369,6 +2389,81 @@ describe('PerpsClosePositionView', () => { }); describe('Price Update Synchronization', () => { + it('recalculates effectivePnL when current price changes', async () => { + // Arrange - Position with entry price + const mockPosition = { + ...defaultPerpsPositionMock, + size: '1', // 1 token long position + entryPrice: '100', // Entry at $100 + marginUsed: '100', + unrealizedPnl: '0', + }; + + useRouteMock.mockReturnValue({ + params: { position: mockPosition }, + }); + + // Mock usePerpsLivePositions to return the test's mock position + usePerpsLivePositionsMock.mockReturnValue({ + positions: [mockPosition], + isInitialLoading: false, + }); + + // Initially price equals entry price (no P&L) + usePerpsLivePricesMock.mockReturnValue({ + ETH: { price: '100' }, // Current price = entry price + }); + + const { rerender, getByText, queryByText } = renderWithProvider( + , + { + state: STATE_MOCK, + }, + true, + ); + + // Assert initial state - effectivePnL should be close to 0 when price = entry + // (100 - 100) * 1 = 0 + // The margin displayed should be just the margin used ($100) + // P&L displayed should be $0 (or very close to it) + await waitFor(() => { + const marginLabel = getByText(strings('perps.close_position.margin')); + expect(marginLabel).toBeDefined(); + // P&L should be ~$0 when price equals entry price + expect(queryByText(/\+.*\$0/)).toBeDefined(); + }); + + // Act - Simulate live price increasing to $150 + // Update both price and position to reflect the P&L change + const updatedPosition = { + ...mockPosition, + unrealizedPnl: '50', // (150 - 100) * 1 = 50 + marginUsed: '150', // 100 initial + 50 P&L + }; + + usePerpsLivePositionsMock.mockReturnValue({ + positions: [updatedPosition], + isInitialLoading: false, + }); + + usePerpsLivePricesMock.mockReturnValue({ + ETH: { price: '150' }, // Live price is $150 (50% profit) + }); + + // Force re-render with new price to trigger dependency recalculation + rerender(); + + // Assert - effectivePnL should update to positive value + // (150 - 100) * 1 = 50 profit + // Margin should now include the P&L: 100 + 50 = 150 + await waitFor(() => { + // Look for the positive P&L display in the "includes P&L" row + expect(queryByText(/\+.*\$50/)).toBeDefined(); + // Look for the margin label to ensure we're checking the right section + expect(getByText(strings('perps.close_position.margin'))).toBeDefined(); + }); + }); + it('syncs input amount when price updates from entry to live price', async () => { // Arrange - Position with entry price const mockPosition = { diff --git a/app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.tsx b/app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.tsx index f75d539c0594..9ec875ace2a7 100644 --- a/app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.tsx +++ b/app/components/UI/Perps/Views/PerpsClosePositionView/PerpsClosePositionView.tsx @@ -46,7 +46,11 @@ import { usePerpsToasts, usePerpsMarketData, } from '../../hooks'; -import { usePerpsLivePrices, usePerpsTopOfBook } from '../../hooks/stream'; +import { + usePerpsLivePositions, + usePerpsLivePrices, + usePerpsTopOfBook, +} from '../../hooks/stream'; import { usePerpsEventTracking } from '../../hooks/usePerpsEventTracking'; import { usePerpsMeasurement } from '../../hooks/usePerpsMeasurement'; import { @@ -125,9 +129,19 @@ const PerpsClosePositionView: React.FC = () => { symbol: position.coin, }); - // Determine position direction - const isLong = parseFloat(position.size) > 0; - const absSize = Math.abs(parseFloat(position.size)); + // Subscribe to live position updates for this coin + // This ensures margin and PnL values include real-time funding fees + const { positions: livePositions } = usePerpsLivePositions({ + throttleMs: 1000, + }); + const livePosition = useMemo( + () => livePositions.find((p) => p.coin === position.coin) || position, + [livePositions, position], + ); + + // Determine position direction using live position data + const isLong = parseFloat(livePosition.size) > 0; + const absSize = Math.abs(parseFloat(livePosition.size)); // Calculate effective price for calculations // For limit orders, use limit price when available; otherwise use current market price const effectivePrice = useMemo(() => { @@ -160,16 +174,25 @@ const PerpsClosePositionView: React.FC = () => { ? closeAmountUSDString : calculatedUSDString; + // Use live position data which includes real-time funding fees + // HyperLiquid's marginUsed already includes accumulated PnL + const marginUsed = parseFloat(livePosition.marginUsed); + + // Use unrealizedPnl from live position (includes funding fees) + const unrealizedPnl = parseFloat(livePosition.unrealizedPnl); + + // Keep pnl reference for backwards compatibility with event tracking + const pnl = unrealizedPnl; + // Calculate position value and effective margin // For limit orders, use limit price for display calculations - // Use ref for market price to prevent recalculation on every WebSocket update const positionValue = useMemo(() => { const priceToUse = orderType === 'limit' && limitPrice ? parseFloat(limitPrice) - : currentPriceRef.current; + : currentPrice; return absSize * priceToUse; - }, [absSize, orderType, limitPrice]); // Exclude currentPrice from deps to prevent recalculation + }, [absSize, orderType, limitPrice, currentPrice]); // Calculate P&L based on effective price (limit price for limit orders) // Use ref for market price to prevent recalculation on every WebSocket update @@ -178,25 +201,19 @@ const PerpsClosePositionView: React.FC = () => { // Calculate P&L based on the effective price (limit price for limit orders) // For long positions: (effectivePrice - entryPrice) * absSize // For short positions: (entryPrice - effectivePrice) * absSize - const priceToUse = - orderType === 'limit' && limitPrice - ? parseFloat(limitPrice) - : currentPriceRef.current; + if (orderType === 'market') { + return pnl; + } + const priceToUse = limitPrice ? parseFloat(limitPrice) : currentPrice; const priceDiff = isLong ? priceToUse - entryPrice : entryPrice - priceToUse; return priceDiff * absSize; - }, [entryPrice, absSize, isLong, orderType, limitPrice]); // Exclude effectivePrice from deps - - // Use the actual initial margin from the position - const initialMargin = parseFloat(position.marginUsed); - - // Use unrealized PnL from position for current market price (for reference/tracking) - const pnl = parseFloat(position.unrealizedPnl); + }, [entryPrice, absSize, isLong, orderType, limitPrice, currentPrice, pnl]); // Exclude effectivePrice from deps // Calculate fees using the unified fee hook const closingValue = useMemo( - () => positionValue * (closePercentage / 100), // Round to 2 decimal places + () => positionValue * (closePercentage / 100), [positionValue, closePercentage], ); const closingValueString = useMemo( @@ -233,13 +250,17 @@ const PerpsClosePositionView: React.FC = () => { orderAmount: closingValueString, }); - // Calculate what user will receive (initial margin + P&L - fees) - // P&L is already shown separately in the margin section as "includes P&L" + // Calculate what user will receive (margin - fees) + // Round each component separately to match what user sees in UI + // This ensures: displayed margin - displayed fees = displayed receive amount const receiveAmount = useMemo(() => { - const marginPortion = (closePercentage / 100) * initialMargin; - const pnlPortion = effectivePnL * (closePercentage / 100); - return marginPortion + pnlPortion - feeResults.totalFee; - }, [closePercentage, initialMargin, effectivePnL, feeResults.totalFee]); + const marginPortion = (closePercentage / 100) * marginUsed; + // Round margin and fees to 2 decimals (what user sees) + const roundedMargin = Math.round(marginPortion * 100) / 100; + const roundedFees = Math.round(feeResults.totalFee * 100) / 100; + // Subtract rounded values for transparent calculation + return roundedMargin - roundedFees; + }, [closePercentage, marginUsed, feeResults.totalFee]); // Get minimum order amount for this asset const { minimumOrderAmount } = useMinimumOrderAmount({ @@ -269,10 +290,10 @@ const PerpsClosePositionView: React.FC = () => { const { handleClosePosition, isClosing } = usePerpsClosePosition(); - const unrealizedPnlPercent = useMemo( - () => (initialMargin > 0 ? (pnl / initialMargin) * 100 : 0), - [initialMargin, pnl], - ); + const unrealizedPnlPercent = useMemo(() => { + const initialMargin = marginUsed - pnl; // Back-calculate initial margin + return initialMargin > 0 ? (pnl / initialMargin) * 100 : 0; + }, [marginUsed, pnl]); usePerpsEventTracking({ eventName: MetaMetricsEvents.PERPS_SCREEN_VIEWED, @@ -328,7 +349,7 @@ const PerpsClosePositionView: React.FC = () => { navigation.goBack(); await handleClosePosition( - position, + livePosition, sizeToClose || '', orderType, orderType === 'limit' ? limitPrice : undefined, @@ -475,10 +496,7 @@ const PerpsClosePositionView: React.FC = () => { const Summary = ( ({ usePerpsHomeData: jest.fn(), @@ -49,6 +50,7 @@ jest.mock('../../hooks', () => ({ navigateTo: jest.fn(), navigateToMarketDetails: jest.fn(), navigateToMarketList: mockNavigateToMarketList, + navigateToWallet: mockNavigateToWallet, navigateBack: mockNavigateBack, goBack: jest.fn(), })), @@ -392,6 +394,7 @@ describe('PerpsHomeView', () => { beforeEach(() => { jest.clearAllMocks(); mockNavigateBack.mockClear(); + mockNavigateToWallet.mockClear(); mockNavigateToMarketList.mockClear(); mockUsePerpsHomeData.mockReturnValue(mockDefaultData); }); @@ -537,15 +540,15 @@ describe('PerpsHomeView', () => { expect(queryByText('perps.home.orders')).toBeNull(); }); - it('handles back button press', () => { + it('navigates to wallet home when back button is pressed', () => { // Arrange const { getByTestId } = render(); // Act fireEvent.press(getByTestId('back-button')); - // Assert - expect(mockNavigateBack).toHaveBeenCalled(); + // Assert - Always navigates to wallet home to avoid loops (e.g., from tutorial) + expect(mockNavigateToWallet).toHaveBeenCalled(); }); it('navigates to close all modal when close all is pressed', () => { diff --git a/app/components/UI/Perps/Views/PerpsHomeView/PerpsHomeView.tsx b/app/components/UI/Perps/Views/PerpsHomeView/PerpsHomeView.tsx index 315373c30773..9167894f8eb3 100644 --- a/app/components/UI/Perps/Views/PerpsHomeView/PerpsHomeView.tsx +++ b/app/components/UI/Perps/Views/PerpsHomeView/PerpsHomeView.tsx @@ -174,8 +174,9 @@ const PerpsHomeView = () => { setShowCancelAllSheet(false); }, []); - // Back button handler - now uses navigation hook - const handleBackPress = perpsNavigation.navigateBack; + // Back button handler - always navigate to wallet home to avoid loops + // (e.g., when coming from tutorial/onboarding flow) + const handleBackPress = perpsNavigation.navigateToWallet; return ( diff --git a/app/components/UI/Rewards/components/Settings/RewardOptInAccountGroupModal.test.tsx b/app/components/UI/Rewards/components/Settings/RewardOptInAccountGroupModal.test.tsx index 59c062a71e81..a189c19a972f 100644 --- a/app/components/UI/Rewards/components/Settings/RewardOptInAccountGroupModal.test.tsx +++ b/app/components/UI/Rewards/components/Settings/RewardOptInAccountGroupModal.test.tsx @@ -162,6 +162,7 @@ jest.mock( { testID: 'bottom-sheet', ref, + onClose, ...props, }, children, @@ -227,42 +228,17 @@ jest.mock( ReactActual.createElement( View, { testID, ...props }, - ReactActual.createElement(View, { testID: `address-${address}` }), ReactActual.createElement(View, { - testID: `network-${networkName}`, + testID: `multichain-address-row-address`, + }), + ReactActual.createElement(View, { + testID: `multichain-address-row-network-name`, }), ), }; }, ); -// Mock RewardsInfoBanner -jest.mock('../RewardsInfoBanner', () => { - const ReactActual = jest.requireActual('react'); - const { View, Text } = jest.requireActual('react-native'); - - return { - __esModule: true, - default: ({ - title, - description, - testID, - ...props - }: { - title: string; - description: string; - testID?: string; - [key: string]: unknown; - }) => - ReactActual.createElement( - View, - { testID, ...props }, - ReactActual.createElement(Text, {}, title), - ReactActual.createElement(Text, {}, description), - ), - }; -}); - const mockUseNavigation = useNavigation as jest.MockedFunction< typeof useNavigation >; @@ -390,80 +366,98 @@ describe('RewardOptInAccountGroupModal', () => { }); describe('Basic Rendering', () => { - it('should render bottom sheet container', () => { + it('renders bottom sheet container', () => { const { getByTestId } = render(); expect(getByTestId('bottom-sheet')).toBeOnTheScreen(); }); - it('should render header with account group name', () => { + it('renders header with account group name', () => { const { getByTestId } = render(); expect(getByTestId('bottom-sheet-header')).toBeOnTheScreen(); }); - it('should render address list', () => { + it('renders address list', () => { const { getByTestId } = render(); expect(getByTestId('reward-opt-in-address-list')).toBeOnTheScreen(); }); - it('should render address items for each address', () => { + it('renders address items for each address', () => { const { getByTestId } = render(); - // Check that the MultichainAddressRow components are rendered expect( - getByTestId('address-0x1234567890123456789012345678901234567890'), + getByTestId( + 'flat-list-item-0x1234567890123456789012345678901234567890-eip155:1', + ), ).toBeOnTheScreen(); expect( - getByTestId('address-0x0987654321098765432109876543210987654321'), + getByTestId( + 'flat-list-item-0x0987654321098765432109876543210987654321-eip155:1', + ), ).toBeOnTheScreen(); }); - }); - describe('Unsupported Accounts Banner', () => { - it('should show banner when there are unsupported accounts', () => { - // Arrange + it('does not render address list when addressData is empty', () => { mockUseRoute.mockReturnValue({ params: { ...defaultRouteParams, - addressData: [ - ...defaultRouteParams.addressData, - { - address: '0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', - hasOptedIn: false, - scopes: ['eip155:1' as CaipChainId], - isSupported: false, - }, - ], + addressData: [], }, key: 'test-route', name: 'RewardOptInAccountGroupModal', } as never); - // Act - const { getByTestId } = render(); + const { queryByTestId } = render(); - // Assert - expect(getByTestId('unsupported-accounts-banner')).toBeOnTheScreen(); + expect(queryByTestId('reward-opt-in-address-list')).toBeNull(); }); - it('should not show banner when all accounts are supported', () => { - const { queryByTestId } = render(); + it('calls navigation.goBack when BottomSheet onClose is triggered', () => { + const { getByTestId } = render(); + + const bottomSheet = getByTestId('bottom-sheet'); + const onClose = bottomSheet.props.onClose; - expect(queryByTestId('unsupported-accounts-banner')).toBeNull(); + if (onClose) { + onClose(); + } + + expect(mockGoBack).toHaveBeenCalledTimes(1); }); }); describe('Link Account Group Button', () => { - it('should render link button when there are accounts that can opt in', () => { + it('renders link button when there are accounts that can opt in', () => { const { getByTestId } = render(); expect(getByTestId('link-account-group-button')).toBeOnTheScreen(); }); - it('should not render link button when all accounts are opted in', () => { - // Arrange + it('does not render link button when all accounts are opted in', () => { + mockUseRoute.mockReturnValue({ + params: { + ...defaultRouteParams, + addressData: [ + { + address: '0x1234567890123456789012345678901234567890', + hasOptedIn: true, + scopes: ['eip155:1' as CaipChainId], + isSupported: true, + }, + ], + }, + key: 'test-route', + name: 'RewardOptInAccountGroupModal', + } as never); + + const { queryByTestId } = render(); + + expect(queryByTestId('link-account-group-button')).toBeNull(); + }); + + it('does not render link button when all supported addresses are opted in but unsupported addresses exist', () => { mockUseRoute.mockReturnValue({ params: { ...defaultRouteParams, @@ -474,21 +468,24 @@ describe('RewardOptInAccountGroupModal', () => { scopes: ['eip155:1' as CaipChainId], isSupported: true, }, + { + address: '0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', + hasOptedIn: false, + scopes: ['eip155:1' as CaipChainId], + isSupported: false, + }, ], }, key: 'test-route', name: 'RewardOptInAccountGroupModal', } as never); - // Act const { queryByTestId } = render(); - // Assert expect(queryByTestId('link-account-group-button')).toBeNull(); }); - it('should call linkAccountGroup when button is pressed', async () => { - // Arrange + it('calls linkAccountGroup when button is pressed', async () => { mockLinkAccountGroup.mockResolvedValue({ success: true, byAddress: { @@ -498,11 +495,9 @@ describe('RewardOptInAccountGroupModal', () => { const { getByTestId } = render(); - // Act const linkButton = getByTestId('link-account-group-button'); fireEvent.press(linkButton); - // Assert await waitFor(() => { expect(mockLinkAccountGroup).toHaveBeenCalledWith( 'keyring:wallet-1/ethereum', @@ -510,24 +505,20 @@ describe('RewardOptInAccountGroupModal', () => { }); }); - it('should show loading state when linking', () => { - // Arrange + it('shows loading state when linking', () => { mockUseLinkAccountGroup.mockReturnValue({ linkAccountGroup: mockLinkAccountGroup, isLoading: true, isError: false, }); - // Act const { getByTestId } = render(); - // Assert const linkButton = getByTestId('link-account-group-button'); expect(linkButton).toHaveProp('disabled', true); }); - it('should update local state after successful link', async () => { - // Arrange + it('updates local state after successful link', async () => { mockLinkAccountGroup.mockResolvedValue({ success: true, byAddress: { @@ -537,18 +528,24 @@ describe('RewardOptInAccountGroupModal', () => { const { getByTestId } = render(); - // Act const linkButton = getByTestId('link-account-group-button'); fireEvent.press(linkButton); - // Assert await waitFor(() => { expect(mockLinkAccountGroup).toHaveBeenCalled(); }); + + // Wait for the async state update to complete + await waitFor(() => { + expect( + getByTestId( + 'flat-list-item-0x0987654321098765432109876543210987654321-eip155:1', + ), + ).toBeOnTheScreen(); + }); }); - it('should handle link failure gracefully', async () => { - // Arrange + it('handles link failure gracefully', async () => { const consoleErrorSpy = jest .spyOn(console, 'error') .mockImplementation(() => { @@ -558,11 +555,9 @@ describe('RewardOptInAccountGroupModal', () => { const { getByTestId } = render(); - // Act const linkButton = getByTestId('link-account-group-button'); fireEvent.press(linkButton); - // Assert await waitFor(() => { expect(consoleErrorSpy).toHaveBeenCalledWith( 'Failed to link account group:', @@ -575,8 +570,7 @@ describe('RewardOptInAccountGroupModal', () => { }); describe('Network Resolution', () => { - it('should resolve non-EVM network names correctly', () => { - // Arrange + it('resolves non-EVM network names correctly', () => { mockUseRoute.mockReturnValue({ params: { ...defaultRouteParams, @@ -595,15 +589,16 @@ describe('RewardOptInAccountGroupModal', () => { name: 'RewardOptInAccountGroupModal', } as never); - // Act const { getByTestId } = render(); - // Assert - expect(getByTestId('network-Bitcoin')).toBeOnTheScreen(); + expect( + getByTestId( + 'flat-list-item-bc1qxy2kgdygjrsqtzq2n0yrf2493p83kkfjhx0wlh-bip122:000000000019d6689c085ae165831e93', + ), + ).toBeOnTheScreen(); }); - it('should handle unknown network scopes', () => { - // Arrange + it('handles unknown network scopes', () => { const consoleWarnSpy = jest .spyOn(console, 'warn') .mockImplementation(() => { @@ -626,10 +621,8 @@ describe('RewardOptInAccountGroupModal', () => { name: 'RewardOptInAccountGroupModal', } as never); - // Act render(); - // Assert expect(consoleWarnSpy).toHaveBeenCalledWith( 'Unknown network for scope:', 'unknown:network', @@ -637,11 +630,37 @@ describe('RewardOptInAccountGroupModal', () => { consoleWarnSpy.mockRestore(); }); + + it('treats addresses with undefined isSupported as supported', () => { + mockUseRoute.mockReturnValue({ + params: { + ...defaultRouteParams, + addressData: [ + { + address: '0x1234567890123456789012345678901234567890', + hasOptedIn: false, + scopes: ['eip155:1' as CaipChainId], + isSupported: undefined, + }, + ], + }, + key: 'test-route', + name: 'RewardOptInAccountGroupModal', + } as never); + + const { getByTestId } = render(); + + expect( + getByTestId( + 'flat-list-item-0x1234567890123456789012345678901234567890-eip155:1', + ), + ).toBeOnTheScreen(); + expect(getByTestId('link-account-group-button')).toBeOnTheScreen(); + }); }); describe('Wildcard Scope Handling', () => { - it('should expand eip155:* wildcard to all EVM networks', () => { - // Arrange + it('expands eip155:* wildcard to all EVM networks', () => { mockUseRoute.mockReturnValue({ params: { ...defaultRouteParams, @@ -658,15 +677,16 @@ describe('RewardOptInAccountGroupModal', () => { name: 'RewardOptInAccountGroupModal', } as never); - // Act const { getByTestId } = render(); - // Assert - Should render Ethereum Mainnet, but not Goerli testnet - expect(getByTestId('network-Ethereum Mainnet')).toBeOnTheScreen(); + expect( + getByTestId( + 'flat-list-item-0x1234567890123456789012345678901234567890-eip155:1', + ), + ).toBeOnTheScreen(); }); - it('should expand bip122:0 wildcard to all Bitcoin networks', () => { - // Arrange + it('expands bip122:0 wildcard to all Bitcoin networks', () => { mockUseRoute.mockReturnValue({ params: { ...defaultRouteParams, @@ -683,15 +703,16 @@ describe('RewardOptInAccountGroupModal', () => { name: 'RewardOptInAccountGroupModal', } as never); - // Act const { getByTestId } = render(); - // Assert - expect(getByTestId('network-Bitcoin')).toBeOnTheScreen(); + expect( + getByTestId( + 'flat-list-item-bc1qxy2kgdygjrsqtzq2n0yrf2493p83kkfjhx0wlh-bip122:000000000019d6689c085ae165831e93', + ), + ).toBeOnTheScreen(); }); - it('should filter out testnets when expanding EVM wildcards', () => { - // Arrange + it('filters out testnets when expanding EVM wildcards', () => { mockUseRoute.mockReturnValue({ params: { ...defaultRouteParams, @@ -708,15 +729,17 @@ describe('RewardOptInAccountGroupModal', () => { name: 'RewardOptInAccountGroupModal', } as never); - // Act const { queryByTestId } = render(); - // Assert - Goerli testnet should not be rendered - expect(queryByTestId('network-Goerli Testnet')).toBeNull(); + // Goerli testnet should be filtered out, so no items with that network should exist + expect( + queryByTestId( + 'flat-list-item-0x1234567890123456789012345678901234567890-eip155:5', + ), + ).toBeNull(); }); - it('should handle invalid CAIP scope format', () => { - // Arrange + it('handles invalid CAIP scope format', () => { const consoleWarnSpy = jest .spyOn(console, 'warn') .mockImplementation(() => { @@ -739,10 +762,8 @@ describe('RewardOptInAccountGroupModal', () => { name: 'RewardOptInAccountGroupModal', } as never); - // Act render(); - // Assert expect(consoleWarnSpy).toHaveBeenCalledWith( 'Unknown network for scope:', 'invalid', @@ -750,22 +771,64 @@ describe('RewardOptInAccountGroupModal', () => { consoleWarnSpy.mockRestore(); }); + + it('handles wildcard scope with missing namespace', () => { + const consoleWarnSpy = jest + .spyOn(console, 'warn') + .mockImplementation(() => { + // Suppress warning output in test + }); + + mockUseRoute.mockReturnValue({ + params: { + ...defaultRouteParams, + addressData: [ + { + address: '0x1234567890123456789012345678901234567890', + hasOptedIn: false, + scopes: [':*' as CaipChainId], + isSupported: true, + }, + ], + }, + key: 'test-route', + name: 'RewardOptInAccountGroupModal', + } as never); + + render(); + + expect(consoleWarnSpy).toHaveBeenCalledWith( + 'Invalid CAIP scope format:', + ':*', + ); + + consoleWarnSpy.mockRestore(); + }); }); describe('Edge Cases', () => { - it('should handle missing account group context', () => { - // Arrange + it('handles missing account group context', () => { mockSelectAccountGroupById.mockReturnValue(undefined); - // Act const { queryByTestId } = render(); - // Assert - Header should not render without account group name expect(queryByTestId('bottom-sheet-header')).toBeNull(); }); - it('should handle EVM chain ID conversion errors', () => { - // Arrange + it('handles account group context without metadata name', () => { + mockSelectAccountGroupById.mockReturnValue({ + id: 'keyring:wallet-1/ethereum', + scopes: [], + keyringType: 'HD Key Tree', + metadata: {}, + } as never); + + const { queryByTestId } = render(); + + expect(queryByTestId('bottom-sheet-header')).toBeNull(); + }); + + it('handles EVM chain ID conversion errors', () => { const consoleWarnSpy = jest .spyOn(console, 'warn') .mockImplementation(() => { @@ -786,10 +849,8 @@ describe('RewardOptInAccountGroupModal', () => { throw new Error('Invalid chain ID'); }); - // Act render(); - // Assert expect(consoleWarnSpy).toHaveBeenCalledWith( 'Invalid EVM chain ID:', 'invalid-chain-id', @@ -799,8 +860,7 @@ describe('RewardOptInAccountGroupModal', () => { consoleWarnSpy.mockRestore(); }); - it('should handle address items with valid scope', () => { - // Arrange + it('handles address items with valid scope', () => { mockUseRoute.mockReturnValue({ params: { ...defaultRouteParams, @@ -817,46 +877,247 @@ describe('RewardOptInAccountGroupModal', () => { name: 'RewardOptInAccountGroupModal', } as never); - // Act const { getByTestId } = render(); - // Assert - Should render the address expect( - getByTestId('address-0x1234567890123456789012345678901234567890'), + getByTestId( + 'flat-list-item-0x1234567890123456789012345678901234567890-eip155:1', + ), + ).toBeOnTheScreen(); + }); + + it('skips address items with missing address', () => { + mockUseRoute.mockReturnValue({ + params: { + ...defaultRouteParams, + addressData: [ + { + address: '', + hasOptedIn: false, + scopes: ['eip155:1' as CaipChainId], + isSupported: true, + }, + ], + }, + key: 'test-route', + name: 'RewardOptInAccountGroupModal', + } as never); + + const { queryByTestId } = render(); + + expect(queryByTestId('reward-opt-in-address-list')).toBeNull(); + }); + + it('skips address items with null address', () => { + mockUseRoute.mockReturnValue({ + params: { + ...defaultRouteParams, + addressData: [ + { + address: null as unknown as string, + hasOptedIn: false, + scopes: ['eip155:1' as CaipChainId], + isSupported: true, + }, + ], + }, + key: 'test-route', + name: 'RewardOptInAccountGroupModal', + } as never); + + const { queryByTestId } = render(); + + expect(queryByTestId('reward-opt-in-address-list')).toBeNull(); + }); + + it('skips address items with empty scopes array', () => { + mockUseRoute.mockReturnValue({ + params: { + ...defaultRouteParams, + addressData: [ + { + address: '0x1234567890123456789012345678901234567890', + hasOptedIn: false, + scopes: [], + isSupported: true, + }, + ], + }, + key: 'test-route', + name: 'RewardOptInAccountGroupModal', + } as never); + + const { queryByTestId } = render(); + + expect(queryByTestId('reward-opt-in-address-list')).toBeNull(); + }); + + it('skips address items with null scopes', () => { + mockUseRoute.mockReturnValue({ + params: { + ...defaultRouteParams, + addressData: [ + { + address: '0x1234567890123456789012345678901234567890', + hasOptedIn: false, + scopes: null as unknown as string[], + isSupported: true, + }, + ], + }, + key: 'test-route', + name: 'RewardOptInAccountGroupModal', + } as never); + + const { queryByTestId } = render(); + + expect(queryByTestId('reward-opt-in-address-list')).toBeNull(); + }); + + it('skips empty scope strings', () => { + mockUseRoute.mockReturnValue({ + params: { + ...defaultRouteParams, + addressData: [ + { + address: '0x1234567890123456789012345678901234567890', + hasOptedIn: false, + scopes: ['', 'eip155:1' as CaipChainId], + isSupported: true, + }, + ], + }, + key: 'test-route', + name: 'RewardOptInAccountGroupModal', + } as never); + + const { getByTestId } = render(); + + expect( + getByTestId( + 'flat-list-item-0x1234567890123456789012345678901234567890-eip155:1', + ), + ).toBeOnTheScreen(); + }); + + it('skips whitespace-only scope strings', () => { + mockUseRoute.mockReturnValue({ + params: { + ...defaultRouteParams, + addressData: [ + { + address: '0x1234567890123456789012345678901234567890', + hasOptedIn: false, + scopes: [' ', 'eip155:1' as CaipChainId], + isSupported: true, + }, + ], + }, + key: 'test-route', + name: 'RewardOptInAccountGroupModal', + } as never); + + const { getByTestId } = render(); + + expect( + getByTestId( + 'flat-list-item-0x1234567890123456789012345678901234567890-eip155:1', + ), + ).toBeOnTheScreen(); + }); + + it('handles address items with multiple scopes', () => { + mockUseRoute.mockReturnValue({ + params: { + ...defaultRouteParams, + addressData: [ + { + address: '0x1234567890123456789012345678901234567890', + hasOptedIn: false, + scopes: [ + 'eip155:1' as CaipChainId, + 'bip122:000000000019d6689c085ae165831e93' as CaipChainId, + ], + isSupported: true, + }, + ], + }, + key: 'test-route', + name: 'RewardOptInAccountGroupModal', + } as never); + + const { getByTestId } = render(); + + expect( + getByTestId( + 'flat-list-item-0x1234567890123456789012345678901234567890-eip155:1', + ), + ).toBeOnTheScreen(); + expect( + getByTestId( + 'flat-list-item-0x1234567890123456789012345678901234567890-bip122:000000000019d6689c085ae165831e93', + ), ).toBeOnTheScreen(); }); }); describe('FlatList Configuration', () => { - it('should render FlatList with correct testID', () => { + it('renders FlatList with correct testID', () => { const { getByTestId } = render(); - // Verify the FlatList is rendered expect(getByTestId('reward-opt-in-address-list')).toBeOnTheScreen(); }); - it('should have correct FlatList props for scrolling', () => { + it('has correct FlatList props for scrolling', () => { const { getByTestId } = render(); const flatList = getByTestId('reward-opt-in-address-list'); expect(flatList).toHaveProp('showsVerticalScrollIndicator', true); }); + + it('generates correct keys for header items', () => { + const { getByTestId } = render(); + + const flatList = getByTestId('reward-opt-in-address-list'); + const keyExtractor = flatList.props.keyExtractor; + + const headerItem = { type: 'header' as const, title: 'Test Header' }; + const key = keyExtractor(headerItem, 0); + + expect(key).toBe('header-Test Header-0'); + }); + + it('generates correct keys for address items', () => { + const { getByTestId } = render(); + + const flatList = getByTestId('reward-opt-in-address-list'); + const keyExtractor = flatList.props.keyExtractor; + + const addressItem = { + type: 'item' as const, + address: '0x123', + scope: 'eip155:1' as CaipChainId, + hasOptedIn: false, + networkName: 'Ethereum', + isSupported: true, + }; + const key = keyExtractor(addressItem, 1); + + expect(key).toBe('0x123-eip155:1-1'); + }); }); describe('Section Headers (Tracked/Untracked)', () => { - it('should show section headers when there are both tracked and untracked addresses', () => { - // Arrange - default route params has both tracked and untracked addresses + it('shows section headers when there are both tracked and untracked addresses', () => { const { getByText } = render(); - // Assert expect(getByText('rewards.link_account_group.tracked')).toBeOnTheScreen(); expect( getByText('rewards.link_account_group.untracked'), ).toBeOnTheScreen(); }); - it('should not show section headers when all addresses are tracked', () => { - // Arrange + it('shows tracked section header when all addresses are tracked', () => { mockUseRoute.mockReturnValue({ params: { ...defaultRouteParams, @@ -873,16 +1134,15 @@ describe('RewardOptInAccountGroupModal', () => { name: 'RewardOptInAccountGroupModal', } as never); - // Act - const { queryByText } = render(); + const { getByText, queryByText } = render( + , + ); - // Assert - Headers should not be rendered when there's only one type - expect(queryByText('rewards.link_account_group.tracked')).toBeNull(); + expect(getByText('rewards.link_account_group.tracked')).toBeOnTheScreen(); expect(queryByText('rewards.link_account_group.untracked')).toBeNull(); }); - it('should not show section headers when all addresses are untracked', () => { - // Arrange + it('shows untracked section header when all addresses are untracked', () => { mockUseRoute.mockReturnValue({ params: { ...defaultRouteParams, @@ -899,16 +1159,17 @@ describe('RewardOptInAccountGroupModal', () => { name: 'RewardOptInAccountGroupModal', } as never); - // Act - const { queryByText } = render(); + const { getByText, queryByText } = render( + , + ); - // Assert - Headers should not be rendered when there's only one type expect(queryByText('rewards.link_account_group.tracked')).toBeNull(); - expect(queryByText('rewards.link_account_group.untracked')).toBeNull(); + expect( + getByText('rewards.link_account_group.untracked'), + ).toBeOnTheScreen(); }); - it('should not show section headers for unsupported addresses', () => { - // Arrange + it('shows unsupported section header for unsupported addresses', () => { mockUseRoute.mockReturnValue({ params: { ...defaultRouteParams, @@ -925,17 +1186,46 @@ describe('RewardOptInAccountGroupModal', () => { name: 'RewardOptInAccountGroupModal', } as never); - // Act - const { queryByText } = render(); + const { getByText } = render(); - // Assert - Unsupported addresses should not be shown in the list - expect(queryByText('rewards.link_account_group.tracked')).toBeNull(); - expect(queryByText('rewards.link_account_group.untracked')).toBeNull(); + expect( + getByText('rewards.link_account_group.unsupported'), + ).toBeOnTheScreen(); + }); + + it('shows unsupported section header when unsupported addresses exist alongside supported ones', () => { + mockUseRoute.mockReturnValue({ + params: { + ...defaultRouteParams, + addressData: [ + { + address: '0x1234567890123456789012345678901234567890', + hasOptedIn: true, + scopes: ['eip155:1' as CaipChainId], + isSupported: true, + }, + { + address: '0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', + hasOptedIn: false, + scopes: ['eip155:1' as CaipChainId], + isSupported: false, + }, + ], + }, + key: 'test-route', + name: 'RewardOptInAccountGroupModal', + } as never); + + const { getByText } = render(); + + expect( + getByText('rewards.link_account_group.unsupported'), + ).toBeOnTheScreen(); }); }); describe('Accessibility', () => { - it('should have proper testIDs for all interactive elements', () => { + it('has proper testIDs for all interactive elements', () => { const { getByTestId } = render(); expect(getByTestId('bottom-sheet')).toBeOnTheScreen(); @@ -943,4 +1233,62 @@ describe('RewardOptInAccountGroupModal', () => { expect(getByTestId('link-account-group-button')).toBeOnTheScreen(); }); }); + + describe('RenderItem Function', () => { + it('renders header items correctly', () => { + const { getByTestId } = render(); + + const flatList = getByTestId('reward-opt-in-address-list'); + const renderItem = flatList.props.renderItem; + + const headerItem = { + item: { type: 'header' as const, title: 'Test Header' }, + }; + const headerComponent = renderItem(headerItem); + + expect(headerComponent).toBeTruthy(); + }); + + it('returns null for items without address', () => { + const { getByTestId } = render(); + + const flatList = getByTestId('reward-opt-in-address-list'); + const renderItem = flatList.props.renderItem; + + const invalidItem = { + item: { + type: 'item' as const, + address: '', + scope: 'eip155:1' as CaipChainId, + hasOptedIn: false, + networkName: 'Ethereum', + isSupported: true, + }, + }; + const result = renderItem(invalidItem); + + expect(result).toBeNull(); + }); + + it('returns null for items without scope', () => { + const { getByTestId } = render(); + + const flatList = getByTestId('reward-opt-in-address-list'); + const renderItem = flatList.props.renderItem; + + const invalidItem = { + item: { + type: 'item' as const, + address: '0x123', + scope: '' as CaipChainId, + hasOptedIn: false, + networkName: 'Ethereum', + isSupported: true, + }, + }; + const result = renderItem(invalidItem); + + expect(result).toBeNull(); + }); + }); }); diff --git a/app/components/UI/Rewards/components/Settings/RewardOptInAccountGroupModal.tsx b/app/components/UI/Rewards/components/Settings/RewardOptInAccountGroupModal.tsx index 6c7c808ca873..d38adebff441 100644 --- a/app/components/UI/Rewards/components/Settings/RewardOptInAccountGroupModal.tsx +++ b/app/components/UI/Rewards/components/Settings/RewardOptInAccountGroupModal.tsx @@ -21,7 +21,6 @@ import { useSelector } from 'react-redux'; import { RootState } from '../../../../../reducers'; import { strings } from '../../../../../../locales/i18n'; import { useLinkAccountGroup } from '../../hooks/useLinkAccountGroup'; -import RewardsInfoBanner from '../RewardsInfoBanner'; import { selectEvmNetworkConfigurationsByChainId } from '../../../../../selectors/networkController'; import { selectNonEvmNetworkConfigurationsByChainId } from '../../../../../selectors/multichainNetworkController'; import { CaipChainId } from '@metamask/utils'; @@ -116,75 +115,139 @@ const RewardOptInAccountGroupModal: React.FC = () => { const flattenedAddressData = useMemo(() => { const flattened: FlattenedAddressItem[] = []; - addressData.forEach((item) => { - if (!item?.address || !Array.isArray(item.scopes)) { + const addFlattenedItem = ( + address: string, + hasOptedIn: boolean, + scope: CaipChainId, + networkName: string, + isSupported?: boolean, + ) => { + flattened.push({ + address, + hasOptedIn, + scope, + networkName, + isSupported, + }); + }; + + const processMatchingNetwork = ( + chainId: string, + network: { name: string; hexChainId?: string }, + namespace: string, + address: string, + hasOptedIn: boolean, + isSupported: boolean | undefined, + ) => { + const chainIdParts = chainId.split(':'); + if (chainIdParts.length < 2) { return; } - const updatedHasOptedIn = - localAccountStatuses[item.address] ?? item.hasOptedIn; + const chainIdNamespace = chainIdParts[0]; + if (chainIdNamespace !== namespace) { + return; + } - item.scopes.forEach((scope: string) => { - if (typeof scope !== 'string' || !scope.trim()) { - return; - } + // Skip testnets for EVM networks + if (network.hexChainId && isTestNet(network.hexChainId)) { + return; + } - const caipScope = scope as CaipChainId; + addFlattenedItem( + address, + hasOptedIn, + chainId as CaipChainId, + network.name, + isSupported, + ); + }; + + const processWildcardScope = ( + caipScope: CaipChainId, + address: string, + hasOptedIn: boolean, + isSupported: boolean | undefined, + ) => { + const scopeParts = caipScope.split(':'); + if (scopeParts.length < 2) { + console.warn('Invalid CAIP scope format:', caipScope); + return; + } - // Handle wildcard patterns (e.g., "eip155:*" or "bip122:0") - if (caipScope.includes(':*') || caipScope.endsWith(':0')) { - const scopeParts = caipScope.split(':'); - if (scopeParts.length < 2) { - console.warn('Invalid CAIP scope format:', caipScope); - return; - } + const namespace = scopeParts[0]; + if (!namespace) { + console.warn('Invalid CAIP scope format:', caipScope); + return; + } - const namespace = scopeParts[0]; - if (!namespace) { - return; - } + // Add all networks matching this namespace (excluding testnets) + Object.entries(allNetworks).forEach(([chainId, network]) => { + processMatchingNetwork( + chainId, + network, + namespace, + address, + hasOptedIn, + isSupported, + ); + }); + }; + + const processSpecificScope = ( + caipScope: CaipChainId, + address: string, + hasOptedIn: boolean, + isSupported: boolean | undefined, + ) => { + const network = allNetworks[caipScope]; + if (network) { + addFlattenedItem( + address, + hasOptedIn, + caipScope, + network.name, + isSupported, + ); + } else { + console.warn('Unknown network for scope:', caipScope); + } + }; + + const processScope = ( + scope: string, + address: string, + hasOptedIn: boolean, + isSupported: boolean | undefined, + ) => { + if (typeof scope !== 'string' || !scope.trim()) { + return; + } - // Add all networks matching this namespace (excluding testnets) - Object.entries(allNetworks).forEach(([chainId, network]) => { - const chainIdParts = chainId.split(':'); - if (chainIdParts.length < 2) { - return; - } - - const chainIdNamespace = chainIdParts[0]; - - if (chainIdNamespace === namespace) { - // Skip testnets for EVM networks - if (network.hexChainId && isTestNet(network.hexChainId)) { - return; - } - - flattened.push({ - address: item.address, - hasOptedIn: updatedHasOptedIn, - scope: chainId as CaipChainId, - networkName: network.name, - isSupported: item.isSupported, - }); - } - }); - } else { - // Specific network scope - const network = allNetworks[caipScope]; - if (network) { - flattened.push({ - address: item.address, - hasOptedIn: updatedHasOptedIn, - scope: caipScope, - networkName: network.name, - isSupported: item.isSupported, - }); - } else { - console.warn('Unknown network for scope:', caipScope); - } - } + const caipScope = scope as CaipChainId; + + // Handle wildcard patterns (e.g., "eip155:*" or "bip122:0") + if (caipScope.includes(':*') || caipScope.endsWith(':0')) { + processWildcardScope(caipScope, address, hasOptedIn, isSupported); + } else { + processSpecificScope(caipScope, address, hasOptedIn, isSupported); + } + }; + + const processAddressItem = (item: AddressItem) => { + if (!item?.address || !Array.isArray(item.scopes)) { + return; + } + + const updatedHasOptedIn = + localAccountStatuses[item.address] ?? item.hasOptedIn; + + item.scopes.forEach((scope: string) => { + processScope(scope, item.address, updatedHasOptedIn, item.isSupported); }); - }); + }; + + addressData.forEach(processAddressItem); return flattened; }, [addressData, localAccountStatuses, allNetworks]); @@ -205,13 +268,13 @@ const RewardOptInAccountGroupModal: React.FC = () => { const renderItem = useCallback( ({ - item, + item: itemCtx, }: { item: | { type: 'header'; title: string } | ({ type: 'item' } & FlattenedAddressItem); }) => { - if (item.type === 'header') { + if (itemCtx.type === 'header') { return ( { fontWeight={FontWeight.Medium} twClassName="text-alternative" > - {item.title} + {itemCtx.title} ); } - if (!item.address || !item.scope) { + if (!itemCtx.address || !itemCtx.scope) { return null; } return ( ); }, [], ); - const unsupportedAddresses = useMemo( - () => - flattenedAddressData?.filter((item) => item.isSupported === false) ?? [], - [flattenedAddressData], - ); - const canOptInAddresses = useMemo( () => flattenedAddressData?.filter( - (item) => item.isSupported === true && !item.hasOptedIn, + (item) => item.isSupported !== false && !item.hasOptedIn, ) ?? [], [flattenedAddressData], ); @@ -260,6 +317,9 @@ const RewardOptInAccountGroupModal: React.FC = () => { const supportedAddresses = flattenedAddressData.filter( (item) => item.isSupported !== false, ); + const unsupportedAddresses = flattenedAddressData.filter( + (item) => item.isSupported === false, + ); const trackedAddresses = supportedAddresses.filter( (item) => item.hasOptedIn, @@ -273,34 +333,36 @@ const RewardOptInAccountGroupModal: React.FC = () => { | ({ type: 'item' } & FlattenedAddressItem) )[] = []; - // Only show headers if there are both tracked and untracked addresses - const showHeaders = - trackedAddresses.length > 0 && untrackedAddresses.length > 0; - if (trackedAddresses.length > 0) { - if (showHeaders) { - data.push({ - type: 'header', - title: strings('rewards.link_account_group.tracked'), - }); - } + data.push({ + type: 'header', + title: strings('rewards.link_account_group.tracked'), + }); trackedAddresses.forEach((item) => { data.push({ type: 'item', ...item }); }); } if (untrackedAddresses.length > 0) { - if (showHeaders) { - data.push({ - type: 'header', - title: strings('rewards.link_account_group.untracked'), - }); - } + data.push({ + type: 'header', + title: strings('rewards.link_account_group.untracked'), + }); untrackedAddresses.forEach((item) => { data.push({ type: 'item', ...item }); }); } + if (unsupportedAddresses.length > 0) { + data.push({ + type: 'header', + title: strings('rewards.link_account_group.unsupported'), + }); + unsupportedAddresses.forEach((item) => { + data.push({ type: 'item', ...item }); + }); + } + return data; }, [flattenedAddressData]); @@ -318,20 +380,6 @@ const RewardOptInAccountGroupModal: React.FC = () => { )} - {unsupportedAddresses.length > 0 && ( - - - - )} - {flatListData.length > 0 && ( { Details: 'details', Check: 'check', Add: 'add', + Minus: 'minus', + }, + TextColor: { + TextAlternative: 'textAlternative', + TextDefault: 'textDefault', + TextMuted: 'textMuted', }, }; }); @@ -325,21 +331,10 @@ jest.mock('../../../../../component-library/components/Texts/Text', () => ({ })); // Mock selectors +const mockSelectIconSeedAddressByAccountGroupId = jest.fn(); jest.mock('../../../../../selectors/multichainAccounts/accounts', () => ({ - selectIconSeedAddressByAccountGroupId: jest.fn(), -})); - -jest.mock('../../../../../selectors/assets/balances', () => ({ - selectBalanceByAccountGroup: jest.fn(), -})); - -jest.mock('../../../../../selectors/preferencesController', () => ({ - selectPrivacyMode: jest.fn(), -})); - -// Mock utility functions -jest.mock('../../../../../util/assets', () => ({ - formatWithThreshold: jest.fn((value: number) => `$${value.toFixed(2)}`), + selectIconSeedAddressByAccountGroupId: (groupId: string) => + mockSelectIconSeedAddressByAccountGroupId(groupId), })); const mockUseSelector = useSelector as jest.MockedFunction; @@ -353,6 +348,7 @@ const mockUseLinkAccountGroup = useLinkAccountGroup as jest.MockedFunction< describe('RewardSettingsAccountGroup', () => { const mockNavigate = jest.fn(); const mockLinkAccountGroup = jest.fn(); + const mockEvmAddress = '0x1234567890123456789012345678901234567890'; const mockAccountGroup: AccountGroupWithOptInStatus = { id: 'keyring:wallet-1/ethereum', @@ -410,6 +406,11 @@ describe('RewardSettingsAccountGroup', () => { beforeEach(() => { jest.clearAllMocks(); + // Mock selectIconSeedAddressByAccountGroupId to return a selector function + mockSelectIconSeedAddressByAccountGroupId.mockReturnValue( + () => mockEvmAddress, + ); + // Mock useNavigation mockUseNavigation.mockReturnValue({ navigate: mockNavigate, @@ -430,28 +431,14 @@ describe('RewardSettingsAccountGroup', () => { isError: false, }); - // Mock useSelector calls + // Mock useSelector to execute the selector function mockUseSelector.mockImplementation((selector) => { - // Mock selectIconSeedAddressByAccountGroupId - if ( - selector.toString().includes('selectIconSeedAddressByAccountGroupId') - ) { - return '0x1234567890123456789012345678901234567890'; - } - - // Mock selectBalanceByAccountGroup - if (selector.toString().includes('selectBalanceByAccountGroup')) { - return { - totalBalanceInUserCurrency: 100.5, - userCurrency: 'usd', - }; + // The selector is a function that takes state and returns a value + // In the component, it calls selectIconSeedAddressByAccountGroupId(accountGroup.id) + // which returns a selector function, then calls that with state + if (typeof selector === 'function') { + return selector({} as never); } - - // Mock selectPrivacyMode - if (selector.toString().includes('selectPrivacyMode')) { - return false; - } - return null; }); }); @@ -496,7 +483,7 @@ describe('RewardSettingsAccountGroup', () => { ).toBeOnTheScreen(); }); - it('should render account group balance when available', () => { + it('should render tracked accounts count', () => { const { getByTestId } = render( { ); expect( - getByTestId(`rewards-account-group-balance-${mockAccountGroup.id}`), + getByTestId(`rewards-account-group-tracked-${mockAccountGroup.id}`), ).toBeOnTheScreen(); }); @@ -546,7 +533,27 @@ describe('RewardSettingsAccountGroup', () => { expect(getByTestId('icon-check')).toBeOnTheScreen(); }); - it('should show add icon when there are opted out accounts', () => { + it('should show add icon when there are only opted out accounts', () => { + const itemWithOnlyOptedOut: RewardSettingsAccountGroupListFlatListItem = { + type: 'accountGroup', + accountGroup: { + ...mockAccountGroup, + optedInAccounts: [], + optedOutAccounts: mockAccountGroup.optedOutAccounts, + }, + }; + + const { getByTestId } = render( + , + ); + + expect(getByTestId('icon-add')).toBeOnTheScreen(); + }); + + it('should show add icon when there are both opted in and opted out accounts', () => { const { getByTestId } = render( { ).toBeNull(); }); - it('should not render balance when balance data is unavailable', () => { - mockUseSelector.mockImplementation((selector) => { - if (selector.toString().includes('selectBalanceByAccountGroup')) { - return null; - } - if ( - selector.toString().includes('selectIconSeedAddressByAccountGroupId') - ) { - return '0x1234567890123456789012345678901234567890'; - } - if (selector.toString().includes('selectPrivacyMode')) { - return false; - } - return null; + it('should use fallback address when evmAddress is undefined', () => { + // Mock selector to throw an error (simulating no accounts found) + mockSelectIconSeedAddressByAccountGroupId.mockReturnValue(() => { + throw new Error('No accounts found'); }); - const { queryByTestId } = render( + const { getByTestId } = render( , ); + // Component should still render with fallback address expect( - queryByTestId(`rewards-account-group-balance-${mockAccountGroup.id}`), - ).toBeNull(); + getByTestId(`rewards-account-group-avatar-${mockAccountGroup.id}`), + ).toBeOnTheScreen(); }); - it('should not render balance when totalBalanceInUserCurrency is zero', () => { - mockUseSelector.mockImplementation((selector) => { - if (selector.toString().includes('selectBalanceByAccountGroup')) { - return { - totalBalanceInUserCurrency: 0, - userCurrency: 'usd', - }; - } - if ( - selector.toString().includes('selectIconSeedAddressByAccountGroupId') - ) { - return '0x1234567890123456789012345678901234567890'; - } - if (selector.toString().includes('selectPrivacyMode')) { - return false; - } - return null; - }); + it('should use fallback address when selector returns undefined', () => { + // Mock selector to return undefined + mockSelectIconSeedAddressByAccountGroupId.mockReturnValue( + () => undefined, + ); - const { queryByTestId } = render( + const { getByTestId } = render( , ); + // Component should still render with fallback address expect( - queryByTestId(`rewards-account-group-balance-${mockAccountGroup.id}`), - ).toBeNull(); + getByTestId(`rewards-account-group-avatar-${mockAccountGroup.id}`), + ).toBeOnTheScreen(); }); - it('should use fallback address when evmAddress is undefined', () => { - mockUseSelector.mockImplementation((selector) => { - if ( - selector.toString().includes('selectIconSeedAddressByAccountGroupId') - ) { - return undefined; - } - if (selector.toString().includes('selectBalanceByAccountGroup')) { - return { - totalBalanceInUserCurrency: 100.5, - userCurrency: 'usd', - }; - } - if (selector.toString().includes('selectPrivacyMode')) { - return false; - } - return null; - }); - - const { getByTestId } = render( + it('should call selectIconSeedAddressByAccountGroupId with correct account group ID', () => { + render( , ); - // Component should still render with fallback address - expect( - getByTestId(`rewards-account-group-avatar-${mockAccountGroup.id}`), - ).toBeOnTheScreen(); + // Verify the selector factory was called with the correct account group ID + expect(mockSelectIconSeedAddressByAccountGroupId).toHaveBeenCalledWith( + mockAccountGroup.id, + ); }); - it('should hide balance when privacy mode is enabled', () => { - mockUseSelector.mockImplementation((selector) => { - if (selector.toString().includes('selectBalanceByAccountGroup')) { - return { - totalBalanceInUserCurrency: 100.5, - userCurrency: 'usd', - }; - } - if ( - selector.toString().includes('selectIconSeedAddressByAccountGroupId') - ) { - return '0x1234567890123456789012345678901234567890'; - } - if (selector.toString().includes('selectPrivacyMode')) { - return true; - } - return null; - }); + it('should not call selector when accountGroup.id is undefined', () => { + const itemWithoutId: RewardSettingsAccountGroupListFlatListItem = { + type: 'accountGroup', + accountGroup: { + ...mockAccountGroup, + id: undefined as never, + }, + }; - const { getByTestId } = render( + render( , ); - // Balance should render but be hidden - const balance = getByTestId( - `rewards-account-group-balance-${mockAccountGroup.id}`, - ); - expect(balance).toBeOnTheScreen(); + // Verify the selector was not called when accountGroup.id is undefined + expect(mockSelectIconSeedAddressByAccountGroupId).not.toHaveBeenCalled(); }); }); @@ -926,7 +881,7 @@ describe('RewardSettingsAccountGroup', () => { getByTestId(`rewards-account-group-name-${mockAccountGroup.id}`), ).toBeOnTheScreen(); expect( - getByTestId(`rewards-account-group-balance-${mockAccountGroup.id}`), + getByTestId(`rewards-account-group-tracked-${mockAccountGroup.id}`), ).toBeOnTheScreen(); expect( getByTestId(`rewards-account-addresses-${mockAccountGroup.id}`), diff --git a/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroup.tsx b/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroup.tsx index a36792f63a7f..195086a49b7a 100644 --- a/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroup.tsx +++ b/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroup.tsx @@ -1,4 +1,4 @@ -import React, { memo, useCallback, useMemo } from 'react'; +import React, { memo, useCallback } from 'react'; import { Box, Text, @@ -9,10 +9,11 @@ import { ButtonIconSize, TextVariant as DsTextVariant, IconName as IconNameDS, + TextColor as DsTextColor, } from '@metamask/design-system-react-native'; import { useTailwind } from '@metamask/design-system-twrnc-preset'; -import I18n from '../../../../../../locales/i18n'; import { isEmpty } from 'lodash'; +import { strings } from '../../../../../../locales/i18n'; import AvatarAccount, { AvatarAccountType, } from '../../../../../component-library/components/Avatars/Avatar/variants/AvatarAccount'; @@ -24,16 +25,6 @@ import { RewardSettingsAccountGroupListFlatListItem } from './types'; import { useSelector } from 'react-redux'; import { RootState } from '../../../../../reducers'; import { selectIconSeedAddressByAccountGroupId } from '../../../../../selectors/multichainAccounts/accounts'; -import { selectBalanceByAccountGroup } from '../../../../../selectors/assets/balances'; -import { selectPrivacyMode } from '../../../../../selectors/preferencesController'; -import SensitiveText, { - SensitiveTextLength, -} from '../../../../../component-library/components/Texts/SensitiveText'; -import { - TextColor, - TextVariant, -} from '../../../../../component-library/components/Texts/Text'; -import { formatWithThreshold } from '../../../../../util/assets'; import { View, ActivityIndicator } from 'react-native'; import { useTheme } from '../../../../../util/theme'; @@ -69,29 +60,6 @@ const RewardSettingsAccountGroup: React.FC = ({ const { linkAccountGroup, isLoading } = useLinkAccountGroup(); const navigation = useNavigation(); - const privacyMode = useSelector(selectPrivacyMode); - - // Get account group balance - const groupBalance = useSelector((state: RootState) => { - if (!accountGroup?.id) return null; - const selector = selectBalanceByAccountGroup(accountGroup.id); - return selector(state); - }); - - const displayBalance = useMemo(() => { - if (!groupBalance?.userCurrency) { - return undefined; - } - return formatWithThreshold( - groupBalance.totalBalanceInUserCurrency, - 0.01, - I18n.locale, - { - style: 'currency', - currency: groupBalance.userCurrency.toUpperCase(), - }, - ); - }, [groupBalance]); // Inline handlers for each account group const handleLinkAccountGroup = useCallback(async () => { @@ -149,10 +117,21 @@ const RewardSettingsAccountGroup: React.FC = ({ return null; } + // Calculate tracked accounts count + const optedInCount = accountGroup.optedInAccounts.length; + const totalTrackableCount = + accountGroup.optedInAccounts.length + accountGroup.optedOutAccounts.length; + + // Determine icon name for link button + const linkButtonIconName = + accountGroup.optedOutAccounts.length === 0 + ? IconNameDS.Check + : IconNameDS.Add; + return ( = ({ {accountGroup.name} - {/* Account Balance */} - {displayBalance && - groupBalance?.totalBalanceInUserCurrency !== undefined && - groupBalance.totalBalanceInUserCurrency > 0 && ( - - {displayBalance} - - )} + {/* Tracked Accounts Count */} + + {strings('rewards.link_account_group.tracked_count', { + optedIn: optedInCount.toString(), + total: totalTrackableCount.toString(), + })} + = ({ ) : ( { data, renderItem, keyExtractor, + getItemType, ListHeaderComponent, ListEmptyComponent, ListFooterComponent, @@ -67,6 +68,7 @@ jest.mock('@shopify/flash-list', () => { index: number; }) => React.ReactElement; keyExtractor: (item: unknown, index: number) => string; + getItemType?: (item: unknown) => string; ListHeaderComponent?: () => React.ReactElement; ListEmptyComponent?: () => React.ReactElement; ListFooterComponent?: () => React.ReactElement; @@ -84,9 +86,15 @@ jest.mock('@shopify/flash-list', () => { data && Array.isArray(data) && data.length > 0 ? data.map((item: unknown, index: number) => { const key = keyExtractor ? keyExtractor(item, index) : index; + const itemType = getItemType ? getItemType(item) : undefined; return ReactActual.createElement( View, - { key, testID: `flash-list-item-${key}` }, + { + key, + testID: `flash-list-item-${key}`, + // Store item type as accessibility label for testing + accessibilityLabel: itemType, + }, renderItem({ item, index, @@ -140,31 +148,37 @@ jest.mock('@metamask/design-system-react-native', () => { onPress, testID, disabled, + isDisabled, ...props }: { children?: React.ReactNode; onPress?: () => void; testID?: string; disabled?: boolean; + isDisabled?: boolean; [key: string]: unknown; - }) => - ReactActual.createElement( + }) => { + const isButtonDisabled = disabled || isDisabled; + return ReactActual.createElement( TouchableOpacity, { onPress, testID, - disabled, + disabled: isButtonDisabled, ...props, }, ReactActual.createElement( RNText, { - disabled, - accessibilityState: disabled ? { disabled: true } : undefined, + disabled: isButtonDisabled, + accessibilityState: isButtonDisabled + ? { disabled: true } + : undefined, }, children, ), - ), + ); + }, TextVariant: { BodyMd: 'BodyMd', BodySm: 'BodySm', @@ -366,6 +380,7 @@ describe('RewardSettingsAccountGroupList', () => { jest.clearAllMocks(); // Mock selectInternalAccountsByGroupId to return accounts for each group + // This selector returns a function that takes state and returns accounts mockSelectInternalAccountsByGroupId.mockImplementation( (groupId: string) => { const mockAccounts: Record = { @@ -379,20 +394,33 @@ describe('RewardSettingsAccountGroupList', () => { { address: '0x1111111111111111111111111111111111111111' }, ], }; - return mockAccounts[groupId] || []; + // Return a selector function that returns the accounts for this group + return () => mockAccounts[groupId] || []; }, ); // Mock useSelector calls - mockUseSelector.mockImplementation((selector) => { + mockUseSelector.mockImplementation((selector: unknown) => { // Mock selectAvatarAccountType selector if (selector === selectAvatarAccountType) { return 'default'; } - // For the allAddresses selector, let it execute normally since we've mocked selectInternalAccountsByGroupId - // The selector will now work correctly with our mocked data - return selector({}); + // For the allAddresses selector, it will call selectInternalAccountsByGroupId + // for each group and build the addresses object + // We need to handle this by executing the selector with a mock state + if (typeof selector === 'function') { + try { + const mockState = {} as never; + return (selector as (state: never) => unknown)(mockState); + } catch { + // If selector fails, return empty object + return {}; + } + } + + // Fallback for unknown selector types + return undefined; }); // Mock useRewardOptinSummary hook @@ -739,6 +767,70 @@ describe('RewardSettingsAccountGroupList', () => { }); }); + describe('getItemType', () => { + it('should return correct item type for wallet items', () => { + const { getByTestId } = render(); + + // The FlashList mock stores item type in accessibilityLabel when getItemType is provided + const walletItem1 = getByTestId('flash-list-item-wallet-wallet-1'); + const walletItem2 = getByTestId('flash-list-item-wallet-wallet-2'); + + expect(walletItem1).toBeOnTheScreen(); + expect(walletItem1.props.accessibilityLabel).toBe('wallet'); + expect(walletItem2).toBeOnTheScreen(); + expect(walletItem2.props.accessibilityLabel).toBe('wallet'); + }); + + it('should return correct item type for account group items', () => { + const { getByTestId } = render(); + + const accountGroup1 = getByTestId('flash-list-item-accountGroup-group-1'); + const accountGroup2 = getByTestId('flash-list-item-accountGroup-group-2'); + const accountGroup3 = getByTestId('flash-list-item-accountGroup-group-3'); + + expect(accountGroup1).toBeOnTheScreen(); + expect(accountGroup1.props.accessibilityLabel).toBe('accountGroup'); + expect(accountGroup2).toBeOnTheScreen(); + expect(accountGroup2.props.accessibilityLabel).toBe('accountGroup'); + expect(accountGroup3).toBeOnTheScreen(); + expect(accountGroup3.props.accessibilityLabel).toBe('accountGroup'); + }); + }); + + describe('allAddresses Data', () => { + it('should pass allAddresses to account group items', () => { + const { getByTestId } = render(); + + // Verify account groups are rendered (they receive allAddresses as prop) + expect(getByTestId('account-group-group-1')).toBeOnTheScreen(); + expect(getByTestId('account-group-group-2')).toBeOnTheScreen(); + expect(getByTestId('account-group-group-3')).toBeOnTheScreen(); + }); + + it('should handle empty addresses for account groups', () => { + // Mock selector to return empty array for a group + mockSelectInternalAccountsByGroupId.mockImplementation( + (groupId: string) => { + const mockAccounts: Record = { + 'group-1': [], + 'group-2': [ + { address: '0x0987654321098765432109876543210987654321' }, + ], + 'group-3': [ + { address: '0x1111111111111111111111111111111111111111' }, + ], + }; + return () => mockAccounts[groupId] || []; + }, + ); + + const { getByTestId } = render(); + + // Should still render even with empty addresses + expect(getByTestId('account-group-group-1')).toBeOnTheScreen(); + }); + }); + describe('Memoization', () => { it('should memoize the component to prevent unnecessary re-renders', () => { const { rerender } = render(); diff --git a/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroupList.tsx b/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroupList.tsx index 2902f225dd4a..715fa41292b7 100644 --- a/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroupList.tsx +++ b/app/components/UI/Rewards/components/Settings/RewardSettingsAccountGroupList.tsx @@ -17,6 +17,7 @@ import { Skeleton } from '../../../../../component-library/components/Skeleton'; import { useRewardOptinSummary } from '../../hooks/useRewardOptinSummary'; import { selectAvatarAccountType } from '../../../../../selectors/settings'; import { selectInternalAccountsByGroupId } from '../../../../../selectors/multichainAccounts/accounts'; +import { AccountGroupId } from '@metamask/account-api'; import Button, { ButtonVariants, } from '../../../../../component-library/components/Buttons/Button'; @@ -43,23 +44,28 @@ const RewardSettingsAccountGroupList: React.FC = () => { refresh: fetchOptInStatus, } = useRewardOptinSummary(); + // Helper function to extract addresses for a single account group + const getAddressesForGroup = useCallback( + (state: RootState, groupId: AccountGroupId): string[] => { + try { + const accounts = selectInternalAccountsByGroupId(state)(groupId); + return accounts.map((account) => account.address).filter(Boolean); + } catch { + return []; + } + }, + [], + ); + // Memoize expensive selector operations to prevent unnecessary re-renders const allAddresses = useSelector((state: RootState) => { const addresses: Record = {}; - byWallet.forEach((walletItem) => { - walletItem.groups.forEach((accountGroup) => { - try { - const accounts = selectInternalAccountsByGroupId(state)( - accountGroup.id, - ); - addresses[accountGroup.id] = accounts - .map((account) => account.address) - .filter(Boolean); - } catch (error) { - addresses[accountGroup.id] = []; - } - }); + const allGroups = byWallet.flatMap((walletItem) => walletItem.groups); + + allGroups.forEach((accountGroup) => { + addresses[accountGroup.id] = getAddressesForGroup(state, accountGroup.id); }); + return addresses; }); @@ -128,7 +134,7 @@ const RewardSettingsAccountGroupList: React.FC = () => { const ListHeaderComponent = useCallback( () => ( - + {strings('rewards.settings.subtitle')} @@ -214,7 +220,7 @@ const RewardSettingsAccountGroupList: React.FC = () => { {[...Array(3)].map((_, index) => ( diff --git a/app/components/Views/BrowserTab/BrowserTab.tsx b/app/components/Views/BrowserTab/BrowserTab.tsx index 73ce163c509f..049c1c54781b 100644 --- a/app/components/Views/BrowserTab/BrowserTab.tsx +++ b/app/components/Views/BrowserTab/BrowserTab.tsx @@ -54,6 +54,8 @@ import { sortMultichainAccountsByLastSelected, } from '../../../core/Permissions'; import Routes from '../../../constants/navigation/Routes'; +import { isInternalDeepLink } from '../../../util/deeplinks'; +import SharedDeeplinkManager from '../../../core/DeeplinkManager/SharedDeeplinkManager'; import { selectIpfsGateway, selectIsIpfsGatewayEnabled, @@ -826,6 +828,29 @@ export const BrowserTab: React.FC = React.memo( } } + // Check if this is an internal MetaMask deeplink that should be handled within the app + if (isInternalDeepLink(urlToLoad)) { + // Handle the deeplink internally instead of passing to OS + SharedDeeplinkManager.parse(urlToLoad, { + origin: AppConstants.DEEPLINKS.ORIGIN_IN_APP_BROWSER, + browserCallBack: (url: string) => { + // If the deeplink handler wants to navigate to a different URL in the browser + if (url && webviewRef.current) { + webviewRef.current.injectJavaScript(` + window.location.href = '${sanitizeUrlInput(url)}'; + true; // Required for iOS + `); + } + }, + }).catch((error) => { + Logger.error( + error, + 'BrowserTab: Failed to handle internal deeplink in browser', + ); + }); + return false; // Stop the webview from loading this URL + } + const { protocol } = new URLParse(urlToLoad); if (trustedProtocolToDeeplink.includes(protocol)) { diff --git a/app/components/hooks/useNetworkSelection/useNetworkSelection.test.ts b/app/components/hooks/useNetworkSelection/useNetworkSelection.test.ts index 7a5a599fa278..ffbe93eea7c6 100644 --- a/app/components/hooks/useNetworkSelection/useNetworkSelection.test.ts +++ b/app/components/hooks/useNetworkSelection/useNetworkSelection.test.ts @@ -1308,21 +1308,6 @@ describe('useNetworkSelection', () => { ); expect(mockEnableNetwork).toHaveBeenCalledWith(bitcoinMainnet); }); - - it('selectPopularNetwork sets selected address for Bitcoin networks', async () => { - const setSelectedAddressSpy = jest.spyOn(Engine, 'setSelectedAddress'); - - const { result } = renderHook(() => - useNetworkSelection({ networks: mockNetworks }), - ); - - await result.current.selectPopularNetwork(bitcoinMainnet); - - expect(setSelectedAddressSpy).toHaveBeenCalledWith( - mockBitcoinAccount.address, - ); - expect(mockEnableNetwork).toHaveBeenCalledWith(bitcoinMainnet); - }); }); describe('error handling and edge cases', () => { diff --git a/app/components/hooks/useNetworkSelection/useNetworkSelection.ts b/app/components/hooks/useNetworkSelection/useNetworkSelection.ts index 104e3318c7f7..ab466ab01a09 100644 --- a/app/components/hooks/useNetworkSelection/useNetworkSelection.ts +++ b/app/components/hooks/useNetworkSelection/useNetworkSelection.ts @@ -205,18 +205,6 @@ export const useNetworkSelection = ({ /** Toggles a popular network and resets all custom networks */ const selectPopularNetwork = useCallback( async (chainId: CaipChainId, onComplete?: () => void) => { - ///: BEGIN:ONLY_INCLUDE_IF(bitcoin) - if (chainId.includes(KnownCaipNamespace.Bip122)) { - const bitcoAccountInScope = bitcoinInternalAccounts.find((account) => - account.scopes.includes(chainId), - ); - - if (bitcoAccountInScope) { - Engine.setSelectedAddress(bitcoAccountInScope.address); - } - } - ///: END:ONLY_INCLUDE_IF - // Enable the network in NetworkEnablementController await enableNetwork(chainId); @@ -225,13 +213,7 @@ export const useNetworkSelection = ({ onComplete?.(); }, - [ - enableNetwork, - switchActiveNetwork, - ///: BEGIN:ONLY_INCLUDE_IF(bitcoin) - bitcoinInternalAccounts, - ///: END:ONLY_INCLUDE_IF(bitcoin) - ], + [enableNetwork, switchActiveNetwork], ); /** Selects a network, automatically handling popular vs custom logic */ diff --git a/app/core/AppConstants.ts b/app/core/AppConstants.ts index 27ffd1de3279..de310f814581 100644 --- a/app/core/AppConstants.ts +++ b/app/core/AppConstants.ts @@ -73,9 +73,11 @@ export default { POLLING_FREQUENCY: 10000, }, DEEPLINKS: { + ORIGIN_CAROUSEL: 'carousel', ORIGIN_DEEPLINK: 'deeplink', ORIGIN_QR_CODE: 'qr-code', ORIGIN_NOTIFICATION: 'notifications', + ORIGIN_IN_APP_BROWSER: 'in-app-browser', }, WALLET_CONNECT: { //One day in hours diff --git a/app/store/migrations/104.test.ts b/app/store/migrations/104.test.ts index df88ebadd861..1018004d78c4 100644 --- a/app/store/migrations/104.test.ts +++ b/app/store/migrations/104.test.ts @@ -1,213 +1,162 @@ import migrate from './104'; -import FilesystemStorage from 'redux-persist-filesystem-storage'; -import Device from '../../util/device'; +import { ensureValidState } from './util'; +import { captureException } from '@sentry/react-native'; -jest.mock('redux-persist-filesystem-storage'); -const mockFilesystemStorage = FilesystemStorage as jest.Mocked< - typeof FilesystemStorage ->; +jest.mock('@sentry/react-native', () => ({ + captureException: jest.fn(), +})); -jest.mock('../../util/device'); -const mockDevice = Device as jest.Mocked; +jest.mock('./util', () => ({ + ensureValidState: jest.fn(), +})); -describe('Migration 104', () => { +const mockedCaptureException = jest.mocked(captureException); +const mockedEnsureValidState = jest.mocked(ensureValidState); + +describe('Migration 104: Reset PhishingController urlScanCache', () => { beforeEach(() => { - jest.clearAllMocks(); - mockDevice.isIos.mockReturnValue(true); - mockFilesystemStorage.setItem.mockResolvedValue(); + jest.resetAllMocks(); }); - it('should migrate existing engine data to individual controller storage', async () => { - const mockState = { - engine: { - backgroundState: { - KeyringController: { - vault: 'encrypted-vault-data', - isUnlocked: false, - }, - NetworkController: { - network: 'mainnet', - chainId: '1', - }, - TransactionController: { - transactions: [{ id: '1', status: 'pending' }], - methodData: { '0x123': { method: 'transfer' } }, - }, - }, - }, - }; - - const result = await migrate(mockState); - - expect(mockFilesystemStorage.setItem).toHaveBeenCalledTimes(3); - - expect(mockFilesystemStorage.setItem).toHaveBeenCalledWith( - 'persist:KeyringController', - JSON.stringify({ - vault: 'encrypted-vault-data', - isUnlocked: false, - }), - true, - ); + it('returns state unchanged if ensureValidState fails', () => { + const state = { some: 'state' }; - expect(mockFilesystemStorage.setItem).toHaveBeenCalledWith( - 'persist:NetworkController', - JSON.stringify({ - network: 'mainnet', - chainId: '1', - }), - true, - ); + mockedEnsureValidState.mockReturnValue(false); - expect(mockFilesystemStorage.setItem).toHaveBeenCalledWith( - 'persist:TransactionController', - JSON.stringify({ - transactions: [{ id: '1', status: 'pending' }], - methodData: { '0x123': { method: 'transfer' } }, - }), - true, - ); + const migratedState = migrate(state); - expect(result).toEqual({ - engine: { - backgroundState: {}, - }, - }); + expect(migratedState).toBe(state); + expect(mockedCaptureException).not.toHaveBeenCalled(); }); - it('should completely clear backgroundState when all controllers migrate successfully', async () => { - const mockState = { + it('captures exception if PhishingController state is invalid', () => { + const state = { engine: { backgroundState: { - KeyringController: { - vault: 'encrypted-vault-data', - }, - NetworkController: { - network: 'mainnet', - }, + // PhishingController is missing }, }, }; - // All migrations succeed - mockFilesystemStorage.setItem.mockResolvedValue(); + mockedEnsureValidState.mockReturnValue(true); - const result = await migrate(mockState); + const migratedState = migrate(state); - expect(mockFilesystemStorage.setItem).toHaveBeenCalledTimes(2); - - // Should completely clear backgroundState when all controllers migrate successfully - expect(result).toEqual({ - engine: { - backgroundState: {}, - }, - }); + expect(migratedState).toEqual(state); + expect(mockedCaptureException).toHaveBeenCalledWith(expect.any(Error)); + expect(mockedCaptureException.mock.calls[0][0].message).toContain( + 'Migration 104: Invalid PhishingController state', + ); }); - it('should handle empty engine data gracefully', async () => { - const mockState = { + it('resets PhishingController urlScanCache to empty object while preserving other fields', () => { + interface TestState { engine: { - backgroundState: {}, - }, - }; - - const result = await migrate(mockState); - - expect(mockFilesystemStorage.setItem).not.toHaveBeenCalled(); - - expect(result).toEqual(mockState); - }); - - it('should handle missing engine data gracefully', async () => { - const mockState = { - engine: {}, - }; - - const result = await migrate(mockState); - - expect(mockFilesystemStorage.setItem).not.toHaveBeenCalled(); - // Should return state unchanged - expect(result).toEqual(mockState); - }); - - it('should handle partial controller data', async () => { - const mockState = { + backgroundState: { + PhishingController: { + c2DomainBlocklistLastFetched: number; + phishingLists: string[]; + whitelist: string[]; + hotlistLastFetched: number; + stalelistLastFetched: number; + urlScanCache: Record; + extraProperty?: string; + }; + OtherController: { + shouldStayUntouched: boolean; + }; + }; + }; + } + + const state: TestState = { engine: { backgroundState: { - KeyringController: { - vault: 'encrypted-vault-data', + PhishingController: { + c2DomainBlocklistLastFetched: 123456789, + phishingLists: ['list1', 'list2'], + whitelist: ['site1', 'site2'], + hotlistLastFetched: 987654321, + stalelistLastFetched: 123123123, + urlScanCache: { + 'example.com': { result: 'safe', timestamp: 1234567890 }, + 'phishing.com': { result: 'malicious', timestamp: 9876543210 }, + }, + extraProperty: 'should remain', }, - TransactionController: { - transactions: [], + OtherController: { + shouldStayUntouched: true, }, }, }, }; - const result = await migrate(mockState); - - expect(mockFilesystemStorage.setItem).toHaveBeenCalledTimes(2); - - expect(mockFilesystemStorage.setItem).toHaveBeenCalledWith( - 'persist:KeyringController', - JSON.stringify({ vault: 'encrypted-vault-data' }), - true, - ); - - expect(mockFilesystemStorage.setItem).toHaveBeenCalledWith( - 'persist:TransactionController', - JSON.stringify({ transactions: [] }), - true, - ); - - expect(result).toEqual({ - engine: { - backgroundState: {}, - }, + mockedEnsureValidState.mockReturnValue(true); + + const migratedState = migrate(state) as typeof state; + + // urlScanCache should be reset to empty object + expect( + migratedState.engine.backgroundState.PhishingController.urlScanCache, + ).toEqual({}); + + // Other fields should remain unchanged + expect( + migratedState.engine.backgroundState.PhishingController + .c2DomainBlocklistLastFetched, + ).toBe(123456789); + expect( + migratedState.engine.backgroundState.PhishingController.phishingLists, + ).toEqual(['list1', 'list2']); + expect( + migratedState.engine.backgroundState.PhishingController.whitelist, + ).toEqual(['site1', 'site2']); + expect( + migratedState.engine.backgroundState.PhishingController + .hotlistLastFetched, + ).toBe(987654321); + expect( + migratedState.engine.backgroundState.PhishingController + .stalelistLastFetched, + ).toBe(123123123); + expect( + migratedState.engine.backgroundState.PhishingController.extraProperty, + ).toBe('should remain'); + + expect(migratedState.engine.backgroundState.OtherController).toEqual({ + shouldStayUntouched: true, }); + + expect(mockedCaptureException).not.toHaveBeenCalled(); }); - it('should handle storage errors gracefully and preserve failed controller state', async () => { - const mockState = { + it('handles error during migration', () => { + // Create state with a PhishingController that throws when urlScanCache is accessed + const state = { engine: { backgroundState: { - KeyringController: { - vault: 'encrypted-vault-data', - }, - NetworkController: { - network: 'mainnet', - }, + PhishingController: Object.defineProperty({}, 'urlScanCache', { + get: () => { + throw new Error('Test error'); + }, + set: () => { + throw new Error('Test error'); + }, + configurable: true, + enumerable: true, + }), }, }, }; - mockFilesystemStorage.setItem - .mockRejectedValueOnce(new Error('Storage error')) - .mockResolvedValueOnce(); - - const result = await migrate(mockState); + mockedEnsureValidState.mockReturnValue(true); - expect(mockFilesystemStorage.setItem).toHaveBeenCalledTimes(2); + const migratedState = migrate(state); - // Should preserve failed controller state to prevent data loss - expect(result).toEqual({ - engine: { - backgroundState: { - KeyringController: { - vault: 'encrypted-vault-data', - }, - // NetworkController should be migrated successfully and removed from backgroundState - }, - }, - }); - }); - - it('should handle invalid state gracefully', async () => { - const invalidState = null; - - const result = await migrate(invalidState); - - expect(result).toBe(invalidState); - expect(mockFilesystemStorage.setItem).not.toHaveBeenCalled(); + expect(migratedState).toEqual(state); + expect(mockedCaptureException).toHaveBeenCalledWith(expect.any(Error)); + expect(mockedCaptureException.mock.calls[0][0].message).toContain( + 'Migration 104: cleaning PhishingController state failed with error', + ); }); }); diff --git a/app/store/migrations/104.ts b/app/store/migrations/104.ts index d86c699f8935..f741eea6693d 100644 --- a/app/store/migrations/104.ts +++ b/app/store/migrations/104.ts @@ -1,101 +1,46 @@ import { hasProperty, isObject } from '@metamask/utils'; import { ensureValidState } from './util'; import { captureException } from '@sentry/react-native'; -import FilesystemStorage from 'redux-persist-filesystem-storage'; -import Device from '../../util/device'; -// Note: Do NOT rely on a static controller list. Iterate discovered -// controllers from the legacy engine.backgroundState to avoid missing any. /** - * Migration to transition from old redux-persist engine data to new individual controller storage system. + * Migration 104: Reset PhishingController urlScanCache * - * This migration: - * 1. Checks if old engine data exists in redux-persist - * 2. Splits the engine data into individual controller files - * 3. Saves each controller to its own file using the new storage system - * 4. Clears the old engine data from redux-persist - * - * @param state - The current MetaMask extension state. - * @returns The updated state with engine data cleared from redux-persist. + * This migration resets only the urlScanCache object in the PhishingController state */ -export default async function migrate(state: unknown) { - if (!ensureValidState(state, 104)) { +const migration = (state: unknown): unknown => { + const migrationVersion = 104; + + if (!ensureValidState(state, migrationVersion)) { return state; } try { - const { engine } = state; - if ( - hasProperty(engine, 'backgroundState') && - isObject(engine.backgroundState) && - Object.keys(engine.backgroundState).length > 0 + !hasProperty(state.engine.backgroundState, 'PhishingController') || + !isObject(state.engine.backgroundState.PhishingController) ) { - const controllers = engine.backgroundState; - let failedControllers = 0; - const failedControllerStates: Record = {}; - - // Migrate every controller present in the legacy backgroundState - for (const controllerName of Object.keys(controllers)) { - try { - if ( - hasProperty(controllers, controllerName) && - isObject(controllers[controllerName]) - ) { - const controllerState = controllers[controllerName]; - - const key = `persist:${controllerName}`; - const value = JSON.stringify(controllerState); - - await FilesystemStorage.setItem(key, value, Device.isIos()); - } - } catch (error) { - failedControllers++; - captureException( - new Error( - `Migration 104: Failed to migrate ${controllerName} to individual storage: ${String( - error, - )}`, - ), - ); - - // Preserve failed controller state to prevent data loss - if (hasProperty(controllers, controllerName)) { - failedControllerStates[controllerName] = - controllers[controllerName]; - } - } - } - - // Only clear successfully migrated controllers, preserve failed ones to prevent data loss - // Create new state object to maintain immutability - const newState = { ...state }; - newState.engine = { - ...engine, - backgroundState: failedControllers > 0 ? failedControllerStates : {}, - }; - - if (failedControllers > 0) { - captureException( - new Error( - `Migration 104: ${failedControllers} controllers failed to migrate, preserving their state in redux-persist`, - ), - ); - } - - return newState; + captureException( + new Error( + `Migration 104: Invalid PhishingController state: '${JSON.stringify( + state.engine.backgroundState.PhishingController, + )}'`, + ), + ); + return state; } + // Only reset the urlScanCache field to an empty object + state.engine.backgroundState.PhishingController.urlScanCache = {}; + return state; } catch (error) { captureException( new Error( - `Migration 104: Failed to migrate from redux-persist to individual controller storage: ${String( - error, - )}`, + `Migration 104: cleaning PhishingController state failed with error: ${error}`, ), ); - return state; } -} +}; + +export default migration; diff --git a/app/store/migrations/105.test.ts b/app/store/migrations/105.test.ts index 8325e06d16e6..53a0a5f7f714 100644 --- a/app/store/migrations/105.test.ts +++ b/app/store/migrations/105.test.ts @@ -13,7 +13,7 @@ jest.mock('./util', () => ({ const mockedCaptureException = jest.mocked(captureException); const mockedEnsureValidState = jest.mocked(ensureValidState); -describe('Migration 105: Reset PhishingController urlScanCache', () => { +describe('Migration 105: Remove RatesController state', () => { beforeEach(() => { jest.resetAllMocks(); }); @@ -29,13 +29,9 @@ describe('Migration 105: Reset PhishingController urlScanCache', () => { expect(mockedCaptureException).not.toHaveBeenCalled(); }); - it('captures exception if PhishingController state is invalid', () => { + it('returns state unchanged if backgroundState is missing', () => { const state = { - engine: { - backgroundState: { - // PhishingController is missing - }, - }, + engine: {}, }; mockedEnsureValidState.mockReturnValue(true); @@ -43,24 +39,20 @@ describe('Migration 105: Reset PhishingController urlScanCache', () => { const migratedState = migrate(state); expect(migratedState).toEqual(state); - expect(mockedCaptureException).toHaveBeenCalledWith(expect.any(Error)); - expect(mockedCaptureException.mock.calls[0][0].message).toContain( - 'Migration 105: Invalid PhishingController state', - ); + expect(mockedCaptureException).not.toHaveBeenCalled(); }); - it('resets PhishingController urlScanCache to empty object while preserving other fields', () => { + it('removes RatesController from backgroundState', () => { interface TestState { engine: { backgroundState: { - PhishingController: { - c2DomainBlocklistLastFetched: number; - phishingLists: string[]; - whitelist: string[]; - hotlistLastFetched: number; - stalelistLastFetched: number; - urlScanCache: Record; - extraProperty?: string; + RatesController?: { + cryptocurrencies: string[]; + fiatCurrency: string; + rates: Record; + }; + MultichainAssetsRatesController: { + someProperty: string; }; OtherController: { shouldStayUntouched: boolean; @@ -72,17 +64,24 @@ describe('Migration 105: Reset PhishingController urlScanCache', () => { const state: TestState = { engine: { backgroundState: { - PhishingController: { - c2DomainBlocklistLastFetched: 123456789, - phishingLists: ['list1', 'list2'], - whitelist: ['site1', 'site2'], - hotlistLastFetched: 987654321, - stalelistLastFetched: 123123123, - urlScanCache: { - 'example.com': { result: 'safe', timestamp: 1234567890 }, - 'phishing.com': { result: 'malicious', timestamp: 9876543210 }, + RatesController: { + cryptocurrencies: ['btc', 'sol'], + fiatCurrency: 'usd', + rates: { + btc: { + conversionRate: 45000, + conversionDate: 1234567890, + usdConversionRate: 45000, + }, + sol: { + conversionRate: 100, + conversionDate: 1234567890, + usdConversionRate: 100, + }, }, - extraProperty: 'should remain', + }, + MultichainAssetsRatesController: { + someProperty: 'should remain', }, OtherController: { shouldStayUntouched: true, @@ -95,34 +94,15 @@ describe('Migration 105: Reset PhishingController urlScanCache', () => { const migratedState = migrate(state) as typeof state; - // urlScanCache should be reset to empty object expect( - migratedState.engine.backgroundState.PhishingController.urlScanCache, - ).toEqual({}); + migratedState.engine.backgroundState.RatesController, + ).toBeUndefined(); - // Other fields should remain unchanged - expect( - migratedState.engine.backgroundState.PhishingController - .c2DomainBlocklistLastFetched, - ).toBe(123456789); - expect( - migratedState.engine.backgroundState.PhishingController.phishingLists, - ).toEqual(['list1', 'list2']); - expect( - migratedState.engine.backgroundState.PhishingController.whitelist, - ).toEqual(['site1', 'site2']); expect( - migratedState.engine.backgroundState.PhishingController - .hotlistLastFetched, - ).toBe(987654321); - expect( - migratedState.engine.backgroundState.PhishingController - .stalelistLastFetched, - ).toBe(123123123); - expect( - migratedState.engine.backgroundState.PhishingController.extraProperty, - ).toBe('should remain'); - + migratedState.engine.backgroundState.MultichainAssetsRatesController, + ).toEqual({ + someProperty: 'should remain', + }); expect(migratedState.engine.backgroundState.OtherController).toEqual({ shouldStayUntouched: true, }); @@ -130,33 +110,62 @@ describe('Migration 105: Reset PhishingController urlScanCache', () => { expect(mockedCaptureException).not.toHaveBeenCalled(); }); - it('handles error during migration', () => { - // Create state with a PhishingController that throws when urlScanCache is accessed - const state = { + it('leaves state unchanged if RatesController does not exist', () => { + interface TestState { engine: { backgroundState: { - PhishingController: Object.defineProperty({}, 'urlScanCache', { - get: () => { - throw new Error('Test error'); - }, - set: () => { - throw new Error('Test error'); - }, - configurable: true, - enumerable: true, - }), + MultichainAssetsRatesController: { + someProperty: string; + }; + OtherController: { + shouldStayUntouched: boolean; + }; + }; + }; + } + + const state: TestState = { + engine: { + backgroundState: { + MultichainAssetsRatesController: { + someProperty: 'should remain', + }, + OtherController: { + shouldStayUntouched: true, + }, }, }, }; mockedEnsureValidState.mockReturnValue(true); + const migratedState = migrate(state) as typeof state; + + expect(migratedState).toEqual(state); + expect(mockedCaptureException).not.toHaveBeenCalled(); + }); + + it('handles error during migration', () => { + const state = { + engine: { + backgroundState: Object.defineProperty({}, 'RatesController', { + get: () => { + throw new Error('Test error'); + }, + configurable: true, + enumerable: true, + }), + }, + }; + + mockedEnsureValidState.mockReturnValue(true); + const migratedState = migrate(state); expect(migratedState).toEqual(state); expect(mockedCaptureException).toHaveBeenCalledWith(expect.any(Error)); expect(mockedCaptureException.mock.calls[0][0].message).toContain( - 'Migration 105: cleaning PhishingController state failed with error', + 'Migration 105 failed', ); }); }); diff --git a/app/store/migrations/105.ts b/app/store/migrations/105.ts index d16233a4262a..66af1713c8a0 100644 --- a/app/store/migrations/105.ts +++ b/app/store/migrations/105.ts @@ -3,9 +3,10 @@ import { ensureValidState } from './util'; import { captureException } from '@sentry/react-native'; /** - * Migration 105: Reset PhishingController urlScanCache + * Migration 105: Remove RatesController state * - * This migration resets only the urlScanCache object in the PhishingController state + * This migration removes the entire RatesController from backgroundState + * as it's no longer used (functionality moved to MultichainAssetsRatesController) */ const migration = (state: unknown): unknown => { const migrationVersion = 105; @@ -15,29 +16,23 @@ const migration = (state: unknown): unknown => { } try { - if ( - !hasProperty(state.engine.backgroundState, 'PhishingController') || - !isObject(state.engine.backgroundState.PhishingController) - ) { - captureException( - new Error( - `Migration 105: Invalid PhishingController state: '${JSON.stringify( - state.engine.backgroundState.PhishingController, - )}'`, - ), - ); + const backgroundState = state?.engine?.backgroundState; + + if (!backgroundState) { return state; } - // Only reset the urlScanCache field to an empty object - state.engine.backgroundState.PhishingController.urlScanCache = {}; + if ( + hasProperty(backgroundState, 'RatesController') && + isObject(backgroundState.RatesController) + ) { + delete backgroundState.RatesController; + } return state; } catch (error) { captureException( - new Error( - `Migration 105: cleaning PhishingController state failed with error: ${error}`, - ), + new Error(`Migration ${migrationVersion} failed: ${error}`), ); return state; } diff --git a/app/store/migrations/106.test.ts b/app/store/migrations/106.test.ts index d5338dbc9763..f1cdd62f44e5 100644 --- a/app/store/migrations/106.test.ts +++ b/app/store/migrations/106.test.ts @@ -1,171 +1,82 @@ +import { MMKV } from 'react-native-mmkv'; import migrate from './106'; -import { ensureValidState } from './util'; -import { captureException } from '@sentry/react-native'; -jest.mock('@sentry/react-native', () => ({ - captureException: jest.fn(), +jest.mock('react-native-mmkv', () => ({ + MMKV: jest.fn(), })); -jest.mock('./util', () => ({ - ensureValidState: jest.fn(), -})); +describe('Migration #106', () => { + const mockMMKVInstance = { + getAllKeys: jest.fn(), + clearAll: jest.fn(), + }; -const mockedCaptureException = jest.mocked(captureException); -const mockedEnsureValidState = jest.mocked(ensureValidState); + (MMKV as jest.Mock).mockImplementation(() => mockMMKVInstance); -describe('Migration 106: Remove RatesController state', () => { beforeEach(() => { - jest.resetAllMocks(); + jest.clearAllMocks(); }); - it('returns state unchanged if ensureValidState fails', () => { - const state = { some: 'state' }; - - mockedEnsureValidState.mockReturnValue(false); - - const migratedState = migrate(state); - - expect(migratedState).toBe(state); - expect(mockedCaptureException).not.toHaveBeenCalled(); + afterAll(() => { + jest.restoreAllMocks(); }); - it('returns state unchanged if backgroundState is missing', () => { - const state = { - engine: {}, + it('should clear PPOM storage when keys exist', () => { + const oldState = { + engine: { + backgroundState: {}, + }, }; - mockedEnsureValidState.mockReturnValue(true); + mockMMKVInstance.getAllKeys.mockReturnValue(['key1-0x1', 'key2-0x1']); - const migratedState = migrate(state); + const newState = migrate(oldState); - expect(migratedState).toEqual(state); - expect(mockedCaptureException).not.toHaveBeenCalled(); + expect(MMKV).toHaveBeenCalledWith({ id: 'PPOMDB' }); + expect(mockMMKVInstance.getAllKeys).toHaveBeenCalled(); + expect(mockMMKVInstance.clearAll).toHaveBeenCalled(); + expect(newState).toEqual(oldState); }); - it('removes RatesController from backgroundState', () => { - interface TestState { - engine: { - backgroundState: { - RatesController?: { - cryptocurrencies: string[]; - fiatCurrency: string; - rates: Record; - }; - MultichainAssetsRatesController: { - someProperty: string; - }; - OtherController: { - shouldStayUntouched: boolean; - }; - }; - }; - } - - const state: TestState = { + it('should not call clearAll when no keys exist', () => { + const oldState = { engine: { - backgroundState: { - RatesController: { - cryptocurrencies: ['btc', 'sol'], - fiatCurrency: 'usd', - rates: { - btc: { - conversionRate: 45000, - conversionDate: 1234567890, - usdConversionRate: 45000, - }, - sol: { - conversionRate: 100, - conversionDate: 1234567890, - usdConversionRate: 100, - }, - }, - }, - MultichainAssetsRatesController: { - someProperty: 'should remain', - }, - OtherController: { - shouldStayUntouched: true, - }, - }, + backgroundState: {}, }, }; - mockedEnsureValidState.mockReturnValue(true); - - const migratedState = migrate(state) as typeof state; + mockMMKVInstance.getAllKeys.mockReturnValue([]); - expect( - migratedState.engine.backgroundState.RatesController, - ).toBeUndefined(); + const newState = migrate(oldState); - expect( - migratedState.engine.backgroundState.MultichainAssetsRatesController, - ).toEqual({ - someProperty: 'should remain', - }); - expect(migratedState.engine.backgroundState.OtherController).toEqual({ - shouldStayUntouched: true, - }); - - expect(mockedCaptureException).not.toHaveBeenCalled(); + expect(MMKV).toHaveBeenCalledWith({ id: 'PPOMDB' }); + expect(mockMMKVInstance.getAllKeys).toHaveBeenCalled(); + expect(mockMMKVInstance.clearAll).not.toHaveBeenCalled(); + expect(newState).toEqual(oldState); }); - it('leaves state unchanged if RatesController does not exist', () => { - interface TestState { - engine: { - backgroundState: { - MultichainAssetsRatesController: { - someProperty: string; - }; - OtherController: { - shouldStayUntouched: boolean; - }; - }; - }; - } - - const state: TestState = { + it('should handle errors gracefully', () => { + const oldState = { engine: { - backgroundState: { - MultichainAssetsRatesController: { - someProperty: 'should remain', - }, - OtherController: { - shouldStayUntouched: true, - }, - }, + backgroundState: {}, }, }; - mockedEnsureValidState.mockReturnValue(true); + mockMMKVInstance.getAllKeys.mockImplementation(() => { + throw new Error('Storage error'); + }); - const migratedState = migrate(state) as typeof state; + const newState = migrate(oldState); - expect(migratedState).toEqual(state); - expect(mockedCaptureException).not.toHaveBeenCalled(); + expect(newState).toEqual(oldState); }); - it('handles error during migration', () => { - const state = { - engine: { - backgroundState: Object.defineProperty({}, 'RatesController', { - get: () => { - throw new Error('Test error'); - }, - configurable: true, - enumerable: true, - }), - }, - }; - - mockedEnsureValidState.mockReturnValue(true); + it('should return state unchanged if state is invalid', () => { + const invalidStates = [null, undefined, {}, { engine: null }]; - const migratedState = migrate(state); - - expect(migratedState).toEqual(state); - expect(mockedCaptureException).toHaveBeenCalledWith(expect.any(Error)); - expect(mockedCaptureException.mock.calls[0][0].message).toContain( - 'Migration 106 failed', - ); + invalidStates.forEach((invalidState) => { + const newState = migrate(invalidState); + expect(newState).toEqual(invalidState); + }); }); }); diff --git a/app/store/migrations/106.ts b/app/store/migrations/106.ts index 1427f2398537..a738c80032de 100644 --- a/app/store/migrations/106.ts +++ b/app/store/migrations/106.ts @@ -1,41 +1,40 @@ -import { hasProperty, isObject } from '@metamask/utils'; -import { ensureValidState } from './util'; import { captureException } from '@sentry/react-native'; +import { MMKV } from 'react-native-mmkv'; +import { ensureValidState } from './util'; + +const migrationVersion = 106; /** - * Migration 106: Remove RatesController state + * Migration 106: Clean up PPOM MMKV storage after removing PPOM local execution * - * This migration removes the entire RatesController from backgroundState - * as it's no longer used (functionality moved to MultichainAssetsRatesController) + * This migration removes any lingering PPOM data stored in MMKV storage + * when the PPOM controller is removed from the codebase. */ -const migration = (state: unknown): unknown => { - const migrationVersion = 106; - +export default function migrate(state: unknown) { if (!ensureValidState(state, migrationVersion)) { return state; } try { - const backgroundState = state?.engine?.backgroundState; + const ppomStorageId = 'PPOMDB'; - if (!backgroundState) { - return state; - } + // Create MMKV instance with the same ID that was used by PPOM + const ppomStorage = new MMKV({ id: ppomStorageId }); - if ( - hasProperty(backgroundState, 'RatesController') && - isObject(backgroundState.RatesController) - ) { - delete backgroundState.RatesController; - } + // Get all keys from the PPOM storage + const allKeys = ppomStorage.getAllKeys(); - return state; + if (allKeys.length > 0) { + // Clear all data from PPOM storage + ppomStorage.clearAll(); + } } catch (error) { captureException( - new Error(`Migration ${migrationVersion} failed: ${error}`), + new Error( + `Migration ${migrationVersion}: Failed to clean up PPOM storage: ${error}`, + ), ); - return state; } -}; -export default migration; + return state; +} diff --git a/app/store/migrations/107.test.ts b/app/store/migrations/107.test.ts deleted file mode 100644 index 55a64d56d53e..000000000000 --- a/app/store/migrations/107.test.ts +++ /dev/null @@ -1,82 +0,0 @@ -import { MMKV } from 'react-native-mmkv'; -import migrate from './107'; - -jest.mock('react-native-mmkv', () => ({ - MMKV: jest.fn(), -})); - -describe('Migration #107', () => { - const mockMMKVInstance = { - getAllKeys: jest.fn(), - clearAll: jest.fn(), - }; - - (MMKV as jest.Mock).mockImplementation(() => mockMMKVInstance); - - beforeEach(() => { - jest.clearAllMocks(); - }); - - afterAll(() => { - jest.restoreAllMocks(); - }); - - it('should clear PPOM storage when keys exist', () => { - const oldState = { - engine: { - backgroundState: {}, - }, - }; - - mockMMKVInstance.getAllKeys.mockReturnValue(['key1-0x1', 'key2-0x1']); - - const newState = migrate(oldState); - - expect(MMKV).toHaveBeenCalledWith({ id: 'PPOMDB' }); - expect(mockMMKVInstance.getAllKeys).toHaveBeenCalled(); - expect(mockMMKVInstance.clearAll).toHaveBeenCalled(); - expect(newState).toEqual(oldState); - }); - - it('should not call clearAll when no keys exist', () => { - const oldState = { - engine: { - backgroundState: {}, - }, - }; - - mockMMKVInstance.getAllKeys.mockReturnValue([]); - - const newState = migrate(oldState); - - expect(MMKV).toHaveBeenCalledWith({ id: 'PPOMDB' }); - expect(mockMMKVInstance.getAllKeys).toHaveBeenCalled(); - expect(mockMMKVInstance.clearAll).not.toHaveBeenCalled(); - expect(newState).toEqual(oldState); - }); - - it('should handle errors gracefully', () => { - const oldState = { - engine: { - backgroundState: {}, - }, - }; - - mockMMKVInstance.getAllKeys.mockImplementation(() => { - throw new Error('Storage error'); - }); - - const newState = migrate(oldState); - - expect(newState).toEqual(oldState); - }); - - it('should return state unchanged if state is invalid', () => { - const invalidStates = [null, undefined, {}, { engine: null }]; - - invalidStates.forEach((invalidState) => { - const newState = migrate(invalidState); - expect(newState).toEqual(invalidState); - }); - }); -}); diff --git a/app/store/migrations/107.ts b/app/store/migrations/107.ts deleted file mode 100644 index 00f8ebdbb9d7..000000000000 --- a/app/store/migrations/107.ts +++ /dev/null @@ -1,40 +0,0 @@ -import { captureException } from '@sentry/react-native'; -import { MMKV } from 'react-native-mmkv'; -import { ensureValidState } from './util'; - -const migrationVersion = 107; - -/** - * Migration 106: Clean up PPOM MMKV storage after removing PPOM local execution - * - * This migration removes any lingering PPOM data stored in MMKV storage - * when the PPOM controller is removed from the codebase. - */ -export default function migrate(state: unknown) { - if (!ensureValidState(state, migrationVersion)) { - return state; - } - - try { - const ppomStorageId = 'PPOMDB'; - - // Create MMKV instance with the same ID that was used by PPOM - const ppomStorage = new MMKV({ id: ppomStorageId }); - - // Get all keys from the PPOM storage - const allKeys = ppomStorage.getAllKeys(); - - if (allKeys.length > 0) { - // Clear all data from PPOM storage - ppomStorage.clearAll(); - } - } catch (error) { - captureException( - new Error( - `Migration ${migrationVersion}: Failed to clean up PPOM storage: ${error}`, - ), - ); - } - - return state; -} diff --git a/app/store/migrations/index.test.ts b/app/store/migrations/index.test.ts index 735f12e88224..66944858111e 100644 --- a/app/store/migrations/index.test.ts +++ b/app/store/migrations/index.test.ts @@ -65,7 +65,7 @@ const asyncMigration = async (state: any) => { }; describe('asyncifyMigrations', () => { - it('should convert synchronous migrations to asynchronous', async () => { + it('converts synchronous migrations to asynchronous', async () => { const testMigrationList = { ...recentMigrations, [numberOfMigrations]: asyncMigration, @@ -109,7 +109,7 @@ describe('migrations', () => { }); }); - it('should migrate successfully when latest migration is synchronous', async () => { + it('migrates successfully when latest migration is synchronous', async () => { const testMigrationList = { ...recentMigrations, [numberOfMigrations]: synchronousMigration, @@ -132,7 +132,7 @@ describe('migrations', () => { expect((migratedState as Record).test).toEqual('sync'); }); - it('should migrate successfully when latest migration is asynchronous', async () => { + it('migrates successfully when latest migration is asynchronous', async () => { const testMigrationList = { ...recentMigrations, [numberOfMigrations]: asyncMigration, @@ -155,7 +155,7 @@ describe('migrations', () => { expect((migratedState as Record).test).toEqual('async'); }); - it('should migrate successfully when using both synchronous and asynchronous migrations', async () => { + it('migrates successfully when using both synchronous and asynchronous migrations', async () => { const testMigrationList = { ...recentMigrations, [numberOfMigrations]: asyncMigration, @@ -194,7 +194,7 @@ describe('Critical Error Handling', () => { }); describe('inflateFromControllers error handling', () => { - it('should crash when ControllerStorage.getAllPersistedState fails', async () => { + it('crashes when ControllerStorage.getAllPersistedState fails', async () => { // Arrange const storageError = new Error('Storage access failed'); mockedControllerStorage.getAllPersistedState.mockRejectedValue( @@ -202,13 +202,13 @@ describe('Critical Error Handling', () => { ); const testMigrationList = { - 105: (state: unknown) => state, // Migration > 104 triggers inflation logic + 107: (state: unknown) => state, // Migration > 106 triggers inflation logic }; const asyncMigrations = asyncifyMigrations(testMigrationList); // Act & Assert - await expect(asyncMigrations['105'](initialState)).rejects.toThrow( + await expect(asyncMigrations['107'](initialState)).rejects.toThrow( 'Critical: Failed to load controller data for migration. Cannot continue safely as migrations may corrupt data without complete state. App will restart to attempt recovery. Error: Error: Storage access failed', ); @@ -220,18 +220,18 @@ describe('Critical Error Handling', () => { ); }); - it('should not crash when no controllers are found (empty state)', async () => { + it('does not crash when no controllers are found (empty state)', async () => { // Arrange mockedControllerStorage.getAllPersistedState.mockResolvedValue({}); const testMigrationList = { - 105: (state: unknown) => ({ ...(state as object), test: 'passed' }), + 107: (state: unknown) => ({ ...(state as object), test: 'passed' }), }; const asyncMigrations = asyncifyMigrations(testMigrationList); // Act - const result = await asyncMigrations['105'](initialState); + const result = await asyncMigrations['107'](initialState); // Assert expect((result as Record).test).toEqual('passed'); @@ -240,7 +240,7 @@ describe('Critical Error Handling', () => { }); describe('deflateToControllersAndStrip error handling', () => { - it('should crash when any controller fails to save during deflation', async () => { + it('crashes when any controller fails to save during deflation', async () => { // Arrange const stateWithControllers = { ...initialState, @@ -270,14 +270,14 @@ describe('Critical Error Handling', () => { }); const testMigrationList = { - 105: (state: unknown) => state, // This will trigger deflation after migration + 107: (state: unknown) => state, // This will trigger deflation after migration (lastVersion >= 106) }; const asyncMigrations = asyncifyMigrations(testMigrationList); // Act & Assert await expect( - asyncMigrations['105'](stateWithControllers), + asyncMigrations['107'](stateWithControllers), ).rejects.toThrow( "Critical: Migration failed for controller 'TestController'. Cannot continue with partial migration as this would corrupt user data. App will restart to attempt recovery. Error: Error: Disk full", ); @@ -290,7 +290,7 @@ describe('Critical Error Handling', () => { ); }); - it('should successfully deflate when all controllers save successfully', async () => { + it('deflates successfully when all controllers save successfully', async () => { // Arrange const stateWithControllers = { ...initialState, @@ -314,13 +314,13 @@ describe('Critical Error Handling', () => { mockedControllerStorage.setItem.mockResolvedValue(); const testMigrationList = { - 105: (state: unknown) => ({ ...(state as object), migrated: true }), + 107: (state: unknown) => ({ ...(state as object), migrated: true }), }; const asyncMigrations = asyncifyMigrations(testMigrationList); // Act - const result = (await asyncMigrations['105']( + const result = (await asyncMigrations['107']( stateWithControllers, )) as Record; @@ -331,7 +331,7 @@ describe('Critical Error Handling', () => { expect(mockedCaptureException).not.toHaveBeenCalled(); // No errors captured }); - it('should crash when deflation fails completely', async () => { + it('crashes when deflation fails completely', async () => { // Arrange const stateWithControllers = { ...initialState, @@ -354,14 +354,14 @@ describe('Critical Error Handling', () => { mockedControllerStorage.setItem.mockRejectedValue(catastrophicError); const testMigrationList = { - 105: (state: unknown) => state, + 107: (state: unknown) => state, }; const asyncMigrations = asyncifyMigrations(testMigrationList); // Act & Assert await expect( - asyncMigrations['105'](stateWithControllers), + asyncMigrations['107'](stateWithControllers), ).rejects.toThrow( "Critical: Migration failed for controller 'TestController'. Cannot continue with partial migration as this would corrupt user data. App will restart to attempt recovery. Error: Error: File system corrupted", ); @@ -376,29 +376,29 @@ describe('Critical Error Handling', () => { }); describe('Migration flow integration', () => { - it('should not trigger inflation/deflation for migrations <= 104', async () => { + it('does not trigger inflation/deflation for migrations < 106', async () => { // Arrange const testMigrationList = { - 104: (state: unknown) => ({ + 105: (state: unknown) => ({ ...(state as object), - test: 'migration104', + test: 'migration105', }), }; const asyncMigrations = asyncifyMigrations(testMigrationList); // Act - const result = await asyncMigrations['104'](initialState); + const result = await asyncMigrations['105'](initialState); // Assert - expect((result as Record).test).toEqual('migration104'); + expect((result as Record).test).toEqual('migration105'); expect( mockedControllerStorage.getAllPersistedState, ).not.toHaveBeenCalled(); expect(mockedControllerStorage.setItem).not.toHaveBeenCalled(); }); - it('should handle mixed migration versions correctly', async () => { + it('handles mixed migration versions correctly', async () => { // Arrange - Reset all mocks to clean state jest.clearAllMocks(); mockedControllerStorage.getAllPersistedState.mockResolvedValue({}); @@ -410,25 +410,25 @@ describe('Critical Error Handling', () => { } as PersistedState; const testMigrationList = { - 103: (state: unknown) => ({ ...(state as object), step103: true }), - 104: (state: unknown) => ({ ...(state as object), step104: true }), 105: (state: unknown) => ({ ...(state as object), step105: true }), + 106: (state: unknown) => ({ ...(state as object), step106: true }), + 107: (state: unknown) => ({ ...(state as object), step107: true }), }; const asyncMigrations = asyncifyMigrations(testMigrationList); // Act - Run migrations in sequence let state = stateWithoutControllers; - state = (await asyncMigrations['103'](state)) as PersistedState; - state = (await asyncMigrations['104'](state)) as PersistedState; - const finalState = await asyncMigrations['105'](state); + state = (await asyncMigrations['105'](state)) as PersistedState; + state = (await asyncMigrations['106'](state)) as PersistedState; + const finalState = await asyncMigrations['107'](state); // Assert - expect((finalState as Record).step103).toBe(true); - expect((finalState as Record).step104).toBe(true); expect((finalState as Record).step105).toBe(true); + expect((finalState as Record).step106).toBe(true); + expect((finalState as Record).step107).toBe(true); - // Inflation should only be called once for migration 105 + // Inflation should only be called once for migration 107 (> 106) expect( mockedControllerStorage.getAllPersistedState, ).toHaveBeenCalledTimes(1); diff --git a/app/store/migrations/index.ts b/app/store/migrations/index.ts index 545910796863..e63084cb88c3 100644 --- a/app/store/migrations/index.ts +++ b/app/store/migrations/index.ts @@ -107,7 +107,6 @@ import migration103 from './103'; import migration104 from './104'; import migration105 from './105'; import migration106 from './106'; -import migration107 from './107'; // Add migrations above this line import { ControllerStorage } from '../persistConfig'; @@ -231,7 +230,6 @@ export const migrationList: MigrationsList = { 104: migration104, 105: migration105, 106: migration106, - 107: migration107, }; // Enable both synchronous and asynchronous migrations @@ -378,13 +376,13 @@ export const asyncifyMigrations = (inputMigrations: MigrationsList) => { ) => { let state = await incomingState; - if (!didInflate && Number(migrationNumber) > 104) { + if (!didInflate && Number(migrationNumber) > 106) { state = await inflateFromControllers(state); didInflate = true; } const migratedState = await migrationFunction(state); - if (Number(migrationNumber) === lastVersion && lastVersion > 104) { + if (Number(migrationNumber) === lastVersion && lastVersion >= 106) { const s2 = migratedState as StateWithEngine; const hasControllers = Boolean( s2.engine?.backgroundState && diff --git a/app/util/deeplinks/index.test.ts b/app/util/deeplinks/index.test.ts new file mode 100644 index 000000000000..fe46622c6f58 --- /dev/null +++ b/app/util/deeplinks/index.test.ts @@ -0,0 +1,83 @@ +import { isInternalDeepLink } from './index'; + +describe('deeplinks utils', () => { + describe('isInternalDeepLink', () => { + it('identifies MetaMask custom scheme deeplinks', () => { + expect(isInternalDeepLink('metamask://connect')).toBe(true); + expect(isInternalDeepLink('metamask://wc?uri=...')).toBe(true); + expect(isInternalDeepLink('metamask://dapp/uniswap.org')).toBe(true); + }); + + it('identifies Ethereum scheme deeplinks', () => { + expect(isInternalDeepLink('ethereum://pay-0x1234')).toBe(true); + expect(isInternalDeepLink('ethereum://0x1234?value=1e18')).toBe(true); + }); + + it('identifies dapp scheme deeplinks', () => { + expect(isInternalDeepLink('dapp://app.uniswap.org')).toBe(true); + expect(isInternalDeepLink('dapp://portfolio.metamask.io')).toBe(true); + }); + + it('identifies MetaMask universal links', () => { + expect(isInternalDeepLink('https://link.metamask.io/swap')).toBe(true); + expect(isInternalDeepLink('https://link.metamask.io/buy-crypto')).toBe( + true, + ); + expect( + isInternalDeepLink('https://link.metamask.io/dapp/uniswap.org'), + ).toBe(true); + }); + + it('identifies MetaMask test universal links', () => { + expect(isInternalDeepLink('https://link-test.metamask.io/swap')).toBe( + true, + ); + expect(isInternalDeepLink('https://link-test.metamask.io/send')).toBe( + true, + ); + }); + + it('identifies MetaMask branch links', () => { + expect(isInternalDeepLink('https://metamask.app.link/swap')).toBe(true); + expect(isInternalDeepLink('https://metamask.test-app.link/home')).toBe( + true, + ); + }); + + it('does not identify external URLs as internal', () => { + expect(isInternalDeepLink('https://google.com')).toBe(false); + expect(isInternalDeepLink('https://uniswap.org')).toBe(false); + expect(isInternalDeepLink('https://portfolio.metamask.io')).toBe(false); + expect(isInternalDeepLink('http://example.com')).toBe(false); + }); + + it('does not identify other protocols as internal', () => { + expect(isInternalDeepLink('mailto:test@example.com')).toBe(false); + expect(isInternalDeepLink('tel:+1234567890')).toBe(false); + expect(isInternalDeepLink('wc://session')).toBe(false); + expect(isInternalDeepLink('https://wc.example.com')).toBe(false); + }); + + it('handles edge cases gracefully', () => { + expect(isInternalDeepLink('')).toBe(false); + expect(isInternalDeepLink(null)).toBe(false); + expect(isInternalDeepLink(undefined)).toBe(false); + expect(isInternalDeepLink('not-a-valid-url')).toBe(false); + expect(isInternalDeepLink('metamask://')).toBe(true); // Still a valid MetaMask scheme + }); + + it('handlesURLs with query parameters and fragments', () => { + expect( + isInternalDeepLink( + 'https://link.metamask.io/swap?chainId=1&token=0x...', + ), + ).toBe(true); + expect( + isInternalDeepLink('metamask://connect?channelId=123#fragment'), + ).toBe(true); + expect(isInternalDeepLink('https://google.com?metamask=true')).toBe( + false, + ); + }); + }); +}); diff --git a/app/util/deeplinks/index.ts b/app/util/deeplinks/index.ts new file mode 100644 index 000000000000..c491d253b99a --- /dev/null +++ b/app/util/deeplinks/index.ts @@ -0,0 +1,55 @@ +import AppConstants from '../../core/AppConstants'; + +const { + MM_UNIVERSAL_LINK_HOST, + MM_IO_UNIVERSAL_LINK_HOST, + MM_IO_UNIVERSAL_LINK_TEST_HOST, +} = AppConstants; + +/** + * Checks if a URL is an internal MetaMask deeplink that should be handled + * within the app rather than passed to the OS + * + * @param url - The URL to check + * @returns true if the URL is a MetaMask internal deeplink + */ +export const isInternalDeepLink = (url: string | null | undefined): boolean => { + if (!url) return false; + + const metamaskHosts = [ + MM_UNIVERSAL_LINK_HOST || 'link.metamask.io', + MM_IO_UNIVERSAL_LINK_HOST || 'link.metamask.io', + MM_IO_UNIVERSAL_LINK_TEST_HOST || 'link-test.metamask.io', + 'metamask.app.link', // todo double-check if we can remove + 'metamask.test-app.link', // todo double-check if we can remove + ].filter(Boolean); + + try { + // Check custom schemes first (more efficient for these cases) + const internalSchemes = ['metamask:', 'ethereum:', 'dapp:']; + if (internalSchemes.some((scheme) => url.startsWith(scheme))) { + return true; + } + + // Parse URL for host checking + const urlObj = new URL(url); + + // Check if it's a MetaMask universal link + return metamaskHosts.includes(urlObj.hostname); + } catch { + // If URL parsing fails, check if it's a simple scheme match + return ['metamask:', 'ethereum:', 'dapp:'].some((scheme) => + url.startsWith(scheme), + ); + } +}; + +/** + * Determines if a URL should be opened externally (Linking.openURL()) + * This is the inverse of isInternalDeepLink but kept separate for clarity + * + * @param url - The URL to check + * @returns true if the URL should be opened externally + */ +export const shouldOpenExternally = (url: string): boolean => + !isInternalDeepLink(url); diff --git a/locales/languages/en.json b/locales/languages/en.json index af5d3310a46d..e1c192593999 100644 --- a/locales/languages/en.json +++ b/locales/languages/en.json @@ -6707,6 +6707,8 @@ "link_account": "Add account", "tracked": "Tracked", "untracked": "Untracked", + "unsupported": "Unsupported", + "tracked_count": "{{optedIn}}/{{total}} tracked", "link_account_success": "{{accountName}} added successfully", "link_account_error": "Failed to add one or more addresses for {{accountName}}" },