Merge pull request #4603 from MetaMask/v4.8.0

V4.8.0

6 files changed, 163 insertions, 63 deletions

diff --git a/test/unit/app/account-import-strategies.spec.js b/test/unit/app/account-import-strategies.spec.js
index 83cfaeb3e..216c2f698 100644
--- a/test/unit/app/account-import-strategies.spec.js
+++ b/test/unit/app/account-import-strategies.spec.js
@@ -1,31 +1,59 @@
 const assert = require('assert')
 const path = require('path')
-const accountImporter = require('../../../app/scripts/account-import-strategies/index')
 const ethUtil = require('ethereumjs-util')
+const accountImporter = require('../../../app/scripts/account-import-strategies/index')
+const { assertRejects } = require('../test-utils')
 
 describe('Account Import Strategies', function () {
   const privkey = '0x4cfd3e90fc78b0f86bf7524722150bb8da9c60cd532564d7ff43f5716514f553'
   const json = '{"version":3,"id":"dbb54385-0a99-437f-83c0-647de9f244c3","address":"a7f92ce3fba24196cf6f4bd2e1eb3db282ba998c","Crypto":{"ciphertext":"bde13d9ade5c82df80281ca363320ce254a8a3a06535bbf6ffdeaf0726b1312c","cipherparams":{"iv":"fbf93718a57f26051b292f072f2e5b41"},"cipher":"aes-128-ctr","kdf":"scrypt","kdfparams":{"dklen":32,"salt":"7ffe00488319dec48e4c49a120ca49c6afbde9272854c64d9541c83fc6acdffe","n":8192,"r":8,"p":1},"mac":"2adfd9c4bc1cdac4c85bddfb31d9e21a684e0e050247a70c5698facf6b7d4681"}}'
 
-  it('imports a private key and strips 0x prefix', async function () {
-    const importPrivKey = await accountImporter.importAccount('Private Key', [ privkey ])
-    assert.equal(importPrivKey, ethUtil.stripHexPrefix(privkey))
-  })
+  describe('private key import', function () {
+    it('imports a private key and strips 0x prefix', async function () {
+      const importPrivKey = await accountImporter.importAccount('Private Key', [ privkey ])
+      assert.equal(importPrivKey, ethUtil.stripHexPrefix(privkey))
+    })
 
-  it('fails when password is incorrect for keystore', async function () {
-    const wrongPassword = 'password2'
+    it('throws an error for empty string private key', async () => {
+      assertRejects(async function() {
+        await accountImporter.importAccount('Private Key', [ '' ])
+      }, Error, 'no empty strings')
+    })
 
-    try {
-      await accountImporter.importAccount('JSON File', [ json, wrongPassword])
-    } catch (error) {
-      assert.equal(error.message, 'Key derivation failed - possibly wrong passphrase')
-    }
-  })
+    it('throws an error for undefined string private key', async () => {
+      assertRejects(async function () {
+        await accountImporter.importAccount('Private Key', [ undefined ])
+      })
+    })
 
-  it('imports json string and password to return a private key', async function () {
-    const fileContentsPassword = 'password1'
-    const importJson = await accountImporter.importAccount('JSON File', [ json, fileContentsPassword])
-    assert.equal(importJson, '0x5733876abe94146069ce8bcbabbde2677f2e35fa33e875e92041ed2ac87e5bc7')
+    it('throws an error for undefined string private key', async () => {
+      assertRejects(async function () {
+        await accountImporter.importAccount('Private Key', [])
+      })
+    })
+
+    it('throws an error for invalid private key', async () => {
+      assertRejects(async function () {
+        await accountImporter.importAccount('Private Key', [ 'popcorn' ])
+      })
+    })
   })
 
+  describe('JSON keystore import', function () {
+    it('fails when password is incorrect for keystore', async function () {
+      const wrongPassword = 'password2'
+
+      try {
+        await accountImporter.importAccount('JSON File', [ json, wrongPassword])
+      } catch (error) {
+        assert.equal(error.message, 'Key derivation failed - possibly wrong passphrase')
+      }
+    })
+
+    it('imports json string and password to return a private key', async function () {
+      const fileContentsPassword = 'password1'
+      const importJson = await accountImporter.importAccount('JSON File', [ json, fileContentsPassword])
+      assert.equal(importJson, '0x5733876abe94146069ce8bcbabbde2677f2e35fa33e875e92041ed2ac87e5bc7')
+    })
+  })
 })
diff --git a/test/unit/app/controllers/metamask-controller-test.js b/test/unit/app/controllers/metamask-controller-test.js
index 266c3f258..0dda4609b 100644
--- a/test/unit/app/controllers/metamask-controller-test.js
+++ b/test/unit/app/controllers/metamask-controller-test.js
@@ -53,6 +53,9 @@ describe('MetaMaskController', function () {
       },
       initState: clone(firstTimeState),
     })
+    // disable diagnostics
+    metamaskController.diagnostics = null
+    // add sinon method spies
     sandbox.spy(metamaskController.keyringController, 'createNewVaultAndKeychain')
     sandbox.spy(metamaskController.keyringController, 'createNewVaultAndRestore')
   })
