Skip to content

Commit

Permalink
add accountsChange notification to inpage provider
Browse files Browse the repository at this point in the history
move functionality to inpage provider
update inpage provider
Remove 'Connections' settings page (#7369)
  • Loading branch information
rekmarks committed Nov 12, 2019
1 parent be4bd67 commit 3d2a368
Show file tree
Hide file tree
Showing 23 changed files with 113 additions and 355 deletions.
10 changes: 5 additions & 5 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ workflows:
- test-unit
- test-unit-global
- test-mozilla-lint
# - test-e2e-chrome
# - test-e2e-firefox
- test-e2e-chrome
- test-e2e-firefox
- test-integration-flat-chrome
- test-integration-flat-firefox
- job-publish-prerelease:
Expand All @@ -74,9 +74,9 @@ workflows:
- prep-build
# - prep-docs
- all-tests-pass
# - coveralls-upload:
# requires:
# - test-unit
- coveralls-upload:
requires:
- test-unit

jobs:
create_release_pull_request:
Expand Down
14 changes: 7 additions & 7 deletions app/scripts/contentscript.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const inpageBundle = inpageContent + inpageSuffix
// If we create a FireFox-only code path using that API,
// MetaMask will be much faster loading and performant on Firefox.

if (shouldInjectWeb3()) {
if (shouldInjectProvider()) {
injectScript(inpageBundle)
start()
}
Expand All @@ -37,7 +37,7 @@ function injectScript (content) {
container.insertBefore(scriptTag, container.children[0])
container.removeChild(scriptTag)
} catch (e) {
console.error('MetaMask script injection failed', e)
console.error('MetaMask provider injection failed.', e)
}
}

Expand Down Expand Up @@ -117,11 +117,11 @@ function logStreamDisconnectWarning (remoteLabel, err) {
}

/**
* Determines if Web3 should be injected
* Determines if the provider should be injected
*
* @returns {boolean} {@code true} if Web3 should be injected
* @returns {boolean} {@code true} if the provider should be injected
*/
function shouldInjectWeb3 () {
function shouldInjectProvider () {
return doctypeCheck() && suffixCheck() &&
documentElementCheck() && !blacklistedDomainCheck()
}
Expand All @@ -144,8 +144,8 @@ function doctypeCheck () {
* Returns whether or not the extension (suffix) of the current document is prohibited
*
* This checks {@code window.location.pathname} against a set of file extensions
* that should not have web3 injected into them. This check is indifferent of query parameters
* in the location.
* that we should not inject the provider into. This check is indifferent of
* query parameters in the location.
*
* @returns {boolean} whether or not the extension of the current document is prohibited
*/
Expand Down
18 changes: 14 additions & 4 deletions app/scripts/controllers/permissions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@ const WALLET_METHOD_PREFIX = 'wallet_'
const CAVEAT_NAMES = {
exposedAccounts: 'exposedAccounts',
}
const ACCOUNTS_CHANGED_NOTIFICATION = 'wallet_accountsChanged'

class PermissionsController {

constructor (
{
openPopup, closePopup, notifyDomain, notifyAllDomains, keyringController
openPopup, closePopup, notifyDomain, notifyAllDomains, keyringController,
} = {},
restoredPermissions = {},
restoredState = {}) {
Expand Down Expand Up @@ -141,7 +142,10 @@ class PermissionsController {
origin, 'eth_accounts', CAVEAT_NAMES.exposedAccounts, accounts
)

this.notifyDomain(origin, 'accountsChanged', accounts)
this.notifyDomain(origin, {
method: ACCOUNTS_CHANGED_NOTIFICATION,
result: accounts,
})
}

/**
Expand Down Expand Up @@ -212,7 +216,10 @@ class PermissionsController {
perms.map(methodName => {

if (methodName === 'eth_accounts') {
this.notifyDomain(origin, 'accountsChanged', [])
this.notifyDomain(
origin,
{ method: ACCOUNTS_CHANGED_NOTIFICATION, result: [] }
)
}

return { parentCapability: methodName }
Expand Down Expand Up @@ -249,7 +256,10 @@ class PermissionsController {
*/
clearPermissions () {
this.permissions.clearDomains()
this.notifyAllDomains('accountsChanged', [])
this.notifyAllDomains({
method: ACCOUNTS_CHANGED_NOTIFICATION,
result: [],
})
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"net_version",
"eth_blockNumber",
"eth_call",
"eth_chainId",
"eth_coinbase",
"eth_estimateGas",
"eth_gasPrice",
Expand Down
6 changes: 3 additions & 3 deletions app/scripts/controllers/transactions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,11 @@ class TransactionController extends EventEmitter {
// validate
const normalizedTxParams = txUtils.normalizeTxParams(txParams)

// TODO:lps once, I saw origin with a value of 'chrome-extension://...' for a
// transaction initiated from the UI.
// TODO:lps once, I saw 'origin' with a value of 'chrome-extension://...'
// for a transaction initiated from the UI.
// When would this happen? It's a problem if it does. I don't think we want to
// hard-code 'chrome-extension://' here
if (origin === 'MetaMask') {
if (origin === 'metamask') {
// Assert the from address is the selected address
if (normalizedTxParams.from !== this.getSelectedAddress()) {
throw ethErrors.rpc.internal(`Internally initiated transaction is using invalid account.`)
Expand Down
67 changes: 13 additions & 54 deletions app/scripts/inpage.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,14 @@ const restoreContextAfterImports = () => {
}

cleanContextForImports()
require('web3/dist/web3.min.js')

const log = require('loglevel')
const LocalMessageDuplexStream = require('post-message-stream')
const setupDappAutoReload = require('./lib/auto-reload.js')
const MetamaskInpageProvider = require('metamask-inpage-provider')

let warned = false
// TODO:deprecate:2019-12-16
require('web3/dist/web3.min.js')
const setupDappAutoReload = require('./lib/auto-reload.js')

restoreContextAfterImports()

Expand All @@ -59,53 +60,9 @@ const inpageProvider = new MetamaskInpageProvider(metamaskStream)
// set a high max listener count to avoid unnecesary warnings
inpageProvider.setMaxListeners(100)

// give the dapps control of a refresh they can toggle this off on the window.ethereum
// this will be default true so it does not break any old apps.
inpageProvider.autoRefreshOnNetworkChange = true

// publicConfig isn't populated until we get a message from background.
// Using this getter will ensure the state is available
const getPublicConfigWhenReady = async () => {
const store = inpageProvider.publicConfigStore
let state = store.getState()
// if state is missing, wait for first update
if (!state.hasOwnProperty('isUnlocked')) {
state = await new Promise(resolve => store.once('update', resolve))
}
return state
}

// add metamask-specific convenience methods
inpageProvider._metamask = new Proxy({

/**
* Determines if MetaMask is unlocked by the user
*
* @returns {Promise<boolean>} - Promise resolving to true if MetaMask is currently unlocked
*/
isUnlocked: async function () {
const { isUnlocked } = await getPublicConfigWhenReady()
return Boolean(isUnlocked)
},

/**
* WILL BE DEPRECATED.
* Asynchronously determines if this domain is currently enabled.
*
* @returns {Promise<boolean>} - Promise resolving to true if this domain is currently enabled
*/
isApproved: async function () {
return Boolean(inpageProvider.selectedAddress)
},
}, {
get: function (obj, prop) {
!warned && console.warn('Heads up! ethereum._metamask exposes methods that have ' +
'not been standardized yet. This means that these methods may not be implemented ' +
'in other dapp browsers and may be removed from MetaMask in the future.')
warned = true
return obj[prop]
},
})
//
// TODO:deprecate:2019-12-16
//

// Work around for [email protected] deleting the bound `sendAsync` but not the unbound
// `sendAsync` method on the prototype, causing `this` reference issues
Expand All @@ -115,11 +72,7 @@ const proxiedInpageProvider = new Proxy(inpageProvider, {
deleteProperty: () => true,
})

window.ethereum = proxiedInpageProvider

//
// setup web3
//

if (typeof window.web3 !== 'undefined') {
throw new Error(`MetaMask detected another web3.
Expand All @@ -137,6 +90,12 @@ log.debug('MetaMask - injected web3')

setupDappAutoReload(web3, inpageProvider.publicConfigStore)

//
// end deprecate:2019-12-16
//

window.ethereum = proxiedInpageProvider

inpageProvider.publicConfigStore.subscribe(function (state) {
if (state.onboardingcomplete) {
window.postMessage('onboardingcomplete', '*')
Expand Down
3 changes: 3 additions & 0 deletions app/scripts/lib/auto-reload.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@

// TODO:deprecate:2019-12-16

module.exports = setupDappAutoReload

function setupDappAutoReload (web3, observable) {
Expand Down
45 changes: 22 additions & 23 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,6 @@ module.exports = class MetamaskController extends EventEmitter {
CachedBalancesController: this.cachedBalancesController.store,
OnboardingController: this.onboardingController.store,
IncomingTransactionsController: this.incomingTransactionsController.store,
ThreeBoxController: this.threeBoxController.store,
ABTestController: this.abTestController.store,
PermissionsController: this.permissionsController.permissions,
PermissionsMetadata: this.permissionsController.store,
Expand Down Expand Up @@ -339,7 +338,7 @@ module.exports = class MetamaskController extends EventEmitter {
version,
// account mgmt
getAccounts: async ({ origin }) => {
if (origin === 'MetaMask') {
if (origin === 'metamask') {
return [this.preferencesController.getSelectedAddress()]
} else if (
this.keyringController.memStore.getState().isUnlocked
Expand Down Expand Up @@ -1419,6 +1418,7 @@ module.exports = class MetamaskController extends EventEmitter {
* @param {object} publicApi - The public API
*/
setupProviderConnection (outStream, senderUrl, extensionId) {
const origin = senderUrl.hostname
const engine = this.setupProviderEngine(senderUrl, extensionId)

// setup connection
Expand Down Expand Up @@ -1505,18 +1505,18 @@ module.exports = class MetamaskController extends EventEmitter {
// manage external connections

/**
* Adds a reference to a connection by origin. Ignores the 'MetaMask' origin.
* Adds a reference to a connection by origin. Ignores the 'metamask' origin.
* Caller must ensure that the returned id is stored such that the reference
* can be deleted later.
*
*
* @param {string} origin - The connection's origin string.
* @param {Object} options - Data associated with the connection
* @param {Object} options.engine - The connection's JSON Rpc Engine
* @returns {string} - The connection's id (so that it can be deleted later)
*/
addConnection (origin, { engine }) {

if (origin === 'MetaMask') return null
if (origin === 'metamask') return null

if (!this.connections[origin]) {
this.connections[origin] = {}
Expand All @@ -1533,56 +1533,55 @@ module.exports = class MetamaskController extends EventEmitter {
/**
* Deletes a reference to a connection, by origin and id.
* Ignores unknown origins.
*
*
* @param {string} origin - The connection's origin string.
* @param {string} id - The connection's id, as returned from addConnection.
*/
removeConnection (origin, id) {

if (!this.connections[origin]) return
const connections = this.connections[origin]
if (!connections) return

delete this.connections[origin][id]
delete connections[id]

if (Object.keys(this.connections[origin].length === 0)) {
if (Object.keys(connections.length === 0)) {
delete this.connections[origin]
}
}

/**
* Causes the RPC engines associated with the connections to the given origin
* to emit an event with the given name and payload.
* to emit a notification event with the given payload.
* Ignores unknown origins.
*
*
* @param {string} origin - The connection's origin string.
* @param {string} eventName - The name of the event to emit.
* @param {any} payload - The event payload.
*/
notifyConnections (origin, eventName, payload) {
notifyConnections (origin, payload) {

if (!this.connections[origin]) return
const connections = this.connections[origin]
if (!connections) return

Object.values(this.connections[origin]).forEach(conn => {
conn.engine && conn.engine.emit(eventName, payload)
Object.values(connections).forEach(conn => {
conn.engine && conn.engine.emit('notification', payload)
})
}

/**
* Causes the RPC engines associated with all connections to emit an event
* with the given name and payload.
*
* @param {string} eventName - The name of the event to emit.
* Causes the RPC engines associated with all connections to emit a
* notification event with the given payload.
*
* @param {any} payload - The event payload.
*/
notifyAllConnections (eventName, payload) {
notifyAllConnections (payload) {

Object.values(this.connections).forEach(origin => {
Object.values(origin).forEach(conn => {
conn.engine && conn.engine.emit(eventName, payload)
conn.engine && conn.engine.emit('notification', payload)
})
})
}


// handlers

/**
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
"lodash.shuffle": "^4.2.0",
"loglevel": "^1.4.1",
"luxon": "^1.8.2",
"metamask-inpage-provider": "rekmarks/metamask-inpage-provider#permissions",
"metamask-inpage-provider": "MetaMask/metamask-inpage-provider#LoginPerSite",
"metamask-logo": "^2.1.4",
"mkdirp": "^0.5.1",
"multihashes": "^0.4.12",
Expand Down Expand Up @@ -171,7 +171,7 @@
"redux-thunk": "^2.2.0",
"request-promise": "^4.2.1",
"reselect": "^3.0.1",
"rpc-cap": "^0.18.0",
"rpc-cap": "^1.0.0",
"safe-event-emitter": "^1.0.1",
"safe-json-stringify": "^1.2.0",
"single-call-balance-checker-abi": "^1.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,7 @@ export default class PermissionPageContainerContent extends PureComponent {

const items = Object.keys(selectedPermissions).map((methodName) => {

// TODO:lps:review is this how we want to handle permissions without
// descriptions? In all current cases, if this condition triggers,
// the request will still fail when rpc-cap attempts to approve the
// permissions.
// the request will almost certainly be reject by rpc-cap if this happens
if (!permissionsDescriptions[methodName]) {
console.warn(`Unknown permission requested: ${methodName}`)
}
Expand Down
Loading

0 comments on commit 3d2a368

Please sign in to comment.