From e14cc4879f2bf7fe9f3d1e4e566f663030caeade Mon Sep 17 00:00:00 2001 From: schmanu Date: Fri, 20 Sep 2024 16:42:39 +0200 Subject: [PATCH 1/5] feat: fetch safe overviews using rtk query --- .../common/NetworkSelector/index.tsx | 36 ++-- .../welcome/MyAccounts/AccountItem.tsx | 13 +- .../welcome/MyAccounts/MultiAccountItem.tsx | 24 ++- .../welcome/MyAccounts/PaginatedSafeList.tsx | 28 +-- .../__tests__/useSafeOverviews.test.ts | 56 ------ .../welcome/MyAccounts/useSafeOverviews.ts | 46 ----- src/store/index.ts | 2 + src/store/safeOverviews.ts | 164 ++++++++++++++++++ src/store/slices.ts | 1 + 9 files changed, 223 insertions(+), 147 deletions(-) delete mode 100644 src/components/welcome/MyAccounts/__tests__/useSafeOverviews.test.ts delete mode 100644 src/components/welcome/MyAccounts/useSafeOverviews.ts create mode 100644 src/store/safeOverviews.ts diff --git a/src/components/common/NetworkSelector/index.tsx b/src/components/common/NetworkSelector/index.tsx index ab9c22e4d2..ca881698be 100644 --- a/src/components/common/NetworkSelector/index.tsx +++ b/src/components/common/NetworkSelector/index.tsx @@ -32,13 +32,34 @@ import { useAllSafesGrouped } from '@/components/welcome/MyAccounts/useAllSafesG import useSafeAddress from '@/hooks/useSafeAddress' import { sameAddress } from '@/utils/addresses' import uniq from 'lodash/uniq' -import useSafeOverviews from '@/components/welcome/MyAccounts/useSafeOverviews' import { useCompatibleNetworks } from '@/features/multichain/hooks/useCompatibleNetworks' import { useSafeCreationData } from '@/features/multichain/hooks/useSafeCreationData' import { type ChainInfo } from '@safe-global/safe-gateway-typescript-sdk' import PlusIcon from '@/public/images/common/plus.svg' import useAddressBook from '@/hooks/useAddressBook' import { CreateSafeOnSpecificChain } from '@/features/multichain/components/CreateSafeOnNewChain' +import { useGetSafeOverviewQuery } from '@/store/safeOverviews' + +const ChainIndicatorWithFiatBalance = ({ + isSelected, + chain, + safeAddress, +}: { + isSelected: boolean + chain: ChainInfo + safeAddress: string +}) => { + const { data: safeOverview } = useGetSafeOverviewQuery({ safeAddress, chainId: chain.chainId }) + + return ( + + ) +} export const getNetworkLink = (router: NextRouter, safeAddress: string, networkShortName: string) => { const isSafeOpened = safeAddress !== '' @@ -273,7 +294,6 @@ const NetworkSelector = ({ () => availableChainIds.map((chain) => ({ address: safeAddress, chainId: chain })), [availableChainIds, safeAddress], ) - const [safeOverviews] = useSafeOverviews(multiChainSafes) const onChange = (event: SelectChangeEvent) => { event.preventDefault() // Prevent the link click @@ -290,9 +310,8 @@ const NetworkSelector = ({ const renderMenuItem = useCallback( (chainId: string, isSelected: boolean) => { + useChainId const chain = chains.data.find((chain) => chain.chainId === chainId) - const safeOverview = safeOverviews?.find((overview) => chainId === overview.chainId) - if (!chain) return null return ( @@ -302,17 +321,12 @@ const NetworkSelector = ({ onClick={onChainSelect} className={css.item} > - + ) }, - [chains.data, onChainSelect, router, safeAddress, safeOverviews], + [chains.data, onChainSelect, router, safeAddress], ) return configs.length ? ( diff --git a/src/components/welcome/MyAccounts/AccountItem.tsx b/src/components/welcome/MyAccounts/AccountItem.tsx index 7c524d6252..fc48eeba94 100644 --- a/src/components/welcome/MyAccounts/AccountItem.tsx +++ b/src/components/welcome/MyAccounts/AccountItem.tsx @@ -26,6 +26,8 @@ import FiatValue from '@/components/common/FiatValue' import QueueActions from './QueueActions' import { useGetHref } from './useGetHref' import { extractCounterfactualSafeSetup, isPredictedSafeProps } from '@/features/counterfactual/utils' +import { useGetSafeOverviewQuery } from '@/store/safeOverviews' +import useWallet from '@/hooks/wallets/useWallet' type AccountItemProps = { safeItem: SafeItem @@ -33,7 +35,7 @@ type AccountItemProps = { onLinkClick?: () => void } -const AccountItem = ({ onLinkClick, safeItem, safeOverview }: AccountItemProps) => { +const AccountItem = ({ onLinkClick, safeItem }: AccountItemProps) => { const { chainId, address } = safeItem const chain = useAppSelector((state) => selectChainById(state, chainId)) const undeployedSafe = useAppSelector((state) => selectUndeployedSafe(state, chainId, address)) @@ -42,6 +44,7 @@ const AccountItem = ({ onLinkClick, safeItem, safeOverview }: AccountItemProps) const router = useRouter() const isCurrentSafe = chainId === currChainId && sameAddress(safeAddress, address) const isWelcomePage = router.pathname === AppRoutes.welcome.accounts + const { address: walletAddress } = useWallet() ?? {} const trackingLabel = isWelcomePage ? OVERVIEW_LABELS.login_page : OVERVIEW_LABELS.sidebar @@ -61,6 +64,14 @@ const AccountItem = ({ onLinkClick, safeItem, safeOverview }: AccountItemProps) const isReplayable = !safeItem.isWatchlist && (!undeployedSafe || !isPredictedSafeProps(undeployedSafe.props)) + const { data: safeOverview } = useGetSafeOverviewQuery({ + chainId: safeItem.chainId, + safeAddress: safeItem.address, + walletAddress, + }) + + console.log('Resulting overview', safeOverview) + return ( { ) } -const MultiAccountItem = ({ onLinkClick, multiSafeAccountItem, safeOverviews }: MultiAccountItemProps) => { +const MultiAccountItem = ({ onLinkClick, multiSafeAccountItem }: MultiAccountItemProps) => { const { address, safes } = multiSafeAccountItem const undeployedSafes = useAppSelector(selectUndeployedSafes) const safeAddress = useSafeAddress() @@ -90,6 +93,10 @@ const MultiAccountItem = ({ onLinkClick, multiSafeAccountItem, safeOverviews }: return Object.values(allAddressBooks).find((ab) => ab[address] !== undefined)?.[address] }, [address, allAddressBooks]) + const currency = useAppSelector(selectCurrency) + const { address: walletAddress } = useWallet() ?? {} + const { data: safeOverviews } = useGetMultipleSafeOverviewsQuery({ currency, walletAddress, safes }) + const sharedSetup = useMemo( () => getSharedSetup(safes, safeOverviews ?? [], undeployedSafes), [safeOverviews, safes, undeployedSafes], @@ -110,11 +117,14 @@ const MultiAccountItem = ({ onLinkClick, multiSafeAccountItem, safeOverviews }: [safes, undeployedSafes], ) - const findOverview = (item: SafeItem) => { - return safeOverviews?.find( - (overview) => item.chainId === overview.chainId && sameAddress(overview.address.value, item.address), - ) - } + const findOverview = useCallback( + (item: SafeItem) => { + return safeOverviews?.find( + (overview) => item.chainId === overview.chainId && sameAddress(overview.address.value, item.address), + ) + }, + [safeOverviews], + ) return ( { - const flattenedSafes = useMemo( - () => safes.flatMap((safe) => (isMultiChainSafeItem(safe) ? safe.safes : safe)), - [safes], - ) - const [overviews] = useSafeOverviews(flattenedSafes) - - const findOverview = (item: SafeItem) => { - return overviews?.find( - (overview) => item.chainId === overview.chainId && sameAddress(overview.address.value, item.address), - ) - } - return ( <> {safes.map((item) => isMultiChainSafeItem(item) ? ( - sameAddress(overview.address.value, item.address))} - /> + ) : ( - + ), )} diff --git a/src/components/welcome/MyAccounts/__tests__/useSafeOverviews.test.ts b/src/components/welcome/MyAccounts/__tests__/useSafeOverviews.test.ts deleted file mode 100644 index dc20ee92cd..0000000000 --- a/src/components/welcome/MyAccounts/__tests__/useSafeOverviews.test.ts +++ /dev/null @@ -1,56 +0,0 @@ -import useSafeOverviews from '../useSafeOverviews' -import * as balances from '@/hooks/loadables/useLoadBalances' -import * as sdk from '@safe-global/safe-gateway-typescript-sdk' -import * as useWallet from '@/hooks/wallets/useWallet' -import * as store from '@/store' -import type { Eip1193Provider } from 'ethers' -import { renderHook } from '@testing-library/react' -import { act } from 'react-dom/test-utils' - -jest.spyOn(balances, 'useTokenListSetting').mockReturnValue(false) -jest.spyOn(store, 'useAppSelector').mockReturnValue('USD') -jest - .spyOn(useWallet, 'default') - .mockReturnValue({ label: 'MetaMask', chainId: '1', address: '0x1234', provider: null as unknown as Eip1193Provider }) - -describe('useSafeOverviews', () => { - it('should filter out undefined addresses', async () => { - const spy = jest.spyOn(sdk, 'getSafeOverviews').mockResolvedValue([]) - const safes = [ - { address: '0x1234', chainId: '1' }, - { address: undefined as unknown as string, chainId: '2' }, - { address: '0x5678', chainId: '3' }, - ] - - renderHook(() => useSafeOverviews(safes)) - - await act(() => Promise.resolve()) - - expect(spy).toHaveBeenCalledWith(['1:0x1234', '3:0x5678'], { - currency: 'USD', - exclude_spam: false, - trusted: true, - wallet_address: '0x1234', - }) - }) - - it('should filter out undefined chain ids', async () => { - const spy = jest.spyOn(sdk, 'getSafeOverviews').mockResolvedValue([]) - const safes = [ - { address: '0x1234', chainId: '1' }, - { address: '0x5678', chainId: undefined as unknown as string }, - { address: '0x5678', chainId: '3' }, - ] - - renderHook(() => useSafeOverviews(safes)) - - await act(() => Promise.resolve()) - - expect(spy).toHaveBeenCalledWith(['1:0x1234', '3:0x5678'], { - currency: 'USD', - exclude_spam: false, - trusted: true, - wallet_address: '0x1234', - }) - }) -}) diff --git a/src/components/welcome/MyAccounts/useSafeOverviews.ts b/src/components/welcome/MyAccounts/useSafeOverviews.ts deleted file mode 100644 index 7c3b2ff2e1..0000000000 --- a/src/components/welcome/MyAccounts/useSafeOverviews.ts +++ /dev/null @@ -1,46 +0,0 @@ -import { useMemo } from 'react' -import { useTokenListSetting } from '@/hooks/loadables/useLoadBalances' -import useAsync, { type AsyncResult } from '@/hooks/useAsync' -import useWallet from '@/hooks/wallets/useWallet' -import { useAppSelector } from '@/store' -import { selectCurrency } from '@/store/settingsSlice' -import { type SafeOverview, getSafeOverviews } from '@safe-global/safe-gateway-typescript-sdk' - -const _cache: Record = {} - -type SafeParams = { - address: string - chainId: string -} - -// EIP155 address format -const makeSafeId = ({ chainId, address }: SafeParams) => `${chainId}:${address}` as `${number}:0x${string}` - -const validateSafeParams = ({ chainId, address }: SafeParams) => chainId != null && address != null - -function useSafeOverviews(safes: Array): AsyncResult { - const excludeSpam = useTokenListSetting() || false - const currency = useAppSelector(selectCurrency) - const wallet = useWallet() - const walletAddress = wallet?.address - const safesIds = useMemo(() => safes.filter(validateSafeParams).map(makeSafeId), [safes]) - - const [data, error, isLoading] = useAsync(async () => { - return await getSafeOverviews(safesIds, { - trusted: true, - exclude_spam: excludeSpam, - currency, - wallet_address: walletAddress, - }) - }, [safesIds, excludeSpam, currency, walletAddress]) - - const cacheKey = safesIds.join() - const result = data ?? _cache[cacheKey] - - // Cache until the next page load - _cache[cacheKey] = result - - return useMemo(() => [result, error, isLoading], [result, error, isLoading]) -} - -export default useSafeOverviews diff --git a/src/store/index.ts b/src/store/index.ts index f1b02b21cd..232a2910f4 100644 --- a/src/store/index.ts +++ b/src/store/index.ts @@ -50,6 +50,7 @@ const rootReducer = combineReducers({ [ofacApi.reducerPath]: ofacApi.reducer, [safePassApi.reducerPath]: safePassApi.reducer, [slices.gatewayApi.reducerPath]: slices.gatewayApi.reducer, + [slices.safeOverviewApi.reducerPath]: slices.safeOverviewApi.reducer, }) const persistedSlices: (keyof Partial)[] = [ @@ -80,6 +81,7 @@ const middleware: Middleware<{}, RootState>[] = [ ofacApi.middleware, safePassApi.middleware, slices.gatewayApi.middleware, + slices.safeOverviewApi.middleware, ] const listeners = [safeMessagesListener, txHistoryListener, txQueueListener, swapOrderListener, swapOrderStatusListener] diff --git a/src/store/safeOverviews.ts b/src/store/safeOverviews.ts new file mode 100644 index 0000000000..c3a2c44baf --- /dev/null +++ b/src/store/safeOverviews.ts @@ -0,0 +1,164 @@ +import { createApi } from '@reduxjs/toolkit/query/react' + +import { type SafeOverview, getSafeOverviews } from '@safe-global/safe-gateway-typescript-sdk' +import { sameAddress } from '@/utils/addresses' +import type { RootState } from '.' +import { selectCurrency } from './settingsSlice' +import { type SafeItem } from '@/components/welcome/MyAccounts/useAllSafes' + +const noopBaseQuery = async () => ({ data: null }) + +type SafeOverviewQueueItem = { + safeAddress: string + walletAddress?: string + chainId: string + currency: string + callback: (result: { data: SafeOverview | undefined; error?: never } | { data?: never; error: string }) => void +} + +const BATCH_SIZE = 5 +const FETCH_TIMEOUT = 200 + +const makeSafeId = (chainId: string, address: string) => `${chainId}:${address}` as `${number}:0x${string}` + +class SafeOverviewFetcher { + private requestQueue: SafeOverviewQueueItem[] = [] + + private fetchTimeout: NodeJS.Timeout | null = null + + private async fetchSafeOverviews({ + safeIds, + walletAddress, + currency, + }: { + safeIds: `${number}:0x${string}`[] + walletAddress?: string + currency: string + }) { + return await getSafeOverviews(safeIds, { + trusted: true, + exclude_spam: true, + currency, + wallet_address: walletAddress, + }) + } + + private async processQueuedItems() { + // Dequeue the first BATCH_SIZE items + const nextBatch = this.requestQueue.slice(0, BATCH_SIZE) + this.requestQueue = this.requestQueue.slice(BATCH_SIZE) + + console.log('[SafeOverviewFetcher] processing Queued Items', nextBatch, [...this.requestQueue]) + + let overviews: SafeOverview[] + try { + this.fetchTimeout && clearTimeout(this.fetchTimeout) + this.fetchTimeout = null + + const safeIds = nextBatch.map((request) => makeSafeId(request.chainId, request.safeAddress)) + const { walletAddress, currency } = nextBatch[0] + overviews = await this.fetchSafeOverviews({ safeIds, currency, walletAddress }) + } catch (err) { + // Overviews could not be fetched + nextBatch.forEach((item) => item.callback({ error: 'Could not fetch Safe overview' })) + return + } + + nextBatch.forEach((item) => { + const overview = overviews.find( + (entry) => sameAddress(entry.address.value, item.safeAddress) && entry.chainId === item.chainId, + ) + + item.callback({ data: overview }) + }) + } + + private enqueueRequest(item: SafeOverviewQueueItem) { + console.log('[SafeOverviewFetcher] Enqueueing Request', item) + + this.requestQueue.push(item) + + if (this.requestQueue.length >= BATCH_SIZE) { + this.processQueuedItems() + } + + // If no timer is running start a timer + if (this.fetchTimeout === null) { + this.fetchTimeout = setTimeout(() => { + console.log('Timeout triggered') + this.processQueuedItems() + }, FETCH_TIMEOUT) + } + } + + async getOverview(item: Omit) { + return new Promise((resolve, reject) => { + this.enqueueRequest({ + ...item, + callback: (result) => { + console.log('[SafeOverviewFetcher] GetOverview Callback triggered', item, result) + if ('data' in result) { + resolve(result.data) + } + reject(result.error) + }, + }) + }) + } +} + +const batchedFetcher = new SafeOverviewFetcher() + +type MultiOverviewQueryParams = { + currency: string + walletAddress?: string + safes: SafeItem[] +} + +export const safeOverviewApi = createApi({ + reducerPath: 'safeOverviewApi', + baseQuery: noopBaseQuery, + endpoints: (builder) => ({ + getSafeOverview: builder.query< + SafeOverview | undefined, + { safeAddress: string; walletAddress?: string; chainId: string } + >({ + async queryFn({ safeAddress, walletAddress, chainId }, { getState }) { + const state = getState() + const currency = selectCurrency(state as RootState) + + try { + const safeOverview = await batchedFetcher.getOverview({ chainId, currency, walletAddress, safeAddress }) + return { data: safeOverview } + } catch (error) { + return { error: { status: 'CUSTOM_ERROR', data: (error as Error).message } } + } + }, + }), + getMultipleSafeOverviews: builder.query({ + async queryFn(params) { + const { safes, walletAddress, currency } = params + + try { + const promisedSafeOverviews = safes.map((safe) => + batchedFetcher.getOverview({ + chainId: safe.chainId, + safeAddress: safe.address, + currency, + walletAddress, + }), + ) + const safeOverviews = await Promise.all(promisedSafeOverviews) + return { data: safeOverviews.filter(Boolean) as SafeOverview[] } + } catch (error) { + return { error: { status: 'CUSTOM_ERROR', data: (error as Error).message } } + } + }, + }), + }), +}) + +// Export hooks for usage in functional components, which are +// auto-generated based on the defined endpoints +export const { useGetSafeOverviewQuery, useLazyGetSafeOverviewQuery, useGetMultipleSafeOverviewsQuery } = + safeOverviewApi diff --git a/src/store/slices.ts b/src/store/slices.ts index c0dce32a5a..d105316365 100644 --- a/src/store/slices.ts +++ b/src/store/slices.ts @@ -20,3 +20,4 @@ export * from '@/features/counterfactual/store/undeployedSafesSlice' export * from '@/features/swap/store/swapParamsSlice' export * from './swapOrderSlice' export * from './gateway' +export * from './safeOverviews' From 3d433adeebec7f265e4b3a237e0a6c0dd48b51c6 Mon Sep 17 00:00:00 2001 From: schmanu Date: Tue, 24 Sep 2024 19:11:31 +0200 Subject: [PATCH 2/5] test: unit test for safeOverview query --- src/store/__tests__/safeOverviews.test.ts | 171 ++++++++++++++++++++++ src/store/safeOverviews.ts | 12 +- 2 files changed, 177 insertions(+), 6 deletions(-) create mode 100644 src/store/__tests__/safeOverviews.test.ts diff --git a/src/store/__tests__/safeOverviews.test.ts b/src/store/__tests__/safeOverviews.test.ts new file mode 100644 index 0000000000..260164885d --- /dev/null +++ b/src/store/__tests__/safeOverviews.test.ts @@ -0,0 +1,171 @@ +import { renderHook, waitFor } from '@/tests/test-utils' +import { _FETCH_TIMEOUT, useGetSafeOverviewQuery } from '../safeOverviews' +import { faker } from '@faker-js/faker' +import { getSafeOverviews } from '@safe-global/safe-gateway-typescript-sdk' + +jest.mock('@safe-global/safe-gateway-typescript-sdk') + +describe('safeOverviews', () => { + const mockedGetSafeOverviews = getSafeOverviews as jest.MockedFunction + + beforeAll(() => { + jest.useFakeTimers() + }) + + beforeEach(() => { + jest.resetAllMocks() + }) + + afterAll(() => { + jest.useRealTimers() + }) + describe('useGetSafeOverviewQuery', () => { + it('should return an error if fetching fails', async () => { + const request = { chainId: '1', safeAddress: faker.finance.ethereumAddress() } + mockedGetSafeOverviews.mockRejectedValue('Service unavailable') + + const { result } = renderHook(() => useGetSafeOverviewQuery(request)) + + // Request should get queued and remain loading for the queue seconds + expect(result.current.isLoading).toBeTruthy() + + jest.advanceTimersByTime(_FETCH_TIMEOUT) + await waitFor(() => { + expect(result.current.isLoading).toBeFalsy() + expect(result.current.error).toBeDefined() + expect(result.current.data).toBeUndefined() + }) + }) + + it('should return the Safe overview if fetching is successful', async () => { + const request = { chainId: '1', safeAddress: faker.finance.ethereumAddress() } + + const mockOverview = { + address: { value: request.safeAddress }, + chainId: '1', + awaitingConfirmation: null, + fiatTotal: '100', + owners: [{ value: faker.finance.ethereumAddress() }], + threshold: 1, + queued: 0, + } + mockedGetSafeOverviews.mockResolvedValueOnce([mockOverview]) + + const { result } = renderHook(() => useGetSafeOverviewQuery(request)) + + // Request should get queued and remain loading for the queue seconds + expect(result.current.isLoading).toBeTruthy() + + jest.advanceTimersByTime(_FETCH_TIMEOUT) + + await Promise.resolve() + + await waitFor(() => { + expect(mockedGetSafeOverviews).toHaveBeenCalled() + expect(result.current.isLoading).toBeFalsy() + expect(result.current.error).toBeUndefined() + expect(result.current.data).toEqual(mockOverview) + }) + }) + + it('should immediately process queue if BATCH SIZE elements are queued', async () => { + const fakeSafeAddress = faker.finance.ethereumAddress() + const requests = [ + { chainId: '1', safeAddress: fakeSafeAddress }, + { chainId: '2', safeAddress: fakeSafeAddress }, + { chainId: '3', safeAddress: fakeSafeAddress }, + { chainId: '4', safeAddress: fakeSafeAddress }, + { chainId: '5', safeAddress: fakeSafeAddress }, + { chainId: '6', safeAddress: fakeSafeAddress }, + { chainId: '7', safeAddress: fakeSafeAddress }, + { chainId: '8', safeAddress: fakeSafeAddress }, + { chainId: '9', safeAddress: fakeSafeAddress }, + { chainId: '10', safeAddress: fakeSafeAddress }, + ] + + const mockOverviews = requests.map((request, idx) => ({ + address: { value: request.safeAddress }, + chainId: (idx + 1).toString(), + awaitingConfirmation: null, + fiatTotal: '100', + owners: [{ value: faker.finance.ethereumAddress() }], + threshold: 1, + queued: 0, + })) + + mockedGetSafeOverviews.mockResolvedValueOnce(mockOverviews) + + const { result: result0 } = renderHook(() => useGetSafeOverviewQuery(requests[0])) + const { result: result1 } = renderHook(() => useGetSafeOverviewQuery(requests[1])) + const { result: result2 } = renderHook(() => useGetSafeOverviewQuery(requests[2])) + const { result: result3 } = renderHook(() => useGetSafeOverviewQuery(requests[3])) + const { result: result4 } = renderHook(() => useGetSafeOverviewQuery(requests[4])) + const { result: result5 } = renderHook(() => useGetSafeOverviewQuery(requests[5])) + const { result: result6 } = renderHook(() => useGetSafeOverviewQuery(requests[6])) + const { result: result7 } = renderHook(() => useGetSafeOverviewQuery(requests[7])) + const { result: result8 } = renderHook(() => useGetSafeOverviewQuery(requests[8])) + + // After 9 requests they should all be loading + expect(result0.current.isLoading).toBeTruthy() + expect(result1.current.isLoading).toBeTruthy() + expect(result2.current.isLoading).toBeTruthy() + expect(result3.current.isLoading).toBeTruthy() + expect(result4.current.isLoading).toBeTruthy() + expect(result5.current.isLoading).toBeTruthy() + expect(result6.current.isLoading).toBeTruthy() + expect(result7.current.isLoading).toBeTruthy() + expect(result8.current.isLoading).toBeTruthy() + + expect(mockedGetSafeOverviews).not.toHaveBeenCalled() + + // Trigger the 10th hook - causing all values to load + const { result: result9 } = renderHook(() => useGetSafeOverviewQuery(requests[9])) + + await waitFor(() => { + // Wait until they all resolve + expect(result0.current.isLoading).toBeFalsy() + expect(result1.current.isLoading).toBeFalsy() + expect(result2.current.isLoading).toBeFalsy() + expect(result3.current.isLoading).toBeFalsy() + expect(result4.current.isLoading).toBeFalsy() + expect(result5.current.isLoading).toBeFalsy() + expect(result6.current.isLoading).toBeFalsy() + expect(result7.current.isLoading).toBeFalsy() + expect(result8.current.isLoading).toBeFalsy() + expect(result9.current.isLoading).toBeFalsy() + + // One request that batched all requests together should have happened + expect(mockedGetSafeOverviews).toHaveBeenCalledWith( + [ + `1:${fakeSafeAddress}`, + `2:${fakeSafeAddress}`, + `3:${fakeSafeAddress}`, + `4:${fakeSafeAddress}`, + `5:${fakeSafeAddress}`, + `6:${fakeSafeAddress}`, + `7:${fakeSafeAddress}`, + `8:${fakeSafeAddress}`, + `9:${fakeSafeAddress}`, + `10:${fakeSafeAddress}`, + ], + { + currency: 'usd', + trusted: true, + exclude_spam: true, + }, + ) + + expect(result0.current.data).toEqual(mockOverviews[0]) + expect(result1.current.data).toEqual(mockOverviews[1]) + expect(result2.current.data).toEqual(mockOverviews[2]) + expect(result3.current.data).toEqual(mockOverviews[3]) + expect(result4.current.data).toEqual(mockOverviews[4]) + expect(result5.current.data).toEqual(mockOverviews[5]) + expect(result6.current.data).toEqual(mockOverviews[6]) + expect(result7.current.data).toEqual(mockOverviews[7]) + expect(result8.current.data).toEqual(mockOverviews[8]) + expect(result9.current.data).toEqual(mockOverviews[9]) + }) + }) + }) +}) diff --git a/src/store/safeOverviews.ts b/src/store/safeOverviews.ts index c3a2c44baf..aa9f6290e5 100644 --- a/src/store/safeOverviews.ts +++ b/src/store/safeOverviews.ts @@ -16,8 +16,8 @@ type SafeOverviewQueueItem = { callback: (result: { data: SafeOverview | undefined; error?: never } | { data?: never; error: string }) => void } -const BATCH_SIZE = 5 -const FETCH_TIMEOUT = 200 +export const _BATCH_SIZE = 10 +export const _FETCH_TIMEOUT = 100 const makeSafeId = (chainId: string, address: string) => `${chainId}:${address}` as `${number}:0x${string}` @@ -45,8 +45,8 @@ class SafeOverviewFetcher { private async processQueuedItems() { // Dequeue the first BATCH_SIZE items - const nextBatch = this.requestQueue.slice(0, BATCH_SIZE) - this.requestQueue = this.requestQueue.slice(BATCH_SIZE) + const nextBatch = this.requestQueue.slice(0, _BATCH_SIZE) + this.requestQueue = this.requestQueue.slice(_BATCH_SIZE) console.log('[SafeOverviewFetcher] processing Queued Items', nextBatch, [...this.requestQueue]) @@ -78,7 +78,7 @@ class SafeOverviewFetcher { this.requestQueue.push(item) - if (this.requestQueue.length >= BATCH_SIZE) { + if (this.requestQueue.length >= _BATCH_SIZE) { this.processQueuedItems() } @@ -87,7 +87,7 @@ class SafeOverviewFetcher { this.fetchTimeout = setTimeout(() => { console.log('Timeout triggered') this.processQueuedItems() - }, FETCH_TIMEOUT) + }, _FETCH_TIMEOUT) } } From fd0c79d93f981ea1e7750e61c547ec71bdf88171 Mon Sep 17 00:00:00 2001 From: schmanu Date: Wed, 25 Sep 2024 10:01:38 +0200 Subject: [PATCH 3/5] test: stabilize test and add more test cases --- src/store/__tests__/safeOverviews.test.ts | 104 +++++++++++++++++++--- src/store/safeOverviews.ts | 15 ++-- 2 files changed, 99 insertions(+), 20 deletions(-) diff --git a/src/store/__tests__/safeOverviews.test.ts b/src/store/__tests__/safeOverviews.test.ts index 260164885d..137c41003d 100644 --- a/src/store/__tests__/safeOverviews.test.ts +++ b/src/store/__tests__/safeOverviews.test.ts @@ -1,5 +1,5 @@ import { renderHook, waitFor } from '@/tests/test-utils' -import { _FETCH_TIMEOUT, useGetSafeOverviewQuery } from '../safeOverviews' +import { useGetMultipleSafeOverviewsQuery, useGetSafeOverviewQuery } from '../safeOverviews' import { faker } from '@faker-js/faker' import { getSafeOverviews } from '@safe-global/safe-gateway-typescript-sdk' @@ -8,28 +8,20 @@ jest.mock('@safe-global/safe-gateway-typescript-sdk') describe('safeOverviews', () => { const mockedGetSafeOverviews = getSafeOverviews as jest.MockedFunction - beforeAll(() => { - jest.useFakeTimers() - }) - beforeEach(() => { jest.resetAllMocks() }) - afterAll(() => { - jest.useRealTimers() - }) describe('useGetSafeOverviewQuery', () => { it('should return an error if fetching fails', async () => { const request = { chainId: '1', safeAddress: faker.finance.ethereumAddress() } - mockedGetSafeOverviews.mockRejectedValue('Service unavailable') + mockedGetSafeOverviews.mockRejectedValueOnce('Service unavailable') const { result } = renderHook(() => useGetSafeOverviewQuery(request)) // Request should get queued and remain loading for the queue seconds expect(result.current.isLoading).toBeTruthy() - jest.advanceTimersByTime(_FETCH_TIMEOUT) await waitFor(() => { expect(result.current.isLoading).toBeFalsy() expect(result.current.error).toBeDefined() @@ -56,8 +48,6 @@ describe('safeOverviews', () => { // Request should get queued and remain loading for the queue seconds expect(result.current.isLoading).toBeTruthy() - jest.advanceTimersByTime(_FETCH_TIMEOUT) - await Promise.resolve() await waitFor(() => { @@ -168,4 +158,94 @@ describe('safeOverviews', () => { }) }) }) + + describe('useGetMultipleSafeOverviewsQuery', () => { + it('Should return empty list for empty list of Safes', async () => { + const request = { currency: 'usd', safes: [] } + + const { result } = renderHook(() => useGetMultipleSafeOverviewsQuery(request)) + + // Request should get queued and remain loading for the queue seconds + expect(result.current.isLoading).toBeTruthy() + + await Promise.resolve() + await Promise.resolve() + + await Promise.resolve() + await Promise.resolve() + + await waitFor(() => { + expect(result.current.error).toBeUndefined() + expect(result.current.data).toEqual([]) + expect(result.current.isLoading).toBeFalsy() + }) + }) + + it('Should return a response for non-empty list', async () => { + const request = { + currency: 'usd', + safes: [ + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + { address: faker.finance.ethereumAddress(), chainId: '10', isWatchlist: false }, + ], + } + + const mockOverview1 = { + address: { value: request.safes[0].address }, + chainId: '1', + awaitingConfirmation: null, + fiatTotal: '100', + owners: [{ value: faker.finance.ethereumAddress() }], + threshold: 1, + queued: 0, + } + + const mockOverview2 = { + address: { value: request.safes[1].address }, + chainId: '10', + awaitingConfirmation: null, + fiatTotal: '200', + owners: [{ value: faker.finance.ethereumAddress() }], + threshold: 1, + queued: 4, + } + + mockedGetSafeOverviews.mockResolvedValueOnce([mockOverview1, mockOverview2]) + + const { result } = renderHook(() => useGetMultipleSafeOverviewsQuery(request)) + + // Request should get queued and remain loading for the queue seconds + expect(result.current.isLoading).toBeTruthy() + + await waitFor(() => { + expect(result.current.isLoading).toBeFalsy() + expect(result.current.error).toBeUndefined() + expect(result.current.data).toEqual([mockOverview1, mockOverview2]) + }) + }) + + it('Should return an error if fetching fails', async () => { + const request = { + currency: 'usd', + safes: [ + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + { address: faker.finance.ethereumAddress(), chainId: '10', isWatchlist: false }, + ], + } + + mockedGetSafeOverviews.mockRejectedValueOnce('Not available') + + const { result } = renderHook(() => useGetMultipleSafeOverviewsQuery(request)) + + // Request should get queued and remain loading for the queue seconds + expect(result.current.isLoading).toBeTruthy() + + await waitFor(async () => { + await Promise.resolve() + expect(result.current.error).toBeDefined() + expect(result.current.data).toBeUndefined() + expect(result.current.isLoading).toBeFalsy() + }) + }) + }) }) diff --git a/src/store/safeOverviews.ts b/src/store/safeOverviews.ts index aa9f6290e5..fbf77e976d 100644 --- a/src/store/safeOverviews.ts +++ b/src/store/safeOverviews.ts @@ -16,8 +16,8 @@ type SafeOverviewQueueItem = { callback: (result: { data: SafeOverview | undefined; error?: never } | { data?: never; error: string }) => void } -export const _BATCH_SIZE = 10 -export const _FETCH_TIMEOUT = 100 +const _BATCH_SIZE = 10 +const _FETCH_TIMEOUT = 50 const makeSafeId = (chainId: string, address: string) => `${chainId}:${address}` as `${number}:0x${string}` @@ -48,13 +48,16 @@ class SafeOverviewFetcher { const nextBatch = this.requestQueue.slice(0, _BATCH_SIZE) this.requestQueue = this.requestQueue.slice(_BATCH_SIZE) - console.log('[SafeOverviewFetcher] processing Queued Items', nextBatch, [...this.requestQueue]) - let overviews: SafeOverview[] try { this.fetchTimeout && clearTimeout(this.fetchTimeout) this.fetchTimeout = null + if (nextBatch.length === 0) { + // If for some reason the queue was already processed we are done + return + } + const safeIds = nextBatch.map((request) => makeSafeId(request.chainId, request.safeAddress)) const { walletAddress, currency } = nextBatch[0] overviews = await this.fetchSafeOverviews({ safeIds, currency, walletAddress }) @@ -74,8 +77,6 @@ class SafeOverviewFetcher { } private enqueueRequest(item: SafeOverviewQueueItem) { - console.log('[SafeOverviewFetcher] Enqueueing Request', item) - this.requestQueue.push(item) if (this.requestQueue.length >= _BATCH_SIZE) { @@ -85,7 +86,6 @@ class SafeOverviewFetcher { // If no timer is running start a timer if (this.fetchTimeout === null) { this.fetchTimeout = setTimeout(() => { - console.log('Timeout triggered') this.processQueuedItems() }, _FETCH_TIMEOUT) } @@ -96,7 +96,6 @@ class SafeOverviewFetcher { this.enqueueRequest({ ...item, callback: (result) => { - console.log('[SafeOverviewFetcher] GetOverview Callback triggered', item, result) if ('data' in result) { resolve(result.data) } From 2496bbdbbea9d9e1f345be5bdf2e5414ce206eb7 Mon Sep 17 00:00:00 2001 From: schmanu Date: Wed, 25 Sep 2024 10:59:14 +0200 Subject: [PATCH 4/5] fix: do not load safe overview for undeployed Safes --- src/components/common/NetworkSelector/index.tsx | 5 ----- .../welcome/MyAccounts/AccountItem.tsx | 17 ++++++++++------- .../welcome/MyAccounts/MultiAccountItem.tsx | 6 +++++- .../InconsistentSignerSetupWarning.tsx | 6 +++++- src/store/safeOverviews.ts | 2 +- 5 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/components/common/NetworkSelector/index.tsx b/src/components/common/NetworkSelector/index.tsx index 9caebb559c..a37d54feab 100644 --- a/src/components/common/NetworkSelector/index.tsx +++ b/src/components/common/NetworkSelector/index.tsx @@ -293,11 +293,6 @@ const NetworkSelector = ({ [availableChainIds, configs], ) - const multiChainSafes = useMemo( - () => availableChainIds.map((chain) => ({ address: safeAddress, chainId: chain })), - [availableChainIds, safeAddress], - ) - const onChange = (event: SelectChangeEvent) => { event.preventDefault() // Prevent the link click diff --git a/src/components/welcome/MyAccounts/AccountItem.tsx b/src/components/welcome/MyAccounts/AccountItem.tsx index fc48eeba94..53b5f2093c 100644 --- a/src/components/welcome/MyAccounts/AccountItem.tsx +++ b/src/components/welcome/MyAccounts/AccountItem.tsx @@ -28,6 +28,7 @@ import { useGetHref } from './useGetHref' import { extractCounterfactualSafeSetup, isPredictedSafeProps } from '@/features/counterfactual/utils' import { useGetSafeOverviewQuery } from '@/store/safeOverviews' import useWallet from '@/hooks/wallets/useWallet' +import { skipToken } from '@reduxjs/toolkit/query' type AccountItemProps = { safeItem: SafeItem @@ -64,13 +65,15 @@ const AccountItem = ({ onLinkClick, safeItem }: AccountItemProps) => { const isReplayable = !safeItem.isWatchlist && (!undeployedSafe || !isPredictedSafeProps(undeployedSafe.props)) - const { data: safeOverview } = useGetSafeOverviewQuery({ - chainId: safeItem.chainId, - safeAddress: safeItem.address, - walletAddress, - }) - - console.log('Resulting overview', safeOverview) + const { data: safeOverview } = useGetSafeOverviewQuery( + undeployedSafe + ? skipToken + : { + chainId: safeItem.chainId, + safeAddress: safeItem.address, + walletAddress, + }, + ) return ( safes.filter((safe) => undeployedSafes[safe.chainId]?.[safe.address] === undefined), + [safes, undeployedSafes], + ) + const { data: safeOverviews } = useGetMultipleSafeOverviewsQuery({ currency, walletAddress, safes: deployedSafes }) const safeSetups = useMemo( () => getSafeSetups(safes, safeOverviews ?? [], undeployedSafes), diff --git a/src/features/multichain/components/SignerSetupWarning/InconsistentSignerSetupWarning.tsx b/src/features/multichain/components/SignerSetupWarning/InconsistentSignerSetupWarning.tsx index 0316296a10..b4ecbd2c7b 100644 --- a/src/features/multichain/components/SignerSetupWarning/InconsistentSignerSetupWarning.tsx +++ b/src/features/multichain/components/SignerSetupWarning/InconsistentSignerSetupWarning.tsx @@ -44,7 +44,11 @@ export const InconsistentSignerSetupWarning = () => { () => allMultiChainSafes?.find((account) => sameAddress(safeAddress, account.safes[0].address))?.safes ?? [], [allMultiChainSafes, safeAddress], ) - const { data: safeOverviews } = useGetMultipleSafeOverviewsQuery({ safes: multiChainGroupSafes, currency }) + const deployedSafes = useMemo( + () => multiChainGroupSafes.filter((safe) => undeployedSafes[safe.chainId]?.[safe.address] === undefined), + [multiChainGroupSafes, undeployedSafes], + ) + const { data: safeOverviews } = useGetMultipleSafeOverviewsQuery({ safes: deployedSafes, currency }) const safeSetups = useMemo( () => getSafeSetups(multiChainGroupSafes, safeOverviews ?? [], undeployedSafes), diff --git a/src/store/safeOverviews.ts b/src/store/safeOverviews.ts index fbf77e976d..e3632d8cb0 100644 --- a/src/store/safeOverviews.ts +++ b/src/store/safeOverviews.ts @@ -54,7 +54,7 @@ class SafeOverviewFetcher { this.fetchTimeout = null if (nextBatch.length === 0) { - // If for some reason the queue was already processed we are done + // Nothing to process return } From a84a4e0adab5a6d421c58da0536b0cac5adc44e8 Mon Sep 17 00:00:00 2001 From: schmanu Date: Wed, 25 Sep 2024 15:46:52 +0200 Subject: [PATCH 5/5] refactor: move safe overviews into gateway api --- .../common/NetworkSelector/index.tsx | 2 +- .../__tests__/SafeTokenWidget.test.tsx | 4 +- .../common/SafeTokenWidget/index.tsx | 2 +- .../transactions/SingleTx/index.tsx | 2 +- .../transactions/TxDetails/index.tsx | 2 +- .../flows/ExecuteBatch/ReviewBatch.tsx | 2 +- src/components/tx/DecodedTx/index.tsx | 2 +- src/components/tx/SignOrExecuteForm/index.tsx | 2 +- .../welcome/MyAccounts/AccountItem.tsx | 2 +- .../welcome/MyAccounts/MultiAccountItem.tsx | 2 +- .../speedup/components/SpeedUpModal.tsx | 2 +- .../stake/components/StakePage/index.tsx | 2 +- src/features/swap/index.tsx | 2 +- src/hooks/useDelegates.ts | 2 +- src/hooks/useTxNotifications.ts | 2 +- src/hooks/useTxTracking.ts | 2 +- src/store/__tests__/safeOverviews.test.ts | 72 ++++++++++++- .../{gateway.ts => api/gateway/index.ts} | 6 +- src/store/{ => api/gateway}/safeOverviews.ts | 101 +++++++++--------- src/store/{ => api}/ofac.ts | 2 +- src/store/{ => api}/safePass.ts | 0 src/store/index.ts | 6 +- src/store/slices.ts | 4 +- 23 files changed, 149 insertions(+), 76 deletions(-) rename src/store/{gateway.ts => api/gateway/index.ts} (91%) rename src/store/{ => api/gateway}/safeOverviews.ts (61%) rename src/store/{ => api}/ofac.ts (98%) rename src/store/{ => api}/safePass.ts (100%) diff --git a/src/components/common/NetworkSelector/index.tsx b/src/components/common/NetworkSelector/index.tsx index a37d54feab..82767e044a 100644 --- a/src/components/common/NetworkSelector/index.tsx +++ b/src/components/common/NetworkSelector/index.tsx @@ -39,7 +39,7 @@ import { type ChainInfo } from '@safe-global/safe-gateway-typescript-sdk' import PlusIcon from '@/public/images/common/plus.svg' import useAddressBook from '@/hooks/useAddressBook' import { CreateSafeOnSpecificChain } from '@/features/multichain/components/CreateSafeOnNewChain' -import { useGetSafeOverviewQuery } from '@/store/safeOverviews' +import { useGetSafeOverviewQuery } from '@/store/api/gateway' const ChainIndicatorWithFiatBalance = ({ isSelected, diff --git a/src/components/common/SafeTokenWidget/__tests__/SafeTokenWidget.test.tsx b/src/components/common/SafeTokenWidget/__tests__/SafeTokenWidget.test.tsx index b92a675f2e..4f7ac6bfdc 100644 --- a/src/components/common/SafeTokenWidget/__tests__/SafeTokenWidget.test.tsx +++ b/src/components/common/SafeTokenWidget/__tests__/SafeTokenWidget.test.tsx @@ -5,8 +5,8 @@ import SafeTokenWidget from '..' import { toBeHex } from 'ethers' import { AppRoutes } from '@/config/routes' import useSafeTokenAllocation, { useSafeVotingPower } from '@/hooks/useSafeTokenAllocation' -import * as safePass from '@/store/safePass' -import type { CampaignLeaderboardEntry } from '@/store/safePass' +import * as safePass from '@/store/api/safePass' +import type { CampaignLeaderboardEntry } from '@/store/api/safePass' jest.mock('@/hooks/useChainId') diff --git a/src/components/common/SafeTokenWidget/index.tsx b/src/components/common/SafeTokenWidget/index.tsx index ee8a9869d1..1ca32b9663 100644 --- a/src/components/common/SafeTokenWidget/index.tsx +++ b/src/components/common/SafeTokenWidget/index.tsx @@ -14,7 +14,7 @@ import css from './styles.module.css' import useSafeAddress from '@/hooks/useSafeAddress' import { skipToken } from '@reduxjs/toolkit/query/react' import { useDarkMode } from '@/hooks/useDarkMode' -import { useGetOwnGlobalCampaignRankQuery } from '@/store/safePass' +import { useGetOwnGlobalCampaignRankQuery } from '@/store/api/safePass' import { formatAmount } from '@/utils/formatNumber' const TOKEN_DECIMALS = 18 diff --git a/src/components/transactions/SingleTx/index.tsx b/src/components/transactions/SingleTx/index.tsx index c2bd9e52a6..034630d3e9 100644 --- a/src/components/transactions/SingleTx/index.tsx +++ b/src/components/transactions/SingleTx/index.tsx @@ -13,7 +13,7 @@ import ExpandableTransactionItem, { } from '@/components/transactions/TxListItem/ExpandableTransactionItem' import GroupLabel from '../GroupLabel' import { isMultisigDetailedExecutionInfo } from '@/utils/transaction-guards' -import { useGetTransactionDetailsQuery } from '@/store/gateway' +import { useGetTransactionDetailsQuery } from '@/store/api/gateway' import { skipToken } from '@reduxjs/toolkit/query/react' import { asError } from '@/services/exceptions/utils' diff --git a/src/components/transactions/TxDetails/index.tsx b/src/components/transactions/TxDetails/index.tsx index 1387362d0b..d64a0d225f 100644 --- a/src/components/transactions/TxDetails/index.tsx +++ b/src/components/transactions/TxDetails/index.tsx @@ -33,7 +33,7 @@ import useIsPending from '@/hooks/useIsPending' import { isImitation, isTrustedTx } from '@/utils/transactions' import { useHasFeature } from '@/hooks/useChains' import { FEATURES } from '@/utils/chains' -import { useGetTransactionDetailsQuery } from '@/store/gateway' +import { useGetTransactionDetailsQuery } from '@/store/api/gateway' import { asError } from '@/services/exceptions/utils' import { POLLING_INTERVAL } from '@/config/constants' diff --git a/src/components/tx-flow/flows/ExecuteBatch/ReviewBatch.tsx b/src/components/tx-flow/flows/ExecuteBatch/ReviewBatch.tsx index 6739daad92..9f4c838a3d 100644 --- a/src/components/tx-flow/flows/ExecuteBatch/ReviewBatch.tsx +++ b/src/components/tx-flow/flows/ExecuteBatch/ReviewBatch.tsx @@ -36,7 +36,7 @@ import WalletRejectionError from '@/components/tx/SignOrExecuteForm/WalletReject import useUserNonce from '@/components/tx/AdvancedParams/useUserNonce' import { getLatestSafeVersion } from '@/utils/chains' import { HexEncodedData } from '@/components/transactions/HexEncodedData' -import { useGetMultipleTransactionDetailsQuery } from '@/store/gateway' +import { useGetMultipleTransactionDetailsQuery } from '@/store/api/gateway' import { skipToken } from '@reduxjs/toolkit/query/react' import NetworkWarning from '@/components/new-safe/create/NetworkWarning' diff --git a/src/components/tx/DecodedTx/index.tsx b/src/components/tx/DecodedTx/index.tsx index 44d8c3087a..618893fc8b 100644 --- a/src/components/tx/DecodedTx/index.tsx +++ b/src/components/tx/DecodedTx/index.tsx @@ -13,7 +13,7 @@ import ExpandMoreIcon from '@mui/icons-material/ExpandMore' import DecodedData from '@/components/transactions/TxDetails/TxData/DecodedData' import accordionCss from '@/styles/accordion.module.css' import HelpToolTip from './HelpTooltip' -import { useGetTransactionDetailsQuery } from '@/store/gateway' +import { useGetTransactionDetailsQuery } from '@/store/api/gateway' import { skipToken } from '@reduxjs/toolkit/query/react' import { asError } from '@/services/exceptions/utils' diff --git a/src/components/tx/SignOrExecuteForm/index.tsx b/src/components/tx/SignOrExecuteForm/index.tsx index 1c2a0b4a53..722f156e3f 100644 --- a/src/components/tx/SignOrExecuteForm/index.tsx +++ b/src/components/tx/SignOrExecuteForm/index.tsx @@ -39,7 +39,7 @@ import { MigrateToL2Information } from './MigrateToL2Information' import { extractMigrationL2MasterCopyAddress } from '@/utils/transactions' import type { TransactionDetails } from '@safe-global/safe-gateway-typescript-sdk' -import { useGetTransactionDetailsQuery, useLazyGetTransactionDetailsQuery } from '@/store/gateway' +import { useGetTransactionDetailsQuery, useLazyGetTransactionDetailsQuery } from '@/store/api/gateway' import { skipToken } from '@reduxjs/toolkit/query/react' import NetworkWarning from '@/components/new-safe/create/NetworkWarning' diff --git a/src/components/welcome/MyAccounts/AccountItem.tsx b/src/components/welcome/MyAccounts/AccountItem.tsx index e33e23d627..32fff77b8c 100644 --- a/src/components/welcome/MyAccounts/AccountItem.tsx +++ b/src/components/welcome/MyAccounts/AccountItem.tsx @@ -26,7 +26,7 @@ import FiatValue from '@/components/common/FiatValue' import QueueActions from './QueueActions' import { useGetHref } from './useGetHref' import { extractCounterfactualSafeSetup, isPredictedSafeProps } from '@/features/counterfactual/utils' -import { useGetSafeOverviewQuery } from '@/store/safeOverviews' +import { useGetSafeOverviewQuery } from '@/store/api/gateway' import useWallet from '@/hooks/wallets/useWallet' import { skipToken } from '@reduxjs/toolkit/query' diff --git a/src/components/welcome/MyAccounts/MultiAccountItem.tsx b/src/components/welcome/MyAccounts/MultiAccountItem.tsx index 7a9950916a..a265d7ad5a 100644 --- a/src/components/welcome/MyAccounts/MultiAccountItem.tsx +++ b/src/components/welcome/MyAccounts/MultiAccountItem.tsx @@ -33,7 +33,7 @@ import { AddNetworkButton } from './AddNetworkButton' import { isPredictedSafeProps } from '@/features/counterfactual/utils' import ChainIndicator from '@/components/common/ChainIndicator' import MultiAccountContextMenu from '@/components/sidebar/SafeListContextMenu/MultiAccountContextMenu' -import { useGetMultipleSafeOverviewsQuery } from '@/store/safeOverviews' +import { useGetMultipleSafeOverviewsQuery } from '@/store/api/gateway' import useWallet from '@/hooks/wallets/useWallet' import { selectCurrency } from '@/store/settingsSlice' diff --git a/src/features/speedup/components/SpeedUpModal.tsx b/src/features/speedup/components/SpeedUpModal.tsx index 50a641b1b1..b529480268 100644 --- a/src/features/speedup/components/SpeedUpModal.tsx +++ b/src/features/speedup/components/SpeedUpModal.tsx @@ -27,7 +27,7 @@ import { getTransactionTrackingType } from '@/services/analytics/tx-tracking' import { trackError } from '@/services/exceptions' import ErrorCodes from '@/services/exceptions/ErrorCodes' import CheckWallet from '@/components/common/CheckWallet' -import { useLazyGetTransactionDetailsQuery } from '@/store/gateway' +import { useLazyGetTransactionDetailsQuery } from '@/store/api/gateway' import NetworkWarning from '@/components/new-safe/create/NetworkWarning' type Props = { diff --git a/src/features/stake/components/StakePage/index.tsx b/src/features/stake/components/StakePage/index.tsx index b42b388f44..e6c4b855ec 100644 --- a/src/features/stake/components/StakePage/index.tsx +++ b/src/features/stake/components/StakePage/index.tsx @@ -4,7 +4,7 @@ import WidgetDisclaimer from '@/components/common/WidgetDisclaimer' import useStakeConsent from '@/features/stake/useStakeConsent' import StakingWidget from '../StakingWidget' import { useRouter } from 'next/router' -import { useGetIsSanctionedQuery } from '@/store/ofac' +import { useGetIsSanctionedQuery } from '@/store/api/ofac' import { skipToken } from '@reduxjs/toolkit/query/react' import useWallet from '@/hooks/wallets/useWallet' import useSafeInfo from '@/hooks/useSafeInfo' diff --git a/src/features/swap/index.tsx b/src/features/swap/index.tsx index 9ac20f238d..12735115ba 100644 --- a/src/features/swap/index.tsx +++ b/src/features/swap/index.tsx @@ -37,7 +37,7 @@ import { import { calculateFeePercentageInBps } from '@/features/swap/helpers/fee' import { UiOrderTypeToOrderType } from '@/features/swap/helpers/utils' import { FEATURES } from '@/utils/chains' -import { useGetIsSanctionedQuery } from '@/store/ofac' +import { useGetIsSanctionedQuery } from '@/store/api/ofac' import { skipToken } from '@reduxjs/toolkit/query/react' import { getKeyWithTrueValue } from '@/utils/helpers' diff --git a/src/hooks/useDelegates.ts b/src/hooks/useDelegates.ts index 74c643ce33..c669ed93c0 100644 --- a/src/hooks/useDelegates.ts +++ b/src/hooks/useDelegates.ts @@ -1,6 +1,6 @@ import useSafeInfo from '@/hooks/useSafeInfo' import useWallet from '@/hooks/wallets/useWallet' -import { useGetDelegatesQuery } from '@/store/gateway' +import { useGetDelegatesQuery } from '@/store/api/gateway' import { skipToken } from '@reduxjs/toolkit/query/react' const useDelegates = () => { diff --git a/src/hooks/useTxNotifications.ts b/src/hooks/useTxNotifications.ts index 5311a8e8a6..c778e43d00 100644 --- a/src/hooks/useTxNotifications.ts +++ b/src/hooks/useTxNotifications.ts @@ -14,7 +14,7 @@ import useSafeAddress from './useSafeAddress' import { getExplorerLink } from '@/utils/gateway' import { isWalletRejection } from '@/utils/wallets' import { getTxLink } from '@/utils/tx-link' -import { useLazyGetTransactionDetailsQuery } from '@/store/gateway' +import { useLazyGetTransactionDetailsQuery } from '@/store/api/gateway' const TxNotifications = { [TxEvent.SIGN_FAILED]: 'Failed to sign. Please try again.', diff --git a/src/hooks/useTxTracking.ts b/src/hooks/useTxTracking.ts index 44a1e570dd..836bb81be9 100644 --- a/src/hooks/useTxTracking.ts +++ b/src/hooks/useTxTracking.ts @@ -2,7 +2,7 @@ import { trackEvent, WALLET_EVENTS } from '@/services/analytics' import { TxEvent, txSubscribe } from '@/services/tx/txEvents' import { useEffect } from 'react' import useChainId from './useChainId' -import { useLazyGetTransactionDetailsQuery } from '@/store/gateway' +import { useLazyGetTransactionDetailsQuery } from '@/store/api/gateway' const events = { [TxEvent.SIGNED]: WALLET_EVENTS.OFFCHAIN_SIGNATURE, diff --git a/src/store/__tests__/safeOverviews.test.ts b/src/store/__tests__/safeOverviews.test.ts index 137c41003d..a7858b6ee7 100644 --- a/src/store/__tests__/safeOverviews.test.ts +++ b/src/store/__tests__/safeOverviews.test.ts @@ -1,5 +1,5 @@ import { renderHook, waitFor } from '@/tests/test-utils' -import { useGetMultipleSafeOverviewsQuery, useGetSafeOverviewQuery } from '../safeOverviews' +import { useGetMultipleSafeOverviewsQuery, useGetSafeOverviewQuery } from '../api/gateway' import { faker } from '@faker-js/faker' import { getSafeOverviews } from '@safe-global/safe-gateway-typescript-sdk' @@ -247,5 +247,75 @@ describe('safeOverviews', () => { expect(result.current.isLoading).toBeFalsy() }) }) + + it('Should split big batches into multiple requests', async () => { + // Requests overviews for 15 Safes at once + const request = { + currency: 'usd', + safes: [ + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + { address: faker.finance.ethereumAddress(), chainId: '1', isWatchlist: false }, + ], + } + + const firstBatchOverviews = request.safes.slice(0, 10).map((safe) => ({ + address: { value: safe.address }, + chainId: '1', + awaitingConfirmation: null, + fiatTotal: faker.string.numeric({ length: { min: 1, max: 6 } }), + owners: [{ value: faker.finance.ethereumAddress() }], + threshold: 1, + queued: 0, + })) + + const secondBatchOverviews = request.safes.slice(10).map((safe) => ({ + address: { value: safe.address }, + chainId: '1', + awaitingConfirmation: null, + fiatTotal: faker.string.numeric({ length: { min: 1, max: 6 } }), + owners: [{ value: faker.finance.ethereumAddress() }], + threshold: 1, + queued: 0, + })) + + // Mock two fetch requests for the 2 batches + mockedGetSafeOverviews.mockResolvedValueOnce(firstBatchOverviews).mockResolvedValueOnce(secondBatchOverviews) + + const { result } = renderHook(() => useGetMultipleSafeOverviewsQuery(request)) + + // Request should get queued and remain loading for the queue seconds + expect(result.current.isLoading).toBeTruthy() + + await waitFor(() => { + expect(result.current.isLoading).toBeFalsy() + expect(result.current.error).toBeUndefined() + expect(result.current.data).toEqual([...firstBatchOverviews, ...secondBatchOverviews]) + }) + + // Expect that the correct requests were sent + expect(mockedGetSafeOverviews).toHaveBeenCalledTimes(2) + expect(mockedGetSafeOverviews).toHaveBeenCalledWith( + request.safes.slice(0, 10).map((safe) => `1:${safe.address}`), + { currency: 'usd', exclude_spam: true, trusted: true }, + ) + + expect(mockedGetSafeOverviews).toHaveBeenCalledWith( + request.safes.slice(10).map((safe) => `1:${safe.address}`), + { currency: 'usd', exclude_spam: true, trusted: true }, + ) + }) }) }) diff --git a/src/store/gateway.ts b/src/store/api/gateway/index.ts similarity index 91% rename from src/store/gateway.ts rename to src/store/api/gateway/index.ts index 3b027245b9..295279353a 100644 --- a/src/store/gateway.ts +++ b/src/store/api/gateway/index.ts @@ -5,8 +5,9 @@ import type { BaseQueryFn } from '@reduxjs/toolkit/dist/query/baseQueryTypes' import type { FetchBaseQueryError } from '@reduxjs/toolkit/dist/query/react' import { getDelegates } from '@safe-global/safe-gateway-typescript-sdk' import type { DelegateResponse } from '@safe-global/safe-gateway-typescript-sdk/dist/types/delegates' +import { safeOverviewEndpoints } from './safeOverviews' -const noopBaseQuery: BaseQueryFn< +export const noopBaseQuery: BaseQueryFn< unknown, // QueryArg type unknown, // ResultType FetchBaseQueryError, // ErrorType @@ -48,6 +49,7 @@ export const gatewayApi = createApi({ } }, }), + ...safeOverviewEndpoints(builder), }), }) @@ -56,4 +58,6 @@ export const { useGetMultipleTransactionDetailsQuery, useLazyGetTransactionDetailsQuery, useGetDelegatesQuery, + useGetSafeOverviewQuery, + useGetMultipleSafeOverviewsQuery, } = gatewayApi diff --git a/src/store/safeOverviews.ts b/src/store/api/gateway/safeOverviews.ts similarity index 61% rename from src/store/safeOverviews.ts rename to src/store/api/gateway/safeOverviews.ts index e3632d8cb0..8caa1ef00a 100644 --- a/src/store/safeOverviews.ts +++ b/src/store/api/gateway/safeOverviews.ts @@ -1,13 +1,11 @@ -import { createApi } from '@reduxjs/toolkit/query/react' +import { type BaseQueryFn, type FetchBaseQueryError, type EndpointBuilder } from '@reduxjs/toolkit/query/react' import { type SafeOverview, getSafeOverviews } from '@safe-global/safe-gateway-typescript-sdk' import { sameAddress } from '@/utils/addresses' -import type { RootState } from '.' -import { selectCurrency } from './settingsSlice' +import type { RootState } from '../..' +import { selectCurrency } from '../../settingsSlice' import { type SafeItem } from '@/components/welcome/MyAccounts/useAllSafes' -const noopBaseQuery = async () => ({ data: null }) - type SafeOverviewQueueItem = { safeAddress: string walletAddress?: string @@ -114,50 +112,53 @@ type MultiOverviewQueryParams = { safes: SafeItem[] } -export const safeOverviewApi = createApi({ - reducerPath: 'safeOverviewApi', - baseQuery: noopBaseQuery, - endpoints: (builder) => ({ - getSafeOverview: builder.query< - SafeOverview | undefined, - { safeAddress: string; walletAddress?: string; chainId: string } - >({ - async queryFn({ safeAddress, walletAddress, chainId }, { getState }) { - const state = getState() - const currency = selectCurrency(state as RootState) - - try { - const safeOverview = await batchedFetcher.getOverview({ chainId, currency, walletAddress, safeAddress }) - return { data: safeOverview } - } catch (error) { - return { error: { status: 'CUSTOM_ERROR', data: (error as Error).message } } - } - }, - }), - getMultipleSafeOverviews: builder.query({ - async queryFn(params) { - const { safes, walletAddress, currency } = params - - try { - const promisedSafeOverviews = safes.map((safe) => - batchedFetcher.getOverview({ - chainId: safe.chainId, - safeAddress: safe.address, - currency, - walletAddress, - }), - ) - const safeOverviews = await Promise.all(promisedSafeOverviews) - return { data: safeOverviews.filter(Boolean) as SafeOverview[] } - } catch (error) { - return { error: { status: 'CUSTOM_ERROR', data: (error as Error).message } } - } - }, - }), +export const safeOverviewEndpoints = ( + builder: EndpointBuilder< + BaseQueryFn< + unknown, // QueryArg type + unknown, // ResultType + FetchBaseQueryError, // ErrorType + {}, // DefinitionExtraOptions + {} // Meta + >, + never, + 'gatewayApi' + >, +) => ({ + getSafeOverview: builder.query< + SafeOverview | undefined, + { safeAddress: string; walletAddress?: string; chainId: string } + >({ + async queryFn({ safeAddress, walletAddress, chainId }, { getState }) { + const state = getState() + const currency = selectCurrency(state as RootState) + + try { + const safeOverview = await batchedFetcher.getOverview({ chainId, currency, walletAddress, safeAddress }) + return { data: safeOverview } + } catch (error) { + return { error: { status: 'CUSTOM_ERROR', error: (error as Error).message } } + } + }, + }), + getMultipleSafeOverviews: builder.query({ + async queryFn(params) { + const { safes, walletAddress, currency } = params + + try { + const promisedSafeOverviews = safes.map((safe) => + batchedFetcher.getOverview({ + chainId: safe.chainId, + safeAddress: safe.address, + currency, + walletAddress, + }), + ) + const safeOverviews = await Promise.all(promisedSafeOverviews) + return { data: safeOverviews.filter(Boolean) as SafeOverview[] } + } catch (error) { + return { error: { status: 'CUSTOM_ERROR', error: (error as Error).message } } + } + }, }), }) - -// Export hooks for usage in functional components, which are -// auto-generated based on the defined endpoints -export const { useGetSafeOverviewQuery, useLazyGetSafeOverviewQuery, useGetMultipleSafeOverviewsQuery } = - safeOverviewApi diff --git a/src/store/ofac.ts b/src/store/api/ofac.ts similarity index 98% rename from src/store/ofac.ts rename to src/store/api/ofac.ts index 0982ce5c9d..3e4be9d4b2 100644 --- a/src/store/ofac.ts +++ b/src/store/api/ofac.ts @@ -2,7 +2,7 @@ import { createApi } from '@reduxjs/toolkit/query/react' import { selectChainById } from '@/store/chainsSlice' import { Contract } from 'ethers' import { createWeb3ReadOnly } from '@/hooks/wallets/web3' -import type { RootState } from '.' +import type { RootState } from '..' import { CHAINALYSIS_OFAC_CONTRACT } from '@/config/constants' import chains from '@/config/chains' diff --git a/src/store/safePass.ts b/src/store/api/safePass.ts similarity index 100% rename from src/store/safePass.ts rename to src/store/api/safePass.ts diff --git a/src/store/index.ts b/src/store/index.ts index f3916f951c..1dcd5903fb 100644 --- a/src/store/index.ts +++ b/src/store/index.ts @@ -24,8 +24,8 @@ import { } from './slices' import * as slices from './slices' import * as hydrate from './useHydrateStore' -import { ofacApi } from '@/store/ofac' -import { safePassApi } from './safePass' +import { ofacApi } from '@/store/api/ofac' +import { safePassApi } from './api/safePass' import { metadata } from '@/markdown/terms/terms.md' const rootReducer = combineReducers({ @@ -53,7 +53,6 @@ const rootReducer = combineReducers({ [ofacApi.reducerPath]: ofacApi.reducer, [safePassApi.reducerPath]: safePassApi.reducer, [slices.gatewayApi.reducerPath]: slices.gatewayApi.reducer, - [slices.safeOverviewApi.reducerPath]: slices.safeOverviewApi.reducer, }) const persistedSlices: (keyof Partial)[] = [ @@ -84,7 +83,6 @@ const middleware: Middleware<{}, RootState>[] = [ ofacApi.middleware, safePassApi.middleware, slices.gatewayApi.middleware, - slices.safeOverviewApi.middleware, ] const listeners = [safeMessagesListener, txHistoryListener, txQueueListener, swapOrderListener, swapOrderStatusListener] diff --git a/src/store/slices.ts b/src/store/slices.ts index d105316365..a36e3c1806 100644 --- a/src/store/slices.ts +++ b/src/store/slices.ts @@ -19,5 +19,5 @@ export * from './batchSlice' export * from '@/features/counterfactual/store/undeployedSafesSlice' export * from '@/features/swap/store/swapParamsSlice' export * from './swapOrderSlice' -export * from './gateway' -export * from './safeOverviews' +export * from './api/gateway' +export * from './api/gateway/safeOverviews'