From 444f4ba9983fce67088b9f1d42fc68f608b8a0e3 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 23 Jan 2025 16:00:23 +0100 Subject: [PATCH 1/9] refactor(StreamProvider): remove unnecessary stream multiplexing --- src/MetaMaskInpageProvider.ts | 2 -- src/StreamProvider.ts | 22 ++------------ .../createExternalExtensionProvider.ts | 15 ++++++++-- src/initializeInpageProvider.ts | 30 ++++++++++++++----- 4 files changed, 37 insertions(+), 32 deletions(-) diff --git a/src/MetaMaskInpageProvider.ts b/src/MetaMaskInpageProvider.ts index 0fd477b2..eb57c29c 100644 --- a/src/MetaMaskInpageProvider.ts +++ b/src/MetaMaskInpageProvider.ts @@ -97,14 +97,12 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { constructor( connectionStream: Duplex, { - jsonRpcStreamName = MetaMaskInpageProviderStreamName, logger = console, maxEventListeners = 100, shouldSendMetadata, }: MetaMaskInpageProviderOptions = {}, ) { super(connectionStream, { - jsonRpcStreamName, logger, maxEventListeners, rpcMiddleware: getDefaultExternalMiddleware(logger), diff --git a/src/StreamProvider.ts b/src/StreamProvider.ts index 60afc8df..8d6fec30 100644 --- a/src/StreamProvider.ts +++ b/src/StreamProvider.ts @@ -1,6 +1,5 @@ import type { JsonRpcMiddleware } from '@metamask/json-rpc-engine'; import { createStreamMiddleware } from '@metamask/json-rpc-middleware-stream'; -import ObjectMultiplex from '@metamask/object-multiplex'; import type SafeEventEmitter from '@metamask/safe-event-emitter'; import type { Json, JsonRpcParams } from '@metamask/utils'; import { duplex as isDuplex } from 'is-stream'; @@ -16,12 +15,7 @@ import { isValidNetworkVersion, } from './utils'; -export type StreamProviderOptions = { - /** - * The name of the stream used to connect to the wallet. - */ - jsonRpcStreamName: string; -} & BaseProviderOptions; +export type StreamProviderOptions = BaseProviderOptions; export type JsonRpcConnection = { events: SafeEventEmitter; @@ -52,7 +46,6 @@ export abstract class AbstractStreamProvider extends BaseProvider { constructor( connectionStream: Duplex, { - jsonRpcStreamName, logger = console, maxEventListeners = 100, rpcMiddleware = [], @@ -67,15 +60,6 @@ export abstract class AbstractStreamProvider extends BaseProvider { // Bind functions to prevent consumers from making unbound calls this._handleStreamDisconnect = this._handleStreamDisconnect.bind(this); - // Set up connectionStream multiplexing - const mux = new ObjectMultiplex(); - pipeline( - connectionStream, - mux as unknown as Duplex, - connectionStream, - this._handleStreamDisconnect.bind(this, 'MetaMask'), - ); - // Set up RPC connection // Typecast: The type of `Duplex` is incompatible with the type of // `JsonRpcConnection`. @@ -84,9 +68,9 @@ export abstract class AbstractStreamProvider extends BaseProvider { }) as unknown as JsonRpcConnection; pipeline( + connectionStream, this._jsonRpcConnection.stream, - mux.createStream(jsonRpcStreamName) as unknown as Duplex, - this._jsonRpcConnection.stream, + connectionStream, this._handleStreamDisconnect.bind(this, 'MetaMask RpcProvider'), ); diff --git a/src/extension-provider/createExternalExtensionProvider.ts b/src/extension-provider/createExternalExtensionProvider.ts index b8bd9171..ececfba2 100644 --- a/src/extension-provider/createExternalExtensionProvider.ts +++ b/src/extension-provider/createExternalExtensionProvider.ts @@ -1,6 +1,7 @@ +import ObjectMultiplex from '@metamask/object-multiplex'; import { detect } from 'detect-browser'; import { PortDuplexStream as PortStream } from 'extension-port-stream'; -import type { Duplex } from 'readable-stream'; +import { pipeline } from 'readable-stream'; import type { Runtime } from 'webextension-polyfill'; import config from './external-extension-config.json'; @@ -28,8 +29,16 @@ export function createExternalExtensionProvider( const metamaskPort = chrome.runtime.connect(extensionId) as Runtime.Port; const pluginStream = new PortStream(metamaskPort); - provider = new StreamProvider(pluginStream as unknown as Duplex, { - jsonRpcStreamName: MetaMaskInpageProviderStreamName, + const streamName = MetaMaskInpageProviderStreamName; + const mux = new ObjectMultiplex(); + pipeline(pluginStream, mux, pluginStream, (error: Error | null) => { + let warningMsg = `MetaMask: Lost connection to "${streamName}".`; + if (error?.stack) { + warningMsg += `\n${error.stack}`; + } + console.warn(warningMsg); + }); + provider = new StreamProvider(mux.createStream(streamName), { logger: console, rpcMiddleware: getDefaultExternalMiddleware(console), }); diff --git a/src/initializeInpageProvider.ts b/src/initializeInpageProvider.ts index 91d2a1ed..ef8f64a8 100644 --- a/src/initializeInpageProvider.ts +++ b/src/initializeInpageProvider.ts @@ -1,11 +1,15 @@ -import type { Duplex } from 'readable-stream'; +import ObjectMultiplex from '@metamask/object-multiplex'; +import { type Duplex, pipeline } from 'readable-stream'; import type { CAIP294WalletData } from './CAIP294'; import { announceWallet } from './CAIP294'; import { announceProvider as announceEip6963Provider } from './EIP6963'; import { getBuildType } from './extension-provider/createExternalExtensionProvider'; import type { MetaMaskInpageProviderOptions } from './MetaMaskInpageProvider'; -import { MetaMaskInpageProvider } from './MetaMaskInpageProvider'; +import { + MetaMaskInpageProvider, + MetaMaskInpageProviderStreamName, +} from './MetaMaskInpageProvider'; import { shimWeb3 } from './shimWeb3'; import type { BaseProviderInfo } from './types'; @@ -47,7 +51,7 @@ type InitializeProviderOptions = { */ export function initializeProvider({ connectionStream, - jsonRpcStreamName, + jsonRpcStreamName = MetaMaskInpageProviderStreamName, logger = console, maxEventListeners = 100, providerInfo, @@ -55,12 +59,22 @@ export function initializeProvider({ shouldSetOnWindow = true, shouldShimWeb3 = false, }: InitializeProviderOptions): MetaMaskInpageProvider { - const provider = new MetaMaskInpageProvider(connectionStream, { - jsonRpcStreamName, - logger, - maxEventListeners, - shouldSendMetadata, + const mux = new ObjectMultiplex(); + pipeline(connectionStream, mux, connectionStream, (error: Error | null) => { + let warningMsg = `MetaMask: Lost connection to "${jsonRpcStreamName}".`; + if (error?.stack) { + warningMsg += `\n${error.stack}`; + } + console.warn(warningMsg); }); + const provider = new MetaMaskInpageProvider( + mux.createStream(jsonRpcStreamName), + { + logger, + maxEventListeners, + shouldSendMetadata, + }, + ); const proxiedProvider = new Proxy(provider, { // some common libraries, e.g. web3@1.x, mess with our API From e7d69150982e337c015321de45fd5e9cd92e3068 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 23 Jan 2025 16:42:32 +0100 Subject: [PATCH 2/9] fix: StreamProvider unit tests --- src/MetaMaskInpageProvider.ts | 2 -- src/StreamProvider.test.ts | 24 ++++++++++++------- src/StreamProvider.ts | 3 +-- .../createExternalExtensionProvider.ts | 2 +- src/initializeInpageProvider.ts | 2 +- 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/MetaMaskInpageProvider.ts b/src/MetaMaskInpageProvider.ts index eb57c29c..c919fb3f 100644 --- a/src/MetaMaskInpageProvider.ts +++ b/src/MetaMaskInpageProvider.ts @@ -86,8 +86,6 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider { * * @param connectionStream - A Node.js duplex stream. * @param options - An options bag. - * @param options.jsonRpcStreamName - The name of the internal JSON-RPC stream. - * Default: `metamask-provider`. * @param options.logger - The logging API to use. Default: `console`. * @param options.maxEventListeners - The maximum number of event * listeners. Default: 100. diff --git a/src/StreamProvider.test.ts b/src/StreamProvider.test.ts index a2a7c06f..60b14b4f 100644 --- a/src/StreamProvider.test.ts +++ b/src/StreamProvider.test.ts @@ -1,5 +1,7 @@ import type { JsonRpcMiddleware } from '@metamask/json-rpc-engine'; +import ObjectMultiplex from '@metamask/object-multiplex'; import type { Json, JsonRpcParams } from '@metamask/utils'; +import { pipeline } from 'readable-stream'; import messages from './messages'; import { StreamProvider } from './StreamProvider'; @@ -20,7 +22,6 @@ function getStreamProvider( ) { const mockStream = new MockConnectionStream(); const streamProvider = new StreamProvider(mockStream, { - jsonRpcStreamName: mockStreamName, rpcMiddleware, }); @@ -38,9 +39,7 @@ describe('StreamProvider', () => { const networkVersion = '1'; const isUnlocked = true; - const streamProvider = new StreamProvider(new MockConnectionStream(), { - jsonRpcStreamName: mockStreamName, - }); + const streamProvider = new StreamProvider(new MockConnectionStream()); const requestMock = jest .spyOn(streamProvider, 'request') @@ -370,10 +369,13 @@ describe('StreamProvider', () => { describe('events', () => { it('calls chainChanged when the chainId changes', async () => { const mockStream = new MockConnectionStream(); - const streamProvider = new StreamProvider(mockStream, { - jsonRpcStreamName: mockStreamName, + const mux = new ObjectMultiplex(); + pipeline(mockStream, mux, mockStream, (error: Error | null) => { + console.error(error); }); - + const streamProvider = new StreamProvider( + mux.createStream(mockStreamName), + ); const requestMock = jest .spyOn(streamProvider, 'request') .mockImplementationOnce(async () => { @@ -404,9 +406,13 @@ describe('StreamProvider', () => { it('handles chain changes with intermittent disconnection', async () => { const mockStream = new MockConnectionStream(); - const streamProvider = new StreamProvider(mockStream, { - jsonRpcStreamName: mockStreamName, + const mux = new ObjectMultiplex(); + pipeline(mockStream, mux, mockStream, (error: Error | null) => { + console.error(error); }); + const streamProvider = new StreamProvider( + mux.createStream(mockStreamName), + ); const requestMock = jest .spyOn(streamProvider, 'request') diff --git a/src/StreamProvider.ts b/src/StreamProvider.ts index 8d6fec30..acc4c942 100644 --- a/src/StreamProvider.ts +++ b/src/StreamProvider.ts @@ -37,7 +37,6 @@ export abstract class AbstractStreamProvider extends BaseProvider { * * @param connectionStream - A Node.js duplex stream. * @param options - An options bag. - * @param options.jsonRpcStreamName - The name of the internal JSON-RPC stream. * @param options.logger - The logging API to use. Default: `console`. * @param options.maxEventListeners - The maximum number of event * listeners. Default: 100. @@ -49,7 +48,7 @@ export abstract class AbstractStreamProvider extends BaseProvider { logger = console, maxEventListeners = 100, rpcMiddleware = [], - }: StreamProviderOptions, + }: StreamProviderOptions = {}, ) { super({ logger, maxEventListeners, rpcMiddleware }); diff --git a/src/extension-provider/createExternalExtensionProvider.ts b/src/extension-provider/createExternalExtensionProvider.ts index ececfba2..b42e74c4 100644 --- a/src/extension-provider/createExternalExtensionProvider.ts +++ b/src/extension-provider/createExternalExtensionProvider.ts @@ -32,7 +32,7 @@ export function createExternalExtensionProvider( const streamName = MetaMaskInpageProviderStreamName; const mux = new ObjectMultiplex(); pipeline(pluginStream, mux, pluginStream, (error: Error | null) => { - let warningMsg = `MetaMask: Lost connection to "${streamName}".`; + let warningMsg = `Lost connection to "${streamName}".`; if (error?.stack) { warningMsg += `\n${error.stack}`; } diff --git a/src/initializeInpageProvider.ts b/src/initializeInpageProvider.ts index ef8f64a8..edb4257c 100644 --- a/src/initializeInpageProvider.ts +++ b/src/initializeInpageProvider.ts @@ -61,7 +61,7 @@ export function initializeProvider({ }: InitializeProviderOptions): MetaMaskInpageProvider { const mux = new ObjectMultiplex(); pipeline(connectionStream, mux, connectionStream, (error: Error | null) => { - let warningMsg = `MetaMask: Lost connection to "${jsonRpcStreamName}".`; + let warningMsg = `Lost connection to "${jsonRpcStreamName}".`; if (error?.stack) { warningMsg += `\n${error.stack}`; } From 5e851400bf66c09bd5ef422e7cf6c369c2334fb5 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 23 Jan 2025 16:49:12 +0100 Subject: [PATCH 3/9] fix: inpage provider unit tests --- src/MetaMaskInpageProvider.test.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/MetaMaskInpageProvider.test.ts b/src/MetaMaskInpageProvider.test.ts index 2dcbc93e..e6d9cdce 100644 --- a/src/MetaMaskInpageProvider.test.ts +++ b/src/MetaMaskInpageProvider.test.ts @@ -1,4 +1,6 @@ +import ObjectMultiplex from '@metamask/object-multiplex'; import type { JsonRpcRequest } from '@metamask/utils'; +import { pipeline } from 'readable-stream'; import messages from './messages'; import { @@ -64,14 +66,14 @@ async function getInitializedProvider({ const onWrite = jest.fn(); const connectionStream = new MockConnectionStream((name, data) => { if ( - name === 'metamask-provider' && + name === MetaMaskInpageProviderStreamName && data.method === 'metamask_getProviderState' ) { // Wrap in `setTimeout` to ensure a reply is received by the provider // after the provider has processed the request, to ensure that the // provider recognizes the id. setTimeout(() => - connectionStream.reply('metamask-provider', { + connectionStream.reply(MetaMaskInpageProviderStreamName, { id: onWrite.mock.calls[0][1].id, jsonrpc: '2.0', result: { @@ -93,8 +95,13 @@ async function getInitializedProvider({ } onWrite(name, data); }); - - const provider = new MetaMaskInpageProvider(connectionStream); + const mux = new ObjectMultiplex(); + pipeline(connectionStream, mux, connectionStream, (error: Error | null) => { + console.error(error); + }); + const provider = new MetaMaskInpageProvider( + mux.createStream(MetaMaskInpageProviderStreamName), + ); await new Promise((resolve: () => void) => { provider.on('_initialized', resolve); }); From 745d3c526f285dd0e8a12b9cbc269096faaeebfc Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 23 Jan 2025 16:57:51 +0100 Subject: [PATCH 4/9] fix: adjust coverage (will be reverted) --- jest.config.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/jest.config.js b/jest.config.js index f73183f3..5a7a5578 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.6, - functions: 69.91, - lines: 69.51, - statements: 69.52, + branches: 66.79, + functions: 68.69, + lines: 68.35, + statements: 68.38, }, }, From 0e364e37d86d4be2ee541820a7a3a36266ca6831 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 23 Jan 2025 17:16:51 +0100 Subject: [PATCH 5/9] fix: in page providerconstructor type --- src/MetaMaskInpageProvider.ts | 4 +--- src/initializeInpageProvider.ts | 4 ++++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/MetaMaskInpageProvider.ts b/src/MetaMaskInpageProvider.ts index c919fb3f..b8156369 100644 --- a/src/MetaMaskInpageProvider.ts +++ b/src/MetaMaskInpageProvider.ts @@ -29,9 +29,7 @@ export type MetaMaskInpageProviderOptions = { * Whether the provider should send page metadata. */ shouldSendMetadata?: boolean; - - jsonRpcStreamName?: string | undefined; -} & Partial>; +} & Partial>; type SentWarningsState = { // methods diff --git a/src/initializeInpageProvider.ts b/src/initializeInpageProvider.ts index edb4257c..79165184 100644 --- a/src/initializeInpageProvider.ts +++ b/src/initializeInpageProvider.ts @@ -33,6 +33,10 @@ type InitializeProviderOptions = { * Whether the window.web3 shim should be set. */ shouldShimWeb3?: boolean; + /** + * The name of the stream used to connect to the wallet. + */ + jsonRpcStreamName?: string | undefined; } & MetaMaskInpageProviderOptions; /** From f21d924a04d43b3fb1768b27722e82237018f411 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 23 Jan 2025 18:04:01 +0100 Subject: [PATCH 6/9] fix: add changelog --- CHANGELOG.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fca7c949..0b184cec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **BREAKING**: `StreamProvider` no longer accepts a `jsonRpcStreamName` parameter ([#400](https://github.com/MetaMask/providers/pull/400)) + - Previously, this parameter was used internally to create an ObjectMultiplex stream and substream for JSON-RPC communication + - Now, the consumer is responsible for creating and managing the stream multiplexing if needed + - The provider will use the provided stream connection directly without any multiplexing + +- **BREAKING**: MetaMaskInpageProvider no longer accepts a `jsonRpcStreamName` parameter ([#400](https://github.com/MetaMask/providers/pull/400)) + - This change is inherited from StreamProvider, as MetaMaskInpageProvider extends StreamProvider + - Stream multiplexing should be handled before provider instantiation +- `initializeInpageProvider` now handles stream multiplexing internally ([#400](https://github.com/MetaMask/providers/pull/400)) + - Creates an ObjectMultiplex instance and substream using the provided `jsonRpcStreamName` + - This maintains backwards compatibility for consumers using `initializeInpageProvider` +- `createExternalExtensionProvider` now handles stream multiplexing internally ([#400](https://github.com/MetaMask/providers/pull/400)) + - Creates an ObjectMultiplex instance and substream for JSON-RPC communication + - This maintains backwards compatibility for consumers using `createExternalExtensionProvider` + ## [18.3.1] ### Changed From 8ea8b05af533b5b64bdd51de7d082e23901c946c Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 23 Jan 2025 18:04:45 +0100 Subject: [PATCH 7/9] fix: changelog linting --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b184cec..dc42f266 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Previously, this parameter was used internally to create an ObjectMultiplex stream and substream for JSON-RPC communication - Now, the consumer is responsible for creating and managing the stream multiplexing if needed - The provider will use the provided stream connection directly without any multiplexing - - **BREAKING**: MetaMaskInpageProvider no longer accepts a `jsonRpcStreamName` parameter ([#400](https://github.com/MetaMask/providers/pull/400)) - This change is inherited from StreamProvider, as MetaMaskInpageProvider extends StreamProvider - Stream multiplexing should be handled before provider instantiation From fb1bb49b1ad0b86751eb37614ceacb325847acb9 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 23 Jan 2025 18:12:12 +0100 Subject: [PATCH 8/9] fix: changelog highlight in page provider --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc42f266..e829f2e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Previously, this parameter was used internally to create an ObjectMultiplex stream and substream for JSON-RPC communication - Now, the consumer is responsible for creating and managing the stream multiplexing if needed - The provider will use the provided stream connection directly without any multiplexing -- **BREAKING**: MetaMaskInpageProvider no longer accepts a `jsonRpcStreamName` parameter ([#400](https://github.com/MetaMask/providers/pull/400)) +- **BREAKING**: `MetaMaskInpageProvider` no longer accepts a `jsonRpcStreamName` parameter ([#400](https://github.com/MetaMask/providers/pull/400)) - This change is inherited from StreamProvider, as MetaMaskInpageProvider extends StreamProvider - Stream multiplexing should be handled before provider instantiation - `initializeInpageProvider` now handles stream multiplexing internally ([#400](https://github.com/MetaMask/providers/pull/400)) From b4e0c12e8087a0c7d20a77f27e68a8c8e788480c Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Thu, 23 Jan 2025 18:16:01 +0100 Subject: [PATCH 9/9] fix: remove useless undefined for jsonRpcStreamName --- src/initializeInpageProvider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/initializeInpageProvider.ts b/src/initializeInpageProvider.ts index 79165184..79e59b4e 100644 --- a/src/initializeInpageProvider.ts +++ b/src/initializeInpageProvider.ts @@ -36,7 +36,7 @@ type InitializeProviderOptions = { /** * The name of the stream used to connect to the wallet. */ - jsonRpcStreamName?: string | undefined; + jsonRpcStreamName?: string; } & MetaMaskInpageProviderOptions; /**