Add missing tests to ECSDA (#1248)

* fix: refactor sign.js and related tests

* fix: remove unused dep

* fix: update package.json correctly

* Add missing tests to ECRecovery

* fix lint

* Reorganize the tests

* Reuse signature

* fix static errors

* Apply suggestions by @frangion and @nventuro

* Remove only

* More suggestions

* Remove unnecessary max-len

* remove only

test/library/ECDSA.test.js

@@ -11,56 +11,117 @@ const WRONG_MESSAGE = web3.sha3('Nope');
 
 contract('ECDSA', function ([_, anyone]) {
   beforeEach(async function () {
-    this.mock = await ECDSAMock.new();
+    this.ecdsa = await ECDSAMock.new();
   });
 
-  it('recover v0', async function () {
-    // Signature generated outside ganache with method web3.eth.sign(signer, message)
-    const signer = '0x2cc1166f6212628a0deef2b33befb2187d35b86c';
-    // eslint-disable-next-line max-len
-    const signature = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be89200';
-    (await this.mock.recover(TEST_MESSAGE, signature)).should.equal(signer);
-  });
+  context('recover with valid signature', function () {
+    context('with v0 signature', function () {
+      // Signature generated outside ganache with method web3.eth.sign(signer, message)
+      const signer = '0x2cc1166f6212628a0deef2b33befb2187d35b86c';
+      // eslint-disable-next-line max-len
+      const signatureWithoutVersion = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be892';
 
-  it('recover v1', async function () {
-    // Signature generated outside ganache with method web3.eth.sign(signer, message)
-    const signer = '0x1e318623ab09fe6de3c9b8672098464aeda9100e';
-    // eslint-disable-next-line max-len
-    const signature = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e001';
-    (await this.mock.recover(TEST_MESSAGE, signature)).should.equal(signer);
-  });
+      context('with 00 as version value', function () {
+        it('works', async function () {
+          const version = '00';
+          const signature = signatureWithoutVersion + version;
+          (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(signer);
+        });
+      });
 
-  it('recover using web3.eth.sign()', async function () {
-    // Create the signature
-    const signature = signMessage(anyone, TEST_MESSAGE);
+      context('with 27 as version value', function () {
+        it('works', async function () {
+          const version = '1b'; // 27 = 1b.
+          const signature = signatureWithoutVersion + version;
+          (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(signer);
+        });
+      });
 
-    // Recover the signer address from the generated message and signature.
-    (await this.mock.recover(
-      toEthSignedMessageHash(TEST_MESSAGE),
-      signature
-    )).should.equal(anyone);
-  });
+      context('with wrong version', function () {
+        it('returns 0', async function () {
+          // The last two hex digits are the signature version.
+          // The only valid values are 0, 1, 27 and 28.
+          const version = '02';
+          const signature = signatureWithoutVersion + version;
+          (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(
+            '0x0000000000000000000000000000000000000000');
+        });
+      });
+    });
 
-  it('recover using web3.eth.sign() should return wrong signer', async function () {
-    // Create the signature
-    const signature = signMessage(anyone, TEST_MESSAGE);
+    context('with v1 signature', function () {
+      const signer = '0x1e318623ab09fe6de3c9b8672098464aeda9100e';
+      // eslint-disable-next-line max-len
+      const signatureWithoutVersion = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e0';
 
-    // Recover the signer address from the generated message and wrong signature.
-    (await this.mock.recover(WRONG_MESSAGE, signature)).should.not.equal(anyone);
+      context('with 01 as version value', function () {
+        it('works', async function () {
+          const version = '01';
+          const signature = signatureWithoutVersion + version;
+          (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(signer);
+        });
+      });
+
+      context('with 28 signature', function () {
+        it('works', async function () {
+          const version = '1c'; // 28 = 1c.
+          const signature = signatureWithoutVersion + version;
+          (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(signer);
+        });
+      });
+
+      context('with wrong version', function () {
+        it('returns 0', async function () {
+          // The last two hex digits are the signature version.
+          // The only valid values are 0, 1, 27 and 28.
+          const version = '02';
+          const signature = signatureWithoutVersion + version;
+          (await this.ecdsa.recover(TEST_MESSAGE, signature)).should.equal(
+            '0x0000000000000000000000000000000000000000');
+        });
+      });
+    });
+
+    context('using web3.eth.sign', function () {
+      context('with correct signature', function () {
+        it('returns signer address', async function () {
+          // Create the signature
+          const signature = signMessage(anyone, TEST_MESSAGE);
+
+          // Recover the signer address from the generated message and signature.
+          (await this.ecdsa.recover(
+            toEthSignedMessageHash(TEST_MESSAGE),
+            signature
+          )).should.equal(anyone);
+        });
+      });
+
+      context('with wrong signature', function () {
+        it('does not return signer address', async function () {
+          // Create the signature
+          const signature = signMessage(anyone, TEST_MESSAGE);
+
+          // Recover the signer address from the generated message and wrong signature.
+          (await this.ecdsa.recover(WRONG_MESSAGE, signature)).should.not.equal(anyone);
+        });
+      });
+    });
   });
 
-  // @TODO - remove `skip` once we upgrade to solc^0.5
-  it.skip('recover should revert when a small hash is sent', async function () {
-    // Create the signature
-    const signature = signMessage(anyone, TEST_MESSAGE);
-    await expectThrow(
-      this.mock.recover(TEST_MESSAGE.substring(2), signature)
-    );
+  context('with small hash', function () {
+    // @TODO - remove `skip` once we upgrade to solc^0.5
+    it.skip('reverts', async function () {
+      // Create the signature
+      const signature = signMessage(anyone, TEST_MESSAGE);
+      await expectThrow(
+        this.ecdsa.recover(TEST_MESSAGE.substring(2), signature)
+      );
+    });
   });
 
   context('toEthSignedMessage', function () {
     it('should prefix hashes correctly', async function () {
-      (await this.mock.toEthSignedMessageHash(TEST_MESSAGE)).should.equal(toEthSignedMessageHash(TEST_MESSAGE));
+      (await this.ecdsa.toEthSignedMessageHash(TEST_MESSAGE)).should.equal(toEthSignedMessageHash(TEST_MESSAGE));
     });
   });
 });