Skip to content

Commit 01739ee

Browse files
SteP-n-sbfullam
andauthored
fix: use correct order for BTC network and fees cp-7.59.0 (#22244)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Fix issue with incorrect network order & fees <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: fix incorrect network order and fees ## **Related issues** Fixes: #22314 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/907764f2-695e-4cda-bda8-eaee609fc562 <img width="420" height="913" alt="simulator_screenshot_AC60FBFE-5FB0-488D-97F1-9D16106F285E" src="https://github.com/user-attachments/assets/711c89d6-8284-4265-b253-37f13c0a583a" /> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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-mobile/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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Sorts destination networks by popularity with BTC included and standardizes network fee display to fiat, with updated tests and e2e adjustments. > > - **Bridge UI** > - **Network ordering**: Export and use `ChainPopularity` to sort destination networks; add BTC (`BtcScope.Mainnet`) and adjust rankings; fallback to `Infinity` for unknown chains. > - **Dest selector**: Apply popularity sort in `BridgeDestNetworkSelector` (excludes current chain unless Unified mode). > - **Snapshots/Tests**: Update snapshots to reflect new order; add test for popularity fallback. > - **Quote Data** > - **Fees**: Always format `networkFee` using fiat `valueInCurrency` (handles `<$0.01`, `$0`, and missing values as `-`); remove primary-currency/ticker-based formatting. > - **Unit Tests**: Update and expand tests to cover new fee formatting cases and edge conditions. > - **E2E** > - Adjust `swipeNetwork` to target updated ordering. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit fc761a6. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Bryan Fullam <bryan.fullam@consensys.net>
1 parent 9e5d761 commit 01739ee

File tree

8 files changed

+156
-98
lines changed

8 files changed

+156
-98
lines changed

app/components/UI/Bridge/components/BridgeDestNetworkSelector/BridgeDestNetworkSelector.test.tsx

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,3 +152,105 @@ describe('BridgeDestNetworkSelector', () => {
152152
expect(queryByText('Optimism')).toBeTruthy();
153153
});
154154
});
155+
156+
describe('BridgeDestNetworkSelector - ChainPopularity fallback', () => {
157+
beforeEach(() => {
158+
jest.clearAllMocks();
159+
});
160+
161+
it('assigns Infinity to chains without defined popularity', () => {
162+
// Add networks with and without defined popularity to test all branch combinations:
163+
// - Optimism: HAS defined popularity (10 in ChainPopularity)
164+
// - Palm: NO defined popularity (triggers ?? Infinity)
165+
// - zkSync Era: NO defined popularity (triggers ?? Infinity)
166+
// This ensures all branch combinations are tested:
167+
// 1. Both have defined popularity (Optimism already tested in existing tests)
168+
// 2. Both lack defined popularity (Palm vs zkSync Era)
169+
// 3. One has, one doesn't (Optimism vs Palm/zkSync)
170+
const stateWithMultipleNetworks = {
171+
...initialState,
172+
engine: {
173+
...initialState.engine,
174+
backgroundState: {
175+
...initialState.engine.backgroundState,
176+
RemoteFeatureFlagController: {
177+
remoteFeatureFlags: {
178+
bridgeConfig: {
179+
minimumVersion: '0.0.0',
180+
maxRefreshCount: 5,
181+
refreshRate: 30000,
182+
support: true,
183+
chains: {
184+
'eip155:1': {
185+
isActiveSrc: true,
186+
isActiveDest: true,
187+
},
188+
'eip155:10': {
189+
// Optimism - HAS defined popularity
190+
isActiveSrc: true,
191+
isActiveDest: true,
192+
},
193+
'eip155:11297108109': {
194+
// Palm - NOT in ChainPopularity
195+
isActiveSrc: true,
196+
isActiveDest: true,
197+
},
198+
'eip155:324': {
199+
// zkSync Era - NOT in ChainPopularity
200+
isActiveSrc: true,
201+
isActiveDest: true,
202+
},
203+
},
204+
},
205+
bridgeConfigV2: {
206+
minimumVersion: '0.0.0',
207+
maxRefreshCount: 5,
208+
refreshRate: 30000,
209+
support: true,
210+
chains: {
211+
'eip155:1': {
212+
isActiveSrc: true,
213+
isActiveDest: true,
214+
isGaslessSwapEnabled: true,
215+
},
216+
'eip155:10': {
217+
// Optimism
218+
isActiveSrc: true,
219+
isActiveDest: true,
220+
isGaslessSwapEnabled: false,
221+
},
222+
'eip155:11297108109': {
223+
// Palm
224+
isActiveSrc: true,
225+
isActiveDest: true,
226+
isGaslessSwapEnabled: false,
227+
},
228+
'eip155:324': {
229+
// zkSync Era
230+
isActiveSrc: true,
231+
isActiveDest: true,
232+
isGaslessSwapEnabled: false,
233+
},
234+
},
235+
},
236+
},
237+
},
238+
},
239+
},
240+
};
241+
242+
const { getByText } = renderScreen(
243+
BridgeDestNetworkSelector,
244+
{
245+
name: Routes.BRIDGE.MODALS.DEST_NETWORK_SELECTOR,
246+
},
247+
{ state: stateWithMultipleNetworks },
248+
);
249+
250+
// All three networks should be visible and sorted by popularity
251+
// Optimism (popularity 10) should appear before Palm and zkSync Era (both Infinity)
252+
expect(getByText('Optimism')).toBeTruthy();
253+
expect(getByText('Palm')).toBeTruthy();
254+
expect(getByText('zkSync Era')).toBeTruthy();
255+
});
256+
});

