Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Moved subscribe and filter into network controller (#16693)" #18129

Merged
merged 4 commits into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 1 addition & 23 deletions app/scripts/controllers/network/network-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,13 @@ import { strict as assert } from 'assert';
import EventEmitter from 'events';
import { ComposedStore, ObservableStore } from '@metamask/obs-store';
import { JsonRpcEngine } from 'json-rpc-engine';
import {
providerFromEngine,
providerFromMiddleware,
} from '@metamask/eth-json-rpc-middleware';
import { providerFromEngine } from '@metamask/eth-json-rpc-middleware';
import log from 'loglevel';
import {
createSwappableProxy,
createEventEmitterProxy,
} from 'swappable-obj-proxy';
import EthQuery from 'eth-query';
import createFilterMiddleware from 'eth-json-rpc-filters';
import createSubscriptionManager from 'eth-json-rpc-filters/subscriptionManager';
import { v4 as random } from 'uuid';
import {
INFURA_PROVIDER_TYPES,
Expand Down Expand Up @@ -460,26 +455,9 @@ export default class NetworkController extends EventEmitter {
}

_setNetworkClient({ networkMiddleware, blockTracker }) {
const networkProvider = providerFromMiddleware(networkMiddleware);
const filterMiddleware = createFilterMiddleware({
provider: networkProvider,
blockTracker,
});
const subscriptionManager = createSubscriptionManager({
provider: networkProvider,
blockTracker,
});

const engine = new JsonRpcEngine();
subscriptionManager.events.on('notification', (message) =>
engine.emit('notification', message),
);
engine.push(filterMiddleware);
engine.push(subscriptionManager.middleware);
engine.push(networkMiddleware);

const provider = providerFromEngine(engine);

this._setProviderAndBlockTracker({ provider, blockTracker });
}

Expand Down
2 changes: 0 additions & 2 deletions app/scripts/controllers/permissions/specifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -260,12 +260,10 @@ export const unrestrictedMethods = Object.freeze([
'eth_signTypedData_v1',
'eth_signTypedData_v3',
'eth_signTypedData_v4',
'eth_subscribe',
'eth_submitHashrate',
'eth_submitWork',
'eth_syncing',
'eth_uninstallFilter',
'eth_unsubscribe',
'metamask_getProviderState',
'metamask_watchAsset',
'net_listening',
Expand Down
25 changes: 16 additions & 9 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
KeyringController,
keyringBuilderFactory,
} from '@metamask/eth-keyring-controller';
import createFilterMiddleware from 'eth-json-rpc-filters';
import createSubscriptionManager from 'eth-json-rpc-filters/subscriptionManager';
import {
errorCodes as rpcErrorCodes,
EthereumRpcError,
Expand Down Expand Up @@ -3898,19 +3900,21 @@ export default class MetamaskController extends EventEmitter {
* @param {tabId} [options.tabId] - The tab ID of the sender - if the sender is within a tab
*/
setupProviderEngine({ origin, subjectType, sender, tabId }) {
const { provider } = this;

// setup json rpc engine stack
const engine = new JsonRpcEngine();
const { blockTracker, provider } = this;

// forward notifications from network provider
provider.on('data', (error, message) => {
if (error) {
// This should never happen, this error parameter is never set
throw error;
}
engine.emit('notification', message);
// create filter polyfill middleware
const filterMiddleware = createFilterMiddleware({ provider, blockTracker });

// create subscription polyfill middleware
const subscriptionManager = createSubscriptionManager({
provider,
blockTracker,
});
subscriptionManager.events.on('notification', (message) =>
engine.emit('notification', message),
);

if (isManifestV3) {
engine.push(createDupeReqFilterMiddleware());
Expand Down Expand Up @@ -4063,6 +4067,9 @@ export default class MetamaskController extends EventEmitter {
);
///: END:ONLY_INCLUDE_IN

// filter and subscription polyfills
engine.push(filterMiddleware);
engine.push(subscriptionManager.middleware);
if (subjectType !== SubjectType.Internal) {
// permissions
engine.push(
Expand Down
78 changes: 78 additions & 0 deletions test/e2e/tests/eth-subscribe.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
const { convertToHexValue, withFixtures } = require('../helpers');
const FixtureBuilder = require('../fixture-builder');

describe('eth_subscribe', function () {
const ganacheOptions = {
accounts: [
{
secretKey:
'0x7C9529A67102755B7E6102D6D950AC5D5863C98713805CEC576B945B15B71EAC',
balance: convertToHexValue(25000000000000000000),
},
],
};

it('only broadcasts subscription notifications on the page that registered the subscription', async function () {
await withFixtures(
{
dapp: true,
fixtures: new FixtureBuilder()
.withPermissionControllerConnectedToTestDapp()
.build(),
ganacheOptions,
dappOptions: { numberOfDapps: 2 },
title: this.test.title,
},
async ({ driver }) => {
await driver.navigate();
await driver.fill('#password', 'correct horse battery staple');
await driver.press('#password', driver.Key.ENTER);

await driver.openNewPage('http://127.0.0.1:8080/');

const setupSubscriptionListener = `
const responseContainer = document.createElement('div');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can replace this with the built-in test dapp subscribe functionality after this PR is merged: MetaMask/test-dapp#202

responseContainer.setAttribute('id', 'eth-subscribe-responses');

const body = window.document.getElementsByTagName('body')[0];
body.appendChild(responseContainer);

window.ethereum.addListener('message', (message) => {
if (message.type === 'eth_subscription') {
const response = document.createElement('p');
response.setAttribute('data-testid', 'eth-subscribe-response');
response.append(JSON.stringify(message.data.result));

const responses = window.document.getElementById('eth-subscribe-responses');
responses.appendChild(response)
}
});
`;

await driver.executeScript(setupSubscriptionListener);
// A `newHeads` subscription will emit a notification for each new block
// See here for more information: https://docs.infura.io/infura/networks/ethereum/json-rpc-methods/subscription-methods/eth_subscribe
await driver.executeScript(`
window.ethereum.request({
method: 'eth_subscribe',
params: ['newHeads']
});
`);

// Verify that the new block is seen on the first dapp
await driver.findElement('[data-testid="eth-subscribe-response"]');

// Switch to the second dapp
await driver.openNewPage('http://127.0.0.1:8081/');

// Setup the same subscription listener as on the first dapp, but without registering a new subscription
await driver.executeScript(setupSubscriptionListener);

// Verify that the new block is not seen on the second dapp
await driver.assertElementNotPresent(
'[data-testid="eth-subscribe-response"]',
);
},
);
});
});