-
-
Notifications
You must be signed in to change notification settings - Fork 281
feat!: Use RemoteFeatureFlagController for isRpcFailoverEnabled
#9013
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.
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
Changes from all commits
ea55ac9
338b64b
49b669a
59d9374
ca1f7a1
5ffe1c5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,10 @@ import { | |
| import type { PollingBlockTrackerOptions } from '@metamask/eth-block-tracker'; | ||
| import EthQuery from '@metamask/eth-query'; | ||
| import type { Messenger } from '@metamask/messenger'; | ||
| import { | ||
| RemoteFeatureFlagControllerGetStateAction, | ||
| RemoteFeatureFlagControllerStateChangeEvent, | ||
| } from '@metamask/remote-feature-flag-controller'; | ||
| import { errorCodes } from '@metamask/rpc-errors'; | ||
| import { | ||
| createEventEmitterProxy, | ||
|
|
@@ -55,6 +59,7 @@ import type { | |
| NetworkControllerMethodActions, | ||
| } from './NetworkController-method-action-types'; | ||
| import type { RpcServiceOptionsWithDefaults } from './rpc-service/rpc-service'; | ||
| import { getIsRpcFailoverEnabled } from './selectors'; | ||
| import { NetworkClientType } from './types'; | ||
| import type { | ||
| BlockTracker, | ||
|
|
@@ -663,7 +668,7 @@ export type NetworkControllerEvents = | |
| /** | ||
| * All events that {@link NetworkController} calls internally. | ||
| */ | ||
| type AllowedEvents = never; | ||
| type AllowedEvents = RemoteFeatureFlagControllerStateChangeEvent; | ||
|
|
||
| const MESSENGER_EXPOSED_METHODS = [ | ||
| 'addNetwork', | ||
|
|
@@ -714,7 +719,9 @@ export type NetworkControllerGetNetworkConfigurationByNetworkClientId = | |
| /** | ||
| * All actions that {@link NetworkController} calls internally. | ||
| */ | ||
| type AllowedActions = ConnectivityControllerGetStateAction; | ||
| type AllowedActions = | ||
| | ConnectivityControllerGetStateAction | ||
| | RemoteFeatureFlagControllerGetStateAction; | ||
|
|
||
| export type NetworkControllerMessenger = Messenger< | ||
| typeof controllerName, | ||
|
|
@@ -767,11 +774,6 @@ export type NetworkControllerOptions = { | |
| * An array of Hex Chain IDs representing the additional networks to be included as default. | ||
| */ | ||
| additionalDefaultNetworks?: AdditionalDefaultNetwork[]; | ||
| /** | ||
| * Whether or not requests sent to unavailable RPC endpoints should be | ||
| * automatically diverted to configured failover RPC endpoints. | ||
| */ | ||
| isRpcFailoverEnabled?: boolean; | ||
| }; | ||
|
|
||
| /** | ||
|
|
@@ -1295,10 +1297,7 @@ export class NetworkController extends BaseController< | |
| NetworkConfiguration | ||
| >; | ||
|
|
||
| #isRpcFailoverEnabled: Exclude< | ||
| NetworkControllerOptions['isRpcFailoverEnabled'], | ||
| undefined | ||
| >; | ||
| #isRpcFailoverEnabled = false; | ||
|
|
||
| /** | ||
| * Constructs a NetworkController. | ||
|
|
@@ -1314,7 +1313,6 @@ export class NetworkController extends BaseController< | |
| getRpcServiceOptions, | ||
| getBlockTrackerOptions, | ||
| additionalDefaultNetworks, | ||
| isRpcFailoverEnabled = false, | ||
| } = options; | ||
| const initialState = { | ||
| ...getDefaultNetworkControllerState(additionalDefaultNetworks), | ||
|
|
@@ -1357,7 +1355,6 @@ export class NetworkController extends BaseController< | |
| this.#log = log; | ||
| this.#getRpcServiceOptions = getRpcServiceOptions; | ||
| this.#getBlockTrackerOptions = getBlockTrackerOptions; | ||
| this.#isRpcFailoverEnabled = isRpcFailoverEnabled; | ||
|
|
||
| this.#previouslySelectedNetworkClientId = | ||
| this.state.selectedNetworkClientId; | ||
|
|
@@ -1395,6 +1392,15 @@ export class NetworkController extends BaseController< | |
| }); | ||
| }, | ||
| ); | ||
|
|
||
| this.messenger.subscribe( | ||
| // eslint-disable-next-line no-restricted-syntax | ||
| 'RemoteFeatureFlagController:stateChange', | ||
| (isRpcFailoverEnabled) => { | ||
| this.#updateRpcFailoverEnabled(isRpcFailoverEnabled); | ||
| }, | ||
| getIsRpcFailoverEnabled, | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -1626,6 +1632,14 @@ export class NetworkController extends BaseController< | |
| await this.lookupNetwork(); | ||
| } | ||
|
|
||
| /** | ||
| * Initialize the NetworkController, updating the RPC failover feature flag. | ||
| */ | ||
| init(): void { | ||
| const state = this.messenger.call('RemoteFeatureFlagController:getState'); | ||
| this.#updateRpcFailoverEnabled(getIsRpcFailoverEnabled(state)); | ||
|
Mrtenz marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Init eagerly builds all clientsMedium Severity
Reviewed by Cursor Bugbot for commit 5ffe1c5. Configure here.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is done at init time anyways via |
||
| } | ||
|
|
||
| /** | ||
| * Creates proxies for accessing the global network client and its block | ||
| * tracker. You must call this method in order to use | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| import { RemoteFeatureFlagControllerState } from '@metamask/remote-feature-flag-controller'; | ||
|
|
||
| export function getIsRpcFailoverEnabled( | ||
| state: RemoteFeatureFlagControllerState, | ||
| ): boolean { | ||
| const walletFrameworkRpcFailoverEnabled = state.remoteFeatureFlags | ||
| .walletFrameworkRpcFailoverEnabled as boolean | undefined; | ||
| return walletFrameworkRpcFailoverEnabled ?? false; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1131,6 +1131,23 @@ describe('NetworkController', () => { | |
| describe('disableRpcFailover', () => { | ||
| describe('if the controller was initialized with isRpcFailoverEnabled = true', () => { | ||
| it('calls disableRpcFailover on only the network clients whose RPC endpoints have configured failover URLs', async () => { | ||
| const originalCreateAutoManagedNetworkClient = | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This spy needed to be applied earlier to ensure it caught the first call to |
||
| createAutoManagedNetworkClientModule.createAutoManagedNetworkClient; | ||
| const autoManagedNetworkClients: AutoManagedNetworkClient<NetworkClientConfiguration>[] = | ||
| []; | ||
| jest | ||
| .spyOn( | ||
| createAutoManagedNetworkClientModule, | ||
| 'createAutoManagedNetworkClient', | ||
| ) | ||
| .mockImplementation((...args) => { | ||
| const autoManagedNetworkClient = | ||
| originalCreateAutoManagedNetworkClient(...args); | ||
| jest.spyOn(autoManagedNetworkClient, 'disableRpcFailover'); | ||
| autoManagedNetworkClients.push(autoManagedNetworkClient); | ||
| return autoManagedNetworkClient; | ||
| }); | ||
|
|
||
| await withController( | ||
| { | ||
| isRpcFailoverEnabled: true, | ||
|
|
@@ -1171,23 +1188,6 @@ describe('NetworkController', () => { | |
| }, | ||
| }, | ||
| async ({ controller }) => { | ||
| const originalCreateAutoManagedNetworkClient = | ||
| createAutoManagedNetworkClientModule.createAutoManagedNetworkClient; | ||
| const autoManagedNetworkClients: AutoManagedNetworkClient<NetworkClientConfiguration>[] = | ||
| []; | ||
| jest | ||
| .spyOn( | ||
| createAutoManagedNetworkClientModule, | ||
| 'createAutoManagedNetworkClient', | ||
| ) | ||
| .mockImplementation((...args) => { | ||
| const autoManagedNetworkClient = | ||
| originalCreateAutoManagedNetworkClient(...args); | ||
| jest.spyOn(autoManagedNetworkClient, 'disableRpcFailover'); | ||
| autoManagedNetworkClients.push(autoManagedNetworkClient); | ||
| return autoManagedNetworkClient; | ||
| }); | ||
|
|
||
| controller.disableRpcFailover(); | ||
|
|
||
| expect(autoManagedNetworkClients).toHaveLength(3); | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the restricted syntax here? 🤔
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a special rule for
stateChanged/stateChange. But not many controllers have exported types for it yet, so I can't really use the new format here.