Skip to content

fix: cp-13.0.0 ensure we enable any missing networks when performing swap/bridge. #34376

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.

Sign up for GitHub

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

Merged
HowardBraham merged 8 commits into from
Jul 21, 2025
Merged

fix: cp-13.0.0 ensure we enable any missing networks when performing swap/bridge. #34376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
db12945
fix: ensure we enable any missing networks when performing swap/bridge.
salimtb Jul 18, 2025
977041e
fix: add e2e test
salimtb Jul 18, 2025
c5fb3d3
fix: fix linter
salimtb Jul 18, 2025
b7d9452
fix: fix flakiness
salimtb Jul 18, 2025
cf588e3
fix: fix flaky test
salimtb Jul 18, 2025
953b6dc
fix: clean up
salimtb Jul 18, 2025
05141ef
fix: fix lint
salimtb Jul 18, 2025
5fdc6e4
fix: clean up
salimtb Jul 18, 2025
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
57 changes: 56 additions & 1 deletion test/e2e/tests/bridge/bridge-positive-cases.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import { Suite } from 'mocha';
import { unlockWallet, withFixtures } from '../../helpers';
import { unlockWallet, veryLargeDelayMs, withFixtures } from '../../helpers';
import HomePage from '../../page-objects/pages/home/homepage';
import {
switchToNetworkFromSendFlow,
searchAndSwitchToNetworkFromSendFlow,
} from '../../page-objects/flows/network.flow';
import { disableStxSetting } from '../../page-objects/flows/toggle-stx-setting.flow';
import BridgeQuotePage from '../../page-objects/pages/bridge/quote-page';
import NetworkManager, {
NetworkId,
} from '../../page-objects/pages/network-manager';
import { DEFAULT_BRIDGE_FEATURE_FLAGS } from './constants';
import { bridgeTransaction, getBridgeFixtures } from './bridge-test-utils';

Expand Down Expand Up @@ -90,4 +94,55 @@ describe('Bridge tests', function (this: Suite) {
},
);
});

it('Execute bridge transactions on non enabled networks', async function () {
await withFixtures(
getBridgeFixtures(
this.test?.fullTitle(),
DEFAULT_BRIDGE_FEATURE_FLAGS,
false,
),
async ({ driver }) => {
await unlockWallet(driver);

// disable Linea network
const networkManager = new NetworkManager(driver);
await networkManager.openNetworkManager();
try {
await networkManager.deselectNetwork(NetworkId.LINEA);
} catch (error) {
console.log('Linea network is not selected');
return;
}
await networkManager.closeNetworkManager();

// Navigate to Bridge page
const homePage = new HomePage(driver);
await homePage.startBridgeFlow();

const bridgePage = new BridgeQuotePage(driver);
await bridgePage.enterBridgeQuote({
amount: '25',
tokenFrom: 'ETH',
tokenTo: 'DAI',
fromChain: 'Linea',
toChain: 'Ethereum',
});

await bridgePage.goBack();

// check if the Linea network is selected
await networkManager.openNetworkManager();
await driver.delay(veryLargeDelayMs);

try {
await networkManager.checkNetworkIsSelected('Linea Mainnet');
} catch (error) {
console.log('Linea network is not selected');
}

await networkManager.closeNetworkManager();
},
);
});
});
73 changes: 72 additions & 1 deletion ui/pages/bridge/prepare/prepare-bridge-page.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
import React from 'react';
import { act } from '@testing-library/react';
import * as reactRouterUtils from 'react-router-dom-v5-compat';
import * as ReactReduxModule from 'react-redux';
import { zeroAddress } from 'ethereumjs-util';
import { userEvent } from '@testing-library/user-event';
import { toEvmCaipChainId } from '@metamask/multichain-network-controller';
import { renderHook } from '@testing-library/react-hooks';
import { fireEvent, renderWithProvider } from '../../../../test/jest';
import configureStore from '../../../store/store';
import { createBridgeMockStore } from '../../../../test/data/bridge/mock-bridge-store';
import { CHAIN_IDS } from '../../../../shared/constants/network';
import { createTestProviderTools } from '../../../../test/stub/provider';
import PrepareBridgePage from './prepare-bridge-page';
import * as SelectorsModule from '../../../selectors/multichain/networks';
import * as ActionsModule from '../../../store/actions';
import PrepareBridgePage, {
useEnableMissingNetwork,
} from './prepare-bridge-page';

