diff --git a/app/scripts/lib/rpc-method-middleware/handlers/get-provider-state.test.ts b/app/scripts/lib/rpc-method-middleware/handlers/get-provider-state.test.ts index 078bd7866a31..2dfda7728c3a 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/get-provider-state.test.ts +++ b/app/scripts/lib/rpc-method-middleware/handlers/get-provider-state.test.ts @@ -2,9 +2,9 @@ import { PendingJsonRpcResponse } from '@metamask/utils'; import { JsonRpcEngineEndCallback } from '@metamask/json-rpc-engine'; import getProviderState, { GetProviderState, + ProviderStateHandlerRequest, ProviderStateHandlerResult, } from './get-provider-state'; -import { HandlerRequestType } from './types'; describe('getProviderState', () => { let mockEnd: JsonRpcEngineEndCallback; @@ -21,9 +21,11 @@ describe('getProviderState', () => { }); it('should call getProviderState when the handler is invoked', async () => { - const req: HandlerRequestType = { + const req: ProviderStateHandlerRequest = { origin: 'testOrigin', - params: [], + params: { + isInitializingStreamProvider: true, + }, id: '22', jsonrpc: '2.0', method: 'metamask_getProviderState', @@ -44,7 +46,9 @@ describe('getProviderState', () => { getProviderState: mockGetProviderState, }); - expect(mockGetProviderState).toHaveBeenCalledWith(req.origin); + expect(mockGetProviderState).toHaveBeenCalledWith(req.origin, { + isInitializingStreamProvider: true, + }); expect(res.result).toStrictEqual({ chainId: '0x539', isUnlocked: true, diff --git a/app/scripts/lib/rpc-method-middleware/handlers/get-provider-state.ts b/app/scripts/lib/rpc-method-middleware/handlers/get-provider-state.ts index 514f8af6dfa7..407ca43d4734 100644 --- a/app/scripts/lib/rpc-method-middleware/handlers/get-provider-state.ts +++ b/app/scripts/lib/rpc-method-middleware/handlers/get-provider-state.ts @@ -4,14 +4,17 @@ import type { } from '@metamask/json-rpc-engine'; import type { PendingJsonRpcResponse, - JsonRpcParams, Hex, + JsonRpcRequest, } from '@metamask/utils'; import { MESSAGE_TYPE } from '../../../../../shared/constants/app'; -import { - HandlerWrapper, - HandlerRequestType as ProviderStateHandlerRequest, -} from './types'; +import { HandlerWrapper } from './types'; + +export type ProviderStateHandlerRequest = JsonRpcRequest<{ + isInitializingStreamProvider?: boolean; +}> & { + origin: string; +}; /** * @property chainId - The current chain ID. @@ -28,18 +31,18 @@ export type ProviderStateHandlerResult = { export type GetProviderState = ( origin: string, + options?: { isInitializingStreamProvider?: boolean }, ) => Promise; -type GetProviderStateConstraint = - { - implementation: ( - _req: ProviderStateHandlerRequest, - res: PendingJsonRpcResponse, - _next: JsonRpcEngineNextCallback, - end: JsonRpcEngineEndCallback, - { _getProviderState }: Record, - ) => Promise; - } & HandlerWrapper; +type GetProviderStateConstraint = { + implementation: ( + _req: ProviderStateHandlerRequest, + res: PendingJsonRpcResponse, + _next: JsonRpcEngineNextCallback, + end: JsonRpcEngineEndCallback, + { _getProviderState }: Record, + ) => Promise; +} & HandlerWrapper; /** * This RPC method gets background state relevant to the provider. @@ -64,17 +67,16 @@ export default getProviderState; * @param options * @param options.getProviderState - An async function that gets the current provider state. */ -async function getProviderStateHandler< - Params extends JsonRpcParams = JsonRpcParams, ->( - req: ProviderStateHandlerRequest, +async function getProviderStateHandler( + req: ProviderStateHandlerRequest, res: PendingJsonRpcResponse, _next: JsonRpcEngineNextCallback, end: JsonRpcEngineEndCallback, { getProviderState: _getProviderState }: Record, ): Promise { + const isInitializingStreamProvider = req.params?.isInitializingStreamProvider; res.result = { - ...(await _getProviderState(req.origin)), + ...(await _getProviderState(req.origin, { isInitializingStreamProvider })), }; return end(); } diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 37721dcf5d9c..162c6d597cc3 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -3297,6 +3297,7 @@ export default class MetamaskController extends EventEmitter { // setup memStore subscription hooks this.on('update', updatePublicConfigStore); + // Update the store asynchronously, out-of-band updatePublicConfigStore(this.getState()); return publicConfigStore; @@ -3306,10 +3307,18 @@ export default class MetamaskController extends EventEmitter { * Gets relevant state for the provider of an external origin. * * @param {string} origin - The origin to get the provider state for. + * @param {object} [options] - Options. + * @param {boolean} [options.isInitializingStreamProvider] - Whether this method is being used to initialize the StreamProvider (default: false). * @returns {Promise<{ isUnlocked: boolean, networkVersion: string, chainId: string, accounts: string[], extensionId: string | undefined }>} An object with relevant state properties. */ - async getProviderState(origin) { - const providerNetworkState = await this.getProviderNetworkState(origin); + async getProviderState( + origin, + { isInitializingStreamProvider = false } = {}, + ) { + const providerNetworkState = await this.getProviderNetworkState({ + origin, + isInitializingStreamProvider, + }); const metadata = {}; if (isManifestV3) { const { chrome } = globalThis; @@ -3330,10 +3339,15 @@ export default class MetamaskController extends EventEmitter { /** * Retrieves network state information relevant for external providers. * - * @param {string} origin - The origin identifier for which network state is requested (default: 'metamask'). + * @param {object} [args] - The arguments to this function. + * @param {string} [args.origin] - The origin identifier for which network state is requested (default: 'metamask'). + * @param {boolean} [args.isInitializingStreamProvider] - Whether this method is being used to initialize the StreamProvider (default: false). * @returns {object} An object containing important network state properties, including chainId and networkVersion. */ - async getProviderNetworkState(origin = METAMASK_DOMAIN) { + async getProviderNetworkState({ + origin = METAMASK_DOMAIN, + isInitializingStreamProvider = false, + } = {}) { const networkClientId = this.controllerMessenger.call( 'SelectedNetworkController:getNetworkClientIdForDomain', origin, @@ -3349,7 +3363,22 @@ export default class MetamaskController extends EventEmitter { const { completedOnboarding } = this.onboardingController.state; let networkVersion = this.deprecatedNetworkVersions[networkClientId]; - if (networkVersion === undefined && completedOnboarding) { + // We use `metamask_getProviderState` to set the initial state of the + // StreamProvider. The StreamProvider allows the UI to make network requests + // through the background connection, and it must be initialized before we + // can show the UI. However, this creates a problem if the selected network + // is slow or unresponsive, because then the network request will hang and + // thus we will be unable to show the UI. To get around this, we prevent a + // request from occurring during initialization + // (`isInitializingStreamProvider` = true). `metamask_getProviderState` is + // called each time the memState is updated, so eventually we _will_ make + // this request (`isInitializingStreamProvider` = false), and if the network + // recovers, the network version will be properly retrieved at that time. + if ( + networkVersion === undefined && + completedOnboarding && + !isInitializingStreamProvider + ) { try { const result = await networkClient.provider.request({ method: 'net_version', @@ -8980,7 +9009,7 @@ export default class MetamaskController extends EventEmitter { this.notifyAllConnections( async (origin) => ({ method: NOTIFICATION_NAMES.chainChanged, - params: await this.getProviderNetworkState(origin), + params: await this.getProviderNetworkState({ origin }), }), API_TYPE.EIP1193, ); @@ -8989,7 +9018,7 @@ export default class MetamaskController extends EventEmitter { async _notifyChainChangeForConnection(connection, origin) { this.notifyConnection(connection, { method: NOTIFICATION_NAMES.chainChanged, - params: await this.getProviderNetworkState(origin), + params: await this.getProviderNetworkState({ origin }), }); } diff --git a/package.json b/package.json index 222c45946e40..002df4ad8776 100644 --- a/package.json +++ b/package.json @@ -333,7 +333,7 @@ "@metamask/ppom-validator": "0.36.0", "@metamask/preinstalled-example-snap": "^0.7.1", "@metamask/profile-sync-controller": "^23.0.0", - "@metamask/providers": "^22.1.0", + "@metamask/providers": "^22.1.1", "@metamask/rate-limit-controller": "^6.0.3", "@metamask/remote-feature-flag-controller": "^1.6.0", "@metamask/rpc-errors": "^7.0.0", diff --git a/test/e2e/tests/network/slow-network.spec.ts b/test/e2e/tests/network/slow-network.spec.ts new file mode 100644 index 000000000000..ba9f9f80d347 --- /dev/null +++ b/test/e2e/tests/network/slow-network.spec.ts @@ -0,0 +1,75 @@ +import { Suite } from 'mocha'; +import { Mockttp } from 'mockttp'; +import { NetworkStatus } from '@metamask/network-controller'; +import { withFixtures } from '../../helpers'; +import FixtureBuilder from '../../fixture-builder'; +import LoginPage from '../../page-objects/pages/login-page'; + +describe('Slow network', function (this: Suite) { + it('does not show the spinner while attempting to determine the network status', async function () { + await withFixtures( + { + fixtures: new FixtureBuilder({ inputChainId: '0x539' }) + .withNetworkController({ + networkConfigurations: { + 'AAAA-AAAA-AAAA-AAAA': { + id: 'AAAA-AAAA-AAAA-AAAA', + chainId: '0x9999', + nickname: 'A Custom Network', + rpcUrl: 'https://customnetwork.test', + ticker: 'ETH', + blockExplorerUrl: undefined, + }, + }, + networksMetadata: { + 'AAAA-AAAA-AAAA-AAAA': { + EIPS: {}, + status: NetworkStatus.Unknown, + }, + }, + selectedNetworkClientId: 'AAAA-AAAA-AAAA-AAAA', + providerConfig: { + id: 'AAAA-AAAA-AAAA-AAAA', + }, + }) + .withEnabledNetworks({ + eip155: { + '0x9999': true, + }, + }) + .build(), + localNodeOptions: 'none', + title: this.test?.fullTitle(), + testSpecificMock: async (mockServer: Mockttp) => { + // Cause requests to the network to be pending indefinitely + return await mockServer + .forPost('https://customnetwork.test') + .thenPassThrough({ + beforeRequest: async () => { + // Wait long enough to cause a timeout + await new Promise((resolve) => { + setTimeout(() => { + resolve(); + }, 10000); + }); + return undefined; + }, + }); + }, + }, + async ({ driver }) => { + await driver.navigate(); + const loginPage = new LoginPage(driver); + await loginPage.checkPageIsLoaded(); + await loginPage.loginToHomepage(); + + // Ensure that the connection spinner shows up. + // This wouldn't happen if we waited for the net_version (mocked above) + // to fully resolve. + await driver.waitForSelector({ + text: 'Connecting to A Custom Network', + }); + }, + ); + }); +}); diff --git a/yarn.lock b/yarn.lock index 2e46f0afe6e8..8183ee44570c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7028,9 +7028,9 @@ __metadata: languageName: node linkType: hard -"@metamask/providers@npm:^22.1.0": - version: 22.1.0 - resolution: "@metamask/providers@npm:22.1.0" +"@metamask/providers@npm:^22.1.0, @metamask/providers@npm:^22.1.1": + version: 22.1.1 + resolution: "@metamask/providers@npm:22.1.1" dependencies: "@metamask/json-rpc-engine": "npm:^10.0.2" "@metamask/json-rpc-middleware-stream": "npm:^8.0.6" @@ -7045,7 +7045,7 @@ __metadata: readable-stream: "npm:^3.6.2" peerDependencies: webextension-polyfill: ^0.10.0 || ^0.11.0 || ^0.12.0 - checksum: 10/d6dc969296e3d478a904228f27adae3b6dcbfdbf49eb6d571c9d73d7506df3c6e3bf3c3464f1e69e4c0acb6f6072d12d1c8182348e626ca1572f4f22f9c585e6 + checksum: 10/50194c608fb308cee268c6eefb8c8d439a9d07aa41d07cb5ddcd7e706819aea7e746150f32688fe06d20309b49346440855b5d56aacf286b343da6fcb217cc12 languageName: node linkType: hard @@ -31815,7 +31815,7 @@ __metadata: "@metamask/preferences-controller": "npm:^17.0.0" "@metamask/preinstalled-example-snap": "npm:^0.7.1" "@metamask/profile-sync-controller": "npm:^23.0.0" - "@metamask/providers": "npm:^22.1.0" + "@metamask/providers": "npm:^22.1.1" "@metamask/rate-limit-controller": "npm:^6.0.3" "@metamask/remote-feature-flag-controller": "npm:^1.6.0" "@metamask/rpc-errors": "npm:^7.0.0"