Skip to content

Commit 1674646

Browse files
micaelaemicaelae
authored
fix: remove all selectedNetworkClientid references in bridge-controller (#6996)
## Explanation <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Removes all selectedNetworkClientId references, deriving network clients via srcChainId, refactors metrics request params, and updates tests/snapshots accordingly. > > - **Bridge Controller**: > - Remove selected network usage: delete `#getSelectedNetworkClientId`/`#getSelectedNetworkClient`; resolve clients via `NetworkController:findNetworkClientIdByChainId` in `#getNetworkClientByChainId`. > - Use srcChainId-derived client for balance checks and provider config; treat non‑EVM src chains accordingly. > - Simplify polling input: drop `networkClientId` from `BridgePollingInput` and `startPolling` calls. > - **Metrics**: > - Refactor `getRequestParams` to take only `quoteRequest` (no external srcChainId param) with ETH fallback; update all event builders to use new signature. > - **Types**: > - Remove `NetworkController:getState` from allowed actions and `NetworkClientId` import/usages. > - **Tests/Snapshots**: > - Update mocks and expectations to remove `selectedNetworkClientId`; adjust call counts and snapshots; ensure network lookup by chainId. > - **Changelog**: > - Add Fixed note documenting removal of `selectedNetworkClientId` usages. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 131d042. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent c4790ab commit 1674646

File tree

8 files changed

+62
-119
lines changed

8 files changed

+62
-119
lines changed

‎packages/bridge-controller/CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Removes all selectedNetworkClientId usages by finding network clients via srcChainId ([#6996](https://github.com/MetaMask/core/pull/6996))
13+
1014
## [56.0.2]
1115

1216
### Fixed

‎packages/bridge-controller/src/__snapshots__/bridge-controller.test.ts.snap‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -368,13 +368,6 @@ Array [
368368

369369
exports[`BridgeController trackUnifiedSwapBridgeEvent client-side calls should track the QuotesReceived event 1`] = `
370370
Array [
371-
Array [
372-
"NetworkController:getState",
373-
],
374-
Array [
375-
"NetworkController:getNetworkClientById",
376-
"selectedNetworkClientId",
377-
],
378371
Array [
379372
"AccountsController:getAccountByAddress",
380373
"0x123",

‎packages/bridge-controller/src/bridge-controller.sse.test.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ describe('BridgeController SSE', function () {
9292
messengerMock.call.mockReturnValue({
9393
address: '0x123',
9494
provider: jest.fn(),
95-
selectedNetworkClientId: 'selectedNetworkClientId',
9695
currencyRates: {},
9796
marketData: {},
9897
conversionRates: {},
@@ -151,7 +150,6 @@ describe('BridgeController SSE', function () {
151150
expect(startPollingSpy).toHaveBeenCalledTimes(1);
152151
expect(hasSufficientBalanceSpy).toHaveBeenCalledTimes(1);
153152
expect(startPollingSpy).toHaveBeenCalledWith({
154-
networkClientId: 'selectedNetworkClientId',
155153
updatedQuoteRequest: {
156154
...quoteRequest,
157155
insufficientBal: false,
@@ -714,7 +712,6 @@ describe('BridgeController SSE', function () {
714712
expect(startPollingSpy).toHaveBeenCalledTimes(1);
715713
expect(hasSufficientBalanceSpy).toHaveBeenCalledTimes(1);
716714
expect(startPollingSpy).toHaveBeenCalledWith({
717-
networkClientId: 'selectedNetworkClientId',
718715
updatedQuoteRequest: {
719716
...quoteRequest,
720717
insufficientBal: false,

‎packages/bridge-controller/src/bridge-controller.test.ts‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,6 @@ describe('BridgeController', function () {
318318
messengerMock.call.mockReturnValue({
319319
address: '0x123',
320320
provider: jest.fn(),
321-
selectedNetworkClientId: 'selectedNetworkClientId',
322321
currencyRates: {},
323322
marketData: {},
324323
conversionRates: {},
@@ -363,7 +362,6 @@ describe('BridgeController', function () {
363362
expect(startPollingSpy).toHaveBeenCalledTimes(1);
364363
expect(hasSufficientBalanceSpy).toHaveBeenCalledTimes(1);
365364
expect(startPollingSpy).toHaveBeenCalledWith({
366-
networkClientId: 'selectedNetworkClientId',
367365
updatedQuoteRequest: {
368366
...quoteRequest,
369367
insufficientBal: false,
@@ -398,7 +396,6 @@ describe('BridgeController', function () {
398396
messengerMock.call.mockReturnValue({
399397
address: '0x123',
400398
provider: jest.fn(),
401-
selectedNetworkClientId: 'selectedNetworkClientId',
402399
currencyRates: {},
403400
marketData: {},
404401
conversionRates: {},
@@ -485,7 +482,6 @@ describe('BridgeController', function () {
485482
expect(startPollingSpy).toHaveBeenCalledTimes(1);
486483
expect(hasSufficientBalanceSpy).toHaveBeenCalledTimes(1);
487484
expect(startPollingSpy).toHaveBeenCalledWith({
488-
networkClientId: 'selectedNetworkClientId',
489485
updatedQuoteRequest: {
490486
...quoteRequest,
491487
insufficientBal: false,
@@ -720,7 +716,6 @@ describe('BridgeController', function () {
720716
}
721717
return {
722718
provider: jest.fn() as never,
723-
selectedNetworkClientId: 'selectedNetworkClientId',
724719
} as never;
725720
},
726721
);
@@ -932,7 +927,6 @@ describe('BridgeController', function () {
932927
messengerMock.call.mockReturnValue({
933928
address: '0x123',
934929
provider: jest.fn(),
935-
selectedNetworkClientId: 'selectedNetworkClientId',
936930
currentCurrency: 'usd',
937931
currencyRates: {},
938932
marketData: {},
@@ -990,7 +984,6 @@ describe('BridgeController', function () {
990984
expect(startPollingSpy).toHaveBeenCalledTimes(1);
991985
expect(hasSufficientBalanceSpy).toHaveBeenCalledTimes(1);
992986
expect(startPollingSpy).toHaveBeenCalledWith({
993-
networkClientId: 'selectedNetworkClientId',
994987
updatedQuoteRequest: {
995988
...quoteRequest,
996989
insufficientBal: true,
@@ -1137,7 +1130,6 @@ describe('BridgeController', function () {
11371130
}
11381131
return {
11391132
provider: jest.fn() as never,
1140-
selectedNetworkClientId: 'selectedNetworkClientId',
11411133
} as never;
11421134
},
11431135
);
@@ -1190,7 +1182,6 @@ describe('BridgeController', function () {
11901182
expect(startPollingSpy).toHaveBeenCalledTimes(1);
11911183
expect(hasSufficientBalanceSpy).not.toHaveBeenCalled();
11921184
expect(startPollingSpy).toHaveBeenCalledWith({
1193-
networkClientId: 'selectedNetworkClientId',
11941185
updatedQuoteRequest: {
11951186
...quoteRequest,
11961187
insufficientBal: true,
@@ -1448,7 +1439,6 @@ describe('BridgeController', function () {
14481439
messengerMock.call.mockReturnValue({
14491440
address: '0x123',
14501441
provider: jest.fn(),
1451-
selectedNetworkClientId: 'selectedNetworkClientId',
14521442
} as never);
14531443

14541444
for (const [index, quote] of quoteResponse.entries()) {
@@ -1506,7 +1496,6 @@ describe('BridgeController', function () {
15061496
expect(startPollingSpy).toHaveBeenCalledTimes(1);
15071497
expect(hasSufficientBalanceSpy).toHaveBeenCalledTimes(1);
15081498
expect(startPollingSpy).toHaveBeenCalledWith({
1509-
networkClientId: 'selectedNetworkClientId',
15101499
updatedQuoteRequest: {
15111500
...quoteRequest,
15121501
insufficientBal: true,
@@ -1717,7 +1706,6 @@ describe('BridgeController', function () {
17171706
messengerMock.call.mockReturnValue({
17181707
address: '0x123',
17191708
provider: jest.fn(),
1720-
selectedNetworkClientId: 'selectedNetworkClientId',
17211709
currencyRates: {},
17221710
marketData: {},
17231711
conversionRates: {},
@@ -1905,7 +1893,6 @@ describe('BridgeController', function () {
19051893
}
19061894
return {
19071895
provider: jest.fn() as never,
1908-
selectedNetworkClientId: 'selectedNetworkClientId',
19091896
} as never;
19101897
},
19111898
);
@@ -2067,7 +2054,6 @@ describe('BridgeController', function () {
20672054

20682055
return {
20692056
provider: jest.fn() as never,
2070-
selectedNetworkClientId: 'selectedNetworkClientId',
20712057
} as never;
20722058
},
20732059
);
@@ -2137,7 +2123,6 @@ describe('BridgeController', function () {
21372123
(): ReturnType<BridgeControllerMessenger['call']> => {
21382124
return {
21392125
provider: jest.fn() as never,
2140-
selectedNetworkClientId: 'selectedNetworkClientId',
21412126
rpcUrl: 'https://mainnet.infura.io/v3/123',
21422127
configuration: {
21432128
chainId: 'eip155:1',
@@ -2149,7 +2134,6 @@ describe('BridgeController', function () {
21492134
(): ReturnType<BridgeControllerMessenger['call']> => {
21502135
return {
21512136
provider: jest.fn() as never,
2152-
selectedNetworkClientId: 'selectedNetworkClientId',
21532137
rpcUrl: 'https://mainnet.infura.io/v3/123',
21542138
configuration: {
21552139
chainId: 'eip155:1',
@@ -2326,7 +2310,6 @@ describe('BridgeController', function () {
23262310
messengerMock.call.mockImplementation(() => {
23272311
return {
23282312
provider: jest.fn() as never,
2329-
selectedNetworkClientId: 'selectedNetworkClientId',
23302313
rpcUrl: 'https://mainnet.infura.io/v3/123',
23312314
configuration: {
23322315
chainId: 'eip155:1',
@@ -2445,7 +2428,7 @@ describe('BridgeController', function () {
24452428
security_warnings: [],
24462429
},
24472430
);
2448-
expect(messengerMock.call).toHaveBeenCalledTimes(2);
2431+
expect(messengerMock.call).toHaveBeenCalledTimes(0);
24492432
expect(trackMetaMetricsFn).toHaveBeenCalledTimes(1);
24502433

24512434
expect(trackMetaMetricsFn.mock.calls).toMatchSnapshot();
@@ -2559,7 +2542,6 @@ describe('BridgeController', function () {
25592542
}
25602543
return {
25612544
provider: jest.fn() as never,
2562-
selectedNetworkClientId: 'selectedNetworkClientId',
25632545
rpcUrl: 'https://mainnet.infura.io/v3/123',
25642546
configuration: {
25652547
chainId: 'eip155:1',

‎packages/bridge-controller/src/bridge-controller.ts‎

Lines changed: 15 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import type { StateMetadata } from '@metamask/base-controller';
55
import type { TraceCallback } from '@metamask/controller-utils';
66
import type { InternalAccount } from '@metamask/keyring-internal-api';
77
import { abiERC20 } from '@metamask/metamask-eth-abis';
8-
import type { NetworkClientId } from '@metamask/network-controller';
98
import { StaticIntervalPollingController } from '@metamask/polling-controller';
109
import type { TransactionController } from '@metamask/transaction-controller';
1110
import type { CaipAssetType, Hex } from '@metamask/utils';
@@ -70,7 +69,6 @@ import {
7069
import type {
7170
QuoteFetchData,
7271
RequestMetadata,
73-
RequestParams,
7472
RequiredEventContextFromClient,
7573
} from './utils/metrics/types';
7674
import { type CrossChainSwapsEventProperties } from './utils/metrics/types';
@@ -139,13 +137,11 @@ const metadata: StateMetadata<BridgeControllerState> = {
139137
/**
140138
* The input to start polling for the {@link BridgeController}
141139
*
142-
* @param networkClientId - The network client ID of the selected network
143140
* @param updatedQuoteRequest - The updated quote request
144141
* @param context - The context contains properties that can't be populated by the
145142
* controller and need to be provided by the client for analytics
146143
*/
147144
type BridgePollingInput = {
148-
networkClientId: NetworkClientId;
149145
updatedQuoteRequest: GenericQuoteRequest;
150146
context: Pick<
151147
RequiredEventContextFromClient,
@@ -295,10 +291,15 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
295291

296292
if (isValidQuoteRequest(updatedQuoteRequest)) {
297293
this.#quotesFirstFetched = Date.now();
298-
const providerConfig = this.#getSelectedNetworkClient()?.configuration;
294+
const isSrcChainNonEVM = isNonEvmChainId(updatedQuoteRequest.srcChainId);
295+
const providerConfig = isSrcChainNonEVM
296+
? undefined
297+
: this.#getNetworkClientByChainId(
298+
formatChainIdToHex(updatedQuoteRequest.srcChainId),
299+
)?.configuration;
299300

300301
let insufficientBal: boolean | undefined;
301-
if (isNonEvmChainId(updatedQuoteRequest.srcChainId)) {
302+
if (isSrcChainNonEVM) {
302303
// If the source chain is not an EVM network, use value from params
303304
insufficientBal = paramsToUpdate.insufficientBal;
304305
} else if (providerConfig?.rpcUrl?.includes('tenderly')) {
@@ -317,11 +318,9 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
317318
}
318319
}
319320

320-
const networkClientId = this.#getSelectedNetworkClientId();
321321
// Set refresh rate based on the source chain before starting polling
322322
this.setChainIntervalLength();
323323
this.startPolling({
324-
networkClientId,
325324
updatedQuoteRequest: {
326325
...updatedQuoteRequest,
327326
insufficientBal,
@@ -473,7 +472,7 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
473472
quoteRequest: GenericQuoteRequest,
474473
) => {
475474
const srcChainIdInHex = formatChainIdToHex(quoteRequest.srcChainId);
476-
const provider = this.#getSelectedNetworkClient()?.provider;
475+
const provider = this.#getNetworkClientByChainId(srcChainIdInHex)?.provider;
477476
const normalizedSrcTokenAddress = formatAddressToCaipReference(
478477
quoteRequest.srcTokenAddress,
479478
);
@@ -536,7 +535,6 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
536535
};
537536

538537
readonly #fetchBridgeQuotes = async ({
539-
networkClientId: _networkClientId,
540538
updatedQuoteRequest,
541539
context,
542540
}: BridgePollingInput) => {
@@ -763,22 +761,6 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
763761
return selectedAccount;
764762
}
765763

766-
#getSelectedNetworkClientId() {
767-
const { selectedNetworkClientId } = this.messenger.call(
768-
'NetworkController:getState',
769-
);
770-
return selectedNetworkClientId;
771-
}
772-
773-
#getSelectedNetworkClient() {
774-
const selectedNetworkClientId = this.#getSelectedNetworkClientId();
775-
const networkClient = this.messenger.call(
776-
'NetworkController:getNetworkClientById',
777-
selectedNetworkClientId,
778-
);
779-
return networkClient;
780-
}
781-
782764
#getNetworkClientByChainId(chainId: Hex) {
783765
const networkClientId = this.messenger.call(
784766
'NetworkController:findNetworkClientIdByChainId',
@@ -794,17 +776,6 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
794776
return networkClient;
795777
}
796778

797-
readonly #getRequestParams = (): Omit<
798-
RequestParams,
799-
'token_symbol_source' | 'token_symbol_destination'
800-
> => {
801-
const srcChainIdCaip = formatChainIdToCaip(
802-
this.state.quoteRequest.srcChainId ||
803-
this.#getSelectedNetworkClient().configuration.chainId,
804-
);
805-
return getRequestParams(this.state.quoteRequest, srcChainIdCaip);
806-
};
807-
808779
readonly #getRequestMetadata = (): Omit<
809780
RequestMetadata,
810781
| 'stx_enabled'
@@ -847,18 +818,18 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
847818
case UnifiedSwapBridgeEventName.ButtonClicked:
848819
case UnifiedSwapBridgeEventName.PageViewed:
849820
return {
850-
...this.#getRequestParams(),
821+
...getRequestParams(this.state.quoteRequest),
851822
...baseProperties,
852823
};
853824
case UnifiedSwapBridgeEventName.QuotesValidationFailed:
854825
return {
855-
...this.#getRequestParams(),
826+
...getRequestParams(this.state.quoteRequest),
856827
refresh_count: this.state.quotesRefreshCount,
857828
...baseProperties,
858829
};
859830
case UnifiedSwapBridgeEventName.QuotesReceived:
860831
return {
861-
...this.#getRequestParams(),
832+
...getRequestParams(this.state.quoteRequest),
862833
...this.#getRequestMetadata(),
863834
...this.#getQuoteFetchData(),
864835
is_hardware_wallet: isHardwareWallet(
@@ -869,7 +840,7 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
869840
};
870841
case UnifiedSwapBridgeEventName.QuotesRequested:
871842
return {
872-
...this.#getRequestParams(),
843+
...getRequestParams(this.state.quoteRequest),
873844
...this.#getRequestMetadata(),
874845
is_hardware_wallet: isHardwareWallet(
875846
this.#getMultichainSelectedAccount(),
@@ -879,7 +850,7 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
879850
};
880851
case UnifiedSwapBridgeEventName.QuotesError:
881852
return {
882-
...this.#getRequestParams(),
853+
...getRequestParams(this.state.quoteRequest),
883854
...this.#getRequestMetadata(),
884855
is_hardware_wallet: isHardwareWallet(
885856
this.#getMultichainSelectedAccount(),
@@ -892,7 +863,7 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
892863
case UnifiedSwapBridgeEventName.AllQuotesSorted:
893864
case UnifiedSwapBridgeEventName.QuoteSelected:
894865
return {
895-
...this.#getRequestParams(),
866+
...getRequestParams(this.state.quoteRequest),
896867
...this.#getRequestMetadata(),
897868
...this.#getQuoteFetchData(),
898869
is_hardware_wallet: isHardwareWallet(
@@ -904,7 +875,7 @@ export class BridgeController extends StaticIntervalPollingController<BridgePoll
904875
// Populate the properties that the error occurred before the tx was submitted
905876
return {
906877
...baseProperties,
907-
...this.#getRequestParams(),
878+
...getRequestParams(this.state.quoteRequest),
908879
...this.#getRequestMetadata(),
909880
...this.#getQuoteFetchData(),
910881
...propertiesFromClient,

‎packages/bridge-controller/src/types.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import type {
1111
import type { Messenger } from '@metamask/messenger';
1212
import type {
1313
NetworkControllerFindNetworkClientIdByChainIdAction,
14-
NetworkControllerGetStateAction,
1514
NetworkControllerGetNetworkClientByIdAction,
1615
} from '@metamask/network-controller';
1716
import type { RemoteFeatureFlagControllerGetStateAction } from '@metamask/remote-feature-flag-controller';
@@ -371,7 +370,6 @@ export type AllowedActions =
371370
| MultichainAssetsRatesControllerGetStateAction
372371
| HandleSnapRequest
373372
| NetworkControllerFindNetworkClientIdByChainIdAction
374-
| NetworkControllerGetStateAction
375373
| NetworkControllerGetNetworkClientByIdAction
376374
| RemoteFeatureFlagControllerGetStateAction;
377375
export type AllowedEvents = never;

0 commit comments

Comments
 (0)