refactor: simplify refresh balance triggers (#6232)

### Description This PR consolidates the way we are refreshing the token balances. I think we let this evolve organically for some time and it has become buggy and inconsistent. Currently, before this change: - we have a bunch of watcher functions that seem overcomplicated (dispatching intermediate actions to trigger other sagas) - we need to remember to dispatch the `fetchTokenBalances` action after every flow that initiates a transaction. i think it is probably unavoidable to need to remember to trigger a balance fetch, but currently we dispatch a success action for the flow and a separate action for the balances (the dreaded double dispatch) - we forget to refresh balances in a bunch of places (e.g. swap flow) which means users can try (and fail) to execute other transactions with outdated balances. This change: - remove the `fetchTokenBalances` action entirely and instead, trigger the balance refresh sagas on specific actions. i've tried to document them all. - there is a loading state that seems unnecessary in the `home` reducer, most flows were never allowing this loading state to be set. only the send flow was, i think that's a bug. balance refreshes should be a background task and not affect whether we show or hide the balance.... - define balance refresh as a combination of refreshing token balances, exchange rates (the total balance is wrong if the exchange rate is outdated), positions so trigger all those sagas ### Test plan I should see a network request to refresh token balance, exchange rate, positions, shortcuts when I have: - completed onboarding - received a payment (transaction feed updated) - executed a transaction via any app flow ### Related issues - Related to RET-1241 ### Backwards compatibility yes ### Network scalability If a new NetworkId and/or Network are added in the future, the changes in this PR will: - [ ] Continue to work without code changes, OR trigger a compilation error (guaranteeing we find it when a new network is added)
valora-inc · Nov 15, 2024 · b393e94 · b393e94
1 parent ee3cc3f
commit b393e94
Show file tree

Hide file tree

Showing 26 changed files with 85 additions and 228 deletions.
diff --git a/src/account/saga.ts b/src/account/saga.ts
@@ -18,7 +18,6 @@ import { inviterAddressSelector } from 'src/app/selectors'
 import { clearStoredMnemonic } from 'src/backup/utils'
 import { FIREBASE_ENABLED } from 'src/config'
 import { firebaseSignOut } from 'src/firebase/firebase'
-import { refreshAllBalances } from 'src/home/actions'
 import { currentLanguageSelector } from 'src/i18n/selectors'
 import { navigate, navigateClearingStack, navigateHome } from 'src/navigator/NavigationService'
 import { Screens } from 'src/navigator/Screens'
@@ -79,7 +78,6 @@ export function* initializeAccountSaga() {
     AppAnalytics.track(OnboardingEvents.initialize_account_start)
     yield* call(getOrCreateAccount)
     yield* call(generateSignedMessage)
-    yield* put(refreshAllBalances())
 
     const choseToRestoreAccount = yield* select(choseToRestoreAccountSelector)
     if (choseToRestoreAccount) {

diff --git a/src/components/IconWithNetworkBadge.test.tsx b/src/components/IconWithNetworkBadge.test.tsx
@@ -20,7 +20,6 @@ describe('IconWithNetworkBadge', () => {
     const store = createMockStore({
       tokens: {
         error: false,
-        loading: false,
         tokenBalances: MOCK_TOKEN_BALANCES,
       },
     })

diff --git a/src/components/TokenBalance.tsx b/src/components/TokenBalance.tsx
@@ -46,7 +46,7 @@ import {
   useTokensWithUsdValue,
   useTotalTokenBalance,
 } from 'src/tokens/hooks'
-import { tokenFetchErrorSelector, tokenFetchLoadingSelector } from 'src/tokens/selectors'
+import { tokenFetchErrorSelector } from 'src/tokens/selectors'
 import { getSupportedNetworkIdsForTokenBalances } from 'src/tokens/utils'
 
 function TokenBalance({
@@ -62,7 +62,6 @@ function TokenBalance({
   const tokensWithUsdValue = useTokensWithUsdValue(supportedNetworkIds)
   const localCurrencySymbol = useSelector(getLocalCurrencySymbol)
   const totalTokenBalanceLocal = useTotalTokenBalance()
-  const tokenFetchLoading = useSelector(tokenFetchLoadingSelector)
   const tokenFetchError = useSelector(tokenFetchErrorSelector)
   const tokensAreStale = useTokenPricesAreStale(supportedNetworkIds)
   // TODO(ACT-1095): Update these to filter out unsupported networks once positions support non-Celo chains
@@ -90,7 +89,7 @@ function TokenBalance({
     )
   }
 
-  if (tokenFetchError || tokenFetchLoading || tokensAreStale) {
+  if (tokenFetchError || tokensAreStale) {
     // Show '-' if we haven't fetched the tokens yet or prices are stale
     return <TotalTokenBalance balanceDisplay={'-'} />
   } else if (

diff --git a/src/components/TokenIcon.test.tsx b/src/components/TokenIcon.test.tsx
@@ -24,7 +24,6 @@ const NO_IMAGE_TOKEN = {
 const store = createMockStore({
   tokens: {
     error: false,
-    loading: false,
     tokenBalances: mockTokenBalances,
   },
 })

diff --git a/src/earn/saga.test.ts b/src/earn/saga.test.ts
@@ -19,7 +19,6 @@ import { isGasSubsidizedForNetwork } from 'src/earn/utils'
 import { navigateHome } from 'src/navigator/NavigationService'
 import { CANCELLED_PIN_INPUT } from 'src/pincode/authentication'
 import { EarnPosition } from 'src/positions/types'
-import { fetchTokenBalances } from 'src/tokens/slice'
 import { Network, NetworkId, TokenTransactionTypeV2 } from 'src/transactions/types'
 import { publicClient } from 'src/viem'
 import { SerializableTransactionRequest } from 'src/viem/preparedTransactionSerialization'
@@ -285,7 +284,6 @@ describe('depositSubmitSaga', () => {
           transactionHash: '0x2',
         })
       )
-      .put(fetchTokenBalances({ showLoading: false }))
       .call.like({ fn: sendPreparedTransactions })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x1' })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x2' })
@@ -331,7 +329,6 @@ describe('depositSubmitSaga', () => {
           transactionHash: '0x2',
         })
       )
-      .put(fetchTokenBalances({ showLoading: false }))
       .call.like({ fn: sendPreparedTransactions })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x2' })
       .run()
@@ -382,7 +379,6 @@ describe('depositSubmitSaga', () => {
           transactionHash: '0x2',
         })
       )
-      .put(fetchTokenBalances({ showLoading: false }))
       .call.like({ fn: sendPreparedTransactions })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x1' })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x2' })
@@ -438,7 +434,6 @@ describe('depositSubmitSaga', () => {
           transactionHash: '0x2',
         })
       )
-      .put(fetchTokenBalances({ showLoading: false }))
       .call.like({ fn: sendPreparedTransactions })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x2' })
       .run()
@@ -491,7 +486,6 @@ describe('depositSubmitSaga', () => {
           transactionHash: '0x3',
         })
       )
-      .put(fetchTokenBalances({ showLoading: false }))
       .call.like({ fn: sendPreparedTransactions })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x1' })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x2' })
@@ -533,7 +527,6 @@ describe('depositSubmitSaga', () => {
           transactionHash: '0x2',
         })
       )
