From 9ea43eee5bd81f0af4ceca0bf94a4b8ac9e6e032 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Mon, 27 Jan 2025 11:17:20 -0800 Subject: [PATCH 01/12] Allow null networkVersion value --- src/MetaMaskInpageProvider.ts | 2 -- src/StreamProvider.ts | 14 +++----------- src/utils.test.ts | 4 ++-- src/utils.ts | 6 +++--- 4 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/MetaMaskInpageProvider.ts b/src/MetaMaskInpageProvider.ts index b8156369..b26d690c 100644 --- a/src/MetaMaskInpageProvider.ts +++ b/src/MetaMaskInpageProvider.ts @@ -463,8 +463,6 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { 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) { diff --git a/src/StreamProvider.ts b/src/StreamProvider.ts index acc4c942..2e7439bd 100644 --- a/src/StreamProvider.ts +++ b/src/StreamProvider.ts @@ -151,12 +151,8 @@ 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. @@ -178,11 +174,7 @@ export abstract class AbstractStreamProvider extends BaseProvider { return; } - if (networkVersion === 'loading') { - this._handleDisconnect(true); - } else { - super._handleChainChanged({ chainId }); - } + super._handleChainChanged({ chainId }); } } diff --git a/src/utils.test.ts b/src/utils.test.ts index a8a5ef87..c160e6ec 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -23,14 +23,14 @@ describe('utils', () => { '1', '10', '999', - 'loading', // this is a hack that we use + null ].forEach((value) => { expect(isValidNetworkVersion(value)).toBe(true); }); }); it('returns `false` for invalid values', () => { - ['', null, undefined, true, 2, 0x1, {}].forEach((value) => { + ['', undefined, true, 2, 0x1, {}].forEach((value) => { expect(isValidNetworkVersion(value)).toBe(false); }); }); diff --git a/src/utils.ts b/src/utils.ts index 8f9cae29..d41e9cf5 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -104,14 +104,14 @@ export const isValidChainId = (chainId: unknown): chainId is string => /** * Checks whether the given network version is valid, meaning if it is non-empty - * string. + * string or null. * * @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 => + (Boolean(networkVersion) && typeof networkVersion === 'string') || networkVersion === null export const NOOP = () => undefined; From 2ab37dd964a6609add321f90129e3bf8fae4380e Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Mon, 27 Jan 2025 11:32:06 -0800 Subject: [PATCH 02/12] update specs --- jest.config.js | 8 ++--- src/MetaMaskInpageProvider.test.ts | 51 ++++++++++-------------------- src/StreamProvider.test.ts | 50 ++++++++--------------------- 3 files changed, 34 insertions(+), 75 deletions(-) diff --git a/jest.config.js b/jest.config.js index 5a7a5578..ad810d64 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, - functions: 68.69, - lines: 68.35, - statements: 68.38, + branches: 63.88, + functions: 66.08, + lines: 68.39, + statements: 66.46, }, }, diff --git a/src/MetaMaskInpageProvider.test.ts b/src/MetaMaskInpageProvider.test.ts index e6d9cdce..6e379617 100644 --- a/src/MetaMaskInpageProvider.test.ts +++ b/src/MetaMaskInpageProvider.test.ts @@ -777,7 +777,7 @@ describe('MetaMaskInpageProvider: RPC', () => { }); }); - it('handles chain changes with intermittent disconnection', async () => { + it('handles chain changes when the wallet is unable to resolve networkVersion', async () => { const { provider, connectionStream } = await getInitializedProvider(); // We check this mostly for the readability of this test. @@ -787,52 +787,33 @@ describe('MetaMaskInpageProvider: RPC', () => { const emitSpy = jest.spyOn(provider, 'emit'); - await new Promise((resolve) => { - provider.once('disconnect', (error) => { - expect((error as any).code).toBe(1013); - resolve(); + await new Promise((resolve, reject) => { + provider.once('disconnect', () => { + reject(); }); - 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' }, - }); - }); - - // Only once, for "disconnect". - expect(emitSpy).toHaveBeenCalledTimes(1); - emitSpy.mockClear(); // Clear the mock to avoid keeping a count. - - expect(provider.isConnected()).toBe(false); - // These should be unchanged. - expect(provider.chainId).toBe('0x0'); - expect(provider.networkVersion).toBe('0'); - - await new Promise((resolve) => { - provider.once('chainChanged', (newChainId) => { - expect(newChainId).toBe('0x1'); - resolve(); - }); + provider.once('chainChanged', (chainId) => { + expect(chainId).toStrictEqual('0x1') + resolve() + }) connectionStream.notify(MetaMaskInpageProviderStreamName, { jsonrpc: '2.0', method: 'metamask_chainChanged', - params: { chainId: '0x1', networkVersion: '1' }, + // A null networkVersion indicates that the network version could not + // be determined for the network. The chainChanged event should still be + // emitted in this case. + params: { chainId: '0x1', networkVersion: null }, }); }); - expect(emitSpy).toHaveBeenCalledTimes(3); - expect(emitSpy).toHaveBeenNthCalledWith(1, 'connect', { chainId: '0x1' }); - expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x1'); - expect(emitSpy).toHaveBeenCalledWith('networkChanged', '1'); + expect(emitSpy).toHaveBeenCalledTimes(2); + expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x1') + expect(emitSpy).toHaveBeenCalledWith('networkChanged', null) expect(provider.isConnected()).toBe(true); expect(provider.chainId).toBe('0x1'); - expect(provider.networkVersion).toBe('1'); + expect(provider.networkVersion).toBe(null); }); }); diff --git a/src/StreamProvider.test.ts b/src/StreamProvider.test.ts index 60b14b4f..b3a54ce2 100644 --- a/src/StreamProvider.test.ts +++ b/src/StreamProvider.test.ts @@ -404,7 +404,7 @@ describe('StreamProvider', () => { }); }); - it('handles chain changes with intermittent disconnection', async () => { + it('handles chain changes when the wallet is unable to resolve networkVersion', async () => { const mockStream = new MockConnectionStream(); const mux = new ObjectMultiplex(); pipeline(mockStream, mux, mockStream, (error: Error | null) => { @@ -434,50 +434,28 @@ describe('StreamProvider', () => { const emitSpy = jest.spyOn(streamProvider, 'emit'); - await new Promise((resolve) => { - streamProvider.once('disconnect', (error) => { - expect(error.code).toBe(1013); - resolve(); - }); - - 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' }, + await new Promise((resolve, reject) => { + streamProvider.once('disconnect', () => { + reject(); }); - }); - // Only once, for "disconnect". - expect(emitSpy).toHaveBeenCalledTimes(1); - emitSpy.mockClear(); // Clear the mock to avoid keeping a count. - - expect(streamProvider.isConnected()).toBe(false); - // These should be unchanged. - expect(streamProvider.chainId).toBe('0x0'); - - await new Promise((resolve) => { - streamProvider.once('chainChanged', (newChainId) => { - expect(newChainId).toBe('0x1'); - resolve(); - }); + streamProvider.once('chainChanged', (chainId) => { + expect(chainId).toStrictEqual('0x1') + 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' }, + // A null networkVersion indicates that the network version could not + // be determined for the network. The chainChanged event should still be + // emitted in this case. + params: { chainId: '0x1', networkVersion: null }, }); }); - expect(emitSpy).toHaveBeenCalledTimes(2); - expect(emitSpy).toHaveBeenNthCalledWith(1, 'connect', { - chainId: '0x1', - }); - expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x1'); + expect(emitSpy).toHaveBeenCalledTimes(1); + expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x1') expect(streamProvider.isConnected()).toBe(true); expect(streamProvider.chainId).toBe('0x1'); From bb5168adc20398976bcf0079cbf740fcc803b63e Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Mon, 27 Jan 2025 14:56:45 -0800 Subject: [PATCH 03/12] add isConnected --- src/BaseProvider.ts | 33 ++++++-- src/MetaMaskInpageProvider.test.ts | 128 +++++++++++++++++++++++++---- src/MetaMaskInpageProvider.ts | 14 +++- src/StreamProvider.test.ts | 71 ++++++++++++---- src/StreamProvider.ts | 9 +- src/utils.test.ts | 7 +- src/utils.ts | 3 +- 7 files changed, 215 insertions(+), 50 deletions(-) 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 6e379617..85a9ca37 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, }, }), ); @@ -777,7 +780,7 @@ describe('MetaMaskInpageProvider: RPC', () => { }); }); - it('handles chain changes when the wallet is unable to resolve networkVersion', async () => { + it('handles chain changes with intermittent disconnection', async () => { const { provider, connectionStream } = await getInitializedProvider(); // We check this mostly for the readability of this test. @@ -787,33 +790,125 @@ describe('MetaMaskInpageProvider: RPC', () => { const emitSpy = jest.spyOn(provider, 'emit'); - await new Promise((resolve, reject) => { - provider.once('disconnect', () => { - reject(); + await new Promise((resolve) => { + provider.once('disconnect', (error) => { + expect((error as any).code).toBe(1013); + resolve(); }); - provider.once('chainChanged', (chainId) => { - expect(chainId).toStrictEqual('0x1') - resolve() - }) + connectionStream.notify(MetaMaskInpageProviderStreamName, { + jsonrpc: '2.0', + method: 'metamask_chainChanged', + params: { chainId: '0x1', networkVersion: '1', isConnected: false }, + }); + }); + + 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); + 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', - // A null networkVersion indicates that the network version could not - // be determined for the network. The chainChanged event should still be - // emitted in this case. - params: { chainId: '0x1', networkVersion: null }, + params: { chainId: '0x2', networkVersion: '2', isConnected: false }, }); }); expect(emitSpy).toHaveBeenCalledTimes(2); - expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x1') - expect(emitSpy).toHaveBeenCalledWith('networkChanged', null) + 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('0x1'); - expect(provider.networkVersion).toBe(null); + expect(provider.chainId).toBe('0x2'); + expect(provider.networkVersion).toBe('2'); + }); + + it('handles chain changes with null networkVersion', 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('networkChanged', (newNetworkId) => { + expect(newNetworkId).toBeNull(); + resolve(); + }); + + connectionStream.notify(MetaMaskInpageProviderStreamName, { + jsonrpc: '2.0', + method: 'metamask_chainChanged', + params: { chainId: '0x0', networkVersion: null, isConnected: true }, + }); + }); + + 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('0x0'); + expect(provider.networkVersion).toBe('1'); }); }); @@ -1017,6 +1112,7 @@ describe('MetaMaskInpageProvider: Miscellanea', () => { chainId: '0x0', isUnlocked: true, networkVersion: '0', + isConnected: true, }; }); diff --git a/src/MetaMaskInpageProvider.ts b/src/MetaMaskInpageProvider.ts index b26d690c..2113aafb 100644 --- a/src/MetaMaskInpageProvider.ts +++ b/src/MetaMaskInpageProvider.ts @@ -458,14 +458,20 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { * @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 } = {}) { - super._handleChainChanged({ chainId, networkVersion }); - - if (this._state.isConnected && networkVersion !== this.#networkVersion) { + isConnected, + }: { + chainId?: string; + networkVersion?: string; + isConnected?: boolean; + } = {}) { + super._handleChainChanged({ chainId, networkVersion, isConnected }); + + if (networkVersion !== this.#networkVersion) { this.#networkVersion = networkVersion as string; if (this._state.initialized) { this.emit('networkChanged', this.#networkVersion); diff --git a/src/StreamProvider.test.ts b/src/StreamProvider.test.ts index b3a54ce2..b74703dc 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, }; }); @@ -404,7 +407,7 @@ describe('StreamProvider', () => { }); }); - it('handles chain changes when the wallet is unable to resolve networkVersion', async () => { + it('handles chain changes with intermittent disconnection', async () => { const mockStream = new MockConnectionStream(); const mux = new ObjectMultiplex(); pipeline(mockStream, mux, mockStream, (error: Error | null) => { @@ -422,6 +425,7 @@ describe('StreamProvider', () => { chainId: '0x0', isUnlocked: true, networkVersion: '0', + isConnected: true, }; }); @@ -434,31 +438,70 @@ describe('StreamProvider', () => { const emitSpy = jest.spyOn(streamProvider, 'emit'); - await new Promise((resolve, reject) => { - streamProvider.once('disconnect', () => { - reject(); + await new Promise((resolve) => { + streamProvider.once('disconnect', (error) => { + expect(error.code).toBe(1013); + resolve(); + }); + + mockStream.notify(mockStreamName, { + jsonrpc: '2.0', + method: 'metamask_chainChanged', + params: { chainId: '0x1', networkVersion: '0', isConnected: false }, }); + }); - streamProvider.once('chainChanged', (chainId) => { - expect(chainId).toStrictEqual('0x1') - resolve() - }) + 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); + expect(streamProvider.chainId).toBe('0x1'); + + await new Promise((resolve) => { + streamProvider.once('chainChanged', (newChainId) => { + expect(newChainId).toBe('0x2'); + resolve(); + }); mockStream.notify(mockStreamName, { jsonrpc: '2.0', method: 'metamask_chainChanged', - // A null networkVersion indicates that the network version could not - // be determined for the network. The chainChanged event should still be - // emitted in this case. - params: { chainId: '0x1', networkVersion: null }, + params: { chainId: '0x2', networkVersion: '0', isConnected: false }, }); }); expect(emitSpy).toHaveBeenCalledTimes(1); - expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x1') + 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(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 2e7439bd..1b741cf5 100644 --- a/src/StreamProvider.ts +++ b/src/StreamProvider.ts @@ -158,13 +158,16 @@ export abstract class AbstractStreamProvider extends BaseProvider { * @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(), { @@ -174,7 +177,11 @@ export abstract class AbstractStreamProvider extends BaseProvider { return; } - super._handleChainChanged({ chainId }); + super._handleChainChanged({ chainId, isConnected }); + + if (!isConnected) { + this._handleDisconnect(true); + } } } diff --git a/src/utils.test.ts b/src/utils.test.ts index c160e6ec..a0147476 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -19,12 +19,7 @@ describe('utils', () => { describe('isValidNetworkVersion', () => { it('returns `true` for valid values', () => { - [ - '1', - '10', - '999', - null - ].forEach((value) => { + ['1', '10', '999', null].forEach((value) => { expect(isValidNetworkVersion(value)).toBe(true); }); }); diff --git a/src/utils.ts b/src/utils.ts index d41e9cf5..458ddeb6 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -112,6 +112,7 @@ export const isValidChainId = (chainId: unknown): chainId is string => export const isValidNetworkVersion = ( networkVersion: unknown, ): networkVersion is string | null => - (Boolean(networkVersion) && typeof networkVersion === 'string') || networkVersion === null + (Boolean(networkVersion) && typeof networkVersion === 'string') || + networkVersion === null; export const NOOP = () => undefined; From 722f5a60fc2ab5e4e7476e2efad4473d5a4ddfdd Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Mon, 27 Jan 2025 14:57:03 -0800 Subject: [PATCH 04/12] jest thresholds --- jest.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jest.config.js b/jest.config.js index ad810d64..9734e539 100644 --- a/jest.config.js +++ b/jest.config.js @@ -47,7 +47,7 @@ const baseConfig = { global: { branches: 63.88, functions: 66.08, - lines: 68.39, + lines: 68.35, statements: 66.46, }, }, From 980a9809d1c7791c3c6aeb70229c3c861d1d6ba3 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Mon, 27 Jan 2025 15:11:05 -0800 Subject: [PATCH 05/12] Fix isValidNetworkVersion check --- src/utils.test.ts | 2 +- src/utils.ts | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/utils.test.ts b/src/utils.test.ts index a0147476..f02585f3 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -19,7 +19,7 @@ describe('utils', () => { describe('isValidNetworkVersion', () => { it('returns `true` for valid values', () => { - ['1', '10', '999', null].forEach((value) => { + ['0', '1', '10', '999', null].forEach((value) => { expect(isValidNetworkVersion(value)).toBe(true); }); }); diff --git a/src/utils.ts b/src/utils.ts index 458ddeb6..57e334de 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,7 +107,7 @@ export const isValidChainId = (chainId: unknown): chainId is string => /** * Checks whether the given network version is valid, meaning if it is non-empty - * string or null. + * string when available or null otherwise. * * @param networkVersion - The network version to validate. * @returns Whether the given network version is valid. @@ -112,7 +115,8 @@ export const isValidChainId = (chainId: unknown): chainId is string => export const isValidNetworkVersion = ( networkVersion: unknown, ): networkVersion is string | null => - (Boolean(networkVersion) && typeof networkVersion === 'string') || + (typeof networkVersion === 'string' && + POSITIVE_INTEGER_REGEX.test(networkVersion)) || networkVersion === null; export const NOOP = () => undefined; From 7060911418d6fe0e72d10bdc2f788bd4b31c73e1 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Mon, 27 Jan 2025 15:11:23 -0800 Subject: [PATCH 06/12] Fix test mock data --- src/StreamProvider.test.ts | 3 ++- src/extension-provider/createExternalExtensionProvider.test.ts | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/StreamProvider.test.ts b/src/StreamProvider.test.ts index b74703dc..e2549f50 100644 --- a/src/StreamProvider.test.ts +++ b/src/StreamProvider.test.ts @@ -387,6 +387,7 @@ describe('StreamProvider', () => { chainId: '0x0', isUnlocked: true, networkVersion: '0', + isConnected: true, }; }); @@ -402,7 +403,7 @@ describe('StreamProvider', () => { mockStream.notify(mockStreamName, { jsonrpc: '2.0', method: 'metamask_chainChanged', - params: { chainId: '0x1', networkVersion: '0x1' }, + params: { chainId: '0x1', networkVersion: '0' }, }); }); }); diff --git a/src/extension-provider/createExternalExtensionProvider.test.ts b/src/extension-provider/createExternalExtensionProvider.test.ts index dff2ee23..11449221 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 }, }), ); From aea8f8c6ab80574483334d7ec277568fa9a09ba5 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Mon, 27 Jan 2025 15:24:51 -0800 Subject: [PATCH 07/12] bump jest --- jest.config.js | 8 +++--- src/StreamProvider.test.ts | 26 +++++++++++++++++++ .../createExternalExtensionProvider.test.ts | 2 +- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/jest.config.js b/jest.config.js index 9734e539..70aa2344 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: 63.88, - functions: 66.08, - lines: 68.35, - statements: 66.46, + branches: 67.58, + functions: 68.69, + lines: 68.42, + statements: 68.45, }, }, diff --git a/src/StreamProvider.test.ts b/src/StreamProvider.test.ts index e2549f50..949c0181 100644 --- a/src/StreamProvider.test.ts +++ b/src/StreamProvider.test.ts @@ -66,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', () => { diff --git a/src/extension-provider/createExternalExtensionProvider.test.ts b/src/extension-provider/createExternalExtensionProvider.test.ts index 11449221..267b4fec 100644 --- a/src/extension-provider/createExternalExtensionProvider.test.ts +++ b/src/extension-provider/createExternalExtensionProvider.test.ts @@ -74,7 +74,7 @@ async function getInitializedProvider({ chainId, isUnlocked, networkVersion, - isConnected + isConnected, }, }), ); From c0af3b9078c3a1d89860b980e68f6d43603ff669 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Mon, 27 Jan 2025 15:29:29 -0800 Subject: [PATCH 08/12] bump jest?.. --- jest.config.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/jest.config.js b/jest.config.js index 70aa2344..ee9cc90a 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: 67.58, + branches: 67.98, functions: 68.69, - lines: 68.42, - statements: 68.45, + lines: 68.62, + statements: 68.65, }, }, From 3f24e8bacd2b96066982f47aa135259a2d22e8ff Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Tue, 28 Jan 2025 12:41:03 -0800 Subject: [PATCH 09/12] interpret 'loading' as null --- src/MetaMaskInpageProvider.test.ts | 8 ++++++-- src/MetaMaskInpageProvider.ts | 11 +++++++++-- src/utils.test.ts | 9 +++++++-- src/utils.ts | 7 +++---- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/MetaMaskInpageProvider.test.ts b/src/MetaMaskInpageProvider.test.ts index 85a9ca37..623ffec4 100644 --- a/src/MetaMaskInpageProvider.test.ts +++ b/src/MetaMaskInpageProvider.test.ts @@ -859,7 +859,7 @@ describe('MetaMaskInpageProvider: RPC', () => { expect(provider.networkVersion).toBe('2'); }); - it('handles chain changes with null networkVersion', async () => { + 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. @@ -878,7 +878,11 @@ describe('MetaMaskInpageProvider: RPC', () => { connectionStream.notify(MetaMaskInpageProviderStreamName, { jsonrpc: '2.0', method: 'metamask_chainChanged', - params: { chainId: '0x0', networkVersion: null, isConnected: true }, + params: { + chainId: '0x0', + networkVersion: 'loading', + isConnected: true, + }, }); }); diff --git a/src/MetaMaskInpageProvider.ts b/src/MetaMaskInpageProvider.ts index 2113aafb..24ae1c09 100644 --- a/src/MetaMaskInpageProvider.ts +++ b/src/MetaMaskInpageProvider.ts @@ -471,8 +471,15 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { } = {}) { super._handleChainChanged({ chainId, networkVersion, isConnected }); - if (networkVersion !== this.#networkVersion) { - this.#networkVersion = networkVersion as string; + // 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. + 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/utils.test.ts b/src/utils.test.ts index f02585f3..a8a5ef87 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -19,13 +19,18 @@ describe('utils', () => { describe('isValidNetworkVersion', () => { it('returns `true` for valid values', () => { - ['0', '1', '10', '999', null].forEach((value) => { + [ + '1', + '10', + '999', + 'loading', // this is a hack that we use + ].forEach((value) => { expect(isValidNetworkVersion(value)).toBe(true); }); }); it('returns `false` for invalid values', () => { - ['', undefined, true, 2, 0x1, {}].forEach((value) => { + ['', null, undefined, true, 2, 0x1, {}].forEach((value) => { expect(isValidNetworkVersion(value)).toBe(false); }); }); diff --git a/src/utils.ts b/src/utils.ts index 57e334de..7f4d2e9d 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -107,7 +107,7 @@ export const isValidChainId = (chainId: unknown): chainId is string => /** * Checks whether the given network version is valid, meaning if it is non-empty - * string when available or null otherwise. + * string when available or the value 'loading' otherwise. * * @param networkVersion - The network version to validate. * @returns Whether the given network version is valid. @@ -115,8 +115,7 @@ export const isValidChainId = (chainId: unknown): chainId is string => export const isValidNetworkVersion = ( networkVersion: unknown, ): networkVersion is string | null => - (typeof networkVersion === 'string' && - POSITIVE_INTEGER_REGEX.test(networkVersion)) || - networkVersion === null; + typeof networkVersion === 'string' && + (POSITIVE_INTEGER_REGEX.test(networkVersion) || networkVersion === 'loading'); export const NOOP = () => undefined; From d9ed19d2848e5da31d4757614f3fcd5ca510d265 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Tue, 28 Jan 2025 12:48:05 -0800 Subject: [PATCH 10/12] update utils comment --- src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils.ts b/src/utils.ts index 7f4d2e9d..7b5f3540 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -107,7 +107,7 @@ export const isValidChainId = (chainId: unknown): chainId is string => /** * Checks whether the given network version is valid, meaning if it is non-empty - * string when available or the value 'loading' otherwise. + * integer string or the value 'loading'. * * @param networkVersion - The network version to validate. * @returns Whether the given network version is valid. From 6abb5a662c27aab5334c8e914bfda6b408170bcf Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Tue, 28 Jan 2025 12:53:02 -0800 Subject: [PATCH 11/12] update comments --- src/MetaMaskInpageProvider.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/MetaMaskInpageProvider.ts b/src/MetaMaskInpageProvider.ts index 24ae1c09..e74ddf48 100644 --- a/src/MetaMaskInpageProvider.ts +++ b/src/MetaMaskInpageProvider.ts @@ -450,9 +450,12 @@ 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. @@ -474,7 +477,9 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { // 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. + // 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; From 44abb41ba8c6d9f50bc67fd9b048573b5f4855a7 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Tue, 28 Jan 2025 12:58:11 -0800 Subject: [PATCH 12/12] jest thresholds --- jest.config.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/jest.config.js b/jest.config.js index ee9cc90a..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: 67.98, + branches: 68.23, functions: 68.69, - lines: 68.62, - statements: 68.65, + lines: 68.68, + statements: 68.71, }, },