app/components/UI/Bridge/components/BridgeDestNetworkSelector/__snapshots__/BridgeDestNetworkSelector.test.tsx.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,7 @@ exports[`BridgeDestNetworkSelector renders with initial state and displays netwo
637637
}
638638
}
639639
>
640-
Optimism
640+
Bitcoin
641641
</Text>
642642
</View>
643643
</View>
@@ -831,7 +831,7 @@ exports[`BridgeDestNetworkSelector renders with initial state and displays netwo
831831
}
832832
}
833833
>
834-
Bitcoin
834+
Optimism
835835
</Text>
836836
</View>
837837
</View>

app/components/UI/Bridge/components/BridgeDestNetworkSelector/index.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { NetworkRow } from '../NetworkRow';
1717
import Routes from '../../../../../constants/navigation/Routes';
1818
import { selectChainId } from '../../../../../selectors/networkController';
1919
import { BridgeViewMode } from '../../types';
20+
import { ChainPopularity } from '../BridgeDestNetworksBar';
2021

2122
export interface BridgeDestNetworkSelectorRouteParams {
2223
shouldGoToTokens?: boolean;
@@ -68,6 +69,11 @@ export const BridgeDestNetworkSelector: React.FC = () => {
6869
}
6970
return chain.chainId !== currentChainId;
7071
})
72+
.sort((a, b) => {
73+
const aPopularity = ChainPopularity[a.chainId] ?? Infinity;
74+
const bPopularity = ChainPopularity[b.chainId] ?? Infinity;
75+
return aPopularity - bPopularity;
76+
})
7177
.map((chain) => (
7278
<TouchableOpacity
7379
key={chain.chainId}

app/components/UI/Bridge/components/BridgeDestNetworksBar.tsx

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import { selectChainId } from '../../../../selectors/networkController';
3939
// Using ScrollView from react-native-gesture-handler to fix scroll issues with the bottom sheet
4040
import { ScrollView } from 'react-native-gesture-handler';
4141
///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
42-
import { SolScope } from '@metamask/keyring-api';
42+
import { BtcScope, SolScope } from '@metamask/keyring-api';
4343
import { BridgeViewMode } from '../types';
4444
///: END:ONLY_INCLUDE_IF
4545
const createStyles = (params: { theme: Theme }) => {
@@ -72,21 +72,22 @@ const createStyles = (params: { theme: Theme }) => {
7272
* 1 = most popular
7373
* Infinity = least popular
7474
*/
75-
const ChainPopularity: Record<Hex | CaipChainId, number> = {
75+
export const ChainPopularity: Record<Hex | CaipChainId, number> = {
7676
[ETH_CHAIN_ID]: 1,
77+
[BSC_CHAIN_ID]: 2,
7778
///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
78-
[SolScope.Mainnet]: 2,
79+
[BtcScope.Mainnet]: 3,
80+
[SolScope.Mainnet]: 4,
7981
///: END:ONLY_INCLUDE_IF
80-
[BASE_CHAIN_ID]: 3,
81-
[BSC_CHAIN_ID]: 4,
82-
[LINEA_CHAIN_ID]: 5,
83-
[OPTIMISM_CHAIN_ID]: 6,
84-
[ARBITRUM_CHAIN_ID]: 7,
85-
[AVALANCHE_CHAIN_ID]: 9,
82+
[BASE_CHAIN_ID]: 5,
83+
[ARBITRUM_CHAIN_ID]: 6,
84+
[LINEA_CHAIN_ID]: 7,
8685
[POLYGON_CHAIN_ID]: 8,
87-
[ZKSYNC_ERA_CHAIN_ID]: 10,
88-
[NETWORKS_CHAIN_ID.SEI]: 11,
89-
[NETWORKS_CHAIN_ID.MONAD]: 12,
86+
[AVALANCHE_CHAIN_ID]: 9,
87+
[OPTIMISM_CHAIN_ID]: 10,
88+
[ZKSYNC_ERA_CHAIN_ID]: 11,
89+
[NETWORKS_CHAIN_ID.SEI]: 12,
90+
[NETWORKS_CHAIN_ID.MONAD]: 13,
9091
};
9192

9293
const ShortChainNames: Record<Hex | CaipChainId, string> = {

app/components/UI/Bridge/components/BridgeDestTokenSelector/__snapshots__/BridgeDestTokenSelector.test.tsx.snap

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -663,7 +663,7 @@ exports[`BridgeDestTokenSelector renders with initial state and displays tokens
663663
}
664664
}
665665
>
666-
Solana
666+
Bitcoin
667667
</Text>
668668
</View>
669669
</TouchableOpacity>
@@ -717,7 +717,7 @@ exports[`BridgeDestTokenSelector renders with initial state and displays tokens
717717
}
718718
}
719719
>
720-
Optimism
720+
Solana
721721
</Text>
722722
</View>
723723
</TouchableOpacity>
@@ -771,7 +771,7 @@ exports[`BridgeDestTokenSelector renders with initial state and displays tokens
771771
}
772772
}
773773
>
774-
Bitcoin
774+
Optimism
775775
</Text>
776776
</View>
777777
</TouchableOpacity>

app/components/UI/Bridge/hooks/useBridgeQuoteData/index.ts

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,12 @@ import {
1717
fromTokenMinimalUnit,
1818
isNumberValue,
1919
} from '../../../../../util/number';
20-
import { selectPrimaryCurrency } from '../../../../../selectors/settings';
2120
import {
2221
isQuoteExpired,
2322
getQuoteRefreshRate,
2423
shouldRefreshQuote,
2524
} from '../../utils/quoteUtils';
2625

27-
import { selectTicker } from '../../../../../selectors/networkController';
2826
import { BigNumber } from 'bignumber.js';
2927
import I18n from '../../../../../../locales/i18n';
3028
import useFiatFormatter from '../../../SimulationDetails/FiatDisplay/useFiatFormatter';
@@ -51,8 +49,6 @@ export const useBridgeQuoteData = ({
5149
const isSubmittingTx = useSelector(selectIsSubmittingTx);
5250
const locale = I18n.locale;
5351
const fiatFormatter = useFiatFormatter();
54-
const primaryCurrency = useSelector(selectPrimaryCurrency) ?? 'ETH';
55-
const ticker = useSelector(selectTicker);
5652
const quotes = useSelector(selectBridgeQuotes);
5753
const bridgeFeatureFlags = useSelector(selectBridgeFeatureFlags);
5854
const isSolanaSwap = useSelector(selectIsSolanaSwap);
@@ -123,20 +119,12 @@ export const useBridgeQuoteData = ({
123119
return '-';
124120
}
125121

126-
const networkFeeFormatter = getIntlNumberFormatter(locale, {
127-
maximumFractionDigits: 6,
128-
});
129-
const formattedAmount = `${networkFeeFormatter.format(
130-
Number(amount),
131-
)} ${ticker}`;
132122
const formattedValueInCurrency = fiatFormatter(
133123
new BigNumber(valueInCurrency),
134124
);
135125

136-
return primaryCurrency === 'ETH'
137-
? formattedAmount
138-
: formattedValueInCurrency;
139-
}, [activeQuote, locale, ticker, fiatFormatter, primaryCurrency]);
126+
return formattedValueInCurrency;
127+
}, [activeQuote, fiatFormatter]);
140128

141129
const formattedQuoteData = useMemo(() => {
142130
if (!activeQuote) return undefined;

0 commit comments

Comments
 (0)