-      .put(fetchTokenBalances({ showLoading: false }))
       .call.like({ fn: sendPreparedTransactions })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x1' })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x2' })
@@ -567,7 +560,6 @@ describe('depositSubmitSaga', () => {
         ...sagaProviders,
       ])
       .put(depositCancel())
-      .not.put.actionType(fetchTokenBalances.type)
       .call.like({ fn: sendPreparedTransactions })
       .not.call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'])
       .run()
@@ -602,7 +594,6 @@ describe('depositSubmitSaga', () => {
         ...sagaProviders,
       ])
       .put(depositError())
-      .not.put.actionType(fetchTokenBalances.type)
       .call.like({ fn: sendPreparedTransactions })
       .not.call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'])
       .run()
@@ -643,7 +634,6 @@ describe('depositSubmitSaga', () => {
         ...sagaProviders,
       ])
       .put(depositError())
-      .not.put.actionType(fetchTokenBalances.type)
       .call.like({ fn: sendPreparedTransactions })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x1' })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x2' })
@@ -797,7 +787,6 @@ describe('withdrawSubmitSaga', () => {
       .withState(createMockStore({ tokens: { tokenBalances: mockTokenBalances } }).getState())
       .provide(sagaProviders)
       .put(withdrawSuccess())
-      .put(fetchTokenBalances({ showLoading: false }))
       .call.like({ fn: sendPreparedTransactions })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x1' })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x2' })
