Skip to content

Commit

Permalink
fix: PortfolioView swap native token bug (#28639)
Browse files Browse the repository at this point in the history
## **Description**

When `PORTFOLIO_VIEW` feature flag is enabled, when swapping a native
token from a different chain than the globally selected chain, the
incorrect native token would be prepoulated in the `fromToken` in the
swap UI.

For instance, if user is on Ethereum mainnet, navigated to POL, then
attempted to swap, the globally selected network would change from
Ethereum mainnet to Polygon mainnet (expected), but the swaps
`fromToken` would still be POL (unexpected)

Changes in this PR fixes this, and prepoulates `fromToken` with the
native token from the correct chain.

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

## **Related issues**

Fixes: #28534

## **Manual testing steps**

1. `PORTFOLIO_VIEW=1 yarn webpack --watch`
2. Import wallet with at least two networks added
3. When "All Networks" is toggled, attempt to swap a native token from
another network. Ensure that the token prepopulated in the swap UI is
the native token from the correct chain
4. Ensure swap completes successfully.

## **Screenshots/Recordings**


https://github.com/user-attachments/assets/016ffa54-9ed1-450c-9aa0-da27f0fd6caa

## **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.

---------

Co-authored-by: David Walsh <[email protected]>
  • Loading branch information
gambinish and darkwing authored Nov 25, 2024
1 parent 9c4d1b4 commit ff635d2
Show file tree
Hide file tree
Showing 3 changed files with 223 additions and 10 deletions.
17 changes: 13 additions & 4 deletions ui/pages/asset/components/asset-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,21 @@ const AssetPage = ({
const selectedAccount = useSelector(getSelectedAccount);
const currency = useSelector(getCurrentCurrency);
const conversionRate = useSelector(getConversionRate);
const isBridgeChain = useSelector(getIsBridgeChain);
const isBuyableChain = useSelector(getIsNativeTokenBuyable);
const defaultSwapsToken = useSelector(getSwapsDefaultToken, isEqual);

const { chainId, type, symbol, name, image, decimals } = asset;

// These need to be specific to the asset and not the current chain
const defaultSwapsToken = useSelector(
(state) => getSwapsDefaultToken(state, chainId),
isEqual,
);
const isSwapsChain = useSelector((state) => getIsSwapsChain(state, chainId));
const isBridgeChain = useSelector((state) =>
getIsBridgeChain(state, chainId),
);

const account = useSelector(getSelectedInternalAccount, isEqual);
const isSwapsChain = useSelector(getIsSwapsChain);
const isSigningEnabled =
account.methods.includes(EthMethod.SignTransaction) ||
account.methods.includes(EthMethod.SignUserOperation);
Expand All @@ -132,7 +142,6 @@ const AssetPage = ({
const selectedAccountTokenBalancesAcrossChains =
tokenBalances[selectedAccount.address];

const { chainId, type, symbol, name, image, decimals } = asset;
const isMetaMetricsEnabled = useSelector(getParticipateInMetaMetrics);
const isMarketingEnabled = useSelector(getDataCollectionForMarketing);
const metaMetricsId = useSelector(getMetaMetricsId);
Expand Down
17 changes: 11 additions & 6 deletions ui/selectors/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1495,14 +1495,17 @@ export function getWeb3ShimUsageStateForOrigin(state, origin) {
* objects, per the above description.
*
* @param {object} state - the redux state object
* @param {string} overrideChainId - the chainId to override the current chainId
* @returns {SwapsEthToken} The token object representation of the currently
* selected account's ETH balance, as expected by the Swaps API.
*/

export function getSwapsDefaultToken(state) {
export function getSwapsDefaultToken(state, overrideChainId = null) {
const selectedAccount = getSelectedAccount(state);
const balance = selectedAccount?.balance;
const chainId = getCurrentChainId(state);
const currentChainId = getCurrentChainId(state);

const chainId = overrideChainId ?? currentChainId;
const defaultTokenObject = SWAPS_CHAINID_DEFAULT_TOKEN_MAP[chainId];

return {
Expand All @@ -1516,8 +1519,9 @@ export function getSwapsDefaultToken(state) {
};
}

export function getIsSwapsChain(state) {
const chainId = getCurrentChainId(state);
export function getIsSwapsChain(state, overrideChainId) {
const currentChainId = getCurrentChainId(state);
const chainId = overrideChainId ?? currentChainId;
const isNotDevelopment =
process.env.METAMASK_ENVIRONMENT !== 'development' &&
process.env.METAMASK_ENVIRONMENT !== 'testing';
Expand All @@ -1526,8 +1530,9 @@ export function getIsSwapsChain(state) {
: ALLOWED_DEV_SWAPS_CHAIN_IDS.includes(chainId);
}

export function getIsBridgeChain(state) {
const chainId = getCurrentChainId(state);
export function getIsBridgeChain(state, overrideChainId) {
const currentChainId = getCurrentChainId(state);
const chainId = overrideChainId ?? currentChainId;
return ALLOWED_BRIDGE_CHAIN_IDS.includes(chainId);
}

Expand Down
199 changes: 199 additions & 0 deletions ui/selectors/selectors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1978,4 +1978,203 @@ describe('#getConnectedSitesList', () => {
expect(selectors.getSelectedEvmInternalAccount(state)).toBe(undefined);
});
});

describe('getSwapsDefaultToken', () => {
it('returns the token object for the current chainId when no overrideChainId is provided', () => {
const expectedToken = {
symbol: 'ETH',
name: 'Ether',
address: '0x0000000000000000000000000000000000000000',
decimals: 18,
balance: '966987986469506564059',
string: '966.988',
iconUrl: './images/black-eth-logo.svg',
};

const result = selectors.getSwapsDefaultToken(mockState);

expect(result).toStrictEqual(expectedToken);
});

it('returns the token object for the overridden chainId when overrideChainId is provided', () => {
const getCurrentChainIdSpy = jest.spyOn(selectors, 'getCurrentChainId');
const expectedToken = {
symbol: 'POL',
name: 'Polygon',
address: '0x0000000000000000000000000000000000000000',
decimals: 18,
balance: '966987986469506564059',
string: '966.988',
iconUrl: './images/pol-token.svg',
};

const result = selectors.getSwapsDefaultToken(
mockState,
CHAIN_IDS.POLYGON,
);

expect(result).toStrictEqual(expectedToken);
expect(getCurrentChainIdSpy).not.toHaveBeenCalled(); // Ensure overrideChainId is used
});
});

describe('getIsSwapsChain', () => {
it('returns true for an allowed chainId in production environment', () => {
process.env.METAMASK_ENVIRONMENT = 'production';

const state = {
...mockState,
metamask: {
...mockState.metamask,
selectedNetworkClientId: 'testNetworkConfigurationId', // corresponds to mainnet RPC in mockState
},
};

const result = selectors.getIsSwapsChain(state);

expect(result).toBe(true);
});

it('returns true for an allowed chainId in development environment', () => {
process.env.METAMASK_ENVIRONMENT = 'development';

const state = {
...mockState,
metamask: {
...mockState.metamask,
selectedNetworkClientId: 'goerli',
},
};

const result = selectors.getIsSwapsChain(state);

expect(result).toBe(true);
});

it('returns false for a disallowed chainId in production environment', () => {
process.env.METAMASK_ENVIRONMENT = 'production';

const state = {
...mockState,
metamask: {
...mockState.metamask,
selectedNetworkClientId: 'fooChain', // corresponds to mainnet RPC in mockState
networkConfigurationsByChainId: {
'0x8080': {
chainId: '0x8080',
name: 'Custom Mainnet RPC',
nativeCurrency: 'ETH',
defaultRpcEndpointIndex: 0,
rpcEndpoints: [
{
type: 'custom',
url: 'https://testrpc.com',
networkClientId: 'fooChain',
},
],
},
},
},
};

const result = selectors.getIsSwapsChain(state);

expect(result).toBe(false);
});

it('returns false for a disallowed chainId in development environment', () => {
process.env.METAMASK_ENVIRONMENT = 'development';

const state = {
...mockState,
metamask: {
...mockState.metamask,
selectedNetworkClientId: 'fooChain', // corresponds to mainnet RPC in mockState
networkConfigurationsByChainId: {
'0x8080': {
chainId: '0x8080',
name: 'Custom Mainnet RPC',
nativeCurrency: 'ETH',
defaultRpcEndpointIndex: 0,
rpcEndpoints: [
{
type: 'custom',
url: 'https://testrpc.com',
networkClientId: 'fooChain',
},
],
},
},
},
};

const result = selectors.getIsSwapsChain(state);

expect(result).toBe(false);
});

it('respects the overrideChainId parameter', () => {
process.env.METAMASK_ENVIRONMENT = 'production';

const getCurrentChainIdSpy = jest.spyOn(selectors, 'getCurrentChainId');

const result = selectors.getIsSwapsChain(mockState, '0x89');
expect(result).toBe(true);
expect(getCurrentChainIdSpy).not.toHaveBeenCalled(); // Ensure overrideChainId is used
});
});

describe('getIsBridgeChain', () => {
it('returns true for an allowed bridge chainId', () => {
const state = {
...mockState,
metamask: {
...mockState.metamask,
selectedNetworkClientId: 'testNetworkConfigurationId', // corresponds to mainnet RPC in mockState
},
};

const result = selectors.getIsBridgeChain(state);

expect(result).toBe(true);
});

it('returns false for a disallowed bridge chainId', () => {
const state = {
...mockState,
metamask: {
...mockState.metamask,
selectedNetworkClientId: 'fooChain', // corresponds to mainnet RPC in mockState
networkConfigurationsByChainId: {
'0x8080': {
chainId: '0x8080',
name: 'Custom Mainnet RPC',
nativeCurrency: 'ETH',
defaultRpcEndpointIndex: 0,
rpcEndpoints: [
{
type: 'custom',
url: 'https://testrpc.com',
networkClientId: 'fooChain',
},
],
},
},
},
};

const result = selectors.getIsBridgeChain(state);

expect(result).toBe(false);
});

it('respects the overrideChainId parameter', () => {
const getCurrentChainIdSpy = jest.spyOn(selectors, 'getCurrentChainId');

const result = selectors.getIsBridgeChain(mockState, '0x89');

expect(result).toBe(true);
expect(getCurrentChainIdSpy).not.toHaveBeenCalled(); // Ensure overrideChainId is used
});
});
});

0 comments on commit ff635d2

Please sign in to comment.