diff --git a/jest.config.js b/jest.config.js index 5a7a5578..683ea221 100644 --- a/jest.config.js +++ b/jest.config.js @@ -45,10 +45,10 @@ const baseConfig = { // An object that configures minimum threshold enforcement for coverage results coverageThreshold: { global: { - branches: 66.79, + branches: 68.23, functions: 68.69, - lines: 68.35, - statements: 68.38, + lines: 68.68, + statements: 68.71, }, }, diff --git a/src/BaseProvider.ts b/src/BaseProvider.ts index 5dfcda5e..9d135a74 100644 --- a/src/BaseProvider.ts +++ b/src/BaseProvider.ts @@ -238,6 +238,7 @@ export abstract class BaseProvider extends SafeEventEmitter { * @param initialState.chainId - The chain ID. * @param initialState.isUnlocked - Whether the user has unlocked MetaMask. * @param initialState.networkVersion - The network version. + * @param initialState.isConnected - Whether the network is available. * @fires BaseProvider#_initialized - If `initialState` is defined. * @fires BaseProvider#connect - If `initialState` is defined. */ @@ -246,17 +247,19 @@ export abstract class BaseProvider extends SafeEventEmitter { chainId: string; isUnlocked: boolean; networkVersion?: string; + isConnected?: boolean; }) { if (this._state.initialized) { throw new Error('Provider already initialized.'); } if (initialState) { - const { accounts, chainId, isUnlocked, networkVersion } = initialState; + const { accounts, chainId, isUnlocked, networkVersion, isConnected } = + initialState; // EIP-1193 connect - this._handleConnect(chainId); - this._handleChainChanged({ chainId, networkVersion }); + this._handleConnect({ chainId, isConnected }); + this._handleChainChanged({ chainId, networkVersion, isConnected }); this._handleUnlockStateChanged({ accounts, isUnlocked }); this._handleAccountsChanged(accounts); } @@ -311,11 +314,19 @@ export abstract class BaseProvider extends SafeEventEmitter { * When the provider becomes connected, updates internal state and emits * required events. Idempotent. * - * @param chainId - The ID of the newly connected chain. + * @param networkInfo - The options object. + * @param networkInfo.chainId - The ID of the newly connected chain. + * @param networkInfo.isConnected - Whether the network is available. * @fires MetaMaskInpageProvider#connect */ - protected _handleConnect(chainId: string) { - if (!this._state.isConnected) { + protected _handleConnect({ + chainId, + isConnected, + }: { + chainId: string; + isConnected?: boolean | undefined; + }) { + if (!this._state.isConnected && isConnected) { this._state.isConnected = true; this.emit('connect', { chainId }); this._log.debug(messages.info.connected(chainId)); @@ -375,18 +386,24 @@ export abstract class BaseProvider extends SafeEventEmitter { * @fires BaseProvider#chainChanged * @param networkInfo - An object with network info. * @param networkInfo.chainId - The latest chain ID. + * @param networkInfo.isConnected - Whether the network is available. */ protected _handleChainChanged({ chainId, + isConnected, }: - | { chainId?: string | undefined; networkVersion?: string | undefined } + | { + chainId?: string; + networkVersion?: string | undefined; + isConnected?: boolean | undefined; + } | undefined = {}) { if (!isValidChainId(chainId)) { this._log.error(messages.errors.invalidNetworkParams(), { chainId }); return; } - this._handleConnect(chainId); + this._handleConnect({ chainId, isConnected }); if (chainId !== this.#chainId) { this.#chainId = chainId; diff --git a/src/MetaMaskInpageProvider.test.ts b/src/MetaMaskInpageProvider.test.ts index e6d9cdce..623ffec4 100644 --- a/src/MetaMaskInpageProvider.test.ts +++ b/src/MetaMaskInpageProvider.test.ts @@ -1,4 +1,5 @@ import ObjectMultiplex from '@metamask/object-multiplex'; +import { JsonRpcError } from '@metamask/rpc-errors'; import type { JsonRpcRequest } from '@metamask/utils'; import { pipeline } from 'readable-stream'; @@ -51,6 +52,7 @@ async function getInitializedProvider({ chainId = '0x0', isUnlocked = true, networkVersion = '0', + isConnected = true, } = {}, onMethodCalled = [], }: { @@ -81,6 +83,7 @@ async function getInitializedProvider({ chainId, isUnlocked, networkVersion, + isConnected, }, }), ); @@ -796,42 +799,119 @@ describe('MetaMaskInpageProvider: RPC', () => { connectionStream.notify(MetaMaskInpageProviderStreamName, { jsonrpc: '2.0', method: 'metamask_chainChanged', - // A "loading" networkVersion indicates the network is changing. - // Although the chainId is different, chainChanged should not be - // emitted in this case. - params: { chainId: '0x1', networkVersion: 'loading' }, + params: { chainId: '0x1', networkVersion: '1', isConnected: false }, }); }); - // Only once, for "disconnect". - expect(emitSpy).toHaveBeenCalledTimes(1); + expect(emitSpy).toHaveBeenCalledTimes(3); + expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x1'); + expect(emitSpy).toHaveBeenCalledWith('networkChanged', '1'); + expect(emitSpy).toHaveBeenCalledWith( + 'disconnect', + new JsonRpcError(1013, messages.errors.disconnected()), + ); emitSpy.mockClear(); // Clear the mock to avoid keeping a count. expect(provider.isConnected()).toBe(false); - // These should be unchanged. + expect(provider.chainId).toBe('0x1'); + expect(provider.networkVersion).toBe('1'); + + await new Promise((resolve) => { + provider.once('chainChanged', (newChainId) => { + expect(newChainId).toBe('0x2'); + resolve(); + }); + + connectionStream.notify(MetaMaskInpageProviderStreamName, { + jsonrpc: '2.0', + method: 'metamask_chainChanged', + params: { chainId: '0x2', networkVersion: '2', isConnected: false }, + }); + }); + + expect(emitSpy).toHaveBeenCalledTimes(2); + expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x2'); + expect(emitSpy).toHaveBeenCalledWith('networkChanged', '2'); + emitSpy.mockClear(); // Clear the mock to avoid keeping a count. + + expect(provider.isConnected()).toBe(false); + expect(provider.chainId).toBe('0x2'); + expect(provider.networkVersion).toBe('2'); + + await new Promise((resolve) => { + provider.once('connect', (message) => { + expect(message).toStrictEqual({ chainId: '0x2' }); + resolve(); + }); + + connectionStream.notify(MetaMaskInpageProviderStreamName, { + jsonrpc: '2.0', + method: 'metamask_chainChanged', + params: { chainId: '0x2', networkVersion: '2', isConnected: true }, + }); + }); + + expect(emitSpy).toHaveBeenCalledTimes(1); + expect(emitSpy).toHaveBeenCalledWith('connect', { chainId: '0x2' }); + + expect(provider.isConnected()).toBe(true); + expect(provider.chainId).toBe('0x2'); + expect(provider.networkVersion).toBe('2'); + }); + + it('handles chain changes when networkVersion is "loading" by interpreting it as null', async () => { + const { provider, connectionStream } = await getInitializedProvider(); + + // We check this mostly for the readability of this test. + expect(provider.isConnected()).toBe(true); expect(provider.chainId).toBe('0x0'); expect(provider.networkVersion).toBe('0'); + const emitSpy = jest.spyOn(provider, 'emit'); + await new Promise((resolve) => { - provider.once('chainChanged', (newChainId) => { - expect(newChainId).toBe('0x1'); + provider.once('networkChanged', (newNetworkId) => { + expect(newNetworkId).toBeNull(); resolve(); }); connectionStream.notify(MetaMaskInpageProviderStreamName, { jsonrpc: '2.0', method: 'metamask_chainChanged', - params: { chainId: '0x1', networkVersion: '1' }, + params: { + chainId: '0x0', + networkVersion: 'loading', + isConnected: true, + }, }); }); - expect(emitSpy).toHaveBeenCalledTimes(3); - expect(emitSpy).toHaveBeenNthCalledWith(1, 'connect', { chainId: '0x1' }); - expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x1'); + expect(emitSpy).toHaveBeenCalledTimes(1); + expect(emitSpy).toHaveBeenCalledWith('networkChanged', null); + emitSpy.mockClear(); // Clear the mock to avoid keeping a count. + + expect(provider.isConnected()).toBe(true); + expect(provider.chainId).toBe('0x0'); + expect(provider.networkVersion).toBeNull(); + + await new Promise((resolve) => { + provider.once('networkChanged', (newNetworkId) => { + expect(newNetworkId).toBe('1'); + resolve(); + }); + + connectionStream.notify(MetaMaskInpageProviderStreamName, { + jsonrpc: '2.0', + method: 'metamask_chainChanged', + params: { chainId: '0x0', networkVersion: '1', isConnected: true }, + }); + }); + + expect(emitSpy).toHaveBeenCalledTimes(1); expect(emitSpy).toHaveBeenCalledWith('networkChanged', '1'); expect(provider.isConnected()).toBe(true); - expect(provider.chainId).toBe('0x1'); + expect(provider.chainId).toBe('0x0'); expect(provider.networkVersion).toBe('1'); }); }); @@ -1036,6 +1116,7 @@ describe('MetaMaskInpageProvider: Miscellanea', () => { chainId: '0x0', isUnlocked: true, networkVersion: '0', + isConnected: true, }; }); diff --git a/src/MetaMaskInpageProvider.ts b/src/MetaMaskInpageProvider.ts index b8156369..e74ddf48 100644 --- a/src/MetaMaskInpageProvider.ts +++ b/src/MetaMaskInpageProvider.ts @@ -450,25 +450,41 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { } /** - * Upon receipt of a new chainId and networkVersion, emits corresponding - * events and sets relevant public state. Does nothing if neither the chainId - * nor the networkVersion are different from existing values. + * Upon receipt of a new chainId, networkVersion, and isConnected value + * emits corresponding events and sets relevant public state. We interpret + * a `networkVersion` with the value of `loading` to be null. The `isConnected` + * value determines if a `connect` or recoverable `disconnect` has occurred. + * Child classes that use the `networkVersion` for other purposes must implement + * additional handling therefore. * * @fires MetamaskInpageProvider#networkChanged * @param networkInfo - An object with network info. * @param networkInfo.chainId - The latest chain ID. * @param networkInfo.networkVersion - The latest network ID. + * @param networkInfo.isConnected - Whether the network is available. */ protected _handleChainChanged({ chainId, networkVersion, - }: { chainId?: string; networkVersion?: string } = {}) { - // This will validate the params and disconnect the provider if the - // networkVersion is 'loading'. - super._handleChainChanged({ chainId, networkVersion }); - - if (this._state.isConnected && networkVersion !== this.#networkVersion) { - this.#networkVersion = networkVersion as string; + isConnected, + }: { + chainId?: string; + networkVersion?: string; + isConnected?: boolean; + } = {}) { + super._handleChainChanged({ chainId, networkVersion, isConnected }); + + // The wallet will send a value of `loading` for `networkVersion` when it intends + // to communicate that this value cannot be resolved and should be intepreted as null. + // The wallet cannot directly send a null value for `networkVersion` because this + // would be a breaking change for existing dapps that use their own embedded MetaMask provider + // that expect this value to always be a integer string or the value 'loading'. + + const targetNetworkVersion = + networkVersion === 'loading' ? null : networkVersion; + + if (targetNetworkVersion !== this.#networkVersion) { + this.#networkVersion = targetNetworkVersion as string; if (this._state.initialized) { this.emit('networkChanged', this.#networkVersion); } diff --git a/src/StreamProvider.test.ts b/src/StreamProvider.test.ts index 60b14b4f..949c0181 100644 --- a/src/StreamProvider.test.ts +++ b/src/StreamProvider.test.ts @@ -1,5 +1,6 @@ import type { JsonRpcMiddleware } from '@metamask/json-rpc-engine'; import ObjectMultiplex from '@metamask/object-multiplex'; +import { JsonRpcError } from '@metamask/rpc-errors'; import type { Json, JsonRpcParams } from '@metamask/utils'; import { pipeline } from 'readable-stream'; @@ -38,6 +39,7 @@ describe('StreamProvider', () => { const chainId = '0x1'; const networkVersion = '1'; const isUnlocked = true; + const isConnected = true; const streamProvider = new StreamProvider(new MockConnectionStream()); @@ -49,6 +51,7 @@ describe('StreamProvider', () => { chainId, isUnlocked, networkVersion, + isConnected, }; }); @@ -63,6 +66,32 @@ describe('StreamProvider', () => { method: 'metamask_getProviderState', }); }); + + it('throws if initialized twice', async () => { + const accounts = ['0xabc']; + const chainId = '0x1'; + const networkVersion = '1'; + const isUnlocked = true; + const isConnected = true; + + const streamProvider = new StreamProvider(new MockConnectionStream()); + + jest.spyOn(streamProvider, 'request').mockImplementation(async () => { + return { + accounts, + chainId, + isUnlocked, + networkVersion, + isConnected, + }; + }); + + await streamProvider.initialize(); + + await expect(async () => streamProvider.initialize()).rejects.toThrow( + new Error('Provider already initialized.'), + ); + }); }); describe('RPC', () => { @@ -384,6 +413,7 @@ describe('StreamProvider', () => { chainId: '0x0', isUnlocked: true, networkVersion: '0', + isConnected: true, }; }); @@ -399,7 +429,7 @@ describe('StreamProvider', () => { mockStream.notify(mockStreamName, { jsonrpc: '2.0', method: 'metamask_chainChanged', - params: { chainId: '0x1', networkVersion: '0x1' }, + params: { chainId: '0x1', networkVersion: '0' }, }); }); }); @@ -422,6 +452,7 @@ describe('StreamProvider', () => { chainId: '0x0', isUnlocked: true, networkVersion: '0', + isConnected: true, }; }); @@ -443,44 +474,61 @@ describe('StreamProvider', () => { mockStream.notify(mockStreamName, { jsonrpc: '2.0', method: 'metamask_chainChanged', - // A "loading" networkVersion indicates the network is changing. - // Although the chainId is different, chainChanged should not be - // emitted in this case. - params: { chainId: '0x1', networkVersion: 'loading' }, + params: { chainId: '0x1', networkVersion: '0', isConnected: false }, }); }); - // Only once, for "disconnect". - expect(emitSpy).toHaveBeenCalledTimes(1); + expect(emitSpy).toHaveBeenCalledTimes(2); + expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x1'); + expect(emitSpy).toHaveBeenCalledWith( + 'disconnect', + new JsonRpcError(1013, messages.errors.disconnected()), + ); emitSpy.mockClear(); // Clear the mock to avoid keeping a count. expect(streamProvider.isConnected()).toBe(false); - // These should be unchanged. - expect(streamProvider.chainId).toBe('0x0'); + expect(streamProvider.chainId).toBe('0x1'); await new Promise((resolve) => { streamProvider.once('chainChanged', (newChainId) => { - expect(newChainId).toBe('0x1'); + expect(newChainId).toBe('0x2'); resolve(); }); mockStream.notify(mockStreamName, { jsonrpc: '2.0', method: 'metamask_chainChanged', - // The networkVersion will be ignored here, we're just setting it - // to something other than 'loading'. - params: { chainId: '0x1', networkVersion: '1' }, + params: { chainId: '0x2', networkVersion: '0', isConnected: false }, }); }); - expect(emitSpy).toHaveBeenCalledTimes(2); - expect(emitSpy).toHaveBeenNthCalledWith(1, 'connect', { - chainId: '0x1', + expect(emitSpy).toHaveBeenCalledTimes(1); + expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x2'); + emitSpy.mockClear(); // Clear the mock to avoid keeping a count. + + expect(streamProvider.isConnected()).toBe(false); + expect(streamProvider.chainId).toBe('0x2'); + + await new Promise((resolve) => { + streamProvider.once('connect', (message) => { + expect(message).toStrictEqual({ chainId: '0x2' }); + resolve(); + }); + + mockStream.notify(mockStreamName, { + jsonrpc: '2.0', + method: 'metamask_chainChanged', + params: { chainId: '0x2', networkVersion: '0', isConnected: true }, + }); + }); + + expect(emitSpy).toHaveBeenCalledTimes(1); + expect(emitSpy).toHaveBeenCalledWith('connect', { + chainId: '0x2', }); - expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x1'); expect(streamProvider.isConnected()).toBe(true); - expect(streamProvider.chainId).toBe('0x1'); + expect(streamProvider.chainId).toBe('0x2'); }); }); }); diff --git a/src/StreamProvider.ts b/src/StreamProvider.ts index acc4c942..1b741cf5 100644 --- a/src/StreamProvider.ts +++ b/src/StreamProvider.ts @@ -151,24 +151,23 @@ export abstract class AbstractStreamProvider extends BaseProvider { /** * Upon receipt of a new chainId and networkVersion, emits corresponding - * events and sets relevant public state. This class does not have a - * `networkVersion` property, but we rely on receiving a `networkVersion` - * with the value of `loading` to detect when the network is changing and - * a recoverable `disconnect` even has occurred. Child classes that use the - * `networkVersion` for other purposes must implement additional handling - * therefore. + * events and sets relevant public state. Child classes that use the + * `networkVersion` for other purposes must implement additional handling. * * @fires BaseProvider#chainChanged * @param networkInfo - An object with network info. * @param networkInfo.chainId - The latest chain ID. * @param networkInfo.networkVersion - The latest network ID. + * @param networkInfo.isConnected - Whether the network is available. */ protected _handleChainChanged({ chainId, networkVersion, + isConnected, }: { chainId?: string | undefined; networkVersion?: string | undefined; + isConnected?: boolean | undefined; } = {}) { if (!isValidChainId(chainId) || !isValidNetworkVersion(networkVersion)) { this._log.error(messages.errors.invalidNetworkParams(), { @@ -178,10 +177,10 @@ export abstract class AbstractStreamProvider extends BaseProvider { return; } - if (networkVersion === 'loading') { + super._handleChainChanged({ chainId, isConnected }); + + if (!isConnected) { this._handleDisconnect(true); - } else { - super._handleChainChanged({ chainId }); } } } diff --git a/src/extension-provider/createExternalExtensionProvider.test.ts b/src/extension-provider/createExternalExtensionProvider.test.ts index dff2ee23..267b4fec 100644 --- a/src/extension-provider/createExternalExtensionProvider.test.ts +++ b/src/extension-provider/createExternalExtensionProvider.test.ts @@ -45,6 +45,7 @@ async function getInitializedProvider({ chainId = '0x0', isUnlocked = true, networkVersion = '0', + isConnected = true, } = {}, onMethodCalled = [], }: { @@ -73,6 +74,7 @@ async function getInitializedProvider({ chainId, isUnlocked, networkVersion, + isConnected, }, }), ); diff --git a/src/utils.ts b/src/utils.ts index 8f9cae29..7b5f3540 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -26,6 +26,9 @@ export const UUID_V4_REGEX = export const FQDN_REGEX = /(?=^.{4,253}$)(^((?!-)[a-zA-Z0-9-]{0,62}[a-zA-Z0-9]\.)+[a-zA-Z]{2,63}$)/u; +// https://stackoverflow.com/a/9523559 +const POSITIVE_INTEGER_REGEX = /^(\d*[1-9]\d*|0)$/u; + export const EMITTED_NOTIFICATIONS = Object.freeze([ 'eth_subscription', // per eth-json-rpc-filters/subscriptionManager ]); @@ -104,14 +107,15 @@ export const isValidChainId = (chainId: unknown): chainId is string => /** * Checks whether the given network version is valid, meaning if it is non-empty - * string. + * integer string or the value 'loading'. * * @param networkVersion - The network version to validate. * @returns Whether the given network version is valid. */ export const isValidNetworkVersion = ( networkVersion: unknown, -): networkVersion is string => - Boolean(networkVersion) && typeof networkVersion === 'string'; +): networkVersion is string | null => + typeof networkVersion === 'string' && + (POSITIVE_INTEGER_REGEX.test(networkVersion) || networkVersion === 'loading'); export const NOOP = () => undefined;