Skip to content

Commit

Permalink
Fix accountsChanged notification (#1413)
Browse files Browse the repository at this point in the history 
* use rpc-engine notifications for accountsChanged

* only notify current web page

Co-authored-by: Esteban Miño <[email protected]>
  • Loading branch information
@rekmarks @estebanmino
rekmarks and estebanmino authored Mar 10, 2020
1 parent 9355c1f commit b64f9fe
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 28 deletions.
63 changes: 51 additions & 12 deletions app/components/Views/BrowserTab/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ import { resemblesAddress } from '../../../util/address';
import createAsyncMiddleware from 'json-rpc-engine/src/createAsyncMiddleware';
import { ethErrors } from 'eth-json-rpc-errors';

const { HOMEPAGE_URL, USER_AGENT } = AppConstants;
const { HOMEPAGE_URL, USER_AGENT, NOTIFICATION_NAMES } = AppConstants;
const HOMEPAGE_HOST = 'home.metamask.io';

const styles = StyleSheet.create({
Expand Down Expand Up @@ -454,15 +454,34 @@ export class BrowserTab extends PureComponent {
webview: this.webview,
url,
getRpcMethodMiddleware: this.getRpcMethodMiddleware.bind(this),
shouldExposeAccounts: hostname => {
const { approvedHosts, privacyMode } = this.props;
return !privacyMode || approvedHosts[hostname];
},
isMainFrame
});
this.backgroundBridges.push(newBridge);
};

notifyConnection = (payload, hostname, restricted = true) => {
const { privacyMode, approvedHosts } = this.props;

// TODO:permissions move permissioning logic elsewhere
this.backgroundBridges.forEach(bridge => {
if (bridge.hostname === hostname && (!restricted || !privacyMode || approvedHosts[bridge.hostname])) {
bridge.sendNotification(payload);
}
});
};

notifyAllConnections = (payload, restricted = true) => {
const { privacyMode, approvedHosts } = this.props;
const { fullHostname } = this.state;

// TODO:permissions move permissioning logic elsewhere
this.backgroundBridges.forEach(bridge => {
if (bridge.hostname === fullHostname && (!privacyMode || !restricted || approvedHosts[bridge.hostname])) {
bridge.sendNotification(payload);
}
});
};

getRpcMethodMiddleware = ({ hostname }) =>
// all user facing RPC calls not implemented by the provider
createAsyncMiddleware(async (req, res, next) => {
Expand Down Expand Up @@ -492,11 +511,6 @@ export class BrowserTab extends PureComponent {

if (approved) {
res.result = [selectedAddress.toLowerCase()];
this.backgroundBridges.forEach(bridge => {
if (bridge.hostname === hostname) {
bridge.emit('update');
}
});
} else {
throw ethErrors.provider.userRejectedRequest('User denied account authorization.');
}
Expand Down Expand Up @@ -845,8 +859,12 @@ export class BrowserTab extends PureComponent {
}

componentDidUpdate(prevProps) {
const prevNavigation = prevProps.navigation;
const { navigation } = this.props;
const {
approvedHosts: prevApprovedHosts,
navigation: prevNavigation,
selectedAddress: prevSelectedAddress
} = prevProps;
const { approvedHosts, navigation, selectedAddress } = this.props;

// If tab wasn't activated and we detect an tab change
// we need to check if it's time to activate the tab
Expand All @@ -864,6 +882,27 @@ export class BrowserTab extends PureComponent {
this.loadUrl();
}
}

const numApprovedHosts = Object.keys(approvedHosts).length;
const prevNumApprovedHosts = Object.keys(prevApprovedHosts).length;

// this will happen if the approved hosts were cleared
if (numApprovedHosts === 0 && prevNumApprovedHosts > 0) {
this.notifyAllConnections(
{
method: NOTIFICATION_NAMES.accountsChanged,
result: []
},
false
); // notification should be sent regardless of approval status
}

if (numApprovedHosts > 0 && selectedAddress !== prevSelectedAddress) {
this.notifyAllConnections({
method: NOTIFICATION_NAMES.accountsChanged,
result: [selectedAddress]
});
}
}

componentWillUnmount() {
Expand Down
5 changes: 4 additions & 1 deletion app/core/AppConstants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,8 @@ export default {
INSTAPAY_GAS_PONDERATOR: 1.2,
USER_AGENT: Device.isAndroid()
? 'Mozilla/5.0 (Linux; Android 10; Android SDK built for x86 Build/OSM1.180201.023) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/77.0.3865.92 Mobile Safari/537.36'
: 'Mozilla/5.0 (iPhone; CPU iPhone OS 13_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) CriOS/76.0.3809.123 Mobile/15E148 Safari/605.1'
: 'Mozilla/5.0 (iPhone; CPU iPhone OS 13_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) CriOS/76.0.3809.123 Mobile/15E148 Safari/605.1',
NOTIFICATION_NAMES: {
accountsChanged: 'wallet_accountsChanged'
}
};
28 changes: 13 additions & 15 deletions app/core/BackgroundBridge.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,20 @@ class Port extends EventEmitter {
}

export class BackgroundBridge extends EventEmitter {
constructor({ webview, url, getRpcMethodMiddleware, shouldExposeAccounts, isMainFrame }) {
constructor({ webview, url, getRpcMethodMiddleware, isMainFrame }) {
super();
this.url = url;
this.hostname = new URL(url).hostname;
this.isMainFrame = isMainFrame;
this._webviewRef = webview && webview.current;

this.createMiddleware = getRpcMethodMiddleware;
this.shouldExposeAccounts = shouldExposeAccounts;
this.provider = Engine.context.NetworkController.provider;
this.blockTracker = this.provider._blockTracker;
this.port = new Port(this._webviewRef, isMainFrame);

this.engine = null;

const portStream = new MobilePortStream(this.port, url);
// setup multiplexing
const mux = setupMultiplex(portStream);
Expand Down Expand Up @@ -81,14 +82,14 @@ export class BackgroundBridge extends EventEmitter {
* @param {*} outStream - The stream to provide over.
*/
setupProviderConnection(outStream) {
const engine = this.setupProviderEngine();
this.engine = this.setupProviderEngine();

// setup connection
const providerStream = createEngineStream({ engine });
const providerStream = createEngineStream({ engine: this.engine });

pump(outStream, providerStream, outStream, err => {
// handle any middleware cleanup
engine._middleware.forEach(mid => {
this.engine._middleware.forEach(mid => {
if (mid.destroy && typeof mid.destroy === 'function') {
mid.destroy();
}
Expand Down Expand Up @@ -146,10 +147,7 @@ export class BackgroundBridge extends EventEmitter {
* @param {*} outStream - The stream to provide public config over.
*/
setupPublicConfig(outStream) {
const configStore = this.createPublicConfigStore({
// check the providerApprovalController's approvedOrigins
checkIsEnabled: () => this.shouldExposeAccounts(this.hostname)
});
const configStore = this.createPublicConfigStore();

const configStream = asStream(configStore);

Expand All @@ -166,19 +164,15 @@ export class BackgroundBridge extends EventEmitter {
* Constructor helper: initialize a public config store.
* This store is used to make some config info available to Dapps synchronously.
*/
createPublicConfigStore({ checkIsEnabled }) {
createPublicConfigStore() {
// subset of state for metamask inpage provider
const publicConfigStore = new ObservableStore();

const selectPublicState = ({ isUnlocked, selectedAddress, network }) => {
const isEnabled = checkIsEnabled();
const isReady = isUnlocked && isEnabled;
const selectPublicState = ({ isUnlocked, network }) => {
const networkType = Engine.context.NetworkController.state.provider.type;
const chainId = Object.keys(NetworkList).indexOf(networkType) > -1 && NetworkList[networkType].chainId;
const result = {
isUnlocked,
isEnabled,
selectedAddress: isReady ? selectedAddress.toLowerCase() : null,
networkVersion: network,
chainId: chainId ? `0x${parseInt(chainId, 10).toString(16)}` : null
};
Expand All @@ -204,6 +198,10 @@ export class BackgroundBridge extends EventEmitter {
return publicConfigStore;
}

sendNotification(payload) {
this.engine && this.engine.emit('notification', payload);
}

/**
* The metamask-state of the various controllers, made available to the UI
*
Expand Down

0 comments on commit b64f9fe

Please sign in to comment.