From af3cb84ff9229f85a8ea4a8c9c71511912ea0947 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 24 May 2017 18:06:58 +0200 Subject: Add isSignatureValid method and tests for it --- package.json | 3 +++ src/ts/0x.js.ts | 32 +++++++++++++++++++++++++++++--- src/ts/globals.d.ts | 1 + test/0x.js.ts | 50 ++++++++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 79 insertions(+), 7 deletions(-) create mode 100644 src/ts/globals.d.ts diff --git a/package.json b/package.json index 340e042ce..091d54f7e 100644 --- a/package.json +++ b/package.json @@ -46,5 +46,8 @@ "typedoc": "^0.7.1", "typescript": "^2.3.3", "webpack": "^2.6.0" + }, + "dependencies": { + "ethereumjs-util": "^5.1.1" } } diff --git a/src/ts/0x.js.ts b/src/ts/0x.js.ts index 95446ad74..6d6d5fed6 100644 --- a/src/ts/0x.js.ts +++ b/src/ts/0x.js.ts @@ -1,6 +1,32 @@ +import * as ethUtil from 'ethereumjs-util'; + +/** + * Elliptic Curve signature + */ +export interface ECSignature { + v: number; + r: string; + s: string; +} + +export type ETHAddress = string; + export class ZeroEx { - /** Verifies the signature */ - public verifySignature() { - // TODO + /** + * Checks if the signature is valid + */ + public static isValidSignature(data: string, signature: ECSignature, signer: ETHAddress): boolean { + const dataBuff = ethUtil.toBuffer(data); + const msgHashBuff = ethUtil.hashPersonalMessage(dataBuff); + try { + const pubKey = ethUtil.ecrecover(msgHashBuff, + signature.v, + ethUtil.toBuffer(signature.r), + ethUtil.toBuffer(signature.s)); + const retrievedAddress = ethUtil.bufferToHex(ethUtil.pubToAddress(pubKey)); + return retrievedAddress === signer; + } catch (err) { + return false; + } } } diff --git a/src/ts/globals.d.ts b/src/ts/globals.d.ts new file mode 100644 index 000000000..99f9cf50b --- /dev/null +++ b/src/ts/globals.d.ts @@ -0,0 +1 @@ +declare module 'ethereumjs-util'; diff --git a/test/0x.js.ts b/test/0x.js.ts index 65475bf32..7216ed34f 100644 --- a/test/0x.js.ts +++ b/test/0x.js.ts @@ -3,10 +3,52 @@ import {expect} from 'chai'; import 'mocha'; describe('ZeroEx library', () => { - describe('#verifySignature', () => { - it('should return undefined', () => { - const zeroEx = new ZeroEx(); - expect(zeroEx.verifySignature()).to.be.undefined; + describe('#isValidSignature', () => { + const data = '0xdeadbeaf'; + const signature = { + v: 27, + r: '0xa3f20717a250c2b0b729b7e5becbff67fdaef7e0699da4de7ca5895b02a170a1', + s: '0x2d887fd3b17bfdce3481f10bea41f45ba9f709d39ce8325427b57afcfc994cee', + }; + const address = '0x9b2055d370f73ec7d8a03e965129118dc8f5bf83'; + describe('should return false for malformed signature', () => { + it('malformed v', () => { + const malformedSignature = { + v: 34, + r: signature.r, + s: signature.s, + }; + const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); + expect(isValid).to.be.false; + }); + it('malformed r', () => { + const malformedSignature = { + v: signature.v, + r: signature.r.replace('0x', ''), + s: signature.s, + }; + const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); + expect(isValid).to.be.false; + }); + }); + describe('should return false for invalid signature', () => { + it('wrong data', () => { + const isValid = ZeroEx.isValidSignature('wrong data', signature, address); + expect(isValid).to.be.false; + }); + it('wrong signer', () => { + const isValid = ZeroEx.isValidSignature(data, signature, '0xIamWrong'); + expect(isValid).to.be.false; + }); + it('wrong signature', () => { + const wrongSignature = Object.assign({}, signature, {v: 28}); + const isValid = ZeroEx.isValidSignature(data, wrongSignature, address); + expect(isValid).to.be.false; + }); + }); + it('should return true for valid signature', () => { + const isValid = ZeroEx.isValidSignature(data, signature, address); + expect(isValid).to.be.true; }); }); }); -- cgit v1.2.3 From 568b9fe742e87b237403cea402b5b31deb548abe Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 24 May 2017 18:19:02 +0200 Subject: Define node version in circleCI --- circle.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/circle.yml b/circle.yml index 7a97d756c..65db7c29a 100644 --- a/circle.yml +++ b/circle.yml @@ -1,3 +1,7 @@ +machine: + node: + version: 6.1.0 + test: override: - npm run test:coverage -- cgit v1.2.3 From 6470cf54e389b45e82a08ad08225d2bb0f35ac4c Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 24 May 2017 18:49:19 +0200 Subject: Type the ethereumjs-util --- src/ts/0x.js.ts | 4 +--- src/ts/globals.d.ts | 12 +++++++++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/ts/0x.js.ts b/src/ts/0x.js.ts index 6d6d5fed6..3b16b643b 100644 --- a/src/ts/0x.js.ts +++ b/src/ts/0x.js.ts @@ -9,13 +9,11 @@ export interface ECSignature { s: string; } -export type ETHAddress = string; - export class ZeroEx { /** * Checks if the signature is valid */ - public static isValidSignature(data: string, signature: ECSignature, signer: ETHAddress): boolean { + public static isValidSignature(data: string, signature: ECSignature, signer: ETHAddressHex): boolean { const dataBuff = ethUtil.toBuffer(data); const msgHashBuff = ethUtil.hashPersonalMessage(dataBuff); try { diff --git a/src/ts/globals.d.ts b/src/ts/globals.d.ts index 99f9cf50b..1c3cbf47d 100644 --- a/src/ts/globals.d.ts +++ b/src/ts/globals.d.ts @@ -1 +1,11 @@ -declare module 'ethereumjs-util'; +declare type PubKey = string; +declare type ETHAddressHex = string; +declare type ETHAddressBuff = Buffer; + +declare module 'ethereumjs-util' { + const toBuffer: (data: string) => Buffer; + const hashPersonalMessage: (msg: Buffer) => Buffer; + const bufferToHex: (buff: Buffer) => string; + const ecrecover: (msgHashBuff: Buffer, v: number, r: Buffer, s: Buffer) => PubKey; + const pubToAddress: (pubKey: PubKey) => ETHAddressBuff; +} -- cgit v1.2.3 From 7a566c6988d48cbfffb5a8946751ea0a644c5c6a Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 24 May 2017 18:50:14 +0200 Subject: Add test vector source --- test/0x.js.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/0x.js.ts b/test/0x.js.ts index 7216ed34f..9783e69b7 100644 --- a/test/0x.js.ts +++ b/test/0x.js.ts @@ -4,6 +4,7 @@ import 'mocha'; describe('ZeroEx library', () => { describe('#isValidSignature', () => { + // Source: https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_sign const data = '0xdeadbeaf'; const signature = { v: 27, -- cgit v1.2.3 From 522300c0ab05b6f8f4c2874b2d0a690196715165 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 24 May 2017 18:58:31 +0200 Subject: change tests descriptions --- test/0x.js.ts | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/test/0x.js.ts b/test/0x.js.ts index 9783e69b7..651407af3 100644 --- a/test/0x.js.ts +++ b/test/0x.js.ts @@ -22,31 +22,40 @@ describe('ZeroEx library', () => { const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); expect(isValid).to.be.false; }); - it('malformed r', () => { + it('r lacks 0x prefix', () => { + const malformedR = signature.r.replace('0x', ''); const malformedSignature = { v: signature.v, - r: signature.r.replace('0x', ''), + r: malformedR, s: signature.s, }; const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); expect(isValid).to.be.false; }); - }); - describe('should return false for invalid signature', () => { - it('wrong data', () => { - const isValid = ZeroEx.isValidSignature('wrong data', signature, address); - expect(isValid).to.be.false; - }); - it('wrong signer', () => { - const isValid = ZeroEx.isValidSignature(data, signature, '0xIamWrong'); - expect(isValid).to.be.false; - }); - it('wrong signature', () => { - const wrongSignature = Object.assign({}, signature, {v: 28}); - const isValid = ZeroEx.isValidSignature(data, wrongSignature, address); + it('r is too short', () => { + const malformedR = signature.r.substr(10); + const malformedSignature = { + v: signature.v, + r: malformedR, + s: signature.s, + }; + const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); expect(isValid).to.be.false; }); }); + it('should return false if the data doesn\'t pertain to the signature & address', () => { + const isValid = ZeroEx.isValidSignature('wrong data', signature, address); + expect(isValid).to.be.false; + }); + it('should return false if the address doesn\'t pertain to the signature & data', () => { + const isValid = ZeroEx.isValidSignature(data, signature, '0xIamWrong'); + expect(isValid).to.be.false; + }); + it('should return false if the signature doesn\'t pertain to the data & address', () => { + const wrongSignature = Object.assign({}, signature, {v: 28}); + const isValid = ZeroEx.isValidSignature(data, wrongSignature, address); + expect(isValid).to.be.false; + }); it('should return true for valid signature', () => { const isValid = ZeroEx.isValidSignature(data, signature, address); expect(isValid).to.be.true; -- cgit v1.2.3 From 762db7961e3592faa277b60f25173a6a7874d5c4 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 24 May 2017 19:01:19 +0200 Subject: Add test for malformed s --- test/0x.js.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/0x.js.ts b/test/0x.js.ts index 651407af3..3d9a7a810 100644 --- a/test/0x.js.ts +++ b/test/0x.js.ts @@ -42,6 +42,16 @@ describe('ZeroEx library', () => { const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); expect(isValid).to.be.false; }); + it('s is not hex', () => { + const malformedS = signature.s.replace('0', 'z'); + const malformedSignature = { + v: signature.v, + r: signature.r, + s: malformedS, + }; + const isValid = ZeroEx.isValidSignature(data, malformedSignature, address); + expect(isValid).to.be.false; + }); }); it('should return false if the data doesn\'t pertain to the signature & address', () => { const isValid = ZeroEx.isValidSignature('wrong data', signature, address); -- cgit v1.2.3 From d8670e5768a97aa8068093dfbb261ef983c99b00 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 24 May 2017 19:06:07 +0200 Subject: Add node types --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 091d54f7e..2f345a3f1 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ "devDependencies": { "@types/chai": "^3.5.2", "@types/mocha": "^2.2.41", + "@types/node": "^7.0.22", "awesome-typescript-loader": "^3.1.3", "chai": "^3.5.0", "mocha": "^3.4.1", -- cgit v1.2.3 From 945a583e895dfd9488aecdfab1bec22449bf7878 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Wed, 24 May 2017 19:23:35 +0200 Subject: Address feedback --- src/ts/0x.js.ts | 3 ++- src/ts/globals.d.ts | 6 +++--- test/0x.js.ts | 3 ++- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/ts/0x.js.ts b/src/ts/0x.js.ts index 3b16b643b..dd67c49a0 100644 --- a/src/ts/0x.js.ts +++ b/src/ts/0x.js.ts @@ -11,7 +11,8 @@ export interface ECSignature { export class ZeroEx { /** - * Checks if the signature is valid + * Verifies that the elliptic curve signature `signature` was generated + * by signing `data` with the private key corresponding to the `signer` address. */ public static isValidSignature(data: string, signature: ECSignature, signer: ETHAddressHex): boolean { const dataBuff = ethUtil.toBuffer(data); diff --git a/src/ts/globals.d.ts b/src/ts/globals.d.ts index 1c3cbf47d..0f7391b39 100644 --- a/src/ts/globals.d.ts +++ b/src/ts/globals.d.ts @@ -1,4 +1,4 @@ -declare type PubKey = string; +declare type ETHPublicKey = string; declare type ETHAddressHex = string; declare type ETHAddressBuff = Buffer; @@ -6,6 +6,6 @@ declare module 'ethereumjs-util' { const toBuffer: (data: string) => Buffer; const hashPersonalMessage: (msg: Buffer) => Buffer; const bufferToHex: (buff: Buffer) => string; - const ecrecover: (msgHashBuff: Buffer, v: number, r: Buffer, s: Buffer) => PubKey; - const pubToAddress: (pubKey: PubKey) => ETHAddressBuff; + const ecrecover: (msgHashBuff: Buffer, v: number, r: Buffer, s: Buffer) => ETHPublicKey; + const pubToAddress: (pubKey: ETHPublicKey) => ETHAddressBuff; } diff --git a/test/0x.js.ts b/test/0x.js.ts index 3d9a7a810..d59df8894 100644 --- a/test/0x.js.ts +++ b/test/0x.js.ts @@ -4,6 +4,7 @@ import 'mocha'; describe('ZeroEx library', () => { describe('#isValidSignature', () => { + // This test data was borrowed from the JSON RPC documentation // Source: https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_sign const data = '0xdeadbeaf'; const signature = { @@ -66,7 +67,7 @@ describe('ZeroEx library', () => { const isValid = ZeroEx.isValidSignature(data, wrongSignature, address); expect(isValid).to.be.false; }); - it('should return true for valid signature', () => { + it('should return true if the signature does pertain to the data & address', () => { const isValid = ZeroEx.isValidSignature(data, signature, address); expect(isValid).to.be.true; }); -- cgit v1.2.3