@@ -833,7 +822,6 @@ describe('withdrawSubmitSaga', () => {
       .withState(createMockStore({ tokens: { tokenBalances: mockTokenBalances } }).getState())
       .provide(sagaProviders)
       .put(withdrawSuccess())
-      .put(fetchTokenBalances({ showLoading: false }))
       .call.like({ fn: sendPreparedTransactions })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x1' })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x2' })
@@ -873,7 +861,6 @@ describe('withdrawSubmitSaga', () => {
       .withState(createMockStore({ tokens: { tokenBalances: mockTokenBalances } }).getState())
       .provide(sagaProviders)
       .put(withdrawSuccess())
-      .put(fetchTokenBalances({ showLoading: false }))
       .call.like({ fn: sendPreparedTransactions })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x1' })
       .run()
@@ -913,7 +900,6 @@ describe('withdrawSubmitSaga', () => {
       .withState(createMockStore({ tokens: { tokenBalances: mockTokenBalances } }).getState())
       .provide(sagaProviders)
       .put(withdrawSuccess())
-      .put(fetchTokenBalances({ showLoading: false }))
       .call.like({ fn: sendPreparedTransactions })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x1' })
       .run()
@@ -953,7 +939,6 @@ describe('withdrawSubmitSaga', () => {
         ...sagaProviders,
       ])
       .put(withdrawCancel())
-      .not.put.actionType(fetchTokenBalances.type)
       .call.like({ fn: sendPreparedTransactions })
       .not.call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'])
       .run()
@@ -985,7 +970,6 @@ describe('withdrawSubmitSaga', () => {
         ...sagaProviders,
       ])
       .put(withdrawError())
-      .not.put.actionType(fetchTokenBalances.type)
       .call.like({ fn: sendPreparedTransactions })
       .not.call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'])
       .run()
@@ -1026,7 +1010,6 @@ describe('withdrawSubmitSaga', () => {
           ...sagaProviders,
         ])
         .put(withdrawError())
-        .not.put.actionType(fetchTokenBalances.type)
         .call.like({ fn: sendPreparedTransactions })
         .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x1' })
         .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x2' })
@@ -1059,7 +1042,6 @@ describe('withdrawSubmitSaga', () => {
       .withState(createMockStore({ tokens: { tokenBalances: mockTokenBalances } }).getState())
       .provide(sagaProviders)
       .put(withdrawSuccess())
-      .put(fetchTokenBalances({ showLoading: false }))
       .call.like({ fn: sendPreparedTransactions })
       .call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'], { hash: '0x1' })
       .run()
@@ -1099,7 +1081,6 @@ describe('withdrawSubmitSaga', () => {
         ...sagaProviders,
       ])
       .put(withdrawCancel())
-      .not.put.actionType(fetchTokenBalances.type)
       .call.like({ fn: sendPreparedTransactions })
       .not.call([publicClient[Network.Arbitrum], 'waitForTransactionReceipt'])
       .run()

diff --git a/src/earn/saga.ts b/src/earn/saga.ts
@@ -20,7 +20,7 @@ import { CANCELLED_PIN_INPUT } from 'src/pincode/authentication'
 import { vibrateError } from 'src/styles/hapticFeedback'
 import { getTokenInfo } from 'src/tokens/saga'
 import { tokensByIdSelector } from 'src/tokens/selectors'
-import { TokenBalances, fetchTokenBalances } from 'src/tokens/slice'
+import { TokenBalances } from 'src/tokens/slice'
 import { BaseStandbyTransaction } from 'src/transactions/slice'
 import {
   NetworkId,
@@ -276,7 +276,6 @@ export function* depositSubmitSaga(action: PayloadAction<DepositInfo>) {
         transactionHash: txHashes[txHashes.length - 1],
       })
     )
-    yield* put(fetchTokenBalances({ showLoading: false }))
   } catch (err) {
     if (err === CANCELLED_PIN_INPUT) {
       Logger.info(`${TAG}/depositSubmitSaga`, 'Transaction cancelled by user')
@@ -426,7 +425,6 @@ export function* withdrawSubmitSaga(action: PayloadAction<WithdrawInfo>) {
     })
 
     yield* put(withdrawSuccess())
-    yield* put(fetchTokenBalances({ showLoading: false }))
     AppAnalytics.track(EarnEvents.earn_withdraw_submit_success, commonAnalyticsProps)
   } catch (err) {
     if (err === CANCELLED_PIN_INPUT) {

diff --git a/src/fiatExchanges/saga.ts b/src/fiatExchanges/saga.ts
@@ -18,7 +18,11 @@ import { AddressRecipient, RecipientType, getDisplayName } from 'src/recipients/
 import { Actions as SendActions } from 'src/send/actions'
 import { TransactionDataInput } from 'src/send/types'
 import { CurrencyTokens, tokensByCurrencySelector } from 'src/tokens/selectors'
-import { UpdateTransactionsPayload, updateTransactions } from 'src/transactions/slice'
+import {
+  UpdateTransactionsPayload,
+  updateFeedFirstPage,
+  updateTransactions,
+} from 'src/transactions/slice'
 import { Network, TokenTransactionTypeV2 } from 'src/transactions/types'
 import Logger from 'src/utils/Logger'
 import { resolveCurrency } from 'src/utils/currencies'
@@ -172,7 +176,10 @@ export function* watchBidaliPaymentRequests() {
 }
 
 function* watchNewFeedTransactions() {
-  yield* takeEvery(updateTransactions.type, safely(tagTxsWithProviderInfo))
+  yield* takeEvery(
+    [updateTransactions.type, updateFeedFirstPage.type],
+    safely(tagTxsWithProviderInfo)
+  )
 }
 
 export function* fiatExchangesSaga() {

diff --git a/src/home/saga.test.ts b/src/home/saga.test.ts
@@ -1,31 +1,12 @@
 import { expectSaga } from 'redux-saga-test-plan'
 import { call, put } from 'redux-saga/effects'
-import { refreshAllBalances, setLoading } from 'src/home/actions'
-import { refreshBalances, watchRefreshBalances, withLoading } from 'src/home/saga'
-import { getConnectedAccount } from 'src/web3/saga'
+import { setLoading } from 'src/home/actions'
+import { withLoading } from 'src/home/saga'
 
 beforeAll(() => {
   jest.useRealTimers()
 })
 
-describe('refreshBalances', () => {
-  test('ask for balance when geth and account are ready', () =>
-    expectSaga(refreshBalances)
-      .provide([[call(getConnectedAccount), true]])
-      .run())
-})
-
-describe('watchRefreshBalances', () => {
-  test('reacts on REFRESH_BALANCES', async () => {
-    await expectSaga(watchRefreshBalances)
-      .put(setLoading(true))
-      .put(setLoading(false))
-      .provide([[call(getConnectedAccount), true]])
-      .dispatch(refreshAllBalances())
-      .run()
-  })
-})
-
 describe('withLoading Saga', () => {
   test('sets Loading on/off while calling fn', async () => {
     const fn = () => true

diff --git a/src/home/saga.ts b/src/home/saga.ts
@@ -1,3 +1,6 @@
+import { Actions as AccountActions } from 'src/account/actions'
+import { depositSuccess, withdrawSuccess } from 'src/earn/slice'
+import { createFiatConnectTransferCompleted } from 'src/fiatconnect/slice'
 import { notificationsChannel } from 'src/firebase/firebase'
 import {
   Actions,
@@ -7,15 +10,18 @@ import {
 } from 'src/home/actions'
 import { CleverTapInboxMessage, cleverTapInboxMessagesChannel } from 'src/home/cleverTapInbox'
 import { IdToNotification } from 'src/home/reducers'
-import { fetchCurrentRate } from 'src/localCurrency/actions'
+import { depositTransactionSucceeded } from 'src/jumpstart/slice'
+import { fetchLocalCurrencyRateSaga } from 'src/localCurrency/saga'
+import { fetchPositionsSaga, fetchShortcutsSaga } from 'src/positions/saga'
 import { executeShortcutSuccess } from 'src/positions/slice'
 import { withTimeout } from 'src/redux/sagas-helpers'
-import { fetchTokenBalances } from 'src/tokens/slice'
-import { updateTransactions } from 'src/transactions/slice'
+import { Actions as SendActions } from 'src/send/actions'
+import { swapSuccess } from 'src/swap/slice'
+import { fetchTokenBalancesSaga } from 'src/tokens/saga'
+import { updateFeedFirstPage, updateTransactions } from 'src/transactions/slice'
 import Logger from 'src/utils/Logger'
 import { safely } from 'src/utils/safely'
-import { getConnectedAccount } from 'src/web3/saga'
-import { call, cancelled, put, spawn, take, takeLeading } from 'typed-redux-saga'
+import { all, call, cancelled, put, spawn, take, takeLeading } from 'typed-redux-saga'
 
 const REFRESH_TIMEOUT = 15000
 const TAG = 'home/saga'
@@ -32,20 +38,34 @@ export function withLoading<Fn extends (...args: any[]) => any>(fn: Fn, ...args:
   }
 }
 
-export function* refreshBalances() {
-  Logger.debug(TAG, 'Fetching all balances')
-  yield* call(getConnectedAccount)
-  yield* put(fetchTokenBalances({ showLoading: false }))
-  yield* put(fetchCurrentRate())
+export function* refreshAllBalances() {
+  yield* all([
+    call(fetchTokenBalancesSaga),
+    call(fetchLocalCurrencyRateSaga),
+    call(fetchPositionsSaga),
+    call(fetchShortcutsSaga),
+  ])
 }
 
 export function* watchRefreshBalances() {
-  yield* call(refreshBalances)
   yield* takeLeading(
-    [Actions.REFRESH_BALANCES, executeShortcutSuccess.type],
-    safely(withLoading(withTimeout(REFRESH_TIMEOUT, refreshBalances)))
+    [
+      updateTransactions.type, // emitted on recieve new transactions from graphql endpoint
+      updateFeedFirstPage.type, // emitted on recieve new transactions from zerion endpoint
+      Actions.REFRESH_BALANCES, // emitted on manual pull to refresh and app foregrounded
+      AccountActions.INITIALIZE_ACCOUNT_SUCCESS, // emitted after onboarding
+      // the below actions are emitted after a successful transaction that is
+      // initiated from the wallet.
+      executeShortcutSuccess.type,
+      depositSuccess.type,
+      withdrawSuccess.type,
+      SendActions.SEND_PAYMENT_SUCCESS,
+      createFiatConnectTransferCompleted.type,
+      depositTransactionSucceeded.type,
+      swapSuccess.type,
+    ],
+    safely(withLoading(withTimeout(REFRESH_TIMEOUT, refreshAllBalances)))
   )
-  yield* takeLeading(updateTransactions.type, safely(withTimeout(REFRESH_TIMEOUT, refreshBalances)))
 }
 
 function* fetchNotifications() {

diff --git a/src/import/saga.test.ts b/src/import/saga.test.ts
@@ -8,7 +8,6 @@ import { showError } from 'src/alert/actions'
 import { ErrorMessages } from 'src/app/ErrorMessages'
 import { phoneNumberVerifiedSelector } from 'src/app/selectors'
 import { storeMnemonic } from 'src/backup/utils'
-import { refreshAllBalances } from 'src/home/actions'
 import { currentLanguageSelector } from 'src/i18n/selectors'
 import {
   importBackupPhrase,
@@ -73,7 +72,6 @@ describe('Import wallet saga', () => {
         ],
       ])
       .put(setBackupCompleted())
-      .put(refreshAllBalances())
       .put(importBackupPhraseSuccess())
       .run()
   }
@@ -129,7 +127,6 @@ describe('Import wallet saga', () => {
           [select(phoneNumberVerifiedSelector), false],
         ])
         .put(setBackupCompleted())
-        .put(refreshAllBalances())
         .put(importBackupPhraseSuccess())
         .run()
     }