describe('PrepareBridgePage', () => {
beforeAll(() => {
Expand Down Expand Up @@ -272,3 +278,68 @@ describe('PrepareBridgePage', () => {
expect(getByTestId('from-amount').closest('input')).toHaveValue('2.131');
});
});

describe('useEnableMissingNetwork', () => {
const arrangeReactReduxMocks = () => {
jest
.spyOn(ReactReduxModule, 'useSelector')
.mockImplementation((selector) => selector({}));
jest.spyOn(ReactReduxModule, 'useDispatch').mockReturnValue(jest.fn());
};

const arrange = () => {
arrangeReactReduxMocks();

const mockGetEnabledNetworksByNamespace = jest
.spyOn(SelectorsModule, 'getEnabledNetworksByNamespace')
.mockReturnValue({
'0x1': true,
'0xe708': true,
});
const mockSetEnabledNetworks = jest.spyOn(
ActionsModule,
'setEnabledNetworks',
);

return {
mockGetEnabledNetworksByNamespace,
mockSetEnabledNetworks,
};
};

beforeEach(() => {
jest.clearAllMocks();
});

it('enables popular network when not already enabled', () => {
const mocks = arrange();
mocks.mockGetEnabledNetworksByNamespace.mockReturnValue({ '0xe708': true }); // Missing 0x1.
const hook = renderHook(() => useEnableMissingNetwork());

// Act - enable 0x1
hook.result.current('0x1');

// Assert - Adds 0x1 to enabled networks
expect(mocks.mockSetEnabledNetworks).toHaveBeenCalledWith(
['0x1', '0xe708'],
'eip155',
);
});

it('does not enable popular network if already enabled', () => {
const mocks = arrange();
const hook = renderHook(() => useEnableMissingNetwork());

// Act - enable 0x1 (already enabled)
hook.result.current('0x1');
expect(mocks.mockSetEnabledNetworks).not.toHaveBeenCalled();
});

it('does not enable non-popular network', () => {
const mocks = arrange();
const hook = renderHook(() => useEnableMissingNetwork());

hook.result.current('0x1111'); // not popular network
expect(mocks.mockSetEnabledNetworks).not.toHaveBeenCalled();
});
});
53 changes: 53 additions & 0 deletions ui/pages/bridge/prepare/prepare-bridge-page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
BRIDGE_DEFAULT_SLIPPAGE,
GenericQuoteRequest,
} from '@metamask/bridge-controller';
import { Hex, parseCaipChainId } from '@metamask/utils';
import {
setFromToken,
setFromTokenInputValue,
Expand Down Expand Up @@ -82,6 +83,7 @@ import {
} from '../../../helpers/constants/design-system';
import { useI18nContext } from '../../../hooks/useI18nContext';
import { useTokensWithFiltering } from '../../../hooks/bridge/useTokensWithFiltering';
import { setEnabledNetworks } from '../../../store/actions';
import { calcTokenValue } from '../../../../shared/lib/swaps-utils';
import {
formatTokenAmount,
Expand All @@ -102,6 +104,7 @@ import useLatestBalance from '../../../hooks/bridge/useLatestBalance';
import { useCountdownTimer } from '../../../hooks/bridge/useCountdownTimer';
import {
getCurrentKeyring,
getEnabledNetworksByNamespace,
getSelectedEvmInternalAccount,
getSelectedInternalAccount,
getTokenList,
Expand All @@ -128,14 +131,58 @@ import type { BridgeToken } from '../../../ducks/bridge/types';
import { toAssetId } from '../../../../shared/lib/asset-utils';
import { getIsSmartTransaction } from '../../../../shared/modules/selectors';
import { endTrace, TraceName } from '../../../../shared/lib/trace';
import { FEATURED_NETWORK_CHAIN_IDS } from '../../../../shared/constants/network';
import { useBridgeQueryParams } from '../../../hooks/bridge/useBridgeQueryParams';
import useBridgeDefaultToToken from '../../../hooks/bridge/useBridgeDefaultToToken';
import { BridgeInputGroup } from './bridge-input-group';
import { BridgeCTAButton } from './bridge-cta-button';
import { DestinationAccountPicker } from './components/destination-account-picker';

/**
* Ensures that any missing network gets added to the NetworkEnabledMap (which handles network polling)
*
* @returns callback to enable a network config.
*/
export const useEnableMissingNetwork = () => {
const enabledNetworksByNamespace = useSelector(getEnabledNetworksByNamespace);
const dispatch = useDispatch();

const enableMissingNetwork = useCallback(
(chainId: Hex) => {
const enabledNetworkKeys = Object.keys(enabledNetworksByNamespace ?? {});

const caipChainId = formatChainIdToCaip(chainId);
const { namespace } = parseCaipChainId(caipChainId);

if (namespace) {
const isPopularNetwork = FEATURED_NETWORK_CHAIN_IDS.includes(chainId);
Copy link
Member

@micaelae micaelae Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this enable all featured networks or just the user's imported chains that are also allowlisted by the bridge service (getFromChains)?

Copy link
Member

@micaelae micaelae Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the other implications of not adding a network to the enabled list? I wonder if this should happen earlier on in the application loading process (maybe in the useBridging hook) so Swap/Bridge networks are enabled by default, regardless of whether the user loads this component. Transaction status polling, for example, is done in the background

Copy link
Contributor Author

@Prithpal-Sooriya Prithpal-Sooriya Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great Q, will ask the team and test it out.
This realistically is just a patch, ideally we should fully decouple/remove the need to manage enabled networks on these pages/flows.

Copy link
Contributor

@salimtb salimtb Jul 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another important implication is that if a network is selected as the source or destination in the swap flow, it must be enabled; otherwise, users won’t see their assets in the token list, leading to a poor user experience.


if (isPopularNetwork) {
const isNetworkEnabled = enabledNetworkKeys.includes(chainId);
if (!isNetworkEnabled) {
const enabledEvmNetworks = enabledNetworkKeys.filter((key) =>
FEATURED_NETWORK_CHAIN_IDS.includes(key as Hex),
);
const newNetworkEnabledEvmNetworks = [
chainId,
...enabledEvmNetworks,
];
dispatch(
setEnabledNetworks(newNetworkEnabledEvmNetworks, namespace),
);
Comment on lines +170 to +172
Copy link
Contributor Author

@Prithpal-Sooriya Prithpal-Sooriya Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the special magic sauce...

For send flows we set the enable network before switching to it.
So similarly, swaps/bridge flows need to ensure that the network is enabled before we can switch to it.

Copy link
Contributor Author

@Prithpal-Sooriya Prithpal-Sooriya Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be ideal that the NetworkOrderController only be responsible for the UI, but we are very closely coupled with the the NetworkController & the selected network.

For example, all of our polling mechanisms rely on the NetworkOrderController, since we are polling on networks a user has toggled.

Copy link
Contributor Author

@Prithpal-Sooriya Prithpal-Sooriya Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleaned this up - it now only enabled missing networks, it does not move to active network.

Copy link
Member

@micaelae micaelae Jul 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: enabling a network before we switch to it

There are a number of instances where the network is switched before routing to the bridge page (see openBridgeExperience usages). Does the network need to be enabled before those routing calls too?

Copy link
Contributor Author

@Prithpal-Sooriya Prithpal-Sooriya Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CC @salimtb for visibility on this.

Longer term I would really like to decouple the need for bridging/swaps/send to rely on this NetworkOrderController.
I don't have as much understanding on the connections that these pages have to this controller. If it is just related to polling, then I feel like we should allow these pages to poll independently from the home screen.

Copy link
Contributor

@salimtb salimtb Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@micaelae yes, I believe the network needs to be enabled — otherwise, certain data like balances and conversion rates won’t be updated. As @Prithpal-Sooriya mentioned, we only keep balances, token metadata, and prices up to date for enabled networks.

Copy link
Contributor

@salimtb salimtb Jul 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the ideal solution will be to create a separated polling for the enabled swaps network when the users start the flow if you see what i mean , but we can do this separately

}
}
}
},
[dispatch, enabledNetworksByNamespace],
);

return enableMissingNetwork;
};

const PrepareBridgePage = () => {
const dispatch = useDispatch();
const enableMissingNetwork = useEnableMissingNetwork();

const t = useI18nContext();

Expand Down Expand Up @@ -622,6 +669,9 @@ const PrepareBridgePage = () => {
) {
dispatch(setToChainId(null));
}
if (isNetworkAdded(networkConfig)) {
enableMissingNetwork(networkConfig.chainId);
}
dispatch(
setFromChain({
networkConfig,
Expand Down Expand Up @@ -803,6 +853,9 @@ const PrepareBridgePage = () => {
network: toChain,
networks: toChains,
onNetworkChange: (networkConfig) => {
if (isNetworkAdded(networkConfig)) {
enableMissingNetwork(networkConfig.chainId);
}
networkConfig.chainId !== toChain?.chainId &&
trackInputEvent({
input: 'chain_destination',
Expand Down
Loading