Skip to content

Commit

Permalink
chore: Add error handling for setCorrectChain (#28740)
Browse files Browse the repository at this point in the history
## **Description**

Adds error handling to `setCurrentNetwork` and additional check to make
sure that the user is never redirected to an incorrect network that is
incompatible with the token they are trying to send/swap.

This is an edge case, and should be revisited post-launch for a more
elegant/user friendly approach. We are now checking to make sure that
the network and token chainIds match before redirecting. Very rarely,
this will result in the user needing to click the send/swap button twice
before they get redirected to the send/swap flow.

In our opinion, this was preferable to potentially redirecting the user
to an incorrect network, where they could transact with a token
incompatible with the network they are on.

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

## **Related issues**

Fixes:

## **Manual testing steps**

## **Screenshots/Recordings**

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.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
gambinish authored Nov 27, 2024
1 parent 3fa28bf commit 781267a
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 18 deletions.
24 changes: 15 additions & 9 deletions ui/components/app/wallet-overview/coin-buttons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ import useBridging from '../../../hooks/bridge/useBridging';
///: END:ONLY_INCLUDE_IF
import { ReceiveModal } from '../../multichain/receive-modal';
import {
setActiveNetwork,
setSwitchedNetworkDetails,
setActiveNetworkWithError,
///: BEGIN:ONLY_INCLUDE_IF(build-flask)
sendMultichainTransaction,
setDefaultHomeActiveTabName,
Expand Down Expand Up @@ -328,13 +328,20 @@ const CoinButtons = ({

const setCorrectChain = useCallback(async () => {
if (currentChainId !== chainId) {
const networkConfigurationId = networks[chainId];
await dispatch(setActiveNetwork(networkConfigurationId));
await dispatch(
setSwitchedNetworkDetails({
networkClientId: networkConfigurationId,
}),
);
try {
const networkConfigurationId = networks[chainId];
await dispatch(setActiveNetworkWithError(networkConfigurationId));
await dispatch(
setSwitchedNetworkDetails({
networkClientId: networkConfigurationId,
}),
);
} catch (err) {
console.error(`Failed to switch chains.
Target chainId: ${chainId}, Current chainId: ${currentChainId}.
${err}`);
throw err;
}
}
}, [currentChainId, chainId, networks, dispatch]);

Expand Down Expand Up @@ -384,7 +391,6 @@ const CoinButtons = ({

const handleSwapOnClick = useCallback(async () => {
await setCorrectChain();

///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
global.platform.openTab({
url: `${mmiPortfolioUrl}/swap`,
Expand Down
25 changes: 16 additions & 9 deletions ui/pages/asset/components/token-buttons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ import useBridging from '../../../hooks/bridge/useBridging';

import { INVALID_ASSET_TYPE } from '../../../helpers/constants/error-keys';
import {
setActiveNetwork,
showModal,
setSwitchedNetworkDetails,
setActiveNetworkWithError,
} from '../../../store/actions';
import { MetaMetricsContext } from '../../../contexts/metametrics';
import {
Expand Down Expand Up @@ -123,14 +123,22 @@ const TokenButtons = ({
}, [token.isERC721, token.address, dispatch]);

const setCorrectChain = async () => {
// If we aren't presently on the chain of the asset, change to it
if (currentChainId !== token.chainId) {
const networkConfigurationId = networks[token.chainId];
await dispatch(setActiveNetwork(networkConfigurationId));
await dispatch(
setSwitchedNetworkDetails({
networkClientId: networkConfigurationId,
}),
);
try {
const networkConfigurationId = networks[token.chainId];
await dispatch(setActiveNetworkWithError(networkConfigurationId));
await dispatch(
setSwitchedNetworkDetails({
networkClientId: networkConfigurationId,
}),
);
} catch (err) {
console.error(`Failed to switch chains.
Target chainId: ${token.chainId}, Current chainId: ${currentChainId}.
${err}`);
throw err;
}
}
};

Expand Down Expand Up @@ -271,7 +279,6 @@ const TokenButtons = ({
}
onClick={async () => {
await setCorrectChain();

///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
global.platform.openTab({
url: `${mmiPortfolioUrl}/swap`,
Expand Down
17 changes: 17 additions & 0 deletions ui/store/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2492,6 +2492,23 @@ export function setActiveNetwork(
};
}

export function setActiveNetworkWithError(
networkConfigurationId: string,
): ThunkAction<void, MetaMaskReduxState, unknown, AnyAction> {
return async (dispatch) => {
log.debug(`background.setActiveNetwork: ${networkConfigurationId}`);
try {
await submitRequestToBackground('setActiveNetwork', [
networkConfigurationId,
]);
} catch (error) {
logErrorWithMessage(error);
dispatch(displayWarning('Had a problem changing networks!'));
throw new Error('Had a problem changing networks!');
}
};
}

export async function setActiveNetworkConfigurationId(
networkConfigurationId: string,
): Promise<undefined> {
Expand Down

0 comments on commit 781267a

Please sign in to comment.