Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: consolidate matching buy account id selection logic #6268

Merged
merged 3 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 3 additions & 32 deletions src/components/MultiHopTrade/hooks/useAccountIds.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
import type { AccountId } from '@shapeshiftoss/caip'
import { useCallback, useMemo } from 'react'
import {
selectAccountIdByAccountNumberAndChainId,
selectAccountNumberByAccountId,
selectFirstHopSellAccountId,
selectInputBuyAsset,
selectLastHopBuyAccountId,
} from 'state/slices/selectors'
import { useCallback } from 'react'
import { selectFirstHopSellAccountId, selectLastHopBuyAccountId } from 'state/slices/selectors'
import { tradeInput } from 'state/slices/tradeInputSlice/tradeInputSlice'
import { useAppDispatch, useAppSelector } from 'state/store'

Expand All @@ -19,31 +13,8 @@ export const useAccountIds = (): {
const dispatch = useAppDispatch()

// Default sellAssetAccountId selection

const sellAssetAccountId = useAppSelector(selectFirstHopSellAccountId)
const sellAssetAccountNumberFilter = useMemo(
() => ({ accountId: sellAssetAccountId }),
[sellAssetAccountId],
)
const sellAssetAccountNumber = useAppSelector(state =>
selectAccountNumberByAccountId(state, sellAssetAccountNumberFilter),
)

// Default buyAssetAccountId selection

const accountIdsByAccountNumberAndChainId = useAppSelector(
selectAccountIdByAccountNumberAndChainId,
)
const inputBuyAsset = useAppSelector(selectInputBuyAsset)
const maybeMatchingBuyAccountId =
accountIdsByAccountNumberAndChainId[sellAssetAccountNumber as number]?.[inputBuyAsset?.chainId]

// We always default the buy asset account to be synchronized with the sellAssetAccountNumber
// - if this isn't possible, i.e there is no matching account number on the buy side, we default to the highest balance
// - if this was to fail for any reason, we default to the first account number as a default
const buyAssetAccountId = useAppSelector(state =>
selectLastHopBuyAccountId(state, { accountId: maybeMatchingBuyAccountId }),
)
kaladinlight marked this conversation as resolved.
Show resolved Hide resolved
const buyAssetAccountId = useAppSelector(selectLastHopBuyAccountId)

// Setters - the selectors above initially select a *default* value, but eventually onAccountIdChange may fire if the user changes the account

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,7 @@ export const useGetTradeQuotes = () => {
const isDebouncing = debouncedSellAmountCryptoPrecision !== sellAmountCryptoPrecision

const sellAccountId = useAppSelector(selectFirstHopSellAccountId)
// No need to pass a sellAssetAccountId to synchronize the buy account here - by the time this is called, we already have a valid buyAccountId
const buyAccountId = useAppSelector(state =>
selectLastHopBuyAccountId(state, { accountId: undefined }),
)
kaladinlight marked this conversation as resolved.
Show resolved Hide resolved
const buyAccountId = useAppSelector(selectLastHopBuyAccountId)

const userslippageTolerancePercentageDecimal = useAppSelector(selectUserSlippagePercentageDecimal)

Expand Down
23 changes: 13 additions & 10 deletions src/components/MultiHopTrade/hooks/useReceiveAddress.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { useWallet } from 'hooks/useWallet/useWallet'
import { selectPortfolioAccountMetadataByAccountId } from 'state/slices/portfolioSlice/selectors'
import { isUtxoAccountId } from 'state/slices/portfolioSlice/utils'
import {
selectFirstHopSellAccountId,
selectInputBuyAsset,
selectLastHopBuyAccountId,
selectManualReceiveAddress,
Expand Down Expand Up @@ -39,21 +38,23 @@ export const useReceiveAddress = ({

// Selectors
const buyAsset = useAppSelector(selectInputBuyAsset)
const sellAssetAccountId = useAppSelector(selectFirstHopSellAccountId)

const buyAccountId = useAppSelector(state =>
selectLastHopBuyAccountId(state, { accountId: sellAssetAccountId }),
)
kaladinlight marked this conversation as resolved.
Show resolved Hide resolved
const buyAccountId = useAppSelector(selectLastHopBuyAccountId)
const buyAccountMetadata = useAppSelector(state =>
selectPortfolioAccountMetadataByAccountId(state, { accountId: buyAccountId }),
)
const manualReceiveAddress = useAppSelector(selectManualReceiveAddress)

const getReceiveAddressFromBuyAsset = useCallback(
async (buyAsset: Asset) => {
if (!wallet) return
if (!buyAccountId) return
if (!buyAccountMetadata) return
if (!wallet) {
return
}
if (!buyAccountId) {
return
}
if (!buyAccountMetadata) {
return
}
if (isUtxoAccountId(buyAccountId) && !buyAccountMetadata.accountType)
throw new Error(`Missing accountType for UTXO account ${buyAccountId}`)
const buyAssetChainId = buyAsset.chainId
Expand All @@ -62,7 +63,9 @@ export const useReceiveAddress = ({
* do NOT remove
* super dangerous - don't use the wrong bip44 params to generate receive addresses
*/
if (buyAssetChainId !== buyAssetAccountChainId) return
if (buyAssetChainId !== buyAssetAccountChainId) {
return
}
const receiveAddress = await getReceiveAddress({
asset: buyAsset,
wallet,
Expand Down
48 changes: 34 additions & 14 deletions src/state/slices/tradeInputSlice/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,17 @@ import { bn } from 'lib/bignumber/bignumber'
import { toBaseUnit } from 'lib/math'
import type { ReduxState } from 'state/reducer'
import { createDeepEqualOutputSelector } from 'state/selector-utils'
import { selectAccountIdParamFromFilter } from 'state/selectors'

import {
selectPortfolioCryptoBalanceBaseUnitByFilter,
selectWalletAccountIds,
} from '../common-selectors'
import { selectCryptoMarketData, selectUserCurrencyToUsdRate } from '../marketDataSlice/selectors'
import { selectPortfolioAssetAccountBalancesSortedUserCurrency } from '../portfolioSlice/selectors'
import {
selectAccountIdByAccountNumberAndChainId,
selectPortfolioAccountMetadata,
selectPortfolioAssetAccountBalancesSortedUserCurrency,
} from '../portfolioSlice/selectors'
import {
getFirstAccountIdByChainId,
getHighestUserCurrencyBalanceAccountByAssetId,
Expand Down Expand Up @@ -105,23 +108,40 @@ export const selectLastHopSellAccountId = selectFirstHopSellAccountId
export const selectLastHopBuyAccountId = createSelector(
selectTradeInput,
selectInputBuyAsset,
selectPortfolioAssetAccountBalancesSortedUserCurrency,
selectWalletAccountIds,
selectAccountIdParamFromFilter,
(tradeInput, buyAsset, accountIdAssetValues, accountIds, maybeMatchingBuyAccountId) => {
selectAccountIdByAccountNumberAndChainId,
selectFirstHopSellAccountId,
selectPortfolioAccountMetadata,
(
tradeInput,
buyAsset,
accountIds,
accountIdByAccountNumberAndChainId,
firstHopSellAccountId,
accountMetadata,
) => {
// return the users selection if it exists
if (tradeInput.buyAssetAccountId) return tradeInput.buyAssetAccountId
// an AccountId was found matching the sell asset's account number, return it
if (maybeMatchingBuyAccountId) return maybeMatchingBuyAccountId
if (tradeInput.buyAssetAccountId) {
return tradeInput.buyAssetAccountId
}

const highestFiatBalanceBuyAccountId = getHighestUserCurrencyBalanceAccountByAssetId(
accountIdAssetValues,
buyAsset.assetId,
)
const firstBuyAssetAccountId = getFirstAccountIdByChainId(accountIds, buyAsset.chainId)
// maybe convert the account id to an account number
const maybeMatchingBuyAccountNumber = firstHopSellAccountId
? accountMetadata[firstHopSellAccountId]?.bip44Params.accountNumber
: undefined

// maybe convert account number to account id on the buy asset chain
const maybeMatchingBuyAccountId = maybeMatchingBuyAccountNumber
gomesalexandre marked this conversation as resolved.
Show resolved Hide resolved
? accountIdByAccountNumberAndChainId[maybeMatchingBuyAccountNumber]?.[buyAsset.chainId]
: undefined

// an AccountId was found matching the sell asset's account number and chainId, return it
if (maybeMatchingBuyAccountId) {
return maybeMatchingBuyAccountId
}

// otherwise return a sane default
return highestFiatBalanceBuyAccountId ?? firstBuyAssetAccountId
return getFirstAccountIdByChainId(accountIds, buyAsset.chainId)
},
)

Expand Down
Loading