From f046660999b03d21438b5e5fa1630593035e035a Mon Sep 17 00:00:00 2001 From: woodenfurniture <125113430+woodenfurniture@users.noreply.github.com> Date: Wed, 21 Feb 2024 14:01:50 +1100 Subject: [PATCH 1/3] fix: only use the matching account when the chain id is the correct one --- .../MultiHopTrade/hooks/useReceiveAddress.tsx | 16 ++++++++++++---- src/state/slices/tradeInputSlice/selectors.ts | 15 ++++++++++++--- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/components/MultiHopTrade/hooks/useReceiveAddress.tsx b/src/components/MultiHopTrade/hooks/useReceiveAddress.tsx index abdbf45fbbb..cd0094b5b12 100644 --- a/src/components/MultiHopTrade/hooks/useReceiveAddress.tsx +++ b/src/components/MultiHopTrade/hooks/useReceiveAddress.tsx @@ -51,9 +51,15 @@ export const useReceiveAddress = ({ 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 @@ -62,7 +68,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, diff --git a/src/state/slices/tradeInputSlice/selectors.ts b/src/state/slices/tradeInputSlice/selectors.ts index a517033982f..8eaf47368f7 100644 --- a/src/state/slices/tradeInputSlice/selectors.ts +++ b/src/state/slices/tradeInputSlice/selectors.ts @@ -1,4 +1,5 @@ import { createSelector } from '@reduxjs/toolkit' +import { fromAccountId } from '@shapeshiftoss/caip' import type { Selector } from 'react-redux' import { bn } from 'lib/bignumber/bignumber' import { toBaseUnit } from 'lib/math' @@ -110,9 +111,17 @@ export const selectLastHopBuyAccountId = createSelector( selectAccountIdParamFromFilter, (tradeInput, buyAsset, accountIdAssetValues, accountIds, maybeMatchingBuyAccountId) => { // 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 + } + // an AccountId was found matching the sell asset's account number, AND the chainId is correct, return it + // TODO: use account number + if ( + maybeMatchingBuyAccountId && + buyAsset.chainId === fromAccountId(maybeMatchingBuyAccountId).chainId + ) { + return maybeMatchingBuyAccountId + } const highestFiatBalanceBuyAccountId = getHighestUserCurrencyBalanceAccountByAssetId( accountIdAssetValues, From 231f2eed2bb6e4523a6087d4a0adfb5a8ca9968f Mon Sep 17 00:00:00 2001 From: woodenfurniture <125113430+woodenfurniture@users.noreply.github.com> Date: Wed, 21 Feb 2024 14:28:24 +1100 Subject: [PATCH 2/3] fix: try to match the buy account number to the sell account number (not id) when selecting assets --- .../MultiHopTrade/hooks/useAccountIds.tsx | 35 ++-------------- .../useGetTradeQuotes/useGetTradeQuotes.tsx | 5 +-- .../MultiHopTrade/hooks/useReceiveAddress.tsx | 7 +--- src/state/slices/tradeInputSlice/selectors.ts | 41 ++++++++++++++----- 4 files changed, 35 insertions(+), 53 deletions(-) diff --git a/src/components/MultiHopTrade/hooks/useAccountIds.tsx b/src/components/MultiHopTrade/hooks/useAccountIds.tsx index e90df7366c4..85f82ac7d86 100644 --- a/src/components/MultiHopTrade/hooks/useAccountIds.tsx +++ b/src/components/MultiHopTrade/hooks/useAccountIds.tsx @@ -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' @@ -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 }), - ) + const buyAssetAccountId = useAppSelector(selectLastHopBuyAccountId) // Setters - the selectors above initially select a *default* value, but eventually onAccountIdChange may fire if the user changes the account diff --git a/src/components/MultiHopTrade/hooks/useGetTradeQuotes/useGetTradeQuotes.tsx b/src/components/MultiHopTrade/hooks/useGetTradeQuotes/useGetTradeQuotes.tsx index f498ce16001..396cc4e0f26 100644 --- a/src/components/MultiHopTrade/hooks/useGetTradeQuotes/useGetTradeQuotes.tsx +++ b/src/components/MultiHopTrade/hooks/useGetTradeQuotes/useGetTradeQuotes.tsx @@ -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 }), - ) + const buyAccountId = useAppSelector(selectLastHopBuyAccountId) const userslippageTolerancePercentageDecimal = useAppSelector(selectUserSlippagePercentageDecimal) diff --git a/src/components/MultiHopTrade/hooks/useReceiveAddress.tsx b/src/components/MultiHopTrade/hooks/useReceiveAddress.tsx index cd0094b5b12..4da30b6e32c 100644 --- a/src/components/MultiHopTrade/hooks/useReceiveAddress.tsx +++ b/src/components/MultiHopTrade/hooks/useReceiveAddress.tsx @@ -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, @@ -39,11 +38,7 @@ export const useReceiveAddress = ({ // Selectors const buyAsset = useAppSelector(selectInputBuyAsset) - const sellAssetAccountId = useAppSelector(selectFirstHopSellAccountId) - - const buyAccountId = useAppSelector(state => - selectLastHopBuyAccountId(state, { accountId: sellAssetAccountId }), - ) + const buyAccountId = useAppSelector(selectLastHopBuyAccountId) const buyAccountMetadata = useAppSelector(state => selectPortfolioAccountMetadataByAccountId(state, { accountId: buyAccountId }), ) diff --git a/src/state/slices/tradeInputSlice/selectors.ts b/src/state/slices/tradeInputSlice/selectors.ts index 8eaf47368f7..98ffa5f460f 100644 --- a/src/state/slices/tradeInputSlice/selectors.ts +++ b/src/state/slices/tradeInputSlice/selectors.ts @@ -1,18 +1,20 @@ import { createSelector } from '@reduxjs/toolkit' -import { fromAccountId } from '@shapeshiftoss/caip' import type { Selector } from 'react-redux' 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, @@ -108,18 +110,35 @@ export const selectLastHopBuyAccountId = createSelector( selectInputBuyAsset, selectPortfolioAssetAccountBalancesSortedUserCurrency, selectWalletAccountIds, - selectAccountIdParamFromFilter, - (tradeInput, buyAsset, accountIdAssetValues, accountIds, maybeMatchingBuyAccountId) => { + selectAccountIdByAccountNumberAndChainId, + selectFirstHopSellAccountId, + selectPortfolioAccountMetadata, + ( + tradeInput, + buyAsset, + accountIdAssetValues, + 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, AND the chainId is correct, return it - // TODO: use account number - if ( - maybeMatchingBuyAccountId && - buyAsset.chainId === fromAccountId(maybeMatchingBuyAccountId).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 + ? accountIdByAccountNumberAndChainId[maybeMatchingBuyAccountNumber]?.[buyAsset.chainId] + : undefined + + // an AccountId was found matching the sell asset's account number and chainId, return it + if (maybeMatchingBuyAccountId) { return maybeMatchingBuyAccountId } From 8b15b182d45db09051b4f494c8652b4fda230de3 Mon Sep 17 00:00:00 2001 From: woodenfurniture <125113430+woodenfurniture@users.noreply.github.com> Date: Wed, 21 Feb 2024 14:31:54 +1100 Subject: [PATCH 3/3] fix: last-resport-default for buy account selection should be account 0 --- src/state/slices/tradeInputSlice/selectors.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/state/slices/tradeInputSlice/selectors.ts b/src/state/slices/tradeInputSlice/selectors.ts index 98ffa5f460f..80060a736cb 100644 --- a/src/state/slices/tradeInputSlice/selectors.ts +++ b/src/state/slices/tradeInputSlice/selectors.ts @@ -108,7 +108,6 @@ export const selectLastHopSellAccountId = selectFirstHopSellAccountId export const selectLastHopBuyAccountId = createSelector( selectTradeInput, selectInputBuyAsset, - selectPortfolioAssetAccountBalancesSortedUserCurrency, selectWalletAccountIds, selectAccountIdByAccountNumberAndChainId, selectFirstHopSellAccountId, @@ -116,7 +115,6 @@ export const selectLastHopBuyAccountId = createSelector( ( tradeInput, buyAsset, - accountIdAssetValues, accountIds, accountIdByAccountNumberAndChainId, firstHopSellAccountId, @@ -142,14 +140,8 @@ export const selectLastHopBuyAccountId = createSelector( return maybeMatchingBuyAccountId } - const highestFiatBalanceBuyAccountId = getHighestUserCurrencyBalanceAccountByAssetId( - accountIdAssetValues, - buyAsset.assetId, - ) - const firstBuyAssetAccountId = getFirstAccountIdByChainId(accountIds, buyAsset.chainId) - // otherwise return a sane default - return highestFiatBalanceBuyAccountId ?? firstBuyAssetAccountId + return getFirstAccountIdByChainId(accountIds, buyAsset.chainId) }, )