Skip to content
Prev Previous commit
Next Next commit
Add missing tests to ECRecovery
  • Loading branch information
Leo Arias committed Aug 29, 2018
commit 1be1641a17281c06db6bd911de42a1af2706a18f
27 changes: 27 additions & 0 deletions test/library/ECRecovery.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,33 @@ contract('ECRecovery', function ([_, anyone]) {
(await this.mock.recover(TEST_MESSAGE, signature)).should.equal(signer);
});

it('recover v27', async function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, these may read more clearly with a structure like the following one:

context with valid signature
  context with v27 signature
    it works
  context with v28 signature
    it works
  context with invalid version signature
    it reverts
context with invalid signature
  it reverts

(the test itself should probably be abstracted into a function to avoid repetition)

// Signature generated outside ganache with method web3.eth.sign(signer, message)
const signer = '0x2cc1166f6212628a0deef2b33befb2187d35b86c';
// eslint-disable-next-line max-len
// The last two hex digits are the signature version: 27 = 1b.
const signature = '0x5d99b6f7f6d1f73d1a26497f2b1c89b24c0993913f86e9a2d02cd69887d9c94f3c880358579d811b21dd1b7fd9bb01c1d81d10e69f0384e675c32b39643be8921b';
(await this.mock.recover(TEST_MESSAGE, signature)).should.equal(signer);
});

it('recover v28', async function () {
// Signature generated outside ganache with method web3.eth.sign(signer, message)
const signer = '0x1e318623ab09fe6de3c9b8672098464aeda9100e';
// eslint-disable-next-line max-len
// The last two hex digits are the signature version: 28 = 1c.
const signature = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e01c';
(await this.mock.recover(TEST_MESSAGE, signature)).should.equal(signer);
});

it('wrong signature version', async function () {
// eslint-disable-next-line max-len
// The last two hex digits are the signature version.
// The only valid values are 0, 1, 27 and 28.
const signature = '0x331fe75a821c982f9127538858900d87d3ec1f9f737338ad67cad133fa48feff48e6fa0c18abc62e42820f05943e47af3e9fbe306ce74d64094bdf1691ee53e002';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this signature different from the others? Shouldn't just the version differ? If so, I'd abstract out the signature prefix, and add the suffix in each test.

But more importantly, why is the signature hardcoded and not generated in the test? :/

(await this.mock.recover(TEST_MESSAGE, signature)).should.equal(
'0x0000000000000000000000000000000000000000');
});

it('recover using web3.eth.sign()', async function () {
// Create the signature
const signature = signMessage(anyone, TEST_MESSAGE);
Expand Down