Fixed tests and few changes on contract comments

OpenZeppelin · Jul 18, 2017 · f8c0fab · f8c0fab
1 parent 478d1b1
commit f8c0fab
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 8 deletions.
diff --git a/contracts/ECRecovery.sol b/contracts/ECRecovery.sol
@@ -11,8 +11,8 @@ library ECRecovery {
 
   /**
    * @dev Recover signer address from a message by using his signature
-   * @param hash bytes32 message hash from which the signature will be recovered
-   * @param sig bytes signature, the siganture is generated using web3.eth.sign()
+   * @param hash bytes32 message, the hash is the signed message. What is recovered is the signer address.
+   * @param sig bytes signature, the signature is generated using web3.eth.sign()
    */
   function recover(bytes32 hash, bytes sig) constant returns (address) {
     bytes32 r;

diff --git a/test/ECRecovery.js b/test/ECRecovery.js
@@ -1,4 +1,5 @@
 var ECRecovery = artifacts.require("../contracts/ECRecovery.sol");
+var utils = require('ethereumjs-util');
 
 contract('ECRecovery', function(accounts) {
 
@@ -9,29 +10,61 @@ contract('ECRecovery', function(accounts) {
   });
 
   it("recover v0", async function() {
+    // Signature generated outside testrpc with method web3.eth.sign(signer, message)
     let signer = '0x2cc1166f6212628a0deef2b33befb2187d35b86c';
     let message = '0x7dbaf558b0a1a5dc7a67202117ab143c1d8605a983e4a743bc06fcc03162dc0d'; // web3.sha3('OpenZeppelin')
     let signature = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be89200';
     assert.equal(signer, await ecrecovery.recover(message, signature));
   });
 
   it("recover v1", async function() {
+    // Signature generated outside testrpc with method web3.eth.sign(signer, message)
     let signer = '0x1e318623ab09fe6de3c9b8672098464aeda9100e';
     let message = '0x7dbaf558b0a1a5dc7a67202117ab143c1d8605a983e4a743bc06fcc03162dc0d'; // web3.sha3('OpenZeppelin')
     let signature = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e001';
     assert.equal(signer, await ecrecovery.recover(message, signature));
   });
 
   it("recover using web3.eth.sign()", async function() {
-    let message = web3.sha3('OpenZeppelin');
-    let signature = web3.eth.sign(web3.eth.accounts[0], 'OpenZeppelin');
-    assert.equal(web3.eth.accounts[0], await ecrecovery.recover(message, signature));
+    // Create the signature using account[0]
+    const signature = web3.eth.sign(web3.eth.accounts[0], web3.sha3('OpenZeppelin'));
+
+    // Testrpc add a prefix to the signed message, we generate the hash of
+    // 'OpenZeppelin' string repeating testrpc steps
+    const message = new Buffer(web3.sha3('OpenZeppelin').substring(2), 'hex');
+    const prefix = utils.toBuffer('\u0019Ethereum Signed Message:\n' + message.length.toString());
+    const prefixedHash = utils.bufferToHex( utils.sha3(Buffer.concat([prefix, message])) );
+
+    // Recover the signer address form the generated message and signature.
+    assert.equal(web3.eth.accounts[0], await ecrecovery.recover(prefixedHash, signature));
   });
 
   it("recover using web3.eth.sign() should return wrong signer", async function() {
-    let message = web3.sha3('OpenZeppelin');
-    let signature = web3.eth.sign(web3.eth.accounts[0], message);
-    assert.notEqual(web3.eth.accounts[0], await ecrecovery.recover(web3.sha3('OpenZeppelin2'), signature));
+    // Create the signature using account[0]
+    const signature = web3.eth.sign(web3.eth.accounts[0], web3.sha3('OpenZeppelin'));
+
+    // Testrpc add a prefix to the signed message, we generate the hash of
+    // 'Test' string repeating testrpc steps
+    const message = new Buffer(web3.sha3('Test').substring(2), 'hex');
+    const prefix = utils.toBuffer('\u0019Ethereum Signed Message:\n' + message.length.toString());
+    const prefixedHash = utils.bufferToHex( utils.sha3(Buffer.concat([prefix, message])) );
+
+    // Recover the signer address form the generated message and wrong signature.
+    assert.notEqual(web3.eth.accounts[0], await ecrecovery.recover(prefixedHash, signature));
+  });
+
+  it("recover should fail when a wrong hash is sent", async function() {
+    // Create the signature using account[0]
+    let signature = web3.eth.sign(web3.eth.accounts[0], web3.sha3('OpenZeppelin'));
+
+    // Testrpc add a prefix to the signed message, we generate the hash of
+    // 'OpenZeppelin' string repeating testrpc steps
+    let message = new Buffer(web3.sha3('OpenZeppelin'), 'hex');
+    let prefix = utils.toBuffer('\u0019Ethereum Signed Message:\n' + message.length.toString());
+    let prefixedHash = utils.sha3(Buffer.concat([prefix, message]));
+
+    // Recover the signer address form the generated message and wrong signature.
+    assert.equal('0x0000000000000000000000000000000000000000', await ecrecovery.recover(prefixedHash, signature));
   });
 
 });