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

Fix synchronous eth_accounts request #353

Merged
merged 3 commits into from
Apr 13, 2020
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## Current Master

- [#353](https://github.com/poanetwork/nifty-wallet/pull/353) - Fix synchronous eth_accounts request

## 5.0.1 Mon Apr 06 2020

- [#347](https://github.com/poanetwork/nifty-wallet/pull/347) - Rollback custom dPath for RSK/ETC
Expand Down
25 changes: 24 additions & 1 deletion app/scripts/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ const isEdge = !isIE && !!window.StyleMedia
let popupIsOpen = false
let notificationIsOpen = false
const openMetamaskTabsIDs = {}
const requestAccountTabIds = {}

// state persistence
const diskStore = new LocalStorageStore({ storageKey: STORAGE_KEY })
Expand Down Expand Up @@ -263,6 +264,9 @@ function setupController (initState, initLangCode) {
initLangCode,
// platform specific api
platform,
getRequestAccountTabIds: () => {
return requestAccountTabIds
},
encryptor: isEdge ? new EdgeEncryptor() : undefined,
})
global.metamaskController = controller
Expand Down Expand Up @@ -328,6 +332,10 @@ function setupController (initState, initLangCode) {
[ENVIRONMENT_TYPE_FULLSCREEN]: true,
}

const metamaskBlacklistedPorts = [
'trezor-connect',
]

const isClientOpenStatus = () => {
return popupIsOpen || Boolean(Object.keys(openMetamaskTabsIDs).length) || notificationIsOpen
}
Expand All @@ -348,11 +356,15 @@ function setupController (initState, initLangCode) {
const processName = remotePort.name
const isMetaMaskInternalProcess = metamaskInternalProcessHash[processName]

if (metamaskBlacklistedPorts.includes(remotePort.name)) {
return false
}

if (isMetaMaskInternalProcess) {
const portStream = new PortStream(remotePort)
// communication with popup
controller.isClientOpen = true
controller.setupTrustedCommunication(portStream, 'MetaMask')
controller.setupTrustedCommunication(portStream, remotePort.sender)

if (processName === ENVIRONMENT_TYPE_POPUP) {
popupIsOpen = true
Expand Down Expand Up @@ -382,6 +394,17 @@ function setupController (initState, initLangCode) {
})
}
} else {
if (remotePort.sender && remotePort.sender.tab && remotePort.sender.url) {
const tabId = remotePort.sender.tab.id
const url = new URL(remotePort.sender.url)
const origin = url.hostname

remotePort.onMessage.addListener((msg) => {
if (msg.data && msg.data.method === 'eth_requestAccounts') {
requestAccountTabIds[origin] = tabId
}
})
}
connectExternal(remotePort)
}
}
Expand Down
1 change: 1 addition & 0 deletions app/scripts/controllers/permissions/enums.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export const SAFE_METHODS = [
'eth_signTypedData',
'eth_signTypedData_v1',
'eth_signTypedData_v3',
'eth_signTypedData_v4',
'eth_submitHashrate',
'eth_submitWork',
'eth_syncing',
Expand Down
65 changes: 3 additions & 62 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,6 @@ module.exports = class MetamaskController extends EventEmitter {
setUsePhishDetect: this.setUsePhishDetect.bind(this),
setCurrentLocale: this.setCurrentLocale.bind(this),
setDProvider: this.setDProvider.bind(this),
markAccountsFound: this.markAccountsFound.bind(this),
markPasswordForgotten: this.markPasswordForgotten.bind(this),
unMarkPasswordForgotten: this.unMarkPasswordForgotten.bind(this),
getGasPrice: (cb) => cb(null, this.getGasPrice()),
Expand Down Expand Up @@ -1380,8 +1379,8 @@ cancelEncryptionPublicKey (msgId, cb) {
* @param {Object} msgParams - The params passed to eth_signTypedData.
* @param {Function} cb - The callback function, called with the signature.
*/
newUnsignedTypedMessage (msgParams, req) {
const promise = this.typedMessageManager.addUnapprovedMessageAsync(msgParams, req)
newUnsignedTypedMessage (msgParams, req, version) {
const promise = this.typedMessageManager.addUnapprovedMessageAsync(msgParams, req, version)
this.sendUpdate()
this.opts.showUnconfirmedMessage()
return promise
Expand All @@ -1392,7 +1391,7 @@ cancelEncryptionPublicKey (msgId, cb) {
* Triggers the callback in newUnsignedTypedMessage.
*
* @param {Object} msgParams - The params passed to eth_signTypedData.
* @returns {Object} Full state update.
* @returns {Object} - Full state update.
*/
async signTypedMessage (msgParams) {
log.info('MetaMaskController - eth_signTypedData')
Expand Down Expand Up @@ -1434,64 +1433,6 @@ cancelEncryptionPublicKey (msgId, cb) {
}
}

// ---------------------------------------------------------------------------
// MetaMask Version 3 Migration Account Restauration Methods

/**
* A legacy method (probably dead code) that was used when we swapped out our
* key management library that we depended on.
*
* Described in:
* https://medium.com/metamask/metamask-3-migration-guide-914b79533cdd
*
* @deprecated
* @param {} migratorOutput
*/
restoreOldVaultAccounts (migratorOutput) {
const { serialized } = migratorOutput
return this.keyringController.restoreKeyring(serialized)
.then(() => migratorOutput)
}

/**
* A legacy method used to record user confirmation that they understand
* that some of their accounts have been recovered but should be backed up.
* This function no longer does anything and will be removed.
*
* @deprecated
* @param {Function} cb - A callback function called with a full state update.
*/
markAccountsFound (cb) {
// TODO Remove me
cb(null, this.getState())
}

/**
* An account object
* @typedef Account
* @property string privateKey - The private key of the account.
*/

/**
* Probably no longer needed, related to the Version 3 migration.
* Imports a hash of accounts to private keys into the vault.
*
* Described in:
* https://medium.com/metamask/metamask-3-migration-guide-914b79533cdd
*
* Uses the array's private keys to create a new Simple Key Pair keychain
* and add it to the keyring controller.
* @deprecated
* @param {Account[]} lostAccounts -
* @returns {Keyring[]} An array of the restored keyrings.
*/
importLostAccounts ({ lostAccounts }) {
const privKeys = lostAccounts.map(acct => acct.privateKey)
return this.keyringController.restoreKeyring({
type: 'Simple Key Pair',
data: privKeys,
})
}

//=============================================================================
// END (VAULT / KEYRING RELATED METHODS)
Expand Down
52 changes: 8 additions & 44 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
"eth-net-props": "^1.0.33",
"eth-phishing-detect": "^1.1.4",
"eth-query": "^2.1.2",
"eth-sig-util": "^2.3.0",
"eth-sig-util": "^2.5.1",
"eth-token-watcher": "^1.1.7",
"eth-trezor-keyring": "github:vbaranov/eth-trezor-keyring#0.4.0",
"ethereumjs-abi": "^0.6.7",
Expand Down Expand Up @@ -156,7 +156,7 @@
"mkdirp": "^0.5.1",
"multihashes": "^0.4.12",
"nanoid": "^2.1.6",
"nifty-wallet-inpage-provider": "github:poanetwork/nifty-wallet-inpage-provider#1.4.0",
"nifty-wallet-inpage-provider": "github:poanetwork/nifty-wallet-inpage-provider#1.5.0",
"nonce-tracker": "^1.0.0",
"number-to-bn": "^1.7.0",
"obj-multiplex": "^1.0.0",
Expand Down
8 changes: 0 additions & 8 deletions test/unit/app/controllers/metamask-controller-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -933,14 +933,6 @@ describe('MetaMaskController', function () {
})
})

describe('#markAccountsFound', function () {
it('adds lost accounts to config manager data', function () {
metamaskController.markAccountsFound(noop)
const state = metamaskController.getState()
assert.deepEqual(state.lostAccounts, [])
})
})

describe('#markPasswordForgotten', function () {
it('adds and sets forgottenPassword to config data to true', function () {
metamaskController.markPasswordForgotten(noop)
Expand Down