diff --git a/test/unit/app/controllers/notice-controller-test.js b/test/unit/app/controllers/notice-controller-test.js
index e78b69623..b3ae75080 100644
--- a/test/unit/app/controllers/notice-controller-test.js
+++ b/test/unit/app/controllers/notice-controller-test.js
@@ -14,18 +14,6 @@ describe('notice-controller', function () {
   })
 
   describe('notices', function () {
-    describe('#getNoticesList', function () {
-      it('should return an empty array when new', function (done) {
-        // const testList = [{
-        //   id: 0,
-        //   read: false,
-        //   title: 'Futuristic Notice',
-        // }]
-        var result = noticeController.getNoticesList()
-        assert.equal(result.length, 0)
-        done()
-      })
-    })
 
     describe('#setNoticesList', function () {
       it('should set data appropriately', function (done) {
@@ -41,36 +29,6 @@ describe('notice-controller', function () {
       })
     })
 
-    describe('#updateNoticeslist', function () {
-      it('should integrate the latest changes from the source', function (done) {
-        var testList = [{
-          id: 55,
-          read: false,
-          title: 'Futuristic Notice',
-        }]
-        noticeController.setNoticesList(testList)
-        noticeController.updateNoticesList().then(() => {
-          var newList = noticeController.getNoticesList()
-          assert.ok(newList[0].id === 55)
-          assert.ok(newList[1])
-          done()
-        })
-      })
-      it('should not overwrite any existing fields', function (done) {
-        var testList = [{
-          id: 0,
-          read: false,
-          title: 'Futuristic Notice',
-        }]
-        noticeController.setNoticesList(testList)
-        var newList = noticeController.getNoticesList()
-        assert.equal(newList[0].id, 0)
-        assert.equal(newList[0].title, 'Futuristic Notice')
-        assert.equal(newList.length, 1)
-        done()
-      })
-    })
-
     describe('#markNoticeRead', function () {
       it('should mark a notice as read', function (done) {
         var testList = [{
@@ -86,7 +44,7 @@ describe('notice-controller', function () {
       })
     })
 
-    describe('#getLatestUnreadNotice', function () {
+    describe('#getNextUnreadNotice', function () {
       it('should retrieve the latest unread notice', function (done) {
         var testList = [
           {id: 0, read: true, title: 'Past Notice'},
@@ -94,8 +52,8 @@ describe('notice-controller', function () {
           {id: 2, read: false, title: 'Future Notice'},
         ]
         noticeController.setNoticesList(testList)
-        var latestUnread = noticeController.getLatestUnreadNotice()
-        assert.equal(latestUnread.id, 2)
+        var latestUnread = noticeController.getNextUnreadNotice()
+        assert.equal(latestUnread.id, 1)
         done()
       })
       it('should return undefined if no unread notices exist.', function (done) {
@@ -105,7 +63,7 @@ describe('notice-controller', function () {
           {id: 2, read: true, title: 'Future Notice'},
         ]
         noticeController.setNoticesList(testList)
-        var latestUnread = noticeController.getLatestUnreadNotice()
+        var latestUnread = noticeController.getNextUnreadNotice()
         assert.ok(!latestUnread)
         done()
       })
diff --git a/test/unit/app/controllers/transactions/recipient-blacklist-checker-test.js b/test/unit/app/controllers/transactions/recipient-blacklist-checker-test.js
new file mode 100644
index 000000000..56e8d50db
--- /dev/null
+++ b/test/unit/app/controllers/transactions/recipient-blacklist-checker-test.js
@@ -0,0 +1,77 @@
+const assert = require('assert')
+const recipientBlackListChecker = require('../../../../../app/scripts/controllers/transactions/lib/recipient-blacklist-checker')
+const {
+  ROPSTEN_CODE,
+  RINKEYBY_CODE,
+  KOVAN_CODE,
+} = require('../../../../../app/scripts/controllers/network/enums')
+
+const KeyringController = require('eth-keyring-controller')
+
+describe('Recipient Blacklist Checker', function () {
+
+  let publicAccounts
+
+  before(async function () {
+    const damnedMnemonic = 'candy maple cake sugar pudding cream honey rich smooth crumble sweet treat'
+    const keyringController = new KeyringController({})
+    const Keyring = keyringController.getKeyringClassForType('HD Key Tree')
+    const opts = {
+      mnemonic: damnedMnemonic,
+      numberOfAccounts: 10,
+    }
+    const keyring = new Keyring(opts)
+    publicAccounts = await keyring.getAccounts()
+  })
+
+  describe('#checkAccount', function () {
+    it('does not fail on test networks', function () {
+      let callCount = 0
+      const networks = [ROPSTEN_CODE, RINKEYBY_CODE, KOVAN_CODE]
+      for (let networkId in networks) {
+        publicAccounts.forEach((account) => {
+           recipientBlackListChecker.checkAccount(networkId, account)
+            callCount++
+        })
+      }
+      assert.equal(callCount, 30)
+    })
+
+    it('fails on mainnet', function () {
+      const mainnetId = 1
+      let callCount = 0
+      publicAccounts.forEach((account) => {
+        try {
+          recipientBlackListChecker.checkAccount(mainnetId, account)
+          assert.fail('function should have thrown an error')
+        } catch (err) {
+          assert.equal(err.message, 'Recipient is a public account')
+        }
+        callCount++
+      })
+      assert.equal(callCount, 10)
+    })
+
+    it('fails for public account - uppercase', function () {
+      const mainnetId = 1
+      const publicAccount = '0X0D1D4E623D10F9FBA5DB95830F7D3839406C6AF2'
+      try {
+        recipientBlackListChecker.checkAccount(mainnetId, publicAccount)
+        assert.fail('function should have thrown an error')
+      } catch (err) {
+        assert.equal(err.message, 'Recipient is a public account')
+      }
+    }) 
+
+    it('fails for public account - lowercase', async function () {
+      const mainnetId = 1
+      const publicAccount = '0x0d1d4e623d10f9fba5db95830f7d3839406c6af2'
+      try {
+        await recipientBlackListChecker.checkAccount(mainnetId, publicAccount)
+        assert.fail('function should have thrown an error')
+      } catch (err) {
+        assert.equal(err.message, 'Recipient is a public account')
+      }
+    }) 
+  })
+})
diff --git a/test/unit/app/controllers/transactions/tx-controller-test.js b/test/unit/app/controllers/transactions/tx-controller-test.js
index 1f32a0f37..9bdfe7c1a 100644
--- a/test/unit/app/controllers/transactions/tx-controller-test.js
+++ b/test/unit/app/controllers/transactions/tx-controller-test.js
@@ -185,6 +185,23 @@ describe('Transaction Controller', function () {
       .catch(done)
     })
 
+    it('should fail if recipient is public', function (done) {
+      txController.networkStore = new ObservableStore(1)
+      txController.addUnapprovedTransaction({ from: '0x1678a085c290ebd122dc42cba69373b5953b831d', to: '0x0d1d4e623D10F9FBA5Db95830F7d3839406C6AF2' })
+      .catch((err) => {
+        if (err.message === 'Recipient is a public account') done()
+        else done(err)
+      })
+    })
+
+    it('should not fail if recipient is public but not on mainnet', function (done) {
+      txController.once('newUnapprovedTx', (txMetaFromEmit) => {
+        assert(txMetaFromEmit, 'txMeta is falsey')
+        done()
+      })
+      txController.addUnapprovedTransaction({ from: '0x1678a085c290ebd122dc42cba69373b5953b831d', to: '0x0d1d4e623D10F9FBA5Db95830F7d3839406C6AF2' })
+      .catch(done)
+    })
   })
 
   describe('#addTxGasDefaults', function () {
diff --git a/test/unit/test-utils.js b/test/unit/test-utils.js
new file mode 100644
index 000000000..7d0ae4d91
--- /dev/null
+++ b/test/unit/test-utils.js
@@ -0,0 +1,17 @@
+const assert = require('assert')
+
+module.exports = {
+  assertRejects,
+}
+
+// assert.rejects added in node v10
+async function assertRejects (asyncFn, regExp) {
+  let f = () => {}
+  try {
+    await asyncFn()
+  } catch (error) {
+    f = () => { throw error }
+  } finally {
+    assert.throws(f, regExp)
+  }
+}