From 02f9d84092128026bdcfa20e375398f293c9f092 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Wed, 6 Mar 2024 15:17:06 -0800 Subject: [PATCH 01/10] remove networkVersion from baseProvider --- src/BaseProvider.ts | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/BaseProvider.ts b/src/BaseProvider.ts index 5dfcda5e..4b3adfe1 100644 --- a/src/BaseProvider.ts +++ b/src/BaseProvider.ts @@ -230,14 +230,10 @@ export abstract class BaseProvider extends SafeEventEmitter { * Sets initial state if provided and marks this provider as initialized. * Throws if called more than once. * - * Permits the `networkVersion` field in the parameter object for - * compatibility with child classes that use this value. - * * @param initialState - The provider's initial state. * @param initialState.accounts - The user's accounts. * @param initialState.chainId - The chain ID. * @param initialState.isUnlocked - Whether the user has unlocked MetaMask. - * @param initialState.networkVersion - The network version. * @fires BaseProvider#_initialized - If `initialState` is defined. * @fires BaseProvider#connect - If `initialState` is defined. */ @@ -245,18 +241,17 @@ export abstract class BaseProvider extends SafeEventEmitter { accounts: string[]; chainId: string; isUnlocked: boolean; - networkVersion?: string; }) { if (this._state.initialized) { throw new Error('Provider already initialized.'); } if (initialState) { - const { accounts, chainId, isUnlocked, networkVersion } = initialState; + const { accounts, chainId, isUnlocked } = initialState; // EIP-1193 connect this._handleConnect(chainId); - this._handleChainChanged({ chainId, networkVersion }); + this._handleChainChanged({ chainId }); this._handleUnlockStateChanged({ accounts, isUnlocked }); this._handleAccountsChanged(accounts); } @@ -369,18 +364,13 @@ export abstract class BaseProvider extends SafeEventEmitter { * and sets relevant public state. Does nothing if the given `chainId` is * equivalent to the existing value. * - * Permits the `networkVersion` field in the parameter object for - * compatibility with child classes that use this value. - * * @fires BaseProvider#chainChanged * @param networkInfo - An object with network info. * @param networkInfo.chainId - The latest chain ID. */ protected _handleChainChanged({ chainId, - }: - | { chainId?: string | undefined; networkVersion?: string | undefined } - | undefined = {}) { + }: { chainId?: string | undefined } | undefined = {}) { if (!isValidChainId(chainId)) { this._log.error(messages.errors.invalidNetworkParams(), { chainId }); return; From 87694a48f3a55ae9ecf6156f9439f3d6ea85ca73 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Wed, 6 Mar 2024 15:19:04 -0800 Subject: [PATCH 02/10] update streamProvider except disconnect spec --- src/StreamProvider.test.ts | 7 +------ src/StreamProvider.ts | 28 ++++++---------------------- src/utils.test.ts | 21 +-------------------- src/utils.ts | 12 ------------ 4 files changed, 8 insertions(+), 60 deletions(-) diff --git a/src/StreamProvider.test.ts b/src/StreamProvider.test.ts index a2a7c06f..20a0d24d 100644 --- a/src/StreamProvider.test.ts +++ b/src/StreamProvider.test.ts @@ -35,7 +35,6 @@ describe('StreamProvider', () => { it('initializes state and emits events', async () => { const accounts = ['0xabc']; const chainId = '0x1'; - const networkVersion = '1'; const isUnlocked = true; const streamProvider = new StreamProvider(new MockConnectionStream(), { @@ -49,14 +48,11 @@ describe('StreamProvider', () => { accounts, chainId, isUnlocked, - networkVersion, }; }); await streamProvider.initialize(); - expect(streamProvider.chainId).toBe(chainId); - expect(streamProvider.selectedAddress).toBe(accounts[0]); expect(streamProvider.isConnected()).toBe(true); expect(requestMock).toHaveBeenCalledTimes(1); @@ -381,7 +377,6 @@ describe('StreamProvider', () => { accounts: [], chainId: '0x0', isUnlocked: true, - networkVersion: '0', }; }); @@ -397,7 +392,7 @@ describe('StreamProvider', () => { mockStream.notify(mockStreamName, { jsonrpc: '2.0', method: 'metamask_chainChanged', - params: { chainId: '0x1', networkVersion: '0x1' }, + params: { chainId: '0x1' }, }); }); }); diff --git a/src/StreamProvider.ts b/src/StreamProvider.ts index 60afc8df..2a3b52cf 100644 --- a/src/StreamProvider.ts +++ b/src/StreamProvider.ts @@ -10,11 +10,7 @@ import type { Duplex } from 'readable-stream'; import type { BaseProviderOptions } from './BaseProvider'; import { BaseProvider } from './BaseProvider'; import messages from './messages'; -import { - EMITTED_NOTIFICATIONS, - isValidChainId, - isValidNetworkVersion, -} from './utils'; +import { EMITTED_NOTIFICATIONS, isValidChainId } from './utils'; export type StreamProviderOptions = { /** @@ -167,39 +163,27 @@ 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. + * Upon receipt of a new chainId, emits corresponding + * events and sets relevant public state. * * @fires BaseProvider#chainChanged * @param networkInfo - An object with network info. * @param networkInfo.chainId - The latest chain ID. - * @param networkInfo.networkVersion - The latest network ID. */ protected _handleChainChanged({ chainId, - networkVersion, }: { chainId?: string | undefined; - networkVersion?: string | undefined; } = {}) { - if (!isValidChainId(chainId) || !isValidNetworkVersion(networkVersion)) { + if (!isValidChainId(chainId)) { this._log.error(messages.errors.invalidNetworkParams(), { chainId, - networkVersion, }); return; } - if (networkVersion === 'loading') { - this._handleDisconnect(true); - } else { - super._handleChainChanged({ chainId }); - } + // should disconnect/state reset happen somehow still? + super._handleChainChanged({ chainId }); } } diff --git a/src/utils.test.ts b/src/utils.test.ts index a8a5ef87..af82f852 100644 --- a/src/utils.test.ts +++ b/src/utils.test.ts @@ -1,4 +1,4 @@ -import { isValidChainId, isValidNetworkVersion } from './utils'; +import { isValidChainId } from './utils'; describe('utils', () => { describe('isValidChainId', () => { @@ -16,23 +16,4 @@ describe('utils', () => { ); }); }); - - describe('isValidNetworkVersion', () => { - it('returns `true` for valid values', () => { - [ - '1', - '10', - '999', - 'loading', // this is a hack that we use - ].forEach((value) => { - expect(isValidNetworkVersion(value)).toBe(true); - }); - }); - - it('returns `false` for invalid values', () => { - ['', null, undefined, true, 2, 0x1, {}].forEach((value) => { - expect(isValidNetworkVersion(value)).toBe(false); - }); - }); - }); }); diff --git a/src/utils.ts b/src/utils.ts index deb35d98..0e54015b 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -94,16 +94,4 @@ export const getRpcPromiseCallback = export const isValidChainId = (chainId: unknown): chainId is string => Boolean(chainId) && typeof chainId === 'string' && chainId.startsWith('0x'); -/** - * Checks whether the given network version is valid, meaning if it is non-empty - * string. - * - * @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'; - export const NOOP = () => undefined; From 6a30cb913a807b8448e5315942b999147a7ae626 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Wed, 6 Mar 2024 15:20:49 -0800 Subject: [PATCH 03/10] Remove networkId. Block access to BaseProvider.chainId and selectedAddress. Remove send net_version. Remove networkChanged event --- src/MetaMaskInpageProvider.test.ts | 84 +++------------------------ src/MetaMaskInpageProvider.ts | 93 ++---------------------------- 2 files changed, 13 insertions(+), 164 deletions(-) diff --git a/src/MetaMaskInpageProvider.test.ts b/src/MetaMaskInpageProvider.test.ts index 22001831..00234381 100644 --- a/src/MetaMaskInpageProvider.test.ts +++ b/src/MetaMaskInpageProvider.test.ts @@ -44,12 +44,7 @@ type InitializedProviderDetails = { * can be used to inspect message sent by the provider. */ async function getInitializedProvider({ - initialState: { - accounts = [], - chainId = '0x0', - isUnlocked = true, - networkVersion = '0', - } = {}, + initialState: { accounts = [], chainId = '0x0', isUnlocked = true } = {}, onMethodCalled = [], }: { initialState?: Partial< @@ -78,7 +73,6 @@ async function getInitializedProvider({ accounts, chainId, isUnlocked, - networkVersion, }, }), ); @@ -713,13 +707,6 @@ describe('MetaMaskInpageProvider: RPC', () => { expect.any(Function), ); }); - - it('net_version', () => { - const result = provider.send({ method: 'net_version' }); - expect(result).toMatchObject({ - result: null, - }); - }); }); it('throws on unsupported sync method', () => { @@ -1028,7 +1015,6 @@ describe('MetaMaskInpageProvider: Miscellanea', () => { accounts: ['0xabc'], chainId: '0x0', isUnlocked: true, - networkVersion: '0', }; }); @@ -1037,9 +1023,6 @@ describe('MetaMaskInpageProvider: Miscellanea', () => { await new Promise((resolve) => setTimeout(() => resolve(), 1)); expect(requestMock).toHaveBeenCalledTimes(1); - expect(inpageProvider.chainId).toBe('0x0'); - expect(inpageProvider.networkVersion).toBe('0'); - expect(inpageProvider.selectedAddress).toBe('0xabc'); expect(inpageProvider.isConnected()).toBe(true); }); }); @@ -1084,52 +1067,10 @@ describe('MetaMaskInpageProvider: Miscellanea', () => { ).provider; }); - it('should warn the first time chainId is accessed', async () => { - const consoleWarnSpy = jest.spyOn(globalThis.console, 'warn'); - - expect(provider.chainId).toBe('0x5'); - expect(consoleWarnSpy).toHaveBeenCalledWith( - messages.warnings.chainIdDeprecation, - ); - expect(consoleWarnSpy).toHaveBeenCalledTimes(1); - }); - - it('should not allow chainId to be modified', () => { - expect(() => (provider.chainId = '0x539')).toThrow( - 'Cannot set property chainId', - ); - expect(provider.chainId).toBe('0x5'); - }); - }); - - describe('networkVersion', () => { - let provider: any | MetaMaskInpageProvider; - - beforeEach(async () => { - provider = ( - await getInitializedProvider({ - initialState: { - networkVersion: '5', - }, - }) - ).provider; - }); - - it('should warn the first time networkVersion is accessed', async () => { - const consoleWarnSpy = jest.spyOn(globalThis.console, 'warn'); - - expect(provider.networkVersion).toBe('5'); - expect(consoleWarnSpy).toHaveBeenCalledWith( - messages.warnings.networkVersionDeprecation, - ); - expect(consoleWarnSpy).toHaveBeenCalledTimes(1); - }); - - it('should not allow networkVersion to be modified', () => { - expect(() => (provider.networkVersion = '1337')).toThrow( - 'Cannot set property networkVersion', + it('should throw an error when accessing chainId', () => { + expect(() => provider.chainId).toThrow( + `'ethereum.chainId' has been removed`, ); - expect(provider.networkVersion).toBe('5'); }); }); @@ -1146,21 +1087,10 @@ describe('MetaMaskInpageProvider: Miscellanea', () => { ).provider; }); - it('should warn the first time selectedAddress is accessed', async () => { - const consoleWarnSpy = jest.spyOn(globalThis.console, 'warn'); - - expect(provider.selectedAddress).toBe('0xdeadbeef'); - expect(consoleWarnSpy).toHaveBeenCalledWith( - messages.warnings.selectedAddressDeprecation, - ); - expect(consoleWarnSpy).toHaveBeenCalledTimes(1); - }); - - it('should not allow selectedAddress to be modified', () => { - expect(() => (provider.selectedAddress = '0x12345678')).toThrow( - 'Cannot set property selectedAddress', + it('should throw an error when accessing selectedAddress', () => { + expect(() => provider.selectedAddress).toThrow( + `'ethereum.selectedAddress' has been removed`, ); - expect(provider.selectedAddress).toBe('0xdeadbeef'); }); }); }); diff --git a/src/MetaMaskInpageProvider.ts b/src/MetaMaskInpageProvider.ts index f3b3aa6a..b7fcce67 100644 --- a/src/MetaMaskInpageProvider.ts +++ b/src/MetaMaskInpageProvider.ts @@ -15,11 +15,7 @@ import { } from './utils'; export type SendSyncJsonRpcRequest = { - method: - | 'eth_accounts' - | 'eth_coinbase' - | 'eth_uninstallFilter' - | 'net_version'; + method: 'eth_accounts' | 'eth_coinbase' | 'eth_uninstallFilter'; } & JsonRpcRequest; type WarningEventName = keyof SentWarningsState['events']; @@ -34,10 +30,6 @@ export type MetaMaskInpageProviderOptions = { } & Partial>; type SentWarningsState = { - // properties - chainId: boolean; - networkVersion: boolean; - selectedAddress: boolean; // methods enable: boolean; experimentalMethods: boolean; @@ -46,7 +38,6 @@ type SentWarningsState = { events: { close: boolean; data: boolean; - networkChanged: boolean; notification: boolean; }; }; @@ -58,10 +49,6 @@ export const MetaMaskInpageProviderStreamName = 'metamask-provider'; export class MetaMaskInpageProvider extends AbstractStreamProvider { protected _sentWarnings: SentWarningsState = { - // properties - chainId: false, - networkVersion: false, - selectedAddress: false, // methods enable: false, experimentalMethods: false, @@ -70,7 +57,6 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { events: { close: false, data: false, - networkChanged: false, notification: false, }, }; @@ -82,8 +68,6 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { MetaMaskInpageProvider['_getExperimentalApi'] >; - #networkVersion: string | null; - /** * Indicating that this provider is a MetaMask provider. */ @@ -124,7 +108,6 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { // eslint-disable-next-line @typescript-eslint/no-floating-promises this._initializeStateAsync(); - this.#networkVersion = null; this.isMetaMask = true; this._sendSync = this._sendSync.bind(this); @@ -167,31 +150,15 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { } //==================== - // Deprecated Properties + // Removed Properties //==================== get chainId(): string | null { - if (!this._sentWarnings.chainId) { - this._log.warn(messages.warnings.chainIdDeprecation); - this._sentWarnings.chainId = true; - } - return super.chainId; - } - - get networkVersion(): string | null { - if (!this._sentWarnings.networkVersion) { - this._log.warn(messages.warnings.networkVersionDeprecation); - this._sentWarnings.networkVersion = true; - } - return this.#networkVersion; + throw new Error(messages.errors.invalidPropertyChainId()) } get selectedAddress(): string | null { - if (!this._sentWarnings.selectedAddress) { - this._log.warn(messages.warnings.selectedAddressDeprecation); - this._sentWarnings.selectedAddress = true; - } - return super.selectedAddress; + throw new Error(messages.errors.invalidPropertySelectedAddress()) } //==================== @@ -249,24 +216,6 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { // Private Methods //==================== - /** - * When the provider becomes disconnected, updates internal state and emits - * required events. Idempotent with respect to the isRecoverable parameter. - * - * Error codes per the CloseEvent status codes as required by EIP-1193: - * https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent#Status_codes. - * - * @param isRecoverable - Whether the disconnection is recoverable. - * @param errorMessage - A custom error message. - * @fires BaseProvider#disconnect - If the disconnection is not recoverable. - */ - protected _handleDisconnect(isRecoverable: boolean, errorMessage?: string) { - super._handleDisconnect(isRecoverable, errorMessage); - if (this.#networkVersion && !isRecoverable) { - this.#networkVersion = null; - } - } - /** * Warns of deprecation for the given event, if applicable. * @@ -392,11 +341,11 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { let result; switch (payload.method) { case 'eth_accounts': - result = this.selectedAddress ? [this.selectedAddress] : []; + result = super.selectedAddress ? [super.selectedAddress] : []; break; case 'eth_coinbase': - result = this.selectedAddress ?? null; + result = super.selectedAddress ?? null; break; case 'eth_uninstallFilter': @@ -404,10 +353,6 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { result = true; break; - case 'net_version': - result = this.#networkVersion ?? null; - break; - default: throw new Error(messages.errors.unsupportedSync(payload.method)); } @@ -474,30 +419,4 @@ 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. - * - * @fires MetamaskInpageProvider#networkChanged - * @param networkInfo - An object with network info. - * @param networkInfo.chainId - The latest chain ID. - * @param networkInfo.networkVersion - The latest network ID. - */ - 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; - if (this._state.initialized) { - this.emit('networkChanged', this.#networkVersion); - } - } - } } From 03ce6757e7a5b8acb0e8c1917a0d5fa5e6293631 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Wed, 6 Mar 2024 15:21:58 -0800 Subject: [PATCH 04/10] remove proxy property access --- src/initializeInpageProvider.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/initializeInpageProvider.ts b/src/initializeInpageProvider.ts index 3fba9600..c31bfc41 100644 --- a/src/initializeInpageProvider.ts +++ b/src/initializeInpageProvider.ts @@ -62,11 +62,6 @@ export function initializeProvider({ const proxiedProvider = new Proxy(provider, { // some common libraries, e.g. web3@1.x, mess with our API deleteProperty: () => true, - // fix issue with Proxy unable to access private variables from getters - // https://stackoverflow.com/a/73051482 - get(target, propName: 'chainId' | 'networkVersion' | 'selectedAddress') { - return target[propName]; - }, }); if (providerInfo) { From 73c69dc33d1753a52cb902465ee98a61e6b14714 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Wed, 6 Mar 2024 15:22:11 -0800 Subject: [PATCH 05/10] fix external provider test --- .../createExternalExtensionProvider.test.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/extension-provider/createExternalExtensionProvider.test.ts b/src/extension-provider/createExternalExtensionProvider.test.ts index 87ce1489..f1f518d8 100644 --- a/src/extension-provider/createExternalExtensionProvider.test.ts +++ b/src/extension-provider/createExternalExtensionProvider.test.ts @@ -37,12 +37,7 @@ type InitializedExtensionProviderDetails = { * "onWrite" stub that can be used to inspect message sent by the provider. */ async function getInitializedProvider({ - initialState: { - accounts = [], - chainId = '0x0', - isUnlocked = true, - networkVersion = '0', - } = {}, + initialState: { accounts = [], chainId = '0x0', isUnlocked = true } = {}, onMethodCalled = [], }: { initialState?: Partial[0]>; @@ -69,7 +64,6 @@ async function getInitializedProvider({ accounts, chainId, isUnlocked, - networkVersion, }, }), ); From 791c5c1824fad589dfd0881886a8ebec9a0b19eb Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Wed, 6 Mar 2024 15:22:23 -0800 Subject: [PATCH 06/10] update warning/error messages --- src/messages.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/messages.ts b/src/messages.ts index c17778c1..79f01929 100644 --- a/src/messages.ts +++ b/src/messages.ts @@ -18,16 +18,14 @@ const messages = { invalidLoggerObject: () => `'args.logger' must be an object if provided.`, invalidLoggerMethod: (method: string) => `'args.logger' must include required method '${method}'.`, + invalidPropertyChainId: () => `MetaMask: 'ethereum.chainId' has been removed. Please use the 'eth_chainId' RPC method instead.\nFor more information, see: https://github.com/MetaMask/metamask-improvement-proposals/discussions/23`, + invalidPropertySelectedAddress: () => `MetaMask: 'ethereum.selectedAddress' has been removed. Please use the 'eth_accounts' RPC method instead.\nFor more information, see: https://github.com/MetaMask/metamask-improvement-proposals/discussions/23`, }, info: { connected: (chainId: string) => `MetaMask: Connected to chain with ID "${chainId}".`, }, warnings: { - // deprecated properties - chainIdDeprecation: `MetaMask: 'ethereum.chainId' is deprecated and may be removed in the future. Please use the 'eth_chainId' RPC method instead.\nFor more information, see: https://github.com/MetaMask/metamask-improvement-proposals/discussions/23`, - networkVersionDeprecation: `MetaMask: 'ethereum.networkVersion' is deprecated and may be removed in the future. Please use the 'net_version' RPC method instead.\nFor more information, see: https://github.com/MetaMask/metamask-improvement-proposals/discussions/23`, - selectedAddressDeprecation: `MetaMask: 'ethereum.selectedAddress' is deprecated and may be removed in the future. Please use the 'eth_accounts' RPC method instead.\nFor more information, see: https://github.com/MetaMask/metamask-improvement-proposals/discussions/23`, // deprecated methods enableDeprecation: `MetaMask: 'ethereum.enable()' is deprecated and may be removed in the future. Please use the 'eth_requestAccounts' RPC method instead.\nFor more information, see: https://eips.ethereum.org/EIPS/eip-1102`, sendDeprecation: `MetaMask: 'ethereum.send(...)' is deprecated and may be removed in the future. Please use 'ethereum.sendAsync(...)' or 'ethereum.request(...)' instead.\nFor more information, see: https://eips.ethereum.org/EIPS/eip-1193`, @@ -35,7 +33,6 @@ const messages = { events: { close: `MetaMask: The event 'close' is deprecated and may be removed in the future. Please use 'disconnect' instead.\nFor more information, see: https://eips.ethereum.org/EIPS/eip-1193#disconnect`, data: `MetaMask: The event 'data' is deprecated and will be removed in the future. Use 'message' instead.\nFor more information, see: https://eips.ethereum.org/EIPS/eip-1193#message`, - networkChanged: `MetaMask: The event 'networkChanged' is deprecated and may be removed in the future. Use 'chainChanged' instead.\nFor more information, see: https://eips.ethereum.org/EIPS/eip-1193#chainchanged`, notification: `MetaMask: The event 'notification' is deprecated and may be removed in the future. Use 'message' instead.\nFor more information, see: https://eips.ethereum.org/EIPS/eip-1193#message`, }, rpc: { From ce0dc773dc37af7b904905c7c0641368e24ca701 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Wed, 6 Mar 2024 15:24:15 -0800 Subject: [PATCH 07/10] Lint --- src/MetaMaskInpageProvider.ts | 4 ++-- src/messages.ts | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/MetaMaskInpageProvider.ts b/src/MetaMaskInpageProvider.ts index b7fcce67..59e4c99a 100644 --- a/src/MetaMaskInpageProvider.ts +++ b/src/MetaMaskInpageProvider.ts @@ -154,11 +154,11 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { //==================== get chainId(): string | null { - throw new Error(messages.errors.invalidPropertyChainId()) + throw new Error(messages.errors.invalidPropertyChainId()); } get selectedAddress(): string | null { - throw new Error(messages.errors.invalidPropertySelectedAddress()) + throw new Error(messages.errors.invalidPropertySelectedAddress()); } //==================== diff --git a/src/messages.ts b/src/messages.ts index 79f01929..2e7f1c35 100644 --- a/src/messages.ts +++ b/src/messages.ts @@ -18,8 +18,10 @@ const messages = { invalidLoggerObject: () => `'args.logger' must be an object if provided.`, invalidLoggerMethod: (method: string) => `'args.logger' must include required method '${method}'.`, - invalidPropertyChainId: () => `MetaMask: 'ethereum.chainId' has been removed. Please use the 'eth_chainId' RPC method instead.\nFor more information, see: https://github.com/MetaMask/metamask-improvement-proposals/discussions/23`, - invalidPropertySelectedAddress: () => `MetaMask: 'ethereum.selectedAddress' has been removed. Please use the 'eth_accounts' RPC method instead.\nFor more information, see: https://github.com/MetaMask/metamask-improvement-proposals/discussions/23`, + invalidPropertyChainId: () => + `MetaMask: 'ethereum.chainId' has been removed. Please use the 'eth_chainId' RPC method instead.\nFor more information, see: https://github.com/MetaMask/metamask-improvement-proposals/discussions/23`, + invalidPropertySelectedAddress: () => + `MetaMask: 'ethereum.selectedAddress' has been removed. Please use the 'eth_accounts' RPC method instead.\nFor more information, see: https://github.com/MetaMask/metamask-improvement-proposals/discussions/23`, }, info: { connected: (chainId: string) => From 5ff60b58c350dc013c72103eed9d187f1aa2f327 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Wed, 6 Mar 2024 15:33:13 -0800 Subject: [PATCH 08/10] Remove recoverable disconnects since they can no longer occur --- src/BaseProvider.ts | 39 +++++---------- src/MetaMaskInpageProvider.test.ts | 77 +----------------------------- src/MetaMaskInpageProvider.ts | 2 +- src/StreamProvider.test.ts | 75 ----------------------------- src/StreamProvider.ts | 3 +- src/messages.ts | 2 - 6 files changed, 16 insertions(+), 182 deletions(-) diff --git a/src/BaseProvider.ts b/src/BaseProvider.ts index 4b3adfe1..21526389 100644 --- a/src/BaseProvider.ts +++ b/src/BaseProvider.ts @@ -324,36 +324,23 @@ export abstract class BaseProvider extends SafeEventEmitter { * Error codes per the CloseEvent status codes as required by EIP-1193: * https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent#Status_codes. * - * @param isRecoverable - Whether the disconnection is recoverable. * @param errorMessage - A custom error message. - * @fires BaseProvider#disconnect - If the disconnection is not recoverable. + * @fires BaseProvider#disconnect */ - protected _handleDisconnect(isRecoverable: boolean, errorMessage?: string) { - if ( - this._state.isConnected || - (!this._state.isPermanentlyDisconnected && !isRecoverable) - ) { + protected _handleDisconnect(errorMessage?: string) { + if (this._state.isConnected || !this._state.isPermanentlyDisconnected) { this._state.isConnected = false; - let error; - if (isRecoverable) { - error = new JsonRpcError( - 1013, // Try again later - errorMessage ?? messages.errors.disconnected(), - ); - this._log.debug(error); - } else { - error = new JsonRpcError( - 1011, // Internal error - errorMessage ?? messages.errors.permanentlyDisconnected(), - ); - this._log.error(error); - this.#chainId = null; - this._state.accounts = null; - this.#selectedAddress = null; - this._state.isUnlocked = false; - this._state.isPermanentlyDisconnected = true; - } + const error = new JsonRpcError( + 1011, // Internal error + errorMessage ?? messages.errors.permanentlyDisconnected(), + ); + this._log.error(error); + this.#chainId = null; + this._state.accounts = null; + this.#selectedAddress = null; + this._state.isUnlocked = false; + this._state.isPermanentlyDisconnected = true; this.emit('disconnect', error); } diff --git a/src/MetaMaskInpageProvider.test.ts b/src/MetaMaskInpageProvider.test.ts index 00234381..3579b108 100644 --- a/src/MetaMaskInpageProvider.test.ts +++ b/src/MetaMaskInpageProvider.test.ts @@ -735,85 +735,10 @@ describe('MetaMaskInpageProvider: RPC', () => { connectionStream.notify(MetaMaskInpageProviderStreamName, { jsonrpc: '2.0', method: 'metamask_chainChanged', - params: { chainId: '0x1', networkVersion: '1' }, + params: { chainId: '0x1' }, }); }); }); - - it('calls networkChanged when receiving a new networkVersion', async () => { - const { provider, connectionStream } = await getInitializedProvider(); - - await new Promise((resolve) => { - provider.once('networkChanged', (newNetworkId) => { - expect(newNetworkId).toBe('1'); - resolve(undefined); - }); - - connectionStream.notify(MetaMaskInpageProviderStreamName, { - jsonrpc: '2.0', - method: 'metamask_chainChanged', - params: { chainId: '0x1', networkVersion: '1' }, - }); - }); - }); - - it('handles chain changes with intermittent disconnection', 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('disconnect', (error) => { - expect((error as any).code).toBe(1013); - resolve(); - }); - - 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(); - }); - - connectionStream.notify(MetaMaskInpageProviderStreamName, { - jsonrpc: '2.0', - method: 'metamask_chainChanged', - params: { chainId: '0x1', networkVersion: '1' }, - }); - }); - - expect(emitSpy).toHaveBeenCalledTimes(3); - expect(emitSpy).toHaveBeenNthCalledWith(1, 'connect', { chainId: '0x1' }); - expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x1'); - expect(emitSpy).toHaveBeenCalledWith('networkChanged', '1'); - - expect(provider.isConnected()).toBe(true); - expect(provider.chainId).toBe('0x1'); - expect(provider.networkVersion).toBe('1'); - }); }); describe('warnings', () => { diff --git a/src/MetaMaskInpageProvider.ts b/src/MetaMaskInpageProvider.ts index 59e4c99a..add5a9af 100644 --- a/src/MetaMaskInpageProvider.ts +++ b/src/MetaMaskInpageProvider.ts @@ -150,7 +150,7 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { } //==================== - // Removed Properties + // Private Properties //==================== get chainId(): string | null { diff --git a/src/StreamProvider.test.ts b/src/StreamProvider.test.ts index 20a0d24d..ec16f273 100644 --- a/src/StreamProvider.test.ts +++ b/src/StreamProvider.test.ts @@ -396,81 +396,6 @@ describe('StreamProvider', () => { }); }); }); - - it('handles chain changes with intermittent disconnection', async () => { - const mockStream = new MockConnectionStream(); - const streamProvider = new StreamProvider(mockStream, { - jsonRpcStreamName: mockStreamName, - }); - - const requestMock = jest - .spyOn(streamProvider, 'request') - .mockImplementationOnce(async () => { - return { - accounts: [], - chainId: '0x0', - isUnlocked: true, - networkVersion: '0', - }; - }); - - await streamProvider.initialize(); - expect(requestMock).toHaveBeenCalledTimes(1); - - // We check this mostly for the readability of this test. - expect(streamProvider.isConnected()).toBe(true); - expect(streamProvider.chainId).toBe('0x0'); - - 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' }, - }); - }); - - // 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(); - }); - - 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' }, - }); - }); - - expect(emitSpy).toHaveBeenCalledTimes(2); - expect(emitSpy).toHaveBeenNthCalledWith(1, 'connect', { - chainId: '0x1', - }); - expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x1'); - - expect(streamProvider.isConnected()).toBe(true); - expect(streamProvider.chainId).toBe('0x1'); - }); }); }); }); diff --git a/src/StreamProvider.ts b/src/StreamProvider.ts index 2a3b52cf..c76c665c 100644 --- a/src/StreamProvider.ts +++ b/src/StreamProvider.ts @@ -159,7 +159,7 @@ export abstract class AbstractStreamProvider extends BaseProvider { this.emit('error', warningMsg); } - this._handleDisconnect(false, error ? error.message : undefined); + this._handleDisconnect(error ? error.message : undefined); } /** @@ -182,7 +182,6 @@ export abstract class AbstractStreamProvider extends BaseProvider { return; } - // should disconnect/state reset happen somehow still? super._handleChainChanged({ chainId }); } } diff --git a/src/messages.ts b/src/messages.ts index 2e7f1c35..c6869d1e 100644 --- a/src/messages.ts +++ b/src/messages.ts @@ -1,7 +1,5 @@ const messages = { errors: { - disconnected: () => - 'MetaMask: Disconnected from chain. Attempting to connect.', permanentlyDisconnected: () => 'MetaMask: Disconnected from MetaMask background. Page reload required.', sendSiteMetadata: () => From 85335f466695316aa9e5cc91c0424a47ea9f71e3 Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Wed, 6 Mar 2024 15:45:49 -0800 Subject: [PATCH 09/10] update jest coverage --- jest.config.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/jest.config.js b/jest.config.js index 677079b5..dc1a9fdf 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: 65.43, - functions: 65.65, - lines: 66.74, - statements: 66.81, + branches: 61.45, + functions: 63.15, + lines: 63.41, + statements: 63.48, }, }, From 10ee3a398fb87a479e656474e0fd80339e4eb96c Mon Sep 17 00:00:00 2001 From: Jiexi Luan Date: Tue, 12 Mar 2024 14:24:23 -0700 Subject: [PATCH 10/10] add networkVersion error --- jest.config.js | 6 +++--- src/MetaMaskInpageProvider.test.ts | 14 ++++++++++++++ src/MetaMaskInpageProvider.ts | 4 ++++ src/messages.ts | 2 ++ 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/jest.config.js b/jest.config.js index dc1a9fdf..67f21b34 100644 --- a/jest.config.js +++ b/jest.config.js @@ -46,9 +46,9 @@ const baseConfig = { coverageThreshold: { global: { branches: 61.45, - functions: 63.15, - lines: 63.41, - statements: 63.48, + functions: 63.91, + lines: 63.59, + statements: 63.65, }, }, diff --git a/src/MetaMaskInpageProvider.test.ts b/src/MetaMaskInpageProvider.test.ts index 3579b108..045ea2dc 100644 --- a/src/MetaMaskInpageProvider.test.ts +++ b/src/MetaMaskInpageProvider.test.ts @@ -999,6 +999,20 @@ describe('MetaMaskInpageProvider: Miscellanea', () => { }); }); + describe('networkVersion', () => { + let provider: any | MetaMaskInpageProvider; + + beforeEach(async () => { + provider = (await getInitializedProvider()).provider; + }); + + it('should throw an error when accessing networkVersion', () => { + expect(() => provider.networkVersion).toThrow( + `'ethereum.networkVersion' has been removed`, + ); + }); + }); + describe('selectedAddress', () => { let provider: any | MetaMaskInpageProvider; diff --git a/src/MetaMaskInpageProvider.ts b/src/MetaMaskInpageProvider.ts index add5a9af..1fbba239 100644 --- a/src/MetaMaskInpageProvider.ts +++ b/src/MetaMaskInpageProvider.ts @@ -157,6 +157,10 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { throw new Error(messages.errors.invalidPropertyChainId()); } + get networkVersion(): string | null { + throw new Error(messages.errors.invalidPropertyNetworkVersion()); + } + get selectedAddress(): string | null { throw new Error(messages.errors.invalidPropertySelectedAddress()); } diff --git a/src/messages.ts b/src/messages.ts index c6869d1e..e43db063 100644 --- a/src/messages.ts +++ b/src/messages.ts @@ -18,6 +18,8 @@ const messages = { `'args.logger' must include required method '${method}'.`, invalidPropertyChainId: () => `MetaMask: 'ethereum.chainId' has been removed. Please use the 'eth_chainId' RPC method instead.\nFor more information, see: https://github.com/MetaMask/metamask-improvement-proposals/discussions/23`, + invalidPropertyNetworkVersion: () => + `MetaMask: 'ethereum.networkVersion' has been removed. Please use the 'net_version' RPC method instead.\nFor more information, see: https://github.com/MetaMask/metamask-improvement-proposals/discussions/23`, invalidPropertySelectedAddress: () => `MetaMask: 'ethereum.selectedAddress' has been removed. Please use the 'eth_accounts' RPC method instead.\nFor more information, see: https://github.com/MetaMask/metamask-improvement-proposals/discussions/23`, },