Skip to content

Commit

Permalink
fix: token detection across multiple networks (#29108)
Browse files Browse the repository at this point in the history
## **Description**

Fixes an issue where even though 'popular networks' was selected in the
token filter, it was only showing detected tokens on the current
network.

There were various places checking for
`Object.keys(tokenNetworkFilter).length ===
Object.keys(allNetworks).length`, which is no longer an accurate way to
check the state of the filter, since it's now been narrowed to only
include popular networks. The selector
`getIsTokenNetworkFilterEqualCurrentNetwork` is used instead.



[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29108?quickstart=1)

## **Related issues**

Fixes:

## **Manual testing steps**

1. Onboard a wallet with tokens across chains
2. When token filter is on 'popular networks', it should show # of
detected tokens across chains
3. When token filter is on current network, it should show # of detected
tokens on current chain

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
  • Loading branch information
bergeron authored Dec 11, 2024
1 parent e0f6575 commit 2693e6c
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 58 deletions.
18 changes: 5 additions & 13 deletions ui/components/app/assets/asset-list/asset-list.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@ import {
getAllDetectedTokensForSelectedAddress,
getDetectedTokensInCurrentNetwork,
getIstokenDetectionInactiveOnNonMainnetSupportedNetwork,
getIsTokenNetworkFilterEqualCurrentNetwork,
getSelectedAccount,
getTokenNetworkFilter,
} from '../../../../selectors';
import { getNetworkConfigurationsByChainId } from '../../../../../shared/modules/selectors/networks';
import {
getMultichainIsEvm,
getMultichainSelectedAccountCachedBalance,
Expand Down Expand Up @@ -79,16 +78,9 @@ const AssetList = ({ onClickAsset, showTokensLinks }: AssetListProps) => {
getIstokenDetectionInactiveOnNonMainnetSupportedNetwork,
);

const allNetworks = useSelector(getNetworkConfigurationsByChainId);
const tokenNetworkFilter = useSelector(getTokenNetworkFilter);
const allOpts: Record<string, boolean> = {};
Object.keys(allNetworks || {}).forEach((chainId) => {
allOpts[chainId] = true;
});

const allNetworksFilterShown =
Object.keys(tokenNetworkFilter).length !==
Object.keys(allOpts || {}).length;
const isTokenNetworkFilterEqualCurrentNetwork = useSelector(
getIsTokenNetworkFilterEqualCurrentNetwork,
);

const [showFundingMethodModal, setShowFundingMethodModal] = useState(false);
const [showReceiveModal, setShowReceiveModal] = useState(false);
Expand Down Expand Up @@ -118,7 +110,7 @@ const AssetList = ({ onClickAsset, showTokensLinks }: AssetListProps) => {

const totalTokens =
process.env.PORTFOLIO_VIEW &&
!allNetworksFilterShown &&
!isTokenNetworkFilterEqualCurrentNetwork &&
detectedTokensMultichain
? (Object.values(detectedTokensMultichain).reduce(
// @ts-expect-error TS18046: 'tokenArray' is of type 'unknown'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
getSelectedAccount,
getAllChainsToPoll,
getTokenNetworkFilter,
getIsTokenNetworkFilterEqualCurrentNetwork,
} from '../../../../../selectors';
import {
getCurrentChainId,
Expand Down Expand Up @@ -49,6 +50,10 @@ const NetworkFilter = ({ handleClose }: SortControlProps) => {
const allNetworks = useSelector(getNetworkConfigurationsByChainId);
const [chainsToShow, setChainsToShow] = useState<string[]>([]);
const tokenNetworkFilter = useSelector(getTokenNetworkFilter);
const isTokenNetworkFilterEqualCurrentNetwork = useSelector(
getIsTokenNetworkFilterEqualCurrentNetwork,
);

const shouldHideZeroBalanceTokens = useSelector(
getShouldHideZeroBalanceTokens,
);
Expand Down Expand Up @@ -101,10 +106,7 @@ const NetworkFilter = ({ handleClose }: SortControlProps) => {
return (
<>
<SelectableListItem
isSelected={
Object.keys(tokenNetworkFilter).length ===
Object.keys(allNetworks || {}).length
}
isSelected={!isTokenNetworkFilterEqualCurrentNetwork}
onClick={() => handleFilter(allOpts)}
testId="network-filter-all"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,12 @@ import {
MetaMetricsEventName,
MetaMetricsTokenEventSource,
} from '../../../../../shared/constants/metametrics';
import {
getCurrentChainId,
getNetworkConfigurationsByChainId,
} from '../../../../../shared/modules/selectors/networks';
import { getCurrentChainId } from '../../../../../shared/modules/selectors/networks';
import {
getAllDetectedTokensForSelectedAddress,
getCurrentNetwork,
getDetectedTokensInCurrentNetwork,
getTokenNetworkFilter,
getIsTokenNetworkFilterEqualCurrentNetwork,
} from '../../../../selectors';

import Popover from '../../../ui/popover';
Expand All @@ -40,16 +37,9 @@ const DetectedTokenSelectionPopover = ({
const chainId = useSelector(getCurrentChainId);

const detectedTokens = useSelector(getDetectedTokensInCurrentNetwork);
const allNetworks = useSelector(getNetworkConfigurationsByChainId);
const tokenNetworkFilter = useSelector(getTokenNetworkFilter);
const allOpts = {};
Object.keys(allNetworks || {}).forEach((networkId) => {
allOpts[networkId] = true;
});

const allNetworksFilterShown =
Object.keys(tokenNetworkFilter).length !==
Object.keys(allOpts || {}).length;
const isTokenNetworkFilterEqualCurrentNetwork = useSelector(
getIsTokenNetworkFilterEqualCurrentNetwork,
);

const currentNetwork = useSelector(getCurrentNetwork);

Expand All @@ -58,13 +48,18 @@ const DetectedTokenSelectionPopover = ({
);

const totalTokens = useMemo(() => {
return process.env.PORTFOLIO_VIEW && !allNetworksFilterShown
return process.env.PORTFOLIO_VIEW &&
!isTokenNetworkFilterEqualCurrentNetwork
? Object.values(detectedTokensMultichain).reduce(
(count, tokenArray) => count + tokenArray.length,
0,
)
: detectedTokens.length;
}, [detectedTokensMultichain, detectedTokens, allNetworksFilterShown]);
}, [
detectedTokensMultichain,
detectedTokens,
isTokenNetworkFilterEqualCurrentNetwork,
]);

const { selected: selectedTokens = [] } =
sortingBasedOnTokenSelection(tokensListDetected);
Expand Down Expand Up @@ -124,7 +119,8 @@ const DetectedTokenSelectionPopover = ({
onClose={onClose}
footer={footer}
>
{process.env.PORTFOLIO_VIEW && !allNetworksFilterShown ? (
{process.env.PORTFOLIO_VIEW &&
!isTokenNetworkFilterEqualCurrentNetwork ? (
<Box margin={3}>
{Object.entries(detectedTokensMultichain).map(
([networkId, tokens]) => {
Expand Down
39 changes: 24 additions & 15 deletions ui/components/app/detected-token/detected-token.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
import {
getAllDetectedTokensForSelectedAddress,
getDetectedTokensInCurrentNetwork,
getTokenNetworkFilter,
getIsTokenNetworkFilterEqualCurrentNetwork,
} from '../../../selectors';
import { MetaMetricsContext } from '../../../contexts/metametrics';

Expand Down Expand Up @@ -63,27 +63,30 @@ const DetectedToken = ({ setShowDetectedTokens }) => {
);
const currentChainId = useSelector(getCurrentChainId);
const allNetworks = useSelector(getNetworkConfigurationsByChainId);
const tokenNetworkFilter = useSelector(getTokenNetworkFilter);
const allOpts = {};
Object.keys(allNetworks || {}).forEach((chainId) => {
allOpts[chainId] = true;
});

const allNetworksFilterShown =
Object.keys(tokenNetworkFilter).length !==
Object.keys(allOpts || {}).length;
const isTokenNetworkFilterEqualCurrentNetwork = useSelector(
getIsTokenNetworkFilterEqualCurrentNetwork,
);

const totalDetectedTokens = useMemo(() => {
return process.env.PORTFOLIO_VIEW && !allNetworksFilterShown
return process.env.PORTFOLIO_VIEW &&
!isTokenNetworkFilterEqualCurrentNetwork
? Object.values(detectedTokensMultichain).flat().length
: detectedTokens.length;
}, [detectedTokens, detectedTokensMultichain, allNetworksFilterShown]);
}, [
detectedTokens,
detectedTokensMultichain,
isTokenNetworkFilterEqualCurrentNetwork,
]);

const [tokensListDetected, setTokensListDetected] = useState({});

useEffect(() => {
const newTokensList = () => {
if (process.env.PORTFOLIO_VIEW && !allNetworksFilterShown) {
if (
process.env.PORTFOLIO_VIEW &&
!isTokenNetworkFilterEqualCurrentNetwork
) {
return Object.entries(detectedTokensMultichain).reduce(
(acc, [chainId, tokens]) => {
if (Array.isArray(tokens)) {
Expand Down Expand Up @@ -112,7 +115,7 @@ const DetectedToken = ({ setShowDetectedTokens }) => {

setTokensListDetected(newTokensList());
}, [
allNetworksFilterShown,
isTokenNetworkFilterEqualCurrentNetwork,
detectedTokensMultichain,
detectedTokens,
currentChainId,
Expand Down Expand Up @@ -141,7 +144,10 @@ const DetectedToken = ({ setShowDetectedTokens }) => {
});
});

if (process.env.PORTFOLIO_VIEW && !allNetworksFilterShown) {
if (
process.env.PORTFOLIO_VIEW &&
!isTokenNetworkFilterEqualCurrentNetwork
) {
const tokensByChainId = selectedTokens.reduce((acc, token) => {
const { chainId } = token;

Expand Down Expand Up @@ -197,7 +203,10 @@ const DetectedToken = ({ setShowDetectedTokens }) => {
},
});

if (process.env.PORTFOLIO_VIEW && !allNetworksFilterShown) {
if (
process.env.PORTFOLIO_VIEW &&
!isTokenNetworkFilterEqualCurrentNetwork
) {
// group deselected tokens by chainId
const groupedByChainId = deSelectedTokens.reduce((acc, token) => {
const { chainId } = token;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
import {
getDetectedTokensInCurrentNetwork,
getAllDetectedTokensForSelectedAddress,
getTokenNetworkFilter,
getIsTokenNetworkFilterEqualCurrentNetwork,
} from '../../../selectors';
import { MetaMetricsContext } from '../../../contexts/metametrics';
import {
Expand All @@ -28,18 +28,17 @@ export const DetectedTokensBanner = ({
}) => {
const t = useI18nContext();
const trackEvent = useContext(MetaMetricsContext);
const tokenNetworkFilter = useSelector(getTokenNetworkFilter);
const isTokenNetworkFilterEqualCurrentNetwork = useSelector(
getIsTokenNetworkFilterEqualCurrentNetwork,
);

const allNetworks = useSelector(getNetworkConfigurationsByChainId);

const allOpts = {};
Object.keys(allNetworks || {}).forEach((chainId) => {
allOpts[chainId] = true;
});

const allNetworksFilterShown =
Object.keys(tokenNetworkFilter).length !==
Object.keys(allOpts || {}).length;

const detectedTokens = useSelector(getDetectedTokensInCurrentNetwork);

const detectedTokensMultichain = useSelector(
Expand All @@ -48,14 +47,14 @@ export const DetectedTokensBanner = ({
const chainId = useSelector(getCurrentChainId);

const detectedTokensDetails =
process.env.PORTFOLIO_VIEW && !allNetworksFilterShown
process.env.PORTFOLIO_VIEW && !isTokenNetworkFilterEqualCurrentNetwork
? Object.values(detectedTokensMultichain)
.flat()
.map(({ address, symbol }) => `${symbol} - ${address}`)
: detectedTokens.map(({ address, symbol }) => `${symbol} - ${address}`);

const totalTokens =
process.env.PORTFOLIO_VIEW && !allNetworksFilterShown
process.env.PORTFOLIO_VIEW && !isTokenNetworkFilterEqualCurrentNetwork
? Object.values(detectedTokensMultichain).reduce(
(count, tokenArray) => count + tokenArray.length,
0,
Expand Down
59 changes: 59 additions & 0 deletions ui/selectors/selectors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2178,4 +2178,63 @@ describe('#getConnectedSitesList', () => {
});
});
});

describe('getIsTokenNetworkFilterEqualCurrentNetwork', () => {
beforeEach(() => {
process.env.PORTFOLIO_VIEW = 'true';
});

afterEach(() => {
process.env.PORTFOLIO_VIEW = undefined;
});

it('returns true when the token network filter is equal to the current network', () => {
const state = {
metamask: {
preferences: {
tokenNetworkFilter: {
'0x1': true,
},
},
selectedNetworkClientId: 'mainnetNetworkConfigurationId',
networkConfigurationsByChainId: {
'0x1': {
chainId: '0x1',
rpcEndpoints: [
{ networkClientId: 'mainnetNetworkConfigurationId' },
],
},
},
},
};
expect(selectors.getIsTokenNetworkFilterEqualCurrentNetwork(state)).toBe(
true,
);
});

it('returns false when the token network filter is on multiple networks', () => {
const state = {
metamask: {
preferences: {
tokenNetworkFilter: {
'0x1': true,
'0x89': true,
},
},
selectedNetworkClientId: 'mainnetNetworkConfigurationId',
networkConfigurationsByChainId: {
'0x1': {
chainId: '0x1',
rpcEndpoints: [
{ networkClientId: 'mainnetNetworkConfigurationId' },
],
},
},
},
};
expect(selectors.getIsTokenNetworkFilterEqualCurrentNetwork(state)).toBe(
false,
);
});
});
});

0 comments on commit 2693e6c

Please sign in to comment.