Skip to content

Commit

Permalink
Remove deprecated properties, networkChanged event, and offline send(…
Browse files Browse the repository at this point in the history 
…) net_version support (#306)

* remove networkVersion from baseProvider

* update streamProvider except disconnect spec

* Remove networkId. Block access to BaseProvider.chainId and selectedAddress. Remove send net_version. Remove networkChanged event

* remove proxy property access

* fix external provider test

* update warning/error messages

* Lint

* Remove recoverable disconnects since they can no longer occur

* update jest coverage

* add networkVersion error

---------

Co-authored-by: Alex Donesky <alex.donesky@consensys.net>
  • Loading branch information
@jiexi @adonesky1
jiexi and adonesky1 authored Mar 12, 2024
1 parent 2426930 commit 25b7e4b
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 425 deletions.
8 changes: 4 additions & 4 deletions jest.config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.91,
lines: 63.59,
statements: 63.65,
},
},

Expand Down
55 changes: 16 additions & 39 deletions src/BaseProvider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,33 +230,28 @@ 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.
*/
protected _initializeState(initialState?: {
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);
}
Expand Down Expand Up @@ -329,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);
}
Expand All @@ -369,18 +351,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;
Expand Down
155 changes: 12 additions & 143 deletions src/MetaMaskInpageProvider.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<
Expand Down Expand Up @@ -78,7 +73,6 @@ async function getInitializedProvider({
accounts,
chainId,
isUnlocked,
networkVersion,
},
}),
);
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -748,84 +735,9 @@ describe('MetaMaskInpageProvider: RPC', () => {
connectionStream.notify(MetaMaskInpageProviderStreamName, {
jsonrpc: '2.0',
method: 'metamask_chainChanged',
params: { chainId: '0x1', networkVersion: '1' },
});
});
});

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<void>((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<void>((resolve) => {
provider.once('chainChanged', (newChainId) => {
expect(newChainId).toBe('0x1');
resolve();
});

connectionStream.notify(MetaMaskInpageProviderStreamName, {
jsonrpc: '2.0',
method: 'metamask_chainChanged',
params: { chainId: '0x1', networkVersion: '1' },
params: { chainId: '0x1' },
});
});

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');
});
});

Expand Down Expand Up @@ -1028,7 +940,6 @@ describe('MetaMaskInpageProvider: Miscellanea', () => {
accounts: ['0xabc'],
chainId: '0x0',
isUnlocked: true,
networkVersion: '0',
};
});

Expand All @@ -1037,9 +948,6 @@ describe('MetaMaskInpageProvider: Miscellanea', () => {

await new Promise<void>((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);
});
});
Expand Down Expand Up @@ -1084,52 +992,24 @@ 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,
it('should throw an error when accessing chainId', () => {
expect(() => provider.chainId).toThrow(
`'ethereum.chainId' has been removed`,
);
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;
provider = (await getInitializedProvider()).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 networkVersion', () => {
expect(() => provider.networkVersion).toThrow(
`'ethereum.networkVersion' has been removed`,
);
expect(provider.networkVersion).toBe('5');
});
});

Expand All @@ -1146,21 +1026,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');
});
});
});
Loading

0 comments on commit 25b7e4b

Please sign in to comment.