Skip to content

Commit

Permalink
Issue MetaMask#5640 Bidirectional QR accounts implemented
Browse files Browse the repository at this point in the history
  • Loading branch information
r001 committed Sep 10, 2020
1 parent eef4ec9 commit ac38747
Show file tree
Hide file tree
Showing 37 changed files with 1,607 additions and 78 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Changelog

## Current Develop Branch
- [#9228](https://github.com/MetaMask/metamask-extension/pull/6267): Feature: Bidirectional QR code accounts

## 8.0.9 Wed Aug 19 2020
- [#9228](https://github.com/MetaMask/metamask-extension/pull/9228): Move transaction confirmation footer buttons to scrollable area
Expand Down
44 changes: 44 additions & 0 deletions app/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,16 @@
"connecting": {
"message": "Connecting..."
},
"bidirectionalQr": {
"message": "QR"
},
"bidirectionalQrAcc": {
"message": "QR",
"description": "Select this for entering Bidirectional QR account without password"
},
"bidirectionalQrMsg": {
"message": "Bidirectional QR accounts do not store private key locally, instead upon transaction confirmation, Metamask will display a QR code with transaction details and expects a signature from a cold wallet. Bidirectional QR accounts can not be recovered with your originally created MetaMask account seedphrase."
},
"connectWithMetaMask": {
"message": "Connect With MetaMask"
},
Expand All @@ -105,6 +115,9 @@
"contractInteraction": {
"message": "Contract Interaction"
},
"airsign": {
"message": "AirSign"
},
"reject": {
"message": "Reject"
},
Expand Down Expand Up @@ -854,6 +867,15 @@
"forbiddenIpfsGateway": {
"message": "Forbidden IPFS Gateway: Please specify a CID gateway"
},
"invalidSignature": {
"message": "Invalid signature"
},
"invalidSignatureLength": {
"message": "Length should be 130 chars."
},
"invalidSignatureChar": {
"message": "Invalid character in signature."
},
"jsonFile": {
"message": "JSON File",
"description": "format for importing an account"
Expand Down Expand Up @@ -1068,6 +1090,9 @@
"parameters": {
"message": "Parameters"
},
"originalTotal": {
"message": "Original Total"
},
"participateInMetaMetrics": {
"message": "Participate in MetaMetrics"
},
Expand All @@ -1087,6 +1112,13 @@
"message": "Paste your private key string here:",
"description": "For importing an account from a private key"
},
"pasteBidirectionalQr": {
"message": "Paste your Bidirectional QR account addresses here (more than one allowed):",
"description": "To import a Bidirectional QR account provide account numbers"
},
"pasteSeed": {
"message": "Paste your seed phrase here!"
},
"pending": {
"message": "Pending"
},
Expand Down Expand Up @@ -1149,6 +1181,18 @@
"recipientAddressPlaceholder": {
"message": "Search, public address (0x), or ENS"
},
"signature": {
"message": "Signature:"
},
"signerDoesNotSupport": {
"message": "Not supported by signer"
},
"signWith": {
"message": "Sign with:"
},
"refundAddress": {
"message": "Your Refund Address"
},
"rejectAll": {
"message": "Reject All"
},
Expand Down
12 changes: 9 additions & 3 deletions app/scripts/controllers/transactions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,11 @@ export default class TransactionController extends EventEmitter {
} catch (err) {
// this is try-catch wrapped so that we can guarantee that the nonceLock is released
try {
this.txStateManager.setTxStatusFailed(txId, err)
if (err.message === 'Cancel pressed') {
this.txStateManager.setTxStatusUnapproved(txId)
} else {
this.txStateManager.setTxStatusFailed(txId, err)
}
} catch (err2) {
log.error(err2)
}
Expand All @@ -477,7 +481,9 @@ export default class TransactionController extends EventEmitter {
nonceLock.releaseLock()
}
// continue with error chain
throw err
if (err.message !== 'Cancel pressed') {
throw err
}
} finally {
this.inProcessOfSigning.delete(txId)
}
Expand All @@ -496,7 +502,7 @@ export default class TransactionController extends EventEmitter {
// sign tx
const fromAddress = txParams.from
const ethTx = new Transaction(txParams)
await this.signEthTx(ethTx, fromAddress)
await this.signEthTx(ethTx, fromAddress, { txId })

// add r,s,v values for provider request purposes see createMetamaskMiddleware
// and JSON rpc standard for further explanation
Expand Down
77 changes: 70 additions & 7 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import createEngineStream from 'json-rpc-middleware-stream/engineStream'
import createFilterMiddleware from 'eth-json-rpc-filters'
import createSubscriptionManager from 'eth-json-rpc-filters/subscriptionManager'
import providerAsMiddleware from 'eth-json-rpc-middleware/providerAsMiddleware'
import KeyringController from 'eth-keyring-controller'
import KeyringController from 'eth-opts-keyring-controller'
import { Mutex } from 'await-semaphore'
import ethUtil from 'ethereumjs-util'
import log from 'loglevel'
Expand All @@ -25,6 +25,7 @@ import {
CurrencyRateController,
PhishingController,
} from '@metamask/controllers'
import BidirectionalQrKeyring from 'eth-bidirectional-qr-keyring'
import ComposableObservableStore from './lib/ComposableObservableStore'
import AccountTracker from './lib/account-tracker'
import createLoggerMiddleware from './lib/createLoggerMiddleware'
Expand Down Expand Up @@ -56,7 +57,6 @@ import nodeify from './lib/nodeify'
import accountImporter from './account-import-strategies'
import selectChainId from './lib/select-chain-id'
import seedPhraseVerifier from './lib/seed-phrase-verifier'

import backgroundMetaMetricsEvent from './lib/background-metametrics'

export default class MetamaskController extends EventEmitter {
Expand Down Expand Up @@ -177,7 +177,12 @@ export default class MetamaskController extends EventEmitter {
this.accountTracker._updateAccounts()
})

const additionalKeyrings = [TrezorKeyring, LedgerBridgeKeyring]
// key mgmt
const additionalKeyrings = [
TrezorKeyring,
LedgerBridgeKeyring,
BidirectionalQrKeyring,
]
this.keyringController = new KeyringController({
keyringTypes: additionalKeyrings,
initState: initState.KeyringController,
Expand All @@ -197,6 +202,14 @@ export default class MetamaskController extends EventEmitter {
showPermissionRequest: opts.showPermissionRequest,
}, initState.PermissionsController, initState.PermissionsMetadata)

// External Keyring state exposed to UI
this.memStoreBidirectionalQrKeyring = new ObservableStore({
bidirectionalQrSignables: [],
})

this.bidirectionalQrKeyring = new BidirectionalQrKeyring()

// detect tokens controller
this.detectTokensController = new DetectTokensController({
preferences: this.preferencesController,
network: this.networkController,
Expand Down Expand Up @@ -297,6 +310,7 @@ export default class MetamaskController extends EventEmitter {
EncryptionPublicKeyManager: this.encryptionPublicKeyManager.memStore,
TypesMessageManager: this.typedMessageManager.memStore,
KeyringController: this.keyringController.memStore,
BidirectionalQrKeyring: this.bidirectionalQrKeyring.memStore,
PreferencesController: this.preferencesController.store,
AddressBookController: this.addressBookController,
CurrencyController: this.currencyRateController,
Expand Down Expand Up @@ -424,6 +438,7 @@ export default class MetamaskController extends EventEmitter {
preferencesController,
threeBoxController,
txController,
bidirectionalQrKeyring,
} = this

return {
Expand Down Expand Up @@ -451,6 +466,15 @@ export default class MetamaskController extends EventEmitter {
removeAccount: nodeify(this.removeAccount, this),
importAccountWithStrategy: nodeify(this.importAccountWithStrategy, this),

// bidirectional qr account management
addBidirectionalQrAccount: nodeify(this.addBidirectionalQrAccount, this),

submitSignatureBidirectionalQr:
nodeify(bidirectionalQrKeyring.submitSignature, bidirectionalQrKeyring),

cancelSignatureBidirectionalQr:
nodeify(bidirectionalQrKeyring.cancelSignature, bidirectionalQrKeyring),

// hardware wallets
connectHardware: nodeify(this.connectHardware, this),
forgetDevice: nodeify(this.forgetDevice, this),
Expand Down Expand Up @@ -984,6 +1008,45 @@ export default class MetamaskController extends EventEmitter {
}
}

/**
* Creates a bidirectional QR account and adds it to a newly created
* BidirectionalQrKeyring.
*
* @param {string} addresses - A string of valid Ethereum addresses
*/
async addBidirectionalQrAccount (addresses) {
const foundAddresses = addresses.match(/(0x[a-fA-F0-9]{40})/ug)
let keyring
if (foundAddresses.length > 0) {
if (
this.keyringController
.getKeyringsByType('Bidirectional Qr Account')
.length === 0
) {

keyring = await this.keyringController
.addNewKeyring('Bidirectional Qr Account', foundAddresses)

} else {
keyring = this.bidirectionalQrKeyring
foundAddresses.map((address) => this.keyringController
.addNewAccount(
keyring,
{ accounts: [address] },
))
}
} else {
throw new Error('No valid address provided')
}

// update accounts in preferences controller
const allAccounts = await this.keyringController.getAccounts()
this.preferencesController.setAddresses(allAccounts)
// set new account as selected
const accounts = await keyring.getAccounts()
await this.preferencesController.setSelectedAddress(accounts[accounts.length - 1])
}

/**
* Clears the transaction history, to allow users to force-reset their nonces.
* Mostly used in development environments, when networks are restarted with
Expand Down Expand Up @@ -1086,7 +1149,7 @@ export default class MetamaskController extends EventEmitter {
return this.messageManager.approveMessage(msgParams)
.then((cleanMsgParams) => {
// signs the message
return this.keyringController.signMessage(cleanMsgParams)
return this.keyringController.signMessage(cleanMsgParams, { msgId })
})
.then((rawSig) => {
// tells the listener that the message has been signed
Expand Down Expand Up @@ -1145,10 +1208,10 @@ export default class MetamaskController extends EventEmitter {
return this.personalMessageManager.approveMessage(msgParams)
.then((cleanMsgParams) => {
// signs the message
return this.keyringController.signPersonalMessage(cleanMsgParams)
return this.keyringController.signPersonalMessage(cleanMsgParams, { msgId })
})
.then((rawSig) => {
// tells the listener that the message has been signed
// tells the listener that the message has been signed
// and can be returned to the dapp
this.personalMessageManager.setMsgStatusSigned(msgId, rawSig)
return this.getState()
Expand Down Expand Up @@ -1349,7 +1412,7 @@ export default class MetamaskController extends EventEmitter {
}
}

const signature = await this.keyringController.signTypedMessage(cleanMsgParams, { version })
const signature = await this.keyringController.signTypedMessage(cleanMsgParams, { version, msgId })
this.typedMessageManager.setMsgStatusSigned(msgId, signature)
return this.getState()
} catch (error) {
Expand Down
12 changes: 11 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,19 @@
"deep-freeze-strict": "1.1.1",
"dnode": "^1.2.2",
"end-of-stream": "^1.4.4",
"eth-airsign-util": "^1.2.1",
"eth-bidirectional-qr-keyring": "^2.0.0",
"eth-block-tracker": "^4.4.2",
"eth-contract-metadata": "^1.15.0",
"eth-ellipal-util": "^1.0.0",
"eth-ens-namehash": "^2.0.8",
"eth-json-rpc-errors": "^2.0.2",
"eth-json-rpc-filters": "^4.1.1",
"eth-json-rpc-infura": "^4.0.2",
"eth-json-rpc-middleware": "^5.0.2",
"eth-keyring-controller": "^6.1.0",
"eth-method-registry": "^1.2.0",
"eth-opts-keyring-controller": "^6.1.0",
"eth-phishing-detect": "^1.1.4",
"eth-query": "^2.1.2",
"eth-sig-util": "^2.3.0",
Expand All @@ -115,6 +119,7 @@
"ethereumjs-util": "5.1.0",
"ethereumjs-wallet": "^0.6.0",
"ethers": "^5.0.8",
"etherscan-link": "^1.0.2",
"ethjs": "^0.4.0",
"ethjs-contract": "^0.2.3",
"ethjs-ens": "^2.0.0",
Expand Down Expand Up @@ -142,7 +147,11 @@
"pubnub": "4.27.3",
"pump": "^3.0.0",
"punycode": "^2.1.1",
"qr-encoding": "^1.2.1",
"qrcode-generator": "1.4.1",
"qrcode-npm": "0.0.3",
"qrcode.react": "0.7.2",
"ramda": "^0.24.1",
"react": "^16.12.0",
"react-dnd": "^3.0.2",
"react-dnd-html5-backend": "^7.4.4",
Expand All @@ -158,6 +167,7 @@
"react-toggle-button": "^2.2.0",
"react-transition-group": "^1.2.1",
"readable-stream": "^2.3.3",
"recompose": "^0.30.0",
"redux": "^4.0.5",
"redux-thunk": "^2.3.0",
"reselect": "^3.0.1",
Expand Down Expand Up @@ -238,7 +248,7 @@
"gulp-rename": "^2.0.0",
"gulp-replace": "^1.0.0",
"gulp-rtlcss": "^1.4.0",
"gulp-sass": "^4.0.0",
"gulp-sass": "^4.0.2",
"gulp-sourcemaps": "^2.6.0",
"gulp-stylelint": "^13.0.0",
"gulp-terser-js": "^5.2.2",
Expand Down
2 changes: 1 addition & 1 deletion test/unit/app/seed-phrase-verifier-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import assert from 'assert'
import { cloneDeep } from 'lodash'
import KeyringController from 'eth-keyring-controller'
import KeyringController from 'eth-opts-keyring-controller'
import firstTimeState from '../../../app/scripts/first-time-state'
import seedPhraseVerifier from '../../../app/scripts/lib/seed-phrase-verifier'
import mockEncryptor from '../../lib/mock-encryptor'
Expand Down
2 changes: 1 addition & 1 deletion test/unit/ui/app/actions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import configureStore from 'redux-mock-store'
import thunk from 'redux-thunk'
import EthQuery from 'eth-query'
import Eth from 'ethjs'
import KeyringController from 'eth-keyring-controller'
import KeyringController from 'eth-opts-keyring-controller'
import { createTestProviderTools } from '../../../stub/provider'
import enLocale from '../../../../app/_locales/en/messages.json'
import * as actions from '../../../../ui/app/store/actions'
Expand Down
3 changes: 3 additions & 0 deletions ui/app/components/app/account-menu/account-menu.component.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ export default class AccountMenu extends Component {
case 'Simple Key Pair':
label = t('imported')
break
case 'Bidirectional Qr Account':
label = t('bidirectionalQr')
break
default:
return null
}
Expand Down
Loading

0 comments on commit ac38747

Please sign in to comment.