Merge pull request #896 from MetaMask/i893-DenodeifyKeyringController

Denodeify most of KeyringController

app/scripts/lib/nodeify.js

@@ -0,0 +1,17 @@
+module.exports = function (promiseFn) {
+  return function () {
+    var args = []
+    for (var i = 0; i < arguments.length - 1; i++) {
+      args.push(arguments[i])
+    }
+    var cb = arguments[arguments.length - 1]
+
+    return promiseFn.apply(this, args)
+    .then(function (result) {
+      cb(null, result)
+    })
+    .catch(function (reason) {
+      cb(reason)
+    })
+  }
+}

app/scripts/metamask-controller.js

@@ -8,6 +8,7 @@ const Web3 = require('web3')
 const ConfigManager = require('./lib/config-manager')
 const extension = require('./lib/extension')
 const autoFaucet = require('./lib/auto-faucet')
+const nodeify = require('./lib/nodeify')
 
 
 module.exports = class MetamaskController {
@@ -62,21 +63,24 @@ module.exports = class MetamaskController {
       setGasMultiplier: this.setGasMultiplier.bind(this),
 
       // forward directly to keyringController
-      placeSeedWords: keyringController.placeSeedWords.bind(keyringController),
+      createNewVaultAndKeychain: nodeify(keyringController.createNewVaultAndKeychain).bind(keyringController),
-      createNewVaultAndKeychain: keyringController.createNewVaultAndKeychain.bind(keyringController),
+      createNewVaultAndRestore: nodeify(keyringController.createNewVaultAndRestore).bind(keyringController),
-      createNewVaultAndRestore: keyringController.createNewVaultAndRestore.bind(keyringController),
+      placeSeedWords: nodeify(keyringController.placeSeedWords).bind(keyringController),
-      clearSeedWordCache: keyringController.clearSeedWordCache.bind(keyringController),
+      clearSeedWordCache: nodeify(keyringController.clearSeedWordCache).bind(keyringController),
-      addNewKeyring: keyringController.addNewKeyring.bind(keyringController),
+      setLocked: nodeify(keyringController.setLocked).bind(keyringController),
-      addNewAccount: keyringController.addNewAccount.bind(keyringController),
+      submitPassword: nodeify(keyringController.submitPassword).bind(keyringController),
-      submitPassword: keyringController.submitPassword.bind(keyringController),
+      addNewKeyring: nodeify(keyringController.addNewKeyring).bind(keyringController),
-      setSelectedAccount: keyringController.setSelectedAccount.bind(keyringController),
+      addNewAccount: nodeify(keyringController.addNewAccount).bind(keyringController),
+      setSelectedAccount: nodeify(keyringController.setSelectedAccount).bind(keyringController),
+      saveAccountLabel: nodeify(keyringController.saveAccountLabel).bind(keyringController),
+      exportAccount: nodeify(keyringController.exportAccount).bind(keyringController),
+
+      // signing methods
       approveTransaction: keyringController.approveTransaction.bind(keyringController),
       cancelTransaction: keyringController.cancelTransaction.bind(keyringController),
       signMessage: keyringController.signMessage.bind(keyringController),
       cancelMessage: keyringController.cancelMessage.bind(keyringController),
-      setLocked: keyringController.setLocked.bind(keyringController),
+
-      exportAccount: keyringController.exportAccount.bind(keyringController),
-      saveAccountLabel: keyringController.saveAccountLabel.bind(keyringController),
       // coinbase
       buyEth: this.buyEth.bind(this),
       // shapeshift

package.json

@@ -33,7 +33,6 @@
     ]
   },
   "dependencies": {
-    "ap": "^0.2.0",
     "async": "^1.5.2",
     "bip39": "^2.2.0",
     "browserify-derequire": "^0.9.4",
@@ -67,6 +66,7 @@
     "pojo-migrator": "^2.1.0",
     "polyfill-crypto.getrandomvalues": "^1.0.0",
     "post-message-stream": "^1.0.0",
+    "promise-filter": "^1.1.0",
     "pumpify": "^1.3.4",
     "qrcode-npm": "0.0.3",
     "react": "^15.0.2",

test/integration/lib/keyring-controller-test.js

@@ -38,8 +38,8 @@ QUnit.test('keyringController:isInitialized', function (assert) {
 QUnit.test('keyringController:submitPassword', function (assert) {
   var done = assert.async()
 
-  this.keyringController.submitPassword(PASSWORD, (err, state) => {
+  this.keyringController.submitPassword(PASSWORD)
-    assert.notOk(err)
+  .then((state) => {
     assert.ok(state.identities[FIRST_ADDRESS])
     done()
   })
@@ -49,9 +49,14 @@ QUnit.test('keyringController:setLocked', function (assert) {
   var done = assert.async()
   var self = this
 
-  this.keyringController.setLocked(function(err) {
+  this.keyringController.setLocked()
+  .then(function() {
     assert.notOk(self.keyringController.password, 'password should be deallocated')
     assert.deepEqual(self.keyringController.keyrings, [], 'keyrings should be deallocated')
     done()
   })
+  .catch((reason) => {
+    assert.ifError(reason)
+    done()
+  })
 })

test/unit/keyring-controller-test.js

@@ -32,8 +32,8 @@ describe('KeyringController', function() {
     // Browser crypto is tested in the integration test suite.
     keyringController.encryptor = mockEncryptor
 
-    keyringController.createNewVaultAndKeychain(password, function (err, newState) {
+    keyringController.createNewVaultAndKeychain(password)
-      assert.ifError(err)
+    .then(function (newState) {
       state = newState
       done()
     })
@@ -50,12 +50,16 @@ describe('KeyringController', function() {
     it('should set a vault on the configManager', function(done) {
       keyringController.configManager.setVault(null)
       assert(!keyringController.configManager.getVault(), 'no previous vault')
-      keyringController.createNewVaultAndKeychain(password, (err, state) => {
+      keyringController.createNewVaultAndKeychain(password)
-        assert.ifError(err)
+      .then(() => {
         const vault = keyringController.configManager.getVault()
         assert(vault, 'vault created')
         done()
       })
+      .catch((reason) => {
+        assert.ifError(reason)
+        done()
+      })
     })
   })
 
@@ -124,13 +128,17 @@ describe('KeyringController', function() {
       const account = addresses[0]
       var nick = 'Test nickname'
       keyringController.identities[ethUtil.addHexPrefix(account)] = {}
-      keyringController.saveAccountLabel(account, nick, (err, label) => {
+      keyringController.saveAccountLabel(account, nick)
-        assert.ifError(err)
+      .then((label) => {
         assert.equal(label, nick)
         const persisted = keyringController.configManager.nicknameForWallet(account)
         assert.equal(persisted, nick)
         done()
       })
+      .catch((reason) => {
+        assert.ifError(reason)
+        done()
+      })
     })
 
     this.timeout(10000)
@@ -138,8 +146,8 @@ describe('KeyringController', function() {
       const account = addresses[0]
       var nick = 'Test nickname'
       keyringController.configManager.setNicknameForWallet(account, nick)
-      keyringController.createNewVaultAndRestore(password, seedWords, (err, state) => {
+      keyringController.createNewVaultAndRestore(password, seedWords)
-        assert.ifError(err)
+      .then((state) => {
 
         const identity = keyringController.identities['0x' + account]
         assert.equal(identity.name, nick)
@@ -147,6 +155,10 @@ describe('KeyringController', function() {
         assert(accounts)
         done()
       })
+      .catch((reason) => {
+        assert.ifError(reason)
+        done()
+      })
     })
   })
 

test/unit/keyrings/hd-test.js

@@ -57,13 +57,11 @@ describe('hd-keyring', function() {
 
   describe('#deserialize a private key', function() {
     it('serializes what it deserializes', function(done) {
-      console.log('deserializing ' + sampleMnemonic)
       keyring.deserialize({
         mnemonic: sampleMnemonic,
         numberOfAccounts: 1
       })
       .then(() => {
-        console.dir(keyring)
         assert.equal(keyring.wallets.length, 1, 'restores two accounts')
         return keyring.addAccounts(1)
       }).then(() => {

test/unit/nodeify-test.js

@@ -0,0 +1,22 @@
+const assert = require('assert')
+const nodeify = require('../../app/scripts/lib/nodeify')
+
+describe.only('nodeify', function() {
+
+  var obj = {
+    foo: 'bar',
+    promiseFunc: function (a) {
+      var solution = this.foo + a
+      return Promise.resolve(solution)
+    }
+  }
+
+  it('should retain original context', function(done) {
+    var nodified = nodeify(obj.promiseFunc).bind(obj)
+    nodified('baz', function (err, res) {
+      assert.equal(res, 'barbaz')
+      done()
+    })
+  })
+
+})

ui/app/components/pending-tx-details.js

@@ -154,8 +154,6 @@ PTXP.render = function () {
         ]),
       ]), // End of Table
 
-      this.warnIfNeeded(),
-
     ])
   )
 }
@@ -201,29 +199,6 @@ PTXP.miniAccountPanelForRecipient = function () {
   }
 }
 
-// Should analyze if there is a DELEGATECALL opcode
-// in the recipient contract, and show a warning if so.
-PTXP.warnIfNeeded = function () {
-  const containsDelegateCall = !!this.props.txData.containsDelegateCall
-
-  if (!containsDelegateCall) {
-    return null
-  }
-
-  return h('span.error', {
-    style: {
-      fontFamily: 'Montserrat Light',
-      fontSize: '13px',
-      display: 'flex',
-      justifyContent: 'center',
-    },
-  }, [
-    h('i.fa.fa-lg.fa-info-circle', { style: { margin: '5px' } }),
-    h('span', ' Your identity may be used in other contracts!'),
-  ])
-}
-
-
 function forwardCarrat () {
   return (