Skip to content

Commit

Permalink
fix: reset tradeInput AccountIds on walletId change (#7916)
Browse files Browse the repository at this point in the history
* fix: reset tradeInput AccountIds on walletId change

* fix: move reset to `<AppContext />`

* feat: debugger-friendly

* feat: temp remove effect

* Revert "feat: temp remove effect"

This reverts commit f13b90a.

* feat: leverage prevWalletId

* chore: trigger CI
  • Loading branch information
gomesalexandre authored Oct 15, 2024
1 parent b141871 commit ddd7c54
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 54 deletions.
10 changes: 6 additions & 4 deletions src/components/MultiHopTrade/hooks/useAccountIds.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,16 @@ export const useAccountIds = (): {
// Setters - the selectors above initially select a *default* value, but eventually onAccountIdChange may fire if the user changes the account

const setSellAssetAccountId = useCallback(
(accountId: AccountId | undefined) =>
dispatch(tradeInput.actions.setSellAssetAccountNumber(accountId)),
(accountId: AccountId | undefined) => {
dispatch(tradeInput.actions.setSellAssetAccountId(accountId))
},
[dispatch],
)

const setBuyAssetAccountId = useCallback(
(accountId: AccountId | undefined) =>
dispatch(tradeInput.actions.setBuyAssetAccountNumber(accountId)),
(accountId: AccountId | undefined) => {
dispatch(tradeInput.actions.setBuyAssetAccountId(accountId))
},
[dispatch],
)

Expand Down
22 changes: 17 additions & 5 deletions src/context/AppProvider/AppContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { DEFAULT_HISTORY_TIMEFRAME } from 'constants/Config'
import { LanguageTypeEnum } from 'constants/LanguageTypeEnum'
import React, { useEffect } from 'react'
import { useTranslate } from 'react-polyglot'
import { useSelector } from 'react-redux'
import { useNfts } from 'components/Nfts/hooks/useNfts'
import { usePlugins } from 'context/PluginProvider/PluginProvider'
import { useIsSnapInstalled } from 'hooks/useIsSnapInstalled/useIsSnapInstalled'
Expand Down Expand Up @@ -39,7 +38,9 @@ import {
selectPortfolioLoadingStatus,
selectSelectedCurrency,
selectSelectedLocale,
selectWalletId,
} from 'state/slices/selectors'
import { tradeInput } from 'state/slices/tradeInputSlice/tradeInputSlice'
import { txHistoryApi } from 'state/slices/txHistorySlice/txHistorySlice'
import { useAppDispatch, useAppSelector } from 'state/store'

Expand All @@ -59,10 +60,12 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => {
const dispatch = useAppDispatch()
const { supportedChains } = usePlugins()
const { wallet, isConnected } = useWallet().state
const assetIds = useSelector(selectAssetIds)
const requestedAccountIds = useSelector(selectEnabledWalletAccountIds)
const portfolioLoadingStatus = useSelector(selectPortfolioLoadingStatus)
const portfolioAssetIds = useSelector(selectPortfolioAssetIds)
const assetIds = useAppSelector(selectAssetIds)
const requestedAccountIds = useAppSelector(selectEnabledWalletAccountIds)
const portfolioLoadingStatus = useAppSelector(selectPortfolioLoadingStatus)
const portfolioAssetIds = useAppSelector(selectPortfolioAssetIds)
const walletId = useAppSelector(selectWalletId)
const prevWalletId = usePrevious(walletId)
const routeAssetId = useRouteAssetId()
const { isSnapInstalled } = useIsSnapInstalled()
const previousIsSnapInstalled = usePrevious(isSnapInstalled)
Expand Down Expand Up @@ -317,6 +320,15 @@ export const AppProvider = ({ children }: { children: React.ReactNode }) => {
)
}, [dispatch, isConnected, portfolioLoadingStatus])

// Resets the sell and buy asset AccountIDs on wallet change to that we don't get stale trade input account selections while we're loading the new wallet
useEffect(() => {
if (!prevWalletId) return
if (walletId === prevWalletId) return

dispatch(tradeInput.actions.setSellAssetAccountId(undefined))
dispatch(tradeInput.actions.setBuyAssetAccountId(undefined))
}, [dispatch, prevWalletId, walletId])

const marketDataPollingInterval = 60 * 15 * 1000 // refetch data every 15 minutes
useQueries({
queries: portfolioAssetIds.map(assetId => ({
Expand Down
70 changes: 27 additions & 43 deletions src/state/slices/portfolioSlice/portfolioSlice.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createSlice, prepareAutoBatched } from '@reduxjs/toolkit'
import { createSlice } from '@reduxjs/toolkit'
import { createApi } from '@reduxjs/toolkit/query/react'
import type { AccountId, ChainId } from '@shapeshiftoss/caip'
import { ASSET_NAMESPACE, bscChainId, fromAccountId, isNft, toAssetId } from '@shapeshiftoss/caip'
Expand Down Expand Up @@ -86,52 +86,36 @@ export const portfolio = createSlice({

Object.assign(state.connectedWallet, { supportedChainIds: payload })
},
upsertAccountMetadata: {
reducer: (
draftState,
{
payload,
}: { payload: { accountMetadataByAccountId: AccountMetadataById; walletId: string } },
) => {
// WARNING: don't use the current state.connectedWallet.id here because it's updated async
// to this and results in account data corruption
const { accountMetadataByAccountId, walletId } = payload
draftState.accountMetadata.byId = merge(
draftState.accountMetadata.byId,
accountMetadataByAccountId,
)
draftState.accountMetadata.ids = Object.keys(draftState.accountMetadata.byId)

if (!draftState.connectedWallet) return // realistically, at this point, we should have a wallet set
const existingWalletAccountIds = draftState.wallet.byId[walletId] ?? []
const newWalletAccountIds = Object.keys(accountMetadataByAccountId)
// keep an index of what account ids belong to this wallet
draftState.wallet.byId[walletId] = uniq(
existingWalletAccountIds.concat(newWalletAccountIds),
)
},
upsertAccountMetadata: (
draftState,
{
payload,
}: { payload: { accountMetadataByAccountId: AccountMetadataById; walletId: string } },
) => {
// WARNING: don't use the current state.connectedWallet.id here because it's updated async
// to this and results in account data corruption
const { accountMetadataByAccountId, walletId } = payload
draftState.accountMetadata.byId = merge(
draftState.accountMetadata.byId,
accountMetadataByAccountId,
)
draftState.accountMetadata.ids = Object.keys(draftState.accountMetadata.byId)

// Use the `prepareAutoBatched` utility to automatically
// add the `action.meta[SHOULD_AUTOBATCH]` field the enhancer needs
prepare: prepareAutoBatched<{
accountMetadataByAccountId: AccountMetadataById
walletId: string
}>(),
if (!draftState.connectedWallet) return // realistically, at this point, we should have a wallet set
const existingWalletAccountIds = draftState.wallet.byId[walletId] ?? []
const newWalletAccountIds = Object.keys(accountMetadataByAccountId)
// keep an index of what account ids belong to this wallet
draftState.wallet.byId[walletId] = uniq(existingWalletAccountIds.concat(newWalletAccountIds))
},
clearWalletMetadata: {
reducer: (draftState, { payload }: { payload: WalletId }) => {
const walletId = payload
// Clear AccountIds that were previously associated with that wallet
draftState.wallet.byId[walletId] = []
draftState.wallet.ids = draftState.wallet.ids.filter(id => id !== walletId)

// TODO(gomes): do we also want to clear draftState.accountMetadata entries themselves?
// Theoretically, not doing so would make reloading these easier?
},
clearWalletMetadata: (draftState, { payload }: { payload: WalletId }) => {
const walletId = payload
// Clear AccountIds that were previously associated with that wallet
draftState.wallet.byId[walletId] = []
draftState.wallet.ids = draftState.wallet.ids.filter(id => id !== walletId)

// Use the `prepareAutoBatched` utility to automatically
// add the `action.meta[SHOULD_AUTOBATCH]` field the enhancer needs
prepare: prepareAutoBatched<WalletId>(),
// TODO(gomes): do we also want to clear draftState.accountMetadata entries themselves?
// Theoretically, not doing so would make reloading these easier?
},
upsertPortfolio: (draftState, { payload }: { payload: Portfolio }) => {
// upsert all
Expand Down
4 changes: 2 additions & 2 deletions src/state/slices/tradeInputSlice/tradeInputSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ export const tradeInput = createSlice({

state.sellAsset = action.payload
},
setSellAssetAccountNumber: (state, action: PayloadAction<AccountId | undefined>) => {
setSellAssetAccountId: (state, action: PayloadAction<AccountId | undefined>) => {
state.sellAssetAccountId = action.payload
},
setBuyAssetAccountNumber: (state, action: PayloadAction<AccountId | undefined>) => {
setBuyAssetAccountId: (state, action: PayloadAction<AccountId | undefined>) => {
state.buyAssetAccountId = action.payload
},
setSellAmountCryptoPrecision: (state, action: PayloadAction<string>) => {
Expand Down

0 comments on commit ddd7c54

Please sign in to comment.