Merge branch 'master' into i1089-networkfront

CHANGELOG.md

@@ -5,8 +5,15 @@
 - Test suite for migrations expanded.
 - Network now changeable from lock screen.
 
+- Improve test coverage of eth.sign behavior, including a code example of verifying a signature.
+
+## 3.2.2 2017-2-8
+
+- Revert eth.sign behavior to the previous one with a big warning.  We will be gradually implementing the new behavior over the coming time. https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_sign
+
 ## 3.2.1 2017-2-8
 
+- Revert back to old style message signing.
 - Fixed some build errors that were causing a variety of bugs.
 
 ## 3.2.0 2017-2-8

app/manifest.json

@@ -1,7 +1,7 @@
 {
   "name": "MetaMask",
   "short_name": "Metamask",
-  "version": "3.2.1",
+  "version": "3.2.2",
   "manifest_version": 2,
   "author": "https://metamask.io",
   "description": "Ethereum Browser Extension",

app/scripts/keyrings/hd.js

@@ -74,7 +74,18 @@ class HdKeyring extends EventEmitter {
   }
 
   // For eth_sign, we need to sign transactions:
-  signMessage (withAccount, msgHex) {
+  // hd
+  signMessage (withAccount, data) {
+    const wallet = this._getWalletForAccount(withAccount)
+    const message = ethUtil.stripHexPrefix(data)
+    var privKey = wallet.getPrivateKey()
+    var msgSig = ethUtil.ecsign(new Buffer(message, 'hex'), privKey)
+    var rawMsgSig = ethUtil.bufferToHex(sigUtil.concatSig(msgSig.v, msgSig.r, msgSig.s))
+    return Promise.resolve(rawMsgSig)
+  }
+
+  // For eth_sign, we need to sign transactions:
+  newGethSignMessage (withAccount, msgHex) {
     const wallet = this._getWalletForAccount(withAccount)
     const privKey = wallet.getPrivateKey()
     const msgBuffer = ethUtil.toBuffer(msgHex)
@@ -101,9 +112,11 @@ class HdKeyring extends EventEmitter {
 
 
   _getWalletForAccount (account) {
+    const targetAddress = sigUtil.normalize(account)
     return this.wallets.find((w) => {
       const address = w.getAddress().toString('hex')
-      return ((address === account) || (sigUtil.normalize(address) === account))
+      return ((address === targetAddress) ||
+              (sigUtil.normalize(address) === targetAddress))
     })
   }
 }

app/scripts/keyrings/simple.js

@@ -58,7 +58,18 @@ class SimpleKeyring extends EventEmitter {
   }
 
   // For eth_sign, we need to sign transactions:
-  signMessage (withAccount, msgHex) {
+  signMessage (withAccount, data) {
+    const wallet = this._getWalletForAccount(withAccount)
+    const message = ethUtil.stripHexPrefix(data)
+    var privKey = wallet.getPrivateKey()
+    var msgSig = ethUtil.ecsign(new Buffer(message, 'hex'), privKey)
+    var rawMsgSig = ethUtil.bufferToHex(sigUtil.concatSig(msgSig.v, msgSig.r, msgSig.s))
+    return Promise.resolve(rawMsgSig)
+  }
+
+  // For eth_sign, we need to sign transactions:
+
+  newGethSignMessage (withAccount, msgHex) {
     const wallet = this._getWalletForAccount(withAccount)
     const privKey = wallet.getPrivateKey()
     const msgBuffer = ethUtil.toBuffer(msgHex)
@@ -77,7 +88,8 @@ class SimpleKeyring extends EventEmitter {
   /* PRIVATE METHODS */
 
   _getWalletForAccount (account) {
-    let wallet = this.wallets.find(w => ethUtil.bufferToHex(w.getAddress()) === account)
+    const address = sigUtil.normalize(account)
+    let wallet = this.wallets.find(w => ethUtil.bufferToHex(w.getAddress()) === address)
     if (!wallet) throw new Error('Simple Keyring - Unable to find matching address.')
     return wallet
   }

test/unit/id-management-test.js

@@ -16,7 +16,7 @@ describe('IdManagement', function() {
   })
 
   describe('#signMsg', function () {
-    it.skip('passes the dennis test', function() {
+    it('passes the dennis test', function() {
       const address = '0x9858e7d8b79fc3e6d989636721584498926da38a'
       const message = '0x879a053d4800c6354e76c7985a865d2922c82fb5b3f4577b2fe08b998954f2e0'
       const privateKey = '0x7dd98753d7b4394095de7d176c58128e2ed6ee600abe97c9f6d9fd65015d9b18'

test/unit/keyrings/simple-test.js

@@ -1,5 +1,7 @@
 const assert = require('assert')
 const extend = require('xtend')
+const Web3 = require('web3')
+const web3 = new Web3()
 const ethUtil = require('ethereumjs-util')
 const SimpleKeyring = require('../../../app/scripts/keyrings/simple')
 const TYPE_STR = 'Simple Key Pair'
@@ -55,7 +57,7 @@ describe('simple-keyring', function() {
     const privateKey = '0x7dd98753d7b4394095de7d176c58128e2ed6ee600abe97c9f6d9fd65015d9b18'
     const expectedResult = '0x28fcb6768e5110144a55b2e6ce9d1ea5a58103033632d272d2b5cf506906f7941a00b539383fd872109633d8c71c404e13dba87bc84166ee31b0e36061a69e161c'
 
-    it.skip('passes the dennis test', function(done) {
+    it('passes the dennis test', function(done) {
       keyring.deserialize([ privateKey ])
       .then(() => {
         return keyring.signMessage(address, message)
@@ -65,6 +67,44 @@ describe('simple-keyring', function() {
         done()
       })
     })
+
+    it('reliably can decode messages it signs', function (done) {
+
+      const message = 'hello there!'
+      const msgHashHex = web3.sha3(message)
+      let address
+      let addresses = []
+
+      keyring.deserialize([ privateKey ])
+      .then(() => {
+        keyring.addAccounts(9)
+      })
+      .then(() => {
+        return keyring.getAccounts()
+      })
+      .then((addrs) => {
+        addresses = addrs
+        return Promise.all(addresses.map((address) => {
+          return keyring.signMessage(address, msgHashHex)
+        }))
+      })
+      .then((signatures) => {
+
+        signatures.forEach((sgn, index) => {
+          const address = addresses[index]
+
+          var r = ethUtil.toBuffer(sgn.slice(0,66))
+          var s = ethUtil.toBuffer('0x' + sgn.slice(66,130))
+          var v = ethUtil.bufferToInt(ethUtil.toBuffer('0x' + sgn.slice(130,132)))
+          var m = ethUtil.toBuffer(msgHashHex)
+          var pub = ethUtil.ecrecover(m, v, r, s)
+          var adr = '0x' + ethUtil.pubToAddress(pub).toString('hex')
+
+          assert.equal(adr, address, 'recovers address from signature correctly')
+        })
+        done()
+      })
+    })
   })
 
   describe('#addAccounts', function() {

ui/app/components/pending-msg.js

@@ -28,6 +28,15 @@ PendingMsg.prototype.render = function () {
         },
       }, 'Sign Message'),
 
+      h('.error', {
+        style: {
+          margin: '10px',
+        },
+      }, `Signing this message can have
+        dangerous side effects. Only sign messages from
+        sites you fully trust with your entire account.
+        This will be fixed in a future version.`),
+
       // message details
       h(PendingTxDetails, state),